From ba3518cb10ef3bba7d1cc1f440e9d2a2981a789b Mon Sep 17 00:00:00 2001
From: Matt Wilkinson <matt@sealedsecurity.com>
Date: Mon, 27 Jul 2026 20:45:06 -0400
Subject: [PATCH 1/5] fix(runner): bound the agent socket path under the
 AF_UNIX sun_path cap
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The agent socket path is RuntimeDir + /containers/<container>/agent.sock
(host.go:291), where <container> is NamePrefix + the server-minted 32-hex
agent account id. That tail is 69 bytes, so the default /run/compass lands
at 81 of the 107 usable bytes and the operator's --runtime-dir budget is
38 on Linux, 34 on darwin/BSD. Nothing bounded it on any hop: --runtime-dir
flows from main.go through run.go to filepath.Join to lc.Listen unchecked.

Over the cap, bind(2) returns a bare EINVAL naming neither the limit nor
the length. Worse, by then MkdirAll has already run and the 0700 directory
survives: nothing in the socket's lifecycle removes it, since Close removes
only the socket file and never its parent. Checking the length before any
directory is created makes that leak moot rather than adding a teardown.

The cap is derived from the platform's own sockaddr_un rather than
hard-coded. The array is not one size across the platforms //go:build unix
admits — 108 bytes on linux, solaris and illumos, 104 on darwin and the
BSDs, 1023 on aix — so no pair of hand-written numbers is correct, and an
over-sized constant would admit exactly the path the kernel rejects at
bind. The expression matches the bound Go's own wrapper enforces:
syscall_linux.go:559 rejects n == len(Path) for a non-abstract name and
syscall_bsd.go:191 rejects n >= len(Path), both landing on len-1.

The same cap was silently breaking the runnerhub integration test, which
passed t.TempDir() as the RuntimeDir. t.TempDir() derives its path from the
test name, and the 38-byte budget implies a 19-character ceiling that every
one of the package's tests exceeds. Only this test failed, because only
this one opens an agent socket — so the latent trap was the next test to
wire a SessionHost, not a future long name. It now uses a fixed short root
and asserts the resulting longest path against the cap.

Verified against a live Postgres: the integration test hangs to the 600s
deadline before the fix and passes in 11.5s after. Two mutations verified
red — dropping the pre-check lets the path reach bind and leaks the
directory; tightening the bound to >= refuses an at-cap path the kernel
accepts.

Refs SEA-1440, SEA-1443

Co-Authored-By: seal <noreply@sealedsecurity.com>
---
 go/internal/runner/gateway/socket.go          |  56 ++++
 go/internal/runner/gateway/socket_test.go     | 105 ++++++
 .../runnerhub/integration_pgtest_test.go      | 310 +++++++++++++-----
 3 files changed, 384 insertions(+), 87 deletions(-)

diff --git a/go/internal/runner/gateway/socket.go b/go/internal/runner/gateway/socket.go
index 193ff785..7a2e0b29 100644
--- a/go/internal/runner/gateway/socket.go
+++ b/go/internal/runner/gateway/socket.go
@@ -46,6 +46,27 @@ const socketDirMode os.FileMode = 0o700
 // teardown (design SocketListener.Close).
 const shutdownGrace = 5 * time.Second
 
+// sunPathMax is the longest AF_UNIX path the kernel accepts: sockaddr_un's
+// sun_path holds the path plus a NUL terminator. Exceeded, bind(2) returns a
+// bare EINVAL naming neither the limit nor the length, so the path is checked
+// here instead (see listenAgentSocket).
+//
+// Derived from the platform's own sockaddr_un rather than hard-coded, because
+// the array is not one size across the platforms //go:build unix admits, and
+// the spread is wider than the two sizes anyone reaches for: measured over all
+// nine, it is 108 bytes on linux, solaris and illumos, 104 on darwin and the
+// BSDs including dragonfly, and 1023 on aix. No pair of hand-written numbers is
+// correct here, and an author who enumerates the ones they thought of ships the
+// bug on the rest — an under-sized constant merely refuses a path that would
+// have worked, but an over-sized one passes a path the kernel rejects at bind,
+// which is the exact failure this check exists to prevent.
+//
+// The expression is also not an approximation of the kernel's rule but exactly
+// the bound Go's own wrapper enforces before the syscall: syscall_linux.go:559
+// rejects n == len(Path) for a non-abstract name, syscall_bsd.go:191 rejects
+// n >= len(Path). Two different predicates, both landing on len-1.
+const sunPathMax = len(syscall.RawSockaddrUnix{}.Path) - 1
+
 // runnerUID reports the uid a reclaimable stale socket must be owned by. It is a
 // package var over os.Getuid so a hermetic test can drive the wrong-owner
 // fail-closed branch (which cannot be forged on disk without root).
@@ -71,7 +92,42 @@ type SocketListener struct {
 // (regular file, dir, symlink, or a socket owned by another uid — a path
 // collision or partial op, never an abandoned Runner socket) is rejected fail-
 // closed and never deleted.
+//
+// Path length is checked FIRST, before any directory is created. Not only for
+// the diagnostic: a bind that fails EINVAL has already run MkdirAll, and the
+// dir survives — measured, 0700 left on disk. Nothing reclaims it on any path,
+// because nothing in this socket's lifecycle removes the directory at all:
+// Close removes the socket file (:210) and never filepath.Dir(path), so the
+// Launch-failure teardown Provision does run (host.go:120-125) leaks it too.
+// Refusing before the mkdir is what makes the question moot, rather than a
+// teardown that would have to be added.
 func listenAgentSocket(ctx context.Context, path string, h http.Handler) (*SocketListener, error) {
+	// Two inputs drive this length. The socket path is RuntimeDir +
+	// /containers/<container>/agent.sock (host.go:291), where <container> is
+	// NamePrefix + the agent account id (spec.go:73).
+	//
+	// Only one of them is a real variable. The account id is server-minted at
+	// exactly 32 hex chars (store/ids.go:21-29), Provision is admin-only
+	// (auth/admin_gate.go:50-56), and the id is FK-constrained to an existing
+	// account (0003_agent_ownership.sql:26) — so a longer id reaches this check
+	// but can never name an account that exists. RuntimeDir is operator-supplied
+	// (--runtime-dir) with no bound on any hop, and it inflates the path for
+	// every agent at once rather than for one doomed request.
+	//
+	// With the minted id the tail is 69 bytes, so the default /run/compass lands
+	// at 81 and the --runtime-dir budget is sunPathMax-69: 38 on Linux, 34 on
+	// darwin/BSD. That is the figure a startup precheck should assert, so an
+	// operator learns it at boot rather than at first provision (SEA-1443).
+	//
+	// The bind's own error is "bind: invalid argument", an EINVAL naming neither
+	// the limit nor the actual length, which reads as a permissions or path
+	// problem. Check first, before any directory is created, so a misconfigured
+	// deployment is self-diagnosing at Provision and leaves nothing behind. The
+	// message names both knobs: the path alone does not say which one to shrink.
+	if len(path) > sunPathMax {
+		return nil, fmt.Errorf("agent socket path %q is %d bytes, over the %d-byte AF_UNIX limit: shorten the Runner's --runtime-dir or the agent account id", path, len(path), sunPathMax)
+	}
+
 	dir := filepath.Dir(path)
 	if err := os.MkdirAll(dir, socketDirMode); err != nil {
 		return nil, fmt.Errorf("creating agent socket dir %q: %w", dir, err)
diff --git a/go/internal/runner/gateway/socket_test.go b/go/internal/runner/gateway/socket_test.go
index 2ce12ac7..32a6831d 100644
--- a/go/internal/runner/gateway/socket_test.go
+++ b/go/internal/runner/gateway/socket_test.go
@@ -19,10 +19,12 @@ package gateway
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net"
 	"net/http"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -361,6 +363,109 @@ func TestRejectsWrongOwnerSocketNeverDeletes(t *testing.T) {
 	}
 }
 
+// Case 7b. A path over the AF_UNIX sun_path limit is refused with an error that
+// names the limit and the actual length, and nothing is created on disk; a path
+// exactly at the limit still binds. The kernel enforces the cap at bind(2) with
+// a bare EINVAL ("invalid argument"), which names neither number and reads as a
+// permissions or path fault; the Runner's socket path is RuntimeDir plus a
+// 69-byte tail at the store-minted 32-hex account id — a width fixed at its
+// minting site, not validated at this hop — and RuntimeDir is operator-supplied
+// with no bound anywhere, so this is a reachable misconfiguration, not a
+// defensive check.
+//
+// Mutations, both RED: dropping the pre-check lets the over-long path reach bind,
+// failing the message assertions and the never-created-dir assertion; tightening
+// the bound to >= refuses the at-cap path the kernel accepts, failing the first
+// subtest.
+func TestRejectsPathOverSunPathLimit(t *testing.T) {
+	// padTo returns a path under a fresh temp root of exactly n bytes, or skips
+	// when the root alone already exceeds the budget (a very deep TMPDIR).
+	padTo := func(t *testing.T, n int) (dir, path string) {
+		t.Helper()
+		// A short fixed root, not t.TempDir(): the latter embeds the test name,
+		// which lands these subtests at ~135 bytes under a darwin-shaped TMPDIR
+		// (/var/folders/<2>/<hash>/T, ~49 bytes) and skips them — on the one
+		// platform where sunPathMax differs from Linux's, and so the one platform
+		// this test most needs to run. MkdirTemp("", "g") is bounded instead: the
+		// random suffix is a uint32 in decimal, so at most 10 digits, giving <=16
+		// bytes on Linux and <=61 under that darwin TMPDIR — both well under the
+		// cap, and a bound rather than a measurement that could drift.
+		root, err := os.MkdirTemp("", "g") //nolint:usetesting // t.TempDir embeds the test name and blows this test's own byte budget on darwin — see above
+		if err != nil {
+			t.Fatalf("MkdirTemp: %v", err)
+		}
+		t.Cleanup(func() {
+			if err := os.RemoveAll(root); err != nil {
+				t.Errorf("removing %q: %v", root, err)
+			}
+		})
+		dir = filepath.Join(root, "run")
+		base := len(dir) + 1 // + separator
+		// Backstop for a pathological TMPDIR; with the short root above this
+		// should not fire in practice on any supported platform.
+		if base >= n {
+			t.Skipf("temp root %q is %d bytes, leaving no room for a %d-byte path", dir, base, n)
+		}
+		path = filepath.Join(dir, strings.Repeat("s", n-base))
+		if len(path) != n {
+			t.Fatalf("constructed path is %d bytes, want exactly %d", len(path), n)
+		}
+		return dir, path
+	}
+
+	t.Run("exactly at the cap binds", func(t *testing.T) {
+		// The boundary case: the check must reject only what the kernel would.
+		// An off-by-one (>= instead of >) refuses a path that binds fine, so this
+		// is what keeps the guard from being over-tight.
+		_, path := padTo(t, sunPathMax)
+		ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
+		defer cancel()
+		l, err := listenAgentSocket(ctx, path, stubHandler(t))
+		if err != nil {
+			t.Fatalf("listen on a %d-byte path (exactly the cap) = %v, want success", len(path), err)
+		}
+		if err := l.Close(context.Background()); err != nil {
+			t.Errorf("Close: %v", err)
+		}
+	})
+
+	t.Run("one byte over is refused with a diagnostic", func(t *testing.T) {
+		dir, path := padTo(t, sunPathMax+1)
+		ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
+		defer cancel()
+		l, err := listenAgentSocket(ctx, path, stubHandler(t))
+		if err == nil {
+			_ = l.Close(context.Background())
+			t.Fatal("listen on an over-long path must fail, got nil error")
+		}
+		// The diagnostic is the whole point: the message must carry both numbers
+		// and the flag to change, none of which the kernel's EINVAL does. Match
+		// the surrounding phrases, not the bare digits — a temp path carries
+		// random digits that could contain the cap by chance and pass a
+		// substring check against a message that never mentioned it.
+		msg := err.Error()
+		for _, want := range []string{
+			fmt.Sprintf("is %d bytes", len(path)),
+			fmt.Sprintf("over the %d-byte", sunPathMax),
+			"--runtime-dir",
+		} {
+			if !strings.Contains(msg, want) {
+				t.Errorf("error %q does not contain %q", msg, want)
+			}
+		}
+		// A bare EINVAL is exactly what the check exists to replace, so the
+		// message must not be one: that would mean the path reached bind.
+		if strings.Contains(msg, "invalid argument") {
+			t.Errorf("error %q reached bind; the pre-check must refuse it first", msg)
+		}
+
+		// Refused before anything was created: no directory left behind.
+		if _, err := os.Lstat(dir); !os.IsNotExist(err) {
+			t.Errorf("Lstat(%q) = %v, want the dir never to have been created", dir, err)
+		}
+	})
+}
+
 // Case 8. Close is idempotent and tolerates the socket file already being gone:
 // a second Close returns nil (the os.Remove ErrNotExist is swallowed), and a
 // Close after the socket was removed out from under the listener does not error
diff --git a/go/internal/runnerhub/integration_pgtest_test.go b/go/internal/runnerhub/integration_pgtest_test.go
index 380e50d1..dd7cbfe2 100644
--- a/go/internal/runnerhub/integration_pgtest_test.go
+++ b/go/internal/runnerhub/integration_pgtest_test.go
@@ -30,6 +30,8 @@ import (
 	"os"
 	"path/filepath"
 	stdruntime "runtime"
+	"strings"
+	"syscall"
 	"testing"
 	"time"
 
@@ -50,37 +52,14 @@ const integrationTimeout = 30 * time.Second
 
 func TestIntegrationProvisionStartRelayToStoreAndBus(t *testing.T) {
 	dsn := pgtest.RequireDSN(t) // SKIPs when no podman/DSN — never fails hard.
+	// First cleanup registered, so LIFO removes the tree LAST — after
+	// runSessionsLoop's drain has confirmed the loop left dispatch (see there).
+	runtimeDir := shortRuntimeDir(t)
 	ctx, cancel := context.WithCancel(context.Background())
-	t.Cleanup(cancel)
 
-	st, err := store.Open(ctx, dsn)
-	if err != nil {
-		t.Fatalf("store.Open: %v", err)
-	}
-	t.Cleanup(st.Close)
-
-	// A real agent account + its home channel: the write-through target for a
-	// relayed conversation frame. The agent is a member of its home channel, so
-	// AppendMessage authorizes.
-	admin, err := st.BootstrapAdmin(ctx, store.NewUser{Handle: "admin", DisplayName: "Administrator"})
-	if err != nil {
-		t.Fatalf("BootstrapAdmin: %v", err)
-	}
-	agent, err := st.CreateAgent(ctx, admin.ID, store.NewAgent{Handle: "atlas", DisplayName: "Atlas"})
-	if err != nil {
-		t.Fatalf("CreateAgent: %v", err)
-	}
+	st, agent, bus, sub := openStoreFixture(t, ctx, dsn)
 	homeChannel := agent.Agent.HomeChannelID
 
-	// The real event bus (the SubscribeEvents surface) + a live subscription to
-	// observe the lifecycle status the seam fans out.
-	bus := events.NewBus[*compassv1.SubscribeEventsResponse]()
-	t.Cleanup(bus.Close)
-	sub, err := bus.Subscribe(0, 0)
-	if err != nil {
-		t.Fatalf("bus.Subscribe: %v", err)
-	}
-
 	// A store-backed ConversationSink: a relayed conversation frame is committed
 	// to the agent's home channel and the commit is signalled so the test
 	// event-gates on the observed write, not a sleep.
@@ -120,51 +99,17 @@ func TestIntegrationProvisionStartRelayToStoreAndBus(t *testing.T) {
 		CheckoutDir: "/work/repo",
 		HomeDir:     "/home/agent",
 		UID:         1000,
-		NamePrefix:  "compass-agent-",
+		NamePrefix:  agentNamePrefix,
 	})
 	if err != nil {
 		t.Fatalf("NewConfigSpecBuilder: %v", err)
 	}
 	registry := runtime.NewAgentRegistry()
 	rt := runtime.NewAgentRuntimeWithRegistry(engine, registry)
-	host := runner.NewSessionHost(link, rt, registry, engine, specs, t.TempDir(), discardLog(), nil)
-	loopDone := make(chan error, 1)
-	go func() { loopDone <- link.RunSessions(ctx, host) }()
-
-	// Provision the workspace through the public hub path → the Runner launches
-	// the (fake) container and returns its name. The Sessions stream is
-	// server-speaks-first: RunSessions' bootstrap Send flushes the headers that
-	// run the server handler's router.attach, and that round-trip is async to the
-	// goroutine spawned above. A command dispatched into the pre-attach window
-	// gets a retriable Unavailable ("no live runner sessions stream") — the same
-	// transient a production client rides out. Gate on the seam being live by
-	// retrying Provision (idempotent on its stable request id "prov-1", so no
-	// double-provision), yielding to the handler goroutine between probes; a
-	// deadline bounds it so a genuinely wedged seam fails fast, never a sleep.
-	var provResp *compassv1.ProvisionAgentWorkspaceResponse
-	provDeadline := time.After(integrationTimeout)
-	for {
-		provResp, err = hub.Provision(ctx, "prov-1", &compassv1.ProvisionAgentWorkspaceRequest{
-			AgentAccountId: string(agent.ID),
-			Repo:           &compassv1.ProvisionAgentWorkspaceRequest_LocalPath{LocalPath: "/mirror/repo.git"},
-		})
-		if err == nil {
-			break
-		}
-		if connect.CodeOf(err) != connect.CodeUnavailable {
-			t.Fatalf("hub.Provision over the seam = %v", err)
-		}
-		select {
-		case <-provDeadline:
-			t.Fatalf("hub.Provision never reached a live Sessions stream: %v", err)
-		default:
-		}
-		stdruntime.Gosched()
-	}
-	containerName := provResp.GetContainerName()
-	if containerName == "" {
-		t.Fatal("Provision returned an empty container name")
-	}
+	host := runner.NewSessionHost(link, rt, registry, engine, specs, runtimeDir, discardLog(), nil)
+	loopDone := runSessionsLoop(t, ctx, cancel, link, host)
+
+	containerName := provisionWhenSeamLive(t, ctx, hub, agent.ID)
 
 	// Start the session → the Runner spawns the agent relay over the pipe engine.
 	startResp, err := hub.Start(ctx, "start-1", &compassv1.StartAgentSessionRequest{ContainerName: containerName})
@@ -176,23 +121,7 @@ func TestIntegrationProvisionStartRelayToStoreAndBus(t *testing.T) {
 		t.Fatal("Start returned an empty session id")
 	}
 
-	// The relay goroutine is now reading the agent's stdout pipe. Write one
-	// conversation frame (→ committed to the store) and one lifecycle frame (→
-	// fanned onto the bus).
-	writeAgentFrame(t, engine.stdoutW, &compassv1internal.AgentFrame{
-		Frame: &compassv1internal.AgentFrame_ConversationPosted{
-			ConversationPosted: &compassv1.MessagePosted{
-				Message: &compassv1.Message{
-					Blocks: []*compassv1.MessageBlock{{Block: &compassv1.MessageBlock_Text{Text: "e2e relayed reply"}}},
-				},
-			},
-		},
-	})
-	writeAgentFrame(t, engine.stdoutW, &compassv1internal.AgentFrame{
-		Frame: &compassv1internal.AgentFrame_Session{
-			Session: &compassv1internal.SessionFrame{State: compassv1.AgentSessionState_AGENT_SESSION_STATE_WORKING},
-		},
-	})
+	writeRelayFrames(t, engine)
 
 	// The conversation frame was written THROUGH Deliver to the real store: gate
 	// on the commit signal, then read the message back.
@@ -221,10 +150,18 @@ func TestIntegrationProvisionStartRelayToStoreAndBus(t *testing.T) {
 		t.Fatalf("lifecycle status state = %v, want WORKING (the relayed transition)", status.GetState())
 	}
 
-	// Clean teardown mirrors production shutdown (run.go): stop the session, then
-	// cancel the loop. closeStdout models the container's stdout closing on death
-	// (the fake's pipe is not ctx-bound the way a real exec's is), so the relay's
-	// scan ends; cancel then unwinds RunSessions' blocking Receive.
+	assertCleanShutdown(t, ctx, cancel, host, engine, sessionID, loopDone)
+}
+
+// assertCleanShutdown mirrors production shutdown (run.go): stop the session,
+// then cancel the loop. closeStdout models the container's stdout closing on
+// death (the fake's pipe is not ctx-bound the way a real exec's is), so the
+// relay's scan ends; cancel then unwinds RunSessions' blocking Receive.
+//
+// This is the CLEAN path, asserted as its own property. The cleanup registered
+// by runSessionsLoop covers the failing paths, where a t.Fatalf skips this.
+func assertCleanShutdown(t *testing.T, ctx context.Context, cancel context.CancelFunc, host runner.SessionHost, engine *integPipeRuntime, sessionID string, loopDone <-chan error) {
+	t.Helper()
 	if err := host.Stop(ctx, sessionID); err != nil {
 		t.Fatalf("host.Stop = %v", err)
 	}
@@ -237,6 +174,101 @@ func TestIntegrationProvisionStartRelayToStoreAndBus(t *testing.T) {
 	}
 }
 
+// provisionWhenSeamLive provisions the workspace through the public hub path →
+// the Runner launches the (fake) container and returns its name.
+//
+// The Sessions stream is server-speaks-first: RunSessions' bootstrap Send
+// flushes the headers that run the server handler's router.attach, and that
+// round-trip is async to the loop goroutine. A command dispatched into the
+// pre-attach window gets a retriable Unavailable ("no live runner sessions
+// stream") — the same transient a production client rides out. So gate on the
+// seam being live by retrying, idempotent on the stable request id "prov-1" so
+// there is no double-provision, yielding to the handler goroutine between
+// probes. A deadline bounds it, so a genuinely wedged seam fails fast rather
+// than spinning, and nothing here is a sleep.
+func provisionWhenSeamLive(t *testing.T, ctx context.Context, hub *runnerhub.Hub, agentID store.AccountID) string {
+	t.Helper()
+	deadline := time.After(integrationTimeout)
+	for {
+		resp, err := hub.Provision(ctx, "prov-1", &compassv1.ProvisionAgentWorkspaceRequest{
+			AgentAccountId: string(agentID),
+			Repo:           &compassv1.ProvisionAgentWorkspaceRequest_LocalPath{LocalPath: "/mirror/repo.git"},
+		})
+		if err == nil {
+			name := resp.GetContainerName()
+			if name == "" {
+				t.Fatal("Provision returned an empty container name")
+			}
+			return name
+		}
+		if connect.CodeOf(err) != connect.CodeUnavailable {
+			t.Fatalf("hub.Provision over the seam = %v", err)
+		}
+		select {
+		case <-deadline:
+			t.Fatalf("hub.Provision never reached a live Sessions stream: %v", err)
+		default:
+		}
+		stdruntime.Gosched()
+	}
+}
+
+// writeRelayFrames drives the agent side of the seam: the relay goroutine is
+// reading the agent's stdout pipe, so write one conversation frame (committed
+// through to the store) and one lifecycle frame (fanned onto the bus). Together
+// they exercise both write-through paths the integration exists to cover.
+func writeRelayFrames(t *testing.T, engine *integPipeRuntime) {
+	t.Helper()
+	writeAgentFrame(t, engine.stdoutW, &compassv1internal.AgentFrame{
+		Frame: &compassv1internal.AgentFrame_ConversationPosted{
+			ConversationPosted: &compassv1.MessagePosted{
+				Message: &compassv1.Message{
+					Blocks: []*compassv1.MessageBlock{{Block: &compassv1.MessageBlock_Text{Text: "e2e relayed reply"}}},
+				},
+			},
+		},
+	})
+	writeAgentFrame(t, engine.stdoutW, &compassv1internal.AgentFrame{
+		Frame: &compassv1internal.AgentFrame_Session{
+			Session: &compassv1internal.SessionFrame{State: compassv1.AgentSessionState_AGENT_SESSION_STATE_WORKING},
+		},
+	})
+}
+
+// openStoreFixture opens the store and builds the account + bus fixture the seam
+// writes through: a real agent account and its home channel (the write-through
+// target for a relayed conversation frame — the agent is a member of its home
+// channel, so AppendMessage authorizes), plus the real event bus and a live
+// subscription observing the lifecycle status the seam fans out.
+//
+// Its cleanups register here, so under LIFO they run after everything the caller
+// registers later: the store and bus outlive the Runner loop that talks to them.
+func openStoreFixture(t *testing.T, ctx context.Context, dsn string) (*store.Store, store.Account, *events.Bus[*compassv1.SubscribeEventsResponse], events.Subscription[*compassv1.SubscribeEventsResponse]) {
+	t.Helper()
+	st, err := store.Open(ctx, dsn)
+	if err != nil {
+		t.Fatalf("store.Open: %v", err)
+	}
+	t.Cleanup(st.Close)
+
+	admin, err := st.BootstrapAdmin(ctx, store.NewUser{Handle: "admin", DisplayName: "Administrator"})
+	if err != nil {
+		t.Fatalf("BootstrapAdmin: %v", err)
+	}
+	agent, err := st.CreateAgent(ctx, admin.ID, store.NewAgent{Handle: "atlas", DisplayName: "Atlas"})
+	if err != nil {
+		t.Fatalf("CreateAgent: %v", err)
+	}
+
+	bus := events.NewBus[*compassv1.SubscribeEventsResponse]()
+	t.Cleanup(bus.Close)
+	sub, err := bus.Subscribe(0, 0)
+	if err != nil {
+		t.Fatalf("bus.Subscribe: %v", err)
+	}
+	return st, agent, bus, sub
+}
+
 // storeConversationSink commits a relayed conversation frame to a fixed channel
 // as a fixed author (the seam's session→channel mapping T5 will own; fixed here
 // so the test drives a real store write-through). It signals each commit so the
@@ -413,6 +445,110 @@ func textOf(m store.Message) string {
 
 func discardLog() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) }
 
+// agentNamePrefix is the container-name prefix this test wires into its
+// SpecDefaults, hoisted so shortRuntimeDir models the same name the Runner
+// actually builds (spec.go:73 joins it with the account id). Editing the prefix
+// in one place would otherwise silently shrink the modelled path and turn the
+// budget assertion into a false negative.
+const agentNamePrefix = "compass-agent-"
+
+// accountIDHexLen is the width of a store account id: 16 random bytes
+// hex-encoded (internal/store/ids.go:21-28). Fixed at that minting site — the
+// only one — rather than validated where the path is built, so it is the right
+// width to model the tail with and the wrong thing to call a guarantee.
+const accountIDHexLen = 32
+
+// runSessionsLoop starts the Runner's dispatch loop and registers the teardown
+// that must bracket it. The ordering is load-bearing in both directions, which
+// is why it lives beside the goroutine rather than beside context.WithCancel.
+//
+// It must run BEFORE httptest's srv.Close (registered inside mountRunnerServer,
+// therefore earlier, therefore later under LIFO): Close waits on its handlers,
+// and the live Sessions handler returns only once ctx is cancelled, so
+// cancelling after Close deadlocks the entire cleanup stack — every later
+// cleanup, the runtime dir removal included, never runs.
+//
+// It must run AFTER the runtime dir removal registered at the top of the test,
+// so LIFO reclaims that tree only once the loop has left dispatch. Cancel alone
+// would not do it: cancel signals and returns, so the WAIT is what makes the
+// ordering mean anything.
+//
+// The wait is what orders the two; it is not a gate on the removal. If the
+// drain times out, the later cleanups still run — a cleanup cannot cancel the
+// ones registered before it. Neither `return` (it exits only its own closure)
+// nor t.Fatalf (it marks the test and runs the rest of the stack anyway) skips
+// them, so the timeout arm reports the collision rather than averting it. That
+// is the right trade at this point: the test has already failed, and the
+// alternative — a flag threaded into shortRuntimeDir to skip RemoveAll — leaks
+// the tree and couples two independent helpers to buy nothing a red test needs.
+func runSessionsLoop(t *testing.T, ctx context.Context, cancel context.CancelFunc, link *runner.ServerLink, host runner.SessionHost) <-chan error {
+	t.Helper()
+	loopDone := make(chan error, 1)
+	go func() {
+		loopDone <- link.RunSessions(ctx, host)
+		// Closed as well as sent: the clean-teardown drain at the end of the test
+		// takes the value, and the cleanup below must still observe the exit.
+		close(loopDone)
+	}()
+	t.Cleanup(func() {
+		cancel()
+		select {
+		case <-loopDone:
+		case <-time.After(integrationTimeout):
+			t.Errorf("RunSessions still running %s after cancel; the runtime dir removal below runs anyway and will race an in-flight Provision", integrationTimeout)
+		}
+	})
+	return loopDone
+}
+
+// shortRuntimeDir is a Runner RuntimeDir bounded to fit the AF_UNIX sun_path
+// limit, replacing t.TempDir() for the one test here that builds a real agent
+// socket. The Runner appends a 69-byte tail to its RuntimeDir
+// (/containers/compass-agent-<32hex>/agent.sock, host.go:291) at the
+// store-minted 32-hex id, leaving 38 bytes of a 107-byte cap on Linux.
+// t.TempDir() derives its path from the TEST NAME, and every one of this
+// package's tests exceeds that budget: the ceiling is a 19-character name and
+// the shortest here is 26. Only this test fails today because only this one
+// opens a socket, so the name-length dependency is a trap for the next test
+// that wires a SessionHost.
+//
+// A fixed short root removes the TEST-NAME dependency. It does not make the
+// budget unconditional: the root still comes from TMPDIR, and a deep one (a CI
+// work dir, or macOS's ~49-byte /var/folders/<2>/<hash>/T) re-inflates it. So
+// the resulting path is asserted rather than assumed.
+//
+// This site FAILS rather than skips on an over-budget root, unlike
+// gateway/socket_test.go:381 which t.Skipf's the same condition: there the
+// environment merely blocks one unit assertion, whereas here a skip would
+// silently drop the only end-to-end coverage of the socket path. The cap is
+// derived the way the production guard derives it (gateway/socket.go:59), never
+// restated — sun_path is not one size across the platforms //go:build unix
+// admits (108 on linux/solaris/illumos, 104 on darwin and the BSDs, 1023 on
+// aix), and this file is //go:build unix.
+func shortRuntimeDir(t *testing.T) string {
+	t.Helper()
+	dir, err := os.MkdirTemp("", "cr") //nolint:usetesting // t.TempDir embeds the test name, which is what put this path over the sun_path cap — the bug this helper exists to prevent
+	if err != nil {
+		t.Fatalf("MkdirTemp for runner runtime dir: %v", err)
+	}
+	// Longest path the Runner builds under dir. sun_path holds the path plus a
+	// NUL, so the usable cap is one less than the platform's array.
+	const sunPathMax = len(syscall.RawSockaddrUnix{}.Path) - 1
+	longest := filepath.Join(dir, "containers", agentNamePrefix+strings.Repeat("f", accountIDHexLen), "agent.sock")
+	if len(longest) > sunPathMax {
+		if rmErr := os.RemoveAll(dir); rmErr != nil {
+			t.Errorf("removing over-budget runner runtime dir %q: %v", dir, rmErr)
+		}
+		t.Fatalf("runner runtime dir %q yields a %d-byte agent socket path, over the %d-byte sun_path cap (TMPDIR too deep)", dir, len(longest), sunPathMax)
+	}
+	t.Cleanup(func() {
+		if err := os.RemoveAll(dir); err != nil {
+			t.Errorf("removing runner runtime dir %q: %v", dir, err)
+		}
+	})
+	return dir
+}
+
 func cleartextH2() *http.Protocols {
 	p := new(http.Protocols)
 	p.SetHTTP1(true)

From 3f4f7cc11b1e34aace35b00e00195d83ecbbc936 Mon Sep 17 00:00:00 2001
From: Matt Wilkinson <matt@sealedsecurity.com>
Date: Mon, 27 Jul 2026 23:42:42 -0400
Subject: [PATCH 2/5] fix(runner): reject an agent account id that is not a
 safe path element
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review of the sun_path guard found the gap beside it: the account id is
interpolated into the container name (spec.go) and that name becomes a path
segment of the agent socket (host.go). filepath.Join cleans, so a "../" in
the id escapes RuntimeDir entirely — verified, "../../../../tmp/pwned"
resolves to /run/tmp/pwned/agent.sock, where listenAgentSocket would
MkdirAll a 0700 directory and bind. The length guard cannot catch it,
because traversal SHORTENS the path: that example is 25 bytes.

The three arguments the guard's own comment made for the id being safe do
not hold at this hop, and the comment claiming otherwise made the gap look
considered-and-closed. The id is minted 32-hex, but the width is a property
of the minting site and is never re-checked in the Runner; the foreign key
tying the id to a real account is enforced by RecordAgentContainer, which
runs after hub.Provision has already created the directory and bound the
socket; and an admin-only RPC narrows who may call it, not what they may
pass. Length and shape are the same validation of the same input at the
same hop, so validate both: validAccountID refuses anything that is not a
single safe path element, at the entry point in BuildSpec.

Also from the review:

Make padTo fail rather than skip. Under a deep TMPDIR both subtests skipped
and the package reported ok — reproduced with a 100-byte TMPDIR. Those two
subtests are the only coverage of the boundary the guard enforces, so a
silent skip reports green for a package that asserted nothing, which is the
same false-green this change exists to remove. The sibling helper
shortRuntimeDir already fails closed on the identical condition; the two now
agree.

Restore t.Cleanup(cancel) adjacent to context.WithCancel in the integration
test. Cancel was registered only inside runSessionsLoop, leaving ~60 lines
of fixture setup that could t.Fatalf with the context never cancelled. The
later registration still runs first under LIFO, so the drain ordering that
loop documents is unchanged, and CancelFunc is idempotent.

Tie the pgtest path budget to the real minted id. shortRuntimeDir models the
account id as accountIDHexLen "f"s because it runs before an account exists;
an assertion after the fixture now reddens if store ids widen, instead of
letting the budget silently go stale while the real path overruns.

Trim the guard's comments to what is load-bearing. Dropped two stdlib line
citations that pin the comment to a toolchain version nothing asserts, and
corrected the claim that the constant is "exactly the bound Go enforces" —
AIX permits len(Path) and a Linux abstract name carries no terminator, so
the constant is deliberately conservative at both edges. Neither reaches the
Runner, which only binds filesystem paths. Fixed a stale line citation to
Close, added in the same diff.

Refs SEA-1440, SEA-1443

Co-Authored-By: seal <noreply@sealedsecurity.com>
---
 go/internal/runner/gateway/socket.go          | 33 ++++++-----
 go/internal/runner/gateway/socket_test.go     |  8 ++-
 go/internal/runner/spec.go                    | 29 +++++++++-
 go/internal/runner/spec_test.go               | 58 +++++++++++++++++++
 .../runnerhub/integration_pgtest_test.go      | 15 +++++
 5 files changed, 123 insertions(+), 20 deletions(-)

diff --git a/go/internal/runner/gateway/socket.go b/go/internal/runner/gateway/socket.go
index 7a2e0b29..9b5bae33 100644
--- a/go/internal/runner/gateway/socket.go
+++ b/go/internal/runner/gateway/socket.go
@@ -61,10 +61,14 @@ const shutdownGrace = 5 * time.Second
 // have worked, but an over-sized one passes a path the kernel rejects at bind,
 // which is the exact failure this check exists to prevent.
 //
-// The expression is also not an approximation of the kernel's rule but exactly
-// the bound Go's own wrapper enforces before the syscall: syscall_linux.go:559
-// rejects n == len(Path) for a non-abstract name, syscall_bsd.go:191 rejects
-// n >= len(Path). Two different predicates, both landing on len-1.
+// The -1 is the NUL terminator, which is justification enough on its own — no
+// appeal to Go's internals is needed, and citing them by line would pin this
+// comment to a toolchain version nothing here asserts. Worth knowing that the
+// bound is deliberately conservative rather than exact at two edges: AIX's
+// wrapper permits a name of len(Path), and a Linux ABSTRACT name carries no
+// terminator, so both admit one byte more than this constant. Neither reaches
+// the Runner, which only ever binds a filesystem path, and refusing a byte that
+// would have worked is the safe direction.
 const sunPathMax = len(syscall.RawSockaddrUnix{}.Path) - 1
 
 // runnerUID reports the uid a reclaimable stale socket must be owned by. It is a
@@ -97,22 +101,21 @@ type SocketListener struct {
 // the diagnostic: a bind that fails EINVAL has already run MkdirAll, and the
 // dir survives — measured, 0700 left on disk. Nothing reclaims it on any path,
 // because nothing in this socket's lifecycle removes the directory at all:
-// Close removes the socket file (:210) and never filepath.Dir(path), so the
+// Close removes the socket file and never filepath.Dir(path), so the
 // Launch-failure teardown Provision does run (host.go:120-125) leaks it too.
 // Refusing before the mkdir is what makes the question moot, rather than a
 // teardown that would have to be added.
 func listenAgentSocket(ctx context.Context, path string, h http.Handler) (*SocketListener, error) {
-	// Two inputs drive this length. The socket path is RuntimeDir +
-	// /containers/<container>/agent.sock (host.go:291), where <container> is
-	// NamePrefix + the agent account id (spec.go:73).
+	// The socket path is RuntimeDir + /containers/<container>/agent.sock
+	// (host.go), where <container> is NamePrefix + the agent account id
+	// (spec.go). RuntimeDir is the operator-supplied variable: --runtime-dir is
+	// unbounded on every hop and inflates the path for every agent at once.
 	//
-	// Only one of them is a real variable. The account id is server-minted at
-	// exactly 32 hex chars (store/ids.go:21-29), Provision is admin-only
-	// (auth/admin_gate.go:50-56), and the id is FK-constrained to an existing
-	// account (0003_agent_ownership.sql:26) — so a longer id reaches this check
-	// but can never name an account that exists. RuntimeDir is operator-supplied
-	// (--runtime-dir) with no bound on any hop, and it inflates the path for
-	// every agent at once rather than for one doomed request.
+	// The account id's SHAPE is validated where it enters, not here — see
+	// validAccountID in spec.go, which refuses an id that is not a single safe
+	// path element. That is a separate property from length and cannot be
+	// checked here: a traversing id SHORTENS the path, so it would sail past
+	// this guard.
 	//
 	// With the minted id the tail is 69 bytes, so the default /run/compass lands
 	// at 81 and the --runtime-dir budget is sunPathMax-69: 38 on Linux, 34 on
diff --git a/go/internal/runner/gateway/socket_test.go b/go/internal/runner/gateway/socket_test.go
index 32a6831d..4d52c771 100644
--- a/go/internal/runner/gateway/socket_test.go
+++ b/go/internal/runner/gateway/socket_test.go
@@ -401,10 +401,12 @@ func TestRejectsPathOverSunPathLimit(t *testing.T) {
 		})
 		dir = filepath.Join(root, "run")
 		base := len(dir) + 1 // + separator
-		// Backstop for a pathological TMPDIR; with the short root above this
-		// should not fire in practice on any supported platform.
+		// These two subtests are the only coverage of the boundary the guard
+		// exists to enforce, so a deep TMPDIR must redden the run rather than
+		// skip it — a skip here reports `ok` for a package that asserted
+		// nothing, which is the failure mode this whole change is about.
 		if base >= n {
-			t.Skipf("temp root %q is %d bytes, leaving no room for a %d-byte path", dir, base, n)
+			t.Fatalf("temp root %q is %d bytes, leaving no room for a %d-byte path; set TMPDIR to a short path", dir, base, n)
 		}
 		path = filepath.Join(dir, strings.Repeat("s", n-base))
 		if len(path) != n {
diff --git a/go/internal/runner/spec.go b/go/internal/runner/spec.go
index 83c19c8c..41e74f98 100644
--- a/go/internal/runner/spec.go
+++ b/go/internal/runner/spec.go
@@ -11,6 +11,7 @@ package runner
 import (
 	"errors"
 	"fmt"
+	"io/fs"
 	"net/url"
 	"strings"
 
@@ -67,8 +68,8 @@ func (b *configSpecBuilder) BuildSpec(req *compassv1.ProvisionAgentWorkspaceRequ
 
 	d := b.defaults
 	accountID := req.GetAgentAccountId()
-	if accountID == "" {
-		return runtime.AgentSpec{}, errors.New("provision request requires an agent account id")
+	if err := validAccountID(accountID); err != nil {
+		return runtime.AgentSpec{}, err
 	}
 	name := d.NamePrefix + accountID
 	return runtime.AgentSpec{
@@ -86,6 +87,30 @@ func (b *configSpecBuilder) BuildSpec(req *compassv1.ProvisionAgentWorkspaceRequ
 	}, nil
 }
 
+// validAccountID refuses an agent account id that is not a single safe path
+// element. The id is not merely a label: it is concatenated into the container
+// name (below) and that name becomes a path segment of the agent socket,
+// RuntimeDir/containers/<container>/agent.sock (host.go). filepath.Join CLEANS,
+// so a "../" in the id escapes RuntimeDir entirely — "../../../../tmp/pwned"
+// resolves to /run/tmp/pwned/agent.sock — and the socket's own length guard
+// cannot catch it, because traversal SHORTENS the path.
+//
+// Nothing upstream makes this check redundant. The id is minted 32-hex, but the
+// width is a property of the minting site and is never re-checked here; the
+// foreign key that ties the id to a real account is enforced by
+// RecordAgentContainer, which runs after hub.Provision has already created the
+// 0700 directory and bound the socket; and an admin-only RPC narrows who calls
+// it, not what they may pass. So this is the hop that has to reject the shape.
+func validAccountID(id string) error {
+	if id == "" {
+		return errors.New("provision request requires an agent account id")
+	}
+	if !fs.ValidPath(id) || id == "." || strings.Contains(id, "/") {
+		return fmt.Errorf("agent account id %q is not a valid path element", id)
+	}
+	return nil
+}
+
 // repoSourceFromRequest maps the request's repo oneof to a runtime.RepoSource,
 // enforcing that exactly one variant is set.
 func repoSourceFromRequest(req *compassv1.ProvisionAgentWorkspaceRequest) (runtime.RepoSource, error) {
diff --git a/go/internal/runner/spec_test.go b/go/internal/runner/spec_test.go
index cd05ce2d..14316d57 100644
--- a/go/internal/runner/spec_test.go
+++ b/go/internal/runner/spec_test.go
@@ -170,6 +170,64 @@ func TestBuildSpecRejectsEmptyAgentAccountID(t *testing.T) {
 	}
 }
 
+// The agent account id becomes a path segment of the agent socket
+// (RuntimeDir/containers/<prefix><id>/agent.sock), and filepath.Join cleans, so
+// a "../" in the id escapes RuntimeDir: "../../../../tmp/pwned" resolves to
+// /run/tmp/pwned/agent.sock, where the Runner would MkdirAll a 0700 directory
+// and bind. The socket's length guard cannot catch this, because traversal
+// SHORTENS the path — every row here is well under the AF_UNIX cap. Each reject
+// must surface a non-nil error AND the zero AgentSpec, so no half-built spec
+// carries an escaping name to Launch. Uses a valid repo so the id is the only
+// failing dimension.
+func TestBuildSpecRejectsAgentAccountIDThatEscapesItsPathElement(t *testing.T) {
+	builder, err := NewConfigSpecBuilder(goodDefaults())
+	if err != nil {
+		t.Fatalf("NewConfigSpecBuilder: %v", err)
+	}
+	rejected := []struct {
+		name      string
+		accountID string
+	}{
+		{"parent traversal escaping the runtime dir", "../../../../tmp/pwned"},
+		{"shallow parent traversal", "../../etc"},
+		{"a single parent segment", ".."},
+		{"the current directory", "."},
+		{"an embedded separator", "abc/def"},
+		{"a leading separator (absolute)", "/etc/passwd"},
+		{"a trailing separator", "abc/"},
+	}
+	for _, tc := range rejected {
+		t.Run(tc.name, func(t *testing.T) {
+			spec, err := builder.BuildSpec(&compassv1.ProvisionAgentWorkspaceRequest{
+				AgentAccountId: tc.accountID,
+				Repo:           &compassv1.ProvisionAgentWorkspaceRequest_RemoteUrl{RemoteUrl: "https://example.com/r.git"},
+			})
+			if err == nil {
+				t.Fatalf("BuildSpec with agent_account_id %q = nil error, want a path-element rejection", tc.accountID)
+			}
+			if !strings.Contains(err.Error(), "agent account id") {
+				t.Fatalf("BuildSpec error = %v, want the agent-account-id validation error (not a repo failure)", err)
+			}
+			if spec.Name != "" {
+				t.Fatalf("BuildSpec on rejection returned spec with name %q, want the zero AgentSpec", spec.Name)
+			}
+		})
+	}
+
+	// The ordinary minted shape still builds, so the guard refuses traversal
+	// rather than every id.
+	spec, err := builder.BuildSpec(&compassv1.ProvisionAgentWorkspaceRequest{
+		AgentAccountId: strings.Repeat("f", 32),
+		Repo:           &compassv1.ProvisionAgentWorkspaceRequest_RemoteUrl{RemoteUrl: "https://example.com/r.git"},
+	})
+	if err != nil {
+		t.Fatalf("BuildSpec with a 32-hex account id: %v", err)
+	}
+	if want := "compass-agent-" + strings.Repeat("f", 32); spec.Name != want {
+		t.Fatalf("BuildSpec name = %q, want %q", spec.Name, want)
+	}
+}
+
 // BuildSpec must constrain the caller-supplied repo value before it is forwarded
 // verbatim as the `git clone` target: an unconstrained remote_url reaches git's
 // `ext::<cmd>` (arbitrary command) / `file://` (read-outside-boundary) transports,
diff --git a/go/internal/runnerhub/integration_pgtest_test.go b/go/internal/runnerhub/integration_pgtest_test.go
index dd7cbfe2..e88b5598 100644
--- a/go/internal/runnerhub/integration_pgtest_test.go
+++ b/go/internal/runnerhub/integration_pgtest_test.go
@@ -56,9 +56,24 @@ func TestIntegrationProvisionStartRelayToStoreAndBus(t *testing.T) {
 	// runSessionsLoop's drain has confirmed the loop left dispatch (see there).
 	runtimeDir := shortRuntimeDir(t)
 	ctx, cancel := context.WithCancel(context.Background())
+	// Registered adjacent to WithCancel so the ~60 lines of fixture setup below
+	// (openStoreFixture, runner.Dial, NewConfigSpecBuilder) cannot t.Fatalf out
+	// with the context never cancelled. runSessionsLoop registers cancel again,
+	// later, so LIFO still runs its copy FIRST and the drain ordering the loop
+	// documents is unchanged; context.CancelFunc is idempotent, so the second
+	// call here is a no-op.
+	t.Cleanup(cancel)
 
 	st, agent, bus, sub := openStoreFixture(t, ctx, dsn)
 	homeChannel := agent.Agent.HomeChannelID
+	// shortRuntimeDir budgeted the path against a MODEL of the account id
+	// (accountIDHexLen "f"s), because it runs before an account exists. Tie the
+	// model to the real minted value now that it does: widen store ids and this
+	// reddens here, rather than silently invalidating that budget and letting
+	// the real socket path overrun.
+	if got := len(agent.ID); got != accountIDHexLen {
+		t.Fatalf("minted account id is %d chars, but shortRuntimeDir budgeted for %d; update accountIDHexLen", got, accountIDHexLen)
+	}
 
 	// A store-backed ConversationSink: a relayed conversation frame is committed
 	// to the agent's home channel and the commit is signalled so the test

From 0211c36e1c22a00e0337c14f9e6a13e215dc3da5 Mon Sep 17 00:00:00 2001
From: Matt Wilkinson <matt@sealedsecurity.com>
Date: Tue, 28 Jul 2026 00:12:18 -0400
Subject: [PATCH 3/5] fix(runner): constrain both operands of the container
 name, and correct the comments the last fix invalidated
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Round 2 of review found no high or medium defect in the traversal guard —
the three conjuncts are each load-bearing and there is no bypass into the
socket-path construction — but it caught the previous commit leaving two
comments asserting the opposite of the code beside them, which is the same
defect class that commit was correcting.

Correct them. padTo's doc line still promised it "skips when the root alone
already exceeds the budget" twenty lines above the t.Fatalf that replaced
the skip. The pgtest helper built a justification on a contrast that no
longer exists ("unlike gateway/socket_test.go:381 which t.Skipf's the same
condition") — that Skipf is gone and :381 never held it, so the next reader
was sent to reconcile something already reconciled. Both now state the
standing reason on its own terms. Two further line citations pointed at a
prose line and a bare brace; they cite the symbol instead, which is what
this diff already argued for when it dropped the stdlib line citations. The
"never restated" claim is narrowed to what is true: the number is never
restated, the derivation is duplicated because the constant is unexported.

Validate NamePrefix at startup. The container name is NamePrefix +
accountID, and the previous commit constrained only the request-derived
operand — so the operator-derived one could still carry a separator and
escape RuntimeDir through the same filepath.Join clean. Not reachable today
(NamePrefix is a literal in main), but NewConfigSpecBuilder exists to make a
misconfigured Runner fail at startup rather than at first provision, and a
path-affecting default is exactly that class. Both operands are now checked.

Refuse a control character in the account id. A NUL is valid UTF-8 and
carries no separator, so it clears every existing check and fails at bind as
a bare EINVAL naming nothing — the same undiagnosable error the length guard
was added to eliminate. There is no traversal or injection consequence
(podman receives the name as a distinct argv element, not through a shell),
so this is diagnostic quality, refused where the message can name the input.

Both new guards verified red by mutation.

Refs SEA-1440, SEA-1443

Co-Authored-By: seal <noreply@sealedsecurity.com>
---
 go/internal/runner/gateway/socket_test.go     |  5 ++--
 go/internal/runner/spec.go                    | 17 ++++++++++++++
 go/internal/runner/spec_test.go               | 11 +++++++++
 .../runnerhub/integration_pgtest_test.go      | 23 ++++++++++---------
 4 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/go/internal/runner/gateway/socket_test.go b/go/internal/runner/gateway/socket_test.go
index 4d52c771..92d10ba8 100644
--- a/go/internal/runner/gateway/socket_test.go
+++ b/go/internal/runner/gateway/socket_test.go
@@ -378,8 +378,9 @@ func TestRejectsWrongOwnerSocketNeverDeletes(t *testing.T) {
 // the bound to >= refuses the at-cap path the kernel accepts, failing the first
 // subtest.
 func TestRejectsPathOverSunPathLimit(t *testing.T) {
-	// padTo returns a path under a fresh temp root of exactly n bytes, or skips
-	// when the root alone already exceeds the budget (a very deep TMPDIR).
+	// padTo returns a path under a fresh temp root of exactly n bytes, or fails
+	// the subtest when the root alone already exceeds the budget (a very deep
+	// TMPDIR).
 	padTo := func(t *testing.T, n int) (dir, path string) {
 		t.Helper()
 		// A short fixed root, not t.TempDir(): the latter embeds the test name,
diff --git a/go/internal/runner/spec.go b/go/internal/runner/spec.go
index 41e74f98..3bfcc18c 100644
--- a/go/internal/runner/spec.go
+++ b/go/internal/runner/spec.go
@@ -52,6 +52,13 @@ func NewConfigSpecBuilder(defaults SpecDefaults) (SpecBuilder, error) {
 	if defaults.CheckoutDir == "" || defaults.HomeDir == "" {
 		return nil, errors.New("spec defaults require checkout and home dirs")
 	}
+	// The other operand of the container name. validAccountID constrains the
+	// request-derived half; this constrains the operator-derived half, so both
+	// inputs to a path segment are checked and a separator here cannot escape
+	// RuntimeDir through the same filepath.Join clean.
+	if strings.Contains(defaults.NamePrefix, "/") {
+		return nil, errors.New("spec defaults name prefix must not contain a path separator")
+	}
 	return &configSpecBuilder{defaults: defaults}, nil
 }
 
@@ -105,9 +112,19 @@ func validAccountID(id string) error {
 	if id == "" {
 		return errors.New("provision request requires an agent account id")
 	}
+	// fs.ValidPath rejects "", "..", any empty segment, a leading or trailing
+	// "/", and invalid UTF-8, but returns true for "." (documented) and for a
+	// legal multi-element path like "abc/def" — hence the other two conjuncts.
 	if !fs.ValidPath(id) || id == "." || strings.Contains(id, "/") {
 		return fmt.Errorf("agent account id %q is not a valid path element", id)
 	}
+	// A control character survives the checks above (a NUL is valid UTF-8 and
+	// carries no separator) and fails much later, at bind, as a bare EINVAL —
+	// the same undiagnosable error the socket's length guard exists to replace.
+	// Refuse it here, where the message can name the input.
+	if strings.ContainsFunc(id, func(r rune) bool { return r < 0x20 || r == 0x7f }) {
+		return fmt.Errorf("agent account id %q contains a control character", id)
+	}
 	return nil
 }
 
diff --git a/go/internal/runner/spec_test.go b/go/internal/runner/spec_test.go
index 14316d57..c9ecb280 100644
--- a/go/internal/runner/spec_test.go
+++ b/go/internal/runner/spec_test.go
@@ -41,6 +41,11 @@ func TestNewConfigSpecBuilderRejectsIncompleteDefaults(t *testing.T) {
 		{"no image", func(d *SpecDefaults) { d.Image = "" }},
 		{"no checkout dir", func(d *SpecDefaults) { d.CheckoutDir = "" }},
 		{"no home dir", func(d *SpecDefaults) { d.HomeDir = "" }},
+		// NamePrefix is the operator-derived half of the container name, which
+		// becomes a path segment of the agent socket. A separator here escapes
+		// RuntimeDir through the same filepath.Join clean that validAccountID
+		// guards on the request-derived half, so both operands are checked.
+		{"a name prefix containing a path separator", func(d *SpecDefaults) { d.NamePrefix = "a/../../" }},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -195,6 +200,12 @@ func TestBuildSpecRejectsAgentAccountIDThatEscapesItsPathElement(t *testing.T) {
 		{"an embedded separator", "abc/def"},
 		{"a leading separator (absolute)", "/etc/passwd"},
 		{"a trailing separator", "abc/"},
+		// Control characters clear every check above — a NUL is valid UTF-8 and
+		// carries no separator — and would otherwise fail at bind as a bare
+		// EINVAL naming nothing.
+		{"an embedded NUL", "abc\x00def"},
+		{"an embedded newline", "abc\ndef"},
+		{"an embedded DEL", "abc\x7fdef"},
 	}
 	for _, tc := range rejected {
 		t.Run(tc.name, func(t *testing.T) {
diff --git a/go/internal/runnerhub/integration_pgtest_test.go b/go/internal/runnerhub/integration_pgtest_test.go
index e88b5598..3e03a27b 100644
--- a/go/internal/runnerhub/integration_pgtest_test.go
+++ b/go/internal/runnerhub/integration_pgtest_test.go
@@ -462,9 +462,9 @@ func discardLog() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard,
 
 // agentNamePrefix is the container-name prefix this test wires into its
 // SpecDefaults, hoisted so shortRuntimeDir models the same name the Runner
-// actually builds (spec.go:73 joins it with the account id). Editing the prefix
-// in one place would otherwise silently shrink the modelled path and turn the
-// budget assertion into a false negative.
+// actually builds (BuildSpec in spec.go joins it with the account id). Editing
+// the prefix in one place would otherwise silently shrink the modelled path and
+// turn the budget assertion into a false negative.
 const agentNamePrefix = "compass-agent-"
 
 // accountIDHexLen is the width of a store account id: 16 random bytes
@@ -532,14 +532,15 @@ func runSessionsLoop(t *testing.T, ctx context.Context, cancel context.CancelFun
 // work dir, or macOS's ~49-byte /var/folders/<2>/<hash>/T) re-inflates it. So
 // the resulting path is asserted rather than assumed.
 //
-// This site FAILS rather than skips on an over-budget root, unlike
-// gateway/socket_test.go:381 which t.Skipf's the same condition: there the
-// environment merely blocks one unit assertion, whereas here a skip would
-// silently drop the only end-to-end coverage of the socket path. The cap is
-// derived the way the production guard derives it (gateway/socket.go:59), never
-// restated — sun_path is not one size across the platforms //go:build unix
-// admits (108 on linux/solaris/illumos, 104 on darwin and the BSDs, 1023 on
-// aix), and this file is //go:build unix.
+// This site FAILS rather than skips on an over-budget root: a skip would
+// silently drop the only end-to-end coverage of the socket path, reporting `ok`
+// for a test that asserted nothing. The gateway's padTo fails closed for the
+// same reason. The cap is derived the way the production guard derives it
+// (gateway.sunPathMax) rather than written down — sun_path is not one size
+// across the platforms //go:build unix admits (108 on linux/solaris/illumos,
+// 104 on darwin and the BSDs, 1023 on aix), and this file is //go:build unix.
+// The derivation is duplicated below because that constant is unexported; the
+// NUMBER is never restated.
 func shortRuntimeDir(t *testing.T) string {
 	t.Helper()
 	dir, err := os.MkdirTemp("", "cr") //nolint:usetesting // t.TempDir embeds the test name, which is what put this path over the sun_path cap — the bug this helper exists to prevent

From f9d1b22a7db6b8513632d8a30cf477a30e6d34b0 Mon Sep 17 00:00:00 2001
From: Matt Wilkinson <matt@sealedsecurity.com>
Date: Tue, 28 Jul 2026 00:31:22 -0400
Subject: [PATCH 4/5] fix(runner): state the measured reason for the
 control-character guard, and widen it to match
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review measured the claim in the previous commit and it was false. The
comment said a control character "fails much later, at bind, as a bare
EINVAL". Probed on Linux against listenAgentSocket's own ordering — MkdirAll
then bind — with the real path shape:

    "abc\x00def"   mkdir=EINVAL          bind=(never reached)
    "abc\ndef"     mkdir=ok              bind=ok
    "abc\x7fdef"   mkdir=ok              bind=ok
    "abc\u202edef" mkdir=ok              bind=ok
    "abc\u0085def" mkdir=ok              bind=ok

So the stated rationale was wrong-hop for the NUL (MkdirAll, not bind) and
simply false for the other rows, which bind cleanly. The guard was right and
its justification was not, which is the worse failure: an unsound reason
invites a later reader to delete a guard that is load-bearing for a reason
nobody wrote down.

The real reason is what those measurements show. Nothing downstream refuses
these characters, so the id reaches the container name and every log line
that quotes it. That name is what an operator reads out of `podman ps` and
the Runner's logs to identify an agent, so a character that forges a line
break or reorders the rendered name makes the identification unreliable.

Widening the predicate follows from stating the reason correctly. `r < 0x20
|| r == 0x7f` admits the C1 range and the bidi overrides — precisely the
characters that spoof a rendered name — while refusing DEL, which does not.
unicode.IsControl covers C0, DEL and C1; the format class is checked
alongside it because a bidi override is not a control character but spoofs
just as effectively. Two rows added for the cases the narrow predicate
admitted; narrowing it back reddens exactly those three.

Also from review: scope the pgtest helper's "never restated" claim to the
code, since the platform sizes quoted two lines above are numbers, and drop
the empty-string case from the fs.ValidPath enumeration, which describes a
callsite that cannot be reached (the empty id returns above with its own
message).

Refs SEA-1440, SEA-1443

Co-Authored-By: seal <noreply@sealedsecurity.com>
---
 go/internal/runner/spec.go                    | 28 +++++++++++++------
 go/internal/runner/spec_test.go               | 10 +++++--
 .../runnerhub/integration_pgtest_test.go      |  3 +-
 3 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/go/internal/runner/spec.go b/go/internal/runner/spec.go
index 3bfcc18c..3f6ea0e2 100644
--- a/go/internal/runner/spec.go
+++ b/go/internal/runner/spec.go
@@ -14,6 +14,7 @@ import (
 	"io/fs"
 	"net/url"
 	"strings"
+	"unicode"
 
 	compassv1 "github.com/sealedsecurity/compass/go/gen/compass/v1"
 	"github.com/sealedsecurity/compass/go/internal/runtime"
@@ -112,18 +113,27 @@ func validAccountID(id string) error {
 	if id == "" {
 		return errors.New("provision request requires an agent account id")
 	}
-	// fs.ValidPath rejects "", "..", any empty segment, a leading or trailing
-	// "/", and invalid UTF-8, but returns true for "." (documented) and for a
-	// legal multi-element path like "abc/def" — hence the other two conjuncts.
+	// fs.ValidPath rejects "..", any empty segment, a leading or trailing "/",
+	// and invalid UTF-8, but returns true for "." (documented) and for a legal
+	// multi-element path like "abc/def" — hence the other two conjuncts. (The
+	// empty id returned above, with its own message.)
 	if !fs.ValidPath(id) || id == "." || strings.Contains(id, "/") {
 		return fmt.Errorf("agent account id %q is not a valid path element", id)
 	}
-	// A control character survives the checks above (a NUL is valid UTF-8 and
-	// carries no separator) and fails much later, at bind, as a bare EINVAL —
-	// the same undiagnosable error the socket's length guard exists to replace.
-	// Refuse it here, where the message can name the input.
-	if strings.ContainsFunc(id, func(r rune) bool { return r < 0x20 || r == 0x7f }) {
-		return fmt.Errorf("agent account id %q contains a control character", id)
+	// A control or format character clears every check above — it is valid UTF-8
+	// and carries no separator — and nothing downstream stops it either.
+	// Measured on Linux against listenAgentSocket's own ordering: a newline, a
+	// DEL, a C1 control and a bidi override all pass MkdirAll AND bind, so the
+	// id reaches the container name and every log line that quotes it intact.
+	// (A NUL is the lone exception, failing at MkdirAll — one hop before the
+	// listener — as a bare EINVAL naming nothing.) The name is what an operator
+	// reads back out of `podman ps` and the Runner's logs to identify an agent,
+	// so a character that can forge a line break or reorder the display makes
+	// that identification unreliable. unicode.IsControl covers C0, DEL and C1;
+	// the format class (Cf) is checked with it because a bidi override is not a
+	// control character but spoofs a rendered name just as effectively.
+	if strings.ContainsFunc(id, func(r rune) bool { return unicode.IsControl(r) || unicode.Is(unicode.Cf, r) }) {
+		return fmt.Errorf("agent account id %q contains a control or format character", id)
 	}
 	return nil
 }
diff --git a/go/internal/runner/spec_test.go b/go/internal/runner/spec_test.go
index c9ecb280..f78d2d2d 100644
--- a/go/internal/runner/spec_test.go
+++ b/go/internal/runner/spec_test.go
@@ -200,12 +200,16 @@ func TestBuildSpecRejectsAgentAccountIDThatEscapesItsPathElement(t *testing.T) {
 		{"an embedded separator", "abc/def"},
 		{"a leading separator (absolute)", "/etc/passwd"},
 		{"a trailing separator", "abc/"},
-		// Control characters clear every check above — a NUL is valid UTF-8 and
-		// carries no separator — and would otherwise fail at bind as a bare
-		// EINVAL naming nothing.
+		// Control and format characters clear every check above and, measured,
+		// pass MkdirAll and bind too (a NUL is the exception, failing at
+		// MkdirAll) — so without this guard they reach the container name and
+		// the logs that quote it. The C1 and bidi rows are the ones a predicate
+		// written as `r < 0x20 || r == 0x7f` would silently admit.
 		{"an embedded NUL", "abc\x00def"},
 		{"an embedded newline", "abc\ndef"},
 		{"an embedded DEL", "abc\x7fdef"},
+		{"an embedded C1 control", "abc\u0085def"},
+		{"an embedded bidi override", "abc\u202edef"},
 	}
 	for _, tc := range rejected {
 		t.Run(tc.name, func(t *testing.T) {
diff --git a/go/internal/runnerhub/integration_pgtest_test.go b/go/internal/runnerhub/integration_pgtest_test.go
index 3e03a27b..b3aa413d 100644
--- a/go/internal/runnerhub/integration_pgtest_test.go
+++ b/go/internal/runnerhub/integration_pgtest_test.go
@@ -540,7 +540,8 @@ func runSessionsLoop(t *testing.T, ctx context.Context, cancel context.CancelFun
 // across the platforms //go:build unix admits (108 on linux/solaris/illumos,
 // 104 on darwin and the BSDs, 1023 on aix), and this file is //go:build unix.
 // The derivation is duplicated below because that constant is unexported; the
-// NUMBER is never restated.
+// code below derives the cap rather than writing a literal. (The sizes quoted
+// just above are for the reader, not values anything computes against.)
 func shortRuntimeDir(t *testing.T) string {
 	t.Helper()
 	dir, err := os.MkdirTemp("", "cr") //nolint:usetesting // t.TempDir embeds the test name, which is what put this path over the sun_path cap — the bug this helper exists to prevent

From c5915951fa6e92b67fae588c9d42bd87f1decd9b Mon Sep 17 00:00:00 2001
From: Matt Wilkinson <matt@sealedsecurity.com>
Date: Tue, 28 Jul 2026 00:45:09 -0400
Subject: [PATCH 5/5] test(runner): assert the RunSessions loop's error instead
 of discarding it

Both receives on the loopDone channel took the value and dropped it, so a
RunSessions stream failure ended the loop while assertCleanShutdown still
read as a clean teardown and the cleanup still read as a timely exit. The
error was the only evidence distinguishing "the ctx cancel ended it" from
"the stream died," and neither site looked at it.

Both now accept a nil or context.Canceled end and fail on anything else.
The predicate admits nil deliberately: probing the assertion by inverting
it shows the clean path delivers a nil, not a context.Canceled, so a
check for Canceled alone would redden every passing run.

Verified the assertion executes rather than assuming it: inverting the
guard reddens the test at integration_pgtest_test.go:169 with err=<nil>,
which is both the reachability proof and the measurement behind admitting
nil. Restored, pgtest green against live Postgres in 6.5s.

Refs SEA-1440, SEA-1443

Co-Authored-By: seal <noreply@sealedsecurity.com>
---
 .../runnerhub/integration_pgtest_test.go      | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/go/internal/runnerhub/integration_pgtest_test.go b/go/internal/runnerhub/integration_pgtest_test.go
index b3aa413d..79ae5ec6 100644
--- a/go/internal/runnerhub/integration_pgtest_test.go
+++ b/go/internal/runnerhub/integration_pgtest_test.go
@@ -22,6 +22,7 @@ package runnerhub_test
 
 import (
 	"context"
+	"errors"
 	"io"
 	"log/slog"
 	"net"
@@ -183,7 +184,13 @@ func assertCleanShutdown(t *testing.T, ctx context.Context, cancel context.Cance
 	engine.closeStdout()
 	cancel()
 	select {
-	case <-loopDone:
+	case err := <-loopDone:
+		// A cancelled ctx is the expected end; anything else is a real stream
+		// failure, and discarding it here would let the loop die of a genuine
+		// error while this assertion still reads as a clean shutdown.
+		if err != nil && !errors.Is(err, context.Canceled) {
+			t.Fatalf("RunSessions = %v, want a clean end after ctx cancel", err)
+		}
 	case <-time.After(integrationTimeout):
 		t.Fatal("RunSessions loop did not end after ctx cancel")
 	}
@@ -508,7 +515,15 @@ func runSessionsLoop(t *testing.T, ctx context.Context, cancel context.CancelFun
 	t.Cleanup(func() {
 		cancel()
 		select {
-		case <-loopDone:
+		case err := <-loopDone:
+			// On the clean path assertCleanShutdown already took the value and
+			// this reads the zero from a closed channel. On a failure path it
+			// takes the real one, and a stream error that killed the loop must
+			// surface here — otherwise the test reports only whatever Fatalf'd
+			// first, with the actual cause discarded.
+			if err != nil && !errors.Is(err, context.Canceled) {
+				t.Errorf("RunSessions ended with %v", err)
+			}
 		case <-time.After(integrationTimeout):
 			t.Errorf("RunSessions still running %s after cancel; the runtime dir removal below runs anyway and will race an in-flight Provision", integrationTimeout)
 		}