From f4cab72f02fdcbd905d1f8a53b8c2002a0347fe8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 08:31:20 +0000 Subject: [PATCH 1/9] install: only constrain transitive file: targets of remote packages A file: package referenced by the root package.json can declare its own file: dependencies that resolve outside the project root. The escape check added in #31417 rejected those at resolve time (bun update / fresh bun install) and at install time, even though the declaring package.json is local and user authored. Scope both checks to dependencies declared by remote packages (registry, git, tarball): resolve the declaring package of the dependency and skip the check when it is the root, a workspace, or a folder package. resolution::Tag::is_local_package names that split and replaces the three hand-written Root|Workspace|Folder matches. --- src/install/PackageInstaller.rs | 34 +- .../PackageManager/PackageManagerEnqueue.rs | 8 +- .../PackageManagerResolution.rs | 9 +- src/install/lockfile.rs | 19 ++ src/install/lockfile/Tree.rs | 9 +- src/install/resolution.rs | 10 + test/cli/install/bun-install.test.ts | 305 +++++++++++++----- 7 files changed, 293 insertions(+), 101 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index d80a7151486a..628327353ebf 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -1455,20 +1455,28 @@ impl<'a> PackageInstaller<'a> { installer.cache_dir = Fd::cwd(); } else { // transitive folder dependencies are not hoisted + // + // Only constrain `file:` targets declared by remote packages + // (registry, git, tarball); the project's own local packages are + // user authored and may point anywhere, like root dependencies. if folder.len() >= self.folder_path_buf.len() - || (bin::bin_target_escapes_package_dir(folder) && { - // overrides/resolutions are only ever parsed from the root - // package.json, so a folder path that reached here via an - // override was written by the user and is trusted the same - // as a direct dependency of the root. - let dep = &self.lockfile().buffers.dependencies.as_slice() - [dependency_id as usize]; - !self.lockfile().overrides.contains_name( - dep.name_hash, - dep.name.slice(string_buf!()), - string_buf!(), - ) - }) + || (bin::bin_target_escapes_package_dir(folder) + && !self + .lockfile() + .is_dependency_of_local_package(dependency_id) + && { + // overrides/resolutions are only ever parsed from the root + // package.json, so a folder path that reached here via an + // override was written by the user and is trusted the same + // as a direct dependency of the root. + let dep = &self.lockfile().buffers.dependencies.as_slice() + [dependency_id as usize]; + !self.lockfile().overrides.contains_name( + dep.name_hash, + dep.name.slice(string_buf!()), + string_buf!(), + ) + }) { if log_level != Options::LogLevel::Silent { bun_core::pretty_errorln!( diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index adf08bded22a..8919c2455836 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -2849,7 +2849,13 @@ fn get_or_put_resolved_package( } // transitive folder dependencies do not have their dependencies resolved - if crate::bin::bin_target_escapes_package_dir(this.lockfile.str(&folder)) { + // + // Only constrain `file:` targets declared by remote packages + // (registry, git, tarball); the project's own local packages are + // user authored and may point anywhere, like root dependencies. + if crate::bin::bin_target_escapes_package_dir(this.lockfile.str(&folder)) + && !this.lockfile.is_dependency_of_local_package(dependency_id) + { // overrides/resolutions are only ever parsed from the root // package.json, so a folder path that reached here via an // override was written by the user and is trusted the same diff --git a/src/install/PackageManager/PackageManagerResolution.rs b/src/install/PackageManager/PackageManagerResolution.rs index 2bfc156cd7ab..78ece7d050b1 100644 --- a/src/install/PackageManager/PackageManagerResolution.rs +++ b/src/install/PackageManager/PackageManagerResolution.rs @@ -326,11 +326,10 @@ impl PackageManager { continue; } - let features = match pkg_resolutions[parent_id].tag { - ResolutionTag::Root | ResolutionTag::Workspace | ResolutionTag::Folder => { - self.options.local_package_features - } - _ => self.options.remote_package_features, + let features = if pkg_resolutions[parent_id].tag.is_local_package() { + self.options.local_package_features + } else { + self.options.remote_package_features }; // even if optional dependencies are enabled, it's still allowed to fail if failed_dep.behavior.is_optional() || !failed_dep.behavior.is_enabled(features) { diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index 4c721ac55ae6..b2138c673d02 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -860,6 +860,25 @@ impl Lockfile { 0 } + /// Is dependency `id` declared by a local package (see + /// [`resolution::Tag::is_local_package`])? Its `file:` paths are then + /// trusted like root dependencies; paths declared by remote packages + /// (registry, git, tarball) are not. + /// + /// A `Folder` declaring package is always itself declared by the root or a + /// workspace: only those folder dependencies have their package.json + /// parsed. A folder dependency of any other package is recorded as a + /// resolution with no dependencies of its own (see the transitive folder + /// branch in `get_or_put_resolved_package`), so it never declares one. + pub(crate) fn is_dependency_of_local_package(&self, id: DependencyID) -> bool { + let Some(parent_id) = self.get_parent_pkg_of_dependency(id) else { + return false; + }; + self.packages.items_resolution()[parent_id as usize] + .tag + .is_local_package() + } + /// Does this tree id belong to a workspace (including workspace root)? /// TODO(dylan-conway) fix! pub(crate) fn is_workspace_tree_id(&self, id: tree::Id) -> bool { diff --git a/src/install/lockfile/Tree.rs b/src/install/lockfile/Tree.rs index c17ca42f3151..2376a83d1e1e 100644 --- a/src/install/lockfile/Tree.rs +++ b/src/install/lockfile/Tree.rs @@ -602,11 +602,10 @@ pub(crate) fn is_filtered_dependency_or_workspace( return true; } - let dep_features = match parent_res.tag { - crate::resolution::Tag::Root - | crate::resolution::Tag::Workspace - | crate::resolution::Tag::Folder => manager.options.local_package_features, - _ => manager.options.remote_package_features, + let dep_features = if parent_res.tag.is_local_package() { + manager.options.local_package_features + } else { + manager.options.remote_package_features }; if !dep.behavior.is_enabled(dep_features) { diff --git a/src/install/resolution.rs b/src/install/resolution.rs index d1e6fa7424e4..855ee6c29296 100644 --- a/src/install/resolution.rs +++ b/src/install/resolution.rs @@ -1014,6 +1014,16 @@ impl Tag { self == Tag::Git || self == Tag::Github } + /// A local package: the root, a workspace, or a `file:` folder package. + /// Their package.jsons are authored by the user (or by files the user + /// explicitly points the project at), unlike content fetched from a + /// registry, git host, or tarball. Local packages get + /// `local_package_features` and their `file:` dependency paths are trusted + /// like root dependencies. + pub(crate) fn is_local_package(self) -> bool { + self == Tag::Root || self == Tag::Workspace || self == Tag::Folder + } + pub(crate) fn can_enqueue_install_task(self) -> bool { self == Tag::Npm || self == Tag::LocalTarball diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index d41a2311f467..9df34190a485 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -10240,56 +10240,131 @@ it("does not extract a tarball for a dependency alias containing '..' path segme }); }); -it("does not install transitive file: dependencies that point outside their package", async () => { - // A dependency declared by a non-workspace package (here: a folder dependency - // of the project) uses a file: specifier pointing at an absolute path outside - // of that package and outside the project. That directory must not be linked - // into node_modules. - using dir = tempDir("transitive-file-dep", { - "secret/credentials.txt": "do-not-link-me", +it("does not install a registry package's transitive file: dependency that points outside its package", async () => { + // A dependency declared by a registry package uses a file: specifier + // pointing at an absolute path outside of that package and outside the + // project. Registry content is not written by the user, so that directory + // must not be resolved or linked into node_modules. (file: dependencies + // declared by the project's own local file: packages are user authored and + // are allowed to point outside the project; see the tests below.) + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + using dir = tempDir("registry-transitive-file-dep", { + "secret/credentials.txt": "do-not-link-me", + }); + const secretDir = join(String(dir), "secret"); + setContextHandler( + ctx, + dummyRegistryForContext(ctx, urls, { + "0.0.3": { + dependencies: { + loot: "file:" + secretDir.replaceAll("\\", "/"), + }, + }, + }), + ); + await writeFile( + join(ctx.package_dir, "package.json"), + JSON.stringify({ + name: "my-app", + version: "1.0.0", + dependencies: { + baz: "0.0.3", + }, + }), + ); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "pipe", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); + + // The directory outside the package must not appear under node_modules, + // neither hoisted nor nested under the declaring package. + expect(await exists(join(ctx.package_dir, "node_modules", "loot"))).toBe(false); + expect(await exists(join(ctx.package_dir, "node_modules", "baz", "node_modules", "loot"))).toBe(false); + // The dependency is reported as unresolvable instead of silently linking local files. + expect(err).toContain("Could not find package.json"); + expect(out).not.toContain("2 packages installed"); + expect(exitCode).toBe(1); + }); +}); + +it("installs transitive file: dependencies of a local file: package that point outside the project", async () => { + // A file: package referenced by the root package.json lives outside the + // project and declares its own relative folder dependencies that also land + // outside the project. The declaring package is local (not from a registry), + // so its file: paths are user authored and must not be rejected by the + // escape check that constrains registry packages. + using dir = tempDir("local-file-dep-outside", { + "packages/plugin/package.json": JSON.stringify({ + name: "plugin", + version: "1.0.0", + dependencies: { + "shared-lib": "../shared-lib", + }, + devDependencies: { + "shared-dev-lib": "file:../shared-dev-lib", + }, + }), + "packages/plugin/index.js": "module.exports = 'plugin';", + "packages/shared-lib/package.json": JSON.stringify({ name: "shared-lib", version: "1.0.0" }), + "packages/shared-dev-lib/package.json": JSON.stringify({ name: "shared-dev-lib", version: "1.0.0" }), "project/package.json": JSON.stringify({ name: "my-app", version: "1.0.0", dependencies: { - "evil-folder-dep": "file:./evil-folder-dep", + plugin: "file:../packages/plugin", }, }), - "project/evil-folder-dep/index.js": "module.exports = 1;", }); const projectDir = join(String(dir), "project"); - const secretDir = join(String(dir), "secret"); - await write( - join(projectDir, "evil-folder-dep", "package.json"), - JSON.stringify({ - name: "evil-folder-dep", - version: "1.0.0", - dependencies: { - loot: "file:" + secretDir.replaceAll("\\", "/"), - }, - }), - ); + // 1) `bun install` with no lockfile and 2) `bun update` exercise the + // resolve/enqueue path; 3) `bun install` with the saved lockfile and an + // empty node_modules exercises the package installer path. + for (const [i, cmd] of ["install", "update", "install"].entries()) { + if (i === 2) { + await rm(join(projectDir, "node_modules"), { recursive: true, force: true }); + } + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), cmd], + cwd: projectDir, + stdout: "pipe", + stdin: "pipe", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); - const { stdout, stderr, exited } = spawn({ - cmd: [bunExe(), "install"], - cwd: projectDir, - stdout: "pipe", - stdin: "pipe", - stderr: "pipe", - env, - }); - const err = await stderr.text(); - const out = await stdout.text(); - const exitCode = await exited; + expect(err).not.toContain("Could not find package.json"); + expect(err).not.toContain("failed to resolve"); + expect(err).not.toContain("unsafe folder path"); + expect(err).not.toContain("refusing to install"); + expect(out).toContain("plugin"); + expect(exitCode).toBe(0); + } - // The directory outside the package must not appear under node_modules, - // neither hoisted nor nested under the declaring package. - expect(await exists(join(projectDir, "node_modules", "loot"))).toBe(false); - expect(await exists(join(projectDir, "node_modules", "evil-folder-dep", "node_modules", "loot"))).toBe(false); - // The dependency is reported as unresolvable instead of silently linking local files. - expect(err).toContain("Could not find package.json"); - expect(out).not.toContain("2 packages installed"); - expect(exitCode).toBe(1); + // Both transitive folder dependencies resolve, relative to the project root. + // On Windows the stored folder path uses backslashes (JSON-escaped in the + // lockfile); normalize them before asserting. + const lock = (await file(join(projectDir, "bun.lock")).text()).replaceAll("\\\\", "/"); + expect(lock).toContain('"plugin/shared-lib"'); + expect(lock).toContain("shared-lib@file:../packages/shared-lib"); + expect(lock).toContain('"plugin/shared-dev-lib"'); + expect(await exists(join(projectDir, "node_modules", "plugin", "package.json"))).toBe(true); + // And both are linked under the declaring package. + expect(await exists(join(projectDir, "node_modules", "plugin", "node_modules", "shared-lib", "package.json"))).toBe( + true, + ); + expect( + await exists(join(projectDir, "node_modules", "plugin", "node_modules", "shared-dev-lib", "package.json")), + ).toBe(true); }); it("does not install transitive file: dependencies with overlong folder targets", async () => { @@ -10478,48 +10553,124 @@ for (const field of ["resolutions", "overrides"]) { expect(await exists(join(projectDir, "node_modules", "shared", "package.json"))).toBe(true); }); - it(`still rejects transitive file: dependencies that escape their package when a different name is in "${field}"`, async () => { + it(`still rejects a registry package's transitive file: dependency that escapes when a different name is in "${field}"`, async () => { // An override for a different name must not whitelist an unrelated - // transitive file: dependency that points outside its package. - using dir = tempDir("override-file-dep-unrelated", { - "secret/credentials.txt": "do-not-link-me", - "shared/package.json": JSON.stringify({ name: "shared", version: "1.0.0" }), - "project/package.json": JSON.stringify({ - name: "my-app", - version: "1.0.0", - dependencies: { - "evil-folder-dep": "file:./evil-folder-dep", - }, - [field]: { - shared: "file:../shared", - }, - }), - "project/evil-folder-dep/index.js": "module.exports = 1;", - "project/evil-folder-dep/package.json": JSON.stringify({ - name: "evil-folder-dep", - version: "1.0.0", - dependencies: { - loot: "file:../../secret", - }, - }), - }); - const projectDir = join(String(dir), "project"); + // transitive file: dependency of a registry package that points outside + // its package. + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + setContextHandler( + ctx, + dummyRegistryForContext(ctx, urls, { + "0.0.3": { + dependencies: { + loot: "file:../../secret", + }, + }, + }), + ); + await write( + join(ctx.package_dir, "shared", "package.json"), + JSON.stringify({ name: "shared", version: "1.0.0" }), + ); + await writeFile( + join(ctx.package_dir, "package.json"), + JSON.stringify({ + name: "my-app", + version: "1.0.0", + dependencies: { + baz: "0.0.3", + }, + [field]: { + shared: "file:./shared", + }, + }), + ); - const { stdout, stderr, exited } = spawn({ - cmd: [bunExe(), "install"], - cwd: projectDir, - stdout: "pipe", - stdin: "pipe", - stderr: "pipe", - env, + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "pipe", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); + + expect(await exists(join(ctx.package_dir, "node_modules", "loot"))).toBe(false); + expect(await exists(join(ctx.package_dir, "node_modules", "baz", "node_modules", "loot"))).toBe(false); + expect(err).toContain("Could not find package.json"); + expect(out).not.toContain("2 packages installed"); + expect(exitCode).toBe(1); }); - const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); + }); - expect(await exists(join(projectDir, "node_modules", "loot"))).toBe(false); - expect(await exists(join(projectDir, "node_modules", "evil-folder-dep", "node_modules", "loot"))).toBe(false); - expect(err).toContain("Could not find package.json"); - expect(out).not.toContain("2 packages installed"); - expect(exitCode).toBe(1); + it(`applies a root "${field}" file: path to a registry package's dependency`, async () => { + // overrides/resolutions are declared in the root package.json, so a file: + // path applied through them is trusted even for a dependency declared by a + // registry package (which is otherwise constrained; see the test above). + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + using dir = tempDir(`override-registry-file-dep-${field}`, { + "shared/package.json": JSON.stringify({ name: "shared", version: "1.0.0" }), + "shared/index.js": "module.exports = 'shared';", + }); + const sharedDir = join(String(dir), "shared"); + setContextHandler( + ctx, + dummyRegistryForContext(ctx, urls, { + "0.0.3": { + dependencies: { + shared: "1.0.0", + }, + }, + }), + ); + await writeFile( + join(ctx.package_dir, "package.json"), + JSON.stringify({ + name: "my-app", + version: "1.0.0", + dependencies: { + baz: "0.0.3", + }, + [field]: { + shared: "file:" + sharedDir.replaceAll("\\", "/"), + }, + }), + ); + + // The first install resolves the override (enqueue path); the second + // starts from the saved lockfile with an empty node_modules (installer path). + for (let i = 0; i < 2; i++) { + if (i === 1) { + await rm(join(ctx.package_dir, "node_modules"), { recursive: true, force: true }); + } + const { stdout, stderr, exited } = spawn({ + cmd: i === 0 ? [bunExe(), "install", "--save-text-lockfile"] : [bunExe(), "install"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "pipe", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); + + expect(err).not.toContain("Could not find package.json"); + expect(err).not.toContain("failed to resolve"); + expect(err).not.toContain("unsafe folder path"); + expect(err).not.toContain("refusing to install"); + expect(out).toContain("baz"); + expect(exitCode).toBe(0); + } + + // The registry package's dependency resolved to the override's file: path + // (no registry request for it was made). + expect(urls.filter(url => url.includes("shared"))).toEqual([]); + const lock = (await file(join(ctx.package_dir, "bun.lock")).text()).replaceAll("\\\\", "/"); + expect(lock).toContain('"baz/shared"'); + expect(lock).toContain("shared@file:"); + }); }); const nestedRule = (value: string) => From f6e0ae6874e132884a70c4a6d74cd148f5c8c5f2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 08:56:14 +0000 Subject: [PATCH 2/9] test: re-anchor the nested override escape test on a registry parent; cover the local parent #38333 added a test that a nested overrides/resolutions rule pointing at a file: path outside the project is rejected. Its parent is a file: package of the root, which this change exempts from the escape check, so the test now uses a registry parent (the case the name-based check still rejects) and a second test asserts that the local parent installs. --- test/cli/install/bun-install.test.ts | 138 ++++++++++++++++++++++----- 1 file changed, 114 insertions(+), 24 deletions(-) diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index 9df34190a485..b390c20b4758 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -10673,11 +10673,82 @@ for (const field of ["resolutions", "overrides"]) { }); }); - const nestedRule = (value: string) => - field === "overrides" ? { "pkg-a": { shared: value } } : { "pkg-a/shared": value }; + const nestedRule = (parent: string, value: string) => + field === "overrides" ? { [parent]: { shared: value } } : { [`${parent}/shared`]: value }; - it(`rejects a nested "${field}" rule pointing at a file: path outside the project`, async () => { - using dir = tempDir("nested-override-file-dep-outside", { + it(`rejects a nested "${field}" rule pointing at a file: path outside the project for a registry package's dependency`, async () => { + // A nested rule only replaces one parent -> child edge, so it does not + // make every edge of that name root authored. The trust checks therefore + // only consult plain rules, and a nested file: path that escapes the + // project is still rejected when the parent is a registry package. + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + setContextHandler( + ctx, + dummyRegistryForContext(ctx, urls, { + "0.0.3": { + dependencies: { + shared: "1.0.0", + }, + }, + }), + ); + // The project is a subdirectory so that `../shared` exists and is outside it. + const projectDir = join(ctx.package_dir, "project"); + await write( + join(ctx.package_dir, "shared", "package.json"), + JSON.stringify({ name: "shared", version: "1.0.0" }), + ); + await write(join(ctx.package_dir, "shared", "index.js"), "module.exports = 'shared';"); + await write( + join(projectDir, "bunfig.toml"), + ` +[install] +cache = false +registry = "${ctx.registry_url}" +`, + ); + await write( + join(projectDir, "package.json"), + JSON.stringify({ + name: "my-app", + version: "1.0.0", + dependencies: { + baz: "0.0.3", + }, + [field]: nestedRule("baz", "file:../shared"), + }), + ); + + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd: projectDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]); + + // stderr also carries the progress lines of the registry fetch of `baz`. + expect(err.split(/\r?\n/).filter(line => line.startsWith("error:"))).toEqual([ + 'error: Could not find package.json for "file:../shared" dependency "shared"', + "error: shared@1.0.0 failed to resolve", + ]); + expect(out).not.toContain("packages installed"); + expect(await exists(join(projectDir, "node_modules", "shared"))).toBe(false); + expect(await exists(join(projectDir, "node_modules", "baz", "node_modules", "shared"))).toBe(false); + expect(await exists(join(projectDir, "bun.lock"))).toBe(false); + expect(urls.filter(url => url.includes("shared"))).toEqual([]); + expect(exitCode).toBe(1); + }); + }); + + it(`installs a nested "${field}" rule pointing at a file: path outside the project for a local file: package's dependency`, async () => { + // The parent is a file: package of the root, so its dependencies' file: + // paths are trusted like root dependencies (see the transitive file: tests + // above), nested rule or not. + using dir = tempDir("nested-override-file-dep-outside-local", { "shared/package.json": JSON.stringify({ name: "shared", version: "1.0.0" }), "shared/index.js": "module.exports = 'shared';", "project/package.json": JSON.stringify({ @@ -10686,7 +10757,7 @@ for (const field of ["resolutions", "overrides"]) { dependencies: { "pkg-a": "file:./pkg-a", }, - [field]: nestedRule("file:../shared"), + [field]: nestedRule("pkg-a", "file:../shared"), }), "project/pkg-a/package.json": JSON.stringify({ name: "pkg-a", @@ -10699,25 +10770,44 @@ for (const field of ["resolutions", "overrides"]) { }); const projectDir = join(String(dir), "project"); - await using proc = spawn({ - cmd: [bunExe(), "install"], - cwd: projectDir, - stdout: "pipe", - stdin: "ignore", - stderr: "pipe", - env, - }); - const [err, out, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]); + // The first pass resolves the rule (enqueue path); the second installs + // from the lockfile it wrote (package installer path). + for (const args of [["install"], ["install", "--frozen-lockfile"]]) { + await rm(join(projectDir, "node_modules"), { recursive: true, force: true }); - expect(normalizeBunSnapshot(err, projectDir)).toMatchInlineSnapshot(` - "error: Could not find package.json for "file:../shared" dependency "shared" - error: shared@1.0.0 failed to resolve" - `); - expect(out).not.toContain("packages installed"); - expect(await exists(join(projectDir, "node_modules", "shared"))).toBe(false); - expect(await exists(join(projectDir, "node_modules", "pkg-a", "node_modules", "shared"))).toBe(false); - expect(await exists(join(projectDir, "bun.lock"))).toBe(false); - expect(exitCode).toBe(1); + await using proc = spawn({ + cmd: [bunExe(), ...args], + cwd: projectDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]); + + expect(err).not.toContain("error:"); + expect(out).toContain("2 packages installed"); + expect(exitCode).toBe(0); + + await using runProc = spawn({ + cmd: [bunExe(), "-e", "console.log(require('pkg-a'))"], + cwd: projectDir, + env, + stdout: "pipe", + stderr: "pipe", + }); + const [runOut, runErr, runExit] = await Promise.all([ + runProc.stdout.text(), + runProc.stderr.text(), + runProc.exited, + ]); + expect(runErr).toBe(""); + expect(runOut).toBe("shared\n"); + expect(runExit).toBe(0); + } + + const lock = (await file(join(projectDir, "bun.lock")).text()).replaceAll("\\\\", "/"); + expect(lock).toContain('"pkg-a/shared": ["shared@file:../shared", {}]'); }); it(`installs a nested "${field}" rule pointing at a file: path inside the project`, async () => { @@ -10728,7 +10818,7 @@ for (const field of ["resolutions", "overrides"]) { dependencies: { "pkg-a": "file:./pkg-a", }, - [field]: nestedRule("file:./vendor/shared"), + [field]: nestedRule("pkg-a", "file:./vendor/shared"), }), "vendor/shared/package.json": JSON.stringify({ name: "shared", version: "2.0.0" }), "vendor/shared/index.js": "module.exports = 'vendored shared';", From 8cd2a5eba02e02e6386e3cdbbd194ad8a9196aba Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 09:01:56 +0000 Subject: [PATCH 3/9] install: shorten the local-package trust comments --- src/install/PackageInstaller.rs | 4 ---- .../PackageManager/PackageManagerEnqueue.rs | 4 ---- src/install/lockfile.rs | 14 +++++--------- src/install/resolution.rs | 10 ++++------ 4 files changed, 9 insertions(+), 23 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 628327353ebf..782f8e1a71be 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -1455,10 +1455,6 @@ impl<'a> PackageInstaller<'a> { installer.cache_dir = Fd::cwd(); } else { // transitive folder dependencies are not hoisted - // - // Only constrain `file:` targets declared by remote packages - // (registry, git, tarball); the project's own local packages are - // user authored and may point anywhere, like root dependencies. if folder.len() >= self.folder_path_buf.len() || (bin::bin_target_escapes_package_dir(folder) && !self diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index 8919c2455836..21d09be532ad 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -2849,10 +2849,6 @@ fn get_or_put_resolved_package( } // transitive folder dependencies do not have their dependencies resolved - // - // Only constrain `file:` targets declared by remote packages - // (registry, git, tarball); the project's own local packages are - // user authored and may point anywhere, like root dependencies. if crate::bin::bin_target_escapes_package_dir(this.lockfile.str(&folder)) && !this.lockfile.is_dependency_of_local_package(dependency_id) { diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index b2138c673d02..0a63602a663e 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -860,16 +860,12 @@ impl Lockfile { 0 } - /// Is dependency `id` declared by a local package (see - /// [`resolution::Tag::is_local_package`])? Its `file:` paths are then - /// trusted like root dependencies; paths declared by remote packages - /// (registry, git, tarball) are not. + /// Is dependency `id` declared by a local package ([`resolution::Tag::is_local_package`])? + /// Its `file:` path is then trusted like a root dependency's. /// - /// A `Folder` declaring package is always itself declared by the root or a - /// workspace: only those folder dependencies have their package.json - /// parsed. A folder dependency of any other package is recorded as a - /// resolution with no dependencies of its own (see the transitive folder - /// branch in `get_or_put_resolved_package`), so it never declares one. + /// A `Folder` parent is itself always declared by the root or a workspace: a + /// folder dependency of any other package is recorded without dependencies + /// (the transitive folder branch of `get_or_put_resolved_package`). pub(crate) fn is_dependency_of_local_package(&self, id: DependencyID) -> bool { let Some(parent_id) = self.get_parent_pkg_of_dependency(id) else { return false; diff --git a/src/install/resolution.rs b/src/install/resolution.rs index 855ee6c29296..748e9084066e 100644 --- a/src/install/resolution.rs +++ b/src/install/resolution.rs @@ -1014,12 +1014,10 @@ impl Tag { self == Tag::Git || self == Tag::Github } - /// A local package: the root, a workspace, or a `file:` folder package. - /// Their package.jsons are authored by the user (or by files the user - /// explicitly points the project at), unlike content fetched from a - /// registry, git host, or tarball. Local packages get - /// `local_package_features` and their `file:` dependency paths are trusted - /// like root dependencies. + /// The root, a workspace, or a `file:` folder package: a package.json the + /// user wrote or pointed the project at, not content fetched from a registry, + /// git host, or tarball. Gets `local_package_features`; its `file:` paths + /// are trusted like root dependencies. pub(crate) fn is_local_package(self) -> bool { self == Tag::Root || self == Tag::Workspace || self == Tag::Folder } From eff80a0f44f264b719fd4606d271ef3af7ea5845 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 09:20:21 +0000 Subject: [PATCH 4/9] install: fold the folder path trust rule into Lockfile::is_trusted_folder_dependency The resolve and install sites evaluated the same predicate (declared by a local package, or named by a plain override) in two copies. One method now holds it. The override test also asserts the file: package is linked. --- src/install/PackageInstaller.rs | 17 +---------------- .../PackageManager/PackageManagerEnqueue.rs | 15 ++------------- src/install/lockfile.rs | 15 ++++++++++++++- test/cli/install/bun-install.test.ts | 6 +++++- 4 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 782f8e1a71be..75472ae62335 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -1457,22 +1457,7 @@ impl<'a> PackageInstaller<'a> { // transitive folder dependencies are not hoisted if folder.len() >= self.folder_path_buf.len() || (bin::bin_target_escapes_package_dir(folder) - && !self - .lockfile() - .is_dependency_of_local_package(dependency_id) - && { - // overrides/resolutions are only ever parsed from the root - // package.json, so a folder path that reached here via an - // override was written by the user and is trusted the same - // as a direct dependency of the root. - let dep = &self.lockfile().buffers.dependencies.as_slice() - [dependency_id as usize]; - !self.lockfile().overrides.contains_name( - dep.name_hash, - dep.name.slice(string_buf!()), - string_buf!(), - ) - }) + && !self.lockfile().is_trusted_folder_dependency(dependency_id)) { if log_level != Options::LogLevel::Silent { bun_core::pretty_errorln!( diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index 21d09be532ad..bce4bec8097f 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -2850,20 +2850,9 @@ fn get_or_put_resolved_package( // transitive folder dependencies do not have their dependencies resolved if crate::bin::bin_target_escapes_package_dir(this.lockfile.str(&folder)) - && !this.lockfile.is_dependency_of_local_package(dependency_id) + && !this.lockfile.is_trusted_folder_dependency(dependency_id) { - // overrides/resolutions are only ever parsed from the root - // package.json, so a folder path that reached here via an - // override was written by the user and is trusted the same - // as a direct dependency of the root. - let buf = this.lockfile.buffers.string_bytes.as_slice(); - if !this.lockfile.overrides.contains_name( - dependency.name_hash, - dependency.name.slice(buf), - buf, - ) { - break 'res FolderResolutionValue::Err(crate::Error::MissingPackageJSON); - } + break 'res FolderResolutionValue::Err(crate::Error::MissingPackageJSON); } let mut package = Package::default(); diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index 0a63602a663e..efce728adeb3 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -861,7 +861,6 @@ impl Lockfile { } /// Is dependency `id` declared by a local package ([`resolution::Tag::is_local_package`])? - /// Its `file:` path is then trusted like a root dependency's. /// /// A `Folder` parent is itself always declared by the root or a workspace: a /// folder dependency of any other package is recorded without dependencies @@ -875,6 +874,20 @@ impl Lockfile { .is_local_package() } + /// May the folder path of dependency `id` leave its package directory? Yes + /// when a local package declares the dependency, or when a plain override or + /// resolution supplies the path (those are only ever parsed from the root + /// package.json). Both are user authored, like a root `file:` dependency. + pub(crate) fn is_trusted_folder_dependency(&self, id: DependencyID) -> bool { + if self.is_dependency_of_local_package(id) { + return true; + } + let buf = self.buffers.string_bytes.as_slice(); + let dep = &self.buffers.dependencies[id as usize]; + self.overrides + .contains_name(dep.name_hash, dep.name.slice(buf), buf) + } + /// Does this tree id belong to a workspace (including workspace root)? /// TODO(dylan-conway) fix! pub(crate) fn is_workspace_tree_id(&self, id: tree::Id) -> bool { diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index b390c20b4758..25dcb2679bc3 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -10665,11 +10665,15 @@ for (const field of ["resolutions", "overrides"]) { } // The registry package's dependency resolved to the override's file: path - // (no registry request for it was made). + // (no registry request for it was made) and is linked under baz, since + // transitive folder dependencies are not hoisted. expect(urls.filter(url => url.includes("shared"))).toEqual([]); const lock = (await file(join(ctx.package_dir, "bun.lock")).text()).replaceAll("\\\\", "/"); expect(lock).toContain('"baz/shared"'); expect(lock).toContain("shared@file:"); + expect(await exists(join(ctx.package_dir, "node_modules", "baz", "node_modules", "shared", "index.js"))).toBe( + true, + ); }); }); From fe8bdff8398936018e5842a5fd438b770dc92534 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 09:23:10 +0000 Subject: [PATCH 5/9] test: point the unrelated-override escape test at an installable folder With a relative path that does not exist, a bypassed check would still fail on the missing folder. An absolute path to a fixture with a package.json makes a bypass visible: loot would be linked. --- test/cli/install/bun-install.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index 25dcb2679bc3..85be8980b2a0 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -10556,15 +10556,21 @@ for (const field of ["resolutions", "overrides"]) { it(`still rejects a registry package's transitive file: dependency that escapes when a different name is in "${field}"`, async () => { // An override for a different name must not whitelist an unrelated // transitive file: dependency of a registry package that points outside - // its package. + // its package. The target exists and is installable, so a bypassed check + // would link it. await withContext(defaultOpts, async ctx => { const urls: string[] = []; + using dir = tempDir("registry-transitive-file-dep-unrelated-override", { + "secret/package.json": JSON.stringify({ name: "loot", version: "1.0.0" }), + "secret/credentials.txt": "do-not-link-me", + }); + const secretDir = join(String(dir), "secret"); setContextHandler( ctx, dummyRegistryForContext(ctx, urls, { "0.0.3": { dependencies: { - loot: "file:../../secret", + loot: "file:" + secretDir.replaceAll("\\", "/"), }, }, }), From 6d7fcc3a80014b622fb8e80a020575cb32c3cd65 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 09:45:50 +0000 Subject: [PATCH 6/9] install: migrate a file: dependency declared by an out-of-tree file: package The package-lock.json migration has its own escape check for transitive folder targets. It exempted root and workspace parents only, so a file: path written in an out-of-tree file: package's own package.json was skipped with a warning, while a fresh bun install now resolves it. Trust that case: a Folder parent whose dependency spec is itself a file: path. A registry spec that npm resolved to a bare folder beside the package stays skipped, as before. --- src/install/migration/npm_lock.rs | 6 ++++ test/cli/install/migration/migrate.test.ts | 41 ++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/install/migration/npm_lock.rs b/src/install/migration/npm_lock.rs index 263077dfa258..bc396c7162cf 100644 --- a/src/install/migration/npm_lock.rs +++ b/src/install/migration/npm_lock.rs @@ -765,9 +765,15 @@ impl<'a> Migrator<'a> { } let version_tag = version.tag; + // A `file:` path in a `file:` package's own package.json is user authored + // (`Lockfile::is_trusted_folder_dependency`); a bare folder npm found + // beside the package for a registry spec is not. + let declares_folder = + res_tag == resolution::Tag::Folder && version_tag == DepTag::Folder; let mut found = self.find_target(key, name); if let Some((t, through_link)) = found && !is_local + && !declares_folder && self.is_external_folder(t, through_link) { self.skip_external(t, name); diff --git a/test/cli/install/migration/migrate.test.ts b/test/cli/install/migration/migrate.test.ts index 0a90dee296c5..a56f9023315e 100644 --- a/test/cli/install/migration/migrate.test.ts +++ b/test/cli/install/migration/migrate.test.ts @@ -1643,6 +1643,47 @@ describe("package-lock.json migration fixes", () => { expect(storeEntries(project.dir)).toStrictEqual(linker === "isolated" ? [project.o.replaceAll(/[/:]/g, "+")] : []); }); + // A file: path written in an out-of-tree file: package's own package.json is trusted like a + // root dependency, the same as a fresh `bun install` does, even when it leaves the project. + test.concurrent("a file: dependency declared by an out-of-tree file: package migrates", async () => { + const dependencies = { plugin: "file:../packages/plugin" }; + using copy = tempDir("npm-migrate-folder-declares-file-dep", { + "root/package.json": JSON.stringify({ name: "root", dependencies }), + "root/package-lock.json": npmLock("root", { + "": { name: "root", dependencies }, + "../packages/plugin": { version: "1.0.0", dependencies: { "shared-lib": "file:../shared-lib" } }, + "../packages/plugin/node_modules/shared-lib": { resolved: "../packages/shared-lib", link: true }, + "../packages/shared-lib": { version: "1.0.0" }, + "node_modules/plugin": { resolved: "../packages/plugin", link: true }, + }), + "packages/plugin/package.json": JSON.stringify({ + name: "plugin", + version: "1.0.0", + dependencies: { "shared-lib": "file:../shared-lib" }, + }), + "packages/shared-lib/package.json": JSON.stringify({ name: "shared-lib", version: "1.0.0" }), + }); + const dir = join(String(copy), "root"); + writeExtra(dir, {}); + + const { stderr, exitCode, lock } = await migrate(dir); + expect(stderr).not.toContain("outside the project"); + expect(exitCode).toBe(0); + expect(lock.packages).toStrictEqual({ + plugin: ["plugin@file:../packages/plugin", { dependencies: { "shared-lib": "file:../shared-lib" } }], + "plugin/shared-lib": ["shared-lib@file:../packages/shared-lib", {}], + }); + await frozen(dir); + + // The same lockfile a fresh `bun install` writes, and the installer links the nested package. + const install = await run(dir, "install", "--frozen-lockfile"); + expect(install.stderr).not.toContain("error"); + expect(install.exitCode).toBe(0); + expect( + await Bun.file(join(dir, "node_modules", "plugin", "node_modules", "shared-lib", "package.json")).json(), + ).toHaveProperty("name", "shared-lib"); + }); + test.concurrent("lockfileVersion 5 is refused, and install falls back to a fresh resolve", async () => { using dir = synthetic("npm-migrate-v5", { "package.json": JSON.stringify({ name: "v5", dependencies: { "dep-1": "file:dep-1" } }), From 6a9b9b9110993227cd3db5e9ff3a5cf9954ad5b7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 09:47:52 +0000 Subject: [PATCH 7/9] install: shorten the migration trust comment --- src/install/migration/npm_lock.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/install/migration/npm_lock.rs b/src/install/migration/npm_lock.rs index bc396c7162cf..1bd8b191b061 100644 --- a/src/install/migration/npm_lock.rs +++ b/src/install/migration/npm_lock.rs @@ -765,9 +765,8 @@ impl<'a> Migrator<'a> { } let version_tag = version.tag; - // A `file:` path in a `file:` package's own package.json is user authored - // (`Lockfile::is_trusted_folder_dependency`); a bare folder npm found - // beside the package for a registry spec is not. + // Trust a `file:` spec written in a `file:` package's own package.json, not a + // bare folder npm found for a registry spec (`is_trusted_folder_dependency`). let declares_folder = res_tag == resolution::Tag::Folder && version_tag == DepTag::Folder; let mut found = self.find_target(key, name); From 64b811ae440485140e33608494fa78321e507d27 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 10:47:42 +0000 Subject: [PATCH 8/9] install: anchor folder path trust at the root or a workspace A Folder parent was trusted on its tag alone. Bun's resolver only records dependencies for file: packages the root or a workspace declares, but a migrated lockfile can carry dependencies for a folder that a registry package ships (package-lock.json records them). Such a parent could then declare an escaping file: path that the installer accepted. is_dependency_of_local_package now requires that the root or a workspace resolves to the Folder parent. The npm migration applies the same rule: it records the targets the root or a workspace declares directly and trusts a file: spec only under one of those. --- src/install/lockfile.rs | 34 +++++++--- src/install/migration/npm_lock.rs | 19 ++++-- src/install/resolution.rs | 7 +-- test/cli/install/bun-install.test.ts | 72 ++++++++++++++++++++++ test/cli/install/migration/migrate.test.ts | 35 +++++++++++ 5 files changed, 151 insertions(+), 16 deletions(-) diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index efce728adeb3..59c81150ce65 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -860,18 +860,36 @@ impl Lockfile { 0 } - /// Is dependency `id` declared by a local package ([`resolution::Tag::is_local_package`])? - /// - /// A `Folder` parent is itself always declared by the root or a workspace: a - /// folder dependency of any other package is recorded without dependencies - /// (the transitive folder branch of `get_or_put_resolved_package`). + /// Does the root or a workspace depend on package `id` directly? + pub(crate) fn is_workspace_declared_package(&self, id: PackageID) -> bool { + let packages = self.packages.slice(); + let resolutions = self.buffers.resolutions.as_slice(); + for (pkg_id, res_list) in packages.items_resolutions().iter().enumerate() { + let tag = packages.items_resolution()[pkg_id].tag; + if tag != ResolutionTag::Workspace && tag != ResolutionTag::Root { + continue; + } + if res_list.get(resolutions).contains(&id) { + return true; + } + } + false + } + + /// Is dependency `id` declared by the root, a workspace, or a `file:` package + /// that one of them depends on directly? Bun's resolver only parses the + /// package.json of such `file:` packages, but a migrated lockfile may carry + /// dependencies for a folder that a registry package shipped, so the anchor + /// is checked rather than assumed. pub(crate) fn is_dependency_of_local_package(&self, id: DependencyID) -> bool { let Some(parent_id) = self.get_parent_pkg_of_dependency(id) else { return false; }; - self.packages.items_resolution()[parent_id as usize] - .tag - .is_local_package() + match self.packages.items_resolution()[parent_id as usize].tag { + ResolutionTag::Root | ResolutionTag::Workspace => true, + ResolutionTag::Folder => self.is_workspace_declared_package(parent_id), + _ => false, + } } /// May the folder path of dependency `id` leave its package directory? Yes diff --git a/src/install/migration/npm_lock.rs b/src/install/migration/npm_lock.rs index 1bd8b191b061..d8e49512fc61 100644 --- a/src/install/migration/npm_lock.rs +++ b/src/install/migration/npm_lock.rs @@ -108,6 +108,8 @@ struct Migrator<'a> { link_entries: DynamicBitSet, shadowed: DynamicBitSet, skipped_external: DynamicBitSet, + /// Targets the root or a workspace depends on directly. + local_declared: DynamicBitSet, entry_package_ids: Vec, queue: Vec<(u32, PackageID)>, probe: Vec, @@ -142,6 +144,7 @@ pub(super) fn migrate_packages( link_entries: DynamicBitSet::init_empty(entry_count)?, shadowed: DynamicBitSet::init_empty(entry_count)?, skipped_external: DynamicBitSet::init_empty(entry_count)?, + local_declared: DynamicBitSet::init_empty(entry_count)?, entry_package_ids: vec![INVALID_PACKAGE_ID; entry_count], queue: Vec::new(), probe: Vec::new(), @@ -765,11 +768,19 @@ impl<'a> Migrator<'a> { } let version_tag = version.tag; - // Trust a `file:` spec written in a `file:` package's own package.json, not a - // bare folder npm found for a registry spec (`is_trusted_folder_dependency`). - let declares_folder = - res_tag == resolution::Tag::Folder && version_tag == DepTag::Folder; + // A `file:` spec in a root-declared `file:` package's own package.json is + // trusted like a root dependency (`is_trusted_folder_dependency`). A folder + // a registry package ships, or a bare folder npm found for a registry spec, + // is not. + let declares_folder = res_tag == resolution::Tag::Folder + && version_tag == DepTag::Folder + && self.local_declared.is_set(j as usize); let mut found = self.find_target(key, name); + if let Some((t, _)) = found + && is_local + { + self.local_declared.set(t as usize); + } if let Some((t, through_link)) = found && !is_local && !declares_folder diff --git a/src/install/resolution.rs b/src/install/resolution.rs index 748e9084066e..f6b89e01bb2f 100644 --- a/src/install/resolution.rs +++ b/src/install/resolution.rs @@ -1014,10 +1014,9 @@ impl Tag { self == Tag::Git || self == Tag::Github } - /// The root, a workspace, or a `file:` folder package: a package.json the - /// user wrote or pointed the project at, not content fetched from a registry, - /// git host, or tarball. Gets `local_package_features`; its `file:` paths - /// are trusted like root dependencies. + /// The root, a workspace, or a `file:` folder package: a package.json on + /// disk rather than content fetched from a registry, git host, or tarball. + /// These get `local_package_features`. pub(crate) fn is_local_package(self) -> bool { self == Tag::Root || self == Tag::Workspace || self == Tag::Folder } diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index 85be8980b2a0..791a8587e777 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -10295,6 +10295,78 @@ it("does not install a registry package's transitive file: dependency that point }); }); +it("refuses to install an escaping file: dependency that a registry package's own folder declares in the lockfile", async () => { + // Bun's resolver records no dependencies for a folder that a registry package + // ships, but a lockfile written elsewhere (a migration) can. Trust for a + // folder parent is anchored at the root or a workspace, so the installer still + // refuses the escaping path. + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + using dir = tempDir("registry-folder-declares-file-dep", { + "secret/package.json": JSON.stringify({ name: "loot", version: "1.0.0" }), + "secret/credentials.txt": "do-not-link-me", + }); + const secretDir = join(String(dir), "secret").replaceAll("\\", "/"); + setContextHandler( + ctx, + dummyRegistryForContext(ctx, urls, { + "0.0.3": { + dependencies: { + inner: "file:./inner", + }, + }, + }), + ); + await writeFile( + join(ctx.package_dir, "package.json"), + JSON.stringify({ + name: "my-app", + version: "1.0.0", + dependencies: { + baz: "0.0.3", + }, + }), + ); + await writeFile( + join(ctx.package_dir, "bun.lock"), + JSON.stringify({ + lockfileVersion: 1, + workspaces: { + "": { + name: "my-app", + dependencies: { + baz: "0.0.3", + }, + }, + }, + packages: { + "baz": ["baz@0.0.3", `${ctx.registry_url}baz-0.0.3.tgz`, { dependencies: { inner: "file:./inner" } }, ""], + "baz/inner": ["inner@file:node_modules/baz/inner", { dependencies: { loot: "file:" + secretDir } }], + "baz/inner/loot": ["loot@file:" + secretDir, {}], + }, + }), + ); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "pipe", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); + + expect(err).toContain("refusing to install dependency loot with unsafe folder path"); + expect(await exists(join(ctx.package_dir, "node_modules", "loot"))).toBe(false); + expect( + await exists(join(ctx.package_dir, "node_modules", "baz", "node_modules", "inner", "node_modules", "loot")), + ).toBe(false); + expect(out).not.toContain("3 packages installed"); + expect(exitCode).toBe(1); + }); +}); + it("installs transitive file: dependencies of a local file: package that point outside the project", async () => { // A file: package referenced by the root package.json lives outside the // project and declares its own relative folder dependencies that also land diff --git a/test/cli/install/migration/migrate.test.ts b/test/cli/install/migration/migrate.test.ts index a56f9023315e..0ba190efb16f 100644 --- a/test/cli/install/migration/migrate.test.ts +++ b/test/cli/install/migration/migrate.test.ts @@ -1684,6 +1684,41 @@ describe("package-lock.json migration fixes", () => { ).toHaveProperty("name", "shared-lib"); }); + // A folder that a registry package ships inside itself is a `Folder` entry too, but the registry + // package declared it, so a `file:` path in its package.json that leaves the project is still skipped. + test.concurrent("a file: dependency declared by a registry package's own folder is still skipped", async () => { + const dependencies = { evil: "1.0.0" }; + using copy = tempDir("npm-migrate-registry-folder-declares-file-dep", { + "root/package.json": JSON.stringify({ name: "root", dependencies }), + "root/package-lock.json": npmLock("root", { + "": { name: "root", dependencies }, + "node_modules/evil": { + version: "1.0.0", + resolved: `${OFFLINE_REGISTRY}evil/-/evil-1.0.0.tgz`, + dependencies: { inner: "file:./inner" }, + }, + "node_modules/evil/node_modules/inner": { resolved: "node_modules/evil/inner", link: true }, + "node_modules/evil/inner": { version: "1.0.0", dependencies: { loot: "file:../../../../secret" } }, + "node_modules/evil/inner/node_modules/loot": { resolved: "../secret", link: true }, + "../secret": { version: "1.0.0" }, + }), + "secret/package.json": JSON.stringify({ name: "loot", version: "1.0.0" }), + "secret/credentials.txt": "do-not-link-me", + }); + const dir = join(String(copy), "root"); + writeExtra(dir, {}); + + const { stderr, exitCode, text, lock } = await migrate(dir); + expect(stderr).toContain( + 'skipped "loot" from package-lock.json: transitive folder dependency "../secret" is outside the project', + ); + expect(exitCode).toBe(0); + expect(Object.keys(lock.packages).sort()).toStrictEqual(["evil", "evil/inner"]); + expect(lock.packages["evil/inner"]).toStrictEqual(["inner@file:node_modules/evil/inner", {}]); + expect(text).not.toContain("../secret"); + expect(text).not.toContain("loot"); + }); + test.concurrent("lockfileVersion 5 is refused, and install falls back to a fresh resolve", async () => { using dir = synthetic("npm-migrate-v5", { "package.json": JSON.stringify({ name: "v5", dependencies: { "dep-1": "file:dep-1" } }), From b5ee3466a3133e2fe952bf094c1a0aabf3f38dba Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 27 Aug 2026 10:49:53 +0000 Subject: [PATCH 9/9] install: shorten the trust anchor comments --- src/install/lockfile.rs | 6 ++---- src/install/migration/npm_lock.rs | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index 59c81150ce65..30b7e495c2cb 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -877,10 +877,8 @@ impl Lockfile { } /// Is dependency `id` declared by the root, a workspace, or a `file:` package - /// that one of them depends on directly? Bun's resolver only parses the - /// package.json of such `file:` packages, but a migrated lockfile may carry - /// dependencies for a folder that a registry package shipped, so the anchor - /// is checked rather than assumed. + /// one of them depends on directly? Checked, not assumed: a migrated lockfile + /// can carry dependencies for a folder that a registry package shipped. pub(crate) fn is_dependency_of_local_package(&self, id: DependencyID) -> bool { let Some(parent_id) = self.get_parent_pkg_of_dependency(id) else { return false; diff --git a/src/install/migration/npm_lock.rs b/src/install/migration/npm_lock.rs index d8e49512fc61..f1e28d8fa5a7 100644 --- a/src/install/migration/npm_lock.rs +++ b/src/install/migration/npm_lock.rs @@ -768,10 +768,8 @@ impl<'a> Migrator<'a> { } let version_tag = version.tag; - // A `file:` spec in a root-declared `file:` package's own package.json is - // trusted like a root dependency (`is_trusted_folder_dependency`). A folder - // a registry package ships, or a bare folder npm found for a registry spec, - // is not. + // Trust a `file:` spec only in a `file:` package the root or a workspace + // declares, not in a folder a registry package ships (`is_trusted_folder_dependency`). let declares_folder = res_tag == resolution::Tag::Folder && version_tag == DepTag::Folder && self.local_declared.is_set(j as usize);