Skip to content

Commit d48125c

Browse files
bolinfestbolinfest
committed
sandboxing: materialize cwd-relative permission globs
1 parent 6f69977 commit d48125c

3 files changed

Lines changed: 220 additions & 23 deletions

File tree

‎codex-rs/core/src/tools/handlers/mod.rs‎

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,7 @@ pub(super) async fn apply_granted_turn_permissions(
193193
);
194194
let permissions_preapproved = match (effective_permissions.as_ref(), granted_permissions) {
195195
(Some(effective_permissions), Some(granted_permissions)) => {
196-
intersect_permission_profiles(effective_permissions.clone(), granted_permissions, cwd)
197-
== *effective_permissions
196+
permissions_are_preapproved(effective_permissions, granted_permissions, cwd)
198197
}
199198
_ => false,
200199
};
@@ -213,17 +212,38 @@ pub(super) async fn apply_granted_turn_permissions(
213212
}
214213
}
215214

215+
fn permissions_are_preapproved(
216+
effective_permissions: &PermissionProfile,
217+
granted_permissions: PermissionProfile,
218+
cwd: &Path,
219+
) -> bool {
220+
let materialized_effective_permissions = intersect_permission_profiles(
221+
effective_permissions.clone(),
222+
effective_permissions.clone(),
223+
cwd,
224+
);
225+
intersect_permission_profiles(effective_permissions.clone(), granted_permissions, cwd)
226+
== materialized_effective_permissions
227+
}
228+
216229
#[cfg(test)]
217230
mod tests {
218231
use super::EffectiveAdditionalPermissions;
219232
use super::implicit_granted_permissions;
220233
use super::normalize_and_validate_additional_permissions;
234+
use super::permissions_are_preapproved;
221235
use crate::sandboxing::SandboxPermissions;
222236
use codex_protocol::models::FileSystemPermissions;
223237
use codex_protocol::models::NetworkPermissions;
224238
use codex_protocol::models::PermissionProfile;
239+
use codex_protocol::permissions::FileSystemAccessMode;
240+
use codex_protocol::permissions::FileSystemPath;
241+
use codex_protocol::permissions::FileSystemSandboxEntry;
242+
use codex_protocol::permissions::FileSystemSpecialPath;
225243
use codex_protocol::protocol::AskForApproval;
226244
use codex_protocol::protocol::GranularApprovalConfig;
245+
use codex_sandboxing::policy_transforms::intersect_permission_profiles;
246+
use codex_sandboxing::policy_transforms::merge_permission_profiles;
227247
use codex_utils_absolute_path::AbsolutePathBuf;
228248
use pretty_assertions::assert_eq;
229249
use tempfile::tempdir;
@@ -326,4 +346,43 @@ mod tests {
326346

327347
assert_eq!(implicit_permissions, None);
328348
}
349+
350+
#[test]
351+
fn relative_deny_glob_grants_remain_preapproved_after_materialization() {
352+
let cwd = tempdir().expect("tempdir");
353+
let requested_permissions = PermissionProfile {
354+
file_system: Some(FileSystemPermissions {
355+
entries: vec![
356+
FileSystemSandboxEntry {
357+
path: FileSystemPath::Special {
358+
value: FileSystemSpecialPath::CurrentWorkingDirectory,
359+
},
360+
access: FileSystemAccessMode::Write,
361+
},
362+
FileSystemSandboxEntry {
363+
path: FileSystemPath::GlobPattern {
364+
pattern: "**/*.env".to_string(),
365+
},
366+
access: FileSystemAccessMode::None,
367+
},
368+
],
369+
glob_scan_max_depth: None,
370+
}),
371+
..Default::default()
372+
};
373+
let stored_grant = intersect_permission_profiles(
374+
requested_permissions.clone(),
375+
requested_permissions.clone(),
376+
cwd.path(),
377+
);
378+
let effective_permissions =
379+
merge_permission_profiles(Some(&requested_permissions), Some(&stored_grant))
380+
.expect("merged permissions");
381+
382+
assert!(permissions_are_preapproved(
383+
&effective_permissions,
384+
stored_grant,
385+
cwd.path(),
386+
));
387+
}
329388
}

‎codex-rs/sandboxing/src/policy_transforms.rs‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -159,20 +159,21 @@ pub fn intersect_permission_profiles(
159159
let requested_policy =
160160
FileSystemSandboxPolicy::restricted(requested_file_system.entries.clone());
161161
let requested_read_deny_matcher = ReadDenyMatcher::new(&requested_policy, cwd);
162-
let accepted_entries: Vec<_> = granted_file_system
163-
.entries
164-
.iter()
165-
.filter(|entry| {
166-
granted_file_system_entry_within_request(
167-
&requested_file_system,
168-
&requested_policy,
169-
requested_read_deny_matcher.as_ref(),
170-
entry,
171-
cwd,
172-
)
173-
})
174-
.map(|entry| materialize_cwd_dependent_entry(entry, cwd))
175-
.collect();
162+
let mut accepted_entries = Vec::new();
163+
for entry in granted_file_system.entries.iter().filter(|entry| {
164+
granted_file_system_entry_within_request(
165+
&requested_file_system,
166+
&requested_policy,
167+
requested_read_deny_matcher.as_ref(),
168+
entry,
169+
cwd,
170+
)
171+
}) {
172+
let entry = materialize_cwd_dependent_entry(entry, cwd);
173+
if !accepted_entries.contains(&entry) {
174+
accepted_entries.push(entry);
175+
}
176+
}
176177
let mut entries = accepted_entries.clone();
177178
let requested_retained_deny_entries = retain_constraining_deny_entries(
178179
&requested_file_system.entries,
@@ -383,9 +384,15 @@ fn materialize_cwd_dependent_entry(
383384
access: entry.access,
384385
})
385386
.unwrap_or_else(|| entry.clone()),
386-
FileSystemPath::Path { .. }
387-
| FileSystemPath::GlobPattern { .. }
388-
| FileSystemPath::Special { .. } => entry.clone(),
387+
FileSystemPath::GlobPattern { pattern } => FileSystemSandboxEntry {
388+
path: FileSystemPath::GlobPattern {
389+
pattern: AbsolutePathBuf::resolve_path_against_base(pattern, cwd)
390+
.to_string_lossy()
391+
.into_owned(),
392+
},
393+
access: entry.access,
394+
},
395+
FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => entry.clone(),
389396
}
390397
}
391398

‎codex-rs/sandboxing/src/policy_transforms_tests.rs‎

Lines changed: 135 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,42 @@ fn intersect_permission_profiles_materializes_cwd_grant_for_reuse() {
377377
);
378378
}
379379

380+
#[test]
381+
fn intersect_permission_profiles_deduplicates_materialized_grants() {
382+
let temp_dir = TempDir::new().expect("create temp dir");
383+
let cwd =
384+
AbsolutePathBuf::from_absolute_path(temp_dir.path().join("cwd")).expect("absolute cwd");
385+
let permissions = PermissionProfile {
386+
file_system: Some(FileSystemPermissions {
387+
entries: vec![
388+
FileSystemSandboxEntry {
389+
path: FileSystemPath::Special {
390+
value: FileSystemSpecialPath::CurrentWorkingDirectory,
391+
},
392+
access: FileSystemAccessMode::Write,
393+
},
394+
FileSystemSandboxEntry {
395+
path: FileSystemPath::Path { path: cwd.clone() },
396+
access: FileSystemAccessMode::Write,
397+
},
398+
],
399+
glob_scan_max_depth: None,
400+
}),
401+
..Default::default()
402+
};
403+
404+
assert_eq!(
405+
intersect_permission_profiles(permissions.clone(), permissions, cwd.as_path()),
406+
PermissionProfile {
407+
file_system: Some(FileSystemPermissions::from_read_write_roots(
408+
/*read*/ None,
409+
Some(vec![cwd]),
410+
)),
411+
..Default::default()
412+
}
413+
);
414+
}
415+
380416
#[test]
381417
fn intersect_permission_profiles_materializes_cwd_deny_entries() {
382418
let temp_dir = TempDir::new().expect("create temp dir");
@@ -509,6 +545,75 @@ fn intersect_permission_profiles_rejects_concrete_grants_matched_by_requested_de
509545
);
510546
}
511547

548+
#[test]
549+
fn intersect_permission_profiles_materializes_relative_deny_globs_for_reuse() {
550+
let temp_dir = TempDir::new().expect("create temp dir");
551+
let request_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("request-cwd"))
552+
.expect("absolute request cwd");
553+
let later_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("later-cwd"))
554+
.expect("absolute later cwd");
555+
let cwd_write = FileSystemSandboxEntry {
556+
path: FileSystemPath::Special {
557+
value: FileSystemSpecialPath::CurrentWorkingDirectory,
558+
},
559+
access: FileSystemAccessMode::Write,
560+
};
561+
let deny_env_files = FileSystemSandboxEntry {
562+
path: FileSystemPath::GlobPattern {
563+
pattern: "**/*.env".to_string(),
564+
},
565+
access: FileSystemAccessMode::None,
566+
};
567+
let permissions = PermissionProfile {
568+
file_system: Some(FileSystemPermissions {
569+
entries: vec![cwd_write, deny_env_files],
570+
glob_scan_max_depth: std::num::NonZeroUsize::new(2),
571+
}),
572+
..Default::default()
573+
};
574+
575+
let intersected =
576+
intersect_permission_profiles(permissions.clone(), permissions, request_cwd.as_path());
577+
578+
assert_eq!(
579+
intersected,
580+
PermissionProfile {
581+
file_system: Some(FileSystemPermissions {
582+
entries: vec![
583+
FileSystemSandboxEntry {
584+
path: FileSystemPath::Path {
585+
path: request_cwd.clone(),
586+
},
587+
access: FileSystemAccessMode::Write,
588+
},
589+
FileSystemSandboxEntry {
590+
path: FileSystemPath::GlobPattern {
591+
pattern: request_cwd.join("**/*.env").to_string_lossy().into_owned(),
592+
},
593+
access: FileSystemAccessMode::None,
594+
},
595+
],
596+
glob_scan_max_depth: std::num::NonZeroUsize::new(2),
597+
}),
598+
..Default::default()
599+
}
600+
);
601+
assert_eq!(
602+
intersect_permission_profiles(
603+
PermissionProfile {
604+
file_system: Some(FileSystemPermissions::from_read_write_roots(
605+
/*read*/ None,
606+
Some(vec![later_cwd.join("token.env")]),
607+
)),
608+
..Default::default()
609+
},
610+
intersected,
611+
later_cwd.as_path(),
612+
),
613+
PermissionProfile::default()
614+
);
615+
}
616+
512617
#[test]
513618
fn intersect_permission_profiles_drops_broader_cwd_grant_for_requested_child_path() {
514619
let temp_dir = TempDir::new().expect("create temp dir");
@@ -567,7 +672,7 @@ fn intersect_permission_profiles_uses_granted_bounded_glob_scan_depth() {
567672
};
568673
let granted = PermissionProfile {
569674
file_system: Some(FileSystemPermissions {
570-
entries: vec![root_write.clone(), deny_env_files.clone()],
675+
entries: vec![root_write.clone(), deny_env_files],
571676
glob_scan_max_depth: std::num::NonZeroUsize::new(4),
572677
}),
573678
..Default::default()
@@ -577,7 +682,20 @@ fn intersect_permission_profiles_uses_granted_bounded_glob_scan_depth() {
577682
intersect_permission_profiles(requested, granted, cwd.as_path()),
578683
PermissionProfile {
579684
file_system: Some(FileSystemPermissions {
580-
entries: vec![root_write, deny_env_files],
685+
entries: vec![
686+
root_write,
687+
FileSystemSandboxEntry {
688+
path: FileSystemPath::GlobPattern {
689+
pattern: AbsolutePathBuf::resolve_path_against_base(
690+
"**/*.env",
691+
cwd.as_path()
692+
)
693+
.to_string_lossy()
694+
.into_owned(),
695+
},
696+
access: FileSystemAccessMode::None,
697+
},
698+
],
581699
glob_scan_max_depth: std::num::NonZeroUsize::new(4),
582700
}),
583701
..Default::default()
@@ -609,7 +727,7 @@ fn intersect_permission_profiles_uses_granted_unbounded_glob_scan_depth() {
609727
};
610728
let granted = PermissionProfile {
611729
file_system: Some(FileSystemPermissions {
612-
entries: vec![root_write.clone(), deny_env_files.clone()],
730+
entries: vec![root_write.clone(), deny_env_files],
613731
glob_scan_max_depth: None,
614732
}),
615733
..Default::default()
@@ -619,7 +737,20 @@ fn intersect_permission_profiles_uses_granted_unbounded_glob_scan_depth() {
619737
intersect_permission_profiles(requested, granted, cwd.as_path()),
620738
PermissionProfile {
621739
file_system: Some(FileSystemPermissions {
622-
entries: vec![root_write, deny_env_files],
740+
entries: vec![
741+
root_write,
742+
FileSystemSandboxEntry {
743+
path: FileSystemPath::GlobPattern {
744+
pattern: AbsolutePathBuf::resolve_path_against_base(
745+
"**/*.env",
746+
cwd.as_path()
747+
)
748+
.to_string_lossy()
749+
.into_owned(),
750+
},
751+
access: FileSystemAccessMode::None,
752+
},
753+
],
623754
glob_scan_max_depth: None,
624755
}),
625756
..Default::default()

0 commit comments

Comments
 (0)