fix(doctor): warn when the hostagent's binary was replaced under it (#17)

#14 made the binary-replacement check work when the hostagent runs a different
path than the current klimax. It still could not see the case that actually
causes the failure: the same path, replaced underneath a running process.

`make dev-install` does `sudo cp` over /usr/local/bin/klimax, overwriting the
file the hostagent is executing — which is what makes macOS amfid kill
subsequent klimax execs. Both sides of a path comparison still read
"/usr/local/bin/klimax", so doctor reported OK while the machine was in exactly
the broken state the check exists to catch.

Compare timestamps instead: if the binary's mtime is later than the process
start time, it was replaced after launch. Process start comes from the kernel
proc table via sysctl — there is no /proc on macOS, and `ps` only offers a
locale-formatted date.

A Homebrew upgrade does NOT trigger amfid, because it writes a new file into the
Caskroom and re-points the symlink, so the running process keeps its own inode.
It does still trip this check — correctly: the hostagent is running an older
klimax and the VM wants rebuilding either way. Hence a warning, never a failure;
mtime is a heuristic and a rebuild producing identical content would also trip
it.

processStartTime is darwin-only and split across build tags, since internal/cli
builds for linux today and must keep doing so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01298bsrxR44xPnT3qM9fu2D

Author: bcollard

go.mod

@@ -8,6 +8,7 @@ require (
 	github.com/sirupsen/logrus v1.10.2
 	github.com/spf13/cobra v1.10.2
 	golang.org/x/crypto v0.56.0
+	golang.org/x/sys v0.47.0
 	golang.org/x/term v0.45.0
 	gopkg.in/yaml.v3 v3.0.1
 )
@@ -82,7 +83,6 @@ require (
 	golang.org/x/mod v0.38.0 // indirect
 	golang.org/x/net v0.57.0 // indirect
 	golang.org/x/sync v0.22.0 // indirect
-	golang.org/x/sys v0.47.0 // indirect
 	golang.org/x/text v0.41.0 // indirect
 	golang.org/x/time v0.9.0 // indirect
 	golang.org/x/tools v0.48.0 // indirect

internal/cli/doctor.go

@@ -12,6 +12,7 @@ import (
 	"strconv"
 	"strings"
 	"syscall"
+	"time"
 
 	"github.com/bcollard/klimax/internal/config"
 	"github.com/bcollard/klimax/internal/guest"
@@ -452,6 +453,16 @@ func checkHostagent(instanceName string) *doctorCheck {
 
 	self, _ := os.Executable()
 	if sameFile(exe, self) {
+		// Same path — but the file at that path may have been replaced since the
+		// hostagent started, in which case it is running code that no longer
+		// exists on disk. Path comparison cannot see this; timestamps can.
+		if replaced, mtime, start := binaryReplacedSinceLaunch(exe, pid); replaced {
+			return &doctorCheck{ID: checkIDHostagent, Status: checkWarn,
+				Message: fmt.Sprintf("hostagent (pid %d) is running a binary that has since been replaced", pid),
+				Detail: fmt.Sprintf("%s was modified at %s, after the hostagent started at %s.\n  It is still running the old code; klimax execs may fail with 'killed'.",
+					exe, mtime.Format(time.RFC3339), start.Format(time.RFC3339)),
+				Fix: "klimax down && klimax up  (or: " + cleanup + ")"}
+		}
 		return &doctorCheck{ID: checkIDHostagent, Status: checkOK,
 			Message: fmt.Sprintf("hostagent is running (pid %d)", pid)}
 	}
@@ -507,3 +518,31 @@ func sameFile(a, b string) bool {
 	}
 	return ra == rb
 }
+
+// binaryReplacedSinceLaunch reports whether the file at path was modified after
+// the process started — i.e. the running process is executing code that path no
+// longer contains.
+//
+// This is the case plain path comparison cannot see. `make dev-install` does
+// `sudo cp` over /usr/local/bin/klimax, overwriting the file underneath the
+// running hostagent, which is what makes macOS amfid kill subsequent execs. Both
+// sides of a path comparison still read the same string, so only the timestamps
+// give it away.
+//
+// A Homebrew upgrade is different — it writes a new file into the Caskroom and
+// re-points the symlink, so the running process keeps its own inode and amfid is
+// not upset. That still trips this check, and rightly so: the hostagent is
+// running an older klimax and the VM wants rebuilding either way. Hence a
+// warning, never a failure — and mtime is a heuristic, so a rebuild producing
+// identical content would also trip it.
+func binaryReplacedSinceLaunch(path string, pid int) (replaced bool, mtime, start time.Time) {
+	start, err := processStartTime(pid)
+	if err != nil {
+		return false, time.Time{}, time.Time{} // cannot tell — stay quiet
+	}
+	fi, err := os.Stat(path)
+	if err != nil {
+		return false, time.Time{}, time.Time{}
+	}
+	return fi.ModTime().After(start), fi.ModTime(), start
+}

internal/cli/doctor_test.go

@@ -3,6 +3,7 @@ package cli
 import (
 	"os"
 	"path/filepath"
+	"time"
 	"testing"
 )
 
@@ -164,3 +165,57 @@ func TestSameFileFollowsSymlinks(t *testing.T) {
 		t.Error("two nonexistent paths must not compare equal")
 	}
 }
+
+func TestProcessStartTimeIsSane(t *testing.T) {
+	start, err := processStartTime(os.Getpid())
+	if err != nil {
+		t.Skipf("process start time unavailable on this platform: %v", err)
+	}
+	if start.IsZero() || start.After(time.Now()) {
+		t.Errorf("implausible start time %v", start)
+	}
+	// The test binary cannot have been running for a year.
+	if time.Since(start) > 365*24*time.Hour {
+		t.Errorf("start time %v is implausibly old", start)
+	}
+}
+
+// The case plain path comparison cannot see: same path, but the file behind it
+// changed after the process launched.
+func TestBinaryReplacedSinceLaunch(t *testing.T) {
+	if _, err := processStartTime(os.Getpid()); err != nil {
+		t.Skipf("process start time unavailable on this platform: %v", err)
+	}
+	dir := t.TempDir()
+
+	// Written now, so newer than this process — the "replaced" case.
+	fresh := filepath.Join(dir, "fresh")
+	if err := os.WriteFile(fresh, []byte("new"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if replaced, _, _ := binaryReplacedSinceLaunch(fresh, os.Getpid()); !replaced {
+		t.Error("a file written after the process started must count as replaced")
+	}
+
+	// Backdated well before the process started — the normal case.
+	old := filepath.Join(dir, "old")
+	if err := os.WriteFile(old, []byte("old"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	past := time.Now().Add(-48 * time.Hour)
+	if err := os.Chtimes(old, past, past); err != nil {
+		t.Fatal(err)
+	}
+	if replaced, _, _ := binaryReplacedSinceLaunch(old, os.Getpid()); replaced {
+		t.Error("a file older than the process must not count as replaced")
+	}
+
+	// Unreadable path must not produce a false warning.
+	if replaced, _, _ := binaryReplacedSinceLaunch(filepath.Join(dir, "nope"), os.Getpid()); replaced {
+		t.Error("a missing file must not count as replaced")
+	}
+	// An impossible pid means we cannot tell — must stay quiet.
+	if replaced, _, _ := binaryReplacedSinceLaunch(fresh, 0x7FFFFFFE); replaced {
+		t.Error("an unknown process must not produce a warning")
+	}
+}

internal/cli/proctime_darwin.go

@@ -0,0 +1,21 @@
+//go:build darwin
+
+package cli
+
+import (
+	"time"
+
+	"golang.org/x/sys/unix"
+)
+
+// processStartTime returns when pid was launched, read from the kernel's proc
+// table. There is no /proc on macOS, and `ps` only reports a locale-formatted
+// date, so go straight to the sysctl.
+func processStartTime(pid int) (time.Time, error) {
+	kp, err := unix.SysctlKinfoProc("kern.proc.pid", pid)
+	if err != nil {
+		return time.Time{}, err
+	}
+	tv := kp.Proc.P_starttime
+	return time.Unix(tv.Sec, int64(tv.Usec)*1000), nil
+}

internal/cli/proctime_other.go

@@ -0,0 +1,15 @@
+//go:build !darwin
+
+package cli
+
+import (
+	"errors"
+	"time"
+)
+
+// processStartTime is implemented for darwin only. klimax targets macOS, and the
+// staleness check that uses it exists for a macOS failure mode; elsewhere the
+// caller treats the error as "cannot tell" and skips the check.
+func processStartTime(int) (time.Time, error) {
+	return time.Time{}, errors.New("process start time is only available on darwin")
+}