diff --git a/go/cmd/compass-runner/backend_flags_test.go b/go/cmd/compass-runner/backend_flags_test.go new file mode 100644 index 00000000..9c722308 --- /dev/null +++ b/go/cmd/compass-runner/backend_flags_test.go @@ -0,0 +1,235 @@ +//go:build unix + +package main + +// The operator-knob wiring for the microVM backend. Every knob here is dead +// code unless it reaches MicroVMConfig, and a knob that silently does not is the +// worst kind of gap: QuotaRequired was declared on the config, consumed by the +// D7 preflight, and unsettable by any operator — so the multi-tenant gate could +// never fire, on any profile. These tests assert BOTH resolution paths (the flag +// and the environment fallback) for the quota knobs, so that failure mode +// cannot recur silently. + +import ( + "strings" + "testing" + + "github.com/RigelBuild/compass/go/internal/runtime" +) + +// flagsFor builds a backendFlags whose pointers address local values, so a test +// sets a "flag" without touching the global flag set (which flag.Parse would +// own and which cannot be re-registered across tests). +func flagsFor() (backendFlags, *bool, *string) { + var ( + empty string + zero int + backend, vmm, virtiofsd, kernel, rootfs, initrd, manifest = empty, empty, empty, empty, empty, empty, empty + runRoot, volumeRoot = empty, empty + cpus, memoryMB = zero, zero + quotaRequired bool + ) + return backendFlags{ + backend: &backend, + vmm: &vmm, + virtiofsd: &virtiofsd, + kernel: &kernel, + rootfs: &rootfs, + initrd: &initrd, + imageManifest: &manifest, + runRoot: &runRoot, + volumeRoot: &volumeRoot, + cpus: &cpus, + memoryMB: &memoryMB, + quotaRequired: "aRequired, + }, "aRequired, &volumeRoot +} + +// TestQuotaRequiredReachesConfigFromFlag: --microvm-quota-required must land on +// MicroVMConfig.QuotaRequired. Without this the D7 preflight gate is +// unreachable dead code and every Runner runs fail-open. +func TestQuotaRequiredReachesConfigFromFlag(t *testing.T) { + f, quotaRequired, _ := flagsFor() + *quotaRequired = true + + cfg, err := f.backendConfig() + if err != nil { + t.Fatalf("backendConfig() = %v, want nil", err) + } + if !cfg.MicroVM.QuotaRequired { + t.Fatal("--microvm-quota-required did not reach MicroVMConfig.QuotaRequired; " + + "the D7 multi-tenant quota gate would be unsettable and could never fire") + } +} + +// TestQuotaRequiredReachesConfigFromEnv: the documented +// $COMPASS_MICROVM_QUOTA_REQUIRED fallback must resolve too — it is how the +// managed profile sets the knob, since it ships env, not argv. +func TestQuotaRequiredReachesConfigFromEnv(t *testing.T) { + for _, raw := range []string{"true", "1", "TRUE"} { + t.Run(raw, func(t *testing.T) { + t.Setenv("COMPASS_MICROVM_QUOTA_REQUIRED", raw) + f, _, _ := flagsFor() + + cfg, err := f.backendConfig() + if err != nil { + t.Fatalf("backendConfig() = %v, want nil", err) + } + if !cfg.MicroVM.QuotaRequired { + t.Fatalf("$COMPASS_MICROVM_QUOTA_REQUIRED=%q did not reach MicroVMConfig.QuotaRequired", raw) + } + }) + } +} + +// TestQuotaRequiredDefaultsOff pins the transitional default: neither knob set +// means the Dogfood single-tenant posture, where an absent quota is documented +// and logged rather than fatal. +func TestQuotaRequiredDefaultsOff(t *testing.T) { + t.Setenv("COMPASS_MICROVM_QUOTA_REQUIRED", "") + f, _, _ := flagsFor() + + cfg, err := f.backendConfig() + if err != nil { + t.Fatalf("backendConfig() = %v, want nil", err) + } + if cfg.MicroVM.QuotaRequired { + t.Fatal("QuotaRequired defaulted to true with no knob set; the transitional default is the single-tenant posture") + } +} + +// TestQuotaRequiredEnvRefusesGarbage: an unparseable value must REFUSE at +// startup naming the variable, never read as false. Silently defaulting a +// misspelled `=yes` to off is exactly the fail-open this knob exists to close. +func TestQuotaRequiredEnvRefusesGarbage(t *testing.T) { + t.Setenv("COMPASS_MICROVM_QUOTA_REQUIRED", "yes-please") + f, _, _ := flagsFor() + + _, err := f.backendConfig() + if err == nil { + t.Fatal("an unparseable $COMPASS_MICROVM_QUOTA_REQUIRED = nil error; a typo must refuse at startup, not read as false") + } + for _, part := range []string{"COMPASS_MICROVM_QUOTA_REQUIRED", "yes-please"} { + if !strings.Contains(err.Error(), part) { + t.Errorf("error %q does not name %q", err.Error(), part) + } + } +} + +// TestQuotaRequiredFlagWinsOverEnv pins the precedence every other knob has +// (orEnv/intOrEnv): an explicitly-passed flag beats the environment fallback. +func TestQuotaRequiredFlagWinsOverEnv(t *testing.T) { + t.Setenv("COMPASS_MICROVM_QUOTA_REQUIRED", "false") + f, quotaRequired, _ := flagsFor() + *quotaRequired = true + + cfg, err := f.backendConfig() + if err != nil { + t.Fatalf("backendConfig() = %v, want nil", err) + } + if !cfg.MicroVM.QuotaRequired { + t.Fatal("--microvm-quota-required=true lost to $COMPASS_MICROVM_QUOTA_REQUIRED=false; the flag must win") + } +} + +// TestQuotaRequiredEnvTrueBeatsExplicitFlagFalse pins the OTHER direction of +// boolOrEnv's precedence, which its sibling above does not cover: an env true +// beats an explicitly-passed `--microvm-quota-required=false`. +// +// This is a DOCUMENTED CONTRACT, not a latent bug (boolOrEnv's doc). A false +// flag is indistinguishable from an unset one under the zero-value convention +// every knob here shares, and the asymmetry resolves in the SAFE direction: the +// input that loses is the one that would turn the multi-tenant quota gate OFF, +// so a conflicting pair leaves the gate ON. Pinning it means a future switch to +// flag.Visit — which WOULD honor the explicit false and thereby move the +// conflict resolution to fail-open — is a deliberate, reviewed change rather +// than an unnoticed regression. +func TestQuotaRequiredEnvTrueBeatsExplicitFlagFalse(t *testing.T) { + t.Setenv("COMPASS_MICROVM_QUOTA_REQUIRED", "true") + f, quotaRequired, _ := flagsFor() + // Explicitly false — which, under the zero-value convention, is + // indistinguishable from "not passed". + *quotaRequired = false + + cfg, err := f.backendConfig() + if err != nil { + t.Fatalf("backendConfig() = %v, want nil", err) + } + if !cfg.MicroVM.QuotaRequired { + t.Fatal("$COMPASS_MICROVM_QUOTA_REQUIRED=true lost to an explicit --microvm-quota-required=false; " + + "the documented contract is that env-true wins (a false flag is indistinguishable from an unset " + + "one, and the conflict must resolve with the multi-tenant gate ON, not off). To disable the gate " + + "on such a box, unset the environment variable") + } +} + +// TestVolumeRootReachesConfig: --microvm-volume-root / its env fallback must +// land on MicroVMConfig.VolumeRoot. It is what makes the quota probe target the +// session-volume filesystem instead of the RunRoot socket dir, so an unwired +// knob would leave the preflight either fail-closed forever or (worse) reporting +// a verdict about the wrong mount. +func TestVolumeRootReachesConfig(t *testing.T) { + t.Run("flag", func(t *testing.T) { + f, _, volumeRoot := flagsFor() + *volumeRoot = "/srv/compass/volumes" + + cfg, err := f.backendConfig() + if err != nil { + t.Fatalf("backendConfig() = %v, want nil", err) + } + if cfg.MicroVM.VolumeRoot != "/srv/compass/volumes" { + t.Fatalf("VolumeRoot = %q, want the flag value", cfg.MicroVM.VolumeRoot) + } + }) + t.Run("env", func(t *testing.T) { + t.Setenv("COMPASS_MICROVM_VOLUME_ROOT", "/mnt/volumes") + f, _, _ := flagsFor() + + cfg, err := f.backendConfig() + if err != nil { + t.Fatalf("backendConfig() = %v, want nil", err) + } + if cfg.MicroVM.VolumeRoot != "/mnt/volumes" { + t.Fatalf("VolumeRoot = %q, want the env value", cfg.MicroVM.VolumeRoot) + } + }) +} + +// TestVolumeRootIsNotTheRunRoot: the two knobs must resolve INDEPENDENTLY. A +// wiring that aliased VolumeRoot to RunRoot would reintroduce the exact bug the +// field exists to fix — a quota verdict read on the short /tmp socket dir rather +// than the durable session-volume filesystem. +func TestVolumeRootIsNotTheRunRoot(t *testing.T) { + t.Setenv("COMPASS_MICROVM_RUNROOT", "/tmp/cvm") + t.Setenv("COMPASS_MICROVM_VOLUME_ROOT", "/srv/compass/volumes") + f, _, _ := flagsFor() + + cfg, err := f.backendConfig() + if err != nil { + t.Fatalf("backendConfig() = %v, want nil", err) + } + if cfg.MicroVM.RunRoot != "/tmp/cvm" { + t.Errorf("RunRoot = %q, want /tmp/cvm", cfg.MicroVM.RunRoot) + } + if cfg.MicroVM.VolumeRoot != "/srv/compass/volumes" { + t.Errorf("VolumeRoot = %q, want /srv/compass/volumes", cfg.MicroVM.VolumeRoot) + } +} + +// TestBackendConfigSelectsMicroVM is the end-to-end shape: the resolved config +// still drives backend selection, so the backendConfig split did not detach the +// knobs from the engine they configure. +func TestBackendConfigSelectsMicroVM(t *testing.T) { + f, quotaRequired, volumeRoot := flagsFor() + *f.backend = "microvm" + *quotaRequired = true + *volumeRoot = "/srv/compass/volumes" + + engine, err := f.selectEngine() + if err != nil { + t.Fatalf("selectEngine() = %v, want the microVM backend", err) + } + if _, ok := engine.(*runtime.MicroVMRuntime); !ok { + t.Fatalf("selectEngine() = %T, want *runtime.MicroVMRuntime", engine) + } +} diff --git a/go/cmd/compass-runner/main.go b/go/cmd/compass-runner/main.go index 1cec32c3..77d0d0ff 100644 --- a/go/cmd/compass-runner/main.go +++ b/go/cmd/compass-runner/main.go @@ -283,7 +283,9 @@ func setupOtel(ctx context.Context) (shutdown func(), err error) { // default flag set before flag.Parse and resolved into a runtime after it. type backendFlags struct { backend, vmm, virtiofsd, kernel, rootfs, initrd, imageManifest, runRoot *string + volumeRoot *string cpus, memoryMB *int + quotaRequired *bool } // registerBackendFlags declares the backend-selection flags. Call before @@ -315,21 +317,46 @@ func registerBackendFlags() backendFlags { memoryMB: flag.Int("microvm-memory-mb", 0, "Default guest RAM in MiB per session (microvm backend); 0 leaves the VMM default. "+ "Defaults to $COMPASS_MICROVM_MEMORY_MB."), + volumeRoot: flag.String("microvm-volume-root", "", + "Parent dir the per-session workspace volumes are minted under (microvm backend); "+ + "the filesystem the D7 session-volume quota is verified on. "+ + "Defaults to $COMPASS_MICROVM_VOLUME_ROOT."), + quotaRequired: flag.Bool("microvm-quota-required", false, + "Require an operator-provisioned project quota on the session-volume filesystem "+ + "(multi-tenant profile); startup fails when none is active (microvm backend). "+ + "Defaults to $COMPASS_MICROVM_QUOTA_REQUIRED."), } } // selectEngine resolves the configured runtime backend from the parsed flags // and their environment fallbacks. func (f backendFlags) selectEngine() (runtime.ContainerRuntime, error) { - cpus, err := intOrEnv(*f.cpus, "COMPASS_MICROVM_CPUS") + cfg, err := f.backendConfig() if err != nil { return nil, err } + return runtime.SelectBackend(cfg) +} + +// backendConfig resolves every backend knob from its flag with an environment +// fallback. Split from selectEngine so the resolution is assertable without +// constructing a real backend: a knob that never reaches the config is dead +// code an operator cannot set (the QuotaRequired fail-open), and only a test +// over THIS function catches that. +func (f backendFlags) backendConfig() (runtime.BackendConfig, error) { + cpus, err := intOrEnv(*f.cpus, "COMPASS_MICROVM_CPUS") + if err != nil { + return runtime.BackendConfig{}, err + } memoryMB, err := intOrEnv(*f.memoryMB, "COMPASS_MICROVM_MEMORY_MB") if err != nil { - return nil, err + return runtime.BackendConfig{}, err } - return runtime.SelectBackend(runtime.BackendConfig{ + quotaRequired, err := boolOrEnv(*f.quotaRequired, "COMPASS_MICROVM_QUOTA_REQUIRED") + if err != nil { + return runtime.BackendConfig{}, err + } + return runtime.BackendConfig{ Backend: orEnv(*f.backend, "COMPASS_RUNTIME_BACKEND"), MicroVM: runtime.MicroVMConfig{ VMMPath: orEnv(*f.vmm, "COMPASS_MICROVM_VMM"), @@ -339,10 +366,12 @@ func (f backendFlags) selectEngine() (runtime.ContainerRuntime, error) { InitrdImage: orEnv(*f.initrd, "COMPASS_MICROVM_INITRD"), ImageManifest: orEnv(*f.imageManifest, "COMPASS_MICROVM_IMAGE_MANIFEST"), RunRoot: orEnv(*f.runRoot, "COMPASS_MICROVM_RUNROOT"), + VolumeRoot: orEnv(*f.volumeRoot, "COMPASS_MICROVM_VOLUME_ROOT"), DefaultCPUs: cpus, DefaultMemoryMB: memoryMB, + QuotaRequired: quotaRequired, }, - }) + }, nil } // orEnv returns flagVal when non-empty, else the named environment variable. @@ -373,6 +402,45 @@ func intOrEnv(flagVal int, envKey string) (int, error) { return parsed, nil } +// boolOrEnv returns flagVal when it is true, else the named environment +// variable parsed as a bool. The flag defaults to false, so a set flag always +// wins and an unset one falls through to the env — the same precedence orEnv +// and intOrEnv give their zero values. An empty env var is false (unset); a +// present-but-unparseable one is an error naming the offending variable and +// value, so `COMPASS_MICROVM_QUOTA_REQUIRED=yes` refuses at startup instead of +// silently reading as false and fail-opening the D7 gate. +// +// PRECEDENCE IS ASYMMETRIC, deliberately, and it is a CONTRACT, not an +// accident: a false flag is indistinguishable from an unset one here (there is +// no flag.Visit and no *bool nil sentinel), so `--microvm-quota-required=false` +// does NOT override `$COMPASS_MICROVM_QUOTA_REQUIRED=true` — the env's true +// wins. Two reasons to keep it that way rather than honoring an explicit false: +// +// - It matches the zero-value convention of every sibling knob (orEnv, +// intOrEnv), so one knob does not read its flags differently from the rest. +// - It fails in the SAFE direction for THIS knob: the losing input is the one +// that would turn the multi-tenant quota gate OFF, so a conflict leaves the +// gate ON. +// +// An operator who genuinely needs the gate off on a box whose environment sets +// it must unset the environment variable; passing the flag is not enough. +// TestQuotaRequiredEnvTrueBeatsExplicitFlagFalse pins this direction so the +// asymmetry cannot be silently "fixed" into the fail-open direction. +func boolOrEnv(flagVal bool, envKey string) (bool, error) { + if flagVal { + return true, nil + } + raw := os.Getenv(envKey) + if raw == "" { + return false, nil + } + parsed, err := strconv.ParseBool(raw) + if err != nil { + return false, fmt.Errorf("$%s=%q is not a boolean (want true/false/1/0): %w", envKey, raw, err) + } + return parsed, nil +} + // parseEgress parses the comma-separated allowlist into a validated EgressPolicy. // An empty list is a valid default-deny policy (no host reachable). func parseEgress(csv string) (runtime.EgressPolicy, error) { diff --git a/go/internal/runner/e2e_vsock_gateway_microvm_test.go b/go/internal/runner/e2e_vsock_gateway_microvm_test.go index 5de32ac6..f4918ae5 100644 --- a/go/internal/runner/e2e_vsock_gateway_microvm_test.go +++ b/go/internal/runner/e2e_vsock_gateway_microvm_test.go @@ -48,6 +48,7 @@ import ( "errors" "fmt" "os" + "strconv" "strings" "testing" "time" @@ -55,6 +56,7 @@ import ( "connectrpc.com/connect" compassv1 "github.com/RigelBuild/compass/go/gen/compass/v1" + "github.com/RigelBuild/compass/go/internal/agentuid" compassv1internal "github.com/RigelBuild/compass/go/internal/gen/compass/v1" "github.com/RigelBuild/compass/go/internal/microvmtest" "github.com/RigelBuild/compass/go/internal/runnertest" @@ -175,7 +177,7 @@ func w3LiveSpec(t *testing.T) runtime.AgentSpec { Workspace: runtime.Workspace{ CheckoutDir: "/workspace", HomeDir: "/workspace", - UID: 1000, + UID: agentuid.AgentUID, }, Mounts: []runtime.Mount{{HostPath: t.TempDir(), ContainerPath: "/workspace"}}, Egress: runtime.EgressPolicy{}, @@ -453,7 +455,7 @@ func runInGuestProbe(t *testing.T, m *runtime.MicroVMRuntime, id runtime.Contain t.Fatalf("marshaling probe request: %v", err) } script := inGuestProbeScript(string(reqBody)) - out, err := m.Exec(ctx, id, runtime.NewExecSpec("bun", "-e", script).AsUser("1000")) + out, err := m.Exec(ctx, id, runtime.NewExecSpec("bun", "-e", script).AsUser(strconv.Itoa(int(agentuid.AgentUID)))) if err != nil { t.Fatalf("in-guest bun probe exec errored (harness fault, not a round-trip verdict): %v", err) } @@ -529,7 +531,7 @@ func inGuestCanReachIP(t *testing.T, m *runtime.MicroVMRuntime, id runtime.Conta ctx, cancel := context.WithTimeout(t.Context(), inGuestProbeTimeout) defer cancel() script := fmt.Sprintf("timeout %d bash -c 'exec 3<>/dev/tcp/%s/443 && echo connected'", guestConnectTimeoutSecs, ip) - out, err := m.Exec(ctx, id, runtime.NewExecSpec("sh", "-c", script).AsUser("1000")) + out, err := m.Exec(ctx, id, runtime.NewExecSpec("sh", "-c", script).AsUser(strconv.Itoa(int(agentuid.AgentUID)))) if err != nil { t.Fatalf("in-guest IP connect probe to %s errored (harness fault, not a firewall verdict): %v", ip, err) } diff --git a/go/internal/runtime/egress_inguest_microvm_test.go b/go/internal/runtime/egress_inguest_microvm_test.go index 15b639a5..558aecf5 100644 --- a/go/internal/runtime/egress_inguest_microvm_test.go +++ b/go/internal/runtime/egress_inguest_microvm_test.go @@ -43,10 +43,12 @@ package runtime import ( "context" + "strconv" "strings" "testing" "time" + "github.com/RigelBuild/compass/go/internal/agentuid" "github.com/RigelBuild/compass/go/internal/microvmtest" ) @@ -84,7 +86,7 @@ func startEgressSession(t *testing.T, egress EgressPolicy, name string) (*MicroV workspace := t.TempDir() id, err := m.Create(t.Context(), ContainerSpec{ Name: name, - UID: 1000, + UID: agentuid.AgentUID, Egress: egress, Mounts: []Mount{{HostPath: workspace, ContainerPath: "/workspace"}}, }) @@ -123,7 +125,7 @@ func canReachIPv4(t *testing.T, m *MicroVMRuntime, id ContainerID, ip string) bo defer cancel() script := "timeout " + guestConnectTimeout + " bash -c 'exec 3<>/dev/tcp/" + ip + "/443 && echo connected'" - out, err := m.Exec(ctx, id, NewExecSpec("sh", "-c", script).AsUser("1000")) + out, err := m.Exec(ctx, id, NewExecSpec("sh", "-c", script).AsUser(strconv.Itoa(int(agentuid.AgentUID)))) if err != nil { t.Fatalf("in-guest connect probe to %s errored (harness fault, not a firewall verdict): %v", ip, err) } @@ -166,7 +168,7 @@ func TestInGuestEgressAgentCannotAlterRuleset(t *testing.T) { // failed command (guest exec model), never a transport error. ctx, cancel := context.WithTimeout(t.Context(), egressProbeTimeout) defer cancel() - flush, err := m.Exec(ctx, id, NewExecSpec("nft", "flush", "ruleset").AsUser("1000")) + flush, err := m.Exec(ctx, id, NewExecSpec("nft", "flush", "ruleset").AsUser(strconv.Itoa(int(agentuid.AgentUID)))) if err != nil { t.Fatalf("nft flush probe errored (harness fault, not a firewall verdict): %v", err) } diff --git a/go/internal/runtime/microvm.go b/go/internal/runtime/microvm.go index e845c247..9a5735aa 100644 --- a/go/internal/runtime/microvm.go +++ b/go/internal/runtime/microvm.go @@ -55,6 +55,25 @@ type MicroVMConfig struct { // DefaultMemoryMB is the RAM each session guest boots with, in MiB // (hotplug-grown later per D5). Zero leaves it to the VMM's own default. DefaultMemoryMB int + // QuotaRequired selects the D7 quota posture. When set (the multi-tenant + // profile), Runner startup verifies an operator-provisioned project quota is + // active on the session-volume filesystem and fails if absent (D7 + // verify-never-assign: the Runner reads the bound, it never assigns one — + // assignment needs CAP_SYS_ADMIN it lacks). Unset (Dogfood single-tenant): + // no host-enforced quota is required, and the observed utilization is logged + // only. Appended, never reordered: the field is additive to a config + // operators already construct positionally in tests. + QuotaRequired bool + // VolumeRoot is the parent dir the per-session workspace volumes are minted + // under (P2's volume lifecycle), i.e. a startup-known path ON THE + // SESSION-VOLUME FILESYSTEM. It exists solely so the D7 quota check probes + // the filesystem sessions actually consume: RunRoot is the socket dir, held + // to a SHORT /tmp path by the AF_UNIX sun_path budget, so it is routinely a + // different filesystem entirely and a verdict read there says nothing about + // the durable volume. When QuotaRequired is set and this is empty the + // preflight fails closed rather than reporting a verdict about a filesystem + // it never probed. Appended, never reordered. + VolumeRoot string } // BackendConfig selects and configures the container runtime backend. Backend diff --git a/go/internal/runtime/microvm/config.go b/go/internal/runtime/microvm/config.go index e76ee0c9..d73c6f54 100644 --- a/go/internal/runtime/microvm/config.go +++ b/go/internal/runtime/microvm/config.go @@ -32,9 +32,17 @@ type BootConfig struct { FSTag string // virtio-fs tag ("workspace") FSSocket string // virtiofsd --socket-path the VMM attaches via --fs FSSharedDir string // virtiofsd --shared-dir: the host tree exported as FSTag→/workspace (per-session checkout dir in V2b; a throwaway temp dir in the V2a spike) - CPUs int - MemoryMB int // always launched with shared=on ((c)) - Net NetConfig + // AgentUID is the in-guest uid the workload runs as (uid==gid, guestd's + // linuxCredential). virtiofsd maps it back to the INVOKING host user's + // (uid,gid) inside its user namespace, so a file the guest agent writes on + // the share lands host-side owned by the invoking user — the same host + // ownership podman's `--userns=keep-id:uid=N,gid=N` produces (record §(d), + // host-ownership parity). Zero disables the mapping (the V2a spike harness, + // which shares a throwaway dir and asserts nothing about ownership). + AgentUID uint32 + CPUs int + MemoryMB int // always launched with shared=on ((c)) + Net NetConfig } // GatewaySocketPath is the host-side AF_UNIX listener path a guest reaches by diff --git a/go/internal/runtime/microvm/launch.go b/go/internal/runtime/microvm/launch.go index 102f9d9a..e09bcf31 100644 --- a/go/internal/runtime/microvm/launch.go +++ b/go/internal/runtime/microvm/launch.go @@ -13,7 +13,6 @@ import ( "strconv" "strings" "sync" - "sync/atomic" "syscall" "time" @@ -52,6 +51,16 @@ const reapGrace = 5 * time.Second // boot log into the test output. const diagnosticTailBytes = 8 << 10 +// modcapsDropMknod is the exact --modcaps literal virtiofsd is launched with, +// pinned in ONE place because virtiofsd does NOT validate capability names. +// Verified by execution: `--modcaps=-not_a_real_cap` and `--modcaps=-bogus_cap` +// are both accepted silently and the daemon starts normally. So a typo or an +// upstream rename turns the flag into a no-op with no diagnostic anywhere, and +// the argv assertion in launch_idmap_test.go is the ONLY guard — which is why +// the literal lives here and both the launch path and that test reference this +// const, so the two cannot drift apart into a mutually-agreeing typo. +const modcapsDropMknod = "--modcaps=-mknod" + // launchOptions selects which virtio devices cloud-hypervisor is booted with. // The full boot wires every device; the OQ-G net-only smoke omits virtio-fs and // vsock so the passt×CH vhost-user-net negotiation — the spike's primary @@ -70,7 +79,31 @@ type child struct { name string cmd *exec.Cmd logPath string - waited atomic.Bool // set once cmd.Wait has returned, so liveness probes and PSS skip a reaped process + + // exited is closed by this child's SOLE reaper, started by startChild once + // Start has succeeded. It carries the ONE cmd.Wait per child: the readiness + // poll's liveness check (waitForSockets), Shutdown's reap, Running and PSS + // all observe the exit THROUGH this channel instead of calling Wait + // themselves, so no two paths ever race on the same process. Nil exactly + // when the process was never started (cmd.Process is nil too). + exited chan struct{} + // waitErr is cmd.Wait's error, written by the reaper BEFORE it closes + // exited. Reading it is only safe after a receive from exited, which is the + // happens-before edge that makes the unsynchronized field race-free. + waitErr error +} + +// hasExited reports whether the child's reaper has already observed its exit, +// without blocking. It is NOT a signal-0 probe: an exited-but-unwaited process +// is a zombie that Signal(0) reports as alive, which is precisely why the +// liveness check reads the reaper's channel instead. +func (c *child) hasExited() bool { + select { + case <-c.exited: + return true + default: + return false + } } // VM is a running (or partially-started, on the Launch error path) guest and @@ -83,10 +116,10 @@ type VM struct { virtiofsd *child // nil under the net-only smoke (no --fs) passt *child - // vmmExited is closed by the sole VMM reaper (started in launch) once the - // cloud-hypervisor process has been Wait'd, so the caller can observe a - // prompt guest self-power-off instead of a zombie-blind Signal(0) poll. Nil - // only under the hermetic fail-closed path (no VMM on PATH). + // vmmExited is the VMM child's reaper channel (vm.vmm.exited), hoisted onto + // the VM so WaitVMMExit can observe a prompt guest self-power-off instead of + // polling a zombie. Nil only under the hermetic fail-closed path (no VMM on + // PATH), where no VMM child was ever started. vmmExited chan struct{} consolePath string // --serial file: the guest serial console @@ -156,14 +189,28 @@ func launch(ctx context.Context, cfg BootConfig, opts launchOptions) (_ *VM, err if lookErr != nil { return nil, fmt.Errorf("microvm: resolving virtiofsd on PATH: %w", lookErr) } + // Both subordinate bases are READ, never assumed and never shared: + // newuidmap validates the requested host uid range against /etc/subuid + // and newgidmap validates the gid range against /etc/subgid, and those + // are INDEPENDENT allocations (subuid.go). Reusing the uid base for the + // gid map boots fine on a shadow-utils-default box and dies on a + // divergent one — after virtiofsd has already bound its socket, which is + // why waitForSockets below is liveness-aware. VerifySubordinateIDRange + // runs the same two reads at startup so this error is rare by + // construction. + subUIDBase, subGIDBase := 0, 0 + if cfg.AgentUID != 0 { + uidBase, gidBase, subErr := SubordinateIDBases() + if subErr != nil { + return nil, subErr + } + subUIDBase, subGIDBase = uidBase, gidBase + } vm.virtiofsd = &child{ name: "virtiofsd", logPath: filepath.Join(dir, "virtiofsd.log"), //nolint:gosec // G204: the microVM harness seam — virtiofsdPath is LookPath-resolved and the argv is harness-built from BootConfig, neither user-controlled - cmd: exec.CommandContext(ctx, virtiofsdPath, - "--socket-path="+cfg.FSSocket, - "--shared-dir="+cfg.FSSharedDir, - "--sandbox=namespace"), + cmd: exec.CommandContext(ctx, virtiofsdPath, virtiofsdArgs(cfg, subUIDBase, subGIDBase)...), } if startErr := startChild(vm.virtiofsd); startErr != nil { return nil, fmt.Errorf("microvm: starting virtiofsd: %w", startErr) @@ -200,14 +247,30 @@ func launch(ctx context.Context, cfg BootConfig, opts launchOptions) (_ *VM, err vm.sockets = append(vm.sockets, cfg.Net.VhostUserSocket) // virtiofsd + passt must be serving before cloud-hypervisor connects to - // their sockets. Bounded poll, not a fixed sleep. + // their sockets. Bounded poll, not a fixed sleep — and LIVENESS-AWARE, not + // merely path-existence: virtiofsd binds its AF_UNIX socket BEFORE the + // id-map setup that can fail, so a mapping failure leaves the socket on + // disk behind a dead daemon. A path-only poll would return nil there and + // launch cloud-hypervisor against a corpse, surfacing as an inscrutable + // vhost-user negotiation error instead of virtiofsd's own "couldn't setup + // id mappings" line. + waiting := []*child{vm.passt} ready := []string{cfg.Net.VhostUserSocket} if opts.withFS { + waiting = append(waiting, vm.virtiofsd) ready = append(ready, cfg.FSSocket) } - if waitErr := waitForSockets(ctx, ready, socketReadyTimeout); waitErr != nil { + if waitErr := waitForSockets(ctx, ready, waiting, socketReadyTimeout); waitErr != nil { return nil, fmt.Errorf("microvm: waiting for daemon sockets: %w", waitErr) } + // Belt-and-braces over the poll above: a daemon that dies in the window + // between the last poll iteration and here must not be handed to the VMM. + for _, c := range waiting { + if c.hasExited() { + return nil, fmt.Errorf("microvm: %s exited before cloud-hypervisor was started: %w; log tail:\n%s", + c.name, waitResult(c.name, c.waitErr), tailFile(c.logPath)) + } + } vmmPath, lookErr := exec.LookPath("cloud-hypervisor") if lookErr != nil { @@ -222,22 +285,128 @@ func launch(ctx context.Context, cfg BootConfig, opts launchOptions) (_ *VM, err if startErr := startChild(vm.vmm); startErr != nil { return nil, fmt.Errorf("microvm: starting cloud-hypervisor: %w", startErr) } - // The sole VMM reaper owns the single cmd.Wait for cloud-hypervisor: it - // unblocks WaitVMMExit on a guest self-power-off and lets Shutdown observe - // the exit without a second Wait. The Wait error is deliberately discarded — - // a killed VMM yields an expected *exec.ExitError, mirroring waitResult. - vm.vmmExited = make(chan struct{}) - go func() { - _ = vm.vmm.cmd.Wait() // discard: a killed VMM's *exec.ExitError is the expected teardown outcome (mirrors waitResult) - vm.vmm.waited.Store(true) - close(vm.vmmExited) - }() + // startChild installed the sole reaper that owns cloud-hypervisor's single + // cmd.Wait; hoist its channel onto the VM so WaitVMMExit observes a guest + // self-power-off promptly instead of polling a zombie. + vm.vmmExited = vm.vmm.exited if opts.withVsock { vm.sockets = append(vm.sockets, cfg.VsockSocket) } return vm, nil } +// virtiofsdArgs builds the whole virtiofsd argv: the socket/share/sandbox flags +// every boot carries, the capability trim, and the id mapping (empty under the +// V2a spike, see virtiofsdIDMapArgs). subUIDBase and subGIDBase are the host +// subordinate ids mapped to namespace-uid 0 and namespace-gid 0, read from +// /etc/subuid and /etc/subgid INDEPENDENTLY by the caller (subuid.go on why +// they must not be one value); both are ignored when cfg.AgentUID is zero. +// +// modcapsDropMknod drops CAP_MKNOD from the capability set virtiofsd retains +// under --sandbox=namespace. A workspace share has no legitimate use for device +// nodes — the agent checks out source and writes build output — so the +// capability is pure escape surface, and dropping it is free (see +// virtiofsdIDMapArgs on the rest of the retained set). The literal is a const +// shared with the test because virtiofsd silently ACCEPTS unknown capability +// names, so the argv assertion is the only guard against a typo — see +// modcapsDropMknod's own doc. +func virtiofsdArgs(cfg BootConfig, subUIDBase, subGIDBase int) []string { + idMap := virtiofsdIDMapArgs(cfg.AgentUID, subUIDBase, subGIDBase) + args := make([]string, 0, 4+len(idMap)) + args = append(args, + "--socket-path="+cfg.FSSocket, + "--shared-dir="+cfg.FSSharedDir, + "--sandbox=namespace", + modcapsDropMknod, + ) + return append(args, idMap...) +} + +// virtiofsdIDMapArgs builds the virtiofsd uid/gid mapping that gives the shared +// volume the SAME host-side ownership podman's `--userns=keep-id:uid=N,gid=N` +// produces (record §(d): "virtiofsd does its own uid/gid translation via that +// userns (subuid/subgid + newuidmap) … the target is the same host-side +// ownership on the session volume, so files stay identical between backends"). +// Empty when agentUID is zero (the V2a spike harness, which asserts nothing +// about ownership on its throwaway share). +// +// --uid-map/--gid-map (not --translate-uid/--translate-gid): both reach the same +// ownership, but the map flags are the record's NAMED mechanism — the mapping is +// performed by the user namespace virtiofsd is placed into by +// --sandbox=namespace, rather than internally by the daemon. That distinction is +// load-bearing here for two reasons beyond fidelity to the record: +// +// 1. Rootless, --sandbox=namespace alone leaves virtiofsd unable to become +// namespace-root ("Couldn't set the process uid as root: -1"), and a +// passthrough daemon that is not root in its own userns cannot chown a newly +// created inode to the requesting guest id — so EVERY guest create on the +// share failed EINVAL. Mapping an id to namespace-uid 0 makes the daemon +// root inside the namespace, which is what makes guest writes work at all. +// 2. --translate-uid is documented as incompatible with +// `--posix-acl=always|auto`, so it would foreclose POSIX ACLs on the share. +// +// The four mapped ranges, all one id wide, TWO PER AXIS: +// - :0::1: — an id from the invoking user's /etc/subuid range +// becomes namespace-root, so the daemon can chown as above. It is a +// subordinate id the invoking user already owns, so no capability is needed +// (newuidmap is setuid and honors /etc/subuid). +// - :::1: — the in-guest agent id maps to the invoking +// host user, which is the parity target itself. +// - :0::1: — the gid arm's namespace-root mapping, from +// /etc/subgid. It is a SEPARATE allocation from the subuid base and is read +// separately (subuid.go): newgidmap validates the gid range against +// /etc/subgid, so reusing subUIDBase here boots on a shadow-utils-default +// host and makes virtiofsd die on a divergent one. +// - :::1: — the guest agent runs uid==gid==agentUID +// (guestd linuxCredential) while a host user's gid is routinely different +// (e.g. 1000:100), so the host side here is the invoking user's real gid. +// Collapsing gid onto uid is precisely the parity break the V6 parity test +// detects. +// +// SECURITY POSTURE — this is NOT pure ownership parity, and the difference is +// deliberate and accepted, not incidental. Mapping a subordinate id to +// namespace-uid 0 makes virtiofsd ROOT IN ITS OWN USER NAMESPACE, so the daemon +// retains a namespace-scoped CAP_CHOWN / CAP_DAC_OVERRIDE / CAP_SETUID / +// CAP_SETGID / CAP_FOWNER / CAP_FSETID / CAP_SETFCAP (virtiofsd README §Usage) +// that a plain rootless daemon with only the invoking user's ambient authority +// would not have. Two things bound the consequence: +// +// - The authority is namespace-scoped and the namespace holds exactly two host +// ids — the subordinate id and the invoking user — so it confers nothing over +// any OTHER host user's files. +// - --sandbox=namespace pivot_roots the daemon into the shared dir, so even +// that authority reaches only the volume subtree it is serving. +// +// CAP_MKNOD is dropped outright by virtiofsdArgs' modcapsDropMknod: a workspace +// share has no legitimate device nodes, so it is surface with no use. +// +// The alternative — --translate-uid/--translate-gid, which reaches the same +// host-side ownership with NO namespace-uid-0 mapping and therefore none of the +// above capabilities, at the cost of foreclosing POSIX ACLs on the share — is +// tracked as a design fork (RIG-3330) for the record's owner to rule on. It is +// NOT swapped in here: --uid-map/--gid-map is the frozen record's named +// mechanism, and changing it is a design decision, not a review fix. +func virtiofsdIDMapArgs(agentUID uint32, subUIDBase, subGIDBase int) []string { + if agentUID == 0 { + return nil + } + hostUID := os.Getuid() + hostGID := os.Getgid() + agent := strconv.FormatUint(uint64(agentUID), 10) + return []string{ + "--uid-map", idMapSpec("0", strconv.Itoa(subUIDBase)), + "--uid-map", idMapSpec(agent, strconv.Itoa(hostUID)), + "--gid-map", idMapSpec("0", strconv.Itoa(subGIDBase)), + "--gid-map", idMapSpec(agent, strconv.Itoa(hostGID)), + } +} + +// idMapSpec renders one virtiofsd --uid-map/--gid-map range in its +// `::::` form, always one id wide. +func idMapSpec(namespaceID, hostID string) string { + return ":" + namespaceID + ":" + hostID + ":1:" +} + // vmmArgs builds the cloud-hypervisor argv exactly per the record (lines // 542-547), dropping --fs/--vsock under the net-only smoke. Launch appends to // the guest cmdline (per BootConfig.Cmdline's contract): @@ -287,6 +456,13 @@ func vmmArgs(cfg BootConfig, consolePath string, opts launchOptions) []string { // bind reliably; a no-op elsewhere) and captures its stdout+stderr to logPath so // a boot failure can surface the daemon's own diagnostics. The real teardown // guarantee is Shutdown, not Pdeathsig (record §(g) lines 300-303). +// +// On a successful Start it also installs the child's SOLE reaper: exactly one +// goroutine per child owning exactly one cmd.Wait, publishing the result on +// c.exited. Every other path — the readiness poll's liveness check, Shutdown's +// reap, Running, PSS — observes the exit through that channel rather than +// Wait'ing itself, so there is never a second concurrent Wait on one process +// (which would race and hand one caller a bogus "waitid: no child processes"). func startChild(c *child) error { logFile, err := os.OpenFile(c.logPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) if err != nil { @@ -303,19 +479,48 @@ func startChild(c *child) error { startErr := c.cmd.Start() // Our handle on the log file is no longer needed: Start dup'd it into the // child (on success) or it stays unused (on failure). Either way close it. - _ = logFile.Close() // the child holds its own dup; our copy is done with + _ = logFile.Close() // deliberate: the child holds its own dup of the fd, so a close error on our spent copy is not actionable if startErr != nil { return startErr } + c.exited = make(chan struct{}) + go func() { + // The single Wait for this child. waitErr is written BEFORE the close, + // so any reader that has received from c.exited sees it (that is the + // happens-before edge); an *exec.ExitError from a killed daemon is the + // expected teardown outcome and is filtered by waitResult at each + // reader, not here. + c.waitErr = c.cmd.Wait() + close(c.exited) + }() return nil } -// waitForSockets polls until every path exists or the deadline elapses. A -// missing socket at the deadline is a named error naming the first path still -// absent, so a daemon that died on launch fails the boot fast. -func waitForSockets(ctx context.Context, paths []string, timeout time.Duration) error { +// waitForSockets polls until every path in paths exists, one of the children in +// waiting has exited, or the deadline elapses. +// +// LIVENESS, not just path existence: virtiofsd binds its AF_UNIX socket BEFORE +// the id-map setup that can fail, so a mapping failure leaves a mode-srwx socket +// on disk with the daemon already exited 1 (verified by execution). A +// path-existence-only poll returns nil there and the boot proceeds to start +// cloud-hypervisor against a dead daemon, where the real cause ("couldn't setup +// id mappings", in virtiofsd's own log) is replaced by an inscrutable vhost-user +// negotiation error much later. So an exited child short-circuits the poll with +// an error NAMING the daemon and carrying its log tail. +// +// A missing socket at the deadline is likewise a named error naming the first +// path still absent. +func waitForSockets(ctx context.Context, paths []string, waiting []*child, timeout time.Duration) error { deadline := time.Now().Add(timeout) for { + // Liveness first: a dead daemon's leftover socket must not read as + // readiness, so this check precedes the Stat sweep. + for _, c := range waiting { + if c.hasExited() { + return fmt.Errorf("%s exited before its socket was serving: %w; log tail:\n%s", + c.name, waitResult(c.name, c.waitErr), tailFile(c.logPath)) + } + } missing := "" for _, p := range paths { if _, err := os.Stat(p); err != nil { @@ -393,24 +598,22 @@ func (vm *VM) Shutdown(ctx context.Context) error { } // reap terminates an auxiliary daemon gracefully then forcibly: SIGTERM, wait up -// to reapGrace, SIGKILL if it is still alive, then Wait to collect the exit and -// avoid a zombie. +// to reapGrace, SIGKILL if it is still alive. It does NOT Wait — startChild's +// sole reaper owns this child's single cmd.Wait, and reap observes the exit +// through c.exited, so no second Wait can race it. func reap(c *child) error { if termErr := c.cmd.Process.Signal(syscall.SIGTERM); termErr != nil && !errors.Is(termErr, os.ErrProcessDone) { return fmt.Errorf("SIGTERM %s: %w", c.name, termErr) } - done := make(chan error, 1) - go func() { done <- c.cmd.Wait() }() select { - case err := <-done: - c.waited.Store(true) - return waitResult(c.name, err) + case <-c.exited: + return waitResult(c.name, c.waitErr) case <-time.After(reapGrace): if killErr := c.cmd.Process.Kill(); killErr != nil && !errors.Is(killErr, os.ErrProcessDone) { return fmt.Errorf("SIGKILL %s: %w", c.name, killErr) } - c.waited.Store(true) - return waitResult(c.name, <-done) + <-c.exited + return waitResult(c.name, c.waitErr) } } @@ -445,11 +648,13 @@ func waitResult(name string, err error) error { } // Running reports whether the named child's process is still alive. It is used -// by the test to assert Shutdown left no orphan. A process that has been Wait'd -// is definitively gone; otherwise signal 0 probes liveness without affecting it. +// by the test to assert Shutdown left no orphan. A child whose sole reaper has +// already observed the exit is definitively gone — and that check must come +// FIRST, because an exited-but-unreaped process is a zombie that Signal(0) +// reports as alive; otherwise signal 0 probes liveness without affecting it. func (vm *VM) Running(name string) bool { c := vm.childByName(name) - if c == nil || c.cmd.Process == nil || c.waited.Load() { + if c == nil || c.cmd.Process == nil || c.hasExited() { return false } return c.cmd.Process.Signal(syscall.Signal(0)) == nil @@ -465,7 +670,7 @@ func (vm *VM) PSS() (map[string]int64, error) { out := make(map[string]int64) var errs []error for _, c := range []*child{vm.vmm, vm.virtiofsd, vm.passt} { - if c == nil || c.cmd.Process == nil || c.waited.Load() { + if c == nil || c.cmd.Process == nil || c.hasExited() { continue } pss, err := readPSS(c.cmd.Process.Pid) diff --git a/go/internal/runtime/microvm/launch_idmap_test.go b/go/internal/runtime/microvm/launch_idmap_test.go new file mode 100644 index 00000000..42eb1f75 --- /dev/null +++ b/go/internal/runtime/microvm/launch_idmap_test.go @@ -0,0 +1,398 @@ +//go:build unix + +package microvm + +// Hermetic argv assertions for virtiofsd's id mapping and capability trim +// (record §(d) host-ownership parity), following launch_cmdline_test.go's shape: +// no VMM boots, no daemon spawns — the exact flag/value pairs are the contract, +// because a wrong pair fails at first boot as an opaque daemon-socket timeout +// rather than as a legible error. +// +// The subordinate bases are injected as fixed values rather than read from the +// test box's /etc/subuid and /etc/subgid, which is the whole point of the +// parse/spawn split in subuid.go: the mapping is pinned identically on a host +// allocated 100000 and one allocated 165536 — and, crucially, on a host whose +// two files DIVERGE, which this box's own /etc (both `mattw:100000:65536`) +// cannot exhibit. + +import ( + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "testing" +) + +// testSubBase is a deliberately NON-conventional subordinate base: 100000 would +// pass even against the old hardcode, so the assertions below would not detect a +// regression back to it. +const testSubBase = 165536 + +// testSubGIDBase is a subordinate GID base DISTINCT from testSubBase, for the +// rows that must prove the two axes carry independent values. /etc/subuid and +// /etc/subgid are separate allocations, so a gid map built from the subuid base +// boots on a lockstep host and kills virtiofsd on a divergent one. +const testSubGIDBase = 265536 + +// flagValues returns every value token following an occurrence of flag in argv, +// in order — so a test asserts both the count and the exact specs of a repeated +// flag rather than a single Contains. +func flagValues(argv []string, flag string) []string { + var out []string + for i, tok := range argv { + if tok == flag && i+1 < len(argv) { + out = append(out, argv[i+1]) + } + } + return out +} + +// TestVirtiofsdIDMapArgsExactSpecs pins all four mapping pairs for a real agent +// uid. Two arms are discriminating: the gid arm's HOST side must be os.Getgid(), +// NOT os.Getuid() — a host user's gid is routinely different from its uid (e.g. +// 1000:100), and collapsing gid onto uid is precisely the host-ownership parity +// break the KVM parity leg detects — and the gid arm's NAMESPACE-ROOT side must +// come from the /etc/subgid base, not the /etc/subuid one (see +// TestVirtiofsdIDMapArgsDivergentSubordinateBases). +func TestVirtiofsdIDMapArgsExactSpecs(t *testing.T) { + const agentUID = 1000 + argv := virtiofsdIDMapArgs(agentUID, testSubBase, testSubGIDBase) + + agent := strconv.Itoa(agentUID) + wantUID := []string{ + ":0:" + strconv.Itoa(testSubBase) + ":1:", + ":" + agent + ":" + strconv.Itoa(os.Getuid()) + ":1:", + } + wantGID := []string{ + ":0:" + strconv.Itoa(testSubGIDBase) + ":1:", + ":" + agent + ":" + strconv.Itoa(os.Getgid()) + ":1:", + } + + if got := flagValues(argv, "--uid-map"); !slices.Equal(got, wantUID) { + t.Errorf("--uid-map specs = %v, want %v (argv %v)", got, wantUID, argv) + } + if got := flagValues(argv, "--gid-map"); !slices.Equal(got, wantGID) { + t.Errorf("--gid-map specs = %v, want %v (argv %v)", got, wantGID, argv) + } + // Neither base may come from a hardcoded 100000 — the correctness bug on + // any host whose /etc/subuid or /etc/subgid range starts elsewhere. + for _, spec := range append(flagValues(argv, "--uid-map"), flagValues(argv, "--gid-map")...) { + if strings.Contains(spec, ":100000:") { + t.Errorf("spec %q carries a hardcoded 100000 base; the subordinate bases must be the parsed ones (%d/%d)", + spec, testSubBase, testSubGIDBase) + } + } +} + +// TestVirtiofsdIDMapArgsDivergentSubordinateBases is the row the whole +// subuid/subgid split exists for: with DIFFERENT uid and gid bases injected, the +// two namespace-root arms must carry DIFFERENT host ids. +// +// It cannot be a host-dependent test. /etc/subuid and /etc/subgid on a +// shadow-utils-default box (this one included: both read `mattw:100000:65536`) +// hold the SAME base, which is exactly why a gid map that reuses the subuid base +// passes everywhere it is developed and then dies on a host provisioned with +// `usermod --add-subgids` alone. newgidmap validates the --gid-map host range +// against /etc/subgid, so on such a host virtiofsd refuses the mapping — AFTER +// binding its socket, which is what makes the failure so illegible. +func TestVirtiofsdIDMapArgsDivergentSubordinateBases(t *testing.T) { + const agentUID = 1000 + argv := virtiofsdIDMapArgs(agentUID, testSubBase, testSubGIDBase) + + uidSpecs := flagValues(argv, "--uid-map") + gidSpecs := flagValues(argv, "--gid-map") + // Guarded rather than indexed blind: a mapping that lost an arm entirely + // would panic here and report a crash instead of the missing flag. + if len(uidSpecs) != 2 || len(gidSpecs) != 2 { + t.Fatalf("argv %v carries %d --uid-map and %d --gid-map specs, want 2 of each "+ + "(namespace-root plus the agent id, per axis)", argv, len(uidSpecs), len(gidSpecs)) + } + uidRoot, gidRoot := uidSpecs[0], gidSpecs[0] + if uidRoot == gidRoot { + t.Fatalf("the --uid-map and --gid-map namespace-root specs are both %q with uidBase=%d != gidBase=%d; "+ + "the gid arm is reusing the SUBUID base, which newgidmap validates against /etc/subgid and rejects "+ + "on any host whose two allocations diverge", uidRoot, testSubBase, testSubGIDBase) + } + if want := ":0:" + strconv.Itoa(testSubBase) + ":1:"; uidRoot != want { + t.Errorf("--uid-map namespace-root spec = %q, want %q (the /etc/subuid base)", uidRoot, want) + } + if want := ":0:" + strconv.Itoa(testSubGIDBase) + ":1:"; gidRoot != want { + t.Errorf("--gid-map namespace-root spec = %q, want %q (the /etc/subgid base)", gidRoot, want) + } +} + +// TestVirtiofsdIDMapArgsUnmappedSpike pins the V2a carve-out: a zero agentUID +// (the spike harness, which shares a throwaway dir and asserts nothing about +// ownership) gets NO mapping at all, so that suite keeps booting unchanged. +func TestVirtiofsdIDMapArgsUnmappedSpike(t *testing.T) { + if argv := virtiofsdIDMapArgs(0, testSubBase, testSubGIDBase); argv != nil { + t.Fatalf("virtiofsdIDMapArgs(0) = %v, want nil (the V2a spike share stays unmapped)", argv) + } +} + +// TestVirtiofsdArgsDropsMknod pins the capability trim on the full argv: a +// workspace share has no legitimate device nodes, so CAP_MKNOD is dropped from +// the set virtiofsd retains as namespace-root — on EVERY boot, including the +// unmapped spike, since the flag costs nothing there. +// +// The assertion is against the modcapsDropMknod CONST, not a literal, and that +// is load-bearing: virtiofsd does NOT validate capability names — verified by +// execution, `--modcaps=-not_a_real_cap` starts the daemon normally — so a typo +// makes the flag a silent no-op with no diagnostic anywhere, and this argv +// assertion is the only guard. A literal repeated here would agree with a typo +// in launch.go; sharing the const means a typo has to be made once to be wrong +// in both places, which the exact-literal check below then catches. +func TestVirtiofsdArgsDropsMknod(t *testing.T) { + if modcapsDropMknod != "--modcaps=-mknod" { + t.Fatalf("modcapsDropMknod = %q, want %q — virtiofsd silently ACCEPTS unknown capability names, "+ + "so a drifted literal is an undetectable no-op that leaves CAP_MKNOD on the share", modcapsDropMknod, "--modcaps=-mknod") + } + for _, agentUID := range []uint32{0, 1000} { + cfg := BootConfig{ + FSSocket: "/tmp/cvm/virtiofsd.sock", + FSSharedDir: "/tmp/cvm/share", + AgentUID: agentUID, + } + argv := virtiofsdArgs(cfg, testSubBase, testSubGIDBase) + if !slices.Contains(argv, modcapsDropMknod) { + t.Errorf("virtiofsdArgs(AgentUID=%d) = %v, want it to carry %q", agentUID, argv, modcapsDropMknod) + } + // The pre-existing flags must survive the argv restructure. + for _, want := range []string{ + "--socket-path=" + cfg.FSSocket, + "--shared-dir=" + cfg.FSSharedDir, + "--sandbox=namespace", + } { + if !slices.Contains(argv, want) { + t.Errorf("virtiofsdArgs(AgentUID=%d) = %v, want it to carry %q", agentUID, argv, want) + } + } + } +} + +// TestVirtiofsdArgsIncludesMapping is the seam assertion the launch path depends +// on: the full argv carries the mapping for a real agent uid and none for the +// spike, so the restructure into a named local did not drop the id args — and it +// carries the gid base through to the gid arm, so the seam cannot collapse the +// two bases onto one on the way from launch to argv. +func TestVirtiofsdArgsIncludesMapping(t *testing.T) { + mapped := virtiofsdArgs(BootConfig{AgentUID: 1000}, testSubBase, testSubGIDBase) + if got := len(flagValues(mapped, "--uid-map")); got != 2 { + t.Errorf("mapped argv %v carries %d --uid-map specs, want 2", mapped, got) + } + if got := flagValues(mapped, "--gid-map"); len(got) != 2 || !strings.Contains(got[0], strconv.Itoa(testSubGIDBase)) { + t.Errorf("mapped argv %v --gid-map specs = %v, want 2 specs whose namespace-root arm carries the subgid base %d", + mapped, got, testSubGIDBase) + } + spike := virtiofsdArgs(BootConfig{AgentUID: 0}, testSubBase, testSubGIDBase) + if got := flagValues(spike, "--uid-map"); len(got) != 0 { + t.Errorf("spike argv %v carries --uid-map specs %v, want none", spike, got) + } +} + +// TestParseSubordinateIDBase pins the subuid(5)/subgid(5) parse (one format, +// both files): the invoking user's entry is matched by NAME or by uid, comments +// and malformed/zero-count entries are skipped, and a user with no range is a +// named error rather than a silent fallback to a base the user does not own. +// The error must name the FILE it was parsing, so a missing subgid range is not +// misreported as a subuid problem. +func TestParseSubordinateIDBase(t *testing.T) { + tests := []struct { + name string + content string + uid int + username string + path string + want int + wantErr []string + }{ + { + name: "matched by username", + content: "root:100000:65536\nmattw:165536:65536\n", + uid: 1000, + username: "mattw", + want: 165536, + }, + { + name: "matched by uid when the name is unresolvable", + content: "1000:200000:65536\n", + uid: 1000, + want: 200000, + }, + { + name: "comments and blank lines are skipped", + content: "# subuid(5)\n\nmattw:100000:65536\n", + uid: 1000, + username: "mattw", + want: 100000, + }, + { + name: "a zero-count range is not trusted", + content: "mattw:100000:0\nmattw:300000:65536\n", + uid: 1000, + username: "mattw", + want: 300000, + }, + { + name: "a malformed entry is skipped, not parsed as a base", + content: "mattw:not-a-number:65536\nmattw:400000:1\n", + uid: 1000, + username: "mattw", + want: 400000, + }, + { + name: "no range for the user names the user, the file and the fix", + content: "root:100000:65536\nother:165536:65536\n", + uid: 1000, + username: "mattw", + path: subordinateUIDPath, + wantErr: []string{ + "mattw", "/etc/subuid", "/etc/subgid", + "usermod --add-subuids", "usermod --add-subgids", "newuidmap", "newgidmap", + }, + }, + { + name: "a missing SUBGID range names the subgid file, not the subuid one", + content: "root:100000:65536\n", + uid: 1000, + username: "mattw", + path: subordinateGIDPath, + wantErr: []string{"mattw", "in /etc/subgid", "usermod --add-subgids", "newgidmap"}, + }, + { + name: "an empty file names the fix", + content: "", + uid: 1000, + path: subordinateUIDPath, + wantErr: []string{"uid 1000", "/etc/subuid"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseSubordinateIDBase(strings.NewReader(tt.content), tt.uid, tt.username, tt.path) + if len(tt.wantErr) > 0 { + if err == nil { + t.Fatalf("parseSubordinateIDBase = %d, want an error naming %v", got, tt.wantErr) + } + for _, part := range tt.wantErr { + if !strings.Contains(err.Error(), part) { + t.Errorf("error %q does not name %q", err.Error(), part) + } + } + return + } + if err != nil { + t.Fatalf("parseSubordinateIDBase = %v, want base %d", err, tt.want) + } + if got != tt.want { + t.Fatalf("parseSubordinateIDBase = %d, want %d", got, tt.want) + } + }) + } +} + +// TestSubordinateIDBasesResolvesBothFilesIndependently is HIGH-1's preflight +// axis: the two bases come from the two files SEPARATELY, so a host whose +// allocations diverge yields divergent bases rather than one base used twice. +// +// It drives fixture files through the path-injected core because this box cannot +// exhibit the bug: its /etc/subuid and /etc/subgid both read +// `mattw:100000:65536`, so a gid arm reusing the subuid base is observationally +// identical to a correct one here. +func TestSubordinateIDBasesResolvesBothFilesIndependently(t *testing.T) { + dir := t.TempDir() + // The owner field must match THIS process, since subordinateBase resolves + // the invoking uid/username itself — the file contents are the only + // injectable half. + owner := strconv.Itoa(os.Getuid()) + uidPath := filepath.Join(dir, "subuid") + gidPath := filepath.Join(dir, "subgid") + if err := os.WriteFile(uidPath, []byte(owner+":100000:65536\n"), 0o600); err != nil { + t.Fatalf("writing the subuid fixture: %v", err) + } + if err := os.WriteFile(gidPath, []byte(owner+":165536:65536\n"), 0o600); err != nil { + t.Fatalf("writing the subgid fixture: %v", err) + } + + uidBase, gidBase, err := subordinateIDBases(uidPath, gidPath) + if err != nil { + t.Fatalf("subordinateIDBases over divergent fixtures = %v, want both bases", err) + } + if uidBase != 100000 { + t.Errorf("uid base = %d, want 100000 (from %s)", uidBase, uidPath) + } + if gidBase != 165536 { + t.Errorf("gid base = %d, want 165536 (from %s); reading it from /etc/subuid instead is the divergence bug", gidBase, gidPath) + } + if uidBase == gidBase { + t.Fatal("the two bases collapsed onto one value despite divergent fixture files; the gid axis is not read independently") + } +} + +// TestSubordinateIDBasesRefusesAMissingSubgidRange pins the OTHER half of the +// preflight fix: a host with a perfectly good /etc/subuid range and NO +// /etc/subgid range must fail at PREFLIGHT with both files and both usermod +// flags named — not at first boot, where newgidmap refuses the gid map AFTER +// virtiofsd has bound its socket and the operator sees only a vhost-user +// negotiation error. +func TestSubordinateIDBasesRefusesAMissingSubgidRange(t *testing.T) { + dir := t.TempDir() + owner := strconv.Itoa(os.Getuid()) + uidPath := filepath.Join(dir, "subuid") + gidPath := filepath.Join(dir, "subgid") + if err := os.WriteFile(uidPath, []byte(owner+":100000:65536\n"), 0o600); err != nil { + t.Fatalf("writing the subuid fixture: %v", err) + } + // A subgid file that exists but allocates the invoking user nothing — the + // `usermod --add-subuids`-only host. + if err := os.WriteFile(gidPath, []byte("root:100000:65536\n"), 0o600); err != nil { + t.Fatalf("writing the subgid fixture: %v", err) + } + + _, _, err := subordinateIDBases(uidPath, gidPath) + if err == nil { + t.Fatal("subordinateIDBases = nil error with no subgid range for the invoking user; " + + "a divergent host must fail PREFLIGHT, not at the first boot as an opaque virtiofsd death") + } + for _, part := range []string{ + gidPath, subordinateUIDPath, subordinateGIDPath, + "usermod --add-subuids", "usermod --add-subgids", "newgidmap", + } { + if !strings.Contains(err.Error(), part) { + t.Errorf("error %q does not name %q", err.Error(), part) + } + } +} + +// TestSubordinateIDBasesRefusesAnUnreadableFile pins the open-error arm on BOTH +// axes: an absent map file is a named refusal, never a fallback to the +// conventional base. The message must show its example range as an example +// (LOW-8) and name the matching --add-subgids alongside --add-subuids, since the +// two allocations are independent. +func TestSubordinateIDBasesRefusesAnUnreadableFile(t *testing.T) { + dir := t.TempDir() + owner := strconv.Itoa(os.Getuid()) + present := filepath.Join(dir, "subuid") + if err := os.WriteFile(present, []byte(owner+":100000:65536\n"), 0o600); err != nil { + t.Fatalf("writing the subuid fixture: %v", err) + } + absent := filepath.Join(dir, "does-not-exist") + + for _, tt := range []struct{ name, uidPath, gidPath string }{ + {"an unreadable subuid file", absent, present}, + {"an unreadable subgid file", present, absent}, + } { + t.Run(tt.name, func(t *testing.T) { + _, _, err := subordinateIDBases(tt.uidPath, tt.gidPath) + if err == nil { + t.Fatal("subordinateIDBases = nil error over an absent map file; an unreadable range must refuse, never fall back") + } + for _, part := range []string{absent, "usermod --add-subuids", "usermod --add-subgids", "only the shadow-utils convention"} { + if !strings.Contains(err.Error(), part) { + t.Errorf("error %q does not name %q", err.Error(), part) + } + } + }) + } +} diff --git a/go/internal/runtime/microvm/launch_teardown_test.go b/go/internal/runtime/microvm/launch_teardown_test.go index e9ee9236..35e7e824 100644 --- a/go/internal/runtime/microvm/launch_teardown_test.go +++ b/go/internal/runtime/microvm/launch_teardown_test.go @@ -85,9 +85,8 @@ sleep 30`) // that exits on its own (as the guest does on RB_POWER_OFF) is observed by // WaitVMMExit via the sole reaper WELL UNDER the grace window — it must NOT burn // the full timeout waiting on a zombie. It assembles a minimal VM with just a -// fake vmm child plus a manually-started reaper mirroring launch's (the full -// launch needs passt/virtiofsd), since the assertion is purely about the -// reaper→vmmExited→WaitVMMExit path. +// fake vmm child (the full launch needs passt/virtiofsd), since the assertion is +// purely about the startChild-reaper→exited→WaitVMMExit path. func TestWaitVMMExitObservesPromptSelfExit(t *testing.T) { dir := t.TempDir() vm := &VM{ @@ -98,17 +97,12 @@ func TestWaitVMMExitObservesPromptSelfExit(t *testing.T) { cmd: exec.CommandContext(t.Context(), "/bin/sh", "-c", "sleep 0.1; exit 0"), }, } + // startChild installs the sole reaper itself, exactly as launch relies on; + // the VM's channel is that child's, so there is no second Wait anywhere. if err := startChild(vm.vmm); err != nil { t.Fatalf("startChild(vmm fake): %v", err) } - // The sole reaper, mirroring launch: it owns the single Wait and closes - // vmmExited once the process has exited. - vm.vmmExited = make(chan struct{}) - go func() { - _ = vm.vmm.cmd.Wait() // reaper mirror: a fake VMM's exit is the expected outcome - vm.vmm.waited.Store(true) - close(vm.vmmExited) - }() + vm.vmmExited = vm.vmm.exited // A generous grace window: the fake exits in ~100ms, so WaitVMMExit must // return true well before this elapses. Measure to prove it did not burn @@ -123,6 +117,98 @@ func TestWaitVMMExitObservesPromptSelfExit(t *testing.T) { } } +// TestWaitForSocketsFailsFastOnADeadDaemon is HIGH-2's regression lock: the +// readiness wait must be LIVENESS-aware, not path-existence-only. +// +// The fake reproduces virtiofsd's real, verified ordering — it BINDS its socket +// and only then hits the id-map step that can fail, exiting non-zero with its +// diagnostic on stderr. (Measured on virtiofsd 1.14.0: a bad gid base left +// `srwx------ .../sb.sock` on disk with the daemon exited 1.) A path-only poll +// therefore Stats a socket that exists, returns nil, and launch starts +// cloud-hypervisor against a corpse; the operator then sees a vhost-user +// negotiation error instead of "couldn't setup id mappings". +// +// Three properties, each of which the pre-fix implementation fails: +// 1. an ERROR rather than nil, even though the socket path exists; +// 2. the error NAMES the daemon and carries its log tail, so virtiofsd's own +// line reaches the operator; +// 3. it returns FAST — well inside socketReadyTimeout, i.e. it short-circuits +// on the exit rather than polling out the full window. +func TestWaitForSocketsFailsFastOnADeadDaemon(t *testing.T) { + dir := t.TempDir() + socket := filepath.Join(dir, "sb.sock") + // virtiofsd's own line, apostrophe included. It travels as an ENV VAR, not + // inline in the -c script: embedded in single quotes the apostrophe would + // terminate the quoting and the fake would die before binding its socket, + // silently defeating the whole reproduction. + const diagnostic = "Couldn't setup id mappings: newgidmap failed" + + c := &child{ + name: "virtiofsd", + logPath: filepath.Join(dir, "virtiofsd.log"), + // Bind first, THEN fail — virtiofsd's actual ordering. + cmd: exec.CommandContext(t.Context(), "/bin/sh", "-c", + ": > \"$SOCKET\"; printf '%s\\n' \"$DIAGNOSTIC\" >&2; exit 1"), + } + c.cmd.Env = append(os.Environ(), "SOCKET="+socket, "DIAGNOSTIC="+diagnostic) + if err := startChild(c); err != nil { + t.Fatalf("startChild(dead-daemon fake): %v", err) + } + // Await the reaper so the poll enters with the exit already observable; + // otherwise this test would race the fake's own exit. + <-c.exited + + // The socket the dead daemon left behind: the exact condition that made the + // old path-existence poll return nil. + if _, err := os.Stat(socket); err != nil { + t.Fatalf("the fake did not leave its socket on disk (%v); the test would not reproduce the defect", err) + } + + start := time.Now() + err := waitForSockets(t.Context(), []string{socket}, []*child{c}, socketReadyTimeout) + elapsed := time.Since(start) + if err == nil { + t.Fatal("waitForSockets = nil for a daemon that exited after binding its socket; " + + "launch would start cloud-hypervisor against a DEAD virtiofsd") + } + if !strings.Contains(err.Error(), "virtiofsd") { + t.Errorf("error %q does not name the daemon", err.Error()) + } + if !strings.Contains(err.Error(), diagnostic) { + t.Errorf("error %q does not carry the daemon's own log tail (%q); the real cause would be lost", err.Error(), diagnostic) + } + // Fast: the exit short-circuits the poll instead of burning the window. + if elapsed > socketReadyTimeout/2 { + t.Errorf("waitForSockets took %v of a %v budget; a dead child must short-circuit the poll", elapsed, socketReadyTimeout) + } +} + +// TestWaitForSocketsSucceedsForALiveDaemon is the non-vacuity control for the +// liveness check: a child that binds its socket and STAYS UP must still be +// accepted. Without it the assertion above would pass on a waitForSockets that +// had simply started erroring unconditionally. +func TestWaitForSocketsSucceedsForALiveDaemon(t *testing.T) { + dir := t.TempDir() + socket := filepath.Join(dir, "live.sock") + c := &child{ + name: "virtiofsd", + logPath: filepath.Join(dir, "virtiofsd.log"), + cmd: exec.CommandContext(t.Context(), "/bin/sh", "-c", ": > "+socket+"; sleep 30"), + } + if err := startChild(c); err != nil { + t.Fatalf("startChild(live fake): %v", err) + } + t.Cleanup(func() { + if err := reap(c); err != nil { + t.Errorf("reaping the live fake: %v", err) + } + }) + + if err := waitForSockets(t.Context(), []string{socket}, []*child{c}, socketReadyTimeout); err != nil { + t.Fatalf("waitForSockets over a LIVE daemon that bound its socket = %v, want nil", err) + } +} + // readPidFile reads a pid a fake wrote, retrying briefly since the fake writes // it asynchronously after exec. func readPidFile(t *testing.T, path string) int { diff --git a/go/internal/runtime/microvm/subuid.go b/go/internal/runtime/microvm/subuid.go new file mode 100644 index 00000000..baefb0b6 --- /dev/null +++ b/go/internal/runtime/microvm/subuid.go @@ -0,0 +1,171 @@ +//go:build unix + +package microvm + +// The subuid(5)/subgid(5) reads behind virtiofsd's id mapping. virtiofsd shells +// out to newuidmap(1)/newgidmap(1) for a non-trivial --uid-map/--gid-map, and +// those setuid helpers VALIDATE the requested host range against subuid(5) / +// subgid(5) respectively ("the range of subordinate user IDs must have been set +// up via subuid(5)", virtiofsd README §--uid-map). So each base mapped to a +// namespace id 0 is a per-host allocation that must be READ, not assumed: +// shadow-utils happens to allocate 100000 to the first user, but a second user +// on the same box gets 165536, and LDAP/AD-backed or container-image-provisioned +// hosts routinely differ. Assuming it fails in the worst way — the map helper +// refuses, virtiofsd dies, and the boot surfaces as an opaque "waiting for +// daemon sockets" timeout (or worse: virtiofsd binds its socket BEFORE the +// id-map step, so a mapping failure can leave a live-looking socket behind a +// dead daemon — see waitForSockets' liveness check) with the real cause only in +// virtiofsd.log. +// +// Split into a pure parse over an io.Reader plus a thin file-reading wrapper so +// the argv tests can pin the exact mapping against fixed bases with no +// dependence on the test box's own /etc/subuid or /etc/subgid. + +import ( + "bufio" + "fmt" + "io" + "os" + "os/user" + "strconv" + "strings" +) + +// The two subordinate-id maps the id-mapping depends on. They are SEPARATE, +// INDEPENDENT allocations and each is read on its own axis: newuidmap validates +// the --uid-map host range against /etc/subuid, newgidmap validates the +// --gid-map host range against /etc/subgid. shadow-utils' useradd default +// happens to allocate the same base in both, but that is a DEFAULT, NOT AN +// INVARIANT — `usermod --add-subuids` and `--add-subgids` are separate flags, +// and LDAP/AD-backed or image-provisioned hosts routinely allocate the two +// ranges independently. Reusing the subuid base for the gid map therefore boots +// fine on a lockstep box and dies on a divergent one, which is why both files +// are resolved here and at preflight (VerifySubordinateIDRange). +const ( + subordinateUIDPath = "/etc/subuid" + subordinateGIDPath = "/etc/subgid" +) + +// subordinateProvisionHint is the one remediation string every failure on either +// axis carries. The range it shows is an EXAMPLE, not a canonical value: the +// whole point of reading these files is that the conventional 100000 base must +// never be assumed. Both usermod flags are named because the two allocations are +// independent — provisioning only one leaves the other axis broken. +const subordinateProvisionHint = "provision both ranges with " + + "`usermod --add-subuids - ` and `usermod --add-subgids - ` " + + "(the base is an operator choice — 100000-65536 is only the shadow-utils convention, not a required value); " + + "rootless podman consumes the same two files" + +// SubordinateIDBases returns the first subordinate uid AND the first subordinate +// gid allocated to the invoking user — the host ids virtiofsd's mapping makes +// namespace-uid 0 and namespace-gid 0 so the daemon can chown guest-created +// inodes (see virtiofsdIDMapArgs). The two are read independently because +// /etc/subuid and /etc/subgid are independent allocations (see the consts +// above). It fails with a named error, never a fallback, when either range is +// missing: silently mapping an id the user does not own is exactly the opaque +// boot failure these reads exist to prevent. +func SubordinateIDBases() (uidBase, gidBase int, err error) { + return subordinateIDBases(subordinateUIDPath, subordinateGIDPath) +} + +// subordinateIDBases is the path-injected core, so the divergent-host and +// missing-subgid axes are unit-testable against fixture files rather than +// against the test box's own /etc — which on a shadow-utils default box carries +// IDENTICAL bases in both files and therefore cannot detect a gid map that +// wrongly reuses the uid base. +func subordinateIDBases(uidPath, gidPath string) (uidBase, gidBase int, err error) { + uidBase, err = subordinateBase(uidPath) + if err != nil { + return 0, 0, err + } + gidBase, err = subordinateBase(gidPath) + if err != nil { + return 0, 0, err + } + return uidBase, gidBase, nil +} + +// VerifySubordinateIDRange is the startup axis over the same reads: it resolves +// BOTH subordinate bases and discards them, so a host missing EITHER range — +// including the host that has a subuid range but no subgid range, which boots +// past every uid-only check and then dies in virtiofsd's gid map — fails +// preflight with the fix named rather than at the first session boot. +func VerifySubordinateIDRange() error { + if _, _, err := SubordinateIDBases(); err != nil { + return err + } + return nil +} + +// subordinateBase reads one subordinate-id map file and returns the invoking +// user's base within it. Path-taking rather than hardcoded so the uid and gid +// axes share one implementation and neither can silently borrow the other's +// allocation. +func subordinateBase(path string) (int, error) { + f, err := os.Open(path) //nolint:gosec // G304: path is one of this package's two subordinateUIDPath/subordinateGIDPath consts (or a test fixture), never caller input + if err != nil { + return 0, fmt.Errorf( + "microvm: virtiofsd id-mapping requires a subordinate id range for the invoking user in both "+ + "%s and %s, but %s is unreadable: %w (%s)", + subordinateUIDPath, subordinateGIDPath, path, err, subordinateProvisionHint) + } + // Read-only handle over a small text file; a close error after a completed + // parse cannot affect the already-returned base and is not actionable. + defer func() { _ = f.Close() }() // deliberate: read-only handle, nothing to flush, and the parse result is already in hand + + uid := os.Getuid() + // The invoking user's NAME as well as its uid: subuid(5)/subgid(5) entries + // key on either, and shadow-utils writes the name. + name := "" + if u, lookupErr := user.LookupId(strconv.Itoa(uid)); lookupErr == nil { + name = u.Username + } + return parseSubordinateIDBase(f, uid, name, path) +} + +// parseSubordinateIDBase is the pure half: the first subuid(5)/subgid(5) entry +// whose owner field matches the invoking user by name or by uid, returning its +// base. The format is `::` with `#` comments; a malformed or +// zero-count entry is skipped rather than trusted, since a range that grants no +// id cannot back a mapping. path names the file being parsed, so a failure on +// the gid axis is not misreported as a subuid problem. +func parseSubordinateIDBase(r io.Reader, uid int, username, path string) (int, error) { + uidStr := strconv.Itoa(uid) + scanner := bufio.NewScanner(r) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + fields := strings.Split(line, ":") + if len(fields) != 3 { + continue + } + owner := strings.TrimSpace(fields[0]) + if owner != uidStr && (username == "" || owner != username) { + continue + } + base, err := strconv.Atoi(strings.TrimSpace(fields[1])) + if err != nil { + continue + } + count, err := strconv.Atoi(strings.TrimSpace(fields[2])) + if err != nil || count < 1 || base < 0 { + continue + } + return base, nil + } + if err := scanner.Err(); err != nil { + return 0, fmt.Errorf("microvm: reading %s: %w", path, err) + } + who := username + if who == "" { + who = "uid " + uidStr + } + return 0, fmt.Errorf( + "microvm: virtiofsd id-mapping requires a subordinate id range for %s in %s, but none is allocated "+ + "(newuidmap validates the mapped host uid range against %s and newgidmap validates the gid range "+ + "against %s — the two are INDEPENDENT allocations, so one being present does not cover the other, "+ + "and virtiofsd would die after binding its socket); %s", + who, path, subordinateUIDPath, subordinateGIDPath, subordinateProvisionHint) +} diff --git a/go/internal/runtime/microvm_isolation_microvm_test.go b/go/internal/runtime/microvm_isolation_microvm_test.go new file mode 100644 index 00000000..d265e644 --- /dev/null +++ b/go/internal/runtime/microvm_isolation_microvm_test.go @@ -0,0 +1,802 @@ +//go:build microvm && unix + +package runtime + +// The KVM-gated virtio-fs ISOLATION suite (record §Plan V6 Test cycle). This is +// the slice that PROVES inter-tenant isolation rather than exercising a happy +// path, so every assertion here drives a real MicroVMRuntime through +// Create→Start and then execs inside the live guest to ATTEMPT an escape, +// asserting confinement from both sides of the boundary (what the guest can +// name, and what actually landed on the host). +// +// Every test calls microvmtest.Require(t) FIRST, mirroring +// microvm_lifecycle_microvm_test.go: on a KVM-less box it SKIPS, and under +// COMPASS_REQUIRE_MICROVM=1 that skip becomes a hard failure — so a green run +// proves the suite really booted guests. +// +// The three legs, and what each actually proves: +// +// 1. Path traversal (TestMicroVMVolumeTraversalConfined) — the guest attempts +// to read and write outside its volume via `..`, an absolute host path, and +// a symlink planted inside the volume pointing at a host path outside it. +// Confinement is asserted on BOTH sides: the guest cannot read the outside +// canary's content, and the host-side outside tree is byte-for-byte +// unchanged afterwards. The host-side half is the load-bearing one — a guest +// read failing could be a missing file, but an unchanged host tree after a +// write attempt is confinement. +// 2. Cross-session unreachability (TestMicroVMCrossSessionVolumeUnreachable) — +// two sessions boot with distinct volumes; guest A cannot read B's secret by +// any path, and nothing A writes appears in B's host-side volume. +// 3. Host-ownership parity (TestMicroVMHostOwnershipParity) — a file the guest +// agent creates on the shared volume must land with the SAME host-side +// (uid,gid) the podman `--userns=keep-id:uid=N,gid=N` path produces: the +// INVOKING host user, not the in-guest agent id (podman.go createArgs). +// This is the test that decided whether launch.go needed virtiofsd uid/gid +// translation (record §(d)) — see parityTargetUID/GID below. +// +// The quota-enforcement-in-guest leg is gated separately and skips honestly; see +// TestMicroVMVolumeQuotaEnforcedInGuest. + +import ( + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" + + "github.com/RigelBuild/compass/go/internal/agentuid" + "github.com/RigelBuild/compass/go/internal/microvmtest" +) + +// outsideCanaryBody is the content planted in a host file OUTSIDE the session +// volume. A guest exec that ever prints this string has escaped the volume +// subtree, so it is a distinctive sentinel rather than a generic word. +const outsideCanaryBody = "HOST-ONLY-CANARY-8f3ac1d0-must-never-be-readable-from-a-guest" + +// The bounds on the quota-enforcement leg's guest write. It must cross the +// project byte bound to observe EDQUOT, but the cost of crossing it is the +// operator's quota size, which the test does not control — and the 120s +// per-exec cap (execDefaultTimeout) is hard, so an unbounded fill fails as a +// transport timeout rather than with the verdict the leg exists to assert. +// +// quotaFillMarginMiB is how far past the remaining headroom to write: enough to +// be unambiguously over the bound even if usage shifts between the pre-read and +// the write, small enough to be free. +// +// quotaFillCeilingMiB is the largest fill this leg will attempt. 1 GiB over +// virtio-fs is comfortably inside the cap on any box that can run this suite at +// all, while a 10 GiB production-sized quota is not — so a volume with more +// headroom than this SKIPS with the reason named rather than timing out. The +// operator's quota volume for this leg is expected to be purpose-sized (a few +// hundred MiB), not a production project. +const ( + quotaFillMarginMiB = 64 + quotaFillCeilingMiB = 1024 +) + +// isolationSession boots one session against a fresh volume dir and returns the +// runtime, its id, and the host-side volume path. Teardown is registered so a +// failed assertion still tears the VM down. The volume is a t.TempDir() child so +// it is removed with the test; the SHORT runroot comes from e2eConfig (the +// AF_UNIX sun_path budget, microvm_lifecycle_microvm_test.go). +func isolationSession(t *testing.T, env microvmtest.Env, name string) (*MicroVMRuntime, ContainerID, string) { + t.Helper() + m := NewMicroVMRuntime(e2eConfig(t, env)) + volume := filepath.Join(t.TempDir(), "volume") + if err := os.MkdirAll(volume, 0o700); err != nil { + t.Fatalf("creating session volume %s: %v", volume, err) + } + id, err := m.Create(t.Context(), ContainerSpec{ + Name: name, + UID: agentuid.AgentUID, + Mounts: []Mount{{HostPath: volume, ContainerPath: workspaceMountPath}}, + }) + if err != nil { + t.Fatalf("Create(%s): %v", name, err) + } + t.Cleanup(func() { + if err := m.Remove(t.Context(), id); err != nil { + t.Errorf("Remove(%s): %v", name, err) + } + }) + if err := m.Start(t.Context(), id); err != nil { + t.Fatalf("Start(%s): %v", name, err) + } + return m, id, volume +} + +// guestSh runs a shell snippet in the guest as the agent uid and returns its +// combined stdout+stderr and exit code. A transport/refusal error is fatal; a +// NON-ZERO EXIT IS NOT — a denied escape attempt is expected to exit non-zero, +// and that is the outcome under test (mirrors rowExecExitCodes' posture). +func guestSh(t *testing.T, m *MicroVMRuntime, id ContainerID, script string) (string, int) { + t.Helper() + out, err := m.Exec(t.Context(), id, + NewExecSpec("sh", "-s").WithStdin(script).AsUser(strconv.Itoa(int(agentuid.AgentUID)))) + if err != nil { + t.Fatalf("guest exec failed at the transport/refusal layer (not the escape itself): %v", err) + } + return out.Stdout + out.Stderr, out.ExitCode +} + +// crossSessionAttempt is one way tenant A could try to reach tenant B's volume, +// paired with what its OUTPUT must not contain. The secret body is the +// discriminator for a `cat`; a command that prints names rather than content +// (`ls`, `grep -r` on paths) needs its own forbidden strings, or the assertion +// is vacuously true and the row proves nothing. +type crossSessionAttempt struct { + script string + forbid []string +} + +// sweepBatchSize is how many paths one awk invocation is handed. The scan's +// dominant cost is process creation, against the HARD 120s per-exec cap +// (execDefaultTimeout, microvm_lifecycle.go). One-spawn-per-file over roots +// including "/" ran the cross-tenant row at ~72s — ~60% of the cap, i.e. a flake +// waiting for a loaded box. Batching collapses that fork cost to per-batch, +// since awk takes many FILENAMEs per run and reports which file each match came +// from itself. +// +// Measured in-guest on this box over an identical 425-file tree, needle in the +// last file: 644ms at batch size 1 versus 47ms at 200 — a 13.7x reduction, i.e. +// batching removes essentially all of the ~1.5ms/file spawn overhead. +// +// 200 is bounded by the guest's ARG_MAX rather than by taste: paths average well +// under 128 bytes, so a 200-path argv stays far inside the limit while cutting +// spawns by two orders of magnitude. +const sweepBatchSize = 200 + +// sweepScript builds a recursive content search the guest can actually run. The +// guest image ships bash 5.3 and awk but NO grep and NO find, so a `grep -r` +// row exits 127 without searching anything — a vacuous pass that looks like +// confinement. bash's globstar walks the trees and awk does the matching, and +// the exit status mirrors grep's: 0 when the needle was found, 1 when it was +// not, so a caller can still assert the non-zero exit a confined command owes. +// +// Matching files are printed as `:`, so BOTH discriminators are +// live: the secret body appears in the output if any file's content was read, +// and the path appears if a file under another tenant's volume was reachable at +// all. +// +// BATCHED, one awk per sweepBatchSize files rather than one per file: the +// per-file variant spent nearly all its time forking (see sweepBatchSize) and +// sat at ~60% of the 120s exec cap. awk is handed many FILENAMEs at once and +// reports the matching one itself, so the output contract is unchanged. +// +// Two things stop the sweep from finding ITS OWN needle, which would be a false +// escape report rather than a real one: +// +// - The needle travels in an EXPORTED ENV VAR, never in argv. Passed as +// `awk -v`, it would land in the searcher's own /proc/self/cmdline, so the +// sweep would match the string it is looking for in its own command line. +// - /proc, /sys and /dev are skipped. They are synthetic kernel interfaces +// that cannot hold another tenant's volume, so excluding them removes the +// self-match surface (the environ/cmdline of the running searcher) without +// narrowing what the row actually probes. +func sweepScript(needle, roots string) string { + // The awk program: scan every FILENAME handed to this invocation, print + // `:` per match, and exit non-zero when the batch had none — so + // the caller's `found` accumulator keeps grep's semantics across batches. + const awkProg = `index($0, ENVIRON["SWEEP_NEEDLE"]) { print FILENAME ":" $0; hit=1 } END { exit !hit }` + return "export SWEEP_NEEDLE='" + needle + "'; " + + "shopt -s globstar nullglob dotglob; found=1; batch=(); " + + // scan() runs one awk over the accumulated batch and clears it. Guarded + // on a non-empty batch so a trailing flush with nothing pending does not + // invoke awk on zero files (which would read stdin and hang). + "scan() { ((${#batch[@]})) || return 0; " + + "if awk '" + awkProg + "' \"${batch[@]}\" 2>/dev/null; then found=0; fi; batch=(); }; " + + "for root in " + roots + "; do " + + "for f in \"$root\"/**/*; do " + + // Collapse repeated slashes before matching: a "/" root globs to + // "//proc/self/environ", which a /proc/* pattern does NOT match — the + // sweep would then read its own environ and report finding the needle + // it was given, a false escape. + "n=$f; while [[ $n == //* ]]; do n=${n#/}; done; " + + "case $n in /proc/*|/sys/*|/dev/*) continue;; esac; " + + "[[ -f $f && -r $f ]] || continue; " + + "batch+=(\"$f\"); " + + "((${#batch[@]} >= " + strconv.Itoa(sweepBatchSize) + ")) && scan; " + + "done; done 2>/dev/null; scan; exit $found" +} + +// TestMicroVMSweepScriptFindsItsNeedle is the non-vacuity control for +// sweepScript itself: pointed at a tree that DOES contain the needle, it must +// find it and exit 0. Without this the cross-tenant sweep row could pass +// because the search is broken rather than because the volume is unreachable — +// exactly the failure mode that made the original `grep -r` row worthless. +func TestMicroVMSweepScriptFindsItsNeedle(t *testing.T) { + env := microvmtest.Require(t) + m, id, _ := isolationSession(t, env, "iso-sweep-control") + + const needle = "SWEEP-CONTROL-CANARY-2d7f4a91" + if out, code := guestSh(t, m, id, "mkdir -p /workspace/deep/nested && printf '%s' '"+needle+"' > /workspace/deep/nested/planted.txt"); code != 0 { + t.Fatalf("planting the sweep control canary: exit %d, %q", code, out) + } + out, code := guestSh(t, m, id, sweepScript(needle, "/workspace")) + if code != 0 { + t.Fatalf("sweepScript did not find a needle planted in its own search root (exit %d, %q); "+ + "the cross-tenant sweep row would pass vacuously", code, truncate(out)) + } + if !strings.Contains(out, needle) { + t.Fatalf("sweepScript exited 0 but its output %q does not carry the needle; the content check would be vacuous", truncate(out)) + } + if !strings.Contains(out, "planted.txt") { + t.Errorf("sweepScript output %q does not name the matching path; the path check would be vacuous", truncate(out)) + } + t.Logf("sweep control: found the planted canary -> exit %d, %q", code, strings.TrimSpace(truncate(out))) +} + +// TestMicroVMSweepScriptFindsANeedleAcrossBatches is the control for the +// BATCHING specifically, which the single-file control above cannot reach: with +// only a handful of files the sweep never fills a batch, so the mid-loop scan +// and the accumulator that carries a hit across batches are both dead code in +// that run — and the cross-tenant row now completes in ~26ms over the targeted +// roots, so it does not exercise them either. +// +// Planting well over sweepBatchSize files and putting the needle ONLY in the +// last one forces two things the batched form must get right: +// +// 1. the mid-loop `scan` flushes full batches instead of accumulating an argv +// past ARG_MAX; +// 2. the trailing flush runs, and `found` survives the batch that matched — +// a batched sweep whose exit status came from the LAST awk alone would +// report "not found" whenever the needle sat in any earlier batch, so the +// early-needle case below pins the other side of the same accumulator. +func TestMicroVMSweepScriptFindsANeedleAcrossBatches(t *testing.T) { + env := microvmtest.Require(t) + m, id, _ := isolationSession(t, env, "iso-sweep-batch") + + // Comfortably more than two full batches, so at least two mid-loop flushes + // happen before the trailing one. + fileCount := sweepBatchSize*2 + 25 + plant := "mkdir -p /workspace/many && for i in $(seq 1 " + strconv.Itoa(fileCount) + "); do " + + "printf 'filler line %s\\n' \"$i\" > /workspace/many/f$i.txt; done && ls /workspace/many | wc -l" + out, code := guestSh(t, m, id, plant) + if code != 0 { + t.Fatalf("planting %d filler files: exit %d, %q", fileCount, code, truncate(out)) + } + if got := strings.TrimSpace(out); got != strconv.Itoa(fileCount) { + t.Fatalf("planted %q files, want %d; the batch boundary would not be crossed", got, fileCount) + } + + for _, tt := range []struct{ name, file, needle string }{ + // Last file: only the TRAILING flush can find it. + {"in the final batch", "/workspace/many/f" + strconv.Itoa(fileCount) + ".txt", "SWEEP-BATCH-LAST-6c1e8f30"}, + // First file: found by a MID-LOOP flush, so `found` must survive every + // later batch that matched nothing. + {"in the first batch", "/workspace/many/f1.txt", "SWEEP-BATCH-FIRST-91ad47b2"}, + } { + t.Run(tt.name, func(t *testing.T) { + if out, code := guestSh(t, m, id, + "printf '%s\\n' '"+tt.needle+"' >> "+tt.file); code != 0 { + t.Fatalf("planting the needle in %s: exit %d, %q", tt.file, code, truncate(out)) + } + start := time.Now() + out, code := guestSh(t, m, id, sweepScript(tt.needle, "/workspace")) + elapsed := time.Since(start) + if code != 0 { + t.Fatalf("the batched sweep did not find a needle planted %s of %d files (exit %d, %q); "+ + "the cross-tenant sweep row would pass vacuously", tt.name, fileCount, code, truncate(out)) + } + if !strings.Contains(out, tt.needle) { + t.Fatalf("the batched sweep exited 0 but its output %q does not carry the needle; "+ + "the content check would be vacuous", truncate(out)) + } + if !strings.Contains(out, filepath.Base(tt.file)) { + t.Errorf("the batched sweep output %q does not name the matching file %s; awk must report the "+ + "FILENAME each match came from, which is what makes a batched scan as diagnostic as a per-file one", + truncate(out), tt.file) + } + t.Logf("batched sweep over %d files (batch size %d) found the needle %s in %v: %q", + fileCount, sweepBatchSize, tt.name, elapsed.Round(time.Millisecond), strings.TrimSpace(truncate(out))) + }) + } +} + +// TestMicroVMVolumeTraversalConfined is the path-traversal leg: the guest tries +// to escape /workspace three ways and is confined every time, proven from both +// sides of the boundary. +func TestMicroVMVolumeTraversalConfined(t *testing.T) { + env := microvmtest.Require(t) + m, id, volume := isolationSession(t, env, "iso-traversal") + + // The host tree OUTSIDE the volume: a sibling of the volume dir, so a + // `/workspace/..` that actually escaped the share would land right in it. + outside := filepath.Join(filepath.Dir(volume), "outside") + if err := os.MkdirAll(outside, 0o700); err != nil { + t.Fatalf("creating outside dir: %v", err) + } + canary := filepath.Join(outside, "canary.txt") + if err := os.WriteFile(canary, []byte(outsideCanaryBody), 0o600); err != nil { + t.Fatalf("planting host canary: %v", err) + } + + // A symlink INSIDE the volume pointing at the outside host path. This is the + // sharpest probe: virtio-fs passes the link through verbatim, so if the + // guest could resolve it against the host's namespace the escape would + // succeed. It cannot — the target is resolved inside the guest's own mount + // namespace, where that path does not exist. + if err := os.Symlink(canary, filepath.Join(volume, "escape-link")); err != nil { + t.Fatalf("planting escape symlink: %v", err) + } + // A symlink to the outside DIRECTORY too, so a traversal through a link + // (rather than a direct read of one) is covered. + if err := os.Symlink(outside, filepath.Join(volume, "escape-dir")); err != nil { + t.Fatalf("planting escape dir symlink: %v", err) + } + + // Read attempts. Each prints nothing on success-of-confinement; the + // assertion is that the canary body never appears in ANY of them. + reads := map[string]string{ + "dot-dot traversal": "cat /workspace/../canary.txt /workspace/../outside/canary.txt", + "absolute host path": "cat " + canary, + "symlink to host file": "cat /workspace/escape-link", + "symlink to host dir": "cat /workspace/escape-dir/canary.txt", + "deep dot-dot to fs root": "cat /workspace/../../../../../.." + canary, + } + for name, script := range reads { + t.Run("read: "+name, func(t *testing.T) { + out, code := guestSh(t, m, id, script) + if strings.Contains(out, outsideCanaryBody) { + t.Fatalf("guest READ a host file outside its volume via %s — VOLUME ESCAPE.\noutput: %q", name, out) + } + if code == 0 { + t.Errorf("escape attempt %q exited 0 (output %q); a confined read must fail", name, out) + } + t.Logf("confined: %s -> exit %d, %q", name, code, strings.TrimSpace(out)) + }) + } + + // Write attempts. The guest's own root filesystem is writable (an overlay), + // so a `/workspace/../pwned` may well succeed INSIDE the guest — that proves + // nothing either way. The real question is whether any byte landed on the + // HOST outside the volume, which the snapshot comparison below answers. + before := snapshotTree(t, outside) + writes := []string{ + "echo pwned > /workspace/../pwned.txt", + "echo pwned > /workspace/../outside/pwned.txt", + "echo pwned > " + filepath.Join(outside, "pwned-abs.txt"), + "echo pwned > /workspace/escape-dir/pwned-link.txt", + "echo overwritten > /workspace/escape-link", + "rm -f " + canary, + "rm -rf " + outside, + } + for _, script := range writes { + out, code := guestSh(t, m, id, script) + t.Logf("write attempt %q -> exit %d, %q", script, code, strings.TrimSpace(out)) + } + after := snapshotTree(t, outside) + if before != after { + t.Fatalf("the host tree OUTSIDE the session volume changed after guest write attempts — VOLUME ESCAPE.\nbefore: %s\nafter: %s", before, after) + } + // And the canary's content specifically: an in-place overwrite through the + // symlink would keep the tree shape identical while corrupting the file. + body, err := os.ReadFile(canary) + if err != nil { + t.Fatalf("reading the host canary after the guest write attempts: %v", err) + } + if string(body) != outsideCanaryBody { + t.Fatalf("the host canary was REWRITTEN by the guest (now %q) — VOLUME ESCAPE", string(body)) + } + t.Logf("host tree outside the volume unchanged after %d write attempts: %s", len(writes), after) +} + +// TestMicroVMCrossSessionVolumeUnreachable is the multi-tenant leg: two live +// sessions, and guest A cannot reach guest B's volume by any path, nor write +// into it. This is the assertion that makes "another tenant's volume is not +// merely unreadable but unnameable" (record §(d)) a tested property. +func TestMicroVMCrossSessionVolumeUnreachable(t *testing.T) { + env := microvmtest.Require(t) + const tenantBSecret = "TENANT-B-SECRET-4b91e7c2-must-never-be-readable-from-tenant-A" + + mA, idA, volumeA := isolationSession(t, env, "iso-tenant-a") + mB, idB, volumeB := isolationSession(t, env, "iso-tenant-b") + + // B's secret, planted host-side inside B's volume, and also written by B's + // OWN guest — so the test covers both a host-authored and a guest-authored + // file (the latter is what a real tenant's workspace actually contains). + if err := os.WriteFile(filepath.Join(volumeB, "host-secret.txt"), []byte(tenantBSecret), 0o600); err != nil { + t.Fatalf("planting tenant B's host secret: %v", err) + } + if out, code := guestSh(t, mB, idB, "printf '%s' '"+tenantBSecret+"' > /workspace/guest-secret.txt"); code != 0 { + t.Fatalf("tenant B could not write its own workspace file: exit %d, %q", code, out) + } + // A's own volume is populated and readable by A. Without this the + // cross-tenant negatives below could pass because A's share is broken + // rather than because B's is unreachable. + if err := os.WriteFile(filepath.Join(volumeA, "a-own.txt"), []byte("tenant-a-own"), 0o600); err != nil { + t.Fatalf("planting tenant A's own file: %v", err) + } + if out, code := guestSh(t, mA, idA, "cat /workspace/a-own.txt"); code != 0 || !strings.Contains(out, "tenant-a-own") { + t.Fatalf("tenant A cannot read its OWN volume (exit %d, %q); the cross-tenant negatives would be vacuous", code, out) + } + // Sanity: B can read its own secret. Without this, the cross-tenant + // negative below could pass vacuously (e.g. if the share were broken for + // everyone). + if out, code := guestSh(t, mB, idB, "cat /workspace/host-secret.txt /workspace/guest-secret.txt"); code != 0 || !strings.Contains(out, tenantBSecret) { + t.Fatalf("tenant B cannot read its OWN volume (exit %d, %q); the cross-tenant negative would be vacuous", code, out) + } + + // Every way A could try to name B's volume. Each row's success condition + // matches WHAT ITS COMMAND EMITS, which the secret-body check alone does + // not: `ls` prints names and never file content, so a Contains(secret) on + // an `ls` is unconditionally true and proves nothing whether B's volume is + // reachable or not. Every row also asserts the NON-ZERO EXIT the traversal + // leg asserts — a confined command must fail, not merely print nothing. + attempts := map[string]crossSessionAttempt{ + "B's absolute host volume path": { + script: "cat " + filepath.Join(volumeB, "host-secret.txt") + " " + filepath.Join(volumeB, "guest-secret.txt"), + }, + "B's volume dir listing": { + // An `ls` that SUCCEEDED and listed B's secrets would pass a + // content check; the discriminator here is the FILENAMES plus the + // exit code. + script: "ls -la " + volumeB, + forbid: []string{"host-secret.txt", "guest-secret.txt"}, + }, + "traversal toward B": { + script: "cat /workspace/../volume/host-secret.txt; cat /workspace/../../*/volume/*secret*", + }, + "a symlink A plants to B": { + script: "ln -sf " + volumeB + " /workspace/b-link && cat /workspace/b-link/host-secret.txt", + }, + "a content sweep of every tree A can name": { + // NOT `grep -r`: the guest image ships no grep (and no find), so + // that row exited 127 without ever searching — vacuous twice over, + // once for printing paths instead of content and once for never + // running. This is the same sweep in what the guest DOES have + // (bash globstar + awk), and sweepScript keeps grep's exit + // semantics: non-zero when nothing matched. + // + // "/" is deliberately NOT a root. The discriminating question is + // whether B's volume path is REACHABLE from A, not whether the + // read-only nix store holds the needle — and sweeping "/" pulled + // the guest's entire ~21.8k-file rootfs through the scan, which + // (even batched) buys nothing this targeted list plus B's own + // volume parent does not already answer. The parent of volume B is + // included explicitly so the one tree that COULD hold the secret is + // definitely walked; a sweep that skipped it would be the vacuous + // pass this row exists to avoid. + script: sweepScript(tenantBSecret, + "/tmp /mnt /media /run /var /home /workspace "+filepath.Dir(volumeB)), + forbid: []string{volumeB}, + }, + } + for name, attempt := range attempts { + t.Run("A cannot reach "+name, func(t *testing.T) { + // Timed: the sweep row is the expensive one, and its cost is only + // bounded relative to the HARD 120s per-exec cap (execDefaultTimeout) + // — a row creeping back toward that cap is a flake on a loaded box, + // so the margin is reported rather than left to be rediscovered. + start := time.Now() + out, code := guestSh(t, mA, idA, attempt.script) + elapsed := time.Since(start) + if strings.Contains(out, tenantBSecret) { + t.Fatalf("tenant A READ tenant B's secret via %s — CROSS-TENANT ESCAPE.\noutput: %q", name, out) + } + for _, forbidden := range attempt.forbid { + if strings.Contains(out, forbidden) { + t.Fatalf("tenant A's %s NAMED %q — tenant B's volume is reachable from A.\noutput: %q", + name, forbidden, truncate(out)) + } + } + if code == 0 { + t.Errorf("cross-tenant attempt %q exited 0 (output %q); a confined command must fail", + name, truncate(out)) + } + // Half the cap is the flake line: past it, a slower box turns this + // confinement assertion into a transport TimeoutError, which guestSh + // treats as fatal — a failure that says nothing about isolation. + if elapsed > execDefaultTimeout/2 { + t.Errorf("attempt %q took %v, over half the %v per-exec cap; it is a flake on a loaded box — "+ + "narrow its roots or increase the scan batch size", name, elapsed, execDefaultTimeout) + } + t.Logf("unreachable: %s -> exit %d in %v (cap %v), %q", + name, code, elapsed.Round(time.Millisecond), execDefaultTimeout, strings.TrimSpace(truncate(out))) + }) + } + + // A's writes must not land in B's volume. Snapshot B's host-side tree, + // let A try, and compare. + before := snapshotTree(t, volumeB) + for _, script := range []string{ + "echo from-a > " + filepath.Join(volumeB, "pwned.txt"), + "echo from-a > /workspace/b-link/pwned-link.txt", + "rm -f " + filepath.Join(volumeB, "host-secret.txt"), + "rm -rf " + volumeB, + } { + out, code := guestSh(t, mA, idA, script) + t.Logf("A write-into-B attempt %q -> exit %d, %q", script, code, strings.TrimSpace(out)) + } + if after := snapshotTree(t, volumeB); before != after { + t.Fatalf("tenant B's host-side volume changed after tenant A's write attempts — CROSS-TENANT ESCAPE.\nbefore: %s\nafter: %s", before, after) + } + // B must still be able to read its own secret: a "confinement" that worked + // by breaking B's share would otherwise pass the comparison above. + if out, code := guestSh(t, mB, idB, "cat /workspace/host-secret.txt"); code != 0 || !strings.Contains(out, tenantBSecret) { + t.Fatalf("tenant B's own volume is broken after A's attempts (exit %d, %q)", code, out) + } + t.Logf("tenant B's volume intact and unreachable from tenant A: %s", before) +} + +// TestMicroVMHostOwnershipParity is the ownership leg (record §(d)): a file the +// guest agent creates on the shared volume must land with the same host-side +// (uid,gid) the podman backend's `--userns=keep-id:uid=N,gid=N` produces — the +// INVOKING HOST user's ids, so files stay identical between backends and the +// invoking user still owns its own workspace tree. +// +// The gid axis is the discriminating one on a uid-1000 dev box: the in-guest +// agent runs as uid==gid==1000 (guestd linuxCredential), while the invoking host +// user's gid is typically NOT 1000 (here: 100/users). So an untranslated +// virtiofsd stamps gid 1000 where podman's keep-id would stamp the host gid — +// which is exactly what this test detects, and why launch.go carries virtiofsd's +// uid/gid translation. +func TestMicroVMHostOwnershipParity(t *testing.T) { + env := microvmtest.Require(t) + m, id, volume := isolationSession(t, env, "iso-parity") + + // The podman keep-id target: the invoking host user's own ids + // (podman.go createArgs `--userns=keep-id:uid=%d,gid=%d` maps the invoking + // host user onto the baked agent id, so the host-side owner is the invoker). + wantUID, wantGID := os.Getuid(), os.Getgid() + + if out, code := guestSh(t, m, id, "umask 022; echo guest-authored > /workspace/from-guest.txt && mkdir -p /workspace/from-guest-dir && id -u && id -g"); code != 0 { + t.Fatalf("guest could not write its own workspace: exit %d, %q", code, out) + } else { + t.Logf("in-guest identity for the write: %q", strings.TrimSpace(out)) + } + + for _, entry := range []string{"from-guest.txt", "from-guest-dir"} { + path := filepath.Join(volume, entry) + info, err := os.Stat(path) + if err != nil { + t.Fatalf("guest-authored %s is absent host-side: %v", entry, err) + } + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + t.Fatalf("stat %s: unexpected Sys type %T", entry, info.Sys()) + } + gotUID, gotGID := int(stat.Uid), int(stat.Gid) + t.Logf("host-side ownership of guest-authored %s: uid=%d gid=%d (podman keep-id target: uid=%d gid=%d)", + entry, gotUID, gotGID, wantUID, wantGID) + if gotUID != wantUID || gotGID != wantGID { + t.Errorf("host-ownership PARITY BROKEN for %s: guest-authored file is %d:%d, "+ + "but the podman --userns=keep-id path yields %d:%d — virtiofsd's uid/gid mapping "+ + "(launch.go --uid-map/--gid-map) must map the in-guest agent id to the invoking host user", + entry, gotUID, gotGID, wantUID, wantGID) + } + } + + // The other direction: a file the HOST creates must be owned by the agent + // in-guest, or the agent cannot write its own workspace tree — the same + // property keep-id gives the container path. + hostAuthored := filepath.Join(volume, "from-host.txt") + if err := os.WriteFile(hostAuthored, []byte("host-authored"), 0o600); err != nil { + t.Fatalf("writing host-authored file: %v", err) + } + out, code := guestSh(t, m, id, "stat -c '%u %g' /workspace/from-host.txt && echo appended >> /workspace/from-host.txt") + if code != 0 { + t.Fatalf("the guest agent cannot read/append a host-authored workspace file (exit %d, %q); "+ + "the translation must leave the invoking user's files owned by the in-guest agent", code, out) + } + t.Logf("in-guest view of a host-authored file (uid gid): %q", strings.TrimSpace(out)) + // A malformed probe must FAIL, not skip: a `stat` variant emitting fewer + // than two fields would otherwise leave this whole direction unasserted + // while the test passed green. + fields := strings.Fields(strings.TrimSpace(out)) + // The guest's stat output is followed by nothing on success, but the append + // leg shares the exec, so take exactly the first line's two fields. + if len(fields) < 2 { + t.Fatalf("stat output %q did not yield uid+gid; the host->guest ownership direction was NOT asserted", out) + } + agentID := strconv.Itoa(int(agentuid.AgentUID)) + if fields[0] != agentID || fields[1] != agentID { + t.Errorf("a host-authored workspace file appears in-guest as %s:%s, want the agent id %s:%s — "+ + "the agent would not own its own checkout", fields[0], fields[1], agentID, agentID) + } + // The host-side owner of that file must be unchanged by the guest's append: + // a translation that rewrote ownership on write would silently reassign the + // invoking user's files. + info, err := os.Stat(hostAuthored) + if err != nil { + t.Fatalf("re-stat host-authored file: %v", err) + } + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + t.Fatalf("re-stat: unexpected Sys type %T", info.Sys()) + } + if int(stat.Uid) != wantUID || int(stat.Gid) != wantGID { + t.Errorf("a host-authored file changed owner to %d:%d after a guest append, want %d:%d", + stat.Uid, stat.Gid, wantUID, wantGID) + } +} + +// requireQuotaFS gates the real in-guest quota-enforcement leg on a capability +// this box structurally cannot have: a prjquota-ACTIVE filesystem, whose +// provisioning needs root (loop file + `mkfs.xfs`/`mkfs.ext4 -O quota` + mount +// -o prjquota + a project id + limits — every step CAP_SYS_ADMIN, D7 / Global +// Constraint "Rootless is hard"). It mirrors microvmtest.Require's shape: SKIP +// with the reason named when absent, and HARD FAIL when the operator asserts the +// capability is present via COMPASS_REQUIRE_QUOTA_FS=1, so a managed/CI profile +// that is supposed to have it cannot silently stop exercising it. +// +// $COMPASS_TEST_QUOTA_VOLUME names a directory on a project-quota'd filesystem. +// Returning it only after verifyVolumeQuota confirms an ACTIVE bound is the +// honesty gate: a path on an unquota'd filesystem skips (or hard-fails under the +// require flag) rather than running a test that can never observe EDQUOT and +// passing as if it proved quota. +func requireQuotaFS(t *testing.T) string { + t.Helper() + required := os.Getenv("COMPASS_REQUIRE_QUOTA_FS") != "" + refuse := func(format string, args ...any) string { + t.Helper() + if required { + t.Fatalf("COMPASS_REQUIRE_QUOTA_FS is set but "+format, args...) + } + t.Skipf("no project-quota'd filesystem available: "+format+ + " — provisioning one needs root (loop + mkfs -o prjquota + project id + limits), which the rootless "+ + "Runner and this suite do not have (D7). The verification LOGIC is proven hermetically in "+ + "microvm_quota_test.go; set $COMPASS_TEST_QUOTA_VOLUME to a quota'd dir to run this leg for real.", args...) + return "" + } + + volume := os.Getenv("COMPASS_TEST_QUOTA_VOLUME") + if volume == "" { + return refuse("$COMPASS_TEST_QUOTA_VOLUME is unset") + } + reading, err := verifyVolumeQuota(volume, VolumeQuota{}, readVolumeQuota) + if err != nil { + return refuse("%v", err) + } + t.Logf("quota'd volume %s: %s (used ratio %.4f)", volume, reading, reading.UsedRatio()) + return volume +} + +// TestMicroVMVolumeQuotaEnforcedInGuest is the real resource-exhaustion leg +// (record §Plan V6: "writes past the byte bound and creates past the inode bound +// fail inside the guest with ENOSPC/EDQUOT while the host filesystem stays +// healthy"). It runs ONLY on a genuinely project-quota'd filesystem and skips +// with the reason named otherwise — it never fake-proves quota. +func TestMicroVMVolumeQuotaEnforcedInGuest(t *testing.T) { + env := microvmtest.Require(t) + quotaRoot := requireQuotaFS(t) + + volume, err := os.MkdirTemp(quotaRoot, "iso-quota-") //nolint:usetesting // the volume MUST live on the operator-provided quota'd filesystem, not the test's own TMPDIR — that is the whole capability under test + if err != nil { + t.Fatalf("creating a session volume on the quota'd filesystem: %v", err) + } + t.Cleanup(func() { + if err := os.RemoveAll(volume); err != nil { + t.Errorf("removing quota'd volume %s: %v", volume, err) + } + }) + + before, err := readVolumeQuota(volume) + if err != nil { + t.Fatalf("reading the quota before the fill: %v", err) + } + + m := NewMicroVMRuntime(e2eConfig(t, env)) + id, err := m.Create(t.Context(), ContainerSpec{ + Name: "iso-quota", + UID: agentuid.AgentUID, + Mounts: []Mount{{HostPath: volume, ContainerPath: workspaceMountPath}}, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + t.Cleanup(func() { + if err := m.Remove(t.Context(), id); err != nil { + t.Errorf("Remove: %v", err) + } + }) + if err := m.Start(t.Context(), id); err != nil { + t.Fatalf("Start: %v", err) + } + + // Write past the byte bound: dd until it fails. The guest MUST hit + // ENOSPC/EDQUOT rather than consuming the whole host filesystem. + // + // Only the REMAINING HEADROOM plus a margin, never the whole limit. The old + // `LimitBytes/MiB + 64` wrote the entire project limit again on top of + // whatever was already used, so against a realistically-sized operator quota + // (10GiB) it was a 10GiB guest write over virtio-fs — past the 120s per-exec + // cap, which surfaces as a transport TimeoutError (fatal at guestSh) instead + // of the ENOSPC/EDQUOT verdict this leg exists to prove. Headroom+margin + // crosses the bound by exactly the same amount while writing only what is + // actually needed to cross it. + fillMiB := (before.LimitBytes-before.UsedBytes)/(1<<20) + quotaFillMarginMiB + if fillMiB > quotaFillCeilingMiB { + t.Skipf("the quota'd volume %s has %d MiB of headroom, over this leg's %d MiB ceiling: crossing the "+ + "bound would be a %d MiB guest write over virtio-fs, past the %v per-exec cap (execDefaultTimeout), "+ + "so it would fail as a transport timeout rather than with the ENOSPC/EDQUOT verdict it exists to "+ + "assert. Point $COMPASS_TEST_QUOTA_VOLUME at a purpose-sized test project (a few hundred MiB); the "+ + "operator's quota volume for this leg is expected to be small. Observed: %s", + volume, fillMiB-quotaFillMarginMiB, quotaFillCeilingMiB, fillMiB, execDefaultTimeout, before) + } + t.Logf("filling %d MiB (headroom %d MiB + %d MiB margin) against a %v exec cap; limit %d B, used %d B", + fillMiB, fillMiB-quotaFillMarginMiB, quotaFillMarginMiB, execDefaultTimeout, before.LimitBytes, before.UsedBytes) + + fill := "dd if=/dev/zero of=/workspace/fill bs=1M count=" + strconv.FormatInt(fillMiB, 10) + " 2>&1" + start := time.Now() + out, code := guestSh(t, m, id, fill) + elapsed := time.Since(start) + if code == 0 { + t.Fatalf("the guest wrote past the project byte bound (%d B) without failing — the quota is not enforced.\noutput: %q", + before.LimitBytes, out) + } + if !strings.Contains(out, "No space left") && !strings.Contains(out, "Disk quota exceeded") { + t.Errorf("the over-bound write failed with %q, want an ENOSPC/EDQUOT diagnostic", strings.TrimSpace(out)) + } + // The ceiling above is a static estimate; this is the measured check that + // the chosen byte count really fit the budget rather than nearly missing it. + if elapsed > execDefaultTimeout/2 { + t.Errorf("the %d MiB fill took %v, over half the %v per-exec cap; lower quotaFillCeilingMiB or use a "+ + "smaller test project", fillMiB, elapsed, execDefaultTimeout) + } + t.Logf("over-bound write confined: exit %d in %v (cap %v), %q", + code, elapsed.Round(time.Millisecond), execDefaultTimeout, strings.TrimSpace(truncate(out))) + + // The HOST filesystem must stay healthy: the mount root's free space is + // still ample, i.e. the guest exhausted its project, not the filesystem. + after, err := readVolumeQuota(volume) + if err != nil { + t.Fatalf("reading the quota after the fill: %v", err) + } + if after.FilesystemBytes-after.UsedBytes <= 0 { + t.Fatalf("the host filesystem is exhausted after the guest fill (%s); the quota failed to contain it", after) + } + t.Logf("host filesystem healthy after the guest fill: %s (used ratio %.4f)", after, after.UsedRatio()) +} + +// snapshotTree renders a stable, comparable description of every entry under +// root: relative path, mode, size, and host uid/gid. It is the host-side half of +// each escape assertion — an unchanged snapshot after a batch of guest write +// attempts is what "confined" means, in a form a diff can show. +func snapshotTree(t *testing.T, root string) string { + t.Helper() + var b strings.Builder + err := filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + rel, relErr := filepath.Rel(root, path) + if relErr != nil { + return relErr + } + info, infoErr := entry.Info() + if infoErr != nil { + return infoErr + } + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("stat %s: unexpected Sys type %T", path, info.Sys()) + } + size := info.Size() + if entry.IsDir() { + // A directory's size is filesystem bookkeeping that shifts as + // entries are added and removed and then re-added, so it is not a + // stable identity — the entry SET (rendered by the walk itself) is. + size = -1 + } + fmt.Fprintf(&b, "%s mode=%s size=%d owner=%d:%d\n", rel, info.Mode(), size, stat.Uid, stat.Gid) + return nil + }) + if err != nil { + t.Fatalf("snapshotting %s: %v", root, err) + } + return b.String() +} + +// truncate bounds a guest output for a log line, so a `grep -r /` that returned +// a lot does not flood the test log. +func truncate(out string) string { + const max = 512 + if len(out) <= max { + return out + } + return out[:max] + "…(truncated)" +} diff --git a/go/internal/runtime/microvm_lifecycle.go b/go/internal/runtime/microvm_lifecycle.go index acea4c41..94eb1815 100644 --- a/go/internal/runtime/microvm_lifecycle.go +++ b/go/internal/runtime/microvm_lifecycle.go @@ -230,7 +230,7 @@ func (m *MicroVMRuntime) Create(_ context.Context, spec ContainerSpec) (Containe session := µvmSession{ id: id, name: spec.Name, - cfg: m.bootConfig(runtimeDir, nonce, shared), + cfg: m.bootConfig(runtimeDir, nonce, shared, spec.UID), uid: spec.UID, env: spec.Env, nonce: nonce, @@ -281,7 +281,7 @@ func (m *MicroVMRuntime) Create(_ context.Context, spec ContainerSpec) (Containe // sizing, and the boot nonce carried on the cmdline as lowercase hex under the // compass.boot_nonce key guestd parses. Split out so spec→BootConfig assembly // is unit-testable without booting. -func (m *MicroVMRuntime) bootConfig(runtimeDir string, nonce []byte, shared Mount) microvm.BootConfig { +func (m *MicroVMRuntime) bootConfig(runtimeDir string, nonce []byte, shared Mount, agentUID uint32) microvm.BootConfig { return microvm.BootConfig{ Kernel: m.config.KernelImage, Initrd: m.config.InitrdImage, @@ -294,8 +294,12 @@ func (m *MicroVMRuntime) bootConfig(runtimeDir string, nonce []byte, shared Moun FSTag: workspaceFSTag, FSSocket: filepath.Join(runtimeDir, "virtiofsd.sock"), FSSharedDir: shared.HostPath, - CPUs: m.config.DefaultCPUs, - MemoryMB: m.config.DefaultMemoryMB, + // The in-guest exec uid, so virtiofsd maps it back to the invoking host + // user and the shared volume carries the same host-side ownership the + // podman --userns=keep-id path produces (record §(d) parity). + AgentUID: agentUID, + CPUs: m.config.DefaultCPUs, + MemoryMB: m.config.DefaultMemoryMB, Net: microvm.NetConfig{ VhostUserSocket: filepath.Join(runtimeDir, "net.sock"), MAC: guestMAC, diff --git a/go/internal/runtime/microvm_lifecycle_test.go b/go/internal/runtime/microvm_lifecycle_test.go index 3b6b3bf0..109ea562 100644 --- a/go/internal/runtime/microvm_lifecycle_test.go +++ b/go/internal/runtime/microvm_lifecycle_test.go @@ -43,7 +43,7 @@ func TestBootConfigAssembly(t *testing.T) { share := Mount{HostPath: "/host/checkout", ContainerPath: "/workspace"} runtimeDir := runRoot + "/microvm/sess1" - cfg := m.bootConfig(runtimeDir, nonce, share) + cfg := m.bootConfig(runtimeDir, nonce, share, 1000) if cfg.Kernel != "/img/kernel" || cfg.Initrd != "/img/initrd" || cfg.Rootfs != "/img/rootfs" { t.Fatalf("boot images = %q/%q/%q, want the config paths", cfg.Kernel, cfg.Initrd, cfg.Rootfs) @@ -54,6 +54,12 @@ func TestBootConfigAssembly(t *testing.T) { if cfg.FSTag != workspaceFSTag { t.Fatalf("FSTag = %q, want %q", cfg.FSTag, workspaceFSTag) } + // The in-guest agent uid rides the BootConfig so virtiofsd can map it back + // to the invoking host user (record §(d) host-ownership parity). Dropping it + // silently reverts the share to unmapped ownership. + if cfg.AgentUID != 1000 { + t.Fatalf("AgentUID = %d, want the spec uid 1000", cfg.AgentUID) + } if cfg.VsockCID != guestVsockCID || cfg.VsockPort != guestVsockPort { t.Fatalf("CID/port = %d/%d, want %d/%d", cfg.VsockCID, cfg.VsockPort, guestVsockCID, guestVsockPort) } diff --git a/go/internal/runtime/microvm_preflight.go b/go/internal/runtime/microvm_preflight.go index fd6409ee..ba9c7455 100644 --- a/go/internal/runtime/microvm_preflight.go +++ b/go/internal/runtime/microvm_preflight.go @@ -39,6 +39,17 @@ type preflightProbes struct { statImage func(string) error // hashImage returns the streaming lowercase-hex SHA-256 of the image file. hashImage func(string) (string, error) + // readQuota reads the active project quota scoping a volume path (D7: + // read-only, never assign). Behind the seam so the quota axis is + // hermetically testable — a prjquota-active filesystem cannot be + // provisioned rootless, so a fake reading is the only way this decision is + // covered on a dev box. + readQuota quotaReadFn + // verifySubordinateIDs resolves the invoking user's /etc/subuid range, the + // host allocation newuidmap validates virtiofsd's uid/gid mapping against. + // Behind the seam so the axis is testable on a box whose own subuid file + // cannot be arranged to fail. + verifySubordinateIDs func() error } // defaultPreflightProbes wires the real host-facing implementations behind the @@ -68,7 +79,9 @@ func defaultPreflightProbes() preflightProbes { _ = f.Close() return nil }, - hashImage: hashFileSHA256, + hashImage: hashFileSHA256, + readQuota: readVolumeQuota, + verifySubordinateIDs: microvm.VerifySubordinateIDRange, } } @@ -110,7 +123,76 @@ func (m *MicroVMRuntime) verifyMicroVMSupport(ctx context.Context, probes prefli // 4. RunRoot: set, writable, and short enough that a session's worst-case // suffixed gateway socket path fits the AF_UNIX budget (record §(b)/§(e)). - return m.verifyRunRoot(probes) + if err := m.verifyRunRoot(probes); err != nil { + return err + } + + // 5. Subordinate id range: virtiofsd's uid/gid mapping is validated by + // newuidmap against the invoking user's /etc/subuid entry, so a host with + // no subordinate range must fail HERE with the fix named — not at the first + // session boot, where virtiofsd dies before binding its socket and the + // cause surfaces only as "waiting for daemon sockets". + if err := probes.verifySubordinateIDs(); err != nil { + return fmt.Errorf("microvm preflight: %w", err) + } + + // 6. Session-volume quota (D7): under the multi-tenant profile an + // operator-provisioned project quota MUST be active on the session-volume + // filesystem, or startup fails naming the fix. Otherwise the observed + // utilization is logged and nothing gates. + return m.verifyQuota(probes) +} + +// verifyQuota runs the D7 volume-quota check against the SESSION-VOLUME +// filesystem, which is VolumeRoot — the parent dir P2's volume lifecycle mints +// per-session volumes under. It is deliberately NOT the RunRoot: that is the +// socket dir, held to a short /tmp path by the AF_UNIX sun_path budget, so a +// verdict read there is routinely about a different filesystem than the one +// sessions consume. Verification is read-only and rootless (microvm_quota.go); +// the Runner never assigns a quota. +// +// FAIL CLOSED when QuotaRequired is set and VolumeRoot is unknown: reporting a +// verdict about a filesystem that was never probed is worse than refusing, since +// it passes the multi-tenant gate on evidence from an unrelated mount. With +// QuotaRequired unset (Dogfood, single trusted tenant) an unknown volume root is +// logged and startup proceeds — an absent quota is the documented posture there. +// +// No meter is registered here; the coherent metric set is V7's. +func (m *MicroVMRuntime) verifyQuota(probes preflightProbes) error { + if m.config.VolumeRoot == "" { + if m.config.QuotaRequired { + return errors.New( + "microvm preflight: session-volume quota is required, but the session-volume filesystem " + + "cannot be identified at startup; set --microvm-volume-root or $COMPASS_MICROVM_VOLUME_ROOT " + + "to the parent dir session volumes are minted under (the run-root is the socket dir and is " + + "routinely a different filesystem, so it is not a valid proxy)") + } + slog.Warn("microvm preflight: session-volume quota is not verified; no volume root is configured", + "fix", "set --microvm-volume-root or $COMPASS_MICROVM_VOLUME_ROOT") + return nil + } + reading, err := verifyVolumeQuota(m.config.VolumeRoot, VolumeQuota{}, probes.readQuota) + if err != nil { + if m.config.QuotaRequired { + return err + } + slog.Warn("microvm preflight: session-volume quota is not verified; the single-tenant profile ships no host-enforced quota", + "volume_root", m.config.VolumeRoot, "reason", err) + return nil + } + // used_ratio is logged ONLY on an active bound, where its denominator is the + // project's own limit and its numerator the project's own usage. With no + // quota projected, statfs reports the whole filesystem, so the same + // expression would silently mean "how full is the host disk" — a different + // number under one key. The raw pair is logged instead, so V7 inherits a + // single-meaning ratio (microvm_quota_linux.go on the dual meaning). + slog.Info("microvm preflight: session-volume project quota is active", + "volume_root", m.config.VolumeRoot, + "limit_bytes", reading.LimitBytes, + "used_bytes", reading.UsedBytes, + "used_ratio", reading.UsedRatio(), + "required", m.config.QuotaRequired) + return nil } // imageKnob names the flag and env knob that sets one guest image path, for the diff --git a/go/internal/runtime/microvm_preflight_test.go b/go/internal/runtime/microvm_preflight_test.go index b921f91a..67e7a6ac 100644 --- a/go/internal/runtime/microvm_preflight_test.go +++ b/go/internal/runtime/microvm_preflight_test.go @@ -17,8 +17,8 @@ import ( ) // okProbes returns a preflightProbes whose every axis passes: KVM opens, the -// trio resolves at floor, and images stat + hash cleanly. Individual rows -// override the axis they exercise. +// trio resolves at floor, images stat + hash cleanly, and the volume quota +// reads as an active bound. Individual rows override the axis they exercise. func okProbes() preflightProbes { return preflightProbes{ openKVM: func() error { return nil }, @@ -34,6 +34,20 @@ func okProbes() preflightProbes { }, statImage: func(string) error { return nil }, hashImage: func(string) (string, error) { return "deadbeef", nil }, + // An active project quota by default (path totals below the mount + // root's = the kernel's projection), so only a row that overrides this + // axis exercises the D7 quota leg. + readQuota: func(path string) (QuotaReading, error) { + return QuotaReading{ + Path: path, MountRoot: "/", + LimitBytes: 10 << 30, UsedBytes: 1 << 30, + LimitInodes: 1 << 20, UsedInodes: 512, + FilesystemBytes: 1 << 40, FilesystemInodes: 1 << 26, + }, nil + }, + // The invoking user's /etc/subuid range: all-green by default, so only + // the row exercising that axis fails it. + verifySubordinateIDs: func() error { return nil }, } } @@ -64,16 +78,58 @@ func okConfig(t *testing.T) MicroVMConfig { RootfsImage: rootfs, InitrdImage: initrd, RunRoot: runRoot, + // A VolumeRoot distinct from RunRoot is the production shape: the + // session-volume filesystem is the durable D9 volume, while RunRoot is + // the short /tmp socket dir. The quota axis targets THIS path. + VolumeRoot: dir, + } +} + +// preflightRow is one verifyMicroVMSupport failure-axis case: mutate the +// all-green config/probes to break (or re-pose) exactly one axis, then assert +// the verdict. Shared by the capability table and the D7 quota table, which are +// separate functions only because one table covering every axis outgrows the +// funlen budget. +type preflightRow struct { + name string + mutate func(cfg *MicroVMConfig, p *preflightProbes) + wantOK bool + wantParts []string +} + +// runPreflightRows drives each row from the all-green baseline: an OK row must +// return nil, and a failing row must name every wantParts fragment so the D3 +// "name the missing capability and the fix" contract is asserted, not just the +// existence of an error. +func runPreflightRows(t *testing.T, rows []preflightRow) { + t.Helper() + for _, tt := range rows { + t.Run(tt.name, func(t *testing.T) { + cfg := okConfig(t) + probes := okProbes() + tt.mutate(&cfg, &probes) + m := NewMicroVMRuntime(cfg) + err := m.verifyMicroVMSupport(t.Context(), probes) + if tt.wantOK { + if err != nil { + t.Fatalf("verifyMicroVMSupport = %v, want nil", err) + } + return + } + if err == nil { + t.Fatalf("verifyMicroVMSupport = nil, want error mentioning %v", tt.wantParts) + } + for _, part := range tt.wantParts { + if !strings.Contains(err.Error(), part) { + t.Errorf("error %q does not name %q", err.Error(), part) + } + } + }) } } func TestVerifyMicroVMSupport(t *testing.T) { - tests := []struct { - name string - mutate func(cfg *MicroVMConfig, p *preflightProbes) - wantOK bool - wantParts []string - }{ + runPreflightRows(t, []preflightRow{ { name: "all green", mutate: func(_ *MicroVMConfig, _ *preflightProbes) {}, @@ -170,30 +226,127 @@ func TestVerifyMicroVMSupport(t *testing.T) { }, wantParts: []string{"not creatable/writable"}, }, + { + // The /etc/subuid axis: virtiofsd's mapping is validated by + // newuidmap against the invoking user's subordinate range, so a + // host without one must fail HERE rather than at the first boot as + // an opaque daemon-socket timeout. + name: "no subordinate uid range for the invoking user", + mutate: func(_ *MicroVMConfig, p *preflightProbes) { + p.verifySubordinateIDs = func() error { + return errors.New("virtiofsd id-mapping requires a subordinate uid range for mattw in /etc/subuid") + } + }, + wantParts: []string{"/etc/subuid", "subordinate uid range"}, + }, + }) +} + +// TestVerifyMicroVMSupportQuota is the D7 session-volume quota axis: with +// QuotaRequired set, an active project quota passes and an absent one is a +// startup error naming the volume and the operator fix; with it unset (Dogfood's +// single trusted tenant) neither an absent quota nor an unreadable one gates. +// The unquota'd readings below are the real shape a plain filesystem produces — +// statfs at the path and at its mount root report the same totals, so nothing is +// projected. +func TestVerifyMicroVMSupportQuota(t *testing.T) { + unquotad := func(path string) (QuotaReading, error) { + return QuotaReading{ + Path: path, MountRoot: "/", + LimitBytes: 1 << 40, UsedBytes: 1 << 30, + FilesystemBytes: 1 << 40, + }, nil } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - cfg := okConfig(t) - probes := okProbes() - tt.mutate(&cfg, &probes) - m := NewMicroVMRuntime(cfg) - err := m.verifyMicroVMSupport(t.Context(), probes) - if tt.wantOK { - if err != nil { - t.Fatalf("verifyMicroVMSupport = %v, want nil", err) + unreadable := func(string) (QuotaReading, error) { + return QuotaReading{}, errors.New("statfs: permission denied") + } + + runPreflightRows(t, []preflightRow{ + { + name: "quota present and under limit, required, passes", + mutate: func(cfg *MicroVMConfig, _ *preflightProbes) { + cfg.QuotaRequired = true + }, + wantOK: true, + }, + { + name: "quota absent and required fails naming the volume and the fix", + mutate: func(cfg *MicroVMConfig, p *preflightProbes) { + cfg.QuotaRequired = true + p.readQuota = unquotad + }, + wantParts: []string{"no enforced project quota", "prjquota", "quota-required off"}, + }, + { + name: "quota absent and NOT required passes (Dogfood single tenant)", + mutate: func(_ *MicroVMConfig, p *preflightProbes) { + p.readQuota = unquotad + }, + wantOK: true, + }, + { + name: "quota probe failure with required set fails with the read cause", + mutate: func(cfg *MicroVMConfig, p *preflightProbes) { + cfg.QuotaRequired = true + p.readQuota = unreadable + }, + wantParts: []string{"permission denied"}, + }, + { + name: "quota probe failure without required set passes", + mutate: func(_ *MicroVMConfig, p *preflightProbes) { + p.readQuota = unreadable + }, + wantOK: true, + }, + { + // FAIL CLOSED: with no VolumeRoot the session-volume filesystem is + // unidentified, and the RunRoot is NOT an acceptable proxy — it is + // the short /tmp socket dir, routinely a different filesystem. A + // verdict about a filesystem that was never probed is worse than a + // refusal, so the required profile refuses. + name: "quota required but the volume root is unknown fails closed", + mutate: func(cfg *MicroVMConfig, p *preflightProbes) { + cfg.QuotaRequired = true + cfg.VolumeRoot = "" + p.readQuota = func(string) (QuotaReading, error) { + t.Error("readQuota must NOT be called with no volume root: probing the run-root proxy " + + "would report a verdict about a filesystem the session volumes do not live on") + return QuotaReading{}, nil } - return - } - if err == nil { - t.Fatalf("verifyMicroVMSupport = nil, want error mentioning %v", tt.wantParts) - } - for _, part := range tt.wantParts { - if !strings.Contains(err.Error(), part) { - t.Errorf("error %q does not name %q", err.Error(), part) + }, + wantParts: []string{"cannot be identified at startup", "--microvm-volume-root", "COMPASS_MICROVM_VOLUME_ROOT"}, + }, + { + name: "no volume root without required set passes with a warning", + mutate: func(cfg *MicroVMConfig, _ *preflightProbes) { + cfg.VolumeRoot = "" + }, + wantOK: true, + }, + { + // The probe must target VolumeRoot, never RunRoot: they are + // different filesystems in production, and a quota verdict read on + // the socket dir says nothing about the durable volume. + name: "the quota probe targets the volume root, not the run root", + mutate: func(cfg *MicroVMConfig, p *preflightProbes) { + cfg.QuotaRequired = true + wantPath := cfg.VolumeRoot + runRoot := cfg.RunRoot + p.readQuota = func(path string) (QuotaReading, error) { + if path != wantPath { + t.Errorf("readQuota probed %q, want the volume root %q (run root is %q)", path, wantPath, runRoot) + } + return QuotaReading{ + Path: path, MountRoot: "/", + LimitBytes: 10 << 30, UsedBytes: 1 << 30, + FilesystemBytes: 1 << 40, + }, nil } - } - }) - } + }, + wantOK: true, + }, + }) } // TestVerifyMicroVMSupportManifest covers the (d) hash-verification axis: a diff --git a/go/internal/runtime/microvm_quota.go b/go/internal/runtime/microvm_quota.go new file mode 100644 index 00000000..bbd6b8bd --- /dev/null +++ b/go/internal/runtime/microvm_quota.go @@ -0,0 +1,214 @@ +package runtime + +// microvm_quota.go is V6's session-volume quota VERIFICATION path: the expected +// bound (VolumeQuota), what a host probe observed (QuotaReading), and the pure +// decision that turns a reading into a startup verdict (verifyVolumeQuota). +// +// Per D7 the Runner NEVER assigns quota. Project-quota assignment +// (FS_IOC_FSSETXATTR + quotactl) and the loopback-image fallback (mount(2)) both +// need CAP_SYS_ADMIN the rootless Runner lacks, and the frozen no-copy invariant +// forbids swapping the virtio-fs volume for a quota-bounded block device. So the +// multi-tenant deployment provisions per-directory project quota on the +// session-volume filesystem via operator IaC at deploy, and this file's job is +// exactly one read-only, rootless-safe question: *is that quota active?* +// +// Mechanism — statvfs-derived, deliberately NOT quotactl (record §(d) leaves the +// choice to "the rootless-safe mechanism"). The obvious reads both fail rootless +// or answer the wrong question: +// - quotactl(Q_XGETQUOTA, PRJQUOTA, …) returns the real limits AND usage, but +// the kernel gates a non-self quota id on CAP_SYS_ADMIN, and a project id is +// never "self" — so it EPERMs for exactly the caller this check exists for. +// - FS_IOC_FSGETXATTR yields the directory's project id, but a project id is +// only a *label*: it is set identically whether or not the filesystem is +// mounted with enforcement on, so it cannot answer "is the bound live". +// +// statfs(2) answers both at once, unprivileged. XFS (xfs_qm_statvfs, gated on +// project quota ACCT+ENFD && a non-zero project id && FS_XFLAG_PROJINHERIT) and +// ext4 (ext4_statfs_project, gated on the prjquota mount option, PROJINHERIT and +// a non-zero block hard limit) both REWRITE the statfs block/inode totals of a +// project-quota'd directory to the project's limit and usage. So a statfs on the +// volume that reports SMALLER totals than a statfs at its mount root is precisely +// the kernel telling us an enforced project quota is scoping this subtree — and +// the same call hands back the utilization V7 will meter. No syscall wrapper, no +// unsafe, no capability. +// +// The syscall half lives in microvm_quota_linux.go (with a named-refusal stub in +// microvm_quota_unsupported.go); everything here is pure and hermetically tested. + +import "fmt" + +// VolumeQuota is the EXPECTED bound on a session-volume filesystem: a byte +// ceiling and an inode ceiling. It is the operator-declared bound the preflight +// compares an observed reading against, and the target V7's +// compass_microvm_quota_used_ratio meters against. A zero field means +// unbounded/unknown — a zero VolumeQuota asks the presence-only question ("is +// *some* enforced quota active"), which is what the QuotaRequired preflight +// poses, since MicroVMConfig carries the required-ness flag and not a number. +type VolumeQuota struct { + // Bytes is the expected block ceiling in bytes; zero means unspecified. + Bytes int64 + // Inodes is the expected inode ceiling; zero means unspecified. + Inodes int64 +} + +// QuotaReading is one rootless statfs observation of a volume path: the totals +// the kernel reports FOR THAT PATH (rewritten to the project bound when an +// enforced project quota scopes it) alongside the same totals read at the +// containing mount root (never rewritten). The pair is what makes an active +// quota detectable without a capability: they are identical on an unquota'd +// tree and diverge exactly when the kernel projected a quota onto the path. +type QuotaReading struct { + // Path is the volume path that was probed. + Path string + // MountRoot is the mount point the path resolves into — the reference the + // path's own totals are compared against. + MountRoot string + // LimitBytes is the block total statfs reports for Path: the project's byte + // bound under an enforced project quota, else the whole filesystem's size. + LimitBytes int64 + // UsedBytes is the consumed bytes within LimitBytes' scope. + UsedBytes int64 + // LimitInodes is the inode total statfs reports for Path (0 on filesystems + // that do not report an inode count, e.g. btrfs). + LimitInodes int64 + // UsedInodes is the consumed inodes within LimitInodes' scope. + UsedInodes int64 + // FilesystemBytes is the block total at MountRoot — the unprojected size. + FilesystemBytes int64 + // FilesystemInodes is the inode total at MountRoot — unprojected, and on + // XFS an ESTIMATE rather than a fixed filesystem property: xfs_statfs_inodes + // computes f_files = min(icount + fakeinos, XFS_MAXINUMBER) where + // fakeinos = XFS_FSB_TO_INO(mp, f_bfree) (fs/xfs/xfs_super.c), because XFS + // allocates inodes on demand — so it SHRINKS as the filesystem fills and is + // read at a different instant than LimitInodes. Active()'s inode arm carries + // a margin for exactly this. + FilesystemInodes int64 +} + +// Active reports whether an ENFORCED project quota is scoping Path: the path's +// own statfs totals are smaller than the mount root's, which only the kernel's +// project-quota projection produces (see the file header). Either axis suffices +// — an operator may bound bytes, inodes, or both — but the two axes are +// ASYMMETRIC: the byte arm accepts any strict inequality, while the inode arm +// requires a 1/16 jitter margin (the reason is two paragraphs down). The inode +// arm is also only consulted on a filesystem that reports inode counts at all. +// +// Deliberately conservative in one direction: a project quota whose byte limit +// happens to equal the whole filesystem's size projects no observable difference +// and reads as absent. That is a bound which constrains nothing, and reading it +// as absent fails a QuotaRequired startup closed (a legible operator refusal) +// rather than passing a tenant-exhaustible volume as bounded. +// +// The INODE arm requires a MARGIN, not merely strict inequality, because the +// mount-root inode total it compares against is an XFS estimate that moves +// between the two statfs calls (FilesystemInodes' doc). Bare `<` on two samples +// of one dynamic number can go true from jitter alone — an unquota'd volume +// reading as bounded, the fail-OPEN direction on a security-relevant preflight. +// A real project inode limit is orders of magnitude below the estimate, so the +// margin costs nothing there. +// +// The arm stays INDEPENDENT of the byte arm rather than being gated behind it: +// an inode-only project quota on a filesystem whose byte limit happens to equal +// its size is a real bound, and gating would read it as absent. The residual +// case the margin cannot rescue — a nearly-full XFS whose shrinking estimate +// falls within the margin of a genuine project inode limit — reads as absent and +// therefore REFUSES a QuotaRequired startup with the observed numbers in the +// message, which is the fail-closed direction. +func (r QuotaReading) Active() bool { + if r.LimitBytes > 0 && r.FilesystemBytes > 0 && r.LimitBytes < r.FilesystemBytes { + return true + } + if r.LimitInodes <= 0 || r.FilesystemInodes <= 0 { + return false + } + return r.LimitInodes < r.FilesystemInodes-r.FilesystemInodes/inodeMarginDivisor +} + +// inodeMarginDivisor sets how much smaller than the mount-root inode estimate a +// path's inode total must be before it counts as a projected bound: 1/16 (6.25%) +// of the estimate. It is a jitter floor, not a tuned threshold — adjacent statfs +// samples of XFS's fakeinos estimate differ by far less, while a provisioned +// project inode limit is smaller by orders of magnitude. +const inodeMarginDivisor = 16 + +// UsedRatio is the observed byte utilization in [0,1] — the value the preflight +// logs and V7's compass_microvm_quota_used_ratio will meter. Zero when no limit +// was observed. +// +// DUAL MEANING, and the caller must gate on Active(): under an enforced project +// quota the kernel has rewritten both totals to the project's own limit and +// usage, so this is the tenant's utilization; with no quota projected the same +// expression is whole-FILESYSTEM utilization, including every other tenant's +// data and the OS's. Logging both under one key would give a dashboard a number +// whose denominator silently changes, so the preflight logs used_ratio ONLY when +// Active() is true and the raw used/total pair otherwise (microvm_preflight.go's +// verifyQuota). V7 inherits a single-meaning number. +// +// NOTE: this file registers NO meter; exposing the number is V6's job, owning +// the metric set is V7's. +func (r QuotaReading) UsedRatio() float64 { + if r.LimitBytes <= 0 { + return 0 + } + return float64(r.UsedBytes) / float64(r.LimitBytes) +} + +// String renders the reading as one operator-legible line, so an error or log +// carries the observed numbers rather than a struct dump. +func (r QuotaReading) String() string { + return fmt.Sprintf("path=%s mount=%s limit=%dB used=%dB inodes=%d/%d filesystem=%dB/%d inodes", + r.Path, r.MountRoot, r.LimitBytes, r.UsedBytes, r.UsedInodes, r.LimitInodes, + r.FilesystemBytes, r.FilesystemInodes) +} + +// quotaReadFn reads the active quota scoping a path. It is the effectful seam +// verifyVolumeQuota drives, mirroring preflightProbes' openKVM/statImage split, +// so the decision below is unit-testable against a fabricated reading with no +// quota'd filesystem (which cannot be provisioned rootless). +type quotaReadFn func(path string) (QuotaReading, error) + +// verifyVolumeQuota answers whether an operator-provisioned project quota is +// active on path and at least as large as want, returning the observed reading +// (for the caller's utilization log) alongside the verdict. It VERIFIES ONLY — +// no FS_IOC_FSSETXATTR, no quotactl set, no mount (D7). +// +// nil when an enforced quota is active and meets want (a zero want asks +// presence only). Otherwise a startup error naming the volume, what was +// observed, and the operator fix — the D3 "name the missing capability and the +// fix" posture, never a degrade signal. The caller decides whether an absent +// quota is fatal (QuotaRequired) or merely logged. +func verifyVolumeQuota(path string, want VolumeQuota, read quotaReadFn) (QuotaReading, error) { + if path == "" { + return QuotaReading{}, fmt.Errorf( + "microvm preflight: session-volume quota: no volume path to verify: %s", + "set --microvm-volume-root or $COMPASS_MICROVM_VOLUME_ROOT to the parent dir session volumes "+ + "are minted under (NOT the run-root: that is the socket dir, held to a short /tmp path by the "+ + "AF_UNIX sun_path budget, and routinely a different filesystem, so it is not a valid proxy)") + } + reading, err := read(path) + if err != nil { + return QuotaReading{}, fmt.Errorf("microvm preflight: reading the session-volume quota for %q: %w", path, err) + } + reading.Path = path + if !reading.Active() { + return reading, fmt.Errorf( + "microvm preflight: no enforced project quota on the session-volume filesystem at %q (observed %s): "+ + "provision a per-directory project quota on that filesystem via operator IaC "+ + "(e.g. mount it with prjquota, set a project id + FS_XFLAG_PROJINHERIT on the volume dir, and set its block/inode limits), "+ + "or run the single-tenant profile with quota-required off", + path, reading) + } + if want.Bytes > 0 && reading.LimitBytes < want.Bytes { + return reading, fmt.Errorf( + "microvm preflight: the session-volume project quota at %q bounds %d bytes, below the required %d "+ + "(observed %s): raise the provisioned project block limit", + path, reading.LimitBytes, want.Bytes, reading) + } + if want.Inodes > 0 && reading.LimitInodes < want.Inodes { + return reading, fmt.Errorf( + "microvm preflight: the session-volume project quota at %q bounds %d inodes, below the required %d "+ + "(observed %s): raise the provisioned project inode limit", + path, reading.LimitInodes, want.Inodes, reading) + } + return reading, nil +} diff --git a/go/internal/runtime/microvm_quota_linux.go b/go/internal/runtime/microvm_quota_linux.go new file mode 100644 index 00000000..a155816e --- /dev/null +++ b/go/internal/runtime/microvm_quota_linux.go @@ -0,0 +1,162 @@ +//go:build linux + +package runtime + +// The syscall half of V6's quota verification: the rootless statfs(2) probe +// microvm_quota.go's pure decision consumes. Mechanism and the rejected +// alternatives (quotactl, FS_IOC_FSGETXATTR): microvm_quota.go's header. +// +// Linux-only because the project-quota-projected statvfs behavior it reads is a +// Linux XFS/ext4 kernel property (xfs_qm_statvfs / ext4_statfs_project); +// microvm_quota_unsupported.go carries the named refusal for every other unix. + +import ( + "fmt" + "path/filepath" + "syscall" +) + +// readVolumeQuota is the production quotaReadFn: it statfs(2)es the volume path +// and, separately, the mount root the path resolves into, so the pure decision +// can compare the two. On a project-quota'd subtree the kernel rewrites the +// path's block/inode totals to the project's limit and usage while the mount +// root's stay the filesystem's real size — so the divergence IS the proof an +// enforced quota is active, readable with no capability (see microvm_quota.go's +// header for why quotactl and FS_IOC_FSGETXATTR are both the wrong read). +func readVolumeQuota(path string) (QuotaReading, error) { + var at syscall.Statfs_t + if err := syscall.Statfs(path, &at); err != nil { + return QuotaReading{}, fmt.Errorf("statfs %q: %w", path, err) + } + root, distinct, err := mountRoot(path) + if err != nil { + return QuotaReading{}, err + } + if !distinct { + // The volume root IS the mount point, so the comparison has no + // UNPROJECTED reference: both statfs calls would target the same path, + // LimitBytes would equal FilesystemBytes identically, and Active() would + // be false BY CONSTRUCTION rather than by observation. Reporting that as + // "no quota" refuses a QuotaRequired startup on a correctly provisioned + // host, so it is an INCONCLUSIVE probe with the fix named — the same + // posture as an unreadable ancestor (mountRoot's doc). + return QuotaReading{}, fmt.Errorf( + "locating an unprojected reference for %q: that path IS the mount point of its filesystem (%q), "+ + "so there is no unquota'd ancestor to compare its statfs totals against and whether a project "+ + "quota scopes it is INDETERMINATE; point --microvm-volume-root (or "+ + "$COMPASS_MICROVM_VOLUME_ROOT) at a SUBDIRECTORY of the quota'd filesystem rather than at its "+ + "mount point — that subdirectory is also where per-session volumes are actually minted, and it "+ + "is the dir the project id and FS_XFLAG_PROJINHERIT belong on", + path, root) + } + var atRoot syscall.Statfs_t + if err := syscall.Statfs(root, &atRoot); err != nil { + return QuotaReading{}, fmt.Errorf("statfs mount root %q: %w", root, err) + } + return QuotaReading{ + Path: path, + MountRoot: root, + // LimitBytes/UsedBytes carry a DUAL MEANING by construction, per + // QuotaReading's field docs: under an enforced project quota the kernel + // has rewritten these totals to the project's own limit and usage; with + // no quota projected they are the whole filesystem's. Active() is what + // distinguishes the two, and it is why the preflight logs used_ratio + // only when Active() is true. + LimitBytes: blocksToBytes(at.Blocks, at.Bsize), + UsedBytes: blocksToBytes(at.Blocks-at.Bfree, at.Bsize), + LimitInodes: int64(at.Files), //nolint:gosec // G115: a statfs inode count is a kernel-reported magnitude, never near the int64 ceiling + UsedInodes: int64(at.Files - at.Ffree), + FilesystemBytes: blocksToBytes(atRoot.Blocks, atRoot.Bsize), + FilesystemInodes: int64(atRoot.Files), //nolint:gosec // G115: as above — a kernel-reported inode count + }, nil +} + +// blocksToBytes converts a statfs block count at the reported fragment size to +// bytes. A non-positive Bsize (never produced by a healthy filesystem, but the +// field is signed) yields 0 rather than a nonsense product, so the decision +// reads it as "no limit observed" and fails a required check closed. +func blocksToBytes(blocks uint64, bsize int64) int64 { + if bsize <= 0 { + return 0 + } + //nolint:gosec // G115: block counts × fragment size are filesystem-sized magnitudes; a real statfs cannot overflow int64 here + return int64(blocks) * bsize +} + +// mountRoot walks path's ancestors until the device number changes, returning +// the deepest ancestor still on the same filesystem — the mount point path +// belongs to — plus whether the walk actually CROSSED a device boundary to get +// there. A project quota does NOT change st_dev (it is an accounting scope +// inside one filesystem, not a separate device), so this reliably reaches the +// unprojected reference point the comparison needs. +// +// Paths are resolved through symlinks first: a symlinked volume dir would +// otherwise walk the link's lexical parents, which may live on a different +// filesystem entirely and make the comparison meaningless. +// +// distinct=false means the resolved path IS ITS OWN mount point, so there is NO +// unprojected reference to compare against — statfs'ing it twice yields +// identical totals and Active() is false BY CONSTRUCTION, not by observation. +// Verified by instrumented run: mountRoot("/tmp") and mountRoot("/") each return +// their own argument. That is the most natural production layout (a dedicated +// XFS mounted at, say, /srv/compass/volumes), so treating it as a negative +// verdict would refuse startup on a CORRECTLY provisioned host. The caller turns +// it into a distinct INCONCLUSIVE error instead — the same fail-closed-but-named +// posture as the unreadable-ancestor case below. +// +// An UNREADABLE ancestor is likewise an INCONCLUSIVE probe, not a mount root. +// The comparison is only sound against an UNPROJECTED reference, and same-device +// is not the same as unprojected: FS_XFLAG_PROJINHERIT propagates a project id +// down the tree, so a walk halted by an EACCES may stop at an ancestor inside +// the SAME project (or inside a larger enclosing one), whose totals are +// themselves rewritten. Comparing against that yields a bogus verdict — +// reporting a quota active because the reference happened to be a bigger +// project, which is the fail-OPEN direction on a security-relevant preflight. So +// the error propagates through readVolumeQuota, and a QuotaRequired startup +// fails CLOSED naming the unreadable ancestor (the posture +// TestReadVolumeQuotaAbsentPath pins for a missing path). +func mountRoot(path string) (root string, distinct bool, err error) { + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + return "", false, fmt.Errorf("resolving %q: %w", path, err) + } + dev, err := deviceOf(resolved) + if err != nil { + return "", false, err + } + current := resolved + for { + parent := filepath.Dir(current) + if parent == current { + // Reached "/" — the filesystem root is the mount root. It is a + // distinct reference only if the walk moved off the given path. + return current, current != resolved, nil + } + parentDev, err := deviceOf(parent) + if err != nil { + return "", false, fmt.Errorf( + "locating the mount root of %q: ancestor %q is not statable (%w), so no unprojected "+ + "reference point could be reached and whether a project quota scopes the volume is "+ + "INDETERMINATE; make the ancestor path traversable by the Runner uid (chmod o+x) "+ + "or place the volume root on a path the Runner can walk to its mount point", + path, parent, err) + } + if parentDev != dev { + return current, current != resolved, nil + } + current = parent + } +} + +// deviceOf returns the st_dev of path, the identity the mount-root walk compares. +// syscall.Stat (not os.Stat) because st_dev is all this needs: it fills a +// caller-owned struct with no os.FileInfo allocation and no Sys() type +// assertion, and it is the same call the readVolumeQuota statfs pair already +// uses, keeping this file on one syscall surface. +func deviceOf(path string) (uint64, error) { + var st syscall.Stat_t + if err := syscall.Stat(path, &st); err != nil { + return 0, fmt.Errorf("stat %q: %w", path, err) + } + return st.Dev, nil +} diff --git a/go/internal/runtime/microvm_quota_linux_test.go b/go/internal/runtime/microvm_quota_linux_test.go new file mode 100644 index 00000000..a75edaa7 --- /dev/null +++ b/go/internal/runtime/microvm_quota_linux_test.go @@ -0,0 +1,210 @@ +//go:build linux + +package runtime + +// Tests for the Linux-only statfs/mount-root half of V6's quota verification +// (microvm_quota_linux.go). Mechanism and the rejected alternatives: +// microvm_quota.go's header. +// +// These are separate from microvm_quota_test.go because mountRoot and deviceOf +// only exist under //go:build linux — the pure decision they feed is covered +// there, on every GOOS. + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestMountRootUnreadableAncestorIsInconclusive pins the fail-CLOSED semantics +// for a stat-blocked ancestor: the walk cannot reach an UNPROJECTED reference, +// and same-device is not the same as unprojected — FS_XFLAG_PROJINHERIT means a +// halted walk may stop inside the same project (or a larger enclosing one), +// whose totals are themselves rewritten. Comparing against that would report a +// bogus ACTIVE quota, the fail-open direction. So mountRoot returns an error +// naming the blocking path instead of a mount root, and no verdict is rendered. +// +// A 0000 ancestor is refused at path resolution rather than at the ancestor +// walk, since an EACCES that blocks stat(parent) also blocks resolving the leaf +// through it — both routes are errors, which is the point: the ONE thing that +// must never happen is a QuotaReading built against an unproven reference. +func TestMountRootUnreadableAncestorIsInconclusive(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("running as root: a 0000 dir is still traversable, so the EACCES this test needs cannot be produced") + } + base := t.TempDir() + blocked := filepath.Join(base, "blocked") + volume := filepath.Join(blocked, "volume") + if err := os.MkdirAll(volume, 0o700); err != nil { + t.Fatalf("creating volume tree: %v", err) + } + if err := os.Chmod(blocked, 0o000); err != nil { + t.Fatalf("blocking ancestor: %v", err) + } + // Restore the mode so t.TempDir's cleanup can remove the tree. + t.Cleanup(func() { _ = os.Chmod(blocked, 0o700) }) + + root, distinct, err := mountRoot(volume) + if err == nil { + t.Fatalf("mountRoot(%q) = %q (distinct=%v) with no error; a stat-blocked ancestor must be an INCONCLUSIVE probe, "+ + "not a mount root — comparing against a possibly-projected ancestor can report a bogus active quota", volume, root, distinct) + } + if root != "" { + t.Errorf("mountRoot returned the reference %q alongside its error; an inconclusive probe must yield no reference", root) + } + // The error must name the path an operator has to fix. + if !strings.Contains(err.Error(), blocked) { + t.Errorf("error %q does not name the blocking path %q", err.Error(), blocked) + } +} + +// TestMountRootResolvesAnUnblockedPath is the positive control for the walk: on +// a path the Runner can traverse to its mount point, mountRoot resolves a real +// reference AND reports that it crossed a device boundary to get there. +// +// It asserts the walk did real WORK, not merely that it returned something: the +// root must be a strict prefix ANCESTOR of the input, and a nested subdirectory +// must resolve to the SAME root. Both fail on an identity-returning mountRoot — +// which is exactly what the real one does for a path that IS a mount point, the +// degeneracy the old non-empty-and-no-error assertion could not catch. +func TestMountRootResolvesAnUnblockedPath(t *testing.T) { + base := t.TempDir() + nested := filepath.Join(base, "a", "b", "c") + if err := os.MkdirAll(nested, 0o700); err != nil { + t.Fatalf("creating the nested tree: %v", err) + } + // t.TempDir resolves through symlinks the same way mountRoot does, so the + // prefix comparison below is against the resolved form on a box where + // $TMPDIR is a symlink. + resolvedBase, err := filepath.EvalSymlinks(base) + if err != nil { + t.Fatalf("resolving the temp dir: %v", err) + } + + root, distinct, err := mountRoot(base) + if err != nil { + t.Fatalf("mountRoot on an unblocked temp dir = %v, want a resolved mount root", err) + } + if !distinct { + t.Fatalf("mountRoot(%q) reported distinct=false; a temp dir is never its own mount point, so the "+ + "device-boundary walk did not run", base) + } + // STRICT ancestor: an identity-returning implementation returns the input + // itself, which this rejects. + if root == resolvedBase { + t.Fatalf("mountRoot(%q) = %q — the input itself. The walk resolved no unprojected reference; an "+ + "identity implementation would pass a mere non-empty check", base, root) + } + if !strings.HasPrefix(resolvedBase, strings.TrimSuffix(root, "/")+"/") { + t.Fatalf("mountRoot(%q) = %q, which is not a prefix ancestor of the input", base, root) + } + // A NESTED path must land on the SAME mount root: the walk climbs to a + // device boundary, not to some depth-relative ancestor. + nestedRoot, nestedDistinct, err := mountRoot(nested) + if err != nil { + t.Fatalf("mountRoot(%q) = %v, want the same mount root as its ancestor", nested, err) + } + if !nestedDistinct { + t.Errorf("mountRoot(%q) reported distinct=false for a deeply nested path", nested) + } + if nestedRoot != root { + t.Fatalf("mountRoot(%q) = %q but mountRoot(%q) = %q; a device-boundary walk must reach the same "+ + "mount root from both (an identity implementation returns each input instead)", nested, nestedRoot, base, root) + } + t.Logf("mount-root walk: %q and %q both resolve to %q", base, nested, root) +} + +// TestMountRootRecognizesASelfReferentialPath is MED-3's core: a path that IS +// its own mount point must be reported as NON-distinct, because there is then no +// unprojected reference to compare statfs totals against. +// +// /tmp is a real mount on this box (a separate st_dev from /), so mountRoot +// returns /tmp itself — verified by instrumented run. Accepting that as a +// reference makes LimitBytes == FilesystemBytes identically, Active() false BY +// CONSTRUCTION, and a QuotaRequired startup a refusal on a host whose volume +// root is (as production layouts naturally do) the mount point of a dedicated +// quota'd filesystem. +func TestMountRootRecognizesASelfReferentialPath(t *testing.T) { + const mountPoint = "/tmp" + if _, err := os.Stat(mountPoint); err != nil { + t.Skipf("%s is not present: %v", mountPoint, err) + } + resolved, err := filepath.EvalSymlinks(mountPoint) + if err != nil { + t.Skipf("resolving %s: %v", mountPoint, err) + } + + root, distinct, err := mountRoot(mountPoint) + if err != nil { + t.Fatalf("mountRoot(%q) = %v", mountPoint, err) + } + if root != resolved { + t.Skipf("%s is not its own mount point on this box (mountRoot = %q); the self-referential case "+ + "cannot be exercised here", mountPoint, root) + } + if distinct { + t.Fatalf("mountRoot(%q) = %q with distinct=true, but the resolved path IS the returned root: no "+ + "device boundary was crossed, so there is NO unprojected reference and the comparison would be "+ + "self-referential (LimitBytes == FilesystemBytes identically, Active() false by construction)", + mountPoint, root) + } +} + +// TestReadVolumeQuotaRefusesASelfReferentialVolumeRoot is MED-3 one layer up: +// the self-referential case must propagate as a DISTINCT inconclusive error +// naming the volume-root knob and the subdirectory fix, not collapse into the +// generic "no enforced project quota" verdict — which would refuse startup on a +// correctly provisioned host and send the operator chasing a quota that is +// already there. +func TestReadVolumeQuotaRefusesASelfReferentialVolumeRoot(t *testing.T) { + const mountPoint = "/tmp" + root, distinct, err := mountRoot(mountPoint) + if err != nil { + t.Skipf("mountRoot(%q) = %v", mountPoint, err) + } + if distinct { + t.Skipf("%s is not its own mount point on this box (root %q); the self-referential case cannot be "+ + "exercised here", mountPoint, root) + } + + reading, readErr := readVolumeQuota(mountPoint) + if readErr == nil { + t.Fatalf("readVolumeQuota(%q) = %s with no error; a volume root that IS its own mount point has no "+ + "unprojected reference, so it must be INCONCLUSIVE rather than a verdict", mountPoint, reading) + } + for _, part := range []string{ + mountPoint, "IS the mount point", "INDETERMINATE", "--microvm-volume-root", "SUBDIRECTORY", + } { + if !strings.Contains(readErr.Error(), part) { + t.Errorf("error %q does not name %q", readErr.Error(), part) + } + } + t.Logf("self-referential volume root refused: %v", readErr) +} + +// TestReadVolumeQuotaPropagatesInconclusiveMountRoot is the same posture one +// layer up: readVolumeQuota must NOT swallow the inconclusive mount-root probe +// into a reading, because a reading is what Active() renders a verdict from. An +// error here is what makes a QuotaRequired startup fail closed. +func TestReadVolumeQuotaPropagatesInconclusiveMountRoot(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("running as root: a 0000 dir is still traversable, so the EACCES this test needs cannot be produced") + } + base := t.TempDir() + blocked := filepath.Join(base, "blocked") + volume := filepath.Join(blocked, "volume") + if err := os.MkdirAll(volume, 0o700); err != nil { + t.Fatalf("creating volume tree: %v", err) + } + if err := os.Chmod(blocked, 0o000); err != nil { + t.Fatalf("blocking ancestor: %v", err) + } + t.Cleanup(func() { _ = os.Chmod(blocked, 0o700) }) + + reading, err := readVolumeQuota(volume) + if err == nil { + t.Fatalf("readVolumeQuota(%q) = %s with no error; an unreachable mount root must propagate so a "+ + "required-quota startup fails closed with the real cause", volume, reading) + } +} diff --git a/go/internal/runtime/microvm_quota_test.go b/go/internal/runtime/microvm_quota_test.go new file mode 100644 index 00000000..e01726c8 --- /dev/null +++ b/go/internal/runtime/microvm_quota_test.go @@ -0,0 +1,354 @@ +//go:build unix + +package runtime + +// Hermetic unit tests for V6's volume-quota VERIFICATION (microvm_quota.go). +// Mechanism and the rejected alternatives: microvm_quota.go's header. +// +// These carry NO microvm build tag on purpose: they are what genuinely proves +// the verification logic on any box, including one where a prjquota-active +// filesystem cannot be provisioned (that needs root, D7 / Global Constraint +// "Rootless is hard"). The decision is split from the syscall behind +// quotaReadFn precisely so it is covered here rather than left to a leg that +// skips. +// +// The real statfs probe (readVolumeQuota) is exercised too, but only for what is +// honestly assertable without a quota'd filesystem: that it reads a real path, +// and that an unquota'd tree correctly reads as NOT active. A green here does +// not claim quota enforcement was proven — the guest-side ENOSPC/EDQUOT proof is +// the root-gated leg in microvm_isolation_microvm_test.go. + +import ( + "errors" + "math" + "strings" + "testing" +) + +// quotaReading builds an "active enforced quota" reading: the path's own statfs +// totals are strictly smaller than the mount root's, which is exactly what the +// kernel's project-quota projection produces (xfs_qm_statvfs / ext4_statfs_project). +func quotaReading(limitBytes, usedBytes, limitInodes, usedInodes int64) QuotaReading { + return QuotaReading{ + Path: "/srv/compass/volumes", + MountRoot: "/srv", + LimitBytes: limitBytes, + UsedBytes: usedBytes, + LimitInodes: limitInodes, + UsedInodes: usedInodes, + FilesystemBytes: 1 << 40, // 1 TiB filesystem + FilesystemInodes: 1 << 26, + } +} + +// TestQuotaReadingActive is the core detection predicate: a path whose statfs +// totals are smaller than its mount root's has an enforced project quota +// projected onto it; equal totals mean none. Either axis (bytes or inodes) can +// carry the bound, and a filesystem that reports no inode count at all (btrfs +// reports Files == 0) must not make the inode arm read as active. +func TestQuotaReadingActive(t *testing.T) { + tests := []struct { + name string + reading QuotaReading + want bool + }{ + { + name: "byte bound below filesystem size is active", + reading: quotaReading(10<<30, 1<<30, 0, 0), + want: true, + }, + { + name: "inode bound below filesystem inodes is active", + reading: quotaReading(0, 0, 1<<20, 512), + want: true, + }, + { + name: "both bounds set is active", + reading: quotaReading(10<<30, 1<<30, 1<<20, 512), + want: true, + }, + { + name: "totals equal to the mount root are not active", + // An unquota'd tree: statfs at the path and at the mount root + // report the same filesystem. This is the shape a dev box produces. + reading: QuotaReading{ + LimitBytes: 1 << 40, UsedBytes: 1 << 30, + LimitInodes: 1 << 26, UsedInodes: 4096, + FilesystemBytes: 1 << 40, FilesystemInodes: 1 << 26, + }, + want: false, + }, + { + name: "a bound equal to the whole filesystem reads as absent", + // Conservative by design: such a quota constrains nothing, and + // reading it as absent fails a required startup closed rather than + // passing a tenant-exhaustible volume off as bounded. + reading: QuotaReading{ + LimitBytes: 1 << 40, FilesystemBytes: 1 << 40, + }, + want: false, + }, + { + name: "zero inode counts (btrfs) do not read as an inode bound", + // btrfs reports Files == 0; a naive `LimitInodes < FilesystemInodes` + // would be 0 < 0 = false here, but a reading where only the + // FILESYSTEM inodes are zero must also not go active. + reading: QuotaReading{ + LimitBytes: 1 << 40, UsedBytes: 1 << 30, + LimitInodes: 0, UsedInodes: 0, + FilesystemBytes: 1 << 40, FilesystemInodes: 0, + }, + want: false, + }, + { + // The XFS fakeinos case (FilesystemInodes' doc): the mount-root + // inode total is an estimate that SHRINKS as the filesystem fills, + // so on a full-ish XFS it can dip just below a generous project + // inode limit. Bare `<` would read that jitter as a projected + // bound, which is the fail-OPEN direction; the margin rejects it. + name: "inode arm where the mount-root estimate shrank to just above the project limit", + reading: QuotaReading{ + LimitBytes: 1 << 40, UsedBytes: 1 << 39, + LimitInodes: 1_000_000, UsedInodes: 900_000, + FilesystemBytes: 1 << 40, FilesystemInodes: 1_010_000, + }, + want: false, + }, + { + // Just INSIDE the 1/16 margin: 940_000 < 1_010_000 - 63_125. This + // pins the boundary, so a change to inodeMarginDivisor is a + // deliberate edit rather than a silent widening. + name: "an inode bound past the margin is active", + reading: QuotaReading{ + LimitBytes: 1 << 40, UsedBytes: 1 << 39, + LimitInodes: 940_000, UsedInodes: 900_000, + FilesystemBytes: 1 << 40, FilesystemInodes: 1_010_000, + }, + want: true, + }, + { + // An inode-only bound must NOT be gated behind the byte arm: here + // the byte limit equals the filesystem size (so the byte arm is + // silent) while a real inode bound is projected. + name: "an inode-only bound is active even when the byte limit equals the filesystem size", + reading: QuotaReading{ + LimitBytes: 1 << 40, UsedBytes: 1 << 30, + LimitInodes: 1 << 20, UsedInodes: 512, + FilesystemBytes: 1 << 40, FilesystemInodes: 1 << 26, + }, + want: true, + }, + { + name: "the zero reading is not active", + reading: QuotaReading{}, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.reading.Active(); got != tt.want { + t.Fatalf("Active() = %v, want %v (reading: %s)", got, tt.want, tt.reading) + } + }) + } +} + +// TestQuotaReadingUsedRatio pins the utilization value V6 exposes and V7 will +// meter: used/limit, and zero (not NaN, not +Inf) when no limit was observed. +func TestQuotaReadingUsedRatio(t *testing.T) { + tests := []struct { + name string + reading QuotaReading + want float64 + }{ + {name: "quarter full", reading: quotaReading(4<<30, 1<<30, 0, 0), want: 0.25}, + {name: "empty", reading: quotaReading(4<<30, 0, 0, 0), want: 0}, + {name: "full", reading: quotaReading(4<<30, 4<<30, 0, 0), want: 1}, + // No limit must not divide by zero into NaN/+Inf — a metric consumer + // (V7) would otherwise export a poisoned sample. + {name: "no limit observed", reading: QuotaReading{UsedBytes: 1 << 30}, want: 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.reading.UsedRatio() + if math.IsNaN(got) || math.IsInf(got, 0) { + t.Fatalf("UsedRatio() = %v, want a finite value", got) + } + if got != tt.want { + t.Fatalf("UsedRatio() = %v, want %v", got, tt.want) + } + }) + } +} + +// TestVerifyVolumeQuota is the verification decision per axis, driven through +// the injected probe so no quota'd filesystem is needed: an active quota meeting +// the want passes; an absent one errors naming the volume and the operator fix; +// a present-but-too-small bound errors naming both numbers; a probe failure +// propagates rather than reading as "absent" or "fine". +func TestVerifyVolumeQuota(t *testing.T) { + const volume = "/srv/compass/volumes" + + tests := []struct { + name string + path string + want VolumeQuota + read quotaReadFn + wantOK bool + wantParts []string + }{ + { + name: "active quota, presence-only want passes", + path: volume, + want: VolumeQuota{}, + read: func(string) (QuotaReading, error) { return quotaReading(10<<30, 1<<30, 1<<20, 512), nil }, + wantOK: true, + }, + { + name: "active quota at or above the wanted bound passes", + path: volume, + want: VolumeQuota{Bytes: 8 << 30, Inodes: 1 << 19}, + read: func(string) (QuotaReading, error) { return quotaReading(10<<30, 1<<30, 1<<20, 512), nil }, + wantOK: true, + }, + { + name: "absent quota names the volume and the operator fix", + path: volume, + want: VolumeQuota{}, + read: func(string) (QuotaReading, error) { + return QuotaReading{ + LimitBytes: 1 << 40, UsedBytes: 1 << 30, + FilesystemBytes: 1 << 40, + }, nil + }, + wantParts: []string{volume, "no enforced project quota", "prjquota", "FS_XFLAG_PROJINHERIT", "quota-required off"}, + }, + { + name: "byte bound below the wanted bound names both numbers", + path: volume, + want: VolumeQuota{Bytes: 100 << 30}, + read: func(string) (QuotaReading, error) { return quotaReading(10<<30, 1<<30, 0, 0), nil }, + wantParts: []string{volume, "10737418240", "107374182400", "raise the provisioned project block limit"}, + }, + { + name: "inode bound below the wanted bound names both numbers", + path: volume, + want: VolumeQuota{Inodes: 1 << 24}, + read: func(string) (QuotaReading, error) { return quotaReading(10<<30, 1<<30, 1<<20, 512), nil }, + wantParts: []string{volume, "1048576", "16777216", "raise the provisioned project inode limit"}, + }, + { + name: "a probe failure propagates, never reads as absent-or-fine", + path: volume, + want: VolumeQuota{}, + read: func(string) (QuotaReading, error) { return QuotaReading{}, errors.New("statfs: permission denied") }, + wantParts: []string{volume, "permission denied"}, + }, + { + // The knob must be the VOLUME root, never the retired run-root: the + // run-root is the socket dir on a routinely different filesystem, so + // setting it cannot make this check pass. Pinning the run-root text + // here is what regression-locked that stale advice. + name: "an empty volume path names the volume-root knob, not the retired run-root", + path: "", + want: VolumeQuota{}, + read: func(string) (QuotaReading, error) { return quotaReading(10<<30, 0, 0, 0), nil }, + wantParts: []string{"--microvm-volume-root", "COMPASS_MICROVM_VOLUME_ROOT"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reading, err := verifyVolumeQuota(tt.path, tt.want, tt.read) + if tt.wantOK { + if err != nil { + t.Fatalf("verifyVolumeQuota = %v, want nil", err) + } + // A passing verify must hand back the utilization the caller + // logs and V7 meters, not a zero struct. + if reading.LimitBytes == 0 { + t.Fatalf("verifyVolumeQuota returned reading %+v with no limit; the caller needs the observed bound", reading) + } + return + } + if err == nil { + t.Fatalf("verifyVolumeQuota = nil, want an error mentioning %v", tt.wantParts) + } + for _, part := range tt.wantParts { + if !strings.Contains(err.Error(), part) { + t.Errorf("error %q does not name %q", err.Error(), part) + } + } + }) + } +} + +// TestVerifyVolumeQuotaNeverAssigns is the D7 posture as a structural test: the +// verification path takes a READ function and nothing else, so there is no seam +// through which it could set a project id or a limit. Driven by counting probe +// calls and asserting the reading is returned unmutated apart from Path — +// verification observes, it does not write. +func TestVerifyVolumeQuotaNeverAssigns(t *testing.T) { + reads := 0 + fixed := quotaReading(10<<30, 2<<30, 1<<20, 1024) + read := func(string) (QuotaReading, error) { + reads++ + return fixed, nil + } + + got, err := verifyVolumeQuota("/srv/compass/volumes", VolumeQuota{Bytes: 1 << 30}, read) + if err != nil { + t.Fatalf("verifyVolumeQuota = %v, want nil", err) + } + if reads != 1 { + t.Fatalf("the quota probe ran %d times, want exactly 1 (one read-only observation, D7)", reads) + } + if got.LimitBytes != fixed.LimitBytes || got.UsedBytes != fixed.UsedBytes || + got.LimitInodes != fixed.LimitInodes || got.UsedInodes != fixed.UsedInodes { + t.Fatalf("verifyVolumeQuota returned %+v, want the probed reading unchanged %+v", got, fixed) + } + if got.Path != "/srv/compass/volumes" { + t.Fatalf("returned reading Path = %q, want the verified path", got.Path) + } +} + +// TestReadVolumeQuotaOnRealPath exercises the PRODUCTION statfs probe against a +// real directory. What it can honestly assert without root is bounded but real: +// the probe succeeds, reports a plausible filesystem, resolves a mount root, and +// — since no test box's temp dir carries a project quota — reads as NOT active. +// That negative is load-bearing: it is what proves the detection does not +// false-positive and pass an unbounded volume off as quota'd. +func TestReadVolumeQuotaOnRealPath(t *testing.T) { + dir := t.TempDir() + reading, err := readVolumeQuota(dir) + if err != nil { + t.Fatalf("readVolumeQuota(%q) = %v, want a successful rootless read", dir, err) + } + if reading.LimitBytes <= 0 { + t.Fatalf("reading %s has no block total; statfs must report the filesystem size", reading) + } + if reading.MountRoot == "" { + t.Fatalf("reading %s resolved no mount root", reading) + } + if reading.UsedBytes < 0 || reading.UsedBytes > reading.LimitBytes { + t.Fatalf("reading %s has nonsensical usage", reading) + } + if reading.Active() { + t.Fatalf("reading %s reports an active project quota on a plain temp dir; "+ + "the detection must not false-positive (that would pass an unbounded volume as quota'd)", reading) + } + // The utilization the preflight logs must be finite and in range even with + // no quota — V7 meters this value. + if ratio := reading.UsedRatio(); ratio < 0 || ratio > 1 || math.IsNaN(ratio) { + t.Fatalf("UsedRatio() = %v on reading %s, want a finite ratio in [0,1]", ratio, reading) + } +} + +// TestReadVolumeQuotaAbsentPath: a path that does not exist is a probe ERROR, +// not a silent "no quota". Under QuotaRequired that difference decides whether +// startup fails with the real cause (an unreachable volume) or with a misleading +// missing-quota message. +func TestReadVolumeQuotaAbsentPath(t *testing.T) { + if _, err := readVolumeQuota(t.TempDir() + "/does-not-exist"); err == nil { + t.Fatal("readVolumeQuota on an absent path = nil error, want a failure naming the path") + } +} diff --git a/go/internal/runtime/microvm_quota_unsupported.go b/go/internal/runtime/microvm_quota_unsupported.go new file mode 100644 index 00000000..5a3ad449 --- /dev/null +++ b/go/internal/runtime/microvm_quota_unsupported.go @@ -0,0 +1,27 @@ +//go:build !linux + +package runtime + +// The non-Linux leg of V6's quota probe. The verification mechanism is a Linux +// kernel property (XFS/ext4 project-quota-projected statvfs); mechanism and the +// rejected alternatives: microvm_quota.go's header. +// +// The microVM backend only ever boots on a KVM-capable Linux host, but this +// seam must still RESOLVE on every GOOS the package is built for: +// microvm_quota.go is untagged, so readVolumeQuota — the function its +// quotaReadFn seam is wired to — has to exist under every build constraint or +// the package does not build off Linux at all. Hence a named refusal rather +// than a build break or a silent "quota active". +// +// Refusing is the fail-closed answer: QuotaRequired on a platform where the +// bound cannot be observed must be a legible startup error, never a pass. + +import "errors" + +// readVolumeQuota refuses on non-Linux: there is no rootless project-quota read +// here, so the required-quota preflight fails closed with the reason named. +func readVolumeQuota(_ string) (QuotaReading, error) { + return QuotaReading{}, errors.New( + "session-volume project-quota verification is Linux-only (it reads the kernel's " + + "project-quota-projected statfs totals); the microVM backend requires a KVM-capable Linux host") +}