diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index f6ec10e..edaf725 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -280,7 +280,7 @@ classification but no changed region. ## Stage 7 — Assemble the complete engine diff operation -Status: planned. Scope: internal-facing records, serialization, and real loads. +Status: complete. Scope: internal-facing records, serialization, and real loads. 1. Define engine `Difference` with `Path`, `Left`, `Right`, and `HiddenChanged`, and the operation over two `*Inspection` values. Wire Stages 2–6 together: @@ -306,6 +306,15 @@ Validate: `go test ./internal/engine -run 'Diff|Explain|History|Secret|Independe comparison works through the engine with no loading or existing inspection regression. Do not expose a partial public API earlier. +Evidence: focused engine tests, `go test -race ./internal/engine`, and +`./scripts/check.sh` passed. Real-load coverage verifies typed secret references, +dynamic values, defaults, recorded resolution and dependency-derived sensitivity, +historical secret removal/replacement, credential rotation without reference +change, reference-name changes, optional nil references, and frozen state after +environment mutation. Effective rendering, explanation, metadata lookup, +independent loads, detached repeated results, unavailable-inspection errors, and +value-free serialization errors remain covered. + ## Stage 8 — Add the root-owned public facade Status: planned. Scope: public declarations, conversion, and external tests. diff --git a/internal/engine/diff.go b/internal/engine/diff.go index 89b7373..19f4a54 100644 --- a/internal/engine/diff.go +++ b/internal/engine/diff.go @@ -1,12 +1,24 @@ package engine import ( + "bytes" "errors" + "io" "gitea.maximumdirect.net/eric/ConfigKit/internal/document" + yamladapter "gitea.maximumdirect.net/eric/ConfigKit/internal/yaml" ) var errDiffInspectionUnavailable = errors.New("configkit: diff inspection unavailable") +var errDiffSnapshotUnavailable = errors.New("configkit: diff snapshot unavailable") + +// Difference describes one canonical changed region with safe snapshots. +type Difference struct { + Path Path + Left ValueSnapshot + Right ValueSnapshot + HiddenChanged bool +} type diffRegion struct { path Path @@ -34,3 +46,53 @@ func prepareDiffRegions(left, right *Inspection) ([]diffRegion, error) { } return regions, nil } + +// Diff compares the final frozen values retained by two successful inspections. +// Values are serialized only after applying the combined sensitivity of both loads. +func Diff(left, right *Inspection) ([]Difference, error) { + regions, err := prepareDiffRegions(left, right) + if err != nil { + return nil, err + } + sensitivity := newDiffSensitivity(left.secrets, right.secrets) + differences := make([]Difference, 0, len(regions)) + for _, region := range regions { + paired, supported := preparePairedDiffSnapshot(region.left, region.right, region.path, sensitivity) + if !supported { + return nil, errDiffSnapshotUnavailable + } + leftSnapshot, err := serializeDiffSnapshot(paired.left, paired.leftRedacted) + if err != nil { + return nil, errDiffSnapshotUnavailable + } + rightSnapshot, err := serializeDiffSnapshot(paired.right, paired.rightRedacted) + if err != nil { + return nil, errDiffSnapshotUnavailable + } + differences = append(differences, Difference{ + Path: region.path, + Left: leftSnapshot, + Right: rightSnapshot, + HiddenChanged: paired.hiddenChanged, + }) + } + return differences, nil +} + +func serializeDiffSnapshot(node *document.Node, redacted bool) (ValueSnapshot, error) { + if node == nil { + return ValueSnapshot{}, nil + } + var output bytes.Buffer + if err := writeDiffDocument(&output, node); err != nil { + return ValueSnapshot{}, errDiffSnapshotUnavailable + } + return ValueSnapshot{Present: true, Redacted: redacted, YAML: output.String()}, nil +} + +func writeDiffDocument(output io.Writer, node *document.Node) error { + if err := yamladapter.Write(output, node); err != nil { + return errDiffSnapshotUnavailable + } + return nil +} diff --git a/internal/engine/diff_operation_test.go b/internal/engine/diff_operation_test.go new file mode 100644 index 0000000..ab463d5 --- /dev/null +++ b/internal/engine/diff_operation_test.go @@ -0,0 +1,282 @@ +package engine + +import ( + "bytes" + "errors" + "strings" + "testing" + + "gitea.maximumdirect.net/eric/ConfigKit/internal/document" +) + +type diffIntegrationConfig struct { + Defaulted string `yaml:"defaulted"` + Dynamic map[string]any `yaml:"dynamic"` + Credential Secret `yaml:"credential" configkit:"secret"` + Derived string `yaml:"derived"` +} + +type diffOptionalSecretConfig struct { + Optional *Secret `yaml:"optional" configkit:"secret"` +} + +func TestDiffUsesFinalValuesAndDependencyDerivedSensitivity(t *testing.T) { + var resolveCalls int + options := Options[diffIntegrationConfig]{Definition: Definition[diffIntegrationConfig]{ + Defaults: func() diffIntegrationConfig { + return diffIntegrationConfig{Defaulted: "from-default", Dynamic: map[string]any{"defaulted": "retained"}} + }, + Resolve: func(configuration *diffIntegrationConfig, resolution *Resolution) error { + resolveCalls++ + configuration.Derived = configuration.Dynamic["visible"].(string) + resolution.Record(Keys("derived"), ChangeNormalization, "derive from effective values", Keys("credential")) + return nil + }, + }} + input := func(value string) Source { + return Bytes("effective.yml", []byte("dynamic: {visible: "+value+"}\ncredential: DIFF_ENGINE_SECRET\n")) + } + t.Setenv("DIFF_ENGINE_SECRET", "credential-left-canary") + left, err := testLoad(input("before"), options) + if err != nil { + t.Fatal(err) + } + t.Setenv("DIFF_ENGINE_SECRET", "credential-right-canary") + right, err := testLoad(input("after"), options) + if err != nil { + t.Fatal(err) + } + if left.Config.Defaulted != "from-default" || right.Config.Defaulted != "from-default" { + t.Fatalf("defaulted values = %q, %q", left.Config.Defaulted, right.Config.Defaulted) + } + if !left.Inspection.secrets["/credential"] || !left.Inspection.secrets["/derived"] || + !right.Inspection.secrets["/credential"] || !right.Inspection.secrets["/derived"] { + t.Fatalf("typed or dependency-derived sensitivity was not retained: left=%#v right=%#v", left.Inspection.secrets, right.Inspection.secrets) + } + if node, present := documentAtPathWithLookup(left.Inspection.final, Keys("derived"), left.Inspection.immutableLookup); !present || node.Text != "before" { + t.Fatalf("left frozen derived value = %#v, present=%v", node, present) + } + + differences, err := Diff(left.Inspection, right.Inspection) + if err != nil { + t.Fatal(err) + } + if len(differences) != 2 || differences[0].Path.String() != "/derived" || differences[1].Path.String() != "/dynamic/visible" { + t.Fatalf("differences = %#v", differences) + } + if !differences[0].Left.Redacted || !differences[0].Right.Redacted || !differences[0].HiddenChanged || + differences[0].Left.YAML != "'[REDACTED]'\n" || differences[0].Right.YAML != "'[REDACTED]'\n" { + t.Fatalf("dependency-derived difference = %#v", differences[0]) + } + if differences[1].Left.Redacted || differences[1].Right.Redacted || differences[1].HiddenChanged || + differences[1].Left.YAML != "before\n" || differences[1].Right.YAML != "after\n" { + t.Fatalf("public dynamic difference = %#v", differences[1]) + } + for _, value := range differences { + if strings.Contains(value.Left.YAML+value.Right.YAML, "credential-") { + t.Fatal("diff snapshot exposed a resolved credential") + } + } + + var effective bytes.Buffer + if err := left.Inspection.WriteEffective(&effective); err != nil { + t.Fatal(err) + } + if strings.Contains(effective.String(), "credential-left-canary") { + t.Fatal("standalone effective rendering exposed a credential after diff") + } + explanation, ok := left.Inspection.Explain(Keys("derived")) + if !ok || !explanation.Effective.Redacted || explanation.Effective.YAML != "'[REDACTED]'\n" { + t.Fatalf("explanation changed after diff: %#v, ok=%v", explanation, ok) + } + if _, ok := left.Inspection.Metadata().Lookup(Keys("dynamic", "visible")); !ok { + t.Fatal("metadata lookup failed after diff") + } + if resolveCalls != 2 { + t.Fatalf("resolution callbacks ran during diff: %d calls", resolveCalls) + } + + // Returned records can be modified without affecting a later comparison. + differences[0].Path.segments[0].key = "changed-by-caller" + differences[0].Left.YAML = "changed-by-caller" + t.Setenv("DIFF_ENGINE_SECRET", "credential-after-load-canary") + repeated, err := Diff(left.Inspection, right.Inspection) + if err != nil || len(repeated) != 2 || repeated[0].Path.String() != "/derived" || repeated[0].Left.YAML != "'[REDACTED]'\n" { + t.Fatalf("repeated diff = %#v, err = %v", repeated, err) + } + if resolveCalls != 2 { + t.Fatalf("repeated diff reran resolution: %d calls", resolveCalls) + } + + independent, err := testLoad(input("before"), options) + if err != nil { + t.Fatal(err) + } + if resolveCalls != 3 { + t.Fatalf("independent load did not retain normal resolution behavior: %d calls", resolveCalls) + } + if _, err := Diff(left.Inspection, independent.Inspection); err != nil { + t.Fatalf("independent load comparison = %v", err) + } +} + +func TestDiffUsesReferenceNamesAndFrozenSecretState(t *testing.T) { + loadReference := func(name string) *testResult[diffOptionalSecretConfig] { + t.Helper() + result, err := testLoad[diffOptionalSecretConfig](Bytes("secret-reference.yml", []byte("optional: "+name+"\n"))) + if err != nil { + t.Fatal(err) + } + return result + } + t.Setenv("DIFF_ROTATING_SECRET", "first-credential-canary") + left := loadReference("DIFF_ROTATING_SECRET") + t.Setenv("DIFF_ROTATING_SECRET", "second-credential-canary") + right := loadReference("DIFF_ROTATING_SECRET") + t.Setenv("DIFF_ROTATING_SECRET", "changed-after-load-canary") + differences, err := Diff(left.Inspection, right.Inspection) + if err != nil || len(differences) != 0 { + t.Fatalf("credential rotation behind one reference = %#v, err = %v", differences, err) + } + if node, present := documentAtPathWithLookup(left.Inspection.final, Keys("optional"), left.Inspection.immutableLookup); !present || node.Text != "DIFF_ROTATING_SECRET" { + t.Fatalf("frozen reference = %#v, present=%v", node, present) + } + if node, present := documentAtPathWithLookup(right.Inspection.final, Keys("optional"), right.Inspection.immutableLookup); !present || node.Text != "DIFF_ROTATING_SECRET" { + t.Fatalf("right frozen reference = %#v, present=%v", node, present) + } + + t.Setenv("DIFF_OTHER_SECRET", "other-credential-canary") + other := loadReference("DIFF_OTHER_SECRET") + differences, err = Diff(left.Inspection, other.Inspection) + if err != nil || len(differences) != 1 || differences[0].Path.String() != "/optional" { + t.Fatalf("reference-name change = %#v, err = %v", differences, err) + } + if !differences[0].Left.Present || !differences[0].Right.Present || !differences[0].Left.Redacted || + !differences[0].Right.Redacted || !differences[0].HiddenChanged || + differences[0].Left.YAML != "'[REDACTED]'\n" || differences[0].Right.YAML != "'[REDACTED]'\n" { + t.Fatalf("classified reference-name diff = %#v", differences[0]) + } + + absent, err := testLoad[diffOptionalSecretConfig](Bytes("absent-secret.yml", []byte("{}\n"))) + if err != nil { + t.Fatal(err) + } + if absent.Config.Optional != nil { + t.Fatalf("missing optional reference decoded as %#v", absent.Config.Optional) + } + if node, present := documentAtPathWithLookup(absent.Inspection.final, Keys("optional"), absent.Inspection.immutableLookup); !present || node.Kind != document.Null { + t.Fatalf("optional nil projection = %#v, present=%v", node, present) + } + differences, err = Diff(absent.Inspection, left.Inspection) + if err != nil || len(differences) != 1 || differences[0].Path.String() != "/optional" { + t.Fatalf("optional absence diff = %#v, err = %v", differences, err) + } + if !differences[0].Left.Present || !differences[0].Left.Redacted || + !differences[0].Right.Present || !differences[0].Right.Redacted || !differences[0].HiddenChanged { + t.Fatalf("optional reference change snapshots = %#v", differences[0]) + } + for _, value := range differences { + if strings.Contains(value.Left.YAML+value.Right.YAML, "credential-canary") { + t.Fatal("reference diff exposed a resolved credential") + } + } +} + +func TestDiffProtectsHistoricalSensitiveRemovalAndReplacement(t *testing.T) { + type configuration struct { + Dynamic map[string]any `yaml:"dynamic"` + } + secretPath := Keys("dynamic", "token") + secretSelector := func(path Path) bool { return path.String() == secretPath.String() } + load := func(input string, resolve func(*configuration, *Resolution) error) *testResult[configuration] { + t.Helper() + result, err := testLoad(Bytes("historical-secret.yml", []byte(input)), Options[configuration]{Definition: Definition[configuration]{ + Secret: secretSelector, + Resolve: resolve, + }}) + if err != nil { + t.Fatal(err) + } + return result + } + + t.Run("removed historical value", func(t *testing.T) { + left := load("dynamic: {token: removed-canary}\n", func(configuration *configuration, resolution *Resolution) error { + delete(configuration.Dynamic, "token") + resolution.Record(secretPath, ChangeNormalization, "remove token") + return nil + }) + right := load("dynamic: {token: current-canary}\n", nil) + if !left.Inspection.secrets[secretPath.String()] { + t.Fatal("removed historical selector was not retained") + } + if _, present := documentAtPathWithLookup(left.Inspection.final, secretPath, left.Inspection.immutableLookup); present { + t.Fatal("removed token remains in the effective left tree") + } + differences, err := Diff(left.Inspection, right.Inspection) + if err != nil || len(differences) != 1 || differences[0].Path.String() != secretPath.String() { + t.Fatalf("historical removal diff = %#v, err = %v", differences, err) + } + if differences[0].Left.Present || !differences[0].Right.Present || !differences[0].Right.Redacted || !differences[0].HiddenChanged { + t.Fatalf("historical removal snapshots = %#v", differences[0]) + } + if strings.Contains(differences[0].Left.YAML+differences[0].Right.YAML, "canary") { + t.Fatal("historical removal diff exposed a sensitive value") + } + }) + + t.Run("replaced historical value", func(t *testing.T) { + left := load("dynamic: {token: original-canary}\n", func(configuration *configuration, resolution *Resolution) error { + configuration.Dynamic["token"] = "replacement-canary" + resolution.Record(secretPath, ChangeNormalization, "replace token") + return nil + }) + right := load("dynamic: {token: other-canary}\n", nil) + if !left.Inspection.secrets[secretPath.String()] { + t.Fatal("replaced historical selector was not retained") + } + differences, err := Diff(left.Inspection, right.Inspection) + if err != nil || len(differences) != 1 || differences[0].Path.String() != secretPath.String() { + t.Fatalf("historical replacement diff = %#v, err = %v", differences, err) + } + if !differences[0].Left.Present || !differences[0].Right.Present || !differences[0].Left.Redacted || + !differences[0].Right.Redacted || !differences[0].HiddenChanged { + t.Fatalf("historical replacement snapshots = %#v", differences[0]) + } + if strings.Contains(differences[0].Left.YAML+differences[0].Right.YAML, "canary") { + t.Fatal("historical replacement diff exposed a sensitive value") + } + }) +} + +func TestDiffReturnsSafeErrorsWithoutRecords(t *testing.T) { + valid := diffTestInspection(&document.Node{Kind: document.String, Text: "safe"}) + for _, test := range []struct { + name string + left, right *Inspection + }{ + {name: "nil left", right: valid}, + {name: "nil right", left: valid}, + {name: "missing final tree", left: &Inspection{}, right: valid}, + } { + t.Run(test.name, func(t *testing.T) { + differences, err := Diff(test.left, test.right) + if differences != nil || err == nil || err != errDiffInspectionUnavailable || strings.Contains(err.Error(), "safe") { + t.Fatalf("differences, err = %#v, %v", differences, err) + } + }) + } +} + +func TestDiffSerializationErrorDoesNotExposeWriterDetails(t *testing.T) { + err := writeDiffDocument(diffErrorWriter{}, &document.Node{Kind: document.String, Text: "safe"}) + if err != errDiffSnapshotUnavailable || strings.Contains(err.Error(), "CANARY") { + t.Fatalf("serialization error = %v", err) + } +} + +type diffErrorWriter struct{} + +func (diffErrorWriter) Write([]byte) (int, error) { + return 0, errors.New("CANARY_WRITER_ERROR") +}