feat(pnpm): block untrusted package postinstall scripts by default #10140
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances security by blocking postinstall scripts for known untrusted packages (es5-ext, less, protobufjs, core-js, core-js-pure, ssh) by default. Users can still explicitly allow these packages via the allowScripts configuration if needed.
- Added a constant list of untrusted package names
- Modified the script policy resolution logic to automatically block these packages unless explicitly allowed
- Maintains backward compatibility through the existing
allowScriptsoverride mechanism
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Add untrusted packages to ignoredBuiltDependencies unless the user explicitly allows them | ||
| for (const untrustedPkgName of UNTRUSTED_PACKAGE_NAMES) { | ||
| if (allowScripts?.[untrustedPkgName] !== true) { | ||
| ignoredBuiltDependencies.push(untrustedPkgName); | ||
| } | ||
| } |
There was a problem hiding this comment.
The untrusted packages blocking logic is only applied in the default mode (when dangerouslyAllowAllScripts is false), but not when dangerouslyAllowAllScripts is true. This means that setting dangerouslyAllowAllScripts: true will bypass the untrusted package protection entirely, even for packages that should never run scripts due to security concerns.
Consider applying the untrusted packages check regardless of the dangerouslyAllowAllScripts setting, or at minimum adding these packages to the neverBuiltDependencies list in the dangerouslyAllowAllScripts branch (lines 407-411) alongside 'core-js'.
| // Add untrusted packages to ignoredBuiltDependencies unless the user explicitly allows them | ||
| for (const untrustedPkgName of UNTRUSTED_PACKAGE_NAMES) { | ||
| if (allowScripts?.[untrustedPkgName] !== true) { | ||
| ignoredBuiltDependencies.push(untrustedPkgName); | ||
| } | ||
| } |
There was a problem hiding this comment.
The UNTRUSTED_PACKAGE_NAMES array includes 'core-js' and 'core-js-pure', but line 410 also hardcodes 'core-js' in the neverBuiltDependencies list for the dangerouslyAllowAllScripts mode. This creates an inconsistency where 'core-js' is treated differently from the other untrusted packages when dangerouslyAllowAllScripts is enabled.
Consider updating line 410 to use UNTRUSTED_PACKAGE_NAMES instead of hardcoding 'core-js' to maintain consistency and ensure all untrusted packages are handled uniformly.
| // Add untrusted packages to ignoredBuiltDependencies unless the user explicitly allows them | ||
| for (const untrustedPkgName of UNTRUSTED_PACKAGE_NAMES) { | ||
| if (allowScripts?.[untrustedPkgName] !== true) { | ||
| ignoredBuiltDependencies.push(untrustedPkgName); | ||
| } | ||
| } |
There was a problem hiding this comment.
The new logic for blocking untrusted packages lacks test coverage. Given that this is a security-sensitive feature that affects package installation behavior, it should have comprehensive tests to verify:
- Untrusted packages are blocked by default
- Explicitly setting
allowScripts[packageName] = trueallows the package - Explicitly setting
allowScripts[packageName] = falseblocks the package - The behavior when
dangerouslyAllowAllScriptsis enabled
Consider adding unit tests for the resolveScriptPolicies function to ensure this security feature works as expected.
Block postinstall scripts for known untrusted packages (es5-ext, less, protobufjs, core-js, core-js-pure, ssh) by default. Users can explicitly allow them via
allowScriptsif needed.Untrusted Packages Analysis
None of these scripts are required for package functionality.