Skip to content

fix(arborist): match allowScripts keys for local paths - #9901

Open
lazerg wants to merge 2 commits into
npm:latestfrom
lazerg:fix/9900-allow-scripts-file-spec
Open

fix(arborist): match allowScripts keys for local paths#9901
lazerg wants to merge 2 commits into
npm:latestfrom
lazerg:fix/9900-allow-scripts-file-spec

Conversation

@lazerg

@lazerg lazerg commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

What / Why

An allowScripts entry for a local dependency never matches it, so the install scripts stay blocked whichever form of the path is used as the key. On Windows that includes the absolute path from the error message, which is what this issue reports; on every platform it also includes the relative form the lockfile stores.

matchFileOrDir in workspaces/arborist/lib/script-allowed.js compared node.resolved against npa's saveSpec and fetchSpec. The loaded tree gets its resolved from consistentResolve, which returns file: glued onto the absolute fetchSpec and therefore keeps backslashes on Windows. npa's saveSpec is always forward-slashed and stays relative when the key is relative, and fetchSpec never carries the file: prefix, so neither side lines up. The matcher now also compares against file:${parsed.fetchSpec}, the same string consistentResolve builds.

keyTargetsNode in lib/utils/allow-scripts-writer.js carried its own copy of that comparison and drifted the same way, which left npm install-scripts approve writing a key it could not then recognise. Both call sites now share one exported helper, so they cannot disagree again.

Matching a local tarball by bare package name is deliberately still unsupported: a local tarball's name comes from its own manifest, so matchRegistry rejects it, and nameKeyFor refuses to write one for the same reason.

Testing

New case in workspaces/arborist/test/script-allowed.js, next to the existing local-tarball one: a node resolved to an absolute file: path matches both the relative and the absolute key. It fails without the change. Also checked by hand, installing a local tgz with an install script and "allowScripts": { "file:scripty-1.0.0.tgz": true }: blocked before, script runs after.

References

Fixes #9900

@lazerg
lazerg requested review from a team as code owners August 20, 2026 10:16
@lazerg

lazerg commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

The Test - Windows - 24.15.0 job fails in Reset Deps, where node crashes with exit code 3221226505 before any test runs, and the other five Windows jobs pass, so it is unrelated to this change. I cannot re-run it from the fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] allowScripts does not work for local tarball on Windows

1 participant