diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 6bc73d0570..749443f79d 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -207,7 +207,7 @@ pub(crate) struct MessageProcessor { thread_processor: ThreadRequestProcessor, turn_processor: TurnRequestProcessor, windows_sandbox_processor: WindowsSandboxRequestProcessor, - owns_external_auth_refresh_bridge: bool, + external_auth_refresh_bridge: Option>, request_serialization_queues: RequestSerializationQueues, } @@ -321,10 +321,13 @@ impl MessageProcessor { remote_control_handle, plugin_startup_tasks, } = args; - let owns_external_auth_refresh_bridge = - auth_manager.set_external_auth_if_absent(Arc::new(ExternalAuthRefreshBridge { + let external_auth_refresh_bridge: Arc = + Arc::new(ExternalAuthRefreshBridge { outgoing: outgoing.clone(), - })); + }); + let external_auth_refresh_bridge = auth_manager + .replace_non_required_external_auth(Arc::clone(&external_auth_refresh_bridge)) + .then_some(external_auth_refresh_bridge); let thread_state_manager = ThreadStateManager::new(); // The thread store is intentionally process-scoped. Config reloads can // affect per-thread behavior, but they must not move newly started, @@ -567,14 +570,15 @@ impl MessageProcessor { thread_processor, turn_processor, windows_sandbox_processor, - owns_external_auth_refresh_bridge, + external_auth_refresh_bridge, request_serialization_queues: RequestSerializationQueues::default(), } } pub(crate) fn clear_runtime_references(&self) { - if self.owns_external_auth_refresh_bridge { - self.account_processor.clear_external_auth(); + if let Some(external_auth_refresh_bridge) = &self.external_auth_refresh_bridge { + self.account_processor + .clear_external_auth_if(external_auth_refresh_bridge); } self.apps_processor.shutdown(); self.models_refresh_worker.shutdown(); diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index b5295b9b74..0ed4d79093 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -363,6 +363,7 @@ use codex_login::AuthManager; use codex_login::CodexAuth; use codex_login::ServerOptions as LoginServerOptions; use codex_login::ShutdownHandle; +use codex_login::auth::ExternalAuth; use codex_login::auth::login_with_chatgpt_auth_tokens; use codex_login::complete_device_code_login; use codex_login::login_with_api_key; diff --git a/codex-rs/app-server/src/request_processors/account_processor.rs b/codex-rs/app-server/src/request_processors/account_processor.rs index 3041d3ae46..5b264abb2f 100644 --- a/codex-rs/app-server/src/request_processors/account_processor.rs +++ b/codex-rs/app-server/src/request_processors/account_processor.rs @@ -158,11 +158,12 @@ impl AccountRequestProcessor { } } - pub(crate) fn clear_external_auth(&self) { - self.auth_manager.clear_external_auth(); - self.thread_manager - .plugins_manager() - .set_auth_mode(self.auth_manager.get_api_auth_mode()); + pub(crate) fn clear_external_auth_if(&self, expected: &Arc) { + if self.auth_manager.clear_external_auth_if(expected) { + self.thread_manager + .plugins_manager() + .set_auth_mode(self.auth_manager.get_api_auth_mode()); + } } fn current_account_updated_notification(&self) -> AccountUpdatedNotification { diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index a0bbf5ba65..da9a71a2b1 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -809,11 +809,16 @@ impl PluginRequestProcessor { if !config.features.enabled(Feature::Plugins) { return Ok(empty_response()); } - let auth = self - .auth_manager - .auth() - .await - .map_err(|err| internal_error(format!("failed to resolve auth: {err}")))?; + let requires_resolved_auth = config.features.enabled(Feature::RemotePlugin) + || config.features.enabled(Feature::PluginSharing); + let auth = if requires_resolved_auth { + self.auth_manager + .auth() + .await + .map_err(|err| internal_error(format!("failed to resolve auth: {err}")))? + } else { + self.auth_manager.auth_cached() + }; if !self .workspace_codex_plugins_enabled(&config, auth.as_ref()) .await diff --git a/codex-rs/app-server/tests/suite/v2/plugin_list.rs b/codex-rs/app-server/tests/suite/v2/plugin_list.rs index 57f1b8e1c3..0364fd6454 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -77,6 +77,7 @@ fn write_plugins_enabled_config_with_workload_identity( codex_home: &std::path::Path, server: &MockServer, token_path: &std::path::Path, + extra_config: &str, ) -> std::io::Result<()> { let token_path = toml::Value::String(token_path.to_string_lossy().into_owned()); let chatgpt_base_url = toml::Value::String(format!("{}/backend-api", server.uri())); @@ -99,6 +100,8 @@ token_url = {token_url} [workload_identity.credential_source] type = "file" path = {token_path} + +{extra_config} "#, ), ) @@ -183,7 +186,12 @@ async fn plugin_list_local_succeeds_when_workload_identity_becomes_unavailable() let server = MockServer::start().await; let token_path = codex_home.path().join("projected-workload-token"); std::fs::write(&token_path, "external-subject-token\n")?; - write_plugins_enabled_config_with_workload_identity(codex_home.path(), &server, &token_path)?; + write_plugins_enabled_config_with_workload_identity( + codex_home.path(), + &server, + &token_path, + "", + )?; let access_token = workload_identity_access_token(); Mock::given(method("POST")) @@ -238,6 +246,83 @@ async fn plugin_list_local_succeeds_when_workload_identity_becomes_unavailable() Ok(()) } +#[tokio::test] +async fn plugin_installed_local_succeeds_when_workload_identity_becomes_unavailable() -> Result<()> +{ + let codex_home = TempDir::new()?; + let server = MockServer::start().await; + let token_path = codex_home.path().join("projected-workload-token"); + std::fs::write(&token_path, "external-subject-token\n")?; + write_openai_curated_marketplace(codex_home.path(), &["linear"])?; + write_installed_plugin(&codex_home, "openai-curated", "linear")?; + write_plugins_enabled_config_with_workload_identity( + codex_home.path(), + &server, + &token_path, + r#"[plugins."linear@openai-curated"] +enabled = true"#, + )?; + + let access_token = workload_identity_access_token(); + Mock::given(method("POST")) + .and(path("/oauth/token")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "access_token": access_token.clone(), + "issued_token_type": "urn:ietf:params:oauth:token-type:access_token", + "token_type": "Bearer", + "expires_in": 3600, + "user_id": "user_test", + "chatgpt_account_id": "workspace_test", + "chatgpt_plan_type": "enterprise", + }))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/backend-api/wham/config/bundle")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({}))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/backend-api/accounts/workspace_test/settings")) + .and(header("authorization", format!("Bearer {access_token}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "beta_settings": { "enable_plugins": true }, + }))) + .expect(1) + .mount(&server) + .await; + + let mut mcp = + TestAppServer::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + std::fs::remove_file(token_path)?; + + let request_id = mcp + .send_plugin_installed_request(PluginInstalledParams { + cwds: None, + install_suggestion_plugin_names: None, + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: PluginInstalledResponse = to_response(response)?; + + assert!(response.marketplace_load_errors.is_empty()); + assert!(response.marketplaces.iter().any(|marketplace| { + marketplace + .plugins + .iter() + .any(|plugin| plugin.id == "linear@openai-curated" && plugin.installed) + })); + server.verify().await; + Ok(()) +} + #[tokio::test] async fn plugin_installed_includes_installed_plugins_and_explicit_install_suggestions() -> Result<()> { diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index a39561f34e..9f8c1f351c 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -1034,9 +1034,14 @@ fn prune_redundant_unreadable_roots( retained.push((candidate, original)); continue; } - for (ancestor, _) in &retained { + for (ancestor, ancestor_original) in &retained { if candidate == *ancestor { - continue 'candidate; + if original == *ancestor_original { + continue 'candidate; + } + // Distinct symlink and canonical spellings need separate + // mounts even when they resolve to the same inode. + continue; } if !candidate.starts_with(ancestor) { continue; @@ -2687,6 +2692,25 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn symlink_and_canonical_deny_spellings_are_both_retained() { + let temp_dir = TempDir::new().expect("temp dir"); + let target = temp_dir.path().join("target-token"); + let symlink = temp_dir.path().join("projected-token"); + std::fs::write(&target, "secret").expect("write token"); + std::os::unix::fs::symlink(&target, &symlink).expect("create token symlink"); + + let retained = prune_redundant_unreadable_roots( + vec![symlink.clone(), target.clone()], + /*allowed_write_paths*/ &[], + ); + + assert_eq!(retained.len(), 2); + assert!(retained.contains(&symlink)); + assert!(retained.contains(&target)); + } + #[test] fn pruned_nested_deny_is_not_readded_as_a_read_only_mount() { let temp_dir = TempDir::new().expect("temp dir"); diff --git a/codex-rs/login/src/auth/auth_tests.rs b/codex-rs/login/src/auth/auth_tests.rs index f26c2d6cdc..3c4fcc29db 100644 --- a/codex-rs/login/src/auth/auth_tests.rs +++ b/codex-rs/login/src/auth/auth_tests.rs @@ -569,6 +569,7 @@ struct TestExternalChatgptAuth { } struct FailingExternalChatgptAuth(&'static str); +struct OptionalFailingExternalChatgptAuth(&'static str); impl ExternalAuth for FailingExternalChatgptAuth { fn auth_mode(&self) -> AuthMode { @@ -593,6 +594,25 @@ impl ExternalAuth for FailingExternalChatgptAuth { } } +impl ExternalAuth for OptionalFailingExternalChatgptAuth { + fn auth_mode(&self) -> AuthMode { + AuthMode::Chatgpt + } + + fn resolve(&self) -> ExternalAuthFuture<'_, Option> { + let message = self.0; + Box::pin(async move { Err(std::io::Error::other(message)) }) + } + + fn refresh( + &self, + _context: ExternalAuthRefreshContext, + ) -> ExternalAuthFuture<'_, ExternalAuthTokens> { + let message = self.0; + Box::pin(async move { Err(std::io::Error::other(message)) }) + } +} + impl ExternalAuth for TestExternalChatgptAuth { fn auth_mode(&self) -> AuthMode { AuthMode::Chatgpt @@ -782,7 +802,7 @@ async fn auth_result_preserves_external_auth_resolution_errors() { } #[tokio::test] -async fn set_external_auth_if_absent_preserves_existing_provider() { +async fn replace_non_required_external_auth_preserves_required_provider() { let manager = AuthManager::shared( PathBuf::from("non-existent"), /*enable_codex_api_key_env*/ false, @@ -792,22 +812,42 @@ async fn set_external_auth_if_absent_preserves_existing_provider() { ) .await; - assert!( - manager.set_external_auth_if_absent(Arc::new(FailingExternalChatgptAuth( - "original external auth", - ))) - ); - assert!( - !manager.set_external_auth_if_absent(Arc::new(FailingExternalChatgptAuth( - "replacement external auth", - ))) - ); + manager.set_external_auth(Arc::new(FailingExternalChatgptAuth( + "required external auth", + ))); + let optional: Arc = + Arc::new(OptionalFailingExternalChatgptAuth("optional external auth")); + assert!(!manager.replace_non_required_external_auth(Arc::clone(&optional))); + assert!(!manager.clear_external_auth_if(&optional)); let error = manager .auth() .await - .expect_err("the original external auth should remain installed"); - assert_eq!(error.to_string(), "original external auth"); + .expect_err("the required external auth should remain installed"); + assert_eq!(error.to_string(), "required external auth"); +} + +#[tokio::test] +async fn non_required_external_auth_cleanup_is_registration_scoped() { + let manager = AuthManager::shared( + PathBuf::from("non-existent"), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + AuthKeyringBackendKind::default(), + ) + .await; + let first: Arc = + Arc::new(OptionalFailingExternalChatgptAuth("first external auth")); + let second: Arc = + Arc::new(OptionalFailingExternalChatgptAuth("second external auth")); + + assert!(manager.replace_non_required_external_auth(Arc::clone(&first))); + assert!(manager.replace_non_required_external_auth(Arc::clone(&second))); + assert!(!manager.clear_external_auth_if(&first)); + assert!(manager.has_external_auth()); + assert!(manager.clear_external_auth_if(&second)); + assert!(!manager.has_external_auth()); } #[tokio::test] diff --git a/codex-rs/login/src/auth/manager.rs b/codex-rs/login/src/auth/manager.rs index 4da3abd4b0..7067a1f1a6 100644 --- a/codex-rs/login/src/auth/manager.rs +++ b/codex-rs/login/src/auth/manager.rs @@ -1878,20 +1878,33 @@ impl AuthManager { } } - pub fn set_external_auth_if_absent(&self, external_auth: Arc) -> bool { - if let Ok(mut guard) = self.external_auth.write() - && guard.is_none() - { + pub fn replace_non_required_external_auth(&self, external_auth: Arc) -> bool { + if external_auth.requires_successful_resolution() { + return false; + } + if let Ok(mut guard) = self.external_auth.write() { + if guard + .as_ref() + .is_some_and(|current| current.requires_successful_resolution()) + { + return false; + } *guard = Some(external_auth); return true; } false } - pub fn clear_external_auth(&self) { - if let Ok(mut guard) = self.external_auth.write() { + pub fn clear_external_auth_if(&self, expected: &Arc) -> bool { + if let Ok(mut guard) = self.external_auth.write() + && guard + .as_ref() + .is_some_and(|current| Arc::ptr_eq(current, expected)) + { *guard = None; + return true; } + false } pub fn set_forced_chatgpt_workspace_id(&self, workspace_id: Option>) {