Skip to content

Commit 6e4de4c

Browse files
evawong-oaievawong-oai
committed
Enforce preserved path names in Seatbelt
1 parent a99da8c commit 6e4de4c

2 files changed

Lines changed: 60 additions & 10 deletions

File tree

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ fn root_absolute_path() -> AbsolutePathBuf {
328328
struct SeatbeltAccessRoot {
329329
root: AbsolutePathBuf,
330330
excluded_subpaths: Vec<AbsolutePathBuf>,
331+
preserved_path_names: Vec<String>,
331332
}
332333

333334
fn build_seatbelt_access_policy(
@@ -342,9 +343,9 @@ fn build_seatbelt_access_policy(
342343
let root =
343344
normalize_path_for_sandbox(access_root.root.as_path()).unwrap_or(access_root.root);
344345
let root_param = format!("{param_prefix}_{index}");
345-
params.push((root_param.clone(), root.into_path_buf()));
346+
params.push((root_param.clone(), root.clone().into_path_buf()));
346347

347-
if access_root.excluded_subpaths.is_empty() {
348+
if access_root.excluded_subpaths.is_empty() && access_root.preserved_path_names.is_empty() {
348349
policy_components.push(format!("(subpath (param \"{root_param}\"))"));
349350
continue;
350351
}
@@ -367,6 +368,11 @@ fn build_seatbelt_access_policy(
367368
"(require-not (subpath (param \"{excluded_param}\")))"
368369
));
369370
}
371+
for preserved_name in access_root.preserved_path_names {
372+
let regex =
373+
seatbelt_preserved_path_name_regex(&root, &preserved_name).replace('"', "\\\"");
374+
require_parts.push(format!(r#"(require-not (regex #"{regex}"))"#));
375+
}
370376
policy_components.push(format!("(require-all {} )", require_parts.join(" ")));
371377
}
372378

@@ -380,6 +386,20 @@ fn build_seatbelt_access_policy(
380386
}
381387
}
382388

389+
fn seatbelt_preserved_path_name_regex(root: &AbsolutePathBuf, name: &str) -> String {
390+
let mut root = root.to_string_lossy().to_string();
391+
while root.len() > 1 && root.ends_with('/') {
392+
root.pop();
393+
}
394+
let root = regex_lite::escape(&root);
395+
let name = regex_lite::escape(name);
396+
if root == "/" {
397+
format!(r#"^/(.*/)?{name}(/.*)?$"#)
398+
} else {
399+
format!(r#"^{root}/(.*/)?{name}(/.*)?$"#)
400+
}
401+
}
402+
383403
fn build_seatbelt_unreadable_glob_policy(
384404
file_system_sandbox_policy: &FileSystemSandboxPolicy,
385405
cwd: &Path,
@@ -586,6 +606,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
586606
vec![SeatbeltAccessRoot {
587607
root: root_absolute_path(),
588608
excluded_subpaths: unreadable_roots.clone(),
609+
preserved_path_names: Vec::new(),
589610
}],
590611
)
591612
}
@@ -599,6 +620,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
599620
.map(|root| SeatbeltAccessRoot {
600621
root: root.root,
601622
excluded_subpaths: root.read_only_subpaths,
623+
preserved_path_names: root.preserved_path_names,
602624
})
603625
.collect(),
604626
)
@@ -618,6 +640,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
618640
vec![SeatbeltAccessRoot {
619641
root: root_absolute_path(),
620642
excluded_subpaths: unreadable_roots,
643+
preserved_path_names: Vec::new(),
621644
}],
622645
);
623646
(
@@ -638,6 +661,7 @@ pub fn create_seatbelt_command_args(args: CreateSeatbeltCommandArgsParams<'_>) -
638661
.filter(|path| path.as_path().starts_with(root.as_path()))
639662
.cloned()
640663
.collect(),
664+
preserved_path_names: Vec::new(),
641665
root,
642666
})
643667
.collect(),

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

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use codex_protocol::permissions::FileSystemSandboxEntry;
2626
use codex_protocol::permissions::FileSystemSandboxPolicy;
2727
use codex_protocol::permissions::FileSystemSpecialPath;
2828
use codex_protocol::permissions::NetworkSandboxPolicy;
29+
use codex_protocol::permissions::PRESERVED_PATH_NAMES;
2930
use codex_protocol::protocol::SandboxPolicy;
3031
use codex_utils_absolute_path::AbsolutePathBuf;
3132
use pretty_assertions::assert_eq;
@@ -59,6 +60,26 @@ fn seatbelt_policy_arg(args: &[String]) -> &str {
5960
.expect("seatbelt args should include policy text")
6061
}
6162

63+
fn seatbelt_preserved_path_name_requirements(root: &Path) -> String {
64+
let mut root = root.to_string_lossy().to_string();
65+
while root.len() > 1 && root.ends_with('/') {
66+
root.pop();
67+
}
68+
let root = regex_lite::escape(&root);
69+
PRESERVED_PATH_NAMES
70+
.iter()
71+
.map(|name| {
72+
let name = regex_lite::escape(name);
73+
if root == "/" {
74+
format!(r#"(require-not (regex #"^/(.*/)?{name}(/.*)?$"))"#)
75+
} else {
76+
format!(r#"(require-not (regex #"^{root}/(.*/)?{name}(/.*)?$"))"#)
77+
}
78+
})
79+
.collect::<Vec<_>>()
80+
.join(" ")
81+
}
82+
6283
struct TestConfigReloader;
6384

6485
#[async_trait::async_trait]
@@ -843,8 +864,7 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
843864
);
844865
assert!(
845866
policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_0")
846-
&& policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_1")
847-
&& policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_2"),
867+
&& policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_1"),
848868
"expected explicit writable root preserved path carveouts in policy:\n{policy_text}",
849869
);
850870
assert!(
@@ -1079,11 +1099,11 @@ fn create_seatbelt_args_block_first_time_dot_codex_creation_with_exact_and_desce
10791099

10801100
let policy_text = seatbelt_policy_arg(&args);
10811101
assert!(
1082-
policy_text.contains("(require-not (literal (param \"WRITABLE_ROOT_0_EXCLUDED_1\")))"),
1102+
policy_text.contains("(require-not (literal (param \"WRITABLE_ROOT_0_EXCLUDED_2\")))"),
10831103
"expected exact .codex carveout in policy:\n{policy_text}"
10841104
);
10851105
assert!(
1086-
policy_text.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_1\")))"),
1106+
policy_text.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_2\")))"),
10871107
"expected descendant .codex carveout in policy:\n{policy_text}"
10881108
);
10891109
}
@@ -1239,11 +1259,17 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
12391259
let slash_tmp = PathBuf::from("/tmp")
12401260
.canonicalize()
12411261
.expect("canonicalize /tmp");
1242-
let tempdir_policy_entry = if tmpdir_env_var.is_some() {
1243-
r#" (require-all (subpath (param "WRITABLE_ROOT_2")) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_2"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_3"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_3"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_4"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_4"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_5"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_5"))) )"#
1262+
let tempdir_policy_entry = if let Some(p) = &tmpdir_env_var {
1263+
let preserved_requirements = seatbelt_preserved_path_name_requirements(Path::new(p));
1264+
format!(
1265+
r#" (require-all (subpath (param "WRITABLE_ROOT_2")) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_2"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_3"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_3"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_4"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_4"))) (require-not (literal (param "WRITABLE_ROOT_2_EXCLUDED_5"))) (require-not (subpath (param "WRITABLE_ROOT_2_EXCLUDED_5"))) {preserved_requirements} )"#
1266+
)
12441267
} else {
1245-
""
1268+
String::new()
12461269
};
1270+
let root_0_preserved_requirements =
1271+
seatbelt_preserved_path_name_requirements(&vulnerable_root_canonical);
1272+
let root_1_preserved_requirements = seatbelt_preserved_path_name_requirements(&slash_tmp);
12471273

12481274
// Build the expected policy text using a raw string for readability.
12491275
// Note that the policy includes:
@@ -1256,7 +1282,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
12561282
; allow read-only file operations
12571283
(allow file-read*)
12581284
(allow file-write*
1259-
(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_2"))) ) (require-all (subpath (param "WRITABLE_ROOT_1")) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_2"))) ){tempdir_policy_entry}
1285+
(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_0_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_0_EXCLUDED_2"))) {root_0_preserved_requirements} ) (require-all (subpath (param "WRITABLE_ROOT_1")) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_0"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_0"))) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_1"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_1"))) (require-not (literal (param "WRITABLE_ROOT_1_EXCLUDED_2"))) (require-not (subpath (param "WRITABLE_ROOT_1_EXCLUDED_2"))) {root_1_preserved_requirements} ){tempdir_policy_entry}
12601286
)
12611287
12621288
"#,

0 commit comments

Comments
 (0)