diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts index 0dbf3f5fa7..34dda03dc7 100644 --- a/packages/bun-types/bun.d.ts +++ b/packages/bun-types/bun.d.ts @@ -842,6 +842,20 @@ declare module "bun" { destination: BunFile, input: BunFile, options?: { + /** + * Set the file permissions of the destination when it is created or overwritten. + * + * Must be a valid Unix permission mode (0 to 0o777 / 511 in decimal). + * If omitted, defaults to the system default based on umask (typically 0o644). + * + * @throws {RangeError} If the mode is outside the valid range (0 to 0o777). + * + * @example + * ```ts + * await Bun.write(Bun.file("./secret.txt"), Bun.file("./source.txt"), { mode: 0o600 }); + * ``` + */ + mode?: number; /** * If `true`, create the parent directory if it doesn't exist. By default, this is `true`. * @@ -875,6 +889,20 @@ declare module "bun" { destinationPath: PathLike, input: BunFile, options?: { + /** + * Set the file permissions of the destination when it is created or overwritten. + * + * Must be a valid Unix permission mode (0 to 0o777 / 511 in decimal). + * If omitted, defaults to the system default based on umask (typically 0o644). + * + * @throws {RangeError} If the mode is outside the valid range (0 to 0o777). + * + * @example + * ```ts + * await Bun.write("./secret.txt", Bun.file("./source.txt"), { mode: 0o600 }); + * ``` + */ + mode?: number; /** * If `true`, create the parent directory if it doesn't exist. By default, this is `true`. * diff --git a/src/bun.js/webcore/Blob.zig b/src/bun.js/webcore/Blob.zig index 4b340fb34d..52296ccc5a 100644 --- a/src/bun.js/webcore/Blob.zig +++ b/src/bun.js/webcore/Blob.zig @@ -902,8 +902,7 @@ fn writeFileWithEmptySourceToDestination(ctx: *jsc.JSGlobalObject, destination_b // SAFETY: we check if `file.pathlike` is an fd or // not above, returning if it is. var buf: bun.PathBuffer = undefined; - // TODO: respect `options.mode` - const mode: bun.Mode = jsc.Node.fs.default_permission; + const mode: bun.Mode = options.mode orelse jsc.Node.fs.default_permission; while (true) { const open_res = bun.sys.open(file.pathlike.path.sliceZ(&buf), bun.O.CREAT | bun.O.TRUNC, mode); switch (open_res) { @@ -1055,6 +1054,7 @@ pub fn writeFileWithSourceDestination(ctx: *jsc.JSGlobalObject, source_blob: *Bl ctx.bunVM().eventLoop(), options.mkdirp_if_not_exists orelse true, destination_blob.size, + options.mode, ); } var file_copier = copy_file.CopyFile.create( @@ -1065,6 +1065,7 @@ pub fn writeFileWithSourceDestination(ctx: *jsc.JSGlobalObject, source_blob: *Bl destination_blob.size, ctx, options.mkdirp_if_not_exists orelse true, + options.mode, ); file_copier.schedule(); return file_copier.promise.value(); @@ -1208,6 +1209,7 @@ pub fn writeFileWithSourceDestination(ctx: *jsc.JSGlobalObject, source_blob: *Bl const WriteFileOptions = struct { mkdirp_if_not_exists: ?bool = null, extra_options: ?JSValue = null, + mode: ?bun.Mode = null, }; /// ## Errors @@ -1536,6 +1538,7 @@ pub fn writeFile(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun return globalThis.throwInvalidArguments("Bun.write(pathOrFdOrBlob, blob) expects a Blob-y thing to write", .{}); }; var mkdirp_if_not_exists: ?bool = null; + var mode: ?bun.Mode = null; const options = args.nextEat(); if (options) |options_object| { if (options_object.isObject()) { @@ -1545,6 +1548,18 @@ pub fn writeFile(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun } mkdirp_if_not_exists = create_directory.toBoolean(); } + if (try options_object.get(globalThis, "mode")) |mode_value| { + if (!mode_value.isEmptyOrUndefinedOrNull()) { + if (!mode_value.isNumber()) { + return globalThis.throwInvalidArgumentType("write", "options.mode", "number"); + } + const mode_int = mode_value.toInt64(); + if (mode_int < 0 or mode_int > 0o777) { + return globalThis.throwRangeError(mode_int, .{ .field_name = "mode", .min = 0, .max = 0o777 }); + } + mode = @intCast(mode_int); + } + } } else if (!options_object.isEmptyOrUndefinedOrNull()) { return globalThis.throwInvalidArgumentType("write", "options", "object"); } @@ -1552,6 +1567,7 @@ pub fn writeFile(globalThis: *jsc.JSGlobalObject, callframe: *jsc.CallFrame) bun return writeFileInternal(globalThis, &path_or_blob, data, .{ .mkdirp_if_not_exists = mkdirp_if_not_exists, .extra_options = options, + .mode = mode, }); } diff --git a/src/bun.js/webcore/blob/copy_file.zig b/src/bun.js/webcore/blob/copy_file.zig index 75f0f8dc08..2547fc2799 100644 --- a/src/bun.js/webcore/blob/copy_file.zig +++ b/src/bun.js/webcore/blob/copy_file.zig @@ -18,6 +18,7 @@ pub const CopyFile = struct { globalThis: *JSGlobalObject, mkdirp_if_not_exists: bool = false, + destination_mode: ?bun.Mode = null, pub const ResultType = anyerror!SizeType; @@ -31,6 +32,7 @@ pub const CopyFile = struct { max_len: SizeType, globalThis: *JSGlobalObject, mkdirp_if_not_exists: bool, + destination_mode: ?bun.Mode, ) *CopyFilePromiseTask { const read_file = bun.new(CopyFile, CopyFile{ .store = store, @@ -41,6 +43,7 @@ pub const CopyFile = struct { .destination_file_store = store.data.file, .source_file_store = source_store.data.file, .mkdirp_if_not_exists = mkdirp_if_not_exists, + .destination_mode = destination_mode, }); store.ref(); source_store.ref(); @@ -98,6 +101,24 @@ pub const CopyFile = struct { const close_input = this.destination_file_store.pathlike != .fd and this.destination_fd != bun.invalid_fd; const close_output = this.source_file_store.pathlike != .fd and this.source_fd != bun.invalid_fd; + // Apply destination mode using fchmod before closing (for POSIX platforms) + // This ensures mode is applied even when overwriting existing files, since + // open()'s mode argument only affects newly created files. + // On macOS clonefile path, chmod is called separately after clonefile. + // On Windows, this is handled via async uv_fs_chmod. + if (comptime !Environment.isWindows) { + if (this.destination_mode) |mode| { + if (this.destination_fd != bun.invalid_fd and this.system_error == null) { + switch (bun.sys.fchmod(this.destination_fd, mode)) { + .err => |err| { + this.system_error = err.toSystemError(); + }, + .result => {}, + } + } + } + } + if (close_input and close_output) { this.doCloseFile(.both); } else if (close_input) { @@ -155,10 +176,11 @@ pub const CopyFile = struct { if (which == .both or which == .destination) { while (true) { const dest = this.destination_file_store.pathlike.path.sliceZ(&path_buf1); + const mode = this.destination_mode orelse jsc.Node.fs.default_permission; this.destination_fd = switch (bun.sys.open( dest, open_destination_flags, - jsc.Node.fs.default_permission, + mode, )) { .result => |result| switch (result.makeLibUVOwnedForSyscall(.open, .close_on_fail)) { .result => |result_fd| result_fd, @@ -446,6 +468,16 @@ pub const CopyFile = struct { } else { this.read_len = @as(SizeType, @intCast(stat_.?.size)); } + // Apply destination mode if specified (clonefile copies source permissions) + if (this.destination_mode) |mode| { + switch (bun.sys.chmod(this.destination_file_store.pathlike.path.sliceZ(&path_buf), mode)) { + .err => |err| { + this.system_error = err.toSystemError(); + return; + }, + .result => {}, + } + } return; } else |_| { @@ -578,10 +610,14 @@ pub const CopyFileWindows = struct { io_request: libuv.fs_t = std.mem.zeroes(libuv.fs_t), promise: jsc.JSPromise.Strong = .{}, mkdirp_if_not_exists: bool = false, + destination_mode: ?bun.Mode = null, event_loop: *jsc.EventLoop, size: Blob.SizeType = Blob.max_size, + /// Bytes written, stored for use after async chmod completes + written_bytes: usize = 0, + /// For mkdirp err: ?bun.sys.Error = null, @@ -791,6 +827,7 @@ pub const CopyFileWindows = struct { event_loop: *jsc.EventLoop, mkdirp_if_not_exists: bool, size_: Blob.SizeType, + destination_mode: ?bun.Mode, ) jsc.JSValue { destination_file_store.ref(); source_file_store.ref(); @@ -801,6 +838,7 @@ pub const CopyFileWindows = struct { .io_request = std.mem.zeroes(libuv.fs_t), .event_loop = event_loop, .mkdirp_if_not_exists = mkdirp_if_not_exists, + .destination_mode = destination_mode, .size = size_, }); const promise = result.promise.value(); @@ -1055,6 +1093,66 @@ pub const CopyFileWindows = struct { this.truncate(); written = @intCast(this.size); } + + // Apply destination mode if specified (async) + if (this.destination_mode) |mode| { + if (this.destination_file_store.data.file.pathlike == .path) { + this.written_bytes = written; + var pathbuf: bun.PathBuffer = undefined; + const path = this.destination_file_store.data.file.pathlike.path.sliceZ(&pathbuf); + const loop = this.event_loop.virtual_machine.event_loop_handle.?; + this.io_request.deinit(); + this.io_request = std.mem.zeroes(libuv.fs_t); + this.io_request.data = @ptrCast(this); + + const rc = libuv.uv_fs_chmod( + loop, + &this.io_request, + path, + @intCast(mode), + &onChmod, + ); + + if (rc.errno()) |errno| { + // chmod failed to start - reject the promise to report the error + var err = bun.sys.Error.fromCode(@enumFromInt(errno), .chmod); + const destination = &this.destination_file_store.data.file; + if (destination.pathlike == .path) { + err = err.withPath(destination.pathlike.path.slice()); + } + this.throw(err); + return; + } + this.event_loop.refConcurrently(); + return; + } + } + + this.resolvePromise(written); + } + + fn onChmod(req: *libuv.fs_t) callconv(.c) void { + var this: *CopyFileWindows = @fieldParentPtr("io_request", req); + bun.assert(req.data == @as(?*anyopaque, @ptrCast(this))); + + var event_loop = this.event_loop; + event_loop.unrefConcurrently(); + + const rc = req.result; + if (rc.errEnum()) |errno| { + var err = bun.sys.Error.fromCode(errno, .chmod); + const destination = &this.destination_file_store.data.file; + if (destination.pathlike == .path) { + err = err.withPath(destination.pathlike.path.slice()); + } + this.throw(err); + return; + } + + this.resolvePromise(this.written_bytes); + } + + fn resolvePromise(this: *CopyFileWindows, written: usize) void { const globalThis = this.event_loop.global; const promise = this.promise.swap(); var event_loop = this.event_loop; diff --git a/test/regression/issue/25903.test.ts b/test/regression/issue/25903.test.ts new file mode 100644 index 0000000000..02a7cebcea --- /dev/null +++ b/test/regression/issue/25903.test.ts @@ -0,0 +1,147 @@ +import { describe, expect, test } from "bun:test"; +import { isWindows, tempDir } from "harness"; +import { stat } from "node:fs/promises"; + +// Tests for issue #25903: Bun.write() mode option when copying files using Bun.file() +// The mode option is respected when copying files via Bun.file() as the source. +// These tests are skipped on Windows where Unix-style file permissions don't apply. + +describe.skipIf(isWindows)("Bun.write() mode option", () => { + test("Bun.write() respects mode option when copying files via Bun.file()", async () => { + using dir = tempDir("issue-25903", {}); + const sourcePath = `${dir}/source.txt`; + const destPath = `${dir}/dest.txt`; + + // Create source file (mode is determined by the write path's default behavior) + await Bun.write(sourcePath, "hello world"); + + // Get source file's actual permissions to verify they differ from what we'll set + const sourceStat = await stat(sourcePath); + const sourceMode = sourceStat.mode & 0o777; + + // Copy using Bun.file() with specific 0o600 permissions (more restrictive) + // The mode option is honored for Bun.file() copy operations + await Bun.write(destPath, Bun.file(sourcePath), { mode: 0o600 }); + + // Verify destination file has the specified permissions, not inherited from source + const destStat = await stat(destPath); + expect(destStat.mode & 0o777).toBe(0o600); + // Also verify it's different from source (unless source happened to be 0o600) + if (sourceMode !== 0o600) { + expect(destStat.mode & 0o777).not.toBe(sourceMode); + } + }); + + test("Bun.write() respects mode option with createPath when copying via Bun.file()", async () => { + using dir = tempDir("issue-25903-createPath", {}); + const sourcePath = `${dir}/source.txt`; + const destPath = `${dir}/subdir/dest.txt`; + + // Create source file + await Bun.write(sourcePath, "hello world"); + + // Copy using Bun.file() to a path that requires directory creation, with specific permissions + await Bun.write(destPath, Bun.file(sourcePath), { mode: 0o755, createPath: true }); + + // Verify destination file has the specified permissions + const destStat = await stat(destPath); + expect(destStat.mode & 0o777).toBe(0o755); + }); + + test("Bun.write() uses default permissions when mode is not specified for Bun.file() copy", async () => { + using dir = tempDir("issue-25903-default", {}); + const sourcePath = `${dir}/source.txt`; + const destPath = `${dir}/dest.txt`; + const baselinePath = `${dir}/baseline.txt`; + + // Create source file + await Bun.write(sourcePath, "hello world"); + + // Create a baseline file using default permissions (to determine what the default is) + await Bun.write(baselinePath, "baseline"); + const baselineStat = await stat(baselinePath); + const defaultMode = baselineStat.mode & 0o777; + + // Copy using Bun.file() without specifying mode - should use default permissions + await Bun.write(destPath, Bun.file(sourcePath)); + + // When mode is not specified, the default permission is used (same as creating a new file) + // This test verifies that the destination doesn't inherit source permissions incorrectly + const destStat = await stat(destPath); + expect(destStat.mode & 0o777).toBe(defaultMode); + }); + + test("Bun.write() respects mode when writing to PathLike from BunFile", async () => { + using dir = tempDir("issue-25903-pathlike", {}); + const sourcePath = `${dir}/source.txt`; + const destPath = `${dir}/dest.txt`; + + // Create source file + await Bun.write(sourcePath, "test content"); + + // Write with specific mode using path string as destination and Bun.file() as source + await Bun.write(destPath, Bun.file(sourcePath), { mode: 0o700 }); + + const destStat = await stat(destPath); + expect(destStat.mode & 0o777).toBe(0o700); + }); + + test("Bun.write() respects mode when both destination and source are BunFile", async () => { + using dir = tempDir("issue-25903-bunfile-dest", {}); + const sourcePath = `${dir}/source.txt`; + const destPath = `${dir}/dest.txt`; + + // Create source file + await Bun.write(sourcePath, "test content for bunfile dest"); + + // Write with specific mode using Bun.file() as both destination and source + await Bun.write(Bun.file(destPath), Bun.file(sourcePath), { mode: 0o700 }); + + const destStat = await stat(destPath); + expect(destStat.mode & 0o777).toBe(0o700); + }); + + test("Bun.write() respects mode when overwriting an existing file", async () => { + using dir = tempDir("issue-25903-overwrite", {}); + const sourcePath = `${dir}/source.txt`; + const destPath = `${dir}/dest.txt`; + + // Create source file + await Bun.write(sourcePath, "source content"); + + // Create destination file with default permissions + await Bun.write(destPath, "original content"); + const originalStat = await stat(destPath); + const originalMode = originalStat.mode & 0o777; + + // Overwrite destination with different mode - should update permissions even for existing file + await Bun.write(destPath, Bun.file(sourcePath), { mode: 0o600 }); + + const destStat = await stat(destPath); + expect(destStat.mode & 0o777).toBe(0o600); + // Verify the mode actually changed (unless original happened to be 0o600) + if (originalMode !== 0o600) { + expect(destStat.mode & 0o777).not.toBe(originalMode); + } + }); + + test("Bun.write() accepts mode: 0 (no permissions)", async () => { + using dir = tempDir("issue-25903-mode-zero", {}); + const sourcePath = `${dir}/source.txt`; + const destPath = `${dir}/dest.txt`; + + // Create source file + await Bun.write(sourcePath, "test content"); + + // Write with mode 0 (no permissions) - this should be accepted, not treated as "not specified" + await Bun.write(destPath, Bun.file(sourcePath), { mode: 0o000 }); + + const destStat = await stat(destPath); + expect(destStat.mode & 0o777).toBe(0o000); + }); +}); // end describe.skipIf(isWindows) + +// Note: The mode option is fully respected for Bun.file() copy operations (the fix for #25903). +// For direct string/buffer writes, mode support depends on the write path used internally. +// The empty file creation path respects mode, but other direct write paths may use +// the default permission (0o664). The tests above validate the Bun.file() copy path.