fix(cli): serialize self-update per target with an OS-level advisory lock

Related #957

Two concurrent 'mcpproxy update' runs could read each other's mid-swap
sentinel state, misclassify it, and delete the only known-good backup.
The whole recover-and-swap sequence now runs under an exclusive
non-blocking flock (LockFileEx on Windows) on <target>.update-lock; the
second invocation fails fast with 'another update is already in
progress'. The lock file is never unlinked (unlink+relock races two
holders onto different inodes); the kernel drops the lock with the
process, so a crash cannot wedge future updates.

Author: Dumbris

cmd/mcpproxy/update_apply.go

@@ -228,6 +228,16 @@ func applyNewBinary(target, staged string, verify func(path string) error) (err
 	backup := target + ".old"
 	sentinel := target + swapSentinelSuffix
 
+	// The whole recover-and-swap sequence runs under an exclusive per-target
+	// lock: a concurrent invocation reading a sibling's sentinel/backup state
+	// mid-swap would misclassify it and could delete the only known-good
+	// backup. Crash-safe (the kernel releases it with the process).
+	release, lockErr := acquireUpdateLock(target)
+	if lockErr != nil {
+		return lockErr
+	}
+	defer release()
+
 	if recoverErr := recoverInterruptedSwap(target, backup, sentinel, verify); recoverErr != nil {
 		return recoverErr
 	}

cmd/mcpproxy/update_apply_test.go

@@ -611,3 +611,54 @@ func TestReportsVersion(t *testing.T) {
 		}
 	}
 }
+
+func TestAcquireUpdateLock_SecondHolderIsRefused(t *testing.T) {
+	target := filepath.Join(t.TempDir(), "mcpproxy")
+
+	release, err := acquireUpdateLock(target)
+	if err != nil {
+		t.Fatalf("first acquire: %v", err)
+	}
+	// flock/LockFileEx conflict even between two opens in the same process,
+	// so this stands in for a concurrent `mcpproxy update` invocation.
+	if _, err := acquireUpdateLock(target); !errors.Is(err, errUpdateInProgress) {
+		t.Fatalf("second acquire: want errUpdateInProgress, got %v", err)
+	}
+
+	release()
+	release2, err := acquireUpdateLock(target)
+	if err != nil {
+		t.Fatalf("re-acquire after release: %v", err)
+	}
+	release2()
+}
+
+func TestApplyNewBinary_RefusedWhileAnotherSwapHoldsTheLock(t *testing.T) {
+	dir := t.TempDir()
+	target := filepath.Join(dir, "mcpproxy")
+	staged := filepath.Join(dir, "staged")
+	if err := os.WriteFile(target, []byte("current"), 0o755); err != nil {
+		t.Fatalf("write target: %v", err)
+	}
+	if err := os.WriteFile(staged, []byte("new"), 0o600); err != nil {
+		t.Fatalf("write staged: %v", err)
+	}
+
+	release, err := acquireUpdateLock(target)
+	if err != nil {
+		t.Fatalf("acquire: %v", err)
+	}
+	defer release()
+
+	if err := applyNewBinary(target, staged, nil); !errors.Is(err, errUpdateInProgress) {
+		t.Fatalf("applyNewBinary under a held lock: want errUpdateInProgress, got %v", err)
+	}
+	// The refused attempt must not have touched anything.
+	got, readErr := os.ReadFile(target)
+	if readErr != nil {
+		t.Fatalf("read target: %v", readErr)
+	}
+	if string(got) != "current" {
+		t.Fatalf("target changed by refused swap: %q", got)
+	}
+}

cmd/mcpproxy/update_lock.go

@@ -0,0 +1,40 @@
+package main
+
+import (
+	"errors"
+	"fmt"
+	"os"
+)
+
+// errUpdateInProgress reports that another mcpproxy process holds the
+// self-update lock for the same target binary.
+var errUpdateInProgress = errors.New("another mcpproxy update is already updating this binary")
+
+// updateLockSuffix names the advisory lock file that serializes the whole
+// recover-and-swap sequence per target. The file is deliberately NEVER
+// unlinked: removing a held lock file lets a third process create-and-lock a
+// fresh inode while a second still holds the old one, which is two "exclusive"
+// holders. A leftover zero-byte <target>.update-lock is the documented cost.
+const updateLockSuffix = ".update-lock"
+
+// acquireUpdateLock takes an exclusive, non-blocking, OS-level advisory lock
+// scoped to the target binary. It guards recoverInterruptedSwap AND the swap
+// itself: without it, a concurrent `mcpproxy update` can read a half-finished
+// sibling's sentinel/backup state, misclassify it, and delete the only
+// known-good backup. The kernel releases the lock if the process dies, so a
+// crash can never wedge future updates.
+func acquireUpdateLock(target string) (release func(), err error) {
+	path := target + updateLockSuffix
+	f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) // #nosec G304 -- derived from the caller-resolved target path
+	if err != nil {
+		return nil, fmt.Errorf("open update lock %s: %w", path, err)
+	}
+	if lockErr := lockFileExclusiveNB(f); lockErr != nil {
+		_ = f.Close()
+		if errors.Is(lockErr, errWouldBlock) {
+			return nil, fmt.Errorf("%w (lock: %s)", errUpdateInProgress, path)
+		}
+		return nil, fmt.Errorf("lock %s: %w", path, lockErr)
+	}
+	return func() { _ = f.Close() }, nil // closing the fd releases the lock
+}

cmd/mcpproxy/update_lock_unix.go

@@ -0,0 +1,25 @@
+//go:build !windows
+
+package main
+
+import (
+	"errors"
+	"os"
+
+	"golang.org/x/sys/unix"
+)
+
+// errWouldBlock is what lockFileExclusiveNB returns when another process
+// already holds the lock.
+var errWouldBlock = unix.EWOULDBLOCK
+
+// lockFileExclusiveNB takes a non-blocking exclusive flock on f. flock locks
+// belong to the open file description, so they conflict even between two
+// opens in the same process, and the kernel drops them when the process exits.
+func lockFileExclusiveNB(f *os.File) error {
+	err := unix.Flock(int(f.Fd()), unix.LOCK_EX|unix.LOCK_NB)
+	if errors.Is(err, unix.EAGAIN) {
+		return errWouldBlock
+	}
+	return err
+}

cmd/mcpproxy/update_lock_windows.go

@@ -0,0 +1,28 @@
+//go:build windows
+
+package main
+
+import (
+	"errors"
+	"os"
+
+	"golang.org/x/sys/windows"
+)
+
+// errWouldBlock is what lockFileExclusiveNB returns when another process
+// already holds the lock.
+var errWouldBlock = errors.New("update lock held by another process")
+
+// lockFileExclusiveNB takes a non-blocking exclusive LockFileEx on the first
+// byte of f. Windows releases the region lock when the handle is closed or
+// the process exits.
+func lockFileExclusiveNB(f *os.File) error {
+	ol := new(windows.Overlapped)
+	err := windows.LockFileEx(windows.Handle(f.Fd()),
+		windows.LOCKFILE_EXCLUSIVE_LOCK|windows.LOCKFILE_FAIL_IMMEDIATELY,
+		0, 1, 0, ol)
+	if errors.Is(err, windows.ERROR_LOCK_VIOLATION) {
+		return errWouldBlock
+	}
+	return err
+}