Skip to content

Bake only the caller's own build box; no redirects in the health probe - #312

Open
vsilent wants to merge 1 commit into
devfrom
fix/bake-only-own-build-box
Open

vsilent wants to merge 1 commit into
devfrom
fix/bake-only-own-build-box

Conversation

@vsilent

@vsilent vsilent commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Security fix for POST /api/admin/bake, found in a review on 2026-10-26.

Problem

  • deployment_id was accepted for any deployment whose server is in the platform's Hetzner project. Quick Deploy buyers' servers live in the same project (124705).
  • Sanitizing is destructive: it removes the SSH host keys and .env and resets the data volumes, then the box is snapshotted. A mistyped id, or a leaked ADMIN_JWT_SECRET, could wreck a paying customer's server and put their data into a snapshot served to later buyers.
  • The old code accepted another user's box with 202; the new test shows it.
  • Separately, the health probe followed redirects. The box decides where a redirect points, so the backend could be made to call an internal address (blind SSRF: only the status code leaks).

Change

  • The deployment must belong to the caller, otherwise 403.
    • admin_service tokens (User.id = "admin_service") own no box and are refused.
    • bake_pipeline.py calls with the CLI token of the admin who deployed the build box, so it is unaffected.
  • A deployment with a managed_usage row (Quick Deploy) is refused with 409, even when the caller made it.
  • probe_health (moved to helpers/bake_job.rs) uses redirect::Policy::none(); a 3xx now fails the health check.

Tests, written failing first:

  • tests/admin_bake.rs, now as a group_admin user who owns the box:
    • three new refusals, with no job recorded: another user's box, a Quick Deploy server, admin_service;
    • the existing cases still pass.
  • Unit test the_health_probe_does_not_follow_a_redirect_off_the_box.
  • Also green, against real Postgres: --lib (2303), quick_deploy_server_type, migration_boot_contract. clippy reports nothing in the changed files.

Note for Stage 5. An admin-ui "Bake" button must call with the admin's own token, not an admin_service JWT.

Merging with #310. Both PRs touch src/routes/admin_bake.rs, in different hunks.

🤖 Generated with Claude Code

POST /api/admin/bake accepted any deployment whose server is in the
platform's Hetzner project. Quick Deploy buyers' servers live there too,
and sanitizing wipes the box (SSH host keys, .env, data volumes) before
snapshotting it, so a mistyped deployment_id could wreck a buyer's server
and put its data into a published snapshot.

- the deployment must belong to the caller (403 otherwise); a service
  token (admin_service) owns no box and is refused. The bake harness calls
  with the CLI token of the admin who deployed the box, so it is unaffected
- a deployment with a Managed-server usage row (Quick Deploy) is refused
  (409), even when the caller made it
- the health probe no longer follows redirects: the box chooses where they
  point, and the backend would call that address for it

Tests: tests/admin_bake.rs (another user's box, a Quick Deploy server,
admin_service; the old code accepted the first with 202) and a unit test
for the redirect, both written failing first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

2 participants