Skip to content

Commit 3e8bfdd

Browse files
itcmsgrclaude
andcommitted
refactor(v1.100 Amendment 3): code-B — split into separate gatherOrphanEvidenceAmendment3 helper per auditor recommendation
Per auditor pre-draft review: implement Sub-gap 2a as a separate gatherOrphanEvidenceAmendment3() helper rather than a mode-conditional inside the existing gatherOrphanEvidence(). Mirrors the AllTrue() vs AllTrueAmendment3() split that PR #523 introduced in types.go and the decideAuthorityNFTBan() vs decideAmbiguityConflictExternal() split in engine.go. "Two helpers, shared rows, different entry-condition filling" — symmetric with the engine pattern. Refactor changes (no behavior delta vs the prior code-B commit on this branch): cmd/nftban-installer/restore_decide_evidence.go - populateSharedOrphanEvidenceRows(): private helper populates all rows EXCEPT E.2 (the entry-condition row). - gatherOrphanEvidence(): Amendment 2 — calls shared helper + sets E.2 = (Authority == AuthorityNFTBan). Restored to Amendment-2-byte-clean E.2 evaluation. - gatherOrphanEvidenceAmendment3(): Amendment 3 — calls shared helper + sets E.2 = (AuthorityAmbiguous + ConflictExternal + external == "csf"). New sibling helper. - E.13 retained at the contract-wording-exact form (Ambiguity != AmbiguityOrphanNFTBan); shared by both helpers via the populate function. cmd/nftban-installer/restore_decide.go - Dispatcher's evidence-gathering now switches on the candidate type: amd2Candidate calls gatherOrphanEvidence (unchanged from pre-Amendment-3 main); amd3Candidate calls gatherOrphanEvidenceAmendment3. - Diagnostic log line is now amendment-specific (no dual-predicate noise) — clearer audit trail per amendment. cmd/nftban-installer/restore_decide_amendment3_test.go - Renamed and reorganized test cases to test the two helpers independently: * TestGatherOrphanEvidenceAmendment3_* tests the Amendment 3 helper specifically (rejects Amendment 2 entry, accepts §62 entry, AMD3-E.2 attribution on external=ufw, omits E.12). * TestGatherOrphanEvidence_Amendment2Unchanged regression-tests the Amendment 2 helper: still strict on AuthorityNFTBan, rejects Amendment 3 entry. Why two helpers (auditor reasoning): - Audit-chain clarity: one helper per amendment, mirroring the engine.go and types.go split structure. - Each helper's CI grep gates and tests can reason about it independently. - No runtime classifier-state coupling between the dispatcher's inspection and the evidence-gathering. - Symmetric with AllTrue()/AllTrueAmendment3() in types.go. Test results (lab4): - go test ./cmd/nftban-installer/... → ok - go test ./internal/installer/restore/... → ok - go test ./... → 64 packages ok, 0 FAIL No host action. No engine.go change. No contract.md change. No new mutation surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 743bb77 commit 3e8bfdd

3 files changed

Lines changed: 255 additions & 107 deletions

File tree

cmd/nftban-installer/restore_decide.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,15 +173,17 @@ func runRestoreDecide(ctx context.Context, exec executor.Executor, sf *state.Sta
173173
panel == detect.PanelDirectAdmin &&
174174
cfg.panelAutoTakeover &&
175175
cfg.acceptOrphanNFTBan
176-
if amd2Candidate || amd3Candidate {
176+
switch {
177+
case amd2Candidate:
177178
ev := gatherOrphanEvidence(exec, log, panel, auth, probe, cfg)
178179
input.OrphanEvidence = &ev
179-
// Log each predicate's result. Both AllTrue() (Amendment 2)
180-
// and AllTrueAmendment3() (Amendment 3) are evaluated for
181-
// observability; the engine consumes whichever is appropriate
182-
// per the entry condition (decided in engine.go, not here).
183-
log.Info("restore decide: orphan-evidence gathered amd2_all_true=%v amd2_failed_row=%q amd3_all_true=%v amd3_failed_row=%q",
184-
ev.AllTrue(), ev.FailedRowID(), ev.AllTrueAmendment3(), ev.FailedRowIDAmendment3())
180+
log.Info("restore decide: orphan-evidence gathered (amendment-2 path) all_true=%v failed_row=%q",
181+
ev.AllTrue(), ev.FailedRowID())
182+
case amd3Candidate:
183+
ev := gatherOrphanEvidenceAmendment3(exec, log, panel, auth, probe, cfg)
184+
input.OrphanEvidence = &ev
185+
log.Info("restore decide: orphan-evidence gathered (amendment-3 path) all_true=%v failed_row=%q",
186+
ev.AllTrueAmendment3(), ev.FailedRowIDAmendment3())
185187
}
186188

187189
// 7. Evaluate — pure, deterministic, no side effects.

cmd/nftban-installer/restore_decide_amendment3_test.go

Lines changed: 128 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// meta:type="test"
77
// meta:owner="Antonios Voulvoulis <contact@nftban.com>"
88
// meta:created_date="2026-04-29"
9-
// meta:description="Amendment 3 dispatcher-side wiring (ExternalIndicator + reframed E.2)"
9+
// meta:description="Amendment 3 dispatcher-side wiring (ExternalIndicator + reframed E.2 + separate helper)"
1010
// meta:inventory.files="cmd/nftban-installer/restore_decide_amendment3_test.go"
1111
// meta:inventory.binaries=""
1212
// meta:inventory.env_vars=""
@@ -19,16 +19,22 @@
1919
// Confirms the dispatcher correctly:
2020
//
2121
// 1. Plumbs ClassifyResult.External into DecisionInput.ExternalIndicator
22-
// so the engine's G1/AmbiguityConflictExternal split sees the entry
23-
// condition string.
22+
// so the engine's G1/AmbiguityConflictExternal split sees the
23+
// §62 entry condition string.
2424
//
25-
// 2. Triggers gatherOrphanEvidence on the Amendment 3 quintuple shape
26-
// (AuthorityAmbiguous + AmbiguityConflictExternal + external=="csf"
27-
// + DirectAdmin + NoRecord + --panel-auto-takeover + --accept-orphan-nftban),
28-
// not only on the Amendment 2 triple.
25+
// 2. Calls the Amendment 3 evidence helper (gatherOrphanEvidenceAmendment3)
26+
// on the Amendment 3 quintuple shape, NOT the Amendment 2 helper
27+
// (auditor-recommended separate-helper structure).
2928
//
30-
// 3. Reframes E.2 in the Amendment 3 path so AllTrueAmendment3() returns
31-
// true on the happy path (per §64.1 + the dispatcher reframing).
29+
// 3. Calls the Amendment 2 evidence helper (gatherOrphanEvidence)
30+
// on the Amendment 2 quintuple shape (regression — Amendment 2
31+
// path unchanged).
32+
//
33+
// 4. Reframes E.2 in the Amendment 3 helper so AllTrueAmendment3()
34+
// returns true on the §62 happy path.
35+
//
36+
// 5. Keeps gatherOrphanEvidence's E.2 strictly evaluating
37+
// AuthorityNFTBan (Amendment 2 unchanged).
3238
//
3339
// =============================================================================
3440
package main
@@ -52,66 +58,57 @@ func amd3HappyAuth() *uninstall.ClassifyResult {
5258
}
5359
}
5460

55-
// TestAmd3Dispatcher_E2Reframed_AllTrueAmendment3_True confirms that
56-
// gatherOrphanEvidence sets E.2=true on the Amendment 3 entry condition
57-
// (so AllTrueAmendment3() is satisfied on the happy path).
58-
func TestAmd3Dispatcher_E2Reframed_AllTrueAmendment3_True(t *testing.T) {
61+
// TestGatherOrphanEvidenceAmendment3_E2Reframed_AllTrueAmendment3_True
62+
// confirms the Amendment 3 helper sets E.2=true on the §62 entry
63+
// condition and AllTrueAmendment3() returns true on the happy path.
64+
// Also verifies E.12 is correctly false (entry condition IS
65+
// AmbiguityConflictExternal) and that AllTrue() (Amendment 2 predicate)
66+
// returns false on this fixture (because E.12 is required-true by §54.1).
67+
func TestGatherOrphanEvidenceAmendment3_E2Reframed_AllTrueAmendment3_True(t *testing.T) {
5968
log := logging.New("/dev/null", false)
60-
ev := gatherOrphanEvidence(happyExec(), log, detect.PanelDirectAdmin, amd3HappyAuth(), happyProbe(), happyCfg())
69+
ev := gatherOrphanEvidenceAmendment3(happyExec(), log, detect.PanelDirectAdmin, amd3HappyAuth(), happyProbe(), happyCfg())
6170

62-
// E.2 must be true under the Amendment 3 entry condition (reframed
63-
// per §64.1).
6471
if !ev.E2AuthorityNFTBan {
6572
t.Errorf("E.2 = false on Amendment 3 entry condition; want true (reframed per §64.1)")
6673
}
67-
68-
// AllTrueAmendment3() must be true (E.12 omitted; E.2 reframed-true).
6974
if !ev.AllTrueAmendment3() {
7075
t.Errorf("AllTrueAmendment3() = false; failed row=%s", ev.FailedRowIDAmendment3())
7176
}
72-
73-
// E.12 (NoConflictExternal) must be FALSE because the entry condition
74-
// IS AmbiguityConflictExternal — this is Amendment 3 by construction.
7577
if ev.E12NoConflictExternal {
7678
t.Errorf("E.12 = true on Amendment 3 entry; want false (entry condition IS AmbiguityConflictExternal)")
7779
}
78-
79-
// Amendment 2's AllTrue() must return false on this fixture because
80-
// E.12 is required-true by §54.1 but the Amendment 3 entry condition
81-
// makes that impossible. This proves the two predicates are
82-
// independently scoped.
8380
if ev.AllTrue() {
8481
t.Errorf("AllTrue() = true on Amendment 3 entry; want false (Amendment 2 predicate requires E.12=true)")
8582
}
8683
}
8784

88-
// TestAmd3Dispatcher_E2_FalseWhenMisclassified confirms the E.2
89-
// reframing is conservative: it only fires when the FULL quintuple
90-
// shape is present in the classifier output. Empty external,
91-
// non-csf external, and OrphanNFTBan ambiguity all keep E.2 false.
92-
func TestAmd3Dispatcher_E2_FalseWhenMisclassified(t *testing.T) {
85+
// TestGatherOrphanEvidenceAmendment3_E2FalseOnNonQualifying confirms
86+
// that the Amendment 3 helper's E.2 is conservative: ONLY fires when
87+
// the full §62 entry condition (AuthorityAmbiguous + ConflictExternal
88+
// + external=="csf") is present.
89+
func TestGatherOrphanEvidenceAmendment3_E2FalseOnNonQualifying(t *testing.T) {
9390
cases := []struct {
9491
name string
9592
auth *uninstall.ClassifyResult
9693
want bool
9794
}{
98-
{
99-
name: "amd2_path_authority_nftban",
100-
auth: &uninstall.ClassifyResult{
101-
State: uninstall.AuthorityNFTBan,
102-
Ambiguity: uninstall.AmbiguityNone,
103-
External: "",
104-
},
105-
want: true, // Amendment 2's classic E.2
106-
},
10795
{
10896
name: "amd3_path_authority_ambiguous_csf",
10997
auth: &uninstall.ClassifyResult{
11098
State: uninstall.AuthorityAmbiguous,
11199
Ambiguity: uninstall.AmbiguityConflictExternal,
112100
External: "csf",
113101
},
114-
want: true, // Amendment 3 reframed E.2
102+
want: true,
103+
},
104+
{
105+
name: "amd2_path_authority_nftban_FAILS_amendment3_helper",
106+
auth: &uninstall.ClassifyResult{
107+
State: uninstall.AuthorityNFTBan,
108+
Ambiguity: uninstall.AmbiguityNone,
109+
External: "",
110+
},
111+
want: false, // gatherOrphanEvidenceAmendment3 ONLY accepts §62 entry
115112
},
116113
{
117114
name: "external_empty_defensive",
@@ -120,7 +117,7 @@ func TestAmd3Dispatcher_E2_FalseWhenMisclassified(t *testing.T) {
120117
Ambiguity: uninstall.AmbiguityConflictExternal,
121118
External: "",
122119
},
123-
want: false, // empty external — not a §62 candidate
120+
want: false,
124121
},
125122
{
126123
name: "external_ufw_out_of_scope",
@@ -129,7 +126,7 @@ func TestAmd3Dispatcher_E2_FalseWhenMisclassified(t *testing.T) {
129126
Ambiguity: uninstall.AmbiguityConflictExternal,
130127
External: "ufw",
131128
},
132-
want: false, // non-csf external — not a §62 candidate
129+
want: false,
133130
},
134131
{
135132
name: "external_multi_csf_ufw",
@@ -138,7 +135,7 @@ func TestAmd3Dispatcher_E2_FalseWhenMisclassified(t *testing.T) {
138135
Ambiguity: uninstall.AmbiguityConflictExternal,
139136
External: "csf,ufw",
140137
},
141-
want: false, // multi-external — not a §62 candidate
138+
want: false,
142139
},
143140
{
144141
name: "ambiguity_orphan_nftban",
@@ -147,18 +144,56 @@ func TestAmd3Dispatcher_E2_FalseWhenMisclassified(t *testing.T) {
147144
Ambiguity: uninstall.AmbiguityOrphanNFTBan,
148145
External: "csf",
149146
},
150-
want: false, // wrong sub-classifier — not a §62 candidate
147+
want: false,
151148
},
152149
{
153150
name: "authority_external",
154151
auth: &uninstall.ClassifyResult{
155152
State: uninstall.AuthorityExternal,
156153
External: "csf",
157154
},
158-
want: false, // AuthorityExternal — locked-REFUSE, not §62
155+
want: false,
159156
},
160157
}
161158

159+
for _, c := range cases {
160+
t.Run(c.name, func(t *testing.T) {
161+
log := logging.New("/dev/null", false)
162+
ev := gatherOrphanEvidenceAmendment3(happyExec(), log, detect.PanelDirectAdmin, c.auth, happyProbe(), happyCfg())
163+
if ev.E2AuthorityNFTBan != c.want {
164+
t.Errorf("E.2 = %v; want %v", ev.E2AuthorityNFTBan, c.want)
165+
}
166+
})
167+
}
168+
}
169+
170+
// TestGatherOrphanEvidence_Amendment2Unchanged confirms the Amendment 2
171+
// helper's E.2 still evaluates strictly AuthorityNFTBan. This is the
172+
// regression check for §66.2 — Amendment 2 path stays passing.
173+
func TestGatherOrphanEvidence_Amendment2Unchanged(t *testing.T) {
174+
cases := []struct {
175+
name string
176+
auth *uninstall.ClassifyResult
177+
want bool
178+
}{
179+
{
180+
name: "amd2_authority_nftban_true",
181+
auth: &uninstall.ClassifyResult{
182+
State: uninstall.AuthorityNFTBan,
183+
Ambiguity: uninstall.AmbiguityNone,
184+
},
185+
want: true,
186+
},
187+
{
188+
name: "amd3_classifier_does_NOT_qualify_amendment2_E2",
189+
auth: &uninstall.ClassifyResult{
190+
State: uninstall.AuthorityAmbiguous,
191+
Ambiguity: uninstall.AmbiguityConflictExternal,
192+
External: "csf",
193+
},
194+
want: false, // Amendment 2 helper rejects Amendment 3 entry
195+
},
196+
}
162197
for _, c := range cases {
163198
t.Run(c.name, func(t *testing.T) {
164199
log := logging.New("/dev/null", false)
@@ -170,24 +205,62 @@ func TestAmd3Dispatcher_E2_FalseWhenMisclassified(t *testing.T) {
170205
}
171206
}
172207

173-
// TestAmd3Dispatcher_FailedRow_Amendment3 confirms FailedRowIDAmendment3()
174-
// returns AMD3-E.{N} on per-row failures from the Amendment 3 path.
175-
// E.12 is omitted from the Amendment 3 walk so its falseness must not
176-
// trigger a failed-row return.
177-
func TestAmd3Dispatcher_FailedRow_Amendment3(t *testing.T) {
208+
// TestGatherOrphanEvidenceAmendment3_FailedRow confirms
209+
// FailedRowIDAmendment3() returns AMD3-E.{N} on per-row failures and
210+
// "" on the happy path. E.12 is omitted from the Amendment 3 walk so
211+
// its falseness must not trigger a failed-row return.
212+
func TestGatherOrphanEvidenceAmendment3_FailedRow(t *testing.T) {
178213
log := logging.New("/dev/null", false)
179214

180-
// Happy path: AllTrueAmendment3 holds, FailedRowIDAmendment3 is empty.
181-
ev := gatherOrphanEvidence(happyExec(), log, detect.PanelDirectAdmin, amd3HappyAuth(), happyProbe(), happyCfg())
215+
ev := gatherOrphanEvidenceAmendment3(happyExec(), log, detect.PanelDirectAdmin, amd3HappyAuth(), happyProbe(), happyCfg())
182216
if id := ev.FailedRowIDAmendment3(); id != "" {
183217
t.Errorf("FailedRowIDAmendment3 on happy path = %q; want \"\"", id)
184218
}
185219

186-
// Failure path: kill E.7 (csf.disabled absent).
187220
exec := happyExec()
188221
delete(exec.Files, "/usr/sbin/csf.disabled")
189-
ev = gatherOrphanEvidence(exec, log, detect.PanelDirectAdmin, amd3HappyAuth(), happyProbe(), happyCfg())
222+
ev = gatherOrphanEvidenceAmendment3(exec, log, detect.PanelDirectAdmin, amd3HappyAuth(), happyProbe(), happyCfg())
190223
if id := ev.FailedRowIDAmendment3(); id != "AMD3-E.7" {
191224
t.Errorf("FailedRowIDAmendment3 with E.7 absent = %q; want %q", id, "AMD3-E.7")
192225
}
193226
}
227+
228+
// TestGatherOrphanEvidenceAmendment3_E2_FalseOnNonCSFExternal pins
229+
// the AMD3-7-equivalent gating: external != "csf" → E.2 = false →
230+
// AllTrueAmendment3 false → FailedRowIDAmendment3 == AMD3-E.2.
231+
func TestGatherOrphanEvidenceAmendment3_E2_FalseOnNonCSFExternal(t *testing.T) {
232+
log := logging.New("/dev/null", false)
233+
auth := &uninstall.ClassifyResult{
234+
State: uninstall.AuthorityAmbiguous,
235+
Ambiguity: uninstall.AmbiguityConflictExternal,
236+
External: "ufw",
237+
}
238+
ev := gatherOrphanEvidenceAmendment3(happyExec(), log, detect.PanelDirectAdmin, auth, happyProbe(), happyCfg())
239+
if ev.E2AuthorityNFTBan {
240+
t.Errorf("E.2 = true on external=ufw; want false")
241+
}
242+
if ev.AllTrueAmendment3() {
243+
t.Errorf("AllTrueAmendment3 = true on external=ufw; want false")
244+
}
245+
if id := ev.FailedRowIDAmendment3(); id != "AMD3-E.2" {
246+
t.Errorf("FailedRowIDAmendment3 on external=ufw = %q; want %q", id, "AMD3-E.2")
247+
}
248+
}
249+
250+
// TestGatherOrphanEvidenceAmendment3_OmitsE12 carries the §66.2
251+
// invariant from the engine_amendment3_test.go suite into the
252+
// dispatcher integration: AllTrueAmendment3() must NOT consider
253+
// E.12 even when it is false.
254+
func TestGatherOrphanEvidenceAmendment3_OmitsE12(t *testing.T) {
255+
log := logging.New("/dev/null", false)
256+
ev := gatherOrphanEvidenceAmendment3(happyExec(), log, detect.PanelDirectAdmin, amd3HappyAuth(), happyProbe(), happyCfg())
257+
258+
// On Amendment 3 entry, E.12 is false (entry IS ConflictExternal).
259+
if ev.E12NoConflictExternal {
260+
t.Fatalf("invariant: E.12 must be false on Amendment 3 entry; got true")
261+
}
262+
// AllTrueAmendment3 must still pass — E.12 omitted from predicate.
263+
if !ev.AllTrueAmendment3() {
264+
t.Errorf("AllTrueAmendment3 = false despite E.12 omission; failed row=%s", ev.FailedRowIDAmendment3())
265+
}
266+
}

0 commit comments

Comments
 (0)