From 33bd5910d90f6cbb21265f7903b9e122b2d53b14 Mon Sep 17 00:00:00 2001 From: oktofeesh1 <287075021+oktofeesh1@users.noreply.github.com> Date: Tue, 30 Jun 2026 13:42:26 -0700 Subject: [PATCH] feat(selfhost): harden Sentry event scrubbing --- src/selfhost/sentry.ts | 193 +++++++++++++++++++++++++++--- test/unit/selfhost-sentry.test.ts | 186 +++++++++++++++++++++++++++- 2 files changed, 361 insertions(+), 18 deletions(-) diff --git a/src/selfhost/sentry.ts b/src/selfhost/sentry.ts index ecd5957e55..10b6422aa6 100644 --- a/src/selfhost/sentry.ts +++ b/src/selfhost/sentry.ts @@ -2,6 +2,10 @@ // env-gated, dynamically-imported selfhost-integration pattern (Redis/Qdrant/embed-provider in server.ts). // @sentry/node is NEVER imported at module top level — it loads lazily inside initSentry(), so it never enters // the Worker bundle (src/index.ts) and cloudflare:* stubbing stays clean. All helpers are safe to call when off. +import { + PUBLIC_LOCAL_PATH_SCRUB_PATTERN, + PUBLIC_UNSAFE_TERMS, +} from "../signals/redaction"; import { currentOtelTraceIds } from "./otel"; type SentryNs = typeof import("@sentry/node"); @@ -17,6 +21,36 @@ let sentryEnvironment = "production"; const SECRET_KEY = /(token|secret|key|password|passwd|authorization|auth|dsn|cookie|bearer|credential|private)/i; +const PAYLOAD_KEY = + /(^|[_-])(body|payload|patch|diff|prompt|rubric|guardrail|headers?|cookies?|title|config|review[-_]?text|review[-_]?content)([_-]|$)|^(body|payload|patch|diff|prompt|rubric|guardrail|headers?|cookies?|title|config|review[-_]?text|review[-_]?content)$/i; +const SECRET_VALUE = new RegExp( + [ + `${"github" + "_pat_"}[A-Za-z0-9_]+`, + String.raw`gh[opsru]_[A-Za-z0-9_]{20,}`, + String.raw`sk-[A-Za-z0-9_-]{20,}`, + String.raw`xox[baprs]-[A-Za-z0-9-]+`, + String.raw`Bearer\s+[A-Za-z0-9._~+/=-]{12,}`, + String.raw`-----BEGIN [^-]+ PRIVATE KEY-----[\s\S]*?-----END [^-]+ PRIVATE KEY-----`, + ].join("|"), + "gi", +); +const JWT_VALUE = /\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\b/g; +const QUERY_SECRET_VALUE = + /([?&;][^=\s&#;]*(?:token|secret|key|password|passwd|authorization|auth|dsn|cookie|bearer|credential|private)[^=\s&#;]*=)[^&#\s;]+/gi; +const PRIVATE_TEXT = + /\b(raw[-_\s]?score|scoring context|private rubric|gate prompt|review prompt|guardrail paths?|pull request body|pr body|pr title|raw diff)\b/gi; +const PUBLIC_UNSAFE_SCRUB = new RegExp(String.raw`\b(${PUBLIC_UNSAFE_TERMS})\b`, "gi"); +const ALLOWED_CONTEXTS = new Set([ + "gittensory", + "review", + "log", + "sentry_monitor", + "otel", + "trace", + "runtime", + "os", +]); +const REDACTED = "[redacted]"; function nonBlank(value: string | undefined): string | undefined { const trimmed = value?.trim(); @@ -105,30 +139,156 @@ export function resolveSentryRelease( } /** beforeSend scrubber — redact anything token/secret-like before an event leaves the box (privacy boundary). */ -export function scrubEvent(event: T): T { - const redact = (obj: unknown, depth: number): void => { - if (!obj || typeof obj !== "object" || depth > 6) return; - for (const key of Object.keys(obj as Record)) { - const rec = obj as Record; - if (SECRET_KEY.test(key)) rec[key] = "[redacted]"; - else if (typeof rec[key] === "object") redact(rec[key], depth + 1); - } - }; +export function scrubEvent(event: T): T | null { try { const e = event as { - request?: { headers?: unknown }; - contexts?: unknown; - extra?: unknown; + request?: Record; + contexts?: Record; + extra?: Record; + tags?: Record; + breadcrumbs?: Array>; + exception?: unknown; + logentry?: unknown; + message?: unknown; + spans?: unknown; + transaction?: unknown; + user?: unknown; }; - redact(e.request?.headers, 0); - redact(e.contexts, 0); - redact(e.extra, 0); + scrubRequest(e.request); + scrubAllowedContexts(e.contexts); + scrubRecord(e.extra, 0); + scrubRecord(e.tags, 0); + scrubRecord(e.exception, 0); + scrubRecord(e.logentry, 0); + scrubRecord(e.spans, 0); + delete e.user; + if (typeof e.message === "string") e.message = scrubString(e.message); + if (typeof e.transaction === "string") e.transaction = scrubString(e.transaction); + if (Array.isArray(e.breadcrumbs)) { + for (const breadcrumb of e.breadcrumbs) scrubRecord(breadcrumb, 0); + } } catch { - /* scrubbing must never break the send */ + return null; } return event; } +function shouldRedactKey(key: string): boolean { + const compact = key.replace(/[^A-Za-z0-9]/g, "").toLowerCase(); + return ( + SECRET_KEY.test(key) || + PAYLOAD_KEY.test(key) || + /(body|payload|patch|diff|prompt|rubric|guardrail|header|cookie|title|config|reviewtext|reviewcontent|prcontent|pullrequest)/.test(compact) + ); +} + +function scrubString(value: string): string { + return value + .replace(QUERY_SECRET_VALUE, `$1${REDACTED}`) + .replace(SECRET_VALUE, REDACTED) + .replace(JWT_VALUE, REDACTED) + .replace(PUBLIC_LOCAL_PATH_SCRUB_PATTERN, "") + .replace(PUBLIC_UNSAFE_SCRUB, "private context") + .replace(PRIVATE_TEXT, "private context"); +} + +function scrubRecord(obj: unknown, depth: number): void { + if (!obj || typeof obj !== "object") return; + if (Array.isArray(obj)) { + for (let i = 0; i < obj.length; i++) { + const value = obj[i]; + if (typeof value === "string") obj[i] = scrubString(value); + else if (value && typeof value === "object") { + if (depth >= 6) obj[i] = REDACTED; + else scrubRecord(value, depth + 1); + } + } + return; + } + const rec = obj as Record; + for (const key of Object.keys(rec)) { + if (shouldRedactKey(key)) { + rec[key] = REDACTED; + continue; + } + const value = rec[key]; + if (typeof value === "string") rec[key] = scrubStringField(key, value); + else if (value && typeof value === "object") { + if (depth >= 6) rec[key] = REDACTED; + else scrubRecord(value, depth + 1); + } + } +} + +function scrubStringField(key: string, value: string): string { + if (isUrlKey(key)) return scrubUrl(value); + if (isQueryKey(key)) return scrubQueryString(value); + return scrubString(value); +} + +function isUrlKey(key: string): boolean { + return key.replace(/[^A-Za-z0-9]/g, "").toLowerCase().endsWith("url"); +} + +function isQueryKey(key: string): boolean { + const compact = key.replace(/[^A-Za-z0-9]/g, "").toLowerCase(); + return compact === "query" || compact === "querystring"; +} + +function scrubUrl(value: string): string { + const scrubbed = scrubString(value); + const queryStart = scrubbed.indexOf("?"); + if (queryStart === -1) return scrubbed; + try { + const parsed = new URL(scrubbed); + parsed.search = scrubQueryString(parsed.search); + return parsed.toString(); + } catch { + return `${scrubbed.slice(0, queryStart + 1)}${scrubQueryString( + scrubbed.slice(queryStart + 1), + )}`; + } +} + +function scrubQueryString(value: string): string { + const hasQuestionMark = value.startsWith("?"); + const source = hasQuestionMark ? value.slice(1) : value; + const params = new URLSearchParams(source); + for (const key of Array.from(new Set(params.keys()))) { + const values = params.getAll(key); + params.delete(key); + for (const entry of values) { + params.append(key, shouldRedactKey(key) ? REDACTED : scrubString(entry)); + } + } + const scrubbed = params.toString(); + return hasQuestionMark ? `?${scrubbed}` : scrubbed; +} + +function scrubRequest(request: Record | undefined): void { + if (!request) return; + scrubRecord(request.headers, 0); + for (const key of ["url", "query_string", "queryString", "query"] as const) { + const value = request[key]; + if (typeof value === "string") request[key] = scrubStringField(key, value); + else if (value && typeof value === "object") scrubRecord(value, 0); + } + for (const key of ["body", "data", "payload", "cookies"] as const) { + if (key in request) delete request[key]; + } +} + +function scrubAllowedContexts(contexts: Record | undefined): void { + if (!contexts) return; + for (const key of Object.keys(contexts)) { + if (!ALLOWED_CONTEXTS.has(key)) { + delete contexts[key]; + continue; + } + scrubRecord(contexts[key], 0); + } +} + /** Initialize Sentry from the environment. Returns false (and stays a no-op) when SENTRY_DSN is unset. */ export async function initSentry(env: NodeJS.ProcessEnv): Promise { if (!env.SENTRY_DSN) return false; @@ -142,6 +302,7 @@ export async function initSentry(env: NodeJS.ProcessEnv): Promise { tracesSampleRate: Number(env.SENTRY_TRACES_SAMPLE_RATE ?? "0"), serverName: env.PUBLIC_API_ORIGIN, beforeSend: (e) => scrubEvent(e), + beforeSendTransaction: (e) => scrubEvent(e), }); active = true; return true; diff --git a/test/unit/selfhost-sentry.test.ts b/test/unit/selfhost-sentry.test.ts index a027241de5..c7e70f8770 100644 --- a/test/unit/selfhost-sentry.test.ts +++ b/test/unit/selfhost-sentry.test.ts @@ -52,10 +52,17 @@ beforeEach(() => { // value) so issues show a real "type: value", never "(No error message)". This reads back the last captured Error. const lastCapturedError = (): Error => mocks.captureException.mock.calls.at(-1)?.[0] as Error; +const scrubbedEvent = (event: T): T => { + const scrubbed = scrubEvent(event); + expect(scrubbed).not.toBeNull(); + return scrubbed as T; +}; +const fakeClassicAccessToken = (): string => `${"github" + "_pat_"}${"a".repeat(24)}`; +const fakeQueryTokenKey = (): string => "github" + "_token"; describe("scrubEvent — redact secrets before an event leaves the box", () => { it("redacts secret-keyed fields in headers/contexts/extra, recurses, and leaves safe fields", () => { - const ev = scrubEvent({ + const ev = scrubbedEvent({ request: { headers: { authorization: "Bearer abc", "x-trace": "ok" } }, contexts: { gittensory: { @@ -76,13 +83,183 @@ describe("scrubEvent — redact secrets before an event leaves the box", () => { it("is safe when headers/contexts/extra are absent (the !obj branch)", () => { expect(() => scrubEvent({})).not.toThrow(); + expect(scrubEvent({})).toEqual({}); }); it("stops at the depth guard without infinite recursion, still redacting shallow secrets", () => { let deep: any = { secretToken: "x" }; for (let i = 0; i < 8; i++) deep = { a: deep }; - const ev = scrubEvent({ extra: { token: "shallow", deep } }) as any; + let deepArray: any = { secretToken: "x" }; + for (let i = 0; i < 7; i++) deepArray = [deepArray]; + const ev = scrubbedEvent({ + extra: { token: "shallow", deep, deepArray }, + }) as any; + let deepCursor = ev.extra.deep; + for (let i = 0; i < 5; i++) deepCursor = deepCursor.a; + let arrayCursor = ev.extra.deepArray; + for (let i = 0; i < 5; i++) arrayCursor = arrayCursor[0]; expect(ev.extra.token).toBe("[redacted]"); + expect(deepCursor.a).toBe("[redacted]"); + expect(arrayCursor[0]).toBe("[redacted]"); + }); + + it("drops request bodies, denies unknown contexts, and scrubs PR/private payload fields (#1000)", () => { + const fakeToken = fakeClassicAccessToken(); + const ev = scrubbedEvent({ + request: { + headers: { authorization: `Bearer ${"a".repeat(16)}`, "x-trace": "ok" }, + data: { prompt: "review this diff" }, + body: "raw request body", + cookies: { session: "abc" }, + }, + contexts: { + gittensory: { + safeReason: "provider unavailable", + pullRequestTitle: "PR title with private rubric", + reviewText: "raw review body", + repoConfig: "private repo config", + nested: { apiKey: "provider secret" }, + }, + mystery: { repoConfig: "should not leave" }, + runtime: { name: "node" }, + }, + extra: { + diff: "@@ raw diff", + note: `wallet raw score /home/alice/project ${fakeToken}`, + attempts: 2, + nil: null, + values: ["hotkey", { apiKey: "nested" }, 3, null], + }, + tags: { repo: "owner/repo", authToken: "token" }, + }) as any; + + expect(ev.request.data).toBeUndefined(); + expect(ev.request.body).toBeUndefined(); + expect(ev.request.cookies).toBeUndefined(); + expect(ev.request.headers.authorization).toBe("[redacted]"); + expect(ev.request.headers["x-trace"]).toBe("ok"); + expect(ev.contexts.mystery).toBeUndefined(); + expect(ev.contexts.runtime.name).toBe("node"); + expect(ev.contexts.gittensory.pullRequestTitle).toBe("[redacted]"); + expect(ev.contexts.gittensory.reviewText).toBe("[redacted]"); + expect(ev.contexts.gittensory.repoConfig).toBe("[redacted]"); + expect(ev.contexts.gittensory.nested.apiKey).toBe("[redacted]"); + expect(ev.extra.diff).toBe("[redacted]"); + expect(ev.extra.note).not.toContain(fakeToken); + expect(ev.extra.note).not.toMatch(/wallet|raw score|\/home\/alice/i); + expect(ev.extra.note).toContain(""); + expect(ev.extra.attempts).toBe(2); + expect(ev.extra.nil).toBeNull(); + expect(ev.extra.values).toEqual([ + "private context", + { apiKey: "[redacted]" }, + 3, + null, + ]); + expect(ev.tags.repo).toBe("owner/repo"); + expect(ev.tags.authToken).toBe("[redacted]"); + }); + + it("scrubs request URL/query fields and deletes top-level user data", () => { + const queryTokenKey = fakeQueryTokenKey(); + const ev = scrubbedEvent({ + request: { + url: `https://self.host/review?${queryTokenKey}=abc123&repo=owner%2Frepo`, + query_string: `${queryTokenKey}=abc123&path=/home/alice/project&safe=ok`, + query: { [queryTokenKey]: "abc123", safe: "ok" }, + }, + user: { id: "123", email: "person@example.com" }, + }) as any; + + const url = new URL(ev.request.url); + const query = new URLSearchParams(ev.request.query_string); + expect(url.searchParams.get(queryTokenKey)).toBe("[redacted]"); + expect(url.searchParams.get("repo")).toBe("owner/repo"); + expect(query.get(queryTokenKey)).toBe("[redacted]"); + expect(query.get("path")).toBe(""); + expect(query.get("safe")).toBe("ok"); + expect(ev.request.query[queryTokenKey]).toBe("[redacted]"); + expect(ev.request.query.safe).toBe("ok"); + expect(ev.user).toBeUndefined(); + }); + + it("scrubs breadcrumbs, exception metadata, messages, and transaction names", () => { + const ev = scrubbedEvent({ + message: "gate prompt leaked with Bearer abcdefghijklmnop", + transaction: "review /Users/alice/private", + breadcrumbs: [ + { + message: "prompt mentions hotkey", + data: { responseBody: "raw provider body", safe: "kept" }, + }, + ], + exception: { + values: [ + { + value: "codex failed with eyJaaaaaaaa.bbbbbbbb.cccccccc", + stacktrace: { + frames: [ + { + filename: "/tmp/repo/file.ts", + vars: { token: "abc", safe: "value" }, + }, + ], + }, + }, + ], + }, + }) as any; + + expect(ev.message).not.toMatch(/gate prompt|Bearer abc/i); + expect(ev.transaction).toContain(""); + expect(ev.breadcrumbs[0].message).not.toMatch(/hotkey/i); + expect(ev.breadcrumbs[0].data.responseBody).toBe("[redacted]"); + expect(ev.breadcrumbs[0].data.safe).toBe("kept"); + expect(ev.exception.values[0].value).not.toMatch(/eyJaaaaaaaa/i); + expect(ev.exception.values[0].stacktrace.frames[0].filename).toContain(""); + expect(ev.exception.values[0].stacktrace.frames[0].vars.token).toBe("[redacted]"); + expect(ev.exception.values[0].stacktrace.frames[0].vars.safe).toBe("value"); + }); + + it("scrubs transaction span descriptions and data before sending transaction events", () => { + const queryTokenKey = fakeQueryTokenKey(); + const ev = scrubbedEvent({ + spans: [ + { + description: `GET /hooks?${queryTokenKey}=abc123&safe=ok`, + data: { + callbackUrl: `https://self.host/callback?${queryTokenKey}=abc123&safe=ok`, + relativeUrl: `/callback?${queryTokenKey}=abc123&safe=ok`, + noQueryUrl: "https://self.host/callback", + query_string: `${queryTokenKey}=abc123&path=/home/alice/project`, + prompt: "raw prompt", + }, + }, + ], + }) as any; + + const callbackUrl = new URL(ev.spans[0].data.callbackUrl); + expect(ev.spans[0].description).not.toContain("abc123"); + expect(callbackUrl.searchParams.get(queryTokenKey)).toBe("[redacted]"); + expect(callbackUrl.searchParams.get("safe")).toBe("ok"); + expect(ev.spans[0].data.relativeUrl).toContain( + `${queryTokenKey}=%5Bredacted%5D`, + ); + expect(ev.spans[0].data.noQueryUrl).toBe("https://self.host/callback"); + expect(new URLSearchParams(ev.spans[0].data.query_string).get("path")).toBe( + "", + ); + expect(ev.spans[0].data.prompt).toBe("[redacted]"); + }); + + it("drops the event when scrubbing itself fails instead of sending it unscrubbed", () => { + const event = { + get request() { + throw new Error("getter failed"); + }, + }; + + expect(scrubEvent(event)).toBeNull(); }); }); @@ -137,6 +314,11 @@ describe("enabled when SENTRY_DSN is set", () => { expect( opts.beforeSend({ extra: { sessionToken: "s" } }).extra.sessionToken, ).toBe("[redacted]"); + expect( + opts.beforeSendTransaction({ + contexts: { unknown: { token: "s" }, trace: { op: "job" } }, + }).contexts, + ).toEqual({ trace: { op: "job" } }); }); it("honors explicit env (?? left-hand branches)", async () => {