From be2ba9a20b4032ed2430684184c4fdb820acac10 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 6 Sep 2026 19:11:30 -0500 Subject: [PATCH] Validate metadata queries against semantic path shapes --- docs/config.md | 5 ++- docs/roadmap/remediation.md | 6 +++ explain.go | 14 +++++-- load.go | 1 + load_metadata.go | 8 ++++ metadata.go | 84 +++++++++++++++++++++++++++++++++++-- metadata_test.go | 49 +++++++++++++++++++++- 7 files changed, 158 insertions(+), 9 deletions(-) diff --git a/docs/config.md b/docs/config.md index 2369a98..4171d22 100644 --- a/docs/config.md +++ b/docs/config.md @@ -87,7 +87,10 @@ successful result. Callback panics remain programming failures. ### Layered metadata -`Result.Metadata` describes every final canonical path. Exact surviving input paths +`Result.Metadata` describes every final canonical path. Queries retain explicit +key/index distinctions using actual effective and historical container shapes; +only parsed numeric tokens are resolved contextually. A descendant that never +existed is not historical merely because an atomic ancestor was replaced. Exact surviving input paths are explicit and have a base or profile origin; omitted paths have a default origin. An explicit parent mapping does not make omitted children explicit. Null state and available source coordinates are retained. `History` records the Go-zero or application diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index f7dc47d..cf1ef8a 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -157,6 +157,12 @@ agree while preserving reasons, dependencies, origins and redaction. ## 6. P2 — Preserve semantic path kinds and reject nonexistent explanation paths +Status: complete. Metadata retains value-free effective/historical path shapes; +queries validate explicit segment kinds, and explanation omits absent-to-absent +operations for paths that never existed. Regressions cover wrong-kind and parsed +numeric queries, out-of-range indices, no-output failures and removed descendants. +Full tests pass with the issue-8 TMPDIR workaround; vet, build and whitespace pass. + Evidence: [metadata.go](../../metadata.go) indexes queries solely by `Path.String`, losing explicit key/index distinctions. [explain.go](../../explain.go), `applicationOperationPath`, considers an atomic ancestor relevant even when the diff --git a/explain.go b/explain.go index 8b82842..9b7ba51 100644 --- a/explain.go +++ b/explain.go @@ -86,7 +86,7 @@ func cloneSecretReferences(values map[string]string) map[string]string { // Explain returns safe effective and chronological values for a known final or // historical path. Unknown and malformed paths return false. func (r *Result[T]) Explain(path Path) (Explanation, bool) { - if r == nil || r.final == nil || r.baseline == nil || !path.valid() { + if r == nil || r.final == nil || r.baseline == nil || !r.metadata.hasHistoricalPath(path) { return Explanation{}, false } explanation := Explanation{Path: path.clone(), Effective: valueSnapshot(r.final, path, r.secrets)} @@ -110,9 +110,13 @@ func (r *Result[T]) Explain(path Path) (Explanation, bool) { if !relevant { continue } + after := valueSnapshot(working, path, r.secrets) + if !before.Present && !after.Present { + continue + } explanation.History = append(explanation.History, ValueChange{ Before: before, - After: valueSnapshot(working, path, r.secrets), + After: after, Origin: historyApplicationOrigin(application, operationPath), }) } @@ -144,9 +148,13 @@ func (r *Result[T]) Explain(path Path) (Explanation, bool) { if !relevant { continue } + after := valueSnapshot(working, path, r.secrets) + if !before.Present && !after.Present { + continue + } explanation.History = append(explanation.History, ValueChange{ Before: before, - After: valueSnapshot(working, path, r.secrets), + After: after, Origin: cloneOrigin(snapshot.change.Origin), Reasons: append([]string(nil), snapshot.change.Reasons...), Dependencies: clonePaths(snapshot.change.Dependencies), diff --git a/load.go b/load.go index 5a6308c..0e00083 100644 --- a/load.go +++ b/load.go @@ -281,6 +281,7 @@ func loadWithSecretLookup[T any](source Source, lookup secretLookup, supplied .. return nil, accumulator.loadError() } configuration = published.Interface().(T) + finalizeMetadataPaths(&metadata, final, applications, decoded.baseline, decoded.final) secrets := classifySecretPaths(schema, options.Definition.Secret, metadata, final, applications, decoded.baseline, decoded.final) return &Result[T]{ Config: configuration, diff --git a/load_metadata.go b/load_metadata.go index 7f5833d..3edebdd 100644 --- a/load_metadata.go +++ b/load_metadata.go @@ -184,6 +184,14 @@ func cloneMetadata(metadata Metadata) Metadata { fields: make(map[string]FieldInfo, len(metadata.fields)), history: make(map[string][]Change, len(metadata.history)), } + cloned.effectivePaths = make(map[string]Path, len(metadata.effectivePaths)) + for pointer, path := range metadata.effectivePaths { + cloned.effectivePaths[pointer] = path.clone() + } + cloned.historicalPaths = make(map[string][]Path, len(metadata.historicalPaths)) + for pointer, paths := range metadata.historicalPaths { + cloned.historicalPaths[pointer] = clonePaths(paths) + } for path, info := range metadata.fields { cloned.fields[path] = cloneFieldInfo(info) } diff --git a/metadata.go b/metadata.go index 22a4734..e777862 100644 --- a/metadata.go +++ b/metadata.go @@ -1,16 +1,20 @@ package configkit +import "gitea.maximumdirect.net/eric/ConfigKit/internal/document" + // Metadata provides detached effective presence/origin and chronological history // queries for a load. Its zero value contains no paths. type Metadata struct { - fields map[string]FieldInfo - history map[string][]Change + effectivePaths map[string]Path + historicalPaths map[string][]Path + fields map[string]FieldInfo + history map[string][]Change } // Lookup returns information for an effective path. Unknown and invalid paths // return false. func (m Metadata) Lookup(path Path) (FieldInfo, bool) { - if !path.valid() { + if !path.valid() || !matchesPathShape(path, m.effectivePaths[path.String()]) { return FieldInfo{}, false } info, ok := m.fields[path.String()] @@ -32,7 +36,7 @@ func (m Metadata) IsExplicit(path Path) bool { // Removed historical paths can have history even though Lookup returns false. // Unknown and invalid paths return nil. func (m Metadata) History(path Path) []Change { - if !path.valid() { + if !m.hasHistoricalPath(path) { return nil } history, ok := m.history[path.String()] @@ -45,3 +49,75 @@ func (m Metadata) History(path Path) []Change { } return cloned } + +// Textual pointers are lookup keys, but explicit key/index kinds remain part of +// the query. Candidate paths are constructed from actual document containers. +func matchesPathShape(query, candidate Path) bool { + if !query.valid() || query.String() != candidate.String() || len(query.segments) != len(candidate.segments) { + return false + } + for i, segment := range query.segments { + actual := candidate.segments[i] + if actual.kind == pathSegmentIndex { + index, ok := schemaIndex(segment) + if !ok || index != actual.index { + return false + } + } else if segment.kind == pathSegmentIndex { + return false + } + } + return true +} + +func (m Metadata) hasHistoricalPath(path Path) bool { + if !path.valid() { + return false + } + for _, candidate := range m.historicalPaths[path.String()] { + if matchesPathShape(path, candidate) { + return true + } + } + return false +} + +func finalizeMetadataPaths(m *Metadata, final *document.Node, applications []historyApplication, earlier ...*document.Node) { + m.effectivePaths = make(map[string]Path) + m.historicalPaths = make(map[string][]Path) + walkDocumentPaths(final, Path{}, func(path Path) { m.effectivePaths[path.String()] = path.clone() }) + add := func(path Path) { + pointer := path.String() + for _, prior := range m.historicalPaths[pointer] { + if matchesPathShape(path, prior) { + return + } + } + m.historicalPaths[pointer] = append(m.historicalPaths[pointer], path.clone()) + } + walkDocumentPaths(final, Path{}, add) + for _, tree := range earlier { + walkDocumentPaths(tree, Path{}, add) + } + for _, application := range applications { + // Overrides have mapping-only prefixes; parsed tokens are keys in this layer. + prefix := Path{} + for _, segment := range application.path.segments { + prefix = prefix.Key(segment.key) + } + walkDocumentPaths(application.value, prefix, add) + } +} + +func walkDocumentPaths(node *document.Node, path Path, visit func(Path)) { + if node == nil { + return + } + visit(path) + for _, entry := range node.Mapping { + walkDocumentPaths(entry.Value, path.Key(entry.Key), visit) + } + for i, child := range node.Sequence { + walkDocumentPaths(child, path.Index(i), visit) + } +} diff --git a/metadata_test.go b/metadata_test.go index 2b771b2..da86d9d 100644 --- a/metadata_test.go +++ b/metadata_test.go @@ -1,12 +1,17 @@ package configkit -import "testing" +import ( + "bytes" + "testing" +) func TestMetadataQueriesReturnDetachedRecords(t *testing.T) { t.Parallel() path := Keys("service", "workers") metadata := Metadata{ + effectivePaths: map[string]Path{path.String(): path}, + historicalPaths: map[string][]Path{path.String(): {path}}, fields: map[string]FieldInfo{ path.String(): { Explicit: true, @@ -56,3 +61,45 @@ func TestMetadataQueriesReturnDetachedRecords(t *testing.T) { t.Fatal("invalid path produced metadata") } } + +func TestQueriesRespectActualHistoricalAndEffectiveContainers(t *testing.T) { + result, err := Load[map[string]any](Bytes("base", []byte("items: [1]\nm: {'0': 2}\nx: 3\nretired: {child: 4}\n")), Options[map[string]any]{Overrides: []Override{{Path: Keys("retired"), Value: "removed", Flag: "--retired"}}}) + if err != nil { + t.Fatal(err) + } + for _, path := range []Path{Keys("items", "0"), Keys("m").Index(0), Keys("x", "missing"), Keys("items").Index(8)} { + if _, ok := result.Metadata().Lookup(path); ok { + t.Fatalf("invalid lookup %s", path) + } + if result.Metadata().History(path) != nil { + t.Fatalf("invalid history %s", path) + } + if _, ok := result.Explain(path); ok { + t.Fatalf("invalid explanation %s", path) + } + var out bytes.Buffer + if err := result.WriteExplanation(&out, path); err == nil || out.Len() != 0 { + t.Fatalf("invalid path wrote output: %s", path) + } + } + for _, pointer := range []string{"/items/0", "/m/0"} { + path, _ := ParsePath(pointer) + if _, ok := result.Metadata().Lookup(path); !ok { + t.Fatal("parsed path lost lookup") + } + if _, ok := result.Explain(path); !ok { + t.Fatal("parsed path lost explanation") + } + } + retired := Keys("retired", "child") + if _, ok := result.Metadata().Lookup(retired); ok { + t.Fatal("removed path remains effective") + } + if len(result.Metadata().History(retired)) == 0 { + t.Fatal("removed history missing") + } + x, ok := result.Explain(retired) + if !ok || x.Effective.Present || len(x.History) == 0 { + t.Fatal("removed explanation missing") + } +}