Repository navigation
guardedWrite: evaluate descriptor-relative publication (openat/renameat) for containment races #2148
Description
Context
CodeRabbit's Security Boundaries pre-merge check on #1914 asks for no-follow directory handles and descriptor-relative publication in the guarded-write path: open and pin the workspace and target-parent directories with symlink rejection, validate identities through those handles, and commit relative to the pinned parent handle, so a path component swapped between the containment check and the publish cannot redirect the write.
This issue tracks that hardening as a follow-up because it cannot be implemented with the Node.js core APIs the extension currently depends on.
What the series already does (in #1914)
guardedWriteauthorizes a target while walking the path from the workspace root: every directory it walks is pinned with its(dev, ino)identity (src/core/tools/guardedWrite.ts, around line 409), and the pinned identities are re-checked immediately before publish (around line 544).- The authorized resolved path is passed down as
expectedResolvedPath, sosafeWriteTextfails fatally if the path it resolves differs from the one the caller authorized. safeWriteTextstages into a sibling temp file and commits withrename, and (where supplied) verifies the expected ancestor identities.
Together these close the practical swap windows; what remains is publication that is relative to an open directory handle, which removes the remaining lexical re-resolution between the identity check and the rename.
Why this is not a small in-PR change
Node core exposes no openat(2) / renameat(2) / fstatat(2)-relative operations. Expressing descriptor-relative publication needs either:
- A platform-specific path through
/proc/self/fd/<dirfd>/<name>on Linux (works for open/rename in practice but is Linux-only and still path-based inside the kernel resolution), or - A native addon wrapping
openat/renameat/fstatat(new build matrix for the extension, cannot ship in a split unit), or - An explicit documented residual-risk decision for the Windows/POSIX baseline.
Proposed acceptance criteria
- Decide between (1), (2) and (3) with maintainer input, recorded in
docs/architecture/. - If (1): implement a Linux-only descriptor-relative publish path behind a capability check, with unit tests for symlink-swap and rename-into-swapped-parent scenarios.
- If (3): document the residual race and the mitigations above in the guarded-write design notes, and add a regression test that the pinned
(dev, ino)re-check rejects a swapped ancestor.
References
- PR feat(tools): guarded write core under the shared lock (U5, #1375) #1914 (guarded-write core unit; Security Boundaries pre-merge check)
src/core/tools/guardedWrite.ts— containment walk, path pinning, identity re-checksrc/services/file-safety/safeWriteText.ts— staging,expectedResolvedPath,expectedAncestors
Activity
fix(file-safety): surface an unremovable pre-write backup and cover f…
fix(file-safety): pin the authorized ancestry before the commit rename
fix(safety, task): cover the directory-failure paths and release the …
easonLiangWorldedtech commentedon Nov 10, 2026
Cross-link, not a routing artifact: a reader who lands here should find #2413 and the cost of the work being deferred.
This issue owns the primitive itself. Descriptor-relative publication, openat and renameat over an already-open directory fd with no-follow semantics, is what src/utils/safeWriteJson.ts row 74 says this runtime does not expose, so confined writes today protect themselves by lstat-and-refuse instead: src/services/file-safety/safeWriteText.ts 326-332 refuses a staging path that is a symlink or not a regular file, 196-208 refuses to publish through a link, and src/utils/safeWriteJson.ts 443-453 throws ConfinedPathEscapeError after staging when the publish target left the confined root.
#2413 (#2413), opened from the review of PR 1913 at head 44a862d, does not re-open that primitive. It carries the two behaviours the primitive is currently standing in for, recovering a matching backup under the referent lock before a dangling link is rejected and carrying the Windows security descriptor across the JSON commit rename at src/utils/safeWriteJson.ts:407 the way src/services/file-safety/safeWriteText.ts already does at 576-578 through _restoreDaclWindows, plus the bounded retry on the target mode read at safeWriteJson.ts:587 whose refusal sits at 591-592.
The cost, so it is attached to the older issue as well: PR 1913 stands at exactly 500 changed executable lines and 192 selectors against its own merge base 73931fe and the same 500 and 192 against main tip 18861b2 through clean synthetic merge d906391b849b, advisories 1 on both legs. A probe commit that adds one 8-line bounded retry around the stat at 587, probe commit c7de3f00eed8 kept off the branch, prices that same tree at 506 changed executable lines against 73931fe and 507 against 18861b2 through synthetic merge 1c09a5e96d1b, against the 500-line gate the mutation-gate runner enforces. That is why the retry and the two confinement behaviours are separate units rather than additions to 1913, and nothing on 1913 was trimmed to make room for them.