github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 00:18:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Restore saved thread permissions during configuration loading (#53419)

Browse Source 
## Why

Restoring unnamed thread permissions after configuration loading leaves network proxy initialization and permission validation using the configured default. A saved read-only thread should also resume when that default is incompatible with current managed requirements.

## What changed

Apply eligible saved permission profiles during configuration loading, before network preparation and final validation. Keep explicit permission overrides and managed requirements authoritative, preserve managed deny-read restrictions, and reject saved external enforcement or materialized filesystem grants. Include the saved profile in the app-server's prepared resume configuration state.

## Testing

Add regression coverage for resuming saved read-only permissions with an incompatible full-access default and for retaining managed, remembered, and newly approved network domains in the restored profile's proxy allowlist.

GitOrigin-RevId: 73e32b0a31ac678fd3f60a8ccf8fa9df627f16c3
This commit is contained in:
jif
2026-10-12 16:09:41 +00:00
committed by copyberry
parent 16845bb8e8
commit bf4ca52b61
10 changed files with 147 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/request_processors/thread_processor.rs
+4 -2
View File
@@ -30,6 +30,7 @@ use codex_protocol::SanitizedGitUrl;
use codex_protocol::config_types::MultiAgentMode;
use codex_protocol::error::CodexErrorDetails;
use codex_protocol::mcp::ClientMcpExtensions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::MultiAgentVersion;
use codex_protocol::protocol::ThreadHistoryMode;
use codex_thread_store::PersistContext;
@@ -96,6 +97,7 @@ struct ResumeConfigState {
workspace_roots: Option<Vec<AbsolutePathBuf>>,
persisted_metadata: Option<ThreadMetadata>,
persisted_settings: Option<PersistedResumeSettings>,
persisted_permission_profile: Option<PermissionProfile>,
}
struct PreparedResumeConfig {
@@ -4131,7 +4133,7 @@ impl ThreadRequestProcessor {
}
let has_explicit_model_resume_override =
has_model_resume_override(request_overrides.as_ref(), &typesafe_overrides);
let persisted_permission_profile = history_settings
typesafe_overrides.persisted_permission_profile = history_settings
.filter(|settings| {
settings.active_permission_profile.is_none()
&& !has_permission_override(request_overrides.as_ref(), &typesafe_overrides)
@@ -4170,6 +4172,7 @@ impl ThreadRequestProcessor {
workspace_roots: typesafe_overrides.workspace_roots.clone(),
persisted_metadata,
persisted_settings,
persisted_permission_profile: typesafe_overrides.persisted_permission_profile.clone(),
};
let mut config = match prepared_config.take() {
Some(prepared) if prepared.state == config_state => prepared.config,
@@ -4200,7 +4203,6 @@ impl ThreadRequestProcessor {
.thread_manager
.start_thread(StartThreadOptions {
initial_history: thread_history,
persisted_permission_profile,
parent_trace: match target {
ThreadResumeTarget::Client(request_id) => {
self.request_trace_context(request_id).await
codex-rs/app-server/tests/suite/v2/thread_resume.rs
+25 -6
View File
@@ -1817,6 +1817,8 @@ async fn cold_resume_permission_overrides_win_over_persisted_profile() -> Result
// Unelevated Windows sandboxes cannot enforce managed deny-read.
#[cfg_attr(unix, test_case(
SandboxMode::DangerFullAccess,
BUILT_IN_PERMISSION_PROFILE_WORKSPACE,
"[permissions.filesystem]\ndeny_read = [\"./private\"]",
AppSandboxPolicy::WorkspaceWrite {
writable_roots: Vec::new(),
@@ -1826,19 +1828,37 @@ async fn cold_resume_permission_overrides_win_over_persisted_profile() -> Result
};
"managed deny-read"
))]
#[test_case("", AppSandboxPolicy::DangerFullAccess; "restore saved full access")]
#[test_case(
SandboxMode::DangerFullAccess,
BUILT_IN_PERMISSION_PROFILE_WORKSPACE,
"", AppSandboxPolicy::DangerFullAccess;
"restore saved full access"
)]
#[test_case(
SandboxMode::DangerFullAccess,
BUILT_IN_PERMISSION_PROFILE_WORKSPACE,
"allowed_sandbox_modes = [\"read-only\"]",
AppSandboxPolicy::ReadOnly { network_access: false };
"managed modes"
)]
#[test_case(
SandboxMode::DangerFullAccess,
BUILT_IN_PERMISSION_PROFILE_WORKSPACE,
"default_permissions = \":read-only\"\n[allowed_permission_profiles]\n\":read-only\" = true",
AppSandboxPolicy::ReadOnly { network_access: false };
"managed catalog"
)]
#[test_case(
SandboxMode::ReadOnly,
BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS,
"allowed_sandbox_modes = [\"read-only\"]",
AppSandboxPolicy::ReadOnly { network_access: false };
"saved read-only wins before incompatible default validation"
)]
#[tokio::test]
async fn cold_resume_without_active_permission_profile_restores_base_permissions(
saved_sandbox: SandboxMode,
configured_default: &str,
requirements: &str,
expected: AppSandboxPolicy,
) -> Result<()> {
@@ -1857,7 +1877,7 @@ async fn cold_resume_without_active_permission_profile_restores_base_permissions
let mut mcp = builder.build_initialized().await?;
let params = ThreadStartParams {
model: Some("mock-model".to_string()),
sandbox: Some(SandboxMode::DangerFullAccess),
sandbox: Some(saved_sandbox),
approval_policy: Some(AskForApproval::Never),
..Default::default()
};
@@ -1873,7 +1893,7 @@ async fn cold_resume_without_active_permission_profile_restores_base_permissions
mcp.start_turn_and_wait_for_completion(TurnStartParams {
thread_id: thread.id.clone(),
input: vec![UserInput::Text {
text: "persist full access".to_string(),
text: "persist permissions".to_string(),
text_elements: Vec::new(),
}],
..Default::default()
@@ -1884,9 +1904,8 @@ async fn cold_resume_without_active_permission_profile_restores_base_permissions
};
MockResponsesConfig::new(&server.uri())
.with_root_config(&format!(
"default_permissions = \"{BUILT_IN_PERMISSION_PROFILE_WORKSPACE}\""
))
.with_approval_policy("on-request")
.with_root_config(&format!("default_permissions = \"{configured_default}\""))
.write(codex_home.path())?;
std::fs::write(codex_home.path().join("requirements.toml"), requirements)?;
let mut builder = TestAppServer::builder().with_codex_home(codex_home.path());
codex-rs/core/src/codex_delegate.rs
-1
View File
@@ -124,7 +124,6 @@ pub(crate) async fn run_codex_thread_interactive(
history_initialization,
disabled_plugin_ids: (isolation == codex_extension_api::SessionIsolation::Inherit)
.then(|| parent_ctx.disabled_plugin_ids.clone()),
persisted_permission_profile: None,
requested_history_mode: None,
fork_persistence: ForkPersistence::Copied,
session_source,
codex-rs/core/src/config/config_tests.rs
+65
View File
@@ -2615,6 +2615,71 @@ async fn permission_profile_override_populates_runtime_permissions() -> std::io:
Ok(())
}
#[tokio::test]
async fn persisted_permissions_initialize_proxy_with_expandable_managed_allowlist()
-> anyhow::Result<()> {
let codex_home = TempDir::new()?;
std::fs::write(
codex_home.path().join(CONFIG_TOML_FILE),
"default_permissions = \":danger-full-access\"\n",
)?;
let config = ConfigBuilder::without_managed_config_for_tests()
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(codex_home.path().to_path_buf()))
.harness_overrides(ConfigOverrides {
persisted_permission_profile: Some(PermissionProfile::read_only()),
..Default::default()
})
.cloud_config_bundle(
CloudConfigBundleFixture::loader_with_enterprise_requirement(
"[experimental_network]\nenabled = true\n[experimental_network.domains]\n'managed.example' = 'allow'\n",
),
)
.build()
.await?;
assert_eq!(
(
config.permissions.permission_profile(),
config.permissions.active_permission_profile()
),
(&PermissionProfile::read_only(), None),
);
let mut rules = codex_execpolicy::Policy::empty();
rules.add_network_rule(
"remembered.example",
codex_execpolicy::NetworkRuleProtocol::Https,
codex_execpolicy::Decision::Allow,
/*justification*/ None,
)?;
let spec = config
.permissions
.network
.as_ref()
.expect("managed proxy")
.with_exec_policy_network_rules(&rules)?;
let state = spec.build_state_with_audit_metadata(
Default::default(),
codex_utils_path_uri::Platform::native(),
codex_network_proxy::LocalBindingPolicy::DefaultFalse,
)?;
let proxy = codex_network_proxy::NetworkProxy::builder()
.state(Arc::new(state))
.managed_by_codex(/*managed_by_codex*/ false)
.build()
.await?;
proxy.add_allowed_domain("newly-approved.example").await?;
assert_eq!(
proxy.current_cfg().await?.allowed_domains(),
Some(vec![
"managed.example".to_string(),
"remembered.example".to_string(),
"newly-approved.example".to_string(),
]),
);
Ok(())
}
#[tokio::test]
async fn persisted_permission_profile_id_wins_over_configured_default() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
codex-rs/core/src/config/mod.rs
+42 -2
View File
@@ -2682,6 +2682,9 @@ pub struct ConfigOverrides {
/// permission overrides take precedence, and stale profile IDs fall back
/// to the configured default.
pub persisted_permission_profile_id: Option<String>,
/// Unnamed permissions from a thread-owned rollout snapshot. These are
/// fallback state: explicit overrides and current requirements take priority.
pub persisted_permission_profile: Option<PermissionProfile>,
pub model_provider: Option<String>,
pub service_tier: Option<Option<String>>,
pub codex_self_exe: Option<PathBuf>,
@@ -3381,6 +3384,7 @@ impl Config {
permission_profile,
default_permissions: default_permissions_override,
persisted_permission_profile_id,
persisted_permission_profile,
model_provider,
service_tier: service_tier_override,
codex_self_exe,
@@ -3525,6 +3529,24 @@ impl Config {
sandbox_mode,
);
let requirements_toml = config_layer_stack.requirements_toml();
let persisted_permission_profile = persisted_permission_profile.filter(|profile| {
sandbox_mode.is_none()
&& permission_profile.is_none()
&& default_permissions_override.is_none()
&& persisted_permission_profile_id.is_none()
&& requirements_toml.allowed_permission_profiles.is_none()
// Executor-owned enforcement and materialized grants are not portable
// thread defaults. Reject the original mode before preserving denials.
&& !matches!(profile, PermissionProfile::External { .. })
&& !profile.file_system_sandbox_policy().entries.iter().any(|entry| {
entry.access.can_read()
&& matches!(entry.path, FileSystemPath::Path { .. } | FileSystemPath::GlobPattern { .. })
})
&& permission_profile_catalog::permission_profile_satisfies_requirements(
&config_layer_stack,
profile,
)
});
let persisted_permission_profile_id = if sandbox_mode.is_some()
|| permission_profile.is_some()
|| default_permissions_override.is_some()
@@ -3637,8 +3659,8 @@ impl Config {
dedupe_absolute_paths(&mut workspace_roots);
let (
configured_network_proxy_config,
permission_profile,
file_system_sandbox_policy,
mut permission_profile,
mut file_system_sandbox_policy,
mut active_permission_profile,
mut profile_workspace_roots,
) = if let Some(permission_profile) = permission_profile {
@@ -3773,6 +3795,24 @@ impl Config {
Vec::new(),
)
};
if let Some(saved) = persisted_permission_profile {
let (mut saved_file_system, saved_network) = saved.to_runtime_permissions();
saved_file_system.preserve_deny_read_restrictions_from(&file_system_sandbox_policy);
let restored = PermissionProfile::from_runtime_permissions_with_enforcement(
saved.enforcement(),
&saved_file_system,
saved_network,
);
if permission_profile_catalog::permission_profile_satisfies_requirements(
&config_layer_stack,
&restored,
) {
permission_profile = restored;
file_system_sandbox_policy = saved_file_system;
active_permission_profile = None;
profile_workspace_roots.clear();
}
}
let prepared_network = PreparedNetworkConfig::from_inputs(NetworkConfigInputs {
configured_proxy: configured_network_proxy_config,
feature_enabled: enable_network_proxy,
codex-rs/core/src/config/permission_profile_catalog.rs
+9 -1
View File
@@ -99,6 +99,14 @@ pub(super) fn permission_profile_is_allowed(
.allowed_permission_profiles
.as_ref()
.is_none_or(|allowed| is_permission_allowed(allowed, profile_id));
allowed_by_id
&& permission_profile_satisfies_requirements(config_layer_stack, permission_profile)
}
pub(super) fn permission_profile_satisfies_requirements(
config_layer_stack: &ConfigLayerStack,
permission_profile: &PermissionProfile,
) -> bool {
let allowed_by_sandbox_mode = config_layer_stack
.requirements()
.permission_profile
@@ -112,7 +120,7 @@ pub(super) fn permission_profile_is_allowed(
value.deny_read.is_empty()
|| validate_permission_profile_for_deny_read(permission_profile, source).is_ok()
});
allowed_by_id && allowed_by_sandbox_mode && allowed_by_filesystem
allowed_by_sandbox_mode && allowed_by_filesystem
}
/// Validates that a permission candidate can enforce managed filesystem denials.
codex-rs/core/src/session/mod.rs
+1 -33
View File
@@ -137,7 +137,6 @@ use codex_protocol::models::PermissionProfile;
use codex_protocol::models::SandboxEnforcement;
use codex_protocol::openai_models::ModelInfo;
use codex_protocol::openai_models::ModelPreset;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSandboxPolicyContext;
use codex_protocol::permissions::NetworkSandboxPolicy;
@@ -460,7 +459,6 @@ pub(crate) struct SessionSpawnArgs {
pub(crate) conversation_history: InitialHistory,
pub(crate) history_initialization: HistoryInitialization,
pub(crate) disabled_plugin_ids: Option<Vec<String>>,
pub(crate) persisted_permission_profile: Option<PermissionProfile>,
pub(crate) requested_history_mode: Option<ThreadHistoryMode>,
pub(crate) fork_persistence: ForkPersistence,
pub(crate) session_source: SessionSource,
@@ -576,7 +574,6 @@ impl Session {
conversation_history,
history_initialization,
disabled_plugin_ids,
persisted_permission_profile,
requested_history_mode,
fork_persistence,
session_source,
@@ -872,7 +869,7 @@ impl Session {
let service_tier =
get_service_tier(config.service_tier.clone(), &config.features, &model_info);
let storage_originator = AuthStorageOriginator::from_client_name(&originator);
let mut session_configuration = SessionConfiguration {
let session_configuration = SessionConfiguration {
turn_extension_init,
provider: create_model_provider(
config.model_provider.clone(),
@@ -915,35 +912,6 @@ impl Session {
dynamic_tools,
user_shell_override,
};
// Restore the thread's base policy, not an executor's enforcement or
// materialized grants that might outlive the workspace they came from.
if let Some(profile) = persisted_permission_profile
&& !matches!(profile, PermissionProfile::External { .. })
&& !profile.file_system_sandbox_policy().entries.iter().any(|entry| {
entry.access.can_read()
&& matches!(
entry.path,
FileSystemPath::Path { .. } | FileSystemPath::GlobPattern { .. }
)
})
&& config
.config_layer_stack
.requirements_toml()
.allowed_permission_profiles
.is_none()
// Reject a disallowed saved mode before the live-update path can
// narrow it into a different policy (for example, Full + denies).
&& config.permissions.can_set_permission_profile(&profile).is_ok()
&& let Ok(restored) = session_configuration.apply(
&SessionSettingsUpdate {
permission_profile: Some(profile),
..Default::default()
},
&environment_selections,
)
{
session_configuration = restored;
}
session_configuration
.validate(&environment_selections)
.map_err(|err| CodexErr::InvalidRequest(err.to_string()))?;
codex-rs/core/src/session/tests/guardian_tests.rs
-1
View File
@@ -1305,7 +1305,6 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
conversation_history: InitialHistory::New,
history_initialization: HistoryInitialization::New,
disabled_plugin_ids: None,
persisted_permission_profile: None,
requested_history_mode: None,
fork_persistence: ForkPersistence::Copied,
session_source: SessionSource::SubAgent(SubAgentSource::Other(
codex-rs/core/src/thread_manager.rs
-6
View File
@@ -304,9 +304,6 @@ pub struct StartThreadOptions {
pub reserved_thread_id: Option<ThreadId>,
/// Initial thread-owned plugin selection; omission restores persisted settings.
pub disabled_plugin_ids: Option<Vec<String>>,
/// Unnamed permissions recovered from an owned rollout snapshot on resume.
/// The session applies these only if current managed requirements allow them.
pub persisted_permission_profile: Option<codex_protocol::models::PermissionProfile>,
}
impl StartThreadOptions {
@@ -331,7 +328,6 @@ impl StartThreadOptions {
client_mcp_extensions: ClientMcpExtensions::default(),
reserved_thread_id: None,
disabled_plugin_ids: None,
persisted_permission_profile: None,
}
}
}
@@ -2125,7 +2121,6 @@ impl ThreadManagerState {
client_mcp_extensions,
mut reserved_thread_id,
disabled_plugin_ids,
persisted_permission_profile,
} = options;
let inherited_environments = captured_environments.or(inherited_environments);
let session_source = session_source.unwrap_or_else(|| self.session_source.clone());
@@ -2346,7 +2341,6 @@ impl ThreadManagerState {
conversation_history: initial_history,
history_initialization,
disabled_plugin_ids,
persisted_permission_profile,
requested_history_mode: history_mode,
fork_persistence,
// Keep only the manager registration internal. The session and its saved
codex-rs/exec/src/lib.rs
+1
View File
@@ -594,6 +594,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
permission_profile: None,
default_permissions: None,
persisted_permission_profile_id: None,
persisted_permission_profile: None,
cwd: resolved_cwd,
workspace_roots: None,
model_provider: model_provider.clone(),