diff --git a/crates/mise-util/src/env.rs b/crates/mise-util/src/env.rs index 1a8e71224d2..9ac9b2d8eb0 100644 --- a/crates/mise-util/src/env.rs +++ b/crates/mise-util/src/env.rs @@ -1,6 +1,6 @@ //! Environment variables mise reads, and the directories derived from them. -use crate::env_diff::{EnvDiff, EnvMap}; +use crate::env_diff::{EnvDiff, EnvMap, env_value_matches}; use crate::env_value::EnvValue; use crate::file::replace_path; use indexmap::IndexMap; @@ -931,7 +931,10 @@ fn get_pristine_env(mise_diff: &EnvDiff, orig_env: EnvMap) -> EnvMap { fn reverse_diff_preserving_overrides(mise_diff: &EnvDiff, mut env: EnvMap) -> EnvMap { for (key, old_value) in &mise_diff.old { match env_diff_get(&mise_diff.new, key) { - Some(new_value) if env_map_get(&env, key) == Some(new_value) => { + Some(new_value) + if env_map_get(&env, key) + .is_some_and(|v| env_value_matches(mise_diff.v, new_value, v)) => + { let key = env_map_key(&env, key) .cloned() .unwrap_or_else(|| key.clone()); @@ -946,7 +949,7 @@ fn reverse_diff_preserving_overrides(mise_diff: &EnvDiff, mut env: EnvMap) -> En for (key, new_value) in &mise_diff.new { if env_diff_get(&mise_diff.old, key).is_none() - && env_map_get(&env, key) == Some(new_value) + && env_map_get(&env, key).is_some_and(|v| env_value_matches(mise_diff.v, new_value, v)) && let Some(key) = env_map_key(&env, key).cloned() { env.remove(&key); @@ -1423,14 +1426,15 @@ mod tests { #[test] fn test_reverse_diff_preserves_runtime_overrides() { let diff = EnvDiff { + v: crate::env_diff::ENV_STATE_VERSION, old: [ ("CHANGED".into(), "before".into()), ("REMOVED".into(), "before".into()), ] .into(), new: [ - ("ADDED".into(), "managed".into()), - ("CHANGED".into(), "managed".into()), + ("ADDED".into(), crate::env_diff::hash_env_value("managed")), + ("CHANGED".into(), crate::env_diff::hash_env_value("managed")), ] .into(), ..Default::default() @@ -1452,17 +1456,94 @@ mod tests { .into() ); } + #[test] + fn test_reverse_diff_with_hashed_new_values() { + let diff = EnvDiff { + v: crate::env_diff::ENV_STATE_VERSION, + old: [("CHANGED".into(), "before".into())].into(), + new: [ + ("ADDED".into(), crate::env_diff::hash_env_value("managed")), + ("CHANGED".into(), crate::env_diff::hash_env_value("managed")), + ] + .into(), + ..Default::default() + }; + let current = [ + ("ADDED".into(), "managed".into()), + ("CHANGED".into(), "override".into()), + ] + .into(); + // CHANGED was overridden after mise applied it, so it is left alone; + // ADDED is still the managed value and is removed. + assert_eq!( + reverse_diff_preserving_overrides(&diff, current), + [("CHANGED".into(), "override".into())].into() + ); + } + + /// A shell that keeps running across a mise upgrade still holds a `__MISE_DIFF` the + /// previous version wrote with plaintext `new` values. It must reverse cleanly, or the + /// managed variables would be treated as part of the pristine environment. + #[test] + fn test_reverse_diff_with_plaintext_new_values_from_older_mise() { + let diff = EnvDiff { + old: [("CHANGED".into(), "before".into())].into(), + new: [ + ("ADDED".into(), "managed".into()), + ("CHANGED".into(), "managed".into()), + ] + .into(), + ..Default::default() + }; + let current = [ + ("ADDED".into(), "managed".into()), + ("CHANGED".into(), "managed".into()), + ] + .into(); + assert_eq!( + reverse_diff_preserving_overrides(&diff, current), + [("CHANGED".into(), "before".into())].into() + ); + } + + /// The pristine env derived from a version 1 diff must not keep managed values: a changed + /// variable's previous value would otherwise be recorded as `old` in the next diff. + #[test] + fn test_pristine_env_from_plaintext_v1_diff() { + let diff = EnvDiff { + v: 1, + old: [("CHANGED".into(), "before".into())].into(), + new: [ + ("ADDED".into(), "managed".into()), + ("CHANGED".into(), "managed".into()), + ] + .into(), + ..Default::default() + }; + let orig = [ + ("ADDED".into(), "managed".into()), + ("CHANGED".into(), "managed".into()), + ("OTHER".into(), "kept".into()), + ] + .into(); + let pristine = get_pristine_env(&diff, orig); + assert_eq!(pristine.get("CHANGED").map(String::as_str), Some("before")); + assert!(!pristine.contains_key("ADDED")); + assert_eq!(pristine.get("OTHER").map(String::as_str), Some("kept")); + } + #[test] fn test_reverse_diff_restores_unchanged_managed_values() { let diff = EnvDiff { + v: crate::env_diff::ENV_STATE_VERSION, old: [ ("CHANGED".into(), "before".into()), ("REMOVED".into(), "before".into()), ] .into(), new: [ - ("ADDED".into(), "managed".into()), - ("CHANGED".into(), "managed".into()), + ("ADDED".into(), crate::env_diff::hash_env_value("managed")), + ("CHANGED".into(), crate::env_diff::hash_env_value("managed")), ] .into(), ..Default::default() @@ -1485,10 +1566,11 @@ mod tests { #[test] fn test_reverse_diff_preserves_runtime_removals() { let diff = EnvDiff { + v: crate::env_diff::ENV_STATE_VERSION, old: [("CHANGED".into(), "before".into())].into(), new: [ - ("ADDED".into(), "managed".into()), - ("CHANGED".into(), "managed".into()), + ("ADDED".into(), crate::env_diff::hash_env_value("managed")), + ("CHANGED".into(), crate::env_diff::hash_env_value("managed")), ] .into(), ..Default::default() @@ -1503,15 +1585,19 @@ mod tests { #[test] fn test_reverse_diff_matches_environment_keys_case_insensitively_on_windows() { let diff = EnvDiff { + v: crate::env_diff::ENV_STATE_VERSION, old: [ ("Changed".into(), "before".into()), ("MÎSE_FOO".into(), "before-unicode".into()), ] .into(), new: [ - ("Added".into(), "managed".into()), - ("Changed".into(), "managed".into()), - ("MÎSE_FOO".into(), "managed-unicode".into()), + ("Added".into(), crate::env_diff::hash_env_value("managed")), + ("Changed".into(), crate::env_diff::hash_env_value("managed")), + ( + "MÎSE_FOO".into(), + crate::env_diff::hash_env_value("managed-unicode"), + ), ] .into(), ..Default::default() diff --git a/crates/mise-util/src/env_diff.rs b/crates/mise-util/src/env_diff.rs index e6500ae41a5..2ddb864efc4 100644 --- a/crates/mise-util/src/env_diff.rs +++ b/crates/mise-util/src/env_diff.rs @@ -17,8 +17,21 @@ use std::sync::LazyLock as Lazy; use crate::env::PATH_KEY; use crate::file; +/// Schema version of the `__MISE_DIFF` and `__MISE_SESSION` blobs. Version 1 (a blob with no +/// version field) stores the added env values as plaintext; version 2 stores a +/// [`hash_env_value`] digest instead. +pub const ENV_STATE_VERSION: u32 = 2; + +/// What a blob without a version field is. +pub fn legacy_env_state_version() -> u32 { + 1 +} + #[derive(Default, Serialize, Deserialize)] pub struct EnvDiff { + /// See [`ENV_STATE_VERSION`]. Decides how `new` is compared, see [`env_value_matches`]. + #[serde(default = "legacy_env_state_version")] + pub v: u32, #[serde(default)] pub old: IndexMap, #[serde(default)] @@ -27,6 +40,28 @@ pub struct EnvDiff { pub path: Vec, } +const HASH_PREFIX: &str = "blake3:"; + +/// Digest stored in place of an env value in `__MISE_DIFF` and `__MISE_SESSION`. +/// This is not a secret store: a low-entropy value can be brute-forced from its +/// digest, and the value itself is in every child's environment anyway. The +/// point is not to keep a second plaintext copy around. +pub fn hash_env_value(value: &str) -> String { + format!("{HASH_PREFIX}{}", blake3::hash(value.as_bytes()).to_hex()) +} + +/// Whether `actual` is the value recorded as `stored` in a blob of schema `version`. Only the +/// version decides how values are compared: a shell that keeps running across an upgrade still +/// holds a version 1 blob of plaintext, which must reverse cleanly, while a version 2 blob holds +/// digests and a plaintext value that happens to look like one is not a match. +pub fn env_value_matches(version: u32, stored: &str, actual: &str) -> bool { + if version >= 2 { + stored == hash_env_value(actual) + } else { + stored == actual + } +} + #[derive(Debug)] pub enum EnvDiffOperation { Add(String, String), @@ -197,9 +232,31 @@ impl EnvDiff { Ok(rmp_serde::from_slice(&writer[..])?) } + /// Serializes for `__MISE_DIFF`. The `new` values are replaced by + /// [`hash_env_value`] digests: they are only ever compared against the live + /// environment (see [`env_value_matches`]), so secrets mise injected are + /// not duplicated as plaintext in a variable every child process inherits. + /// `old` stays as-is because restoring the environment needs those values. pub fn serialize(&self) -> Result { + #[derive(Serialize)] + struct Wire<'a> { + v: u32, + old: &'a IndexMap, + new: IndexMap<&'a str, String>, + path: &'a [PathBuf], + } + let wire = Wire { + v: ENV_STATE_VERSION, + old: &self.old, + new: self + .new + .iter() + .map(|(k, v)| (k.as_str(), hash_env_value(v))) + .collect(), + path: &self.path, + }; let mut gz = ZlibEncoder::new(Vec::new(), Compression::fast()); - gz.write_all(&rmp_serde::to_vec_named(self)?)?; + gz.write_all(&rmp_serde::to_vec_named(&wire)?)?; Ok(BASE64_STANDARD_NO_PAD.encode(gz.finish()?)) } @@ -223,6 +280,8 @@ impl EnvDiff { pub fn reverse(&self) -> EnvDiff { EnvDiff { + // `new` now holds the plaintext originals. + v: legacy_env_state_version(), old: self.new.clone(), new: self.old.clone(), path: self.path.clone(), @@ -496,4 +555,82 @@ mod tests { assert_eq!(parsed.get("AFTER").map(String::as_str), Some("\"after\"")); assert!(!parsed.keys().any(|k| k.starts_with("BASH_FUNC_"))); } + + #[test] + fn test_serialize_does_not_store_new_values() { + let original: EnvMap = [("CHANGED".into(), "before".into())].into(); + let diff = EnvDiff::new( + &original, + [ + ("CHANGED".to_string(), "after-secret".to_string()), + ("ADDED".to_string(), "added-secret".to_string()), + ], + ); + let raw = diff.serialize().unwrap(); + let back = EnvDiff::deserialize(&raw).unwrap(); + + assert_eq!(back.old.get("CHANGED").map(String::as_str), Some("before")); + for value in back.new.values() { + assert!(!value.contains("secret"), "{value}"); + } + assert!(env_value_matches( + back.v, + &back.new["CHANGED"], + "after-secret" + )); + assert!(!env_value_matches(back.v, &back.new["CHANGED"], "other")); + assert!(env_value_matches( + back.v, + &back.new["ADDED"], + "added-secret" + )); + } + + #[test] + fn test_env_value_matches_by_version() { + // v1: plaintext + assert!(env_value_matches(1, "plain", "plain")); + assert!(!env_value_matches(1, "plain", "other")); + // v2: digests only + assert!(env_value_matches(2, &hash_env_value("plain"), "plain")); + assert!(!env_value_matches(2, "plain", "plain")); + // a plaintext value that looks like a digest is not a match in v2 + let lookalike = hash_env_value("x"); + assert!(!env_value_matches(2, &lookalike, &lookalike)); + // ...and v1 does not hash + assert!(!env_value_matches(1, &hash_env_value("plain"), "plain")); + } + + #[test] + fn test_blob_without_version_is_v1_and_reserializes_as_v2() { + // what the previous mise wrote: no version field, plaintext `new` + #[derive(Serialize)] + struct Legacy { + old: IndexMap, + new: IndexMap, + path: Vec, + } + let legacy = Legacy { + old: IndexMap::new(), + new: [("K".to_string(), "secret".to_string())].into(), + path: vec![], + }; + let mut gz = ZlibEncoder::new(Vec::new(), Compression::fast()); + gz.write_all(&rmp_serde::to_vec_named(&legacy).unwrap()) + .unwrap(); + let raw = BASE64_STANDARD_NO_PAD.encode(gz.finish().unwrap()); + + let diff = EnvDiff::deserialize(&raw).unwrap(); + assert_eq!(diff.v, 1); + assert!(env_value_matches(diff.v, &diff.new["K"], "secret")); + + let rewritten = EnvDiff::deserialize(&diff.serialize().unwrap()).unwrap(); + assert_eq!(rewritten.v, ENV_STATE_VERSION); + assert!(!rewritten.new["K"].contains("secret")); + assert!(env_value_matches( + rewritten.v, + &rewritten.new["K"], + "secret" + )); + } } diff --git a/docs/cache-behavior.md b/docs/cache-behavior.md index a1e6abae14d..64cac545a55 100644 --- a/docs/cache-behavior.md +++ b/docs/cache-behavior.md @@ -63,6 +63,11 @@ a cached environment. Clear or disable the cache if those edits are not reflecte Changes in an external service, such as a rotated secret, are not file changes: choose a suitable TTL or disable environment caching. +mise does not cache secrets. A configuration with an `age`-encrypted value, a sops-encrypted +`_.file`, any directive with `redact = true`, or an env plugin that returns `redact = true` is +not environment-cached at all; mise recomputes its environment every time instead of writing the +decrypted values to disk. + For a command that must recompute environment values, set `MISE_ENV_CACHE=0` before starting mise. For example, in a Node.js project with a `test` script: diff --git a/docs/env-plugin-development.md b/docs/env-plugin-development.md index 6d41a59ab35..4a618f3ceb8 100644 --- a/docs/env-plugin-development.md +++ b/docs/env-plugin-development.md @@ -100,6 +100,7 @@ separate policy if your plugin also accepts absolute paths. | `redact` | Request redaction of returned values in mise's processed output; defaults to `false` | A user's explicit directive-level `redact` option overrides the plugin's preference. +A module whose values are redacted is never environment-cached, whatever `cacheable` says. Redaction does not remove values from the environment and raw task output bypasses it. See [redactions](/environments/#redactions). diff --git a/e2e/env/test_env_age_not_cached b/e2e/env/test_env_age_not_cached new file mode 100644 index 00000000000..3c424efdb28 --- /dev/null +++ b/e2e/env/test_env_age_not_cached @@ -0,0 +1,35 @@ +#!/usr/bin/env bash + +# A decrypted age value is never written to the env cache, even with redact = false. +export MISE_EXPERIMENTAL=true +export MISE_ENV_CACHE=1 +export __MISE_ENV_CACHE_KEY="dGVzdGtleXRlc3RrZXl0ZXN0a2V5dGVzdGtleXRlc3Q=" +rm -f mise.toml age.txt + +mise x age -- age-keygen -o age.txt 2>/dev/null +AGE_PUBLIC_KEY=$(grep "# public key:" age.txt | cut -d: -f2 | tr -d ' ') +export MISE_AGE_KEY +MISE_AGE_KEY=$(grep "AGE-SECRET-KEY-" age.txt) + +mise set AGE_SECRET="my-secret-value" --age-encrypt --age-recipient "$AGE_PUBLIC_KEY" +sed -i.bak 's/^\(AGE_SECRET = { age = "[^"]*"\) }/\1, redact = false }/' mise.toml +assert_contains "cat mise.toml" "redact = false" + +rm -rf "$MISE_STATE_DIR/env-cache" +assert_contains "mise env -s bash" "export AGE_SECRET=my-secret-value" +assert_contains "mise env -s bash" "export AGE_SECRET=my-secret-value" +assert "find '$MISE_STATE_DIR/env-cache' -type f 2>/dev/null | wc -l | tr -d ' '" "0" + +# The same holds for an age value in [vars] that [env] renders through a template. +VARS_LINE=$(grep '^AGE_SECRET = ' mise.toml | sed 's/^AGE_SECRET/VAR_SECRET/; s/, redact = false//') +cat >mise.toml </dev/null | wc -l | tr -d ' '" "0" diff --git a/e2e/env/test_env_cache_plugin_uncacheable b/e2e/env/test_env_cache_plugin_uncacheable new file mode 100644 index 00000000000..5de83506f77 --- /dev/null +++ b/e2e/env/test_env_cache_plugin_uncacheable @@ -0,0 +1,80 @@ +#!/usr/bin/env bash + +# An env module that is not cacheable must keep the whole environment out of the env cache, +# whether it says so itself (cacheable = false) or the user marks its values redact = true. + +export MISE_ENV_CACHE=1 +export __MISE_ENV_CACHE_KEY="dGVzdGtleXRlc3RrZXl0ZXN0a2V5dGVzdGtleXRlc3Q=" + +COUNTER_FILE="$MISE_DATA_DIR/env_cache_test_counter" + +make_plugin() { + local name="$1" cacheable="$2" + local dir="$MISE_DATA_DIR/plugins/$name" + mkdir -p "$dir/hooks" + cat >"$dir/metadata.lua" <"$dir/hooks/mise_env.lua" </dev/null | wc -l | tr -d ' ' +} + +run_three_times() { + rm -rf "$MISE_STATE_DIR/env-cache" + echo 0 >"$COUNTER_FILE" + mise env -s bash >/dev/null 2>&1 + mise env -s bash >/dev/null 2>&1 + mise env -s bash >/dev/null 2>&1 + cat "$COUNTER_FILE" +} + +make_plugin test-uncacheable-env false +cat >"$MISE_CONFIG_DIR/config.toml" <"$MISE_CONFIG_DIR/config.toml" <"$MISE_CONFIG_DIR/config.toml" <"$MISE_CONFIG_DIR/config.toml" </dev/null | wc -l | tr -d ' '" "0" + +# without a secret, the same config is cached as before +cat >"$MISE_CONFIG_DIR/config.toml" </dev/null +assert_directory_exists "$MISE_STATE_DIR/env-cache" diff --git a/e2e/shell/test_fish_exec_env_not_in_argv b/e2e/shell/test_fish_exec_env_not_in_argv new file mode 100644 index 00000000000..3d383e86fb1 --- /dev/null +++ b/e2e/shell/test_fish_exec_env_not_in_argv @@ -0,0 +1,41 @@ +#!/usr/bin/env bash +# shellcheck disable=SC2016 +# `mise exec -- fish` re-applies mise's env from `-C` because fish's config files run first and may +# overwrite what it inherited (see `mise en`, #1697). The values must not be in fish's argv, which +# any local user can read through ps and /proc. + +if ! type -p fish >/dev/null; then + echo "fish not installed, skipping" + exit 0 +fi + +export XDG_CONFIG_HOME="$PWD/xdg" +mkdir -p "$XDG_CONFIG_HOME/fish" +# a user config that clobbers a variable mise set +echo 'set -gx FISH_ENV_SECRET clobbered-by-config' >"$XDG_CONFIG_HOME/fish/config.fish" + +cat <<'TOML' >mise.toml +[env] +FISH_ENV_SECRET = "s3cr3t-fish-marker" +FISH_ENV_MULTILINE = "line1\nline2" +TOML + +# the value mise set wins over the config file, as before +assert "mise x -- fish -c 'echo \$FISH_ENV_SECRET'" "s3cr3t-fish-marker" +assert "mise x -- fish -c 'echo \$FISH_ENV_MULTILINE | string join /'" "line1/line2" +# the temporary carrier variables are gone +assert "mise x -- fish -c 'set -x | grep -c __MISE_FISH_ENV; true'" "0" + +# fish's own argv has the name but not the value +if [ -r /proc/self/cmdline ]; then + assert_not_contains "mise x -- fish -c 'tr \"\\0\" \" \" >() - .difference(&cur_env.keys().collect::>()) - .map(|k| EnvDiffOperation::Remove(k.to_string())) + let mut env_diff = cur_env + .iter() + .filter_map(|(k, v)| match PREV_SESSION.env.get(k) { + Some(prev) if env_value_matches(PREV_SESSION.v, prev, v) => None, + Some(_) => Some(EnvDiffOperation::Change(k.clone(), String::new())), + None => Some(EnvDiffOperation::Add(k.clone(), String::new())), + }) .collect_vec(); - env_diff.extend(removed_keys); + env_diff.extend( + PREV_SESSION + .env + .keys() + .filter(|k| !cur_env.contains_key(*k)) + .map(|k| EnvDiffOperation::Remove(k.clone())), + ); if !env_diff.is_empty() { let env_diff = env_diff.into_iter().map(patch_to_status).join(" "); info!("{}", format_status(&env_diff)); diff --git a/src/config/config_file/mise_toml.rs b/src/config/config_file/mise_toml.rs index 6499b5892e5..672e8b0cab3 100644 --- a/src/config/config_file/mise_toml.rs +++ b/src/config/config_file/mise_toml.rs @@ -2579,6 +2579,17 @@ impl<'de> de::Deserialize<'de> for EnvList { opts.tools = tools.as_bool().unwrap_or(false); } + // The user's explicit `redact` overrides the plugin's own preference. + if let Some(table) = value.as_table_mut() + && let Some(redact) = + table.remove("redact") + { + opts.redact = Some(redact.as_bool().ok_or_else(|| { + de::Error::custom(format!( + "_.{key}.redact must be a boolean" + )) + })?); + } directives.push(EnvDirective::Module( key, value, opts, )); @@ -4931,6 +4942,41 @@ run = "cargo build" "#); } + #[test] + fn test_env_module_redact_option() { + let toml = indoc! {r#" + [env] + _.secret-plugin = { redact = false, vault = "main" } + _.other-plugin = { vault = "main" } + "#} + .to_string(); + let entries = parse(toml).env_entries().unwrap(); + let module = |name: &str| { + entries + .iter() + .find_map(|d| match d { + EnvDirective::Module(n, value, opts) if n == name => Some((value, opts)), + _ => None, + }) + .unwrap() + }; + let (value, opts) = module("secret-plugin"); + assert_eq!(opts.redact, Some(false)); + // `redact` is a mise directive control, not an option for the plugin + assert!(value.get("redact").is_none()); + assert_eq!(value.get("vault").and_then(|v| v.as_str()), Some("main")); + assert_eq!(module("other-plugin").1.redact, None); + } + + #[test] + fn test_env_module_redact_must_be_boolean() { + let err = parse_error("[env]\n_.my-plugin = { redact = \"true\" }\n"); + assert!( + err.contains("_.my-plugin.redact must be a boolean"), + "{err}" + ); + } + #[test] fn test_env_default_invalid_combinations() { let err = parse_error("[env]\nFOO = { value = \"x\", default = \"y\" }\n"); diff --git a/src/config/env_directive/file.rs b/src/config/env_directive/file.rs index 023c846818d..5aa96a42310 100644 --- a/src/config/env_directive/file.rs +++ b/src/config/env_directive/file.rs @@ -48,12 +48,17 @@ impl EnvResults { .extension() .map(|e| e.to_string_lossy().to_string()) .unwrap_or_default(); + let mut sops_used = false; let mut loaded = match ext.as_str() { - "json" => Self::json(config, exec_env, &p, parse_template).await?, - "yaml" => Self::yaml(config, exec_env, &p, parse_template).await?, - "toml" => Self::toml(config, exec_env, &p, parse_template).await?, + "json" => Self::json(config, exec_env, &p, parse_template, &mut sops_used).await?, + "yaml" => Self::yaml(config, exec_env, &p, parse_template, &mut sops_used).await?, + "toml" => Self::toml(config, exec_env, &p, parse_template, &mut sops_used).await?, _ => Self::dotenv(&p, &acc, expand).await?, }; + // Decrypted values must not be written to the env cache. + if sops_used { + ctx.results.has_uncacheable = true; + } // Structured files are literal by default. With `expand = true`, run // their values through the same `$VAR` engine used by `[env]` values // and accumulate key-by-key for same-file references. @@ -80,6 +85,7 @@ impl EnvResults { exec_env: &TeraEnvMap, p: &Path, parse_template: PT, + sops_used: &mut bool, ) -> Result where PT: FnMut(String) -> Result, @@ -92,6 +98,7 @@ impl EnvResults { let raw = file::strip_utf8_bom(&raw); let mut f: Env = serde_json::from_str(raw).wrap_err_with(errfn)?; if !f.sops.is_empty() { + *sops_used = true; let decrypted = sops::decrypt::<_, JsonFileFormat>( config, exec_env, @@ -130,6 +137,7 @@ impl EnvResults { exec_env: &TeraEnvMap, p: &Path, parse_template: PT, + sops_used: &mut bool, ) -> Result where PT: FnMut(String) -> Result, @@ -138,6 +146,7 @@ impl EnvResults { if let Ok(raw) = file::read_to_string(p) { let mut f: Env = serde_yaml::from_str(&raw).wrap_err_with(errfn)?; if !f.sops.is_empty() { + *sops_used = true; let decrypted = sops::decrypt::<_, YamlFileFormat>( config, exec_env, @@ -176,6 +185,7 @@ impl EnvResults { exec_env: &TeraEnvMap, p: &Path, parse_template: PT, + sops_used: &mut bool, ) -> Result where PT: FnMut(String) -> Result, @@ -184,6 +194,7 @@ impl EnvResults { if let Ok(raw) = file::read_to_string(p) { let mut f: Env = toml::from_str(&raw).wrap_err_with(errfn)?; if !f.sops.is_empty() { + *sops_used = true; let decrypted = sops::decrypt::<_, TomlFileFormat>( config, exec_env, @@ -310,8 +321,12 @@ mod tests { file::write(&p, encrypted_toml()).unwrap(); let exec_env = TeraEnvMap::new(); - let env = EnvResults::toml(&config, &exec_env, &p, Ok).await.unwrap(); + let mut sops_used = false; + let env = EnvResults::toml(&config, &exec_env, &p, Ok, &mut sops_used) + .await + .unwrap(); assert_eq!(env.get("SECRET").unwrap(), "mysecret"); + assert!(sops_used, "decrypted sops values must not be env-cached"); restore_env_var("MISE_SOPS_AGE_KEY", prev_age_key); restore_env_var("MISE_SOPS_ROPS", prev_rops); @@ -340,7 +355,9 @@ mod tests { "MISE_SOPS_AGE_KEY_FILE".into(), key_file.to_string_lossy().to_string(), ); - let env = EnvResults::toml(&config, &exec_env, &p, Ok).await.unwrap(); + let env = EnvResults::toml(&config, &exec_env, &p, Ok, &mut false) + .await + .unwrap(); assert_eq!(env.get("SECRET").unwrap(), "mysecret"); restore_env_var("MISE_SOPS_AGE_KEY", prev_age_key); @@ -377,7 +394,9 @@ mod tests { "MISE_SOPS_AGE_KEY_FILE".into(), key_file.to_string_lossy().to_string(), ); - let env = EnvResults::toml(&config, &exec_env, &p, Ok).await.unwrap(); + let env = EnvResults::toml(&config, &exec_env, &p, Ok, &mut false) + .await + .unwrap(); assert_eq!(env.get("SECRET").unwrap(), "mysecret"); restore_env_var("MISE_SOPS_AGE_KEY", prev_age_key); @@ -408,7 +427,7 @@ mod tests { "MISE_SOPS_AGE_KEY_FILE".into(), key_file.to_string_lossy().to_string(), ); - let err = EnvResults::toml(&config, &exec_env, &p, Ok) + let err = EnvResults::toml(&config, &exec_env, &p, Ok, &mut false) .await .unwrap_err(); assert!( @@ -443,7 +462,9 @@ mod tests { let mut exec_env = TeraEnvMap::new(); exec_env.insert("SOPS_AGE_KEY".into(), "not-an-age-key".into()); - let env = EnvResults::toml(&config, &exec_env, &p, Ok).await.unwrap(); + let env = EnvResults::toml(&config, &exec_env, &p, Ok, &mut false) + .await + .unwrap(); assert_eq!(env.get("SECRET").unwrap(), "mysecret"); restore_env_var("MISE_SOPS_AGE_KEY", prev_mise_age_key); @@ -468,7 +489,7 @@ mod tests { file::write(&p, encrypted_toml()).unwrap(); let exec_env = TeraEnvMap::new(); - let err = EnvResults::toml(&config, &exec_env, &p, Ok) + let err = EnvResults::toml(&config, &exec_env, &p, Ok, &mut false) .await .unwrap_err(); assert!( diff --git a/src/config/env_directive/mod.rs b/src/config/env_directive/mod.rs index c5babad0ad5..616cca45fad 100644 --- a/src/config/env_directive/mod.rs +++ b/src/config/env_directive/mod.rs @@ -259,7 +259,8 @@ pub struct EnvResults { pub tool_add_paths: Vec, /// Files to watch for cache invalidation (from modules and _.source directives) pub watch_files: Vec, - /// True if any directive declared cacheable=false or is a dynamic module + /// True if any directive declared cacheable=false, is a dynamic module, or produced a + /// redacted or decrypted (age, sops) value, none of which may be written to the env cache pub has_uncacheable: bool, } @@ -638,6 +639,9 @@ impl EnvResults { } }; + // Decrypted, so never env-cached, even when `redact = false`. + r.has_uncacheable = true; + if resolve_opts.vars { match options.redact { Some(false) => {} @@ -886,6 +890,12 @@ impl EnvResults { resolve_opts.vars, )?; + // A redacted value is a secret by the user's or the directive's own account + // (age values are redacted by default), and the env cache would write it to disk. + if !r.redactions.is_empty() { + r.has_uncacheable = true; + } + Ok(r) } @@ -1331,6 +1341,52 @@ mod tests { assert!(results.env_paths.is_empty()); } + async fn resolve_for_cache_test(directives: Vec) -> EnvResults { + let config = Config::get().await.unwrap(); + EnvResults::resolve( + &config, + BASE_CONTEXT.clone(), + &EnvMap::new(), + directives + .into_iter() + .map(|d| (d, PathBuf::from("/config"))) + .collect(), + EnvResolveOptions { + vars: false, + tools: ToolsFilter::Both, + warn_on_missing_required: false, + }, + ) + .await + .unwrap() + } + + #[tokio::test] + async fn test_plain_value_stays_cacheable() { + let results = resolve_for_cache_test(vec![EnvDirective::Val( + "PLAIN".into(), + "value".into(), + Default::default(), + )]) + .await; + assert!(!results.has_uncacheable); + } + + #[tokio::test] + async fn test_redacted_value_is_uncacheable() { + let options = EnvDirectiveOptions { + redact: Some(true), + ..Default::default() + }; + let results = resolve_for_cache_test(vec![EnvDirective::Val( + "SECRET".into(), + "value".into(), + options, + )]) + .await; + assert!(results.has_uncacheable); + } + #[tokio::test] async fn test_skipped_default_does_not_override_redaction() { let env = EnvMap::from_iter([("SECRET_TOKEN".to_string(), "secret".to_string())]); diff --git a/src/config/mod.rs b/src/config/mod.rs index c520bf386e3..28edd6e9e4b 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1520,8 +1520,16 @@ impl Config { time!("load_env start"); // `mise oci` lets `[oci.env]` satisfy `required`, so what it resolves must not // be cached for a command that enforces it. + // `{{ vars.X }}` renders vars into the env, so a var that is decrypted or redacted + // (see `EnvResults::has_uncacheable`) taints every env value that may use it. Vars + // are loaded with the config, so a missing result is not expected; if it is, assume + // the worst. + let vars_uncacheable = self + .vars_results_cached() + .is_none_or(|vars| vars.has_uncacheable); let cache_enabled = use_cache && CachedNonToolEnv::is_enabled() + && !vars_uncacheable && !env_directive::is_oci_env_satisfying_required(); let cache_key = if cache_enabled { let config_files: Vec<(PathBuf, u64)> = self @@ -1660,6 +1668,7 @@ impl Config { &env_results.redactable_env(&env::PRISTINE_ENV), &env_results.redaction_exclusions, ); + env_results.has_uncacheable |= vars_uncacheable; if cache_enabled && !env_results.has_uncacheable && let Some(cache_key) = cache_key diff --git a/src/env_diff.rs b/src/env_diff.rs index 3c17be17f6d..f784223c1e8 100644 --- a/src/env_diff.rs +++ b/src/env_diff.rs @@ -7,6 +7,7 @@ mod tests { use std::path::PathBuf; use super::*; + use itertools::Itertools; use insta::assert_debug_snapshot; @@ -77,7 +78,14 @@ mod tests { let diff = EnvDiff::new(&new_from_hashmap(), new_to_hashmap()); let serialized = diff.serialize().unwrap(); let deserialized = EnvDiff::deserialize(&serialized).unwrap(); - assert_debug_snapshot!(deserialized.to_patches()); + assert_eq!(deserialized.old, diff.old); + assert_eq!(deserialized.new.keys().collect_vec(), ["b", "c"]); + assert_eq!(deserialized.v, ENV_STATE_VERSION); + assert!(env_value_matches( + deserialized.v, + &deserialized.new["b"], + "3" + )); } #[tokio::test] diff --git a/src/hook_env.rs b/src/hook_env.rs index bf740091376..446141c9f58 100644 --- a/src/hook_env.rs +++ b/src/hook_env.rs @@ -17,7 +17,10 @@ use std::sync::LazyLock as Lazy; use crate::config::{Config, DEFAULT_CONFIG_FILENAMES, Settings, config_file}; use crate::env::PATH_KEY; -use crate::env_diff::{EnvDiffOperation, EnvDiffPatches, EnvMap}; +use crate::env_diff::{ + ENV_STATE_VERSION, EnvDiffOperation, EnvDiffPatches, EnvMap, hash_env_value, + legacy_env_state_version, +}; use crate::hash::hash_to_str; use crate::shell::Shell; use crate::{dirs, duration, env, file, hooks, watch_files}; @@ -492,9 +495,14 @@ fn have_mise_env_vars_been_modified() -> bool { #[derive(Debug, Default, Serialize, Deserialize)] pub struct HookEnvSession { + /// See [`ENV_STATE_VERSION`]. Decides how `env` is compared. + #[serde(default = "legacy_env_state_version")] + pub v: u32, pub loaded_tools: IndexSet, pub loaded_configs: IndexSet, pub config_paths: IndexSet, + /// Env var name to [`hash_env_value`] digest (plaintext in a version 1 session). Values + /// are only compared, so they are not stored. pub env: EnvMap, #[serde(default)] pub aliases: indexmap::IndexMap, @@ -636,9 +644,13 @@ pub async fn build_session( } Ok(HookEnvSession { + v: ENV_STATE_VERSION, dir: dirs::CWD.clone(), env_var_hash: get_mise_env_vars_hashed(), - env, + env: env + .into_iter() + .map(|(k, v)| (k, hash_env_value(&v))) + .collect(), aliases, tera_files: config.tera_files.clone(), watch_files: resolved_watch_files.into_iter().collect(), @@ -886,6 +898,45 @@ mod tests { values.iter().map(|value| value.to_string()).collect() } + #[test] + fn session_version_decides_how_env_is_compared() { + use super::{HookEnvSession, deserialize, serialize}; + use crate::env_diff::{ENV_STATE_VERSION, env_value_matches, hash_env_value}; + + // what the previous mise wrote: no version field, plaintext values + #[derive(serde::Serialize)] + struct Legacy { + loaded_tools: Vec, + loaded_configs: Vec, + config_paths: Vec, + env: std::collections::BTreeMap, + env_var_hash: String, + latest_update: u64, + } + let legacy = serialize(&Legacy { + loaded_tools: vec![], + loaded_configs: vec![], + config_paths: vec![], + env: [("K".to_string(), "secret".to_string())].into(), + env_var_hash: String::new(), + latest_update: 0, + }) + .unwrap(); + let session: HookEnvSession = deserialize(legacy).unwrap(); + assert_eq!(session.v, 1); + assert!(env_value_matches(session.v, &session.env["K"], "secret")); + + let current = HookEnvSession { + v: ENV_STATE_VERSION, + env: [("K".to_string(), hash_env_value("secret"))].into(), + ..Default::default() + }; + let current: HookEnvSession = deserialize(serialize(¤t).unwrap()).unwrap(); + assert_eq!(current.v, ENV_STATE_VERSION); + assert!(env_value_matches(current.v, ¤t.env["K"], "secret")); + assert!(!env_value_matches(current.v, ¤t.env["K"], "other")); + } + #[test] fn force_full_run_records_the_decision_and_reports_not_exiting_early() { let prev = FAST_PATH_FORCED_FULL_RUN.swap(false, Ordering::Relaxed); diff --git a/src/snapshots/mise__env_diff__tests__serialize.snap b/src/snapshots/mise__env_diff__tests__serialize.snap deleted file mode 100644 index 28ef79ea6d5..00000000000 --- a/src/snapshots/mise__env_diff__tests__serialize.snap +++ /dev/null @@ -1,14 +0,0 @@ ---- -source: src/env_diff.rs -expression: deserialized.to_patches() ---- -[ - Change( - "b", - "3", - ), - Add( - "c", - "4", - ), -] diff --git a/src/toolset/toolset_env.rs b/src/toolset/toolset_env.rs index 3734446bdb9..6919b3c90d8 100644 --- a/src/toolset/toolset_env.rs +++ b/src/toolset/toolset_env.rs @@ -573,6 +573,10 @@ impl Toolset { .extend(non_tool_env.watch_files.clone()); } + // The non-tool env is merged into the final env too, so a module there + // that returned cacheable = false must keep the combined env uncached. + env_results.has_uncacheable |= non_tool_env.has_uncacheable; + // Store add_paths separately to maintain consistent PATH ordering env_results.tool_add_paths = add_paths;