From a9ffc0e23cf65b515e146b4b0dc88849da536906 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Fri, 7 Aug 2026 17:38:37 +0200 Subject: [PATCH 1/7] test(install): isolate bunx registry state Give concurrent bunx tests independent registry handlers, request counters, and package directories so one case cannot consume another case's fixture state. Dispose temporary install/cache directories after the suite instead of leaving debug binaries behind. Also make the user-agent assertion independent of inherited npm config and the debug-only version suffix. --- test/cli/install/bunx.test.ts | 424 ++++++++++++++++------------- test/cli/install/dummy.registry.ts | 79 +++--- 2 files changed, 281 insertions(+), 222 deletions(-) diff --git a/test/cli/install/bunx.test.ts b/test/cli/install/bunx.test.ts index ec0a9fb5a508..c0e7b61aae4b 100644 --- a/test/cli/install/bunx.test.ts +++ b/test/cli/install/bunx.test.ts @@ -1,23 +1,45 @@ import { spawn } from "bun"; import { afterAll, beforeAll, beforeEach, describe, expect, it, setDefaultTimeout } from "bun:test"; import { mkdir, rm, writeFile } from "fs/promises"; -import { bunEnv, bunExe, isWindows, readdirSorted, tmpdirSync } from "harness"; +import { bunEnv, bunExe, isWindows, readdirSorted, tempDir, tmpdirSync } from "harness"; import { chmodSync, copyFileSync, readdirSync, symlinkSync } from "node:fs"; import { tmpdir } from "os"; import { delimiter, join, resolve } from "path"; -import { dummyAfterAll, dummyBeforeAll, dummyBeforeEach, dummyRegistry, getPort, setHandler } from "./dummy.registry"; +import type { TestContext } from "./dummy.registry"; +import { + createTestContext, + destroyTestContext, + dummyAfterAll, + dummyBeforeAll, + dummyBeforeEach, + dummyRegistry, + dummyRegistryForContext, + getPort, + setHandler, + setContextHandler, +} from "./dummy.registry"; setDefaultTimeout(1000 * 60 * 5); let x_dir: string; -let env: Record = { ...bunEnv }; +type BunxTestEnv = Record & { + TEMP: string; + BUN_TMPDIR: string; + TMPDIR: string; + BUN_INSTALL_CACHE_DIR: string; +}; +let env: BunxTestEnv; +const temporaryDirectories = new Set>(); // Each test that hits the network gets its own isolated tmpdir + install cache // so the network-heavy tests can run concurrently without sharing bunx cache state. function setup() { - const install_cache_dir = tmpdirSync(); - const current_tmpdir = tmpdirSync(); - const x_dir = tmpdirSync(); + const install_cache_dir = tempDir("bunx-install-cache", {}); + const current_tmpdir = tempDir("bunx-tmp", {}); + const x_dir = tempDir("bunx-cwd", {}); + temporaryDirectories.add(install_cache_dir); + temporaryDirectories.add(current_tmpdir); + temporaryDirectories.add(x_dir); return { x_dir, env: { @@ -26,10 +48,22 @@ function setup() { BUN_TMPDIR: current_tmpdir, TMPDIR: current_tmpdir, BUN_INSTALL_CACHE_DIR: install_cache_dir, - } as Record, + } as BunxTestEnv, }; } +async function withTestContext( + opts: { linker?: "hoisted" | "isolated" } | undefined, + fn: (ctx: TestContext) => Promise, +): Promise { + const ctx = await createTestContext(opts?.linker ? { linker: opts.linker } : undefined); + try { + await fn(ctx); + } finally { + destroyTestContext(ctx); + } +} + // Drop every PATH entry that already provides `name`, so `bunx ` cannot // short-circuit to a binary that happens to be installed on this machine. // Bun.which does the resolving, so Windows' .exe/.cmd lookup matches bunx's. @@ -52,6 +86,11 @@ beforeAll(async () => { await Promise.all(waiting); }); +afterAll(async () => { + await Promise.all(Array.from(temporaryDirectories, directory => directory[Symbol.asyncDispose]())); + temporaryDirectories.clear(); +}); + beforeEach(() => { // Sequential tests (the mock-registry suites below) still read these module-level vars. ({ x_dir, env } = setup()); @@ -377,6 +416,7 @@ it.concurrent("should pass --version to the package if specified", async () => { it.concurrent('should set "npm_config_user_agent" to bun', async () => { const { x_dir, env } = setup(); + const testEnv = { ...env, npm_config_user_agent: undefined }; await writeFile( join(x_dir, "package.json"), JSON.stringify({ @@ -389,7 +429,7 @@ it.concurrent('should set "npm_config_user_agent" to bun', async () => { const { exited: installFinished } = spawn({ cmd: [bunExe(), "install"], cwd: x_dir, - env, + env: testEnv, }); expect(await installFinished).toBe(0); @@ -398,13 +438,13 @@ it.concurrent('should set "npm_config_user_agent" to bun', async () => { cwd: x_dir, stdout: "pipe", stderr: "pipe", - env, + env: testEnv, }); const [err, out, exited] = await Promise.all([subprocess.stderr.text(), subprocess.stdout.text(), subprocess.exited]); expect(err).not.toContain("error:"); - expect(out.trim()).toContain(`bun/${Bun.version}`); + expect(out.trim()).toContain(`bun/${Bun.version.replace(/-debug$/, "")}`); expect(exited).toBe(0); }); @@ -414,7 +454,7 @@ it.concurrent('should set "npm_config_user_agent" to bun', async () => { */ describe("bunx --no-install", () => { const run = ( - ctx: { x_dir: string; env: Record }, + ctx: { x_dir: string; env: BunxTestEnv }, ...args: string[] ): Promise<[stderr: string, stdout: string, exitCode: number]> => { const subprocess = spawn({ @@ -846,81 +886,79 @@ console.log("EXECUTED: multi-tool-alt (alternate binary)"); // `bunx @uidotsh/install` matching /usr/bin/install — the system binary // was executed instead of the package's actual bin. describe("scoped packages should not match unrelated system binaries", () => { - let port: number; - beforeAll(() => { dummyBeforeAll(); - port = getPort()!; }); afterAll(() => { dummyAfterAll(); }); - beforeEach(async () => { - await dummyBeforeEach(); - }); - it("`bunx @scope/install` runs the package's bin, not a system binary named `install`", async () => { - // Create a scoped package whose bin name does NOT match the unscoped - // portion of the package name, mirroring @uidotsh/install whose bin is - // "uidotsh-installer". - const pkgRoot = tmpdirSync(); - const packageDir = join(pkgRoot, "package"); - await mkdir(packageDir, { recursive: true }); - await writeFile( - join(packageDir, "package.json"), - JSON.stringify({ - name: "@scope/install", - version: "1.0.0", - bin: { "scoped-tool": "cli.js" }, - }), - ); - await writeFile( - join(packageDir, "cli.js"), - `#!/usr/bin/env node\nconsole.log("CORRECT: ran the scoped package's bin");\n`, - ); - const tgzDir = tmpdirSync(); - // The dummy registry serves the tarball by basename of the request URL, - // which for `@scope/install` + version 1.0.0 is `install-1.0.0.tgz`. - await Bun.$`tar -czf ${join(tgzDir, "install-1.0.0.tgz")} -C ${pkgRoot} package`; - - // Create a fake "install" binary in $PATH to simulate /usr/bin/install. - const fakeBinDir = tmpdirSync(); - if (isWindows) { - await writeFile(join(fakeBinDir, "install.cmd"), `@echo WRONG: ran a system binary from PATH\r\n`); - } else { - const fakeBin = join(fakeBinDir, "install"); - await writeFile(fakeBin, `#!/bin/sh\necho "WRONG: ran a system binary from PATH"\n`); - await Bun.$`chmod +x ${fakeBin}`; - } + await withTestContext(undefined, async ctx => { + // Create a scoped package whose bin name does NOT match the unscoped + // portion of the package name, mirroring @uidotsh/install whose bin is + // "uidotsh-installer". + using pkgRoot = tempDir("bunx-scoped-package", {}); + const packageDir = join(pkgRoot, "package"); + await mkdir(packageDir, { recursive: true }); + await writeFile( + join(packageDir, "package.json"), + JSON.stringify({ + name: "@scope/install", + version: "1.0.0", + bin: { "scoped-tool": "cli.js" }, + }), + ); + await writeFile( + join(packageDir, "cli.js"), + `#!/usr/bin/env node\nconsole.log("CORRECT: ran the scoped package's bin");\n`, + ); + using tgzDir = tempDir("bunx-scoped-tarball", {}); + // The dummy registry serves the tarball by basename of the request URL, + // which for `@scope/install` + version 1.0.0 is `install-1.0.0.tgz`. + await Bun.$`tar -czf ${join(tgzDir, "install-1.0.0.tgz")} -C ${pkgRoot} package`; + + // Create a fake "install" binary in $PATH to simulate /usr/bin/install. + using fakeBinDir = tempDir("bunx-scoped-path", {}); + if (isWindows) { + await writeFile(join(fakeBinDir, "install.cmd"), `@echo WRONG: ran a system binary from PATH\r\n`); + } else { + const fakeBin = join(fakeBinDir, "install"); + await writeFile(fakeBin, `#!/bin/sh\necho "WRONG: ran a system binary from PATH"\n`); + await Bun.$`chmod +x ${fakeBin}`; + } - const urls: string[] = []; - setHandler(dummyRegistry(urls, { "1.0.0": { bin: { "scoped-tool": "cli.js" }, as: "1.0.0" } }, 0, tgzDir)); + const urls: string[] = []; + setContextHandler( + ctx, + dummyRegistryForContext(ctx, urls, { "1.0.0": { bin: { "scoped-tool": "cli.js" }, as: "1.0.0" } }, 0, tgzDir), + ); - const subprocess = spawn({ - cmd: [bunExe(), "x", "@scope/install"], - cwd: x_dir, - stdout: "pipe", - stdin: "inherit", - stderr: "pipe", - env: { - ...env, - npm_config_registry: `http://localhost:${port}/`, - PATH: `${fakeBinDir}${delimiter}${env.PATH ?? process.env.PATH ?? ""}`, - }, - }); + const subprocess = spawn({ + cmd: [bunExe(), "x", "@scope/install"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "inherit", + stderr: "pipe", + env: { + ...env, + npm_config_registry: ctx.registry_url, + PATH: `${fakeBinDir}${delimiter}${env.PATH ?? process.env.PATH ?? ""}`, + }, + }); - const [err, out, exited] = await Promise.all([ - subprocess.stderr.text(), - subprocess.stdout.text(), - subprocess.exited, - ]); + const [err, out, exited] = await Promise.all([ + subprocess.stderr.text(), + subprocess.stdout.text(), + subprocess.exited, + ]); - expect(out).not.toContain("WRONG"); - expect(err).not.toContain("WRONG"); - expect(out).toContain("CORRECT: ran the scoped package's bin"); - expect(exited).toBe(0); + expect(out).not.toContain("WRONG"); + expect(err).not.toContain("WRONG"); + expect(out).toContain("CORRECT: ran the scoped package's bin"); + expect(exited).toBe(0); + }); }); // Also covers https://github.com/oven-sh/bun/issues/19458 and @@ -954,7 +992,7 @@ describe("scoped packages should not match unrelated system binaries", () => { } // Put a decoy named after the unscoped basename ("collide") in $PATH. - const fakeBinDir = tmpdirSync(); + using fakeBinDir = tempDir("bunx-scoped-local-path", {}); if (isWindows) { await writeFile(join(fakeBinDir, "collide.cmd"), `@echo off\r\necho DECOY_RAN\r\n`); } else { @@ -1041,145 +1079,151 @@ describe("scoped packages should not match unrelated system binaries", () => { // package.json — collides with a system binary, bunx must run the // cached bin via the absolute-path probe, not the system binary. it("bunx-cache-only `@scope/name` whose real bin collides with a system binary runs the cached bin", async () => { - // Create a scoped package with a bin name that differs from the - // unscoped portion AND collides with a system binary we control. - const pkgRoot = tmpdirSync(); - const packageDir = join(pkgRoot, "package"); - await mkdir(packageDir, { recursive: true }); - await writeFile( - join(packageDir, "package.json"), - JSON.stringify({ - name: "@cacheonly/pkg", - version: "1.0.0", - bin: { "colliding-tool": "cli.js" }, - }), - ); - await writeFile( - join(packageDir, "cli.js"), - `#!/usr/bin/env node\nconsole.log("CORRECT: ran the cached package's bin");\n`, - ); - const tgzDir = tmpdirSync(); - await Bun.$`tar -czf ${join(tgzDir, "pkg-1.0.0.tgz")} -C ${pkgRoot} package`; - - // Put a decoy "colliding-tool" (the REAL bin name) in $PATH. - const fakeBinDir = tmpdirSync(); - if (isWindows) { - await writeFile(join(fakeBinDir, "colliding-tool.cmd"), `@echo WRONG: ran a system binary from PATH\r\n`); - } else { - const fakeBin = join(fakeBinDir, "colliding-tool"); - await writeFile(fakeBin, `#!/bin/sh\necho "WRONG: ran a system binary from PATH"\n`); - chmodSync(fakeBin, 0o755); - } + await withTestContext(undefined, async ctx => { + // Create a scoped package with a bin name that differs from the + // unscoped portion AND collides with a system binary we control. + using pkgRoot = tempDir("bunx-cache-package", {}); + const packageDir = join(pkgRoot, "package"); + await mkdir(packageDir, { recursive: true }); + await writeFile( + join(packageDir, "package.json"), + JSON.stringify({ + name: "@cacheonly/pkg", + version: "1.0.0", + bin: { "colliding-tool": "cli.js" }, + }), + ); + await writeFile( + join(packageDir, "cli.js"), + `#!/usr/bin/env node\nconsole.log("CORRECT: ran the cached package's bin");\n`, + ); + using tgzDir = tempDir("bunx-cache-tarball", {}); + await Bun.$`tar -czf ${join(tgzDir, "pkg-1.0.0.tgz")} -C ${pkgRoot} package`; + + // Put a decoy "colliding-tool" (the REAL bin name) in $PATH. + using fakeBinDir = tempDir("bunx-cache-path", {}); + if (isWindows) { + await writeFile(join(fakeBinDir, "colliding-tool.cmd"), `@echo WRONG: ran a system binary from PATH\r\n`); + } else { + const fakeBin = join(fakeBinDir, "colliding-tool"); + await writeFile(fakeBin, `#!/bin/sh\necho "WRONG: ran a system binary from PATH"\n`); + chmodSync(fakeBin, 0o755); + } - const urls: string[] = []; - setHandler(dummyRegistry(urls, { "1.0.0": { bin: { "colliding-tool": "cli.js" }, as: "1.0.0" } }, 0, tgzDir)); + const urls: string[] = []; + setContextHandler( + ctx, + dummyRegistryForContext( + ctx, + urls, + { "1.0.0": { bin: { "colliding-tool": "cli.js" }, as: "1.0.0" } }, + 0, + tgzDir, + ), + ); - const runEnv = { - ...env, - npm_config_registry: `http://localhost:${port}/`, - PATH: `${fakeBinDir}${delimiter}${env.PATH ?? process.env.PATH ?? ""}`, - }; + const runEnv = { + ...env, + npm_config_registry: ctx.registry_url, + PATH: `${fakeBinDir}${delimiter}${env.PATH ?? process.env.PATH ?? ""}`, + }; - // First run: installs into the bunx cache (no local node_modules). - { - const subprocess = spawn({ - cmd: [bunExe(), "x", "@cacheonly/pkg"], - cwd: x_dir, - stdout: "pipe", - stdin: "inherit", - stderr: "pipe", - env: runEnv, - }); - const [err, out, exited] = await Promise.all([ - subprocess.stderr.text(), - subprocess.stdout.text(), - subprocess.exited, - ]); - expect(out).not.toContain("WRONG"); - expect(err).not.toContain("WRONG"); - expect(out).toContain("CORRECT: ran the cached package's bin"); - expect(exited).toBe(0); - } + // First run: installs into the bunx cache (no local node_modules). + { + const subprocess = spawn({ + cmd: [bunExe(), "x", "@cacheonly/pkg"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "inherit", + stderr: "pipe", + env: runEnv, + }); + const [err, out, exited] = await Promise.all([ + subprocess.stderr.text(), + subprocess.stdout.text(), + subprocess.exited, + ]); + expect(out).not.toContain("WRONG"); + expect(err).not.toContain("WRONG"); + expect(out).toContain("CORRECT: ran the cached package's bin"); + expect(exited).toBe(0); + } - // Second run with --no-install: must resolve the real bin name from - // the cached package.json and run the cached bin, NOT the colliding - // system binary. - { - const subprocess = spawn({ - cmd: [bunExe(), "x", "--no-install", "@cacheonly/pkg"], - cwd: x_dir, - stdout: "pipe", - stdin: "inherit", - stderr: "pipe", - env: runEnv, - }); - const [err, out, exited] = await Promise.all([ - subprocess.stderr.text(), - subprocess.stdout.text(), - subprocess.exited, - ]); - expect(out).not.toContain("WRONG"); - expect(err).not.toContain("WRONG"); - expect(out).toContain("CORRECT: ran the cached package's bin"); - expect(exited).toBe(0); - } + // Second run with --no-install: must resolve the real bin name from + // the cached package.json and run the cached bin, NOT the colliding + // system binary. + { + const subprocess = spawn({ + cmd: [bunExe(), "x", "--no-install", "@cacheonly/pkg"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "inherit", + stderr: "pipe", + env: runEnv, + }); + const [err, out, exited] = await Promise.all([ + subprocess.stderr.text(), + subprocess.stdout.text(), + subprocess.exited, + ]); + expect(out).not.toContain("WRONG"); + expect(err).not.toContain("WRONG"); + expect(out).toContain("CORRECT: ran the cached package's bin"); + expect(exited).toBe(0); + } + }); }); }); describe("package name aliases", () => { - let port: number; - beforeAll(() => { dummyBeforeAll(); - port = getPort()!; }); afterAll(() => { dummyAfterAll(); }); - beforeEach(async () => { - await dummyBeforeEach(); - }); - // `bunx claude` should resolve to `@anthropic-ai/claude-code` (same shape as // the `tsc` -> `typescript` rewrite). The npm package named `claude` is an // unrelated squatter with no bin, so redirecting is strictly more useful. it("`bunx claude` requests @anthropic-ai/claude-code, not the 'claude' squatter", async () => { - const urls: string[] = []; - setHandler(async request => { - urls.push(request.url); - return new Response("{}", { status: 404 }); - }); + await withTestContext(undefined, async ctx => { + const urls: string[] = []; + setContextHandler(ctx, async request => { + urls.push(request.url); + return new Response("{}", { status: 404 }); + }); - const subprocess = spawn({ - cmd: [bunExe(), "x", "claude", "--version"], - cwd: x_dir, - stdout: "pipe", - stdin: "inherit", - stderr: "pipe", - env: { - ...env, - // An untagged `bunx ` runs a matching binary already on PATH - // instead of querying the registry, so a machine with `claude` - // installed never makes the request this test asserts on. Drop those - // entries so the alias is what gets exercised, not the developer's or - // the agent's PATH. - PATH: pathWithout("claude", env.PATH), - npm_config_registry: `http://localhost:${port}/`, - }, - }); + const subprocess = spawn({ + cmd: [bunExe(), "x", "claude", "--version"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "inherit", + stderr: "pipe", + env: { + ...env, + // An untagged `bunx ` runs a matching binary already on PATH + // instead of querying the registry, so a machine with `claude` + // installed never makes the request this test asserts on. Drop those + // entries so the alias is what gets exercised, not the developer's or + // the agent's PATH. + PATH: pathWithout("claude", env.PATH), + npm_config_registry: ctx.registry_url, + }, + }); - const [, , exited] = await Promise.all([subprocess.stderr.text(), subprocess.stdout.text(), subprocess.exited]); + const [, , exited] = await Promise.all([subprocess.stderr.text(), subprocess.stdout.text(), subprocess.exited]); - const paths = urls.map(u => new URL(u).pathname); - // The manifest request must be for the real package, and must never hit - // the squatter package name. - expect(paths).toContain("/@anthropic-ai%2fclaude-code"); - expect(paths).not.toContain("/claude"); - // Install fails because the mock registry 404s; that's fine, we only care - // about which manifest was requested. - expect(exited).not.toBe(0); + const paths = urls.map(u => new URL(u).pathname.replace(`/${ctx.id}`, "")); + // The manifest request must be for the real package, and must never hit + // the squatter package name. + expect(paths).toContain("/@anthropic-ai%2fclaude-code"); + expect(paths).not.toContain("/claude"); + // Install fails because the mock registry 404s; that's fine, we only care + // about which manifest was requested. + expect(exited).not.toBe(0); + }); }); }); diff --git a/test/cli/install/dummy.registry.ts b/test/cli/install/dummy.registry.ts index 282efcb0507a..676583ae98e1 100644 --- a/test/cli/install/dummy.registry.ts +++ b/test/cli/install/dummy.registry.ts @@ -27,15 +27,14 @@ * }); * ``` */ -import { file, Server } from "bun"; -import { tmpdirSync } from "harness"; - -let expect: (typeof import("bun:test"))["expect"]; +import { file } from "bun"; +import { tempDir, tmpdirSync, toTOMLString } from "harness"; import { writeFile } from "fs/promises"; import { basename, join } from "path"; type Handler = (req: Request) => Response | Promise; +type ExpectToBe = (value: unknown) => { toBe(expected: unknown): void }; type Pkg = { name: string; version: string; @@ -43,10 +42,18 @@ type Pkg = { tarball: string; }; }; +type DummyRegistryVersion = Record & { + as?: string; +}; +type DummyRegistryInfo = Record & { + latest?: string; +}; -let server: Server; +let expect: ExpectToBe; + +let server: ReturnType; export let root_url: string; -export let check_npm_auth_type = { check: true }; +export const check_npm_auth_type = { check: true }; // ============================================================================ // Concurrent Test Context Support @@ -63,7 +70,7 @@ export interface TestContext { /** Unique identifier for this test context (e.g., "test-1") */ id: string; /** The package directory for this test (a unique temp directory) */ - package_dir: string; + package_dir: ReturnType; /** Number of requests made to this test's handler */ requested: number; /** The handler for this test's registry requests */ @@ -110,7 +117,7 @@ function extractTestPrefix(url: string): { prefix: string; remainingPath: string */ export async function createTestContext(opts?: { linker: "hoisted" | "isolated" }): Promise { const id = `test-${++testIdCounter}`; - const pkg_dir = tmpdirSync(); + const pkg_dir = tempDir("dummy-registry", {}); const ctx: TestContext = { id, @@ -125,7 +132,7 @@ export async function createTestContext(opts?: { linker: "hoisted" | "isolated" // Create bunfig.toml with the prefixed registry URL await writeFile( join(pkg_dir, "bunfig.toml"), - Bun.TOML.stringify({ + toTOMLString({ install: { cache: false, registry: ctx.registry_url, @@ -144,6 +151,7 @@ export async function createTestContext(opts?: { linker: "hoisted" | "isolated" */ export function destroyTestContext(ctx: TestContext): void { testContexts.delete(ctx.id); + ctx.package_dir[Symbol.dispose](); } /** @@ -162,14 +170,16 @@ export function setContextHandler(ctx: TestContext, newHandler: Handler): void { * @param urls - Array to collect requested URLs (passed by reference) * @param info - Package version info (default: { "0.0.2": {} }) * @param numberOfTimesTo500PerURL - Number of times to return 500 before success (for retry testing) + * @param tgzDir - Directory containing package tarballs (defaults to this file's directory) */ export function dummyRegistryForContext( ctx: TestContext, urls: string[], - info: any = { "0.0.2": {} }, + info: DummyRegistryInfo = { "0.0.2": {} }, numberOfTimesTo500PerURL = 0, + tgzDir?: string, ): Handler { - let retryCountsByURL = new Map(); + const retryCountsByURL = new Map(); const _handler: Handler = async request => { urls.push(request.url); const url = request.url.replaceAll("%2f", "/"); @@ -177,7 +187,7 @@ export function dummyRegistryForContext( let status = 200; if (numberOfTimesTo500PerURL > 0) { - let currentCount = retryCountsByURL.get(request.url); + const currentCount = retryCountsByURL.get(request.url); if (currentCount === undefined) { retryCountsByURL.set(request.url, numberOfTimesTo500PerURL); status = 500; @@ -189,7 +199,7 @@ export function dummyRegistryForContext( expect(request.method).toBe("GET"); if (url.endsWith(".tgz")) { - return new Response(file(join(import.meta.dir, basename(url).toLowerCase())), { status }); + return new Response(file(join(tgzDir ?? import.meta.dir, basename(url).toLowerCase())), { status }); } expect(request.headers.get("accept")).toBe( "application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*", @@ -205,16 +215,19 @@ export function dummyRegistryForContext( const name = pathAfterPrefix.slice(1); // Remove leading slash const versions: Record = {}; - let version; - for (version in info) { + let latestVersion: string | undefined; + for (const version in info) { if (!/^[0-9]/.test(version)) continue; + const metadata = info[version]; + if (!metadata || typeof metadata !== "object") continue; + latestVersion = version; versions[version] = { name, version, dist: { - tarball: `${ctx.registry_url}${name}-${info[version].as ?? version}.tgz`, + tarball: `${ctx.registry_url}${name}-${metadata.as ?? version}.tgz`, }, - ...info[version], + ...metadata, }; } @@ -223,7 +236,7 @@ export function dummyRegistryForContext( name, versions, "dist-tags": { - latest: info.latest ?? version, + latest: info.latest ?? latestVersion, }, }), { status }, @@ -241,11 +254,11 @@ export function dummyRegistryForContext( */ export function dummyRegistry( urls: string[], - info: any = { "0.0.2": {} }, + info: DummyRegistryInfo = { "0.0.2": {} }, numberOfTimesTo500PerURL = 0, tgzDir?: string, ): Handler { - let retryCountsByURL = new Map(); + const retryCountsByURL = new Map(); const _handler: Handler = async request => { urls.push(request.url); const url = request.url.replaceAll("%2f", "/"); @@ -253,7 +266,7 @@ export function dummyRegistry( let status = 200; if (numberOfTimesTo500PerURL > 0) { - let currentCount = retryCountsByURL.get(request.url); + const currentCount = retryCountsByURL.get(request.url); if (currentCount === undefined) { retryCountsByURL.set(request.url, numberOfTimesTo500PerURL); status = 500; @@ -278,16 +291,19 @@ export function dummyRegistry( const name = url.slice(url.indexOf("/", root_url.length) + 1); const versions: Record = {}; - let version; - for (version in info) { + let latestVersion: string | undefined; + for (const version in info) { if (!/^[0-9]/.test(version)) continue; + const metadata = info[version]; + if (!metadata || typeof metadata !== "object") continue; + latestVersion = version; versions[version] = { name, version, dist: { - tarball: `${url}-${info[version].as ?? version}.tgz`, + tarball: `${url}-${metadata.as ?? version}.tgz`, }, - ...info[version], + ...metadata, }; } @@ -296,7 +312,7 @@ export function dummyRegistry( name, versions, "dist-tags": { - latest: info.latest ?? version, + latest: info.latest ?? latestVersion, }, }), { status }, @@ -309,7 +325,7 @@ export function dummyRegistry( // Legacy API (for backward compatibility with non-concurrent tests) // ============================================================================ -/** @deprecated Use createTestContext() for concurrent tests */ +/** @deprecated Use {@linkcode createTestContext()} for concurrent tests */ export let package_dir: string; /** @deprecated Use ctx.requested for concurrent tests */ @@ -328,7 +344,7 @@ export function read(path: string) { return Bun.file(join(package_dir, path)); } -/** @deprecated Use setContextHandler() for concurrent tests */ +/** @deprecated Use {@linkcode setContextHandler()} for concurrent tests */ export function setHandler(newHandler: Handler) { legacyHandler = newHandler; } @@ -376,14 +392,14 @@ let packageDirGetter: () => string = () => { return tmpdirSync(); }; -/** @deprecated Use createTestContext() for concurrent tests */ +/** @deprecated Use {@linkcode createTestContext()} for concurrent tests */ export async function dummyBeforeEach(opts?: { linker: "hoisted" | "isolated" }) { resetHandler(); requested = 0; package_dir = packageDirGetter(); await writeFile( join(package_dir, "bunfig.toml"), - Bun.TOML.stringify({ + toTOMLString({ install: { cache: false, registry: `http://localhost:${server.port}/`, @@ -400,7 +416,6 @@ export async function dummyAfterEach() { } if (Bun.main === import.meta.path) { - // @ts-expect-error expect = value => { return { toBe(expected) { @@ -414,7 +429,7 @@ if (Bun.main === import.meta.path) { packageDirGetter = () => process.env.PACKAGE_DIR_TO_USE!; } - await dummyBeforeAll(); + dummyBeforeAll(); await dummyBeforeEach(); setHandler(dummyRegistry([])); console.log("Running dummy registry!\n\n URL: ", root_url!, "\n", "DIR: ", package_dir!); From b629225faec59d9ef0f257ad7d814ff8bd9dc8e0 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Fri, 7 Aug 2026 17:39:06 +0200 Subject: [PATCH 2/7] bunx: resolve bins from named package install When multiple packages expose the same bin, the shared node_modules/.bin winner may not belong to the package selected by bunx. Resolve requested and default bins from the named package's package.json for local and bunx cache installs, then use the installer's validated cross-platform linker. Keep explicit package selection from falling through to unrelated bins. Preserve native-binlink redirects only when the installed target matches the declaring optional dependency's name, version, and platform, and share the installer's fallback policy through Linker. Cover explicit and implicit bunx forms, warm and cold caches, hoisted and isolated linkers, unsafe or missing bins, and native redirect fallback. --- src/install/PackageInstaller.rs | 7 +- src/install/bin.rs | 369 ++++++++++- src/install/isolated_install/Installer.rs | 8 +- src/install/npm.rs | 28 +- src/install/postinstall_optimizer.rs | 17 +- src/runtime/cli/bunx_command.rs | 346 ++++++++-- .../bun-install-native-binlink.test.ts | 456 +++++++++---- test/cli/install/bunx.test.ts | 626 ++++++++++++------ 8 files changed, 1448 insertions(+), 409 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index a74f458d2cbc..4085df51cc22 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -591,7 +591,6 @@ impl<'a> PackageInstaller<'a> { .slice(string_buf); let package_name_ = strings::StringOrTinyString::init(alias); let mut target_package_name = package_name_; - let mut can_retry_without_native_binlink_optimization = false; let mut target_node_modules_path_opt: Option = None; let mut defer_this_bin = false; // `defer if (target_node_modules_path_opt) |*path| path.deinit()` — Option drops. @@ -656,7 +655,6 @@ impl<'a> PackageInstaller<'a> { pkg_names[replacement_pkg_id as usize].slice(string_buf); target_package_name = strings::StringOrTinyString::init(replacement_name); - can_retry_without_native_binlink_optimization = true; } } PostinstallOptimizer::Ignore => {} @@ -717,10 +715,7 @@ impl<'a> PackageInstaller<'a> { bin_linker.link(global); - if can_retry_without_native_binlink_optimization - && (bin_linker.skipped_due_to_missing_bin || bin_linker.err.is_some()) - { - can_retry_without_native_binlink_optimization = false; + if bin_linker.should_retry_without_native_binlink() { if PackageManager::verbose_install() { bun_core::pretty_errorln!( "[Bin Linker] {} -> {} retrying without native bin link", diff --git a/src/install/bin.rs b/src/install/bin.rs index 176a382377ff..f4006e908534 100644 --- a/src/install/bin.rs +++ b/src/install/bin.rs @@ -23,6 +23,7 @@ use bun_sys::{self as sys, Fd, FdExt as _}; use crate::bun_json::{Expr, ExprData}; use crate::dependency::{Dependency, DependencyExt as _}; use crate::install::{DependencyID, ExternalStringList}; +use crate::postinstall_optimizer::{self, PostinstallOptimizer}; #[cfg(windows)] use crate::windows_shim::BinLinkingShim as WinBinLinkingShim; #[cfg(windows)] @@ -890,7 +891,7 @@ impl<'a> Linker<'a> { abs_dest: &ZStr, global: bool, target_needs_resolved_containment_check: bool, - ) { + ) -> bool { debug_assert!(path::is_absolute(abs_target.as_bytes())); debug_assert!(path::is_absolute(abs_dest.as_bytes())); debug_assert!(abs_target.as_bytes()[abs_target.as_bytes().len() - 1] != SEP); @@ -900,7 +901,7 @@ impl<'a> Linker<'a> { // Skip seen destinations for this tree // https://github.com/npm/cli/blob/22731831e22011e32fa0ca12178e242c2ee2b33d/node_modules/bin-links/lib/link-gently.js#L30 if seen.contains_key(abs_dest.as_bytes()) { - return; + return true; } } @@ -908,13 +909,13 @@ impl<'a> Linker<'a> { // shim in path might break a postinstall if !sys::exists(abs_target) { self.skipped_due_to_missing_bin = true; - return; + return false; } if target_needs_resolved_containment_check { #[cfg(not(windows))] if self.resolved_target_parent_escapes_package_dir(abs_target) { - return; + return false; } } @@ -939,7 +940,7 @@ impl<'a> Linker<'a> { // ignore directories, creating a shim for one won't do anything self.err = Some(err); } - return; + return false; } }; self.create_windows_shim(&target, abs_target, abs_dest, global); @@ -948,13 +949,59 @@ impl<'a> Linker<'a> { if self.err.is_some() { // cleanup on error just in case Self::unlink_bin_or_shim(abs_dest); - return; + return false; } #[cfg(not(windows))] { Self::try_normalize_shebang(abs_target); } + + true + } + + pub fn link_package_bin(&mut self, target: &[u8], destination_name: &[u8]) -> bool { + if target.is_empty() + || bin_target_escapes_package_dir(target) + || normalized_bin_name(destination_name) != destination_name + { + return false; + } + + let package_dir_len = self.build_target_package_dir().len(); + let mut dest_off = self.build_destination_dir(false); + if destination_name.len() >= self.abs_dest_buf.len().saturating_sub(dest_off) { + self.err = Some(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG)); + return false; + } + + let abs_target = { + let package_dir = &self.abs_target_buf[..package_dir_len]; + let resolved = Self::resolve_bin_target( + self.is_native_binlink_redirect(), + package_dir, + target, + destination_name, + ); + // SAFETY: `resolve_bin_target` stores its result in a thread-local buffer, + // so it does not borrow `self.abs_target_buf`. + unsafe { ZStr::from_raw(resolved.as_bytes().as_ptr(), resolved.len()) } + }; + + self.abs_dest_buf[dest_off..dest_off + destination_name.len()] + .copy_from_slice(destination_name); + dest_off += destination_name.len(); + self.abs_dest_buf[dest_off] = 0; + // SAFETY: `link_bin_or_create_shim` does not read or write + // `abs_dest_buf`; the detached slice remains valid for the call. + let abs_dest = unsafe { ZStr::from_raw(self.abs_dest_buf.as_ptr(), dest_off) }; + + self.link_bin_or_create_shim( + abs_target, + abs_dest, + false, + bin_target_needs_resolved_containment_check(target), + ) } #[cfg(not(windows))] @@ -1437,6 +1484,10 @@ impl<'a> Linker<'a> { !strings::eql(self.target_package_name.slice(), self.package_name.slice()) } + pub(crate) fn should_retry_without_native_binlink(&self) -> bool { + self.is_native_binlink_redirect() && (self.skipped_due_to_missing_bin || self.err.is_some()) + } + /// Resolve the absolute target for a bin entry inside `package_dir`. /// /// When redirected into a platform-specific optional dependency (native @@ -1981,3 +2032,309 @@ impl<'a> Linker<'a> { } } } + +struct InstalledNativeBinlinkTarget { + node_modules_path: AbsPath, + package_name: Box<[u8]>, +} + +struct InstalledNativeBinlinkDependency { + install_name: Box<[u8]>, + package_name: Box<[u8]>, + version_literal: Box<[u8]>, + version_range: bun_semver::query::Group, +} + +fn with_package_json( + package_dir: &[u8], + callback: impl FnOnce(&Expr) -> Option, +) -> Option { + let package_json_path = + resolve_path::join_abs_string_z::(package_dir, &[b"package.json"]); + let package_json = sys::File::openat(Fd::cwd(), package_json_path, sys::O::RDONLY, 0).ok()?; + let contents = package_json.read_to_end().ok()?; + let source = bun_ast::Source::init_path_string(package_json_path.as_bytes(), &*contents); + bun_ast::initialize_store(); + let mut log = bun_ast::Log::init(); + let parsed = crate::bun_json::ParsedJson::parse_package_json(&source, &mut log).ok()?; + callback(&parsed.root) +} + +fn parse_installed_native_binlink_dependency( + install_name: &[u8], + version_literal: &[u8], +) -> Option { + let sliced = bun_semver::SlicedString::init(version_literal, version_literal); + let parsed = crate::dependency::parse( + String::init(install_name, install_name), + bun_semver::string::Builder::string_hash(install_name), + version_literal, + &sliced, + None, + None, + )?; + if parsed.tag != crate::dependency::Tag::Npm { + return None; + } + + let npm = parsed.npm(); + let package_name = if npm.is_alias { + npm.name.slice(version_literal) + } else { + install_name + }; + + Some(InstalledNativeBinlinkDependency { + install_name: Box::from(install_name), + package_name: Box::from(package_name), + version_literal: Box::from(version_literal), + version_range: npm.version.clone(), + }) +} + +fn native_binlink_package_info( + package_dir: &[u8], +) -> Option<(Box<[u8]>, Vec)> { + with_package_json(package_dir, |expr| { + let package_name_expr = expr.get(b"name")?; + let package_name = package_name_expr.as_utf8_string_literal()?; + let mut optional_dependencies = Vec::new(); + if let Some(optional) = expr.get(b"optionalDependencies") + && let ExprData::EObjectJSON(object) = &optional.data + { + optional_dependencies.reserve(object.get().properties().len()); + for prop in object.get().properties() { + let dependency_name = prop.key.slice(); + if crate::package_installer::alias_is_safe_install_target(dependency_name) + && let Some(version) = prop.value.as_str() + && let Some(dependency) = + parse_installed_native_binlink_dependency(dependency_name, version) + { + optional_dependencies.push(dependency); + } + } + } + Some((Box::from(package_name), optional_dependencies)) + }) +} + +fn native_binlink_is_enabled(install_root: &[u8], package_name: &[u8]) -> bool { + let optimizers = with_package_json(install_root, |expr| { + let mut list = postinstall_optimizer::List::default(); + PostinstallOptimizer::from_package_json(&mut list, expr).ok()?; + Some(list) + }) + .unwrap_or_default(); + + optimizers.is_native_binlink_enabled() + && matches!( + optimizers.get(&postinstall_optimizer::PkgInfo { + name_hash: bun_semver::string::Builder::string_hash(package_name), + ..Default::default() + }), + Some(PostinstallOptimizer::NativeBinlink) + ) +} + +fn platform_package_matches( + package_dir: &[u8], + dependency: &InstalledNativeBinlinkDependency, +) -> bool { + with_package_json(package_dir, |expr| { + if expr.get(b"name")?.as_utf8_string_literal()? != dependency.package_name.as_ref() { + return Some(false); + } + let version_expr = expr.get(b"version")?; + let version_bytes = version_expr.as_utf8_string_literal()?; + let parsed_version = bun_semver::Version::parse_utf8(version_bytes); + if !parsed_version.valid { + return Some(false); + } + let cpu = expr + .get(b"cpu") + .map(|value| crate::npm::negatable_from_json::(&value)) + .transpose() + .ok()? + .unwrap_or(crate::npm::Architecture::ALL); + let os = expr + .get(b"os") + .map(|value| crate::npm::negatable_from_json::(&value)) + .transpose() + .ok()? + .unwrap_or(crate::npm::OperatingSystem::ALL); + + Some( + dependency.version_range.satisfies( + parsed_version.version.min(), + &dependency.version_literal, + version_bytes, + ) && PostinstallOptimizer::is_native_binlink_replacement( + cpu, + os, + crate::npm::Architecture::CURRENT, + crate::npm::OperatingSystem::CURRENT, + ), + ) + }) + .unwrap_or(false) +} + +fn resolve_installed_native_binlink_target( + install_root: &[u8], + package_name: &[u8], +) -> Option { + let mut root_node_modules: AbsPath = + AbsPath::from(strings::without_trailing_slash(install_root)).ok()?; + root_node_modules.append(b"node_modules").ok()?; + + let mut package_dir: AbsPath = AbsPath::from(root_node_modules.slice()).ok()?; + package_dir.append(package_name).ok()?; + let (actual_package_name, optional_dependencies) = + native_binlink_package_info(package_dir.slice())?; + if !native_binlink_is_enabled(install_root, &actual_package_name) { + return None; + } + + let mut nested_node_modules: AbsPath = AbsPath::from(package_dir.slice()).ok()?; + nested_node_modules.append(b"node_modules").ok()?; + + let real_node_modules: AbsPath = { + let mut package_dir_z_buf = path::path_buffer_pool::get(); + let package_dir_z = resolve_path::z(package_dir.slice(), &mut *package_dir_z_buf); + let mut real_package_dir_buf = path::path_buffer_pool::get(); + let real_package_dir = sys::realpath(package_dir_z, &mut *real_package_dir_buf).ok()?; + AbsPath::from(resolve_path::dirname::(real_package_dir)).ok()? + }; + + let node_modules_paths = [nested_node_modules, real_node_modules, root_node_modules]; + for dependency in optional_dependencies { + for node_modules_path in &node_modules_paths { + let mut target_package_dir: AbsPath = AbsPath::from(node_modules_path.slice()).ok()?; + target_package_dir.append(&dependency.install_name).ok()?; + if platform_package_matches(target_package_dir.slice(), &dependency) { + return Some(InstalledNativeBinlinkTarget { + node_modules_path: AbsPath::from(node_modules_path.slice()).ok()?, + package_name: dependency.install_name, + }); + } + } + } + + None +} + +pub fn link_package_bin<'a>( + install_root: &[u8], + package_name: &[u8], + target: &[u8], + destination_scope: &[u8], + destination_name: &[u8], + executable_buf: &'a mut PathBuffer, +) -> Result, crate::Error> { + if normalized_bin_name(destination_scope) != destination_scope { + return Ok(None); + } + + let mut node_modules_path = AbsPath::from(strings::without_trailing_slash(install_root)) + .map_err(|_| crate::Error::PathTooLong)?; + node_modules_path + .append(b"node_modules") + .map_err(|_| crate::Error::PathTooLong)?; + // Keep the real bin name as the executable basename because CLI frameworks + // commonly derive their usage name from the entry script path. + let mut destination_node_modules_path = + AbsPath::from(strings::without_trailing_slash(install_root)) + .map_err(|_| crate::Error::PathTooLong)?; + destination_node_modules_path + .append(b"node_modules") + .map_err(|_| crate::Error::PathTooLong)?; + destination_node_modules_path + .append(destination_scope) + .map_err(|_| crate::Error::PathTooLong)?; + + let mut abs_target_buf = PathBuffer::uninit(); + let mut abs_dest_buf = PathBuffer::uninit(); + let mut rel_buf = PathBuffer::uninit(); + let empty_z_buf = [0u8; 1]; + let empty_z = ZStr::from_buf(&empty_z_buf, 0); + let node_modules_ptr = &raw const node_modules_path; + let native_target = resolve_installed_native_binlink_target(install_root, package_name); + let target_node_modules_path = native_target + .as_ref() + .map(|target| &raw const target.node_modules_path) + .unwrap_or(node_modules_ptr); + let target_package_name = native_target + .as_ref() + .map(|target| target.package_name.as_ref()) + .unwrap_or(package_name); + + Linker::ensure_umask(); + let mut linker = Linker { + bin: Bin::default(), + target_node_modules_path, + target_package_name: strings::StringOrTinyString::init(target_package_name), + seen: None, + node_modules_path: &mut destination_node_modules_path, + package_name: strings::StringOrTinyString::init(package_name), + global_bin_path: empty_z, + string_buf: b"", + extern_string_buf: &[], + abs_target_buf: &mut abs_target_buf, + abs_dest_buf: &mut abs_dest_buf, + rel_buf: &mut rel_buf, + err: None, + skipped_due_to_missing_bin: false, + }; + + let mut linked = linker.link_package_bin(target, destination_name); + if linker.should_retry_without_native_binlink() { + linker.target_node_modules_path = node_modules_ptr; + linker.target_package_name = strings::StringOrTinyString::init(package_name); + linker.err = None; + linker.skipped_due_to_missing_bin = false; + linked = linker.link_package_bin(target, destination_name); + } + if let Some(err) = linker.err { + return Err(err); + } + if !linked { + return Ok(None); + } + + let root = strings::without_trailing_slash(install_root); + let suffix = std::env::consts::EXE_SUFFIX.as_bytes(); + let required = root.len() + + b"/node_modules/".len() + + destination_scope.len() + + b"/.bin/".len() + + destination_name.len() + + suffix.len(); + if required >= executable_buf.len() { + return Err(crate::Error::PathTooLong); + } + + let mut off = 0; + executable_buf[..root.len()].copy_from_slice(root); + off += root.len(); + executable_buf[off] = SEP; + off += 1; + executable_buf[off..off + b"node_modules".len()].copy_from_slice(b"node_modules"); + off += b"node_modules".len(); + executable_buf[off] = SEP; + off += 1; + executable_buf[off..off + destination_scope.len()].copy_from_slice(destination_scope); + off += destination_scope.len(); + executable_buf[off] = SEP; + off += 1; + executable_buf[off..off + b".bin".len()].copy_from_slice(b".bin"); + off += b".bin".len(); + executable_buf[off] = SEP; + off += 1; + executable_buf[off..off + destination_name.len()].copy_from_slice(destination_name); + off += destination_name.len(); + executable_buf[off..off + suffix.len()].copy_from_slice(suffix); + off += suffix.len(); + executable_buf[off] = 0; + + Ok(Some(ZStr::from_buf(executable_buf, off))) +} diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index 6b31bbfd28a5..5c2b62bb52f0 100644 --- a/src/install/isolated_install/Installer.rs +++ b/src/install/isolated_install/Installer.rs @@ -1878,9 +1878,7 @@ impl Task { bin_linker.link(false); - if target_node_modules_path.is_some() - && (bin_linker.skipped_due_to_missing_bin || bin_linker.err.is_some()) - { + if bin_linker.should_retry_without_native_binlink() { bin_linker.target_node_modules_path = bin_linker.node_modules_path; bin_linker.target_package_name = strings::StringOrTinyString::init(dep_name); @@ -2376,9 +2374,7 @@ impl<'a> Installer<'a> { bin_linker.link(false); - if target_node_modules_path.is_some() - && (bin_linker.skipped_due_to_missing_bin || bin_linker.err.is_some()) - { + if bin_linker.should_retry_without_native_binlink() { bin_linker.target_node_modules_path = bin_linker.node_modules_path; bin_linker.target_package_name = package_name; diff --git a/src/install/npm.rs b/src/install/npm.rs index 7c1b77b4a08d..e37e6c47f273 100644 --- a/src/install/npm.rs +++ b/src/install/npm.rs @@ -635,16 +635,28 @@ pub use bun_install_types::resolver_hooks::{ /// reachable from that crate. pub(crate) fn negatable_from_json(expr: &JSON::Expr) -> Result { let mut this = T::NONE.negatable(); - if let JSON::ExprData::EArray(a) = &expr.data { - for item in a.items.slice() { - // JSON parsed via `parse_utf8` always yields UTF-8 EStrings, - // so no transcode allocator is needed. - if let Some(value) = item.as_utf8_string_literal() { - this.apply(value); + match &expr.data { + JSON::ExprData::EArray(a) => { + for item in a.items.slice() { + // JSON parsed via `parse_utf8` always yields UTF-8 EStrings, + // so no transcode allocator is needed. + if let Some(value) = item.as_utf8_string_literal() { + this.apply(value); + } + } + } + JSON::ExprData::EArrayJSON(a) => { + for item in a.get().items() { + if let Some(value) = item.as_str() { + this.apply(value); + } + } + } + _ => { + if let Some(str) = expr.as_utf8_string_literal() { + this.apply(str); } } - } else if let Some(str) = expr.as_utf8_string_literal() { - this.apply(str); } Ok(this.combine()) diff --git a/src/install/postinstall_optimizer.rs b/src/install/postinstall_optimizer.rs index 5a7f06e89d28..9dd41d91a421 100644 --- a/src/install/postinstall_optimizer.rs +++ b/src/install/postinstall_optimizer.rs @@ -102,16 +102,25 @@ impl PostinstallOptimizer { continue; } let meta: &Meta = &metas[resolution as usize]; - if meta.arch == npm::Architecture::ALL || meta.os == npm::OperatingSystem::ALL { - continue; - } - if meta.arch.is_match(target_cpu) && meta.os.is_match(target_os) { + if Self::is_native_binlink_replacement(meta.arch, meta.os, target_cpu, target_os) { return Some(resolution); } } None } + + pub(crate) fn is_native_binlink_replacement( + arch: npm::Architecture, + os: npm::OperatingSystem, + target_cpu: npm::Architecture, + target_os: npm::OperatingSystem, + ) -> bool { + arch != npm::Architecture::ALL + && os != npm::OperatingSystem::ALL + && arch.is_match(target_cpu) + && os.is_match(target_os) + } } // The key is already a hash, so use the identity context rather than diff --git a/src/runtime/cli/bunx_command.rs b/src/runtime/cli/bunx_command.rs index 15e2b2287af6..f6ff88cd3f45 100644 --- a/src/runtime/cli/bunx_command.rs +++ b/src/runtime/cli/bunx_command.rs @@ -213,6 +213,17 @@ pub(crate) enum GetBinNameError { NeedToInstall, } +struct ResolvedPackageBin { + name: Box<[u8]>, + target: Box<[u8]>, +} + +enum PackageBinLookup<'a> { + PackageNotFound, + BinNotFound, + Found(&'a ZStr), +} + impl BunxCommand { /// Adds `create-` to the string, but also handles scoped packages correctly. /// Always clones the string in the process. @@ -282,11 +293,27 @@ impl BunxCommand { && strings::index_of_char(name, b'\\').is_none() } - fn get_bin_name_from_subpath( + fn exit_package_bin_not_found(package_name: &[u8], bin_name: Option<&[u8]>) -> ! { + if let Some(bin_name) = bin_name { + Output::err_generic( + "Package {} does not provide a binary named {}", + (BStr::new(package_name), BStr::new(bin_name)), + ); + } else { + Output::err_generic( + "could not determine executable to run for package {}", + format_args!("{}", BStr::new(package_name)), + ); + } + Global::exit(1); + } + + fn get_bin_from_subpath( transpiler: &mut Transpiler, dir_fd: Fd, subpath_z: &ZStr, - ) -> crate::Result> { + wanted_bin: Option<&[u8]>, + ) -> crate::Result { let target_package_json_fd = bun_sys::openat(dir_fd, subpath_z, O::RDONLY, 0)?; let target_package_json = bun_sys::File::from_fd(target_package_json_fd); @@ -301,30 +328,42 @@ impl BunxCommand { let parsed = json::ParsedJson::parse_package_json(&source, log)?; let expr = parsed.root; - // choose the first package that fits if let Some(bin_expr) = expr.get(b"bin") { match &bin_expr.data { ExprData::EObjectJSON(object) => { for prop in object.get().properties() { let bin_name = prop.key.slice(); - if !Self::is_safe_bin_name(bin_name) { + if !Self::is_safe_bin_name(bin_name) + || wanted_bin.is_some_and(|wanted| wanted != bin_name) + { continue; } - return Ok(Box::<[u8]>::from(bin_name)); + let Some(target) = prop.value.as_str() else { + continue; + }; + if target.is_empty() { + continue; + } + return Ok(ResolvedPackageBin { + name: Box::from(bin_name), + target: Box::from(target), + }); } } ExprData::EString(_) => { - if let Some(name_expr) = expr.get(b"name") { + if let (Some(target), Some(name_expr)) = + (bin_expr.as_utf8_string_literal(), expr.get(b"name")) + { if let Some(name) = name_expr.as_utf8_string_literal() { - // A scoped `name` (`@scope/pkg`) is legitimate here; - // the command name is its unscoped portion. - let bin_name = if name.is_empty() { - name - } else { - bun_install::dependency::unscoped_package_name(name) - }; - if Self::is_safe_bin_name(bin_name) { - return Ok(Box::<[u8]>::from(bin_name)); + let bin_name = bun_install::dependency::unscoped_package_name(name); + if !target.is_empty() + && Self::is_safe_bin_name(bin_name) + && wanted_bin.is_none_or(|wanted| wanted == bin_name) + { + return Ok(ResolvedPackageBin { + name: Box::from(bin_name), + target: Box::from(target), + }); } } } @@ -336,6 +375,22 @@ impl BunxCommand { if let Some(dirs) = expr.as_property(b"directories") { if let Some(bin_prop) = dirs.expr.as_property(b"bin") { if let Some(dir_name) = bin_prop.expr.as_utf8_string_literal() { + if let Some(wanted) = wanted_bin { + if Self::is_safe_bin_name(wanted) { + let mut target = Vec::with_capacity(dir_name.len() + 1 + wanted.len()); + target.extend_from_slice(dir_name); + if !dir_name.ends_with(b"/") && !dir_name.ends_with(b"\\") { + target.push(bun_paths::SEP); + } + target.extend_from_slice(wanted); + return Ok(ResolvedPackageBin { + name: Box::from(wanted), + target: target.into_boxed_slice(), + }); + } + return Err(crate::Error::NoBinFound); + } + let bin_dir = bun_sys::openat_a(dir_fd, dir_name, O::RDONLY | O::DIRECTORY, 0)?; // Fd is non-owning Copy; guard it. let _close_bin_dir = bun_sys::CloseOnDrop::new(bin_dir); @@ -355,7 +410,17 @@ impl BunxCommand { entry = iterator.next(); continue; } - return Ok(Box::<[u8]>::from(current.name.slice_u8())); + let name = current.name.slice_u8(); + let mut target = Vec::with_capacity(dir_name.len() + 1 + name.len()); + target.extend_from_slice(dir_name); + if !dir_name.ends_with(b"/") && !dir_name.ends_with(b"\\") { + target.push(bun_paths::SEP); + } + target.extend_from_slice(name); + return Ok(ResolvedPackageBin { + name: Box::from(name), + target: target.into_boxed_slice(), + }); } entry = iterator.next(); @@ -367,6 +432,14 @@ impl BunxCommand { Err(crate::Error::NoBinFound) } + fn get_bin_name_from_subpath( + transpiler: &mut Transpiler, + dir_fd: Fd, + subpath_z: &ZStr, + ) -> crate::Result> { + Self::get_bin_from_subpath(transpiler, dir_fd, subpath_z, None).map(|bin| bin.name) + } + fn get_bin_name_from_project_directory( transpiler: &mut Transpiler, dir_fd: Fd, @@ -394,6 +467,69 @@ impl BunxCommand { Self::get_bin_name_from_subpath(transpiler, dir_fd, subpath_z) } + fn link_bin_from_installed_package<'a>( + transpiler: &mut Transpiler, + package_name: &[u8], + bin_name: Option<&[u8]>, + install_root: &[u8], + executable_buf: &'a mut PathBuffer, + ) -> crate::Result> { + if bin_name.is_some_and(|name| !Self::is_safe_bin_name(name)) { + return Ok(PackageBinLookup::BinNotFound); + } + + let mut package_subpath = PathBuffer::uninit(); + let package_subpath_len = { + let total = package_subpath.len(); + let mut cursor: &mut [u8] = &mut package_subpath[..]; + write!( + cursor, + "{root}{sep}node_modules{sep}{pkg}", + root = BStr::new(strings::without_trailing_slash(install_root)), + sep = bun_paths::SEP as char, + pkg = BStr::new(package_name), + ) + .map_err(|_| crate::Error::PathTooLong)?; + total - cursor.len() + }; + package_subpath[package_subpath_len] = 0; + let package_subpath_z = ZStr::from_buf(&package_subpath, package_subpath_len); + let package_fd = + match bun_sys::openat(Fd::cwd(), package_subpath_z, O::RDONLY | O::DIRECTORY, 0) { + Ok(fd) => fd, + Err(err) if err.get_errno() == bun_sys::Errno::ENOENT => { + return Ok(PackageBinLookup::PackageNotFound); + } + Err(err) => return Err(err.into()), + }; + let _close_package = bun_sys::CloseOnDrop::new(package_fd); + + let package_json_buf = *b"package.json\0"; + let package_json_z = ZStr::from_buf(&package_json_buf, b"package.json".len()); + let resolved = + match Self::get_bin_from_subpath(transpiler, package_fd, package_json_z, bin_name) { + Ok(resolved) => resolved, + Err(crate::Error::NoBinFound) => return Ok(PackageBinLookup::BinNotFound), + Err(err) => return Err(err), + }; + + let mut destination_scope = Vec::with_capacity(b".bunx-".len() + 16); + write!(&mut destination_scope, ".bunx-{:x}", hash(package_name)) + .map_err(|_| crate::Error::Alloc(AllocError))?; + + match bun_install::bin::link_package_bin( + install_root, + package_name, + &resolved.target, + &destination_scope, + &resolved.name, + executable_buf, + )? { + Some(executable) => Ok(PackageBinLookup::Found(executable)), + None => Ok(PackageBinLookup::BinNotFound), + } + } + fn get_bin_name_from_temp_directory( transpiler: &mut Transpiler, tempdir_name: &[u8], @@ -977,6 +1113,8 @@ impl BunxCommand { let top_level_dir: &[u8] = fs.top_level_dir; let mut absolute_in_cache_dir_buf = PathBuffer::uninit(); + let mut package_bin_abs_buf = PathBuffer::uninit(); + let mut cache_manifest_buf = PathBuffer::uninit(); let buf_total = absolute_in_cache_dir_buf.len(); let mut absolute_in_cache_dir: &[u8] = { let mut cursor: &mut [u8] = &mut absolute_in_cache_dir_buf[..]; @@ -994,6 +1132,20 @@ impl BunxCommand { // SAFETY: `written` bytes were just initialized above unsafe { core::slice::from_raw_parts(absolute_in_cache_dir_buf.as_ptr(), written) } }; + let cache_manifest = { + let total = cache_manifest_buf.len(); + let mut cursor: &mut [u8] = &mut cache_manifest_buf; + write!( + cursor, + "{cache}{sep}package.json", + cache = BStr::new(bunx_cache_dir), + sep = bun_paths::SEP as char, + ) + .map_err(|_| crate::Error::PathTooLong)?; + let written = total - cursor.len(); + cache_manifest_buf[written] = 0; + ZStr::from_buf(&cache_manifest_buf, written) + }; if !Self::is_trusted_cache_root(bunx_cache_dir, temp_dir.len(), uid) { Output::err_generic( @@ -1017,13 +1169,56 @@ impl BunxCommand { // 1. Try the bin in the current node_modules and then we try the bin in the global cache // // Both probes are folded into one labeled block. + let mut is_package_owned_bin = false; let dest_or_cache: Option<&ZStr> = 'find: { + if update_request.version.literal.is_empty() { + match Self::link_bin_from_installed_package( + this_transpiler, + result_package_name, + opts.binary_name, + top_level_dir, + &mut package_bin_abs_buf, + )? { + PackageBinLookup::Found(d) => { + is_package_owned_bin = true; + break 'find Some(d); + } + PackageBinLookup::BinNotFound => { + Self::exit_package_bin_not_found( + result_package_name, + opts.binary_name, + ); + } + PackageBinLookup::PackageNotFound => {} + } + } + match Self::link_bin_from_installed_package( + this_transpiler, + result_package_name, + opts.binary_name, + bunx_cache_dir, + &mut package_bin_abs_buf, + )? { + PackageBinLookup::Found(d) => { + is_package_owned_bin = true; + break 'find Some(d); + } + PackageBinLookup::BinNotFound => { + Self::exit_package_bin_not_found(result_package_name, opts.binary_name); + } + PackageBinLookup::PackageNotFound => {} + } + + if opts.specified_package.is_some() { + break 'find None; + } + // Only use the system-installed version if there is no version specified if update_request.version.literal.is_empty() { - // If the bin name is a guess derived from a scoped package name, - // exclude the original system $PATH so we don't match unrelated - // system binaries. Only search local node_modules/.bin directories. if let Some(d) = bun_which::which( + // If the bin name is a guess derived from a scoped package name, + // exclude the original system $PATH so we don't match unrelated + // system binaries. Only search local node_modules/.bin directories. &mut path_buf, if initial_bin_name_is_a_guess { &local_bin_dirs @@ -1071,18 +1266,23 @@ impl BunxCommand { break 'try_run_existing; } let is_stale: bool = 'is_stale: { + let stale_marker = if is_package_owned_bin { + cache_manifest + } else { + destination + }; #[cfg(windows)] { use bun_sys::windows as win; - let fd = match bun_sys::openat(Fd::cwd(), destination, O::RDONLY, 0) - { - Ok(fd) => fd, - Err(_) => { - // if we cant open this, we probably will just fail when we run it - // and that error message is likely going to be better than the one from `bun add` - break 'is_stale false; - } - }; + let fd = + match bun_sys::openat(Fd::cwd(), stale_marker, O::RDONLY, 0) { + Ok(fd) => fd, + Err(_) => { + // if we cant open this, we probably will just fail when we run it + // and that error message is likely going to be better than the one from `bun add` + break 'is_stale false; + } + }; // The fd is closed explicitly below before // any `break 'is_stale` (no early-return between open & close). @@ -1113,7 +1313,7 @@ impl BunxCommand { } #[cfg(not(windows))] { - let stat = match bun_sys::stat(destination) { + let stat = match bun_sys::stat(stale_marker) { Ok(s) => s, Err(_) => break 'is_stale true, }; @@ -1466,20 +1666,79 @@ impl BunxCommand { unsafe { core::slice::from_raw_parts(absolute_in_cache_dir_buf.as_ptr(), written) } }; + if opts.specified_package.is_some() { + match Self::link_bin_from_installed_package( + this_transpiler, + result_package_name, + opts.binary_name, + bunx_cache_dir, + &mut package_bin_abs_buf, + )? { + PackageBinLookup::Found(destination) => { + let out = destination.as_bytes(); + if Self::is_trusted_cached_binary(destination, uid) { + let stored = fs.dirname_store.append_slice(out)?; + Run::run_binary( + ctx, + stored, + destination, + top_level_dir, + env_loader, + passthrough, + None, + )?; + } + } + PackageBinLookup::PackageNotFound | PackageBinLookup::BinNotFound => { + Self::exit_package_bin_not_found(result_package_name, opts.binary_name); + } + } + } else { + match Self::link_bin_from_installed_package( + this_transpiler, + result_package_name, + None, + bunx_cache_dir, + &mut package_bin_abs_buf, + )? { + PackageBinLookup::Found(destination) => { + let out = destination.as_bytes(); + if Self::is_trusted_cached_binary(destination, uid) { + let stored = fs.dirname_store.append_slice(out)?; + Run::run_binary( + ctx, + stored, + destination, + top_level_dir, + env_loader, + passthrough, + None, + )?; + } + } + PackageBinLookup::BinNotFound => { + Self::exit_package_bin_not_found(result_package_name, None); + } + PackageBinLookup::PackageNotFound => {} + } + } + // Similar to "npx": // // 1. Try the bin in the global cache // Do not try $PATH because we already checked it above if we should - if let Some(destination) = bun_which::which( - &mut path_buf, - bunx_cache_dir, - if !ignore_cwd.is_empty() { - b"".as_slice() - } else { - top_level_dir - }, - absolute_in_cache_dir, - ) { + if opts.specified_package.is_none() + && let Some(destination) = bun_which::which( + &mut path_buf, + bunx_cache_dir, + if !ignore_cwd.is_empty() { + b"".as_slice() + } else { + top_level_dir + }, + absolute_in_cache_dir, + ) + { let out: &[u8] = destination.as_bytes(); // The install we just ran should have created this symlink as the // current user, but the cache lives in a world-writable temp dir; an @@ -1570,14 +1829,7 @@ impl BunxCommand { } if let (Some(_), Some(binary_name)) = (opts.specified_package, opts.binary_name) { - Output::err_generic( - "Package {} does not provide a binary named {}", - (BStr::new(&update_request.name), BStr::new(binary_name)), - ); - bun_core::prettyln!( - " hint: try running without --package to install and run {} directly", - BStr::new(binary_name), - ); + Self::exit_package_bin_not_found(&update_request.name, Some(binary_name)); } else { Output::err_generic( "could not determine executable to run for package {}", diff --git a/test/cli/install/bun-install-native-binlink.test.ts b/test/cli/install/bun-install-native-binlink.test.ts index 71df3ee19c83..243ae3f07658 100644 --- a/test/cli/install/bun-install-native-binlink.test.ts +++ b/test/cli/install/bun-install-native-binlink.test.ts @@ -2,10 +2,12 @@ import { spawn } from "bun"; import { afterAll, beforeAll, describe, expect, setDefaultTimeout, test } from "bun:test"; import { chmodSync, existsSync, readFileSync, realpathSync, statSync, symlinkSync } from "fs"; import { rm, writeFile } from "fs/promises"; -import { bunEnv, bunExe, isWindows, tempDir, VerdaccioRegistry } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir, toTOMLString, VerdaccioRegistry } from "harness"; import { join, sep } from "path"; let verdaccio: VerdaccioRegistry; +type LinkerCase = { linker: "hoisted" | "isolated" }; +const linkerCases: LinkerCase[] = [{ linker: "hoisted" }, { linker: "isolated" }]; setDefaultTimeout(1000 * 60 * 5); @@ -36,9 +38,9 @@ function readBinTarget(binDir: string, name: string) { } describe.concurrent("native binlink optimization", () => { - for (const linker of ["hoisted", "isolated"]) { - test(`uses platform-specific bin instead of main package bin with linker ${linker}`, async () => { - let env = { ...bunEnv }; + describe.each(linkerCases)("with $linker linker", ({ linker }) => { + test("uses platform-specific bin instead of main package bin", async () => { + const env = { ...bunEnv }; const { packageDir, packageJson } = await verdaccio.createTestDir(); env.BUN_INSTALL_CACHE_DIR = join(packageDir, ".bun-cache"); env.BUN_TMPDIR = env.TMPDIR = env.TEMP = join(packageDir, ".bun-tmp"); @@ -46,7 +48,7 @@ describe.concurrent("native binlink optimization", () => { // Create bunfig await writeFile( join(packageDir, "bunfig.toml"), - Bun.TOML.stringify({ + toTOMLString({ install: { cache: join(packageDir, ".bun-cache"), registry: verdaccio.registryUrl(), @@ -98,9 +100,27 @@ describe.concurrent("native binlink optimization", () => { expect(exitCode).toBe(0); } + async function expectBunxPlatformBin() { + const proc = spawn({ + cmd: [bunExe(), "x", "--package", "test-native-binlink", "test-binlink-cmd"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: "SUCCESS: Using platform-specific bin (test-native-binlink-target)\n", + stderr: "", + exitCode: 0, + }); + } + await runInstall(); expect(readBinTarget(binDir, "test-binlink-cmd")).toContain(join("test-native-binlink-target", "bin", "main.js")); await expectPlatformBin(); + await expectBunxPlatformBin(); // Now delete the node_modules folder, keep the bun.lock, re-install await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); @@ -117,20 +137,202 @@ describe.concurrent("native binlink optimization", () => { await expectPlatformBin(); }); + test("ignores an installed native package that does not satisfy the optional dependency", async () => { + const env = { ...bunEnv }; + const { packageDir, packageJson } = await verdaccio.createTestDir(); + env.BUN_INSTALL_CACHE_DIR = join(packageDir, ".bun-cache"); + env.BUN_TMPDIR = env.TMPDIR = env.TEMP = join(packageDir, ".bun-tmp"); + + await writeFile( + join(packageDir, "bunfig.toml"), + toTOMLString({ + install: { + cache: join(packageDir, ".bun-cache"), + registry: verdaccio.registryUrl(), + linker, + }, + }), + ); + await writeFile( + packageJson, + JSON.stringify({ + name: "test-app", + version: "1.0.0", + dependencies: { "test-native-binlink": "1.0.0" }, + nativeDependencies: ["test-native-binlink"], + trustedDependencies: ["test-native-binlink"], + }), + ); + + await using install = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [installStderr, installExitCode] = await Promise.all([install.stderr.text(), install.exited]); + expect(installStderr).not.toContain("error:"); + expect(installExitCode).toBe(0); + + const targetPackageJsons = new Set( + Array.from( + new Bun.Glob("**/test-native-binlink-target/package.json").scanSync({ + cwd: join(packageDir, "node_modules"), + absolute: true, + dot: true, + }), + path => realpathSync(path), + ), + ); + expect(targetPackageJsons.size).toBeGreaterThan(0); + await Promise.all( + Array.from(targetPackageJsons, targetPackageJson => + writeFile( + targetPackageJson, + JSON.stringify({ + name: "test-native-binlink-target", + version: "2.0.0", + os: [process.platform], + cpu: [process.arch], + }), + ), + ), + ); + + await using bunx = spawn({ + cmd: [bunExe(), "x", "--package", "test-native-binlink", "test-binlink-cmd"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [stdout, stderr, exitCode] = await Promise.all([bunx.stdout.text(), bunx.stderr.text(), bunx.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: "ERROR: Using main package bin, not platform-specific!\n", + stderr: "", + exitCode: 1, + }); + }); + + test("resolves an npm-aliased native optional dependency", async () => { + const env = { ...bunEnv }; + const { packageDir, packageJson } = await verdaccio.createTestDir(); + env.BUN_INSTALL_CACHE_DIR = join(packageDir, ".bun-cache"); + env.BUN_TMPDIR = env.TMPDIR = env.TEMP = join(packageDir, ".bun-tmp"); + + await writeFile( + join(packageDir, "bunfig.toml"), + toTOMLString({ + install: { + cache: join(packageDir, ".bun-cache"), + registry: verdaccio.registryUrl(), + linker, + }, + }), + ); + await writeFile( + packageJson, + JSON.stringify({ + name: "test-app", + version: "1.0.0", + dependencies: { + "test-native-binlink": "1.0.0", + "test-native-alias": "npm:test-native-binlink-target@1.0.0", + }, + nativeDependencies: ["test-native-binlink"], + trustedDependencies: ["test-native-binlink"], + }), + ); + + await using install = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [installStderr, installExitCode] = await Promise.all([install.stderr.text(), install.exited]); + expect(installStderr).not.toContain("error:"); + expect(installExitCode).toBe(0); + + const declaringPackageJsons = new Set( + Array.from( + new Bun.Glob("**/test-native-binlink/package.json").scanSync({ + cwd: join(packageDir, "node_modules"), + absolute: true, + dot: true, + }), + path => realpathSync(path), + ), + ); + expect(declaringPackageJsons.size).toBeGreaterThan(0); + await Promise.all( + Array.from(declaringPackageJsons, declaringPackageJson => + writeFile( + declaringPackageJson, + JSON.stringify({ + name: "test-native-binlink", + version: "1.0.0", + bin: { "test-binlink-cmd": "./bin/main.js" }, + optionalDependencies: { + "test-native-alias": "npm:test-native-binlink-target@1.0.0", + }, + }), + ), + ), + ); + + const sharedBinDir = join(packageDir, "node_modules", ".bin"); + await rm(binEntry(sharedBinDir, "test-binlink-cmd"), { force: true }); + if (isWindows) { + await rm(join(sharedBinDir, "test-binlink-cmd.bunx"), { force: true }); + await writeFile( + join(sharedBinDir, "test-binlink-cmd.cmd"), + "@echo ERROR: Using shared bin instead of package-owned alias target!\r\n@exit /b 1\r\n", + ); + } else { + const sharedBin = join(sharedBinDir, "test-binlink-cmd"); + await writeFile( + sharedBin, + '#!/usr/bin/env bun\nconsole.log("ERROR: Using shared bin instead of package-owned alias target!");\nprocess.exit(1);\n', + ); + chmodSync(sharedBin, 0o755); + } + + await using bunx = spawn({ + cmd: [bunExe(), "x", "--package", "test-native-binlink", "test-binlink-cmd"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [stdout, stderr, exitCode] = await Promise.all([bunx.stdout.text(), bunx.stderr.text(), bunx.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: "SUCCESS: Using platform-specific bin (test-native-binlink-target)\n", + stderr: "", + exitCode: 0, + }); + }); + // Regression: a package on the nativeDependencies list whose platform-specific // optionalDependency does NOT contain the bin file at the expected path must // fall back to linking the original package's bin. Previously the `seen` map // was poisoned by the failed redirect attempt, so the retry silently no-op'd // and `.bin/` was never created (broke `bunx @anthropic-ai/claude-code`). - test(`falls back to main package bin when platform dep has no matching bin file with linker ${linker}`, async () => { - let env = { ...bunEnv }; + test("falls back to main package bin when platform dep has no matching bin file", async () => { + const env = { ...bunEnv }; const { packageDir, packageJson } = await verdaccio.createTestDir(); env.BUN_INSTALL_CACHE_DIR = join(packageDir, ".bun-cache"); env.BUN_TMPDIR = env.TMPDIR = env.TEMP = join(packageDir, ".bun-tmp"); await writeFile( join(packageDir, "bunfig.toml"), - Bun.TOML.stringify({ + toTOMLString({ install: { cache: join(packageDir, ".bun-cache"), registry: verdaccio.registryUrl(), @@ -184,6 +386,25 @@ describe.concurrent("native binlink optimization", () => { expect(binStdout).toContain("SUCCESS: Using main package bin"); expect(binExitCode).toBe(0); + const bunxProc = spawn({ + cmd: [bunExe(), "x", "--package", "test-native-binlink-fallback", "fallback-cmd"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [bunxStdout, bunxStderr, bunxExitCode] = await Promise.all([ + bunxProc.stdout.text(), + bunxProc.stderr.text(), + bunxProc.exited, + ]); + expect({ stdout: bunxStdout, stderr: bunxStderr, exitCode: bunxExitCode }).toEqual({ + stdout: "SUCCESS: Using main package bin (test-native-binlink-fallback)\n", + stderr: "", + exitCode: 0, + }); + // Re-install with node_modules removed (lockfile-only path) await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); const installProc2 = spawn({ @@ -208,22 +429,26 @@ describe.concurrent("native binlink optimization", () => { expect(binStdout2).toContain("SUCCESS: Using main package bin"); expect(binExitCode2).toBe(0); }); - } + }); // The postinstall skip must apply to every copy of a nativeDependencies // package in the tree, not just the hoisted one. Previously a second, // differently-versioned esbuild nested under a transitive dependent would // still run `node install.js`. describe("nested nativeDependencies", () => { - async function setup(opts: { linker: "hoisted" | "isolated"; deps: Record; extraEnv?: object }) { - let env: Record = { ...bunEnv, ...(opts.extraEnv ?? {}) }; + async function setup(opts: { + linker: "hoisted" | "isolated"; + deps: Record; + extraEnv?: Record; + }) { + const env = { ...bunEnv, ...opts.extraEnv }; const { packageDir, packageJson } = await verdaccio.createTestDir(); env.BUN_INSTALL_CACHE_DIR = join(packageDir, ".bun-cache"); env.BUN_TMPDIR = env.TMPDIR = env.TEMP = join(packageDir, ".bun-tmp"); await writeFile( join(packageDir, "bunfig.toml"), - Bun.TOML.stringify({ + toTOMLString({ install: { cache: join(packageDir, ".bun-cache"), registry: verdaccio.registryUrl(), @@ -277,8 +502,9 @@ describe.concurrent("native binlink optimization", () => { return Object.fromEntries(Object.entries(dirs).map(([k, d]) => [k, existsSync(join(d, "postinstall-ran"))])); } - for (const linker of ["hoisted", "isolated"] as const) { - test(`skips postinstall for nested copies (${linker}, platform dep in child tree)`, async () => { + test.each(linkerCases)( + "skips postinstall for nested copies ($linker, platform dep in child tree)", + async ({ linker }) => { const { packageDir, install, runBin } = await setup({ linker, deps: { "test-postinstall-skip": "2.0.0", "test-postinstall-skip-parent": "1.0.0" }, @@ -334,8 +560,8 @@ describe.concurrent("native binlink optimization", () => { expect(readBinTarget(nestedBinDir, "skip-test-cmd")).toContain("test-postinstall-skip-native"); expect(await runBin(nestedBinDir, "skip-test-cmd")).toEqual({ out: "native v1.0.0", err: "", code: 0 }); } - }); - } + }, + ); test("skips postinstall for nested copies (hoisted, platform dep as sibling)", async () => { // parent@2.0.0 also depends on test-postinstall-skip-native@1.0.0 directly, @@ -444,7 +670,7 @@ describe.concurrent("native binlink optimization", () => { // of the fixture exercises one of the alternate-path probes in // `bin::Linker::resolve_bin_target`. describe.concurrent("native binlink altpath", () => { - const shapes = [ + const shapes: { version: string; targetFile: string; description: string }[] = [ { version: "1.0.0", targetFile: "altpath-cmd", @@ -460,113 +686,111 @@ describe.concurrent("native binlink altpath", () => { targetFile: "altpath-cmd.exe", description: "/.exe (@esbuild/win32 shape)", }, - ] as const; + ]; - for (const linker of ["hoisted", "isolated"]) { - for (const { version, targetFile, description } of shapes) { - test(`finds native bin via ${description} with linker ${linker}`, async () => { - let env = { ...bunEnv }; - const { packageDir, packageJson } = await verdaccio.createTestDir(); - env.BUN_INSTALL_CACHE_DIR = join(packageDir, ".bun-cache"); - env.BUN_TMPDIR = env.TMPDIR = env.TEMP = join(packageDir, ".bun-tmp"); + describe.each(linkerCases)("with $linker linker", ({ linker }) => { + test.each(shapes)("finds native bin via $description", async ({ version, targetFile }) => { + const env = { ...bunEnv }; + const { packageDir, packageJson } = await verdaccio.createTestDir(); + env.BUN_INSTALL_CACHE_DIR = join(packageDir, ".bun-cache"); + env.BUN_TMPDIR = env.TMPDIR = env.TEMP = join(packageDir, ".bun-tmp"); - await writeFile( - join(packageDir, "bunfig.toml"), - Bun.TOML.stringify({ - install: { - cache: join(packageDir, ".bun-cache"), - registry: verdaccio.registryUrl(), - linker, - }, - }), - ); + await writeFile( + join(packageDir, "bunfig.toml"), + toTOMLString({ + install: { + cache: join(packageDir, ".bun-cache"), + registry: verdaccio.registryUrl(), + linker, + }, + }), + ); - await writeFile( - packageJson, - JSON.stringify({ - name: "test-app", - version: "1.0.0", - dependencies: { - "test-native-binlink-altpath": version, - }, - nativeDependencies: ["test-native-binlink-altpath"], - trustedDependencies: ["test-native-binlink-altpath"], - }), - ); + await writeFile( + packageJson, + JSON.stringify({ + name: "test-app", + version: "1.0.0", + dependencies: { + "test-native-binlink-altpath": version, + }, + nativeDependencies: ["test-native-binlink-altpath"], + trustedDependencies: ["test-native-binlink-altpath"], + }), + ); - const installProc = spawn({ - cmd: [bunExe(), "install"], - cwd: packageDir, - stdout: "pipe", - stdin: "ignore", - stderr: "pipe", - env, - }); - const [, installStderr, installExit] = await Promise.all([ - installProc.stdout.text(), - installProc.stderr.text(), - installProc.exited, - ]); - expect(installStderr).not.toContain("error:"); - expect(installExit).toBe(0); - - const binDir = join(packageDir, "node_modules", ".bin"); - const binPath = binEntry(binDir, "altpath-cmd"); - // The bin must resolve into the platform-specific package, not back into - // the parent package's placeholder stub. - expect(readBinTarget(binDir, "altpath-cmd")).toContain(join("test-native-binlink-altpath-target", targetFile)); - - const binProc = spawn({ - cmd: [binPath], - cwd: packageDir, - stdout: "pipe", - stdin: "ignore", - stderr: "pipe", - env, - }); - const [binStdout, binStderr, binExitCode] = await Promise.all([ - binProc.stdout.text(), - binProc.stderr.text(), - binProc.exited, - ]); - expect({ stdout: binStdout, stderr: binStderr }).toEqual({ - stdout: expect.stringContaining("SUCCESS: Using platform-specific bin at package root"), - stderr: "", - }); - expect(binExitCode).toBe(0); + const installProc = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [, installStderr, installExit] = await Promise.all([ + installProc.stdout.text(), + installProc.stderr.text(), + installProc.exited, + ]); + expect(installStderr).not.toContain("error:"); + expect(installExit).toBe(0); - // Because the redirect succeeded, the postinstall should have been - // skipped entirely (that's the point of the optimization). - expect( - existsSync(join(packageDir, "node_modules", "test-native-binlink-altpath", "postinstall-ran")), - ).toBeFalse(); + const binDir = join(packageDir, "node_modules", ".bin"); + const binPath = binEntry(binDir, "altpath-cmd"); + // The bin must resolve into the platform-specific package, not back into + // the parent package's placeholder stub. + expect(readBinTarget(binDir, "altpath-cmd")).toContain(join("test-native-binlink-altpath-target", targetFile)); - // Re-install with node_modules removed (lockfile-only path) - await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); - const installProc2 = spawn({ - cmd: [bunExe(), "install"], - cwd: packageDir, - stdout: "inherit", - stdin: "ignore", - stderr: "inherit", - env, - }); - expect(await installProc2.exited).toBe(0); + const binProc = spawn({ + cmd: [binPath], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [binStdout, binStderr, binExitCode] = await Promise.all([ + binProc.stdout.text(), + binProc.stderr.text(), + binProc.exited, + ]); + expect({ stdout: binStdout, stderr: binStderr }).toEqual({ + stdout: expect.stringContaining("SUCCESS: Using platform-specific bin at package root"), + stderr: "", + }); + expect(binExitCode).toBe(0); - const binProc2 = spawn({ - cmd: [binPath], - cwd: packageDir, - stdout: "pipe", - stdin: "ignore", - stderr: "inherit", - env, - }); - const [binStdout2, binExitCode2] = await Promise.all([binProc2.stdout.text(), binProc2.exited]); - expect(binStdout2).toContain("SUCCESS: Using platform-specific bin at package root"); - expect(binExitCode2).toBe(0); + // Because the redirect succeeded, the postinstall should have been + // skipped entirely (that's the point of the optimization). + expect( + existsSync(join(packageDir, "node_modules", "test-native-binlink-altpath", "postinstall-ran")), + ).toBeFalse(); + + // Re-install with node_modules removed (lockfile-only path) + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + const installProc2 = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "inherit", + stdin: "ignore", + stderr: "inherit", + env, }); - } - } + expect(await installProc2.exited).toBe(0); + + const binProc2 = spawn({ + cmd: [binPath], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "inherit", + env, + }); + const [binStdout2, binExitCode2] = await Promise.all([binProc2.stdout.text(), binProc2.exited]); + expect(binStdout2).toContain("SUCCESS: Using platform-specific bin at package root"); + expect(binExitCode2).toBe(0); + }); + }); }); // The bin linker must not create a `node_modules/.bin` entry (nor chmod or rewrite the diff --git a/test/cli/install/bunx.test.ts b/test/cli/install/bunx.test.ts index c0e7b61aae4b..cc2fb877c347 100644 --- a/test/cli/install/bunx.test.ts +++ b/test/cli/install/bunx.test.ts @@ -1,7 +1,7 @@ import { spawn } from "bun"; import { afterAll, beforeAll, beforeEach, describe, expect, it, setDefaultTimeout } from "bun:test"; import { mkdir, rm, writeFile } from "fs/promises"; -import { bunEnv, bunExe, isWindows, readdirSorted, tempDir, tmpdirSync } from "harness"; +import { bunEnv, bunExe, isWindows, readdirSorted, tempDir } from "harness"; import { chmodSync, copyFileSync, readdirSync, symlinkSync } from "node:fs"; import { tmpdir } from "os"; import { delimiter, join, resolve } from "path"; @@ -11,11 +11,7 @@ import { destroyTestContext, dummyAfterAll, dummyBeforeAll, - dummyBeforeEach, - dummyRegistry, dummyRegistryForContext, - getPort, - setHandler, setContextHandler, } from "./dummy.registry"; @@ -52,6 +48,21 @@ function setup() { }; } +type PackageInvocationCase = { + invocation: string; + useBunx: boolean; + explicitPackage: boolean; +}; +type LinkerCase = { linker: "hoisted" | "isolated" }; + +const packageInvocationCases: PackageInvocationCase[] = [ + { invocation: "bun x --package", useBunx: false, explicitPackage: true }, + { invocation: "bunx --package", useBunx: true, explicitPackage: true }, + { invocation: "bun x", useBunx: false, explicitPackage: false }, + { invocation: "bunx", useBunx: true, explicitPackage: false }, +]; +const linkerCases: LinkerCase[] = [{ linker: "hoisted" }, { linker: "isolated" }]; + async function withTestContext( opts: { linker?: "hoisted" | "isolated" } | undefined, fn: (ctx: TestContext) => Promise, @@ -64,6 +75,15 @@ async function withTestContext( } } +function packageInvocationCommand( + { useBunx, explicitPackage }: PackageInvocationCase, + packageSpec: string, +): { cmd: string[]; argv0?: string } { + const cmd = useBunx ? [bunExe()] : [bunExe(), "x"]; + cmd.push(...(explicitPackage ? ["--package", packageSpec, "what-bin"] : [packageSpec])); + return useBunx ? { cmd, argv0: isWindows ? "bunx.exe" : "bunx" } : { cmd }; +} + // Drop every PATH entry that already provides `name`, so `bunx ` cannot // short-circuit to a binary that happens to be installed on this machine. // Bun.which does the resolving, so Windows' .exe/.cmd lookup matches bunx's. @@ -571,7 +591,7 @@ it.concurrent("should handle package that requires node 24", async () => { expect(exited).toBe(0); }); -describe("--package flag", () => { +describe("package selection", () => { const run = async (...args: string[]): Promise<[err: string, out: string, exited: number]> => { const subprocess = spawn({ cmd: [bunExe(), "x", ...args], @@ -604,278 +624,452 @@ describe("--package flag", () => { }); describe("with mock registry", () => { - let port: number; - beforeAll(() => { dummyBeforeAll(); - port = getPort()!; }); afterAll(() => { dummyAfterAll(); }); - beforeEach(async () => { - await dummyBeforeEach(); - }); - - const runWithRegistry = async ( - ...args: string[] - ): Promise<[err: string, out: string, exited: number, urls: string[]]> => { + async function installBinCollisionFixture(ctx: TestContext) { const urls: string[] = []; + const fixtureDir = join(import.meta.dir, "registry", "packages", "what-bin"); + setContextHandler( + ctx, + dummyRegistryForContext( + ctx, + urls, + { + "1.0.0": { bin: { "what-bin": "what-bin.js" }, as: "1.0.0" }, + "1.5.0": { bin: { "what-bin": "what-bin.js" }, as: "1.5.0" }, + }, + 0, + fixtureDir, + ), + ); - const subprocess = spawn({ - cmd: [bunExe(), "x", ...args], - cwd: x_dir, + const xDir = ctx.package_dir; + await mkdir(join(xDir, "no-bin")); + await writeFile(join(xDir, "no-bin", "package.json"), JSON.stringify({ name: "no-bin", version: "1.0.0" })); + await mkdir(join(xDir, "unsafe-bin")); + await writeFile( + join(xDir, "unsafe-bin", "package.json"), + JSON.stringify({ name: "unsafe-bin", version: "1.0.0", bin: { "what-bin": "../../outside.js" } }), + ); + await writeFile( + join(xDir, "package.json"), + JSON.stringify({ + name: "bunx-bin-collision", + private: true, + devDependencies: { + "z-old-what-bin": "npm:what-bin@1.0.0", + "what-bin": "1.5.0", + "no-bin": "file:./no-bin", + "unsafe-bin": "file:./unsafe-bin", + }, + }), + ); + + const install = spawn({ + cmd: [bunExe(), "install"], + cwd: xDir, stdout: "pipe", - stdin: "inherit", stderr: "pipe", - env: { - ...env, - npm_config_registry: `http://localhost:${port}/`, - }, + env: { ...env, npm_config_registry: ctx.registry_url }, }); - - const [err, out, exited] = await Promise.all([ - subprocess.stderr.text(), - subprocess.stdout.text(), - subprocess.exited, + const [installOut, installErr, installExitCode] = await Promise.all([ + install.stdout.text(), + install.stderr.text(), + install.exited, ]); + expect({ installOut, installErr, installExitCode }).toMatchObject({ + installErr: expect.stringContaining("Saved lockfile"), + installExitCode: 0, + }); - return [err, out, exited, urls]; - }; + const sharedBin = Bun.which("what-bin", { PATH: join(xDir, "node_modules", ".bin") }); + expect(sharedBin).not.toBeNull(); + const shared = spawn({ + cmd: [sharedBin!], + cwd: xDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [sharedOut, sharedErr, sharedExitCode] = await Promise.all([ + shared.stdout.text(), + shared.stderr.text(), + shared.exited, + ]); + expect({ sharedOut, sharedErr, sharedExitCode }).toEqual({ + sharedOut: "", + sharedErr: "", + sharedExitCode: 0, + }); + expect(await Bun.file(join(xDir, "what-bin.txt")).text()).toBe("what-bin@1.5.0"); + await rm(join(xDir, "what-bin.txt")); + } it("should install specified package when binary differs from package name", async () => { - const urls: string[] = []; - - // Set up dummy registry with a package that has a different binary name - setHandler( - dummyRegistry(urls, { - "1.0.0": { - bin: { - "different-bin": "index.js", + await withTestContext(undefined, async ctx => { + const urls: string[] = []; + setContextHandler( + ctx, + dummyRegistryForContext(ctx, urls, { + "1.0.0": { + bin: { + "different-bin": "index.js", + }, + as: "1.0.0", }, - as: "1.0.0", - }, - }), - ); - - // Tarball already exists in test directory + }), + ); - // Without --package, bunx different-bin would fail - // With --package, we correctly install my-special-pkg - const subprocess = spawn({ - cmd: [bunExe(), "x", "--package", "my-special-pkg", "different-bin", "--help"], - cwd: x_dir, - stdout: "pipe", - stdin: "inherit", - stderr: "pipe", - env: { - ...env, - npm_config_registry: `http://localhost:${port}/`, - }, - }); + const subprocess = spawn({ + cmd: [bunExe(), "x", "--package", "my-special-pkg", "different-bin", "--help"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "inherit", + stderr: "pipe", + env: { ...env, npm_config_registry: ctx.registry_url }, + }); - const [err, out, exited] = await Promise.all([ - subprocess.stderr.text(), - subprocess.stdout.text(), - subprocess.exited, - ]); + const [err, out, exited] = await Promise.all([ + subprocess.stderr.text(), + subprocess.stdout.text(), + subprocess.exited, + ]); - expect(urls.some(url => url.includes("/my-special-pkg"))).toBe(true); - // The package should install successfully - expect(err).toContain("Saved lockfile"); + expect(urls.some(url => url.includes("/my-special-pkg"))).toBe(true); + expect(err).toContain("Saved lockfile"); + }); }); it("should support -p shorthand with mock registry", async () => { - const urls: string[] = []; - - setHandler( - dummyRegistry(urls, { - "2.0.0": { - bin: { - "tool": "cli.js", + await withTestContext(undefined, async ctx => { + const urls: string[] = []; + setContextHandler( + ctx, + dummyRegistryForContext(ctx, urls, { + "2.0.0": { + bin: { + "tool": "cli.js", + }, + as: "2.0.0", }, - as: "2.0.0", - }, - }), - ); + }), + ); - // Tarball already exists in test directory - - const subprocess = spawn({ - cmd: [bunExe(), "x", "-p", "actual-package", "tool", "--version"], - cwd: x_dir, - stdout: "pipe", - stdin: "inherit", - stderr: "pipe", - env: { - ...env, - npm_config_registry: `http://localhost:${port}/`, - }, - }); + const subprocess = spawn({ + cmd: [bunExe(), "x", "-p", "actual-package", "tool", "--version"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "inherit", + stderr: "pipe", + env: { ...env, npm_config_registry: ctx.registry_url }, + }); - const [err, out, exited] = await Promise.all([ - subprocess.stderr.text(), - subprocess.stdout.text(), - subprocess.exited, - ]); + const [err, out, exited] = await Promise.all([ + subprocess.stderr.text(), + subprocess.stdout.text(), + subprocess.exited, + ]); - expect(urls.some(url => url.includes("/actual-package"))).toBe(true); + expect(urls.some(url => url.includes("/actual-package"))).toBe(true); + }); }); it("should support --package= syntax with mock registry", async () => { - const urls: string[] = []; - - setHandler( - dummyRegistry(urls, { - "3.0.0": { - bin: { - "runner": "run.js", + await withTestContext(undefined, async ctx => { + const urls: string[] = []; + setContextHandler( + ctx, + dummyRegistryForContext(ctx, urls, { + "3.0.0": { + bin: { + "runner": "run.js", + }, + as: "3.0.0", }, - as: "3.0.0", - }, - }), - ); + }), + ); - // Tarball already exists in test directory - - const subprocess = spawn({ - cmd: [bunExe(), "x", "--package=runner-pkg", "runner", "--help"], - cwd: x_dir, - stdout: "pipe", - stdin: "inherit", - stderr: "pipe", - env: { - ...env, - npm_config_registry: `http://localhost:${port}/`, - }, - }); + const subprocess = spawn({ + cmd: [bunExe(), "x", "--package=runner-pkg", "runner", "--help"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "inherit", + stderr: "pipe", + env: { ...env, npm_config_registry: ctx.registry_url }, + }); - const [err, out, exited] = await Promise.all([ - subprocess.stderr.text(), - subprocess.stdout.text(), - subprocess.exited, - ]); + const [err, out, exited] = await Promise.all([ + subprocess.stderr.text(), + subprocess.stdout.text(), + subprocess.exited, + ]); - expect(urls.some(url => url.includes("/runner-pkg"))).toBe(true); + expect(urls.some(url => url.includes("/runner-pkg"))).toBe(true); + }); }); it("should fail to run alternate binary without --package flag", async () => { - // Attempt to run multi-tool-alt without --package flag - // This should fail because bunx would try to install a package named "multi-tool-alt" - const subprocess = spawn({ - cmd: [bunExe(), "x", "multi-tool-alt"], - cwd: x_dir, - stdout: "pipe", - stdin: "inherit", - stderr: "pipe", - env: { - ...env, - npm_config_registry: `http://localhost:${port}/`, - }, - }); + await withTestContext(undefined, async ctx => { + // Attempt to run multi-tool-alt without --package flag + // This should fail because bunx would try to install a package named "multi-tool-alt" + const subprocess = spawn({ + cmd: [bunExe(), "x", "multi-tool-alt"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "inherit", + stderr: "pipe", + env: { ...env, npm_config_registry: ctx.registry_url }, + }); - const [err, _out, exited] = await Promise.all([ - subprocess.stderr.text(), - subprocess.stdout.text(), - subprocess.exited, - ]); + const [err, _out, exited] = await Promise.all([ + subprocess.stderr.text(), + subprocess.stdout.text(), + subprocess.exited, + ]); - // Should fail because there's no package named "multi-tool-alt" - expect(err).toContain("error:"); - expect(exited).not.toBe(0); + // Should fail because there's no package named "multi-tool-alt" + expect(err).toContain("error:"); + expect(exited).not.toBe(0); + }); }); it("should execute the correct binary when package has multiple binaries", async () => { - const urls: string[] = []; + await withTestContext(undefined, async ctx => { + const urls: string[] = []; - // Create the tarball with both binaries that output different messages - // First, let's create the package structure - const tempDir = tmpdirSync(); - const packageDir = join(tempDir, "package"); + // Create the tarball with both binaries that output different messages + // First, let's create the package structure + using packageRoot = tempDir("bunx-multi-tool-package", {}); + const packageDir = join(packageRoot, "package"); - await Bun.$`mkdir -p ${packageDir}/bin`; + await Bun.$`mkdir -p ${packageDir}/bin`; - await writeFile( - join(packageDir, "package.json"), - JSON.stringify({ - name: "multi-tool-pkg", - version: "1.0.0", - bin: { - "multi-tool": "bin/multi-tool.js", - "multi-tool-alt": "bin/multi-tool-alt.js", - }, - }), - ); + await writeFile( + join(packageDir, "package.json"), + JSON.stringify({ + name: "multi-tool-pkg", + version: "1.0.0", + bin: { + "multi-tool": "bin/multi-tool.js", + "multi-tool-alt": "bin/multi-tool-alt.js", + }, + }), + ); - await writeFile( - join(packageDir, "bin", "multi-tool.js"), - `#!/usr/bin/env node + await writeFile( + join(packageDir, "bin", "multi-tool.js"), + `#!/usr/bin/env node console.log("EXECUTED: multi-tool (main binary)"); `, - ); + ); - await writeFile( - join(packageDir, "bin", "multi-tool-alt.js"), - `#!/usr/bin/env node + await writeFile( + join(packageDir, "bin", "multi-tool-alt.js"), + `#!/usr/bin/env node console.log("EXECUTED: multi-tool-alt (alternate binary)"); `, - ); + ); - // Make the binaries executable - await Bun.$`chmod +x ${packageDir}/bin/multi-tool.js ${packageDir}/bin/multi-tool-alt.js`; + // Make the binaries executable + await Bun.$`chmod +x ${packageDir}/bin/multi-tool.js ${packageDir}/bin/multi-tool-alt.js`; - // Create the tarball with package/ prefix. It goes to a temp dir the - // registry is pointed at — writing it under import.meta.dir would - // rewrite a checked-in file on every run. - const tgzDir = tmpdirSync(); - await Bun.$`cd ${tempDir} && tar -czf ${join(tgzDir, "multi-tool-pkg-1.0.0.tgz")} package`; + // Create the tarball with package/ prefix. It goes to a temp dir the + // registry is pointed at — writing it under import.meta.dir would + // rewrite a checked-in file on every run. + using tgzDir = tempDir("bunx-multi-tool-tarball", {}); + await Bun.$`tar -czf ${join(tgzDir, "multi-tool-pkg-1.0.0.tgz")} package`.cwd(packageRoot); - setHandler( - dummyRegistry( - urls, - { - "1.0.0": { - bin: { - "multi-tool": "bin/multi-tool.js", - "multi-tool-alt": "bin/multi-tool-alt.js", + setContextHandler( + ctx, + dummyRegistryForContext( + ctx, + urls, + { + "1.0.0": { + bin: { + "multi-tool": "bin/multi-tool.js", + "multi-tool-alt": "bin/multi-tool-alt.js", + }, + as: "1.0.0", }, - as: "1.0.0", }, - }, - 0, - tgzDir, - ), - ); + 0, + tgzDir, + ), + ); - // Test 1: Without --package, bunx multi-tool-alt should fail or install wrong package - // Test 2: With --package, we can run the alternate binary - const subprocess = spawn({ - cmd: [bunExe(), "x", "--package", "multi-tool-pkg", "multi-tool-alt"], - cwd: x_dir, - stdout: "pipe", - stdin: "inherit", - stderr: "pipe", - env: { - ...env, - npm_config_registry: `http://localhost:${port}/`, - }, + // Test 1: Without --package, bunx multi-tool-alt should fail or install wrong package + // Test 2: With --package, we can run the alternate binary + const subprocess = spawn({ + cmd: [bunExe(), "x", "--package", "multi-tool-pkg", "multi-tool-alt"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "inherit", + stderr: "pipe", + env: { ...env, npm_config_registry: ctx.registry_url }, + }); + + const [_err, out, exited] = await Promise.all([ + subprocess.stderr.text(), + subprocess.stdout.text(), + subprocess.exited, + ]); + + // Verify the correct package was requested + expect(urls.some(url => url.includes("/multi-tool-pkg"))).toBe(true); + + // Verify the correct binary was executed + expect(out).toContain("EXECUTED: multi-tool-alt (alternate binary)"); + expect(out).not.toContain("EXECUTED: multi-tool (main binary)"); + expect(exited).toBe(0); }); + }); - const [_err, out, exited] = await Promise.all([ - subprocess.stderr.text(), - subprocess.stdout.text(), - subprocess.exited, - ]); + describe.each(linkerCases)("with $linker linker", ({ linker }) => { + it.each(packageInvocationCases)("$invocation uses the named package's bin on naming collision", async invocationCase => { + await withTestContext({ linker }, async ctx => { + await installBinCollisionFixture(ctx); + const xDir = ctx.package_dir; + + const selected = spawn({ + ...packageInvocationCommand(invocationCase, "z-old-what-bin"), + cwd: xDir, + stdout: "pipe", + stderr: "pipe", + env: { ...env, npm_config_registry: ctx.registry_url }, + }); + const [selectedOut, selectedErr, selectedExitCode] = await Promise.all([ + selected.stdout.text(), + selected.stderr.text(), + selected.exited, + ]); + expect({ selectedOut, selectedErr, selectedExitCode }).toEqual({ + selectedOut: "", + selectedErr: "", + selectedExitCode: 0, + }); + expect(await Bun.file(join(xDir, "what-bin.txt")).text()).toBe("what-bin@1.0.0"); + }); + }); - // Verify the correct package was requested - expect(urls.some(url => url.includes("/multi-tool-pkg"))).toBe(true); + it("an explicit package never falls back to another package's bin", async () => { + await withTestContext({ linker }, async ctx => { + await installBinCollisionFixture(ctx); + const xDir = ctx.package_dir; + + const missing = spawn({ + cmd: [bunExe(), "x", "--package", "no-bin", "what-bin"], + cwd: xDir, + stdout: "pipe", + stderr: "pipe", + env: { ...env, npm_config_registry: ctx.registry_url }, + }); + const [missingOut, missingErr, missingExitCode] = await Promise.all([ + missing.stdout.text(), + missing.stderr.text(), + missing.exited, + ]); + expect({ missingOut, missingErr, missingExitCode }).toEqual({ + missingOut: "", + missingErr: expect.stringContaining("Package no-bin does not provide a binary named what-bin"), + missingExitCode: 1, + }); + expect(await Bun.file(join(xDir, "what-bin.txt")).exists()).toBe(false); + + const unsafe = spawn({ + cmd: [bunExe(), "x", "--package", "unsafe-bin", "what-bin"], + cwd: xDir, + stdout: "pipe", + stderr: "pipe", + env: { ...env, npm_config_registry: ctx.registry_url }, + }); + const [unsafeOut, unsafeErr, unsafeExitCode] = await Promise.all([ + unsafe.stdout.text(), + unsafe.stderr.text(), + unsafe.exited, + ]); + expect({ unsafeOut, unsafeErr, unsafeExitCode }).toEqual({ + unsafeOut: "", + unsafeErr: expect.stringContaining("Package unsafe-bin does not provide a binary named what-bin"), + unsafeExitCode: 1, + }); + expect(await Bun.file(join(xDir, "what-bin.txt")).exists()).toBe(false); + }); + }); + }); - // Verify the correct binary was executed - expect(out).toContain("EXECUTED: multi-tool-alt (alternate binary)"); - expect(out).not.toContain("EXECUTED: multi-tool (main binary)"); - expect(exited).toBe(0); + describe("cold-cache install", () => { + it.each(packageInvocationCases)("$invocation uses the named package's bin", async invocationCase => { + await withTestContext(undefined, async ctx => { + const urls: string[] = []; + const fixtureDir = join(import.meta.dir, "registry", "packages", "what-bin"); + setContextHandler( + ctx, + dummyRegistryForContext( + ctx, + urls, + { + "1.0.0": { + bin: { "what-bin": "what-bin.js" }, + dependencies: { "new-what-bin": "npm:what-bin@1.5.0" }, + }, + "1.5.0": { bin: { "what-bin": "what-bin.js" } }, + }, + 0, + fixtureDir, + ), + ); + + const xDir = ctx.package_dir; + const packageSpec = `z-cold-${invocationCase.invocation.replaceAll(" ", "-")}@npm:what-bin@1.0.0`; + const subprocess = spawn({ + ...packageInvocationCommand(invocationCase, packageSpec), + cwd: xDir, + stdout: "pipe", + stderr: "pipe", + env: { ...env, npm_config_registry: ctx.registry_url }, + }); + const [stdout, stderr, exitCode] = await Promise.all([ + subprocess.stdout.text(), + subprocess.stderr.text(), + subprocess.exited, + ]); + expect({ stdout, stderr, exitCode }).toMatchObject({ exitCode: 0 }); + expect(await Bun.file(join(xDir, "what-bin.txt")).text()).toBe("what-bin@1.0.0"); + await rm(join(xDir, "what-bin.txt")); + + const cacheEntry = (await readdirSorted(env.BUN_TMPDIR)).find(entry => entry.startsWith("bunx-")); + expect(cacheEntry).toBeDefined(); + const sharedBin = Bun.which("what-bin", { + PATH: join(env.BUN_TMPDIR, cacheEntry!, "node_modules", ".bin"), + }); + expect(sharedBin).not.toBeNull(); + const shared = spawn({ + cmd: [sharedBin!], + cwd: xDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [sharedOut, sharedErr, sharedExitCode] = await Promise.all([ + shared.stdout.text(), + shared.stderr.text(), + shared.exited, + ]); + expect({ sharedOut, sharedErr, sharedExitCode }).toEqual({ + sharedOut: "", + sharedErr: "", + sharedExitCode: 0, + }); + expect(await Bun.file(join(xDir, "what-bin.txt")).text()).toBe("what-bin@1.5.0"); + }); + }); }); }); }); From 0bfd62be8bea90ca7fb2d859e6f8654a3bec23a0 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Fri, 7 Aug 2026 20:12:10 +0200 Subject: [PATCH 3/7] bunx: address automated review findings - validate package name before building node_modules paths; make alias_is_safe_install_target pub for the bunx-side guard - return PathTooLong instead of panicking when a formatted path exactly fills its buffer; same for three pre-existing expect("unreachable") sites - skip a native-binlink candidate on path overflow instead of aborting the whole search - capture abs_dest_buf pointer before writes, matching link() - tests: drain piped stdout, parameterize bin name, assert bin output and exit codes, chmodSync over shell chmod - rebuild my-special-pkg/actual-package/runner-pkg fixture tarballs: #21517 shipped them with a literal '#\!' shebang (shell history-escaping artifact) plus macOS ._ AppleDouble entries, so executing their bin always failed with ENOEXEC; its assertions never checked the run, so the three package-selection tests passed while exercising nothing --- src/install/PackageInstaller.rs | 2 +- src/install/bin.rs | 21 ++++-- src/runtime/cli/bunx_command.rs | 24 +++++- test/cli/install/actual-package-2.0.0.tgz | Bin 606 -> 270 bytes .../bun-install-native-binlink.test.ts | 12 ++- test/cli/install/bunx.test.ts | 69 +++++++++++------- test/cli/install/my-special-pkg-1.0.0.tgz | Bin 620 -> 279 bytes test/cli/install/runner-pkg-3.0.0.tgz | Bin 605 -> 267 bytes 8 files changed, 88 insertions(+), 40 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index 4085df51cc22..2450afe92c95 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -411,7 +411,7 @@ impl<'a> LazyPackageDestinationDir<'a> { /// anything that could escape `node_modules`: empty names, `.`/`..` /// components, absolute paths, drive letters, backslashes, NUL bytes, and any /// separator other than the single `/` in a scoped name (`@scope/name`). -pub(crate) fn alias_is_safe_install_target(alias: &[u8]) -> bool { +pub fn alias_is_safe_install_target(alias: &[u8]) -> bool { if alias.is_empty() || alias.len() >= MAX_PATH_BYTES || strings::contains_any(alias, b"\\:\0") { return false; } diff --git a/src/install/bin.rs b/src/install/bin.rs index f4006e908534..869b585097bf 100644 --- a/src/install/bin.rs +++ b/src/install/bin.rs @@ -974,6 +974,8 @@ impl<'a> Linker<'a> { self.err = Some(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG)); return false; } + // Detached like `abs_dest_buf_ptr` in `link`; same SAFETY invariant. + let abs_dest_buf_ptr: *mut u8 = self.abs_dest_buf.as_mut_ptr(); let abs_target = { let package_dir = &self.abs_target_buf[..package_dir_len]; @@ -992,9 +994,9 @@ impl<'a> Linker<'a> { .copy_from_slice(destination_name); dest_off += destination_name.len(); self.abs_dest_buf[dest_off] = 0; - // SAFETY: `link_bin_or_create_shim` does not read or write - // `abs_dest_buf`; the detached slice remains valid for the call. - let abs_dest = unsafe { ZStr::from_raw(self.abs_dest_buf.as_ptr(), dest_off) }; + // SAFETY: abs_dest_buf[dest_off] == 0 written above; `link_bin_or_create_shim` + // does not read or write `abs_dest_buf`. + let abs_dest = unsafe { ZStr::from_raw(abs_dest_buf_ptr, dest_off) }; self.link_bin_or_create_shim( abs_target, @@ -2209,8 +2211,13 @@ fn resolve_installed_native_binlink_target( let node_modules_paths = [nested_node_modules, real_node_modules, root_node_modules]; for dependency in optional_dependencies { for node_modules_path in &node_modules_paths { - let mut target_package_dir: AbsPath = AbsPath::from(node_modules_path.slice()).ok()?; - target_package_dir.append(&dependency.install_name).ok()?; + let mut target_package_dir: AbsPath = match AbsPath::from(node_modules_path.slice()) { + Ok(path) => path, + Err(_) => continue, + }; + if target_package_dir.append(&dependency.install_name).is_err() { + continue; + } if platform_package_matches(target_package_dir.slice(), &dependency) { return Some(InstalledNativeBinlinkTarget { node_modules_path: AbsPath::from(node_modules_path.slice()).ok()?, @@ -2231,7 +2238,9 @@ pub fn link_package_bin<'a>( destination_name: &[u8], executable_buf: &'a mut PathBuffer, ) -> Result, crate::Error> { - if normalized_bin_name(destination_scope) != destination_scope { + if normalized_bin_name(destination_scope) != destination_scope + || !crate::package_installer::alias_is_safe_install_target(package_name) + { return Ok(None); } diff --git a/src/runtime/cli/bunx_command.rs b/src/runtime/cli/bunx_command.rs index f6ff88cd3f45..99caf8d9e317 100644 --- a/src/runtime/cli/bunx_command.rs +++ b/src/runtime/cli/bunx_command.rs @@ -458,9 +458,12 @@ impl BunxCommand { pkg = BStr::new(package_name), ), ) - .expect("unreachable"); + .map_err(|_| crate::Error::PathTooLong)?; total - cursor.len() }; + if len >= subpath.len() { + return Err(crate::Error::PathTooLong); + } subpath[len] = 0; // SAFETY: subpath[len] == 0 written above let subpath_z = ZStr::from_buf(&subpath[..], len); @@ -477,6 +480,9 @@ impl BunxCommand { if bin_name.is_some_and(|name| !Self::is_safe_bin_name(name)) { return Ok(PackageBinLookup::BinNotFound); } + if !bun_install::package_installer::alias_is_safe_install_target(package_name) { + return Ok(PackageBinLookup::PackageNotFound); + } let mut package_subpath = PathBuffer::uninit(); let package_subpath_len = { @@ -492,6 +498,9 @@ impl BunxCommand { .map_err(|_| crate::Error::PathTooLong)?; total - cursor.len() }; + if package_subpath_len >= package_subpath.len() { + return Err(crate::Error::PathTooLong); + } package_subpath[package_subpath_len] = 0; let package_subpath_z = ZStr::from_buf(&package_subpath, package_subpath_len); let package_fd = @@ -547,9 +556,12 @@ impl BunxCommand { BStr::new(tempdir_name), bun_paths::SEP as char, ) - .expect("unreachable"); + .map_err(|_| crate::Error::PathTooLong)?; total - cursor.len() }; + if len >= subpath.len() { + return Err(crate::Error::PathTooLong); + } subpath[len] = 0; // SAFETY: subpath[len] == 0 written above let subpath_z = ZStr::from_buf(&subpath[..], len); @@ -616,9 +628,12 @@ impl BunxCommand { sep = bun_paths::SEP as char, pkg = BStr::new(package_name), ) - .expect("unreachable"); + .map_err(|_| crate::Error::PathTooLong)?; total - cursor.len() }; + if len >= subpath.len() { + return Err(crate::Error::PathTooLong); + } subpath[len] = 0; // SAFETY: subpath[len] == 0 written above let subpath_z = ZStr::from_buf(&subpath[..], len); @@ -1143,6 +1158,9 @@ impl BunxCommand { ) .map_err(|_| crate::Error::PathTooLong)?; let written = total - cursor.len(); + if written >= cache_manifest_buf.len() { + return Err(crate::Error::PathTooLong); + } cache_manifest_buf[written] = 0; ZStr::from_buf(&cache_manifest_buf, written) }; diff --git a/test/cli/install/actual-package-2.0.0.tgz b/test/cli/install/actual-package-2.0.0.tgz index 4afdde22ae413c68cafd86896c8e3a3944315c47..45ac2a5a925c5e9ae24efaf62e725c69bd5a60a3 100644 GIT binary patch literal 270 zcmb2|=3oE==C@ZJ^P3Dr*bC0pb=`_hTXcb`^+@fkzS6eH-#o1P$KLPjd+0bRwVl`W zp$yXefnqViEf>wq*-Npwm<&Nf9Cmo_LHq;orUbrUYtFn zRWxr+r*7`NO7E1L)y~$-R&TppW?Exd^U-K`>E{pW&;RhJ-(Y<5Z}+kABi%Ot4gV{? zTm0v=lJ~^_*%n31%iGrf)VS+iXVAO;WNUb)PLg2Wp-B&3C+`|_#plF#GH5U`003)Agt-6! literal 606 zcmV-k0-^mMiwFRB=80$k1MQeiZ__Xw#}G@u;5 zhA;aT55u0Y!blE9zyn_xYwO0(Z|{Cx|3R)T;UP8W@q&+w zWy{-mi*`3YHp6FauZCT)tr~5;nbqhIjo1G5d#EZC)z^#cW1jxUdih)bHls@aq{u;K zR9yer_qQ@1Xnl%hu-rb>`=A&v;{OPy+W(b<_J5To)BltYT;e(~SN~J~kJ(83zfw5c z|3TN){;v{b`Y$f}&(}YKI3NFQOmzG&gF`dmePKQ@xt|Pp@BU;SnD@+~h~lmc)Fs=3 z7E~p>U3GS7CQ|lPso(3W*&fEWW8`!nA|NrmF^j}=`zrg-4qxtv`8PoB<3~s-_RUgDrU3vSu2!N!ui*Wa$R;@;((X1@L0l5>6Rsbdd0AOZ5EdT%j diff --git a/test/cli/install/bun-install-native-binlink.test.ts b/test/cli/install/bun-install-native-binlink.test.ts index 243ae3f07658..666e0cf3a65a 100644 --- a/test/cli/install/bun-install-native-binlink.test.ts +++ b/test/cli/install/bun-install-native-binlink.test.ts @@ -172,7 +172,11 @@ describe.concurrent("native binlink optimization", () => { stderr: "pipe", env, }); - const [installStderr, installExitCode] = await Promise.all([install.stderr.text(), install.exited]); + const [, installStderr, installExitCode] = await Promise.all([ + install.stdout.text(), + install.stderr.text(), + install.exited, + ]); expect(installStderr).not.toContain("error:"); expect(installExitCode).toBe(0); @@ -255,7 +259,11 @@ describe.concurrent("native binlink optimization", () => { stderr: "pipe", env, }); - const [installStderr, installExitCode] = await Promise.all([install.stderr.text(), install.exited]); + const [, installStderr, installExitCode] = await Promise.all([ + install.stdout.text(), + install.stderr.text(), + install.exited, + ]); expect(installStderr).not.toContain("error:"); expect(installExitCode).toBe(0); diff --git a/test/cli/install/bunx.test.ts b/test/cli/install/bunx.test.ts index cc2fb877c347..b9a2545a9334 100644 --- a/test/cli/install/bunx.test.ts +++ b/test/cli/install/bunx.test.ts @@ -78,9 +78,10 @@ async function withTestContext( function packageInvocationCommand( { useBunx, explicitPackage }: PackageInvocationCase, packageSpec: string, + binName: string, ): { cmd: string[]; argv0?: string } { const cmd = useBunx ? [bunExe()] : [bunExe(), "x"]; - cmd.push(...(explicitPackage ? ["--package", packageSpec, "what-bin"] : [packageSpec])); + cmd.push(...(explicitPackage ? ["--package", packageSpec, binName] : [packageSpec])); return useBunx ? { cmd, argv0: isWindows ? "bunx.exe" : "bunx" } : { cmd }; } @@ -743,6 +744,8 @@ describe("package selection", () => { expect(urls.some(url => url.includes("/my-special-pkg"))).toBe(true); expect(err).toContain("Saved lockfile"); + expect(out).toContain("different-bin from my-special-pkg"); + expect(exited).toBe(0); }); }); @@ -777,6 +780,9 @@ describe("package selection", () => { ]); expect(urls.some(url => url.includes("/actual-package"))).toBe(true); + expect(err).not.toContain("error:"); + expect(out).toContain("tool from actual-package"); + expect(exited).toBe(0); }); }); @@ -811,6 +817,9 @@ describe("package selection", () => { ]); expect(urls.some(url => url.includes("/runner-pkg"))).toBe(true); + expect(err).not.toContain("error:"); + expect(out).toContain("runner from runner-pkg"); + expect(exited).toBe(0); }); }); @@ -877,7 +886,8 @@ console.log("EXECUTED: multi-tool-alt (alternate binary)"); ); // Make the binaries executable - await Bun.$`chmod +x ${packageDir}/bin/multi-tool.js ${packageDir}/bin/multi-tool-alt.js`; + chmodSync(join(packageDir, "bin", "multi-tool.js"), 0o755); + chmodSync(join(packageDir, "bin", "multi-tool-alt.js"), 0o755); // Create the tarball with package/ prefix. It goes to a temp dir the // registry is pointed at — writing it under import.meta.dir would @@ -932,31 +942,34 @@ console.log("EXECUTED: multi-tool-alt (alternate binary)"); }); describe.each(linkerCases)("with $linker linker", ({ linker }) => { - it.each(packageInvocationCases)("$invocation uses the named package's bin on naming collision", async invocationCase => { - await withTestContext({ linker }, async ctx => { - await installBinCollisionFixture(ctx); - const xDir = ctx.package_dir; - - const selected = spawn({ - ...packageInvocationCommand(invocationCase, "z-old-what-bin"), - cwd: xDir, - stdout: "pipe", - stderr: "pipe", - env: { ...env, npm_config_registry: ctx.registry_url }, + it.each(packageInvocationCases)( + "$invocation uses the named package's bin on naming collision", + async invocationCase => { + await withTestContext({ linker }, async ctx => { + await installBinCollisionFixture(ctx); + const xDir = ctx.package_dir; + + const selected = spawn({ + ...packageInvocationCommand(invocationCase, "z-old-what-bin", "what-bin"), + cwd: xDir, + stdout: "pipe", + stderr: "pipe", + env: { ...env, npm_config_registry: ctx.registry_url }, + }); + const [selectedOut, selectedErr, selectedExitCode] = await Promise.all([ + selected.stdout.text(), + selected.stderr.text(), + selected.exited, + ]); + expect({ selectedOut, selectedErr, selectedExitCode }).toEqual({ + selectedOut: "", + selectedErr: "", + selectedExitCode: 0, + }); + expect(await Bun.file(join(xDir, "what-bin.txt")).text()).toBe("what-bin@1.0.0"); }); - const [selectedOut, selectedErr, selectedExitCode] = await Promise.all([ - selected.stdout.text(), - selected.stderr.text(), - selected.exited, - ]); - expect({ selectedOut, selectedErr, selectedExitCode }).toEqual({ - selectedOut: "", - selectedErr: "", - selectedExitCode: 0, - }); - expect(await Bun.file(join(xDir, "what-bin.txt")).text()).toBe("what-bin@1.0.0"); - }); - }); + }, + ); it("an explicit package never falls back to another package's bin", async () => { await withTestContext({ linker }, async ctx => { @@ -1029,7 +1042,7 @@ console.log("EXECUTED: multi-tool-alt (alternate binary)"); const xDir = ctx.package_dir; const packageSpec = `z-cold-${invocationCase.invocation.replaceAll(" ", "-")}@npm:what-bin@1.0.0`; const subprocess = spawn({ - ...packageInvocationCommand(invocationCase, packageSpec), + ...packageInvocationCommand(invocationCase, packageSpec, "what-bin"), cwd: xDir, stdout: "pipe", stderr: "pipe", @@ -1120,7 +1133,7 @@ describe("scoped packages should not match unrelated system binaries", () => { } else { const fakeBin = join(fakeBinDir, "install"); await writeFile(fakeBin, `#!/bin/sh\necho "WRONG: ran a system binary from PATH"\n`); - await Bun.$`chmod +x ${fakeBin}`; + chmodSync(fakeBin, 0o755); } const urls: string[] = []; diff --git a/test/cli/install/my-special-pkg-1.0.0.tgz b/test/cli/install/my-special-pkg-1.0.0.tgz index dd7cce5c2ad05936900f3f93ef080b1dba0c44e3..dd84b07da34da75c5076567bbbd23092b76729da 100644 GIT binary patch literal 279 zcmb2|=3oE==C@ZbW;Gj#uqCYjEs|Nb`QVi$vpDWvPLWbwt(J7_QsA$5r@i*BFknmG z*8KPW*@-S5Pd><3hvtPWzMSEvqtRk2KUNz4pDv}= zuz7#&vGil9^Xli;$N7BLH-7S8t?$Yzzssg`TYk-a_w_;V=EQDwoy0^{9)?|ZasDkE bZ2wukc-i6z@j09<;Fk~QP1I-5U|;|MK(mSd literal 620 zcmV-y0+an8iwFR5=80$k1MQeiZ__Xw#FBi1`D>RAzXFhQCaUfOyXg>Zk+kx8u<$(>) z_kXI0ZEXPmdlLO;xBI8-A1$8$2qt>}&x89P?^O=tsIu?LijYGfgSIHSfs6yC)qc=@ zV79$ZM?^vn%Bm0OM1c?dqjDSy*W>+i*z1}PA8YwuRc8KwT2e6}kgJ_T>46b)8UIfS z)Bk_-LHqwgv*`a<9|uzPzu^BJ44L--d2o^c$3*-8B9Q1myWKxs{|L?df2u5j-v9I9 z*p%EC=B7F4=^vRx5yf60RTHwH1yzmm|K{fL*#m)UPL7V|$%&S+Mx)VO6@CFnaG1^j GCIA4^i#*2w diff --git a/test/cli/install/runner-pkg-3.0.0.tgz b/test/cli/install/runner-pkg-3.0.0.tgz index 166a49a3a76a316e1d274c9e8e911ae80f830e90..69de648f1b1fd495213070cf4ade809bc74ba77a 100644 GIT binary patch literal 267 zcmb2|=3oE==C@bQW*s)*VgE4ax5&Km%tY@u7JdtAXRZ0{wEszh{@yA3s<)c8ZZ#;& zymj#3{_Y-w6AIqBw((bcPfudh42!Jpd1EDa{?@Z&V!Jk_p8I(!VlIdElf~zD{fJ~& zd>8miKkN6u_P$H0GRuyw|GWN7>i?~qcKNhOJ+qwtuk!!=`J43G-sC#YT)bA{|2wy0 z|BLHmcYQk>@#(mN_!0LH(uG$0x2<;bOIz%2FPrzk=6~%I*#|GLneKDjqthk-Jbv|` ztbe7MBG3NIl}&opJ307g?zZ?R*IIXcIFdPeRY+l*c*~r3zoM7F{uBPBuHO{mTR8cl OfZg1HHJU+#fdK&jq=g>< literal 605 zcmV-j0;2sNiwFRI=80$k1MQeiZ__Xs$FoTY(OuvGXA>?%gVgV7hIS|jsg=fCHzD4n zDeK%)x;VjUtD22y+X-6xVb{H}3dKFGaQD_Ln(vI;_!{^LcVB z;HLL zdKfr<6demEQejU>U)F`Swr>6W_WswcAMEA|ZqhJ|mwvw@{3D@?>Fomx^^f3${wZhF z0{s-$GDXhyPap_7tw8J`myr^8J%}CT!cqwn<`tL=cB_xewI`M8>!(iN?{t*&FS+iN zUscOn@6h4a$4d0PRxVHv?CC+fZ|4mb1c&TKresmAG=)ZpQX8| z5jofY==XOHR98Bm;xy)1-v3cTnel&_VEkXM8R>uQ16RKfEbsp~bcykQSun%@c^GpK z8~=CZ8RHY~?>{n{y#kE=|FETZfl#h?4y^}H=`#MGQEtwEv%&cPTr=(eSHBM| z)c=(KcQG>OznL)W{1;PX{C^G@=|8>fzgYhW;>rB4ErH4ZS#V-YKM?k|ea_lHuzNzq rtx)Q2;y?%L9{vBo-aZ*dCE@yAu}@Epat#K9;ZpJ&_Ffz~044wcUL`k% From c3fea799269ae92282302cea61cb870c77702df9 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Fri, 7 Aug 2026 20:35:20 +0200 Subject: [PATCH 4/7] bunx: fix scoped native binlink resolution, harden probes - resolve_installed_native_binlink_target derived the real node_modules dir with a single dirname; for a scoped package that lands on the @scope directory, so the realpath candidate never matched and the isolated linker fell back to the main package bin. Strip one more component for scoped names. Covered by new scoped fixtures (`@binlink-scope/test-native-binlink`) and a per-linker bunx test that fails on the isolated linker without the fix. - a probe failure (unreadable node_modules, malformed package.json, failed symlink) aborted bunx before the install fallback could repair it; probes now degrade to PackageNotFound with a debug log - log 'refusing untrusted cached binary' in the post-install lookups, matching every other trust check in exec() - cold-cache test: assert exactly one bunx cache entry instead of taking the first match --- src/install/bin.rs | 8 +- src/runtime/cli/bunx_command.rs | 48 +++++- .../bun-install-native-binlink.test.ts | 61 ++++++++ test/cli/install/bunx.test.ts | 8 +- .../test-native-binlink/package.json | 25 ++++ .../test-native-binlink-1.0.0.tgz | Bin 0 -> 401 bytes .../create-native-binlink-scoped-packages.ts | 140 ++++++++++++++++++ .../package.json | 28 ++++ ...est-native-binlink-scoped-target-1.0.0.tgz | Bin 0 -> 385 bytes 9 files changed, 309 insertions(+), 9 deletions(-) create mode 100644 test/cli/install/registry/packages/@binlink-scope/test-native-binlink/package.json create mode 100644 test/cli/install/registry/packages/@binlink-scope/test-native-binlink/test-native-binlink-1.0.0.tgz create mode 100644 test/cli/install/registry/packages/create-native-binlink-scoped-packages.ts create mode 100644 test/cli/install/registry/packages/test-native-binlink-scoped-target/package.json create mode 100644 test/cli/install/registry/packages/test-native-binlink-scoped-target/test-native-binlink-scoped-target-1.0.0.tgz diff --git a/src/install/bin.rs b/src/install/bin.rs index 869b585097bf..cd989711a639 100644 --- a/src/install/bin.rs +++ b/src/install/bin.rs @@ -974,7 +974,6 @@ impl<'a> Linker<'a> { self.err = Some(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG)); return false; } - // Detached like `abs_dest_buf_ptr` in `link`; same SAFETY invariant. let abs_dest_buf_ptr: *mut u8 = self.abs_dest_buf.as_mut_ptr(); let abs_target = { @@ -2205,7 +2204,12 @@ fn resolve_installed_native_binlink_target( let package_dir_z = resolve_path::z(package_dir.slice(), &mut *package_dir_z_buf); let mut real_package_dir_buf = path::path_buffer_pool::get(); let real_package_dir = sys::realpath(package_dir_z, &mut *real_package_dir_buf).ok()?; - AbsPath::from(resolve_path::dirname::(real_package_dir)).ok()? + let mut parent = resolve_path::dirname::(real_package_dir); + // `node_modules/@scope/name` nests one level deeper than `node_modules/name`. + if strings::contains_char(package_name, b'/') { + parent = resolve_path::dirname::(parent); + } + AbsPath::from(parent).ok()? }; let node_modules_paths = [nested_node_modules, real_node_modules, root_node_modules]; diff --git a/src/runtime/cli/bunx_command.rs b/src/runtime/cli/bunx_command.rs index 99caf8d9e317..f63f156e342b 100644 --- a/src/runtime/cli/bunx_command.rs +++ b/src/runtime/cli/bunx_command.rs @@ -539,6 +539,34 @@ impl BunxCommand { } } + /// A probe failure must not abort bunx; the install fallback repairs or bypasses whatever the probe tripped over. + fn probe_bin_from_installed_package<'a>( + transpiler: &mut Transpiler, + package_name: &[u8], + bin_name: Option<&[u8]>, + install_root: &[u8], + executable_buf: &'a mut PathBuffer, + ) -> PackageBinLookup<'a> { + match Self::link_bin_from_installed_package( + transpiler, + package_name, + bin_name, + install_root, + executable_buf, + ) { + Ok(lookup) => lookup, + Err(err) => { + bun_output::scoped_log!( + bunx, + "package bin probe failed in {}: {}", + BStr::new(install_root), + err + ); + PackageBinLookup::PackageNotFound + } + } + } + fn get_bin_name_from_temp_directory( transpiler: &mut Transpiler, tempdir_name: &[u8], @@ -1190,13 +1218,13 @@ impl BunxCommand { let mut is_package_owned_bin = false; let dest_or_cache: Option<&ZStr> = 'find: { if update_request.version.literal.is_empty() { - match Self::link_bin_from_installed_package( + match Self::probe_bin_from_installed_package( this_transpiler, result_package_name, opts.binary_name, top_level_dir, &mut package_bin_abs_buf, - )? { + ) { PackageBinLookup::Found(d) => { is_package_owned_bin = true; break 'find Some(d); @@ -1210,13 +1238,13 @@ impl BunxCommand { PackageBinLookup::PackageNotFound => {} } } - match Self::link_bin_from_installed_package( + match Self::probe_bin_from_installed_package( this_transpiler, result_package_name, opts.binary_name, bunx_cache_dir, &mut package_bin_abs_buf, - )? { + ) { PackageBinLookup::Found(d) => { is_package_owned_bin = true; break 'find Some(d); @@ -1705,6 +1733,12 @@ impl BunxCommand { passthrough, None, )?; + } else { + bun_output::scoped_log!( + bunx, + "refusing untrusted cached binary: {}", + BStr::new(out) + ); } } PackageBinLookup::PackageNotFound | PackageBinLookup::BinNotFound => { @@ -1732,6 +1766,12 @@ impl BunxCommand { passthrough, None, )?; + } else { + bun_output::scoped_log!( + bunx, + "refusing untrusted cached binary: {}", + BStr::new(out) + ); } } PackageBinLookup::BinNotFound => { diff --git a/test/cli/install/bun-install-native-binlink.test.ts b/test/cli/install/bun-install-native-binlink.test.ts index 666e0cf3a65a..a2b4e84c6d70 100644 --- a/test/cli/install/bun-install-native-binlink.test.ts +++ b/test/cli/install/bun-install-native-binlink.test.ts @@ -137,6 +137,67 @@ describe.concurrent("native binlink optimization", () => { await expectPlatformBin(); }); + // With the isolated linker the platform package is only reachable through the realpath-derived + // node_modules candidate, which must strip an extra path component for a scoped package name. + test("bunx resolves the platform bin of a scoped native package", async () => { + const env = { ...bunEnv }; + const { packageDir, packageJson } = await verdaccio.createTestDir(); + env.BUN_INSTALL_CACHE_DIR = join(packageDir, ".bun-cache"); + env.BUN_TMPDIR = env.TMPDIR = env.TEMP = join(packageDir, ".bun-tmp"); + + await writeFile( + join(packageDir, "bunfig.toml"), + toTOMLString({ + install: { + cache: join(packageDir, ".bun-cache"), + registry: verdaccio.registryUrl(), + linker, + }, + }), + ); + await writeFile( + packageJson, + JSON.stringify({ + name: "test-app", + version: "1.0.0", + dependencies: { "@binlink-scope/test-native-binlink": "1.0.0" }, + nativeDependencies: ["@binlink-scope/test-native-binlink"], + trustedDependencies: ["@binlink-scope/test-native-binlink"], + }), + ); + + await using install = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [, installStderr, installExitCode] = await Promise.all([ + install.stdout.text(), + install.stderr.text(), + install.exited, + ]); + expect(installStderr).not.toContain("error:"); + expect(installExitCode).toBe(0); + + await using bunx = spawn({ + cmd: [bunExe(), "x", "--package", "@binlink-scope/test-native-binlink", "test-binlink-scoped-cmd"], + cwd: packageDir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [stdout, stderr, exitCode] = await Promise.all([bunx.stdout.text(), bunx.stderr.text(), bunx.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: "SUCCESS: Using platform-specific bin (test-native-binlink-scoped-target)\n", + stderr: "", + exitCode: 0, + }); + }); + test("ignores an installed native package that does not satisfy the optional dependency", async () => { const env = { ...bunEnv }; const { packageDir, packageJson } = await verdaccio.createTestDir(); diff --git a/test/cli/install/bunx.test.ts b/test/cli/install/bunx.test.ts index b9a2545a9334..0eacfdb16aa6 100644 --- a/test/cli/install/bunx.test.ts +++ b/test/cli/install/bunx.test.ts @@ -1057,10 +1057,12 @@ console.log("EXECUTED: multi-tool-alt (alternate binary)"); expect(await Bun.file(join(xDir, "what-bin.txt")).text()).toBe("what-bin@1.0.0"); await rm(join(xDir, "what-bin.txt")); - const cacheEntry = (await readdirSorted(env.BUN_TMPDIR)).find(entry => entry.startsWith("bunx-")); - expect(cacheEntry).toBeDefined(); + const cacheEntries = (await readdirSorted(env.BUN_TMPDIR)).filter(entry => entry.startsWith("bunx-")); + // beforeEach gives each case its own BUN_TMPDIR. + expect(cacheEntries).toHaveLength(1); + const cacheEntry = cacheEntries[0]; const sharedBin = Bun.which("what-bin", { - PATH: join(env.BUN_TMPDIR, cacheEntry!, "node_modules", ".bin"), + PATH: join(env.BUN_TMPDIR, cacheEntry, "node_modules", ".bin"), }); expect(sharedBin).not.toBeNull(); const shared = spawn({ diff --git a/test/cli/install/registry/packages/@binlink-scope/test-native-binlink/package.json b/test/cli/install/registry/packages/@binlink-scope/test-native-binlink/package.json new file mode 100644 index 000000000000..389bb86ca7a5 --- /dev/null +++ b/test/cli/install/registry/packages/@binlink-scope/test-native-binlink/package.json @@ -0,0 +1,25 @@ +{ + "_id": "@binlink-scope/test-native-binlink", + "name": "@binlink-scope/test-native-binlink", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": { + "1.0.0": { + "name": "@binlink-scope/test-native-binlink", + "version": "1.0.0", + "_id": "@binlink-scope/test-native-binlink@1.0.0", + "bin": { + "test-binlink-scoped-cmd": "./bin/main.js" + }, + "optionalDependencies": { + "test-native-binlink-scoped-target": "1.0.0" + }, + "dist": { + "integrity": "sha512-t1IJYVu3SueyzsG4Ne3oNdjGjOdIV6aquSvSggCT6iVBPk3bk3Bia/rOVC1cHs+FKsqAIWbBVF68xSgQWp1Yrw==", + "shasum": "bef5bdbdb53886720dd44627040d3374c6e94b38", + "tarball": "http://localhost:4873/@binlink-scope/test-native-binlink/-/test-native-binlink-1.0.0.tgz" + } + } + } +} diff --git a/test/cli/install/registry/packages/@binlink-scope/test-native-binlink/test-native-binlink-1.0.0.tgz b/test/cli/install/registry/packages/@binlink-scope/test-native-binlink/test-native-binlink-1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..cd2a502bbe5e5260fa23cd8b437ed17f1c1c4a9f GIT binary patch literal 401 zcmV;C0dD>uiwFP!000001MSsMYlAQp0C3NKiqu^;sMe^d&|P-e&V#}}K&G#fRueI4 z8H|4S#ZFmUTFRh*7<<1i!>9clCc4@?vM;_w|ohjP<{Y?fO?-$Y8F|{*1Q3 z<0Ls+f6C&$`X}}$z#-w;cXGf#um9a(wA9U}{2*6EsvO)*Nv%o$u@FVpGkbVA;= zkOgT^2l!jK6WPFfcy#MVWuSEcAHv*IJ8|n*MF;f#-;K8Y zl|1(|PW68r)BXOBk{J8{8a`b@JjpBYCd7NPEnNz^@O7qY7#YyUw=XSL;CFYtVY_|> zjTTC_Av6es@O#!Su`_K#d$Eb#NtgRsmA4_m_MeSsm#;VBsx~$!FQ1_XnS;y(=)HWq vd+F2K`W0Y)Hq&Nw*SG$#2qAZ&4o)(&%$Oy9#O!@;S`IB>Ur))i6<>&fbw)l(AHtir0tN9lL*{%3c1iQl0HT@b~<~c!>NjVn6?aOXV%i$=}fi zoJ7&V{Hfc26x%It&Glvm+M8(nM}vyr^#f* zrbem-D|24Ybyc{g6q%gMjL}xstA(juIv=?fE){aAR<6l(DP}Gm??Tjlr~hb|mCl4Q zp4iB`=TqU@zaF~&-;LD%TAlhG$NE2{dq4U=jL`oV@oqEbC|(F>#GIpD>^!u~*P=2~ zt9D1=`JVq()rJbsG^6>9S8q~n>vToWX*S&j)rX-&^Ox Date: Fri, 7 Aug 2026 21:06:31 +0200 Subject: [PATCH 5/7] install: reset bin-linker state before native-binlink retry - the isolated installer's retry-without-native-binlink kept the first attempt's err; link_bin_or_create_shim treats a pre-set err as its own failure and unlinks the retry's symlink, so the retry was a no-op whenever err (not skipped_due_to_missing_bin) triggered it. Reset both fields in both retry blocks, matching bin.rs link_package_bin. The hoisted installer rebuilds its Linker per iteration and was unaffected. - dummy registry: extract the duplicated version-map construction into buildVersions, note that 'latest' falls back to insertion order - scoped-fixture generator: clean up temp dirs in a finally block --- src/install/isolated_install/Installer.rs | 4 + test/cli/install/dummy.registry.ts | 60 +++-- .../create-native-binlink-scoped-packages.ts | 210 +++++++++--------- 3 files changed, 139 insertions(+), 135 deletions(-) diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index 5c2b62bb52f0..20eed5f5fde1 100644 --- a/src/install/isolated_install/Installer.rs +++ b/src/install/isolated_install/Installer.rs @@ -1882,6 +1882,8 @@ impl Task { bin_linker.target_node_modules_path = bin_linker.node_modules_path; bin_linker.target_package_name = strings::StringOrTinyString::init(dep_name); + bin_linker.err = None; + bin_linker.skipped_due_to_missing_bin = false; if manager_ref.options.log_level.is_verbose() { bun_core::pretty_errorln!( @@ -2377,6 +2379,8 @@ impl<'a> Installer<'a> { if bin_linker.should_retry_without_native_binlink() { bin_linker.target_node_modules_path = bin_linker.node_modules_path; bin_linker.target_package_name = package_name; + bin_linker.err = None; + bin_linker.skipped_due_to_missing_bin = false; if self.manager().options.log_level.is_verbose() { bun_core::pretty_errorln!( diff --git a/test/cli/install/dummy.registry.ts b/test/cli/install/dummy.registry.ts index 676583ae98e1..178976e0703e 100644 --- a/test/cli/install/dummy.registry.ts +++ b/test/cli/install/dummy.registry.ts @@ -162,6 +162,32 @@ export function setContextHandler(ctx: TestContext, newHandler: Handler): void { ctx.handler = newHandler; } +function buildVersions( + info: DummyRegistryInfo, + name: string, + tarballPrefix: string, +): { versions: Record; latestVersion: string | undefined } { + const versions: Record = {}; + // Without an explicit `info.latest`, `latest` resolves to the last valid + // version key in object insertion order, not the highest version. + let latestVersion: string | undefined; + for (const version in info) { + if (!/^[0-9]/.test(version)) continue; + const metadata = info[version]; + if (!metadata || typeof metadata !== "object") continue; + latestVersion = version; + versions[version] = { + name, + version, + dist: { + tarball: `${tarballPrefix}-${metadata.as ?? version}.tgz`, + }, + ...metadata, + }; + } + return { versions, latestVersion }; +} + /** * Creates a dummy registry handler for a specific test context. * This is the concurrent-safe version that uses the context's registry_url for tarballs. @@ -214,22 +240,7 @@ export function dummyRegistryForContext( const pathAfterPrefix = urlObj.pathname.replace(`/${ctx.id}/`, "/"); const name = pathAfterPrefix.slice(1); // Remove leading slash - const versions: Record = {}; - let latestVersion: string | undefined; - for (const version in info) { - if (!/^[0-9]/.test(version)) continue; - const metadata = info[version]; - if (!metadata || typeof metadata !== "object") continue; - latestVersion = version; - versions[version] = { - name, - version, - dist: { - tarball: `${ctx.registry_url}${name}-${metadata.as ?? version}.tgz`, - }, - ...metadata, - }; - } + const { versions, latestVersion } = buildVersions(info, name, `${ctx.registry_url}${name}`); return new Response( JSON.stringify({ @@ -290,22 +301,7 @@ export function dummyRegistry( expect(await request.text()).toBe(""); const name = url.slice(url.indexOf("/", root_url.length) + 1); - const versions: Record = {}; - let latestVersion: string | undefined; - for (const version in info) { - if (!/^[0-9]/.test(version)) continue; - const metadata = info[version]; - if (!metadata || typeof metadata !== "object") continue; - latestVersion = version; - versions[version] = { - name, - version, - dist: { - tarball: `${url}-${metadata.as ?? version}.tgz`, - }, - ...metadata, - }; - } + const { versions, latestVersion } = buildVersions(info, name, url); return new Response( JSON.stringify({ diff --git a/test/cli/install/registry/packages/create-native-binlink-scoped-packages.ts b/test/cli/install/registry/packages/create-native-binlink-scoped-packages.ts index 403d4b51808e..7d27f746ae9d 100644 --- a/test/cli/install/registry/packages/create-native-binlink-scoped-packages.ts +++ b/test/cli/install/registry/packages/create-native-binlink-scoped-packages.ts @@ -9,7 +9,7 @@ */ import { $ } from "bun"; -import { mkdir, writeFile } from "fs/promises"; +import { mkdir, rm, writeFile } from "fs/promises"; import { join } from "path"; const packagesDir = import.meta.dir; @@ -19,122 +19,126 @@ const mainName = `${scope}/test-native-binlink`; const targetName = "test-native-binlink-scoped-target"; const version = "1.0.0"; -// Main package that should NOT be used const mainPkgDir = join(packagesDir, "test-native-binlink-scoped-tmp"); -await mkdir(join(mainPkgDir, "package", "bin"), { recursive: true }); - -await writeFile( - join(mainPkgDir, "package", "package.json"), - JSON.stringify( - { - name: mainName, - version, - bin: { - "test-binlink-scoped-cmd": "./bin/main.js", - }, - optionalDependencies: { - [targetName]: version, +const targetPkgDir = join(packagesDir, `${targetName}-tmp`); + +try { + // Main package that should NOT be used + await mkdir(join(mainPkgDir, "package", "bin"), { recursive: true }); + + await writeFile( + join(mainPkgDir, "package", "package.json"), + JSON.stringify( + { + name: mainName, + version, + bin: { + "test-binlink-scoped-cmd": "./bin/main.js", + }, + optionalDependencies: { + [targetName]: version, + }, }, - }, - null, - 2, - ), -); - -await writeFile( - join(mainPkgDir, "package", "bin", "main.js"), - `#!/usr/bin/env node + null, + 2, + ), + ); + + await writeFile( + join(mainPkgDir, "package", "bin", "main.js"), + `#!/usr/bin/env node console.log("ERROR: Using main package bin, not platform-specific!"); process.exit(1); `, -); - -await mkdir(join(packagesDir, scope, "test-native-binlink"), { recursive: true }); -await $`cd ${mainPkgDir} && tar -czf ${join(packagesDir, scope, "test-native-binlink", `test-native-binlink-${version}.tgz`)} package`; - -// Platform-specific package -const targetPkgDir = join(packagesDir, `${targetName}-tmp`); -await mkdir(join(targetPkgDir, "package", "bin"), { recursive: true }); - -await writeFile( - join(targetPkgDir, "package", "package.json"), - JSON.stringify( - { - name: targetName, - version, - os: ["darwin", "linux", "win32"], - cpu: ["arm64", "x64"], - }, - null, - 2, - ), -); - -// Use the SAME filename as the main package! -await writeFile( - join(targetPkgDir, "package", "bin", "main.js"), - `#!/usr/bin/env node -console.log("SUCCESS: Using platform-specific bin (${targetName})"); -process.exit(0); -`, -); - -await mkdir(join(packagesDir, targetName), { recursive: true }); -await $`cd ${targetPkgDir} && tar -czf ${join(packagesDir, targetName, `${targetName}-${version}.tgz`)} package`; + ); -// Create package.json for verdaccio registry with proper integrity hashes -for (const [pkgName, pkgDir, tarballName] of [ - [mainName, join(packagesDir, scope, "test-native-binlink"), `test-native-binlink-${version}.tgz`], - [targetName, join(packagesDir, targetName), `${targetName}-${version}.tgz`], -] as const) { - const tarballBytes = await Bun.file(join(pkgDir, tarballName)).arrayBuffer(); - const hash = new Bun.CryptoHasher("sha512"); - hash.update(tarballBytes); - const integrity = `sha512-${Buffer.from(hash.digest()).toString("base64")}`; + await mkdir(join(packagesDir, scope, "test-native-binlink"), { recursive: true }); + await $`cd ${mainPkgDir} && tar -czf ${join(packagesDir, scope, "test-native-binlink", `test-native-binlink-${version}.tgz`)} package`; - const sha1Hash = new Bun.CryptoHasher("sha1"); - sha1Hash.update(tarballBytes); - const shasum = Buffer.from(sha1Hash.digest()).toString("hex"); + // Platform-specific package + await mkdir(join(targetPkgDir, "package", "bin"), { recursive: true }); await writeFile( - join(pkgDir, "package.json"), - `${JSON.stringify( + join(targetPkgDir, "package", "package.json"), + JSON.stringify( { - _id: pkgName, - name: pkgName, - "dist-tags": { - latest: version, - }, - versions: { - [version]: { - name: pkgName, - version, - _id: `${pkgName}@${version}`, - bin: pkgName === mainName ? { "test-binlink-scoped-cmd": "./bin/main.js" } : undefined, - optionalDependencies: - pkgName === mainName - ? { - [targetName]: version, - } - : undefined, - os: pkgName === targetName ? ["darwin", "linux", "win32"] : undefined, - cpu: pkgName === targetName ? ["arm64", "x64"] : undefined, - dist: { - integrity, - shasum, - tarball: `http://localhost:4873/${pkgName}/-/${tarballName}`, - }, - }, - }, + name: targetName, + version, + os: ["darwin", "linux", "win32"], + cpu: ["arm64", "x64"], }, null, 2, - )}\n`, + ), + ); + + // Use the SAME filename as the main package! + await writeFile( + join(targetPkgDir, "package", "bin", "main.js"), + `#!/usr/bin/env node +console.log("SUCCESS: Using platform-specific bin (${targetName})"); +process.exit(0); +`, ); -} -// Clean up temp directories -await $`rm -rf ${mainPkgDir}`; -await $`rm -rf ${targetPkgDir}`; + await mkdir(join(packagesDir, targetName), { recursive: true }); + await $`cd ${targetPkgDir} && tar -czf ${join(packagesDir, targetName, `${targetName}-${version}.tgz`)} package`; + + // Create package.json for verdaccio registry with proper integrity hashes + for (const [pkgName, pkgDir, tarballName] of [ + [mainName, join(packagesDir, scope, "test-native-binlink"), `test-native-binlink-${version}.tgz`], + [targetName, join(packagesDir, targetName), `${targetName}-${version}.tgz`], + ] as const) { + const tarballBytes = await Bun.file(join(pkgDir, tarballName)).arrayBuffer(); + const hash = new Bun.CryptoHasher("sha512"); + hash.update(tarballBytes); + const integrity = `sha512-${Buffer.from(hash.digest()).toString("base64")}`; + + const sha1Hash = new Bun.CryptoHasher("sha1"); + sha1Hash.update(tarballBytes); + const shasum = Buffer.from(sha1Hash.digest()).toString("hex"); + + await writeFile( + join(pkgDir, "package.json"), + `${JSON.stringify( + { + _id: pkgName, + name: pkgName, + "dist-tags": { + latest: version, + }, + versions: { + [version]: { + name: pkgName, + version, + _id: `${pkgName}@${version}`, + bin: pkgName === mainName ? { "test-binlink-scoped-cmd": "./bin/main.js" } : undefined, + optionalDependencies: + pkgName === mainName + ? { + [targetName]: version, + } + : undefined, + os: pkgName === targetName ? ["darwin", "linux", "win32"] : undefined, + cpu: pkgName === targetName ? ["arm64", "x64"] : undefined, + dist: { + integrity, + shasum, + tarball: `http://localhost:4873/${pkgName}/-/${tarballName}`, + }, + }, + }, + }, + null, + 2, + )}\n`, + ); + } +} finally { + await Promise.all([ + rm(mainPkgDir, { recursive: true, force: true }), + rm(targetPkgDir, { recursive: true, force: true }), + ]); +} console.log("✅ Created scoped native binlink test packages"); From 92443324a7426ed9ede07eda2a53ec2eaf98e0f9 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Fri, 7 Aug 2026 19:42:14 +0200 Subject: [PATCH 6/7] bunx: support anonymous URL packages Remote tarball URLs do not provide a package or bin name before installation. Install them under a deterministic internal alias, then use package-owned bin discovery to select and link the executable from package.json. Cover `bun x` and `bunx` cold installs, warm cache reuse, and `--no-install` errors with a hermetic opaque URL fixture. Fixes #3675 --- src/runtime/cli/bunx_command.rs | 84 +++++++++++++++++++++++---------- test/cli/install/bunx.test.ts | 68 ++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 26 deletions(-) diff --git a/src/runtime/cli/bunx_command.rs b/src/runtime/cli/bunx_command.rs index f63f156e342b..89b592e5c541 100644 --- a/src/runtime/cli/bunx_command.rs +++ b/src/runtime/cli/bunx_command.rs @@ -15,7 +15,7 @@ use bun_bundler::Transpiler; use bun_collections::BoundedArray; use bun_core::{self, Global, Output}; use bun_core::{ZStr, strings}; -use bun_install::dependency::VersionTag; +use bun_install::dependency::{URI, VersionTag}; use bun_install::update_request::{self, UpdateRequest}; use bun_parsers::json; use bun_paths::{self, DELIMITER, PathBuffer}; @@ -887,6 +887,27 @@ impl BunxCommand { } } + let display_version: &[u8] = if update_request.version.literal.is_empty() { + b"latest" + } else { + update_request + .version + .literal + .slice(update_request.version_buf()) + }; + + let anonymous_package_alias = if update_request.name.is_empty() + && opts.binary_name.is_none() + && update_request.version.tag == VersionTag::Tarball + && matches!(update_request.version.tarball().uri, URI::Remote(_)) + { + let mut alias = Vec::new(); + write!(&mut alias, "bunx-url-{:x}", hash(display_version)) + .map_err(|_| crate::Error::Alloc(bun_alloc::AllocError))?; + Some(alias) + } else { + None + }; // When the user types a scoped package like `@foo/bar`, the initial bin // name ("bar") is only a guess — the package's actual bin may be named // something else entirely. In that case we must not search the original @@ -917,6 +938,9 @@ impl BunxCommand { } else { update_request.name }; + let fallback_package_name = anonymous_package_alias + .as_deref() + .unwrap_or(initial_bin_name); bun_output::scoped_log!(bunx, "initial_bin_name: {}", BStr::new(initial_bin_name)); // fast path: they're actually using this interchangeably with `bun run` @@ -985,15 +1009,6 @@ impl BunxCommand { }; // Cloned to avoid borrowck overlap when PATH is reassigned below. - let display_version: &[u8] = if update_request.version.literal.is_empty() { - b"latest" - } else { - update_request - .version - .literal - .slice(update_request.version_buf()) - }; - // package_fmt is used for the path to install in. let package_fmt: Vec = 'brk: { // Includes the delimiters because we use this as a part of $PATH @@ -1016,7 +1031,7 @@ impl BunxCommand { write!( &mut v, "{}@{}@{}", - BStr::new(initial_bin_name), + BStr::new(fallback_package_name), <&'static str>::from(update_request.version.tag), hash(update_request.name).wrapping_add(hash(display_version)), ) @@ -1048,19 +1063,22 @@ impl BunxCommand { .map_err(|_| crate::Error::Alloc(bun_alloc::AllocError))?; (v, update_request.name) } else { - // When there is not a clear package name (URL/GitHub/etc), we force the package name - // to be the same as the calculated initial bin name. This allows us to have a predictable - // node_modules folder structure. + // Unnamed sources need a deterministic alias for their node_modules path. let mut v = Vec::new(); write!( &mut v, "{}@{}", - BStr::new(initial_bin_name), + BStr::new(fallback_package_name), BStr::new(display_version), ) .map_err(|_| crate::Error::Alloc(bun_alloc::AllocError))?; - (v, initial_bin_name) + (v, fallback_package_name) }; + let package_name_for_error = if anonymous_package_alias.is_some() { + opts.package_name + } else { + result_package_name + }; bun_output::scoped_log!(bunx, "install_param: {}", BStr::new(&install_param)); bun_output::scoped_log!( bunx, @@ -1231,7 +1249,7 @@ impl BunxCommand { } PackageBinLookup::BinNotFound => { Self::exit_package_bin_not_found( - result_package_name, + package_name_for_error, opts.binary_name, ); } @@ -1250,7 +1268,10 @@ impl BunxCommand { break 'find Some(d); } PackageBinLookup::BinNotFound => { - Self::exit_package_bin_not_found(result_package_name, opts.binary_name); + Self::exit_package_bin_not_found( + package_name_for_error, + opts.binary_name, + ); } PackageBinLookup::PackageNotFound => {} } @@ -1281,6 +1302,9 @@ impl BunxCommand { break 'find Some(d); } } + if initial_bin_name.is_empty() { + break 'find None; + } bun_which::which( &mut path_buf, bunx_cache_dir, @@ -1375,7 +1399,7 @@ impl BunxCommand { if opts.no_install { bun_core::warn!( "Using a stale installation of {} because --no-install was passed. Run `bunx` without --no-install to use a fresh binary.", - BStr::new(&update_request.name), + BStr::new(package_name_for_error), ); } else { break 'try_run_existing; @@ -1528,10 +1552,17 @@ impl BunxCommand { // Which is not very helpful. if opts.no_install { - Output::err_generic( - "Could not find an existing '{}' binary to run. Stopping because --no-install was passed.", - format_args!("{}", BStr::new(initial_bin_name)), - ); + if initial_bin_name.is_empty() { + Output::err_generic( + "Could not find an existing installation for package '{}'. Stopping because --no-install was passed.", + format_args!("{}", BStr::new(package_name_for_error)), + ); + } else { + Output::err_generic( + "Could not find an existing '{}' binary to run. Stopping because --no-install was passed.", + format_args!("{}", BStr::new(initial_bin_name)), + ); + } Global::exit(1); } @@ -1742,7 +1773,7 @@ impl BunxCommand { } } PackageBinLookup::PackageNotFound | PackageBinLookup::BinNotFound => { - Self::exit_package_bin_not_found(result_package_name, opts.binary_name); + Self::exit_package_bin_not_found(package_name_for_error, opts.binary_name); } } } else { @@ -1775,7 +1806,7 @@ impl BunxCommand { } } PackageBinLookup::BinNotFound => { - Self::exit_package_bin_not_found(result_package_name, None); + Self::exit_package_bin_not_found(package_name_for_error, None); } PackageBinLookup::PackageNotFound => {} } @@ -1786,6 +1817,7 @@ impl BunxCommand { // 1. Try the bin in the global cache // Do not try $PATH because we already checked it above if we should if opts.specified_package.is_none() + && !initial_bin_name.is_empty() && let Some(destination) = bun_which::which( &mut path_buf, bunx_cache_dir, @@ -1891,7 +1923,7 @@ impl BunxCommand { } else { Output::err_generic( "could not determine executable to run for package {}", - format_args!("{}", BStr::new(&update_request.name)), + format_args!("{}", BStr::new(package_name_for_error)), ); } Global::exit(1); diff --git a/test/cli/install/bunx.test.ts b/test/cli/install/bunx.test.ts index 0eacfdb16aa6..060b09f6410e 100644 --- a/test/cli/install/bunx.test.ts +++ b/test/cli/install/bunx.test.ts @@ -61,6 +61,7 @@ const packageInvocationCases: PackageInvocationCase[] = [ { invocation: "bun x", useBunx: false, explicitPackage: false }, { invocation: "bunx", useBunx: true, explicitPackage: false }, ]; +const implicitPackageInvocationCases = packageInvocationCases.filter(({ explicitPackage }) => !explicitPackage); const linkerCases: LinkerCase[] = [{ linker: "hoisted" }, { linker: "isolated" }]; async function withTestContext( @@ -338,6 +339,73 @@ it.concurrent("should work for github repository", async () => { expect(exited).toBe(0); }); +it.concurrent.each(implicitPackageInvocationCases)( + "$invocation discovers the bin from an anonymous URL package", + async invocationCase => { + const { x_dir, env } = setup(); + const tarball = Bun.gzipSync( + await new Bun.Archive({ + "package/package.json": JSON.stringify({ + name: "actual-url-package", + version: "1.0.0", + bin: { "actual-url-cli": "cli.js" }, + }), + "package/cli.js": `#!/usr/bin/env bun +console.log("url-package:" + process.argv.slice(2).join(",")); +`, + }).bytes(), + ); + const requests: string[] = []; + await using server = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch(request) { + const url = new URL(request.url); + requests.push(`${request.method} ${url.pathname}`); + return new Response(tarball); + }, + }); + const run = async (packageUrl: string, argument: string, noInstall = false) => { + const command = packageInvocationCommand(invocationCase, packageUrl); + if (noInstall) command.cmd.splice(invocationCase.useBunx ? 1 : 2, 0, "--no-install"); + command.cmd.push(argument); + const subprocess = spawn({ + ...command, + cwd: x_dir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [stdout, stderr, exitCode] = await Promise.all([ + subprocess.stdout.text(), + subprocess.stderr.text(), + subprocess.exited, + ]); + return { stdout, stderr, exitCode }; + }; + + const uncachedUrl = new URL("/opaque/not-cached@8", server.url).href; + const uncached = await run(uncachedUrl, "uncached", true); + expect(uncached).toEqual({ + stdout: "", + stderr: `error: Could not find an existing installation for package '${uncachedUrl}'. Stopping because --no-install was passed.\n`, + exitCode: 1, + }); + expect(requests).toEqual([]); + + const packageUrl = new URL("/opaque/download@8", server.url).href; + const cold = await run(packageUrl, "cold"); + expect(cold).toMatchObject({ stdout: "url-package:cold\n", exitCode: 0 }); + expect(cold.stderr).not.toContain("unrecognised dependency format"); + expect(requests).toEqual(["GET /opaque/download@8"]); + + const warm = await run(packageUrl, "warm"); + expect(warm).toMatchObject({ stdout: "url-package:warm\n", exitCode: 0 }); + expect(warm.stderr).not.toContain("unrecognised dependency format"); + expect(requests).toEqual(["GET /opaque/download@8"]); + }, +); + it.concurrent("should work for github repository with committish", async () => { const { x_dir, env } = setup(); const withoutCache = spawn({ From 256f152e8da008b9882fc054cc6f9f49c638fd47 Mon Sep 17 00:00:00 2001 From: Kaj Kowalski Date: Sun, 9 Aug 2026 21:36:14 +0200 Subject: [PATCH 7/7] bunx: log the binary selected after install The post-install execution paths ran without a scoped log, so a BUN_DEBUG_bunx trace ended at the install and never showed which executable was chosen. Only the rejecting branches logged. --- src/runtime/cli/bunx_command.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/runtime/cli/bunx_command.rs b/src/runtime/cli/bunx_command.rs index 89b592e5c541..da238e7558e3 100644 --- a/src/runtime/cli/bunx_command.rs +++ b/src/runtime/cli/bunx_command.rs @@ -1754,6 +1754,11 @@ impl BunxCommand { PackageBinLookup::Found(destination) => { let out = destination.as_bytes(); if Self::is_trusted_cached_binary(destination, uid) { + bun_output::scoped_log!( + bunx, + "running installed binary: {}", + BStr::new(out) + ); let stored = fs.dirname_store.append_slice(out)?; Run::run_binary( ctx, @@ -1787,6 +1792,11 @@ impl BunxCommand { PackageBinLookup::Found(destination) => { let out = destination.as_bytes(); if Self::is_trusted_cached_binary(destination, uid) { + bun_output::scoped_log!( + bunx, + "running installed binary: {}", + BStr::new(out) + ); let stored = fs.dirname_store.append_slice(out)?; Run::run_binary( ctx, @@ -1835,6 +1845,7 @@ impl BunxCommand { // attacker can race the install and plant a uid-mismatched entry. // Bail out to the generic error rather than execute it. if Self::is_trusted_cached_binary(destination, uid) { + bun_output::scoped_log!(bunx, "running installed binary: {}", BStr::new(out)); let stored = fs.dirname_store.append_slice(out)?; Run::run_binary( ctx, @@ -1895,6 +1906,11 @@ impl BunxCommand { let out: &[u8] = destination.as_bytes(); // Same TOCTOU hardening as the post-install probe above. if Self::is_trusted_cached_binary(destination, uid) { + bun_output::scoped_log!( + bunx, + "running installed binary: {}", + BStr::new(out) + ); let stored = fs.dirname_store.append_slice(out)?; Run::run_binary( ctx,