From e2c8b394ccb80b60de10c5739defdb21ae554082 Mon Sep 17 00:00:00 2001 From: convee Date: Sat, 5 Sep 2026 18:11:47 +0000 Subject: [PATCH 01/10] fix(onboarding): remove a false-positive audit event, three docs gaps, and two CLI/Console defects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 5. 修复过程 问题现象：以新人身份从零 clone 走安装-部署-试用全流程，发现六处问题： 1. 每次租户首次使用一个新 Workspace，审计表都会落一条 `workspace.access / denied`，与真正的跨租户越权尝试逐字段同形； 2. `sandbox stop --name demo`（README 教了 `run --name`）被拒，且 CLI 所有参数无任何 help 文本； 3. 控制台英文渲染 `1 minutes ago` / `in 1 minutes`； 4. 控制面在集群外启动只抛 `KeyError: 'KUBERNETES_SERVICE_HOST'`； 5. README 前置表未列 Linux 上 `make doctor` 硬要求的 qemu 与 shasum， 文档第一条命令在合规机器上即失败； 6. DEPLOYMENT.md 说 chart 的四项集群依赖都由 values.yaml 配置， 而 gVisor RuntimeClass 没有对应 value；同文档让运营者自带 PostgreSQL，却未提两个 namespace 都是 enforce=restricted。 根因分析： 1. `/v1/workspaces/resolve` 的 workspace_id 由调用方自己的 tenant_id 经 HMAC 派生（`ws:v2::...`），结构上不可能属于别的租户， 该路径上的所有权失败只可能是"这个 session 还没有 Workspace"。 它却复用了 by-id 路由的审计分支，于是每建一个 Workspace 就伪造一条 入侵信号，把这张表唯一要暴露的东西淹掉。 2. `run` 的位置参数是待执行命令，只能把名字写成 `--name`；其余三个 子命令用位置参数。差异真实存在，但既无 help 也未进文档。 4. `KubeClient.__init__` 直接 `os.environ[...]`，与 core.py:655 自己写下的 "给容器一条能照做的指令，不是堆栈"相悖。 修复方案： 1. `require_workspace_tenant` 增加 `audit_denial` 形参，仅 resolve 站点 传 False；所有权校验本身照旧执行。by-id 路由的审计不变。 2. 补齐全部参数 help（与 sandboxctl 一致），USAGE.md 增补 `sandbox stop` 示例并说明为何只有 run 用 `--name`。 3. 按仓库既有的 Intl.PluralRules 复数机制重构 relative.* 文案： 单位各自成 one/other 家族，复合形式由两个已变复数的片段拼装， 消除 "in 5 hours 1 minutes" 这类单一模板无法回避的错误。 中英渲染结果除 zh 复合形式由「分」统一为「分钟」外保持不变。 4. 两处失败改抛 SystemExit 并说明该怎么做。 5. README 补两项，并新增闸门：README 前置表必须覆盖 dev-doctor.sh 解析出的每个命令（变异验证：还原旧 README 即报出 qemu/shasum 两项）。 6. 按事实改写两处文档。 验证：make verify 全绿（848 用例，原 844 + 4）；四条新断言逐条做过变异 验证；kind 单节点真机复验：resolve 404 后审计计数不变（10→10）， 而 beta 跨租户读 alpha Workspace 仍照常记账。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015s137CobDdyvryGDawa5Rh --- README.md | 4 +- console/src/format.ts | 66 ++++++++++++++++++------------- console/src/i18n/config.ts | 3 ++ console/src/i18n/locales/en.ts | 17 ++++---- console/src/i18n/locales/zh-CN.ts | 17 ++++---- control_plane/api.py | 21 ++++++++-- control_plane/kube.py | 33 ++++++++++++++-- docs/DEPLOYMENT.md | 21 ++++++++-- docs/USAGE.md | 5 +++ sandbox_platform/sandbox_cli.py | 41 ++++++++++++------- tests/test_dev_scripts.py | 40 +++++++++++++++++++ tests/test_kube_client_startup.py | 61 ++++++++++++++++++++++++++++ tests/test_workspace_lifecycle.py | 27 +++++++++++++ 13 files changed, 286 insertions(+), 70 deletions(-) create mode 100644 tests/test_kube_client_startup.py diff --git a/README.md b/README.md index 5c09926..482b56f 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 # 848 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/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/api.py b/control_plane/api.py index 571b467..657a30a 100644 --- a/control_plane/api.py +++ b/control_plane/api.py @@ -1443,11 +1443,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 +1469,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 @@ -2808,7 +2819,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/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 94f482f..f3bc8c2 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -14,9 +14,14 @@ 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 @@ -185,6 +190,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/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/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/tests/test_dev_scripts.py b/tests/test_dev_scripts.py index 32611b7..e93e620 100644 --- a/tests/test_dev_scripts.py +++ b/tests/test_dev_scripts.py @@ -184,6 +184,46 @@ 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 DevelopmentScriptTests(unittest.TestCase): def test_shell_entrypoints_parse(self) -> None: for relative in ( 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_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]) From dbeb6e6e8787b7ed266a3bd901307bd33c57bca4 Mon Sep 17 00:00:00 2001 From: convee Date: Sat, 5 Sep 2026 18:30:28 +0000 Subject: [PATCH 02/10] fix(docs): make the Pending-Pod check discriminate, and document the Helm install order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 5. 修复过程 问题现象：以新人身份第二轮验证（从修复后的树全新 clone）发现两处： 1. TROUBLESHOOTING「Runtime Pod stays Pending」给的检查命令是 `kubectl get nodes --show-labels | grep sandbox-node`，在正确打了标签的 集群上和完全没打标签的集群上**都返回空**； 2. 按 DEPLOYMENT 的顺序装 Helm chart 装不上：chart 自己创建两个 namespace，又不创建任何 Secret，而 Secret 必须先于工作负载存在。 先建 namespace 以便放 Secret，`helm install` 就报 `invalid ownership metadata`。 根因分析： 1. `sandbox-node` 这个串在整个仓库里只出现在这一行，不是任何地方写入的 标签键（真实键是 `sandbox.hullwork.com/node-role`）。读者正在排查 调度失败时，这条命令恒定答"标签不存在"。 test_label_domain 的扫描范围有意排除 docs/，所以没人挡住它。 2. 文档的 Pre-provisioned resources 一节是按 kustomize（`kubectl apply`） 写的，Helm 一节只说"直接 helm install"，两者的先后顺序没有交代。 修复方案： 1. 改成 `kubectl get nodes -l <真实标签>`，空列表本身就是结论； 新增闸门：该小节必须按 k8s/control-plane.yaml 里 SANDBOX_RUNTIME_NODE_SELECTOR 的实际值来选（变异验证：还原旧命令即红）。 2. 新增「Install order」小节，给出实测走通的顺序（install → 建 Secret → 重启三个挂载它们的工作负载），并说明 GitOps 侧不需要重启这步； 新增闸门：渲染结果里凡挂载这四个 Secret 的工作负载都必须在该小节被 点名（变异验证：从文档里去掉 sandbox-volume 即红）。 顺带核实、确认**不是**缺陷的：chart 的 workspace PVC 是 RWX，kind 的 local-path 拒绝——文档已明确要求 RWX StorageClass；控制面启动时那条 WORKSPACE_IDLE_TTL_SECONDS 告警是 core.py 里写明的有意设计，base 与 chart 默认值一致。 验证：make verify 全绿（850 用例）；两条新断言各做过变异验证； kind 上实测走通 helm install 的正确顺序，并确认错误顺序会复现原始报错。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015s137CobDdyvryGDawa5Rh --- README.md | 2 +- docs/DEPLOYMENT.md | 23 +++++++++++++++++++++ docs/TROUBLESHOOTING.md | 6 +++++- tests/test_helm_package.py | 41 ++++++++++++++++++++++++++++++++++++++ tests/test_label_domain.py | 33 ++++++++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 482b56f..e8be2f8 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 # 848 unit and contract tests, no network, no cluster +make test # 850 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 ``` diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index f3bc8c2..a184070 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -27,6 +27,29 @@ schedule and then loop in `RunContainerError`. All four are described in the 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 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/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_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", + ) From fb88fd82459930f2aaaba94880f4ca1768efc08c Mon Sep 17 00:00:00 2001 From: convee Date: Sat, 5 Sep 2026 18:43:42 +0000 Subject: [PATCH 03/10] fix(mcp): report the RuntimeClass the Control Plane placed the Runtime under MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 5. 修复过程 问题现象：第三轮以新人身份试用 MCP bridge 时，在一个**没有 gVisor** 的集群 （SANDBOX_RUNTIME_CLASS 置空，Runtime 跑在集群默认运行时）上调用 `sandbox_status`，返回 `"runtime": "gvisor"`。同一时刻控制面对同一个 sandbox 回的是 `runtime_class: "cluster-default"`，控制台也照实显示 `Isolation: cluster-default`。 根因分析：`sandbox_platform/mcp.py` 把 `"runtime": "gvisor"` 作为**字面量** 合进结果，与部署实际配置无关。这是 agent 用来判断自身被如何隔离的那个面， 而仓库在控制面一侧本就有明确不变量守着同一件事 （`test_empty_runtime_class_reports_cluster_default_not_gvisor_isolation`）， README 也写着「runtime-driver 抽象拒绝假装」。SDK 其实在建箱应答 （SandboxLease.runtime_class，契约里已声明）里收到了真值，只是丢掉了。 修复方案：Lease 记录控制面报告的 runtime_class（建箱与恢复两条路径写入， runtime 被回收/释放的两处清空），`SandboxManager.status()` 增加同名字段， MCP 直接透传，不再注入常量。工具描述改为说明该字段的三种取值 （gvisor / cluster-default / null）。 surface 变化：`sandbox_status` 结果中的 `runtime` 键被 `runtime_class` 取代。 键名与控制面、契约、控制台一致；旧键的值在半数部署上是错的，保留它没有意义。 验证：make verify 全绿（851 用例）；新断言做过变异验证（把常量塞回去即红）； kind 真机复验：MCP 报 cluster-default，与 `GET /v1/sandboxes` 对同一 sandbox id 的回答逐字一致。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015s137CobDdyvryGDawa5Rh --- README.md | 2 +- sandbox_platform/mcp.py | 13 +++++++++++-- sandbox_platform/sandbox_client.py | 11 +++++++++++ tests/test_mcp_contract.py | 27 +++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index e8be2f8..9b8412a 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 # 850 unit and contract tests, no network, no cluster +make test # 851 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 ``` 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_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/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() From 4929d5ed0f14ce3ffb50a4337b098403326a133d Mon Sep 17 00:00:00 2001 From: convee Date: Sun, 6 Sep 2026 00:56:35 +0000 Subject: [PATCH 04/10] fix(api): accept the absent request body the contract declares optional on checkpoint restore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 5. 修复过程 问题现象：第五轮把对象存储真正接上（kind 里起 MinIO，用仓库自带的 bucket-init Job 建桶）后，checkpoint 全路径首次可测。按契约不带 body 调用 `POST /v1/workspaces/{id}/checkpoints/{id}/restore` 返回 `400 Content-Length is required`；带 `Content-Length: 0` 返回 `400 request body must be valid JSON`；只有 `-d '{}'` 才走得通。 而 OpenAPI 对该路由写的是 `requestBody: required: false`，且该 body 只承载 一个可选的 `sha256`。副作用：checkpoint 不存在时也被这条挡在前面，报的是 400 而不是 404。 根因分析：处理器无条件调 `read_json()`，它在缺 Content-Length 时即抛错。 兄弟路由 `POST /checkpoints` 不读 body，所以同一资源上的两个动作行为不一致。 全契约扫描确认只有这一个路由声明了可选 body。 修复方案：新增 `read_optional_json()`，仅此站点使用。 🔴 **不能把「读不到 body」当成「没要求 sha256」**：chunked 编码同样没有 Content-Length，而本服务从不解码 chunked——那样会在调用方明确要求完整性 校验时静默跳过它。因此显式拒绝 Transfer-Encoding，只有真正缺席才当空。 验证：新增 6 条断言，两个方向各做变异（改回 read_json、去掉 chunked 守卫， 都能转红）；make verify 全绿（857 用例）。kind 真机复验六种情况： 无 body/CL:0/`{}` 均 200，错 sha256 与坏 body 仍 400，缺失的 checkpoint 现在正确报 404。 本轮同时核实、**未发现问题**的面：checkpoint 建/列/恢复往返（恢复是替换 语义，与 LIFECYCLE_AND_DATA.md 一致）、sha256 完整性闸门、checkpoint 的 跨租户 404、对象 put/stat/list 的 owner 分区（同租户异 subject、异租户同 subject 均读不到）、`/healthz` 的 object_storage 由 unchecked 转 ok。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015s137CobDdyvryGDawa5Rh --- README.md | 2 +- control_plane/api.py | 24 ++++- tests/test_optional_request_body.py | 149 ++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 tests/test_optional_request_body.py diff --git a/README.md b/README.md index 9b8412a..7e8a621 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 # 851 unit and contract tests, no network, no cluster +make test # 857 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 ``` diff --git a/control_plane/api.py b/control_plane/api.py index 657a30a..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", "")) @@ -2434,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( 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() From 6b7e53793bd15e4c57a40266254e1a25147ea373 Mon Sep 17 00:00:00 2001 From: convee Date: Sun, 6 Sep 2026 01:13:00 +0000 Subject: [PATCH 05/10] fix(bench): report the RuntimeClass that ran and mark truncated environment captures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 5. 修复过程 问题现象：按 docs/BENCHMARKS.md 对 kind 部署跑基准（100 轮 0 失败）后核对证据， `environment.json` 有两处让报告说不清自己测的是什么： 1. `systemPods` / `workloadPods` 是被 `output[:20_000]` 截断的 `kubectl get pods -o json`。本次实测 71,099 与 47,045 字节， 也就是**每次运行**这两块都是从对象中间截断的非法 JSON，且不留任何标记—— 解析的人只拿到 JSONDecodeError，看不出是被截断还是命令半路失败。 2. `runtimeClass` 记的是集群里那个 `gvisor` RuntimeClass 对象，而快照在 run 开始**之前**采集，此时被测 Runtime 一个都还没建。于是一份在 `SANDBOX_RUNTIME_CLASS` 为空（Pod 跑在集群默认运行时）的部署上跑出来的 报告，读起来像 gVisor 测量——而发布报告的头条数字正是「gVisor 冷启动」。 根因分析：runner 的 docstring 写着「summary 与 environment 只从观测派生」， 但隔离这一项是从集群配置这个**代理量**推出来的，不是从发生的事。而 `runtime_cold_start` 每次拿到的 SandboxLease **本来就带** `runtime_class` （契约里早有该字段），收到后被丢弃。 修复方案： 1. 超过上限时记 `truncated` 与 `fullLength`；上限本身不动。 2. 每次冷启动把 lease 的 `runtime_class` 收进集合，summary 增 `observedRuntimeClasses`。缺字段记 `unreported` 而不是猜——老版本控制面 不发这个字段，那既不是 gVisor 的证据也不是反证。 3. BENCHMARKS.md 说明这两个字段，并点明 environment.json 描述的是 「集群提供什么」而非「这些数字跑在什么上面」。 验证：新增 3 条断言，两处各做变异（去掉 lease 记录、去掉截断标记，都能转红）； make verify 全绿（860 用例）；打过补丁的 runner 真机跑 20 轮， summary 出 `observedRuntimeClasses: ["cluster-default"]`， environment 标出两处 71,099→20,000 与 47,045→20,000 的截断。 本次基准读数（kind 单节点 / 无 gVisor / 4 核 EC2 且经 port-forward， **不可与发布报告的数字合并**）：100 轮 0 失败，成功率 100%； 冷启动 p50 3.12s / p95 3.63s，warm exec p50 141ms / p95 194ms， 1KiB 读 p50 132ms、写 p50 148ms；excellent 未达标三项： health.p95 100.6ms（阈值 100ms）、workspace_create.p95 172ms 与 p99 279ms。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015s137CobDdyvryGDawa5Rh --- README.md | 2 +- bench/runner.py | 31 ++++++++++- docs/BENCHMARKS.md | 13 ++++- tests/test_benchmark_runner.py | 98 +++++++++++++++++++++++++++++++++- 4 files changed, 138 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 7e8a621..36a69f2 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 # 857 unit and contract tests, no network, no cluster +make test # 860 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 ``` 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/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/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: From 02813d5bfb46d4f7418cf924c42726c7d5301adc Mon Sep 17 00:00:00 2001 From: convee Date: Sun, 6 Sep 2026 01:20:41 +0000 Subject: [PATCH 06/10] docs(api): write down the object namespace a caller has to obey MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 4. 功能描述 功能概要：docs/API.md 增「Object namespace」一节，说明对象定位器两个 scope 各自必需的字段、path 首段允许的根、生成的 key 形状，以及 workspace import/export 两条路由方向相反的收窄规则。 用户可感知变化：调用对象存储不必再靠试错拼参数。 背景：第六轮以新人身份走 objects 全路径时，我连撞五次 400 才拼出一条能走通 的调用——依次是 `agent_id must be a lowercase DNS-style identifier`、 `object path must start with one of: artifacts, inputs, logs, meta, outputs`、 `workspace path must start with artifacts/`、`only upload objects can be imported`、`object path must start with one of: derived, meta, source`、 `upload destination must start with data/uploads/`。每条报错都点名了允许集合， 所以这套 API **试得出来**；但这些前缀规则在 docs/ 与 contracts/ 里零命中， 只存在于 `control_plane/core.py` 的 `object_location`，也就是**读不出来**。 同时新增闸门 `tests/test_object_namespace_doc.py`：用 AST 取出 `object_location` 里两处 `allowed_roots={...}` 字面量，要求 docs/API.md 逐个 列出；并从 api.py 正则取出 import 目标前缀，要求文档写的是同一个值。 变异验证两个方向：代码加一个根而文档不加 → 红；文档删一个根 → 红。 本轮同时核实、**未发现问题**：workspace export（`artifacts/` 源）与 upload import（`data/uploads/` 目标）双向往返字节完全一致；export 返回完整对象记录 （嵌在 `object` 键下）。 验证：make verify 全绿（862 用例）。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015s137CobDdyvryGDawa5Rh --- README.md | 2 +- docs/API.md | 27 +++++++++++ tests/test_object_namespace_doc.py | 77 ++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/test_object_namespace_doc.py diff --git a/README.md b/README.md index 36a69f2..51fb1c6 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 # 860 unit and contract tests, no network, no cluster +make test # 862 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 ``` 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/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() From 1073effe5f3c37ebb197fee42758908c3fdaab08 Mon Sep 17 00:00:00 2001 From: convee Date: Sun, 6 Sep 2026 01:44:03 +0000 Subject: [PATCH 07/10] fix(lifecycle): tie the eviction probe budget to the timeout that enforces it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 5. 修复过程 问题现象：调研「架构能否换用 Kata/Firecracker 等其它虚拟化」时发现一条 **跨组件的时序不变量**，两端互不知道对方： · `control_plane/core.py:501` `ACTIVITY_PROBE_TIMEOUT` 默认 2 秒， `:3854-3857` 超时即 `return False` —— 判定「不忙」，也就是可删； · `runtime/shell_sessions.py:772-779` 记着实测（2026-08-19）：Runtime 在回收 被 SIGKILL 的子进程时，`activity_snapshot` 要 2.60s，注释原文 「The 2 second probe line has been crossed」，后果是「一个正在工作的 Runtime 连同它所有会话被一起删掉」。 守住它的是一条编码纪律（绝不在持锁时 `close()`）， `tests/test_shell_sessions.py:632` 已有一条**真行为测试**守着这一半 （注入 2.0s 慢回收 + 断言 `activity_snapshot` < `PROBE_BUDGET=1.0`）。 根因分析：`PROBE_BUDGET` 是 runtime 侧测试文件里的本地常量， `ACTIVITY_PROBE_TIMEOUT` 在 control_plane 里，两者**零关联**（各自只在自己 文件中出现）。于是有两个静默失效方向：把控制面超时调到 1.0 以下，既有测试 守的那条线就不再是真线；把它调到 2.0 以上，`SLOW_REAP` 就不足以触发真探针， 那条测试**通过也证明不了任何东西**。两种情况全套测试都是绿的。 `docs/CONFIGURATION.md` 把这个旋钮列了出来，但没说调小它会删掉工作中的 Runtime。 修复方案： 1. 新增 `ActivityProbeBudgetTests`（不依赖 bash，用正则从 core.py 源码取 默认值，避免为一个数字去满足 control_plane 的导入环境）： 断言 `PROBE_BUDGET < 默认探针超时`，且 `SLOW_REAP >= 默认探针超时`。 2. `docs/CONFIGURATION.md` 该行补上后果、那条 2.60s 实测， 并点明**这条余量是隔离后端 syscall 成本的属性，不是这个设置的属性**， 换 RuntimeClass 前必须重测。 验证：两个方向各做变异（探针改 0.8 / 改 5），都能转红； make verify 全绿（864 用例）。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015s137CobDdyvryGDawa5Rh --- README.md | 2 +- docs/CONFIGURATION.md | 2 +- tests/test_shell_sessions.py | 48 ++++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 51fb1c6..d2688ea 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 # 862 unit and contract tests, no network, no cluster +make test # 864 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 ``` 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/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. From 4ab25bcd8f5c35b204218544361f1adbb8c88901 Mon Sep 17 00:00:00 2001 From: convee Date: Sun, 6 Sep 2026 01:49:16 +0000 Subject: [PATCH 08/10] docs(architecture): stop advertising a provider selector that does not exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 5. 修复过程 问题现象：`docs/ARCHITECTURE.md:146` 写「the Runtime provider is selected independently by `SANDBOX_RUNTIME_DRIVER`」，而同一仓库的 `README.md:88-92` 写的是「`SANDBOX_RUNTIME_DRIVER` accepts only `gvisor` ... **There is no provider plug-in surface advertised that does not exist.**」——两句话直接打架， 而代码站在 README 这边。 根因分析：`configured_runtime_driver()`（`core.py:2573-2581`）不读这个变量， 无条件 `return GVisorRuntimeDriver(...)`；该变量唯一作用是 `core.py:247` 的 相等校验。也就是说 ARCHITECTURE 那句宣传的正是 README 声明「不存在」的那个 插件面。评估「能不能换 Kata/Firecracker」的人会先读架构文档，这句话会让他 以为只要改个环境变量。 修复方案：按事实改写该段（变量是启动期校验的闸门，不是选择器），并新增 `tests/test_runtime_provider_claims.py`。该闸门**是条件式的**：用 AST 检查 `configured_runtime_driver` 函数体内是否真的引用了 `SANDBOX_RUNTIME_DRIVER`， 没有选择器时禁止文档承诺选择器，将来选择器落地后同一条测试反过来要求文档 补上。变异验证两向：还原旧措辞 → 红；让函数体引用该变量而文档不改 → 也红。 验证：make verify 全绿（867 用例）。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015s137CobDdyvryGDawa5Rh --- README.md | 2 +- docs/ARCHITECTURE.md | 8 ++- tests/test_runtime_provider_claims.py | 73 +++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 tests/test_runtime_provider_claims.py diff --git a/README.md b/README.md index d2688ea..0336114 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 # 864 unit and contract tests, no network, no cluster +make test # 867 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 ``` 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/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() From 692de56b8e774ffe9514e2b31078af5337a0c69f Mon Sep 17 00:00:00 2001 From: convee Date: Sun, 6 Sep 2026 02:01:54 +0000 Subject: [PATCH 09/10] fix(e2e): stop announcing gVisor after a run that skipped the gVisor check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 5. 修复过程 问题现象：`scripts/test.sh:381` 的收尾行无条件打印 `Sandbox E2E passed: Runtime MCP files/shell + shared Workspace + gVisor`， 而同一脚本 `:141` 的隔离断言是**有条件的**： `assert sys.argv[2] != "gvisor" or "gVisor" in data["stdout"]`。 于是在 `SANDBOX_RUNTIME_CLASS=""` 下——这是文档支持的配置， `control_plane/core.py:257` 的 AI-LOCK 明说节点没有 runsc 时必须置空—— 断言被短路跳过，报告却照样宣布验过了 gVisor。 根因分析：`:138-139` 的注释把条件化的理由讲得很对（跑集群默认运行时时那行 dmesg 是宿主内核的启动日志，断言它必然误报），但收尾那行没有跟着同一个条件。 断言与结论各走各的，于是「跳过检查」被渲染成「检查通过」。 修复方案：收尾行按 `SANDBOX_RUNTIME_CLASS` 分支——是 gvisor 就说 gVisor， 否则报出实际 runtimeClass 并明写 `isolation not asserted`。 新增 `E2EVerdictTests` 三条断言：条件化的前提仍在、收尾行不是常量、 且三种取值下输出正确。 🔴 第三条**从 scripts/test.sh 里把该分支取出来跑**，不是在测试里抄一份—— 抄一份的话改了脚本而没改拷贝，测试会继续验旧代码并照常绿。 变异验证两向：还原无条件那行 → 红；让分支对所有 class 都答 gVisor → 红。 背景：本条是评估「架构能否换用 Kata/Firecracker」时顺带发现的。 `:141` 的守卫写法本身在换后端时也会**静默丢覆盖**（断言整条跳过、测试照常 绿），所以那条守卫不宜当作模板照抄；正确做法是按 handler 分派，且没有对应 取证手法时显式失败。 验证：make verify 全绿（870 用例）；`bash -n scripts/test.sh` 通过。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015s137CobDdyvryGDawa5Rh --- README.md | 2 +- scripts/test.sh | 13 ++++++++- tests/test_dev_scripts.py | 60 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0336114..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 # 867 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 ``` 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_dev_scripts.py b/tests/test_dev_scripts.py index e93e620..4fd7a7c 100644 --- a/tests/test_dev_scripts.py +++ b/tests/test_dev_scripts.py @@ -224,6 +224,66 @@ def test_the_readme_names_every_command_the_doctor_demands(self) -> None: 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 ( From 7ff6006f2c4c1026e02220be721508cc60a36255 Mon Sep 17 00:00:00 2001 From: convee Date: Sun, 6 Sep 2026 02:37:57 +0000 Subject: [PATCH 10/10] build: upgrade libuuid past the util-linux advisories the image gate refuses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 5. 修复过程 问题现象：PR #23 上四个 `images` job 全红，构建本身成功，红在第 5 步 「Scan built image with the release gate's parameters」。Trivy 报 `Total: 7 (HIGH: 7, CRITICAL: 0)`，全部落在 `libuuid`： CVE-2026-53612 / 53613 / 53614（mount TOCTOU）、CVE-2026-76642、 CVE-2026-78408 / 78409 / 78410。main 昨天的 CI 四个 job 都是 success。 根因分析：与本分支无关。本分支不动任何 Dockerfile、`.lock`、`requirements`、 `package-lock`、`pyproject`，镜像内容与 main 的差异只有 .py/.ts 源码。 四个镜像的两个基础镜像（`python:3.14-alpine` 与 `nginxinc/nginx-unprivileged:1.31-alpine3.24`，均 Alpine 3.24.1）都装着 `libuuid-2.42.1-r0`，而这批公告是在 main 上次跑 CI 之后才披露的。 实测确认 Alpine v3.24 main 已发布修复版 `libuuid-2.42.3-r1`。 修复方案：沿用 `console/Dockerfile` 已有的先例（该文件本就为 libexpat 做了 同样的事，并写明「Remove once a newer base digest passes trivy on its own」）， 对四个镜像加 `apk upgrade --no-cache libuuid`：console 那行扩成 `libexpat libuuid`，其余三个各加一条，注释同样点明这是临时措施、 以及它是 FROM 之后唯一一处访问包索引的地方。 验证：四个镜像本地重建后逐个查证 `libuuid-2.42.3-r1`（改动前均为 2.42.1-r0）； make verify 全绿（870 用例）。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015s137CobDdyvryGDawa5Rh --- console/Dockerfile | 12 +++++++----- control_plane/Dockerfile | 8 ++++++++ file-service/Dockerfile | 8 ++++++++ runtime/Dockerfile | 8 ++++++++ 4 files changed, 31 insertions(+), 5 deletions(-) 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/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/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 \