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
2 changes: 1 addition & 1 deletion review-enrichment/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"build": "tsc -p tsconfig.json",
"start": "node dist/server.js",
"dev": "node --experimental-strip-types --watch src/server.ts",
"test": "node --test --experimental-strip-types"
"test": "npm run build && node --test --experimental-strip-types \"test/**/*.test.ts\""
},
"dependencies": {
"@hono/node-server": "^1.13.7",
Expand Down
175 changes: 175 additions & 0 deletions review-enrichment/src/analyzers/dependency-scan.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
// Dependency-diff + OSV.dev CVE analyzer (#1474). Parses the changed manifests in the PR diff for added/upgraded
// dependencies, then queries OSV.dev (free, no key) for known vulnerabilities in the NEW versions. This is the
// heavy/external work the no-checkout `claude --print` reviewer cannot do (Bash/WebFetch disallowed, no CVE DB).
import type { EnrichRequest, DependencyFinding, Cve } from "../types.js";

interface DepChange {
ecosystem: string;
package: string;
from: string | null;
to: string;
}

// Per-manifest line parsers. Each returns [name, version] for a `+`/`-` diff line, or null. Heuristic (line-based,
// not a full manifest parse) — good enough to flag the deps a PR adds/bumps without resolving the whole tree.
const NPM_RE = /^"([^"]+)"\s*:\s*"([\^~>=<\s]*[0-9][^"]*)"/;
const PYPI_RE = /^([A-Za-z0-9._-]+)\s*==\s*([0-9][^\s;]*)/;
const GO_RE = /^([a-z0-9.\/-]+)\s+v([0-9][^\s]*)/;

function parseLine(
manifest: string,
body: string,
): { name: string; version: string } | null {
if (manifest === "package.json") {
const m = NPM_RE.exec(body);
if (m)
return { name: m[1]!, version: m[2]!.replace(/^[\^~>=<\s]+/, "").trim() };
} else if (manifest === "requirements.txt") {
const m = PYPI_RE.exec(body);
if (m) return { name: m[1]!, version: m[2]! };
} else if (manifest === "go.mod") {
const m = GO_RE.exec(body.replace(/^require\s+/, "").trim());
if (m) return { name: m[1]!, version: m[2]! };
}
return null;
}

const ECOSYSTEM: Record<string, string> = {
"package.json": "npm",
"requirements.txt": "PyPI",
"go.mod": "Go",
};

/** Extract added/changed (not removed) dependency versions from the changed manifests in the diff. Pure. */
export function extractDependencyChanges(
files: NonNullable<EnrichRequest["files"]>,
): DepChange[] {
const byKey = new Map<
string,
{ ecosystem: string; package: string; added?: string; removed?: string }
>();
for (const file of files) {
const manifest = file.path.split("/").pop() ?? file.path;
const ecosystem = ECOSYSTEM[manifest];
if (!ecosystem || !file.patch) continue;
for (const line of file.patch.split("\n")) {
const sign = line[0];
if (
(sign !== "+" && sign !== "-") ||
line.startsWith("+++") ||
line.startsWith("---")
)
continue;
const parsed = parseLine(manifest, line.slice(1).trim());
if (!parsed) continue;
const key = ecosystem + "::" + parsed.name;
const entry = byKey.get(key) ?? { ecosystem, package: parsed.name };
if (sign === "+") entry.added = parsed.version;
else entry.removed = parsed.version;
byKey.set(key, entry);
}
}
const changes: DepChange[] = [];
for (const entry of byKey.values()) {
// Only scan a version that's present after the change, and only when it actually changed.
if (!entry.added || entry.added === entry.removed) continue;
changes.push({
ecosystem: entry.ecosystem,
package: entry.package,
from: entry.removed ?? null,
to: entry.added,
});
}
return changes;
}

interface OsvVuln {
id: string;
summary?: string;
details?: string;
severity?: Array<{ type: string; score: string }>;
database_specific?: { severity?: string };
affected?: Array<{ ranges?: Array<{ events?: Array<{ fixed?: string }> }> }>;
}

function severityOf(vuln: OsvVuln): Cve["severity"] {
const label = vuln.database_specific?.severity?.toLowerCase();
if (
label === "critical" ||
label === "high" ||
label === "medium" ||
label === "low"
)
return label;
const score = Number(
vuln.severity?.find((s) => s.type?.startsWith("CVSS"))?.score,
);
if (!Number.isFinite(score)) return "unknown";
return score >= 9
? "critical"
: score >= 7
? "high"
: score >= 4
? "medium"
: "low";
}

function fixedOf(vuln: OsvVuln): string | null {
for (const affected of vuln.affected ?? []) {
for (const range of affected.ranges ?? []) {
for (const event of range.events ?? []) {
if (event.fixed) return event.fixed;
}
}
}
return null;
}

/** Query OSV.dev for vulnerabilities affecting a specific package version. Best-effort: returns [] on any error. */
export async function queryOsv(
ecosystem: string,
name: string,
version: string,
fetchImpl: typeof fetch = fetch,
): Promise<Cve[]> {
const response = await fetchImpl("https://api.osv.dev/v1/query", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify({ package: { name, ecosystem }, version }),
});
if (!response.ok) return [];
const data = (await response.json()) as { vulns?: OsvVuln[] };
return (data.vulns ?? []).map((vuln) => ({
id: vuln.id,
severity: severityOf(vuln),
summary: (vuln.summary ?? vuln.details ?? "")
.replace(/\s+/g, " ")
.slice(0, 180),
fixedIn: fixedOf(vuln),
}));
}

/** Analyzer entrypoint: changed deps → OSV → only the deps that carry vulnerabilities. */
export async function scanDependencies(
req: EnrichRequest,
fetchImpl: typeof fetch = fetch,
): Promise<DependencyFinding[]> {
const changes = extractDependencyChanges(req.files ?? []);
const findings: DependencyFinding[] = [];
for (const change of changes) {
const cves = await queryOsv(
change.ecosystem,
change.package,
change.to,
fetchImpl,
);
if (cves.length) {
findings.push({
...change,
direction: change.from ? "change" : "add",
cves,
});
}
}
return findings;
}
80 changes: 80 additions & 0 deletions review-enrichment/src/brief.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Orchestrator: fan out the enabled analyzers under a time budget, assemble the ReviewBrief, render the prompt
// block. Each analyzer is independent + best-effort — one that throws/times out marks the brief `partial` and the
// others still contribute, so the engine always gets a usable (possibly empty) brief and never blocks on us.
import type {
EnrichRequest,
ReviewBrief,
BriefFindings,
AnalyzerStatus,
} from "./types.js";
import { scanDependencies } from "./analyzers/dependency-scan.js";
import { renderBrief } from "./render.js";

type AnalyzerFn = (req: EnrichRequest) => Promise<unknown>;

// The analyzer registry. More land behind this same shape: license (#1475), secret (#1476), static (#1477), history (#1478).
const ANALYZERS: Record<keyof BriefFindings, AnalyzerFn> = {
dependency: (req) => scanDependencies(req),
};

function withTimeout<T>(promise: Promise<T>, ms: number): Promise<T> {
return new Promise((resolve, reject) => {
const timer = setTimeout(() => reject(new Error("analyzer_timeout")), ms);
promise.then(
(value) => {
clearTimeout(timer);
resolve(value);
},
(error) => {
clearTimeout(timer);
reject(error);
},
);
});
}

export async function buildBrief(req: EnrichRequest): Promise<ReviewBrief> {
const start = Date.now();
const all = Object.keys(ANALYZERS) as Array<keyof BriefFindings>;
const requested = req.analyzers?.length
? all.filter((name) => req.analyzers!.includes(name))
: all;
const budgetMs = req.budget?.timeoutMs ?? 8000;

const findings: BriefFindings = {};
const analyzerStatus: Record<string, AnalyzerStatus> = {};
let partial = false;

await Promise.all(
requested.map(async (name) => {
try {
const result = await withTimeout(ANALYZERS[name](req), budgetMs);
findings[name] = result as never;
analyzerStatus[name] = "ok";
} catch {
analyzerStatus[name] = "degraded";
partial = true;
}
}),
);
for (const name of all)
if (!requested.includes(name)) analyzerStatus[name] = "skipped";

const { promptSection, systemSuffix } = renderBrief(
findings,
req.budget?.maxBriefChars ?? 6000,
);
return {
schemaVersion: 1,
repoFullName: req.repoFullName,
prNumber: req.prNumber,
headSha: req.headSha ?? null,
generatedAtIso: new Date().toISOString(),
elapsedMs: Date.now() - start,
partial,
analyzerStatus,
findings,
promptSection,
systemSuffix,
};
}
48 changes: 48 additions & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Render structured findings into the public-safe prompt block the engine splices into the review. Kept separate
// so each analyzer's rendering is one function and the brief stays deterministic + cap-bounded.
import type { BriefFindings } from "./types.js";

const SEVERITY_RANK: Record<string, number> = {
critical: 0,
high: 1,
medium: 2,
low: 3,
unknown: 4,
};

/** Build the `promptSection` (verbatim splice) + a one-line `systemSuffix` from the findings. Empty when nothing found. */
export function renderBrief(
findings: BriefFindings,
maxChars = 6000,
): { promptSection: string; systemSuffix: string } {
const lines: string[] = [];

const deps = findings.dependency ?? [];
if (deps.length) {
lines.push("### Dependency vulnerabilities (OSV.dev)");
const flat = deps
.flatMap((dep) => dep.cves.map((cve) => ({ dep, cve })))
.sort(
(a, b) =>
(SEVERITY_RANK[a.cve.severity] ?? 4) -
(SEVERITY_RANK[b.cve.severity] ?? 4),
);
for (const { dep, cve } of flat) {
const fix = cve.fixedIn ? ` — fixed in ${cve.fixedIn}` : "";
lines.push(
`- \`${dep.package}@${dep.to}\` (${dep.ecosystem}): **${cve.severity}** ${cve.id} — ${cve.summary}${fix}`,
);
}
}

if (!lines.length) return { promptSection: "", systemSuffix: "" };

const header =
"## EXTERNAL REVIEW BRIEF (heavy/external analysis the in-prompt reviewer cannot run)";
let body = `${header}\n${lines.join("\n")}\n`;
if (body.length > maxChars)
body = body.slice(0, maxChars) + "\n…(brief truncated)\n";
const systemSuffix =
"When the EXTERNAL REVIEW BRIEF lists a CVE for a package+version, treat it as verified ground truth — do not re-derive it.";
return { promptSection: body, systemSuffix };
}
Loading