openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-05 15:18:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(permissions): use effective filesystem permissions for enforcement preflight

Browse Source 
Co-authored-by: Codex noreply@openai.com
This commit is contained in:
viyatb-oai
2026-05-27 17:28:05 -07:00
parent e7864acdbe
commit 4418bce894
8 changed files with 123 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
codex-rs/core/src/sandboxing/mod.rs
View File

@@ -122,6 +122,7 @@ impl ExecRequest {
windows_sandbox_level,
windows_sandbox_private_desktop,
permission_profile,
effective_filesystem_permissions: _,
file_system_sandbox_policy,
network_sandbox_policy,
arg0,

53
codex-rs/core/src/tools/handlers/apply_patch.rs
View File

@@ -46,7 +46,9 @@ use codex_protocol::models::FileSystemPermissions;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::FileChange;
use codex_protocol::protocol::PatchApplyUpdatedEvent;
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
use codex_sandboxing::EffectiveFilesystemPermissions;
use codex_sandboxing::FilesystemPermissionsContext;
use codex_sandboxing::policy_transforms::effective_permission_profile;
use codex_sandboxing::policy_transforms::merge_permission_profiles;
use codex_sandboxing::policy_transforms::normalize_additional_permissions;
use codex_tools::ToolName;
@@ -224,8 +226,7 @@ fn to_abs_path(cwd: &AbsolutePathBuf, path: &Path) -> Option<AbsolutePathBuf> {
fn write_permissions_for_paths(
file_paths: &[AbsolutePathBuf],
file_system_sandbox_policy: &codex_protocol::permissions::FileSystemSandboxPolicy,
cwd: &AbsolutePathBuf,
effective_filesystem_permissions: &EffectiveFilesystemPermissions,
) -> Option<AdditionalPermissionProfile> {
let write_paths = file_paths
.iter()
@@ -234,9 +235,7 @@ fn write_permissions_for_paths(
.unwrap_or_else(|| path.clone())
.into_path_buf()
})
.filter(|path| {
!file_system_sandbox_policy.can_write_path_with_cwd(path.as_path(), cwd.as_path())
})
.filter(|path| !effective_filesystem_permissions.can_write(path.as_path()))
.collect::<BTreeSet<_>>()
.into_iter()
.map(AbsolutePathBuf::from_absolute_path)
@@ -267,34 +266,46 @@ async fn effective_patch_permissions(
turn: &TurnContext,
action: &ApplyPatchAction,
cwd: &AbsolutePathBuf,
) -> (
Vec<AbsolutePathBuf>,
crate::tools::handlers::EffectiveAdditionalPermissions,
codex_protocol::permissions::FileSystemSandboxPolicy,
) {
) -> Result<
(
Vec<AbsolutePathBuf>,
crate::tools::handlers::EffectiveAdditionalPermissions,
codex_protocol::permissions::FileSystemSandboxPolicy,
),
FunctionCallError,
> {
let file_paths = file_paths_for_action(action);
let granted_permissions = merge_permission_profiles(
session.granted_session_permissions().await.as_ref(),
session.granted_turn_permissions().await.as_ref(),
);
let base_file_system_sandbox_policy = turn.file_system_sandbox_policy();
let file_system_sandbox_policy = effective_file_system_sandbox_policy(
&base_file_system_sandbox_policy,
granted_permissions.as_ref(),
);
let effective_permission_profile =
effective_permission_profile(&turn.permission_profile(), granted_permissions.as_ref());
let file_system_sandbox_policy = effective_permission_profile.file_system_sandbox_policy();
let effective_filesystem_permissions = EffectiveFilesystemPermissions::from_profile(
&effective_permission_profile,
FilesystemPermissionsContext {
policy_evaluation_cwd: cwd,
},
)
.map_err(|err| {
FunctionCallError::RespondToModel(format!(
"failed to derive effective filesystem permissions for apply_patch: {err}"
))
})?;
let effective_additional_permissions = apply_granted_turn_permissions(
session,
cwd.as_path(),
crate::sandboxing::SandboxPermissions::UseDefault,
write_permissions_for_paths(&file_paths, &file_system_sandbox_policy, cwd),
write_permissions_for_paths(&file_paths, &effective_filesystem_permissions),
)
.await;
(
Ok((
file_paths,
effective_additional_permissions,
file_system_sandbox_policy,
)
))
}
#[async_trait::async_trait]
@@ -354,7 +365,7 @@ impl ToolExecutor<ToolInvocation> for ApplyPatchHandler {
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
let (file_paths, effective_additional_permissions, file_system_sandbox_policy) =
effective_patch_permissions(session.as_ref(), turn.as_ref(), &changes, &cwd)
.await;
.await?;
match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes)
.await
{
@@ -506,7 +517,7 @@ pub(crate) async fn intercept_apply_patch(
{
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
let (approval_keys, effective_additional_permissions, file_system_sandbox_policy) =
effective_patch_permissions(session.as_ref(), turn.as_ref(), &changes, cwd).await;
effective_patch_permissions(session.as_ref(), turn.as_ref(), &changes, cwd).await?;
match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes)
.await
{

26
codex-rs/core/src/tools/handlers/apply_patch_tests.rs
View File

@@ -1,7 +1,9 @@
use super::*;
use codex_apply_patch::MaybeApplyPatchVerified;
use codex_exec_server::LOCAL_FS;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::FileChange;
use core_test_support::PathBufExt;
use core_test_support::PathExt;
@@ -27,6 +29,24 @@ fn sample_patch() -> &'static str {
*** End Patch"#
}
fn effective_permissions(
sandbox_policy: &FileSystemSandboxPolicy,
cwd: &AbsolutePathBuf,
) -> codex_sandboxing::EffectiveFilesystemPermissions {
let permission_profile = PermissionProfile::from_runtime_permissions(
sandbox_policy,
NetworkSandboxPolicy::Restricted,
)
.materialize_project_roots_with_workspace_roots(std::slice::from_ref(cwd));
codex_sandboxing::EffectiveFilesystemPermissions::from_profile(
&permission_profile,
codex_sandboxing::FilesystemPermissionsContext {
policy_evaluation_cwd: cwd,
},
)
.expect("derive effective filesystem permissions")
}
async fn invocation_for_payload(payload: ToolPayload) -> ToolInvocation {
let (session, turn) = make_session_and_context().await;
ToolInvocation {
@@ -255,8 +275,9 @@ fn write_permissions_for_paths_skip_dirs_already_writable_under_workspace_root()
/*exclude_tmpdir_env_var*/ true,
/*exclude_slash_tmp*/ false,
);
let effective_permissions = effective_permissions(&sandbox_policy, &cwd);
let permissions = write_permissions_for_paths(&[file_path], &sandbox_policy, &cwd);
let permissions = write_permissions_for_paths(&[file_path], &effective_permissions);
assert_eq!(permissions, None);
}
@@ -276,8 +297,9 @@ fn write_permissions_for_paths_keep_dirs_outside_workspace_root() {
/*exclude_tmpdir_env_var*/ true,
/*exclude_slash_tmp*/ true,
);
let effective_permissions = effective_permissions(&sandbox_policy, &cwd_abs);
let permissions = write_permissions_for_paths(&[file_path], &sandbox_policy, &cwd_abs);
let permissions = write_permissions_for_paths(&[file_path], &effective_permissions);
let expected_outside =
dunce::simplified(&outside.canonicalize().expect("canonicalize outside dir")).abs();

8
codex-rs/sandboxing/src/effective_filesystem_permissions.rs
View File

@@ -25,7 +25,7 @@ pub enum FilesystemPermissionsMode {
External,
}
/// A deny-read glob accepted by the existing runtime matcher.
/// A deny-read glob retained in effective filesystem enforcement inputs.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ValidatedDenyGlob {
pattern: String,
@@ -83,9 +83,11 @@ impl EffectiveFilesystemPermissions {
if contains_unmaterialized_workspace_roots(&file_system_policy) {
return Err(FilesystemPermissionsError::UnmaterializedWorkspaceRoots);
}
// Direct enforcement queries have historically failed closed for malformed
// deny patterns. Platform lowering that expands concrete targets can still
// validate the patterns before acting on the filesystem.
let read_deny_matcher =
ReadDenyMatcher::try_new(&file_system_policy, context.policy_evaluation_cwd.as_path())
.map_err(FilesystemPermissionsError::InvalidDenyGlob)?;
ReadDenyMatcher::new(&file_system_policy, context.policy_evaluation_cwd.as_path());
let mode = match file_system_policy.kind {
FileSystemSandboxKind::Restricted => FilesystemPermissionsMode::Restricted,
FileSystemSandboxKind::Unrestricted => FilesystemPermissionsMode::Unrestricted,

25
codex-rs/sandboxing/src/effective_filesystem_permissions_tests.rs
View File

@@ -252,3 +252,28 @@ fn effective_permissions_preserve_accepted_deny_glob_matching() {
);
assert_eq!(effective.can_read(denied_path.as_path()), false);
}
#[test]
fn effective_permissions_fail_closed_for_malformed_deny_globs() {
let temp_dir = TempDir::new().expect("temp dir");
let cwd = absolute_path(temp_dir.path());
let readable_path = cwd.join("readable.txt");
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path { path: cwd.clone() },
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::GlobPattern {
pattern: format!("{}/**/[z-a]", cwd.as_path().display()),
},
access: FileSystemAccessMode::Deny,
},
]);
let permission_profile =
PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted);
let effective = derive_effective(&permission_profile, &cwd);
assert_eq!(effective.is_read_denied(readable_path.as_path()), true);
assert_eq!(effective.can_read(readable_path.as_path()), false);
}

6
codex-rs/sandboxing/src/lib.rs
View File

@@ -41,6 +41,12 @@ impl From<SandboxTransformError> for CodexErr {
SandboxTransformError::MissingLinuxSandboxExecutable => {
CodexErr::LandlockSandboxExecutableNotProvided
}
SandboxTransformError::InvalidPermissionProfileCwd(message) => {
CodexErr::UnsupportedOperation(message)
}
SandboxTransformError::EffectiveFilesystemPermissions(err) => {
CodexErr::UnsupportedOperation(err.to_string())
}
#[cfg(target_os = "linux")]
SandboxTransformError::Wsl1UnsupportedForBubblewrap => {
CodexErr::UnsupportedOperation(crate::bwrap::WSL1_BWRAP_WARNING.to_string())

18
codex-rs/sandboxing/src/manager.rs
View File

@@ -1,3 +1,6 @@
use crate::EffectiveFilesystemPermissions;
use crate::FilesystemPermissionsContext;
use crate::FilesystemPermissionsError;
#[cfg(target_os = "linux")]
use crate::bwrap::WSL1_BWRAP_WARNING;
#[cfg(target_os = "linux")]
@@ -80,6 +83,7 @@ pub struct SandboxExecRequest {
pub windows_sandbox_level: WindowsSandboxLevel,
pub windows_sandbox_private_desktop: bool,
pub permission_profile: PermissionProfile,
pub effective_filesystem_permissions: EffectiveFilesystemPermissions,
pub file_system_sandbox_policy: FileSystemSandboxPolicy,
pub network_sandbox_policy: NetworkSandboxPolicy,
pub arg0: Option<String>,
@@ -106,6 +110,8 @@ pub struct SandboxTransformRequest<'a> {
#[derive(Debug)]
pub enum SandboxTransformError {
MissingLinuxSandboxExecutable,
InvalidPermissionProfileCwd(String),
EffectiveFilesystemPermissions(FilesystemPermissionsError),
#[cfg(target_os = "linux")]
Wsl1UnsupportedForBubblewrap,
#[cfg(not(target_os = "macos"))]
@@ -118,6 +124,8 @@ impl std::fmt::Display for SandboxTransformError {
Self::MissingLinuxSandboxExecutable => {
write!(f, "missing codex-linux-sandbox executable path")
}
Self::InvalidPermissionProfileCwd(message) => f.write_str(message),
Self::EffectiveFilesystemPermissions(err) => write!(f, "{err}"),
#[cfg(target_os = "linux")]
Self::Wsl1UnsupportedForBubblewrap => write!(f, "{WSL1_BWRAP_WARNING}"),
#[cfg(not(target_os = "macos"))]
@@ -186,6 +194,15 @@ impl SandboxManager {
effective_permission_profile(permissions, additional_permissions.as_ref());
let (effective_file_system_policy, effective_network_policy) =
effective_permission_profile.to_runtime_permissions();
let policy_evaluation_cwd = AbsolutePathBuf::from_absolute_path(sandbox_policy_cwd)
.map_err(|err| SandboxTransformError::InvalidPermissionProfileCwd(err.to_string()))?;
let effective_filesystem_permissions = EffectiveFilesystemPermissions::from_profile(
&effective_permission_profile,
FilesystemPermissionsContext {
policy_evaluation_cwd: &policy_evaluation_cwd,
},
)
.map_err(SandboxTransformError::EffectiveFilesystemPermissions)?;
let mut argv = Vec::with_capacity(1 + command.args.len());
argv.push(command.program);
argv.extend(command.args.into_iter().map(OsString::from));
@@ -253,6 +270,7 @@ impl SandboxManager {
windows_sandbox_level,
windows_sandbox_private_desktop,
permission_profile: effective_permission_profile,
effective_filesystem_permissions,
file_system_sandbox_policy: effective_file_system_policy,
network_sandbox_policy: effective_network_policy,
arg0: arg0_override,

12
codex-rs/sandboxing/src/manager_tests.rs
View File

@@ -217,6 +217,18 @@ fn transform_additional_permissions_preserves_denied_entries() {
})
.expect("transform");
assert_eq!(
exec_request
.effective_filesystem_permissions
.can_write(allowed_path.as_path()),
true
);
assert_eq!(
exec_request
.effective_filesystem_permissions
.can_read(denied_path.as_path()),
false
);
assert_eq!(
exec_request.file_system_sandbox_policy,
FileSystemSandboxPolicy::restricted(vec![