diff --git a/docs/setup/sandbox-troubleshooting.md b/docs/setup/sandbox-troubleshooting.md index 65b022b4e..caa800aa0 100644 --- a/docs/setup/sandbox-troubleshooting.md +++ b/docs/setup/sandbox-troubleshooting.md @@ -752,6 +752,8 @@ A request that reaches the gateway but fails its policy comes back as `403`, and The message names the rule that refused the request, and it points back at this very catalog entry. The full create-time refusal table lives in [`tools/container-gateway/README.md` → What the policy refuses](../../tools/container-gateway/README.md#what-the-policy-refuses). Adjust the request rather than widening the sandbox: a `403` from the gateway is the policy working as intended, not a sandbox misconfiguration. +One refusal is not a `403`: an inspect of a container, pod, volume or network whose name is not one of the project's resources answers `404`, whether the name is missing or another project's, with the same message. +Clients probe for a name that way and carry on only after a `404`, so an untyped `docker inspect `, which asks for a container of that name first, then finds the image. A tool that drives the CLI for you can hide that message, by capturing the CLI's output or by calling an HTTP-upgrade endpoint, for which the CLI prints only the status (`unable to upgrade to h2c, received 403`). The gateway logs every refusal as a `deny : ` line in `/container-gateway.log`, so read the reason there. @@ -761,9 +763,8 @@ Some workflows cannot pass the gateway, however the request is adjusted: - **BuildKit.** `docker buildx build`, and `docker build` wherever buildx is its default, talk to the builder over `POST /grpc` and `/session`, a gRPC stream the gateway cannot inspect, so it refuses both (`deny POST /grpc: unknown-endpoint`). A buildx `docker-container` builder container is label-checked like any other resource and is refused when it does not carry the project label (`label-check on buildx_buildkit_default`). - **The daemon socket inside a container.** A tool that bind-mounts `DOCKER_HOST`'s socket into its own container is refused, since the run directory is never a bind source. -- **An untyped `docker inspect `.** The CLI asks for a container of that name first, and a name that is not a project container gets a label-check `403` (`label-check on ` in the log), where a tool checking whether an image exists may expect a `404`; `docker image inspect` asks for the image directly. -Apache Airflow's `breeze`, for example, runs into all three. +Apache Airflow's `breeze`, for example, runs into both. Excluding it from the sandbox is no way past them, for the reason given under Root cause above. An agent that meets such a refusal stops and reports it to the user, with the `deny` lines from the log. diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py index 902847266..500e02046 100644 --- a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py +++ b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py @@ -21,4 +21,4 @@ for installs that do not carry the framework source (see `isolated.py`). """ -FRAMEWORK_FINGERPRINT = "sha256:3a24bbc916526a0e" +FRAMEWORK_FINGERPRINT = "sha256:ee448afa483c5fb5" diff --git a/tools/container-gateway/README.md b/tools/container-gateway/README.md index 91c093028..146576764 100644 --- a/tools/container-gateway/README.md +++ b/tools/container-gateway/README.md @@ -158,6 +158,8 @@ A volume create gets the project label and is refused when it names a driver oth An exec body is allow-listed to the exec fields, with `Privileged: true` refused; an update body to resource limits and the restart policy. A denial comes back as `403` with a one-line reason both CLIs print verbatim. +An inspect of a container, pod, volume or network whose name is not one of the project's resources is the exception: it answers `404`, whether the name is missing or another project's, because clients probe for a name with an inspect and carry on only after a `404`. +When the daemon gives no usable answer to the label check, the inspect is refused with `403` like every other call. `auth` (registry login), image push, swarm, services, tasks, nodes, plugins, secrets, configs, distribution, session and `system/dial-stdio` are denied outright, along with any path not in the allowed families. So are `checkpoint`, `restore`, `generate` and `play` (the first two write a host path daemon-side, the last two hand the daemon a Kubernetes manifest), and `export` / `import` as query parameters on any route. diff --git a/tools/container-gateway/src/container_gateway/relay.py b/tools/container-gateway/src/container_gateway/relay.py index 7f7f335ca..ad58ab098 100644 --- a/tools/container-gateway/src/container_gateway/relay.py +++ b/tools/container-gateway/src/container_gateway/relay.py @@ -25,13 +25,18 @@ copied from the client's own start line, so a target the policy read differently than the daemon would cannot survive the trip. -*A failed label check answers 403, never 404.* The client is inside the -sandbox and the daemon is root-equivalent on the host; answering 404 for -"exists but belongs to another project" and 403 for "yours" would turn the -gateway into an existence oracle for every other project's containers. The -same reasoning makes every indeterminate backend answer (unreachable, -unparsable, an error status) deny rather than pass: the check fails -closed. +*A failed label check on a by-name call never tells "no such resource" +from "another project's".* The client is inside the sandbox and the daemon is +root-equivalent on the host; one status for the first and another for the +second would turn the gateway into an existence oracle for every other +project's containers. Both get 403, except on an inspect of a container, +pod, volume or network, where both get 404: the docker CLI, compose and +buildx probe for a name with an inspect and carry on only when the answer +is 404 (an untyped ``docker inspect `` asks for a container of that +name first). The same reasoning makes every indeterminate backend answer +(unreachable, unparsable, an error status) deny rather than pass: the +check fails closed, and with a 403 on every route, so that an outage never +reads as "not found". """ from __future__ import annotations @@ -198,28 +203,27 @@ async def _round_trip() -> tuple[int | None, Any]: finally: writer.close() - async def _get_json(self, target: str) -> Any | None: - status, payload = await self._request_json("GET", target) - return payload if status is not None and 200 <= status < 300 else None + async def resource_id_if_labelled(self, route: Route, name: str) -> tuple[str | None, bool]: + """The resource's ID when it carries this project's label, and whether the backend answered. - async def resource_id_if_labelled(self, route: Route, name: str) -> str | None: - """The resource's ID when it carries this project's label, else ``None``. - - ``None`` covers "no such resource" and "someone else's resource" - alike -- the caller answers 403 either way. + A ``None`` ID covers "no such resource" and "someone else's resource" + alike -- the caller gives both the same answer. The flag is false + when the backend gave neither a 2xx nor a 404, so that an outage is + never reported to the client as "not found". """ if route.family is Family.EXEC: - info = await self._get_json(_inspect_target(Family.EXEC, name, route.libpod)) - cid = info.get("ContainerID") if isinstance(info, dict) else None + status, info = await self._request_json("GET", _inspect_target(Family.EXEC, name, route.libpod)) + answered = _answered(status) + cid = info.get("ContainerID") if isinstance(info, dict) and _ok(status) else None if not isinstance(cid, str) or not _IDENT_RE.fullmatch(cid): - return None - owner = await self._get_json(_inspect_target(Family.CONTAINERS, cid, route.libpod)) - return name if has_label(_labels_of(owner), self.ctx.slug) else None - info = await self._get_json(_inspect_target(route.family, name, route.libpod)) - if not isinstance(info, dict) or not has_label(_labels_of(info), self.ctx.slug): - return None + return None, answered + owner = await self._request_json("GET", _inspect_target(Family.CONTAINERS, cid, route.libpod)) + return (name if self._owned(*owner) else None), _answered(owner[0]) + status, info = await self._request_json("GET", _inspect_target(route.family, name, route.libpod)) + if not self._owned(status, info): + return None, _answered(status) ident = str(info.get("Id") or info.get("ID") or info.get("Name") or name) - return ident if _IDENT_RE.fullmatch(ident) else name + return (ident if _IDENT_RE.fullmatch(ident) else name), True async def _resource_denial(self, allow: Allow) -> bytes | None: """403 payload for a named volume / network this project does not own. @@ -370,10 +374,13 @@ async def _forward( # front of it. Nothing to inspect, so nothing to authorise. log.info("deny %s %s: empty resource name", req.method, req.path) return await _refuse(writer, _denial("label-check: empty resource name")) - ident = await self.resource_id_if_labelled(allow.route, name) + ident, answered = await self.resource_id_if_labelled(allow.route, name) if ident is None: log.info("deny %s %s: label-check on %s", req.method, req.path, name) - return await _refuse(writer, _denial(f"label-check: {name} does not belong to this project")) + status = 404 if answered and allow.route.action == "inspect" else 403 + return await _refuse( + writer, _denial(f"label-check: {name} does not belong to this project", status) + ) if allow.name_in_query is not None: # The name came out of the query (compat # `POST /commit?container=`), so the resolved id goes @@ -575,8 +582,17 @@ def _content_type(head: Head) -> str: return (head.get("content-type") or "").split(";")[0].strip().lower() -def _denial(reason: str) -> bytes: - return error_response(403, Deny(reason).message) +def _ok(status: int | None) -> bool: + return status is not None and 200 <= status < 300 + + +def _answered(status: int | None) -> bool: + """The backend settled whether the resource exists: it sent a 2xx or a 404.""" + return _ok(status) or status == 404 + + +def _denial(reason: str, status: int = 403) -> bytes: + return error_response(status, Deny(reason).message) def _label_denial(kind: str, name: str) -> bytes: diff --git a/tools/container-gateway/tests/test_relay.py b/tools/container-gateway/tests/test_relay.py index 2960ccc42..dc9dade0b 100644 --- a/tools/container-gateway/tests/test_relay.py +++ b/tools/container-gateway/tests/test_relay.py @@ -249,6 +249,46 @@ async def scenario() -> None: run(scenario()) +@pytest.mark.parametrize("target", ["containers/theirs/json", "volumes/theirvol", "networks/theirnet"]) +def test_inspect_of_a_foreign_or_missing_resource_is_the_same_404(tmp_path: Path, target: str) -> None: + """An inspect is how clients probe for a name, and they carry on only after a 404.""" + + async def scenario() -> None: + backend, relay = stack(tmp_path) + name = target.split("/")[1] + missing = target.replace(name, "ghost") + foreign_status, _, foreign_body = await call( + relay, f"GET /v1.45/{target} HTTP/1.1\r\nHost: x\r\n\r\n".encode() + ) + ghost_status, _, ghost_body = await call( + relay, f"GET /v1.45/{missing} HTTP/1.1\r\nHost: x\r\n\r\n".encode() + ) + assert foreign_status == ghost_status == 404 + assert b"label-check" in foreign_body + assert foreign_body.replace(name.encode(), b"ghost") == ghost_body + # Only the relay's own unversioned label-check inspect reached the daemon. + assert all(not seen_target.startswith("/v1.45/") for _, seen_target, _ in backend.seen) + owned_status, _, _ = await call(relay, b"GET /v1.45/containers/mine/json HTTP/1.1\r\nHost: x\r\n\r\n") + assert owned_status == 200 + + run(scenario()) + + +def test_inspect_is_403_when_the_backend_cannot_settle_the_label_check(tmp_path: Path) -> None: + """An outage must not read as "not found": a client would go on to create the resource.""" + + async def refuse() -> tuple[asyncio.StreamReader, asyncio.StreamWriter]: + raise ConnectionRefusedError + + async def scenario() -> None: + _, relay = stack(tmp_path) + relay.connect = refuse + status, _, body = await call(relay, b"GET /v1.45/containers/mine/json HTTP/1.1\r\nHost: x\r\n\r\n") + assert status == 403 and b"label-check" in body + + run(scenario()) + + def test_exec_start_checks_owning_container(tmp_path: Path) -> None: async def scenario() -> None: _, relay = stack(tmp_path) diff --git a/tools/spec-loop/specs/container-gateway.md b/tools/spec-loop/specs/container-gateway.md index eb3e2663e..54bed784f 100644 --- a/tools/spec-loop/specs/container-gateway.md +++ b/tools/spec-loop/specs/container-gateway.md @@ -442,7 +442,12 @@ plain create, pass. Rewrites are logged at debug level; denials are returned as `403 {"message": "container-gateway: — ; see docs/setup/sandbox-troubleshooting.md#…"}` so both CLIs print the -reason verbatim. +reason verbatim. A failed label check on an inspect of a container, pod, +volume or network is returned as `404` with the same body, for a missing +name and for another project's alike, because clients probe for a name +with an inspect and carry on only after a `404`. When the backend gives +neither a 2xx nor a 404 to the label check, the inspect is refused with +`403`, so an outage never reads as "not found". ### Egress rule diff --git a/tools/spec-loop/specs/sandbox-diagnostics.md b/tools/spec-loop/specs/sandbox-diagnostics.md index 9eae44b55..f07a503f5 100644 --- a/tools/spec-loop/specs/sandbox-diagnostics.md +++ b/tools/spec-loop/specs/sandbox-diagnostics.md @@ -73,7 +73,7 @@ error, and two skills that probe or verify the setup on demand. (`.mcp.json`, `.bash_profile`: the character-device placeholders bubblewrap mounts over denied paths, #1809). The Docker / Podman entry also covers the container gateway's policy - refusals (BuildKit, a socket bind-mount, an untyped `docker inspect`): + refusals (BuildKit, a socket bind-mount): a tool that drives the docker CLI may hide the 403 body or report only `unable to upgrade to h2c, received 403`, and the expected behaviour is to report the refusal, never to route around the gateway (#1810).