From 942b8269351d7fa2c2b2e54ea8a69f5f52d019fe Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Fri, 18 Sep 2026 08:22:12 +0300 Subject: [PATCH 01/20] release: bump version to 0.3.3 - Chat session management with archive and encryption - Agent hardening: per-tenant ownership, token digest verification, fail-closed auth - Marketplace field policy: config_contract, generated-field stripping, derived_jwt signing - Project sync, one-click deploy improvements, deployment container tracking - SSH key authorization fixes, mTLS for Vault, port validation - Stale project/server cleanup, audit-log cron, env size validator - Multiple BDD and migration fixes --- CHANGELOG.md | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++- Cargo.lock | 2 +- Cargo.toml | 2 +- 3 files changed, 82 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abc3d2b8..2b6106f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,35 @@ All notable changes to this project will be documented in this file. ## [Unreleased] -### Added — Developer project synchronization +## [0.3.3] — 2026-09-18 + +### Added — Chat session management + +- Added chat session endpoints with archive and per-message encryption. +- Added Casbin RBAC rules for `/api/chat` routes. + +### Added — Agent hardening & ownership + +- Enforced per-tenant ownership on `/v1/agent` routes. +- Agent tokens now verified against a stored digest instead of round-tripping Vault. +- Agent authentication fails closed on Vault errors. +- Agent registration requires the service key; marketplace registration returns 501. +- Accept aggregate `all_health` report from agents for container status. +- Return `project_id` in agent snapshots and one-click clone endpoint. +- Container scope classified from Docker label, not container name. +- Moved `rotate-token` from the console binary to the CLI (`stacker agent rotate-token`). + +### Added — Marketplace field policy & secret federation + +- Added `config_contract` field-policy support (`fixed`/`editable`/`generated` + types: `hex`, `alphanumeric`, `uuid`, `derived_jwt`). +- At publish, `generated`-field values are stripped from the stored `stack_definition` (fail-closed). +- `derived_jwt` fields signed on cloned boxes via HMAC. +- Config contract federated to the User Service. +- Added `backfill_field_policy` one-shot tool to re-gate the existing catalog. +- Policy-driven `generate-secrets.sh` reads `mutability:generated` field policy instead of hardcoding `openssl rand`. +- Added `DisplayType` enum and `display` field to `FieldPolicy`. + +### Added — Project synchronization & one-click deploy - Added `stacker sync` to synchronize declarative project and app configuration with Stacker without creating a deployment, contacting a target server, or @@ -14,6 +42,57 @@ All notable changes to this project will be documented in this file. synchronization. - Added centralized sensitive environment-name redaction and validation for marketplace asset and seed-job metadata. +- One-click clone now registers the server in inventory and creates a cloud firewall. +- One-click clone seeds `project_app` records for the Applications panel. + +### Added — Deployment lifecycle & cleanup + +- Added `deployment_container` table to track containers per deployment (replaces name inference). +- Added deployment container sweeper for stale containers. +- Added stale project and server cleanup with notification (`cleanup-notify` binary). +- Added scheduled audit-log cleanup cron job. +- Added env size validator. + +### Added — Security & infrastructure + +- mTLS for Vault access; Vault client reports missing CA. +- `yaml_quote` now escapes control characters (`\n`, `\r`, `\t`). +- Docker preflight check (`docker info`) before deploy. +- New `W003` warning: `deploy.server.ssh_key` silently ignored on cloud deploys. + +### Fixed — SSH key authorization + +- Cloud deploy now fails when SSH key cannot be stored (was a silent warning). +- Cloud deploy fails when no SSH access is verified after provisioning. +- SSH key authorization retried while the VM boots; both Vault-managed and user keys authorized independently. +- User's configured SSH key from `deploy.cloud.ssh_key` authorized through the correct endpoint. + +### Fixed — CLI & config + +- Fixed `server --dry-run` no longer runs a real Docker deploy (#238). +- Fixed 500 on `PUT /cloud/{id}`: owner set before conversion. +- Fixed #251: escape env/label values so multiline config survives YAML. +- Ports validated by range, not by digit count. +- Port values in `stacker.yml` handled correctly when unquoted. +- Healthcheck `test` field emitted in the form docker compose expects (CMD list vs CMD-SHELL). + +### Fixed — Database & migrations + +- Fixed migration version collisions breaking CI. +- Guard optional cron job lookup to prevent panics. +- Reconcile audit-log cleanup cron after extension install. +- Restored `sqlx prepare` with missing `config_contract` field. + +### Fixed — Auth & Casbin + +- Added missing Casbin rules for admin `detect-secrets` endpoint. +- Agent auth accepts both Vault response shapes for the token. +- Credentials tests no longer read the developer's own config. + +### Fixed — Marketplace BDD + +- BDD marketplace analytics fixtures: cast `template_id` to UUID. +- BDD marketplace scenarios: seed `source_project_id` + deployment. ## [0.3.2] — 2026-08-26 diff --git a/Cargo.lock b/Cargo.lock index c54aea07..57cccdc4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7013,7 +7013,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stacker" -version = "0.3.2" +version = "0.3.3" dependencies = [ "actix", "actix-casbin-auth", diff --git a/Cargo.toml b/Cargo.toml index c110271d..7f5fb148 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "stacker" -version = "0.3.2" +version = "0.3.3" edition = "2021" default-run= "server" From 098c80fe2f7846479979fc8556cf858c5f8e3f2c Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Fri, 18 Sep 2026 10:09:24 +0300 Subject: [PATCH 02/20] logging, db cronjob migration fix, reclaim disk space query / VACUUM --- ...pe_mismatch_cleanup_log_retention.down.sql | 78 +++++++++++++ ...type_mismatch_cleanup_log_retention.up.sql | 106 ++++++++++++++++++ src/bin/bake.rs | 23 +++- src/console/commands/cli/agent.rs | 101 ++++++++++++++++- src/routes/server/ssh_key.rs | 2 +- 5 files changed, 301 insertions(+), 9 deletions(-) create mode 100644 migrations/20260917120000_fix_cleanup_type_mismatch_cleanup_log_retention.down.sql create mode 100644 migrations/20260917120000_fix_cleanup_type_mismatch_cleanup_log_retention.up.sql diff --git a/migrations/20260917120000_fix_cleanup_type_mismatch_cleanup_log_retention.down.sql b/migrations/20260917120000_fix_cleanup_type_mismatch_cleanup_log_retention.down.sql new file mode 100644 index 00000000..24d0ff43 --- /dev/null +++ b/migrations/20260917120000_fix_cleanup_type_mismatch_cleanup_log_retention.down.sql @@ -0,0 +1,78 @@ +-- Revert: restore original stacker_cleanup_terminal_commands, drop cleanup_log cleanup + +-- Unschedule cleanup_log cron job +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_extension WHERE extname = 'pg_cron') THEN + PERFORM cron.unschedule('stacker_cleanup_cleanup_log'); + END IF; +EXCEPTION WHEN OTHERS THEN NULL; +END; +$$; + +-- Drop cleanup_log cleanup function +DROP FUNCTION IF EXISTS stacker_cleanup_cleanup_log(INTERVAL, BOOLEAN); + +-- Restore original stacker_cleanup_terminal_commands (with the type mismatch bug) +CREATE OR REPLACE FUNCTION stacker_cleanup_terminal_commands( + retention INTERVAL DEFAULT INTERVAL '30 days', + p_dry_run BOOLEAN DEFAULT true +) +RETURNS void LANGUAGE plpgsql AS $$ +DECLARE + v_count BIGINT; +BEGIN + IF p_dry_run THEN + SELECT count(*) INTO v_count FROM commands + WHERE status IN ('completed','failed','cancelled') + AND COALESCE(completed_at, updated_at) < NOW() - retention; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'commands', v_count, retention, true); + + SELECT count(*) INTO v_count FROM dead_letter_queue + WHERE status IN ('exhausted','discarded','resolved') + AND updated_at < NOW() - retention; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'dead_letter_queue', v_count, retention, true); + + SELECT count(*) INTO v_count FROM pipe_executions + WHERE completed_at < NOW() - retention; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'pipe_executions', v_count, retention, true); + + SELECT count(*) INTO v_count FROM pipe_dag_step_executions + WHERE completed_at < NOW() - retention; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'pipe_dag_step_executions', v_count, retention, true); + ELSE + -- Delete command_queue entries for terminal commands first (FK) + DELETE FROM command_queue WHERE command_id IN ( + SELECT id FROM commands + WHERE status IN ('completed','failed','cancelled') + AND COALESCE(completed_at, updated_at) < NOW() - retention + ); + + DELETE FROM commands + WHERE status IN ('completed','failed','cancelled') + AND COALESCE(completed_at, updated_at) < NOW() - retention; + GET DIAGNOSTICS v_count = ROW_COUNT; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'commands', v_count, retention, false); + + DELETE FROM dead_letter_queue + WHERE status IN ('exhausted','discarded','resolved') + AND updated_at < NOW() - retention; + GET DIAGNOSTICS v_count = ROW_COUNT; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'dead_letter_queue', v_count, retention, false); + + -- FK-safe order: step executions before executions + DELETE FROM pipe_dag_step_executions WHERE completed_at < NOW() - retention; + + DELETE FROM pipe_executions WHERE completed_at < NOW() - retention; + GET DIAGNOSTICS v_count = ROW_COUNT; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'pipe_executions', v_count, retention, false); + END IF; +END; +$$; diff --git a/migrations/20260917120000_fix_cleanup_type_mismatch_cleanup_log_retention.up.sql b/migrations/20260917120000_fix_cleanup_type_mismatch_cleanup_log_retention.up.sql new file mode 100644 index 00000000..7a02df8d --- /dev/null +++ b/migrations/20260917120000_fix_cleanup_type_mismatch_cleanup_log_retention.up.sql @@ -0,0 +1,106 @@ +-- Fix stacker_cleanup_terminal_commands type mismatch and add cleanup_log retention + +-- 1. Fix stacker_cleanup_terminal_commands: command_queue.command_id is VARCHAR +-- referencing commands.command_id (VARCHAR), NOT commands.id (UUID). +-- The original function used `SELECT id FROM commands` which caused: +-- ERROR: operator does not exist: character varying = uuid +CREATE OR REPLACE FUNCTION stacker_cleanup_terminal_commands( + retention INTERVAL DEFAULT INTERVAL '30 days', + p_dry_run BOOLEAN DEFAULT true +) +RETURNS void LANGUAGE plpgsql AS $$ +DECLARE + v_count BIGINT; +BEGIN + IF p_dry_run THEN + SELECT count(*) INTO v_count FROM commands + WHERE status IN ('completed','failed','cancelled') + AND COALESCE(completed_at, updated_at) < NOW() - retention; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'commands', v_count, retention, true); + + SELECT count(*) INTO v_count FROM dead_letter_queue + WHERE status IN ('exhausted','discarded','resolved') + AND updated_at < NOW() - retention; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'dead_letter_queue', v_count, retention, true); + + SELECT count(*) INTO v_count FROM pipe_executions + WHERE completed_at < NOW() - retention; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'pipe_executions', v_count, retention, true); + + SELECT count(*) INTO v_count FROM pipe_dag_step_executions + WHERE completed_at < NOW() - retention; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'pipe_dag_step_executions', v_count, retention, true); + ELSE + -- Delete command_queue entries for terminal commands first (FK) + -- Use command_id (VARCHAR) not id (UUID) to match command_queue.command_id type + DELETE FROM command_queue WHERE command_id IN ( + SELECT command_id FROM commands + WHERE status IN ('completed','failed','cancelled') + AND COALESCE(completed_at, updated_at) < NOW() - retention + ); + + DELETE FROM commands + WHERE status IN ('completed','failed','cancelled') + AND COALESCE(completed_at, updated_at) < NOW() - retention; + GET DIAGNOSTICS v_count = ROW_COUNT; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'commands', v_count, retention, false); + + DELETE FROM dead_letter_queue + WHERE status IN ('exhausted','discarded','resolved') + AND updated_at < NOW() - retention; + GET DIAGNOSTICS v_count = ROW_COUNT; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'dead_letter_queue', v_count, retention, false); + + -- FK-safe order: step executions before executions + DELETE FROM pipe_dag_step_executions WHERE completed_at < NOW() - retention; + + DELETE FROM pipe_executions WHERE completed_at < NOW() - retention; + GET DIAGNOSTICS v_count = ROW_COUNT; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_terminal_commands', 'pipe_executions', v_count, retention, false); + END IF; +END; +$$; + + +-- 2. Add cleanup_log self-cleanup (90 days retention) +-- cleanup_log accumulates entries from all 7 cleanup functions daily but has no purge. +CREATE OR REPLACE FUNCTION stacker_cleanup_cleanup_log( + retention INTERVAL DEFAULT INTERVAL '90 days', + p_dry_run BOOLEAN DEFAULT true +) +RETURNS void LANGUAGE plpgsql AS $$ +DECLARE + v_count BIGINT; +BEGIN + IF p_dry_run THEN + SELECT count(*) INTO v_count FROM cleanup_log WHERE run_at < NOW() - retention; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_cleanup_log', 'cleanup_log', v_count, retention, true); + ELSE + DELETE FROM cleanup_log WHERE run_at < NOW() - retention; + GET DIAGNOSTICS v_count = ROW_COUNT; + INSERT INTO cleanup_log (function_name, table_name, rows_deleted, retention, dry_run) + VALUES ('stacker_cleanup_cleanup_log', 'cleanup_log', v_count, retention, false); + END IF; +END; +$$; + + +-- 3. Schedule cleanup_log purge (weekly Sunday 5:30 AM) +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_extension WHERE extname = 'pg_cron') THEN + IF NOT EXISTS (SELECT 1 FROM cron.job WHERE jobname = 'stacker_cleanup_cleanup_log') THEN + PERFORM cron.schedule('stacker_cleanup_cleanup_log', '30 5 * * 0', + $cron$SELECT stacker_cleanup_cleanup_log(p_dry_run := false);$cron$); + END IF; + END IF; +END; +$$; diff --git a/src/bin/bake.rs b/src/bin/bake.rs index 28bf542b..fb5ba0fb 100644 --- a/src/bin/bake.rs +++ b/src/bin/bake.rs @@ -106,9 +106,20 @@ async fn main() -> Result<(), Box> { let config_contract = match stacker::db::marketplace::get_approved_by_slug(&pool, &record.stack).await { Ok(Some(template)) => { + eprintln!( + "DEBUG: resolved template '{}' id={} for stack '{}'", + template.name, template.id, record.stack + ); match stacker::db::marketplace::get_config_contract(&pool, template.id).await { - Ok(serde_json::Value::Null) => None, - Ok(contract) => Some(contract), + Ok(serde_json::Value::Null) => { + eprintln!("DEBUG: config_contract is Null for template id={}", template.id); + None + } + Ok(contract) => { + eprintln!("DEBUG: config_contract resolved, keys={:?}", + contract.as_object().map(|o| o.keys().collect::>())); + Some(contract) + } Err(err) => { eprintln!( "WARNING: could not read config_contract for '{}': {err}", @@ -118,7 +129,10 @@ async fn main() -> Result<(), Box> { } } } - Ok(None) => None, + Ok(None) => { + eprintln!("DEBUG: no approved template found for stack '{}'", record.stack); + None + } Err(err) => { eprintln!( "WARNING: could not resolve template for '{}': {err}", @@ -128,6 +142,9 @@ async fn main() -> Result<(), Box> { } }; + eprintln!("DEBUG: config_contract to record: {:?}", + config_contract.as_ref().map(|c| c.as_object().map(|o| o.keys().collect::>()))); + let row = stacker::db::baked_snapshot::record( &pool, &record.stack, diff --git a/src/console/commands/cli/agent.rs b/src/console/commands/cli/agent.rs index 6e18d438..d0ad16df 100644 --- a/src/console/commands/cli/agent.rs +++ b/src/console/commands/cli/agent.rs @@ -1754,11 +1754,15 @@ impl CallableTrait for AgentStatusCommand { n_apps, ), ); - let live_containers = match fetch_live_containers(&ctx, &hash) { - Ok(list) => list, - Err(err) => { - eprintln!("Warning: failed to fetch live containers: {}", err); - None + let live_containers = if agent_status == "offline" { + None + } else { + match fetch_live_containers(&ctx, &hash) { + Ok(list) => list, + Err(err) => { + eprintln!("Warning: failed to fetch live containers: {}", err); + None + } } }; @@ -2260,10 +2264,32 @@ fn run_logs_command( ) } +/// Returns `true` if the agent for the given deployment is offline. +/// +/// Fetches a lightweight snapshot to check status before attempting +/// any agent commands that would hang waiting for an unreachable agent. +fn is_agent_offline(ctx: &CliRuntime, deployment_hash: &str) -> bool { + let snapshot = ctx.block_on(ctx.client.agent_snapshot(deployment_hash)); + match snapshot { + Ok(snap) => { + let item = snap.get("item").unwrap_or(&snap); + item.get("agent") + .and_then(|a| a.get("status")) + .and_then(|s| s.as_str()) + == Some("offline") + } + Err(_) => false, // If we can't fetch snapshot, let the caller try + } +} + pub(crate) fn fetch_live_containers( ctx: &CliRuntime, deployment_hash: &str, ) -> Result>, CliError> { + if is_agent_offline(ctx, deployment_hash) { + return Ok(None); + } + let params = crate::forms::status_panel::ListContainersCommandRequest { include_health: true, include_logs: false, @@ -4341,6 +4367,71 @@ monitoring: None => std::env::remove_var("STACKER_DOCKER_REGISTRY"), } } + + // ── Agent status snapshot parsing ──────────────────────────────────── + + #[test] + fn extract_agent_status_from_snapshot_offline() { + let snap = serde_json::json!({ + "item": { + "agent": { "status": "offline", "version": "1.0.0" }, + "apps": [] + } + }); + let item = snapshot_item(&snap); + let status = item + .get("agent") + .and_then(|a| a.get("status")) + .and_then(|s| s.as_str()) + .unwrap_or("unknown"); + assert_eq!(status, "offline"); + } + + #[test] + fn extract_agent_status_from_snapshot_online() { + let snap = serde_json::json!({ + "item": { + "agent": { "status": "online", "version": "1.0.0" }, + "apps": [] + } + }); + let item = snapshot_item(&snap); + let status = item + .get("agent") + .and_then(|a| a.get("status")) + .and_then(|s| s.as_str()) + .unwrap_or("unknown"); + assert_eq!(status, "online"); + } + + #[test] + fn extract_agent_status_missing_defaults_unknown() { + let snap = serde_json::json!({ "item": { "apps": [] } }); + let item = snapshot_item(&snap); + let status = item + .get("agent") + .and_then(|a| a.get("status")) + .and_then(|s| s.as_str()) + .unwrap_or("unknown"); + assert_eq!(status, "unknown"); + } + + #[test] + fn snapshot_item_prefers_item_key() { + let snap = serde_json::json!({ + "item": { "agent": { "status": "online" } }, + "agent": { "status": "offline" } + }); + let item = snapshot_item(&snap); + assert_eq!(item["agent"]["status"], "online"); + } + + #[test] + fn snapshot_item_falls_back_to_root() { + let snap = serde_json::json!({ "agent": { "status": "offline" } }); + let item = snapshot_item(&snap); + assert_eq!(item["agent"]["status"], "offline"); + } } // ── rotate-token ──────────────────────────────────────────────────────────── diff --git a/src/routes/server/ssh_key.rs b/src/routes/server/ssh_key.rs index 0de36d81..4b8c4035 100644 --- a/src/routes/server/ssh_key.rs +++ b/src/routes/server/ssh_key.rs @@ -710,7 +710,7 @@ pub async fn validate_all( }; } - let vault_key_path = match &server.vault_key_path { + let _vault_key_path = match &server.vault_key_path { Some(p) if !p.is_empty() => p, _ => { return ValidateResponse { From 108be8c1cbaed215182b1676d3a04a2063cf7f75 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Fri, 18 Sep 2026 12:02:05 +0300 Subject: [PATCH 03/20] fix: allow re-submitting approved marketplace templates - create_handler now uses update_metadata_for_resubmit for submitted/under_review/approved templates - CLI submit command uses resubmit endpoint for approved templates instead of submit endpoint - adds marketplace_resubmit client method for POST /api/templates/{id}/resubmit --- src/cli/stacker_client.rs | 35 +++++++++++++++ src/console/commands/cli/submit.rs | 21 ++++++--- src/routes/marketplace/creator.rs | 71 ++++++++++++++++++++++-------- 3 files changed, 103 insertions(+), 24 deletions(-) diff --git a/src/cli/stacker_client.rs b/src/cli/stacker_client.rs index 340a2a1f..df25763b 100644 --- a/src/cli/stacker_client.rs +++ b/src/cli/stacker_client.rs @@ -3609,6 +3609,41 @@ impl StackerClient { Ok(()) } + + /// Resubmit an approved/rejected/needs_changes template with a new version. + pub async fn marketplace_resubmit( + &self, + template_id: &str, + body: serde_json::Value, + ) -> Result<(), CliError> { + let url = format!("{}/api/templates/{}/resubmit", self.base_url, template_id); + let resp = self + .http + .post(&url) + .bearer_auth(&self.token) + .json(&body) + .send() + .await + .map_err(|e| { + CliError::MarketplaceFailed(format!("Stacker server unreachable: {}", e)) + })?; + + if !resp.status().is_success() { + let status = resp.status().as_u16(); + let body = resp.text().await.unwrap_or_default(); + return Err(CliError::MarketplaceFailed( + stacker_api_failure_with_message( + "Resubmit failed", + &format!("POST /api/templates/{template_id}/resubmit"), + status, + &body, + cli_debug_enabled(), + ), + )); + } + + Ok(()) + } } // ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ diff --git a/src/console/commands/cli/submit.rs b/src/console/commands/cli/submit.rs index de3608f8..9aa1932a 100644 --- a/src/console/commands/cli/submit.rs +++ b/src/console/commands/cli/submit.rs @@ -153,11 +153,22 @@ impl CallableTrait for SubmitCommand { // Create or update the template on the server eprintln!("Creating/updating template '{}'...", name); - let template = client.marketplace_create_or_update(body).await?; - - // Submit for review - eprintln!("Submitting for marketplace review..."); - client.marketplace_submit(&template.id).await?; + let template = client.marketplace_create_or_update(body.clone()).await?; + + // Submit for review — use resubmit endpoint for templates that + // are already approved (submit_for_review only allows + // draft/rejected/needs_changes). + if template.status == "approved" { + eprintln!("Resubmitting approved template for marketplace review..."); + let mut resubmit_body = body.clone(); + resubmit_body["confirm_no_secrets"] = serde_json::json!(true); + client + .marketplace_resubmit(&template.id, resubmit_body) + .await?; + } else { + eprintln!("Submitting for marketplace review..."); + client.marketplace_submit(&template.id).await?; + } // Success message println!(); diff --git a/src/routes/marketplace/creator.rs b/src/routes/marketplace/creator.rs index a4da4772..219105fd 100644 --- a/src/routes/marketplace/creator.rs +++ b/src/routes/marketplace/creator.rs @@ -165,25 +165,58 @@ pub async fn create_handler( let template = if let Some(existing_template) = existing { // Update existing template tracing::info!("Updating existing template with slug: {}", req.slug); - let updated = db::marketplace::update_metadata( - pg_pool.get_ref(), - &existing_template.id, - Some(&req.name), - req.short_description.as_deref(), - req.long_description.as_deref(), - req.category_code.as_deref(), - Some(tags.clone()), - Some(tech_stack.clone()), - Some(infrastructure_requirements.clone()), - Some(price), - Some(billing_cycle.as_str()), - req.required_plan_name.as_deref(), - Some(currency.as_str()), - req.public_ports.clone(), - req.vendor_url.as_deref(), - ) - .await - .map_err(|err| JsonResponse::::build().internal_server_error(err))?; + + // Use the resubmit-aware update for templates that are already + // submitted, under review, or approved — `update_metadata` only + // allows draft/rejected/needs_changes. + let updated = if matches!( + existing_template.status.as_str(), + "submitted" | "under_review" | "approved" + ) { + db::marketplace::update_metadata_for_resubmit( + pg_pool.get_ref(), + &existing_template.id, + Some(&req.name), + req.short_description.as_deref(), + req.long_description.as_deref(), + req.category_code.as_deref(), + Some(tags.clone()), + Some(tech_stack.clone()), + Some(infrastructure_requirements.clone()), + Some(price), + Some(billing_cycle.as_str()), + req.required_plan_name.as_deref(), + Some(currency.as_str()), + req.public_ports.clone(), + req.vendor_url.as_deref(), + ) + .await + .map_err(|err| { + JsonResponse::::build().internal_server_error(err) + })? + } else { + db::marketplace::update_metadata( + pg_pool.get_ref(), + &existing_template.id, + Some(&req.name), + req.short_description.as_deref(), + req.long_description.as_deref(), + req.category_code.as_deref(), + Some(tags.clone()), + Some(tech_stack.clone()), + Some(infrastructure_requirements.clone()), + Some(price), + Some(billing_cycle.as_str()), + req.required_plan_name.as_deref(), + Some(currency.as_str()), + req.public_ports.clone(), + req.vendor_url.as_deref(), + ) + .await + .map_err(|err| { + JsonResponse::::build().internal_server_error(err) + })? + }; if !updated { return Err(JsonResponse::::build() From b1df4cfbbcdeea1165953ba7ee8f056c82b422d0 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Fri, 18 Sep 2026 12:04:00 +0300 Subject: [PATCH 04/20] PIPE audit log, not implemented, on hold, should be discussed --- src/forms/project/app.rs | 5 +++++ src/routes/agent/audit.rs | 32 ++++++++++++++++++-------------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/forms/project/app.rs b/src/forms/project/app.rs index 6387666f..b176fee9 100644 --- a/src/forms/project/app.rs +++ b/src/forms/project/app.rs @@ -72,6 +72,11 @@ pub struct App { pub network: forms::project::ServiceNetworks, #[validate] pub shared_ports: Option>, + /// Author-declared per-field policy (`config_contract` from stacker.yml). + /// Persisted to `project_app.config_contract` during sync so the Stack + /// Builder UI can render form fields with proper generation/validation rules. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub config_contract: Option, } impl App { diff --git a/src/routes/agent/audit.rs b/src/routes/agent/audit.rs index 07dc5284..91ef21ef 100644 --- a/src/routes/agent/audit.rs +++ b/src/routes/agent/audit.rs @@ -1,10 +1,10 @@ use crate::db::agent_audit_log as audit_db; -use crate::helpers::JsonResponse; +use crate::{helpers, models}; use crate::models::agent_audit_log::{AgentAuditLog, AuditBatchRequest}; -use actix_web::error::ErrorUnauthorized; -use actix_web::{get, post, web, HttpRequest, HttpResponse, Result}; +use actix_web::{get, post, web, HttpResponse, Result}; use serde::{Deserialize, Serialize}; use sqlx::PgPool; +use std::sync::Arc; // ── POST /api/v1/agent/audit ─────────────────────────────────────────────── @@ -13,20 +13,24 @@ pub struct IngestResponse { pub accepted: usize, } -/// Receive a batch of audit events from the Status Panel. +/// Receive a batch of audit events from the Status Panel agent. /// -/// Auth: `X-Internal-Key` header must match the `INTERNAL_SERVICES_ACCESS_KEY` -/// environment variable. +/// Auth: agent token via `X-Agent-Id` + `Bearer` headers (handled by +/// middleware). The `installation_hash` in the request body must match the +/// authenticated agent's `deployment_hash`. #[tracing::instrument(name = "Agent audit ingest", skip_all)] #[post("/audit")] pub async fn agent_audit_ingest_handler( - req: HttpRequest, + agent: web::ReqData>, body: web::Json, pool: web::Data, ) -> Result { - // Shared with the other service-to-service endpoints; the comparison is - // now constant-time. - crate::helpers::internal_key::require_internal_key(&req)?; + // Verify the agent owns this installation + if agent.deployment_hash != body.installation_hash { + return Err(helpers::JsonResponse::forbidden( + "Not authorized for this installation", + )); + } // Short-circuit on empty batch if body.events.is_empty() { @@ -36,7 +40,7 @@ pub async fn agent_audit_ingest_handler( let accepted = audit_db::insert_batch(&pool, &body.installation_hash, &body.events) .await .map_err(|err| { - JsonResponse::<()>::build() + helpers::JsonResponse::<()>::build() .internal_server_error(format!("Failed to store audit events: {}", err)) })?; @@ -72,7 +76,7 @@ pub async fn agent_audit_query_handler( // installation and prove they own it. let user = caller_user .as_deref() - .ok_or_else(|| JsonResponse::::forbidden("Authentication required"))?; + .ok_or_else(|| helpers::JsonResponse::::forbidden("Authentication required"))?; let is_admin = matches!(user.role.as_str(), "admin_service" | "group_admin" | "root"); @@ -80,7 +84,7 @@ pub async fn agent_audit_query_handler( let installation_hash = params .installation_hash .as_deref() - .ok_or_else(|| JsonResponse::::bad_request("installation_hash is required"))?; + .ok_or_else(|| helpers::JsonResponse::::bad_request("installation_hash is required"))?; crate::routes::agent::guard::authorize_deployment_access( &pool, @@ -100,7 +104,7 @@ pub async fn agent_audit_query_handler( ) .await .map_err(|err| { - JsonResponse::<()>::build() + helpers::JsonResponse::<()>::build() .internal_server_error(format!("Failed to fetch audit log: {}", err)) })?; From ae8fc6473b934e75bd55f6b4dcf1357bd2dbcf5f Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Fri, 18 Sep 2026 13:47:06 +0300 Subject: [PATCH 05/20] fix: propagate config_contract during sync and fix source_project_id lookup for resubmit - build_project_app now copies config_contract from the form app - get_source_project_id checks all versions (not just latest) since resubmit_with_new_version creates a new version row before set_source_project_id is called --- src/db/marketplace.rs | 8 +++++++- src/project_app/sync.rs | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/db/marketplace.rs b/src/db/marketplace.rs index 2e05627e..adf0a6cb 100644 --- a/src/db/marketplace.rs +++ b/src/db/marketplace.rs @@ -686,6 +686,10 @@ pub async fn set_source_project_id( } /// Read back the source project linked via [`set_source_project_id`]. +/// Checks the latest version first, then falls back to any version — +/// `source_project_id` is semantically tied to the template, not a +/// specific version, and `resubmit_with_new_version` may create a new +/// latest row before the caller has a chance to re-set it. pub async fn get_source_project_id( pool: &PgPool, template_id: uuid::Uuid, @@ -696,7 +700,9 @@ pub async fn get_source_project_id( sqlx::query_scalar::<_, Option>( r#"SELECT source_project_id FROM stack_template_version - WHERE template_id = $1 AND is_latest = true + WHERE template_id = $1 + AND source_project_id IS NOT NULL + ORDER BY is_latest DESC, created_at DESC LIMIT 1"#, ) .bind(template_id) diff --git a/src/project_app/sync.rs b/src/project_app/sync.rs index 9fcf401c..4627f718 100644 --- a/src/project_app/sync.rs +++ b/src/project_app/sync.rs @@ -86,6 +86,7 @@ fn build_project_app( project_app.networks = app_networks_json(app, all_networks); project_app.enabled = Some(true); project_app.deploy_order = Some(deploy_order); + project_app.config_contract = app.config_contract.clone(); project_app } From 0c0e37f403a0b795733382de4360f6b97bd9a4f4 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Fri, 18 Sep 2026 14:09:18 +0300 Subject: [PATCH 06/20] test: add coverage for config_contract sync and approved template resubmit - unit test: project_level_apps_from_form propagates config_contract - integration test: sync persists config_contract on project apps - integration test: create_handler updates approved template metadata - integration test: resubmit with new version preserves source_project_id --- src/project_app/sync.rs | 72 ++++++++++++ tests/marketplace_create_template.rs | 158 +++++++++++++++++++++++++++ tests/project_app_sync.rs | 80 ++++++++++++++ 3 files changed, 310 insertions(+) diff --git a/src/project_app/sync.rs b/src/project_app/sync.rs index 4627f718..d7c37670 100644 --- a/src/project_app/sync.rs +++ b/src/project_app/sync.rs @@ -304,4 +304,76 @@ mod tests { assert!(apps.is_empty()); } + + #[test] + fn project_level_apps_from_form_propagates_config_contract() { + let form: ProjectForm = serde_json::from_value(json!({ + "custom": { + "custom_stack_code": "contract-project", + "project_name": "Contract project", + "networks": [ + {"id": "net-1", "name": "default_network"} + ], + "web": [{ + "_id": "web-1", + "name": "Website", + "code": "website", + "type": "web", + "custom": true, + "dockerhub_image": "nginx:1.27", + "domain": "example.com", + "restart": "always", + "network": ["net-1"], + "environment": [{"key": "JWT_SECRET", "value": "auto"}], + "shared_ports": [{"host_port": "80", "container_port": "8080"}], + "volumes": [], + "config_contract": { + "services": { + "web": { + "fields": { + "JWT_SECRET": { "mutability": "generated" } + } + } + } + } + }], + "service": [{ + "_id": "svc-1", + "name": "Redis", + "code": "redis", + "type": "service", + "custom": true, + "dockerhub_image": "redis:7-alpine", + "domain": "", + "restart": "unless-stopped", + "network": ["net-1"], + "environment": [], + "shared_ports": [], + "volumes": [] + }], + "feature": [] + } + })) + .expect("project form should deserialize"); + + let apps = project_level_apps_from_form(42, &form); + + assert_eq!( + apps[0].config_contract, + Some(json!({ + "services": { + "web": { + "fields": { + "JWT_SECRET": { "mutability": "generated" } + } + } + } + })), + "config_contract should be propagated from form app to project app" + ); + assert_eq!( + apps[1].config_contract, None, + "apps without config_contract should remain None" + ); + } } diff --git a/tests/marketplace_create_template.rs b/tests/marketplace_create_template.rs index 164db9e3..e4b73b27 100644 --- a/tests/marketplace_create_template.rs +++ b/tests/marketplace_create_template.rs @@ -1665,3 +1665,161 @@ async fn submit_template_with_generated_policy_for_secret_field_is_accepted() { .expect("Failed to submit template for review"); assert_eq!(StatusCode::OK, submit_response.status()); } + +#[tokio::test] +async fn create_handler_updates_approved_template_metadata() { + let app = match common::spawn_app().await { + Some(app) => app, + None => return, + }; + let client = Client::new(); + + let create_response = create_template_with_body( + &client, + &app.address, + "test-bearer-token", + json!({ + "name": "Approved Metadata Template", + "slug": "approved-metadata-template", + "version": "1.0.0", + "stack_definition": { "services": { "web": { "image": "nginx:1.27" } } } + }), + ) + .await; + assert_eq!(StatusCode::CREATED, create_response.status()); + let template_id = create_response + .json::() + .await + .expect("Create response should be valid JSON")["item"]["id"] + .as_str() + .expect("Template id should be a string") + .to_string(); + + sqlx::query( + r#"UPDATE stack_template SET status = 'approved', approved_at = NOW() WHERE id = $1"#, + ) + .bind(Uuid::parse_str(&template_id).expect("Template id should be a UUID")) + .execute(&app.db_pool) + .await + .expect("Failed to mark template approved"); + + let update_response = create_template_with_body( + &client, + &app.address, + "test-bearer-token", + json!({ + "name": "Approved Metadata Template v2", + "slug": "approved-metadata-template", + "version": "1.0.0", + "stack_definition": { "services": { "web": { "image": "nginx:1.28" } } } + }), + ) + .await; + assert_eq!( + StatusCode::CREATED, + update_response.status(), + "POST /api/templates should succeed for approved templates (uses update_metadata_for_resubmit)" + ); + let body: Value = update_response + .json() + .await + .expect("Update response should be valid JSON"); + assert_eq!( + "Approved Metadata Template v2", + body["item"]["name"].as_str().expect("name should be a string"), + "Template name should be updated" + ); +} + +#[tokio::test] +async fn resubmit_approved_template_with_new_version_preserves_source_project_id() { + let _env_lock = env_lock().lock().expect("env lock should be available"); + let app = match common::spawn_app().await { + Some(app) => app, + None => return, + }; + let client = Client::new(); + + let project_id = common::create_test_project(&app.db_pool, "test_user_id").await; + common::create_test_deployment( + &app.db_pool, + "test_user_id", + project_id, + &format!("dpl-{}", Uuid::new_v4()), + ) + .await; + + let create_response = create_template_with_body( + &client, + &app.address, + "test-bearer-token", + json!({ + "name": "Resubmit Version Template", + "slug": "resubmit-version-template", + "source_project_id": project_id, + "version": "1.0.0", + "stack_definition": { "services": { "web": { "image": "nginx:1.27" } } } + }), + ) + .await; + assert_eq!(StatusCode::CREATED, create_response.status()); + let template_id = create_response + .json::() + .await + .expect("Create response should be valid JSON")["item"]["id"] + .as_str() + .expect("Template id should be a string") + .to_string(); + + sqlx::query( + r#"UPDATE stack_template SET status = 'approved', approved_at = NOW() WHERE id = $1"#, + ) + .bind(Uuid::parse_str(&template_id).expect("Template id should be a UUID")) + .execute(&app.db_pool) + .await + .expect("Failed to mark template approved"); + + let resubmit_response = client + .post(format!( + "{}/api/templates/{}/resubmit", + app.address, template_id + )) + .bearer_auth("test-bearer-token") + .json(&json!({ + "version": "1.1.0", + "stack_definition": { "services": { "web": { "image": "nginx:1.28" } } }, + "confirm_no_secrets": true, + "source_project_id": project_id + })) + .send() + .await + .expect("Failed to resubmit template"); + assert_eq!( + StatusCode::OK, + resubmit_response.status(), + "Resubmit with new version should succeed even when source_project_id was on the old version" + ); + + let stored_source_id: Option = sqlx::query_scalar( + r#"SELECT source_project_id FROM stack_template_version + WHERE template_id = $1::uuid AND is_latest = true"#, + ) + .bind(Uuid::parse_str(&template_id).expect("Template id should be a UUID")) + .fetch_one(&app.db_pool) + .await + .expect("Failed to query latest version source_project_id"); + assert_eq!( + Some(project_id), + stored_source_id, + "source_project_id should be persisted on the new version after resubmit" + ); + + let template_status: String = sqlx::query_scalar( + r#"SELECT status FROM stack_template WHERE id = $1::uuid"#, + ) + .bind(Uuid::parse_str(&template_id).expect("Template id should be a UUID")) + .fetch_one(&app.db_pool) + .await + .expect("Failed to query template status"); + assert_eq!("submitted", template_status, "Template should be in submitted status after resubmit"); +} diff --git a/tests/project_app_sync.rs b/tests/project_app_sync.rs index 67876680..20c5474b 100644 --- a/tests/project_app_sync.rs +++ b/tests/project_app_sync.rs @@ -223,3 +223,83 @@ async fn sync_project_updates_apps_without_creating_a_deployment() { .expect("deployments should load"); assert_eq!(deployment_count, 0, "sync must not create a deployment"); } + +#[tokio::test] +async fn sync_project_persists_config_contract_on_apps() { + let Some(app) = common::spawn_app().await else { + return; + }; + + let client = reqwest::Client::new(); + let create_response = client + .post(format!("{}/project", app.address)) + .header("Authorization", format!("Bearer {}", common::USER_A_TOKEN)) + .json(&json!({ + "custom": { + "custom_stack_code": "sync-contract-project", + "project_name": "Sync contract project", + "networks": [{ + "id": "default-network", + "name": "default_network" + }], + "web": [{ + "_id": "web-1", + "name": "Website", + "code": "website", + "type": "web", + "custom": true, + "dockerhub_image": "nginx:1.27", + "domain": "example.com", + "restart": "always", + "network": ["default-network"], + "environment": [{"key": "JWT_SECRET", "value": "auto"}], + "shared_ports": [{"host_port": "80", "container_port": "8080"}], + "volumes": [], + "config_contract": { + "services": { + "web": { + "fields": { + "JWT_SECRET": { "mutability": "generated" } + } + } + } + } + }], + "service": [], + "feature": [] + } + })) + .send() + .await + .expect("project create request should succeed"); + assert_eq!(create_response.status(), StatusCode::OK); + + let project_id = create_response + .json::() + .await + .expect("create response should be json")["item"]["id"] + .as_i64() + .expect("project id should be present") as i32; + + let apps = db::project_app::fetch_by_project(&app.db_pool, project_id) + .await + .expect("project apps should load"); + let website = apps + .iter() + .find(|app| app.code == "website") + .expect("website app should exist"); + + assert_eq!( + website.config_contract, + Some(json!({ + "services": { + "web": { + "fields": { + "JWT_SECRET": { "mutability": "generated" } + } + } + } + })), + "config_contract should be persisted on the project app during sync" + ); +} From 0be7f0638337f9d45bda90f389e38e7e54a90a50 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Fri, 18 Sep 2026 14:36:38 +0300 Subject: [PATCH 07/20] fix: persist config_contract via set_config_contract during app sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The insert/update SQL does not include config_contract — it is persisted via a dedicated set_config_contract call. sync_project_level_apps_from_form now calls set_config_contract after each insert/update when the form app declares a config_contract. --- src/project_app/sync.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/project_app/sync.rs b/src/project_app/sync.rs index d7c37670..f148393f 100644 --- a/src/project_app/sync.rs +++ b/src/project_app/sync.rs @@ -161,13 +161,24 @@ pub(crate) async fn sync_project_level_apps_from_form( .map(|app| (app.code.clone(), app.id)) .collect::>(); - for mut desired_app in desired_apps { + for desired_app in desired_apps { if let Some(existing_id) = existing_by_code.remove(&desired_app.code) { - desired_app.id = existing_id; - desired_app.deployment_id = None; - db::project_app::update(pool, &desired_app).await?; + let mut app = desired_app; + app.id = existing_id; + app.deployment_id = None; + db::project_app::update(pool, &app).await?; + if let Some(contract) = app.config_contract.clone() { + db::project_app::set_config_contract(pool, project_id, &app.code, contract) + .await?; + } } else { + let code = desired_app.code.clone(); + let contract = desired_app.config_contract.clone(); db::project_app::insert(pool, &desired_app).await?; + if let Some(contract) = contract { + db::project_app::set_config_contract(pool, project_id, &code, contract) + .await?; + } } } From b04fbe08eeae382d54ebfe45cee4b19631ba373d Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Fri, 18 Sep 2026 15:23:37 +0300 Subject: [PATCH 08/20] test: authenticate audit BDD ingest as a registered agent --- src/routes/agent/audit.rs | 6 ++-- tests/features/agent_audit.feature | 8 ++--- tests/steps/agent.rs | 55 +++++++++++++++++++++++++++--- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/src/routes/agent/audit.rs b/src/routes/agent/audit.rs index 91ef21ef..65cd6206 100644 --- a/src/routes/agent/audit.rs +++ b/src/routes/agent/audit.rs @@ -15,9 +15,8 @@ pub struct IngestResponse { /// Receive a batch of audit events from the Status Panel agent. /// -/// Auth: agent token via `X-Agent-Id` + `Bearer` headers (handled by -/// middleware). The `installation_hash` in the request body must match the -/// authenticated agent's `deployment_hash`. +/// Auth: agent token via `X-Agent-Id` and `Bearer` headers (handled by +/// middleware). The installation hash must belong to the authenticated agent. #[tracing::instrument(name = "Agent audit ingest", skip_all)] #[post("/audit")] pub async fn agent_audit_ingest_handler( @@ -25,7 +24,6 @@ pub async fn agent_audit_ingest_handler( body: web::Json, pool: web::Data, ) -> Result { - // Verify the agent owns this installation if agent.deployment_hash != body.installation_hash { return Err(helpers::JsonResponse::forbidden( "Not authorized for this installation", diff --git a/tests/features/agent_audit.feature b/tests/features/agent_audit.feature index 985a8fdc..637c5570 100644 --- a/tests/features/agent_audit.feature +++ b/tests/features/agent_audit.feature @@ -2,15 +2,15 @@ Feature: Agent Audit Log As a service operator I can ingest and query audit events via the /api/v1/agent/audit endpoints. - Scenario: Ingest audit events with valid internal key + Scenario: Ingest audit events with a registered agent Given I am authenticated as User A When I ingest audit events for installation "bdd-audit-hash" Then the response status should be 200 - Scenario: Ingest audit events with invalid internal key + Scenario: Ingest audit events with an invalid agent token Given I am authenticated as User A - When I ingest audit events with invalid internal key - Then the response status should be 401 + When I ingest audit events with invalid agent token + Then the response status should be 400 # Querying is scoped to deployments the caller owns, so the installation # must belong to User A. Without that link the query is a cross-tenant read: diff --git a/tests/steps/agent.rs b/tests/steps/agent.rs index 306bf771..fc30bc9f 100644 --- a/tests/steps/agent.rs +++ b/tests/steps/agent.rs @@ -328,12 +328,59 @@ async fn enqueue_command(world: &mut StepWorld, deployment_hash: String, cmd_typ // ─── Audit steps ───────────────────────────────────────────────── +async fn register_audit_agent(world: &StepWorld, installation_hash: &str) -> (String, String) { + let response = world + .client + .post(format!("{}/api/v1/agent/register", world.base_url)) + .header("X-Internal-Key", crate::steps::common::BDD_INTERNAL_KEY) + .json(&json!({ + "deployment_hash": installation_hash, + "agent_version": "1.0.0-bdd", + "capabilities": ["audit"], + "system_info": { "os": "linux", "arch": "x86_64" } + })) + .send() + .await + .expect("Audit agent registration request failed"); + + let status = response.status(); + let body: serde_json::Value = response + .json() + .await + .expect("Audit agent registration response should be JSON"); + assert!( + status.is_success(), + "Audit agent registration failed: {status} {body}" + ); + + let item = body + .pointer("/data/item") + .or_else(|| body.pointer("/item")) + .expect("Audit agent registration response should contain an item"); + let agent_id = item["agent_id"] + .as_str() + .expect("Audit agent registration should return agent_id") + .to_string(); + let agent_token = item["agent_token"] + .as_str() + .expect("Audit agent registration should return agent_token") + .to_string(); + + (agent_id, agent_token) +} + async fn do_ingest( world: &mut StepWorld, installation_hash: &str, - key: &str, + token: &str, events: serde_json::Value, ) { + let (agent_id, agent_token) = register_audit_agent(world, installation_hash).await; + let auth_token = if token == "wrong-key" { + token.to_string() + } else { + agent_token + }; let url = format!("{}/api/v1/agent/audit", world.base_url); let body = json!({ "installation_hash": installation_hash, @@ -342,8 +389,8 @@ async fn do_ingest( let resp = world .client .post(&url) - .header("Authorization", format!("Bearer {}", world.auth_token)) - .header("x-internal-key", key) + .header("X-Agent-Id", agent_id) + .header("Authorization", format!("Bearer {}", auth_token)) .json(&body) .send() .await @@ -364,7 +411,7 @@ async fn ingest_audit(world: &mut StepWorld, installation_hash: String) { do_ingest(world, &installation_hash, "bdd-internal-key", events).await; } -#[when("I ingest audit events with invalid internal key")] +#[when("I ingest audit events with invalid agent token")] async fn ingest_audit_invalid_key(world: &mut StepWorld) { let events = json!([ {"id": 1, "event_type": "test", "payload": {}, "created_at": 1711000000} From c85091d29ad4fb112237e83bdb90ff169ee3e78b Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Fri, 18 Sep 2026 16:17:49 +0300 Subject: [PATCH 09/20] feat(agents): add sweep_dead and sweep_malformed for periodic cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove dead agent rows whose deployment is deleted/missing and that show no sign of life within 30 days (last_heartbeat AND audit_log). Remove rows with structurally invalid deployment_hash unconditionally — these can never authenticate and often leak a raw token in plaintext. The audit_log check protects agents that are alive but failing authentication: last_heartbeat only advances on successful wait/report, while audit_log captures auth_failure entries. Migration 20260113000002 already converted audit_log.created_at to timestamptz — no new migration needed. Includes 9 integration tests covering the key cases from the sweep plan. --- src/db/agent.rs | 69 +++++++++ src/services/agent_sweeper.rs | 76 ++++++++++ src/services/mod.rs | 1 + src/startup.rs | 5 + tests/agent_sweep.rs | 265 ++++++++++++++++++++++++++++++++++ 5 files changed, 416 insertions(+) create mode 100644 src/services/agent_sweeper.rs create mode 100644 tests/agent_sweep.rs diff --git a/src/db/agent.rs b/src/db/agent.rs index 16cb9f2f..260a6dc1 100644 --- a/src/db/agent.rs +++ b/src/db/agent.rs @@ -239,6 +239,75 @@ pub async fn delete(pool: &PgPool, agent_id: Uuid) -> Result<(), String> { }) } +/// Delete agents whose deployment is gone (or soft-deleted) and that show no +/// sign of life within the retention window. +/// +/// "No sign of life" means both: +/// - `last_heartbeat` is NULL or older than the window, AND +/// - no `audit_log` row references the agent (by id or deployment_hash) +/// within the same window. +/// +/// The second condition protects agents that are alive but failing +/// authentication — `last_heartbeat` only advances on successful `wait`/`report`, +/// while `audit_log` captures `auth_failure` entries. +#[tracing::instrument(name = "Sweep dead agents", skip(pool))] +pub async fn sweep_dead(pool: &PgPool, retention_days: i32) -> Result { + let result = sqlx::query( + r#" + DELETE FROM agents a + WHERE NOT EXISTS ( + SELECT 1 FROM deployment d + WHERE d.deployment_hash = a.deployment_hash + AND d.deleted IS NOT TRUE + ) + AND (a.last_heartbeat IS NULL + OR a.last_heartbeat < NOW() - make_interval(days => $1)) + AND NOT EXISTS ( + SELECT 1 FROM audit_log l + WHERE (l.agent_id = a.id OR l.deployment_hash = a.deployment_hash) + AND l.created_at > NOW() - make_interval(days => $1) + ) + "#, + ) + .bind(retention_days) + .execute(pool) + .await + .map_err(|err| { + tracing::error!("Failed to sweep dead agents: {:?}", err); + format!("Database error: {}", err) + })?; + + Ok(result.rows_affected()) +} + +/// Delete agents whose `deployment_hash` is structurally invalid. +/// +/// A valid hash matches `deployment_` (36-char UUID with hyphens). +/// Rows with an invalid hash can never be matched by their own agent — the +/// lookup in `fetch_by_deployment_hash` will never find them. Among the 18 +/// currently broken rows, 17 store a raw agent token (86-char base64url) +/// instead of a hash, leaking the secret in plaintext. +/// +/// Returns its own count separate from `sweep_dead` so the caller can log +/// exactly how many malformed rows were removed. +#[tracing::instrument(name = "Sweep malformed agent rows", skip(pool))] +pub async fn sweep_malformed(pool: &PgPool) -> Result { + let result = sqlx::query( + r#" + DELETE FROM agents + WHERE deployment_hash !~ '^deployment_[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$' + "#, + ) + .execute(pool) + .await + .map_err(|err| { + tracing::error!("Failed to sweep malformed agents: {:?}", err); + format!("Database error: {}", err) + })?; + + Ok(result.rows_affected()) +} + pub async fn log_audit( pool: &PgPool, audit_log: models::AuditLog, diff --git a/src/services/agent_sweeper.rs b/src/services/agent_sweeper.rs new file mode 100644 index 00000000..daa1c53e --- /dev/null +++ b/src/services/agent_sweeper.rs @@ -0,0 +1,76 @@ +//! Housekeeping for dead and malformed agent rows. +//! +//! The `agents` table accumulates rows that no longer serve a purpose: agents +//! whose deployment was deleted or soft-deleted, agents that never sent a +//! heartbeat, and rows whose `deployment_hash` is structurally invalid (e.g. a +//! raw agent token stored instead of a hash). This sweeper removes them on a +//! daily cadence. +//! +//! A row is considered "dead" only when **both** of the following hold: +//! +//! 1. The deployment is gone — either no matching row in `deployment`, or the +//! row exists with `deleted IS TRUE`. +//! 2. There is no sign of life within the retention window — neither a +//! `last_heartbeat` nor an `audit_log` entry (the latter protects agents +//! that are alive but failing authentication, since `auth_failure` is +//! recorded in `audit_log` while `last_heartbeat` only advances on +//! successful `wait`/`report`). +//! +//! Malformed rows (invalid `deployment_hash`) are deleted unconditionally, +//! without a retention window — such an agent can never authenticate by its own +//! hash. +//! +//! Agents whose deployment is alive but silent are **not** touched: the row is +//! the agent's identity, and removing it would require a full reinstall. + +use std::time::Duration; + +use sqlx::PgPool; + +use crate::db; + +/// How often to sweep. Rows appear rarely; daily is ample. +const TICK: Duration = Duration::from_secs(86_400); + +/// How long a dead agent row is kept before removal. Long enough that a +/// temporarily stopped server can come back without losing its identity. +const RETENTION_DAYS: i32 = 30; + +pub fn spawn(pg_pool: PgPool) { + tokio::spawn(async move { + tracing::info!( + "agent_sweeper started (tick={:?}, retention={} days)", + TICK, + RETENTION_DAYS + ); + loop { + // Sleep first: startup is busy enough, and nothing here is urgent. + tokio::time::sleep(TICK).await; + + // Malformed rows are independent of retention — log at warn because + // new appearances indicate a write path that still needs fixing. + match db::agent::sweep_malformed(&pg_pool).await { + Ok(0) => {} + Ok(count) => tracing::warn!( + "agent_sweeper: removed {} malformed agent row(s) — \ + the write path producing these has not been found yet", + count + ), + Err(err) => { + tracing::warn!("agent_sweeper: malformed sweep error: {}", err) + } + } + + match db::agent::sweep_dead(&pg_pool, RETENTION_DAYS).await { + Ok(0) => tracing::debug!("agent_sweeper: nothing to remove"), + Ok(count) => tracing::info!( + "agent_sweeper: removed {} dead agent row(s)", + count + ), + Err(err) => { + tracing::warn!("agent_sweeper: dead sweep error: {}", err) + } + } + } + }); +} diff --git a/src/services/mod.rs b/src/services/mod.rs index 7ecfb2d3..87912848 100644 --- a/src/services/mod.rs +++ b/src/services/mod.rs @@ -1,4 +1,5 @@ pub mod agent_dispatcher; +pub mod agent_sweeper; pub mod agent_token; pub mod config_renderer; pub mod dag_executor; diff --git a/src/startup.rs b/src/startup.rs index 9c44f0a1..ea66fc06 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -122,6 +122,11 @@ pub async fn run( // it touches nothing the dashboard shows, and skipping it costs a slowly // growing table rather than correctness. crate::services::deployment_container_sweeper::spawn(api_pool.get_ref().clone()); + // Removes dead agent rows (no live deployment, no recent activity) and + // rows with structurally invalid deployment_hash. Runs daily; skipping it + // means the agents table slowly accumulates noise that obscures real + // problems. + crate::services::agent_sweeper::spawn(api_pool.get_ref().clone()); let payout_provider = crate::services::init_payout_provider(&settings.payouts) .map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidInput, err.to_string()))?; diff --git a/tests/agent_sweep.rs b/tests/agent_sweep.rs new file mode 100644 index 00000000..0f8388ad --- /dev/null +++ b/tests/agent_sweep.rs @@ -0,0 +1,265 @@ +mod common; + +use chrono::Utc; +use sqlx::Row; +use stacker::db; +use stacker::models::Agent; +use tokio::sync::OnceCell; + +static APP: OnceCell = OnceCell::const_new(); + +async fn app() -> common::TestAppWithVaultFresh { + common::get_or_init_vault_app_fresh(&APP) + .await + .expect("Failed to start test app") +} + +async fn create_project(pool: &sqlx::PgPool, user_id: &str) -> i32 { + sqlx::query_scalar::<_, i32>( + "INSERT INTO project (stack_id, user_id, name, metadata, created_at, updated_at) + VALUES (gen_random_uuid(), $1, $2, '{}'::jsonb, NOW(), NOW()) + RETURNING id", + ) + .bind(user_id) + .bind(format!("sweep-test-{}", uuid::Uuid::new_v4())) + .fetch_one(pool) + .await + .expect("Failed to create project") +} + +async fn create_deployment( + pool: &sqlx::PgPool, + project_id: i32, + user_id: &str, + deployment_hash: &str, + deleted: bool, +) { + sqlx::query( + "INSERT INTO deployment (project_id, deployment_hash, user_id, metadata, status, deleted, created_at, updated_at) + VALUES ($1, $2, $3, '{}'::jsonb, 'running', $4, NOW(), NOW())", + ) + .bind(project_id) + .bind(deployment_hash) + .bind(user_id) + .bind(deleted) + .execute(pool) + .await + .expect("Failed to create deployment"); +} + +async fn insert_agent(pool: &sqlx::PgPool, agent: &Agent) { + sqlx::query( + "INSERT INTO agents (id, deployment_hash, capabilities, version, system_info, + last_heartbeat, status, token_hash, token_hash_updated_at, + created_at, updated_at) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)", + ) + .bind(agent.id) + .bind(&agent.deployment_hash) + .bind(&agent.capabilities) + .bind(&agent.version) + .bind(&agent.system_info) + .bind(agent.last_heartbeat) + .bind(&agent.status) + .bind(&agent.token_hash) + .bind(agent.token_hash_updated_at) + .bind(agent.created_at) + .bind(agent.updated_at) + .execute(pool) + .await + .expect("Failed to insert agent"); +} + +async fn agent_exists(pool: &sqlx::PgPool, agent_id: uuid::Uuid) -> bool { + sqlx::query_scalar::<_, bool>("SELECT EXISTS(SELECT 1 FROM agents WHERE id = $1)") + .bind(agent_id) + .fetch_one(pool) + .await + .expect("Failed to check agent existence") +} + +async fn count_agents(pool: &sqlx::PgPool) -> i64 { + sqlx::query_scalar::<_, i64>("SELECT COUNT(*) FROM agents") + .fetch_one(pool) + .await + .expect("Failed to count agents") +} + +/// Case 1: deployment deleted, no activity → row deleted +#[tokio::test] +async fn sweep_dead_deletes_agent_with_deleted_deployment() { + let app = app().await; + let project_id = create_project(&app.db_pool, "test_user_id").await; + let hash = format!("deployment_{}", uuid::Uuid::new_v4()); + create_deployment(&app.db_pool, project_id, "test_user_id", &hash, true).await; + + let mut agent = Agent::new(hash); + agent.last_heartbeat = Some(Utc::now() - chrono::Duration::days(60)); + insert_agent(&app.db_pool, &agent).await; + + let removed = db::agent::sweep_dead(&app.db_pool, 30).await.unwrap(); + assert_eq!(removed, 1); + assert!(!agent_exists(&app.db_pool, agent.id).await); +} + +/// Case 2: deployment deleted, but fresh audit_log entry → row preserved +#[tokio::test] +async fn sweep_dead_preserves_agent_with_recent_audit_log() { + let app = app().await; + let project_id = create_project(&app.db_pool, "test_user_id").await; + let hash = format!("deployment_{}", uuid::Uuid::new_v4()); + create_deployment(&app.db_pool, project_id, "test_user_id", &hash, true).await; + + let mut agent = Agent::new(hash.clone()); + agent.last_heartbeat = Some(Utc::now() - chrono::Duration::days(60)); + insert_agent(&app.db_pool, &agent).await; + + // Insert a recent audit_log entry (auth_failure — agent is alive but failing auth) + sqlx::query( + "INSERT INTO audit_log (id, agent_id, deployment_hash, action, status, created_at) + VALUES ($1, $2, $3, 'auth_failure', 'failure', NOW())", + ) + .bind(uuid::Uuid::new_v4()) + .bind(agent.id) + .bind(&hash) + .execute(&app.db_pool) + .await + .expect("Failed to insert audit log"); + + let removed = db::agent::sweep_dead(&app.db_pool, 30).await.unwrap(); + assert_eq!(removed, 0); + assert!(agent_exists(&app.db_pool, agent.id).await); +} + +/// Case 3: deployment missing entirely, no activity → row deleted +#[tokio::test] +async fn sweep_dead_deletes_agent_with_missing_deployment() { + let app = app().await; + let hash = format!("deployment_{}", uuid::Uuid::new_v4()); + // No deployment row at all + + let mut agent = Agent::new(hash); + agent.last_heartbeat = Some(Utc::now() - chrono::Duration::days(60)); + insert_agent(&app.db_pool, &agent).await; + + let removed = db::agent::sweep_dead(&app.db_pool, 30).await.unwrap(); + assert_eq!(removed, 1); + assert!(!agent_exists(&app.db_pool, agent.id).await); +} + +/// Case 4: deployment alive, agent silent for a year → row preserved +#[tokio::test] +async fn sweep_dead_preserves_agent_with_live_deployment() { + let app = app().await; + let project_id = create_project(&app.db_pool, "test_user_id").await; + let hash = format!("deployment_{}", uuid::Uuid::new_v4()); + create_deployment(&app.db_pool, project_id, "test_user_id", &hash, false).await; + + let mut agent = Agent::new(hash); + agent.last_heartbeat = Some(Utc::now() - chrono::Duration::days(365)); + insert_agent(&app.db_pool, &agent).await; + + let removed = db::agent::sweep_dead(&app.db_pool, 30).await.unwrap(); + assert_eq!(removed, 0); + assert!(agent_exists(&app.db_pool, agent.id).await); +} + +/// Case 5: deployment_hash of length 86 (raw token) → row deleted +#[tokio::test] +async fn sweep_malformed_deletes_agent_with_86_char_hash() { + let app = app().await; + // 86-char base64url string — the pattern seen in production + let bad_hash = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwx\ + yz0123456789-_ABCDEFGHIJKLMNOPQRSTUVWXYZabcdef\ + ghij"; + + let agent = Agent::new(bad_hash.to_string()); + insert_agent(&app.db_pool, &agent).await; + + let removed = db::agent::sweep_malformed(&app.db_pool).await.unwrap(); + assert_eq!(removed, 1); + assert!(!agent_exists(&app.db_pool, agent.id).await); +} + +/// Case 6: empty deployment_hash → row deleted +#[tokio::test] +async fn sweep_malformed_deletes_agent_with_empty_hash() { + let app = app().await; + + let agent = Agent::new(String::new()); + insert_agent(&app.db_pool, &agent).await; + + let removed = db::agent::sweep_malformed(&app.db_pool).await.unwrap(); + assert_eq!(removed, 1); + assert!(!agent_exists(&app.db_pool, agent.id).await); +} + +/// Valid hashes are not affected by sweep_malformed +#[tokio::test] +async fn sweep_malformed_preserves_valid_hash() { + let app = app().await; + let project_id = create_project(&app.db_pool, "test_user_id").await; + let hash = format!("deployment_{}", uuid::Uuid::new_v4()); + create_deployment(&app.db_pool, project_id, "test_user_id", &hash, false).await; + + let agent = Agent::new(hash); + insert_agent(&app.db_pool, &agent).await; + + let removed = db::agent::sweep_malformed(&app.db_pool).await.unwrap(); + assert_eq!(removed, 0); + assert!(agent_exists(&app.db_pool, agent.id).await); +} + +/// Case 7: after agent deletion, audit_log records survive and retain deployment_hash +#[tokio::test] +async fn audit_log_preserved_after_agent_deletion() { + let app = app().await; + let project_id = create_project(&app.db_pool, "test_user_id").await; + let hash = format!("deployment_{}", uuid::Uuid::new_v4()); + create_deployment(&app.db_pool, project_id, "test_user_id", &hash, true).await; + + let mut agent = Agent::new(hash.clone()); + agent.last_heartbeat = Some(Utc::now() - chrono::Duration::days(60)); + insert_agent(&app.db_pool, &agent).await; + + let audit_id = uuid::Uuid::new_v4(); + sqlx::query( + "INSERT INTO audit_log (id, agent_id, deployment_hash, action, status, created_at) + VALUES ($1, $2, $3, 'register', 'success', NOW() - interval '60 days')", + ) + .bind(audit_id) + .bind(agent.id) + .bind(&hash) + .execute(&app.db_pool) + .await + .expect("Failed to insert audit log"); + + // sweep_dead should NOT delete this agent (audit_log blocks it via the + // retention window). But we want to test what happens when the agent IS + // deleted — so delete it directly. + db::agent::delete(&app.db_pool, agent.id).await.unwrap(); + + // Audit log should survive, with agent_id NULLed and deployment_hash preserved + let row = sqlx::query( + "SELECT agent_id, deployment_hash FROM audit_log WHERE id = $1", + ) + .bind(audit_id) + .fetch_one(&app.db_pool) + .await + .expect("Audit log row should still exist"); + + let agent_id: Option = row.get("agent_id"); + let dep_hash: Option = row.get("deployment_hash"); + assert!(agent_id.is_none(), "agent_id should be NULL after ON DELETE SET NULL"); + assert_eq!(dep_hash.as_deref(), Some(hash.as_str())); +} + +/// sweep_dead with no matching rows returns 0 +#[tokio::test] +async fn sweep_dead_returns_zero_when_nothing_to_remove() { + let app = app().await; + let before = count_agents(&app.db_pool).await; + let removed = db::agent::sweep_dead(&app.db_pool, 30).await.unwrap(); + assert_eq!(removed, 0); + assert_eq!(count_agents(&app.db_pool).await, before); +} From 0515e96834260c24cc9ec1beaa2e8693efda677c Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Fri, 18 Sep 2026 16:36:02 +0300 Subject: [PATCH 10/20] fix: include config_contract in sync payload --- src/cli/stacker_client.rs | 49 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/cli/stacker_client.rs b/src/cli/stacker_client.rs index df25763b..ce05dd3b 100644 --- a/src/cli/stacker_client.rs +++ b/src/cli/stacker_client.rs @@ -4096,6 +4096,21 @@ pub fn build_project_body(config: &StackerConfig) -> serde_json::Value { service_apps.push(service_to_app_json(svc, &network_ids)); } + // `project_app.config_contract` is persisted by a separate server-side + // accessor. Include the full contract on each generated app so `stacker + // sync` does not silently discard the policy from stacker.yml. + if let Ok(config_contract) = serde_json::to_value(&config.config_contract) { + let has_services = config_contract + .get("services") + .and_then(serde_json::Value::as_object) + .is_some_and(|services| !services.is_empty()); + if has_services { + for app in web_apps.iter_mut().chain(service_apps.iter_mut()) { + app["config_contract"] = config_contract.clone(); + } + } + } + serde_json::json!({ "custom": { "custom_stack_code": stack_code, @@ -5129,6 +5144,40 @@ mod tests { ); } + #[test] + fn build_project_body_includes_config_contract_on_apps() { + let mut config = crate::cli::config_parser::ConfigBuilder::new() + .name("contract-project") + .app_image("nginx:1.27") + .build() + .expect("config should build"); + config.config_contract = serde_json::from_value(serde_json::json!({ + "services": { + "app": { + "fields": { + "JWT_SECRET": { + "mutability": "generated", + "type": "hex", + "length": 32 + } + } + } + } + })) + .expect("config contract should deserialize"); + + let body = build_project_body(&config); + let contract = &body["custom"]["web"][0]["config_contract"]; + assert_eq!( + contract["services"]["app"]["fields"]["JWT_SECRET"]["mutability"], + "generated" + ); + assert_eq!( + contract["services"]["app"]["fields"]["JWT_SECRET"]["type"], + "hex" + ); + } + #[test] fn pipe_instance_request_serializes_adapter_references() { let request = CreatePipeInstanceApiRequest { From 7b6c5e6a9fd82a8f091ee484d637ae7030adbc2e Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Sat, 19 Sep 2026 12:38:24 +0300 Subject: [PATCH 11/20] feat(compose): parameterize env vars to prevent secret leakage in baked snapshots - Add parameterize_compose_env_vars() to replace literal env values with ${VAR} references in generated compose files - Integrate into deploy pipeline so compose never contains author secrets - Docker Compose resolves ${VAR} from co-located .env at runtime - Add 3 unit tests for parameterization behavior This fixes the security issue where every buyer of a marketplace template received the author's literal secrets in the baked compose file. --- src/cli/generator/compose.rs | 138 +++++++++++++++++++++++++++++ src/console/commands/cli/deploy.rs | 16 +++- 2 files changed, 153 insertions(+), 1 deletion(-) diff --git a/src/cli/generator/compose.rs b/src/cli/generator/compose.rs index 745a7b33..41780286 100644 --- a/src/cli/generator/compose.rs +++ b/src/cli/generator/compose.rs @@ -773,6 +773,83 @@ impl fmt::Display for ComposeDefinition { } } +/// Replace literal environment values with `${VAR}` references for a set of +/// env var names. This is used during **bake** so the snapshot's compose file +/// never contains the author's secrets — Docker Compose resolves `${VAR}` from +/// the env file (`/etc/stacker/env`) at runtime. +/// +/// Only environment blocks inside service definitions are touched; other parts +/// of the compose file (labels, volumes, etc.) are left alone. +/// +/// `env_keys` is the set of env var names whose values should be +/// parameterized. Typically this is every key declared in the author's +/// `config_contract` with `mutability: generated` plus any `provided` fields. +pub fn parameterize_compose_env_vars( + compose_content: &str, + env_keys: &std::collections::HashSet, +) -> String { + if env_keys.is_empty() { + return compose_content.to_string(); + } + + let mut in_environment = false; + let mut env_indent = 0usize; + let mut result = String::with_capacity(compose_content.len()); + + for line in compose_content.lines() { + let trimmed = line.trim_start(); + let indent = line.len() - trimmed.len(); + + // Track whether we're inside an `environment:` block belonging to a + // service. The block ends when we hit a line at the same or shallower + // indent that isn't blank. + if trimmed == "environment:" { + in_environment = true; + env_indent = indent; + result.push_str(line); + result.push('\n'); + continue; + } + + if in_environment { + // A blank line or a line at the same / shallower indent ends the block. + if trimmed.is_empty() || indent <= env_indent { + in_environment = false; + } + } + + if in_environment { + // Match " KEY: value" — the key must be a valid env identifier. + if let Some((key, _rest)) = trimmed.split_once(':') { + let key = key.trim(); + if is_env_identifier(key) && env_keys.contains(key) { + // Preserve the original indent and replace the value. + let prefix = &line[..indent + key.len()]; + // Find where the value starts (after "KEY: "). + result.push_str(prefix); + result.push_str(": ${"); + result.push_str(key); + result.push_str("}\n"); + continue; + } + } + } + + result.push_str(line); + result.push('\n'); + } + + result +} + +/// Returns `true` when `s` looks like a POSIX env-variable name. +fn is_env_identifier(s: &str) -> bool { + !s.is_empty() + && s.chars().enumerate().all(|(i, c)| { + c.is_ascii_alphanumeric() || c == '_' || (i == 0 && c == '.') + }) +} + // ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ // Tests // ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ @@ -1861,4 +1938,65 @@ services: output ); } + + #[test] + fn parameterize_replaces_secret_values_with_env_refs() { + let compose = "\ +services: + app: + image: trydirect/stackpilot:latest + ports: + - \"8080:8000\" + environment: + ADMIN_PASSWORD: 4f4237dd9bfe8e1622706cac7bab63c7 + ADMIN_USER: admin + DATABASE_URL: postgresql://stackpilot:2213a996143863b99a0f2d3e22907690@db:5432/stackpilot + SECRET_KEY: b838f1f22379b8c268a4d3e0268459761c18947e1576956a6d9b1f3928070df4 + OLLAMA_MODEL: llama3.1 + restart: unless-stopped +"; + let mut keys = std::collections::HashSet::new(); + keys.insert("ADMIN_PASSWORD".to_string()); + keys.insert("SECRET_KEY".to_string()); + keys.insert("DATABASE_URL".to_string()); + + let result = parameterize_compose_env_vars(compose, &keys); + + assert!(result.contains("ADMIN_PASSWORD: ${ADMIN_PASSWORD}"), "admin pw:\n{result}"); + assert!(result.contains("SECRET_KEY: ${SECRET_KEY}"), "secret key:\n{result}"); + assert!(result.contains("DATABASE_URL: ${DATABASE_URL}"), "db url:\n{result}"); + // Non-secret values stay as-is. + assert!(result.contains("ADMIN_USER: admin"), "admin user:\n{result}"); + assert!(result.contains("OLLAMA_MODEL: llama3.1"), "model:\n{result}"); + } + + #[test] + fn parameterize_leaves_non_env_blocks_alone() { + let compose = "\ +services: + app: + image: myapp:latest + environment: + SECRET_KEY: abc123 + labels: + my.stacker.service: myapp + volumes: + - app_data:/app/data +"; + let mut keys = std::collections::HashSet::new(); + keys.insert("SECRET_KEY".to_string()); + + let result = parameterize_compose_env_vars(compose, &keys); + + assert!(result.contains("SECRET_KEY: ${SECRET_KEY}"), "secret:\n{result}"); + assert!(result.contains("my.stacker.service: myapp"), "label:\n{result}"); + assert!(result.contains("- app_data:/app/data"), "volume:\n{result}"); + } + + #[test] + fn parameterize_no_keys_returns_original() { + let compose = "services:\n app:\n environment:\n FOO: bar\n"; + let keys = std::collections::HashSet::new(); + assert_eq!(parameterize_compose_env_vars(compose, &keys), compose); + } } diff --git a/src/console/commands/cli/deploy.rs b/src/console/commands/cli/deploy.rs index 2c9ff383..eb0d7abb 100644 --- a/src/console/commands/cli/deploy.rs +++ b/src/console/commands/cli/deploy.rs @@ -3556,7 +3556,21 @@ fn run_deploy_with_credentials_manager( let compose = ComposeDefinition::try_from(&config)?; // `write_to` refuses to clobber an existing file unless told to, // so a staleness-driven regeneration must opt in explicitly. - compose.write_to(&compose_out, force_rebuild || compose_is_stale)?; + // Parameterize secret env vars: replace literal values with + // `${VAR}` references so the compose file never contains the + // author's secrets. Docker Compose resolves them from the + // co-located `.env` file at runtime. + let rendered = compose.render(); + let env_keys: std::collections::HashSet = + config.env.keys().cloned().collect(); + let parameterized = + crate::cli::generator::compose::parameterize_compose_env_vars( + &rendered, + &env_keys, + ); + if force_rebuild || compose_is_stale || !compose_out.exists() { + std::fs::write(&compose_out, ¶meterized)?; + } // The synthesized caddy/nginx proxy service mounts a config file // (./Caddyfile, ./nginx/conf.d) from the compose directory. For // local/server deploys the tfa proxy role does NOT run, so the From 1a10914dd68f4cb13f7577abfcdb6a6da8554ea3 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Sat, 19 Sep 2026 13:51:06 +0300 Subject: [PATCH 12/20] fix(compose): cover protected service environment keys --- src/console/commands/cli/deploy.rs | 44 ++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/src/console/commands/cli/deploy.rs b/src/console/commands/cli/deploy.rs index eb0d7abb..37481d7d 100644 --- a/src/console/commands/cli/deploy.rs +++ b/src/console/commands/cli/deploy.rs @@ -686,6 +686,32 @@ fn normalize_generated_compose_paths(compose_path: &Path) -> Result<(), CliError Ok(()) } +fn compose_env_keys(config: &StackerConfig) -> std::collections::HashSet { + let mut keys: std::collections::HashSet = config.env.keys().cloned().collect(); + + // Include policy-declared fields even when they are defined only in + // app.environment or services[].environment rather than top-level env. + if let Ok(contract) = serde_json::to_value(&config.config_contract) { + if let Some(services) = contract.get("services").and_then(|v| v.as_object()) { + for service in services.values() { + if let Some(fields) = service.get("fields").and_then(|v| v.as_object()) { + for (name, policy) in fields { + let protected = matches!( + policy.get("mutability").and_then(|v| v.as_str()), + Some("generated") | Some("provided") + ); + if protected { + keys.insert(name.clone()); + } + } + } + } + } + } + + keys +} + /// A compose service that declares a `build:` section. struct ComposeBuildService { name: String, @@ -3561,8 +3587,7 @@ fn run_deploy_with_credentials_manager( // author's secrets. Docker Compose resolves them from the // co-located `.env` file at runtime. let rendered = compose.render(); - let env_keys: std::collections::HashSet = - config.env.keys().cloned().collect(); + let env_keys = compose_env_keys(&config); let parameterized = crate::cli::generator::compose::parameterize_compose_env_vars( &rendered, @@ -3589,6 +3614,21 @@ fn run_deploy_with_credentials_manager( (compose_out, false) }; + // Parameterize an existing generated compose file as well. This prevents + // a previously rendered file with literal secrets from bypassing the + // protection merely because it was considered up to date. + if !compose_is_user_supplied { + let env_keys = compose_env_keys(&config); + if !env_keys.is_empty() { + let content = std::fs::read_to_string(&compose_path)?; + let parameterized = + crate::cli::generator::compose::parameterize_compose_env_vars(&content, &env_keys); + if parameterized != content { + std::fs::write(&compose_path, parameterized)?; + } + } + } + normalize_generated_compose_paths(&compose_path)?; validate_compose_for_deploy(&compose_path)?; reject_build_sections_for_cloud( From 27017fa0cfec2c3cef87e175d2954753ccbf03f4 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Sat, 19 Sep 2026 22:30:04 +0300 Subject: [PATCH 13/20] prepare server for baking, clean creds, keys, logs etc --- ...baked_snapshots_required_env_keys.down.sql | 1 + ...0_baked_snapshots_required_env_keys.up.sql | 10 + src/bin/bake.rs | 194 +++++--- src/cli/generator/compose.rs | 386 +++++++++++++++- src/db/baked_snapshot.rs | 6 +- src/helpers/bake.rs | 5 + src/helpers/bake_finalize.rs | 434 ++++++++++++++++++ src/helpers/mod.rs | 1 + src/models/baked_snapshot.rs | 9 + src/routes/oneclick_deploy/clone.rs | 97 +++- 10 files changed, 1071 insertions(+), 72 deletions(-) create mode 100644 migrations/20260919120000_baked_snapshots_required_env_keys.down.sql create mode 100644 migrations/20260919120000_baked_snapshots_required_env_keys.up.sql create mode 100644 src/helpers/bake_finalize.rs diff --git a/migrations/20260919120000_baked_snapshots_required_env_keys.down.sql b/migrations/20260919120000_baked_snapshots_required_env_keys.down.sql new file mode 100644 index 00000000..1f427e62 --- /dev/null +++ b/migrations/20260919120000_baked_snapshots_required_env_keys.down.sql @@ -0,0 +1 @@ +ALTER TABLE baked_snapshots DROP COLUMN required_env_keys; diff --git a/migrations/20260919120000_baked_snapshots_required_env_keys.up.sql b/migrations/20260919120000_baked_snapshots_required_env_keys.up.sql new file mode 100644 index 00000000..3a35e504 --- /dev/null +++ b/migrations/20260919120000_baked_snapshots_required_env_keys.up.sql @@ -0,0 +1,10 @@ +-- Record which ${VAR} references the baked compose actually needs, so the clone +-- path can refuse a deploy whose env file would not satisfy them. +-- +-- Without this, a missing key is silent: Docker Compose substitutes an empty +-- string with only a warning, the unit's ExecStartPre ends in `|| true`, and the +-- systemd unit still reports active while the stack is misconfigured. +-- +-- Nullable on purpose: snapshots baked before this column carry NULL and skip +-- the check, so existing images keep deploying unchanged. +ALTER TABLE baked_snapshots ADD COLUMN required_env_keys JSONB; diff --git a/src/bin/bake.rs b/src/bin/bake.rs index fb5ba0fb..547d5a96 100644 --- a/src/bin/bake.rs +++ b/src/bin/bake.rs @@ -7,7 +7,13 @@ //! Usage: //! HETZNER_TOKEN=... DATABASE_URL=... cargo run --bin bake -- \ //! --ip --stack ai-workflows-v2 --version 1.0.0 \ -//! --health-url http:///health +//! --health-url http:///health --ssh-key ~/.ssh/id_ed25519 +//! +//! `--ssh-key` is required: before snapshotting we sanitize the build box +//! (strip machine identity, blank the author's `.env`, parameterize secrets +//! embedded in compose values, drop initialized data volumes). Without it the +//! image would carry the author's credentials to every buyer, so the bake is +//! refused unless `--allow-unsanitized-snapshot` is passed deliberately. //! //! `DATABASE_URL` (the stacker Postgres) persists the BakeRecord; without it //! the bake still snapshots and prints the record, but it is not registered. @@ -23,6 +29,10 @@ async fn main() -> Result<(), Box> { let mut stack = "lamp".to_string(); let mut version = "v1".to_string(); let mut health_url: Option = None; + let mut ssh_key: Option = None; + let mut ssh_user = "root".to_string(); + let mut project_dir = "/home/trydirect/project".to_string(); + let mut allow_unsanitized = false; let mut i = 1; while i < args.len() { @@ -47,6 +57,22 @@ async fn main() -> Result<(), Box> { health_url = args.get(i + 1).cloned(); i += 2; } + "--ssh-key" => { + ssh_key = args.get(i + 1).cloned(); + i += 2; + } + "--ssh-user" => { + ssh_user = args.get(i + 1).cloned().unwrap_or(ssh_user); + i += 2; + } + "--project-dir" => { + project_dir = args.get(i + 1).cloned().unwrap_or(project_dir); + i += 2; + } + "--allow-unsanitized-snapshot" => { + allow_unsanitized = true; + i += 1; + } other => { eprintln!("ignoring unknown arg: {other}"); i += 1; @@ -77,7 +103,77 @@ async fn main() -> Result<(), Box> { let target = HetznerSnapshotTarget { provider_server_id: server_id, server_name: None, - public_ip: ip, + public_ip: ip.clone(), + }; + + // Resolve the author's field policy *before* finalizing: it decides which + // keys get blanked in `.env` and which names an embedded secret may be + // parameterized to. Also pinned to the snapshot further down so the clone + // path can regenerate those fields per buyer. + let pool = match std::env::var("DATABASE_URL") { + Ok(db_url) => Some(sqlx::PgPool::connect(&db_url).await?), + Err(_) => { + eprintln!("WARNING: DATABASE_URL not set — bake will NOT be registered in the snapshot registry."); + None + } + }; + + let config_contract = match &pool { + Some(pool) => resolve_config_contract(pool, &stack).await, + None => None, + }; + let protected_keys = config_contract + .as_ref() + .map(stacker::helpers::bake_finalize::protected_keys_from_contract) + .unwrap_or_default(); + + // Sanitize the build box before the snapshot is taken. + let finalize_outcome = match (&ssh_key, allow_unsanitized) { + (Some(key_path), _) => { + let Some(host) = ip.clone() else { + return Err("--ssh-key needs --ip (the build box address to connect to)".into()); + }; + let private_key_pem = std::fs::read_to_string(key_path) + .map_err(|e| format!("could not read --ssh-key {key_path}: {e}"))?; + + eprintln!("==> Finalizing build box before snapshot..."); + let ctx = stacker::helpers::bake_finalize::FinalizeContext { + host, + port: 22, + user: ssh_user.clone(), + private_key_pem, + project_dir: project_dir.clone(), + stack: stack.clone(), + protected_keys: protected_keys.clone(), + }; + let outcome = stacker::helpers::bake_finalize::finalize_build_box(&ctx).await?; + eprintln!( + " Sanitized. Compose requires {} env key(s): {}", + outcome.required_env_keys.len(), + outcome + .required_env_keys + .iter() + .cloned() + .collect::>() + .join(", ") + ); + Some(outcome) + } + (None, true) => { + eprintln!( + "WARNING: --allow-unsanitized-snapshot set. The image will keep the author's \ + .env values, data volumes and SSH host keys. Do NOT publish it to buyers." + ); + None + } + (None, false) => { + return Err( + "--ssh-key is required so the build box can be sanitized before \ + snapshotting (pass --allow-unsanitized-snapshot to skip, for a \ + private image only)" + .into(), + ) + } }; let connector = HetznerCloudClient::from_env().map_err(|e| e.to_string())?; @@ -94,56 +190,16 @@ async fn main() -> Result<(), Box> { ); // Persist into the snapshot registry so /api/v1/deploy/clone can resolve it. - if let Ok(db_url) = std::env::var("DATABASE_URL") { - let pool = sqlx::PgPool::connect(&db_url).await?; - - // Pin the author's field policy to this image so the clone path can - // regenerate `mutability: generated` fields fresh per buyer instead of - // shipping the single value baked into the snapshot. Resolved by the same - // slug the registry keys on (record.stack == stack_template.slug). - // Best-effort: an un-catalogued or unapproved stack bakes with no - // contract, and the clone path degrades to the baked values. - let config_contract = - match stacker::db::marketplace::get_approved_by_slug(&pool, &record.stack).await { - Ok(Some(template)) => { - eprintln!( - "DEBUG: resolved template '{}' id={} for stack '{}'", - template.name, template.id, record.stack - ); - match stacker::db::marketplace::get_config_contract(&pool, template.id).await { - Ok(serde_json::Value::Null) => { - eprintln!("DEBUG: config_contract is Null for template id={}", template.id); - None - } - Ok(contract) => { - eprintln!("DEBUG: config_contract resolved, keys={:?}", - contract.as_object().map(|o| o.keys().collect::>())); - Some(contract) - } - Err(err) => { - eprintln!( - "WARNING: could not read config_contract for '{}': {err}", - record.stack - ); - None - } - } - } - Ok(None) => { - eprintln!("DEBUG: no approved template found for stack '{}'", record.stack); - None - } - Err(err) => { - eprintln!( - "WARNING: could not resolve template for '{}': {err}", - record.stack - ); - None - } - }; - - eprintln!("DEBUG: config_contract to record: {:?}", - config_contract.as_ref().map(|c| c.as_object().map(|o| o.keys().collect::>()))); + if let Some(pool) = pool { + let required_env_keys = finalize_outcome.as_ref().map(|outcome| { + serde_json::Value::Array( + outcome + .required_env_keys + .iter() + .map(|key| serde_json::Value::String(key.clone())) + .collect(), + ) + }); let row = stacker::db::baked_snapshot::record( &pool, @@ -154,6 +210,7 @@ async fn main() -> Result<(), Box> { record.healthy, None, config_contract, + required_env_keys, ) .await .map_err(|e| e.to_string())?; @@ -161,9 +218,38 @@ async fn main() -> Result<(), Box> { "Registered snapshot in registry: id={} stack={}:{} image_id={}", row.id, row.stack, row.version, row.image_id ); - } else { - eprintln!("WARNING: DATABASE_URL not set — bake NOT registered in the snapshot registry."); } Ok(()) } + +/// The author's field policy for `stack`, resolved by the same slug the +/// snapshot registry keys on (`baked_snapshots.stack == stack_template.slug`). +/// +/// Best-effort: an un-catalogued or unapproved stack bakes with no contract and +/// the clone path degrades to the baked values. +async fn resolve_config_contract(pool: &sqlx::PgPool, stack: &str) -> Option { + match stacker::db::marketplace::get_approved_by_slug(pool, stack).await { + Ok(Some(template)) => { + match stacker::db::marketplace::get_config_contract(pool, template.id).await { + Ok(serde_json::Value::Null) => { + eprintln!("WARNING: template '{stack}' declares no config_contract — nothing will be regenerated per buyer."); + None + } + Ok(contract) => Some(contract), + Err(err) => { + eprintln!("WARNING: could not read config_contract for '{stack}': {err}"); + None + } + } + } + Ok(None) => { + eprintln!("WARNING: no approved template found for stack '{stack}'."); + None + } + Err(err) => { + eprintln!("WARNING: could not resolve template for '{stack}': {err}"); + None + } + } +} diff --git a/src/cli/generator/compose.rs b/src/cli/generator/compose.rs index 41780286..75eeb85f 100644 --- a/src/cli/generator/compose.rs +++ b/src/cli/generator/compose.rs @@ -842,12 +842,221 @@ pub fn parameterize_compose_env_vars( result } +/// Shortest value we will treat as a secret when searching *inside* other +/// values. Short strings ("admin", "postgres", a port) collide with ordinary +/// text and would corrupt the compose file. +const MIN_EMBEDDED_SECRET_LEN: usize = 12; + +/// Two protected fields resolved to the same literal value, so the compose +/// cannot be parameterized unambiguously. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct EmbeddedSecretConflict { + /// The contract field names that share one value, sorted. + pub keys: Vec, +} + +impl std::fmt::Display for EmbeddedSecretConflict { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "the same secret value is declared under {} protected fields ({}). \ + Each is regenerated independently per buyer, so they would receive \ + different values and the stack would fail to authenticate. Declare \ + one field and reference it from the others.", + self.keys.len(), + self.keys.join(", ") + ) + } +} + +/// Replace secret values that appear *inside* a larger value with a `${KEY}` +/// reference — the case [`parameterize_compose_env_vars`] structurally cannot +/// reach, because it matches whole values by key name. +/// +/// The motivating case is a DSN: `DATABASE_URL: +/// postgresql://user:@host/db` carries the database password inside +/// its value, under a key name (`DATABASE_URL`) that no secret-name heuristic +/// recognises. Every existing protection layer keys off the *variable name*, so +/// such a credential is invisible to all of them at once and stays literal in +/// the baked image. +/// +/// `env_values` are the build box's resolved `KEY=value` pairs; `protected` are +/// the contract fields with `mutability: generated`/`provided`, i.e. the ones +/// that will be regenerated on the buyer's box and therefore can be referenced +/// safely. +pub fn parameterize_embedded_secret_values( + compose_content: &str, + env_values: &std::collections::BTreeMap, + protected: &std::collections::BTreeSet, +) -> Result { + // Every place a literal value is known by a name: the build box's .env plus + // the compose's own `KEY: value` pairs (a value interpolated into a service + // env block has no .env entry of its own). + let mut names_by_value: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + + for (key, value) in env_values + .iter() + .chain(compose_literal_env_pairs(compose_content).iter()) + { + if value.len() < MIN_EMBEDDED_SECRET_LEN { + continue; + } + names_by_value + .entry(value.clone()) + .or_default() + .insert(key.clone()); + } + + // A value reachable under two *protected* names diverges at regeneration + // time — refuse rather than bake an image that cannot boot. + let mut substitutions: Vec<(String, String)> = Vec::new(); + for (value, names) in &names_by_value { + let protected_names: Vec = names + .iter() + .filter(|name| protected.contains(*name)) + .cloned() + .collect(); + + match protected_names.len() { + 0 => continue, // not a managed secret; nothing regenerates it + 1 => substitutions.push((value.clone(), protected_names[0].clone())), + _ => { + return Err(EmbeddedSecretConflict { + keys: protected_names, + }) + } + } + } + + // Longest first, so a value that contains another is replaced whole. + substitutions.sort_by(|a, b| b.0.len().cmp(&a.0.len()).then(a.0.cmp(&b.0))); + + Ok(rewrite_environment_lines(compose_content, |line| { + let mut rewritten = line.to_string(); + for (value, key) in &substitutions { + if rewritten.contains(value.as_str()) { + rewritten = rewritten.replace(value.as_str(), &format!("${{{key}}}")); + } + } + rewritten + })) +} + +/// The literal `KEY: value` pairs declared in service `environment:` blocks. +fn compose_literal_env_pairs(compose_content: &str) -> std::collections::BTreeMap { + let mut pairs = std::collections::BTreeMap::new(); + + for_each_environment_line(compose_content, |line| { + if let Some((key, value)) = line.trim().split_once(':') { + let key = key.trim(); + let value = value.trim().trim_matches('"').trim_matches('\''); + if is_env_identifier(key) && !value.is_empty() && !value.starts_with("${") { + pairs.insert(key.to_string(), value.to_string()); + } + } + }); + + pairs +} + +/// Apply `rewrite` to every line inside a service `environment:` block, +/// leaving the rest of the document untouched. +fn rewrite_environment_lines( + compose_content: &str, + mut rewrite: impl FnMut(&str) -> String, +) -> String { + let mut result = String::with_capacity(compose_content.len()); + + for (line, in_environment) in environment_lines(compose_content) { + if in_environment { + result.push_str(&rewrite(line)); + } else { + result.push_str(line); + } + result.push('\n'); + } + + result +} + +fn for_each_environment_line(compose_content: &str, mut visit: impl FnMut(&str)) { + for (line, in_environment) in environment_lines(compose_content) { + if in_environment { + visit(line); + } + } +} + +/// Pair every line with whether it sits inside a service `environment:` block. +/// +/// Shares the block-tracking rule used by [`parameterize_compose_env_vars`]: +/// the block ends at the first non-blank line indented at or above the +/// `environment:` key itself. +fn environment_lines(compose_content: &str) -> Vec<(&str, bool)> { + let mut in_environment = false; + let mut env_indent = 0usize; + let mut out = Vec::new(); + + for line in compose_content.lines() { + let trimmed = line.trim_start(); + let indent = line.len() - trimmed.len(); + + if trimmed == "environment:" { + in_environment = true; + env_indent = indent; + out.push((line, false)); + continue; + } + + if in_environment && (trimmed.is_empty() || indent <= env_indent) { + in_environment = false; + } + + out.push((line, in_environment)); + } + + out +} + +/// The `${VAR}` names a compose file references, in sorted order. +/// +/// Captured at bake time and pinned to the snapshot so the clone path can +/// verify the buyer's environment satisfies the image *before* a server is +/// created. Compose resolves an unsatisfied reference to an empty string and +/// only warns, so without this check the failure surfaces as a misconfigured +/// stack rather than a refused deploy. +pub fn collect_env_var_references(compose_content: &str) -> std::collections::BTreeSet { + let mut names = std::collections::BTreeSet::new(); + let bytes = compose_content.as_bytes(); + let mut i = 0usize; + + while i + 1 < bytes.len() { + if bytes[i] != b'$' || bytes[i + 1] != b'{' { + i += 1; + continue; + } + let Some(end) = compose_content[i + 2..].find('}') else { + break; + }; + let raw = &compose_content[i + 2..i + 2 + end]; + // Compose allows ${VAR:-default} / ${VAR-default} / ${VAR:?err}. + let name = raw.split([':', '-', '?', '+']).next().unwrap_or("").trim(); + if is_env_identifier(name) { + names.insert(name.to_string()); + } + i += 2 + end + 1; + } + + names +} + /// Returns `true` when `s` looks like a POSIX env-variable name. fn is_env_identifier(s: &str) -> bool { !s.is_empty() - && s.chars().enumerate().all(|(i, c)| { - c.is_ascii_alphanumeric() || c == '_' || (i == 0 && c == '.') - }) + && s.chars() + .enumerate() + .all(|(i, c)| c.is_ascii_alphanumeric() || c == '_' || (i == 0 && c == '.')) } // ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ @@ -1962,12 +2171,27 @@ services: let result = parameterize_compose_env_vars(compose, &keys); - assert!(result.contains("ADMIN_PASSWORD: ${ADMIN_PASSWORD}"), "admin pw:\n{result}"); - assert!(result.contains("SECRET_KEY: ${SECRET_KEY}"), "secret key:\n{result}"); - assert!(result.contains("DATABASE_URL: ${DATABASE_URL}"), "db url:\n{result}"); + assert!( + result.contains("ADMIN_PASSWORD: ${ADMIN_PASSWORD}"), + "admin pw:\n{result}" + ); + assert!( + result.contains("SECRET_KEY: ${SECRET_KEY}"), + "secret key:\n{result}" + ); + assert!( + result.contains("DATABASE_URL: ${DATABASE_URL}"), + "db url:\n{result}" + ); // Non-secret values stay as-is. - assert!(result.contains("ADMIN_USER: admin"), "admin user:\n{result}"); - assert!(result.contains("OLLAMA_MODEL: llama3.1"), "model:\n{result}"); + assert!( + result.contains("ADMIN_USER: admin"), + "admin user:\n{result}" + ); + assert!( + result.contains("OLLAMA_MODEL: llama3.1"), + "model:\n{result}" + ); } #[test] @@ -1988,11 +2212,153 @@ services: let result = parameterize_compose_env_vars(compose, &keys); - assert!(result.contains("SECRET_KEY: ${SECRET_KEY}"), "secret:\n{result}"); - assert!(result.contains("my.stacker.service: myapp"), "label:\n{result}"); + assert!( + result.contains("SECRET_KEY: ${SECRET_KEY}"), + "secret:\n{result}" + ); + assert!( + result.contains("my.stacker.service: myapp"), + "label:\n{result}" + ); assert!(result.contains("- app_data:/app/data"), "volume:\n{result}"); } + // ── embedded secret values (secrets inside a larger value) ────────────── + + fn env_map(pairs: &[(&str, &str)]) -> std::collections::BTreeMap { + pairs + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() + } + + fn key_set(keys: &[&str]) -> std::collections::BTreeSet { + keys.iter().map(|k| k.to_string()).collect() + } + + #[test] + fn embedded_secret_inside_a_dsn_is_parameterized() { + // The case whole-value replacement structurally cannot reach. + let compose = "\ +services: + app: + environment: + DATABASE_URL: postgresql://stackpilot:2213a996143863b99a0f2d3e22907690@db:5432/stackpilot +"; + let env = env_map(&[("POSTGRES_PASSWORD", "2213a996143863b99a0f2d3e22907690")]); + let result = + parameterize_embedded_secret_values(compose, &env, &key_set(&["POSTGRES_PASSWORD"])) + .expect("no conflict"); + + assert!( + result.contains("postgresql://stackpilot:${POSTGRES_PASSWORD}@db:5432/stackpilot"), + "password replaced in place:\n{result}" + ); + assert!( + !result.contains("2213a996143863b99a0f2d3e22907690"), + "no literal left:\n{result}" + ); + } + + #[test] + fn short_values_are_left_alone() { + // "admin" would otherwise corrupt every word containing it. + let compose = "services:\n app:\n environment:\n ADMIN_USER: admin\n GREETING: administrator\n"; + let env = env_map(&[("ADMIN_USER", "admin")]); + let result = parameterize_embedded_secret_values(compose, &env, &key_set(&["ADMIN_USER"])) + .expect("no conflict"); + assert!( + result.contains("GREETING: administrator"), + "untouched:\n{result}" + ); + } + + #[test] + fn undeclared_values_are_left_alone() { + // Nothing regenerates it on the buyer's box, so a ${REF} would resolve empty. + let compose = + "services:\n app:\n environment:\n URL: http://host/aaaaaaaaaaaaaaaa\n"; + let env = env_map(&[("SOME_KEY", "aaaaaaaaaaaaaaaa")]); + let result = + parameterize_embedded_secret_values(compose, &env, &std::collections::BTreeSet::new()) + .expect("no conflict"); + assert!( + result.contains("aaaaaaaaaaaaaaaa"), + "left literal:\n{result}" + ); + } + + #[test] + fn one_value_under_two_protected_names_is_refused() { + // stackpilot's DB_PASSWORD/POSTGRES_PASSWORD duplication: both are + // `generated`, so regeneration would hand them different values. + let compose = "\ +services: + db: + environment: + POSTGRES_PASSWORD: 2213a996143863b99a0f2d3e22907690 +"; + let env = env_map(&[("DB_PASSWORD", "2213a996143863b99a0f2d3e22907690")]); + let err = parameterize_embedded_secret_values( + compose, + &env, + &key_set(&["DB_PASSWORD", "POSTGRES_PASSWORD"]), + ) + .expect_err("duplicate must be refused"); + + assert_eq!( + err.keys, + vec!["DB_PASSWORD".to_string(), "POSTGRES_PASSWORD".to_string()] + ); + } + + #[test] + fn non_environment_blocks_are_never_rewritten() { + let compose = "\ +services: + app: + image: myapp:2213a996143863b99a0f2d3e22907690 + environment: + TOKEN: 2213a996143863b99a0f2d3e22907690 +"; + let env = env_map(&[("TOKEN", "2213a996143863b99a0f2d3e22907690")]); + let result = parameterize_embedded_secret_values(compose, &env, &key_set(&["TOKEN"])) + .expect("no conflict"); + + assert!( + result.contains("image: myapp:2213a996143863b99a0f2d3e22907690"), + "image digest untouched:\n{result}" + ); + assert!( + result.contains("TOKEN: ${TOKEN}"), + "env replaced:\n{result}" + ); + } + + #[test] + fn collects_env_var_references_with_defaults_and_ignores_literals() { + let compose = "\ +services: + app: + image: app:latest + environment: + A: ${ALPHA} + B: ${BETA:-fallback} + C: ${GAMMA?required} + D: plain-value +"; + let refs = collect_env_var_references(compose); + let found: Vec<&str> = refs.iter().map(String::as_str).collect(); + assert_eq!(found, vec!["ALPHA", "BETA", "GAMMA"]); + } + + #[test] + fn collects_env_var_reference_embedded_in_a_dsn() { + let compose = + "services:\n app:\n environment:\n DATABASE_URL: postgres://u:${PW}@h/db\n"; + assert!(collect_env_var_references(compose).contains("PW")); + } + #[test] fn parameterize_no_keys_returns_original() { let compose = "services:\n app:\n environment:\n FOO: bar\n"; diff --git a/src/db/baked_snapshot.rs b/src/db/baked_snapshot.rs index 7aed2106..160839f2 100644 --- a/src/db/baked_snapshot.rs +++ b/src/db/baked_snapshot.rs @@ -64,11 +64,12 @@ pub async fn record( healthy: bool, digests: Option, config_contract: Option, + required_env_keys: Option, ) -> Result { sqlx::query_as::<_, BakedSnapshot>( r#" - INSERT INTO baked_snapshots (stack, version, provider, image_id, healthy, digests, config_contract) - VALUES ($1, $2, $3, $4, $5, $6, $7) + INSERT INTO baked_snapshots (stack, version, provider, image_id, healthy, digests, config_contract, required_env_keys) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING * "#, ) @@ -79,6 +80,7 @@ pub async fn record( .bind(healthy) .bind(digests) .bind(config_contract) + .bind(required_env_keys) .fetch_one(pool) .await .map_err(|e| format!("Failed to record baked snapshot: {e}")) diff --git a/src/helpers/bake.rs b/src/helpers/bake.rs index 658446a1..7503890e 100644 --- a/src/helpers/bake.rs +++ b/src/helpers/bake.rs @@ -35,6 +35,11 @@ pub enum BakeError { NoImageId, #[error("bake backend error: {0}")] Backend(String), + /// The build box could not be sanitized before snapshotting. Refused rather + /// than published: an unsanitized image carries the author's credentials to + /// every buyer. + #[error("bake rejected: could not finalize build box: {0}")] + Finalize(String), } /// Pure gate: only a healthy build box with a real `image_id` yields a record. diff --git a/src/helpers/bake_finalize.rs b/src/helpers/bake_finalize.rs new file mode 100644 index 00000000..56eb5c89 --- /dev/null +++ b/src/helpers/bake_finalize.rs @@ -0,0 +1,434 @@ +//! Finalize a build box before it is snapshotted (immutable-deploy BAKE step). +//! +//! A bake snapshots the *whole disk* of a build box that the author deployed +//! normally. That disk carries three classes of the author's own secrets, none +//! of which a buyer's clone should inherit: +//! +//! 1. **Machine identity** — SSH host keys and `machine-id`. Left in place, +//! every clone of the image shares them, so any buyer can impersonate the +//! SSH host of any other buyer. This is the standard "sysprep" step every +//! golden-image pipeline performs (`virt-sysprep`, Packer, the DigitalOcean +//! 1-Click checklist); we had none. +//! 2. **The co-located `.env`** — shipped verbatim next to the compose file by +//! the deploy bundle, so the author's literal secrets sit in the image even +//! when the compose itself is fully parameterized. Parameterizing compose +//! alone just moves the secret from one file in the image to another. +//! 3. **Initialized data volumes** — a secret the app wrote into its own +//! database/volume on first run is frozen in the snapshot and is *not* +//! governed by environment variables any more. Postgres is the canonical +//! case: `POSTGRES_PASSWORD` is honoured only when it initializes an empty +//! data directory, so a cloned box silently keeps the author's role password. +//! +//! Everything here is a pure function producing shell, so the policy is +//! unit-tested without infra; [`crate::helpers::bake`] wires it to a real SSH +//! session. + +use std::collections::BTreeSet; + +/// Volumes whose content must survive the bake, keyed by stack slug. +/// +/// Defaulting to *reset* is deliberate: wrongly resetting a volume costs a +/// rebuild of cheap state, while wrongly keeping one leaks the author's +/// credentials to every buyer. Only volumes that are expensive to rebuild +/// **and** carry no credentials belong here. +/// +/// `stackpilot`'s Ollama volume holds the pulled model weights — gigabytes, +/// with a 600s pull timeout in `scripts/download-model.sh`. Preserving it is +/// the entire economic point of baking that stack. +pub fn volumes_to_keep(stack: &str) -> &'static [&'static str] { + match stack { + "stackpilot" => &["ollama"], + _ => &[], + } +} + +/// Shell to strip machine identity so each clone boots as a distinct host. +/// +/// `sshd` regenerates host keys on first boot when none are present, and +/// `systemd` repopulates an empty `/etc/machine-id`; clearing cloud-init's +/// instance state makes it treat the clone as a new instance and re-run its +/// per-instance modules. +pub fn identity_reset_commands() -> Vec { + vec![ + "rm -f /etc/ssh/ssh_host_*".to_string(), + ": > /etc/machine-id".to_string(), + "rm -f /var/lib/dbus/machine-id".to_string(), + "rm -rf /var/lib/cloud/instances /var/lib/cloud/instance".to_string(), + "rm -f /root/.bash_history /home/*/.bash_history".to_string(), + "find /var/log -type f -exec truncate -s 0 {} + 2>/dev/null || true".to_string(), + ] +} + +/// Shell to stop the stack and drop every data volume that is not explicitly +/// preserved, so the buyer's box initializes them from scratch with the +/// buyer's own generated values. +/// +/// Volume names are matched by suffix because Compose prefixes them with the +/// project name (`project_stackpilot_pgdata` for a declared `stackpilot_pgdata`). +pub fn volume_reset_commands(project_dir: &str, keep: &[&str]) -> Vec { + let mut cmds = vec![format!( + "cd {project_dir} && docker compose down --remove-orphans" + )]; + + let filter = if keep.is_empty() { + "cat".to_string() + } else { + let pattern = keep.join("|"); + format!("grep -Ev '({pattern})'") + }; + + cmds.push(format!( + "docker volume ls -q | {filter} | xargs -r docker volume rm -f" + )); + cmds +} + +/// Blank the values of secret-bearing keys in a `.env` file while keeping the +/// file's shape: keys, comments, blank lines and non-secret values survive. +/// +/// The keys are kept (rather than the lines dropped) so the file still +/// documents what the stack expects; on the buyer's box the whole file is +/// replaced from `/etc/stacker/env` by the systemd unit's `ExecStartPre`, so +/// the blanked values are never read. +pub fn scrub_env_file(content: &str, protected: &BTreeSet) -> String { + let mut out = String::with_capacity(content.len()); + + for line in content.lines() { + let trimmed = line.trim_start(); + if trimmed.is_empty() || trimmed.starts_with('#') { + out.push_str(line); + out.push('\n'); + continue; + } + + match line.split_once('=') { + Some((key, _value)) if should_blank(key.trim(), protected) => { + out.push_str(key); + out.push_str("=\n"); + } + _ => { + out.push_str(line); + out.push('\n'); + } + } + } + + out +} + +/// A key is blanked when the author's contract declared it regenerable/buyer- +/// supplied, or when its name is secret-shaped by the same heuristic the CLI +/// already uses for `generate-secrets.sh`. +fn should_blank(key: &str, protected: &BTreeSet) -> bool { + protected.contains(key) || crate::console::commands::cli::init::is_secret_env_key(key) +} + +/// Everything the finalize step needs to reach and sanitize a build box. +#[derive(Debug, Clone)] +pub struct FinalizeContext { + pub host: String, + pub port: u16, + pub user: String, + pub private_key_pem: String, + /// Where the deploy put the compose file and its co-located `.env`. + pub project_dir: String, + /// Stack slug — selects the volume keep-list. + pub stack: String, + /// Contract fields with `mutability: generated`/`provided`. + pub protected_keys: BTreeSet, +} + +/// What the finalize step learned about the image it just sanitized. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FinalizeOutcome { + /// `${VAR}` names the sanitized compose references — pinned to the snapshot + /// so the clone path can fail closed on an environment that cannot satisfy + /// them. + pub required_env_keys: BTreeSet, +} + +/// Sanitize a build box in place, immediately before it is snapshotted. +/// +/// Order matters: the compose/env rewrite has to happen while the stack is +/// still described on disk, the volume reset tears the stack down, and the +/// identity reset goes last because it leaves the box unable to present a +/// stable SSH identity afterwards. The build box is throwaway, so none of this +/// needs to be reversible. +pub async fn finalize_build_box( + ctx: &FinalizeContext, +) -> Result { + use crate::helpers::bake::BakeError; + use crate::helpers::ssh_client::{disconnect_ssh, exec_remote, open_ssh}; + + let fail = |stage: &str, err: String| BakeError::Finalize(format!("{stage}: {err}")); + + let session = open_ssh( + &ctx.host, + ctx.port, + &ctx.user, + &ctx.private_key_pem, + std::time::Duration::from_secs(30), + ) + .await + .map_err(|e| fail("ssh connect", e.to_string()))?; + + let run = |cmd: String| { + let session = &session; + async move { + let (stdout, stderr, code) = exec_remote(session, &cmd, 300) + .await + .map_err(|e| e.to_string())?; + if code != 0 { + return Err(format!("`{cmd}` exited {code}: {stderr}")); + } + Ok::(stdout) + } + }; + + let compose_path = format!("{}/docker-compose.yml", ctx.project_dir); + let env_path = format!("{}/.env", ctx.project_dir); + + let result = async { + // 1. Read what the deploy left on the box. + let compose = run(format!("cat {compose_path}")) + .await + .map_err(|e| fail("read compose", e))?; + // A stack may legitimately have no .env; treat that as empty. + let env_raw = run(format!("cat {env_path} 2>/dev/null || true")) + .await + .map_err(|e| fail("read .env", e))?; + let env_values = parse_env_pairs(&env_raw); + + // 2. Replace secrets embedded inside larger values (DSNs) with ${KEY} + // references. Whole-value keys were already parameterized at deploy + // time by `parameterize_compose_env_vars`. + let sanitized = crate::cli::generator::compose::parameterize_embedded_secret_values( + &compose, + &env_values, + &ctx.protected_keys, + ) + .map_err(|conflict| BakeError::Finalize(conflict.to_string()))?; + + if sanitized != compose { + write_remote_file(&run, &compose_path, &sanitized) + .await + .map_err(|e| fail("write compose", e))?; + } + + // 3. Record what the image now needs from the buyer's env file. + let required_env_keys = + crate::cli::generator::compose::collect_env_var_references(&sanitized); + + // 4. Blank the author's secrets in the co-located .env. The buyer's box + // overwrites this file wholesale from /etc/stacker/env at boot, so + // the blanked values are never read. + if !env_raw.trim().is_empty() { + let scrubbed = scrub_env_file(&env_raw, &ctx.protected_keys); + write_remote_file(&run, &env_path, &scrubbed) + .await + .map_err(|e| fail("write .env", e))?; + } + + // 5. Drop initialized data volumes so the buyer's box re-initializes + // them with the buyer's own values. A secret the app persisted on + // first run is not governed by env vars any more. + for cmd in volume_reset_commands(&ctx.project_dir, volumes_to_keep(&ctx.stack)) { + run(cmd).await.map_err(|e| fail("volume reset", e))?; + } + + // 6. Strip machine identity last. + for cmd in identity_reset_commands() { + run(cmd).await.map_err(|e| fail("identity reset", e))?; + } + + Ok(FinalizeOutcome { required_env_keys }) + } + .await; + + disconnect_ssh(session).await; + result +} + +/// Write `content` to `path` on the remote box without any quoting hazards: +/// the payload travels base64-encoded and is decoded on the far side. +async fn write_remote_file(run: &F, path: &str, content: &str) -> Result<(), String> +where + F: Fn(String) -> Fut, + Fut: std::future::Future>, +{ + use base64::{engine::general_purpose::STANDARD, Engine as _}; + let encoded = STANDARD.encode(content.as_bytes()); + run(format!("printf %s {encoded} | base64 -d > {path}")) + .await + .map(|_| ()) +} + +/// Parse `KEY=value` lines into a map, skipping comments and blanks. +pub fn parse_env_pairs(content: &str) -> std::collections::BTreeMap { + let mut pairs = std::collections::BTreeMap::new(); + + for line in content.lines() { + let trimmed = line.trim(); + if trimmed.is_empty() || trimmed.starts_with('#') { + continue; + } + if let Some((key, value)) = trimmed.split_once('=') { + let value = value.trim(); + if !value.is_empty() { + pairs.insert(key.trim().to_string(), value.to_string()); + } + } + } + + pairs +} + +/// The contract fields a buyer's box regenerates or supplies — i.e. the ones a +/// `${KEY}` reference can safely point at. +/// +/// Mirrors the selection `compose_env_keys` makes at deploy time +/// (`src/console/commands/cli/deploy.rs`), reading the contract as raw JSON so +/// an unparseable or partially-shaped contract degrades to "nothing protected" +/// rather than failing the bake outright. +pub fn protected_keys_from_contract(contract: &serde_json::Value) -> BTreeSet { + let mut keys = BTreeSet::new(); + + let Some(services) = contract.get("services").and_then(|v| v.as_object()) else { + return keys; + }; + + for service in services.values() { + let Some(fields) = service.get("fields").and_then(|v| v.as_object()) else { + continue; + }; + for (name, policy) in fields { + let protected = matches!( + policy.get("mutability").and_then(|v| v.as_str()), + Some("generated") | Some("provided") + ); + if protected { + keys.insert(name.clone()); + } + } + } + + keys +} + +#[cfg(test)] +mod tests { + use super::*; + + fn protected(keys: &[&str]) -> BTreeSet { + keys.iter().map(|k| k.to_string()).collect() + } + + #[test] + fn scrub_blanks_contract_declared_keys() { + let env = "SECRET_KEY=b838f1f2\nOLLAMA_MODEL=llama3.1\n"; + let out = scrub_env_file(env, &protected(["SECRET_KEY"].as_slice())); + assert!(out.contains("SECRET_KEY=\n"), "blanked:\n{out}"); + assert!( + out.contains("OLLAMA_MODEL=llama3.1"), + "non-secret kept:\n{out}" + ); + } + + #[test] + fn scrub_blanks_secret_shaped_keys_without_a_contract() { + // No contract at all — the name heuristic still has to catch these. + let env = "DB_PASSWORD=2213a996\nADMIN_USER=admin\n"; + let out = scrub_env_file(env, &BTreeSet::new()); + assert!(out.contains("DB_PASSWORD=\n"), "blanked:\n{out}"); + assert!(out.contains("ADMIN_USER=admin"), "non-secret kept:\n{out}"); + } + + #[test] + fn scrub_preserves_comments_and_blank_lines() { + let env = "# Secrets\n\nSECRET_KEY=abc\n"; + let out = scrub_env_file(env, &BTreeSet::new()); + assert_eq!(out, "# Secrets\n\nSECRET_KEY=\n"); + } + + #[test] + fn scrub_keeps_values_containing_equals_signs() { + // A blanked key must not be confused by '=' inside a kept value. + let env = "OLLAMA_MODEL=llama3.1\nJWT_SECRET=a=b=c\n"; + let out = scrub_env_file(env, &BTreeSet::new()); + assert!(out.contains("OLLAMA_MODEL=llama3.1"), "kept:\n{out}"); + assert!(out.contains("JWT_SECRET=\n"), "blanked:\n{out}"); + } + + #[test] + fn identity_reset_covers_host_keys_machine_id_and_cloud_init() { + let cmds = identity_reset_commands().join(" ; "); + assert!(cmds.contains("/etc/ssh/ssh_host_*"), "host keys: {cmds}"); + assert!(cmds.contains("/etc/machine-id"), "machine-id: {cmds}"); + assert!( + cmds.contains("/var/lib/cloud/instance"), + "cloud-init: {cmds}" + ); + } + + #[test] + fn volume_reset_preserves_the_kept_volumes() { + let cmds = volume_reset_commands("/home/trydirect/project", &["ollama"]).join(" ; "); + assert!( + cmds.contains("docker compose down"), + "stack stopped: {cmds}" + ); + assert!( + cmds.contains("grep -Ev '(ollama)'"), + "kept volume excluded from removal: {cmds}" + ); + } + + #[test] + fn volume_reset_without_a_keep_list_removes_everything() { + let cmds = volume_reset_commands("/home/trydirect/project", &[]).join(" ; "); + assert!(cmds.contains("| cat |"), "no filter applied: {cmds}"); + } + + #[test] + fn parse_env_pairs_skips_comments_blanks_and_empty_values() { + let env = "# header\n\nA=1\nEMPTY=\nB=two\n"; + let pairs = parse_env_pairs(env); + assert_eq!(pairs.get("A").map(String::as_str), Some("1")); + assert_eq!(pairs.get("B").map(String::as_str), Some("two")); + assert!(!pairs.contains_key("EMPTY"), "empty value is not a secret"); + } + + #[test] + fn protected_keys_cover_generated_and_provided_only() { + let contract = serde_json::json!({ + "services": { + "db": { "fields": { + "POSTGRES_PASSWORD": { "mutability": "generated" }, + "POSTGRES_USER": { "mutability": "fixed" } + }}, + "app": { "fields": { + "LICENSE_KEY": { "mutability": "provided" }, + "LOG_LEVEL": { "mutability": "editable" } + }} + } + }); + let keys = protected_keys_from_contract(&contract); + assert!(keys.contains("POSTGRES_PASSWORD")); + assert!(keys.contains("LICENSE_KEY")); + assert!(!keys.contains("POSTGRES_USER"), "fixed is not regenerated"); + assert!(!keys.contains("LOG_LEVEL"), "editable is not regenerated"); + } + + #[test] + fn protected_keys_from_a_shapeless_contract_is_empty() { + assert!(protected_keys_from_contract(&serde_json::Value::Null).is_empty()); + } + + #[test] + fn stackpilot_keeps_only_its_model_volume() { + assert_eq!(volumes_to_keep("stackpilot"), &["ollama"]); + // An unknown stack defaults to resetting everything — leaking is worse + // than rebuilding. + assert!(volumes_to_keep("something-else").is_empty()); + } +} diff --git a/src/helpers/mod.rs b/src/helpers/mod.rs index d741d19f..f071e56e 100644 --- a/src/helpers/mod.rs +++ b/src/helpers/mod.rs @@ -34,6 +34,7 @@ pub use dockerhub::*; pub use cloud::*; pub mod audit_cache; pub mod bake; +pub mod bake_finalize; pub mod bake_registry; pub mod cloud_init; pub mod compose_yaml; diff --git a/src/models/baked_snapshot.rs b/src/models/baked_snapshot.rs index 188b1e75..18f2a56a 100644 --- a/src/models/baked_snapshot.rs +++ b/src/models/baked_snapshot.rs @@ -19,6 +19,15 @@ pub struct BakedSnapshot { /// single value baked into the snapshot. `None` for snapshots baked before /// this column existed. pub config_contract: Option, + /// The `${VAR}` names the baked compose file references, captured at bake + /// time. The clone path checks the buyer's env against this before creating + /// a server: an unsatisfied reference resolves to an empty string at boot + /// with nothing but a Compose warning to show for it. + /// + /// `None` for snapshots baked before this column existed — those skip the + /// check rather than becoming undeployable. + #[sqlx(default)] + pub required_env_keys: Option, pub created_at: DateTime, } diff --git a/src/routes/oneclick_deploy/clone.rs b/src/routes/oneclick_deploy/clone.rs index 7f567f74..33b76a45 100644 --- a/src/routes/oneclick_deploy/clone.rs +++ b/src/routes/oneclick_deploy/clone.rs @@ -204,6 +204,44 @@ pub async fn clone_server( None => (Vec::new(), Vec::new()), }; + // ── fail closed on an env the image cannot boot with ───────────────── + // The bake pinned the `${VAR}` references its compose file actually needs. + // Compose resolves an unsatisfied reference to an *empty string* and only + // warns, and the unit's ExecStartPre ends in `|| true`, so a missing key + // produces a running-but-broken stack rather than a visible failure. Check + // before a server exists, so the buyer gets a refusal instead of a bill. + // + // Snapshots baked before `required_env_keys` carry NULL and skip this. + let provided: std::collections::BTreeSet = form + .env + .iter() + .filter(|(_, value)| !value.trim().is_empty()) + .map(|(key, _)| key.clone()) + .chain(regen.iter().map(|(key, _)| key.clone())) + .chain(regen_jwt.iter().map(|spec| spec.target_key.clone())) + .collect(); + + let missing = missing_required_env_keys(&snapshot.required_env_keys, &provided); + if !missing.is_empty() { + tracing::error!( + stack = %form.stack, + version = %snapshot.version, + missing = ?missing, + "refusing clone: baked compose references env keys the deploy would not supply" + ); + return HttpResponse::UnprocessableEntity().json(json!({ + "error": "Incomplete environment for this snapshot", + "details": format!( + "the baked image for '{}' v{} references {} environment variable(s) that this \ + deploy would not set ({}). They would resolve to empty strings at boot.", + form.stack, + snapshot.version, + missing.len(), + missing.join(", ") + ), + })); + } + // Render cloud-init with per-user env + domain. Secrets are pre-resolved (by // the user service) into `form.env`; `regen` mints fresh values for // `mutability: generated` fields on the box at first boot, reusing the same @@ -333,12 +371,11 @@ pub async fn clone_server( // template fallback stores a stack_definition blob that is not a // ProjectForm — parsing fails and the panel stays empty (the user can // add apps manually). - if let Ok(form) = serde_json::from_value::( - project.request_json.clone(), - ) { + if let Ok(form) = + serde_json::from_value::(project.request_json.clone()) + { if let Err(err) = - crate::project_app::sync_project_level_apps_from_form(&pg_pool, project.id, &form) - .await + crate::project_app::sync_project_level_apps_from_form(&pg_pool, project.id, &form).await { tracing::warn!( error = %err, @@ -652,6 +689,26 @@ pub async fn clone_server( }) } +/// The pinned `${VAR}` references the buyer's environment would leave unset. +/// +/// `required` is `baked_snapshots.required_env_keys` — a JSON array recorded at +/// bake time. `None` (a snapshot baked before that column) yields no misses, so +/// existing images keep deploying unchanged rather than becoming undeployable. +fn missing_required_env_keys( + required: &Option, + provided: &std::collections::BTreeSet, +) -> Vec { + let Some(serde_json::Value::Array(keys)) = required else { + return Vec::new(); + }; + + keys.iter() + .filter_map(serde_json::Value::as_str) + .filter(|key| !provided.contains(*key)) + .map(str::to_string) + .collect() +} + /// The generated fields whose fresh value must be minted on the cloned box, /// paired with the canonical shell generator for each. Reuses the *single* /// source of truth for the type→generator mapping @@ -744,13 +801,41 @@ fn derived_jwt_commands( #[cfg(test)] mod regen_tests { - use super::{derived_jwt_commands, regen_commands}; + use super::{derived_jwt_commands, missing_required_env_keys, regen_commands}; use serde_json::json; use std::collections::BTreeMap; /// A `generated` field with no installer-supplied value gets a regen command /// reusing the canonical shell generator; a `fixed` field and an /// already-supplied value are left alone. + #[test] + fn missing_required_env_keys_reports_only_unsatisfied_ones() { + let required = Some(json!(["ALPHA", "BETA", "GAMMA"])); + let provided: std::collections::BTreeSet = + ["ALPHA".to_string(), "GAMMA".to_string()] + .into_iter() + .collect(); + + assert_eq!( + missing_required_env_keys(&required, &provided), + vec!["BETA".to_string()] + ); + } + + /// Snapshots baked before the column must stay deployable. + #[test] + fn missing_required_env_keys_is_empty_for_legacy_snapshots() { + assert!(missing_required_env_keys(&None, &std::collections::BTreeSet::new()).is_empty()); + } + + #[test] + fn missing_required_env_keys_is_empty_when_all_supplied() { + let required = Some(json!(["POSTGRES_PASSWORD"])); + let provided: std::collections::BTreeSet = + ["POSTGRES_PASSWORD".to_string()].into_iter().collect(); + assert!(missing_required_env_keys(&required, &provided).is_empty()); + } + #[test] fn regen_commands_only_for_unset_generated_fields() { let contract: crate::cli::config_parser::ConfigContract = serde_json::from_value(json!({ From 07828d0a387d04fd70f163ca761d9dacb129ecc0 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Mon, 21 Sep 2026 14:37:55 +0300 Subject: [PATCH 14/20] fix(bake): close the gaps an audit found in build-box sanitization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to 27017fa0. An adversarial review of that change found the sanitize step could destroy the build box, refuse healthy deploys, or report success over an image it had not actually checked. Volume reset was enumerating every volume on the host, not the project's. On a real build box that removes the ingress' certificates and the agent's state, and aborts the bake on the first volume still held by a running container. Scoped to the project's own compose and matched through Compose's own label; keep entries are validated and matched on whole segments. The required-key list was read from the text of the compose file, so `${VAR:-default}` and `$$`-escaped text counted as required and no buyer could ever satisfy them. It now comes from the contract — the only thing that has a source on a buyer's machine. References the contract does not cover are resolved back to their literals before the snapshot, since a buyer's env file is replaced wholesale and would leave them empty. Further: - config_contract is now the only authority on what is sensitive; the weaker name heuristic is gone. A credential embedded inside a larger value (a password inside a DSN) is cleared in .env as well as in compose. - The author's own access no longer survives: authorized_keys, private keys, known_hosts and registry credentials are removed alongside the machine identity. Cloud-init appends the buyer's key rather than replacing the file, so a key left behind would grant root on every server cloned from the image. - The tear-down runs before the rewrites, so a cleared .env cannot fail `docker compose down` with the files already modified. - Files are written in chunks, so a large compose no longer exceeds the argument-length limit mid-sanitize; error messages no longer echo the payload. - A failure reports which steps completed and whether a retry against the same box is still equivalent. - The bake refuses when no contract resolved, when a compose reads values through `env_file:` that a buyer would silently lose, on an unknown argument, and when the contract describes a different version. - Value-stripping no longer clears the field policies inside config_contract itself. Adds scripts/check-staged-secrets.sh, wired as a pre-commit check — the scanner already configured in .pre-commit-config.yaml was never installed, so nothing was checking. Adds docs/SECRET_LIFECYCLE.md, tracing where one value lives at each stage and which component owns it. 2083 unit tests and 329 BDD scenarios green. Co-Authored-By: Claude Opus 5 --- .pre-commit-config.yaml | 6 + docs/ONE_CLICK_DEPLOY.md | 26 +- docs/SECRET_LIFECYCLE.md | 172 +++++ scripts/check-staged-secrets.sh | 182 +++++ src/bin/bake.rs | 43 +- src/cli/generator/compose.rs | 319 ++++++++- src/helpers/bake_finalize.rs | 858 +++++++++++++++++++++-- src/helpers/redact.rs | 96 ++- tests/features/bake_sanitization.feature | 331 +++++++++ tests/steps/bake_sanitization.rs | 603 ++++++++++++++++ tests/steps/mod.rs | 4 + 11 files changed, 2548 insertions(+), 92 deletions(-) create mode 100644 docs/SECRET_LIFECYCLE.md create mode 100755 scripts/check-staged-secrets.sh create mode 100644 tests/features/bake_sanitization.feature create mode 100644 tests/steps/bake_sanitization.rs diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c4e0b886..d394d9d6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,6 +7,12 @@ repos: stages: [commit] - repo: local hooks: + - id: check-staged-secrets + name: no concrete secret values in the commit + entry: scripts/check-staged-secrets.sh + language: script + pass_filenames: false + stages: [commit] - id: cargo-fmt name: cargo fmt --all entry: cargo fmt --all diff --git a/docs/ONE_CLICK_DEPLOY.md b/docs/ONE_CLICK_DEPLOY.md index f454403a..ee38626f 100644 --- a/docs/ONE_CLICK_DEPLOY.md +++ b/docs/ONE_CLICK_DEPLOY.md @@ -61,11 +61,31 @@ stacker/ StackerConfig::from_str+validate_semantics registry lookup → cloud - 200 `{ valid, name, version, composition {app, services[]} }`; 422 `{ valid:false, errors[], warnings[] }`. - Casbin `group_anonymous` rule (new migration, pattern `20260726120000_casbin_audit_public_rules.up.sql`). +> For how one secret value travels from the author's machine into a buyer's +> clone — and which component owns each step — see +> [SECRET_LIFECYCLE.md](SECRET_LIFECYCLE.md). + ### 2. `baked_snapshots` registry -- Migration `bake_snapshots` (stack, version, provider, image_id, healthy, digests JSONB, created_at). +- Columns: `stack`, `version`, `provider`, `image_id`, `healthy`, `digests` JSONB, + `created_at`, plus two added later: + - `config_contract` JSONB (`20260911120000`) — the author's field policy pinned to + the image, so the clone path regenerates `mutability: generated` fields per buyer + instead of every clone inheriting the one value baked at bake time. + - `required_env_keys` JSONB (`20260919120000`) — the `${VAR}` names the baked compose + references. The clone path refuses a deploy whose environment cannot satisfy them, + because Compose resolves an unsatisfied reference to an empty string with only a + warning. NULL means the check is skipped: either the snapshot predates the column, + or it was baked with `--allow-unsanitized-snapshot`. - `src/db/baked_snapshot.rs`, `src/models/baked_snapshot.rs`: `resolve`/`record`. -- Extend `src/bin/bake.rs` / `src/helpers/bake.rs` to persist the `BakeRecord`. -- Run `cargo sqlx prepare` after any sqlx query change. + Both reads are `SELECT *`. `required_env_keys` carries `#[sqlx(default)]`, so it + tolerates a database that has not run its migration yet; `config_contract` does + **not**, so `resolve()` fails outright against a database missing that column. +- `src/bin/bake.rs` / `src/helpers/bake.rs` persist the `BakeRecord`. Before snapshotting, + `src/helpers/bake_finalize.rs` sanitizes the build box over SSH (blank the co-located + `.env`, parameterize secrets embedded in compose values, drop credential-bearing data + volumes, strip SSH host keys / machine-id / cloud-init state) — hence `bake --ssh-key`. +- These three queries use runtime `sqlx::query_as`, not the compile-time macros, so they + need no `.sqlx` entry. Run `cargo sqlx prepare` after changing any *macro* query. ### 3. `POST /api/v1/deploy/clone` (protected) - Request `{ stack, version, region, server_type, domain, admin_email, env{} }`. diff --git a/docs/SECRET_LIFECYCLE.md b/docs/SECRET_LIFECYCLE.md new file mode 100644 index 00000000..1c0bed3b --- /dev/null +++ b/docs/SECRET_LIFECYCLE.md @@ -0,0 +1,172 @@ +# Lifecycle of one secret value + +Tracing a single value — a database password — from the author's laptop to a +buyer's cloned server. This is the reference for deciding **when a literal must +become a `${VAR}` reference**, and which component is responsible at each point. + +Related: [FIELD_POLICY.md](FIELD_POLICY.md) (author-facing guide to declaring the +policy) and [ONE_CLICK_DEPLOY.md](ONE_CLICK_DEPLOY.md) (the clone path). + +## The journey + +`` below stands for the author's literal password. + +| # | Stage | Where it runs | Form of the value | +|---|-------|---------------|-------------------| +| 1 | Author's `.env` | author's machine | `POSTGRES_PASSWORD=` | +| 2 | `stacker.yml` parsed, `${...}` expanded | **stacker CLI** (`src/cli/config_parser.rs`, `resolve_env_vars_with_fallback`) | literal | +| 3 | Compose rendered | **stacker CLI** (`src/console/commands/cli/deploy.rs`, `ComposeDefinition::try_from`) | literal | +| 4 | Protected keys turned back into references | **stacker CLI** (`parameterize_compose_env_vars`) | `${POSTGRES_PASSWORD}` | +| 5 | Config bundle posted to the backend | stacker CLI → **stacker server** (`src/routes/project/deploy.rs`) | compose with references, `.env` with literals | +| 6 | Queued, then Terraform + Ansible | **install service** (`src/connectors/install_service/client.rs`) | unchanged | +| 7 | Files land in `/home/trydirect//` | **target machine** (build box) | compose + co-located `.env` | +| 8 | `docker compose up` | **target machine** | literal, resolved from `./.env` — **and Postgres writes it into its data directory** | +| 9 | Finalize before the snapshot | `bake` binary, over SSH to the **target machine** (`src/helpers/bake_finalize.rs`) | stack stopped and data volumes dropped, embedded secrets replaced, references outside the contract put back to their values, `.env` cleared, machine identity and the author's own access stripped | +| 10 | Snapshot taken | Hetzner API → **baked image** | only `${POSTGRES_PASSWORD}`; no value anywhere | +| 11 | Cloud-init rendered for a clone | **stacker server** (`src/routes/oneclick_deploy/clone.rs`) | a fresh value per buyer | +| 12 | First boot: `/etc/stacker/env` copied over `./.env` | **buyer's machine** | new literal | +| 13 | `docker compose up` | **buyer's machine** | Postgres initializes from scratch | + +Stage 8 is the one that surprises people: the value does not only sit in files, +it is written **into the data directory**. Replacing it in compose is therefore +not enough — the volume has to be dropped at stage 9, or the buyer's server +keeps authenticating with the author's password. + +## When a literal needs a reference + +A value must become `${VAR}` when **both** hold: + +1. it survives the snapshot (it is in a file, or in a volume that is kept), and +2. it must differ per buyer. + +If only the first holds, leave it literal (`OLLAMA_MODEL: llama3.1`). If only the +second holds, there is nothing to replace. + +The mirror rule matters just as much: **every `${VAR}` baked into the image must +have something that fills it on the buyer's machine.** There are exactly two +such sources. + +| Class | Form in the baked compose | Filled on the buyer's machine by | Produced at | +|-------|---------------------------|----------------------------------|-------------| +| contract `generated` | `${VAR}` | cloud-init regeneration commands | stage 11, stacker server | +| contract `provided` | `${VAR}` | the buyer's submitted values | stage 11, stacker server | +| contract `fixed` | literal | nothing needed | — | +| not declared in the contract | literal | nothing needed | — | +| `${X:-default}` | left as written | its own default | stage 13, buyer's machine | +| `$$`-escaped | left as written | never interpolated at all | — | + +`config_contract` is the only authority on what is sensitive. Nothing is guessed +from variable names: name heuristics both miss real secrets and clear harmless +values, and the author already stated which fields matter. + +### Variables the author parameterized outside the contract + +An author may write `${SOMETHING}` in `stacker.yml` for a value that is not a +contract field. That reference is expanded at stage 2, but stage 4 turns plain +top-level `env:` keys back into references — and nothing fills those on the +buyer's machine. **Stage 9 therefore puts their value back**, so the image holds +a literal: the same for every buyer, which is exactly what a non-contract value +is. `${X:-default}` and `$$`-escaped text are left alone, since neither needs a +source. + +A reference with no value on the build box and no default fails the bake rather +than shipping an image that boots with an empty string. + +The single exception is a literal that *contains* a contract secret, such as +`DATABASE_URL=postgresql://user:@db:5432/app`. Stage 9 rewrites the embedded +part to `${POSTGRES_PASSWORD}`, because the surrounding key name (`DATABASE_URL`) +is not something any policy mentions, and a name-based rule cannot see it. + +Consequence worth accepting deliberately: a secret the author never declared +stays in the image. That follows directly from making the contract the only +authority. + +## What else must not survive the snapshot + +A secret value is not the only thing a snapshot carries forward. Two more are +removed at stage 9, both for the same reason — they are specific to the author's +machine, and a snapshot turns "specific to one machine" into "shared by every +buyer": + +| What | Why it matters | +|------|----------------| +| SSH host keys, `machine-id`, cloud-init instance state | every clone would present the same host identity, so one buyer can impersonate another buyer's server | +| `authorized_keys`, private keys, `known_hosts`, `~/.docker/config.json` | cloud-init *appends* the buyer's key rather than replacing the file, so an author key left in the image grants its holder root on every server cloned from it | + +`sshd` regenerates host keys on first boot when none are present, and `systemd` +repopulates an empty `machine-id`, so removal is enough — nothing has to be +recreated. + +Note the consequence for the operator: after stage 9 the build box no longer +accepts the bake key, so a failed bake cannot be retried by reconnecting to the +same machine. Start from a fresh build box instead. + +### Order of the finalize steps + +The tear-down runs **before** the files are rewritten. `docker compose down` +parses the compose, and a cleared `.env` would make any `${VAR}` outside an +environment block — `image: ${REGISTRY}/app:${TAG}`, `ports: ["${PORT}:80"]` — +resolve to empty and fail the tear-down, with the files already modified and +nothing to roll back to. Machine identity goes last, because after it the box no +longer accepts the bake key. + +### When a finalize fails part-way + +The steps are not transactional and most are not reversible, so a failure +reports what already completed. Re-running against the same box is rarely +equivalent: the compose is already sanitized, so the embedded-secret scan has +nothing left to find; the `.env` is already cleared, so there are no values to +search for; the data volumes are gone; and after the identity reset the box no +longer accepts the bake key at all. In every one of those cases the answer is a +fresh build box, and the error says so rather than leaving it to be discovered. + +### Values a clone would lose + +A service declaring `env_file:` reads its values out of that file, not through +`${VAR}`. Stage 12 replaces the file wholesale, so anything in it that is not a +contract field disappears on the buyer's machine — the container then starts +without values it had on the build box. The bake stops and names them; the fix +is to move those values into an `environment:` block, where they become part of +the image. + +Stacker's own generator writes values into `environment:`, so this only arises +with a hand-written `deploy.compose_file`. Both spellings of that block are +covered: + +```yaml +environment: environment: + KEY: value - KEY=value +``` + +## When the bake refuses + +Sanitizing depends entirely on the contract resolving. If it does not — the +template is not approved yet, `DATABASE_URL` is unset, the query failed, or the +author declared no fields — then nothing is substituted, nothing is cleared, and +an image carrying the author's values would be published with a confident +"Sanitized" line. The bake therefore stops unless `--allow-unsanitized-snapshot` +says the stack genuinely has no secrets. + +A project that ships no `.env` is normal and does **not** stop the bake: a +reference the buyer's machine cannot fill is already caught on its own. What the +bake does say out loud is that the embedded-secret scan had no values to search +for, since a mistyped `--project-dir` looks identical from here. + +## A reference with no source + +When a `${VAR}` is baked in and nothing fills it, three different stages are +involved: + +| | Stage | What happens | +|---|-------|--------------| +| **Created** | 4 — stacker CLI | the value is turned into a reference that nothing will fill later | +| **Caught** | 11 — stacker server | the clone compares the image's references against what will actually arrive, and refuses before creating a server | +| **Would surface** | 13 — buyer's machine | Compose substitutes an empty string and only warns; the systemd unit still reports active while the stack is broken | + +The refusal at stage 11 is therefore not noise — it covers the silent failure at +stage 13. The fix belongs at stage 4: never create a reference that has no source. + +Both sets are derived from the same place — the contract +(`required_env_keys` in `src/cli/generator/compose.rs`). Deriving the required +list from the *text* of the compose file instead pulls in things that need no +source at all (`${X:-default}`, `$$`-escaped text) and misses the ones that do. diff --git a/scripts/check-staged-secrets.sh b/scripts/check-staged-secrets.sh new file mode 100755 index 00000000..13c81b99 --- /dev/null +++ b/scripts/check-staged-secrets.sh @@ -0,0 +1,182 @@ +#!/usr/bin/env bash +# +# Refuse a commit that introduces a concrete secret value. +# +# This repository is public: a value committed here is disclosed permanently, +# and rotating it afterwards does not un-publish it. A real password reached +# `src/cli/generator/compose.rs` this way — the scanner configured in +# .pre-commit-config.yaml was never installed, so nothing checked. +# +# Deliberately narrow, because a noisy hook gets bypassed: +# 1. an assignment to a secret-named key whose value looks concrete +# 2. a long hex/base64 blob used as a value +# +# Placeholders, ${REFERENCES} and empty values are fine — those are what test +# fixtures and templates should contain. +# +# Opt out for a line that is genuinely not a secret: +# API_KEY=abcdef0123456789abcdef # pragma: allowlist secret +# +# Scan the whole tree instead of the staged diff with --all. + +set -uo pipefail + +RED=$'\033[31m'; YELLOW=$'\033[33m'; RESET=$'\033[0m' +[ -t 1 ] || { RED=''; YELLOW=''; RESET=''; } + +if [ "${1:-}" = "--all" ]; then + added=$(git grep -n '' -- . | sed 's/^/+/') +else + # Only added lines of the staged diff, with their file and line numbers. + added=$(git diff --cached --unified=0 --no-color -- . | awk ' + /^\+\+\+ b\// { file = substr($0, 7); next } + /^@@/ { + match($0, /\+[0-9]+/) + line = substr($0, RSTART + 1, RLENGTH - 1) - 1 + next + } + /^\+/ && !/^\+\+\+/ { line++; print file ":" line ":" substr($0, 2) } + ') +fi + +[ -n "$added" ] || exit 0 + +SECRET_KEY_RE='(PASSWORD|PASSWD|PWD|SECRET|TOKEN|API_?KEY|ACCESS_KEY|CREDENTIAL|PRIVATE_KEY|MASTERKEY|SIGNING_KEY|ENCRYPTION_KEY)' + +findings=$(printf '%s\n' "$added" | awk -v keyre="$SECRET_KEY_RE" ' + # An explicit opt-out wins. + /pragma: allowlist secret/ { next } + + { + # Split "path:line:content" while keeping colons inside the content. + i = index($0, ":"); path = substr($0, 1, i - 1) + rest = substr($0, i + 1) + j = index(rest, ":"); lineno = substr(rest, 1, j - 1) + content = substr(rest, j + 1) + } + + # The value side of KEY=value or "KEY: value". + { + value = "" + if (match(content, /[A-Za-z_][A-Za-z0-9_]*[[:space:]]*[=:][[:space:]]*/)) { + key = substr(content, RSTART, RLENGTH) + value = substr(content, RSTART + RLENGTH) + } + } + + { + gsub(/^["\x27[:space:]]+|["\x27,[:space:]]+$/, "", value) + } + + # Placeholders and references are exactly what belongs in a template. + value == "" { next } + value ~ /^\$/ { next } + value ~ /^<.*>$/ { next } + tolower(value) ~ /^(changeme|change-me|placeholder|example|redacted|secret|password|test|dummy|xxx+|\*+)$/ { next } + # Self-describing placeholders: your_x_here, x_goes_here, SHOULD_BE_*, TODO. + tolower(value) ~ /^(your|my|some)_/ { next } + tolower(value) ~ /_here$|_goes_here$|^todo|^fixme|should_be/ { next } + # A value equal to its own key name is a template, not a credential. + toupper(value) == toupper(keyname(key)) { next } + # Well-known defaults that belong to nobody. + tolower(value) ~ /^(postgres|mysql|root|admin|guest|user|local|localhost|none|null)$/ { next } + value ~ /^(.)\1+$/ { next } + + # 0. Credentials inside a URL. The key name says nothing here + # (DATABASE_URL, REDIS_URL, AMQP_URL), so nothing else catches it — + # this is the shape that actually leaked. + content ~ /[a-zA-Z][a-zA-Z0-9+.-]*:\/\/[^:\/@[:space:]]+:[^@[:space:]]+@/ { + # A ${REFERENCE} or a in the password position is the + # shape we want authors to use, not a leak. + if (content !~ /:\/\/[^:\/@[:space:]]+:[\$<]/ && + !looks_synthetic(url_password(content)) && + !is_placeholder(url_password(content))) { + print path ":" lineno ": credentials embedded in a URL" + next + } + } + + # A secret value is a single opaque token. Anything with whitespace or code + # punctuation is a line of source, not an assignment of a credential — + # `const MIN_SECRET_LEN: usize = 12;` must not stop a commit. + value ~ /[[:space:]]/ { next } + value ~ /[(){}<>;,]|::/ { next } + + # Obvious test fixtures: a real secret is not a repeated block. + looks_synthetic(value) { next } + + # 1. A secret-named key carrying something that looks generated. Generated + # credentials mix digits and letters (hex, base64, alphanumeric); a + # descriptive fixture word like `author-value` does not. + toupper(key) ~ keyre && length(value) >= 8 && + ((value ~ /[0-9]/ && value ~ /[A-Za-z]/) || length(value) >= 20) { + print path ":" lineno ": secret-named key with a literal value" + next + } + + # 2. A long hex or base64 blob as a value, whatever the key is called. + value ~ /^[0-9a-fA-F]{32,}$/ { + print path ":" lineno ": " length(value) "-char hex value" + next + } + value ~ /^[A-Za-z0-9+\/]{40,}={0,2}$/ { + print path ":" lineno ": long base64-looking value" + } + + # The bare key name from a "KEY=" / "KEY:" capture. + function keyname(k) { + gsub(/[[:space:]=:]+$/, "", k) + return k + } + + # Placeholder credentials: well-known defaults and self-describing stand-ins. + function is_placeholder(v) { + v = tolower(v) + if (v ~ /^(postgres|mysql|root|admin|guest|user|password|changeme|secret|example|test)$/) return 1 + if (v ~ /^(your|my|some)_/) return 1 + if (v ~ /_here$|_goes_here$|should_be/) return 1 + return 0 + } + + # The credential between "//user:" and "@" of the first URL on the line. + function url_password(line, rest, at) { + if (!match(line, /:\/\/[^:\/@[:space:]]+:/)) return "" + rest = substr(line, RSTART + RLENGTH) + at = index(rest, "@") + return at ? substr(rest, 1, at - 1) : rest + } + + # True for values that no random generator would produce: a repetition of a + # shorter block (0123456789abcdef0123456789abcdef) or a single repeated + # character. Test fixtures need secret-shaped values; real secrets are not + # shaped like this. + function looks_synthetic(v, n, half, i) { + if (v == "") return 0 + if (v ~ /^(.)\1+$/) return 1 + n = length(v) + for (half = 1; half <= n / 2; half++) { + if (n % half != 0) continue + if (v == repeat(substr(v, 1, half), n / half)) return 1 + } + return 0 + } + + function repeat(unit, times, out, i) { + out = "" + for (i = 0; i < times; i++) out = out unit + return out + } +') + +[ -n "$findings" ] || exit 0 + +echo "${RED}Commit refused: a concrete secret value would be committed.${RESET}" >&2 +echo >&2 +printf '%s\n' "$findings" | sed 's/^/ /' >&2 +echo >&2 +echo "${YELLOW}This repository is public — committing a value discloses it permanently," >&2 +echo "and rotating afterwards does not un-publish it.${RESET}" >&2 +echo >&2 +echo "Use a \${REFERENCE} or a placeholder. If the line is genuinely not a secret:" >&2 +echo " append # pragma: allowlist secret" >&2 +exit 1 diff --git a/src/bin/bake.rs b/src/bin/bake.rs index 547d5a96..17558565 100644 --- a/src/bin/bake.rs +++ b/src/bin/bake.rs @@ -74,8 +74,15 @@ async fn main() -> Result<(), Box> { i += 1; } other => { - eprintln!("ignoring unknown arg: {other}"); - i += 1; + // Shrugging this off is how `--sshkey` silently became "no + // --ssh-key", and a mistyped `--stack` silently bakes under the + // default slug — pinning the wrong contract to the image. + return Err(format!( + "unknown argument `{other}`. Supported: --ip, --server-id, --stack, \ + --version, --health-url, --ssh-key, --ssh-user, --project-dir, \ + --allow-unsanitized-snapshot" + ) + .into()); } } } @@ -119,7 +126,7 @@ async fn main() -> Result<(), Box> { }; let config_contract = match &pool { - Some(pool) => resolve_config_contract(pool, &stack).await, + Some(pool) => resolve_config_contract(pool, &stack, &version).await, None => None, }; let protected_keys = config_contract @@ -127,6 +134,12 @@ async fn main() -> Result<(), Box> { .map(stacker::helpers::bake_finalize::protected_keys_from_contract) .unwrap_or_default(); + // Refuse before touching the box: with no contract there is nothing to + // sanitize, and publishing anyway is how the author's credentials reach + // every buyer. + stacker::helpers::bake_finalize::check_contract_usable(&protected_keys, allow_unsanitized) + .map_err(|e| e.to_string())?; + // Sanitize the build box before the snapshot is taken. let finalize_outcome = match (&ssh_key, allow_unsanitized) { (Some(key_path), _) => { @@ -226,11 +239,29 @@ async fn main() -> Result<(), Box> { /// The author's field policy for `stack`, resolved by the same slug the /// snapshot registry keys on (`baked_snapshots.stack == stack_template.slug`). /// -/// Best-effort: an un-catalogued or unapproved stack bakes with no contract and -/// the clone path degrades to the baked values. -async fn resolve_config_contract(pool: &sqlx::PgPool, stack: &str) -> Option { +/// `get_config_contract` reads the template's *latest* version. Baking some +/// other version would pin the wrong field set to the image, so the versions +/// are compared and a mismatch stops the bake rather than shipping a snapshot +/// whose contract describes a different release. +async fn resolve_config_contract( + pool: &sqlx::PgPool, + stack: &str, + version: &str, +) -> Option { match stacker::db::marketplace::get_approved_by_slug(pool, stack).await { Ok(Some(template)) => { + match stacker::db::marketplace::get_by_slug_with_latest(pool, stack).await { + Ok((_, Some(latest))) if latest.version != version => { + eprintln!( + "WARNING: baking '{stack}' v{version}, but the marketplace's latest \ + version is v{}. The contract describes the latest version, so it \ + would not match this image — resubmit or bake the latest version.", + latest.version + ); + return None; + } + _ => {} + } match stacker::db::marketplace::get_config_contract(pool, template.id).await { Ok(serde_json::Value::Null) => { eprintln!("WARNING: template '{stack}' declares no config_contract — nothing will be regenerated per buyer."); diff --git a/src/cli/generator/compose.rs b/src/cli/generator/compose.rs index 75eeb85f..203eb68d 100644 --- a/src/cli/generator/compose.rs +++ b/src/cli/generator/compose.rs @@ -1,3 +1,4 @@ +use std::collections::BTreeSet; use std::collections::HashMap; use std::convert::TryFrom; use std::fmt; @@ -819,8 +820,26 @@ pub fn parameterize_compose_env_vars( } if in_environment { - // Match " KEY: value" — the key must be a valid env identifier. - if let Some((key, _rest)) = trimmed.split_once(':') { + // Compose accepts either form inside `environment:`; a hand-written + // compose (`deploy.compose_file`) commonly uses the list one. + // + // environment: environment: + // KEY: value - KEY=value + if let Some(entry) = trimmed.strip_prefix("- ") { + if let Some((key, _value)) = entry.split_once('=') { + let key = key.trim(); + if is_env_identifier(key) && env_keys.contains(key) { + result.push_str(&line[..indent]); + result.push_str("- "); + result.push_str(key); + result.push_str("=${"); + result.push_str(key); + result.push_str("}\n"); + continue; + } + } + } else if let Some((key, _rest)) = trimmed.split_once(':') { + // Match " KEY: value" — the key must be a valid env identifier. let key = key.trim(); if is_env_identifier(key) && env_keys.contains(key) { // Preserve the original indent and replace the value. @@ -1032,12 +1051,24 @@ pub fn collect_env_var_references(compose_content: &str) -> std::collections::BT let mut i = 0usize; while i + 1 < bytes.len() { + // `$$` is Compose's escape — the text is emitted literally and never + // interpolated, so neither `$` may start a reference. + if bytes[i] == b'$' && bytes[i + 1] == b'$' { + i += 2; + continue; + } if bytes[i] != b'$' || bytes[i + 1] != b'{' { i += 1; continue; } - let Some(end) = compose_content[i + 2..].find('}') else { - break; + // A reference never spans lines: bound the search so an unterminated + // `${` cannot swallow the next line's reference along with it. + let Some(end) = closing_brace_on_line(&compose_content[i + 2..]) else { + // Unterminated `${` — skip it and keep scanning. Abandoning the rest + // of the document here would silently under-report the references + // that follow. + i += 2; + continue; }; let raw = &compose_content[i + 2..i + 2 + end]; // Compose allows ${VAR:-default} / ${VAR-default} / ${VAR:?err}. @@ -1051,6 +1082,117 @@ pub fn collect_env_var_references(compose_content: &str) -> std::collections::BT names } +/// A `${VAR}` in the baked compose that nothing will fill on the buyer's machine. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct UnresolvedReference { + pub name: String, +} + +/// Put back the literal value of every `${VAR}` the contract does **not** cover, +/// and report the ones that cannot be resolved. +/// +/// Stage 4 (`parameterize_compose_env_vars`, in the CLI) turns two different +/// things into references: contract fields, and plain top-level `env:` keys. For +/// an ordinary deploy both are fine — the `.env` travels next to the compose and +/// stays intact. For a baked image they are not: at first boot the buyer's +/// machine overwrites that `.env` wholesale from `/etc/stacker/env`, which only +/// ever holds contract fields and the buyer's own values. A reference to +/// anything else would resolve to an empty string, and Compose only warns. +/// +/// So the image keeps references *only* for values that have a source on the +/// buyer's machine, and everything else goes back to being a literal — which is +/// correct, because those values are the same for every buyer. +/// +/// Left untouched: `$$`-escaped text (Compose never interpolates it) and +/// `${VAR:-default}` forms that carry their own fallback. +pub fn resolve_non_contract_references( + compose_content: &str, + env_values: &std::collections::BTreeMap, + protected: &BTreeSet, +) -> (String, Vec) { + let mut out = String::with_capacity(compose_content.len()); + let mut unresolved = Vec::new(); + let bytes = compose_content.as_bytes(); + let mut i = 0usize; + + while i < bytes.len() { + // `$$` is Compose's escape: it is emitted literally, never interpolated. + // Copy both bytes untouched so we do not mistake the second `$` for the + // start of a reference. + if bytes[i] == b'$' && i + 1 < bytes.len() && bytes[i + 1] == b'$' { + out.push_str("$$"); + i += 2; + continue; + } + + if bytes[i] != b'$' || i + 1 >= bytes.len() || bytes[i + 1] != b'{' { + out.push(compose_content[i..].chars().next().unwrap_or('\0')); + i += compose_content[i..] + .chars() + .next() + .map(char::len_utf8) + .unwrap_or(1); + continue; + } + + let Some(end) = closing_brace_on_line(&compose_content[i + 2..]) else { + // Unterminated `${` — copy it and carry on rather than abandoning + // the rest of the document. + out.push_str("${"); + i += 2; + continue; + }; + + let raw = &compose_content[i + 2..i + 2 + end]; + let whole = &compose_content[i..i + 2 + end + 1]; + let has_default = raw.contains(":-") || raw.contains(":?") || raw.contains('-'); + let name = raw.split([':', '-', '?', '+']).next().unwrap_or("").trim(); + + if !is_env_identifier(name) || protected.contains(name) { + // Not a reference we manage, or one the buyer's machine will fill. + out.push_str(whole); + } else if let Some(value) = env_values.get(name) { + out.push_str(value); + } else if has_default { + // Compose supplies the fallback itself; nothing is missing. + out.push_str(whole); + } else { + out.push_str(whole); + unresolved.push(UnresolvedReference { + name: name.to_string(), + }); + } + + i += 2 + end + 1; + } + + unresolved.sort_by(|a, b| a.name.cmp(&b.name)); + unresolved.dedup(); + (out, unresolved) +} + +/// The contract fields the sanitized compose still references — exactly the set +/// the buyer's machine has to fill. +/// +/// Derived from the contract rather than from the text of the file: a reference +/// is only "required" if something is expected to supply it, and the only +/// suppliers are the regeneration commands and the buyer's own values, both of +/// which are driven by the contract. +pub fn required_env_keys(compose_content: &str, protected: &BTreeSet) -> BTreeSet { + collect_env_var_references(compose_content) + .into_iter() + .filter(|name| protected.contains(name)) + .collect() +} + +/// Offset of the `}` that closes a `${` opened at the start of `rest`, or +/// `None` when the line ends first — a reference never spans lines, and letting +/// the search run on would consume the next line's reference too. +fn closing_brace_on_line(rest: &str) -> Option { + let line_end = rest.find('\n').unwrap_or(rest.len()); + rest[..line_end].find('}') +} + /// Returns `true` when `s` looks like a POSIX env-variable name. fn is_env_identifier(s: &str) -> bool { !s.is_empty() @@ -2157,10 +2299,10 @@ services: ports: - \"8080:8000\" environment: - ADMIN_PASSWORD: 4f4237dd9bfe8e1622706cac7bab63c7 + ADMIN_PASSWORD: fedcba9876543210fedcba9876543210 ADMIN_USER: admin - DATABASE_URL: postgresql://stackpilot:2213a996143863b99a0f2d3e22907690@db:5432/stackpilot - SECRET_KEY: b838f1f22379b8c268a4d3e0268459761c18947e1576956a6d9b1f3928070df4 + DATABASE_URL: postgresql://stackpilot:0123456789abcdef0123456789abcdef@db:5432/stackpilot + SECRET_KEY: 00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff OLLAMA_MODEL: llama3.1 restart: unless-stopped "; @@ -2243,9 +2385,9 @@ services: services: app: environment: - DATABASE_URL: postgresql://stackpilot:2213a996143863b99a0f2d3e22907690@db:5432/stackpilot + DATABASE_URL: postgresql://stackpilot:0123456789abcdef0123456789abcdef@db:5432/stackpilot "; - let env = env_map(&[("POSTGRES_PASSWORD", "2213a996143863b99a0f2d3e22907690")]); + let env = env_map(&[("POSTGRES_PASSWORD", "0123456789abcdef0123456789abcdef")]); let result = parameterize_embedded_secret_values(compose, &env, &key_set(&["POSTGRES_PASSWORD"])) .expect("no conflict"); @@ -2255,7 +2397,7 @@ services: "password replaced in place:\n{result}" ); assert!( - !result.contains("2213a996143863b99a0f2d3e22907690"), + !result.contains("0123456789abcdef0123456789abcdef"), "no literal left:\n{result}" ); } @@ -2296,9 +2438,9 @@ services: services: db: environment: - POSTGRES_PASSWORD: 2213a996143863b99a0f2d3e22907690 + POSTGRES_PASSWORD: 0123456789abcdef0123456789abcdef "; - let env = env_map(&[("DB_PASSWORD", "2213a996143863b99a0f2d3e22907690")]); + let env = env_map(&[("DB_PASSWORD", "0123456789abcdef0123456789abcdef")]); let err = parameterize_embedded_secret_values( compose, &env, @@ -2317,16 +2459,16 @@ services: let compose = "\ services: app: - image: myapp:2213a996143863b99a0f2d3e22907690 + image: myapp:0123456789abcdef0123456789abcdef environment: - TOKEN: 2213a996143863b99a0f2d3e22907690 + TOKEN: 0123456789abcdef0123456789abcdef "; - let env = env_map(&[("TOKEN", "2213a996143863b99a0f2d3e22907690")]); + let env = env_map(&[("TOKEN", "0123456789abcdef0123456789abcdef")]); let result = parameterize_embedded_secret_values(compose, &env, &key_set(&["TOKEN"])) .expect("no conflict"); assert!( - result.contains("image: myapp:2213a996143863b99a0f2d3e22907690"), + result.contains("image: myapp:0123456789abcdef0123456789abcdef"), "image digest untouched:\n{result}" ); assert!( @@ -2335,6 +2477,25 @@ services: ); } + #[test] + fn collects_env_var_reference_skips_escaped_and_survives_unterminated() { + // `$$` is Compose's escape — the container sees literal `${HOME}` and + // nothing is interpolated, so it is not a reference. + let escaped = "services:\n app:\n command: sh -c 'echo $${HOME}'\n"; + assert!( + collect_env_var_references(escaped).is_empty(), + "escaped text must not count as a reference" + ); + + // A stray `${` must not discard everything after it. + let broken = + "services:\n app:\n environment:\n A: ${BROKEN\n B: ${REAL_ONE}\n"; + assert!( + collect_env_var_references(broken).contains("REAL_ONE"), + "scanning must continue past an unterminated reference" + ); + } + #[test] fn collects_env_var_references_with_defaults_and_ignores_literals() { let compose = "\ @@ -2359,6 +2520,132 @@ services: assert!(collect_env_var_references(compose).contains("PW")); } + // ── references that survive into the baked image ─────────────────────── + + #[test] + fn non_contract_reference_goes_back_to_its_literal() { + // A top-level `env:` key is turned into a reference at deploy time, but + // nothing fills it on the buyer's machine — the .env there is replaced + // wholesale from /etc/stacker/env, which only holds contract fields. + let compose = "services:\n app:\n environment:\n REGION: ${REGION}\n"; + let env = env_map(&[("REGION", "fsn1")]); + + let (out, unresolved) = + resolve_non_contract_references(compose, &env, &std::collections::BTreeSet::new()); + + assert!( + out.contains("REGION: fsn1"), + "put back as a literal:\n{out}" + ); + assert!(unresolved.is_empty(), "nothing missing: {unresolved:?}"); + } + + #[test] + fn contract_reference_is_kept_as_a_reference() { + let compose = + "services:\n db:\n environment:\n POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}\n"; + let env = env_map(&[("POSTGRES_PASSWORD", "aaaaaaaaaaaaaaaa")]); + + let (out, _) = + resolve_non_contract_references(compose, &env, &key_set(&["POSTGRES_PASSWORD"])); + + assert!( + out.contains("POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}"), + "the buyer's machine fills this one:\n{out}" + ); + assert!( + !out.contains("aaaaaaaaaaaaaaaa"), + "the author's value must not come back:\n{out}" + ); + } + + #[test] + fn escaped_and_defaulted_references_are_left_untouched() { + let compose = "services:\n app:\n environment:\n LOG: ${LOG_LEVEL:-info}\n\ + \x20 CMD: echo $${HOME}\n"; + let (out, unresolved) = resolve_non_contract_references( + compose, + &std::collections::BTreeMap::new(), + &std::collections::BTreeSet::new(), + ); + + assert!(out.contains("${LOG_LEVEL:-info}"), "default kept:\n{out}"); + assert!(out.contains("$${HOME}"), "escape kept:\n{out}"); + assert!( + unresolved.is_empty(), + "neither needs a source: {unresolved:?}" + ); + } + + #[test] + fn a_reference_with_no_value_and_no_default_is_reported() { + let compose = "services:\n app:\n environment:\n TOKEN: ${MISSING}\n"; + let (_, unresolved) = resolve_non_contract_references( + compose, + &std::collections::BTreeMap::new(), + &std::collections::BTreeSet::new(), + ); + + assert_eq!( + unresolved, + vec![UnresolvedReference { + name: "MISSING".to_string() + }] + ); + } + + #[test] + fn required_keys_come_from_the_contract_not_the_file_text() { + let compose = "services:\n app:\n environment:\n\ + \x20 SECRET_KEY: ${SECRET_KEY}\n\ + \x20 LOG: ${LOG_LEVEL:-info}\n\ + \x20 CMD: echo $${HOME}\n\ + \x20 REGION: ${REGION}\n"; + + let required = required_env_keys(compose, &key_set(&["SECRET_KEY"])); + let found: Vec<&str> = required.iter().map(String::as_str).collect(); + + assert_eq!( + found, + vec!["SECRET_KEY"], + "only contract fields have a source on the buyer's machine" + ); + } + + /// M5 — Compose accepts `environment:` as a list as well as a mapping, and + /// a user-supplied compose (`deploy.compose_file`) often uses the list form. + /// Missing it leaves the author's secrets literal in the snapshot. + #[test] + fn parameterize_handles_the_list_form_of_environment() { + let compose = "\ +services: + db: + environment: + - POSTGRES_PASSWORD=aaaaaaaaaaaaaaaa + - POSTGRES_USER=stackpilot +"; + let mut keys = std::collections::HashSet::new(); + keys.insert("POSTGRES_PASSWORD".to_string()); + + let result = parameterize_compose_env_vars(compose, &keys); + + assert!( + result.contains("- POSTGRES_PASSWORD=${POSTGRES_PASSWORD}"), + "list entry replaced:\n{result}" + ); + assert!( + result.contains("- POSTGRES_USER=stackpilot"), + "undeclared entry untouched:\n{result}" + ); + } + + #[test] + fn parameterize_list_form_leaves_undeclared_keys_alone() { + let compose = "services:\n app:\n environment:\n - LOG_LEVEL=debug\n"; + let keys = std::collections::HashSet::new(); + assert_eq!(parameterize_compose_env_vars(compose, &keys), compose); + } + #[test] fn parameterize_no_keys_returns_original() { let compose = "services:\n app:\n environment:\n FOO: bar\n"; diff --git a/src/helpers/bake_finalize.rs b/src/helpers/bake_finalize.rs index 56eb5c89..a2a08c27 100644 --- a/src/helpers/bake_finalize.rs +++ b/src/helpers/bake_finalize.rs @@ -42,55 +42,228 @@ pub fn volumes_to_keep(stack: &str) -> &'static [&'static str] { } } -/// Shell to strip machine identity so each clone boots as a distinct host. +/// Values a service would lose when the buyer's machine replaces the env file. /// -/// `sshd` regenerates host keys on first boot when none are present, and -/// `systemd` repopulates an empty `/etc/machine-id`; clearing cloud-init's -/// instance state makes it treat the clone as a new instance and re-run its -/// per-instance modules. +/// A service declaring `env_file:` reads its values out of that file rather than +/// through `${VAR}` in the compose. At first boot the buyer's machine overwrites +/// the file wholesale from `/etc/stacker/env`, which carries only contract +/// fields and the buyer's own values — so every other key in it disappears, and +/// the container starts without values it had on the build box. +/// +/// Returns the keys that would be lost, in sorted order. Empty when the compose +/// declares no `env_file:` (stacker's own generator writes values into +/// `environment:` instead, so this only arises with a hand-written +/// `deploy.compose_file`). +/// +/// The author's fix is to move those values into an `environment:` block, where +/// they become literals in the image and survive. +pub fn env_file_values_lost_on_clone( + compose_content: &str, + env_values: &std::collections::BTreeMap, + protected: &BTreeSet, +) -> Vec { + let declares_env_file = compose_content + .lines() + .any(|line| line.trim_start().starts_with("env_file:")); + + if !declares_env_file { + return Vec::new(); + } + + env_values + .keys() + .filter(|key| !protected.contains(*key)) + .cloned() + .collect() +} + +/// Refuse a bake that has no field policy to sanitize against. +/// +/// With no protected keys the embedded-secret scan substitutes nothing and the +/// `.env` scrub clears nothing, yet the bake would still print a confident +/// "Sanitized" line and publish an image carrying the author's credentials. +/// The usual causes are mundane — the template is not approved yet, so +/// `get_approved_by_slug` returns nothing; `DATABASE_URL` is unset so the +/// contract was never looked up; or the author declared no fields at all. +/// +/// A stack that genuinely has no secrets can pass `--allow-unsanitized-snapshot`. +pub fn check_contract_usable( + protected: &BTreeSet, + allow_unsanitized: bool, +) -> Result<(), crate::helpers::bake::BakeError> { + if allow_unsanitized || !protected.is_empty() { + return Ok(()); + } + + Err(crate::helpers::bake::BakeError::Finalize( + "no config_contract fields resolved for this stack, so there is nothing to \ + sanitize and the image would keep the author's values. Usual causes: the \ + template is not approved yet, DATABASE_URL is not set, or the contract \ + declares no generated/provided fields. Pass --allow-unsanitized-snapshot \ + if the stack really has no secrets." + .to_string(), + )) +} + +/// Report a blind spot in the embedded-secret scan, if there is one. +/// +/// The scan finds a credential hidden inside a larger value (the DSN case) by +/// searching for the *values* the contract protects. With no values to search +/// for — no `.env` beside the compose — it cannot run, and a DSN would sail +/// through untouched. +/// +/// A project having no `.env` is perfectly normal, so this is not an error: a +/// reference the buyer's machine cannot fill is already caught separately, by +/// [`crate::cli::generator::compose::resolve_non_contract_references`]. This +/// only says out loud that one check did not happen, because a mistyped +/// `--project-dir` looks identical from here. +pub fn env_scan_warning( + env_values: &std::collections::BTreeMap, + protected: &BTreeSet, +) -> Option { + if protected.is_empty() || !env_values.is_empty() { + return None; + } + + Some( + "no values were found beside the compose file, so secrets embedded inside \ + larger values (a password inside DATABASE_URL, for example) could not be \ + searched for. If this stack does ship a .env, check --project-dir." + .to_string(), + ) +} + +/// Shell to strip everything host- or author-specific, so a clone boots as a +/// distinct machine that its author cannot reach. +/// +/// Two separate problems, both solved by removal: +/// +/// **Machine identity.** `sshd` regenerates host keys on first boot when none +/// are present and `systemd` repopulates an empty `/etc/machine-id`; clearing +/// cloud-init's instance state makes it treat the clone as a new instance and +/// re-run its per-instance modules. Left in place, every clone of the image +/// shares one host key, so any buyer can impersonate another buyer's server. +/// +/// **The author's own access.** Hetzner's cloud-init *appends* the buyer's key +/// to `authorized_keys` — it never truncates the file. A key left in the image +/// therefore grants its holder root on every server ever cloned from that +/// snapshot. The same applies to private keys, `known_hosts`, and to +/// `~/.docker/config.json`, which holds base64 registry credentials whenever the +/// build performed a `docker login`. +/// +/// `/etc/stacker/env` is deliberately absent from this list: cloud-init +/// overwrites it wholesale on the buyer's first boot (see `helpers::cloud_init`), +/// and the co-located `.env` is cleared separately by [`scrub_env_file`]. pub fn identity_reset_commands() -> Vec { vec![ + // Machine identity. "rm -f /etc/ssh/ssh_host_*".to_string(), ": > /etc/machine-id".to_string(), "rm -f /var/lib/dbus/machine-id".to_string(), "rm -rf /var/lib/cloud/instances /var/lib/cloud/instance".to_string(), + // The author's access — root and any other account on the box. + "rm -f /root/.ssh/authorized_keys /home/*/.ssh/authorized_keys".to_string(), + "rm -f /root/.ssh/id_* /home/*/.ssh/id_*".to_string(), + "rm -f /root/.ssh/known_hosts /home/*/.ssh/known_hosts".to_string(), + // Registry credentials left by a `docker login` during the build. + "rm -f /root/.docker/config.json /home/*/.docker/config.json".to_string(), + // Traces of the build itself. "rm -f /root/.bash_history /home/*/.bash_history".to_string(), "find /var/log -type f -exec truncate -s 0 {} + 2>/dev/null || true".to_string(), ] } -/// Shell to stop the stack and drop every data volume that is not explicitly -/// preserved, so the buyer's box initializes them from scratch with the -/// buyer's own generated values. +/// Shell to stop the stack and drop its credential-bearing data volumes, so the +/// buyer's box initializes them from scratch with the buyer's own values. /// -/// Volume names are matched by suffix because Compose prefixes them with the -/// project name (`project_stackpilot_pgdata` for a declared `stackpilot_pgdata`). -pub fn volume_reset_commands(project_dir: &str, keep: &[&str]) -> Vec { - let mut cmds = vec![format!( - "cd {project_dir} && docker compose down --remove-orphans" - )]; - - let filter = if keep.is_empty() { - "cat".to_string() +/// Scoped to **this project's declared volumes only**. A build box also runs +/// platform-managed services in their own Compose projects — the nginx-proxy-manager +/// ingress and the status-panel agent — whose volumes hold Let's Encrypt +/// certificates and agent state. Enumerating the host (`docker volume ls` with no +/// filter) would delete those, and would additionally abort the bake: `docker volume +/// rm` refuses a volume still held by a running container, and `-f` only suppresses +/// "no such volume", not "volume is in use". +/// +/// So the list comes from `docker compose config --volumes` (the names this stack +/// declares), and each is resolved to its real volume through Compose's own +/// `com.docker.compose.volume` label rather than by guessing the project prefix. +pub fn volume_reset_commands( + project_dir: &str, + keep: &[&str], +) -> Result, crate::helpers::bake::BakeError> { + if let Some(bad) = keep.iter().find(|name| !is_plain_volume_name(name)) { + return Err(crate::helpers::bake::BakeError::Finalize(format!( + "volume keep entry `{bad}` contains characters that are shell pattern syntax. \ + It would match something other than intended, and keeping a volume that should \ + have been reset leaves the author's credentials in the image. Use only letters, \ + digits, `_`, `-` and `.`." + ))); + } + + // Match whole `_`/`-` separated segments rather than a bare substring, so + // keeping `ollama` keeps `stackpilot_ollama` without also keeping + // `not-ollama-backup`. + let skip_kept = if keep.is_empty() { + String::new() } else { - let pattern = keep.join("|"); - format!("grep -Ev '({pattern})'") + let patterns = keep + .iter() + .flat_map(|name| { + [ + name.to_string(), + format!("*[-_]{name}"), + format!("{name}[-_]*"), + format!("*[-_]{name}[-_]*"), + ] + }) + .collect::>() + .join("|"); + format!("case \"$v\" in {patterns}) continue;; esac; ") }; - cmds.push(format!( - "docker volume ls -q | {filter} | xargs -r docker volume rm -f" - )); - cmds + Ok(vec![ + format!("cd {project_dir} && docker compose down --remove-orphans"), + format!( + "cd {project_dir} && for v in $(docker compose config --volumes); do \ + {skip_kept}docker volume ls -q \ + --filter label=com.docker.compose.volume=\"$v\" \ + | xargs -r docker volume rm -f; done" + ), + ]) +} + +/// A volume name safe to interpolate into a shell `case` pattern: no `*`, `?`, +/// `[`, `|`, `)` or anything else the shell would read as syntax. +fn is_plain_volume_name(name: &str) -> bool { + !name.is_empty() + && name + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-' || c == '.') } -/// Blank the values of secret-bearing keys in a `.env` file while keeping the -/// file's shape: keys, comments, blank lines and non-secret values survive. +/// Blank the author's secret values in a `.env` file while keeping the file's +/// shape: keys, comments, blank lines and non-secret values survive. +/// +/// **The contract is the only authority.** A value is blanked when its key is +/// declared `generated`/`provided` in `config_contract`, or when the value +/// *contains* such a secret — the DSN case, where the credential hides inside a +/// larger string under a name no policy mentions +/// (`DATABASE_URL=postgres://user:@host/db`). Nothing is guessed from +/// key names: name heuristics both miss real secrets and blank harmless values, +/// and the author already told us which fields are sensitive. /// /// The keys are kept (rather than the lines dropped) so the file still /// documents what the stack expects; on the buyer's box the whole file is /// replaced from `/etc/stacker/env` by the systemd unit's `ExecStartPre`, so /// the blanked values are never read. pub fn scrub_env_file(content: &str, protected: &BTreeSet) -> String { + // The literal values the contract protects — what a DSN may be hiding. + let secrets: Vec = parse_env_pairs(content) + .into_iter() + .filter(|(key, value)| protected.contains(key) && value.len() >= MIN_EMBEDDED_SECRET_LEN) + .map(|(_, value)| value) + .collect(); + let mut out = String::with_capacity(content.len()); for line in content.lines() { @@ -102,7 +275,10 @@ pub fn scrub_env_file(content: &str, protected: &BTreeSet) -> String { } match line.split_once('=') { - Some((key, _value)) if should_blank(key.trim(), protected) => { + Some((key, value)) + if protected.contains(key.trim()) + || secrets.iter().any(|secret| value.contains(secret.as_str())) => + { out.push_str(key); out.push_str("=\n"); } @@ -116,12 +292,9 @@ pub fn scrub_env_file(content: &str, protected: &BTreeSet) -> String { out } -/// A key is blanked when the author's contract declared it regenerable/buyer- -/// supplied, or when its name is secret-shaped by the same heuristic the CLI -/// already uses for `generate-secrets.sh`. -fn should_blank(key: &str, protected: &BTreeSet) -> bool { - protected.contains(key) || crate::console::commands::cli::init::is_secret_env_key(key) -} +/// Shortest value treated as a secret when searching *inside* another value. +/// Short strings collide with ordinary text and would blank harmless lines. +const MIN_EMBEDDED_SECRET_LEN: usize = 12; /// Everything the finalize step needs to reach and sanitize a build box. #[derive(Debug, Clone)] @@ -179,7 +352,9 @@ pub async fn finalize_build_box( .await .map_err(|e| e.to_string())?; if code != 0 { - return Err(format!("`{cmd}` exited {code}: {stderr}")); + // Never echo the command back whole: a file write carries the + // file's own contents as a base64 argument. + return Err(format!("`{}` exited {code}: {stderr}", summarize(&cmd))); } Ok::(stdout) } @@ -188,6 +363,10 @@ pub async fn finalize_build_box( let compose_path = format!("{}/docker-compose.yml", ctx.project_dir); let env_path = format!("{}/.env", ctx.project_dir); + // The steps are not transactional, so a failure has to be able to say what + // already happened — see `recovery_advice`. + let mut done: Vec = Vec::new(); + let result = async { // 1. Read what the deploy left on the box. let compose = run(format!("cat {compose_path}")) @@ -197,9 +376,37 @@ pub async fn finalize_build_box( let env_raw = run(format!("cat {env_path} 2>/dev/null || true")) .await .map_err(|e| fail("read .env", e))?; + done.push(FinalizeStage::Read); let env_values = parse_env_pairs(&env_raw); - // 2. Replace secrets embedded inside larger values (DSNs) with ${KEY} + let lost = env_file_values_lost_on_clone(&compose, &env_values, &ctx.protected_keys); + if !lost.is_empty() { + return Err(BakeError::Finalize(format!( + "this compose reads values through `env_file:`, and {} of them are not \ + contract fields ({}). The buyer's machine replaces that file wholesale \ + from /etc/stacker/env, which carries only contract fields and the buyer's \ + own values, so those would silently disappear at first boot. Move them \ + into an `environment:` block, where they become part of the image.", + lost.len(), + lost.join(", ") + ))); + } + if let Some(warning) = env_scan_warning(&env_values, &ctx.protected_keys) { + eprintln!("WARNING: {warning}"); + } + + // 2. Stop the stack and drop its credential-bearing data volumes, while + // the compose and .env on disk are still the ones Compose itself + // wrote. Doing this after the rewrites would hand `docker compose` + // a cleared .env, and any `${VAR}` outside an environment block + // (`image: ${REGISTRY}/app:${TAG}`) would then resolve empty and fail + // the teardown — with the files already modified and no way back. + for cmd in volume_reset_commands(&ctx.project_dir, volumes_to_keep(&ctx.stack))? { + run(cmd).await.map_err(|e| fail("volume reset", e))?; + } + done.push(FinalizeStage::Teardown); + + // 3. Replace secrets embedded inside larger values (DSNs) with ${KEY} // references. Whole-value keys were already parameterized at deploy // time by `parameterize_compose_env_vars`. let sanitized = crate::cli::generator::compose::parameterize_embedded_secret_values( @@ -209,17 +416,43 @@ pub async fn finalize_build_box( ) .map_err(|conflict| BakeError::Finalize(conflict.to_string()))?; + // 4. Put back the literal value of every reference the contract does not + // cover. Stage 4 in the CLI turns plain top-level `env:` keys into + // references too, and nothing fills those on the buyer's machine — + // the .env there is replaced wholesale from /etc/stacker/env, which + // only ever holds contract fields and the buyer's own values. + let (sanitized, unresolved) = + crate::cli::generator::compose::resolve_non_contract_references( + &sanitized, + &env_values, + &ctx.protected_keys, + ); + if !unresolved.is_empty() { + let names: Vec<&str> = unresolved.iter().map(|r| r.name.as_str()).collect(); + return Err(BakeError::Finalize(format!( + "the compose references {} environment variable(s) with no value on the \ + build box and no default ({}). They are not contract fields, so nothing \ + will fill them on a buyer's machine either — they would resolve to empty \ + strings at boot.", + names.len(), + names.join(", ") + ))); + } + if sanitized != compose { write_remote_file(&run, &compose_path, &sanitized) .await .map_err(|e| fail("write compose", e))?; + done.push(FinalizeStage::RewriteCompose); } - // 3. Record what the image now needs from the buyer's env file. + // 5. Record what the image needs from the buyer's env file — taken from + // the contract, not from the text of the file. A reference only counts + // as required when something is expected to supply it. let required_env_keys = - crate::cli::generator::compose::collect_env_var_references(&sanitized); + crate::cli::generator::compose::required_env_keys(&sanitized, &ctx.protected_keys); - // 4. Blank the author's secrets in the co-located .env. The buyer's box + // 6. Clear the author's secrets in the co-located .env. The buyer's box // overwrites this file wholesale from /etc/stacker/env at boot, so // the blanked values are never read. if !env_raw.trim().is_empty() { @@ -227,40 +460,166 @@ pub async fn finalize_build_box( write_remote_file(&run, &env_path, &scrubbed) .await .map_err(|e| fail("write .env", e))?; + done.push(FinalizeStage::ClearEnv); } - // 5. Drop initialized data volumes so the buyer's box re-initializes - // them with the buyer's own values. A secret the app persisted on - // first run is not governed by env vars any more. - for cmd in volume_reset_commands(&ctx.project_dir, volumes_to_keep(&ctx.stack)) { - run(cmd).await.map_err(|e| fail("volume reset", e))?; - } - - // 6. Strip machine identity last. + // 7. Strip machine identity last. for cmd in identity_reset_commands() { run(cmd).await.map_err(|e| fail("identity reset", e))?; } + done.push(FinalizeStage::StripIdentity); Ok(FinalizeOutcome { required_env_keys }) } .await; disconnect_ssh(session).await; - result + + result.map_err(|err| match err { + BakeError::Finalize(message) => { + BakeError::Finalize(format!("{message}\n\n{}", recovery_advice(&done))) + } + other => other, + }) +} + +/// A step of [`finalize_build_box`], in the order they run. +/// +/// Tracked so a failure can say what already happened: the steps are not +/// transactional and most of them are not reversible. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FinalizeStage { + /// Read the compose file and the co-located `.env`. + Read, + /// Stop the stack and drop its data volumes. + Teardown, + /// Write the sanitized compose back. + RewriteCompose, + /// Write the cleared `.env` back. + ClearEnv, + /// Remove machine identity and the author's access. + StripIdentity, +} + +impl FinalizeStage { + fn describe(self) -> &'static str { + match self { + FinalizeStage::Read => "read the compose file and .env", + FinalizeStage::Teardown => "stopped the stack and dropped its data volumes", + FinalizeStage::RewriteCompose => "rewrote the compose file", + FinalizeStage::ClearEnv => "cleared the .env", + FinalizeStage::StripIdentity => "stripped machine identity and the author's access", + } + } +} + +/// What the operator needs to know after a failed finalize. +/// +/// Re-running against the same box is rarely equivalent: the compose is already +/// sanitized so the embedded-secret scan has nothing left to find, the `.env` is +/// already cleared so there are no values to search for, and the data volumes +/// are gone. Saying this plainly is the difference between a lost afternoon and +/// a fresh build box. +pub fn recovery_advice(completed: &[FinalizeStage]) -> String { + let mut lines = Vec::new(); + + if completed.is_empty() { + lines.push("Nothing was changed on the build box.".to_string()); + } else { + lines.push("Completed before the failure:".to_string()); + for stage in completed { + lines.push(format!(" - {}", stage.describe())); + } + } + + let touched_files = completed + .iter() + .any(|s| matches!(s, FinalizeStage::RewriteCompose | FinalizeStage::ClearEnv)); + let torn_down = completed.contains(&FinalizeStage::Teardown); + let lost_access = completed.contains(&FinalizeStage::StripIdentity); + + if lost_access { + lines.push( + "The box no longer accepts the bake key, so it cannot be reconnected to.".to_string(), + ); + } + if torn_down { + lines.push( + "Its data volumes are gone and the stack is stopped, so it no longer \ + represents a working deployment." + .to_string(), + ); + } + if touched_files { + lines.push( + "Its compose and .env are already sanitized, so a retry would find nothing \ + left to search for and would report success over an unchecked image." + .to_string(), + ); + } + + if lost_access || torn_down || touched_files { + lines.push("Deploy a fresh build box and bake that instead.".to_string()); + } else { + lines.push("The bake can be retried against this box as-is.".to_string()); + } + + lines.join("\n") } -/// Write `content` to `path` on the remote box without any quoting hazards: -/// the payload travels base64-encoded and is decoded on the far side. +/// A command shortened for an error message. A file write carries the file's +/// own contents as a base64 argument, which must not end up in the bake log. +fn summarize(cmd: &str) -> String { + const MAX: usize = 120; + if cmd.len() <= MAX { + return cmd.to_string(); + } + let head: String = cmd.chars().take(MAX).collect(); + format!("{head}… ({} chars)", cmd.len()) +} + +/// Source bytes per chunk. Kept a multiple of 3 so every chunk is a whole +/// number of base64 groups and decodes on its own; 48 KiB of source becomes +/// 64 KiB of base64, comfortably inside Linux's 128 KiB limit on a single +/// command-line argument (`MAX_ARG_STRLEN`). +const WRITE_CHUNK_BYTES: usize = 48 * 1024; + +/// The shell to write `content` to `path`, split so no single command exceeds +/// the argument-length limit. +/// +/// The payload travels base64-encoded, which sidesteps every quoting hazard — +/// a compose file is full of `$`, quotes and newlines. The first command +/// truncates, the rest append. +pub fn write_file_commands(path: &str, content: &str) -> Vec { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + + let bytes = content.as_bytes(); + if bytes.is_empty() { + // Still create (or truncate) the file. + return vec![format!(": > {path}")]; + } + + bytes + .chunks(WRITE_CHUNK_BYTES) + .enumerate() + .map(|(index, chunk)| { + let redirect = if index == 0 { ">" } else { ">>" }; + let encoded = STANDARD.encode(chunk); + format!("printf %s {encoded} | base64 -d {redirect} {path}") + }) + .collect() +} + +/// Write `content` to `path` on the remote box. async fn write_remote_file(run: &F, path: &str, content: &str) -> Result<(), String> where F: Fn(String) -> Fut, Fut: std::future::Future>, { - use base64::{engine::general_purpose::STANDARD, Engine as _}; - let encoded = STANDARD.encode(content.as_bytes()); - run(format!("printf %s {encoded} | base64 -d > {path}")) - .await - .map(|_| ()) + for cmd in write_file_commands(path, content) { + run(cmd).await?; + } + Ok(()) } /// Parse `KEY=value` lines into a map, skipping comments and blanks. @@ -323,6 +682,264 @@ mod tests { keys.iter().map(|k| k.to_string()).collect() } + /// L1 — a keep entry is interpolated into a shell `case` pattern, where + /// `|`, `)`, `*`, `?` and `[` are all syntax. An entry carrying one of them + /// would silently match something else, or break the command outright. + /// Keeping a volume that should have been reset leaks the author's + /// credentials, so this fails rather than guesses. + #[test] + fn a_keep_entry_with_shell_syntax_is_refused() { + for bad in ["oll*ama", "a|b", "x)y", "a[bc]", "back`tick`", "semi;colon"] { + assert!( + volume_reset_commands("/home/trydirect/project", &[bad]).is_err(), + "`{bad}` must not reach a shell pattern" + ); + } + } + + #[test] + fn ordinary_volume_names_are_accepted() { + for good in ["ollama", "stackpilot_ollama", "model-cache", "data1"] { + assert!( + volume_reset_commands("/home/trydirect/project", &[good]).is_ok(), + "`{good}` should be usable" + ); + } + } + + /// Matching is on whole path segments, so `ollama` keeps + /// `stackpilot_ollama` without also keeping `not-ollama-backup`. + #[test] + fn a_keep_entry_does_not_match_a_longer_unrelated_name() { + let cmds = volume_reset_commands("/home/trydirect/project", &["ollama"]) + .expect("valid") + .join(" ; "); + assert!( + !cmds.contains("*ollama*"), + "a bare substring match would also keep `not-ollama-backup`: {cmds}" + ); + } + + /// H3 — a service reading its values through `env_file:` takes them from the + /// file, not through `${VAR}`. On the buyer's machine that file is replaced + /// wholesale from `/etc/stacker/env`, which only carries contract fields and + /// the buyer's own values, so everything else in it simply disappears. + #[test] + fn env_file_values_outside_the_contract_are_reported_as_lost() { + let compose = "services:\n app:\n env_file:\n - .env\n"; + let env: std::collections::BTreeMap = [ + ("SECRET_KEY".to_string(), "value".to_string()), + ("OLLAMA_MODEL".to_string(), "llama3.1".to_string()), + ] + .into_iter() + .collect(); + + let lost = + env_file_values_lost_on_clone(compose, &env, &protected(["SECRET_KEY"].as_slice())); + + assert_eq!( + lost, + vec!["OLLAMA_MODEL".to_string()], + "contract fields survive; anything else does not" + ); + } + + #[test] + fn a_compose_without_env_file_loses_nothing() { + let compose = "services:\n app:\n environment:\n A: b\n"; + let env: std::collections::BTreeMap = + [("OLLAMA_MODEL".to_string(), "llama3.1".to_string())] + .into_iter() + .collect(); + + assert!(env_file_values_lost_on_clone(compose, &env, &BTreeSet::new()).is_empty()); + } + + #[test] + fn env_file_carrying_only_contract_fields_is_fine() { + let compose = "services:\n app:\n env_file: .env\n"; + let env: std::collections::BTreeMap = + [("SECRET_KEY".to_string(), "value".to_string())] + .into_iter() + .collect(); + + assert!(env_file_values_lost_on_clone( + compose, + &env, + &protected(["SECRET_KEY"].as_slice()) + ) + .is_empty()); + } + + /// H5 — the steps are not transactional. When one fails, the operator has + /// to be told what already happened, because most of it is not reversible + /// and a retry against the same box is not equivalent. + #[test] + fn nothing_done_means_the_bake_can_be_retried() { + let advice = recovery_advice(&[FinalizeStage::Read]); + assert!( + advice.contains("can be retried"), + "a read-only failure leaves the box usable: {advice}" + ); + assert!(!advice.contains("fresh build box"), "no need: {advice}"); + } + + #[test] + fn modified_files_require_a_fresh_build_box() { + let advice = recovery_advice(&[FinalizeStage::Read, FinalizeStage::RewriteCompose]); + assert!(advice.contains("fresh build box"), "{advice}"); + assert!( + advice.contains("already sanitized") || advice.contains("already-sanitized"), + "the reason a retry is not equivalent should be stated: {advice}" + ); + } + + #[test] + fn a_completed_teardown_is_called_out_as_destructive() { + let advice = recovery_advice(&[FinalizeStage::Read, FinalizeStage::Teardown]); + assert!(advice.contains("data volumes"), "{advice}"); + assert!(advice.contains("fresh build box"), "{advice}"); + } + + #[test] + fn a_completed_identity_reset_means_no_way_back_in() { + let advice = recovery_advice(&[ + FinalizeStage::Read, + FinalizeStage::Teardown, + FinalizeStage::StripIdentity, + ]); + assert!( + advice.contains("no longer accepts"), + "losing SSH access must be stated plainly: {advice}" + ); + } + + #[test] + fn the_advice_lists_what_completed() { + let advice = recovery_advice(&[FinalizeStage::Read, FinalizeStage::Teardown]); + assert!(advice.contains("read"), "{advice}"); + assert!(advice.contains("stopped the stack"), "{advice}"); + } + + /// M4 — the payload travels as a shell command, and Linux caps a single + /// argument at 128 KiB. A large compose must therefore be written in pieces + /// rather than aborting the sanitize half-way with "Argument list too long". + #[test] + fn a_small_file_is_written_in_one_command() { + let cmds = write_file_commands("/tmp/x", "hello"); + assert_eq!(cmds.len(), 1); + assert!( + cmds[0].contains("> /tmp/x"), + "truncating write: {}", + cmds[0] + ); + assert!(!cmds[0].contains(">> /tmp/x"), "not appending: {}", cmds[0]); + } + + #[test] + fn a_large_file_is_written_in_appended_chunks() { + let big = "x".repeat(300_000); + let cmds = write_file_commands("/tmp/x", &big); + + assert!(cmds.len() > 1, "expected chunking, got {}", cmds.len()); + assert!(cmds[0].contains("> /tmp/x") && !cmds[0].contains(">> /tmp/x")); + for cmd in &cmds[1..] { + assert!(cmd.contains(">> /tmp/x"), "chunk must append: {cmd}"); + } + } + + #[test] + fn every_chunk_stays_under_the_argument_limit() { + let big = "y".repeat(500_000); + for cmd in write_file_commands("/tmp/x", &big) { + assert!( + cmd.len() < 128 * 1024, + "a single command must fit in one argument, got {}", + cmd.len() + ); + } + } + + /// Each chunk has to decode on its own, so the split must fall on a 3-byte + /// boundary — otherwise base64 padding corrupts the seams. + #[test] + fn chunks_round_trip_to_the_original_content() { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + + let original: String = (0..200_000) + .map(|i| ((i % 26) as u8 + b'a') as char) + .collect(); + let mut rebuilt = Vec::new(); + + for cmd in write_file_commands("/tmp/x", &original) { + let encoded = cmd.split_whitespace().nth(2).expect("printf %s "); + rebuilt.extend(STANDARD.decode(encoded).expect("each chunk decodes alone")); + } + + assert_eq!(String::from_utf8(rebuilt).unwrap(), original); + } + + #[test] + fn an_empty_file_still_produces_a_write() { + let cmds = write_file_commands("/tmp/x", ""); + assert_eq!(cmds.len(), 1, "the file must still be created/truncated"); + } + + /// H6 — a contract that did not resolve must stop the bake, not produce a + /// confident "Sanitized" line over an image that still carries the author's + /// credentials. + #[test] + fn empty_contract_refuses_the_bake() { + let err = check_contract_usable(&BTreeSet::new(), false) + .expect_err("an empty contract cannot sanitize anything"); + let message = err.to_string(); + assert!( + message.contains("approved"), + "the message should name the usual cause: {message}" + ); + } + + #[test] + fn empty_contract_is_allowed_only_deliberately() { + assert!(check_contract_usable(&BTreeSet::new(), true).is_ok()); + } + + #[test] + fn a_contract_with_fields_passes() { + assert!(check_contract_usable(&protected(["SECRET_KEY"].as_slice()), false).is_ok()); + } + + /// M2 — a project may legitimately ship no `.env`, so its absence is not an + /// error. What it does mean is that the embedded-secret scan has nothing to + /// search for, which is a blind spot worth saying out loud — a wrong + /// `--project-dir` looks exactly the same from here. + #[test] + fn missing_env_file_warns_when_secrets_are_declared() { + let warning = env_scan_warning( + &std::collections::BTreeMap::new(), + &protected(["SECRET_KEY"].as_slice()), + ) + .expect("a blind spot should be reported"); + assert!( + warning.contains("--project-dir"), + "the warning should name the likely cause: {warning}" + ); + } + + #[test] + fn missing_env_file_is_silent_when_nothing_is_declared() { + assert!(env_scan_warning(&std::collections::BTreeMap::new(), &BTreeSet::new()).is_none()); + } + + #[test] + fn present_env_values_warn_about_nothing() { + let env: std::collections::BTreeMap = + [("SECRET_KEY".to_string(), "value".to_string())] + .into_iter() + .collect(); + assert!(env_scan_warning(&env, &protected(["SECRET_KEY"].as_slice())).is_none()); + } + #[test] fn scrub_blanks_contract_declared_keys() { let env = "SECRET_KEY=b838f1f2\nOLLAMA_MODEL=llama3.1\n"; @@ -334,19 +951,56 @@ mod tests { ); } + /// Without a contract nothing is a declared secret, so nothing is blanked. + /// The contract is the only authority — no name guessing. #[test] - fn scrub_blanks_secret_shaped_keys_without_a_contract() { - // No contract at all — the name heuristic still has to catch these. - let env = "DB_PASSWORD=2213a996\nADMIN_USER=admin\n"; + fn scrub_without_a_contract_blanks_nothing() { + let env = "DB_PASSWORD=0123456789abcdef0123456789abcdef\nADMIN_USER=admin\n"; let out = scrub_env_file(env, &BTreeSet::new()); - assert!(out.contains("DB_PASSWORD=\n"), "blanked:\n{out}"); - assert!(out.contains("ADMIN_USER=admin"), "non-secret kept:\n{out}"); + assert_eq!(out, env); + } + + /// The DSN case: the credential hides inside a value whose own key is not + /// declared anywhere. + #[test] + fn scrub_blanks_a_value_that_embeds_a_declared_secret() { + let env = "POSTGRES_PASSWORD=0123456789abcdef0123456789abcdef\n\ + DATABASE_URL=postgresql://stackpilot:0123456789abcdef0123456789abcdef@db/s\n\ + REDIS_URL=redis://stackpilot-redis:6379\n"; + let out = scrub_env_file(env, &protected(["POSTGRES_PASSWORD"].as_slice())); + + assert!(out.contains("POSTGRES_PASSWORD=\n"), "declared key:\n{out}"); + assert!(out.contains("DATABASE_URL=\n"), "embedded secret:\n{out}"); + assert!( + !out.contains("0123456789abcdef0123456789abcdef"), + "no literal anywhere:\n{out}" + ); + assert!( + out.contains("REDIS_URL=redis://stackpilot-redis:6379"), + "credential-free URL kept:\n{out}" + ); + } + + /// A short declared value must not blank unrelated lines that happen to + /// contain it as a substring. + #[test] + fn scrub_ignores_short_declared_values_when_scanning_inside_others() { + let env = "PORT_TOKEN=8080\nPUBLIC_URL=http://host:8080/app\n"; + let out = scrub_env_file(env, &protected(["PORT_TOKEN"].as_slice())); + assert!( + out.contains("PORT_TOKEN=\n"), + "declared key still blanked:\n{out}" + ); + assert!( + out.contains("PUBLIC_URL=http://host:8080/app"), + "short value must not match inside others:\n{out}" + ); } #[test] fn scrub_preserves_comments_and_blank_lines() { let env = "# Secrets\n\nSECRET_KEY=abc\n"; - let out = scrub_env_file(env, &BTreeSet::new()); + let out = scrub_env_file(env, &protected(["SECRET_KEY"].as_slice())); assert_eq!(out, "# Secrets\n\nSECRET_KEY=\n"); } @@ -354,11 +1008,55 @@ mod tests { fn scrub_keeps_values_containing_equals_signs() { // A blanked key must not be confused by '=' inside a kept value. let env = "OLLAMA_MODEL=llama3.1\nJWT_SECRET=a=b=c\n"; - let out = scrub_env_file(env, &BTreeSet::new()); + let out = scrub_env_file(env, &protected(["JWT_SECRET"].as_slice())); assert!(out.contains("OLLAMA_MODEL=llama3.1"), "kept:\n{out}"); assert!(out.contains("JWT_SECRET=\n"), "blanked:\n{out}"); } + /// The author's own access must not survive into a buyer's server. + /// + /// Hetzner's cloud-init *appends* the buyer's key to `authorized_keys`; it + /// never truncates the file. An author key left in the image therefore + /// grants its holder root on every server ever cloned from that snapshot — + /// the same class of defect as the shared host keys, and a worse one. + #[test] + fn identity_reset_removes_operator_access_to_the_image() { + let cmds = identity_reset_commands().join(" ; "); + assert!( + cmds.contains("/root/.ssh/authorized_keys"), + "the author's SSH access must not be baked in: {cmds}" + ); + assert!( + cmds.contains("/home/*/.ssh/authorized_keys"), + "non-root accounts carry authorized_keys too: {cmds}" + ); + } + + /// A registry login performed during the build leaves base64 credentials in + /// `~/.docker/config.json`, which the snapshot would hand to every buyer. + #[test] + fn identity_reset_removes_registry_credentials() { + let cmds = identity_reset_commands().join(" ; "); + assert!( + cmds.contains("/root/.docker/config.json"), + "registry credentials must not be baked in: {cmds}" + ); + } + + /// Private keys and the operator's known_hosts are equally author-specific. + #[test] + fn identity_reset_removes_operator_private_keys() { + let cmds = identity_reset_commands().join(" ; "); + assert!( + cmds.contains("/root/.ssh/id_"), + "operator private keys must not be baked in: {cmds}" + ); + assert!( + cmds.contains("known_hosts"), + "known_hosts is author-specific: {cmds}" + ); + } + #[test] fn identity_reset_covers_host_keys_machine_id_and_cloud_init() { let cmds = identity_reset_commands().join(" ; "); @@ -372,21 +1070,49 @@ mod tests { #[test] fn volume_reset_preserves_the_kept_volumes() { - let cmds = volume_reset_commands("/home/trydirect/project", &["ollama"]).join(" ; "); + let cmds = volume_reset_commands("/home/trydirect/project", &["ollama"]) + .expect("valid keep list") + .join(" ; "); assert!( cmds.contains("docker compose down"), "stack stopped: {cmds}" ); assert!( - cmds.contains("grep -Ev '(ollama)'"), - "kept volume excluded from removal: {cmds}" + cmds.contains(r#"case "$v" in ollama|*[-_]ollama|"#), + "kept volume skipped: {cmds}" ); } + /// Regression: enumerating the host would delete the nginx-proxy-manager + /// ingress' certificates and the agent's state, and would abort the bake on + /// the first volume still held by a running container. + #[test] + fn volume_reset_never_enumerates_the_whole_host() { + for keep in [&[][..], &["ollama"][..]] { + let cmds = volume_reset_commands("/home/trydirect/project", keep) + .expect("valid keep list") + .join(" ; "); + assert!( + !cmds.contains("docker volume ls -q |"), + "must not pipe an unfiltered host-wide listing: {cmds}" + ); + assert!( + cmds.contains("docker compose config --volumes"), + "volume list must come from the project's compose: {cmds}" + ); + assert!( + cmds.contains("--filter label=com.docker.compose.volume="), + "removal must be scoped by compose's own label: {cmds}" + ); + } + } + #[test] - fn volume_reset_without_a_keep_list_removes_everything() { - let cmds = volume_reset_commands("/home/trydirect/project", &[]).join(" ; "); - assert!(cmds.contains("| cat |"), "no filter applied: {cmds}"); + fn volume_reset_without_a_keep_list_skips_nothing() { + let cmds = volume_reset_commands("/home/trydirect/project", &[]) + .expect("valid keep list") + .join(" ; "); + assert!(!cmds.contains("case "), "no skip clause: {cmds}"); } #[test] diff --git a/src/helpers/redact.rs b/src/helpers/redact.rs index ebf9656d..5ba1f349 100644 --- a/src/helpers/redact.rs +++ b/src/helpers/redact.rs @@ -151,6 +151,11 @@ pub fn redact_yaml_string(yaml: &str) -> String { use std::collections::BTreeSet; +/// The author-declared field policy block. Its leaf values describe *how* a +/// field is produced (`mutability`/`type`/`length`), never a secret value, so +/// value-stripping must skip this subtree entirely. +const CONFIG_CONTRACT_KEY: &str = "config_contract"; + /// Replace, in place, the values of env entries whose key is in `keys`. /// Handles the same shapes as [`redact_sensitive_json_values`] plus `KEY=value` /// strings in environment arrays. @@ -173,6 +178,14 @@ pub fn strip_json_values_for_keys( } } for (key, val) in map.iter_mut() { + // The contract declares the *policy* for a field, not its value: + // inside it, a key named e.g. SECRET_KEY maps to + // {mutability, type, length}, which is not a secret and must + // survive. Blanking it there destroys the very policy that + // drives per-buyer regeneration. + if key == CONFIG_CONTRACT_KEY { + continue; + } if keys.contains(key) && !val.is_null() { *val = serde_json::Value::String(replacement.to_string()); } else { @@ -206,6 +219,11 @@ fn strip_yaml_values_for_keys( serde_yaml::Value::Mapping(map) => { for (key, val) in map.iter_mut() { if let serde_yaml::Value::String(k) = key { + // See the note in `strip_json_values_for_keys`: field + // policies are not secrets and must not be blanked. + if k == CONFIG_CONTRACT_KEY { + continue; + } if keys.contains(k) && !val.is_null() { *val = serde_yaml::Value::String(replacement.to_string()); continue; @@ -251,8 +269,12 @@ pub fn strip_yaml_string_for_keys( #[cfg(test)] mod tests { - use super::{is_sensitive_env_key, redact_sensitive_json_values, redact_yaml_string}; + use super::{ + is_sensitive_env_key, redact_sensitive_json_values, redact_yaml_string, + strip_json_values_for_keys, strip_yaml_string_for_keys, + }; use serde_json::json; + use std::collections::BTreeSet; // JSON tests @@ -407,4 +429,76 @@ mod tests { // Either returned as-is (parse failed) or survived round-trip without panicking assert!(!result.contains("PANIC")); } + + // config_contract must survive value-stripping + + fn generated_keys() -> BTreeSet { + ["SECRET_KEY".to_string(), "POSTGRES_PASSWORD".to_string()] + .into_iter() + .collect() + } + + #[test] + fn strip_json_blanks_values_but_not_contract_policies() { + let mut v = json!({ + "app": { "environment": { "SECRET_KEY": "b838f1f2" } }, + "config_contract": { + "services": { + "app": { + "fields": { + "SECRET_KEY": { + "mutability": "generated", + "type": "alphanumeric", + "length": 32 + } + } + } + } + } + }); + + strip_json_values_for_keys(&mut v, &generated_keys(), ""); + + assert_eq!(v["app"]["environment"]["SECRET_KEY"], "", "secret blanked"); + assert_eq!( + v["config_contract"]["services"]["app"]["fields"]["SECRET_KEY"]["mutability"], + "generated", + "the policy that drives regeneration must survive" + ); + assert_eq!( + v["config_contract"]["services"]["app"]["fields"]["SECRET_KEY"]["length"], + 32 + ); + } + + #[test] + fn strip_yaml_blanks_values_but_not_contract_policies() { + let yaml = "\ +app: + environment: + POSTGRES_PASSWORD: author-value +config_contract: + services: + stackpilot-db: + fields: + POSTGRES_PASSWORD: + mutability: generated + type: alphanumeric +"; + let out = strip_yaml_string_for_keys(yaml, &generated_keys(), ""); + let parsed: serde_yaml::Value = serde_yaml::from_str(&out).unwrap(); + + assert_eq!( + parsed["app"]["environment"]["POSTGRES_PASSWORD"].as_str(), + Some(""), + "secret blanked:\n{out}" + ); + assert_eq!( + parsed["config_contract"]["services"]["stackpilot-db"]["fields"]["POSTGRES_PASSWORD"] + ["mutability"] + .as_str(), + Some("generated"), + "policy survives:\n{out}" + ); + } } diff --git a/tests/features/bake_sanitization.feature b/tests/features/bake_sanitization.feature new file mode 100644 index 00000000..d53b3830 --- /dev/null +++ b/tests/features/bake_sanitization.feature @@ -0,0 +1,331 @@ +Feature: Bake-time sanitization of a build box + A marketplace snapshot is taken from the author's own build box, so the + author's secrets must not survive into the image the buyer clones. + The author-declared config_contract is the only authority on what is + sensitive — nothing is guessed from variable names. + + Rule: the .env co-located with the compose file is scrubbed + + Scenario: A contract-declared field is blanked + Given the contract declares "POSTGRES_PASSWORD" as generated + And the build box .env contains: + """ + POSTGRES_PASSWORD=0123456789abcdef0123456789abcdef + OLLAMA_MODEL=llama3.1 + """ + When the .env is scrubbed for the snapshot + Then the scrubbed .env has "POSTGRES_PASSWORD" blanked + And the scrubbed .env still has "OLLAMA_MODEL" set to "llama3.1" + + Scenario: A DSN embedding a declared secret is blanked + Given the contract declares "POSTGRES_PASSWORD" as generated + And the build box .env contains: + """ + POSTGRES_PASSWORD=0123456789abcdef0123456789abcdef + DATABASE_URL=postgresql://stackpilot:0123456789abcdef0123456789abcdef@db:5432/s + REDIS_URL=redis://stackpilot-redis:6379 + """ + When the .env is scrubbed for the snapshot + Then the scrubbed .env has "DATABASE_URL" blanked + And the scrubbed .env still has "REDIS_URL" set to "redis://stackpilot-redis:6379" + And the scrubbed .env contains no occurrence of "0123456789abcdef0123456789abcdef" + + Scenario: Without a contract nothing is treated as secret + Given the contract declares nothing + And the build box .env contains: + """ + DB_PASSWORD=0123456789abcdef0123456789abcdef + ADMIN_USER=admin + """ + When the .env is scrubbed for the snapshot + Then the scrubbed .env is unchanged + + Scenario: A short declared value does not blank unrelated lines + Given the contract declares "PORT_TOKEN" as generated + And the build box .env contains: + """ + PORT_TOKEN=8080 + PUBLIC_URL=http://host:8080/app + """ + When the .env is scrubbed for the snapshot + Then the scrubbed .env has "PORT_TOKEN" blanked + And the scrubbed .env still has "PUBLIC_URL" set to "http://host:8080/app" + + Rule: secrets embedded inside compose values become ${VAR} references + + Scenario: The password inside a DSN is parameterized + Given the contract declares "POSTGRES_PASSWORD" as generated + And "POSTGRES_PASSWORD" on the build box resolves to "0123456789abcdef0123456789abcdef" + And the generated compose is: + """ + services: + app: + environment: + DATABASE_URL: postgresql://stackpilot:0123456789abcdef0123456789abcdef@db:5432/s + """ + When the compose is sanitized for the snapshot + Then the sanitized compose contains "DATABASE_URL: postgresql://stackpilot:${POSTGRES_PASSWORD}@db:5432/s" + And the sanitized compose contains no occurrence of "0123456789abcdef0123456789abcdef" + + Scenario: One secret declared under two protected names aborts the bake + Given the contract declares "DB_PASSWORD" as generated + And the contract declares "POSTGRES_PASSWORD" as generated + And "DB_PASSWORD" on the build box resolves to "0123456789abcdef0123456789abcdef" + And the generated compose is: + """ + services: + db: + environment: + POSTGRES_PASSWORD: 0123456789abcdef0123456789abcdef + """ + When the compose is sanitized for the snapshot + Then sanitizing fails naming both "DB_PASSWORD" and "POSTGRES_PASSWORD" + + Scenario: A value nobody declared is left alone + Given the contract declares nothing + And the generated compose is: + """ + services: + app: + environment: + PUBLIC_URL: http://host/aaaaaaaaaaaaaaaa + """ + When the compose is sanitized for the snapshot + Then the sanitized compose contains "http://host/aaaaaaaaaaaaaaaa" + + Rule: only the project's own volumes are reset + + Scenario: A keep entry carrying shell syntax is refused + Given the stack keeps the volume matching "oll*ama" + When the volume reset commands are built and may fail + Then building the commands is refused + + Scenario: A keep entry matches whole segments, not any substring + Given the stack keeps the volume matching "ollama" + When the volume reset commands are built for "/home/trydirect/project" + Then the commands do not keep every name containing "ollama" + + Scenario: Volume removal never enumerates the whole host + Given the stack keeps the volume matching "ollama" + When the volume reset commands are built for "/home/trydirect/project" + Then the commands list volumes from the project compose + And the commands scope removal by the compose volume label + And the commands never list every volume on the host + And the commands skip the volume matching "ollama" + + Rule: the image keeps a reference only when something will fill it + + Scenario: A variable outside the contract goes back to its value + Given the contract declares nothing + And "REGION" on the build box resolves to "fsn1" + And the generated compose is: + """ + services: + app: + environment: + REGION: ${REGION} + """ + When references outside the contract are resolved + Then the resolved compose contains "REGION: fsn1" + And no reference is reported as unfillable + + Scenario: A contract field stays a reference + Given the contract declares "POSTGRES_PASSWORD" as generated + And "POSTGRES_PASSWORD" on the build box resolves to "aaaaaaaaaaaaaaaa" + And the generated compose is: + """ + services: + db: + environment: + POSTGRES_PASSWORD: ${POSTGRES_PASSWORD} + """ + When references outside the contract are resolved + Then the resolved compose contains "POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}" + And the resolved compose contains no occurrence of "aaaaaaaaaaaaaaaa" + + Scenario: Defaults and escaped text need no source + Given the contract declares nothing + And the generated compose is: + """ + services: + app: + environment: + LOG: ${LOG_LEVEL:-info} + CMD: echo $${HOME} + """ + When references outside the contract are resolved + Then the resolved compose contains "${LOG_LEVEL:-info}" + And the resolved compose contains "$${HOME}" + And no reference is reported as unfillable + + Scenario: A reference with no value and no default is reported + Given the contract declares nothing + And the generated compose is: + """ + services: + app: + environment: + TOKEN: ${MISSING} + """ + When references outside the contract are resolved + Then "MISSING" is reported as unfillable + + Scenario: The required list is drawn from the contract, not the file text + Given the contract declares "SECRET_KEY" as generated + And the generated compose is: + """ + services: + app: + environment: + SECRET_KEY: ${SECRET_KEY} + LOG: ${LOG_LEVEL:-info} + CMD: echo $${HOME} + REGION: ${REGION} + """ + When the required environment keys are collected + Then the required keys are exactly "SECRET_KEY" + + Rule: the image carries no access belonging to the author + + Scenario: The author's SSH access is removed + When the identity reset commands are built + Then the commands remove "/root/.ssh/authorized_keys" + And the commands remove "/home/*/.ssh/authorized_keys" + + Scenario: The author's private keys and known hosts are removed + When the identity reset commands are built + Then the commands remove "/root/.ssh/id_" + And the commands remove "known_hosts" + + Scenario: Registry credentials are removed + When the identity reset commands are built + Then the commands remove "/root/.docker/config.json" + + Scenario: Machine identity is still stripped + When the identity reset commands are built + Then the commands remove "/etc/ssh/ssh_host_*" + And the commands remove "/etc/machine-id" + And the commands remove "/var/lib/cloud/instance" + + Rule: a bake that cannot sanitize refuses to publish + + Scenario: An unresolved contract stops the bake + Given the contract declares nothing + When the bake checks whether it can sanitize + Then the bake is refused + And the refusal mentions "approved" + + Scenario: An unresolved contract may be overridden deliberately + Given the contract declares nothing + And unsanitized snapshots are explicitly allowed + When the bake checks whether it can sanitize + Then the bake is allowed + + Scenario: A resolved contract lets the bake proceed + Given the contract declares "SECRET_KEY" as generated + When the bake checks whether it can sanitize + Then the bake is allowed + + Scenario: A missing env file is reported but does not stop the bake + Given the contract declares "SECRET_KEY" as generated + And the build box has no .env beside the compose + When the bake checks the values it has to work from + Then the bake is allowed + And a warning mentions "--project-dir" + + Scenario: A project without an env file draws no warning + Given the contract declares nothing + And the build box has no .env beside the compose + When the bake checks the values it has to work from + Then the bake is allowed + And no warning is raised + + Rule: a large file is written without exceeding the command-length limit + + Scenario: A small file is written in one command + When a file of 500 bytes is written to the build box + Then it takes 1 command + And the first command truncates the file + + Scenario: A large compose is written in appended chunks + When a file of 300000 bytes is written to the build box + Then it takes more than one command + And the first command truncates the file + And every later command appends + And every command fits in a single argument + + Scenario: The chunks reassemble into the original file + When a file of 200000 bytes is written to the build box + Then decoding the chunks in order yields the original content + + Rule: a failed finalize says what it already did + + Scenario: A failure before anything changed leaves the box usable + Given the finalize completed "read" + When the recovery advice is produced + Then the advice says the bake can be retried + And the advice does not ask for a fresh build box + + Scenario: A failure after the tear-down is destructive + Given the finalize completed "read, teardown" + When the recovery advice is produced + Then the advice mentions "data volumes" + And the advice asks for a fresh build box + + Scenario: A failure after the files were rewritten cannot be retried in place + Given the finalize completed "read, teardown, rewrite compose" + When the recovery advice is produced + Then the advice mentions "already sanitized" + And the advice asks for a fresh build box + + Scenario: A failure after the identity reset locks the operator out + Given the finalize completed "read, teardown, strip identity" + When the recovery advice is produced + Then the advice mentions "no longer accepts" + And the advice asks for a fresh build box + + Rule: values a buyer would silently lose stop the bake + + Scenario: A service reading through env_file loses its non-contract values + Given the contract declares "SECRET_KEY" as generated + And "SECRET_KEY" on the build box resolves to "aaaaaaaaaaaaaaaa" + And "OLLAMA_MODEL" on the build box resolves to "llama3.1" + And the generated compose is: + """ + services: + app: + env_file: + - .env + """ + When the bake checks what a clone would lose + Then "OLLAMA_MODEL" is reported as lost + And "SECRET_KEY" is not reported as lost + + Scenario: A compose without env_file loses nothing + Given the contract declares nothing + And "OLLAMA_MODEL" on the build box resolves to "llama3.1" + And the generated compose is: + """ + services: + app: + environment: + OLLAMA_MODEL: llama3.1 + """ + When the bake checks what a clone would lose + Then nothing is reported as lost + + Rule: both spellings of an environment block are covered + + Scenario: The list form is parameterized too + Given "POSTGRES_PASSWORD" is a protected compose key + And the generated compose is: + """ + services: + db: + environment: + - POSTGRES_PASSWORD=aaaaaaaaaaaaaaaa + - POSTGRES_USER=stackpilot + """ + When whole-value keys are parameterized + Then the parameterized compose contains "- POSTGRES_PASSWORD=${POSTGRES_PASSWORD}" + And the parameterized compose contains "- POSTGRES_USER=stackpilot" diff --git a/tests/steps/bake_sanitization.rs b/tests/steps/bake_sanitization.rs new file mode 100644 index 00000000..9be0705c --- /dev/null +++ b/tests/steps/bake_sanitization.rs @@ -0,0 +1,603 @@ +//! Steps for `tests/features/bake_sanitization.feature`. +//! +//! These exercise the pure policy functions directly — no HTTP, no database. +//! The SSH orchestration around them (`finalize_build_box`) needs a real build +//! box and is out of scope here; what is covered is the part that decides *what* +//! gets removed, which is where a mistake destroys an image or leaks a secret. + +use cucumber::{given, then, when}; +use std::collections::{BTreeMap, BTreeSet}; + +use stacker::cli::generator::compose::parameterize_embedded_secret_values; +use stacker::helpers::bake_finalize::{scrub_env_file, volume_reset_commands}; + +use super::StepWorld; + +// ─── Given ─────────────────────────────────────────────────────── + +#[given(regex = r#"^the contract declares "([^"]*)" as generated$"#)] +async fn given_contract_declares(world: &mut StepWorld, key: String) { + world.bake.protected.insert(key); +} + +#[given(regex = r#"^the contract declares nothing$"#)] +async fn given_contract_empty(world: &mut StepWorld) { + world.bake.protected.clear(); +} + +#[given(regex = r#"^the build box \.env contains:$"#)] +async fn given_env_contents(world: &mut StepWorld, step: &cucumber::gherkin::Step) { + let body = step.docstring().cloned().unwrap_or_default(); + world.bake.env_content = format!("{}\n", body.trim_matches('\n')); +} + +#[given(regex = r#"^"([^"]*)" on the build box resolves to "([^"]*)"$"#)] +async fn given_env_value(world: &mut StepWorld, key: String, value: String) { + world.bake.env_values.insert(key, value); +} + +#[given(regex = r#"^the generated compose is:$"#)] +async fn given_compose(world: &mut StepWorld, step: &cucumber::gherkin::Step) { + let body = step.docstring().cloned().unwrap_or_default(); + world.bake.compose = format!("{}\n", body.trim_matches('\n')); +} + +#[given(regex = r#"^the stack keeps the volume matching "([^"]*)"$"#)] +async fn given_keep_volume(world: &mut StepWorld, name: String) { + world.bake.keep_volumes.push(name); +} + +// ─── When ──────────────────────────────────────────────────────── + +#[when(regex = r#"^the \.env is scrubbed for the snapshot$"#)] +async fn when_scrub_env(world: &mut StepWorld) { + world.bake.scrubbed_env = scrub_env_file(&world.bake.env_content, &world.bake.protected); +} + +#[when(regex = r#"^the compose is sanitized for the snapshot$"#)] +async fn when_sanitize_compose(world: &mut StepWorld) { + match parameterize_embedded_secret_values( + &world.bake.compose, + &world.bake.env_values, + &world.bake.protected, + ) { + Ok(out) => { + world.bake.sanitized_compose = Some(out); + world.bake.sanitize_error = None; + } + Err(conflict) => { + world.bake.sanitized_compose = None; + world.bake.sanitize_error = Some(conflict.keys); + } + } +} + +#[when(regex = r#"^the volume reset commands are built for "([^"]*)"$"#)] +async fn when_build_volume_commands(world: &mut StepWorld, project_dir: String) { + let keep: Vec<&str> = world.bake.keep_volumes.iter().map(String::as_str).collect(); + world.bake.volume_commands = volume_reset_commands(&project_dir, &keep) + .expect("valid keep list") + .join(" ; "); +} + +// ─── Then: .env ────────────────────────────────────────────────── + +#[then(regex = r#"^the scrubbed \.env has "([^"]*)" blanked$"#)] +async fn then_env_blanked(world: &mut StepWorld, key: String) { + let expected = format!("{key}=\n"); + assert!( + world.bake.scrubbed_env.contains(&expected), + "expected `{key}` blanked in:\n{}", + world.bake.scrubbed_env + ); +} + +#[then(regex = r#"^the scrubbed \.env still has "([^"]*)" set to "([^"]*)"$"#)] +async fn then_env_kept(world: &mut StepWorld, key: String, value: String) { + let expected = format!("{key}={value}\n"); + assert!( + world.bake.scrubbed_env.contains(&expected), + "expected `{key}={value}` preserved in:\n{}", + world.bake.scrubbed_env + ); +} + +#[then(regex = r#"^the scrubbed \.env contains no occurrence of "([^"]*)"$"#)] +async fn then_env_has_no_literal(world: &mut StepWorld, literal: String) { + assert!( + !world.bake.scrubbed_env.contains(&literal), + "literal still present in:\n{}", + world.bake.scrubbed_env + ); +} + +#[then(regex = r#"^the scrubbed \.env is unchanged$"#)] +async fn then_env_unchanged(world: &mut StepWorld) { + assert_eq!( + world.bake.scrubbed_env, world.bake.env_content, + "nothing is declared, so nothing may be blanked" + ); +} + +// ─── Then: compose ─────────────────────────────────────────────── + +#[then(regex = r#"^the sanitized compose contains "([^"]*)"$"#)] +async fn then_compose_contains(world: &mut StepWorld, needle: String) { + let out = world + .bake + .sanitized_compose + .as_ref() + .expect("sanitizing should have succeeded"); + assert!(out.contains(&needle), "expected `{needle}` in:\n{out}"); +} + +#[then(regex = r#"^the sanitized compose contains no occurrence of "([^"]*)"$"#)] +async fn then_compose_has_no_literal(world: &mut StepWorld, literal: String) { + let out = world + .bake + .sanitized_compose + .as_ref() + .expect("sanitizing should have succeeded"); + assert!(!out.contains(&literal), "literal still present in:\n{out}"); +} + +#[then(regex = r#"^sanitizing fails naming both "([^"]*)" and "([^"]*)"$"#)] +async fn then_sanitize_conflict(world: &mut StepWorld, first: String, second: String) { + let keys = world + .bake + .sanitize_error + .as_ref() + .expect("sanitizing should have been refused"); + assert!( + keys.contains(&first) && keys.contains(&second), + "expected both `{first}` and `{second}` in the conflict, got {keys:?}" + ); +} + +// ─── Then: volumes ─────────────────────────────────────────────── + +#[then(regex = r#"^the commands list volumes from the project compose$"#)] +async fn then_volumes_from_project(world: &mut StepWorld) { + assert!( + world + .bake + .volume_commands + .contains("docker compose config --volumes"), + "commands: {}", + world.bake.volume_commands + ); +} + +#[then(regex = r#"^the commands scope removal by the compose volume label$"#)] +async fn then_volumes_scoped_by_label(world: &mut StepWorld) { + assert!( + world + .bake + .volume_commands + .contains("--filter label=com.docker.compose.volume="), + "commands: {}", + world.bake.volume_commands + ); +} + +#[then(regex = r#"^the commands never list every volume on the host$"#)] +async fn then_volumes_not_host_wide(world: &mut StepWorld) { + assert!( + !world.bake.volume_commands.contains("docker volume ls -q |"), + "an unfiltered host-wide listing would delete the ingress' certificates \ + and abort the bake on the first in-use volume; commands: {}", + world.bake.volume_commands + ); +} + +#[then(regex = r#"^the commands skip the volume matching "([^"]*)"$"#)] +async fn then_volumes_skip_kept(world: &mut StepWorld, name: String) { + let expected = format!("case \"$v\" in {name}|"); + assert!( + world.bake.volume_commands.contains(&expected), + "expected `{expected}`; commands: {}", + world.bake.volume_commands + ); +} + +/// Scratch state for the bake-sanitization scenarios. +#[derive(Debug, Default)] +pub struct BakeWorld { + pub protected: BTreeSet, + pub env_values: BTreeMap, + pub env_content: String, + pub scrubbed_env: String, + pub compose: String, + pub sanitized_compose: Option, + pub sanitize_error: Option>, + pub keep_volumes: Vec, + pub volume_commands: String, + pub resolved_compose: String, + pub unfillable: Vec, + pub required: Vec, + pub identity_commands: String, + pub allow_unsanitized: bool, + pub refusal: Option, + pub warning: Option, + pub file_content: String, + pub write_commands: Vec, + pub stages: Vec, + pub advice: String, + pub lost: Vec, +} + +// ─── references that survive into the baked image ──────────────── + +#[when(regex = r#"^references outside the contract are resolved$"#)] +async fn when_resolve_references(world: &mut StepWorld) { + let (out, unresolved) = stacker::cli::generator::compose::resolve_non_contract_references( + &world.bake.compose, + &world.bake.env_values, + &world.bake.protected, + ); + world.bake.resolved_compose = out; + world.bake.unfillable = unresolved.into_iter().map(|r| r.name).collect(); +} + +#[when(regex = r#"^the required environment keys are collected$"#)] +async fn when_collect_required(world: &mut StepWorld) { + world.bake.required = stacker::cli::generator::compose::required_env_keys( + &world.bake.compose, + &world.bake.protected, + ) + .into_iter() + .collect(); +} + +#[then(regex = r#"^the resolved compose contains "([^"]*)"$"#)] +async fn then_resolved_contains(world: &mut StepWorld, needle: String) { + assert!( + world.bake.resolved_compose.contains(&needle), + "expected `{needle}` in:\n{}", + world.bake.resolved_compose + ); +} + +#[then(regex = r#"^the resolved compose contains no occurrence of "([^"]*)"$"#)] +async fn then_resolved_lacks(world: &mut StepWorld, literal: String) { + assert!( + !world.bake.resolved_compose.contains(&literal), + "literal still present in:\n{}", + world.bake.resolved_compose + ); +} + +#[then(regex = r#"^no reference is reported as unfillable$"#)] +async fn then_nothing_unfillable(world: &mut StepWorld) { + assert!( + world.bake.unfillable.is_empty(), + "unexpected: {:?}", + world.bake.unfillable + ); +} + +#[then(regex = r#"^"([^"]*)" is reported as unfillable$"#)] +async fn then_reported_unfillable(world: &mut StepWorld, name: String) { + assert!( + world.bake.unfillable.contains(&name), + "expected `{name}` among {:?}", + world.bake.unfillable + ); +} + +#[then(regex = r#"^the required keys are exactly "([^"]*)"$"#)] +async fn then_required_exactly(world: &mut StepWorld, csv: String) { + let expected: Vec = csv.split(',').map(|s| s.trim().to_string()).collect(); + assert_eq!(world.bake.required, expected); +} + +// ─── access belonging to the author ────────────────────────────── + +#[when(regex = r#"^the identity reset commands are built$"#)] +async fn when_build_identity_commands(world: &mut StepWorld) { + world.bake.identity_commands = + stacker::helpers::bake_finalize::identity_reset_commands().join(" ; "); +} + +#[then(regex = r#"^the commands remove "([^"]*)"$"#)] +async fn then_commands_remove(world: &mut StepWorld, path: String) { + assert!( + world.bake.identity_commands.contains(&path), + "expected `{path}` to be removed; commands: {}", + world.bake.identity_commands + ); +} + +// ─── a bake that cannot sanitize ───────────────────────────────── + +#[given(regex = r#"^unsanitized snapshots are explicitly allowed$"#)] +async fn given_allow_unsanitized(world: &mut StepWorld) { + world.bake.allow_unsanitized = true; +} + +#[given(regex = r#"^the build box has no \.env beside the compose$"#)] +async fn given_no_env_file(world: &mut StepWorld) { + world.bake.env_values.clear(); +} + +#[when(regex = r#"^the bake checks whether it can sanitize$"#)] +async fn when_check_contract(world: &mut StepWorld) { + world.bake.refusal = stacker::helpers::bake_finalize::check_contract_usable( + &world.bake.protected, + world.bake.allow_unsanitized, + ) + .err() + .map(|e| e.to_string()); +} + +#[when(regex = r#"^the bake checks the values it has to work from$"#)] +async fn when_check_env_values(world: &mut StepWorld) { + world.bake.refusal = None; + world.bake.warning = stacker::helpers::bake_finalize::env_scan_warning( + &world.bake.env_values, + &world.bake.protected, + ); +} + +#[then(regex = r#"^the bake is refused$"#)] +async fn then_bake_refused(world: &mut StepWorld) { + assert!( + world.bake.refusal.is_some(), + "the bake should not have been allowed to proceed" + ); +} + +#[then(regex = r#"^the bake is allowed$"#)] +async fn then_bake_allowed(world: &mut StepWorld) { + assert!( + world.bake.refusal.is_none(), + "unexpected refusal: {:?}", + world.bake.refusal + ); +} + +#[then(regex = r#"^the refusal mentions "([^"]*)"$"#)] +async fn then_refusal_mentions(world: &mut StepWorld, needle: String) { + let message = world + .bake + .refusal + .as_ref() + .expect("there should be a refusal"); + assert!( + message.contains(&needle), + "expected `{needle}` in: {message}" + ); +} + +#[then(regex = r#"^a warning mentions "([^"]*)"$"#)] +async fn then_warning_mentions(world: &mut StepWorld, needle: String) { + let warning = world + .bake + .warning + .as_ref() + .expect("a warning should have been raised"); + assert!( + warning.contains(&needle), + "expected `{needle}` in: {warning}" + ); +} + +#[then(regex = r#"^no warning is raised$"#)] +async fn then_no_warning(world: &mut StepWorld) { + assert!( + world.bake.warning.is_none(), + "unexpected warning: {:?}", + world.bake.warning + ); +} + +// ─── writing a file to the build box ───────────────────────────── + +#[when(regex = r#"^a file of (\d+) bytes is written to the build box$"#)] +async fn when_write_file(world: &mut StepWorld, size: usize) { + // Varied content, so a seam corrupted by bad chunking is detectable. + world.bake.file_content = (0..size).map(|i| ((i % 26) as u8 + b'a') as char).collect(); + world.bake.write_commands = + stacker::helpers::bake_finalize::write_file_commands("/tmp/x", &world.bake.file_content); +} + +#[then(regex = r#"^it takes (\d+) command$"#)] +async fn then_command_count(world: &mut StepWorld, expected: usize) { + assert_eq!(world.bake.write_commands.len(), expected); +} + +#[then(regex = r#"^it takes more than one command$"#)] +async fn then_more_than_one(world: &mut StepWorld) { + assert!( + world.bake.write_commands.len() > 1, + "expected chunking, got {}", + world.bake.write_commands.len() + ); +} + +#[then(regex = r#"^the first command truncates the file$"#)] +async fn then_first_truncates(world: &mut StepWorld) { + let first = &world.bake.write_commands[0]; + assert!(first.contains("> /tmp/x"), "not a write: {first}"); + assert!(!first.contains(">> /tmp/x"), "must not append: {first}"); +} + +#[then(regex = r#"^every later command appends$"#)] +async fn then_rest_append(world: &mut StepWorld) { + for cmd in &world.bake.write_commands[1..] { + assert!(cmd.contains(">> /tmp/x"), "chunk must append: {cmd}"); + } +} + +#[then(regex = r#"^every command fits in a single argument$"#)] +async fn then_commands_fit(world: &mut StepWorld) { + for cmd in &world.bake.write_commands { + assert!( + cmd.len() < 128 * 1024, + "a command of {} chars would be rejected as too long", + cmd.len() + ); + } +} + +#[then(regex = r#"^decoding the chunks in order yields the original content$"#)] +async fn then_chunks_round_trip(world: &mut StepWorld) { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + + let mut rebuilt = Vec::new(); + for cmd in &world.bake.write_commands { + let encoded = cmd.split_whitespace().nth(2).expect("printf %s "); + rebuilt.extend(STANDARD.decode(encoded).expect("each chunk decodes alone")); + } + + assert_eq!( + String::from_utf8(rebuilt).expect("valid utf-8"), + world.bake.file_content + ); +} + +// ─── what a failed finalize reports ────────────────────────────── + +#[given(regex = r#"^the finalize completed "([^"]*)"$"#)] +async fn given_stages_completed(world: &mut StepWorld, csv: String) { + use stacker::helpers::bake_finalize::FinalizeStage; + + world.bake.stages = csv + .split(',') + .map(|s| match s.trim() { + "read" => FinalizeStage::Read, + "teardown" => FinalizeStage::Teardown, + "rewrite compose" => FinalizeStage::RewriteCompose, + "clear env" => FinalizeStage::ClearEnv, + "strip identity" => FinalizeStage::StripIdentity, + other => panic!("unknown finalize stage: {other}"), + }) + .collect(); +} + +#[when(regex = r#"^the recovery advice is produced$"#)] +async fn when_recovery_advice(world: &mut StepWorld) { + world.bake.advice = stacker::helpers::bake_finalize::recovery_advice(&world.bake.stages); +} + +#[then(regex = r#"^the advice says the bake can be retried$"#)] +async fn then_advice_retry(world: &mut StepWorld) { + assert!( + world.bake.advice.contains("can be retried"), + "advice: {}", + world.bake.advice + ); +} + +#[then(regex = r#"^the advice asks for a fresh build box$"#)] +async fn then_advice_fresh_box(world: &mut StepWorld) { + assert!( + world.bake.advice.contains("fresh build box"), + "advice: {}", + world.bake.advice + ); +} + +#[then(regex = r#"^the advice does not ask for a fresh build box$"#)] +async fn then_advice_no_fresh_box(world: &mut StepWorld) { + assert!( + !world.bake.advice.contains("fresh build box"), + "advice: {}", + world.bake.advice + ); +} + +#[then(regex = r#"^the advice mentions "([^"]*)"$"#)] +async fn then_advice_mentions(world: &mut StepWorld, needle: String) { + assert!( + world.bake.advice.contains(&needle), + "expected `{needle}` in: {}", + world.bake.advice + ); +} + +// ─── values a clone would lose, and the list form ──────────────── + +#[given(regex = r#"^"([^"]*)" is a protected compose key$"#)] +async fn given_protected_compose_key(world: &mut StepWorld, key: String) { + world.bake.protected.insert(key); +} + +#[when(regex = r#"^the bake checks what a clone would lose$"#)] +async fn when_check_lost(world: &mut StepWorld) { + world.bake.lost = stacker::helpers::bake_finalize::env_file_values_lost_on_clone( + &world.bake.compose, + &world.bake.env_values, + &world.bake.protected, + ); +} + +#[when(regex = r#"^whole-value keys are parameterized$"#)] +async fn when_parameterize_whole_values(world: &mut StepWorld) { + let keys: std::collections::HashSet = world.bake.protected.iter().cloned().collect(); + world.bake.resolved_compose = + stacker::cli::generator::compose::parameterize_compose_env_vars(&world.bake.compose, &keys); +} + +#[then(regex = r#"^"([^"]*)" is reported as lost$"#)] +async fn then_reported_lost(world: &mut StepWorld, key: String) { + assert!( + world.bake.lost.contains(&key), + "expected `{key}` among {:?}", + world.bake.lost + ); +} + +#[then(regex = r#"^"([^"]*)" is not reported as lost$"#)] +async fn then_not_reported_lost(world: &mut StepWorld, key: String) { + assert!( + !world.bake.lost.contains(&key), + "`{key}` should survive; got {:?}", + world.bake.lost + ); +} + +#[then(regex = r#"^nothing is reported as lost$"#)] +async fn then_nothing_lost(world: &mut StepWorld) { + assert!( + world.bake.lost.is_empty(), + "unexpected: {:?}", + world.bake.lost + ); +} + +#[then(regex = r#"^the parameterized compose contains "([^"]*)"$"#)] +async fn then_parameterized_contains(world: &mut StepWorld, needle: String) { + assert!( + world.bake.resolved_compose.contains(&needle), + "expected `{needle}` in:\n{}", + world.bake.resolved_compose + ); +} + +#[when(regex = r#"^the volume reset commands are built and may fail$"#)] +async fn when_build_volume_commands_fallible(world: &mut StepWorld) { + let keep: Vec<&str> = world.bake.keep_volumes.iter().map(String::as_str).collect(); + world.bake.refusal = + stacker::helpers::bake_finalize::volume_reset_commands("/home/trydirect/project", &keep) + .err() + .map(|e| e.to_string()); +} + +#[then(regex = r#"^building the commands is refused$"#)] +async fn then_build_refused(world: &mut StepWorld) { + assert!( + world.bake.refusal.is_some(), + "a keep entry with shell syntax must not be turned into a pattern" + ); +} + +#[then(regex = r#"^the commands do not keep every name containing "([^"]*)"$"#)] +async fn then_not_bare_substring(world: &mut StepWorld, name: String) { + let bare = format!("*{name}*"); + assert!( + !world.bake.volume_commands.contains(&bare), + "a bare substring match would also keep `not-{name}-backup`: {}", + world.bake.volume_commands + ); +} diff --git a/tests/steps/mod.rs b/tests/steps/mod.rs index 9e149818..7e506eb7 100644 --- a/tests/steps/mod.rs +++ b/tests/steps/mod.rs @@ -2,6 +2,7 @@ pub mod agent; pub mod agent_executor; +pub mod bake_sanitization; pub mod cdc; pub mod cloud_server; pub mod common; @@ -61,6 +62,8 @@ pub struct StepWorld { pub cdc_payload: Option, /// CDC trigger config for CDC BDD tests pub cdc_trigger: Option, + /// Scratch state for the bake-sanitization scenarios + pub bake: bake_sanitization::BakeWorld, } /// Wrapper for WebSocket stream that implements Debug @@ -124,6 +127,7 @@ impl StepWorld { cdc_event: None, cdc_payload: None, cdc_trigger: None, + bake: Default::default(), } } From 27508ec0b5136e36dde08aea5dff8d05b772c57c Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Mon, 21 Sep 2026 19:01:58 +0300 Subject: [PATCH 15/20] fix(compose): stop double-wrapping healthcheck test commands Every healthcheck written the way the reference documents came out broken: /bin/sh: 1: CMD-SHELL: not found Docker wraps a plain string `test` in `CMD-SHELL` itself, so a string that already spells the prefix out gets wrapped twice and the container tries to execute a program named `CMD-SHELL`. The prefix only means anything in the list form. The generator was emitting the author's string verbatim. Observed on a real deploy: both stackpilot services that declare a healthcheck came up unhealthy while the application itself was fine. Harmless on its own, but the state is frozen into a baked snapshot, and a stack using `depends_on: condition: service_healthy` would never start. The decision now lives in one place. `compose_service_sync` already had `healthcheck_test_value` doing this correctly for the server-side path; the generator delegates to it and only renders the result as inline YAML. That also picks up a subtlety a second implementation would have missed: `CMD` executes argv directly, so a command containing `&&`, `|`, `$` or redirection is emitted as `CMD-SHELL` instead. An explicit list written by the author now passes through untouched. The reference said `test: "CMD pg_isready -U postgres"` and left it there. It now documents all three accepted forms, what each one runs, and why the prefix works here but not in a plain compose file. Co-Authored-By: Claude Opus 5 --- docs/STACKER_YML_REFERENCE.md | 20 +++++++- src/cli/compose_service_sync.rs | 12 ++++- src/cli/generator/compose.rs | 88 ++++++++++++++++++++++++++++++++- 3 files changed, 117 insertions(+), 3 deletions(-) diff --git a/docs/STACKER_YML_REFERENCE.md b/docs/STACKER_YML_REFERENCE.md index 9830eeff..788f4e30 100644 --- a/docs/STACKER_YML_REFERENCE.md +++ b/docs/STACKER_YML_REFERENCE.md @@ -431,11 +431,29 @@ Docker health check configuration. Mapped directly to the compose `healthcheck:` | Field | Type | Default | Description | |-------|------|---------|-------------| -| `test` | `string` | — | Health check command (e.g. `"CMD pg_isready -U postgres"`) | +| `test` | `string` | — | Health check command. See the forms below. | | `interval` | `string` | `30s` | Time between checks | | `timeout` | `string` | `30s` | Maximum time per check | | `retries` | `string \| integer` | `3` | Number of failures before unhealthy | +**Forms of `test`.** Three spellings, all accepted: + +| What you write | What runs | +|----------------|-----------| +| `pg_isready -U postgres` | the command, through a shell | +| `CMD-SHELL pg_isready -U postgres` | the same — the prefix is stripped and the list form emitted | +| `CMD pg_isready -U postgres` | the command directly, without a shell | + +`CMD` executes the arguments as-is, so it cannot use `&&`, `|`, `$` or +redirection; a `CMD` command containing any of those is emitted as `CMD-SHELL` +instead, which is what the author meant. + +Write the prefix or leave it out, whichever reads better — stacker converts to +the list form Docker expects either way. Writing the prefix inside a plain +compose file would not work: Docker wraps a bare string in `CMD-SHELL` itself, +so the prefix would be wrapped a second time and the container would try to run +a program named `CMD-SHELL`. + ```yaml services: - name: postgres diff --git a/src/cli/compose_service_sync.rs b/src/cli/compose_service_sync.rs index 1b843db2..99ccbcc0 100644 --- a/src/cli/compose_service_sync.rs +++ b/src/cli/compose_service_sync.rs @@ -276,13 +276,23 @@ fn upsert_compose_service( /// shell syntax cannot run in exec form, so it degrades to `CMD-SHELL` rather /// than being split into nonsense argv. Unprefixed strings are already valid /// and are left alone. -fn healthcheck_test_value(test: &str) -> serde_yaml::Value { +pub(crate) fn healthcheck_test_value(test: &str) -> serde_yaml::Value { fn list(parts: impl IntoIterator) -> serde_yaml::Value { serde_yaml::Value::Sequence(parts.into_iter().map(serde_yaml::Value::String).collect()) } let trimmed = test.trim(); + // An author who already wrote the list form knows what they are doing; + // re-quoting it as a string would break the very thing they got right. + if trimmed.starts_with('[') { + if let Ok(serde_yaml::Value::Sequence(parts)) = + serde_yaml::from_str::(trimmed) + { + return serde_yaml::Value::Sequence(parts); + } + } + if let Some(rest) = trimmed.strip_prefix("CMD-SHELL ") { return list(["CMD-SHELL".to_string(), rest.trim().to_string()]); } diff --git a/src/cli/generator/compose.rs b/src/cli/generator/compose.rs index 203eb68d..34c6c9d1 100644 --- a/src/cli/generator/compose.rs +++ b/src/cli/generator/compose.rs @@ -703,7 +703,10 @@ impl ComposeDefinition { if let Some(ref hc) = svc.healthcheck { out.push_str(" healthcheck:\n"); - out.push_str(&format!(" test: {}\n", yaml_quote(&hc.test))); + out.push_str(&format!( + " test: {}\n", + render_healthcheck_test(&hc.test) + )); out.push_str(&format!(" interval: {}\n", hc.interval)); out.push_str(&format!(" timeout: {}\n", hc.timeout)); out.push_str(&format!(" retries: {}\n", hc.retries)); @@ -1193,6 +1196,39 @@ fn closing_brace_on_line(rest: &str) -> Option { rest[..line_end].find('}') } +/// Render a healthcheck `test` in the form Docker actually expects. +/// +/// Docker wraps a plain *string* in `CMD-SHELL` on its own, so a string that +/// already spells the prefix out is wrapped twice and the container ends up +/// trying to run a program named `CMD-SHELL`: +/// +/// ```text +/// /bin/sh: 1: CMD-SHELL: not found +/// ``` +/// +/// The prefix only means anything in the list form — and the prefix form is +/// what the stacker.yml reference tells authors to write, so it is converted +/// rather than rejected. +/// +/// The decision itself lives in [`crate::cli::compose_service_sync::healthcheck_test_value`], +/// which the server-side sync path already uses; this only renders its result +/// as inline YAML. One rule, two call sites. +fn render_healthcheck_test(test: &str) -> String { + match crate::cli::compose_service_sync::healthcheck_test_value(test) { + serde_yaml::Value::Sequence(parts) => { + let rendered = parts + .iter() + .filter_map(serde_yaml::Value::as_str) + .map(yaml_quote) + .collect::>() + .join(", "); + format!("[{rendered}]") + } + serde_yaml::Value::String(plain) => yaml_quote(&plain), + other => yaml_quote(other.as_str().unwrap_or_default()), + } +} + /// Returns `true` when `s` looks like a POSIX env-variable name. fn is_env_identifier(s: &str) -> bool { !s.is_empty() @@ -2646,6 +2682,56 @@ services: assert_eq!(parameterize_compose_env_vars(compose, &keys), compose); } + // ── healthcheck test form ────────────────────────────────────────────── + + /// Docker wraps a *string* `test` in `CMD-SHELL` itself, so a string that + /// already carries the prefix is wrapped twice and the container tries to + /// execute a program literally named `CMD-SHELL`: + /// + /// /bin/sh: 1: CMD-SHELL: not found + /// + /// The prefix is only meaningful in the list form, and the prefix form is + /// what `docs/STACKER_YML_REFERENCE.md` tells authors to write. + #[test] + fn healthcheck_cmd_shell_prefix_becomes_a_list() { + let rendered = render_healthcheck_test("CMD-SHELL pg_isready -d app -U app"); + assert_eq!(rendered, r#"["CMD-SHELL", "pg_isready -d app -U app"]"#); + } + + /// `CMD` runs the argv directly, so each word is its own element. + #[test] + fn healthcheck_cmd_prefix_becomes_an_argv_list() { + let rendered = render_healthcheck_test("CMD redis-cli ping"); + assert_eq!(rendered, r#"["CMD", "redis-cli", "ping"]"#); + } + + /// Without a prefix the string form is correct — Docker supplies the shell. + #[test] + fn healthcheck_without_a_prefix_stays_a_string() { + let rendered = render_healthcheck_test("curl -f http://localhost/health"); + assert_eq!(rendered, r#""curl -f http://localhost/health""#); + } + + /// The list form an author wrote by hand must survive untouched. + #[test] + fn healthcheck_already_a_list_is_left_alone() { + let rendered = render_healthcheck_test(r#"["CMD", "true"]"#); + assert_eq!(rendered, r#"["CMD", "true"]"#); + } + + /// A value that merely starts with the letters CMD is not a prefix. + #[test] + fn healthcheck_cmdline_is_not_mistaken_for_a_prefix() { + let rendered = render_healthcheck_test("cmdline-check --fast"); + assert_eq!(rendered, r#""cmdline-check --fast""#); + } + + #[test] + fn healthcheck_quotes_are_escaped_inside_the_list() { + let rendered = render_healthcheck_test(r#"CMD-SHELL test "$(id -u)" = 0"#); + assert_eq!(rendered, r#"["CMD-SHELL", "test \"$(id -u)\" = 0"]"#); + } + #[test] fn parameterize_no_keys_returns_original() { let compose = "services:\n app:\n environment:\n FOO: bar\n"; From 3ee14659de0dd55c4c09bec4b1dbe09c26f058f4 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Mon, 21 Sep 2026 20:49:02 +0300 Subject: [PATCH 16/20] ci: stop caching the Rust target directory in the docker workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The build filled the runner's disk and died in post-job cleanup: Unhandled exception. System.IO.IOException: No space left on device Every check had passed — cargo check, the test suite, the BDD suite, rustfmt, all four release binaries. The job only failed while saving its cache, after 33 minutes. `actions/cache` was storing the whole `target` directory, and `restore-keys: docker-` meant each run started from an older, already bloated cache, added to it, and saved a larger one — so every run raised the next one's floor. For a workspace building five binaries in both debug and release that ratchets past the ~14 GB a runner has free. Replaced with Swatinem/rust-cache, caching registry and git only. rust.yml already made this exact call, with a comment explaining why; the two workflows now follow one rule. The trade-off is a slower docker workflow, since compilation output is no longer reused between runs. That is the right side to err on: a slow pipeline is an inconvenience, a pipeline that cannot finish is not. Co-Authored-By: Claude Opus 5 --- .github/workflows/docker.yml | 38 ++++++++++++------------------------ 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 2a9150c3..8ddc86d1 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -66,23 +66,20 @@ jobs: override: true components: rustfmt, clippy - - name: Cache cargo registry - uses: actions/cache@v4 + # Registry and git only — not `target`. Caching the target directory here + # filled the runner's disk: `restore-keys: docker-` pulled in an older, + # already-bloated cache, the build added to it, and the post-job step + # saved a larger one still, so every run grew the next one's starting + # point. A 33-minute job that passed every check then died in cleanup + # with "No space left on device". + # + # rust.yml reached the same conclusion for the same reason; this keeps + # both workflows on one rule. + - name: Cache Cargo registry/git (no target — it grows without bound here) + uses: Swatinem/rust-cache@v2 with: - path: ~/.cargo/registry - key: docker-registry-${{ hashFiles('**/Cargo.lock') }} - restore-keys: | - docker-registry- - docker- - - - name: Cache cargo index - uses: actions/cache@v4 - with: - path: ~/.cargo/git - key: docker-index-${{ hashFiles('**/Cargo.lock') }} - restore-keys: | - docker-index- - docker- + cache-targets: "false" + key: docker-cicd - name: Generate Secret Key run: | @@ -99,15 +96,6 @@ jobs: echo "PostgreSQL did not become ready in time" >&2 exit 1 - - name: Cache cargo build - uses: actions/cache@v4 - with: - path: target - key: docker-build-${{ hashFiles('**/Cargo.lock') }} - restore-keys: | - docker-build- - docker- - - name: Cargo check uses: actions-rs/cargo@v1 with: From 96866848f783cf4ddb9c6ec69804433f61c50a2b Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Wed, 23 Sep 2026 13:34:20 +0300 Subject: [PATCH 17/20] feat(contract): let the author declare which volumes survive a bake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bake drops a stack's volumes so a buyer's machine initialises them with its own credentials. Some must survive that: Ollama weights are gigabytes behind a 600s pull, Qdrant embeddings cost as much to recompute — preserving them is why the snapshot exists. Until now the exceptions were a `match` on the stack slug inside `bake_finalize.rs`. Four stacks were listed, floci and the rest were queued, and each one needed a code change and a rebuilt binary. The author knows which of their volumes are expensive and which hold credentials; the platform does not. So it goes in `config_contract`, beside the fields, reusing everything already built for them: the `mutability` vocabulary, and the path from stacker.yml through the submit body to `stack_template_version.config_contract` and on to `bake.rs`, which resolves it before finalize runs. config_contract: services: stackpilot-ollama: volumes: stackpilot_ollama: { mutability: fixed } `fixed` ships the content inside the image; `generated` — the default for anything undeclared — drops it. Erring that way costs a rebuild; the opposite default would hand the author's credentials to every buyer. `provided` and `editable` describe who types a value and are rejected: a volume holds state, not a value. The platform validates only the name, which is interpolated into a shell pattern. It deliberately does not second-guess the declaration. An earlier revision of this change did. It refused any volume whose service declares `generated` or `provided` fields, reasoning that such a service persists the secret. Measuring real containers killed that rule: a Postgres data directory holds `SCRAM-SHA-256$4096:…` and not the password in any searchable form; n8n keeps its own encryption key inside `database.sqlite`; a Qdrant volume holds only collections, because Qdrant reads its API key from the environment at every start. The secret is absent from all three — so neither the field-based rule nor a search of the volume's bytes tells the two that must reset from the one that must be kept. The difference is behavioural, and only the author can see it. Left in, the rule would have forced ai-knowledge-base to recompute its embeddings on every buyer's machine: the exact expense a snapshot avoids. FinalizeContext now carries the parsed contract rather than a flattened key set, since the kind-per-service structure is what the volume policy needs. Co-Authored-By: Claude Opus 5 --- docs/STACKER_YML_REFERENCE.md | 40 +++++ src/bin/bake.rs | 10 +- src/cli/config_parser.rs | 198 ++++++++++++++++++++++- src/helpers/bake_finalize.rs | 193 +++++++++++++++++++--- tests/features/bake_sanitization.feature | 35 ++++ tests/steps/bake_sanitization.rs | 95 +++++++++++ 6 files changed, 542 insertions(+), 29 deletions(-) diff --git a/docs/STACKER_YML_REFERENCE.md b/docs/STACKER_YML_REFERENCE.md index 788f4e30..ac7212b1 100644 --- a/docs/STACKER_YML_REFERENCE.md +++ b/docs/STACKER_YML_REFERENCE.md @@ -847,6 +847,46 @@ environments: --- +### Volume policy in `config_contract` + +A baked marketplace image is cloned for every buyer, and a volume that travels +inside it arrives identical for all of them. Declare which ones should: + +```yaml +config_contract: + services: + stackpilot-ollama: + volumes: + stackpilot_ollama: { mutability: fixed } +``` + +`fixed` — the content ships inside the image. `generated` — the volume is dropped +before the snapshot so the buyer's machine initialises it from scratch. **An +undeclared volume behaves as `generated`**: forgetting a declaration costs a +rebuild, whereas the opposite default would hand the author's credentials to +every buyer. + +`provided` and `editable` describe who types a *value*; a volume holds state and +has no value to type, so both are rejected. + +**Declare `fixed` only for volumes holding data the service does not derive from +a secret** — model weights, embeddings, a content cache. The distinction is not +whether the service *has* a secret but whether it *persists* something built from +one: + +| Service | Volume holds | Declare | +|---|---|---| +| Ollama | model weights | `fixed` | +| Qdrant | collections; the API key is read from the environment at every start | `fixed` | +| Postgres | the role password as `SCRAM-SHA-256$4096:…` | `generated` | +| n8n | its own encryption key inside `database.sqlite` | `generated` | + +Nothing distinguishes these automatically: the secret is not present verbatim in +any of the four volumes, so searching for it finds nothing in the safe and the +unsafe case alike. The author knows how their service treats the secret; the +platform cannot compute it. Get this wrong in the unsafe direction and every +buyer inherits the author's credential. + ## `volumes` *Optional* · `map` · Default: `{}` diff --git a/src/bin/bake.rs b/src/bin/bake.rs index 17558565..b33d2cc3 100644 --- a/src/bin/bake.rs +++ b/src/bin/bake.rs @@ -134,6 +134,14 @@ async fn main() -> Result<(), Box> { .map(stacker::helpers::bake_finalize::protected_keys_from_contract) .unwrap_or_default(); + // Parsed form: the finalize step needs the service a field belongs to, which + // the flat key set above has thrown away. An unparseable contract is treated + // as absent — `check_contract_usable` below then refuses the bake. + let parsed_contract: stacker::cli::config_parser::ConfigContract = config_contract + .clone() + .and_then(|c| serde_json::from_value(c).ok()) + .unwrap_or_default(); + // Refuse before touching the box: with no contract there is nothing to // sanitize, and publishing anyway is how the author's credentials reach // every buyer. @@ -157,7 +165,7 @@ async fn main() -> Result<(), Box> { private_key_pem, project_dir: project_dir.clone(), stack: stack.clone(), - protected_keys: protected_keys.clone(), + contract: parsed_contract.clone(), }; let outcome = stacker::helpers::bake_finalize::finalize_build_box(&ctx).await?; eprintln!( diff --git a/src/cli/config_parser.rs b/src/cli/config_parser.rs index f871ffa6..4b3a503a 100644 --- a/src/cli/config_parser.rs +++ b/src/cli/config_parser.rs @@ -1100,6 +1100,61 @@ impl FieldPolicy { } } +/// Declared policy for one `config_contract.services..volumes.` +/// entry — the second kind a service block can carry, after `fields`. +/// +/// A volume holds *state*, not a value, so only two of the four mutabilities +/// mean anything: +/// +/// * `fixed` — the author's content ships inside the image and is identical for +/// every buyer. Correct for expensive, credential-free content: model weights, +/// embeddings. +/// * `generated` — the volume is dropped before the snapshot, so the buyer's +/// machine initialises it from scratch with the buyer's own values. +/// +/// `provided` and `editable` describe who *types* a value; there is nothing to +/// type here, and accepting them would leave the bake guessing. They are +/// rejected at parse time. +/// +/// An undeclared volume behaves as `generated`. The error direction is +/// deliberate: forgetting a declaration costs a rebuild, while the opposite +/// default would hand the author's credentials to every buyer. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +pub struct VolumePolicy { + pub mutability: Mutability, +} + +impl<'de> Deserialize<'de> for VolumePolicy { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + #[derive(Deserialize)] + #[serde(deny_unknown_fields)] + struct Raw { + mutability: Mutability, + } + + let raw = Raw::deserialize(deserializer)?; + match raw.mutability { + Mutability::Fixed | Mutability::Generated => Ok(VolumePolicy { + mutability: raw.mutability, + }), + other => Err(serde::de::Error::custom(format!( + "`mutability: {}` is not meaningful for a volume — a volume holds \ + state, not a value somebody types. Use `fixed` to ship the \ + author's content in the image, or `generated` to have the buyer's \ + machine create it from scratch.", + match other { + Mutability::Provided => "provided", + Mutability::Editable => "editable", + _ => unreachable!("fixed and generated are handled above"), + } + ))), + } + } +} + /// Per-service field policy declarations. /// /// Backed by a single `fields: HashMap`. Accepts and @@ -1110,6 +1165,9 @@ impl FieldPolicy { #[derive(Debug, Clone, Default, PartialEq)] pub struct TargetConfigContract { pub fields: HashMap, + /// Volumes this service owns, and whether each survives the bake. + /// Absent means every volume resets — see [`VolumePolicy`]. + pub volumes: HashMap, } impl TargetConfigContract { @@ -1139,7 +1197,11 @@ impl TargetConfigContract { .entry(key) .or_insert_with(|| FieldPolicy::fixed(false)); } - TargetConfigContract { fields } + // The legacy three-list shape predates volumes and never declared any. + TargetConfigContract { + fields, + volumes: HashMap::new(), + } } fn keys_where(&self, predicate: impl Fn(&FieldPolicy) -> bool) -> Vec { @@ -1183,6 +1245,17 @@ impl TargetConfigContract { pub fn editable_keys(&self) -> Vec { self.keys_where(|p| p.mutability == Mutability::Editable) } + + /// Volumes declared `mutability: fixed` — the ones whose content survives + /// into the image. Everything else, declared or not, is dropped before the + /// snapshot so the buyer's machine starts it clean. + pub fn fixed_volumes(&self) -> Vec { + self.volumes + .iter() + .filter(|(_, policy)| policy.mutability == Mutability::Fixed) + .map(|(name, _)| name.clone()) + .collect() + } } #[derive(Deserialize, Default)] @@ -1192,6 +1265,7 @@ struct RawTargetConfigContract { optional: Vec, secret: Vec, fields: HashMap, + volumes: HashMap, } impl<'de> Deserialize<'de> for TargetConfigContract { @@ -1221,7 +1295,10 @@ impl<'de> Deserialize<'de> for TargetConfigContract { .or_insert_with(|| FieldPolicy::fixed(false)); } - Ok(TargetConfigContract { fields }) + Ok(TargetConfigContract { + fields, + volumes: raw.volumes, + }) } } @@ -2609,6 +2686,123 @@ config_contract: assert!(format!("{err}").contains("derived_jwt")); } + // ── volume policy ────────────────────────────────────────────────────── + + /// A volume is the second kind a service block can declare, after `fields`. + /// `fixed` means the author's content ships in the image as-is; `generated` + /// means the buyer's machine creates it from scratch. + #[test] + fn volume_policy_parses_fixed_and_generated() { + let yaml = r#" +name: stackpilot +config_contract: + services: + stackpilot-ollama: + volumes: + stackpilot_ollama: + mutability: fixed + stackpilot-db: + fields: + POSTGRES_PASSWORD: + mutability: generated + type: alphanumeric + volumes: + stackpilot_pgdata: + mutability: generated +"#; + let config = StackerConfig::from_str(yaml).unwrap(); + let ollama = &config.config_contract.services["stackpilot-ollama"]; + assert_eq!( + ollama.volumes["stackpilot_ollama"].mutability, + Mutability::Fixed + ); + + let db = &config.config_contract.services["stackpilot-db"]; + assert_eq!( + db.volumes["stackpilot_pgdata"].mutability, + Mutability::Generated + ); + // Fields and volumes coexist in one service block. + assert_eq!( + db.fields["POSTGRES_PASSWORD"].mutability, + Mutability::Generated + ); + } + + /// `fixed_volumes()` is what the bake asks for: the volumes that survive. + #[test] + fn fixed_volumes_lists_only_the_ones_that_survive() { + let yaml = r#" +name: kb +config_contract: + services: + worker: + volumes: + kb_ollama: { mutability: fixed } + kb_qdrant: { mutability: fixed } + kb_pgdata: { mutability: generated } +"#; + let config = StackerConfig::from_str(yaml).unwrap(); + let mut kept = config.config_contract.services["worker"].fixed_volumes(); + kept.sort(); + assert_eq!(kept, vec!["kb_ollama".to_string(), "kb_qdrant".to_string()]); + } + + /// `provided` and `editable` describe who types a *value*; a volume has no + /// value to type. Accepting them would leave the bake guessing. + #[test] + fn volume_policy_rejects_mutabilities_that_make_no_sense_for_state() { + for mutability in ["provided", "editable"] { + let yaml = format!( + r#" +name: s +config_contract: + services: + app: + volumes: + app_data: + mutability: {mutability} +"# + ); + assert!( + StackerConfig::from_str(&yaml).is_err(), + "`mutability: {mutability}` is meaningless for a volume and must be rejected" + ); + } + } + + /// An undeclared volume resets. Losing rebuildable content costs time; + /// keeping a credential-bearing one leaks the author's secrets. + #[test] + fn a_service_with_no_volume_block_keeps_nothing() { + let yaml = r#" +name: s +config_contract: + services: + app: + fields: + SECRET_KEY: { mutability: generated, type: hex } +"#; + let config = StackerConfig::from_str(yaml).unwrap(); + assert!(config.config_contract.services["app"] + .fixed_volumes() + .is_empty()); + } + + /// The service block is a closed set of kinds — a typo must not be ignored. + #[test] + fn an_unknown_kind_in_a_service_block_is_rejected() { + let yaml = r#" +name: s +config_contract: + services: + app: + volumez: + app_data: { mutability: fixed } +"#; + assert!(StackerConfig::from_str(yaml).is_err()); + } + #[test] fn field_policy_unknown_mutability_is_rejected() { let yaml = r#" diff --git a/src/helpers/bake_finalize.rs b/src/helpers/bake_finalize.rs index a2a08c27..b7188400 100644 --- a/src/helpers/bake_finalize.rs +++ b/src/helpers/bake_finalize.rs @@ -25,21 +25,64 @@ use std::collections::BTreeSet; -/// Volumes whose content must survive the bake, keyed by stack slug. +/// Volumes the author declared `mutability: fixed` — the ones whose content +/// travels into the image. /// -/// Defaulting to *reset* is deliberate: wrongly resetting a volume costs a -/// rebuild of cheap state, while wrongly keeping one leaks the author's -/// credentials to every buyer. Only volumes that are expensive to rebuild -/// **and** carry no credentials belong here. +/// Everything else resets, declared or not. The error direction is deliberate: +/// forgetting a declaration costs a rebuild of cheap state, while keeping a +/// credential-bearing volume hands the author's secrets to every buyer. /// -/// `stackpilot`'s Ollama volume holds the pulled model weights — gigabytes, -/// with a 600s pull timeout in `scripts/download-model.sh`. Preserving it is -/// the entire economic point of baking that stack. -pub fn volumes_to_keep(stack: &str) -> &'static [&'static str] { - match stack { - "stackpilot" => &["ollama"], - _ => &[], +/// This used to be a `match` on the stack slug in this file. The author knows +/// which of their volumes are expensive and which hold credentials; the platform +/// does not, and a hardcoded list needed a code change and a rebuilt binary for +/// every new stack. +pub fn volumes_to_keep(contract: &crate::cli::config_parser::ConfigContract) -> Vec { + contract + .services + .values() + .flat_map(|service| service.fixed_volumes()) + .collect() +} + +/// Validate the author's volume declarations. +/// +/// Only what a machine can actually establish is checked here: that the name is +/// safe to interpolate into a shell `case` pattern. Whether a volume is *safe to +/// keep* is the author's call, and deliberately so. +/// +/// An earlier version refused any volume belonging to a service that declares +/// `generated` or `provided` fields, on the theory that such a service wrote the +/// secret into its own data. Measurement killed that rule: a Postgres data +/// directory holds `SCRAM-SHA-256$4096:…`, a hash — the password appears nowhere, +/// literally or base64-encoded; n8n keeps its own encryption key in +/// `database.sqlite`; and a Qdrant volume holds nothing but collections, because +/// Qdrant reads its API key from the environment on every start. Searching the +/// volume for the secret finds nothing in any of the three, so that cannot +/// distinguish them either. +/// +/// The real difference is behavioural — whether the service derives persistent +/// state from the secret — and it is not visible in the volume's bytes. The +/// author knows it; the platform cannot compute it. So the platform checks what +/// it can and trusts the author with the rest, the same way `config_contract` +/// trusts the author about which fields are sensitive. +pub fn check_volume_declarations( + contract: &crate::cli::config_parser::ConfigContract, +) -> Result<(), crate::helpers::bake::BakeError> { + for (service_name, service) in &contract.services { + for volume in service.fixed_volumes() { + if !is_plain_volume_name(&volume) { + return Err(crate::helpers::bake::BakeError::Finalize(format!( + "volume `{volume}` on service `{service_name}` contains characters \ + that are shell pattern syntax. It would match something other than \ + intended, and keeping a volume that should have been reset leaves \ + the author's credentials in the image. Use only letters, digits, \ + `_`, `-` and `.`." + ))); + } + } } + + Ok(()) } /// Values a service would lose when the buyer's machine replaces the env file. @@ -307,8 +350,10 @@ pub struct FinalizeContext { pub project_dir: String, /// Stack slug — selects the volume keep-list. pub stack: String, - /// Contract fields with `mutability: generated`/`provided`. - pub protected_keys: BTreeSet, + /// The author's field policy, parsed. Kept whole rather than flattened: the + /// volume check needs to know *which service* declares a protected field, + /// and flattening loses exactly that. + pub contract: crate::cli::config_parser::ConfigContract, } /// What the finalize step learned about the image it just sanitized. @@ -360,6 +405,11 @@ pub async fn finalize_build_box( } }; + // The flat set is still what the scrub and the parameterizer want. + let protected_keys = protected_keys_from_contract( + &serde_json::to_value(&ctx.contract).unwrap_or(serde_json::Value::Null), + ); + let compose_path = format!("{}/docker-compose.yml", ctx.project_dir); let env_path = format!("{}/.env", ctx.project_dir); @@ -367,6 +417,13 @@ pub async fn finalize_build_box( // already happened — see `recovery_advice`. let mut done: Vec = Vec::new(); + // Refuse a declaration that would ship the author's credentials, before + // anything on the box is touched. + if let Err(err) = check_volume_declarations(&ctx.contract) { + disconnect_ssh(session).await; + return Err(err); + } + let result = async { // 1. Read what the deploy left on the box. let compose = run(format!("cat {compose_path}")) @@ -379,7 +436,7 @@ pub async fn finalize_build_box( done.push(FinalizeStage::Read); let env_values = parse_env_pairs(&env_raw); - let lost = env_file_values_lost_on_clone(&compose, &env_values, &ctx.protected_keys); + let lost = env_file_values_lost_on_clone(&compose, &env_values, &protected_keys); if !lost.is_empty() { return Err(BakeError::Finalize(format!( "this compose reads values through `env_file:`, and {} of them are not \ @@ -391,7 +448,7 @@ pub async fn finalize_build_box( lost.join(", ") ))); } - if let Some(warning) = env_scan_warning(&env_values, &ctx.protected_keys) { + if let Some(warning) = env_scan_warning(&env_values, &protected_keys) { eprintln!("WARNING: {warning}"); } @@ -401,7 +458,9 @@ pub async fn finalize_build_box( // a cleared .env, and any `${VAR}` outside an environment block // (`image: ${REGISTRY}/app:${TAG}`) would then resolve empty and fail // the teardown — with the files already modified and no way back. - for cmd in volume_reset_commands(&ctx.project_dir, volumes_to_keep(&ctx.stack))? { + let keep = volumes_to_keep(&ctx.contract); + let keep_refs: Vec<&str> = keep.iter().map(String::as_str).collect(); + for cmd in volume_reset_commands(&ctx.project_dir, &keep_refs)? { run(cmd).await.map_err(|e| fail("volume reset", e))?; } done.push(FinalizeStage::Teardown); @@ -412,7 +471,7 @@ pub async fn finalize_build_box( let sanitized = crate::cli::generator::compose::parameterize_embedded_secret_values( &compose, &env_values, - &ctx.protected_keys, + &protected_keys, ) .map_err(|conflict| BakeError::Finalize(conflict.to_string()))?; @@ -425,7 +484,7 @@ pub async fn finalize_build_box( crate::cli::generator::compose::resolve_non_contract_references( &sanitized, &env_values, - &ctx.protected_keys, + &protected_keys, ); if !unresolved.is_empty() { let names: Vec<&str> = unresolved.iter().map(|r| r.name.as_str()).collect(); @@ -450,13 +509,13 @@ pub async fn finalize_build_box( // the contract, not from the text of the file. A reference only counts // as required when something is expected to supply it. let required_env_keys = - crate::cli::generator::compose::required_env_keys(&sanitized, &ctx.protected_keys); + crate::cli::generator::compose::required_env_keys(&sanitized, &protected_keys); // 6. Clear the author's secrets in the co-located .env. The buyer's box // overwrites this file wholesale from /etc/stacker/env at boot, so // the blanked values are never read. if !env_raw.trim().is_empty() { - let scrubbed = scrub_env_file(&env_raw, &ctx.protected_keys); + let scrubbed = scrub_env_file(&env_raw, &protected_keys); write_remote_file(&run, &env_path, &scrubbed) .await .map_err(|e| fail("write .env", e))?; @@ -682,6 +741,86 @@ mod tests { keys.iter().map(|k| k.to_string()).collect() } + fn contract(yaml: serde_json::Value) -> crate::cli::config_parser::ConfigContract { + serde_json::from_value(yaml).expect("contract parses") + } + + /// The author declares which volumes survive; the platform stops hardcoding + /// a list per stack slug. + #[test] + fn kept_volumes_come_from_the_contract() { + let c = contract(serde_json::json!({ + "services": { + "ollama": { "volumes": { "app_ollama": { "mutability": "fixed" } } }, + "db": { "volumes": { "app_pgdata": { "mutability": "generated" } } } + } + })); + let mut kept = volumes_to_keep(&c); + kept.sort(); + assert_eq!(kept, vec!["app_ollama".to_string()]); + } + + #[test] + fn a_contract_declaring_no_volume_keeps_nothing() { + let c = contract(serde_json::json!({ + "services": { "app": { "fields": { "SECRET_KEY": { "mutability": "generated", "type": "hex" } } } } + })); + assert!(volumes_to_keep(&c).is_empty()); + } + + /// The author decides which volumes are safe to keep, including on services + /// that regenerate secrets — because a machine cannot tell the difference. + /// + /// Measured on real containers: a Postgres data directory stores + /// `SCRAM-SHA-256$4096:...`, so the password is not in the volume in any + /// searchable form; n8n keeps its own encryption key inside + /// `database.sqlite`; a Qdrant volume holds only collections, because Qdrant + /// reads its API key from the environment at every start. All three look + /// identical to any automated check — yet the first two must be reset and the + /// third must be kept. The difference is behavioural, not observable. + /// + /// An earlier revision refused a kept volume whenever its service declared a + /// protected field. That rule would have forced `ai-knowledge-base` to + /// recompute its embeddings on every buyer's machine — precisely the expense + /// the snapshot exists to avoid. + #[test] + fn a_volume_of_a_service_with_generated_fields_is_the_authors_call() { + let c = contract(serde_json::json!({ + "services": { + "qdrant": { + "fields": { "QDRANT__SERVICE__API_KEY": { "mutability": "generated", "type": "alphanumeric" } }, + "volumes": { "kb_qdrant_data": { "mutability": "fixed" } } + } + } + })); + assert!( + check_volume_declarations(&c).is_ok(), + "Qdrant reads its key from the environment; its volume holds only vectors" + ); + assert_eq!(volumes_to_keep(&c), vec!["kb_qdrant_data".to_string()]); + } + + #[test] + fn a_service_without_protected_fields_may_keep_its_volume() { + let c = contract(serde_json::json!({ + "services": { + "ollama": { "volumes": { "app_ollama": { "mutability": "fixed" } } }, + "web": { "fields": { "LOG_LEVEL": { "mutability": "editable" } } } + } + })); + assert!(check_volume_declarations(&c).is_ok()); + } + + /// Names now arrive from an author rather than a constant, so the shell + /// pattern gate applies to them. + #[test] + fn an_author_supplied_name_with_shell_syntax_is_refused() { + let c = contract(serde_json::json!({ + "services": { "app": { "volumes": { "oll*ama": { "mutability": "fixed" } } } } + })); + assert!(check_volume_declarations(&c).is_err()); + } + /// L1 — a keep entry is interpolated into a shell `case` pattern, where /// `|`, `)`, `*`, `?` and `[` are all syntax. An entry carrying one of them /// would silently match something else, or break the command outright. @@ -1150,11 +1289,13 @@ mod tests { assert!(protected_keys_from_contract(&serde_json::Value::Null).is_empty()); } + /// A contract that declares nothing keeps nothing — the same default the + /// stack-slug `match` used to give an unlisted stack, now without needing + /// the platform to know the stack at all. #[test] - fn stackpilot_keeps_only_its_model_volume() { - assert_eq!(volumes_to_keep("stackpilot"), &["ollama"]); - // An unknown stack defaults to resetting everything — leaking is worse - // than rebuilding. - assert!(volumes_to_keep("something-else").is_empty()); + fn a_contract_that_declares_nothing_keeps_nothing() { + let empty = crate::cli::config_parser::ConfigContract::default(); + assert!(volumes_to_keep(&empty).is_empty()); + assert!(check_volume_declarations(&empty).is_ok()); } } diff --git a/tests/features/bake_sanitization.feature b/tests/features/bake_sanitization.feature index d53b3830..f1bb7443 100644 --- a/tests/features/bake_sanitization.feature +++ b/tests/features/bake_sanitization.feature @@ -329,3 +329,38 @@ Feature: Bake-time sanitization of a build box When whole-value keys are parameterized Then the parameterized compose contains "- POSTGRES_PASSWORD=${POSTGRES_PASSWORD}" And the parameterized compose contains "- POSTGRES_USER=stackpilot" + + Rule: the author declares which volumes survive, and the platform checks it + + Scenario: A volume declared fixed survives the bake + Given the contract declares volume "app_ollama" on service "ollama" as fixed + When the kept volumes are collected + Then "app_ollama" is kept + + Scenario: An undeclared volume is reset + Given the contract declares nothing + When the kept volumes are collected + Then nothing is kept + + # Measured on real containers: Postgres stores the password as a SCRAM hash, + # n8n keeps its encryption key inside database.sqlite, and a Qdrant volume + # holds only collections because the API key is read from the environment at + # every start. The secret is absent from all three, so no automated check can + # separate the volume that must be reset from the one that must be kept. The + # author knows; the platform does not. + Scenario: Keeping a volume of a service that regenerates a secret is the author's call + Given the contract declares volume "kb_qdrant_data" on service "qdrant" as fixed + And service "qdrant" regenerates "QDRANT__SERVICE__API_KEY" + When the declaration is checked + Then the declaration is accepted + + Scenario: A volume of a service without per-buyer secrets is allowed + Given the contract declares volume "app_ollama" on service "ollama" as fixed + When the declaration is checked + Then the declaration is accepted + + Scenario: A name carrying shell syntax is refused + Given the contract declares volume "oll*ama" on service "ollama" as fixed + When the declaration is checked + Then the declaration is refused + And the refusal names "oll*ama" diff --git a/tests/steps/bake_sanitization.rs b/tests/steps/bake_sanitization.rs index 9be0705c..10fb2d2d 100644 --- a/tests/steps/bake_sanitization.rs +++ b/tests/steps/bake_sanitization.rs @@ -224,6 +224,9 @@ pub struct BakeWorld { pub stages: Vec, pub advice: String, pub lost: Vec, + pub declared_volumes: Vec<(String, String)>, + pub declared_fields: Vec<(String, String)>, + pub kept: Vec, } // ─── references that survive into the baked image ──────────────── @@ -601,3 +604,95 @@ async fn then_not_bare_substring(world: &mut StepWorld, name: String) { world.bake.volume_commands ); } + +// ─── author-declared volume policy ─────────────────────────────── + +#[given(regex = r#"^the contract declares volume "([^"]*)" on service "([^"]*)" as fixed$"#)] +async fn given_fixed_volume(world: &mut StepWorld, volume: String, service: String) { + world.bake.declared_volumes.push((service, volume)); +} + +#[given(regex = r#"^service "([^"]*)" regenerates "([^"]*)"$"#)] +async fn given_service_regenerates(world: &mut StepWorld, service: String, field: String) { + world.bake.declared_fields.push((service, field)); +} + +fn build_contract(world: &StepWorld) -> stacker::cli::config_parser::ConfigContract { + let mut services = serde_json::Map::new(); + for (service, volume) in &world.bake.declared_volumes { + let entry = services + .entry(service.clone()) + .or_insert_with(|| serde_json::json!({})); + entry["volumes"][volume] = serde_json::json!({ "mutability": "fixed" }); + } + for (service, field) in &world.bake.declared_fields { + let entry = services + .entry(service.clone()) + .or_insert_with(|| serde_json::json!({})); + entry["fields"][field] = + serde_json::json!({ "mutability": "generated", "type": "alphanumeric" }); + } + serde_json::from_value(serde_json::json!({ "services": services })).expect("contract parses") +} + +#[when(regex = r#"^the kept volumes are collected$"#)] +async fn when_collect_kept(world: &mut StepWorld) { + let contract = build_contract(world); + world.bake.kept = stacker::helpers::bake_finalize::volumes_to_keep(&contract); +} + +#[when(regex = r#"^the declaration is checked$"#)] +async fn when_check_declaration(world: &mut StepWorld) { + let contract = build_contract(world); + world.bake.refusal = stacker::helpers::bake_finalize::check_volume_declarations(&contract) + .err() + .map(|e| e.to_string()); +} + +#[then(regex = r#"^"([^"]*)" is kept$"#)] +async fn then_volume_kept(world: &mut StepWorld, name: String) { + assert!( + world.bake.kept.contains(&name), + "expected `{name}` among {:?}", + world.bake.kept + ); +} + +#[then(regex = r#"^nothing is kept$"#)] +async fn then_nothing_kept(world: &mut StepWorld) { + assert!( + world.bake.kept.is_empty(), + "unexpected: {:?}", + world.bake.kept + ); +} + +#[then(regex = r#"^the declaration is refused$"#)] +async fn then_declaration_refused(world: &mut StepWorld) { + assert!( + world.bake.refusal.is_some(), + "a volume holding the author's credentials must not be shipped" + ); +} + +#[then(regex = r#"^the declaration is accepted$"#)] +async fn then_declaration_accepted(world: &mut StepWorld) { + assert!( + world.bake.refusal.is_none(), + "unexpected refusal: {:?}", + world.bake.refusal + ); +} + +#[then(regex = r#"^the refusal names "([^"]*)"$"#)] +async fn then_refusal_names(world: &mut StepWorld, needle: String) { + let message = world + .bake + .refusal + .as_ref() + .expect("there should be a refusal"); + assert!( + message.contains(&needle), + "expected `{needle}` in: {message}" + ); +} From 7fc85139a0766b1b448760bfb66659f09fae27b9 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Wed, 23 Sep 2026 15:53:54 +0300 Subject: [PATCH 18/20] ci: build the release binaries once, not twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A run takes 47 minutes, and the release binaries are compiled twice in it: once by the test job, then again from scratch inside the Docker build. The first set was already being packed into `app.tar.gz` and uploaded — nothing ever downloaded it. The reason it could not simply be reused is the one that has to be got right: the binaries are dynamically linked against glibc, and glibc is forward- but not backward-compatible. Built on the runner (Ubuntu 24.04, glibc 2.39) they would not start on debian:bookworm-slim (glibc 2.36). So the test job now runs inside `rust:bookworm` — the same image the Dockerfile builds in, and the one the runtime stage is derived from. The environments match exactly, and the Docker job copies the artifact instead of recompiling. The Dockerfile keeps both paths. `BINARIES=prebuilt` takes them from a build context; the default still compiles from source, so a local `docker build` works unchanged. Both were verified to parse, and the prebuilt path was built end to end: the four binaries, the config files and the sqlx CLI all land in the image with no compilation. Two things fell out along the way. The job now builds all four binaries the image needs — `console` and `backfill_field_policy` were missing from the artifact, which is part of why it could not be used. And `cargo install sqlx-cli` moved to its own small stage with only the postgres and rustls features, so the prebuilt path no longer pays 110 seconds of it to fetch two YAML files. Inside a job container, service containers resolve by name rather than 127.0.0.1, so PGHOST changes accordingly. Co-Authored-By: Claude Opus 5 --- .github/workflows/docker.yml | 57 ++++++++++++++++++++++++++++-------- Dockerfile | 54 ++++++++++++++++++++++++++++------ 2 files changed, 90 insertions(+), 21 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 8ddc86d1..28446ec6 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -19,6 +19,14 @@ jobs: name: Cargo and npm build runs-on: ubuntu-latest #runs-on: [self-hosted, linux] + # Build inside the same image the Dockerfile builds in, so the binaries + # produced here can be copied into the runtime image instead of being + # compiled a second time. They are dynamically linked against glibc, and + # glibc is forward- but not backward-compatible: a binary built on the + # runner (Ubuntu 24.04, glibc 2.39) would not start on debian:bookworm-slim + # (glibc 2.36). Building in rust:bookworm makes the two match exactly. + container: + image: rust:bookworm services: postgres: image: postgres:16 @@ -42,7 +50,9 @@ jobs: ref: ${{ github.ref }} - name: Export PostgreSQL connection env run: | - echo "PGHOST=127.0.0.1" >> "$GITHUB_ENV" + # Inside a job container, services resolve by name on the shared + # network — 127.0.0.1 is the container itself. + echo "PGHOST=postgres" >> "$GITHUB_ENV" echo "PGPORT=5432" >> "$GITHUB_ENV" echo "PGUSER=postgres" >> "$GITHUB_ENV" echo "PGPASSWORD=postgres" >> "$GITHUB_ENV" @@ -136,17 +146,16 @@ jobs: command: clippy args: -- -D warnings - - name: Build server (release) - uses: actions-rs/cargo@v1 - with: - command: build - args: --release --bin server - - - name: Build cleanup-notify (release) - uses: actions-rs/cargo@v1 - with: - command: build - args: --release --bin cleanup-notify + # One invocation, so the four binaries share a single compilation of the + # workspace instead of four sequential ones. These are the binaries the + # runtime image needs; the Docker job copies them rather than rebuilding. + - name: Build release binaries + run: | + cargo build --release \ + --bin server \ + --bin console --features explain \ + --bin cleanup-notify \ + --bin backfill_field_policy - name: Set up Node.js if: ${{ hashFiles('web/package.json') != '' }} @@ -181,7 +190,9 @@ jobs: run: | mkdir -p app/stacker/dist cp target/release/server app/stacker/server + cp target/release/console app/stacker/console cp target/release/cleanup-notify app/stacker/cleanup-notify + cp target/release/backfill_field_policy app/stacker/backfill_field_policy if [ -d web/dist ]; then cp -a web/dist/. app/stacker; fi cp Dockerfile app/Dockerfile cp access_control.conf.dist app/access_control.conf.dist @@ -210,6 +221,25 @@ jobs: run: | test -d "${GITHUB_WORKSPACE}/tests/fixtures/pipe-contract" + # The test job already compiled these, in the same rust:bookworm image the + # runtime stage is based on. Without this the Dockerfile compiles the whole + # workspace a second time — around fourteen minutes of the run. + - name: Download binaries built by the test job + uses: actions/download-artifact@v4 + with: + name: artifact-linux-docker + + - name: Unpack binaries + run: | + mkdir -p prebuilt + tar -xzf app.tar.gz -C prebuilt + # The build context expects them at its root. + mv prebuilt/stacker/server prebuilt/stacker/console \ + prebuilt/stacker/cleanup-notify prebuilt/stacker/backfill_field_policy \ + prebuilt/ + chmod +x prebuilt/server prebuilt/console \ + prebuilt/cleanup-notify prebuilt/backfill_field_policy + - name: Set up QEMU uses: docker/setup-qemu-action@v3 @@ -235,8 +265,11 @@ jobs: uses: docker/build-push-action@v6 with: context: . + build-args: | + BINARIES=prebuilt build-contexts: | shared_fixtures=${{ github.workspace }}/tests/fixtures + prebuilt_binaries=${{ github.workspace }}/prebuilt push: ${{ github.event_name != 'pull_request' }} tags: ${{ steps.docker_tags.outputs.tags }} diff --git a/Dockerfile b/Dockerfile index a04089a7..8ca9f0c3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,10 +1,22 @@ # syntax=docker/dockerfile:1.4 +# +# Two ways in, selected by the `binaries` build context: +# +# prebuilt — the CI job already compiled the release binaries and passes them +# in. It builds inside this same `rust:bookworm` image, so the +# glibc the binaries link against matches the runtime stage. That +# skips a second full compile of the workspace. +# +# builder — nothing was passed in (a local `docker build`, or CI without the +# artifact). Compiles from source, as before. +# +# Select with `--build-arg BINARIES=prebuilt`. Default is a self-contained build. +ARG BINARIES=builder + FROM rust:bookworm AS builder RUN apt-get update && apt-get install --no-install-recommends -y protobuf-compiler libprotobuf-dev && rm -rf /var/lib/apt/lists/* -RUN cargo install sqlx-cli - WORKDIR /app COPY --from=shared_fixtures / /shared-fixtures # copy manifests @@ -45,6 +57,30 @@ RUN apt-get update && apt-get install --no-install-recommends -y libssl-dev; \ #RUN ls -la /app/target/release/ >&2 +# Config files and the sqlx CLI, needed by both paths. Separate from `builder` +# so the prebuilt path does not drag in a compile of the workspace just to get +# two YAML files. +FROM rust:bookworm AS config +RUN cargo install sqlx-cli --no-default-features --features rustls,postgres +WORKDIR /app +COPY ./docker/local/.env . +COPY ./docker/local/configuration.yaml . + +# The two sources of binaries, each putting them at the image root so the +# production stage copies from one place regardless of which was used. + +# Handed in by CI, already compiled in this same rust:bookworm image. +FROM scratch AS prebuilt-source +COPY --from=prebuilt_binaries / / + +FROM scratch AS builder-source +COPY --from=builder /app/target/release/server /server +COPY --from=builder /app/target/release/console /console +COPY --from=builder /app/target/release/cleanup-notify /cleanup-notify +COPY --from=builder /app/target/release/backfill_field_policy /backfill_field_policy + +FROM ${BINARIES}-source AS binaries + # deploy production FROM debian:bookworm-slim AS production @@ -54,13 +90,13 @@ WORKDIR /app RUN mkdir ./files && chmod 0777 ./files # copy binary and configuration files -COPY --from=builder /app/target/release/server . -COPY --from=builder /app/target/release/console . -COPY --from=builder /app/target/release/cleanup-notify . -COPY --from=builder /app/target/release/backfill_field_policy . -COPY --from=builder /app/.env . -COPY --from=builder /app/configuration.yaml . -COPY --from=builder /usr/local/cargo/bin/sqlx /usr/local/bin/sqlx +COPY --from=binaries /server . +COPY --from=binaries /console . +COPY --from=binaries /cleanup-notify . +COPY --from=binaries /backfill_field_policy . +COPY --from=config /app/.env . +COPY --from=config /app/configuration.yaml . +COPY --from=config /usr/local/cargo/bin/sqlx /usr/local/bin/sqlx COPY ./access_control.conf.dist ./access_control.conf EXPOSE 8000 From a865c3f00e66c0d15032152b5f2c13316f2d6c03 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Wed, 23 Sep 2026 16:08:21 +0300 Subject: [PATCH 19/20] docs: restore the config_contract reference, with names that parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reference documented `required_env:` under `config_contract`. No such field exists — the parser knows `required`, and since every contract type denies unknown fields, anyone following the example would have had their submission rejected. Removing it was right. It took the whole section with it, though, leaving the reference with no description of field policy at all: only the volume subsection added last week remained. An author reading this file would not learn that `mutability` exists. The design document in config/docs covers it, but that is not where someone writing a stacker.yml looks. Restored with the four mutabilities, the keys that apply to each, the legacy three-list shorthand, and a note that publishing is refused until secret-shaped fields carry a policy. Every example here was run through the parser, including the removed `required_env`, which is confirmed to be rejected. Co-Authored-By: Claude Opus 5 --- docs/STACKER_YML_REFERENCE.md | 76 ++++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 20 deletions(-) diff --git a/docs/STACKER_YML_REFERENCE.md b/docs/STACKER_YML_REFERENCE.md index ac7212b1..da7658fe 100644 --- a/docs/STACKER_YML_REFERENCE.md +++ b/docs/STACKER_YML_REFERENCE.md @@ -23,7 +23,6 @@ - [install — Marketplace Install Inputs](#install) - [environments — Named Environments](#environments) - [volumes — Named Volumes](#volumes) -- [config_contract — Service Config Contracts](#config_contract) - [ai — AI Assistant](#ai) - [monitoring — Health & Metrics](#monitoring) - [status_panel](#monitoringstatus_panel) · [healthcheck](#monitoringhealthcheck) · [metrics](#monitoringmetrics) · [alerts](#monitoringalerts) @@ -847,6 +846,62 @@ environments: --- +## `config_contract` + +Declares who controls each of a service's inputs when somebody else installs the +stack. Read at publish time and on the marketplace install path; ignored by a +plain local deploy. + +Without it, the literal values that are correct for *your* deployment — a +`JWT_SECRET`, a database password — are copied verbatim into every buyer's +install, so every buyer and you share one set of credentials. + +```yaml +config_contract: + services: + my-service: # must match a service name, or `app` + fields: + DATABASE_URL: + mutability: fixed # your value ships as-is + LOG_LEVEL: + mutability: editable # your value is a default the buyer may override + LICENSE_KEY: + mutability: provided # the buyer must supply it; yours is never shipped + SECRET_KEY: + mutability: generated # a fresh value per install; the buyer never types it + type: alphanumeric + length: 32 + display: password +``` + +| Key | Applies to | Meaning | +|---|---|---| +| `mutability` | every field | `fixed`, `editable`, `provided` or `generated` — see above | +| `required` | every field | whether a value must resolve at all. Default `true` | +| `type` | `generated` | `hex`, `base64`, `alphanumeric`, `uuid`, `enum`, `derived_jwt` | +| `length` / `min_length` | `generated` | exact or minimum length | +| `values` | `enum` | the allowed set | +| `signing_key`, `claims`, `alg` | `derived_jwt` | `"service.FIELD"` to sign with, the claims, and one of `HS256`/`HS384`/`HS512` | +| `display` | any field | UI hint — `boolean`, `string`, `number`, `password`. Independent of `type` | + +Publishing to the marketplace is refused until every secret-shaped field carries +a `generated` or `provided` policy. + +**Shorthand.** Three plain lists are still accepted and mean +`fixed`+required, `fixed`+optional, and `generated` respectively: + +```yaml +config_contract: + services: + my-service: + required: [DATABASE_URL] + optional: [LOG_LEVEL] + secret: [SECRET_KEY] +``` + +Mixing is fine; an explicit `fields:` entry wins over a list mentioning the same +name. + ### Volume policy in `config_contract` A baked marketplace image is cloned for every buyer, and a volume that travels @@ -905,25 +960,6 @@ Named volumes referenced in `app.volumes` or `services[].volumes` but not listed --- -## `config_contract` - -*Optional* · `object` · Default: none - -Declares service-level configuration contracts — metadata consumed by the TryDirect Install Service and marketplace pipeline to validate and pre-populate service inputs. Not used during local deploys. - -```yaml -config_contract: - services: - my-service: - required_env: - - DATABASE_URL - - SECRET_KEY -``` - -> This section is primarily written by `stacker install` and the marketplace generator. You rarely need to set it by hand. - ---- - ## `ai` *Optional* · `object` · Default: `enabled: false` From 492a29bc6483febeca3cbcf978af3b636326feea Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Wed, 23 Sep 2026 17:10:42 +0300 Subject: [PATCH 20/20] ci: fix what moving the job into a container broke MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The run failed on `sudo: not found`. The container runs as root and does not ship sudo, and it does not need to: `rust:bookworm` already has pkg-config, libssl-dev and a C toolchain, and protoc never comes from the system — `build.rs` points PROTOC at a vendored binary unless one is set. Verified against the image. The step is gone. The error that was actually reported was `no such command: nextest`, which is not what went wrong. Both test steps carried `if: always()`, so they ran after the setup step failed and the nextest install had been skipped. Dropped on the first, narrowed to `success() || failure()` on the second, which is what was wanted: run both suites even if one fails, without reporting on an environment that was never built. Two more, found while looking rather than by the next 47-minute run: `--features explain` applies to the whole `cargo build`, not to the `--bin` it follows, so folding four binaries into one invocation was quietly shipping `server` with explain-logging on — a different binary from the one the image has always carried. Split in two; only `console` and the re-featured casbin dependency recompile. `.dockerignore` is empty, so the unpacked binaries and `app.tar.gz` were being sent to buildkit as part of `context: .` — hundreds of megabytes, twice, eating back the time this change exists to save. They now unpack to `runner.temp`. Co-Authored-By: Claude Opus 5 --- .github/workflows/docker.yml | 46 +++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 28446ec6..8b97853b 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -57,11 +57,11 @@ jobs: echo "PGUSER=postgres" >> "$GITHUB_ENV" echo "PGPASSWORD=postgres" >> "$GITHUB_ENV" - - name: Install OpenSSL and protoc build deps - if: runner.os == 'Linux' - run: | - sudo apt-get update - sudo apt-get install -y pkg-config libssl-dev protobuf-compiler + # No apt step here: `rust:bookworm` already carries pkg-config, libssl-dev + # and a C toolchain, and protoc never comes from the system — `build.rs` + # points PROTOC at a vendored binary unless one is already set. The step + # that used to be here called `sudo`, which the image does not have, and + # does not need: the job runs as root. - name: Verify .sqlx cache exists run: | @@ -118,12 +118,16 @@ jobs: # env vars no longer race and the suite runs in parallel (no more # RUST_TEST_THREADS=1 serialization, no 25-minute timeout). The `bdd` # target uses a custom harness nextest cannot run, so it runs separately. + # Both suites run even if one of them fails, so a single broken test does + # not hide the state of the other. Not `always()`: that also runs them + # after an earlier *setup* step fails, and then reports something + # unrelated — a missing apt package once surfaced as "no such command: + # nextest", because the install step had been skipped. - name: Cargo test - if: ${{ always() }} run: cargo nextest run --tests -E 'not binary(bdd)' - name: Cargo test (bdd suite) - if: ${{ always() }} + if: success() || failure() run: cargo test --test bdd - name: Rustfmt @@ -149,13 +153,18 @@ jobs: # One invocation, so the four binaries share a single compilation of the # workspace instead of four sequential ones. These are the binaries the # runtime image needs; the Docker job copies them rather than rebuilding. + # Two invocations, not one: `--features` applies to the whole command, not + # to the `--bin` it follows. Listing them together builds `server` with + # `explain` too — a differently configured binary from the one the image + # has always shipped. The second call is nearly free; only `console` and + # the re-featured casbin dependency recompile. - name: Build release binaries run: | cargo build --release \ --bin server \ - --bin console --features explain \ --bin cleanup-notify \ --bin backfill_field_policy + cargo build --release --bin console --features explain - name: Set up Node.js if: ${{ hashFiles('web/package.json') != '' }} @@ -229,16 +238,19 @@ jobs: with: name: artifact-linux-docker + # Unpacked outside the workspace: `.dockerignore` is empty, so anything + # left here is sent to buildkit as part of `context: .` — hundreds of + # megabytes of release binaries, twice, eating back the time this change + # exists to save. - name: Unpack binaries run: | - mkdir -p prebuilt - tar -xzf app.tar.gz -C prebuilt - # The build context expects them at its root. - mv prebuilt/stacker/server prebuilt/stacker/console \ - prebuilt/stacker/cleanup-notify prebuilt/stacker/backfill_field_policy \ - prebuilt/ - chmod +x prebuilt/server prebuilt/console \ - prebuilt/cleanup-notify prebuilt/backfill_field_policy + mkdir -p "${{ runner.temp }}/prebuilt" + tar -xzf app.tar.gz -C "${{ runner.temp }}/prebuilt" + cd "${{ runner.temp }}/prebuilt" + mv stacker/server stacker/console stacker/cleanup-notify \ + stacker/backfill_field_policy . + chmod +x server console cleanup-notify backfill_field_policy + rm -f "${GITHUB_WORKSPACE}/app.tar.gz" - name: Set up QEMU @@ -269,7 +281,7 @@ jobs: BINARIES=prebuilt build-contexts: | shared_fixtures=${{ github.workspace }}/tests/fixtures - prebuilt_binaries=${{ github.workspace }}/prebuilt + prebuilt_binaries=${{ runner.temp }}/prebuilt push: ${{ github.event_name != 'pull_request' }} tags: ${{ steps.docker_tags.outputs.tags }}