diff --git a/go/cmd/compass-runner/main.go b/go/cmd/compass-runner/main.go index 969093fe3..4b028ebc3 100644 --- a/go/cmd/compass-runner/main.go +++ b/go/cmd/compass-runner/main.go @@ -23,7 +23,6 @@ import ( "connectrpc.com/connect" - "github.com/RigelBuild/compass/go/internal/agentuid" "github.com/RigelBuild/compass/go/internal/otel" "github.com/RigelBuild/compass/go/internal/runner" "github.com/RigelBuild/compass/go/internal/runtime" @@ -145,15 +144,7 @@ func run() error { return err } } - specs, err := runner.NewConfigSpecBuilder(runner.SpecDefaults{ - Image: img, - Egress: egress, - CheckoutDir: *checkoutDir, - HomeDir: *homeDir, - UID: agentuid.AgentUID, - NamePrefix: runner.AgentContainerNamePrefix, - Mounts: mounts, - }) + specs, err := newSpecBuilder(engine, img, egress, *checkoutDir, *homeDir, mounts) if err != nil { return err } @@ -182,6 +173,28 @@ func run() error { }, specs, log) } +// newSpecBuilder assembles the config spec builder from the resolved engine and +// operator inputs. The uid every workspace runs as is resolved per backend by +// runner.ResolveWorkspaceUID (the host tier names its own euid; the container +// tiers keep the baked fleet constant); everything else is the operator's +// flags/env verbatim. The per-backend uid policy lives in internal/runner beside +// the non-root check that validates the result — this binary stays a thin wrapper. +func newSpecBuilder(engine runtime.WorkloadRuntime, image string, egress runtime.EgressPolicy, checkoutDir, homeDir string, mounts []runtime.Mount) (runner.SpecBuilder, error) { + uid, err := runner.ResolveWorkspaceUID(engine) + if err != nil { + return nil, err + } + return runner.NewConfigSpecBuilder(runner.SpecDefaults{ + Image: image, + Egress: egress, + CheckoutDir: checkoutDir, + HomeDir: homeDir, + UID: uid, + NamePrefix: runner.AgentContainerNamePrefix, + Mounts: mounts, + }) +} + // microVMPreflighter is the microVM backend's static host-capability probe: // the host trio (VMM/virtiofsd binaries), KVM access, and guest-image presence. type microVMPreflighter interface { diff --git a/go/cmd/compass-runner/uid_test.go b/go/cmd/compass-runner/uid_test.go new file mode 100644 index 000000000..837e41088 --- /dev/null +++ b/go/cmd/compass-runner/uid_test.go @@ -0,0 +1,48 @@ +//go:build unix + +package main + +import ( + "errors" + "strconv" + "testing" + + "github.com/RigelBuild/compass/go/internal/runner" + "github.com/RigelBuild/compass/go/internal/runtime" +) + +// TestHostDerivedUIDAcceptedByProvisionPath is the end-to-end regression this +// task exists to prevent: the uid the Runner derives for the host tier is +// exactly the uid the backend's AsUser rule accepts, so every provision-path +// exec (which passes Workspace.UID as AsUser) is accepted rather than rejected. +// +// The derivation and the AsUser rule now close structurally: ResolveWorkspaceUID +// returns HostRuntime.WorkspaceUID(), which reports the euid the backend captured +// at construction — the very same h.euid checkUser enforces. Both read one +// captured value, not two independent geteuid syscalls that agreed by accident. +// +// This box's euid is not asserted to differ from AgentUID, so it does not +// discriminate the "euid != 1000" regression on a 1000-box — that acceptance +// needs a box whose euid is not 1000 (see the report). What it proves here is +// the contract closure: derived uid == the uid checkUser trusts. +func TestHostDerivedUIDAcceptedByProvisionPath(t *testing.T) { + host := runtime.NewHostRuntime(t.TempDir()) + uid, err := runner.ResolveWorkspaceUID(host) + if err != nil { + t.Fatalf("ResolveWorkspaceUID(host) err = %v", err) + } + id, err := host.Create(t.Context(), runtime.WorkloadSpec{Name: "agent-provision"}) + if err != nil { + t.Fatalf("Create: %v", err) + } + if err := host.Start(t.Context(), id); err != nil { + t.Fatalf("Start: %v", err) + } + asUser := strconv.FormatUint(uint64(uid), 10) + if _, err := host.Exec(t.Context(), id, runtime.NewExecSpec("true").AsUser(asUser)); err != nil { + if _, ok := errors.AsType[*runtime.UnsupportedUserError](err); ok { + t.Fatalf("provision AsUser(%s) rejected by host backend: %v", asUser, err) + } + t.Fatalf("Exec AsUser(%s) = %v, want nil", asUser, err) + } +} diff --git a/go/internal/runner/resolve_uid_test.go b/go/internal/runner/resolve_uid_test.go new file mode 100644 index 000000000..a45af74b9 --- /dev/null +++ b/go/internal/runner/resolve_uid_test.go @@ -0,0 +1,106 @@ +//go:build unix + +package runner + +// ResolveWorkspaceUID: the per-backend uid policy that lives beside the non-root +// check. A backend that names its own uid (the host tier) wins; every container +// tier falls back to the baked AgentUID. These moved here from the cmd binary +// when the resolution moved off a concrete type assertion onto a capability +// probe on the runtime. + +import ( + "errors" + "strings" + "testing" + + "github.com/RigelBuild/compass/go/internal/agentuid" + "github.com/RigelBuild/compass/go/internal/runtime" +) + +// fixedUIDBackend is a WorkloadRuntime that implements the workspaceUIDResolver +// capability with a fixed answer, so the resolver's probe hit (and its error and +// root paths) are exercisable without a real host euid, which this box cannot set +// to 0 or -1. +type fixedUIDBackend struct { + *pipeRuntime + uid uint32 + err error +} + +func (b fixedUIDBackend) WorkspaceUID() (uint32, error) { return b.uid, b.err } + +// TestResolveWorkspaceUIDCapabilityWins: a backend implementing the capability +// has its WorkspaceUID honored, so the resolved uid is the backend's own value, +// never the baked fleet constant. The fake reports a uid distinct from AgentUID +// so the probe-over-default behavior discriminates — a resolver that ignored the +// capability and returned AgentUID would fail here. The real host backend proves +// the euid-capture closure end to end in the cmd package. +func TestResolveWorkspaceUIDCapabilityWins(t *testing.T) { + const backendUID = 4242 + uid, err := ResolveWorkspaceUID(fixedUIDBackend{pipeRuntime: newPipeRuntime(), uid: backendUID}) + if err != nil { + t.Fatalf("ResolveWorkspaceUID(capability backend) err = %v, want nil", err) + } + if uid != backendUID { + t.Fatalf("ResolveWorkspaceUID(capability backend) = %d, want the backend's uid %d", uid, backendUID) + } + if uid == agentuid.AgentUID { + t.Fatalf("ResolveWorkspaceUID = %d, must not be the baked fleet constant", uid) + } +} + +// TestResolveWorkspaceUIDContainerTiersKeepConstant: the podman/microvm/ +// apple-container tiers do not implement the capability, so the resolver falls +// back to AgentUID byte-identically — their userns remap maps the invoking host +// uid onto the baked 1000. +func TestResolveWorkspaceUIDContainerTiersKeepConstant(t *testing.T) { + engines := map[string]runtime.WorkloadRuntime{ + "podman": runtime.NewPodmanCLI(), + "microvm": runtime.NewMicroVMRuntime(runtime.MicroVMConfig{}), + "apple-container": runtime.NewAppleContainerCLI(runtime.AppleContainerConfig{}), + } + for name, engine := range engines { + t.Run(name, func(t *testing.T) { + uid, err := ResolveWorkspaceUID(engine) + if err != nil { + t.Fatalf("ResolveWorkspaceUID(%s) err = %v, want nil", name, err) + } + if uid != agentuid.AgentUID { + t.Fatalf("ResolveWorkspaceUID(%s) = %d, want AgentUID %d unchanged", name, uid, agentuid.AgentUID) + } + }) + } +} + +// TestResolveWorkspaceUIDPropagatesRefusal: a backend whose WorkspaceUID reports +// the no-syscall case (geteuid unavailable) is refused, not wrapped into a huge +// uid — the resolver returns the backend's error verbatim rather than swallowing +// it and defaulting to AgentUID. +func TestResolveWorkspaceUIDPropagatesRefusal(t *testing.T) { + refusal := errors.New("host backend: geteuid returned -1, no usable effective uid to run agents as") + _, err := ResolveWorkspaceUID(fixedUIDBackend{pipeRuntime: newPipeRuntime(), err: refusal}) + if !errors.Is(err, refusal) { + t.Fatalf("ResolveWorkspaceUID(refusing backend) err = %v, want the backend's refusal", err) + } +} + +// TestResolveWorkspaceUIDRootRefusedAtStartup: a backend resolving to uid 0 +// reaches the pre-existing non-root refusal in NewConfigSpecBuilder at startup, +// not at the first provision. The resolver itself returns 0 (root reaches the +// same check the container tiers do); the constructor is what refuses it. +func TestResolveWorkspaceUIDRootRefusedAtStartup(t *testing.T) { + uid, err := ResolveWorkspaceUID(fixedUIDBackend{pipeRuntime: newPipeRuntime(), uid: 0}) + if err != nil { + t.Fatalf("ResolveWorkspaceUID(uid=0) err = %v, want nil (the non-root check refuses it)", err) + } + if uid != 0 { + t.Fatalf("ResolveWorkspaceUID(uid=0) = %d, want 0", uid) + } + d := goodDefaults() + d.UID = uid + if _, err := NewConfigSpecBuilder(d); err == nil { + t.Fatal("NewConfigSpecBuilder with a root uid err = nil, want a startup refusal") + } else if !strings.Contains(err.Error(), "non-root") { + t.Fatalf("error %q is not the non-root refusal", err) + } +} diff --git a/go/internal/runner/spec.go b/go/internal/runner/spec.go index 99b53bb8b..560aa1eaa 100644 --- a/go/internal/runner/spec.go +++ b/go/internal/runner/spec.go @@ -13,6 +13,7 @@ import ( "strings" compassv1 "github.com/RigelBuild/compass/go/gen/compass/v1" + "github.com/RigelBuild/compass/go/internal/agentuid" "github.com/RigelBuild/compass/go/internal/runtime" ) @@ -74,6 +75,31 @@ func NewConfigSpecBuilder(defaults SpecDefaults) (SpecBuilder, error) { return &configSpecBuilder{defaults: defaults}, nil } +// workspaceUIDResolver is the backend capability of naming the uid its agent +// workspaces run as. The host backend implements it (it runs agents as direct +// children under the Runner's own euid, so the uid is that euid); the container +// tiers do not, because their userns remap maps the invoking host uid onto the +// baked fleet constant, so they need no per-Runner uid. +type workspaceUIDResolver interface { + WorkspaceUID() (uint32, error) +} + +// ResolveWorkspaceUID resolves the uid every agent workspace runs as, keyed off +// the resolved engine. A backend that names its own uid (the host tier) wins; +// every other backend falls back to agentuid.AgentUID. +// +// Unlike verifyBackendPreflight's fail-closed default, an unrecognized backend +// here is NOT an error: the container tiers legitimately do not implement this +// capability, so AgentUID is the correct, deliberate default for them — not an +// oversight. The result feeds NewConfigSpecBuilder's non-root check below, which +// refuses a root uid at startup whichever branch produced it. +func ResolveWorkspaceUID(engine runtime.WorkloadRuntime) (uint32, error) { + if r, ok := engine.(workspaceUIDResolver); ok { + return r.WorkspaceUID() + } + return agentuid.AgentUID, nil +} + // BuildSpec maps the request's agent account onto a full AgentSpec, filling // image/egress/workspace-layout from the defaults. func (b *configSpecBuilder) BuildSpec(req *compassv1.ProvisionAgentWorkspaceRequest) (runtime.AgentSpec, error) { diff --git a/go/internal/runtime/host_backend.go b/go/internal/runtime/host_backend.go index 6d5567cac..61b6d1569 100644 --- a/go/internal/runtime/host_backend.go +++ b/go/internal/runtime/host_backend.go @@ -137,6 +137,20 @@ func NewHostRuntime(stateRoot string) *HostRuntime { } } +// WorkspaceUID resolves the uid host-backend agents run as: the Runner's own +// effective uid captured at construction — the same h.euid checkUser enforces — +// so the uid handed to each workspace is exactly the uid its execs will be +// accepted under. os.Geteuid returns -1 where the syscall is unavailable; a +// blind uint32 conversion would wrap it to a huge uid, so the negative case is +// refused before any narrowing. The error wording is stable so the startup +// failure text does not regress. +func (h *HostRuntime) WorkspaceUID() (uint32, error) { + if h.euid < 0 { + return 0, fmt.Errorf("host backend: geteuid returned %d, no usable effective uid to run agents as", h.euid) + } + return uint32(h.euid), nil //nolint:gosec // G115: euid is non-negative here (the < 0 case returned above), so the narrowing cannot wrap. +} + // WithTimeout overrides the per-command wall-clock cap Exec applies. func (h *HostRuntime) WithTimeout(timeout time.Duration) *HostRuntime { h.timeout = timeout diff --git a/go/internal/runtime/host_backend_test.go b/go/internal/runtime/host_backend_test.go index ad2e14fc1..bf9f3b525 100644 --- a/go/internal/runtime/host_backend_test.go +++ b/go/internal/runtime/host_backend_test.go @@ -467,3 +467,37 @@ func TestHostExecStreamingRemovedDuringSpawn(t *testing.T) { t.Fatalf("err = %v, want the removed-during-spawn refusal", err) } } + +// TestHostWorkspaceUIDIsCapturedEuid: WorkspaceUID reports the euid captured at +// construction — the same value checkUser enforces — so the uid handed to a +// workspace is exactly the uid its execs are accepted under. Injects a known +// non-zero, non-AgentUID euid so the value discriminates a stubbed constant. +func TestHostWorkspaceUIDIsCapturedEuid(t *testing.T) { + const euid = 4242 + h := &HostRuntime{euid: euid} + uid, err := h.WorkspaceUID() + if err != nil { + t.Fatalf("WorkspaceUID() err = %v, want nil", err) + } + if uid != euid { + t.Fatalf("WorkspaceUID() = %d, want the captured euid %d", uid, euid) + } + // The uid must be exactly the value checkUser accepts. + if err := h.checkUser(new(strconv.FormatUint(uint64(uid), 10))); err != nil { + t.Fatalf("checkUser(%d) = %v, want the resolved uid accepted", uid, err) + } +} + +// TestHostWorkspaceUIDNegativeEuidRefused: os.Geteuid returns -1 where the +// syscall is unavailable; WorkspaceUID must refuse it rather than wrap it into a +// huge uint32, naming the offending euid so the startup failure is actionable. +func TestHostWorkspaceUIDNegativeEuidRefused(t *testing.T) { + h := &HostRuntime{euid: -1} + _, err := h.WorkspaceUID() + if err == nil { + t.Fatal("WorkspaceUID() with euid=-1 err = nil, want a refusal") + } + if !strings.Contains(err.Error(), "-1") { + t.Fatalf("error %q does not name the offending euid", err) + } +}