[sandbox audit] Bind the sandbox pool cache to the endpoint, credential and namespace that wrote it

[Wauplin]

[sandbox audit] — PR 4 of 9 in this repo's stack; merge in order.
Previous: #4834 · Next: #4839
Review only the commits this PR adds on top of its base; bases collapse to main as the stack lands.

What was wrong

The local pool cache is not passive data. SandboxPool.connect() rebuilds a whole pool from $HF_HOME/sandbox/pools/<pool-id>.json with no HTTP at all, _seed_hosts_from_cache turns each entry into a _SandboxServer from the cached base_url and nonce, and the first thing that happens to that transport is an authenticated POST /v1/sandboxes carrying the HF bearer and the token derived for that nonce. Two consequences:

1. The cache was not bound to the context that wrote it. The key was the pool id alone, and the file's namespace was read as configuration rather than compared with what the caller asked for:

namespace=cache.namespace if namespace is None else namespace,

So a pool cached for org-A and then reached with connect(pool, namespace="org-B") ran the workload — and its env — on org-A's hosts, while everything the caller could see said org-B. That needs no attacker: one credential and two orgs is an ordinary state of affairs. The key also ignored the endpoint and the credential, so two users sharing a machine (or one user with two accounts) shared each other's entries.

2. The URL was taken on trust. A cache file — or anything able to write $HF_HOME — could name http://127.0.0.1:1234 and receive both credentials on the first create, before any API call could contradict it. follow_redirects=True on that client made a well-formed HTTPS URL enough too, since httpx re-sends Authorization on a same-scheme redirect. verified=False only means "drop the host if it fails"; by then the credentials are gone.

Alongside that, some local hygiene on the same files: the cache temp file was <name>.<pid>.tmp at 0666 & ~umask in a default-mode directory; files.download() wrote in place, so an interrupted download left a truncated file at the final name and open(path, "wb") followed a symlink planted at the destination; files.upload() stated the path and then reopened it. And a well-shaped file with "capacity": "50" parsed fine — dataclass enforces nothing — then failed much later on host.capacity - host.live as an uncaught TypeError in the middle of create().

What changed

Cache binding:

• The cache lives at pools/<context>/<pool-id>.json, where <context> is a digest of endpoint + credential fingerprint + namespace. The same digest is stored in the payload and re-checked on read, so an entry moved or copied into a matching directory is still a miss. The credential appears only as a sha256 fingerprint — the token is never written to disk. Fingerprinting is deliberately local: resolving the credential to a user id would cost a whoami on every read and defeat the point of the cache.
• connect() now honours namespace= instead of inheriting the cached one. Any mismatch on the three is a plain cache miss: debug log, cold path, never an exception.
• A cached base_url is used only if it is the HTTPS jobs-proxy URL of the job that same entry names. This reuses the reasoning already applied to a discovered host's URL — both go through the new _server_host_rejection(url, job_id=...) helper — so a cache file can only name a job the client could have identified by itself. Entries naming anything else are dropped from the file.
• Types and ranges are checked on read (counts are counts, bool is not a count, nonce looks like a nonce, job id can't contain a path separator), so the TypeError failure mode becomes the documented miss.
• follow_redirects=False on the credentialed client. There is no redirect in this protocol; the Jobs proxy serves the in-job server directly.
• Freshness carve-out, so the latency win survives: an entry written by this principal within 15 minutes is still credited with no round-trip — the hf sandbox pool create → hf sandbox create --pool sequence the cache exists for. Past that, inspect_job must confirm the job is running, labelled for this pool, carrying that nonce, adoptable under the handle's adopt_hosts policy, and exposing exactly the cached URL before the transport is built. A definitive "no" prunes the entry; a transient failure just skips it.

Local hardening:

• Cache directories 0700 (tightened even if they predate this), and the atomic write goes through tempfile.mkstemp: 0600 regardless of umask, unguessable name, removed on any failure.
• files.download() writes to an O_EXCL|O_NOFOLLOW, 0600 temp file next to the destination and os.replaces it into place (both the small and the ranged path). rename does not follow symlinks, so a symlinked destination is replaced rather than written through, an interrupted download leaves nothing behind, and the temp name is unpredictable.
• files.upload() opens the file once and both fstats and streams that same descriptor — no stat-then-reopen.

Docs: the cache section of concepts/sandbox.md describes the new key, the freshness rule and the URL admission, and its "keyed by pool id alone / an explicit namespace= still uses the cached hosts" caveat is gone.

Validation

$ ruff check src/huggingface_hub/_sandbox.py src/huggingface_hub/_sandbox_cache.py
All checks passed!
$ ruff format --check src/huggingface_hub/_sandbox.py src/huggingface_hub/_sandbox_cache.py
2 files already formatted
$ PYTHONPATH=src python -m pytest tests/test_sandbox.py -q
83 passed in 21.19s

New cases: cross-credential / cross-namespace / cross-endpoint misses; an entry copied into another context's directory, and a renamed one; mistyped and implausible fields (including that connect() on a mistyped file raises the ordinary "no running host" rather than TypeError); URL admission (loopback, plain HTTP, another job, another port, and a staging domain that must keep working); the freshness carve-out in all three directions (fresh → no inspect_job; aged → confirmed; aged-and-no-longer-backed → dropped and pruned, with a transient API failure only skipping); 0600/0700 modes and no temp file left behind; the redirect; download-over-a-symlink, failed download, and upload through a symlink.

Two of them are the reproducers, asserted against a recording server: a cache naming http://127.0.0.1:<port> and a cross-namespace connect() both result in zero requests reaching it.

Latency, modelling a 100 ms round-trip and timing connect(pool) + create() in the cross-process CLI shape (median of 5, same harness on both branches):

	before	after
cache hit, entry written seconds ago	141 ms	141 ms
cache hit, entry past the 15-min TTL	139 ms	249 ms (one added round-trip)
cache miss (cold path: discovery)	560 ms	557 ms

The common case is unchanged; only entries the fast path can no longer be honest about pay.

Behaviour changes

• Existing cache files become misses (new key, new layout, version 2), so the first create --pool after upgrading takes the cold path once. No migration: rewriting a legacy entry would mean trusting the very fields being bound.
• A pool cached under an explicit namespace= is only found again when the same namespace= is passed — hf sandbox create --pool <id> wants the same --namespace the pool was created with, or it takes the cold path and looks in your own namespace.
• A rotated or swapped credential reads as a different principal and gets a miss (the cold path, which always works).
• A failed download no longer leaves a partial file at the destination.
• upload() still follows a symlinked source: hf_hub_download() returns a symlink into the blobs directory, and refusing those would break a normal use. The TOCTOU fix here is the single descriptor; the source path is one the caller chose in their own filesystem.

Adjacent, not folded in

• upload() of a file above the parallel threshold still materializes it with f.read(). That is the separate full-file-buffering issue, and it belongs with the transfer-memory work rather than here.
• The cache has always bypassed adopt_hosts for hosts it recorded itself (a fresh entry is seeded without the policy check). Pre-existing, and arguably right — the entry was written by this principal — but worth a look when adopt_hosts is next revisited.

---

Note

High Risk
Changes authentication and credential routing for pool cache fast paths and sandbox HTTP clients; mistakes could break connects or, before this fix, leak tokens to attacker-chosen URLs.

Overview
Hardens the sandbox pool fast-path cache and related client behavior so local $HF_HOME files cannot steer HF bearer and host tokens to the wrong namespace, endpoint, or URL.

The on-disk layout moves to pools/<context>/<pool-id>.json (cache v2), where <context> fingerprints endpoint + credential + namespace; SandboxPool.connect() no longer inherits namespace from the file. Cached host URLs must match the entry’s job id (HTTPS jobs-proxy only); entries older than 15 minutes need inspect_job before use. Corrupt or mistyped cache JSON is a cache miss, not a mid-create() error.

Transport and file I/O: the sandbox httpx client sets follow_redirects=False; files.download() writes via a private temp file and atomic replace (symlink-safe); files.upload() sizes and reads one open file descriptor. Cache dirs/files are tightened to 0700/0600 with safer atomic writes.

Docs in concepts/sandbox.md describe the new cache key, freshness rule, and trust model. Existing v1 cache files are ignored (one cold path after upgrade).

^{Reviewed by Cursor Bugbot for commit ee78958. Bugbot is set up for automated code reviews on this repo. Configure here.}

[bot-ci-comment]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit ee78958. Configure here.}

[cursor]

Glob chars wipe other pool caches

Low Severity

Without a context, delete_pool_cache builds a glob pattern from pool_id itself. Metacharacters in that id therefore match other pools' files, so deleting one pool can unlink unrelated cache entries.

 

^{Reviewed by Cursor Bugbot for commit ee78958. Configure here.}