B2: scrub Sentry events and add per-request scope tags

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",

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() });

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<void> => {
+    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.
+      }
+    }
+  };
+}

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<string, string> = {};
+      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<string, unknown>;
+    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<string, unknown> = {};
+    for (const [k, v] of Object.entries(breadcrumb.data)) {
+      if (ALLOWED_BREADCRUMB_DATA_KEYS.has(k)) safe[k] = v;
+    }
+    breadcrumb.data = safe;
+  }
+
+  return breadcrumb;
+}

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<typeof vi.fn>;
+  addBreadcrumb: ReturnType<typeof vi.fn>;
+  capturedTags: Record<string, string>;
+}
+
+function makeMockSentry(): MockSentry {
+  const capturedTags: Record<string, string> = {};
+  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);
+  });
+});