From 052b89ba907d172cb25355919a0dfead6408b7d5 Mon Sep 17 00:00:00 2001 From: Vasili Pascal Date: Thu, 15 Oct 2026 09:23:23 +0300 Subject: [PATCH] Keep ${VAR} references when stripping generated secrets from templates strip_generated_field_values blanks every value of a generated/provided field before a template is stored, so the author's secrets never ship. It also blanked values that only name another variable, like AWS_SECRET_ACCESS_KEY: ${MINIO_ROOT_PASSWORD}. That reference is how one generated secret feeds two services; without it each key is regenerated on its own and the app and MinIO get different values. A value that is exactly ${NAME} or $NAME is now kept, in all five shapes the stripper handles (JSON map, JSON key/value pair, JSON and YAML K=V lists, YAML map). ${NAME:-default} is still blanked: the default can be the secret itself. Tests: helpers::redact strip_*_keeps_references_to_other_variables (fail before the fix). lib 2240 passed, marketplace and field-policy integration 64 passed, BDD 334 scenarios passed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012TKZy6f5shPzM9WLdswH6f --- src/helpers/redact.rs | 107 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 102 insertions(+), 5 deletions(-) diff --git a/src/helpers/redact.rs b/src/helpers/redact.rs index 5ba1f349..be12910d 100644 --- a/src/helpers/redact.rs +++ b/src/helpers/redact.rs @@ -159,6 +159,31 @@ 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. +/// Whether `value` only names another variable (`${NAME}` or `$NAME`). +/// +/// Such a value is not a secret: it is how one generated secret feeds several +/// keys, so stripping keeps it. `${NAME:-default}` does not count, the default +/// can be the secret itself. +fn is_env_reference(value: &str) -> bool { + let name = match value.strip_prefix("${") { + Some(rest) => match rest.strip_suffix('}') { + Some(name) => name, + None => return false, + }, + None => match value.strip_prefix('$') { + Some(name) => name, + None => return false, + }, + }; + let mut chars = name.chars(); + matches!(chars.next(), Some(c) if c == '_' || c.is_ascii_alphabetic()) + && chars.all(|c| c == '_' || c.is_ascii_alphanumeric()) +} + +fn json_is_env_reference(value: &serde_json::Value) -> bool { + value.as_str().map_or(false, is_env_reference) +} + pub fn strip_json_values_for_keys( value: &mut serde_json::Value, keys: &BTreeSet, @@ -170,7 +195,7 @@ pub fn strip_json_values_for_keys( if let Some(name) = map.get("key").and_then(|v| v.as_str()) { if keys.contains(name) { if let Some(val) = map.get_mut("value") { - if !val.is_null() { + if !val.is_null() && !json_is_env_reference(val) { *val = serde_json::Value::String(replacement.to_string()); } } @@ -187,7 +212,9 @@ pub fn strip_json_values_for_keys( continue; } if keys.contains(key) && !val.is_null() { - *val = serde_json::Value::String(replacement.to_string()); + if !json_is_env_reference(val) { + *val = serde_json::Value::String(replacement.to_string()); + } } else { strip_json_values_for_keys(val, keys, replacement); } @@ -198,7 +225,9 @@ pub fn strip_json_values_for_keys( if let serde_json::Value::String(s) = item { if let Some(eq) = s.find('=') { if keys.contains(&s[..eq]) { - *s = format!("{}={}", &s[..eq], replacement); + if !is_env_reference(&s[eq + 1..]) { + *s = format!("{}={}", &s[..eq], replacement); + } continue; } } @@ -225,7 +254,9 @@ fn strip_yaml_values_for_keys( continue; } if keys.contains(k) && !val.is_null() { - *val = serde_yaml::Value::String(replacement.to_string()); + if !val.as_str().map_or(false, is_env_reference) { + *val = serde_yaml::Value::String(replacement.to_string()); + } continue; } } @@ -236,7 +267,7 @@ fn strip_yaml_values_for_keys( for item in seq.iter_mut() { if let serde_yaml::Value::String(s) = item { if let Some(eq) = s.find('=') { - if keys.contains(&s[..eq]) { + if keys.contains(&s[..eq]) && !is_env_reference(&s[eq + 1..]) { *s = format!("{}={}", &s[..eq], replacement); } } @@ -501,4 +532,70 @@ config_contract: "policy survives:\n{out}" ); } + + // A value that only names another variable is not a secret. It is how one + // generated secret feeds two keys (plane: the app's AWS_SECRET_ACCESS_KEY + // is "${MINIO_ROOT_PASSWORD}"); blanking it broke that link and the two + // services got different values. A default inside the reference can hold + // the secret itself, so `${VAR:-value}` is still blanked. + + #[test] + fn strip_json_keeps_references_to_other_variables() { + let mut v = json!({ + "services": { + "api": { + "environment": { + "SECRET_KEY": "${MINIO_ROOT_PASSWORD}", + "POSTGRES_PASSWORD": "${DB_PASSWORD:-author-secret}" + } + }, + "worker": { "environment": ["SECRET_KEY=$MINIO_ROOT_PASSWORD"] } + }, + "vars": [{ "key": "SECRET_KEY", "value": "${MINIO_ROOT_PASSWORD}" }] + }); + + strip_json_values_for_keys(&mut v, &generated_keys(), ""); + + let env = &v["services"]["api"]["environment"]; + assert_eq!(env["SECRET_KEY"], "${MINIO_ROOT_PASSWORD}"); + assert_eq!(env["POSTGRES_PASSWORD"], "", "a default may be the secret"); + assert_eq!( + v["services"]["worker"]["environment"][0], + "SECRET_KEY=$MINIO_ROOT_PASSWORD" + ); + assert_eq!(v["vars"][0]["value"], "${MINIO_ROOT_PASSWORD}"); + } + + #[test] + fn strip_yaml_keeps_references_to_other_variables() { + let yaml = "\ +services: + api: + environment: + SECRET_KEY: ${MINIO_ROOT_PASSWORD} + POSTGRES_PASSWORD: ${DB_PASSWORD:-author-secret} + worker: + environment: + - SECRET_KEY=${MINIO_ROOT_PASSWORD} +"; + let out = strip_yaml_string_for_keys(yaml, &generated_keys(), ""); + let parsed: serde_yaml::Value = serde_yaml::from_str(&out).unwrap(); + + assert_eq!( + parsed["services"]["api"]["environment"]["SECRET_KEY"].as_str(), + Some("${MINIO_ROOT_PASSWORD}"), + "{out}" + ); + assert_eq!( + parsed["services"]["api"]["environment"]["POSTGRES_PASSWORD"].as_str(), + Some(""), + "{out}" + ); + assert_eq!( + parsed["services"]["worker"]["environment"][0].as_str(), + Some("SECRET_KEY=${MINIO_ROOT_PASSWORD}"), + "{out}" + ); + assert!(!out.contains("author-secret"), "{out}"); + } }