mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Simplify permissions available in request permissions tool (#14529)
This commit is contained in:
@@ -106,6 +106,7 @@ use codex_protocol::protocol::TurnContextNetworkItem;
|
||||
use codex_protocol::protocol::TurnStartedEvent;
|
||||
use codex_protocol::protocol::W3cTraceContext;
|
||||
use codex_protocol::request_permissions::PermissionGrantScope;
|
||||
use codex_protocol::request_permissions::RequestPermissionProfile;
|
||||
use codex_protocol::request_permissions::RequestPermissionsArgs;
|
||||
use codex_protocol::request_permissions::RequestPermissionsEvent;
|
||||
use codex_protocol::request_permissions::RequestPermissionsResponse;
|
||||
@@ -2908,7 +2909,7 @@ impl Session {
|
||||
match turn_context.approval_policy.value() {
|
||||
AskForApproval::Never => {
|
||||
return Some(RequestPermissionsResponse {
|
||||
permissions: PermissionProfile::default(),
|
||||
permissions: RequestPermissionProfile::default(),
|
||||
scope: PermissionGrantScope::Turn,
|
||||
});
|
||||
}
|
||||
@@ -2916,7 +2917,7 @@ impl Session {
|
||||
if !granular_config.allows_request_permissions() =>
|
||||
{
|
||||
return Some(RequestPermissionsResponse {
|
||||
permissions: PermissionProfile::default(),
|
||||
permissions: RequestPermissionProfile::default(),
|
||||
scope: PermissionGrantScope::Turn,
|
||||
});
|
||||
}
|
||||
@@ -3102,7 +3103,7 @@ impl Session {
|
||||
if entry.is_some() && !response.permissions.is_empty() {
|
||||
match response.scope {
|
||||
PermissionGrantScope::Turn => {
|
||||
ts.record_granted_permissions(response.permissions.clone());
|
||||
ts.record_granted_permissions(response.permissions.clone().into());
|
||||
}
|
||||
PermissionGrantScope::Session => {
|
||||
granted_for_session = Some(response.permissions.clone());
|
||||
@@ -3116,7 +3117,7 @@ impl Session {
|
||||
};
|
||||
if let Some(permissions) = granted_for_session {
|
||||
let mut state = self.state.lock().await;
|
||||
state.record_granted_permissions(permissions);
|
||||
state.record_granted_permissions(permissions.into());
|
||||
}
|
||||
match entry {
|
||||
Some(tx_response) => {
|
||||
|
||||
@@ -1,13 +1,13 @@
|
||||
use super::*;
|
||||
use async_channel::bounded;
|
||||
use codex_protocol::models::NetworkPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::AgentStatus;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::RawResponseItemEvent;
|
||||
use codex_protocol::protocol::TurnAbortReason;
|
||||
use codex_protocol::protocol::TurnAbortedEvent;
|
||||
use codex_protocol::request_permissions::RequestPermissionProfile;
|
||||
use codex_protocol::request_permissions::RequestPermissionsEvent;
|
||||
use codex_protocol::request_permissions::RequestPermissionsResponse;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -150,11 +150,11 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() {
|
||||
|
||||
let call_id = "tool-call-1".to_string();
|
||||
let expected_response = RequestPermissionsResponse {
|
||||
permissions: PermissionProfile {
|
||||
permissions: RequestPermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
..PermissionProfile::default()
|
||||
..RequestPermissionProfile::default()
|
||||
},
|
||||
scope: PermissionGrantScope::Turn,
|
||||
};
|
||||
@@ -175,11 +175,11 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() {
|
||||
call_id: request_call_id,
|
||||
turn_id: "child-turn-1".to_string(),
|
||||
reason: Some("need access".to_string()),
|
||||
permissions: PermissionProfile {
|
||||
permissions: RequestPermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
..PermissionProfile::default()
|
||||
..RequestPermissionProfile::default()
|
||||
},
|
||||
},
|
||||
&cancel_token,
|
||||
|
||||
@@ -25,6 +25,7 @@ use codex_protocol::permissions::FileSystemSpecialPath;
|
||||
use codex_protocol::protocol::ReadOnlyAccess;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::request_permissions::PermissionGrantScope;
|
||||
use codex_protocol::request_permissions::RequestPermissionProfile;
|
||||
use tracing::Span;
|
||||
|
||||
use crate::protocol::CompactedItem;
|
||||
@@ -2216,11 +2217,11 @@ async fn notify_request_permissions_response_ignores_unmatched_call_id() {
|
||||
.notify_request_permissions_response(
|
||||
"missing",
|
||||
codex_protocol::request_permissions::RequestPermissionsResponse {
|
||||
permissions: codex_protocol::models::PermissionProfile {
|
||||
permissions: RequestPermissionProfile {
|
||||
network: Some(codex_protocol::models::NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
..RequestPermissionProfile::default()
|
||||
},
|
||||
scope: PermissionGrantScope::Turn,
|
||||
},
|
||||
@@ -2252,11 +2253,11 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
|
||||
let turn_context = Arc::new(turn_context);
|
||||
let call_id = "call-1".to_string();
|
||||
let expected_response = codex_protocol::request_permissions::RequestPermissionsResponse {
|
||||
permissions: codex_protocol::models::PermissionProfile {
|
||||
permissions: RequestPermissionProfile {
|
||||
network: Some(codex_protocol::models::NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
..RequestPermissionProfile::default()
|
||||
},
|
||||
scope: PermissionGrantScope::Turn,
|
||||
};
|
||||
@@ -2272,11 +2273,11 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
|
||||
call_id,
|
||||
codex_protocol::request_permissions::RequestPermissionsArgs {
|
||||
reason: Some("need network".to_string()),
|
||||
permissions: codex_protocol::models::PermissionProfile {
|
||||
permissions: RequestPermissionProfile {
|
||||
network: Some(codex_protocol::models::NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
..RequestPermissionProfile::default()
|
||||
},
|
||||
},
|
||||
)
|
||||
@@ -2332,11 +2333,11 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req
|
||||
call_id,
|
||||
codex_protocol::request_permissions::RequestPermissionsArgs {
|
||||
reason: Some("need network".to_string()),
|
||||
permissions: codex_protocol::models::PermissionProfile {
|
||||
permissions: RequestPermissionProfile {
|
||||
network: Some(codex_protocol::models::NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
..RequestPermissionProfile::default()
|
||||
},
|
||||
},
|
||||
)
|
||||
@@ -2346,7 +2347,7 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req
|
||||
response,
|
||||
Some(
|
||||
codex_protocol::request_permissions::RequestPermissionsResponse {
|
||||
permissions: codex_protocol::models::PermissionProfile::default(),
|
||||
permissions: RequestPermissionProfile::default(),
|
||||
scope: PermissionGrantScope::Turn,
|
||||
}
|
||||
)
|
||||
|
||||
@@ -11,7 +11,7 @@ use crate::tools::registry::ToolHandler;
|
||||
use crate::tools::registry::ToolKind;
|
||||
|
||||
pub(crate) fn request_permissions_tool_description() -> String {
|
||||
"Request additional permissions from the user and wait for the client to grant a subset of the requested permission profile. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session if the client approves them at session scope."
|
||||
"Request additional filesystem or network permissions from the user and wait for the client to grant a subset of the requested permission profile. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session if the client approves them at session scope."
|
||||
.to_string()
|
||||
}
|
||||
|
||||
@@ -45,7 +45,8 @@ impl ToolHandler for RequestPermissionsHandler {
|
||||
|
||||
let mut args: RequestPermissionsArgs =
|
||||
parse_arguments_with_base_path(&arguments, turn.cwd.as_path())?;
|
||||
args.permissions = normalize_additional_permissions(args.permissions)
|
||||
args.permissions = normalize_additional_permissions(args.permissions.into())
|
||||
.map(codex_protocol::request_permissions::RequestPermissionProfile::from)
|
||||
.map_err(FunctionCallError::RespondToModel)?;
|
||||
if args.permissions.is_empty() {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
|
||||
@@ -494,44 +494,21 @@ fn create_file_system_permissions_schema() -> JsonSchema {
|
||||
}
|
||||
}
|
||||
|
||||
fn create_macos_permissions_schema() -> JsonSchema {
|
||||
fn create_additional_permissions_schema() -> JsonSchema {
|
||||
JsonSchema::Object {
|
||||
properties: BTreeMap::from([
|
||||
("network".to_string(), create_network_permissions_schema()),
|
||||
(
|
||||
"preferences".to_string(),
|
||||
JsonSchema::String {
|
||||
description: Some(
|
||||
"macOS preferences access. Supported values: `none`, `read_only`, or `read_write`."
|
||||
.to_string(),
|
||||
),
|
||||
},
|
||||
"file_system".to_string(),
|
||||
create_file_system_permissions_schema(),
|
||||
),
|
||||
(
|
||||
"automations".to_string(),
|
||||
JsonSchema::Array {
|
||||
items: Box::new(JsonSchema::String { description: None }),
|
||||
description: Some("macOS automation access as app bundle identifiers.".to_string()),
|
||||
},
|
||||
),
|
||||
(
|
||||
"accessibility".to_string(),
|
||||
JsonSchema::Boolean {
|
||||
description: Some("Whether to request macOS accessibility access.".to_string()),
|
||||
},
|
||||
),
|
||||
(
|
||||
"calendar".to_string(),
|
||||
JsonSchema::Boolean {
|
||||
description: Some("Whether to request macOS calendar access.".to_string()),
|
||||
},
|
||||
),
|
||||
]),
|
||||
required: None,
|
||||
additional_properties: Some(false.into()),
|
||||
}
|
||||
}
|
||||
|
||||
fn create_permissions_schema() -> JsonSchema {
|
||||
fn create_request_permissions_schema() -> JsonSchema {
|
||||
JsonSchema::Object {
|
||||
properties: BTreeMap::from([
|
||||
("network".to_string(), create_network_permissions_schema()),
|
||||
@@ -539,7 +516,6 @@ fn create_permissions_schema() -> JsonSchema {
|
||||
"file_system".to_string(),
|
||||
create_file_system_permissions_schema(),
|
||||
),
|
||||
("macos".to_string(), create_macos_permissions_schema()),
|
||||
]),
|
||||
required: None,
|
||||
additional_properties: Some(false.into()),
|
||||
@@ -555,7 +531,7 @@ fn create_approval_parameters(
|
||||
JsonSchema::String {
|
||||
description: Some(
|
||||
if exec_permission_approvals_enabled {
|
||||
"Sandbox permissions for the command. Use \"with_additional_permissions\" to request additional sandboxed filesystem, network, or macOS permissions (preferred), or \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"."
|
||||
"Sandbox permissions for the command. Use \"with_additional_permissions\" to request additional sandboxed filesystem or network permissions (preferred), or \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"."
|
||||
} else {
|
||||
"Sandbox permissions for the command. Set to \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"."
|
||||
}
|
||||
@@ -592,7 +568,7 @@ fn create_approval_parameters(
|
||||
if exec_permission_approvals_enabled {
|
||||
properties.insert(
|
||||
"additional_permissions".to_string(),
|
||||
create_permissions_schema(),
|
||||
create_additional_permissions_schema(),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1455,7 +1431,10 @@ fn create_request_permissions_tool() -> ToolSpec {
|
||||
),
|
||||
},
|
||||
);
|
||||
properties.insert("permissions".to_string(), create_permissions_schema());
|
||||
properties.insert(
|
||||
"permissions".to_string(),
|
||||
create_request_permissions_schema(),
|
||||
);
|
||||
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "request_permissions".to_string(),
|
||||
|
||||
@@ -2205,7 +2205,7 @@ fn shell_tool_with_request_permission_includes_additional_permissions() {
|
||||
panic!("expected sandbox_permissions description");
|
||||
};
|
||||
assert!(description.contains("with_additional_permissions"));
|
||||
assert!(description.contains("macOS permissions"));
|
||||
assert!(description.contains("filesystem or network permissions"));
|
||||
|
||||
let Some(JsonSchema::Object {
|
||||
properties: additional_properties,
|
||||
@@ -2216,7 +2216,7 @@ fn shell_tool_with_request_permission_includes_additional_permissions() {
|
||||
};
|
||||
assert!(additional_properties.contains_key("network"));
|
||||
assert!(additional_properties.contains_key("file_system"));
|
||||
assert!(additional_properties.contains_key("macos"));
|
||||
assert!(!additional_properties.contains_key("macos"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -2240,7 +2240,7 @@ fn request_permissions_tool_includes_full_permission_schema() {
|
||||
assert_eq!(additional_properties, &Some(false.into()));
|
||||
assert!(permission_properties.contains_key("network"));
|
||||
assert!(permission_properties.contains_key("file_system"));
|
||||
assert!(permission_properties.contains_key("macos"));
|
||||
assert!(!permission_properties.contains_key("macos"));
|
||||
|
||||
let Some(JsonSchema::Object {
|
||||
properties: network_properties,
|
||||
@@ -2264,20 +2264,6 @@ fn request_permissions_tool_includes_full_permission_schema() {
|
||||
assert_eq!(additional_properties, &Some(false.into()));
|
||||
assert!(file_system_properties.contains_key("read"));
|
||||
assert!(file_system_properties.contains_key("write"));
|
||||
|
||||
let Some(JsonSchema::Object {
|
||||
properties: macos_properties,
|
||||
additional_properties,
|
||||
..
|
||||
}) = permission_properties.get("macos")
|
||||
else {
|
||||
panic!("expected macos object");
|
||||
};
|
||||
assert_eq!(additional_properties, &Some(false.into()));
|
||||
assert!(macos_properties.contains_key("preferences"));
|
||||
assert!(macos_properties.contains_key("automations"));
|
||||
assert!(macos_properties.contains_key("accessibility"));
|
||||
assert!(macos_properties.contains_key("calendar"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user