From c0a541814f4ffa7b6a69af515582f7211932e379 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Thu, 24 Sep 2026 11:46:37 +0300 Subject: [PATCH] fix(bake): confirm the box exists before sanitizing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sanitizing removes the operator's own SSH key — deliberately, since a key left in the image would grant its holder root on every clone. The provider lookup happened afterwards, so a target Hetzner could not match left a box that was already cleaned, no longer reachable, and never snapshotted. One wrong flag reaches it: `--server-id` takes the *provider's* server id, and Stacker's own server id is a different number entirely. Passing 702 sanitized a build box and then failed with "server not found", costing a redeploy and a 5 GB model pull. `resolve_snapshot_target` exposes the lookup the snapshot call already performed internally, and bake now runs it first — before the box is touched — reporting the resolved id so an operator can see which machine is about to be captured. Co-Authored-By: Claude Opus 5 --- src/bin/bake.rs | 25 ++++++++++- src/connectors/hetzner.rs | 19 +++++++++ src/helpers/bake.rs | 90 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 2 deletions(-) diff --git a/src/bin/bake.rs b/src/bin/bake.rs index 89ab8675..b441c195 100644 --- a/src/bin/bake.rs +++ b/src/bin/bake.rs @@ -18,7 +18,9 @@ //! `DATABASE_URL` (the stacker Postgres) persists the BakeRecord; without it //! the bake still snapshots and prints the record, but it is not registered. -use stacker::connectors::hetzner::{HetznerCloudClient, HetznerSnapshotTarget}; +use stacker::connectors::hetzner::{ + HetznerCloudClient, HetznerCloudConnector, HetznerSnapshotTarget, +}; use stacker::helpers::bake::run_bake; #[tokio::main] @@ -161,6 +163,26 @@ async fn main() -> Result<(), Box> { stacker::helpers::bake_finalize::check_contract_usable(&protected_keys, allow_unsanitized) .map_err(|e| e.to_string())?; + // Confirm the provider knows this box *before* sanitizing it. Sanitizing + // removes the operator's own SSH key — that is the point, since a key left + // in the image would grant its holder root on every clone — so a target the + // provider cannot match leaves a cleaned box that can no longer be reached + // and never got snapshotted. Happens with one wrong flag: `--server-id` + // takes the *provider's* id, and passing Stacker's own server id instead + // matches nothing. + let connector = HetznerCloudClient::from_env().map_err(|e| e.to_string())?; + let resolved_server_id = connector + .resolve_snapshot_target(&token, &target) + .await + .map_err(|e| { + format!( + "refusing to touch the build box: the Hetzner server could not be \ + identified ({e}). --server-id must be the Hetzner server id, not \ + Stacker's; --ip must be the box's current public address." + ) + })?; + eprintln!("==> Build box resolved to Hetzner server {resolved_server_id}."); + // Sanitize the build box before the snapshot is taken. let finalize_outcome = match (&ssh_key, allow_unsanitized) { (Some(key_path), _) => { @@ -211,7 +233,6 @@ async fn main() -> Result<(), Box> { } }; - let connector = HetznerCloudClient::from_env().map_err(|e| e.to_string())?; let record = run_bake( &connector, &token, target, &stack, &version, healthy, &detail, ) diff --git a/src/connectors/hetzner.rs b/src/connectors/hetzner.rs index acb01d41..6ee1e2b9 100644 --- a/src/connectors/hetzner.rs +++ b/src/connectors/hetzner.rs @@ -121,6 +121,17 @@ pub trait HetznerCloudConnector: Send + Sync { rules: Vec, server_id: i64, ) -> Result; + + /// Resolve a snapshot target to the provider's server id without acting on + /// it. Exposed so a caller that is about to modify the machine can confirm + /// the provider actually knows it first — a wrong id discovered after the + /// build box has been sanitized costs the whole box, since sanitizing + /// removes the operator's own SSH access. + async fn resolve_snapshot_target( + &self, + token: &str, + target: &HetznerSnapshotTarget, + ) -> Result; } #[derive(Clone)] @@ -186,6 +197,14 @@ impl HetznerCloudClient { #[async_trait] impl HetznerCloudConnector for HetznerCloudClient { + async fn resolve_snapshot_target( + &self, + token: &str, + target: &HetznerSnapshotTarget, + ) -> Result { + self.resolve_server_id(token, target).await + } + async fn create_server_snapshot( &self, token: &str, diff --git a/src/helpers/bake.rs b/src/helpers/bake.rs index 7503890e..2f02e91b 100644 --- a/src/helpers/bake.rs +++ b/src/helpers/bake.rs @@ -122,6 +122,13 @@ mod tests { struct MockOk; #[async_trait::async_trait] impl HetznerCloudConnector for MockOk { + async fn resolve_snapshot_target( + &self, + _t: &str, + _target: &HetznerSnapshotTarget, + ) -> Result { + Ok(123) + } async fn create_server_snapshot( &self, _t: &str, @@ -180,6 +187,89 @@ mod tests { } } + /// A connector that cannot match the target — the shape of a wrong + /// `--server-id`, which Hetzner answers with "server not found". + struct MockUnknownTarget; + #[async_trait::async_trait] + impl HetznerCloudConnector for MockUnknownTarget { + async fn resolve_snapshot_target( + &self, + _t: &str, + _target: &HetznerSnapshotTarget, + ) -> Result { + Err(crate::connectors::ConnectorError::NotFound( + "server not found".to_string(), + )) + } + async fn create_server_snapshot( + &self, + _t: &str, + _target: HetznerSnapshotTarget, + _d: &str, + ) -> Result + { + unreachable!("must not be reached when the target does not resolve") + } + async fn create_server_from_image( + &self, + _t: &str, + _r: crate::connectors::hetzner::HetznerCreateServerRequest, + ) -> Result< + crate::connectors::hetzner::HetznerProvisionedServer, + crate::connectors::ConnectorError, + > { + unreachable!() + } + async fn list_server_types( + &self, + _t: &str, + ) -> Result, crate::connectors::ConnectorError> { + Ok(vec![]) + } + async fn add_ssh_key( + &self, + _t: &str, + _n: &str, + _k: &str, + ) -> Result + { + unreachable!() + } + async fn create_firewall( + &self, + _t: &str, + _n: &str, + _r: Vec, + _s: i64, + ) -> Result< + crate::connectors::hetzner::HetznerFirewallResult, + crate::connectors::ConnectorError, + > { + unreachable!() + } + } + + /// The bake binary resolves the target before it sanitizes the box, because + /// sanitizing removes the operator's own SSH key. Getting this order wrong + /// leaves a cleaned, unreachable box and no snapshot — which is what a + /// Stacker server id passed to `--server-id` produced in practice. + #[tokio::test] + async fn an_unmatched_target_is_refused_before_anything_is_snapshotted() { + let target = HetznerSnapshotTarget { + provider_server_id: Some(702), + server_name: None, + public_ip: Some("203.0.113.10".to_string()), + }; + let err = MockUnknownTarget + .resolve_snapshot_target("tok", &target) + .await + .unwrap_err(); + assert!( + err.to_string().contains("server not found"), + "the provider's reason should survive to the caller: {err}" + ); + } + #[tokio::test] async fn run_bake_snapshots_only_when_healthy() { let target = HetznerSnapshotTarget {