diff --git a/review-enrichment/README.md b/review-enrichment/README.md index 9a9cd0bc9d..c3b704c475 100644 --- a/review-enrichment/README.md +++ b/review-enrichment/README.md @@ -78,6 +78,11 @@ Do **not** pass `SENTRY_AUTH_TOKEN` as a Docker build arg. Railway deploys this can leak through image metadata. Keeping the upload at runtime means Sentry sees the same `dist/` files that the service executes, without exposing source maps over HTTP. +Analyzer failures are still fail-open: the `/v1/enrich` response marks the analyzer as `degraded` and returns a partial +brief. When Sentry is enabled, those degradations are captured as `rees_analyzer_degraded` events with tags for +`analyzer`, `repo`, `pullNumber`, `headSha`, `release`, `environment`, and `timeoutMs`. Use those tags to spot a broken +analyzer without exposing request bodies, diffs, tokens, or review content. + If Sentry still shows frames such as `/app/dist/server.js`, check: 1. The event's `release` is `gittensory-rees@` or your exact `SENTRY_RELEASE` override. diff --git a/review-enrichment/src/brief.ts b/review-enrichment/src/brief.ts index db4b211c28..7550f7db89 100644 --- a/review-enrichment/src/brief.ts +++ b/review-enrichment/src/brief.ts @@ -21,8 +21,10 @@ import { scanSecretLog } from "./analyzers/secret-log.js"; import { scanAssetWeight } from "./analyzers/asset-weight.js"; import { scanTyposquat } from "./analyzers/typosquat.js"; import { renderBrief } from "./render.js"; +import { captureAnalyzerDegradation } from "./sentry.js"; type AnalyzerFn = (req: EnrichRequest, signal: AbortSignal) => Promise; +type AnalyzerRegistry = Partial>; // The analyzer registry. More land behind this same shape: license (#1475), secret (#1476), static (#1477), history (#1478). const ANALYZERS: Record = { @@ -64,9 +66,12 @@ function runWithTimeout( }); } -export async function buildBrief(req: EnrichRequest): Promise { +export async function buildBrief( + req: EnrichRequest, + analyzers: AnalyzerRegistry = ANALYZERS, +): Promise { const start = Date.now(); - const all = Object.keys(ANALYZERS) as Array; + const all = Object.keys(analyzers) as Array; const requested = req.analyzers?.length ? all.filter((name) => req.analyzers!.includes(name)) : all; @@ -79,15 +84,24 @@ export async function buildBrief(req: EnrichRequest): Promise { await Promise.all( requested.map(async (name) => { try { + const analyzer = analyzers[name]; + if (!analyzer) throw new Error("analyzer_unregistered"); const result = await runWithTimeout( - (signal) => ANALYZERS[name](req, signal), + (signal) => analyzer(req, signal), budgetMs, ); findings[name] = result as never; analyzerStatus[name] = "ok"; - } catch { + } catch (error) { analyzerStatus[name] = "degraded"; partial = true; + captureAnalyzerDegradation(error, { + analyzer: name, + repoFullName: req.repoFullName, + prNumber: req.prNumber, + headSha: req.headSha, + timeoutMs: budgetMs, + }); } }), ); diff --git a/review-enrichment/src/sentry.ts b/review-enrichment/src/sentry.ts index 9730d20d2a..44006e530c 100644 --- a/review-enrichment/src/sentry.ts +++ b/review-enrichment/src/sentry.ts @@ -1,9 +1,12 @@ import type { ErrorEvent, EventHint } from "@sentry/node"; type SentryNs = typeof import("@sentry/node"); +type SentryClient = Pick; -let Sentry: SentryNs | undefined; +let Sentry: SentryClient | undefined; let active = false; +let activeRelease: string | undefined; +let activeEnvironment = "production"; const SECRET_FIELD = /(?:authorization|cookie|token|secret|password|private[_-]?key|shared[_-]?secret)/i; const SECRET_VALUE = /\b(?:github_pat_[A-Za-z0-9_]+|gh[pousr]_[A-Za-z0-9_]+|gts_[a-f0-9]{64}|eyJ[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+)\b/g; @@ -50,6 +53,14 @@ function scrubValue(value: unknown): unknown { return value; } +function sentryTagValue(value: string | number | undefined): string | undefined { + if (value === undefined) return undefined; + const scrubbed = scrubValue(String(value)); + if (typeof scrubbed !== "string") return undefined; + const text = nonBlank(scrubbed); + return text ? text.slice(0, 200) : undefined; +} + function scrubEvent(event: ErrorEvent): ErrorEvent { return scrubValue(event) as ErrorEvent; } @@ -58,10 +69,12 @@ export async function initSentry(env: NodeJS.ProcessEnv): Promise { if (!nonBlank(env.SENTRY_DSN)) return false; try { Sentry = await import("@sentry/node"); + activeRelease = resolveReesSentryRelease(env); + activeEnvironment = resolveSentryEnvironment(env); Sentry.init({ dsn: env.SENTRY_DSN, - environment: resolveSentryEnvironment(env), - release: resolveReesSentryRelease(env), + environment: activeEnvironment, + release: activeRelease, tracesSampleRate: resolveTracesSampleRate(env), beforeSend: (event: ErrorEvent, _hint: EventHint) => scrubEvent(event), }); @@ -70,6 +83,8 @@ export async function initSentry(env: NodeJS.ProcessEnv): Promise { } catch (error) { active = false; Sentry = undefined; + activeRelease = undefined; + activeEnvironment = "production"; warn("rees_sentry_init_failed", { message: error instanceof Error ? error.message : String(error) }); return false; } @@ -83,7 +98,64 @@ export function captureError(error: unknown, context?: Record): }); } +export interface AnalyzerDegradationContext { + analyzer: string; + repoFullName: string; + prNumber: number; + headSha?: string; + timeoutMs?: number; +} + +export function captureAnalyzerDegradation(error: unknown, context: AnalyzerDegradationContext): void { + if (!active || !Sentry) return; + const safeContext = { + event: "rees_analyzer_degraded", + analyzer: context.analyzer, + repoFullName: context.repoFullName, + prNumber: context.prNumber, + headSha: nonBlank(context.headSha), + timeoutMs: context.timeoutMs, + release: activeRelease, + environment: activeEnvironment, + }; + Sentry.withScope((scope) => { + const analyzerTag = sentryTagValue(context.analyzer) ?? "unknown"; + const headShaTag = sentryTagValue(safeContext.headSha); + const timeoutTag = sentryTagValue(context.timeoutMs); + const releaseTag = sentryTagValue(activeRelease); + scope.setLevel("error"); + scope.setContext("rees_analyzer", scrubValue(safeContext) as Record); + scope.setFingerprint(["rees-analyzer-degraded", analyzerTag]); + scope.setTag("event", "rees_analyzer_degraded"); + scope.setTag("analyzer", analyzerTag); + scope.setTag("repo", sentryTagValue(context.repoFullName) ?? "unknown"); + scope.setTag("pullNumber", sentryTagValue(context.prNumber) ?? "unknown"); + if (headShaTag) scope.setTag("headSha", headShaTag); + if (timeoutTag) scope.setTag("timeoutMs", timeoutTag); + if (releaseTag) scope.setTag("release", releaseTag); + scope.setTag("environment", sentryTagValue(activeEnvironment) ?? "production"); + Sentry!.captureException(error instanceof Error ? error : new Error(String(error))); + }); +} + export async function flushSentry(timeoutMs = 2000): Promise { if (!active || !Sentry) return; await Sentry.flush(timeoutMs).catch(() => undefined); } + +export function resetSentryForTest(): void { + Sentry = undefined; + active = false; + activeRelease = undefined; + activeEnvironment = "production"; +} + +export function setSentryForTest( + sentry: Pick, + options: { release?: string; environment?: string } = {}, +): void { + Sentry = sentry as SentryClient; + active = true; + activeRelease = options.release; + activeEnvironment = options.environment ?? "production"; +} diff --git a/review-enrichment/test/sentry-degradation.test.ts b/review-enrichment/test/sentry-degradation.test.ts new file mode 100644 index 0000000000..c20d133c14 --- /dev/null +++ b/review-enrichment/test/sentry-degradation.test.ts @@ -0,0 +1,149 @@ +import assert from "node:assert/strict"; +import test, { afterEach } from "node:test"; + +import { buildBrief } from "../dist/brief.js"; +import { + captureAnalyzerDegradation, + resetSentryForTest, + setSentryForTest, +} from "../dist/sentry.js"; + +function sentryHarness() { + const tags: Record = {}; + const contexts: Record = {}; + const fingerprints: unknown[][] = []; + const levels: string[] = []; + const captured: Error[] = []; + const scope = { + setLevel: (level: string) => levels.push(level), + setContext: (name: string, context: unknown) => { + contexts[name] = context; + }, + setFingerprint: (fingerprint: unknown[]) => fingerprints.push(fingerprint), + setTag: (name: string, value: string) => { + tags[name] = value; + }, + }; + setSentryForTest( + { + withScope: (run: (value: typeof scope) => void) => run(scope), + captureException: (error: unknown) => { + captured.push(error instanceof Error ? error : new Error(String(error))); + return "event-id"; + }, + flush: async () => true, + }, + { release: "gittensory-rees@test", environment: "test" }, + ); + return { tags, contexts, fingerprints, levels, captured }; +} + +afterEach(() => { + resetSentryForTest(); +}); + +test("captureAnalyzerDegradation is inert when Sentry is disabled", () => { + assert.doesNotThrow(() => + captureAnalyzerDegradation(new Error("boom"), { + analyzer: "dependency", + repoFullName: "JSONbored/gittensory", + prNumber: 7, + headSha: "abc123", + timeoutMs: 8000, + }), + ); +}); + +test("captureAnalyzerDegradation tags and fingerprints sanitized analyzer failures", () => { + const sentry = sentryHarness(); + const fakeGithubPat = ["github", "pat", "should_never_be_attached"].join("_"); + const fakeGhp = ["ghp", "should_never_be_attached"].join("_"); + + captureAnalyzerDegradation(new Error("registry timeout"), { + analyzer: "dependency", + repoFullName: "JSONbored/gittensory", + prNumber: 7, + headSha: "abc123", + timeoutMs: 8000, + diff: fakeGithubPat, + githubToken: fakeGhp, + authorization: "Bearer should_never_be_attached", + } as never); + + assert.deepEqual(sentry.levels, ["error"]); + assert.deepEqual(sentry.fingerprints, [["rees-analyzer-degraded", "dependency"]]); + assert.equal(sentry.tags.event, "rees_analyzer_degraded"); + assert.equal(sentry.tags.analyzer, "dependency"); + assert.equal(sentry.tags.repo, "JSONbored/gittensory"); + assert.equal(sentry.tags.pullNumber, "7"); + assert.equal(sentry.tags.headSha, "abc123"); + assert.equal(sentry.tags.timeoutMs, "8000"); + assert.equal(sentry.tags.release, "gittensory-rees@test"); + assert.equal(sentry.tags.environment, "test"); + assert.equal(sentry.captured[0].message, "registry timeout"); + + const analyzerContext = sentry.contexts.rees_analyzer as Record; + assert.deepEqual(analyzerContext, { + event: "rees_analyzer_degraded", + analyzer: "dependency", + repoFullName: "JSONbored/gittensory", + prNumber: 7, + headSha: "abc123", + timeoutMs: 8000, + release: "gittensory-rees@test", + environment: "test", + }); + const serializedContext = JSON.stringify(analyzerContext); + assert.equal(serializedContext.includes(fakeGithubPat), false); + assert.equal(serializedContext.includes(fakeGhp), false); + assert.equal(serializedContext.includes("Bearer should_never_be_attached"), false); +}); + +test("captureAnalyzerDegradation filters tag values before sending them", () => { + const sentry = sentryHarness(); + const secretLikeValue = ["ghp", "abcdefghijklmnopqrstuvwxyz1234567890"].join("_"); + + captureAnalyzerDegradation(new Error("registry timeout"), { + analyzer: secretLikeValue, + repoFullName: `JSONbored/${secretLikeValue}`, + prNumber: 7, + headSha: secretLikeValue, + timeoutMs: 8000, + }); + + assert.deepEqual(sentry.fingerprints, [["rees-analyzer-degraded", "[Filtered]"]]); + assert.equal(sentry.tags.analyzer, "[Filtered]"); + assert.equal(sentry.tags.repo, "JSONbored/[Filtered]"); + assert.equal(sentry.tags.headSha, "[Filtered]"); +}); + +test("buildBrief stays fail-open and captures a degraded analyzer", async () => { + const sentry = sentryHarness(); + + const brief = await buildBrief( + { + repoFullName: "JSONbored/gittensory", + prNumber: 42, + headSha: "head-sha", + budget: { timeoutMs: 50 }, + }, + { + dependency: async () => { + throw new Error("osv unavailable"); + }, + }, + ); + + assert.equal(brief.partial, true); + assert.equal(brief.analyzerStatus.dependency, "degraded"); + assert.deepEqual(brief.findings, {}); + assert.equal(brief.repoFullName, "JSONbored/gittensory"); + assert.equal(brief.prNumber, 42); + assert.equal(sentry.captured.length, 1); + assert.equal(sentry.captured[0].message, "osv unavailable"); + assert.equal(sentry.tags.analyzer, "dependency"); + assert.equal(sentry.tags.repo, "JSONbored/gittensory"); + assert.equal(sentry.tags.pullNumber, "42"); + assert.equal(sentry.tags.headSha, "head-sha"); + assert.equal(sentry.tags.timeoutMs, "50"); +});