openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-18 17:18:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Resolve permission requests in the executor context (#42146)

Browse Source 
## Why

`request_permissions` paths and grants need to be evaluated against the selected executor environment, including its path convention, home directory, workspace roots, and temporary directories.

## What changed

- Resolve relative and home-relative permission paths using the executor context, reject mismatched path conventions and lossy paths, and support legacy `read` and `write` path lists.
- Move grant intersection into core so requested and granted permissions use the originating environment's sandbox context. Preserve deny entries conservatively when a special path cannot be resolved.
- Keep the full originating environment with pending permission requests so delayed responses are normalized against the same context.

## Testing

- Cover POSIX, Windows, UNC, relative, and home-relative path resolution and invalid path contexts.
- Verify end-to-end app-server grants are limited to the requested workspace scope and unresolved temporary-directory denies are preserved.

GitOrigin-RevId: 730a2aacd391262e92a6314f3a5b6c262e3dca10
This commit is contained in:
iceweasel-oai
2026-09-01 22:08:08 +00:00
committed by copyberry
parent 86b7d12742
commit cd8bd62c6e
10 changed files with 492 additions and 307 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/bespoke_event_handling.rs
+37 -222
View File
@@ -119,7 +119,6 @@ use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequest
use codex_protocol::request_permissions::RequestPermissionsResponse as CoreRequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputAnswer as CoreRequestUserInputAnswer;
use codex_protocol::request_user_input::RequestUserInputResponse as CoreRequestUserInputResponse;
use codex_sandboxing::policy_transforms::intersect_permission_profiles;
use codex_shell_command::parse_command::shlex_join;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::LegacyAppPathString;
@@ -930,8 +929,7 @@ pub(crate) async fn apply_bespoke_event_handling(
let permission_guard = thread_watch_manager
.note_permission_requested(&conversation_id.to_string())
.await;
let requested_permissions = request.permissions.clone();
let request_cwd = match request.cwd.clone() {
let request_cwd = match request.cwd {
Some(cwd) => cwd,
None => conversation.config_snapshot().await.cwd().clone(),
};
@@ -941,7 +939,7 @@ pub(crate) async fn apply_bespoke_event_handling(
item_id: request.call_id.clone(),
environment_id: request.environment_id.clone(),
started_at_ms: request.started_at_ms,
cwd: request_cwd.clone(),
cwd: request_cwd,
reason: request.reason,
permissions: request.permissions.into(),
};
@@ -952,8 +950,6 @@ pub(crate) async fn apply_bespoke_event_handling(
call_id: request.call_id,
conversation_id,
turn_id: request.turn_id,
requested_permissions,
request_cwd,
pending_request_id,
outgoing,
receiver: rx,
@@ -1887,8 +1883,6 @@ async fn on_request_permissions_response(
call_id,
conversation_id,
turn_id,
requested_permissions,
request_cwd,
pending_request_id,
outgoing,
receiver,
@@ -1897,11 +1891,7 @@ async fn on_request_permissions_response(
let response = receiver.await;
resolve_server_request_on_thread_listener(&thread_state, pending_request_id.clone()).await;
drop(request_permissions_guard);
let response = match request_permissions_response_from_client_result(
requested_permissions,
response,
request_cwd.as_path(),
) {
let response = match request_permissions_response_from_client_result(response) {
Ok(Some(response)) => response,
Ok(None) => return,
// TODO(anp): Remove this native-path localization error path once core permission paths
@@ -1944,8 +1934,6 @@ struct PendingRequestPermissionsResponse {
call_id: String,
conversation_id: ThreadId,
turn_id: String,
requested_permissions: CoreRequestPermissionProfile,
request_cwd: AbsolutePathBuf,
pending_request_id: RequestId,
outgoing: ThreadScopedOutgoingMessageSender,
receiver: oneshot::Receiver<ClientRequestResult>,
@@ -1953,9 +1941,7 @@ struct PendingRequestPermissionsResponse {
}
fn request_permissions_response_from_client_result(
requested_permissions: CoreRequestPermissionProfile,
response: std::result::Result<ClientRequestResult, oneshot::error::RecvError>,
cwd: &std::path::Path,
) -> std::io::Result<Option<CoreRequestPermissionsResponse>> {
let value = match response {
Ok(Ok(value)) => value,
@@ -2002,13 +1988,9 @@ fn request_permissions_response_from_client_result(
}));
}
let granted_permissions: CoreAdditionalPermissionProfile = response.permissions.try_into()?;
let permissions = if granted_permissions.is_empty() {
CoreRequestPermissionProfile::default()
} else {
intersect_permission_profiles(requested_permissions.into(), granted_permissions, cwd).into()
};
// Core intersects with the request using the originating environment's policy context.
Ok(Some(CoreRequestPermissionsResponse {
permissions,
permissions: CoreRequestPermissionProfile::from(granted_permissions),
scope: response.scope.to_core(),
strict_auto_review,
}))
@@ -2229,10 +2211,6 @@ mod tests {
use codex_protocol::models::FileSystemPermissions as CoreFileSystemPermissions;
use codex_protocol::models::NetworkPermissions as CoreNetworkPermissions;
use codex_protocol::models::PermissionProfile;
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::AgentMessageEvent;
@@ -3058,18 +3036,14 @@ mod tests {
data: Some(serde_json::json!({ "reason": "turnTransition" })),
};
let response = request_permissions_response_from_client_result(
CoreRequestPermissionProfile::default(),
Ok(Err(error)),
std::env::current_dir().expect("current dir").as_path(),
)
.expect("paths should localize");
let response = request_permissions_response_from_client_result(Ok(Err(error)))
.expect("paths should localize");
assert_eq!(response, None);
}
#[test]
fn request_permissions_response_accepts_partial_network_and_file_system_grants() {
fn request_permissions_response_deserializes_network_and_file_system_grants() {
let input_path = if cfg!(target_os = "windows") {
r"C:\tmp\input"
} else {
@@ -3080,23 +3054,14 @@ mod tests {
} else {
"/tmp/output"
};
let ignored_path = if cfg!(target_os = "windows") {
r"C:\tmp\ignored"
let extra_path = if cfg!(target_os = "windows") {
r"C:\tmp\extra"
} else {
"/tmp/ignored"
"/tmp/extra"
};
let absolute_path = |path: &str| {
AbsolutePathBuf::try_from(std::path::PathBuf::from(path)).expect("absolute path")
};
let requested_permissions = CoreRequestPermissionProfile {
network: Some(CoreNetworkPermissions {
enabled: Some(true),
}),
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path(input_path)]),
Some(vec![absolute_path(output_path)]),
)),
};
let cases = vec![
(
serde_json::json!({}),
@@ -3133,7 +3098,7 @@ mod tests {
serde_json::json!({
"fileSystem": {
"read": [input_path],
"write": [output_path, ignored_path],
"write": [output_path, extra_path],
},
"macos": {
"calendar": true,
@@ -3142,24 +3107,20 @@ mod tests {
CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path(input_path)]),
Some(vec![absolute_path(output_path)]),
Some(vec![absolute_path(output_path), absolute_path(extra_path)]),
)),
..CoreRequestPermissionProfile::default()
},
),
];
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!({
let response =
request_permissions_response_from_client_result(Ok(Ok(serde_json::json!({
"permissions": granted_permissions,
}))),
cwd.as_path(),
)
.expect("paths should localize")
.expect("response should be accepted");
}))))
.expect("paths should localize")
.expect("response should be accepted");
assert_eq!(
response,
@@ -3174,14 +3135,10 @@ mod tests {
#[test]
fn request_permissions_response_preserves_session_scope() {
let response = request_permissions_response_from_client_result(
CoreRequestPermissionProfile::default(),
Ok(Ok(serde_json::json!({
"scope": "session",
"permissions": {},
}))),
std::env::current_dir().expect("current dir").as_path(),
)
let response = request_permissions_response_from_client_result(Ok(Ok(serde_json::json!({
"scope": "session",
"permissions": {},
}))))
.expect("paths should localize")
.expect("response should be accepted");
@@ -3197,19 +3154,15 @@ mod tests {
#[test]
fn request_permissions_response_rejects_session_scoped_strict_auto_review() {
let response = request_permissions_response_from_client_result(
CoreRequestPermissionProfile::default(),
Ok(Ok(serde_json::json!({
"scope": "session",
"strictAutoReview": true,
"permissions": {
"network": {
"enabled": true,
},
let response = request_permissions_response_from_client_result(Ok(Ok(serde_json::json!({
"scope": "session",
"strictAutoReview": true,
"permissions": {
"network": {
"enabled": true,
},
}))),
std::env::current_dir().expect("current dir").as_path(),
)
},
}))))
.expect("paths should localize")
.expect("response should be accepted");
@@ -3225,23 +3178,14 @@ mod tests {
#[test]
fn request_permissions_response_preserves_turn_scoped_strict_auto_review() {
let response = request_permissions_response_from_client_result(
CoreRequestPermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
enabled: Some(true),
}),
..Default::default()
},
Ok(Ok(serde_json::json!({
"strictAutoReview": true,
"permissions": {
"network": {
"enabled": true,
},
let response = request_permissions_response_from_client_result(Ok(Ok(serde_json::json!({
"strictAutoReview": true,
"permissions": {
"network": {
"enabled": true,
},
}))),
std::env::current_dir().expect("current dir").as_path(),
)
},
}))))
.expect("paths should localize")
.expect("response should be accepted");
@@ -3249,135 +3193,6 @@ mod tests {
assert!(response.strict_auto_review);
}
#[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::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
}],
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("paths should localize")
.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::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
}],
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("paths should localize")
.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": "project_roots",
"subpath": null
}
},
"access": "write"
}],
},
},
}))),
cwd.as_path(),
)
.expect("paths should localize")
.expect("response should be accepted");
assert_eq!(
response.permissions,
CoreRequestPermissionProfile::default()
);
}
#[tokio::test]
async fn test_handle_error_records_message() -> Result<()> {
let conversation_id = ThreadId::new();
codex-rs/app-server/tests/suite/v2/request_permissions.rs
+79 -6
View File
@@ -2,7 +2,6 @@ use anyhow::Result;
use app_test_support::MockResponsesConfig;
use app_test_support::TestAppServer;
use app_test_support::create_final_assistant_message_sse_response;
use app_test_support::create_mock_responses_server_sequence;
use app_test_support::create_request_permissions_sse_response;
use codex_app_server_protocol::ClientRequest;
use codex_app_server_protocol::JSONRPCMessage;
@@ -16,7 +15,15 @@ use codex_app_server_protocol::TurnStartParams;
use codex_app_server_protocol::TurnStartResponse;
use codex_app_server_protocol::UserInput as V2UserInput;
use codex_features::Feature;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::request_permissions::PermissionGrantScope as CorePermissionGrantScope;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_utils_path_uri::LegacyAppPathString;
use core_test_support::responses;
use core_test_support::skip_if_wine_exec;
use pretty_assertions::assert_eq;
use serde_json::json;
use tokio::time::timeout;
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10);
@@ -30,11 +37,31 @@ async fn request_permissions_round_trip() -> Result<()> {
);
let codex_home = tempfile::TempDir::new()?;
let project_root_entry = json!({
"path": {
"type": "special",
"value": {"kind": "project_roots", "subpath": "output"}
},
"access": "write"
});
let responses = vec![
create_request_permissions_sse_response("call1")?,
responses::sse(vec![
responses::ev_response_created("resp-2"),
responses::ev_function_call(
"call2",
"request_permissions",
&json!({
"permissions": {"file_system": {"entries": [project_root_entry]}}
})
.to_string(),
),
responses::ev_completed("resp-2"),
]),
create_final_assistant_message_sse_response("done")?,
];
let server = create_mock_responses_server_sequence(responses).await;
let server = responses::start_mock_server().await;
let mock = responses::mount_sse_sequence(&server, responses).await;
MockResponsesConfig::new(&server.uri())
.with_approval_policy("on-request")
.enable_feature(Feature::RequestPermissionsTool)
@@ -44,6 +71,10 @@ async fn request_permissions_round_trip() -> Result<()> {
.with_codex_home(codex_home.path())
.build_initialized()
.await?;
let cwd = mcp.auto_env()?.selection().cwd.clone();
let workspace_root = cwd.parent().expect("test cwd has a parent");
let mut environment = mcp.auto_env_params()?;
environment.runtime_workspace_roots = Some(vec![workspace_root.clone().into()]);
let ThreadStartResponse { thread, .. } = mcp
.start_thread(ThreadStartParams {
@@ -58,6 +89,7 @@ async fn request_permissions_round_trip() -> Result<()> {
params: TurnStartParams {
thread_id: thread.id.clone(),
client_user_message_id: None,
environments: Some(vec![environment]),
input: vec![V2UserInput::Text {
text: "pick a directory".to_string(),
text_elements: Vec::new(),
@@ -108,8 +140,6 @@ async fn request_permissions_round_trip() -> Result<()> {
},
])
);
let resolved_request_id = request_id.clone();
mcp.send_response(
request_id,
serde_json::to_value(PermissionsRequestApprovalResponse {
@@ -128,6 +158,28 @@ async fn request_permissions_round_trip() -> Result<()> {
)
.await?;
let server_req = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_request_message(),
)
.await??;
let ServerRequest::PermissionsRequestApproval { request_id, params } = server_req else {
panic!("expected PermissionsRequestApproval request, got: {server_req:?}");
};
assert_eq!(params.item_id, "call2");
let resolved_request_id = request_id.clone();
let outside_request: LegacyAppPathString = cwd.join("output")?.into();
mcp.send_response(
request_id,
json!({
"permissions": {"fileSystem": {"entries": [
project_root_entry,
{"path": {"type": "path", "path": outside_request}, "access": "write"}
]}}
}),
)
.await?;
let mut saw_resolved = false;
loop {
let message = timeout(DEFAULT_READ_TIMEOUT, mcp.read_next_message()).await??;
@@ -143,8 +195,9 @@ async fn request_permissions_round_trip() -> Result<()> {
.expect("serverRequest/resolved params"),
)?;
assert_eq!(resolved.thread_id, thread.id);
assert_eq!(resolved.request_id, resolved_request_id);
saw_resolved = true;
if resolved.request_id == resolved_request_id {
saw_resolved = true;
}
}
"turn/completed" => {
assert!(saw_resolved, "serverRequest/resolved should arrive first");
@@ -154,5 +207,25 @@ async fn request_permissions_round_trip() -> Result<()> {
}
}
let (output, _) = mock.requests()[2]
.function_call_output_content_and_success("call2")
.expect("permission tool output");
let response: RequestPermissionsResponse =
serde_json::from_str(&output.expect("permission response text"))?;
assert_eq!(
response,
RequestPermissionsResponse {
permissions: RequestPermissionProfile {
file_system: Some(FileSystemPermissions::from_read_write_path_uris(
/*read*/ None,
Some(vec![workspace_root.join("output")?]),
)),
..Default::default()
},
scope: CorePermissionGrantScope::Turn,
strict_auto_review: false,
}
);
Ok(())
}
codex-rs/core/src/session/mod.rs
+53 -34
View File
@@ -2,7 +2,6 @@ use std::borrow::Cow;
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;
@@ -121,6 +120,7 @@ use codex_protocol::models::SandboxEnforcement;
use codex_protocol::openai_models::ModelInfo;
use codex_protocol::openai_models::ModelPreset;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSandboxPolicyContext;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::FileChange;
use codex_protocol::protocol::HasLegacyEvent;
@@ -156,7 +156,7 @@ use codex_rollout::state_db;
use codex_rollout_trace::AgentResultTracePayload;
use codex_rollout_trace::ThreadStartedTraceMetadata;
use codex_rollout_trace::ThreadTraceContext;
use codex_sandboxing::policy_transforms::intersect_permission_profiles;
use codex_sandboxing::policy_transforms::intersect_permission_profiles_with_context;
use codex_shell_command::parse_command::parse_command;
use codex_terminal_detection::user_agent;
use codex_thread_store::CreateThreadParams;
@@ -2757,12 +2757,27 @@ impl Session {
step_context: &Arc<StepContext>,
call_id: String,
args: RequestPermissionsArgs,
environment: TurnEnvironmentSelection,
environment_selection: TurnEnvironmentSelection,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
let turn_context = &step_context.turn;
let approval_policy = step_context.settings.approval_policy();
let approvals_reviewer = step_context.settings.approvals_reviewer();
let Some(environment) = step_context
.environments
.turn_environments()
.find(|candidate| {
candidate.selection.environment_id == environment_selection.environment_id
})
.cloned()
else {
return Some(RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
});
};
let cwd = environment.cwd();
match approval_policy {
AskForApproval::Never => {
return Some(RequestPermissionsResponse {
@@ -2786,19 +2801,14 @@ impl Session {
}
let requested_permissions = args.permissions;
// TODO(anp): Migrate request_permissions to support paths from foreign environments.
let Ok(native_environment_cwd) = environment.cwd.to_abs_path() else {
warn!(
cwd = %environment.cwd,
"request_permissions requires a cwd native to the Codex host"
);
let sandbox_context = environment.sandbox_context(/*additional_permissions*/ None);
let Some(context) = sandbox_context.policy_context() else {
return Some(RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
});
};
if crate::guardian::routes_approval_policy_to_guardian(approval_policy, approvals_reviewer)
{
let originating_turn_state = {
@@ -2868,17 +2878,30 @@ impl Session {
let response = Self::normalize_request_permissions_response(
requested_permissions,
response,
native_environment_cwd.as_path(),
&context,
);
self.record_granted_request_permissions_for_turn(
&response,
&environment.environment_id,
&environment.selection.environment_id,
originating_turn_state.as_ref(),
)
.await;
return Some(response);
}
// The interactive approval event remains host-native until its public
// app-server/TUI boundary migrates in the next stack stage.
let Ok(native_cwd) = cwd.to_abs_path() else {
warn!(
cwd = %cwd,
"request_permissions interactive approval requires a cwd native to the Codex host"
);
return Some(RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
});
};
let _elicitation = self.services.elicitations.register();
let (tx_response, rx_response) = oneshot::channel();
let prev_entry = {
@@ -2905,11 +2928,11 @@ impl Session {
let event = EventMsg::RequestPermissions(RequestPermissionsEvent {
call_id: call_id.clone(),
turn_id: turn_context.sub_id.clone(),
environment_id: Some(environment.environment_id.clone()),
environment_id: Some(environment.selection.environment_id.clone()),
started_at_ms: now_unix_timestamp_ms(),
reason: args.reason,
permissions: requested_permissions,
cwd: Some(native_environment_cwd),
cwd: Some(native_cwd),
});
self.send_event(turn_context.as_ref(), event).await;
tokio::select! {
@@ -3020,29 +3043,25 @@ impl Session {
};
match entry {
Some(entry) => {
// TODO(anp): Migrate request_permissions to support paths from foreign environments.
let response = match entry.environment.cwd.to_abs_path() {
Ok(native_environment_cwd) => Self::normalize_request_permissions_response(
let sandbox_context = entry
.environment
.sandbox_context(/*additional_permissions*/ None);
let response = if let Some(context) = sandbox_context.policy_context() {
Self::normalize_request_permissions_response(
entry.requested_permissions,
response,
native_environment_cwd.as_path(),
),
Err(err) => {
warn!(
cwd = %entry.environment.cwd,
%err,
"request_permissions requires a cwd native to the Codex host"
);
RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
}
&context,
)
} else {
RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
}
};
self.record_granted_request_permissions_for_turn(
&response,
&entry.environment.environment_id,
&entry.environment.selection.environment_id,
originating_turn_state.as_ref(),
)
.await;
@@ -3057,7 +3076,7 @@ impl Session {
fn normalize_request_permissions_response(
requested_permissions: RequestPermissionProfile,
response: RequestPermissionsResponse,
cwd: &Path,
context: &FileSystemSandboxPolicyContext<'_>,
) -> RequestPermissionsResponse {
if response.strict_auto_review && matches!(response.scope, PermissionGrantScope::Session) {
return RequestPermissionsResponse {
@@ -3072,10 +3091,10 @@ impl Session {
}
RequestPermissionsResponse {
permissions: intersect_permission_profiles(
permissions: intersect_permission_profiles_with_context(
requested_permissions.into(),
response.permissions.into(),
cwd,
context,
)
.into(),
scope: response.scope,
codex-rs/core/src/session/tests.rs
+46 -29
View File
@@ -79,6 +79,7 @@ use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSandboxPolicyContext;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::protocol::EnvironmentConfigState;
use codex_protocol::protocol::ErrorEvent;
@@ -7244,6 +7245,13 @@ fn strict_auto_review_session_scope_grants_no_permissions() {
..RequestPermissionProfile::default()
};
let cwd = PathUri::parse("file:///tmp").expect("test cwd should be valid");
let context = FileSystemSandboxPolicyContext {
cwd: &cwd,
workspace_roots: &[],
user_home_dir: None,
temporary_directories: None,
};
let response = Session::normalize_request_permissions_response(
requested_permissions.clone(),
codex_protocol::request_permissions::RequestPermissionsResponse {
@@ -7251,7 +7259,7 @@ fn strict_auto_review_session_scope_grants_no_permissions() {
scope: PermissionGrantScope::Session,
strict_auto_review: true,
},
std::path::Path::new("/tmp"),
&context,
);
assert_eq!(
@@ -7355,7 +7363,7 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
}
#[tokio::test]
async fn request_permissions_tool_resolves_relative_paths_against_selected_environment() {
async fn request_permissions_tool_resolves_legacy_paths_against_selected_environment() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
*session.active_turn.lock().await = Some(ActiveTurn::default());
let environment_cwd = {
@@ -7363,6 +7371,7 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir
let legacy_cwd = turn_context.cwd.clone();
legacy_cwd.join("request-permissions-environment")
};
let environment_home = environment_cwd.join("home");
std::fs::create_dir_all(environment_cwd.as_path()).expect("create environment cwd");
let turn_context_mut = Arc::get_mut(&mut turn_context).expect("single thread settings ref");
Arc::make_mut(&mut turn_context_mut.config)
@@ -7382,8 +7391,9 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir
.expect("primary environment")
.clone();
let current_environment_config = current_environment.config().clone();
turn_context_mut.environments.environments[0] =
TurnEnvironmentState::Ready(TurnEnvironment::new(
let environment = TurnEnvironment {
user_home_dir: Some(PathUri::from_abs_path(&environment_home)),
..TurnEnvironment::new(
TurnEnvironmentSelection {
environment_id: "remote".to_string(),
cwd: PathUri::from_abs_path(&environment_cwd),
@@ -7393,7 +7403,9 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir
current_environment.config_origin,
current_environment.environment,
current_environment.shell,
));
)
};
turn_context_mut.environments.environments[0] = TurnEnvironmentState::Ready(environment);
let call_id = "call-1".to_string();
let handler = RequestPermissionsHandler;
@@ -7422,13 +7434,8 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir
"reason": "need write",
"permissions": {
"file_system": {
"entries": [{
"path": {
"type": "path",
"path": "relative.txt",
},
"access": "write",
}],
"read": null,
"write": ["relative.txt", "~/home-relative.txt"],
},
},
})
@@ -7447,16 +7454,13 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir
panic!("expected request_permissions event");
};
let expected_permissions = RequestPermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: environment_cwd.join("relative.txt").into(),
},
access: FileSystemAccessMode::Write,
missing_path_behavior: None,
}],
glob_scan_max_depth: None,
}),
file_system: Some(FileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![
environment_cwd.join("relative.txt"),
environment_home.join("home-relative.txt"),
]),
)),
..Default::default()
};
assert_eq!(request.environment_id.as_deref(), Some("remote"));
@@ -7479,9 +7483,22 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir
.expect("request_permissions handler should succeed");
}
#[test_case("missing", "unknown turn environment id `missing`"; "unknown environment")]
#[test_case("local", "permission path cannot be represented losslessly"; "lossy path")]
#[tokio::test]
async fn request_permissions_tool_rejects_unknown_environment_id() {
let (session, turn_context) = make_session_and_context().await;
async fn request_permissions_tool_rejects_invalid_requests(
environment_id: &str,
expected_error: &str,
) {
let (session, mut turn_context) = make_session_and_context().await;
Arc::make_mut(&mut turn_context.config)
.permissions
.approval_policy = codex_config::Constrained::allow_any(AskForApproval::Never);
let TurnEnvironmentState::Ready(environment) = &mut turn_context.environments.environments[0]
else {
panic!("turn environment should be ready");
};
environment.selection.cwd = PathUri::parse("file:///workspace/%FF").expect("non-UTF8 cwd");
let turn_context = Arc::new(turn_context);
let step_context = StepContext::for_test(Arc::clone(&turn_context));
let result = RequestPermissionsHandler
@@ -7496,10 +7513,10 @@ async fn request_permissions_tool_rejects_unknown_environment_id() {
source: ToolCallSource::Direct,
payload: ToolPayload::Function {
arguments: json!({
"environment_id": "missing",
"environment_id": environment_id,
"permissions": {
"network": {
"enabled": true,
"file_system": {
"write": ["relative"],
},
},
})
@@ -7509,9 +7526,9 @@ async fn request_permissions_tool_rejects_unknown_environment_id() {
.await;
let Err(FunctionCallError::RespondToModel(output)) = result else {
panic!("expected unknown environment id to be rejected");
panic!("expected invalid request to be rejected");
};
assert_eq!(output, "unknown turn environment id `missing`");
assert_eq!(output, expected_error);
}
#[tokio::test]
codex-rs/core/src/state/turn.rs
+2 -2
View File
@@ -9,7 +9,6 @@ use tokio_util::task::AbortOnDropHandle;
use codex_diagnostics::GaugeGuard;
use codex_protocol::dynamic_tools::DynamicToolResponse;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputResponse;
@@ -23,6 +22,7 @@ use crate::mcp_tool_call::McpToolApprovalMetadata;
use crate::session::TurnInputQueue;
use crate::session::step_context::StepContext;
use crate::session::turn_context::TurnContext;
use crate::session::turn_context::TurnEnvironment;
use crate::tasks::AnySessionTask;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::protocol::McpInvocation;
@@ -109,7 +109,7 @@ pub(crate) struct TurnState {
pub(crate) struct PendingRequestPermissions {
pub(crate) tx_response: oneshot::Sender<RequestPermissionsResponse>,
pub(crate) requested_permissions: RequestPermissionProfile,
pub(crate) environment: TurnEnvironmentSelection,
pub(crate) environment: TurnEnvironment,
}
impl TurnState {
codex-rs/core/src/tools/handlers/request_permissions.rs
+101 -13
View File
@@ -1,5 +1,8 @@
use codex_protocol::permissions::FileSystemSandboxPolicyContext;
use codex_protocol::request_permissions::RequestPermissionsArgs;
use codex_sandboxing::policy_transforms::normalize_additional_permissions;
use codex_sandboxing::policy_transforms::normalize_additional_permissions_with_context;
use codex_utils_path_uri::LegacyAppPathString;
use serde_json::Value;
use crate::function_tool::FunctionCallError;
use crate::tools::context::FunctionToolOutput;
@@ -7,7 +10,6 @@ use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolPayload;
use crate::tools::context::boxed_tool_output;
use crate::tools::handlers::parse_arguments;
use crate::tools::handlers::parse_arguments_with_base_path;
use crate::tools::handlers::resolve_tool_environment;
use crate::tools::handlers::shell_spec::create_request_permissions_tool;
use crate::tools::handlers::shell_spec::request_permissions_tool_description;
@@ -75,19 +77,25 @@ impl RequestPermissionsHandler {
"request_permissions requires a primary environment".to_string(),
));
};
// TODO(anp): Migrate request_permissions parsing and permission profiles to PathUri so
// environment-native foreign paths do not require host conversion.
let native_cwd = turn_environment.cwd().to_abs_path().map_err(|err| {
FunctionCallError::RespondToModel(format!(
"request_permissions cwd `{}` is not native to the Codex host: {err}",
turn_environment.cwd()
))
let sandbox_context =
turn_environment.sandbox_context(/*additional_permissions*/ None);
let context = sandbox_context.policy_context().ok_or_else(|| {
FunctionCallError::RespondToModel(
"request_permissions requires an executor cwd".to_string(),
)
})?;
let mut arguments: Value = parse_arguments(&arguments)?;
resolve_permission_path_strings(&mut arguments, &context)?;
let mut args: RequestPermissionsArgs =
parse_arguments_with_base_path(&arguments, &native_cwd)?;
args.permissions = normalize_additional_permissions(args.permissions.into())
.map(codex_protocol::request_permissions::RequestPermissionProfile::from)
.map_err(FunctionCallError::RespondToModel)?;
serde_json::from_value(arguments).map_err(|err| {
FunctionCallError::RespondToModel(format!(
"failed to parse function arguments: {err}"
))
})?;
args.permissions =
normalize_additional_permissions_with_context(args.permissions.into(), &context)
.map(codex_protocol::request_permissions::RequestPermissionProfile::from)
.map_err(FunctionCallError::RespondToModel)?;
if args.permissions.is_empty() {
return Err(FunctionCallError::RespondToModel(
"request_permissions requires at least one permission".to_string(),
@@ -123,3 +131,83 @@ impl RequestPermissionsHandler {
}
impl CoreToolRuntime for RequestPermissionsHandler {}
fn resolve_permission_path_strings(
arguments: &mut Value,
context: &FileSystemSandboxPolicyContext<'_>,
) -> Result<(), FunctionCallError> {
let Some(file_system) = arguments
.pointer_mut("/permissions/file_system")
.and_then(Value::as_object_mut)
else {
return Ok(());
};
let mut legacy_entries = Vec::new();
for (field, access) in [("read", "read"), ("write", "write")] {
if let Some(paths) = file_system.remove(field) {
if paths.is_null() {
continue;
}
let Value::Array(paths) = paths else {
file_system.insert(field.to_string(), paths);
continue;
};
for mut path in paths {
resolve_permission_path_string(&mut path, context)?;
legacy_entries.push(serde_json::json!({
"path": {"type": "path", "path": path},
"access": access,
}));
}
}
}
if !legacy_entries.is_empty() {
file_system
.entry("entries".to_string())
.or_insert_with(|| Value::Array(Vec::new()))
.as_array_mut()
.ok_or_else(|| {
FunctionCallError::RespondToModel(
"request_permissions file_system.entries must be an array".to_string(),
)
})?
.extend(legacy_entries);
}
if let Some(entries) = file_system.get_mut("entries").and_then(Value::as_array_mut) {
for entry in entries {
if entry.pointer("/path/type").and_then(Value::as_str) == Some("path")
&& let Some(path) = entry.pointer_mut("/path/path")
{
resolve_permission_path_string(path, context)?;
}
}
}
Ok(())
}
fn resolve_permission_path_string(
value: &mut Value,
context: &FileSystemSandboxPolicyContext<'_>,
) -> Result<(), FunctionCallError> {
let Some(path) = value.as_str() else {
return Ok(());
};
let cwd = context.cwd;
let convention = cwd.infer_path_convention().ok_or_else(|| {
FunctionCallError::RespondToModel(format!(
"request_permissions cwd `{cwd}` has no path convention"
))
})?;
let path = LegacyAppPathString::from_string(path)
.resolve_against(cwd, context.user_home_dir)
.map_err(|err| FunctionCallError::RespondToModel(err.to_string()))?;
if path.is_opaque() || std::str::from_utf8(&path.decoded_path_bytes()).is_err() {
return Err(FunctionCallError::RespondToModel(
"permission path cannot be represented losslessly".to_string(),
));
}
let path = LegacyAppPathString::from_path_uri(&path, convention)
.map_err(|err| FunctionCallError::RespondToModel(err.to_string()))?;
*value = Value::String(path.into_string());
Ok(())
}
codex-rs/sandboxing/src/policy_transforms.rs
+1 -1
View File
@@ -411,7 +411,7 @@ fn deny_entry_constrains_accepted_grant(
return true;
}
let Some(grant_path) = resolve_permission_path(&entry.path, context) else {
return false;
return true;
};
match &deny_entry.path {
FileSystemPath::GlobPattern { pattern } => {
codex-rs/sandboxing/src/policy_transforms_tests.rs
+41
View File
@@ -562,6 +562,47 @@ fn intersect_permission_profiles_preserves_deny_across_case_variant_grant() {
);
}
#[test]
fn intersect_permission_profiles_preserves_denies_for_unresolved_tmpdir() {
let cwd = PathUri::parse("file:///workspace").expect("cwd URI");
let context = FileSystemSandboxPolicyContext {
cwd: &cwd,
workspace_roots: std::slice::from_ref(&cwd),
user_home_dir: None,
temporary_directories: None,
};
let write = FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::Tmpdir,
},
FileSystemAccessMode::Write,
);
let deny = FileSystemSandboxEntry::new(
PathUri::parse("file:///tmp/private").expect("deny").into(),
FileSystemAccessMode::Deny,
);
let profile = |entries| PermissionProfile {
file_system: Some(FileSystemPermissions {
entries,
glob_scan_max_depth: None,
}),
..Default::default()
};
let grant = profile(vec![write.clone()]);
let grant_with_deny = profile(vec![write, deny]);
for (requested, granted) in [
(grant_with_deny.clone(), grant.clone()),
(grant, grant_with_deny.clone()),
(grant_with_deny.clone(), grant_with_deny.clone()),
] {
assert_eq!(
intersect_permission_profiles_with_context(requested, granted, &context),
grant_with_deny
);
}
}
#[test]
fn intersect_permission_profiles_preserves_rooted_first_segment_glob_deny() {
use FileSystemAccessMode::Deny;
codex-rs/utils/path-uri/src/api_path_string.rs
+64
View File
@@ -1,5 +1,6 @@
use crate::PathConvention;
use crate::PathUri;
use crate::PathUriParseError;
use crate::is_windows_separator_byte;
use codex_utils_absolute_path::AbsolutePathBuf;
use schemars::JsonSchema;
@@ -93,6 +94,54 @@ impl LegacyAppPathString {
})
}
/// Resolves this raw API path spelling against an executor cwd.
///
/// Relative paths use the cwd's inferred convention. Home-relative paths
/// use the supplied executor home, and clearly foreign absolute paths are
/// rejected rather than reinterpreted as relative path text.
pub fn resolve_against(
&self,
cwd: &PathUri,
user_home_dir: Option<&PathUri>,
) -> Result<PathUri, LegacyAppPathStringError> {
let convention = cwd.infer_path_convention().ok_or_else(|| {
LegacyAppPathStringError::MissingBaseConvention {
cwd: cwd.to_string(),
}
})?;
let is_windows = convention == PathConvention::Windows;
let path = self.as_str();
let home_relative = path
.strip_prefix("~/")
.or_else(|| (path == "~").then_some(""))
.or_else(|| is_windows.then(|| path.strip_prefix(r"~\")).flatten());
if let Some(suffix) = home_relative {
let home =
user_home_dir.ok_or_else(|| LegacyAppPathStringError::MissingHomeDirectory {
path: path.to_string(),
})?;
return Ok(home.join(suffix.trim_start_matches(|separator| {
separator == '/' || is_windows && separator == '\\'
}))?);
}
if is_windows && (path.starts_with("//") || path.starts_with(r"\\")) {
return self.to_path_uri(PathConvention::Windows);
}
match self.infer_absolute_path_convention() {
Some(path_convention) if path_convention == convention => self.to_path_uri(convention),
Some(PathConvention::Posix) if is_windows => Ok(cwd.join(path)?),
Some(path_convention) => Err(LegacyAppPathStringError::MismatchedConvention {
path: path.to_string(),
path_convention,
cwd: cwd.to_string(),
convention,
}),
None => Ok(cwd.join(path)?),
}
}
/// Parses this API string as an absolute path using the convention inferred from its spelling.
pub fn to_inferred_path_uri(&self) -> Option<PathUri> {
PathUri::try_from(self.clone()).ok()
@@ -364,6 +413,21 @@ pub enum LegacyAppPathStringError {
path: String,
convention: Option<PathConvention>,
},
#[error("path URI `{cwd}` has no path convention")]
MissingBaseConvention { cwd: String },
#[error("cannot resolve home-relative path `{path}` without an executor home")]
MissingHomeDirectory { path: String },
#[error(
"path {path} uses {path_convention} paths, but executor cwd {cwd} uses {convention} paths"
)]
MismatchedConvention {
path: String,
path_convention: PathConvention,
cwd: String,
convention: PathConvention,
},
#[error(transparent)]
PathUri(#[from] PathUriParseError),
}
#[cfg(test)]
codex-rs/utils/path-uri/src/api_path_string_tests.rs
+68
View File
@@ -466,6 +466,74 @@ fn converts_absolute_api_paths_using_the_inferred_convention() {
}
}
#[test]
fn resolves_legacy_paths_against_executor_context() {
for (cwd, path, user_home_dir, expected) in [
(
"file:///workspace",
"relative.txt",
None,
"file:///workspace/relative.txt",
),
(
"file:///C:/workspace",
"/Windows",
None,
"file:///C:/Windows",
),
(
"file:///C:/workspace",
"//server/share/file.txt",
None,
"file://server/share/file.txt",
),
(
"file:///workspace",
"~//notes",
Some("file:///home/executor"),
"file:///home/executor/notes",
),
(
"file:///C:/workspace",
r"~\\notes",
Some("file:///C:/Users/executor"),
"file:///C:/Users/executor/notes",
),
] {
let cwd = PathUri::parse(cwd).expect("valid cwd");
let user_home_dir = user_home_dir.map(|home| PathUri::parse(home).expect("valid home"));
assert_eq!(
LegacyAppPathString::from_string(path).resolve_against(&cwd, user_home_dir.as_ref()),
Ok(PathUri::parse(expected).expect("valid expected path")),
"resolving {path:?} against {cwd}"
);
}
}
#[test]
fn rejects_legacy_paths_without_executor_context() {
let cwd = PathUri::parse("file:///workspace").expect("valid cwd");
assert_eq!(
LegacyAppPathString::from_string(r"C:\\tmp")
.resolve_against(&cwd, /*user_home_dir*/ None),
Err(LegacyAppPathStringError::MismatchedConvention {
path: r"C:\\tmp".to_string(),
path_convention: PathConvention::Windows,
cwd: cwd.to_string(),
convention: PathConvention::Posix,
})
);
assert_eq!(
LegacyAppPathString::from_string("~/secret")
.resolve_against(&cwd, /*user_home_dir*/ None),
Err(LegacyAppPathStringError::MissingHomeDirectory {
path: "~/secret".to_string(),
})
);
}
#[test]
fn ambiguous_absolute_api_paths_preserve_their_inferred_convention() {
for (raw_path, convention) in [