feat(security): move secret env out of podman inspect and Quadlet unit files
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=<name>,type=env,target=<KEY> 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/<pid>/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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
eed830e1ee
commit
4665e497d7
@@ -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<HashMap<String, String>>,
|
||||
#[cfg(test)]
|
||||
test_disk_gb: Option<u64>,
|
||||
#[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<String>) {
|
||||
let values: HashMap<String, String> = 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<HashMap<String, bool>>,
|
||||
/// container_name -> env that create_container received.
|
||||
created_env: StdMutex<HashMap<String, Vec<String>>>,
|
||||
/// container_name -> secret env refs that create_container received.
|
||||
created_secret_refs:
|
||||
StdMutex<HashMap<String, Vec<archipelago_container::manifest::SecretEnvRef>>>,
|
||||
/// If set, the next `build_image` call fails with this message.
|
||||
fail_build: StdMutex<Option<String>>,
|
||||
/// 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<archipelago_container::manifest::SecretEnvRef> {
|
||||
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]
|
||||
|
||||
@@ -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<String>,
|
||||
/// Secret-backed env: (env_key, podman secret name). Rendered as
|
||||
/// `Secret=<name>,type=env,target=<key>` 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<String>,
|
||||
pub add_hosts: Vec<(String, String)>,
|
||||
pub network_aliases: Vec<String>,
|
||||
@@ -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(),
|
||||
|
||||
Reference in New Issue
Block a user