From 4665e497d7d1793b2a5d42cf5e2b8e27dbe6d42a Mon Sep 17 00:00:00 2001 From: archipelago Date: Sun, 5 Jul 2026 13:55:15 -0400 Subject: [PATCH] feat(security): move secret env out of podman inspect and Quadlet unit files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Secret env used to merge into manifest.app.environment, landing in 'podman inspect' Config.Env on the API backend and — worse — as plaintext Environment= lines in Quadlet unit files on disk. Now: - expand_and_partition_env (container crate, pure + tested) expands ${KEY} placeholders and splits env into plain entries and secret-bearing pairs. Plain entries that interpolate a secret (btcpay's Password=${BTCPAY_DB_PASS} connection strings) are tainted and travel as secrets too. Secret values themselves are never expanded (a generated value containing '${' passes verbatim). - values register as podman secrets: stdin (never argv/tempfile), --replace, content-hash label to skip no-op rewrites; a per-app hash cache in the orchestrator makes steady-state reconciles free of podman secret calls. Registration goes through the runtime trait (default no-op keeps mocks/docker inert). - containers reference secrets by name: secret_env map in the libpod create spec, Secret=,type=env,target= in Quadlet units. Verified empirically on fleet podman 5.4.2: value absent from inspect Config.Env, runtime injection works rootless. - rotation detection: io.archipelago.secret-env-hash container label (API) / the changed unit bytes (Quadlet). Pre-upgrade containers lack the label, so every secret-bearing app recreates ONCE on the first reconcile after deploy — deliberate, it scrubs the plaintext secrets out of existing container configs. Data dirs untouched. - docker dev fallback keeps plain -e injection (no secret store); podman secrets persist across uninstall, matching the preserve-credentials invariant (reinstall re-registers by hash). In-container /proc//environ is unchanged — env remains the app-compat contract; the closed leaks are inspect output and unit files on disk. Tests: archipelago-container 61/61 (3 new: taint partition, verbatim secrets, hash order-independence), archipelago container:: 160/160 (fedimint install test now asserts the secret arrives as a ref, not env; quadlet render test asserts Secret=/Label= lines). NEEDS the on-node gate re-run before the item counts as verified. Co-Authored-By: Claude Fable 5 --- core/Cargo.lock | 2 + .../src/container/prod_orchestrator.rs | 158 ++++++++++++---- core/archipelago/src/container/quadlet.rs | 49 +++++ core/container/Cargo.toml | 2 + core/container/src/manifest.rs | 169 +++++++++++++++++- core/container/src/podman_client.rs | 20 +++ core/container/src/runtime.rs | 84 +++++++++ docs/1.8.0-RELEASE-HARDENING-PLAN.md | 18 +- 8 files changed, 467 insertions(+), 35 deletions(-) diff --git a/core/Cargo.lock b/core/Cargo.lock index af604ddc..68425044 100644 --- a/core/Cargo.lock +++ b/core/Cargo.lock @@ -169,6 +169,7 @@ dependencies = [ "async-trait", "chrono", "futures", + "hex", "hyper 0.14.32", "indexmap", "log", @@ -176,6 +177,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml", + "sha2 0.10.9", "thiserror 1.0.69", "tokio", "tracing", diff --git a/core/archipelago/src/container/prod_orchestrator.rs b/core/archipelago/src/container/prod_orchestrator.rs index 6346c0fa..f72a3d8a 100644 --- a/core/archipelago/src/container/prod_orchestrator.rs +++ b/core/archipelago/src/container/prod_orchestrator.rs @@ -1064,6 +1064,11 @@ pub struct ProdContainerOrchestrator { /// false so the legacy path remains the production path until the /// 5× lifecycle harness goes green against the new path. use_quadlet_backends: bool, + /// app_id → last secret-env content hash pushed to the runtime's + /// secret store. Makes steady-state reconciles free of podman + /// secret calls; a rotation (hash change) falls through and + /// re-registers. + env_secret_cache: Mutex>, #[cfg(test)] test_disk_gb: Option, #[cfg(test)] @@ -1126,6 +1131,7 @@ impl ProdContainerOrchestrator { lnd_paths: lnd::EnsurePaths::default(), secrets_dir: PathBuf::from("/var/lib/archipelago/secrets"), use_quadlet_backends: config.use_quadlet_backends, + env_secret_cache: Mutex::new(HashMap::new()), #[cfg(test)] test_disk_gb: None, #[cfg(test)] @@ -1147,6 +1153,7 @@ impl ProdContainerOrchestrator { lnd_paths: lnd::EnsurePaths::default(), secrets_dir: PathBuf::from("/var/lib/archipelago/secrets"), use_quadlet_backends: false, + env_secret_cache: Mutex::new(HashMap::new()), test_disk_gb: None, test_bitcoin_host: None, } @@ -2892,6 +2899,13 @@ impl ProdContainerOrchestrator { } async fn resolve_dynamic_env(&self, manifest: &mut AppManifest) -> Result<()> { + // Idempotency guard: partitioning already ran on this instance. + // Re-running would re-taint against an environment that no longer + // contains the composite entries and silently drop them. Callers + // always resolve a fresh clone, so this only trips on misuse. + if !manifest.app.container.secret_env_refs.is_empty() { + return Ok(()); + } // Materialise any manifest-declared generated secrets before they're // read below. This is the single chokepoint every install/reconcile // path funnels through, so an app's secrets exist by the time its @@ -2913,13 +2927,18 @@ impl ProdContainerOrchestrator { let mut env = manifest.app.environment.clone(); env.extend(manifest.app.container.resolve_derived_env(&facts)); + if manifest.app.id == "fedimint" || manifest.app.id == "fedimintd" { + env.retain(|entry| !entry.starts_with("FM_BITCOIND_URL=")); + env.push("FM_BITCOIND_URL=http://bitcoin-knots:8332".to_string()); + } + let provider = FileSecretsProvider { root: self.secrets_dir.clone(), }; - let secrets = manifest + let secret_pairs = manifest .app .container - .resolve_secret_env(&provider) + .resolve_secret_env_pairs(&provider) .map_err(|e| anyhow::anyhow!(e.to_string())) .with_context(|| { format!( @@ -2928,36 +2947,57 @@ impl ProdContainerOrchestrator { self.secrets_dir.display() ) })?; - env.extend(secrets); - if manifest.app.id == "fedimint" || manifest.app.id == "fedimintd" { - env.retain(|entry| !entry.starts_with("FM_BITCOIND_URL=")); - env.push("FM_BITCOIND_URL=http://bitcoin-knots:8332".to_string()); - } - Self::expand_env_placeholders(&mut env); - manifest.app.environment = env; - Ok(()) - } - fn expand_env_placeholders(env: &mut Vec) { - let values: HashMap = env - .iter() - .filter_map(|entry| { - let (key, value) = entry.split_once('=')?; - Some((key.to_string(), value.to_string())) - }) - .collect(); + // Secret values never merge into `environment` — they'd land in + // `podman inspect` output and Quadlet unit files on disk. Instead: + // expand ${KEY} placeholders (a plain entry that interpolates a + // secret is tainted and travels as a secret itself — btcpay's + // Password=${BTCPAY_DB_PASS} connection strings), keep the plain + // remainder as env, and hand the secret-bearing pairs to the + // runtime's secret store by reference. + let (plain, secret_bearing) = + archipelago_container::manifest::expand_and_partition_env(env, secret_pairs); + manifest.app.environment = plain; + if secret_bearing.is_empty() { + manifest.app.container.secret_env_refs = Vec::new(); + manifest.app.container.secret_env_hash = None; + } else { + let hash = + archipelago_container::manifest::secret_env_content_hash(&secret_bearing); + let app_id = manifest.app.id.clone(); + manifest.app.container.secret_env_refs = secret_bearing + .into_iter() + .map(|(key, value)| archipelago_container::manifest::SecretEnvRef { + secret_name: format!( + "archy-env-{}-{}", + app_id, + key.to_ascii_lowercase() + ), + env_key: key, + value, + }) + .collect(); + manifest.app.container.secret_env_hash = Some(hash.clone()); - for entry in env.iter_mut() { - let Some((key, value)) = entry.split_once('=') else { - continue; - }; - let mut expanded = value.to_string(); - for (placeholder_key, placeholder_value) in &values { - expanded = - expanded.replace(&format!("${{{}}}", placeholder_key), placeholder_value); + // Register/refresh the podman secrets. A per-app hash cache makes + // the steady-state reconcile free: podman is only consulted when + // the resolved content actually changed (or on first touch after + // boot). Mock runtimes no-op via the trait default. + let cached = self + .env_secret_cache + .lock() + .await + .get(&app_id) + .cloned(); + if cached.as_deref() != Some(hash.as_str()) { + self.runtime + .ensure_env_secrets(&manifest.app.container.secret_env_refs) + .await + .with_context(|| format!("registering env secrets for {app_id}"))?; + self.env_secret_cache.lock().await.insert(app_id, hash); } - *entry = format!("{}={}", key, expanded); } + Ok(()) } async fn container_env_drifted(&self, name: &str, manifest: &AppManifest) -> bool { @@ -2993,12 +3033,40 @@ impl ProdContainerOrchestrator { }) .collect(); - manifest.app.environment.iter().any(|entry| { + if manifest.app.environment.iter().any(|entry| { let Some((key, expected)) = entry.split_once('=') else { return false; }; current.get(key).map_or(true, |actual| actual != expected) - }) + }) { + return true; + } + + // Secret-backed env never appears in Config.Env (that's the point) — + // rotation is detected via the content-hash label stamped at create + // time. A pre-upgrade container has no label, which reads as drift + // and triggers the one-time recreate that scrubs its plaintext + // secrets out of `podman inspect`. + if let Some(expected_hash) = &manifest.app.container.secret_env_hash { + let fmt = format!( + "{{{{ index .Config.Labels \"{}\" }}}}", + archipelago_container::manifest::SECRET_ENV_HASH_LABEL + ); + let inspect = tokio::process::Command::new("podman") + .args(["inspect", name, "--format", &fmt]) + .output() + .await; + let Ok(output) = inspect else { + return false; + }; + if !output.status.success() { + return false; + } + if String::from_utf8_lossy(&output.stdout).trim() != expected_hash { + return true; + } + } + false } async fn container_command_drifted(&self, name: &str, manifest: &AppManifest) -> bool { @@ -3883,6 +3951,9 @@ mod tests { images: StdMutex>, /// container_name -> env that create_container received. created_env: StdMutex>>, + /// container_name -> secret env refs that create_container received. + created_secret_refs: + StdMutex>>, /// If set, the next `build_image` call fails with this message. fail_build: StdMutex>, /// If set, `image_exists` fails for this image reference. @@ -3921,6 +3992,17 @@ mod tests { .cloned() .unwrap_or_default() } + fn created_secret_refs_for( + &self, + name: &str, + ) -> Vec { + self.created_secret_refs + .lock() + .unwrap() + .get(name) + .cloned() + .unwrap_or_default() + } } #[async_trait] @@ -3942,6 +4024,10 @@ mod tests { .lock() .unwrap() .insert(name.to_string(), manifest.app.environment.clone()); + self.created_secret_refs.lock().unwrap().insert( + name.to_string(), + manifest.app.container.secret_env_refs.clone(), + ); Ok(name.to_string()) } async fn start_container(&self, name: &str) -> Result<()> { @@ -4581,7 +4667,17 @@ app: assert!(env .iter() .any(|e| e.starts_with("FM_API_URL=ws://") && e.ends_with(":8174"))); - assert!(env.iter().any(|e| e == "FM_BITCOIND_PASSWORD=secret-pass")); + // The secret must NOT ride in plain env (that's the podman-inspect / + // quadlet-unit-file leak this pipeline exists to close) — it travels + // as a secret ref with the value bound for the podman secret store. + assert!(!env.iter().any(|e| e.starts_with("FM_BITCOIND_PASSWORD="))); + let refs = rt.created_secret_refs_for("fedimint"); + let r = refs + .iter() + .find(|r| r.env_key == "FM_BITCOIND_PASSWORD") + .expect("secret env must arrive as a ref"); + assert_eq!(r.value, "secret-pass"); + assert_eq!(r.secret_name, "archy-env-fedimint-fm_bitcoind_password"); } #[tokio::test] diff --git a/core/archipelago/src/container/quadlet.rs b/core/archipelago/src/container/quadlet.rs index 79aa6a99..cfe1f42b 100644 --- a/core/archipelago/src/container/quadlet.rs +++ b/core/archipelago/src/container/quadlet.rs @@ -142,6 +142,14 @@ pub struct QuadletUnit { // companion's rendered bytes are unchanged from before this PR. pub ports: Vec<(u16, u16, String)>, pub environment: Vec, + /// Secret-backed env: (env_key, podman secret name). Rendered as + /// `Secret=,type=env,target=` so the VALUE never lands in + /// this unit file on disk — only a reference to the podman secret + /// store. The orchestrator registers the secrets before writing units. + pub secret_env: Vec<(String, String)>, + /// Container labels (`Label=k=v`). Carries the secret-env content hash + /// for rotation-drift detection. + pub labels: Vec<(String, String)>, pub devices: Vec, pub add_hosts: Vec<(String, String)>, pub network_aliases: Vec, @@ -247,6 +255,12 @@ impl QuadletUnit { // accepts that form on a single Environment= line per pair. let _ = writeln!(s, "Environment={}", quote_environment(env)); } + for (key, secret_name) in &self.secret_env { + let _ = writeln!(s, "Secret={secret_name},type=env,target={key}"); + } + for (k, v) in &self.labels { + let _ = writeln!(s, "Label={k}={v}"); + } for dev in &self.devices { let _ = writeln!(s, "AddDevice={dev}"); } @@ -415,6 +429,23 @@ impl QuadletUnit { .map(|p| (p.host, p.container, p.protocol.clone())) .collect(), environment: app.environment.clone(), + secret_env: app + .container + .secret_env_refs + .iter() + .map(|r| (r.env_key.clone(), r.secret_name.clone())) + .collect(), + labels: app + .container + .secret_env_hash + .iter() + .map(|h| { + ( + archipelago_container::manifest::SECRET_ENV_HASH_LABEL.to_string(), + h.clone(), + ) + }) + .collect(), devices: app.devices.clone(), add_hosts: vec![("host.archipelago".into(), "10.89.0.1".into())], // Container always answers to its own name; manifest extras add the @@ -847,6 +878,24 @@ mod tests { use super::*; use tempfile::tempdir; + #[test] + fn render_emits_secret_env_by_reference_never_value() { + let u = QuadletUnit { + name: "t".into(), + description: "t".into(), + image: "img".into(), + secret_env: vec![("DB_PASS".into(), "archy-env-app-db_pass".into())], + labels: vec![("io.archipelago.secret-env-hash".into(), "abc123".into())], + ..QuadletUnit::default() + }; + let s = u.render(); + assert!(s.contains("Secret=archy-env-app-db_pass,type=env,target=DB_PASS")); + assert!(s.contains("Label=io.archipelago.secret-env-hash=abc123")); + // the secret VALUE never had a path into this unit — but guard the + // env channel anyway: no Environment= line may mention the key + assert!(!s.contains("Environment=DB_PASS")); + } + fn sample_unit() -> QuadletUnit { QuadletUnit { name: "archy-bitcoin-ui".into(), diff --git a/core/container/Cargo.toml b/core/container/Cargo.toml index 217de3e4..3100d40f 100644 --- a/core/container/Cargo.toml +++ b/core/container/Cargo.toml @@ -19,6 +19,8 @@ chrono = { version = "0.4", features = ["serde"] } uuid = { version = "1.0", features = ["v4"] } log = "0.4" tracing = "0.1" +sha2 = "0.10" +hex = "0.4" [lib] name = "archipelago_container" diff --git a/core/container/src/manifest.rs b/core/container/src/manifest.rs index 9746f93c..d365ac32 100644 --- a/core/container/src/manifest.rs +++ b/core/container/src/manifest.rs @@ -243,6 +243,22 @@ pub struct ContainerConfig { /// Example: `"100070:100070"` for Postgres' mapped subuid. #[serde(default)] pub data_uid: Option, + + /// Runtime-resolved secret env entries (never serialized, never in a + /// manifest file). Populated by the orchestrator's env-resolution + /// chokepoint; the backends turn each ref into a podman secret + /// reference (`secret_env` in the API spec / `Secret=…,type=env` in + /// Quadlet) so the VALUE never lands in `podman inspect` output or a + /// unit file on disk. The dev-only docker fallback injects `value` as + /// plain env — docker has no rootless secret store. + #[serde(skip)] + pub secret_env_refs: Vec, + + /// sha256 over all resolved secret env pairs, stamped onto the + /// container as a label so rotation is detectable as drift without + /// exposing values. None when the app has no secret env. + #[serde(skip)] + pub secret_env_hash: Option, } /// Derived-env entry. The template is rendered against `HostFacts` at @@ -265,6 +281,75 @@ pub struct SecretEnv { pub secret_file: String, } +/// A fully resolved secret env entry, produced at apply time. `value` lives +/// only in memory on its way to the podman secret store (or, on the dev +/// docker fallback, straight into the container env). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SecretEnvRef { + pub env_key: String, + /// Podman secret object name: `archy-env--`. + pub secret_name: String, + pub value: String, +} + +/// Container label carrying the combined secret-env content hash, used by +/// the reconciler to detect secret rotation as env drift. +pub const SECRET_ENV_HASH_LABEL: &str = "io.archipelago.secret-env-hash"; + +/// Podman secret label carrying the individual secret's content hash, used +/// to skip re-registration when nothing changed. +pub const SECRET_HASH_LABEL: &str = "io.archipelago.hash"; + +/// Expand `${KEY}` placeholders across plain + secret values, then split +/// the result into (plain env, secret-bearing pairs). Any plain entry whose +/// value interpolates a secret key is *tainted* — it embeds the secret and +/// must travel as a secret itself (btcpay's +/// `BTCPAY_POSTGRES=…Password=${BTCPAY_DB_PASS};…` pattern). Secret values +/// themselves are taken verbatim: a generated secret that happens to +/// contain `${` must not be mangled by expansion. +pub fn expand_and_partition_env( + plain: Vec, + secrets: Vec<(String, String)>, +) -> (Vec, Vec<(String, String)>) { + let plain_values: std::collections::HashMap = plain + .iter() + .filter_map(|entry| { + let (key, value) = entry.split_once('=')?; + Some((key.to_string(), value.to_string())) + }) + .collect(); + + let mut out_plain = Vec::with_capacity(plain.len()); + let mut out_secret: Vec<(String, String)> = Vec::with_capacity(secrets.len()); + + for entry in plain { + let Some((key, value)) = entry.split_once('=') else { + out_plain.push(entry); + continue; + }; + let mut expanded = value.to_string(); + let mut tainted = false; + for (k, v) in &plain_values { + expanded = expanded.replace(&format!("${{{k}}}"), v); + } + for (k, v) in &secrets { + let placeholder = format!("${{{k}}}"); + if expanded.contains(&placeholder) { + expanded = expanded.replace(&placeholder, v); + tainted = true; + } + } + if tainted { + out_secret.push((key.to_string(), expanded)); + } else { + out_plain.push(format!("{key}={expanded}")); + } + } + + out_secret.extend(secrets); + (out_plain, out_secret) +} + /// How a [`GeneratedSecret`] is produced. Each kind is deterministic in shape /// (so the orchestrator knows which files to expect) but random in value. #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] @@ -1209,6 +1294,19 @@ impl ContainerConfig { &self, provider: &dyn SecretsProvider, ) -> Result, ManifestError> { + Ok(self + .resolve_secret_env_pairs(provider)? + .into_iter() + .map(|(k, v)| format!("{k}={v}")) + .collect()) + } + + /// Like `resolve_secret_env` but returns (key, value) pairs — the shape + /// the podman-secret pipeline needs. + pub fn resolve_secret_env_pairs( + &self, + provider: &dyn SecretsProvider, + ) -> Result, ManifestError> { let mut out = Vec::with_capacity(self.secret_env.len()); for e in &self.secret_env { let v = provider.read(&e.secret_file)?; @@ -1220,12 +1318,28 @@ impl ContainerConfig { e.key, e.secret_file ))); } - out.push(format!("{}={}", e.key, v)); + out.push((e.key.clone(), v)); } Ok(out) } } +/// Deterministic content hash over resolved secret env pairs (sorted by +/// key), used for the container drift label and per-secret labels. +pub fn secret_env_content_hash(pairs: &[(String, String)]) -> String { + use sha2::{Digest, Sha256}; + let mut sorted: Vec<&(String, String)> = pairs.iter().collect(); + sorted.sort_by(|a, b| a.0.cmp(&b.0)); + let mut hasher = Sha256::new(); + for (k, v) in sorted { + hasher.update(k.as_bytes()); + hasher.update([0u8]); + hasher.update(v.as_bytes()); + hasher.update([0u8]); + } + hex::encode(hasher.finalize()) +} + #[cfg(test)] mod tests { use super::*; @@ -1233,6 +1347,53 @@ mod tests { use std::fs; use std::path::{Path, PathBuf}; + #[test] + fn partition_taints_plain_entries_that_interpolate_secrets() { + let plain = vec![ + "PLAIN=1".to_string(), + "COMPOSED=${BASE}/x".to_string(), + "DB_URL=postgres://u:${DB_PASS}@db/x".to_string(), + "BASE=/srv".to_string(), + "UNKNOWN=${NOT_DEFINED}".to_string(), + ]; + let secrets = vec![("DB_PASS".to_string(), "s3cret".to_string())]; + let (p, s) = expand_and_partition_env(plain, secrets); + + // plain-from-plain expansion stays plain; unknown placeholders stay + // literal (legacy expander parity) + assert!(p.contains(&"PLAIN=1".to_string())); + assert!(p.contains(&"COMPOSED=/srv/x".to_string())); + assert!(p.contains(&"UNKNOWN=${NOT_DEFINED}".to_string())); + // the tainted entry moved out of plain, fully expanded + assert!(!p.iter().any(|e| e.starts_with("DB_URL="))); + assert!(s.contains(&("DB_URL".to_string(), "postgres://u:s3cret@db/x".to_string()))); + // the original secret rides along verbatim + assert!(s.contains(&("DB_PASS".to_string(), "s3cret".to_string()))); + } + + #[test] + fn secret_values_are_never_expanded() { + // A generated secret containing `${` must pass through untouched. + let secrets = vec![("WEIRD".to_string(), "pa${PLAIN}ss".to_string())]; + let (_, s) = expand_and_partition_env(vec!["PLAIN=1".to_string()], secrets); + assert!(s.contains(&("WEIRD".to_string(), "pa${PLAIN}ss".to_string()))); + } + + #[test] + fn secret_env_hash_is_order_independent() { + let a = vec![ + ("K1".to_string(), "v1".to_string()), + ("K2".to_string(), "v2".to_string()), + ]; + let b = vec![ + ("K2".to_string(), "v2".to_string()), + ("K1".to_string(), "v1".to_string()), + ]; + assert_eq!(secret_env_content_hash(&a), secret_env_content_hash(&b)); + let c = vec![("K1".to_string(), "CHANGED".to_string())]; + assert_ne!(secret_env_content_hash(&a), secret_env_content_hash(&c)); + } + #[test] fn hooks_parse_and_validate() { let yaml = r#" @@ -1765,6 +1926,8 @@ app: generated_secrets: vec![], generated_certs: vec![], data_uid: None, + secret_env_refs: vec![], + secret_env_hash: None, }; let facts = HostFacts { host_ip: "192.168.1.116".to_string(), @@ -1817,6 +1980,8 @@ app: generated_secrets: vec![], generated_certs: vec![], data_uid: None, + secret_env_refs: vec![], + secret_env_hash: None, }; let p = MapSecretsProvider { data: HashMap::from([ @@ -1855,6 +2020,8 @@ app: generated_secrets: vec![], generated_certs: vec![], data_uid: None, + secret_env_refs: vec![], + secret_env_hash: None, }; let p = MapSecretsProvider { data: HashMap::from([("bitcoin-rpc-password".to_string(), " \n".to_string())]), diff --git a/core/container/src/podman_client.rs b/core/container/src/podman_client.rs index fb93058d..1a965ea5 100644 --- a/core/container/src/podman_client.rs +++ b/core/container/src/podman_client.rs @@ -382,12 +382,32 @@ impl PodmanClient { manifest.app.security.network_policy.as_str(), ); + // Secret env travels by reference (podman injects the value at + // start), so it never shows up in `podman inspect` output. The + // combined content hash rides as a label for rotation-drift checks. + let mut secret_env_map = serde_json::Map::new(); + for r in &manifest.app.container.secret_env_refs { + secret_env_map.insert( + r.env_key.clone(), + serde_json::Value::String(r.secret_name.clone()), + ); + } + let mut labels_map = serde_json::Map::new(); + if let Some(hash) = &manifest.app.container.secret_env_hash { + labels_map.insert( + crate::manifest::SECRET_ENV_HASH_LABEL.to_string(), + serde_json::Value::String(hash.clone()), + ); + } + let mut body = serde_json::json!({ "name": name, "image": image_ref, "portmappings": port_mappings, "mounts": mounts, "env": env_map, + "secret_env": secret_env_map, + "labels": labels_map, "entrypoint": manifest.app.container.entrypoint.clone(), "command": manifest.app.container.custom_args.clone(), "hostadd": [ diff --git a/core/container/src/runtime.rs b/core/container/src/runtime.rs index 97ca4f26..4ee040b3 100644 --- a/core/container/src/runtime.rs +++ b/core/container/src/runtime.rs @@ -82,6 +82,18 @@ pub trait ContainerRuntime: Send + Sync { /// `create_container` / `image_exists` calls. Stdout/stderr are collected /// and included in the error on failure; on success they are discarded. async fn build_image(&self, config: &BuildConfig) -> Result<()>; + + /// Register the app's resolved secret-env entries in the runtime's + /// secret store (idempotent — skips entries whose content hash already + /// matches). Called before `create_container` for manifests with + /// `secret_env_refs`, so the create can reference secrets by name and + /// the values never appear in inspect output or unit files. Default is + /// a no-op for runtimes without a secret store (mocks, docker — the + /// docker fallback injects plain env in `create_container` instead). + async fn ensure_env_secrets(&self, refs: &[crate::manifest::SecretEnvRef]) -> Result<()> { + let _ = refs; + Ok(()) + } } pub struct PodmanRuntime { @@ -131,6 +143,68 @@ impl ContainerRuntime for PodmanRuntime { self.client.pull_image(image, signature).await } + async fn ensure_env_secrets(&self, refs: &[crate::manifest::SecretEnvRef]) -> Result<()> { + use crate::manifest::{secret_env_content_hash, SECRET_HASH_LABEL}; + use tokio::io::AsyncWriteExt; + + for r in refs { + let hash = secret_env_content_hash(&[(r.env_key.clone(), r.value.clone())]); + + // Skip the write when the stored secret already has this content + // (label round-trip beats rewriting the secret store every + // reconcile). Any inspect failure just falls through to create. + let fmt = format!("{{{{ index .Spec.Labels \"{SECRET_HASH_LABEL}\" }}}}"); + if let Ok(out) = self + .podman_cli(&["secret", "inspect", &r.secret_name, "--format", &fmt]) + .await + { + if out.status.success() + && String::from_utf8_lossy(&out.stdout).trim() == hash + { + continue; + } + } + + // Value goes via stdin — never argv, never a temp file. + let mut cmd = TokioCommand::new("podman"); + cmd.args([ + "secret", + "create", + "--replace", + "--label", + &format!("{SECRET_HASH_LABEL}={hash}"), + &r.secret_name, + "-", + ]); + cmd.stdin(std::process::Stdio::piped()); + cmd.stdout(std::process::Stdio::piped()); + cmd.stderr(std::process::Stdio::piped()); + cmd.kill_on_drop(true); + let mut child = cmd + .spawn() + .with_context(|| format!("spawning podman secret create {}", r.secret_name))?; + { + let mut stdin = child + .stdin + .take() + .context("podman secret create stdin unavailable")?; + stdin.write_all(r.value.as_bytes()).await?; + // drop closes the pipe so podman sees EOF + } + let out = tokio::time::timeout(PODMAN_CLI_DEFAULT_TIMEOUT, child.wait_with_output()) + .await + .with_context(|| format!("podman secret create {} timed out", r.secret_name))??; + if !out.status.success() { + anyhow::bail!( + "podman secret create {} failed: {}", + r.secret_name, + String::from_utf8_lossy(&out.stderr) + ); + } + } + Ok(()) + } + async fn create_container( &self, manifest: &AppManifest, @@ -621,6 +695,12 @@ impl ContainerRuntime for DockerRuntime { for env in &manifest.app.environment { cmd.arg("-e").arg(env); } + // Dev-only fallback: docker has no rootless secret store, so secret + // env rides as plain env here. The podman path (production) passes + // these by secret reference instead — see ensure_env_secrets. + for r in &manifest.app.container.secret_env_refs { + cmd.arg("-e").arg(format!("{}={}", r.env_key, r.value)); + } // Resource limits if let Some(cpu) = manifest.app.resources.cpu_limit { @@ -893,6 +973,10 @@ impl ContainerRuntime for AutoRuntime { self.runtime.pull_image(image, signature).await } + async fn ensure_env_secrets(&self, refs: &[crate::manifest::SecretEnvRef]) -> Result<()> { + self.runtime.ensure_env_secrets(refs).await + } + async fn create_container( &self, manifest: &AppManifest, diff --git a/docs/1.8.0-RELEASE-HARDENING-PLAN.md b/docs/1.8.0-RELEASE-HARDENING-PLAN.md index dd9ab0cd..02ac70ab 100644 --- a/docs/1.8.0-RELEASE-HARDENING-PLAN.md +++ b/docs/1.8.0-RELEASE-HARDENING-PLAN.md @@ -149,9 +149,21 @@ modules; production request/boot paths are essentially panic-free. The real risk public; zmq 28332/28333 should get the same look — unauthenticated). ⚠ May break external wallets pointed straight at nodeIP:8332 — needs a user call + on-node gate re-run, so NOT changed drive-by from the dev box. -- [ ] 🟡 **Move generated secrets from env to file mounts.** `manifest.rs:1208-1226` - injects secrets as `-e KEY=value`, readable via `podman inspect` / `/proc//environ`. - Prefer bind-mounting the existing `0600` secret file or `podman --secret`. +- [x] 🟡 **Move secret env out of plaintext channels → podman secrets.** DONE 2026-07-05 + (code + unit tests; needs the on-node gate re-run before it counts as verified): + secret env no longer merges into `environment` — it would land in `podman inspect` + AND as plaintext `Environment=` lines in Quadlet unit files on disk (the worse leak). + New pipeline: `expand_and_partition_env` taints plain entries that interpolate + secrets (btcpay's `Password=${BTCPAY_DB_PASS}` connection strings travel as secrets + too), values register as podman secrets (stdin, `--replace`, content-hash label, + per-app cache so steady-state reconciles are podman-free), containers reference + them via `secret_env` (API) / `Secret=…,type=env` (Quadlet). Verified empirically + on fleet podman 5.4.2: value absent from inspect, runtime injection works. Rotation + drift via `io.archipelago.secret-env-hash` container label; pre-upgrade containers + lack the label → ONE-TIME recreate wave on first reconcile after deploy (by design — + scrubs plaintext secrets from existing container configs). Docker dev fallback keeps + plain env (no secret store). `/proc//environ` inside the container is unchanged + (env is the app-compat contract); the closed leaks are inspect output + unit files. - [x] 🟡 **Harden rate-limit IP extraction.** DONE 2026-07-03: the accept loop injects the TCP `PeerAddr` into request extensions; `extract_client_ip` honors `X-Real-IP`/`X-Forwarded-For` ONLY when the connection is from loopback (our nginx,