From ee6fde220534af2942aebc7e555b4d6c5c334c6e Mon Sep 17 00:00:00 2001 From: cjimti Date: Wed, 6 May 2026 10:06:10 -0700 Subject: [PATCH 1/3] feat(audit): portal inspection drawer, comparison page, walkthrough docs Closes the remaining subtasks of issue #8. Drawer: 4-tab side panel (Overview / Request / Response / Notifications) opened by clicking any audit row; deep-linkable via ?id=, Esc/backdrop close, role=dialog/aria-modal for assistive tech, Replay button posts to the existing replay endpoint with banner output, Compare button stashes the event id in localStorage so two rows can be opened side-by-side. Notifications tab count appends '+' when the captured list was truncated. Compare page: /portal/audit/compare?a=&b= renders a JSON-path-aware structural diff. Walks objects/arrays by key/index so reordered keys don't show as changes; type-vs-tree mismatches collapse to a single diff at the path. One-side-undefined trees still show per-key only-A or only-B leaves rather than a single root-level "(undefined) -> {...}" line. Indentation comes from each nested ul's own padding so deep trees don't double-indent off-panel. Audit page: live-tail toggle subscribes to the SSE stream into a 20-event buffer above the table (the table stays a historical filter view to avoid refetch storms under load). JSONB filter editor produces the parseQueryFilter syntax (?param.=v / ?response.=v / ?header.=v / ?has=); compare-stash bar exposes the stashed event id with a one-click "compare with selected". API client: streamAuditEvents uses fetch + ReadableStream rather than EventSource so the X-API-Key header is carried; SSE framer follows spec on leading-space stripping, surfaces 401 to the unauthorized handler without flashing a per-stream error banner, caps the line buffer at 1 MiB so a misbehaving producer can't OOM the tab, drops events without an id at parse time, and reports server-close so the operator can re-enable. Docs: docs/operations/inspection.md walks the full workflow end-to-end, cross-referenced against the actual replayBurst / replayRefill / maxExportEvents constants. Per-identity rate-limit scope and the current export-truncation behavior are both called out explicitly. Pre-commit adversarial review: 4 review rounds across two phases (initial 3 rounds during development plus the gate's own 2 rounds at commit-time). Findings F1-F19 plus regression N1 all resolved before the first commit. --- docs/operations/inspection.md | 123 ++++++++++ ui/src/components/EventDrawer.tsx | 345 ++++++++++++++++++++++++++++ ui/src/components/JsonView.tsx | 46 ++++ ui/src/lib/api.ts | 126 ++++++++++ ui/src/main.tsx | 2 + ui/src/pages/Audit.tsx | 324 ++++++++++++++++++++++++-- ui/src/pages/Compare.tsx | 369 ++++++++++++++++++++++++++++++ 7 files changed, 1316 insertions(+), 19 deletions(-) create mode 100644 docs/operations/inspection.md create mode 100644 ui/src/components/EventDrawer.tsx create mode 100644 ui/src/components/JsonView.tsx create mode 100644 ui/src/pages/Compare.tsx diff --git a/docs/operations/inspection.md b/docs/operations/inspection.md new file mode 100644 index 0000000..8ee1de8 --- /dev/null +++ b/docs/operations/inspection.md @@ -0,0 +1,123 @@ +--- +title: Inspection workflow +description: End-to-end walkthrough of the audit inspection utility — capture a call, open the drawer, replay it, compare to a baseline, filter via JSONB paths, and export. +--- + +# Inspection workflow + +The audit pipeline records every tool call. The inspection utility is the operator-facing toolset for working with those records: a click-to-expand drawer, a per-event replay, side-by-side comparison, server-side JSONB-path filters, and an NDJSON export. This page is the workflow that ties them together. + +## What you need + +- A running mcp-test instance (`make dev` or a deployment). +- An API key or portal session for the user account that's allowed to read the audit log. +- (Optional, for replay) The MCP server registered in this deployment must still know about the tool you're replaying. Replays of removed tools are refused with `400`. + +## 1. Capture a call + +The pipeline captures every `tools/call` automatically when `audit.enabled: true` (default). Two tables are written in one transaction: + +- `audit_events` — indexed summary (timestamp, tool, user, success, duration). Used for browsing and filtering. +- `audit_payloads` — full request / response envelope (parameters, headers, response result, response error, notifications, replay linkage). Optional; `capture_payloads: false` keeps the summary only. + +To produce a fresh row to inspect, fire any tool. The portal's Try-It page (`/portal/tools/`) is the easiest way; any MCP client works too. + +## 2. Open the drawer + +In the portal, navigate to `Audit`. Each row in the events table is clickable; the click opens a side drawer with four tabs: + +### Overview tab +Timing, identity, request id, session id, source (`mcp` for real client calls, `portal-tryit` for /admin/tryit invocations, `portal-replay` for replays), and the replay linkage (`Replayed from`) when present. + +### Request tab +The captured `request_params` (sanitized via `audit.redact_keys`, with redacted values shown as `"[redacted]"`). Captured request headers when `audit.capture_headers: true`. A truncation warning when the request body exceeded `audit.max_payload_bytes`. + +### Response tab +The full `CallToolResult` content blocks (text, image, audio, structured) plus `response_error` when the call errored. The shape matches what the SDK serializes to the wire so you can see what the client saw. A truncation warning fires when the response body was too large. + +### Notifications tab +Chronological list of every `notifications/*` (progress, log message) the tool dispatched during the call window. Each entry is `{ts, method, params}` with `params` rendered as JSON. A trim warning fires when the notification list exceeded `max_payload_bytes` (the trailing entries are missing; the prefix is what's stored). + +Drawer interactions: + +- The browser URL gets `?id=` appended so the drawer is deep-linkable; share the URL and the recipient lands on the same row. +- The **Compare** button stashes the open event id in `localStorage`. Open another row's drawer and you'll see "Compare with selected" — clicking opens the comparison page with both events. +- The **Replay** button is the next step. +- `Esc` and the backdrop close the drawer. + +## 3. Replay a captured call + +The drawer's **Replay** button calls `POST /api/v1/portal/audit/events/{id}/replay`. The server re-invokes the tool through an in-process MCP client with the same `request_params` the original call had. A new audit row lands tagged `source=portal-replay` with `replayed_from` pointing at the original event; the new event is fired with **your** identity, not the original caller's, so the audit row reflects who triggered the replay. + +The replay banner inside the drawer shows the new event id; clicking it deep-links to that row. Refused replays show a banner explaining why (most common: redacted parameter values, no captured payload, or a tool that's no longer registered). + +**Replay re-runs side effects.** If the original call wrote to a database, sent a notification, or charged a card, the replay does it again. There is no dry-run mode and no per-tool allow list. Treat replay like Try-It: a developer affordance for debugging, not a production self-service. + +Per-identity rate limit (scoped by API key id or OIDC subject): 5 burst, one token refilled every 12 seconds (sustained 5/min). `429` with `Retry-After` when exhausted. + +## 4. Compare to a baseline + +Two events you stashed via the drawer's Compare button can be opened side-by-side at `/portal/audit/compare?a=&b=`. The page renders: + +- A summary block (tool, source, result, duration, user, auth type) with diffs highlighted. +- Per-payload diff trees for `request_params`, `response_result`, `response_error`, plus a count comparison for notifications. +- Each leaf in the tree is annotated: same (muted), differ (warning color, `before → after`), only-in-A (red `-`), only-in-B (green `+`). + +The diff is JSON-path-aware: it walks objects and arrays by key/index instead of doing a text diff, so reordered keys (a Postgres read returning fields in any order) don't show as changes, and a string-vs-object swap appears as one diff at the path it happened — not as a wall of red lines. + +Common compare workflows: + +- A successful call and a failed call with the "same" arguments. The summary highlights `Result`; the response trees show what differed in the tool's output. +- Two captures of the same tool name spanning a deploy. Use the comparison to sanity-check that a refactor didn't change the response shape. +- A replay against its original. The drawer has a quick path: open the replay row, the drawer's Overview tab shows `Replayed from: `; navigate to that row, stash, then back to the replay row, stash, then Compare. + +## 5. Filter via JSONB paths + +The Audit page has a **JSONB filters** toggle that opens an editor for the path-aware filters the server compiles to JSONB containment queries. Operators routinely live with these set: + +- `param.user.id=alice` — every call where the request param at the dotted path `user.id` equals `alice`. +- `response.isError=true` — every call whose response had `IsError=true` (matches the JSON literal `true`, not the string `"true"`; values are type-detected). +- `header.User-Agent=curl/8.0` — every call from a specific User-Agent. Header names are canonicalized (`user-agent` matches `User-Agent`). +- `has=response_error` — every call that recorded a transport-level error. +- `has=notifications` — every call that fired any notification. + +Filters are AND-combined with each other and with the indexed-column filters (tool, user, success, etc.). They run against `audit_payloads` via `EXISTS` subqueries that hit the existing GIN indexes on `request_params` and `response_result`; `request_headers` is unindexed today so pair `header.*` with a time-range filter on busy deployments. + +**Quoting forces strings.** `?param.code=200` matches the JSON number `200`; `?param.code="200"` matches the JSON string `"200"`. Header values are always strings; type-detection does not apply there. + +## 6. Live tail + +The **Live tail** toggle on the Audit page opens an SSE connection to `/api/v1/portal/audit/stream`. New audit events appear in a small ring buffer above the table as they're written; clicking one opens the drawer. The table itself stays a historical-filter view so the live tail doesn't blow away your filtered context. + +The stream sends an opening `: connected` comment on connect, an `event: audit\ndata: ` per write, and a `: keepalive` comment every 30 seconds. Slow consumers see per-subscriber drops; the producer never blocks. + +## 7. Export + +`GET /api/v1/portal/audit/export?format=jsonl` streams the filtered set as newline-delimited summary rows for offline analysis, ad-hoc ETL, or backups. + +```bash +# Every error from the last 24h, piped through jq. +curl -H "X-API-Key: $KEY" \ + "$BASE/api/v1/portal/audit/export?success=false&from=$(date -u -v-24H +%FT%TZ)" \ + | jq -r '.tool_name + "\t" + .error_message' +``` + +The same JSONB filters work; combine `?success=false&has=notifications&from=...` to scope a backfill. + +The export omits the captured payload from each line; if you need the full envelope, follow up with `/audit/events/{id}` per event. The endpoint is currently capped at 100,000 rows per request and truncates at the cap with no in-band marker; verify the row count against your filter window and tighten if you hit the ceiling. (Future versions may emit a sentinel line or trailer; do not rely on the current silent-truncation behavior.) + +## End-to-end example + +The shortest path from "a call broke" to a written-up bug report: + +1. **Find the failure.** Audit page, set Status=`error`, glance at the table. +2. **Understand it.** Click the row. Overview shows the tool + duration; Response shows the `response_error.category` + message; Notifications shows what the tool got partway through before failing. +3. **Reproduce it.** Click Replay. New row in the table tagged `portal-replay`. Open it; if it failed the same way, you have a deterministic repro. +4. **Compare.** Stash the current failed event via the drawer's Compare button. Open a healthy past call of the same tool, stash that. Compare opens both side-by-side; the Response tree highlights what changed. +5. **Hand it off.** Copy the event id (from the URL `?id=` or the drawer's id field) into the bug report. The recipient navigates `/portal/audit?id=` and lands on the same drawer. + +## Reference + +- HTTP endpoints: `docs/reference/http-api.md` +- Audit schema and retention: `docs/operations/audit.md` +- v1.1.0 baseline + v1.1.1 schema follow-up: see the audit.md "Two-table layout" section. diff --git a/ui/src/components/EventDrawer.tsx b/ui/src/components/EventDrawer.tsx new file mode 100644 index 0000000..5b8807b --- /dev/null +++ b/ui/src/components/EventDrawer.tsx @@ -0,0 +1,345 @@ +import { useEffect, useState } from "react"; +import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; +import { Link } from "react-router-dom"; +import { X, Play, GitCompare, AlertCircle, CheckCircle2 } from "lucide-react"; +import { portalAPI, type AuditEvent, type ReplayResponse, HttpError } from "@/lib/api"; +import { JsonView } from "./JsonView"; + +type Tab = "overview" | "request" | "response" | "notifications"; + +// EventDrawer is the audit-row click-to-expand panel. Slides in from the +// right; closes via the X button, the backdrop, or the Escape key. Four +// tabs: Overview / Request / Response / Notifications. The replay +// button calls POST /audit/events/{id}/replay; the Compare-to picker +// stashes this event id in localStorage so the Audit page can wire a +// "compare to last selected" link. +export function EventDrawer({ + eventId, + onClose, + onCompareSelect, +}: { + eventId: string | null; + onClose: () => void; + onCompareSelect: (id: string) => void; +}) { + const [tab, setTab] = useState("overview"); + const qc = useQueryClient(); + + // ESC closes. + useEffect(() => { + if (!eventId) return; + const onKey = (e: KeyboardEvent) => { + if (e.key === "Escape") onClose(); + }; + window.addEventListener("keydown", onKey); + return () => window.removeEventListener("keydown", onKey); + }, [eventId, onClose]); + + // Reset to overview when the selected event changes. + useEffect(() => { + if (eventId) setTab("overview"); + }, [eventId]); + + const detail = useQuery({ + queryKey: ["audit-event", eventId], + queryFn: () => portalAPI.auditEvent(eventId!), + enabled: !!eventId, + }); + + const replay = useMutation({ + mutationFn: () => portalAPI.auditReplay(eventId!), + onSuccess: () => { + // Refresh the events list so the new replay row appears. + void qc.invalidateQueries({ queryKey: ["audit"] }); + }, + }); + + // Reset replay state on event change so a banner from a prior replay + // doesn't bleed into a different drawer. + useEffect(() => { + replay.reset(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [eventId]); + + if (!eventId) return null; + + const ev = detail.data; + + return ( + <> + {/* Backdrop */} +
+ {/* Panel */} +
+
+
+
+ {ev?.success === false ? ( + + ) : ( + + )} +

+ {ev?.tool_name ?? "Loading..."} +

+ {ev?.source && ( + + {ev.source} + + )} +
+
+ {eventId} +
+
+
+ + + +
+
+ + + + + +
+ {detail.isLoading &&
Loading...
} + {detail.isError && ( +
+ {detail.error.message} +
+ )} + {ev && tab === "overview" && } + {ev && tab === "request" && } + {ev && tab === "response" && } + {ev && tab === "notifications" && } +
+
+ + ); +} + +function ReplayBanner({ + replay, +}: { + replay: ReturnType>; +}) { + if (replay.isError) { + return ( +
+ +
+
Replay failed
+
{replay.error.message}
+
+
+ ); + } + if (replay.data) { + return ( +
+ {replay.data.success ? ( + + ) : ( + + )} +
+
+ {replay.data.success ? "Replay succeeded" : "Replay returned tool error"} +
+
+ new event:{" "} + + {replay.data.replay_event_id} + +
+
+
+ ); + } + return null; +} + +function OverviewTab({ ev }: { ev: AuditEvent }) { + return ( +
+ + + + {ev.tool_group && } + + {ev.error_category ?? "error"} + {ev.error_message ? ` — ${ev.error_message}` : ""} + + ) + } /> + + + {ev.request_id && {ev.request_id}} />} + {ev.session_id && {ev.session_id}} />} + {ev.user_subject && {ev.user_subject}} />} + {ev.user_email && } + {ev.auth_type && } + {ev.api_key_name && } + {ev.remote_addr && {ev.remote_addr}} />} + {ev.user_agent && {ev.user_agent}} />} + {(ev.request_chars ?? 0) > 0 && } + {(ev.response_chars ?? 0) > 0 && } + {(ev.content_blocks ?? 0) > 0 && } + {ev.payload?.replayed_from && ( + + {ev.payload.replayed_from} + + } /> + )} +
+ ); +} + +function RequestTab({ ev }: { ev: AuditEvent }) { + const params = ev.payload?.request_params ?? ev.parameters; + const headers = ev.payload?.request_headers; + const noPayload = !ev.payload; + return ( +
+ {ev.payload?.request_truncated && ( +
+ Request payload was truncated at storage time (exceeded max_payload_bytes). +
+ )} + {noPayload && !params && ( +
+ No request captured. Either capture_payloads is off for this deployment, or this row + predates payload capture (events written before v1.1.0). +
+ )} + {noPayload && params && ( +
+ Showing summary parameters only; full payload was not captured for this row. +
+ )} + {params && } + {headers && Object.keys(headers).length > 0 && ( + + )} +
+ ); +} + +function ResponseTab({ ev }: { ev: AuditEvent }) { + const result = ev.payload?.response_result; + const err = ev.payload?.response_error; + return ( +
+ {ev.payload?.response_truncated && ( +
+ Response payload was truncated at storage time. +
+ )} + {err && } + {result && } + {!err && !result && ( +
+ No response captured. Either capture_payloads is off for this deployment, or this row + predates payload capture (events written before v1.1.0). +
+ )} +
+ ); +} + +function NotificationsTab({ ev }: { ev: AuditEvent }) { + const ns = ev.payload?.notifications ?? []; + if (ns.length === 0) { + return ( +
+ No notifications were dispatched during this call. +
+ ); + } + return ( +
+ {ev.payload?.notifications_truncated && ( +
+ Notification list was trimmed to fit max_payload_bytes; trailing entries are missing. +
+ )} +
    + {ns.map((n, i) => ( +
  1. +
    + {n.method} + {new Date(n.ts).toLocaleTimeString()} +
    + +
  2. + ))} +
+
+ ); +} + +function Row({ k, v }: { k: string; v: React.ReactNode }) { + return ( + <> +
{k}
+
{v}
+ + ); +} diff --git a/ui/src/components/JsonView.tsx b/ui/src/components/JsonView.tsx new file mode 100644 index 0000000..07554d4 --- /dev/null +++ b/ui/src/components/JsonView.tsx @@ -0,0 +1,46 @@ +import { useState } from "react"; +import { Copy, Check } from "lucide-react"; + +// JsonView renders a value as pretty-printed JSON with a copy-to-clipboard +// button. We use plain JSON.stringify (not a tree viewer) for now: it's +// honest about what's stored, fast, and keyboard-selectable. A +// react-json-tree integration can replace the inner element later +// without changing the API. +export function JsonView({ value, label }: { value: unknown; label?: string }) { + const [copied, setCopied] = useState(false); + const json = value === undefined || value === null ? "" : JSON.stringify(value, null, 2); + + async function copy() { + try { + await navigator.clipboard.writeText(json); + setCopied(true); + setTimeout(() => setCopied(false), 1500); + } catch { + // clipboard API may be unavailable on http:// origins; ignore. + } + } + + if (!json) { + return ( +
+ {label ? `${label}: ` : ""}(empty) +
+ ); + } + + return ( +
+ {label &&
{label}
} +
+        {json}
+      
+ +
+ ); +} diff --git a/ui/src/lib/api.ts b/ui/src/lib/api.ts index 43cb734..e802645 100644 --- a/ui/src/lib/api.ts +++ b/ui/src/lib/api.ts @@ -129,6 +129,39 @@ export type AuditEvent = { content_blocks?: number; transport: string; source: string; + remote_addr?: string; + user_agent?: string; + payload?: AuditPayload; +}; + +export type AuditNotification = { + ts: string; + method: string; + params?: Record; +}; + +export type AuditPayload = { + jsonrpc_method?: string; + request_params?: Record; + request_size_bytes?: number; + request_truncated?: boolean; + request_headers?: Record; + request_remote_addr?: string; + response_result?: Record; + response_error?: Record; + response_size_bytes?: number; + response_truncated?: boolean; + notifications?: AuditNotification[]; + notifications_truncated?: boolean; + replayed_from?: string; +}; + +export type ReplayResponse = { + replay_event_id: string; + replayed_from: string; + result: unknown; + success: boolean; + error?: string; }; export type DashboardResponse = { @@ -164,10 +197,103 @@ export const portalAPI = { tools: () => api.get<{ tools: ToolMeta[] }>("/api/v1/portal/tools"), toolDetail: (name: string) => api.get(`/api/v1/portal/tools/${encodeURIComponent(name)}`), audit: (qs: string) => api.get<{ events: AuditEvent[]; total: number; limit: number; offset: number }>(`/api/v1/portal/audit/events${qs ? "?" + qs : ""}`), + auditEvent: (id: string) => api.get(`/api/v1/portal/audit/events/${encodeURIComponent(id)}`), + auditReplay: (id: string) => api.post(`/api/v1/portal/audit/events/${encodeURIComponent(id)}/replay`, {}), dashboard: () => api.get("/api/v1/portal/dashboard"), wellknown: () => api.get<{ protected_resource_url: string; authorization_server: string; oidc_enabled: boolean; audience: string; mcp_endpoint: string }>("/api/v1/portal/wellknown"), }; +// streamAuditEvents opens a fetch-based SSE connection to the live tail +// endpoint and invokes onEvent per `event: audit` frame. Returns an +// unsubscribe function that aborts the request. +// +// Uses fetch + ReadableStream rather than EventSource because EventSource +// can't send custom headers (X-API-Key); cookie-only auth would lock out +// CLI / API-key callers. SSE comments (": connected", ": keepalive") are +// silently skipped. +export function streamAuditEvents( + onEvent: (ev: AuditEvent) => void, + onError?: (err: Error) => void, +): () => void { + const ctrl = new AbortController(); + void (async () => { + try { + const headers = new Headers(); + const key = getApiKey(); + if (key) headers.set("X-API-Key", key); + headers.set("Accept", "text/event-stream"); + const resp = await fetch("/api/v1/portal/audit/stream", { + credentials: "include", + headers, + signal: ctrl.signal, + }); + if (resp.status === 401) { + // Auth handler will redirect to /login; suppress the per-stream + // error callback so a flash banner doesn't precede the redirect. + clearApiKey(); + onUnauthorized?.(); + return; + } + if (!resp.ok || !resp.body) { + throw new HttpError(resp.status, `stream open failed: HTTP ${resp.status}${resp.statusText ? " " + resp.statusText : ""}`); + } + const reader = resp.body.getReader(); + const decoder = new TextDecoder(); + // Cap the line buffer so a misbehaving producer that never emits + // a newline can't grow it without bound and OOM the tab. + const maxBuf = 1 << 20; // 1 MiB + let buf = ""; + let event = ""; + let data = ""; + while (true) { + const { done, value } = await reader.read(); + if (done) { + // The server closed the stream (heartbeat write failure, idle + // timeout, or shutdown). Surface so the UI can re-enable the tail. + throw new Error("stream closed by server"); + } + buf += decoder.decode(value, { stream: true }); + if (buf.length > maxBuf) { + throw new Error("stream buffer overflow (no newline within 1 MiB)"); + } + let nl: number; + while ((nl = buf.indexOf("\n")) >= 0) { + const raw = buf.slice(0, nl); + buf = buf.slice(nl + 1); + const line = raw.endsWith("\r") ? raw.slice(0, -1) : raw; + if (line === "") { + if (event === "audit" && data) { + try { + const parsed = JSON.parse(data) as AuditEvent; + if (parsed && typeof parsed.id === "string" && parsed.id) { + onEvent(parsed); + } + } catch { + // Malformed event payload — skip and keep reading. + } + } + event = ""; + data = ""; + continue; + } + if (line.startsWith(":")) continue; // SSE comment + if (line.startsWith("event:")) { + event = line.slice(6).replace(/^ /, ""); + } else if (line.startsWith("data:")) { + // SSE: strip exactly one optional leading space; preserve any other whitespace. + const fragment = line.slice(5).replace(/^ /, ""); + data = data ? data + "\n" + fragment : fragment; + } + } + } + } catch (err) { + if ((err as Error).name === "AbortError") return; + onError?.(err as Error); + } + })(); + return () => ctrl.abort(); +} + export const adminAPI = { listKeys: () => api.get<{ keys: Key[] }>("/api/v1/admin/keys"), createKey: (name: string, description?: string) => api.post<{ key: Key; plaintext: string }>("/api/v1/admin/keys", { name, description }), diff --git a/ui/src/main.tsx b/ui/src/main.tsx index a2ead68..4d7daab 100644 --- a/ui/src/main.tsx +++ b/ui/src/main.tsx @@ -9,6 +9,7 @@ import Login from "./pages/Login"; import Dashboard from "./pages/Dashboard"; import Tools from "./pages/Tools"; import Audit from "./pages/Audit"; +import Compare from "./pages/Compare"; import ApiKeys from "./pages/ApiKeys"; import Config from "./pages/Config"; import Wellknown from "./pages/Wellknown"; @@ -35,6 +36,7 @@ ReactDOM.createRoot(document.getElementById("root")!).render( } /> } /> } /> + } /> } /> } /> } /> diff --git a/ui/src/pages/Audit.tsx b/ui/src/pages/Audit.tsx index e45430a..c137306 100644 --- a/ui/src/pages/Audit.tsx +++ b/ui/src/pages/Audit.tsx @@ -1,6 +1,9 @@ import { useQuery } from "@tanstack/react-query"; -import { useEffect, useState } from "react"; -import { portalAPI } from "@/lib/api"; +import { useEffect, useMemo, useRef, useState } from "react"; +import { useNavigate, useSearchParams } from "react-router-dom"; +import { Radio, GitCompare, Filter, X } from "lucide-react"; +import { portalAPI, streamAuditEvents, type AuditEvent } from "@/lib/api"; +import { EventDrawer } from "@/components/EventDrawer"; // useDebounced returns `value` after `ms` of stillness; used to avoid // firing an audit query on every keystroke. @@ -13,50 +16,225 @@ function useDebounced(value: T, ms = 300): T { return v; } +// JSONB filter shape. Matches the parseQueryFilter syntax in +// pkg/httpsrv/portal_api.go (param.=v / response.=v / +// header.=v / has=). +type JsonFilter = { + source: "param" | "response" | "header" | "has"; + // For param/response/header: the dotted path (header is single-segment). + // For has: the column name. + path: string; + // For param/response/header only. + value: string; +}; + +const HAS_KEYS = [ + "request_params", + "request_headers", + "response_result", + "response_error", + "notifications", + "replayed_from", +]; + +const COMPARE_KEY = "audit-compare-stash"; + export default function Audit() { + const [params, setParams] = useSearchParams(); + const navigate = useNavigate(); + const [tool, setTool] = useState(""); const [user, setUser] = useState(""); const [success, setSuccess] = useState<"" | "true" | "false">(""); const [search, setSearch] = useState(""); const [page, setPage] = useState(0); + const [showFilters, setShowFilters] = useState(false); + const [jsonFilters, setJsonFilters] = useState([]); + const [liveTail, setLiveTail] = useState(false); + const [tailEvents, setTailEvents] = useState([]); + const [tailError, setTailError] = useState(null); const limit = 50; const debouncedSearch = useDebounced(search, 300); const debouncedTool = useDebounced(tool, 300); const debouncedUser = useDebounced(user, 300); - const qs = new URLSearchParams(); - if (debouncedTool) qs.set("tool", debouncedTool); - if (debouncedUser) qs.set("user", debouncedUser); - if (success) qs.set("success", success); - if (debouncedSearch) qs.set("q", debouncedSearch); - qs.set("limit", String(limit)); - qs.set("offset", String(page * limit)); + // Drawer selection comes from URL ?id= so deep-linking works. + const selectedId = params.get("id"); + + // Compare-to: the most recently stashed event id from the drawer's + // Compare button. localStorage persists across reloads. + const [compareId, setCompareId] = useState(() => localStorage.getItem(COMPARE_KEY)); + + const qs = useMemo(() => { + const u = new URLSearchParams(); + if (debouncedTool) u.set("tool", debouncedTool); + if (debouncedUser) u.set("user", debouncedUser); + if (success) u.set("success", success); + if (debouncedSearch) u.set("q", debouncedSearch); + for (const f of jsonFilters) { + if (f.source === "has") { + if (f.path) u.append("has", f.path); + } else if (f.path && f.value) { + u.append(`${f.source}.${f.path}`, f.value); + } + } + u.set("limit", String(limit)); + u.set("offset", String(page * limit)); + return u; + }, [debouncedTool, debouncedUser, success, debouncedSearch, jsonFilters, page]); const q = useQuery({ queryKey: ["audit", qs.toString()], queryFn: () => portalAPI.audit(qs.toString()), placeholderData: (p) => p, + refetchInterval: liveTail ? false : undefined, }); + // Live tail: open the SSE stream when the toggle is on; close on toggle + // off or unmount. Incoming events go into the cap-20 buffer shown above + // the table; the table itself stays a historical-filter view to avoid a + // refetch-per-event storm under load (use the buffer for the live read, + // page the table for context). + useEffect(() => { + if (!liveTail) { + setTailEvents([]); + setTailError(null); + return; + } + const stop = streamAuditEvents( + (ev) => { + setTailEvents((prev) => [ev, ...prev].slice(0, 20)); + }, + (err) => setTailError(err.message), + ); + return stop; + }, [liveTail]); + const totalPages = q.data ? Math.ceil(q.data.total / limit) : 1; + function selectEvent(id: string | null) { + const next = new URLSearchParams(params); + if (id) next.set("id", id); + else next.delete("id"); + setParams(next, { replace: true }); + } + + function stashCompare(id: string) { + localStorage.setItem(COMPARE_KEY, id); + setCompareId(id); + } + + function openCompare() { + if (!compareId || !selectedId || compareId === selectedId) return; + navigate(`/audit/compare?a=${encodeURIComponent(compareId)}&b=${encodeURIComponent(selectedId)}`); + } + return (
-

Audit

+
+

Audit

+
+ + +
+
+
- { setTool(e.target.value); setPage(0); }} /> - { setUser(e.target.value); setPage(0); }} /> + { setTool(e.target.value); setPage(0); }} /> + { setUser(e.target.value); setPage(0); }} /> - { setSuccess(e.target.value as "" | "true" | "false"); setPage(0); }}> - { setSearch(e.target.value); setPage(0); }} /> + { setSearch(e.target.value); setPage(0); }} />
+ {showFilters && ( + { setJsonFilters(fs); setPage(0); }} + /> + )} + + {liveTail && ( +
+
+ Live tail (most recent first) + {tailError && {tailError}} +
+ {tailEvents.length === 0 ? ( +
Waiting for events...
+ ) : ( +
    + {tailEvents.map((e) => ( +
  • + + + {e.success ? "ok" : (e.error_category ?? "error")} + +
  • + ))} +
+ )} +
+ )} + + {compareId && ( +
+
+ Stashed for compare:{" "} + {compareId.slice(0, 8)}...{compareId.slice(-4)} +
+
+ {selectedId && selectedId !== compareId && ( + + )} + +
+
+ )} +
@@ -71,7 +249,13 @@ export default function Audit() { {q.data?.events.map((e) => ( - + selectEvent(e.id)} + > - {q.data?.events.map((e) => ( + {(q.data?.events ?? []).map((e) => ( - {d.recent.map((e) => ( + {(d.recent ?? []).map((e) => (
{new Date(e.timestamp).toLocaleString()} {e.tool_name} @@ -100,11 +284,116 @@ export default function Audit() { )} + + selectEvent(null)} + onCompareSelect={stashCompare} + /> + + ); +} + +function JsonFiltersEditor({ + filters, + onChange, +}: { + filters: JsonFilter[]; + onChange: (fs: JsonFilter[]) => void; +}) { + const newRef = useRef(null); + const [draft, setDraft] = useState({ source: "param", path: "", value: "" }); + + function add() { + if (draft.source === "has") { + if (!draft.path) return; + onChange([...filters, { ...draft, value: "" }]); + } else { + if (!draft.path || !draft.value) return; + onChange([...filters, { ...draft }]); + } + setDraft({ source: draft.source, path: "", value: "" }); + newRef.current?.focus(); + } + + function remove(i: number) { + onChange(filters.filter((_, j) => j !== i)); + } + + return ( +
+
+ JSONB path filters narrow against audit_payloads. + Values are type-detected (true/false → bool, integers / floats → number, + else string; quote to force string). Header values are always strings. +
+
    + {filters.map((f, i) => ( +
  • + + {f.source === "has" ? `has=${f.path}` : `${f.source}.${f.path}=${f.value}`} + + +
  • + ))} +
+
+ + {draft.source === "has" ? ( + + ) : ( + <> + setDraft({ ...draft, path: e.target.value })} + onKeyDown={(e) => { if (e.key === "Enter") add(); }} + /> + setDraft({ ...draft, value: e.target.value })} + onKeyDown={(e) => { if (e.key === "Enter") add(); }} + /> + + )} + +
); } -const inputCls = "w-full bg-background border border-input rounded px-2 py-1 text-sm focus:outline-none focus:ring-2 focus:ring-ring"; +const inputCls = "bg-background border border-input rounded px-2 py-1 text-sm focus:outline-none focus:ring-2 focus:ring-ring"; function Field({ label, children }: { label: string; children: React.ReactNode }) { return ( @@ -115,9 +404,6 @@ function Field({ label, children }: { label: string; children: React.ReactNode } ); } -// displayUser shows the most human-readable identifier we have for the audit -// row's caller: email first (OIDC), then API-key name ("apikey:NAME"), then -// the raw subject (Keycloak UUID, etc.) as a last resort. function displayUser(e: { user_email?: string; user_subject?: string }): string { if (e.user_email) return e.user_email; const sub = e.user_subject ?? ""; diff --git a/ui/src/pages/Compare.tsx b/ui/src/pages/Compare.tsx new file mode 100644 index 0000000..d424c4e --- /dev/null +++ b/ui/src/pages/Compare.tsx @@ -0,0 +1,369 @@ +import { useQuery } from "@tanstack/react-query"; +import { useSearchParams, Link } from "react-router-dom"; +import { ArrowLeft } from "lucide-react"; +import { portalAPI, type AuditEvent, type AuditPayload, HttpError } from "@/lib/api"; + +// Compare renders a side-by-side structural diff of two audit events. +// /portal/audit/compare?a=&b=. We fetch both events in parallel +// and walk a JSON-path tree, marking each leaf as one of: +// - same (value equal in both) +// - diff (key present in both, values differ) +// - only-A / only-B (key present in one side) +// Highlights propagate up to parent nodes for quick scanning. +// +// JSON-tree differs from text-diff: it doesn't get confused by key +// reordering between Postgres reads, and it understands leaf-vs-tree +// distinction (a key whose value changes from a string to a map shows +// up as a single diff at that path, not 20 lines of "everything moved"). +export default function Compare() { + const [params] = useSearchParams(); + const aId = params.get("a") ?? ""; + const bId = params.get("b") ?? ""; + + const a = useQuery({ + queryKey: ["audit-event", aId], + queryFn: () => portalAPI.auditEvent(aId), + enabled: !!aId, + }); + const b = useQuery({ + queryKey: ["audit-event", bId], + queryFn: () => portalAPI.auditEvent(bId), + enabled: !!bId, + }); + + if (!aId || !bId) { + return ( +
+

Compare events

+

+ Open this page with ?a=<id>&b=<id> in the + URL, or use the Compare button in the audit drawer to stage two events. +

+
+ ); + } + + return ( +
+
+ + back to Audit + +

Compare events

+
+ +
+
+
+
+ + {a.data && b.data && ( + <> +
+ + + + + + )} +
+ ); +} + +function Header({ + title, + id, + ev, + loading, + error, +}: { + title: string; + id: string; + ev?: AuditEvent; + loading: boolean; + error: HttpError | null; +}) { + return ( +
+
{title}
+
{id}
+ {loading &&
Loading...
} + {error &&
{error.message}
} + {ev && ( +
+ {ev.tool_name}{" "} + {new Date(ev.timestamp).toLocaleString()}{" "} + + {ev.success ? "ok" : (ev.error_category ?? "error")} + +
+ )} +
+ ); +} + +type Row = { k: string; a?: React.ReactNode; b?: React.ReactNode; diff: boolean }; + +function summaryRows(a: AuditEvent, b: AuditEvent): Row[] { + const rows: Row[] = []; + const add = (k: string, av?: React.ReactNode, bv?: React.ReactNode) => { + rows.push({ k, a: av, b: bv, diff: stringify(av) !== stringify(bv) }); + }; + add("Tool", a.tool_name, b.tool_name); + add("Source", a.source, b.source); + add("Result", a.success ? "ok" : (a.error_category ?? "error"), b.success ? "ok" : (b.error_category ?? "error")); + add("Duration", `${a.duration_ms} ms`, `${b.duration_ms} ms`); + add("User", a.user_email ?? a.user_subject ?? "-", b.user_email ?? b.user_subject ?? "-"); + add("Auth type", a.auth_type ?? "-", b.auth_type ?? "-"); + return rows; +} + +function stringify(v: React.ReactNode): string { + return v === undefined || v === null ? "" : String(v); +} + +function Section({ title, rows }: { title: string; rows: Row[] }) { + const anyDiff = rows.some((r) => r.diff); + return ( +
+
+ {title} + {anyDiff && differences} +
+ + + {rows.map((r) => ( + + + + + + ))} + +
{r.k}{r.a ?? -}{r.b ?? -}
+
+ ); +} + +function PayloadSection({ + title, + aPath, + bPath, +}: { + title: string; + aPath?: Record; + bPath?: Record; +}) { + if (!aPath && !bPath) return null; + const tree = buildDiffTree(aPath, bPath); + return ( +
0 ? "border-warning/40" : "border-border" + }`}> +
+ {title} + {tree.diffCount > 0 && ( + {tree.diffCount} difference{tree.diffCount === 1 ? "" : "s"} + )} +
+
+ {tree.children && tree.children.length > 0 ? ( + + ) : tree.diffCount > 0 ? ( + // Leaf-only root with an actual difference: one side is missing + // the entire payload object, or both sides are non-equal scalars. + // Render a single-line before/after so the user sees what changed. +
+ +
+ ) : ( + // Both sides are present and equal at root with no leaf children + // (e.g. both response_error are {}). Don't render a noisy + // "(root): (undefined)" line. +
No differences.
+ )} +
+
+ ); +} + +function NotificationsSection({ + a, + b, +}: { + a?: AuditPayload; + b?: AuditPayload; +}) { + const aN = a?.notifications ?? []; + const bN = b?.notifications ?? []; + if (aN.length === 0 && bN.length === 0) return null; + return ( +
+
+ Notifications + A: {aN.length} / B: {bN.length} +
+
+
    + {aN.map((n, i) => ( +
  1. + {n.method}{typeof n.params?.message === "string" ? ` "${n.params.message}"` : ""} +
  2. + ))} + {aN.length === 0 &&
  3. none
  4. } +
+
    + {bN.map((n, i) => ( +
  1. + {n.method}{typeof n.params?.message === "string" ? ` "${n.params.message}"` : ""} +
  2. + ))} + {bN.length === 0 &&
  3. none
  4. } +
+
+
+ ); +} + +// --- Diff tree --- + +type DiffNode = { + kind: "same" | "diff" | "only-a" | "only-b"; + // For object/array nodes, children are the named keys / indices. + children?: { key: string; node: DiffNode }[]; + aValue?: unknown; + bValue?: unknown; + diffCount: number; +}; + +function buildDiffTree(a: unknown, b: unknown): DiffNode { + // Treat one-side-undefined as walking the present side fully so the + // tree shows per-key only-A / only-B markers instead of a single + // root-level "(undefined) → {…}" leaf. + if (a === undefined && (isObj(b) || Array.isArray(b))) { + a = isObj(b) ? {} : []; + } else if (b === undefined && (isObj(a) || Array.isArray(a))) { + b = isObj(a) ? {} : []; + } + if (isObj(a) && isObj(b)) { + const keys = new Set([...Object.keys(a), ...Object.keys(b)]); + const children: { key: string; node: DiffNode }[] = []; + let diffCount = 0; + for (const k of Array.from(keys).sort()) { + const inA = k in a; + const inB = k in b; + let node: DiffNode; + if (inA && inB) { + node = buildDiffTree((a as Record)[k], (b as Record)[k]); + } else if (inA) { + node = { kind: "only-a", aValue: (a as Record)[k], diffCount: 1 }; + } else { + node = { kind: "only-b", bValue: (b as Record)[k], diffCount: 1 }; + } + diffCount += node.diffCount; + children.push({ key: k, node }); + } + return { kind: diffCount > 0 ? "diff" : "same", children, diffCount }; + } + if (Array.isArray(a) && Array.isArray(b)) { + const len = Math.max(a.length, b.length); + const children: { key: string; node: DiffNode }[] = []; + let diffCount = 0; + for (let i = 0; i < len; i++) { + const inA = i < a.length; + const inB = i < b.length; + let node: DiffNode; + if (inA && inB) { + node = buildDiffTree(a[i], b[i]); + } else if (inA) { + node = { kind: "only-a", aValue: a[i], diffCount: 1 }; + } else { + node = { kind: "only-b", bValue: b[i], diffCount: 1 }; + } + diffCount += node.diffCount; + children.push({ key: String(i), node }); + } + return { kind: diffCount > 0 ? "diff" : "same", children, diffCount }; + } + // Leaves: deep-equal via JSON.stringify (cheap; both values are + // already in JSON-shape from the audit_payloads JSONB columns). + if (JSON.stringify(a) === JSON.stringify(b)) { + return { kind: "same", aValue: a, bValue: b, diffCount: 0 }; + } + return { kind: "diff", aValue: a, bValue: b, diffCount: 1 }; +} + +function isObj(v: unknown): v is Record { + return v !== null && typeof v === "object" && !Array.isArray(v); +} + +function DiffTreeView({ node, nested = false }: { node: DiffNode; nested?: boolean }) { + if (!node.children) return null; + // Indentation comes from each nested
    's own left padding; the + // outer (root)
      renders flush so deep trees don't walk off-panel. + return ( +
        + {node.children.map((c) => ( +
      • + + {c.node.children && c.node.children.length > 0 && ( + + )} +
      • + ))} +
      + ); +} + +function DiffTreeRow({ keyName, node }: { keyName: string; node: DiffNode }) { + if (node.children && node.children.length > 0) { + return ( + + {keyName}: + + ); + } + switch (node.kind) { + case "same": + return ( + + {keyName}: {fmt(node.aValue)} + + ); + case "diff": + return ( + + {keyName}:{" "} + {fmt(node.aValue)} + {" → "} + {fmt(node.bValue)} + + ); + case "only-a": + return ( + + - {keyName}: {fmt(node.aValue)} + + ); + case "only-b": + return ( + + + {keyName}: {fmt(node.bValue)} + + ); + } +} + +function fmt(v: unknown): string { + if (v === null) return "null"; + if (v === undefined) return "(undefined)"; + if (typeof v === "string") return JSON.stringify(v); + if (typeof v === "number" || typeof v === "boolean") return String(v); + // Compact rendering for nested structures so the row stays single-line. + const s = JSON.stringify(v); + return s.length > 80 ? s.slice(0, 77) + "..." : s; +} From 99073ad80c6b786b52c3c32a95c4cf8977f445d8 Mon Sep 17 00:00:00 2001 From: cjimti Date: Wed, 6 May 2026 15:44:29 -0700 Subject: [PATCH 2/3] fix(audit): address PR #12 critical review Fixes the 4 MAJORs and 6 Notable MINORs surfaced by the post-push critical review. Server (Go): - Header redaction at the source: auth.WithHeaders now stashes a RedactHeaders-cloned copy of the inbound HTTP headers, so credential- bearing names (Authorization, Proxy-Authorization, Cookie, Set-Cookie, X-API-Key in any case) land in audit_payloads.request_headers as "[redacted]" rather than verbatim. Pre-existing leak (the doc comment claimed redaction; the implementation didn't); PR #12 first put those bytes in front of UI users so the fix lands here. - Replay rate-limit ordering: pkg/httpsrv/portal_api.auditReplay now consumes a token only after all four validation checks pass (event exists, payload captured, no redacted params, tool registered). An operator clicking Replay on five summary-only rows in a row no longer loses their burst budget. Auth check still runs first so unauthenticated callers can't fan out reads. - Filter contract endpoint: GET /api/v1/portal/audit/meta returns {has_keys, json_sources, replay, export} so a UI can build its filter editor against the server's source of truth instead of duplicating allow-lists. AllowedHasKeysList / AllowedJSONSourcesList are derived from the existing exported vars (single source of truth); TestAllowList_FunctionAndSliceAgree extended to enforce symmetry and slice-isolation. UI (TypeScript): - Drawer header on error: spinner during loading, AlertCircle on detail-fetch failure, title reads "Failed to load event" rather than "Loading...". - Replay confirmation: the Replay button now opens a ConfirmModal that calls out the side-effect re-run; default focus is on Cancel so a reflexive Enter dismisses rather than fires. Esc cancels via capture-phase handler so it doesn't bubble to the drawer's own Esc. - Replay client-side preflight: button is disabled with a tooltip reason when the row has no captured payload or any param is redacted. Mirrors the server's hasRedactedParam check via a small hasRedactedValue helper. - Stale-replay guard: the mutation now takes the event id as a variable; ReplayBanner is only rendered when replay.variables matches the open drawer, so a navigate-away-mid-flight no longer shows the prior result against the new event. - Replay reset race: skip replay.reset() while replay.isPending so switching events mid-flight doesn't clobber the in-flight UI state. - Drawer focus management: save the previously-focused element on open, focus the close button, restore on unmount. - Filter editor sources its has-keys list from /audit/meta via TanStack Query (cached, no retry); HAS_KEYS_FALLBACK kept for offline-first rendering. - Compare stash cleared on signOut and on 401: extracted COMPARE_KEY to ui/src/lib/storage-keys.ts; auth.signOut and the 401 handler call clearSessionScopedState so a stashed event id doesn't survive the session. Docs: - inspection.md: header-redaction policy added to the Request-tab section; replay-button confirmation behavior and "tokens consumed only after validation" called out; "ring buffer" replaced with "fixed-cap most-recent-first list (cap 20)". - http-api.md: new /audit/meta row; replay row updated to mention the post-validation token-consumption contract. Pre-commit gate: 2 review rounds. Round 1 caught the original 11 findings + 1 regression (ConfirmModal capture-phase Enter would have fired Confirm regardless of focus); fixed by removing the Enter handler entirely so native button activation handles the Cancel default. Round 2 verified all fixes, surfaced one asymmetric-test gap on AllowedJSONSourcesList mutation isolation; fixed in-tree. make verify + make codeql + pnpm tsc + pnpm build all green. --- docs/operations/inspection.md | 8 +- docs/reference/http-api.md | 3 +- pkg/audit/jsonfilter.go | 23 +++++- pkg/audit/jsonfilter_test.go | 38 +++++++++ pkg/auth/context.go | 40 +++++++++- pkg/auth/context_test.go | 38 +++++++++ pkg/httpsrv/portal_api.go | 44 +++++++++-- pkg/httpsrv/portal_api_replay_test.go | 52 +++++++++++++ pkg/httpsrv/portal_api_test.go | 46 +++++++++++ pkg/mcpmw/audit.go | 14 ++-- pkg/mcpmw/audit_payload_test.go | 19 ++++- ui/src/components/ConfirmModal.tsx | 107 ++++++++++++++++++++++++++ ui/src/components/EventDrawer.tsx | 103 +++++++++++++++++++++---- ui/src/lib/api.ts | 8 ++ ui/src/lib/storage-keys.ts | 4 + ui/src/pages/Audit.tsx | 23 +++++- ui/src/stores/auth.ts | 15 ++++ 17 files changed, 540 insertions(+), 45 deletions(-) create mode 100644 ui/src/components/ConfirmModal.tsx create mode 100644 ui/src/lib/storage-keys.ts diff --git a/docs/operations/inspection.md b/docs/operations/inspection.md index 8ee1de8..2f7d0d4 100644 --- a/docs/operations/inspection.md +++ b/docs/operations/inspection.md @@ -30,7 +30,7 @@ In the portal, navigate to `Audit`. Each row in the events table is clickable; t Timing, identity, request id, session id, source (`mcp` for real client calls, `portal-tryit` for /admin/tryit invocations, `portal-replay` for replays), and the replay linkage (`Replayed from`) when present. ### Request tab -The captured `request_params` (sanitized via `audit.redact_keys`, with redacted values shown as `"[redacted]"`). Captured request headers when `audit.capture_headers: true`. A truncation warning when the request body exceeded `audit.max_payload_bytes`. +The captured `request_params` (sanitized via `audit.redact_keys`, with redacted values shown as `"[redacted]"`). Captured request headers when `audit.capture_headers: true` — credential-bearing names (`Authorization`, `Cookie`, `Set-Cookie`, `Proxy-Authorization`, `X-API-Key`) are stored as `"[redacted]"` regardless of the redact-keys config; the names remain visible so an operator can confirm "this request carried an Authorization header" without seeing the token. A truncation warning when the request body exceeded `audit.max_payload_bytes`. ### Response tab The full `CallToolResult` content blocks (text, image, audio, structured) plus `response_error` when the call errored. The shape matches what the SDK serializes to the wire so you can see what the client saw. A truncation warning fires when the response body was too large. @@ -51,9 +51,9 @@ The drawer's **Replay** button calls `POST /api/v1/portal/audit/events/{id}/repl The replay banner inside the drawer shows the new event id; clicking it deep-links to that row. Refused replays show a banner explaining why (most common: redacted parameter values, no captured payload, or a tool that's no longer registered). -**Replay re-runs side effects.** If the original call wrote to a database, sent a notification, or charged a card, the replay does it again. There is no dry-run mode and no per-tool allow list. Treat replay like Try-It: a developer affordance for debugging, not a production self-service. +**Replay re-runs side effects.** If the original call wrote to a database, sent a notification, or charged a card, the replay does it again. There is no dry-run mode and no per-tool allow list. Treat replay like Try-It: a developer affordance for debugging, not a production self-service. The portal asks for confirmation before firing the request; the disabled-state of the Replay button telegraphs whether the row is replayable at all (it isn't, when the original payload wasn't captured or any param was redacted). -Per-identity rate limit (scoped by API key id or OIDC subject): 5 burst, one token refilled every 12 seconds (sustained 5/min). `429` with `Retry-After` when exhausted. +Per-identity rate limit (scoped by API key id or OIDC subject): 5 burst, one token refilled every 12 seconds (sustained 5/min). `429` with `Retry-After` when exhausted. Tokens are only consumed after validation passes, so clicks on non-replayable rows return `400` without burning the operator's budget. ## 4. Compare to a baseline @@ -87,7 +87,7 @@ Filters are AND-combined with each other and with the indexed-column filters (to ## 6. Live tail -The **Live tail** toggle on the Audit page opens an SSE connection to `/api/v1/portal/audit/stream`. New audit events appear in a small ring buffer above the table as they're written; clicking one opens the drawer. The table itself stays a historical-filter view so the live tail doesn't blow away your filtered context. +The **Live tail** toggle on the Audit page opens an SSE connection to `/api/v1/portal/audit/stream`. New audit events appear in a fixed-cap most-recent-first list (cap 20) above the table as they're written; clicking one opens the drawer. The table itself stays a historical-filter view so the live tail doesn't blow away your filtered context. The stream sends an opening `: connected` comment on connect, an `event: audit\ndata: ` per write, and a `: keepalive` comment every 30 seconds. Slow consumers see per-subscriber drops; the producer never blocks. diff --git a/docs/reference/http-api.md b/docs/reference/http-api.md index a0caa71..5636f25 100644 --- a/docs/reference/http-api.md +++ b/docs/reference/http-api.md @@ -49,9 +49,10 @@ Behind the cookie or `X-API-Key` / `Authorization: Bearer`. | `GET` | `/api/v1/portal/instructions` | The `server.instructions` text the MCP server hands to clients at initialize time. | | `GET` | `/api/v1/portal/tools` | List of `{name, group, description, input_schema}` for every registered tool. | | `GET` | `/api/v1/portal/tools/{name}` | Same shape, single tool. | +| `GET` | `/api/v1/portal/audit/meta` | Filter contract surface: `{has_keys, json_sources, replay: {burst, refill_secs, sustained_min}, export: {max_rows}}`. Lets a UI build its filter editor against the server's source of truth without duplicating allow-lists. | | `GET` | `/api/v1/portal/audit/events` | Paginated audit events. Query: `from`, `to` (RFC 3339), `tool`, `user`, `session`, `success`, `q`, `limit`, `offset`, plus the JSONB filters described below. | | `GET` | `/api/v1/portal/audit/events/{id}` | Single event by id (UUID); includes the captured payload row when present. 400 on a non-UUID id, 404 when the event isn't recorded. | -| `POST` | `/api/v1/portal/audit/events/{id}/replay` | Re-invokes the captured tool call through an in-process MCP client. Writes a new audit event tagged `source=portal-replay` with `replayed_from` pointing at `{id}`. Per-identity rate limited (5 burst, 1 token / 12s); returns `429 Too Many Requests` with `Retry-After` when exhausted. Refuses (`400`) if the original event has no captured payload, has redacted parameter values, or names a tool no longer registered. CSRF-gated via `X-Requested-With`. | +| `POST` | `/api/v1/portal/audit/events/{id}/replay` | Re-invokes the captured tool call through an in-process MCP client. Writes a new audit event tagged `source=portal-replay` with `replayed_from` pointing at `{id}`. Per-identity rate limited (5 burst, 1 token / 12s); returns `429 Too Many Requests` with `Retry-After` when exhausted. Tokens are consumed *after* validation passes, so a click on a non-replayable row (no payload, redacted params, missing tool) returns `400` without burning the operator's budget. CSRF-gated via `X-Requested-With`. | | `GET` | `/api/v1/portal/audit/export` | NDJSON stream of summary rows for a filter. `format=jsonl` (default) is the only supported format. Same filter surface as `/events`. Capped at 100,000 rows per request. | | `GET` | `/api/v1/portal/audit/stream` | SSE live tail of new audit events. One `event: audit\ndata: ` per write; opening comment `: connected` confirms the connection; `: keepalive` every 30 seconds. Sets `X-Accel-Buffering: no` for nginx-fronted deployments. | | `GET` | `/api/v1/portal/audit/timeseries` | Bucketed counts. Query: `from`, `to`, `bucket` (Go duration). | diff --git a/pkg/audit/jsonfilter.go b/pkg/audit/jsonfilter.go index 5a743ec..a25c1b1 100644 --- a/pkg/audit/jsonfilter.go +++ b/pkg/audit/jsonfilter.go @@ -134,12 +134,27 @@ func numericEq(a float64, b any) bool { return false } +// AllowedHasKeysList returns a fresh clone of AllowedHasKeys for callers +// that need to surface the list (e.g. a portal /audit/meta endpoint). +// Cloning ensures a downstream caller cannot mutate the package-level +// var. The actual gate at parse time stays the closed-switch +// IsAllowedHasKey; this helper, the exported var, and the switch must +// stay synchronized — TestAllowList_FunctionAndSliceAgree enforces it. +func AllowedHasKeysList() []string { + return append([]string(nil), AllowedHasKeys...) +} + +// AllowedJSONSourcesList returns a fresh clone of AllowedJSONSources. +// Same contract as AllowedHasKeysList. +func AllowedJSONSourcesList() []string { + return append([]string(nil), AllowedJSONSources...) +} + // IsAllowedHasKey reports whether key is an allowlisted has= column. // Implemented as a closed switch (not a slice iteration) so the -// AllowedHasKeys exported var cannot be mutated by an importing package -// to widen what gets spliced into the verbatim SQL column reference in -// buildSelect. The slice stays exported for documentation generators -// and reflection callers; the gate is the function. +// internal allowlist cannot be mutated by an importing package to widen +// what gets spliced into the verbatim SQL column reference in +// buildSelect. func IsAllowedHasKey(key string) bool { switch key { case "request_params", diff --git a/pkg/audit/jsonfilter_test.go b/pkg/audit/jsonfilter_test.go index e998d04..8c2fa5a 100644 --- a/pkg/audit/jsonfilter_test.go +++ b/pkg/audit/jsonfilter_test.go @@ -226,4 +226,42 @@ func TestAllowList_FunctionAndSliceAgree(t *testing.T) { t.Errorf("IsAllowedJSONSource(%q) = true, want false", s) } } + + // AllowedHasKeysList / AllowedJSONSourcesList must mirror the + // underlying slices exactly and must return a fresh clone each call + // (a downstream caller mutating the returned slice must not affect + // the next caller). + if got := AllowedHasKeysList(); len(got) != len(AllowedHasKeys) { + t.Fatalf("AllowedHasKeysList len = %d, want %d", len(got), len(AllowedHasKeys)) + } + for i, k := range AllowedHasKeysList() { + if k != AllowedHasKeys[i] { + t.Errorf("AllowedHasKeysList[%d] = %q, want %q (drift vs AllowedHasKeys)", i, k, AllowedHasKeys[i]) + } + } + first := AllowedHasKeysList() + first[0] = "MUTATED" + if AllowedHasKeysList()[0] == "MUTATED" { + t.Error("AllowedHasKeysList() returned a shared slice; mutation leaked across callers") + } + if AllowedHasKeys[0] == "MUTATED" { + t.Error("AllowedHasKeysList() returned the package var directly; mutation leaked into AllowedHasKeys") + } + + if got := AllowedJSONSourcesList(); len(got) != len(AllowedJSONSources) { + t.Fatalf("AllowedJSONSourcesList len = %d, want %d", len(got), len(AllowedJSONSources)) + } + for i, s := range AllowedJSONSourcesList() { + if s != AllowedJSONSources[i] { + t.Errorf("AllowedJSONSourcesList[%d] = %q, want %q", i, s, AllowedJSONSources[i]) + } + } + firstSrc := AllowedJSONSourcesList() + firstSrc[0] = "MUTATED" + if AllowedJSONSourcesList()[0] == "MUTATED" { + t.Error("AllowedJSONSourcesList() returned a shared slice; mutation leaked across callers") + } + if AllowedJSONSources[0] == "MUTATED" { + t.Error("AllowedJSONSourcesList() returned the package var directly; mutation leaked into AllowedJSONSources") + } } diff --git a/pkg/auth/context.go b/pkg/auth/context.go index 64a0953..2b89f92 100644 --- a/pkg/auth/context.go +++ b/pkg/auth/context.go @@ -41,10 +41,46 @@ func GetIdentity(ctx context.Context) *Identity { return nil } +// sensitiveHeaders is the set of inbound HTTP header names whose values are +// stripped before the headers reach ctx (and from there, the audit_payloads +// row). These carry credentials (Authorization, Cookie, X-API-Key) or proxy +// auth state, none of which a tool needs to introspect and all of which are +// dangerous to surface in an audit-log UI. Names are matched +// case-insensitively via http.Header's canonical form. +var sensitiveHeaders = map[string]struct{}{ + "Authorization": {}, + "Proxy-Authorization": {}, + "Cookie": {}, + "Set-Cookie": {}, + "X-Api-Key": {}, // canonical form of X-API-Key. +} + +// RedactHeaders returns a clone of h with sensitive header values replaced by +// a single "[redacted]" entry. Header names are preserved so an operator +// reading the audit log can still see "this request carried an Authorization +// header" without seeing the bearer. +func RedactHeaders(h http.Header) http.Header { + if h == nil { + return nil + } + out := make(http.Header, len(h)) + for k, vs := range h { + if _, ok := sensitiveHeaders[http.CanonicalHeaderKey(k)]; ok { + out[k] = []string{"[redacted]"} + continue + } + out[k] = append([]string(nil), vs...) + } + return out +} + // WithHeaders stashes a redacted clone of the inbound HTTP headers, so MCP tool -// handlers can introspect them via GetHeaders. +// handlers can introspect them via GetHeaders. Sensitive headers +// (Authorization, Cookie, X-API-Key, etc.) are replaced with "[redacted]" +// before storage; any future audit-log reader sees the names but not the +// secret values. func WithHeaders(ctx context.Context, h http.Header) context.Context { - return context.WithValue(ctx, keyHeaders, h) + return context.WithValue(ctx, keyHeaders, RedactHeaders(h)) } // GetHeaders retrieves the captured headers, or nil if none were stashed. diff --git a/pkg/auth/context_test.go b/pkg/auth/context_test.go index 3d07920..f58d987 100644 --- a/pkg/auth/context_test.go +++ b/pkg/auth/context_test.go @@ -56,3 +56,41 @@ func TestAnonymous(t *testing.T) { t.Errorf("anonymous identity wrong: %+v", id) } } + +func TestWithHeaders_RedactsSensitive(t *testing.T) { + ctx := context.Background() + in := http.Header{ + "Authorization": []string{"Bearer secret-token"}, + "Cookie": []string{"session=abc; csrf=def"}, + "Set-Cookie": []string{"new=value"}, + "Proxy-Authorization": []string{"Basic xyz"}, + "X-Api-Key": []string{"real-api-key"}, + "X-API-KEY": []string{"shouted-api-key"}, + "User-Agent": []string{"curl/8.0"}, + "X-Test": []string{"v1", "v2"}, + } + ctx = WithHeaders(ctx, in) + out := GetHeaders(ctx) + for _, name := range []string{"Authorization", "Cookie", "Set-Cookie", "Proxy-Authorization", "X-Api-Key", "X-API-KEY"} { + if got := out.Get(name); got != "[redacted]" { + t.Errorf("%s should be redacted, got %q", name, got) + } + } + if got := out.Get("User-Agent"); got != "curl/8.0" { + t.Errorf("User-Agent should pass through, got %q", got) + } + if got := out.Values("X-Test"); len(got) != 2 || got[0] != "v1" || got[1] != "v2" { + t.Errorf("X-Test multi-value should pass through, got %v", got) + } + // Mutating the input after stash must not affect what the audit row sees. + in.Set("Authorization", "Bearer different") + if got := out.Get("Authorization"); got != "[redacted]" { + t.Errorf("post-stash mutation leaked: %q", got) + } +} + +func TestRedactHeaders_NilSafe(t *testing.T) { + if got := RedactHeaders(nil); got != nil { + t.Errorf("nil input should yield nil, got %v", got) + } +} diff --git a/pkg/httpsrv/portal_api.go b/pkg/httpsrv/portal_api.go index 49807c3..48259e1 100644 --- a/pkg/httpsrv/portal_api.go +++ b/pkg/httpsrv/portal_api.go @@ -75,6 +75,7 @@ func (p *PortalAPI) Mount(mux *http.ServeMux, mw func(http.Handler) http.Handler mux.Handle("GET /api/v1/portal/instructions", mw(http.HandlerFunc(p.instructions))) mux.Handle("GET /api/v1/portal/tools", mw(http.HandlerFunc(p.tools))) mux.Handle("GET /api/v1/portal/tools/{name}", mw(http.HandlerFunc(p.toolDetail))) + mux.Handle("GET /api/v1/portal/audit/meta", mw(http.HandlerFunc(p.auditMeta))) mux.Handle("GET /api/v1/portal/audit/events", mw(http.HandlerFunc(p.auditEvents))) mux.Handle("GET /api/v1/portal/audit/events/{id}", mw(http.HandlerFunc(p.auditEventDetail))) mux.Handle("GET /api/v1/portal/audit/export", mw(http.HandlerFunc(p.auditExport))) @@ -148,15 +149,13 @@ func (p *PortalAPI) auditReplay(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusUnauthorized, errors.New("authenticated identity required")) return } - if !p.limiterForReplay().Allow(idKey) { - retry := p.limiterForReplay().RetryAfter(idKey) - w.Header().Set("Retry-After", strconv.Itoa(int(retry.Round(time.Second).Seconds()))) - writeError(w, http.StatusTooManyRequests, - fmt.Errorf("replay rate limit exceeded; retry in %s", retry.Round(time.Second))) - return - } - // Fetch original event + payload. + // Validate the replay request BEFORE consuming a rate-limit token so + // that operator clicks on rows that can't replay (no captured payload, + // redacted params, missing tool) don't burn the user's per-identity + // burst quota. The auth check above keeps the DB-read fan-out gated + // to authenticated callers; the cost of a Query + GetPayload per + // pre-validation call is acceptable. events, err := p.audit.Query(r.Context(), audit.QueryFilter{EventID: eventID, Limit: 1}) if err != nil { writeError(w, http.StatusInternalServerError, err) @@ -205,6 +204,16 @@ func (p *PortalAPI) auditReplay(w http.ResponseWriter, r *http.Request) { } } + // Validation passed — only now consume a token. A 429 here costs a + // real replay attempt, not a click on a non-replayable row. + if !p.limiterForReplay().Allow(idKey) { + retry := p.limiterForReplay().RetryAfter(idKey) + w.Header().Set("Retry-After", strconv.Itoa(int(retry.Round(time.Second).Seconds()))) + writeError(w, http.StatusTooManyRequests, + fmt.Errorf("replay rate limit exceeded; retry in %s", retry.Round(time.Second))) + return + } + // Deep-copy the captured params before passing to CallTool so // the SDK / tool handlers can't mutate the original-event audit // row's RequestParams via the shared map pointer (the @@ -767,6 +776,25 @@ func parseQueryFilter(r *http.Request) audit.QueryFilter { return f } +// auditMeta returns the JSON-filter contract surface (allowlisted has= +// columns and JSON-source prefixes) so the portal UI can build its +// filter editor against the server's source of truth instead of +// duplicating the list in client code. +func (p *PortalAPI) auditMeta(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, http.StatusOK, map[string]any{ + "has_keys": audit.AllowedHasKeysList(), + "json_sources": audit.AllowedJSONSourcesList(), + "replay": map[string]any{ + "burst": replayBurst, + "refill_secs": int(replayRefill / time.Second), + "sustained_min": int(time.Minute / replayRefill), + }, + "export": map[string]any{ + "max_rows": maxExportEvents, + }, + }) +} + func (p *PortalAPI) auditEvents(w http.ResponseWriter, r *http.Request) { f := parseQueryFilter(r) if f.Limit == 0 { diff --git a/pkg/httpsrv/portal_api_replay_test.go b/pkg/httpsrv/portal_api_replay_test.go index 185f11d..186aabc 100644 --- a/pkg/httpsrv/portal_api_replay_test.go +++ b/pkg/httpsrv/portal_api_replay_test.go @@ -290,6 +290,58 @@ func TestPortalAPI_AuditReplay_RateLimit(t *testing.T) { } } +// TestPortalAPI_AuditReplay_FailedValidationDoesNotConsumeToken verifies +// that 4xx pre-validation failures (no captured payload, redacted params, +// missing tool) don't burn the per-identity rate-limit budget. Operators +// clicking Replay on summary-only rows shouldn't lose their replay quota +// for an hour. +func TestPortalAPI_AuditReplay_FailedValidationDoesNotConsumeToken(t *testing.T) { + mux, mem := portalReplayMux(t, nil) + + // Stage replayBurst+1 events that will all fail validation + // (no captured RequestParams). Then a final replayable event. + noPayloadIDs := make([]string, 0, replayBurst+1) + for i := 0; i < replayBurst+1; i++ { + ev := audit.Event{ + Timestamp: time.Now(), + ToolName: "whoami", // distinct from stagedEvent's "echo" so the + Success: true, // helper's tool-name lookup doesn't pick these. + Payload: nil, // nothing captured -> 400 from auditReplay + } + _ = mem.Log(context.Background(), ev) + // MemoryLogger.Log assigns an id when none is set; read it + // back from the snapshot so we can target the new row. + snap := mem.Snapshot() + noPayloadIDs = append(noPayloadIDs, snap[len(snap)-1].ID) + } + good := stagedEvent(t, mem, map[string]any{"message": "ok"}) + + // Each no-payload click returns 400 — must NOT consume a token. + for _, id := range noPayloadIDs { + body := bytes.NewReader([]byte(`{}`)) + req := httptest.NewRequest(http.MethodPost, + "/api/v1/portal/audit/events/"+id+"/replay", body) + req.Header.Set("X-Requested-With", "XMLHttpRequest") + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Fatalf("no-payload replay status = %d, want 400", w.Code) + } + } + + // After replayBurst+1 failed clicks, the burst budget must still be + // untouched: the next valid replay must succeed (not 429). + body := bytes.NewReader([]byte(`{}`)) + req := httptest.NewRequest(http.MethodPost, + "/api/v1/portal/audit/events/"+good+"/replay", body) + req.Header.Set("X-Requested-With", "XMLHttpRequest") + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Errorf("post-failures replay status = %d, want 200 (failed validations should not have burned tokens)", w.Code) + } +} + func TestCallToolResultToMap_ContentTypes(t *testing.T) { cases := []struct { name string diff --git a/pkg/httpsrv/portal_api_test.go b/pkg/httpsrv/portal_api_test.go index b9ed674..45f9778 100644 --- a/pkg/httpsrv/portal_api_test.go +++ b/pkg/httpsrv/portal_api_test.go @@ -85,6 +85,52 @@ func TestPortalAPI_Wellknown(t *testing.T) { } } +func TestPortalAPI_AuditMeta_Shape(t *testing.T) { + mux := portalTestMux(t, audit.NewMemoryLogger()) + resp := httptest.NewRecorder() + mux.ServeHTTP(resp, httptest.NewRequest(http.MethodGet, "/api/v1/portal/audit/meta", nil)) + if resp.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", resp.Code) + } + var got struct { + HasKeys []string `json:"has_keys"` + JSONSources []string `json:"json_sources"` + Replay struct { + Burst int `json:"burst"` + RefillSecs int `json:"refill_secs"` + SustainedMin int `json:"sustained_min"` + } `json:"replay"` + Export struct { + MaxRows int `json:"max_rows"` + } `json:"export"` + } + if err := json.NewDecoder(resp.Body).Decode(&got); err != nil { + t.Fatalf("decode: %v", err) + } + if len(got.HasKeys) != len(audit.AllowedHasKeys) { + t.Errorf("has_keys len = %d, want %d", len(got.HasKeys), len(audit.AllowedHasKeys)) + } + for i, k := range got.HasKeys { + if k != audit.AllowedHasKeys[i] { + t.Errorf("has_keys[%d] = %q, want %q", i, k, audit.AllowedHasKeys[i]) + } + } + if got.Replay.Burst != replayBurst { + t.Errorf("replay.burst = %d, want %d", got.Replay.Burst, replayBurst) + } + if got.Replay.RefillSecs != int(replayRefill/time.Second) { + t.Errorf("replay.refill_secs = %d, want %d", got.Replay.RefillSecs, int(replayRefill/time.Second)) + } + // sustained_min must be derived from refill, not hardcoded to burst. + wantSustained := int(time.Minute / replayRefill) + if got.Replay.SustainedMin != wantSustained { + t.Errorf("replay.sustained_min = %d, want %d (60 / refill_secs)", got.Replay.SustainedMin, wantSustained) + } + if got.Export.MaxRows != maxExportEvents { + t.Errorf("export.max_rows = %d, want %d", got.Export.MaxRows, maxExportEvents) + } +} + func TestPortalAPI_AuditTimeseriesAndBreakdown(t *testing.T) { mem := audit.NewMemoryLogger() now := time.Now().UTC() diff --git a/pkg/mcpmw/audit.go b/pkg/mcpmw/audit.go index 5d53e30..285ae76 100644 --- a/pkg/mcpmw/audit.go +++ b/pkg/mcpmw/audit.go @@ -185,9 +185,10 @@ func Audit(chain *auth.Chain, logger audit.Logger, redactKeys []string, toolGrou // // Each side (request, response) is size-bounded; oversize JSON content // is dropped wholesale and the matching truncated flag is set. Headers -// are reflected exactly as ctx already carries them (the audit -// middleware clones + redacts them in enrichContext via the caller's -// HeaderCapture middleware). +// are reflected exactly as ctx already carries them; auth.WithHeaders +// redacts credential-bearing names (Authorization / Cookie / X-API-Key +// / Proxy-Authorization / Set-Cookie) at stash time so the bytes never +// reach this row. func buildPayload( ctx context.Context, method string, @@ -215,10 +216,9 @@ func buildPayload( p.RequestSizeBytes = size } - // Headers: only when the operator opted in. enrichContext already - // cloned the inbound header set; HeaderCapture (HTTP layer) is - // responsible for stripping Authorization / Cookie before they - // reach ctx. + // Headers: only when the operator opted in. enrichContext stashed + // them via auth.WithHeaders, which already redacted credential- + // bearing names; the values here are safe to land in audit_payloads. if opts.captureHeaders { if h := auth.GetHeaders(ctx); h != nil { p.RequestHeaders = map[string][]string(h) diff --git a/pkg/mcpmw/audit_payload_test.go b/pkg/mcpmw/audit_payload_test.go index 3a7d6c6..f0b1a90 100644 --- a/pkg/mcpmw/audit_payload_test.go +++ b/pkg/mcpmw/audit_payload_test.go @@ -144,11 +144,20 @@ func TestAudit_PayloadCapture_HeadersOptIn(t *testing.T) { t.Errorf("headers captured without opt-in: %+v", h) } - // With WithHeaderCapture: headers stored. + // With WithHeaderCapture: headers stored, sensitive ones redacted. mem2 := audit.NewMemoryLogger() mw2 := Audit(chain, mem2, nil, nil, WithPayloadCapture(0), WithHeaderCapture()) wrapped2 := mw2((&fakeMethodHandler{}).handle) - _, _ = wrapped2(context.Background(), "tools/call", req) + reqWithSecrets := &mcp.ServerRequest[*mcp.CallToolParams]{ + Params: &mcp.CallToolParams{Name: "headers"}, + Extra: &mcp.RequestExtra{Header: http.Header{ + "X-Test": []string{"abc"}, + "Authorization": []string{"Bearer leak-me"}, + "Cookie": []string{"session=secret"}, + "X-Api-Key": []string{"real-key"}, + }}, + } + _, _ = wrapped2(context.Background(), "tools/call", reqWithSecrets) h := mem2.Snapshot()[0].Payload.RequestHeaders if h == nil { t.Fatal("expected headers captured with WithHeaderCapture") @@ -156,6 +165,12 @@ func TestAudit_PayloadCapture_HeadersOptIn(t *testing.T) { if got := h["X-Test"]; len(got) != 1 || got[0] != "abc" { t.Errorf("X-Test = %v", got) } + for _, name := range []string{"Authorization", "Cookie", "X-Api-Key"} { + got := http.Header(h).Get(name) + if got != "[redacted]" { + t.Errorf("%s should be redacted in audit_payloads.request_headers, got %q", name, got) + } + } } func TestAudit_PayloadCapture_PreservesNonTextContent(t *testing.T) { diff --git a/ui/src/components/ConfirmModal.tsx b/ui/src/components/ConfirmModal.tsx new file mode 100644 index 0000000..45a0357 --- /dev/null +++ b/ui/src/components/ConfirmModal.tsx @@ -0,0 +1,107 @@ +import { useEffect, useRef } from "react"; +import { AlertTriangle } from "lucide-react"; + +// ConfirmModal renders a small confirm/cancel dialog over the page. +// Esc cancels. Auto-focuses the cancel button so a reflexive Enter +// fires Cancel (the button's native default-action behavior), not +// Confirm — defense against a user who opens a destructive-action +// modal and immediately hits Enter. +export function ConfirmModal({ + open, + title, + message, + confirmLabel = "Confirm", + cancelLabel = "Cancel", + danger = false, + onConfirm, + onCancel, +}: { + open: boolean; + title: string; + message: React.ReactNode; + confirmLabel?: string; + cancelLabel?: string; + danger?: boolean; + onConfirm: () => void; + onCancel: () => void; +}) { + const cancelRef = useRef(null); + + useEffect(() => { + if (!open) return; + cancelRef.current?.focus(); + // Capture-phase Esc handler so the modal cancels itself before any + // ancestor (e.g., the EventDrawer's window-level Esc handler) sees + // the key and closes the drawer too. Enter is intentionally NOT + // captured here: with focus on the Cancel button, the browser's + // native button-activation handles Enter for us, so a reflexive + // Enter cancels rather than confirms. + const onKey = (e: KeyboardEvent) => { + if (e.key === "Escape") { + e.stopPropagation(); + onCancel(); + } + }; + window.addEventListener("keydown", onKey, true); + return () => window.removeEventListener("keydown", onKey, true); + }, [open, onCancel]); + + if (!open) return null; + + return ( + <> +
      +
      +
      +
      + {danger && ( + + )} +
      +

      {title}

      +
      {message}
      +
      +
      +
      + + +
      +
      +
      + + ); +} + +// hasRedactedValue walks v looking for any string equal to the +// audit-pipeline's "[redacted]" marker. Mirrors the server-side +// hasRedactedParam check so the UI can disable Replay before the +// server has to refuse it. Returns true on a matching leaf at any +// depth. +export function hasRedactedValue(v: unknown): boolean { + if (v === "[redacted]") return true; + if (Array.isArray(v)) return v.some(hasRedactedValue); + if (v !== null && typeof v === "object") { + return Object.values(v as Record).some(hasRedactedValue); + } + return false; +} diff --git a/ui/src/components/EventDrawer.tsx b/ui/src/components/EventDrawer.tsx index 5b8807b..f49cc68 100644 --- a/ui/src/components/EventDrawer.tsx +++ b/ui/src/components/EventDrawer.tsx @@ -1,9 +1,10 @@ -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import { Link } from "react-router-dom"; -import { X, Play, GitCompare, AlertCircle, CheckCircle2 } from "lucide-react"; +import { X, Play, GitCompare, AlertCircle, CheckCircle2, Loader2 } from "lucide-react"; import { portalAPI, type AuditEvent, type ReplayResponse, HttpError } from "@/lib/api"; import { JsonView } from "./JsonView"; +import { hasRedactedValue, ConfirmModal } from "./ConfirmModal"; type Tab = "overview" | "request" | "response" | "notifications"; @@ -23,7 +24,10 @@ export function EventDrawer({ onCompareSelect: (id: string) => void; }) { const [tab, setTab] = useState("overview"); + const [confirmReplay, setConfirmReplay] = useState(false); const qc = useQueryClient(); + const closeBtnRef = useRef(null); + const restoreFocusRef = useRef(null); // ESC closes. useEffect(() => { @@ -40,14 +44,33 @@ export function EventDrawer({ if (eventId) setTab("overview"); }, [eventId]); + // Save the previously-focused element on open and restore it on close. + // Auto-focus the close button so keyboard users can dismiss with Enter. + useEffect(() => { + if (!eventId) return; + restoreFocusRef.current = document.activeElement as HTMLElement | null; + closeBtnRef.current?.focus(); + return () => { + restoreFocusRef.current?.focus?.(); + restoreFocusRef.current = null; + }; + }, [eventId]); + const detail = useQuery({ queryKey: ["audit-event", eventId], queryFn: () => portalAPI.auditEvent(eventId!), enabled: !!eventId, }); - const replay = useMutation({ - mutationFn: () => portalAPI.auditReplay(eventId!), + // The mutation takes the event id as a variable (rather than closing + // over the outer `eventId`) so we can: (a) tell at render time which + // event the in-flight replay was fired for via `replay.variables`, + // and (b) suppress the banner when the user has navigated to a + // different drawer mid-flight. Without the variable, a replay fired + // on event A would resolve into the open drawer for event B and + // mislead the operator into thinking they replayed B. + const replay = useMutation({ + mutationFn: (id: string) => portalAPI.auditReplay(id), onSuccess: () => { // Refresh the events list so the new replay row appears. void qc.invalidateQueries({ queryKey: ["audit"] }); @@ -55,15 +78,27 @@ export function EventDrawer({ }); // Reset replay state on event change so a banner from a prior replay - // doesn't bleed into a different drawer. + // doesn't bleed into a different drawer. Skip the reset while a replay + // is in flight so we don't clobber an isPending state and mislead the + // user into thinking nothing's running. useEffect(() => { - replay.reset(); + if (!replay.isPending) replay.reset(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [eventId]); if (!eventId) return null; const ev = detail.data; + // Replay is impossible when the original payload wasn't captured or + // any param was redacted; mirror the server-side validation client- + // side so a click doesn't even attempt the request (and so the + // disabled button telegraphs "this row can't be replayed"). + const replayBlockReason = (() => { + if (!ev) return "Loading event detail..."; + if (!ev.payload?.request_params) return "No captured request payload to replay."; + if (hasRedactedValue(ev.payload.request_params)) return "Captured params contain redacted values; replay would not exercise the same call."; + return null; + })(); return ( <> @@ -83,13 +118,17 @@ export function EventDrawer({
      - {ev?.success === false ? ( + {detail.isLoading ? ( + + ) : detail.isError ? ( + + ) : ev?.success === false ? ( ) : ( )}

      - {ev?.tool_name ?? "Loading..."} + {detail.isError ? "Failed to load event" : ev?.tool_name ?? "Loading..."}

      {ev?.source && ( @@ -110,14 +149,15 @@ export function EventDrawer({ Compare
      - + +

      + Replay re-runs the captured request through the live MCP server with the + same arguments. Any side effect the original call had + (database writes, outbound API calls, billable operations) will fire again. +

      + {ev?.tool_name && ( +

      + Tool: {ev.tool_name} +

      + )} +

      + Replay re-runs side effects. Treat like Try-It, not a production self-service. +

      + + } + confirmLabel="Replay" + danger + onConfirm={() => { + setConfirmReplay(false); + replay.mutate(eventId); + }} + onCancel={() => setConfirmReplay(false)} + /> + + {/* Suppress the banner when the in-flight replay was fired + against a different event (user navigated away mid-flight). + replay.variables is undefined in idle state and equal to the + event id passed to mutate() while pending or settled. */} + {(!replay.variables || replay.variables === eventId) && ( + + )}
{new Date(e.timestamp).toLocaleTimeString()} {e.tool_name}