From 0910a33baa2b71716b861429e0dfaf8381e1587a Mon Sep 17 00:00:00 2001 From: Lucain Pouget Date: Tue, 8 Sep 2026 15:39:41 +0200 Subject: [PATCH] feat(sandbox): use each pooled sandbox's own capability token The pool host's token authorized every sandbox on the host plus its management routes, and it was the only credential the client had. So every per-sandbox call -- and `proxy_headers`, which is handed to browsers and WebSocket clients -- offered authority over the whole host. sbx-server 0.6.0 mints a capability token per sandbox. This uses it: - `_SandboxServer.request`/`stream` take an optional `sandbox_token` that overrides `X-Sandbox-Token` for one request; the client-level header keeps carrying the host credential for management calls. - `Sandbox` holds its own token and sends it on every scoped call, including the `DELETE` that kills a pooled sandbox. - `SandboxPool.create()` keeps the token from the create response. - `Sandbox.connect(".")` recovers it via `GET /v1/sandboxes/{id}/token`, so reattaching stays stateless. - `proxy_headers` hands out the sandbox's token, not the host's. Hosts running an older server return no token and have no recovery route; those sandboxes fall back to the host credential, so a pool booted before the server upgrade keeps working. Requires the server change to be deployed first. Also makes the test fake enforce the same scoping as the real server -- host token on management routes, the sandbox's own token on scoped ones -- rather than accepting one value everywhere. That laxness is exactly how a client/server protocol mismatch survives a green suite, and it caught two of this change's own call sites while writing it. Validation: - 44 tests pass, including new ones asserting per-sandbox operations present the scoped token, `proxy_headers` does not leak the host one, and the older-server fallback still works. - Driven end-to-end against a real sbx-server 0.6.0 in a container: A's token cannot exec, read, write, delete or proxy into B, cannot create, list or delete sandboxes, and cannot recover any token; the host token recovers A's token and matches. Co-Authored-By: Claude Opus 5 (1M context) --- docs/source/en/concepts/sandbox.md | 19 ++++--- src/huggingface_hub/_sandbox.py | 80 ++++++++++++++++++++++++++---- tests/test_sandbox.py | 61 +++++++++++++++++++++-- 3 files changed, 140 insertions(+), 20 deletions(-) diff --git a/docs/source/en/concepts/sandbox.md b/docs/source/en/concepts/sandbox.md index 7cbac6f2ac..0585850a38 100644 --- a/docs/source/en/concepts/sandbox.md +++ b/docs/source/en/concepts/sandbox.md @@ -68,14 +68,22 @@ The token is delivered to the server via a Job secret. The client re-derives it ### Token scope -One nonce is minted per **Job**, not per sandbox. That distinction matters in pool mode: +One nonce is minted per **Job**, so the derived token is a *job* credential. In dedicated mode the job is the sandbox, so the two coincide. In pool mode the job is the host, and a host holds many sandboxes — so a pooled sandbox gets a second, narrower credential of its own: -| mode | how many tokens | what a leaked token gives access to | +| credential | derived how | authorizes | | --- | --- | --- | -| dedicated (`Sandbox.create`) | one per job, and the job *is* the sandbox | that sandbox | -| pool (`SandboxPool`) | one per **host job**, shared by every sandbox packed on it | **every sandbox on that host**, current and future, plus the host's management routes (create/list/delete sandboxes) | +| **dedicated sandbox token** | `HMAC(hf_token, nonce)` from the job's label | that sandbox (which is the whole job) | +| **pool host token** | `HMAC(hf_token, nonce)` from the host job's label | pool management: create, list and delete sandboxes on that host, and recover their tokens | +| **pooled sandbox token** | random 256 bits, minted by the host server per sandbox | that one sandbox — not a sibling, not the pool | -So in a pool, a token leak is a host-wide event, not a per-sandbox one. Members of your namespace hold a different HF token and cannot derive yours — but see [Known limitations](#known-limitations) for how a token can be *delivered* to the wrong place. +The client uses the narrow one automatically: `pool.create()` receives it in the create response, `Sandbox.connect(".")` recovers it with the host token, and [`proxy_headers`] hands out the sandbox's token rather than the host's — those headers usually end up in a browser or WebSocket client, so they should confer access to one sandbox and nothing more. + +What this means for a leak: a **pooled sandbox token** compromises that sandbox. A **host token** compromises the host — every sandbox on it, current and future, plus its management routes — so treat it as the pool's admin credential. Members of your namespace hold a different HF token and cannot derive yours, but see [Known limitations](#known-limitations) for how a token can be *delivered* to the wrong place. + +> [!NOTE] +> During the rollout the host server still accepts the host token on per-sandbox routes, so +> clients that predate per-sandbox tokens keep working. That is a management credential +> reaching the sandboxes it created; a sandbox credential can never reach a sibling. ## Dedicated sandboxes (`Sandbox.create`) @@ -229,7 +237,6 @@ This section is deliberately exhaustive rather than reassuring: if you are decid Both require a legitimate caller to invoke the endpoint (the sandbox has no token of its own), so they are confused-deputy problems rather than direct escapes — but they do break confidentiality and integrity between pooled sandboxes. - **Host discovery trusts Job labels.** Hosts are found by filtering Jobs on labels, which any Job creator in the namespace can set, and the nonce that derives the token is a public label. Nothing binds a Job to its creator, image, or pool. In a namespace whose members do not all trust each other, prefer `Sandbox.create`, or use a namespace you control for pools. -- **One token per host.** See [Token scope](#token-scope). - **Landlock can degrade silently.** If Landlock is unavailable, or its ruleset cannot be built, the server currently falls back to uid-only isolation and creates the sandbox anyway — without telling the client. Under uid-only isolation, `/tmp`, `/dev/shm`, TCP bind and cross-home filesystem access are *not* denied. The server also accepts Landlock ABI 1, while the ✅ list above needs ABI 4 (TCP bind) and ABI 6 (abstract sockets); production kernels provide ABI 6, but a lower one would silently drop those two guarantees. - **Residual shared channels**, none of which Landlock or uid isolation closes: unrestricted outbound TCP; loopback access to the control server; **UDP bind is allowed** (Landlock has no UDP coverage); a sibling's `/proc//cmdline` and `status` are readable (`environ` is not — that is the part that would leak credentials, and it is denied); `/proc` and `/sys` are readable and `/dev` is broadly readable and writable; kernel IPC and all machine resources are shared. - **No CPU, disk, FD or total-memory quotas.** Only per-process `RLIMIT_NPROC` and `RLIMIT_AS` are set; cgroup delegation is not available on Jobs. One sandbox can starve its neighbours. The `max_procs`/`max_mem_mb` values are caller-supplied and not clamped server-side. diff --git a/src/huggingface_hub/_sandbox.py b/src/huggingface_hub/_sandbox.py index 0931961558..760465b095 100644 --- a/src/huggingface_hub/_sandbox.py +++ b/src/huggingface_hub/_sandbox.py @@ -100,8 +100,10 @@ def _derive_sandbox_token(hf_token: str, nonce: str) -> str: itself is not. Scope: one nonce is minted per *job*, not per sandbox. For a dedicated sandbox those are - the same thing, but in a pool every sandbox on a host shares that host's token, which - also gates the host's management routes -- so a leak there is host-wide. + the same thing. In a pool this derives the *host* credential, which manages the pool and + can recover per-sandbox tokens -- a leak there is host-wide. Each pooled sandbox also has + its own random capability token, minted by the host server, which is what per-sandbox + operations and `proxy_headers` use. This is not a hardened boundary in either direction. An untrusted image can own the sandbox port, so don't treat it as a guarantee that credentials stay out of the sandbox; @@ -428,23 +430,53 @@ def from_job( def image(self) -> str | None: return self._image - def request(self, method: str, path: str, **kwargs) -> httpx.Response: + def _with_token(self, kwargs: dict, sandbox_token: str | None) -> dict: + """Override `X-Sandbox-Token` for one request. + + The client-level header carries the *host* credential, which manages the + pool. A pooled sandbox's own operations should present that sandbox's + capability token instead, so nothing wider than the sandbox is offered on + a per-sandbox call. + """ + if sandbox_token is not None: + kwargs["headers"] = {**kwargs.get("headers", {}), "X-Sandbox-Token": sandbox_token} + return kwargs + + def request(self, method: str, path: str, *, sandbox_token: str | None = None, **kwargs) -> httpx.Response: """Request to the in-job server. Raises SandboxError on API errors.""" timeout = kwargs.pop("timeout", httpx.Timeout(60.0, connect=10.0)) + kwargs = self._with_token(kwargs, sandbox_token) response = self._client.request(method, self.base_url + path, timeout=timeout, **kwargs) if response.status_code >= 400: _raise_for_status(response) return response @contextmanager - def stream(self, method: str, path: str, **kwargs) -> Iterator[httpx.Response]: + def stream( + self, method: str, path: str, *, sandbox_token: str | None = None, **kwargs + ) -> Iterator[httpx.Response]: """Streaming request to the in-job server. Raises SandboxError on API errors.""" timeout = kwargs.pop("timeout", httpx.Timeout(70.0, connect=10.0)) # server pings every 15s + kwargs = self._with_token(kwargs, sandbox_token) with self._client.stream(method, self.base_url + path, timeout=timeout, **kwargs) as response: if response.status_code >= 400: _raise_for_status(response) yield response + def sandbox_token(self, local_id: str) -> str | None: + """Recover a pooled sandbox's capability token, using the host credential. + + Keeps reconnection stateless: `Sandbox.connect` can reattach to a sandbox + it never created. Returns `None` when the host runs a server that predates + per-sandbox tokens, in which case the caller falls back to the host token. + """ + try: + return self.request("GET", f"/v1/sandboxes/{local_id}/token").json()["token"] + except SandboxError as e: + if e.status_code == 404: + return None # older sbx-server: no such route + raise + def close(self) -> None: self._client.close() @@ -516,9 +548,17 @@ def __init__( local_id: str | None, owns_sandbox: bool, owns_server: bool, + sandbox_token: str | None = None, ) -> None: self.id = id self._server = server + # Capability token for this sandbox alone (pool mode). Sent instead of the + # host credential on every per-sandbox call, so a pooled sandbox's + # operations -- and the headers handed to a port-proxy client -- confer no + # authority over its siblings or over the pool. None in dedicated mode + # (where the job is the sandbox, so the job token is already scoped to it) + # and on hosts running a server that predates per-sandbox tokens. + self._sandbox_token = sandbox_token # None in dedicated mode; the host-local sandbox id in shared mode. self._local_id = local_id # Path prefix for all in-server operations: dedicated routes live under @@ -654,10 +694,20 @@ def connect(cls, sandbox_id: str, *, namespace: str | None = None, token: str | existing = {item["id"] for item in server.request("GET", "/v1/sandboxes").json()} if local_id not in existing: raise SandboxError(f"Sandbox {sandbox_id} no longer exists on host {host_job_id}.") + # Recover the sandbox's capability token with the host credential, so + # this handle operates with the narrow one from here on. + sandbox_token = server.sandbox_token(local_id) except Exception: server.close() # don't leak the HTTP client when the host is gone/unreachable raise - return cls(id=sandbox_id, server=server, local_id=local_id, owns_sandbox=False, owns_server=True) + return cls( + id=sandbox_id, + server=server, + local_id=local_id, + owns_sandbox=False, + owns_server=True, + sandbox_token=sandbox_token, + ) job = api.inspect_job(job_id=sandbox_id, namespace=namespace) labels = job.labels or {} @@ -697,7 +747,7 @@ def _kill(self) -> None: if self._local_id is None: self._server.cancel_job() else: - self._server.request("DELETE", f"/v1/sandboxes/{self._local_id}") + self._server.request("DELETE", f"/v1/sandboxes/{self._local_id}", sandbox_token=self._sandbox_token) except Exception as e: # Don't mark as killed: a later kill() call should retry so nothing leaks. logger.warning(f"Failed to kill sandbox {self.id}: {e}") @@ -925,10 +975,15 @@ def proxy_url_for(self, port: int | str, path: str = "/", *, scheme: str = "http @property def proxy_headers(self) -> dict[str, str]: - """Auth headers to send with [`proxy_url_for`] requests (HF token + sandbox token).""" + """Auth headers to send with [`proxy_url_for`] requests (HF token + sandbox token). + + For a pooled sandbox this is the sandbox's own capability token, not the + pool host's: these headers typically end up in a browser or WebSocket + client, so they should confer access to this sandbox and nothing else. + """ return { "Authorization": f"Bearer {self._server._auth_token}", - "X-Sandbox-Token": self._server._sandbox_token, + "X-Sandbox-Token": self._sandbox_token or self._server._sandbox_token, } def __repr__(self) -> str: @@ -937,11 +992,13 @@ def __repr__(self) -> str: # ------------------------------------------------------------------ internals def _request(self, method: str, resource: str, **kwargs) -> httpx.Response: - return self._server.request(method, self._base_path + resource, **kwargs) + return self._server.request(method, self._base_path + resource, sandbox_token=self._sandbox_token, **kwargs) @contextmanager def _stream(self, method: str, resource: str, **kwargs) -> Iterator[httpx.Response]: - with self._server.stream(method, self._base_path + resource, **kwargs) as response: + with self._server.stream( + method, self._base_path + resource, sandbox_token=self._sandbox_token, **kwargs + ) as response: yield response @@ -1630,6 +1687,9 @@ def _create_one(self, host: "_SandboxServer", env: dict[str, Any], idle_secs: in local_id=item["id"], owns_sandbox=True, owns_server=False, + # Absent on hosts running a server that predates per-sandbox tokens; + # those keep using the host credential. + sandbox_token=item.get("token"), ) sandbox._on_kill = self._on_sandbox_killed return sandbox diff --git a/tests/test_sandbox.py b/tests/test_sandbox.py index f580ef1add..5f9213df3b 100644 --- a/tests/test_sandbox.py +++ b/tests/test_sandbox.py @@ -101,6 +101,20 @@ def _json(self, obj) -> None: self.end_headers() self.wfile.write(body) + def _expect_token(self) -> None: + """Mirror the real server's credential scoping. + + Management routes (`/v1/sandboxes`, token recovery) take the host token; + every per-sandbox route takes that sandbox's own capability token. Keeping + the fake as strict as the server is the point: a lax fake is how a client + bug like addressing a process by pid survives a green test suite. + """ + provided = self.headers["X-Sandbox-Token"] + parts = self.path.split("?")[0].strip("/").split("/") + scoped = len(parts) >= 3 and parts[:2] == ["v1", "sandboxes"] and parts[3:4] != ["token"] + expected = f"tok-{parts[2]}" if scoped else "secret" + assert provided == expected, f"{self.path}: expected {expected!r}, got {provided!r}" + def _exec(self, body) -> None: type(self).last_exec = body self._ndjson( @@ -114,7 +128,7 @@ def _exec(self, body) -> None: ) def do_POST(self) -> None: - assert self.headers["X-Sandbox-Token"] == "secret" + self._expect_token() body = json.loads(self.rfile.read(int(self.headers.get("Content-Length", 0))) or b"{}") cls = type(self) # exec (dedicated /v1/exec or shared /v1/sandboxes//exec) @@ -144,11 +158,11 @@ def do_POST(self) -> None: sid = f"sbx{cls.seq}" cls.seq += 1 cls.sandboxes.add(sid) - created.append({"id": sid}) + created.append({"id": sid, "token": f"tok-{sid}"}) self._json({"sandboxes": created, "rejected": rejected}) def do_DELETE(self) -> None: - assert self.headers["X-Sandbox-Token"] == "secret" + self._expect_token() last = self.path.rsplit("/", 1)[-1] if "/processes/" in self.path: # kill a background process type(self).processes = [p for p in type(self).processes if str(p["pid"]) != last] @@ -158,6 +172,7 @@ def do_DELETE(self) -> None: self._json({"id": last, "deleted": True}) def do_GET(self) -> None: + self._expect_token() cls = type(self) if self.path.startswith("/v1/files/stat") or "/files/stat" in self.path: self._json({"name": "x", "path": "/x", "type": "file", "size": 5}) @@ -170,6 +185,9 @@ def do_GET(self) -> None: self._json(cls.processes) elif self.path == "/v1/sandboxes": self._json([{"id": sid} for sid in sorted(cls.sandboxes)]) + elif self.path.startswith("/v1/sandboxes/") and self.path.endswith("/token"): + sid = self.path.split("/")[3] + self._json({"id": sid, "token": f"tok-{sid}"}) @pytest.fixture() @@ -324,7 +342,15 @@ class TestSharedSandbox: def _make_shared(self, base_url: str) -> Sandbox: server = _make_server(base_url, capacity=10) _FakeServer.sandboxes.add("local1") - return Sandbox(id="job123.local1", server=server, local_id="local1", owns_sandbox=True, owns_server=False) + return Sandbox( + id="job123.local1", + server=server, + local_id="local1", + owns_sandbox=True, + owns_server=False, + # What `SandboxPool.create()` would have received in the create response. + sandbox_token="tok-local1", + ) def test_base_path_is_scoped(self, fake_server: str) -> None: sandbox = self._make_shared(fake_server) @@ -339,6 +365,33 @@ def test_kill_deletes_sandbox_not_job(self, fake_server: str) -> None: sandbox._server._api.cancel_job.assert_not_called() # host keeps running assert "local1" not in _FakeServer.sandboxes + def test_per_sandbox_operations_present_the_sandbox_token(self, fake_server: str) -> None: + # The fake asserts the scoping itself (host token on management routes, the + # sandbox's own token on scoped ones), so any operation reaching the server + # is evidence the narrow credential was sent. + sandbox = self._make_shared(fake_server) + assert sandbox.run("echo").stdout == "out1" + assert sandbox.files.read_text("f") == "hello" + assert sandbox.processes() == [] + + def test_proxy_headers_carry_the_sandbox_token_not_the_host_one(self, fake_server: str) -> None: + # These headers are handed to browsers and WebSocket clients, so they must + # not confer authority over the pool or over sibling sandboxes. + sandbox = self._make_shared(fake_server) + assert sandbox.proxy_headers["X-Sandbox-Token"] == "tok-local1" + assert sandbox.proxy_headers["X-Sandbox-Token"] != sandbox._server._sandbox_token + + def test_falls_back_to_the_host_token_on_an_older_server(self, fake_server: str) -> None: + # A host running a server that predates per-sandbox tokens returns no token + # in the create response; those sandboxes keep working with the host one. + server = _make_server(fake_server, capacity=10) + _FakeServer.sandboxes.add("local1") + sandbox = Sandbox( + id="job123.local1", server=server, local_id="local1", owns_sandbox=True, owns_server=False + ) + assert sandbox._sandbox_token is None + assert sandbox.proxy_headers["X-Sandbox-Token"] == "secret" + class TestSandboxPool: def _pool(self, fake_server: str, monkeypatch, per_host: int = 4) -> SandboxPool: