diff --git a/README.md b/README.md index 5c09926..6f26b8e 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ operation fails; it never falls back to running on the host. ```bash make bootstrap # create .venv and install SDK + test dependencies -make test # 844 unit and contract tests, no network, no cluster +make test # 870 unit and contract tests, no network, no cluster make verify # complete Python, Console, manifest, Helm, wheel gate make help # every Make target with its one-line description ``` @@ -201,7 +201,7 @@ it first rather than discovering a gap halfway through the VM build. | Requirement | Detail | | --- | --- | -| Commands on `PATH` | `docker` (daemon reachable), `limactl`, `kubectl`, `helm`, `python3`, `openssl` | +| Commands on `PATH` | `docker` (daemon reachable), `limactl`, `kubectl`, `helm`, `python3`, `openssl`; on Linux also `qemu-system-` and `shasum`. Lima has one vmType on Linux, qemu, and boots the VM with the host architecture's system emulator, which `limactl` does not ship; `shasum` is what the Cilium and Rook chart installers verify with. macOS uses its own hypervisor framework and needs neither. | | Python | 3.11 or newer | | Host OS | macOS or Linux | | Host architecture | amd64 or arm64 (`scripts/local-cluster.yaml` pins Ubuntu images for both; gVisor is installed for `x86_64` and `aarch64`) | diff --git a/bench/runner.py b/bench/runner.py index def4ea0..a52b39c 100644 --- a/bench/runner.py +++ b/bench/runner.py @@ -28,6 +28,10 @@ ROOT = pathlib.Path(__file__).resolve().parents[1] DEFAULT_THRESHOLDS = ROOT / "bench/excellent-thresholds.json" PROTOCOL_VERSION = "2026-07-28" +# Captured command output is capped so one `kubectl get pods -o json` cannot +# dominate the report. Exceeding it is recorded rather than left to be +# discovered as a JSON decode error. +OUTPUT_CAP = 20_000 def utc_now() -> str: @@ -160,11 +164,19 @@ def command_output(command: list[str]) -> dict[str, Any]: except (OSError, subprocess.TimeoutExpired) as error: return {"available": False, "error": type(error).__name__} output = completed.stdout.strip() or completed.stderr.strip() - return { + # `kubectl get pods -o json` routinely passes the cap, and a JSON document + # cut mid-object is not distinguishable from a broken command once it is in + # the report: a reader parsing it gets a decode error and no reason for it. + # The cap stays; what changes is that the file now says so. + captured = { "available": completed.returncode == 0, "returnCode": completed.returncode, - "output": output[:20_000], + "output": output[:OUTPUT_CAP], } + if len(output) > OUTPUT_CAP: + captured["truncated"] = True + captured["fullLength"] = len(output) + return captured def environment_snapshot(base_url: str, kube_context: str | None) -> dict[str, Any]: @@ -202,6 +214,13 @@ def __init__(self, client: ApiClient, sample_file: pathlib.Path, run_id: str) -> self.sample_file = sample_file self.run_id = run_id self.samples: list[dict[str, Any]] = [] + # Every Runtime lease reports the RuntimeClass the Control Plane placed + # it under. The environment snapshot cannot answer this: it is taken + # before the first Runtime exists, and the `gvisor` RuntimeClass object + # it dumps is a cluster fact, not a statement about what these + # measurements ran on. A deployment with SANDBOX_RUNTIME_CLASS empty + # has that object and still runs every Pod on the default runtime. + self.runtime_classes: set[str] = set() def measure(self, metric: str, iteration: int, operation: Any) -> Any: started = time.perf_counter() @@ -304,6 +323,13 @@ def one_iteration(self, iteration: int) -> None: ) sandbox_id = sandbox["id"] sandbox_token = sandbox["access_token"] + # "cluster-default" here means the Pods these numbers describe ran + # without gVisor. Recorded per lease, so the summary states what + # was measured rather than what the cluster could have offered. + # "unreported" rather than a guess: a Control Plane too old to send + # the field is not evidence of gVisor, and it is not evidence + # against it either. Sorting the set needs a string for that case. + self.runtime_classes.add(sandbox.get("runtime_class") or "unreported") mcp_payload = { "jsonrpc": "2.0", "id": f"bench-{iteration}", @@ -411,6 +437,7 @@ def main() -> int: "iterationFailures": failures, "thresholdProfile": thresholds["profile"], "metrics": summaries, + "observedRuntimeClasses": sorted(benchmark.runtime_classes), "evaluation": evaluation, } (args.output_dir / "summary.json").write_text( diff --git a/console/Dockerfile b/console/Dockerfile index 245f218..a2f1c04 100644 --- a/console/Dockerfile +++ b/console/Dockerfile @@ -7,12 +7,14 @@ RUN npm run build FROM nginxinc/nginx-unprivileged:1.31-alpine3.24@sha256:d9083fe47768377ef55dedafd67d4da7c2f2bc2bece7554954f29359deb0dce9 # Temporary, until the base image is rebuilt: this digest ships libexpat -# 2.8.3-r0 (CVE-2026-66046, CVE-2026-76641) while Alpine 3.24 already carries -# the fixed 2.8.4-r0, so the image gate would refuse it. Remove the two lines -# below once a newer base digest passes `trivy image` on its own; they are -# the one place this build reaches the package index after the FROM. +# 2.8.3-r0 (CVE-2026-66046, CVE-2026-76641) and libuuid 2.42.1-r0 +# (CVE-2026-53612/53613/53614, CVE-2026-76642, CVE-2026-78408/78409/78410) +# while Alpine 3.24 already carries the fixed 2.8.4-r0 and 2.42.3-r1, so the +# image gate would refuse it. Remove the two lines below once a newer base +# digest passes `trivy image` on its own; they are the one place this build +# reaches the package index after the FROM. USER root -RUN apk upgrade --no-cache libexpat +RUN apk upgrade --no-cache libexpat libuuid COPY nginx.conf /etc/nginx/templates/default.conf.template COPY entrypoint.sh /usr/local/bin/sandbox-console-entrypoint COPY --from=build /app/dist /usr/share/nginx/html diff --git a/console/src/format.ts b/console/src/format.ts index 102c1fe..9529d24 100644 --- a/console/src/format.ts +++ b/console/src/format.ts @@ -33,6 +33,38 @@ export function formatUnix( }).format(new Date(seconds * 1000)); } +const CATALOGS = { en, "zh-CN": zhCN } as const; + +/** + * Renders one pluralised amount, for example "1 minute" or "12 minutes". + * + * The catalogs carry the unit as its own `.one`/`.other` family rather than + * baking it into a whole sentence: the compound form needs hours and minutes + * pluralised independently, and "in 5 hours 1 minutes" is exactly what a single + * sentence template cannot avoid. Locales without plural categories declare the + * same text twice, which is what Intl.PluralRules then never has to choose + * between. + */ +function amount( + locale: Locale, + unit: "seconds" | "minutes" | "hours", + count: number, +): string { + const catalog = CATALOGS[locale]; + const category = new Intl.PluralRules(locale).select(count); + const message = + catalog[`relative.${unit}.${category}` as keyof typeof catalog] + ?? catalog[`relative.${unit}.other` as keyof typeof catalog]; + return String(message).replace("{count}", String(count)); +} + +/** Places one rendered amount in its past or future frame. */ +function directed(locale: Locale, value: string, past: boolean): string { + const catalog = CATALOGS[locale]; + return catalog[past ? "relative.past" : "relative.future"] + .replace("{value}", value); +} + function relativeMessage( locale: Locale, unit: "seconds" | "minutes" | "hours" | "compoundHours", @@ -40,33 +72,13 @@ function relativeMessage( past: boolean, minutes = 0, ): string { - const messages = { - en: { - secondsPast: en["relative.secondsPast"], - secondsFuture: en["relative.secondsFuture"], - minutesPast: en["relative.minutesPast"], - minutesFuture: en["relative.minutesFuture"], - hoursPast: en["relative.hoursPast"], - hoursFuture: en["relative.hoursFuture"], - compoundHoursPast: en["relative.compoundHoursPast"], - compoundHoursFuture: en["relative.compoundHoursFuture"], - }, - "zh-CN": { - secondsPast: zhCN["relative.secondsPast"], - secondsFuture: zhCN["relative.secondsFuture"], - minutesPast: zhCN["relative.minutesPast"], - minutesFuture: zhCN["relative.minutesFuture"], - hoursPast: zhCN["relative.hoursPast"], - hoursFuture: zhCN["relative.hoursFuture"], - compoundHoursPast: zhCN["relative.compoundHoursPast"], - compoundHoursFuture: zhCN["relative.compoundHoursFuture"], - }, - }[locale]; - const template = messages[`${unit}${past ? "Past" : "Future"}`]; - return template - .replace("{count}", String(count)) - .replace("{hours}", String(count)) - .replace("{minutes}", String(minutes)); + if (unit === "compoundHours") { + const value = CATALOGS[locale]["relative.compound"] + .replace("{hours}", amount(locale, "hours", count)) + .replace("{minutes}", amount(locale, "minutes", minutes)); + return directed(locale, value, past); + } + return directed(locale, amount(locale, unit, count), past); } /** diff --git a/console/src/i18n/config.ts b/console/src/i18n/config.ts index ffa9669..a7581fe 100644 --- a/console/src/i18n/config.ts +++ b/console/src/i18n/config.ts @@ -20,6 +20,9 @@ export const PLURAL_KEYS = [ "tenants.keyCount", "monitoring.nodes.count", "monitoring.runtimes.count", + "relative.seconds", + "relative.minutes", + "relative.hours", ] as const; export type PluralTranslationKey = (typeof PLURAL_KEYS)[number]; diff --git a/console/src/i18n/locales/en.ts b/console/src/i18n/locales/en.ts index 6f8c802..f60a376 100644 --- a/console/src/i18n/locales/en.ts +++ b/console/src/i18n/locales/en.ts @@ -216,14 +216,15 @@ export const en = { "templates.invalidInput": "Both template ID and image are required.", "templates.confirmDelete": "Delete {scope} template {id}? Running sandboxes are unaffected, but future starts using this ID will fail.", - "relative.secondsFuture": "in {count} seconds", - "relative.secondsPast": "{count} seconds ago", - "relative.minutesFuture": "in {count} minutes", - "relative.minutesPast": "{count} minutes ago", - "relative.hoursFuture": "in {count} hours", - "relative.hoursPast": "{count} hours ago", - "relative.compoundHoursFuture": "in {hours} hours {minutes} minutes", - "relative.compoundHoursPast": "{hours} hours {minutes} minutes ago", + "relative.seconds.one": "{count} second", + "relative.seconds.other": "{count} seconds", + "relative.minutes.one": "{count} minute", + "relative.minutes.other": "{count} minutes", + "relative.hours.one": "{count} hour", + "relative.hours.other": "{count} hours", + "relative.compound": "{hours} {minutes}", + "relative.future": "in {value}", + "relative.past": "{value} ago", "observability.title": "Platform metrics", "observability.subtitle": "Live panels from the operator's Grafana, proxied on this origin. Read-only.", "observability.crossTenantNotice": "These panels are platform-wide. The metrics behind them carry no tenant dimension, so nothing here can be filtered to one tenant.", diff --git a/console/src/i18n/locales/zh-CN.ts b/console/src/i18n/locales/zh-CN.ts index d10aef7..4b2d4ff 100644 --- a/console/src/i18n/locales/zh-CN.ts +++ b/console/src/i18n/locales/zh-CN.ts @@ -218,14 +218,15 @@ export const zhCN: EnglishMessages = { "templates.invalidInput": "模板 ID 和镜像都必须填。", "templates.confirmDelete": "删除{scope}模板 {id}？已经在跑的沙箱不受影响，之后按这个 id 起沙箱会失败。", - "relative.secondsFuture": "{count} 秒后", - "relative.secondsPast": "{count} 秒前", - "relative.minutesFuture": "{count} 分钟后", - "relative.minutesPast": "{count} 分钟前", - "relative.hoursFuture": "{count} 小时后", - "relative.hoursPast": "{count} 小时前", - "relative.compoundHoursFuture": "{hours} 小时 {minutes} 分后", - "relative.compoundHoursPast": "{hours} 小时 {minutes} 分前", + "relative.seconds.one": "{count} 秒", + "relative.seconds.other": "{count} 秒", + "relative.minutes.one": "{count} 分钟", + "relative.minutes.other": "{count} 分钟", + "relative.hours.one": "{count} 小时", + "relative.hours.other": "{count} 小时", + "relative.compound": "{hours} {minutes}", + "relative.future": "{value}后", + "relative.past": "{value}前", "observability.title": "平台指标", "observability.subtitle": "运营方 Grafana 的实时面板，经本站同源反代，只读。", "observability.crossTenantNotice": "这些面板是平台级的。背后的指标不带租户维度，因此无法按单个租户筛选。", diff --git a/control_plane/Dockerfile b/control_plane/Dockerfile index 98edee6..86cba05 100644 --- a/control_plane/Dockerfile +++ b/control_plane/Dockerfile @@ -1,5 +1,13 @@ FROM python:3.14-alpine@sha256:c6ead215bfd31f1e433d968853b7a769989117115b728874824e6c0a27cb96fc +# Temporary, until the base image is rebuilt: this digest ships libuuid +# 2.42.1-r0 (CVE-2026-53612/53613/53614, CVE-2026-76642, CVE-2026-78408/78409/78410) +# while Alpine 3.24 already carries the fixed 2.42.3-r1, so the image gate would +# refuse it. Remove the two lines below once a newer base digest passes +# `trivy image` on its own; they are the one place this build reaches the +# package index after the FROM. +RUN apk upgrade --no-cache libuuid + RUN addgroup -g 65532 control-plane \ && adduser -D -u 65532 -G control-plane -h /nonexistent control-plane diff --git a/control_plane/api.py b/control_plane/api.py index 571b467..1e4ecdf 100644 --- a/control_plane/api.py +++ b/control_plane/api.py @@ -238,6 +238,28 @@ def read_json(self) -> dict: raise ValueError("request body must be a JSON object") return payload + def read_optional_json(self) -> dict: + """Body-or-empty, for a route whose request body the contract marks optional. + + ``read_json`` is right for every other POST: a missing body there is a + client that forgot the payload, and answering 400 says so. The + checkpoint-restore body carries one optional field, so the OpenAPI + declares ``required: false`` and a conforming client sends no body at + all - and used to be told ``Content-Length is required``. + + 🔴 Absent is not the same as unreadable. A chunked body has no + Content-Length either, and this server never decodes one; treating that + as "no sha256 supplied" would restore an archive **without** the + integrity check the caller asked for, which is the one thing this body + exists to request. Only a body that is genuinely absent becomes {}. + """ + if self.headers.get("Transfer-Encoding"): + raise ValueError("chunked request bodies are not supported") + raw_length = self.headers.get("Content-Length") + if raw_length is None or raw_length.strip() in {"", "0"}: + return {} + return self.read_json() + def bearer_token(self) -> str: return control_plane.parse_bearer_token(self.headers.get("Authorization", "")) @@ -1443,11 +1465,21 @@ def _workspace_owner_matches(self, workspace_id: str) -> bool | None: self.send_store_outage(exc) return None - def require_workspace_tenant(self, workspace_id: str) -> bool: + def require_workspace_tenant( + self, workspace_id: str, *, audit_denial: bool = True + ) -> bool: """Before operating a Workspace by ID, confirm that it belongs to this tenant. List filtering only blocks "seeing", but this block blocks "guessing the ID and then acting directly". - workspace_id is HMAC-derived and non-enumerable, but non-enumerability is not an access control.""" + workspace_id is HMAC-derived and non-enumerable, but non-enumerability is not an access control. + + audit_denial=False is for the one caller that does not receive an ID at + all: /v1/workspaces/resolve derives the ID from this tenant's own + credential, so a miss there means "this session has no Workspace yet", + never "someone is probing another tenant". Auditing it would file one + denial per Workspace ever created, in a row an operator cannot tell + apart from a real cross-tenant attempt - which is the only thing this + audit exists to surface. The ownership check itself still runs.""" if control_plane.STORE is None or self.tenant_id is None: return True matches = self._workspace_owner_matches(workspace_id) @@ -1459,7 +1491,8 @@ def require_workspace_tenant(self, workspace_id: str) -> bool: #That's a signal that can be used to enumerate. #But the rejection itself leaves a mark - continuous rejection means someone is testing the ID, which is a precursor to an attack. #Instead of noise, there is precisely nothing to say in the response. - self.audit("workspace.access", target=workspace_id, outcome="denied") + if audit_denial: + self.audit("workspace.access", target=workspace_id, outcome="denied") self.send_json(HTTPStatus.NOT_FOUND, {"error": "workspace not found"}) return False @@ -2423,7 +2456,7 @@ def do_POST(self) -> None: if not self.require_workspace_tenant(workspace_id): return control_plane.touch_workspace(workspace_id) - payload = self.read_json() + payload = self.read_optional_json() self.send_json( HTTPStatus.OK, control_plane.restore_workspace_checkpoint( @@ -2808,7 +2841,9 @@ def do_POST(self) -> None: principal_kind=principal_kind, principal_id=principal_id, ) - if not self.require_workspace_tenant(workspace_id): + if not self.require_workspace_tenant( + workspace_id, audit_denial=False + ): return status, listing, content_type = control_plane.volume_agent_request( "GET", "/v1/workspaces" diff --git a/control_plane/kube.py b/control_plane/kube.py index add849f..74b0b89 100644 --- a/control_plane/kube.py +++ b/control_plane/kube.py @@ -21,8 +21,24 @@ def __init__(self, status: int, message: str, payload: object | None = None): class KubeClient: + # SystemExit instead of KeyError/FileNotFoundError, for the same reason + # core.py states for the configuration block: what an operator wants out of + # a container that will not start is an instruction to follow, not a stack + # trace. All three inputs are absent together in the two situations that + # actually happen - the process was started outside a cluster, and the + # Deployment was given automountServiceAccountToken: false - and neither + # is diagnosable from `KeyError: 'KUBERNETES_SERVICE_HOST'`. def __init__(self) -> None: - host = os.environ["KUBERNETES_SERVICE_HOST"] + host = os.getenv("KUBERNETES_SERVICE_HOST") + if not host: + raise SystemExit( + "control_plane: KUBERNETES_SERVICE_HOST is unset, so this " + "process is not running inside a Kubernetes Pod. The Control " + "Plane talks to the API server through its in-cluster service " + "account and has no kubeconfig path; deploy it to the cluster " + "(k8s/, overlays/, or charts/sandbox) rather than running it " + "on a workstation." + ) port = os.getenv("KUBERNETES_SERVICE_PORT_HTTPS", "443") self.base_url = f"https://{host}:{port}" token_path = os.getenv( @@ -33,9 +49,18 @@ def __init__(self) -> None: "KUBERNETES_CA_FILE", "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt", ) - with open(token_path, encoding="utf-8") as handle: - self.token = handle.read().strip() - self.ssl_context = ssl.create_default_context(cafile=ca_path) + try: + with open(token_path, encoding="utf-8") as handle: + self.token = handle.read().strip() + self.ssl_context = ssl.create_default_context(cafile=ca_path) + except OSError as exc: + raise SystemExit( + f"control_plane: cannot read the service-account credential " + f"{exc.filename or token_path}: {exc.strerror}. The Pod needs " + "automountServiceAccountToken left on (the volume role is the " + "only one that turns it off), or KUBERNETES_TOKEN_FILE and " + "KUBERNETES_CA_FILE pointed at readable copies." + ) from exc def request( self, diff --git a/docs/API.md b/docs/API.md index 39fa8f9..36974c5 100644 --- a/docs/API.md +++ b/docs/API.md @@ -193,6 +193,33 @@ Two properties worth knowing before you wire it in: exception is a management-plane credential naming an `owner` outright, which needs no subject to build a partition from. +### Object namespace + +Object keys are built by the platform, never by the caller: the owner segment comes +from the credential and `X-Acting-Subject`, and the rest from a locator. Both scopes +constrain the first path segment, and a value outside the set is a `400` naming the +allowed roots. + +| Scope | Locator also needs | `path` must start with | Resulting key | +| --- | --- | --- | --- | +| `upload` | `upload_id` | `source/`, `derived/`, `meta/` | `users///uploads//` | +| `agent` | `agent_id`, `run_id` | `inputs/`, `outputs/`, `artifacts/`, `logs/`, `meta/` | `users///agents//runs//` | + +`upload_id`, `agent_id` and `run_id` are lowercase DNS-style identifiers. A path may +not be absolute, contain `..`, or exceed 512 bytes. + +The two routes that move bytes between a Workspace and an object are narrower still, +and in opposite directions: + +| Route | Object side | Workspace side | +| --- | --- | --- | +| `POST /v1/workspaces/{id}/objects/export` | `scope=agent` only | `workspace_path` must start with `artifacts/`; an `archive: true` export must name exactly `artifacts` | +| `POST /v1/workspaces/{id}/objects/import` | `scope=upload` only | destination must start with `data/uploads/` | + +The asymmetry is the point: what an agent produced leaves through `artifacts/`, and +what a user supplied enters under `data/uploads/`, so neither can be mistaken for the +other after the fact. + ### Blocking, idempotency, and retry The SDK performs **no retries and no backoff** of its own. Per ADR 0001, callers diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 9c8dfe1..bd435c5 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -142,8 +142,12 @@ is valid only when both services intentionally address the same objects. The component is named `Sandbox Control Plane` throughout the source package, image, Kubernetes resources, token audience, client configuration, and -observability artifacts. Process roles are `api` and `volume`; the Runtime -provider is selected independently by `SANDBOX_RUNTIME_DRIVER`. +observability artifacts. Process roles are `api` and `volume`. The Runtime +provider is named by `SANDBOX_RUNTIME_DRIVER`, which is validated at startup and +is not a selector: this release ships one provider, and `configured_runtime_driver()` +constructs it without reading the variable. Any value other than `gvisor` exits +rather than starting, which is the same promise the README makes - no provider +plug-in surface is advertised that does not exist. ## Known split debt diff --git a/docs/BENCHMARKS.md b/docs/BENCHMARKS.md index 2cdd143..93f4685 100644 --- a/docs/BENCHMARKS.md +++ b/docs/BENCHMARKS.md @@ -23,9 +23,18 @@ The output directory is new for every run and contains: - `samples.jsonl`: one measured operation per line, including failures; - `warmup-samples.jsonl`: excluded warmup observations, retained for audit; -- `summary.json`: sample counts, success rates, percentiles, and every threshold check; +- `summary.json`: sample counts, success rates, percentiles, every threshold check, and + `observedRuntimeClasses` - the RuntimeClass each measured Runtime lease actually + reported. `["gvisor"]` is what makes a report a gVisor measurement; + `["cluster-default"]` says the Pods behind these numbers had no gVisor kernel + isolation, whatever the cluster has installed; - `environment.json`: source state, OS/architecture/Python, endpoint, Kubernetes version, - RuntimeClass, and Pod image identities. It never contains the supplied token. + the cluster's `gvisor` RuntimeClass object, and Pod identities. It never contains the + supplied token. It is captured **before** the first Runtime is created, so it describes + what the cluster offers, not what these measurements ran under - read + `observedRuntimeClasses` for that. Captured command output is capped; a capture that + hit the cap carries `truncated` and `fullLength`, because a JSON dump cut mid-object + is otherwise indistinguishable from a command that failed halfway. The smoke default is 20 recorded iterations. Formal publication uses at least 100 iterations and five independent runs on each architecture. Do not combine arm64 and diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 4311193..d46c5a4 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -150,7 +150,7 @@ attributed to a person, so: | `WORKSPACE_IDLE_TTL_SECONDS` | `21600` | Workspace data idle TTL | | `SANDBOX_WORKSPACE_QUOTA` | `1Gi` | Requested PVC size in optional per-workspace mode | | `SANDBOX_PENDING_STALE_SECONDS` | `600` | Age after which a `pending` Runtime admission record is treated as abandoned and its slot released; keep well above the Runtime creation budget | -| `SANDBOX_ACTIVITY_PROBE_TIMEOUT` | `2` | Seconds allowed for the Runtime activity check that runs before an idle Runtime is evicted | +| `SANDBOX_ACTIVITY_PROBE_TIMEOUT` | `2` | Seconds allowed for the Runtime activity check that runs before an idle Runtime is evicted. A timeout counts as "not busy", so lowering it deletes working Runtimes: the probe reads in-process session state, and `runtime/shell_sessions.py` records 2.60s for that read on a Runtime reaping a SIGKILLed child under gVisor with `cpu: 500m`. The margin is small and it is a property of the isolation backend's syscall cost, not of this setting; re-measure before lowering it or changing RuntimeClass | | `SANDBOX_MAX_OBJECT_QUEUE` | `32` | Requests allowed to wait for the object-store slot; beyond it Control Plane answers `503` at once | | `SANDBOX_MAX_CONCURRENT_OBJECT_OPS` | `1` | Object-store operations in flight at once; each holds its body in memory, and the boto3 connection pool is sized to match | | `SANDBOX_MAX_LIST_ENTRIES` | `10000` | Rows one listing may return before it is refused. `read_timeout` bounds a single socket read, not an operation, so without this a slow trickle holds the operation slot indefinitely while the list grows in memory | diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 94f482f..a184070 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -14,14 +14,42 @@ helm template sandbox charts/sandbox `values.schema.json` validates the public configuration surface. Every product image accepts an immutable `sha256:` digest; when set, the digest takes -precedence over its tag. What the chart needs from the cluster - a PostgreSQL or -MySQL database, S3-compatible object storage, an RWX StorageClass, and a gVisor -RuntimeClass - is configured through `values.yaml` and described in the +precedence over its tag. Three of the four things the chart needs from the +cluster are `values.yaml` settings: the database (`postgresql.*`, embedded by +default), S3-compatible object storage (`objectStore.*`), and an RWX +StorageClass (`workspace.storageClass`). The fourth is not. The chart creates +the `gvisor` RuntimeClass itself and pins `SANDBOX_RUNTIME_CLASS` to it with no +value to override, so installing the `runsc` handler on every node that will +host Runtimes is a prerequisite, not a configuration choice: without it the Pods +schedule and then loop in `RunContainerError`. All four are described in the [Production guide](PRODUCTION.md). The existing Kustomize bases remain supported for source deployments. The Helm chart is the versioned package boundary for external GitOps composition. +### Install order + +The chart creates both namespaces, and it creates no Secrets. Those two facts +fix the order: creating `sandbox-system` yourself so you can put the Secrets in +it first makes `helm install` refuse the namespace it does not own +(`invalid ownership metadata; ... missing key "app.kubernetes.io/managed-by"`). +Install first, then fill in the Secrets from +[Pre-provisioned resources](#pre-provisioned-resources), then restart the three +workloads that mount them: + +```bash +helm install sandbox charts/sandbox --set workspace.storageClass= +# create the four Secrets here +kubectl -n sandbox-system rollout restart deploy/sandbox-control-plane statefulset/sandbox-postgres +kubectl -n sandbox-workloads rollout restart deploy/sandbox-volume +``` + +Between the first and second step the Control Plane, PostgreSQL, and Volume +Agent Pods restart on missing Secrets; that is expected and clears on the +rollout. A GitOps controller does not need the restarts: it reconciles the +Secrets alongside the release, and only the namespace objects have to stay +owned by the chart. + This is a multi-namespace package. Override `namespaces.system` and `namespaces.workloads` together; an Infra Stack destination namespace does not replace these values. Release `package-metadata.json` follows the shared schema @@ -185,6 +213,16 @@ local profile generates the Secrets with random values (`scripts/bootstrap-local-secrets.sh`); every other environment must create them before `kubectl apply`. +Both namespaces ship with `pod-security.kubernetes.io/enforce: restricted`, so +anything you add alongside them - the PostgreSQL you bring, a debug Pod, an +object-store shim - has to satisfy that profile too: `runAsNonRoot: true`, +`allowPrivilegeEscalation: false`, `capabilities.drop: ["ALL"]`, and +`seccompProfile.type: RuntimeDefault`. Stock database images do not set these, +and the rejection arrives from the ReplicaSet controller as a `FailedCreate` +event rather than from `kubectl apply`, so the Deployment looks accepted while +no Pod is ever created. Put such a dependency in its own namespace, or add the +security context. + | Resource | Kind | Namespace | Keys or requirement | | --- | --- | --- | --- | | `sandbox-api-credentials` | Secret | `sandbox-system` | `control-plane-token`, `signing-key`, `workspace-id-key` | diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 87bacab..b6e1038 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -59,11 +59,15 @@ elsewhere). ```bash kubectl get runtimeclass gvisor -kubectl get nodes --show-labels | grep sandbox-node +kubectl get nodes -l sandbox.hullwork.com/node-role=runtime kubectl -n sandbox-workloads get pods kubectl -n sandbox-workloads describe pod | sed -n '/Events/,$p' ``` +The selector is the placement label itself, so an empty node list *is* the +finding. Substitute your own key when the deployment sets a different +`SANDBOX_RUNTIME_NODE_SELECTOR`. + `describe` shows `FailedCreatePodSandBox ... runsc` when the handler is missing on the node and `0/N nodes are available: node(s) didn't match Pod's node affinity/selector` when the label is missing. diff --git a/docs/USAGE.md b/docs/USAGE.md index ddab9f2..6ea93d2 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -34,9 +34,14 @@ operations used in [Common tasks](#common-tasks). sandbox create demo sandbox exec demo -- python -c 'print("sandbox-ready")' sandbox run --name demo --stop -- sh -lc 'printf "done\\n"' +sandbox stop demo sandbox list ``` +`run` is the only subcommand that spells the Workspace name as `--name`; its +positional is already the command to execute. `create`, `exec`, and `stop` take +the name positionally, so `sandbox stop --name demo` is rejected. + `sandbox create` prints the name, Runtime id, and Workspace id separated by tabs (`demosb-...ws-...`). `sandbox list` prints one active Runtime per line as `id`, `workspace_id`, `status`, `template`. Both accept `--json` for machine output. diff --git a/file-service/Dockerfile b/file-service/Dockerfile index d45a29c..1b47c83 100644 --- a/file-service/Dockerfile +++ b/file-service/Dockerfile @@ -7,6 +7,14 @@ FROM python:3.14-alpine@sha256:c6ead215bfd31f1e433d968853b7a769989117115b7288748 #It was SIGKILL (actually measured 10s, 1s with init). When Control Plane deletes a Pod, it passes gracePeriodSeconds: 0 #This matter is covered up, but node emptying, eviction, and manual kubectl delete all use the default grace period. #The version is consistent with sandbox/runtime/Dockerfile (same base digest). +# Temporary, until the base image is rebuilt: this digest ships libuuid +# 2.42.1-r0 (CVE-2026-53612/53613/53614, CVE-2026-76642, CVE-2026-78408/78409/78410) +# while Alpine 3.24 already carries the fixed 2.42.3-r1, so the image gate would +# refuse it. Remove the two lines below once a newer base digest passes +# `trivy image` on its own; they are the one place this build reaches the +# package index after the FROM. +RUN apk upgrade --no-cache libuuid + RUN apk add --no-cache tini=0.19.0-r3 \ && addgroup -g 65532 workspace \ && adduser -D -u 65532 -G workspace -h /workspace workspace \ diff --git a/runtime/Dockerfile b/runtime/Dockerfile index 7a5f31c..400e8f3 100644 --- a/runtime/Dockerfile +++ b/runtime/Dockerfile @@ -12,6 +12,14 @@ FROM python:3.14-alpine@sha256:c6ead215bfd31f1e433d968853b7a769989117115b7288748 # Deliberately not installed: gcc and other compilation chains (volume ~200MB, few scenarios), openssh (key management interface). # All pins are the same as the above bash/tini version by version; dependabot only takes care of the base image above # To upgrade digest and apk pin, you need to manually change this. +# Temporary, until the base image is rebuilt: this digest ships libuuid +# 2.42.1-r0 (CVE-2026-53612/53613/53614, CVE-2026-76642, CVE-2026-78408/78409/78410) +# while Alpine 3.24 already carries the fixed 2.42.3-r1, so the image gate would +# refuse it. Remove the two lines below once a newer base digest passes +# `trivy image` on its own; they are the one place this build reaches the +# package index after the FROM. +RUN apk upgrade --no-cache libuuid + RUN apk add --no-cache bash=5.3.9-r1 tini=0.19.0-r3 \ nodejs=24.18.1-r0 npm=11.12.1-r0 \ curl=8.22.0-r0 git=2.54.0-r0 jq=1.8.2-r0 unzip=6.0-r16 make=4.4.1-r4 \ diff --git a/sandbox_platform/mcp.py b/sandbox_platform/mcp.py index 3bf2e93..f7ea0c3 100644 --- a/sandbox_platform/mcp.py +++ b/sandbox_platform/mcp.py @@ -105,7 +105,10 @@ "name": "sandbox_status", "description": ( "Show this process's cached lease for the session: workspace and " - "runtime (gVisor Pod) ids as last seen. It does not contact the " + "Runtime ids as last seen, plus runtime_class, the RuntimeClass the " + "Control Plane reported placing the Runtime under - 'gvisor' only " + "where the deployment actually configures it, 'cluster-default' " + "where it does not, null with no Runtime. It does not contact the " "Control Plane, so it cannot tell whether either is still reachable; " "run shell to find out." ), @@ -260,7 +263,13 @@ def call_tool(name: str, arguments: dict) -> dict: # implementation only: Runtime MCP. No sidecar or volume fallback. status = manager.status() files = "runtime_mcp" if status.get("runtime_ready") else "offline" - return _tool_result({**status, "runtime": "gvisor", "files": files}) + # runtime_class comes from the lease the Control Plane issued. It used + # to be the literal "gvisor", which is a claim about confinement that + # this process cannot make: a deployment that leaves + # SANDBOX_RUNTIME_CLASS empty places Runtimes on the cluster default + # runtime, the Control Plane reports "cluster-default" for it, and the + # constant asserted gVisor isolation to the agent anyway. + return _tool_result({**status, "files": files}) if name == "file_read": path = arguments.get("path") if not isinstance(path, str) or not path: diff --git a/sandbox_platform/sandbox_cli.py b/sandbox_platform/sandbox_cli.py index 158ae94..9bdc2f0 100644 --- a/sandbox_platform/sandbox_cli.py +++ b/sandbox_platform/sandbox_cli.py @@ -37,28 +37,41 @@ def build_parser() -> argparse.ArgumentParser: subparsers = parser.add_subparsers(dest="action", required=True) create = subparsers.add_parser("create", help="create or resume a sandbox") - create.add_argument("name", nargs="?") - create.add_argument("--template") - create.add_argument("--json", action="store_true") + create.add_argument( + "name", + nargs="?", + help="Workspace name; omit for a throwaway one. Only `run` spells this " + "as --name, because its own positional is the command to execute", + ) + create.add_argument("--template", help="runtime template id (see `sandboxctl templates`)") + create.add_argument("--json", action="store_true", help="output raw JSON") run = subparsers.add_parser("run", help="create/resume and run a command") - run.add_argument("--name") - run.add_argument("--template") - run.add_argument("--timeout", type=int, default=30) - run.add_argument("--stop", action="store_true") - run.add_argument("command", nargs=argparse.REMAINDER) + run.add_argument( + "--name", + help="Workspace name; omit for a throwaway one. A flag here, and a " + "positional in create/exec/stop, because the positional is the command", + ) + run.add_argument("--template", help="runtime template id (see `sandboxctl templates`)") + run.add_argument("--timeout", type=int, default=30, help="seconds to wait for the command") + run.add_argument( + "--stop", + action="store_true", + help="release the Runtime afterwards; the Workspace and its files stay", + ) + run.add_argument("command", nargs=argparse.REMAINDER, help="command to run, after --") execute = subparsers.add_parser("exec", help="run in an existing sandbox") - execute.add_argument("name") - execute.add_argument("--timeout", type=int, default=30) - execute.add_argument("command", nargs=argparse.REMAINDER) + execute.add_argument("name", help="Workspace name, positional (not --name)") + execute.add_argument("--timeout", type=int, default=30, help="seconds to wait for the command") + execute.add_argument("command", nargs=argparse.REMAINDER, help="command to run, after --") stop = subparsers.add_parser("stop", help="stop a sandbox Runtime") - stop.add_argument("name") - stop.add_argument("--json", action="store_true") + stop.add_argument("name", help="Workspace name, positional (not --name)") + stop.add_argument("--json", action="store_true", help="output raw JSON") listing = subparsers.add_parser("list", help="list active Runtimes") - listing.add_argument("--json", action="store_true") + listing.add_argument("--json", action="store_true", help="output raw JSON") return parser diff --git a/sandbox_platform/sandbox_client.py b/sandbox_platform/sandbox_client.py index aff1bcd..a592efe 100644 --- a/sandbox_platform/sandbox_client.py +++ b/sandbox_platform/sandbox_client.py @@ -88,6 +88,12 @@ class Lease: #The template actually used by the current runtime. Once the runtime is built, the image is fixed: change the template #It must be released first and then created, so remember here that it is used to block "thinking the switch is successful". sandbox_template: str | None = None + # The RuntimeClass the Control Plane actually placed the Runtime under, as + # it reported it. None means no Runtime, or a Control Plane too old to say. + # Recorded rather than assumed: a deployment with SANDBOX_RUNTIME_CLASS + # empty runs Pods on the cluster default runtime and the Control Plane + # answers "cluster-default", which is the opposite of a gVisor claim. + sandbox_runtime_class: str | None = None @dataclasses.dataclass(frozen=True) @@ -750,6 +756,7 @@ def ensure_runtime( #The runtime has been recycled by the control_plane, and the template records must be cleared accordingly. #Otherwise, when rebuilding below, the old template will be compared to a non-existent sandbox. lease.sandbox_template = None + lease.sandbox_runtime_class = None else: lease.sandbox_token = _required_str( result, "access_token", "POST /v1/sandboxes/{id}/token" @@ -774,6 +781,7 @@ def ensure_runtime( #The template returned by the Control Plane shall prevail rather than the one in the request: the request can be made without template #Using the platform default, only the response knows which one will take effect in the end. lease.sandbox_template = result.get("template") + lease.sandbox_runtime_class = result.get("runtime_class") return lease def list_runtimes(self) -> list[dict]: @@ -833,6 +841,7 @@ def lookup_runtime(self, session_key: str) -> Lease: ) lease.sandbox_checked_at = time.time() lease.sandbox_template = resolved.get("template") + lease.sandbox_runtime_class = resolved.get("runtime_class") return lease def read_file(self, path: str, offset: int = 1, limit: int = 0) -> dict: @@ -1290,6 +1299,7 @@ def status(self, session_key: str | None = None) -> dict: "sandbox_id": lease.sandbox_id, "runtime_ready": bool(lease.sandbox_id), "template": lease.sandbox_template, + "runtime_class": lease.sandbox_runtime_class, } def release_runtime(self, session_key: str | None = None) -> dict: @@ -1308,6 +1318,7 @@ def release_runtime(self, session_key: str | None = None) -> dict: lease.sandbox_token_expires_at = 0 lease.sandbox_checked_at = 0 lease.sandbox_template = None + lease.sandbox_runtime_class = None return result diff --git a/scripts/test.sh b/scripts/test.sh index 9df23c6..81aefd0 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -378,4 +378,15 @@ curl --fail-with-body --silent --show-error \ "${API_URL}/v1/workspaces/${SDK_WORKSPACE_ID}?purge=true" >/dev/null SDK_WORKSPACE_ID="" -echo "Sandbox E2E passed: Runtime MCP files/shell + shared Workspace + gVisor" +#The isolation assertion above is conditional on SANDBOX_RUNTIME_CLASS, and the +#comment at the assertion says why: on the cluster default runtime the dmesg line +#is the host kernel's, so asserting it would be a false alarm. This line has to +#follow the same condition. Announcing "+ gVisor" after skipping the check is the +#report claiming a property the run never observed - and an empty runtimeClass is +#a supported configuration, not a misconfiguration. +if [ "$SANDBOX_RUNTIME_CLASS" = gvisor ]; then + isolation_result="gVisor" +else + isolation_result="runtimeClass=${SANDBOX_RUNTIME_CLASS:-}, isolation not asserted" +fi +echo "Sandbox E2E passed: Runtime MCP files/shell + shared Workspace + ${isolation_result}" diff --git a/tests/test_benchmark_runner.py b/tests/test_benchmark_runner.py index c5a5d78..9c52f15 100644 --- a/tests/test_benchmark_runner.py +++ b/tests/test_benchmark_runner.py @@ -2,11 +2,19 @@ import json import pathlib +import sys import tempfile import unittest from unittest import mock -from bench.runner import BenchmarkRun, evaluate_thresholds, metric_summary, percentile +from bench import runner +from bench.runner import ( + BenchmarkRun, + command_output, + evaluate_thresholds, + metric_summary, + percentile, +) ROOT = pathlib.Path(__file__).resolve().parents[1] @@ -92,6 +100,94 @@ def request(self, method: str, path: str, **kwargs: object) -> tuple[int, object self.assertIn(("DELETE", "/v1/sandboxes/sb-test"), client.calls) self.assertIn(("DELETE", "/v1/workspaces/ws-test?purge=true"), client.calls) + def test_the_runtime_class_that_ran_is_taken_from_the_lease(self) -> None: + """What the Pods ran under is an observation, not a cluster fact. + + ``environment.json`` dumps the ``gvisor`` RuntimeClass object, and it is + written before the first Runtime exists. A deployment that leaves + ``SANDBOX_RUNTIME_CLASS`` empty has that object and still places every + Pod on the cluster default runtime, so the snapshot alone would let a + report read as a gVisor measurement when it is not one. Each lease says + which class it got; the summary reports those. + """ + class FakeClient: + def request(self, method: str, path: str, **kwargs: object) -> tuple[int, object]: + if path == "/healthz": + return 200, {"status": "ok"} + if method == "POST" and path == "/v1/workspaces": + return 201, {"workspace_id": "ws-test", "access_token": "workspace-token"} + if method == "POST" and path == "/v1/sandboxes": + return 201, { + "id": "sb-test", + "access_token": "sandbox-token", + "runtime_class": "cluster-default", + } + if path.endswith("/mcp"): + return 200, {"jsonrpc": "2.0", "result": {"structuredContent": { + "exit_code": 0, "stdout": "bench-ready", + }}} + if path.endswith("/files/write"): + return 200, {"ok": True} + if "/files/read?" in path: + return 200, {"content": "x" * 1024} + if method == "DELETE": + return 200, {"ok": True} + raise AssertionError((method, path, kwargs)) + + with tempfile.TemporaryDirectory() as directory: + run = BenchmarkRun( # type: ignore[arg-type] + FakeClient(), pathlib.Path(directory) / "samples.jsonl", "unit" + ) + run.one_iteration(0) + self.assertEqual(run.runtime_classes, {"cluster-default"}) + + def test_a_lease_without_the_field_is_recorded_as_unreported(self) -> None: + # A Control Plane too old to send runtime_class is not evidence of + # gVisor and not evidence against it. It must not become "gvisor" by + # omission, and it must not crash the summary's sort either. + class FakeClient: + def request(self, method: str, path: str, **kwargs: object) -> tuple[int, object]: + if path == "/healthz": + return 200, {"status": "ok"} + if method == "POST" and path == "/v1/workspaces": + return 201, {"workspace_id": "ws-test", "access_token": "workspace-token"} + if method == "POST" and path == "/v1/sandboxes": + return 201, {"id": "sb-test", "access_token": "sandbox-token"} + if path.endswith("/mcp"): + return 200, {"jsonrpc": "2.0", "result": {"structuredContent": { + "exit_code": 0, "stdout": "bench-ready", + }}} + if path.endswith("/files/write"): + return 200, {"ok": True} + if "/files/read?" in path: + return 200, {"content": "x" * 1024} + if method == "DELETE": + return 200, {"ok": True} + raise AssertionError((method, path, kwargs)) + + with tempfile.TemporaryDirectory() as directory: + run = BenchmarkRun( # type: ignore[arg-type] + FakeClient(), pathlib.Path(directory) / "samples.jsonl", "unit" + ) + run.one_iteration(0) + self.assertEqual(run.runtime_classes, {"unreported"}) + self.assertEqual(sorted(run.runtime_classes), ["unreported"]) + + def test_truncated_command_output_says_so(self) -> None: + # A `kubectl get pods -o json` past the cap lands in the report as a + # JSON document that stops mid-object. Without the marker the reader + # cannot tell that from a command that failed halfway. + long_output = command_output( + [sys.executable, "-c", f"print('x' * {runner.OUTPUT_CAP + 500})"] + ) + self.assertTrue(long_output["truncated"]) + self.assertEqual(long_output["fullLength"], runner.OUTPUT_CAP + 500) + self.assertEqual(len(long_output["output"]), runner.OUTPUT_CAP) + + short_output = command_output([sys.executable, "-c", "print('ok')"]) + self.assertNotIn("truncated", short_output) + self.assertEqual(short_output["output"], "ok") + def test_cleanup_waits_for_asynchronous_runtime_deletion(self) -> None: class FakeClient: def __init__(self) -> None: diff --git a/tests/test_dev_scripts.py b/tests/test_dev_scripts.py index 32611b7..4fd7a7c 100644 --- a/tests/test_dev_scripts.py +++ b/tests/test_dev_scripts.py @@ -184,6 +184,106 @@ def test_the_restore_is_verified_by_hashing_the_file(self) -> None: ) +def doctor_required_commands() -> list[str]: + """Every command name `required=`/`required+=` in the doctor, script-order. + + Parsed rather than duplicated: the two Linux-only entries were in the script + and in this file's fixtures for a while, and still missing from the README + table a newcomer reads before running anything. A name that expands at run + time (`qemu-system-$(uname -m)`) is reduced to its literal prefix, which is + what prose can name. + """ + script = (ROOT / "scripts/dev-doctor.sh").read_text(encoding="utf-8") + names: list[str] = [] + for line in script.splitlines(): + # Drop command substitutions first: `$(uname -m)` contains a space and + # a paren, so splitting the raw line yields the fragment `-m)`. + line = re.sub(r"\$\([^)]*\)", "", line) + match = re.search(r"required\+?=\((.*)\)", line) + if not match: + continue + for word in match.group(1).split(): + word = word.strip("\"'") + if word: + names.append(word) + return names + + +class PrerequisiteDocumentationTests(unittest.TestCase): + def test_the_readme_names_every_command_the_doctor_demands(self) -> None: + # `make doctor` is the first command the README tells a newcomer to run + # and it exits non-zero on a missing tool. A tool it demands but the + # prerequisites table omits turns that into a failure the reader was + # given no way to prevent. + required = doctor_required_commands() + self.assertIn("qemu-system-", required, "doctor parse produced no qemu entry") + readme = (ROOT / "README.md").read_text(encoding="utf-8") + start = readme.index("### Prerequisites") + section = readme[start:readme.index("### Bring it up", start)] + missing = [name for name in required if name not in section] + self.assertEqual(missing, [], f"README prerequisites omit: {missing}") + + +class E2EVerdictTests(unittest.TestCase): + """The E2E's closing line may not claim isolation the run skipped. + + `scripts/test.sh` asserts the gVisor self-report only when + `SANDBOX_RUNTIME_CLASS` is `gvisor`, and the comment at the assertion says + why: on the cluster default runtime that dmesg line belongs to the host + kernel, so asserting it would be a false alarm. The closing line said + "+ gVisor" unconditionally, so a run against an empty runtimeClass - a + supported configuration, and what the AI-LOCK in `control_plane/core.py` + requires on nodes without runsc - announced a property nothing checked. + """ + + def setUp(self) -> None: + self.script = (ROOT / "scripts/test.sh").read_text(encoding="utf-8") + + def test_the_isolation_assertion_is_still_conditional(self) -> None: + # The premise. If this assertion became unconditional the closing line + # could go back to being a constant, and this class would be wrong. + self.assertIn( + 'assert sys.argv[2] != "gvisor" or "gVisor" in data["stdout"]', + self.script, + ) + + def test_the_closing_line_is_not_a_constant_claim(self) -> None: + closing = [ + line for line in self.script.splitlines() + if line.startswith("echo \"Sandbox E2E passed:") + ] + self.assertEqual(len(closing), 1, closing) + self.assertNotIn("gVisor", closing[0], closing[0]) + self.assertIn("${isolation_result}", closing[0], closing[0]) + + def verdict_branch(self) -> str: + """The branch lifted out of scripts/test.sh, not a copy of it. + + A restated snippet would keep passing against the old wording after the + script changed - the assertion would be pinned to the test's own copy + rather than to the code that ships. + """ + start = self.script.index('if [ "$SANDBOX_RUNTIME_CLASS" = gvisor ]; then') + end = self.script.index('\n', self.script.index('echo "Sandbox E2E passed:')) + return self.script[start:end] + + def test_the_verdict_follows_the_configured_class(self) -> None: + branch = self.verdict_branch() + for configured, expected in ( + ("gvisor", "+ gVisor"), + ("", "isolation not asserted"), + ("kata-clh", "runtimeClass=kata-clh, isolation not asserted"), + ): + with self.subTest(runtime_class=configured): + result = subprocess.run( + ["bash", "-c", branch], + env={**os.environ, "SANDBOX_RUNTIME_CLASS": configured}, + check=True, capture_output=True, text=True, + ) + self.assertIn(expected, result.stdout) + + + class DevelopmentScriptTests(unittest.TestCase): def test_shell_entrypoints_parse(self) -> None: for relative in ( diff --git a/tests/test_helm_package.py b/tests/test_helm_package.py index a2ab7f0..f002242 100644 --- a/tests/test_helm_package.py +++ b/tests/test_helm_package.py @@ -57,6 +57,47 @@ def test_values_schema_supports_digest_pinned_images(self) -> None: self.assertIn("digest", image_with_policy) self.assertIn("sha256", image["digest"]["pattern"]) + def test_the_documented_restart_set_matches_what_mounts_the_secrets(self) -> None: + """Every rendered workload that mounts a pre-provisioned Secret is named. + + The chart owns both namespaces and creates none of the four Secrets, so + `helm install` has to run before they can exist and the workloads that + want them come up first and fail. The install-order section is the only + place that says which ones to restart; a workload added later that also + mounts one of these Secrets would silently stay broken. + """ + if not shutil.which("helm"): + self.skipTest("helm is not installed") + rendered = subprocess.run( + ["helm", "template", "sandbox", str(CHART)], + check=True, + capture_output=True, + text=True, + ).stdout + wanted = { + "sandbox-api-credentials", + "sandbox-postgres-auth", + "object-store-credentials", + "sandbox-volume-auth", + } + consumers = set() + for document in yaml.safe_load_all(rendered): + if not document or document.get("kind") not in { + "Deployment", "StatefulSet", "Job", "CronJob", + }: + continue + serialised = yaml.safe_dump(document) + if any(name in serialised for name in wanted): + consumers.add(document["metadata"]["name"]) + self.assertTrue(consumers, "no rendered workload mounts a pre-provisioned Secret") + guide = (ROOT / "docs/DEPLOYMENT.md").read_text(encoding="utf-8") + start = guide.index("### Install order") + section = guide[start:guide.index("\nThis is a multi-namespace package", start)] + missing = sorted(name for name in consumers if name not in section) + self.assertEqual( + missing, [], f"install order does not say to restart: {missing}" + ) + def test_embedded_postgres_accepts_only_control_plane_ingress(self) -> None: if not shutil.which("helm"): self.skipTest("helm is not installed") diff --git a/tests/test_kube_client_startup.py b/tests/test_kube_client_startup.py new file mode 100644 index 0000000..f7a3190 --- /dev/null +++ b/tests/test_kube_client_startup.py @@ -0,0 +1,61 @@ +"""A Control Plane that cannot reach its service account must say what to do. + +``core.py`` states the rule for the configuration block: SystemExit rather than +KeyError, because "what the container wants is an instruction to follow, not a +stack trace". ``KubeClient`` is constructed at import time from the same kind of +environment, so it is held to the same rule. Both inputs are missing together in +the two situations that actually occur - started outside a cluster, and +``automountServiceAccountToken: false`` - and a traceback names neither. +""" + +from __future__ import annotations + +import os +import pathlib +import sys +import unittest + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parents[1])) + +from control_plane import kube # noqa: E402 + + +class KubeClientStartupTests(unittest.TestCase): + def setUp(self) -> None: + self.saved = { + name: os.environ.get(name) + for name in ( + "KUBERNETES_SERVICE_HOST", + "KUBERNETES_TOKEN_FILE", + "KUBERNETES_CA_FILE", + ) + } + + def tearDown(self) -> None: + for name, value in self.saved.items(): + if value is None: + os.environ.pop(name, None) + else: + os.environ[name] = value + + def test_outside_a_cluster_it_names_the_cause_not_the_variable(self) -> None: + os.environ.pop("KUBERNETES_SERVICE_HOST", None) + with self.assertRaises(SystemExit) as caught: + kube.KubeClient() + message = str(caught.exception) + self.assertIn("KUBERNETES_SERVICE_HOST", message) + self.assertIn("not running inside a Kubernetes Pod", message) + + def test_an_unreadable_service_account_token_names_the_file(self) -> None: + os.environ["KUBERNETES_SERVICE_HOST"] = "10.0.0.1" + os.environ["KUBERNETES_TOKEN_FILE"] = "/nonexistent/sandbox-token" + os.environ["KUBERNETES_CA_FILE"] = "/nonexistent/sandbox-ca.crt" + with self.assertRaises(SystemExit) as caught: + kube.KubeClient() + message = str(caught.exception) + self.assertIn("/nonexistent/sandbox-token", message) + self.assertIn("automountServiceAccountToken", message) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_label_domain.py b/tests/test_label_domain.py index be3cf0c..849cce5 100644 --- a/tests/test_label_domain.py +++ b/tests/test_label_domain.py @@ -48,3 +48,36 @@ def test_the_retired_prefix_is_gone(self) -> None: if __name__ == "__main__": unittest.main() + + +class PlacementLabelDocumentationTests(unittest.TestCase): + """The placement label a reader is told to check must be the real one. + + The scan above deliberately excludes ``docs/``: prose mentions keys it does + not write. That exemption let the "Runtime Pod stays Pending" check ship as + ``kubectl get nodes --show-labels | grep sandbox-node`` - a string this + product never writes, so it printed nothing on a correctly labelled cluster + and nothing on an unlabelled one. A check that cannot separate the two + answers "the label is missing" to a reader who is debugging placement. + """ + + @staticmethod + def deployed_node_selector() -> str: + manifest = (ROOT / "k8s/control-plane.yaml").read_text(encoding="utf-8") + match = re.search( + r"name: SANDBOX_RUNTIME_NODE_SELECTOR\s*\n\s*value: (\S+)", manifest + ) + assert match, "SANDBOX_RUNTIME_NODE_SELECTOR is no longer set in k8s/control-plane.yaml" + return match.group(1).strip("\"'") + + def test_the_pending_pod_check_selects_on_the_deployed_label(self) -> None: + selector = self.deployed_node_selector() + self.assertTrue(selector.startswith(PREFIX), selector) + guide = (ROOT / "docs/TROUBLESHOOTING.md").read_text(encoding="utf-8") + start = guide.index("## Runtime Pod stays `Pending`") + section = guide[start:guide.index("\n## ", start)] + self.assertIn( + f"kubectl get nodes -l {selector}", + section, + "the Pending-Pod check must select on the label the Control Plane places by", + ) diff --git a/tests/test_mcp_contract.py b/tests/test_mcp_contract.py index b08e810..223b6ce 100644 --- a/tests/test_mcp_contract.py +++ b/tests/test_mcp_contract.py @@ -24,6 +24,33 @@ def test_status_description_says_it_is_a_local_view(self) -> None: self.assertIn("does not contact the Control Plane", description) self.assertIn("cached lease", description) + def test_status_reports_the_reported_runtime_class_not_a_constant(self) -> None: + """The agent-facing status must not assert gVisor on a cluster without it. + + ``sandbox_status`` used to merge the literal ``"runtime": "gvisor"`` + into its result. A deployment that leaves ``SANDBOX_RUNTIME_CLASS`` + empty runs Runtimes on the cluster default runtime; the Control Plane + reports ``cluster-default`` for exactly that case + (``test_empty_runtime_class_reports_cluster_default_not_gvisor_isolation``) + and the Console renders it, while this surface - the one an agent uses + to reason about its own confinement - claimed isolation regardless. + """ + recorded = { + "session_id": "s", "workspace_id": "ws-000000000000", + "workspace_ready": True, "sandbox_id": "sb-000000000000", + "runtime_ready": True, "template": "default", + "runtime_class": "cluster-default", + } + original = mcp.manager.status + mcp.manager.status = lambda *args, **kwargs: dict(recorded) + try: + result = mcp.call_tool("sandbox_status", {}) + finally: + mcp.manager.status = original + payload = result["structuredContent"] + self.assertEqual(payload["runtime_class"], "cluster-default") + self.assertNotIn("gvisor", str(payload)) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_object_namespace_doc.py b/tests/test_object_namespace_doc.py new file mode 100644 index 0000000..1f01375 --- /dev/null +++ b/tests/test_object_namespace_doc.py @@ -0,0 +1,77 @@ +"""The object path vocabulary a caller must obey has to be written down. + +Every prefix rule below lives in ``control_plane/core.py`` and nowhere else: an +integrator discovers them by sending requests and reading 400s. The errors name +the allowed set, which makes the API usable by trial; the documentation is what +makes it usable by reading. This test keeps the two from drifting apart, in the +direction that matters - a set widened or narrowed in code without the table +following. +""" + +from __future__ import annotations + +import ast +import pathlib +import re +import unittest + + +ROOT = pathlib.Path(__file__).resolve().parents[1] + + +def allowed_roots_in_source() -> list[set[str]]: + """Every ``allowed_roots={...}`` literal passed inside ``object_location``.""" + source = (ROOT / "control_plane/core.py").read_text(encoding="utf-8") + tree = ast.parse(source) + function = next( + node for node in ast.walk(tree) + if isinstance(node, ast.FunctionDef) and node.name == "object_location" + ) + found = [] + for node in ast.walk(function): + if not isinstance(node, ast.Call): + continue + for keyword in node.keywords: + if keyword.arg == "allowed_roots" and isinstance(keyword.value, ast.Set): + found.append({element.value for element in keyword.value.elts}) + return found + + +class ObjectNamespaceDocumentationTests(unittest.TestCase): + def setUp(self) -> None: + document = (ROOT / "docs/API.md").read_text(encoding="utf-8") + start = document.index("### Object namespace") + self.section = document[start:document.index("\n### ", start + 1)] + + def test_both_scopes_declare_the_roots_the_code_enforces(self) -> None: + found = allowed_roots_in_source() + self.assertEqual( + [ + {"source", "derived", "meta"}, + {"inputs", "outputs", "artifacts", "logs", "meta"}, + ], + found, + "object_location's allowed roots changed; update docs/API.md with them", + ) + for roots in found: + for root in roots: + self.assertIn( + f"`{root}/`", + self.section, + f"docs/API.md does not list the {root!r} object root", + ) + + def test_the_workspace_transfer_directions_are_stated(self) -> None: + api = (ROOT / "control_plane/api.py").read_text(encoding="utf-8") + destination = re.search(r'destination\.startswith\("([^"]+)"\)', api) + self.assertIsNotNone(destination, "the import destination guard moved") + self.assertIn(destination.group(1), self.section) + # Export is the other direction and names a different root; a table that + # only mentioned one of them would read as if they were interchangeable. + self.assertIn("`artifacts/`", self.section) + self.assertIn("objects/export", self.section) + self.assertIn("objects/import", self.section) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_optional_request_body.py b/tests/test_optional_request_body.py new file mode 100644 index 0000000..22734c9 --- /dev/null +++ b/tests/test_optional_request_body.py @@ -0,0 +1,149 @@ +"""The one route whose request body the contract marks optional must accept none. + +``POST /v1/workspaces/{id}/checkpoints/{id}/restore`` takes a body carrying a +single optional ``sha256``. The OpenAPI says ``requestBody: required: false``; +the handler called ``read_json`` anyway, so a client that sent no body was told +``Content-Length is required`` and one that sent ``Content-Length: 0`` was told +``request body must be valid JSON``. Both were observed against a live +deployment; ``-d '{}'`` was the undocumented way through. + +The probe runs in a subprocess: importing ``control_plane.api`` needs a +configured environment, and setting one in this process would follow every +module imported after it. +""" + +from __future__ import annotations + +import json +import os +import pathlib +import subprocess +import sys +import textwrap +import unittest + +import yaml + + +ROOT = pathlib.Path(__file__).resolve().parents[1] + +PROBE = textwrap.dedent( + """ + import io + import json + + from control_plane import api + + class Headers: + def __init__(self, values): + self._values = values + + def get(self, name, default=None): + return self._values.get(name, default) + + class Probe: + # The real methods, on a request that owns nothing but its headers and + # its body. read_optional_json delegates to read_json, so both come + # from ApiHandler rather than being restated here. + read_json = api.ApiHandler.read_json + read_optional_json = api.ApiHandler.read_optional_json + + def __init__(self, headers, body=b""): + self.headers = Headers(headers) + self.rfile = io.BytesIO(body) + + def attempt(headers, body=b""): + try: + return {"value": Probe(headers, body).read_optional_json()} + except ValueError as exc: + return {"error": str(exc)} + + body = b'{"sha256": "ab"}' + print(json.dumps({ + "absent": attempt({}), + "zero_length": attempt({"Content-Length": "0"}), + "present": attempt({"Content-Length": str(len(body))}, body), + "malformed": attempt({"Content-Length": "8"}, b"not json"), + "chunked": attempt({"Transfer-Encoding": "chunked"}), + })) + """ +) + + +def run_probe() -> dict: + # The volume role is the one role core.py does not build a KubeClient for, + # and it relaxes the configuration gate to a single variable. Nothing here + # touches Kubernetes. + environment = { + **os.environ, + "SANDBOX_CONTROL_PLANE_ROLE": "volume", + "VOLUME_AGENT_TOKEN": "test-volume-token", + "PYTHONPATH": str(ROOT), + } + environment.pop("KUBERNETES_SERVICE_HOST", None) + result = subprocess.run( + [sys.executable, "-c", PROBE], + cwd=ROOT, + env=environment, + capture_output=True, + text=True, + timeout=120, + ) + if result.returncode != 0: + raise AssertionError(result.stdout + result.stderr) + return json.loads(result.stdout.strip().splitlines()[-1]) + + +class OptionalRequestBodyTests(unittest.TestCase): + @classmethod + def setUpClass(cls) -> None: + cls.results = run_probe() + + def test_no_body_reads_as_empty(self) -> None: + self.assertEqual(self.results["absent"], {"value": {}}) + + def test_zero_length_body_reads_as_empty(self) -> None: + self.assertEqual(self.results["zero_length"], {"value": {}}) + + def test_a_present_body_is_still_parsed(self) -> None: + self.assertEqual(self.results["present"], {"value": {"sha256": "ab"}}) + + def test_a_present_but_malformed_body_still_fails(self) -> None: + self.assertIn("error", self.results["malformed"], self.results["malformed"]) + + def test_a_chunked_body_is_refused_rather_than_read_as_absent(self) -> None: + # This server never decodes chunked bodies. Silently calling one "no + # body" would drop the sha256 the caller sent and restore the archive + # with no integrity check, and the request would look like it worked. + self.assertIn("error", self.results["chunked"], self.results["chunked"]) + + +class ContractAndHandlerAgreeTests(unittest.TestCase): + def test_every_optional_body_route_uses_the_optional_reader(self) -> None: + spec = yaml.safe_load( + (ROOT / "contracts/control-plane-openapi.yaml").read_text(encoding="utf-8") + ) + optional = [ + (method, path) + for path, item in spec["paths"].items() + for method, operation in item.items() + if isinstance(operation, dict) + and isinstance(operation.get("requestBody"), dict) + and operation["requestBody"].get("required") is False + ] + self.assertEqual( + optional, + [("post", "/v1/workspaces/{workspace_id}/checkpoints/{checkpoint_id}/restore")], + "a new optional-body route needs read_optional_json and a case here", + ) + source = (ROOT / "control_plane/api.py").read_text(encoding="utf-8") + start = source.index('r"([a-z0-9][a-z0-9-]{0,62})/restore"') + # Stop at the next dispatch branch: the block after this one is + # objects/import, which requires its body and rightly calls read_json. + block = source[start:source.index("match = self.match_path(", start)] + self.assertIn("self.read_optional_json()", block) + self.assertNotIn("self.read_json()", block) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_runtime_provider_claims.py b/tests/test_runtime_provider_claims.py new file mode 100644 index 0000000..4790d13 --- /dev/null +++ b/tests/test_runtime_provider_claims.py @@ -0,0 +1,73 @@ +"""Documentation may claim a provider selector only once one exists. + +`README.md` states the rule the project holds itself to: "There is no provider +plug-in surface advertised that does not exist." `docs/ARCHITECTURE.md` broke it +by saying the Runtime provider "is selected independently by +SANDBOX_RUNTIME_DRIVER" while `configured_runtime_driver()` constructs one +driver and never reads that variable - the variable's only effect is an equality +check that refuses every other value. + +The check is conditional on purpose. It reads the selector's presence out of the +source and flips: while there is no selector the prose must not promise one, and +the day a registry lands the same test starts requiring the prose to say so. +""" + +from __future__ import annotations + +import ast +import pathlib +import unittest + + +ROOT = pathlib.Path(__file__).resolve().parents[1] +SELECTOR_VARIABLE = "SANDBOX_RUNTIME_DRIVER" + + +def selector_exists() -> bool: + """Whether ``configured_runtime_driver`` actually dispatches on the name.""" + source = (ROOT / "control_plane/core.py").read_text(encoding="utf-8") + tree = ast.parse(source) + function = next( + node for node in ast.walk(tree) + if isinstance(node, ast.FunctionDef) + and node.name == "configured_runtime_driver" + ) + return any( + isinstance(node, ast.Name) and node.id == SELECTOR_VARIABLE + for node in ast.walk(function) + ) + + +class RuntimeProviderClaimTests(unittest.TestCase): + def test_the_startup_check_still_refuses_every_other_name(self) -> None: + # The premise of the whole file: the variable is a gate, not a switch. + source = (ROOT / "control_plane/core.py").read_text(encoding="utf-8") + self.assertIn(f'if {SELECTOR_VARIABLE} != "gvisor":', source) + + def test_architecture_does_not_promise_a_selector_that_is_absent(self) -> None: + architecture = (ROOT / "docs/ARCHITECTURE.md").read_text(encoding="utf-8") + claim = f"provider is selected independently by `{SELECTOR_VARIABLE}`" + if selector_exists(): + self.assertIn( + claim, + architecture, + "a provider selector now exists; docs/ARCHITECTURE.md should say so", + ) + else: + self.assertNotIn( + claim, + architecture, + "docs/ARCHITECTURE.md advertises a provider selector that " + "configured_runtime_driver() does not implement", + ) + + def test_the_readme_still_states_the_rule_this_enforces(self) -> None: + readme = (ROOT / "README.md").read_text(encoding="utf-8") + self.assertIn( + "There is no provider plug-in\n surface advertised that does not exist.", + readme, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_shell_sessions.py b/tests/test_shell_sessions.py index 344e7c4..9ec8755 100644 --- a/tests/test_shell_sessions.py +++ b/tests/test_shell_sessions.py @@ -597,6 +597,54 @@ def test_close_releases_the_pty_fd_once_the_reader_is_gone(self) -> None: os.fstat(master_fd) +class ActivityProbeBudgetTests(unittest.TestCase): + """The bar the eviction test holds must stay under the probe that enforces it. + + ``SessionManagerEvictionTests`` proves ``activity_snapshot`` is not blocked + by an eviction, and it does so against ``PROBE_BUDGET``, a constant local to + this file. The timeout that actually deletes Runtimes is + ``ACTIVITY_PROBE_TIMEOUT`` in ``control_plane/core.py``, in another + component. Nothing tied the two together, so lowering the control-plane + timeout below the runtime-side bar would leave the suite green while a probe + that timed out started reporting working Runtimes as deletable. + + The default is read out of the source rather than imported: importing + ``control_plane.core`` needs a configured Kubernetes environment, and this + check needs one number. + """ + + @staticmethod + def control_plane_probe_default() -> float: + source = ( + pathlib.Path(__file__).resolve().parents[1] / "control_plane/core.py" + ).read_text(encoding="utf-8") + match = re.search( + r'ACTIVITY_PROBE_TIMEOUT = float\(\s*os\.getenv\(\s*"SANDBOX_ACTIVITY_PROBE_TIMEOUT",\s*"([0-9.]+)"', + source, + ) + assert match, "ACTIVITY_PROBE_TIMEOUT is no longer read from SANDBOX_ACTIVITY_PROBE_TIMEOUT" + return float(match.group(1)) + + def test_the_eviction_bar_stays_below_the_control_plane_probe(self) -> None: + probe = self.control_plane_probe_default() + self.assertLess( + SessionManagerEvictionTests.PROBE_BUDGET, + probe, + "the eviction test asserts a bar the deleting probe no longer allows", + ) + + def test_the_induced_reap_would_actually_cross_that_probe(self) -> None: + # The eviction test is only evidence if its induced slowness is enough + # to trip the real probe when the lock is held. SLOW_REAP below the + # probe timeout would make a passing run prove nothing. + self.assertGreaterEqual( + SessionManagerEvictionTests.SLOW_REAP, + self.control_plane_probe_default(), + "SLOW_REAP no longer exceeds the probe timeout, so holding the lock " + "would not be caught", + ) + + @unittest.skipUnless(os.path.exists(BASH), "bash is required for PTY session tests") class SessionManagerEvictionTests(unittest.TestCase): """Eviction must not reap a process while holding the manager lock. diff --git a/tests/test_workspace_lifecycle.py b/tests/test_workspace_lifecycle.py index 0fae60b..affe1f0 100644 --- a/tests/test_workspace_lifecycle.py +++ b/tests/test_workspace_lifecycle.py @@ -164,6 +164,23 @@ def age(workspace_id): ws = lease["workspace_id"] scoped = lease["access_token"] + # /v1/workspaces/resolve derives the Workspace ID from the caller's own + # credential, so a miss there is "this session has no Workspace yet", + # never "someone is probing another tenant". It must file no denial: + # one per Workspace ever created would bury the cross-tenant rows below, + # which are the only thing this table exists to surface. + results["denied_before_resolve"] = sorted( + (row["action"], row["target"]) + for row in control_plane.STORE.list_audit(limit=200) + if row.get("outcome") == "denied" + ) + call("a_resolve_fresh", "POST", "/v1/workspaces/resolve", key_a, {"session_id": "session-never-used"}) + results["denied_after_resolve"] = sorted( + (row["action"], row["target"]) + for row in control_plane.STORE.list_audit(limit=200) + if row.get("outcome") == "denied" + ) + backdate(ws, 7200) results["age_before"] = age(ws) # Denied by ownership: must not move the clock. @@ -288,6 +305,16 @@ class DeniedAuditThrottleTests(unittest.TestCase): def setUpClass(cls) -> None: cls.results = cached_probe() + def test_resolving_a_session_without_a_workspace_files_no_denial(self) -> None: + # 404 is the right answer and the ownership check still runs; what must + # not happen is an audit row shaped exactly like a cross-tenant probe. + self.assertEqual(self.results["a_resolve_fresh"]["status"], 404) + self.assertEqual( + [tuple(row) for row in self.results["denied_after_resolve"]], + [tuple(row) for row in self.results["denied_before_resolve"]], + "resolve filed a denial for a Workspace the caller's own credential derived", + ) + def test_repeated_denials_by_one_actor_on_one_target_write_one_row(self) -> None: for name in ("b_files", "b_files_again", "b_checkpoints", "b_other_target"): self.assertEqual(self.results[name]["status"], 404, self.results[name])