controllerswap: F1 verify hardening — reject crash-looping no-healthcheck image v0.47.0

controllerHealthy reads RestartCount (running&&rc>0 -> not ok) + signals needsDwell for no-healthcheck;
verify requires verifyDwell(=3) consecutive ok polls for a no-healthcheck image (real healthcheck
trusted immediately). Closes the F1 hole (alpine crash-loop passed the point-in-time check). Red-proof
+ dwell + real-image tests. No sudoers/orchestration change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pg8ANF97SEeKYSN5Jxw3qJ
This commit is contained in:
2026-06-29 22:45:41 +02:00
parent bb548e3c5a
commit 3844df7c23
8 changed files with 218 additions and 57 deletions
+20
View File
@@ -1,3 +1,23 @@
## v0.47.0 — controller-swap verify hardening: reject a crash-looping no-healthcheck image (F1) (2026-06-29)
Closes F1 from the no-mercy testrun: a controller image with **no HEALTHCHECK that crash-loops** could
land a single "Running" inspect poll → the swap marked it healthy → **no rollback** (alpine tagged as
the controller passed in ~4 s, then `Restarting (0)`). The real controller image has a healthcheck so
the live severity is low, but the rollback safety net had a hole.
- **`internal/localapi/controllerswap.go`:** `controllerHealthy` now also reads `{{.RestartCount}}` (a
4th `docker inspect -f` field) — `running && RestartCount>0` → not-ok (a process that has already
crash-restarted isn't stably up, regardless of healthcheck). It also signals `needsDwell` for the
no-healthcheck (`none`) case. `verify` adds a **stability dwell**: a no-healthcheck image must report
ok on `verifyDwell` (=3) **consecutive** polls before it's accepted; a real `healthy` result is
trusted immediately (Docker already gated it). Any not-ok resets the dwell. Timeout → existing
rollback path runs. No change to writeImage, the sudoers grants (the `*` in `docker inspect -f *`
spans the extended template — confirmed live), or the state-file/rollback orchestration.
- Tests: F1 **red-proof** (`RestartCount>0` → verify false; companion: rc=0+dwell=1 verifies → the rc
check is what blocks it); the **dwell** (single ok then crash → verify false; companion dwell=1
accepts it); a real `healthy` image verifies promptly (no false rollback). Existing
`RollbackOnUnhealthy` / `HealthyWithNoHealthcheck` stay green. Version `0.46.0 → 0.47.0`.
## v0.46.0 — leaf lifecycle: signal + loud-log a regenerated leaf (prevention, Part B.1) (2026-06-29)
Makes an accidental local-API leaf **regeneration** (the 2026-06-28 root→non-root migration class —
+1 -1
View File
@@ -45,7 +45,7 @@ import (
// version is the agent version. Overridable at build time with
// -ldflags "-X main.version=<v>"; defaults to the in-repo CHANGELOG version.
var version = "0.46.0"
var version = "0.47.0"
// runGuestHook is the PVE pre-start hook body (`felhom-agent guest-hook <vmid> <phase>`). On the
// pre-start phase it creates placeholder dirs for any absent bind-mount source so the guest always boots
+48 -17
View File
@@ -10,6 +10,7 @@ import (
"os"
"path/filepath"
"regexp"
"strconv"
"strings"
"time"
)
@@ -65,6 +66,8 @@ type ControllerSwapper struct {
logger *slog.Logger
verifyTimeout time.Duration
verifyInterval time.Duration
verifyDwell int // F1: consecutive ok polls required to accept a NO-healthcheck image (a real
// healthcheck passes without dwell). Catches a slow crash-loop that hasn't bumped RestartCount yet.
}
// NewControllerSwapper builds a swapper. stateDir holds controller-swap-<vmid>.json (default
@@ -82,6 +85,7 @@ func NewControllerSwapper(exec GuestExecutor, stateDir string, logger *slog.Logg
logger: logger,
verifyTimeout: 90 * time.Second,
verifyInterval: 3 * time.Second,
verifyDwell: 3,
}
}
@@ -152,40 +156,67 @@ func (c *ControllerSwapper) restartBootstrap(ctx context.Context, vmid int) erro
return err
}
// controllerHealthy reports running + (healthy or no healthcheck) + the running image == want.
// Returns (ok, stillStarting): stillStarting=true means keep polling.
func (c *ControllerSwapper) controllerHealthy(ctx context.Context, vmid int, want string) (ok, starting bool) {
// controllerHealthy is a pure point-in-time predicate: running + RestartCount==0 + (healthy OR no
// healthcheck) + the running image == want. Returns (ok, stillStarting, needsDwell):
// - stillStarting=true → keep polling (container absent, not-yet-running, wrong image, unhealthy, or
// already-restarted).
// - needsDwell=true → ok BUT the image has NO healthcheck, so the caller must confirm it stays ok for
// verifyDwell consecutive polls before trusting it (F1: a no-healthcheck crash-loop can flicker
// Running for one instant). A real `healthy` result is trusted immediately (Docker already gated it).
//
// RestartCount>0 means the process has already crashed+restarted → not stably up, regardless of
// healthcheck presence (the F1 hole: alpine flickered Running between restarts and passed).
func (c *ControllerSwapper) controllerHealthy(ctx context.Context, vmid int, want string) (ok, starting, needsDwell bool) {
out, err := c.exec.GuestExec(ctx, vmid, "docker", "inspect", "-f",
"{{.State.Running}}|{{if .State.Health}}{{.State.Health.Status}}{{else}}none{{end}}|{{.Config.Image}}", controllerContainer)
"{{.State.Running}}|{{if .State.Health}}{{.State.Health.Status}}{{else}}none{{end}}|{{.Config.Image}}|{{.RestartCount}}", controllerContainer)
if err != nil {
return false, true // container not there yet (rm -f window) → keep polling
return false, true, false // container not there yet (rm -f window) → keep polling
}
f := strings.SplitN(strings.TrimSpace(out), "|", 3)
if len(f) != 3 {
return false, true
f := strings.SplitN(strings.TrimSpace(out), "|", 4)
if len(f) != 4 {
return false, true, false
}
running, health, image := f[0] == "true", f[1], f[2]
running, health, image, restartStr := f[0] == "true", f[1], f[2], f[3]
if !running {
return false, true
return false, true, false
}
if image != want {
return false, true // bootstrap may not have re-run yet
return false, true, false // bootstrap may not have re-run yet
}
if rc, err := strconv.Atoi(strings.TrimSpace(restartStr)); err == nil && rc > 0 {
return false, true, false // already crash-restarted → not stably up
}
switch health {
case "healthy", "none":
return true, false
case "healthy":
return true, false, false // Docker gated it → trust immediately
case "none":
return true, false, true // no healthcheck → ok, but require the dwell
case "starting":
return false, true
return false, true, false
default: // unhealthy
return false, true
return false, true, false
}
}
func (c *ControllerSwapper) verify(ctx context.Context, vmid int, want string) bool {
deadline := time.Now().Add(c.verifyTimeout)
consecutiveOK := 0
dwell := c.verifyDwell
if dwell < 1 {
dwell = 1
}
for {
if ok, _ := c.controllerHealthy(ctx, vmid, want); ok {
return true
ok, _, needsDwell := c.controllerHealthy(ctx, vmid, want)
switch {
case ok && !needsDwell:
return true // real healthcheck passed → done
case ok: // no-healthcheck image: require verifyDwell consecutive ok polls
consecutiveOK++
if consecutiveOK >= dwell {
return true
}
default:
consecutiveOK = 0 // any not-ok resets the dwell (a crash between polls)
}
if time.Now().After(deadline) || ctx.Err() != nil {
return false
+93 -1
View File
@@ -26,6 +26,11 @@ type fakeGuestExec struct {
teeStdin []string // raw bytes piped into each `tee` write (the swap's write vector)
failRestart bool
noHealthBlock bool // if set, .State.Health is absent ("none")
restartCount int // F1: .RestartCount reported by docker inspect (a crash-looper has >0)
// runningSeq, when non-nil, drives .State.Running per docker-inspect poll (last value repeats) —
// e.g. {true, false} models a container that flickers Running then crashes. nil → always running.
runningSeq []bool
inspectN int
}
func (f *fakeGuestExec) GuestExec(_ context.Context, _ int, args ...string) (string, error) {
@@ -50,13 +55,22 @@ func (f *fakeGuestExec) GuestExec(_ context.Context, _ int, args ...string) (str
if f.containerImg == "" {
return "", fmt.Errorf("no such container")
}
running := true
if f.runningSeq != nil {
i := f.inspectN
if i >= len(f.runningSeq) {
i = len(f.runningSeq) - 1
}
running = f.runningSeq[i]
f.inspectN++
}
health := "healthy"
if f.noHealthBlock {
health = "none"
} else if !f.good[f.containerImg] {
health = "unhealthy"
}
return fmt.Sprintf("true|%s|%s", health, f.containerImg), nil
return fmt.Sprintf("%t|%s|%s|%d", running, health, f.containerImg, f.restartCount), nil
}
return "", fmt.Errorf("fake: unexpected exec %v", args)
}
@@ -267,3 +281,81 @@ func TestControllerSwapHandler_SingleFlight(t *testing.T) {
t.Fatalf("status = %d, want 409 (swap already in progress)", rr.Code)
}
}
// inspectScript is a minimal GuestExecutor for verify()-level F1 tests: it returns scripted
// `docker inspect` outputs (one per poll; the last repeats) and no-ops everything else.
type inspectScript struct {
outs []string
i int
}
func (s *inspectScript) GuestExec(_ context.Context, _ int, args ...string) (string, error) {
if len(args) >= 2 && args[0] == "docker" && args[1] == "inspect" {
j := s.i
if j >= len(s.outs) {
j = len(s.outs) - 1
}
s.i++
return s.outs[j], nil
}
return "", nil
}
func (s *inspectScript) GuestExecStdin(_ context.Context, _ int, _ io.Reader, _ ...string) (string, error) {
return "", nil
}
func fastSwapper(exec GuestExecutor) *ControllerSwapper {
s := NewControllerSwapper(exec, "", discardLogger())
s.verifyTimeout = 100 * time.Millisecond
s.verifyInterval = 4 * time.Millisecond
return s
}
// F1 RED-PROOF: a no-healthcheck image with RestartCount>0 (a crash-looper) must NOT verify ok →
// verify() returns false → the swap's existing rollback path runs. Companion: the same image with
// RestartCount=0 and dwell=1 DOES verify — proving the RestartCount check is what blocks the crasher
// (the old `running + none → ok` shape had no such guard, the F1 hole found live).
func TestF1_Verify_CrashLoopRestartCountBlocks(t *testing.T) {
s := fastSwapper(&inspectScript{outs: []string{"true|none|" + newImg + "|2"}})
if s.verify(context.Background(), 9201, newImg) {
t.Fatal("RestartCount>0 (crash-looping no-healthcheck) must NOT verify ok")
}
// controllerHealthy is a pure predicate: rc>0 → (false, starting).
ok, starting, _ := s.controllerHealthy(context.Background(), 9201, newImg)
if ok || !starting {
t.Fatalf("controllerHealthy(rc=2) = (ok=%v,starting=%v), want (false,true)", ok, starting)
}
// Companion: rc=0 + dwell=1 → verifies (shows the rc check, not something else, blocks the crasher).
s2 := fastSwapper(&inspectScript{outs: []string{"true|none|" + newImg + "|0"}})
s2.verifyDwell = 1
if !s2.verify(context.Background(), 9201, newImg) {
t.Fatal("companion: no-healthcheck rc=0 dwell=1 should verify")
}
}
// F1 dwell: a single ok poll then a crash (Running=false) must NOT verify with the dwell. Companion:
// dwell=1 accepts the single ok (the false-positive the dwell removes).
func TestF1_Verify_DwellSingleOkThenCrash(t *testing.T) {
seq := []string{"true|none|" + newImg + "|0", "false|none|" + newImg + "|0"}
s := fastSwapper(&inspectScript{outs: seq}) // verifyDwell=3 (default)
if s.verify(context.Background(), 9201, newImg) {
t.Fatal("a single ok then crash must NOT verify with the dwell")
}
s2 := fastSwapper(&inspectScript{outs: seq})
s2.verifyDwell = 1
if !s2.verify(context.Background(), 9201, newImg) {
t.Fatal("companion: dwell=1 accepts the single ok")
}
}
// A real healthy image verifies promptly (no dwell) — the real controller path is unaffected.
func TestF1_Verify_RealHealthcheckPassesPromptly(t *testing.T) {
s := fastSwapper(&inspectScript{outs: []string{"true|healthy|" + newImg + "|0"}})
if !s.verify(context.Background(), 9201, newImg) {
t.Fatal("a real healthy image must verify promptly")
}
ok, _, needsDwell := s.controllerHealthy(context.Background(), 9201, newImg)
if !ok || needsDwell {
t.Fatalf("controllerHealthy(healthy) = (ok=%v,needsDwell=%v), want (true,false)", ok, needsDwell)
}
}
+21 -11
View File
@@ -51,7 +51,11 @@ func (f *fakeDiskOps) Unmount(_ context.Context, where string) error {
f.mu.Unlock()
return nil
}
func (f *fakeDiskOps) formatted() []string { f.mu.Lock(); defer f.mu.Unlock(); return append([]string(nil), f.formatCalls...) }
func (f *fakeDiskOps) formatted() []string {
f.mu.Lock()
defer f.mu.Unlock()
return append([]string(nil), f.formatCalls...)
}
type fakeGate struct {
mu sync.Mutex
@@ -80,11 +84,11 @@ func (f fakeGuestList) ListLXC(context.Context) ([]proxmox.Guest, error) { retur
// a different whole-disk (e.g. /dev/sdb1) classifies as user-data without touching the real host.
type fakeHostReader struct{ mounts []storage.Mount }
func (f fakeHostReader) Mounts() ([]storage.Mount, error) { return f.mounts, nil }
func (f fakeHostReader) ResolveUUID(string) (string, bool) { return "", false }
func (f fakeHostReader) DeviceExists(string) bool { return true }
func (f fakeHostReader) Rotational(string) (bool, bool) { return false, false }
func (f fakeHostReader) Removable(string) (bool, bool) { return false, false }
func (f fakeHostReader) Mounts() ([]storage.Mount, error) { return f.mounts, nil }
func (f fakeHostReader) ResolveUUID(string) (string, bool) { return "", false }
func (f fakeHostReader) DeviceExists(string) bool { return true }
func (f fakeHostReader) Rotational(string) (bool, bool) { return false, false }
func (f fakeHostReader) Removable(string) (bool, bool) { return false, false }
// sysOnSDA is the default system-disk fixture (root on /dev/sda) used by the disk-server test helpers.
func sysOnSDA() fakeHostReader {
@@ -345,7 +349,7 @@ func TestEject_RoleGated(t *testing.T) {
sv := fakeStorage{targets: []hub.StorageTarget{
{Name: "bulk", Type: hub.StorageTypeUSB, BackingDevice: "/dev/sdb1", MountPath: "/mnt/bulk"}, // user-data
{Name: "local", Type: "local", MountPath: "/var/lib/vz"}, // system (builtin dir)
{Name: "felhom-pbs", Type: hub.StorageTypePBS, MountPath: "/mnt/pbs"}, // backup (PBS)
{Name: "felhom-pbs", Type: hub.StorageTypePBS, MountPath: "/mnt/pbs"}, // backup (PBS)
}}
// system mount → refused, no Unmount.
@@ -438,7 +442,7 @@ func (f *fakeGuestAttacher) AttachDrive(_ context.Context, _ int, where string)
return StablePathForRaw(where), nil
}
func (f *fakeGuestAttacher) GuestSeesMount(_ context.Context, _ int, _ string) bool { return true }
func (f *fakeGuestAttacher) GuestBootID(_ context.Context, _ int) string { return "boot-1" }
func (f *fakeGuestAttacher) GuestBootID(_ context.Context, _ int) string { return "boot-1" }
func (f *fakeGuestAttacher) attachDriveCount() int {
f.mu.Lock()
defer f.mu.Unlock()
@@ -480,7 +484,11 @@ func (f *fakeGuestAttacher) RebootGuest(_ context.Context, vmid int) error {
f.reboots = append(f.reboots, vmid)
return nil
}
func (f *fakeGuestAttacher) rebootCount() int { f.mu.Lock(); defer f.mu.Unlock(); return len(f.reboots) }
func (f *fakeGuestAttacher) rebootCount() int {
f.mu.Lock()
defer f.mu.Unlock()
return len(f.reboots)
}
func newAttachServer(t *testing.T, ga GuestAttacher, mounts map[int]map[string]string) http.Handler {
t.Helper()
@@ -646,8 +654,10 @@ func (f *fakeGuestsCfg) GuestConfig(_ context.Context, vmid int) (proxmox.GuestC
}
return proxmox.GuestConfig{Extra: extra}, nil
}
func (f *fakeGuestsCfg) Snapshot(context.Context, int, string, string) (string, error) { return "", nil }
func (f *fakeGuestsCfg) Rollback(context.Context, int, string) (string, error) { return "", nil }
func (f *fakeGuestsCfg) Snapshot(context.Context, int, string, string) (string, error) {
return "", nil
}
func (f *fakeGuestsCfg) Rollback(context.Context, int, string) (string, error) { return "", nil }
func (f *fakeGuestsCfg) WaitTask(context.Context, string, proxmox.WaitOptions) (proxmox.TaskStatus, error) {
return proxmox.TaskStatus{ExitStatus: "OK"}, nil
}
+18 -18
View File
@@ -207,24 +207,24 @@ func NewServer(o Options) (*Server, error) {
cadence = defaultBackupCadence
}
s := &Server{
addr: o.ListenAddr,
cert: o.Cert,
guests: o.Guests,
backups: o.Backups,
store: o.Store,
storage: o.Storage,
tokens: o.Tokens,
cadence: cadence,
logger: o.Logger,
now: func() time.Time { return time.Now().UTC() },
disks: o.Disks,
diskGate: o.DiskGate,
guestList: o.Guests2,
guestAttach: o.GuestAttach,
intent: o.Intent,
guestBinds: o.GuestBinds,
formatJobs: o.FormatJobs,
host: o.HostReader,
addr: o.ListenAddr,
cert: o.Cert,
guests: o.Guests,
backups: o.Backups,
store: o.Store,
storage: o.Storage,
tokens: o.Tokens,
cadence: cadence,
logger: o.Logger,
now: func() time.Time { return time.Now().UTC() },
disks: o.Disks,
diskGate: o.DiskGate,
guestList: o.Guests2,
guestAttach: o.GuestAttach,
intent: o.Intent,
guestBinds: o.GuestBinds,
formatJobs: o.FormatJobs,
host: o.HostReader,
hostMetrics: o.HostMetrics,
hostID: o.HostID,
jobs: map[int]*backupJob{},
+10 -2
View File
@@ -89,7 +89,11 @@ func (f *fakeBackups) BackupWithSnapshotHook(_ context.Context, vmid int, onSnap
}
return hub.Backup{VMID: vmid, Success: true, Archive: "local:backup/vzdump-x", StartedAt: "2026-06-10T00:00:00Z"}, nil
}
func (f *fakeBackups) called() []int { f.mu.Lock(); defer f.mu.Unlock(); return append([]int(nil), f.vmids...) }
func (f *fakeBackups) called() []int {
f.mu.Lock()
defer f.mu.Unlock()
return append([]int(nil), f.vmids...)
}
type fakeStore struct {
mu sync.Mutex
@@ -97,7 +101,11 @@ type fakeStore struct {
tests []hub.RestoreTest
}
func (s *fakeStore) RecordBackup(b hub.Backup) { s.mu.Lock(); s.backups = append(s.backups, b); s.mu.Unlock() }
func (s *fakeStore) RecordBackup(b hub.Backup) {
s.mu.Lock()
s.backups = append(s.backups, b)
s.mu.Unlock()
}
func (s *fakeStore) Backups(context.Context) []hub.Backup {
s.mu.Lock()
defer s.mu.Unlock()
+7 -7
View File
@@ -22,13 +22,13 @@ func TestAntiRetargetResolve(t *testing.T) {
okInspect := func(string) (storage.DeviceProbe, error) { return dataBearing, nil }
cases := []struct {
name string
durable string
resolve func(string) (string, error)
derive func(string) (string, error)
inspect func(string) (storage.DeviceProbe, error)
wantDev string
wantErr string // substring; "" = expect success
name string
durable string
resolve func(string) (string, error)
derive func(string) (string, error)
inspect func(string) (storage.DeviceProbe, error)
wantDev string
wantErr string // substring; "" = expect success
}{
{
name: "happy-path-matches",