From 07828d0a387d04fd70f163ca761d9dacb129ecc0 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Mon, 21 Sep 2026 14:37:55 +0300 Subject: [PATCH] fix(bake): close the gaps an audit found in build-box sanitization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to 27017fa0. An adversarial review of that change found the sanitize step could destroy the build box, refuse healthy deploys, or report success over an image it had not actually checked. Volume reset was enumerating every volume on the host, not the project's. On a real build box that removes the ingress' certificates and the agent's state, and aborts the bake on the first volume still held by a running container. Scoped to the project's own compose and matched through Compose's own label; keep entries are validated and matched on whole segments. The required-key list was read from the text of the compose file, so `${VAR:-default}` and `$$`-escaped text counted as required and no buyer could ever satisfy them. It now comes from the contract — the only thing that has a source on a buyer's machine. References the contract does not cover are resolved back to their literals before the snapshot, since a buyer's env file is replaced wholesale and would leave them empty. Further: - config_contract is now the only authority on what is sensitive; the weaker name heuristic is gone. A credential embedded inside a larger value (a password inside a DSN) is cleared in .env as well as in compose. - The author's own access no longer survives: authorized_keys, private keys, known_hosts and registry credentials are removed alongside the machine identity. Cloud-init appends the buyer's key rather than replacing the file, so a key left behind would grant root on every server cloned from the image. - The tear-down runs before the rewrites, so a cleared .env cannot fail `docker compose down` with the files already modified. - Files are written in chunks, so a large compose no longer exceeds the argument-length limit mid-sanitize; error messages no longer echo the payload. - A failure reports which steps completed and whether a retry against the same box is still equivalent. - The bake refuses when no contract resolved, when a compose reads values through `env_file:` that a buyer would silently lose, on an unknown argument, and when the contract describes a different version. - Value-stripping no longer clears the field policies inside config_contract itself. Adds scripts/check-staged-secrets.sh, wired as a pre-commit check — the scanner already configured in .pre-commit-config.yaml was never installed, so nothing was checking. Adds docs/SECRET_LIFECYCLE.md, tracing where one value lives at each stage and which component owns it. 2083 unit tests and 329 BDD scenarios green. Co-Authored-By: Claude Opus 5 --- .pre-commit-config.yaml | 6 + docs/ONE_CLICK_DEPLOY.md | 26 +- docs/SECRET_LIFECYCLE.md | 172 +++++ scripts/check-staged-secrets.sh | 182 +++++ src/bin/bake.rs | 43 +- src/cli/generator/compose.rs | 319 ++++++++- src/helpers/bake_finalize.rs | 858 +++++++++++++++++++++-- src/helpers/redact.rs | 96 ++- tests/features/bake_sanitization.feature | 331 +++++++++ tests/steps/bake_sanitization.rs | 603 ++++++++++++++++ tests/steps/mod.rs | 4 + 11 files changed, 2548 insertions(+), 92 deletions(-) create mode 100644 docs/SECRET_LIFECYCLE.md create mode 100755 scripts/check-staged-secrets.sh create mode 100644 tests/features/bake_sanitization.feature create mode 100644 tests/steps/bake_sanitization.rs diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c4e0b886..d394d9d6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,6 +7,12 @@ repos: stages: [commit] - repo: local hooks: + - id: check-staged-secrets + name: no concrete secret values in the commit + entry: scripts/check-staged-secrets.sh + language: script + pass_filenames: false + stages: [commit] - id: cargo-fmt name: cargo fmt --all entry: cargo fmt --all diff --git a/docs/ONE_CLICK_DEPLOY.md b/docs/ONE_CLICK_DEPLOY.md index f454403a..ee38626f 100644 --- a/docs/ONE_CLICK_DEPLOY.md +++ b/docs/ONE_CLICK_DEPLOY.md @@ -61,11 +61,31 @@ stacker/ StackerConfig::from_str+validate_semantics registry lookup → cloud - 200 `{ valid, name, version, composition {app, services[]} }`; 422 `{ valid:false, errors[], warnings[] }`. - Casbin `group_anonymous` rule (new migration, pattern `20260726120000_casbin_audit_public_rules.up.sql`). +> For how one secret value travels from the author's machine into a buyer's +> clone — and which component owns each step — see +> [SECRET_LIFECYCLE.md](SECRET_LIFECYCLE.md). + ### 2. `baked_snapshots` registry -- Migration `bake_snapshots` (stack, version, provider, image_id, healthy, digests JSONB, created_at). +- Columns: `stack`, `version`, `provider`, `image_id`, `healthy`, `digests` JSONB, + `created_at`, plus two added later: + - `config_contract` JSONB (`20260911120000`) — the author's field policy pinned to + the image, so the clone path regenerates `mutability: generated` fields per buyer + instead of every clone inheriting the one value baked at bake time. + - `required_env_keys` JSONB (`20260919120000`) — the `${VAR}` names the baked compose + references. The clone path refuses a deploy whose environment cannot satisfy them, + because Compose resolves an unsatisfied reference to an empty string with only a + warning. NULL means the check is skipped: either the snapshot predates the column, + or it was baked with `--allow-unsanitized-snapshot`. - `src/db/baked_snapshot.rs`, `src/models/baked_snapshot.rs`: `resolve`/`record`. -- Extend `src/bin/bake.rs` / `src/helpers/bake.rs` to persist the `BakeRecord`. -- Run `cargo sqlx prepare` after any sqlx query change. + Both reads are `SELECT *`. `required_env_keys` carries `#[sqlx(default)]`, so it + tolerates a database that has not run its migration yet; `config_contract` does + **not**, so `resolve()` fails outright against a database missing that column. +- `src/bin/bake.rs` / `src/helpers/bake.rs` persist the `BakeRecord`. Before snapshotting, + `src/helpers/bake_finalize.rs` sanitizes the build box over SSH (blank the co-located + `.env`, parameterize secrets embedded in compose values, drop credential-bearing data + volumes, strip SSH host keys / machine-id / cloud-init state) — hence `bake --ssh-key`. +- These three queries use runtime `sqlx::query_as`, not the compile-time macros, so they + need no `.sqlx` entry. Run `cargo sqlx prepare` after changing any *macro* query. ### 3. `POST /api/v1/deploy/clone` (protected) - Request `{ stack, version, region, server_type, domain, admin_email, env{} }`. diff --git a/docs/SECRET_LIFECYCLE.md b/docs/SECRET_LIFECYCLE.md new file mode 100644 index 00000000..1c0bed3b --- /dev/null +++ b/docs/SECRET_LIFECYCLE.md @@ -0,0 +1,172 @@ +# Lifecycle of one secret value + +Tracing a single value — a database password — from the author's laptop to a +buyer's cloned server. This is the reference for deciding **when a literal must +become a `${VAR}` reference**, and which component is responsible at each point. + +Related: [FIELD_POLICY.md](FIELD_POLICY.md) (author-facing guide to declaring the +policy) and [ONE_CLICK_DEPLOY.md](ONE_CLICK_DEPLOY.md) (the clone path). + +## The journey + +`` below stands for the author's literal password. + +| # | Stage | Where it runs | Form of the value | +|---|-------|---------------|-------------------| +| 1 | Author's `.env` | author's machine | `POSTGRES_PASSWORD=` | +| 2 | `stacker.yml` parsed, `${...}` expanded | **stacker CLI** (`src/cli/config_parser.rs`, `resolve_env_vars_with_fallback`) | literal | +| 3 | Compose rendered | **stacker CLI** (`src/console/commands/cli/deploy.rs`, `ComposeDefinition::try_from`) | literal | +| 4 | Protected keys turned back into references | **stacker CLI** (`parameterize_compose_env_vars`) | `${POSTGRES_PASSWORD}` | +| 5 | Config bundle posted to the backend | stacker CLI → **stacker server** (`src/routes/project/deploy.rs`) | compose with references, `.env` with literals | +| 6 | Queued, then Terraform + Ansible | **install service** (`src/connectors/install_service/client.rs`) | unchanged | +| 7 | Files land in `/home/trydirect//` | **target machine** (build box) | compose + co-located `.env` | +| 8 | `docker compose up` | **target machine** | literal, resolved from `./.env` — **and Postgres writes it into its data directory** | +| 9 | Finalize before the snapshot | `bake` binary, over SSH to the **target machine** (`src/helpers/bake_finalize.rs`) | stack stopped and data volumes dropped, embedded secrets replaced, references outside the contract put back to their values, `.env` cleared, machine identity and the author's own access stripped | +| 10 | Snapshot taken | Hetzner API → **baked image** | only `${POSTGRES_PASSWORD}`; no value anywhere | +| 11 | Cloud-init rendered for a clone | **stacker server** (`src/routes/oneclick_deploy/clone.rs`) | a fresh value per buyer | +| 12 | First boot: `/etc/stacker/env` copied over `./.env` | **buyer's machine** | new literal | +| 13 | `docker compose up` | **buyer's machine** | Postgres initializes from scratch | + +Stage 8 is the one that surprises people: the value does not only sit in files, +it is written **into the data directory**. Replacing it in compose is therefore +not enough — the volume has to be dropped at stage 9, or the buyer's server +keeps authenticating with the author's password. + +## When a literal needs a reference + +A value must become `${VAR}` when **both** hold: + +1. it survives the snapshot (it is in a file, or in a volume that is kept), and +2. it must differ per buyer. + +If only the first holds, leave it literal (`OLLAMA_MODEL: llama3.1`). If only the +second holds, there is nothing to replace. + +The mirror rule matters just as much: **every `${VAR}` baked into the image must +have something that fills it on the buyer's machine.** There are exactly two +such sources. + +| Class | Form in the baked compose | Filled on the buyer's machine by | Produced at | +|-------|---------------------------|----------------------------------|-------------| +| contract `generated` | `${VAR}` | cloud-init regeneration commands | stage 11, stacker server | +| contract `provided` | `${VAR}` | the buyer's submitted values | stage 11, stacker server | +| contract `fixed` | literal | nothing needed | — | +| not declared in the contract | literal | nothing needed | — | +| `${X:-default}` | left as written | its own default | stage 13, buyer's machine | +| `$$`-escaped | left as written | never interpolated at all | — | + +`config_contract` is the only authority on what is sensitive. Nothing is guessed +from variable names: name heuristics both miss real secrets and clear harmless +values, and the author already stated which fields matter. + +### Variables the author parameterized outside the contract + +An author may write `${SOMETHING}` in `stacker.yml` for a value that is not a +contract field. That reference is expanded at stage 2, but stage 4 turns plain +top-level `env:` keys back into references — and nothing fills those on the +buyer's machine. **Stage 9 therefore puts their value back**, so the image holds +a literal: the same for every buyer, which is exactly what a non-contract value +is. `${X:-default}` and `$$`-escaped text are left alone, since neither needs a +source. + +A reference with no value on the build box and no default fails the bake rather +than shipping an image that boots with an empty string. + +The single exception is a literal that *contains* a contract secret, such as +`DATABASE_URL=postgresql://user:@db:5432/app`. Stage 9 rewrites the embedded +part to `${POSTGRES_PASSWORD}`, because the surrounding key name (`DATABASE_URL`) +is not something any policy mentions, and a name-based rule cannot see it. + +Consequence worth accepting deliberately: a secret the author never declared +stays in the image. That follows directly from making the contract the only +authority. + +## What else must not survive the snapshot + +A secret value is not the only thing a snapshot carries forward. Two more are +removed at stage 9, both for the same reason — they are specific to the author's +machine, and a snapshot turns "specific to one machine" into "shared by every +buyer": + +| What | Why it matters | +|------|----------------| +| SSH host keys, `machine-id`, cloud-init instance state | every clone would present the same host identity, so one buyer can impersonate another buyer's server | +| `authorized_keys`, private keys, `known_hosts`, `~/.docker/config.json` | cloud-init *appends* the buyer's key rather than replacing the file, so an author key left in the image grants its holder root on every server cloned from it | + +`sshd` regenerates host keys on first boot when none are present, and `systemd` +repopulates an empty `machine-id`, so removal is enough — nothing has to be +recreated. + +Note the consequence for the operator: after stage 9 the build box no longer +accepts the bake key, so a failed bake cannot be retried by reconnecting to the +same machine. Start from a fresh build box instead. + +### Order of the finalize steps + +The tear-down runs **before** the files are rewritten. `docker compose down` +parses the compose, and a cleared `.env` would make any `${VAR}` outside an +environment block — `image: ${REGISTRY}/app:${TAG}`, `ports: ["${PORT}:80"]` — +resolve to empty and fail the tear-down, with the files already modified and +nothing to roll back to. Machine identity goes last, because after it the box no +longer accepts the bake key. + +### When a finalize fails part-way + +The steps are not transactional and most are not reversible, so a failure +reports what already completed. Re-running against the same box is rarely +equivalent: the compose is already sanitized, so the embedded-secret scan has +nothing left to find; the `.env` is already cleared, so there are no values to +search for; the data volumes are gone; and after the identity reset the box no +longer accepts the bake key at all. In every one of those cases the answer is a +fresh build box, and the error says so rather than leaving it to be discovered. + +### Values a clone would lose + +A service declaring `env_file:` reads its values out of that file, not through +`${VAR}`. Stage 12 replaces the file wholesale, so anything in it that is not a +contract field disappears on the buyer's machine — the container then starts +without values it had on the build box. The bake stops and names them; the fix +is to move those values into an `environment:` block, where they become part of +the image. + +Stacker's own generator writes values into `environment:`, so this only arises +with a hand-written `deploy.compose_file`. Both spellings of that block are +covered: + +```yaml +environment: environment: + KEY: value - KEY=value +``` + +## When the bake refuses + +Sanitizing depends entirely on the contract resolving. If it does not — the +template is not approved yet, `DATABASE_URL` is unset, the query failed, or the +author declared no fields — then nothing is substituted, nothing is cleared, and +an image carrying the author's values would be published with a confident +"Sanitized" line. The bake therefore stops unless `--allow-unsanitized-snapshot` +says the stack genuinely has no secrets. + +A project that ships no `.env` is normal and does **not** stop the bake: a +reference the buyer's machine cannot fill is already caught on its own. What the +bake does say out loud is that the embedded-secret scan had no values to search +for, since a mistyped `--project-dir` looks identical from here. + +## A reference with no source + +When a `${VAR}` is baked in and nothing fills it, three different stages are +involved: + +| | Stage | What happens | +|---|-------|--------------| +| **Created** | 4 — stacker CLI | the value is turned into a reference that nothing will fill later | +| **Caught** | 11 — stacker server | the clone compares the image's references against what will actually arrive, and refuses before creating a server | +| **Would surface** | 13 — buyer's machine | Compose substitutes an empty string and only warns; the systemd unit still reports active while the stack is broken | + +The refusal at stage 11 is therefore not noise — it covers the silent failure at +stage 13. The fix belongs at stage 4: never create a reference that has no source. + +Both sets are derived from the same place — the contract +(`required_env_keys` in `src/cli/generator/compose.rs`). Deriving the required +list from the *text* of the compose file instead pulls in things that need no +source at all (`${X:-default}`, `$$`-escaped text) and misses the ones that do. diff --git a/scripts/check-staged-secrets.sh b/scripts/check-staged-secrets.sh new file mode 100755 index 00000000..13c81b99 --- /dev/null +++ b/scripts/check-staged-secrets.sh @@ -0,0 +1,182 @@ +#!/usr/bin/env bash +# +# Refuse a commit that introduces a concrete secret value. +# +# This repository is public: a value committed here is disclosed permanently, +# and rotating it afterwards does not un-publish it. A real password reached +# `src/cli/generator/compose.rs` this way — the scanner configured in +# .pre-commit-config.yaml was never installed, so nothing checked. +# +# Deliberately narrow, because a noisy hook gets bypassed: +# 1. an assignment to a secret-named key whose value looks concrete +# 2. a long hex/base64 blob used as a value +# +# Placeholders, ${REFERENCES} and empty values are fine — those are what test +# fixtures and templates should contain. +# +# Opt out for a line that is genuinely not a secret: +# API_KEY=abcdef0123456789abcdef # pragma: allowlist secret +# +# Scan the whole tree instead of the staged diff with --all. + +set -uo pipefail + +RED=$'\033[31m'; YELLOW=$'\033[33m'; RESET=$'\033[0m' +[ -t 1 ] || { RED=''; YELLOW=''; RESET=''; } + +if [ "${1:-}" = "--all" ]; then + added=$(git grep -n '' -- . | sed 's/^/+/') +else + # Only added lines of the staged diff, with their file and line numbers. + added=$(git diff --cached --unified=0 --no-color -- . | awk ' + /^\+\+\+ b\// { file = substr($0, 7); next } + /^@@/ { + match($0, /\+[0-9]+/) + line = substr($0, RSTART + 1, RLENGTH - 1) - 1 + next + } + /^\+/ && !/^\+\+\+/ { line++; print file ":" line ":" substr($0, 2) } + ') +fi + +[ -n "$added" ] || exit 0 + +SECRET_KEY_RE='(PASSWORD|PASSWD|PWD|SECRET|TOKEN|API_?KEY|ACCESS_KEY|CREDENTIAL|PRIVATE_KEY|MASTERKEY|SIGNING_KEY|ENCRYPTION_KEY)' + +findings=$(printf '%s\n' "$added" | awk -v keyre="$SECRET_KEY_RE" ' + # An explicit opt-out wins. + /pragma: allowlist secret/ { next } + + { + # Split "path:line:content" while keeping colons inside the content. + i = index($0, ":"); path = substr($0, 1, i - 1) + rest = substr($0, i + 1) + j = index(rest, ":"); lineno = substr(rest, 1, j - 1) + content = substr(rest, j + 1) + } + + # The value side of KEY=value or "KEY: value". + { + value = "" + if (match(content, /[A-Za-z_][A-Za-z0-9_]*[[:space:]]*[=:][[:space:]]*/)) { + key = substr(content, RSTART, RLENGTH) + value = substr(content, RSTART + RLENGTH) + } + } + + { + gsub(/^["\x27[:space:]]+|["\x27,[:space:]]+$/, "", value) + } + + # Placeholders and references are exactly what belongs in a template. + value == "" { next } + value ~ /^\$/ { next } + value ~ /^<.*>$/ { next } + tolower(value) ~ /^(changeme|change-me|placeholder|example|redacted|secret|password|test|dummy|xxx+|\*+)$/ { next } + # Self-describing placeholders: your_x_here, x_goes_here, SHOULD_BE_*, TODO. + tolower(value) ~ /^(your|my|some)_/ { next } + tolower(value) ~ /_here$|_goes_here$|^todo|^fixme|should_be/ { next } + # A value equal to its own key name is a template, not a credential. + toupper(value) == toupper(keyname(key)) { next } + # Well-known defaults that belong to nobody. + tolower(value) ~ /^(postgres|mysql|root|admin|guest|user|local|localhost|none|null)$/ { next } + value ~ /^(.)\1+$/ { next } + + # 0. Credentials inside a URL. The key name says nothing here + # (DATABASE_URL, REDIS_URL, AMQP_URL), so nothing else catches it — + # this is the shape that actually leaked. + content ~ /[a-zA-Z][a-zA-Z0-9+.-]*:\/\/[^:\/@[:space:]]+:[^@[:space:]]+@/ { + # A ${REFERENCE} or a in the password position is the + # shape we want authors to use, not a leak. + if (content !~ /:\/\/[^:\/@[:space:]]+:[\$<]/ && + !looks_synthetic(url_password(content)) && + !is_placeholder(url_password(content))) { + print path ":" lineno ": credentials embedded in a URL" + next + } + } + + # A secret value is a single opaque token. Anything with whitespace or code + # punctuation is a line of source, not an assignment of a credential — + # `const MIN_SECRET_LEN: usize = 12;` must not stop a commit. + value ~ /[[:space:]]/ { next } + value ~ /[(){}<>;,]|::/ { next } + + # Obvious test fixtures: a real secret is not a repeated block. + looks_synthetic(value) { next } + + # 1. A secret-named key carrying something that looks generated. Generated + # credentials mix digits and letters (hex, base64, alphanumeric); a + # descriptive fixture word like `author-value` does not. + toupper(key) ~ keyre && length(value) >= 8 && + ((value ~ /[0-9]/ && value ~ /[A-Za-z]/) || length(value) >= 20) { + print path ":" lineno ": secret-named key with a literal value" + next + } + + # 2. A long hex or base64 blob as a value, whatever the key is called. + value ~ /^[0-9a-fA-F]{32,}$/ { + print path ":" lineno ": " length(value) "-char hex value" + next + } + value ~ /^[A-Za-z0-9+\/]{40,}={0,2}$/ { + print path ":" lineno ": long base64-looking value" + } + + # The bare key name from a "KEY=" / "KEY:" capture. + function keyname(k) { + gsub(/[[:space:]=:]+$/, "", k) + return k + } + + # Placeholder credentials: well-known defaults and self-describing stand-ins. + function is_placeholder(v) { + v = tolower(v) + if (v ~ /^(postgres|mysql|root|admin|guest|user|password|changeme|secret|example|test)$/) return 1 + if (v ~ /^(your|my|some)_/) return 1 + if (v ~ /_here$|_goes_here$|should_be/) return 1 + return 0 + } + + # The credential between "//user:" and "@" of the first URL on the line. + function url_password(line, rest, at) { + if (!match(line, /:\/\/[^:\/@[:space:]]+:/)) return "" + rest = substr(line, RSTART + RLENGTH) + at = index(rest, "@") + return at ? substr(rest, 1, at - 1) : rest + } + + # True for values that no random generator would produce: a repetition of a + # shorter block (0123456789abcdef0123456789abcdef) or a single repeated + # character. Test fixtures need secret-shaped values; real secrets are not + # shaped like this. + function looks_synthetic(v, n, half, i) { + if (v == "") return 0 + if (v ~ /^(.)\1+$/) return 1 + n = length(v) + for (half = 1; half <= n / 2; half++) { + if (n % half != 0) continue + if (v == repeat(substr(v, 1, half), n / half)) return 1 + } + return 0 + } + + function repeat(unit, times, out, i) { + out = "" + for (i = 0; i < times; i++) out = out unit + return out + } +') + +[ -n "$findings" ] || exit 0 + +echo "${RED}Commit refused: a concrete secret value would be committed.${RESET}" >&2 +echo >&2 +printf '%s\n' "$findings" | sed 's/^/ /' >&2 +echo >&2 +echo "${YELLOW}This repository is public — committing a value discloses it permanently," >&2 +echo "and rotating afterwards does not un-publish it.${RESET}" >&2 +echo >&2 +echo "Use a \${REFERENCE} or a placeholder. If the line is genuinely not a secret:" >&2 +echo " append # pragma: allowlist secret" >&2 +exit 1 diff --git a/src/bin/bake.rs b/src/bin/bake.rs index 547d5a96..17558565 100644 --- a/src/bin/bake.rs +++ b/src/bin/bake.rs @@ -74,8 +74,15 @@ async fn main() -> Result<(), Box> { i += 1; } other => { - eprintln!("ignoring unknown arg: {other}"); - i += 1; + // Shrugging this off is how `--sshkey` silently became "no + // --ssh-key", and a mistyped `--stack` silently bakes under the + // default slug — pinning the wrong contract to the image. + return Err(format!( + "unknown argument `{other}`. Supported: --ip, --server-id, --stack, \ + --version, --health-url, --ssh-key, --ssh-user, --project-dir, \ + --allow-unsanitized-snapshot" + ) + .into()); } } } @@ -119,7 +126,7 @@ async fn main() -> Result<(), Box> { }; let config_contract = match &pool { - Some(pool) => resolve_config_contract(pool, &stack).await, + Some(pool) => resolve_config_contract(pool, &stack, &version).await, None => None, }; let protected_keys = config_contract @@ -127,6 +134,12 @@ async fn main() -> Result<(), Box> { .map(stacker::helpers::bake_finalize::protected_keys_from_contract) .unwrap_or_default(); + // Refuse before touching the box: with no contract there is nothing to + // sanitize, and publishing anyway is how the author's credentials reach + // every buyer. + stacker::helpers::bake_finalize::check_contract_usable(&protected_keys, allow_unsanitized) + .map_err(|e| e.to_string())?; + // Sanitize the build box before the snapshot is taken. let finalize_outcome = match (&ssh_key, allow_unsanitized) { (Some(key_path), _) => { @@ -226,11 +239,29 @@ async fn main() -> Result<(), Box> { /// The author's field policy for `stack`, resolved by the same slug the /// snapshot registry keys on (`baked_snapshots.stack == stack_template.slug`). /// -/// Best-effort: an un-catalogued or unapproved stack bakes with no contract and -/// the clone path degrades to the baked values. -async fn resolve_config_contract(pool: &sqlx::PgPool, stack: &str) -> Option { +/// `get_config_contract` reads the template's *latest* version. Baking some +/// other version would pin the wrong field set to the image, so the versions +/// are compared and a mismatch stops the bake rather than shipping a snapshot +/// whose contract describes a different release. +async fn resolve_config_contract( + pool: &sqlx::PgPool, + stack: &str, + version: &str, +) -> Option { match stacker::db::marketplace::get_approved_by_slug(pool, stack).await { Ok(Some(template)) => { + match stacker::db::marketplace::get_by_slug_with_latest(pool, stack).await { + Ok((_, Some(latest))) if latest.version != version => { + eprintln!( + "WARNING: baking '{stack}' v{version}, but the marketplace's latest \ + version is v{}. The contract describes the latest version, so it \ + would not match this image — resubmit or bake the latest version.", + latest.version + ); + return None; + } + _ => {} + } match stacker::db::marketplace::get_config_contract(pool, template.id).await { Ok(serde_json::Value::Null) => { eprintln!("WARNING: template '{stack}' declares no config_contract — nothing will be regenerated per buyer."); diff --git a/src/cli/generator/compose.rs b/src/cli/generator/compose.rs index 75eeb85f..203eb68d 100644 --- a/src/cli/generator/compose.rs +++ b/src/cli/generator/compose.rs @@ -1,3 +1,4 @@ +use std::collections::BTreeSet; use std::collections::HashMap; use std::convert::TryFrom; use std::fmt; @@ -819,8 +820,26 @@ pub fn parameterize_compose_env_vars( } if in_environment { - // Match " KEY: value" — the key must be a valid env identifier. - if let Some((key, _rest)) = trimmed.split_once(':') { + // Compose accepts either form inside `environment:`; a hand-written + // compose (`deploy.compose_file`) commonly uses the list one. + // + // environment: environment: + // KEY: value - KEY=value + if let Some(entry) = trimmed.strip_prefix("- ") { + if let Some((key, _value)) = entry.split_once('=') { + let key = key.trim(); + if is_env_identifier(key) && env_keys.contains(key) { + result.push_str(&line[..indent]); + result.push_str("- "); + result.push_str(key); + result.push_str("=${"); + result.push_str(key); + result.push_str("}\n"); + continue; + } + } + } else if let Some((key, _rest)) = trimmed.split_once(':') { + // Match " KEY: value" — the key must be a valid env identifier. let key = key.trim(); if is_env_identifier(key) && env_keys.contains(key) { // Preserve the original indent and replace the value. @@ -1032,12 +1051,24 @@ pub fn collect_env_var_references(compose_content: &str) -> std::collections::BT let mut i = 0usize; while i + 1 < bytes.len() { + // `$$` is Compose's escape — the text is emitted literally and never + // interpolated, so neither `$` may start a reference. + if bytes[i] == b'$' && bytes[i + 1] == b'$' { + i += 2; + continue; + } if bytes[i] != b'$' || bytes[i + 1] != b'{' { i += 1; continue; } - let Some(end) = compose_content[i + 2..].find('}') else { - break; + // A reference never spans lines: bound the search so an unterminated + // `${` cannot swallow the next line's reference along with it. + let Some(end) = closing_brace_on_line(&compose_content[i + 2..]) else { + // Unterminated `${` — skip it and keep scanning. Abandoning the rest + // of the document here would silently under-report the references + // that follow. + i += 2; + continue; }; let raw = &compose_content[i + 2..i + 2 + end]; // Compose allows ${VAR:-default} / ${VAR-default} / ${VAR:?err}. @@ -1051,6 +1082,117 @@ pub fn collect_env_var_references(compose_content: &str) -> std::collections::BT names } +/// A `${VAR}` in the baked compose that nothing will fill on the buyer's machine. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct UnresolvedReference { + pub name: String, +} + +/// Put back the literal value of every `${VAR}` the contract does **not** cover, +/// and report the ones that cannot be resolved. +/// +/// Stage 4 (`parameterize_compose_env_vars`, in the CLI) turns two different +/// things into references: contract fields, and plain top-level `env:` keys. For +/// an ordinary deploy both are fine — the `.env` travels next to the compose and +/// stays intact. For a baked image they are not: at first boot the buyer's +/// machine overwrites that `.env` wholesale from `/etc/stacker/env`, which only +/// ever holds contract fields and the buyer's own values. A reference to +/// anything else would resolve to an empty string, and Compose only warns. +/// +/// So the image keeps references *only* for values that have a source on the +/// buyer's machine, and everything else goes back to being a literal — which is +/// correct, because those values are the same for every buyer. +/// +/// Left untouched: `$$`-escaped text (Compose never interpolates it) and +/// `${VAR:-default}` forms that carry their own fallback. +pub fn resolve_non_contract_references( + compose_content: &str, + env_values: &std::collections::BTreeMap, + protected: &BTreeSet, +) -> (String, Vec) { + let mut out = String::with_capacity(compose_content.len()); + let mut unresolved = Vec::new(); + let bytes = compose_content.as_bytes(); + let mut i = 0usize; + + while i < bytes.len() { + // `$$` is Compose's escape: it is emitted literally, never interpolated. + // Copy both bytes untouched so we do not mistake the second `$` for the + // start of a reference. + if bytes[i] == b'$' && i + 1 < bytes.len() && bytes[i + 1] == b'$' { + out.push_str("$$"); + i += 2; + continue; + } + + if bytes[i] != b'$' || i + 1 >= bytes.len() || bytes[i + 1] != b'{' { + out.push(compose_content[i..].chars().next().unwrap_or('\0')); + i += compose_content[i..] + .chars() + .next() + .map(char::len_utf8) + .unwrap_or(1); + continue; + } + + let Some(end) = closing_brace_on_line(&compose_content[i + 2..]) else { + // Unterminated `${` — copy it and carry on rather than abandoning + // the rest of the document. + out.push_str("${"); + i += 2; + continue; + }; + + let raw = &compose_content[i + 2..i + 2 + end]; + let whole = &compose_content[i..i + 2 + end + 1]; + let has_default = raw.contains(":-") || raw.contains(":?") || raw.contains('-'); + let name = raw.split([':', '-', '?', '+']).next().unwrap_or("").trim(); + + if !is_env_identifier(name) || protected.contains(name) { + // Not a reference we manage, or one the buyer's machine will fill. + out.push_str(whole); + } else if let Some(value) = env_values.get(name) { + out.push_str(value); + } else if has_default { + // Compose supplies the fallback itself; nothing is missing. + out.push_str(whole); + } else { + out.push_str(whole); + unresolved.push(UnresolvedReference { + name: name.to_string(), + }); + } + + i += 2 + end + 1; + } + + unresolved.sort_by(|a, b| a.name.cmp(&b.name)); + unresolved.dedup(); + (out, unresolved) +} + +/// The contract fields the sanitized compose still references — exactly the set +/// the buyer's machine has to fill. +/// +/// Derived from the contract rather than from the text of the file: a reference +/// is only "required" if something is expected to supply it, and the only +/// suppliers are the regeneration commands and the buyer's own values, both of +/// which are driven by the contract. +pub fn required_env_keys(compose_content: &str, protected: &BTreeSet) -> BTreeSet { + collect_env_var_references(compose_content) + .into_iter() + .filter(|name| protected.contains(name)) + .collect() +} + +/// Offset of the `}` that closes a `${` opened at the start of `rest`, or +/// `None` when the line ends first — a reference never spans lines, and letting +/// the search run on would consume the next line's reference too. +fn closing_brace_on_line(rest: &str) -> Option { + let line_end = rest.find('\n').unwrap_or(rest.len()); + rest[..line_end].find('}') +} + /// Returns `true` when `s` looks like a POSIX env-variable name. fn is_env_identifier(s: &str) -> bool { !s.is_empty() @@ -2157,10 +2299,10 @@ services: ports: - \"8080:8000\" environment: - ADMIN_PASSWORD: 4f4237dd9bfe8e1622706cac7bab63c7 + ADMIN_PASSWORD: fedcba9876543210fedcba9876543210 ADMIN_USER: admin - DATABASE_URL: postgresql://stackpilot:2213a996143863b99a0f2d3e22907690@db:5432/stackpilot - SECRET_KEY: b838f1f22379b8c268a4d3e0268459761c18947e1576956a6d9b1f3928070df4 + DATABASE_URL: postgresql://stackpilot:0123456789abcdef0123456789abcdef@db:5432/stackpilot + SECRET_KEY: 00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff OLLAMA_MODEL: llama3.1 restart: unless-stopped "; @@ -2243,9 +2385,9 @@ services: services: app: environment: - DATABASE_URL: postgresql://stackpilot:2213a996143863b99a0f2d3e22907690@db:5432/stackpilot + DATABASE_URL: postgresql://stackpilot:0123456789abcdef0123456789abcdef@db:5432/stackpilot "; - let env = env_map(&[("POSTGRES_PASSWORD", "2213a996143863b99a0f2d3e22907690")]); + let env = env_map(&[("POSTGRES_PASSWORD", "0123456789abcdef0123456789abcdef")]); let result = parameterize_embedded_secret_values(compose, &env, &key_set(&["POSTGRES_PASSWORD"])) .expect("no conflict"); @@ -2255,7 +2397,7 @@ services: "password replaced in place:\n{result}" ); assert!( - !result.contains("2213a996143863b99a0f2d3e22907690"), + !result.contains("0123456789abcdef0123456789abcdef"), "no literal left:\n{result}" ); } @@ -2296,9 +2438,9 @@ services: services: db: environment: - POSTGRES_PASSWORD: 2213a996143863b99a0f2d3e22907690 + POSTGRES_PASSWORD: 0123456789abcdef0123456789abcdef "; - let env = env_map(&[("DB_PASSWORD", "2213a996143863b99a0f2d3e22907690")]); + let env = env_map(&[("DB_PASSWORD", "0123456789abcdef0123456789abcdef")]); let err = parameterize_embedded_secret_values( compose, &env, @@ -2317,16 +2459,16 @@ services: let compose = "\ services: app: - image: myapp:2213a996143863b99a0f2d3e22907690 + image: myapp:0123456789abcdef0123456789abcdef environment: - TOKEN: 2213a996143863b99a0f2d3e22907690 + TOKEN: 0123456789abcdef0123456789abcdef "; - let env = env_map(&[("TOKEN", "2213a996143863b99a0f2d3e22907690")]); + let env = env_map(&[("TOKEN", "0123456789abcdef0123456789abcdef")]); let result = parameterize_embedded_secret_values(compose, &env, &key_set(&["TOKEN"])) .expect("no conflict"); assert!( - result.contains("image: myapp:2213a996143863b99a0f2d3e22907690"), + result.contains("image: myapp:0123456789abcdef0123456789abcdef"), "image digest untouched:\n{result}" ); assert!( @@ -2335,6 +2477,25 @@ services: ); } + #[test] + fn collects_env_var_reference_skips_escaped_and_survives_unterminated() { + // `$$` is Compose's escape — the container sees literal `${HOME}` and + // nothing is interpolated, so it is not a reference. + let escaped = "services:\n app:\n command: sh -c 'echo $${HOME}'\n"; + assert!( + collect_env_var_references(escaped).is_empty(), + "escaped text must not count as a reference" + ); + + // A stray `${` must not discard everything after it. + let broken = + "services:\n app:\n environment:\n A: ${BROKEN\n B: ${REAL_ONE}\n"; + assert!( + collect_env_var_references(broken).contains("REAL_ONE"), + "scanning must continue past an unterminated reference" + ); + } + #[test] fn collects_env_var_references_with_defaults_and_ignores_literals() { let compose = "\ @@ -2359,6 +2520,132 @@ services: assert!(collect_env_var_references(compose).contains("PW")); } + // ── references that survive into the baked image ─────────────────────── + + #[test] + fn non_contract_reference_goes_back_to_its_literal() { + // A top-level `env:` key is turned into a reference at deploy time, but + // nothing fills it on the buyer's machine — the .env there is replaced + // wholesale from /etc/stacker/env, which only holds contract fields. + let compose = "services:\n app:\n environment:\n REGION: ${REGION}\n"; + let env = env_map(&[("REGION", "fsn1")]); + + let (out, unresolved) = + resolve_non_contract_references(compose, &env, &std::collections::BTreeSet::new()); + + assert!( + out.contains("REGION: fsn1"), + "put back as a literal:\n{out}" + ); + assert!(unresolved.is_empty(), "nothing missing: {unresolved:?}"); + } + + #[test] + fn contract_reference_is_kept_as_a_reference() { + let compose = + "services:\n db:\n environment:\n POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}\n"; + let env = env_map(&[("POSTGRES_PASSWORD", "aaaaaaaaaaaaaaaa")]); + + let (out, _) = + resolve_non_contract_references(compose, &env, &key_set(&["POSTGRES_PASSWORD"])); + + assert!( + out.contains("POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}"), + "the buyer's machine fills this one:\n{out}" + ); + assert!( + !out.contains("aaaaaaaaaaaaaaaa"), + "the author's value must not come back:\n{out}" + ); + } + + #[test] + fn escaped_and_defaulted_references_are_left_untouched() { + let compose = "services:\n app:\n environment:\n LOG: ${LOG_LEVEL:-info}\n\ + \x20 CMD: echo $${HOME}\n"; + let (out, unresolved) = resolve_non_contract_references( + compose, + &std::collections::BTreeMap::new(), + &std::collections::BTreeSet::new(), + ); + + assert!(out.contains("${LOG_LEVEL:-info}"), "default kept:\n{out}"); + assert!(out.contains("$${HOME}"), "escape kept:\n{out}"); + assert!( + unresolved.is_empty(), + "neither needs a source: {unresolved:?}" + ); + } + + #[test] + fn a_reference_with_no_value_and_no_default_is_reported() { + let compose = "services:\n app:\n environment:\n TOKEN: ${MISSING}\n"; + let (_, unresolved) = resolve_non_contract_references( + compose, + &std::collections::BTreeMap::new(), + &std::collections::BTreeSet::new(), + ); + + assert_eq!( + unresolved, + vec![UnresolvedReference { + name: "MISSING".to_string() + }] + ); + } + + #[test] + fn required_keys_come_from_the_contract_not_the_file_text() { + let compose = "services:\n app:\n environment:\n\ + \x20 SECRET_KEY: ${SECRET_KEY}\n\ + \x20 LOG: ${LOG_LEVEL:-info}\n\ + \x20 CMD: echo $${HOME}\n\ + \x20 REGION: ${REGION}\n"; + + let required = required_env_keys(compose, &key_set(&["SECRET_KEY"])); + let found: Vec<&str> = required.iter().map(String::as_str).collect(); + + assert_eq!( + found, + vec!["SECRET_KEY"], + "only contract fields have a source on the buyer's machine" + ); + } + + /// M5 — Compose accepts `environment:` as a list as well as a mapping, and + /// a user-supplied compose (`deploy.compose_file`) often uses the list form. + /// Missing it leaves the author's secrets literal in the snapshot. + #[test] + fn parameterize_handles_the_list_form_of_environment() { + let compose = "\ +services: + db: + environment: + - POSTGRES_PASSWORD=aaaaaaaaaaaaaaaa + - POSTGRES_USER=stackpilot +"; + let mut keys = std::collections::HashSet::new(); + keys.insert("POSTGRES_PASSWORD".to_string()); + + let result = parameterize_compose_env_vars(compose, &keys); + + assert!( + result.contains("- POSTGRES_PASSWORD=${POSTGRES_PASSWORD}"), + "list entry replaced:\n{result}" + ); + assert!( + result.contains("- POSTGRES_USER=stackpilot"), + "undeclared entry untouched:\n{result}" + ); + } + + #[test] + fn parameterize_list_form_leaves_undeclared_keys_alone() { + let compose = "services:\n app:\n environment:\n - LOG_LEVEL=debug\n"; + let keys = std::collections::HashSet::new(); + assert_eq!(parameterize_compose_env_vars(compose, &keys), compose); + } + #[test] fn parameterize_no_keys_returns_original() { let compose = "services:\n app:\n environment:\n FOO: bar\n"; diff --git a/src/helpers/bake_finalize.rs b/src/helpers/bake_finalize.rs index 56eb5c89..a2a08c27 100644 --- a/src/helpers/bake_finalize.rs +++ b/src/helpers/bake_finalize.rs @@ -42,55 +42,228 @@ pub fn volumes_to_keep(stack: &str) -> &'static [&'static str] { } } -/// Shell to strip machine identity so each clone boots as a distinct host. +/// Values a service would lose when the buyer's machine replaces the env file. /// -/// `sshd` regenerates host keys on first boot when none are present, and -/// `systemd` repopulates an empty `/etc/machine-id`; clearing cloud-init's -/// instance state makes it treat the clone as a new instance and re-run its -/// per-instance modules. +/// A service declaring `env_file:` reads its values out of that file rather than +/// through `${VAR}` in the compose. At first boot the buyer's machine overwrites +/// the file wholesale from `/etc/stacker/env`, which carries only contract +/// fields and the buyer's own values — so every other key in it disappears, and +/// the container starts without values it had on the build box. +/// +/// Returns the keys that would be lost, in sorted order. Empty when the compose +/// declares no `env_file:` (stacker's own generator writes values into +/// `environment:` instead, so this only arises with a hand-written +/// `deploy.compose_file`). +/// +/// The author's fix is to move those values into an `environment:` block, where +/// they become literals in the image and survive. +pub fn env_file_values_lost_on_clone( + compose_content: &str, + env_values: &std::collections::BTreeMap, + protected: &BTreeSet, +) -> Vec { + let declares_env_file = compose_content + .lines() + .any(|line| line.trim_start().starts_with("env_file:")); + + if !declares_env_file { + return Vec::new(); + } + + env_values + .keys() + .filter(|key| !protected.contains(*key)) + .cloned() + .collect() +} + +/// Refuse a bake that has no field policy to sanitize against. +/// +/// With no protected keys the embedded-secret scan substitutes nothing and the +/// `.env` scrub clears nothing, yet the bake would still print a confident +/// "Sanitized" line and publish an image carrying the author's credentials. +/// The usual causes are mundane — the template is not approved yet, so +/// `get_approved_by_slug` returns nothing; `DATABASE_URL` is unset so the +/// contract was never looked up; or the author declared no fields at all. +/// +/// A stack that genuinely has no secrets can pass `--allow-unsanitized-snapshot`. +pub fn check_contract_usable( + protected: &BTreeSet, + allow_unsanitized: bool, +) -> Result<(), crate::helpers::bake::BakeError> { + if allow_unsanitized || !protected.is_empty() { + return Ok(()); + } + + Err(crate::helpers::bake::BakeError::Finalize( + "no config_contract fields resolved for this stack, so there is nothing to \ + sanitize and the image would keep the author's values. Usual causes: the \ + template is not approved yet, DATABASE_URL is not set, or the contract \ + declares no generated/provided fields. Pass --allow-unsanitized-snapshot \ + if the stack really has no secrets." + .to_string(), + )) +} + +/// Report a blind spot in the embedded-secret scan, if there is one. +/// +/// The scan finds a credential hidden inside a larger value (the DSN case) by +/// searching for the *values* the contract protects. With no values to search +/// for — no `.env` beside the compose — it cannot run, and a DSN would sail +/// through untouched. +/// +/// A project having no `.env` is perfectly normal, so this is not an error: a +/// reference the buyer's machine cannot fill is already caught separately, by +/// [`crate::cli::generator::compose::resolve_non_contract_references`]. This +/// only says out loud that one check did not happen, because a mistyped +/// `--project-dir` looks identical from here. +pub fn env_scan_warning( + env_values: &std::collections::BTreeMap, + protected: &BTreeSet, +) -> Option { + if protected.is_empty() || !env_values.is_empty() { + return None; + } + + Some( + "no values were found beside the compose file, so secrets embedded inside \ + larger values (a password inside DATABASE_URL, for example) could not be \ + searched for. If this stack does ship a .env, check --project-dir." + .to_string(), + ) +} + +/// Shell to strip everything host- or author-specific, so a clone boots as a +/// distinct machine that its author cannot reach. +/// +/// Two separate problems, both solved by removal: +/// +/// **Machine identity.** `sshd` regenerates host keys on first boot when none +/// are present and `systemd` repopulates an empty `/etc/machine-id`; clearing +/// cloud-init's instance state makes it treat the clone as a new instance and +/// re-run its per-instance modules. Left in place, every clone of the image +/// shares one host key, so any buyer can impersonate another buyer's server. +/// +/// **The author's own access.** Hetzner's cloud-init *appends* the buyer's key +/// to `authorized_keys` — it never truncates the file. A key left in the image +/// therefore grants its holder root on every server ever cloned from that +/// snapshot. The same applies to private keys, `known_hosts`, and to +/// `~/.docker/config.json`, which holds base64 registry credentials whenever the +/// build performed a `docker login`. +/// +/// `/etc/stacker/env` is deliberately absent from this list: cloud-init +/// overwrites it wholesale on the buyer's first boot (see `helpers::cloud_init`), +/// and the co-located `.env` is cleared separately by [`scrub_env_file`]. pub fn identity_reset_commands() -> Vec { vec![ + // Machine identity. "rm -f /etc/ssh/ssh_host_*".to_string(), ": > /etc/machine-id".to_string(), "rm -f /var/lib/dbus/machine-id".to_string(), "rm -rf /var/lib/cloud/instances /var/lib/cloud/instance".to_string(), + // The author's access — root and any other account on the box. + "rm -f /root/.ssh/authorized_keys /home/*/.ssh/authorized_keys".to_string(), + "rm -f /root/.ssh/id_* /home/*/.ssh/id_*".to_string(), + "rm -f /root/.ssh/known_hosts /home/*/.ssh/known_hosts".to_string(), + // Registry credentials left by a `docker login` during the build. + "rm -f /root/.docker/config.json /home/*/.docker/config.json".to_string(), + // Traces of the build itself. "rm -f /root/.bash_history /home/*/.bash_history".to_string(), "find /var/log -type f -exec truncate -s 0 {} + 2>/dev/null || true".to_string(), ] } -/// Shell to stop the stack and drop every data volume that is not explicitly -/// preserved, so the buyer's box initializes them from scratch with the -/// buyer's own generated values. +/// Shell to stop the stack and drop its credential-bearing data volumes, so the +/// buyer's box initializes them from scratch with the buyer's own values. /// -/// Volume names are matched by suffix because Compose prefixes them with the -/// project name (`project_stackpilot_pgdata` for a declared `stackpilot_pgdata`). -pub fn volume_reset_commands(project_dir: &str, keep: &[&str]) -> Vec { - let mut cmds = vec![format!( - "cd {project_dir} && docker compose down --remove-orphans" - )]; - - let filter = if keep.is_empty() { - "cat".to_string() +/// Scoped to **this project's declared volumes only**. A build box also runs +/// platform-managed services in their own Compose projects — the nginx-proxy-manager +/// ingress and the status-panel agent — whose volumes hold Let's Encrypt +/// certificates and agent state. Enumerating the host (`docker volume ls` with no +/// filter) would delete those, and would additionally abort the bake: `docker volume +/// rm` refuses a volume still held by a running container, and `-f` only suppresses +/// "no such volume", not "volume is in use". +/// +/// So the list comes from `docker compose config --volumes` (the names this stack +/// declares), and each is resolved to its real volume through Compose's own +/// `com.docker.compose.volume` label rather than by guessing the project prefix. +pub fn volume_reset_commands( + project_dir: &str, + keep: &[&str], +) -> Result, crate::helpers::bake::BakeError> { + if let Some(bad) = keep.iter().find(|name| !is_plain_volume_name(name)) { + return Err(crate::helpers::bake::BakeError::Finalize(format!( + "volume keep entry `{bad}` contains characters that are shell pattern syntax. \ + It would match something other than intended, and keeping a volume that should \ + have been reset leaves the author's credentials in the image. Use only letters, \ + digits, `_`, `-` and `.`." + ))); + } + + // Match whole `_`/`-` separated segments rather than a bare substring, so + // keeping `ollama` keeps `stackpilot_ollama` without also keeping + // `not-ollama-backup`. + let skip_kept = if keep.is_empty() { + String::new() } else { - let pattern = keep.join("|"); - format!("grep -Ev '({pattern})'") + let patterns = keep + .iter() + .flat_map(|name| { + [ + name.to_string(), + format!("*[-_]{name}"), + format!("{name}[-_]*"), + format!("*[-_]{name}[-_]*"), + ] + }) + .collect::>() + .join("|"); + format!("case \"$v\" in {patterns}) continue;; esac; ") }; - cmds.push(format!( - "docker volume ls -q | {filter} | xargs -r docker volume rm -f" - )); - cmds + Ok(vec![ + format!("cd {project_dir} && docker compose down --remove-orphans"), + format!( + "cd {project_dir} && for v in $(docker compose config --volumes); do \ + {skip_kept}docker volume ls -q \ + --filter label=com.docker.compose.volume=\"$v\" \ + | xargs -r docker volume rm -f; done" + ), + ]) +} + +/// A volume name safe to interpolate into a shell `case` pattern: no `*`, `?`, +/// `[`, `|`, `)` or anything else the shell would read as syntax. +fn is_plain_volume_name(name: &str) -> bool { + !name.is_empty() + && name + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-' || c == '.') } -/// Blank the values of secret-bearing keys in a `.env` file while keeping the -/// file's shape: keys, comments, blank lines and non-secret values survive. +/// Blank the author's secret values in a `.env` file while keeping the file's +/// shape: keys, comments, blank lines and non-secret values survive. +/// +/// **The contract is the only authority.** A value is blanked when its key is +/// declared `generated`/`provided` in `config_contract`, or when the value +/// *contains* such a secret — the DSN case, where the credential hides inside a +/// larger string under a name no policy mentions +/// (`DATABASE_URL=postgres://user:@host/db`). Nothing is guessed from +/// key names: name heuristics both miss real secrets and blank harmless values, +/// and the author already told us which fields are sensitive. /// /// The keys are kept (rather than the lines dropped) so the file still /// documents what the stack expects; on the buyer's box the whole file is /// replaced from `/etc/stacker/env` by the systemd unit's `ExecStartPre`, so /// the blanked values are never read. pub fn scrub_env_file(content: &str, protected: &BTreeSet) -> String { + // The literal values the contract protects — what a DSN may be hiding. + let secrets: Vec = parse_env_pairs(content) + .into_iter() + .filter(|(key, value)| protected.contains(key) && value.len() >= MIN_EMBEDDED_SECRET_LEN) + .map(|(_, value)| value) + .collect(); + let mut out = String::with_capacity(content.len()); for line in content.lines() { @@ -102,7 +275,10 @@ pub fn scrub_env_file(content: &str, protected: &BTreeSet) -> String { } match line.split_once('=') { - Some((key, _value)) if should_blank(key.trim(), protected) => { + Some((key, value)) + if protected.contains(key.trim()) + || secrets.iter().any(|secret| value.contains(secret.as_str())) => + { out.push_str(key); out.push_str("=\n"); } @@ -116,12 +292,9 @@ pub fn scrub_env_file(content: &str, protected: &BTreeSet) -> String { out } -/// A key is blanked when the author's contract declared it regenerable/buyer- -/// supplied, or when its name is secret-shaped by the same heuristic the CLI -/// already uses for `generate-secrets.sh`. -fn should_blank(key: &str, protected: &BTreeSet) -> bool { - protected.contains(key) || crate::console::commands::cli::init::is_secret_env_key(key) -} +/// Shortest value treated as a secret when searching *inside* another value. +/// Short strings collide with ordinary text and would blank harmless lines. +const MIN_EMBEDDED_SECRET_LEN: usize = 12; /// Everything the finalize step needs to reach and sanitize a build box. #[derive(Debug, Clone)] @@ -179,7 +352,9 @@ pub async fn finalize_build_box( .await .map_err(|e| e.to_string())?; if code != 0 { - return Err(format!("`{cmd}` exited {code}: {stderr}")); + // Never echo the command back whole: a file write carries the + // file's own contents as a base64 argument. + return Err(format!("`{}` exited {code}: {stderr}", summarize(&cmd))); } Ok::(stdout) } @@ -188,6 +363,10 @@ pub async fn finalize_build_box( let compose_path = format!("{}/docker-compose.yml", ctx.project_dir); let env_path = format!("{}/.env", ctx.project_dir); + // The steps are not transactional, so a failure has to be able to say what + // already happened — see `recovery_advice`. + let mut done: Vec = Vec::new(); + let result = async { // 1. Read what the deploy left on the box. let compose = run(format!("cat {compose_path}")) @@ -197,9 +376,37 @@ pub async fn finalize_build_box( let env_raw = run(format!("cat {env_path} 2>/dev/null || true")) .await .map_err(|e| fail("read .env", e))?; + done.push(FinalizeStage::Read); let env_values = parse_env_pairs(&env_raw); - // 2. Replace secrets embedded inside larger values (DSNs) with ${KEY} + let lost = env_file_values_lost_on_clone(&compose, &env_values, &ctx.protected_keys); + if !lost.is_empty() { + return Err(BakeError::Finalize(format!( + "this compose reads values through `env_file:`, and {} of them are not \ + contract fields ({}). The buyer's machine replaces that file wholesale \ + from /etc/stacker/env, which carries only contract fields and the buyer's \ + own values, so those would silently disappear at first boot. Move them \ + into an `environment:` block, where they become part of the image.", + lost.len(), + lost.join(", ") + ))); + } + if let Some(warning) = env_scan_warning(&env_values, &ctx.protected_keys) { + eprintln!("WARNING: {warning}"); + } + + // 2. Stop the stack and drop its credential-bearing data volumes, while + // the compose and .env on disk are still the ones Compose itself + // wrote. Doing this after the rewrites would hand `docker compose` + // a cleared .env, and any `${VAR}` outside an environment block + // (`image: ${REGISTRY}/app:${TAG}`) would then resolve empty and fail + // the teardown — with the files already modified and no way back. + for cmd in volume_reset_commands(&ctx.project_dir, volumes_to_keep(&ctx.stack))? { + run(cmd).await.map_err(|e| fail("volume reset", e))?; + } + done.push(FinalizeStage::Teardown); + + // 3. Replace secrets embedded inside larger values (DSNs) with ${KEY} // references. Whole-value keys were already parameterized at deploy // time by `parameterize_compose_env_vars`. let sanitized = crate::cli::generator::compose::parameterize_embedded_secret_values( @@ -209,17 +416,43 @@ pub async fn finalize_build_box( ) .map_err(|conflict| BakeError::Finalize(conflict.to_string()))?; + // 4. Put back the literal value of every reference the contract does not + // cover. Stage 4 in the CLI turns plain top-level `env:` keys into + // references too, and nothing fills those on the buyer's machine — + // the .env there is replaced wholesale from /etc/stacker/env, which + // only ever holds contract fields and the buyer's own values. + let (sanitized, unresolved) = + crate::cli::generator::compose::resolve_non_contract_references( + &sanitized, + &env_values, + &ctx.protected_keys, + ); + if !unresolved.is_empty() { + let names: Vec<&str> = unresolved.iter().map(|r| r.name.as_str()).collect(); + return Err(BakeError::Finalize(format!( + "the compose references {} environment variable(s) with no value on the \ + build box and no default ({}). They are not contract fields, so nothing \ + will fill them on a buyer's machine either — they would resolve to empty \ + strings at boot.", + names.len(), + names.join(", ") + ))); + } + if sanitized != compose { write_remote_file(&run, &compose_path, &sanitized) .await .map_err(|e| fail("write compose", e))?; + done.push(FinalizeStage::RewriteCompose); } - // 3. Record what the image now needs from the buyer's env file. + // 5. Record what the image needs from the buyer's env file — taken from + // the contract, not from the text of the file. A reference only counts + // as required when something is expected to supply it. let required_env_keys = - crate::cli::generator::compose::collect_env_var_references(&sanitized); + crate::cli::generator::compose::required_env_keys(&sanitized, &ctx.protected_keys); - // 4. Blank the author's secrets in the co-located .env. The buyer's box + // 6. Clear the author's secrets in the co-located .env. The buyer's box // overwrites this file wholesale from /etc/stacker/env at boot, so // the blanked values are never read. if !env_raw.trim().is_empty() { @@ -227,40 +460,166 @@ pub async fn finalize_build_box( write_remote_file(&run, &env_path, &scrubbed) .await .map_err(|e| fail("write .env", e))?; + done.push(FinalizeStage::ClearEnv); } - // 5. Drop initialized data volumes so the buyer's box re-initializes - // them with the buyer's own values. A secret the app persisted on - // first run is not governed by env vars any more. - for cmd in volume_reset_commands(&ctx.project_dir, volumes_to_keep(&ctx.stack)) { - run(cmd).await.map_err(|e| fail("volume reset", e))?; - } - - // 6. Strip machine identity last. + // 7. Strip machine identity last. for cmd in identity_reset_commands() { run(cmd).await.map_err(|e| fail("identity reset", e))?; } + done.push(FinalizeStage::StripIdentity); Ok(FinalizeOutcome { required_env_keys }) } .await; disconnect_ssh(session).await; - result + + result.map_err(|err| match err { + BakeError::Finalize(message) => { + BakeError::Finalize(format!("{message}\n\n{}", recovery_advice(&done))) + } + other => other, + }) +} + +/// A step of [`finalize_build_box`], in the order they run. +/// +/// Tracked so a failure can say what already happened: the steps are not +/// transactional and most of them are not reversible. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FinalizeStage { + /// Read the compose file and the co-located `.env`. + Read, + /// Stop the stack and drop its data volumes. + Teardown, + /// Write the sanitized compose back. + RewriteCompose, + /// Write the cleared `.env` back. + ClearEnv, + /// Remove machine identity and the author's access. + StripIdentity, +} + +impl FinalizeStage { + fn describe(self) -> &'static str { + match self { + FinalizeStage::Read => "read the compose file and .env", + FinalizeStage::Teardown => "stopped the stack and dropped its data volumes", + FinalizeStage::RewriteCompose => "rewrote the compose file", + FinalizeStage::ClearEnv => "cleared the .env", + FinalizeStage::StripIdentity => "stripped machine identity and the author's access", + } + } +} + +/// What the operator needs to know after a failed finalize. +/// +/// Re-running against the same box is rarely equivalent: the compose is already +/// sanitized so the embedded-secret scan has nothing left to find, the `.env` is +/// already cleared so there are no values to search for, and the data volumes +/// are gone. Saying this plainly is the difference between a lost afternoon and +/// a fresh build box. +pub fn recovery_advice(completed: &[FinalizeStage]) -> String { + let mut lines = Vec::new(); + + if completed.is_empty() { + lines.push("Nothing was changed on the build box.".to_string()); + } else { + lines.push("Completed before the failure:".to_string()); + for stage in completed { + lines.push(format!(" - {}", stage.describe())); + } + } + + let touched_files = completed + .iter() + .any(|s| matches!(s, FinalizeStage::RewriteCompose | FinalizeStage::ClearEnv)); + let torn_down = completed.contains(&FinalizeStage::Teardown); + let lost_access = completed.contains(&FinalizeStage::StripIdentity); + + if lost_access { + lines.push( + "The box no longer accepts the bake key, so it cannot be reconnected to.".to_string(), + ); + } + if torn_down { + lines.push( + "Its data volumes are gone and the stack is stopped, so it no longer \ + represents a working deployment." + .to_string(), + ); + } + if touched_files { + lines.push( + "Its compose and .env are already sanitized, so a retry would find nothing \ + left to search for and would report success over an unchecked image." + .to_string(), + ); + } + + if lost_access || torn_down || touched_files { + lines.push("Deploy a fresh build box and bake that instead.".to_string()); + } else { + lines.push("The bake can be retried against this box as-is.".to_string()); + } + + lines.join("\n") } -/// Write `content` to `path` on the remote box without any quoting hazards: -/// the payload travels base64-encoded and is decoded on the far side. +/// A command shortened for an error message. A file write carries the file's +/// own contents as a base64 argument, which must not end up in the bake log. +fn summarize(cmd: &str) -> String { + const MAX: usize = 120; + if cmd.len() <= MAX { + return cmd.to_string(); + } + let head: String = cmd.chars().take(MAX).collect(); + format!("{head}… ({} chars)", cmd.len()) +} + +/// Source bytes per chunk. Kept a multiple of 3 so every chunk is a whole +/// number of base64 groups and decodes on its own; 48 KiB of source becomes +/// 64 KiB of base64, comfortably inside Linux's 128 KiB limit on a single +/// command-line argument (`MAX_ARG_STRLEN`). +const WRITE_CHUNK_BYTES: usize = 48 * 1024; + +/// The shell to write `content` to `path`, split so no single command exceeds +/// the argument-length limit. +/// +/// The payload travels base64-encoded, which sidesteps every quoting hazard — +/// a compose file is full of `$`, quotes and newlines. The first command +/// truncates, the rest append. +pub fn write_file_commands(path: &str, content: &str) -> Vec { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + + let bytes = content.as_bytes(); + if bytes.is_empty() { + // Still create (or truncate) the file. + return vec![format!(": > {path}")]; + } + + bytes + .chunks(WRITE_CHUNK_BYTES) + .enumerate() + .map(|(index, chunk)| { + let redirect = if index == 0 { ">" } else { ">>" }; + let encoded = STANDARD.encode(chunk); + format!("printf %s {encoded} | base64 -d {redirect} {path}") + }) + .collect() +} + +/// Write `content` to `path` on the remote box. async fn write_remote_file(run: &F, path: &str, content: &str) -> Result<(), String> where F: Fn(String) -> Fut, Fut: std::future::Future>, { - use base64::{engine::general_purpose::STANDARD, Engine as _}; - let encoded = STANDARD.encode(content.as_bytes()); - run(format!("printf %s {encoded} | base64 -d > {path}")) - .await - .map(|_| ()) + for cmd in write_file_commands(path, content) { + run(cmd).await?; + } + Ok(()) } /// Parse `KEY=value` lines into a map, skipping comments and blanks. @@ -323,6 +682,264 @@ mod tests { keys.iter().map(|k| k.to_string()).collect() } + /// L1 — a keep entry is interpolated into a shell `case` pattern, where + /// `|`, `)`, `*`, `?` and `[` are all syntax. An entry carrying one of them + /// would silently match something else, or break the command outright. + /// Keeping a volume that should have been reset leaks the author's + /// credentials, so this fails rather than guesses. + #[test] + fn a_keep_entry_with_shell_syntax_is_refused() { + for bad in ["oll*ama", "a|b", "x)y", "a[bc]", "back`tick`", "semi;colon"] { + assert!( + volume_reset_commands("/home/trydirect/project", &[bad]).is_err(), + "`{bad}` must not reach a shell pattern" + ); + } + } + + #[test] + fn ordinary_volume_names_are_accepted() { + for good in ["ollama", "stackpilot_ollama", "model-cache", "data1"] { + assert!( + volume_reset_commands("/home/trydirect/project", &[good]).is_ok(), + "`{good}` should be usable" + ); + } + } + + /// Matching is on whole path segments, so `ollama` keeps + /// `stackpilot_ollama` without also keeping `not-ollama-backup`. + #[test] + fn a_keep_entry_does_not_match_a_longer_unrelated_name() { + let cmds = volume_reset_commands("/home/trydirect/project", &["ollama"]) + .expect("valid") + .join(" ; "); + assert!( + !cmds.contains("*ollama*"), + "a bare substring match would also keep `not-ollama-backup`: {cmds}" + ); + } + + /// H3 — a service reading its values through `env_file:` takes them from the + /// file, not through `${VAR}`. On the buyer's machine that file is replaced + /// wholesale from `/etc/stacker/env`, which only carries contract fields and + /// the buyer's own values, so everything else in it simply disappears. + #[test] + fn env_file_values_outside_the_contract_are_reported_as_lost() { + let compose = "services:\n app:\n env_file:\n - .env\n"; + let env: std::collections::BTreeMap = [ + ("SECRET_KEY".to_string(), "value".to_string()), + ("OLLAMA_MODEL".to_string(), "llama3.1".to_string()), + ] + .into_iter() + .collect(); + + let lost = + env_file_values_lost_on_clone(compose, &env, &protected(["SECRET_KEY"].as_slice())); + + assert_eq!( + lost, + vec!["OLLAMA_MODEL".to_string()], + "contract fields survive; anything else does not" + ); + } + + #[test] + fn a_compose_without_env_file_loses_nothing() { + let compose = "services:\n app:\n environment:\n A: b\n"; + let env: std::collections::BTreeMap = + [("OLLAMA_MODEL".to_string(), "llama3.1".to_string())] + .into_iter() + .collect(); + + assert!(env_file_values_lost_on_clone(compose, &env, &BTreeSet::new()).is_empty()); + } + + #[test] + fn env_file_carrying_only_contract_fields_is_fine() { + let compose = "services:\n app:\n env_file: .env\n"; + let env: std::collections::BTreeMap = + [("SECRET_KEY".to_string(), "value".to_string())] + .into_iter() + .collect(); + + assert!(env_file_values_lost_on_clone( + compose, + &env, + &protected(["SECRET_KEY"].as_slice()) + ) + .is_empty()); + } + + /// H5 — the steps are not transactional. When one fails, the operator has + /// to be told what already happened, because most of it is not reversible + /// and a retry against the same box is not equivalent. + #[test] + fn nothing_done_means_the_bake_can_be_retried() { + let advice = recovery_advice(&[FinalizeStage::Read]); + assert!( + advice.contains("can be retried"), + "a read-only failure leaves the box usable: {advice}" + ); + assert!(!advice.contains("fresh build box"), "no need: {advice}"); + } + + #[test] + fn modified_files_require_a_fresh_build_box() { + let advice = recovery_advice(&[FinalizeStage::Read, FinalizeStage::RewriteCompose]); + assert!(advice.contains("fresh build box"), "{advice}"); + assert!( + advice.contains("already sanitized") || advice.contains("already-sanitized"), + "the reason a retry is not equivalent should be stated: {advice}" + ); + } + + #[test] + fn a_completed_teardown_is_called_out_as_destructive() { + let advice = recovery_advice(&[FinalizeStage::Read, FinalizeStage::Teardown]); + assert!(advice.contains("data volumes"), "{advice}"); + assert!(advice.contains("fresh build box"), "{advice}"); + } + + #[test] + fn a_completed_identity_reset_means_no_way_back_in() { + let advice = recovery_advice(&[ + FinalizeStage::Read, + FinalizeStage::Teardown, + FinalizeStage::StripIdentity, + ]); + assert!( + advice.contains("no longer accepts"), + "losing SSH access must be stated plainly: {advice}" + ); + } + + #[test] + fn the_advice_lists_what_completed() { + let advice = recovery_advice(&[FinalizeStage::Read, FinalizeStage::Teardown]); + assert!(advice.contains("read"), "{advice}"); + assert!(advice.contains("stopped the stack"), "{advice}"); + } + + /// M4 — the payload travels as a shell command, and Linux caps a single + /// argument at 128 KiB. A large compose must therefore be written in pieces + /// rather than aborting the sanitize half-way with "Argument list too long". + #[test] + fn a_small_file_is_written_in_one_command() { + let cmds = write_file_commands("/tmp/x", "hello"); + assert_eq!(cmds.len(), 1); + assert!( + cmds[0].contains("> /tmp/x"), + "truncating write: {}", + cmds[0] + ); + assert!(!cmds[0].contains(">> /tmp/x"), "not appending: {}", cmds[0]); + } + + #[test] + fn a_large_file_is_written_in_appended_chunks() { + let big = "x".repeat(300_000); + let cmds = write_file_commands("/tmp/x", &big); + + assert!(cmds.len() > 1, "expected chunking, got {}", cmds.len()); + assert!(cmds[0].contains("> /tmp/x") && !cmds[0].contains(">> /tmp/x")); + for cmd in &cmds[1..] { + assert!(cmd.contains(">> /tmp/x"), "chunk must append: {cmd}"); + } + } + + #[test] + fn every_chunk_stays_under_the_argument_limit() { + let big = "y".repeat(500_000); + for cmd in write_file_commands("/tmp/x", &big) { + assert!( + cmd.len() < 128 * 1024, + "a single command must fit in one argument, got {}", + cmd.len() + ); + } + } + + /// Each chunk has to decode on its own, so the split must fall on a 3-byte + /// boundary — otherwise base64 padding corrupts the seams. + #[test] + fn chunks_round_trip_to_the_original_content() { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + + let original: String = (0..200_000) + .map(|i| ((i % 26) as u8 + b'a') as char) + .collect(); + let mut rebuilt = Vec::new(); + + for cmd in write_file_commands("/tmp/x", &original) { + let encoded = cmd.split_whitespace().nth(2).expect("printf %s "); + rebuilt.extend(STANDARD.decode(encoded).expect("each chunk decodes alone")); + } + + assert_eq!(String::from_utf8(rebuilt).unwrap(), original); + } + + #[test] + fn an_empty_file_still_produces_a_write() { + let cmds = write_file_commands("/tmp/x", ""); + assert_eq!(cmds.len(), 1, "the file must still be created/truncated"); + } + + /// H6 — a contract that did not resolve must stop the bake, not produce a + /// confident "Sanitized" line over an image that still carries the author's + /// credentials. + #[test] + fn empty_contract_refuses_the_bake() { + let err = check_contract_usable(&BTreeSet::new(), false) + .expect_err("an empty contract cannot sanitize anything"); + let message = err.to_string(); + assert!( + message.contains("approved"), + "the message should name the usual cause: {message}" + ); + } + + #[test] + fn empty_contract_is_allowed_only_deliberately() { + assert!(check_contract_usable(&BTreeSet::new(), true).is_ok()); + } + + #[test] + fn a_contract_with_fields_passes() { + assert!(check_contract_usable(&protected(["SECRET_KEY"].as_slice()), false).is_ok()); + } + + /// M2 — a project may legitimately ship no `.env`, so its absence is not an + /// error. What it does mean is that the embedded-secret scan has nothing to + /// search for, which is a blind spot worth saying out loud — a wrong + /// `--project-dir` looks exactly the same from here. + #[test] + fn missing_env_file_warns_when_secrets_are_declared() { + let warning = env_scan_warning( + &std::collections::BTreeMap::new(), + &protected(["SECRET_KEY"].as_slice()), + ) + .expect("a blind spot should be reported"); + assert!( + warning.contains("--project-dir"), + "the warning should name the likely cause: {warning}" + ); + } + + #[test] + fn missing_env_file_is_silent_when_nothing_is_declared() { + assert!(env_scan_warning(&std::collections::BTreeMap::new(), &BTreeSet::new()).is_none()); + } + + #[test] + fn present_env_values_warn_about_nothing() { + let env: std::collections::BTreeMap = + [("SECRET_KEY".to_string(), "value".to_string())] + .into_iter() + .collect(); + assert!(env_scan_warning(&env, &protected(["SECRET_KEY"].as_slice())).is_none()); + } + #[test] fn scrub_blanks_contract_declared_keys() { let env = "SECRET_KEY=b838f1f2\nOLLAMA_MODEL=llama3.1\n"; @@ -334,19 +951,56 @@ mod tests { ); } + /// Without a contract nothing is a declared secret, so nothing is blanked. + /// The contract is the only authority — no name guessing. #[test] - fn scrub_blanks_secret_shaped_keys_without_a_contract() { - // No contract at all — the name heuristic still has to catch these. - let env = "DB_PASSWORD=2213a996\nADMIN_USER=admin\n"; + fn scrub_without_a_contract_blanks_nothing() { + let env = "DB_PASSWORD=0123456789abcdef0123456789abcdef\nADMIN_USER=admin\n"; let out = scrub_env_file(env, &BTreeSet::new()); - assert!(out.contains("DB_PASSWORD=\n"), "blanked:\n{out}"); - assert!(out.contains("ADMIN_USER=admin"), "non-secret kept:\n{out}"); + assert_eq!(out, env); + } + + /// The DSN case: the credential hides inside a value whose own key is not + /// declared anywhere. + #[test] + fn scrub_blanks_a_value_that_embeds_a_declared_secret() { + let env = "POSTGRES_PASSWORD=0123456789abcdef0123456789abcdef\n\ + DATABASE_URL=postgresql://stackpilot:0123456789abcdef0123456789abcdef@db/s\n\ + REDIS_URL=redis://stackpilot-redis:6379\n"; + let out = scrub_env_file(env, &protected(["POSTGRES_PASSWORD"].as_slice())); + + assert!(out.contains("POSTGRES_PASSWORD=\n"), "declared key:\n{out}"); + assert!(out.contains("DATABASE_URL=\n"), "embedded secret:\n{out}"); + assert!( + !out.contains("0123456789abcdef0123456789abcdef"), + "no literal anywhere:\n{out}" + ); + assert!( + out.contains("REDIS_URL=redis://stackpilot-redis:6379"), + "credential-free URL kept:\n{out}" + ); + } + + /// A short declared value must not blank unrelated lines that happen to + /// contain it as a substring. + #[test] + fn scrub_ignores_short_declared_values_when_scanning_inside_others() { + let env = "PORT_TOKEN=8080\nPUBLIC_URL=http://host:8080/app\n"; + let out = scrub_env_file(env, &protected(["PORT_TOKEN"].as_slice())); + assert!( + out.contains("PORT_TOKEN=\n"), + "declared key still blanked:\n{out}" + ); + assert!( + out.contains("PUBLIC_URL=http://host:8080/app"), + "short value must not match inside others:\n{out}" + ); } #[test] fn scrub_preserves_comments_and_blank_lines() { let env = "# Secrets\n\nSECRET_KEY=abc\n"; - let out = scrub_env_file(env, &BTreeSet::new()); + let out = scrub_env_file(env, &protected(["SECRET_KEY"].as_slice())); assert_eq!(out, "# Secrets\n\nSECRET_KEY=\n"); } @@ -354,11 +1008,55 @@ mod tests { fn scrub_keeps_values_containing_equals_signs() { // A blanked key must not be confused by '=' inside a kept value. let env = "OLLAMA_MODEL=llama3.1\nJWT_SECRET=a=b=c\n"; - let out = scrub_env_file(env, &BTreeSet::new()); + let out = scrub_env_file(env, &protected(["JWT_SECRET"].as_slice())); assert!(out.contains("OLLAMA_MODEL=llama3.1"), "kept:\n{out}"); assert!(out.contains("JWT_SECRET=\n"), "blanked:\n{out}"); } + /// The author's own access must not survive into a buyer's server. + /// + /// Hetzner's cloud-init *appends* the buyer's key to `authorized_keys`; it + /// never truncates the file. An author key left in the image therefore + /// grants its holder root on every server ever cloned from that snapshot — + /// the same class of defect as the shared host keys, and a worse one. + #[test] + fn identity_reset_removes_operator_access_to_the_image() { + let cmds = identity_reset_commands().join(" ; "); + assert!( + cmds.contains("/root/.ssh/authorized_keys"), + "the author's SSH access must not be baked in: {cmds}" + ); + assert!( + cmds.contains("/home/*/.ssh/authorized_keys"), + "non-root accounts carry authorized_keys too: {cmds}" + ); + } + + /// A registry login performed during the build leaves base64 credentials in + /// `~/.docker/config.json`, which the snapshot would hand to every buyer. + #[test] + fn identity_reset_removes_registry_credentials() { + let cmds = identity_reset_commands().join(" ; "); + assert!( + cmds.contains("/root/.docker/config.json"), + "registry credentials must not be baked in: {cmds}" + ); + } + + /// Private keys and the operator's known_hosts are equally author-specific. + #[test] + fn identity_reset_removes_operator_private_keys() { + let cmds = identity_reset_commands().join(" ; "); + assert!( + cmds.contains("/root/.ssh/id_"), + "operator private keys must not be baked in: {cmds}" + ); + assert!( + cmds.contains("known_hosts"), + "known_hosts is author-specific: {cmds}" + ); + } + #[test] fn identity_reset_covers_host_keys_machine_id_and_cloud_init() { let cmds = identity_reset_commands().join(" ; "); @@ -372,21 +1070,49 @@ mod tests { #[test] fn volume_reset_preserves_the_kept_volumes() { - let cmds = volume_reset_commands("/home/trydirect/project", &["ollama"]).join(" ; "); + let cmds = volume_reset_commands("/home/trydirect/project", &["ollama"]) + .expect("valid keep list") + .join(" ; "); assert!( cmds.contains("docker compose down"), "stack stopped: {cmds}" ); assert!( - cmds.contains("grep -Ev '(ollama)'"), - "kept volume excluded from removal: {cmds}" + cmds.contains(r#"case "$v" in ollama|*[-_]ollama|"#), + "kept volume skipped: {cmds}" ); } + /// Regression: enumerating the host would delete the nginx-proxy-manager + /// ingress' certificates and the agent's state, and would abort the bake on + /// the first volume still held by a running container. + #[test] + fn volume_reset_never_enumerates_the_whole_host() { + for keep in [&[][..], &["ollama"][..]] { + let cmds = volume_reset_commands("/home/trydirect/project", keep) + .expect("valid keep list") + .join(" ; "); + assert!( + !cmds.contains("docker volume ls -q |"), + "must not pipe an unfiltered host-wide listing: {cmds}" + ); + assert!( + cmds.contains("docker compose config --volumes"), + "volume list must come from the project's compose: {cmds}" + ); + assert!( + cmds.contains("--filter label=com.docker.compose.volume="), + "removal must be scoped by compose's own label: {cmds}" + ); + } + } + #[test] - fn volume_reset_without_a_keep_list_removes_everything() { - let cmds = volume_reset_commands("/home/trydirect/project", &[]).join(" ; "); - assert!(cmds.contains("| cat |"), "no filter applied: {cmds}"); + fn volume_reset_without_a_keep_list_skips_nothing() { + let cmds = volume_reset_commands("/home/trydirect/project", &[]) + .expect("valid keep list") + .join(" ; "); + assert!(!cmds.contains("case "), "no skip clause: {cmds}"); } #[test] diff --git a/src/helpers/redact.rs b/src/helpers/redact.rs index ebf9656d..5ba1f349 100644 --- a/src/helpers/redact.rs +++ b/src/helpers/redact.rs @@ -151,6 +151,11 @@ pub fn redact_yaml_string(yaml: &str) -> String { use std::collections::BTreeSet; +/// The author-declared field policy block. Its leaf values describe *how* a +/// field is produced (`mutability`/`type`/`length`), never a secret value, so +/// value-stripping must skip this subtree entirely. +const CONFIG_CONTRACT_KEY: &str = "config_contract"; + /// Replace, in place, the values of env entries whose key is in `keys`. /// Handles the same shapes as [`redact_sensitive_json_values`] plus `KEY=value` /// strings in environment arrays. @@ -173,6 +178,14 @@ pub fn strip_json_values_for_keys( } } for (key, val) in map.iter_mut() { + // The contract declares the *policy* for a field, not its value: + // inside it, a key named e.g. SECRET_KEY maps to + // {mutability, type, length}, which is not a secret and must + // survive. Blanking it there destroys the very policy that + // drives per-buyer regeneration. + if key == CONFIG_CONTRACT_KEY { + continue; + } if keys.contains(key) && !val.is_null() { *val = serde_json::Value::String(replacement.to_string()); } else { @@ -206,6 +219,11 @@ fn strip_yaml_values_for_keys( serde_yaml::Value::Mapping(map) => { for (key, val) in map.iter_mut() { if let serde_yaml::Value::String(k) = key { + // See the note in `strip_json_values_for_keys`: field + // policies are not secrets and must not be blanked. + if k == CONFIG_CONTRACT_KEY { + continue; + } if keys.contains(k) && !val.is_null() { *val = serde_yaml::Value::String(replacement.to_string()); continue; @@ -251,8 +269,12 @@ pub fn strip_yaml_string_for_keys( #[cfg(test)] mod tests { - use super::{is_sensitive_env_key, redact_sensitive_json_values, redact_yaml_string}; + use super::{ + is_sensitive_env_key, redact_sensitive_json_values, redact_yaml_string, + strip_json_values_for_keys, strip_yaml_string_for_keys, + }; use serde_json::json; + use std::collections::BTreeSet; // JSON tests @@ -407,4 +429,76 @@ mod tests { // Either returned as-is (parse failed) or survived round-trip without panicking assert!(!result.contains("PANIC")); } + + // config_contract must survive value-stripping + + fn generated_keys() -> BTreeSet { + ["SECRET_KEY".to_string(), "POSTGRES_PASSWORD".to_string()] + .into_iter() + .collect() + } + + #[test] + fn strip_json_blanks_values_but_not_contract_policies() { + let mut v = json!({ + "app": { "environment": { "SECRET_KEY": "b838f1f2" } }, + "config_contract": { + "services": { + "app": { + "fields": { + "SECRET_KEY": { + "mutability": "generated", + "type": "alphanumeric", + "length": 32 + } + } + } + } + } + }); + + strip_json_values_for_keys(&mut v, &generated_keys(), ""); + + assert_eq!(v["app"]["environment"]["SECRET_KEY"], "", "secret blanked"); + assert_eq!( + v["config_contract"]["services"]["app"]["fields"]["SECRET_KEY"]["mutability"], + "generated", + "the policy that drives regeneration must survive" + ); + assert_eq!( + v["config_contract"]["services"]["app"]["fields"]["SECRET_KEY"]["length"], + 32 + ); + } + + #[test] + fn strip_yaml_blanks_values_but_not_contract_policies() { + let yaml = "\ +app: + environment: + POSTGRES_PASSWORD: author-value +config_contract: + services: + stackpilot-db: + fields: + POSTGRES_PASSWORD: + mutability: generated + type: alphanumeric +"; + let out = strip_yaml_string_for_keys(yaml, &generated_keys(), ""); + let parsed: serde_yaml::Value = serde_yaml::from_str(&out).unwrap(); + + assert_eq!( + parsed["app"]["environment"]["POSTGRES_PASSWORD"].as_str(), + Some(""), + "secret blanked:\n{out}" + ); + assert_eq!( + parsed["config_contract"]["services"]["stackpilot-db"]["fields"]["POSTGRES_PASSWORD"] + ["mutability"] + .as_str(), + Some("generated"), + "policy survives:\n{out}" + ); + } } diff --git a/tests/features/bake_sanitization.feature b/tests/features/bake_sanitization.feature new file mode 100644 index 00000000..d53b3830 --- /dev/null +++ b/tests/features/bake_sanitization.feature @@ -0,0 +1,331 @@ +Feature: Bake-time sanitization of a build box + A marketplace snapshot is taken from the author's own build box, so the + author's secrets must not survive into the image the buyer clones. + The author-declared config_contract is the only authority on what is + sensitive — nothing is guessed from variable names. + + Rule: the .env co-located with the compose file is scrubbed + + Scenario: A contract-declared field is blanked + Given the contract declares "POSTGRES_PASSWORD" as generated + And the build box .env contains: + """ + POSTGRES_PASSWORD=0123456789abcdef0123456789abcdef + OLLAMA_MODEL=llama3.1 + """ + When the .env is scrubbed for the snapshot + Then the scrubbed .env has "POSTGRES_PASSWORD" blanked + And the scrubbed .env still has "OLLAMA_MODEL" set to "llama3.1" + + Scenario: A DSN embedding a declared secret is blanked + Given the contract declares "POSTGRES_PASSWORD" as generated + And the build box .env contains: + """ + POSTGRES_PASSWORD=0123456789abcdef0123456789abcdef + DATABASE_URL=postgresql://stackpilot:0123456789abcdef0123456789abcdef@db:5432/s + REDIS_URL=redis://stackpilot-redis:6379 + """ + When the .env is scrubbed for the snapshot + Then the scrubbed .env has "DATABASE_URL" blanked + And the scrubbed .env still has "REDIS_URL" set to "redis://stackpilot-redis:6379" + And the scrubbed .env contains no occurrence of "0123456789abcdef0123456789abcdef" + + Scenario: Without a contract nothing is treated as secret + Given the contract declares nothing + And the build box .env contains: + """ + DB_PASSWORD=0123456789abcdef0123456789abcdef + ADMIN_USER=admin + """ + When the .env is scrubbed for the snapshot + Then the scrubbed .env is unchanged + + Scenario: A short declared value does not blank unrelated lines + Given the contract declares "PORT_TOKEN" as generated + And the build box .env contains: + """ + PORT_TOKEN=8080 + PUBLIC_URL=http://host:8080/app + """ + When the .env is scrubbed for the snapshot + Then the scrubbed .env has "PORT_TOKEN" blanked + And the scrubbed .env still has "PUBLIC_URL" set to "http://host:8080/app" + + Rule: secrets embedded inside compose values become ${VAR} references + + Scenario: The password inside a DSN is parameterized + Given the contract declares "POSTGRES_PASSWORD" as generated + And "POSTGRES_PASSWORD" on the build box resolves to "0123456789abcdef0123456789abcdef" + And the generated compose is: + """ + services: + app: + environment: + DATABASE_URL: postgresql://stackpilot:0123456789abcdef0123456789abcdef@db:5432/s + """ + When the compose is sanitized for the snapshot + Then the sanitized compose contains "DATABASE_URL: postgresql://stackpilot:${POSTGRES_PASSWORD}@db:5432/s" + And the sanitized compose contains no occurrence of "0123456789abcdef0123456789abcdef" + + Scenario: One secret declared under two protected names aborts the bake + Given the contract declares "DB_PASSWORD" as generated + And the contract declares "POSTGRES_PASSWORD" as generated + And "DB_PASSWORD" on the build box resolves to "0123456789abcdef0123456789abcdef" + And the generated compose is: + """ + services: + db: + environment: + POSTGRES_PASSWORD: 0123456789abcdef0123456789abcdef + """ + When the compose is sanitized for the snapshot + Then sanitizing fails naming both "DB_PASSWORD" and "POSTGRES_PASSWORD" + + Scenario: A value nobody declared is left alone + Given the contract declares nothing + And the generated compose is: + """ + services: + app: + environment: + PUBLIC_URL: http://host/aaaaaaaaaaaaaaaa + """ + When the compose is sanitized for the snapshot + Then the sanitized compose contains "http://host/aaaaaaaaaaaaaaaa" + + Rule: only the project's own volumes are reset + + Scenario: A keep entry carrying shell syntax is refused + Given the stack keeps the volume matching "oll*ama" + When the volume reset commands are built and may fail + Then building the commands is refused + + Scenario: A keep entry matches whole segments, not any substring + Given the stack keeps the volume matching "ollama" + When the volume reset commands are built for "/home/trydirect/project" + Then the commands do not keep every name containing "ollama" + + Scenario: Volume removal never enumerates the whole host + Given the stack keeps the volume matching "ollama" + When the volume reset commands are built for "/home/trydirect/project" + Then the commands list volumes from the project compose + And the commands scope removal by the compose volume label + And the commands never list every volume on the host + And the commands skip the volume matching "ollama" + + Rule: the image keeps a reference only when something will fill it + + Scenario: A variable outside the contract goes back to its value + Given the contract declares nothing + And "REGION" on the build box resolves to "fsn1" + And the generated compose is: + """ + services: + app: + environment: + REGION: ${REGION} + """ + When references outside the contract are resolved + Then the resolved compose contains "REGION: fsn1" + And no reference is reported as unfillable + + Scenario: A contract field stays a reference + Given the contract declares "POSTGRES_PASSWORD" as generated + And "POSTGRES_PASSWORD" on the build box resolves to "aaaaaaaaaaaaaaaa" + And the generated compose is: + """ + services: + db: + environment: + POSTGRES_PASSWORD: ${POSTGRES_PASSWORD} + """ + When references outside the contract are resolved + Then the resolved compose contains "POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}" + And the resolved compose contains no occurrence of "aaaaaaaaaaaaaaaa" + + Scenario: Defaults and escaped text need no source + Given the contract declares nothing + And the generated compose is: + """ + services: + app: + environment: + LOG: ${LOG_LEVEL:-info} + CMD: echo $${HOME} + """ + When references outside the contract are resolved + Then the resolved compose contains "${LOG_LEVEL:-info}" + And the resolved compose contains "$${HOME}" + And no reference is reported as unfillable + + Scenario: A reference with no value and no default is reported + Given the contract declares nothing + And the generated compose is: + """ + services: + app: + environment: + TOKEN: ${MISSING} + """ + When references outside the contract are resolved + Then "MISSING" is reported as unfillable + + Scenario: The required list is drawn from the contract, not the file text + Given the contract declares "SECRET_KEY" as generated + And the generated compose is: + """ + services: + app: + environment: + SECRET_KEY: ${SECRET_KEY} + LOG: ${LOG_LEVEL:-info} + CMD: echo $${HOME} + REGION: ${REGION} + """ + When the required environment keys are collected + Then the required keys are exactly "SECRET_KEY" + + Rule: the image carries no access belonging to the author + + Scenario: The author's SSH access is removed + When the identity reset commands are built + Then the commands remove "/root/.ssh/authorized_keys" + And the commands remove "/home/*/.ssh/authorized_keys" + + Scenario: The author's private keys and known hosts are removed + When the identity reset commands are built + Then the commands remove "/root/.ssh/id_" + And the commands remove "known_hosts" + + Scenario: Registry credentials are removed + When the identity reset commands are built + Then the commands remove "/root/.docker/config.json" + + Scenario: Machine identity is still stripped + When the identity reset commands are built + Then the commands remove "/etc/ssh/ssh_host_*" + And the commands remove "/etc/machine-id" + And the commands remove "/var/lib/cloud/instance" + + Rule: a bake that cannot sanitize refuses to publish + + Scenario: An unresolved contract stops the bake + Given the contract declares nothing + When the bake checks whether it can sanitize + Then the bake is refused + And the refusal mentions "approved" + + Scenario: An unresolved contract may be overridden deliberately + Given the contract declares nothing + And unsanitized snapshots are explicitly allowed + When the bake checks whether it can sanitize + Then the bake is allowed + + Scenario: A resolved contract lets the bake proceed + Given the contract declares "SECRET_KEY" as generated + When the bake checks whether it can sanitize + Then the bake is allowed + + Scenario: A missing env file is reported but does not stop the bake + Given the contract declares "SECRET_KEY" as generated + And the build box has no .env beside the compose + When the bake checks the values it has to work from + Then the bake is allowed + And a warning mentions "--project-dir" + + Scenario: A project without an env file draws no warning + Given the contract declares nothing + And the build box has no .env beside the compose + When the bake checks the values it has to work from + Then the bake is allowed + And no warning is raised + + Rule: a large file is written without exceeding the command-length limit + + Scenario: A small file is written in one command + When a file of 500 bytes is written to the build box + Then it takes 1 command + And the first command truncates the file + + Scenario: A large compose is written in appended chunks + When a file of 300000 bytes is written to the build box + Then it takes more than one command + And the first command truncates the file + And every later command appends + And every command fits in a single argument + + Scenario: The chunks reassemble into the original file + When a file of 200000 bytes is written to the build box + Then decoding the chunks in order yields the original content + + Rule: a failed finalize says what it already did + + Scenario: A failure before anything changed leaves the box usable + Given the finalize completed "read" + When the recovery advice is produced + Then the advice says the bake can be retried + And the advice does not ask for a fresh build box + + Scenario: A failure after the tear-down is destructive + Given the finalize completed "read, teardown" + When the recovery advice is produced + Then the advice mentions "data volumes" + And the advice asks for a fresh build box + + Scenario: A failure after the files were rewritten cannot be retried in place + Given the finalize completed "read, teardown, rewrite compose" + When the recovery advice is produced + Then the advice mentions "already sanitized" + And the advice asks for a fresh build box + + Scenario: A failure after the identity reset locks the operator out + Given the finalize completed "read, teardown, strip identity" + When the recovery advice is produced + Then the advice mentions "no longer accepts" + And the advice asks for a fresh build box + + Rule: values a buyer would silently lose stop the bake + + Scenario: A service reading through env_file loses its non-contract values + Given the contract declares "SECRET_KEY" as generated + And "SECRET_KEY" on the build box resolves to "aaaaaaaaaaaaaaaa" + And "OLLAMA_MODEL" on the build box resolves to "llama3.1" + And the generated compose is: + """ + services: + app: + env_file: + - .env + """ + When the bake checks what a clone would lose + Then "OLLAMA_MODEL" is reported as lost + And "SECRET_KEY" is not reported as lost + + Scenario: A compose without env_file loses nothing + Given the contract declares nothing + And "OLLAMA_MODEL" on the build box resolves to "llama3.1" + And the generated compose is: + """ + services: + app: + environment: + OLLAMA_MODEL: llama3.1 + """ + When the bake checks what a clone would lose + Then nothing is reported as lost + + Rule: both spellings of an environment block are covered + + Scenario: The list form is parameterized too + Given "POSTGRES_PASSWORD" is a protected compose key + And the generated compose is: + """ + services: + db: + environment: + - POSTGRES_PASSWORD=aaaaaaaaaaaaaaaa + - POSTGRES_USER=stackpilot + """ + When whole-value keys are parameterized + Then the parameterized compose contains "- POSTGRES_PASSWORD=${POSTGRES_PASSWORD}" + And the parameterized compose contains "- POSTGRES_USER=stackpilot" diff --git a/tests/steps/bake_sanitization.rs b/tests/steps/bake_sanitization.rs new file mode 100644 index 00000000..9be0705c --- /dev/null +++ b/tests/steps/bake_sanitization.rs @@ -0,0 +1,603 @@ +//! Steps for `tests/features/bake_sanitization.feature`. +//! +//! These exercise the pure policy functions directly — no HTTP, no database. +//! The SSH orchestration around them (`finalize_build_box`) needs a real build +//! box and is out of scope here; what is covered is the part that decides *what* +//! gets removed, which is where a mistake destroys an image or leaks a secret. + +use cucumber::{given, then, when}; +use std::collections::{BTreeMap, BTreeSet}; + +use stacker::cli::generator::compose::parameterize_embedded_secret_values; +use stacker::helpers::bake_finalize::{scrub_env_file, volume_reset_commands}; + +use super::StepWorld; + +// ─── Given ─────────────────────────────────────────────────────── + +#[given(regex = r#"^the contract declares "([^"]*)" as generated$"#)] +async fn given_contract_declares(world: &mut StepWorld, key: String) { + world.bake.protected.insert(key); +} + +#[given(regex = r#"^the contract declares nothing$"#)] +async fn given_contract_empty(world: &mut StepWorld) { + world.bake.protected.clear(); +} + +#[given(regex = r#"^the build box \.env contains:$"#)] +async fn given_env_contents(world: &mut StepWorld, step: &cucumber::gherkin::Step) { + let body = step.docstring().cloned().unwrap_or_default(); + world.bake.env_content = format!("{}\n", body.trim_matches('\n')); +} + +#[given(regex = r#"^"([^"]*)" on the build box resolves to "([^"]*)"$"#)] +async fn given_env_value(world: &mut StepWorld, key: String, value: String) { + world.bake.env_values.insert(key, value); +} + +#[given(regex = r#"^the generated compose is:$"#)] +async fn given_compose(world: &mut StepWorld, step: &cucumber::gherkin::Step) { + let body = step.docstring().cloned().unwrap_or_default(); + world.bake.compose = format!("{}\n", body.trim_matches('\n')); +} + +#[given(regex = r#"^the stack keeps the volume matching "([^"]*)"$"#)] +async fn given_keep_volume(world: &mut StepWorld, name: String) { + world.bake.keep_volumes.push(name); +} + +// ─── When ──────────────────────────────────────────────────────── + +#[when(regex = r#"^the \.env is scrubbed for the snapshot$"#)] +async fn when_scrub_env(world: &mut StepWorld) { + world.bake.scrubbed_env = scrub_env_file(&world.bake.env_content, &world.bake.protected); +} + +#[when(regex = r#"^the compose is sanitized for the snapshot$"#)] +async fn when_sanitize_compose(world: &mut StepWorld) { + match parameterize_embedded_secret_values( + &world.bake.compose, + &world.bake.env_values, + &world.bake.protected, + ) { + Ok(out) => { + world.bake.sanitized_compose = Some(out); + world.bake.sanitize_error = None; + } + Err(conflict) => { + world.bake.sanitized_compose = None; + world.bake.sanitize_error = Some(conflict.keys); + } + } +} + +#[when(regex = r#"^the volume reset commands are built for "([^"]*)"$"#)] +async fn when_build_volume_commands(world: &mut StepWorld, project_dir: String) { + let keep: Vec<&str> = world.bake.keep_volumes.iter().map(String::as_str).collect(); + world.bake.volume_commands = volume_reset_commands(&project_dir, &keep) + .expect("valid keep list") + .join(" ; "); +} + +// ─── Then: .env ────────────────────────────────────────────────── + +#[then(regex = r#"^the scrubbed \.env has "([^"]*)" blanked$"#)] +async fn then_env_blanked(world: &mut StepWorld, key: String) { + let expected = format!("{key}=\n"); + assert!( + world.bake.scrubbed_env.contains(&expected), + "expected `{key}` blanked in:\n{}", + world.bake.scrubbed_env + ); +} + +#[then(regex = r#"^the scrubbed \.env still has "([^"]*)" set to "([^"]*)"$"#)] +async fn then_env_kept(world: &mut StepWorld, key: String, value: String) { + let expected = format!("{key}={value}\n"); + assert!( + world.bake.scrubbed_env.contains(&expected), + "expected `{key}={value}` preserved in:\n{}", + world.bake.scrubbed_env + ); +} + +#[then(regex = r#"^the scrubbed \.env contains no occurrence of "([^"]*)"$"#)] +async fn then_env_has_no_literal(world: &mut StepWorld, literal: String) { + assert!( + !world.bake.scrubbed_env.contains(&literal), + "literal still present in:\n{}", + world.bake.scrubbed_env + ); +} + +#[then(regex = r#"^the scrubbed \.env is unchanged$"#)] +async fn then_env_unchanged(world: &mut StepWorld) { + assert_eq!( + world.bake.scrubbed_env, world.bake.env_content, + "nothing is declared, so nothing may be blanked" + ); +} + +// ─── Then: compose ─────────────────────────────────────────────── + +#[then(regex = r#"^the sanitized compose contains "([^"]*)"$"#)] +async fn then_compose_contains(world: &mut StepWorld, needle: String) { + let out = world + .bake + .sanitized_compose + .as_ref() + .expect("sanitizing should have succeeded"); + assert!(out.contains(&needle), "expected `{needle}` in:\n{out}"); +} + +#[then(regex = r#"^the sanitized compose contains no occurrence of "([^"]*)"$"#)] +async fn then_compose_has_no_literal(world: &mut StepWorld, literal: String) { + let out = world + .bake + .sanitized_compose + .as_ref() + .expect("sanitizing should have succeeded"); + assert!(!out.contains(&literal), "literal still present in:\n{out}"); +} + +#[then(regex = r#"^sanitizing fails naming both "([^"]*)" and "([^"]*)"$"#)] +async fn then_sanitize_conflict(world: &mut StepWorld, first: String, second: String) { + let keys = world + .bake + .sanitize_error + .as_ref() + .expect("sanitizing should have been refused"); + assert!( + keys.contains(&first) && keys.contains(&second), + "expected both `{first}` and `{second}` in the conflict, got {keys:?}" + ); +} + +// ─── Then: volumes ─────────────────────────────────────────────── + +#[then(regex = r#"^the commands list volumes from the project compose$"#)] +async fn then_volumes_from_project(world: &mut StepWorld) { + assert!( + world + .bake + .volume_commands + .contains("docker compose config --volumes"), + "commands: {}", + world.bake.volume_commands + ); +} + +#[then(regex = r#"^the commands scope removal by the compose volume label$"#)] +async fn then_volumes_scoped_by_label(world: &mut StepWorld) { + assert!( + world + .bake + .volume_commands + .contains("--filter label=com.docker.compose.volume="), + "commands: {}", + world.bake.volume_commands + ); +} + +#[then(regex = r#"^the commands never list every volume on the host$"#)] +async fn then_volumes_not_host_wide(world: &mut StepWorld) { + assert!( + !world.bake.volume_commands.contains("docker volume ls -q |"), + "an unfiltered host-wide listing would delete the ingress' certificates \ + and abort the bake on the first in-use volume; commands: {}", + world.bake.volume_commands + ); +} + +#[then(regex = r#"^the commands skip the volume matching "([^"]*)"$"#)] +async fn then_volumes_skip_kept(world: &mut StepWorld, name: String) { + let expected = format!("case \"$v\" in {name}|"); + assert!( + world.bake.volume_commands.contains(&expected), + "expected `{expected}`; commands: {}", + world.bake.volume_commands + ); +} + +/// Scratch state for the bake-sanitization scenarios. +#[derive(Debug, Default)] +pub struct BakeWorld { + pub protected: BTreeSet, + pub env_values: BTreeMap, + pub env_content: String, + pub scrubbed_env: String, + pub compose: String, + pub sanitized_compose: Option, + pub sanitize_error: Option>, + pub keep_volumes: Vec, + pub volume_commands: String, + pub resolved_compose: String, + pub unfillable: Vec, + pub required: Vec, + pub identity_commands: String, + pub allow_unsanitized: bool, + pub refusal: Option, + pub warning: Option, + pub file_content: String, + pub write_commands: Vec, + pub stages: Vec, + pub advice: String, + pub lost: Vec, +} + +// ─── references that survive into the baked image ──────────────── + +#[when(regex = r#"^references outside the contract are resolved$"#)] +async fn when_resolve_references(world: &mut StepWorld) { + let (out, unresolved) = stacker::cli::generator::compose::resolve_non_contract_references( + &world.bake.compose, + &world.bake.env_values, + &world.bake.protected, + ); + world.bake.resolved_compose = out; + world.bake.unfillable = unresolved.into_iter().map(|r| r.name).collect(); +} + +#[when(regex = r#"^the required environment keys are collected$"#)] +async fn when_collect_required(world: &mut StepWorld) { + world.bake.required = stacker::cli::generator::compose::required_env_keys( + &world.bake.compose, + &world.bake.protected, + ) + .into_iter() + .collect(); +} + +#[then(regex = r#"^the resolved compose contains "([^"]*)"$"#)] +async fn then_resolved_contains(world: &mut StepWorld, needle: String) { + assert!( + world.bake.resolved_compose.contains(&needle), + "expected `{needle}` in:\n{}", + world.bake.resolved_compose + ); +} + +#[then(regex = r#"^the resolved compose contains no occurrence of "([^"]*)"$"#)] +async fn then_resolved_lacks(world: &mut StepWorld, literal: String) { + assert!( + !world.bake.resolved_compose.contains(&literal), + "literal still present in:\n{}", + world.bake.resolved_compose + ); +} + +#[then(regex = r#"^no reference is reported as unfillable$"#)] +async fn then_nothing_unfillable(world: &mut StepWorld) { + assert!( + world.bake.unfillable.is_empty(), + "unexpected: {:?}", + world.bake.unfillable + ); +} + +#[then(regex = r#"^"([^"]*)" is reported as unfillable$"#)] +async fn then_reported_unfillable(world: &mut StepWorld, name: String) { + assert!( + world.bake.unfillable.contains(&name), + "expected `{name}` among {:?}", + world.bake.unfillable + ); +} + +#[then(regex = r#"^the required keys are exactly "([^"]*)"$"#)] +async fn then_required_exactly(world: &mut StepWorld, csv: String) { + let expected: Vec = csv.split(',').map(|s| s.trim().to_string()).collect(); + assert_eq!(world.bake.required, expected); +} + +// ─── access belonging to the author ────────────────────────────── + +#[when(regex = r#"^the identity reset commands are built$"#)] +async fn when_build_identity_commands(world: &mut StepWorld) { + world.bake.identity_commands = + stacker::helpers::bake_finalize::identity_reset_commands().join(" ; "); +} + +#[then(regex = r#"^the commands remove "([^"]*)"$"#)] +async fn then_commands_remove(world: &mut StepWorld, path: String) { + assert!( + world.bake.identity_commands.contains(&path), + "expected `{path}` to be removed; commands: {}", + world.bake.identity_commands + ); +} + +// ─── a bake that cannot sanitize ───────────────────────────────── + +#[given(regex = r#"^unsanitized snapshots are explicitly allowed$"#)] +async fn given_allow_unsanitized(world: &mut StepWorld) { + world.bake.allow_unsanitized = true; +} + +#[given(regex = r#"^the build box has no \.env beside the compose$"#)] +async fn given_no_env_file(world: &mut StepWorld) { + world.bake.env_values.clear(); +} + +#[when(regex = r#"^the bake checks whether it can sanitize$"#)] +async fn when_check_contract(world: &mut StepWorld) { + world.bake.refusal = stacker::helpers::bake_finalize::check_contract_usable( + &world.bake.protected, + world.bake.allow_unsanitized, + ) + .err() + .map(|e| e.to_string()); +} + +#[when(regex = r#"^the bake checks the values it has to work from$"#)] +async fn when_check_env_values(world: &mut StepWorld) { + world.bake.refusal = None; + world.bake.warning = stacker::helpers::bake_finalize::env_scan_warning( + &world.bake.env_values, + &world.bake.protected, + ); +} + +#[then(regex = r#"^the bake is refused$"#)] +async fn then_bake_refused(world: &mut StepWorld) { + assert!( + world.bake.refusal.is_some(), + "the bake should not have been allowed to proceed" + ); +} + +#[then(regex = r#"^the bake is allowed$"#)] +async fn then_bake_allowed(world: &mut StepWorld) { + assert!( + world.bake.refusal.is_none(), + "unexpected refusal: {:?}", + world.bake.refusal + ); +} + +#[then(regex = r#"^the refusal mentions "([^"]*)"$"#)] +async fn then_refusal_mentions(world: &mut StepWorld, needle: String) { + let message = world + .bake + .refusal + .as_ref() + .expect("there should be a refusal"); + assert!( + message.contains(&needle), + "expected `{needle}` in: {message}" + ); +} + +#[then(regex = r#"^a warning mentions "([^"]*)"$"#)] +async fn then_warning_mentions(world: &mut StepWorld, needle: String) { + let warning = world + .bake + .warning + .as_ref() + .expect("a warning should have been raised"); + assert!( + warning.contains(&needle), + "expected `{needle}` in: {warning}" + ); +} + +#[then(regex = r#"^no warning is raised$"#)] +async fn then_no_warning(world: &mut StepWorld) { + assert!( + world.bake.warning.is_none(), + "unexpected warning: {:?}", + world.bake.warning + ); +} + +// ─── writing a file to the build box ───────────────────────────── + +#[when(regex = r#"^a file of (\d+) bytes is written to the build box$"#)] +async fn when_write_file(world: &mut StepWorld, size: usize) { + // Varied content, so a seam corrupted by bad chunking is detectable. + world.bake.file_content = (0..size).map(|i| ((i % 26) as u8 + b'a') as char).collect(); + world.bake.write_commands = + stacker::helpers::bake_finalize::write_file_commands("/tmp/x", &world.bake.file_content); +} + +#[then(regex = r#"^it takes (\d+) command$"#)] +async fn then_command_count(world: &mut StepWorld, expected: usize) { + assert_eq!(world.bake.write_commands.len(), expected); +} + +#[then(regex = r#"^it takes more than one command$"#)] +async fn then_more_than_one(world: &mut StepWorld) { + assert!( + world.bake.write_commands.len() > 1, + "expected chunking, got {}", + world.bake.write_commands.len() + ); +} + +#[then(regex = r#"^the first command truncates the file$"#)] +async fn then_first_truncates(world: &mut StepWorld) { + let first = &world.bake.write_commands[0]; + assert!(first.contains("> /tmp/x"), "not a write: {first}"); + assert!(!first.contains(">> /tmp/x"), "must not append: {first}"); +} + +#[then(regex = r#"^every later command appends$"#)] +async fn then_rest_append(world: &mut StepWorld) { + for cmd in &world.bake.write_commands[1..] { + assert!(cmd.contains(">> /tmp/x"), "chunk must append: {cmd}"); + } +} + +#[then(regex = r#"^every command fits in a single argument$"#)] +async fn then_commands_fit(world: &mut StepWorld) { + for cmd in &world.bake.write_commands { + assert!( + cmd.len() < 128 * 1024, + "a command of {} chars would be rejected as too long", + cmd.len() + ); + } +} + +#[then(regex = r#"^decoding the chunks in order yields the original content$"#)] +async fn then_chunks_round_trip(world: &mut StepWorld) { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + + let mut rebuilt = Vec::new(); + for cmd in &world.bake.write_commands { + let encoded = cmd.split_whitespace().nth(2).expect("printf %s "); + rebuilt.extend(STANDARD.decode(encoded).expect("each chunk decodes alone")); + } + + assert_eq!( + String::from_utf8(rebuilt).expect("valid utf-8"), + world.bake.file_content + ); +} + +// ─── what a failed finalize reports ────────────────────────────── + +#[given(regex = r#"^the finalize completed "([^"]*)"$"#)] +async fn given_stages_completed(world: &mut StepWorld, csv: String) { + use stacker::helpers::bake_finalize::FinalizeStage; + + world.bake.stages = csv + .split(',') + .map(|s| match s.trim() { + "read" => FinalizeStage::Read, + "teardown" => FinalizeStage::Teardown, + "rewrite compose" => FinalizeStage::RewriteCompose, + "clear env" => FinalizeStage::ClearEnv, + "strip identity" => FinalizeStage::StripIdentity, + other => panic!("unknown finalize stage: {other}"), + }) + .collect(); +} + +#[when(regex = r#"^the recovery advice is produced$"#)] +async fn when_recovery_advice(world: &mut StepWorld) { + world.bake.advice = stacker::helpers::bake_finalize::recovery_advice(&world.bake.stages); +} + +#[then(regex = r#"^the advice says the bake can be retried$"#)] +async fn then_advice_retry(world: &mut StepWorld) { + assert!( + world.bake.advice.contains("can be retried"), + "advice: {}", + world.bake.advice + ); +} + +#[then(regex = r#"^the advice asks for a fresh build box$"#)] +async fn then_advice_fresh_box(world: &mut StepWorld) { + assert!( + world.bake.advice.contains("fresh build box"), + "advice: {}", + world.bake.advice + ); +} + +#[then(regex = r#"^the advice does not ask for a fresh build box$"#)] +async fn then_advice_no_fresh_box(world: &mut StepWorld) { + assert!( + !world.bake.advice.contains("fresh build box"), + "advice: {}", + world.bake.advice + ); +} + +#[then(regex = r#"^the advice mentions "([^"]*)"$"#)] +async fn then_advice_mentions(world: &mut StepWorld, needle: String) { + assert!( + world.bake.advice.contains(&needle), + "expected `{needle}` in: {}", + world.bake.advice + ); +} + +// ─── values a clone would lose, and the list form ──────────────── + +#[given(regex = r#"^"([^"]*)" is a protected compose key$"#)] +async fn given_protected_compose_key(world: &mut StepWorld, key: String) { + world.bake.protected.insert(key); +} + +#[when(regex = r#"^the bake checks what a clone would lose$"#)] +async fn when_check_lost(world: &mut StepWorld) { + world.bake.lost = stacker::helpers::bake_finalize::env_file_values_lost_on_clone( + &world.bake.compose, + &world.bake.env_values, + &world.bake.protected, + ); +} + +#[when(regex = r#"^whole-value keys are parameterized$"#)] +async fn when_parameterize_whole_values(world: &mut StepWorld) { + let keys: std::collections::HashSet = world.bake.protected.iter().cloned().collect(); + world.bake.resolved_compose = + stacker::cli::generator::compose::parameterize_compose_env_vars(&world.bake.compose, &keys); +} + +#[then(regex = r#"^"([^"]*)" is reported as lost$"#)] +async fn then_reported_lost(world: &mut StepWorld, key: String) { + assert!( + world.bake.lost.contains(&key), + "expected `{key}` among {:?}", + world.bake.lost + ); +} + +#[then(regex = r#"^"([^"]*)" is not reported as lost$"#)] +async fn then_not_reported_lost(world: &mut StepWorld, key: String) { + assert!( + !world.bake.lost.contains(&key), + "`{key}` should survive; got {:?}", + world.bake.lost + ); +} + +#[then(regex = r#"^nothing is reported as lost$"#)] +async fn then_nothing_lost(world: &mut StepWorld) { + assert!( + world.bake.lost.is_empty(), + "unexpected: {:?}", + world.bake.lost + ); +} + +#[then(regex = r#"^the parameterized compose contains "([^"]*)"$"#)] +async fn then_parameterized_contains(world: &mut StepWorld, needle: String) { + assert!( + world.bake.resolved_compose.contains(&needle), + "expected `{needle}` in:\n{}", + world.bake.resolved_compose + ); +} + +#[when(regex = r#"^the volume reset commands are built and may fail$"#)] +async fn when_build_volume_commands_fallible(world: &mut StepWorld) { + let keep: Vec<&str> = world.bake.keep_volumes.iter().map(String::as_str).collect(); + world.bake.refusal = + stacker::helpers::bake_finalize::volume_reset_commands("/home/trydirect/project", &keep) + .err() + .map(|e| e.to_string()); +} + +#[then(regex = r#"^building the commands is refused$"#)] +async fn then_build_refused(world: &mut StepWorld) { + assert!( + world.bake.refusal.is_some(), + "a keep entry with shell syntax must not be turned into a pattern" + ); +} + +#[then(regex = r#"^the commands do not keep every name containing "([^"]*)"$"#)] +async fn then_not_bare_substring(world: &mut StepWorld, name: String) { + let bare = format!("*{name}*"); + assert!( + !world.bake.volume_commands.contains(&bare), + "a bare substring match would also keep `not-{name}-backup`: {}", + world.bake.volume_commands + ); +} diff --git a/tests/steps/mod.rs b/tests/steps/mod.rs index 9e149818..7e506eb7 100644 --- a/tests/steps/mod.rs +++ b/tests/steps/mod.rs @@ -2,6 +2,7 @@ pub mod agent; pub mod agent_executor; +pub mod bake_sanitization; pub mod cdc; pub mod cloud_server; pub mod common; @@ -61,6 +62,8 @@ pub struct StepWorld { pub cdc_payload: Option, /// CDC trigger config for CDC BDD tests pub cdc_trigger: Option, + /// Scratch state for the bake-sanitization scenarios + pub bake: bake_sanitization::BakeWorld, } /// Wrapper for WebSocket stream that implements Debug @@ -124,6 +127,7 @@ impl StepWorld { cdc_event: None, cdc_payload: None, cdc_trigger: None, + bake: Default::default(), } }