mirror of
https://github.com/openai/codex.git
synced 2026-09-08 15:50:34 +00:00
fix: propagate runtime permissions to active session
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -530,7 +530,7 @@ impl App {
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox_policy,
|
||||
permission_profile,
|
||||
permission_profile: _,
|
||||
model,
|
||||
effort,
|
||||
summary,
|
||||
@@ -614,7 +614,11 @@ impl App {
|
||||
approvals_reviewer
|
||||
.unwrap_or(self.chat_widget.config_ref().approvals_reviewer),
|
||||
sandbox_policy.clone(),
|
||||
permission_profile.clone(),
|
||||
runtime_permission_profile_for_turn_start(
|
||||
self.runtime_sandbox_policy_override.as_ref(),
|
||||
sandbox_policy,
|
||||
cwd.as_path(),
|
||||
),
|
||||
model.to_string(),
|
||||
effort,
|
||||
*summary,
|
||||
@@ -1482,3 +1486,56 @@ impl App {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
fn runtime_permission_profile_for_turn_start(
|
||||
runtime_sandbox_policy_override: Option<&SandboxPolicy>,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
cwd: &std::path::Path,
|
||||
) -> Option<codex_protocol::models::PermissionProfile> {
|
||||
runtime_sandbox_policy_override?;
|
||||
match sandbox_policy {
|
||||
SandboxPolicy::ExternalSandbox { .. } => None,
|
||||
SandboxPolicy::ReadOnly { .. }
|
||||
| SandboxPolicy::WorkspaceWrite { .. }
|
||||
| SandboxPolicy::DangerFullAccess => Some(
|
||||
codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy(
|
||||
sandbox_policy,
|
||||
cwd,
|
||||
),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn runtime_permission_profile_for_turn_start_only_when_sandbox_was_overridden() {
|
||||
let cwd = std::path::Path::new("/tmp/project");
|
||||
let sandbox_policy = SandboxPolicy::DangerFullAccess;
|
||||
|
||||
assert_eq!(
|
||||
runtime_permission_profile_for_turn_start(
|
||||
/*runtime_sandbox_policy_override*/ None,
|
||||
&sandbox_policy,
|
||||
cwd,
|
||||
),
|
||||
None
|
||||
);
|
||||
|
||||
let profile =
|
||||
runtime_permission_profile_for_turn_start(Some(&sandbox_policy), &sandbox_policy, cwd)
|
||||
.expect("runtime sandbox override should send active permissions");
|
||||
|
||||
assert_eq!(
|
||||
profile,
|
||||
codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy(
|
||||
&sandbox_policy,
|
||||
cwd,
|
||||
)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -548,7 +548,7 @@ impl AppServerSession {
|
||||
let request_id = self.next_request_id();
|
||||
let (sandbox_policy, permission_profile) = turn_start_permission_overrides(
|
||||
self.thread_params_mode(),
|
||||
sandbox_policy,
|
||||
&sandbox_policy,
|
||||
permission_profile,
|
||||
);
|
||||
self.client
|
||||
@@ -1107,19 +1107,19 @@ fn sandbox_mode_from_policy(
|
||||
}
|
||||
|
||||
fn turn_start_permission_overrides(
|
||||
mode: ThreadParamsMode,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
thread_params_mode: ThreadParamsMode,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
permission_profile: Option<PermissionProfile>,
|
||||
) -> (
|
||||
Option<codex_app_server_protocol::SandboxPolicy>,
|
||||
Option<codex_app_server_protocol::PermissionProfile>,
|
||||
) {
|
||||
match (mode, permission_profile) {
|
||||
(ThreadParamsMode::Embedded, Some(permission_profile)) => {
|
||||
(None, Some(permission_profile.into()))
|
||||
}
|
||||
(ThreadParamsMode::Embedded, None) => (None, None),
|
||||
(ThreadParamsMode::Remote, _) => (Some(sandbox_policy.into()), None),
|
||||
if matches!(thread_params_mode, ThreadParamsMode::Remote)
|
||||
|| matches!(sandbox_policy, SandboxPolicy::ExternalSandbox { .. })
|
||||
{
|
||||
(Some(sandbox_policy.clone().into()), None)
|
||||
} else {
|
||||
(None, permission_profile.map(Into::into))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1131,7 +1131,14 @@ fn permission_profile_override_from_config(
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(config.permissions.permission_profile().into())
|
||||
if matches!(
|
||||
config.permissions.sandbox_policy.get(),
|
||||
SandboxPolicy::ExternalSandbox { .. }
|
||||
) {
|
||||
None
|
||||
} else {
|
||||
Some(config.permissions.permission_profile().into())
|
||||
}
|
||||
}
|
||||
|
||||
fn thread_start_params_from_config(
|
||||
@@ -1520,6 +1527,52 @@ mod tests {
|
||||
assert_eq!(params.model_provider, Some(config.model_provider_id));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn embedded_turn_start_permission_overrides_send_runtime_profile_only_when_provided() {
|
||||
let cwd = std::path::Path::new("/tmp/project");
|
||||
let sandbox_policy = SandboxPolicy::DangerFullAccess;
|
||||
let permission_profile =
|
||||
PermissionProfile::from_legacy_sandbox_policy(&sandbox_policy, cwd);
|
||||
|
||||
assert_eq!(
|
||||
turn_start_permission_overrides(
|
||||
ThreadParamsMode::Embedded,
|
||||
&sandbox_policy,
|
||||
/*permission_profile*/ None,
|
||||
),
|
||||
(None, None)
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
turn_start_permission_overrides(
|
||||
ThreadParamsMode::Embedded,
|
||||
&sandbox_policy,
|
||||
Some(permission_profile.clone()),
|
||||
),
|
||||
(None, Some(permission_profile.into()))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remote_turn_start_permission_overrides_keep_legacy_sandbox_policy() {
|
||||
let cwd = std::path::Path::new("/tmp/project");
|
||||
let sandbox_policy = SandboxPolicy::DangerFullAccess;
|
||||
let permission_profile =
|
||||
PermissionProfile::from_legacy_sandbox_policy(&sandbox_policy, cwd);
|
||||
|
||||
assert_eq!(
|
||||
turn_start_permission_overrides(
|
||||
ThreadParamsMode::Remote,
|
||||
&sandbox_policy,
|
||||
Some(permission_profile),
|
||||
),
|
||||
(
|
||||
Some(codex_app_server_protocol::SandboxPolicy::DangerFullAccess),
|
||||
None
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_start_params_can_mark_clear_source() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
@@ -1632,7 +1685,7 @@ mod tests {
|
||||
|
||||
let (sandbox, profile) = turn_start_permission_overrides(
|
||||
ThreadParamsMode::Embedded,
|
||||
workspace_write.clone(),
|
||||
&workspace_write,
|
||||
Some(workspace_write_profile.clone()),
|
||||
);
|
||||
assert_eq!(sandbox, None);
|
||||
@@ -1640,7 +1693,7 @@ mod tests {
|
||||
|
||||
let (sandbox, profile) = turn_start_permission_overrides(
|
||||
ThreadParamsMode::Embedded,
|
||||
workspace_write.clone(),
|
||||
&workspace_write,
|
||||
/*permission_profile*/ None,
|
||||
);
|
||||
assert_eq!(sandbox, None);
|
||||
@@ -1648,12 +1701,12 @@ mod tests {
|
||||
|
||||
let (sandbox, profile) = turn_start_permission_overrides(
|
||||
ThreadParamsMode::Remote,
|
||||
workspace_write.clone(),
|
||||
&workspace_write,
|
||||
Some(PermissionProfile::from_legacy_sandbox_policy(
|
||||
&workspace_write,
|
||||
)),
|
||||
);
|
||||
assert_eq!(sandbox, Some(workspace_write.into()));
|
||||
assert_eq!(sandbox, Some(workspace_write.clone().into()));
|
||||
assert_eq!(profile, None);
|
||||
|
||||
let external_sandbox = SandboxPolicy::ExternalSandbox {
|
||||
@@ -1661,16 +1714,13 @@ mod tests {
|
||||
};
|
||||
let (sandbox, profile) = turn_start_permission_overrides(
|
||||
ThreadParamsMode::Embedded,
|
||||
external_sandbox.clone(),
|
||||
&external_sandbox,
|
||||
Some(PermissionProfile::from_legacy_sandbox_policy(
|
||||
&external_sandbox,
|
||||
)),
|
||||
);
|
||||
assert_eq!(sandbox, None);
|
||||
assert_eq!(
|
||||
profile,
|
||||
Some(PermissionProfile::from_legacy_sandbox_policy(&external_sandbox).into())
|
||||
);
|
||||
assert_eq!(sandbox, Some(external_sandbox.into()));
|
||||
assert_eq!(profile, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user