fix: seven newcomer passes — a false-positive audit event, a hardcoded isolation claim, and eleven other defects found by installing this from scratch

[convee]

Seven newcomer passes over this repository — clone, read the docs, install, deploy, use it — fixing what each pass turned up and re-running from a fresh clone until a pass found nothing. Then a benchmark run and an isolation-backend assessment, which turned up three more.

Every fix has a test, and every new assertion was mutation-checked: the old behaviour was put back and the test had to go red. Where a claim could be settled by measurement rather than reasoning, it was.

Onboarding, as measured

From a fresh clone on a 4-core Linux host: make bootstrap 13s, make verify 122s, 0 manual interventions, 870 tests. The source-level experience is genuinely good. The wall is the cluster: make up-local needs Lima + /dev/kvm + 10.5 GiB free memory + 35 GiB free disk, and make doctor stops honestly before wasting anyone's time. There is no lighter shipped profile, so a machine without nested virtualisation cannot run the product at all.

For the record, overlays/rwo-single-node does work on a single-node kind cluster with three adjustments — empty SANDBOX_RUNTIME_CLASS, the sandbox.hullwork.com/node-role=runtime label, and a restricted-PSS-compliant PostgreSQL. Everything below was verified against that deployment with a real PostgreSQL and a real S3 (MinIO).

Correctness

A denial audit event that could only ever be a false positive. /v1/workspaces/resolve derives its Workspace ID from the caller's own tenant (ws:v2:<tenant>:…), so ownership can never fail there for the reason the audit exists to record — yet a miss filed workspace.access / denied, field-for-field identical to a real cross-tenant probe. One per Workspace ever created, burying the signal. Live evidence after the fix: five Workspaces created, one denied row — the genuine cross-tenant read.

sandbox_status asserted gVisor isolation from a constant. mcp.py merged the literal "runtime": "gvisor" into the agent-facing status. On a deployment with SANDBOX_RUNTIME_CLASS empty the Control Plane reports cluster-default and the Console renders it, while the one surface an agent uses to reason about its own confinement claimed isolation regardless. The lease already carries runtime_class; the SDK was discarding it.

Checkpoint restore rejected the absent body its contract declares optional. The OpenAPI says requestBody: required: false; the handler called read_json(), so no body answered 400 Content-Length is required and Content-Length: 0 answered 400 request body must be valid JSON. A missing checkpoint was reported as 400 rather than 404 for the same reason. Chunked bodies are refused explicitly rather than read as absent — treating an unreadable body as "no sha256 requested" would skip the integrity check the caller asked for.

The E2E announced isolation it had skipped. scripts/test.sh asserts the gVisor self-report only when SANDBOX_RUNTIME_CLASS is gvisor — correctly, because on the cluster default runtime that dmesg line belongs to the host kernel. The closing line said + gVisor unconditionally, so a run against an empty runtimeClass, which the AI-LOCK in core.py requires on nodes without runsc, announced a property nothing checked.

A cross-component timing invariant neither side knew about. ACTIVITY_PROBE_TIMEOUT is 2s and a timeout means "deletable"; shell_sessions.py records a measured 2.60s for that read while reaping a SIGKILLed child, with the comment "The 2 second probe line has been crossed". The lock discipline that holds it together is guarded by a real behavioural test, but that test's PROBE_BUDGET was a constant local to runtime/ and tied to nothing. Both drift directions failed silently: lower the timeout and the guarded bar stops being the real bar; raise it and the test's induced slowness no longer trips the probe, so passing proves nothing.

A bare KeyError where the codebase's own rule says otherwise. core.py states it: "SystemExit instead of KeyError: what the container wants is an instruction to follow, not a stack trace." KubeClient.__init__ raised KeyError: 'KUBERNETES_SERVICE_HOST' outside a cluster, and FileNotFoundError when automountServiceAccountToken was off.

Evidence chain

Benchmark environment captures were silently truncated into invalid JSON. output[:20_000] applied to every captured command with no marker; a kubectl get pods -o json measured 71,099 bytes on a real run. Anyone parsing environment.json to confirm what a report measured got a decode error.

And it described the cluster, not the run. runtimeClass was captured as the cluster's gvisor RuntimeClass object, snapshotted before the first Runtime exists. A report produced on a cluster with no gVisor read as a gVisor measurement. Each lease already reports the class it got; summary.json now carries observedRuntimeClasses, with unreported rather than a guess when a Control Plane is too old to send it.

Documentation that contradicted the code

docs/ARCHITECTURE.md said the Runtime provider "is selected independently by SANDBOX_RUNTIME_DRIVER" while configured_runtime_driver() never reads it — advertising exactly the plug-in surface README.md promises does not exist. The guard added for it is conditional: it reads the selector's presence out of the AST, so the prose may not promise a selector today and must mention one the day a registry lands.

docs/TROUBLESHOOTING.md told a reader debugging Runtime placement to run kubectl get nodes --show-labels | grep sandbox-node. That string appears nowhere else in the repository; the label is sandbox.hullwork.com/node-role. The command returned nothing on a correctly labelled cluster and nothing on an unlabelled one — no discriminating power, in the one place a reader is already lost.

docs/DEPLOYMENT.md said all four cluster dependencies are configured through values.yaml; the gVisor RuntimeClass has no value at all, and the chart pins both isolation variables as literals in a block where everything else templates. The same file's operator recipe never mentioned that both namespaces enforce PodSecurity restricted, so a stock PostgreSQL is rejected by the ReplicaSet controller while kubectl apply looks like it worked. And installing the chart in the documented order is impossible: it creates both namespaces and no Secrets, so creating the namespaces first to hold the Secrets makes helm install refuse them.

The object namespace — which path roots each scope accepts, which locator fields each requires, and the opposite constraints on workspace import and export — existed only in control_plane/core.py. It took six 400s to assemble one working call. The errors name the allowed set, which makes the API usable by trial; the table makes it usable by reading.

README.md's prerequisites omitted qemu-system-<arch> and shasum, which make doctor hard-requires on Linux — so the first documented command failed on a machine that had followed the documentation. The test suite already knew both, with a comment explaining why.

CLI and Console

run spells the Workspace name --name while create, exec and stop take it positionally, and no argument carried help text, unlike sandboxctl. sandbox stop --name demo — the natural next command after the README — was rejected with the top-level usage.

The Console rendered 1 minutes ago. The relative-time messages now use the plural mechanism the catalogs already had, with the compound form assembled from two independently pluralised halves, because in 5 hours 1 minutes is what a single template cannot avoid.

Ratchets

Nine of these could recur silently, so each is now pinned by a test that fails when it does: the README prerequisites against what dev-doctor.sh actually requires; the troubleshooting selector against the deployed label; the Helm install order against every workload that mounts a pre-provisioned Secret; the object namespace against the allowed_roots literals, read by AST; the architecture prose against the selector's presence, also by AST; the eviction bar against the probe timeout that enforces it; and the E2E verdict against the branch lifted out of scripts/test.sh rather than restated in the test.

Reported, not changed

The Console offers "Suspend" and "View keys" on the reserved management tenant. The backend refuses both with 403, so nothing breaks, but an operator is asked to confirm a destructive-sounding action that cannot succeed. Fixing it means either duplicating a backend constant in the frontend — the drift that produced the sandbox-node bug — or adding a reserved field to the public Tenant schema. That is a maintainer's call.

Not covered

gVisor isolation itself, OIDC, the Lima profile and scale-workers, the 1,000-file collection export, 30-minute steady load, and Control Plane restart recovery. The host has no /dev/kvm.

🤖 Generated with Claude Code

https://claude.ai/code/session_015s137CobDdyvryGDawa5Rh