Skip to content

[o365] Convert stringified numbers back to numbers in the audit pipeline - #21875

Merged
chrisberkhout merged 4 commits into
elastic:mainfrom
chrisberkhout:o365-numbers-back-in-_source
Oct 13, 2026
Merged

chrisberkhout merged 4 commits into
elastic:mainfrom
chrisberkhout:o365-numbers-back-in-_source

Conversation

@chrisberkhout

Copy link
Copy Markdown
Contributor

Proposed commit message

[o365] Convert stringified numbers back to numbers in the audit pipeline

Since 3.8.2 (#19245) the CEL input has used decode_json_string_numbers()
so that large integers survive CEL without float64 rounding. The pipeline
passed those strings through, so every value the API sends as a JSON
number reached Elasticsearch as a string. Fields mapped as long were
still indexed as numbers by coercion, but _source held a string; fields
mapped as keyword or flattened were indexed as strings where they had
been numbers; and file.size, which had no explicit mapping, was
dynamically mapped as keyword instead of long (#21137).

Add a script_convert_numbers processor that converts a fixed list of
fields back to numbers. A string that parses as a number becomes a
long, or a double if it is fractional or too large; anything else is
left unchanged, so an unparseable value in a long field still fails at
index time as it did before. The CEL program is unchanged. The list
covers the fields mapped as long, including the two numeric keys of
ExchangeMetaData that were never converted before, and the fields the
API sends as numbers but that are mapped as keyword or flattened and
were numbers before 3.8.2 only because CEL passed them through.

The string normalisation of UserType, AzureActiveDirectoryEventType,
ListBaseType and Actor/Target[].Type that #19245 dropped is reinstated
in script_normalize_numbers, so the legacy o365audit input, which still
sends real numbers, produces the same output as the CEL input.

The output matches the pre-3.8.2 output except that:

- FileSize becomes a long where it was a double, and an API string such
  as "1000.0" is converted too; the pipeline cannot tell an API string
  from an API number once CEL has stringified both.

- ID fields that happen to look numeric (UserId, OperationId, RequestId,
  WorkspaceId and their copies event.id, host.id and organization.id)
  stay strings, since an ID such as "0123" must not be altered. Real
  values are UPNs or GUIDs.

- event.original keeps the numbers quoted, because that is how the CEL
  input delivers them.

What does this PR do?

See the commit message. For reviewers:

  • The regenerated pipeline test output was compared value by value against the expected files from before [o365] Keep numbers as strings through to Elasticsearch #19245 (git show 01f43440d6^:...). The only differences are the three deliberate ones in the commit message and unrelated whitespace trimming in email.attachments.file.* from o365: fix Painless regex character-limit and Sender mapping errors #19801.
  • Paths are listed explicitly rather than converting every numeric-looking string under objects like Actions, PolicyDetails or ExchangeMetaData, which would also catch IDs and other values the API sends as strings. Numeric keys not in the list stay strings (under ExchangeMetaData they are still coerced by the long wildcard mapping). A path applies to every element wherever a list is found along it, and a final * covers every key of a map; * is only used for the CountBy* maps, where every key is a count.
  • Independent of [o365] Add explicit ECS mapping for file.size #21868, which adds the explicit file.size mapping, but whichever merges second needs its version bumped to 3.11.3.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

How to test this PR locally

cd packages/o365
elastic-package check
elastic-package test pipeline

Related issues

@chrisberkhout chrisberkhout self-assigned this Oct 6, 2026
@chrisberkhout
chrisberkhout requested review from a team as code owners October 6, 2026 15:06
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

✅ Elastic Docs Style Checker (Vale)

No issues found on modified lines!


The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale.

@chrisberkhout chrisberkhout added Integration:o365 Microsoft Office 365 bugfix Pull request that fixes a bug issue Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] labels Oct 6, 2026
@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

Comment thread packages/o365/changelog.yml Outdated
@@ -1,4 +1,12 @@
# newer versions go on top
- version: "3.11.2"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is a breaking change for the same reason the previous change here was.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay.

If we said the earlier _source changes were an unintentional breakage, then I'd call reverting those a bug fix rather than a new breaking change.

But you could say they're two intentional breaking changes.

Also this doesn't fully revert. Some things that should be strings remain strings.

In an case, I've given it a major version bump and marked it as a breaking change in the changelog.

@chrisberkhout chrisberkhout added breaking change and removed bugfix Pull request that fixes a bug issue labels Oct 7, 2026
@chrisberkhout
chrisberkhout requested a review from efd6 October 7, 2026 07:42
@vera-review-bot

Copy link
Copy Markdown
Contributor

🟢 No issues across the latest commits e639fc5.

Review summary

Issues found across earlier commits d6e67d4 — 1 low

Package-level:

  • 🔵 Proposed commit message

    o365: convert stringified numbers back to numbers in the audit pipeline
    
    Since 3.8.2 the CEL input has used decode_json_string_numbers() so that
    large integers survive CEL without float64 rounding. The audit pipeline
    passed those strings through, so every value the API sends as a JSON
    number reached Elasticsearch as a string. Fields mapped as long were
    still indexed as numbers by coercion but _source held a string; fields
    mapped as keyword or flattened were indexed as strings; and file.size,
    which had no explicit mapping, was dynamically mapped as keyword.
    
    Add a script_convert_numbers processor that converts a fixed list of
    field paths back to numbers. A string that parses as a number becomes a
    long, or a double if it is fractional or too large; anything else is
    left unchanged. The list covers the fields mapped as long, including
    the two numeric keys of ExchangeMetaData, and the fields the API sends
    as numbers but that are mapped as keyword or flattened. The CEL program
    is unchanged.
    
    The string normalisation of UserType, AzureActiveDirectoryEventType,
    ListBaseType and Actor/Target[].Type is reinstated in
    script_normalize_numbers so the legacy o365audit input, which still
    sends real numbers, produces the same output as the CEL input.
    
    Compared with the pre-3.8.2 output, FileSize becomes a long where it
    was a double, numeric-looking ID fields stay strings, and
    event.original keeps the numbers quoted as the CEL input delivers them.
    
    Relates #​21137, #​19245, #​21868
    

A new commit triggers another review — at most once every 15 minutes. I skip the PR while it's approved or has merge conflicts.

🤖 AI-Generated Review | Vera Review Bot - v0.4.2 | 📚 Knowledge base: integration-skills

⚠️ Automated review — verify suggestions before applying.

@elastic-vault-github-plugin-prod

Copy link
Copy Markdown
Contributor

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

- version: "3.11.2"
- version: "4.0.0"
changes:
- description: >-

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a breaking change, there should be the breaking change change, and the bug-fix/enhancement change. See https://github.com/elastic/integrations/wiki/Fleet-Package-Code-Review-Comments#use-of-bugfixenhancementbreaking-change.

The breaking change would say that some _source fields have changed type and there may be a requirement for updates to rules if they are referenced, and the bugfix would say that commonly used semantically numeric fields are now the expected type instead of strings.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Updated.

@efd6 efd6 mentioned this pull request Oct 13, 2026
5 tasks done
@chrisberkhout
chrisberkhout requested a review from efd6 October 13, 2026 07:37
@chrisberkhout
chrisberkhout enabled auto-merge (squash) October 13, 2026 07:48
@chrisberkhout
chrisberkhout force-pushed the o365-numbers-back-in-_source branch from 7117701 to c7d5175 Compare October 13, 2026 07:48
@elastic-vault-github-plugin-prod

Copy link
Copy Markdown
Contributor

✅ All changelog entries have the correct PR link.

@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

💚 Build Succeeded

History

cc @chrisberkhout

@mergify

mergify Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@chrisberkhout
chrisberkhout merged commit cbb6345 into elastic:main Oct 13, 2026
14 checks passed
@elastic-vault-github-plugin-prod

Copy link
Copy Markdown
Contributor

Package o365 - 4.0.0 containing this change is available at https://epr.elastic.co/package/o365/4.0.0/

brijesh-elastic added a commit that referenced this pull request Oct 28, 2026
…22177)

SharePoint and OneDrive list events (RecordType 36, such as ListViewed,
ListUpdated, ListColumnCreated and ListColumnUpdated) report
ListBaseType as a name such as "DocumentLibrary". Since 4.0.0 (#21875),
ListBaseType has been in the script_normalize_numbers field list, and
these events failed with:

  Character D is neither a decimal digit number, decimal point, nor "e"
  notation exponential mark.

The script treats any string that contains an "e" or "E" as a number in
scientific notation and passes it to
new BigDecimal(value).toBigIntegerExact(). The names contain an "e", so
BigDecimal threw, and the whole document was indexed as
event.kind: pipeline_error with only the raw o365audit.* fields.

Microsoft documents ListBaseType as a string enumeration, so remove it
from the list. Also catch NumberFormatException and ArithmeticException
in normalize() and return the value unchanged, the same way
script_convert_numbers handles values it cannot parse, so that a
non-integer string in another listed field no longer fails the whole
document. Numbers from the o365audit input, and numeric strings in
scientific notation from the CEL input, are normalized as before.

Add a SharePoint ListViewed event with a ListBaseType of
"DocumentLibrary" to the pipeline tests, and an event with a
non-integer UserType ("1.5e0") and a named LogonType ("Delegate") so
both catch branches are covered.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Integration:o365 Microsoft Office 365 Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants