diff --git a/.agents/skills/tui-development/SKILL.md b/.agents/skills/tui-development/SKILL.md index 832bb75144..6c6e5828ca 100644 --- a/.agents/skills/tui-development/SKILL.md +++ b/.agents/skills/tui-development/SKILL.md @@ -330,6 +330,8 @@ TUI actions should parallel `openshell` CLI commands so users have familiar ment When adding new TUI features, check what the CLI offers and maintain consistency. +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. + ### Scrollable views follow k9s conventions Any scrollable content (logs, future long lists) should follow the k9s autoscroll pattern: diff --git a/Cargo.lock b/Cargo.lock index fc0b173702..f74f47f6e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4898,6 +4898,7 @@ dependencies = [ "owo-colors", "ratatui", "serde", + "shell-words", "terminal-colorsaurus", "tokio", "tonic", diff --git a/Cargo.toml b/Cargo.toml index a50457f136..1f25efe413 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -128,6 +128,7 @@ tokio-stream = "0.1" protoc-bin-vendored = "3.2.0" url = "2" indexmap = "2" +shell-words = "1.1.1" # Database sqlx = { version = "0.9", default-features = false, features = ["runtime-tokio", "tls-rustls-aws-lc-rs", "postgres", "sqlite", "migrate", "macros"] } diff --git a/crates/openshell-tui/Cargo.toml b/crates/openshell-tui/Cargo.toml index bc79d114b2..68d6e11c93 100644 --- a/crates/openshell-tui/Cargo.toml +++ b/crates/openshell-tui/Cargo.toml @@ -28,6 +28,7 @@ owo-colors = { workspace = true } serde = { workspace = true } url = { workspace = true } indexmap = { workspace = true } +shell-words = { workspace = true } [lints] workspace = true diff --git a/crates/openshell-tui/src/app.rs b/crates/openshell-tui/src/app.rs index 4b218a37cf..a6de6ce38e 100644 --- a/crates/openshell-tui/src/app.rs +++ b/crates/openshell-tui/src/app.rs @@ -241,9 +241,8 @@ impl GatewayEntry { // --------------------------------------------------------------------------- /// Data extracted from the create sandbox form: -/// `(name, image, command, selected_provider_names, forward_specs)`. +/// `(name, image, selected_provider_names, forward_specs)`. pub type CreateFormData = ( - String, String, String, Vec, @@ -723,8 +722,8 @@ pub struct App { pub pending_create_sandbox: bool, /// Forward specs to apply after sandbox creation completes. pub pending_forward_ports: Vec, - /// Command to exec via SSH after sandbox creation completes. - pub pending_exec_command: String, + /// Parsed arguments to exec via SSH after sandbox creation completes. + pub pending_exec_command: Vec, /// Animation ticker handle — aborted when animation stops. pub anim_handle: Option>, @@ -1078,7 +1077,7 @@ impl App { create_form: None, pending_create_sandbox: false, pending_forward_ports: Vec::new(), - pending_exec_command: String::new(), + pending_exec_command: Vec::new(), anim_handle: None, sandbox_log_lines: Vec::new(), sandbox_log_scroll: 0, @@ -2396,6 +2395,13 @@ impl App { } CreateFormField::Submit => { if key.code == KeyCode::Enter { + match shell_words::split(&form.command) { + Ok(command) => self.pending_exec_command = command, + Err(error) => { + form.status = Some(format!("Invalid command: {error}")); + return; + } + } form.anim_start = Some(Instant::now()); form.status = None; form.phase = CreatePhase::Creating; @@ -2408,7 +2414,7 @@ impl App { } /// Build the form data needed for the gRPC `CreateSandbox` request. - /// Returns `(name, image, command, selected_provider_names, forward_ports)`. + /// Returns `(name, image, selected_provider_names, forward_ports)`. pub fn create_form_data(&self) -> Option { let form = self.create_form.as_ref()?; let providers: Vec = form @@ -2428,13 +2434,7 @@ impl App { openshell_core::forward::ForwardSpec::parse(s).ok() }) .collect(); - Some(( - form.name.clone(), - form.image.clone(), - form.command.clone(), - providers, - ports, - )) + Some((form.name.clone(), form.image.clone(), providers, ports)) } // ------------------------------------------------------------------ @@ -3653,6 +3653,71 @@ mod tests { ) } + #[tokio::test] + async fn create_command_preserves_quoted_and_escaped_arguments() { + let cases: &[(&str, &[&str])] = &[ + ( + r#"/bin/sh -c "echo GOOD; read x""#, + &["/bin/sh", "-c", "echo GOOD; read x"], + ), + ( + r#"echo 'hello world' "" a\ b "it's""#, + &["echo", "hello world", "", "a b", "it's"], + ), + (r#"echo pre"fix value"post"#, &["echo", "prefix valuepost"]), + ( + "echo $HOME $(id) ; | > *.txt", + &["echo", "$HOME", "$(id)", ";", "|", ">", "*.txt"], + ), + ("echo hello", &["echo", "hello"]), + ("", &[]), + (" \t", &[]), + ]; + for (command, expected) in cases { + let mut app = test_app(); + app.create_form = Some(CreateSandboxForm { + command: (*command).into(), + focused_field: CreateFormField::Submit, + ..Default::default() + }); + app.handle_create_form_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(app.pending_create_sandbox, "{command}"); + assert_eq!(app.pending_exec_command, *expected, "{command}"); + let form = app.create_form.as_ref().unwrap(); + assert_eq!(form.phase, CreatePhase::Creating); + assert!(form.status.is_none()); + } + } + + #[tokio::test] + async fn malformed_create_command_stays_in_form_until_corrected() { + for command in ["echo \"unfinished", "echo 'unfinished", "echo \"trailing\\"] { + let mut app = test_app(); + app.create_form = Some(CreateSandboxForm { + command: command.into(), + focused_field: CreateFormField::Submit, + ..Default::default() + }); + app.handle_create_form_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(!app.pending_create_sandbox, "{command}"); + assert!(app.pending_exec_command.is_empty()); + let form = app.create_form.as_mut().unwrap(); + assert_eq!(form.phase, CreatePhase::Form); + assert!(form.anim_start.is_none()); + assert!( + form.status + .as_ref() + .unwrap() + .starts_with("Invalid command:") + ); + form.command = "echo corrected".into(); + app.handle_create_form_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(app.pending_create_sandbox); + assert_eq!(app.pending_exec_command, ["echo", "corrected"]); + assert!(app.create_form.as_ref().unwrap().status.is_none()); + } + } + fn provider_profile( id: &str, credentials: Vec, diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index f235c4e003..2265329bcf 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -1083,7 +1083,7 @@ async fn handle_exec_command( terminal: &mut Terminal>, events: &mut EventHandler, sandbox_name: &str, - command: &str, + command: &[String], workspace: &str, ) -> Result<()> { let session = { @@ -1135,13 +1135,9 @@ async fn handle_exec_command( ); // Step 3: Build SSH command — same flags as handle_shell_connect but with - // the user's command appended. Each word is escaped individually so the + // the parsed command appended. Each argument is escaped individually so the // remote shell parses it correctly. - let command_str = command - .split_whitespace() - .map(shell_escape) - .collect::>() - .join(" "); + let command_str = build_exec_command(command); let mut ssh = std::process::Command::new("ssh"); ssh.arg("-o") .arg(format!("ProxyCommand={proxy_command}")) @@ -1217,6 +1213,14 @@ use openshell_core::forward::{ validate_ssh_session_response, }; +fn build_exec_command(command: &[String]) -> String { + command + .iter() + .map(|arg| shell_escape(arg)) + .collect::>() + .join(" ") +} + /// Convert a `SandboxPolicy` proto into styled ratatui lines for the policy viewer. fn render_policy_lines( policy: &openshell_core::proto::SandboxPolicy, @@ -1400,12 +1404,10 @@ fn start_anim_ticker(app: &mut App, tx: mpsc::UnboundedSender) { fn spawn_create_sandbox(app: &mut App, tx: mpsc::UnboundedSender) { let mut client = app.client.clone(); - let Some((name, image, command, selected_providers, ports)) = app.create_form_data() else { + let Some((name, image, selected_providers, ports)) = app.create_form_data() else { return; }; - // Stash command so we can exec after sandbox creation + Ready. - app.pending_exec_command = command; // Stash ports so we can include them in the status text. app.pending_forward_ports.clone_from(&ports); @@ -3130,6 +3132,46 @@ fn format_age(epoch_ms: i64) -> String { } } +#[cfg(all(test, unix))] +mod exec_command_tests { + use super::build_exec_command; + use std::io::Write; + use std::process::{Command, Stdio}; + + fn run_command(command: &str, stdin: &[u8]) -> std::process::Output { + let args = shell_words::split(command).unwrap(); + let mut child = Command::new("/bin/sh") + .args(["-c", &build_exec_command(&args)]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + child.stdin.take().unwrap().write_all(stdin).unwrap(); + child.wait_with_output().unwrap() + } + + #[test] + fn quoted_script_runs_and_reads_stdin() { + let output = run_command(r#"/bin/sh -c "echo GOOD; read x; echo $x""#, b"entered\n"); + assert!(output.status.success(), "{:?}", output.stderr); + assert_eq!(output.stdout, b"GOOD\nentered\n"); + } + + #[test] + fn arguments_remain_literal_at_remote_shell_boundary() { + let output = run_command( + r#"printf '%s\n' 'hello world' "" "it's" a\ b $HOME '$(echo BAD)' ';' '|' '>' '*.txt'"#, + b"", + ); + assert!(output.status.success(), "{:?}", output.stderr); + assert_eq!( + output.stdout, + b"hello world\n\nit's\na b\n$HOME\n$(echo BAD)\n;\n|\n>\n*.txt\n" + ); + } +} + #[cfg(test)] mod draft_approve_all_message_tests { use super::*; diff --git a/docs/how-it-works/sandboxes/overview.mdx b/docs/how-it-works/sandboxes/overview.mdx index c39c71a3a4..53e213165f 100644 --- a/docs/how-it-works/sandboxes/overview.mdx +++ b/docs/how-it-works/sandboxes/overview.mdx @@ -759,6 +759,8 @@ The dashboard has three panels stacked vertically: Gateways, Providers (or Globa 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 -o json`. The note clears after the configuration is repaired; active port forwards remain listed. +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. + ## Port Forwarding Forward a local port to a running sandbox to access services inside it, such as a web server or database: