Skip to content

Commit 48b9e50

Browse files
itcmsgrclaude
andauthored
fix(v1.100 Amendment 2): code-B — preflight accepts /usr/sbin/csf.disabled for restore-to-CSF (#520)
Surfaced by Amendment-2-code-E srv3 destructive run (2026-04-28T21:06:03Z): the dispatcher reached G1/AuthorityNFTBan/OrphanProceed PROCEED but RESTORE_FAILED_EXECUTION at preflight stage with exit=8 because the existing PR-25 §23.1 preflight required the canonical csf binary to be present in PATH. The Amendment-2 orphan-restore scenario specifically has /usr/sbin/csf ABSENT and /usr/sbin/csf.disabled PRESENT (the install-time-disabled state from switchop.DisableConflicts step 4), which the dispatcher then restores via §32 A.3 rename. This is a contract gap in PR-25 preflight, NOT in Amendment 2's decision layer. Code-A (#519) and Amendment 2 §53 + §54 work as designed. Surgical patch ============== In productionPreflightDep.PreflightTarget: - For firewallType=="csf": if no canonical csf binary in PATH, accept /usr/sbin/csf.disabled as a restorable candidate. - For non-CSF firewalls (ufw / firewalld / iptables): existing strict in-PATH check unchanged. No .disabled relaxation. Per Amendment 1 §30.2 (CSF-only inverse-of-install scope). - Existing unit-file presence check unchanged. - Preflight remains read-only — uses existing exec.FileExists. A.3 (Amendment 1 §31) remains the authoritative rename and detects the ambiguous-both-present state for refusal at §32 step 1. Tests added (9 cases per operator scope) ======================================== 1. AMD2-1: csf in PATH, .disabled absent, unit present → PASS 2. AMD2-2: csf absent, .disabled present, unit present → PASS (§54 case) 3. AMD2-3: csf absent, .disabled absent, unit present → REFUSE ErrPreflightBinaryMissing 4. AMD2-4: csf in PATH AND .disabled present, unit present → PASS at preflight; ambiguity is A.3's responsibility 5. AMD2-5: csf in PATH, unit absent → REFUSE ErrPreflightUnitMissing 6. AMD2-6: ufw absent + ufw.disabled present → REFUSE (no relaxation) 7. AMD2-7: firewalld absent + firewall-cmd.disabled present → REFUSE 8. AMD2-8: iptables absent + iptables.disabled present → REFUSE 9. AMD2-9: unknown firewallType → REFUSE ErrPreflightUnknownFirewall 10. AMD2-10: §54 branch records ZERO mutation commands (read-only contract preserved) Test results (lab4 build host) ============================== - go test ./cmd/nftban-installer/... -run 'Preflight|RestoreDeps' → PASS - go test ./cmd/nftban-installer/... → PASS - go test ./internal/installer/restore/... → PASS - go test ./... → 64 pkgs PASS, 0 FAIL Files changed ============= cmd/nftban-installer/restore_deps.go | +24 / -4 (28 lines) cmd/nftban-installer/restore_deps_test.go | +163 (10 new tests) No files outside the operator-allowed two. Invariants preserved ==================== - §23.1 preflight read-only: unchanged. New branch uses FileExists only. - INV-PR26-NEW-MUTATION-SURFACES-BOUNDED: zero new surfaces. - §32 ordering: unchanged. A.3 still authoritative rename. - §22 / §19.4 terminals + exit codes: unchanged. - Amendment 1 §30.2 CSF-only scope: preserved (non-CSF .disabled files explicitly refuse — tests AMD2-6/7/8 pin this). - §19.2 layer 4 / main.go:132 history gate: untouched. - Existing 4B-1 fixture matrix: unchanged behavior, all pass. No CI workflow edit. No state machine change. No flag change. No contract change. No execute.go change. No engine.go change. No main.go change. No srv3 action. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 433f53c commit 48b9e50

2 files changed

Lines changed: 187 additions & 4 deletions

File tree

cmd/nftban-installer/restore_deps.go

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,31 @@ func (p *productionPreflightDep) PreflightTarget(_ context.Context, firewallType
187187
}
188188
}
189189
if binaryFound == "" {
190-
if p.log != nil {
191-
p.log.Info("restore preflight: refusing firewallType=%q — no canonical binary in PATH (looked for %v)",
192-
firewallType, presence.binaries)
190+
// Amendment 2 §54: for CSF-only restore-to-CSF, accept the
191+
// install-time-disabled binary `/usr/sbin/csf.disabled` as an
192+
// acceptable restorable candidate. The Amendment-2 orphan
193+
// evidence chain proves NFTBan disabled CSF by renaming
194+
// /usr/sbin/csf → /usr/sbin/csf.disabled at install-time
195+
// (switchop.DisableConflicts step 4, Amendment 1 §31 A.3).
196+
// §32 A.3 later performs the authoritative rename back.
197+
// Preflight remains read-only and only verifies that a
198+
// restorable candidate is present; it does NOT mutate or
199+
// rename here. The .disabled relaxation is CSF-only — ufw,
200+
// firewalld, and iptables retain the strict in-PATH check
201+
// per Amendment 1 §30.2 (CSF-only inverse-of-install scope).
202+
if firewallType == "csf" && p.exec.FileExists("/usr/sbin/csf.disabled") {
203+
if p.log != nil {
204+
p.log.Info("restore preflight: firewallType=%q canonical binary absent from PATH but /usr/sbin/csf.disabled present — accepting (Amendment 2 §54 / restore-to-CSF)",
205+
firewallType)
206+
}
207+
binaryFound = "/usr/sbin/csf.disabled"
208+
} else {
209+
if p.log != nil {
210+
p.log.Info("restore preflight: refusing firewallType=%q — no canonical binary in PATH (looked for %v)",
211+
firewallType, presence.binaries)
212+
}
213+
return false, ErrPreflightBinaryMissing
193214
}
194-
return false, ErrPreflightBinaryMissing
195215
}
196216

197217
// Check unit files (OR-list — at least one path must exist).

cmd/nftban-installer/restore_deps_test.go

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,3 +899,166 @@ func keysOf(m map[string][]byte) []string {
899899
}
900900
return out
901901
}
902+
903+
// =============================================================================
904+
// =============================================================================
905+
// Amendment 2 code-B — preflight csf.disabled acceptance
906+
// =============================================================================
907+
// =============================================================================
908+
//
909+
// Amendment 2 §54 + §53 G1/AuthorityNFTBan/OrphanProceed path requires
910+
// preflight to accept /usr/sbin/csf.disabled as a restorable candidate
911+
// when /usr/sbin/csf is absent. The .disabled relaxation is CSF-only;
912+
// ufw / firewalld / iptables retain the strict in-PATH check per
913+
// Amendment 1 §30.2 (CSF-only inverse-of-install scope).
914+
//
915+
// pf4B1MockWith already supports adding arbitrary file paths to the
916+
// mock; we extend the test fixture set with explicit
917+
// /usr/sbin/csf.disabled presence/absence variations.
918+
919+
// pfAMD2MockWith builds a MockExecutor with the given binary in PATH
920+
// (or empty for absent), unit-file paths, and an optional
921+
// /usr/sbin/csf.disabled presence flag.
922+
func pfAMD2MockWith(binary string, unitFiles []string, csfDisabledPresent bool) *executor.MockExecutor {
923+
mock := pf4B1MockWith(binary, unitFiles)
924+
if csfDisabledPresent {
925+
mock.Files["/usr/sbin/csf.disabled"] = []byte{}
926+
}
927+
return mock
928+
}
929+
930+
// AMD2-1: csf in PATH + unit present + .disabled absent → PASS.
931+
// Pinned to confirm the §54 relaxation does NOT regress the existing
932+
// happy path that 4B-1.1 already covers.
933+
func TestPreflightTarget_AMD2_CSF_InPath_DisabledAbsent_Pass(t *testing.T) {
934+
mock := pfAMD2MockWith("csf", []string{"/etc/systemd/system/csf.service"}, false)
935+
d := &productionPreflightDep{exec: mock}
936+
ok, err := d.PreflightTarget(context.Background(), "csf")
937+
if !ok || err != nil {
938+
t.Errorf("PreflightTarget(csf) ok=%v err=%v; want ok=true err=nil", ok, err)
939+
}
940+
}
941+
942+
// AMD2-2: csf absent + /usr/sbin/csf.disabled present + unit present → PASS.
943+
// This is the load-bearing Amendment 2 §54 case.
944+
func TestPreflightTarget_AMD2_CSF_Absent_DisabledPresent_Pass(t *testing.T) {
945+
mock := pfAMD2MockWith("", []string{"/etc/systemd/system/csf.service"}, true)
946+
d := &productionPreflightDep{exec: mock}
947+
ok, err := d.PreflightTarget(context.Background(), "csf")
948+
if !ok {
949+
t.Errorf("PreflightTarget(csf) = false; want true (Amendment 2 §54 csf.disabled relaxation)")
950+
}
951+
if err != nil {
952+
t.Errorf("PreflightTarget(csf) returned err: %v", err)
953+
}
954+
}
955+
956+
// AMD2-3: csf absent + .disabled absent + unit present → REFUSE
957+
// ErrPreflightBinaryMissing. Both restorable candidates are missing.
958+
func TestPreflightTarget_AMD2_CSF_BothAbsent_Refuse(t *testing.T) {
959+
mock := pfAMD2MockWith("", []string{"/etc/systemd/system/csf.service"}, false)
960+
d := &productionPreflightDep{exec: mock}
961+
ok, err := d.PreflightTarget(context.Background(), "csf")
962+
if ok {
963+
t.Errorf("PreflightTarget(csf) accepted with both csf and csf.disabled absent; want refusal")
964+
}
965+
if !errors.Is(err, ErrPreflightBinaryMissing) {
966+
t.Errorf("err = %v; want ErrPreflightBinaryMissing", err)
967+
}
968+
}
969+
970+
// AMD2-4: csf in PATH AND .disabled present + unit present → PASS at
971+
// preflight. The "ambiguous-both-present" state is A.3's responsibility
972+
// per Amendment 1 §31 A.3 ("Refuse the entire restore if both
973+
// /usr/sbin/csf and /usr/sbin/csf.disabled are present"). Preflight
974+
// remains read-only and only verifies presence of a restorable
975+
// candidate; the ambiguity is detected and refused later in §32 step 3.
976+
func TestPreflightTarget_AMD2_CSF_BothPresent_Pass_AmbiguityIsA3(t *testing.T) {
977+
mock := pfAMD2MockWith("csf", []string{"/etc/systemd/system/csf.service"}, true)
978+
d := &productionPreflightDep{exec: mock}
979+
ok, err := d.PreflightTarget(context.Background(), "csf")
980+
if !ok || err != nil {
981+
t.Errorf("PreflightTarget(csf) ok=%v err=%v; want preflight PASS (ambiguity is A.3 territory)", ok, err)
982+
}
983+
}
984+
985+
// AMD2-5: csf in PATH + unit ABSENT → REFUSE ErrPreflightUnitMissing.
986+
// Existing unit-file presence check remains unchanged by §54.
987+
func TestPreflightTarget_AMD2_CSF_InPath_UnitAbsent_Refuse(t *testing.T) {
988+
mock := pfAMD2MockWith("csf", nil, false)
989+
d := &productionPreflightDep{exec: mock}
990+
ok, err := d.PreflightTarget(context.Background(), "csf")
991+
if ok {
992+
t.Errorf("PreflightTarget(csf) accepted with no unit file; want refusal")
993+
}
994+
if !errors.Is(err, ErrPreflightUnitMissing) {
995+
t.Errorf("err = %v; want ErrPreflightUnitMissing", err)
996+
}
997+
}
998+
999+
// AMD2-6/7/8: non-CSF firewalls do NOT receive the .disabled relaxation.
1000+
// Even if a hypothetical "<binary>.disabled" file exists, preflight
1001+
// still refuses with ErrPreflightBinaryMissing. CSF-only scope per
1002+
// Amendment 1 §30.2.
1003+
func TestPreflightTarget_AMD2_NonCSF_NoDisabledRelaxation(t *testing.T) {
1004+
cases := []struct {
1005+
fwt string
1006+
unitPath string
1007+
disabledPath string // hypothetical .disabled filename for the firewall
1008+
}{
1009+
{"ufw", "/usr/lib/systemd/system/ufw.service", "/usr/sbin/ufw.disabled"},
1010+
{"firewalld", "/usr/lib/systemd/system/firewalld.service", "/usr/bin/firewall-cmd.disabled"},
1011+
{"iptables", "/usr/lib/systemd/system/iptables.service", "/usr/sbin/iptables.disabled"},
1012+
}
1013+
for _, c := range cases {
1014+
t.Run(c.fwt, func(t *testing.T) {
1015+
mock := pf4B1MockWith("", []string{c.unitPath})
1016+
// Add a hypothetical .disabled for this non-CSF firewall.
1017+
// Must NOT be accepted by preflight under any rationale.
1018+
mock.Files[c.disabledPath] = []byte{}
1019+
// Also seed /usr/sbin/csf.disabled to prove the CSF-only
1020+
// branch does not bleed into non-CSF paths.
1021+
mock.Files["/usr/sbin/csf.disabled"] = []byte{}
1022+
d := &productionPreflightDep{exec: mock}
1023+
ok, err := d.PreflightTarget(context.Background(), c.fwt)
1024+
if ok {
1025+
t.Errorf("PreflightTarget(%q) accepted with a hypothetical %q present — Amendment 1 §30.2 CSF-only scope violated",
1026+
c.fwt, c.disabledPath)
1027+
}
1028+
if !errors.Is(err, ErrPreflightBinaryMissing) {
1029+
t.Errorf("PreflightTarget(%q) err = %v; want ErrPreflightBinaryMissing", c.fwt, err)
1030+
}
1031+
})
1032+
}
1033+
}
1034+
1035+
// AMD2-9: unknown firewallType still refuses with
1036+
// ErrPreflightUnknownFirewall regardless of any .disabled file
1037+
// presence on disk.
1038+
func TestPreflightTarget_AMD2_UnknownFirewall_StillRefuse(t *testing.T) {
1039+
mock := pf4B1MockWith("", nil)
1040+
mock.Files["/usr/sbin/csf.disabled"] = []byte{} // red herring
1041+
d := &productionPreflightDep{exec: mock}
1042+
ok, err := d.PreflightTarget(context.Background(), "shorewall")
1043+
if ok {
1044+
t.Errorf("PreflightTarget(shorewall) accepted; want refusal")
1045+
}
1046+
if !errors.Is(err, ErrPreflightUnknownFirewall) {
1047+
t.Errorf("err = %v; want ErrPreflightUnknownFirewall", err)
1048+
}
1049+
}
1050+
1051+
// AMD2-10: preflight remains read-only on the .disabled-acceptance
1052+
// branch. Calling PreflightTarget with the §54 relaxation active must
1053+
// still record ZERO commands (FileExists is a typed read; no Run).
1054+
func TestPreflightTarget_AMD2_CSF_DisabledBranch_NoMutationCalls(t *testing.T) {
1055+
mock := pfAMD2MockWith("", []string{"/etc/systemd/system/csf.service"}, true)
1056+
d := &productionPreflightDep{exec: mock}
1057+
_, err := d.PreflightTarget(context.Background(), "csf")
1058+
if err != nil {
1059+
t.Fatalf("setup error: %v", err)
1060+
}
1061+
if len(mock.Commands) != 0 {
1062+
t.Errorf("§54 relaxation branch recorded mutation commands: %+v", mock.Commands)
1063+
}
1064+
}

0 commit comments

Comments
 (0)