diff --git a/src/runtime/cli/pack_command.rs b/src/runtime/cli/pack_command.rs index 4b767be83280..82f00a0f2482 100644 --- a/src/runtime/cli/pack_command.rs +++ b/src/runtime/cli/pack_command.rs @@ -1446,7 +1446,7 @@ fn get_bundled_deps( // ─────────────────────────────────────────────────────────────────────────── #[derive(Clone, Copy, PartialEq, Eq)] -enum BinType { +pub(crate) enum BinType { File, Dir, } @@ -1463,13 +1463,9 @@ fn get_package_bins(json: &Expr) -> Result, AllocError> { if let Some(bin) = json.as_property(b"bin") { if let Some(bin_str) = bin.expr.as_string(pack_bump()) { - let normalized = resolve_path::normalize_buf::( - bin_str, - &mut path_buf, - ); - if !bin_path_escapes_root(normalized) { + if let Some(subpath) = bin_subpath(bin_str, BinType::File, &mut path_buf) { bins.push(BinInfo { - path: ZBox::from_bytes(normalized), + path: ZBox::from_bytes(subpath), ty: BinType::File, }); } @@ -1484,13 +1480,16 @@ fn get_package_bins(json: &Expr) -> Result, AllocError> { for bin_prop in bin_obj.properties.slice() { if let Some(bin_prop_value) = &bin_prop.value { if let Some(bin_str) = bin_prop_value.as_string(pack_bump()) { - let normalized = resolve_path::normalize_buf::( - bin_str, - &mut path_buf, - ); - if !bin_path_escapes_root(normalized) { + let Some(subpath) = bin_subpath(bin_str, BinType::File, &mut path_buf) + else { + continue; + }; + let already_listed = bins.iter().any(|existing| { + strings::eql_long(existing.path.as_bytes(), subpath, true) + }); + if !already_listed { bins.push(BinInfo { - path: ZBox::from_bytes(normalized), + path: ZBox::from_bytes(subpath), ty: BinType::File, }); } @@ -1506,13 +1505,9 @@ fn get_package_bins(json: &Expr) -> Result, AllocError> { if let ExprData::EObject(directories_obj) = &directories.expr.data { if let Some(bin) = directories_obj.as_property(b"bin") { if let Some(bin_str) = bin.expr.as_string(pack_bump()) { - let normalized = resolve_path::normalize_buf::( - bin_str, - &mut path_buf, - ); - if !bin_path_escapes_root(normalized) { + if let Some(subpath) = bin_subpath(bin_str, BinType::Dir, &mut path_buf) { bins.push(BinInfo { - path: ZBox::from_bytes(normalized), + path: ZBox::from_bytes(subpath), ty: BinType::Dir, }); } @@ -1524,8 +1519,21 @@ fn get_package_bins(json: &Expr) -> Result, AllocError> { Ok(bins) } -fn bin_path_escapes_root(p: &[u8]) -> bool { - path::is_absolute_loose(p) || p == b".." || p.starts_with(b"../") +pub(crate) fn bin_subpath<'a>(value: &[u8], ty: BinType, buf: &'a mut [u8]) -> Option<&'a [u8]> { + let normalized: &'a [u8] = + resolve_path::normalize_buf::(value, buf); + let subpath = match ty { + BinType::Dir => strings::without_trailing_slash(normalized), + BinType::File if normalized.ends_with(b"/") || normalized == b"package.json" => { + return None; + } + BinType::File => normalized, + }; + (!is_package_root_or_outside(subpath)).then_some(subpath) +} + +pub(crate) fn is_package_root_or_outside(p: &[u8]) -> bool { + p.is_empty() || p == b"." || path::is_absolute_loose(p) || p == b".." || p.starts_with(b"../") } fn is_package_bin(bins: &[BinInfo], maybe_bin_path: &[u8]) -> bool { @@ -1537,9 +1545,8 @@ fn is_package_bin(bins: &[BinInfo], maybe_bin_path: &[u8]) -> bool { } } BinType::Dir => { - let bin_without_trailing = strings::without_trailing_slash(bin.path.as_bytes()); - if maybe_bin_path.starts_with(bin_without_trailing) { - let remain = &maybe_bin_path[bin_without_trailing.len()..]; + if maybe_bin_path.starts_with(bin.path.as_bytes()) { + let remain = &maybe_bin_path[bin.path.as_bytes().len()..]; if remain.len() > 1 && remain[0] == b'/' && strings::index_of_char(&remain[1..], b'/').is_none() diff --git a/src/runtime/cli/publish_command.rs b/src/runtime/cli/publish_command.rs index ac6e1736eb84..73742a515d48 100644 --- a/src/runtime/cli/publish_command.rs +++ b/src/runtime/cli/publish_command.rs @@ -1610,6 +1610,11 @@ impl PublishCommand { None } + fn bin_target<'a>(value: &[u8], path_buf: &'a mut [u8]) -> Option<&'a ZStr> { + let target: &'a ZStr = normalize_buf_z::(value, path_buf); + (!pack::is_package_root_or_outside(target.as_bytes())).then_some(target) + } + fn normalize_bin( json: &mut Expr, bump: &bun_alloc::Arena, @@ -1629,31 +1634,26 @@ impl PublishCommand { match &bin_query.expr.data { ExprData::EString(bin_str) => { let mut bin_props: Vec = Vec::new(); - let normalized = strings::without_prefix_comptime_z( - normalize_buf_z::( - bin_str.string(bump)?, - &mut *path_buf, - ), - b"./", - ); - if !bun_sys::exists_at(workspace_root, normalized) { - bun_core::warn!( - "bin '{}' does not exist", - bstr::BStr::new(normalized.as_bytes()), - ); - } + if let Some(value) = Self::bin_target(bin_str.string(bump)?, &mut *path_buf) { + if !bun_sys::exists_at(workspace_root, value) { + bun_core::warn!( + "bin '{}' does not exist", + bstr::BStr::new(value.as_bytes()), + ); + } - bin_props.push(G::Property { - key: Some(Expr::init( - E::String::init(leak!(package_name)), - bun_ast::Loc::EMPTY, - )), - value: Some(Expr::init( - E::String::init(leak!(normalized.as_bytes())), - bun_ast::Loc::EMPTY, - )), - ..Default::default() - }); + bin_props.push(G::Property { + key: Some(Expr::init( + E::String::init(leak!(package_name)), + bun_ast::Loc::EMPTY, + )), + value: Some(Expr::init( + E::String::init(leak!(value.as_bytes())), + bun_ast::Loc::EMPTY, + )), + ..Default::default() + }); + } json.data .e_object_mut() @@ -1695,32 +1695,17 @@ impl PublishCommand { continue; } - let value: Option = 'value: { - if let Some(value) = &bin_prop.value { - if let Some(vs) = value.data.as_e_string() { - if vs.len() != 0 { - break 'value Some(bun_core::ZBox::from_bytes( - strings::without_prefix_comptime_z( - // replace separators - normalize_buf_z::( - vs.string(bump)?, - &mut *path_buf, - ), - b"./", - ) - .as_bytes(), - )); - } - } - } - None + let Some(value) = + bin_prop.value.as_ref().and_then(|v| v.data.as_e_string()) + else { + continue; }; - let Some(value) = value else { continue }; - if value.is_empty() { + let Some(value) = Self::bin_target(value.string(bump)?, &mut *path_buf) + else { continue; - } + }; - if !bun_sys::exists_at(workspace_root, &value) { + if !bun_sys::exists_at(workspace_root, value) { bun_core::warn!( "bin '{}' does not exist", bstr::BStr::new(value.as_bytes()), @@ -1761,16 +1746,12 @@ impl PublishCommand { return Ok(()); }; let mut bin_props: Vec = Vec::new(); - let normalized_bin_dir = bun_core::ZBox::from_bytes( - strings::without_trailing_slash(strings::without_prefix( - normalize_buf::(bin_dir_str, &mut *path_buf), - b"./", - )), - ); - - if normalized_bin_dir.is_empty() { + let Some(bin_dir_subpath) = + pack::bin_subpath(bin_dir_str, pack::BinType::Dir, &mut *path_buf) + else { return Ok(()); - } + }; + let normalized_bin_dir = bun_core::ZBox::from_bytes(bin_dir_subpath); let bin_dir = match bun_sys::openat( workspace_root, diff --git a/test/cli/install/bun-pack.test.ts b/test/cli/install/bun-pack.test.ts index b89765712514..42601173b06b 100644 --- a/test/cli/install/bun-pack.test.ts +++ b/test/cli/install/bun-pack.test.ts @@ -1647,6 +1647,154 @@ describe("bins", () => { }, ]); }); + + // Paths resolve against the package root, as in npm, so all four name one file. + test("the same file under several bin names is packed once", async () => { + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "pack-bins-same-file", + version: "1.0.0", + bin: { + "one": "cli.js", + "two": "cli.js", + "three": "./cli.js", + "four": "../cli.js", + }, + }), + ), + write(join(packageDir, "cli.js"), "console.log('hello')"), + ]); + + const { out } = await pack(packageDir, bunEnv); + + const tarball = readTarball(join(packageDir, "pack-bins-same-file-1.0.0.tgz")); + expect(tarball.entries).toMatchObject([{ pathname: "package/package.json" }, { pathname: "package/cli.js" }]); + expect(tarball.entries[1].perm & 0o111).toBe(0o111); + expect(out).toContain("Total files: 2"); + }); + + // None of these name a file that can be packed as a bin: the package root, a + // file spelled as a directory, a directory, and the root package.json (which + // is always in the tarball). The package packs as if "bin" were absent. + test.each(["", ".", "cli.js/", "lib/", "package.json"])('"bin" of %p is ignored', async bin => { + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "pack-bins-not-a-file", + version: "1.0.0", + bin, + }), + ), + write(join(packageDir, "cli.js"), "console.log('cli')"), + write(join(packageDir, "lib", "a.js"), "console.log('a')"), + ]); + + await pack(packageDir, bunEnv); + + const tarball = readTarball(join(packageDir, "pack-bins-not-a-file-1.0.0.tgz")); + expect( + tarball.entries.map(entry => ({ pathname: entry.pathname, executable: (entry.perm & 0o111) !== 0 })), + ).toEqual([ + { pathname: "package/package.json", executable: false }, + { pathname: "package/cli.js", executable: false }, + { pathname: "package/lib/a.js", executable: false }, + ]); + }); + + test("ignored entries of a bin object do not affect the others", async () => { + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "pack-bins-partly-ignored", + version: "1.0.0", + bin: { + "dir": "lib/", + "cli": "cli.js", + "pkg": "package.json", + }, + }), + ), + write(join(packageDir, "cli.js"), "console.log('cli')"), + write(join(packageDir, "lib", "a.js"), "console.log('a')"), + ]); + + await pack(packageDir, bunEnv); + + const tarball = readTarball(join(packageDir, "pack-bins-partly-ignored-1.0.0.tgz")); + expect( + tarball.entries.map(entry => ({ pathname: entry.pathname, executable: (entry.perm & 0o111) !== 0 })), + ).toEqual([ + { pathname: "package/package.json", executable: false }, + { pathname: "package/cli.js", executable: true }, + { pathname: "package/lib/a.js", executable: false }, + ]); + }); + + // The bin directory is packed by its own walk, and the walk over the rest of + // the package has to skip it. Each `files` value below routes that skip + // through a different walk. + describe('"directories.bin" with a trailing slash', () => { + test.each([ + [undefined, ["package/index.js", "package/lib/bins/bin.js", "package/lib/index.js"]], + [["index.js"], ["package/index.js", "package/lib/bins/bin.js"]], + [["lib"], ["package/lib/bins/bin.js", "package/lib/index.js"]], + [["lib/bins"], ["package/lib/bins/bin.js"]], + ])("files: %p", async (files, expected) => { + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "pack-bins-dir-trailing-slash", + version: "1.0.0", + files, + directories: { + bin: "./lib/bins/", + }, + }), + ), + write(join(packageDir, "index.js"), "console.log('index')"), + write(join(packageDir, "lib", "index.js"), "console.log('lib')"), + write(join(packageDir, "lib", "bins", "bin.js"), "console.log('bin')"), + ]); + + await pack(packageDir, bunEnv); + + const tarball = readTarball(join(packageDir, "pack-bins-dir-trailing-slash-1.0.0.tgz")); + expect(tarball.entries.map(entry => entry.pathname)).toEqual(["package/package.json", ...expected]); + const bin = tarball.entries.find(entry => entry.pathname === "package/lib/bins/bin.js"); + expect(bin.perm & 0o111).toBe(0o111); + }); + }); + + test.each(["", ".", "./"])('"directories.bin" of %p (the package root) is ignored', async bin => { + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "pack-bins-dir-root", + version: "1.0.0", + directories: { + bin, + }, + }), + ), + write(join(packageDir, "index.js"), "console.log('index')"), + write(join(packageDir, "lib", "a.js"), "console.log('a')"), + ]); + + await pack(packageDir, bunEnv); + + const tarball = readTarball(join(packageDir, "pack-bins-dir-root-1.0.0.tgz")); + expect(tarball.entries).toMatchObject([ + { pathname: "package/package.json" }, + { pathname: "package/index.js" }, + { pathname: "package/lib/a.js" }, + ]); + }); }); test("unicode", async () => { diff --git a/test/cli/install/bun-publish.test.ts b/test/cli/install/bun-publish.test.ts index c7f9df4fc406..c6e3caea89ff 100644 --- a/test/cli/install/bun-publish.test.ts +++ b/test/cli/install/bun-publish.test.ts @@ -1207,6 +1207,56 @@ describe("readme", () => { }); }); +// The manifest "bin" that reaches the registry. Paths are resolved against the +// package root the way npm does it ("../cli.js" is "cli.js"), a value that +// resolves to the root itself is dropped, other values are kept as written (npm +// keeps "lib/" as well), and "directories.bin" is expanded with the same rule +// the tarball uses. +describe("bin in the published manifest", () => { + test.each([ + [{ bin: "cli.js" }, { "bin-pkg": "cli.js" }], + [{ bin: "../cli.js" }, { "bin-pkg": "cli.js" }], + [{ bin: "" }, {}], + [{ bin: "." }, {}], + [{ bin: { x: "lib/", y: "./cli.js", z: "", w: ".", v: "../../cli.js" } }, { x: "lib/", y: "cli.js", v: "cli.js" }], + [{ directories: { bin: "bins/" } }, { "a.js": "bins/a.js" }], + [{ directories: { bin: "" } }, undefined], + [{ directories: { bin: "." } }, undefined], + ])("%j", async (fields, expected) => { + let captured: any = null; + using mock = Bun.serve({ + port: 0, + async fetch(req) { + if (req.method === "PUT") captured = await req.json(); + return new Response("OK", { status: 200 }); + }, + }); + + const packageDir = tmpdirSync(); + await Promise.all([ + write( + join(packageDir, "bunfig.toml"), + Bun.TOML.stringify({ + install: { + cache: false, + registry: { url: `http://localhost:${mock.port}`, token: "unused" }, + }, + }), + ), + write(join(packageDir, "package.json"), JSON.stringify({ name: "bin-pkg", version: "1.0.0", ...fields })), + write(join(packageDir, "cli.js"), "#!/usr/bin/env node\n"), + write(join(packageDir, "lib", "a.js"), "module.exports = 1;"), + write(join(packageDir, "bins", "a.js"), "#!/usr/bin/env node\n"), + ]); + + const { err, exitCode } = await publish(env, packageDir); + expect(err).not.toContain("error:"); + expect(exitCode).toBe(0); + + expect(captured.versions["1.0.0"].bin).toEqual(expected); + }); +}); + test("dist.tarball in the published manifest does not include userinfo from the registry url", async () => { let captured: any = null; using mock = Bun.serve({