Skip to content

docs: clarify trustedDependencies replaces the default allow list#31027

Merged
alii merged 4 commits into
mainfrom
farm/00f0a41e/docs-trusted-dependencies-empty-array
May 26, 2026
Merged

docs: clarify trustedDependencies replaces the default allow list#31027
alii merged 4 commits into
mainfrom
farm/00f0a41e/docs-trusted-dependencies-empty-array

Conversation

@robobun

@robobun robobun commented May 19, 2026

Copy link
Copy Markdown
Collaborator

What

Closes #31026.

The trustedDependencies docs at docs/pm/lifecycle.mdx don't explain what happens when the field is set to an empty array. Users reasonably assume "trustedDependencies": [] would add nothing on top of the default top-500 list — but it actually disables the default list entirely.

Behavior (verified against source)

From Lockfile.hasTrustedDependency in src/install/lockfile.zig:2126:

pub fn hasTrustedDependency(this: *const Lockfile, name: []const u8, resolution: *const Resolution) bool {
    if (this.trusted_dependencies) |trusted_dependencies| {
        const hash = @as(u32, @truncate(String.Builder.stringHash(name)));
        return trusted_dependencies.contains(hash);
    }
    // Only allow default trusted dependencies for npm packages
    return resolution.tag == .npm and default_trusted_dependencies.has(name);
}

The parser (src/install/lockfile/Package.zig:1556) initialises lockfile.trusted_dependencies to a non-null empty map the moment the trustedDependencies key is present — even if the array is empty. So the fallback to the default list is only reached when the key is absent from package.json. Three cases:

package.json Trusted packages
key omitted the ~500 built-in defaults (npm sources only)
["pkg"] only pkg — default list ignored
[] nothing — default list ignored

Change

Adds a short subsection + table to docs/pm/lifecycle.mdx covering all three cases, and notes that the field replaces rather than extends the default list. Docs-only, no code changes.

@robobun

robobun commented May 19, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 11:36 PM PT - May 18th, 2026

@autofix-ci[bot], your commit d86b8a35592e34c33db4092bf6901912c83d8ff9 passed in Build #55991! 🎉


🧪   To try this PR locally:

bunx bun-pr 31027

That installs a local version of the PR into your bun-31027 executable, so you can run:

bun-31027 --bun

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4a730709-9a9a-43c2-8e5d-1b0680dfd199

📥 Commits

Reviewing files that changed from the base of the PR and between 37e98da and d86b8a3.

📒 Files selected for processing (1)
  • docs/pm/lifecycle.mdx

Walkthrough

Updated docs/pm/lifecycle.mdx to change the built-in trusted-dependencies description to “a curated list of popular npm packages” and add a subsection explaining that trustedDependencies replaces the default list and documents three modes: omitted (use Bun built-in list for npm sources), non-empty array (allow only listed packages), and empty array (allow none).

Changes

Lifecycle script trusted dependencies documentation

Layer / File(s) Summary
trustedDependencies description and behavior subsection
docs/pm/lifecycle.mdx
Reworded the built-in trusted-dependencies description to “a curated list of popular npm packages” and added a new subsection describing trustedDependencies semantics: the field replaces Bun’s default allow list and supports three modes (omitted → Bun built-in list for npm sources; non-empty array → only listed packages allowed; empty array → no packages allowed).
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main change: documenting that trustedDependencies replaces rather than extends the default allow list.
Description check ✅ Passed The PR description is comprehensive and well-structured with clear sections, but it doesn't explicitly follow the repository's template structure for 'How did you verify your code works?' section.
Linked Issues check ✅ Passed The PR successfully addresses all objectives from #31026: documents that empty trustedDependencies arrays disable the default list, clarifies the replacement behavior, and covers all three cases (omitted, non-empty, empty).
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue #31026; the PR is documentation-only and updates lifecycle.mdx as required without introducing unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/pm/lifecycle.mdx`:
- Line 66: Reword the ambiguous sentence about trustedDependencies to clearly
state that defining trustedDependencies replaces the default allow list; update
the line mentioning "To trust a specific subset of packages, list them
explicitly — you do not need to re-list entries from the default list for
packages you already depend on." to something like: "To trust a specific subset
of packages, list them explicitly. Because defining trustedDependencies replaces
the default list, include only the packages you want to trust." Keep references
to trustedDependencies and --ignore-scripts intact.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: baad4860-dcab-471a-bd91-9ff9c291f0da

📥 Commits

Reviewing files that changed from the base of the PR and between 08f4ab4 and 76756b4.

📒 Files selected for processing (1)
  • docs/pm/lifecycle.mdx

Comment thread docs/pm/lifecycle.mdx Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Found 2 issues this PR may fix:

  1. Allow disabling postinstall for top 500 packages #7642 - Users want to disable postinstall for the default ~500 packages; the docs clarification shows that setting trustedDependencies (even to []) already replaces the default list entirely
  2. Add an option to explicitly untrust a dependency #12855 - The proposed untrustedDependencies feature stems from a misunderstanding that trustedDependencies extends the defaults; the docs clarification explains it replaces them

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #7642
Fixes #12855

🤖 Generated with Claude Code

@robobun

robobun commented May 19, 2026

Copy link
Copy Markdown
Collaborator Author

Re: find-issues-bot — intentionally not adding `Fixes` for #7642 or #12855. Both are open feature requests (#7642 wants an interactive prompt; #12855 wants an untrustedDependencies field to silence the "Blocked N postinstalls" warning). This PR only clarifies existing behavior in the docs, so it shouldn't auto-close either on merge.

Comment thread docs/pm/lifecycle.mdx Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — thanks for the reword in 4003b70, the follow-up paragraph now matches the implementation. Left one non-blocking nit inline about the "~500" figure.

Extended reasoning...

Overview

Docs-only change: adds a 12-line subsection to docs/pm/lifecycle.mdx with a three-row table explaining that trustedDependencies replaces Bun's built-in allow list rather than extending it, and covering the omitted / non-empty / [] cases. No code, build, or config changes.

Security risks

None. This is prose in a .mdx file; it does not affect which lifecycle scripts Bun actually executes. The only external link added points to a file within this same repo on github.com.

Level of scrutiny

Low. The behavior described was verified against Lockfile.hasTrustedDependency in src/install/lockfile.zig and the parser in src/install/lockfile/Package.zig (cited in the PR description and re-checked during my first review). I confirmed sharp and esbuild — now named as examples — are present at lines 309 and 163 of default-trusted-dependencies.txt. docs/ has no CODEOWNERS entry.

Other factors

  • My earlier blocking comment (the closing clause that read as the inverse of the actual semantics) was addressed in 4003b70 and the thread is resolved; the new wording is accurate.
  • CodeRabbit's overlapping concern is also resolved.
  • The one remaining finding is a nit: the table says "~500 packages" but the file has 367 entries. This figure is inherited from the pre-existing "top 500" wording two paragraphs up (line 47), so the new row is internally consistent with surrounding docs, links directly to the source file, and doesn't affect the replace-vs-extend semantics this PR exists to clarify. Not a blocker.

Comment thread docs/pm/lifecycle.mdx Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — thanks for addressing both rounds of feedback; the final wording accurately reflects the replace-not-extend semantics.

Extended reasoning...

Overview

This PR touches a single docs file, docs/pm/lifecycle.mdx, adding a subsection that clarifies trustedDependencies replaces Bun's default allow list rather than extending it, with a three-row table covering omitted / non-empty / empty-array cases. It also rewords the pre-existing "top 500" line to be count-neutral.

Security risks

None. This is documentation only — no code, config, or build changes. The content describes existing security-relevant behavior more accurately than before, which is a net positive for users.

Level of scrutiny

Low. Docs-only change to an .mdx file with no executable content. The technical claims were verified against src/install/lockfile.zig:2126-2134 (hasTrustedDependency) and the parser behavior in src/install/lockfile/Package.zig, and they are correct: a non-null trusted_dependencies (set whenever the key is present) bypasses the default list entirely. The sharp/esbuild examples named in the new prose are confirmed present in src/install/default-trusted-dependencies.txt.

Other factors

I left two inline comments on earlier revisions (a misleading closing clause that contradicted the table, and a stale "~500" figure). Both were addressed in commits 4003b70 and 37e98da respectively, and CodeRabbit's overlapping nit was resolved by the same fix. The author also correctly declined to auto-close unrelated feature-request issues. The autofix bot applied formatting in d86b8a3 and CI is green. No CODEOWNERS entry covers docs/. Nothing remains outstanding.

@pauljohnberry

Copy link
Copy Markdown

I tried this with Bun v1.3.14 using "trustedDependencies": [] and bun pm untrusted, but it did not block the default trusted list.

However, adding at least one entry did work and prevented the default list from being applied, e.g.:

"trustedDependencies": ["cypress"]

@alii alii merged commit a6a5614 into main May 26, 2026
6 checks passed
@alii alii deleted the farm/00f0a41e/docs-trusted-dependencies-empty-array branch May 26, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docs/Security | Clarify behavior of empty trustedDependencies in docs/lifecycle.mdx

3 participants