diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a28adc41..a5354d721 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,15 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### The audit page no longer says "Allowed" about six kinds of refusal + +A hop one Bot was not allowed to make, an endpoint this deployment would not dial, a rotation the +vault refused and a sign-in it turned away were all drawn as **Allowed**, in the muted colour every +ordinary row uses, and none of them appeared under **Blocked**. The same for a hop that ran out of +attempts and a question that reached nobody, which are "Did not happen" rather than allowed. The +page recognised six refusal types and the six added since were never added to it. Refusals now read +as refusals, the two saved views are built from the same lists the rows are labelled from, and a +refusal added later is added in one place or in none. ### A conversation deleted while a server was reconnecting no longer lingers on the screen Announcements between servers travel as Postgres notifications, which reach whoever is subscribed at diff --git a/app/src/lib/audit/outcome.ts b/app/src/lib/audit/outcome.ts new file mode 100644 index 000000000..be2af9e1b --- /dev/null +++ b/app/src/lib/audit/outcome.ts @@ -0,0 +1,83 @@ +/** + * Whether a row in the trail is something this deployment turned away, something that was allowed + * and then did not happen, or something that went through. + * + * THE TWO PLACES THAT DECIDE THIS HAVE TO BE ONE PLACE. The audit page asks the question twice: once + * to colour and label a row, and once to build the `eventType` list behind the `Blocked` and + * `Did not happen` saved views. They were two hand-written lists, and they had already drifted — a + * refusal missing from the first is drawn as "Allowed", a refusal missing from the second is + * missing from the view somebody clicks to ask what this deployment refused, and neither omission + * says anything. So the lists live here, the page derives both from them, and a new refusal is added + * in one place or in none. + * + * WHY "ALLOWED" IS THE ONE WRONG ANSWER. The page falls back to it for anything it does not + * recognise, which is the right default for the many rows that are neither a refusal nor a failure — + * a credential saved, a component published, a person's role changed. For a refusal it is not a + * missing label, it is the opposite of what happened, on the screen an administrator opens to find + * out what happened. A trail that is confidently wrong is worse than a silent one. + */ + +/** + * Refused: this deployment declined, and nothing was attempted. + * + * `mcp.callback_refused`, `routines.dispatch_refused` and `session.refused` are here even though no + * policy judged anybody and, in the last two, no Bot was involved at all. Somebody filtering for + * "what did this deployment turn away" wants them, and for each of the three this row is the only + * evidence anywhere that anything was attempted: the wire answers all of them with the same opaque + * refusal on purpose. + */ +export const REFUSED_EVENT_TYPES = [ + "computer.action_refused", + "component.refused", + "component.function_refused", + "mcp.call_rejected", + "mcp.callback_refused", + "routines.dispatch_refused", + /* + * A hop one Bot was not allowed to make. `server/src/audit.ts` calls this "the more important of + * the pair": a hop that happened is visible in the transcript anyway, and a hop that was refused + * is invisible everywhere else. + */ + "agent.handoff_refused", + /** An endpoint a stored agent tried to reach and the deployment would not dial. */ + "agent.dial_refused", + /** A rotation aimed at a key the credential does not belong to, or at a revoked one. */ + "credential.rotation_refused", + /** A revoked person still holding a bookmark, or an address outside the deployment. */ + "session.refused", +] as const; + +/** + * Did not happen: nothing was refused, and nothing came of it either. + * + * Its own family because the difference is what somebody came to the row to find out. A boundary + * holding and a Bot that was asked and never answered are different faults with different fixes, and + * only one of them is the deployment working as configured. + */ +export const DID_NOT_HAPPEN_EVENT_TYPES = [ + "computer.action_failed", + "agent.stream_stalled", + /** A hop that was accepted, ran out of attempts, and never became the other Bot's turn. */ + "agent.handoff_failed", + /** A question that reached nobody: the Bot stopped, and the person was never asked. */ + "agent.escalation_failed", +] as const; + +export type AuditOutcome = "refused" | "did-not-happen" | "allowed"; + +const REFUSED = new Set(REFUSED_EVENT_TYPES); +const DID_NOT_HAPPEN = new Set(DID_NOT_HAPPEN_EVENT_TYPES); + +/** What kind of thing this row is, for the label and the colour it is drawn in. */ +export function outcomeOf(eventType: string): AuditOutcome { + if (REFUSED.has(eventType)) return "refused"; + if (DID_NOT_HAPPEN.has(eventType)) return "did-not-happen"; + return "allowed"; +} + +/** The `eventType` query one of the saved views filters by. */ +export function eventTypeFilter( + types: readonly string[], +): `?eventType=${string}` { + return `?eventType=${types.join(",")}`; +} diff --git a/app/src/routes/_authed/admin/audit.tsx b/app/src/routes/_authed/admin/audit.tsx index 403294adb..18b4d98e5 100644 --- a/app/src/routes/_authed/admin/audit.tsx +++ b/app/src/routes/_authed/admin/audit.tsx @@ -9,6 +9,12 @@ import { } from "@/components/layout/page-shell"; import { Button } from "@/components/ui/button"; import { useBotNames } from "@/lib/agents/bot-names"; +import { + DID_NOT_HAPPEN_EVENT_TYPES, + eventTypeFilter, + outcomeOf, + REFUSED_EVENT_TYPES, +} from "@/lib/audit/outcome"; import { auditEventsQueryOptions } from "@/lib/audit/queries"; import { silenceOf } from "@/lib/audit/silence"; @@ -30,32 +36,21 @@ type AuditEvent = { createdAt: string; }; +/* + * The saved views, built from the same lists the row label and colour are decided by. + * + * Written out by hand here and again below, they had already drifted: a refusal on one list and not + * the other is a row drawn as "Allowed" or a row missing from the view somebody clicks to ask what + * this deployment refused. See `@/lib/audit/outcome`, which is now the only place either question is + * answered. + */ const FILTERS = [ { label: "Everything", search: "" }, { label: "Computer actions", search: "?eventType=computer.action_allowed" }, - { - label: "Blocked", - /* - * Include every refusal family, not only browser policy refusals. - * - * `mcp.callback_refused` is here because it is a refusal, even though nothing about a Bot was - * judged: a caller could not prove which Bot it was. Somebody filtering for what this deployment - * turned away wants that in the list, and it is the one refusal with no policy behind it, so - * leaving it out would hide the only evidence that anything was attempted. - * - * `routines.dispatch_refused` is the same shape one boundary over: the worker, not a Bot, and a - * stale or missing secret rather than a policy decision. The same reasoning that put - * `mcp.callback_refused` here applies unchanged — nobody was judged, something was still turned - * away, and the saved view a person clicks for "what did this deployment block" should show it. - */ - search: - "?eventType=computer.action_refused,mcp.call_rejected,mcp.callback_refused,component.refused,component.function_refused,routines.dispatch_refused", - }, + { label: "Blocked", search: eventTypeFilter(REFUSED_EVENT_TYPES) }, { label: "Did not happen", - // A stalled stream belongs here. It is the same complaint as an action that was allowed and then - // did not take: nothing was refused, and nothing came of it either. - search: "?eventType=computer.action_failed,agent.stream_stalled", + search: eventTypeFilter(DID_NOT_HAPPEN_EVENT_TYPES), }, ] as const; @@ -148,19 +143,8 @@ function Row({ | { role?: string; name?: string } | string | undefined; - const refused = - event.eventType === "computer.action_refused" || - event.eventType === "component.refused" || - event.eventType === "component.function_refused" || - event.eventType === "mcp.call_rejected" || - /* - * A caller that could not prove which Bot it was. Refused like the others, and it has to read - * that way here: the fallback below calls anything it does not recognise "Allowed", which for a - * refusal is the one wrong answer. A trail that is confidently wrong is worse than a silent one. - */ - event.eventType === "mcp.callback_refused" || - // The worker turned away at the door, same reasoning as the caller above. - event.eventType === "routines.dispatch_refused"; + const outcome = outcomeOf(event.eventType); + const refused = outcome === "refused"; const stalled = event.eventType === "agent.stream_stalled"; /* * Three different things, and the difference is what somebody comes to this row to find out. @@ -181,7 +165,7 @@ function Row({ // Allowed by policy but not carried out. A stalled turn belongs in the same family: the Bot was // asked and the answer never arrived. Colour is how this table is read, and a row left in the // muted foreground reads as "Allowed", which a turn nobody ever got an answer to was not. - const failed = event.eventType === "computer.action_failed" || stalled; + const failed = outcome === "did-not-happen"; const silence = stalled ? silenceOf(payload) : null; return ( diff --git a/app/tests/audit-outcome.test.ts b/app/tests/audit-outcome.test.ts new file mode 100644 index 000000000..8aa1e5916 --- /dev/null +++ b/app/tests/audit-outcome.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, test } from "bun:test"; +import { + DID_NOT_HAPPEN_EVENT_TYPES, + eventTypeFilter, + outcomeOf, + REFUSED_EVENT_TYPES, +} from "../src/lib/audit/outcome"; + +/** + * What the audit page says about a row, which for a refusal has exactly one wrong answer. + * + * The page falls back to "Allowed" for anything it does not recognise, which is right for the many + * rows that are neither a refusal nor a failure. For a refusal it is the opposite of what happened, + * on the screen somebody opens to find out what happened. + */ + +describe("what the trail says a row was", () => { + test("names a hop a boundary refused as a refusal", () => { + expect(outcomeOf("agent.handoff_refused")).toBe("refused"); + }); + + test("names a sign-in turned away as a refusal", () => { + expect(outcomeOf("session.refused")).toBe("refused"); + }); + + test("names a rotation the vault refused as a refusal", () => { + expect(outcomeOf("credential.rotation_refused")).toBe("refused"); + }); + + test("names an endpoint this deployment would not dial as a refusal", () => { + expect(outcomeOf("agent.dial_refused")).toBe("refused"); + }); + + test("keeps the refusals that were already named", () => { + for (const eventType of [ + "computer.action_refused", + "component.refused", + "component.function_refused", + "mcp.call_rejected", + "mcp.callback_refused", + "routines.dispatch_refused", + ]) { + expect(outcomeOf(eventType)).toBe("refused"); + } + }); + + test("tells a hop that never landed from one that was refused", () => { + // Nothing was refused: the hop was accepted, tried, and ran out of attempts. + expect(outcomeOf("agent.handoff_failed")).toBe("did-not-happen"); + // And a question that reached nobody, which nothing else anywhere records. + expect(outcomeOf("agent.escalation_failed")).toBe("did-not-happen"); + expect(outcomeOf("computer.action_failed")).toBe("did-not-happen"); + expect(outcomeOf("agent.stream_stalled")).toBe("did-not-happen"); + }); + + test("still calls something that went through allowed", () => { + for (const eventType of [ + "computer.action_allowed", + "mcp.call_succeeded", + "agent.handoff_delivered", + "agent.escalated", + "credential.created", + "session.signed_in", + ]) { + expect(outcomeOf(eventType)).toBe("allowed"); + } + }); + + test("does not call an unknown row a refusal", () => { + // The fallback has to stay open: a row type this build has never heard of is not a refusal, and + // claiming otherwise would be the same fault in the other direction. + expect(outcomeOf("something.nobody.has.written.yet")).toBe("allowed"); + }); +}); + +describe("the saved views ask the same question the rows do", () => { + /* + * The drift this exists to stop. Two hand-written lists meant a refusal could be drawn correctly + * on the row and be absent from the view somebody clicks to ask what this deployment refused — + * which is the harder failure to notice, because the view is not empty, it is just short. + */ + test("Blocked filters by every event type drawn as a refusal", () => { + const filtered = eventTypeFilter(REFUSED_EVENT_TYPES) + .replace("?eventType=", "") + .split(","); + + expect(filtered).toEqual([...REFUSED_EVENT_TYPES]); + for (const eventType of filtered) { + expect(outcomeOf(eventType)).toBe("refused"); + } + }); + + test("Did not happen filters by every event type drawn that way", () => { + const filtered = eventTypeFilter(DID_NOT_HAPPEN_EVENT_TYPES) + .replace("?eventType=", "") + .split(","); + + expect(filtered).toEqual([...DID_NOT_HAPPEN_EVENT_TYPES]); + for (const eventType of filtered) { + expect(outcomeOf(eventType)).toBe("did-not-happen"); + } + }); + + test("no event type is in both families", () => { + const refused = new Set(REFUSED_EVENT_TYPES); + for (const eventType of DID_NOT_HAPPEN_EVENT_TYPES) { + expect(refused.has(eventType)).toBe(false); + } + }); +});