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

Restore thread-owned permissions on cold resume (#53150)

Browse Source 
## Why

Cold resume can replace saved unnamed permissions with current configuration defaults or recover an executor's permissions from turn context. A first-turn context can also predate an acknowledged settings update.

## What changed

- Restore eligible unnamed permission profiles from settings snapshots owned by the resumed thread, honoring explicit caller overrides and current managed requirements.
- Use owned snapshots as the authority for named permission profiles and reapply workspace permissions for the resumed workspace roots.
- Persist a thread-owned settings checkpoint after the first turn's context, serialized with settings updates so older checkpoints cannot supersede newer settings.

## Testing

Extend regression coverage for cold resume with changed defaults, managed permission restrictions, workspace-root overrides, foreign snapshots, and settings updates during first-turn processing.

GitOrigin-RevId: e411d9fd3d586a4162498c9c77c4184459071bd1
This commit is contained in:
Eric Traut
2026-10-11 15:48:35 +00:00
committed by copyberry
parent 97c1f62898
commit 7c9bfb910e
8 changed files with 208 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/request_processors/thread_processor.rs
+34 -16
View File
@@ -4039,7 +4039,7 @@ impl ThreadRequestProcessor {
}
// Copied or referenced history can contain another thread's settings. Only snapshots
// explicitly owned by this thread can override its startup cwd and workspace folders.
// explicitly owned by this thread can restore its permissions, cwd, and workspace folders.
let history_settings = if let InitialHistory::Resumed(resumed) = &thread_history {
resumed.history.iter().rev().find_map(|item| match item {
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event))
@@ -4131,9 +4131,31 @@ impl ThreadRequestProcessor {
}
let has_explicit_model_resume_override =
has_model_resume_override(request_overrides.as_ref(), &typesafe_overrides);
let persisted_permission_profile = history_settings
.filter(|settings| {
settings.active_permission_profile.is_none()
&& !has_permission_override(request_overrides.as_ref(), &typesafe_overrides)
&& !request_overrides.as_ref().is_some_and(|overrides| {
overrides.keys().any(|key| {
key == "sandbox_workspace_write"
|| key.starts_with("sandbox_workspace_write.")
})
})
})
.map(|settings| settings.permission_profile.clone());
let mut persisted_settings = match &thread_history {
InitialHistory::Resumed(resumed) => latest_persisted_resume_settings(&resumed.history),
InitialHistory::New | InitialHistory::Cleared | InitialHistory::Forked(_) => None,
};
// A TurnContext can describe an executor's profile, so an owned snapshot
// is authoritative for named permissions too.
if let (Some(settings), Some(snapshot)) = (&mut persisted_settings, history_settings) {
settings.active_permission_profile = snapshot.active_permission_profile.clone();
}
let persisted_metadata = self
.load_and_apply_persisted_resume_metadata(
&thread_history,
persisted_settings.as_ref(),
&mut request_overrides,
&mut typesafe_overrides,
)
@@ -4147,12 +4169,7 @@ impl ThreadRequestProcessor {
history_cwd: history_cwd.clone(),
workspace_roots: typesafe_overrides.workspace_roots.clone(),
persisted_metadata,
persisted_settings: match &thread_history {
InitialHistory::Resumed(resumed) => {
latest_persisted_resume_settings(&resumed.history)
}
InitialHistory::New | InitialHistory::Cleared | InitialHistory::Forked(_) => None,
},
persisted_settings,
};
let mut config = match prepared_config.take() {
Some(prepared) if prepared.state == config_state => prepared.config,
@@ -4181,18 +4198,18 @@ impl ThreadRequestProcessor {
match self
.thread_manager
.resume_thread_with_history(
config,
thread_history,
self.auth_manager.clone(),
match target {
.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
}
ThreadResumeTarget::DaemonRecovery(_) => None,
},
client_mcp_extensions,
)
..StartThreadOptions::new(config)
})
.await
{
Ok(NewThread {
@@ -4441,14 +4458,14 @@ impl ThreadRequestProcessor {
async fn load_and_apply_persisted_resume_metadata(
&self,
thread_history: &InitialHistory,
persisted_settings: Option<&PersistedResumeSettings>,
request_overrides: &mut Option<HashMap<String, serde_json::Value>>,
typesafe_overrides: &mut ConfigOverrides,
) -> Option<ThreadMetadata> {
let InitialHistory::Resumed(resumed_history) = thread_history else {
return None;
};
if let Some(persisted_settings) = latest_persisted_resume_settings(&resumed_history.history)
{
if let Some(persisted_settings) = persisted_settings {
if typesafe_overrides.approval_policy.is_none() {
typesafe_overrides.approval_policy = Some(persisted_settings.approval_policy);
}
@@ -4462,7 +4479,8 @@ impl ThreadRequestProcessor {
if !has_permission_override(request_overrides.as_ref(), typesafe_overrides) {
typesafe_overrides.persisted_permission_profile_id = persisted_settings
.active_permission_profile
.map(|profile| profile.id);
.as_ref()
.map(|profile| profile.id.clone());
}
}
let state_db_ctx = self.state_db.clone()?;
codex-rs/app-server/tests/suite/v2/daemon_update_recovery.rs
+1 -1
View File
@@ -641,7 +641,7 @@ async fn managed_shutdown_records_interrupted_turn(outcome: &str) -> Result<()>
daemon_recovery::write_snapshot(&path, &snapshot)?;
}
let mut successor = spawn_server(home.path(), &socket)?;
if !matches!(outcome, "running" | "plan") {
if !matches!(outcome, "running" | "plan" | "read_only") {
let mut client = connect_default_daemon_client(&socket).await?;
timeout(DEFAULT_READ_TIMEOUT, async {
loop {
codex-rs/app-server/tests/suite/v2/thread_resume.rs
+90 -33
View File
@@ -139,6 +139,7 @@ use std::path::PathBuf;
use std::process::Command;
use std::time::Duration;
use tempfile::TempDir;
use test_case::test_case;
use tokio::sync::oneshot;
use tokio::time::timeout;
use uuid::Uuid;
@@ -805,7 +806,18 @@ async fn thread_resume_restores_startup_and_updated_runtime_workspace_roots() ->
async fn cold_resume_workspace_roots_honor_owned_snapshot_and_caller_overrides() -> Result<()> {
let server = create_mock_responses_server_repeating_assistant("Done").await;
let codex_home = TempDir::new()?;
mock_responses_config(&server.uri()).write(codex_home.path())?;
MockResponsesConfig::new(&server.uri())
.with_sandbox_mode("workspace-write")
.with_extra_config(
r#"[sandbox_workspace_write]
exclude_tmpdir_env_var = true
exclude_slash_tmp = true
[windows]
sandbox = "unelevated"
"#,
)
.write(codex_home.path())?;
let saved_cwd = normalized_existing_path(codex_home.path())?.abs();
let new_workspace = TempDir::new()?;
let new_cwd = normalized_existing_path(new_workspace.path())?.abs();
@@ -829,13 +841,17 @@ async fn cold_resume_workspace_roots_honor_owned_snapshot_and_caller_overrides()
"runtime_workspace_roots": [saved_cwd, new_cwd, extra_root],
"approval_policy": "never",
"approvals_reviewer": "user",
"permission_profile": PermissionProfile::read_only(),
"permission_profile": PermissionProfile::workspace_write()
.materialize_project_roots_with_workspace_roots(
&[saved_cwd.clone(), new_cwd.clone(), extra_root.clone()]
),
"collaboration_mode": { "mode": "default", "settings": { "model": "gpt-5.4" } },
},
}))?;
let mut foreign = owned.clone();
foreign.thread_id = Some(ThreadId::new());
foreign.thread_settings.runtime_workspace_roots = Some(vec![extra_root.clone()]);
foreign.thread_settings.permission_profile = PermissionProfile::Disabled;
for settings in [owned, foreign] {
append_rollout_item_to_path(
&path,
@@ -843,7 +859,6 @@ async fn cold_resume_workspace_roots_honor_owned_snapshot_and_caller_overrides()
)
.await?;
}
// Every iteration is a cold resume; the final one must retain the preceding explicit clear.
for (cwd_override, roots_override, expected_cwd, expected_roots) in [
(
@@ -864,6 +879,7 @@ async fn cold_resume_workspace_roots_honor_owned_snapshot_and_caller_overrides()
let mut mcp = TestAppServer::builder()
.with_codex_home(codex_home.path())
.without_auto_env()
.without_managed_config()
.build_initialized()
.await?;
let resume_id = mcp
@@ -891,14 +907,25 @@ async fn cold_resume_workspace_roots_honor_owned_snapshot_and_caller_overrides()
Some((
event.thread_settings.cwd,
event.thread_settings.runtime_workspace_roots,
event.thread_settings.permission_profile,
))
}
_ => None,
});
let (persisted_cwd, persisted_roots, permission_profile) =
persisted.expect("resumed thread settings");
assert_eq!(
persisted,
Some((expected_cwd.clone(), Some(expected_roots)))
(persisted_cwd, persisted_roots),
(expected_cwd.clone(), Some(expected_roots.clone()))
);
let file_system = permission_profile.file_system_sandbox_policy();
for root in [&saved_cwd, &new_cwd, &extra_root] {
assert_eq!(
file_system.can_write_local_path_with_cwd(root.as_path(), expected_cwd.as_path()),
expected_roots.contains(root),
"resumed write permission at {root:?}"
);
}
timeout(DEFAULT_READ_TIMEOUT, mcp.shutdown_gracefully()).await??;
}
Ok(())
@@ -1788,23 +1815,59 @@ async fn cold_resume_permission_overrides_win_over_persisted_profile() -> Result
Ok(())
}
// Unelevated Windows sandboxes cannot enforce managed deny-read.
#[cfg_attr(unix, test_case(
"[permissions.filesystem]\ndeny_read = [\"./private\"]",
AppSandboxPolicy::WorkspaceWrite {
writable_roots: Vec::new(),
network_access: false,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
};
"managed deny-read"
))]
#[test_case("", AppSandboxPolicy::DangerFullAccess; "restore saved full access")]
#[test_case(
"allowed_sandbox_modes = [\"read-only\"]",
AppSandboxPolicy::ReadOnly { network_access: false };
"managed modes"
)]
#[test_case(
"default_permissions = \":read-only\"\n[allowed_permission_profiles]\n\":read-only\" = true",
AppSandboxPolicy::ReadOnly { network_access: false };
"managed catalog"
)]
#[tokio::test]
async fn cold_resume_without_active_permission_profile_uses_current_config() -> Result<()> {
async fn cold_resume_without_active_permission_profile_restores_base_permissions(
requirements: &str,
expected: AppSandboxPolicy,
) -> Result<()> {
let server = create_mock_responses_server_repeating_assistant("Done").await;
let codex_home = TempDir::new()?;
mock_responses_config(&server.uri()).write(codex_home.path())?;
// Only deny-read names a path on the app-server host.
let local = requirements.contains("deny_read");
let thread_id = {
let mut mcp = TestAppServer::builder()
let mut builder = TestAppServer::builder()
.with_codex_home(codex_home.path())
.without_managed_config()
.build_initialized()
.await?;
let ThreadStartResponse { thread, .. } = mcp
.start_thread(ThreadStartParams {
model: Some("mock-model".to_string()),
..Default::default()
})
.await?;
.without_managed_config();
if local {
builder = builder.without_auto_env();
}
let mut mcp = builder.build_initialized().await?;
let params = ThreadStartParams {
model: Some("mock-model".to_string()),
sandbox: Some(SandboxMode::DangerFullAccess),
approval_policy: Some(AskForApproval::Never),
..Default::default()
};
let request_id = if local {
mcp.send_thread_start_request(params).await?
} else {
mcp.send_thread_start_request_with_auto_env(params).await?
};
let ThreadStartResponse { thread, .. } =
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(request_id)).await??;
timeout(
DEFAULT_READ_TIMEOUT,
mcp.start_turn_and_wait_for_completion(TurnStartParams {
@@ -1813,7 +1876,6 @@ async fn cold_resume_without_active_permission_profile_uses_current_config() ->
text: "persist full access".to_string(),
text_elements: Vec::new(),
}],
sandbox_policy: Some(AppSandboxPolicy::DangerFullAccess),
..Default::default()
}),
)
@@ -1826,29 +1888,24 @@ async fn cold_resume_without_active_permission_profile_uses_current_config() ->
"default_permissions = \"{BUILT_IN_PERMISSION_PROFILE_WORKSPACE}\""
))
.write(codex_home.path())?;
let mut mcp = TestAppServer::builder()
.with_codex_home(codex_home.path())
.without_managed_config()
.build_initialized()
.await?;
std::fs::write(codex_home.path().join("requirements.toml"), requirements)?;
let mut builder = TestAppServer::builder().with_codex_home(codex_home.path());
if local {
builder = builder.without_auto_env();
}
let mut mcp = builder.build_initialized().await?;
let resume_id = mcp
.send_thread_resume_request(ThreadResumeParams {
thread_id,
..Default::default()
})
.await?;
let ThreadResumeResponse {
sandbox,
active_permission_profile,
..
} = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??;
assert!(matches!(sandbox, AppSandboxPolicy::WorkspaceWrite { .. }));
let response: ThreadResumeResponse =
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??;
assert_eq!(
active_permission_profile,
Some(ActivePermissionProfile::new(
BUILT_IN_PERMISSION_PROFILE_WORKSPACE
))
(response.sandbox, response.approval_policy),
(expected, AskForApproval::Never),
"requirements: {requirements}"
);
Ok(())
}
codex-rs/core/src/codex_delegate.rs
+1
View File
@@ -124,6 +124,7 @@ 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/session/mod.rs
+48 -3
View File
@@ -137,6 +137,7 @@ 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;
@@ -459,6 +460,7 @@ 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,
@@ -574,6 +576,7 @@ impl Session {
conversation_history,
history_initialization,
disabled_plugin_ids,
persisted_permission_profile,
requested_history_mode,
fork_persistence,
session_source,
@@ -869,7 +872,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 session_configuration = SessionConfiguration {
let mut session_configuration = SessionConfiguration {
turn_extension_init,
provider: create_model_provider(
config.model_provider.clone(),
@@ -912,6 +915,35 @@ 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()))?;
@@ -4804,8 +4836,21 @@ impl Session {
}
// Persist one `TurnContextItem` per real user turn so resume/lazy replay can recover the
// latest durable baseline even when this turn emitted no model-visible context diffs.
self.persist_rollout_items(&[RolloutItem::TurnContext(turn_context_item.clone())])
.await;
// Checkpoint current thread-owned settings after the first turn's context:
// the step may predate an acknowledged update or use executor-owned permissions.
// Serialize with settings updates so an older checkpoint cannot supersede a newer one.
let _settings_guard = if should_inject_full_context {
Some(thread_settings::acquire_persistence_lock(self).await)
} else {
None
};
let mut rollout_items = vec![RolloutItem::TurnContext(turn_context_item.clone())];
if should_inject_full_context {
rollout_items.push(RolloutItem::EventMsg(
thread_settings::applied_event(self).await,
));
}
self.persist_rollout_items(&rollout_items).await;
// Advance the persisted-settings baseline even when this turn emitted no model-visible
// context items.
codex-rs/core/src/session/tests/guardian_tests.rs
+1
View File
@@ -1305,6 +1305,7 @@ 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,6 +304,9 @@ 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 {
@@ -328,6 +331,7 @@ impl StartThreadOptions {
client_mcp_extensions: ClientMcpExtensions::default(),
reserved_thread_id: None,
disabled_plugin_ids: None,
persisted_permission_profile: None,
}
}
}
@@ -2100,6 +2104,7 @@ 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());
@@ -2320,6 +2325,7 @@ 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/core/tests/suite/settings_commits.rs
+27 -4
View File
@@ -8,6 +8,7 @@ use codex_extension_api::ExtensionData;
use codex_extension_api::ExtensionRegistryBuilder;
use codex_history::RolloutItem;
use codex_history::RolloutLine;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::ThreadHistoryMode;
@@ -62,7 +63,7 @@ fn assert_checkpoints(test: &TestCodex, expected: &[ThreadSettingsSnapshot]) ->
#[test_case(ThreadHistoryMode::Legacy; "legacy")]
#[test_case(ThreadHistoryMode::Paginated; "paginated")]
#[tokio::test]
async fn initial_plugin_ids_use_turn_context_without_extra_settings_checkpoints(
async fn initial_settings_include_plugin_ids_without_duplicate_checkpoints(
history_mode: ThreadHistoryMode,
) -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -95,7 +96,8 @@ async fn initial_plugin_ids_use_turn_context_without_extra_settings_checkpoints(
)
.await;
test.codex.flush_rollout().await?;
assert_checkpoints(&test, &[])?;
let mut expected = vec![test.codex.thread_settings_snapshot().await];
assert_checkpoints(&test, &expected)?;
let rollout_path = test.codex.rollout_path().expect("rollout path");
let (items, _, parse_errors) =
codex_rollout::RolloutRecorder::load_rollout_items(&rollout_path).await?;
@@ -124,8 +126,9 @@ async fn initial_plugin_ids_use_turn_context_without_extra_settings_checkpoints(
},
)
.await?;
let expected = vec![test.codex.thread_settings_snapshot().await];
assert!(expected[0].disabled_plugin_ids.is_empty());
let cleared = test.codex.thread_settings_snapshot().await;
assert!(cleared.disabled_plugin_ids.is_empty());
expected.push(cleared);
test.codex.flush_rollout().await?;
assert_checkpoints(&test, &expected)?;
let response =
@@ -203,6 +206,7 @@ async fn settings_notifications_keep_their_commit_across_postcommit_work(
initial.disabled_plugin_ids = None;
let thread_settings = ThreadSettingsOverrides {
model: Some(COMMITTED_MODEL.to_string()),
approval_policy: Some(AskForApproval::Never),
disabled_plugin_ids: Some(vec!["slack@openai".to_string()]),
..Default::default()
};
@@ -289,6 +293,25 @@ async fn settings_notifications_keep_their_commit_across_postcommit_work(
expected_request_model
);
assert_eq!(test.codex.thread_settings_snapshot().await, restored);
test.codex.flush_rollout().await?;
let rollout_path = test.codex.rollout_path().expect("rollout path");
let (items, _, parse_errors) =
codex_rollout::RolloutRecorder::load_rollout_items(&rollout_path).await?;
assert_eq!(parse_errors, 0);
let latest_settings = items.iter().rev().find_map(|item| match item {
RolloutItem::TurnContext(context) => {
Some((context.model.as_str(), context.approval_policy))
}
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(applied)) => Some((
applied.thread_settings.model.as_str(),
applied.thread_settings.approval_policy,
)),
_ => None,
});
assert_eq!(
latest_settings,
Some((restored.model.as_str(), restored.approval_policy))
);
Ok(())
}