From e2b82746ab7ac5e4af5433d6835422aa1caba591 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 8 Aug 2026 21:16:30 +0000 Subject: [PATCH] Audit reference materialization and ordered handoffs --- docs/roadmap/audit.md | 172 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 168 insertions(+), 4 deletions(-) diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md index f74c669..43c5397 100644 --- a/docs/roadmap/audit.md +++ b/docs/roadmap/audit.md @@ -19,8 +19,8 @@ change only roadmap audit documents do not change that production target. ## Executive Summary Pending final synthesis. The initial baseline is healthy. The architecture, -configuration/CLI, and pipeline composition reviews have found one Medium -correctness finding and four Low findings, with no production dependency +configuration/CLI, pipeline composition, and reference/handoff reviews have +found two Medium findings and seven Low findings, with no production dependency inversion or unsafe typed-erasure boundary. ## Finding Index @@ -34,6 +34,9 @@ Final cross-area ordering is pending synthesis. | CFGCLI-002 | Low | Correctness | Reject a blank command-level LLM profile | | PIPE-001 | Low | Correctness | Reject normalized module-reference collisions in the resolver | | PIPE-002 | Low | Efficiency | Clone construction inputs once per builder boundary | +| REF-001 | Medium | Efficiency | Bound reference reads before allocating the file | +| REF-002 | Low | Efficiency | Index accepted outputs once per ordered handoff | +| REF-003 | Low | Correctness | Include canonical size in generated-reference fingerprints | ## Findings @@ -192,6 +195,103 @@ Final cross-area ordering is pending synthesis. `go test ./internal/framework/contracts ./internal/framework/pipeline`. - **Grouping:** Independent. +### References And Ordered Handoffs + +### REF-001 — Bound reference reads before allocating the file + +- **Severity:** Medium +- **Category:** Efficiency +- **Evidence:** `internal/framework/pipeline/references.go:88`–`164` implements + the external-reference materialization boundary. At lines 128–146, + `materializeReferenceTarget` calls `os.ReadFile(path)` before comparing the + resulting allocation with `ReferenceSlot.MaxBytes`. The focused + `TestMaterializeReferencesEnforcesMaxBytes` case uses a nine-byte file and + verifies the post-read diagnostic, but does not prove that reads are bounded + by the declared three-byte limit. +- **Impact:** A mistakenly selected very large file, growing file, device, or + named pipe can consume memory far beyond the slot's advertised bound before + the framework rejects it. CLI reference overrides expose the same path, so a + local operator error can terminate the process instead of producing the + intended bounded validation failure. +- **Recommendation:** Open the path and read through a limit of + `MaxBytes + 1` when a positive maximum is declared, rejecting an extra byte + before retaining or cloning content. A regular-file size precheck may improve + diagnostics, but the bounded reader must remain authoritative for changing or + non-regular inputs. Preserve the existing unbounded behavior only for slots + that explicitly declare no maximum. +- **Preserve:** Keep config-relative versus working-directory-relative path + resolution, UTF-8 and media-type validation, empty-file warnings, canonical + digest/size/origin metadata, contextual errors without content, and owned + reference bytes. +- **Validation:** Add a reader or file fixture that proves no more than + `MaxBytes + 1` bytes are consumed, including a non-regular or growing-input + case, while retaining the current UTF-8, media, empty, and ordinary oversize + diagnostics; run `go test ./internal/framework/pipeline ./internal/cli`. +- **Grouping:** Independent. + +### REF-002 — Index accepted outputs once per ordered handoff + +- **Severity:** Low +- **Category:** Efficiency +- **Evidence:** `buildStepReferenceSets` walks every generated binding in the + receiving step (`internal/framework/pipeline/handoff.go:37`–`89`). For each + previously unseen producer, `generatedReferenceItem` allocates a `matches` + slice and scans the complete cumulative `outputs` slice to find that + step/lane (`handoff.go:104`–`133`). Its cache avoids rescanning when several + targets fan out from the same producer, but a step consuming `P` distinct + producers from `O` earlier outputs still performs `P * O` comparisons and up + to `P` temporary allocations. +- **Impact:** Ordered pipelines with many distinct generated dependencies pay + quadratic handoff preparation work before the consumer step can start. The + current production profiles are small and the scan is outside the lane + worker hot path, which limits present impact. +- **Recommendation:** Build one map from normalized `(step ID, lane ID)` to an + explicit zero/one/many accepted-output state at the start of + `buildStepReferenceSets`, then let `generatedReferenceItem` perform a direct + lookup. Keep ambiguity as data in the index so duplicate producer outputs are + still rejected rather than overwritten. +- **Preserve:** Retain exact accepted-output cardinality, codec + decode/re-encode canonicalization, producer lookup, complete schema/media + validation, per-target byte ownership, deterministic contextual errors, and + the rule that no consumer lane starts after a failed handoff. +- **Validation:** Add a many-producer/fanout case that retains missing and + duplicate rejection, then use a focused benchmark or comparison counter to + demonstrate one output-index pass plus direct producer lookups; run + `go test ./internal/framework/pipeline`. +- **Grouping:** Independent; do not combine with PIPE-002, which concerns + construction-request copying rather than runtime producer lookup. + +### REF-003 — Include canonical size in generated-reference fingerprints + +- **Severity:** Low +- **Category:** Correctness +- **Evidence:** The generated `ReferenceItem` records canonical content length + in `SizeBytes` (`internal/framework/pipeline/handoff.go:154`–`177`), and the + manifest provenance also retains that value at lines 210–232. However, + `generatedReferenceFingerprintIdentity` and + `generatedReferenceDependencies` (`handoff.go:234`–`287`) hash producer, + kind, complete schema identity, media type, and content digest without the + canonical size. This differs from the documented resume contract in + `docs/internal/state.md:49`–`54`. The focused fingerprint test changes only + canonical content and does not assert size participation. +- **Impact:** Consumer checkpoint identity does not cover one field that the + handoff and manifest declare part of canonical reference identity. Current + construction derives size directly from canonical bytes, so a practical + stale reuse also requires malformed internal metadata, a digest collision, + or a future producer-path change; the immediate risk is therefore low. +- **Recommendation:** Add `SizeBytes` to the private fingerprint identity and + populate it from the canonical generated item before JSON hashing. Keep the + existing normalized fingerprint name and all current identity fields. +- **Preserve:** Do not weaken the content digest, producer provenance, + artifact kind, complete schema digest/fields, media type, or canonical codec + checks. Continue to keep content bytes out of checkpoints, manifests, debug + summaries, and errors. +- **Validation:** Add a direct dependency-fingerprint case that holds the + other identity fields constant while changing size metadata, retain the + canonical-content sensitivity case, and run + `go test ./internal/framework/pipeline ./internal/modules/integration/...`. +- **Grouping:** Independent. +