Repository navigation
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 …