From 756149beaafa828852d8c44ae13167cf89f6d1bd Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 9 Oct 2026 12:24:01 +0000 Subject: [PATCH] Track hidden changes behind redaction --- docs/roadmap/implementation.md | 8 +- internal/engine/diff_hidden_test.go | 130 ++++++++++++++++++++++++++ internal/engine/diff_snapshot.go | 5 + internal/engine/diff_snapshot_test.go | 3 + 4 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 internal/engine/diff_hidden_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index af39822..f6ec10e 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -252,7 +252,7 @@ preserving an absent reported side. ## Stage 6 — Distinguish hidden changes from unchanged masked content -Status: planned. Scope: one flag threaded through the existing paired traversal. +Status: complete. Scope: one flag threaded through the existing paired traversal. 1. Have paired preparation return `HiddenChanged` alongside the safe nodes and each side's redaction state. At directly masked frontiers compare raw nodes @@ -272,6 +272,12 @@ Validate: `go test ./internal/engine -run 'Diff|Secret|Sensitivity'` and the everyday check. Done: the flag communicates withheld change without claiming that every masked child changed or revealing additional change details. +Evidence: the focused engine tests and `./scripts/check.sh` passed. Tests cover +changed raw values behind identical markers, mixed and public-only changes, +unchanged masked mapping and sequence members, masked additions/removals, +sequence-length changes, structural fallback, and equal values with differing +classification but no changed region. + ## Stage 7 — Assemble the complete engine diff operation Status: planned. Scope: internal-facing records, serialization, and real loads. diff --git a/internal/engine/diff_hidden_test.go b/internal/engine/diff_hidden_test.go new file mode 100644 index 0000000..956ee46 --- /dev/null +++ b/internal/engine/diff_hidden_test.go @@ -0,0 +1,130 @@ +package engine + +import ( + "testing" + + "gitea.maximumdirect.net/eric/ConfigKit/internal/document" +) + +func TestPreparePairedDiffSnapshotReportsWhollyMaskedChanges(t *testing.T) { + left := &document.Node{Kind: document.String, Text: "private-before"} + right := &document.Node{Kind: document.String, Text: "private-after"} + paired, supported := preparePairedDiffSnapshot( + left, right, Keys("credential"), newDiffSensitivity(nil, map[string]bool{"/credential": true}), + ) + if !supported || !paired.hiddenChanged || !paired.leftRedacted || !paired.rightRedacted { + t.Fatalf("masked change = %#v, supported=%v", paired, supported) + } + assertDiffStringNode(t, paired.left, "[REDACTED]") + assertDiffStringNode(t, paired.right, "[REDACTED]") +} + +func TestPreparePairedDiffSnapshotDetectsRawChangesBehindIdenticalMarkers(t *testing.T) { + left := parseCompositionDocument(t, "left", "token: old-canary\n") + right := parseCompositionDocument(t, "right", "token: new-canary\n") + paired, supported := preparePairedDiffSnapshot( + mappingValue(t, left, "token"), mappingValue(t, right, "token"), Keys("token"), + newDiffSensitivity(map[string]bool{"/token": true}, nil), + ) + if !supported || !paired.hiddenChanged || paired.left.Text != paired.right.Text { + t.Fatalf("identically rendered masked values lost raw change: %#v, supported=%v", paired, supported) + } +} + +func TestPreparePairedDiffSnapshotCombinesPublicAndHiddenChanges(t *testing.T) { + left := parseCompositionDocument(t, "left", "public: before\ntoken: old\n") + right := parseCompositionDocument(t, "right", "public: after\ntoken: new\n") + paired, supported := preparePairedDiffSnapshot( + left, right, Path{}, newDiffSensitivity(nil, map[string]bool{"/token": true}), + ) + if !supported || !paired.hiddenChanged { + t.Fatalf("mixed public and hidden change = %#v, supported=%v", paired, supported) + } + assertDiffStringNode(t, mappingValue(t, paired.left, "public"), "before") + assertDiffStringNode(t, mappingValue(t, paired.right, "public"), "after") + assertDiffStringNode(t, mappingValue(t, paired.left, "token"), "[REDACTED]") + assertDiffStringNode(t, mappingValue(t, paired.right, "token"), "[REDACTED]") +} + +func TestPreparePairedDiffSnapshotDoesNotFlagUnchangedMaskedValues(t *testing.T) { + left := parseCompositionDocument(t, "left", "public: before\ntoken: unchanged\n") + right := parseCompositionDocument(t, "right", "public: after\ntoken: unchanged\n") + regions, err := prepareDiffRegions(diffTestInspection(left), diffTestInspection(right)) + if err != nil || len(regions) != 1 || regions[0].path.String() != "/public" { + t.Fatalf("changed regions = %#v, err = %v; want only /public", regions, err) + } + paired, supported := preparePairedDiffSnapshot( + left, right, Path{}, newDiffSensitivity(map[string]bool{"/token": true}, nil), + ) + if !supported || !paired.leftRedacted || !paired.rightRedacted || paired.hiddenChanged { + t.Fatalf("unchanged masked values affected hidden-change state: %#v, supported=%v", paired, supported) + } + + leftSequence := parseCompositionDocument(t, "left-sequence", "items: [unchanged, before]\n") + rightSequence := parseCompositionDocument(t, "right-sequence", "items: [unchanged, after]\n") + sequence, supported := preparePairedDiffSnapshot( + mappingValue(t, leftSequence, "items"), mappingValue(t, rightSequence, "items"), Keys("items"), + newDiffSensitivity(nil, map[string]bool{"/items/0": true}), + ) + if !supported || !sequence.leftRedacted || !sequence.rightRedacted || sequence.hiddenChanged { + t.Fatalf("unchanged masked sequence member affected hidden-change state: %#v, supported=%v", sequence, supported) + } +} + +func TestPreparePairedDiffSnapshotReportsMaskedAdditionAndRemoval(t *testing.T) { + value := &document.Node{Kind: document.String, Text: "private"} + tests := []struct { + name string + left, right *document.Node + leftHidden bool + rightHidden bool + }{ + {name: "addition", right: value, rightHidden: true}, + {name: "removal", left: value, leftHidden: true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + paired, supported := preparePairedDiffSnapshot( + test.left, test.right, Keys("token"), newDiffSensitivity(nil, map[string]bool{"/token": true}), + ) + if !supported || !paired.hiddenChanged || paired.leftRedacted != test.leftHidden || paired.rightRedacted != test.rightHidden { + t.Fatalf("masked presence change = %#v, supported=%v", paired, supported) + } + if test.left == nil && paired.left != nil || test.right == nil && paired.right != nil { + t.Fatalf("absence became a value: %#v", paired) + } + }) + } +} + +func TestPreparePairedDiffSnapshotReportsHiddenSequenceLengthChange(t *testing.T) { + left := parseCompositionDocument(t, "left", "items: [public]\n") + right := parseCompositionDocument(t, "right", "items: [public, private]\n") + paired, supported := preparePairedDiffSnapshot( + mappingValue(t, left, "items"), mappingValue(t, right, "items"), Keys("items"), + newDiffSensitivity(nil, map[string]bool{"/items/1": true}), + ) + if !supported || !paired.hiddenChanged || paired.leftRedacted || !paired.rightRedacted { + t.Fatalf("masked sequence length change = %#v, supported=%v", paired, supported) + } + if len(paired.left.Sequence) != 1 || len(paired.right.Sequence) != 2 { + t.Fatalf("safe sequence lengths = %d, %d; want 1, 2", len(paired.left.Sequence), len(paired.right.Sequence)) + } + assertDiffStringNode(t, paired.right.Sequence[1], "[REDACTED]") +} + +func TestPreparePairedDiffSnapshotClassificationAloneDoesNotCreateHiddenChange(t *testing.T) { + left := parseCompositionDocument(t, "left", "token: same\n") + right := parseCompositionDocument(t, "right", "token: same\n") + regions, err := prepareDiffRegions(diffTestInspection(left), diffTestInspection(right)) + if err != nil || len(regions) != 0 { + t.Fatalf("classification-independent changed regions = %#v, err = %v", regions, err) + } + paired, supported := preparePairedDiffSnapshot( + mappingValue(t, left, "token"), mappingValue(t, right, "token"), Keys("token"), + newDiffSensitivity(map[string]bool{"/token": true}, nil), + ) + if !supported || !paired.leftRedacted || !paired.rightRedacted || paired.hiddenChanged { + t.Fatalf("classification alone reported hidden change: %#v, supported=%v", paired, supported) + } +} diff --git a/internal/engine/diff_snapshot.go b/internal/engine/diff_snapshot.go index 1ead9dc..4b8f4c6 100644 --- a/internal/engine/diff_snapshot.go +++ b/internal/engine/diff_snapshot.go @@ -12,6 +12,7 @@ type pairedDiffSnapshot struct { leftRedacted bool rightRedacted bool requiresFallback bool + hiddenChanged bool } func preparePairedDiffSnapshot(left, right *document.Node, path Path, sensitivity *diffSensitivity) (pairedDiffSnapshot, bool) { @@ -24,6 +25,7 @@ func preparePairedDiffSnapshot(left, right *document.Node, path Path, sensitivit right: redactedDiffNode(right), leftRedacted: left != nil, rightRedacted: right != nil, + hiddenChanged: true, }, true } @@ -37,6 +39,7 @@ func preparePairedDiffNodes(left, right *document.Node, path Path, sensitivity * right: redactedDiffNode(right), leftRedacted: left != nil, rightRedacted: right != nil, + hiddenChanged: !configurationValuesEqual(left, right), }, true } if left != nil && right != nil && left.Kind != right.Kind { @@ -121,6 +124,7 @@ func preparePairedDiffMapping(left, right *document.Node, path Path, sensitivity } redacted.leftRedacted = redacted.leftRedacted || child.leftRedacted redacted.rightRedacted = redacted.rightRedacted || child.rightRedacted + redacted.hiddenChanged = redacted.hiddenChanged || child.hiddenChanged } redacted.left = leftCopy redacted.right = rightCopy @@ -170,6 +174,7 @@ func preparePairedDiffSequence(left, right *document.Node, path Path, sensitivit } result.leftRedacted = result.leftRedacted || child.leftRedacted result.rightRedacted = result.rightRedacted || child.rightRedacted + result.hiddenChanged = result.hiddenChanged || child.hiddenChanged } return result, true } diff --git a/internal/engine/diff_snapshot_test.go b/internal/engine/diff_snapshot_test.go index 83fb962..75924a9 100644 --- a/internal/engine/diff_snapshot_test.go +++ b/internal/engine/diff_snapshot_test.go @@ -298,6 +298,9 @@ func assertWholeRegionFallback(t *testing.T, paired pairedDiffSnapshot, supporte if !supported { t.Fatal("sensitive incompatible shape was not handled") } + if !paired.hiddenChanged { + t.Fatal("fallback for a changed reported region did not mark hidden change") + } if (left == nil) != (paired.left == nil) || (right == nil) != (paired.right == nil) { t.Fatalf("fallback changed root presence: %#v", paired) }