Skip to content

Commit 613fe13

Browse files
evawong-oaievawong-oai
committed
Enforce preserved path names in Seatbelt
1 parent 0cfa95f commit 613fe13

2 files changed

Lines changed: 142 additions & 25 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: 116 additions & 23 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]
@@ -200,8 +221,10 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access()
200221
writable_definitions,
201222
vec![
202223
"-DWRITABLE_ROOT_0=/".to_string(),
203-
"-DWRITABLE_ROOT_0_EXCLUDED_0=/.codex".to_string(),
204-
format!("-DWRITABLE_ROOT_0_EXCLUDED_1={}", unreadable_root.display()),
224+
"-DWRITABLE_ROOT_0_EXCLUDED_0=/.git".to_string(),
225+
"-DWRITABLE_ROOT_0_EXCLUDED_1=/.agents".to_string(),
226+
"-DWRITABLE_ROOT_0_EXCLUDED_2=/.codex".to_string(),
227+
format!("-DWRITABLE_ROOT_0_EXCLUDED_3={}", unreadable_root.display()),
205228
],
206229
"unexpected write carveout parameters in args: {args:#?}"
207230
);
@@ -773,12 +796,13 @@ fn create_seatbelt_args_full_network_with_proxy_is_still_proxy_only() {
773796
#[test]
774797
fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
775798
// Create a temporary workspace with two writable roots: one containing
776-
// top-level .git and .codex directories and one without them.
799+
// top-level preserved paths and one without them.
777800
let tmp = TempDir::new().expect("tempdir");
778801
let PopulatedTmp {
779802
vulnerable_root,
780803
vulnerable_root_canonical,
781804
dot_git_canonical,
805+
dot_agents_canonical: _,
782806
dot_codex_canonical,
783807
empty_root,
784808
empty_root_canonical,
@@ -828,12 +852,20 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
828852
);
829853
assert!(
830854
policy_text.contains("WRITABLE_ROOT_0_EXCLUDED_0"),
855+
"expected cwd .git carveout in policy:\n{policy_text}",
856+
);
857+
assert!(
858+
policy_text.contains("WRITABLE_ROOT_0_EXCLUDED_1"),
859+
"expected cwd .agents carveout in policy:\n{policy_text}",
860+
);
861+
assert!(
862+
policy_text.contains("WRITABLE_ROOT_0_EXCLUDED_2"),
831863
"expected cwd .codex carveout in policy:\n{policy_text}",
832864
);
833865
assert!(
834866
policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_0")
835867
&& policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_1"),
836-
"expected explicit writable root .git/.codex carveouts in policy:\n{policy_text}",
868+
"expected explicit writable root preserved path carveouts in policy:\n{policy_text}",
837869
);
838870
assert!(
839871
policy_text.contains("(subpath (param \"WRITABLE_ROOT_2\"))"),
@@ -849,6 +881,20 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
849881
),
850882
format!(
851883
"-DWRITABLE_ROOT_0_EXCLUDED_0={}",
884+
cwd.canonicalize()
885+
.expect("canonicalize cwd")
886+
.join(".git")
887+
.display()
888+
),
889+
format!(
890+
"-DWRITABLE_ROOT_0_EXCLUDED_1={}",
891+
cwd.canonicalize()
892+
.expect("canonicalize cwd")
893+
.join(".agents")
894+
.display()
895+
),
896+
format!(
897+
"-DWRITABLE_ROOT_0_EXCLUDED_2={}",
852898
cwd.canonicalize()
853899
.expect("canonicalize cwd")
854900
.join(".codex")
@@ -864,12 +910,28 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
864910
),
865911
format!(
866912
"-DWRITABLE_ROOT_1_EXCLUDED_1={}",
913+
vulnerable_root_canonical.join(".agents").to_string_lossy()
914+
),
915+
format!(
916+
"-DWRITABLE_ROOT_1_EXCLUDED_2={}",
867917
dot_codex_canonical.to_string_lossy()
868918
),
869919
format!(
870920
"-DWRITABLE_ROOT_2={}",
871921
empty_root_canonical.to_string_lossy()
872922
),
923+
format!(
924+
"-DWRITABLE_ROOT_2_EXCLUDED_0={}",
925+
empty_root_canonical.join(".git").to_string_lossy()
926+
),
927+
format!(
928+
"-DWRITABLE_ROOT_2_EXCLUDED_1={}",
929+
empty_root_canonical.join(".agents").to_string_lossy()
930+
),
931+
format!(
932+
"-DWRITABLE_ROOT_2_EXCLUDED_2={}",
933+
empty_root_canonical.join(".codex").to_string_lossy()
934+
),
873935
];
874936
let writable_definitions: Vec<String> = args
875937
.iter()
@@ -1037,11 +1099,11 @@ fn create_seatbelt_args_block_first_time_dot_codex_creation_with_exact_and_desce
10371099

10381100
let policy_text = seatbelt_policy_arg(&args);
10391101
assert!(
1040-
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\")))"),
10411103
"expected exact .codex carveout in policy:\n{policy_text}"
10421104
);
10431105
assert!(
1044-
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\")))"),
10451107
"expected descendant .codex carveout in policy:\n{policy_text}"
10461108
);
10471109
}
@@ -1146,19 +1208,20 @@ fn create_seatbelt_args_with_read_only_git_pointer_file() {
11461208
#[test]
11471209
fn create_seatbelt_args_for_cwd_as_git_repo() {
11481210
// Create a temporary workspace with two writable roots: one containing
1149-
// top-level .git and .codex directories and one without them.
1211+
// top-level preserved paths and one without them.
11501212
let tmp = TempDir::new().expect("tempdir");
11511213
let PopulatedTmp {
11521214
vulnerable_root,
11531215
vulnerable_root_canonical,
11541216
dot_git_canonical,
1217+
dot_agents_canonical,
11551218
dot_codex_canonical,
11561219
..
11571220
} = populate_tmpdir(tmp.path());
11581221

11591222
// Build a policy that does not specify any writable_roots, but does
1160-
// use the default ones (cwd and TMPDIR) and verifies the `.git` and
1161-
// `.codex` checks are done properly for cwd.
1223+
// use the default ones (cwd and TMPDIR) and verifies the preserved
1224+
// path checks are done properly for cwd.
11621225
let policy = SandboxPolicy::WorkspaceWrite {
11631226
writable_roots: vec![],
11641227
network_access: false,
@@ -1193,23 +1256,33 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
11931256
.and_then(|p| p.canonicalize().ok())
11941257
.map(|p| p.to_string_lossy().to_string());
11951258

1196-
let tempdir_policy_entry = if tmpdir_env_var.is_some() {
1197-
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"))) )"#
1259+
let slash_tmp = PathBuf::from("/tmp")
1260+
.canonicalize()
1261+
.expect("canonicalize /tmp");
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+
)
11981267
} else {
1199-
""
1268+
String::new()
12001269
};
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);
12011273

12021274
// Build the expected policy text using a raw string for readability.
12031275
// Note that the policy includes:
12041276
// - the base policy,
12051277
// - read-only access to the filesystem,
1206-
// - write access to WRITABLE_ROOT_0 (but not its .git or .codex), WRITABLE_ROOT_1, and cwd as WRITABLE_ROOT_2.
1278+
// - write access to WRITABLE_ROOT_0, WRITABLE_ROOT_1, and cwd as
1279+
// WRITABLE_ROOT_2, each with preserved path carveouts.
12071280
let expected_policy = format!(
12081281
r#"{MACOS_SEATBELT_BASE_POLICY}
12091282
; allow read-only file operations
12101283
(allow file-read*)
12111284
(allow file-write*
1212-
(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"))) ) (subpath (param "WRITABLE_ROOT_1")){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}
12131286
)
12141287
12151288
"#,
@@ -1228,25 +1301,42 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
12281301
),
12291302
format!(
12301303
"-DWRITABLE_ROOT_0_EXCLUDED_1={}",
1304+
dot_agents_canonical.to_string_lossy()
1305+
),
1306+
format!(
1307+
"-DWRITABLE_ROOT_0_EXCLUDED_2={}",
12311308
dot_codex_canonical.to_string_lossy()
12321309
),
1310+
format!("-DWRITABLE_ROOT_1={}", slash_tmp.to_string_lossy()),
12331311
format!(
1234-
"-DWRITABLE_ROOT_1={}",
1235-
PathBuf::from("/tmp")
1236-
.canonicalize()
1237-
.expect("canonicalize /tmp")
1238-
.to_string_lossy()
1312+
"-DWRITABLE_ROOT_1_EXCLUDED_0={}",
1313+
slash_tmp.join(".git").to_string_lossy()
1314+
),
1315+
format!(
1316+
"-DWRITABLE_ROOT_1_EXCLUDED_1={}",
1317+
slash_tmp.join(".agents").to_string_lossy()
1318+
),
1319+
format!(
1320+
"-DWRITABLE_ROOT_1_EXCLUDED_2={}",
1321+
slash_tmp.join(".codex").to_string_lossy()
12391322
),
12401323
];
12411324

12421325
if let Some(p) = tmpdir_env_var {
12431326
expected_args.push(format!("-DWRITABLE_ROOT_2={p}"));
1327+
expected_args.push(format!("-DWRITABLE_ROOT_2_EXCLUDED_0={p}/.git"));
1328+
expected_args.push(format!("-DWRITABLE_ROOT_2_EXCLUDED_1={p}/.agents"));
1329+
expected_args.push(format!("-DWRITABLE_ROOT_2_EXCLUDED_2={p}/.codex"));
12441330
expected_args.push(format!(
1245-
"-DWRITABLE_ROOT_2_EXCLUDED_0={}",
1331+
"-DWRITABLE_ROOT_2_EXCLUDED_3={}",
12461332
dot_git_canonical.to_string_lossy()
12471333
));
12481334
expected_args.push(format!(
1249-
"-DWRITABLE_ROOT_2_EXCLUDED_1={}",
1335+
"-DWRITABLE_ROOT_2_EXCLUDED_4={}",
1336+
dot_agents_canonical.to_string_lossy()
1337+
));
1338+
expected_args.push(format!(
1339+
"-DWRITABLE_ROOT_2_EXCLUDED_5={}",
12501340
dot_codex_canonical.to_string_lossy()
12511341
));
12521342
}
@@ -1264,7 +1354,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
12641354
}
12651355

12661356
struct PopulatedTmp {
1267-
/// Path containing a .git and .codex subfolder.
1357+
/// Path containing preserved subfolders.
12681358
/// For the purposes of this test, we consider this a "vulnerable" root
12691359
/// because a bad actor could write to .git/hooks/pre-commit so an
12701360
/// unsuspecting user would run code as privileged the next time they
@@ -1274,9 +1364,10 @@ struct PopulatedTmp {
12741364
vulnerable_root: PathBuf,
12751365
vulnerable_root_canonical: PathBuf,
12761366
dot_git_canonical: PathBuf,
1367+
dot_agents_canonical: PathBuf,
12771368
dot_codex_canonical: PathBuf,
12781369

1279-
/// Path without .git or .codex subfolders.
1370+
/// Path without preserved subfolders.
12801371
empty_root: PathBuf,
12811372
/// Canonicalized version of `empty_root`.
12821373
empty_root_canonical: PathBuf,
@@ -1310,12 +1401,14 @@ fn populate_tmpdir(tmp: &Path) -> PopulatedTmp {
13101401
.canonicalize()
13111402
.expect("canonicalize vulnerable_root");
13121403
let dot_git_canonical = vulnerable_root_canonical.join(".git");
1404+
let dot_agents_canonical = vulnerable_root_canonical.join(".agents");
13131405
let dot_codex_canonical = vulnerable_root_canonical.join(".codex");
13141406
let empty_root_canonical = empty_root.canonicalize().expect("canonicalize empty_root");
13151407
PopulatedTmp {
13161408
vulnerable_root,
13171409
vulnerable_root_canonical,
13181410
dot_git_canonical,
1411+
dot_agents_canonical,
13191412
dot_codex_canonical,
13201413
empty_root,
13211414
empty_root_canonical,

0 commit comments

Comments
 (0)