[ssi] Fix missing event categorization for packages - #20276
[ssi] Fix missing event categorization for packages#20276brijesh-elastic wants to merge 11 commits into
Conversation
brijesh-elastic
added
the
documentation
brijesh-elastic
added
Integration:azure
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
✅ 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. |
![vera-review-bot[bot]](https://avatars.githubusercontent.com/in/3297022?s=60&v=4){kind=small}
| change_calendar_description: configuration | ||
| export_calendar: configuration | ||
| change_calendar_location: configuration | ||
| print_preview_calendar: configuration |
There was a problem hiding this comment.
Severity: 🟠 High confidence: high path: packages/google_workspace/data_stream/calendar/elasticsearch/ingest_pipeline/default.yml:115
YAML syntax error in default.yml
Details
packages/google_workspace/data_stream/calendar/elasticsearch/ingest_pipeline/default.yml does not parse as valid YAML. This will break elastic-package and ingestion.
Recommendation:
Fix the YAML syntax so the file parses:
# correct the indentation / quoting at the flagged line🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
🚀 Benchmarks reportPackage
|
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
signinlogs |
2347.42 | 1919.39 | -428.03 (-18.23%) | 💔 |
aadgraphactivitylogs |
1912.05 | 1428.57 | -483.48 (-25.29%) | 💔 |
activitylogs |
2544.53 | 1692.05 | -852.48 (-33.5%) | 💔 |
eventhub |
1e+06 | 500000 | -500000 (-50%) | 💔 |
identity_protection |
9523.81 | 6134.97 | -3388.84 (-35.58%) | 💔 |
Package github 👍(1) 💚(4) 💔(1)
Expand to view
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
issues |
5555.56 | 3076.92 | -2478.64 (-44.62%) | 💔 |
Package google_workspace 👍(10) 💚(6) 💔(6)
Expand to view
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
gcp |
9803.92 | 6329.11 | -3474.81 (-35.44%) | 💔 |
gmail |
1351.35 | 1010.1 | -341.25 (-25.25%) | 💔 |
groups |
11494.25 | 9259.26 | -2234.99 (-19.44%) | 💔 |
keep |
13513.51 | 9523.81 | -3989.7 (-29.52%) | 💔 |
token |
3436.43 | 2873.56 | -562.87 (-16.38%) | 💔 |
context_aware_access |
5102.04 | 4115.23 | -986.81 (-19.34%) | 💔 |
Package sublime_security 👍(0) 💚(0) 💔(3)
Expand to view
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
audit |
2557.54 | 2079 | -478.54 (-18.71%) | 💔 |
email_message |
1033.06 | 747.94 | -285.12 (-27.6%) | 💔 |
message_event |
13157.89 | 6369.43 | -6788.46 (-51.59%) | 💔 |
To see the full report comment with /test benchmark fullreport
| remove_event_guest: configuration | ||
| change_event_guest_response: configuration | ||
| change_event: configuration | ||
| print_preview_event: configuration |
There was a problem hiding this comment.
Severity: 🟠 High confidence: high path: packages/google_workspace/data_stream/calendar/elasticsearch/ingest_pipeline/default.yml:125
YAML syntax error in default.yml
Details
packages/google_workspace/data_stream/calendar/elasticsearch/ingest_pipeline/default.yml does not parse as valid YAML. This will break elastic-package and ingestion.
Recommendation:
Fix the YAML syntax so the file parses:
# correct the indentation / quoting at the flagged line🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
|
@vera-review-bot review |
…le ECS option is available
| field: event.type | ||
| value: creation | ||
| if: ctx.github?.issues?.created_at == ctx.github?.issues?.updated_at | ||
| # A closed issue or a merged pull request has reached the end of its lifecycle. |
There was a problem hiding this comment.
Severity: 🔵 Low confidence: medium path: packages/github/data_stream/issues/elasticsearch/ingest_pipeline/default.yml:36
The new comment documents mapping a closed issue to event.type deletion; end is the ECS value for a lifecycle ending, deletion means the object was removed.
Details
The comment added here states that a closed issue or merged pull request "has reached the end of its lifecycle", and the processor below maps that to event.type: deletion. In ECS, deletion means the entity was deleted, while end is the value for something that has ended. A closed issue still exists and is still returned by the API, so deletion mis-labels it. Since the PR is explicitly documenting and revisiting this mapping, it is worth correcting rather than codifying.
Recommendation:
Use end for the closed/merged terminal state:
# A closed issue or a merged pull request has reached the end of its lifecycle.
- append:
field: event.type
tag: append_end_into_event_type
value: end
allow_duplicates: false
if: ctx.github?.issues?.closed_at != null || ctx.github?.issues?.pull_request?.merged_at != null🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
end makes sense, but changing deletion to end alters pre-existing behavior for users and would result in a breaking change.
| if (params.email_resources.contains(res)) { | ||
| cats.add('email'); | ||
| types.add('info'); | ||
| } else if (res == 'auth') { |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: high path: packages/sublime_security/data_stream/audit/elasticsearch/ingest_pipeline/default.yml:434
The new ~90-line categorization script has only one pipeline test event, so the auth, iam and configuration branches are entirely untested; add fixtures covering them.
Details
_dev/test/pipeline/test-audit.log contains exactly one line, message_group.search, which only exercises the email_resources branch at default.yml:431-433. The auth branch (start/end/info split on auth.login* / auth.logout), the iam_resources branch (user/group plus the create/connect/upload vs delete vs change split) and the configuration_resources branch, including the inline_signed_headers_retrieved special case and the configuration_types list, all have zero coverage. The substring matching in those branches (t.contains('create') against the full type string) is the kind of logic that needs fixtures to stay correct.
Recommendation:
Add representative lines to _dev/test/pipeline/test-audit.log covering each branch, for example:
{"id":"...","type":"auth.login_success","created_at":"2024-08-12T06:04:03.714126Z","data":{}}
{"id":"...","type":"auth.logout","created_at":"2024-08-12T06:05:03.714126Z","data":{}}
{"id":"...","type":"user.create","created_at":"2024-08-12T06:06:03.714126Z","data":{}}
{"id":"...","type":"rbac.delete","created_at":"2024-08-12T06:07:03.714126Z","data":{}}
{"id":"...","type":"rules.update","created_at":"2024-08-12T06:08:03.714126Z","data":{}}
{"id":"...","type":"inline_signed_headers_retrieved","created_at":"2024-08-12T06:09:03.714126Z","data":{}}🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
We don't have live logs to add to pipeline coverage.
| field: event.kind | ||
| tag: set_event_kind_to_alert | ||
| value: alert | ||
| if: ctx.event?.action != null && ["DEVICE_COMPROMISED_EVENT", "SUSPICIOUS_ACTIVITY_EVENT"].contains(ctx.event.action) |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: high path: packages/google_workspace/data_stream/device/elasticsearch/ingest_pipeline/default.yml:545
Twelve new conditional branches were added to the device pipeline but the test fixture still has only two events; add fixtures for the alert, outcome and register/unregister paths.
Details
_dev/test/pipeline/test-device.log contains two events, APPLICATION_EVENT and DEVICE_SYNC_EVENT. None of the following new branches are covered: event.kind: alert for DEVICE_COMPROMISED_EVENT / SUSPICIOUS_ACTIVITY_EVENT (default.yml:541-545), the malware/threat/authentication categories (546-560), the start/end split on DEVICE_REGISTER_UNREGISTER_EVENT via account_state (581-590), and the policy-sync and compliance event.outcome rules (609-645). The failure-outcome rule at 627-630 uses case-sensitive contains('REJECT')/contains('FAIL') while its success counterpart uses equalsIgnoreCase, which is exactly the kind of asymmetry a fixture would pin down.
Recommendation:
Add events to _dev/test/pipeline/test-device.log covering the untested branches, e.g.:
{"kind":"admin#reports#activity","id":{"time":"2020-10-02T15:00:00Z","uniqueQualifier":1,"applicationName":"mobile","customerId":"1"},"events":{"type":"device_updates","name":"DEVICE_COMPROMISED_EVENT","parameters":[{"name":"DEVICE_COMPROMISED_STATE","value":"COMPROMISED"}]}}
{"kind":"admin#reports#activity","id":{"time":"2020-10-02T15:01:00Z","uniqueQualifier":2,"applicationName":"mobile","customerId":"1"},"events":{"type":"device_updates","name":"DEVICE_REGISTER_UNREGISTER_EVENT","parameters":[{"name":"ACCOUNT_STATE","value":"UNREGISTERED"}]}}
{"kind":"admin#reports#activity","id":{"time":"2020-10-02T15:02:00Z","uniqueQualifier":3,"applicationName":"mobile","customerId":"1"},"events":{"type":"device_updates","name":"ADVANCED_POLICY_SYNC_EVENT","parameters":[{"name":"POLICY_SYNC_RESULT","value":"POLICY_SYNC_SUCCEEDED"}]}}🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
We don't have live logs to add to pipeline coverage.
| tag: append_network_into_event_category | ||
| value: network | ||
| if: ctx.google_workspace?.meet?.name != null && ctx.google_workspace.meet.name == "call_ended" | ||
| if: ctx.google_workspace?.meet?.name != null |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/google_workspace/data_stream/meet/elasticsearch/ingest_pipeline/default.yml:100
Widening event.category: network to every Meet event mislabels non-network activity such as poll_created, hand_raised and send_chat_everyone; restrict it to the call/stream events.
Details
The condition changed from ctx.google_workspace.meet.name == "call_ended" to ctx.google_workspace?.meet?.name != null, so network is now appended for every event in the stream. The event.type map expanded in the same hunk includes names like poll_answered, hand_raised, reaction_sent, room_check_in, send_chat_everyone and smart_notes_session_started, none of which report network state or traffic. ECS defines network as events reporting the state or events of a network connection, so these events are now indexed under a category that does not describe them.
Recommendation:
Keep network for the call/stream lifecycle events and give the rest a category that matches, for example:
- append:
field: event.category
tag: append_network_into_event_category
value: network
allow_duplicates: false
if: ctx.google_workspace?.meet?.name != null && ["call_ended", "dialed_out", "ring_sent", "ring_answered", "ring_missed", "broadcast_activity", "in_meet_broadcast_activity", "livestream_watched"].contains(ctx.google_workspace.meet.name)🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| field: event.type | ||
| tag: append_creation_into_event_type_for_passkey_enrolled | ||
| value: creation | ||
| if: '["passkey_enrolled"].contains(ctx?.event?.action)' |
There was a problem hiding this comment.
| if: '["passkey_enrolled"].contains(ctx?.event?.action)' | |
| if: ctx.event?.action == 'passkey_enrolled' |
| field: event.type | ||
| tag: append_deletion_into_event_type_for_passkey_removed | ||
| value: deletion | ||
| if: '["passkey_removed"].contains(ctx?.event?.action)' |
There was a problem hiding this comment.
| if: '["passkey_removed"].contains(ctx?.event?.action)' | |
| if: ctx.event?.action == 'passkey_removed' |
| field: event.category | ||
| value: iam | ||
| if: '["account_disabled_password_leak","2sv_disable","2sv_enroll","account_disabled_generic","account_disabled_spamming_through_relay","account_disabled_spamming","account_disabled_hijacked","titanium_enroll","titanium_unenroll","gov_attack_warning"].contains(ctx?.event?.action)' | ||
| if: '["account_disabled_password_leak","2sv_disable","2sv_enroll","account_disabled_generic","account_disabled_spamming_through_relay","account_disabled_spamming","account_disabled_hijacked","titanium_enroll","titanium_unenroll","gov_attack_warning","passkey_enrolled","passkey_removed"].contains(ctx?.event?.action)' |
There was a problem hiding this comment.
| if: '["account_disabled_password_leak","2sv_disable","2sv_enroll","account_disabled_generic","account_disabled_spamming_through_relay","account_disabled_spamming","account_disabled_hijacked","titanium_enroll","titanium_unenroll","gov_attack_warning","passkey_enrolled","passkey_removed"].contains(ctx?.event?.action)' | |
| if: '["account_disabled_password_leak","2sv_disable","2sv_enroll","account_disabled_generic","account_disabled_spamming_through_relay","account_disabled_spamming","account_disabled_hijacked","titanium_enroll","titanium_unenroll","gov_attack_warning","passkey_enrolled","passkey_removed"].contains(ctx.event?.action)' |
|
✅ All changelog entries have the correct PR link. |
| tag: append_network_into_event_category | ||
| value: network | ||
| if: ctx.google_workspace?.meet?.name != null && ctx.google_workspace.meet.name == "call_ended" | ||
| if: '["call_ended","dialed_out","ring_sent","ring_answered","ring_missed"].contains(ctx.google_workspace?.meet?.name)' |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/google_workspace/data_stream/meet/elasticsearch/ingest_pipeline/default.yml:100
Narrowing the network category to five ring/call events leaves the stream and presentation events with an event.type but no event.category; extend the list to the conference/stream events as well.
Details
The condition now matches only call_ended, dialed_out, ring_sent, ring_answered and ring_missed, but the set_event_type_from_meet_name script directly below (lines 107-146) assigns an event.type to roughly forty event names. Everything outside those five now emits an event.type with no event.category at all: the regenerated test-meet.log-expected.json shows presentation-started and invitation-sent with "type": ["info"] and no category, and sample_event.json lost its category block entirely. poll_created and question_created are the sharpest case - they emit event.type: ["creation"] with nothing to group under, and in ECS event.type is a sub-bucket of event.category, so a type without a category cannot be filtered or aggregated in Security. The earlier feedback on this line asked for the category to be restricted to the call/stream events; the stream and presentation events (presentation_started, presentation_stopped, livestream_watched, broadcast_activity, in_meet_broadcast_activity, recording_activity, transcription_activity, streaming_session_explicit_consent) are part of that set but were left out of the new list.
Recommendation:
Extend the network list to the conference and streaming events so they keep a category alongside their type:
- append:
field: event.category
tag: append_network_into_event_category
value: network
if: >-
["call_ended","dialed_out","ring_sent","ring_answered","ring_missed",
"presentation_started","presentation_stopped","livestream_watched",
"broadcast_activity","in_meet_broadcast_activity","recording_activity",
"transcription_activity","streaming_session_explicit_consent"].contains(ctx.google_workspace?.meet?.name)For the remaining in-meeting activity that has an event.type but no natural network semantics (poll_created, question_created, hand_raised, send_chat_everyone, ...), add a configuration fallback so no event ships a type without a category:
- append:
field: event.category
tag: append_configuration_into_event_category
value: configuration
if: ctx.event?.category == null && ctx.google_workspace?.meet?.name != null🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
Review summaryIssues found across the latest commits fb9cc32 — 1 medium
Issues found across earlier commits fed75d7 — 6 medium, 2 low
Issues found across earlier commits a3a239d…63d97a1 (41 commits) — 3 high, 6 medium, 2 low
Issues found across earlier commits 6c76d69 — 1 high
Issues found across earlier commits 91db7fe — 1 high
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
|
💚 Build Succeeded
History
|
|
Tick the box to add this pull request to the merge queue (same as
|
| - set: | ||
| field: event.action | ||
| value: closed | ||
| tag: set_event_action_closed | ||
| if: ctx.github?.issues?.state == 'closed' |
There was a problem hiding this comment.
Could also add event.reason based on state_reason.
4 participants
Proposed commit message
Checklist
changelog.ymlfile.Related issues
event.categoryacross events #18343