Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions .github/workflows/ci-update-canonization.yml
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,84 @@ jobs:
set -Eeuo pipefail
go test -v ./internal/installer/update/... ./cmd/nftban-installer/...

# ------------------------------------------------------------------
# PR-19 G3-U11 — state↔exit agreement. Structural check: every
# state.Transition(...) call inside runUpdateApply must be followed
# by a return of either that state's own ExitCode() OR the exit
# code variable that drove the state selection (preventing
# hard-coded contradictions like the original StateFailedAbort +
# ExitDegraded split).
# ------------------------------------------------------------------
- name: G3-U11 — state↔exit agreement structural check
shell: bash
run: |
set -Eeuo pipefail
src=cmd/nftban-installer/update_apply.go
# Forbidden pattern: any "sf.Transition(state.StateFailed..."
# paired with "return state.ExitDegraded" in the same branch.
# We detect this by looking for returns of literal exit constants
# adjacent to StateFailed transitions.
if grep -B3 -nE 'return state\.(ExitDegraded|ExitCommitted)' "$src" | \
grep -qE 'sf\.Transition\(state\.StateFailed'; then
echo "::error::G3-U11 FAIL: hard-coded exit literal returned after StateFailed transition — use st.ExitCode() or <phase>.ExitCode"
exit 1
fi
echo "G3-U11 PASS — no state↔exit contradiction detected structurally"

# ------------------------------------------------------------------
# PR-19 G3-U12 — history integrity. Unit tests above already cover
# every state → status mapping. This step is an explicit grep to
# catch any future "success" coercion — if someone later hard-codes
# a success status for a non-committed state, CI fails.
# ------------------------------------------------------------------
- name: G3-U12 — history status-from-state non-coercion
shell: bash
run: |
set -Eeuo pipefail
src=cmd/nftban-installer/main.go
# Every "return history.StatusSuccess" must be on a line where
# the previous non-empty line is "case state.StateCommitted:".
# This catches future changes that move StatusSuccess to a
# different case arm.
lines=$(grep -n 'history\.StatusSuccess' "$src" | wc -l)
if [[ "$lines" -eq 0 ]]; then
echo "::error::G3-U12 FAIL: no history.StatusSuccess reference found at all"
exit 1
fi
# For each line N containing StatusSuccess, confirm line N-1
# contains "case state.StateCommitted:" (after trimming).
fail=0
while IFS=: read -r lineno _; do
prev=$((lineno - 1))
prevline=$(sed -n "${prev}p" "$src")
if ! echo "$prevline" | grep -qE 'case\s+state\.StateCommitted'; then
echo "::error::G3-U12 FAIL: history.StatusSuccess at line $lineno not preceded by 'case state.StateCommitted:' — prev line: $prevline"
fail=1
fi
done < <(grep -n 'history\.StatusSuccess' "$src")
if (( fail > 0 )); then
exit 1
fi
echo "G3-U12 PASS — every history.StatusSuccess is gated by case state.StateCommitted"

# ------------------------------------------------------------------
# PR-19 G3-U13 — source/package coherence. Unit test
# TestHistoryInstallType_Source asserts --source → "source". This
# step adds a structural check: historyInstallType's switch MUST
# contain a cfg.source case. Catches accidental regression on the
# "source installs labeled as rpm" bug.
# ------------------------------------------------------------------
- name: G3-U13 — historyInstallType has source case
shell: bash
run: |
set -Eeuo pipefail
src=cmd/nftban-installer/main.go
if ! grep -qE 'case cfg\.source' "$src"; then
echo "::error::G3-U13 FAIL: historyInstallType missing 'case cfg.source' — source installs will be mislabeled"
exit 1
fi
echo "G3-U13 PASS — source case present in historyInstallType"

summary:
name: Update Canonization summary
needs: update-canonization
Expand Down
130 changes: 130 additions & 0 deletions cmd/nftban-installer/history_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// =============================================================================
// NFTBan v1.99 PR-19 — History Integrity + Source/Package Coherence Tests
// =============================================================================
// SPDX-License-Identifier: MPL-2.0
// meta:name="nftban-installer-history-test"
// meta:type="test"
// meta:owner="Antonios Voulvoulis <contact@nftban.com>"
// meta:created_date="2026-04-19"
// meta:description="Unit tests for historyStatusForState + historyInstallType"
// meta:inventory.files="cmd/nftban-installer/history_test.go"
// meta:inventory.binaries=""
// meta:inventory.env_vars=""
// meta:inventory.config_files=""
// meta:inventory.systemd_units=""
// meta:inventory.network=""
// meta:inventory.privileges="none"
// =============================================================================
//
// PR-19 G3-U12 (update history integrity):
// - historyStatusForState must never coerce non-committed states to "success"
// - intermediate non-terminal states (e.g. DETECT_COMPLETE) report install_fail
//
// PR-19 G3-U13 (source/package coherence):
// - historyInstallType must return "source" for --source installs (not "rpm")
// - priority order: source > deb > rpm > default
//
// =============================================================================
package main

import (
"testing"

"github.com/itcmsgr/nftban/internal/installer/history"
"github.com/itcmsgr/nftban/internal/installer/state"
)

// historyStatusForState ───────────────────────────────────────────────────

func TestHistoryStatusForState_Committed_IsSuccess(t *testing.T) {
if got := historyStatusForState(state.StateCommitted); got != history.StatusSuccess {
t.Errorf("StateCommitted → %q; want %q", got, history.StatusSuccess)
}
}

func TestHistoryStatusForState_Degraded_IsVerifyFail(t *testing.T) {
if got := historyStatusForState(state.StateDegraded); got != history.StatusVerifyFail {
t.Errorf("StateDegraded → %q; want %q", got, history.StatusVerifyFail)
}
}

// G3-U12: every failure state maps to install_fail — no coercion to success.
func TestHistoryStatusForState_AllFailureStates_AreInstallFail(t *testing.T) {
failureStates := []state.InstallState{
state.StateFailedSSH,
state.StateFailedAbort,
state.StateFailedRender,
state.StateFailedRebuild,
state.StateFailedNoFirewall,
state.StateFailedTakeover,
}
for _, s := range failureStates {
if got := historyStatusForState(s); got != history.StatusInstallFail {
t.Errorf("failure state %s → %q; want %q (no success coercion)", s, got, history.StatusInstallFail)
}
}
}

// G3-U12 regression guard: intermediate non-terminal states must NOT report
// success. Previously an interrupted apply (timeout / signal) left sf.State
// on something like DETECT_COMPLETE, and a careless review of the status
// could misread this as "install in progress" — the history writer must
// record it as a failure so operator audits remain honest.
func TestHistoryStatusForState_IntermediateStates_AreInstallFail(t *testing.T) {
intermediateStates := []state.InstallState{
state.StateFilesInstalled,
state.StateDetectComplete,
state.StatePrepareComplete,
state.StateSwitchComplete,
state.StateServicesComplete,
}
for _, s := range intermediateStates {
if got := historyStatusForState(s); got != history.StatusInstallFail {
t.Errorf("intermediate state %s → %q; want %q (non-terminal = not a success)", s, got, history.StatusInstallFail)
}
}
}

// historyInstallType ──────────────────────────────────────────────────────

func TestHistoryInstallType_Source(t *testing.T) {
cfg := &config{source: true}
if got := historyInstallType(cfg); got != "source" {
t.Errorf("--source → %q; want %q (G3-U13 coherence: source installs must not be mislabeled)", got, "source")
}
}

func TestHistoryInstallType_DEB(t *testing.T) {
cfg := &config{deb: true}
if got := historyInstallType(cfg); got != "deb" {
t.Errorf("--deb → %q; want %q", got, "deb")
}
}

func TestHistoryInstallType_RPM(t *testing.T) {
cfg := &config{rpm: true}
if got := historyInstallType(cfg); got != "rpm" {
t.Errorf("--rpm → %q; want %q", got, "rpm")
}
}

func TestHistoryInstallType_Priority_SourceOverridesDEB(t *testing.T) {
cfg := &config{source: true, deb: true}
if got := historyInstallType(cfg); got != "source" {
t.Errorf("--source --deb → %q; want %q (source wins)", got, "source")
}
}

func TestHistoryInstallType_Priority_DEBOverridesRPM(t *testing.T) {
cfg := &config{deb: true, rpm: true}
if got := historyInstallType(cfg); got != "deb" {
t.Errorf("--deb --rpm → %q; want %q (deb wins, source not set)", got, "deb")
}
}

func TestHistoryInstallType_NoFlag_DefaultsToRPM(t *testing.T) {
cfg := &config{}
if got := historyInstallType(cfg); got != "rpm" {
t.Errorf("no flag → %q; want %q (legacy default)", got, "rpm")
}
}
85 changes: 67 additions & 18 deletions cmd/nftban-installer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,28 +332,30 @@ func report(sf *state.StateFile, log *logging.Logger) int {

// writeHistory writes a JSON entry to /var/lib/nftban/update-history.json
// compatible with `nftban update history --json`.
//
// PR-19 G3-U12 (history integrity): only StateCommitted is reported as
// success. Any other state — including non-terminal intermediate states
// from a timeout / signal mid-apply — maps to install_fail or
// verify_fail, never success. No coercion.
//
// PR-19 G3-U13 (source/package coherence): installType now has a
// "source" case. Source installs are no longer silently mislabeled as
// "rpm". Detection order: explicit --deb flag → "deb", explicit --source
// flag → "source", otherwise "rpm" (matches the RPM post-install hook
// default).
func writeHistory(sf *state.StateFile, cfg *config, previousVersion, hostname string, log *logging.Logger) {
// Map state to history status
var status string
switch {
case sf.State == state.StateCommitted:
status = history.StatusSuccess
case sf.State == state.StateDegraded:
status = history.StatusVerifyFail
default:
status = history.StatusInstallFail
}
// Map state to history status — no success coercion for non-terminal
// or non-committed states.
status := historyStatusForState(sf.State)

// Determine install type
installType := "rpm"
if cfg.deb {
installType = "deb"
}
// Determine install type — source installs must not be silently
// mislabeled as rpm/deb (G3-U13).
installType := historyInstallType(cfg)

// Duration from state file timestamp
// Duration from state file timestamp.
durationSecs := sf.RebuildDurationMs / 1000
if durationSecs == 0 {
// Fallback: use wall clock from run start (captured in logger)
// Fallback: use wall clock from run start (captured in logger).
durationSecs = 1
}

Expand All @@ -365,6 +367,53 @@ func writeHistory(sf *state.StateFile, cfg *config, previousVersion, hostname st
if err := history.WriteEntry("", entry); err != nil {
log.Warn("failed to write update history: %v", err)
} else {
log.Debug("wrote history entry: %s -> %s status=%s", previousVersion, sf.Version, status)
log.Debug("wrote history entry: %s -> %s status=%s type=%s", previousVersion, sf.Version, status, installType)
}
}

// historyStatusForState maps an InstallState to the history status string
// without coercing non-committed states into success. Extracted from
// writeHistory so the mapping is unit-testable in isolation.
//
// StateCommitted → "success"
// StateDegraded → "verify_fail"
// everything else → "install_fail" (including non-terminal
// intermediate states, which
// indicate the run was
// interrupted and never
// reached a terminal state)
func historyStatusForState(s state.InstallState) string {
switch s {
case state.StateCommitted:
return history.StatusSuccess
case state.StateDegraded:
return history.StatusVerifyFail
default:
return history.StatusInstallFail
}
}

// historyInstallType returns the install origin label for the history
// record. Priority: explicit --source > explicit --deb > explicit --rpm
// > default "rpm" (historical default for the RPM post-install hook).
//
// PR-19 G3-U13 fix: source installs were previously mislabeled as "rpm"
// because the switch only considered --deb and fell through to the
// hard-coded default.
func historyInstallType(cfg *config) string {
switch {
case cfg.source:
return "source"
case cfg.deb:
return "deb"
case cfg.rpm:
return "rpm"
default:
// No package-manager flag passed — caller is either the RPM
// post-install hook without the --rpm flag (legacy) or an
// operator-initiated apply. Default to "rpm" preserves the
// historical legacy behaviour; the history consumer can treat
// this as "origin unknown" if stricter attribution is needed.
return "rpm"
}
}
28 changes: 26 additions & 2 deletions cmd/nftban-installer/update_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,16 @@ func runUpdateApply(_ context.Context, exec executor.Executor, sf *state.StateFi
}
}
fmt.Fprintln(os.Stderr, "update apply: preflight failed — see log for details")
_ = sf.Transition(state.StateFailedAbort, state.PhasePrepare,
// PR-19 G3-U11: state↔exit agreement. Persist the state derived
// from preflight severity and return the SAME state's ExitCode().
// Previously this branch transitioned to StateFailedAbort (which
// maps to ExitAborted=3) but returned hard-coded ExitDegraded=1
// — a silent truth split of the same class PR-18 fixed for the
// validator-fail branch.
st := stateForPreflightFailure(pre)
_ = sf.Transition(st, state.PhasePrepare,
"update preflight failed before rebuild")
return state.ExitDegraded
return st.ExitCode()
}

// 2. Canonical rebuild entrypoint — the ONLY mutation path.
Expand Down Expand Up @@ -205,6 +212,23 @@ func postStateInspection(exec executor.Executor, log *logging.Logger) {
}
}

// stateForPreflightFailure maps a failing preflight result to the
// InstallState this apply run should persist. Keeps state↔exit aligned
// by construction — the returned state's ExitCode() equals the process
// exit runUpdateApply returns on this branch.
//
// Today: any critical preflight failure maps to StateFailedNoFirewall
// (ExitCode == ExitFailed == 2). This is honest: preflight exists to
// gate apply on "nftban is functionally present and authoritative" —
// a critical preflight failure means at least one of those conditions
// is not met. PR-19 intentionally avoids building a mini policy engine
// that varies the state per check; the signature accepts the full
// preflight result so a future PR can deepen mapping without changing
// callers.
func stateForPreflightFailure(_ *update.PreflightResult) state.InstallState {
return state.StateFailedNoFirewall
}

// stateForValidatorExit maps the validator binary's process exit code to
// the InstallState this apply run should persist. Local and narrow by
// design (PR-18 review blocker #1):
Expand Down
11 changes: 10 additions & 1 deletion cmd/nftban-installer/update_apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ func TestUpdateApply_HappyPath_Exits0(t *testing.T) {
}
}

// T2 — Preflight failure blocks rebuild invocation.
// T2 — Preflight failure blocks rebuild invocation AND maintains
// state↔exit agreement (PR-19 G3-U11 regression guard).
func TestUpdateApply_PreflightFail_DoesNotInvokeRebuild(t *testing.T) {
mock := executor.NewMockExecutor()
seedHappyApplyHost(mock)
Expand All @@ -133,6 +134,14 @@ func TestUpdateApply_PreflightFail_DoesNotInvokeRebuild(t *testing.T) {
t.Error("preflight-fail path must not return ExitCommitted")
}

// PR-19 G3-U11: persisted state's derived exit must equal returned rc.
if got := sf.State.ExitCode(); got != rc {
t.Errorf("state↔exit contradiction: state.ExitCode() = %d, rc = %d", got, rc)
}
if sf.State != state.StateFailedNoFirewall {
t.Errorf("persisted state = %s; want StateFailedNoFirewall for preflight-fail", sf.State)
}

// Rebuild must never have been invoked.
for _, c := range mock.Commands {
if c.Name == "nftban" && len(c.Args) >= 2 && c.Args[0] == "firewall" && c.Args[1] == "rebuild" {
Expand Down
Loading
Loading