Stop dependency install scripts running in CI, and detect new ones - #7
Merged
Conversation
A transitive dependency executes arbitrary Node at install time, on every machine that installs this project and on every deployment build. Nothing in the toolchain looked at that. npm audit reports known advisories, not execution surface, and gitleaks reads content rather than lifecycle hooks. Two layers. CI now installs with --ignore-scripts, which removes the execution path rather than merely watching it. Verified that a clean --ignore-scripts install still passes lint, tests, palette and build. The only two install scripts in the tree are dev-only, and the native binding the live one prepares is installed by npm through optionalDependencies without the script needing to run at all. scripts/check-install-scripts.mjs then fails the build if any dependency gains an install script that is not on a reviewed allowlist. Each entry carries the reason it was allowed, so a future failure is read against a recorded standard rather than waved through. Production entries are called out separately, since those run wherever the app is built. The one currently in the tree is unrs-resolver, behind eslint-import-resolver-typescript, behind eslint-config-next. Its postinstall resolves the platform binary npm has already installed and exits, reaching the network only when that resolution fails. Confirmed a no-op here: running it changed nothing on disk. Verified both directions. The check passes on the current lockfile, and fails with a non-zero exit when install scripts are injected into it.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #6. Closes a supply chain gap that review did not look at.
The gap
A transitive dependency executes arbitrary Node at install time, on every machine that installs this project and on every deployment build. Nothing in the toolchain examined that.
npm auditreports known advisories rather than execution surface, and gitleaks reads content rather than lifecycle hooks. A dependency can acquire a postinstall in a patch release and nothing would say so.What is actually there
Two install scripts in the whole tree, both dev-only. No production dependency has one:
unrs-resolver@1.12.2eslint-config-next→eslint-import-resolver-typescript→ hereoptionalDependencies, then exits. Reaches the network only when that resolution fails, which is the--no-optionalcasefsevents@2.3.3unrs-resolverwas verified as a no-op here: running it left the filesystem byte-identical, and the binding resolves without it. It is also not new, despite appearing in a recent build log. It sits at the same version in the lockfile before and after thenext@16.3.2bump.Two layers
CI installs with
--ignore-scripts. This removes the execution path rather than watching it. Verified that a clean--ignore-scriptsinstall still passes lint, tests, palette and build, including lint specifically, which is the only thing that exercises the resolver.scripts/check-install-scripts.mjsfails the build on anything new. An allowlist where each entry carries the reason it was allowed, so a future failure is read against a recorded standard rather than waved through. Production entries are reported separately, since those run wherever the app is built.Runnable locally with
npm run check:install-scripts.Verification, both directions
A check that only ever passes proves nothing, so it was tested failing as well:
npm ci --ignore-scriptsthen lint, tests, palette, buildnpm run lint, 155 tests,npm run paletteandnpm run buildall pass on this branch.Not covered here, and not yet done
The warning that prompted this appeared in the Vercel build log, and Vercel runs its own install. This PR hardens CI only. The deploy path is still executing that postinstall on every production build.
Closing that means setting the Vercel Install Command to
npm ci --ignore-scripts. That has not been tested and is not done. It is a change to the deploy pipeline and should be verified on a preview deployment before being trusted on production. Recorded as outstanding rather than complete.