Skip to content

never let sandbox write to .codex/ or .codex/.sandbox/ #8683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iceweasel-oai merged 1 commit into from
Jan 5, 2026
Merged

never let sandbox write to .codex/ or .codex/.sandbox/ #8683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion codex-rs/windows-sandbox-rs/src/audit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const SKIP_DIR_SUFFIXES: &[&str] = &[
"/programdata",
];

fn normalize_path_key(p: &Path) -> String {
pub(crate) fn normalize_path_key(p: &Path) -> String {
let n = dunce::canonicalize(p).unwrap_or_else(|_| p.to_path_buf());
n.to_string_lossy().replace('\\', "/").to_ascii_lowercase()
}
Expand Down
7 changes: 4 additions & 3 deletions codex-rs/windows-sandbox-rs/src/identity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,10 @@ pub fn require_logon_sandbox_creds(
) -> Result<SandboxCreds> {
let sandbox_dir = crate::setup::sandbox_dir(codex_home);
let needed_read = gather_read_roots(command_cwd, policy);
let mut needed_write = gather_write_roots(policy, policy_cwd, command_cwd, env_map);
// Ensure the sandbox directory itself is writable by sandbox users.
needed_write.push(sandbox_dir.clone());
let needed_write = gather_write_roots(policy, policy_cwd, command_cwd, env_map);
// NOTE: Do not add CODEX_HOME/.sandbox to `needed_write`; it must remain non-writable by the
// restricted capability token. The setup helper's `lock_sandbox_dir` is responsible for
// granting the sandbox group access to this directory without granting the capability SID.
let mut setup_reason: Option<String> = None;
let mut _existing_marker: Option<SetupMarker> = None;

Expand Down
21 changes: 16 additions & 5 deletions codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,15 +408,12 @@ fn build_payload_roots(
read_roots_override: Option<Vec<PathBuf>>,
write_roots_override: Option<Vec<PathBuf>>,
) -> (Vec<PathBuf>, Vec<PathBuf>) {
let sbx_dir = sandbox_dir(codex_home);
let mut write_roots = if let Some(roots) = write_roots_override {
let write_roots = if let Some(roots) = write_roots_override {
canonical_existing(&roots)
} else {
gather_write_roots(policy, policy_cwd, command_cwd, env_map)
};
if !write_roots.contains(&sbx_dir) {
write_roots.push(sbx_dir.clone());
}
let write_roots = filter_sensitive_write_roots(write_roots, codex_home);
let mut read_roots = if let Some(roots) = read_roots_override {
canonical_existing(&roots)
} else {
Expand All @@ -426,3 +423,17 @@ fn build_payload_roots(
read_roots.retain(|root| !write_root_set.contains(root));
(read_roots, write_roots)
}

fn filter_sensitive_write_roots(mut roots: Vec<PathBuf>, codex_home: &Path) -> Vec<PathBuf> {
// Never grant capability write access to CODEX_HOME or anything under CODEX_HOME/.sandbox.
// These locations contain sandbox control/state and must remain tamper-resistant.
let codex_home_key = crate::audit::normalize_path_key(codex_home);
let sbx_dir_key = crate::audit::normalize_path_key(&sandbox_dir(codex_home));
let sbx_dir_prefix = format!("{}/", sbx_dir_key.trim_end_matches('/'));

roots.retain(|root| {
let key = crate::audit::normalize_path_key(root);
key != codex_home_key && key != sbx_dir_key && !key.starts_with(&sbx_dir_prefix)
Comment on lines +434 to +436

@chatgpt-codex-connector chatgpt-codex-connector Bot Jan 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Ensure upgrade removes old .sandbox write ACEs

Because filter_sensitive_write_roots now strips CODEX_HOME/.sandbox from every payload, the common refresh-only path won’t touch that directory. On machines that already ran setup before this change (where .sandbox was granted capability write via write_roots), those ACEs will persist indefinitely since refresh-only does not call lock_sandbox_dir to reset the DACL. That means upgrading doesn’t actually revoke sandbox write access to .sandbox unless a full setup is forced. Consider explicitly re-locking/removing ACLs during refresh or bumping the setup version to guarantee a full setup run.

Useful? React with 👍 / 👎.

@dylan-hurd-oai dylan-hurd-oai Jan 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this edge case is probably okay. However, what's the expected behavior if a user runs codex in ~/.codex?

@iceweasel-oai iceweasel-oai Jan 5, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with this PR, we never make ~/.codex/ or ~/.codex/.sandbox/ a writable root for the sandbox. If the user runs codex in one of those directories, writes in the cwd will fail in the sandbox.

@dylan-hurd-oai dylan-hurd-oai Jan 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's update docs accordingly, since this differs from behavior on other platforms.

});
roots
}
Loading