Skip to content

feat: by default trust the most downloaded packages with build scripts#10134

Merged
zkochan merged 3 commits into
teambit:masterfrom
zkochan:allow-scripts-default-list
Dec 19, 2025
Merged

feat: by default trust the most downloaded packages with build scripts#10134
zkochan merged 3 commits into
teambit:masterfrom
zkochan:allow-scripts-default-list

Conversation

@zkochan
Copy link
Copy Markdown
Member

@zkochan zkochan commented Dec 19, 2025

Proposed Changes

  • Added a default allowlist of popular packages containing postinstall scripts that are known to be safe. You can see this list here. Packages from this allowlist will run postinstall scripts during installation even if they are not added to the "allowScripts" field in workspace.jsonc.

Copilot AI review requested due to automatic review settings December 19, 2025 08:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces default trust for the most downloaded packages with build scripts by adding the @pnpm/plugin-trusted-deps package and integrating its trusted package list into the script policy resolution logic.

Key Changes:

  • Added @pnpm/plugin-trusted-deps dependency to enable access to a curated list of trusted packages
  • Modified resolveScriptPolicies function to automatically allow build scripts for trusted packages unless explicitly configured otherwise

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
workspace.jsonc Adds @pnpm/plugin-trusted-deps as a new dependency to provide the list of trusted package names
scopes/dependencies/pnpm/lynx.ts Imports TRUSTED_PACKAGE_NAMES and updates script policy resolution to auto-trust popular packages with build scripts when not explicitly configured
Comments suppressed due to low confidence (1)

scopes/dependencies/pnpm/lynx.ts:425

  • Avoid automated semicolon insertion (92% of all statements in the enclosing function have an explicit semicolon).
        onlyBuiltDependencies.push(trustedPkgName)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scopes/dependencies/pnpm/lynx.ts Outdated
Comment thread scopes/dependencies/pnpm/lynx.ts Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings December 19, 2025 10:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread workspace.jsonc
"@pnpm/parse-overrides": "1001.0.3",
"@pnpm/plugin-commands-publishing": "1000.2.25",
"@pnpm/plugin-commands-rebuild": "1006.0.0",
"@pnpm/plugin-trusted-deps": "^0.2.0",
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version format for this dependency uses a caret range (^0.2.0) which is inconsistent with the other pnpm plugin dependencies in this file that use exact versions. For example, "@pnpm/plugin-commands-rebuild" uses "1006.0.0" without a caret. Consider using an exact version "0.2.0" to maintain consistency with the established versioning pattern in this configuration.

Suggested change
"@pnpm/plugin-trusted-deps": "^0.2.0",
"@pnpm/plugin-trusted-deps": "0.2.0",

Copilot uses AI. Check for mistakes.
Comment on lines +423 to +427
for (const trustedPkgName of TRUSTED_PACKAGE_NAMES) {
if (allowScripts?.[trustedPkgName] == null) {
onlyBuiltDependencies.push(trustedPkgName);
}
}
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This security-sensitive functionality that automatically trusts packages from TRUSTED_PACKAGE_NAMES lacks test coverage. Given that this directory contains test files (e.g., lockfile-deps-graph-converter.spec.ts) and this change affects script execution permissions, tests should be added to verify: 1) trusted packages are correctly added to onlyBuiltDependencies when not in allowScripts, 2) explicit user configurations (both true and false) are respected and not overridden, and 3) the interaction between trusted packages and the dangerouslyAllowAllScripts flag works as expected.

Copilot uses AI. Check for mistakes.
@zkochan zkochan merged commit 3f58358 into teambit:master Dec 19, 2025
12 checks passed
@zkochan zkochan deleted the allow-scripts-default-list branch December 19, 2025 12:29
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.

3 participants