mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
permissions: start using PermissionProfile as the canonical runtime model
This commit is contained in:
@@ -118,6 +118,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;
|
||||
@@ -2566,9 +2567,12 @@ async fn on_request_permissions_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 cwd = conversation.cwd().await;
|
||||
let Some(response) = request_permissions_response_from_client_result(
|
||||
requested_permissions,
|
||||
cwd.as_path(),
|
||||
response,
|
||||
) else {
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -2585,6 +2589,7 @@ async fn on_request_permissions_response(
|
||||
|
||||
fn request_permissions_response_from_client_result(
|
||||
requested_permissions: CoreRequestPermissionProfile,
|
||||
cwd: &Path,
|
||||
response: std::result::Result<ClientRequestResult, oneshot::error::RecvError>,
|
||||
) -> Option<CoreRequestPermissionsResponse> {
|
||||
let value = match response {
|
||||
@@ -2615,15 +2620,25 @@ fn request_permissions_response_from_client_result(
|
||||
}
|
||||
});
|
||||
Some(CoreRequestPermissionsResponse {
|
||||
permissions: intersect_permission_profiles(
|
||||
requested_permissions.into(),
|
||||
response.permissions.into(),
|
||||
)
|
||||
.into(),
|
||||
permissions: granted_request_permissions_within_requested_scope(
|
||||
requested_permissions,
|
||||
CorePermissionProfile::from(response.permissions).into(),
|
||||
cwd,
|
||||
),
|
||||
scope: response.scope.to_core(),
|
||||
})
|
||||
}
|
||||
|
||||
fn granted_request_permissions_within_requested_scope(
|
||||
requested: CoreRequestPermissionProfile,
|
||||
granted: CoreRequestPermissionProfile,
|
||||
cwd: &Path,
|
||||
) -> CoreRequestPermissionProfile {
|
||||
let requested: CorePermissionProfile = requested.into();
|
||||
let granted: CorePermissionProfile = granted.into();
|
||||
intersect_permission_profiles(requested, granted, cwd).into()
|
||||
}
|
||||
|
||||
const REVIEW_FALLBACK_MESSAGE: &str = "Reviewer failed to output a response.";
|
||||
|
||||
fn render_review_output_text(output: &ReviewOutputEvent) -> String {
|
||||
@@ -3673,6 +3688,7 @@ mod tests {
|
||||
|
||||
let response = request_permissions_response_from_client_result(
|
||||
CoreRequestPermissionProfile::default(),
|
||||
Path::new("/tmp"),
|
||||
Ok(Err(error)),
|
||||
);
|
||||
|
||||
@@ -3703,10 +3719,10 @@ mod tests {
|
||||
network: Some(CoreNetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
file_system: Some(CoreFileSystemPermissions {
|
||||
read: Some(vec![absolute_path(input_path)]),
|
||||
write: Some(vec![absolute_path(output_path)]),
|
||||
}),
|
||||
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![absolute_path(input_path)]),
|
||||
Some(vec![absolute_path(output_path)]),
|
||||
)),
|
||||
};
|
||||
let cases = vec![
|
||||
(
|
||||
@@ -3733,10 +3749,10 @@ mod tests {
|
||||
},
|
||||
}),
|
||||
CoreRequestPermissionProfile {
|
||||
file_system: Some(CoreFileSystemPermissions {
|
||||
read: None,
|
||||
write: Some(vec![absolute_path(output_path)]),
|
||||
}),
|
||||
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
|
||||
/*read*/ None,
|
||||
Some(vec![absolute_path(output_path)]),
|
||||
)),
|
||||
..CoreRequestPermissionProfile::default()
|
||||
},
|
||||
),
|
||||
@@ -3751,10 +3767,10 @@ mod tests {
|
||||
},
|
||||
}),
|
||||
CoreRequestPermissionProfile {
|
||||
file_system: Some(CoreFileSystemPermissions {
|
||||
read: Some(vec![absolute_path(input_path)]),
|
||||
write: Some(vec![absolute_path(output_path)]),
|
||||
}),
|
||||
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![absolute_path(input_path)]),
|
||||
Some(vec![absolute_path(output_path)]),
|
||||
)),
|
||||
..CoreRequestPermissionProfile::default()
|
||||
},
|
||||
),
|
||||
@@ -3763,6 +3779,7 @@ mod tests {
|
||||
for (granted_permissions, expected_permissions) in cases {
|
||||
let response = request_permissions_response_from_client_result(
|
||||
requested_permissions.clone(),
|
||||
Path::new("/tmp"),
|
||||
Ok(Ok(serde_json::json!({
|
||||
"permissions": granted_permissions,
|
||||
}))),
|
||||
@@ -3783,6 +3800,7 @@ mod tests {
|
||||
fn request_permissions_response_preserves_session_scope() {
|
||||
let response = request_permissions_response_from_client_result(
|
||||
CoreRequestPermissionProfile::default(),
|
||||
Path::new("/tmp"),
|
||||
Ok(Ok(serde_json::json!({
|
||||
"scope": "session",
|
||||
"permissions": {},
|
||||
@@ -3799,6 +3817,153 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_permissions_response_accepts_explicit_child_grant_for_requested_cwd_scope() {
|
||||
let cwd = TempDir::new().expect("create temp dir");
|
||||
let child =
|
||||
AbsolutePathBuf::try_from(cwd.path().join("child.txt")).expect("absolute child path");
|
||||
let requested_permissions = CoreRequestPermissionProfile {
|
||||
file_system: Some(CoreFileSystemPermissions {
|
||||
entries: vec![codex_protocol::permissions::FileSystemSandboxEntry {
|
||||
path: codex_protocol::permissions::FileSystemPath::Special {
|
||||
value: codex_protocol::permissions::FileSystemSpecialPath::CurrentWorkingDirectory,
|
||||
},
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::Write,
|
||||
}],
|
||||
}),
|
||||
..CoreRequestPermissionProfile::default()
|
||||
};
|
||||
|
||||
let response = request_permissions_response_from_client_result(
|
||||
requested_permissions,
|
||||
cwd.path(),
|
||||
Ok(Ok(serde_json::json!({
|
||||
"permissions": {
|
||||
"fileSystem": {
|
||||
"write": [child],
|
||||
},
|
||||
},
|
||||
}))),
|
||||
)
|
||||
.expect("response should be accepted");
|
||||
|
||||
assert_eq!(
|
||||
response,
|
||||
CoreRequestPermissionsResponse {
|
||||
permissions: CoreRequestPermissionProfile {
|
||||
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
|
||||
/*read*/ None,
|
||||
Some(vec![child]),
|
||||
)),
|
||||
..CoreRequestPermissionProfile::default()
|
||||
},
|
||||
scope: CorePermissionGrantScope::Turn,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_permissions_response_caps_broader_cwd_grant_to_requested_child_path() {
|
||||
let cwd = TempDir::new().expect("create temp dir");
|
||||
let child =
|
||||
AbsolutePathBuf::try_from(cwd.path().join("child.txt")).expect("absolute child path");
|
||||
let requested_permissions = CoreRequestPermissionProfile {
|
||||
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
|
||||
/*read*/ None,
|
||||
Some(vec![child.clone()]),
|
||||
)),
|
||||
..CoreRequestPermissionProfile::default()
|
||||
};
|
||||
|
||||
let response = request_permissions_response_from_client_result(
|
||||
requested_permissions,
|
||||
cwd.path(),
|
||||
Ok(Ok(serde_json::json!({
|
||||
"permissions": {
|
||||
"fileSystem": {
|
||||
"entries": [{
|
||||
"path": {
|
||||
"type": "special",
|
||||
"value": {
|
||||
"kind": "current_working_directory",
|
||||
},
|
||||
},
|
||||
"access": "write",
|
||||
}],
|
||||
},
|
||||
},
|
||||
}))),
|
||||
)
|
||||
.expect("response should be accepted");
|
||||
|
||||
assert_eq!(
|
||||
response,
|
||||
CoreRequestPermissionsResponse {
|
||||
permissions: CoreRequestPermissionProfile {
|
||||
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
|
||||
/*read*/ None,
|
||||
Some(vec![child]),
|
||||
)),
|
||||
..CoreRequestPermissionProfile::default()
|
||||
},
|
||||
scope: CorePermissionGrantScope::Turn,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_permissions_response_preserves_requested_carveouts_when_client_grants_broad_root() {
|
||||
let cwd = TempDir::new().expect("create temp dir");
|
||||
let blocked = AbsolutePathBuf::try_from(cwd.path().join("blocked.txt"))
|
||||
.expect("absolute blocked path");
|
||||
let requested_permissions = CoreRequestPermissionProfile {
|
||||
file_system: Some(CoreFileSystemPermissions {
|
||||
entries: vec![
|
||||
codex_protocol::permissions::FileSystemSandboxEntry {
|
||||
path: codex_protocol::permissions::FileSystemPath::Special {
|
||||
value: codex_protocol::permissions::FileSystemSpecialPath::Root,
|
||||
},
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::Write,
|
||||
},
|
||||
codex_protocol::permissions::FileSystemSandboxEntry {
|
||||
path: codex_protocol::permissions::FileSystemPath::Path { path: blocked },
|
||||
access: codex_protocol::permissions::FileSystemAccessMode::None,
|
||||
},
|
||||
],
|
||||
}),
|
||||
..CoreRequestPermissionProfile::default()
|
||||
};
|
||||
|
||||
let response = request_permissions_response_from_client_result(
|
||||
requested_permissions.clone(),
|
||||
cwd.path(),
|
||||
Ok(Ok(serde_json::json!({
|
||||
"permissions": {
|
||||
"fileSystem": {
|
||||
"entries": [{
|
||||
"path": {
|
||||
"type": "special",
|
||||
"value": {
|
||||
"kind": "root",
|
||||
},
|
||||
},
|
||||
"access": "write",
|
||||
}],
|
||||
},
|
||||
},
|
||||
}))),
|
||||
)
|
||||
.expect("response should be accepted");
|
||||
|
||||
assert_eq!(
|
||||
response,
|
||||
CoreRequestPermissionsResponse {
|
||||
permissions: requested_permissions,
|
||||
scope: CorePermissionGrantScope::Turn,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collab_resume_begin_maps_to_item_started_resume_agent() {
|
||||
let event = CollabResumeBeginEvent {
|
||||
|
||||
@@ -780,6 +780,7 @@ mod tests {
|
||||
codex_app_server_protocol::AdditionalFileSystemPermissions {
|
||||
read: Some(vec![absolute_path("/tmp/allowed")]),
|
||||
write: None,
|
||||
entries: None,
|
||||
},
|
||||
),
|
||||
},
|
||||
@@ -842,6 +843,7 @@ mod tests {
|
||||
codex_app_server_protocol::AdditionalFileSystemPermissions {
|
||||
read: Some(vec![absolute_path("/tmp/allowed")]),
|
||||
write: None,
|
||||
entries: None,
|
||||
},
|
||||
),
|
||||
},
|
||||
@@ -868,7 +870,8 @@ mod tests {
|
||||
"network": null,
|
||||
"fileSystem": {
|
||||
"read": [allowed_path],
|
||||
"write": null,
|
||||
"write": null,
|
||||
"entries": null,
|
||||
},
|
||||
})
|
||||
);
|
||||
|
||||
@@ -93,6 +93,7 @@ async fn request_permissions_round_trip() -> Result<()> {
|
||||
file_system: Some(codex_app_server_protocol::AdditionalFileSystemPermissions {
|
||||
read: None,
|
||||
write: Some(vec![requested_writes[0].clone()]),
|
||||
entries: None,
|
||||
}),
|
||||
},
|
||||
scope: PermissionGrantScope::Turn,
|
||||
|
||||
Reference in New Issue
Block a user