Merge branch 'codex/permission-request-hook-suggestions' into codex/permission-request-updated-permissions

codex-rs/core/src/tools/runtimes/shell.rs

@@ -30,7 +30,7 @@ use crate::tools::sandboxing::Sandboxable;
 use crate::tools::sandboxing::ToolCtx;
 use crate::tools::sandboxing::ToolError;
 use crate::tools::sandboxing::ToolRuntime;
-use crate::tools::sandboxing::exec_policy_permission_suggestions;
+use crate::tools::sandboxing::approval_permission_suggestions;
 use crate::tools::sandboxing::sandbox_override_for_first_attempt;
 use crate::tools::sandboxing::with_cached_approval;
 use codex_hooks::PermissionSuggestionDestination;
@@ -204,11 +204,13 @@ impl Approvable<ShellRequest> for ShellRuntime {
     fn permission_request_payload(
         &self,
         req: &ShellRequest,
-        _approval_ctx: &ApprovalCtx<'_>,
+        approval_ctx: &ApprovalCtx<'_>,
     ) -> Option<PermissionRequestPayload> {
-        let permission_suggestions = exec_policy_permission_suggestions(
+        let permission_suggestions = approval_permission_suggestions(
+            approval_ctx.network_approval_context.as_ref(),
             req.exec_approval_requirement
                 .proposed_execpolicy_amendment(),
+            req.additional_permissions.as_ref(),
             &[PermissionSuggestionDestination::UserSettings],
         );
         Some(PermissionRequestPayload {

codex-rs/core/src/tools/runtimes/unified_exec.rs

@@ -27,7 +27,7 @@ use crate::tools::sandboxing::Sandboxable;
 use crate::tools::sandboxing::ToolCtx;
 use crate::tools::sandboxing::ToolError;
 use crate::tools::sandboxing::ToolRuntime;
-use crate::tools::sandboxing::exec_policy_permission_suggestions;
+use crate::tools::sandboxing::approval_permission_suggestions;
 use crate::tools::sandboxing::sandbox_override_for_first_attempt;
 use crate::tools::sandboxing::with_cached_approval;
 use crate::unified_exec::NoopSpawnLifecycle;
@@ -184,11 +184,13 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
     fn permission_request_payload(
         &self,
         req: &UnifiedExecRequest,
-        _approval_ctx: &ApprovalCtx<'_>,
+        approval_ctx: &ApprovalCtx<'_>,
     ) -> Option<PermissionRequestPayload> {
-        let permission_suggestions = exec_policy_permission_suggestions(
+        let permission_suggestions = approval_permission_suggestions(
+            approval_ctx.network_approval_context.as_ref(),
             req.exec_approval_requirement
                 .proposed_execpolicy_amendment(),
+            req.additional_permissions.as_ref(),
             &[PermissionSuggestionDestination::UserSettings],
         );
         Some(PermissionRequestPayload {

codex-rs/core/src/tools/sandboxing.rs

@@ -19,10 +19,12 @@ use codex_network_proxy::NetworkProxy;
 use codex_protocol::approvals::ExecPolicyAmendment;
 use codex_protocol::approvals::NetworkApprovalContext;
 use codex_protocol::error::CodexErr;
+use codex_protocol::models::PermissionProfile;
 use codex_protocol::permissions::FileSystemSandboxKind;
 use codex_protocol::permissions::FileSystemSandboxPolicy;
 use codex_protocol::permissions::NetworkSandboxPolicy;
 use codex_protocol::protocol::AskForApproval;
+use codex_protocol::protocol::ExecApprovalRequestEvent;
 use codex_protocol::protocol::ReviewDecision;
 #[cfg(test)]
 use codex_protocol::protocol::SandboxPolicy;
@@ -166,6 +168,29 @@ pub(crate) fn exec_policy_permission_suggestions(
         .collect()
 }
 
+pub(crate) fn approval_permission_suggestions(
+    network_approval_context: Option<&NetworkApprovalContext>,
+    proposed_execpolicy_amendment: Option<&ExecPolicyAmendment>,
+    additional_permissions: Option<&PermissionProfile>,
+    destinations: &[PermissionSuggestionDestination],
+) -> Vec<PermissionSuggestion> {
+    let available_decisions = ExecApprovalRequestEvent::default_available_decisions(
+        network_approval_context,
+        proposed_execpolicy_amendment,
+        /*proposed_network_policy_amendments*/ None,
+        additional_permissions,
+    );
+    let allows_execpolicy_amendment = available_decisions
+        .iter()
+        .any(|decision| matches!(decision, ReviewDecision::ApprovedExecpolicyAmendment { .. }));
+
+    if allows_execpolicy_amendment {
+        exec_policy_permission_suggestions(proposed_execpolicy_amendment, destinations)
+    } else {
+        Vec::new()
+    }
+}
+
 // Specifies what tool orchestrator should do with a given tool call.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub(crate) enum ExecApprovalRequirement {

codex-rs/core/src/tools/sandboxing_tests.rs

@@ -5,8 +5,13 @@ use codex_hooks::PermissionSuggestionDestination;
 use codex_hooks::PermissionSuggestionRule;
 use codex_hooks::PermissionSuggestionType;
 use codex_protocol::approvals::ExecPolicyAmendment;
+use codex_protocol::approvals::NetworkApprovalContext;
+use codex_protocol::approvals::NetworkApprovalProtocol;
+use codex_protocol::models::FileSystemPermissions;
+use codex_protocol::models::PermissionProfile;
 use codex_protocol::protocol::GranularApprovalConfig;
 use codex_protocol::protocol::NetworkAccess;
+use codex_utils_absolute_path::AbsolutePathBuf;
 use pretty_assertions::assert_eq;
 
 #[test]
@@ -116,12 +121,14 @@ fn guardian_bypasses_sandbox_for_explicit_escalation_on_first_attempt() {
 
 #[test]
 fn command_approval_execpolicy_amendment_maps_to_user_settings_suggestion() {
-    let suggestions = exec_policy_permission_suggestions(
+    let suggestions = approval_permission_suggestions(
+        /*network_approval_context*/ None,
         Some(&ExecPolicyAmendment::new(vec![
             "rm".to_string(),
             "-rf".to_string(),
             "node_modules".to_string(),
         ])),
+        /*additional_permissions*/ None,
         &[PermissionSuggestionDestination::UserSettings],
     );
 
@@ -141,3 +148,45 @@ fn command_approval_execpolicy_amendment_maps_to_user_settings_suggestion() {
         }]
     );
 }
+
+#[test]
+fn command_approval_with_additional_permissions_has_no_persistent_suggestions() {
+    let suggestions = approval_permission_suggestions(
+        /*network_approval_context*/ None,
+        Some(&ExecPolicyAmendment::new(vec![
+            "cat".to_string(),
+            "/tmp/secret".to_string(),
+        ])),
+        Some(&PermissionProfile {
+            network: None,
+            file_system: Some(FileSystemPermissions {
+                read: Some(vec![
+                    AbsolutePathBuf::from_absolute_path("/tmp/secret")
+                        .expect("/tmp/secret should be an absolute path"),
+                ]),
+                write: None,
+            }),
+        }),
+        &[PermissionSuggestionDestination::UserSettings],
+    );
+
+    assert_eq!(suggestions, Vec::<PermissionSuggestion>::new());
+}
+
+#[test]
+fn network_approval_with_execpolicy_amendment_has_no_persistent_suggestions() {
+    let suggestions = approval_permission_suggestions(
+        Some(&NetworkApprovalContext {
+            host: "example.com".to_string(),
+            protocol: NetworkApprovalProtocol::Https,
+        }),
+        Some(&ExecPolicyAmendment::new(vec![
+            "curl".to_string(),
+            "https://example.com".to_string(),
+        ])),
+        /*additional_permissions*/ None,
+        &[PermissionSuggestionDestination::UserSettings],
+    );
+
+    assert_eq!(suggestions, Vec::<PermissionSuggestion>::new());
+}