mirror of
https://github.com/openai/codex.git
synced 2026-09-09 15:58:47 +00:00
sandboxing: intersect permission profiles semantically
This commit is contained in:
@@ -2566,9 +2566,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.config_snapshot().await.cwd;
|
||||
let Some(response) = request_permissions_response_from_client_result(
|
||||
requested_permissions,
|
||||
response,
|
||||
cwd.as_path(),
|
||||
) else {
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -2586,6 +2589,7 @@ async fn on_request_permissions_response(
|
||||
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,
|
||||
@@ -2618,6 +2622,7 @@ fn request_permissions_response_from_client_result(
|
||||
permissions: intersect_permission_profiles(
|
||||
requested_permissions.into(),
|
||||
response.permissions.into(),
|
||||
cwd,
|
||||
)
|
||||
.into(),
|
||||
scope: response.scope.to_core(),
|
||||
@@ -2992,6 +2997,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;
|
||||
@@ -3676,6 +3685,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);
|
||||
@@ -3705,10 +3715,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![
|
||||
(
|
||||
@@ -3735,10 +3745,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(
|
||||
None,
|
||||
Some(vec![absolute_path(output_path)]),
|
||||
)),
|
||||
..CoreRequestPermissionProfile::default()
|
||||
},
|
||||
),
|
||||
@@ -3753,21 +3763,23 @@ 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()
|
||||
},
|
||||
),
|
||||
];
|
||||
|
||||
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");
|
||||
|
||||
@@ -3789,6 +3801,7 @@ mod tests {
|
||||
"scope": "session",
|
||||
"permissions": {},
|
||||
}))),
|
||||
std::env::current_dir().expect("current dir").as_path(),
|
||||
)
|
||||
.expect("response should be accepted");
|
||||
|
||||
@@ -3801,6 +3814,88 @@ 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,
|
||||
}],
|
||||
}),
|
||||
..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(
|
||||
None,
|
||||
Some(vec![child]),
|
||||
)),
|
||||
..Default::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(
|
||||
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 {
|
||||
|
||||
@@ -226,6 +226,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),
|
||||
)
|
||||
|
||||
@@ -148,7 +148,12 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() {
|
||||
dunce::simplified(&outside.canonicalize().expect("canonicalize outside dir")).abs();
|
||||
|
||||
assert_eq!(
|
||||
permissions.and_then(|profile| profile.file_system.and_then(|fs| fs.write)),
|
||||
permissions.and_then(|profile| {
|
||||
profile.file_system.and_then(|fs| {
|
||||
let (_, write) = fs.legacy_read_write_roots()?;
|
||||
write
|
||||
})
|
||||
}),
|
||||
Some(vec![expected_outside])
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
@@ -188,10 +188,10 @@ fn exec_command_args_resolve_relative_additional_permissions_against_workdir() -
|
||||
assert_eq!(
|
||||
args.additional_permissions,
|
||||
Some(PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: None,
|
||||
write: Some(vec![expected_write.abs()]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
None,
|
||||
Some(vec![expected_write.abs()]),
|
||||
)),
|
||||
..Default::default()
|
||||
})
|
||||
);
|
||||
|
||||
@@ -82,10 +82,10 @@ fn file_system_sandbox_context_uses_active_attempt() {
|
||||
.abs();
|
||||
let additional_permissions = PermissionProfile {
|
||||
network: None,
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![path.clone()]),
|
||||
write: Some(Vec::new()),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![path.clone()]),
|
||||
Some(Vec::new()),
|
||||
)),
|
||||
};
|
||||
let req = ApplyPatchRequest {
|
||||
action: ApplyPatchAction::new_add_for_test(&path, "hello".to_string()),
|
||||
|
||||
@@ -258,12 +258,12 @@ fn map_exec_result_preserves_stdout_and_stderr() {
|
||||
#[test]
|
||||
fn shell_request_escalation_execution_is_explicit() {
|
||||
let requested_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: None,
|
||||
write: Some(vec![
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
None,
|
||||
Some(vec![
|
||||
AbsolutePathBuf::from_absolute_path("/tmp/output").unwrap(),
|
||||
]),
|
||||
}),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
|
||||
|
||||
@@ -293,20 +293,20 @@ fn workspace_write_excluding_tmp() -> SandboxPolicy {
|
||||
|
||||
fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfile {
|
||||
RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![absolute_path(path)]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![absolute_path(path)]),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
}
|
||||
}
|
||||
|
||||
fn normalized_directory_write_permissions(path: &Path) -> Result<RequestPermissionProfile> {
|
||||
Ok(RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
})
|
||||
}
|
||||
@@ -343,10 +343,10 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res
|
||||
let call_id = "request_permissions_skip_approval";
|
||||
let command = "touch requested-dir/requested-but-unused.txt";
|
||||
let requested_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![absolute_path(&requested_dir_canonical)]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![absolute_path(&requested_dir_canonical)]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let event = shell_event_with_request_permissions(call_id, command, &requested_permissions)?;
|
||||
@@ -521,10 +521,10 @@ async fn relative_additional_permissions_resolve_against_tool_workdir() -> Resul
|
||||
let call_id = "request_permissions_relative_workdir";
|
||||
let command = "touch relative-write.txt";
|
||||
let expected_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: None,
|
||||
write: Some(vec![absolute_path(&nested_dir_canonical)]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
None,
|
||||
Some(vec![absolute_path(&nested_dir_canonical)]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let event = shell_event_with_raw_request_permissions(
|
||||
@@ -624,10 +624,10 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_cwd
|
||||
"cwd-widened", unrequested_write, unrequested_write
|
||||
);
|
||||
let requested_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![absolute_path(&requested_write)]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![absolute_path(&requested_write)]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?;
|
||||
@@ -725,10 +725,10 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_tmp
|
||||
"tmp-widened", tmp_write, tmp_write
|
||||
);
|
||||
let requested_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![absolute_path(&requested_write)]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![absolute_path(&requested_write)]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?;
|
||||
@@ -824,19 +824,19 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() ->
|
||||
"outside-cwd-ok", outside_write, outside_write
|
||||
);
|
||||
let requested_permissions = RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![absolute_path(outside_dir.path())]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![absolute_path(outside_dir.path())]),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
};
|
||||
let normalized_requested_permissions = RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![AbsolutePathBuf::try_from(
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![AbsolutePathBuf::try_from(
|
||||
outside_dir.path().canonicalize()?,
|
||||
)?]),
|
||||
}),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
};
|
||||
let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?;
|
||||
@@ -926,19 +926,19 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul
|
||||
"should-not-write", outside_write, outside_write
|
||||
);
|
||||
let requested_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![absolute_path(outside_dir.path())]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![absolute_path(outside_dir.path())]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let normalized_requested_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![AbsolutePathBuf::try_from(
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![AbsolutePathBuf::try_from(
|
||||
outside_dir.path().canonicalize()?,
|
||||
)?]),
|
||||
}),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?;
|
||||
@@ -1028,19 +1028,19 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul
|
||||
"sticky-grant-ok", outside_write, outside_write
|
||||
);
|
||||
let requested_permissions = RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![absolute_path(outside_dir.path())]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![absolute_path(outside_dir.path())]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let normalized_requested_permissions = RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![AbsolutePathBuf::try_from(
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![AbsolutePathBuf::try_from(
|
||||
outside_dir.path().canonicalize()?,
|
||||
)?]),
|
||||
}),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
let responses = mount_sse_sequence(
|
||||
@@ -1492,35 +1492,35 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions()
|
||||
);
|
||||
|
||||
let requested_permissions = RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![
|
||||
absolute_path(first_dir.path()),
|
||||
absolute_path(second_dir.path()),
|
||||
]),
|
||||
}),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
};
|
||||
let normalized_requested_permissions = RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![
|
||||
AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?,
|
||||
AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?,
|
||||
]),
|
||||
}),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
};
|
||||
let granted_permissions = normalized_directory_write_permissions(first_dir.path())?;
|
||||
let second_dir_permissions = requested_directory_write_permissions(second_dir.path());
|
||||
let merged_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![
|
||||
AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?,
|
||||
AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?,
|
||||
]),
|
||||
}),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
@@ -1584,16 +1584,20 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions()
|
||||
let approval_file_system = approval_permissions
|
||||
.file_system
|
||||
.unwrap_or_else(|| panic!("expected filesystem permissions"));
|
||||
assert!(approval_file_system.read.as_ref().is_none_or(Vec::is_empty));
|
||||
let (approval_reads, approval_writes) = approval_file_system
|
||||
.legacy_read_write_roots()
|
||||
.unwrap_or_else(|| panic!("expected legacy-compatible permissions"));
|
||||
assert!(approval_reads.as_ref().is_none_or(Vec::is_empty));
|
||||
|
||||
let mut approval_writes = approval_file_system.write.unwrap_or_default();
|
||||
let mut approval_writes = approval_writes.unwrap_or_default();
|
||||
approval_writes.sort_by_key(|path| path.display().to_string());
|
||||
|
||||
let mut expected_writes = merged_permissions
|
||||
let (_, expected_writes) = merged_permissions
|
||||
.file_system
|
||||
.unwrap_or_else(|| panic!("expected merged filesystem permissions"))
|
||||
.write
|
||||
.unwrap_or_default();
|
||||
.legacy_read_write_roots()
|
||||
.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());
|
||||
|
||||
assert_eq!(approval_writes, expected_writes);
|
||||
|
||||
@@ -81,20 +81,20 @@ fn workspace_write_excluding_tmp() -> SandboxPolicy {
|
||||
|
||||
fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfile {
|
||||
RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![absolute_path(path)]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![absolute_path(path)]),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
}
|
||||
}
|
||||
|
||||
fn normalized_directory_write_permissions(path: &Path) -> Result<RequestPermissionProfile> {
|
||||
Ok(RequestPermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![]),
|
||||
write: Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
Some(vec![]),
|
||||
Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]),
|
||||
)),
|
||||
..RequestPermissionProfile::default()
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::models::NetworkPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
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;
|
||||
@@ -12,6 +14,8 @@ use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::canonicalize_preserving_symlinks;
|
||||
use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct EffectiveSandboxPermissions {
|
||||
@@ -125,15 +129,26 @@ 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 = requested_file_system
|
||||
let requested_policy =
|
||||
FileSystemSandboxPolicy::restricted(requested_file_system.entries.clone());
|
||||
let entries = granted
|
||||
.file_system
|
||||
.unwrap_or_default()
|
||||
.entries
|
||||
.into_iter()
|
||||
.filter(|entry| granted_file_system.entries.contains(entry))
|
||||
.filter(|entry| {
|
||||
granted_file_system_entry_within_request(
|
||||
&requested_file_system,
|
||||
&requested_policy,
|
||||
entry,
|
||||
cwd,
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
FileSystemPermissions { entries }
|
||||
})
|
||||
@@ -158,6 +173,74 @@ pub fn intersect_permission_profiles(
|
||||
}
|
||||
}
|
||||
|
||||
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 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],
|
||||
|
||||
@@ -189,7 +189,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
|
||||
);
|
||||
}
|
||||
@@ -210,7 +210,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()
|
||||
);
|
||||
}
|
||||
@@ -231,7 +231,73 @@ 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,
|
||||
}],
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
let granted = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions::from_read_write_roots(
|
||||
None,
|
||||
Some(vec![child.clone()]),
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
intersect_permission_profiles(requested, granted.clone(), cwd.as_path()),
|
||||
granted
|
||||
);
|
||||
}
|
||||
|
||||
#[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(
|
||||
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,
|
||||
}],
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
intersect_permission_profiles(requested, granted, cwd.as_path()),
|
||||
PermissionProfile::default()
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user