diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index c725a3d..1665116 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -8,7 +8,8 @@ Comparison baseline: `0dd16aa8a935645ae2847d32edd86461dc4618f0` Scope: the completed [implementation plan](implementation.md) and [interface remediation roadmap](interface-remediation.md). This report records remaining defects and newly introduced scalability problems; it does not change -the selected architecture or public contracts. Completion is recorded per finding. +the selected architecture or public contracts. All eight findings are complete; +implementation and validation evidence is recorded per finding. ## Architecture And Review Evidence @@ -387,7 +388,17 @@ should protect output equivalence without a timing assertion. ## 8. Sensitivity Preparation Expands One Dependency List Per Affected Path -Priority: P2. Status: open. +Priority: P2. Status: complete. + +Completion: ordinary dependency lists are expanded once per recorded target/ +attribution, and dependency candidates once per attribution, without descendant +dependency-list copies. Classification supplies already computed inheritance +links to candidate preparation, avoiding a second ordinary-edge construction. +`TestResolutionSensitivityKeepsLogicalEdgesWithoutSubtreeDuplication` failed +before the fix and now verifies the original edges, including a coalesced child +record, and sensitivity isolation from unrelated paths. The full ordinary +suite, full race suite, everyday check, and `git diff --check` passed. The +original public redaction/history probes also now produce the expected results. Relevant code: `sensitivityCandidates`, lines 238–247, and `sensitivityDependencies`, lines 272–286, in diff --git a/internal/engine/history_resolution_test.go b/internal/engine/history_resolution_test.go index 7049b77..5bb8051 100644 --- a/internal/engine/history_resolution_test.go +++ b/internal/engine/history_resolution_test.go @@ -7,6 +7,44 @@ import ( "gitea.maximumdirect.net/eric/ConfigKit/internal/document" ) +func TestResolutionSensitivityKeepsLogicalEdgesWithoutSubtreeDuplication(t *testing.T) { + t.Parallel() + before := parseDocumentForDecode(t, "settings: {a: old, b: old, c: old}\ncredentials: {private: p, public: q, child: r}\nunrelated: visible\n") + after := parseDocumentForDecode(t, "settings: {a: new, b: new, c: new}\n") + beforeSettings, _ := documentAtPath(before, Keys("settings")) + afterSettings, _ := documentAtPath(after, Keys("settings")) + beforeChild, _ := documentAtPath(before, Keys("settings", "a")) + afterChild, _ := documentAtPath(after, Keys("settings", "a")) + first := parseOverrideTestPath(t, "/credentials/private") + second := parseOverrideTestPath(t, "/credentials/public") + child := parseOverrideTestPath(t, "/credentials/child") + builder := newHistoryBuilder() + builder.observeBaseline(before) + builder.setScratch(before) + builder.observeResolutionEvidence([]resolutionSnapshot{ + {change: Change{Path: Keys("settings"), Origin: Origin{Layer: LayerResolution, Operation: ChangeNormalization}, Dependencies: []Path{first, second}}, before: beforeSettings, after: afterSettings}, + {change: Change{Path: Keys("settings", "a"), Origin: Origin{Layer: LayerResolution, Operation: ChangeNormalization}, Dependencies: []Path{child}}, before: beforeChild, after: afterChild}, + }) + model := builder.freeze() + dependencies, inherited := model.sensitivityDependencies() + if len(dependencies) != 3 || len(inherited) != 0 { + t.Fatalf("got %d ordinary and %d inherited edges; want the three original dependencies", len(dependencies), len(inherited)) + } + for index, expected := range []struct{ target, dependency string }{ + {"/settings", first.String()}, {"/settings", second.String()}, {"/settings/a", child.String()}, + } { + if dependencies[index].target.String() != expected.target || dependencies[index].dependency.String() != expected.dependency { + t.Fatalf("edge %d lost its recorded target/dependency", index) + } + } + classified := classifySecretPaths(nil, func(path Path) bool { + return path.String() == child.String() + }, Metadata{historyModel: model}) + if !classified["/settings/a"] || classified["/settings"] || classified["/settings/b"] || classified["/unrelated"] { + t.Fatalf("original child dependency omitted by replay grouping or propagated to unrelated values: %#v", classified) + } +} + func TestHistoryResolutionRetainsOriginalShapeAttributionBehindReplayGroup(t *testing.T) { before := &document.Node{Kind: document.Mapping, Mapping: []document.Entry{{ Key: "0", Value: &document.Node{Kind: document.String, Text: "old"}, diff --git a/internal/engine/history_select.go b/internal/engine/history_select.go index dee621c..9a95b39 100644 --- a/internal/engine/history_select.go +++ b/internal/engine/history_select.go @@ -21,10 +21,9 @@ type historySelection struct { } type historySensitivityCandidate struct { - path Path - sensitive bool - classify bool - dependencies []Path + path Path + sensitive bool + classify bool } func (model *historyModel) selectHistory(path Path) []historySelection { @@ -198,28 +197,28 @@ func (model *historyModel) latestHistoricalLocation(path Path) (Location, bool) return Location{}, false } -func (model *historyModel) sensitivityCandidates() []historySensitivityCandidate { +func (model *historyModel) sensitivityCandidates(inherited []secretDependency) []historySensitivityCandidate { if model == nil { return nil } candidates := make([]historySensitivityCandidate, 0, len(model.observations)) indices := make(map[string][]int) - add := func(path Path, sensitive, classify bool) int { + seenAttributions := make(map[historyAttributionRef]struct{}) + add := func(path Path, sensitive, classify bool) { if !path.valid() { - return -1 + return } pointer := path.String() for _, index := range indices[pointer] { if slices.Equal(candidates[index].path.segments, path.segments) { candidates[index].sensitive = candidates[index].sensitive || sensitive candidates[index].classify = candidates[index].classify || classify - return index + return } } index := len(candidates) candidates = append(candidates, historySensitivityCandidate{path: path.clone(), sensitive: sensitive, classify: classify}) indices[pointer] = append(indices[pointer], index) - return index } for _, observation := range model.observations { concreteCandidate := observation.role == historyRoleControl @@ -228,23 +227,22 @@ func (model *historyModel) sensitivityCandidates() []historySensitivityCandidate concreteCandidate = true } } - index := add(observation.path, observation.sensitive, concreteCandidate) + add(observation.path, observation.sensitive, concreteCandidate) if parsed, err := ParsePath(observation.path.String()); err == nil { add(parsed, observation.sensitive, true) } - if index < 0 { + if _, seen := seenAttributions[observation.attributionRef]; seen { continue } + seenAttributions[observation.attributionRef] = struct{}{} if attribution, found := model.resolveAttribution(observation.attributionRef); found { for _, dependency := range attribution.dependencies { if dependency.valid() { - appendDistinctHistoryPath(&candidates[index].dependencies, dependency) add(dependency, false, true) } } } } - _, inherited := model.sensitivityDependencies() for _, link := range inherited { add(link.target, false, true) add(link.dependency, false, true) @@ -252,15 +250,6 @@ func (model *historyModel) sensitivityCandidates() []historySensitivityCandidate return candidates } -func appendDistinctHistoryPath(paths *[]Path, path Path) { - for _, existing := range *paths { - if slices.Equal(existing.segments, path.segments) { - return - } - } - *paths = append(*paths, path.clone()) -} - // sensitivityDependencies returns the original attribution edges and the // bidirectional links represented by inheritance payloads. It intentionally // does not derive edges from the public metadata projection, which may omit @@ -269,10 +258,22 @@ func (model *historyModel) sensitivityDependencies() (dependencies, inherited [] if model == nil { return nil, nil } + // Every affected subtree fact borrows its operation's attribution. Expand + // each logical target/dependency list once, not once for every descendant. + type dependencyAttribution struct { + ref historyAttributionRef + target string + } + seen := make(map[dependencyAttribution]struct{}) for _, observation := range model.observations { - if observation.phase == historyPhaseInheritance { + if observation.phase == historyPhaseInheritance || observation.attributionRef == noHistoryAttribution { continue } + key := dependencyAttribution{ref: observation.attributionRef, target: observation.reportedPath.String()} + if _, found := seen[key]; found { + continue + } + seen[key] = struct{}{} attribution, found := model.resolveAttribution(observation.attributionRef) if !found { continue diff --git a/internal/engine/history_select_test.go b/internal/engine/history_select_test.go index 9ef2ba0..00245c7 100644 --- a/internal/engine/history_select_test.go +++ b/internal/engine/history_select_test.go @@ -27,7 +27,9 @@ func TestSensitivityCandidatesMergeRepeatedParsedPaths(t *testing.T) { }) } } - candidates := builder.freeze().sensitivityCandidates() + model := builder.freeze() + _, inherited := model.sensitivityDependencies() + candidates := model.sensitivityCandidates(inherited) for _, path := range []Path{key, index, parsed, dependency} { count := 0 for _, candidate := range candidates { @@ -236,19 +238,20 @@ func TestHistoryModelFinalizesLocationAndSensitivityWithoutRetainingInventoryTre t.Fatalf("latest historical location = %#v, %v", got, ok) } - candidates := model.sensitivityCandidates() - var sawCandidate bool + dependencies, inherited := model.sensitivityDependencies() + candidates := model.sensitivityCandidates(inherited) + var sawCandidate, sawDependency bool for _, candidate := range candidates { - if !sameConcreteHistoryPath(candidate.path, keyPath) || !candidate.sensitive || len(candidate.dependencies) == 0 { - continue + if sameConcreteHistoryPath(candidate.path, keyPath) && candidate.sensitive { + sawCandidate = true } - if len(candidate.dependencies) != 1 || candidate.dependencies[0].String() != dependency.String() { - t.Fatalf("sensitivity dependencies = %#v", candidate.dependencies) + if slices.Equal(candidate.path.segments, dependency.segments) && candidate.classify { + sawDependency = true } - sawCandidate = true } - if !sawCandidate { - t.Fatal("sensitivity candidate omitted the concrete historical path") + if !sawCandidate || !sawDependency || len(dependencies) != 1 || + dependencies[0].target.String() != "/shape" || dependencies[0].dependency.String() != dependency.String() { + t.Fatal("sensitivity evidence omitted the concrete historical path or its recorded dependency") } } diff --git a/internal/engine/secret.go b/internal/engine/secret.go index 0edb70d..b20e8fb 100644 --- a/internal/engine/secret.go +++ b/internal/engine/secret.go @@ -34,7 +34,7 @@ func classifySecretPathsWithEvidence(schema *schemaDescriptor, predicate func(Pa var dependencies, inherited []secretDependency if model := metadata.historyEvidence(); model != nil { dependencies, inherited = model.sensitivityDependencies() - for _, candidate := range model.sensitivityCandidates() { + for _, candidate := range model.sensitivityCandidates(inherited) { pointer := candidate.path.String() paths[pointer] = candidate.path.clone() if candidate.classify { @@ -43,10 +43,6 @@ func classifySecretPathsWithEvidence(schema *schemaDescriptor, predicate func(Pa if candidate.sensitive { secret[pointer] = true } - for _, dependency := range candidate.dependencies { - paths[dependency.String()] = dependency.clone() - addCandidate(dependency) - } } for _, edge := range dependencies { paths[edge.target.String()] = edge.target.clone()