Repository navigation
Conversation
… build opens A syscall reads a path only up to its first NUL byte, so a NUL in outdir, naming, metafile, compile.outfile, root or tsconfig made Bun.build open a different path than the one the caller named. A release build reported success and wrote to the cut path. A debug build on Linux aborted in ZStr::as_cstr. Config::from_js now throws ERR_INVALID_ARG_VALUE for a NUL in a path option that the build opens as given, and for a root directory or an executable name that it takes from the entry points. PathTemplate::check_output_path fails the build for a NUL in the part of an output name that the build opens. compute_chunks no longer opens a [dir] path that has a NUL, and bun build no longer opens a root directory that has one.
|
Updated 11:08 AM PT - Oct 24th, 2026
✅ @robobun, your commit 655d78ee65a3ba199d5612a2a8e2c0b8315baed4 passed in 🧪 To try this PR locally: bunx bun-pr 46752That installs a local version of the PR into your bun-46752 --bun |
|
Reproduction on 1.4.3-canary.1 and on a release build of main a8fcd31 (Linux x64): import fs from "node:fs";
fs.writeFileSync("a.ts", "console.log('A')");
fs.writeFileSync("b.ts", "console.log('B')");
fs.mkdirSync("out", { recursive: true });
fs.writeFileSync("out/pre", "PRECIOUS");
const build = await Bun.build({
entrypoints: ["./a.ts", "./b.ts"],
outdir: "out",
naming: "pre\0-[name].js",
throw: false,
});
console.log(build.success, build.logs.map(log => log.message));
console.log(fs.readdirSync("out"), fs.readFileSync("out/pre", "utf8").trim());Main prints A debug build of main aborts on the script with With this PR the script prints PR: #46752 |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Input path validation src/runtime/node/types.rs, src/runtime/api/JSBundler.rs, src/runtime/cli/build_command.rs, test/bundler/bun-build-api.test.ts, test/bundler/cli.test.ts |
Build API options and selected root paths receive NUL-byte checks. Tests cover invalid path options, derived paths, and CLI root-directory errors. |
Compile-mode-aware output path validation src/bundler/options.rs, src/bundler/linker_context/computeChunks.rs, src/bundler/linker_context/generateChunksInParallel.rs, src/bundler/bundle_v2.rs, test/bundler/bun-build-api.test.ts |
Output validation checks the path portion that will be opened. Compile mode determines which paths are checked; tests cover failures and successful builds when names are not opened. |
Suggested reviewers: jarred-sumner, dylan-conway, alii
Priority:
Merge Risk: 
Low · up to 655d7
-
The change is mergeable with owner awareness: an opt-in debug code dump can overwrite a dump file when a generated name contains a NUL byte.
Pre-merge checks: 
4
-
Full details: Title check
-
Explanation
The title names the main change: Bun.build now rejects NUL bytes in path options and in output names the build opens. The raw summary and PR objectives confirm the changeset covers this, with checks in JSBundler, options.rs, generateChunksInParallel, computeChunks, and the CLI. The title is specific and concise, and it fits the 'Bun.build:' prefix convention. It is a single sentence of 15 words, so it is slightly long but still readable.
Full details: Description check
-
Explanation
The description is complete and relevant. It explains the problem, implementation, behavior changes, limitations, and verification results. It does not use the template headings exactly, but it provides the required information under equivalent sections.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked the Valid::no_null_bytes wording change ("argument" to "property" for dotted names): the only pre-existing callers are path (src/runtime/node/types.rs) and dir (src/runtime/api/filesystem_router.rs), neither dotted, so no existing fs/FileSystemRouter message or test assertion changes. The three check_output_path / [dir] / bunfig root sites all route the byte scan through bun_core::strings::contains_char with the alias already imported in each file.
Extended reasoning...
The change adds NUL-byte rejection across Bun.build option parsing (src/runtime/api/JSBundler.rs), the output-path check in src/bundler/options.rs and its two callers, the [dir] directory open in computeChunks.rs, and the bunfig-derived root in build_command.rs, plus ~430 lines of tests. It touches path-to-syscall validation, which is the security-relevant surface here, and the inline findings already flag the remaining uncovered writer and a test-convention nit, so a human look is warranted rather than approval.
… they copy the executable The two builds only have to pass the output name check. An asset that does not exist stops each one right after it, so the test writes no executable and needs no per-test timeout.
|
Changes since the first review:
Not changed: the |
There was a problem hiding this comment.
I re-reviewed the latest push and found no bugs; the executable test now stops at the missing compile.assets entry instead of copying the binary, which resolves the timeout nit. A human look is still worthwhile because the PR deliberately picks answers to open API questions (throwing from option parsing even with throw: false, failing the build rather than renaming a plugin-derived output name) that a maintainer should confirm.
What was reviewed:
- The
openedslice chosen per compile mode ingenerateChunksInParallel.rsagainst the actual writers: executable.mapfiles are written bybasename(dest_path)injs_bundle_completion_task.rs, andhas_external_files()is exactly Linked|External, so the checked slice matches what is opened. - The reworked test's ordering claim:
collect_compile_assetsruns indo_compilationafter bundling, socheck_output_pathruns first and the test can still tell "name not checked" from "name rejected". - The remaining variant matrix (executable
.mapbasename, standalone HTML.mapand HTML name, names never opened) is still asserted with exact messages in the sibling tests.
Extended reasoning...
The change adds NUL-byte validation to Bun.build option parsing (outdir, tsconfig, root, metafile, compile paths, bytecodeOrder, and two entrypoint-derived values), a NUL check on the opened portion of output names in check_output_path with per-compile-mode selection of that portion, a guarded directory open in compute_chunks, and a bunfig-derived root check in the CLI, plus about 430 lines of tests. The security-relevant surface is path handling before syscalls, which the change tightens rather than loosens. Since the prior review only one test and three comments changed, the earlier timeout nit is addressed, and no third-party objections appear in the timeline. Not approved because the PR description itself lists design questions for a maintainer (throw vs fail-build, fail vs rename) and the behavior change under throw: false is a user-facing API decision.
|
About the code dump in the merge risk note: |
Problem
Bun.buildpasses one through.naming: "pre\0-[name].js"with two entry points reportssuccess: trueand replacesout/pre.panic: ZStr::as_cstr: interior NUL would truncate the C view(bun_sys::write_file_with_path_buffer).Fix
Config::from_js(src/runtime/api/JSBundler.rs) throwsERR_INVALID_ARG_VALUEfor a NUL inoutdir,tsconfig,root,metafile, fourcompilepaths, and two values it derives fromentrypoints.PathTemplate::check_output_pathfails the build for a NUL in an output name the build opens.compute_chunksandbun buildskip such a directory open.bun-build-api.test.ts,cli.test.ts, 6 fail on main) and 12 more bundler suites.bun build --app, abytecodeOrderindex, two process items.Background
[dir]/[name].[ext]. Without anoutdirit is only returned, so a NUL stays legal.check_output_path(Hardening: fix crashes across the runtime, bundler, dev server, shell and CLI #46021) already fails the build for a name too long to open.bun_syscheck: an errno cannot name the option. Option parse alone misses plugin paths.Downsides
throw: false. A plugin-derived name with a NUL fails the build. esbuild renames it.Config::from_js(31,521 to 31,672), +71 per asset written, 0 allocations, 0 syscalls,.text+2,048 bytes.Notes
No user reported this. A code read found it. Earlier attempts: #37007 first carried
Bun.buildchecks and split them out to a branch that failed the build in the disk writer. This PR replaces that branch. #40531 (publicPath) was closed because that value reaches no syscall.Questions for a maintainer (from the comment on #37007, with the answers this PR picks)
outdir: throw when the options are parsed, or fail the build? This PR throws, asnode:fs,Bun.file,Bun.spawnandFileSystemRouter(Hardening: fix crashes across the runtime, bundler, dev server, shell and CLI #46021) do. esbuild 0.21.5 fails the build (mkdir ...: invalid argument). The Hardening: fix crashes across the runtime, bundler, dev server, shell and CLI #46021 test for a too-longcompile.outfileexpects a failed build.fileskey with a NUL: fail the build, or rename? This PR fails the build. esbuild 0.21.5 writesvirtual_a_b.js. Windows Bun Bundler output filename corrupted #25546 asks the same question for:on Windows. A rename needs a choice of spelling and of whether in-memory names change too.bun_syshelpers that copy a slice into a C string return an errno for a NUL? Not changed here. Some writers build their own C string (.mapfiles, assets, the metafile), so that check alone does not close this.Where each check is
Bun.build()call asTypeError [ERR_INVALID_ARG_VALUE], before anything is opened:outdir,tsconfig,root,metafile,metafile.json,metafile.markdown,compile.outfile,compile.executablePath,compile.windows.icon,compile.bytecodeOrder.entrypoints: the root directory whenrootis not set, and the executable name whencompile.outfileis not set. The message names the value and the option that overrides it.logs, before the first write:namingin its four forms, and a name taken from afileskey or a plugin path. A standalone HTML build checks the HTML name and.mapnames. An executable checks.mapbasenames. Both embed the other names.bun build: a root directory with a NUL is an error. OnlyentryPointsinbunfig.tomlcan carry one, because an argument cannot.Valid::no_null_bytesnow says "property" for a dotted name, as Node'sERR_INVALID_ARG_VALUEdoes. Its error path moved to a cold function. The 10 new callers had stopped the compiler from inlining it into thefspath parser (243 to 258 instructions inpath_like_from_string). It is 239 now.Outcome changes (release build of main a8fcd31 against this branch, 28 test rows)
outdir, the threemetafileforms,compile.outfile. Alsotsconfig,root,compile.windows.iconandcompile.bytecodeOrderwhen the path cut at the NUL exists.entrypointsthrough the root directory (main:ModuleNotFound, orENOENT: failed to open root directorywhen the cut directory is missing),compile.executablePath(main:failed to write compiled executable).[dir]without anoutdiris nowsub\0tail/virtual.js. Main returnedsub/virtual.js, the name of another directory.outdir, a plugin path orfileskey inside a chunk,publicPath,banner, an entry point that does not exist (ModuleNotFound), names that a standalone HTML build inlines or an executable embeds.compile.assetsalready rejects a NUL.metafilepath with a NUL now throws in an executable build with nooutdir. That build never opened it.Still open in
Bun.build(run here on release main and on a debug build of this branch)onResolvepath in thefilenamespacesuccess: trueZStr::as_cstrextendsin an on-disktsconfig.jsonsuccess: truefileskey in a directory whose name has a NUL, with a relative importEISDIR reading filecheck: truewith a plugin module path (bun_sys::exists,src/bundler/bundle_v2.rs)TS6053: File ... not foundbun build --app, a plugin asset pathpic.p\0ngdist/_bun/<hash>.pBUN_FEATURE_FLAG_DUMP_CODE(canary and debug builds), an embedded chunkch\0unk-lazy.jsch#43341 has the resolver checks and needs a rebase. The
compile.windowsstrings other thaniconare not paths and are not checked.Rejected from the self-review
bun build --app: its writer has nooutdir, socheck_output_pathdoes not run for it. It writes client files only, so the check needs the side of each output. It would also turn on the too-long check for that command.compile.bytecodeOrdererror: the two other errors of that parser have none, and the message shows the value.Bun.build.Overlaps
outdir. bundler: resolve./and../keys ofBun.buildfilesagainst the cwd inFileMap::put#46667 resolves relativefileskeys.Measurements (release builds of main a8fcd31 and this branch, Linux x64.
perf,valgrindandstraceare not installed here, so the counts are gdb single-step counts and gdb syscall catchpoints.)Config::from_js,{ entrypoints, outdir }: +151 instructions (31,521 to 31,672). Nooutdir: +79 (30,940 to 31,019). Nine path options set: +619 (51,599 to 52,218). Allocator entries: 58, 56 and 97 on both.check_output_path: 69 or 83 instructions per output name. On main it is an inlined length compare.process_files_to_copywith one asset: 7,809 to 7,880 with anoutdir, no change without one (7,785 to 7,812 against 7,795 to 7,798 over three runs).compute_chunks: one more scan per entry chunk that uses[dir], 28 instructions for a 30-byte path.openatof the project directory comes and goes between runs of the same binary..text+2,048 bytes (82,257,737 to 82,259,785),.rodata+0. 19 symbols differ, net +1,920 bytes.Tests
bun bd test test/bundler/bun-build-api.test.ts -t "NUL byte": 6 pass.cli.test.ts -t "bunfig.toml with a NUL": 1 pass.cli.test.tstest fails. The sixth test pins the names that an executable does not check. It passes on both.bun-build-api95 pass,cli44 pass, and 0 failures inbundler_naming,metafile,bundler_files,bundler_plugin,bundler_splitting,bundler_html,standalone,bun-build-compile-sourcemap,bundler_compile_splitting,html-import-manifest,bundler_edgecase,bundler_compile.bun-build-api93 pass and 3 timeouts (bytecode: function record on an encoder page boundary,bytecode: repeated builds don't retain the generated code,Bun.build can be called thousands of times in one process without crashing).clihas 3 timeouts in tests that compile an executable. Each passes on the release builds.bundler_naming,metafile,bundler_files,html-import-manifest,bundler_html,filesystem_router,mmapandtest-fs-null-bytes.jspass.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/cli.test.ts, test/bundler/bun-build-api.test.ts