From e9c856d32f299861da89b6b3c670e9ddca42c921 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Thu, 4 Jun 2026 10:24:24 -0700 Subject: [PATCH 1/3] Honor process.cwd and process.env in the LXC backend --- src/backends/lxc/common/src/lxc_bindings.rs | 155 +++++++++++++++++++- src/backends/lxc/common/src/lxc_runner.rs | 8 +- tests/configs/lxc_env_cwd_test.json | 18 +++ tests/scripts/run_lxc_all_tests.sh | 1 + tests/scripts/run_lxc_env_cwd_test.sh | 33 +++++ 5 files changed, 211 insertions(+), 4 deletions(-) create mode 100644 tests/configs/lxc_env_cwd_test.json create mode 100644 tests/scripts/run_lxc_env_cwd_test.sh diff --git a/src/backends/lxc/common/src/lxc_bindings.rs b/src/backends/lxc/common/src/lxc_bindings.rs index e72cc89e..d4e33fd2 100644 --- a/src/backends/lxc/common/src/lxc_bindings.rs +++ b/src/backends/lxc/common/src/lxc_bindings.rs @@ -68,6 +68,48 @@ pub fn resolve_default_lxcpath() -> String { resolve_lxcpath_with_env(|k| std::env::var(k).ok(), current_euid) } +/// Build the post-binary argv for `lxc-attach` (the args that follow the +/// `-n NAME -P lxcpath` flags already appended by `lxc_command`). +/// +/// Extracted so the env / cwd / command layering is unit-testable without +/// actually spawning `lxc-attach`. See [`LxcContainer::attach_run`] for +/// the full contract. +/// +/// Gated to Linux + test builds because `attach_run` is a Windows stub +/// that never calls this helper, and the workspace clippy lane on +/// `windows-latest` would otherwise flag it as dead code. +#[cfg(any(target_os = "linux", test))] +fn build_attach_args(env: &[String], working_directory: &str, command: &str) -> Vec { + let mut args: Vec = Vec::with_capacity(env.len() + 6); + + // Each `KEY=VAL` becomes a separate `--set-var=KEY=VAL`. Skip entries + // without `=` rather than letting `lxc-attach` reject the whole call — + // matches the permissive `split_once('=')` semantics other backends use. + for kv in env { + if kv.contains('=') { + args.push(format!("--set-var={}", kv)); + } + } + + args.push("--".to_string()); + args.push("/bin/sh".to_string()); + args.push("-c".to_string()); + + if working_directory.is_empty() { + args.push(command.to_string()); + } else { + // Positional-arg trick: `$1` and `$2` carry the cwd and command + // verbatim through sh, so neither needs shell-escaping. The leading + // `_` is a conventional placeholder for `$0` (sh's script name slot). + args.push("cd -- \"$1\" && exec /bin/sh -c \"$2\"".to_string()); + args.push("_".to_string()); + args.push(working_directory.to_string()); + args.push(command.to_string()); + } + + args +} + /// Safe wrapper around an LXC container. pub struct LxcContainer { name: String, @@ -214,6 +256,17 @@ impl LxcContainer { /// contract (output streamed live to host stdio, stdin forwarded after /// first byte arrives from inner shell, etc.). /// + /// `working_directory` is honored by wrapping the user command in a + /// `cd -- "$1" && exec /bin/sh -c "$2"` shell prelude with cwd and + /// command passed as positional args so neither needs additional + /// shell escaping. Empty string preserves the container default cwd. + /// + /// `env` is honored by translating each `KEY=VAL` entry into a + /// repeated `--set-var=KEY=VAL` argument to `lxc-attach`. Entries + /// without `=` are skipped (matches the permissive contract Seatbelt + /// and WSLC apply to malformed env entries). The container's default + /// environment is preserved; user entries override on key collision. + /// /// We pass `unblock_signals = [SIGHUP, SIGTERM, SIGINT]` because /// [`crate::signal_cleanup::install`] blocks them in this process so /// its watchdog thread can `sigwait` on them; that mask is inherited @@ -230,7 +283,8 @@ impl LxcContainer { pub fn attach_run( &self, command: &str, - _working_directory: &str, + working_directory: &str, + env: &[String], timeout: Option, ) -> Result<(i32, String, String), String> { use mxc_pty::{run_with_pty, PtyOptions, PtyOutcome, Signal}; @@ -238,7 +292,7 @@ impl LxcContainer { const UNBLOCK: &[Signal] = &[Signal::SIGHUP, Signal::SIGTERM, Signal::SIGINT]; let mut cmd = self.lxc_command("lxc-attach"); - cmd.args(["--", "/bin/sh", "-c", command]); + cmd.args(build_attach_args(env, working_directory, command)); let options = PtyOptions { unblock_signals: UNBLOCK, @@ -264,6 +318,7 @@ impl LxcContainer { &self, _command: &str, _working_directory: &str, + _env: &[String], _timeout: Option, ) -> Result<(i32, String, String), String> { Err("LxcContainer::attach_run is only supported on Linux".to_string()) @@ -493,4 +548,100 @@ mod tests { err ); } + + // ---- build_attach_args ---------------------------------------------- + + #[test] + fn build_attach_args_no_env_no_cwd_is_unchanged_legacy_shape() { + // Empty env + empty cwd must reproduce the original argv shape: + // `-- /bin/sh -c ` so we don't perturb existing call sites + // when neither cwd nor env is set. + let args = build_attach_args(&[], "", "echo hi"); + assert_eq!(args, vec!["--", "/bin/sh", "-c", "echo hi"]); + } + + #[test] + fn build_attach_args_env_is_translated_to_set_var_flags() { + let env = vec![ + "FOO=bar".to_string(), + "EMPTY=".to_string(), + "HAS_EQ_IN_VAL=a=b=c".to_string(), + ]; + let args = build_attach_args(&env, "", "cmd"); + assert_eq!( + args, + vec![ + "--set-var=FOO=bar", + "--set-var=EMPTY=", + "--set-var=HAS_EQ_IN_VAL=a=b=c", + "--", + "/bin/sh", + "-c", + "cmd", + ] + ); + } + + #[test] + fn build_attach_args_env_entries_without_equals_are_skipped() { + // Matches Seatbelt/WSLC semantics (split_once('=') drops malformed + // entries) so a bad entry can't break the whole attach call. + let env = vec!["BADENTRY".to_string(), "OK=val".to_string()]; + let args = build_attach_args(&env, "", "cmd"); + assert_eq!(args, vec!["--set-var=OK=val", "--", "/bin/sh", "-c", "cmd"]); + } + + #[test] + fn build_attach_args_cwd_wraps_command_with_cd_prelude() { + let args = build_attach_args(&[], "/opt/work", "echo hi"); + assert_eq!( + args, + vec![ + "--", + "/bin/sh", + "-c", + "cd -- \"$1\" && exec /bin/sh -c \"$2\"", + "_", + "/opt/work", + "echo hi", + ] + ); + } + + #[test] + fn build_attach_args_cwd_with_special_chars_does_not_require_escaping() { + // The whole point of the positional-arg trick is that nasty cwd + // values (spaces, single/double quotes, dollar signs, backticks) + // pass through sh as `$1` verbatim — no escaping needed here. + let cwd = "/tmp/has spaces & 'quotes' $vars `cmd`"; + let cmd = "printf '%s' \"$PWD\""; + let args = build_attach_args(&[], cwd, cmd); + + // cwd and command must appear verbatim as the last two argv entries. + assert_eq!(args[args.len() - 2], cwd); + assert_eq!(args[args.len() - 1], cmd); + // And the wrapper script must reference them positionally. + assert!(args + .iter() + .any(|a| a == "cd -- \"$1\" && exec /bin/sh -c \"$2\"")); + } + + #[test] + fn build_attach_args_combines_env_and_cwd() { + let env = vec!["FOO=bar".to_string()]; + let args = build_attach_args(&env, "/work", "cmd"); + assert_eq!( + args, + vec![ + "--set-var=FOO=bar", + "--", + "/bin/sh", + "-c", + "cd -- \"$1\" && exec /bin/sh -c \"$2\"", + "_", + "/work", + "cmd", + ] + ); + } } diff --git a/src/backends/lxc/common/src/lxc_runner.rs b/src/backends/lxc/common/src/lxc_runner.rs index 4ee844d2..6204cd9a 100644 --- a/src/backends/lxc/common/src/lxc_runner.rs +++ b/src/backends/lxc/common/src/lxc_runner.rs @@ -214,8 +214,12 @@ impl LxcScriptRunner { Some(Duration::from_millis(u64::from(request.script_timeout))) }; let _ = writeln!(logger, "Executing script inside container..."); - let result = - container.attach_run(&request.script_code, &request.working_directory, timeout); + let result = container.attach_run( + &request.script_code, + &request.working_directory, + &request.env, + timeout, + ); let response = match result { Ok((exit_code, stdout, stderr)) => ScriptResponse { diff --git a/tests/configs/lxc_env_cwd_test.json b/tests/configs/lxc_env_cwd_test.json new file mode 100644 index 00000000..faa15a16 --- /dev/null +++ b/tests/configs/lxc_env_cwd_test.json @@ -0,0 +1,18 @@ +{ + "version": "0.4.0-alpha", + "containerId": "CLI-LXC-Env-Cwd-Test", + "containment": "lxc", + "platform": "linux", + "process": { + "commandLine": "set -e; ACTUAL_PWD=$(pwd); [ \"$ACTUAL_PWD\" = \"/etc\" ] || { echo \"FAIL cwd: got '$ACTUAL_PWD', want '/etc'\" >&2; exit 11; }; [ \"${MXC_TEST_FOO:-}\" = \"bar baz\" ] || { echo \"FAIL env MXC_TEST_FOO: got '${MXC_TEST_FOO:-}', want 'bar baz'\" >&2; exit 12; }; [ \"${MXC_TEST_EQ:-}\" = \"a=b=c\" ] || { echo \"FAIL env MXC_TEST_EQ: got '${MXC_TEST_EQ:-}', want 'a=b=c'\" >&2; exit 13; }; echo \"OK cwd=$ACTUAL_PWD MXC_TEST_FOO=$MXC_TEST_FOO MXC_TEST_EQ=$MXC_TEST_EQ\"", + "cwd": "/etc", + "env": ["MXC_TEST_FOO=bar baz", "MXC_TEST_EQ=a=b=c"] + }, + "lifecycle": { + "destroyOnExit": true + }, + "lxc": { + "distribution": "alpine", + "release": "3.23" + } +} diff --git a/tests/scripts/run_lxc_all_tests.sh b/tests/scripts/run_lxc_all_tests.sh index 7c218116..0e2f5d7c 100644 --- a/tests/scripts/run_lxc_all_tests.sh +++ b/tests/scripts/run_lxc_all_tests.sh @@ -44,6 +44,7 @@ run_test "Basic LXC" "$SCRIPT_DIR/run_lxc_basic_test.sh" run_test "LXC Filesystem" "$SCRIPT_DIR/run_lxc_filesystem_test.sh" run_test "LXC Network" "$SCRIPT_DIR/run_lxc_network_test.sh" run_test "LXC Timeout" "$SCRIPT_DIR/run_lxc_timeout_test.sh" +run_test "LXC Env+Cwd" "$SCRIPT_DIR/run_lxc_env_cwd_test.sh" echo "================================" echo "Results: $PASSED passed, $FAILED failed" diff --git a/tests/scripts/run_lxc_env_cwd_test.sh b/tests/scripts/run_lxc_env_cwd_test.sh new file mode 100644 index 00000000..e199be3d --- /dev/null +++ b/tests/scripts/run_lxc_env_cwd_test.sh @@ -0,0 +1,33 @@ +#!/bin/bash +# LXC env + cwd plumbing test. +# +# Asserts that `process.cwd` and `process.env` from the config actually +# reach the inner shell inside the container. The config's `commandLine` +# self-validates and exits with distinct non-zero codes if either field +# was silently dropped: +# 11 = cwd not honored (still at container default) +# 12 = MXC_TEST_FOO env not honored (or value with spaces lost) +# 13 = MXC_TEST_EQ env not honored (or embedded `=` lost) +# +# `lxc-exec` propagates the inner exit code (see `core/lxc/src/main.rs` +# line 294), so `set -e` here catches any regression. Guards against the +# silent-drop bug pattern fixed in `fix/lxc-cwd-env` — without that fix +# this test exits 11 (cwd) on the first assertion. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_DIR="$(dirname "$(dirname "$SCRIPT_DIR")")" +LXC_EXEC="$REPO_DIR/src/target/release/lxc-exec" + +if [ ! -f "$LXC_EXEC" ]; then + LXC_EXEC="$REPO_DIR/src/target/debug/lxc-exec" +fi + +if [ ! -f "$LXC_EXEC" ]; then + echo "Error: lxc-exec not found. Run build.sh first." + exit 1 +fi + +echo "Running LXC env+cwd test..." +"$LXC_EXEC" "$REPO_DIR/tests/configs/lxc_env_cwd_test.json" +echo "LXC env+cwd test complete." From 0196760c0bb6f58cfbb1b54f226dac96bc60e4e7 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Fri, 5 Jun 2026 11:19:04 -0700 Subject: [PATCH 2/3] Replace, don't merge, when caller supplies env --- src/backends/lxc/common/src/lxc_bindings.rs | 157 ++++++++++++++++++-- 1 file changed, 144 insertions(+), 13 deletions(-) diff --git a/src/backends/lxc/common/src/lxc_bindings.rs b/src/backends/lxc/common/src/lxc_bindings.rs index d4e33fd2..558ba813 100644 --- a/src/backends/lxc/common/src/lxc_bindings.rs +++ b/src/backends/lxc/common/src/lxc_bindings.rs @@ -80,15 +80,45 @@ pub fn resolve_default_lxcpath() -> String { /// `windows-latest` would otherwise flag it as dead code. #[cfg(any(target_os = "linux", test))] fn build_attach_args(env: &[String], working_directory: &str, command: &str) -> Vec { - let mut args: Vec = Vec::with_capacity(env.len() + 6); - - // Each `KEY=VAL` becomes a separate `--set-var=KEY=VAL`. Skip entries - // without `=` rather than letting `lxc-attach` reject the whole call — - // matches the permissive `split_once('=')` semantics other backends use. - for kv in env { - if kv.contains('=') { - args.push(format!("--set-var={}", kv)); - } + // Upper bound: 1 `--clear-env` + env.len() set-vars + up to 7 fixed + // elements in the cwd branch (`--`, `/bin/sh`, `-c`, prelude, `_`, cwd, + // command). Loose by ~3 in the no-cwd branch — Vec reallocs anyway if + // wrong, so this is purely a "save the realloc" hint. + let mut args: Vec = Vec::with_capacity(env.len() + 8); + + // Replace, don't merge: when the caller supplies env, strip lxc-exec's + // own environment via `--clear-env` so host-operator vars (PATH, tokens, + // ambient credentials, anything else the launching shell exported) do + // NOT leak into the sandbox. This matches Seatbelt's `env_clear()` model + // (`seatbelt_runner.rs` ~line 149) and WSLC's distro-launch semantics + // (`wsl_container_runner.rs` ~line 929), and matches `lxc-attach(1)`'s + // own recommendation for non-interactive sandbox-spawn callers (the + // manpage flags `--keep-env`, its current default, as "likely to change + // in the future" because it leaks undesirable information). + // + // Residual: lxc-attach still injects a small baseline (`container`, + // `HOME`, `TERM`, default `PATH`, `USER`) and applies the container's + // `lxc.environment` config; both layers sit below the user vars and are + // outside this helper's control. Strict bit-identical parity with + // Seatbelt would require bypassing lxc-attach entirely (out of scope). + // + // Empty env (or all-malformed env after the `split_once('=')` skip) + // preserves the legacy keep-env shape so existing call sites without + // usable env are undisturbed — clearing without replacing would strand + // the user with only lxc-attach's baseline, which is rarely intended. + // + // Each well-formed `KEY=VAL` becomes a separate `--set-var=KEY=VAL`. + // Entries without `=` are silently skipped (matches the permissive + // `split_once('=')` semantics Seatbelt and WSLC apply to malformed + // entries) so one bad entry can't break the whole attach call. + let set_vars: Vec = env + .iter() + .filter(|kv| kv.contains('=')) + .map(|kv| format!("--set-var={}", kv)) + .collect(); + if !set_vars.is_empty() { + args.push("--clear-env".to_string()); + args.extend(set_vars); } args.push("--".to_string()); @@ -101,6 +131,25 @@ fn build_attach_args(env: &[String], working_directory: &str, command: &str) -> // Positional-arg trick: `$1` and `$2` carry the cwd and command // verbatim through sh, so neither needs shell-escaping. The leading // `_` is a conventional placeholder for `$0` (sh's script name slot). + // + // `cd --` guards against a cwd that starts with `-` (which would + // otherwise be misparsed as a cd flag). The `--` is POSIX and is + // honored by busybox ash 1.36+ (Alpine 3.23 is the integration-test + // target — verified end-to-end via `tests/scripts/run_lxc_env_cwd_test.sh`). + // + // `exec` is load-bearing: it replaces the wrapper shell with the + // user's command in-place so signal/timeout delivery (see + // `unblock_signals` in `attach_run` and the pty bridge's kill path) + // hits the user process directly rather than the wrapper sh. + // Removing `exec` would silently regress the timeout/cancel contract. + // + // Failure mode: a nonexistent or non-permitted cwd makes `cd` fail, + // `&&` short-circuits, the user command never runs, and the wrapper + // sh exits with cd's status (1 for "not found", etc.). The caller + // sees a generic non-zero exit with no structured signal that the + // cwd was the cause — same observable behavior as a bad + // `Command::current_dir` on the other backends. Callers needing + // strong cwd validation should pre-check the path. args.push("cd -- \"$1\" && exec /bin/sh -c \"$2\"".to_string()); args.push("_".to_string()); args.push(working_directory.to_string()); @@ -260,12 +309,28 @@ impl LxcContainer { /// `cd -- "$1" && exec /bin/sh -c "$2"` shell prelude with cwd and /// command passed as positional args so neither needs additional /// shell escaping. Empty string preserves the container default cwd. + /// A nonexistent or non-permitted cwd surfaces as a generic non-zero + /// exit (typically 1, from `cd`'s own status) with no structured + /// signal that the cwd was the cause — same observable behavior as + /// a bad `Command::current_dir` on the other backends. Callers + /// needing strong cwd validation should pre-check the path. /// /// `env` is honored by translating each `KEY=VAL` entry into a /// repeated `--set-var=KEY=VAL` argument to `lxc-attach`. Entries - /// without `=` are skipped (matches the permissive contract Seatbelt - /// and WSLC apply to malformed env entries). The container's default - /// environment is preserved; user entries override on key collision. + /// without `=` are silently skipped (matches the permissive + /// `split_once('=')` semantics Seatbelt and WSLC apply). + /// + /// When `env` is non-empty, `--clear-env` is also passed so + /// `lxc-exec`'s own caller environment does **not** leak into the + /// sandbox — this is the replace-on-non-empty-env contract Seatbelt + /// and WSLC apply, and the posture `lxc-attach(1)` itself recommends + /// for sandbox-spawn callers. `lxc-attach` still injects a small + /// baseline (`container`, `HOME`, `TERM`, default `PATH`, `USER`) and + /// applies the container's `lxc.environment` config; those layers sit + /// below the user vars and are outside this function's control. + /// + /// When `env` is empty, the legacy keep-env behavior is preserved so + /// existing call sites without explicit env are undisturbed. /// /// We pass `unblock_signals = [SIGHUP, SIGTERM, SIGINT]` because /// [`crate::signal_cleanup::install`] blocks them in this process so @@ -571,6 +636,7 @@ mod tests { assert_eq!( args, vec![ + "--clear-env", "--set-var=FOO=bar", "--set-var=EMPTY=", "--set-var=HAS_EQ_IN_VAL=a=b=c", @@ -588,7 +654,17 @@ mod tests { // entries) so a bad entry can't break the whole attach call. let env = vec!["BADENTRY".to_string(), "OK=val".to_string()]; let args = build_attach_args(&env, "", "cmd"); - assert_eq!(args, vec!["--set-var=OK=val", "--", "/bin/sh", "-c", "cmd"]); + assert_eq!( + args, + vec![ + "--clear-env", + "--set-var=OK=val", + "--", + "/bin/sh", + "-c", + "cmd", + ] + ); } #[test] @@ -633,6 +709,7 @@ mod tests { assert_eq!( args, vec![ + "--clear-env", "--set-var=FOO=bar", "--", "/bin/sh", @@ -644,4 +721,58 @@ mod tests { ] ); } + + #[test] + fn build_attach_args_emits_clear_env_when_env_non_empty() { + // Containment guarantee: when the caller supplies env, lxc-exec's + // own environment must NOT leak into the sandbox. `--clear-env` + // also has to land BEFORE the `--set-var` entries so lxc-attach + // clears first, then applies user vars on top. + let env = vec!["FOO=bar".to_string()]; + let args = build_attach_args(&env, "", "cmd"); + let clear_idx = args + .iter() + .position(|a| a == "--clear-env") + .expect("--clear-env should be present when env is non-empty"); + let set_idx = args + .iter() + .position(|a| a == "--set-var=FOO=bar") + .expect("--set-var entry should be present"); + assert!( + clear_idx < set_idx, + "--clear-env must precede --set-var entries, got {:?}", + args + ); + } + + #[test] + fn build_attach_args_omits_clear_env_when_env_empty() { + // Backward-compat guarantee: empty env preserves the legacy + // keep-env shape so existing call sites with no explicit env are + // undisturbed. + let args = build_attach_args(&[], "", "echo hi"); + assert!( + !args.iter().any(|a| a == "--clear-env"), + "--clear-env must not appear when env is empty, got {:?}", + args + ); + } + + #[test] + fn build_attach_args_omits_clear_env_when_env_is_only_malformed() { + // Edge case: caller passed env but every entry was malformed + // (no `=`). After the permissive skip there are no user vars to + // apply, so `--clear-env` would strip the inherited env without + // replacing it with anything — exactly the surprise we want to + // avoid. Treat all-malformed as "no env supplied" and preserve + // the keep-env shape. + let env = vec!["BADENTRY".to_string(), "ALSO_BAD".to_string()]; + let args = build_attach_args(&env, "", "cmd"); + assert!( + !args.iter().any(|a| a == "--clear-env"), + "--clear-env must not appear when no entry survives the malformed-skip, got {:?}", + args + ); + assert_eq!(args, vec!["--", "/bin/sh", "-c", "cmd"]); + } } From a1f99b9afcbe5f58d8f1dfd77988983de94453b4 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Fri, 5 Jun 2026 11:31:18 -0700 Subject: [PATCH 3/3] Make comments more concise --- src/backends/lxc/common/src/lxc_bindings.rs | 62 ++++----------------- 1 file changed, 11 insertions(+), 51 deletions(-) diff --git a/src/backends/lxc/common/src/lxc_bindings.rs b/src/backends/lxc/common/src/lxc_bindings.rs index 558ba813..4d78d080 100644 --- a/src/backends/lxc/common/src/lxc_bindings.rs +++ b/src/backends/lxc/common/src/lxc_bindings.rs @@ -80,37 +80,14 @@ pub fn resolve_default_lxcpath() -> String { /// `windows-latest` would otherwise flag it as dead code. #[cfg(any(target_os = "linux", test))] fn build_attach_args(env: &[String], working_directory: &str, command: &str) -> Vec { - // Upper bound: 1 `--clear-env` + env.len() set-vars + up to 7 fixed - // elements in the cwd branch (`--`, `/bin/sh`, `-c`, prelude, `_`, cwd, - // command). Loose by ~3 in the no-cwd branch — Vec reallocs anyway if - // wrong, so this is purely a "save the realloc" hint. + // Loose upper bound: 1 --clear-env + env.len() set-vars + up to 7 fixed + // elements in the cwd branch. Realloc-avoidance hint only. let mut args: Vec = Vec::with_capacity(env.len() + 8); - // Replace, don't merge: when the caller supplies env, strip lxc-exec's - // own environment via `--clear-env` so host-operator vars (PATH, tokens, - // ambient credentials, anything else the launching shell exported) do - // NOT leak into the sandbox. This matches Seatbelt's `env_clear()` model - // (`seatbelt_runner.rs` ~line 149) and WSLC's distro-launch semantics - // (`wsl_container_runner.rs` ~line 929), and matches `lxc-attach(1)`'s - // own recommendation for non-interactive sandbox-spawn callers (the - // manpage flags `--keep-env`, its current default, as "likely to change - // in the future" because it leaks undesirable information). - // - // Residual: lxc-attach still injects a small baseline (`container`, - // `HOME`, `TERM`, default `PATH`, `USER`) and applies the container's - // `lxc.environment` config; both layers sit below the user vars and are - // outside this helper's control. Strict bit-identical parity with - // Seatbelt would require bypassing lxc-attach entirely (out of scope). - // - // Empty env (or all-malformed env after the `split_once('=')` skip) - // preserves the legacy keep-env shape so existing call sites without - // usable env are undisturbed — clearing without replacing would strand - // the user with only lxc-attach's baseline, which is rarely intended. - // - // Each well-formed `KEY=VAL` becomes a separate `--set-var=KEY=VAL`. - // Entries without `=` are silently skipped (matches the permissive - // `split_once('=')` semantics Seatbelt and WSLC apply to malformed - // entries) so one bad entry can't break the whole attach call. + // Replace-on-non-empty-env: when at least one well-formed entry survives + // the malformed-skip, emit --clear-env so lxc-exec's caller env doesn't + // leak into the sandbox. All-malformed and empty env both preserve + // keep-env semantics — see `attach_run` doc for the full contract. let set_vars: Vec = env .iter() .filter(|kv| kv.contains('=')) @@ -128,28 +105,11 @@ fn build_attach_args(env: &[String], working_directory: &str, command: &str) -> if working_directory.is_empty() { args.push(command.to_string()); } else { - // Positional-arg trick: `$1` and `$2` carry the cwd and command - // verbatim through sh, so neither needs shell-escaping. The leading - // `_` is a conventional placeholder for `$0` (sh's script name slot). - // - // `cd --` guards against a cwd that starts with `-` (which would - // otherwise be misparsed as a cd flag). The `--` is POSIX and is - // honored by busybox ash 1.36+ (Alpine 3.23 is the integration-test - // target — verified end-to-end via `tests/scripts/run_lxc_env_cwd_test.sh`). - // - // `exec` is load-bearing: it replaces the wrapper shell with the - // user's command in-place so signal/timeout delivery (see - // `unblock_signals` in `attach_run` and the pty bridge's kill path) - // hits the user process directly rather than the wrapper sh. - // Removing `exec` would silently regress the timeout/cancel contract. - // - // Failure mode: a nonexistent or non-permitted cwd makes `cd` fail, - // `&&` short-circuits, the user command never runs, and the wrapper - // sh exits with cd's status (1 for "not found", etc.). The caller - // sees a generic non-zero exit with no structured signal that the - // cwd was the cause — same observable behavior as a bad - // `Command::current_dir` on the other backends. Callers needing - // strong cwd validation should pre-check the path. + // Positional-arg trick: cwd and command travel through sh as $1/$2 + // verbatim, so neither needs shell-escaping; `_` fills sh's $0 slot. + // `cd --` guards a leading-dash cwd; `exec` is required so signals + // and timeout delivery hit the user process instead of the wrapper + // sh. Bad-cwd surfaces as cd's exit status (see `attach_run` doc). args.push("cd -- \"$1\" && exec /bin/sh -c \"$2\"".to_string()); args.push("_".to_string()); args.push(working_directory.to_string());