From aacbf5a909bbfb99672f828c5038f03a36bdd33b Mon Sep 17 00:00:00 2001 From: kkdev92 <112151103+kkdev92@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:15:54 +0900 Subject: [PATCH 1/2] test: fail when allowScripts drifts from the lockfile npm 11 skips a dependency's install script unless allowScripts names it, and reports the skip as a warning. An approval pinned to a version stops matching as soon as the lockfile moves that package, and a new dependency with an install script matches nothing, so both cases leave every job green. The new suite checks that each pinned approval matches the version the lockfile installs, that every entry names a package the lockfile installs with an install script, and that every install script CI would run is approved or denied. Packages the lockfile limits to other operating systems are left out: they never install on Linux, where CI runs. Against the manifest and lockfile of 1bf2da4, where the esbuild pin had fallen behind, the pin check fails on esbuild@0.28.1 while the lockfile installs 0.28.2. A moved-back pin, a removed entry and an entry for a package that is not installed each fail exactly one case. Co-Authored-By: Claude Opus 5.5 --- tests/allow-scripts.test.ts | 97 +++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 tests/allow-scripts.test.ts diff --git a/tests/allow-scripts.test.ts b/tests/allow-scripts.test.ts new file mode 100644 index 0000000..ae22a38 --- /dev/null +++ b/tests/allow-scripts.test.ts @@ -0,0 +1,97 @@ +import { readFileSync } from 'node:fs'; + +import { describe, expect, it } from 'vitest'; + +/** + * `allowScripts` in the manifest, checked against the lockfile. + * + * npm 11 runs a dependency's install script only when `allowScripts` has a + * matching entry, and skips every other one with a warning rather than an error. + * An approval pinned to `pkg@1.2.3` stops matching the day the lockfile moves the + * package, and a newly added dependency with an install script matches nothing, + * so in both cases the script is skipped and every job stays green. These cases + * turn that into a failure here, with the fix in the message. + * + * Reviewing an install script means approving it (`true`) or denying it + * (`false`); either counts. Only packages that install on Linux, where CI runs, + * need a review: an entry the lockfile limits to other operating systems never + * installs there. + */ + +interface LockEntry { + readonly version?: string; + readonly hasInstallScript?: boolean; + readonly os?: readonly string[]; +} + +const manifest = JSON.parse(readFileSync('package.json', 'utf8')) as { + allowScripts?: Record; +}; + +const lock = JSON.parse(readFileSync('package-lock.json', 'utf8')) as { + packages: Record; +}; + +const MARKER = 'node_modules/'; + +/** `@scope/name@1.2.3` names a version; `@scope/name` alone covers every version. */ +function parseKey(key: string): { name: string; version: string | undefined } { + const at = key.lastIndexOf('@'); + return at > 0 + ? { name: key.slice(0, at), version: key.slice(at + 1) } + : { name: key, version: undefined }; +} + +function installsOnLinux(os: readonly string[] | undefined): boolean { + if (os === undefined || os.length === 0) return true; + if (os.includes('!linux')) return false; + const named = os.filter((entry) => !entry.startsWith('!')); + return named.length === 0 || named.includes('linux'); +} + +const keys = Object.keys(manifest.allowScripts ?? {}).map(parseKey); + +const withInstallScript = Object.entries(lock.packages) + .filter(([path, entry]) => path.includes(MARKER) && entry.hasInstallScript === true) + .map(([path, entry]) => ({ + name: path.slice(path.lastIndexOf(MARKER) + MARKER.length), + version: entry.version ?? '', + onLinux: installsOnLinux(entry.os), + })); + +describe('allowScripts against the lockfile', () => { + it('finds install scripts in the lockfile to check', () => { + // Without this, a lockfile the parsing below misreads would pass every case. + expect(withInstallScript.length).toBeGreaterThan(0); + }); + + it('pins each approval to the version the lockfile installs', () => { + const stale = keys.flatMap(({ name, version }) => + version === undefined + ? [] + : withInstallScript + .filter((pkg) => pkg.name === name && pkg.version !== version) + .map( + (pkg) => + `${name}@${version} is pinned but the lockfile installs ${pkg.version}; ` + + `run \`npm install-scripts approve ${name}\`` + ) + ); + expect(stale).toEqual([]); + }); + + it('names only packages the lockfile installs with an install script', () => { + const unused = keys + .filter(({ name }) => !withInstallScript.some((pkg) => pkg.name === name)) + .map(({ name, version }) => (version === undefined ? name : `${name}@${version}`)); + expect(unused).toEqual([]); + }); + + it('reviews every install script CI would run', () => { + const reviewed = new Set(keys.map(({ name }) => name)); + const unreviewed = withInstallScript + .filter((pkg) => pkg.onLinux && !reviewed.has(pkg.name)) + .map((pkg) => `${pkg.name}@${pkg.version} has an install script allowScripts does not name`); + expect(unreviewed).toEqual([]); + }); +}); From 20b82d89469679dc66d719e491880410434f51f5 Mon Sep 17 00:00:00 2001 From: kkdev92 <112151103+kkdev92@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:16:09 +0900 Subject: [PATCH 2/2] chore: deny esbuild's install script esbuild's platform binary arrives as an optional dependency. Its install script checks that binary and fetches one only when the optional dependency is missing, and the fixtures use the JavaScript API, which resolves the binary from that dependency at run time. Both fixtures build with the script never having run. A name-only denial also cannot fall behind the lockfile the way a pinned approval does. That leaves one pinned approval, @playwright/browser-chromium, whose script downloads the browser the web extension host lane drives. The //allowScripts note now says so. Co-Authored-By: Claude Opus 5.5 --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index ba5954b..50af61b 100644 --- a/package.json +++ b/package.json @@ -183,9 +183,9 @@ "lint-staged" ] }, - "//allowScripts": "The host contract lanes need these two install scripts: esbuild fetches its platform binary, and @playwright/browser-chromium downloads the browser @vscode/test-web drives. Both are the package's own postinstall fetching its own artifact.", + "//allowScripts": "One install script runs: @playwright/browser-chromium downloads the browser @vscode/test-web drives, and its approval is pinned to the version the lockfile installs. esbuild's is denied: its platform binary arrives as an optional dependency, and the script only fetches one when that dependency is missing. tests/allow-scripts.test.ts fails when the lockfile moves past a pin or brings in an install script that is neither approved nor denied.", "allowScripts": { - "esbuild@0.28.2": true, + "esbuild": false, "@playwright/browser-chromium@1.63.0": true } }