fix(store): enforce portable keys across methods

[steipete]

Summary

• enforce one portable key grammar across every async and sync FileStore method, in private and non-private stores;
• reject parent-segment normalization and literal/Windows backslash aliases with invalid-path instead of letting selected async methods delegate them to Root;
• preserve missing-root error precedence and Root's broader confined existing-object compatibility;
• add full method/private-mode regressions, exact grammar documentation, and a 0.7.3 compatibility note.

Root cause

FileStore already documented every relativePath, including reads, exists, and remove, as a portable store key. Its central assertion nevertheless returned raw inputs containing .., backslashes, or absolute syntax so some async methods could delegate them to Root. Methods that also called resolveStorePath()—including path(), all sync methods, writes, and json() construction—rejected those same spellings.

That split produced method- and platform-dependent key identity:

• async reads accepted a/../b on POSIX and returned the bytes of b, while path() and sync reads rejected the key;
• async reads, exists, and remove accepted a literal POSIX filename a\b, while Windows interpreted the same spelling as a separator and strict FileStore methods rejected it;
• private and non-private stores behaved the same.

No root escape was demonstrated. This is a portable key identity and method-parity defect; filesystem confinement remains owned by Root.

History confirms the distinction is intentional: the portable FileStore contract was added separately from Root's legal existing-object compatibility. A later canonicalization change introduced the raw delegation branch and changed POSIX backslashes from separator conversion to literal lookup without naming that behavior as a compatibility surface.

Repair

assertRelativePath() now reuses splitSafeRelativePath() and then applies FileStore's stricter canonical checks. It rejects empty, dot/parent, empty-segment, backslash, absolute/rooted, drive-relative, NUL, non-NFC, padded, and trailing-dot/space spellings while preserving accepted nested/NFC names, .hidden, a..b, internal spaces, and non-drive colons such as timestamps.

The implementation removes the exceptional delegation path and its redundant drive check: production delta is +4/-9, net -5 lines. src/file-store.ts remains below its 500-line budget.

Compatibility and boundaries

For an existing root, async reads, exists, and remove now reject a/../b, a\b, and the other nonportable aliases with invalid-path, matching path(), writes, sync methods, and json() construction.

Evaluation order is intentionally unchanged:

• async methods still open the root before key validation, so with a missing root, strict reads/exists/remove report not-found and IfExists readers return null, even for an invalid key;
• private copyIn still reads its source before destination-key validation;
• writeJson still serializes before destination-key validation.

This does not change Root, confinement/link/identity policy, pruning, native code, exports/types, standalone jsonStore({ filePath }), tilde handling, Windows device/ADS/case policy, or dependencies.

Validation

• new portable-key suite: 162 passed;
• focused store/path/API set: 328 passed / 4 skipped;
• full CI=1 pnpm check: 201 test files passed / 2 skipped, 7,080 tests passed / 80 skipped;
• security suite: 84 passed;
• release-package smoke: passed;
• git diff --check: passed;
• Codex autoreview: no accepted/actionable findings, patch correct at 0.97.

The regression suite covers every keyed async method (path, open, all read forms, exists, remove, all write forms, writeStream, copyIn, and json) and every keyed sync method, for both confirmed aliases with private: false and true. It verifies invalid reads disclose no sentinel bytes, invalid mutations preserve byte-exact fixtures, missing-root precedence stays stable, accepted Unicode/nested keys round-trip, JSON helpers work with locking on/off, and direct Root controls retain broader confined spellings while rejecting escapes.

Fresh packed-consumer proof

Candidate package SRI:

sha512-+ZAByrjWo95ZkmK0Df7jJ+jVGuhvZiPXZvu/XMs0zV41FZuViy9M6DbsNk0x9eLrUoxIvtFussd76FnxAdYjuQ==

A fresh physical consumer ran on macOS 26.6.2 arm64, and fresh local containers ran Debian 12 arm64. Each environment exercised exact Node.js 22.0.0, 22.23.2, and 24.20.0 in native modes off and require.

• 6 macOS lanes × 110 assertions = 660 passed;
• 6 Linux lanes × 110 assertions = 660 passed;
• total: 1,320 passed packed-consumer observations.

Every lane exercised all 22 keyed methods across both private modes for a/../b and a\b, checked exact fixture preservation, pinned missing-root precedence, and ran successful nested Unicode controls. Native off loaded no package .node binding. Every require lane actually loaded its matching @openclaw/fs-safe-darwin-arm64 or @openclaw/fs-safe-linux-arm64-gnu binary; the key-validation behavior itself remains JavaScript-owned.

The baseline packed-main/released behavior was captured in both native modes before this repair. Linux results are local-container proof, not remote/AWS proof. Windows filesystem behavior will be covered by exact-head CI, including the new cross-platform regression suite; no separate physical Windows packed consumer is claimed.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 3, 2026, 10:05 AM ET / 14:05 UTC.

ClawSweeper review

What this changes

The PR makes every FileStore key-taking method reject noncanonical aliases such as parent segments and backslashes, adds cross-method regression coverage, and documents the strict portable-key contract.

Merge readiness

⛔ Blocked before merge - 1 item remains

Keep this PR open as a merge-ready repair. The prior compatibility-policy blocker is resolved by the explicit strict-key decision in the discussion, and the patch consistently restores FileStore’s documented portable-key identity without changing Root confinement.

Priority: P1
Reviewed head: 5499408f14a5b0f96353ddefcf7b4648ad11a708

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	A focused public-contract repair with strong packaged-consumer evidence, targeted regression coverage, and an explicitly resolved compatibility policy.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The changed production owner is FileStore key validation; the PR’s packed-consumer evidence reports exercising its keyed methods across private modes, Node versions, native modes, and macOS/Linux environments, with canonical-key controls and preserved missing-root behavior. Exact-head Windows CI also passed the new cross-platform suite.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The changed production owner is FileStore key validation; the PR’s packed-consumer evidence reports exercising its keyed methods across private modes, Node versions, native modes, and macOS/Linux environments, with canonical-key controls and preserved missing-root behavior. Exact-head Windows CI also passed the new cross-platform suite.
Evidence reviewed	6 items	Central validation now rejects aliases: The changed FileStore assertion delegates parsing to splitSafeRelativePath and rejects any spelling that does not round-trip as canonical; all keyed async reads, existence checks, and removal call that assertion after opening Root. Base behavior had an exceptional delegation path: Before this PR, the assertion returned raw keys containing parent segments, backslashes, or absolute syntax, allowing selected async operations to pass them to Root while path resolution and synchronous operations rejected them. Regression coverage spans the changed contract: The new suite exercises the two demonstrated aliases across keyed async and sync APIs, both private modes, missing-root precedence, accepted Unicode keys, and Root’s intentionally broader confined lookup behavior.
Findings	None	None.
Security	None	None.

How this fits together

FileStore is the package’s keyed file abstraction for application data, while Root provides the lower-level confined filesystem operations. Application-supplied keys pass through FileStore validation before reads, writes, existence checks, or deletion reach Root.

flowchart LR
  A[Application key] --> B[FileStore method]
  B --> C[Open store root]
  C --> D[Portable key validation]
  D -->|canonical key| E[Root confinement]
  D -->|invalid alias| F[invalid-path error]
  E --> G[Read write exists or remove]

Loading

Before merge

• Resolve merge risk (P1) - Existing callers that relied on selected async methods accepting noncanonical aliases such as a/../b or POSIX backslashes must switch to canonical FileStore keys or intentionally use store.root() for Root pathname semantics.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production versus regression delta	production +4/-9; tests +263/-1; docs/changelog +40/-9	The small validation change is backed by broad method- and platform-oriented regression coverage plus an explicit compatibility record.
Keyed API coverage	16 async and 6 sync method entries exercised	The new suite targets the prior method-parity gap rather than only the originally observed read aliases.

Merge-risk options

Maintainer options:

1. Accept the documented strict-key upgrade (recommended)
   Merge the repair under the recorded 0.7.3 policy: FileStore callers use canonical keys, while broader confined pathname handling remains available through Root.

Technical review

Best possible solution:

Land the strict documented FileStore key policy with the stated 0.7.3 compatibility note, retaining Root as the explicit opt-in surface for broader confined existing-object path semantics.

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

Yes—source inspection establishes the base-path discrepancy: selected async operations delegated raw aliases to Root while FileStore path resolution and synchronous methods rejected them. The added regression suite provides a focused current-head verification path.

Is this the best way to solve the issue?

Yes—the shared strict parser is the narrowest maintainable repair because it makes all FileStore methods enforce one documented portable key grammar while preserving Root’s separate confinement contract.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

• add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed production owner is FileStore key validation; the PR’s packed-consumer evidence reports exercising its keyed methods across private modes, Node versions, native modes, and macOS/Linux environments, with canonical-key controls and preserved missing-root behavior. Exact-head Windows CI also passed the new cross-platform suite.
• remove rating: 🦐 gold shrimp: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.
• remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

• P1: This repairs an existing method- and platform-dependent FileStore key identity defect in public filesystem APIs.
• merge-risk: 🚨 compatibility: Selected existing-root async calls change from accepting noncanonical aliases to returning invalid-path, as intentionally documented for the upgrade.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed production owner is FileStore key validation; the PR’s packed-consumer evidence reports exercising its keyed methods across private modes, Node versions, native modes, and macOS/Linux environments, with canonical-key controls and preserved missing-root behavior. Exact-head Windows CI also passed the new cross-platform suite.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner is FileStore key validation; the PR’s packed-consumer evidence reports exercising its keyed methods across private modes, Node versions, native modes, and macOS/Linux environments, with canonical-key controls and preserved missing-root behavior. Exact-head Windows CI also passed the new cross-platform suite.

Evidence

What I checked:

• Central validation now rejects aliases: The changed FileStore assertion delegates parsing to splitSafeRelativePath and rejects any spelling that does not round-trip as canonical; all keyed async reads, existence checks, and removal call that assertion after opening Root. (src/file-store.ts:111, 5499408f14a5)
• Base behavior had an exceptional delegation path: Before this PR, the assertion returned raw keys containing parent segments, backslashes, or absolute syntax, allowing selected async operations to pass them to Root while path resolution and synchronous operations rejected them. (src/file-store.ts:112, e940b0da8717)
• Regression coverage spans the changed contract: The new suite exercises the two demonstrated aliases across keyed async and sync APIs, both private modes, missing-root precedence, accepted Unicode keys, and Root’s intentionally broader confined lookup behavior. (test/file-store-key-policy.test.ts:105, 5499408f14a5)
• Established FileStore history owner: Feature history shows Peter Steinberger as the dominant contributor to the current FileStore implementation, including prior store, path, and async/sync contract repairs. (src/file-store.ts:106, 8b5709750ede)
• Compatibility direction is explicitly approved: The discussion records a strict portable-key policy for the 0.7.3 repair, directs callers needing broader confined pathname semantics to store.root(), and rejects adding a legacy compatibility mode.
• Repository policy applied: The PR updates Unreleased release notes and focused traversal/path regressions while preserving the separation between FileStore key identity and Root confinement required by repository policy. (CHANGELOG.md:4, 5499408f14a5)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

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 (1 earlier review cycle)

• reviewed 2026-09-03T13:52:50.774Z sha 5499408 :: blocked before merge. :: [P1] Gate legacy-key rejection on an approved upgrade policy

[steipete]

Maintainer decision: approve strict portable-key rejection for this 0.7.3 - Unreleased repair.

FileStore already documents one portable key language across every method. The accepted aliases are not a named compatibility contract: a/../b worked only through selected async delegation to Root, while path(), writes, json(), and sync methods rejected it; literal POSIX a\b changed from separator conversion to literal lookup during a later canonicalization change. Preserving either behavior would institutionalize method- and platform-dependent key identities or require adding a new public legacy mode.

The supported upgrade is explicit and bounded: callers should pass canonical portable FileStore keys. Callers that intentionally need broader confined existing-object pathname semantics can use store.root(), whose contract and containment checks remain unchanged. Missing-root precedence also remains unchanged, so this repair does not reorder operational failures.

The compatibility impact is called out in CHANGELOG.md and the exact grammar, method coverage, Root distinction, and evaluation-order caveats are documented in docs/file-store.md. The 1,320 packed-consumer observations and cross-platform CI cover the strict behavior and preserved boundaries.

No compatibility shim or mode should be added for these undocumented aliases.

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/33764999122
• Updated: 2026-09-03T14:07:43.344Z

[steipete]

Landed as e9b98dff22daf0700dd012bcf90b74048fc489ce with tree 986455be35a9efefc3a53d0f4897459919086462.

Verified final head 5499408f14a5b0f96353ddefcf7b4648ad11a708 before merge:

• new portable-key suite: 162 passed;
• focused store/path/API set: 328 passed / 4 skipped;
• full check: 201 test files passed / 2 skipped, 7,080 tests passed / 80 skipped;
• security suite: 84 passed;
• release-package smoke and git diff --check: passed;
• Codex autoreview: no accepted/actionable finding, patch correct at 0.97;
• replacement ClawSweeper review: compatibility policy resolved, no correctness or security finding, proof sufficient, merge-ready;
• exact-head CI: Node 22/24 on Linux, macOS, and Windows; native Linux/macOS/Windows/musl; package smoke on all three OSes; coverage and merged coverage; CodeQL; Cargo clippy/audit; workflow lint; and benchmarks all passed.

Fresh packed-consumer proof covered physical macOS 26.6.2 arm64 and Debian 12 Linux arm64 local containers, exact Node.js 22.0.0/22.23.2/24.20.0, native off/require, and private false/true: 1,320/1,320 observations passed. Every lane exercised all 22 keyed method entries for a/../b and a\b, preserved fixture bytes and missing-root precedence, and passed nested Unicode controls. off loaded no package binary; every require lane loaded its matching darwin-arm64 or linux-arm64-gnu binding. Linux is local-container proof, not remote/AWS; no physical Windows packed consumer is claimed, while exact-head Windows CI ran the cross-platform regression suite.

The compatibility policy is intentional and recorded in this PR: FileStore callers must use canonical portable keys; callers intentionally needing broader confined existing-object path semantics use store.root(). No legacy mode was added for undocumented, method-dependent aliases.

Production became smaller: +4/-9 lines, net -5, with FileStore still under its 500-line budget.