Skip to content

Commit 45fc63e

Browse files
itcmsgrclaude
andauthored
feat(v1.100 PR-26-code-B): typed executor.ServiceUnmask + executor.Rename + raw-Run policy tightening (#515)
PR-26-code-B — restore verification / evidence hardening, slice B. Executor hardening per §43 lock + §51.5-A2 invariant. CSF restore A.1 + A.3 migrate from raw Run("systemctl","unmask",…) + Run("mv",…) indirections to typed executor methods. Authority: - PR #512 / contract.md Part IV §§37-50 - PR #513 / §51 lock record (§51.5-A2: read-only typed introspection is outside the mutation cap; this commit's ServiceUnmask + Rename ARE mutation surfaces, but already enumerated by §44 row 2) - PR #514 / code-A merge 4e98ff5 - §43 executor hardening - §46 CI gate requirements - §51 code-B authorization Behavior delta: - Before: A.1 used exec.Run("systemctl","unmask",csfServiceUnit) wrapped by helper unmaskCSFService; A.3 used exec.Run("mv",old,new) wrapped by helper renameAtomicViaExec. - After: A.1 uses m.exec.ServiceUnmask(csfServiceUnit); A.3 uses m.exec.Rename(csfBinaryDisabled, csfBinary). Both helpers REMOVED. Mutation surface is unchanged in operational meaning; the typed call shape lets the CI gate enforce per-call discipline at the symbol level instead of via fragile Run-arg parsing. Files changed (6): internal/installer/executor/executor.go - Executor interface gains: ServiceUnmask(unit string) error // inverse of ServiceMask Rename(oldpath, newpath string) error // atomic same-FS rename - Header doc updated to list both new methods. internal/installer/executor/real.go - RealExecutor.ServiceUnmask runs systemctl unmask via Run with the same typed error wrapping pattern as ServiceMask. - RealExecutor.Rename calls os.Rename directly (consistent with WriteFileAtomic's existing os.Rename usage). No process spawn. internal/installer/executor/mock.go - MockExecutor.ServiceUnmask records ("systemctl","unmask",unit) and returns m.ServiceUnmaskErr (nil by default). - MockExecutor.Rename records ("rename",oldpath,newpath), returns m.RenameErr if non-nil; otherwise simulates the rename in the in-memory Files map. - New error-injection fields: ServiceUnmaskErr, RenameErr — mirror the RunResults exit-code injection pattern. cmd/nftban-installer/restore_deps_csf.go - Helpers unmaskCSFService and renameAtomicViaExec REMOVED. Replaced by a comment block documenting the §43.2 lock. - A.1 call site: m.exec.ServiceUnmask(csfServiceUnit). - A.3 call site: m.exec.Rename(csfBinaryDisabled, csfBinary). - No raw Run("systemctl","unmask",…) and no raw Run("mv",…) remain. - Log messages preserved; error wrapping (ErrCSFRestoreUnmaskFailed + ErrCSFRestoreBinaryRestoreFailed) preserved. cmd/nftban-installer/restore_deps_csf_test.go - buildCSFFixture: unmaskFailsExit injects mock.ServiceUnmaskErr; mvBinaryFailsExit injects mock.RenameErr (the previous RunResults-based simulation is removed; the OnCommand callback that simulated the rename in the Files map is also removed — Mock.Rename does that natively now). - TestCSFMutate_4B3csf_A3_* tests updated: assertions move from CommandCalled("mv", …) to CommandCalled("rename", …) because Mock.Rename records "rename", not "mv". - HappyPath_NoOutOfTargetMutation allow-list and OrderingPin expected sequences updated: A.3's recorded shape becomes ("rename", oldpath, newpath) instead of ("mv", oldpath, newpath). - Seven new TestCSFMutate_PR26B_* tests added: 1. A1_ServiceUnmaskOnlyCSFService — pins ServiceUnmask called only on csf.service 2. A3_RenameOnlyCSFBinaryRestore — pins Rename called only with (csf.disabled → csf) 3. NoRawSystemctlUnmaskRun_FileScan — pins no raw Run("systemctl","unmask",…) remains in production source 4. NoRawMvRun_FileScan — pins no raw Run("mv",…) remains 5. A1_UnmaskFailure_TypedErrorPreserved — error contract preserved through the migration 6. A3_RenameFailure_TypedErrorPreserved — same for A.3 7. RemovedHelpersGone_FileScan — pins removal of the unmaskCSFService and renameAtomicViaExec function definitions .github/workflows/ci-restore-canonization.yml - G4-RESTORE-EXEC-NO-OUT-OF-TARGET strengthened per §43.4 + §46.1: * \bexec\.ServiceUnmask\( REMOVED from forbidden list (now the authorized typed method for A.1). * Added forbidden patterns for raw Run("systemctl",…) with any mutating verb (start/stop/enable/disable/mask/unmask/restart/ reload/daemon-reload). Read-only Run("systemctl","is-enabled",…) remains authorized. * Added forbidden pattern for raw Run("mv",…). Typed Rename is the only authorized atomic-rename path. * §46.1 line-skipping discipline applied: gate strips line-leading "//" comments before pattern matching, preventing the false-positive class that broke Policy Gates on PR #511. * Header rewritten to reflect the post-PR-26-code-B authorized mutation set (typed ServiceUnmask / typed Rename; no raw Run for mutating systemctl verbs or mv). Constraints honored (per §51.6 + operator scope): IN scope: - typed executor.ServiceUnmask ✓ - typed executor.Rename ✓ - migration of CSF restore A.1 + A.3 to typed methods ✓ - raw Run policy tightening (CI gate) ✓ - G4-RESTORE-EXEC-NO-OUT-OF-TARGET strengthened ✓ OUT of scope (and untouched): - cron backup / A.4 (PR-26-code-C) - destructive soak (PR-26-code-E) - IptablesRuleExists / iptables introspection (Option B lock) - target-specific predicate changes (already done in code-A) - inline verification behavior changes - restore decision lattice - TargetAuthority / planner - main.go history gate - state machine / exit codes - contract.md - repo hygiene / UX / GOTH / metrics / module cleanup Verified on lab2 (Ubuntu 24.04, go1.22.2): - go build ./... clean - go test ./cmd/nftban-installer/... ./internal/installer/restore/... ./internal/installer/state/... ./internal/installer/executor/... PASS - go test ./... PASS (full suite) - go test -race -count=1 ./cmd/nftban-installer ./internal/installer/restore/... ./internal/installer/state/... PASS - go vet (cmd + restore + state + executor) clean - go mod tidy no-op - 7 new TestCSFMutate_PR26B_* tests all PASS - Local replay of strengthened G4-RESTORE-EXEC-NO-OUT-OF-TARGET gate: FAIL=0 against the migrated restore_deps_csf.go (only authorized NftDeleteTable("ip","nftban") + NftDeleteTable("ip6","nftban") calls; no raw mutating Run, no os.* bypass, no custombuild/iptables/rebuild/purge symbols). Awaiting auditor pass before push. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4e98ff5 commit 45fc63e

6 files changed

Lines changed: 360 additions & 94 deletions

File tree

.github/workflows/ci-restore-canonization.yml

Lines changed: 60 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -263,31 +263,35 @@ jobs:
263263
# ------------------------------------------------------------------
264264
# G4-RESTORE-EXEC-NO-OUT-OF-TARGET — closed mutation surface in
265265
# the cmd/-side restore-execution code. Per Amendment 1 §31 / §32,
266-
# the only authorized mutations on the CSF restore path are:
266+
# the authorized mutations on the CSF restore path are:
267267
#
268-
# - Run("systemctl", "unmask", "csf.service") A.1
269-
# - ServiceEnable("csf.service") A.2
270-
# - Run("mv", "/usr/sbin/csf.disabled",
271-
# "/usr/sbin/csf") A.3
272-
# - ServiceStart("csf.service") A.5
273-
# - ServiceStop("nftband.service") A.6
274-
# - NftDeleteTable("ip", "nftban") A.7
275-
# - NftDeleteTable("ip6", "nftban") A.7
268+
# - exec.ServiceUnmask("csf.service") A.1 (typed; PR-26-code-B)
269+
# - exec.ServiceEnable("csf.service") A.2
270+
# - exec.Rename("/usr/sbin/csf.disabled",
271+
# "/usr/sbin/csf") A.3 (typed; PR-26-code-B)
272+
# - exec.ServiceStart("csf.service") A.5
273+
# - exec.ServiceStop("nftband.service") A.6
274+
# - exec.NftDeleteTable("ip", "nftban") A.7
275+
# - exec.NftDeleteTable("ip6", "nftban") A.7
276276
#
277-
# The gate's job is forbidden-symbol coverage at the call-expression
278-
# level. Per-call argument enforcement (which unit is started, which
279-
# path is renamed) is delegated to the in-Go runtime tests in
280-
# restore_deps_csf_test.go (TestCSFMutate_4B3csf_A1_NoUnmaskOfOtherServices,
277+
# PR-26-code-B (§43) promoted A.1 + A.3 from raw `Run("systemctl",
278+
# "unmask", …)` and `Run("mv", …)` to the typed methods listed
279+
# above. The gate's old `\bexec\.ServiceUnmask\(` forbid is
280+
# therefore REMOVED, and explicit raw-Run forbids replace it.
281+
#
282+
# The gate's job is forbidden-symbol coverage. Per-call argument
283+
# enforcement (which unit is started, which path is renamed) is
284+
# delegated to the in-Go runtime tests in restore_deps_csf_test.go
285+
# (TestCSFMutate_4B3csf_A1_NoUnmaskOfOtherServices,
281286
# …_A2_EnableOnlyCSFService, …_A5_StartsOnlyCSFService,
282-
# …_A6_StopsOnlyNftband_AfterCSFStarts,
283-
# …_HappyPath_NoOutOfTargetMutation) which assert against
284-
# MockExecutor with full Go-level type information. Trying to
285-
# reproduce that in shell regex on identifier arguments
286-
# (csfServiceUnit, oldpath/newpath) gives false confidence.
287+
# …_A6_StopsOnlyNftband_AfterCSFStarts, …_HappyPath_NoOutOfTargetMutation,
288+
# TestCSFMutate_PR26B_A1_ServiceUnmaskOnlyCSFService,
289+
# TestCSFMutate_PR26B_A3_RenameOnlyCSFBinaryRestore).
287290
#
288-
# Forbidden patterns are call-expression-anchored (\bexec\.) so
289-
# they catch real call sites and skip prose / error messages /
290-
# comments / const definitions.
291+
# §46.1 line-skipping discipline: the gate scans only the
292+
# production file (no _test.go), and skips line-leading "//"
293+
# comments to avoid the false-positive class that hit Policy
294+
# Gates on PR #511.
291295
# ------------------------------------------------------------------
292296
- name: G4-RESTORE-EXEC-NO-OUT-OF-TARGET — restore_deps_csf.go closed mutation surface
293297
shell: bash
@@ -300,23 +304,24 @@ jobs:
300304
exit 1
301305
fi
302306
307+
# §46.1 production-only scan: build a comment-stripped view
308+
# so doc-comment lines never trip the forbidden-pattern grep.
309+
stripped=$(grep -vE '^[[:space:]]*//' "$target" || true)
310+
303311
# Forbidden: out-of-Amendment-1 mutation surface. Each pattern
304312
# is anchored to a call expression (\bexec\. or similar) so
305-
# legitimate doc strings, error messages, and const
306-
# definitions don't false-match.
313+
# legitimate string literals don't false-match. Doc comments
314+
# are already excluded by the line-leading // strip above.
307315
forbidden_patterns=(
308-
# Service-policy mutations (mask/disable/unmask-typed/daemon-reload).
309-
# The csf restore path uses Run("systemctl","unmask",csf.service)
310-
# for A.1; the typed exec.ServiceUnmask method is forbidden so
311-
# we don't accidentally land on a future executor surface that
312-
# bypasses the audit trail.
316+
# Service-policy mutations (mask/disable/daemon-reload) are
317+
# forbidden by Amendment 1 §34. ServiceUnmask is no longer
318+
# forbidden — PR-26-code-B promoted it to authorized typed
319+
# method for A.1.
313320
'\bexec\.ServiceMask\('
314321
'\bexec\.ServiceDisable\('
315-
'\bexec\.ServiceUnmask\('
316322
'\bexec\.DaemonReload\('
317-
# Filesystem-mutation primitives — A.4 cron-restore is a
318-
# soft-skip in 4B-3-csf; any WriteFileAtomic in this file is
319-
# therefore an out-of-scope mutation.
323+
# Filesystem-mutation primitives. A.4 cron-restore is a
324+
# soft-skip; any WriteFileAtomic here is out of scope.
320325
'\bexec\.WriteFileAtomic\('
321326
# Direct OS bypass (must route through the executor).
322327
'"os/exec"'
@@ -326,6 +331,22 @@ jobs:
326331
'\bos\.WriteFile\('
327332
'\bos\.Create\('
328333
'\bsyscall\.'
334+
# PR-26-code-B raw-Run policy tightening (§43.3): mutating
335+
# systemctl verbs MUST go through their typed methods.
336+
# Read-only `Run("systemctl", "is-enabled", …)` and similar
337+
# probes remain authorized.
338+
'Run\("systemctl",[[:space:]]*"start"'
339+
'Run\("systemctl",[[:space:]]*"stop"'
340+
'Run\("systemctl",[[:space:]]*"enable"'
341+
'Run\("systemctl",[[:space:]]*"disable"'
342+
'Run\("systemctl",[[:space:]]*"mask"'
343+
'Run\("systemctl",[[:space:]]*"unmask"'
344+
'Run\("systemctl",[[:space:]]*"restart"'
345+
'Run\("systemctl",[[:space:]]*"reload"'
346+
'Run\("systemctl",[[:space:]]*"daemon-reload"'
347+
# Raw mv via Run is forbidden — typed exec.Rename is the
348+
# only authorized atomic-rename surface.
349+
'Run\("mv"\b'
329350
# DirectAdmin custombuild rewrite forbidden (§34).
330351
'\bcustombuild\b'
331352
'"build set csf"'
@@ -343,8 +364,11 @@ jobs:
343364
344365
fail=0
345366
for pat in "${forbidden_patterns[@]}"; do
346-
if grep -nE "$pat" "$target" 2>/dev/null; then
367+
if echo "$stripped" | grep -nE "$pat" >/dev/null 2>&1; then
368+
# Re-grep against the original file (with line numbers)
369+
# so the error message points at the right line.
347370
echo "::error::G4-RESTORE-EXEC-NO-OUT-OF-TARGET: forbidden call expression matching '$pat' found in $target"
371+
grep -nE "$pat" "$target" || true
348372
fail=1
349373
fi
350374
done
@@ -354,8 +378,8 @@ jobs:
354378
# NftDeleteTable's args in the production code ARE literal
355379
# quoted strings (no constants), so this allow-list pin works
356380
# cleanly without identifier resolution. Per-unit / per-path
357-
# enforcement for systemctl + mv is delegated to the Go runtime
358-
# tests against MockExecutor.
381+
# enforcement for systemctl + Rename is delegated to the Go
382+
# runtime tests against MockExecutor.
359383
while IFS= read -r line; do
360384
args=$(echo "$line" | grep -oE 'NftDeleteTable\([^)]*\)' || true)
361385
if [[ -n "$args" ]]; then
@@ -368,7 +392,7 @@ jobs:
368392
;;
369393
esac
370394
fi
371-
done < <(grep -n 'NftDeleteTable(' "$target" || true)
395+
done < <(echo "$stripped" | grep -n 'NftDeleteTable(' || true)
372396
373397
if [[ "$fail" -ne 0 ]]; then
374398
echo "::error::Amendment 1 §30 / §34 violation — restore_deps_csf.go must contain only the closed §31/§32 mutation surface."

cmd/nftban-installer/restore_deps_csf.go

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -182,32 +182,13 @@ func isCSFServiceMasked(exec executor.Executor) bool {
182182
return strings.TrimSpace(res.Stdout) == "masked"
183183
}
184184

185-
// unmaskCSFService runs systemctl unmask via the executor abstraction.
186-
// The Executor interface does not expose a typed ServiceUnmask method
187-
// (only ServiceMask). Routing the inverse through Run is the chosen
188-
// in-scope option for 4B-3-csf — it stays inside cmd/ and uses the
189-
// existing executor seam without expanding the interface mid-PR.
190-
func unmaskCSFService(exec executor.Executor) error {
191-
res := exec.Run("systemctl", "unmask", csfServiceUnit)
192-
if res.ExitCode != 0 {
193-
return fmt.Errorf("systemctl unmask %s: %s", csfServiceUnit, strings.TrimSpace(res.Stderr))
194-
}
195-
return nil
196-
}
197-
198-
// renameAtomicViaExec performs an atomic rename via the executor's
199-
// Run("mv", ...) — same-filesystem mv is atomic at the syscall level.
200-
// The Executor interface does not expose a Rename method; routing
201-
// through Run keeps the surface inside the executor abstraction. The
202-
// direct stdlib rename API (in the os package) is forbidden by the
203-
// file-scan and intentionally not used here.
204-
func renameAtomicViaExec(exec executor.Executor, oldpath, newpath string) error {
205-
res := exec.Run("mv", oldpath, newpath)
206-
if res.ExitCode != 0 {
207-
return fmt.Errorf("mv %s -> %s: %s", oldpath, newpath, strings.TrimSpace(res.Stderr))
208-
}
209-
return nil
210-
}
185+
// (PR-26-code-B / §43.2 lock — the prior unmaskCSFService and
186+
// renameAtomicViaExec helper functions are removed. Both routed
187+
// through the raw `Run("systemctl","unmask",…)` and `Run("mv",…)`
188+
// indirections that PR-26-code-B closes by promoting the operations
189+
// to typed `executor.ServiceUnmask` and `executor.Rename` methods.
190+
// The §31 A.1 + A.3 call sites in mutateToCSFTarget call those typed
191+
// methods directly — no thin wrapper is needed.)
211192

212193
// =============================================================================
213194
// mutateToCSFTarget — the §31/§32 entry point invoked by
@@ -292,7 +273,7 @@ func mutateToCSFTarget(ctx context.Context, m *productionMutationDep) error {
292273
if m.log != nil {
293274
m.log.Info("restore csf: A.1 unmasking %s", csfServiceUnit)
294275
}
295-
if err := unmaskCSFService(m.exec); err != nil {
276+
if err := m.exec.ServiceUnmask(csfServiceUnit); err != nil {
296277
return fmt.Errorf("%w: %v", ErrCSFRestoreUnmaskFailed, err)
297278
}
298279
} else if m.log != nil {
@@ -317,7 +298,7 @@ func mutateToCSFTarget(ctx context.Context, m *productionMutationDep) error {
317298
if m.log != nil {
318299
m.log.Info("restore csf: A.3 renaming %s -> %s", csfBinaryDisabled, csfBinary)
319300
}
320-
if err := renameAtomicViaExec(m.exec, csfBinaryDisabled, csfBinary); err != nil {
301+
if err := m.exec.Rename(csfBinaryDisabled, csfBinary); err != nil {
321302
return fmt.Errorf("%w: %v", ErrCSFRestoreBinaryRestoreFailed, err)
322303
}
323304
} else if m.log != nil {

0 commit comments

Comments
 (0)