From 48c7ee78c41c3b7796dd034399be6a5d06a293b8 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Wed, 7 Oct 2026 12:44:26 +0530 Subject: [PATCH 1/2] feat(admin): export audit trail as CSV and strictly validate enterprise list queries --- app/src/lib/audit/queries.ts | 19 ++ app/src/routes/_authed/admin/audit.tsx | 64 ++++- app/tests/audit-export-url.test.ts | 34 +++ docs/architecture.md | 9 + server/src/admin/routes.ts | 55 ++++- server/src/app.ts | 46 +++- server/src/audit.ts | 104 +++++++++ server/tests/audit-export.test.ts | 221 ++++++++++++++++++ .../enterprise-list-query-validation.test.ts | 65 ++++++ 9 files changed, 602 insertions(+), 15 deletions(-) create mode 100644 app/tests/audit-export-url.test.ts create mode 100644 server/tests/audit-export.test.ts create mode 100644 server/tests/enterprise-list-query-validation.test.ts diff --git a/app/src/lib/audit/queries.ts b/app/src/lib/audit/queries.ts index fe15f9c8a..60ed55bc8 100644 --- a/app/src/lib/audit/queries.ts +++ b/app/src/lib/audit/queries.ts @@ -14,3 +14,22 @@ export function auditEventsQueryOptions(search = "") { }, }); } + +/** + * The CSV download behind the Audit page's Export button. + * + * Carries the page's current filter (`search`, e.g. `?eventType=...`) into + * `GET /api/admin/audit-events?format=csv`, so the file and the table agree. A `format` + * already on the search is replaced rather than doubled, and paging params are dropped: + * the export walks every matching page server-side up to its cap. + */ +export function auditExportUrl(search = ""): string { + const params = new URLSearchParams( + search.startsWith("?") ? search.slice(1) : search, + ); + params.delete("cursor"); + params.delete("limit"); + params.set("format", "csv"); + const query = params.toString(); + return `/api/admin/audit-events${query ? `?${query}` : "?format=csv"}`; +} diff --git a/app/src/routes/_authed/admin/audit.tsx b/app/src/routes/_authed/admin/audit.tsx index e90d90eea..f366b2aec 100644 --- a/app/src/routes/_authed/admin/audit.tsx +++ b/app/src/routes/_authed/admin/audit.tsx @@ -1,4 +1,4 @@ -import { IconRefresh } from "@tabler/icons-react"; +import { IconDownload, IconRefresh } from "@tabler/icons-react"; import { useQuery } from "@tanstack/react-query"; import { createFileRoute } from "@tanstack/react-router"; import { useState } from "react"; @@ -15,8 +15,9 @@ import { outcomeOf, REFUSED_EVENT_TYPES, } from "@/lib/audit/outcome"; -import { auditEventsQueryOptions } from "@/lib/audit/queries"; +import { auditEventsQueryOptions, auditExportUrl } from "@/lib/audit/queries"; import { silenceOf } from "@/lib/audit/silence"; +import { client } from "@/lib/client"; /** * Read surface for policy, computer, component, MCP, and credential audit events. @@ -66,6 +67,41 @@ function AuditPage() { const events = useQuery(auditEventsQueryOptions(search)); const rows = (events.data?.events ?? []) as AuditEvent[]; const nameFor = useBotNames(); + const [exporting, setExporting] = useState(false); + const [exportError, setExportError] = useState(null); + + /* + * The same filtered query as the table, downloaded as a file. The server walks every + * matching page up to its cap, so this stays a single request no matter how many pages + * the filter matches; the filename carries the day it was pulled. + */ + async function exportCsv() { + if (exporting) return; + setExporting(true); + setExportError(null); + try { + const response = await client(auditExportUrl(search), { + fallback: "Could not export audit events", + }); + const blob = await response.blob(); + const url = URL.createObjectURL(blob); + const anchor = document.createElement("a"); + anchor.href = url; + anchor.download = `audit-events-${new Date().toISOString().slice(0, 10)}.csv`; + document.body.appendChild(anchor); + anchor.click(); + anchor.remove(); + URL.revokeObjectURL(url); + } catch (error) { + setExportError( + error instanceof Error + ? error.message + : "Could not export audit events", + ); + } finally { + setExporting(false); + } + } return ( /* @@ -76,10 +112,21 @@ function AuditPage() { */ events.refetch()} size="sm" variant="ghost"> - - Refresh - +
+ + +
} description="Every action a Bot took, and every one this deployment's policy refused." title="Audit" @@ -101,6 +148,11 @@ function AuditPage() { ))} + {exportError ? ( +

+ {exportError} +

+ ) : null} {events.isPending ? null : events.isError ? (

The audit trail could not be loaded. diff --git a/app/tests/audit-export-url.test.ts b/app/tests/audit-export-url.test.ts new file mode 100644 index 000000000..8b894e88b --- /dev/null +++ b/app/tests/audit-export-url.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, test } from "bun:test"; +import { auditExportUrl } from "../src/lib/audit/queries"; + +/** + * The download behind Audit's Export button. + * + * The file must carry the page's current filter, or the download and the table disagree + * about what was reviewed. Paging belongs to the JSON list and is dropped: the server + * walks every matching page up to its cap. + */ + +describe("auditExportUrl", () => { + test("no filter exports the whole trail", () => { + expect(auditExportUrl()).toBe("/api/admin/audit-events?format=csv"); + expect(auditExportUrl("")).toBe("/api/admin/audit-events?format=csv"); + }); + + test("the current filter travels into the download", () => { + expect(auditExportUrl("?eventType=computer.action_allowed")).toBe( + "/api/admin/audit-events?eventType=computer.action_allowed&format=csv", + ); + }); + + test("paging is dropped and a stray format is replaced, not doubled", () => { + expect( + auditExportUrl("?eventType=computer.action_allowed&limit=50&cursor=abc"), + ).toBe( + "/api/admin/audit-events?eventType=computer.action_allowed&format=csv", + ); + expect(auditExportUrl("?format=json")).toBe( + "/api/admin/audit-events?format=csv", + ); + }); +}); diff --git a/docs/architecture.md b/docs/architecture.md index 6eccfad74..0caccd45a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -126,6 +126,15 @@ Computer actions carry the initiator too. A run with no browser attached (a rout a group turn) is given the computer tools server-side, with the Bot and the person bound before the model supplies any arguments, and the gateway writes that run's initiator on every decision. +### Taking the trail out + +The Audit screen's **Export CSV** downloads the current filter as +`GET /api/admin/audit-events?format=csv`: the same filtered query as the table, walked page by +page up to 5,000 rows and returned as RFC 4180 CSV with the redacted payload as one JSON column. +An export that hits the cap answers with `X-Audit-Export-Truncated`, so a reviewer narrows +`from`/`to` or a filter instead of citing a partial trail. An unknown `format` is a 400, like any +other malformed audit query. + ## Human control and secrets Handovers are audited as control events: diff --git a/server/src/admin/routes.ts b/server/src/admin/routes.ts index 4fc3e932c..e1c8b45c7 100644 --- a/server/src/admin/routes.ts +++ b/server/src/admin/routes.ts @@ -19,6 +19,7 @@ import { parseEgressRules, } from "../computer/policy-network"; import { mcpServers } from "../db/schema"; +import { parsePageLimit } from "../paging"; import { otelConfigured } from "../telemetry/otel"; import { CAPABILITIES, @@ -29,6 +30,40 @@ import { import { ACTION_RECORD_RETENTION_DAYS, enterpriseControls } from "./controls"; import type { EnterpriseSettings } from "./settings-store"; +/** + * Strict readers for the two paged enterprise GETs, matching the shared `parsePageLimit` + * rule every other paged list follows: a run of digits is clamped into range, anything + * else is a 400 naming the parameter rather than a silently coerced page. + * + * `GET /models/usage?days=` used `Number(raw) || 30`, so `?days=abc`, `?days=12abc` + * and `?days=` all quietly returned the 30-day view, and `?days=3.9` averaged a + * fractional window. `GET /actions?limit=` did the same with the 100-row default. + * Exported pure so the 400 paths are unit-tested without standing up the controls. + */ +export const ENTERPRISE_DAYS_ERROR = + 'Query parameter "days" must be a positive integer.'; + +export function parseEnterpriseDays( + raw: string | null, +): { ok: true; days: number } | { ok: false; error: string } { + if (raw === null || raw.trim() === "") return { ok: true, days: 30 }; + const trimmed = raw.trim(); + if (!/^\d+$/.test(trimmed)) + return { ok: false, error: ENTERPRISE_DAYS_ERROR }; + return { + ok: true, + days: Math.min(Math.max(Number.parseInt(trimmed, 10), 1), 365), + }; +} + +export function parseEnterpriseActionsLimit( + raw: string | null, +): { ok: true; limit: number } | { ok: false; error: string } { + const parsed = parsePageLimit(raw, 500); + if (!parsed.ok) return parsed; + return { ok: true, limit: parsed.limit ?? 100 }; +} + const capabilityInput = z.strictObject({ scopeKind: z.enum(["organization", "role", "group"]), scopeId: z.string().trim().max(200).default(""), @@ -326,10 +361,11 @@ export function createEnterpriseAdminRoutes( routes.get("/models/usage", async (context) => { const controls = enterpriseControls(); if (!controls) return unavailable(context); - const days = Math.min( - Math.max(Number(context.req.query("days") ?? 30) || 30, 1), - 365, - ); + const parsedDays = parseEnterpriseDays(context.req.query("days") ?? null); + if (!parsedDays.ok) { + return context.json({ error: parsedDays.error }, 400); + } + const days = parsedDays.days; const summary = await controls.store.modelUsageSummary(days); const recent = await controls.store.recentModelUsage(50); const emails = await controls.store.usersById([ @@ -348,8 +384,15 @@ export function createEnterpriseAdminRoutes( routes.get("/actions", async (context) => { const controls = enterpriseControls(); if (!controls) return unavailable(context); - const limit = Number(context.req.query("limit") ?? 100) || 100; - return context.json({ actions: await controls.store.actions(limit) }); + const parsedLimit = parseEnterpriseActionsLimit( + context.req.query("limit") ?? null, + ); + if (!parsedLimit.ok) { + return context.json({ error: parsedLimit.error }, 400); + } + return context.json({ + actions: await controls.store.actions(parsedLimit.limit), + }); }); routes.post("/people/:userId/terminate-computers", async (context) => { diff --git a/server/src/app.ts b/server/src/app.ts index 636b8a3e3..0147a0a30 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -25,11 +25,13 @@ import type { ApprovalService } from "./approvals/service"; import { type AuditEventType, type AuditInitiator, + auditEventsToCsv, AuditQueryError, type AuditReader, type AuditStore, auditQueryFromUrl, DEPLOYMENT_INITIATOR, + readAuditExportRows, recordAuditEvent, } from "./audit"; import { createDevRequireUser } from "./auth/dev-actor"; @@ -778,9 +780,47 @@ export function createApp( } try { - return context.json( - await auditReader.list(auditQueryFromUrl(new URL(context.req.url))), - ); + const url = new URL(context.req.url); + const format = url.searchParams.get("format"); + if ( + format !== null && + format !== "" && + format !== "json" && + format !== "csv" + ) { + return context.json( + { error: 'Query parameter "format" must be "json" or "csv".' }, + 400, + ); + } + /* + * The export answers the same filtered query as the list — `eventType`, + * `actorUserId`, `initiatorKind`, `targetType`, `targetId`, `from`, `to` — so the + * downloaded file and the table agree. Paging (`cursor`, `limit`) belongs to the + * JSON list and is ignored here; the export walks every matching page up to the + * cap in `readAuditExportRows` and names the file after the day it was pulled. + */ + if (format === "csv") { + const query = auditQueryFromUrl(url); + const { events, truncated } = await readAuditExportRows( + auditReader, + query, + ); + const day = new Date().toISOString().slice(0, 10); + context.header("Content-Type", "text/csv; charset=utf-8"); + context.header( + "Content-Disposition", + `attachment; filename="audit-events-${day}.csv"`, + ); + if (truncated) { + context.header( + "X-Audit-Export-Truncated", + "true; narrow from/to or a filter to export the full range", + ); + } + return context.body(auditEventsToCsv(events)); + } + return context.json(await auditReader.list(auditQueryFromUrl(url))); } catch (error) { if (error instanceof AuditQueryError) { return context.json({ error: error.message }, 400); diff --git a/server/src/audit.ts b/server/src/audit.ts index 990453831..12c46b321 100644 --- a/server/src/audit.ts +++ b/server/src/audit.ts @@ -953,3 +953,107 @@ export function auditQueryFromUrl(url: URL): AuditEventQuery { to, }; } + +/** + * CSV export for the admin audit trail. + * + * The table at Admin > Audit is the only read surface, so a review that needs evidence — + * a compliance pull, an incident timeline, a count of what policy refused — has nowhere to + * take it. `GET /api/admin/audit-events?format=csv` answers the same filtered query as the + * JSON list and returns it as RFC 4180 CSV, capped so an unbounded trail cannot become an + * unbounded response. Payloads arrive here already redacted (see `redactAuditPayload`), and + * the export carries them as one JSON column rather than spreading unknown keys across + * columns a spreadsheet would then misread. + */ +export const AUDIT_EXPORT_MAX_ROWS = 5_000; +export const AUDIT_EXPORT_PAGE_SIZE = 500; + +export const AUDIT_EXPORT_COLUMNS = [ + "id", + "createdAt", + "eventType", + "actorUserId", + "initiatorKind", + "initiatorId", + "targetType", + "targetId", + "decisionAllowed", + "decisionRule", + "bot", + "payload", +] as const; + +function toCsvCell(value: string): string { + if ( + value.includes('"') || + value.includes(",") || + value.includes("\n") || + value.includes("\r") + ) { + return `"${value.replaceAll('"', '""')}"`; + } + return value; +} + +function auditEventToCsvRow(event: AuditEvent): string { + const payload = event.payload ?? {}; + const decision = (payload.decision ?? {}) as { + allowed?: unknown; + rule?: unknown; + }; + const cells = [ + event.id, + event.createdAt, + event.eventType, + event.actorUserId ?? "", + event.initiatorKind ?? "", + event.initiatorId ?? "", + event.targetType ?? "", + event.targetId ?? "", + typeof decision.allowed === "boolean" ? String(decision.allowed) : "", + typeof decision.rule === "string" ? decision.rule : "", + typeof payload.bot === "string" ? payload.bot : "", + JSON.stringify(payload), + ]; + return cells.map(toCsvCell).join(","); +} + +export function auditEventsToCsv(events: AuditEvent[]): string { + const header = AUDIT_EXPORT_COLUMNS.join(","); + if (events.length === 0) return `${header}\n`; + return `${header}\n${events.map(auditEventToCsvRow).join("\n")}\n`; +} + +/** + * The rows behind an export, walked page by page through the same reader the JSON list + * uses, so the CSV and the table never disagree about what a filter matches. + * + * `cursor` and `limit` are the JSON list's paging, not the export's, and are ignored here: + * the export takes everything the filters match up to `maxRows` and says whether it + * stopped early, so the caller can narrow the range instead of silently receiving a + * partial trail. + */ +export async function readAuditExportRows( + reader: AuditReader, + baseQuery: AuditEventQuery, + options?: { maxRows?: number; pageSize?: number }, +): Promise<{ events: AuditEvent[]; truncated: boolean }> { + const maxRows = options?.maxRows ?? AUDIT_EXPORT_MAX_ROWS; + const pageSize = options?.pageSize ?? AUDIT_EXPORT_PAGE_SIZE; + const { cursor: _paging, limit: _limit, ...filters } = baseQuery; + const events: AuditEvent[] = []; + let cursor: string | undefined; + let truncated = false; + for (;;) { + const page = await reader.list({ ...filters, limit: pageSize, cursor }); + for (const event of page.events) { + if (events.length >= maxRows) { + truncated = true; + return { events, truncated }; + } + events.push(event); + } + if (!page.nextCursor) return { events, truncated }; + cursor = page.nextCursor; + } +} diff --git a/server/tests/audit-export.test.ts b/server/tests/audit-export.test.ts new file mode 100644 index 000000000..7e514d4e2 --- /dev/null +++ b/server/tests/audit-export.test.ts @@ -0,0 +1,221 @@ +import { describe, expect, test } from "bun:test"; +import { createApp } from "../src/app"; +import { + AUDIT_EXPORT_COLUMNS, + type AuditEvent, + type AuditEventQuery, + type AuditReader, + auditEventsToCsv, + readAuditExportRows, +} from "../src/audit"; +import { loadConfig } from "../src/config"; +import { testEnvironment } from "./support/environment"; + +/** + * CSV export for the admin audit trail. + * + * Admin > Audit was the only read surface, so a review that needed evidence — a compliance + * pull, an incident timeline — had nowhere to take it. `?format=csv` answers the same + * filtered query as the JSON list as RFC 4180 CSV, capped so an unbounded trail cannot + * become an unbounded response. + */ + +const config = loadConfig({ ...testEnvironment() }); + +const adminAuth = { + handler: () => new Response(null, { status: 204 }), + api: { + getSession: async () => ({ + user: { id: "admin", email: "admin@openbot.test" }, + }), + }, +}; + +const event = (overrides: Partial = {}): AuditEvent => ({ + id: "6f1b7f28-6b2d-4d1b-9a2a-1c0b4b2c8a11", + actorUserId: "user-1", + initiatorKind: "person", + initiatorId: null, + eventType: "computer.action_allowed", + targetType: "computer", + targetId: null, + payload: { action: "computer_click", bot: "bot-1" }, + createdAt: "2026-08-13T12:00:00.000Z", + ...overrides, +}); + +const readerOf = (pages: { events: AuditEvent[]; nextCursor?: string }[]) => { + const seen: AuditEventQuery[] = []; + let call = 0; + const reader: AuditReader = { + list: async (query) => { + seen.push(query); + const page = pages[Math.min(call, pages.length - 1)]; + call += 1; + return page; + }, + }; + return { reader, seen }; +}; + +const appWith = (reader: AuditReader) => + createApp(config, adminAuth, { rolesForUser: async () => ["admin"] }, reader); + +describe("auditEventsToCsv", () => { + test("an empty trail is a header, so an empty export still opens as a table", () => { + expect(auditEventsToCsv([])).toBe(`${AUDIT_EXPORT_COLUMNS.join(",")}\n`); + }); + + test("one row carries the filterable columns plus the redacted payload as JSON", () => { + const csv = auditEventsToCsv([ + event({ + payload: { + bot: "bot-1", + decision: { allowed: false, rule: "deny-outside-hours" }, + }, + }), + ]); + const [header, row] = csv.trim().split("\n"); + expect(header).toBe(AUDIT_EXPORT_COLUMNS.join(",")); + expect(row).toContain("computer.action_allowed"); + expect(row).toContain("bot-1"); + expect(row).toContain("false"); + expect(row).toContain("deny-outside-hours"); + }); + + test("commas, quotes and newlines are quoted rather than split into columns", () => { + const csv = auditEventsToCsv([ + event({ + eventType: 'weird,"type"', + targetId: "line1\nline2", + payload: { note: 'say "hi", then go' }, + }), + ]); + const [, row = ""] = csv.trim().split("\n"); + // The row is still one line per field-join: embedded newline is inside quotes. + expect(row).toContain('"weird,""type"""'); + expect(csv).toContain('"line1\nline2"'); + // The payload travels as one JSON column, CSV-quoted: quotes doubled, JSON's own + // backslash-escapes intact. Assert the shape rather than the full escape stack. + expect(csv).toContain('"{""note""'); + expect(csv).toContain("hi"); + }); + + test("a missing decision or bot leaves its column empty instead of guessing", () => { + const [, row = ""] = auditEventsToCsv([event({ payload: {} })]) + .trim() + .split("\n"); + const cells = row.split(","); + // decisionAllowed, decisionRule, bot are columns 9-11 of 12. + expect(cells.slice(8, 11)).toEqual(["", "", ""]); + }); +}); + +describe("readAuditExportRows", () => { + test("walks every matching page and drops the list's own paging", async () => { + const { reader, seen } = readerOf([ + { events: [event()], nextCursor: "page-2" }, + { events: [event({ id: "second" })] }, + ]); + const base: AuditEventQuery = { + limit: 1, + cursor: "stale-bookmark", + eventType: "computer.action_allowed", + }; + const { events, truncated } = await readAuditExportRows(reader, base, { + pageSize: 1, + }); + expect(events.map((row) => row.id)).toEqual([ + "6f1b7f28-6b2d-4d1b-9a2a-1c0b4b2c8a11", + "second", + ]); + expect(truncated).toBe(false); + // Filters travel; the JSON list's cursor/limit do not. + expect(seen[0]?.eventType).toBe("computer.action_allowed"); + expect(seen[0]?.limit).toBe(1); + expect(seen[0]?.cursor).toBeUndefined(); + }); + + test("stops at the cap and says so, rather than silently handing back a partial trail", async () => { + const { reader } = readerOf([ + { events: [event(), event(), event()], nextCursor: "more" }, + { events: [event()], nextCursor: "more" }, + ]); + const { events, truncated } = await readAuditExportRows( + reader, + { limit: 50 }, + { maxRows: 2, pageSize: 3 }, + ); + expect(events).toHaveLength(2); + expect(truncated).toBe(true); + }); +}); + +describe("GET /api/admin/audit-events?format=csv", () => { + test("answers CSV with a download disposition for the same filtered query", async () => { + const { reader, seen } = readerOf([ + { events: [event()], nextCursor: undefined }, + ]); + const response = await appWith(reader).request( + "http://openbot.local/api/admin/audit-events?format=csv&eventType=computer.action_allowed", + ); + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toContain("text/csv"); + expect(response.headers.get("content-disposition")).toContain("attachment"); + expect(response.headers.get("content-disposition")).toContain( + "audit-events-", + ); + expect(await response.text()).toContain("computer.action_allowed"); + expect(seen[0]?.eventType).toBe("computer.action_allowed"); + }); + + test("a truncated export names it in a header instead of looking complete", async () => { + const many = Array.from({ length: 600 }, (_, index) => + event({ + id: `00000000-0000-4000-8000-${String(index).padStart(12, "0")}`, + }), + ); + const { reader } = readerOf([ + { events: many.slice(0, 500), nextCursor: "page-2" }, + { events: many.slice(500), nextCursor: "page-3" }, + { events: [], nextCursor: undefined }, + ]); + // Walk the real collector path at a small cap through the route's pages is + // covered above; here the route streams its default cap without truncation. + const response = await appWith(reader).request( + "http://openbot.local/api/admin/audit-events?format=csv", + ); + expect(response.status).toBe(200); + expect(response.headers.get("x-audit-export-truncated")).toBeNull(); + }); + + test("an unknown format is a 400 naming the parameter", async () => { + const { reader } = readerOf([{ events: [] }]); + const response = await appWith(reader).request( + "http://openbot.local/api/admin/audit-events?format=xlsx", + ); + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ + error: 'Query parameter "format" must be "json" or "csv".', + }); + }); + + test("a bad date filter is still a 400 on the export path, not a downloaded error", async () => { + const { reader } = readerOf([{ events: [] }]); + const response = await appWith(reader).request( + "http://openbot.local/api/admin/audit-events?format=csv&from=not-a-date", + ); + expect(response.status).toBe(400); + expect(response.headers.get("content-type")).toContain("application/json"); + }); + + test("the JSON list is unchanged when no format is asked for", async () => { + const { reader } = readerOf([{ events: [event()] }]); + const response = await appWith(reader).request( + "http://openbot.local/api/admin/audit-events?eventType=computer.action_allowed", + ); + expect(response.status).toBe(200); + const body = (await response.json()) as { events: AuditEvent[] }; + expect(body.events).toHaveLength(1); + }); +}); diff --git a/server/tests/enterprise-list-query-validation.test.ts b/server/tests/enterprise-list-query-validation.test.ts new file mode 100644 index 000000000..c3fe58b9a --- /dev/null +++ b/server/tests/enterprise-list-query-validation.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, test } from "bun:test"; +import { + ENTERPRISE_DAYS_ERROR, + parseEnterpriseActionsLimit, + parseEnterpriseDays, +} from "../src/admin/routes"; +import { PAGE_LIMIT_ERROR } from "../src/paging"; + +/** + * The two paged enterprise GETs, held to the same strict rule as every other list. + * + * `GET /models/usage?days=` used `Number(raw) || 30`, so `?days=abc`, `?days=12abc` + * and `?days=` all quietly returned the 30-day view while `?days=3.9` averaged a + * fractional window. `GET /actions?limit=` did the same with its 100-row default, + * where the audit, channel and people lists already answer 400 through `parsePageLimit`. + */ + +describe("parseEnterpriseDays", () => { + test("absent or blank means the 30-day default", () => { + expect(parseEnterpriseDays(null)).toEqual({ ok: true, days: 30 }); + expect(parseEnterpriseDays("")).toEqual({ ok: true, days: 30 }); + expect(parseEnterpriseDays(" ")).toEqual({ ok: true, days: 30 }); + }); + + test("a plain integer is clamped into 1..365 like the store enforces", () => { + expect(parseEnterpriseDays("7")).toEqual({ ok: true, days: 7 }); + expect(parseEnterpriseDays(" 30 ")).toEqual({ ok: true, days: 30 }); + expect(parseEnterpriseDays("0")).toEqual({ ok: true, days: 1 }); + expect(parseEnterpriseDays("9999")).toEqual({ ok: true, days: 365 }); + }); + + test("anything that is not digits is a 400, not a silent default", () => { + for (const raw of ["abc", "12abc", "3.9", "-5", "0x10", "30days"]) { + expect(parseEnterpriseDays(raw)).toEqual({ + ok: false, + error: ENTERPRISE_DAYS_ERROR, + }); + } + }); +}); + +describe("parseEnterpriseActionsLimit", () => { + test("absent or blank means the 100-row default", () => { + expect(parseEnterpriseActionsLimit(null)).toEqual({ ok: true, limit: 100 }); + expect(parseEnterpriseActionsLimit("")).toEqual({ ok: true, limit: 100 }); + }); + + test("a plain integer is clamped into 1..500 like the store enforces", () => { + expect(parseEnterpriseActionsLimit("25")).toEqual({ ok: true, limit: 25 }); + expect(parseEnterpriseActionsLimit("9999")).toEqual({ + ok: true, + limit: 500, + }); + expect(parseEnterpriseActionsLimit("0")).toEqual({ ok: true, limit: 1 }); + }); + + test("anything that is not digits is the shared limit 400", () => { + for (const raw of ["abc", "12abc", "3.9", "-5", "0x10"]) { + expect(parseEnterpriseActionsLimit(raw)).toEqual({ + ok: false, + error: PAGE_LIMIT_ERROR, + }); + } + }); +}); From 8647103a1447ffc4ac0d80b1797617c5c338a5be Mon Sep 17 00:00:00 2001 From: David McKay Date: Mon, 12 Oct 2026 17:12:22 -0700 Subject: [PATCH 2/2] Neutralise formula cells, record each export, and say when a file stopped at the cap Audit rows carry text people and Bots control, and a CSV cell starting with =, +, - or @ runs as a formula in a spreadsheet, so such a cell is prefixed with a quote and opens as text. Taking the trail out is now on the trail itself, as audit.exported with who, how many rows and which filter. An export that hit the 5,000-row cap set a header the page ignored, so the file read as the whole trail; the header now carries the row count and the page says the file holds the newest that many. --- app/src/routes/_authed/admin/audit.tsx | 11 ++ app/tests/audit-export-truncated.test.tsx | 63 +++++++++++ docs/architecture.md | 9 +- server/src/app.ts | 15 ++- server/src/audit.ts | 27 +++-- server/tests/audit-export.test.ts | 122 +++++++++++++++++++--- 6 files changed, 218 insertions(+), 29 deletions(-) create mode 100644 app/tests/audit-export-truncated.test.tsx diff --git a/app/src/routes/_authed/admin/audit.tsx b/app/src/routes/_authed/admin/audit.tsx index f366b2aec..9d661a1da 100644 --- a/app/src/routes/_authed/admin/audit.tsx +++ b/app/src/routes/_authed/admin/audit.tsx @@ -69,6 +69,7 @@ function AuditPage() { const nameFor = useBotNames(); const [exporting, setExporting] = useState(false); const [exportError, setExportError] = useState(null); + const [exportCapped, setExportCapped] = useState(null); /* * The same filtered query as the table, downloaded as a file. The server walks every @@ -79,10 +80,14 @@ function AuditPage() { if (exporting) return; setExporting(true); setExportError(null); + setExportCapped(null); try { const response = await client(auditExportUrl(search), { fallback: "Could not export audit events", }); + // A file that stopped early must say so, or it reads as the whole trail. + const capped = Number(response.headers.get("x-audit-export-truncated")); + if (Number.isInteger(capped) && capped > 0) setExportCapped(capped); const blob = await response.blob(); const url = URL.createObjectURL(blob); const anchor = document.createElement("a"); @@ -153,6 +158,12 @@ function AuditPage() { {exportError}

) : null} + {exportCapped ? ( +

+ The file holds the newest {exportCapped.toLocaleString()} events, + not all of them. Narrow the filter or the dates to export the rest. +

+ ) : null} {events.isPending ? null : events.isError ? (

The audit trail could not be loaded. diff --git a/app/tests/audit-export-truncated.test.tsx b/app/tests/audit-export-truncated.test.tsx new file mode 100644 index 000000000..7111a1297 --- /dev/null +++ b/app/tests/audit-export-truncated.test.tsx @@ -0,0 +1,63 @@ +import { afterAll, afterEach, beforeAll, expect, test } from "bun:test"; +import { GlobalRegistrator } from "@happy-dom/global-registrator"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + createMemoryHistory, + createRootRoute, + createRouter, + RouterProvider, +} from "@tanstack/react-router"; +import { cleanup, fireEvent, render } from "@testing-library/react"; +import { agentKeys } from "@/lib/agents/queries"; +import { auditKeys } from "@/lib/audit/queries"; +import { Route as AuditRoute } from "@/routes/_authed/admin/audit"; + +beforeAll(() => GlobalRegistrator.register()); + +const realFetch = globalThis.fetch; +afterEach(() => { + cleanup(); + globalThis.fetch = realFetch; +}); + +afterAll(() => GlobalRegistrator.unregister()); + +async function exportWith(headers: Record) { + globalThis.fetch = (async () => + new Response("id\n", { + headers: { "content-type": "text/csv", ...headers }, + })) as unknown as typeof fetch; + URL.createObjectURL = () => "blob:audit"; + URL.revokeObjectURL = () => {}; + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false, staleTime: Number.POSITIVE_INFINITY }, + }, + }); + queryClient.setQueryData([...auditKeys.all, ""], { events: [] }); + queryClient.setQueryData(agentKeys.list(false), []); + const router = createRouter({ + routeTree: createRootRoute({ component: AuditRoute.options.component }), + history: createMemoryHistory({ initialEntries: ["/"] }), + }); + const view = render( + + + , + ); + fireEvent.click(await view.findByText("Export CSV")); + await view.findByText("Export CSV"); + return view; +} + +// A file that stopped at the cap reads as the whole trail unless the page says otherwise. +test("an export that stopped at the cap says how much the file holds", async () => { + const view = await exportWith({ "x-audit-export-truncated": "5000" }); + const notice = await view.findByRole("status"); + expect(notice.textContent).toContain("newest 5,000 events"); +}); + +test("a whole export says nothing extra", async () => { + const view = await exportWith({}); + expect(view.queryByRole("status")).toBeNull(); +}); diff --git a/docs/architecture.md b/docs/architecture.md index 4a3fd7119..e3095a861 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -131,9 +131,12 @@ model supplies any arguments, and the gateway writes that run's initiator on eve The Audit screen's **Export CSV** downloads the current filter as `GET /api/admin/audit-events?format=csv`: the same filtered query as the table, walked page by page up to 5,000 rows and returned as RFC 4180 CSV with the redacted payload as one JSON column. -An export that hits the cap answers with `X-Audit-Export-Truncated`, so a reviewer narrows -`from`/`to` or a filter instead of citing a partial trail. An unknown `format` is a 400, like any -other malformed audit query. +An export that hits the cap holds the newest 5,000 and answers with `X-Audit-Export-Truncated` +(the row count), and the screen says so, so a reviewer narrows `from`/`to` or a filter instead of +citing a partial trail. A cell that starts with `=`, `+`, `-` or `@` is prefixed with `'`, so a +name or URL somebody chose opens in a spreadsheet as text, never as a formula. Every export is +itself recorded as `audit.exported`, with who took it, how many rows, and under which filter. An +unknown `format` is a 400, like any other malformed audit query. ## Human control and secrets diff --git a/server/src/app.ts b/server/src/app.ts index e104fca9f..b214f490e 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -873,17 +873,24 @@ export function createApp( auditReader, query, ); + if (auditStore) { + const { cursor: _cursor, limit: _limit, ...filters } = query; + await recordAuditEvent(auditStore, { + eventType: "audit.exported", + targetType: "audit_trail", + actorUserId: context.var.actor.id, + payload: { format: "csv", rows: events.length, truncated, filters }, + }); + } const day = new Date().toISOString().slice(0, 10); context.header("Content-Type", "text/csv; charset=utf-8"); context.header( "Content-Disposition", `attachment; filename="audit-events-${day}.csv"`, ); + // The row count it stopped at, so the page can say how much the file holds. if (truncated) { - context.header( - "X-Audit-Export-Truncated", - "true; narrow from/to or a filter to export the full range", - ); + context.header("X-Audit-Export-Truncated", String(events.length)); } return context.body(auditEventsToCsv(events)); } diff --git a/server/src/audit.ts b/server/src/audit.ts index debf1cd8c..1e9ae871f 100644 --- a/server/src/audit.ts +++ b/server/src/audit.ts @@ -368,6 +368,11 @@ export const auditEventTypes = [ "person.role_changed", "person.access_revoked", "person.access_restored", + /* + * An administrator took the trail out as a file. The trail is evidence, so who copied how much of + * it, under which filter, belongs on it too. + */ + "audit.exported", /* * Getting in, and being turned away. * @@ -1030,16 +1035,26 @@ export const AUDIT_EXPORT_COLUMNS = [ "payload", ] as const; +/** + * One CSV cell. + * + * A spreadsheet runs a cell that starts with `=`, `+`, `-` or `@` (or a tab or carriage return + * ahead of one) as a formula, and audit rows carry text people and Bots control: names, URLs, + * target IDs. Such a cell is prefixed with `'` so it opens as the text it is (OWASP's CSV injection + * guidance). JSON payloads start with `{` and timestamps and IDs with a digit or letter, so nothing + * the trail itself writes is changed. + */ function toCsvCell(value: string): string { + const text = /^[=+\-@\t\r]/.test(value) ? `'${value}` : value; if ( - value.includes('"') || - value.includes(",") || - value.includes("\n") || - value.includes("\r") + text.includes('"') || + text.includes(",") || + text.includes("\n") || + text.includes("\r") ) { - return `"${value.replaceAll('"', '""')}"`; + return `"${text.replaceAll('"', '""')}"`; } - return value; + return text; } function auditEventToCsvRow(event: AuditEvent): string { diff --git a/server/tests/audit-export.test.ts b/server/tests/audit-export.test.ts index 7e514d4e2..c7e2f6066 100644 --- a/server/tests/audit-export.test.ts +++ b/server/tests/audit-export.test.ts @@ -2,9 +2,12 @@ import { describe, expect, test } from "bun:test"; import { createApp } from "../src/app"; import { AUDIT_EXPORT_COLUMNS, + AUDIT_EXPORT_MAX_ROWS, type AuditEvent, + type AuditEventInput, type AuditEventQuery, type AuditReader, + type AuditStore, auditEventsToCsv, readAuditExportRows, } from "../src/audit"; @@ -58,10 +61,52 @@ const readerOf = (pages: { events: AuditEvent[]; nextCursor?: string }[]) => { return { reader, seen }; }; -const appWith = (reader: AuditReader) => - createApp(config, adminAuth, { rolesForUser: async () => ["admin"] }, reader); +const appWith = (reader: AuditReader, store?: AuditStore) => + createApp( + config, + adminAuth, + { rolesForUser: async () => ["admin"] }, + reader, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + store, + ); + +const storeOf = () => { + const recorded: AuditEventInput[] = []; + const store: AuditStore = { + insert: async (input) => { + recorded.push(input); + }, + }; + return { store, recorded }; +}; describe("auditEventsToCsv", () => { + // Text people and Bots control opens in a spreadsheet as text, never as a formula. + test("a cell a spreadsheet would run as a formula is neutralised", () => { + const csv = auditEventsToCsv([ + event({ + targetId: '=HYPERLINK("https://example.test","x")', + actorUserId: "@admin", + payload: { bot: "+cmd", decision: { allowed: true, rule: "-1+1" } }, + }), + ]); + const row = csv.split("\n")[1] as string; + expect(row).toContain(`"'=HYPERLINK(""https://example.test"",""x"")"`); + expect(row).toContain(",'@admin,"); + expect(row).toContain(",'-1+1,'+cmd,"); + // What the trail writes itself is left alone: the ID, the time and the JSON payload. + expect(row.startsWith("6f1b7f28-")).toBe(true); + expect(row).toContain(",2026-08-13T12:00:00.000Z,"); + }); + test("an empty trail is a header, so an empty export still opens as a table", () => { expect(auditEventsToCsv([])).toBe(`${AUDIT_EXPORT_COLUMNS.join(",")}\n`); }); @@ -170,23 +215,68 @@ describe("GET /api/admin/audit-events?format=csv", () => { }); test("a truncated export names it in a header instead of looking complete", async () => { - const many = Array.from({ length: 600 }, (_, index) => - event({ - id: `00000000-0000-4000-8000-${String(index).padStart(12, "0")}`, - }), + const page = (start: number, next?: string) => ({ + events: Array.from({ length: 500 }, (_, index) => + event({ + id: `00000000-0000-4000-8000-${String(start + index).padStart(12, "0")}`, + }), + ), + nextCursor: next, + }); + // Eleven full pages: one more than the cap holds. + const { reader } = readerOf( + Array.from({ length: 11 }, (_, index) => + page(index * 500, index < 10 ? `page-${index + 2}` : undefined), + ), ); - const { reader } = readerOf([ - { events: many.slice(0, 500), nextCursor: "page-2" }, - { events: many.slice(500), nextCursor: "page-3" }, - { events: [], nextCursor: undefined }, - ]); - // Walk the real collector path at a small cap through the route's pages is - // covered above; here the route streams its default cap without truncation. - const response = await appWith(reader).request( + const capped = await appWith(reader).request( "http://openbot.local/api/admin/audit-events?format=csv", ); - expect(response.status).toBe(200); - expect(response.headers.get("x-audit-export-truncated")).toBeNull(); + expect(capped.status).toBe(200); + expect(capped.headers.get("x-audit-export-truncated")).toBe( + String(AUDIT_EXPORT_MAX_ROWS), + ); + expect((await capped.text()).trim().split("\n")).toHaveLength( + AUDIT_EXPORT_MAX_ROWS + 1, + ); + + const { reader: small } = readerOf([{ events: [event()] }]); + const whole = await appWith(small).request( + "http://openbot.local/api/admin/audit-events?format=csv", + ); + expect(whole.headers.get("x-audit-export-truncated")).toBeNull(); + }); + + test("taking the trail out is itself on the trail, with who, how much and which filter", async () => { + const { reader } = readerOf([{ events: [event(), event()] }]); + const { store, recorded } = storeOf(); + await appWith(reader, store).request( + "http://openbot.local/api/admin/audit-events?format=csv&eventType=computer.action_allowed&limit=5", + ); + expect(recorded).toHaveLength(1); + expect(recorded[0]).toMatchObject({ + eventType: "audit.exported", + targetType: "audit_trail", + actorUserId: "admin", + payload: { + format: "csv", + rows: 2, + truncated: false, + filters: { eventType: "computer.action_allowed" }, + }, + }); + // Paging belongs to the table, not the export, so it is not part of what was taken. + const [exported] = recorded; + expect( + (exported?.payload as { filters?: Record } | undefined) + ?.filters, + ).not.toHaveProperty("limit"); + + // Reading the table is not an export. + await appWith(reader, store).request( + "http://openbot.local/api/admin/audit-events", + ); + expect(recorded).toHaveLength(1); }); test("an unknown format is a 400 naming the parameter", async () => {