Skip to content

fix(sentinel-one): set failure only from an explicit final status (rebase of #2679) - #2753

Merged
osmontero merged 1 commit into
v11from
s1-action-result-320
Sep 25, 2026
Merged

osmontero merged 1 commit into
v11from
s1-action-result-320

Conversation

@osmontero

Copy link
Copy Markdown
Member

Rebase of #2679 onto current v11 (post #2677; sentinel-one filter 3.0.1 -> 3.2.0). #2679's original fork branch now conflicts with the merged #2677 on the version line and CEF extraction; this applies its actual change (the consolidated single-step ailure verdict replacing the 3-step ailed block) on top of current v11, plus its action-result contract test + fixture (10 cases, passing against go-sdk v1.1.36). No rule consumes sentinel-one �ctionResult, so the ailed-> ailure swap is safe. Closes #2679.

Rebase of #2679 onto current v11 (post #2677, sentinel-one 3.0.1 -> 3.2.0).
Consolidates the 3-step 'failed' actionResult block into one guarded
single-step verdict: failure only on exact/anchored mitigation_failed /
failed, case-insensitive, with a negative guard so contradictory or
successful statuses stay unknown. Adds the action-result contract test
and fixture (10 cases) which passes against go-sdk v1.1.36.
@osmontero
osmontero requested a review from a team September 25, 2026 15:34
@osmontero
osmontero merged commit 88b4d71 into v11 Sep 25, 2026
5 checks passed
@osmontero
osmontero deleted the s1-action-result-320 branch September 25, 2026 15:34
@github-actions

Copy link
Copy Markdown

❌ Go dependencies check failed

There are outdated Go dependencies, or modules that could not be inspected.
Run bash .github/scripts/go-deps.sh --update --discover locally and
commit the updated go.mod / go.sum files.

Script output
🔍 Discovered 25 Go projects

📦 Dependencies with updates available:

  📁 ./utmstack-collector:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/gcp:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/aws:
     - github.com/aws/aws-sdk-go-v2: v1.47.0 → v1.47.1
     - github.com/aws/aws-sdk-go-v2/config: v1.33.5 → v1.33.6
     - github.com/aws/aws-sdk-go-v2/credentials: v1.20.5 → v1.20.6
     - github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs: v1.88.0 → v1.88.1
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/events:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/inputs:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/stats:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/rule-flood-guard:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/o365:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/modules-config:
     - github.com/aws/aws-sdk-go-v2/config: v1.33.5 → v1.33.6
     - github.com/aws/aws-sdk-go-v2/credentials: v1.20.5 → v1.20.6
     - github.com/aws/aws-sdk-go-v2/service/cloudwatchlogs: v1.88.0 → v1.88.1
     - github.com/aws/aws-sdk-go-v2/service/sts: v1.51.0 → v1.51.1
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/config:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/soc-ai:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/sophos:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/azure:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/crowdstrike:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/bitdefender:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./plugins/geolocation:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./agent-manager:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./agent:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./as400:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

  📁 ./as400/updater:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.36

�[0;31m❌ Please update dependencies before merging.�[0m

@github-actions

Copy link
Copy Markdown

🛑 AI review — Blocking issues

One or more high/critical issues can break things and must be fixed before merging. Details below.

✅ architecture (silas-1.7-pro) — clean

Summary: Test-only addition for SentinelOne alert outcome mapping; no agent, proto, migration, CI, installer, or contract changes detected.

No findings.

🛑 bugs (silas-1.7-pro) — blocking — must fix before merge

Summary: New test has likely failing CEL predicate and unguarded Params lookups that can nil-deref; fixture absent keys are ignored.

  • high plugins/alerts/sentinel_one_action_result_test.go:89 — The final predicate passes equals("actionResult", result) to CEL, so the first argument is the string literal "actionResult" rather than the event field. For every non-empty Result case the expected predicate should be true but the expression evaluates false, causing the test to fail. Reference the field without quotes or use the SDK's intended field-equality form.
  • high plugins/alerts/sentinel_one_action_result_test.go:56 — step.Add.Params["key"].GetStringValue() dereferences a map lookup without checking whether key exists. If any Add step in an antivirus-sentinel-one stage lacks a key parameter, the test panics with a nil pointer dereference. Guard the lookup before calling GetStringValue.
  • high plugins/alerts/sentinel_one_action_result_test.go:69 — step.Add.Params["value"].GetStringValue() has the same unguarded lookup. A matching Add step without a value parameter causes a nil pointer panic. Validate that the value parameter exists before dereferencing it.
  • low plugins/alerts/sentinel_one_action_result_test.go:17 — The test case struct only decodes name, result, and expected, so the absent arrays in testdata/sentinel_one_action_result.json are silently ignored. If those absent fields are intended as regression assertions, add an Absent []string field and assert the listed paths are not present.

✅ security (silas-1.7-pro) — clean

Summary: Test-only additions with synthetic fixtures introduce no new vulnerabilities or customer-facing information disclosure.

No findings.

@utmstackprapprover utmstackprapprover Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Changes requested — Go dependencies check failed (see above).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant