From d82791b029e8417ef08da9ab9840ef39b02abc94 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Fri, 26 Jun 2026 14:14:57 -0700 Subject: [PATCH] fix(enrichment): harden install script package rendering --- .../src/analyzers/install-scripts.ts | 17 ++++-- review-enrichment/src/render.ts | 10 +++- review-enrichment/test/enrichment.test.ts | 52 ++++++++++++++++++- 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/review-enrichment/src/analyzers/install-scripts.ts b/review-enrichment/src/analyzers/install-scripts.ts index dcc3d7b37b..6937aab462 100644 --- a/review-enrichment/src/analyzers/install-scripts.ts +++ b/review-enrichment/src/analyzers/install-scripts.ts @@ -7,6 +7,14 @@ import type { EnrichRequest, InstallScriptFinding } from "../types.js"; import { extractDependencyChanges } from "./dependency-scan.js"; const INSTALL_HOOKS = ["preinstall", "install", "postinstall"]; +const NPM_PACKAGE_RE = + /^(?:@[a-z0-9][a-z0-9._-]*\/[a-z0-9][a-z0-9._-]*|[a-z0-9][a-z0-9._-]*)$/; +const SEMVER_RE = + /^(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/; + +function isSafeNpmChange(name: string, version: string): boolean { + return NPM_PACKAGE_RE.test(name) && SEMVER_RE.test(version); +} /** Analyzer entrypoint: changed npm deps → registry packument → only the versions that run install scripts. */ export async function scanInstallScripts( @@ -15,10 +23,13 @@ export async function scanInstallScripts( ): 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. + if ( + change.ecosystem !== "npm" || + !isSafeNpmChange(change.package, change.to) + ) + continue; const response = await fetchImpl( - `https://registry.npmjs.org/${change.package.replace("/", "%2F")}`, + `https://registry.npmjs.org/${encodeURIComponent(change.package)}`, ); if (!response.ok) continue; const data = (await response.json()) as { diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 993f36f085..716bfd3b6f 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -10,6 +10,14 @@ const SEVERITY_RANK: Record = { unknown: 4, }; +function promptText(value: string): string { + return value + .replace(/[\u0000-\u001f\u007f]/g, " ") + .replace(/\\/g, "\\\\") + .replace(/`/g, "\\`") + .replace(/([*_{}[\]()#+.!|-])/g, "\\$1"); +} + /** Build the `promptSection` (verbatim splice) + a one-line `systemSuffix` from the findings. Empty when nothing found. */ export function renderBrief( findings: BriefFindings, @@ -67,7 +75,7 @@ export function renderBrief( ? ` (published ${dep.publishedAt.slice(0, 10)})` : ""; lines.push( - `- \`${dep.package}@${dep.version}\` runs ${dep.hooks.join("/")} on install${when}`, + `- \`${promptText(dep.package)}@${promptText(dep.version)}\` runs ${promptText(dep.hooks.join("/"))} on install${when}`, ); } } diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index 966a25ef6b..ba4cbed788 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -402,6 +402,56 @@ test("scanInstallScripts: flags npm deps with install hooks, skips clean + non-n assert.equal(fail.length, 0); }); +test("scanInstallScripts: validates npm names and encodes the full registry path", async () => { + const calls: string[] = []; + const fetchImpl = async (url) => { + calls.push(String(url)); + return { + ok: true, + json: async () => ({ + versions: { "1.0.0": { scripts: { install: "x" } } }, + }), + }; + }; + const findings = await scanInstallScripts( + { + repoFullName: "o/r", + prNumber: 1, + files: [ + { + path: "package.json", + patch: [ + '+ "@scope/pkg": "1.0.0",', + '+ "core-js#` **inject** `": "1.0.0",', + '+ "bad-version": "1.0.0 || 2.0.0",', + ].join("\n"), + }, + ], + }, + fetchImpl, + ); + assert.deepEqual(calls, ["https://registry.npmjs.org/%40scope%2Fpkg"]); + assert.equal(findings.length, 1); + assert.equal(findings[0].package, "@scope/pkg"); +}); + +test("renderBrief: escapes install-script markdown and control characters", () => { + const r = renderBrief({ + installScript: [ + { + package: "core-js` **inject**\nnext", + version: "1.0.0", + hooks: ["postinstall"], + publishedAt: null, + }, + ], + }); + assert.ok( + r.promptSection.includes("core\\-js\\` \\*\\*inject\\*\\* next@1\\.0\\.0"), + ); + assert.doesNotMatch(r.promptSection, /core-js` \*\*inject\*\*/); +}); + test("renderBrief: renders the install-script block", () => { const r = renderBrief({ installScript: [ @@ -416,7 +466,7 @@ test("renderBrief: renders the install-script block", () => { 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\)/, + /`evil@1\\.0\\.0` runs preinstall\/postinstall on install \(published 2026-06-01\)/, ); });