mirror of
https://github.com/openai/codex.git
synced 2026-11-03 00:21:31 +00:00
Protect first-time project .codex creation across Linux and macOS sandboxes (#15067)
## Problem Codex already treated an existing top-level project `./.codex` directory as protected, but there was a gap on first creation. If `./.codex` did not exist yet, a turn could create files under it, such as `./.codex/config.toml`, without going through the same approval path as later modifications. That meant the initial write could bypass the intended protection for project-local Codex state. ## What this changes This PR closes that first-creation gap in the Unix enforcement layers: - `codex-protocol` - treat the top-level project `./.codex` path as a protected carveout even when it does not exist yet - avoid injecting the default carveout when the user already has an explicit rule for that exact path - macOS Seatbelt - deny writes to both the exact protected path and anything beneath it, so creating `./.codex` itself is blocked in addition to writes inside it - Linux bubblewrap - preserve the same protected-path behavior for first-time creation under `./.codex` - tests - add protocol regressions for missing `./.codex` and explicit-rule collisions - add Unix sandbox coverage for blocking first-time `./.codex` creation - tighten Seatbelt policy assertions around excluded subpaths ## Scope This change is intentionally scoped to protecting the top-level project `.codex` subtree from agent writes. It does not make `.codex` unreadable, and it does not change the product behavior around loading project skills from `.codex` when project config is untrusted. ## Why this shape The fix is pointed rather than broad: - it preserves the current model of “project `.codex` is protected from writes” - it closes the security-relevant first-write hole - it avoids folding a larger permissions-model redesign into this PR ## Validation - `cargo test -p codex-protocol` - `cargo test -p codex-sandboxing seatbelt` - `cargo test -p codex-exec --test all sandbox_blocks_first_time_dot_codex_creation -- --nocapture` --------- Co-authored-by: Michael Bolin <mbolin@openai.com>
This commit is contained in:
1 parent
9736fa5e3d
commit
86764af684
10 files changed
+571
-112
No files matched your search
@@ -330,8 +330,14 @@ fn build_seatbelt_access_policy(
|
||||
{
|
||||
let excluded_subpath =
|
||||
normalize_path_for_sandbox(excluded_subpath.as_path()).unwrap_or(excluded_subpath);
|
||||
let excluded_param = format!("{param_prefix}_{index}_RO_{excluded_index}");
|
||||
let excluded_param = format!("{param_prefix}_{index}_EXCLUDED_{excluded_index}");
|
||||
params.push((excluded_param.clone(), excluded_subpath.into_path_buf()));
|
||||
// Exclude both the exact protected path and anything beneath it.
|
||||
// `subpath` alone leaves a gap for first-time creation of the
|
||||
// protected directory itself, such as `mkdir .codex`.
|
||||
require_parts.push(format!(
|
||||
"(require-not (literal (param \"{excluded_param}\")))"
|
||||
));
|
||||
require_parts.push(format!(
|
||||
"(require-not (subpath (param \"{excluded_param}\")))"
|
||||
));
|
||||
|
||||
@@ -129,22 +129,40 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access()
|
||||
let unreadable_roots = file_system_policy.get_unreadable_roots_with_cwd(Path::new("/"));
|
||||
let unreadable_root = unreadable_roots.first().expect("expected unreadable root");
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_RO_0\")))"),
|
||||
policy.contains("(require-not (literal (param \"READABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected exact read carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected read carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_RO_0\")))"),
|
||||
policy.contains("(require-not (literal (param \"WRITABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected exact write carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected write carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
args.iter()
|
||||
.any(|arg| arg == &format!("-DREADABLE_ROOT_0_RO_0={}", unreadable_root.display())),
|
||||
args.iter().any(
|
||||
|arg| arg == &format!("-DREADABLE_ROOT_0_EXCLUDED_0={}", unreadable_root.display())
|
||||
),
|
||||
"expected read carveout parameter in args: {args:#?}"
|
||||
);
|
||||
assert!(
|
||||
args.iter()
|
||||
.any(|arg| arg == &format!("-DWRITABLE_ROOT_0_RO_0={}", unreadable_root.display())),
|
||||
"expected write carveout parameter in args: {args:#?}"
|
||||
let writable_definitions: Vec<String> = args
|
||||
.iter()
|
||||
.filter(|arg| arg.starts_with("-DWRITABLE_ROOT_"))
|
||||
.cloned()
|
||||
.collect();
|
||||
assert_eq!(
|
||||
writable_definitions,
|
||||
vec![
|
||||
"-DWRITABLE_ROOT_0=/".to_string(),
|
||||
"-DWRITABLE_ROOT_0_EXCLUDED_0=/.codex".to_string(),
|
||||
format!("-DWRITABLE_ROOT_0_EXCLUDED_1={}", unreadable_root.display()),
|
||||
],
|
||||
"unexpected write carveout parameters in args: {args:#?}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -179,7 +197,11 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() {
|
||||
let unreadable_roots = file_system_policy.get_unreadable_roots_with_cwd(Path::new("/"));
|
||||
let unreadable_root = unreadable_roots.first().expect("expected unreadable root");
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_RO_0\")))"),
|
||||
policy.contains("(require-not (literal (param \"READABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected exact read carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
policy.contains("(require-not (subpath (param \"READABLE_ROOT_0_EXCLUDED_0\")))"),
|
||||
"expected read carveout in policy:\n{policy}"
|
||||
);
|
||||
assert!(
|
||||
@@ -188,8 +210,9 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() {
|
||||
"expected readable root parameter in args: {args:#?}"
|
||||
);
|
||||
assert!(
|
||||
args.iter()
|
||||
.any(|arg| arg == &format!("-DREADABLE_ROOT_0_RO_0={}", unreadable_root.display())),
|
||||
args.iter().any(
|
||||
|arg| arg == &format!("-DREADABLE_ROOT_0_EXCLUDED_0={}", unreadable_root.display())
|
||||
),
|
||||
"expected read carveout parameter in args: {args:#?}"
|
||||
);
|
||||
}
|
||||
@@ -314,14 +337,17 @@ fn seatbelt_legacy_workspace_write_nested_readable_root_stays_writable() {
|
||||
/*extensions*/ None,
|
||||
);
|
||||
|
||||
let docs_param = format!("-DWRITABLE_ROOT_0_RO_0={}", docs.as_path().display());
|
||||
assert!(
|
||||
!seatbelt_policy_arg(&args).contains("WRITABLE_ROOT_0_RO_0"),
|
||||
"legacy workspace-write readable roots under cwd should not become seatbelt carveouts:\n{args:#?}"
|
||||
!args
|
||||
.iter()
|
||||
.any(|arg| arg.ends_with(&format!("={}", docs.as_path().display()))),
|
||||
"legacy workspace-write readable roots under cwd should not become seatbelt carveouts:\n{args:#?}",
|
||||
);
|
||||
assert!(
|
||||
!args.iter().any(|arg| arg == &docs_param),
|
||||
"unexpected seatbelt carveout parameter for redundant legacy readable root: {args:#?}"
|
||||
args.iter()
|
||||
.any(|arg| arg.starts_with("-DWRITABLE_ROOT_0_EXCLUDED_")
|
||||
&& arg.ends_with("/workspace/.codex")),
|
||||
"expected proactive .codex carveout for cwd root: {args:#?}",
|
||||
);
|
||||
}
|
||||
|
||||
@@ -647,30 +673,24 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
|
||||
/*extensions*/ None,
|
||||
);
|
||||
|
||||
// Build the expected policy text using a raw string for readability.
|
||||
// Note that the policy includes:
|
||||
// - the base policy,
|
||||
// - read-only access to the filesystem,
|
||||
// - write access to WRITABLE_ROOT_0 (but not its .git or .codex), WRITABLE_ROOT_1, and cwd as WRITABLE_ROOT_2.
|
||||
let expected_policy = format!(
|
||||
r#"{MACOS_SEATBELT_BASE_POLICY}
|
||||
; allow read-only file operations
|
||||
(allow file-read*)
|
||||
(allow file-write*
|
||||
(subpath (param "WRITABLE_ROOT_0")) (require-all (subpath (param "WRITABLE_ROOT_1")) (require-not (subpath (param "WRITABLE_ROOT_1_RO_0"))) (require-not (subpath (param "WRITABLE_ROOT_1_RO_1"))) ) (subpath (param "WRITABLE_ROOT_2"))
|
||||
)
|
||||
|
||||
; macOS permission profile extensions
|
||||
(allow ipc-posix-shm-read* (ipc-posix-name-prefix "apple.cfprefs."))
|
||||
(allow mach-lookup
|
||||
(global-name "com.apple.cfprefsd.daemon")
|
||||
(global-name "com.apple.cfprefsd.agent")
|
||||
(local-name "com.apple.cfprefsd.agent"))
|
||||
(allow user-preference-read)
|
||||
"#,
|
||||
let policy_text = seatbelt_policy_arg(&args);
|
||||
assert!(
|
||||
policy_text.contains("(require-all (subpath (param \"WRITABLE_ROOT_0\"))"),
|
||||
"expected cwd writable root to carry protected carveouts:\n{policy_text}",
|
||||
);
|
||||
assert!(
|
||||
policy_text.contains("WRITABLE_ROOT_0_EXCLUDED_0"),
|
||||
"expected cwd .codex carveout in policy:\n{policy_text}",
|
||||
);
|
||||
assert!(
|
||||
policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_0")
|
||||
&& policy_text.contains("WRITABLE_ROOT_1_EXCLUDED_1"),
|
||||
"expected explicit writable root .git/.codex carveouts in policy:\n{policy_text}",
|
||||
);
|
||||
assert!(
|
||||
policy_text.contains("(subpath (param \"WRITABLE_ROOT_2\"))"),
|
||||
"expected second explicit writable root grant in policy:\n{policy_text}",
|
||||
);
|
||||
|
||||
assert_eq!(seatbelt_policy_arg(&args), expected_policy);
|
||||
|
||||
let expected_definitions = [
|
||||
format!(
|
||||
@@ -679,16 +699,23 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
|
||||
.expect("canonicalize cwd")
|
||||
.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_0_EXCLUDED_0={}",
|
||||
cwd.canonicalize()
|
||||
.expect("canonicalize cwd")
|
||||
.join(".codex")
|
||||
.display()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_1={}",
|
||||
vulnerable_root_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_1_RO_0={}",
|
||||
"-DWRITABLE_ROOT_1_EXCLUDED_0={}",
|
||||
dot_git_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_1_RO_1={}",
|
||||
"-DWRITABLE_ROOT_1_EXCLUDED_1={}",
|
||||
dot_codex_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
@@ -696,12 +723,15 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
|
||||
empty_root_canonical.to_string_lossy()
|
||||
),
|
||||
];
|
||||
for expected_definition in expected_definitions {
|
||||
assert!(
|
||||
args.contains(&expected_definition),
|
||||
"expected definition arg `{expected_definition}` in {args:#?}"
|
||||
);
|
||||
}
|
||||
let writable_definitions: Vec<String> = args
|
||||
.iter()
|
||||
.filter(|arg| arg.starts_with("-DWRITABLE_ROOT_"))
|
||||
.cloned()
|
||||
.collect();
|
||||
assert_eq!(
|
||||
writable_definitions, expected_definitions,
|
||||
"unexpected writable-root parameter definitions in {args:#?}"
|
||||
);
|
||||
for (key, value) in macos_dir_params() {
|
||||
let expected_definition = format!("-D{key}={}", value.to_string_lossy());
|
||||
assert!(
|
||||
@@ -818,6 +848,60 @@ fn create_seatbelt_args_with_read_only_git_and_codex_subpaths() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_seatbelt_args_block_first_time_dot_codex_creation_with_exact_and_descendant_carveouts() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let repo_root = tmp.path().join("repo");
|
||||
fs::create_dir_all(&repo_root).expect("create repo root");
|
||||
|
||||
Command::new("git")
|
||||
.arg("init")
|
||||
.arg(".")
|
||||
.current_dir(&repo_root)
|
||||
.output()
|
||||
.expect("git init .");
|
||||
|
||||
let dot_codex = repo_root.join(".codex");
|
||||
let config_toml = dot_codex.join("config.toml");
|
||||
let policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![repo_root.as_path().try_into().expect("absolute repo root")],
|
||||
read_only_access: Default::default(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
exclude_slash_tmp: true,
|
||||
};
|
||||
|
||||
let shell_command: Vec<String> = [
|
||||
"bash",
|
||||
"-c",
|
||||
"mkdir -p \"$1\" && echo 'sandbox_mode = \"danger-full-access\"' > \"$2\"",
|
||||
"bash",
|
||||
dot_codex.to_string_lossy().as_ref(),
|
||||
config_toml.to_string_lossy().as_ref(),
|
||||
]
|
||||
.iter()
|
||||
.map(std::string::ToString::to_string)
|
||||
.collect();
|
||||
let args = create_seatbelt_command_args_with_extensions(
|
||||
shell_command,
|
||||
&policy,
|
||||
repo_root.as_path(),
|
||||
/*enforce_managed_network*/ false,
|
||||
/*network*/ None,
|
||||
/*extensions*/ None,
|
||||
);
|
||||
|
||||
let policy_text = seatbelt_policy_arg(&args);
|
||||
assert!(
|
||||
policy_text.contains("(require-not (literal (param \"WRITABLE_ROOT_0_EXCLUDED_1\")))"),
|
||||
"expected exact .codex carveout in policy:\n{policy_text}"
|
||||
);
|
||||
assert!(
|
||||
policy_text.contains("(require-not (subpath (param \"WRITABLE_ROOT_0_EXCLUDED_1\")))"),
|
||||
"expected descendant .codex carveout in policy:\n{policy_text}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_seatbelt_args_with_read_only_git_pointer_file() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
@@ -971,7 +1055,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
|
||||
.map(|p| p.to_string_lossy().to_string());
|
||||
|
||||
let tempdir_policy_entry = if tmpdir_env_var.is_some() {
|
||||
r#" (subpath (param "WRITABLE_ROOT_2"))"#
|
||||
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"))) )"#
|
||||
} else {
|
||||
""
|
||||
};
|
||||
@@ -986,7 +1070,7 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
|
||||
; allow read-only file operations
|
||||
(allow file-read*)
|
||||
(allow file-write*
|
||||
(require-all (subpath (param "WRITABLE_ROOT_0")) (require-not (subpath (param "WRITABLE_ROOT_0_RO_0"))) (require-not (subpath (param "WRITABLE_ROOT_0_RO_1"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry}
|
||||
(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}
|
||||
)
|
||||
|
||||
; macOS permission profile extensions
|
||||
@@ -1007,11 +1091,11 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
|
||||
vulnerable_root_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_0_RO_0={}",
|
||||
"-DWRITABLE_ROOT_0_EXCLUDED_0={}",
|
||||
dot_git_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
"-DWRITABLE_ROOT_0_RO_1={}",
|
||||
"-DWRITABLE_ROOT_0_EXCLUDED_1={}",
|
||||
dot_codex_canonical.to_string_lossy()
|
||||
),
|
||||
format!(
|
||||
@@ -1025,6 +1109,14 @@ fn create_seatbelt_args_for_cwd_as_git_repo() {
|
||||
|
||||
if let Some(p) = tmpdir_env_var {
|
||||
expected_args.push(format!("-DWRITABLE_ROOT_2={p}"));
|
||||
expected_args.push(format!(
|
||||
"-DWRITABLE_ROOT_2_EXCLUDED_0={}",
|
||||
dot_git_canonical.to_string_lossy()
|
||||
));
|
||||
expected_args.push(format!(
|
||||
"-DWRITABLE_ROOT_2_EXCLUDED_1={}",
|
||||
dot_codex_canonical.to_string_lossy()
|
||||
));
|
||||
}
|
||||
|
||||
expected_args.extend(
|
||||
|
||||
Reference in new issue
Block a user