From 5e236a9b30cf321bd390cfb5ee3a93295e9f6b6c Mon Sep 17 00:00:00 2001 From: jif-oai Date: Tue, 10 Feb 2026 13:13:25 +0000 Subject: [PATCH] Extract command safety crate --- codex-rs/Cargo.lock | 18 ++ codex-rs/Cargo.toml | 2 + codex-rs/command-safety/Cargo.toml | 26 +++ codex-rs/command-safety/src/bash_parse.rs | 207 ++++++++++++++++++ .../src}/is_dangerous_command.rs | 2 +- .../src}/is_safe_command.rs | 6 +- codex-rs/command-safety/src/lib.rs | 8 + .../src}/powershell_parser.ps1 | 0 .../src}/windows_dangerous_commands.rs | 0 .../src}/windows_safe_commands.rs | 0 codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/command_safety/mod.rs | 3 - codex-rs/core/src/exec_policy.rs | 4 +- codex-rs/core/src/lib.rs | 3 - codex-rs/core/src/tools/handlers/shell.rs | 4 +- .../core/src/tools/handlers/unified_exec.rs | 2 +- codex-rs/exec-server/Cargo.toml | 1 + codex-rs/exec-server/src/posix.rs | 2 +- 18 files changed, 273 insertions(+), 16 deletions(-) create mode 100644 codex-rs/command-safety/Cargo.toml create mode 100644 codex-rs/command-safety/src/bash_parse.rs rename codex-rs/{core/src/command_safety => command-safety/src}/is_dangerous_command.rs (99%) rename codex-rs/{core/src/command_safety => command-safety/src}/is_safe_command.rs (98%) create mode 100644 codex-rs/command-safety/src/lib.rs rename codex-rs/{core/src/command_safety => command-safety/src}/powershell_parser.ps1 (100%) rename codex-rs/{core/src/command_safety => command-safety/src}/windows_dangerous_commands.rs (100%) rename codex-rs/{core/src/command_safety => command-safety/src}/windows_safe_commands.rs (100%) delete mode 100644 codex-rs/core/src/command_safety/mod.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 2bb451b583..59e92e2997 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1628,6 +1628,22 @@ dependencies = [ "thiserror 2.0.18", ] +[[package]] +name = "codex-command-safety" +version = "0.0.0" +dependencies = [ + "base64 0.22.1", + "once_cell", + "pretty_assertions", + "regex", + "serde", + "serde_json", + "shlex", + "tree-sitter", + "tree-sitter-bash", + "url", +] + [[package]] name = "codex-common" version = "0.0.0" @@ -1665,6 +1681,7 @@ dependencies = [ "codex-arg0", "codex-async-utils", "codex-client", + "codex-command-safety", "codex-core", "codex-execpolicy", "codex-file-search", @@ -1803,6 +1820,7 @@ dependencies = [ "anyhow", "async-trait", "clap", + "codex-command-safety", "codex-core", "codex-execpolicy", "codex-utils-cargo-bin", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 02a09e5624..950fb2e506 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -14,6 +14,7 @@ members = [ "cloud-requirements", "cloud-tasks", "cloud-tasks-client", + "command-safety", "cli", "common", "core", @@ -80,6 +81,7 @@ codex-cloud-requirements = { path = "cloud-requirements" } codex-chatgpt = { path = "chatgpt" } codex-cli = { path = "cli"} codex-client = { path = "codex-client" } +codex-command-safety = { path = "command-safety" } codex-common = { path = "common" } codex-core = { path = "core" } codex-secrets = { path = "secrets" } diff --git a/codex-rs/command-safety/Cargo.toml b/codex-rs/command-safety/Cargo.toml new file mode 100644 index 0000000000..4a91f808b0 --- /dev/null +++ b/codex-rs/command-safety/Cargo.toml @@ -0,0 +1,26 @@ +[package] +name = "codex-command-safety" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lib] +name = "codex_command_safety" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +base64 = { workspace = true } +once_cell = { workspace = true } +regex = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +shlex = { workspace = true } +tree-sitter = { workspace = true } +tree-sitter-bash = { workspace = true } +url = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } diff --git a/codex-rs/command-safety/src/bash_parse.rs b/codex-rs/command-safety/src/bash_parse.rs new file mode 100644 index 0000000000..b96f81955a --- /dev/null +++ b/codex-rs/command-safety/src/bash_parse.rs @@ -0,0 +1,207 @@ +use std::path::Path; + +use tree_sitter::Node; +use tree_sitter::Parser; +use tree_sitter::Tree; +use tree_sitter_bash::LANGUAGE as BASH; + +/// Parse the provided bash source using tree-sitter-bash, returning a Tree on +/// success or None if parsing failed. +pub fn try_parse_shell(shell_lc_arg: &str) -> Option { + let lang = BASH.into(); + let mut parser = Parser::new(); + #[expect(clippy::expect_used)] + parser.set_language(&lang).expect("load bash grammar"); + let old_tree: Option<&Tree> = None; + parser.parse(shell_lc_arg, old_tree) +} + +/// Parse a script which may contain multiple simple commands joined only by +/// the safe logical/pipe/sequencing operators: `&&`, `||`, `;`, `|`. +/// +/// Returns `Some(Vec)` if every command is a plain word‑only +/// command and the parse tree does not contain disallowed constructs +/// (parentheses, redirections, substitutions, control flow, etc.). Otherwise +/// returns `None`. +pub fn try_parse_word_only_commands_sequence(tree: &Tree, src: &str) -> Option>> { + if tree.root_node().has_error() { + return None; + } + + // List of allowed (named) node kinds for a "word only commands sequence". + // If we encounter a named node that is not in this list we reject. + const ALLOWED_KINDS: &[&str] = &[ + // top level containers + "program", + "list", + "pipeline", + // commands & words + "command", + "command_name", + "word", + "string", + "string_content", + "raw_string", + "number", + "concatenation", + ]; + // Allow only safe punctuation / operator tokens; anything else causes reject. + const ALLOWED_PUNCT_TOKENS: &[&str] = &["&&", "||", ";", "|", "\"", "'"]; + + let root = tree.root_node(); + let mut cursor = root.walk(); + let mut stack = vec![root]; + let mut command_nodes = Vec::new(); + while let Some(node) = stack.pop() { + let kind = node.kind(); + if node.is_named() { + if !ALLOWED_KINDS.contains(&kind) { + return None; + } + if kind == "command" { + command_nodes.push(node); + } + } else { + // Reject any punctuation / operator tokens that are not explicitly allowed. + if kind.chars().any(|c| "&;|".contains(c)) && !ALLOWED_PUNCT_TOKENS.contains(&kind) { + return None; + } + if !(ALLOWED_PUNCT_TOKENS.contains(&kind) || kind.trim().is_empty()) { + // If it's a quote token or operator it's allowed above; we also allow whitespace tokens. + // Any other punctuation like parentheses, braces, redirects, backticks, etc are rejected. + return None; + } + } + for child in node.children(&mut cursor) { + stack.push(child); + } + } + + // Walk uses a stack (LIFO), so re-sort by position to restore source order. + command_nodes.sort_by_key(Node::start_byte); + + let mut commands = Vec::new(); + for node in command_nodes { + if let Some(words) = parse_plain_command_from_node(node, src) { + commands.push(words); + } else { + return None; + } + } + Some(commands) +} + +pub fn extract_bash_command(command: &[String]) -> Option<(&str, &str)> { + let [shell, flag, script] = command else { + return None; + }; + if !matches!(flag.as_str(), "-lc" | "-c") || !is_supported_posix_shell(shell) { + return None; + } + Some((shell, script)) +} + +fn is_supported_posix_shell(shell: &str) -> bool { + Path::new(shell) + .file_stem() + .and_then(|stem| stem.to_str()) + .is_some_and(|name| matches!(name, "bash" | "zsh" | "sh")) +} + +/// Returns the sequence of plain commands within a `bash -lc "..."` or +/// `zsh -lc "..."` invocation when the script only contains word-only commands +/// joined by safe operators. +pub fn parse_shell_lc_plain_commands(command: &[String]) -> Option>> { + let (_, script) = extract_bash_command(command)?; + + let tree = try_parse_shell(script)?; + try_parse_word_only_commands_sequence(&tree, script) +} + +fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option> { + if cmd.kind() != "command" { + return None; + } + let mut words = Vec::new(); + let mut cursor = cmd.walk(); + for child in cmd.named_children(&mut cursor) { + match child.kind() { + "command_name" => { + let word_node = child.named_child(0)?; + if word_node.kind() != "word" { + return None; + } + words.push(word_node.utf8_text(src.as_bytes()).ok()?.to_owned()); + } + "word" | "number" => { + words.push(child.utf8_text(src.as_bytes()).ok()?.to_owned()); + } + "string" => { + let parsed = parse_double_quoted_string(child, src)?; + words.push(parsed); + } + "raw_string" => { + let parsed = parse_raw_string(child, src)?; + words.push(parsed); + } + "concatenation" => { + // Handle concatenated arguments like -g"*.py" + let mut concatenated = String::new(); + let mut concat_cursor = child.walk(); + for part in child.named_children(&mut concat_cursor) { + match part.kind() { + "word" | "number" => { + concatenated + .push_str(part.utf8_text(src.as_bytes()).ok()?.to_owned().as_str()); + } + "string" => { + let parsed = parse_double_quoted_string(part, src)?; + concatenated.push_str(&parsed); + } + "raw_string" => { + let parsed = parse_raw_string(part, src)?; + concatenated.push_str(&parsed); + } + _ => return None, + } + } + if concatenated.is_empty() { + return None; + } + words.push(concatenated); + } + _ => return None, + } + } + Some(words) +} + +fn parse_double_quoted_string(node: Node, src: &str) -> Option { + if node.kind() != "string" { + return None; + } + + let mut cursor = node.walk(); + for part in node.named_children(&mut cursor) { + if part.kind() != "string_content" { + return None; + } + } + let raw = node.utf8_text(src.as_bytes()).ok()?; + let stripped = raw + .strip_prefix('"') + .and_then(|text| text.strip_suffix('"'))?; + Some(stripped.to_string()) +} + +fn parse_raw_string(node: Node, src: &str) -> Option { + if node.kind() != "raw_string" { + return None; + } + + let raw_string = node.utf8_text(src.as_bytes()).ok()?; + let stripped = raw_string + .strip_prefix('\'') + .and_then(|s| s.strip_suffix('\'')); + stripped.map(str::to_owned) +} diff --git a/codex-rs/core/src/command_safety/is_dangerous_command.rs b/codex-rs/command-safety/src/is_dangerous_command.rs similarity index 99% rename from codex-rs/core/src/command_safety/is_dangerous_command.rs rename to codex-rs/command-safety/src/is_dangerous_command.rs index 3e2c669c44..993f05c6ad 100644 --- a/codex-rs/core/src/command_safety/is_dangerous_command.rs +++ b/codex-rs/command-safety/src/is_dangerous_command.rs @@ -1,4 +1,4 @@ -use crate::bash::parse_shell_lc_plain_commands; +use crate::bash_parse::parse_shell_lc_plain_commands; #[cfg(windows)] #[path = "windows_dangerous_commands.rs"] mod windows_dangerous_commands; diff --git a/codex-rs/core/src/command_safety/is_safe_command.rs b/codex-rs/command-safety/src/is_safe_command.rs similarity index 98% rename from codex-rs/core/src/command_safety/is_safe_command.rs rename to codex-rs/command-safety/src/is_safe_command.rs index e52079c74b..69ac1992bd 100644 --- a/codex-rs/core/src/command_safety/is_safe_command.rs +++ b/codex-rs/command-safety/src/is_safe_command.rs @@ -1,9 +1,9 @@ -use crate::bash::parse_shell_lc_plain_commands; +use crate::bash_parse::parse_shell_lc_plain_commands; // Find the first matching git subcommand, skipping known global options that // may appear before it (e.g., `-C`, `-c`, `--git-dir`). // Implemented in `is_dangerous_command` and shared here. -use crate::command_safety::is_dangerous_command::find_git_subcommand; -use crate::command_safety::windows_safe_commands::is_safe_command_windows; +use crate::is_dangerous_command::find_git_subcommand; +use crate::windows_safe_commands::is_safe_command_windows; pub fn is_known_safe_command(command: &[String]) -> bool { let command: Vec = command diff --git a/codex-rs/command-safety/src/lib.rs b/codex-rs/command-safety/src/lib.rs new file mode 100644 index 0000000000..5409fad88d --- /dev/null +++ b/codex-rs/command-safety/src/lib.rs @@ -0,0 +1,8 @@ +pub mod is_dangerous_command; +pub mod is_safe_command; +pub mod windows_safe_commands; + +mod bash_parse; +#[cfg(windows)] +#[path = "windows_dangerous_commands.rs"] +mod windows_dangerous_commands; diff --git a/codex-rs/core/src/command_safety/powershell_parser.ps1 b/codex-rs/command-safety/src/powershell_parser.ps1 similarity index 100% rename from codex-rs/core/src/command_safety/powershell_parser.ps1 rename to codex-rs/command-safety/src/powershell_parser.ps1 diff --git a/codex-rs/core/src/command_safety/windows_dangerous_commands.rs b/codex-rs/command-safety/src/windows_dangerous_commands.rs similarity index 100% rename from codex-rs/core/src/command_safety/windows_dangerous_commands.rs rename to codex-rs/command-safety/src/windows_dangerous_commands.rs diff --git a/codex-rs/core/src/command_safety/windows_safe_commands.rs b/codex-rs/command-safety/src/windows_safe_commands.rs similarity index 100% rename from codex-rs/core/src/command_safety/windows_safe_commands.rs rename to codex-rs/command-safety/src/windows_safe_commands.rs diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 8b9f76a681..d545643177 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -33,6 +33,7 @@ codex-app-server-protocol = { workspace = true } codex-apply-patch = { workspace = true } codex-async-utils = { workspace = true } codex-client = { workspace = true } +codex-command-safety = { workspace = true } codex-execpolicy = { workspace = true } codex-file-search = { workspace = true } codex-git = { workspace = true } diff --git a/codex-rs/core/src/command_safety/mod.rs b/codex-rs/core/src/command_safety/mod.rs deleted file mode 100644 index caf5c9f6ea..0000000000 --- a/codex-rs/core/src/command_safety/mod.rs +++ /dev/null @@ -1,3 +0,0 @@ -pub mod is_dangerous_command; -pub mod is_safe_command; -pub mod windows_safe_commands; diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 5e73fb0735..327904b0f6 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -7,8 +7,8 @@ use arc_swap::ArcSwap; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigLayerStackOrdering; -use crate::is_dangerous_command::command_might_be_dangerous; -use crate::is_safe_command::is_known_safe_command; +use codex_command_safety::is_dangerous_command::command_might_be_dangerous; +use codex_command_safety::is_safe_command::is_known_safe_command; use codex_execpolicy::AmendError; use codex_execpolicy::Decision; use codex_execpolicy::Error as ExecPolicyRuleError; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 883c89682c..5f10d62e63 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -21,7 +21,6 @@ pub use codex_thread::CodexThread; pub use codex_thread::ThreadConfigSnapshot; mod agent; mod codex_delegate; -mod command_safety; pub mod config; pub mod config_loader; pub mod connectors; @@ -135,8 +134,6 @@ pub mod util; pub use apply_patch::CODEX_APPLY_PATCH_ARG1; pub use client::X_CODEX_TURN_METADATA_HEADER; -pub use command_safety::is_dangerous_command; -pub use command_safety::is_safe_command; pub use exec_policy::ExecPolicyError; pub use exec_policy::check_execpolicy_for_warnings; pub use exec_policy::load_exec_policy; diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index b9e2a97d6d..ce15ae440c 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -10,7 +10,6 @@ use crate::exec::ExecParams; use crate::exec_env::create_env; use crate::exec_policy::ExecApprovalRequest; use crate::function_tool::FunctionCallError; -use crate::is_safe_command::is_known_safe_command; use crate::protocol::ExecCommandSource; use crate::shell::Shell; use crate::tools::context::ToolInvocation; @@ -26,6 +25,7 @@ use crate::tools::registry::ToolKind; use crate::tools::runtimes::shell::ShellRequest; use crate::tools::runtimes::shell::ShellRuntime; use crate::tools::sandboxing::ToolCtx; +use codex_command_safety::is_safe_command::is_known_safe_command; pub struct ShellHandler; @@ -348,7 +348,6 @@ mod tests { use crate::codex::make_session_and_context; use crate::exec_env::create_env; - use crate::is_safe_command::is_known_safe_command; use crate::powershell::try_find_powershell_executable_blocking; use crate::powershell::try_find_pwsh_executable_blocking; use crate::sandboxing::SandboxPermissions; @@ -356,6 +355,7 @@ mod tests { use crate::shell::ShellType; use crate::shell_snapshot::ShellSnapshot; use crate::tools::handlers::ShellCommandHandler; + use codex_command_safety::is_safe_command::is_known_safe_command; use tokio::sync::watch; /// The logic for is_known_safe_command() has heuristics for known shells, diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index c06889b37b..74bdde4b23 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -1,5 +1,4 @@ use crate::function_tool::FunctionCallError; -use crate::is_safe_command::is_known_safe_command; use crate::protocol::EventMsg; use crate::protocol::TerminalInteractionEvent; use crate::sandboxing::SandboxPermissions; @@ -18,6 +17,7 @@ use crate::unified_exec::UnifiedExecProcessManager; use crate::unified_exec::UnifiedExecResponse; use crate::unified_exec::WriteStdinRequest; use async_trait::async_trait; +use codex_command_safety::is_safe_command::is_known_safe_command; use codex_protocol::models::FunctionCallOutputBody; use serde::Deserialize; use std::path::PathBuf; diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 9346db63c1..a1bcad88a3 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -23,6 +23,7 @@ workspace = true anyhow = { workspace = true } async-trait = { workspace = true } clap = { workspace = true, features = ["derive"] } +codex-command-safety = { workspace = true } codex-core = { workspace = true } codex-execpolicy = { workspace = true } libc = { workspace = true } diff --git a/codex-rs/exec-server/src/posix.rs b/codex-rs/exec-server/src/posix.rs index 7f9ce569c6..5be747e364 100644 --- a/codex-rs/exec-server/src/posix.rs +++ b/codex-rs/exec-server/src/posix.rs @@ -61,8 +61,8 @@ use std::sync::Arc; use anyhow::Context as _; use clap::Parser; +use codex_command_safety::is_dangerous_command::command_might_be_dangerous; use codex_core::config::find_codex_home; -use codex_core::is_dangerous_command::command_might_be_dangerous; use codex_core::sandboxing::SandboxPermissions; use codex_execpolicy::Decision; use codex_execpolicy::Policy;