feat(microvm): virtio-fs mount-ns isolation + volume-quota verification (RIG-2497) - #912
Open
rigel-mintaka wants to merge 3 commits into
Open
feat(microvm): virtio-fs mount-ns isolation + volume-quota verification (RIG-2497)#912rigel-mintaka wants to merge 3 commits into
rigel-mintaka wants to merge 3 commits into
Conversation
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Compass engineering docs preview: https://compass-runner-rig-2497-micr.compass-eng-docs.pages.dev Deployed from |
rigel-mintaka
added a commit
that referenced
this pull request
Sep 6, 2026
…, test non-vacuity (RIG-2497) Addresses the round-1 review of #912 (2 highs + 6 mediums + 4 lows fixed; the third high — `--uid-map` vs `--translate-uid` capability surface — is a design fork filed as RIG-3330, with its record-faithful hardening landed here). ## Fail-open fixes (the two acted highs) - **QuotaRequired was dead code.** `MicroVMConfig.QuotaRequired` was never wired to any operator knob, so the D7 quota gate could never fire in production — it was settable only from tests. Added `--microvm-quota-required` (+ `$COMPASS_MICROVM_QUOTA_REQUIRED`, via a new `boolOrEnv` that *refuses* an unparseable value rather than silently reading false) and threaded it into the config literal. `selectEngine` split out a testable `backendConfig()` seam; new `backend_flags_test.go` pins flag/env/precedence end to end. - **verifyQuota probed the wrong filesystem.** It targeted `RunRoot` (the short `/tmp` socket dir), not the session-volume filesystem (the durable D9 volume, arriving as `Mount.HostPath`). Added `MicroVMConfig.VolumeRoot` (+ `--microvm-volume-root`) and pointed the check at it. When `QuotaRequired` is set and `VolumeRoot` is unknown, startup now **fails closed** with a named error instead of reporting a verdict about a filesystem it never probed. ## id-map hardening (RIG-3330 interim, no mechanism swap) Kept the record's named `--uid-map`/`--gid-map` mechanism; added `--modcaps=-mknod` (the one capability with no legitimate use on a workspace share) and an explicit security-posture comment stating the namespace-scoped capability set the ns-uid-0 mapping confers (bounded to the volume subtree by the mount-ns pivot_root). The narrower `--translate-uid` alternative is tracked as fork RIG-3330. ## Correctness + test-adequacy fixes - **subuid base was hardcoded 100000** — a silent EINVAL boot failure on any host whose `/etc/subuid` range starts elsewhere. Now parsed from the invoking user's `/etc/subuid` entry (behind a pure seam), with a named error when absent, and added as a startup preflight axis so it fails legibly at startup, not at first boot's opaque socket-wait. New `launch_idmap_test.go` pins the exact argv (injecting a non-100000 base so a regression is caught). - **XFS inode-only quota false-negative** — the mount-root `f_files` is a dynamic estimate (`fakeinos` shrinks as the fs fills); added a tolerance margin + documented the dynamism so a genuinely-quota'd volume is not read as unbounded. - **mountRoot false-positive** — a stat-blocked ancestor was swallowed and could report a bogus active quota (a sibling project's bound); now an inconclusive probe that fails closed under `QuotaRequired`. - **Cross-session test vacuity** — the `ls` row asserted content that `ls` never prints, and the sweep row depended on `grep`, which the guest image does not ship, so it exited 127 having never searched. Rewrote each row's success condition to match what its command emits (a bash+awk sweep printing `path:line`, needle via env, self-`/proc` matches excluded), added the non-zero-exit assertion to every confined row, and added a control test proving the sweep finds a planted canary. - **Hardcoded exec uid** `"1000"` → `agentuid.AgentUID` in the isolation, egress, and vsock-gateway microVM suites (a routine const change would otherwise silently make every escape attempt run as an unmapped uid and pass green). - **Parity leg** now hard-fails on malformed `stat` output instead of silently skipping the host→guest direction, and its error text names the flags the code actually uses. Lows: corrected the non-Linux stub's rationale, documented `UsedRatio`'s projected-vs-whole-fs dual meaning (and log it only when a quota is active), flattened the virtiofsd argv construction, and de-duplicated the quota rationale headers to a single source. ## Verification All gates green on a real-KVM box: build+vet (default + `-tags microvm` + darwin `!linux` stub), hermetic `-race` (runtime + microvm + cmd), the KVM isolation suite (real boots — the stricter cross-session sweep genuinely walks the tree and failed twice on real self-match artifacts during development, proving it no longer passes vacuously), golangci-lint (tagged + untagged) 0 issues, nilaway clean (default + microvm). Spec-impact: none. Refs RIG-2497 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-runner/rig-2497-microvm-v6-isolation-quota
branch
from
September 6, 2026 02:25
bc65d7d to
a2aa9ec
Compare
…on (RIG-2497)
V6 of the frozen microVM Runner backend design (record § Plan > V6): per-session virtio-fs isolation proven under real KVM, plus the D7 verify-never-assign volume-quota preflight. Executes against the frozen record (`docs/designs/infra/runtime/compass-elastic-session-runtime/microvm-runner.md`, D7 + Approach (d)).
## What lands
- **Quota verification (D7: verify, never assign).** `VolumeQuota{Bytes,Inodes}` (the expected bound), `verifyVolumeQuota(path, want, probe) → (QuotaReading, error)`, and `MicroVMConfig.QuotaRequired` (appended, additive). The Runner never *assigns* quota (that needs `CAP_SYS_ADMIN` it lacks); it only *verifies* an operator-provisioned project quota is active — a rootless, read-only check wired into `verifyMicroVMSupport` as step 5. When `QuotaRequired` is set (multi-tenant profile) an absent quota fails startup naming the volume + fix; unset (Dogfood single-tenant) it logs utilization and proceeds.
- **The rootless quota read is `statfs(2)`-derived, not `quotactl`.** `quotactl(Q_XGETQUOTA, PRJQUOTA)` EPERMs rootless (the kernel gates a non-self quota id on `CAP_SYS_ADMIN`); `FS_IOC_FSGETXATTR` yields only the project-id label, not whether enforcement is live. On XFS and ext4 the kernel rewrites a project-quota'd directory's `statfs` totals to the project's limit+usage, so path-totals < mount-root-totals *is* the kernel reporting an enforced quota — answered unprivileged, with the utilization in the same call.
- **The isolation-proving KVM suite.** Real guests booted under KVM: traversal-confined (dot-dot, symlink-to-host, absolute-path, deep-dot-dot — all blocked), cross-session-unreachable (two live guests; A cannot reach B's volume by any vector), and host-ownership parity (guest-authored files land host-side `uid:gid` matching podman's `--userns=keep-id` target). The real ENOSPC/EDQUOT-in-guest leg is honestly gated (`requireQuotaFS`): it skips where no quota'd filesystem + root exist (a `COMPASS_REQUIRE_QUOTA_FS=1` hard-fail switch for the CI/managed profile), never fake-passing. The verification *logic* is proven by the hermetic unit tests, which run everywhere.
## Load-bearing fix: guest writes to /workspace were broken
The parity test surfaced a real defect masked because no prior test wrote to the volume: under `--sandbox=namespace` alone, rootless virtiofsd could not become namespace-root (`Couldn't set the process uid as root: -1`), so it could not chown a newly created inode to the requesting guest id — **every** guest create on the share failed `EINVAL`. The share was effectively read-only to the guest.
The fix is the record's named mechanism (§(d): "virtiofsd does its own uid/gid translation via that userns"): `--uid-map`/`--gid-map` map a subordinate id to namespace-root (so the daemon can chown) and the agent id to the invoking host `(uid, gid)`. gid maps to the host *gid*, not the uid — the guest agent runs `uid==gid`, but a host user's gid differs (e.g. `1000:100`), and collapsing gid onto uid is exactly the parity break the test detects. Rootless throughout (a `/etc/subuid` subordinate id, no `CAP_SYS_ADMIN`); `--uid-map` over `--translate-uid` also keeps POSIX ACLs available on the share.
`BootConfig.AgentUID` carries the guest uid (zero disables the mapping, preserving the V2a spike harness); `Create` threads `spec.UID` through.
## Verification
All gates green on a real-KVM box (Intel, nested virt): build+vet (default + `-tags microvm`, darwin cross-build of the non-linux stub), hermetic `-race` suite, the KVM isolation suite (real boots, `COMPASS_REQUIRE_MICROVM=1`), full tagged regression (boot/contract/egress/Q-budget unregressed), golangci-lint (tagged + untagged) 0 issues, nilaway clean (default + microvm).
No OTel metric registration (V7 owns the metric set — V6 only exposes `QuotaReading.UsedRatio()`), no teardown/reap (V7), no benchmark (V8). `go/internal/vfs` untouched. `ContainerRuntime` interface unchanged.
Spec-impact: none. Refs RIG-2497
Co-authored-by: Matt Wilkinson <matt@rigel.build>
…, test non-vacuity (RIG-2497) Addresses the round-1 review of #912 (2 highs + 6 mediums + 4 lows fixed; the third high — `--uid-map` vs `--translate-uid` capability surface — is a design fork filed as RIG-3330, with its record-faithful hardening landed here). ## Fail-open fixes (the two acted highs) - **QuotaRequired was dead code.** `MicroVMConfig.QuotaRequired` was never wired to any operator knob, so the D7 quota gate could never fire in production — it was settable only from tests. Added `--microvm-quota-required` (+ `$COMPASS_MICROVM_QUOTA_REQUIRED`, via a new `boolOrEnv` that *refuses* an unparseable value rather than silently reading false) and threaded it into the config literal. `selectEngine` split out a testable `backendConfig()` seam; new `backend_flags_test.go` pins flag/env/precedence end to end. - **verifyQuota probed the wrong filesystem.** It targeted `RunRoot` (the short `/tmp` socket dir), not the session-volume filesystem (the durable D9 volume, arriving as `Mount.HostPath`). Added `MicroVMConfig.VolumeRoot` (+ `--microvm-volume-root`) and pointed the check at it. When `QuotaRequired` is set and `VolumeRoot` is unknown, startup now **fails closed** with a named error instead of reporting a verdict about a filesystem it never probed. ## id-map hardening (RIG-3330 interim, no mechanism swap) Kept the record's named `--uid-map`/`--gid-map` mechanism; added `--modcaps=-mknod` (the one capability with no legitimate use on a workspace share) and an explicit security-posture comment stating the namespace-scoped capability set the ns-uid-0 mapping confers (bounded to the volume subtree by the mount-ns pivot_root). The narrower `--translate-uid` alternative is tracked as fork RIG-3330. ## Correctness + test-adequacy fixes - **subuid base was hardcoded 100000** — a silent EINVAL boot failure on any host whose `/etc/subuid` range starts elsewhere. Now parsed from the invoking user's `/etc/subuid` entry (behind a pure seam), with a named error when absent, and added as a startup preflight axis so it fails legibly at startup, not at first boot's opaque socket-wait. New `launch_idmap_test.go` pins the exact argv (injecting a non-100000 base so a regression is caught). - **XFS inode-only quota false-negative** — the mount-root `f_files` is a dynamic estimate (`fakeinos` shrinks as the fs fills); added a tolerance margin + documented the dynamism so a genuinely-quota'd volume is not read as unbounded. - **mountRoot false-positive** — a stat-blocked ancestor was swallowed and could report a bogus active quota (a sibling project's bound); now an inconclusive probe that fails closed under `QuotaRequired`. - **Cross-session test vacuity** — the `ls` row asserted content that `ls` never prints, and the sweep row depended on `grep`, which the guest image does not ship, so it exited 127 having never searched. Rewrote each row's success condition to match what its command emits (a bash+awk sweep printing `path:line`, needle via env, self-`/proc` matches excluded), added the non-zero-exit assertion to every confined row, and added a control test proving the sweep finds a planted canary. - **Hardcoded exec uid** `"1000"` → `agentuid.AgentUID` in the isolation, egress, and vsock-gateway microVM suites (a routine const change would otherwise silently make every escape attempt run as an unmapped uid and pass green). - **Parity leg** now hard-fails on malformed `stat` output instead of silently skipping the host→guest direction, and its error text names the flags the code actually uses. Lows: corrected the non-Linux stub's rationale, documented `UsedRatio`'s projected-vs-whole-fs dual meaning (and log it only when a quota is active), flattened the virtiofsd argv construction, and de-duplicated the quota rationale headers to a single source. ## Verification All gates green on a real-KVM box: build+vet (default + `-tags microvm` + darwin `!linux` stub), hermetic `-race` (runtime + microvm + cmd), the KVM isolation suite (real boots — the stricter cross-session sweep genuinely walks the tree and failed twice on real self-match artifacts during development, proving it no longer passes vacuously), golangci-lint (tagged + untagged) 0 issues, nilaway clean (default + microvm). Spec-impact: none. Refs RIG-2497 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…ness, mount-point quota degeneracy (RIG-2497) Round 2 of the #912 review returned 2 highs / 4 mediums / 5 lows, all folded here. The two highs are defects the round-1 fix code itself introduced or left latent, and both fail in the direction that matters: silently, on a host that differs from this one. ## Fail-silent fixes (the highs) - **`/etc/subgid` is a separate allocation.** Round 1 correctly stopped hardcoding the subordinate base, but read only `/etc/subuid` and fed that one value into *both* `--uid-map` and `--gid-map` — while `newgidmap` validates the gid range against `/etc/subgid`, an independently-editable file (`usermod --add-subuids` and `--add-subgids` are separate flags; the code's own remediation text conceded they can diverge). A host whose two ranges differ would die in virtiofsd's gid map at first boot with no useful diagnostic. Both files are now read independently through a path-injected core, the two arms carry their own bases, and `VerifySubordinateIDRange` resolves both so a missing subgid range fails at *startup* naming the file and the fix. The false "allocated in lockstep" claim is gone. This box cannot observe the bug (both files read `mattw:100000:65536`), so the regression test injects divergent bases as fixtures rather than reading the host. - **Readiness ignored liveness.** `waitForSockets` polled only for socket-path existence, and virtiofsd binds its socket *before* the id-map step that can fail — so any mapping failure left the socket on disk, readiness returned nil, and cloud-hypervisor was started against a dead virtiofsd, surfacing as an inscrutable vhost-user error instead of virtiofsd's own message. Readiness is now liveness-aware and its error carries the daemon's name and log tail. Doing this properly required fixing the supervision model underneath it: only the VMM had a reaper, virtiofsd and passt were `Wait`ed lazily during teardown, and liveness was tracked by an atomic set by whichever path happened to `Wait` first — so a poll-loop check would have raced a second `Wait`. There is now exactly one reaper per child, installed at spawn, with `waitErr` published before the `exited` channel closes as the happens-before edge. `Running()` checks exit *first*, since a zombie still answers `Signal(0)`. ## Correctness - **Quota verification refused startup on a correctly-provisioned host.** `mountRoot` never checked whether the path it was handed *is* the mount point — `mountRoot("/tmp") == "/tmp"` — so when the volume root is the mount point of a dedicated quota'd filesystem (the natural production layout), the probe compared a filesystem against itself and `Active()` was false by construction. That case is now a distinct inconclusive verdict naming the fix (point `--microvm-volume-root` at a subdirectory), not a false negative. The former "positive control" would have passed an identity-returning implementation; it now asserts the walk actually climbs and converges from two depths. - The empty-path error still named the retired `--microvm-runroot` knob, with its own test pinning the stale text — a defect regression-locked by its test. Both corrected. ## Test cost and honesty - The cross-session sweep forked one `awk` per file across the whole guest filesystem: ~72s against a hard 120s exec cap, i.e. a flake waiting for a loaded box. Batched (one `awk` per 200 files) and scoped to the trees that discriminate, it now runs in milliseconds. Batching made the existing control partly vacuous — it never fills a batch — so a cross-batch test covers the mid-loop flush and the accumulator, with needles planted in both the first and final batch. Every confined row is now timed against the cap so creep is caught rather than rediscovered. - The quota-fill leg wrote the entire project limit plus 64MiB, which against a realistic quota would blow the exec cap and report a timeout instead of the `EDQUOT` verdict it exists to prove. It now fills only remaining headroom plus margin, and refuses with a named reason when the volume is too large to be a purpose-sized test quota. Lows: pinned the `--modcaps` literal in a shared constant (virtiofsd silently accepts misspelled capability names, so the argv assertion is the only guard), reworded the provisioning hint so `100000` reads as an example rather than canonical, and documented `Active()`'s deliberate byte/inode asymmetry. `boolOrEnv`'s precedence is unchanged and now documented as a contract — an explicit `--microvm-quota-required=false` does not override an env true, which fails toward leaving the multi-tenant gate on — with a test pinning that direction. ## Verification Gates green on a real-KVM box: build + vet (default, `-tags microvm`, darwin `!linux` stub), hermetic `-race`, the KVM suite under `COMPASS_REQUIRE_MICROVM=1` (real boots), golangci-lint 0 issues both tag sets, nilaway clean both. The load-bearing tests are mutation-verified: reverting the liveness check to path-existence-only reproduces the finding's exact failure, and an identity-returning `mountRoot` fails the strengthened control while passing the old one. Spec-impact: none. Refs RIG-2497 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-runner/rig-2497-microvm-v6-isolation-quota
branch
from
September 6, 2026 04:08
a2aa9ec to
3b798cc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
V6 of the frozen microVM Runner backend design (record § Plan > V6): per-session virtio-fs isolation proven under real KVM, plus the D7 verify-never-assign volume-quota preflight. Executes against the frozen record (
docs/designs/infra/runtime/compass-elastic-session-runtime/microvm-runner.md, D7 + Approach (d)).What lands
VolumeQuota{Bytes,Inodes}(the expected bound),verifyVolumeQuota(path, want, probe) → (QuotaReading, error), andMicroVMConfig.QuotaRequired(appended, additive). The Runner never assigns quota (that needsCAP_SYS_ADMINit lacks); it only verifies an operator-provisioned project quota is active — a rootless, read-only check wired intoverifyMicroVMSupportas step 5. WhenQuotaRequiredis set (multi-tenant profile) an absent quota fails startup naming the volume + fix; unset (Dogfood single-tenant) it logs utilization and proceeds.statfs(2)-derived, notquotactl.quotactl(Q_XGETQUOTA, PRJQUOTA)EPERMs rootless (the kernel gates a non-self quota id onCAP_SYS_ADMIN);FS_IOC_FSGETXATTRyields only the project-id label, not whether enforcement is live. On XFS and ext4 the kernel rewrites a project-quota'd directory'sstatfstotals to the project's limit+usage, so path-totals < mount-root-totals is the kernel reporting an enforced quota — answered unprivileged, with the utilization in the same call.uid:gidmatching podman's--userns=keep-idtarget). The real ENOSPC/EDQUOT-in-guest leg is honestly gated (requireQuotaFS): it skips where no quota'd filesystem + root exist (aCOMPASS_REQUIRE_QUOTA_FS=1hard-fail switch for the CI/managed profile), never fake-passing. The verification logic is proven by the hermetic unit tests, which run everywhere.Load-bearing fix: guest writes to /workspace were broken
The parity test surfaced a real defect masked because no prior test wrote to the volume: under
--sandbox=namespacealone, rootless virtiofsd could not become namespace-root (Couldn't set the process uid as root: -1), so it could not chown a newly created inode to the requesting guest id — every guest create on the share failedEINVAL. The share was effectively read-only to the guest.The fix is the record's named mechanism (§(d): "virtiofsd does its own uid/gid translation via that userns"):
--uid-map/--gid-mapmap a subordinate id to namespace-root (so the daemon can chown) and the agent id to the invoking host(uid, gid). gid maps to the host gid, not the uid — the guest agent runsuid==gid, but a host user's gid differs (e.g.1000:100), and collapsing gid onto uid is exactly the parity break the test detects. Rootless throughout (a/etc/subuidsubordinate id, noCAP_SYS_ADMIN);--uid-mapover--translate-uidalso keeps POSIX ACLs available on the share.BootConfig.AgentUIDcarries the guest uid (zero disables the mapping, preserving the V2a spike harness);Createthreadsspec.UIDthrough.Verification
All gates green on a real-KVM box (Intel, nested virt): build+vet (default +
-tags microvm, darwin cross-build of the non-linux stub), hermetic-racesuite, the KVM isolation suite (real boots,COMPASS_REQUIRE_MICROVM=1), full tagged regression (boot/contract/egress/Q-budget unregressed), golangci-lint (tagged + untagged) 0 issues, nilaway clean (default + microvm).No OTel metric registration (V7 owns the metric set — V6 only exposes
QuotaReading.UsedRatio()), no teardown/reap (V7), no benchmark (V8).go/internal/vfsuntouched.ContainerRuntimeinterface unchanged.Spec-impact: none. Refs RIG-2497
Co-authored-by: Matt Wilkinson matt@rigel.build