elastic / elastic/example-mcp-app-security

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

Open
#49 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
TypeScript
Stars
24
Forks
16
Avg merge
1d 41m
Merged PRs (30d)
2

Description

## 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

- The immediate blank-panel fix — shipped in PR #48.

Related: #47, PR #48

Contributor guide

Open the contributing guide

Research direction

Start with normalizeAlertEntityFields() in AlertsService.getAlerts(), then inspect useAlertSort.ts and the sortAlerts() "rule" case for other Elasticsearch-sourced comparisons. Audit the alert-triage sort/group call sites and the ErrorBoundary coverage for detection-rules, case-management, and attack-discovery. Done means array-shaped fields cannot crash these views without relying on a hand-maintained allowlist.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend, security
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.