v1.100 PR-22A — uninstall dry-run boundary repair (no-mutation + history + classifier fix)

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

@@ -96,10 +96,15 @@ jobs:
             exit 1
           fi
           fail=0
+          # PR-22A audit fix: the original grep missed os.WriteFile /
+          # os.Create / os.MkdirAll / os.Rename, which is exactly how
+          # the uninstall_plan.json write slipped past PR-22 review.
+          # Direct os.* filesystem writers are now enumerated here.
           for pat in \
               'exec\.Run\("nft"[^)]*add' \
               'exec\.Run\("nft"[^)]*flush' \
               'exec\.Run\("nft"[^)]*delete' \
+              'exec\.Run\("nft"[^)]*create' \
               'exec\.Run\("systemctl"[^)]*stop' \
               'exec\.Run\("systemctl"[^)]*start' \
               'exec\.Run\("systemctl"[^)]*restart' \
@@ -112,16 +117,29 @@ jobs:
               'exec\.Run\("iptables"[^-]' \
               'exec\.Run\("csf"' \
               'exec\.Run\("apt-get"[^)]*remove' \
+              'exec\.Run\("apt-get"[^)]*purge' \
               'exec\.Run\("dnf"[^)]*remove' \
+              'exec\.Run\("dnf"[^)]*erase' \
               'exec\.WriteFileAtomic\(' \
+              'os\.WriteFile\(' \
+              'os\.Create\(' \
+              'os\.MkdirAll\(' \
+              'os\.Mkdir\(' \
+              'os\.Rename\(' \
               'os\.Remove\(' \
-              'os\.RemoveAll\('; do
+              'os\.RemoveAll\(' \
+              'sf\.Transition\('; do
             # Note: ".conf.local" is NOT in this list — the contract
             # language in plan.go + contract.md LEGITIMATELY mentions
             # .conf.local when describing the §4.4 artifact policy.
             # The mutation danger is write/delete operations, which
-            # the os.Remove + WriteFileAtomic patterns above already
-            # cover regardless of which file they target.
+            # the os.*/exec.WriteFileAtomic patterns above cover
+            # regardless of which file they target.
+            #
+            # sf.Transition is forbidden in PR-22 scope because it
+            # persists install_state under /var/lib/nftban/state. PR-22
+            # is observational only — no state transitions during
+            # dry-run.
             if grep -nE "$pat" $files 2>/dev/null; then
               echo "::error::G3-UN-NO-MUTATION FAIL: forbidden pattern '$pat' in PR-22 uninstall scope"
               fail=1
@@ -145,6 +163,17 @@ jobs:
           set -Eeuo pipefail
           go test -v ./internal/installer/uninstall/...
 
+      # PR-22A boundary repair: the orchestrator-level purity test lives
+      # in cmd/nftban-installer/uninstall_dryrun_test.go. It falsifies
+      # the "no mutation" claim directly by calling runUninstallDryRun
+      # with a MockExecutor + real tempdir and asserting zero writes,
+      # zero directory creates, and zero mutation-flavored commands.
+      - name: Unit tests — runUninstallDryRun purity (R4)
+        shell: bash
+        run: |
+          set -Eeuo pipefail
+          go test -v -run 'TestRunUninstallDryRun_' ./cmd/nftban-installer/...
+
       # ------------------------------------------------------------------
       # Build the installer binary so we can exercise end-to-end.
       # ------------------------------------------------------------------
@@ -172,6 +201,12 @@ jobs:
         shell: bash
         run: |
           set -Eeuo pipefail
+          # PR-22A boundary repair: snapshot /var/lib/nftban/ truth
+          # surfaces BEFORE the dry-run. After the dry-run, the snapshot
+          # must be byte-identical. Any difference is a PR-22 contract
+          # Stop Condition violation.
+          before_varlib=$(find /var/lib/nftban -type f 2>/dev/null | sort | xargs -r sha256sum 2>/dev/null | sort || true)
+          before_etcnftban=$(find /etc/nftban -type f 2>/dev/null | sort | xargs -r sha256sum 2>/dev/null | sort || true)
           set +e
           sudo ./bin/nftban-installer --mode=uninstall --dry-run \
               --state-dir=/var/lib/nftban/state 2>&1 | tee /tmp/uninstall-dryrun.out
@@ -182,6 +217,19 @@ jobs:
             echo "::error::uninstall dry-run must exit 0 even on a host with no firewall authority"
             exit 1
           fi
+          after_varlib=$(find /var/lib/nftban -type f 2>/dev/null | sort | xargs -r sha256sum 2>/dev/null | sort || true)
+          after_etcnftban=$(find /etc/nftban -type f 2>/dev/null | sort | xargs -r sha256sum 2>/dev/null | sort || true)
+          if [[ "$before_varlib" != "$after_varlib" ]]; then
+            echo "::error::PR-22 Stop Condition violated — uninstall dry-run modified /var/lib/nftban/"
+            diff <(echo "$before_varlib") <(echo "$after_varlib") || true
+            exit 1
+          fi
+          if [[ "$before_etcnftban" != "$after_etcnftban" ]]; then
+            echo "::error::PR-22 Stop Condition violated — uninstall dry-run modified /etc/nftban/"
+            diff <(echo "$before_etcnftban") <(echo "$after_etcnftban") || true
+            exit 1
+          fi
+          echo "G3-UN-NO-WRITES PASS — /var/lib/nftban and /etc/nftban unchanged after dry-run"
           # Mandatory contract-language elements — each one corresponds
           # to a row PR-22 promised to render.
           for needle in \
@@ -212,24 +260,55 @@ jobs:
         shell: bash
         run: |
           set -Eeuo pipefail
-          # Snapshot nothing should change outside /var/lib/nftban/state.
-          before=$(find /etc/nftban /usr/lib/nftban /usr/sbin/nftban* -type f 2>/dev/null | sort | xargs -r sha256sum 2>/dev/null | sort || true)
+          # PR-22A boundary repair: the earlier gate only snapshot
+          # /etc/nftban + /usr/lib/nftban + /usr/sbin/nftban*. It did
+          # NOT cover /var/lib/nftban/ — the exact directory that leaked
+          # three writes in PR #480. The snapshot now includes all
+          # three protected trees. No exceptions.
+          before=$(find /etc/nftban /usr/lib/nftban /usr/sbin/nftban* /var/lib/nftban -type f 2>/dev/null | sort | xargs -r sha256sum 2>/dev/null | sort || true)
           set +e
           sudo ./bin/nftban-installer --mode=uninstall \
               --state-dir=/var/lib/nftban/state 2>&1 | tee /tmp/uninstall-implicit.out
           rc=${PIPESTATUS[0]}
           set -e
-          after=$(find /etc/nftban /usr/lib/nftban /usr/sbin/nftban* -type f 2>/dev/null | sort | xargs -r sha256sum 2>/dev/null | sort || true)
+          after=$(find /etc/nftban /usr/lib/nftban /usr/sbin/nftban* /var/lib/nftban -type f 2>/dev/null | sort | xargs -r sha256sum 2>/dev/null | sort || true)
           if [[ "$before" != "$after" ]]; then
-            echo "::error::PR-22 contract violation — --mode=uninstall touched files outside /var/lib/nftban/state"
+            echo "::error::PR-22 contract violation — --mode=uninstall touched protected filesystem"
             diff <(echo "$before") <(echo "$after") || true
             exit 1
           fi
           if [[ "$rc" -ne 0 ]]; then
             echo "::error::--mode=uninstall (no explicit dry-run) must exit 0 in PR-22 scope"
             exit 1
           fi
-          echo "G3-UN-PLAN-RENDERS no-mutation PASS — no files changed under protected paths"
+          echo "G3-UN-PLAN-RENDERS no-mutation PASS — no files changed under /etc/nftban, /usr/lib/nftban, /usr/sbin/nftban*, /var/lib/nftban"
+
+      # ------------------------------------------------------------------
+      # G3-UN-HISTORY-PURITY — regression guard for audit finding A.3.
+      # Seeds a realistic update-history.json, runs the dry-run twice
+      # (explicit + implicit), then asserts the history file is byte-
+      # identical. Before PR-22A, each dry-run appended an install_fail
+      # entry here.
+      # ------------------------------------------------------------------
+      - name: G3-UN-HISTORY-PURITY — history file unchanged by dry-run
+        shell: bash
+        run: |
+          set -Eeuo pipefail
+          hist=/var/lib/nftban/update-history.json
+          sudo mkdir -p /var/lib/nftban
+          echo '{"schema_version":"1.0","entries":[{"from":"1.99.0","to":"1.100.0","status":"success","type":"rpm","duration":1,"hostname":"ci","timestamp":"2026-04-19T00:00:00Z"}]}' | sudo tee "$hist" >/dev/null
+          before_hash=$(sha256sum "$hist" | awk '{print $1}')
+          sudo ./bin/nftban-installer --mode=uninstall --dry-run --state-dir=/var/lib/nftban/state >/dev/null 2>&1 || true
+          sudo ./bin/nftban-installer --mode=uninstall --state-dir=/var/lib/nftban/state >/dev/null 2>&1 || true
+          after_hash=$(sha256sum "$hist" | awk '{print $1}')
+          if [[ "$before_hash" != "$after_hash" ]]; then
+            echo "::error::G3-UN-HISTORY-PURITY FAIL — uninstall dry-run modified update-history.json"
+            echo "before: $before_hash"
+            echo "after:  $after_hash"
+            cat "$hist"
+            exit 1
+          fi
+          echo "G3-UN-HISTORY-PURITY PASS — update-history.json byte-identical after 2 dry-runs"
 
   summary:
     name: Uninstall Canonization summary

cmd/nftban-installer/main.go

@@ -98,8 +98,17 @@ func main() {
 
 	exitCode := run(ctx, exec, sf, cfg, log)
 
-	// Write JSON update history (compatible with nftban update history --json)
-	writeHistory(sf, cfg, previousVersion, hostname, log)
+	// Write JSON update history (compatible with nftban update history --json).
+	//
+	// PR-22A boundary repair (audit finding A.3): uninstall-mode dry-run
+	// is strictly observational and must not create install/update history
+	// entries. Before this guard, a successful uninstall dry-run would
+	// reach historyStatusForState(StateUninstallPlanning), fall through to
+	// the default case, and be recorded as "install_fail" — poisoning the
+	// audit trail and any dashboard that alerts on install_fail.
+	if cfg.mode != "uninstall" {
+		writeHistory(sf, cfg, previousVersion, hostname, log)
+	}
 
 	// Write run footer with final state for post-mortem
 	log.RunFooter(string(sf.State), exitCode)

cmd/nftban-installer/uninstall_dryrun.go

@@ -45,6 +45,26 @@ import (
 	"github.com/itcmsgr/nftban/internal/installer/uninstall"
 )
 
+// PR-22A boundary repair (audit finding A):
+//
+//   The original PR-22 orchestrator persisted two artifacts under
+//   /var/lib/nftban/ during a dry-run that the CLI banner described as
+//   "no mutation": a JSON plan artifact under the state directory, and
+//   an installer-state transition to the planning state. Both tripped
+//   the PR-22 contract Stop Condition: "writing to anywhere under
+//   /etc/nftban/ or /var/lib/nftban/ → STOP PR-22."
+//
+//   PR-22A removes both. The plan is rendered to stdout for the operator
+//   and callers that capture stdout can consume the JSON form via the
+//   exported Plan.JSON method. No filesystem persistence occurs during
+//   the dry-run. Operators who need a machine-consumable plan can
+//   capture stdout; the dry-run remains strictly observational, as PR-22
+//   originally promised.
+//
+//   Installer-state persistence during dry-run is deliberately removed
+//   (Option B in the repair contract). A read-only planning run has no
+//   reason to change the installer state file.
+
 // runUninstallDryRun orchestrates the PR-22 detect + plan-render flow.
 //
 // Exit codes:
@@ -86,26 +106,19 @@ func runUninstallDryRun(_ context.Context, exec executor.Executor, sf *state.Sta
 	plan := uninstall.BuildPlan(mode, auth, prior, restoreRequested)
 	log.PhaseEnd("Plan")
 
-	// 5. Render to stdout (operator-visible) and persist JSON to state
-	// dir for audit trail. Persisting is NOT mutation of install state;
-	// it is a plan artifact under the installer's own state dir.
+	// 5. Render to stdout (operator-visible). No filesystem persistence —
+	// the dry-run is strictly observational per PR-22 contract Stop
+	// Condition. Machine consumers can capture stdout; see also Note A
+	// above for the PR-22A audit rationale.
 	plan.Render(os.Stdout)
 
-	if data, err := plan.JSON(); err == nil {
-		dst := cfg.stateDir + "/uninstall_plan.json"
-		if werr := os.WriteFile(dst, data, 0600); werr != nil { //lint:ignore G306 plan audit artifact under state dir
-			log.Warn("uninstall dry-run: could not persist plan to %s: %v", dst, werr)
-		} else {
-			log.Info("uninstall dry-run: plan written to %s", dst)
-		}
-	}
-
-	// 6. Transition to the planning terminal state so the state file
-	// reflects what happened, and exit 0. No further phases exist.
-	_ = sf.Transition(state.StateUninstallPlanning, state.PhaseDetect,
-		"uninstall plan computed; no mutation (PR-22 scope)")
-
-	// 7. Emit the literal scope-boundary log marker per PR-22 contract.
+	// 6. Emit the literal scope-boundary log marker per PR-22 contract.
+	// The state file is NOT transitioned here: a read-only plan computes
+	// no state change, and persisting StateUninstallPlanning during a
+	// dry-run would trip the same Stop Condition the orchestrator
+	// exists to respect. sf is retained in the signature for symmetry
+	// with install/update orchestrators and for PR-23 takeover.
+	_ = sf
 	log.Info("uninstall dry-run complete — PR-22 scope boundary: NO mutation phase exists in this release")
 	fmt.Fprintln(os.Stderr, "uninstall dry-run: plan rendered; no mutation (PR-22 scope)")
 	return state.ExitCommitted