diff --git a/codex-rs/app-server/tests/suite/send_message.rs b/codex-rs/app-server/tests/suite/send_message.rs index d5e015412..25dad0e0a 100644 --- a/codex-rs/app-server/tests/suite/send_message.rs +++ b/codex-rs/app-server/tests/suite/send_message.rs @@ -477,7 +477,6 @@ fn assert_permissions_message(item: &ResponseItem) { &SandboxPolicy::DangerFullAccess, AskForApproval::Never, &Policy::empty(), - false, &PathBuf::from("/tmp"), ) .into_text(); diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 05c300dc8..1341e6341 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2004,7 +2004,6 @@ impl Session { &next.sandbox_policy, next.approval_policy, self.services.exec_policy.current().as_ref(), - self.features.enabled(Feature::RequestRule), &next.cwd, ) .into(), @@ -2624,7 +2623,6 @@ impl Session { &turn_context.sandbox_policy, turn_context.approval_policy, self.services.exec_policy.current().as_ref(), - self.features.enabled(Feature::RequestRule), &turn_context.cwd, ) .into(), diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 298bc2679..c20471047 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -686,8 +686,6 @@ mod tests { use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigRequirements; use crate::config_loader::ConfigRequirementsToml; - use crate::features::Feature; - use crate::features::Features; use codex_app_server_protocol::ConfigLayerSource; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; @@ -1281,8 +1279,6 @@ prefix_rule( "cargo-insta".to_string(), ]; let manager = ExecPolicyManager::default(); - let mut features = Features::with_defaults(); - features.enable(Feature::RequestRule); let requirement = manager .create_exec_approval_requirement_for_command(ExecApprovalRequest { diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 00e38223b..c7b119aa3 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -511,8 +511,8 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::RequestRule, key: "request_rule", - stage: Stage::Stable, - default_enabled: true, + stage: Stage::Removed, + default_enabled: false, }, FeatureSpec { id: Feature::WindowsSandbox, diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 9093dfba4..31bf3f129 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -243,14 +243,6 @@ impl ShellHandler { freeform, } = args; - let features = session.features(); - let request_rule_enabled = features.enabled(crate::features::Feature::RequestRule); - let prefix_rule = if request_rule_enabled { - prefix_rule - } else { - None - }; - let mut exec_params = exec_params; let dependency_env = session.dependency_env().await; if !dependency_env.is_empty() { diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index c06889b37..e689e5bba 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -142,14 +142,6 @@ impl ToolHandler for UnifiedExecHandler { .. } = args; - let features = session.features(); - let request_rule_enabled = features.enabled(crate::features::Feature::RequestRule); - let prefix_rule = if request_rule_enabled { - prefix_rule - } else { - None - }; - if sandbox_permissions.requires_escalated_permissions() && !matches!( context.turn.approval_policy, diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 305ef85ba..a897e5a8f 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -41,7 +41,6 @@ pub(crate) struct ToolsConfig { pub js_repl_tools_only: bool, pub collab_tools: bool, pub collaboration_modes_tools: bool, - pub request_rule_enabled: bool, pub experimental_supported_tools: Vec, } @@ -64,7 +63,6 @@ impl ToolsConfig { include_js_repl && features.enabled(Feature::JsReplToolsOnly); let include_collab_tools = features.enabled(Feature::Collab); let include_collaboration_modes_tools = features.enabled(Feature::CollaborationModes); - let request_rule_enabled = features.enabled(Feature::RequestRule); let include_search_tool = features.enabled(Feature::Apps); let shell_type = if !features.enabled(Feature::ShellTool) { @@ -101,7 +99,6 @@ impl ToolsConfig { js_repl_tools_only: include_js_repl_tools_only, collab_tools: include_collab_tools, collaboration_modes_tools: include_collaboration_modes_tools, - request_rule_enabled, experimental_supported_tools: model_info.experimental_supported_tools.clone(), } } @@ -174,7 +171,7 @@ impl From for AdditionalProperties { } } -fn create_approval_parameters(include_prefix_rule: bool) -> BTreeMap { +fn create_approval_parameters() -> BTreeMap { let mut properties = BTreeMap::from([ ( "sandbox_permissions".to_string(), @@ -200,23 +197,22 @@ fn create_approval_parameters(include_prefix_rule: bool) -> BTreeMap ToolSpec { +fn create_exec_command_tool() -> ToolSpec { let mut properties = BTreeMap::from([ ( "cmd".to_string(), @@ -274,7 +270,7 @@ fn create_exec_command_tool(include_prefix_rule: bool) -> ToolSpec { }, ), ]); - properties.extend(create_approval_parameters(include_prefix_rule)); + properties.extend(create_approval_parameters()); ToolSpec::Function(ResponsesApiTool { name: "exec_command".to_string(), @@ -337,7 +333,7 @@ fn create_write_stdin_tool() -> ToolSpec { }) } -fn create_shell_tool(include_prefix_rule: bool) -> ToolSpec { +fn create_shell_tool() -> ToolSpec { let mut properties = BTreeMap::from([ ( "command".to_string(), @@ -359,7 +355,7 @@ fn create_shell_tool(include_prefix_rule: bool) -> ToolSpec { }, ), ]); - properties.extend(create_approval_parameters(include_prefix_rule)); + properties.extend(create_approval_parameters()); let description = if cfg!(windows) { r#"Runs a Powershell command (Windows) and returns its output. Arguments to `shell` will be passed to CreateProcessW(). Most commands should be prefixed with ["powershell.exe", "-Command"]. @@ -390,7 +386,7 @@ Examples of valid command strings: }) } -fn create_shell_command_tool(include_prefix_rule: bool) -> ToolSpec { +fn create_shell_command_tool() -> ToolSpec { let mut properties = BTreeMap::from([ ( "command".to_string(), @@ -422,7 +418,7 @@ fn create_shell_command_tool(include_prefix_rule: bool) -> ToolSpec { }, ), ]); - properties.extend(create_approval_parameters(include_prefix_rule)); + properties.extend(create_approval_parameters()); let description = if cfg!(windows) { r#"Runs a Powershell command (Windows) and returns its output. @@ -1442,19 +1438,13 @@ pub(crate) fn build_specs( match &config.shell_type { ConfigShellToolType::Default => { - builder.push_spec_with_parallel_support( - create_shell_tool(config.request_rule_enabled), - true, - ); + builder.push_spec_with_parallel_support(create_shell_tool(), true); } ConfigShellToolType::Local => { builder.push_spec_with_parallel_support(ToolSpec::LocalShell {}, true); } ConfigShellToolType::UnifiedExec => { - builder.push_spec_with_parallel_support( - create_exec_command_tool(config.request_rule_enabled), - true, - ); + builder.push_spec_with_parallel_support(create_exec_command_tool(), true); builder.push_spec(create_write_stdin_tool()); builder.register_handler("exec_command", unified_exec_handler.clone()); builder.register_handler("write_stdin", unified_exec_handler); @@ -1463,10 +1453,7 @@ pub(crate) fn build_specs( // Do nothing. } ConfigShellToolType::ShellCommand => { - builder.push_spec_with_parallel_support( - create_shell_command_tool(config.request_rule_enabled), - true, - ); + builder.push_spec_with_parallel_support(create_shell_command_tool(), true); } } @@ -1832,7 +1819,7 @@ mod tests { // Build expected from the same helpers used by the builder. let mut expected: BTreeMap = BTreeMap::from([]); for spec in [ - create_exec_command_tool(true), + create_exec_command_tool(), create_write_stdin_tool(), PLAN_TOOL.clone(), create_request_user_input_tool(), @@ -2815,7 +2802,7 @@ mod tests { #[test] fn test_shell_tool() { - let tool = super::create_shell_tool(true); + let tool = super::create_shell_tool(); let ToolSpec::Function(ResponsesApiTool { description, name, .. }) = &tool @@ -2845,7 +2832,7 @@ Examples of valid command strings: #[test] fn test_shell_command_tool() { - let tool = super::create_shell_command_tool(true); + let tool = super::create_shell_command_tool(); let ToolSpec::Function(ResponsesApiTool { description, name, .. }) = &tool diff --git a/codex-rs/core/tests/suite/permissions_messages.rs b/codex-rs/core/tests/suite/permissions_messages.rs index d27203606..5db3bf9f0 100644 --- a/codex-rs/core/tests/suite/permissions_messages.rs +++ b/codex-rs/core/tests/suite/permissions_messages.rs @@ -488,7 +488,6 @@ async fn permissions_message_includes_writable_roots() -> Result<()> { &sandbox_policy, AskForApproval::OnRequest, &Policy::empty(), - true, test.config.cwd.as_path(), ) .into_text(); diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index eba748f2e..3a98aa539 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -225,8 +225,6 @@ const APPROVAL_POLICY_UNLESS_TRUSTED: &str = include_str!("prompts/permissions/approval_policy/unless_trusted.md"); const APPROVAL_POLICY_ON_FAILURE: &str = include_str!("prompts/permissions/approval_policy/on_failure.md"); -const APPROVAL_POLICY_ON_REQUEST: &str = - include_str!("prompts/permissions/approval_policy/on_request.md"); const APPROVAL_POLICY_ON_REQUEST_RULE: &str = include_str!("prompts/permissions/approval_policy/on_request_rule.md"); @@ -241,29 +239,20 @@ impl DeveloperInstructions { Self { text: text.into() } } - pub fn from( - approval_policy: AskForApproval, - exec_policy: &Policy, - request_rule_enabled: bool, - ) -> DeveloperInstructions { + pub fn from(approval_policy: AskForApproval, exec_policy: &Policy) -> DeveloperInstructions { let text = match approval_policy { AskForApproval::Never => APPROVAL_POLICY_NEVER.to_string(), AskForApproval::UnlessTrusted => APPROVAL_POLICY_UNLESS_TRUSTED.to_string(), AskForApproval::OnFailure => APPROVAL_POLICY_ON_FAILURE.to_string(), AskForApproval::OnRequest => { - if !request_rule_enabled { - APPROVAL_POLICY_ON_REQUEST.to_string() - } else { - let command_prefixes = - format_allow_prefixes(exec_policy.get_allowed_prefixes()); - match command_prefixes { - Some(prefixes) => { - format!( - "{APPROVAL_POLICY_ON_REQUEST_RULE}\n## Approved command prefixes\nThe following prefix rules have already been approved: {prefixes}" - ) - } - None => APPROVAL_POLICY_ON_REQUEST_RULE.to_string(), + let command_prefixes = format_allow_prefixes(exec_policy.get_allowed_prefixes()); + match command_prefixes { + Some(prefixes) => { + format!( + "{APPROVAL_POLICY_ON_REQUEST_RULE}\n## Approved command prefixes\nThe following prefix rules have already been approved: {prefixes}" + ) } + None => APPROVAL_POLICY_ON_REQUEST_RULE.to_string(), } } }; @@ -301,7 +290,6 @@ impl DeveloperInstructions { sandbox_policy: &SandboxPolicy, approval_policy: AskForApproval, exec_policy: &Policy, - request_rule_enabled: bool, cwd: &Path, ) -> Self { let network_access = if sandbox_policy.has_full_network_access() { @@ -325,7 +313,6 @@ impl DeveloperInstructions { network_access, approval_policy, exec_policy, - request_rule_enabled, writable_roots, ) } @@ -349,7 +336,6 @@ impl DeveloperInstructions { network_access: NetworkAccess, approval_policy: AskForApproval, exec_policy: &Policy, - request_rule_enabled: bool, writable_roots: Option>, ) -> Self { let start_tag = DeveloperInstructions::new(""); @@ -359,11 +345,7 @@ impl DeveloperInstructions { sandbox_mode, network_access, )) - .concat(DeveloperInstructions::from( - approval_policy, - exec_policy, - request_rule_enabled, - )) + .concat(DeveloperInstructions::from(approval_policy, exec_policy)) .concat(DeveloperInstructions::from_writable_roots(writable_roots)) .concat(end_tag) } @@ -1207,7 +1189,6 @@ mod tests { NetworkAccess::Enabled, AskForApproval::OnRequest, &Policy::empty(), - false, None, ); @@ -1217,7 +1198,7 @@ mod tests { "expected network access to be enabled in message" ); assert!( - text.contains("`approval_policy` is `on-request`"), + text.contains("How to request escalation"), "expected approval guidance to be included" ); } @@ -1236,7 +1217,6 @@ mod tests { &policy, AskForApproval::UnlessTrusted, &Policy::empty(), - false, &PathBuf::from("/tmp"), ); let text = instructions.into_text(); @@ -1245,7 +1225,7 @@ mod tests { } #[test] - fn includes_request_rule_instructions_when_enabled() { + fn includes_request_rule_instructions_for_on_request() { let mut exec_policy = Policy::empty(); exec_policy .add_prefix_rule( @@ -1258,7 +1238,6 @@ mod tests { NetworkAccess::Enabled, AskForApproval::OnRequest, &exec_policy, - true, None, ); diff --git a/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request.md b/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request.md deleted file mode 100644 index 16c90e801..000000000 --- a/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request.md +++ /dev/null @@ -1,12 +0,0 @@ - Approvals are your mechanism to get user consent to run shell commands without the sandbox. `approval_policy` is `on-request`: Commands will be run in the sandbox by default, and you can specify in your tool call if you want to escalate a command to run without sandboxing. If the completing the task requires escalated permissions, Do not let these settings or the sandbox deter you from attempting to accomplish the user's task. - -Here are scenarios where you'll need to request approval: -- You need to run a command that writes to a directory that requires it (e.g. running tests that write to /var) -- You need to run a GUI app (e.g., open/xdg-open/osascript) to open browsers or files. -- You are running sandboxed and need to run a command that requires network access (e.g. installing packages) -- If you run a command that is important to solving the user's query, but it fails because of sandboxing, rerun the command with approval. ALWAYS proceed to use the `sandbox_permissions` and `justification` parameters - do not message the user before requesting approval for the command. -- You are about to take a potentially destructive action such as an `rm` or `git reset` that the user did not explicitly ask for. - -When requesting approval to execute a command that will require escalated privileges: - - Provide the `sandbox_permissions` parameter with the value `"require_escalated"` - - Include a short, 1 sentence explanation for why you need escalated permissions in the justification parameter \ No newline at end of file