Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
246 changes: 244 additions & 2 deletions src/backends/lxc/common/src/lxc_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,57 @@ 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<String> {
// 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<String> = Vec::with_capacity(env.len() + 8);

// 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<String> = 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());
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: 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());
args.push(command.to_string());
}

args
}

/// Safe wrapper around an LXC container.
pub struct LxcContainer {
name: String,
Expand Down Expand Up @@ -214,6 +265,33 @@ 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.
/// 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 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
/// its watchdog thread can `sigwait` on them; that mask is inherited
Expand All @@ -230,15 +308,16 @@ impl LxcContainer {
pub fn attach_run(
&self,
command: &str,
_working_directory: &str,
working_directory: &str,
env: &[String],
timeout: Option<std::time::Duration>,
) -> Result<(i32, String, String), String> {
use mxc_pty::{run_with_pty, PtyOptions, PtyOutcome, Signal};

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,
Expand All @@ -264,6 +343,7 @@ impl LxcContainer {
&self,
_command: &str,
_working_directory: &str,
_env: &[String],
_timeout: Option<std::time::Duration>,
) -> Result<(i32, String, String), String> {
Err("LxcContainer::attach_run is only supported on Linux".to_string())
Expand Down Expand Up @@ -493,4 +573,166 @@ 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 <command>` 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![
"--clear-env",
"--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![
"--clear-env",
"--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![
"--clear-env",
"--set-var=FOO=bar",
"--",
"/bin/sh",
"-c",
"cd -- \"$1\" && exec /bin/sh -c \"$2\"",
"_",
"/work",
"cmd",
]
);
}

#[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"]);
}
}
8 changes: 6 additions & 2 deletions src/backends/lxc/common/src/lxc_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions tests/configs/lxc_env_cwd_test.json
Original file line number Diff line number Diff line change
@@ -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:-<unset>}', want 'bar baz'\" >&2; exit 12; }; [ \"${MXC_TEST_EQ:-}\" = \"a=b=c\" ] || { echo \"FAIL env MXC_TEST_EQ: got '${MXC_TEST_EQ:-<unset>}', 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"
}
}
1 change: 1 addition & 0 deletions tests/scripts/run_lxc_all_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
33 changes: 33 additions & 0 deletions tests/scripts/run_lxc_env_cwd_test.sh
Original file line number Diff line number Diff line change
@@ -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."
Loading