diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 50f11cc..cb4237c 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -310,7 +310,7 @@ Validate: `go test ./internal/engine -run 'Secret|Sensitivity|Inheritance|Histor relevant root facade secret/inheritance tests, engine race tests, and the everyday check. Record collection and propagation outcomes separately. -## Stage 12: Index CLI overlaps and validation reads +## Stage 12: Index CLI overlaps and validation reads — Complete Roadmap candidate 4. Owner: engine `override.go`. diff --git a/internal/engine/override.go b/internal/engine/override.go index ae620eb..f41981c 100644 --- a/internal/engine/override.go +++ b/internal/engine/override.go @@ -50,12 +50,16 @@ func prepareOverrides(base *composedDocument, sources []SourceInfo, overrides [] } seen := make(map[string]int, len(overrides)) - pathAccepted := make([]bool, len(overrides)) + var acceptedPaths pathPrefixIndex + var baseLookup *documentLookup + if len(overrides) != 0 { + baseLookup = buildDocumentLookup(base.root) + } var usedBytes int64 usedNodes := 0 for index, override := range overrides { location := &Location{SourceID: prepared[index].source.ID} - if issue := validateOverridePath(override.Path, base.root); issue != nil { + if issue := validateOverridePath(override.Path, base.root, baseLookup); issue != nil { issue.Primary = location diagnostics = append(diagnostics, *issue) continue @@ -70,7 +74,7 @@ func prepareOverrides(base *composedDocument, sources []SourceInfo, overrides [] continue } seen[pointer] = index - if conflicting := overlappingOverride(overrides[:index], pathAccepted[:index], override.Path); conflicting >= 0 { + if conflicting, overlaps := acceptedPaths.earliestIntersectingID(pointer); overlaps { diagnostics = append(diagnostics, Diagnostic{ Code: CodeOverrideOverlap, Severity: SeverityError, Path: override.Path.clone(), Message: "configuration override paths overlap", Primary: location, @@ -92,7 +96,7 @@ func prepareOverrides(base *composedDocument, sources []SourceInfo, overrides [] }) continue } - pathAccepted[index] = true + acceptedPaths.insert(pointer, index) builder := valueBuilder{sourceID: prepared[index].source.ID, active: make(map[valueVisit]bool)} value, issue := builder.build(override.Value, override.Path, len(override.Path.segments)) @@ -150,7 +154,7 @@ func validOverrideLabel(label string) bool { return true } -func validateOverridePath(path Path, root *document.Node) *Diagnostic { +func validateOverridePath(path Path, root *document.Node, lookup *documentLookup) *Diagnostic { if !path.valid() || len(path.segments) == 0 { return &Diagnostic{Code: CodeOverrideInvalidPath, Severity: SeverityError, Path: path.clone(), Message: "configuration override path must be a valid non-root mapping path"} } @@ -165,7 +169,7 @@ func validateOverridePath(path Path, root *document.Node) *Diagnostic { if current.Kind != document.Mapping { return &Diagnostic{Code: CodeOverrideInvalidPath, Severity: SeverityError, Path: path.clone(), Message: "configuration override path traverses a non-mapping value"} } - position := documentEntryIndex(current.Mapping, segment.key) + position := lookup.find(current, segment.key) if index == len(path.segments)-1 { continue } @@ -178,15 +182,6 @@ func validateOverridePath(path Path, root *document.Node) *Diagnostic { return nil } -func overlappingOverride(previous []Override, valid []bool, path Path) int { - for index, override := range previous { - if valid[index] && (pathContains(override.Path, path) || pathContains(path, override.Path)) { - return index - } - } - return -1 -} - func pathContainsDirective(path Path) bool { for _, segment := range path.segments { if segment.key == directiveKey { diff --git a/internal/engine/override_test.go b/internal/engine/override_test.go index ea3b8f1..d3c7153 100644 --- a/internal/engine/override_test.go +++ b/internal/engine/override_test.go @@ -206,6 +206,132 @@ func TestOverrideValidationRejectsPathsOverlapValuesAndProtectedTargets(t *testi } } +func TestOverrideOverlapIndexPreservesFirstAcceptedSource(t *testing.T) { + type config struct { + Nested map[string]any `yaml:"nested"` + } + base := Bytes("override-overlap.yml", []byte("nested: {}\n")) + _, err := testLoad(base, Options[config]{Overrides: []Override{ + {Path: Keys("nested"), Value: map[string]any{}, Flag: "--nested"}, + {Path: Keys("nested", "first"), Value: "one", Flag: "--first"}, + {Path: Keys("nested", "second"), Value: "two", Flag: "--second"}, + }}) + if err == nil { + t.Fatal("load succeeded with overlapping overrides") + } + var loadErr *LoadError + if !errors.As(err, &loadErr) { + t.Fatalf("error = %T %v, want *LoadError", err, err) + } + diagnostics := loadErr.Diagnostics() + var conflicts []Diagnostic + for _, diagnostic := range diagnostics { + if diagnostic.Code == CodeOverrideOverlap { + conflicts = append(conflicts, diagnostic) + } + } + if len(conflicts) != 2 { + t.Fatalf("overlap diagnostics = %#v, want two conflicts", diagnostics) + } + for index, path := range []string{"/nested/first", "/nested/second"} { + if conflicts[index].Path.String() != path || len(conflicts[index].Related) != 1 || conflicts[index].Related[0].SourceID != "source-2" { + t.Fatalf("conflict[%d] = %#v, want path %q related to first CLI source", index, conflicts[index], path) + } + } +} + +func TestOverrideOverlapIndexIncludesAcceptedPathWithInvalidValue(t *testing.T) { + type config struct { + Nested map[string]any `yaml:"nested"` + } + base := Bytes("override-value-overlap.yml", []byte("nested: {}\n")) + _, err := testLoad(base, Options[config]{Overrides: []Override{ + {Path: Keys("nested"), Value: time.Second, Flag: "--nested"}, + {Path: Keys("nested", "child"), Value: "value", Flag: "--child"}, + }}) + if err == nil { + t.Fatal("load succeeded with an invalid value and overlapping path") + } + var loadErr *LoadError + if !errors.As(err, &loadErr) { + t.Fatalf("error = %T %v, want *LoadError", err, err) + } + var invalidValue, overlap bool + for _, diagnostic := range loadErr.Diagnostics() { + switch diagnostic.Code { + case CodeOverrideInvalidValue: + invalidValue = diagnostic.Path.String() == "/nested" + case CodeOverrideOverlap: + overlap = diagnostic.Path.String() == "/nested/child" && len(diagnostic.Related) == 1 && diagnostic.Related[0].SourceID == "source-2" + } + } + if !invalidValue || !overlap { + t.Fatalf("diagnostics = %#v, want invalid parent value and child overlap related to its source", loadErr.Diagnostics()) + } +} + +func TestRejectedOverridePathDoesNotCauseOverlap(t *testing.T) { + type config struct { + Scalar string `yaml:"scalar"` + } + base := Bytes("override-rejected-overlap.yml", []byte("scalar: value\n")) + _, err := testLoad(base, Options[config]{Overrides: []Override{ + {Path: Keys("scalar", "child"), Value: "invalid", Flag: "--child"}, + {Path: Keys("scalar"), Value: "valid", Flag: "--scalar"}, + }}) + if err == nil { + t.Fatal("load succeeded with a path traversing a scalar") + } + var loadErr *LoadError + if !errors.As(err, &loadErr) { + t.Fatalf("error = %T %v, want *LoadError", err, err) + } + diagnostics := loadErr.Diagnostics() + if len(diagnostics) != 1 || diagnostics[0].Code != CodeOverrideInvalidPath || diagnostics[0].Path.String() != "/scalar/child" { + t.Fatalf("diagnostics = %#v, want only the invalid descendant path", diagnostics) + } +} + +func TestVersionRejectedOverridePathDoesNotCauseOverlap(t *testing.T) { + type config struct { + Nested map[string]any `yaml:"nested"` + } + base := Bytes("override-version-overlap.yml", []byte("nested: {}\n")) + version := &VersionRule{Path: Keys("nested", "locked"), Check: func(Version, *Validation) error { return nil }} + _, err := testLoad(base, Options[config]{ + Definition: Definition[config]{Version: version}, + Overrides: []Override{ + {Path: Keys("nested"), Value: map[string]any{}, Flag: "--nested"}, + {Path: Keys("nested", "other"), Value: "allowed", Flag: "--other"}, + }, + }) + if err == nil { + t.Fatal("load succeeded despite an override of a schema-version ancestor") + } + var loadErr *LoadError + if !errors.As(err, &loadErr) { + t.Fatalf("error = %T %v, want *LoadError", err, err) + } + diagnostics := loadErr.Diagnostics() + if len(diagnostics) != 1 || diagnostics[0].Code != CodeOverrideVersion || diagnostics[0].Path.String() != "/nested" { + t.Fatalf("diagnostics = %#v, want only the protected ancestor path", diagnostics) + } +} + +func TestEscapedOverrideKeysStayDistinctFromMultipleSegments(t *testing.T) { + type config struct { + Nested map[string]any `yaml:"nested"` + } + base := Bytes("override-escaped-keys.yml", []byte("nested: {}\n")) + _, err := testLoad(base, Options[config]{Overrides: []Override{ + {Path: Keys("nested", "a/b"), Value: "single key", Flag: "--single"}, + {Path: Keys("nested", "a", "b"), Value: "nested key", Flag: "--nested"}, + }}) + if err != nil { + t.Fatalf("load rejected distinct escaped path tokens: %v", err) + } +} + func TestOverrideCannotReplaceSchemaVersionAncestor(t *testing.T) { type schemaConfig struct { Version string `yaml:"version"`