[9.4] fix(security-solution): preserve Painless scripts and fix group take-actions in bulk close (#289549) - #290804
Conversation
…actions in bulk close (elastic#289549) ## Summary Follow-up to elastic#288946. ### The problem When a user adds a **data view runtime field** to the alerts table — particularly a **scripted** runtime field (one with a Painless expression, e.g. combining two source fields or lowercasing a value) — and then tries to bulk-close those alerts, they see: > _Successfully closed 0 alerts_ The same failure occurs whether the user: - Uses **Select all → Mark as closed** from the toolbar, or - Uses **Group alerts by [field] → Take actions → Mark as closed** from a group row. The root cause is that Elasticsearch needs the full runtime field definition (including the Painless script) to evaluate the filter at query time. Without it, the filter matches nothing. ### Why it happens Bulk closing by query sends a `_update_by_query` to Elasticsearch. The filter in that query can reference runtime fields — fields that don't exist in the alerts index mapping and must be evaluated on the fly by ES. For that to work, the runtime field's script must travel with the request. The merged fix in elastic#288946 passed the runtime field names and types, but **stripped the Painless scripts**. The server synthesised a replacement script that reads the field value from `_source`. That works for rule-source fields (where the alerting framework copies the value into `_source` when the alert is created), but not for data view runtime fields — ES computes those at query time from the raw document fields, and there is no copy in `_source`. The filter therefore always resolves to nothing. The group take-actions path had a separate problem: it never forwarded any runtime field information at all, so both scripted and scriptless data view fields failed there. ### How it's fixed The full runtime field definition — type, Painless script, format — is now forwarded to Elasticsearch as-is for data view fields. The older path that synthesises a `_source` reader is preserved for the exceptions-flyout use case, which genuinely needs those semantics. The group take-actions path is now wired to the same runtime field data the rest of the alerts table already had available. ### A note on data integrity This is worth backporting. Beyond the "0 alerts closed" UX failure there is a subtler correctness issue: if the runtime field *transforms* a value (e.g. `.toLowerCase()`), the synthesised `_source` reader returns the untransformed value — so "select all → close" silently closes a *different set of alerts* than the grid showed. Alerts the user intended to close stay open; alerts they didn't intend to close may be closed. --- <details> <summary>End-to-end request flow</summary> **User action:** selects all alerts filtered on `display_name: "sudo-scripted"` → clicks "Mark as closed" **Client (`toBulkCloseRuntimeMappings`):** reads the data view's runtime field definitions, strips unsupported types (`composite`, `lookup`) and unsupported script shapes (stored scripts, scripts with `params`/`lang`), produces a clean `{ name → { type, script.source, format } }` map. **HTTP request to Kibana server:** ```json POST /api/detection_engine/signals/status { "status": "closed", "query": { "term": { "display_name": "sudo-scripted" } }, "runtime_mappings": { "display_name": { "type": "keyword", "script": { "source": "emit(doc['process.name'].value + '-scripted')" } } } } ``` **Zod (`.strict()` on the script object):** rejects any request that includes unsupported script properties (`params`, `lang`, etc.) with a 400 before the handler runs. **Server handler (`mergeBulkCloseRuntimeMappings`):** merges any `runtime_fields` (exceptions-flyout path, which synthesises a `_source` reader) with the new `runtime_mappings` passthrough, and stamps `on_script_error: continue` on each scripted entry so a single failing document doesn't abort the whole operation. **`_update_by_query` to Elasticsearch:** ```json { "script": { "source": "ctx._source['kibana.alert.workflow_status'] = 'closed'" }, "query": { "term": { "display_name": "sudo-scripted" } }, "runtime_mappings": { "display_name": { "type": "keyword", "on_script_error": "continue", "script": { "source": "emit(doc['process.name'].value + '-scripted')" } } } } ``` **Elasticsearch:** defines `display_name` as a virtual field for this request, runs the Painless script per document to evaluate the filter, and updates matching documents in one pass. If the script throws on any document, that document is skipped and the rest proceed. **Response:** `{ updated: 47 }` → toast shows *Successfully closed 47 alerts*. The whole fix is ensuring the Painless script survives the journey from the data view definition all the way to ES, unchanged. </details> --- ## Backwards compatibility This PR is fully backwards compatible. The fix adds a new **optional** body parameter (`runtime_mappings`) to the existing `POST /api/detection_engine/signals/status` endpoint (API version `2023-10-31`). Callers that do not send it receive identical behaviour to today. The existing `runtime_fields` parameter — used by the exceptions-flyout path — is untouched. Its behaviour is unchanged. --- ## How to reproduce the original issue > **Note:** these steps demonstrate the pre-fix behaviour. To observe the bug directly, check out `main` (before this PR is merged) — the fix on this branch means steps 6–7 will show the correct behaviour, not the bug. > Requires a running Kibana + Elasticsearch stack with auditbeat data (or any rule that produces alerts). 1. Generate some test alerts: ``` node x-pack/solutions/security/plugins/security_solution/scripts/data/generate_cli.js \ -n 150 --start-date 1d --end-date now \ --attacks ``` 1. Open the Stack Management / Data Views. 1. Select "Security solution default" 1. Click the "Add field" button and fill in - Name: `display_name` - Type: Keyword - Switch on the "Set value" toggle and fill in: `emit(doc['process.name'].value + '-scripted')` in the script textarea - Click "Save" 1. Go to the Alerts page and filter the alerts for "display_name: explorer.exe-scripted" 1. Select one alert then use the "Select all n alerts" button 1. Then click "Selected n alerts" → **Mark as closed**. 1. **Expected:** alerts matching the filter are closed. **Actual (before this fix):** toast shows _Successfully closed 0 alerts_. https://github.com/user-attachments/assets/1f438a34-f725-4a58-9e7b-54a21d2fe7dc Repeat steps 6–7 using **Group alerts by Rule name → Take actions → Mark as closed** on a group row to reproduce the second failure path. https://github.com/user-attachments/assets/98aad712-8386-42eb-91c2-9fd8ff3cf2c0 ## How to verify the fix in this branch 1. Follow steps 1–5 above. 3. Select all → Mark as closed. The toast should now show a non-zero count. 4. Refresh the alerts table and confirm the alerts are gone (or switch to the Closed filter). 5. Repeat via the group take-actions menu — the count should also be non-zero. 6. **Regression check:** create a rule exception with "Close all matching alerts" ticked, using a field from the rule source index (not a data view runtime field). The close should still work — this exercises the older `_source`-reader path that must remain intact. 7. **Scripted transforming field check:** add a script that transforms the value (e.g. `.toLowerCase()`), filter the grid on the transformed value, and select all → close. The closed count should equal the number of visible rows — not a superset or subset. **Fixed** https://github.com/user-attachments/assets/7f87cd8b-a0c4-48ac-8a55-c1f02448c5b0 ## Security note — inline Painless scripts in a public API The new `runtime_mappings` body parameter accepts inline Painless scripts on `POST /api/detection_engine/signals/status` (`access: 'public'`). A caller can supply an arbitrary Painless expression that Elasticsearch evaluates inside the `_update_by_query` filter context. The risk is equivalent to (and no greater than) the existing data view scripted-field feature, which already allows any authenticated user to write arbitrary Painless scripts that run in this same ES context. The key mitigations: - **Privilege gate is more restrictive than data views** — callers must hold `ALERTS_API_ALL` (or the deprecated update privilege); scripted data view fields require only the user's data-view access. - **Inline-only** — stored script IDs are rejected by schema validation; the script cannot reference external artefacts. - **ES Painless sandbox** — scripts run in the ES security manager sandbox; they cannot make network calls, access the filesystem, or reach other indices. - **`asCurrentUser`** — the `_update_by_query` executes with the caller's own ES privileges; no privilege escalation occurs. - **Schema bounds** — `script.source` is capped at 10 000 characters; `runtime_mappings` accepts at most 100 unique field names combined with `runtime_fields`. ## Test plan - [x] Unit tests — script preservation, composite/lookup filtering, group actions forwarding, merge precedence, `on_script_error` on scripted entries only, combined-cap validation (49 tests across 4 files) - [x] API integration tests — scripted runtime field close returns `updated > 0`; `_source`-reading field close returns `updated > 0` (both in `alert_status.ts`, running against a live ES stack with auditbeat data) - [x] Manual E2E as described above ## Checklist - [x] Any text added follows EUI's writing guidelines - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) Closes: elastic/security-team#19185 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Florent LB <florent.leborgne@elastic.co> (cherry picked from commit afaff8e) # Conflicts: # x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/signals/set_signal_status/set_signals_status_route.gen.ts # x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/signals/set_signal_status/set_signals_status_route.schema.yaml # x-pack/solutions/security/plugins/security_solution/public/common/components/toolbar/bulk_actions/update_alerts.ts # x-pack/solutions/security/plugins/security_solution/public/detections/components/alerts_table/alerts_sub_grouping.tsx # x-pack/solutions/security/plugins/security_solution/public/detections/containers/detection_engine/alerts/types.ts # x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/routes/signals/bulk_close_runtime_mappings.test.ts # x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/routes/signals/open_close_signals_route.test.ts # x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/routes/signals/open_close_signals_route.ts
…lity - Widen asField type in bulk_close_runtime_mappings.test.ts to include on_script_error (not yet in @elastic/elasticsearch TS types) - Remove SecuritySolutionEventBus import (events/event_bus module not in 9.4) - Fix setSignalsStatusRoute call to 3 args (event bus 4th arg not in 9.4) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Pull request overview
Backports the Security Solution bulk-close fix to preserve data view runtime field Painless scripts end-to-end and ensure group “take actions” bulk close forwards runtime mappings correctly, preventing “Successfully closed 0 alerts” and incorrect close sets.
Changes:
- Add
runtime_mappingsrequest support (schema + client + server) and merge logic to preserve full runtime field definitions (including scripts). - Enforce a combined per-request cap across
runtime_fieldsandruntime_mappings(unique-name union). - Extend unit/integration tests to cover scripted runtime fields and group take-actions forwarding plus new mapping-merging behavior.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| x-pack/solutions/security/test/security_solution_api_integration/test_suites/detections_response/detection_engine/alerts/basic_license_essentials_tier/alert_status/alert_status.ts | Adds API integration regressions proving scripted and _source-reading runtime_mappings actually affect bulk close-by-query. |
| x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/routes/signals/open_close_signals_route.ts | Adds runtime_mappings handling, combined key-limit enforcement, and merges synthesized + passthrough mappings before ES _update_by_query. |
| x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/routes/signals/open_close_signals_route.test.ts | Adds route-level tests for runtime mapping limits and other behaviors. |
| x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/routes/signals/bulk_close_runtime_mappings.ts | Introduces BulkCloseRuntimeMappings type and merge helper that preserves scripts and stamps on_script_error appropriately. |
| x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/routes/signals/bulk_close_runtime_mappings.test.ts | Adds unit tests for synthesized runtime fields and new merge precedence/shape rules. |
| x-pack/solutions/security/plugins/security_solution/public/detections/hooks/trigger_actions_alert_table/use_alert_actions.tsx | Switches from projecting runtime field types to forwarding narrowed runtime_mappings that preserve scripts. |
| x-pack/solutions/security/plugins/security_solution/public/detections/hooks/trigger_actions_alert_table/use_alert_actions.test.tsx | Updates tests to assert scripts are preserved and unsupported types are dropped before forwarding. |
| x-pack/solutions/security/plugins/security_solution/public/detections/hooks/alerts_table/use_group_take_action_items.tsx | Wires group take-actions to forward runtime mappings (via converter) into bulk status updates. |
| x-pack/solutions/security/plugins/security_solution/public/detections/hooks/alerts_table/use_group_take_action_items.test.tsx | Adds tests asserting runtime mappings are forwarded for group actions. |
| x-pack/solutions/security/plugins/security_solution/public/detections/containers/detection_engine/alerts/types.ts | Extends the by-query status update props with runtimeMappings documentation/type. |
| x-pack/solutions/security/plugins/security_solution/public/detections/containers/detection_engine/alerts/api.ts | Sends runtime_mappings in the status update by-query HTTP body. |
| x-pack/solutions/security/plugins/security_solution/public/detections/components/alerts_table/types.ts | Extends group take-action props to include optional runtime mappings. |
| x-pack/solutions/security/plugins/security_solution/public/detections/components/alerts_table/alerts_sub_grouping.tsx | Forwards page-scoped runtime mappings into group take-actions. |
| x-pack/solutions/security/plugins/security_solution/public/common/components/toolbar/bulk_actions/update_alerts.ts | Threads runtimeMappings through the bulk status update helper API. |
| x-pack/solutions/security/plugins/security_solution/public/common/components/toolbar/bulk_actions/runtime_mappings_for_bulk_close.ts | Adds shared client-side converter to narrow data view runtime mappings to the route-accepted shape while preserving scripts. |
| x-pack/solutions/security/plugins/security_solution/public/common/components/toolbar/bulk_actions/runtime_mappings_for_bulk_close.test.ts | Adds unit tests for the converter: supported types, script normalization, and dropping unsupported script shapes. |
| x-pack/solutions/security/plugins/security_solution/docs/openapi/serverless/security_solution_detections_api_2023_10_31.bundled.schema.yaml | Documents new runtime_mappings parameter and RuntimeFieldMapping schema (serverless). |
| x-pack/solutions/security/plugins/security_solution/docs/openapi/ess/security_solution_detections_api_2023_10_31.bundled.schema.yaml | Documents new runtime_mappings parameter and RuntimeFieldMapping schema (ESS). |
| x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/signals/set_signal_status/set_signals_status_route.schema.yaml | Adds runtime_mappings and RuntimeFieldMapping to the API schema source. |
| x-pack/solutions/security/plugins/security_solution/common/api/detection_engine/signals/set_signal_status/set_signals_status_route.gen.ts | Updates generated Zod schema/types to include runtime_mappings and RuntimeFieldMapping. |
| oas_docs/output/kibana.yaml | Updates generated OAS output to include runtime_mappings + schema. |
| oas_docs/output/kibana.serverless.yaml | Updates generated serverless OAS output to include runtime_mappings + schema. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… workflow tests - Remove 'workflow trigger emission' describe block: setSignalsStatusRoute on 9.4 takes 3 args (no event-bus 4th param), so mockEventBus was never wired in and all emitAlertStatusChanged assertions could never pass - Add jest.clearAllMocks() at top of outer beforeEach in use_group_take_action_items.test.tsx to prevent order-dependent failures - Fix comment: 'every passthrough entry' → 'every scripted passthrough entry' in bulk_close_runtime_mappings.ts (on_script_error is only set when a script is present, not on scriptless entries) - Fix inaccurate union-size comment in open_close_signals_route.test.ts: the test has 100 keys in runtime_fields (99 rf_field_* + sharedKey) and 1 in runtime_mappings (sharedKey) — union is 100, not 99 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eField The snapshot test was added in the backport but the .snap file was not committed, causing CI to fail (snapshots are not auto-written in CI). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
cc @jonwalstedt |
nikitaindik
left a comment
There was a problem hiding this comment.
Checked changes against the base PR. LGTM.
Backport
This will backport the following commits from
mainto9.4:Questions ?
Please refer to the Backport tool documentation