Add a feature-gated account check for CLI thread resume (#58890)

## What changed

Add the disabled-by-default `cli_resume_account_check` feature. When enabled for `codex-tui` and `codex_exec`, reject resuming or attaching to a local thread if its recorded creator user or workspace differs from the current ChatGPT account. Return an error directing the user to sign in with the original account.

Carry creator IDs through stored thread metadata and check them before loading resume history. Preserve existing behavior for untagged threads, other clients, and authentication modes without a comparable ChatGPT account.

## Testing

Add integration coverage for account and workspace mismatches, matching accounts, legacy and paginated history, ID and path lookup, and the feature's default and bypass behavior. Verify that switching back to the original account permits attachment to a loaded thread and that creator metadata remains unchanged.

GitOrigin-RevId: a3ab089254703417dde0890a5f8d6326c4cbb75a
This commit is contained in:
jif
2026-11-02 13:51:49 +00:00
committed by copyberry
parent 020e0f7708
commit 1e8b536dc7
13 changed files with 394 additions and 27 deletions
@@ -6,6 +6,9 @@ mod daemon_snapshot;
mod workspace_switching;
#[path = "thread_resume_account.rs"]
mod thread_resume_account;
use super::persisted_resume_settings::PersistedResumeSettings;
use super::persisted_resume_settings::latest_owned_thread_settings;
use super::persisted_resume_settings::latest_persisted_model_settings;
@@ -4012,37 +4015,37 @@ impl ThreadRequestProcessor {
let prepared_model_context = prepared_config
.as_mut()
.and_then(|prepared| prepared.model_context.take());
let resume_result = if let Some(history) = history {
self.resume_thread_from_history(history.as_slice())
.await
.map(|thread_history| (thread_history, None))
} else if let Some(stored_thread) = stored_thread_from_running_probe {
self.load_resume_initial_history_from_stored_thread(
*stored_thread,
prepared_model_context,
let (thread_history, resume_source_thread) = if let Some(history) = history {
(
self.resume_thread_from_history(history.as_slice()).await?,
None,
)
.await
.map(|(thread_history, stored_thread)| (thread_history, Some(stored_thread)))
} else {
match self
.read_stored_thread_for_resume(
&thread_id,
path.as_ref(),
/*include_history*/ false,
)
.await
{
Ok(stored_thread) => self
.load_resume_initial_history_from_stored_thread(
stored_thread,
prepared_model_context,
let stored_thread = match stored_thread_from_running_probe {
Some(stored_thread) => *stored_thread,
None => {
self.read_stored_thread_for_resume(
&thread_id,
path.as_ref(),
/*include_history*/ false,
)
.await
.map(|(thread_history, stored_thread)| (thread_history, Some(stored_thread))),
Err(error) => Err(error),
}
.await?
}
};
thread_resume_account::check(
&self.config.features,
app_server_client_name.as_deref(),
&self.auth_manager,
&stored_thread,
)?;
let (thread_history, stored_thread) = self
.load_resume_initial_history_from_stored_thread(
stored_thread,
prepared_model_context,
)
.await?;
(thread_history, Some(stored_thread))
};
let (thread_history, resume_source_thread) = resume_result?;
// Path-based resume can use an empty request thread ID. Coordinate once its real
// identity is known; unrelated loaded threads never wait for this cold startup.
let _goal_resume_guard = if let InitialHistory::Resumed(resumed) = &thread_history {
@@ -4684,6 +4687,12 @@ impl ThreadRequestProcessor {
};
if let Some((existing_thread_id, existing_thread, mut source_thread)) = running_thread {
thread_resume_account::check(
&self.config.features,
app_server_client_name.as_deref(),
&self.auth_manager,
&source_thread,
)?;
let paginated_resume =
matches!(source_thread.history_mode, ThreadHistoryMode::Paginated);
let existing_thread_rollout_path = existing_thread.rollout_path();
@@ -502,6 +502,8 @@ mod thread_processor_behavior_tests {
let thread_id =
ThreadId::from_string("00000000-0000-0000-0000-000000000123").expect("valid thread");
let stored_thread = StoredThread {
creator_user_id: None,
creator_account_id: None,
originator: None,
thread_id,
extra_config: None,
@@ -0,0 +1,46 @@
//! Applies the rollout-gated CLI account check before resuming or attaching to a local thread.
//! Creator tags are immutable; untagged threads and other clients retain existing behavior.
use crate::error_code::invalid_request;
use codex_app_server_protocol::JSONRPCErrorError;
use codex_features::Feature;
use codex_features::Features;
use codex_login::AuthManager;
use codex_login::CodexAuth;
use codex_thread_store::StoredThread;
pub(super) fn check(
features: &Features,
client_name: Option<&str>,
auth_manager: &AuthManager,
thread: &StoredThread,
) -> Result<(), JSONRPCErrorError> {
if !features.enabled(Feature::CliResumeAccountCheck)
|| !matches!(client_name, Some("codex-tui" | "codex_exec"))
|| thread.rollout_path.is_none()
|| (thread.creator_user_id.is_none() && thread.creator_account_id.is_none())
{
return Ok(());
}
let Some(auth) = auth_manager
.auth_cached()
.filter(CodexAuth::is_chatgpt_auth)
else {
// API keys and other auth modes do not identify a comparable ChatGPT account.
return Ok(());
};
if thread
.creator_user_id
.as_ref()
.is_some_and(|creator| auth.get_chatgpt_user_id().as_ref() != Some(creator))
|| thread
.creator_account_id
.as_ref()
.is_some_and(|creator| auth.get_account_id().as_ref() != Some(creator))
{
return Err(invalid_request(
"This thread was created with a different ChatGPT account or workspace. Sign in with the original account to resume it.",
));
}
Ok(())
}
@@ -0,0 +1,8 @@
---
source: app-server/tests/suite/v2/thread_resume_account_tests.rs
expression: "serde_json::to_string_pretty(&error.error)?"
---
{
"code": -32600,
"message": "This thread was created with a different ChatGPT account or workspace. Sign in with the original account to resume it."
}
@@ -158,6 +158,9 @@ use super::analytics::wait_for_analytics_payload;
use super::analytics::wait_for_goal_event;
use super::analytics::wait_for_matching_analytics_event;
#[path = "thread_resume_account_tests.rs"]
mod account_tests;
#[cfg(target_os = "linux")]
#[path = "thread_resume_network_tests.rs"]
mod network_tests;
@@ -0,0 +1,253 @@
//! Exercises CLI account checks through thread/resume, including attachment to loaded threads.
use super::*;
use app_test_support::ChatGptIdTokenClaims;
use app_test_support::encode_id_token;
use codex_app_server_protocol::LoginAccountResponse;
use pretty_assertions::assert_eq;
use test_case::test_case;
#[derive(Clone, Copy)]
enum AccountCheck {
Default,
Enabled,
}
#[derive(Clone, Copy)]
enum ResumeOutcome {
Allowed,
Denied,
}
#[test_case("codex-tui", AccountCheck::Enabled, Some(("other-user", "account")), Some(("user", "account")), ThreadHistoryMode::Legacy, ResumeOutcome::Denied; "different user")]
#[test_case("codex_exec", AccountCheck::Enabled, Some(("user", "other-account")), Some(("user", "account")), ThreadHistoryMode::Legacy, ResumeOutcome::Denied; "different workspace exec")]
#[test_case("codex-tui", AccountCheck::Enabled, Some(("user", "other-account")), Some(("user", "account")), ThreadHistoryMode::Paginated, ResumeOutcome::Denied; "paginated different workspace")]
#[test_case("codex-tui", AccountCheck::Enabled, Some(("user", "account")), Some(("user", "account")), ThreadHistoryMode::Legacy, ResumeOutcome::Allowed; "same owner")]
#[test_case("codex-tui", AccountCheck::Enabled, Some(("user", "account")), Some(("user", "account")), ThreadHistoryMode::Paginated, ResumeOutcome::Allowed; "paginated same owner")]
#[test_case("codex-tui", AccountCheck::Enabled, None, Some(("user", "account")), ThreadHistoryMode::Legacy, ResumeOutcome::Allowed; "untagged")]
#[test_case("codex-tui", AccountCheck::Default, Some(("other-user", "other-account")), Some(("user", "account")), ThreadHistoryMode::Legacy, ResumeOutcome::Allowed; "disabled by default")]
#[test_case("third-party-client", AccountCheck::Enabled, Some(("other-user", "other-account")), Some(("user", "account")), ThreadHistoryMode::Legacy, ResumeOutcome::Allowed; "third party")]
#[test_case("codex-tui", AccountCheck::Enabled, Some(("user", "account")), None, ThreadHistoryMode::Legacy, ResumeOutcome::Allowed; "api key")]
#[tokio::test]
async fn thread_resume_account_check(
client_name: &str,
account_check: AccountCheck,
creator: Option<(&str, &str)>,
current_owner: Option<(&str, &str)>,
history_mode: ThreadHistoryMode,
expected: ResumeOutcome,
) -> Result<()> {
let server = create_mock_responses_server_repeating_assistant("Done").await;
let codex_home = TempDir::new()?;
let mut config = mock_responses_config(&server.uri());
if matches!(account_check, AccountCheck::Enabled) {
config = config.enable_feature(Feature::CliResumeAccountCheck);
}
config.write(codex_home.path())?;
if let Some((user, account)) = current_owner {
write_chatgpt_auth(
codex_home.path(),
ChatGptAuthFixture::new("test-token")
.chatgpt_user_id(user)
.chatgpt_account_id(account)
.account_id(account),
AuthCredentialsStoreMode::File,
)?;
}
let create_rollout = match history_mode {
ThreadHistoryMode::Legacy => create_fake_rollout,
ThreadHistoryMode::Paginated => create_fake_paginated_rollout,
};
let thread_id = create_rollout(
codex_home.path(),
"2025-01-05T12-00-00",
"2025-01-05T12:00:00Z",
"Saved user message",
Some("mock_provider"),
/*git_info*/ None,
)?;
let path = rollout_path(codex_home.path(), "2025-01-05T12-00-00", &thread_id);
if let Some((user, account)) = creator {
let contents = std::fs::read_to_string(&path)?;
let (first, rest) = contents.split_once('\n').expect("rollout metadata");
let mut first: serde_json::Value = serde_json::from_str(first)?;
first["payload"]["creator_user_id"] = json!(user);
first["payload"]["creator_account_id"] = json!(account);
if history_mode == ThreadHistoryMode::Paginated && matches!(expected, ResumeOutcome::Denied)
{
// The owner mismatch must win even when loading the inherited history would fail.
first["payload"]["history_base"] = json!({
"thread_id": ThreadId::new(),
"end_ordinal_exclusive": 1,
"end_byte_offset": 1,
});
}
std::fs::write(&path, format!("{first}\n{rest}"))?;
}
let original_meta = serde_json::to_value(read_session_meta_line(&path).await?)?;
let mut mcp = TestAppServer::builder()
.with_codex_home(codex_home.path())
.with_env_overrides(&[(
"OPENAI_API_KEY",
current_owner.is_none().then_some("test-api-key"),
)])
.build()
.await?;
mcp.initialize_with_client_info(ClientInfo {
name: client_name.to_string(),
title: None,
version: "0.0.0".to_string(),
})
.await?;
// Check both direct ID lookup and explicit path lookup. Successful second
// resumes also exercise attachment to the already loaded thread.
for resume_path in [None, Some(path.clone())] {
let resume_id = mcp
.send_thread_resume_request(ThreadResumeParams {
thread_id: thread_id.clone(),
path: resume_path,
exclude_turns: history_mode == ThreadHistoryMode::Paginated,
..Default::default()
})
.await?;
match expected {
ResumeOutcome::Allowed => {
let response: ThreadResumeResponse = mcp.read_response(resume_id).await?;
assert_eq!(response.thread.id, thread_id);
}
ResumeOutcome::Denied => {
let error = mcp
.read_stream_until_error_message(RequestId::Integer(resume_id))
.await?;
insta::assert_snapshot!(
"thread_resume_account_mismatch",
serde_json::to_string_pretty(&error.error)?,
);
let list_id = mcp
.send_thread_loaded_list_request(ThreadLoadedListParams::default())
.await?;
let loaded: ThreadLoadedListResponse = mcp.read_response(list_id).await?;
assert_eq!(
loaded,
ThreadLoadedListResponse {
data: Vec::new(),
next_cursor: None
},
);
}
}
}
assert_eq!(
serde_json::to_value(read_session_meta_line(&path).await?)?,
original_meta,
);
Ok(())
}
#[tokio::test]
async fn thread_resume_account_check_loaded_thread_recovers_after_switching_back() -> Result<()> {
let server = create_mock_responses_server_repeating_assistant("Done").await;
let codex_home = TempDir::new()?;
mock_responses_config(&server.uri())
.enable_feature(Feature::CliResumeAccountCheck)
.write(codex_home.path())?;
write_chatgpt_auth(
codex_home.path(),
ChatGptAuthFixture::new("test-token")
.chatgpt_user_id("user")
.chatgpt_account_id("account")
.account_id("account"),
AuthCredentialsStoreMode::File,
)?;
let mut mcp = TestAppServer::builder()
.with_codex_home(codex_home.path())
.with_env_overrides(&[("OPENAI_API_KEY", None)])
.build()
.await?;
mcp.initialize_with_client_info(ClientInfo {
name: "codex-tui".to_string(),
title: None,
version: "0.0.0".to_string(),
})
.await?;
let start_id = mcp
.send_thread_start_request_with_auto_env(ThreadStartParams::default())
.await?;
let ThreadStartResponse { thread, .. } = mcp.read_response(start_id).await?;
let turn_id = mcp
.send_turn_start_request(TurnStartParams {
thread_id: thread.id.clone(),
input: vec![UserInput::Text {
text: "Save this thread".to_string(),
text_elements: Vec::new(),
}],
..Default::default()
})
.await?;
let _: TurnStartResponse = mcp.read_response(turn_id).await?;
mcp.read_stream_until_notification_message("turn/completed")
.await?;
let read_id = mcp
.send_thread_read_request(ThreadReadParams {
thread_id: thread.id,
include_turns: false,
})
.await?;
let ThreadReadResponse { thread, .. } = mcp.read_response(read_id).await?;
let path = thread.path.as_ref().expect("persisted thread path");
let original_meta = serde_json::to_value(read_session_meta_line(path.as_path()).await?)?;
assert_eq!(
(
original_meta["creator_user_id"].clone(),
original_meta["creator_account_id"].clone()
),
(json!("user"), json!("account")),
);
for (account, expected) in [
("other-account", ResumeOutcome::Denied),
("account", ResumeOutcome::Allowed),
] {
let token = encode_id_token(
&ChatGptIdTokenClaims::new()
.chatgpt_user_id("user")
.chatgpt_account_id(account),
)?;
let login_id = mcp
.send_chatgpt_auth_tokens_login_request(
token,
account.to_string(),
/*chatgpt_plan_type*/ None,
)
.await?;
let _: LoginAccountResponse = mcp.read_response(login_id).await?;
let resume_id = mcp
.send_thread_resume_request(ThreadResumeParams {
thread_id: thread.id.clone(),
exclude_turns: true,
..Default::default()
})
.await?;
match expected {
ResumeOutcome::Denied => {
let error = mcp
.read_stream_until_error_message(RequestId::Integer(resume_id))
.await?;
insta::assert_snapshot!(
"thread_resume_account_mismatch",
serde_json::to_string_pretty(&error.error)?,
);
}
ResumeOutcome::Allowed => {
let response: ThreadResumeResponse = mcp.read_response(resume_id).await?;
assert_eq!(response.thread.id, thread.id);
}
}
}
assert_eq!(
serde_json::to_value(read_session_meta_line(path.as_path()).await?)?,
original_meta,
);
Ok(())
}
+6
View File
@@ -915,6 +915,9 @@
"cli_daybreak": {
"type": "boolean"
},
"cli_resume_account_check": {
"type": "boolean"
},
"code_mode": {
"$ref": "#/definitions/FeatureToml_for_CodeModeConfigToml"
},
@@ -7626,6 +7629,9 @@
"cli_daybreak": {
"type": "boolean"
},
"cli_resume_account_check": {
"type": "boolean"
},
"code_mode": {
"$ref": "#/definitions/FeatureToml_for_CodeModeConfigToml"
},
@@ -29,6 +29,8 @@ use tempfile::TempDir;
fn stored_thread(cwd: &str, title: &str, first_user_message: &str) -> StoredThread {
StoredThread {
creator_user_id: None,
creator_account_id: None,
originator: None,
thread_id: ThreadId::new(),
extra_config: None,
+8
View File
@@ -105,6 +105,8 @@ pub enum Feature {
ApiKeyCyberAccessPrograms,
/// Enable Daybreak controls and automatic access-program selection in CLI clients.
CliDaybreak,
/// Restrict CLI resume of attributed local threads to their creator.
CliResumeAccountCheck,
/// Removed no-op; use `tui.fullscreen_transcript` instead.
TranscriptV2,
// Stable.
@@ -1096,6 +1098,12 @@ pub const FEATURES: &[FeatureSpec] = &[
},
default_enabled: false,
},
FeatureSpec {
id: Feature::CliResumeAccountCheck,
key: "cli_resume_account_check",
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::DaemonAutoStart,
key: "daemon_auto_start",
+8
View File
@@ -1115,6 +1115,14 @@ fn stored_thread_from_state(
});
Ok(StoredThread {
creator_user_id: created
.creator_user_id
.clone()
.or_else(|| metadata.and_then(|metadata| metadata.creator_user_id.clone())),
creator_account_id: created
.creator_account_id
.clone()
.or_else(|| metadata.and_then(|metadata| metadata.creator_account_id.clone())),
originator: (!created.originator.is_empty()).then(|| created.originator.clone()),
thread_id,
extra_config: created.extra_config.clone(),
@@ -140,6 +140,8 @@ pub(super) fn stored_thread_from_rollout_item(
let rollout_path = codex_rollout::plain_rollout_path(item.path.as_path());
Some(StoredThread {
creator_user_id: None,
creator_account_id: None,
thread_id,
extra_config: None,
rollout_path: Some(rollout_path),
@@ -323,6 +323,8 @@ async fn read_thread_from_rollout_path(
})?;
thread.rollout_path = Some(codex_rollout::plain_rollout_path(path.as_path()));
let meta_line = read_required_session_meta_line(path.as_path()).await?;
thread.creator_user_id = meta_line.meta.creator_user_id;
thread.creator_account_id = meta_line.meta.creator_account_id;
thread.forked_from_id = meta_line.meta.forked_from_id;
thread.parent_thread_id = meta_line.meta.parent_thread_id;
thread.history_mode = meta_line.meta.history_mode;
@@ -411,6 +413,14 @@ async fn stored_thread_from_metadata(
.unwrap_or(metadata.history_mode);
let name = thread_name_from_metadata(store, &metadata, history_mode).await;
let mut thread = stored_thread_from_state_metadata(store, metadata, parent_thread_id);
if let Some(meta) = &session_meta {
thread.creator_user_id = thread
.creator_user_id
.or_else(|| meta.creator_user_id.clone());
thread.creator_account_id = thread
.creator_account_id
.or_else(|| meta.creator_account_id.clone());
}
thread.originator = thread.originator.or_else(|| {
session_meta
.map(|meta| meta.originator)
@@ -440,6 +450,8 @@ pub(super) fn stored_thread_from_state_metadata(
let permission_profile =
permission_profile_from_metadata_value(&metadata.sandbox_policy, metadata.cwd.as_path());
StoredThread {
creator_user_id: metadata.creator_user_id,
creator_account_id: metadata.creator_account_id,
thread_id: metadata.id,
extra_config: None,
rollout_path: Some(rollout_path),
@@ -543,6 +555,8 @@ fn stored_thread_from_meta_line(
.unwrap_or(created_at);
let rollout_path = codex_rollout::plain_rollout_path(path.as_path());
StoredThread {
creator_user_id: meta_line.meta.creator_user_id,
creator_account_id: meta_line.meta.creator_account_id,
thread_id: meta_line.meta.id,
extra_config: None,
rollout_path: Some(rollout_path),
+6
View File
@@ -615,6 +615,12 @@ pub struct ThreadOccurrenceSearchPage {
/// Store-owned thread metadata used by list/read/resume responses.
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct StoredThread {
/// Recorded creator user, when available from the backing store.
#[serde(default)]
pub creator_user_id: Option<String>,
/// Recorded creator workspace, when available from the backing store.
#[serde(default)]
pub creator_account_id: Option<String>,
/// Originator recorded at creation, if available from the backing store.
#[serde(default)]
pub originator: Option<String>,