mosaicstack/stack
21
0
Fork 1
Code Issues 338 Pull Requests 13 Actions Packages Projects Releases 15 Wiki Activity

fix(wake): #927 enqueue TOCTOU — move stale-tmp cleanup off the hot enqueue path (no concurrent in-flight-write clobber) #928

Merged
Ghost merged 2 commits from fix/wake-927-enqueue-toctou into main 2026-07-26 10:56:41 +00:00
Conversation 2 Commits 2 Files Changed 7
+301 -11
jason.woltje commented 2026-07-26 10:28:28 +00:00
Owner
Copy Link
Copy Source

Summary

Fixes a genuine product-level TOCTOU race in the wake store enqueue path (#927), reachable under live co-feed (detector + reconciler concurrently enqueue).

The race (RED, deterministic)

cmd_enqueue called _wake_init_dir() at store.sh:166 — which invoked _wake_clean_stale_tmp(), an unconditional delete of EVERY .wake.tmp.* — BEFORE _wake_lock_acquire() at store.sh:174. So a 2nd enqueue B, still in its pre-lock setup, deleted the live in-flight tmp of a 1st enqueue A that held the enqueue lock throughout its atomic write. A's mv tmp -> pending.jsonl then failed with 'No such file or directory' → spurious durable pending write FAILED abort of a perfectly valid enqueue. A's obligation was dropped on the floor.

Root cause is broader than enqueue-vs-enqueue: _wake_init_dir (hence the reap) is also called without the enqueue lock by cmd_consume, cmd_cursors, and ack.sh — any of those could clobber a concurrent enqueue's in-flight tmp. That ruled out the 'move it inside the enqueue lock' alternative (it would leave cursors/consume/ack still racing) in favour of Mos's lean: take the reap off the hot path entirely.

The fix (scalpel)

  • _wake_init_dir: no longer reaps tmps — it only ensures the layout + seeds cursor files. Nothing on the enqueue/consume/cursors/ack hot paths can delete another process's live write.
  • _wake_clean_stale_tmp: AGE-SCOPED (find -mmin +${WAKE_TMP_STALE_MIN:-5}). A live in-flight atomic write's tmp is ms-old, so it can never match — the reap only removes demonstrably-orphaned (crash-left) tmps, safe even if it ever overlaps a concurrent write.
  • Reaping now runs as an explicit maintenance action at store.sh init (daemon-start) and the detector poll tick (detector.sh), bounding accumulation once-per-pass instead of racing per-enqueue.

Invariants preserved

  • #908 seq-integrity unchanged: single store-side allocator, atomic allocate+enqueue under flock, arrow-1 no-burn, anti-swallow fail-loud. T8/T9/T10 all green.
  • reconcile.sh unchanged — reconciler-enumeration retry semantics (Mos's stated worst-case safety net) confirmed via the dragon-lin pilot trace at installed main: on an enqueue abort the retry is structural and total — NEITHER the seen-ledger (atomic-written only on success) NOR observed_seq advances, so the source stays unaccounted on BOTH sides and the next reconcile cycle re-enumerates it from scratch with fresh allocation. Loudness is double (reconcile rc=1 → co-feed alarm carrying the FAILED line + systemd unit failure); the loop continues past an aborted source (no starvation). Worst case = one loud 6h-cycle delay, zero silent-loss window. A 48h pilot journal+drain.log+store scan found zero occurrences (real-but-unfired: elevated, not emergency).

Tests / RED→GREEN

  • New deterministic harness test-wake-store-enqueue-race.sh (separate file to avoid the #923/#926 T10 union-rebase). It PATH-shadows mv (freezes enqueue A exactly at its rename with a live tmp, lock held) and flock (fires the instant enqueue B finishes its pre-lock cleanup), so the race window is held open by signal files, not scheduling luck. RED against unmodified store: A aborts with durable pending write FAILED (its tmp deleted by B's pre-lock reap). GREEN post-fix: both succeed with distinct gapless seqs {1,2}, zero spurious aborts, observed_seq=2, depth=2, CONSUMED 2 valid.
  • T5 (test-wake-store-ack.sh) updated: its old tail asserted the per-enqueue hot-path reap that is the bug; it now asserts the hot path does NOT reap tmps and that the age-scoped maintenance path (store.sh init) reaps a demonstrably-orphaned tmp. T5's core crash-safety invariant is intact.
  • All store-ack groups incl T10 stay green (11 groups).

Gates

  • shellcheck 0.11 clean on all changed shell files
  • verify-sanitized PASS; firewall (jason|woltje|jarvis) = 0, no operator hosts/paths
  • pnpm run test:framework-shell rc=0 normal AND openssl-masked (command -v openssl empty)
  • wake manifest bumped 0.6.4 → 0.6.5 + inventory (store.sh/detector.sh notes)

Files changed: _wake-common.sh, store.sh, detector.sh, test-wake-store-ack.sh (T5), test-wake-store-enqueue-race.sh (new), manifest.txt, package.json (suite wiring).

Closes #927
Part of #892

## Summary Fixes a genuine product-level **TOCTOU race** in the wake store enqueue path (#927), reachable under live co-feed (detector + reconciler concurrently enqueue). ## The race (RED, deterministic) `cmd_enqueue` called `_wake_init_dir()` at store.sh:166 — which invoked `_wake_clean_stale_tmp()`, an **unconditional delete of EVERY `.wake.tmp.*`** — **BEFORE** `_wake_lock_acquire()` at store.sh:174. So a 2nd enqueue B, still in its **pre-lock setup**, deleted the **live in-flight tmp** of a 1st enqueue A that held the enqueue lock throughout its atomic write. A's `mv tmp -> pending.jsonl` then failed with *'No such file or directory'* → spurious **`durable pending write FAILED`** abort of a perfectly valid enqueue. A's obligation was dropped on the floor. Root cause is broader than enqueue-vs-enqueue: `_wake_init_dir` (hence the reap) is also called **without the enqueue lock** by `cmd_consume`, `cmd_cursors`, and `ack.sh` — any of those could clobber a concurrent enqueue's in-flight tmp. That ruled out the 'move it inside the enqueue lock' alternative (it would leave cursors/consume/ack still racing) in favour of Mos's lean: **take the reap off the hot path entirely.** ## The fix (scalpel) - **`_wake_init_dir`**: no longer reaps tmps — it only ensures the layout + seeds cursor files. Nothing on the enqueue/consume/cursors/ack hot paths can delete another process's live write. - **`_wake_clean_stale_tmp`**: **AGE-SCOPED** (`find -mmin +${WAKE_TMP_STALE_MIN:-5}`). A live in-flight atomic write's tmp is ms-old, so it can never match — the reap only removes demonstrably-orphaned (crash-left) tmps, safe even if it ever overlaps a concurrent write. - Reaping now runs as an explicit **maintenance** action at **`store.sh init`** (daemon-start) and the **detector poll tick** (`detector.sh`), bounding accumulation once-per-pass instead of racing per-enqueue. ## Invariants preserved - **#908 seq-integrity unchanged**: single store-side allocator, atomic allocate+enqueue under flock, arrow-1 no-burn, anti-swallow fail-loud. T8/T9/T10 all green. - **reconcile.sh unchanged** — reconciler-enumeration retry semantics (Mos's stated worst-case safety net) confirmed via the dragon-lin pilot trace at installed main: on an enqueue abort the retry is **structural and total** — NEITHER the seen-ledger (atomic-written only on success) NOR observed_seq advances, so the source stays unaccounted on BOTH sides and the next reconcile cycle re-enumerates it from scratch with fresh allocation. Loudness is **double** (reconcile rc=1 → co-feed alarm carrying the FAILED line + systemd unit failure); the loop continues past an aborted source (no starvation). Worst case = one loud 6h-cycle delay, **zero silent-loss window**. A 48h pilot journal+drain.log+store scan found **zero** occurrences (real-but-unfired: elevated, not emergency). ## Tests / RED→GREEN - New deterministic harness **`test-wake-store-enqueue-race.sh`** (separate file to avoid the #923/#926 T10 union-rebase). It PATH-shadows `mv` (freezes enqueue A exactly at its rename with a live tmp, lock held) and `flock` (fires the instant enqueue B finishes its pre-lock cleanup), so the race window is held open by signal files, not scheduling luck. **RED against unmodified store**: A aborts with `durable pending write FAILED` (its tmp deleted by B's pre-lock reap). **GREEN post-fix**: both succeed with distinct gapless seqs {1,2}, zero spurious aborts, observed_seq=2, depth=2, CONSUMED 2 valid. - **T5** (test-wake-store-ack.sh) updated: its old tail asserted the per-enqueue hot-path reap that *is* the bug; it now asserts the hot path does NOT reap tmps and that the age-scoped maintenance path (`store.sh init`) reaps a demonstrably-orphaned tmp. T5's core crash-safety invariant is intact. - All store-ack groups incl **T10** stay green (11 groups). ## Gates - shellcheck 0.11 clean on all changed shell files - verify-sanitized PASS; firewall (jason|woltje|jarvis) = 0, no operator hosts/paths - `pnpm run test:framework-shell` **rc=0 normal** AND **openssl-masked** (`command -v openssl` empty) - wake manifest bumped **0.6.4 → 0.6.5** + inventory (store.sh/detector.sh notes) Files changed: `_wake-common.sh`, `store.sh`, `detector.sh`, `test-wake-store-ack.sh` (T5), `test-wake-store-enqueue-race.sh` (new), `manifest.txt`, `package.json` (suite wiring). Closes #927 Part of #892
jason.woltje added 1 commit 2026-07-26 10:28:28 +00:00
fix(wake): #927 enqueue TOCTOU — move stale-tmp cleanup off the hot enqueue path
ci/woodpecker/pr/ci Pipeline was successful
Details
7714697f7a 
cmd_enqueue called _wake_init_dir() (which reaped EVERY .wake.tmp.*
unconditionally) BEFORE taking the enqueue lock. So a 2nd enqueue's PRE-LOCK
cleanup deleted the LIVE in-flight tmp of a 1st enqueue that held the lock
through its atomic write -> spurious "durable pending write FAILED" abort of a
valid enqueue. Reachable under live co-feed (detector + reconciler concurrently
enqueue).

Fix (scalpel, Mos's lean = off the hot path):
- _wake_init_dir no longer reaps tmps; it only ensures the layout. Nothing on
  the enqueue/consume/cursors/ack hot paths can clobber a concurrent live write.
- _wake_clean_stale_tmp is AGE-SCOPED (-mmin +${WAKE_TMP_STALE_MIN:-5}) so it
  can only remove demonstrably-orphaned crash-left tmps, never a live (ms-old)
  in-flight write — safe even if it ever overlaps a concurrent enqueue.
- Reaping runs as an explicit MAINTENANCE action at store.sh init (daemon-start)
  and the detector poll tick, bounding accumulation once-per-pass, not raced
  per-enqueue.

#908 seq-integrity is unchanged (single store-side allocator, atomic
allocate+enqueue under flock, arrow-1 no-burn, anti-swallow fail-loud).
reconcile.sh is unchanged: an aborted enqueue advances neither the seen-ledger
nor observed_seq, so the source is re-enumerated next reconcile cycle (no
obligation loss).

Tests: new deterministic RED->GREEN harness test-wake-store-enqueue-race.sh
(PATH-shadowed mv/flock freeze enqueue A at its rename while B runs its pre-lock
cleanup — RED pre-fix: A aborts; GREEN post-fix: both succeed, distinct gapless
seqs {1,2}, zero spurious aborts). T5 updated to assert the hot path no longer
reaps tmps and that the maintenance path reaps a demonstrably-orphaned tmp. All
store-ack groups incl T10 stay green. manifest 0.6.4 -> 0.6.5.

Closes #927
Part of #892

Co-Authored-By: Claude Opus 4.8 <[email protected]>
jason.woltje added 1 commit 2026-07-26 10:42:54 +00:00
Merge remote-tracking branch 'origin/main' into fix/wake-927-enqueue-toctou
ci/woodpecker/pr/ci Pipeline was successful
Details
a8af5ed7f0
jason.woltje commented 2026-07-26 10:56:18 +00:00
Author
Owner
Copy Link
Copy Source

Record of Review — PR #928 (issue #927): enqueue TOCTOU fix (off-hot-path + age-scoped reap) — FINAL, CI-GREEN

VERDICT: APPROVE — GO. REVIEWED-HEAD (full-40): a8af5ed7f09631352a2d6bac084c0fc45781ecdd. CI: TERMINAL-GREEN (pipeline 2075; validates the combined #926-T10-deflake + #928-T5 store-ack file).

Review basis

Independent reviewer a3774b40 (opus, ≠ builder mosaic-coder) — APPROVE, all 7 criteria PASS, criteria 3/4 independently REPRODUCED:

  1. Scope/provenance — head==a8af5ed7…; mergeable; id2 ≠ commit-author mosaic-coder; body Closes #927 + Part of #892; net delta = #928s 7 files; T10 UNCHANGED vs main (#926s de-flake preserved; only T5 in the store-ack test); reconcile.sh BYTE-UNCHANGED; manifest 0.6.4→0.6.5.
  2. ★ Off-hot-path fix COMPLETE by construction — the reap was removed from the shared _wake_init_dir helper, so it closes the race for EVERY pre-lock caller at once (enqueue/consume/cursors/ack ×3), not just enqueue-vs-enqueue (the builders all-callers analysis showed "inside-the-lock" would be incomplete). Reap now runs ONLY as maintenance at store cmd_init + detector cmd_poll_once (bounded).
  3. ★ Age-scope safety (reproduced) — _wake_clean_stale_tmp uses find -mmin +${WAKE_TMP_STALE_MIN:-5} (non-numeric guard). Reviewer: a fresh ms-old .wake.tmp SURVIVES, a 10-min-aged orphan is REAPED. A sub-second atomic write can never match +5min → cannot clobber a live write.
  4. ★ Deterministic race RED→GREEN (reproduced) — the new test-wake-store-enqueue-race.sh (mv+flock PATH-shadow holds the window open): fix → GREEN (distinct gapless {1,2}, zero abort); mains PRE-FIX code under the same harness → RED ("durable pending write FAILED for seq 1", A aborts). Harness catches the bug; fix closes it.
  5. #908 seq-integrity intact — all 11 store-ack groups green incl T8 (sole allocator/anti-swallow), T9 (arrow-1 no-burn), T10 (concurrency); T5 rewrite preserves crash-safety core + now asserts no-hot-path-reap + age-scoped-maintenance (a strengthening).
  6. reconcile-retry — reconcile.sh byte-unchanged; R1-R9 green; the pilot-confirmed recovery model (structural+total retry, zero silent-loss) is confirmation-only in the body, no code delta.
  7. Gates — shellcheck 0.11 clean; verify-sanitized PASS; test:framework-shell rc=0 normal AND openssl-masked; firewall 0; manifest 0.6.5.

6-check — GO

  1. open/mergeable base main ✓ · 2. CI TERMINAL-GREEN @ full-40 a8af5ed7f09631352a2d6bac084c0fc45781ecdd ✓ · 3. reviewer(a3774b40)≠builder + MS-LEAD git-verified update, durable RoR ✓ · 4. reviewed==CI==merge-head all a8af5ed7 ✓ · 5. body Closes #927 + Part of #892 ✓ · 6. queue-guard at merge.
    GO for id-11 + squash-merge at FULL-40 a8af5ed7f09631352a2d6bac084c0fc45781ecdd. Closes #927 (the ELEVATED live-co-feed enqueue TOCTOU, fixed at root off the hot path). Non-executor.
# Record of Review — PR #928 (issue #927): enqueue TOCTOU fix (off-hot-path + age-scoped reap) — FINAL, CI-GREEN **VERDICT: APPROVE — GO.** **REVIEWED-HEAD (full-40):** `a8af5ed7f09631352a2d6bac084c0fc45781ecdd`. **CI: TERMINAL-GREEN** (pipeline 2075; validates the combined #926-T10-deflake + #928-T5 store-ack file). ## Review basis Independent reviewer `a3774b40` (opus, ≠ builder `mosaic-coder`) — **APPROVE, all 7 criteria PASS**, criteria 3/4 independently REPRODUCED: 1. **Scope/provenance** — head==`a8af5ed7…`; mergeable; id2 ≠ commit-author `mosaic-coder`; body `Closes #927` + `Part of #892`; net delta = #928s 7 files; **T10 UNCHANGED vs main** (#926s de-flake preserved; only T5 in the store-ack test); **reconcile.sh BYTE-UNCHANGED**; manifest 0.6.4→0.6.5. 2. **★ Off-hot-path fix COMPLETE by construction** — the reap was removed from the shared `_wake_init_dir` helper, so it closes the race for EVERY pre-lock caller at once (enqueue/consume/cursors/ack ×3), not just enqueue-vs-enqueue (the builders all-callers analysis showed "inside-the-lock" would be incomplete). Reap now runs ONLY as maintenance at store `cmd_init` + detector `cmd_poll_once` (bounded). 3. **★ Age-scope safety (reproduced)** — `_wake_clean_stale_tmp` uses `find -mmin +${WAKE_TMP_STALE_MIN:-5}` (non-numeric guard). Reviewer: a fresh ms-old `.wake.tmp` SURVIVES, a 10-min-aged orphan is REAPED. A sub-second atomic write can never match +5min → cannot clobber a live write. 4. **★ Deterministic race RED→GREEN (reproduced)** — the new `test-wake-store-enqueue-race.sh` (mv+flock PATH-shadow holds the window open): fix → GREEN (distinct gapless {1,2}, zero abort); mains PRE-FIX code under the same harness → RED ("durable pending write FAILED for seq 1", A aborts). Harness catches the bug; fix closes it. 5. **#908 seq-integrity intact** — all 11 store-ack groups green incl T8 (sole allocator/anti-swallow), T9 (arrow-1 no-burn), T10 (concurrency); T5 rewrite preserves crash-safety core + now asserts no-hot-path-reap + age-scoped-maintenance (a strengthening). 6. **reconcile-retry** — reconcile.sh byte-unchanged; R1-R9 green; the pilot-confirmed recovery model (structural+total retry, zero silent-loss) is confirmation-only in the body, no code delta. 7. **Gates** — shellcheck 0.11 clean; verify-sanitized PASS; test:framework-shell rc=0 normal AND openssl-masked; firewall 0; manifest 0.6.5. ## 6-check — GO 1. open/mergeable base main ✓ · 2. **CI TERMINAL-GREEN @ full-40 `a8af5ed7f09631352a2d6bac084c0fc45781ecdd`** ✓ · 3. reviewer(`a3774b40`)≠builder + MS-LEAD git-verified update, durable RoR ✓ · 4. reviewed==CI==merge-head all `a8af5ed7` ✓ · 5. body `Closes #927` + `Part of #892` ✓ · 6. queue-guard at merge. **GO for id-11 + squash-merge at FULL-40 `a8af5ed7f09631352a2d6bac084c0fc45781ecdd`. Closes #927 (the ELEVATED live-co-feed enqueue TOCTOU, fixed at root off the hot path). Non-executor.**
Ghost approved these changes 2026-07-26 10:56:39 +00:00
Ghost left a comment
Copy Link
Copy Source

GO — Gate-16 id-11 stamp, pinned to exact FULL head a8af5ed7f0.
Basis: verbatim FINAL RoR (comment 19191) — independent review APPROVE 7/7 with the load-bearing property reproduced: the off-hot-path fix is COMPLETE BY CONSTRUCTION (removing the reap from the shared _wake_init_dir helper closes the race for every pre-lock caller simultaneously — enqueue, consume, cursors, ack — the builder's caller-sweep having proven inside-the-lock would be incomplete); age-scoped maintenance verified safe both ways (fresh tmp survives, aged orphan reaped); the deterministic PATH-shadow race harness RED pre-fix, GREEN post-fix; #908 seq-integrity and the reconciler recovery model byte-untouched. Union-validated against post-#926 main in CI. Closes #927 — the elevated live-co-feed race fixed at root while its designed safety net (loud abort, structural retry, fallback bounds) covered the interim, exactly as calibrated. Part of #892. Named executor: Mos (id-11), squash pinned.

GO — Gate-16 id-11 stamp, pinned to exact FULL head a8af5ed7f09631352a2d6bac084c0fc45781ecdd. Basis: verbatim FINAL RoR (comment 19191) — independent review APPROVE 7/7 with the load-bearing property reproduced: the off-hot-path fix is COMPLETE BY CONSTRUCTION (removing the reap from the shared _wake_init_dir helper closes the race for every pre-lock caller simultaneously — enqueue, consume, cursors, ack — the builder's caller-sweep having proven inside-the-lock would be incomplete); age-scoped maintenance verified safe both ways (fresh tmp survives, aged orphan reaped); the deterministic PATH-shadow race harness RED pre-fix, GREEN post-fix; #908 seq-integrity and the reconciler recovery model byte-untouched. Union-validated against post-#926 main in CI. Closes #927 — the elevated live-co-feed race fixed at root while its designed safety net (loud abort, structural retry, fallback bounds) covered the interim, exactly as calibrated. Part of #892. Named executor: Mos (id-11), squash pinned.
Ghost merged commit 13e6ce5e5c into main 2026-07-26 10:56:41 +00:00
Ghost deleted branch fix/wake-927-enqueue-toctou 2026-07-26 10:56:41 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Ghost
Labels
Clear labels
bug
Defect in framework tooling or delivered code
 fleet-enhancement
Fleet-wide reliability/speed/cost improvements surfaced by the nightly enhance pass
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
code-be-01 (Mosaic fleet seat code-be-01) code-be-02 (Mosaic fleet seat code-be-02) code-dogfood-01 (Mosaic fleet seat code-dogfood-01) code-infra-01 (Mosaic fleet seat code-infra-01) fargo fred gate-merge-01 (Mosaic fleet seat gate-merge-01) happy jason.woltje (Jason Woltje) marcie merge-gate ops-01 (Mosaic fleet seat ops-01) ops-02 (Mosaic fleet seat ops-02) ops-03 (Mosaic fleet seat ops-03) ops-ci-01 (Mosaic fleet seat ops-ci-01) ops-deploy-01 (Mosaic fleet seat ops-deploy-01) orch-01 (Mosaic fleet seat orch-01) pepper resume rev-code-01 rev-code-02 rev-security-01 rev-security-02 rev-security-03 (Mosaic fleet seat rev-security-03) sanity scooby (Scooby) scrappy shaggy tiny topher (Mosaic fleet seat topher) velma veronica (Mosaic fleet seat veronica) vision woodpecker
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mosaicstack/stack#928