openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-23 18:08:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(permissions): enforce managed allow_git false

Browse Source 
Co-authored-by: Codex noreply@openai.com
This commit is contained in:
viyatb-oai
2026-06-01 16:42:21 -07:00
co-authored by Codex noreply@openai.com
parent 5ce02a8a74
commit b0913e0fcf
9 changed files with 128 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/request_processors/command_exec_processor.rs
+8 -2
View File
@@ -240,8 +240,14 @@ impl CommandExecRequestProcessor {
.map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?;
let mut file_system_sandbox_policy =
codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, &sandbox_cwd);
if self.config.managed_allow_limited_git_writes() == Some(true) {
file_system_sandbox_policy.apply_managed_limited_git_writes(&sandbox_cwd);
match self.config.managed_allow_limited_git_writes() {
Some(true) => {
file_system_sandbox_policy.apply_managed_limited_git_writes(&sandbox_cwd);
}
Some(false) => {
file_system_sandbox_policy.apply_managed_git_write_protection(&sandbox_cwd);
}
None => {}
}
let network_sandbox_policy =
codex_protocol::permissions::NetworkSandboxPolicy::from(&policy);
codex-rs/config/src/requirements_layers/permissions.rs
+1
View File
@@ -49,6 +49,7 @@ impl DenyReadMergeState {
PermissionsRequirementsToml {
filesystem: Some(FilesystemRequirementsToml {
deny_read: Some(self.deny_read),
allow_git: None,
}),
profiles: Default::default(),
},
codex-rs/config/src/types.rs
+3 -1
View File
@@ -883,6 +883,8 @@ pub struct SandboxWorkspaceWrite {
#[serde(default)]
pub network_access: bool,
#[serde(default)]
pub allow_limited_git_writes: bool,
#[serde(default)]
pub exclude_tmpdir_env_var: bool,
#[serde(default)]
pub exclude_slash_tmp: bool,
@@ -893,7 +895,7 @@ impl From<SandboxWorkspaceWrite> for codex_app_server_protocol::SandboxSettings
Self {
writable_roots: sandbox_workspace_write.writable_roots,
network_access: Some(sandbox_workspace_write.network_access),
allow_limited_git_writes: None,
allow_limited_git_writes: Some(sandbox_workspace_write.allow_limited_git_writes),
exclude_tmpdir_env_var: Some(sandbox_workspace_write.exclude_tmpdir_env_var),
exclude_slash_tmp: Some(sandbox_workspace_write.exclude_slash_tmp),
}
codex-rs/core/config.schema.json
+4
View File
@@ -2518,6 +2518,10 @@
"SandboxWorkspaceWrite": {
"additionalProperties": false,
"properties": {
"allow_limited_git_writes": {
"default": false,
"type": "boolean"
},
"exclude_slash_tmp": {
"default": false,
"type": "boolean"
codex-rs/core/src/config/config_loader_tests.rs
-8
View File
@@ -1567,10 +1567,6 @@ allow_git = true
.await?;
let policy = config.permissions.file_system_sandbox_policy();
assert!(
policy.allow_limited_git_writes,
"expected system requirements to enable limited git writes for `{source}`"
);
assert!(
policy.can_write_path_with_cwd(&tmp.path().join(".git").join("HEAD"), tmp.path()),
"expected system requirements to permit Git metadata writes for `{source}`"
@@ -1616,10 +1612,6 @@ allow_git = false
.await?;
let policy = config.permissions.file_system_sandbox_policy();
assert!(
!policy.allow_limited_git_writes,
"expected system requirements to override local limited git write opt-in"
);
assert!(
!policy.can_write_path_with_cwd(&tmp.path().join(".git").join("HEAD"), tmp.path()),
"expected the disabled managed setting to protect Git metadata"
codex-rs/core/src/config/config_tests.rs
+2
View File
@@ -2373,6 +2373,7 @@ async fn explicit_builtin_workspace_profile_ignores_legacy_workspace_write_setti
sandbox_workspace_write: Some(SandboxWorkspaceWrite {
writable_roots: vec![extra_root.path().abs()],
network_access: true,
allow_limited_git_writes: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
}),
@@ -2589,6 +2590,7 @@ async fn implicit_builtin_workspace_profile_preserves_sandbox_workspace_write_se
sandbox_workspace_write: Some(SandboxWorkspaceWrite {
writable_roots: vec![extra_root.clone()],
network_access: true,
allow_limited_git_writes: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: false,
}),
codex-rs/core/src/config/mod.rs
+4 -2
View File
@@ -2150,8 +2150,10 @@ fn apply_managed_filesystem_constraints(
filesystem_constraints: &codex_config::FilesystemConstraints,
cwd: &Path,
) {
if filesystem_constraints.allow_git == Some(true) {
file_system_sandbox_policy.apply_managed_limited_git_writes(cwd);
match filesystem_constraints.allow_git {
Some(true) => file_system_sandbox_policy.apply_managed_limited_git_writes(cwd),
Some(false) => file_system_sandbox_policy.apply_managed_git_write_protection(cwd),
None => {}
}
for deny_read in &filesystem_constraints.deny_read {
let deny_entry = if deny_read.contains_glob() {
codex-rs/core/src/session/session.rs
+8 -2
View File
@@ -176,8 +176,14 @@ impl SessionConfiguration {
&self,
file_system_sandbox_policy: &mut FileSystemSandboxPolicy,
) {
if self.managed_allow_limited_git_writes == Some(true) {
file_system_sandbox_policy.apply_managed_limited_git_writes(self.cwd.as_path());
match self.managed_allow_limited_git_writes {
Some(true) => {
file_system_sandbox_policy.apply_managed_limited_git_writes(self.cwd.as_path());
}
Some(false) => {
file_system_sandbox_policy.apply_managed_git_write_protection(self.cwd.as_path());
}
None => {}
}
}
codex-rs/protocol/src/permissions.rs
+98 -45
View File
@@ -464,6 +464,49 @@ impl FileSystemSandboxPolicy {
}
}
/// Apply a managed override that keeps Git metadata protected even when a
/// profile opted into limited Git metadata writes.
pub fn apply_managed_git_write_protection(&mut self, cwd: &Path) {
if !matches!(self.kind, FileSystemSandboxKind::Restricted) {
return;
}
let writable_root_paths = dedup_absolute_paths(
self.resolved_entries_with_cwd(cwd)
.into_iter()
.filter(|entry| entry.access.can_write())
.filter(|entry| self.can_write_path_with_cwd(entry.path.as_path(), cwd))
.map(|entry| entry.path)
.collect(),
/*normalize_effective_paths*/ false,
);
let protected_git_roots = dedup_absolute_paths(
writable_root_paths
.iter()
.flat_map(|root| managed_git_read_only_subpaths_for_writable_root(root, false))
.chain(
writable_root_paths
.iter()
.filter(|root| path_contains_git_metadata_component(root))
.cloned(),
)
.collect(),
/*normalize_effective_paths*/ true,
);
self.entries.retain(|entry| {
!entry.access.can_write()
|| !is_managed_git_write_entry(entry, cwd, &protected_git_roots)
});
append_default_read_only_project_root_subpath_if_no_explicit_rule(
&mut self.entries,
PROTECTED_METADATA_GIT_PATH_NAME,
);
for git_root in protected_git_roots {
append_default_read_only_path_if_no_explicit_rule(&mut self.entries, git_root);
}
}
fn has_root_access(&self, predicate: impl Fn(FileSystemAccessMode) -> bool) -> bool {
matches!(self.kind, FileSystemSandboxKind::Restricted)
&& self.entries.iter().any(|entry| {
@@ -1749,6 +1792,23 @@ fn pointed_gitdirs_for_writable_roots(writable_roots: &[AbsolutePathBuf]) -> Vec
.collect();
dedup_absolute_paths(gitdirs, /*normalize_effective_paths*/ true)
}
pub(crate) fn add_limited_git_writable_roots(
roots: &mut Vec<WritableRoot>,
writable_root_paths: &[AbsolutePathBuf],
) {
for gitdir in pointed_gitdirs_for_writable_roots(writable_root_paths) {
if roots.iter().any(|root| root.root == gitdir) {
continue;
}
roots.push(WritableRoot {
read_only_subpaths: protected_git_subpaths_when_writable(&gitdir),
protected_metadata_names: Vec::new(),
root: gitdir,
});
}
}
/// Rebuilds the filesystem policy that legacy sandbox runtimes enforce for a
/// concrete cwd.
///
@@ -1841,6 +1901,16 @@ fn protected_git_subpaths_when_writable(gitdir: &AbsolutePathBuf) -> Vec<Absolut
.collect()
}
fn path_contains_git_metadata_component(path: &AbsolutePathBuf) -> bool {
path.as_path().components().any(|component| {
matches!(
component,
std::path::Component::Normal(name)
if name == OsStr::new(PROTECTED_METADATA_GIT_PATH_NAME)
)
})
}
fn append_default_read_only_project_root_subpath_if_no_explicit_rule(
entries: &mut Vec<FileSystemSandboxEntry>,
subpath: impl Into<PathBuf>,
@@ -1877,51 +1947,6 @@ fn append_default_read_only_entry_if_no_explicit_rule(
});
}
fn git_root_protection_children(path: &FileSystemPath) -> Option<[FileSystemPath; 2]> {
match path {
FileSystemPath::Special {
value:
FileSystemSpecialPath::ProjectRoots {
subpath: Some(subpath),
},
} if subpath == Path::new(PROTECTED_METADATA_GIT_PATH_NAME) => Some([
FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(Some(subpath.join("config"))),
},
FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(Some(subpath.join("hooks"))),
},
]),
FileSystemPath::Path { .. }
| FileSystemPath::GlobPattern { .. }
| FileSystemPath::Special { .. } => None,
}
}
fn git_child_protection_root(path: &FileSystemPath) -> Option<FileSystemPath> {
match path {
FileSystemPath::Special {
value:
FileSystemSpecialPath::ProjectRoots {
subpath: Some(subpath),
},
} if matches!(
subpath.as_path(),
path if path == Path::new(".git/config") || path == Path::new(".git/hooks")
) =>
{
Some(FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(Some(PathBuf::from(
PROTECTED_METADATA_GIT_PATH_NAME,
))),
})
}
FileSystemPath::Path { .. }
| FileSystemPath::GlobPattern { .. }
| FileSystemPath::Special { .. } => None,
}
}
fn is_managed_git_protected_write_entry(
entry: &FileSystemSandboxEntry,
cwd: &Path,
@@ -1960,6 +1985,34 @@ fn is_managed_git_protected_write_entry(
}
}
fn is_managed_git_write_entry(
entry: &FileSystemSandboxEntry,
cwd: &Path,
protected_paths: &[AbsolutePathBuf],
) -> bool {
match &entry.path {
FileSystemPath::Special {
value:
FileSystemSpecialPath::ProjectRoots {
subpath: Some(subpath),
},
} => subpath.starts_with(Path::new(PROTECTED_METADATA_GIT_PATH_NAME)),
FileSystemPath::Special { .. } | FileSystemPath::Path { .. } => resolve_file_system_path(
&entry.path,
AbsolutePathBuf::from_absolute_path(cwd).ok().as_ref(),
)
.is_some_and(|path| {
let path = normalize_effective_absolute_path(path);
protected_paths.iter().any(|protected_path| {
path.as_path().starts_with(
normalize_effective_absolute_path(protected_path.clone()).as_path(),
)
})
}),
FileSystemPath::GlobPattern { .. } => false,
}
}
fn has_explicit_resolved_path_entry(
entries: &[ResolvedFileSystemEntry],
path: &AbsolutePathBuf,