From 7ec6f29e77db0c36cc81b21a684a6fe23e5eba5c Mon Sep 17 00:00:00 2001 From: Wauplin Date: Tue, 8 Sep 2026 16:42:40 +0200 Subject: [PATCH] feat: publish a verifiable binary and declare the wire protocol Every sandbox job downloads `sbx-server` from a public bucket, `chmod +x`es it and runs it as root, as PID 1, holding the derived sandbox token. Two things about that were unnecessarily weak. **The object every client consumes is mutable, and the pipeline that writes it was loose.** The workflow published an immutable `sbx-server-` copy -- good -- and then overwrote the `sbx-server` alias that every client actually fetches. It was `workflow_dispatch` against whatever `main` said at that moment, used mutable action tags, and installed the `hf` CLI with `curl -LsSf https://hf.co/cli/install.sh | bash` *in the job holding the bucket-write OIDC token*. That last one is the sharpest edge here: an unpinned remote script running with publish rights over every sandbox of every user. No compromise is suspected -- the published bytes match this source -- the point is that nothing guaranteed the next fetch. **`/health` reported `version`, and nothing negotiated it.** `version` moves for a doc fix as readily as for a protocol break, so a client cannot decide from it whether talking to a server is safe. A pool host keeps the binary it downloaded at boot for up to 24h, so a client and a server from different releases meet in production routinely -- and the recent per-sandbox capability token is exactly the kind of change that fails quietly, as a 403 on some later route. What changed: - `/health` reports `protocol` (an integer, currently 2; 1 was the pre-per-sandbox-token contract). It moves only when the wire contract changes in a way a client can be wrong about. The payload moved into its own `health()` so the handshake can be asserted on without a socket. - `publish.yml` publishes `sbx-server-` plus a manifest (`{version, commit, sha256, target, size}`) as the canonical content-addressed names. The commit-labelled copy and the `sbx-server` alias are still written, as server-side copies of the digest object, so every name in the bucket is byte-identical by construction and existing clients keep working. - The workflow triggers on a `v*` tag, so the published bytes always correspond to a named, reviewed commit. `workflow_dispatch` stays as an emergency path but now requires the ref to be named. - `actions/*` pinned by commit SHA with Dependabot's `# vX.Y.Z` comment. - The `hf` CLI is installed from PyPI at a pinned version in a venv instead of piping a remote script into bash. - `cargo build --locked`, so `Cargo.lock` is authoritative rather than re-resolved at publish time. - Build and publish are separate jobs: the build has no `id-token` permission and no environment, so nothing running during compilation -- a build script, a proc macro, a dependency -- shares a job with the bucket-write credential. The digest is recomputed from the bytes the publish job is about to upload rather than trusted across the boundary. - `environment: production`, so the alias update can be gated on reviewers and the bucket-write OIDC scope can be restricted to it. - README gains a "Releasing" section: pinning the digest in the client couples the two repos, and that trade needs to be written down rather than learned. Not done here: signing and build provenance attestation (`actions/attest-build-provenance` or cosign) over the digest, and client-side signature verification. The pinned digest is what closes the gap for now, because it is baked into a reviewed client release; a signature is the follow-up. Validation: - `cargo test`: 39 passed, including a new case asserting `/health` reports `protocol` as an integer matching `PROTOCOL` -- dropping or renaming that field turns the client's check into a silent no-op. - `cargo build --locked --release --target x86_64-unknown-linux-musl` succeeds; the resulting binary reports `sbx-server 0.6.0 listening on 0.0.0.0:49912 (mode: dedicated, ...)`. - `actionlint 1.7.12` on `publish.yml`: clean. - The bootstrap that consumes this (huggingface_hub side) was exercised against a local HTTP server across 14 image/tool shapes; see that PR. Not validated: the workflow has not been run. It needs a dry run on a branch with the alias-update step disabled, and the `production` environment plus the narrowed OIDC scope have to be created before the first real publish. Companion client change: huggingface_hub branch `security/pin-and-verify-binary` (linked from the PR description). Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/publish.yml | 145 +++++++++++++++++++++++++++------- README.md | 26 +++++- src/main.rs | 124 ++++++++++++++++++++++------- 3 files changed, 237 insertions(+), 58 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 5b3f5f9..d26311b 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -1,58 +1,147 @@ name: Build & publish binary -# Manually triggered. Builds sbx-server from the latest main branch and publishes -# it to the `huggingface/sbx-server` bucket twice: -# 1. sbx-server- -> immutable, commit-labelled history -# 2. sbx-server -> the default file downstream users consume -# The second upload is a server-side remote copy (no re-upload). +# Publishes sbx-server to the `huggingface/sbx-server` bucket under four names: +# 1. sbx-server- -> content-addressed; what a digest-pinned client fetches +# 2. sbx-server-.json -> manifest {version, commit, sha256, target, size} +# 3. sbx-server -> the mutable alias, for clients that predate pinning +# 4. sbx-server- -> commit-labelled history (as before) +# Only (1) and (2) are uploaded; the rest are server-side remote copies of (1), so +# every name in the bucket is byte-identical to the digest by construction. +# +# Triggered by a `v*` tag so the published bytes always correspond to a named, +# reviewed commit. `workflow_dispatch` is kept as an emergency path, but has to be +# told which ref to publish -- neither trigger builds "whatever main says right now". # # Auth uses Trusted Publishers (OIDC) — no HF_TOKEN secret to store/rotate. +# +# Releasing is a two-repo operation: the client pins this binary's digest, so a +# publish is only consumed once `SANDBOX_SERVER_SHA256` in huggingface_hub is +# updated to the digest printed in this run's summary. See README "Releasing". on: + push: + tags: ["v*"] workflow_dispatch: + inputs: + ref: + description: Tag or commit SHA to build and publish + required: true permissions: - id-token: write # required so the job can mint an OIDC token for the HF exchange contents: read +env: + TARGET: x86_64-unknown-linux-musl + jobs: + # Split from `publish` so that nothing running during the build -- a build + # script, a proc macro, a compromised crate -- is in the same job as the + # bucket-write OIDC token. This job cannot mint one: it has no `id-token` + # permission and no environment. + build: + runs-on: ubuntu-latest + outputs: + sha256: ${{ steps.artifact.outputs.sha256 }} + commit: ${{ steps.artifact.outputs.commit }} + steps: + - name: Check out the ref being published + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ inputs.ref || github.ref }} + persist-credentials: false + + - name: Build static musl binary + run: | + rustup target add "$TARGET" + # --locked: publish exactly the dependency versions that were reviewed + # in Cargo.lock, and fail rather than silently resolving newer ones. + cargo build --locked --release --target "$TARGET" + + - name: Compute digest and write the manifest + id: artifact + run: | + set -euo pipefail + binary="target/$TARGET/release/sbx-server" + sha256=$(sha256sum "$binary" | cut -d' ' -f1) + size=$(stat -c%s "$binary") + commit=$(git rev-parse HEAD) + version=$(cargo metadata --format-version 1 --no-deps \ + | python3 -c 'import json,sys; print(json.load(sys.stdin)["packages"][0]["version"])') + mkdir -p dist + cp "$binary" "dist/sbx-server-$sha256" + printf '{"version":"%s","commit":"%s","sha256":"%s","target":"%s","size":%s}\n' \ + "$version" "$commit" "$sha256" "$TARGET" "$size" > "dist/sbx-server-$sha256.json" + { + echo "sha256=$sha256" + echo "commit=$commit" + } >> "$GITHUB_OUTPUT" + + - name: Upload the build for the publish job + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: sbx-server-dist + path: dist/ + if-no-files-found: error + publish: + needs: build runs-on: ubuntu-latest + # Gates the bucket write behind this environment's reviewers, and lets the + # bucket-write OIDC scope be restricted to it: this job overwrites the object + # that every sandbox job downloads and executes as root. + environment: production + permissions: + id-token: write # required so the job can mint an OIDC token for the HF exchange + contents: read env: - TARGET: x86_64-unknown-linux-musl BUCKET: hf://buckets/huggingface/sbx-server # The HF resource the minted OIDC token is scoped to. The `hf` CLI detects # GitHub Actions and performs the token exchange automatically. HF_OIDC_RESOURCE: buckets/huggingface/sbx-server - + SHA256: ${{ needs.build.outputs.sha256 }} + COMMIT: ${{ needs.build.outputs.commit }} steps: - - name: Check out latest main - uses: actions/checkout@v7 + - name: Download the build + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: - ref: main + name: sbx-server-dist + path: dist - - name: Resolve commit hash - id: vars - run: echo "commit=$(git rev-parse --short HEAD)" >> "$GITHUB_OUTPUT" - - - name: Build static musl binary + # The digest is what the client pins, so it has to be recomputed from the + # bytes this job is about to upload rather than trusted from the build job. + - name: Verify the digest before uploading run: | - rustup target add "$TARGET" - cargo build --release --target "$TARGET" + set -euo pipefail + echo "$SHA256 dist/sbx-server-$SHA256" | sha256sum -c - - name: Install the hf CLI + # Pinned PyPI install rather than `curl https://hf.co/cli/install.sh | bash`: + # this job holds the bucket-write credential, so an unpinned remote script + # here runs with publish rights over every sandbox. + run: | + python3 -m venv /tmp/hf-cli + /tmp/hf-cli/bin/pip install --quiet --no-input "huggingface_hub==1.30.0" + echo "/tmp/hf-cli/bin" >> "$GITHUB_PATH" + + - name: Publish the content-addressed binary and its manifest run: | - curl -LsSf https://hf.co/cli/install.sh | bash - echo "$HOME/.local/bin" >> "$GITHUB_PATH" + HF_DEBUG=1 hf cp "dist/sbx-server-$SHA256" "$BUCKET/sbx-server-$SHA256" + HF_DEBUG=1 hf cp "dist/sbx-server-$SHA256.json" "$BUCKET/sbx-server-$SHA256.json" - - name: Upload commit-labelled binary + - name: Copy to the commit-labelled and default names (server-side copies) run: | - HF_DEBUG=1 hf cp \ - "target/$TARGET/release/sbx-server" \ - "$BUCKET/sbx-server-${{ steps.vars.outputs.commit }}" + HF_DEBUG=1 hf cp "$BUCKET/sbx-server-$SHA256" "$BUCKET/sbx-server-${COMMIT:0:7}" + HF_DEBUG=1 hf cp "$BUCKET/sbx-server-$SHA256" "$BUCKET/sbx-server" - - name: Update default binary (server-side copy) + - name: Report the digest to paste into the client run: | - HF_DEBUG=1 hf cp \ - "$BUCKET/sbx-server-${{ steps.vars.outputs.commit }}" \ - "$BUCKET/sbx-server" + { + echo "### Published \`sbx-server\`" + echo + echo '```python' + echo "SANDBOX_SERVER_SHA256 = \"$SHA256\"" + echo '```' + echo + echo "Update that constant in \`huggingface_hub/_sandbox.py\`; until then no" + echo "client fetches these bytes." + } >> "$GITHUB_STEP_SUMMARY" diff --git a/README.md b/README.md index 20380d2..326f243 100644 --- a/README.md +++ b/README.md @@ -42,8 +42,8 @@ ABI-6 abstract-socket scoping). ## HTTP API ``` -GET /health no auth → {"status"} with a valid token → adds version, uptime_ms, - sandboxes, mode, auth, landlock{abi,features} +GET /health no auth → {"status","protocol"} with a valid token → adds version, + uptime_ms, sandboxes, mode, auth, landlock{abi,features} POST /v1/exec {cmd, shell?, env?, cwd?, timeout?, stdin?, background?, tag?} foreground → NDJSON stream: start / stdout / stderr / ping / exit background → {"id", "pid", "tag"} @@ -147,7 +147,27 @@ cargo build --release --target x86_64-unknown-linux-musl ``` The binary is distributed via a Hugging Face bucket and downloaded at job startup by a -`/bin/sh` bootstrap (wget → curl → python3 fallback chain). +`/bin/sh` bootstrap (wget → curl → bucket-mount fallback chain), which verifies the download +against a digest pinned in the client before making it executable. + +## Releasing + +`.github/workflows/publish.yml` runs on a `v*` tag (or a `workflow_dispatch` naming an +explicit ref) and publishes the built binary under its own sha256, plus a manifest, plus the +`sbx-server` alias. Because the client pins that digest, publishing alone changes nothing — +the release is a two-repo operation: + +1. Bump `version` in `Cargo.toml`, and `PROTOCOL` in `src/main.rs` if the wire contract + changed in a way a client can be wrong about. +2. Tag and push; the workflow prints the digest in its run summary. +3. Update `SANDBOX_SERVER_SHA256` (and `SANDBOX_SERVER_VERSION`) in `huggingface_hub`'s + `_sandbox.py`, and its expected protocol if it moved. Until that ships, every job keeps + fetching and verifying the previously pinned digest. + +Pool hosts keep running the binary they downloaded at boot for up to 24h, so a client and a +server from different releases *will* meet in production. That is what the `protocol` field in +`/health` is for: the client refuses the host instead of discovering the difference on some +later route. ## Status diff --git a/src/main.rs b/src/main.rs index ccaecd8..ca44c72 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,6 +17,20 @@ use http::{Request, ResponseWriter}; pub const VERSION: &str = env!("CARGO_PKG_VERSION"); +/// The wire contract this binary speaks, reported by `/health`. +/// +/// `version` is a release label: it moves for a doc fix as readily as for a +/// protocol break, so a client cannot decide from it whether talking to this +/// server is safe. This integer moves only when the contract changes in a way a +/// client can be wrong about, and it matters because a pool host keeps running +/// the binary it downloaded at boot for up to 24h — a new client can meet an +/// old server long after the publish. Bump it, and the client's minimum, on any +/// incompatible change; leave it alone for additive ones. +/// +/// 1: pre-per-sandbox-token. 2: `POST /v1/sandboxes` returns a `token` per +/// sandbox and per-sandbox routes accept it. +pub const PROTOCOL: u32 = 2; + pub fn now_ms() -> i64 { SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_millis() as i64 } @@ -172,6 +186,40 @@ fn route_mode_allowed(host_mode: bool, segments: &[&str]) -> bool { host_mode == host_scoped } +/// The `/health` payload served without a credential. +/// +/// Split out of [`route`] because it is the handshake every client runs before +/// it trusts this server, so it is worth being able to assert on it without a +/// socket. Deliberately just liveness and the protocol number: a client needs +/// both before it has decided anything, and neither tells an unauthenticated +/// caller how to attack this host. +fn public_health() -> serde_json::Value { + serde_json::json!({"status": "ok", "protocol": PROTOCOL}) +} + +/// The `/health` payload served to a caller holding the server's credential. +fn authenticated_health(state: &State) -> serde_json::Value { + serde_json::json!({ + "status": "ok", + "protocol": PROTOCOL, + "version": VERSION, + "uptime_ms": now_ms() - state.started_at_ms, + "sandboxes": state.sandboxes.count(), + "mode": if state.host_mode { "host" } else { "dedicated" }, + // Whether authentication is actually being enforced. A server running + // with it disabled should be able to say so to a client that cares, + // rather than looking identical to one that is not. + "auth": if matches!(state.auth, Auth::Required(_)) { "required" } else { "disabled" }, + // So a client can refuse to run untrusted work on a host whose + // confinement is weaker than it expects, instead of finding out by not + // finding out. + "landlock": { + "abi": landlock::abi(), + "features": landlock::features(landlock::abi()), + }, + }) +} + fn route( state: &Arc, request: &mut Request, @@ -183,35 +231,17 @@ fn route( if method == "GET" && path == "/health" { // Liveness has to stay reachable without a credential: the client polls - // it while a job boots, before it is confident about anything. But the - // *detail* used to come with it, so a read-only namespace member who - // reached the proxy learned the exact server version -- i.e. which known - // issues this host has not been patched for -- plus its uptime and how - // many sandboxes it is packing. + // it while a job boots, before it is confident about anything. The + // protocol number belongs there too, for the same reason -- a client has + // to know whether it can talk to this server *before* it trusts it. + // + // The rest is detail a read-only namespace member should not get: the + // exact server version tells them which known issues this host has not + // been patched for, plus its uptime and how many sandboxes it packs. if !authorized(state, request) { - return resp.json(200, &serde_json::json!({"status": "ok"})); + return resp.json(200, &public_health()); } - return resp.json( - 200, - &serde_json::json!({ - "status": "ok", - "version": VERSION, - "uptime_ms": now_ms() - state.started_at_ms, - "sandboxes": state.sandboxes.count(), - "mode": if state.host_mode { "host" } else { "dedicated" }, - // Whether authentication is actually being enforced. A server - // running with it disabled should be able to say so to a client - // that cares, rather than looking identical to one that is not. - "auth": if matches!(state.auth, Auth::Required(_)) { "required" } else { "disabled" }, - // So a client can refuse to run untrusted work on a host whose - // confinement is weaker than it expects, instead of finding out - // by not finding out. - "landlock": { - "abi": landlock::abi(), - "features": landlock::features(landlock::abi()), - }, - }), - ); + return resp.json(200, &authenticated_health(state)); } let segments: Vec<&str> = path.trim_matches('/').split('/').collect(); @@ -583,6 +613,46 @@ mod tests { path.trim_matches('/').split('/').collect() } + fn test_state() -> State { + State { + auth: Auth::Required("host-management-token".to_string()), + started_at_ms: now_ms(), + last_activity_ms: AtomicI64::new(now_ms()), + procs: exec::ProcRegistry::default(), + sandboxes: sandboxes::SandboxRegistry::new(4, true), + host_mode: true, + compat_host_token: true, + } + } + + /// A client pins a digest of this binary but cannot pin which binary a + /// long-lived pool host already downloaded, so `protocol` is how it finds + /// out. Dropping or renaming the field turns that check into a silent + /// no-op, which is exactly the failure it exists to prevent. + /// + /// It has to be in the *unauthenticated* payload: the client reads it while + /// a job is still booting, before it has decided this server is one it can + /// talk to at all. + #[test] + fn health_advertises_the_protocol_as_an_integer() { + for payload in [public_health(), authenticated_health(&test_state())] { + assert_eq!(payload["protocol"].as_u64(), Some(PROTOCOL as u64)); + } + assert_eq!(authenticated_health(&test_state())["version"].as_str(), Some(VERSION)); + } + + /// The detail is for a caller holding the credential. An unauthenticated + /// one gets liveness and the protocol number, and nothing that says which + /// known issues this host has not been patched for. + #[test] + fn the_public_health_payload_carries_no_server_detail() { + let payload = public_health(); + for field in ["version", "uptime_ms", "sandboxes", "mode", "auth", "landlock"] { + assert!(payload.get(field).is_none(), "public /health leaked {field}"); + } + assert_eq!(payload["status"].as_str(), Some("ok")); + } + /// The whole point of the mode gate: a route that runs with the server's own /// root privileges must not exist in the mode that multiplexes tenants, and /// vice versa. Table-driven so a future refactor cannot quietly re-register