sandboxing: intersect permission profiles semantically

This commit is contained in:
Michael Bolin
2026-04-21 09:18:39 -07:00
parent 53cf12cd52
commit 87b4c7d788
16 changed files with 686 additions and 63 deletions
+177 -23
View File
@@ -120,6 +120,7 @@ use codex_protocol::ThreadId;
use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem as CoreDynamicToolCallOutputContentItem;
use codex_protocol::dynamic_tools::DynamicToolResponse as CoreDynamicToolResponse;
use codex_protocol::items::parse_hook_prompt_message;
use codex_protocol::models::PermissionProfile as CorePermissionProfile;
use codex_protocol::plan_tool::UpdatePlanArgs;
use codex_protocol::protocol::CodexErrorInfo as CoreCodexErrorInfo;
use codex_protocol::protocol::Event;
@@ -908,6 +909,10 @@ pub(crate) async fn apply_bespoke_event_handling(
.note_permission_requested(&conversation_id.to_string())
.await;
let requested_permissions = request.permissions.clone();
let request_cwd = match request.cwd.clone() {
Some(cwd) => cwd,
None => conversation.config_snapshot().await.cwd,
};
let params = PermissionsRequestApprovalParams {
thread_id: conversation_id.to_string(),
turn_id: request.turn_id.clone(),
@@ -918,17 +923,17 @@ pub(crate) async fn apply_bespoke_event_handling(
let (pending_request_id, rx) = outgoing
.send_request(ServerRequestPayload::PermissionsRequestApproval(params))
.await;
let pending_response = PendingRequestPermissionsResponse {
call_id: request.call_id,
requested_permissions,
request_cwd,
pending_request_id,
receiver: rx,
request_permissions_guard: permission_guard,
};
tokio::spawn(async move {
on_request_permissions_response(
request.call_id,
requested_permissions,
pending_request_id,
rx,
conversation,
thread_state,
permission_guard,
)
.await;
on_request_permissions_response(pending_response, conversation, thread_state)
.await;
});
} else {
error!(
@@ -2590,20 +2595,26 @@ fn mcp_server_elicitation_response_from_client_result(
}
async fn on_request_permissions_response(
call_id: String,
requested_permissions: CoreRequestPermissionProfile,
pending_request_id: RequestId,
receiver: oneshot::Receiver<ClientRequestResult>,
pending_response: PendingRequestPermissionsResponse,
conversation: Arc<CodexThread>,
thread_state: Arc<Mutex<ThreadState>>,
request_permissions_guard: ThreadWatchActiveGuard,
) {
let PendingRequestPermissionsResponse {
call_id,
requested_permissions,
request_cwd,
pending_request_id,
receiver,
request_permissions_guard,
} = pending_response;
let response = receiver.await;
resolve_server_request_on_thread_listener(&thread_state, pending_request_id).await;
drop(request_permissions_guard);
let Some(response) =
request_permissions_response_from_client_result(requested_permissions, response)
else {
let Some(response) = request_permissions_response_from_client_result(
requested_permissions,
response,
request_cwd.as_path(),
) else {
return;
};
@@ -2618,9 +2629,19 @@ async fn on_request_permissions_response(
}
}
struct PendingRequestPermissionsResponse {
call_id: String,
requested_permissions: CoreRequestPermissionProfile,
request_cwd: AbsolutePathBuf,
pending_request_id: RequestId,
receiver: oneshot::Receiver<ClientRequestResult>,
request_permissions_guard: ThreadWatchActiveGuard,
}
fn request_permissions_response_from_client_result(
requested_permissions: CoreRequestPermissionProfile,
response: std::result::Result<ClientRequestResult, oneshot::error::RecvError>,
cwd: &std::path::Path,
) -> Option<CoreRequestPermissionsResponse> {
let value = match response {
Ok(Ok(value)) => value,
@@ -2649,12 +2670,14 @@ fn request_permissions_response_from_client_result(
scope: codex_app_server_protocol::PermissionGrantScope::Turn,
}
});
let granted_permissions: CorePermissionProfile = response.permissions.into();
let permissions = if granted_permissions.is_empty() {
CoreRequestPermissionProfile::default()
} else {
intersect_permission_profiles(requested_permissions.into(), granted_permissions, cwd).into()
};
Some(CoreRequestPermissionsResponse {
permissions: intersect_permission_profiles(
requested_permissions.into(),
response.permissions.into(),
)
.into(),
permissions,
scope: response.scope.to_core(),
})
}
@@ -3027,6 +3050,10 @@ mod tests {
use codex_protocol::mcp::CallToolResult;
use codex_protocol::models::FileSystemPermissions as CoreFileSystemPermissions;
use codex_protocol::models::NetworkPermissions as CoreNetworkPermissions;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::plan_tool::PlanItemArg;
use codex_protocol::plan_tool::StepStatus;
use codex_protocol::protocol::CollabResumeBeginEvent;
@@ -3711,6 +3738,7 @@ mod tests {
let response = request_permissions_response_from_client_result(
CoreRequestPermissionProfile::default(),
Ok(Err(error)),
std::env::current_dir().expect("current dir").as_path(),
);
assert_eq!(response, None);
@@ -3797,12 +3825,14 @@ mod tests {
),
];
let cwd = std::env::current_dir().expect("current dir");
for (granted_permissions, expected_permissions) in cases {
let response = request_permissions_response_from_client_result(
requested_permissions.clone(),
Ok(Ok(serde_json::json!({
"permissions": granted_permissions,
}))),
cwd.as_path(),
)
.expect("response should be accepted");
@@ -3824,6 +3854,7 @@ mod tests {
"scope": "session",
"permissions": {},
}))),
std::env::current_dir().expect("current dir").as_path(),
)
.expect("response should be accepted");
@@ -3836,6 +3867,129 @@ mod tests {
);
}
#[test]
fn request_permissions_response_accepts_explicit_child_grant_for_requested_cwd_scope() {
let temp_dir = TempDir::new().expect("temp dir");
let cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute cwd");
let child = cwd.join("child");
let requested_permissions = CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
let response = request_permissions_response_from_client_result(
requested_permissions,
Ok(Ok(serde_json::json!({
"permissions": {
"fileSystem": {
"write": [child],
},
},
}))),
cwd.as_path(),
)
.expect("response should be accepted");
assert_eq!(
response.permissions,
CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![child]),
)),
..Default::default()
}
);
}
#[test]
fn request_permissions_response_rejects_child_grant_outside_requested_cwd_scope() {
let temp_dir = TempDir::new().expect("temp dir");
let request_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("request-cwd"))
.expect("absolute request cwd");
let later_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("later-cwd"))
.expect("absolute later cwd");
let later_child = later_cwd.join("child");
let requested_permissions = CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
let response = request_permissions_response_from_client_result(
requested_permissions,
Ok(Ok(serde_json::json!({
"permissions": {
"fileSystem": {
"write": [later_child],
},
},
}))),
request_cwd.as_path(),
)
.expect("response should be accepted");
assert_eq!(
response.permissions,
CoreRequestPermissionProfile::default()
);
}
#[test]
fn request_permissions_response_ignores_broader_cwd_grant_for_requested_child_path() {
let temp_dir = TempDir::new().expect("temp dir");
let cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute cwd");
let child = cwd.join("child");
let requested_permissions = CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![child]),
)),
..Default::default()
};
let response = request_permissions_response_from_client_result(
requested_permissions,
Ok(Ok(serde_json::json!({
"permissions": {
"fileSystem": {
"entries": [{
"path": {
"type": "special",
"value": {
"kind": "current_working_directory"
}
},
"access": "write"
}],
},
},
}))),
cwd.as_path(),
)
.expect("response should be accepted");
assert_eq!(
response.permissions,
CoreRequestPermissionProfile::default()
);
}
#[test]
fn collab_resume_begin_maps_to_item_started_resume_agent() {
let event = CollabResumeBeginEvent {
+8 -2
View File
@@ -741,8 +741,14 @@ async fn handle_request_permissions(
reason: event.reason,
permissions: event.permissions,
};
let response_fut =
parent_session.request_permissions(parent_ctx, call_id.clone(), args, cancel_token.clone());
let cwd = event.cwd.unwrap_or_else(|| parent_ctx.cwd.clone());
let response_fut = parent_session.request_permissions_for_cwd(
parent_ctx,
call_id.clone(),
args,
cwd,
cancel_token.clone(),
);
let response =
await_request_permissions_with_cancel(response_fut, parent_session, &call_id, cancel_token)
.await;
@@ -180,8 +180,10 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() {
},
scope: PermissionGrantScope::Turn,
};
let delegated_cwd = parent_ctx.cwd.join("delegated-cwd");
let cancel_token = CancellationToken::new();
let request_call_id = call_id.clone();
let request_cwd = delegated_cwd.clone();
let handle = tokio::spawn({
let codex = Arc::clone(&codex);
@@ -203,6 +205,7 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() {
}),
..RequestPermissionProfile::default()
},
cwd: Some(request_cwd),
},
&cancel_token,
)
@@ -218,6 +221,7 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() {
panic!("expected RequestPermissions event");
};
assert_eq!(request.call_id, call_id.clone());
assert_eq!(request.cwd, Some(delegated_cwd));
parent_session
.notify_request_permissions_response(&call_id, expected_response.clone())
+77 -19
View File
@@ -1,6 +1,7 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt::Debug;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::atomic::AtomicU64;
@@ -113,6 +114,7 @@ use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_rmcp_client::ElicitationResponse;
use codex_rollout::RolloutConfig;
use codex_rollout::state_db;
use codex_sandboxing::policy_transforms::intersect_permission_profiles;
use codex_shell_command::parse_command::parse_command;
use codex_terminal_detection::user_agent;
use codex_thread_store::LocalThreadStore;
@@ -269,6 +271,7 @@ use crate::skills_watcher::SkillsWatcher;
use crate::skills_watcher::SkillsWatcherEvent;
use crate::state::ActiveTurn;
use crate::state::MailboxDeliveryPhase;
use crate::state::PendingRequestPermissions;
use crate::state::SessionServices;
use crate::state::SessionState;
#[cfg(test)]
@@ -1901,16 +1904,34 @@ impl Session {
rx_approve
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub async fn request_permissions(
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
self.request_permissions_for_cwd(
turn_context,
call_id,
args,
turn_context.cwd.clone(),
cancellation_token,
)
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub(crate) async fn request_permissions_for_cwd(
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
cwd: AbsolutePathBuf,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
match turn_context.as_ref().approval_policy.value() {
AskForApproval::Never => {
@@ -1933,8 +1954,9 @@ impl Session {
| AskForApproval::Granular(_) => {}
}
let requested_permissions = args.permissions;
if crate::guardian::routes_approval_to_guardian(turn_context.as_ref()) {
let requested_permissions = args.permissions;
let originating_turn_state = {
let active = self.active_turn.lock().await;
active.as_ref().map(|active| Arc::clone(&active.turn_state))
@@ -1964,19 +1986,19 @@ impl Session {
let response = match decision {
ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } => {
RequestPermissionsResponse {
permissions: requested_permissions,
permissions: requested_permissions.clone(),
scope: PermissionGrantScope::Turn,
}
}
ReviewDecision::ApprovedForSession => RequestPermissionsResponse {
permissions: requested_permissions,
permissions: requested_permissions.clone(),
scope: PermissionGrantScope::Session,
},
ReviewDecision::NetworkPolicyAmendment {
network_policy_amendment,
} => match network_policy_amendment.action {
NetworkPolicyRuleAction::Allow => RequestPermissionsResponse {
permissions: requested_permissions,
permissions: requested_permissions.clone(),
scope: PermissionGrantScope::Turn,
},
NetworkPolicyRuleAction::Deny => RequestPermissionsResponse {
@@ -1991,6 +2013,11 @@ impl Session {
}
}
};
let response = Self::normalize_request_permissions_response(
requested_permissions,
response,
cwd.as_path(),
);
self.record_granted_request_permissions_for_turn(
&response,
originating_turn_state.as_ref(),
@@ -2005,7 +2032,14 @@ impl Session {
match active.as_mut() {
Some(at) => {
let mut ts = at.turn_state.lock().await;
ts.insert_pending_request_permissions(call_id.clone(), tx_response)
ts.insert_pending_request_permissions(
call_id.clone(),
PendingRequestPermissions {
tx_response,
requested_permissions: requested_permissions.clone(),
cwd: cwd.clone(),
},
)
}
None => None,
}
@@ -2018,7 +2052,8 @@ impl Session {
call_id: call_id.clone(),
turn_id: turn_context.sub_id.clone(),
reason: args.reason,
permissions: args.permissions,
permissions: requested_permissions,
cwd: Some(cwd),
});
self.send_event(turn_context.as_ref(), event).await;
tokio::select! {
@@ -2121,16 +2156,19 @@ impl Session {
None => (None, None),
}
};
if entry.is_some() {
self.record_granted_request_permissions_for_turn(
&response,
originating_turn_state.as_ref(),
)
.await;
}
match entry {
Some(tx_response) => {
tx_response.send(response).ok();
Some(entry) => {
let response = Self::normalize_request_permissions_response(
entry.requested_permissions,
response,
entry.cwd.as_path(),
);
self.record_granted_request_permissions_for_turn(
&response,
originating_turn_state.as_ref(),
)
.await;
entry.tx_response.send(response).ok();
}
None => {
warn!("No pending request_permissions found for call_id: {call_id}");
@@ -2138,6 +2176,26 @@ impl Session {
}
}
fn normalize_request_permissions_response(
requested_permissions: RequestPermissionProfile,
response: RequestPermissionsResponse,
cwd: &Path,
) -> RequestPermissionsResponse {
if response.permissions.is_empty() {
return response;
}
RequestPermissionsResponse {
permissions: intersect_permission_profiles(
requested_permissions.into(),
response.permissions.into(),
cwd,
)
.into(),
scope: response.scope,
}
}
async fn record_granted_request_permissions_for_turn(
&self,
response: &RequestPermissionsResponse,
+97
View File
@@ -40,6 +40,7 @@ use codex_protocol::AgentPath;
use codex_protocol::ThreadId;
use codex_protocol::config_types::TrustLevel;
use codex_protocol::exec_output::ExecToolCallOutput;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::FunctionCallOutputBody;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::permissions::FileSystemAccessMode;
@@ -3536,6 +3537,7 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
panic!("expected request_permissions event");
};
assert_eq!(request.call_id, call_id);
assert_eq!(request.cwd, Some(turn_context.cwd.clone()));
session
.notify_request_permissions_response(&request.call_id, expected_response.clone())
@@ -3549,6 +3551,101 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
assert_eq!(response, Some(expected_response));
}
#[tokio::test]
async fn request_permissions_response_materializes_session_cwd_grants_before_recording() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
*session.active_turn.lock().await = Some(ActiveTurn::default());
Arc::get_mut(&mut turn_context)
.expect("single turn context ref")
.approval_policy
.set(AskForApproval::Granular(GranularApprovalConfig {
sandbox_approval: true,
rules: true,
skill_approval: true,
request_permissions: true,
mcp_elicitations: true,
}))
.expect("test setup should allow updating approval policy");
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let call_id = "call-1".to_string();
let requested_permissions = RequestPermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
let handle = tokio::spawn({
let session = Arc::clone(&session);
let turn_context = Arc::clone(&turn_context);
let call_id = call_id.clone();
let requested_permissions = requested_permissions.clone();
async move {
session
.request_permissions(
&turn_context,
call_id,
codex_protocol::request_permissions::RequestPermissionsArgs {
reason: Some("need cwd write".to_string()),
permissions: requested_permissions,
},
CancellationToken::new(),
)
.await
}
});
let request_event = tokio::time::timeout(StdDuration::from_secs(1), rx.recv())
.await
.expect("request_permissions event timed out")
.expect("request_permissions event missing");
let EventMsg::RequestPermissions(request) = request_event.msg else {
panic!("expected request_permissions event");
};
let request_cwd = request.cwd.clone().expect("request cwd");
session
.notify_request_permissions_response(
&request.call_id,
codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: request.permissions,
scope: PermissionGrantScope::Session,
},
)
.await;
let expected_permissions = RequestPermissionProfile {
file_system: Some(FileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![request_cwd]),
)),
..Default::default()
};
let expected_response = codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: expected_permissions.clone(),
scope: PermissionGrantScope::Session,
};
let response = tokio::time::timeout(StdDuration::from_secs(1), handle)
.await
.expect("request_permissions future timed out")
.expect("request_permissions join error");
assert_eq!(response, Some(expected_response));
assert_eq!(
session.granted_session_permissions().await,
Some(expected_permissions.into())
);
}
#[tokio::test]
async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_requests() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
+1
View File
@@ -6,6 +6,7 @@ pub(crate) use service::SessionServices;
pub(crate) use session::SessionState;
pub(crate) use turn::ActiveTurn;
pub(crate) use turn::MailboxDeliveryPhase;
pub(crate) use turn::PendingRequestPermissions;
pub(crate) use turn::RunningTask;
pub(crate) use turn::TaskKind;
pub(crate) use turn::TurnState;
+14 -5
View File
@@ -11,9 +11,11 @@ use tokio_util::task::AbortOnDropHandle;
use codex_protocol::dynamic_tools::DynamicToolResponse;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_rmcp_client::ElicitationResponse;
use codex_utils_absolute_path::AbsolutePathBuf;
use rmcp::model::RequestId;
use tokio::sync::oneshot;
@@ -97,7 +99,7 @@ impl ActiveTurn {
#[derive(Default)]
pub(crate) struct TurnState {
pending_approvals: HashMap<String, oneshot::Sender<ReviewDecision>>,
pending_request_permissions: HashMap<String, oneshot::Sender<RequestPermissionsResponse>>,
pending_request_permissions: HashMap<String, PendingRequestPermissions>,
pending_user_input: HashMap<String, oneshot::Sender<RequestUserInputResponse>>,
pending_elicitations: HashMap<(String, RequestId), oneshot::Sender<ElicitationResponse>>,
pending_dynamic_tools: HashMap<String, oneshot::Sender<DynamicToolResponse>>,
@@ -109,6 +111,12 @@ pub(crate) struct TurnState {
pub(crate) token_usage_at_turn_start: TokenUsage,
}
pub(crate) struct PendingRequestPermissions {
pub(crate) tx_response: oneshot::Sender<RequestPermissionsResponse>,
pub(crate) requested_permissions: RequestPermissionProfile,
pub(crate) cwd: AbsolutePathBuf,
}
impl TurnState {
pub(crate) fn insert_pending_approval(
&mut self,
@@ -137,15 +145,16 @@ impl TurnState {
pub(crate) fn insert_pending_request_permissions(
&mut self,
key: String,
tx: oneshot::Sender<RequestPermissionsResponse>,
) -> Option<oneshot::Sender<RequestPermissionsResponse>> {
self.pending_request_permissions.insert(key, tx)
pending_request_permissions: PendingRequestPermissions,
) -> Option<PendingRequestPermissions> {
self.pending_request_permissions
.insert(key, pending_request_permissions)
}
pub(crate) fn remove_pending_request_permissions(
&mut self,
key: &str,
) -> Option<oneshot::Sender<RequestPermissionsResponse>> {
) -> Option<PendingRequestPermissions> {
self.pending_request_permissions.remove(key)
}
@@ -259,6 +259,7 @@ async fn effective_patch_permissions(
);
let effective_additional_permissions = apply_granted_turn_permissions(
session,
turn.cwd.as_path(),
crate::sandboxing::SandboxPermissions::UseDefault,
write_permissions_for_paths(&file_paths, &file_system_sandbox_policy, &turn.cwd),
)
+2 -1
View File
@@ -169,6 +169,7 @@ pub(super) fn implicit_granted_permissions(
pub(super) async fn apply_granted_turn_permissions(
session: &Session,
cwd: &std::path::Path,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<PermissionProfile>,
) -> EffectiveAdditionalPermissions {
@@ -192,7 +193,7 @@ pub(super) async fn apply_granted_turn_permissions(
);
let permissions_preapproved = match (effective_permissions.as_ref(), granted_permissions) {
(Some(effective_permissions), Some(granted_permissions)) => {
intersect_permission_profiles(effective_permissions.clone(), granted_permissions)
intersect_permission_profiles(effective_permissions.clone(), granted_permissions, cwd)
== *effective_permissions
}
_ => false,
@@ -424,6 +424,7 @@ impl ShellHandler {
let requested_additional_permissions = additional_permissions.clone();
let effective_additional_permissions = apply_granted_turn_permissions(
session.as_ref(),
turn.cwd.as_path(),
exec_params.sandbox_permissions,
additional_permissions,
)
@@ -230,6 +230,7 @@ impl ToolHandler for UnifiedExecHandler {
let requested_additional_permissions = additional_permissions.clone();
let effective_additional_permissions = apply_granted_turn_permissions(
context.session.as_ref(),
context.turn.cwd.as_path(),
sandbox_permissions,
additional_permissions,
)
@@ -1586,17 +1586,17 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions()
.unwrap_or_else(|| panic!("expected filesystem permissions"));
let (approval_reads, approval_writes) = approval_file_system
.legacy_read_write_roots()
.unwrap_or_default();
.unwrap_or_else(|| panic!("expected legacy-compatible permissions"));
assert!(approval_reads.as_ref().is_none_or(Vec::is_empty));
let mut approval_writes = approval_writes.unwrap_or_default();
approval_writes.sort_by_key(|path| path.display().to_string());
let (_expected_reads, expected_writes) = merged_permissions
let (_, expected_writes) = merged_permissions
.file_system
.unwrap_or_else(|| panic!("expected merged filesystem permissions"))
.legacy_read_write_roots()
.unwrap_or_default();
.unwrap_or_else(|| panic!("expected legacy-compatible permissions"));
let mut expected_writes = expected_writes.unwrap_or_default();
expected_writes.sort_by_key(|path| path.display().to_string());
@@ -1,6 +1,7 @@
use crate::models::FileSystemPermissions;
use crate::models::NetworkPermissions;
use crate::models::PermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;
@@ -70,4 +71,7 @@ pub struct RequestPermissionsEvent {
#[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>,
pub permissions: RequestPermissionProfile,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub cwd: Option<AbsolutePathBuf>,
}
+119 -5
View File
@@ -6,6 +6,7 @@ use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::NetworkAccess;
use codex_protocol::protocol::ReadOnlyAccess;
@@ -14,6 +15,8 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::canonicalize_preserving_symlinks;
use std::collections::HashSet;
use std::num::NonZeroUsize;
use std::path::Path;
use std::path::PathBuf;
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct EffectiveSandboxPermissions {
@@ -146,21 +149,43 @@ pub fn merge_permission_profiles(
pub fn intersect_permission_profiles(
requested: PermissionProfile,
granted: PermissionProfile,
cwd: &Path,
) -> PermissionProfile {
let file_system = requested
.file_system
.map(|requested_file_system| {
let granted_file_system = granted.file_system.unwrap_or_default();
let entries: Vec<_> = requested_file_system
let requested_policy =
FileSystemSandboxPolicy::restricted(requested_file_system.entries.clone());
let mut entries: Vec<_> = granted_file_system
.entries
.into_iter()
.filter(|entry| granted_file_system.entries.contains(entry))
.iter()
.filter(|entry| {
granted_file_system_entry_within_request(
&requested_file_system,
&requested_policy,
entry,
cwd,
)
})
.map(|entry| materialize_cwd_dependent_entry(entry, cwd))
.collect();
for entry in requested_file_system
.entries
.iter()
.chain(granted_file_system.entries.iter())
.filter(|entry| entry.access == FileSystemAccessMode::None)
{
let entry = materialize_cwd_dependent_entry(entry, cwd);
if !entries.contains(&entry) {
entries.push(entry);
}
}
FileSystemPermissions {
glob_scan_max_depth: merge_glob_scan_max_depth(
&entries,
&requested_file_system.entries,
requested_file_system.glob_scan_max_depth.map(usize::from),
&entries,
&granted_file_system.entries,
granted_file_system.glob_scan_max_depth.map(usize::from),
)
.and_then(NonZeroUsize::new),
@@ -230,6 +255,95 @@ enum GlobScanDepth {
Unbounded,
}
fn granted_file_system_entry_within_request(
requested: &FileSystemPermissions,
requested_policy: &FileSystemSandboxPolicy,
granted_entry: &FileSystemSandboxEntry,
cwd: &Path,
) -> bool {
if !granted_entry.access.can_read() {
return false;
}
if let Some(path) = resolve_permission_path(&granted_entry.path, cwd) {
return access_covers(
requested_policy.resolve_access_with_cwd(path.as_path(), cwd),
granted_entry.access,
);
}
requested.entries.iter().any(|requested_entry| {
access_covers(requested_entry.access, granted_entry.access)
&& requested_entry.path == granted_entry.path
})
}
fn access_covers(requested: FileSystemAccessMode, granted: FileSystemAccessMode) -> bool {
match granted {
FileSystemAccessMode::Read => requested.can_read(),
FileSystemAccessMode::Write => requested.can_write(),
FileSystemAccessMode::None => false,
}
}
fn materialize_cwd_dependent_entry(
entry: &FileSystemSandboxEntry,
cwd: &Path,
) -> FileSystemSandboxEntry {
match &entry.path {
FileSystemPath::Special {
value:
FileSystemSpecialPath::CurrentWorkingDirectory
| FileSystemSpecialPath::ProjectRoots { .. },
} => resolve_permission_path(&entry.path, cwd)
.map(|path| FileSystemSandboxEntry {
path: FileSystemPath::Path { path },
access: entry.access,
})
.unwrap_or_else(|| entry.clone()),
FileSystemPath::Path { .. }
| FileSystemPath::GlobPattern { .. }
| FileSystemPath::Special { .. } => entry.clone(),
}
}
fn resolve_permission_path(path: &FileSystemPath, cwd: &Path) -> Option<AbsolutePathBuf> {
match path {
FileSystemPath::Path { path } => Some(path.clone()),
FileSystemPath::GlobPattern { .. } => None,
FileSystemPath::Special { value } => match value {
FileSystemSpecialPath::Root => {
let root = cwd.ancestors().last()?;
AbsolutePathBuf::from_absolute_path(root).ok()
}
FileSystemSpecialPath::CurrentWorkingDirectory => {
AbsolutePathBuf::from_absolute_path(cwd).ok()
}
FileSystemSpecialPath::ProjectRoots { subpath } => {
let cwd = AbsolutePathBuf::from_absolute_path(cwd).ok()?;
Some(match subpath {
Some(subpath) => {
AbsolutePathBuf::resolve_path_against_base(subpath, cwd.as_path())
}
None => cwd,
})
}
FileSystemSpecialPath::Tmpdir => {
let tmpdir = std::env::var_os("TMPDIR")?;
if tmpdir.is_empty() {
None
} else {
AbsolutePathBuf::from_absolute_path(PathBuf::from(tmpdir)).ok()
}
}
FileSystemSpecialPath::SlashTmp => AbsolutePathBuf::from_absolute_path("/tmp")
.ok()
.filter(|path| path.as_path().is_dir()),
FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => None,
},
}
}
fn merge_permission_entries(
base: &[FileSystemSandboxEntry],
permissions: &[FileSystemSandboxEntry],
@@ -244,7 +244,7 @@ fn intersect_permission_profiles_preserves_explicit_empty_requested_reads() {
let granted = requested.clone();
assert_eq!(
intersect_permission_profiles(requested.clone(), granted),
intersect_permission_profiles(requested.clone(), granted, temp_dir.path()),
requested
);
}
@@ -265,7 +265,7 @@ fn intersect_permission_profiles_drops_ungranted_nonempty_path_requests() {
};
assert_eq!(
intersect_permission_profiles(requested, PermissionProfile::default()),
intersect_permission_profiles(requested, PermissionProfile::default(), temp_dir.path()),
PermissionProfile::default()
);
}
@@ -286,13 +286,183 @@ fn intersect_permission_profiles_drops_explicit_empty_reads_without_grant() {
};
assert_eq!(
intersect_permission_profiles(requested, PermissionProfile::default()),
intersect_permission_profiles(requested, PermissionProfile::default(), temp_dir.path()),
PermissionProfile::default()
);
}
#[test]
fn intersect_permission_profiles_accepts_child_path_granted_for_requested_cwd() {
let temp_dir = TempDir::new().expect("create temp dir");
let cwd = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let child = cwd.join("child");
let requested = PermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
let granted = PermissionProfile {
file_system: Some(FileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![child]),
)),
..Default::default()
};
assert_eq!(
intersect_permission_profiles(requested, granted.clone(), cwd.as_path()),
granted
);
}
#[test]
fn intersect_permission_profiles_materializes_cwd_grant_for_reuse() {
let temp_dir = TempDir::new().expect("create temp dir");
let request_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("request-cwd"))
.expect("absolute request cwd");
let later_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("later-cwd"))
.expect("absolute later cwd");
let cwd_write_permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
let intersected = intersect_permission_profiles(
cwd_write_permissions.clone(),
cwd_write_permissions,
request_cwd.as_path(),
);
assert_eq!(
intersected,
PermissionProfile {
file_system: Some(FileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![request_cwd]),
)),
..Default::default()
}
);
assert_eq!(
intersect_permission_profiles(
PermissionProfile {
file_system: Some(FileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![later_cwd.join("child")]),
)),
..Default::default()
},
intersected,
later_cwd.as_path(),
),
PermissionProfile::default()
);
}
#[test]
fn intersect_permission_profiles_materializes_cwd_deny_entries() {
let temp_dir = TempDir::new().expect("create temp dir");
let request_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("request-cwd"))
.expect("absolute request cwd");
let permissions = PermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::None,
},
],
glob_scan_max_depth: None,
}),
..Default::default()
};
assert_eq!(
intersect_permission_profiles(permissions.clone(), permissions, request_cwd.as_path()),
PermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Write,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: request_cwd },
access: FileSystemAccessMode::None,
},
],
glob_scan_max_depth: None,
}),
..Default::default()
}
);
}
#[test]
fn intersect_permission_profiles_drops_broader_cwd_grant_for_requested_child_path() {
let temp_dir = TempDir::new().expect("create temp dir");
let cwd = AbsolutePathBuf::from_absolute_path(
canonicalize(temp_dir.path()).expect("canonicalize temp dir"),
)
.expect("absolute temp dir");
let child = cwd.join("child");
let requested = PermissionProfile {
file_system: Some(FileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![child]),
)),
..Default::default()
};
let granted = PermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
assert_eq!(
intersect_permission_profiles(requested, granted, cwd.as_path()),
PermissionProfile::default()
);
}
#[test]
fn intersect_permission_profiles_uses_granted_bounded_glob_scan_depth() {
let cwd = std::env::current_dir().expect("current dir");
let deny_env_files = FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: "**/*.env".to_string(),
@@ -315,7 +485,7 @@ fn intersect_permission_profiles_uses_granted_bounded_glob_scan_depth() {
};
assert_eq!(
intersect_permission_profiles(requested, granted),
intersect_permission_profiles(requested, granted, cwd.as_path()),
PermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![deny_env_files],
@@ -328,6 +498,7 @@ fn intersect_permission_profiles_uses_granted_bounded_glob_scan_depth() {
#[test]
fn intersect_permission_profiles_uses_granted_unbounded_glob_scan_depth() {
let cwd = std::env::current_dir().expect("current dir");
let deny_env_files = FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: "**/*.env".to_string(),
@@ -350,7 +521,7 @@ fn intersect_permission_profiles_uses_granted_unbounded_glob_scan_depth() {
};
assert_eq!(
intersect_permission_profiles(requested, granted),
intersect_permission_profiles(requested, granted, cwd.as_path()),
PermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![deny_env_files],
+1
View File
@@ -1673,6 +1673,7 @@ fn request_permissions_from_params(
call_id: params.item_id,
reason: params.reason,
permissions: params.permissions.into(),
cwd: None,
}
}