Skip to content

Commit a2429fc

Browse files
fix(tui): preserve quoted post-create command arguments (#4137)
Signed-off-by: Thota shashank <thotashashank302@gmail.com>
1 parent e7d14ed commit a2429fc

7 files changed

Lines changed: 137 additions & 23 deletions

File tree

‎.agents/skills/tui-development/SKILL.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,8 @@ TUI actions should parallel `openshell` CLI commands so users have familiar ment
330330

331331
When adding new TUI features, check what the CLI offers and maintain consistency.
332332

333+
The create form parses the optional Command field as shell words before starting creation. Preserve the parsed argument vector through post-create execution and shell-escape each argument at the SSH boundary. Quoting groups arguments; expansions and operators remain literal unless the user explicitly invokes a shell such as `sh -c`. Invalid quoting must leave the form open with an error and must not queue sandbox creation.
334+
333335
### Scrollable views follow k9s conventions
334336

335337
Any scrollable content (logs, future long lists) should follow the k9s autoscroll pattern:

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ tokio-stream = "0.1"
128128
protoc-bin-vendored = "3.2.0"
129129
url = "2"
130130
indexmap = "2"
131+
shell-words = "1.1.1"
131132

132133
# Database
133134
sqlx = { version = "0.9", default-features = false, features = ["runtime-tokio", "tls-rustls-aws-lc-rs", "postgres", "sqlite", "migrate", "macros"] }

‎crates/openshell-tui/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ owo-colors = { workspace = true }
2828
serde = { workspace = true }
2929
url = { workspace = true }
3030
indexmap = { workspace = true }
31+
shell-words = { workspace = true }
3132

3233
[lints]
3334
workspace = true

‎crates/openshell-tui/src/app.rs‎

Lines changed: 78 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,8 @@ impl GatewayEntry {
241241
// ---------------------------------------------------------------------------
242242

243243
/// Data extracted from the create sandbox form:
244-
/// `(name, image, command, selected_provider_names, forward_specs)`.
244+
/// `(name, image, selected_provider_names, forward_specs)`.
245245
pub type CreateFormData = (
246-
String,
247246
String,
248247
String,
249248
Vec<String>,
@@ -723,8 +722,8 @@ pub struct App {
723722
pub pending_create_sandbox: bool,
724723
/// Forward specs to apply after sandbox creation completes.
725724
pub pending_forward_ports: Vec<openshell_core::forward::ForwardSpec>,
726-
/// Command to exec via SSH after sandbox creation completes.
727-
pub pending_exec_command: String,
725+
/// Parsed arguments to exec via SSH after sandbox creation completes.
726+
pub pending_exec_command: Vec<String>,
728727
/// Animation ticker handle — aborted when animation stops.
729728
pub anim_handle: Option<tokio::task::JoinHandle<()>>,
730729

@@ -1078,7 +1077,7 @@ impl App {
10781077
create_form: None,
10791078
pending_create_sandbox: false,
10801079
pending_forward_ports: Vec::new(),
1081-
pending_exec_command: String::new(),
1080+
pending_exec_command: Vec::new(),
10821081
anim_handle: None,
10831082
sandbox_log_lines: Vec::new(),
10841083
sandbox_log_scroll: 0,
@@ -2396,6 +2395,13 @@ impl App {
23962395
}
23972396
CreateFormField::Submit => {
23982397
if key.code == KeyCode::Enter {
2398+
match shell_words::split(&form.command) {
2399+
Ok(command) => self.pending_exec_command = command,
2400+
Err(error) => {
2401+
form.status = Some(format!("Invalid command: {error}"));
2402+
return;
2403+
}
2404+
}
23992405
form.anim_start = Some(Instant::now());
24002406
form.status = None;
24012407
form.phase = CreatePhase::Creating;
@@ -2408,7 +2414,7 @@ impl App {
24082414
}
24092415

24102416
/// Build the form data needed for the gRPC `CreateSandbox` request.
2411-
/// Returns `(name, image, command, selected_provider_names, forward_ports)`.
2417+
/// Returns `(name, image, selected_provider_names, forward_ports)`.
24122418
pub fn create_form_data(&self) -> Option<CreateFormData> {
24132419
let form = self.create_form.as_ref()?;
24142420
let providers: Vec<String> = form
@@ -2428,13 +2434,7 @@ impl App {
24282434
openshell_core::forward::ForwardSpec::parse(s).ok()
24292435
})
24302436
.collect();
2431-
Some((
2432-
form.name.clone(),
2433-
form.image.clone(),
2434-
form.command.clone(),
2435-
providers,
2436-
ports,
2437-
))
2437+
Some((form.name.clone(), form.image.clone(), providers, ports))
24382438
}
24392439

24402440
// ------------------------------------------------------------------
@@ -3653,6 +3653,71 @@ mod tests {
36533653
)
36543654
}
36553655

3656+
#[tokio::test]
3657+
async fn create_command_preserves_quoted_and_escaped_arguments() {
3658+
let cases: &[(&str, &[&str])] = &[
3659+
(
3660+
r#"/bin/sh -c "echo GOOD; read x""#,
3661+
&["/bin/sh", "-c", "echo GOOD; read x"],
3662+
),
3663+
(
3664+
r#"echo 'hello world' "" a\ b "it's""#,
3665+
&["echo", "hello world", "", "a b", "it's"],
3666+
),
3667+
(r#"echo pre"fix value"post"#, &["echo", "prefix valuepost"]),
3668+
(
3669+
"echo $HOME $(id) ; | > *.txt",
3670+
&["echo", "$HOME", "$(id)", ";", "|", ">", "*.txt"],
3671+
),
3672+
("echo hello", &["echo", "hello"]),
3673+
("", &[]),
3674+
(" \t", &[]),
3675+
];
3676+
for (command, expected) in cases {
3677+
let mut app = test_app();
3678+
app.create_form = Some(CreateSandboxForm {
3679+
command: (*command).into(),
3680+
focused_field: CreateFormField::Submit,
3681+
..Default::default()
3682+
});
3683+
app.handle_create_form_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
3684+
assert!(app.pending_create_sandbox, "{command}");
3685+
assert_eq!(app.pending_exec_command, *expected, "{command}");
3686+
let form = app.create_form.as_ref().unwrap();
3687+
assert_eq!(form.phase, CreatePhase::Creating);
3688+
assert!(form.status.is_none());
3689+
}
3690+
}
3691+
3692+
#[tokio::test]
3693+
async fn malformed_create_command_stays_in_form_until_corrected() {
3694+
for command in ["echo \"unfinished", "echo 'unfinished", "echo \"trailing\\"] {
3695+
let mut app = test_app();
3696+
app.create_form = Some(CreateSandboxForm {
3697+
command: command.into(),
3698+
focused_field: CreateFormField::Submit,
3699+
..Default::default()
3700+
});
3701+
app.handle_create_form_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
3702+
assert!(!app.pending_create_sandbox, "{command}");
3703+
assert!(app.pending_exec_command.is_empty());
3704+
let form = app.create_form.as_mut().unwrap();
3705+
assert_eq!(form.phase, CreatePhase::Form);
3706+
assert!(form.anim_start.is_none());
3707+
assert!(
3708+
form.status
3709+
.as_ref()
3710+
.unwrap()
3711+
.starts_with("Invalid command:")
3712+
);
3713+
form.command = "echo corrected".into();
3714+
app.handle_create_form_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
3715+
assert!(app.pending_create_sandbox);
3716+
assert_eq!(app.pending_exec_command, ["echo", "corrected"]);
3717+
assert!(app.create_form.as_ref().unwrap().status.is_none());
3718+
}
3719+
}
3720+
36563721
fn provider_profile(
36573722
id: &str,
36583723
credentials: Vec<openshell_core::proto::ProviderProfileCredential>,

‎crates/openshell-tui/src/lib.rs‎

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,7 +1083,7 @@ async fn handle_exec_command(
10831083
terminal: &mut Terminal<CrosstermBackend<io::Stdout>>,
10841084
events: &mut EventHandler,
10851085
sandbox_name: &str,
1086-
command: &str,
1086+
command: &[String],
10871087
workspace: &str,
10881088
) -> Result<()> {
10891089
let session = {
@@ -1135,13 +1135,9 @@ async fn handle_exec_command(
11351135
);
11361136

11371137
// Step 3: Build SSH command — same flags as handle_shell_connect but with
1138-
// the user's command appended. Each word is escaped individually so the
1138+
// the parsed command appended. Each argument is escaped individually so the
11391139
// remote shell parses it correctly.
1140-
let command_str = command
1141-
.split_whitespace()
1142-
.map(shell_escape)
1143-
.collect::<Vec<_>>()
1144-
.join(" ");
1140+
let command_str = build_exec_command(command);
11451141
let mut ssh = std::process::Command::new("ssh");
11461142
ssh.arg("-o")
11471143
.arg(format!("ProxyCommand={proxy_command}"))
@@ -1217,6 +1213,14 @@ use openshell_core::forward::{
12171213
validate_ssh_session_response,
12181214
};
12191215

1216+
fn build_exec_command(command: &[String]) -> String {
1217+
command
1218+
.iter()
1219+
.map(|arg| shell_escape(arg))
1220+
.collect::<Vec<_>>()
1221+
.join(" ")
1222+
}
1223+
12201224
/// Convert a `SandboxPolicy` proto into styled ratatui lines for the policy viewer.
12211225
fn render_policy_lines(
12221226
policy: &openshell_core::proto::SandboxPolicy,
@@ -1400,12 +1404,10 @@ fn start_anim_ticker(app: &mut App, tx: mpsc::UnboundedSender<Event>) {
14001404

14011405
fn spawn_create_sandbox(app: &mut App, tx: mpsc::UnboundedSender<Event>) {
14021406
let mut client = app.client.clone();
1403-
let Some((name, image, command, selected_providers, ports)) = app.create_form_data() else {
1407+
let Some((name, image, selected_providers, ports)) = app.create_form_data() else {
14041408
return;
14051409
};
14061410

1407-
// Stash command so we can exec after sandbox creation + Ready.
1408-
app.pending_exec_command = command;
14091411
// Stash ports so we can include them in the status text.
14101412
app.pending_forward_ports.clone_from(&ports);
14111413

@@ -3130,6 +3132,46 @@ fn format_age(epoch_ms: i64) -> String {
31303132
}
31313133
}
31323134

3135+
#[cfg(all(test, unix))]
3136+
mod exec_command_tests {
3137+
use super::build_exec_command;
3138+
use std::io::Write;
3139+
use std::process::{Command, Stdio};
3140+
3141+
fn run_command(command: &str, stdin: &[u8]) -> std::process::Output {
3142+
let args = shell_words::split(command).unwrap();
3143+
let mut child = Command::new("/bin/sh")
3144+
.args(["-c", &build_exec_command(&args)])
3145+
.stdin(Stdio::piped())
3146+
.stdout(Stdio::piped())
3147+
.stderr(Stdio::piped())
3148+
.spawn()
3149+
.unwrap();
3150+
child.stdin.take().unwrap().write_all(stdin).unwrap();
3151+
child.wait_with_output().unwrap()
3152+
}
3153+
3154+
#[test]
3155+
fn quoted_script_runs_and_reads_stdin() {
3156+
let output = run_command(r#"/bin/sh -c "echo GOOD; read x; echo $x""#, b"entered\n");
3157+
assert!(output.status.success(), "{:?}", output.stderr);
3158+
assert_eq!(output.stdout, b"GOOD\nentered\n");
3159+
}
3160+
3161+
#[test]
3162+
fn arguments_remain_literal_at_remote_shell_boundary() {
3163+
let output = run_command(
3164+
r#"printf '%s\n' 'hello world' "" "it's" a\ b $HOME '$(echo BAD)' ';' '|' '>' '*.txt'"#,
3165+
b"",
3166+
);
3167+
assert!(output.status.success(), "{:?}", output.stderr);
3168+
assert_eq!(
3169+
output.stdout,
3170+
b"hello world\n\nit's\na b\n$HOME\n$(echo BAD)\n;\n|\n>\n*.txt\n"
3171+
);
3172+
}
3173+
}
3174+
31333175
#[cfg(test)]
31343176
mod draft_approve_all_message_tests {
31353177
use super::*;

‎docs/how-it-works/sandboxes/overview.mdx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,8 @@ The dashboard has three panels stacked vertically: Gateways, Providers (or Globa
763763
764764
The sandbox table’s NOTES column shows `Invalid config` when policy or provider configuration blocks provisioning. Open the sandbox detail view for the full rejection reason, or run `openshell sandbox get <name> -o json`. The note clears after the configuration is repaired; active port forwards remain listed.
765765
766+
Press `c` in the Sandboxes panel to create a sandbox. In the optional Command field, use quotes or backslashes to group arguments containing spaces. For example, `/bin/sh -c "echo GOOD; read x"` prints `GOOD` and waits for Enter. Invalid quoting keeps the form open without creating a sandbox. The field does not expand variables or evaluate shell operators; invoke a shell explicitly with `sh -c` when you need that behavior.
767+
766768
## Port Forwarding
767769
768770
Forward a local port to a running sandbox to access services inside it, such as a web server or database:

0 commit comments

Comments
 (0)