install: support path-form link: dependencies - #35461
Conversation
WalkthroughChangesThe installer now recognizes filesystem-shaped Filesystem link dependencies
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 11:05 PM PT - Aug 14th, 2026
❌ @robobun, your commit 96f05e2 has some failures in 🧪 To try this PR locally: bunx bun-pr 35461That installs a local version of the PR into your bun-35461 --bun |
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
src/install/PackageManager/PackageManagerEnqueue.rs:2695-2711— When alink:./pathtarget is missing (no directory or no package.json), install still emitsPackage "…" is not linkedwith thebun link my-pkg-name-from-package-jsonremedy — which is actively wrong for the path form this PR adds. Consider adding anis_link_pathcheck in the else-branch here (and the mirrored verbose-warning branch at ~1451-1459) that instead says something likepackage.json not found at "./lib/mypkg".Extended reasoning...
What happens
With this PR,
"mypkg": "link:./does-not-exist"now takes the newRelative(Symlink)path inget_or_put_resolved_package(PackageManagerEnqueue.rs:2691+).FolderResolution::get_or_putcallsread_package_json_from_disk, which fails to open./does-not-exist/package.jsonwithENOENT;folder_resolver.rsmaps that toFolderResolution::Err(MissingPackageJSON).Back in
enqueue_dependency, theErr(MissingPackageJSON)result is mapped toNone(~line 1370). With_result == Noneand the dependency required, control reaches theif dependency_tag == Workspace { … } else { … }block.dependency_tagisSymlink, so the else-branch at 1428-1436 fires and prints:error: Package "mypkg" is not linked To install a linked package: bun link my-pkg-name-from-package-json Tip: the package name is from package.json, which can differ from the folder name.The verbose-warning branch at 1451-1459 has the identical text.
Why it is wrong for the new syntax
That message and remedy were written for
link:<name>, where the fix genuinely is "runbun linkin the target package first". Forlink:./path, the user never intended to touch the global link registry — the path they wrote inpackage.jsonis bad (typo, directory not created yet, missingpackage.json). Telling them to runbun linksends them in the wrong direction. Per REVIEW.md: "Error messages are reviewed word-for-word as code. Name what failed and why: the specific resource (quoted path/URL) … a concrete remedy."Step-by-step repro
package.jsoncontains{ "dependencies": { "mypkg": "link:./lib/mypkg" } }and./lib/mypkgdoes not exist.bun install→Tag::inferseeslink:→Tag::Symlink;version.symlink()is./lib/mypkg.get_or_put_resolved_package:is_link_path("./lib/mypkg")is true → resolves<top_level_dir>/lib/mypkgand callsFolderResolution::get_or_put(Relative(Symlink), …).read_package_json_from_disk→File::openat(cwd, "<abs>/lib/mypkg/package.json", O_RDONLY)→ENOENT→ returned asFolderResolution::Err(MissingPackageJSON).- Caller maps
Err(MissingPackageJSON)→None;dependency.behavior.is_required()is true;dependency_tag == Symlink(notWorkspace) → else-branch at 1428 emitsPackage "mypkg" is not linkedwith thebun linkhint. - Install exits nonzero (correct), but the diagnostic misdirects.
Why nothing prevents it
The else-branch at 1428 only distinguishes
Workspacefrom everything else. There is no third arm that checksdependency::is_link_path(this.lockfile.str(version.symlink()))before falling back to the global-link message.Suggested fix
Add that third arm in both the required-error branch (1428-1436) and the verbose-warning branch (1451-1459), e.g.:
} else if dependency::is_link_path(this.lockfile.str(version.symlink())) { bun_ast::add_error_pretty!( this.log_mut(), None, bun_ast::Loc::EMPTY, "package.json not found at \"{}\" for <b>link:<r> dependency \"{}\"\n\n", bstr::BStr::new(this.lockfile.str(version.symlink())), bstr::BStr::new(this.lockfile.str(&name)), ); } else { // existing "is not linked" message }
Impact / severity
This is diagnostic quality only — the happy path works, and the failure path still exits nonzero, so nothing silently succeeds. Filed as a nit; worth polishing since it affects the first error users of the new syntax will hit, but should not block the fix for #4719.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/install/lockfile/Package.rs`:
- Around line 1759-1796: Guard the manual "./" prefix construction in the
symlink handling at src/install/lockfile/Package.rs:1789-1793 and
src/install/resolvers/folder_resolver.rs:480-486 with a capacity check before
writing to PathBuffer; when relative.len() + 2 exceeds the buffer capacity, use
each location’s existing unsafe-folder-path error path and return the
appropriate install/resolution error instead of indexing the buffer. Preserve
the current path formatting for values that fit.
In `@test/cli/install/bun-link.test.ts`:
- Around line 477-608: Add a regression test within the “link: with a filesystem
path” suite where a non-workspace transitive dependency declares a path-form
link: pointing outside the project. Use the existing install helpers and
assertions to verify installation fails closed with the specific path-form/link
error, and confirm it does not resolve or link the host path; keep the existing
direct root and workspace-member cases unchanged.
🪄 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: 907656df-892c-4337-b36b-61d6c32fee50
📒 Files selected for processing (10)
src/install/PackageInstaller.rssrc/install/PackageManager/PackageManagerDirectories.rssrc/install/PackageManager/PackageManagerEnqueue.rssrc/install/dependency.rssrc/install/isolated_install.rssrc/install/isolated_install/Installer.rssrc/install/lockfile/Package.rssrc/install/migration.rssrc/install/resolvers/folder_resolver.rstest/cli/install/bun-link.test.ts
There was a problem hiding this comment.
No new issues found; the fixes in 911a9a6 address the three items from the last round (Symlink Version::eql now compares literal, the enqueue path-form gate now rejects non-workspace/non-override deps and honours overrides, and the test helper uses runBunInstall's returned strings directly). Given this is a feature addition to the package manager touching lockfile resolution shape and both installers, a human should sign off.
What was reviewed:
is_link_pathclassification vs.Tag::infer's folder shapes;~/is classified but not expanded (documented, matchesfile:).- Transitive/override trust gate in
PackageManagerEnqueue.rs— now fails closed withMissingPackageJSONand mirrors theTag::Folderarm. - Lockfile round-trip:
Version::eqlforSymlinkcomparesliteral; workspace-member test runs a second--frozen-lockfileinstall. ./-prefix buffer writes inPackage.rsandfolder_resolver.rsguarded againstMAX_PATH_BYTESoverflow.
Extended reasoning...
Overview
This PR adds support for link:./path (yarn/pnpm syntax) in bun install, resolving the path relative to the declaring package instead of treating it as a global-link-dir name. It touches 9 Rust files across dependency parsing (is_link_path), package.json normalization (Package.rs), resolution enqueue (PackageManagerEnqueue.rs), the folder resolver, and both hoisted (PackageInstaller.rs, PackageManagerDirectories.rs) and isolated (isolated_install/{Installer.rs,mod}) installers, plus a migration warning rewording. ~130 lines of new tests cover ./, ../, absolute, scoped, workspace-member, and error-path cases under both linkers.
Security risks
Path-form link: from an untrusted transitive package could symlink an arbitrary host directory into node_modules. The enqueue arm now gates on is_workspace_dependency || overrides.contains_name and returns Err(MissingPackageJSON) otherwise — the same fail-closed shape as the sibling file: arm. The Package.rs normalization still runs unconditionally (no features gate), but since the enqueue rejection precedes any filesystem access this is fail-closed. Practical exposure was already narrow (npm registries reject link: deps; git/tarball packages already run lifecycle scripts).
Level of scrutiny
High. This is the package manager: it changes what gets written into bun.lock for a resolution tag, adds a new GlobalOrRelative::Relative(Symlink) resolver arm, and branches both installers on a new predicate. Lockfile-shape changes are hard to revert once shipped, and the isolated installer's append_store_path runs on worker threads. Five prior review rounds surfaced a lockfile round-trip bug (spurious --frozen-lockfile diff via Version::eql), a trust-gate bypass, a stale migration warning, and several test-hygiene issues — all now addressed.
Other factors
Test coverage is solid for the happy paths and includes a --frozen-lockfile re-install to guard the round-trip fix. CodeRabbit's transitive-escape test suggestion was reasonably declined (would need a synthetic registry package with a link: dep; the gate is identical to the covered file: arm). Not approving because this is a user-facing feature in a critical subsystem with lockfile-format implications — a maintainer should confirm the design (storing project-relative paths as the Symlink resolution, keeping literal unchanged) is the intended long-term shape.
There was a problem hiding this comment.
Thanks — the three items from the last pass are all addressed in 911a9a6 (Symlink Version::eql now compares literal; path-form link: from a non-workspace/non-override dep is rejected outright instead of falling through to the global link dir; checkLink uses out/err directly and the --frozen-lockfile round-trip is asserted). No new findings this pass.
Deferring to a human reviewer given this adds a new link:./path protocol form across the resolver, both linkers, and lockfile serialization, and the is_workspace_dependency || overrides trust gate is security-adjacent.
What was reviewed:
is_link_pathshape parity withTag::infer(.,/,~/, Windows drive/\).- Lockfile round-trip:
Version::eqlonliteral+--frozen-lockfilere-install in tests. - Enqueue trust gate now rejects (not falls through) and includes the override exemption; error/warning wording for path vs name form.
./-prefix buffer writes inPackage.rs/folder_resolver.rsare length-checked beforecopy_from_slice.
Extended reasoning...
Overview
The PR teaches bun install to treat link:./path (and ../, /abs, ~/, Windows-drive) as a filesystem symlink target relative to the project, instead of a globally-registered package name. It touches 10 files: the shared is_link_path classifier in dependency.rs, the Tag::Symlink normalization arm in lockfile/Package.rs, the enqueue branch in PackageManagerEnqueue.rs, three installers (PackageInstaller.rs, PackageManagerDirectories.rs, isolated_install/Installer.rs + its main-thread pre-init in isolated_install.rs), a new Relative(Symlink) arm in folder_resolver.rs, a reworded pnpm-migration warning, and 12 new test cases across hoisted/isolated linkers.
Security risks
The material risk is a downloaded (transitive) package declaring link:/etc or link:../../.. and having that resolve to a host path. Commit 911a9a6 addresses my prior finding here: the enqueue branch now computes trusted = is_workspace_dependency || overrides.contains_name(...) and returns Err(MissingPackageJSON) when untrusted, rather than falling through to the global-link-dir lookup (which the earlier Package.rs normalization could have bypassed). This mirrors the sibling file: gate. The ./-prefix buffer writes in Package.rs and folder_resolver.rs are both bounds-checked before the copy_from_slice.
Level of scrutiny
High. This is package-manager core: dependency parsing, lockfile serialization/diff, and both install linkers. It introduces a new user-facing protocol form (yarn/pnpm parity), and the link: value is untrusted input that becomes a symlink target. The PR went through three review iterations with substantive corrections each time (workspace-relative normalization, Version::eql asymmetry breaking --frozen-lockfile, the trust-gate fallthrough), which is exactly why a human should give the final shape a look before it lands.
Other factors
Test coverage is reasonable — ./, ../, scoped, absolute, workspace-member-relative, and the missing-target error path, each × {hoisted, isolated}, plus a --frozen-lockfile re-install to prove the lockfile round-trips cleanly after the Version::eql fix. All prior review threads are resolved. The one design decision worth a human eye is whether the stored resolution being a project-relative path (kept path-shaped with a ./ prefix so installers can distinguish it from link:<name>) is the right lockfile representation long-term, and whether Features::LINK is the correct feature set for the target's own package.json parse.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/cli/install/bun-link.test.ts (1)
585-608: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winVerify that the missing-package diagnostic is path-specific.
Checking only
Could not find package.jsondoes not prove that the rejectedlink:value or resolved target path is identified. Assert the requested path in the error, ideally against the complete expected diagnostic. The PR objective specifically requires a path-specific missing-target error.🤖 Prompt for 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. In `@test/cli/install/bun-link.test.ts` around lines 585 - 608, Strengthen the assertion in the “errors with the path when package.json is missing” test to verify the complete missing-target diagnostic, including the requested `./does-not-exist` path (or its resolved target path), rather than only checking the generic `Could not find package.json` text. Keep the existing assertions that reject linked-package guidance and confirm exit status 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 `@src/install/lockfile/Package.rs`:
- Around line 1790-1800: Update the error message in the unsafe-folder-path
branch to include the rejected link target, the maximum permitted path length,
and a remedy such as shortening the path. Preserve the existing error location,
logging call, and InstallFailed return.
In `@test/cli/install/bun-link.test.ts`:
- Around line 491-492: Update the symlink assertions around the readlink calls
in bun-link tests to compare the fully normalized resolved target with the
expected path, replacing basename-based containment checks in both locations.
Preserve platform-independent slash normalization and assert exact equality
rather than matching only a target fragment.
---
Outside diff comments:
In `@test/cli/install/bun-link.test.ts`:
- Around line 585-608: Strengthen the assertion in the “errors with the path
when package.json is missing” test to verify the complete missing-target
diagnostic, including the requested `./does-not-exist` path (or its resolved
target path), rather than only checking the generic `Could not find
package.json` text. Keep the existing assertions that reject linked-package
guidance and confirm exit status 1.
🪄 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: deb0d16b-541d-4574-aeb1-143a852d8a6e
📒 Files selected for processing (5)
src/install/PackageManager/PackageManagerEnqueue.rssrc/install/dependency.rssrc/install/lockfile/Package.rssrc/install/resolvers/folder_resolver.rstest/cli/install/bun-link.test.ts
|
CI is green on build #80252 (c325929). All review threads are addressed; the two remaining bot suggestions (error-message wording, symlink basename assertion) are intentionally kept consistent with the sibling |
alii
left a comment
alii
left a comment
There was a problem hiding this comment.
Requesting changes. The path form itself and the six tests for it look right; the problems are around it.
- The trust check on the Symlink arm is not the file: check, only runs on a fresh resolve, reports a refusal as a missing package.json, and nothing exercises it.
- is_link_path is a second answer to a question pnpm.rs already answers with !is_npm_package_name, and the two disagree on link:lib/foo. bun.lock stores whichever rule ships, so settle it in this PR.
- pnpm.rs still writes importer-relative link: strings into Symlink resolutions, and the new installer arms now read those as root-relative.
One correction for the body: link:../x does not fail with "is not linked" on main. It resolves, writes the same link:../x resolution this branch writes, and fails at link time (FileNotFound, verified with bun-debug on main). So lockfiles main already wrote with ../ entries start installing after this lands, which is worth saying since it is a lockfile behavior change.
robobun
changed the title
|
Thanks, all four points addressed in 02f42b0 (comments trimmed in f29d94e); body rewritten to match, including the correction on what main does with Summary of the rework:
Re-requesting review. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/install/pnpm.rs (1)
702-731: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the pnpm link path before calling
join_string_buf. Oversized relative paths can exceed the fixedPathBuffer; the unchecked normalizer then panics beforelink_path_for_lockfilecan returninvalid_pnpm_lockfile(). Return the existing invalid-lockfile error when the combined path does not fit.🤖 Prompt for 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. In `@src/install/pnpm.rs` around lines 702 - 731, Guard the relative-path branch in the link-path rebasing logic before calling join_string_buf, using the available PathBuffer capacity or checked join result to detect overflow. When combining workspace_path and link_path cannot fit, return invalid_pnpm_lockfile() instead of invoking the unchecked normalizer; preserve absolute-path handling and successful link_path_for_lockfile processing.
🤖 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 `@src/install/PackageInstaller.rs`:
- Around line 1454-1477: Update the DanglingSymlink diagnostic handler to branch
on is_link_path(folder): for missing path-form links, report the target path and
its resolved project-relative location instead of instructing the user to run
bun link, while preserving the existing diagnostic for non-path links. Add a
regression test covering a missing link:../missing-package target.
- Around line 1458-1468: The combined condition in the dependency-link
validation must distinguish `folder.len() >= self.folder_path_buf.len()` from
`link_target_allowed_for_package(...)` failures. Emit a dedicated path-length
error for overlong targets that identifies the rejected target and supported
limit; retain the existing authorization error only for containment failures and
include the concrete remedy and relevant target details.
In `@src/install/PackageManager/PackageManagerEnqueue.rs`:
- Around line 1371-1383: Update the UnsafeLinkTarget handling in the dependency
enqueue flow so optional dependencies emit a warning when
this.options.log_level.is_verbose() is enabled before returning Ok(()). Match
the existing verbose skip-warning pattern and include the dependency and link
target context; preserve the current error diagnostic for required dependencies
and the immediate successful return.
---
Outside diff comments:
In `@src/install/pnpm.rs`:
- Around line 702-731: Guard the relative-path branch in the link-path rebasing
logic before calling join_string_buf, using the available PathBuffer capacity or
checked join result to detect overflow. When combining workspace_path and
link_path cannot fit, return invalid_pnpm_lockfile() instead of invoking the
unchecked normalizer; preserve absolute-path handling and successful
link_path_for_lockfile processing.
🪄 Autofix
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: 02f51b18-7ca0-4552-b60b-5a3fd82c7d84
📒 Files selected for processing (11)
src/install/PackageInstaller.rssrc/install/PackageManager/PackageManagerEnqueue.rssrc/install/dependency.rssrc/install/error.rssrc/install/isolated_install.rssrc/install/lockfile.rssrc/install/lockfile/Package.rssrc/install/pnpm.rssrc/install/resolvers/folder_resolver.rstest/cli/install/bun-link.test.tstest/cli/install/migration/pnpm-lock-migration.test.ts
alii
left a comment
alii
left a comment
There was a problem hiding this comment.
Looks ready to merge from this side. Checked the new head against each point from the last round: is_link_path is now the one rule and pnpm.rs calls it, the containment check is the file: rule with its own error and runs at resolve time and in both installers, pnpm migration re-bases onto the root, and the body's account of what main does with link:../x is now right; each has a test (read, not run here, and the body itself says bun-link.test.ts was deferred to CI, so confirm CI on this head ran it). One non-blocking note inline. The branch now conflicts with main in src/install/dependency.rs and test/cli/install/bun-link.test.ts and needs a rebase before it can land.
f29d94e to
8aa43aa
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Rebased onto main (bdb7382). The branch is now the reviewed change squashed into one commit (7a65e13, conflicts were the
On CI running |
|
CI on a5b7327 (build 94180): 155 of 156 test lanes green, |
alii
left a comment
alii
left a comment
There was a problem hiding this comment.
Still looks ready to merge from this side. The rebased commit is the reviewed diff apart from main's pub(crate) and stderrForInstall changes; the isolated refusal now runs in the pre-scheduling pass over resolutions and the per-entry arm is back to main's text, and the refusal tests now glob for the name under node_modules, which does catch the .bun//node_modules/ link the current build creates from that fixture. CI on this head is as reported by the author, not checked here.
|
@robobun this conflicts with main now, please rebase and get a fresh CI run so it can be merged. |
|
Rebased onto main and pushed (96f05e2); the rebase notes are in the PR description ( CI on the rebased head (build 97320): every Linux lane (x64, aarch64, ASAN), both Windows lanes and both macOS builds passed; the only test failures recorded are unrelated flakes that passed on retry. The two remaining jobs are the darwin 14 aarch64 test shards, which no agent has picked up in over five hours (the same is true of every other branch build in the queue right now), so I am not retriggering. Ready for review from my side. |
"pkg": "link:./dir" (and ../dir, lib/dir, /abs) now symlinks that directory, as in yarn and pnpm. A link: value is a path unless it is a package name; names keep going through the bun link global directory. Path targets are re-based onto the project root when the declaring package.json is parsed (as file: already is) and stored ./-prefixed and /-separated, so a stored value always reads back as a path. The pnpm migration produces the same form from pnpm's importer-relative value. Containment follows the file: rule: a target inside the project is always allowed; one that leaves it is allowed only when the root, a workspace, or a root override declared the dependency. Checked at resolve time and in both installers, since an install from an existing lockfile never resolves. Version::eql for Symlink compares the literal like Folder does; bun.lock round-trips the literal, so comparing the re-based value produced a diff on every second install. Fixes #4719 Fixes #5045
…schedules any task Entries are scheduled parent first, so a dependent's SymlinkDependencies step could create the link before the per-entry refusal exited. Do the check in the pass over resolutions that already runs before the first start_task; the per-entry arm goes back to what it was. Tests: assert that nothing named after the refused package exists anywhere under node_modules, store entries included, instead of the two hoisted-layout paths (which the isolated half never created, so it passed on main too). Pin the transitive block to the dummy registry like the first block. Migration test uses tempDir like the rest of its file.
Error::PathTooLong was removed on main; report an overlong link: target as ENAMETOOLONG like the other path-buffer overflows in this crate. `bun add --filter` re-spells a local path relative to each target. It only did so for link: values starting with ".", but a link: value is a path whenever it is not a package name, so use the same predicate; otherwise `bun add link:vendor/foo --filter api` writes a value that the target workspace reads as packages/api/vendor/foo. The add --filter test that asserted the old "is not linked" failure for a link: path now asserts the re-spelled value is written and installs.
a5b7327 to
96f05e2
Compare
2 participants
Problem
"pkg": "link:./lib/pkg"in package.json failsbun installwitherror: Package "pkg" is not linked(the yarn/pnpm spelling for "symlink this directory"; Bun link works but installs says package is not linked #4719, Install package from local directory #5045).Symlinkresolution treats the value as a name registered bybun linkand joins it onto the global link dir (PackageManagerEnqueue.rsTag::Symlinkarm,PackageInstaller.rs,PackageManagerDirectories.rs,isolated_install/Installer.rs::append_store_path).link:../xis the one shape that got further on main:normalize_package_json_pathresolves a.-prefixed value against the project root regardless of the global dir, so it resolves, is written to bun.lock aslink:../x, and then fails at link time (FileNotFound: failed linking dependency). See lockfile note below.pnpm.rs) already had a notion of path-formlink:(it stored pnpm's value verbatim) that disagreed with the rest of install on both what counts as a path and what the stored value is relative to.Fix
dependency::is_link_path: alink:value is a path unless it is an npm package name. This is the rulepnpm.rsalready used, andpnpm.rsnow calls the shared helper, so the two writers of bun.lock agree (link:lib/foois a path on both;link:fooandlink:@s/foostay global).Package::parse_dependencyre-bases a path-form value from the declaring package.json onto the project root (as it already does forfile:), andlink_path_for_lockfilemakes it/-separated and./-prefixed, so what is written always reads back as a path.pnpm.rsproduces the same form from pnpm's importer-relative value.Relative(Symlink)arm infolder_resolver.rs(project-relative,Features::LINKas before); name-form is unchanged. An overlong target is reported asENAMETOOLONG, as the other path-buffer overflows in the crate are.file:: a target inside the project is always allowed, including one declared by a transitivefile:package. One that leaves the project (..or absolute) is allowed only when the root or a workspace declared the dependency, or a rootoverrides/resolutionsentry names it (Lockfile::link_target_allowed_for_*). Checked at resolve time and again in both installers, because an install driven by an existing lockfile never resolves (hoisted: per package inPackageInstaller; isolated: in the pass over resolutions that runs before any task is scheduled, so no worker can create the link first). Refusal is its own error (UnsafeLinkTarget, "refusing to link ...") rather than a missing package.json.bun add --filter(add_remove_with_filter.rs::local_relative_path) re-spells a local path relative to each target; it now decides "is thislink:value a path" withis_link_pathtoo, sobun add link:vendor/foo --filter apiwriteslink:../../vendor/foointo the workspace rather than a value the workspace would read aspackages/api/vendor/foo. Name-form values are still passed through untouched.Version::eqlforSymlinkcomparesliterallikeFolderdoes; bun.lock round-tripsliteral, so comparing the re-based value made every second install see a diff.Could not find package.json at "<path>"instead of thebun linkhint.Why this is right:
link:with a path has meant "symlink this directory, do not install its dependencies" in yarn and pnpm for years, andfile:in bun already defines where a relative path is anchored and who may point outside the project; this makeslink:follow both.Lockfile note: bun.lock entries of the form
x@link:../yorx@link:/absthat main already writes (and then fails to install) start installing once this lands, subject to the containment rule above. Name-form entries are unaffected. Migrated pnpm lockfiles that previously carried importer-relative targets are now written root-relative; ones migrated by older versions were never installable.Verified:
test/cli/install/bun-link.test.ts:./, barelib/x(asserts the./stored form),../, absolute, scoped, workspace-member-relative, missing target; each under both linkers with a--frozen-lockfilere-install. Containment block: transitive escape refused at resolve and from an existing lockfile (asserting nothing by that name exists anywhere under node_modules, isolated store entries included; the released binary creates one at.bun/<pkg>/node_modules/<name>), transitive in-project target installed,overrides/resolutionsexemption honoured; both linkers.test/cli/install/bun-add-filter.test.ts:link:./vendor/fooandlink:vendor/fooare written to the target aslink:../../vendor/foo, recorded asfoo@link:./vendor/foo, install, and survive--frozen-lockfile; a missing target is refused naming the per-target spelling and writes nothing. This replaces the test that asserted the old "is not linked" failure for alink:path.test/cli/install/migration/pnpm-lock-migration.test.ts:file:specifier from a non-root importer migrates to a root-relative target and installs.bun add --filter, scoped overrides andpnpm-lock-v9.test.ts); that file, the other pnpm migration suites, thefile:containment tests inbun-install.test.ts,bun-add-catalog.test.tslocal-path tests, and the name-formlink:tests inbun-prune/bun-pm-licensespass locally. Inbun-link.test.ts, the pre-existingshould link dependency without crashingfails locally under a debug build because the install failure it expects dumps a symbolized trace to stdout (cfg(bun_debug)inPackageInstaller.rs, not touched here); it is green in CI.cargo check/clippyon linux andcargo checkforx86_64-pc-windows-msvc.Fixes #4719
Fixes #5045
Background
bun link/ name form:bun linkin a package dir symlinks it into~/.bun/install/global/node_modules/<name>;"dep": "link:<name>"elsewhere then symlinks from there. That is the only form bun supported; this PR adds the path form beside it.Symlinkresolution: the lockfile entry for alink:dependency. Its one string field was always a name; it is now either a name or a root-relative/absolute path, andis_link_pathtells them apart.Features::LINK: the package.json parse mode for link targets; it reads the manifest but does not enqueue the target's dependencies (unchanged).bin_target_escapes_package_dir: the existingfile:check for "does this relative path have a..that climbs above its base, or is it absolute"; reused here with the project root as the base.bun installwith no usable lockfile runs resolution (PackageManagerEnqueue.rs); with one, it goes straight to the installers. Any policy about a resolution therefore has to be enforced in both places, which is how thefile:checks are laid out.bun add --filter: adds a positional to the selected workspaces. A local path on the command line is relative to the cwd, so it is re-spelled relative to each workspace's package.json before being written;local_relative_pathdecides which positionals get that treatment.no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-add-filter.test.ts test/cli/install/bun-link.test.ts