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(), } }