Skip to main content
← Back to list
01Issue
BugShippedSwamp CLI
Assigneesstack72

#249 Pre-existing TOCTOU windows in YAML repo walkers (findAll directory level + findById)

Opened by stack72 · 5/5/2026· Shipped 5/5/2026

Description

Two pre-existing TOCTOU windows in the YAML repository walkers surfaced during adversarial review of systeminit/swamp#1307 (followup to swamp-club#240). They were noted as out-of-scope for that PR but worth tracking for a future cleanup. Both can mis-report results under concurrent operations (GC, bulk delete, parallel CLI invocations).

Issue 1 — Directory-level TOCTOU in YamlOutputRepository.findAll

File: src/infrastructure/persistence/yaml_output_repository.ts:118 (the findAll(type) method).

If a method directory is deleted between Deno.readDir(typeDir) and Deno.readDir(methodDir), the inner readDir throws NotFound. PR #1307 fixed the per-file TOCTOU but the directory-level race is one level higher: the NotFound escapes to the outer catch (line 142) which returns [], discarding every output already collected for the type from earlier method directories.

The per-file pattern this PR established doesn't help here because the loss happens at the directory boundary, not the file boundary. The fix is structurally analogous: wrap Deno.readDir(methodDir) (or the entire inner method-directory body) in its own try/catch that continues on NotFound.

Directory deletion is rarer than file deletion — it only happens via bulk ops (deleteAllByWorkflowId for run dirs; output cleanup for method dirs) — so this is lower-priority than the file-level case.

Issue 2 — findById aborts on a non-target file's deletion in both YAML repos

Files:

  • src/infrastructure/persistence/yaml_output_repository.ts:65 (YamlOutputRepository.findById)
  • src/infrastructure/persistence/yaml_workflow_run_repository.ts:71 (YamlWorkflowRunRepository.findById)
  • The same shape exists in YamlOutputRepository.delete (yaml_output_repository.ts:195).

Both findById methods iterate the directory and call Deno.readTextFile per entry, looking for a matching id field. The readTextFile is unguarded inside an outer try/catch. If a non-target file is deleted between readDir and readTextFile, the NotFound propagates to the outer catch, the iteration aborts, and the method returns null even when the actual target exists later in the directory.

Failure mode is benign — a false "not found" — and the race window is much narrower than the multi-file walkers PR #1307 fixed (single lookup, returns on first match). But it can manifest as flaky-looking "row not found" errors during high-traffic operations.

Fix is the same per-file try/catch pattern PR #1307 used on the multi-file walkers.

Steps to Reproduce

Both are race conditions, so deterministic reproduction needs an injection point. The patterns to demonstrate them in tests:

Issue 1 (directory-level findAll):

// Seed two method dirs under the same type, each with one output.
// Delete methodDir B between readDir(typeDir) and readDir(methodDirB).
// Expected: returns A's output. Actual (current): returns [].

Issue 2 (findById on non-target deletion):

// Seed two outputs for (type, method): A (target) and B (non-target).
// If iteration order returns B before A, delete B's file between readDir
// and readTextFile(B). Expected: returns A. Actual (current): returns null.

PR #1307 already establishes the test pattern for "file deleted mid-iteration" against findAllGlobalSince, findAllByWorkflowId, findAll, and the output findAllGlobalSince walkers — those tests can be adapted to cover findById and the directory-level case.

Environment

  • swamp version: post-#1307 (currently on main)
  • Affected files:
    • src/infrastructure/persistence/yaml_output_repository.ts
    • src/infrastructure/persistence/yaml_workflow_run_repository.ts
  • Discovered during: adversarial review of swamp#1307 followup to swamp-club#240
  • Related: in-tree precedent for the per-file try/catch pattern is at src/infrastructure/persistence/unified_data_repository.ts:273 (data repo) and now in the four walkers fixed by #1307
02Bog Flow
OPENTRIAGEDIN PROGRESSSHIPPED+ 1 MOREASSIGNED+ 5 MOREREVIEW+ 3 MOREPR_MERGEDSHIPPED

Shipped

5/5/2026, 8:34:57 PM

Click a lifecycle step above to view its details.

03Sludge Pulse
stack72 assigned stack725/5/2026, 5:48:45 PM

Sign in to post a ripple.