Skip to content

Commit b4ba733

Browse files
committed
fix(sandbox): deliver user command via K8s args instead of env var
The join(" ")/split_whitespace() round-trip through OPENSHELL_SANDBOX_COMMAND mangled arguments containing embedded spaces (e.g. python -c "print('hello world')"). Pass the command as K8s container args instead, which delivers each token as a separate argv entry — preserving argument boundaries exactly with no shell interpretation or encoding.
1 parent ad87da4 commit b4ba733

File tree

2 files changed

+89
-31
lines changed
  • crates

2 files changed

+89
-31
lines changed

crates/openshell-sandbox/src/main.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,13 @@ async fn main() -> Result<()> {
163163
None
164164
};
165165

166-
// Get command - either from CLI args, environment variable, or default to /bin/bash
166+
// Get command - prefer CLI args (delivered via K8s container `args` which
167+
// preserves argument boundaries exactly), then fall back to the
168+
// OPENSHELL_SANDBOX_COMMAND env var (always `sleep infinity` when set by
169+
// the server), then `/bin/bash` as a last resort.
167170
let command = if !args.command.is_empty() {
168171
args.command
169172
} else if let Ok(c) = std::env::var("OPENSHELL_SANDBOX_COMMAND") {
170-
// Simple shell-like splitting on whitespace
171173
c.split_whitespace().map(String::from).collect()
172174
} else {
173175
vec!["/bin/bash".to_string()]

crates/openshell-server/src/sandbox/mod.rs

Lines changed: 85 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,7 +1052,6 @@ fn sandbox_template_to_k8s(
10521052
sandbox_id,
10531053
sandbox_name,
10541054
grpc_endpoint,
1055-
sandbox_command,
10561055
ssh_listen_addr,
10571056
ssh_handshake_secret,
10581057
ssh_handshake_skew_secs,
@@ -1061,6 +1060,15 @@ fn sandbox_template_to_k8s(
10611060

10621061
container.insert("env".to_string(), serde_json::Value::Array(env));
10631062

1063+
// Pass the user's command as container args so it reaches the sandbox
1064+
// binary as argv elements (via clap's trailing_var_arg). This preserves
1065+
// argument boundaries exactly — no shell interpretation, no whitespace
1066+
// splitting — because Kubernetes delivers `args` entries as separate
1067+
// argv strings to the process specified in `command`.
1068+
if !sandbox_command.is_empty() {
1069+
container.insert("args".to_string(), serde_json::json!(sandbox_command));
1070+
}
1071+
10641072
// The sandbox process needs SYS_ADMIN (for seccomp filter installation and
10651073
// network namespace creation), NET_ADMIN (for network namespace veth setup),
10661074
// SYS_PTRACE (for the CONNECT proxy to read /proc/<pid>/fd/ of sandbox-user
@@ -1184,7 +1192,6 @@ fn build_env_list(
11841192
sandbox_id: &str,
11851193
sandbox_name: &str,
11861194
grpc_endpoint: &str,
1187-
sandbox_command: &[String],
11881195
ssh_listen_addr: &str,
11891196
ssh_handshake_secret: &str,
11901197
ssh_handshake_skew_secs: u64,
@@ -1198,7 +1205,6 @@ fn build_env_list(
11981205
sandbox_id,
11991206
sandbox_name,
12001207
grpc_endpoint,
1201-
sandbox_command,
12021208
ssh_listen_addr,
12031209
ssh_handshake_secret,
12041210
ssh_handshake_skew_secs,
@@ -1221,7 +1227,6 @@ fn apply_required_env(
12211227
sandbox_id: &str,
12221228
sandbox_name: &str,
12231229
grpc_endpoint: &str,
1224-
sandbox_command: &[String],
12251230
ssh_listen_addr: &str,
12261231
ssh_handshake_secret: &str,
12271232
ssh_handshake_skew_secs: u64,
@@ -1230,14 +1235,12 @@ fn apply_required_env(
12301235
upsert_env(env, "OPENSHELL_SANDBOX_ID", sandbox_id);
12311236
upsert_env(env, "OPENSHELL_SANDBOX", sandbox_name);
12321237
upsert_env(env, "OPENSHELL_ENDPOINT", grpc_endpoint);
1233-
// Use the user-provided command if present, otherwise fall back to
1234-
// `sleep infinity` so the sandbox pod stays alive for interactive SSH.
1235-
let command_value = if sandbox_command.is_empty() {
1236-
"sleep infinity".to_string()
1237-
} else {
1238-
sandbox_command.join(" ")
1239-
};
1240-
upsert_env(env, "OPENSHELL_SANDBOX_COMMAND", &command_value);
1238+
// Default fallback command so the sandbox pod stays alive for interactive
1239+
// SSH when no user command is provided. When the user *does* supply a
1240+
// command it is delivered as K8s container `args` (preserving argument
1241+
// boundaries exactly) and the sandbox binary's clap parser picks it up
1242+
// from argv before ever consulting this env var.
1243+
upsert_env(env, "OPENSHELL_SANDBOX_COMMAND", "sleep infinity");
12411244
if !ssh_listen_addr.is_empty() {
12421245
upsert_env(env, "OPENSHELL_SSH_LISTEN_ADDR", ssh_listen_addr);
12431246
}
@@ -1635,7 +1638,6 @@ mod tests {
16351638
"sandbox-1",
16361639
"my-sandbox",
16371640
"https://endpoint:8080",
1638-
&[],
16391641
"0.0.0.0:2222",
16401642
"my-secret-value",
16411643
300,
@@ -1655,14 +1657,13 @@ mod tests {
16551657
}
16561658

16571659
#[test]
1658-
fn apply_required_env_uses_sleep_infinity_when_no_command() {
1660+
fn apply_required_env_always_sets_sleep_infinity() {
16591661
let mut env = Vec::new();
16601662
apply_required_env(
16611663
&mut env,
16621664
"sandbox-1",
16631665
"my-sandbox",
16641666
"https://endpoint:8080",
1665-
&[],
16661667
"0.0.0.0:2222",
16671668
"secret",
16681669
300,
@@ -1676,33 +1677,89 @@ mod tests {
16761677
assert_eq!(
16771678
cmd_entry.get("value").and_then(|v| v.as_str()),
16781679
Some("sleep infinity"),
1679-
"default sandbox command should be 'sleep infinity'"
1680+
"OPENSHELL_SANDBOX_COMMAND should always be 'sleep infinity' (user commands are delivered via K8s args)"
16801681
);
16811682
}
16821683

16831684
#[test]
1684-
fn apply_required_env_uses_user_command_when_provided() {
1685-
let mut env = Vec::new();
1686-
apply_required_env(
1687-
&mut env,
1688-
"sandbox-1",
1689-
"my-sandbox",
1690-
"https://endpoint:8080",
1691-
&["python".to_string(), "app.py".to_string()],
1685+
fn user_command_delivered_as_container_args() {
1686+
let user_cmd = vec![
1687+
"python".to_string(),
1688+
"-c".to_string(),
1689+
"print('hello world')".to_string(),
1690+
];
1691+
let pod_template = sandbox_template_to_k8s(
1692+
&SandboxTemplate::default(),
1693+
false,
1694+
"openshell/sandbox:latest",
1695+
"",
1696+
"sandbox-id",
1697+
"sandbox-name",
1698+
"https://gateway.example.com",
1699+
&user_cmd,
16921700
"0.0.0.0:2222",
16931701
"secret",
16941702
300,
1695-
false,
1703+
&std::collections::HashMap::new(),
1704+
"",
1705+
"",
1706+
true,
16961707
);
16971708

1709+
// The user command should appear as container args, preserving each
1710+
// argument as a separate element (no whitespace join/split).
1711+
let args = pod_template["spec"]["containers"][0]["args"]
1712+
.as_array()
1713+
.expect("container args should be set when user provides a command");
1714+
assert_eq!(
1715+
args,
1716+
&[
1717+
serde_json::json!("python"),
1718+
serde_json::json!("-c"),
1719+
serde_json::json!("print('hello world')"),
1720+
],
1721+
"args must preserve argument boundaries exactly"
1722+
);
1723+
1724+
// The env var should still be sleep infinity (fallback only).
1725+
let env = pod_template["spec"]["containers"][0]["env"]
1726+
.as_array()
1727+
.expect("env should exist");
16981728
let cmd_entry = env
16991729
.iter()
17001730
.find(|e| e.get("name").and_then(|v| v.as_str()) == Some("OPENSHELL_SANDBOX_COMMAND"))
1701-
.expect("OPENSHELL_SANDBOX_COMMAND must be present in env");
1731+
.expect("OPENSHELL_SANDBOX_COMMAND must be present");
17021732
assert_eq!(
17031733
cmd_entry.get("value").and_then(|v| v.as_str()),
1704-
Some("python app.py"),
1705-
"sandbox command should reflect user-provided command"
1734+
Some("sleep infinity"),
1735+
"env var should be sleep infinity even when user command is provided"
1736+
);
1737+
}
1738+
1739+
#[test]
1740+
fn no_container_args_when_command_empty() {
1741+
let pod_template = sandbox_template_to_k8s(
1742+
&SandboxTemplate::default(),
1743+
false,
1744+
"openshell/sandbox:latest",
1745+
"",
1746+
"sandbox-id",
1747+
"sandbox-name",
1748+
"https://gateway.example.com",
1749+
&[],
1750+
"0.0.0.0:2222",
1751+
"secret",
1752+
300,
1753+
&std::collections::HashMap::new(),
1754+
"",
1755+
"",
1756+
true,
1757+
);
1758+
1759+
// No args should be set when the user didn't provide a command.
1760+
assert!(
1761+
pod_template["spec"]["containers"][0]["args"].is_null(),
1762+
"container args should not be set when no user command is provided"
17061763
);
17071764
}
17081765

@@ -1818,7 +1875,6 @@ mod tests {
18181875
"sandbox-1",
18191876
"my-sandbox",
18201877
"https://endpoint:8080",
1821-
&[],
18221878
"0.0.0.0:2222",
18231879
"secret",
18241880
300,

0 commit comments

Comments
 (0)