fix(client): pin the skills root through /proc/self/fd on Linux (SEC-8985 row 2)

[XieX]

Closes the root-swap window in the Agent Skills filesystem layer.

• SEC-8985 row 2
• Supersedes test(client): Agent Skills — root-swap races the pinned handle does not close (SEC-8985 row 2) #49 (Security's expected-failure tests, cherry-picked here with -x so authorship is preserved). Please close test(client): Agent Skills — root-swap races the pinned handle does not close (SEC-8985 row 2) #49 as superseded — I have deliberately left it open for you.
• Security review response: https://launchdarkly.atlassian.net/wiki/spaces/PD/pages/5293965360

The first commit is Security's tests and CI was red on it (run — three failures, real assertion failures rather than harness errors). The head of this branch is green.

The gap

resolveRoot validated the managed root and returned a path; nothing held it open. writeThroughPinnedDirectory, pruneOne and sweepSkillDirectory then opened <root>/<key> by absolute path with O_NOFOLLOW, which guards only the final component — so the root was re-resolved on every one of those opens, and a root swapped for a symlink after validation redirected the open into the attacker's directory. On the create path mkdir(<root>/<key>) followed it too. assertUnswapped did not notice, because it compared the handle against an lstat of the same swapped path the handle came from, so the two agreed. Only the manifest write pinned the root, and by then the skill files were already outside it.

The attacker precondition is write access to the root's parent — .claude for a root of .claude/skills, which the agent identity typically owns. The README's privilege-separation checklist did not mention ancestors, so the documented mitigation for Node's residual exposure did not cover this.

The fix

1. New SUPPORTS_PROC_FD probe in safe-fs.ts: true on Linux when /proc/self/fd is usable, verified at module load by opening a descriptor and confirming the magic symlink resolves back to that descriptor's own inode. A genuine probe, not a platform string, for the same reason SUPPORTS_DIR_FD is one — the swap-race tests are gated on it.
2. writeSkills opens the root O_RDONLY|O_DIRECTORY|O_NOFOLLOW immediately after resolveRoot, fstats it, and holds it for the whole reconcile (closed in finally). Threaded through writeAll, pruneEntries, rewriteManifest, sweepOrphanTemps, loadManifest and the per-skill helpers as a PinnedDirectory, which carries both the real path (for reports, manifest keys and containment checks) and the address (for filesystem calls).
3. On the fast path every child is addressed as /proc/self/fd/<fd>/<name> for mkdir, open, rename, unlink, rmdir, lstat, readdir and the manifest write. The kernel resolves that prefix to the pinned inode, so a swap of the root's name cannot redirect the operation. The per-skill directory is pinned in turn for the files inside it.
4. Off the fast path, path-based operations and the assertUnswapped floor are unchanged. assertUnswapped is skipped only for a directory the caller addressed through a descriptor — driven by how the caller addressed it rather than by a global flag, which is what keeps the floor's own tests in safe-fs.test.ts exercising it on Linux too.
5. unsafePathReason containment checks stay as defense in depth. realpath collapses a descriptor address to the real location, so the comparison reads identically on both paths.

A swap that lands before the pin fails the open and is reported as a run-level error action rather than thrown: resolveRoot throws because an unusable root is a caller mistake, whereas a root that was a real directory an instant ago is an attack in flight.

Beyond the brief

Two things I did not find in the ticket but that follow from it:

• atomicWriteIn removed. It existed to re-open the root by path for the manifest write; with the root pinned for the whole reconcile that is both redundant and exactly the footgun this bug was made of. rewriteManifest now writes through the held handle. Its one direct test went with it — the property it covered (a symlinked root is refused) is now covered by the new root-pin refusal test.
• Descriptor addresses stripped from errno messages. Otherwise an operator reads open '/proc/self/fd/23/a/SKILL.md'. Stripping the prefix leaves the file named relative to whichever directory was pinned, which is the form the rest of the messages already use.

Tests

Security's assertions are unchanged. Their trigger was not, and this matters: the hook fired the swap only when the intercepted mkdir/open argument strictly equalled path.join(root, 'a'). After the fix the code passes /proc/self/fd/N/a, so the trigger would never have matched and all three tests would have passed vacuously. The trigger now matches on the final component plus the shape of its parent — the root's own path or a descriptor prefix — so one test body fires the swap against either implementation, which is the only thing that makes the red-to-green transition evidence of anything.

• Gated describe.skipIf(!SUPPORTS_PROC_FD): runs on Linux CI, skipped on macOS dev machines.
• Added a fourth case: a root swapped before the pin is refused with a run-level error.
• The two existing <root>/<key> races in skills-fs.test.ts now run when either capability probe is true, so they are live on Linux for the first time.
• Added a probe test asserting SUPPORTS_PROC_FD reflects the real property (never true off Linux; on Linux only when the descriptor resolves to its own inode).
• interceptRename/interceptUnlink resolve descriptor addresses back to real paths at interception time, while the descriptor is still open — four existing tests assert on where an operation landed.

Verified against the pre-fix code in a Linux container (docker run --rm -v "$PWD":/w node:24), with both source files reverted to the base and only the probe shimmed in: all four fail with real assertion failures — the write escapes into outside, the outside SKILL.md is clobbered with served content, the outside victim is unlinked. All four pass against this branch.

Local: yarn test green (652 client tests; 646 + 6 skipped on macOS, all 652 on Linux), biome check clean, yarn typecheck clean, sherif clean.

Docs

packages/client/README.md: the privilege-separation checklist and its shell snippet now cover the root's parent and every ancestor (the snippet walks up to /), and the Security posture paragraph no longer claims the Python SDK is alone in holding a descriptor for the whole reconcile or that Node's floor runs on every platform. It now splits by platform: Linux holds the root handle and addresses children through /proc/self/fd; macOS and Windows keep the per-component lstat floor, where write permission on the root and now its ancestors is the security boundary.

Not in this PR

The companion Python fix is being done separately in python-ai-sdk.

🤖 Generated with Claude Code

---

Note

Overview
Closes a TOCTOU hole where swapping the managed skills root for a symlink after resolveRoot could redirect writeSkills writes and prunes outside the intended directory (attackers need write access on the root’s parent, e.g. .claude for .claude/skills).

Linux: writeSkills now opens and holds the managed root for the entire reconcile and routes child operations through /proc/self/fd/<fd>/<name> via a new SUPPORTS_PROC_FD probe and directoryAddress / PinnedDirectory (real paths for reports, descriptor addresses for syscalls). atomicWriteIn is removed; the manifest write uses the pinned root. assertUnswapped is skipped when paths are already descriptor-relative.

macOS / Windows: Behavior stays on the per-component lstat floor; README now documents that the security boundary includes ancestor directories, with an expanded privilege-separation checklist and shell snippet.

Tests: New Linux-gated root-swap race suite; existing swap-race and intercept tests updated so triggers match both path and proc-fd spellings and assertions still resolve real paths.

^{Reviewed by Cursor Bugbot for commit 35f3779. Bugbot is set up for automated code reviews on this repo. Configure here.}