Repository navigation
Conversation
…OSIX
FSWatcher::init and StatWatcher::init built the absolute path to watch
with join_abs_string_buf, whose POSIX normalizer splits on `\` and folds
`..` lexically. So fs.watch("a\\b") and fs.watchFile("a\\b") watched a/b,
and fs.watch("d/link/..") watched d/ instead of the parent of the link
target. Node (libuv) passes the string to inotify/kqueue/stat unchanged.
Both now go through absolute_watch_path_z: on POSIX an absolute path is
used verbatim and a relative one is appended to the cwd without
normalization; Windows keeps the normalizing join, since Win32 resolves
paths lexically itself.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review. WalkthroughWatch path construction now preserves POSIX path bytes and uses Windows-specific joining. Both watcher implementations use the shared helper. Regression tests cover literal backslashes and symlink parent resolution. ChangesWatch path handling
Suggested reviewers: Priority: ➖ Normal — Schedule the POSIX watcher path fix because it corrects user-visible fs.watch and fs.watchFile behavior for literal backslashes and symlink-based paths. Merge Risk: ⚪ Minimal · up to POSIX fs.watch and fs.watchFile now preserve literal path bytes, fixing backslash filenames and symlink/.. resolution while retaining Windows joining behavior. The targeted regression coverage supports merge readiness. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Status Reproduced on bun 1.4.3-canary (Linux x64) against node v26.3.0, from a temp dir: fs.mkdirSync("a"); fs.writeFileSync("a\\b", "LL"); fs.writeFileSync("a/b", "S");
fs.watchFile("a\\b", { interval: 20 }, (c, p) => console.log(p.size, "->", c.size));
setTimeout(() => { fs.appendFileSync("a\\b", "+"); fs.appendFileSync("a/b", "+"); }, 150);
// node: 2 -> 3 (the literal a\b) bun: 1 -> 2 (a/b)
fs.rmSync("a/b"); fs.watch("a\\b");
// node: watches a\b bun: ENOENT: no such file or directory, watch 'a\b'
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — thanks for wiring the "error" event to reject in the new fs.watch tests.
Checked absolute_watch_path_z bounds arithmetic: cap = buf.len() - 1 reserves the NUL slot and both length checks prevent the final buf[len] = 0 from indexing past the buffer. Confirmed bun_paths::is_absolute on POSIX only tests for leading /, so a \-prefixed path correctly falls into the relative branch and gets cwd-prepended. Windows path keeps the checked normalizing join, so behavior there is unchanged. The remaining nit about attaching the path to the ENAMETOOLONG error in StatWatcher::init was optional and doesn't block.
Extended reasoning...
Overview
This PR fixes fs.watch() and fs.watchFile() on POSIX to hand the caller's path bytes to the kernel unnormalized, so filenames containing literal \ and paths containing symlink/.. behave the way Node does. A single helper absolute_watch_path_z in node_fs_watcher.rs replaces two separate join_abs_string_buf[_checked]::<platform::Auto> calls; on POSIX it just prepends cwd + / for relative paths and copies bytes verbatim, on Windows it delegates to the existing checked Windows join. StatWatcher::init now returns ENAMETOOLONG on overflow instead of using the unchecked join. Three POSIX-only tests are added to the existing watch test files.
Security risks
None identified. The helper never widens what the OS is asked to open — it removes lexical rewriting so the kernel sees exactly what the caller passed (which is what Node does and what open()/stat() would see if called directly). Bounds checks on the scratch buffer are correct: cap reserves one byte for the NUL terminator, both cwd.len() + 1 > cap and len + path.len() > cap guard the copies, and buf[len] = 0 is in-bounds because len <= cap = buf.len() - 1. No untrusted network/archive input is involved.
Level of scrutiny
Moderate. Path handling is subtle and cross-platform, but the change is small (~80 lines net), extracts the two callers into one helper per REVIEW.md's "fix the whole class" guidance, and the POSIX branch is deliberately minimal (no normalization at all — just concatenation). I verified bun_paths::is_absolute → is_absolute_native on POSIX checks only for leading /, so backslash-leading inputs are correctly treated as relative. The Windows branch is behavior-preserving modulo the explicit platform::Windows in place of platform::Auto, which resolves to Windows on Windows anyway.
Other factors
This is my third look at this PR. The earlier review raised two optional nits: (1) StatWatcher's ENAMETOOLONG omits the path, and (2) the new tests didn't wire "error" to reject. Commit 15b55a40 addressed (2) — both new fs.watch tests now watcher.once("error", reject) before starting the write interval. The author declined (1), which was explicitly optional and is still a strict improvement over base (unchecked join → potential OOB). Subsequent commits only shortened the helper's doc comment. The tests follow harness conventions (tempDirWithFiles, test.skipIf(isWindows) with a reason comment, Promise.withResolvers resolved from event handlers with a bounded repeat driver, finally cleanup). No outstanding third-party CHANGES_REQUESTED reviews.
|
Updated 8:32 AM PT - Sep 8th, 2026
✅ @robobun, your commit 64de9ac7099f21e8f2f59e3e8260348e232f57af passed in 🧪 To try this PR locally: bunx bun-pr 41986That installs a local version of the PR into your bun-41986 --bun |
This branch has not been deployed
2 participants
Problem
fs.watch("a\\b")andfs.watchFile("a\\b")watcheda/b:fs.watchthrewENOENT: no such file or directory, watch 'a\b'when only the literal file existed, andfs.watchFilereported the other file's stats.fs.watch("d/link/..")watchedd/, not the parent of the link target. Node hands the string to inotify / kqueue /statunchanged.FSWatcher::init(src/runtime/node/node_fs_watcher.rs) andStatWatcher::init(node_fs_stat_watcher.rs) built the absolute path withjoin_abs_string_buf. ItsPlatform::Posixnormalizer splits on\and folds..lexically. Same root cause as Preserve literal backslashes in fs.realpath on POSIX #33410.Fix
absolute_watch_path_z, for both watchers. On POSIX an absolute path is copied verbatim and a relative one is appended to the cwd, unnormalized. Windows keeps the normalizing join: Win32 resolves..lexically itself.open()s this string and then keys, stores, and reports theget_fd_path()realpath, andfs.watchFileonlystat()s it.test/js/node/watch/fs.watch.test.ts(backslash,symlink/..) andfs.watchFile.test.ts(backslash) fail on 1.4.3-canary and pass here. Alltest/js/node/watch/*.test.tsfiles and 45 vendoredtest-fs-watch*node tests pass.Background
\is an ordinary filename byte, and the kernel resolves..in the directory a symlink points to. A lexical normalizer does neither.fs.watchFilealreadypath.resolve()s its argument in JS (node too).fs.watchpasses the caller's string through.Platform::Posixnormalizer breaks npm lockfile migration (packages\pkg1), so this change avoids the normalizer rather than changing it.Notes
fs.watchFile('a\\b')stat'ed<cwd>/a/b(append to the literala\b: no event; append toa/b: event), node the literal name.fs.watchhad been recorded earlier with the same backslash behavior.path_watcher::watch(POSIX) flow:open(path, O_PATH|O_DIRECTORY)(retried withoutO_DIRECTORYfor files),get_fd_path(fd)for the canonical path, dedup map keyed by that realpath plus the recursive flag, inotify wd owners tracked per wd inwd_map: HashMap<i32, Vec<WdOwner>>, so two spellings of one directory were already handled. Only ifget_fd_pathfails does the caller's string become the key, as before.top_level_dirtracksprocess.chdir()(node_process.rs), so a relativefs.watchpath still resolves against the cwd at call time, as before.StatWatcher::initused the unchecked join; an over-long path now returnsENAMETOOLONGthrough the existing "Failed to watch file" error instead of indexing past the buffer.parentPath), split from the same report.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/watch/fs.watchFile.test.ts, test/js/node/watch/fs.watch.test.ts