From 880ad710ae7a0ba14814bef8716d286645ef3154 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 12 Aug 2026 16:11:55 +0000 Subject: [PATCH] Record Stage 10 prompt boundary audit --- docs/roadmap/audit.md | 301 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 297 insertions(+), 4 deletions(-) diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md index 10540ef..490f83e 100644 --- a/docs/roadmap/audit.md +++ b/docs/roadmap/audit.md @@ -1,6 +1,6 @@ # Repository Audit Ledger -Status: In progress; Stages 1-9 complete. +Status: In progress; Stages 1-10 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 @@ -65,7 +65,17 @@ positive zero-degree bands, and embedded descriptions labeled as official SPC definitions have drifted from the current SPC category table. Derived precipitation timing, deterministic display ordering, missing-data behavior, SPC overlap and threshold selection, and the intended Today versus -Daily/Tomorrow planning variation otherwise match their contracts. Subsystem +Daily/Tomorrow planning variation otherwise match their contracts. Stage 10 +found two high-severity prompt-contract defects: the shared prompt names +nonexistent paths for primary derived facts and alert metadata, and it +incorrectly characterizes every retained SPC product as location-matched. It +also found three low-severity boundary and contract gaps: raw source-warning +types and transport paths cross into provider input, the exported YAML loader +accepts several malformed package shapes, and stanza serialization failures +lack a focused regression test. Prompt/profile/schema identities, +deterministic category ordering, neutral execution lifecycle and error +categories, safe bounded errors, debug opt-in, and output-copy isolation +otherwise match their contracts. Subsystem conclusions and final disposition remain pending the later stages. ## Baseline Metadata @@ -202,7 +212,7 @@ inventory commands, graph index refresh, and graph architecture inspection. | 7 | Audit forecast and fact derivation | Complete | | 8 | Audit module contracts, registry, and source-facing briefing modules | Complete | | 9 | Audit derived, planning, formatting, and SPC briefing modules | Complete | -| 10 | Audit prompt inputs, assets, and neutral execution contracts | Pending | +| 10 | Audit prompt inputs, assets, and neutral execution contracts | Complete | | 11 | Audit Promptkit adaptation and secure prompt debugging | Pending | | 12 | Audit generated-text validation and catalog contracts | Pending | | 13 | Audit render contexts, templates, and Markdown rendering | Pending | @@ -236,7 +246,7 @@ with evidence about meaningful risks, test ownership, gaps, and duplication. | Weather transport and normalized collection | `internal/adapters/weatherapi`, `internal/collect`, `internal/weatherdata` | 6 | Partly insufficient. All eight source requests, required/optional policy, checked-empty products, retryable source statuses, cancellation, normalized provenance, and collection error ownership have focused offline coverage. `AUD-011` through `AUD-015` record unsupported schemes, unsafe response-body diagnostics, an unenforced body-size limit, structurally invalid required hourly periods, and overbroad warmup retries. | | Forecast and fact derivation | `internal/forecast`, `internal/facts` | 7 | Partly insufficient. Half-open selection, stable ordering, alert parsing and clipping, precipitation-window grouping, report-family selection, and SPC checked-empty behavior have focused owners. `AUD-016` through `AUD-019` record unit collapse, overnight alert omission, invalid percentage acceptance, and incomplete ice-text classification. | | Module and briefing contracts | `internal/module`, `internal/briefing` | 8-9 | Partly insufficient. Registry/default composition, report support, missing optional sources, checked-empty alerts, output identity, ordering, current/hourly prompt curation, precipitation timing, and SPC selection have focused owners. `AUD-020` through `AUD-028` record inconsistent pointer options, false weather-story availability, permissive snapshot schema validation, apparent-temperature mislabeling, daypart-key collisions, inconsistent planning identities, incomplete outdoor hazard scoring, incorrect subzero phrases, and stale official SPC descriptions. | -| Prompt inputs, embedded assets, and execution contracts | `internal/promptinput`, `internal/promptassets`, `internal/promptexec` | 10 | Pending | +| Prompt inputs, embedded assets, and execution contracts | `internal/promptinput`, `internal/promptassets`, `internal/promptexec` | 10 | Partly insufficient. Prompt/profile/schema identities, deterministic grouping, validation/execution result categories, safe bounded errors, explicit debug, and copy isolation have focused owners. `AUD-029` through `AUD-033` record raw warning types and transport paths crossing the boundary, stale embedded prompt paths, incorrect SPC locality instructions, permissive YAML loading, and missing serialization-failure coverage. | | Promptkit boundary and sensitive debug output | `internal/adapters/promptkit`, `internal/promptdebug` | 11 | Pending | | Generated-text validation | `internal/generatedtext` | 12 | Pending | | Render contexts and templates | `internal/generatedtext`, `internal/reporttemplate` | 13 | Pending | @@ -1267,6 +1277,189 @@ with evidence about meaningful risks, test ownership, gaps, and duplication. - Related findings: none - Remediation reference: pending +### AUD-029: Raw source warnings cross the curated prompt boundary + +- Stage: 10 +- Status: candidate +- Severity: low +- Confidence: high +- Category: security +- Area: `internal/promptinput.Metadata`, `promptinput.Package.SourceWarnings`, + and `weatherdata.SourceWarning.Endpoint` +- Evidence: `promptinput.Build` copies the complete normalized + `[]weatherdata.SourceWarning` into the top-level prompt package, and YAML + serialization includes each warning's `endpoint`. The Weather API adapter + populates that field with the source's fixed relative transport path whenever + an optional source is missing or malformed. In contrast, the briefing + metadata module intentionally projects warnings into a prompt-safe type + without endpoint or provenance fields, and its test asserts those omissions. + No prompt-input test asserts that raw weather-domain fields or transport + paths are absent. +- Contract at risk: Only curated prompt-facing values may cross the model + boundary; raw weather-domain and transport/provenance fields must not be + serialized merely because they are present on an upstream type. +- Impact: An optional-source failure sends an internal relative transport path + to Promptkit and the selected model provider and duplicates a safer warning + summary already in the briefing. Current adapter paths contain no host, + query, or credentials, limiting present disclosure, but future fields added + to the raw shared type would serialize without a prompt-boundary review. +- Recommendation: Replace the top-level raw warning field with a prompt-owned + safe projection, or remove it in favor of the metadata-module summary. Admit + only reviewed source, code, severity, bounded safe message, and completeness + fields; never endpoint, query, hash, or response detail. +- Test implications: Build and marshal a package from a warning with a unique + endpoint marker, then assert the marker is absent while intended safe warning + facts remain. Keep the assertion at the serialized provider-boundary owner. +- Validation: The exact bytes submitted for prompt execution contain no raw + transport/provenance fields; ordinary warning meaning and existing + deterministic YAML tests remain intact. +- Related findings: none +- Remediation reference: pending + +### AUD-030: The shared prompt instructs nonexistent briefing paths + +- Stage: 10 +- Status: candidate +- Severity: high +- Confidence: high +- Category: correctness +- Area: `internal/promptassets/assets/prompts/common/data_package.user.md` + and `internal/promptinput.BriefingStanzas` +- Evidence: The shared user prompt directs every report prompt to read + `briefing.derived_daily_summary`, `briefing.derived_daypart_summaries`, + `briefing.precip_timing`, and `briefing.outdoor_windows`. The deterministic + serializer places all four under `briefing.derived_summaries.`. + It also directs alert relevance checks to `briefing.metadata.alerts`, but the + prompt-facing metadata module has no `alerts` field; relevant counts live in + `briefing.applicable_risk_products.alert_digest`. Asset tests verify prompt, + profile, input-schema, and output-schema identities and successfully inspect + the real Promptkit definitions, but do not relate prose field references to + a representative serialized package. +- Contract at risk: Embedded instructions and the exact curated input schema + must evolve as one provider-facing contract. +- Impact: All four normal report prompts can overlook their primary derived + summaries or search the wrong alert location, pushing the model toward raw + data, unsupported inference, or false statements about alert relevance. +- Recommendation: Update the common prompt to name the actual categorized + paths and define one maintainable contract manifest or representative + serialized fixture from which semantic path assertions can be made. Avoid a + brittle whole-prose snapshot. +- Test implications: Inspect all embedded report prompts and assert the shared + instruction references paths present in a representative real + `promptinput.MarshalYAML` output, including derived and alert examples. +- Validation: Every named input path in the shared instruction resolves in the + current serialized package and a category move or prompt-path regression + fails one focused compatibility test. +- Related findings: none +- Remediation reference: pending + +### AUD-031: The shared prompt overstates SPC product locality + +- Stage: 10 +- Status: candidate +- Severity: high +- Confidence: high +- Category: correctness +- Area: `internal/promptassets/assets/prompts/common/data_package.user.md`, + `briefing.buildSPCConvectiveOutlooks`, and + `briefing.buildSPCConvectiveDiscussion` +- Evidence: The shared instruction says all active hazard and risk products + have already been filtered or matched to the forecast location and should be + treated as locally relevant. The SPC outlook module deliberately preserves + all selected overlapping records; only its categorical `risk_digest` + applies `ContainsLocation`, rank, type, and overlap filters. SPC discussions + are regional text selected by qualifying outlook day/type/rank/overlap, not + by point containment. Stage 9 retained this distinction as `RET-020` and + focused builder tests protect it, but the prompt-assets suite does not + protect the corresponding instruction semantics. +- Contract at risk: Safety-sensitive prompt guidance must distinguish + location-matched conclusions from regional context retained for grounded + interpretation. +- Impact: A model can present a non-containing SPC outlook record or regional + discussion as a hazard at the configured point, overstating severe-weather + risk despite the curated digest carrying the correct local conclusion. +- Recommendation: State explicitly that the alert relevant count and SPC + categorical risk digest are location-matched, while individual outlook + records and discussions require their containment/geographic limitations to + be preserved. Remove the blanket locality claim. +- Test implications: Add a focused semantic assertion for the shared + instruction and a representative package containing a non-local outlook + alongside a location-matched digest. Do not duplicate SPC selection tests. +- Validation: The embedded prompt cannot reasonably instruct a consumer to + treat regional or non-containing SPC material as point-local, while still + requiring locally relevant digest facts to lead the report. +- Related findings: `AUD-030` +- Remediation reference: pending + +### AUD-032: The exported YAML loader accepts malformed package shapes + +- Stage: 10 +- Status: candidate +- Severity: low +- Confidence: high +- Category: correctness +- Area: `internal/promptinput.LoadYAML` and `Validate` +- Evidence: Focused temporary probes confirmed that `LoadYAML` accepts a + package with no metadata stanza, ignores a value-map entry absent from + briefing order, ignores unknown top-level or report fields, and silently + accepts a trailing YAML document. `Validate` checks the current schema, + report identity, nonempty order, ordered-value presence, duplicates, and + known category placement, but not metadata presence or an exact + order/value bijection. The decoder neither enables `KnownFields(true)` nor + performs a second decode expecting EOF. Inbound graph traces find only test + callers for this loader, limiting current runtime impact. +- Contract at risk: Loading and validating an exported data package should + reject ambiguous, incomplete, or forward-unknown representations rather + than normalize them silently. +- Impact: Tooling or a future production consumer can accept a package that + loses stanzas on remarshal, lacks required provenance, carries unnoticed + unsupported fields, or hides a second document, giving a false compatibility + signal. The live Build-to-Marshal execution path remains valid. +- Recommendation: Require metadata exactly once, enforce an exact order/value + bijection, enable strict known-field decoding for structured fields, and + reject all content after the first YAML document. +- Test implications: Add a compact table for missing metadata, orphan value, + unknown root/nested fields, and a second document, retaining the current + round-trip, duplicate, misplaced, and old-schema cases. +- Validation: Each malformed probe fails with bounded context, one canonical + document round-trips deterministically, and no production execution behavior + changes. +- Related findings: none +- Remediation reference: pending + +### AUD-033: Stanza serialization failures lack a focused contract test + +- Stage: 10 +- Status: candidate +- Severity: low +- Confidence: high +- Category: testing +- Area: `internal/promptinput.BriefingStanzas.MarshalYAML` and `yamlNode` +- Evidence: Stanza values are normalized through JSON before YAML, and the + implementation wraps conversion failures with the stanza name. Existing + tests cover deterministic output, category placement, round trips, + uncategorized stanzas, and schema rejection, but none supplies an + unserializable value such as a non-finite float to execute this failure + branch. Normal registry outputs currently use fixed serializable shapes, so + no present default module triggers it. +- Contract at risk: A serialization failure must remain contextual and must + stop execution before any provider call; this behavior should not depend on + incidental library diagnostics. +- Impact: A future or changed prompt export can fail without a durable test + protecting stanza attribution, making provider-preflight errors harder to + diagnose or tempting unsafe fallback serialization. Current default reports + are unaffected. +- Recommendation: Add one focused prompt-input test using a deliberately + unsupported stanza value and assert a non-nil, stanza-attributed error. The + 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. +- Validation: The focused test fails if conversion errors lose stanza context + or are swallowed, and deterministic successful serialization remains green. +- Related findings: none +- Remediation reference: pending + ## Retained Decisions ### RET-001: Keep the application package as the explicit composition owner @@ -1496,6 +1689,32 @@ record set distinct from narrower editorial digests preserves evidence without forcing every consumer through one filter. Reconsider only if SPC product semantics or the report contract adopts a single shared selection policy. +### RET-021: Keep prompt category grouping explicit and serialization normalized + +The prompt package owns a small fixed category vocabulary and emits metadata, +applicable risk products, derived summaries, narrative products, and raw data +in one explicit order while preserving module order inside each category. +Stanza values pass through JSON normalization before YAML, preventing Go- and +YAML-specific value shapes from becoming an accidental provider contract. +The highest scoped marshal complexity operates over those fixed categories +and service-sized module lists, so a generic reflective serializer would add +policy opacity without a realistic cost benefit. Reconsider if the module +catalog becomes large or category metadata can be generated from one reviewed +manifest without weakening unknown/misplaced-stanza rejection. + +### RET-022: Keep neutral prompt execution lifecycle values dependency-owned + +`promptexec` defines only repository-owned inspection and execution requests, +completed results, safe categories, validation rejections, and explicit debug +values. The interface carries context to both operations; callbacks run before +provider execution; validation rejection is a completed result, while +operational failure returns no result; errors preserve causes without exposing +their text; and mutable returned bytes/maps are copied. Promptkit production +types remain adapter-local. This is a coherent narrow seam rather than an +anemic duplicate of the dependency API. Reconsider only if a second executor +implementation demonstrates a missing neutral semantic; Promptkit-specific +cancellation and diagnostic translation remain Stage 11. + ## Open Questions No Stage 1 open questions or unexplained baseline failures remain. @@ -1649,6 +1868,24 @@ Stage 9 routed these investigation leads to their assigned later stages: definition source and review date belong in canonical documentation after `AUD-028` is remediated. +Stage 10 routed these investigation leads to their assigned later stages: + +- `promptinput.Save` and `LoadYAML` have no production callers, while the + prompt-input document describes ordinary atomic saves and focused save + coverage that do not exist. `AUD-001` and `AUD-032` capture the code risks; + Stages 23-24 should decide whether to remove the unused API and reconcile the + canonical document rather than expanding a dormant persistence workflow. +- JSON unmarshalling reconstructs stanza order from map iteration and therefore + does not promise deterministic order. There is no exported JSON loader or + production consumer, and YAML is the execution format. Stage 23 should + assess retiring or explicitly limiting this incidental surface instead of + Stage 10 inventing a second ordering contract. +- The neutral interface passes context and makes debug detail explicit, but + provider cancellation, dependency error translation, and secure prompt-debug + publication belong to Stage 11. Application proof that serialization failure + prevents provider work belongs to Stage 14 with `AUD-033` as its focused + package-level prerequisite. + ## Stage Log ### Stage 1: Establish The Baseline And Audit Ledger @@ -2101,3 +2338,59 @@ Stage 9 routed these investigation leads to their assigned later stages: - Retained decisions: `RET-019` and `RET-020`. - Open questions: the three leads recorded above are routed to their assigned later stages. + +### Stage 10: Audit Prompt Inputs, Assets, And Neutral Execution Contracts + +- Status: Complete. +- Scope reviewed: all production and focused-test code in + `internal/promptinput`, `internal/promptassets`, and `internal/promptexec`; + every embedded prompt, Promptkit profile, input schema, and output schema; + prompt-input and generated-text internal documents; the Promptkit integration + contract; and immediate briefing/application consumers needed to establish + the provider-boundary data flow. +- Exclusions: Promptkit adaptation, provider cancellation/error translation, + and secure debug files remain Stage 11; output schemas and generated-text + semantics remain Stage 12; application preparation/provider preflight remains + Stage 14; cross-cutting API cleanup and documentation coherence remain Stages + 23-24; and adversarial repository-wide checks remain Stage 25. + +#### Prompt Boundary And Execution Accounting + +| Contract or risk | Serialization, asset, caller, and test evidence | Disposition | +| --- | --- | --- | +| Curated work-package construction | Application preparation reaches `promptinput.Build` and `MarshalYAML` from ordered module `DataPackageValue` exports, not normalized bundle serialization. Report identity, local date, period, and module categories are explicit. | Module values follow the curated path, but the top-level raw warning copy violates it in `AUD-029`. | +| Category order and normalization | One fixed table groups known stanzas as metadata, applicable risk, derived, narrative, and raw data; module order is retained within groups. Values normalize through JSON before YAML, and repeated marshals are byte-identical in focused tests. | Coherent and retained as `RET-021`; missing failure-branch coverage is `AUD-033`. | +| Package validation and loading | Current schema, report/run metadata, ordered stanza presence, duplicate order entries, known category membership, and misplaced stanzas are rejected. A temporary table-driven probe exercised additional malformed shapes. | Incomplete for required metadata, orphan values, unknown struct fields, and trailing documents; `AUD-032`. | +| Embedded prompt identities | Exactly four report definitions bind the registry's prompt ID/version, embedded profile, `weatherreporter.data_package.v4` input schema, and matching generated-text output schema. Real offline Promptkit inspection succeeds for each. | Identity is coherent; common prose references nonexistent serialized paths in `AUD-030`. | +| Safety-sensitive prompt semantics | The shared prompt prioritizes risk, forbids invention, and asks regional discussion limitations to be preserved. | Its blanket assertion that all retained products are location matched contradicts SPC module semantics; `AUD-031`. | +| Embedded profiles and schemas | Exactly three profiles carry the intended backend/model/timeouts/reasoning/tier settings; tests reject unsafe/incidental profile settings and retired runtime strings. Embedded schema bytes are independently copied. | Matches the scoped identity and immutability contracts. Output-schema field meaning remains Stage 12. | +| Inspection and execution lifecycle | Dependency-neutral requests carry context and exact prompt/profile identity. Execution runs validation callbacks before provider work and distinguishes completed validation rejection from operational failure without a result. | Coherent and retained as `RET-022`; adapter realization remains Stage 11. | +| Safe errors, debug, and result isolation | All stable error categories are covered; safe errors preserve causes while omitting cause text; messages and invalid UTF-8 diagnostics are bounded; debug is opt-in; returned output/debug bytes and maps are copied. | Sufficient at the neutral contract boundary. Dependency diagnostics remain Stage 11. | +| Complexity and realistic input size | The highest scoped production path is stanza YAML marshaling, with two shallow loops over five fixed categories and the report's service-sized module list. Asset lookup and neutral result construction are constant or small linear work. | No Stage 10 performance finding; explicit grouping remains cheaper to reason about than generic reflection. | +| Test ownership | Prompt-input tests own construction, validation, grouping, ordering, category placement, and YAML round trips; prompt-assets tests own embedded inventories/identities and real inspection; prompt-exec tests own lifecycle, category, error, debug, bound, and copy semantics. | Mostly behavioral and nonduplicated. Missing provider-boundary warning/path/locality and serialization/load cases are attached to `AUD-029` through `AUD-033`. | + +#### Commands And Evidence + +- Used graph architecture, symbol and augmented-code search, snippets, + inbound/outbound traces, data-flow traces, and complexity queries for + `Build`, `Validate`, `MarshalYAML`, `LoadYAML`, stanza grouping/normalization, + prompt asset lookup, execution request/result construction, and application + preparation callers. Bounded text inspection covered embedded Markdown, + YAML, and JSON assets and their focused assertions. +- Compared implementation and tests with `docs/internal/prompt-input.md`, + `docs/internal/generatedtext.md`, `docs/internal/module.md`, + `docs/integrations/promptkit.md`, and the architecture, documentation, and + testing policies. Every embedded prompt/profile/schema reference was + inventoried and cross-checked against the report registry. +- Ran a temporary focused malformed-package probe proving acceptance of + missing metadata, orphan stanza values, unknown fields, and a trailing YAML + document, then removed the probe without retaining test or product changes. +- Ran + `go test -coverprofile=/tmp/weatherreporter-stage10-cover.out ./internal/promptinput ./internal/promptassets ./internal/promptexec`; + all focused packages passed. Coverage guided branch inspection but was not + itself treated as a finding. Ran `go test ./...`, `go vet ./...`, and + `git diff --check`; all passed. +- Findings: `AUD-029`, `AUD-030`, `AUD-031`, `AUD-032`, and `AUD-033`. +- Retained decisions: `RET-021` and `RET-022`. +- Open questions: the three leads recorded above are routed to their assigned + later stages.