openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-08 15:50:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

sandboxing: intersect permission profiles semantically

Browse Source
This commit is contained in:
Michael Bolin
2026-04-20 19:42:55 -07:00
parent 3d2f123895
commit 8aa3a11ada
8 changed files with 279 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

102
codex-rs/app-server/src/bespoke_event_handling.rs
View File

@@ -2597,9 +2597,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;
};
@@ -2617,6 +2620,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,
@@ -2649,6 +2653,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(),
@@ -3023,6 +3028,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;
@@ -3707,6 +3716,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);
@@ -3793,12 +3803,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");
@@ -3820,6 +3832,7 @@ mod tests {
"scope": "session",
"permissions": {},
}))),
std::env::current_dir().expect("current dir").as_path(),
)
.expect("response should be accepted");
@@ -3832,6 +3845,89 @@ 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(
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 {

1
codex-rs/core/src/tools/handlers/apply_patch.rs
View File

@@ -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),
)

3
codex-rs/core/src/tools/handlers/mod.rs
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,

1
codex-rs/core/src/tools/handlers/shell.rs
View File

@@ -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,
)

1
codex-rs/core/src/tools/handlers/unified_exec.rs
View File

@@ -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,
)

6
codex-rs/core/tests/suite/request_permissions.rs
View File

@@ -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());

102
codex-rs/sandboxing/src/policy_transforms.rs
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,42 @@ 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,
)
})
.cloned()
.collect();
for entry in requested_file_system
.entries
.iter()
.chain(granted_file_system.entries.iter())
.filter(|entry| entry.access == FileSystemAccessMode::None)
{
if !entries.contains(entry) {
entries.push(entry.clone());
}
}
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 +254,74 @@ 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 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],

80
codex-rs/sandboxing/src/policy_transforms_tests.rs
View File

@@ -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,82 @@ 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(
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,
}],
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 +384,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 +397,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 +420,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],