diff --git a/review-enrichment/src/analyzers/install-scripts.ts b/review-enrichment/src/analyzers/install-scripts.ts new file mode 100644 index 0000000000..dcc3d7b37b --- /dev/null +++ b/review-enrichment/src/analyzers/install-scripts.ts @@ -0,0 +1,42 @@ +// Install-script & lifecycle-hook auditor (brainstorm #2). For each npm dependency a PR adds/upgrades, fetches the +// registry packument and flags ones that ship preinstall/install/postinstall scripts — the #1 npm-malware execution +// vector (a script runs on `npm install`, before any code review of the package's source). The shipped CVE scan +// misses this entirely; the no-checkout reviewer can't fetch a packument. Public-safe output: package@version + the +// hook names + publish date (NOT the script body, to keep the brief compact and non-executable). +import type { EnrichRequest, InstallScriptFinding } from "../types.js"; +import { extractDependencyChanges } from "./dependency-scan.js"; + +const INSTALL_HOOKS = ["preinstall", "install", "postinstall"]; + +/** Analyzer entrypoint: changed npm deps → registry packument → only the versions that run install scripts. */ +export async function scanInstallScripts( + req: EnrichRequest, + fetchImpl: typeof fetch = fetch, +): Promise { + const findings: InstallScriptFinding[] = []; + for (const change of extractDependencyChanges(req.files ?? [])) { + if (change.ecosystem !== "npm") continue; + // Scoped packages (@scope/name) encode only the slash in the registry path; the @ stays literal. + const response = await fetchImpl( + `https://registry.npmjs.org/${change.package.replace("/", "%2F")}`, + ); + if (!response.ok) continue; + const data = (await response.json()) as { + versions?: Record }>; + time?: Record; + }; + const scripts = data.versions?.[change.to]?.scripts ?? {}; + const hooks = INSTALL_HOOKS.filter( + (hook) => typeof scripts[hook] === "string", + ); + if (hooks.length) { + findings.push({ + package: change.package, + version: change.to, + hooks, + publishedAt: data.time?.[change.to] ?? null, + }); + } + } + return findings; +} diff --git a/review-enrichment/src/brief.ts b/review-enrichment/src/brief.ts index 52cb0275d5..ec39f2bd7e 100644 --- a/review-enrichment/src/brief.ts +++ b/review-enrichment/src/brief.ts @@ -10,6 +10,7 @@ import type { import { scanDependencies } from "./analyzers/dependency-scan.js"; import { scanSecrets } from "./analyzers/secret-scan.js"; import { scanLicenses } from "./analyzers/license-check.js"; +import { scanInstallScripts } from "./analyzers/install-scripts.js"; import { renderBrief } from "./render.js"; type AnalyzerFn = (req: EnrichRequest) => Promise; @@ -19,6 +20,7 @@ const ANALYZERS: Record = { dependency: (req) => scanDependencies(req), secret: (req) => scanSecrets(req), license: (req) => scanLicenses(req), + installScript: (req) => scanInstallScripts(req), }; function withTimeout(promise: Promise, ms: number): Promise { diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index ca1ba1dccd..90682a6ca0 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -57,6 +57,21 @@ export function renderBrief( } } + const installScripts = findings.installScript ?? []; + if (installScripts.length) { + lines.push( + "### Dependency install scripts (supply-chain risk — review before merging)", + ); + for (const dep of installScripts) { + const when = dep.publishedAt + ? ` (published ${dep.publishedAt.slice(0, 10)})` + : ""; + lines.push( + `- \`${dep.package}@${dep.version}\` runs ${dep.hooks.join("/")} on install${when}`, + ); + } + } + if (!lines.length) return { promptSection: "", systemSuffix: "" }; const header = diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 6656bb30ab..9cad867382 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -59,11 +59,20 @@ export interface LicenseFinding { classification: "copyleft" | "unknown"; } +/** A newly-added/upgraded npm dependency version that runs install lifecycle scripts (supply-chain risk). */ +export interface InstallScriptFinding { + package: string; + version: string; + hooks: string[]; + publishedAt: string | null; +} + /** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */ export interface BriefFindings { dependency?: DependencyFinding[]; secret?: SecretFinding[]; license?: LicenseFinding[]; + installScript?: InstallScriptFinding[]; } export type AnalyzerStatus = "ok" | "degraded" | "skipped"; diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index e15094faae..29587e951a 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -9,6 +9,14 @@ import { renderBrief } from "../dist/render.js"; import { buildBrief } from "../dist/brief.js"; import { scanPatch, scanSecrets } from "../dist/analyzers/secret-scan.js"; import { scanLicenses } from "../dist/analyzers/license-check.js"; +import { scanInstallScripts } from "../dist/analyzers/install-scripts.js"; + +const npmFetch = + (scripts, time = {}) => + async () => ({ + ok: true, + json: async () => ({ versions: { "1.0.0": { scripts } }, time }), + }); const licFetch = (licenses, ok = true) => @@ -317,3 +325,79 @@ test("buildBrief: license analyzer runs alongside the others", async () => { globalThis.fetch = realFetch; } }); + +test("scanInstallScripts: flags npm deps with install hooks, skips clean + non-npm + non-ok", async () => { + const flagged = await scanInstallScripts( + pkgPatch("evil"), + npmFetch( + { postinstall: "node steal.js" }, + { "1.0.0": "2026-01-01T00:00:00Z" }, + ), + ); + assert.equal(flagged.length, 1); + assert.deepEqual(flagged[0].hooks, ["postinstall"]); + assert.equal(flagged[0].publishedAt, "2026-01-01T00:00:00Z"); + const clean = await scanInstallScripts( + pkgPatch("good"), + npmFetch({ test: "jest" }), + ); + assert.equal(clean.length, 0); + const py = await scanInstallScripts( + { + repoFullName: "o/r", + prNumber: 1, + files: [{ path: "requirements.txt", patch: "+evil==1.0.0" }], + }, + npmFetch({ postinstall: "x" }), + ); + assert.equal(py.length, 0); + const fail = await scanInstallScripts(pkgPatch("x"), async () => ({ + ok: false, + json: async () => ({}), + })); + assert.equal(fail.length, 0); +}); + +test("renderBrief: renders the install-script block", () => { + const r = renderBrief({ + installScript: [ + { + package: "evil", + version: "1.0.0", + hooks: ["preinstall", "postinstall"], + publishedAt: "2026-06-01T00:00:00Z", + }, + ], + }); + assert.match(r.promptSection, /install scripts \(supply-chain risk/); + assert.match( + r.promptSection, + /`evil@1.0.0` runs preinstall\/postinstall on install \(published 2026-06-01\)/, + ); +}); + +test("buildBrief: install-script analyzer runs alongside the others", async () => { + const realFetch = globalThis.fetch; + globalThis.fetch = async (url) => { + const u = String(url); + if (u.includes("registry.npmjs.org")) + return { + ok: true, + json: async () => ({ + versions: { "1.0.0": { scripts: { postinstall: "x" } } }, + time: {}, + }), + }; + if (u.includes("deps.dev")) + return { ok: true, json: async () => ({ licenses: ["MIT"] }) }; + return { ok: true, json: async () => ({ vulns: [] }) }; + }; + try { + const brief = await buildBrief(pkgPatch("evil")); + assert.equal(brief.analyzerStatus.installScript, "ok"); + assert.equal(brief.findings.installScript.length, 1); + assert.match(brief.promptSection, /supply-chain risk/); + } finally { + globalThis.fetch = realFetch; + } +});