diff --git a/packages/server/package.json b/packages/server/package.json index 28e3c193..4555e801 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -23,6 +23,7 @@ "author": "", "license": "Apache-2.0", "devDependencies": { + "@sentry/types": "^8.9.2", "@types/node": "^25.5.2", "tsx": "^4.21.0", "typescript": "^6.0.2", diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index dd7ac9c3..6f62d28c 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -10,6 +10,8 @@ import { createNodeWebSocket } from "@hono/node-ws"; import { cors } from "hono/cors"; import { sentry } from "@hono/sentry"; import { requestLog } from "./middleware/requestLog.js"; +import { sentryContext } from "./middleware/sentryContext.js"; +import { scrubBeforeBreadcrumb, scrubBeforeSend } from "./middleware/sentryScrubber.js"; import piloRoutes from "./routes/pilo.js"; import { createPiloWsRoute } from "./routes/piloWs.js"; @@ -18,11 +20,14 @@ const app = new Hono(); // Create WebSocket support const { upgradeWebSocket, injectWebSocket } = createNodeWebSocket({ app }); -// Add Sentry middleware +// Add Sentry middleware. Scrubbers run on every event/breadcrumb before +// transport so user content never leaves the process via Sentry. app.use( "*", sentry({ dsn: process.env.SENTRY_DSN, + beforeSend: scrubBeforeSend, + beforeBreadcrumb: scrubBeforeBreadcrumb, }), ); @@ -46,6 +51,9 @@ app.use( // Structured request access log (metadata only — no path/body/headers/IP). app.use("*", requestLog()); +// Sentry per-request scope tags (taskId, method, route, status). +app.use("*", sentryContext()); + // Health check endpoint app.get("/health", (c) => { return c.json({ status: "ok", timestamp: new Date().toISOString() }); diff --git a/packages/server/src/middleware/sentryContext.ts b/packages/server/src/middleware/sentryContext.ts new file mode 100644 index 00000000..793fa065 --- /dev/null +++ b/packages/server/src/middleware/sentryContext.ts @@ -0,0 +1,39 @@ +/** + * Per-request Sentry scope tags. + * + * Runs after the @hono/sentry middleware so each request has its own Toucan + * instance to tag. Adds bounded, non-sensitive tags so Sentry events from a + * given request can be filtered/grouped on the dashboard: + * + * - method (GET/POST/...) + * - route (matched pattern, e.g. "/pilo/run") + * - status (HTTP status as a string) + * - taskId (when the response carries an x-pilo-task-id header) + * + * No request body, URL, headers, or other request-derived content is tagged. + * + * Defensive: if @hono/sentry middleware isn't registered (no DSN, tests), + * getSentry() throws and we silently no-op. + */ +import type { Context, Next } from "hono"; +import { getSentry } from "@hono/sentry"; + +export function sentryContext() { + return async (c: Context, next: Next): Promise => { + try { + await next(); + } finally { + try { + const sentry = getSentry(c); + if (!sentry) return; + sentry.setTag("method", c.req.method); + sentry.setTag("route", c.req.routePath || "unknown"); + sentry.setTag("status", String(c.res?.status ?? 0)); + const taskId = c.res?.headers.get("x-pilo-task-id"); + if (taskId) sentry.setTag("taskId", taskId); + } catch { + // Sentry not available (no DSN, or middleware not registered) — no-op. + } + } + }; +} diff --git a/packages/server/src/middleware/sentryScrubber.ts b/packages/server/src/middleware/sentryScrubber.ts new file mode 100644 index 00000000..4626a4ec --- /dev/null +++ b/packages/server/src/middleware/sentryScrubber.ts @@ -0,0 +1,99 @@ +/** + * Sentry scrubber hooks. + * + * Acts as a defense-in-depth layer at the boundary where Sentry events leave + * the process. Even when upstream code is careful with `error.message` and + * span attributes (Stack A's invariants), Sentry's @hono/sentry middleware + * also auto-captures request data and unhandled exceptions, which can include + * `error.message` strings and request bodies. These hooks ensure that any + * such auto-captured content is stripped before transport. + * + * If a regression introduces a leak upstream, these scrubbers still catch it. + * Treat them as safety net, not primary defense. + */ +import type { Breadcrumb, BreadcrumbHint, ErrorEvent, EventHint } from "@sentry/types"; + +const STRIPPED_HEADER_NAMES = new Set(["authorization", "cookie", "set-cookie"]); + +/** + * Allowlist of breadcrumb data keys that are bounded, non-sensitive, and + * server-controlled. Any breadcrumb data key not in this set is dropped + * before the breadcrumb reaches Sentry, regardless of where it came from. + */ +const ALLOWED_BREADCRUMB_DATA_KEYS = new Set([ + "taskId", + "method", + "route", + "status", + "phase", + "reason", + "result", + "provider", + "model", + "iteration", + "tool", + "error_class", + "duration_ms", +]); + +/** + * Sentry `beforeSend` hook. Removes potentially sensitive fields from the + * event before it leaves the process. + */ +export function scrubBeforeSend(event: ErrorEvent, _hint?: EventHint): ErrorEvent { + if (event.request) { + const req = event.request; + delete req.data; + delete req.query_string; + delete req.cookies; + if (req.headers) { + const safeHeaders: Record = {}; + for (const [k, v] of Object.entries(req.headers)) { + if (STRIPPED_HEADER_NAMES.has(k.toLowerCase())) continue; + if (typeof v === "string") safeHeaders[k] = v; + } + req.headers = safeHeaders; + } + } + + // Reduce exception values to class names only; drop stacktrace. + if (event.exception?.values) { + for (const ex of event.exception.values) { + ex.value = ex.type ?? "Unknown"; + delete ex.stacktrace; + } + } + + // Drop response body if Sentry's request integration attached one. + if (event.contexts?.response) { + const response = event.contexts.response as Record; + delete response.body; + } + + // Don't forward arbitrary `extra` payloads — they're often where ad-hoc + // user content gets attached. Tags are bounded; extras are not. + delete event.extra; + + return event; +} + +/** + * Sentry `beforeBreadcrumb` hook. Drops console breadcrumbs entirely (verbose + * + may contain user content) and applies an allowlist to breadcrumb `data`. + */ +export function scrubBeforeBreadcrumb( + breadcrumb: Breadcrumb, + _hint?: BreadcrumbHint, +): Breadcrumb | null { + if (breadcrumb.category === "console") return null; + + if (breadcrumb.data) { + const safe: Record = {}; + for (const [k, v] of Object.entries(breadcrumb.data)) { + if (ALLOWED_BREADCRUMB_DATA_KEYS.has(k)) safe[k] = v; + } + breadcrumb.data = safe; + } + + return breadcrumb; +} diff --git a/packages/server/test/middleware/sentryContext.test.ts b/packages/server/test/middleware/sentryContext.test.ts new file mode 100644 index 00000000..d4702dde --- /dev/null +++ b/packages/server/test/middleware/sentryContext.test.ts @@ -0,0 +1,106 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Hono } from "hono"; + +const SENTINEL = "SENSITIVE-CANARY-q4r7"; + +interface MockSentry { + setTag: ReturnType; + addBreadcrumb: ReturnType; + capturedTags: Record; +} + +function makeMockSentry(): MockSentry { + const capturedTags: Record = {}; + return { + capturedTags, + setTag: vi.fn((key: string, value: string) => { + capturedTags[key] = value; + }), + addBreadcrumb: vi.fn(), + }; +} + +// Mock @hono/sentry's getSentry so we can intercept the request-scoped Toucan +// without standing up a real Sentry transport. +let mockSentry: MockSentry | null = null; +let getSentryThrows = false; + +vi.mock("@hono/sentry", () => ({ + getSentry: (_c: unknown) => { + if (getSentryThrows) throw new Error("sentry middleware not registered"); + return mockSentry; + }, +})); + +describe("sentryContext middleware", () => { + let app: Hono; + + beforeEach(async () => { + mockSentry = makeMockSentry(); + getSentryThrows = false; + app = new Hono(); + const { sentryContext } = await import("../../src/middleware/sentryContext.js"); + app.use("*", sentryContext()); + }); + + it("sets method, route, and status tags after the request handler runs", async () => { + app.get("/foo", (c) => c.json({ ok: true })); + await app.request("/foo"); + + expect(mockSentry!.capturedTags.method).toBe("GET"); + expect(mockSentry!.capturedTags.route).toBe("/foo"); + expect(mockSentry!.capturedTags.status).toBe("200"); + }); + + it("sets the taskId tag from x-pilo-task-id response header when present", async () => { + app.get("/foo", (c) => { + c.header("x-pilo-task-id", "task-abc-123"); + return c.json({ ok: true }); + }); + await app.request("/foo"); + + expect(mockSentry!.capturedTags.taskId).toBe("task-abc-123"); + }); + + it("omits the taskId tag when no x-pilo-task-id header is set", async () => { + app.get("/foo", (c) => c.json({ ok: true })); + await app.request("/foo"); + + expect(mockSentry!.capturedTags.taskId).toBeUndefined(); + }); + + it("sets tags even when the handler throws", async () => { + app.get("/foo", () => { + throw new Error("boom"); + }); + try { + await app.request("/foo"); + } catch { + // ignore — the throw is incidental, we want to verify the finally block + } + expect(mockSentry!.capturedTags.method).toBe("GET"); + expect(mockSentry!.capturedTags.route).toBe("/foo"); + }); + + it("does not throw when @hono/sentry middleware is not registered", async () => { + getSentryThrows = true; + app.get("/foo", (c) => c.json({ ok: true })); + const res = await app.request("/foo"); + expect(res.status).toBe(200); + }); + + it("never sets a tag value derived from request body / URL (canary)", async () => { + app.post("/foo", async (c) => { + await c.req.json().catch(() => undefined); + return c.json({ ok: true }); + }); + await app.request(`/foo?secret=${SENTINEL}`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ task: SENTINEL }), + }); + + const allTagValues = Object.values(mockSentry!.capturedTags).join(" "); + expect(allTagValues).not.toContain(SENTINEL); + }); +}); diff --git a/packages/server/test/middleware/sentryScrubber.test.ts b/packages/server/test/middleware/sentryScrubber.test.ts new file mode 100644 index 00000000..e3e5139d --- /dev/null +++ b/packages/server/test/middleware/sentryScrubber.test.ts @@ -0,0 +1,200 @@ +import { describe, it, expect } from "vitest"; +import { scrubBeforeBreadcrumb, scrubBeforeSend } from "../../src/middleware/sentryScrubber.js"; +import type { Breadcrumb, ErrorEvent } from "@sentry/types"; + +const SENTINEL = "SENSITIVE-CANARY-q4r7"; + +describe("scrubBeforeSend", () => { + it("strips request.data (request body)", () => { + const event: ErrorEvent = { + request: { + method: "POST", + url: "https://api.example.com/pilo/run", + data: { task: SENTINEL, secret: "abc" }, + }, + }; + const result = scrubBeforeSend(event); + expect(result.request?.data).toBeUndefined(); + }); + + it("strips request.query_string", () => { + const event: ErrorEvent = { + request: { query_string: `token=${SENTINEL}` }, + }; + const result = scrubBeforeSend(event); + expect(result.request?.query_string).toBeUndefined(); + }); + + it("strips request.cookies", () => { + const event: ErrorEvent = { + request: { cookies: { session: SENTINEL } }, + }; + const result = scrubBeforeSend(event); + expect(result.request?.cookies).toBeUndefined(); + }); + + it("strips Authorization, Cookie, Set-Cookie headers (case-insensitive)", () => { + const event: ErrorEvent = { + request: { + headers: { + Authorization: `Bearer ${SENTINEL}`, + cookie: `session=${SENTINEL}`, + "Set-Cookie": `refresh=${SENTINEL}`, + "Content-Type": "application/json", + "X-Custom": "safe-value", + }, + }, + }; + const result = scrubBeforeSend(event); + const headers = result.request?.headers ?? {}; + expect(headers.Authorization).toBeUndefined(); + expect(headers.cookie).toBeUndefined(); + expect(headers["Set-Cookie"]).toBeUndefined(); + expect(headers["Content-Type"]).toBe("application/json"); + expect(headers["X-Custom"]).toBe("safe-value"); + }); + + it("reduces exception.value to exception.type (drops error.message)", () => { + const event: ErrorEvent = { + exception: { + values: [ + { + type: "TypeError", + value: `Cannot read property of ${SENTINEL}`, + }, + ], + }, + }; + const result = scrubBeforeSend(event); + expect(result.exception?.values?.[0].value).toBe("TypeError"); + }); + + it("drops exception.stacktrace (may contain page content via selectors)", () => { + const event: ErrorEvent = { + exception: { + values: [ + { + type: "Error", + value: "boom", + stacktrace: { + frames: [{ filename: "x", function: `processSelector("${SENTINEL}")` }], + }, + }, + ], + }, + }; + const result = scrubBeforeSend(event); + expect(result.exception?.values?.[0].stacktrace).toBeUndefined(); + }); + + it("drops contexts.response.body", () => { + const event: ErrorEvent = { + contexts: { + response: { status_code: 200, body: { task: SENTINEL } }, + }, + }; + const result = scrubBeforeSend(event); + const response = result.contexts?.response as { body?: unknown }; + expect(response?.body).toBeUndefined(); + }); + + it("drops event.extra (arbitrary user-attached data)", () => { + const event: ErrorEvent = { + extra: { context: SENTINEL }, + }; + const result = scrubBeforeSend(event); + expect(result.extra).toBeUndefined(); + }); + + it("never lets the sentinel reach the post-scrub event (canary)", () => { + const event: ErrorEvent = { + request: { + data: { task: SENTINEL }, + query_string: `q=${SENTINEL}`, + cookies: { c: SENTINEL }, + headers: { Authorization: `Bearer ${SENTINEL}` }, + }, + exception: { + values: [{ type: "Err", value: SENTINEL, stacktrace: { frames: [] } }], + }, + extra: { foo: SENTINEL }, + contexts: { + response: { body: SENTINEL }, + }, + }; + const result = scrubBeforeSend(event); + expect(JSON.stringify(result)).not.toContain(SENTINEL); + }); + + it("preserves bounded metadata (tags, level, environment)", () => { + const event: ErrorEvent = { + tags: { taskId: "task-123", phase: "execution", reason: "INTERNAL_ERROR" }, + level: "error", + environment: "production", + }; + const result = scrubBeforeSend(event); + expect(result.tags).toEqual({ + taskId: "task-123", + phase: "execution", + reason: "INTERNAL_ERROR", + }); + expect(result.level).toBe("error"); + expect(result.environment).toBe("production"); + }); +}); + +describe("scrubBeforeBreadcrumb", () => { + it("drops console breadcrumbs (Sentry auto-captures, often verbose)", () => { + const breadcrumb: Breadcrumb = { + category: "console", + message: "some log", + data: { args: [SENTINEL] }, + }; + expect(scrubBeforeBreadcrumb(breadcrumb)).toBeNull(); + }); + + it("strips disallowed keys from data, keeps allowlisted ones", () => { + const breadcrumb: Breadcrumb = { + category: "task", + message: "task.start", + data: { + taskId: "task-123", + method: "POST", + route: "/pilo/run", + url: `https://example.com/${SENTINEL}`, + body: { task: SENTINEL }, + secret: SENTINEL, + }, + }; + const result = scrubBeforeBreadcrumb(breadcrumb); + expect(result?.data).toEqual({ + taskId: "task-123", + method: "POST", + route: "/pilo/run", + }); + }); + + it("never lets sentinel data reach the post-scrub breadcrumb (canary)", () => { + const breadcrumb: Breadcrumb = { + category: "task", + message: "anything", + data: { + taskId: "task-123", + url: `https://example.com/${SENTINEL}`, + body: SENTINEL, + anything: SENTINEL, + }, + }; + const result = scrubBeforeBreadcrumb(breadcrumb); + expect(JSON.stringify(result)).not.toContain(SENTINEL); + }); + + it("returns the breadcrumb unchanged when data is absent", () => { + const breadcrumb: Breadcrumb = { + category: "task", + message: "task.start", + }; + const result = scrubBeforeBreadcrumb(breadcrumb); + expect(result).toEqual(breadcrumb); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0518c752..3f6a9c5a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -391,6 +391,9 @@ importers: specifier: workspace:* version: link:../core devDependencies: + '@sentry/types': + specifier: ^8.9.2 + version: 8.9.2 '@types/node': specifier: ^25.5.2 version: 25.6.0