diff --git a/src/elastic/service/alertsService.test.ts b/src/elastic/service/alertsService.test.ts index 10e447b..a051a17 100644 --- a/src/elastic/service/alertsService.test.ts +++ b/src/elastic/service/alertsService.test.ts @@ -193,6 +193,92 @@ describe("AlertsService", () => { alerts: [], }); }); + + // Regression test for a real bug: some ECS alert fields (host.name, + // user.name/domain, process.name/executable, source.ip, destination.ip) + // come back from Elasticsearch as arrays instead of scalars for certain + // correlated/network-derived alerts, even though `SecurityAlert._source` + // types them as scalars. Downstream sort/group logic in the Alert Triage + // view (useAlertSort.ts) assumed a scalar and threw on `.localeCompare()` + // when it got an array, crashing the whole widget to a blank panel with + // no error boundary to catch it. getAlerts() must normalize these fields + // before returning so every caller sees the scalar shape the type promises. + it("normalizes array-shaped ECS entity/IP fields to scalars", async () => { + const alertsClient = createMockAlertsClient(); + vi.mocked(alertsClient.searchAlerts).mockResolvedValueOnce({ + hits: { + total: { value: 1 }, + hits: [ + { + _id: "a1", + _index: ".alerts-security.alerts-default", + _source: { + "@timestamp": "2024-01-01T00:00:00Z", + "kibana.alert.rule.name": "R", + "kibana.alert.rule.uuid": "r", + "kibana.alert.severity": "low", + "kibana.alert.risk_score": 21, + "kibana.alert.workflow_status": "open", + "kibana.alert.reason": "x", + host: { name: ["sa-da-vm-lls-01"] }, + user: { name: ["root"], domain: ["CORP"] }, + process: { + name: ["python3.9"], + executable: ["/usr/bin/python3.9"], + parent: { name: ["bash"] }, + }, + source: { ip: ["10.132.0.108"] }, + destination: { ip: ["35.195.130.253"] }, + } as unknown as SecurityAlert["_source"], + }, + ], + }, + }); + + const service = new AlertsService({ alertsClient }); + const out = await service.getAlerts(); + + expect(out.alerts).toHaveLength(1); + const src = out.alerts[0]._source; + expect(src.host?.name).toBe("sa-da-vm-lls-01"); + expect(src.user?.name).toBe("root"); + expect(src.user?.domain).toBe("CORP"); + expect(src.process?.name).toBe("python3.9"); + expect(src.process?.executable).toBe("/usr/bin/python3.9"); + expect(src.process?.parent?.name).toBe("bash"); + expect(src.source?.ip).toBe("10.132.0.108"); + expect(src.destination?.ip).toBe("35.195.130.253"); + }); + + it("leaves already-scalar ECS entity fields untouched", async () => { + const alertsClient = createMockAlertsClient(); + vi.mocked(alertsClient.searchAlerts).mockResolvedValueOnce({ + hits: { + total: { value: 1 }, + hits: [ + { + _id: "a2", + _index: ".alerts-security.alerts-default", + _source: { + "@timestamp": "2024-01-01T00:00:00Z", + "kibana.alert.rule.name": "R", + "kibana.alert.rule.uuid": "r", + "kibana.alert.severity": "high", + "kibana.alert.risk_score": 47, + "kibana.alert.workflow_status": "open", + "kibana.alert.reason": "x", + host: { name: "sa-da-ingest-01" }, + }, + }, + ], + }, + }); + + const service = new AlertsService({ alertsClient }); + const out = await service.getAlerts(); + + expect(out.alerts[0]._source.host?.name).toBe("sa-da-ingest-01"); + }); }); describe("getAlertContext", () => { diff --git a/src/elastic/service/alertsService.ts b/src/elastic/service/alertsService.ts index c907a85..49830c8 100644 --- a/src/elastic/service/alertsService.ts +++ b/src/elastic/service/alertsService.ts @@ -13,6 +13,7 @@ import type { SecurityAlert, } from "../../shared/types.js"; import type { AlertsClient } from "../client/alertsClient.js"; +import { toScalar } from "../../shared/field-utils.js"; /** Time window (ms) on either side of an alert used to gather host context. */ const CONTEXT_WINDOW_MS = 5 * 60 * 1000; @@ -116,7 +117,7 @@ export class AlertsService { name: b.key, count: b.doc_count, })), - alerts: response.hits.hits, + alerts: response.hits.hits.map(normalizeAlertEntityFields), }; } @@ -242,6 +243,57 @@ export class AlertsService { } } +/** + * Some ECS fields (`host.name`, `user.name`, `process.name`, `source.ip`, ...) + * come back as arrays instead of scalars for certain correlated/network- + * derived alerts, even though {@link SecurityAlert}'s `_source` types them as + * scalars — Elasticsearch does not enforce that shape at the document level. + * Downstream code (sorting/grouping in the Alert Triage view) assumes a + * scalar and throws (`TypeError: ... .localeCompare is not a function`) if it + * gets an array, so normalize once here at the boundary rather than + * defensively at every read site. + * + * The `Record` reads below reflect reality: this is raw JSON + * off the wire, not yet as trustworthy as the `SecurityAlert` type the rest of + * the codebase assumes. `host.ip` is intentionally left untouched — it's + * already typed and consumed as `string[]`. + */ +function normalizeAlertEntityFields(alert: SecurityAlert): SecurityAlert { + const src = alert._source as Record; + const scalar = (value: unknown): string | undefined => + toScalar(value as string | string[] | undefined); + + const host = src.host as Record | undefined; + const user = src.user as Record | undefined; + const proc = src.process as Record | undefined; + const parent = proc?.parent as Record | undefined; + const file = src.file as Record | undefined; + const source = src.source as Record | undefined; + const destination = src.destination as Record | undefined; + + return { + ...alert, + _source: { + ...alert._source, + host: host ? { ...host, name: scalar(host.name) } : host, + user: user ? { ...user, name: scalar(user.name), domain: scalar(user.domain) } : user, + process: proc + ? { + ...proc, + name: scalar(proc.name), + executable: scalar(proc.executable), + parent: parent + ? { ...parent, name: scalar(parent.name), executable: scalar(parent.executable) } + : parent, + } + : proc, + file: file ? { ...file, name: scalar(file.name), path: scalar(file.path) } : file, + source: source ? { ...source, ip: scalar(source.ip) } : source, + destination: destination ? { ...destination, ip: scalar(destination.ip) } : destination, + } as SecurityAlert["_source"], + }; +} + function buildHostEventQuery( hostName: string, timeRange: TimeRange diff --git a/src/elastic/service/entityDetailService.ts b/src/elastic/service/entityDetailService.ts index 10e92d4..f3945a4 100644 --- a/src/elastic/service/entityDetailService.ts +++ b/src/elastic/service/entityDetailService.ts @@ -7,6 +7,7 @@ import type { EntityDetailClient } from "../client/entityDetailClient.js"; import type { EntityDetail } from "../client/entityDetailClient.js"; +import { toScalar } from "../../shared/field-utils.js"; interface EntityDetailServiceOptions { readonly entityDetailClient: EntityDetailClient; @@ -184,11 +185,11 @@ export class EntityDetailService { if (firstProc) { const os = firstProc["host.os.name"] || firstProc["host.os.platform"]; if (os) fields.push({ label: "OS", value: String(os) }); - const ips = firstProc["host.ip"]; - if (ips) { + const ip = toScalar(firstProc["host.ip"] as string | string[] | undefined); + if (ip) { fields.push({ label: "IP", - value: String(Array.isArray(ips) ? ips[0] : ips), + value: String(ip), mono: true, }); } diff --git a/src/shared/components/ErrorBoundary/ErrorBoundary.tsx b/src/shared/components/ErrorBoundary/ErrorBoundary.tsx new file mode 100644 index 0000000..4251a56 --- /dev/null +++ b/src/shared/components/ErrorBoundary/ErrorBoundary.tsx @@ -0,0 +1,55 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from "react"; +import { EmptyState } from "../States/States"; + +export interface ErrorBoundaryProps { + /** Replaces the default fallback message. */ + fallback?: React.ReactNode; + children: React.ReactNode; +} + +interface ErrorBoundaryState { + error: Error | null; +} + +/** + * Catches render-time exceptions anywhere in `children` and shows a fallback + * instead of letting React unmount the whole tree to a blank screen. + * + * MCP app views render inside a host-provided webview with no dev console + * visible to the end user — an uncaught render error otherwise looks + * identical to "nothing loaded," with no indication anything went wrong. + * Wrap each view's root render in this boundary so an unexpected data shape + * degrades to a visible message instead of a silent blank panel. + */ +export class ErrorBoundary extends React.Component { + state: ErrorBoundaryState = { error: null }; + + static getDerivedStateFromError(error: Error): ErrorBoundaryState { + return { error }; + } + + componentDidCatch(error: Error, info: React.ErrorInfo): void { + console.error("Unhandled render error:", error, info.componentStack); + } + + render(): React.ReactNode { + if (this.state.error) { + return ( + this.props.fallback ?? ( + + Something went wrong rendering this view. Try reloading — if it keeps + happening, the underlying data may be in an unexpected shape. + + ) + ); + } + return this.props.children; + } +} diff --git a/src/shared/components/index.ts b/src/shared/components/index.ts index 610c28d..350c607 100644 --- a/src/shared/components/index.ts +++ b/src/shared/components/index.ts @@ -41,6 +41,9 @@ export type { KpiStripProps, KpiTileProps, KpiAccent } from "./KpiStrip/KpiStrip export { LoadingState, EmptyState } from "./States/States"; export type { LoadingStateProps, EmptyStateProps } from "./States/States"; +export { ErrorBoundary } from "./ErrorBoundary/ErrorBoundary"; +export type { ErrorBoundaryProps } from "./ErrorBoundary/ErrorBoundary"; + export { DetailPane } from "./DetailPane/DetailPane"; export type { DetailPaneProps } from "./DetailPane/DetailPane"; diff --git a/src/shared/field-utils.test.ts b/src/shared/field-utils.test.ts new file mode 100644 index 0000000..a73580a --- /dev/null +++ b/src/shared/field-utils.test.ts @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { describe, expect, it } from "vitest"; +import { toScalar } from "./field-utils.js"; + +describe("toScalar", () => { + it("returns a scalar value unchanged", () => { + expect(toScalar("sa-da-vm-lls-01")).toBe("sa-da-vm-lls-01"); + }); + + it("returns the first element of a non-empty array", () => { + expect(toScalar(["sa-da-vm-lls-01", "sa-da-ingest-01"])).toBe("sa-da-vm-lls-01"); + }); + + it("returns undefined for an empty array", () => { + expect(toScalar([])).toBeUndefined(); + }); + + it("returns undefined for undefined", () => { + expect(toScalar(undefined)).toBeUndefined(); + }); +}); diff --git a/src/shared/field-utils.ts b/src/shared/field-utils.ts new file mode 100644 index 0000000..326c3a0 --- /dev/null +++ b/src/shared/field-utils.ts @@ -0,0 +1,25 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +/** + * Coerce an ECS field that may come back as either a scalar or an array (a + * multi-valued field, common on correlated/network-derived alerts) into a + * single scalar. + * + * Elasticsearch does not enforce single- vs. multi-valued fields at the + * document level, so the *same* mapped field can be a plain string on one + * document and a `string[]` on another. Code that expects a scalar (e.g. + * grouping/sorting by `host.name`) must normalize at the boundary rather than + * assume the shape declared in a TypeScript type actually holds at runtime. + * + * @returns the first element for a non-empty array, `undefined` for an empty + * array, or the value unchanged if it isn't an array. + */ +export function toScalar(value: T | T[] | undefined): T | undefined { + if (!Array.isArray(value)) return value; + return value.length > 0 ? value[0] : undefined; +} diff --git a/src/views/alert-triage/App.test.tsx b/src/views/alert-triage/App.test.tsx new file mode 100644 index 0000000..96aa0ae --- /dev/null +++ b/src/views/alert-triage/App.test.tsx @@ -0,0 +1,195 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { describe, it, expect, vi } from "vitest"; +import { render, fireEvent, waitFor, act } from "@testing-library/react"; +import type { App as McpApp } from "@modelcontextprotocol/ext-apps"; +import { McpAppContext, type McpAppContextValue } from "../../shared/hooks/McpAppContext.js"; +import { ToastProvider } from "../../shared/components/index.js"; +import { AppContent } from "./App.js"; +import type { AlertSummary, SecurityAlert } from "../../shared/types.js"; + +/** Mirrors the real `App`'s wrapping (`ToastProvider` + `McpAppProvider`), + * swapping the real provider/transport for a stubbed context. */ +function renderAppContent(ctx: McpAppContextValue) { + return render( + + + + + + ); +} + +interface FakeApp { + callServerTool: ReturnType; +} + +function makeContext(app: FakeApp): McpAppContextValue { + return { + app: app as unknown as McpApp, + getApp: () => app as unknown as McpApp, + connected: true, + bootstrapState: { status: "idle" }, + subscribeToToolResult: () => () => {}, + }; +} + +function makeAlert(id: string, host: string): SecurityAlert { + return { + _id: id, + _index: ".alerts-security.alerts-default", + _source: { + "@timestamp": "2024-01-01T00:00:00Z", + "kibana.alert.rule.name": "R", + "kibana.alert.rule.uuid": "r", + "kibana.alert.severity": "high", + "kibana.alert.risk_score": 73, + "kibana.alert.workflow_status": "open", + "kibana.alert.reason": "x", + host: { name: host }, + }, + }; +} + +function unfilteredSummary(): AlertSummary { + return { + total: 2, + bySeverity: { high: 2 }, + byRule: [{ name: "R", count: 2 }], + byHost: [ + { name: "host-a", count: 1 }, + { name: "host-b", count: 1 }, + ], + alerts: [makeAlert("a1", "host-a"), makeAlert("a2", "host-b")], + }; +} + +function filteredSummary(host: string): AlertSummary { + return { + total: 1, + bySeverity: { high: 1 }, + byRule: [{ name: "R", count: 1 }], + byHost: [{ name: host, count: 1 }], + alerts: [makeAlert("f1", host)], + }; +} + +function toolResult(summary: AlertSummary) { + return { content: [{ type: "text", text: JSON.stringify(summary) }] }; +} + +function pollAlertsCalls(callServerTool: ReturnType) { + return callServerTool.mock.calls.filter((call) => call[0]?.name === "poll-alerts"); +} + +describe("Alert Triage AppContent — filter clearing", () => { + it("clears the filter and reloads the unfiltered list when the input is emptied directly, not via Enter/Escape/the chip", async () => { + const callServerTool = vi.fn().mockImplementation(async ({ name, arguments: args }) => { + if (name !== "poll-alerts") return toolResult(unfilteredSummary()); + return toolResult(args?.query ? filteredSummary("host-x") : unfilteredSummary()); + }); + const ctx = makeContext({ callServerTool }); + + const { getByLabelText, queryByLabelText } = renderAppContent(ctx); + + // Initial unfiltered load on mount. + await waitFor(() => expect(pollAlertsCalls(callServerTool).length).toBeGreaterThan(0)); + + const input = getByLabelText("Filter") as HTMLInputElement; + + // Apply a filter the normal way: type + Enter. + fireEvent.change(input, { target: { value: "host-x" } }); + fireEvent.keyDown(input, { key: "Enter" }); + + await waitFor(() => expect(queryByLabelText("Clear filter")).toBeTruthy()); + const afterFilter = pollAlertsCalls(callServerTool); + expect(afterFilter[afterFilter.length - 1][0].arguments.query).toBe("host-x"); + + // Regression: clear by deleting the input's text directly — no Enter, + // no Escape, no click on the chip's "x". Before the fix, this only + // updated local text state and never re-queried, leaving the widget + // filtered on "host-x" even though the box (and, per the bug report, + // the chip) looked empty. + fireEvent.change(input, { target: { value: "" } }); + + await waitFor(() => { + const afterClear = pollAlertsCalls(callServerTool); + expect(afterClear[afterClear.length - 1][0].arguments.query).toBeUndefined(); + }); + + // The chip is driven by the same underlying filter state, so it should + // be gone too — proving the filter was actually cleared, not just the + // text box. + await waitFor(() => expect(queryByLabelText("Clear filter")).toBeNull()); + }); + + it("does not re-fire a reload when the input is already empty and no filter is active", async () => { + const callServerTool = vi.fn().mockResolvedValue(toolResult(unfilteredSummary())); + const ctx = makeContext({ callServerTool }); + + const { getByLabelText } = renderAppContent(ctx); + + await waitFor(() => expect(pollAlertsCalls(callServerTool).length).toBeGreaterThan(0)); + const countAfterMount = pollAlertsCalls(callServerTool).length; + + const input = getByLabelText("Filter") as HTMLInputElement; + // Typing and then deleting back to empty without ever having an active + // filter should not trigger an extra reload. + fireEvent.change(input, { target: { value: "h" } }); + fireEvent.change(input, { target: { value: "" } }); + + expect(pollAlertsCalls(callServerTool).length).toBe(countAfterMount); + }); +}); + +describe("Alert Triage AppContent — stale response guard", () => { + it("does not let an older, slower response overwrite a newer one that resolved first", async () => { + const deferred: Array<() => void> = []; + let pollCallIndex = 0; + + const callServerTool = vi.fn().mockImplementation(async ({ name, arguments: args }) => { + if (name !== "poll-alerts") return toolResult(unfilteredSummary()); + const idx = pollCallIndex++; + if (idx === 0) { + // Initial mount load — resolve immediately. + return toolResult(unfilteredSummary()); + } + return new Promise((resolve) => { + deferred[idx] = () => + resolve(toolResult(args?.query ? filteredSummary("host-old") : unfilteredSummary())); + }); + }); + const ctx = makeContext({ callServerTool }); + + const { getByLabelText, queryByText, getAllByText } = renderAppContent(ctx); + + await waitFor(() => expect(pollCallIndex).toBe(1)); + + const input = getByLabelText("Filter") as HTMLInputElement; + + // Request #1 (older): apply a filter — its response will be resolved LAST. + fireEvent.change(input, { target: { value: "host-old" } }); + fireEvent.keyDown(input, { key: "Enter" }); + await waitFor(() => expect(pollCallIndex).toBe(2)); + + // Request #2 (newer): clear it via the chip — its response will be + // resolved FIRST, simulating the older request's network round-trip + // taking longer than the newer one's. + fireEvent.click(getByLabelText("Clear filter")); + await waitFor(() => expect(pollCallIndex).toBe(3)); + + // Resolve out of order: newer (#2, unfiltered) first, older (#1, filtered) second. + await act(async () => deferred[2]()); + await act(async () => deferred[1]()); + + // The stale filtered response must have been discarded — the list + // should reflect the newer, unfiltered result, not "host-old". + expect(queryByText("host-old")).toBeNull(); + expect(getAllByText("host-a").length).toBeGreaterThan(0); + }); +}); diff --git a/src/views/alert-triage/App.tsx b/src/views/alert-triage/App.tsx index 1623419..bb63745 100644 --- a/src/views/alert-triage/App.tsx +++ b/src/views/alert-triage/App.tsx @@ -85,7 +85,11 @@ export function App() { ); } -function AppContent() { +// Exported (alongside the default `App`) so tests can render it directly +// against a stubbed `McpAppContext.Provider`, bypassing the real +// `McpAppProvider`/transport — mirrors the pattern used for the hook tests in +// src/shared/hooks/*.test.tsx. +export function AppContent() { const [summary, setSummary] = useState(null); const [selectedAlert, setSelectedAlert] = useState(null); const [alertContext, setAlertContext] = useState(null); @@ -103,19 +107,28 @@ function AppContent() { const [relatedOpen, setRelatedOpen] = useState(false); const paramsRef = useRef({ days: 7, limit: DEFAULT_LIMIT_NUM }); const listRef = useRef(null); + // Bumped at the start of every loadAlertsImpl call so a response that + // resolves after a newer request has already started can be told apart + // from the latest one — see requestId check below. + const requestIdRef = useRef(0); const loadAlertsImpl = useCallback(async (app: McpApp, overrideParams?: Partial) => { + const requestId = ++requestIdRef.current; setLoading(true); try { const args = { ...paramsRef.current, ...overrideParams }; if (overrideParams) paramsRef.current = { ...paramsRef.current, ...overrideParams }; const result = await app.callServerTool({ name: "poll-alerts", arguments: args }); + // A newer call started while this one was in flight (e.g. the 60s poll, + // or the user clearing/changing the filter) — its response should win, + // so discard this now-stale one rather than overwrite fresher data. + if (requestId !== requestIdRef.current) return; const text = extractCallResult(result); if (text) setSummary(JSON.parse(text)); } catch (e) { console.error("Load alerts failed:", e); } finally { - setLoading(false); + if (requestId === requestIdRef.current) setLoading(false); } }, []); @@ -432,6 +445,22 @@ function AppContent() { loadAlerts({ query: undefined }); }, [loadAlerts]); + /** + * `SearchInput`'s onChange fires on every keystroke but only updates local + * text state — it does not, by itself, clear the actual filter + * (`paramsRef.current.query`). Pressing Enter/Escape or clicking the chip's + * "x" all correctly clear it via `clearQuery`, but a user who selects the + * text and deletes it (or backspaces to empty) without pressing Enter would + * see an empty box and no chip while the list stayed filtered on the old + * value. Treat "input became empty" the same as clicking the chip's "x". + */ + const handleInputChange = useCallback((value: string) => { + setSearchInput(value); + if (value === "" && paramsRef.current.query) { + loadAlerts({ query: undefined }); + } + }, [loadAlerts]); + /** * Filter the alert list by a specific ECS field/value pair. * Called when the user clicks a dotted-underline fact value on a card or in the detail pane. @@ -647,7 +676,7 @@ function AppContent() { actions={ diff --git a/src/views/alert-triage/hooks/useAlertSort.test.ts b/src/views/alert-triage/hooks/useAlertSort.test.ts new file mode 100644 index 0000000..4e73d40 --- /dev/null +++ b/src/views/alert-triage/hooks/useAlertSort.test.ts @@ -0,0 +1,91 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { describe, expect, it } from "vitest"; +import { groupAlerts, sortAlerts } from "./useAlertSort.js"; +import type { SecurityAlert } from "../../../shared/types.js"; + +function makeAlert( + id: string, + overrides: Partial = {} +): SecurityAlert { + return { + _id: id, + _index: ".alerts-security.alerts-default", + _source: { + "@timestamp": "2024-01-01T00:00:00Z", + "kibana.alert.rule.name": "R", + "kibana.alert.rule.uuid": "r", + "kibana.alert.severity": "low", + "kibana.alert.risk_score": 21, + "kibana.alert.workflow_status": "open", + "kibana.alert.reason": "x", + ...overrides, + }, + }; +} + +// `AlertsService.getAlerts()` normalizes ECS entity fields to scalars before +// this hook ever sees them, but these tests exercise the hook's own +// defense-in-depth guards directly — in case a stray array-shaped field ever +// reaches it via a different path (e.g. a future bootstrap/mock data source). +const arrayShapedHost = { name: ["sa-da-vm-lls-01"] } as unknown as { name: string }; + +describe("sortAlerts", () => { + it("sorts by host without throwing when a host.name is an array", () => { + const alerts = [ + makeAlert("a1", { host: { name: "zzz-host" } }), + makeAlert("a2", { host: arrayShapedHost }), + ]; + + expect(() => sortAlerts(alerts, "host")).not.toThrow(); + }); +}); + +describe("groupAlerts", () => { + // Regression test: previously, an array-shaped host.name passed the + // `!key || !name` truthiness guard (a non-empty array is truthy), then + // crashed the group sort at `a.name.localeCompare(b.name)` — arrays have no + // `.localeCompare`. This exact crash rendered the Alert Triage widget as a + // blank panel with no error toast, against real alert data. + it("groups by host without throwing when one alert's host.name is an array", () => { + const alerts = [ + makeAlert("a1", { host: { name: "sa-da-ingest-01" } }), + makeAlert("a2", { host: arrayShapedHost }), + ]; + + expect(() => groupAlerts(alerts, "host")).not.toThrow(); + }); + + it("skips an alert whose host.name is an array rather than grouping it incorrectly", () => { + const alerts = [ + makeAlert("a1", { host: { name: "sa-da-ingest-01" } }), + makeAlert("a2", { host: arrayShapedHost }), + ]; + + const groups = groupAlerts(alerts, "host")!; + expect(groups).toHaveLength(1); + expect(groups[0].name).toBe("sa-da-ingest-01"); + expect(groups[0].alerts).toHaveLength(1); + }); + + it("groups normally when all host names are scalar strings", () => { + const alerts = [ + makeAlert("a1", { host: { name: "host-b" } }), + makeAlert("a2", { host: { name: "host-a" } }), + makeAlert("a3", { host: { name: "host-a" } }), + ]; + + const groups = groupAlerts(alerts, "host")!; + expect(groups.map((g) => g.name).sort()).toEqual(["host-a", "host-b"]); + expect(groups.find((g) => g.name === "host-a")!.alerts).toHaveLength(2); + }); + + it("returns null for groupBy 'none'", () => { + expect(groupAlerts([makeAlert("a1")], "none")).toBeNull(); + }); +}); diff --git a/src/views/alert-triage/hooks/useAlertSort.ts b/src/views/alert-triage/hooks/useAlertSort.ts index b07e84c..a426fbf 100644 --- a/src/views/alert-triage/hooks/useAlertSort.ts +++ b/src/views/alert-triage/hooks/useAlertSort.ts @@ -45,7 +45,8 @@ export function sortAlerts(alerts: SecurityAlert[], sortBy: SortKey): SecurityAl arr.sort((a, b) => (a._source["kibana.alert.rule.name"] || "").localeCompare(b._source["kibana.alert.rule.name"] || "")); break; case "host": - arr.sort((a, b) => (a._source.host?.name || "").localeCompare(b._source.host?.name || "")); + arr.sort((a, b) => + String(a._source.host?.name ?? "").localeCompare(String(b._source.host?.name ?? ""))); break; } return arr; @@ -74,7 +75,11 @@ export function groupAlerts(sortedAlerts: SecurityAlert[], groupBy: GroupKey): A key = name; subtitle = src.process?.executable || (src.process?.parent?.name ? `Parent ${src.process.parent.name}` : undefined); } - if (!key || !name) continue; + // `key`/`name` are expected to be scalar strings (normalized upstream in + // AlertsService.getAlerts()), but guard against a stray array slipping + // through — a non-empty array is truthy and would otherwise pass this + // check, then throw at the `.localeCompare()` call below. + if (typeof key !== "string" || !key || typeof name !== "string" || !name) continue; let bucket = buckets.get(key); if (!bucket) { bucket = { key, name, subtitle, topSeverity: "low", alerts: [] }; @@ -90,7 +95,7 @@ export function groupAlerts(sortedAlerts: SecurityAlert[], groupBy: GroupKey): A if (d !== 0) return d; const c = b.alerts.length - a.alerts.length; if (c !== 0) return c; - return a.name.localeCompare(b.name); + return String(a.name ?? "").localeCompare(String(b.name ?? "")); }); } diff --git a/src/views/alert-triage/mcp-app.tsx b/src/views/alert-triage/mcp-app.tsx index 7251dbf..be21f81 100644 --- a/src/views/alert-triage/mcp-app.tsx +++ b/src/views/alert-triage/mcp-app.tsx @@ -7,6 +7,11 @@ import React from "react"; import { createRoot } from "react-dom/client"; +import { ErrorBoundary } from "../../shared/components"; import { App } from "./App"; -createRoot(document.getElementById("root")!).render(); +createRoot(document.getElementById("root")!).render( + + + +);