From 2585edba2efda9ff50e6bed0a6de8cfaef8254fc Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 15 Aug 2026 18:53:00 -0700 Subject: [PATCH] fix(audit): show valid values for rejected filters (#124336) * fix(audit): validate filter flags before gateway call * refactor(audit): keep filter constants owner-local * fix(audit): keep cursor errors operator-facing --- .../src/schema/audit-activity.ts | 45 ++++++------ src/cli/program/register.audit.ts | 14 ++-- src/commands/audit.test.ts | 71 +++++++++++++++++-- src/commands/audit.ts | 34 +++++++-- 4 files changed, 126 insertions(+), 38 deletions(-) diff --git a/packages/gateway-protocol/src/schema/audit-activity.ts b/packages/gateway-protocol/src/schema/audit-activity.ts index 51606b384b9..b3057f72bdc 100644 --- a/packages/gateway-protocol/src/schema/audit-activity.ts +++ b/packages/gateway-protocol/src/schema/audit-activity.ts @@ -5,26 +5,27 @@ import { NonEmptyString } from "./primitives.js"; const AuditActivitySchemaVersionV1Schema = Type.Integer({ minimum: 1, maximum: 1 }); -const AuditActivityStatusV1Schema: TSchema = Type.Union([ - Type.Literal("started"), - Type.Literal("succeeded"), - Type.Literal("failed"), - Type.Literal("cancelled"), - Type.Literal("timed_out"), - Type.Literal("blocked"), - Type.Literal("unknown"), -]); +export const AUDIT_ACTIVITY_STATUSES = [ + "started", + "succeeded", + "failed", + "cancelled", + "timed_out", + "blocked", + "unknown", +] as const; +export const AUDIT_ACTIVITY_KINDS = ["agent_run", "tool_action", "message"] as const; +export const AUDIT_ACTIVITY_DIRECTIONS = ["inbound", "outbound"] as const; -const AuditActivityKindV1Schema: TSchema = Type.Union([ - Type.Literal("agent_run"), - Type.Literal("tool_action"), - Type.Literal("message"), -]); - -const AuditActivityDirectionV1Schema: TSchema = Type.Union([ - Type.Literal("inbound"), - Type.Literal("outbound"), -]); +const AuditActivityStatusV1Schema: TSchema = Type.Union( + AUDIT_ACTIVITY_STATUSES.map((value) => Type.Literal(value)), +); +const AuditActivityKindV1Schema: TSchema = Type.Union( + AUDIT_ACTIVITY_KINDS.map((value) => Type.Literal(value)), +); +const AuditActivityDirectionV1Schema: TSchema = Type.Union( + AUDIT_ACTIVITY_DIRECTIONS.map((value) => Type.Literal(value)), +); const AuditActivityConversationKindV1Schema = Type.Union([ Type.Literal("direct"), @@ -607,9 +608,9 @@ export type AuditActivityListParams = { agentId?: string; sessionKey?: string; runId?: string; - kind?: "agent_run" | "tool_action" | "message"; - status?: "started" | "succeeded" | "failed" | "cancelled" | "timed_out" | "blocked" | "unknown"; - direction?: "inbound" | "outbound"; + kind?: (typeof AUDIT_ACTIVITY_KINDS)[number]; + status?: (typeof AUDIT_ACTIVITY_STATUSES)[number]; + direction?: (typeof AUDIT_ACTIVITY_DIRECTIONS)[number]; channel?: string; after?: number; before?: number; diff --git a/src/cli/program/register.audit.ts b/src/cli/program/register.audit.ts index 81d689d958a..a1f895dd50f 100644 --- a/src/cli/program/register.audit.ts +++ b/src/cli/program/register.audit.ts @@ -1,9 +1,15 @@ // Audit command registration for privacy-preserving activity history. import type { Command } from "commander"; +import { + AUDIT_ACTIVITY_DIRECTIONS, + AUDIT_ACTIVITY_KINDS, + AUDIT_ACTIVITY_STATUSES, +} from "../../../packages/gateway-protocol/src/schema/audit-activity.js"; import { formatDocsLink } from "../../../packages/terminal-core/src/links.js"; import { theme } from "../../../packages/terminal-core/src/theme.js"; import { auditListCommand, type AuditListCommandOptions } from "../../commands/audit.js"; import { defaultRuntime } from "../../runtime.js"; +import { formatHumanList } from "../../shared/human-list.js"; import { runCommandWithRuntime } from "../cli-utils.js"; /** Register the bounded operator audit query command. */ @@ -15,12 +21,12 @@ export function registerAuditCommand(program: Command): void { .option("--session ", "Filter by exact session key") .option("--run ", "Filter by run id") .option("--execution ", "Inspect one exact execution id") - .option("--kind ", "Filter by kind (agent_run, tool_action, or message)") + .option("--kind ", `Filter by kind (${formatHumanList(AUDIT_ACTIVITY_KINDS)})`) + .option("--status ", `Filter by status (${formatHumanList(AUDIT_ACTIVITY_STATUSES)})`) .option( - "--status ", - "Filter by status (started, succeeded, failed, cancelled, timed_out, blocked, unknown)", + "--direction ", + `Filter message direction (${formatHumanList(AUDIT_ACTIVITY_DIRECTIONS)})`, ) - .option("--direction ", "Filter message direction (inbound or outbound)") .option("--channel ", "Filter message channel") .option("--after ", "Include records at/after ISO time or Unix milliseconds") .option("--before ", "Include records at/before ISO time or Unix milliseconds") diff --git a/src/commands/audit.test.ts b/src/commands/audit.test.ts index ad602ad913c..21338ad3fda 100644 --- a/src/commands/audit.test.ts +++ b/src/commands/audit.test.ts @@ -1,4 +1,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + AUDIT_ACTIVITY_DIRECTIONS, + AUDIT_ACTIVITY_KINDS, + AUDIT_ACTIVITY_STATUSES, +} from "../../packages/gateway-protocol/src/schema/audit-activity.js"; +import { runCommandWithRuntime } from "../cli/cli-utils.js"; import type { RuntimeEnv } from "../runtime.js"; import { auditListCommand } from "./audit.js"; @@ -107,13 +113,39 @@ describe("audit command parsing", () => { expect(callGateway).not.toHaveBeenCalled(); }); - it("rejects unknown event kinds before querying the Gateway", async () => { - await expect( - auditListCommand({ kind: "bogus" as never, limit: "10" }, runtime), - ).rejects.toThrow("--kind must be agent_run, tool_action, or message"); + it.each([ + { + options: { kind: "bogus" as never }, + message: "--kind must be agent_run, tool_action, or message.", + }, + { + options: { status: "bogus" as never }, + message: + "--status must be started, succeeded, failed, cancelled, timed_out, blocked, or unknown.", + }, + { + options: { direction: "sideways" as never }, + message: "--direction must be inbound or outbound.", + }, + ])("rejects invalid audit filters before querying the Gateway", async ({ options, message }) => { + await expect(auditListCommand({ ...options, limit: "10" }, runtime)).rejects.toThrow(message); expect(callGateway).not.toHaveBeenCalled(); }); + it.each([ + ["kind", AUDIT_ACTIVITY_KINDS], + ["status", AUDIT_ACTIVITY_STATUSES], + ["direction", AUDIT_ACTIVITY_DIRECTIONS], + ] as const)("forwards every canonical %s value unchanged", async (filter, values) => { + for (const value of values) { + await auditListCommand({ [filter]: value }, runtime); + expect(callGateway).toHaveBeenLastCalledWith({ + method: "audit.activity.list", + params: { limit: 100, [filter]: value }, + }); + } + }); + it("renders activity safely without inventing message provenance", async () => { callGateway.mockResolvedValue({ events: [ @@ -164,6 +196,8 @@ describe("audit command gateway compatibility", () => { beforeEach(() => { callGateway.mockReset(); callGateway.mockResolvedValue({ events: [] }); + vi.mocked(runtime.error).mockClear(); + vi.mocked(runtime.exit).mockClear(); }); it("forwards all filters to audit.activity.list", async () => { @@ -278,16 +312,41 @@ describe("audit command gateway compatibility", () => { expect(callGateway).toHaveBeenCalledTimes(1); }); - it("does not fall back for other request errors", async () => { + it("renders other request errors without the Gateway error class name", async () => { const error = Object.assign(new Error("invalid audit activity params"), { name: "GatewayClientRequestError", gatewayCode: "INVALID_REQUEST", }); callGateway.mockRejectedValueOnce(error); - await expect(auditListCommand({ limit: "10" }, runtime)).rejects.toBe(error); + await runCommandWithRuntime(runtime, () => auditListCommand({ limit: "10" }, runtime)); + + expect(runtime.error).toHaveBeenCalledWith("Error: invalid audit activity params"); + expect(String(vi.mocked(runtime.error).mock.calls[0]?.[0])).not.toContain( + "GatewayClientRequestError", + ); + expect(runtime.exit).toHaveBeenCalledWith(1); expect(callGateway).toHaveBeenCalledTimes(1); }); + + it("turns an opaque cursor rejection into an operator recovery step", async () => { + callGateway.mockRejectedValueOnce( + Object.assign(new Error("invalid audit.activity.list range or cursor"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + }), + ); + + await runCommandWithRuntime(runtime, () => auditListCommand({ cursor: "abc" }, runtime)); + + expect(runtime.error).toHaveBeenCalledWith( + "Error: --cursor must be a continuation token returned by a previous audit result.", + ); + expect(String(vi.mocked(runtime.error).mock.calls[0]?.[0])).not.toContain( + "audit.activity.list", + ); + expect(runtime.exit).toHaveBeenCalledWith(1); + }); }); describe("audit run explanation", () => { diff --git a/src/commands/audit.ts b/src/commands/audit.ts index 5674c048081..fab4abb3e3b 100644 --- a/src/commands/audit.ts +++ b/src/commands/audit.ts @@ -15,11 +15,18 @@ import type { ExecutionIdentityContextV1, PrincipalRefV1, } from "../../packages/gateway-protocol/src/index.js"; +import { + AUDIT_ACTIVITY_DIRECTIONS, + AUDIT_ACTIVITY_KINDS, + AUDIT_ACTIVITY_STATUSES, +} from "../../packages/gateway-protocol/src/schema/audit-activity.js"; import { sanitizeTerminalText } from "../../packages/terminal-core/src/safe-text.js"; import { parsePositiveAuditCursor } from "../audit/audit-cursor.js"; import { parseAbsoluteTimeMs } from "../cron/parse.js"; import { callGateway } from "../gateway/call.js"; +import { formatErrorMessage } from "../infra/errors.js"; import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js"; +import { formatHumanList } from "../shared/human-list.js"; const DEFAULT_AUDIT_LIMIT = 100; const MAX_AUDIT_LIMIT = 500; @@ -161,12 +168,25 @@ function hasMessageSpecificFilters(options: AuditListCommandOptions): boolean { ); } -function validateAuditKind(kind: AuditListCommandOptions["kind"]): void { - if (kind !== undefined && kind !== "agent_run" && kind !== "tool_action" && kind !== "message") { - throw new Error("--kind must be agent_run, tool_action, or message."); +function validateAuditFilter( + value: string | undefined, + flag: string, + allowed: readonly string[], +): void { + if (value !== undefined && !allowed.includes(value)) { + throw new Error(`${flag} must be ${formatHumanList(allowed)}.`); } } +function formatAuditGatewayError(error: unknown): Error { + const message = formatErrorMessage(error); + const operatorMessage = + message === "invalid audit.activity.list range or cursor" + ? "--cursor must be a continuation token returned by a previous audit result." + : message; + return new Error(operatorMessage); +} + function toLegacyAuditListParams(params: AuditActivityListParams): AuditListParams { return { ...(params.agentId ? { agentId: params.agentId } : {}), @@ -192,7 +212,7 @@ async function queryAuditActivity( }); } catch (error) { if (!isUnsupportedActivityMethodError(error)) { - throw error; + throw formatAuditGatewayError(error); } if (hasMessageSpecificFilters(options)) { throw new Error( @@ -237,7 +257,7 @@ async function queryAuditRunInspection( return await callGateway({ method: "audit.run.inspect", params }); } catch (error) { if (!isUnsupportedRunInspectMethodError(error)) { - throw error; + throw formatAuditGatewayError(error); } return unsupportedRunInspection( typeof params.runId === "string" @@ -503,7 +523,9 @@ export async function auditListCommand( if (options.executionId) { throw new Error("--execution requires --explain."); } - validateAuditKind(options.kind); + validateAuditFilter(options.kind, "--kind", AUDIT_ACTIVITY_KINDS); + validateAuditFilter(options.status, "--status", AUDIT_ACTIVITY_STATUSES); + validateAuditFilter(options.direction, "--direction", AUDIT_ACTIVITY_DIRECTIONS); const after = parseAuditTimestamp(options.after, "--after"); const before = parseAuditTimestamp(options.before, "--before"); if (after !== undefined && before !== undefined && after > before) {