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
42 changes: 42 additions & 0 deletions review-enrichment/src/analyzers/install-scripts.ts
Original file line number Diff line number Diff line change
@@ -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<InstallScriptFinding[]> {
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<string, { scripts?: Record<string, string> }>;
time?: Record<string, string>;
};
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;
}
2 changes: 2 additions & 0 deletions review-enrichment/src/brief.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<unknown>;
Expand All @@ -19,6 +20,7 @@ const ANALYZERS: Record<keyof BriefFindings, AnalyzerFn> = {
dependency: (req) => scanDependencies(req),
secret: (req) => scanSecrets(req),
license: (req) => scanLicenses(req),
installScript: (req) => scanInstallScripts(req),
};

function withTimeout<T>(promise: Promise<T>, ms: number): Promise<T> {
Expand Down
15 changes: 15 additions & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
9 changes: 9 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
84 changes: 84 additions & 0 deletions review-enrichment/test/enrichment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down Expand Up @@ -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;
}
});