diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md index 5cc0773..b1489cd 100644 --- a/docs/roadmap/audit.md +++ b/docs/roadmap/audit.md @@ -1,6 +1,6 @@ # Repository Audit Ledger -Status: In progress; Stages 1-13 complete. +Status: In progress; Stages 1-14 complete. This temporary roadmap document is the evidence ledger for the staged audit defined by [the audit plan](audit-plan.md). It records audit evidence and @@ -97,8 +97,15 @@ found one low-severity missing-value defect because Today can emit an empty daypart heading when every row lacks displayable conditions. Typed module projection, stable daypart ordering, conditional deterministic sections, template/partial lookup, actionable execution errors, and repeated-render -determinism otherwise match their contracts. Subsystem conclusions and final -disposition remain pending the later stages. +determinism otherwise match their contracts. Stage 14 found two medium-severity +application-boundary defects: generated-text catalog compatibility is checked +only after weather collection and report construction, and execution proceeds +without reconciling inspected, prepared, callback, completed-execution, and +schema-validation provenance. Side-effect-free prompt/profile inspection, +credential preflight, immutable prepared values, one-build comparison reuse, +per-execution copies, typed generated-text validation, debug failure handling, +and partial execution results otherwise match their contracts. Subsystem +conclusions and final disposition remain pending the later stages. ## Baseline Metadata @@ -272,7 +279,7 @@ with evidence about meaningful risks, test ownership, gaps, and duplication. | Promptkit boundary and sensitive debug output | `internal/adapters/promptkit`, `internal/promptdebug` | 11 | Partly insufficient. Exact prompt/profile inspection, source precedence, inline execution, credential checks, cancellation, safe classified errors, callback ordering, debug opt-in, static symlink rejection, restrictive modes, atomic replacement, and distinct concurrent captures have focused owners. `AUD-034` and `AUD-035` record incomplete credential redaction and a check/use symlink race. | | Generated-text validation | `internal/generatedtext` | 12 | Partly insufficient. The four current registry definitions dispatch to typed validators with coherent required-field and normalization behavior, and schema bytes are independently copied. `AUD-036` through `AUD-039` record report-independent catalog matching, case-insensitive Go field aliases, unbounded prose/diagnostics, and duplicated or misplaced contract tests. | | Render contexts and templates | `internal/generatedtext`, `internal/reporttemplate` | 13 | Partly insufficient. Typed module extraction, deterministic daypart ordering, report-specific sections, partial conditions, template lookup, and semantic output assertions are coherent. `AUD-040` through `AUD-042` record conflicting report identities, structural Markdown injection, and Today's empty daypart section. | -| Prompt preflight and prepared inputs | `internal/app` | 14 | Pending | +| Prompt preflight and prepared inputs | `internal/app` | 14 | Partly insufficient. Exact prompt input/output shape, profile selection, credentials, immutable preparation, per-execution copies, typed output validation, rendering, callback errors, and partial execution outcomes have focused owners. `AUD-043` and `AUD-044` record late generated-text catalog validation and missing end-to-end execution-provenance reconciliation. | | Single-report publication and preservation | `internal/app`, `internal/fileutil` | 15 | Pending | | Batch partial success and notification | `internal/app`, `internal/adapters/distributor` | 16 | Pending | | Comparison identity and transactional publication | `internal/comparison` | 17 | Pending | @@ -1476,10 +1483,12 @@ with evidence about meaningful risks, test ownership, gaps, and duplication. application-level no-provider-work consequence belongs to Stage 14. - Test implications: Protect the repository context and error category, not the complete encoding-library message; do not duplicate every unsupported - Go value shape. + Go value shape. Stage 14 confirmed that application tests also do not inject + this failure and prove zero executor calls, so add that single consequence + assertion at the preparation owner rather than repeating serializer cases. - Validation: The focused test fails if conversion errors lose stanza context or are swallowed, and deterministic successful serialization remains green. -- Related findings: none +- Related findings: `AUD-043` - Remediation reference: pending ### AUD-034: Prompt-debug credential redaction misses common aliases and URL forms @@ -1841,6 +1850,97 @@ with evidence about meaningful risks, test ownership, gaps, and duplication. - Related findings: none - Remediation reference: pending +### AUD-043: Generated-text compatibility is validated after collection + +- Stage: 14 +- Status: candidate +- Severity: medium +- Confidence: high +- Category: correctness +- Area: `internal/app.InspectPromptExecution`, + `InspectPromptExecutions`, `InspectComparisonExecution`, and `prepareReport` +- Evidence: Prompt preflight validates the exact prompt ID/version, its one + YAML input, the configured output schema path, the effective profile, and + credentials before weather collection. It never validates the report's + generated-text template or complete catalog binding. `prepareReport` calls + `generatedtext.LookupDefinition` only after collection and after building + report facts, the module snapshot, metadata, the prompt package, and its + serialized YAML. A temporary offline probe changed a copied Daily definition + to an unknown template while retaining its valid prompt and schema; + `InspectPromptExecution` succeeded. The same definition fails catalog lookup + only during preparation. Current default registry entries are valid. +- Contract at risk: Every configuration-independent report, schema, template, + validator, and renderer precondition must fail before external collection or + model execution, leaving collected-data validation and serialization for the + post-collection preparation boundary. +- Impact: A registry regression or constructed definition with an unknown or + mismatched template can pass advertised preflight, perform all remote weather + requests, and build the complete prompt package before failing without a + model call. This wastes externally visible work and weakens the promise that + report-contract defects are configuration preflight failures. +- Recommendation: Resolve and validate the exact report/schema/template + catalog entry as part of pre-collection inspection or report resolution, + after strengthening lookup to enforce the report-specific triple in + `AUD-036`. Carry the validated immutable handler into preparation so the + post-collection path does not perform a weaker or duplicate lookup. +- Test implications: At the application owner, inject an invalid template and + known-but-wrong report pair and assert no collector or executor call. Keep + catalog pairing cases in `internal/generatedtext`; the app test should + protect only ordering and consequences. +- Validation: All invalid report/catalog bindings fail before collection in + ordinary, batch, and comparison preflight, while each valid definition is + resolved once and no model execution occurs on later preparation failures. +- Related findings: `AUD-036` +- Remediation reference: pending + +### AUD-044: Execution provenance is accepted without reconciliation + +- Stage: 14 +- Status: candidate +- Severity: medium +- Confidence: high +- Category: correctness +- Area: `internal/app.inspectPromptContract`, `inspectPromptProfile`, and + `executePreparedProfile` +- Evidence: Inspection verifies returned prompt and profile IDs, input shape, + output shape, and credentials, but accepts an empty prompt hash, backend ID, + and model name. Execution sends the inspected prompt/profile IDs and an + independent data-package copy, then trusts the preparation callback to + replace result profile/backend/model identity. It does not compare the + prepared report definition with the inspection, require the callback, or + reconcile callback and completed `Execution` prompt ID/version/hash, + profile/backend/model, output contract, validation mode, or schema path. A + temporary executor probe returned mutually different inspected, callback, + and execution identities plus `ValidationPassed` for an unrelated mode and + schema path; `executePreparedProfile` accepted the raw Daily-shaped JSON and + rendered Markdown successfully. Existing fakes and the current Promptkit + adapter return coherent provenance, so focused tests never exercise a + mismatch. +- Contract at risk: A validation-passed output must be demonstrably produced + by the exact inspected prompt version and selected profile, against the + prepared report's exact schema, before repository-owned typed validation and + rendering can attribute it to that report. +- Impact: An adapter regression, stale inspection passed by an internal caller, + or alternate executor can render and later publish output from a different + prompt, profile, backend, model, or schema while the active result records + callback-supplied identity. Similar output shapes can make the error silent + rather than causing typed validation to fail. +- Recommendation: Define one application-owned provenance validator. Require + complete safe inspection identity, match it to the prepared definition and + request, count exactly one preparation callback, reconcile callback and + completed execution identity/hash fields, and require the expected JSON + Schema validation mode and path before accepting `ValidationPassed`. +- Test implications: Add one table-driven execution-contract suite that + changes one provenance field or omits the callback, plus one coherent happy + path. Assert mismatch errors are classified and content-safe, produce no + rendered bytes, and retain only verified partial-result fields. +- Validation: Every provenance mismatch fails before generated-text decoding + or rendering; coherent Promptkit and fake executions retain the exact + inspected prompt hash and selected profile/backend/model in results and + debug artifacts. +- Related findings: `AUD-036`, `AUD-037`, `AUD-040` +- Remediation reference: pending + ## Retained Decisions ### RET-001: Keep the application package as the explicit composition owner @@ -2178,6 +2278,31 @@ report variation and Markdown meaning with less incidental whitespace coupling than full-output goldens; retain it while adding narrow regression cases for `AUD-041` and `AUD-042`. +### RET-029: Keep one immutable prepared report as the execution boundary + +`preparedReport` is the right shared boundary between collected weather and +profile execution. Preparation constructs facts, the ordered module snapshot, +briefing metadata, curated YAML, and the generated-text handler once. It deep- +copies mutable facts, snapshots, metadata, warnings, and YAML bytes; every +executor receives another byte copy, and every renderer receives newly cloned +typed inputs. Focused mutation tests protect caller and consumer isolation, +while concurrent comparison tests prove that one preparation can feed ordered +profile executions with identical bytes. Retain this explicit value boundary +rather than rebuilding report facts per profile or sharing mutable render +structures; complete catalog validation earlier as required by `AUD-043`. + +### RET-030: Keep side-effect-free prompt and profile inspection before collection + +Application preflight asks the neutral executor for one exact prompt contract +and the effective safe profile identity before collecting weather. Batch +inspection caches a shared effective profile, comparison preserves requested +profile order and a successfully inspected prefix, credential checks expose no +secret value, and inspection errors are classified without copying dependency +diagnostics into normal text. This is the appropriate owner and stage for +remote-work prerequisites. Retain the inspection seam while expanding it to +cover catalog compatibility and complete, later-reconciled provenance under +`AUD-043` and `AUD-044`. + ## Open Questions No Stage 1 open questions or unexplained baseline failures remain. @@ -2402,6 +2527,25 @@ Stage 13 routed these investigation leads to their assigned later stages: large fixture surface and repeated report assertions across the whole test portfolio rather than Stage 13 rewriting unrelated cases. +Stage 14 routed these investigation leads to their assigned later stages: + +- `executePreparedProfile` deliberately performs no durable write and ordinary + generation checks context only after rendering, immediately before atomic + publication. Stage 15 owns the final cancellation, destination-preservation, + and partial-result truthfulness consequences; Stage 18 owns comparison-wide + cancellation aggregation after concurrently started executions. +- `cloneResolved` copies the current slice-bearing AFD option explicitly while + all other accepted module options are value-only. This is safe for the + present registry, but the manual type switch can miss a future reference- + bearing option. Stage 23 should assess an ownership-preserving clone contract + if module option shapes expand rather than introducing reflection during this + audit stage. +- Preparation clones typed values through JSON, so impossible-under-current- + shapes clone failures remain wrapped but are not independently injectable. + Stages 20-21 should decide whether existing successful mutation isolation and + the actionable wrapper are sufficient when consolidating application + fixtures; `AUD-033` separately owns the reachable stanza serialization gap. + ## Stage Log ### Stage 1: Establish The Baseline And Audit Ledger @@ -3087,3 +3231,65 @@ Stage 13 routed these investigation leads to their assigned later stages: - Retained decisions: `RET-027` and `RET-028`. - Open questions: the three leads recorded above are routed to their assigned later stages. + +### Stage 14: Audit Application Preparation And Prompt Preflight + +- Status: Complete. +- Scope reviewed: all production code and focused tests in + `internal/app/prepared_report.go`, `prompt_inspection.go`, + `prompt_generate.go`, and `profile_execution.go`; the narrow ordinary, + batch, and comparison callers needed to establish collection order, + preparation count, cancellation, and partial-result consequences; and the + prepared-report and application-orchestration internal contracts. +- Exclusions: Single-report destination resolution and atomic publication + remain Stage 15; batch continuation and notification remain Stage 16; + comparison identity/publication and scheduling remain Stages 17-18; + portfolio-wide test consolidation and application refactoring remain Stages + 20-23; documentation-wide reconciliation remains Stage 24; and broader + adversarial execution remains Stage 25. + +#### Preparation And Execution Accounting + +| Contract or risk | Ordering, copy, execution, and test evidence | Disposition | +| --- | --- | --- | +| Pre-collection validation | Ordinary and comparison flows resolve report and destination before optional debug initialization, then inspect prompt/profile and credentials before `collectWeather`; batch inspects all needed prompt/profile pairs before its one collection. Exact prompt ID/version, one required YAML input, JSON Schema output path, selected profile ID, and environment credential availability fail before collection. | The generated-text template/catalog binding is absent from this preflight and is checked late; `AUD-043`. Output publication preflight remains with the assigned later stages. | +| Prompt/profile inspection | Inspection is side-effect-free, caches one effective batch profile, preserves explicit comparison order, and returns a successfully inspected comparison prefix on profile failure. Unclassified dependency failures become content-safe configuration errors. | Coherent and retained as `RET-030`, except that safe provenance can be incomplete and is not later reconciled; `AUD-044`. | +| Post-collection preparation | Preparation requires a bundle, builds facts once, builds one ordered module snapshot and metadata value, constructs and deterministically serializes the curated package, resolves a generated-text handler, and wraps every failure with its operation. No model or normal publication occurs on a preparation error. | The construction boundary is explicit. Catalog lookup order is `AUD-043`; missing serialization/no-executor regression remains `AUD-033`. | +| Immutable prepared state | Facts, snapshots, metadata, source warnings, resolved slice-bearing fields, and YAML bytes are copied before retention. Executor requests receive byte copies and rendering receives fresh typed clones. Focused tests mutate the source bundle and returned render values; concurrent comparison tests observe equal package bytes for every profile. | Demonstrably isolated for current types and retained as `RET-029`. The manual future option-shape risk is routed to Stage 23, not treated as a current alias defect. | +| Single-build reuse | Ordinary generation prepares once for its one selected profile. Comparison collects and prepares once, then passes the same immutable logical package through independent copies to every profile; it does not recompute facts or modules per provider. | Matches the prepared-report contract and avoids cross-profile derivation drift. | +| Executor request and cancellation | Execution receives the inspected prompt ID/version, selected profile ID, caller context, independent YAML bytes, and debug capture only when explicitly enabled. Classified provider errors retain their category; otherwise they become generation errors. The reusable executor/render core performs no durable writes. | Request shape and context propagation are coherent. Missing provenance checks are `AUD-044`; post-render publication cancellation and comparison aggregation remain Stages 15 and 18. | +| Debug callbacks | Preparation debug is written only through the explicit writer. A requested callback write failure is marked local and returned directly, preventing provider work under the executor contract; execution-debug failures stop before rendering/publication. Normal results retain only safe identities and an artifact path. | Matches Stage 11's secure-debug ownership. Callback invocation and identity are nevertheless trusted rather than verified; part of `AUD-044`. | +| Generated text and rendering | Only completed `passed` validation proceeds; `failed` becomes validation rejection and incomplete states become operational validation errors. The prepared handler independently validates raw JSON, clones render inputs, builds a typed context, and renders in memory. | Correct layer order under coherent provenance. Schema/mode and execution identity are not tied to the inspected contract; `AUD-044`. Existing schema, size, context, and Markdown findings remain `AUD-036` through `AUD-042`. | +| Errors and partial results | Preparation and execution wrappers preserve actionable operation names. Ordinary generation copies source warnings after preparation and records resolved profile identity, validation status, and debug path before returning execution errors. Comparison outcomes independently retain ordered safe execution status. | Coherent for reachable callers. Durable-path truthfulness and aggregation policy remain explicitly assigned to Stages 15-18. | +| Complexity and test ownership | The preparation path is a guarded straight-line pipeline over service-sized typed values; its dominant cost is deliberate deep copy and serialization. Execution is another guarded pipeline with no loop. Focused suites own inspection, mutation isolation, debug callbacks, validation states, rendering without publication, and concurrent shared-package reuse. | No Stage 14 performance finding. Missing invalid-catalog ordering, provenance mismatch, and serialization consequence tests attach directly to `AUD-043`, `AUD-044`, and `AUD-033`. | + +#### Commands And Evidence + +- Used graph symbol and augmented-code search, exact source snippets, + inbound/data-flow traces, and scoped complexity metadata for preparation, + inspection, ordinary generation, profile execution, collection, and + comparison reuse. Bounded text inspection covered the four scoped production + files, focused tests, task-specific internal documents, audit plan, and all + repository policies. +- Enumerated every pre-collection failure (report/output/debug setup, executor, + exact prompt shape, profile selection, and credentials), post-collection + preparation failure (bundle, facts, modules, package construction, + serialization, catalog, and immutable copies), and post-model boundary + (completed schema status, typed validation, context construction, rendering, + debug callback, cancellation handoff, and partial outcome). +- Ran temporary offline probes proving that prompt preflight accepts an unknown + generated-text template plus blank safe provenance, and that mutually + inconsistent callback/execution identities and schema-validation provenance + still render valid Daily-shaped output. The probes were removed without + retaining source or test changes and contacted no provider. +- Ran + `go test -race -coverprofile=/tmp/weatherreporter-stage14-cover.out ./internal/app`; + the focused package passed with 76.0% statement coverage. Coverage guided + missing-branch inspection but was not itself treated as a finding. Ran + `go test ./...`, `go vet ./...`, `go run ./cmd/weatherreporter --help`, and + `git diff --check`; all passed. +- Findings: `AUD-043` and `AUD-044`; application consequences also extend the + existing test finding `AUD-033`. +- Retained decisions: `RET-029` and `RET-030`. +- Open questions: the three leads recorded above are routed to their assigned + later stages.