diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 636122c..af39822 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -224,7 +224,7 @@ unchanged, and unsupported intermediate shapes return no nodes. ## Stage 5 — Complete structural-replacement masking -Status: planned. Scope: the fallback rule and its shape-sensitive regressions. +Status: complete. Scope: the fallback rule and its shape-sensitive regressions. 1. Extend the paired traversal to detect incompatible present kinds at the root or nested positions. Use strict-descendant classification under the mismatch @@ -243,6 +243,13 @@ Validate: focused `Diff` tests, existing rendering tests, and the everyday check. Done: every shape branch has defined disclosure behavior without blanket masking of otherwise compatible public regions. +Evidence: `go test ./internal/engine -run 'Diff|Effective|Snapshot'` and +`./scripts/check.sh` passed. Coverage includes both mapping/scalar directions, +mapping/sequence replacements with numeric path tokens, null/container changes, +nested sequence mismatches, historical-only descendant classification, +unrelated sensitive siblings, targeted masking through absence, and fallback +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. diff --git a/internal/engine/diff_snapshot.go b/internal/engine/diff_snapshot.go index df87e81..1ead9dc 100644 --- a/internal/engine/diff_snapshot.go +++ b/internal/engine/diff_snapshot.go @@ -7,13 +7,27 @@ import ( ) type pairedDiffSnapshot struct { - left *document.Node - right *document.Node - leftRedacted bool - rightRedacted bool + left *document.Node + right *document.Node + leftRedacted bool + rightRedacted bool + requiresFallback bool } func preparePairedDiffSnapshot(left, right *document.Node, path Path, sensitivity *diffSensitivity) (pairedDiffSnapshot, bool) { + paired, supported := preparePairedDiffNodes(left, right, path, sensitivity) + if !supported || !paired.requiresFallback { + return paired, supported + } + return pairedDiffSnapshot{ + left: redactedDiffNode(left), + right: redactedDiffNode(right), + leftRedacted: left != nil, + rightRedacted: right != nil, + }, true +} + +func preparePairedDiffNodes(left, right *document.Node, path Path, sensitivity *diffSensitivity) (pairedDiffSnapshot, bool) { if left == nil && right == nil { return pairedDiffSnapshot{}, true } @@ -26,7 +40,10 @@ func preparePairedDiffSnapshot(left, right *document.Node, path Path, sensitivit }, true } if left != nil && right != nil && left.Kind != right.Kind { - return pairedDiffSnapshot{}, false + if sensitivity.hasStrictDescendant(path.String()) { + return pairedDiffSnapshot{requiresFallback: true}, true + } + return pairedDiffSnapshot{left: left, right: right}, true } kind := document.Null @@ -42,7 +59,7 @@ func preparePairedDiffSnapshot(left, right *document.Node, path Path, sensitivit return preparePairedDiffSequence(left, right, path, sensitivity) default: if sensitivity.hasStrictDescendant(path.String()) { - return pairedDiffSnapshot{}, false + return pairedDiffSnapshot{requiresFallback: true}, true } return pairedDiffSnapshot{left: left, right: right}, true } @@ -87,10 +104,13 @@ func preparePairedDiffMapping(left, right *document.Node, path Path, sensitivity if rightPresent { rightChild = rightEntry.Value } - child, supported := preparePairedDiffSnapshot(leftChild, rightChild, path.Key(key), sensitivity) + child, supported := preparePairedDiffNodes(leftChild, rightChild, path.Key(key), sensitivity) if !supported { return pairedDiffSnapshot{}, false } + if child.requiresFallback { + return pairedDiffSnapshot{requiresFallback: true}, true + } if leftPresent { leftEntry.Value = child.left leftCopy.Mapping = append(leftCopy.Mapping, leftEntry) @@ -135,10 +155,13 @@ func preparePairedDiffSequence(left, right *document.Node, path Path, sensitivit if right != nil && index < len(right.Sequence) { rightChild = right.Sequence[index] } - child, supported := preparePairedDiffSnapshot(leftChild, rightChild, path.Index(index), sensitivity) + child, supported := preparePairedDiffNodes(leftChild, rightChild, path.Index(index), sensitivity) if !supported { return pairedDiffSnapshot{}, false } + if child.requiresFallback { + return pairedDiffSnapshot{requiresFallback: true}, true + } if left != nil && index < len(left.Sequence) { leftCopy.Sequence[index] = child.left } diff --git a/internal/engine/diff_snapshot_test.go b/internal/engine/diff_snapshot_test.go index aaebfd3..83fb962 100644 --- a/internal/engine/diff_snapshot_test.go +++ b/internal/engine/diff_snapshot_test.go @@ -166,14 +166,152 @@ func TestPreparePairedDiffSnapshotDoesNotInterpretMarkerLookingInput(t *testing. assertDiffStringNode(t, paired.right, "[REDACTED]") } -func TestPreparePairedDiffSnapshotWithholdsUnsupportedIntermediateShape(t *testing.T) { - left := parseCompositionDocument(t, "left", "token: undisclosed-canary\npublic: safe\n") - right := &document.Node{Kind: document.String, Text: "replacement"} +func TestPreparePairedDiffSnapshotMasksSensitiveStructuralReplacements(t *testing.T) { + tests := []struct { + name string + left, right string + path Path + selector string + }{ + { + name: "mapping to scalar", + left: "changed: {token: old-secret, public: old}\n", + right: "changed: new-scalar\n", + path: Keys("changed"), + selector: "/changed/token", + }, + { + name: "scalar to mapping", + left: "changed: old-scalar\n", + right: "changed: {token: new-secret, public: new}\n", + path: Keys("changed"), + selector: "/changed/token", + }, + { + name: "mapping to sequence with numeric paths", + left: "changed: {'0': {token: old-secret}}\n", + right: "changed: [{token: new-secret}]\n", + path: Keys("changed"), + selector: "/changed/0/token", + }, + { + name: "sequence to mapping with numeric paths", + left: "changed: [{token: old-secret}]\n", + right: "changed: {'0': {token: new-secret}}\n", + path: Keys("changed"), + selector: "/changed/0/token", + }, + { + name: "null to container", + left: "changed: null\n", + right: "changed: {token: new-secret}\n", + path: Keys("changed"), + selector: "/changed/token", + }, + { + name: "container to null", + left: "changed: {token: old-secret}\n", + right: "changed: null\n", + path: Keys("changed"), + selector: "/changed/token", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + leftRoot := parseCompositionDocument(t, "left", test.left) + rightRoot := parseCompositionDocument(t, "right", test.right) + left := mappingValue(t, leftRoot, test.path.segments[0].key) + right := mappingValue(t, rightRoot, test.path.segments[0].key) + paired, supported := preparePairedDiffSnapshot( + left, right, test.path, newDiffSensitivity(nil, map[string]bool{test.selector: true}), + ) + assertWholeRegionFallback(t, paired, supported, left, right) + }) + } +} + +func TestPreparePairedDiffSnapshotMasksOriginalRegionForNestedSequenceMismatch(t *testing.T) { + leftRoot := parseCompositionDocument(t, "left", "items: [{branch: {token: old-secret, public: old}}]\n") + rightRoot := parseCompositionDocument(t, "right", "items: [{branch: new-scalar}]\n") + left := mappingValue(t, leftRoot, "items") + right := mappingValue(t, rightRoot, "items") paired, supported := preparePairedDiffSnapshot( - left, right, Path{}, newDiffSensitivity(nil, map[string]bool{"/token": true}), + left, right, Keys("items"), newDiffSensitivity(nil, map[string]bool{"/items/0/branch/token": true}), ) - if supported || paired.left != nil || paired.right != nil { - t.Fatalf("unsupported shape returned potentially unsafe nodes: %#v, supported=%v", paired, supported) + assertWholeRegionFallback(t, paired, supported, left, right) +} + +func TestPreparePairedDiffSnapshotUsesHistoricalDescendantClassification(t *testing.T) { + leftRoot := parseCompositionDocument(t, "left", "changed: before\n") + rightRoot := parseCompositionDocument(t, "right", "changed: after\n") + left := mappingValue(t, leftRoot, "changed") + right := mappingValue(t, rightRoot, "changed") + paired, supported := preparePairedDiffSnapshot( + left, right, Keys("changed"), newDiffSensitivity(map[string]bool{"/changed/removed/secret": true}, nil), + ) + assertWholeRegionFallback(t, paired, supported, left, right) +} + +func TestPreparePairedDiffSnapshotFallbackPreservesAbsentReportedSide(t *testing.T) { + right := parseCompositionDocument(t, "right", "branch: scalar\n") + paired, supported := preparePairedDiffSnapshot( + nil, right, Keys("added"), newDiffSensitivity(nil, map[string]bool{"/added/branch/secret": true}), + ) + if !supported || paired.left != nil || paired.leftRedacted || !paired.rightRedacted { + t.Fatalf("fallback presence/redaction = %#v, supported=%v", paired, supported) + } + assertDiffStringNode(t, paired.right, "[REDACTED]") +} + +func TestPreparePairedDiffSnapshotPreservesPublicReplacementBesideSensitiveSibling(t *testing.T) { + left := parseCompositionDocument(t, "left", "child: before\n") + right := &document.Node{Kind: document.String, Text: "after"} + paired, supported := preparePairedDiffSnapshot( + mappingValue(t, left, "child"), right, Keys("public"), + newDiffSensitivity(nil, map[string]bool{"/private/token": true}), + ) + if !supported || paired.left != mappingValue(t, left, "child") || paired.right != right || paired.leftRedacted || paired.rightRedacted { + t.Fatalf("unrelated classification changed public replacement: %#v, supported=%v", paired, supported) + } +} + +func TestPreparePairedDiffSnapshotTargetsRedactionAcrossAbsence(t *testing.T) { + left := parseCompositionDocument(t, "left", "items: []\n") + right := parseCompositionDocument(t, "right", "items: [{token: new-secret, public: visible}]\n") + leftItems := mappingValue(t, left, "items") + rightItems := mappingValue(t, right, "items") + paired, supported := preparePairedDiffSnapshot( + leftItems, rightItems, Keys("items"), newDiffSensitivity(nil, map[string]bool{"/items/0/token": true}), + ) + if !supported || paired.left == nil || paired.right == nil || paired.leftRedacted || !paired.rightRedacted { + t.Fatalf("targeted sequence redaction = %#v, supported=%v", paired, supported) + } + if len(paired.left.Sequence) != 0 || len(paired.right.Sequence) != 1 { + t.Fatalf("sequence lengths = %d, %d; want 0, 1", len(paired.left.Sequence), len(paired.right.Sequence)) + } + assertDiffStringNode(t, mappingValue(t, paired.right.Sequence[0], "token"), "[REDACTED]") + assertDiffStringNode(t, mappingValue(t, paired.right.Sequence[0], "public"), "visible") +} + +func assertWholeRegionFallback(t *testing.T, paired pairedDiffSnapshot, supported bool, left, right *document.Node) { + t.Helper() + if !supported { + t.Fatal("sensitive incompatible shape was not handled") + } + if (left == nil) != (paired.left == nil) || (right == nil) != (paired.right == nil) { + t.Fatalf("fallback changed root presence: %#v", paired) + } + if left != nil { + assertDiffStringNode(t, paired.left, "[REDACTED]") + if !paired.leftRedacted { + t.Fatal("present left region was not marked redacted") + } + } + if right != nil { + assertDiffStringNode(t, paired.right, "[REDACTED]") + if !paired.rightRedacted { + t.Fatal("present right region was not marked redacted") + } } }