Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions review-enrichment/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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@<same Railway commit sha>` or your exact `SENTRY_RELEASE` override.
Expand Down
22 changes: 18 additions & 4 deletions review-enrichment/src/brief.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<unknown>;
type AnalyzerRegistry = Partial<Record<keyof BriefFindings, AnalyzerFn>>;

// The analyzer registry. More land behind this same shape: license (#1475), secret (#1476), static (#1477), history (#1478).
const ANALYZERS: Record<keyof BriefFindings, AnalyzerFn> = {
Expand Down Expand Up @@ -64,9 +66,12 @@ function runWithTimeout<T>(
});
}

export async function buildBrief(req: EnrichRequest): Promise<ReviewBrief> {
export async function buildBrief(
req: EnrichRequest,
analyzers: AnalyzerRegistry = ANALYZERS,
): Promise<ReviewBrief> {
const start = Date.now();
const all = Object.keys(ANALYZERS) as Array<keyof BriefFindings>;
const all = Object.keys(analyzers) as Array<keyof BriefFindings>;
const requested = req.analyzers?.length
? all.filter((name) => req.analyzers!.includes(name))
: all;
Expand All @@ -79,15 +84,24 @@ export async function buildBrief(req: EnrichRequest): Promise<ReviewBrief> {
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,
});
}
}),
);
Expand Down
78 changes: 75 additions & 3 deletions review-enrichment/src/sentry.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import type { ErrorEvent, EventHint } from "@sentry/node";

type SentryNs = typeof import("@sentry/node");
type SentryClient = Pick<SentryNs, "init" | "withScope" | "captureException" | "flush">;

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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -58,10 +69,12 @@ export async function initSentry(env: NodeJS.ProcessEnv): Promise<boolean> {
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),
});
Expand All @@ -70,6 +83,8 @@ export async function initSentry(env: NodeJS.ProcessEnv): Promise<boolean> {
} 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;
}
Expand All @@ -83,7 +98,64 @@ export function captureError(error: unknown, context?: Record<string, unknown>):
});
}

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<string, unknown>);
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<void> {
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<SentryClient, "withScope" | "captureException" | "flush">,
options: { release?: string; environment?: string } = {},
): void {
Sentry = sentry as SentryClient;
active = true;
activeRelease = options.release;
activeEnvironment = options.environment ?? "production";
}
149 changes: 149 additions & 0 deletions review-enrichment/test/sentry-degradation.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, string> = {};
const contexts: Record<string, unknown> = {};
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<string, unknown>;
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");
});