Replace setenv with env list supporting host passthrough
This commit is contained in:
+4
-18
@@ -86,14 +86,10 @@ pub struct Args {
|
||||
#[arg(long = "mask", value_name = "PATH", action = clap::ArgAction::Append)]
|
||||
pub mask: Vec<PathBuf>,
|
||||
|
||||
/// Force-set an environment variable inside the sandbox (repeatable)
|
||||
#[arg(
|
||||
long = "setenv",
|
||||
value_name = "KEY=VALUE",
|
||||
value_parser = parse_key_value,
|
||||
action = clap::ArgAction::Append,
|
||||
)]
|
||||
pub setenv: Vec<(String, String)>,
|
||||
/// Configure an env var inside the sandbox (repeatable).
|
||||
/// `KEY` passes through from the host if set; `KEY=VALUE` sets explicitly.
|
||||
#[arg(long = "env", value_name = "KEY[=VALUE]", action = clap::ArgAction::Append)]
|
||||
pub env: Vec<String>,
|
||||
|
||||
/// Force-unset an environment variable inside the sandbox (repeatable)
|
||||
#[arg(long = "unsetenv", value_name = "KEY", action = clap::ArgAction::Append)]
|
||||
@@ -111,13 +107,3 @@ pub struct Args {
|
||||
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
|
||||
pub command_and_args: Vec<OsString>,
|
||||
}
|
||||
|
||||
fn parse_key_value(raw: &str) -> Result<(String, String), String> {
|
||||
let (key, value) = raw
|
||||
.split_once('=')
|
||||
.ok_or_else(|| format!("expected KEY=VALUE, got {raw:?}"))?;
|
||||
if key.is_empty() {
|
||||
return Err(format!("empty key in {raw:?}"));
|
||||
}
|
||||
Ok((key.to_string(), value.to_string()))
|
||||
}
|
||||
|
||||
+135
-31
@@ -1,11 +1,11 @@
|
||||
use std::collections::{BTreeMap, HashMap};
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::ffi::OsString;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use serde::Deserialize;
|
||||
|
||||
use crate::cli::Args;
|
||||
use crate::{SandboxConfig, SandboxError, SandboxMode};
|
||||
use crate::{EnvEntry, SandboxConfig, SandboxError, SandboxMode};
|
||||
|
||||
pub fn build(args: Args, file_config: Option<FileConfig>) -> Result<SandboxConfig, SandboxError> {
|
||||
let (mut globals, mut profile) = match file_config {
|
||||
@@ -26,6 +26,14 @@ pub fn build(args: Args, file_config: Option<FileConfig>) -> Result<SandboxConfi
|
||||
let command = resolve_binary(&command)
|
||||
.ok_or_else(|| SandboxError::CommandNotFound(PathBuf::from(&command)))?;
|
||||
|
||||
let env = dedupe_env_last_wins(parse_env_entries(&merge_vecs(
|
||||
args.env,
|
||||
&profile.env,
|
||||
&globals.env,
|
||||
))?);
|
||||
let unsetenv = merge_vecs(args.unsetenv, &profile.unsetenv, &globals.unsetenv);
|
||||
reject_env_key_conflicts(&env, &unsetenv)?;
|
||||
|
||||
Ok(SandboxConfig {
|
||||
mode: merge_mode(args.blacklist, args.whitelist, &profile, &globals),
|
||||
hardened: merge_flag(
|
||||
@@ -59,8 +67,8 @@ pub fn build(args: Args, file_config: Option<FileConfig>) -> Result<SandboxConfi
|
||||
extra_rw: merge_paths(args.extra_rw, &profile.rw, &globals.rw)?,
|
||||
extra_ro: merge_paths(args.extra_ro, &profile.ro, &globals.ro)?,
|
||||
mask: merge_vecs(args.mask, &profile.mask, &globals.mask),
|
||||
setenv: merge_setenv(args.setenv, &profile.setenv, &globals.setenv),
|
||||
unsetenv: merge_vecs(args.unsetenv, &profile.unsetenv, &globals.unsetenv),
|
||||
env,
|
||||
unsetenv,
|
||||
bwrap_args: split_bwrap_args(merge_vecs(
|
||||
args.bwrap_args,
|
||||
&profile.bwrap_args,
|
||||
@@ -166,19 +174,36 @@ fn merge_vecs<T: Clone>(cli: Vec<T>, profile: &[T], globals: &[T]) -> Vec<T> {
|
||||
globals.iter().chain(profile).cloned().chain(cli).collect()
|
||||
}
|
||||
|
||||
fn merge_setenv(
|
||||
cli: Vec<(String, String)>,
|
||||
profile: &BTreeMap<String, String>,
|
||||
globals: &BTreeMap<String, String>,
|
||||
) -> Vec<(String, String)> {
|
||||
let mut merged: BTreeMap<String, String> = globals.clone();
|
||||
for (k, v) in profile {
|
||||
merged.insert(k.clone(), v.clone());
|
||||
fn parse_env_entries(raw: &[String]) -> Result<Vec<EnvEntry>, SandboxError> {
|
||||
raw.iter()
|
||||
.map(|s| match s.split_once('=') {
|
||||
None if s.is_empty() => Err(SandboxError::InvalidEnvEntry(s.clone())),
|
||||
None => Ok(EnvEntry::Keep(s.clone())),
|
||||
Some(("", _)) => Err(SandboxError::InvalidEnvEntry(s.clone())),
|
||||
Some((key, value)) => Ok(EnvEntry::Set(key.to_string(), value.to_string())),
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
for (k, v) in cli {
|
||||
merged.insert(k, v);
|
||||
|
||||
fn dedupe_env_last_wins(entries: Vec<EnvEntry>) -> Vec<EnvEntry> {
|
||||
let mut seen: HashSet<String> = HashSet::new();
|
||||
let mut result: Vec<EnvEntry> = Vec::new();
|
||||
for entry in entries.into_iter().rev() {
|
||||
if seen.insert(entry.key().to_string()) {
|
||||
result.push(entry);
|
||||
}
|
||||
}
|
||||
result.reverse();
|
||||
result
|
||||
}
|
||||
|
||||
fn reject_env_key_conflicts(env: &[EnvEntry], unsetenv: &[String]) -> Result<(), SandboxError> {
|
||||
let env_keys: HashSet<&str> = env.iter().map(EnvEntry::key).collect();
|
||||
let unsetenv_keys: HashSet<&str> = unsetenv.iter().map(String::as_str).collect();
|
||||
match env_keys.intersection(&unsetenv_keys).next() {
|
||||
Some(&key) => Err(SandboxError::ConflictingEnvKey(key.to_string())),
|
||||
None => Ok(()),
|
||||
}
|
||||
merged.into_iter().collect()
|
||||
}
|
||||
|
||||
fn resolve_command(
|
||||
@@ -295,7 +320,7 @@ pub struct Options {
|
||||
#[serde(default)]
|
||||
pub mask: Vec<PathBuf>,
|
||||
#[serde(default)]
|
||||
pub setenv: BTreeMap<String, String>,
|
||||
pub env: Vec<String>,
|
||||
#[serde(default)]
|
||||
pub unsetenv: Vec<String>,
|
||||
#[serde(default)]
|
||||
@@ -937,22 +962,16 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_setenv_merges_globals_profile_cli() {
|
||||
fn build_env_accumulates_disjoint_keys() {
|
||||
let file_config = FileConfig {
|
||||
options: Options {
|
||||
setenv: BTreeMap::from([
|
||||
("A".into(), "global".into()),
|
||||
("B".into(), "global".into()),
|
||||
]),
|
||||
env: vec!["A=global".into(), "B".into()],
|
||||
..Options::default()
|
||||
},
|
||||
profile: HashMap::from([(
|
||||
"p".into(),
|
||||
Options {
|
||||
setenv: BTreeMap::from([
|
||||
("B".into(), "profile".into()),
|
||||
("C".into(), "profile".into()),
|
||||
]),
|
||||
env: vec!["C=profile".into()],
|
||||
..Options::default()
|
||||
},
|
||||
)]),
|
||||
@@ -960,21 +979,106 @@ mod tests {
|
||||
};
|
||||
let args = Args {
|
||||
profile: Some("p".into()),
|
||||
setenv: vec![("C".into(), "cli".into()), ("D".into(), "cli".into())],
|
||||
env: vec!["D".into()],
|
||||
..args_with_command()
|
||||
};
|
||||
let config = build(args, Some(file_config)).unwrap();
|
||||
assert_eq!(
|
||||
config.setenv,
|
||||
config.env,
|
||||
vec![
|
||||
("A".into(), "global".into()),
|
||||
("B".into(), "profile".into()),
|
||||
("C".into(), "cli".into()),
|
||||
("D".into(), "cli".into()),
|
||||
EnvEntry::Set("A".into(), "global".into()),
|
||||
EnvEntry::Keep("B".into()),
|
||||
EnvEntry::Set("C".into(), "profile".into()),
|
||||
EnvEntry::Keep("D".into()),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_env_later_tier_overrides_earlier() {
|
||||
let file_config = FileConfig {
|
||||
options: Options {
|
||||
env: vec!["A=global".into()],
|
||||
..Options::default()
|
||||
},
|
||||
profile: HashMap::from([(
|
||||
"p".into(),
|
||||
Options {
|
||||
env: vec!["A=profile".into()],
|
||||
..Options::default()
|
||||
},
|
||||
)]),
|
||||
..FileConfig::default()
|
||||
};
|
||||
let args = Args {
|
||||
profile: Some("p".into()),
|
||||
env: vec!["A=cli".into()],
|
||||
..args_with_command()
|
||||
};
|
||||
let config = build(args, Some(file_config)).unwrap();
|
||||
assert_eq!(config.env, vec![EnvEntry::Set("A".into(), "cli".into())]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_env_conflicts_with_unsetenv() {
|
||||
let file_config = FileConfig {
|
||||
options: Options {
|
||||
env: vec!["X=1".into()],
|
||||
unsetenv: vec!["X".into()],
|
||||
..Options::default()
|
||||
},
|
||||
..FileConfig::default()
|
||||
};
|
||||
assert!(matches!(
|
||||
build(args_with_command(), Some(file_config)),
|
||||
Err(SandboxError::ConflictingEnvKey(k)) if k == "X"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_env_keep_conflicts_with_unsetenv() {
|
||||
let file_config = FileConfig {
|
||||
options: Options {
|
||||
env: vec!["X".into()],
|
||||
unsetenv: vec!["X".into()],
|
||||
..Options::default()
|
||||
},
|
||||
..FileConfig::default()
|
||||
};
|
||||
assert!(matches!(
|
||||
build(args_with_command(), Some(file_config)),
|
||||
Err(SandboxError::ConflictingEnvKey(k)) if k == "X"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_env_rejects_empty_key() {
|
||||
let file_config = FileConfig {
|
||||
options: Options {
|
||||
env: vec!["=foo".into()],
|
||||
..Options::default()
|
||||
},
|
||||
..FileConfig::default()
|
||||
};
|
||||
assert!(matches!(
|
||||
build(args_with_command(), Some(file_config)),
|
||||
Err(SandboxError::InvalidEnvEntry(_))
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_env_set_to_empty_string_is_allowed() {
|
||||
let file_config = FileConfig {
|
||||
options: Options {
|
||||
env: vec!["DEBUG=".into()],
|
||||
..Options::default()
|
||||
},
|
||||
..FileConfig::default()
|
||||
};
|
||||
let config = build(args_with_command(), Some(file_config)).unwrap();
|
||||
assert_eq!(config.env, vec![EnvEntry::Set("DEBUG".into(), "".into())]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_unsetenv_accumulates() {
|
||||
let file_config = FileConfig {
|
||||
|
||||
+53
@@ -1,3 +1,15 @@
|
||||
pub fn keepenv_args(keys: &[String], parent_env: &[(String, String)]) -> Vec<String> {
|
||||
let mut args = Vec::new();
|
||||
for key in keys {
|
||||
if let Some(value) = parent_env.iter().find_map(|(k, v)| (k == key).then_some(v)) {
|
||||
args.push("--setenv".to_string());
|
||||
args.push(key.clone());
|
||||
args.push(value.clone());
|
||||
}
|
||||
}
|
||||
args
|
||||
}
|
||||
|
||||
pub fn whitelist_env_args(parent_env: &[(String, String)]) -> Vec<String> {
|
||||
let mut args = vec!["--clearenv".to_string()];
|
||||
for (key, value) in parent_env {
|
||||
@@ -134,3 +146,44 @@ const BLACKLIST_DROP_SUFFIXES: &[&str] = &[
|
||||
"_PRIVATE_KEY",
|
||||
"_CLIENT_SECRET",
|
||||
];
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn keepenv_emits_setenv_for_present_key() {
|
||||
let parent = vec![("XDG_RUNTIME_DIR".into(), "/run/user/1000".into())];
|
||||
let args = keepenv_args(&["XDG_RUNTIME_DIR".into()], &parent);
|
||||
assert_eq!(args, vec!["--setenv", "XDG_RUNTIME_DIR", "/run/user/1000"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keepenv_skips_absent_keys() {
|
||||
let parent = vec![("HOME".into(), "/home/me".into())];
|
||||
let args = keepenv_args(&["XDG_RUNTIME_DIR".into()], &parent);
|
||||
assert!(args.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keepenv_preserves_caller_key_order() {
|
||||
let parent = vec![
|
||||
("B".into(), "2".into()),
|
||||
("A".into(), "1".into()),
|
||||
("C".into(), "3".into()),
|
||||
];
|
||||
let args = keepenv_args(&["A".into(), "B".into(), "C".into()], &parent);
|
||||
assert_eq!(
|
||||
args,
|
||||
vec![
|
||||
"--setenv", "A", "1", "--setenv", "B", "2", "--setenv", "C", "3"
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keepenv_empty_keys_yields_nothing() {
|
||||
let parent = vec![("A".into(), "1".into())];
|
||||
assert!(keepenv_args(&[], &parent).is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -22,6 +22,8 @@ pub enum SandboxError {
|
||||
},
|
||||
ProfileNotFound(String),
|
||||
ConflictingMode,
|
||||
ConflictingEnvKey(String),
|
||||
InvalidEnvEntry(String),
|
||||
UnknownConfigKey(String),
|
||||
ConfigPathNotAbsolute(PathBuf),
|
||||
InvalidBwrapArg(String),
|
||||
@@ -65,6 +67,12 @@ impl std::fmt::Display for SandboxError {
|
||||
f,
|
||||
"config section sets both blacklist and whitelist to true"
|
||||
),
|
||||
Self::ConflictingEnvKey(key) => {
|
||||
write!(f, "conflicting environment variable options for key: {key}")
|
||||
}
|
||||
Self::InvalidEnvEntry(raw) => {
|
||||
write!(f, "invalid env entry (expected KEY or KEY=VALUE): {raw:?}")
|
||||
}
|
||||
Self::UnknownConfigKey(key) => write!(f, "unknown config key: {key}"),
|
||||
Self::ConfigPathNotAbsolute(p) => {
|
||||
write!(f, "config path is not absolute: {}", p.display())
|
||||
|
||||
+16
-1
@@ -20,6 +20,21 @@ pub enum SandboxMode {
|
||||
Whitelist,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum EnvEntry {
|
||||
Keep(String),
|
||||
Set(String, String),
|
||||
}
|
||||
|
||||
impl EnvEntry {
|
||||
pub fn key(&self) -> &str {
|
||||
match self {
|
||||
Self::Keep(k) => k,
|
||||
Self::Set(k, _) => k,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub struct SandboxConfig {
|
||||
pub mode: SandboxMode,
|
||||
pub hardened: bool,
|
||||
@@ -29,7 +44,7 @@ pub struct SandboxConfig {
|
||||
pub extra_rw: Vec<PathBuf>,
|
||||
pub extra_ro: Vec<PathBuf>,
|
||||
pub mask: Vec<PathBuf>,
|
||||
pub setenv: Vec<(String, String)>,
|
||||
pub env: Vec<EnvEntry>,
|
||||
pub unsetenv: Vec<String>,
|
||||
pub bwrap_args: Vec<String>,
|
||||
pub command: PathBuf,
|
||||
|
||||
+14
-2
@@ -5,7 +5,7 @@ use crate::agents;
|
||||
use crate::blacklist;
|
||||
use crate::env;
|
||||
use crate::seccomp;
|
||||
use crate::{SandboxConfig, SandboxError, SandboxMode};
|
||||
use crate::{EnvEntry, SandboxConfig, SandboxError, SandboxMode};
|
||||
|
||||
pub fn build_command(config: &SandboxConfig) -> Result<Command, SandboxError> {
|
||||
let mut cmd = Command::new("bwrap");
|
||||
@@ -71,9 +71,21 @@ fn add_env_policy(cmd: &mut Command, config: &SandboxConfig) {
|
||||
}
|
||||
|
||||
fn add_user_env_overrides(cmd: &mut Command, config: &SandboxConfig) {
|
||||
for (key, value) in &config.setenv {
|
||||
let mut keep_keys: Vec<String> = Vec::new();
|
||||
for entry in &config.env {
|
||||
match entry {
|
||||
EnvEntry::Set(key, value) => {
|
||||
cmd.arg("--setenv").arg(key).arg(value);
|
||||
}
|
||||
EnvEntry::Keep(key) => keep_keys.push(key.clone()),
|
||||
}
|
||||
}
|
||||
|
||||
if !keep_keys.is_empty() {
|
||||
let parent_env: Vec<(String, String)> = std::env::vars().collect();
|
||||
cmd.args(env::keepenv_args(&keep_keys, &parent_env));
|
||||
}
|
||||
|
||||
for key in &config.unsetenv {
|
||||
cmd.arg("--unsetenv").arg(key);
|
||||
}
|
||||
|
||||
+35
-9
@@ -1163,13 +1163,39 @@ fn whitelist_strips_dbus_vars() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn whitelist_setenv_injects_user_var() {
|
||||
fn whitelist_env_sets_user_var() {
|
||||
let stdout = printenv_inside(
|
||||
&["--whitelist", "--setenv", "USER_INJECTED=forced"],
|
||||
&["--whitelist", "--env", "USER_INJECTED=forced"],
|
||||
&[],
|
||||
&["USER_INJECTED"],
|
||||
);
|
||||
assert!(stdout.contains("forced"), "setenv not applied: {stdout}");
|
||||
assert!(stdout.contains("forced"), "env not applied: {stdout}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn whitelist_env_keep_passes_through_host_var() {
|
||||
let stdout = printenv_inside(
|
||||
&["--whitelist", "--env", "PASSED_THROUGH"],
|
||||
&[("PASSED_THROUGH", "from-host")],
|
||||
&["PASSED_THROUGH"],
|
||||
);
|
||||
assert!(
|
||||
stdout.contains("from-host"),
|
||||
"expected --env KEY to pass host value through: {stdout}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn whitelist_env_keep_absent_host_var_is_skipped() {
|
||||
let stdout = printenv_inside(
|
||||
&["--whitelist", "--env", "NEVER_SET_ON_HOST"],
|
||||
&[],
|
||||
&["NEVER_SET_ON_HOST"],
|
||||
);
|
||||
assert!(
|
||||
stdout.contains("MISSING:NEVER_SET_ON_HOST"),
|
||||
"expected absent keep-var to remain unset: {stdout}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1301,28 +1327,28 @@ fn no_env_filter_blacklist_keeps_secrets() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_env_filter_still_honors_user_setenv() {
|
||||
fn no_env_filter_still_honors_user_env() {
|
||||
let stdout = printenv_inside(
|
||||
&["--no-env-filter", "--setenv", "FORCED=yes"],
|
||||
&["--no-env-filter", "--env", "FORCED=yes"],
|
||||
&[],
|
||||
&["FORCED"],
|
||||
);
|
||||
assert!(
|
||||
stdout.contains("yes"),
|
||||
"expected user --setenv to still work with --no-env-filter, got: {stdout}"
|
||||
"expected user --env to still work with --no-env-filter, got: {stdout}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn blacklist_setenv_overrides_builtin_deny() {
|
||||
fn blacklist_env_overrides_builtin_deny() {
|
||||
let stdout = printenv_inside(
|
||||
&["--setenv", "GH_TOKEN=overridden"],
|
||||
&["--env", "GH_TOKEN=overridden"],
|
||||
&[("GH_TOKEN", "original")],
|
||||
&["GH_TOKEN"],
|
||||
);
|
||||
assert!(
|
||||
stdout.contains("overridden"),
|
||||
"expected --setenv to override deny, got: {stdout}"
|
||||
"expected --env to override deny, got: {stdout}"
|
||||
);
|
||||
assert!(!stdout.contains("original"));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user