fix(task-graph): route RunPrivateCacheRepo by-ref through *ForRun to prevent cross-run leak (follow-up to #611)

[sroussey]

Summary

RunPrivateCacheRepo wraps a TaskOutputRepository so entries are namespaced by runId, but its by-ref forwarders passed CacheRefs straight to the backing's runId-agnostic getOutputByRef / getOutputStreamByRef / deleteOutputByRef. Any wrapper on the same backing could therefore resolve — and delete — another run's blob just by receiving its CacheRef. The private tier's whole point is per-run isolation; this made runs observable and mutable across the boundary.

The fix pushes enforcement into the backing: three optional *ForRun by-ref methods on the base interface, and an FsFolder implementation that rejects any ref whose blob name does not start with the sanitized runScopePrefix(runId). The wrapper now routes exclusively through those variants, threading its own runId; the unscoped surface is never called from the wrapper again.

Failure scenario

Two RunPrivateCacheRepo wrappers (run-A, run-B) share one FsFolderTaskOutputRepository. run-B writes a streamed port and gets refB. Before this change, wrapperA.getOutputStreamByRef(refB) returned run-B's bytes, wrapperA.getOutputByRef(refB) returned run-B's blob, and wrapperA.deleteOutputByRef(refB) unlinked it — a cross-run read/write channel via any leaked ref.

Approach

• Add three optional *ForRun by-ref methods to TaskOutputRepository: getOutputByRefForRun, getOutputStreamByRefForRun, deleteOutputByRefForRun. Each carries a JSDoc contract that a foreign ref MUST resolve to undefined (readers) or be a no-op (delete), matching the base by-ref idempotency contract.
• Implement them on FsFolderTaskOutputRepository. A shared blobPathInRunScope(ref, runId) helper parses the ref through the existing REF_PATTERN and requires the captured blob name to start with sanitize(runScopePrefix(runId)). On a mismatch the readers return undefined and delete never touches disk.
• Rewire RunPrivateCacheRepo's constructor by-ref forwarders to route only through the *ForRun variants, gated on the existing canStreamForRun || canStreamPortForRun capability. Delete the forwards to backing.getOutputByRef / .getOutputStreamByRef / .deleteOutputByRef entirely. Backings without the *ForRun ref methods leave the wrapper's method undefined (private tier degrades gracefully to accumulation for reads).

Test coverage

New describe("run-scope enforcement", ...) block in RunPrivateFsFolderStream.test.ts:

1. Negative — cross-run stream/blob read. wrapperA.getOutputStreamByRef(refB) and wrapperA.getOutputByRef(refB) both resolve to undefined.
2. Negative — cross-run delete. wrapperA.deleteOutputByRef(refB) resolves without throwing; wrapperB can still read the blob and the blob count is unchanged. Case carries a comment justifying the silent no-op: matches the base contract's best-effort idempotency; alarms belong in operator observability, not the security-invariant path.
3. Positive regression. wrapperA reads and deletes its own refA through all three methods; delete prunes exactly one blob.
4. Edge — malformed / foreign-scheme refs. makeCacheRef({ $ref: "fsfolder://blobs/foreign.bin" }) (missing run-scope prefix) and makeCacheRef({ $ref: "other://something" }) are both uniformly rejected: readers undefined, delete silent no-op.

Strengthened clearRun() assertion: after repoA.clearRun(), repoB.getOutputStreamByRef(refA) also resolves to undefined.

Sequencing

Three commits, in order:

1. feat(task-graph): add optional *ForRun ref methods to TaskOutputRepository — interface-only. No behavior change; every existing implementer compiles unchanged.
2. feat(task-graph): implement runId-scoped ref readers on FsFolder — the three *ForRun methods with the sanitized-prefix check. Existing tests still pass; wrapper still uses the unscoped path so the fix is not yet observable.
3. fix(task-graph): route RunPrivateCacheRepo by-ref through *ForRun to prevent cross-run leak — wrapper rewire + drop the unscoped forwarders + four new tests + strengthened clearRun assertion. The vulnerability closes here.

Verification

• bun run build:types — clean.
• bun scripts/test.ts graph vitest — 1033 passed / 1033 total (up from 1029 pre-change; four new tests added, no existing test weakened).
• grep -rn 'fsfolder://' packages --include='*.ts' — the only hand-constructed fsfolder:// refs outside the sanctioned writer paths in FsFolderTaskOutputRepository.ts are in test files (pre-existing edge-case tests plus the new negative-edge test added here).

Base is PR #611's head (claude/task-graph-streaming-phase3-yg5msj), not main, because #611 is not yet merged.

---

Generated by Claude Code

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	63.34%	26583 / 41967
🔵	Statements	63.17%	27499 / 43528
🔵	Functions	64.64%	5033 / 7786
🔵	Branches	52.32%	13167 / 25164

File Coverage

No changed files found.

Generated in workflow #2650 for commit 17352a0 by the Vitest Coverage Report Action