Skip to content

Commit 786a116

Browse files
itcmsgrclaude
andauthored
fix(v1.100 Amendment 2): code-C — preflight rejects mask-only unit-file (symlink to /dev/null) (#521)
Surfaced by Amendment-2-code-E v2 corrected run on srv3 (2026-04-28T21:30:13Z): the dispatcher reached G1/AuthorityNFTBan/OrphanProceed PROCEED, code-B csf.disabled relaxation correctly accepted, §32 step 2 safety-net inserted, A.1 unmask succeeded — then A.2 ServiceEnable failed with "Unit file csf.service does not exist" because the canonical unit-file path (/etc/systemd/system/csf.service) was a mask symlink to /dev/null with NO real backing unit-file anywhere on the host. Result: RESTORE_FAILED_EXECUTION at stage=mutate, exit=8, partial mutation (safety-net retained, csf still unmasked). This is the third (and final, on the §32 path) implementation gap surfaced by the destructive-cycle audit chain. Like code-B, the fix is implementation-class — no contract amendment, no decision-lattice change, no §32 ordering change. Surgical patch ============== In productionPreflightDep.PreflightTarget's unit-file OR-list loop: - After exec.FileExists returns true for a candidate path, route through exec.Run("readlink", "-f", <path>) to obtain the resolved canonical path (read-only per §43.3 raw-Run policy for read-only probes). - If resolved == "/dev/null", treat the candidate as not-found and continue the OR-list. Records a maskOnlyObserved flag. - A real file (resolved == path) or symlink to a real file (resolved != /dev/null) is accepted. - If every candidate is absent: preserve existing ErrPreflightUnitMissing semantic. - If at least one candidate was a mask-only symlink and no real backing path resolved: refuse with new typed sentinel ErrPreflightUnitFileMaskingOnly. The new sentinel: ErrPreflightUnitFileMaskingOnly = errors.New( "restore preflight: unit file is a mask symlink (points to /dev/null); no real unit file backs the service") Existing exit code remains 8. Existing terminal remains RESTORE_FAILED_EXECUTION. Same hardening applies uniformly to ufw / firewalld / iptables / csf — every firewall in §18.2. Tests added (10 new test functions) =================================== AMD2C-1: regular file at /etc/systemd/system/csf.service → PASS AMD2C-2: real unit at /usr/lib/systemd/system/csf.service → PASS AMD2C-3: mask symlink only → REFUSE ErrPreflightUnitFileMaskingOnly AMD2C-4: mask at /etc/ AND real at /usr/lib/ → PASS (OR-list resolves) AMD2C-5: plain regular file → PASS AMD2C-6: symlink to real backing file → PASS AMD2C-7: ufw / firewalld / iptables mask hardening (3 sub-tests) AMD2C-8: unknown firewallType → REFUSE ErrPreflightUnknownFirewall AMD2C-9: NoMutationCalls — readlink only, zero mutation primitives AMD2C-10: no candidate path at all → REFUSE ErrPreflightUnitMissing (distinguished from MaskingOnly) 4B-1.7 NoMutationCalls test updated: now allows `readlink` Run calls (read-only per §43.3) while still asserting ZERO mutation primitives. 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 | +61 / -8 (53 line net) cmd/nftban-installer/restore_deps_test.go | +252 / -4 (10 new tests + 1 update) No files outside the operator-allowed two. Invariants preserved ==================== - §23.1 preflight read-only: PRESERVED — readlink is read-only per §43.3. - INV-PR26-NEW-MUTATION-SURFACES-BOUNDED: PRESERVED — zero new surfaces. - §32 ordering: UNTOUCHED — A.1/A.2 not modified; mask detection happens at preflight before §32 step 2. - §22 / §19.4 terminals + exit codes: UNTOUCHED — same exit=8, same RESTORE_FAILED_EXECUTION. - Amendment 1 §30.2 CSF-only mutation scope: PRESERVED — non-CSF firewalls get the same mask-symlink hardening but still strict no-.disabled-relaxation. - §19.2 layer 4 / main.go:132 history gate: UNTOUCHED. - Existing 4B-1 fixture matrix: behavior unchanged on real-file paths. - Code-B csf.disabled acceptance: PRESERVED — §54 binary relaxation still fires before unit-file check. 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 host action. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 48b9e50 commit 786a116

2 files changed

Lines changed: 301 additions & 12 deletions

File tree

cmd/nftban-installer/restore_deps.go

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
"context"
4343
"errors"
4444
"fmt"
45+
"strings"
4546

4647
"github.com/itcmsgr/nftban/internal/installer/detect"
4748
"github.com/itcmsgr/nftban/internal/installer/executor"
@@ -160,6 +161,19 @@ var (
160161
// service-unit file paths exist.
161162
ErrPreflightUnitMissing = errors.New("restore preflight: no canonical systemd unit file present")
162163

164+
// ErrPreflightUnitFileMaskingOnly is returned when every canonical
165+
// service-unit file path is either absent OR a mask symlink to
166+
// /dev/null. systemd treats /dev/null-symlinks as "masked" units;
167+
// `systemctl unmask` removes the symlink, but `systemctl enable`
168+
// then fails because no real backing unit-file exists. Surfaced
169+
// during the Amendment-2-code-E v2 corrected run on srv3
170+
// (2026-04-28T21:30:13Z) — preflight passed against the mask
171+
// symlink, A.1 unmask succeeded, A.2 enable failed with "Unit
172+
// file csf.service does not exist". This sentinel refuses the
173+
// run before §32 step 2 safety-net insertion to prevent partial
174+
// mutation on hosts that lack a real unit file.
175+
ErrPreflightUnitFileMaskingOnly = errors.New("restore preflight: unit file is a mask symlink (points to /dev/null); no real unit file backs the service")
176+
163177
// ErrPreflightNilExecutor is returned when the dep was
164178
// constructed without a usable executor. Defensive guard.
165179
ErrPreflightNilExecutor = errors.New("restore preflight: executor is nil")
@@ -214,15 +228,54 @@ func (p *productionPreflightDep) PreflightTarget(_ context.Context, firewallType
214228
}
215229
}
216230

217-
// Check unit files (OR-list — at least one path must exist).
231+
// Check unit files (OR-list — at least one path must exist AND
232+
// resolve to a real backing unit-file, not /dev/null).
233+
//
234+
// Amendment-2-code-C hardening (2026-04-28): a path that exists
235+
// as a symlink to /dev/null is a systemd "masked" unit. After
236+
// A.1 unmask removes the symlink, A.2 ServiceEnable fails with
237+
// "Unit file does not exist" because no real backing unit-file
238+
// is present. Preflight detects this state and refuses with
239+
// ErrPreflightUnitFileMaskingOnly rather than allowing the §32
240+
// step 2 safety-net insertion + A.1 unmask to run, which would
241+
// partially mutate the host.
242+
//
243+
// Mechanic: after FileExists returns true, route through
244+
// Run("readlink", "-f", path) — read-only per §43.3 raw-Run
245+
// policy for read-only probes. If the resolved canonical path
246+
// equals "/dev/null", skip this candidate. If all candidates
247+
// resolve to /dev/null OR are absent, refuse with the mask-only
248+
// sentinel; if some are missing and at least one is a real
249+
// unit-file, that real unit-file is accepted.
218250
var unitFound string
251+
maskOnlyObserved := false
219252
for _, path := range presence.unitFiles {
220-
if p.exec.FileExists(path) {
221-
unitFound = path
222-
break
253+
if !p.exec.FileExists(path) {
254+
continue
223255
}
256+
// FileExists returned true. Resolve to detect mask-only.
257+
res := p.exec.Run("readlink", "-f", path)
258+
resolved := strings.TrimSpace(res.Stdout)
259+
if resolved == "/dev/null" {
260+
maskOnlyObserved = true
261+
if p.log != nil {
262+
p.log.Info("restore preflight: candidate unit path %q is a mask symlink to /dev/null — skipping (Amendment-2-code-C)",
263+
path)
264+
}
265+
continue
266+
}
267+
// Real file or symlink to real file: acceptable.
268+
unitFound = path
269+
break
224270
}
225271
if unitFound == "" {
272+
if maskOnlyObserved {
273+
if p.log != nil {
274+
p.log.Info("restore preflight: refusing firewallType=%q — every candidate unit-file path is a mask symlink to /dev/null; no real backing unit-file exists",
275+
firewallType)
276+
}
277+
return false, ErrPreflightUnitFileMaskingOnly
278+
}
226279
if p.log != nil {
227280
p.log.Info("restore preflight: refusing firewallType=%q — no canonical service unit (looked at %v)",
228281
firewallType, presence.unitFiles)

cmd/nftban-installer/restore_deps_test.go

Lines changed: 244 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -444,11 +444,16 @@ func TestPreflightTarget_4B1_NoMutationCalls(t *testing.T) {
444444
d := &productionPreflightDep{exec: mock}
445445
_, _ = d.PreflightTarget(context.Background(), c.fwt)
446446

447-
// Commands recorded must be empty: preflight only calls
448-
// CommandExists + FileExists, neither of which records a
449-
// command in MockExecutor.Commands.
450-
if len(mock.Commands) != 0 {
451-
t.Errorf("preflight recorded mutation commands: %+v", mock.Commands)
447+
// Recorded commands must contain ONLY read-only probes:
448+
// CommandExists + FileExists are typed seams that don't
449+
// record. Amendment-2-code-C adds `readlink -f` Run calls
450+
// for unit-file resolution — read-only per §43.3 raw-Run
451+
// policy. Mutation primitives (Service*, Nft*,
452+
// WriteFileAtomic, mv, rename, etc.) must remain ZERO.
453+
for _, cmd := range mock.Commands {
454+
if cmd.Name != "readlink" {
455+
t.Errorf("preflight recorded non-readonly command: %+v", cmd)
456+
}
452457
}
453458
})
454459
}
@@ -1050,15 +1055,246 @@ func TestPreflightTarget_AMD2_UnknownFirewall_StillRefuse(t *testing.T) {
10501055

10511056
// AMD2-10: preflight remains read-only on the .disabled-acceptance
10521057
// branch. Calling PreflightTarget with the §54 relaxation active must
1053-
// still record ZERO commands (FileExists is a typed read; no Run).
1058+
// still record ZERO mutation commands. (After Amendment-2-code-C the
1059+
// branch records a read-only `readlink -f` Run for unit-file resolution;
1060+
// that is a read-only systemctl-class probe per §43.3 and is allowed.)
10541061
func TestPreflightTarget_AMD2_CSF_DisabledBranch_NoMutationCalls(t *testing.T) {
10551062
mock := pfAMD2MockWith("", []string{"/etc/systemd/system/csf.service"}, true)
10561063
d := &productionPreflightDep{exec: mock}
10571064
_, err := d.PreflightTarget(context.Background(), "csf")
10581065
if err != nil {
10591066
t.Fatalf("setup error: %v", err)
10601067
}
1061-
if len(mock.Commands) != 0 {
1062-
t.Errorf("§54 relaxation branch recorded mutation commands: %+v", mock.Commands)
1068+
// All recorded commands must be read-only `readlink` invocations
1069+
// (or empty). Mutation primitives must be zero.
1070+
for _, c := range mock.Commands {
1071+
if c.Name != "readlink" {
1072+
t.Errorf("§54 relaxation branch recorded non-readonly command: %+v", c)
1073+
}
1074+
}
1075+
}
1076+
1077+
// =============================================================================
1078+
// =============================================================================
1079+
// Amendment 2 code-C — preflight unit-file mask-symlink hardening
1080+
// =============================================================================
1081+
// =============================================================================
1082+
//
1083+
// Amendment-2-code-E v2 corrected run on srv3 surfaced this gap:
1084+
// /etc/systemd/system/csf.service was a mask symlink to /dev/null.
1085+
// FileExists returned true → preflight passed → safety-net inserted
1086+
// → A.1 unmask removed the symlink → A.2 ServiceEnable failed because
1087+
// no real backing unit-file existed → host left in partial-mutation
1088+
// state with safety-net retained.
1089+
//
1090+
// Code-C closes the gap by routing every found unit-file path through
1091+
// `readlink -f` and rejecting paths that resolve to /dev/null. The
1092+
// OR-list still wins if at least one candidate is a real backing file.
1093+
// If every candidate is mask-only, refuse with the new typed sentinel
1094+
// ErrPreflightUnitFileMaskingOnly before any mutation runs.
1095+
1096+
// pfAMD2CMockWith configures a mock with given binary in PATH (or
1097+
// empty), the given unit-files (each entry is the path → resolved
1098+
// readlink target). A resolved target of "/dev/null" simulates a mask
1099+
// symlink. A resolved target equal to the path itself simulates a
1100+
// regular file. A resolved target pointing elsewhere simulates a
1101+
// normal symlink chain to a real unit-file.
1102+
func pfAMD2CMockWith(binary string, unitFiles map[string]string) *executor.MockExecutor {
1103+
mock := executor.NewMockExecutor()
1104+
if binary != "" {
1105+
mock.ExistingCommands[binary] = true
1106+
}
1107+
for path, resolved := range unitFiles {
1108+
mock.Files[path] = []byte{} // FileExists → true
1109+
// readlink -f result: stdout is the resolved canonical path.
1110+
key := "readlink:-f:" + path
1111+
mock.RunResults[key] = executor.Result{
1112+
ExitCode: 0,
1113+
Stdout: resolved + "\n",
1114+
}
1115+
}
1116+
return mock
1117+
}
1118+
1119+
// AMD2C-1: regular file at /etc/systemd/system/csf.service → PASS.
1120+
// Resolved canonical path equals the path itself (regular file).
1121+
func TestPreflightTarget_AMD2C_RegularFile_EtcSystemd_Pass(t *testing.T) {
1122+
mock := pfAMD2CMockWith("csf", map[string]string{
1123+
"/etc/systemd/system/csf.service": "/etc/systemd/system/csf.service",
1124+
})
1125+
d := &productionPreflightDep{exec: mock}
1126+
ok, err := d.PreflightTarget(context.Background(), "csf")
1127+
if !ok || err != nil {
1128+
t.Errorf("PreflightTarget(csf) ok=%v err=%v; want PASS", ok, err)
1129+
}
1130+
}
1131+
1132+
// AMD2C-2: real unit at /usr/lib/systemd/system/csf.service → PASS.
1133+
// Existing OR-list path; same shape as srv2.
1134+
func TestPreflightTarget_AMD2C_RegularFile_UsrLib_Pass(t *testing.T) {
1135+
mock := pfAMD2CMockWith("csf", map[string]string{
1136+
"/usr/lib/systemd/system/csf.service": "/usr/lib/systemd/system/csf.service",
1137+
})
1138+
d := &productionPreflightDep{exec: mock}
1139+
ok, err := d.PreflightTarget(context.Background(), "csf")
1140+
if !ok || err != nil {
1141+
t.Errorf("PreflightTarget(csf) ok=%v err=%v; want PASS", ok, err)
1142+
}
1143+
}
1144+
1145+
// AMD2C-3: mask symlink at /etc/systemd/system/csf.service → /dev/null,
1146+
// no real unit anywhere else → REFUSE ErrPreflightUnitFileMaskingOnly.
1147+
// Reproduces the srv3 v2 corrected-run failure mode.
1148+
func TestPreflightTarget_AMD2C_MaskOnly_NoBackingFile_Refuse(t *testing.T) {
1149+
mock := pfAMD2CMockWith("csf", map[string]string{
1150+
"/etc/systemd/system/csf.service": "/dev/null",
1151+
})
1152+
d := &productionPreflightDep{exec: mock}
1153+
ok, err := d.PreflightTarget(context.Background(), "csf")
1154+
if ok {
1155+
t.Errorf("PreflightTarget(csf) accepted mask-only state; want refusal (Amendment-2-code-C)")
1156+
}
1157+
if !errors.Is(err, ErrPreflightUnitFileMaskingOnly) {
1158+
t.Errorf("err = %v; want ErrPreflightUnitFileMaskingOnly", err)
1159+
}
1160+
}
1161+
1162+
// AMD2C-4: mask symlink at /etc/systemd/system/csf.service → /dev/null
1163+
// AND real unit at /usr/lib/systemd/system/csf.service → PASS. The
1164+
// OR-list resolves through the second canonical path. This is the
1165+
// "srv3 was rescued by srv2"-shaped case if both paths existed.
1166+
func TestPreflightTarget_AMD2C_MaskAtEtcButRealAtUsrLib_Pass(t *testing.T) {
1167+
mock := pfAMD2CMockWith("csf", map[string]string{
1168+
"/etc/systemd/system/csf.service": "/dev/null",
1169+
"/usr/lib/systemd/system/csf.service": "/usr/lib/systemd/system/csf.service",
1170+
})
1171+
d := &productionPreflightDep{exec: mock}
1172+
ok, err := d.PreflightTarget(context.Background(), "csf")
1173+
if !ok || err != nil {
1174+
t.Errorf("PreflightTarget(csf) ok=%v err=%v; want PASS (OR-list resolves through real backing path)", ok, err)
1175+
}
1176+
}
1177+
1178+
// AMD2C-5: same as AMD2C-1 but with explicit assertion of the
1179+
// "regular file" semantic — file path == resolved canonical path.
1180+
// Already covered by AMD2C-1; kept for explicit table coverage.
1181+
func TestPreflightTarget_AMD2C_PlainRegularFile_Pass(t *testing.T) {
1182+
mock := pfAMD2CMockWith("csf", map[string]string{
1183+
"/etc/systemd/system/csf.service": "/etc/systemd/system/csf.service",
1184+
})
1185+
d := &productionPreflightDep{exec: mock}
1186+
ok, err := d.PreflightTarget(context.Background(), "csf")
1187+
if !ok || err != nil {
1188+
t.Errorf("PreflightTarget(csf) ok=%v err=%v; want PASS", ok, err)
1189+
}
1190+
}
1191+
1192+
// AMD2C-6: symlink at /etc/systemd/system/csf.service pointing to a
1193+
// real unit-file at /usr/lib/systemd/system/csf.service → PASS. The
1194+
// resolved canonical path is the real backing file (NOT /dev/null),
1195+
// so preflight accepts.
1196+
func TestPreflightTarget_AMD2C_SymlinkToRealUnit_Pass(t *testing.T) {
1197+
mock := pfAMD2CMockWith("csf", map[string]string{
1198+
"/etc/systemd/system/csf.service": "/usr/lib/systemd/system/csf.service",
1199+
})
1200+
d := &productionPreflightDep{exec: mock}
1201+
ok, err := d.PreflightTarget(context.Background(), "csf")
1202+
if !ok || err != nil {
1203+
t.Errorf("PreflightTarget(csf) ok=%v err=%v; want PASS (symlink resolves to real backing file)", ok, err)
1204+
}
1205+
}
1206+
1207+
// AMD2C-7: same mask-symlink hardening applies to ufw / firewalld /
1208+
// iptables. A mask-only unit at the canonical path for these firewalls
1209+
// must REFUSE with ErrPreflightUnitFileMaskingOnly identically to csf.
1210+
func TestPreflightTarget_AMD2C_NonCSF_MaskHardening(t *testing.T) {
1211+
cases := []struct {
1212+
fwt string
1213+
binary string
1214+
unitPath string
1215+
}{
1216+
{"ufw", "ufw", "/usr/lib/systemd/system/ufw.service"},
1217+
{"firewalld", "firewall-cmd", "/usr/lib/systemd/system/firewalld.service"},
1218+
{"iptables", "iptables", "/usr/lib/systemd/system/iptables.service"},
1219+
}
1220+
for _, c := range cases {
1221+
t.Run(c.fwt, func(t *testing.T) {
1222+
mock := pfAMD2CMockWith(c.binary, map[string]string{
1223+
c.unitPath: "/dev/null",
1224+
})
1225+
d := &productionPreflightDep{exec: mock}
1226+
ok, err := d.PreflightTarget(context.Background(), c.fwt)
1227+
if ok {
1228+
t.Errorf("PreflightTarget(%q) accepted mask-only unit; want refusal", c.fwt)
1229+
}
1230+
if !errors.Is(err, ErrPreflightUnitFileMaskingOnly) {
1231+
t.Errorf("err = %v; want ErrPreflightUnitFileMaskingOnly", err)
1232+
}
1233+
})
1234+
}
1235+
}
1236+
1237+
// AMD2C-8: unknown firewall type still refuses with
1238+
// ErrPreflightUnknownFirewall, regardless of any mask-symlink state.
1239+
func TestPreflightTarget_AMD2C_UnknownFirewall_StillRefuse(t *testing.T) {
1240+
mock := pfAMD2CMockWith("", map[string]string{
1241+
"/etc/systemd/system/shorewall.service": "/dev/null",
1242+
})
1243+
d := &productionPreflightDep{exec: mock}
1244+
ok, err := d.PreflightTarget(context.Background(), "shorewall")
1245+
if ok {
1246+
t.Errorf("PreflightTarget(shorewall) accepted; want refusal")
1247+
}
1248+
if !errors.Is(err, ErrPreflightUnknownFirewall) {
1249+
t.Errorf("err = %v; want ErrPreflightUnknownFirewall", err)
1250+
}
1251+
}
1252+
1253+
// AMD2C-9: NoMutationCalls — code-C adds `readlink -f` Run invocations
1254+
// for read-only resolution. Confirm ZERO mutation calls (no Service*,
1255+
// Nft*, WriteFileAtomic etc.) regardless of which branch fires.
1256+
func TestPreflightTarget_AMD2C_NoMutationCalls(t *testing.T) {
1257+
cases := []struct {
1258+
name string
1259+
unitFiles map[string]string
1260+
}{
1261+
{"happy_real_file", map[string]string{
1262+
"/etc/systemd/system/csf.service": "/etc/systemd/system/csf.service",
1263+
}},
1264+
{"mask_only", map[string]string{
1265+
"/etc/systemd/system/csf.service": "/dev/null",
1266+
}},
1267+
{"mixed_mask_and_real", map[string]string{
1268+
"/etc/systemd/system/csf.service": "/dev/null",
1269+
"/usr/lib/systemd/system/csf.service": "/usr/lib/systemd/system/csf.service",
1270+
}},
1271+
}
1272+
for _, c := range cases {
1273+
t.Run(c.name, func(t *testing.T) {
1274+
mock := pfAMD2CMockWith("csf", c.unitFiles)
1275+
d := &productionPreflightDep{exec: mock}
1276+
_, _ = d.PreflightTarget(context.Background(), "csf")
1277+
for _, cmd := range mock.Commands {
1278+
if cmd.Name != "readlink" {
1279+
t.Errorf("recorded non-readonly command: %+v", cmd)
1280+
}
1281+
}
1282+
})
1283+
}
1284+
}
1285+
1286+
// AMD2C-10: existing 4B-1 ErrPreflightUnitMissing semantic preserved
1287+
// when no candidate path exists at all (FileExists false everywhere).
1288+
// Distinguishes "no file present" (UnitMissing) from "all files are
1289+
// mask-only" (UnitFileMaskingOnly).
1290+
func TestPreflightTarget_AMD2C_NoUnitFileAnywhere_StillUnitMissing(t *testing.T) {
1291+
mock := pfAMD2CMockWith("csf", nil) // no unit files seeded
1292+
d := &productionPreflightDep{exec: mock}
1293+
ok, err := d.PreflightTarget(context.Background(), "csf")
1294+
if ok {
1295+
t.Errorf("PreflightTarget(csf) accepted with no unit files; want refusal")
1296+
}
1297+
if !errors.Is(err, ErrPreflightUnitMissing) {
1298+
t.Errorf("err = %v; want ErrPreflightUnitMissing (NOT MaskingOnly — no file ever existed)", err)
10631299
}
10641300
}

0 commit comments

Comments
 (0)