diff --git a/src/js/node/v8.ts b/src/js/node/v8.ts index b5fae9334a97..7ce040774086 100644 --- a/src/js/node/v8.ts +++ b/src/js/node/v8.ts @@ -366,7 +366,8 @@ function writeHeapSnapshot(path, _options) { if (!fs) { fs = require("node:fs"); } - fs.writeFileSync(path, Bun.generateHeapSnapshot("v8"), "utf-8"); + // 0600 on create, like Node's WriteSnapshot: https://github.com/nodejs/node/blob/v26.3.0/src/heap_utils.cc#L361-L372 + fs.writeFileSync(path, Bun.generateHeapSnapshot("v8"), { encoding: "utf-8", mode: 0o600 }); return path; } diff --git a/src/jsc/BunCPUProfiler.rs b/src/jsc/BunCPUProfiler.rs index 02d0f6baea99..285f6fb1be51 100644 --- a/src/jsc/BunCPUProfiler.rs +++ b/src/jsc/BunCPUProfiler.rs @@ -80,7 +80,13 @@ pub(crate) fn write_sampling_profiler_report( .join(&[directory, filename]) .map_err(|_| ProfilerError::FilenameTooLong)?; - write_file_creating_dir(&mut path_buf, directory, report.slice()) + // Not a Node output: it keeps the default mode of `write_file_os_path`. + write_file_creating_dir( + &mut path_buf, + directory, + report.slice(), + |path, contents| bun_sys::File::write_file_os_path(Fd::cwd(), path, contents), + ) } pub(crate) fn stop_and_write_profile( @@ -109,6 +115,12 @@ pub(crate) fn stop_and_write_profile( Ok(()) } +/// 0600 on create, like Node's WriteFileSync: https://github.com/nodejs/node/blob/v26.3.0/src/node_file_utils.cc#L29-L36 +pub(crate) fn write_owner_only(path: &bun_paths::OSPathSliceZ, data: &[u8]) -> bun_sys::Maybe<()> { + const FLAGS: i32 = bun_sys::O::WRONLY | bun_sys::O::CREAT | bun_sys::O::TRUNC; + bun_sys::File::openat_os_path(Fd::cwd(), path, FLAGS, 0o600)?.write_all(data) +} + fn write_profile_to_file( profile_string: &BunString, config: &CPUProfilerConfig, @@ -122,14 +134,20 @@ fn write_profile_to_file( build_output_path(&mut path_buf, config, is_md_format)?; - write_file_creating_dir(&mut path_buf, config.dir, profile_slice.slice()) + write_file_creating_dir( + &mut path_buf, + config.dir, + profile_slice.slice(), + write_owner_only, + ) } -/// Writes `contents` to `path`. On ENOENT, EPERM or EACCES it creates `dir` and retries once. +/// Writes `contents` to `path` with `write`. On ENOENT, EPERM or EACCES it creates `dir` and retries once. fn write_file_creating_dir( path: &mut AutoAbsPathChecked, dir: &[u8], contents: &[u8], + write: fn(&bun_paths::OSPathSliceZ, &[u8]) -> bun_sys::Maybe<()>, ) -> Result<(), ProfilerError> { // Convert to OS-specific path (UTF-16 on Windows, UTF-8 elsewhere) #[cfg(windows)] @@ -140,8 +158,7 @@ fn write_file_creating_dir( #[cfg(not(windows))] let output_path_os = path.slice_z(); - // Write the profile to disk using bun.sys.File.writeFile - let result = bun_sys::File::write_file_os_path(Fd::cwd(), output_path_os, contents); + let result = write(output_path_os, contents); if let Err(err) = result { // If we got ENOENT, PERM, or ACCES, try creating the directory and retry let errno = err.get_errno(); @@ -149,8 +166,7 @@ fn write_file_creating_dir( if !dir.is_empty() { let _ = Fd::cwd().make_path(dir); // Retry write - let retry_result = - bun_sys::File::write_file_os_path(Fd::cwd(), output_path_os, contents); + let retry_result = write(output_path_os, contents); if retry_result.is_err() { return Err(ProfilerError::WriteFailed); } diff --git a/src/jsc/BunHeapProfiler.rs b/src/jsc/BunHeapProfiler.rs index 1bb3babf3493..3e1328ce9e1f 100644 --- a/src/jsc/BunHeapProfiler.rs +++ b/src/jsc/BunHeapProfiler.rs @@ -2,7 +2,7 @@ use crate::CrateError as Error; use bun_core::Output; use bun_core::String as BunString; use bun_paths::{AutoAbsPathChecked, PathBuffer, resolve_path}; -use bun_sys::{self as sys, E, Fd, FdDirExt}; +use bun_sys::{E, Fd, FdDirExt}; use crate::VM; @@ -54,13 +54,13 @@ pub(crate) fn generate_and_write_profile( path_buf.slice_z().as_bytes(), ); - // Write the profile to disk using bun.sys.File.writeFile // `slice_z()` borrows `path_buf` mutably, so we re-derive it at each call // site instead of holding a single binding. #[cfg(windows)] - let result = sys::File::write_file_os_path(Fd::cwd(), output_path_os, profile_slice.slice()); + let result = crate::bun_cpu_profiler::write_owner_only(output_path_os, profile_slice.slice()); #[cfg(not(windows))] - let result = sys::File::write_file(Fd::cwd(), path_buf.slice_z(), profile_slice.slice()); + let result = + crate::bun_cpu_profiler::write_owner_only(path_buf.slice_z(), profile_slice.slice()); if let Err(err) = result { // If we got ENOENT, PERM, or ACCES, try creating the directory and retry let errno = err.get_errno(); @@ -71,11 +71,15 @@ pub(crate) fn generate_and_write_profile( let _ = Fd::cwd().make_path(dir_path); // Retry write #[cfg(windows)] - let retry_result = - sys::File::write_file_os_path(Fd::cwd(), output_path_os, profile_slice.slice()); + let retry_result = crate::bun_cpu_profiler::write_owner_only( + output_path_os, + profile_slice.slice(), + ); #[cfg(not(windows))] - let retry_result = - sys::File::write_file(Fd::cwd(), path_buf.slice_z(), profile_slice.slice()); + let retry_result = crate::bun_cpu_profiler::write_owner_only( + path_buf.slice_z(), + profile_slice.slice(), + ); if retry_result.is_err() { return Err(crate::CrateError::WriteFailed); } diff --git a/test/cli/heap-prof.test.ts b/test/cli/heap-prof.test.ts index f94ce71e788e..24a4a5dfd384 100644 --- a/test/cli/heap-prof.test.ts +++ b/test/cli/heap-prof.test.ts @@ -1,4 +1,5 @@ import { expect, test } from "bun:test"; +import { statSync } from "fs"; import { bunEnv, bunExe, isWindows, tempDir } from "harness"; import { join } from "path"; @@ -228,6 +229,49 @@ test("--heap-prof-name specifies output filename", async () => { expectV8HeapSnapshotShape(profile); }); +// Node creates a --heap-prof file with mode 0600 (WriteFileSync in src/node_file_utils.cc). +// Node has no --heap-prof-md. That file gets the same mode. +test.skipIf(isWindows).concurrent.each([ + { + name: "--heap-prof", + flags: ["--heap-prof", "--heap-prof-name", "out.heapprofile"], + stderr: /^$/, + modes: { "out.heapprofile": "600" }, + }, + { + name: "--heap-prof-md", + flags: ["--heap-prof-md", "--heap-prof-name", "out.md"], + stderr: /^Heap profile written to: .*out\.md\n$/, + modes: { "out.md": "600" }, + }, + { + // The first open fails because the directory is missing. The open after the mkdir creates the file. + name: "--heap-prof with a missing --heap-prof-dir", + flags: ["--heap-prof", "--heap-prof-dir", "missing/dir", "--heap-prof-name", "out.heapprofile"], + stderr: /^$/, + modes: { "missing": "755", "missing/dir": "755", "missing/dir/out.heapprofile": "600" }, + }, +])("$name creates the profile with mode 0600", async ({ flags, stderr: expectedStderr, modes }) => { + using dir = tempDir("heap-prof-mode-test", {}); + + await using proc = Bun.spawn({ + // The profile is written at exit, so the umask that the script sets applies to it. + cmd: [bunExe(), ...flags, "-e", "process.umask(0o022);"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toMatch(expectedStderr); + expect({ stdout, exitCode }).toEqual({ stdout: "", exitCode: 0 }); + + const entries = [...new Bun.Glob("**").scanSync({ cwd: String(dir), onlyFiles: false })]; + const permissions = entries.map(entry => [entry, (statSync(join(String(dir), entry)).mode & 0o777).toString(8)]); + expect(Object.fromEntries(permissions)).toEqual(modes); +}); + test("--heap-prof-name and --heap-prof-dir work together", async () => { using dir = tempDir("heap-prof-both-test", { "output": {}, diff --git a/test/cli/run/cpu-prof.test.ts b/test/cli/run/cpu-prof.test.ts index 75df02a8a199..2097f19a40b5 100644 --- a/test/cli/run/cpu-prof.test.ts +++ b/test/cli/run/cpu-prof.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { readdirSync, readFileSync } from "fs"; +import { chmodSync, readdirSync, readFileSync, statSync } from "fs"; import { bunEnv, bunExe, isWindows, tempDir } from "harness"; import { join } from "path"; @@ -189,6 +189,52 @@ describe.concurrent("--cpu-prof", () => { expect(exitCode).toBe(0); }); + // Node creates a --cpu-prof file with mode 0600 (WriteFileSync in src/node_file_utils.cc). + // Node has no --cpu-prof-md. That file gets the same mode. + test.skipIf(isWindows)("--cpu-prof and --cpu-prof-md create the profile with mode 0600", async () => { + using dir = tempDir("cpu-prof-mode", { + // Longer than a profile, and not JSON: the run must replace all of it. + "existing.cpuprofile": Buffer.alloc(1024 * 1024, "x").toString(), + }); + chmodSync(join(String(dir), "existing.cpuprofile"), 0o644); + + const results = await Promise.all( + [ + // With both formats, each file gets its extension: new.cpuprofile and new.md. + ["--cpu-prof", "--cpu-prof-md", "--cpu-prof-name", "new"], + // The first open fails because the directory is missing. The open after the mkdir creates the file. + ["--cpu-prof", "--cpu-prof-dir", "missing/dir", "--cpu-prof-name", "retry.cpuprofile"], + ["--cpu-prof", "--cpu-prof-name", "existing.cpuprofile"], + ].map(async flags => { + await using proc = Bun.spawn({ + // The profile is written at exit, so the umask that the script sets applies to it. + cmd: [bunExe(), ...flags, "-e", "process.umask(0o022);"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + }), + ); + const silentSuccess = { stdout: "", stderr: "", exitCode: 0 }; + expect(results).toEqual([silentSuccess, silentSuccess, silentSuccess]); + + const entries = [...new Bun.Glob("**").scanSync({ cwd: String(dir), onlyFiles: false })]; + const permissions = entries.map(entry => [entry, (statSync(join(String(dir), entry)).mode & 0o777).toString(8)]); + expect(Object.fromEntries(permissions)).toEqual({ + "new.cpuprofile": "600", + "new.md": "600", + "missing": "755", + "missing/dir": "755", + "missing/dir/retry.cpuprofile": "600", + // The mode only applies when the file is created, as in node. + "existing.cpuprofile": "644", + }); + expect(JSON.parse(readFileSync(join(String(dir), "existing.cpuprofile"), "utf-8"))).toHaveProperty("nodes"); + }); + test("profile captures function names", async () => { using dir = tempDir("cpu-prof-functions", { "test.js": ` diff --git a/test/js/node/v8/v8-module.test.ts b/test/js/node/v8/v8-module.test.ts index fe9fa3a8788f..3392be316ca5 100644 --- a/test/js/node/v8/v8-module.test.ts +++ b/test/js/node/v8/v8-module.test.ts @@ -1,5 +1,7 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; +import { chmodSync, readFileSync, statSync } from "node:fs"; +import { join } from "node:path"; import { GCProfiler, isStringOneByteRepresentation } from "node:v8"; describe("v8.isStringOneByteRepresentation", () => { @@ -131,3 +133,46 @@ describe("v8.GCProfiler", () => { }); }); }); + +// Node creates the snapshot file with mode 0600 (WriteSnapshot in src/heap_utils.cc), and only when +// the call creates the file. Windows has no permission bits to check. +describe.concurrent.skipIf(isWindows)("v8.writeHeapSnapshot", () => { + // Writes one snapshot into `cwd`, to `path` or to the default name, and returns where it is. + async function writeHeapSnapshot(cwd: string, path?: string) { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + // The umask is pinned so that the mode of the file depends on the call alone. + `process.umask(0o022); console.log(require("node:v8").writeHeapSnapshot(process.argv[1]));`, + ...(path === undefined ? [] : [path]), + ], + cwd, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stderr, exitCode }).toEqual({ stderr: "", exitCode: 0 }); + return join(cwd, stdout.trim()); + } + + const permissions = (path: string) => (statSync(path).mode & 0o777).toString(8); + + test.each([ + ["a path", "new.heapsnapshot"], + ["the default name", undefined], + ])("creates the file with mode 0600 for %s", async (_label, path) => { + using dir = tempDir("v8-write-heap-snapshot", {}); + expect(permissions(await writeHeapSnapshot(String(dir), path))).toBe("600"); + }); + + test("keeps the mode of an existing file", async () => { + using dir = tempDir("v8-write-heap-snapshot", { "existing.heapsnapshot": "not a snapshot" }); + const existing = join(String(dir), "existing.heapsnapshot"); + chmodSync(existing, 0o644); + expect(await writeHeapSnapshot(String(dir), "existing.heapsnapshot")).toBe(existing); + expect(JSON.parse(readFileSync(existing, "utf8"))).toHaveProperty("snapshot"); + expect(permissions(existing)).toBe("644"); + }); +});