mirror of
https://github.com/openai/codex.git
synced 2026-06-01 19:02:59 +00:00
feat: introduce Permissions (#11633)
## Why We currently carry multiple permission-related concepts directly on `Config` for shell/unified-exec behavior (`approval_policy`, `sandbox_policy`, `network`, `shell_environment_policy`, `windows_sandbox_mode`). Consolidating these into one in-memory struct makes permission handling easier to reason about and sets up the next step: supporting named permission profiles (`[permissions.PROFILE_NAME]`) without changing behavior now. This change is mostly mechanical: it updates existing callsites to go through `config.permissions`, but it does not yet refactor those callsites to take a single `Permissions` value in places where multiple permission fields are still threaded separately. This PR intentionally **does not** change the on-disk `config.toml` format yet and keeps compatibility with legacy config keys. ## What Changed - Introduced `Permissions` in `core/src/config/mod.rs`. - Added `Config::permissions` and moved effective runtime permission fields under it: - `approval_policy` - `sandbox_policy` - `network` - `shell_environment_policy` - `windows_sandbox_mode` - Updated config loading/building so these effective values are still derived from the same existing config inputs and constraints. - Updated Windows sandbox helpers/resolution to read/write via `permissions`. - Threaded the new field through all permission consumers across core runtime, app-server, CLI/exec, TUI, and sandbox summary code. - Updated affected tests to reference `config.permissions.*`. - Renamed the struct/field from `EffectivePermissions`/`effective_permissions` to `Permissions`/`permissions` and aligned variable naming accordingly. ## Verification - `just fix -p codex-core -p codex-tui -p codex-cli -p codex-app-server -p codex-exec -p codex-utils-sandbox-summary` - `cargo build -p codex-core -p codex-tui -p codex-cli -p codex-app-server -p codex-exec -p codex-utils-sandbox-summary`
This commit is contained in:
@@ -628,7 +628,7 @@ impl App {
|
||||
|
||||
fn apply_runtime_policy_overrides(&mut self, config: &mut Config) {
|
||||
if let Some(policy) = self.runtime_approval_policy_override.as_ref()
|
||||
&& let Err(err) = config.approval_policy.set(*policy)
|
||||
&& let Err(err) = config.permissions.approval_policy.set(*policy)
|
||||
{
|
||||
tracing::warn!(%err, "failed to carry forward approval policy override");
|
||||
self.chat_widget.add_error_message(format!(
|
||||
@@ -636,7 +636,7 @@ impl App {
|
||||
));
|
||||
}
|
||||
if let Some(policy) = self.runtime_sandbox_policy_override.as_ref()
|
||||
&& let Err(err) = config.sandbox_policy.set(policy.clone())
|
||||
&& let Err(err) = config.permissions.sandbox_policy.set(policy.clone())
|
||||
{
|
||||
tracing::warn!(%err, "failed to carry forward sandbox policy override");
|
||||
self.chat_widget.add_error_message(format!(
|
||||
@@ -1156,7 +1156,7 @@ impl App {
|
||||
let should_check = WindowsSandboxLevel::from_config(&app.config)
|
||||
!= WindowsSandboxLevel::Disabled
|
||||
&& matches!(
|
||||
app.config.sandbox_policy.get(),
|
||||
app.config.permissions.sandbox_policy.get(),
|
||||
codex_core::protocol::SandboxPolicy::WorkspaceWrite { .. }
|
||||
| codex_core::protocol::SandboxPolicy::ReadOnly { .. }
|
||||
)
|
||||
@@ -1170,7 +1170,7 @@ impl App {
|
||||
let env_map: std::collections::HashMap<String, String> = std::env::vars().collect();
|
||||
let tx = app.app_event_tx.clone();
|
||||
let logs_base_dir = app.config.codex_home.clone();
|
||||
let sandbox_policy = app.config.sandbox_policy.get().clone();
|
||||
let sandbox_policy = app.config.permissions.sandbox_policy.get().clone();
|
||||
Self::spawn_world_writable_scan(cwd, env_map, logs_base_dir, sandbox_policy, tx);
|
||||
}
|
||||
}
|
||||
@@ -1835,7 +1835,7 @@ impl App {
|
||||
None,
|
||||
));
|
||||
|
||||
let policy = self.config.sandbox_policy.get().clone();
|
||||
let policy = self.config.permissions.sandbox_policy.get().clone();
|
||||
let policy_cwd = self.config.cwd.clone();
|
||||
let command_cwd = self.config.cwd.clone();
|
||||
let env_map: std::collections::HashMap<String, String> =
|
||||
@@ -1913,8 +1913,9 @@ impl App {
|
||||
self.config.set_windows_sandbox_enabled(true);
|
||||
self.config.set_windows_elevated_sandbox_enabled(false);
|
||||
}
|
||||
self.chat_widget
|
||||
.set_windows_sandbox_mode(self.config.windows_sandbox_mode);
|
||||
self.chat_widget.set_windows_sandbox_mode(
|
||||
self.config.permissions.windows_sandbox_mode,
|
||||
);
|
||||
let windows_sandbox_level =
|
||||
WindowsSandboxLevel::from_config(&self.config);
|
||||
if let Some((sample_paths, extra_count, failed_scan)) =
|
||||
@@ -2060,7 +2061,7 @@ impl App {
|
||||
}
|
||||
AppEvent::UpdateAskForApprovalPolicy(policy) => {
|
||||
self.runtime_approval_policy_override = Some(policy);
|
||||
if let Err(err) = self.config.approval_policy.set(policy) {
|
||||
if let Err(err) = self.config.permissions.approval_policy.set(policy) {
|
||||
tracing::warn!(%err, "failed to set approval policy on app config");
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to set approval policy: {err}"));
|
||||
@@ -2077,7 +2078,7 @@ impl App {
|
||||
);
|
||||
let policy_for_chat = policy.clone();
|
||||
|
||||
if let Err(err) = self.config.sandbox_policy.set(policy) {
|
||||
if let Err(err) = self.config.permissions.sandbox_policy.set(policy) {
|
||||
tracing::warn!(%err, "failed to set sandbox policy on app config");
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to set sandbox policy: {err}"));
|
||||
@@ -2090,7 +2091,7 @@ impl App {
|
||||
return Ok(AppRunControl::Continue);
|
||||
}
|
||||
self.runtime_sandbox_policy_override =
|
||||
Some(self.config.sandbox_policy.get().clone());
|
||||
Some(self.config.permissions.sandbox_policy.get().clone());
|
||||
|
||||
// If sandbox policy becomes workspace-write or read-only, run the Windows world-writable scan.
|
||||
#[cfg(target_os = "windows")]
|
||||
@@ -2111,7 +2112,7 @@ impl App {
|
||||
std::env::vars().collect();
|
||||
let tx = self.app_event_tx.clone();
|
||||
let logs_base_dir = self.config.codex_home.clone();
|
||||
let sandbox_policy = self.config.sandbox_policy.get().clone();
|
||||
let sandbox_policy = self.config.permissions.sandbox_policy.get().clone();
|
||||
Self::spawn_world_writable_scan(
|
||||
cwd,
|
||||
env_map,
|
||||
|
||||
@@ -3323,7 +3323,12 @@ impl ChatWidget {
|
||||
return;
|
||||
};
|
||||
|
||||
if let Err(err) = self.config.approval_policy.can_set(&preset.approval) {
|
||||
if let Err(err) = self
|
||||
.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.can_set(&preset.approval)
|
||||
{
|
||||
self.add_error_message(err.to_string());
|
||||
return;
|
||||
}
|
||||
@@ -3814,8 +3819,8 @@ impl ChatWidget {
|
||||
let op = Op::UserTurn {
|
||||
items,
|
||||
cwd: self.config.cwd.clone(),
|
||||
approval_policy: self.config.approval_policy.value(),
|
||||
sandbox_policy: self.config.sandbox_policy.get().clone(),
|
||||
approval_policy: self.config.permissions.approval_policy.value(),
|
||||
sandbox_policy: self.config.permissions.sandbox_policy.get().clone(),
|
||||
model: effective_mode.model().to_string(),
|
||||
effort: effective_mode.reasoning_effort(),
|
||||
summary: self.config.model_reasoning_summary,
|
||||
@@ -5272,8 +5277,8 @@ impl ChatWidget {
|
||||
/// Open a popup to choose the permissions mode (approval policy + sandbox policy).
|
||||
pub(crate) fn open_permissions_popup(&mut self) {
|
||||
let include_read_only = cfg!(target_os = "windows");
|
||||
let current_approval = self.config.approval_policy.value();
|
||||
let current_sandbox = self.config.sandbox_policy.get();
|
||||
let current_approval = self.config.permissions.approval_policy.value();
|
||||
let current_sandbox = self.config.permissions.sandbox_policy.get();
|
||||
let mut items: Vec<SelectionItem> = Vec::new();
|
||||
let presets: Vec<ApprovalPreset> = builtin_approval_presets();
|
||||
|
||||
@@ -5301,7 +5306,12 @@ impl ChatWidget {
|
||||
preset.label.to_string()
|
||||
};
|
||||
let description = Some(preset.description.replace(" (Identical to Agent mode)", ""));
|
||||
let disabled_reason = match self.config.approval_policy.can_set(&preset.approval) {
|
||||
let disabled_reason = match self
|
||||
.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.can_set(&preset.approval)
|
||||
{
|
||||
Ok(()) => None,
|
||||
Err(err) => Some(err.to_string()),
|
||||
};
|
||||
@@ -5462,7 +5472,7 @@ impl ChatWidget {
|
||||
self.config.codex_home.as_path(),
|
||||
cwd.as_path(),
|
||||
&env_map,
|
||||
self.config.sandbox_policy.get(),
|
||||
self.config.permissions.sandbox_policy.get(),
|
||||
Some(self.config.codex_home.as_path()),
|
||||
) {
|
||||
Ok(_) => None,
|
||||
@@ -5569,7 +5579,7 @@ impl ChatWidget {
|
||||
let mode_label = preset
|
||||
.as_ref()
|
||||
.map(|p| describe_policy(&p.sandbox))
|
||||
.unwrap_or_else(|| describe_policy(self.config.sandbox_policy.get()));
|
||||
.unwrap_or_else(|| describe_policy(self.config.permissions.sandbox_policy.get()));
|
||||
let info_line = if failed_scan {
|
||||
Line::from(vec![
|
||||
"We couldn't complete the world-writable scan, so protections cannot be verified. "
|
||||
@@ -5912,7 +5922,7 @@ impl ChatWidget {
|
||||
|
||||
/// Set the approval policy in the widget's config copy.
|
||||
pub(crate) fn set_approval_policy(&mut self, policy: AskForApproval) {
|
||||
if let Err(err) = self.config.approval_policy.set(policy) {
|
||||
if let Err(err) = self.config.permissions.approval_policy.set(policy) {
|
||||
tracing::warn!(%err, "failed to set approval_policy on chat config");
|
||||
}
|
||||
}
|
||||
@@ -5920,13 +5930,13 @@ impl ChatWidget {
|
||||
/// Set the sandbox policy in the widget's config copy.
|
||||
#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
|
||||
pub(crate) fn set_sandbox_policy(&mut self, policy: SandboxPolicy) -> ConstraintResult<()> {
|
||||
self.config.sandbox_policy.set(policy)?;
|
||||
self.config.permissions.sandbox_policy.set(policy)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg_attr(not(target_os = "windows"), allow(dead_code))]
|
||||
pub(crate) fn set_windows_sandbox_mode(&mut self, mode: Option<WindowsSandboxModeToml>) {
|
||||
self.config.windows_sandbox_mode = mode;
|
||||
self.config.permissions.windows_sandbox_mode = mode;
|
||||
#[cfg(target_os = "windows")]
|
||||
self.bottom_pane.set_windows_degraded_sandbox_active(
|
||||
codex_core::windows_sandbox::ELEVATED_SANDBOX_NUX_ENABLED
|
||||
|
||||
@@ -4510,7 +4510,7 @@ async fn disabled_slash_command_while_task_running_snapshot() {
|
||||
async fn approvals_popup_shows_disabled_presets() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
chat.config.approval_policy =
|
||||
chat.config.permissions.approval_policy =
|
||||
Constrained::new(AskForApproval::OnRequest, |candidate| match candidate {
|
||||
AskForApproval::OnRequest => Ok(()),
|
||||
_ => Err(invalid_value(
|
||||
@@ -4546,7 +4546,7 @@ async fn approvals_popup_shows_disabled_presets() {
|
||||
async fn approvals_popup_navigation_skips_disabled() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
chat.config.approval_policy =
|
||||
chat.config.permissions.approval_policy =
|
||||
Constrained::new(AskForApproval::OnRequest, |candidate| match candidate {
|
||||
AskForApproval::OnRequest => Ok(()),
|
||||
_ => Err(invalid_value(candidate.to_string(), "[on-request]")),
|
||||
@@ -4623,7 +4623,10 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> {
|
||||
// Build a chat widget with manual channels to avoid spawning the agent.
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
// Ensure policy allows surfacing approvals explicitly (not strictly required for direct event).
|
||||
chat.config.approval_policy.set(AskForApproval::OnRequest)?;
|
||||
chat.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)?;
|
||||
// Inject an exec approval request to display the approval modal.
|
||||
let ev = ExecApprovalRequestEvent {
|
||||
call_id: "call-approve-cmd".into(),
|
||||
@@ -4678,7 +4681,10 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> {
|
||||
#[tokio::test]
|
||||
async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
chat.config.approval_policy.set(AskForApproval::OnRequest)?;
|
||||
chat.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)?;
|
||||
|
||||
let ev = ExecApprovalRequestEvent {
|
||||
call_id: "call-approve-cmd-noreason".into(),
|
||||
@@ -4720,7 +4726,10 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> {
|
||||
async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot()
|
||||
-> anyhow::Result<()> {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
chat.config.approval_policy.set(AskForApproval::OnRequest)?;
|
||||
chat.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)?;
|
||||
|
||||
let script = "python - <<'PY'\nprint('hello')\nPY".to_string();
|
||||
let command = vec!["bash".into(), "-lc".into(), script];
|
||||
@@ -4760,7 +4769,10 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot()
|
||||
#[tokio::test]
|
||||
async fn approval_modal_patch_snapshot() -> anyhow::Result<()> {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
chat.config.approval_policy.set(AskForApproval::OnRequest)?;
|
||||
chat.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)?;
|
||||
|
||||
// Build a small changeset and a reason/grant_root to exercise the prompt text.
|
||||
let mut changes = HashMap::new();
|
||||
@@ -5529,7 +5541,10 @@ async fn apply_patch_full_flow_integration_like() {
|
||||
async fn apply_patch_untrusted_shows_approval_modal() -> anyhow::Result<()> {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
// Ensure approval policy is untrusted (OnRequest)
|
||||
chat.config.approval_policy.set(AskForApproval::OnRequest)?;
|
||||
chat.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)?;
|
||||
|
||||
// Simulate a patch approval request from backend
|
||||
let mut changes = HashMap::new();
|
||||
@@ -5577,7 +5592,10 @@ async fn apply_patch_request_shows_diff_summary() -> anyhow::Result<()> {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
// Ensure we are in OnRequest so an approval is surfaced
|
||||
chat.config.approval_policy.set(AskForApproval::OnRequest)?;
|
||||
chat.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)?;
|
||||
|
||||
// Simulate backend asking to apply a patch adding two lines to README.md
|
||||
let mut changes = HashMap::new();
|
||||
|
||||
@@ -33,6 +33,7 @@ pub(crate) fn new_debug_config_output(
|
||||
http_addr,
|
||||
socks_addr,
|
||||
config
|
||||
.permissions
|
||||
.network
|
||||
.as_ref()
|
||||
.is_some_and(codex_core::config::NetworkProxySpec::socks_enabled),
|
||||
|
||||
@@ -290,7 +290,9 @@ pub async fn run_main(
|
||||
.await;
|
||||
set_default_client_residency_requirement(config.enforce_residency.value());
|
||||
|
||||
if let Some(warning) = add_dir_warning_message(&cli.add_dir, config.sandbox_policy.get()) {
|
||||
if let Some(warning) =
|
||||
add_dir_warning_message(&cli.add_dir, config.permissions.sandbox_policy.get())
|
||||
{
|
||||
#[allow(clippy::print_stderr)]
|
||||
{
|
||||
eprintln!("Error adding directories: {warning}");
|
||||
@@ -1004,8 +1006,8 @@ mod tests {
|
||||
TurnContextItem {
|
||||
turn_id: None,
|
||||
cwd,
|
||||
approval_policy: config.approval_policy.value(),
|
||||
sandbox_policy: config.sandbox_policy.get().clone(),
|
||||
approval_policy: config.permissions.approval_policy.value(),
|
||||
sandbox_policy: config.permissions.sandbox_policy.get().clone(),
|
||||
model,
|
||||
personality: None,
|
||||
collaboration_mode: None,
|
||||
@@ -1123,7 +1125,7 @@ trust_level = "untrusted"
|
||||
.build()
|
||||
.await?;
|
||||
assert_eq!(
|
||||
trusted_config.approval_policy.value(),
|
||||
trusted_config.permissions.approval_policy.value(),
|
||||
AskForApproval::OnRequest
|
||||
);
|
||||
|
||||
@@ -1137,7 +1139,7 @@ trust_level = "untrusted"
|
||||
.build()
|
||||
.await?;
|
||||
assert_eq!(
|
||||
untrusted_config.approval_policy.value(),
|
||||
untrusted_config.permissions.approval_policy.value(),
|
||||
AskForApproval::UnlessTrusted
|
||||
);
|
||||
Ok(())
|
||||
|
||||
@@ -168,10 +168,13 @@ impl StatusHistoryCell {
|
||||
("workdir", config.cwd.display().to_string()),
|
||||
("model", model_name.to_string()),
|
||||
("provider", config.model_provider_id.clone()),
|
||||
("approval", config.approval_policy.value().to_string()),
|
||||
(
|
||||
"approval",
|
||||
config.permissions.approval_policy.value().to_string(),
|
||||
),
|
||||
(
|
||||
"sandbox",
|
||||
summarize_sandbox_policy(config.sandbox_policy.get()),
|
||||
summarize_sandbox_policy(config.permissions.sandbox_policy.get()),
|
||||
),
|
||||
];
|
||||
if config.model_provider.wire_api == WireApi::Responses {
|
||||
@@ -191,7 +194,7 @@ impl StatusHistoryCell {
|
||||
.find(|(k, _)| *k == "approval")
|
||||
.map(|(_, v)| v.clone())
|
||||
.unwrap_or_else(|| "<unknown>".to_string());
|
||||
let sandbox = match config.sandbox_policy.get() {
|
||||
let sandbox = match config.permissions.sandbox_policy.get() {
|
||||
SandboxPolicy::DangerFullAccess => "danger-full-access".to_string(),
|
||||
SandboxPolicy::ReadOnly { .. } => "read-only".to_string(),
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
@@ -207,12 +210,13 @@ impl StatusHistoryCell {
|
||||
}
|
||||
}
|
||||
};
|
||||
let permissions = if config.approval_policy.value() == AskForApproval::OnRequest
|
||||
&& *config.sandbox_policy.get() == SandboxPolicy::new_workspace_write_policy()
|
||||
let permissions = if config.permissions.approval_policy.value() == AskForApproval::OnRequest
|
||||
&& *config.permissions.sandbox_policy.get()
|
||||
== SandboxPolicy::new_workspace_write_policy()
|
||||
{
|
||||
"Default".to_string()
|
||||
} else if config.approval_policy.value() == AskForApproval::Never
|
||||
&& *config.sandbox_policy.get() == SandboxPolicy::DangerFullAccess
|
||||
} else if config.permissions.approval_policy.value() == AskForApproval::Never
|
||||
&& *config.permissions.sandbox_policy.get() == SandboxPolicy::DangerFullAccess
|
||||
{
|
||||
"Full Access".to_string()
|
||||
} else {
|
||||
|
||||
@@ -98,6 +98,7 @@ async fn status_snapshot_includes_reasoning_details() {
|
||||
config.model_provider_id = "openai".to_string();
|
||||
config.model_reasoning_summary = ReasoningSummary::Detailed;
|
||||
config
|
||||
.permissions
|
||||
.sandbox_policy
|
||||
.set(SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: Vec::new(),
|
||||
@@ -177,10 +178,12 @@ async fn status_permissions_non_default_workspace_write_is_custom() {
|
||||
config.model = Some("gpt-5.1-codex-max".to_string());
|
||||
config.model_provider_id = "openai".to_string();
|
||||
config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(AskForApproval::OnRequest)
|
||||
.expect("set approval policy");
|
||||
config
|
||||
.permissions
|
||||
.sandbox_policy
|
||||
.set(SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: Vec::new(),
|
||||
|
||||
Reference in New Issue
Block a user