feat: if .codex is a sub-folder of a writable root, then make it read-only to the sandbox

This commit is contained in:
Michael Bolin
2025-12-15 17:29:15 -08:00
parent 412dd37956
commit c02d9ee1ab
3 changed files with 34 additions and 11 deletions

View File

@@ -175,13 +175,14 @@ mod tests {
#[test]
fn create_seatbelt_args_with_read_only_git_subpath() {
// Create a temporary workspace with two writable roots: one containing
// a top-level .git directory and one without it.
// top-level .git and .codex directories and one without them.
let tmp = TempDir::new().expect("tempdir");
let PopulatedTmp {
root_with_git,
root_without_git,
root_with_git_canon,
root_with_git_git_canon,
root_with_git_codex_canon,
root_without_git_canon,
} = populate_tmpdir(tmp.path());
let cwd = tmp.path().join("cwd");
@@ -208,13 +209,13 @@ mod tests {
// Note that the policy includes:
// - the base policy,
// - read-only access to the filesystem,
// - write access to WRITABLE_ROOT_0 (but not its .git) and WRITABLE_ROOT_1.
// - write access to WRITABLE_ROOT_0 (but not its .git or .codex) and WRITABLE_ROOT_1.
let expected_policy = format!(
r#"{MACOS_SEATBELT_BASE_POLICY}
; 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"))) ) (subpath (param "WRITABLE_ROOT_1")) (subpath (param "WRITABLE_ROOT_2"))
(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")) (subpath (param "WRITABLE_ROOT_2"))
)
"#,
);
@@ -230,6 +231,10 @@ mod tests {
"-DWRITABLE_ROOT_0_RO_0={}",
root_with_git_git_canon.to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_0_RO_1={}",
root_with_git_codex_canon.to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_1={}",
root_without_git_canon.to_string_lossy()
@@ -255,18 +260,19 @@ mod tests {
#[test]
fn create_seatbelt_args_for_cwd_as_git_repo() {
// Create a temporary workspace with two writable roots: one containing
// a top-level .git directory and one without it.
// top-level .git and .codex directories and one without them.
let tmp = TempDir::new().expect("tempdir");
let PopulatedTmp {
root_with_git,
root_with_git_canon,
root_with_git_git_canon,
root_with_git_codex_canon,
..
} = populate_tmpdir(tmp.path());
// Build a policy that does not specify any writable_roots, but does
// use the default ones (cwd and TMPDIR) and verifies the `.git` check
// is done properly for cwd.
// use the default ones (cwd and TMPDIR) and verifies the `.git` and
// `.codex` checks are done properly for cwd.
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
network_access: false,
@@ -296,13 +302,13 @@ mod tests {
// Note that the policy includes:
// - the base policy,
// - read-only access to the filesystem,
// - write access to WRITABLE_ROOT_0 (but not its .git) and WRITABLE_ROOT_1.
// - write access to WRITABLE_ROOT_0 (but not its .git or .codex) and WRITABLE_ROOT_1.
let expected_policy = format!(
r#"{MACOS_SEATBELT_BASE_POLICY}
; 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"))) ) (subpath (param "WRITABLE_ROOT_1")){tempdir_policy_entry}
(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}
)
"#,
);
@@ -318,6 +324,10 @@ mod tests {
"-DWRITABLE_ROOT_0_RO_0={}",
root_with_git_git_canon.to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_0_RO_1={}",
root_with_git_codex_canon.to_string_lossy()
),
format!(
"-DWRITABLE_ROOT_1={}",
PathBuf::from("/tmp")
@@ -351,6 +361,7 @@ mod tests {
root_without_git: PathBuf,
root_with_git_canon: PathBuf,
root_with_git_git_canon: PathBuf,
root_with_git_codex_canon: PathBuf,
root_without_git_canon: PathBuf,
}
@@ -360,10 +371,12 @@ mod tests {
fs::create_dir_all(&root_with_git).expect("create with_git");
fs::create_dir_all(&root_without_git).expect("create no_git");
fs::create_dir_all(root_with_git.join(".git")).expect("create .git");
fs::create_dir_all(root_with_git.join(".codex")).expect("create .codex");
// Ensure we have canonical paths for -D parameter matching.
let root_with_git_canon = root_with_git.canonicalize().expect("canonicalize with_git");
let root_with_git_git_canon = root_with_git_canon.join(".git");
let root_with_git_codex_canon = root_with_git_canon.join(".codex");
let root_without_git_canon = root_without_git
.canonicalize()
.expect("canonicalize no_git");
@@ -372,6 +385,7 @@ mod tests {
root_without_git,
root_with_git_canon,
root_with_git_git_canon,
root_with_git_codex_canon,
root_without_git_canon,
}
}

View File

@@ -306,12 +306,14 @@ pub enum SandboxPolicy {
/// A writable root path accompanied by a list of subpaths that should remain
/// readonly even when the root is writable. This is primarily used to ensure
/// toplevel VCS metadata directories (e.g. `.git`) under a writable root are
/// not modified by the agent.
/// that folders containing files that could be modified to escalate the
/// privileges of the agent (e.g. `.codex`, `.git`, notably `.git/hooks`) under
/// a writable root are not modified by the agent.
#[derive(Debug, Clone, PartialEq, Eq, JsonSchema)]
pub struct WritableRoot {
pub root: AbsolutePathBuf,
/// By construction, these subpaths are all under `root`.
pub read_only_subpaths: Vec<AbsolutePathBuf>,
}
@@ -458,6 +460,13 @@ impl SandboxPolicy {
if top_level_git.as_path().is_dir() {
subpaths.push(top_level_git);
}
#[allow(clippy::expect_used)]
let top_level_codex = writable_root
.join(".codex")
.expect(".codex is a valid relative path");
if top_level_codex.as_path().is_dir() {
subpaths.push(top_level_codex);
}
WritableRoot {
root: writable_root,
read_only_subpaths: subpaths,

View File

@@ -316,7 +316,7 @@ disk, but attempts to write a file or access the network will be blocked.
A more relaxed policy is `workspace-write`. When specified, the current working directory for the Codex task will be writable (as well as `$TMPDIR` on macOS). Note that the CLI defaults to using the directory where it was spawned as `cwd`, though this can be overridden using `--cwd/-C`.
On macOS (and soon Linux), all writable roots (including `cwd`) that contain a `.git/` folder _as an immediate child_ will configure the `.git/` folder to be read-only while the rest of the Git repository will be writable. This means that commands like `git commit` will fail, by default (as it entails writing to `.git/`), and will require Codex to ask for permission.
On macOS (and soon Linux), all writable roots (including `cwd`) that contain a `.git/` or `.codex/` folder _as an immediate child_ will configure those folders to be read-only while the rest of the root stays writable. This means that commands like `git commit` will fail, by default (as it entails writing to `.git/`), and will require Codex to ask for permission.
```toml
# same as `--sandbox workspace-write`