From fc54a8c81a5191959d85656c10a01598220c90dd Mon Sep 17 00:00:00 2001 From: gleaming9 Date: Tue, 22 Sep 2026 15:51:12 +0900 Subject: [PATCH] Keep shared-workspace accounts distinct by user --- .../codex_accounts/account_manager/tests.rs | 50 ++++++++- rust/src/codex_accounts/models.rs | 103 ++++++++++++++++++ 2 files changed, 152 insertions(+), 1 deletion(-) diff --git a/rust/src/codex_accounts/account_manager/tests.rs b/rust/src/codex_accounts/account_manager/tests.rs index bfb8584f6c..aefd6f9718 100644 --- a/rust/src/codex_accounts/account_manager/tests.rs +++ b/rust/src/codex_accounts/account_manager/tests.rs @@ -5,9 +5,13 @@ mod tests { /// Write an auth.json carrying a JWT identity for the given account id. fn write_auth(home_path: &Path, email: &str, account_id: &str) { + write_user_auth(home_path, email, account_id, &format!("auth0|{account_id}")); + } + + fn write_user_auth(home_path: &Path, email: &str, account_id: &str, subject: &str) { let payload = serde_json::json!({ "email": email, - "sub": format!("auth0|{account_id}"), + "sub": subject, "https://api.openai.com/auth": { "chatgpt_plan_type": "team", "chatgpt_account_id": account_id, @@ -88,6 +92,50 @@ mod tests { super::super::file_locations::clear_app_support_directory_override(); } + #[test] + fn shared_team_users_keep_separate_discovery_and_managed_homes() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + super::super::file_locations::with_app_support_directory(root.to_path_buf()); + let first_home = root.join("managed-homes").join("first"); + let second_home = root.join("managed-homes").join("second"); + let ambient_home = root.join("ambient"); + for home in [&first_home, &second_home, &ambient_home] { + std::fs::create_dir_all(home).unwrap(); + } + write_user_auth(&first_home, "a@x.test", "shared-team", "user-a"); + write_user_auth(&second_home, "b@x.test", "shared-team", "user-b"); + write_user_auth(&ambient_home, "a@x.test", "shared-team", "user-a"); + let second_auth = std::fs::read(second_home.join("auth.json")).unwrap(); + let mut first = make_account(first_home.clone(), "a@x.test", "shared-team"); + first.auth_subject = Some("user-a".into()); + let manager = CodexAccountManager::new(); + let discovered = manager + .discover_managed_accounts(std::slice::from_ref(&first)) + .unwrap(); + assert_eq!(discovered.len(), 2); + let second = discovered + .iter() + .find(|a| a.codex_home_path == second_home) + .unwrap(); + assert_ne!(second.id, first.id); + assert!(!first.matches(second)); + + let mut ambient = first.clone(); + ambient.codex_home_path = ambient_home; + manager.remove_managed_files_if_owned(&first).unwrap(); + assert!(!first_home.exists()); + assert!(second_home.exists()); + let materialized = manager.materialize_as_managed(&ambient).unwrap(); + assert_ne!(materialized.codex_home_path, second_home); + assert_eq!( + std::fs::read(second_home.join("auth.json")).unwrap(), + second_auth + ); + assert_eq!(managed_home_count(root), 2); + super::super::file_locations::clear_app_support_directory_override(); + } + /// Write an auth.json whose credentials carry an explicit refresh time. fn write_auth_refreshed_at( home_path: &Path, diff --git a/rust/src/codex_accounts/models.rs b/rust/src/codex_accounts/models.rs index 40c29abb60..53757fb844 100644 --- a/rust/src/codex_accounts/models.rs +++ b/rust/src/codex_accounts/models.rs @@ -61,6 +61,25 @@ fn normalize_identifier(value: Option<&str>) -> Option { .filter(|v| !v.is_empty()) } +/// A shared workspace or home cannot override conflicting user evidence. +fn user_identity_conflicts( + subject: Option<&str>, + other_subject: Option<&str>, + email: Option<&str>, + other_email: Option<&str>, +) -> bool { + if let (Some(a), Some(b)) = ( + normalize_identifier(subject), + normalize_identifier(other_subject), + ) { + return a != b; + } + matches!( + (normalize_identifier(email), normalize_identifier(other_email)), + (Some(a), Some(b)) if a != b + ) +} + /// Where an account's `CODEX_HOME` lives. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -206,6 +225,14 @@ impl CodexAccount { /// Whether two accounts refer to the same identity. pub fn matches(&self, other: &CodexAccount) -> bool { + if user_identity_conflicts( + self.auth_subject.as_deref(), + other.auth_subject.as_deref(), + self.email_hint.as_deref(), + other.email_hint.as_deref(), + ) { + return false; + } if let (Some(a), Some(b)) = ( self.effective_workspace_account_id(), other.effective_workspace_account_id(), @@ -323,6 +350,14 @@ impl RemovedAccountIdentity { } pub fn matches(&self, account: &CodexAccount) -> bool { + if user_identity_conflicts( + self.auth_subject.as_deref(), + account.auth_subject.as_deref(), + self.email_hint.as_deref(), + account.email_hint.as_deref(), + ) { + return false; + } if self.standardized_home_path() == account.standardized_home_path() { return true; } @@ -573,6 +608,74 @@ mod tests { assert!(a.matches(&b)); } + #[test] + fn different_subjects_do_not_match_in_a_shared_workspace_or_home() { + let mut a = account( + "11111111-1111-1111-1111-111111111111", + "/managed/shared", + CodexAccountSource::ManagedByApp, + Some("shared-team"), + ); + a.auth_subject = Some("user-a".into()); + a.email_hint = Some("same@x.test".into()); + let mut b = a.clone(); + b.id = Uuid::new_v4(); + b.auth_subject = Some("user-b".into()); + + assert!(!a.matches(&b)); + assert!(!b.matches(&a)); + assert!(!RemovedAccountIdentity::from_account(&a).matches(&b)); + } + + #[test] + fn different_emails_do_not_match_in_a_shared_workspace_without_subjects() { + let mut a = account( + "11111111-1111-1111-1111-111111111111", + "/managed/a", + CodexAccountSource::ManagedByApp, + Some("shared-team"), + ); + a.email_hint = Some("user-a@x.test".into()); + let mut b = a.clone(); + b.id = Uuid::new_v4(); + b.codex_home_path = PathBuf::from("/managed/b"); + b.email_hint = Some("user-b@x.test".into()); + + assert!(!a.matches(&b)); + assert!(!RemovedAccountIdentity::from_account(&a).matches(&b)); + } + + #[test] + fn matching_subject_remains_authoritative_when_email_changes() { + let mut a = account( + "11111111-1111-1111-1111-111111111111", + "/managed/a", + CodexAccountSource::ManagedByApp, + Some("shared-team"), + ); + a.auth_subject = Some("user-a".into()); + a.email_hint = Some("old@x.test".into()); + let mut b = a.clone(); + b.auth_subject = Some("USER-A".into()); + b.email_hint = Some("new@x.test".into()); + assert!(a.matches(&b)); + } + + #[test] + fn same_user_in_different_workspaces_remains_separate() { + let mut a = account( + "11111111-1111-1111-1111-111111111111", + "/managed/a", + CodexAccountSource::ManagedByApp, + Some("team-a"), + ); + a.auth_subject = Some("same-user".into()); + let mut b = a.clone(); + b.provider_account_id = Some("team-b".into()); + assert!(!a.matches(&b)); + assert!(!b.matches(&a)); + } + #[test] fn disambiguates_different_provider_ids() { let a = account(