mirror of
https://github.com/openai/codex.git
synced 2026-05-03 21:01:55 +03:00
feat: add experimental additionalPermissions to v2 command execution approval requests (#12737)
This adds additionalPermissions to the app-server v2
item/commandExecution/requestApproval payload as an experimental field.
The field is now exposed on CommandExecutionRequestApprovalParams and is
populated from the existing core approval event when a command requests
additional sandbox permissions.
This PR also contains changes to make server requests to support
experiment API.
A real app server test client test:
sample payload with experimental flag off:
```
{
< "id": 0,
< "method": "item/commandExecution/requestApproval",
< "params": {
< "command": "/bin/zsh -lc 'mkdir -p ~/some/test && touch ~/some/test/file'",
< "commandActions": [
< {
< "command": "mkdir -p '~/some/test'",
< "type": "unknown"
< },
< {
< "command": "touch '~/some/test/file'",
< "type": "unknown"
< }
< ],
< "cwd": "/Users/celia/code/codex/codex-rs",
< "itemId": "call_QLp0LWkQ1XkU6VW9T2vUZFWB",
< "proposedExecpolicyAmendment": [
< "mkdir",
< "-p",
< "~/some/test"
< ],
< "reason": "Do you want to allow creating ~/some/test/file outside the workspace?",
< "threadId": "019c9309-e209-7d82-a01b-dcf9556a354d",
< "turnId": "019c9309-e27a-7f33-834f-6011e795c2d6"
< }
< }
```
with experimental flag on:
```
< {
< "id": 0,
< "method": "item/commandExecution/requestApproval",
< "params": {
< "additionalPermissions": {
< "fileSystem": null,
< "macos": null,
< "network": true
< },
< "command": "/bin/zsh -lc 'install -D /dev/null ~/some/test/file'",
< "commandActions": [
< {
< "command": "install -D /dev/null '~/some/test/file'",
< "type": "unknown"
< }
< ],
< "cwd": "/Users/celia/code/codex/codex-rs",
< "itemId": "call_K3U4b3dRbj3eMCqslmncbGsq",
< "proposedExecpolicyAmendment": [
< "install",
< "-D"
< ],
< "reason": "Do you want to allow creating the file at ~/some/test/file outside the workspace sandbox?",
< "threadId": "019c9303-3a8e-76e1-81bf-d67ac446d892",
< "turnId": "019c9303-3af1-7143-88a1-73132f771234"
< }
< }
```
This commit is contained in:
@@ -6,6 +6,7 @@ use crate::outgoing_message::OutgoingError;
|
||||
use crate::outgoing_message::OutgoingMessage;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_app_server_protocol::JSONRPCMessage;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
use futures::SinkExt;
|
||||
use futures::StreamExt;
|
||||
use owo_colors::OwoColorize;
|
||||
@@ -144,6 +145,7 @@ pub(crate) enum TransportEvent {
|
||||
|
||||
pub(crate) struct ConnectionState {
|
||||
pub(crate) outbound_initialized: Arc<AtomicBool>,
|
||||
pub(crate) outbound_experimental_api_enabled: Arc<AtomicBool>,
|
||||
pub(crate) outbound_opted_out_notification_methods: Arc<RwLock<HashSet<String>>>,
|
||||
pub(crate) session: ConnectionSessionState,
|
||||
}
|
||||
@@ -151,10 +153,12 @@ pub(crate) struct ConnectionState {
|
||||
impl ConnectionState {
|
||||
pub(crate) fn new(
|
||||
outbound_initialized: Arc<AtomicBool>,
|
||||
outbound_experimental_api_enabled: Arc<AtomicBool>,
|
||||
outbound_opted_out_notification_methods: Arc<RwLock<HashSet<String>>>,
|
||||
) -> Self {
|
||||
Self {
|
||||
outbound_initialized,
|
||||
outbound_experimental_api_enabled,
|
||||
outbound_opted_out_notification_methods,
|
||||
session: ConnectionSessionState::default(),
|
||||
}
|
||||
@@ -163,6 +167,7 @@ impl ConnectionState {
|
||||
|
||||
pub(crate) struct OutboundConnectionState {
|
||||
pub(crate) initialized: Arc<AtomicBool>,
|
||||
pub(crate) experimental_api_enabled: Arc<AtomicBool>,
|
||||
pub(crate) opted_out_notification_methods: Arc<RwLock<HashSet<String>>>,
|
||||
pub(crate) writer: mpsc::Sender<OutgoingMessage>,
|
||||
disconnect_sender: Option<CancellationToken>,
|
||||
@@ -172,11 +177,13 @@ impl OutboundConnectionState {
|
||||
pub(crate) fn new(
|
||||
writer: mpsc::Sender<OutgoingMessage>,
|
||||
initialized: Arc<AtomicBool>,
|
||||
experimental_api_enabled: Arc<AtomicBool>,
|
||||
opted_out_notification_methods: Arc<RwLock<HashSet<String>>>,
|
||||
disconnect_sender: Option<CancellationToken>,
|
||||
) -> Self {
|
||||
Self {
|
||||
initialized,
|
||||
experimental_api_enabled,
|
||||
opted_out_notification_methods,
|
||||
writer,
|
||||
disconnect_sender,
|
||||
@@ -577,6 +584,7 @@ async fn send_message_to_connection(
|
||||
warn!("dropping message for disconnected connection: {connection_id:?}");
|
||||
return false;
|
||||
};
|
||||
let message = filter_outgoing_message_for_connection(connection_state, message);
|
||||
if should_skip_notification_for_connection(connection_state, &message) {
|
||||
return false;
|
||||
}
|
||||
@@ -602,6 +610,30 @@ async fn send_message_to_connection(
|
||||
}
|
||||
}
|
||||
|
||||
fn filter_outgoing_message_for_connection(
|
||||
connection_state: &OutboundConnectionState,
|
||||
message: OutgoingMessage,
|
||||
) -> OutgoingMessage {
|
||||
let experimental_api_enabled = connection_state
|
||||
.experimental_api_enabled
|
||||
.load(Ordering::Acquire);
|
||||
match message {
|
||||
OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval {
|
||||
request_id,
|
||||
mut params,
|
||||
}) => {
|
||||
if !experimental_api_enabled {
|
||||
params.strip_experimental_fields();
|
||||
}
|
||||
OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval {
|
||||
request_id,
|
||||
params,
|
||||
})
|
||||
}
|
||||
_ => message,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn route_outgoing_envelope(
|
||||
connections: &mut HashMap<ConnectionId, OutboundConnectionState>,
|
||||
envelope: OutgoingEnvelope,
|
||||
@@ -641,6 +673,7 @@ mod tests {
|
||||
use crate::error_code::OVERLOADED_ERROR_CODE;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::path::PathBuf;
|
||||
use tokio::time::Duration;
|
||||
use tokio::time::timeout;
|
||||
|
||||
@@ -880,6 +913,7 @@ mod tests {
|
||||
OutboundConnectionState::new(
|
||||
writer_tx,
|
||||
initialized,
|
||||
Arc::new(AtomicBool::new(true)),
|
||||
opted_out_notification_methods,
|
||||
None,
|
||||
),
|
||||
@@ -905,6 +939,136 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_execution_request_approval_strips_experimental_fields_without_capability() {
|
||||
let connection_id = ConnectionId(8);
|
||||
let (writer_tx, mut writer_rx) = mpsc::channel(1);
|
||||
|
||||
let mut connections = HashMap::new();
|
||||
connections.insert(
|
||||
connection_id,
|
||||
OutboundConnectionState::new(
|
||||
writer_tx,
|
||||
Arc::new(AtomicBool::new(true)),
|
||||
Arc::new(AtomicBool::new(false)),
|
||||
Arc::new(RwLock::new(HashSet::new())),
|
||||
None,
|
||||
),
|
||||
);
|
||||
|
||||
route_outgoing_envelope(
|
||||
&mut connections,
|
||||
OutgoingEnvelope::ToConnection {
|
||||
connection_id,
|
||||
message: OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval {
|
||||
request_id: codex_app_server_protocol::RequestId::Integer(1),
|
||||
params: codex_app_server_protocol::CommandExecutionRequestApprovalParams {
|
||||
thread_id: "thr_123".to_string(),
|
||||
turn_id: "turn_123".to_string(),
|
||||
item_id: "call_123".to_string(),
|
||||
approval_id: None,
|
||||
reason: Some("Need extra read access".to_string()),
|
||||
network_approval_context: None,
|
||||
command: Some("cat file".to_string()),
|
||||
cwd: Some(PathBuf::from("/tmp")),
|
||||
command_actions: None,
|
||||
additional_permissions: Some(
|
||||
codex_app_server_protocol::AdditionalPermissionProfile {
|
||||
network: None,
|
||||
file_system: Some(
|
||||
codex_app_server_protocol::AdditionalFileSystemPermissions {
|
||||
read: Some(vec![PathBuf::from("/tmp/allowed")]),
|
||||
write: None,
|
||||
},
|
||||
),
|
||||
macos: None,
|
||||
},
|
||||
),
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
}),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let message = writer_rx
|
||||
.recv()
|
||||
.await
|
||||
.expect("request should be delivered to the connection");
|
||||
let json = serde_json::to_value(message).expect("request should serialize");
|
||||
assert_eq!(json["params"].get("additionalPermissions"), None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn command_execution_request_approval_keeps_experimental_fields_with_capability() {
|
||||
let connection_id = ConnectionId(9);
|
||||
let (writer_tx, mut writer_rx) = mpsc::channel(1);
|
||||
|
||||
let mut connections = HashMap::new();
|
||||
connections.insert(
|
||||
connection_id,
|
||||
OutboundConnectionState::new(
|
||||
writer_tx,
|
||||
Arc::new(AtomicBool::new(true)),
|
||||
Arc::new(AtomicBool::new(true)),
|
||||
Arc::new(RwLock::new(HashSet::new())),
|
||||
None,
|
||||
),
|
||||
);
|
||||
|
||||
route_outgoing_envelope(
|
||||
&mut connections,
|
||||
OutgoingEnvelope::ToConnection {
|
||||
connection_id,
|
||||
message: OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval {
|
||||
request_id: codex_app_server_protocol::RequestId::Integer(1),
|
||||
params: codex_app_server_protocol::CommandExecutionRequestApprovalParams {
|
||||
thread_id: "thr_123".to_string(),
|
||||
turn_id: "turn_123".to_string(),
|
||||
item_id: "call_123".to_string(),
|
||||
approval_id: None,
|
||||
reason: Some("Need extra read access".to_string()),
|
||||
network_approval_context: None,
|
||||
command: Some("cat file".to_string()),
|
||||
cwd: Some(PathBuf::from("/tmp")),
|
||||
command_actions: None,
|
||||
additional_permissions: Some(
|
||||
codex_app_server_protocol::AdditionalPermissionProfile {
|
||||
network: None,
|
||||
file_system: Some(
|
||||
codex_app_server_protocol::AdditionalFileSystemPermissions {
|
||||
read: Some(vec![PathBuf::from("/tmp/allowed")]),
|
||||
write: None,
|
||||
},
|
||||
),
|
||||
macos: None,
|
||||
},
|
||||
),
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
}),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let message = writer_rx
|
||||
.recv()
|
||||
.await
|
||||
.expect("request should be delivered to the connection");
|
||||
let json = serde_json::to_value(message).expect("request should serialize");
|
||||
assert_eq!(
|
||||
json["params"]["additionalPermissions"],
|
||||
json!({
|
||||
"network": null,
|
||||
"fileSystem": {
|
||||
"read": ["/tmp/allowed"],
|
||||
"write": null,
|
||||
},
|
||||
"macos": null,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn broadcast_does_not_block_on_slow_connection() {
|
||||
let fast_connection_id = ConnectionId(1);
|
||||
@@ -921,6 +1085,7 @@ mod tests {
|
||||
OutboundConnectionState::new(
|
||||
fast_writer_tx,
|
||||
Arc::new(AtomicBool::new(true)),
|
||||
Arc::new(AtomicBool::new(true)),
|
||||
Arc::new(RwLock::new(HashSet::new())),
|
||||
Some(fast_disconnect_token.clone()),
|
||||
),
|
||||
@@ -930,6 +1095,7 @@ mod tests {
|
||||
OutboundConnectionState::new(
|
||||
slow_writer_tx.clone(),
|
||||
Arc::new(AtomicBool::new(true)),
|
||||
Arc::new(AtomicBool::new(true)),
|
||||
Arc::new(RwLock::new(HashSet::new())),
|
||||
Some(slow_disconnect_token.clone()),
|
||||
),
|
||||
@@ -1006,6 +1172,7 @@ mod tests {
|
||||
OutboundConnectionState::new(
|
||||
writer_tx,
|
||||
Arc::new(AtomicBool::new(true)),
|
||||
Arc::new(AtomicBool::new(true)),
|
||||
Arc::new(RwLock::new(HashSet::new())),
|
||||
None,
|
||||
),
|
||||
|
||||
Reference in New Issue
Block a user