openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-12 21:20:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Preserve legacy sandbox templates in TUI permission conversions (#60936)

Browse Source 
## What changed

Add lossless conversions between legacy sandbox templates and `PermissionProfile` without probing workspace metadata, collapsing fixed roots into current-directory access, or deduplicating writable roots. Reject profiles whose restrictions cannot survive the round trip.

Use these conversions for sandbox inheritance in `create_thread` and for validating read-only temporary structured threads.

## Testing

Extend coverage for duplicate writable roots, existing `.git` metadata, unrepresentable restrictions, and child threads inheriting writable roots, network access, and temporary-directory exclusions.

GitOrigin-RevId: cb37acaf615b09f45680b4b0c4152470fbc04abb
This commit is contained in:
ioanbudea-OAI
2026-11-12 00:13:27 +00:00
committed by copyberry
parent 27294165dc
commit c93732887e
7 changed files with 240 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server-protocol/src/protocol/v2/tests.rs
+31 -2
View File
@@ -2876,8 +2876,11 @@ fn mcp_server_status_serializes_absent_server_info_metadata_as_null() {
#[test]
fn sandbox_policy_round_trips_workspace_write_access() {
let directory = tempfile::tempdir().expect("tempdir");
let root = AbsolutePathBuf::from_absolute_path(directory.path()).expect("absolute root");
std::fs::create_dir(root.join(".git")).expect("git metadata");
let v2_policy = SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
writable_roots: vec![root.clone(), root.clone()],
network_access: true,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
@@ -2887,13 +2890,39 @@ fn sandbox_policy_round_trips_workspace_write_access() {
assert_eq!(
core_policy,
codex_protocol::protocol::SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
writable_roots: vec![root.clone(), root],
network_access: true,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
}
);
let profile = codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy_template(
&v2_policy.to_core(),
);
assert!(
!profile
.file_system_sandbox_policy()
.entries
.iter()
.any(|entry| {
entry.access == codex_protocol::permissions::FileSystemAccessMode::Read
&& matches!(
entry.path,
codex_protocol::permissions::FileSystemPath::Path { .. }
)
})
);
assert_eq!(
SandboxPolicy::from(
profile
.to_legacy_sandbox_policy_template()
.expect("portable legacy template")
),
v2_policy,
);
let back_to_v2 = SandboxPolicy::from(core_policy);
assert_eq!(back_to_v2, v2_policy);
}
codex-rs/protocol/src/models.rs
+40
View File
@@ -21,6 +21,7 @@ use crate::permissions::FileSystemSandboxEntry;
use crate::permissions::FileSystemSandboxKind;
use crate::permissions::FileSystemSandboxPolicy;
use crate::permissions::FileSystemSpecialPath;
use crate::permissions::LegacySandboxPolicyProjection;
use crate::permissions::NetworkSandboxPolicy;
use crate::permissions::RawFileSystemSandboxEntry;
use crate::protocol::SandboxPolicy;
@@ -662,6 +663,28 @@ impl PermissionProfile {
)
}
/// Decodes a portable legacy template without probing workspace metadata.
pub fn from_legacy_sandbox_policy_template(sandbox_policy: &SandboxPolicy) -> Self {
if let SandboxPolicy::WorkspaceWrite {
writable_roots,
exclude_tmpdir_env_var,
exclude_slash_tmp,
..
} = sandbox_policy
{
Self::from_runtime_permissions(
&FileSystemSandboxPolicy::workspace_write_template(
writable_roots,
*exclude_tmpdir_env_var,
*exclude_slash_tmp,
),
NetworkSandboxPolicy::from(sandbox_policy),
)
} else {
Self::from_legacy_sandbox_policy(sandbox_policy)
}
}
pub fn from_legacy_sandbox_policy_for_cwd(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self {
Self::from_runtime_permissions_with_enforcement(
SandboxEnforcement::from_legacy_sandbox_policy(sandbox_policy),
@@ -736,6 +759,23 @@ impl PermissionProfile {
}
}
/// Encodes an unresolved template without collapsing fixed roots into cwd access.
pub fn to_legacy_sandbox_policy_template(&self) -> io::Result<SandboxPolicy> {
let policy = self
.file_system_sandbox_policy()
.to_legacy_sandbox_policy_with_projection(
self.network_sandbox_policy(),
LegacySandboxPolicyProjection::Template,
)?;
if Self::from_legacy_sandbox_policy_template(&policy) != *self {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"permission profile cannot be represented by a legacy sandbox template",
));
}
Ok(policy)
}
pub fn to_runtime_permissions(&self) -> (FileSystemSandboxPolicy, NetworkSandboxPolicy) {
(
self.file_system_sandbox_policy(),
codex-rs/protocol/src/permissions.rs
+59 -16
View File
@@ -40,6 +40,13 @@ pub use local_aliases::LocalFileSystemPolicyMatcher;
pub use windows_glob::WindowsDenyReadGlobScan;
pub use windows_glob::windows_deny_read_glob_scan;
/// Selects native execution projection or an unresolved, lossless transport template.
#[derive(Clone, Copy)]
pub(crate) enum LegacySandboxPolicyProjection<'a> {
Cwd(&'a Path),
Template,
}
const PROTECTED_METADATA_GIT_PATH_NAME: &str = ".git";
const PROTECTED_METADATA_AGENTS_PATH_NAME: &str = ".agents";
const PROTECTED_METADATA_CODEX_PATH_NAME: &str = ".codex";
@@ -805,6 +812,32 @@ impl FileSystemSandboxPolicy {
writable_roots: &[AbsolutePathBuf],
exclude_tmpdir_env_var: bool,
exclude_slash_tmp: bool,
) -> Self {
let mut policy = Self::workspace_write_template(
writable_roots,
exclude_tmpdir_env_var,
exclude_slash_tmp,
);
for writable_root in writable_roots {
for protected_path in default_read_only_subpaths_for_writable_root(
writable_root,
/*protect_missing_dot_codex*/ false,
) {
append_default_read_only_path_if_no_explicit_rule(
&mut policy.entries,
protected_path,
);
}
}
policy
}
/// Builds an unresolved template; admission must bind native workspace metadata.
pub(crate) fn workspace_write_template(
writable_roots: &[AbsolutePathBuf],
exclude_tmpdir_env_var: bool,
exclude_slash_tmp: bool,
) -> Self {
let mut entries = vec![FileSystemSandboxEntry::new(
FileSystemPath::Special {
@@ -846,16 +879,7 @@ impl FileSystemSandboxPolicy {
append_default_read_only_project_root_subpath_if_no_explicit_rule(&mut entries, ".agents");
append_default_read_only_project_root_subpath_if_no_explicit_rule(&mut entries, ".codex");
append_default_read_only_project_root_subpath_if_no_explicit_rule(&mut entries, ".aws");
for writable_root in writable_roots {
for protected_path in default_read_only_subpaths_for_writable_root(
writable_root,
/*protect_missing_dot_codex*/ false,
) {
append_default_read_only_path_if_no_explicit_rule(&mut entries, protected_path);
}
}
FileSystemSandboxPolicy::restricted(entries)
Self::restricted(entries)
}
/// Converts a legacy sandbox policy into an equivalent filesystem policy
@@ -1899,6 +1923,17 @@ impl FileSystemSandboxPolicy {
&self,
network_policy: NetworkSandboxPolicy,
cwd: &Path,
) -> io::Result<SandboxPolicy> {
self.to_legacy_sandbox_policy_with_projection(
network_policy,
LegacySandboxPolicyProjection::Cwd(cwd),
)
}
pub(crate) fn to_legacy_sandbox_policy_with_projection(
&self,
network_policy: NetworkSandboxPolicy,
projection: LegacySandboxPolicyProjection<'_>,
) -> io::Result<SandboxPolicy> {
Ok(match self.kind {
FileSystemSandboxKind::ExternalSandbox => SandboxPolicy::ExternalSandbox {
@@ -1918,7 +1953,12 @@ impl FileSystemSandboxPolicy {
}
}
FileSystemSandboxKind::Restricted => {
let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
let cwd = match projection {
LegacySandboxPolicyProjection::Cwd(cwd) => Some(cwd),
LegacySandboxPolicyProjection::Template => None,
};
let cwd_absolute =
cwd.and_then(|cwd| AbsolutePathBuf::from_absolute_path(cwd).ok());
let has_full_disk_write_access = self.has_full_disk_write_access();
let mut workspace_root_writable = false;
let mut writable_roots = Vec::new();
@@ -1976,15 +2016,18 @@ impl FileSystemSandboxPolicy {
if workspace_root_writable {
SandboxPolicy::WorkspaceWrite {
writable_roots: dedup_absolute_paths(
writable_roots,
/*normalize_effective_paths*/ false,
),
writable_roots: match projection {
LegacySandboxPolicyProjection::Cwd(_) => dedup_absolute_paths(
writable_roots,
/*normalize_effective_paths*/ false,
),
LegacySandboxPolicyProjection::Template => writable_roots,
},
network_access: network_policy.is_enabled(),
exclude_tmpdir_env_var: !tmpdir_writable,
exclude_slash_tmp: !slash_tmp_writable,
}
} else if self.has_legacy_write_intent(cwd) {
} else if cwd.is_some_and(|cwd| self.has_legacy_write_intent(cwd)) {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"permissions profile requests filesystem writes outside the workspace root, which is not supported until the runtime enforces FileSystemSandboxPolicy directly",
codex-rs/tui/src/app_server_session.rs
+6
View File
@@ -4115,6 +4115,12 @@ mod tests {
missing_path_behavior: None,
});
assert!(
permission_profile
.to_legacy_sandbox_policy_template()
.is_err()
);
let error = turn_permissions_overrides(
TurnPermissionsOverride::LegacySandbox(permission_profile),
cwd.as_path(),
codex-rs/tui/src/dynamic_tools.rs
+40 -23
View File
@@ -22,7 +22,6 @@ use codex_app_server_protocol::ModelListParams;
use codex_app_server_protocol::ModelListResponse;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::SandboxMode;
use codex_app_server_protocol::SandboxPolicy;
use codex_app_server_protocol::SortDirection;
use codex_app_server_protocol::Thread;
use codex_app_server_protocol::ThreadArchiveParams;
@@ -60,6 +59,7 @@ use codex_features::Feature;
use codex_features::Features;
use codex_protocol::ThreadId;
use codex_protocol::models::FunctionCallOutputBody;
use codex_protocol::models::PermissionProfile;
use serde::Deserialize;
use serde::de::DeserializeOwned;
use serde_json::Value;
@@ -535,25 +535,38 @@ async fn execute_inner(
thread_start_params.runtime_workspace_roots = Some(source.runtime_workspace_roots);
thread_start_params.approval_policy = Some(source.approval_policy);
thread_start_params.approvals_reviewer = Some(source.approvals_reviewer);
let sandbox_policy = if let Some(profile) = source.active_permission_profile {
thread_start_params.permissions = Some(profile.id);
thread_start_params.sandbox = None;
None
} else {
thread_start_params.permissions = None;
thread_start_params.sandbox = Some(match &source.sandbox {
SandboxPolicy::DangerFullAccess => SandboxMode::DangerFullAccess,
SandboxPolicy::ReadOnly { .. } => SandboxMode::ReadOnly,
SandboxPolicy::WorkspaceWrite { .. } => SandboxMode::WorkspaceWrite,
SandboxPolicy::ExternalSandbox { .. } => {
return Err(
"Cannot inherit an external sandbox without a permission profile"
.to_string(),
);
}
});
Some(source.sandbox)
};
let source_permissions =
PermissionProfile::from_legacy_sandbox_policy_template(&source.sandbox.to_core());
let permission_profile_override =
if let Some(profile) = source.active_permission_profile {
thread_start_params.permissions = Some(profile.id);
thread_start_params.sandbox = None;
None
} else {
thread_start_params.permissions = None;
thread_start_params.sandbox = Some(match &source_permissions {
PermissionProfile::Disabled => SandboxMode::DangerFullAccess,
PermissionProfile::Managed { .. } => {
if source_permissions
.file_system_sandbox_policy()
.entries
.iter()
.any(|entry| entry.access.can_write())
{
SandboxMode::WorkspaceWrite
} else {
SandboxMode::ReadOnly
}
}
PermissionProfile::External { .. } => {
return Err(
"Cannot inherit an external sandbox without a permission profile"
.to_string(),
);
}
});
Some(source_permissions)
};
if let Some(model) = arguments.model {
thread_start_params.model = Some(model);
}
@@ -597,7 +610,7 @@ async fn execute_inner(
"create_thread",
prompt,
/*model*/ None,
sandbox_policy,
permission_profile_override,
cyber_access_program,
)
.await?;
@@ -735,7 +748,7 @@ async fn execute_inner(
"send_message_to_thread",
prompt,
arguments.model,
/*sandbox_policy*/ None,
/*permission_profile_override*/ None,
cyber_access_program,
)
.await?;
@@ -1243,9 +1256,13 @@ async fn start_turn(
tool: &str,
prompt: String,
model: Option<String>,
sandbox_policy: Option<SandboxPolicy>,
permission_profile_override: Option<PermissionProfile>,
cyber_access_program: Option<codex_app_server_protocol::CyberAccessProgram>,
) -> Result<TurnStartResponse, String> {
let sandbox_policy = permission_profile_override
.map(|profile| profile.to_legacy_sandbox_policy_template().map(Into::into))
.transpose()
.map_err(|error| error.to_string())?;
request(handle, |request_id| ClientRequest::TurnStart {
request_id,
params: TurnStartParams {
codex-rs/tui/src/dynamic_tools_tests.rs
+48 -1
View File
@@ -5,6 +5,8 @@ use crate::legacy_core::config::ConfigBuilder;
use app_test_support::create_fake_paginated_rollout;
use app_test_support::create_fake_rollout;
use app_test_support::rollout_path;
use codex_app_server_client::AppServerEvent;
use codex_app_server_protocol::ServerNotification;
use codex_protocol::ThreadId;
use core_test_support::responses;
use pretty_assertions::assert_eq;
@@ -707,7 +709,7 @@ async fn wait_threads_preserves_snapshots_and_rejects_self_wait() -> color_eyre:
#[tokio::test]
async fn task_creation_and_followup_start_background_turns() -> color_eyre::Result<()> {
let (_codex_home, server, source, target) = test_server(json!({})).await?;
let (codex_home, mut server, source, target) = test_server(json!({})).await?;
for (tool, arguments) in [
(
@@ -753,6 +755,36 @@ async fn task_creation_and_followup_start_background_turns() -> color_eyre::Resu
.await;
assert!(!rejected.success);
let inherited_sandbox = codex_app_server_protocol::SandboxPolicy::WorkspaceWrite {
writable_roots: vec![
codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(codex_home.path())?,
],
network_access: true,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
};
request::<codex_app_server_protocol::ThreadSettingsUpdateResponse>(
&server.request_handle(),
|request_id| ClientRequest::ThreadSettingsUpdate {
request_id,
params: codex_app_server_protocol::ThreadSettingsUpdateParams {
thread_id: target.clone(),
sandbox_policy: Some(inherited_sandbox.clone()),
..Default::default()
},
},
)
.await
.map_err(color_eyre::eyre::Error::msg)?;
while let Some(event) = server.next_event().await {
if let AppServerEvent::ServerNotification(notification) = event.event
&& matches!(*notification, ServerNotification::ThreadSettingsUpdated(ref update)
if update.thread_id == target && update.thread_settings.sandbox_policy == inherited_sandbox)
{
break;
}
}
let created = response_json(
call_tool(
&server,
@@ -763,6 +795,21 @@ async fn task_creation_and_followup_start_background_turns() -> color_eyre::Resu
.await,
);
assert!(created["threadId"].is_string());
let child: ThreadResumeResponse = request(&server.request_handle(), |request_id| {
ClientRequest::ThreadResume {
request_id,
params: ThreadResumeParams {
thread_id: created["threadId"]
.as_str()
.expect("created thread id")
.to_string(),
..Default::default()
},
}
})
.await
.map_err(color_eyre::eyre::Error::msg)?;
assert_eq!(child.sandbox, inherited_sandbox);
let continued = response_json(
call_tool(
codex-rs/tui/src/temporary_structured_request.rs
+16 -6
View File
@@ -5,7 +5,6 @@ use codex_app_server_protocol::AskForApproval;
use codex_app_server_protocol::ClientRequest;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::SandboxMode;
use codex_app_server_protocol::SandboxPolicy;
use codex_app_server_protocol::ServerNotification;
use codex_app_server_protocol::ThreadItemPayload;
use codex_app_server_protocol::ThreadSource;
@@ -20,6 +19,7 @@ use codex_app_server_protocol::TurnStartResponse;
use codex_app_server_protocol::TurnStatus;
use codex_app_server_protocol::UserInput;
use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY;
use codex_protocol::models::PermissionProfile;
use codex_protocol::openai_models::ReasoningEffort;
use color_eyre::eyre::eyre;
use serde_json::Value;
@@ -163,11 +163,21 @@ pub(crate) async fn start_temporary_thread(
"temporary structured thread did not preserve permission profile {expected_profile}"
));
}
} else if !matches!(response.sandbox, SandboxPolicy::ReadOnly { .. }) {
unsubscribe_temporary_thread(request_handle, response.thread.id).await;
return Err(eyre!(
"temporary structured thread did not start with read-only permissions"
));
} else {
let profile =
PermissionProfile::from_legacy_sandbox_policy_template(&response.sandbox.to_core());
if !matches!(profile, PermissionProfile::Managed { .. })
|| profile
.file_system_sandbox_policy()
.entries
.iter()
.any(|entry| entry.access.can_write())
{
unsubscribe_temporary_thread(request_handle, response.thread.id).await;
return Err(eyre!(
"temporary structured thread did not start with read-only permissions"
));
}
}
Ok(response)