diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 1ba14c0..c0e69fe 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -352,7 +352,10 @@ Completion record (2026-09-12): all acceptance criteria have named evidence: The final review found that queries use only the callback guard and detached metadata/source indexes; they do not extend callback lifetime, expose raw values, or perform environment/filesystem work. The view is created only for resolver loads and -does not retain history. The comparison with `v0.2.0` found only additive exported +now retains only effective field/path metadata and source records. Post-implementation +review found that the original constructor unnecessarily cloned history; remediation +replaced that clone with detached field-only construction, leaving final result +history unchanged. The comparison with `v0.2.0` found only additive exported methods; loading, parser, path, composition, credential, and existing callback semantics remain unchanged. Canonical Go, configuration, consumer, internal, and roadmap documentation are synchronized; the feature roadmap is retained only until diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..ac64fda --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,78 @@ +# Resolution provenance remediation + +Review baseline: `715364b`. Reviewed implementation commits `88f21d0` through +`715364b` against `818d36f`. Status: finding resolved. + +## R1 — Avoid copying unused history into the resolution entry view + +Status: resolved. Priority: medium. + +### Evidence and impact + +At the review baseline, `newResolutionInput` in [load_metadata.go](../../load_metadata.go) called +`cloneMetadata(metadata)` before adding its effective-path index. `cloneMetadata` +deep-copies `metadata.history`, including each `Change` and its nested records, +as well as any historical-path index. At this lifecycle boundary, +`metadataFromDecoded` and `applyOverlayMetadata` have already populated history. + +The entry view only exposes `Metadata.Lookup` and source-ID lookup. Neither query +uses history. Every load with a resolver therefore duplicates historical records +that are unnecessary for the requested entry-state queries, including records for +displaced/default paths that the view cannot query. This adds allocation and copy +work proportional to retained history even for a resolver that never calls the +new methods. No wall-clock or memory threshold was measured or is asserted here. + +The Stage 5 completion record in [implementation.md](implementation.md) also states +that the view "does not retain history"; the constructor contradicts that claim. + +### Correction + +Construct the entry view from the effective field/path information and source +records needed by its queries, avoiding the unused deep copy of history. A small +field-only copy using the existing record clone helpers is one straightforward +solution. Immutable sharing or another ownership arrangement is also acceptable +if it preserves the agreed stable queries and detached public returns. + +Do not change `cloneMetadata` globally to drop history: final result metadata +still requires it. Do not alter public APIs, weaken detachment, introduce resource +caps, or make a particular representation/complexity bound a public guarantee. +Correct the completion record to describe the verified implementation. + +This finding concerns demonstrably unused work, not a requirement for eager +copying, zero allocations, or a new security restriction. + +### Acceptance and validation + +- Inspection of the entry-view construction confirms it no longer deep-copies + history solely for these queries. +- Existing public tests still prove fixed entry information during normalization, + moves, and deletions; detached field/source returns; and correct final history. +- Update the completion record only after verifying its claims. Existing behavioral + tests are sufficient unless the chosen fix exposes a new risk; do not freeze a + private struct layout in a test just to prove this optimization. +- Run `go test . -run 'Resolution|Metadata|Provenance|Historical|Explain'`, + `./scripts/check.sh`, and `go test -race ./...`. Use the full release checker + when preparing a candidate under the release procedure. + +## Review results and boundaries + +Resolution: the entry constructor now copies only effective `FieldInfo` records +using `cloneFieldInfo`, builds the existing effective-path index, and retains its +detached source records. It no longer invokes `cloneMetadata`; final-result +metadata cloning and history are unchanged. The Stage 5 completion record has +been corrected. No public API, resource limit, or security restriction was added. + +Post-fix validation passed on Linux with Go 1.26.7: the focused resolution, +metadata, provenance, historical, and explanation tests; `./scripts/check.sh`; +and `go test -race ./...`. + +The full Linux release checker passed on `715364b` using Go 1.25.5 and Go 1.26.7, +including race tests, consumer examples, documentation links, and cross-builds. +No functional provenance, detachment, credential-timing, or application-policy +restriction regression was identified in this review. This is not native macOS +release evidence or publication approval. + +The warning-and-retain fallback in the maintained Notarius-shaped example is +explicitly documented as application policy, not a new library requirement. +The existing callback-lifetime and redaction contracts remain authoritative. +No production code or existing tests were changed during review. diff --git a/load_metadata.go b/load_metadata.go index 20e3081..1454caa 100644 --- a/load_metadata.go +++ b/load_metadata.go @@ -15,8 +15,13 @@ type resolutionInput struct { } func newResolutionInput(metadata Metadata, final *document.Node, sources []SourceInfo) resolutionInput { - frozen := cloneMetadata(metadata) - frozen.effectivePaths = effectivePathIndex(final) + frozen := Metadata{ + fields: make(map[string]FieldInfo, len(metadata.fields)), + effectivePaths: effectivePathIndex(final), + } + for pointer, info := range metadata.fields { + frozen.fields[pointer] = cloneFieldInfo(info) + } indexed := make(map[string]SourceInfo, len(sources)) for _, source := range sources { if source.ID == "" {