Skip to content

[sandbox audit] Validate a pool host before sending it a credential - #4834

Draft
Wauplin wants to merge 1 commit into
security/per-sandbox-capability-tokensfrom
security/host-adoption-admission-control
Draft

[sandbox audit] Validate a pool host before sending it a credential#4834
Wauplin wants to merge 1 commit into
security/per-sandbox-capability-tokensfrom
security/host-adoption-admission-control

Conversation

@Wauplin

@Wauplin Wauplin commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

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

Why

Pool hosts were found by filtering Jobs on labels — and adopted on that basis alone. Labels
are set by whoever creates the Job, and the nonce the host token is derived from is itself a
public label. So any namespace member who can create a Job could publish one carrying another
user's pool labels plus that host's nonce, and the victim's client would adopt it and send it
the token derived for the real host. The Jobs proxy strips Authorization but forwards
X-Sandbox-Token, so the impostor receives exactly the capability the real host accepts.

Nothing in the old path bound a Job to its creator, image, flavor, command or URL.
_discover_hosts even claimed in its docstring to match on image/flavor — it didn't.

Approach

Adoption now requires everything the backend asserts about a Job to line up, not just the
labels its creator chose:

check why it can't be forged
initiator run_job has no initiator parameter — the backend derives it from the authenticated caller. This is the load-bearing check.
owner namespace backend-assigned
image, flavor backend-recorded from the create request; normalized for registry prefixes and case
command a Job labelled as a host but running something else is not a host
exposed URL exactly one, HTTPS, hostname derived from this job's id — so a Job cannot name where the credentials should go

New adopt_hosts argument on SandboxPool and SandboxPool.connect:

  • "own" (default) — only hosts this principal started. Safe in a shared namespace.
  • "namespace" — restores cross-user host sharing, for namespaces whose members trust each
    other. Still enforces the spec checks.
  • "never" — no adoption; only hosts this handle booted.

A refusal names the opt-in, so someone legitimately sharing hosts isn't left guessing:

Found host job(s) labelled for pool 'pool-x' but none usable: impostor (started by a
different principal (attacker)). Pass adopt_hosts='namespace' to SandboxPool if you intend
to share hosts with other members of this namespace.

SandboxPool.connect's cold path gets the same treatment — it rebuilds the pool's entire
configuration
from a discovered host's spec, so an unvalidated match would let a Job in the
namespace choose the image the pool's next hosts boot. _connect_host addresses a job the
caller named explicitly, so ownership there is the caller's assertion, but the URL and
bootstrap checks still apply.

Also: the bearer is no longer frozen (the L-01 half)

The HF bearer was captured once when the transport was built. A host lives up to 24h, and an
OIDC/OAuth credential can expire inside that window, after which every request failed even
though the handle was fine. An httpx.Auth now reads it per request.

The X-Sandbox-Token is deliberately not re-derived. The server holds the value derived
from the bearer that created the job, so re-deriving from a rotated bearer would produce a
token it has never seen. The two credentials answer different gates — the proxy wants a live
bearer, the server wants the original capability — and only the first needs refreshing.

The consequence worth stating plainly: a host created under one HF token stays reachable only
by holders of that token. That is inherent to deriving the credential from the bearer, not
something this PR can fix, and it is now in the docs rather than being a surprise.

The tests were part of the problem

The discovery fixtures set only labels — so they asserted that a label match is sufficient,
which is the bug. They now describe a Job the way the Jobs API really does, and a new
TestHostAdmission suite covers the hostile cases: an impostor's initiator, a mismatched
image / flavor / command / namespace, a URL belonging to another job, a plain-HTTP URL, extra
exposed ports, and a missing initiator (refused rather than assumed). Plus an end-to-end
create() where the impostor is listed, matches on labels, and is not adopted.

Validation

  • 55 tests pass, up from 44.
  • Writing them caught two real bugs in this change: urlparse lowercases hostnames (breaking
    the URL check on a mixed-case job id), and a fixture whose host had zero capacity.

Not done here

The durable fix is a backend-signed attestation binding a Job to its creator, image
digest, pool and exposed port. This is the client asserting things about a Job it fetched —
good defence in depth, and it closes the practical attack, but it is still the client's
judgement. Worth filing with the Jobs team; initiator being non-settable is what makes the
interim version meaningful, and an authoritative job.creator plus server-side immutability
for a reserved hf-sandbox-* label prefix would close it properly.

Behaviour changes

  • Cross-user host sharing now requires adopt_hosts="namespace". Workflows that relied on
    picking up a colleague's warm host will boot their own host (and bill for it) until they opt
    in. This is the intended trade; the error message points at the flag.
  • proxy_headers resolves the HF token on access rather than returning a captured one.

Note

High Risk
Changes authentication and credential delivery for sandbox pools (host discovery and HTTP auth), which are security-sensitive; default behavior also shifts so colleagues’ hosts are no longer adopted without adopt_hosts="namespace".

Overview
Closes a pool host impersonation hole where any namespace member could create a Job with another pool’s labels (and copied nonce) and receive the victim’s host X-Sandbox-Token. Adoption no longer trusts labels alone: _host_rejection requires backend-asserted initiator (default adopt_hosts="own"), matching image/flavor/bootstrap command, and a single HTTPS expose URL tied to that job id. SandboxPool / connect gain adopt_hosts ("own" | "namespace" | "never"); cross-user warm-host sharing needs an explicit "namespace" opt-in, with errors that mention it.

Long-lived handles no longer freeze the Jobs proxy bearer: _SandboxAuth resolves the HF token on each request; proxy_headers does the same on read. X-Sandbox-Token is unchanged (still derived from the token that created the job).

Docs update the threat model and token/proxy_headers behavior; tests add TestHostAdmission and realistic host job fixtures.

Reviewed by Cursor Bugbot for commit 04c3deb. Bugbot is set up for automated code reviews on this repo. Configure here.

Pool hosts were found by filtering Jobs on labels, and adopted on that
basis alone. Labels are set by whoever creates the Job, and the nonce the
host token is derived from is itself a public label -- so any namespace
member who can create a Job could publish one carrying another user's pool
labels and that host's nonce, and the victim's client would adopt it and
send it the token derived for the *real* host. The Jobs proxy strips
`Authorization` but forwards `X-Sandbox-Token`, so the impostor receives
exactly the capability the real host accepts.

Nothing in the old path bound a Job to its creator, image, flavor, command
or URL. `_discover_hosts` even claimed in its docstring to match on
image/flavor; it did not.

Adoption now requires everything the *backend* asserts about a Job to line
up, not just the labels its creator chose:

- `initiator` -- the load-bearing check, and the one field `run_job` gives no
  way to set, so it cannot be forged client-side;
- owner namespace, image (normalized for registry prefixes and case), flavor,
  and that the command is the sandbox bootstrap;
- exactly one exposed URL, over HTTPS, whose hostname is derived from *this*
  job's id -- so a Job cannot name where the credentials should go.

New `adopt_hosts` argument on `SandboxPool` and `SandboxPool.connect`:
`"own"` (default) adopts only this principal's hosts, `"namespace"` restores
cross-user sharing for namespaces whose members trust each other, `"never"`
disables adoption. A refusal names the opt-in, so someone legitimately
sharing hosts is not left guessing.

`SandboxPool.connect`'s cold path gets the same treatment: it rebuilds the
pool's entire configuration from a discovered host's spec, so an unvalidated
match would let a Job in the namespace choose the image the pool's next
hosts boot. `_connect_host` addresses a job the caller named explicitly, so
ownership there is the caller's assertion, but the URL and bootstrap checks
still apply.

Separately, the HF bearer is no longer frozen when the transport is built.
A host lives up to 24h, and an OIDC/OAuth credential can expire inside that
window, after which every request failed even though the handle was fine. An
`httpx.Auth` now reads it per request. The `X-Sandbox-Token` is deliberately
*not* re-derived: the server holds the value derived from the bearer that
created the job, so re-deriving from a rotated bearer would produce a token
it has never seen. The two credentials answer different gates and only the
bearer needs refreshing. (A host created under one HF token therefore stays
reachable only by holders of that token -- inherent to deriving the
credential, and now stated in the docs.)

The discovery tests were part of the problem: their Job fixtures set only
labels, so they asserted that a label match is sufficient. They now describe
a Job the way the Jobs API really does, and a new suite covers the hostile
cases -- an impostor's initiator, a mismatched image/flavor/command/namespace,
a URL belonging to another job, a plain-HTTP URL, extra exposed ports, and a
missing initiator (refused rather than assumed).

Validation: 55 tests pass, up from 44. Writing them caught two real bugs in
this change -- `urlparse` lowercases hostnames, which broke the URL check on
a mixed-case job id, and a fixture whose host had no capacity.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bot-ci-comment

bot-ci-comment Bot commented Sep 8, 2026

Copy link
Copy Markdown

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 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

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

Reviewed by Cursor Bugbot for commit 04c3deb. Configure here.

"Authorization": f"Bearer {self._auth_token}",
"X-Sandbox-Token": sandbox_token,
},
headers={"X-Sandbox-Token": sandbox_token},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead _auth_token assignment after removing all readers

Low Severity

self._auth_token is assigned via _effective_token(api) but is never read anywhere in the codebase. This PR removed both consumers — the httpx.Client Authorization header and the proxy_headers property — leaving this as a dead store that also triggers an unnecessary token resolution on every _SandboxServer construction.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 04c3deb. Configure here.

# A sandbox host runs the bootstrap script and nothing else. A Job running
# anything else is not a host, whatever its labels say.
if job.command is not None and list(job.command) != _bootstrap_command():
return "does not run the sandbox bootstrap command"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Null command/flavor silently bypass host validation checks

Low Severity

The flavor check (if flavor is not None and job.flavor is not None) and command check (if job.command is not None) both silently pass when the job field is None. The PR table states a job "running something else is not a host," but a job with no command is equally not verifiably running the bootstrap — yet it passes. Under ADOPT_NAMESPACE policy (where initiator is not checked), this is an inconsistency with the stated security model.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 04c3deb. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant