fix(core) disable command_might_be_dangerous when unsandboxed

codex-rs/core/src/exec_policy.rs

@@ -549,7 +549,7 @@ pub fn render_decision_for_unmatched_command(
 
     // On Windows, ReadOnly sandbox is not a real sandbox, so special-case it
     // here.
-    let runtime_sandbox_provides_safety =
+    let environment_lacks_sandbox_protections =
         cfg!(windows) && matches!(sandbox_policy, SandboxPolicy::ReadOnly { .. });
 
     // If the command is flagged as dangerous or we have no sandbox protection,
@@ -558,9 +558,20 @@ pub fn render_decision_for_unmatched_command(
     // We prefer to prompt the user rather than outright forbid the command,
     // but if the user has explicitly disabled prompts, we must
     // forbid the command.
-    if command_might_be_dangerous(command) || runtime_sandbox_provides_safety {
+    if command_might_be_dangerous(command) || environment_lacks_sandbox_protections {
         return match approval_policy {
-            AskForApproval::Never => Decision::Forbidden,
+            AskForApproval::Never => {
+                let sandbox_is_explicitly_disabled = matches!(
+                    sandbox_policy,
+                    SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. }
+                );
+                if sandbox_is_explicitly_disabled {
+                    // If the sandbox is explicitly disabled, we should allow the command to run
+                    Decision::Allow
+                } else {
+                    Decision::Forbidden
+                }
+            }
             AskForApproval::OnFailure
             | AskForApproval::OnRequest
             | AskForApproval::UnlessTrusted

codex-rs/core/src/exec_policy_tests.rs

@@ -81,6 +81,10 @@ fn unrestricted_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
     FileSystemSandboxPolicy::unrestricted()
 }
 
+fn external_file_system_sandbox_policy() -> FileSystemSandboxPolicy {
+    FileSystemSandboxPolicy::external_sandbox()
+}
+
 async fn test_config() -> (TempDir, Config) {
     let home = TempDir::new().expect("create temp dir");
     let config = ConfigBuilder::default()
@@ -1686,3 +1690,100 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
             (unless AskForApproval::Never is specified)."#
     );
 }
+
+#[tokio::test]
+async fn dangerous_command_allowed_when_sandbox_is_explicitly_disabled() {
+    let command = vec_str(&["rm", "-rf", "/tmp/nonexistent"]);
+    assert_exec_approval_requirement_for_command(
+        ExecApprovalRequirementScenario {
+            policy_src: None,
+            command,
+            approval_policy: AskForApproval::Never,
+            sandbox_policy: SandboxPolicy::ExternalSandbox {
+                network_access: Default::default(),
+            },
+            file_system_sandbox_policy: external_file_system_sandbox_policy(),
+            sandbox_permissions: SandboxPermissions::UseDefault,
+            prefix_rule: None,
+        },
+        ExecApprovalRequirement::Skip {
+            bypass_sandbox: false,
+            proposed_execpolicy_amendment: Some(ExecPolicyAmendment {
+                command: vec_str(&["rm", "-rf", "/tmp/nonexistent"]),
+            }),
+        },
+    )
+    .await;
+}
+
+#[tokio::test]
+async fn dangerous_command_forbidden_in_external_sandbox_when_policy_matches() {
+    let command = vec_str(&["rm", "-rf", "/tmp/nonexistent"]);
+    assert_exec_approval_requirement_for_command(
+        ExecApprovalRequirementScenario {
+            policy_src: Some("prefix_rule(pattern=['rm'], decision='prompt')".to_string()),
+            command,
+            approval_policy: AskForApproval::Never,
+            sandbox_policy: SandboxPolicy::ExternalSandbox {
+                network_access: Default::default(),
+            },
+            file_system_sandbox_policy: external_file_system_sandbox_policy(),
+            sandbox_permissions: SandboxPermissions::UseDefault,
+            prefix_rule: None,
+        },
+        ExecApprovalRequirement::Forbidden {
+            reason: "approval required by policy, but AskForApproval is set to Never".to_string(),
+        },
+    )
+    .await;
+}
+
+struct ExecApprovalRequirementScenario {
+    /// Source for the Starlark `.rules` file.
+    policy_src: Option<String>,
+    command: Vec<String>,
+    approval_policy: AskForApproval,
+    sandbox_policy: SandboxPolicy,
+    file_system_sandbox_policy: FileSystemSandboxPolicy,
+    sandbox_permissions: SandboxPermissions,
+    prefix_rule: Option<Vec<String>>,
+}
+
+async fn assert_exec_approval_requirement_for_command(
+    test: ExecApprovalRequirementScenario,
+    expected_requirement: ExecApprovalRequirement,
+) {
+    let ExecApprovalRequirementScenario {
+        policy_src,
+        command,
+        approval_policy,
+        sandbox_policy,
+        file_system_sandbox_policy,
+        sandbox_permissions,
+        prefix_rule,
+    } = test;
+
+    let policy = match policy_src {
+        Some(src) => {
+            let mut parser = PolicyParser::new();
+            parser
+                .parse("test.rules", src.as_str())
+                .expect("parse policy");
+            Arc::new(parser.build())
+        }
+        None => Arc::new(Policy::empty()),
+    };
+
+    let requirement = ExecPolicyManager::new(policy)
+        .create_exec_approval_requirement_for_command(ExecApprovalRequest {
+            command: &command,
+            approval_policy,
+            sandbox_policy: &sandbox_policy,
+            file_system_sandbox_policy: &file_system_sandbox_policy,
+            sandbox_permissions,
+            prefix_rule,
+        })
+        .await;
+
+    assert_eq!(requirement, expected_requirement);
+}