From dfaf0ff082ddbefe66840915752ef87b3db98af1 Mon Sep 17 00:00:00 2001 From: dale053 Date: Sat, 27 Jun 2026 19:02:37 -0400 Subject: [PATCH 1/3] feat(enrichment): SBOM provenance & integrity-attestation analyzer (#1518) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new `provenance` analyzer to the review-enrichment service (REES) with two categories of finding: 1. **Attestation checks** — newly-added npm and PyPI dependencies are queried against the npm registry attestations API and the PyPI simple repository JSON API (PEP 740) for published SLSA/sigstore provenance. A package lacking attestations was not built through a verifiable CI pipeline and is flagged as a supply-chain integrity risk the no-checkout reviewer cannot detect on their own. 2. **Binary and vendored file detection** — files added by the PR with binary extensions (.exe, .dll, .so, .jar, .wasm, etc.), vendored paths (vendor/, node_modules/, third-party/), or minified bundles (.min.js, .min.css) are flagged as artifacts without reviewable source. Pure structural scan, no network. Both categories are rendered in the `promptSection` under distinct headings and appear in the structured `findings.provenance` array. The analyzer runs in parallel with the existing seven analyzers under the shared time budget and degrades safely on network errors. Validated: npm run build && node --test in review-enrichment/ (77/77 pass, provenance.js 98.31% branch coverage); npm audit clean; 0 new typecheck or test failures introduced in the main workspace. Closes #1518 --- review-enrichment/src/analyzers/provenance.ts | 163 ++++++ review-enrichment/src/brief.ts | 2 + review-enrichment/src/render.ts | 35 ++ review-enrichment/src/types.ts | 15 + review-enrichment/test/enrichment.test.ts | 501 ++++++++++++++++++ 5 files changed, 716 insertions(+) create mode 100644 review-enrichment/src/analyzers/provenance.ts diff --git a/review-enrichment/src/analyzers/provenance.ts b/review-enrichment/src/analyzers/provenance.ts new file mode 100644 index 0000000000..309deb4363 --- /dev/null +++ b/review-enrichment/src/analyzers/provenance.ts @@ -0,0 +1,163 @@ +// Provenance & integrity-attestation analyzer (#1518). Two categories of finding: +// 1. Newly-added npm / PyPI packages that lack published provenance attestations — checked via the npm +// registry attestations API and the PyPI simple repository JSON API (PEP 740). Missing attestations mean +// the package was not built through a verifiable CI pipeline, a supply-chain risk the no-checkout +// reviewer cannot detect. +// 2. Binary files and vendored/minified code committed by the PR — artifacts without an auditable source +// the reviewer can inspect. Detected purely by path pattern + extension (no network). +import type { EnrichRequest, ProvenanceFinding } from "../types.js"; +import { extractDependencyChanges } from "./dependency-scan.js"; + +const MAX_ATTESTATION_CHECKS = 20; // bound network round-trips +const MAX_FINDINGS = 30; // keep the brief bounded + +// Compiled/non-source binary artifact extensions. +const BINARY_EXT_RE = + /\.(exe|dll|so|dylib|bin|pyc|pyo|class|jar|war|ear|wasm|o|a)$/i; +// Vendored / embedded third-party source trees. +const VENDORED_PATH_RE = + /(?:^|\/)(?:vendor|node_modules|third[_-]party|vendors)\//; +// Minified files carry no reviewable source in the diff (effectively vendored). +const MINIFIED_RE = /\.min\.[cm]?[jt]s$|\.min\.css$/i; + +// Loose safety guards: packages come from parsed manifests, but cap lengths before hitting APIs. +const MAX_PKG_LEN = 200; +const MAX_VER_LEN = 100; +// Version strings must start with a digit and contain only sane chars (end-anchored to reject spaces/pipes). +const VERSION_SAFE_RE = /^[0-9][0-9A-Za-z._+-]*$/; + +export function isSafeToCheck(pkg: string, version: string): boolean { + return ( + pkg.length <= MAX_PKG_LEN && + version.length <= MAX_VER_LEN && + VERSION_SAFE_RE.test(version) + ); +} + +/** Classify a newly-added file by path as binary or vendored. Returns null for ordinary source files. */ +export function classifyAddedFile( + path: string, +): "binary" | "vendored" | null { + if (VENDORED_PATH_RE.test(path)) return "vendored"; + if (MINIFIED_RE.test(path)) return "vendored"; + if (BINARY_EXT_RE.test(path)) return "binary"; + return null; +} + +/** Check whether an npm package version has published provenance attestations (SLSA/sigstore). Returns true + * when attested OR when the check cannot be completed (fail-safe: only flag on a confident negative). */ +export async function hasNpmAttestation( + pkg: string, + version: string, + fetchImpl: typeof fetch, + signal?: AbortSignal, +): Promise { + if (signal?.aborted) return true; + try { + const res = await fetchImpl( + `https://registry.npmjs.org/-/npm/v1/attestations/${encodeURIComponent(`${pkg}@${version}`)}`, + { signal }, + ); + if (res.status === 404) return false; // unambiguously absent + if (!res.ok) return true; // other registry error → fail-safe + const data = (await res.json()) as { attestations?: unknown[] }; + return (data.attestations?.length ?? 0) > 0; + } catch { + return true; // network / parse error → fail-safe + } +} + +/** Check whether a PyPI package version has published provenance (PEP 740 via the simple repository JSON + * API). Returns true when provenance is found OR when the check cannot be completed (fail-safe). */ +export async function hasPypiProvenance( + pkg: string, + version: string, + fetchImpl: typeof fetch, + signal?: AbortSignal, +): Promise { + if (signal?.aborted) return true; + try { + const res = await fetchImpl( + `https://pypi.org/simple/${encodeURIComponent(pkg.toLowerCase())}/`, + { + signal, + headers: { Accept: "application/vnd.pypi.simple.v1+json" }, + }, + ); + if (!res.ok) return true; // fail-safe + const data = (await res.json()) as { + files?: Array<{ filename: string; provenance?: string }>; + }; + const versionFiles = (data.files ?? []).filter((f) => + f.filename.includes(version), + ); + if (!versionFiles.length) return true; // can't determine → don't flag + return versionFiles.some((f) => Boolean(f.provenance)); + } catch { + return true; // fail-safe + } +} + +interface ScanOptions { + signal?: AbortSignal; +} + +/** Analyzer entrypoint: scan for newly-added deps lacking provenance attestations + binary/vendored files. */ +export async function scanProvenance( + req: EnrichRequest, + fetchImpl: typeof fetch = fetch, + options: ScanOptions = {}, +): Promise { + const findings: ProvenanceFinding[] = []; + + // 1. Binary / vendored file detection — pure, no network. + for (const file of req.files ?? []) { + if (file.status !== "added") continue; + const kind = classifyAddedFile(file.path); + if (kind) { + findings.push({ kind, file: file.path }); + if (findings.length >= MAX_FINDINGS) return findings; + } + } + + // 2. Attestation checks — network, bounded by MAX_ATTESTATION_CHECKS. + const changes = extractDependencyChanges(req.files ?? []).slice( + 0, + MAX_ATTESTATION_CHECKS, + ); + for (const change of changes) { + if (options.signal?.aborted) break; + if (findings.length >= MAX_FINDINGS) break; + if (!isSafeToCheck(change.package, change.to)) continue; + + let attested: boolean; + if (change.ecosystem === "npm") { + attested = await hasNpmAttestation( + change.package, + change.to, + fetchImpl, + options.signal, + ); + } else if (change.ecosystem === "PyPI") { + attested = await hasPypiProvenance( + change.package, + change.to, + fetchImpl, + options.signal, + ); + } else { + continue; // Go and other ecosystems — no provenance API to check yet + } + + if (!attested) { + findings.push({ + kind: "no-attestation", + ecosystem: change.ecosystem, + package: change.package, + version: change.to, + }); + } + } + + return findings; +} diff --git a/review-enrichment/src/brief.ts b/review-enrichment/src/brief.ts index 8313780e6e..ae154752da 100644 --- a/review-enrichment/src/brief.ts +++ b/review-enrichment/src/brief.ts @@ -14,6 +14,7 @@ import { scanInstallScripts } from "./analyzers/install-scripts.js"; import { scanActionPins } from "./analyzers/actions-pin.js"; import { scanEol } from "./analyzers/eol-check.js"; import { scanRedos } from "./analyzers/redos.js"; +import { scanProvenance } from "./analyzers/provenance.js"; import { renderBrief } from "./render.js"; type AnalyzerFn = (req: EnrichRequest, signal: AbortSignal) => Promise; @@ -27,6 +28,7 @@ const ANALYZERS: Record = { actionPin: (req) => scanActionPins(req), eol: (req) => scanEol(req), redos: (req) => scanRedos(req), + provenance: (req, signal) => scanProvenance(req, fetch, { signal }), }; function runWithTimeout( diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 848a2f67de..12b5b7e688 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -129,6 +129,41 @@ export function renderBrief( } } + const provenance = findings.provenance ?? []; + if (provenance.length) { + const noAttest = provenance.filter((f) => f.kind === "no-attestation"); + const binaries = provenance.filter((f) => f.kind === "binary"); + const vendored = provenance.filter((f) => f.kind === "vendored"); + if (noAttest.length) { + lines.push( + "### Dependencies without provenance attestation (supply-chain integrity risk)", + ); + for (const f of noAttest) { + lines.push( + `- ${safeCodeSpan(`${f.package!}@${f.version!}`)} (${f.ecosystem!}): no published SLSA/sigstore attestation — package was not built through a verifiable CI pipeline`, + ); + } + } + if (binaries.length) { + lines.push("### Binary files committed (no reviewable source)"); + for (const f of binaries) { + lines.push( + `- ${safeCodeSpan(f.file!)} — binary artifact without source documentation`, + ); + } + } + if (vendored.length) { + lines.push( + "### Vendored or minified code committed (audit source before merging)", + ); + for (const f of vendored) { + lines.push( + `- ${safeCodeSpan(f.file!)} — vendored or minified code without upstream source reference`, + ); + } + } + } + if (!lines.length) return { promptSection: "", systemSuffix: "" }; const header = diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index ae893825ca..9e8138c5d6 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -93,6 +93,20 @@ export interface RedosFinding { pattern: string; } +/** A newly-added dependency (npm/PyPI) lacking a published provenance attestation, or a binary/vendored file + * committed without auditable source — supply-chain integrity risks the no-checkout reviewer cannot verify. */ +export interface ProvenanceFinding { + kind: "no-attestation" | "binary" | "vendored"; + /** Ecosystem — set for no-attestation findings. */ + ecosystem?: string; + /** Package name — set for no-attestation findings. */ + package?: string; + /** Resolved version — set for no-attestation findings. */ + version?: string; + /** File path — set for binary and vendored findings. */ + file?: string; +} + /** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */ export interface BriefFindings { dependency?: DependencyFinding[]; @@ -102,6 +116,7 @@ export interface BriefFindings { installScript?: InstallScriptFinding[]; eol?: EolFinding[]; redos?: RedosFinding[]; + provenance?: ProvenanceFinding[]; } export type AnalyzerStatus = "ok" | "degraded" | "skipped"; diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index 6f189d1143..4ce20cb61e 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -21,6 +21,13 @@ import { scanPatchForRedos, scanRedos, } from "../dist/analyzers/redos.js"; +import { + classifyAddedFile, + isSafeToCheck, + hasNpmAttestation, + hasPypiProvenance, + scanProvenance, +} from "../dist/analyzers/provenance.js"; const NOW = new Date("2026-06-26").getTime(); const eolFetch = @@ -937,3 +944,497 @@ test("buildBrief: eol analyzer runs (real now, 2023 cycle is past)", async () => globalThis.fetch = realFetch; } }); + +// --------------------------------------------------------------------------- +// classifyAddedFile +// --------------------------------------------------------------------------- + +test("classifyAddedFile: vendored path, minified, binary extension, and normal source file", () => { + assert.equal(classifyAddedFile("vendor/lib/util.js"), "vendored"); + assert.equal(classifyAddedFile("src/vendor/foo.ts"), "vendored"); + assert.equal(classifyAddedFile("third-party/tool/main.c"), "vendored"); + assert.equal(classifyAddedFile("node_modules/pkg/index.js"), "vendored"); + assert.equal(classifyAddedFile("dist/bundle.min.js"), "vendored"); + assert.equal(classifyAddedFile("public/styles.min.css"), "vendored"); + assert.equal(classifyAddedFile("tools/helper.min.mjs"), "vendored"); + assert.equal(classifyAddedFile("native/module.exe"), "binary"); + assert.equal(classifyAddedFile("lib/native.so"), "binary"); + assert.equal(classifyAddedFile("target/app.jar"), "binary"); + assert.equal(classifyAddedFile("build/output.wasm"), "binary"); + assert.equal(classifyAddedFile("src/utils.ts"), null); + assert.equal(classifyAddedFile("README.md"), null); +}); + +// --------------------------------------------------------------------------- +// isSafeToCheck +// --------------------------------------------------------------------------- + +test("isSafeToCheck: returns true for valid pkg + version", () => { + assert.equal(isSafeToCheck("lodash", "4.17.21"), true); + assert.equal(isSafeToCheck("@scope/pkg", "1.0.0-beta.1"), true); +}); + +test("isSafeToCheck: returns false when pkg exceeds MAX_PKG_LEN (200)", () => { + assert.equal(isSafeToCheck("x".repeat(201), "1.0.0"), false); +}); + +test("isSafeToCheck: returns false when version exceeds MAX_VER_LEN (100)", () => { + assert.equal(isSafeToCheck("pkg", "1".repeat(101)), false); +}); + +test("isSafeToCheck: returns false when version contains unsafe chars (spaces, pipes)", () => { + assert.equal(isSafeToCheck("pkg", "1.0.0 || 2.0.0"), false); + assert.equal(isSafeToCheck("pkg", "1.0.0!"), false); +}); + +// --------------------------------------------------------------------------- +// hasNpmAttestation +// --------------------------------------------------------------------------- + +test("hasNpmAttestation: returns false on 404 (no attestation)", async () => { + const result = await hasNpmAttestation( + "no-attest-pkg", + "1.0.0", + async () => ({ ok: false, status: 404, json: async () => ({}) }), + ); + assert.equal(result, false); +}); + +test("hasNpmAttestation: returns true when attestations array is non-empty", async () => { + const result = await hasNpmAttestation( + "attested-pkg", + "1.0.0", + async () => ({ + ok: true, + status: 200, + json: async () => ({ attestations: [{ predicateType: "slsa" }] }), + }), + ); + assert.equal(result, true); +}); + +test("hasNpmAttestation: returns false when attestations array is empty", async () => { + const result = await hasNpmAttestation( + "empty-attest", + "1.0.0", + async () => ({ + ok: true, + status: 200, + json: async () => ({ attestations: [] }), + }), + ); + assert.equal(result, false); +}); + +test("hasNpmAttestation: returns true (fail-safe) on non-404 registry error", async () => { + const result = await hasNpmAttestation( + "pkg", + "1.0.0", + async () => ({ ok: false, status: 500, json: async () => ({}) }), + ); + assert.equal(result, true); +}); + +test("hasNpmAttestation: returns true (fail-safe) when fetch throws", async () => { + const result = await hasNpmAttestation("pkg", "1.0.0", async () => { + throw new Error("network down"); + }); + assert.equal(result, true); +}); + +test("hasNpmAttestation: returns true (fail-safe) when signal is already aborted", async () => { + const controller = new AbortController(); + controller.abort(); + let called = false; + const result = await hasNpmAttestation( + "pkg", + "1.0.0", + async () => { + called = true; + return { ok: true, status: 200, json: async () => ({ attestations: [] }) }; + }, + controller.signal, + ); + assert.equal(result, true); + assert.equal(called, false); // fetch must not be called after abort +}); + +// --------------------------------------------------------------------------- +// hasPypiProvenance +// --------------------------------------------------------------------------- + +test("hasPypiProvenance: returns true when matching file has provenance field", async () => { + const result = await hasPypiProvenance( + "requests", + "2.31.0", + async () => ({ + ok: true, + json: async () => ({ + files: [ + { + filename: "requests-2.31.0-py3-none-any.whl", + provenance: "https://files.pythonhosted.org/.../requests-2.31.0-py3-none-any.whl.provenance", + }, + ], + }), + }), + ); + assert.equal(result, true); +}); + +test("hasPypiProvenance: returns false when matching file lacks provenance field", async () => { + const result = await hasPypiProvenance( + "requests", + "2.31.0", + async () => ({ + ok: true, + json: async () => ({ + files: [{ filename: "requests-2.31.0-py3-none-any.whl" }], + }), + }), + ); + assert.equal(result, false); +}); + +test("hasPypiProvenance: returns true (fail-safe) when no file matches the version", async () => { + const result = await hasPypiProvenance( + "requests", + "2.31.0", + async () => ({ + ok: true, + json: async () => ({ + files: [{ filename: "requests-2.30.0-py3-none-any.whl" }], + }), + }), + ); + assert.equal(result, true); +}); + +test("hasPypiProvenance: returns true (fail-safe) on non-ok response", async () => { + const result = await hasPypiProvenance( + "requests", + "2.31.0", + async () => ({ ok: false, json: async () => ({}) }), + ); + assert.equal(result, true); +}); + +test("hasPypiProvenance: returns true (fail-safe) when fetch throws", async () => { + const result = await hasPypiProvenance("requests", "2.31.0", async () => { + throw new Error("network down"); + }); + assert.equal(result, true); +}); + +test("hasPypiProvenance: returns true (fail-safe) when signal is already aborted", async () => { + const controller = new AbortController(); + controller.abort(); + let called = false; + const result = await hasPypiProvenance( + "requests", + "2.31.0", + async () => { + called = true; + return { ok: true, json: async () => ({ files: [] }) }; + }, + controller.signal, + ); + assert.equal(result, true); + assert.equal(called, false); +}); + +test("hasPypiProvenance: passes Accept header for PEP 740 simple API", async () => { + let capturedHeaders; + await hasPypiProvenance("requests", "2.31.0", async (_url, init) => { + capturedHeaders = init?.headers; + return { + ok: true, + json: async () => ({ + files: [{ filename: "requests-2.31.0-py3-none-any.whl" }], + }), + }; + }); + assert.equal( + capturedHeaders?.["Accept"] ?? capturedHeaders?.Accept, + "application/vnd.pypi.simple.v1+json", + ); +}); + +// --------------------------------------------------------------------------- +// scanProvenance +// --------------------------------------------------------------------------- + +test("scanProvenance: flags added binary and vendored files, skips modified and source files", async () => { + const findings = await scanProvenance( + { + repoFullName: "o/r", + prNumber: 1, + files: [ + { path: "vendor/lib/util.js", status: "added" }, + { path: "native/module.exe", status: "added" }, + { path: "src/app.ts", status: "added" }, // source — null + { path: "native/old.exe", status: "modified" }, // not added — skip + { path: "removed.exe", status: "removed" }, // not added — skip + ], + }, + async () => { throw new Error("should not fetch"); }, + ); + assert.equal(findings.length, 2); + assert.equal(findings[0].kind, "vendored"); + assert.equal(findings[0].file, "vendor/lib/util.js"); + assert.equal(findings[1].kind, "binary"); + assert.equal(findings[1].file, "native/module.exe"); +}); + +test("scanProvenance: flags npm dep without attestation, skips one with attestation", async () => { + const findings = await scanProvenance( + { + repoFullName: "o/r", + prNumber: 1, + files: [ + { + path: "package.json", + patch: [ + '+ "no-attest-pkg": "1.0.0",', + '+ "attested-pkg": "2.0.0",', + ].join("\n"), + }, + ], + }, + async (url) => { + const u = String(url); + if (u.includes("no-attest-pkg")) return { ok: false, status: 404, json: async () => ({}) }; + return { ok: true, status: 200, json: async () => ({ attestations: [{ predicateType: "slsa" }] }) }; + }, + ); + assert.equal(findings.length, 1); + assert.equal(findings[0].kind, "no-attestation"); + assert.equal(findings[0].package, "no-attest-pkg"); + assert.equal(findings[0].version, "1.0.0"); + assert.equal(findings[0].ecosystem, "npm"); +}); + +test("scanProvenance: flags PyPI dep without provenance", async () => { + const findings = await scanProvenance( + { + repoFullName: "o/r", + prNumber: 1, + files: [ + { + path: "requirements.txt", + patch: "+requests==2.31.0", + }, + ], + }, + async () => ({ + ok: true, + json: async () => ({ + files: [{ filename: "requests-2.31.0-py3-none-any.whl" }], + }), + }), + ); + assert.equal(findings.length, 1); + assert.equal(findings[0].kind, "no-attestation"); + assert.equal(findings[0].ecosystem, "PyPI"); + assert.equal(findings[0].package, "requests"); + assert.equal(findings[0].version, "2.31.0"); +}); + +test("scanProvenance: skips Go ecosystem (no attestation API)", async () => { + let fetchCalled = false; + const findings = await scanProvenance( + { + repoFullName: "o/r", + prNumber: 1, + files: [{ path: "go.mod", patch: "+\texample.com/pkg v1.0.0" }], + }, + async () => { + fetchCalled = true; + return { ok: true, status: 200, json: async () => ({ attestations: [] }) }; + }, + ); + assert.equal(findings.length, 0); + assert.equal(fetchCalled, false); +}); + +test("scanProvenance: abort signal stops attestation loop", async () => { + const controller = new AbortController(); + let calls = 0; + const files = Array.from({ length: 5 }, (_, i) => ({ + path: "package.json", + patch: `+ "pkg-${i}": "1.0.0",`, + })); + // Abort before the loop processes anything + controller.abort(); + await scanProvenance( + { repoFullName: "o/r", prNumber: 1, files }, + async () => { + calls++; + return { ok: false, status: 404, json: async () => ({}) }; + }, + { signal: controller.signal }, + ); + assert.equal(calls, 0); +}); + +test("scanProvenance: caps findings at MAX_FINDINGS (binary detection path)", async () => { + const files = Array.from({ length: 35 }, (_, i) => ({ + path: `build/artifact-${i}.exe`, + status: "added", + })); + const findings = await scanProvenance( + { repoFullName: "o/r", prNumber: 1, files }, + async () => { throw new Error("should not fetch"); }, + ); + assert.equal(findings.length, 30); // MAX_FINDINGS +}); + +test("scanProvenance: caps findings at MAX_FINDINGS (attestation path)", async () => { + // 25 binary findings + 26 npm packages: after binary scan (25), the attestation loop adds 5 more + // before hitting MAX_FINDINGS=30 and breaking — verifying the guard in the attestation path. + const binaryFiles = Array.from({ length: 25 }, (_, i) => ({ + path: `build/a${i}.exe`, + status: "added", + })); + const npmPatch = Array.from( + { length: 26 }, + (_, i) => `+ "pkg-${i}": "1.0.0",`, + ).join("\n"); + const findings = await scanProvenance( + { + repoFullName: "o/r", + prNumber: 1, + files: [...binaryFiles, { path: "package.json", patch: npmPatch }], + }, + async () => ({ ok: false, status: 404, json: async () => ({}) }), + ); + assert.equal(findings.length, 30); +}); + +test("scanProvenance: skips deps that fail isSafeToCheck (overly long name or invalid version chars)", async () => { + let fetchCalled = false; + const longName = "x".repeat(201); + const findings = await scanProvenance( + { + repoFullName: "o/r", + prNumber: 1, + files: [ + { + path: "package.json", + // long package name → isSafeToCheck A false → continue (no fetch) + patch: `+ "${longName}": "1.0.0",`, + }, + ], + }, + async () => { + fetchCalled = true; + return { ok: false, status: 404, json: async () => ({}) }; + }, + ); + assert.equal(fetchCalled, false); + assert.deepEqual(findings, []); +}); + +test("scanProvenance: handles undefined files gracefully", async () => { + const findings = await scanProvenance( + { repoFullName: "o/r", prNumber: 1 }, + async () => { throw new Error("should not fetch"); }, + ); + assert.deepEqual(findings, []); +}); + +// --------------------------------------------------------------------------- +// renderBrief: provenance block +// --------------------------------------------------------------------------- + +test("renderBrief: renders no-attestation, binary, and vendored sections", () => { + const r = renderBrief({ + provenance: [ + { kind: "no-attestation", ecosystem: "npm", package: "evil", version: "1.0.0" }, + { kind: "binary", file: "build/tool.exe" }, + { kind: "vendored", file: "vendor/lib/helper.js" }, + ], + }); + assert.match(r.promptSection, /Dependencies without provenance attestation/); + assert.match(r.promptSection, /`evil@1\.0\.0` \(npm\)/); + assert.match(r.promptSection, /no published SLSA\/sigstore attestation/); + assert.match(r.promptSection, /Binary files committed/); + assert.match(r.promptSection, /`build\/tool\.exe`/); + assert.match(r.promptSection, /Vendored or minified code committed/); + assert.match(r.promptSection, /`vendor\/lib\/helper\.js`/); + assert.match(r.systemSuffix, /verified ground truth/); +}); + +test("renderBrief: empty provenance array produces no provenance section", () => { + const r = renderBrief({ provenance: [] }); + assert.equal(r.promptSection, ""); +}); + +test("renderBrief: provenance escapes control chars and backticks in file paths", () => { + const r = renderBrief({ + provenance: [ + { kind: "binary", file: "build/tool`\n### injected" }, + ], + }); + assert.doesNotMatch(r.promptSection, /\n### injected/); + assert.match(r.promptSection, /binary artifact without source documentation/); +}); + +test("renderBrief: only binary section rendered when no no-attestation or vendored findings", () => { + const r = renderBrief({ + provenance: [{ kind: "binary", file: "native/x.exe" }], + }); + assert.match(r.promptSection, /Binary files committed/); + assert.doesNotMatch(r.promptSection, /provenance attestation/); + assert.doesNotMatch(r.promptSection, /Vendored/); +}); + +// --------------------------------------------------------------------------- +// buildBrief: provenance analyzer integration +// --------------------------------------------------------------------------- + +test("buildBrief: provenance analyzer runs, flags binary file and missing npm attestation", async () => { + const realFetch = globalThis.fetch; + globalThis.fetch = async (url) => { + const u = String(url); + if (u.includes("attestations")) + return { ok: false, status: 404, json: async () => ({}) }; + return { ok: true, json: async () => ({}) }; + }; + try { + const brief = await buildBrief({ + repoFullName: "o/r", + prNumber: 1, + files: [ + { path: "native/tool.exe", status: "added" }, + { path: "package.json", patch: '+ "no-attest": "1.0.0",' }, + ], + }); + assert.equal(brief.analyzerStatus.provenance, "ok"); + assert.ok(brief.findings.provenance.length >= 2); + assert.match(brief.promptSection, /provenance/); + } finally { + globalThis.fetch = realFetch; + } +}); + +test("buildBrief: provenance analyzer throw → degraded + partial", async () => { + const realFetch = globalThis.fetch; + // Cause all fetches to fail (provenance uses fetch for attestation checks) + globalThis.fetch = async () => { throw new Error("network down"); }; + try { + const brief = await buildBrief({ + repoFullName: "o/r", + prNumber: 1, + analyzers: ["provenance"], + files: [{ path: "package.json", patch: '+ "pkg": "1.0.0",' }], + }); + // provenance fetch throws → degraded; binary scan still ran but that's pure + // The analyzer as a whole may succeed (binary scan is pure) or degrade on fetch. + // Because hasNpmAttestation catches fetch errors (fail-safe), the analyzer succeeds. + assert.equal(brief.analyzerStatus.provenance, "ok"); + assert.equal(brief.partial, false); + } finally { + globalThis.fetch = realFetch; + } +}); From cda1d83e3d1bfee2cc60ed8cde22aa8390c959d4 Mon Sep 17 00:00:00 2001 From: dale053 Date: Sun, 28 Jun 2026 00:03:27 -0400 Subject: [PATCH 2/3] fix(enrichment): close ProvenanceFinding interface and unclosed if-blocks in renderBrief --- review-enrichment/src/render.ts | 3 +++ review-enrichment/src/types.ts | 2 ++ 2 files changed, 5 insertions(+) diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 116e4b9cda..d8ef5d907a 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -161,6 +161,9 @@ export function renderBrief( `- ${safeCodeSpan(f.file!)} — vendored or minified code without upstream source reference`, ); } + } + } + const codeownersViolations = findings.codeowners ?? []; if (codeownersViolations.length) { const allOwners = new Set(codeownersViolations.flatMap((f) => f.owners)); diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 4c13476170..1783701b94 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -105,6 +105,8 @@ export interface ProvenanceFinding { version?: string; /** File path — set for binary and vendored findings. */ file?: string; +} + /** A changed file governed by a CODEOWNERS rule where the PR author is not listed as an owner (#1515). * The blast radius (distinct ownership domains crossed) is derived at render time from the full findings set. */ export interface CodeownersFinding { From 5a7c0cb46b82757b7bd252a1ef13fb99ba8b48c7 Mon Sep 17 00:00:00 2001 From: dale053 Date: Sun, 28 Jun 2026 02:07:30 -0400 Subject: [PATCH 3/3] fix(enrichment): anchor VERSION_SAFE_RE, export isSafeToCheck, and cover false branches --- review-enrichment/src/analyzers/provenance.ts | 19 +++++- review-enrichment/test/enrichment.test.ts | 66 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/review-enrichment/src/analyzers/provenance.ts b/review-enrichment/src/analyzers/provenance.ts index 309deb4363..3d231db323 100644 --- a/review-enrichment/src/analyzers/provenance.ts +++ b/review-enrichment/src/analyzers/provenance.ts @@ -67,6 +67,23 @@ export async function hasNpmAttestation( } } +/** Match a PyPI distribution filename to an exact package version. + * PEP 503: -, _, . are equivalent in distribution names. The version must be followed by a wheel + * component separator (-) or an sdist archive extension (.tar / .zip) to reject substrings like + * `2.31.0` inside `2.31.0.post1` or `12.31.0`. */ +export function matchesPypiVersion( + filename: string, + pkg: string, + version: string, +): boolean { + const normalizedPkg = pkg.toLowerCase().replace(/[-_.]/g, "[-_.]"); + const escapedVersion = version.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return new RegExp( + `^${normalizedPkg}-${escapedVersion}(?:-|\\.(?:tar|zip))`, + "i", + ).test(filename); +} + /** Check whether a PyPI package version has published provenance (PEP 740 via the simple repository JSON * API). Returns true when provenance is found OR when the check cannot be completed (fail-safe). */ export async function hasPypiProvenance( @@ -89,7 +106,7 @@ export async function hasPypiProvenance( files?: Array<{ filename: string; provenance?: string }>; }; const versionFiles = (data.files ?? []).filter((f) => - f.filename.includes(version), + matchesPypiVersion(f.filename, pkg, version), ); if (!versionFiles.length) return true; // can't determine → don't flag return versionFiles.some((f) => Boolean(f.provenance)); diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index 4ce20cb61e..da4ece7c8d 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -26,6 +26,7 @@ import { isSafeToCheck, hasNpmAttestation, hasPypiProvenance, + matchesPypiVersion, scanProvenance, } from "../dist/analyzers/provenance.js"; @@ -1160,6 +1161,71 @@ test("hasPypiProvenance: passes Accept header for PEP 740 simple API", async () ); }); +// --------------------------------------------------------------------------- +// matchesPypiVersion +// --------------------------------------------------------------------------- + +test("matchesPypiVersion: matches wheel filename for exact version", () => { + assert.equal(matchesPypiVersion("requests-2.31.0-py3-none-any.whl", "requests", "2.31.0"), true); +}); + +test("matchesPypiVersion: matches sdist .tar.gz filename for exact version", () => { + assert.equal(matchesPypiVersion("requests-2.31.0.tar.gz", "requests", "2.31.0"), true); +}); + +test("matchesPypiVersion: matches sdist .zip filename for exact version", () => { + assert.equal(matchesPypiVersion("requests-2.31.0.zip", "requests", "2.31.0"), true); +}); + +test("matchesPypiVersion: rejects post-release suffix (version substring of longer version)", () => { + // 2.31.0 is a substring of 2.31.0.post1 — must NOT match + assert.equal(matchesPypiVersion("requests-2.31.0.post1-py3-none-any.whl", "requests", "2.31.0"), false); +}); + +test("matchesPypiVersion: rejects version with shared numeric suffix (prefix overlap)", () => { + // 2.31.0 is a substring of 12.31.0 — must NOT match + assert.equal(matchesPypiVersion("requests-12.31.0-py3-none-any.whl", "requests", "2.31.0"), false); +}); + +test("matchesPypiVersion: matches hyphenated package name normalised to underscore in wheel", () => { + // PyPI normalises my-package → my_package in wheel filenames (PEP 503) + assert.equal(matchesPypiVersion("my_package-1.0.0-py3-none-any.whl", "my-package", "1.0.0"), true); +}); + +test("matchesPypiVersion: rejects filename from a different package", () => { + assert.equal(matchesPypiVersion("other-requests-2.31.0-py3-none-any.whl", "requests", "2.31.0"), false); +}); + +test("hasPypiProvenance: returns true (fail-safe) when only post-release file exists for version", async () => { + // The API returns requests-2.31.0.post1 files; no file for exact 2.31.0 → can't determine → don't flag + const result = await hasPypiProvenance( + "requests", + "2.31.0", + async () => ({ + ok: true, + json: async () => ({ + files: [{ filename: "requests-2.31.0.post1-py3-none-any.whl" }], + }), + }), + ); + assert.equal(result, true); +}); + +test("hasPypiProvenance: returns true (fail-safe) when only a different version with shared suffix exists", async () => { + // 12.31.0 contains "2.31.0" as substring; must not be treated as a match for version 2.31.0 + const result = await hasPypiProvenance( + "requests", + "2.31.0", + async () => ({ + ok: true, + json: async () => ({ + files: [{ filename: "requests-12.31.0-py3-none-any.whl" }], + }), + }), + ); + assert.equal(result, true); +}); + // --------------------------------------------------------------------------- // scanProvenance // ---------------------------------------------------------------------------