diff --git a/codex-rs/core/src/command_safety/is_safe_command.rs b/codex-rs/core/src/command_safety/is_safe_command.rs index 5e7a7034d..01a52026e 100644 --- a/codex-rs/core/src/command_safety/is_safe_command.rs +++ b/codex-rs/core/src/command_safety/is_safe_command.rs @@ -47,24 +47,47 @@ fn is_safe_to_call_with_exec(command: &[String]) -> bool { .file_name() .and_then(|osstr| osstr.to_str()) { + Some(cmd) if cfg!(target_os = "linux") && matches!(cmd, "numfmt" | "tac") => true, + #[rustfmt::skip] Some( "cat" | "cd" | + "cut" | "echo" | + "expr" | "false" | "grep" | "head" | + "id" | "ls" | "nl" | + "paste" | "pwd" | + "rev" | + "seq" | + "stat" | "tail" | + "tr" | "true" | + "uname" | + "uniq" | "wc" | - "which") => { + "which" | + "whoami") => { true }, + Some("base64") => { + const UNSAFE_BASE64_OPTIONS: &[&str] = &["-o", "--output"]; + + !command.iter().skip(1).any(|arg| { + UNSAFE_BASE64_OPTIONS.contains(&arg.as_str()) + || arg.starts_with("--output=") + || (arg.starts_with("-o") && arg != "-o") + }) + } + Some("find") => { // Certain options to `find` can delete files, write to files, or // execute arbitrary commands, so we cannot auto-approve the @@ -184,6 +207,7 @@ mod tests { fn known_safe_examples() { assert!(is_safe_to_call_with_exec(&vec_str(&["ls"]))); assert!(is_safe_to_call_with_exec(&vec_str(&["git", "status"]))); + assert!(is_safe_to_call_with_exec(&vec_str(&["base64"]))); assert!(is_safe_to_call_with_exec(&vec_str(&[ "sed", "-n", "1,5p", "file.txt" ]))); @@ -197,6 +221,14 @@ mod tests { assert!(is_safe_to_call_with_exec(&vec_str(&[ "find", ".", "-name", "file.txt" ]))); + + if cfg!(target_os = "linux") { + assert!(is_safe_to_call_with_exec(&vec_str(&["numfmt", "1000"]))); + assert!(is_safe_to_call_with_exec(&vec_str(&["tac", "Cargo.toml"]))); + } else { + assert!(!is_safe_to_call_with_exec(&vec_str(&["numfmt", "1000"]))); + assert!(!is_safe_to_call_with_exec(&vec_str(&["tac", "Cargo.toml"]))); + } } #[test] @@ -233,6 +265,21 @@ mod tests { } } + #[test] + fn base64_output_options_are_unsafe() { + for args in [ + vec_str(&["base64", "-o", "out.bin"]), + vec_str(&["base64", "--output", "out.bin"]), + vec_str(&["base64", "--output=out.bin"]), + vec_str(&["base64", "-ob64.txt"]), + ] { + assert!( + !is_safe_to_call_with_exec(&args), + "expected {args:?} to be considered unsafe due to output option" + ); + } + } + #[test] fn ripgrep_rules() { // Safe ripgrep invocations – none of the unsafe flags are present. diff --git a/codex-rs/core/src/powershell.rs b/codex-rs/core/src/powershell.rs index 844866262..8ce540dac 100644 --- a/codex-rs/core/src/powershell.rs +++ b/codex-rs/core/src/powershell.rs @@ -1,5 +1,8 @@ use std::path::PathBuf; +#[cfg(windows)] +use codex_utils_absolute_path::AbsolutePathBuf; + use crate::shell::ShellType; use crate::shell::detect_shell_type; @@ -40,6 +43,93 @@ pub fn extract_powershell_command(command: &[String]) -> Option<(&str, &str)> { None } +/// This function attempts to find a valid PowerShell executable on the system. +/// It first tries to find pwsh.exe, and if that fails, it tries to find +/// powershell.exe. +#[cfg(windows)] +#[allow(dead_code)] +pub(crate) fn try_find_powershellish_executable_blocking() -> Option { + if let Some(pwsh_path) = try_find_pwsh_executable_blocking() { + Some(pwsh_path) + } else { + try_find_powershell_executable_blocking() + } +} + +/// This function attempts to find a powershell.exe executable on the system. +#[cfg(windows)] +pub(crate) fn try_find_powershell_executable_blocking() -> Option { + try_find_powershellish_executable_in_path(&["powershell.exe"]) +} + +/// This function attempts to find a pwsh.exe executable on the system. +/// Note that pwsh.exe and powershell.exe are different executables: +/// +/// - pwsh.exe is the cross-platform PowerShell Core (v6+) executable +/// - powershell.exe is the Windows PowerShell (v5.1 and earlier) executable +/// +/// Further, while powershell.exe is included by default on Windows systems, +/// pwsh.exe must be installed separately by the user. And even when the user +/// has installed pwsh.exe, it may not be available in the system PATH, in which +/// case we attempt to locate it via other means. +#[cfg(windows)] +pub(crate) fn try_find_pwsh_executable_blocking() -> Option { + if let Some(ps_home) = std::process::Command::new("cmd") + .args(["/C", "pwsh", "-NoProfile", "-Command", "$PSHOME"]) + .output() + .ok() + .and_then(|out| { + if !out.status.success() { + return None; + } + let stdout = String::from_utf8_lossy(&out.stdout); + let trimmed = stdout.trim(); + (!trimmed.is_empty()).then(|| trimmed.to_string()) + }) + { + let candidate = AbsolutePathBuf::resolve_path_against_base("pwsh.exe", &ps_home); + + if let Ok(candidate_abs_path) = candidate + && is_powershellish_executable_available(candidate_abs_path.as_path()) + { + return Some(candidate_abs_path); + } + } + + try_find_powershellish_executable_in_path(&["pwsh.exe"]) +} + +#[cfg(windows)] +fn try_find_powershellish_executable_in_path(candidates: &[&str]) -> Option { + for candidate in candidates { + let Ok(resolved_path) = which::which(candidate) else { + continue; + }; + + if !is_powershellish_executable_available(&resolved_path) { + continue; + } + + let Ok(abs_path) = AbsolutePathBuf::from_absolute_path(resolved_path) else { + continue; + }; + + return Some(abs_path); + } + + None +} + +#[cfg(windows)] +fn is_powershellish_executable_available(powershell_or_pwsh_exe: &std::path::Path) -> bool { + // This test works for both powershell.exe and pwsh.exe. + std::process::Command::new(powershell_or_pwsh_exe) + .args(["-NoLogo", "-NoProfile", "-Command", "Write-Output ok"]) + .output() + .map(|output| output.status.success()) + .unwrap_or(false) +} + #[cfg(test)] mod tests { use super::extract_powershell_command; diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index c014af314..f3ed7f023 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -22,8 +22,8 @@ struct SkillFrontmatter { const SKILLS_FILENAME: &str = "SKILL.md"; const SKILLS_DIR_NAME: &str = "skills"; const REPO_ROOT_CONFIG_DIR_NAME: &str = ".codex"; -const MAX_NAME_LEN: usize = 100; -const MAX_DESCRIPTION_LEN: usize = 500; +const MAX_NAME_LEN: usize = 64; +const MAX_DESCRIPTION_LEN: usize = 1024; #[derive(Debug)] enum SkillParseError { @@ -171,7 +171,7 @@ fn validate_field( if value.is_empty() { return Err(SkillParseError::MissingField(field_name)); } - if value.len() > max_len { + if value.chars().count() > max_len { return Err(SkillParseError::InvalidField { field: field_name, reason: format!("exceeds maximum length of {max_len} characters"), @@ -295,12 +295,22 @@ mod tests { #[test] fn enforces_length_limits() { let codex_home = tempfile::tempdir().expect("tempdir"); - let long_desc = "a".repeat(MAX_DESCRIPTION_LEN + 1); - write_skill(&codex_home, "too-long", "toolong", &long_desc); + let max_desc = "\u{1F4A1}".repeat(MAX_DESCRIPTION_LEN); + write_skill(&codex_home, "max-len", "max-len", &max_desc); let cfg = make_config(&codex_home); let outcome = load_skills(&cfg); - assert_eq!(outcome.skills.len(), 0); + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!(outcome.skills.len(), 1); + + let too_long_desc = "\u{1F4A1}".repeat(MAX_DESCRIPTION_LEN + 1); + write_skill(&codex_home, "too-long", "too-long", &too_long_desc); + let outcome = load_skills(&cfg); + assert_eq!(outcome.skills.len(), 1); assert_eq!(outcome.errors.len(), 1); assert!( outcome.errors[0].message.contains("invalid description"), diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 5c3668720..1b53a8d1c 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2397,7 +2397,8 @@ impl ChatWidget { format!("⚠ {effort_label} reasoning effort can quickly consume Plus plan rate limits.") }); let warn_for_model = preset.model.starts_with("gpt-5.1-codex") - || preset.model.starts_with("gpt-5.1-codex-max"); + || preset.model.starts_with("gpt-5.1-codex-max") + || preset.model.starts_with("gpt-5.2"); struct EffortChoice { stored: Option, diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 36f6cc2cb..f9566da90 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -277,33 +277,42 @@ impl Tui { let event_stream = async_stream::stream! { loop { select! { - Some(Ok(event)) = crossterm_events.next() => { - match event { - Event::Key(key_event) => { - #[cfg(unix)] - if SUSPEND_KEY.is_press(key_event) { - let _ = suspend_context.suspend(&alt_screen_active); - // We continue here after resume. - yield TuiEvent::Draw; - continue; + event_result = crossterm_events.next() => { + match event_result { + Some(Ok(event)) => { + match event { + Event::Key(key_event) => { + #[cfg(unix)] + if SUSPEND_KEY.is_press(key_event) { + let _ = suspend_context.suspend(&alt_screen_active); + // We continue here after resume. + yield TuiEvent::Draw; + continue; + } + yield TuiEvent::Key(key_event); + } + Event::Resize(_, _) => { + yield TuiEvent::Draw; + } + Event::Paste(pasted) => { + yield TuiEvent::Paste(pasted); + } + Event::FocusGained => { + terminal_focused.store(true, Ordering::Relaxed); + crate::terminal_palette::requery_default_colors(); + yield TuiEvent::Draw; + } + Event::FocusLost => { + terminal_focused.store(false, Ordering::Relaxed); + } + _ => {} } - yield TuiEvent::Key(key_event); } - Event::Resize(_, _) => { - yield TuiEvent::Draw; - } - Event::Paste(pasted) => { - yield TuiEvent::Paste(pasted); - } - Event::FocusGained => { - terminal_focused.store(true, Ordering::Relaxed); - crate::terminal_palette::requery_default_colors(); - yield TuiEvent::Draw; - } - Event::FocusLost => { - terminal_focused.store(false, Ordering::Relaxed); + Some(Err(_)) | None => { + // Exit the loop in case of broken pipe as we will never + // recover from it + break; } - _ => {} } } result = draw_rx.recv() => { @@ -316,7 +325,9 @@ impl Tui { yield TuiEvent::Draw; } Err(tokio::sync::broadcast::error::RecvError::Closed) => { - // Sender dropped; stop emitting draws from this source. + // Sender dropped. This stream likely outlived its owning `Tui`; + // exit to avoid spinning on a permanently-closed receiver. + break; } } } diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index ea29c00d9..320f5b54d 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -2379,7 +2379,8 @@ impl ChatWidget { format!("⚠ {effort_label} reasoning effort can quickly consume Plus plan rate limits.") }); let warn_for_model = preset.model.starts_with("gpt-5.1-codex") - || preset.model.starts_with("gpt-5.1-codex-max"); + || preset.model.starts_with("gpt-5.1-codex-max") + || preset.model.starts_with("gpt-5.2"); struct EffortChoice { stored: Option, diff --git a/codex-rs/utils/pty/src/lib.rs b/codex-rs/utils/pty/src/lib.rs index f06b44f29..e21af5249 100644 --- a/codex-rs/utils/pty/src/lib.rs +++ b/codex-rs/utils/pty/src/lib.rs @@ -54,6 +54,7 @@ impl ExecCommandSession { pub fn new( writer_tx: mpsc::Sender>, output_tx: broadcast::Sender>, + initial_output_rx: broadcast::Receiver>, killer: Box, reader_handle: JoinHandle<()>, writer_handle: JoinHandle<()>, @@ -62,7 +63,6 @@ impl ExecCommandSession { exit_code: Arc>>, pair: PtyPairWrapper, ) -> (Self, broadcast::Receiver>) { - let initial_output_rx = output_tx.subscribe(); ( Self { writer_tx, @@ -177,6 +177,8 @@ pub async fn spawn_pty_process( let (writer_tx, mut writer_rx) = mpsc::channel::>(128); let (output_tx, _) = broadcast::channel::>(256); + // Subscribe before starting the reader thread. + let initial_output_rx = output_tx.subscribe(); let mut reader = pair.master.try_clone_reader()?; let output_tx_clone = output_tx.clone(); @@ -242,6 +244,7 @@ pub async fn spawn_pty_process( let (session, output_rx) = ExecCommandSession::new( writer_tx, output_tx, + initial_output_rx, killer, reader_handle, writer_handle,