mirror of
https://github.com/openai/codex.git
synced 2026-09-06 15:29:32 +00:00
Merge ab151d008d into sapling-pr-archive-bolinfest
This commit is contained in:
@@ -601,7 +601,6 @@ impl App {
|
||||
cwd.clone(),
|
||||
*approval_policy,
|
||||
approvals_reviewer,
|
||||
permission_profile.clone(),
|
||||
active_permission_profile,
|
||||
model.to_string(),
|
||||
*effort,
|
||||
|
||||
@@ -5,7 +5,6 @@
|
||||
|
||||
use crate::bottom_pane::FeedbackAudience;
|
||||
use crate::legacy_core::config::Config;
|
||||
use crate::permission_compat::legacy_compatible_permission_profile;
|
||||
use crate::session_state::MessageHistoryMetadata;
|
||||
use crate::session_state::ThreadSessionState;
|
||||
use crate::status::StatusAccountDisplay;
|
||||
@@ -523,7 +522,6 @@ impl AppServerSession {
|
||||
cwd: PathBuf,
|
||||
approval_policy: AskForApproval,
|
||||
approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer,
|
||||
permission_profile: PermissionProfile,
|
||||
active_permission_profile: Option<ActivePermissionProfile>,
|
||||
model: String,
|
||||
effort: Option<codex_protocol::openai_models::ReasoningEffort>,
|
||||
@@ -534,12 +532,8 @@ impl AppServerSession {
|
||||
output_schema: Option<serde_json::Value>,
|
||||
) -> Result<TurnStartResponse> {
|
||||
let request_id = self.next_request_id();
|
||||
let (sandbox_policy, permissions) = turn_permissions_overrides(
|
||||
&permission_profile,
|
||||
active_permission_profile,
|
||||
cwd.as_path(),
|
||||
self.thread_params_mode(),
|
||||
);
|
||||
let permissions =
|
||||
turn_permissions_selection(active_permission_profile, self.thread_params_mode());
|
||||
self.client
|
||||
.request_typed(ClientRequest::TurnStart {
|
||||
request_id,
|
||||
@@ -552,7 +546,7 @@ impl AppServerSession {
|
||||
workspace_roots: None,
|
||||
approval_policy: Some(approval_policy),
|
||||
approvals_reviewer: Some(approvals_reviewer.into()),
|
||||
sandbox_policy,
|
||||
sandbox_policy: None,
|
||||
permissions,
|
||||
model: Some(model),
|
||||
service_tier,
|
||||
@@ -1114,32 +1108,15 @@ fn permissions_selection_from_active_profile(
|
||||
PermissionProfileSelectionParams::Profile { id: active.id }
|
||||
}
|
||||
|
||||
fn turn_permissions_overrides(
|
||||
permission_profile: &PermissionProfile,
|
||||
fn turn_permissions_selection(
|
||||
active_permission_profile: Option<ActivePermissionProfile>,
|
||||
cwd: &std::path::Path,
|
||||
thread_params_mode: ThreadParamsMode,
|
||||
) -> (
|
||||
Option<codex_app_server_protocol::SandboxPolicy>,
|
||||
Option<PermissionProfileSelectionParams>,
|
||||
) {
|
||||
let permissions = if matches!(thread_params_mode, ThreadParamsMode::Embedded) {
|
||||
active_permission_profile.map(permissions_selection_from_active_profile)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let sandbox_policy = (matches!(thread_params_mode, ThreadParamsMode::Remote)
|
||||
|| permissions.is_none())
|
||||
.then(|| {
|
||||
let legacy_profile = legacy_compatible_permission_profile(permission_profile, cwd);
|
||||
let policy = legacy_profile
|
||||
.to_legacy_sandbox_policy(cwd)
|
||||
.unwrap_or_else(|err| {
|
||||
unreachable!("legacy-compatible permissions must project to legacy policy: {err}")
|
||||
});
|
||||
policy.into()
|
||||
});
|
||||
(sandbox_policy, permissions)
|
||||
) -> Option<PermissionProfileSelectionParams> {
|
||||
if matches!(thread_params_mode, ThreadParamsMode::Remote) {
|
||||
return None;
|
||||
}
|
||||
|
||||
active_permission_profile.map(permissions_selection_from_active_profile)
|
||||
}
|
||||
|
||||
fn permissions_selection_from_config(
|
||||
@@ -1566,59 +1543,33 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn embedded_turn_permissions_use_active_profile_selection() {
|
||||
let cwd = test_path_buf("/workspace/project").abs();
|
||||
let active_permission_profile = ActivePermissionProfile::new(":workspace");
|
||||
let expected_permissions =
|
||||
permissions_selection_from_active_profile(active_permission_profile.clone());
|
||||
|
||||
let (sandbox_policy, permissions) = turn_permissions_overrides(
|
||||
&PermissionProfile::workspace_write(),
|
||||
Some(active_permission_profile),
|
||||
cwd.as_path(),
|
||||
ThreadParamsMode::Embedded,
|
||||
);
|
||||
let permissions =
|
||||
turn_permissions_selection(Some(active_permission_profile), ThreadParamsMode::Embedded);
|
||||
|
||||
assert_eq!(sandbox_policy, None);
|
||||
assert_eq!(permissions, Some(expected_permissions));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn embedded_turn_permissions_fall_back_to_sandbox_without_active_profile() {
|
||||
let cwd = test_path_buf("/workspace/project").abs();
|
||||
|
||||
let (sandbox_policy, permissions) = turn_permissions_overrides(
|
||||
&PermissionProfile::read_only(),
|
||||
fn embedded_turn_permissions_omit_overrides_without_active_profile() {
|
||||
let permissions = turn_permissions_selection(
|
||||
/*active_permission_profile*/ None,
|
||||
cwd.as_path(),
|
||||
ThreadParamsMode::Embedded,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
sandbox_policy,
|
||||
Some(codex_app_server_protocol::SandboxPolicy::ReadOnly {
|
||||
network_access: false
|
||||
})
|
||||
);
|
||||
assert_eq!(permissions, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remote_turn_permissions_use_sandbox_even_with_active_profile() {
|
||||
let cwd = test_path_buf("/workspace/project").abs();
|
||||
|
||||
let (sandbox_policy, permissions) = turn_permissions_overrides(
|
||||
&PermissionProfile::read_only(),
|
||||
fn remote_turn_permissions_omit_overrides_even_with_active_profile() {
|
||||
let permissions = turn_permissions_selection(
|
||||
Some(ActivePermissionProfile::new(":read-only")),
|
||||
cwd.as_path(),
|
||||
ThreadParamsMode::Remote,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
sandbox_policy,
|
||||
Some(codex_app_server_protocol::SandboxPolicy::ReadOnly {
|
||||
network_access: false
|
||||
})
|
||||
);
|
||||
assert_eq!(permissions, None);
|
||||
}
|
||||
|
||||
|
||||
@@ -151,7 +151,6 @@ mod npm_registry;
|
||||
pub(crate) mod onboarding;
|
||||
mod oss_selection;
|
||||
mod pager_overlay;
|
||||
mod permission_compat;
|
||||
pub(crate) mod public_widgets;
|
||||
mod render;
|
||||
mod resize_reflow_cap;
|
||||
|
||||
@@ -1,95 +0,0 @@
|
||||
//! Compatibility projections from the canonical permission profile model into
|
||||
//! legacy shapes still required by older or remote app-server APIs.
|
||||
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::path::Path;
|
||||
|
||||
pub(crate) fn legacy_compatible_permission_profile(
|
||||
permission_profile: &PermissionProfile,
|
||||
cwd: &Path,
|
||||
) -> PermissionProfile {
|
||||
if permission_profile.to_legacy_sandbox_policy(cwd).is_ok() {
|
||||
return permission_profile.clone();
|
||||
}
|
||||
|
||||
let file_system_policy = permission_profile.file_system_sandbox_policy();
|
||||
let network_policy = permission_profile.network_sandbox_policy();
|
||||
let cwd_abs = AbsolutePathBuf::from_absolute_path(cwd).ok();
|
||||
let writable_roots = file_system_policy
|
||||
.get_writable_roots_with_cwd(cwd)
|
||||
.into_iter()
|
||||
.map(|root| root.root)
|
||||
.filter(|root| cwd_abs.as_ref() != Some(root))
|
||||
.collect::<Vec<_>>();
|
||||
let tmpdir_writable = std::env::var_os("TMPDIR")
|
||||
.filter(|tmpdir| !tmpdir.is_empty())
|
||||
.and_then(|tmpdir| {
|
||||
AbsolutePathBuf::from_absolute_path(std::path::PathBuf::from(tmpdir)).ok()
|
||||
})
|
||||
.is_some_and(|tmpdir| file_system_policy.can_write_path_with_cwd(tmpdir.as_path(), cwd));
|
||||
let slash_tmp = Path::new("/tmp");
|
||||
let slash_tmp_writable = slash_tmp.is_absolute()
|
||||
&& slash_tmp.is_dir()
|
||||
&& file_system_policy.can_write_path_with_cwd(slash_tmp, cwd);
|
||||
|
||||
PermissionProfile::workspace_write_with(
|
||||
&writable_roots,
|
||||
network_policy,
|
||||
!tmpdir_writable,
|
||||
!slash_tmp_writable,
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_app_server_protocol::FileSystemAccessMode;
|
||||
use codex_app_server_protocol::FileSystemPath;
|
||||
use codex_app_server_protocol::FileSystemSandboxEntry;
|
||||
use codex_app_server_protocol::FileSystemSpecialPath;
|
||||
use codex_app_server_protocol::PermissionProfile as AppServerPermissionProfile;
|
||||
use codex_app_server_protocol::PermissionProfileFileSystemPermissions;
|
||||
use codex_app_server_protocol::PermissionProfileNetworkPermissions;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn compatibility_profile_preserves_unbridgeable_write_roots() {
|
||||
let cwd = AbsolutePathBuf::try_from("/workspace/project").expect("absolute cwd");
|
||||
let extra_root = AbsolutePathBuf::try_from("/workspace/extra").expect("absolute root");
|
||||
let permission_profile: PermissionProfile = AppServerPermissionProfile::Managed {
|
||||
network: PermissionProfileNetworkPermissions { enabled: false },
|
||||
file_system: PermissionProfileFileSystemPermissions::Restricted {
|
||||
entries: vec![
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Special {
|
||||
value: FileSystemSpecialPath::Root,
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
},
|
||||
FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: extra_root.clone(),
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
},
|
||||
],
|
||||
glob_scan_max_depth: None,
|
||||
},
|
||||
}
|
||||
.into();
|
||||
|
||||
let compatibility_profile =
|
||||
legacy_compatible_permission_profile(&permission_profile, cwd.as_path());
|
||||
let policy = compatibility_profile
|
||||
.to_legacy_sandbox_policy(cwd.as_path())
|
||||
.expect("compatibility profile should project to legacy policy");
|
||||
let roots = policy
|
||||
.get_writable_roots_with_cwd(cwd.as_path())
|
||||
.into_iter()
|
||||
.map(|root| root.root)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert_eq!(roots, vec![extra_root, cwd]);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user