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: