diff --git a/src/bun.zig b/src/bun.zig index a1cf3361bfa..2c40e7f64e0 100644 --- a/src/bun.zig +++ b/src/bun.zig @@ -2283,20 +2283,33 @@ pub inline fn serializableInto(comptime T: type, init: anytype) T { return result.*; } +/// Mode passed to `mkdirat(2)` when creating directories. +/// +/// POSIX applies the process umask on top of this, so the final permission is +/// `0o777 & ~umask`. With the default umask of `0o022` this yields `0o755` +/// (same as before), but `umask 0o002` yields `0o775` — letting multi-user +/// setups work like they do with Node, npm, pnpm, and `/bin/mkdir`. +pub const umask_mkdir_mode: sys.Mode = 0o777; + /// Like std.fs.Dir.makePath except instead of infinite looping on dangling -/// symlink, it deletes the symlink and tries again. +/// symlink, it deletes the symlink and tries again. Uses `umask_mkdir_mode` +/// so the kernel honors the process umask. pub fn makePath(dir: std.fs.Dir, sub_path: []const u8) !void { var it = try std.fs.path.componentIterator(sub_path); var component = it.last() orelse return; while (true) { - dir.makeDir(component.path) catch |err| switch (err) { + makeDirUmask(dir, component.path) catch |err| switch (err) { error.PathAlreadyExists => { var path_buf2: [MAX_PATH_BYTES * 2]u8 = undefined; copy(u8, &path_buf2, component.path); path_buf2[component.path.len] = 0; const path_to_use = path_buf2[0..component.path.len :0]; - const result = try sys.lstat(path_to_use).unwrap(); + // The mkdirat above and the deleteTree below are both + // relative to `dir`. Stat the same dir-fd so the decision + // lines up — `sys.lstat` would resolve against cwd, which + // is wrong whenever `dir` isn't cwd. + const result = try sys.lstatat(.fromStdDir(dir), path_to_use).unwrap(); const is_dir = S.ISDIR(@intCast(result.mode)); // dangling symlink if (!is_dir) { @@ -2314,6 +2327,13 @@ pub fn makePath(dir: std.fs.Dir, sub_path: []const u8) !void { } } +/// Like std.fs.Dir.makeDir but uses `umask_mkdir_mode` so the kernel +/// applies the process umask to the final permissions. +fn makeDirUmask(dir: std.fs.Dir, sub_path: []const u8) std.posix.MakeDirError!void { + if (Environment.isWindows) return dir.makeDir(sub_path); + return std.posix.mkdirat(dir.fd, sub_path, umask_mkdir_mode); +} + /// Like std.fs.Dir.makePath except instead of infinite looping on dangling /// symlink, it deletes the symlink and tries again. pub fn makePathW(dir: std.fs.Dir, sub_path: []const u16) !void { @@ -2457,6 +2477,8 @@ pub const MakePath = struct { } } + /// On POSIX, uses `bun.umask_mkdir_mode` when creating directories so the + /// kernel honors the process umask (matches `mkdir(1)` / Node / npm). pub fn makeOpenPath(self: std.fs.Dir, sub_path: anytype, opts: std.fs.Dir.OpenOptions) !std.fs.Dir { if (comptime Environment.isWindows) { return makeOpenPathAccessMaskW( @@ -2472,12 +2494,27 @@ pub const MakePath = struct { ); } - return self.makeOpenPath(sub_path, opts); + // POSIX: avoid std's makeOpenPath which hardcodes mode 0o755. Try + // openDir first, then fall back to the file-level `bun.makePath` + // (umask-honoring mkdirat plus dangling-symlink handling) and retry. + // The qualified `bun.makePath` is unambiguous — only the bare + // `makePath` identifier is shadowed by `MakePath.makePath` below, + // and `MakePath.makePath` infinite-loops on dangling intermediate + // symlinks. + return self.openDir(sub_path, opts) catch |err| switch (err) { + error.FileNotFound => { + try bun.makePath(self, sub_path); + return self.openDir(sub_path, opts); + }, + else => |e| return e, + }; } /// copy/paste of `std.fs.Dir.makePath` and related functions and modified to support u16 slices. /// inside `MakePath` scope to make deleting later easier. /// TODO(dylan-conway) delete `MakePath` + /// + /// On POSIX, uses `bun.umask_mkdir_mode` so the kernel honors the process umask. pub fn makePath(comptime T: type, self: std.fs.Dir, sub_path: []const T) !void { if (Environment.isWindows) { var dir = try makeOpenPath(self, sub_path, .{}); @@ -2488,7 +2525,7 @@ pub const MakePath = struct { var it = try componentIterator(T, sub_path); var component = it.last() orelse return; while (true) { - std.fs.Dir.makeDir(self, component.path) catch |err| switch (err) { + std.posix.mkdirat(self.fd, component.path, bun.umask_mkdir_mode) catch |err| switch (err) { error.PathAlreadyExists => { // TODO stat the file and return an error if it's not a directory // this is important because otherwise a dangling symlink diff --git a/src/install/PackageInstall.rs b/src/install/PackageInstall.rs index abea7ad087c..3123d6f5a15 100644 --- a/src/install/PackageInstall.rs +++ b/src/install/PackageInstall.rs @@ -1065,7 +1065,7 @@ impl<'a> PackageInstall<'a> { while let Some(entry) = walker.next()? { match entry.kind { EntryKind::Directory => { - let _ = sys::mkdirat(destination_dir_, entry.path, 0o755); + let _ = sys::mkdirat(destination_dir_, entry.path, sys::UMASK_MKDIR_MODE); } EntryKind::File => { let path_len = entry.path.len(); @@ -1132,7 +1132,7 @@ impl<'a> PackageInstall<'a> { self.destination_dir_subpath_buf[slash] = 0; // SAFETY: NUL written above. let subdir = ZStr::from_buf(self.destination_dir_subpath_buf, slash); - let _ = sys::mkdirat(destination_dir, subdir, 0o755); + let _ = sys::mkdirat(destination_dir, subdir, sys::UMASK_MKDIR_MODE); self.destination_dir_subpath_buf[slash] = SEP; } } diff --git a/src/install/PackageInstall.zig b/src/install/PackageInstall.zig index 21752443b7c..3a678880618 100644 --- a/src/install/PackageInstall.zig +++ b/src/install/PackageInstall.zig @@ -396,7 +396,7 @@ pub const PackageInstall = struct { while (try walker.next().unwrap()) |entry| { switch (entry.kind) { .directory => { - _ = bun.sys.mkdirat(.fromStdDir(destination_dir_), entry.path, 0o755); + _ = bun.sys.mkdirat(.fromStdDir(destination_dir_), entry.path, bun.umask_mkdir_mode); }, .file => { bun.copy(u8, &stackpath, entry.path); @@ -432,7 +432,7 @@ pub const PackageInstall = struct { } }; - var subdir = destination_dir.makeOpenPath(bun.span(this.destination_dir_subpath), .{}) catch |err| return Result.fail(err, .opening_dest_dir, @errorReturnTrace()); + var subdir = bun.MakePath.makeOpenPath(destination_dir, bun.span(this.destination_dir_subpath), .{}) catch |err| return Result.fail(err, .opening_dest_dir, @errorReturnTrace()); defer subdir.close(); this.file_count = FileCopier.copy( @@ -451,7 +451,7 @@ pub const PackageInstall = struct { if (strings.indexOfCharZ(this.destination_dir_subpath, std.fs.path.sep)) |slash| { this.destination_dir_subpath_buf[slash] = 0; const subdir = this.destination_dir_subpath_buf[0..slash :0]; - destination_dir.makeDirZ(subdir) catch {}; + _ = bun.sys.mkdiratZ(.fromStdDir(destination_dir), subdir, bun.umask_mkdir_mode); this.destination_dir_subpath_buf[slash] = std.fs.path.sep; } } @@ -527,7 +527,7 @@ pub const PackageInstall = struct { state.walker.resolve_unknown_entry_types = true; if (!Environment.isWindows) { - state.subdir = destbase.makeOpenPath(bun.span(destpath), .{ + state.subdir = bun.MakePath.makeOpenPath(destbase, bun.span(destpath), .{ .iterate = true, .access_sub_paths = true, }) catch |err| { diff --git a/src/install/PackageInstaller.zig b/src/install/PackageInstaller.zig index 60a9fee746c..111b5ed232f 100644 --- a/src/install/PackageInstaller.zig +++ b/src/install/PackageInstaller.zig @@ -127,7 +127,7 @@ pub const PackageInstaller = struct { pub fn makeAndOpenDir(this: *NodeModulesFolder, root: std.fs.Dir) !std.fs.Dir { const out = brk: { if (comptime Environment.isPosix) { - break :brk try root.makeOpenPath(this.path.items, .{ .iterate = true, .access_sub_paths = true }); + break :brk try bun.MakePath.makeOpenPath(root, this.path.items, .{ .iterate = true, .access_sub_paths = true }); } break :brk (try bun.sys.openDirAtWindowsA(.fromStdDir(root), this.path.items, .{ diff --git a/src/install/PackageManager.zig b/src/install/PackageManager.zig index d78c32759c9..e2397036966 100644 --- a/src/install/PackageManager.zig +++ b/src/install/PackageManager.zig @@ -463,7 +463,7 @@ var ensureTempNodeGypScriptOnce = bun.once(struct { // used later for adding to path for scripts manager.node_gyp_tempdir_name = try manager.allocator.dupe(u8, node_gyp_tempdir_name); - var node_gyp_tempdir = tempdir.handle.makeOpenPath(manager.node_gyp_tempdir_name, .{}) catch |err| { + var node_gyp_tempdir = bun.MakePath.makeOpenPath(tempdir.handle, manager.node_gyp_tempdir_name, .{}) catch |err| { if (err == error.EEXIST) { // it should not exist Output.prettyErrorln("error: node-gyp tempdir already exists", .{}); diff --git a/src/install/PackageManager/PackageManagerDirectories.zig b/src/install/PackageManager/PackageManagerDirectories.zig index d653a97e6e2..2d0ecf81d38 100644 --- a/src/install/PackageManager/PackageManagerDirectories.zig +++ b/src/install/PackageManager/PackageManagerDirectories.zig @@ -69,7 +69,7 @@ var getTemporaryDirectoryOnce = bun.once(struct { std.posix.renameatZ(tempdir.fd, tmpname, cache_directory.fd, tmpname) catch |err| { if (!tried_dot_tmp) { tried_dot_tmp = true; - tempdir = cache_directory.makeOpenPath(".tmp", .{}) catch |err2| { + tempdir = bun.MakePath.makeOpenPath(cache_directory, ".tmp", .{}) catch |err2| { Output.prettyErrorln("error: bun is unable to write files to tempdir: {s}", .{@errorName(err2)}); Global.crash(); }; @@ -124,7 +124,7 @@ noinline fn ensureCacheDirectory(this: *PackageManager) std.fs.Dir { const cache_dir = fetchCacheDirectoryPath(this.env, &this.options); this.cache_directory_path = bun.handleOom(this.allocator.dupeZ(u8, cache_dir.path)); - return std.fs.cwd().makeOpenPath(cache_dir.path, .{}) catch { + return bun.MakePath.makeOpenPath(std.fs.cwd(), cache_dir.path, .{}) catch { this.options.enable.cache = false; this.allocator.free(this.cache_directory_path); continue :loop; @@ -140,7 +140,7 @@ noinline fn ensureCacheDirectory(this: *PackageManager) std.fs.Dir { .auto, )) catch |err| bun.handleOom(err); - return std.fs.cwd().makeOpenPath("node_modules/.cache", .{}) catch |err| { + return bun.MakePath.makeOpenPath(std.fs.cwd(), "node_modules/.cache", .{}) catch |err| { Output.prettyErrorln("error: bun is unable to write files: {s}", .{@errorName(err)}); Global.crash(); }; @@ -375,7 +375,7 @@ pub fn setupGlobalDir(manager: *PackageManager, ctx: Command.Context) !void { pub fn globalLinkDir(this: *PackageManager) std.fs.Dir { return this.global_link_dir orelse brk: { - var global_dir = Options.openGlobalDir(this.options.explicit_global_directory) catch |err| switch (err) { + const global_dir = Options.openGlobalDir(this.options.explicit_global_directory) catch |err| switch (err) { error.@"No global directory found" => { Output.errGeneric("failed to find a global directory for package caching and global link directories", .{}); Global.exit(1); @@ -386,7 +386,7 @@ pub fn globalLinkDir(this: *PackageManager) std.fs.Dir { }, }; this.global_dir = global_dir; - this.global_link_dir = global_dir.makeOpenPath("node_modules", .{}) catch |err| { + this.global_link_dir = bun.MakePath.makeOpenPath(global_dir, "node_modules", .{}) catch |err| { Output.err(err, "failed to open global link dir node_modules at '{f}'", .{FD.fromStdDir(global_dir)}); Global.exit(1); }; diff --git a/src/install/PackageManager/PackageManagerOptions.zig b/src/install/PackageManager/PackageManagerOptions.zig index df21858bf53..572daa92af7 100644 --- a/src/install/PackageManager/PackageManagerOptions.zig +++ b/src/install/PackageManager/PackageManagerOptions.zig @@ -156,25 +156,25 @@ pub const Update = struct { pub fn openGlobalDir(explicit_global_dir: string) !std.fs.Dir { if (bun.env_var.BUN_INSTALL_GLOBAL_DIR.get()) |home_dir| { - return try std.fs.cwd().makeOpenPath(home_dir, .{}); + return try bun.MakePath.makeOpenPath(std.fs.cwd(), home_dir, .{}); } if (explicit_global_dir.len > 0) { - return try std.fs.cwd().makeOpenPath(explicit_global_dir, .{}); + return try bun.MakePath.makeOpenPath(std.fs.cwd(), explicit_global_dir, .{}); } if (bun.env_var.BUN_INSTALL.get()) |home_dir| { var buf: bun.PathBuffer = undefined; var parts = [_]string{ "install", "global" }; const path = Path.joinAbsStringBuf(home_dir, &buf, &parts, .auto); - return try std.fs.cwd().makeOpenPath(path, .{}); + return try bun.MakePath.makeOpenPath(std.fs.cwd(), path, .{}); } if (bun.env_var.XDG_CACHE_HOME.get() orelse bun.env_var.HOME.get()) |home_dir| { var buf: bun.PathBuffer = undefined; var parts = [_]string{ ".bun", "install", "global" }; const path = Path.joinAbsStringBuf(home_dir, &buf, &parts, .auto); - return try std.fs.cwd().makeOpenPath(path, .{}); + return try bun.MakePath.makeOpenPath(std.fs.cwd(), path, .{}); } return error.@"No global directory found"; @@ -182,13 +182,13 @@ pub fn openGlobalDir(explicit_global_dir: string) !std.fs.Dir { pub fn openGlobalBinDir(opts_: ?*const Api.BunInstall) !std.fs.Dir { if (bun.env_var.BUN_INSTALL_BIN.get()) |home_dir| { - return try std.fs.cwd().makeOpenPath(home_dir, .{}); + return try bun.MakePath.makeOpenPath(std.fs.cwd(), home_dir, .{}); } if (opts_) |opts| { if (opts.global_bin_dir) |home_dir| { if (home_dir.len > 0) { - return try std.fs.cwd().makeOpenPath(home_dir, .{}); + return try bun.MakePath.makeOpenPath(std.fs.cwd(), home_dir, .{}); } } } @@ -199,7 +199,7 @@ pub fn openGlobalBinDir(opts_: ?*const Api.BunInstall) !std.fs.Dir { "bin", }; const path = Path.joinAbsStringBuf(home_dir, &buf, &parts, .auto); - return try std.fs.cwd().makeOpenPath(path, .{}); + return try bun.MakePath.makeOpenPath(std.fs.cwd(), path, .{}); } if (bun.env_var.XDG_CACHE_HOME.get() orelse bun.env_var.HOME.get()) |home_dir| { @@ -209,7 +209,7 @@ pub fn openGlobalBinDir(opts_: ?*const Api.BunInstall) !std.fs.Dir { "bin", }; const path = Path.joinAbsStringBuf(home_dir, &buf, &parts, .auto); - return try std.fs.cwd().makeOpenPath(path, .{}); + return try bun.MakePath.makeOpenPath(std.fs.cwd(), path, .{}); } return error.@"Missing global bin directory: try setting $BUN_INSTALL"; diff --git a/src/install/bin.rs b/src/install/bin.rs index 39655707bda..673f7f1c485 100644 --- a/src/install/bin.rs +++ b/src/install/bin.rs @@ -1317,6 +1317,16 @@ impl<'a> Linker<'a> { fn chmod_on_ok(err: Option, abs_target: &ZStr) { // hoisted from `defer` block in create_symlink if err.is_none() { + // `UMASK` is populated by `ensure_umask()`, which the hoisted + // installer, isolated installer, and `bun link` / `bun unlink` + // all call on the main thread before scheduling bin-link work. + // If a future caller forgets to prime, `UMASK` stays at `0` and + // `0o777 & !0` silently widens bin perms to `0o777`. Assert in + // debug builds so a miss is loud. + debug_assert!( + HAS_SET_UMASK.load(Ordering::Acquire), + "Bin::Linker::ensure_umask() not primed before bin chmod — would widen to 0o777", + ); let mode = 0o777 & !(UMASK.load(Ordering::Acquire) as Mode); let _ = sys::lchmod(abs_target, mode); } diff --git a/src/install/bin.zig b/src/install/bin.zig index 3a072fc24c4..fb2cfeeadef 100644 --- a/src/install/bin.zig +++ b/src/install/bin.zig @@ -537,10 +537,23 @@ pub const Bin = extern struct { var has_set_umask = false; + /// Capture the process umask once so it can be consulted later, + /// but do NOT zero it out. Historically we called `umask(0)` here + /// to avoid losing tarball-stored bits (the executable bit on + /// `node_modules/.bin` entries etc.). In practice the code that + /// needed exact modes already calls `fchmod` after the fact, and + /// zeroing umask prevented users' `umask 0o002` from producing + /// group-writable install trees — which is what this function was + /// blocking in issue #29723. Leaving umask alone lets the kernel + /// apply it to directory/file creation like Node, npm, and pnpm. pub fn ensureUmask() void { if (!has_set_umask) { has_set_umask = true; - umask = bun.sys.umask(0); + // `umask(mask)` returns the previous mask. Read-and-restore + // so we capture the caller's umask without changing it. + const previous = bun.sys.umask(0); + _ = bun.sys.umask(previous); + umask = previous; } } @@ -797,7 +810,21 @@ pub const Bin = extern struct { fn createSymlink(this: *Linker, abs_target: [:0]const u8, abs_dest: [:0]const u8, global: bool) void { defer { if (this.err == null) { - _ = bun.sys.chmod(abs_target, umask | 0o777); + // `umask` is populated by `ensureUmask()`, which the + // hoisted installer, isolated installer, and + // `bun link`/`bun unlink` all call on the main thread + // before scheduling bin-link work. Calling it here + // would race — `Bin.Linker.link()` runs on thread-pool + // workers in isolated mode and `ensureUmask()`'s + // read-and-restore of the process umask is not atomic. + // + // Assert the invariant so a future caller that forgets + // to prime umask fails loudly in debug builds instead + // of silently widening bin perms to 0o777. + bun.assertWithLocation(has_set_umask, @src()); + // Mark the bin executable. Honor the process umask the + // same way npm and pnpm do: final mode = 0o777 & ~umask. + _ = bun.sys.chmod(abs_target, 0o777 & ~umask); } } diff --git a/src/install/hoisted_install.rs b/src/install/hoisted_install.rs index 86206cbe894..05ad33465f0 100644 --- a/src/install/hoisted_install.rs +++ b/src/install/hoisted_install.rs @@ -191,8 +191,10 @@ pub(crate) fn install_hoisted_packages( new_node_modules = true; - // Attempt to create a new node_modules folder - if let Err(err) = sys::mkdir(bun_core::zstr!("node_modules"), 0o755) { + // Attempt to create a new node_modules folder. Pass 0o777 so the + // kernel's umask application decides the final permission + // (matches Node/npm/pnpm; issue #29723). + if let Err(err) = sys::mkdir(bun_core::zstr!("node_modules"), sys::UMASK_MKDIR_MODE) { if err.errno != sys::E::EEXIST as _ { Output::err( err, diff --git a/src/install/hoisted_install.zig b/src/install/hoisted_install.zig index 87e27b42d5f..a01b453feaf 100644 --- a/src/install/hoisted_install.zig +++ b/src/install/hoisted_install.zig @@ -60,8 +60,10 @@ pub fn installHoistedPackages( new_node_modules = true; - // Attempt to create a new node_modules folder - if (bun.sys.mkdir("node_modules", 0o755).asErr()) |err| { + // Attempt to create a new node_modules folder. Pass 0o777 so the + // kernel's umask application decides the final permission + // (matches Node/npm/pnpm). + if (bun.sys.mkdir("node_modules", bun.umask_mkdir_mode).asErr()) |err| { if (err.errno != @intFromEnum(bun.sys.E.EXIST)) { Output.err(err, "could not create the \"node_modules\" directory", .{}); Global.crash(); diff --git a/src/install/isolated_install.rs b/src/install/isolated_install.rs index 40d22bd7e0b..82713993090 100644 --- a/src/install/isolated_install.rs +++ b/src/install/isolated_install.rs @@ -40,6 +40,8 @@ use bun_sys::{self as sys, Fd}; use bun_wyhash::{Wyhash, Wyhash11}; use crate::analytics; +#[cfg(unix)] +use crate::bin_real as bin; use crate::bun_bunfig::Arguments as Command; use crate::bun_progress::{Node as ProgressNode, Progress}; use crate::lockfile::tree::is_filtered_dependency_or_workspace; @@ -223,6 +225,15 @@ pub(crate) fn install_isolated_packages( ) -> Result { analytics::features::isolated_bun_install.fetch_add(1, Ordering::Relaxed); + // Prime `Bin::Linker::UMASK` on the main thread before any bin-linking + // tasks are scheduled. `ensure_umask()` uses a compare-exchange gate + // plus umask(0)/umask(prev) probe on the process-global umask — calling + // it later from worker threads would add a window where umask is + // temporarily 0 while concurrent mkdirat calls run (issue #29723). + // Hoisted install primes it the same way from its main-thread entry. + #[cfg(unix)] + bin::Linker::ensure_umask(); + // Take a raw pointer so column borrows below don't tie up `&mut manager` // (which owns the lockfile). let lockfile: *mut Lockfile = &raw mut *manager.lockfile; @@ -1675,12 +1686,12 @@ pub(crate) fn install_isolated_packages( // matches `Installer::NODE_MODULES_BUN`. let bun_modules_path = paths::path_literal!("node_modules/.bun"); - match sys::mkdirat(Fd::cwd(), node_modules_path, 0o755) { + match sys::mkdirat(Fd::cwd(), node_modules_path, sys::UMASK_MKDIR_MODE) { Ok(()) => { // fallthrough to creating bun_modules below } Err(_) => { - match sys::mkdirat(Fd::cwd(), bun_modules_path, 0o755) { + match sys::mkdirat(Fd::cwd(), bun_modules_path, sys::UMASK_MKDIR_MODE) { Err(_) => break 'is_new_bun_modules false, Ok(()) => {} } @@ -1707,7 +1718,9 @@ pub(crate) fn install_isolated_packages( .assume_ok(); // 1 - if sys::mkdirat(Fd::cwd(), rename_path.slice_z(), 0o755).is_err() { + if sys::mkdirat(Fd::cwd(), rename_path.slice_z(), sys::UMASK_MKDIR_MODE) + .is_err() + { break 'is_new_bun_modules true; } @@ -1817,12 +1830,16 @@ pub(crate) fn install_isolated_packages( } // 2 - if let Err(err) = sys::mkdirat(Fd::cwd(), node_modules_path, 0o755) { + if let Err(err) = + sys::mkdirat(Fd::cwd(), node_modules_path, sys::UMASK_MKDIR_MODE) + { Output::err(err, "failed to create './node_modules'", format_args!("")); Global::exit(1); } - if let Err(err) = sys::mkdirat(Fd::cwd(), bun_modules_path, 0o755) { + if let Err(err) = + sys::mkdirat(Fd::cwd(), bun_modules_path, sys::UMASK_MKDIR_MODE) + { Output::err( err, "failed to create './node_modules/.bun'", @@ -1904,7 +1921,7 @@ pub(crate) fn install_isolated_packages( } } - if let Err(err) = sys::mkdirat(Fd::cwd(), bun_modules_path, 0o755) { + if let Err(err) = sys::mkdirat(Fd::cwd(), bun_modules_path, sys::UMASK_MKDIR_MODE) { Output::err( err, "failed to create './node_modules/.bun'", diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index 521e638b010..1cff2d10e4e 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -10,6 +10,16 @@ pub fn installIsolatedPackages( ) OOM!PackageInstall.Summary { bun.analytics.Features.isolated_bun_install += 1; + // Prime Bin.Linker.umask on the main thread before any bin-linking + // tasks are scheduled. The isolated installer runs `Bin.Linker.link()` + // on thread-pool workers; `ensureUmask()` isn't thread-safe (plain + // bool + two non-atomic `umask(2)` syscalls) so calling it from + // concurrent workers races and can leave the process umask at 0. + // Hoisted install primes it the same way on its main-thread entry. + if (comptime Environment.isPosix) { + Bin.Linker.ensureUmask(); + } + const lockfile = manager.lockfile; const store: Store = store: { @@ -1267,8 +1277,8 @@ pub fn installIsolatedPackages( const node_modules_path = bun.OSPathLiteral("node_modules"); const bun_modules_path = bun.OSPathLiteral("node_modules/" ++ Store.modules_dir_name); - sys.mkdirat(FD.cwd(), node_modules_path, 0o755).unwrap() catch { - sys.mkdirat(FD.cwd(), bun_modules_path, 0o755).unwrap() catch { + sys.mkdirat(FD.cwd(), node_modules_path, bun.umask_mkdir_mode).unwrap() catch { + sys.mkdirat(FD.cwd(), bun_modules_path, bun.umask_mkdir_mode).unwrap() catch { break :is_new_bun_modules false; }; @@ -1291,7 +1301,7 @@ pub fn installIsolatedPackages( rename_path.append(mkdir_path.slice()); // 1 - sys.mkdirat(FD.cwd(), mkdir_path.sliceZ(), 0o755).unwrap() catch { + sys.mkdirat(FD.cwd(), mkdir_path.sliceZ(), bun.umask_mkdir_mode).unwrap() catch { break :is_new_bun_modules true; }; } @@ -1356,12 +1366,12 @@ pub fn installIsolatedPackages( }; // 2 - sys.mkdirat(FD.cwd(), node_modules_path, 0o755).unwrap() catch |err| { + sys.mkdirat(FD.cwd(), node_modules_path, bun.umask_mkdir_mode).unwrap() catch |err| { Output.err(err, "failed to create './node_modules'", .{}); Global.exit(1); }; - sys.mkdirat(FD.cwd(), bun_modules_path, 0o755).unwrap() catch |err| { + sys.mkdirat(FD.cwd(), bun_modules_path, bun.umask_mkdir_mode).unwrap() catch |err| { Output.err(err, "failed to create './node_modules/.bun'", .{}); Global.exit(1); }; @@ -1410,7 +1420,7 @@ pub fn installIsolatedPackages( break :is_new_bun_modules true; }; - sys.mkdirat(FD.cwd(), bun_modules_path, 0o755).unwrap() catch |err| { + sys.mkdirat(FD.cwd(), bun_modules_path, bun.umask_mkdir_mode).unwrap() catch |err| { Output.err(err, "failed to create './node_modules/.bun'", .{}); Global.exit(1); }; @@ -1940,6 +1950,7 @@ const sys = bun.sys; const Command = bun.cli.Command; const install = bun.install; +const Bin = install.Bin; const DependencyID = install.DependencyID; const PackageID = install.PackageID; const PackageInstall = install.PackageInstall; diff --git a/src/runtime/cli/bunx_command.rs b/src/runtime/cli/bunx_command.rs index 92c834d0ec8..28ddbd2f071 100644 --- a/src/runtime/cli/bunx_command.rs +++ b/src/runtime/cli/bunx_command.rs @@ -1231,6 +1231,12 @@ impl BunxCommand { Global::exit(1); } + // The bunx cache root lives in the shared temp directory, and + // `is_trusted_cache_root` refuses group/other-writable roots. Request + // 0o755 explicitly instead of the umask-honoring default (0o777) so a + // permissive umask like 0o002 cannot widen the root into a state bunx + // itself rejects; the kernel can only subtract bits from 0o755. + bun_sys::mkdir_recursive_at_mode(Fd::cwd(), bunx_cache_dir, 0o755)?; let bunx_install_dir = Fd::cwd().make_open_path(bunx_cache_dir)?; 'create_package_json: { diff --git a/src/runtime/cli/link_command.zig b/src/runtime/cli/link_command.zig index 97fe5213d0a..64a2f9aa0cf 100644 --- a/src/runtime/cli/link_command.zig +++ b/src/runtime/cli/link_command.zig @@ -66,7 +66,7 @@ fn link(ctx: Command.Context) !void { try manager.setupGlobalDir(ctx); - break :brk manager.global_dir.?.makeOpenPath("node_modules", .{}) catch |err| { + break :brk bun.MakePath.makeOpenPath(manager.global_dir.?, "node_modules", .{}) catch |err| { if (manager.options.log_level != .silent) Output.prettyErrorln("error: failed to create node_modules in global dir due to error {s}", .{@errorName(err)}); Global.crash(); @@ -78,11 +78,14 @@ fn link(ctx: Command.Context) !void { // delete it if it exists node_modules.deleteTree(name) catch {}; - // create scope if specified + // create scope if specified. Use bun.makePath so the mkdir + // honors the process umask (final mode = 0o777 & ~umask) like + // every other install-created directory, instead of + // std.fs.Dir.makeDir's hardcoded 0o755. bun.makePath also + // handles PathAlreadyExists internally, so no catch is needed. if (name[0] == '@') { if (strings.indexOfChar(name, '/')) |i| { - node_modules.makeDir(name[0..i]) catch |err| brk: { - if (err == error.PathAlreadyExists) break :brk; + bun.makePath(node_modules, name[0..i]) catch |err| { if (manager.options.log_level != .silent) Output.prettyErrorln("error: failed to create scope in global dir due to error {s}", .{@errorName(err)}); Global.crash(); diff --git a/src/runtime/cli/unlink_command.zig b/src/runtime/cli/unlink_command.zig index 86d1a935546..b157f86054e 100644 --- a/src/runtime/cli/unlink_command.zig +++ b/src/runtime/cli/unlink_command.zig @@ -79,7 +79,7 @@ fn unlink(ctx: Command.Context) !void { try manager.setupGlobalDir(ctx); - break :brk manager.global_dir.?.makeOpenPath("node_modules", .{}) catch |err| { + break :brk bun.MakePath.makeOpenPath(manager.global_dir.?, "node_modules", .{}) catch |err| { if (manager.options.log_level != .silent) Output.prettyErrorln("error: failed to create node_modules in global dir due to error {s}", .{@errorName(err)}); Global.crash(); diff --git a/src/sys/dir.rs b/src/sys/dir.rs index 1b63424ae09..df673245c1f 100644 --- a/src/sys/dir.rs +++ b/src/sys/dir.rs @@ -329,9 +329,13 @@ pub struct CreateFlags { } impl Dir { - /// Single-level `mkdirat` (mode 0o755) relative to + /// Single-level `mkdirat` relative to /// this dir. Unlike `make_path`, does NOT create intermediate directories /// and surfaces `error.PathAlreadyExists` for callers to branch on. + /// + /// Passes `UMASK_MKDIR_MODE` (0o777) so the kernel applies the process + /// umask — default umask `0o022` still yields `0o755`, `umask 0o002` + /// yields `0o775` (issue #29723). pub fn make_dir(&self, sub_path: &[u8]) -> core::result::Result<(), bun_core::Error> { let mut buf = bun_paths::PathBuffer::default(); let len = sub_path.len().min(buf.0.len() - 1); @@ -339,7 +343,7 @@ impl Dir { buf.0[len] = 0; // SAFETY: NUL-terminated above. let z = ZStr::from_buf(&buf.0[..], len); - match mkdirat(self.fd, z, 0o755) { + match mkdirat(self.fd, z, UMASK_MKDIR_MODE) { Ok(()) => Ok(()), Err(e) if e.get_errno() == E::EEXIST => Err(bun_core::err!("PathAlreadyExists")), Err(e) => Err(e.into()), diff --git a/src/sys/lib.rs b/src/sys/lib.rs index bda5fb865ce..382323f9248 100644 --- a/src/sys/lib.rs +++ b/src/sys/lib.rs @@ -976,6 +976,15 @@ impl AsFd for &Dir { } } +/// Mode passed to `mkdirat(2)` when creating directories. +/// +/// POSIX applies the process umask on top of this, so the final permission +/// is `0o777 & ~umask`. With the default umask of `0o022` this yields +/// `0o755` (same as before), but `umask 0o002` yields `0o775` — letting +/// multi-user setups work like they do with Node, npm, pnpm, and `mkdir(1)` +/// (issue #29723). +pub const UMASK_MKDIR_MODE: Mode = 0o777; + // Raw Linux syscalls via rustix (linux_raw backend). Hot-path I/O on Linux // routes through here instead of glibc — see module doc. Android: same kernel, // same syscall ABI; `linux_syscall.rs` carries its own @@ -2438,10 +2447,16 @@ mod posix_impl { Ok(()) } /// `bun.makePath` — `mkdirat` walking up parents on ENOENT, like `mkdir -p`. + /// + /// Passes `UMASK_MKDIR_MODE` (0o777) so the kernel applies the process + /// umask to the final permissions, matching `mkdir(1)`, Node, npm, and + /// pnpm. Default umask `0o022` still yields `0o755`; `umask 0o002` + /// yields `0o775` — letting multi-user repos work with bun install + /// (issue #29723). #[inline] pub fn mkdir_recursive_at(dir: impl AsFd, sub_path: &[u8]) -> Maybe<()> { let dir = dir.as_fd(); - mkdir_recursive_at_mode(dir, sub_path, 0o755) + mkdir_recursive_at_mode(dir, sub_path, UMASK_MKDIR_MODE) } /// `mkdir_recursive_at` with an explicit `mode` for created directories /// (matches `bun.api.node.fs.NodeFS.mkdirRecursive`'s `mode` arg). diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index 818bea8e285..5a2a2850cbd 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -8877,6 +8877,74 @@ describe.concurrent("bun-install", () => { }); }); + // Regression for https://github.com/oven-sh/bun/issues/29723: + // `bun install` hard-coded 0o755 when creating directories, which meant the + // caller's umask could never loosen the perms beyond 0o755. With + // `umask 0o002` a user would still get 0o755 cache/node_modules dirs — + // blocking shared multi-user repos. + // + // POSIX-only. Windows has no umask, and directory creation goes through a + // different code path there. + // + // Parametrized over the two linkers because they share almost no directory- + // creation code — isolated goes through `isolated_install.rs` / + // `isolated_install/Installer.rs`, hoisted through `hoisted_install.rs` + // and `PackageInstall::install_with_hardlink`. Bin linking in particular + // relied on `Bin::Linker::ensure_umask()` being primed by the hoisted + // entry point; the isolated path never called it, so bin targets would + // chmod to 0o777 regardless of umask. + for (const linker of ["hoisted", "isolated"] as const) { + it.skipIf(isWindows)( + `${linker} install respects process umask for directories and bin targets (#29723)`, + async () => { + const pkgDir = tempDirWithFiles(`bun-install-umask-29723-${linker}`, { + // Local tarball that ships executable bins — exercises both the + // package-dir mkdir path and `Bin.Linker.createSymlink`'s chmod. + "multi-tool-pkg-1.0.0.tgz": await file(join(import.meta.dir, "multi-tool-pkg-1.0.0.tgz")).arrayBuffer(), + "package.json": JSON.stringify({ + name: "foo", + version: "0.0.1", + dependencies: { + "multi-tool-pkg": "file:multi-tool-pkg-1.0.0.tgz", + }, + }), + "bunfig.toml": `[install]\nlinker = "${linker}"\n`, + }); + const cacheDir = join(pkgDir, ".umask-cache"); + + // Drive umask through a shell wrapper — calling process.umask() in + // the test runner would leak into unrelated concurrent tests. + // + // BUN_INSTALL_CACHE_DIR wins over bunfig's `cache = "..."`, and the + // CI runner sets it to a per-test tmpdir. Point it at our own dir + // so the assertion below checks the cache we actually created. + await using proc = Bun.spawn({ + cmd: ["sh", "-c", `umask 0002 && exec "$@"`, "sh", bunExe(), "install"], + cwd: pkgDir, + env: { ...env, BUN_INSTALL_CACHE_DIR: cacheDir }, + stderr: "pipe", + stdout: "ignore", + stdin: "ignore", + }); + const [errText, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(errText).not.toContain("error:"); + expect(exitCode).toBe(0); + + const statMode = (p: string) => stat(p).then(s => s.mode & 0o777); + + // Directories we create: final mode = 0o777 & ~0o002 = 0o775. + expect(await statMode(join(pkgDir, "node_modules"))).toBe(0o775); + expect(await statMode(cacheDir)).toBe(0o775); + + // Bin target — `stat` follows the symlink so this is the mode of + // the executable file that `Bin.Linker.createSymlink` chmod'd. + // Pre-fix isolated installs left this at 0o777. + const binLink = join(pkgDir, "node_modules", ".bin", "multi-tool"); + expect((await stat(binLink)).mode & 0o777).toBe(0o775); + }, + ); + } + it("should handle @scoped name that contains tilde, issue#7045", async () => { await withContext(defaultOpts, async ctx => { await writeFile( diff --git a/test/cli/install/bunx.test.ts b/test/cli/install/bunx.test.ts index cb99066c38c..b9f09099db4 100644 --- a/test/cli/install/bunx.test.ts +++ b/test/cli/install/bunx.test.ts @@ -817,6 +817,65 @@ console.log("EXECUTED: multi-tool-alt (alternate binary)"); expect(out).not.toContain("EXECUTED: multi-tool (main binary)"); expect(exited).toBe(0); }); + + // bunx creates its cache root in the shared temp dir, and + // `is_trusted_cache_root` refuses roots that are group/other-writable. A + // permissive umask must not widen the root bunx creates for itself, or + // the second invocation refuses the cache the first one made (#29723). + it.skipIf(isWindows)("reuses its own cache when created under a permissive umask", async () => { + const urls: string[] = []; + setHandler( + dummyRegistry(urls, { + "1.0.0": { + bin: { + "multi-tool": "bin/multi-tool.js", + }, + as: "1.0.0", + }, + }), + ); + + const run = () => { + const subprocess = spawn({ + cmd: [ + "sh", + "-c", + `umask 0002 && exec "$@"`, + "sh", + bunExe(), + "x", + "--package", + "multi-tool-pkg", + "multi-tool", + ], + cwd: x_dir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env: { + ...env, + npm_config_registry: `http://localhost:${port}/`, + }, + }); + return Promise.all([subprocess.stderr.text(), subprocess.stdout.text(), subprocess.exited] as const); + }; + + // First run creates the cache root under umask 0o002 and installs into it. + { + const [err, out, exited] = await run(); + expect(err).not.toContain("refusing to use bunx cache directory"); + expect(out).toContain("EXECUTED: multi-tool (main binary)"); + expect(exited).toBe(0); + } + + // Second run must accept the cache root the first run just created. + { + const [err, out, exited] = await run(); + expect(err).not.toContain("refusing to use bunx cache directory"); + expect(out).toContain("EXECUTED: multi-tool (main binary)"); + expect(exited).toBe(0); + } + }); }); });