diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 51667c60132b..aafcbdd1b906 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2126,6 +2126,7 @@ dependencies = [ "codex-rmcp-client", "codex-rollout-trace", "codex-sandboxing", + "codex-shell-command", "codex-state", "codex-stdio-to-uds", "codex-terminal-detection", diff --git a/codex-rs/cli/Cargo.toml b/codex-rs/cli/Cargo.toml index cdee241b4252..8601de5851e6 100644 --- a/codex-rs/cli/Cargo.toml +++ b/codex-rs/cli/Cargo.toml @@ -44,6 +44,7 @@ codex-responses-api-proxy = { workspace = true } codex-rmcp-client = { workspace = true } codex-rollout-trace = { workspace = true } codex-sandboxing = { workspace = true } +codex-shell-command = { workspace = true } codex-state = { workspace = true } codex-stdio-to-uds = { workspace = true } codex-terminal-detection = { workspace = true } diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index c85da0f5f2d0..838cdcfcece8 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -142,6 +142,14 @@ async fn run_command_under_sandbox( // sandbox policy. In the future, we could add a CLI option to set them // separately. let sandbox_policy_cwd = cwd.clone(); + if let Some(reason) = codex_shell_command::metadata_write_forbidden_reason( + &command, + cwd.as_path(), + sandbox_policy_cwd.as_path(), + &config.permissions.file_system_sandbox_policy(), + ) { + anyhow::bail!("{reason}"); + } let env = create_env( &config.permissions.shell_environment_policy, diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index b7512b707618..22e81b4d13dc 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -35,9 +35,11 @@ use crate::tools::runtimes::shell::ShellRequest; use crate::tools::runtimes::shell::ShellRuntime; use crate::tools::runtimes::shell::ShellRuntimeBackend; use crate::tools::sandboxing::ToolCtx; +use crate::tools::sandboxing::apply_protected_metadata_write_preflight; use codex_features::Feature; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::protocol::ExecCommandSource; +use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy; use codex_shell_command::is_safe_command::is_known_safe_command; use codex_tools::ShellCommandBackendConfig; @@ -513,7 +515,11 @@ impl ShellHandler { ); emitter.begin(event_ctx).await; - let file_system_sandbox_policy = turn.file_system_sandbox_policy(); + let base_file_system_sandbox_policy = turn.file_system_sandbox_policy(); + let file_system_sandbox_policy = effective_file_system_sandbox_policy( + &base_file_system_sandbox_policy, + normalized_additional_permissions.as_ref(), + ); let exec_approval_requirement = session .services .exec_policy @@ -531,6 +537,14 @@ impl ShellHandler { prefix_rule, }) .await; + let exec_approval_requirement = apply_protected_metadata_write_preflight( + exec_approval_requirement, + effective_additional_permissions.sandbox_permissions, + &exec_params.command, + &exec_params.cwd, + turn.cwd.as_path(), + &file_system_sandbox_policy, + ); let req = ShellRequest { command: exec_params.command.clone(), diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 10c8deeb3f6a..1119a2e62ccd 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -1,3 +1,4 @@ +use crate::exec_policy::ExecApprovalRequest; use crate::function_tool::FunctionCallError; use crate::maybe_emit_implicit_skill_invocation; use crate::sandboxing::SandboxPermissions; @@ -19,6 +20,7 @@ use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; +use crate::tools::sandboxing::apply_protected_metadata_write_preflight; use crate::unified_exec::ExecCommandRequest; use crate::unified_exec::UnifiedExecContext; use crate::unified_exec::UnifiedExecError; @@ -32,6 +34,7 @@ use codex_otel::TOOL_CALL_UNIFIED_EXEC_METRIC; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::TerminalInteractionEvent; +use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy; use codex_shell_command::is_safe_command::is_known_safe_command; use codex_tools::UnifiedExecShellMode; use codex_utils_output_truncation::TruncationPolicy; @@ -330,6 +333,40 @@ impl ToolHandler for UnifiedExecHandler { }); } + let base_file_system_sandbox_policy = context.turn.file_system_sandbox_policy(); + let file_system_sandbox_policy = effective_file_system_sandbox_policy( + &base_file_system_sandbox_policy, + normalized_additional_permissions.as_ref(), + ); + let exec_approval_requirement = context + .session + .services + .exec_policy + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: context.turn.approval_policy.value(), + permission_profile: context.turn.permission_profile(), + file_system_sandbox_policy: &file_system_sandbox_policy, + sandbox_cwd: context.turn.cwd.as_path(), + sandbox_permissions: if effective_additional_permissions + .permissions_preapproved + { + SandboxPermissions::UseDefault + } else { + effective_additional_permissions.sandbox_permissions + }, + prefix_rule: prefix_rule.clone(), + }) + .await; + let exec_approval_requirement = apply_protected_metadata_write_preflight( + exec_approval_requirement, + effective_additional_permissions.sandbox_permissions, + &command, + &cwd, + context.turn.cwd.as_path(), + &file_system_sandbox_policy, + ); + emit_unified_exec_tty_metric(&turn.session_telemetry, tty); match manager .exec_command( @@ -345,10 +382,11 @@ impl ToolHandler for UnifiedExecHandler { sandbox_permissions: effective_additional_permissions .sandbox_permissions, additional_permissions: normalized_additional_permissions, + #[cfg(unix)] additional_permissions_preapproved: effective_additional_permissions .permissions_preapproved, justification, - prefix_rule, + exec_approval_requirement, }, &context, ) diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index f6b960ee1ac6..8846df4df95c 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -34,6 +34,7 @@ use serde::Serialize; use std::collections::HashMap; use std::fmt::Debug; use std::hash::Hash; +use std::path::Path; use std::sync::Arc; #[derive(Clone, Default, Debug)] @@ -194,6 +195,50 @@ impl ExecApprovalRequirement { } } +/// Applies an early UX check for protected metadata writes only to commands +/// that would otherwise run in the sandbox. Explicit sandbox-bypass approval +/// paths keep their existing approval semantics. +pub(crate) fn apply_protected_metadata_write_preflight( + exec_approval_requirement: ExecApprovalRequirement, + sandbox_permissions: SandboxPermissions, + command: &[String], + command_cwd: &Path, + sandbox_policy_cwd: &Path, + file_system_sandbox_policy: &FileSystemSandboxPolicy, +) -> ExecApprovalRequirement { + let bypasses_sandbox = matches!( + &exec_approval_requirement, + ExecApprovalRequirement::Skip { + bypass_sandbox: true, + .. + } + ); + let approval_is_for_sandbox_override = sandbox_permissions.requests_sandbox_override() + && matches!( + &exec_approval_requirement, + ExecApprovalRequirement::NeedsApproval { .. } + ); + if bypasses_sandbox + || approval_is_for_sandbox_override + || matches!( + &exec_approval_requirement, + ExecApprovalRequirement::Forbidden { .. } + ) + { + return exec_approval_requirement; + } + + codex_shell_command::metadata_write_forbidden_reason( + command, + command_cwd, + sandbox_policy_cwd, + file_system_sandbox_policy, + ) + .map_or(exec_approval_requirement, |reason| { + ExecApprovalRequirement::Forbidden { reason } + }) +} + /// - Never, OnFailure: do not ask /// - OnRequest: ask unless filesystem access is unrestricted /// - Granular: ask unless filesystem access is unrestricted, but auto-reject diff --git a/codex-rs/core/src/tools/sandboxing_tests.rs b/codex-rs/core/src/tools/sandboxing_tests.rs index 19eb7a67d967..9fc3f7cbcbdc 100644 --- a/codex-rs/core/src/tools/sandboxing_tests.rs +++ b/codex-rs/core/src/tools/sandboxing_tests.rs @@ -5,6 +5,7 @@ use codex_protocol::protocol::GranularApprovalConfig; use codex_protocol::protocol::NetworkAccess; use pretty_assertions::assert_eq; use serde_json::json; +use tempfile::tempdir; #[test] fn bash_permission_request_payload_omits_missing_description() { @@ -138,3 +139,107 @@ fn guardian_bypasses_sandbox_for_explicit_escalation_on_first_attempt() { SandboxOverride::BypassSandboxFirstAttempt ); } + +fn metadata_redirect_command(target: &str) -> Vec { + vec![ + "/bin/bash".to_string(), + "-lc".to_string(), + format!("printf pwned > {target}"), + ] +} + +fn workspace_write_file_system_sandbox_policy() -> FileSystemSandboxPolicy { + FileSystemSandboxPolicy::workspace_write(&[], false, false) +} + +#[test] +fn protected_metadata_write_preflight_forbids_only_sandboxed_skip() { + let repo = tempdir().expect("create tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create .git"); + let file_system_sandbox_policy = workspace_write_file_system_sandbox_policy(); + + let requirement = apply_protected_metadata_write_preflight( + ExecApprovalRequirement::Skip { + bypass_sandbox: false, + proposed_execpolicy_amendment: None, + }, + SandboxPermissions::UseDefault, + &metadata_redirect_command(".git/config"), + repo.path(), + repo.path(), + &file_system_sandbox_policy, + ); + + assert_eq!( + requirement, + ExecApprovalRequirement::Forbidden { + reason: "command targets protected workspace metadata path `.git`".to_string() + } + ); +} + +#[test] +fn protected_metadata_write_preflight_preserves_approval_and_bypass_paths() { + let repo = tempdir().expect("create tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create .git"); + let file_system_sandbox_policy = workspace_write_file_system_sandbox_policy(); + let command = metadata_redirect_command(".git/config"); + + let needs_approval = ExecApprovalRequirement::NeedsApproval { + reason: Some("requires approval".to_string()), + proposed_execpolicy_amendment: None, + }; + assert_eq!( + apply_protected_metadata_write_preflight( + needs_approval.clone(), + SandboxPermissions::RequireEscalated, + &command, + repo.path(), + repo.path(), + &file_system_sandbox_policy, + ), + needs_approval + ); + + let bypass_sandbox = ExecApprovalRequirement::Skip { + bypass_sandbox: true, + proposed_execpolicy_amendment: None, + }; + assert_eq!( + apply_protected_metadata_write_preflight( + bypass_sandbox.clone(), + SandboxPermissions::UseDefault, + &command, + repo.path(), + repo.path(), + &file_system_sandbox_policy, + ), + bypass_sandbox + ); +} + +#[test] +fn protected_metadata_write_preflight_forbids_sandboxed_approval_prompts() { + let repo = tempdir().expect("create tempdir"); + std::fs::create_dir(repo.path().join(".git")).expect("create .git"); + let file_system_sandbox_policy = workspace_write_file_system_sandbox_policy(); + + let requirement = apply_protected_metadata_write_preflight( + ExecApprovalRequirement::NeedsApproval { + reason: Some("dangerous command".to_string()), + proposed_execpolicy_amendment: None, + }, + SandboxPermissions::UseDefault, + &metadata_redirect_command(".git/config"), + repo.path(), + repo.path(), + &file_system_sandbox_policy, + ); + + assert_eq!( + requirement, + ExecApprovalRequirement::Forbidden { + reason: "command targets protected workspace metadata path `.git`".to_string() + } + ); +} diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index acf361f94b8c..9d523ba8a40e 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -37,6 +37,7 @@ use tokio::sync::Mutex; use crate::sandboxing::SandboxPermissions; use crate::session::session::Session; use crate::session::turn_context::TurnContext; +use crate::tools::sandboxing::ExecApprovalRequirement; mod async_watcher; mod errors; @@ -97,9 +98,10 @@ pub(crate) struct ExecCommandRequest { pub tty: bool, pub sandbox_permissions: SandboxPermissions, pub additional_permissions: Option, + #[cfg(unix)] pub additional_permissions_preapproved: bool, pub justification: Option, - pub prefix_rule: Option>, + pub exec_approval_requirement: ExecApprovalRequirement, } #[derive(Debug)] diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 76d8021d3d93..8ca4c99bc3f8 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -13,7 +13,6 @@ use tokio_util::sync::CancellationToken; use crate::exec_env::CODEX_THREAD_ID_ENV_VAR; use crate::exec_env::create_env; -use crate::exec_policy::ExecApprovalRequest; use crate::sandboxing::ExecRequest; use crate::sandboxing::ExecServerEnvConfig; use crate::tools::context::ExecCommandToolOutput; @@ -789,25 +788,6 @@ impl UnifiedExecProcessManager { self, context.turn.tools_config.unified_exec_shell_mode.clone(), ); - let file_system_sandbox_policy = context.turn.file_system_sandbox_policy(); - let exec_approval_requirement = context - .session - .services - .exec_policy - .create_exec_approval_requirement_for_command(ExecApprovalRequest { - command: &request.command, - approval_policy: context.turn.approval_policy.value(), - permission_profile: context.turn.permission_profile(), - file_system_sandbox_policy: &file_system_sandbox_policy, - sandbox_cwd: context.turn.cwd.as_path(), - sandbox_permissions: if request.additional_permissions_preapproved { - crate::sandboxing::SandboxPermissions::UseDefault - } else { - request.sandbox_permissions - }, - prefix_rule: request.prefix_rule.clone(), - }) - .await; let req = UnifiedExecToolRequest { command: request.command.clone(), hook_command: request.hook_command.clone(), @@ -823,7 +803,7 @@ impl UnifiedExecProcessManager { #[cfg(unix)] additional_permissions_preapproved: request.additional_permissions_preapproved, justification: request.justification.clone(), - exec_approval_requirement, + exec_approval_requirement: request.exec_approval_requirement.clone(), }; let tool_ctx = ToolCtx { session: context.session.clone(), diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index c1f3d0724b8a..c7f0db256c15 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -46,7 +46,8 @@ pub fn is_protected_metadata_directory_name(name: &OsStr) -> bool { /// should be blocked before execution. pub fn forbidden_agent_metadata_write( path: &Path, - cwd: &Path, + path_cwd: &Path, + policy_cwd: &Path, file_system_sandbox_policy: &FileSystemSandboxPolicy, ) -> Option<&'static str> { if !matches!( @@ -56,19 +57,19 @@ pub fn forbidden_agent_metadata_write( return None; } - let target = resolve_candidate_path(path, cwd)?; + let target = resolve_candidate_path(path, path_cwd)?; let (protected_metadata_path, metadata_name) = - metadata_child_of_writable_root(file_system_sandbox_policy, target.as_path(), cwd)?; + metadata_child_of_writable_root(file_system_sandbox_policy, target.as_path(), policy_cwd)?; if has_explicit_write_entry_for_metadata_path( file_system_sandbox_policy, &protected_metadata_path, target.as_path(), - cwd, + policy_cwd, ) { return None; } - if !file_system_sandbox_policy.can_write_path_with_cwd(target.as_path(), cwd) { + if !file_system_sandbox_policy.can_write_path_with_cwd(target.as_path(), policy_cwd) { return Some(metadata_name); } @@ -2007,6 +2008,7 @@ mod tests { forbidden_agent_metadata_write( Path::new(".git/config"), relative_cwd, + relative_cwd, &file_system_policy, ), Some(".git") diff --git a/codex-rs/shell-command/src/bash.rs b/codex-rs/shell-command/src/bash.rs index 2fe299108c20..9a10ba458be9 100644 --- a/codex-rs/shell-command/src/bash.rs +++ b/codex-rs/shell-command/src/bash.rs @@ -119,6 +119,55 @@ pub fn parse_shell_lc_plain_commands(command: &[String]) -> Option Option>> { + let (_, script) = extract_bash_command(command)?; + let tree = try_parse_shell(script)?; + let root = tree.root_node(); + if root.has_error() { + return None; + } + + let mut commands = Vec::new(); + for command_node in find_command_nodes(root) { + if let Some(words) = parse_command_word_prefix_from_node(command_node, script) + && !words.is_empty() + { + commands.push(words); + } + } + + (!commands.is_empty()).then_some(commands) +} + +/// Returns literal write redirection targets within a `bash -lc "..."` or +/// `zsh -lc "..."` invocation. +pub fn parse_shell_lc_write_redirection_targets(command: &[String]) -> Option> { + let (_, script) = extract_bash_command(command)?; + let tree = try_parse_shell(script)?; + let root = tree.root_node(); + if root.has_error() { + return None; + } + + let mut targets = Vec::new(); + for redirect_node in find_nodes_by_kind(root, "file_redirect") { + if file_redirect_uses_write_operator(redirect_node) + && let Some(target) = parse_redirection_target(redirect_node, script) + { + targets.push(target); + } + } + + (!targets.is_empty()).then_some(targets) +} + /// Returns the parsed argv for a single shell command in a here-doc style /// script (`<<`), as long as the script contains exactly one command node. pub fn parse_shell_lc_single_command_prefix(command: &[String]) -> Option> { @@ -194,6 +243,100 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option, 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 !matches!(word_node.kind(), "word" | "number") { + 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" => { + let parsed = parse_concatenation(child, src)?; + words.push(parsed); + } + "variable_assignment" | "comment" => {} + kind if is_allowed_heredoc_attachment_kind(kind) => {} + _ => {} + } + } + Some(words) +} + +fn file_redirect_uses_write_operator(node: Node<'_>) -> bool { + let mut cursor = node.walk(); + for child in node.children(&mut cursor) { + if !child.is_named() && child.kind().contains('>') { + return true; + } + } + false +} + +fn parse_redirection_target(node: Node<'_>, src: &str) -> Option { + let mut target = None; + let mut cursor = node.walk(); + for child in node.named_children(&mut cursor) { + if let Some(parsed) = parse_literal_shell_word(child, src) { + target = Some(parsed); + } + } + target +} + +fn parse_literal_shell_word(node: Node<'_>, src: &str) -> Option { + match node.kind() { + "word" | "number" => Some(node.utf8_text(src.as_bytes()).ok()?.to_owned()), + "string" => parse_double_quoted_string(node, src), + "raw_string" => parse_raw_string(node, src), + "concatenation" => parse_concatenation(node, src), + _ => None, + } +} + +fn parse_concatenation(node: Node<'_>, src: &str) -> Option { + let mut concatenated = String::new(); + let mut cursor = node.walk(); + for part in node.named_children(&mut 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; + } + Some(concatenated) +} + fn parse_heredoc_command_words(cmd: Node<'_>, src: &str) -> Option> { if cmd.kind() != "command" { return None; @@ -268,6 +411,29 @@ fn find_single_command_node(root: Node<'_>) -> Option> { single_command } +fn find_command_nodes(root: Node<'_>) -> Vec> { + let mut command_nodes = find_nodes_by_kind(root, "command"); + command_nodes.sort_by_key(Node::start_byte); + command_nodes +} + +fn find_nodes_by_kind<'a>(root: Node<'a>, kind: &str) -> Vec> { + let mut stack = vec![root]; + let mut matches = Vec::new(); + while let Some(node) = stack.pop() { + if node.kind() == kind { + matches.push(node); + } + + let mut cursor = node.walk(); + for child in node.named_children(&mut cursor) { + stack.push(child); + } + } + matches.sort_by_key(Node::start_byte); + matches +} + fn has_named_descendant_kind(node: Node<'_>, kind: &str) -> bool { let mut stack = vec![node]; while let Some(current) = stack.pop() { @@ -453,6 +619,64 @@ mod tests { assert_eq!(parsed, vec![vec!["ls".to_string()]]); } + #[test] + fn parse_shell_lc_command_word_prefixes_extracts_complex_script_commands() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + r#"set -e +top=$(git rev-parse --show-toplevel) +if git init -q; then + exit 22 +fi +if mkdir .codex; then + exit 23 +fi +printf pwned > .git/config +"# + .to_string(), + ]; + + let parsed = parse_shell_lc_command_word_prefixes(&command).expect("parse script"); + + assert!(parsed.contains(&vec![ + "git".to_string(), + "rev-parse".to_string(), + "--show-toplevel".to_string() + ])); + assert!(parsed.contains(&vec![ + "git".to_string(), + "init".to_string(), + "-q".to_string() + ])); + assert!(parsed.contains(&vec!["mkdir".to_string(), ".codex".to_string()])); + } + + #[test] + fn parse_shell_lc_write_redirection_targets_extracts_literal_writes() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + r#"printf pwned > .git +cat < .git/config +printf ok >> .codex/log +printf ok > ".agents/config" +"# + .to_string(), + ]; + + let parsed = parse_shell_lc_write_redirection_targets(&command).expect("parse redirects"); + + assert_eq!( + parsed, + vec![ + ".git".to_string(), + ".codex/log".to_string(), + ".agents/config".to_string() + ] + ); + } + #[test] fn accepts_concatenated_flag_and_value() { // Test case: -g"*.py" (flag directly concatenated with quoted value) diff --git a/codex-rs/shell-command/src/lib.rs b/codex-rs/shell-command/src/lib.rs index 1d9e302a4e70..7311f22c2814 100644 --- a/codex-rs/shell-command/src/lib.rs +++ b/codex-rs/shell-command/src/lib.rs @@ -4,8 +4,10 @@ mod shell_detect; pub mod bash; pub(crate) mod command_safety; +mod metadata_write; pub mod parse_command; pub mod powershell; pub use command_safety::is_dangerous_command; pub use command_safety::is_safe_command; +pub use metadata_write::metadata_write_forbidden_reason; diff --git a/codex-rs/shell-command/src/metadata_write.rs b/codex-rs/shell-command/src/metadata_write.rs new file mode 100644 index 000000000000..8b819de0eb02 --- /dev/null +++ b/codex-rs/shell-command/src/metadata_write.rs @@ -0,0 +1,192 @@ +use std::path::Path; + +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::forbidden_agent_metadata_write; + +pub fn metadata_write_forbidden_reason( + command: &[String], + command_cwd: &Path, + sandbox_policy_cwd: &Path, + file_system_sandbox_policy: &FileSystemSandboxPolicy, +) -> Option { + if let Some(targets) = crate::bash::parse_shell_lc_write_redirection_targets(command) { + for target in targets { + if let Some(name) = forbidden_agent_metadata_write( + Path::new(&target), + command_cwd, + sandbox_policy_cwd, + file_system_sandbox_policy, + ) { + return Some(metadata_write_reason(name)); + } + } + } + None +} + +fn metadata_write_reason(name: &str) -> String { + format!("command targets protected workspace metadata path `{name}`") +} + +#[cfg(test)] +mod tests { + use std::path::Path; + use std::path::PathBuf; + + use codex_protocol::permissions::FileSystemAccessMode; + use codex_protocol::permissions::FileSystemPath; + use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSandboxPolicy; + use pretty_assertions::assert_eq; + + use super::metadata_write_forbidden_reason; + + struct TestDir { + path: PathBuf, + } + + impl TestDir { + fn new(name: &str) -> Self { + let path = std::env::temp_dir().join(format!( + "codex-metadata-write-{name}-{}", + std::process::id() + )); + let _ = std::fs::remove_dir_all(&path); + std::fs::create_dir(&path).expect("create tempdir"); + Self { path } + } + + fn path(&self) -> &Path { + &self.path + } + } + + impl Drop for TestDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.path); + } + } + + fn workspace_write_policy() -> FileSystemSandboxPolicy { + FileSystemSandboxPolicy::workspace_write(&[], false, false) + } + + #[test] + fn metadata_write_detector_allows_normal_git_under_parent_repo() { + let repo = TestDir::new("normal-git-under-parent-repo"); + std::fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + std::fs::create_dir(&cwd).expect("create cwd"); + let policy = workspace_write_policy(); + + let reason = metadata_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "git status --short".to_string(), + ], + &cwd, + &cwd, + &policy, + ); + + assert_eq!(reason, None); + } + + #[test] + fn metadata_write_detector_leaves_direct_writes_to_sandbox_policy() { + let cwd = TestDir::new("direct-metadata-writes"); + let policy = workspace_write_policy(); + + let reason = metadata_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "touch .git && mkdir -p .codex".to_string(), + ], + cwd.path(), + cwd.path(), + &policy, + ); + + assert_eq!(reason, None); + } + + #[test] + fn metadata_write_detector_blocks_metadata_redirections() { + let repo = TestDir::new("metadata-write-redirections"); + std::fs::create_dir(repo.path().join(".git")).expect("create parent .git"); + let cwd = repo.path().join("sub"); + std::fs::create_dir(&cwd).expect("create cwd"); + let policy = workspace_write_policy(); + + let reason = metadata_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "printf pwned > .git".to_string(), + ], + &cwd, + &cwd, + &policy, + ); + + assert_eq!( + reason, + Some("command targets protected workspace metadata path `.git`".to_string()) + ); + } + + #[test] + fn metadata_write_detector_resolves_targets_against_command_cwd() { + let repo = TestDir::new("metadata-write-command-cwd"); + let command_cwd = repo.path().join("sub"); + std::fs::create_dir(&command_cwd).expect("create command cwd"); + let policy = workspace_write_policy(); + + let reason = metadata_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "printf pwned > ../.codex/config.toml".to_string(), + ], + &command_cwd, + repo.path(), + &policy, + ); + + assert_eq!( + reason, + Some("command targets protected workspace metadata path `.codex`".to_string()) + ); + } + + #[test] + fn metadata_write_detector_honors_explicit_metadata_write_entry() { + let repo = TestDir::new("metadata-write-explicit-entry"); + let mut policy = workspace_write_policy(); + policy.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: repo + .path() + .join(".codex") + .try_into() + .expect("absolute path"), + }, + access: FileSystemAccessMode::Write, + }); + + let reason = metadata_write_forbidden_reason( + &[ + "/bin/bash".to_string(), + "-lc".to_string(), + "printf pwned > .codex/config.toml".to_string(), + ], + repo.path(), + repo.path(), + &policy, + ); + + assert_eq!(reason, None); + } +}