From b4d06e89609dfd5a8cbd4cc834054e5cd7a201bf Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 14 Aug 2026 14:48:04 -0700 Subject: [PATCH 1/6] core: add profile variable expansion grammar Profiles are not portable between machines: a grant under the author's home has to be hand-edited wherever the username differs. CLI flags avoid this because the shell expands them first, but a profile has no shell. This adds the grammar only, not the wiring. Every unrecognised form is a hard error so that adding variables later cannot silently reinterpret a grant that exists today. The fixture is shared with the Python loader so the two implementations cannot drift. Signed-off-by: Cong Wang --- crates/sandlock-core/Cargo.toml | 2 +- crates/sandlock-core/src/expand.rs | 198 ++++++++++++++++++++++++++ crates/sandlock-core/src/lib.rs | 1 + tests/fixtures/profile_expansion.toml | 93 ++++++++++++ 4 files changed, 293 insertions(+), 1 deletion(-) create mode 100644 crates/sandlock-core/src/expand.rs create mode 100644 tests/fixtures/profile_expansion.toml diff --git a/crates/sandlock-core/Cargo.toml b/crates/sandlock-core/Cargo.toml index 67c232b3..90e59b8f 100644 --- a/crates/sandlock-core/Cargo.toml +++ b/crates/sandlock-core/Cargo.toml @@ -11,7 +11,7 @@ description = "Lightweight process sandbox using Landlock, seccomp-bpf, and secc [dependencies] libc = "0.2" syscalls = { version = "0.8", default-features = false } -nix = { version = "0.29", features = ["process", "signal", "fs", "ioctl", "poll"] } +nix = { version = "0.29", features = ["process", "signal", "fs", "ioctl", "poll", "user"] } tokio = { version = "1", features = ["rt", "net", "time", "sync", "macros", "io-util", "fs"] } serde = { version = "1", features = ["derive"] } thiserror = "2" diff --git a/crates/sandlock-core/src/expand.rs b/crates/sandlock-core/src/expand.rs new file mode 100644 index 00000000..ec0c36bd --- /dev/null +++ b/crates/sandlock-core/src/expand.rs @@ -0,0 +1,198 @@ +//! Variable expansion for profile path values. +//! +//! Every unrecognised form is an error rather than a literal, so adding a +//! variable later cannot silently reinterpret a grant written today. + +use crate::error::{SandboxError, SandlockError}; + +/// The closed vocabulary. Values are resolved by sandlock, never looked up +/// in the environment by name. +const VARS: &[&str] = &["HOME"]; + +fn invalid(msg: String) -> SandlockError { + SandlockError::Sandbox(SandboxError::Invalid(msg)) +} + +/// Resolve `${HOME}`. +/// +/// The environment wins over passwd because the sandboxed program resolves +/// its own `~` through `$HOME`: a passwd-derived grant would cover a +/// directory the program never opens while denying the one it does. +pub fn resolve_home() -> Result { + let env = std::env::var("HOME").ok(); + let uid = nix::unistd::Uid::current(); + let passwd = nix::unistd::User::from_uid(uid) + .ok() + .flatten() + .map(|u| u.dir.to_string_lossy().into_owned()); + resolve_home_from(env.as_deref(), passwd.as_deref()) +} + +/// Split out from `resolve_home` so the precedence rule is testable without +/// mutating the process environment. +fn resolve_home_from(env: Option<&str>, passwd: Option<&str>) -> Result { + for candidate in [env, passwd].into_iter().flatten() { + if candidate.starts_with('/') { + return Ok(candidate.to_string()); + } + } + Err(invalid( + "cannot resolve ${HOME}: $HOME is unset or not absolute, and this uid \ + has no passwd entry with an absolute home directory" + .to_string(), + )) +} + +/// Expand `${HOME}` in one profile path value. +pub fn expand(value: &str, home: &str) -> Result { + if value.starts_with('~') { + return Err(invalid(format!( + "{value:?}: tilde is not expanded in profiles; write ${{HOME}} instead" + ))); + } + let mut out = String::with_capacity(value.len()); + let mut rest = value; + while let Some(pos) = rest.find('$') { + out.push_str(&rest[..pos]); + let after = &rest[pos + 1..]; + if let Some(tail) = after.strip_prefix('{') { + let end = tail + .find('}') + .ok_or_else(|| invalid(format!("{value:?}: unterminated ${{")))?; + out.push_str(lookup(&tail[..end], home, value)?); + rest = &tail[end + 1..]; + } else { + return Err(invalid(bare_dollar_message(value, after))); + } + } + out.push_str(rest); + Ok(out) +} + +fn lookup<'a>(name: &str, home: &'a str, value: &str) -> Result<&'a str, SandlockError> { + if !is_well_formed(name) { + return Err(invalid(format!( + "{value:?}: malformed variable name ${{{name}}}; names match [A-Za-z_][A-Za-z0-9_]*" + ))); + } + if name == "HOME" { + return Ok(home); + } + let suggestion = VARS + .iter() + .find(|v| v.eq_ignore_ascii_case(name)) + .map(|v| format!("; did you mean ${{{v}}}?")) + .unwrap_or_default(); + Err(invalid(format!( + "{value:?}: unknown variable ${{{name}}}{suggestion}; supported: {}", + VARS.iter() + .map(|v| format!("${{{v}}}")) + .collect::>() + .join(", ") + ))) +} + +fn is_well_formed(name: &str) -> bool { + let mut chars = name.chars(); + match chars.next() { + Some(c) if c.is_ascii_alphabetic() || c == '_' => {} + _ => return false, + } + chars.all(|c| c.is_ascii_alphanumeric() || c == '_') +} + +fn bare_dollar_message(value: &str, after: &str) -> String { + let name: String = after + .chars() + .take_while(|c| c.is_ascii_alphanumeric() || *c == '_') + .collect(); + if name.is_empty() { + format!("{value:?}: bare $ is not allowed; a literal $ cannot appear in a profile path") + } else { + format!("{value:?}: bare $ is not a variable; write ${{{name}}} for a variable") + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde::Deserialize; + + #[derive(Deserialize)] + struct Fixture { + home: String, + case: Vec, + } + + #[derive(Deserialize)] + struct Case { + input: String, + home: Option, + expect: Option, + error: Option, + } + + #[test] + fn fixture_cases() { + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../tests/fixtures/profile_expansion.toml" + ); + let text = std::fs::read_to_string(path).expect("read fixture"); + let fixture: Fixture = toml::from_str(&text).expect("parse fixture"); + + for c in &fixture.case { + let home = c.home.as_deref().unwrap_or(&fixture.home); + let got = expand(&c.input, home); + match (&c.expect, &c.error) { + (Some(want), None) => { + let got = got.unwrap_or_else(|e| panic!("{:?}: unexpected error {e}", c.input)); + assert_eq!(&got, want, "input {:?}", c.input); + } + (None, Some(want)) => { + let err = got.expect_err(&format!("{:?}: expected an error", c.input)); + let msg = err.to_string(); + assert!( + msg.contains(want), + "input {:?}: error {msg:?} does not contain {want:?}", + c.input + ); + } + _ => panic!("{:?}: case needs exactly one of expect/error", c.input), + } + } + } + + #[test] + fn resolve_home_prefers_absolute_env() { + assert_eq!( + resolve_home_from(Some("/env/home"), Some("/passwd/home")).unwrap(), + "/env/home" + ); + } + + #[test] + fn resolve_home_falls_back_to_passwd() { + // Unset, empty, and relative all fail the absolute test, so passwd wins. + for env in [None, Some(""), Some("relative/home")] { + assert_eq!( + resolve_home_from(env, Some("/passwd/home")).unwrap(), + "/passwd/home", + "env {env:?}" + ); + } + } + + #[test] + fn resolve_home_errors_when_neither_is_absolute() { + let err = resolve_home_from(None, None).unwrap_err().to_string(); + assert!(err.contains("cannot resolve ${HOME}"), "error was {err:?}"); + assert!(resolve_home_from(Some("rel"), Some("also-rel")).is_err()); + } + + #[test] + fn resolve_home_on_this_host_is_absolute() { + let home = resolve_home().expect("resolve home"); + assert!(home.starts_with('/'), "home {home:?} must be absolute"); + } +} diff --git a/crates/sandlock-core/src/lib.rs b/crates/sandlock-core/src/lib.rs index 5b30f3e3..b0771fd8 100644 --- a/crates/sandlock-core/src/lib.rs +++ b/crates/sandlock-core/src/lib.rs @@ -3,6 +3,7 @@ pub mod http; pub(crate) mod credential; pub mod sandbox; // formerly `policy`; contains Sandbox + SandboxBuilder + Confinement pub mod profile; +pub mod expand; pub mod result; pub(crate) mod arch; pub(crate) mod sys; diff --git a/tests/fixtures/profile_expansion.toml b/tests/fixtures/profile_expansion.toml new file mode 100644 index 00000000..60c9db63 --- /dev/null +++ b/tests/fixtures/profile_expansion.toml @@ -0,0 +1,93 @@ +# Shared expansion cases for the Rust and Python profile loaders. +# Both implementations must agree on every case here. +# Each case has `input` plus exactly one of `expect` (success) or +# `error` (a substring that must appear in the error message). + +home = "/home/alice" + +[[case]] +input = "/usr/lib" +expect = "/usr/lib" + +[[case]] +input = "${HOME}" +expect = "/home/alice" + +[[case]] +input = "${HOME}/src" +expect = "/home/alice/src" + +[[case]] +input = "/prefix${HOME}" +expect = "/prefix/home/alice" + +[[case]] +input = "${HOME}/a/${HOME}/b" +expect = "/home/alice/a//home/alice/b" + +[[case]] +input = "$$" +error = "bare $" + +[[case]] +input = "/opt/foo$$bar" +error = "bare $" + +[[case]] +input = "/opt/~x" +expect = "/opt/~x" + +# Substituted text is never rescanned: a home containing ${HOME} must +# survive verbatim. +[[case]] +input = "${HOME}/x" +home = "/home/${HOME}" +expect = "/home/${HOME}/x" + +[[case]] +input = "${UNKNOWN}" +error = "unknown variable" + +[[case]] +input = "${home}" +error = "did you mean ${HOME}" + +[[case]] +input = "${Home}" +error = "did you mean ${HOME}" + +[[case]] +input = "${}" +error = "malformed variable name" + +[[case]] +input = "${HO-ME}" +error = "malformed variable name" + +[[case]] +input = "${1FOO}" +error = "malformed variable name" + +[[case]] +input = "${HOME" +error = "unterminated" + +[[case]] +input = "$HOME/src" +error = "write ${HOME}" + +[[case]] +input = "/opt/$" +error = "bare $" + +[[case]] +input = "~" +error = "tilde is not expanded" + +[[case]] +input = "~/src" +error = "tilde is not expanded" + +[[case]] +input = "~alice/src" +error = "tilde is not expanded" From 3bfe22dc3b902b5a0a0483fbc462c6bf4b6873a7 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 14 Aug 2026 14:55:41 -0700 Subject: [PATCH 2/6] profile: expand ${HOME} in path fields Expansion runs in parse_input, the ProfileInput to Sandbox step, rather than during deserialization. Keeping it out of Deserialize is what lets 'learn --merge' read a profile and write it back with the author's ${HOME} still spelled as a variable. Home resolution is lazy so a profile that uses no variables still loads on a host where home cannot be resolved. Mount specs expand after the VIRTUAL:HOST split so a resolved value containing a colon cannot be read as a separator. Signed-off-by: Cong Wang --- .../sandlock-cli/tests/profile_integration.rs | 39 ++++++ crates/sandlock-core/src/profile.rs | 118 ++++++++++++++++-- 2 files changed, 145 insertions(+), 12 deletions(-) diff --git a/crates/sandlock-cli/tests/profile_integration.rs b/crates/sandlock-cli/tests/profile_integration.rs index e2a825f6..6b003e5a 100644 --- a/crates/sandlock-cli/tests/profile_integration.rs +++ b/crates/sandlock-cli/tests/profile_integration.rs @@ -158,3 +158,42 @@ fn no_supervisor_rejects_supervisor_only_profile_fields() { stderr, ); } + +#[test] +fn profile_home_variable_grants_real_access() { + let home = std::env::var("HOME").expect("HOME set"); + let probe = std::path::PathBuf::from(&home).join(".sandlock-expand-probe"); + std::fs::write(&probe, b"ok").expect("write probe"); + + let tmp = tempfile::tempdir().unwrap(); + let profile_path = tmp.path().join("expand.toml"); + // The write grant is what proves expansion happened: it is mandatory, so + // a literal "${HOME}" would abort the run on a path that does not exist. + std::fs::write(&profile_path, format!(r#" + [filesystem] + {read} + write = ["${{HOME}}/.sandlock-expand-probe"] + "#, read = read_list())).unwrap(); + + let out = sandlock_bin() + .args([ + "run", + "--profile-file", + profile_path.to_str().unwrap(), + "--fs-read", + probe.to_str().unwrap(), + "--", + "/bin/cat", + probe.to_str().unwrap(), + ]) + .output() + .expect("spawn sandlock"); + + std::fs::remove_file(&probe).ok(); + assert!( + out.status.success(), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!(String::from_utf8_lossy(&out.stdout).trim(), "ok"); +} diff --git a/crates/sandlock-core/src/profile.rs b/crates/sandlock-core/src/profile.rs index 875d3202..a7d7577c 100644 --- a/crates/sandlock-core/src/profile.rs +++ b/crates/sandlock-core/src/profile.rs @@ -197,6 +197,37 @@ impl ProfileInput { } } +/// Lazily resolves `${HOME}` so a profile that uses no variables still loads +/// on a host where home cannot be resolved. +struct Expander { + home: Option, +} + +impl Expander { + fn new() -> Self { + Self { home: None } + } + + fn path(&mut self, field: &str, p: &std::path::Path) -> Result { + Ok(PathBuf::from(self.text(field, &p.to_string_lossy())?)) + } + + fn text(&mut self, field: &str, s: &str) -> Result { + if !s.contains('$') && !s.starts_with('~') { + return Ok(s.to_string()); + } + if self.home.is_none() { + self.home = Some(crate::expand::resolve_home()?); + } + crate::expand::expand(s, self.home.as_deref().unwrap()).map_err(|e| match e { + SandlockError::Sandbox(crate::error::SandboxError::Invalid(m)) => { + SandlockError::Sandbox(crate::error::SandboxError::Invalid(format!("{field}: {m}"))) + } + other => other, + }) + } +} + /// Convert a parsed `ProfileInput` into a `(Sandbox, ProgramSpec)` pair. /// /// Forwards each schema section's fields to the corresponding `SandboxBuilder` @@ -205,14 +236,17 @@ impl ProfileInput { /// that lack `FromStr` impls on their target types. pub fn parse_input(input: ProfileInput) -> Result<(Sandbox, ProgramSpec), SandlockError> { let mut b = Sandbox::builder(); + let mut ex = Expander::new(); // [config] - if let Some(p) = input.config.http_ca { b = b.http_ca(p); } - if let Some(p) = input.config.http_key { b = b.http_key(p); } - for p in input.config.http_inject_ca { b = b.http_inject_ca(p); } - if let Some(p) = input.config.http_ca_out { b = b.http_ca_out(p); } - if let Some(p) = input.config.fs_storage { b = b.fs_storage(p); } - if let Some(p) = input.config.workdir { b = b.workdir(p); } + if let Some(p) = input.config.http_ca { b = b.http_ca(ex.path("[config].http_ca", &p)?); } + if let Some(p) = input.config.http_key { b = b.http_key(ex.path("[config].http_key", &p)?); } + for p in input.config.http_inject_ca.iter() { + b = b.http_inject_ca(ex.path("[config].http_inject_ca", p)?); + } + if let Some(p) = input.config.http_ca_out { b = b.http_ca_out(ex.path("[config].http_ca_out", &p)?); } + if let Some(p) = input.config.fs_storage { b = b.fs_storage(ex.path("[config].fs_storage", &p)?); } + if let Some(p) = input.config.workdir { b = b.workdir(ex.path("[config].workdir", &p)?); } // [determinism] if let Some(s) = input.determinism.random_seed { b = b.random_seed(s); } @@ -224,7 +258,7 @@ pub fn parse_input(input: ProfileInput) -> Result<(Sandbox, ProgramSpec), Sandlo // [program] — process knobs go to Sandbox; exec/args go to ProgramSpec. for (k, v) in input.program.env.iter() { b = b.env_var(k, v); } - if let Some(c) = input.program.cwd { b = b.cwd(c); } + if let Some(c) = input.program.cwd { b = b.cwd(ex.path("[program].cwd", &c)?); } match (input.program.uid, input.program.gid) { (Some(u), Some(g)) => b = b.user(u, g), (None, None) => {} @@ -237,12 +271,16 @@ pub fn parse_input(input: ProfileInput) -> Result<(Sandbox, ProgramSpec), Sandlo if input.program.no_huge_pages { b = b.no_huge_pages(true); } // [filesystem] - for p in input.filesystem.read.iter() { b = b.fs_read(p); } - for p in input.filesystem.write.iter() { b = b.fs_write(p); } - for p in input.filesystem.deny.iter() { b = b.fs_deny(p); } - if let Some(c) = input.filesystem.chroot { b = b.chroot(c); } + for p in input.filesystem.read.iter() { b = b.fs_read(ex.path("[filesystem].read", p)?); } + for p in input.filesystem.write.iter() { b = b.fs_write(ex.path("[filesystem].write", p)?); } + for p in input.filesystem.deny.iter() { b = b.fs_deny(ex.path("[filesystem].deny", p)?); } + if let Some(c) = input.filesystem.chroot { b = b.chroot(ex.path("[filesystem].chroot", &c)?); } for spec in input.filesystem.mount.iter() { let (virt, host, read_only) = parse_mount_spec(spec)?; + // Expand after the split so a resolved value containing a colon + // cannot be read as a spec separator. + let virt = ex.path("[filesystem].mount", &virt)?; + let host = ex.path("[filesystem].mount", &host)?; b = if read_only { b.fs_mount_ro(virt, host) } else { b.fs_mount(virt, host) }; } if let Some(s) = input.filesystem.on_exit.as_deref() { b = b.on_exit(parse_branch_action(s)?); } @@ -292,8 +330,12 @@ pub fn parse_input(input: ProfileInput) -> Result<(Sandbox, ProgramSpec), Sandlo if let Some(c) = input.limits.cpu_cores { b = b.cpu_cores(c); } if let Some(n) = input.limits.num_cpus { b = b.num_cpus(n); } + let exec = match input.program.exec { + Some(p) => Some(ex.path("[program].exec", &p)?), + None => None, + }; let policy = b.build()?; - let spec = ProgramSpec { exec: input.program.exec, args: input.program.args }; + let spec = ProgramSpec { exec, args: input.program.args }; Ok((policy, spec)) } @@ -620,6 +662,58 @@ pub fn list_profiles() -> Result, SandlockError> { mod tests { use super::*; + #[test] + fn parse_profile_expands_home_in_path_fields() { + let home = crate::expand::resolve_home().unwrap(); + let toml = r#" + [filesystem] + read = ["${HOME}/src"] + write = ["${HOME}/out"] + mount = ["/work:${HOME}/host:ro"] + + [program] + cwd = "${HOME}/src" + args = ["${HOME}"] + "#; + let (policy, _spec) = parse_profile(toml).unwrap(); + assert_eq!(policy.fs_readable, vec![PathBuf::from(format!("{home}/src"))]); + assert_eq!(policy.fs_writable, vec![PathBuf::from(format!("{home}/out"))]); + assert_eq!(policy.cwd, Some(PathBuf::from(format!("{home}/src")))); + } + + #[test] + fn parse_profile_leaves_program_args_untouched() { + let toml = r#" + [program] + args = ["${HOME}", "$PATH", "~/x"] + "#; + let (_policy, spec) = parse_profile(toml).unwrap(); + assert_eq!(spec.args, vec!["${HOME}", "$PATH", "~/x"]); + } + + #[test] + fn parse_profile_reports_the_offending_field() { + let toml = r#" + [filesystem] + read = ["${NOPE}/src"] + "#; + let err = parse_profile(toml).unwrap_err().to_string(); + assert!(err.contains("[filesystem].read"), "error was {err:?}"); + assert!(err.contains("unknown variable"), "error was {err:?}"); + } + + #[test] + fn parse_profile_without_variables_never_resolves_home() { + // A profile with no variables must load even where HOME cannot be + // resolved, so resolution has to stay lazy. + let toml = r#" + [filesystem] + read = ["/usr/lib"] + "#; + let (policy, _spec) = parse_profile(toml).unwrap(); + assert_eq!(policy.fs_readable, vec![PathBuf::from("/usr/lib")]); + } + #[test] fn sandbox_to_profile_hides_cow_upper_grant() { // Simulate the spawn-time upper grant: pushed into fs_readable with From 672cc9be2bafd121f0e59cbfd6c761a81d747b12 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 14 Aug 2026 15:13:18 -0700 Subject: [PATCH 3/6] learn: dedup merges against expanded paths A merge compared raw strings, so observing an access under ${HOME}/.config appended this machine absolute path alongside the variable. Repeated merges would fill a shared profile with machine-specific duplicates and undo the portability the variable exists to provide. Dedup now keys on the expanded path while the raw entry is what gets written back. An observed path can carry a literal $ the grammar refuses to expand; it keys on its raw spelling so an odd filename cannot abort the merge. Signed-off-by: Cong Wang --- crates/sandlock-cli/src/learn.rs | 49 ++++++++++++++++++------- crates/sandlock-cli/tests/learn_test.rs | 32 ++++++++++++++++ 2 files changed, 68 insertions(+), 13 deletions(-) diff --git a/crates/sandlock-cli/src/learn.rs b/crates/sandlock-cli/src/learn.rs index 40dfcc07..e86b3363 100644 --- a/crates/sandlock-cli/src/learn.rs +++ b/crates/sandlock-cli/src/learn.rs @@ -3,7 +3,7 @@ //! Runs a workload under observation and emits a sandlock profile TOML //! usable by `sandlock run -p`. -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::path::PathBuf; use std::sync::{Arc, Mutex}; @@ -702,7 +702,7 @@ pub async fn run(args: LearnArgs) -> Result<()> { // TODO: learn limits.open_files via supervisor once open_files enforcement is implemented. // --merge: union observed profile into an existing one. - // Start from the existing profile so all fields we don't observe + // Start from the existing profile so all fields we don't observe // are preserved. Then union in the observed fields. if let Some(ref merge_path) = args.merge { let existing_toml = std::fs::read_to_string(merge_path) @@ -714,19 +714,42 @@ pub async fn run(args: LearnArgs) -> Result<()> { let observed = profile_out; profile_out = existing.clone(); - // Union filesystem paths, re-run dedup after merging. - let merged_reads: Vec = { - let mut set: BTreeSet = observed.filesystem.read.iter().cloned().collect(); - set.extend(existing.filesystem.read.iter().cloned()); - dedup_subsumed(set.into_iter().collect()) + // Union filesystem paths, re-run dedup after merging. Dedup keys on + // the expanded path but writes back the raw entry, so a profile's + // ${HOME} survives instead of being replaced by this machine's + // absolute path. + let mut home: Option = None; + let mut expand_key = |p: &PathBuf| -> Result { + let s = p.to_string_lossy(); + if !s.contains('$') { + return Ok(p.clone()); + } + if home.is_none() { + home = Some(sandlock_core::expand::resolve_home()?); + } + // Observed kernel paths can contain a literal $, which the grammar + // rejects; key them raw so an odd filename cannot abort the merge. + match sandlock_core::expand::expand(&s, home.as_deref().unwrap()) { + Ok(e) => Ok(PathBuf::from(e)), + Err(_) => Ok(p.clone()), + } }; - let merged_writes: Vec = { - let mut set: BTreeSet = observed.filesystem.write.iter().cloned().collect(); - set.extend(existing.filesystem.write.iter().cloned()); - dedup_subsumed(set.into_iter().collect()) + let mut merge_paths = |observed: &[PathBuf], existing: &[PathBuf]| -> Result> { + let mut by_expanded: BTreeMap = BTreeMap::new(); + for p in observed { + by_expanded.insert(expand_key(p)?, p.clone()); + } + // Existing entries land second so their raw spelling wins. + for p in existing { + by_expanded.insert(expand_key(p)?, p.clone()); + } + let kept = dedup_subsumed(by_expanded.keys().cloned().collect()); + Ok(kept.into_iter().map(|k| by_expanded[&k].clone()).collect()) }; - profile_out.filesystem.read = merged_reads; - profile_out.filesystem.write = merged_writes; + profile_out.filesystem.read = + merge_paths(&observed.filesystem.read, &existing.filesystem.read)?; + profile_out.filesystem.write = + merge_paths(&observed.filesystem.write, &existing.filesystem.write)?; // Union network.allow. let mut allow_set: std::collections::BTreeSet = diff --git a/crates/sandlock-cli/tests/learn_test.rs b/crates/sandlock-cli/tests/learn_test.rs index af92abcd..08a60373 100644 --- a/crates/sandlock-cli/tests/learn_test.rs +++ b/crates/sandlock-cli/tests/learn_test.rs @@ -956,4 +956,36 @@ fn test_learn_captures_https_request() { assert!(profile.contains(ca_bundle), "expected ca bundle path in [config]: {profile}"); } +/// --merge keeps a hand-written ${HOME} and does not append its expanded twin. +#[test] +fn test_learn_merge_preserves_home_variable() { + let home = std::env::var("HOME").expect("HOME set"); + let probe_dir = std::path::PathBuf::from(&home).join(".config"); + std::fs::create_dir_all(&probe_dir).expect("create probe dir"); + let probe = probe_dir.join("sandlock-merge-probe"); + std::fs::write(&probe, b"ok").expect("write probe"); + + let profile = tempfile::NamedTempFile::new().expect("tempfile"); + let profile_path = profile.path().to_str().unwrap().to_owned(); + std::fs::write(&profile_path, "[filesystem]\nread = [\"${HOME}/.config\", \"/usr\"]\n") + .expect("seed profile"); + let learn = sandlock_bin() + .args(["learn", "--merge", &profile_path, "--", "cat", probe.to_str().unwrap()]) + .output() + .expect("failed to run sandlock learn --merge"); + let _ = std::fs::remove_file(&probe); + assert!(learn.status.success(), + "merge learn failed: {}", String::from_utf8_lossy(&learn.stderr)); + + let merged = std::fs::read_to_string(&profile_path).expect("read merged profile"); + let parsed: sandlock_core::ProfileInput = toml::from_str(&merged).expect("parse merged"); + let reads: Vec = parsed.filesystem.read.iter() + .map(|p| p.to_string_lossy().into_owned()) + .collect(); + + assert!(reads.iter().any(|r| r == "${HOME}/.config"), + "merge dropped the ${{HOME}} spelling: {reads:?}"); + assert!(!reads.iter().any(|r| r.starts_with(&format!("{home}/.config"))), + "merge added a machine-specific duplicate of ${{HOME}}/.config: {reads:?}"); +} From e716f8bdd04889f38d887fdd374c785ff907076a Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 14 Aug 2026 15:15:06 -0700 Subject: [PATCH 4/6] python: expand ${HOME} in profile path fields The Python loader parses profiles independently of the Rust one, so a profile that expanded under the CLI would otherwise fail to expand under the SDK. Both now run the same grammar against the same shared fixture, which is what keeps the two from drifting. Home resolution stays lazy here too, so a profile with no variables loads even where home cannot be resolved. Signed-off-by: Cong Wang --- python/src/sandlock/_profile.py | 130 +++++++++++++++++++++++++++++++- python/tests/test_profile.py | 77 +++++++++++++++++++ 2 files changed, 204 insertions(+), 3 deletions(-) diff --git a/python/src/sandlock/_profile.py b/python/src/sandlock/_profile.py index da6b919a..d78c1311 100644 --- a/python/src/sandlock/_profile.py +++ b/python/src/sandlock/_profile.py @@ -30,6 +30,8 @@ from __future__ import annotations +import os +import pwd import sys if sys.version_info >= (3, 11): @@ -115,6 +117,108 @@ } +_VARS = ("HOME",) + +# Sandbox attribute names whose values are paths. Program args and env are +# data belonging to the sandboxed program and are never expanded. +_PATH_KEYS = frozenset({ + "http_ca", "http_key", "http_ca_out", "fs_storage", "workdir", "cwd", "chroot", +}) +_PATH_LIST_KEYS = frozenset({ + "http_inject_ca", "fs_readable", "fs_writable", "fs_denied", +}) + + +def _resolve_home() -> str: + """Resolve ``${HOME}``. + + The environment wins over passwd because the sandboxed program resolves + its own ``~`` through ``$HOME``. + """ + env = os.environ.get("HOME") + if env and env.startswith("/"): + return env + try: + entry = pwd.getpwuid(os.getuid()) + except KeyError: + entry = None + if entry is not None and entry.pw_dir.startswith("/"): + return entry.pw_dir + raise PolicyError( + "cannot resolve ${HOME}: $HOME is unset or not absolute, and this uid " + "has no passwd entry with an absolute home directory" + ) + + +def _well_formed(name: str) -> bool: + if not name or not (name[0].isascii() and (name[0].isalpha() or name[0] == "_")): + return False + return all(c.isascii() and (c.isalnum() or c == "_") for c in name[1:]) + + +def _lookup(name: str, home: str, value: str) -> str: + if not _well_formed(name): + raise PolicyError( + f"{value!r}: malformed variable name ${{{name}}}; " + "names match [A-Za-z_][A-Za-z0-9_]*" + ) + if name == "HOME": + return home + suggestion = "" + for known in _VARS: + if known.lower() == name.lower(): + suggestion = f"; did you mean ${{{known}}}?" + break + supported = ", ".join(f"${{{v}}}" for v in _VARS) + raise PolicyError( + f"{value!r}: unknown variable ${{{name}}}{suggestion}; supported: {supported}" + ) + + +def _expand(value: str, home: str) -> str: + """Expand ``${HOME}`` in one profile path value. + + Every unrecognised form raises, so adding a variable later cannot + silently reinterpret a grant written today. + """ + if value.startswith("~"): + raise PolicyError( + f"{value!r}: tilde is not expanded in profiles; write ${{HOME}} instead" + ) + out: list[str] = [] + rest = value + while True: + pos = rest.find("$") + if pos < 0: + out.append(rest) + return "".join(out) + out.append(rest[:pos]) + after = rest[pos + 1:] + if after.startswith("{"): + tail = after[1:] + end = tail.find("}") + if end < 0: + raise PolicyError(f"{value!r}: unterminated ${{") + out.append(_lookup(tail[:end], home, value)) + rest = tail[end + 1:] + else: + name = "" + for c in after: + if c.isascii() and (c.isalnum() or c == "_"): + name += c + else: + break + if name: + raise PolicyError( + f"{value!r}: bare $ is not a variable; write ${{{name}}} " + "for a variable" + ) + raise PolicyError( + f"{value!r}: bare $ is not allowed; a literal $ cannot appear " + "in a profile path" + ) + + def profiles_dir() -> Path: """Return the profiles directory path.""" return _PROFILES_DIR @@ -181,6 +285,8 @@ def policy_from_dict(data: dict, source: str = "") -> Sandbox: ) kwargs: dict[str, Any] = {} + # One-element cache so a profile with no variables never resolves home. + home: list[str] = [] for section_name, section_data in data.items(): if not isinstance(section_data, dict): @@ -205,16 +311,32 @@ def policy_from_dict(data: dict, source: str = "") -> Sandbox: f"{source}: [{section_name}].{toml_key} expected " f"{expected_type.__name__}, got {type(value).__name__}" ) - value = _coerce(section_name, toml_key, sandbox_key, value, source) + value = _coerce(section_name, toml_key, sandbox_key, value, source, home) kwargs[sandbox_key] = value return Sandbox(**kwargs) def _coerce( - section: str, toml_key: str, sandbox_key: str, value: Any, source: str + section: str, toml_key: str, sandbox_key: str, value: Any, source: str, + home: list[str], ) -> Any: """Per-field value coercion (enums, mount-spec parsing, port lists).""" + + def expand(text: str) -> str: + if "$" not in text and not text.startswith("~"): + return text + if not home: + home.append(_resolve_home()) + try: + return _expand(text, home[0]) + except PolicyError as e: + raise PolicyError(f"{source}: [{section}].{toml_key}: {e}") from None + + if sandbox_key in _PATH_KEYS: + return expand(value) + if sandbox_key in _PATH_LIST_KEYS: + return [expand(v) for v in value] if sandbox_key in ("on_exit", "on_error"): try: return BranchAction(value) @@ -275,7 +397,9 @@ def _coerce( f"{source}: [{section}].{toml_key} entry {spec!r} " "requires both VIRTUAL and HOST to be non-empty" ) - mount[virt] = host + # Expand after the split so a resolved value containing a colon + # cannot be read as a spec separator. + mount[expand(virt)] = expand(host) return mount if sandbox_key == "net_allow_bind": # Coerce TOML integers to strings for port specs (existing behaviour). diff --git a/python/tests/test_profile.py b/python/tests/test_profile.py index 1994bb9b..c9a7ffe6 100644 --- a/python/tests/test_profile.py +++ b/python/tests/test_profile.py @@ -3,6 +3,7 @@ from __future__ import annotations +import os import re import textwrap @@ -356,3 +357,79 @@ def test_bool_override(self): def test_profiles_dir_is_a_path(): assert profiles_dir().is_absolute() or str(profiles_dir()).startswith("~") + + +class TestExpansion: + @staticmethod + def _fixture(): + from pathlib import Path + + # _profile binds `tomllib` to tomli on 3.10, so reuse its binding + # rather than importing tomllib directly. + from sandlock._profile import tomllib + + path = ( + Path(__file__).resolve().parents[2] + / "tests" + / "fixtures" + / "profile_expansion.toml" + ) + with open(path, "rb") as f: + return tomllib.load(f) + + def test_shared_fixture_cases(self): + from sandlock._profile import _expand + + data = self._fixture() + default_home = data["home"] + for case in data["case"]: + home = case.get("home", default_home) + if "expect" in case: + assert _expand(case["input"], home) == case["expect"], case["input"] + else: + with pytest.raises(PolicyError) as exc: + _expand(case["input"], home) + assert case["error"] in str(exc.value), case["input"] + + def test_path_fields_expand(self, monkeypatch): + monkeypatch.setenv("HOME", "/home/alice") + p = policy_from_dict( + { + "filesystem": { + "read": ["${HOME}/src"], + "mount": ["/work:${HOME}/host"], + }, + "program": {"cwd": "${HOME}/src"}, + } + ) + assert p.fs_readable == ["/home/alice/src"] + assert p.cwd == "/home/alice/src" + assert p.fs_mount == {"/work": "/home/alice/host"} + + def test_error_names_the_field(self, monkeypatch): + monkeypatch.setenv("HOME", "/home/alice") + with pytest.raises(PolicyError, match=r"\[filesystem\]\.read"): + policy_from_dict({"filesystem": {"read": ["${NOPE}"]}}) + + def test_no_variables_never_resolves_home(self, monkeypatch): + monkeypatch.delenv("HOME", raising=False) + p = policy_from_dict({"filesystem": {"read": ["/usr/lib"]}}) + assert p.fs_readable == ["/usr/lib"] + + def test_absolute_env_home_wins(self, monkeypatch): + from sandlock._profile import _resolve_home + + monkeypatch.setenv("HOME", "/env/home") + assert _resolve_home() == "/env/home" + + @pytest.mark.parametrize("bad", ["", "relative/home"]) + def test_non_absolute_env_home_falls_back_to_passwd(self, monkeypatch, bad): + import pwd + + from sandlock._profile import _resolve_home + + monkeypatch.setenv("HOME", bad) + expected = pwd.getpwuid(os.getuid()).pw_dir + if not expected.startswith("/"): + pytest.skip("this uid has no absolute passwd home") + assert _resolve_home() == expected From fa41c0915f24bf649b98dd967ef35682ae9c6557 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 14 Aug 2026 15:15:47 -0700 Subject: [PATCH 5/6] docs: document profile variable expansion The strictness is the part worth explaining: every unrecognised form is an error precisely so that adding variables later cannot change what a profile written today grants. Signed-off-by: Cong Wang --- README.md | 7 ++++++- docs/sandbox-reference.md | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8d329ffc..624ab045 100644 --- a/README.md +++ b/README.md @@ -472,7 +472,7 @@ clean_env = true env = { CC = "gcc", LANG = "C.UTF-8" } [filesystem] -read = ["/usr", "/lib", "/lib64", "/bin", "/etc"] +read = ["/usr", "/lib", "/lib64", "/bin", "/etc", "${HOME}/.cargo"] write = ["/tmp/work"] [limits] @@ -483,6 +483,11 @@ processes = 50 extra_deny = [] ``` +Path fields expand `${HOME}`, so the same profile works on machines with +different usernames. `${HOME}` is the only variable; anything else, including +a leading `~`, is a load error. See +[docs/sandbox-reference.md](docs/sandbox-reference.md) for the full grammar. + ```bash sandlock profile list sandlock profile show build diff --git a/docs/sandbox-reference.md b/docs/sandbox-reference.md index 5c05f860..759550c3 100644 --- a/docs/sandbox-reference.md +++ b/docs/sandbox-reference.md @@ -261,6 +261,44 @@ fields on `Sandbox`. | `no_huge_pages` | `no_huge_pages` | `bool` | `False` | Disable transparent huge pages via `prctl(PR_SET_THP_DISABLE)`. | | `no_supervisor` | `no_supervisor` | `bool` | `False` | Skip the seccomp user-notification supervisor. The sandbox runs with Landlock + a kernel-only deny filter, without IP allowlisting, resource limits, COW, chroot mediation, `/proc` virtualization, or custom handlers. Required when nesting inside another sandlock (the kernel only allows one `SECCOMP_FILTER_FLAG_NEW_LISTENER` per task). | +## Variable expansion + +Path-typed profile fields expand `${HOME}`, which makes a profile portable +between machines where the username differs. + +`${HOME}` resolves to `$HOME` when that is set and absolute, and otherwise to +the home directory in the passwd entry for the real uid. The environment wins +because the sandboxed program resolves its own `~` through `$HOME`, so a +passwd-derived grant could cover a directory the program never opens. + +Expansion applies to `[config].http_ca`, `http_key`, `http_inject_ca`, +`http_ca_out`, `fs_storage`, `workdir`; `[program].exec` and `cwd`; and +`[filesystem].read`, `write`, `deny`, `chroot`, and both halves of each +`mount` entry. It never applies to `[program].args` or `[program].env`, where +a `$` belongs to the sandboxed program, nor to network rules, syscall names, +or limits. + +The grammar is strict, so that adding variables in a later release cannot +change what an existing profile grants: + +| Form | Meaning | +| ---- | ------- | +| `${HOME}` | the home directory | +| `${OTHER}` | error, unknown variable | +| `${home}` | error, lookup is case-sensitive | +| `${}`, `${HO-ME}` | error, malformed name (`[A-Za-z_][A-Za-z0-9_]*`) | +| `${` with no `}` | error, unterminated | +| `$` anywhere else | error, `${HOME}` is the only meaning `$` can have | +| leading `~` | error, write `${HOME}` | + +A profile cannot express a path whose first character is a literal `~` +(write it as `./~name` instead), and cannot express a path containing a +literal `$` at all. `sandlock learn` and `sandlock inspect --toml` emit +such observed paths verbatim, so a profile generated from a workload that +touched a `$`-named file fails to load until the entry is removed. +`sandlock learn --merge` preserves a `${HOME}` you wrote by hand and will +not add its expanded duplicate. + ## `[filesystem]` Landlock filesystem rules plus chroot, mount mapping, and COW From cbc23db48703fbab77b906f395f7f62c9fa4d387 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 14 Aug 2026 17:45:08 -0700 Subject: [PATCH 6/6] profile: guard the two paths where ${HOME} is not a home ${HOME} is a portability macro for the person writing the profile, so it is the launcher's home and nothing more. Two inputs slip through that reading. $HOME can be /, which is absolute and so passed the old test, but is nobody's home: a bare write = ["${HOME}"] would have become a grant over the entire filesystem, the one grant learn already refuses to emit. It now joins the empty and relative cases as unusable, falling through to the passwd entry. Under chroot a path names the jail, not the host, so a host home builds a rule from a directory the jail does not have and Landlock skips it without a word. Refuse instead. Nothing is lost: jail layouts are fixed by the image rather than by the host username, so the portability problem ${HOME} exists to solve does not arise there. Signed-off-by: Cong Wang --- crates/sandlock-core/src/expand.rs | 35 ++++++++++++++++++++-- crates/sandlock-core/src/profile.rs | 45 +++++++++++++++++++++++++++-- docs/sandbox-reference.md | 17 ++++++++--- python/src/sandlock/_profile.py | 36 +++++++++++++++++++---- python/tests/test_profile.py | 35 ++++++++++++++++++++++ 5 files changed, 152 insertions(+), 16 deletions(-) diff --git a/crates/sandlock-core/src/expand.rs b/crates/sandlock-core/src/expand.rs index ec0c36bd..5e9f4b29 100644 --- a/crates/sandlock-core/src/expand.rs +++ b/crates/sandlock-core/src/expand.rs @@ -32,17 +32,26 @@ pub fn resolve_home() -> Result { /// mutating the process environment. fn resolve_home_from(env: Option<&str>, passwd: Option<&str>) -> Result { for candidate in [env, passwd].into_iter().flatten() { - if candidate.starts_with('/') { + if is_usable_home(candidate) { return Ok(candidate.to_string()); } } Err(invalid( - "cannot resolve ${HOME}: $HOME is unset or not absolute, and this uid \ - has no passwd entry with an absolute home directory" + "cannot resolve ${HOME}: $HOME is unset, not absolute, or is the \ + filesystem root, and this uid has no passwd entry with a usable home \ + directory" .to_string(), )) } +/// `/` is rejected alongside the relative and empty cases: it is nobody's home, +/// and expanding it would turn `write = ["${HOME}"]` into a grant over the +/// entire filesystem, which is the one thing a sandbox must never hand out by +/// accident. +fn is_usable_home(dir: &str) -> bool { + dir.starts_with('/') && !dir.trim_end_matches('/').is_empty() +} + /// Expand `${HOME}` in one profile path value. pub fn expand(value: &str, home: &str) -> Result { if value.starts_with('~') { @@ -183,6 +192,26 @@ mod tests { } } + #[test] + fn resolve_home_refuses_the_filesystem_root() { + // `/` is absolute but is nobody's home, and expanding it would turn + // `write = ["${HOME}"]` into a grant over the whole filesystem. + for env in [Some("/"), Some("//")] { + assert_eq!( + resolve_home_from(env, Some("/passwd/home")).unwrap(), + "/passwd/home", + "env {env:?}" + ); + assert!(resolve_home_from(env, Some("/")).is_err(), "env {env:?}"); + } + } + + #[test] + fn resolve_home_keeps_a_trailing_slash_home() { + // Only the all-slashes case is meaningless; `/root/` is a real home. + assert_eq!(resolve_home_from(Some("/root/"), None).unwrap(), "/root/"); + } + #[test] fn resolve_home_errors_when_neither_is_absolute() { let err = resolve_home_from(None, None).unwrap_err().to_string(); diff --git a/crates/sandlock-core/src/profile.rs b/crates/sandlock-core/src/profile.rs index a7d7577c..9e21b72c 100644 --- a/crates/sandlock-core/src/profile.rs +++ b/crates/sandlock-core/src/profile.rs @@ -200,12 +200,15 @@ impl ProfileInput { /// Lazily resolves `${HOME}` so a profile that uses no variables still loads /// on a host where home cannot be resolved. struct Expander { + /// Under chroot the grants are relative to the jail, so the only home + /// sandlock can see is in the wrong namespace. + chroot: bool, home: Option, } impl Expander { - fn new() -> Self { - Self { home: None } + fn new(chroot: bool) -> Self { + Self { chroot, home: None } } fn path(&mut self, field: &str, p: &std::path::Path) -> Result { @@ -216,6 +219,17 @@ impl Expander { if !s.contains('$') && !s.starts_with('~') { return Ok(s.to_string()); } + if self.chroot { + // Expanding anyway builds the rule from a host path that does not + // exist in the jail, and Landlock then skips it in silence. + return Err(SandlockError::Sandbox(crate::error::SandboxError::Invalid( + format!( + "{field}: {s:?}: ${{HOME}} cannot be expanded under \ + [filesystem].chroot, where paths name the jail rather than \ + the host. Write the path as it exists inside the jail" + ), + ))); + } if self.home.is_none() { self.home = Some(crate::expand::resolve_home()?); } @@ -236,7 +250,7 @@ impl Expander { /// that lack `FromStr` impls on their target types. pub fn parse_input(input: ProfileInput) -> Result<(Sandbox, ProgramSpec), SandlockError> { let mut b = Sandbox::builder(); - let mut ex = Expander::new(); + let mut ex = Expander::new(input.filesystem.chroot.is_some()); // [config] if let Some(p) = input.config.http_ca { b = b.http_ca(ex.path("[config].http_ca", &p)?); } @@ -662,6 +676,31 @@ pub fn list_profiles() -> Result, SandlockError> { mod tests { use super::*; + #[test] + fn parse_profile_refuses_home_under_chroot() { + // Grants are relative to the jail, so a host home is the wrong + // namespace: Landlock would drop the rule without a word. + let toml = r#" + [filesystem] + chroot = "/jail" + read = ["${HOME}/src"] + "#; + let err = parse_profile(toml).unwrap_err().to_string(); + assert!(err.contains("[filesystem].read"), "error was {err:?}"); + assert!(err.contains("chroot"), "error was {err:?}"); + } + + #[test] + fn parse_profile_allows_chroot_without_variables() { + let toml = r#" + [filesystem] + chroot = "/jail" + read = ["/usr/lib"] + "#; + let (policy, _spec) = parse_profile(toml).unwrap(); + assert_eq!(policy.chroot, Some(PathBuf::from("/jail"))); + } + #[test] fn parse_profile_expands_home_in_path_fields() { let home = crate::expand::resolve_home().unwrap(); diff --git a/docs/sandbox-reference.md b/docs/sandbox-reference.md index 759550c3..5f2e2058 100644 --- a/docs/sandbox-reference.md +++ b/docs/sandbox-reference.md @@ -266,10 +266,19 @@ fields on `Sandbox`. Path-typed profile fields expand `${HOME}`, which makes a profile portable between machines where the username differs. -`${HOME}` resolves to `$HOME` when that is set and absolute, and otherwise to -the home directory in the passwd entry for the real uid. The environment wins -because the sandboxed program resolves its own `~` through `$HOME`, so a -passwd-derived grant could cover a directory the program never opens. +`${HOME}` is the home directory of whoever runs sandlock: `$HOME` when that is +usable, and otherwise the home directory in the passwd entry for the real uid. +The environment wins because the sandboxed program resolves its own `~` through +`$HOME`, so a passwd-derived grant could cover a directory the program never +opens. A usable home is an absolute path other than the filesystem root: `/` is +nobody's home, and `write = ["${HOME}"]` would otherwise become a grant over +everything. If neither source is usable, loading the profile fails. + +Under `[filesystem].chroot`, `${HOME}` in a path field is an error. Paths there +name the jail rather than the host, so expanding a host home would build the +rule from a directory that does not exist inside it, and Landlock would skip +the rule in silence. Jail layouts do not vary by username, so write the path +as it exists inside the jail. Expansion applies to `[config].http_ca`, `http_key`, `http_inject_ca`, `http_ca_out`, `fs_storage`, `workdir`; `[program].exec` and `cwd`; and diff --git a/python/src/sandlock/_profile.py b/python/src/sandlock/_profile.py index d78c1311..c9cde890 100644 --- a/python/src/sandlock/_profile.py +++ b/python/src/sandlock/_profile.py @@ -136,20 +136,31 @@ def _resolve_home() -> str: its own ``~`` through ``$HOME``. """ env = os.environ.get("HOME") - if env and env.startswith("/"): + if env and _usable_home(env): return env try: entry = pwd.getpwuid(os.getuid()) except KeyError: entry = None - if entry is not None and entry.pw_dir.startswith("/"): + if entry is not None and _usable_home(entry.pw_dir): return entry.pw_dir raise PolicyError( - "cannot resolve ${HOME}: $HOME is unset or not absolute, and this uid " - "has no passwd entry with an absolute home directory" + "cannot resolve ${HOME}: $HOME is unset, not absolute, or is the " + "filesystem root, and this uid has no passwd entry with a usable " + "home directory" ) +def _usable_home(directory: str) -> bool: + """Reject ``/`` alongside the relative and empty cases. + + It is nobody's home, and expanding it would turn ``write = ["${HOME}"]`` + into a grant over the entire filesystem, which is the one thing a sandbox + must never hand out by accident. + """ + return directory.startswith("/") and directory.rstrip("/") != "" + + def _well_formed(name: str) -> bool: if not name or not (name[0].isascii() and (name[0].isalpha() or name[0] == "_")): return False @@ -287,6 +298,8 @@ def policy_from_dict(data: dict, source: str = "") -> Sandbox: kwargs: dict[str, Any] = {} # One-element cache so a profile with no variables never resolves home. home: list[str] = [] + filesystem = data.get("filesystem") + chroot = isinstance(filesystem, dict) and "chroot" in filesystem for section_name, section_data in data.items(): if not isinstance(section_data, dict): @@ -311,7 +324,9 @@ def policy_from_dict(data: dict, source: str = "") -> Sandbox: f"{source}: [{section_name}].{toml_key} expected " f"{expected_type.__name__}, got {type(value).__name__}" ) - value = _coerce(section_name, toml_key, sandbox_key, value, source, home) + value = _coerce( + section_name, toml_key, sandbox_key, value, source, home, chroot + ) kwargs[sandbox_key] = value return Sandbox(**kwargs) @@ -319,13 +334,22 @@ def policy_from_dict(data: dict, source: str = "") -> Sandbox: def _coerce( section: str, toml_key: str, sandbox_key: str, value: Any, source: str, - home: list[str], + home: list[str], chroot: bool, ) -> Any: """Per-field value coercion (enums, mount-spec parsing, port lists).""" def expand(text: str) -> str: if "$" not in text and not text.startswith("~"): return text + if chroot: + # Expanding anyway builds the rule from a host path that does not + # exist in the jail, and Landlock then skips it in silence. + raise PolicyError( + f"{source}: [{section}].{toml_key}: {text!r}: ${{HOME}} cannot " + "be expanded under [filesystem].chroot, where paths name the " + "jail rather than the host. Write the path as it exists inside " + "the jail" + ) if not home: home.append(_resolve_home()) try: diff --git a/python/tests/test_profile.py b/python/tests/test_profile.py index c9a7ffe6..23bc5eb1 100644 --- a/python/tests/test_profile.py +++ b/python/tests/test_profile.py @@ -422,6 +422,41 @@ def test_absolute_env_home_wins(self, monkeypatch): monkeypatch.setenv("HOME", "/env/home") assert _resolve_home() == "/env/home" + @pytest.mark.parametrize("bad", ["/", "//"]) + def test_root_env_home_falls_back_to_passwd(self, monkeypatch, bad): + # `/` is absolute but is nobody's home, and expanding it would turn + # write = ["${HOME}"] into a grant over the whole filesystem. + import pwd + + from sandlock._profile import _resolve_home + + monkeypatch.setenv("HOME", bad) + expected = pwd.getpwuid(os.getuid()).pw_dir + if not expected.startswith("/") or expected.rstrip("/") == "": + pytest.skip("this uid has no usable passwd home") + assert _resolve_home() == expected + + def test_trailing_slash_home_is_kept(self, monkeypatch): + # Only the all-slashes case is meaningless; `/root/` is a real home. + from sandlock._profile import _resolve_home + + monkeypatch.setenv("HOME", "/root/") + assert _resolve_home() == "/root/" + + def test_home_under_chroot_is_an_error(self, monkeypatch): + monkeypatch.setenv("HOME", "/env/home") + with pytest.raises(PolicyError, match="chroot"): + policy_from_dict( + {"filesystem": {"chroot": "/jail", "read": ["${HOME}/src"]}} + ) + + def test_chroot_without_variables_still_loads(self, monkeypatch): + monkeypatch.setenv("HOME", "/env/home") + p = policy_from_dict( + {"filesystem": {"chroot": "/jail", "read": ["/usr/lib"]}} + ) + assert p.chroot == "/jail" + @pytest.mark.parametrize("bad", ["", "relative/home"]) def test_non_absolute_env_home_falls_back_to_passwd(self, monkeypatch, bad): import pwd