Record the Stage 13 rendering audit
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# Repository Audit Ledger
|
||||
|
||||
Status: In progress; Stages 1-12 complete.
|
||||
Status: In progress; Stages 1-13 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
|
||||
@@ -89,8 +89,16 @@ 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.
|
||||
the current registry's four identities otherwise match their contracts. Stage
|
||||
13 found two medium-severity rendering-boundary defects: context builders do
|
||||
not reject conflicting report identities, and unescaped generated prose can
|
||||
create Markdown structure that impersonates deterministic sections. It also
|
||||
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.
|
||||
|
||||
## Baseline Metadata
|
||||
|
||||
@@ -229,7 +237,7 @@ inventory commands, graph index refresh, and graph architecture inspection.
|
||||
| 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 | Complete |
|
||||
| 13 | Audit render contexts, templates, and Markdown rendering | Pending |
|
||||
| 13 | Audit render contexts, templates, and Markdown rendering | Complete |
|
||||
| 14 | Audit application preparation and prompt preflight | Pending |
|
||||
| 15 | Audit single-report generation and atomic output | Pending |
|
||||
| 16 | Audit batch orchestration and Distributor notification | Pending |
|
||||
@@ -263,7 +271,7 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
|
||||
| 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 | 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 |
|
||||
| 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 |
|
||||
| Single-report publication and preservation | `internal/app`, `internal/fileutil` | 15 | Pending |
|
||||
| Batch partial success and notification | `internal/app`, `internal/adapters/distributor` | 16 | Pending |
|
||||
@@ -1712,6 +1720,127 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
|
||||
- Related findings: `AUD-037`
|
||||
- Remediation reference: pending
|
||||
|
||||
### AUD-040: Render contexts accept conflicting report identities
|
||||
|
||||
- Stage: 13
|
||||
- Status: candidate
|
||||
- Severity: medium
|
||||
- Confidence: high
|
||||
- Category: correctness
|
||||
- Area: `internal/generatedtext.BuildHourlyRenderContext`, the three day-style
|
||||
context builders, and their module metadata projection
|
||||
- Evidence: Each exported builder selects its report-specific title, context
|
||||
type, and module surface from the function called, but none checks
|
||||
`briefing.Metadata.ReportID` or reconciles the separately decoded
|
||||
`Modules.Metadata.ReportID`. A temporary probe passed outer metadata labeled
|
||||
Hourly and a snapshot metadata stanza labeled Daily to
|
||||
`BuildDailyRenderContext`; it returned a Daily context without error, titled
|
||||
it as Daily, and retained the conflicting Daily module identity. Focused
|
||||
context tests supply consistent identities but have no mismatch case.
|
||||
Combined with `AUD-036`, a Daily definition selecting the known Hourly pair
|
||||
can reach the Hourly context builder with Daily metadata and render rather
|
||||
than fail at this boundary.
|
||||
- Contract at risk: A report-specific render context must be assembled only
|
||||
from metadata and module values for that report, with one authoritative
|
||||
identity before repository-owned Markdown rendering.
|
||||
- Impact: A registry, preparation, or direct package regression can render the
|
||||
wrong report shape while preserving conflicting metadata inside the same
|
||||
context. Current top-level templates do not display `Modules.Metadata`, but
|
||||
it is a documented template field and future edits can choose a different
|
||||
identity or timing source without an error exposing the inconsistency.
|
||||
- Recommendation: Make the expected report identity explicit in the common
|
||||
builder path, reject mismatched outer metadata, and when a metadata stanza is
|
||||
present verify its report ID, run ID, prompt ID, period, timezone, and other
|
||||
duplicated canonical metadata against the outer value. Prefer one documented
|
||||
template-facing owner for overlapping metadata rather than two unchecked
|
||||
copies.
|
||||
- Test implications: Add one shared mismatch table covering all four builders
|
||||
and one outer-versus-module metadata disagreement case. Keep successful
|
||||
report-specific context assertions without duplicating every metadata field
|
||||
per report.
|
||||
- Validation: Every builder rejects another report's metadata and any present
|
||||
metadata stanza that disagrees with the canonical context input; all four
|
||||
normal prepared contexts still render.
|
||||
- Related findings: `AUD-036`
|
||||
- Remediation reference: pending
|
||||
|
||||
### AUD-041: Dynamic prose can impersonate deterministic Markdown sections
|
||||
|
||||
- Stage: 13
|
||||
- Status: candidate
|
||||
- Severity: medium
|
||||
- Confidence: high
|
||||
- Category: security
|
||||
- Area: all top-level report templates and their interpolation of generated
|
||||
text and source-derived module strings through `text/template`
|
||||
- Evidence: `reporttemplate.Render` uses `text/template`, which performs no
|
||||
Markdown or HTML escaping, and the templates insert generated summary,
|
||||
forecast-discussion, and precipitation-timing strings directly. The schemas
|
||||
permit newlines and Markdown characters, while typed validation only trims
|
||||
outer whitespace. A temporary probe placed a second-level `Alert Digest`
|
||||
heading and fabricated warning bullet in an otherwise valid Daily summary;
|
||||
rendering succeeded and preserved both as active Markdown immediately before
|
||||
the repository-owned Alert Digest position. The same raw interpolation is
|
||||
used for externally sourced alert labels, condition text, daypart display
|
||||
strings, timing phrases, and other module values.
|
||||
- Contract at risk: Generated text fills bounded prose slots only;
|
||||
deterministic facts remain authoritative, and repository-owned templates
|
||||
alone define Markdown structure.
|
||||
- Impact: Malformed model output, indirect prompt injection through source
|
||||
material, or unexpected upstream Markdown can create headings, bullets,
|
||||
links, raw HTML, or formatting that visually masquerades as deterministic
|
||||
alerts and forecast facts. Schema validation does not distinguish prose from
|
||||
Markdown structure, so a validation-passed provider response can corrupt the
|
||||
trust boundary of the published report.
|
||||
- Recommendation: Define field-specific Markdown policies at the context or
|
||||
renderer boundary. Preserve intended paragraph breaks while escaping or
|
||||
rejecting block structure, raw HTML, unsafe links, control characters, and
|
||||
inline delimiters where they are not part of the contract; render module
|
||||
labels and values through reviewed inline-safe helpers. Do not rely solely on
|
||||
prompt instructions or downstream Markdown sanitization.
|
||||
- Test implications: Use synthetic markers across each generated prose slot
|
||||
and representative module label/value positions; assert none can introduce
|
||||
a heading, list item, code block, raw HTML block, or unsafe link while normal
|
||||
punctuation and intended paragraphs remain readable. Include the complete
|
||||
validation-to-render path for at least one report.
|
||||
- Validation: Parsing rendered output as Markdown shows that only template-
|
||||
owned nodes create report headings and deterministic list structure, and
|
||||
adversarial dynamic strings remain text within their authorized slots.
|
||||
- Related findings: `AUD-038`
|
||||
- Remediation reference: pending
|
||||
|
||||
### AUD-042: Today can render a daypart heading with no rows or fallback
|
||||
|
||||
- Stage: 13
|
||||
- Status: candidate
|
||||
- Severity: low
|
||||
- Confidence: high
|
||||
- Category: correctness
|
||||
- Area: `templates/partials/today_daypart_forecast.md.tmpl`
|
||||
- Evidence: The Today partial decides its fallback from whether
|
||||
`.Modules.Dayparts` is nonempty, then independently suppresses every row
|
||||
whose summary lacks both `DominantConditionDisplay` and
|
||||
`DominantCondition`. A temporary probe supplied one named daypart with no
|
||||
displayable condition; output contained `## Daypart Forecast` but neither a
|
||||
row nor `No daypart forecast details are available.` Existing tests protect
|
||||
the zero-length fallback and suppression of one empty Evening row beside
|
||||
valid rows, but not the all-filtered case.
|
||||
- Contract at risk: Missing deterministic values should produce an explicit,
|
||||
coherent fallback or omit the section, not leave an empty Markdown heading.
|
||||
- Impact: When selected Today dayparts exist structurally but all lack usable
|
||||
condition summaries, the published report presents a blank section that
|
||||
implies lost content and gives the operator no explanation.
|
||||
- Recommendation: Compute whether at least one Today row is displayable before
|
||||
emitting the heading. Render valid rows when present; otherwise render the
|
||||
existing no-details fallback, or consistently omit the complete section if
|
||||
that becomes the documented policy.
|
||||
- Test implications: Cover zero rows, all-filtered rows, mixed valid/filtered
|
||||
rows, and all-valid rows through the shared partial's observable output.
|
||||
- Validation: The Daypart Forecast heading is always followed by at least one
|
||||
row or the explicit fallback, and mixed rows retain their existing order.
|
||||
- Related findings: none
|
||||
- Remediation reference: pending
|
||||
|
||||
## Retained Decisions
|
||||
|
||||
### RET-001: Keep the application package as the explicit composition owner
|
||||
@@ -2021,6 +2150,34 @@ 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.
|
||||
|
||||
### RET-027: Keep report-specific contexts over one generic template map
|
||||
|
||||
Hourly, Daily, Today, and Tomorrow contexts expose concrete report metadata,
|
||||
generated-text types, and module surfaces. A private day-style projection
|
||||
shares metadata and common module extraction, while separate builders retain
|
||||
Daily outdoor/planning values, Today current-conditions and planning behavior,
|
||||
Tomorrow planning, Hourly's rolling-period labels, and distinct ordered
|
||||
daypart row types. Missing optional stanzas remain typed nil pointers, and
|
||||
module values cross the context boundary through a JSON-normalized typed copy.
|
||||
This is useful compile-time review friction: replacing it with a generic map
|
||||
would make misspelled fields, cross-report leakage, and incidental template
|
||||
compatibility harder to detect. Retain the typed split while adding the
|
||||
identity checks in `AUD-040`.
|
||||
|
||||
### RET-028: Keep templates as embedded assets with semantic output assertions
|
||||
|
||||
Each report's Markdown remains a separately reviewable embedded asset, and the
|
||||
four shared partials centralize only identical alert, precipitation, or
|
||||
daypart presentation policy. `missingkey=error`, template/partial-attributed
|
||||
parse failures, and template-attributed execution failures make drift
|
||||
actionable. Focused tests assert meaningful sections, conditions, exact
|
||||
weather phrases where semantics matter, and relative ordering without a large
|
||||
golden snapshot of every newline. Generated-text integration tests then render
|
||||
real typed contexts across the package boundary. This combination protects
|
||||
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`.
|
||||
|
||||
## Open Questions
|
||||
|
||||
No Stage 1 open questions or unexplained baseline failures remain.
|
||||
@@ -2227,6 +2384,24 @@ Stage 12 routed these investigation leads to their assigned later stages:
|
||||
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 13 routed these investigation leads to their assigned later stages:
|
||||
|
||||
- Render contexts expose complete `.Collected` and `.Derived` values plus a
|
||||
decoded `.Modules.Metadata` even though current templates use neither and
|
||||
`.Report` already owns display metadata. This is not a current output defect,
|
||||
but Stage 23 should assess narrowing the maintainer surface after `AUD-040`
|
||||
establishes one canonical identity and timing source.
|
||||
- `Render` reparses every top-level template and all four partials for every
|
||||
report. The embedded set is small and measured behavior is deterministic, so
|
||||
Stage 13 did not infer an efficiency defect. Stage 22 should compare a
|
||||
pre-parsed immutable template set only if repository-wide profiling shows
|
||||
rendering cost material.
|
||||
- Current template tests use concise package-local context structs while
|
||||
generated-text tests exercise the real typed contexts. That division gives
|
||||
useful focused and integration coverage, but Stages 20-21 should assess the
|
||||
large fixture surface and repeated report assertions across the whole test
|
||||
portfolio rather than Stage 13 rewriting unrelated cases.
|
||||
|
||||
## Stage Log
|
||||
|
||||
### Stage 1: Establish The Baseline And Audit Ledger
|
||||
@@ -2851,3 +3026,64 @@ Stage 12 routed these investigation leads to their assigned later stages:
|
||||
- Retained decisions: `RET-025` and `RET-026`.
|
||||
- Open questions: the three leads recorded above are routed to their assigned
|
||||
later stages.
|
||||
|
||||
### Stage 13: Audit Render Contexts, Templates, And Markdown Rendering
|
||||
|
||||
- Status: Complete.
|
||||
- Scope reviewed: `internal/generatedtext/render_context.go` and all focused
|
||||
context tests; all production code, tests, four top-level templates, and four
|
||||
partials in `internal/reporttemplate`; the generated-text and report-template
|
||||
internal documents; the complete maintainer template guide; and the narrow
|
||||
catalog/application caller path needed to establish report identity,
|
||||
template pairing, and in-memory render behavior.
|
||||
- Exclusions: Generated-text JSON decoding and schema semantics remain Stage
|
||||
12 except where their validated strings enter Markdown; application
|
||||
preparation, immutable copies, and execution ordering remain Stage 14;
|
||||
publication and notification consequences remain Stages 15-18; portfolio-
|
||||
wide test and complexity work remains Stages 19-23; documentation-wide
|
||||
coherence remains Stage 24; and broader adversarial rendering remains Stage
|
||||
25.
|
||||
|
||||
#### Context And Markdown Accounting
|
||||
|
||||
| Contract or risk | Context, template, partial, caller, and test evidence | Disposition |
|
||||
| --- | --- | --- |
|
||||
| Report metadata and identity | Hourly builds location and rolling-period labels; day-style reports derive civil-date/day-name labels and preserve distinct titles. Timezones, generated times, and periods are validated before construction. | Builders do not validate the report ID or reconcile the separately decoded metadata stanza; `AUD-040`. Error branches for invalid time metadata also lack focused tests, to be covered with the identity remediation rather than as a second finding. |
|
||||
| Module-to-context mapping | Snapshot stanzas are selected by canonical stanza name, JSON-normalized through `module.StanzaValue`, and projected into typed optional pointers. Common day-style extraction is followed by report-specific Daily, Today, or Tomorrow planning fields; Hourly has its own surface. Extraction errors name the failing module. | Coherent under valid snapshots and retained as `RET-027`. Snapshot schema-version permissiveness remains the upstream `AUD-022`; duplicate stanzas are rejected by `module.NewSnapshot`. |
|
||||
| Deterministic daypart ordering | Typed row slices follow `DerivedFacts.DaypartSummaries`, try canonical and date-qualified keys, suppress already selected keys, then append remaining module keys lexically. Templates range only the ordered slices, not the source map. | Byte-stable and semantically protected for configured and remaining rows. Upstream key collisions remain `AUD-024`; no new ordering defect. |
|
||||
| Generated prose slots | Summary and forecast discussion occupy explicit top-level prose positions. Precipitation prose renders only when deterministic precipitation windows exist, preventing that slot from creating a dry-period timing section by itself. | Slot selection is coherent, but raw Markdown lets any prose slot create unauthorized report structure; `AUD-041`. Unbounded prose remains `AUD-038`. |
|
||||
| Deterministic sections | Alert/SPC, current conditions, hourly/daypart forecast, precipitation windows, and their numeric values come from typed module fields. Alert instructions/descriptions and unused planning/narrative modules do not silently enter current output. Enhanced-or-higher SPC policy is centralized in helpers. | Current field ownership is explicit. Dynamic module strings share the Markdown-escaping defect in `AUD-041`; Today's all-filtered daypart case is `AUD-042`. |
|
||||
| Missing values and report variation | Optional module pointers are guarded. Hourly provides explicit current/hourly fallbacks; day-style reports provide daypart fallbacks; Today alone includes current conditions and deliberately suppresses conditionless rows. Daily/Tomorrow share one richer daypart partial. | Intentional variation is preserved, except Today's row filter can leave a bare heading; `AUD-042`. |
|
||||
| Template and partial lookup | Four fixed IDs resolve independent embedded source strings. `Render` applies `missingkey=error`, parses the chosen top-level asset and every shared partial, and attributes lookup, top-level parse, partial read/parse, and execution failures. All partials being parsed for every report makes a broken shared asset fail closed. | Coherent and retained as `RET-028`. Schema ownership remains in `promptassets`; catalog report pairing remains `AUD-036` and misplaced schema tests remain `AUD-039`. |
|
||||
| Escaping and formatting | Repository literals define headings, bullets, ordering, and spacing. Templates use `text/template`; dynamic strings are not recursively evaluated as template actions. | There is no Markdown/HTML structural escaping for provider or source-derived strings; `AUD-041`. |
|
||||
| Output determinism | Embedded assets, typed slices, fixed partial order, and absence of current map iteration make repeated rendering of an unchanged context byte-identical. A temporary repeat-render probe confirmed identical output. | Sufficient. Context aliasing/immutability before rendering belongs to Stage 14. |
|
||||
| Test review value | Tests cover real context construction for all reports, rich/omitted modules, module decoding errors, ordering, partial thresholds, conditional sections, fallback rows, open-ended precipitation, missing context, actionable unknown IDs, and semantically important output order. They use focused fragments rather than full-output goldens. | Good semantic review value under `RET-028`. Identity mismatch, Markdown structure, and all-filtered Today rows are missing and attach directly to `AUD-040` through `AUD-042`; cross-suite fixture cost remains Stages 20-21. |
|
||||
| Complexity and cost | Builders are straight-line projections plus linear stanza/daypart passes; remaining daypart keys sort deterministically. Template helpers scan small alert/SPC slices, and rendering parses eight embedded assets per call. | Proportionate at current service-sized inputs. Output-size amplification remains `AUD-038`; cross-cutting render performance remains Stage 22. |
|
||||
|
||||
#### Commands And Evidence
|
||||
|
||||
- Used graph architecture, symbol and augmented-code search, snippets,
|
||||
inbound traces, and complexity queries for all context builders, module
|
||||
extraction, daypart ordering, template lookup/rendering, partial helpers, and
|
||||
application callers. Text inspection covered every scoped Go file, template,
|
||||
partial, focused test, and canonical template/internal document.
|
||||
- Cross-checked every current template field against its typed context
|
||||
producer, confirmed no template ranges over a map, inventoried optional
|
||||
guards and fallbacks, compared the Today/Daily/Tomorrow variations, and
|
||||
traced normalized snapshot values and validated generated prose into
|
||||
repository-owned Markdown.
|
||||
- Ran temporary offline probes showing that a Daily builder accepts outer
|
||||
Hourly metadata alongside a Daily metadata stanza, a schema-valid generated
|
||||
summary can inject an Alert Digest heading and fabricated warning bullet,
|
||||
repeated rendering is byte-identical, and an all-filtered Today daypart slice
|
||||
produces a bare heading. The probes were removed without retaining source or
|
||||
test changes.
|
||||
- Ran focused coverage for `./internal/generatedtext` and
|
||||
`./internal/reporttemplate`; both passed with 81.3% and 86.2% statement
|
||||
coverage respectively. Coverage identified untested metadata failure paths
|
||||
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-040`, `AUD-041`, and `AUD-042`.
|
||||
- Retained decisions: `RET-027` and `RET-028`.
|
||||
- Open questions: the three leads recorded above are routed to their assigned
|
||||
later stages.
|
||||
|
||||
Reference in New Issue
Block a user