Record the Stage 12 generated text audit
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# Repository Audit Ledger
|
||||
|
||||
Status: In progress; Stages 1-11 complete.
|
||||
Status: In progress; Stages 1-12 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
|
||||
@@ -82,8 +82,15 @@ before sensitive files are created or replaced. Promptkit type containment,
|
||||
source precedence, exact inline execution, credential preflight, cancellation,
|
||||
safe error classification, callback ordering, restrictive modes, atomic file
|
||||
replacement, and ordinary concurrent execution otherwise match their
|
||||
contracts. Subsystem conclusions and final disposition remain pending the
|
||||
later stages.
|
||||
contracts. Stage 12 found one medium-severity catalog defect because known
|
||||
schema/template pairs are accepted for the wrong report, one low-severity
|
||||
schema/decoder mismatch because Go accepts case-variant property names, one
|
||||
medium-severity resource and diagnostic-boundary defect because generated
|
||||
prose and unknown-field errors are unbounded, and one low-severity test
|
||||
maintenance defect from duplicated day-style and misplaced schema assertions.
|
||||
Typed normalization, required-field semantics, immutable schema copies, and
|
||||
the current registry's four identities otherwise match their contracts.
|
||||
Subsystem conclusions and final disposition remain pending the later stages.
|
||||
|
||||
## Baseline Metadata
|
||||
|
||||
@@ -221,7 +228,7 @@ inventory commands, graph index refresh, and graph architecture inspection.
|
||||
| 9 | Audit derived, planning, formatting, and SPC briefing modules | Complete |
|
||||
| 10 | Audit prompt inputs, assets, and neutral execution contracts | Complete |
|
||||
| 11 | Audit Promptkit adaptation and secure prompt debugging | Complete |
|
||||
| 12 | Audit generated-text validation and catalog contracts | Pending |
|
||||
| 12 | Audit generated-text validation and catalog contracts | Complete |
|
||||
| 13 | Audit render contexts, templates, and Markdown rendering | Pending |
|
||||
| 14 | Audit application preparation and prompt preflight | Pending |
|
||||
| 15 | Audit single-report generation and atomic output | Pending |
|
||||
@@ -255,7 +262,7 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
|
||||
| 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 | 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 | 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 | Pending |
|
||||
| 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 | Pending |
|
||||
| Prompt preflight and prepared inputs | `internal/app` | 14 | Pending |
|
||||
| Single-report publication and preservation | `internal/app`, `internal/fileutil` | 15 | Pending |
|
||||
@@ -1546,6 +1553,165 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
|
||||
- Related findings: none
|
||||
- Remediation reference: pending
|
||||
|
||||
### AUD-036: The generated-text catalog accepts a known pair for the wrong report
|
||||
|
||||
- Stage: 12
|
||||
- Status: candidate
|
||||
- Severity: medium
|
||||
- Confidence: high
|
||||
- Category: correctness
|
||||
- Area: `internal/generatedtext.LookupDefinition` and the generated-text
|
||||
catalog
|
||||
- Evidence: Catalog entries contain only schema ID, template ID, validator,
|
||||
and render-context builder. `LookupDefinition` accepts the first matching
|
||||
schema/template pair and copies the caller's report ID into the handler
|
||||
without checking that the pair belongs to that report. A temporary probe
|
||||
changed a copied Daily definition to the known `hourly`/`hourly` pair; lookup
|
||||
returned an Hourly handler with no error. The current registry loop test
|
||||
compares handler IDs with the same definition it supplied, and the report
|
||||
registry test requires only nonempty schema/template IDs, so neither test
|
||||
detects a coherent but wrong known pair.
|
||||
- Contract at risk: Each generated-text report definition must resolve to its
|
||||
one report-specific schema, validator, template, and context builder before
|
||||
provider work begins; the separate Daily, Today, Tomorrow, and Hourly value
|
||||
types preserve that identity even where fields currently match.
|
||||
- Impact: A registry or constructed-definition regression can route a report
|
||||
through another report's output contract. Depending on the selected pair it
|
||||
can fail only after provider execution during context assembly, or render a
|
||||
semantically wrong report if the downstream inputs remain structurally
|
||||
compatible.
|
||||
- Recommendation: Include `report.ID` in every catalog entry and require the
|
||||
exact report/schema/template triple. Keep unknown schema, unknown template,
|
||||
known-but-mismatched pair, and wrong-report diagnostics distinct and bounded.
|
||||
- Test implications: Assert the exact four report-to-pair mappings from an
|
||||
independent expectation table, and reject every known pair when attached to
|
||||
a different report. Avoid deriving the expected pair from the definition
|
||||
under test.
|
||||
- Validation: Every current definition resolves once, and swapping any two
|
||||
known pairs fails during lookup before prompt inspection or provider work.
|
||||
- Related findings: none
|
||||
- Remediation reference: pending
|
||||
|
||||
### AUD-037: Typed decoding accepts property names rejected by the schema
|
||||
|
||||
- Stage: 12
|
||||
- Status: candidate
|
||||
- Severity: low
|
||||
- Confidence: high
|
||||
- Category: correctness
|
||||
- Area: `internal/generatedtext.decodeGeneratedText` and the four generated-
|
||||
text schemas
|
||||
- Evidence: The schemas require the exact lowercase names `summary`,
|
||||
`forecast_discussion`, and `precipitation_timing` while rejecting additional
|
||||
properties. Go's `encoding/json` matches tagged struct fields without regard
|
||||
to ASCII case. A temporary probe showed that Hourly validation accepts
|
||||
`Summary` and `Forecast_Discussion`, then normalizes them to the canonical
|
||||
lowercase names. A real offline Promptkit execution against the embedded
|
||||
schema classified the same response as failed. The exact-name raw-map check
|
||||
happens only for `precipitation_timing`, leaving the other two fields
|
||||
inconsistent.
|
||||
- Contract at risk: The schema validator and repository-owned typed validator
|
||||
should accept the same property vocabulary, and typed decoding is documented
|
||||
as rejecting unknown fields without a second JSON Schema engine.
|
||||
- Impact: The normal Promptkit path currently rejects these aliases first, but
|
||||
direct handler callers or another conforming executor that reports completed
|
||||
validation can accept output the canonical schema forbids. This weakens the
|
||||
typed validator as an independent boundary and makes behavior depend on the
|
||||
executor implementation.
|
||||
- Recommendation: Inspect top-level raw object keys with exact matching before
|
||||
struct decoding, rejecting case variants, duplicates, missing names, and
|
||||
additions under one explicit policy. Preserve typed decoding for field
|
||||
shapes and semantic normalization.
|
||||
- Test implications: Add one shared exact-key contract across all four public
|
||||
validators and an embedded-schema integration case proving the two layers
|
||||
agree. Do not repeat every casing variant in each report-specific suite.
|
||||
- Validation: A corpus of canonical, missing, added, case-varied, duplicate,
|
||||
null, and wrong-type fields produces compatible schema and Go decisions.
|
||||
- Related findings: none
|
||||
- Remediation reference: pending
|
||||
|
||||
### AUD-038: Generated prose and decoder diagnostics have no size boundary
|
||||
|
||||
- Stage: 12
|
||||
- Status: candidate
|
||||
- Severity: medium
|
||||
- Confidence: high
|
||||
- Category: reliability
|
||||
- Area: the generated-text schemas, `decodeGeneratedText`, day-style arrays,
|
||||
and application propagation of validation errors
|
||||
- Evidence: None of the four schemas declares string-length, array-item, or
|
||||
total-output bounds, and the Go validators impose none before decoding,
|
||||
trimming, filtering, and normalizing. Embedded profiles intentionally omit
|
||||
output-token limits. Temporary offline probes showed both Promptkit schema
|
||||
validation and `ValidateHourly` accepting and copying a synthetic response
|
||||
with a two-MiB summary. Another probe used a 64-KiB synthetic unknown-field
|
||||
name; `encoding/json` copied the complete name into a 65,603-byte error.
|
||||
`profileExecutionError` and `generatedReportError` wrap validator text
|
||||
verbatim, so an executor that reaches this independent layer can carry that
|
||||
content into ordinary errors. All probes were removed.
|
||||
- Contract at risk: Provider output crosses a bounded, content-safe validation
|
||||
boundary before normalization and rendering; ordinary errors must exclude
|
||||
response-body content and remain safe to summarize or log.
|
||||
- Impact: A provider, fake, or future executor can force repeated large
|
||||
allocations in validation/normalization and produce an unexpectedly large
|
||||
Markdown artifact from schema-valid prose. Malformed field names can also
|
||||
turn provider-controlled response text into normal diagnostics. Provider
|
||||
context limits and current Promptkit rejection of unknown fields reduce but
|
||||
do not define the repository's accepted size or independent error contract.
|
||||
- Recommendation: Define reviewed total raw-output, per-string, and day-style
|
||||
paragraph-count/aggregate bounds; express compatible bounds in JSON Schema
|
||||
and enforce the total before decoding. Translate decoder failures into
|
||||
bounded categories that identify canonical fields without echoing arbitrary
|
||||
keys or raw content.
|
||||
- Test implications: Exercise exact boundary and one-over cases for total
|
||||
bytes, string lengths, paragraph count/aggregate size, and oversized unknown
|
||||
names with synthetic markers. Assert normalized bytes and every ordinary
|
||||
error remain bounded without duplicating all cases per day-style report.
|
||||
- Validation: Schema and Go limits agree, excessive output fails before
|
||||
normalization/rendering, and no malformed provider value can make normal
|
||||
error text exceed the documented diagnostic bound.
|
||||
- Related findings: none
|
||||
- Remediation reference: pending
|
||||
|
||||
### AUD-039: Generated-text contract tests duplicate policy across owners
|
||||
|
||||
- Stage: 12
|
||||
- Status: candidate
|
||||
- Severity: low
|
||||
- Confidence: high
|
||||
- Category: testing
|
||||
- Area: `internal/generatedtext/{daily,today,tomorrow,day_style}_test.go` and
|
||||
schema assertions in `internal/reporttemplate/reporttemplate_test.go`
|
||||
- Evidence: Daily, Today, and Tomorrow each carry nearly line-for-line copies
|
||||
of normalization and malformed-input tables, while the shared day-style
|
||||
suite repeats summary, discussion, normalization, empty timing, missing
|
||||
timing, unknown-field, and retired-field behavior across the same three
|
||||
validators. Separately, `reporttemplate_test.go` imports `promptassets` and
|
||||
repeats four schema-shape assertions, including three duplicated day-style
|
||||
blocks, even though `reporttemplate` owns Markdown assets and
|
||||
`promptassets_test.go` already owns the schema inventory, strict-object
|
||||
shape, required fields, identity, and copy isolation.
|
||||
- Contract at risk: Similar public validators should remain independently
|
||||
protected without copying shared semantics, and schema tests should reside
|
||||
with the asset owner rather than an unrelated template package.
|
||||
- Impact: One intentional day-style or schema change requires synchronized
|
||||
edits across several files and packages, creates noisy failures outside the
|
||||
responsible owner, and can still miss cross-layer discrepancies such as
|
||||
`AUD-037` because duplicated examples are not an agreement test.
|
||||
- Recommendation: Keep one table-driven shared day-style behavior suite with a
|
||||
thin per-exported-validator identity/type case. Move complete schema-shape
|
||||
ownership to `promptassets`; keep only a generated-text integration contract
|
||||
that compares schema acceptance with typed validation where their semantics
|
||||
intentionally overlap. Remove schema assertions from `reporttemplate`.
|
||||
- Test implications: Preserve distinct Hourly behavior and each public report
|
||||
identity while deleting repeated semantic cases. A mechanical refactor of a
|
||||
private day-style helper should not require unrelated test edits.
|
||||
- Validation: The leaner suite still fails for a wrong report dispatch,
|
||||
schema/type disagreement, required-field regression, or normalization
|
||||
change, and report-template tests fail only for template/render behavior.
|
||||
- Related findings: `AUD-037`
|
||||
- Remediation reference: pending
|
||||
|
||||
## Retained Decisions
|
||||
|
||||
### RET-001: Keep the application package as the explicit composition owner
|
||||
@@ -1830,6 +1996,31 @@ owner while remediating `AUD-034` and `AUD-035`; reconsider shared mechanics
|
||||
only after a common primitive can preserve every artifact's security and
|
||||
transaction boundary.
|
||||
|
||||
### RET-025: Keep report-specific generated values over shared day-style mechanics
|
||||
|
||||
Daily, Today, and Tomorrow currently share the same three fields and one
|
||||
private validation path, but they remain distinct exported value types and
|
||||
catalog handlers. This lets context builders reject a value validated for a
|
||||
different report and leaves each report free to evolve without converting a
|
||||
temporary structural similarity into a public interchangeability promise.
|
||||
The private `dayStyleFields` projection, trimming, nonblank filtering, and
|
||||
normalization remove mechanical duplication without erasing identity. Retain
|
||||
that split while fixing catalog identity in `AUD-036` and consolidating tests
|
||||
under `AUD-039`; reconsider only if the reports intentionally adopt one shared
|
||||
generated-text contract and migration policy.
|
||||
|
||||
### RET-026: Keep semantic typed validation after schema validation
|
||||
|
||||
Promptkit's embedded JSON Schema validates provider shape, while
|
||||
`internal/generatedtext` produces repository-owned typed values, trims prose,
|
||||
removes blank day-style paragraphs, requires usable summaries/discussions, and
|
||||
normalizes the result. Those are distinct boundary and domain responsibilities:
|
||||
schema success alone cannot construct the typed render input, and decoding
|
||||
alone should not make execution correctness depend on a particular executor.
|
||||
Retain both layers and make their overlapping vocabulary and limits agree when
|
||||
remediating `AUD-037` and `AUD-038`; do not replace semantic validation with a
|
||||
second general-purpose schema engine.
|
||||
|
||||
## Open Questions
|
||||
|
||||
No Stage 1 open questions or unexplained baseline failures remain.
|
||||
@@ -2019,6 +2210,23 @@ Stage 11 routed these investigation leads to their assigned later stages:
|
||||
propagation and Promptkit's concurrent-engine contract without duplicating
|
||||
those orchestration findings.
|
||||
|
||||
Stage 12 routed these investigation leads to their assigned later stages:
|
||||
|
||||
- The application discards the canonical normalized JSON returned beside the
|
||||
typed generated value. This is harmless in the current render path and the
|
||||
package documents the return value, but Stage 23 should decide whether a
|
||||
production consumer justifies retaining that API surface rather than
|
||||
treating non-use alone as a defect.
|
||||
- `encoding/json` also accepts duplicate object names with last-value wins.
|
||||
The embedded schema validator follows parsed-JSON semantics and no canonical
|
||||
contract currently promises duplicate rejection, so Stage 12 did not invent
|
||||
a uniqueness rule. Stage 25 may include duplicates in its broader
|
||||
adversarial corpus when validating the remediation for `AUD-037`.
|
||||
- Context construction, template fields, partial execution, and Markdown
|
||||
semantics remain Stage 13. Application proof that catalog and generated-text
|
||||
failures occur before publication remains with Stages 14-18; Stage 12 traced
|
||||
only enough of those callers to establish the raw-output validation boundary.
|
||||
|
||||
## Stage Log
|
||||
|
||||
### Stage 1: Establish The Baseline And Audit Ledger
|
||||
@@ -2584,3 +2792,62 @@ Stage 11 routed these investigation leads to their assigned later stages:
|
||||
- Retained decisions: `RET-023` and `RET-024`.
|
||||
- Open questions: the three leads recorded above are routed to their assigned
|
||||
later stages.
|
||||
|
||||
### Stage 12: Audit Generated-Text Validation And Catalog Contracts
|
||||
|
||||
- Status: Complete.
|
||||
- Scope reviewed: all production and focused validation/catalog tests in
|
||||
`internal/generatedtext` except `render_context.go` and its focused tests;
|
||||
all four embedded generated-text schemas and their `promptassets` owner;
|
||||
schema assertions currently placed in `internal/reporttemplate`; the
|
||||
generated-text and Promptkit integration contracts; and the immediate
|
||||
Promptkit/application path needed to establish schema validation, raw-output
|
||||
dispatch, and error propagation.
|
||||
- Exclusions: Render-context assembly, template/partial semantics, and Markdown
|
||||
rendering remain Stage 13; application preparation and preflight ordering
|
||||
remain Stage 14; publication consequences remain Stages 15-18; suite-wide
|
||||
durability and refactoring remain Stages 19-23 except for duplication
|
||||
explicitly assigned to this stage; documentation-wide coherence remains
|
||||
Stage 24; and repository-wide adversarial checks remain Stage 25.
|
||||
|
||||
#### Generated-Text Contract Accounting
|
||||
|
||||
| Contract or risk | Schema, decoder, catalog, caller, and test evidence | Disposition |
|
||||
| --- | --- | --- |
|
||||
| Raw output to typed value | Promptkit validates raw provider JSON against the report definition's embedded schema and reports passed/failed status. After a pass, app dispatches the same raw bytes through the prepared handler; the report validator strictly decodes shape, trims prose, removes blank day-style paragraphs, applies semantic required-field rules, and returns a typed value plus canonical JSON before context construction. | The path and ownership are explicit. Retain both validation layers under `RET-026`; their exact-name disagreement is `AUD-037`. |
|
||||
| Required and empty fields | Every schema requires summary, forecast discussion, and precipitation timing and rejects additional properties. Go requires nonblank trimmed summary/discussion; day-style discussion retains one or more nonblank paragraphs; precipitation timing must be present and string-valued but may normalize to empty. Null and wrong structural types fail. | Coherent aside from case-insensitive aliases. The schema intentionally permits lexical blanks that repository semantic validation rejects. |
|
||||
| JSON document strictness | Typed decoding rejects malformed JSON, unknown canonical additions, wrong field types, and multiple top-level values. It accepts surrounding whitespace. Duplicate names follow Go/parsed-JSON last-value semantics because no stronger canonical rule exists. | Sufficient for documented document shape, with exact-key enforcement missing in `AUD-037`. Duplicate policy is routed to Stage 25 remediation validation rather than declared retroactively. |
|
||||
| Array and size behavior | Day-style arrays require one schema item and one nonblank normalized paragraph but have no item-count or aggregate bound. No schema or Go string/total-output bound exists, and decoder unknown-field errors quote arbitrary names. | Unbounded accepted prose and diagnostics are `AUD-038`. |
|
||||
| Report-specific types and shared mechanics | Daily, Today, and Tomorrow are distinct types and validators over one private field projection and validation helper; Hourly deliberately uses a single discussion string. Typed context builders can therefore distinguish reports even when day-style fields match. | Appropriate separation retained as `RET-025`; tests duplicate rather than merely protect that choice in `AUD-039`. |
|
||||
| Catalog compatibility | Four entries bind schema/template pairs to validators and builders; current registry iteration resolves all entries, schema/template lookup returns nonempty assets, and unknown IDs or mismatched known pairs fail. | Entries omit report identity, so a complete known pair can be assigned to the wrong report; `AUD-036`. |
|
||||
| Asset ownership and immutability | `promptassets` embeds and returns independent copies of all four schema byte slices. `generatedtext.Handler.Schema` adds report context; `reporttemplate` production code owns templates and does not own schemas. | Production ownership and copy isolation are coherent. Schema tests placed in `reporttemplate` duplicate the asset owner's assertions; `AUD-039`. |
|
||||
| Error safety | Semantic required-field errors and catalog errors contain stable report/schema/template identifiers. JSON library errors are wrapped with report kind and preserve causes. | Arbitrary unknown-field names are emitted without a diagnostic bound and can be wrapped into normal application errors; part of `AUD-038`. No raw value is otherwise deliberately included. |
|
||||
| Complexity and realistic cost | Scoped validators are straight-line except for one pass over discussion paragraphs; catalog lookup scans four fixed entries. Normal work is linear in provider output and paragraph count. | Algorithms are proportionate, but absent input/array limits make their allocation and normalization cost attacker/provider-controlled; `AUD-038`. |
|
||||
| Test ownership and durability | Focused tests cover all four dispatches, canonical normalization, malformed/type/multiple-value cases, required semantics, retired fields, catalog lookup, schema inventory/identity, and independent schema bytes. | Exact report pairing, schema/Go agreement, and bounds are unprotected. Day-style semantics and schema shapes are redundantly asserted across packages; `AUD-036` through `AUD-039`. |
|
||||
|
||||
#### Commands And Evidence
|
||||
|
||||
- Used graph architecture, symbol and augmented-code search, inbound/data-flow
|
||||
tracing, snippets, and complexity queries for catalog lookup, schema access,
|
||||
typed decoding/normalization, report validators, application execution, and
|
||||
their production/test callers. Bounded text inspection covered the four JSON
|
||||
schemas, scoped tests, generated-text/Promptkit documentation, and policy.
|
||||
- Compared each schema's exact required fields and property shapes with Daily,
|
||||
Today, Tomorrow, and Hourly structs and semantic validators. Traced schema
|
||||
validation status through Promptkit adaptation into app's handler dispatch
|
||||
and verified schema byte-copy isolation and fixed catalog scan cost.
|
||||
- Ran temporary offline probes showing that a Daily definition accepts the
|
||||
known Hourly pair, Go accepts case-varied summary/discussion keys while real
|
||||
embedded-schema validation rejects them, both layers accept a two-MiB valid
|
||||
summary, and a 64-KiB synthetic unknown key appears in full in the decoder
|
||||
error. The probes were removed without retaining source or test changes.
|
||||
- Ran
|
||||
`go test -coverprofile=/tmp/weatherreporter-stage12-cover.out ./internal/generatedtext ./internal/promptassets ./internal/reporttemplate`;
|
||||
all packages passed with 81.3%, 73.7%, and 86.2% statement coverage
|
||||
respectively. 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-036`, `AUD-037`, `AUD-038`, and `AUD-039`.
|
||||
- Retained decisions: `RET-025` and `RET-026`.
|
||||
- Open questions: the three leads recorded above are routed to their assigned
|
||||
later stages.
|
||||
|
||||
Reference in New Issue
Block a user