fix(runner): mint restart-safe session IDs (RIG-3696)

go/internal/runner/config_delivery_e2e_test.go

@@ -289,7 +289,7 @@ func TestConfigDeliveryReloadPicksUpNewBundle(t *testing.T) {
 	t.Cleanup(func() { _ = exec.Command("podman", "rm", "--force", name).Run() })
 
 	cfg := AgentHostConfig{RuntimeDir: shortRuntimeDir(t)}
-	host := NewSessionHost(link, rt, registry, engine, e2eSpecBuilder{name: name}, cfg, log.logger(), monotonicIDs()).(*agentHost)
+	host := NewSessionHost(link, rt, registry, engine, e2eSpecBuilder{name: name}, cfg, log.logger(), randomIDs()).(*agentHost)
 	// Close the host last: it Closes the per-container socket listener that
 	// Provision opens, which neither Stop (agent stream only) nor the container
 	// force-remove reaches. Mirrors e2e_transport_test.go.

go/internal/runner/config_refresh_test.go

@@ -96,7 +96,7 @@ func newConfigRefreshFixture(t *testing.T) (*agentHost, *configFanoutRuntime, *c
 	pub := newCapturePublish()
 	link := newLink(newRunnerServiceServer(t, pub))
 	cfg := AgentHostConfig{RuntimeDir: shortRuntimeDir(t)}
-	host := NewSessionHost(link, rt, registry, engine, accountSpecBuilder{}, cfg, discardLoggerRunner(), monotonicIDs()).(*agentHost)
+	host := NewSessionHost(link, rt, registry, engine, accountSpecBuilder{}, cfg, discardLoggerRunner(), randomIDs()).(*agentHost)
 	return host, engine, pub
 }
 

go/internal/runner/host.go

@@ -8,10 +8,11 @@ package runner
 
 import (
 	"context"
+	"crypto/rand"
+	"encoding/hex"
 	"fmt"
 	"log/slog"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"sync"
 
@@ -134,13 +135,13 @@ type AgentHostConfig struct {
 // NewSessionHost builds the production SessionHost over the link, the agent
 // runtime + registry (so a launched container resolves by name), the container
 // engine, the spec builder Provision derives its AgentSpec from, and the host's
-// own config. newID mints session ids; nil uses a monotonic counter.
+// own config. newID mints session ids; nil uses a crypto-random allocator.
 func NewSessionHost(link *ServerLink, rt *runtime.AgentRuntime, registry *runtime.AgentRegistry, engine runtime.WorkloadRuntime, specs SpecBuilder, cfg AgentHostConfig, log *slog.Logger, newID func() string) SessionHost {
 	if log == nil {
 		log = slog.Default()
 	}
 	if newID == nil {
-		newID = monotonicIDs()
+		newID = randomIDs()
 	}
 	return &agentHost{
 		link:           link,
@@ -1049,15 +1050,15 @@ func (h *agentHost) closeSocket(ctx context.Context, containerName string) {
 	}
 }
 
-// monotonicIDs returns a session-id minter — a simple monotonic counter,
-// sufficient for the single-Runner MVP where ids are Runner-local.
-func monotonicIDs() func() string {
-	var mu sync.Mutex
-	var n uint64
+// randomIDs returns a session-id minter backed by the OS CSPRNG. The sess-
+// prefix preserves the operator-facing session shape; hex encoding keeps the
+// id safe as a path element and fabric subject while making separate Runner
+// lifetimes overwhelmingly unlikely to collide.
+func randomIDs() func() string {
 	return func() string {
-		mu.Lock()
-		defer mu.Unlock()
-		n++
-		return "sess-" + strconv.FormatUint(n, 10)
+		var b [16]byte
+		// crypto/rand.Read is infallible on supported platforms, matching store.newID.
+		_, _ = rand.Read(b[:])
+		return "sess-" + hex.EncodeToString(b[:])
 	}
 }

go/internal/runner/host_test.go

@@ -33,6 +33,20 @@ import (
 	"github.com/RigelBuild/compass/go/internal/runtime"
 )
 
+func TestRandomIDsAreIndependentPathSafeSessionIDs(t *testing.T) {
+	first := randomIDs()()
+	second := randomIDs()()
+	if first == second {
+		t.Fatalf("independent allocators minted duplicate session id %q", first)
+	}
+	if len(first) != 37 || filepath.Base(first) != first || strings.ContainsAny(first, `/\\.`) {
+		t.Fatalf("session id %q is not a safe path element", first)
+	}
+	if strings.ContainsAny(first, "*>") {
+		t.Fatalf("session id %q is not safe as a fabric subject token", first)
+	}
+}
+
 // fakeSpecBuilder is a hand-written SpecBuilder: it records the request it was
 // asked to build and returns a scripted spec (or error), so Provision's wiring
 // to Launch is asserted without deriving a real image/egress spec.

go/internal/runnerhub/binding_cache_test.go

@@ -529,39 +529,31 @@ func TestStoreFaultsFallBackWithoutLosingFailClosed(t *testing.T) {
 	})
 }
 
-// TestReusedSessionIDConflictIsSwallowed WITNESSES a known gap rather than a
-// guarantee: production mints session ids with a counter that resets on every
-// Runner restart, so a joint Server+Runner restart re-mints "sess-1" over a
-// surviving row. The store answers ErrConflict to keep the binding
-// single-valued; promoteSession currently treats it as a transient fault and
-// falls back to RAM, leaving the durable row pointing at the OLD account. The
-// single-instance MVP shadows that row and later retires it, but a second
-// Server instance would read the stale durable truth. Pinned so the behaviour
-// cannot change silently while the fix is decided (RIG-3108 review F1).
-func TestReusedSessionIDConflictIsSwallowed(t *testing.T) {
+// TestFreshSessionBindingUsesNewLogicalID asserts the approved restart invariant:
+// a fresh session uses a distinct logical id, so the durable binding can be
+// recorded for the new account without conflicting with an older session row.
+func TestFreshSessionBindingUsesNewLogicalID(t *testing.T) {
 	hub := newHubOnly()
 	bindings := newFakeBindingStore()
 	hub.SetSessionBindingStore(bindings)
 	hub.enroll(context.Background(), "runner-1", runnerSubject(), compassv1.RuntimeTier_RUNTIME_TIER_UNSPECIFIED, compassv1.EgressPosture_EGRESS_POSTURE_UNSPECIFIED)
 
-	// A row from before the joint restart, under a DIFFERENT account.
+	// A row from before the restart, under a DIFFERENT account.
 	bindings.mu.Lock()
-	bindings.bindings["sess-1"] = store.SessionBinding{SessionID: "sess-1", AccountID: "acct-stale", RunnerID: "runner-1"}
+	bindings.bindings["sess-old"] = store.SessionBinding{SessionID: "sess-old", AccountID: "acct-stale", RunnerID: "runner-1"}
 	bindings.mu.Unlock()
 
 	hub.bindContainer("cont-1", testAgentAccount)
-	hub.promoteSession(context.Background(), "cont-1", "sess-1")
+	hub.promoteSession(context.Background(), "cont-1", "sess-new")
 
-	// This instance resolves the NEW account from RAM.
-	if acct, ok := hub.accountForSession(context.Background(), "sess-1"); !ok || acct != testAgentAccount {
-		t.Fatalf("accountForSession(sess-1) = (%q, %v), want (%s, true)", acct, ok, testAgentAccount)
+	if acct, ok := hub.accountForSession(context.Background(), "sess-new"); !ok || acct != testAgentAccount {
+		t.Fatalf("accountForSession(sess-new) = (%q, %v), want (%s, true)", acct, ok, testAgentAccount)
 	}
-	// ...but the durable row still names the stale account: the divergence.
 	bindings.mu.Lock()
-	got := bindings.bindings["sess-1"].AccountID
+	got := bindings.bindings["sess-new"].AccountID
 	bindings.mu.Unlock()
-	if got != "acct-stale" {
-		t.Fatalf("durable binding for sess-1 = %q, want %q — if this now agrees with the cache, the ErrConflict gap was fixed and this witness test should become a real assertion", got, "acct-stale")
+	if got != testAgentAccount {
+		t.Fatalf("durable binding for sess-new = %q, want %q", got, testAgentAccount)
 	}
 }