sentinel_one: fix error handling for first phase of threat_event agent data collection - #20484
Conversation
efd6
added
Integration:sentinel_one
4198cad to
2ad6292
Compare
✅ 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. |
🚀 Benchmarks reportTo see the full report comment with |
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
![vera-review-bot[bot]](https://avatars.githubusercontent.com/in/3297022?s=60&v=4){kind=small}
| }, | ||
| "fetch_more": body.?pagination.nextCursor.orValue(null) != null, | ||
| "list_updated_at_gte": state.?cursor.?list_updated_at_gte.orValue( | ||
| "list_updated_at_gte": state.?cursor.list_updated_at_gte.orValue( |
There was a problem hiding this comment.
Severity: 🟠 High confidence: high path: packages/sentinel_one/data_stream/threat_event/agent/stream/cel.yml.hbs:66
The list watermark list_updated_at_gte is carried forward unconditionally and is never cleared, so updatedAt__gte freezes at its first value and every completed pass re-lists the whole threat history; clear it with optional.none() when there is no next page, as the sibling threat stream does.
Details
list_updated_at_gte is meant to pin updatedAt__gte for the duration of one paginated pass over /threats so paging stays stable. Here it is only ever self-referentially preserved: this line computes it as state.?cursor.list_updated_at_gte.orValue(state.?cursor.?last_timestamp.orValue(now - initial_interval)), and phase two (line 141) copies it forward with ?"list_updated_at_gte": state.?cursor.list_updated_at_gte. No branch ever removes it.
Consequences, all present in the current code:
- Once the first phase-one request sets the key,
orValuenever falls through again, soupdatedAt__gteis frozen at the value computed on the very first execution. last_timestampis advanced in phase two (line 145) but is only reachable through that deadorValuefallback, so the advancing watermark is never consumed.- When a pass finishes,
next_page.tokenis dropped (line 63 resolves tooptional.none()on a nullnextCursor), so the next phase-one request has no page token and restarts from the frozen timestamp - re-listing every threat and re-issuing a/explore/eventsrequest per threat, forever, with the volume growing as threats accumulate. Ingest-side fingerprint dedup hides the duplicate documents but not the API load.
The sibling threat data stream implements the same watermark correctly in packages/sentinel_one/data_stream/threat/agent/stream/httpjson_as_cel.yml.hbs:69: ?"list_updated_at_gte": has_more_list ? optional.of(state.cursor.list_updated_at_gte) : optional.none() - it drops the key when the listing is complete so the next pass starts from last_update_at.
Recommendation:
Make the key optional in phase one and drop it when there is no next page, mirroring the threat data stream. Phase two's ?"list_updated_at_gte": state.?cursor.list_updated_at_gte then correctly carries it only while a pass is still in flight:
"fetch_more": body.?pagination.nextCursor.orValue(null) != null,
?"list_updated_at_gte": (body.?pagination.nextCursor.orValue(null) != null) ?
optional.of(
state.?cursor.list_updated_at_gte.orValue(
state.?cursor.last_timestamp.orValue(
(now - duration(state.initial_interval)).format(time_layout.RFC3339)
)
)
)
:
optional.none(),
?"last_timestamp": state.?cursor.last_timestamp,🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| }, | ||
| } | ||
| ).do_request().as(resp, (resp.StatusCode == 200) ? | ||
| ).do_request().as(resp, (resp.StatusCode == 200 || resp.StatusCode == 404) ? |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/sentinel_one/data_stream/threat_event/agent/stream/cel.yml.hbs:113
Folding 404 into the 200 branch sends the 404 body through resp.Body.decode_json(), so an empty or non-JSON 404 body aborts the whole CEL evaluation; give 404 its own branch that advances the worklist without decoding, as the sibling threat stream does.
Details
Accepting 404 here routes the response into resp.Body.decode_json() on line 114. That is safe only for SentinelOne's JSON error document. For any 404 whose body is empty or not JSON - the case the program's own error branch explicitly anticipates on line 158 with size(resp.Body) != 0 - decode_json() raises a CEL evaluation error, which aborts the entire program run: no events are published, the cursor is not advanced, and the input logs an evaluation failure every interval instead of skipping the threat. The 404 path is now a normal, expected path in this program, so it should not depend on the body being well-formed JSON.
The sibling threat data stream keeps 404 in a dedicated branch that never decodes the body (packages/sentinel_one/data_stream/threat/agent/stream/httpjson_as_cel.yml.hbs:164-176): it emits its placeholder event, advances worklist with tail(...), clears next_chain, and moves the timestamp forward.
Splitting the branch also removes the two now-conditional expressions the shared branch needs - resp.StatusCode == 200 && has(body.data) ... on line 120 and the has_more_events computation on line 115, which can only ever be false for a 404.
Recommendation:
Restore 200 as the sole decode branch and handle 404 separately:
).do_request().as(resp, (resp.StatusCode == 200) ?
resp.Body.decode_json().as(body,
# ...unchanged 200 handling...
)
: (resp.StatusCode == 404) ?
{
"events": [{"message": "retry"}],
"want_more": state.?cursor.fetch_more.orValue(false) ?
state.cursor.fetch_more
:
size(state.cursor.worklist.data) > 1,
"cursor": {
"worklist": {"data": tail(state.cursor.worklist.data)},
"next_page": {
?"token": state.?cursor.next_page.token,
},
"next_chain": {},
"fetch_more": state.?cursor.fetch_more.orValue(false),
?"list_updated_at_gte": state.?cursor.list_updated_at_gte,
?"last_timestamp": state.cursor.worklist.data[0].?threatInfo.updatedAt.or(state.?cursor.last_timestamp),
},
}
:
# ...unchanged error branch...
)🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| @@ -1,4 +1,15 @@ | |||
| # newer versions go on top | |||
| - version: "2.11.2" | |||
There was a problem hiding this comment.
Severity: 🔵 Low confidence: medium path: packages/sentinel_one/changelog.yml:2
The 2.11.2 entry lists three fixes but the PR also fixes a null-pointer exception in the convert_file_size_to_long guard; add a fourth changelog entry for it.
Details
packages/sentinel_one/data_stream/threat_event/elasticsearch/ingest_pipeline/default.yml:822 changes the guard from ctx.sentinel_one?.threat_event?.file.size != '' to ctx.sentinel_one?.threat_event?.file?.size != ''. Painless's null-safe operator only guards the access it is attached to, so for any threat event without file data - for example the DNS and NETWORK events in the new deleted_threat_404.txt fixture - ?.file resolved to null and the unguarded .size threw a NullPointerException. The processor then failed, error.message was appended and event.kind was set to pipeline_error.
That is a user-visible behaviour change (affected documents stop being tagged as pipeline errors), and it is what the new test's select(.error.message != null) | length == 0 assertion checks, but none of the three 2.11.2 entries covers it.
Recommendation:
Add an entry alongside the existing three:
- version: "2.11.2"
changes:
- description: Fix error handling for first phase of agent data collection.
type: bugfix
link: https://github.com/elastic/integrations/pull/20484
- description: Fix handling of deleted threats.
type: bugfix
link: https://github.com/elastic/integrations/pull/20484
- description: Fix global error handler message formatting.
type: bugfix
link: https://github.com/elastic/integrations/pull/20484
- description: Fix null pointer exception when a threat event has no file information.
type: bugfix
link: https://github.com/elastic/integrations/pull/20484🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| ).do_request().as(resp, (resp.StatusCode == 200) ? | ||
| resp.Body.decode_json().as(body, | ||
| ).do_request().as(resp, (resp.StatusCode == 200 || resp.StatusCode == 404) ? | ||
| resp.Body.decode_json().as(maybe, is_error(maybe) ? {} : maybe).as(body, |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/sentinel_one/data_stream/threat_event/agent/stream/cel.yml.hbs:114
The is_error(maybe) ? {} fallback applies to 200 responses as well as 404s, so a threat whose events body fails to decode is silently dropped from the worklist with no error event. Scope the empty-map fallback to the 404 branch.
Details
The decode guard sits outside the status-code test, so it covers both accepted statuses. On a 200 whose body is not decodable JSON, body becomes {}; has_more_events is then false, events falls through to the [{"message": "retry"}] placeholder (which the ingest pipeline drops via drop_046d48c9), new_worklist is tail(...) so the threat is removed from the worklist, and ?"last_timestamp" advances to that threat's threatInfo.updatedAt. The result is that the threat's events are permanently skipped and the watermark moves past them, with nothing indexed and no error surfaced anywhere. Before this change a 200 with an undecodable body failed evaluation, which left the cursor untouched and let the agent retry. The 404 case genuinely wants the empty-map fallback (the SentinelOne error body carries no data/pagination), but a 200 does not.
Separately, the skill's canonical shape for a catchable decode is try(resp.Body.decode_json()) — is_error() is documented as the test for a value produced by try().
Recommendation:
Restrict the fallback to the 404 branch so an undecodable 200 still fails loudly (or routes to the existing error-event shape):
).do_request().as(resp, (resp.StatusCode == 200 || resp.StatusCode == 404) ?
(resp.StatusCode == 404 ?
try(resp.Body.decode_json()).as(maybe, is_error(maybe) ? {} : maybe)
:
resp.Body.decode_json()
).as(body,
(body.?pagination.nextCursor.orValue(null) != null).as(has_more_events,Remember to regenerate _dev/test/policy/test-all.expected and _dev/test/policy/test-default.expected after changing the program.
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| # newer versions go on top | ||
| - version: "2.11.2" | ||
| changes: | ||
| - description: Fix error handling for first phase of agent data collection. |
There was a problem hiding this comment.
Severity: 🔵 Low confidence: medium path: packages/sentinel_one/changelog.yml:4
The changelog entry says "first phase of agent data collection" without naming the data stream; since this package also ships an agent data stream it reads as if that stream changed. Name threat_event explicitly.
Details
All three entries in 2.11.2 describe changes that are specific to the threat_event data stream, but none of them says so. The first entry is the most ambiguous: packages/sentinel_one/data_stream/agent/ exists, so "agent data collection" is naturally read as the agent data stream rather than as Elastic Agent collection for threat_event. The changelog is user-facing and is what an operator reads when deciding whether an upgrade affects them.
Recommendation:
Name the data stream in each entry:
- version: "2.11.2"
changes:
- description: Fix error handling for the first collection phase of the threat_event data stream.
type: bugfix
link: https://github.com/elastic/integrations/pull/20484
- description: Fix handling of deleted threats in the threat_event data stream.
type: bugfix
link: https://github.com/elastic/integrations/pull/20484
- description: Fix global error handler message formatting in the threat_event data stream.
type: bugfix
link: https://github.com/elastic/integrations/pull/20484🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
273d564 to
90dce79
Compare
| }, | ||
| "fetch_more": state.?cursor.fetch_more.orValue(false), | ||
| "list_updated_at_gte": state.cursor.list_updated_at_gte, | ||
| ?"list_updated_at_gte": (body.?pagination.nextCursor.orValue(null) != null) ? |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/sentinel_one/data_stream/threat_event/agent/stream/cel.yml.hbs:147
The new ?"list_updated_at_gte" clear is keyed on the explore/events page cursor, but the worklist only advances on that same condition, so the value is cleared every time the worklist moves and is always absent when the list request reads it. Key the clear on the threats-list pagination (fetch_more) instead.
Details
body here is the /explore/events response, so the guard (body.?pagination.nextCursor.orValue(null) != null) is exactly has_more_events, already bound on line 121. The worklist advances (tail(...), line 123) under precisely the negation of that condition. So list_updated_at_gte is written only in states where the worklist has NOT advanced, and it is cleared on every step that does advance it. The first phase (line 42, and its own write at line 66) only runs when the worklist is empty, and the worklist can only become empty via a tail() step that cleared the key — so state.?cursor.list_updated_at_gte at line 42 is always absent and updatedAt__gte always falls back to last_timestamp. The freeze this key exists to provide never takes effect: when the threats list itself is paginated (fetch_more true, next_page.token set), the continuation request is sent with an updatedAt__gte that has moved to the last processed threat's updatedAt rather than the value the first page was queried with. The sibling threat stream keys the same key on the list-level has_more_list (packages/sentinel_one/data_stream/threat/agent/stream/httpjson_as_cel.yml.hbs:69), which is the signal intended here.
Recommendation:
Clear on the threats-list pagination state (fetch_more, set from the list response at line 65) rather than on the events page cursor, mirroring the threat stream:
?"list_updated_at_gte": state.?cursor.fetch_more.orValue(false) ?
optional.of(
state.?cursor.list_updated_at_gte.orValue(
state.?cursor.last_timestamp.orValue(
(now - duration(state.initial_interval)).format(time_layout.RFC3339)
)
)
)
:
optional.none(),Apply the identical change to the two rendered policy expectations (_dev/test/policy/test-all.expected, _dev/test/policy/test-default.expected).
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| state.?cursor.?list_updated_at_gte.orValue( | ||
| state.?cursor.?last_timestamp.orValue( | ||
| state.?cursor.list_updated_at_gte.orValue( | ||
| state.?cursor.last_timestamp.orValue( |
There was a problem hiding this comment.
Severity: 🔵 Low confidence: medium path: packages/sentinel_one/data_stream/threat_event/agent/stream/cel.yml.hbs:43
This PR drops the second ? from state.?cursor.?last_timestamp here (line 43) and in the new phase-two code (line 150), but leaves it in the equivalent expression at line 67, so the same fallback chain is now spelled two ways in one program. Normalise line 67 as well.
Details
Optional chaining propagates absence through the rest of the selection chain, so state.?cursor.last_timestamp and state.?cursor.?last_timestamp behave identically. The diff normalises the query-parameter expression on line 43 and writes the new phase-two expression on line 150 without the extra ?, but the cursor write at lines 66-68 still carries state.?cursor.?last_timestamp. Three copies of one expression, two spellings, makes it harder to see that they are meant to stay in lockstep.
Recommendation:
Drop the redundant ? at line 67 so all three copies match:
"list_updated_at_gte": state.?cursor.list_updated_at_gte.orValue(
state.?cursor.last_timestamp.orValue(
(now - duration(state.initial_interval)).format(time_layout.RFC3339)
)
),Mirror the change in _dev/test/policy/test-all.expected and _dev/test/policy/test-default.expected.
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
90dce79 to
f68f576
Compare
| }, | ||
| "fetch_more": state.?cursor.fetch_more.orValue(false), | ||
| "list_updated_at_gte": state.cursor.list_updated_at_gte, | ||
| ?"list_updated_at_gte": (body.?pagination.nextCursor.orValue(null) != null) ? |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/sentinel_one/data_stream/threat_event/agent/stream/cel.yml.hbs:147
The new ?"list_updated_at_gte" retention is gated on the event-chain cursor (body.pagination.nextCursor from explore/events) instead of the threat-list cursor (cursor.fetch_more), so the list watermark is dropped while list pagination is still in flight; gate it on state.cursor.fetch_more instead.
Details
list_updated_at_gte pins the updatedAt__gte filter used by the phase-1 threats-list request (line 42) so that it stays constant while the list is paginated with cursor.next_page.token. The new conditional keeps it only when body.?pagination.nextCursor != null, but in this branch body is the explore/events response for a single threat, so that expression is has_more_events — it describes the event chain of the current threat, not the threats list.
The two conditions are effectively inverted with respect to what the pin is for:
- While an event chain is paginating (
has_more_eventstrue) the worklist is non-empty, so phase 1 does not run and the pin is never read. - While the threats list is paginating (
cursor.fetch_moretrue,cursor.next_page.tokenset) the pin IS read by phase 1, but it is exactly the case where this code drops it.
Concretely: after phase 1 returns page 1 of the threats list with a nextCursor, the first threat processed clears list_updated_at_gte, so when the worklist drains phase 1 re-issues the list request with the same cursor token but a different updatedAt__gte (now last_timestamp, the updatedAt of the last threat from page 1). The filter sent alongside an in-flight cursor changes between pages, and because updatedAt__gte is inclusive the boundary threat is re-selected and its explore/events re-fetched.
The sibling threat data stream implements this correctly: in packages/sentinel_one/data_stream/threat/agent/stream/httpjson_as_cel.yml.hbs the pin is retained with ?"list_updated_at_gte": has_more_list ? optional.of(state.cursor.list_updated_at_gte) : optional.none(), where has_more_list is the list response's nextCursor.
Recommendation:
Gate the pin on the threats-list pagination state (cursor.fetch_more), matching the threat data stream:
"fetch_more": state.?cursor.fetch_more.orValue(false),
?"list_updated_at_gte": state.?cursor.fetch_more.orValue(false) ?
optional.of(
state.?cursor.list_updated_at_gte.orValue(
state.?cursor.last_timestamp.orValue(
(now - duration(state.initial_interval)).format(time_layout.RFC3339)
)
)
)
:
optional.none(),
Remember to regenerate _dev/test/policy/test-all.expected and _dev/test/policy/test-default.expected after the change.
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| - description: Fix handling of deleted threats. | ||
| type: bugfix | ||
| link: https://github.com/elastic/integrations/pull/20484 | ||
| - description: Fix global error handler message formatting. |
There was a problem hiding this comment.
Severity: 🔵 Low confidence: medium path: packages/sentinel_one/changelog.yml:10
The 2.11.2 changelog covers the three CEL/error-formatting fixes but not the ingest pipeline null-safety fix to convert_file_size_to_long; add a fourth bugfix entry for it.
Details
elasticsearch/ingest_pipeline/default.yml changes the convert_file_size_to_long condition from ctx.sentinel_one?.threat_event?.file.size to ctx.sentinel_one?.threat_event?.file?.size. That is a distinct, user-visible fix: without it the condition raises a null-pointer error for any document where sentinel_one.threat_event.file is absent, which surfaces as a pipeline_error document. None of the three entries in the 2.11.2 block describes it — 'Fix error handling for first phase of agent data collection', 'Fix handling of deleted threats' and 'Fix global error handler message formatting' all refer to the CEL program and the pipeline's global on_failure block.
Recommendation:
Add an entry to the 2.11.2 block:
- version: "2.11.2"
changes:
- description: Fix error handling for first phase of agent data collection.
type: bugfix
link: https://github.com/elastic/integrations/pull/20484
- description: Fix handling of deleted threats.
type: bugfix
link: https://github.com/elastic/integrations/pull/20484
- description: Fix global error handler message formatting.
type: bugfix
link: https://github.com/elastic/integrations/pull/20484
- description: Fix null pointer error when converting file size for threat events without file information.
type: bugfix
link: https://github.com/elastic/integrations/pull/20484🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| @@ -0,0 +1,104 @@ | |||
| # Test that a 404 on a deleted threat's explore/events endpoint does not | |||
There was a problem hiding this comment.
Severity: 🔵 Low confidence: medium path: packages/sentinel_one/data_stream/threat_event/_dev/test/scripts/deleted_threat_404.txt:1
The new script test covers the deleted-threat 404 path but nothing exercises the headline fix — preserving a phase-1 error when cursor.worklist already exists with an empty data list; add a script test for that sequence.
Details
The has(state.?events.error) guard added at cel.yml.hbs:93 fixes the case where a phase-1 (threats list) failure is silently discarded: when state.cursor.worklist exists but data is empty, the second phase previously fell through to its {"events": [], "want_more": false} branch and overwrote the error event produced by phase 1, so the failure was never indexed.
Neither test in this data stream asserts that behaviour. deleted_threat_404.txt only covers the explore/events 404 path. auth_errors.txt hits phase-1 failures only on the first two polls, before any cursor exists, so it exercises the !has(state.?cursor.worklist) branch rather than the empty-worklist branch that this PR fixes.
Recommendation:
Add a script test whose mock returns a successful threats list first (so cursor.worklist is created and then drained) and a failure on the next list call, then assert the error event is indexed:
# s1-mock/config.yml
rules:
- path: /web/api/v2.1/threats
methods: [GET]
responses:
# 1st call: one threat, drains into an empty worklist.
- status_code: 200
body: |-
{"data":[{"id":"threat-001","threatInfo":{"updatedAt":"2024-01-15T10:01:00.000Z"}}],"pagination":{"nextCursor":null}}
# 2nd call: fails while cursor.worklist exists with an empty data list.
- status_code: 500
body: |-
{"errors":[{"code":5000000,"detail":"Internal error."}]}# Assert the phase-1 error survives instead of being overwritten by phase 2.
get_docs -profile ${CONFIG_PROFILES}/${PROFILE} -want 3 -confirm 15s -timeout 5m ${DATA_STREAM_NAME}
cp stdout got_docs.json
exec jq '[.hits.hits[]._source | select(.error.code == "500")] | length' got_docs.json
stdout '^1$'
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
…t data collection In the previous code, an error event could include a cursor object from a previous successful data collection. This would allow the the second phase to run expecting the cursor list_updated_at_gte field to exist. If the cursor was written by a version prior to 2.11.1 this would not be present. The second phase did not properly defend against this field being absent. In cases where worklist items have been deleted, the code did not gracefully handle their absence. Make the second phase bail early if there is an error object returned from the first phase sub-expression, and add an optional type access to the cursor list_updated_at_gte field. To handle and move past deleted items, 404 status codes are now treated with 200, for cursor handling, but return a retry object to allow the collection to step past them. Also fix an unsafe field access in a convert processor condition and formatting of the global on_failure error message.
f68f576 to
53872a0
Compare
|
✅ All changelog entries have the correct PR link. |
| }, | ||
| "fetch_more": state.?cursor.fetch_more.orValue(false), | ||
| "list_updated_at_gte": state.cursor.list_updated_at_gte, | ||
| ?"list_updated_at_gte": (body.?pagination.nextCursor.orValue(null) != null) ? |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/sentinel_one/data_stream/threat_event/agent/stream/cel.yml.hbs:147
The new ?"list_updated_at_gte" guard tests the explore/events cursor (has_more_events) rather than the threats-list pagination state, so the pinned list window is dropped while list pagination is still in flight; key the retention off state.cursor.fetch_more instead.
Details
list_updated_at_gte exists to pin updatedAt__gte on the phase-1 threats-list request so the value stays constant while the cursor-paginated list is walked. The new guard retains it only when body.?pagination.nextCursor != null, where body is the explore/events response for the current threat — i.e. exactly has_more_events, which is about event pagination inside one threat, not about list pagination.
Those two conditions are inverted with respect to when the value is read. When has_more_events is true the worklist is not consumed (line 123 keeps state.cursor.worklist.data), so phase 1 is skipped on the next execution and the retained value is never read. When has_more_events is false the worklist entry is popped and the value is dropped — including the case where fetch_more is still true and more threats-list pages remain.
Concrete effect on the multi-page path: want_more (line 134) is unconditionally true while fetch_more is true, so as soon as the worklist drains the input immediately re-enters phase 1 with next_page.token still set (line 141 preserves it) but with list_updated_at_gte now absent. updatedAt__gte therefore falls back to last_timestamp, which phase 2 has advanced to the last processed threat's threatInfo.updatedAt (line 160). The continuation request for page N+1 is sent with the page-N cursor token but a different updatedAt__gte than the request that produced that token. The only path on which the retained value is ever read back is the empty-list branch at line 185, where the pin then sticks indefinitely — the opposite of the intended behaviour.
This change is also not described in the 2.11.3 changelog entries, which cover only the phase-1 error handling, the deleted-threat 404, and the on_failure message formatting.
Recommendation:
Gate the retention on the threats-list pagination flag (fetch_more) rather than the events cursor, so the window stays pinned for as long as the list cursor is being walked and is cleared once the list is exhausted:
?"list_updated_at_gte": state.?cursor.fetch_more.orValue(false) ?
optional.of(
state.?cursor.list_updated_at_gte.orValue(
state.?cursor.last_timestamp.orValue(
(now - duration(state.initial_interval)).format(time_layout.RFC3339)
)
)
)
:
optional.none(),Apply the same edit to the rendered policy fixtures (_dev/test/policy/test-all.expected and test-default.expected), and add a changelog entry describing the cursor-window change.
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
Review summaryIssues found across the latest commits cad70f5…53872a0 (24 commits) — 1 medium
Issues found across earlier commits f68f576 — 1 medium, 2 low
Issues found across earlier commits 90dce79 — 1 medium, 1 low
Issues found across earlier commits 273d564 — 1 medium, 1 low
Issues found across earlier commits 2ad6292 — 1 high, 1 medium, 1 low
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
|
💚 Build Succeeded
History
cc @efd6 |
|
Tick the box to add this pull request to the merge queue (same as
|
|
Package sentinel_one - 2.11.3 containing this change is available at https://epr.elastic.co/package/sentinel_one/2.11.3/ |
2 participants
Proposed commit message
Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
Related issues
Screenshots