Skip to content

fix(core): CAS write gateway + per-path advisory locks for TOCTOU safety - #68

Open
bigknoxy wants to merge 1 commit into
mainfrom
fix/toctou-cas-lock-21
Open

fix(core): CAS write gateway + per-path advisory locks for TOCTOU safety#68
bigknoxy wants to merge 1 commit into
mainfrom
fix/toctou-cas-lock-21

Conversation

@bigknoxy

Copy link
Copy Markdown
Owner

Problem

Closes #21

Every tier (AST, hash, diff) does read→transform→write with no re-validation between the read and the write. Concurrent edits silently lose work (last-write-wins). The hash tier's stale-anchor check defaults to auto-recovery instead of aborting.

Approach

1. CAS (Compare-And-Swap) Write Gateway

  • routeRead() returns file content + hash for CAS gating
  • casWrite() re-reads file hash immediately before write, aborts with STALE_ANCHOR if stale, returns fresh hash for retry
  • Applies to all three tiers (AST, hash, diff), not just hash

2. Per-Path Advisory Lock (src/core/lock.ts)

  • Atomic file creation (.hashpilot/locks/<sha256>.lock)
  • PID-liveness stale lock breaking
  • Sorted lock acquisition order prevents deadlock
  • LOCK_TIMEOUT error code with bounded wait

3. Hash Tier noRecovery

  • replaceHash() called with noRecovery: true when oldHash is set
  • Dry-run bypasses stale check for simulation

4. editMany Conflict Distinction

  • BatchSummary.conflicts counts stale-anchor failures separately from logic failures

Tests (23 new)

Category Tests
routeRead 3 (content, hash, nonexistent)
CAS: AST tier 3 (stale abort, correct hash, backward compat)
CAS: diff tier 2 (stale abort, correct hash)
CAS: hash tier 2 (no auto-recover, correct hash)
Dry-run bypass 2 (AST, hash)
No regression 3 (AST, hash, diff)
LOCK_TIMEOUT 2
Advisory lock 3 (create/release, timeout, stale break)
Deadlock prevention 2 (sorted order, property check)
editMany conflicts 1

Sabotage run verified: reverting CAS changes → tests fail, confirming they catch the absence of the fix.

Risk

  • Breaking change: Hash tier with oldHash now fails instead of auto-recovering. Callers relying on auto-recovery need to handle STALE_ANCHOR and retry.
  • Backward compat: Calling routeEdit without oldHash still works (CAS is opt-in via the hash parameter).

Exclusions

  • Per-path locks are process-local only (no networked locking for remote FS)
  • Advisory locks use polling (100ms interval) rather than inotify/fsevents
  • No changes to the CLI interface

Closes #21

Problem:
Every tier (AST, hash, diff) does read→transform→write with no
re-validation between the read and the write. Concurrent edits
silently lose work (last-write-wins). The hash tier's stale-anchor
check defaulted to auto-recovery instead of aborting.

Solution:
1. CAS (compare-and-swap) write gateway in router.ts:
   - routeRead() returns file hash for CAS gating
   - casWrite() re-reads file hash immediately before write,
     aborts with STALE_ANCHOR if stale, returns fresh hash for retry
   - Applies to all three tiers (AST, hash, diff)

2. Per-path advisory lock (src/core/lock.ts):
   - Atomic file creation (.hashpilot/locks/<sha256>.lock)
   - PID-liveness stale lock breaking
   - Sorted lock acquisition order prevents deadlock
   - LOCK_TIMEOUT error code with bounded wait

3. Hash tier noRecovery when oldHash provided:
   - replaceHash() called with noRecovery: true when oldHash is set
   - Dry-run bypasses stale check for simulation

4. editMany conflict distinction:
   - BatchSummary.conflicts counts stale-anchor failures separately
   - editManySerial also tracks conflicts

Tests: 23 new toctou tests covering:
- Stale hash detection across all 3 tiers
- CAS success with correct hash
- Backward compat (no oldHash)
- Dry-run bypass
- LOCK_TIMEOUT error code
- Per-path advisory lock acquire/release
- Stale lock breaking (dead PID)
- Deadlock prevention (sorted lock ordering)
- editMany conflict count

Sabotage run: reverting CAS changes → tests fail, confirming they
catch the absence of the fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P1] Read-modify-write TOCTOU across all tiers — concurrent edits silently lose work

1 participant