From 46017a628fda7555d8505ae8e0f590ebf53c5560 Mon Sep 17 00:00:00 2001 From: Cooper Gamble Date: Wed, 17 Jun 2026 08:28:44 +0000 Subject: [PATCH] [codex] close workload identity context and auth gaps [ci changed_files] --- .../src/request_processors/apps_processor.rs | 68 +++++++++---------- .../src/request_processors/plugins.rs | 37 +++++++--- .../app-server/tests/suite/v2/plugin_list.rs | 12 +++- codex-rs/chatgpt/src/chatgpt_client.rs | 22 +++--- codex-rs/chatgpt/src/connectors.rs | 48 ++++++------- codex-rs/chatgpt/src/workspace_settings.rs | 1 + codex-rs/core/src/session/tests.rs | 43 ++++++++++++ codex-rs/core/src/session/turn_context.rs | 15 ++-- 8 files changed, 161 insertions(+), 85 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/apps_processor.rs b/codex-rs/app-server/src/request_processors/apps_processor.rs index 3cc3d0c47a..076c536816 100644 --- a/codex-rs/app-server/src/request_processors/apps_processor.rs +++ b/codex-rs/app-server/src/request_processors/apps_processor.rs @@ -10,6 +10,15 @@ pub(crate) struct AppsRequestProcessor { _shutdown_drop_guard: DropGuard, } +#[derive(Clone)] +struct AppsListLoadContext { + config: Config, + auth: Option, + environment_manager: Arc, + mcp_manager: Arc, + plugins_manager: Arc, +} + impl AppsRequestProcessor { pub(crate) fn new( auth_manager: Arc, @@ -94,6 +103,13 @@ impl AppsRequestProcessor { let environment_manager = self.thread_manager.environment_manager(); let mcp_manager = self.thread_manager.mcp_manager(); let plugins_manager = self.thread_manager.plugins_manager(); + let load_context = AppsListLoadContext { + config, + auth, + environment_manager, + mcp_manager, + plugins_manager, + }; let shutdown_token = self.shutdown_token.child_token(); tokio::spawn(async move { tokio::select! { @@ -102,10 +118,7 @@ impl AppsRequestProcessor { outgoing, request, params, - config, - environment_manager, - mcp_manager, - plugins_manager, + load_context, ) => {} } }); @@ -120,25 +133,11 @@ impl AppsRequestProcessor { outgoing: Arc, request_id: ConnectionRequestId, params: AppsListParams, - config: Config, - environment_manager: Arc, - mcp_manager: Arc, - plugins_manager: Arc, + load_context: AppsListLoadContext, ) { let retry_params = params.clone(); - let retry_config = config.clone(); - let retry_environment_manager = Arc::clone(&environment_manager); - let retry_mcp_manager = Arc::clone(&mcp_manager); - let retry_plugins_manager = Arc::clone(&plugins_manager); - let result = Self::apps_list_response( - &outgoing, - params, - config, - environment_manager, - mcp_manager, - plugins_manager, - ) - .await; + let retry_load_context = load_context.clone(); + let result = Self::apps_list_response(&outgoing, params, load_context).await; let should_retry = result .as_ref() .is_ok_and(|(_, codex_apps_ready)| !codex_apps_ready); @@ -149,15 +148,8 @@ impl AppsRequestProcessor { if should_retry && !retry_params.force_refetch { let mut retry_params = retry_params; retry_params.force_refetch = true; - if let Err(err) = Self::apps_list_response( - &outgoing, - retry_params, - retry_config, - retry_environment_manager, - retry_mcp_manager, - retry_plugins_manager, - ) - .await + if let Err(err) = + Self::apps_list_response(&outgoing, retry_params, retry_load_context).await { warn!("failed to refresh app list after codex-apps readiness retry: {err:?}"); } @@ -167,11 +159,15 @@ impl AppsRequestProcessor { async fn apps_list_response( outgoing: &Arc, params: AppsListParams, - config: Config, - environment_manager: Arc, - mcp_manager: Arc, - plugins_manager: Arc, + load_context: AppsListLoadContext, ) -> Result<(AppsListResponse, bool), JSONRPCErrorError> { + let AppsListLoadContext { + config, + auth, + environment_manager, + mcp_manager, + plugins_manager, + } = load_context; let AppsListParams { cursor, limit, @@ -192,7 +188,7 @@ impl AppsRequestProcessor { .effective_apps(); let (mut accessible_connectors, mut all_connectors) = tokio::join!( connectors::list_cached_accessible_connectors_from_mcp_tools(&config), - connectors::list_cached_all_connectors(&config, &plugin_apps) + connectors::list_cached_all_connectors(&config, auth.as_ref(), &plugin_apps) ); let cached_all_connectors = all_connectors.clone(); @@ -213,10 +209,12 @@ impl AppsRequestProcessor { }); let all_config = config.clone(); + let all_auth = auth.clone(); let all_plugin_apps = plugin_apps.clone(); tokio::spawn(async move { let result = connectors::list_all_connectors_with_options( &all_config, + all_auth.as_ref(), force_refetch, &all_plugin_apps, ) diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index e4401a3f1a..a0bbf5ba65 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1082,6 +1082,7 @@ impl PluginRequestProcessor { }; let app_summaries = load_plugin_app_summaries( &config, + auth.as_ref(), &outcome.plugin.apps, &outcome.plugin.app_category_by_id, ) @@ -1166,8 +1167,13 @@ impl PluginRequestProcessor { .as_ref() .map(plugin_app_category_by_id_from_value) .unwrap_or_default(); - let app_summaries = - load_plugin_app_summaries(&config, &plugin_apps, &app_category_by_id).await; + let app_summaries = load_plugin_app_summaries( + &config, + auth.as_ref(), + &plugin_apps, + &app_category_by_id, + ) + .await; remote_plugin_detail_to_info(remote_detail, app_summaries) } }; @@ -1508,7 +1514,7 @@ impl PluginRequestProcessor { let apps_needing_auth = self .plugin_apps_needing_auth_for_install( &config, - auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth), + auth.as_ref(), &result.plugin_id.as_key(), &plugin_apps, ) @@ -1645,9 +1651,10 @@ impl PluginRequestProcessor { .as_ref() .map(plugin_app_category_by_id_from_value) .unwrap_or_default(); - let all_connectors = connectors::list_cached_all_connectors(&config, &[]) - .await - .unwrap_or_default(); + let all_connectors = + connectors::list_cached_all_connectors(&config, auth.as_ref(), &[]) + .await + .unwrap_or_default(); connectors::connectors_for_plugin_apps(all_connectors, &plugin_apps) .into_iter() .map(|connector| { @@ -1671,7 +1678,7 @@ impl PluginRequestProcessor { codex_plugin::app_connector_ids_from_declarations(&plugin_app_declarations); self.plugin_apps_needing_auth_for_install( &config, - is_chatgpt_auth, + auth.as_ref(), &result.plugin_id.as_key(), &plugin_apps, ) @@ -1687,17 +1694,23 @@ impl PluginRequestProcessor { async fn plugin_apps_needing_auth_for_install( &self, config: &Config, - is_chatgpt_auth: bool, + auth: Option<&CodexAuth>, plugin_id: &str, plugin_apps: &[codex_plugin::AppConnectorId], ) -> Vec { + let is_chatgpt_auth = auth.is_some_and(CodexAuth::is_chatgpt_auth); if plugin_apps.is_empty() || !config.features.apps_enabled_for_auth(is_chatgpt_auth) { return Vec::new(); } let environment_manager = self.thread_manager.environment_manager(); let (all_connectors_result, accessible_connectors_result) = tokio::join!( - connectors::list_all_connectors_with_options(config, /*force_refetch*/ false, &[]), + connectors::list_all_connectors_with_options( + config, + auth, + /*force_refetch*/ false, + &[], + ), connectors::list_accessible_connectors_from_mcp_tools_with_mcp_manager( config, /*force_refetch*/ true, @@ -1713,7 +1726,7 @@ impl PluginRequestProcessor { plugin = plugin_id, "failed to load app metadata after plugin install: {err:#}" ); - connectors::list_cached_all_connectors(config, &[]) + connectors::list_cached_all_connectors(config, auth, &[]) .await .unwrap_or_default() } @@ -1974,6 +1987,7 @@ impl PluginRequestProcessor { async fn load_plugin_app_summaries( config: &Config, + auth: Option<&CodexAuth>, plugin_apps: &[codex_plugin::AppConnectorId], app_category_by_id: &HashMap, ) -> Vec { @@ -1983,6 +1997,7 @@ async fn load_plugin_app_summaries( let connectors = match connectors::list_all_connectors_with_options( config, + auth, /*force_refetch*/ false, &[], ) @@ -1991,7 +2006,7 @@ async fn load_plugin_app_summaries( Ok(connectors) => connectors, Err(err) => { warn!("failed to load app metadata for plugin/read: {err:#}"); - connectors::list_cached_all_connectors(config, &[]) + connectors::list_cached_all_connectors(config, auth, &[]) .await .unwrap_or_default() } 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 007a74244d..13db2bb16d 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -185,10 +185,11 @@ async fn plugin_list_local_succeeds_when_workload_identity_becomes_unavailable() std::fs::write(&token_path, "external-subject-token\n")?; write_plugins_enabled_config_with_workload_identity(codex_home.path(), &server, &token_path)?; + 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": workload_identity_access_token(), + "access_token": access_token.clone(), "issued_token_type": "urn:ietf:params:oauth:token-type:access_token", "token_type": "Bearer", "expires_in": 3600, @@ -204,6 +205,15 @@ async fn plugin_list_local_succeeds_when_workload_identity_becomes_unavailable() .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?; diff --git a/codex-rs/chatgpt/src/chatgpt_client.rs b/codex-rs/chatgpt/src/chatgpt_client.rs index e4975d46a2..3707172c1e 100644 --- a/codex-rs/chatgpt/src/chatgpt_client.rs +++ b/codex-rs/chatgpt/src/chatgpt_client.rs @@ -1,5 +1,6 @@ use codex_core::config::Config; use codex_login::AuthManager; +use codex_login::CodexAuth; use codex_login::default_client::create_client; use anyhow::Context; @@ -14,21 +15,22 @@ pub(crate) async fn chatgpt_get_request( config: &Config, path: String, ) -> anyhow::Result { - chatgpt_get_request_with_timeout(config, path, /*timeout*/ None).await -} - -pub(crate) async fn chatgpt_get_request_with_timeout( - config: &Config, - path: String, - timeout: Option, -) -> anyhow::Result { - let chatgpt_base_url = &config.chatgpt_base_url; let auth_manager = AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; let auth = auth_manager .auth() .await? .ok_or_else(|| anyhow::anyhow!("ChatGPT auth not available"))?; + chatgpt_get_request_with_timeout(config, &auth, path, /*timeout*/ None).await +} + +pub(crate) async fn chatgpt_get_request_with_timeout( + config: &Config, + auth: &CodexAuth, + path: String, + timeout: Option, +) -> anyhow::Result { + let chatgpt_base_url = &config.chatgpt_base_url; anyhow::ensure!( auth.uses_codex_backend(), "ChatGPT backend requests require Codex backend auth" @@ -48,7 +50,7 @@ pub(crate) async fn chatgpt_get_request_with_timeout( let mut request = client .get(&url) - .headers(codex_model_provider::auth_provider_from_auth(&auth).to_auth_headers()) + .headers(codex_model_provider::auth_provider_from_auth(auth).to_auth_headers()) .header(OAI_PRODUCT_SKU_HEADER, CODEX_PRODUCT_SKU) .header("Content-Type", "application/json"); diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index cb6c526164..6c4da64058 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -19,29 +19,20 @@ pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_o pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_options_and_status; pub use codex_core::connectors::list_cached_accessible_connectors_from_mcp_tools; pub use codex_core::connectors::with_app_enabled_state; -use codex_login::AuthManager; use codex_login::CodexAuth; use codex_login::default_client::originator; use codex_plugin::AppConnectorId; const DIRECTORY_CONNECTORS_TIMEOUT: Duration = Duration::from_secs(60); -async fn apps_enabled(config: &Config) -> bool { - let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; - let auth = auth_manager.auth().await.ok().flatten(); +fn apps_enabled(config: &Config, auth: Option<&CodexAuth>) -> bool { config .features - .apps_enabled_for_auth(auth.as_ref().is_some_and(CodexAuth::uses_codex_backend)) + .apps_enabled_for_auth(auth.is_some_and(CodexAuth::uses_codex_backend)) } -async fn connector_auth(config: &Config) -> anyhow::Result { - let auth_manager = - AuthManager::shared_from_config(config, /*enable_codex_api_key_env*/ false).await; - let auth = auth_manager - .auth() - .await? - .ok_or_else(|| anyhow::anyhow!("ChatGPT auth not available"))?; +fn connector_auth(auth: Option<&CodexAuth>) -> anyhow::Result<&CodexAuth> { + let auth = auth.ok_or_else(|| anyhow::anyhow!("ChatGPT auth not available"))?; anyhow::ensure!( auth.uses_codex_backend(), "ChatGPT connectors require Codex backend auth" @@ -49,12 +40,15 @@ async fn connector_auth(config: &Config) -> anyhow::Result { Ok(auth) } -pub async fn list_connectors(config: &Config) -> anyhow::Result> { - if !apps_enabled(config).await { +pub async fn list_connectors( + config: &Config, + auth: Option<&CodexAuth>, +) -> anyhow::Result> { + if !apps_enabled(config, auth) { return Ok(Vec::new()); } let (connectors_result, accessible_result) = tokio::join!( - list_all_connectors(config), + list_all_connectors(config, auth), list_accessible_connectors_from_mcp_tools(config), ); let connectors = connectors_result?; @@ -67,34 +61,39 @@ pub async fn list_connectors(config: &Config) -> anyhow::Result> { )) } -pub async fn list_all_connectors(config: &Config) -> anyhow::Result> { - list_all_connectors_with_options(config, /*force_refetch*/ false, &[]).await +pub async fn list_all_connectors( + config: &Config, + auth: Option<&CodexAuth>, +) -> anyhow::Result> { + list_all_connectors_with_options(config, auth, /*force_refetch*/ false, &[]).await } pub async fn list_cached_all_connectors( config: &Config, + auth: Option<&CodexAuth>, plugin_apps: &[AppConnectorId], ) -> Option> { - if !apps_enabled(config).await { + if !apps_enabled(config, auth) { return Some(Vec::new()); } - let auth = connector_auth(config).await.ok()?; - let cache_context = connector_directory_cache_context(config, &auth); + let auth = connector_auth(auth).ok()?; + let cache_context = connector_directory_cache_context(config, auth); let connectors = codex_connectors::cached_directory_connectors(&cache_context)?; Some(merge_and_filter_plugin_connectors(connectors, plugin_apps)) } pub async fn list_all_connectors_with_options( config: &Config, + auth: Option<&CodexAuth>, force_refetch: bool, plugin_apps: &[AppConnectorId], ) -> anyhow::Result> { - if !apps_enabled(config).await { + if !apps_enabled(config, auth) { return Ok(Vec::new()); } - let auth = connector_auth(config).await?; - let cache_context = connector_directory_cache_context(config, &auth); + let auth = connector_auth(auth)?; + let cache_context = connector_directory_cache_context(config, auth); let connectors = codex_connectors::list_all_connectors_with_options( cache_context, auth.is_workspace_account(), @@ -102,6 +101,7 @@ pub async fn list_all_connectors_with_options( |path| async move { chatgpt_get_request_with_timeout::( config, + auth, path, Some(DIRECTORY_CONNECTORS_TIMEOUT), ) diff --git a/codex-rs/chatgpt/src/workspace_settings.rs b/codex-rs/chatgpt/src/workspace_settings.rs index d5875adcf8..c2b6e9c5e8 100644 --- a/codex-rs/chatgpt/src/workspace_settings.rs +++ b/codex-rs/chatgpt/src/workspace_settings.rs @@ -113,6 +113,7 @@ pub async fn codex_plugins_enabled_for_workspace( let encoded_account_id = encode_path_segment(&account_id); let settings: WorkspaceSettingsResponse = chatgpt_get_request_with_timeout( config, + auth, format!("/accounts/{encoded_account_id}/settings"), Some(WORKSPACE_SETTINGS_TIMEOUT), ) diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 0f798bce51..586736a8ad 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -8235,6 +8235,49 @@ async fn turn_context_item_stores_split_file_system_sandbox_policy_when_differen ); } +#[tokio::test] +async fn turn_context_item_redacts_enforcement_only_paths_from_split_policy() { + let (_session, mut turn_context) = make_session_and_context().await; + let credential_path = turn_context.config.codex_home.join("workload-token"); + let mut config = (*turn_context.config).clone(); + config.workload_identity_credential_deny_paths = vec![credential_path.clone()]; + turn_context.config = Arc::new(config); + + let visible_file_system_policy = file_system_policy_with_unreadable_glob(&turn_context); + let mut enforced_file_system_policy = visible_file_system_policy.clone(); + enforced_file_system_policy + .entries + .push(FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: credential_path.clone(), + }, + access: FileSystemAccessMode::Deny, + }); + let visible_permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( + turn_context.permission_profile.enforcement(), + &visible_file_system_policy, + turn_context.network_sandbox_policy(), + ); + turn_context.permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( + turn_context.permission_profile.enforcement(), + &enforced_file_system_policy, + turn_context.network_sandbox_policy(), + ); + + let item = turn_context.to_turn_context_item(); + + assert_eq!( + item.file_system_sandbox_policy, + Some(visible_file_system_policy) + ); + assert_eq!(item.permission_profile, Some(visible_permission_profile)); + assert!( + !serde_json::to_string(&item) + .expect("serialize turn context item") + .contains(credential_path.to_string_lossy().as_ref()) + ); +} + #[tokio::test] async fn record_context_updates_and_set_reference_context_item_injects_full_context_when_baseline_missing() { diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 9dbc552600..4015f0d705 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -374,7 +374,10 @@ impl TurnContext { } } - fn non_legacy_file_system_sandbox_policy(&self) -> Option { + fn non_legacy_file_system_sandbox_policy( + &self, + model_visible_permission_profile: &PermissionProfile, + ) -> Option { // Omit the derived split filesystem policy when it is equivalent to // the legacy sandbox policy. This keeps turn-context payloads stable // while both fields exist; once callers consume only the split policy, @@ -385,13 +388,17 @@ impl TurnContext { #[allow(deprecated)] &self.cwd, ); - let file_system_sandbox_policy = self.file_system_sandbox_policy(); + let file_system_sandbox_policy = + model_visible_permission_profile.file_system_sandbox_policy(); (file_system_sandbox_policy != legacy_file_system_sandbox_policy) .then_some(file_system_sandbox_policy) } pub(crate) fn to_turn_context_item(&self) -> TurnContextItem { let workspace_roots = self.config.effective_workspace_roots(); + let model_visible_permission_profile = self.model_visible_permission_profile(); + let model_visible_file_system_sandbox_policy = + self.non_legacy_file_system_sandbox_policy(&model_visible_permission_profile); #[allow(deprecated)] let cwd = self.cwd.clone(); TurnContextItem { @@ -402,9 +409,9 @@ impl TurnContext { timezone: self.timezone.clone(), approval_policy: self.approval_policy.value(), sandbox_policy: self.sandbox_policy(), - permission_profile: Some(self.model_visible_permission_profile()), + permission_profile: Some(model_visible_permission_profile), network: self.turn_context_network_item(), - file_system_sandbox_policy: self.non_legacy_file_system_sandbox_policy(), + file_system_sandbox_policy: model_visible_file_system_sandbox_policy, model: self.model_info.slug.clone(), comp_hash: self.model_info.comp_hash.clone(), personality: self.personality,