fix(path): harden root reads and store keys

[steipete]

Summary

• enforce the documented default rejection of intermediate symlinks across Root reads and sync/async root-file opens while preserving explicit in-root following
• bind reads to the pre-open inode, use nonblocking opens where supported, and normalize symlink-loop and overlong-path errors
• reject padded FileStore keys and Windows device names disguised with ignored trailing characters
• exhaustively cover both sides of the drive-relative Root/FileStore contract, including sync and wrapper methods

Verification

• pnpm check
• pnpm test:security
• git diff --check
• source-blind built-artifact behavior contract: 7/7 clauses passed
• Codex autoreview: clean, no accepted/actionable findings

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed August 2, 2026, 7:45 PM ET / 23:45 UTC.

ClawSweeper review

What this changes

The branch hardens root-bounded reads and root-file opens against symlink and swap races, makes read opens nonblocking where supported, and rejects ambiguous FileStore keys and Windows device-name spellings.

Merge readiness

⛔ Blocked until real behavior proof is added - 5 items remain

Keep this PR open. The prior intermediate-symlink finding is addressed in the current head with focused regression coverage, but the PR still lacks after-fix real behavior evidence and the Node 24 Windows check is failing without an inspected failure log.

Priority: P2
Reviewed head: 756e02c72a748e7127042427bdceb9ba1e80e97b

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	The patch has focused source-level coverage, but absent real behavior proof and an unexplained Windows check failure leave it below merge-ready confidence.
Proof confidence	🧂 unranked krab (1/6)	Needs real behavior proof before merge: The PR body claims checks and a behavior contract, but it provides no after-fix runtime capture, log, recording, or linked artifact; add redacted live evidence before merge and update the PR body to trigger re-review.
Patch quality	🦐 gold shrimp (3/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The PR body claims checks and a behavior contract, but it provides no after-fix runtime capture, log, recording, or linked artifact; add redacted live evidence before merge and update the PR body to trigger re-review.
Evidence reviewed	5 items	Intermediate-symlink repair: The branch makes lexical root-path resolution throw a typed symlink error after detecting any symlink component when rejection is requested; Root read operations request that policy unless callers explicitly select in-root following. Read-path policy wiring: The read helper passes both unsafe-device precedence and default symlink rejection into root-path resolution before opening the file, preserving the explicit follow-within-root mode. Focused regression coverage: The new stress regression covers default rejection of an intermediate symlink, explicit in-root following, parent-directory swap detection, FIFO replacement, padded keys, and overlong paths.
Findings	None	None.
Security	None	None.

How this fits together

Root is fs-safe’s capability boundary for application paths: callers supply an untrusted relative or in-root absolute path, which is resolved and opened under confinement before data reaches file-reading APIs. FileStore builds on that boundary to map caller keys to files safely across platforms.

flowchart LR
  A[Caller path or store key] --> B[Root and FileStore validation]
  B --> C[Root path component resolution]
  C --> D[Symlink and root-boundary policy]
  D --> E[Descriptor open and identity comparison]
  E --> F[Safe file data or typed error]
  B --> G[Portable filename and key checks]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The PR body claims checks and a behavior contract, but it provides no after-fix runtime capture, log, recording, or linked artifact; add redacted live evidence before merge and update the PR body to trigger re-review.
• Resolve merge risk (P1) - Existing applications that relied on stable intermediate symlinks during default Root reads will now receive a typed symlink failure unless they opt into follow-within-root.
• Resolve merge risk (P1) - The current check set reports a failing Node 24 Windows check; without its failure details, cross-platform compatibility of this path-sensitive change remains unconfirmed.
• Resolve merge risk (P1) - The PR body lists checks and a built-artifact contract but includes no redacted live terminal output, runtime log, recording, or linked artifact demonstrating the changed symlink, swap, or FIFO behavior.
• Complete next step (P2) - The remaining blockers are contributor-supplied live proof and cross-platform check interpretation, not a narrow mechanical repair that ClawSweeper can safely perform.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Public contract surface	21 files affected; 364 added, 104 removed	The change spans read semantics, key validation, documentation, and cross-platform regression coverage, so upgrade proof matters beyond unit tests.
Focused regression cases	6 security/portability scenarios in 1 new stress test	The new test exercises intermediate symlinks, identity swaps, FIFO replacement, padded keys, and path-length classification.

Merge-risk options

Maintainer options:

1. Verify platform behavior before merge (recommended)
   Resolve or explain the Node 24 Windows failure and attach redacted after-fix runtime proof for the symlink and race-sensitive read behavior before accepting the compatibility change.
2. Pause for a narrower follow-up
   Pause this branch if Windows support cannot retain the documented error and containment behavior without a smaller platform-specific design.

Technical review

Best possible solution:

Land the centralized hardening only after the Windows Node 24 failure is resolved and the contributor supplies redacted after-fix runtime evidence showing default rejection, explicit in-root following, and at least one race/FIFO protection path.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: the added stress test creates the intermediate-symlink, parent-swap, and FIFO scenarios directly. This read-only review did not execute the branch or inspect a live proof artifact.

Is this the best way to solve the issue?

Yes, conditionally: centralizing the policy in Root path resolution and descriptor identity checks is the narrowest maintainable boundary fix, provided Windows behavior and real runtime results confirm the intended contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4696341685a9.

Labels

Label justifications:

• P2: This is a bounded security and compatibility hardening change to a public filesystem API, with no confirmed active outage.
• merge-risk: 🚨 compatibility: Default Root-read behavior changes for callers that currently traverse intermediate symlinks without explicitly opting into following.
• merge-risk: 🚨 security-boundary: The patch changes root-confinement, symlink, descriptor-identity, and FIFO handling at the package’s core filesystem boundary.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body claims checks and a behavior contract, but it provides no after-fix runtime capture, log, recording, or linked artifact; add redacted live evidence before merge and update the PR body to trigger re-review.

Evidence

What I checked:

• Intermediate-symlink repair: The branch makes lexical root-path resolution throw a typed symlink error after detecting any symlink component when rejection is requested; Root read operations request that policy unless callers explicitly select in-root following. (src/root-path.ts:494, 756e02c72a74)
• Read-path policy wiring: The read helper passes both unsafe-device precedence and default symlink rejection into root-path resolution before opening the file, preserving the explicit follow-within-root mode. (src/root-impl.ts:649, 756e02c72a74)
• Focused regression coverage: The new stress regression covers default rejection of an intermediate symlink, explicit in-root following, parent-directory swap detection, FIFO replacement, padded keys, and overlong paths. (test/path-stress-regression.test.ts:32, 756e02c72a74)
• Feature-history provenance: Blame attributes the new symlink-rejection lines to the branch’s initial hardening commit, while nearby resolver code originates in the v0.5.1 baseline; Peter Steinberger is also the dominant contributor across the central path files. (src/root-path.ts:494, cc17c4d67485)
• Release status: The latest tagged release is v0.5.1 at 16e1bd4; this open branch head is not part of that release. (CHANGELOG.md:12, 16e1bd489ae8)

Likely related people:

• steipete: Peter Steinberger authored the branch hardening commits and dominates the available history for the Root path, Root implementation, root-file, and FileStore surfaces. (role: Recent area contributor; confidence: high; commits: cc17c4d67485, afd85b10e6b7, 16e1bd489ae8; files: src/root-path.ts, src/root-impl.ts, src/root-file.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Attach redacted terminal output, a runtime log, or a recording showing the changed symlink and race/FIFO behavior after the fix.
• Resolve or link the cause of the Node 24 Windows check failure, then refresh validation for the current head.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)

• reviewed 2026-08-02T23:04:24.598Z sha 3bc5886 :: needs real behavior proof before merge. :: [P2] Reject intermediate symlinks before resolving their targets
• reviewed 2026-08-02T23:35:10.595Z sha cf0c8a0 :: needs real behavior proof before merge. :: [P2] Reject intermediate symlinks before resolving their targets
• reviewed 2026-08-02T23:40:01.217Z sha dc7bbbb :: needs real behavior proof before merge. :: [P2] Reject intermediate symlinks before resolving their targets