Skip to content

[tech-debt] Make array-shaped ECS alert fields safe generically, not via a hand-maintained field allowlist #49

Description

@maxcold

Context

PR #48 (fixing #47) ships a short-term fix for the Alert Triage blank-panel crash: some ECS alert fields (host.name, user.name, process.name, source.ip, …) come back from Elasticsearch as arrays instead of the scalars SecurityAlert._source types promise, and unguarded .localeCompare() in useAlertSort.ts threw and blanked the widget.

That fix is intentionally pragmatic. This issue tracks doing it properly going forward.

Why the short-term fix is not the final shape

The PR #48 fix has two layers at two different altitudes:

  1. normalizeAlertEntityFields() in AlertsService.getAlerts() — a hand-maintained allowlist of specific nested fields to coerce (host.name, user.name/domain, process.name/executable, process.parent.name/executable, file.name/path, source.ip, destination.ip).

    • It manually duplicates the structure of the SecurityAlert._source type. Nothing (type, test, lint) fails when the list drifts from reality.
    • Any field not on the list that arrives array-shaped is untouched. Notably sortAlerts()'s "rule" case calls kibana.alert.rule.name.localeCompare(...)not normalized and not guarded — so the identical crash class can still occur there.
    • It's a special case layered on shared infrastructure — a sign the fix isn't at the right depth.
  2. ErrorBoundary wrapping the Alert Triage view root — the genuinely general safety net, but it only degrades crash → visible fallback message, not crash → correctly rendered data. A new array-shaped field in a sort key still collapses the entire view to the fallback.

  3. Coverage gap: the ErrorBoundary is reusable but applied to only 1 of 6 views. detection-rules, case-management, and attack-discovery use the same .localeCompare() sort pattern and are unwrapped.

Definition of done

A field arriving as an array instead of a scalar should never crash a view, and the protection should not depend on remembering to add each field to a manual list. Pick one (or combine):

  • Make read sites array-safe generically (preferred). Introduce a single scalar-field accessor (e.g. scalarField(src, "host.name")) and use it at every sort/group/compare site in the alert-triage hooks, so the crash class is removed at the point of use regardless of which field is multi-valued. This likely makes normalizeAlertEntityFields largely unnecessary.
  • Or normalize at the deserialization boundary driven by a single source of truth (the type/schema), so the scalar-field list cannot silently drift.
  • Cover the currently-unguarded "rule" sort key (and audit other .localeCompare/.toLowerCase/new Date call sites on ES-sourced fields).
  • Decide on ErrorBoundary scope for the remaining views (detection-rules, case-management, attack-discovery) — either wrap their roots or consciously document why not.
  • Consider whether finer-grained boundary placement (per section/group) is worth it so a single bad row degrades gracefully instead of blanking the whole view.

Out of scope

Related: #47, PR #48

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions