Skip to content

Preserve managed deny-read rules across permission updates #40004

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
copyberry merged 1 commit into from
Aug 21, 2026
+290 −33
Merged

Preserve managed deny-read rules across permission updates #40004

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
22 changes: 4 additions & 18 deletions codex-rs/app-server/src/request_processors/command_exec_processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,26 +238,12 @@ impl CommandExecRequestProcessor {
config.effective_workspace_roots(),
)
} else if let Some(policy) = sandbox_policy.map(|policy| policy.to_core()) {
self.config
.permissions
.can_set_legacy_sandbox_policy(&policy, &sandbox_cwd)
.map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?;
let file_system_sandbox_policy =
codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, &sandbox_cwd);
let network_sandbox_policy =
codex_protocol::permissions::NetworkSandboxPolicy::from(&policy);
let permission_profile =
codex_protocol::models::PermissionProfile::from_runtime_permissions_with_enforcement(
codex_protocol::models::SandboxEnforcement::from_legacy_sandbox_policy(&policy),
&file_system_sandbox_policy,
network_sandbox_policy,
);
self.config
.permissions
.can_set_permission_profile(&permission_profile)
let mut permissions = self.config.permissions.clone();
permissions
.set_legacy_sandbox_policy(policy, &sandbox_cwd)
.map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?;
(
permission_profile,
permissions.effective_permission_profile(),
self.config.permissions.network.clone(),
self.config.permissions.permission_profile().clone(),
self.config.managed_network_requirements_enabled(),
Expand Down
113 changes: 113 additions & 0 deletions codex-rs/app-server/tests/suite/v2/command_exec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,119 @@ async fn command_exec_accepts_permission_profile() -> Result<()> {
Ok(())
}

#[cfg(unix)]
#[tokio::test]
async fn command_exec_enforces_managed_deny_read_requirements() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri(), "never")?;
let denied_root = codex_home.path().join("private");
let nested_root = denied_root.join("nested");
std::fs::create_dir_all(&nested_root)?;
let denied_path = nested_root.join("secret.txt");
std::fs::write(&denied_path, "managed secret")?;
let user_denied_root = codex_home.path().join("user-private");
std::fs::create_dir_all(&user_denied_root)?;
let user_denied_path = user_denied_root.join("secret.txt");
std::fs::write(&user_denied_path, "user secret")?;
std::fs::write(
codex_home.path().join("requirements.toml"),
format!("[permissions.filesystem]\ndeny_read = [{denied_root:?}]\n"),
)?;
insert_command_exec_config(
codex_home.path(),
&format!(
"default_permissions = \"thread-policy\"\n\n[permissions.thread-policy.filesystem]\n\":root\" = \"read\"\n{user_denied_root:?} = \"deny\"\n\n[permissions.nested.filesystem]\n\":root\" = \"read\"\n{nested_root:?} = \"write\"\n"
),
)?;
let mut app_server = TestAppServer::builder()
.with_codex_home(codex_home.path())
.without_auto_env()
.build_initialized_with_timeout(DEFAULT_READ_TIMEOUT)
.await?;

let request = CommandExecParams {
command: vec![
"sh".to_string(),
"-c".to_string(),
"cat \"$1\"".to_string(),
"sh".to_string(),
denied_path.to_string_lossy().into_owned(),
],
process_id: None,
tty: false,
stream_stdin: false,
stream_stdout_stderr: false,
output_bytes_cap: None,
disable_output_cap: false,
disable_timeout: false,
timeout_ms: None,
cwd: Some(codex_home.path().to_path_buf()),
env: None,
size: None,
sandbox_policy: Some(SandboxPolicy::ReadOnly {
network_access: false,
}),
permission_profile: None,
};
let request_id = app_server
.send_command_exec_request(request.clone())
.await?;
let response: CommandExecResponse = app_server.read_response(request_id).await?;
assert_ne!(response.exit_code, 0);
assert!(!response.stdout.contains("managed secret"));

let mut user_request = request.clone();
*user_request
.command
.last_mut()
.expect("cat command includes a file path") = user_denied_path.to_string_lossy().into();
let request_id = app_server.send_command_exec_request(user_request).await?;
let response: CommandExecResponse = app_server.read_response(request_id).await?;
assert_eq!(
response,
CommandExecResponse {
exit_code: 0,
stdout: "user secret".to_string(),
stderr: String::new(),
}
);

for sandbox_policy in [
SandboxPolicy::DangerFullAccess,
SandboxPolicy::WorkspaceWrite {
writable_roots: vec![nested_root.try_into()?],
network_access: false,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
},
] {
let request_id = app_server
.send_command_exec_request(CommandExecParams {
sandbox_policy: Some(sandbox_policy),
..request.clone()
})
.await?;
let error = app_server
.read_stream_until_error_message(RequestId::Integer(request_id))
.await?;
assert!(error.error.message.contains("invalid sandbox policy"));
}

let request_id = app_server
.send_command_exec_request(CommandExecParams {
sandbox_policy: None,
permission_profile: Some("nested".to_string()),
..request
})
.await?;
let error = app_server
.read_stream_until_error_message(RequestId::Integer(request_id))
.await?;
assert!(error.error.message.contains("invalid permission profile"));
Ok(())
}

#[tokio::test]
async fn command_exec_permission_profile_starts_selected_network_proxy() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
Expand Down
114 changes: 99 additions & 15 deletions codex-rs/core/src/config/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,10 @@ use codex_protocol::models::SandboxEnforcement;
use codex_protocol::openai_models::ModelMessages;
use codex_protocol::openai_models::ModelsResponse;
use codex_protocol::openai_models::ReasoningEffort;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::permissions::ReadDenyMatcher;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::MultiAgentVersion;
use codex_protocol::protocol::SandboxPolicy;
Expand Down Expand Up @@ -297,6 +299,8 @@ pub struct Permissions {
/// Constrained permission profile plus its selected profile identity, if
/// the profile came from a built-in or named config profile.
permission_profile_state: PermissionProfileState,
/// Managed deny-read rules retained independently from user-defined denies.
managed_deny_read_policy: Option<Arc<FileSystemSandboxPolicy>>,
/// Thread-scoped runtime workspace roots. Symbolic `:workspace_roots`
/// entries in the permission profile are materialized against these roots.
workspace_roots: Vec<AbsolutePathBuf>,
Expand Down Expand Up @@ -332,6 +336,7 @@ impl Permissions {
permission_profile_state: PermissionProfileState::from_constrained_legacy(
permission_profile,
)?,
managed_deny_read_policy: None,
workspace_roots: Vec::new(),
network: None,
allow_login_shell: true,
Expand Down Expand Up @@ -388,8 +393,10 @@ impl Permissions {
&self,
permission_profile: &PermissionProfile,
) -> ConstraintResult<()> {
let permission_profile =
self.permission_profile_preserving_managed_denied_reads(permission_profile.clone());
self.permission_profile_state
.can_set_legacy_permission_profile(permission_profile)
.can_set_legacy_permission_profile(&permission_profile)
}

pub fn set_workspace_roots(&mut self, workspace_roots: Vec<AbsolutePathBuf>) {
Expand Down Expand Up @@ -455,8 +462,7 @@ impl Permissions {
&file_system_sandbox_policy,
network_sandbox_policy,
);
self.permission_profile_state
.can_set_legacy_permission_profile(&permission_profile)
self.can_set_permission_profile(&permission_profile)
}

/// Set permissions from a legacy sandbox policy and keep every permission
Expand Down Expand Up @@ -496,19 +502,42 @@ impl Permissions {
],
};

self.permission_profile_state
.set_legacy_permission_profile(permission_profile)?;
Ok(())
self.set_permission_profile(permission_profile)
}

/// Set permissions from the canonical profile.
pub fn set_permission_profile(
&mut self,
permission_profile: PermissionProfile,
) -> ConstraintResult<()> {
let permission_profile =
self.permission_profile_preserving_managed_denied_reads(permission_profile);
self.permission_profile_state
.set_legacy_permission_profile(permission_profile)
}

fn permission_profile_preserving_managed_denied_reads(
&self,
permission_profile: PermissionProfile,
) -> PermissionProfile {
let Some(managed_deny_read_policy) = self.managed_deny_read_policy.as_ref() else {
return permission_profile;
};
if matches!(
sandbox_mode_requirement_for_permission_profile(&permission_profile),
SandboxModeRequirement::DangerFullAccess | SandboxModeRequirement::ExternalSandbox
) {
return permission_profile;
}
let enforcement = permission_profile.enforcement();
let (mut file_system_policy, network_policy) = permission_profile.to_runtime_permissions();
file_system_policy.preserve_deny_read_restrictions_from(managed_deny_read_policy);
PermissionProfile::from_runtime_permissions_with_enforcement(
enforcement,
&file_system_policy,
network_policy,
)
}
}

// A profile override only inherits the selected profile's proxy/allowlist config
Expand Down Expand Up @@ -3985,15 +4014,17 @@ impl Config {
effective_file_system_sandbox_policy
.preserve_deny_read_restrictions_from(&file_system_sandbox_policy);
}
if let Some(Sourced {
value: filesystem_requirements,
..
}) = filesystem_requirements.as_ref()
{
apply_managed_filesystem_constraints(
&mut effective_file_system_sandbox_policy,
filesystem_requirements,
);
let managed_deny_read_policy = filesystem_requirements
.as_ref()
.filter(|Sourced { value, .. }| !value.deny_read.is_empty())
.map(|Sourced { value, .. }| {
let mut policy = FileSystemSandboxPolicy::restricted(Vec::new());
apply_managed_filesystem_constraints(&mut policy, value);
Arc::new(policy)
});
if let Some(managed_deny_read_policy) = managed_deny_read_policy.as_ref() {
effective_file_system_sandbox_policy
.preserve_deny_read_restrictions_from(managed_deny_read_policy);
}
let effective_file_system_sandbox_policy = effective_file_system_sandbox_policy
.with_additional_readable_roots(resolved_cwd.as_path(), &helper_readable_roots);
Expand All @@ -4006,6 +4037,58 @@ impl Config {
.value
.set(effective_permission_profile)
.map_err(std::io::Error::from)?;
if let Some(Sourced {
source: requirement_source,
..
}) = filesystem_requirements.as_ref()
&& let Some(managed_file_system_policy) = managed_deny_read_policy.as_ref()
{
let _initial_matcher =
ReadDenyMatcher::try_new(managed_file_system_policy, resolved_cwd.as_path())
.map_err(std::io::Error::other)?;
let managed_file_system_policy = Arc::clone(managed_file_system_policy);
let permission_cwd = resolved_cwd.clone();
let requirement_source = requirement_source.clone();
constrained_permission_profile
.value
.add_validator(move |permission_profile| {
let managed_deny_matcher =
ReadDenyMatcher::new(&managed_file_system_policy, permission_cwd.as_path());
let file_system_policy = permission_profile.file_system_sandbox_policy();
let missing_required_deny = managed_file_system_policy
.entries
.iter()
.any(|entry| !file_system_policy.entries.contains(entry));
let violating_root = file_system_policy
.entries
.iter()
.filter(|entry| entry.access.can_read())
.find_map(|entry| {
let FileSystemPath::Path { path } = &entry.path else {
return None;
};
let path = path.to_abs_path().ok()?;
managed_deny_matcher
.as_ref()
.is_some_and(|matcher| matcher.is_read_denied(path.as_path()))
.then_some(path)
});
if missing_required_deny || violating_root.is_some() {
return Err(ConstraintError::InvalidValue {
field_name: "permissions.filesystem",
candidate: violating_root
.map_or_else(|| "missing managed deny".to_string(), |path| {
path.to_string_lossy().into_owned()
}),
allowed: "all managed deny_read restrictions".to_string(),
requirement_source: requirement_source.clone(),
});
}

Ok(())
})
.map_err(std::io::Error::from)?;
}
let permission_profile_state = PermissionProfileState::from_constrained_active_profile(
constrained_permission_profile.value,
active_permission_profile,
Expand All @@ -4031,6 +4114,7 @@ impl Config {
permissions: Permissions {
approval_policy: constrained_approval_policy.value,
permission_profile_state,
managed_deny_read_policy,
workspace_roots,
network,
allow_login_shell,
Expand Down
Loading
Loading