[codex] close workload identity review gaps [ci changed_files]

codex-rs/Cargo.lock

@@ -4418,6 +4418,7 @@ name = "codex-workload-identity"
 version = "0.0.0"
 dependencies = [
  "anyhow",
+ "libc",
  "pretty_assertions",
  "reqwest 0.12.28",
  "schemars 0.8.22",

codex-rs/core/src/session/mod.rs

@@ -2998,7 +2998,12 @@ impl Session {
         let recommended_plugin_candidates = if crate::tools::spec_plan::tool_suggest_enabled(
             turn_context,
         ) {
-            match self.services.auth_manager.auth().await {
+            let auth = if turn_context.config.model_provider.requires_openai_auth {
+                self.services.auth_manager.auth().await
+            } else {
+                Ok(self.services.auth_manager.auth_cached())
+            };
+            match auth {
                 Ok(auth) => {
                     let plugins_config = turn_context.config.plugins_config_input();
                     self.services

codex-rs/core/src/session/session.rs

@@ -604,7 +604,11 @@ impl Session {
         let mcp_manager_for_mcp = Arc::clone(&mcp_manager);
         let mcp_thread_init_for_startup = &mcp_thread_init;
         let auth_and_mcp_fut = async move {
-            let auth = auth_manager_clone.auth_result().await?;
+            let auth = if config_for_mcp.model_provider.requires_openai_auth {
+                auth_manager_clone.auth_result().await?
+            } else {
+                auth_manager_clone.auth_cached()
+            };
             let mcp_config = mcp_manager_for_mcp
                 .runtime_config_for_thread(&config_for_mcp, mcp_thread_init_for_startup)
                 .await;

codex-rs/core/src/session/tests.rs

@@ -29,6 +29,9 @@ use core_test_support::test_codex::local_selections;
 
 use codex_features::Feature;
 use codex_login::CodexAuth;
+use codex_login::ExternalAuth;
+use codex_login::ExternalAuthFuture;
+use codex_login::ExternalAuthRefreshContext;
 use codex_model_provider_info::ModelProviderInfo;
 use codex_models_manager::bundled_models_response;
 use codex_models_manager::model_info;
@@ -81,6 +84,7 @@ use crate::tools::registry::ToolExecutor;
 use crate::tools::router::ToolCallSource;
 use crate::turn_diff_tracker::TurnDiffTracker;
 use codex_app_server_protocol::AppInfo;
+use codex_app_server_protocol::AuthMode;
 use codex_app_server_protocol::McpElicitationSchema;
 use codex_config::config_toml::ConfigToml;
 use codex_config::config_toml::ProjectConfig;
@@ -4767,18 +4771,33 @@ async fn absolute_cwd_update_with_turn_environment_is_allowed() {
     assert_eq!(turn_context.environments.turn_environments.len(), 1);
 }
 
-#[tokio::test]
-async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
-    let codex_home = tempfile::tempdir().expect("create temp dir");
-    let mut config = build_test_config(codex_home.path()).await;
-    config
-        .features
-        .enable(Feature::ShellZshFork)
-        .expect("test config should allow shell_zsh_fork");
-    config.zsh_path = None;
-    let config = Arc::new(config);
+struct FailingRequiredExternalAuth;
 
-    let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
+impl ExternalAuth for FailingRequiredExternalAuth {
+    fn auth_mode(&self) -> AuthMode {
+        AuthMode::Chatgpt
+    }
+
+    fn requires_successful_resolution(&self) -> bool {
+        true
+    }
+
+    fn resolve(&self) -> ExternalAuthFuture<'_, Option<codex_login::ExternalAuthTokens>> {
+        Box::pin(async { Err(std::io::Error::other("required external auth failed")) })
+    }
+
+    fn refresh(
+        &self,
+        _context: ExternalAuthRefreshContext,
+    ) -> ExternalAuthFuture<'_, codex_login::ExternalAuthTokens> {
+        Box::pin(async { Err(std::io::Error::other("required external auth failed")) })
+    }
+}
+
+async fn start_test_session(
+    config: Arc<Config>,
+    auth_manager: Arc<AuthManager>,
+) -> anyhow::Result<Arc<Session>> {
     let models_manager = models_manager_with_provider(
         config.codex_home.to_path_buf(),
         auth_manager.clone(),
@@ -4787,17 +4806,16 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
     let model = get_model_offline_for_tests(config.model.as_deref());
     let model_info =
         construct_model_info_offline_for_tests(model.as_str(), &config.to_models_manager_config());
-    let collaboration_mode = CollaborationMode {
-        mode: ModeKind::Default,
-        settings: Settings {
-            model,
-            reasoning_effort: config.model_reasoning_effort.clone(),
-            developer_instructions: None,
-        },
-    };
     let session_configuration = SessionConfiguration {
         provider: config.model_provider.clone(),
-        collaboration_mode,
+        collaboration_mode: CollaborationMode {
+            mode: ModeKind::Default,
+            settings: Settings {
+                model,
+                reasoning_effort: config.model_reasoning_effort.clone(),
+                developer_instructions: None,
+            },
+        },
         model_reasoning_summary: config.model_reasoning_summary,
         developer_instructions: config.developer_instructions.clone(),
         loaded_agents_md: None,
@@ -4827,7 +4845,6 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
         dynamic_tools: Vec::new(),
         user_shell_override: None,
     };
-
     let (tx_event, _rx_event) = async_channel::unbounded();
     let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit);
     let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf()));
@@ -4837,7 +4854,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
         /*bundled_skills_enabled*/ true,
     ));
     let environment_manager = Arc::new(EnvironmentManager::default_for_tests());
-    let result = Session::new(
+    Session::new(
         session_configuration,
         Arc::clone(&config),
         /*user_instructions*/ None,
@@ -4866,7 +4883,22 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
         /*attestation_provider*/ None,
         Some(config.multi_agent_version_from_features()),
     )
-    .await;
+    .await
+}
+
+#[tokio::test]
+async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
+    let codex_home = tempfile::tempdir().expect("create temp dir");
+    let mut config = build_test_config(codex_home.path()).await;
+    config
+        .features
+        .enable(Feature::ShellZshFork)
+        .expect("test config should allow shell_zsh_fork");
+    config.zsh_path = None;
+    let config = Arc::new(config);
+
+    let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key"));
+    let result = start_test_session(config, auth_manager).await;
 
     let err = match result {
         Ok(_) => panic!("expected startup to fail"),
@@ -4876,6 +4908,39 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
     assert!(msg.contains("zsh fork feature enabled, but no packaged zsh fork is available"));
 }
 
+#[tokio::test]
+async fn session_new_skips_required_wif_for_unauthenticated_provider() {
+    let codex_home = tempfile::tempdir().expect("create temp dir");
+    let mut config = build_test_config(codex_home.path()).await;
+    config.model_provider.requires_openai_auth = false;
+    let auth_manager =
+        AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await;
+    assert_eq!(auth_manager.auth_cached(), None);
+    auth_manager.set_external_auth(Arc::new(FailingRequiredExternalAuth));
+
+    let _session = start_test_session(Arc::new(config), auth_manager)
+        .await
+        .expect("unauthenticated provider should not resolve WIF during session startup");
+}
+
+#[tokio::test]
+async fn session_new_propagates_required_wif_failure_for_authenticated_provider() {
+    let codex_home = tempfile::tempdir().expect("create temp dir");
+    let config = build_test_config(codex_home.path()).await;
+    assert!(config.model_provider.requires_openai_auth);
+    let auth_manager =
+        AuthManager::shared_from_config(&config, /*enable_codex_api_key_env*/ false).await;
+    assert_eq!(auth_manager.auth_cached(), None);
+    auth_manager.set_external_auth(Arc::new(FailingRequiredExternalAuth));
+
+    let error = match start_test_session(Arc::new(config), auth_manager).await {
+        Ok(_) => panic!("authenticated provider should require successful WIF resolution"),
+        Err(error) => error,
+    };
+
+    assert!(error.to_string().contains("required external auth failed"));
+}
+
 // todo: use online model info
 pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
     let (tx_event, _rx_event) = async_channel::unbounded();

codex-rs/core/src/session/turn.rs

@@ -1192,7 +1192,11 @@ pub(crate) async fn built_tools(
     };
     let tool_suggest_is_enabled = tool_suggest_enabled(turn_context);
     let auth = if tool_suggest_is_enabled {
-        sess.services.auth_manager.auth().await?
+        if turn_context.config.model_provider.requires_openai_auth {
+            sess.services.auth_manager.auth().await?
+        } else {
+            sess.services.auth_manager.auth_cached()
+        }
     } else {
         None
     };

codex-rs/model-provider/src/models_endpoint.rs

@@ -51,7 +51,13 @@ impl OpenAiModelsEndpoint {
 
     async fn auth(&self) -> CoreResult<Option<CodexAuth>> {
         match self.auth_manager.as_ref() {
-            Some(auth_manager) => Ok(auth_manager.auth().await?),
+            Some(auth_manager)
+                if self.provider_info.requires_openai_auth
+                    || self.provider_info.has_command_auth() =>
+            {
+                Ok(auth_manager.auth().await?)
+            }
+            Some(auth_manager) => Ok(auth_manager.auth_cached()),
             None => Ok(None),
         }
     }

codex-rs/model-provider/src/provider.rs

@@ -225,14 +225,19 @@ impl ModelProvider for ConfiguredModelProvider {
     fn supports_attestation(&self) -> bool {
         self.auth_manager
             .as_ref()
-            .and_then(|auth_manager| auth_manager.auth_cached())
-            .is_some_and(|auth| auth.is_chatgpt_auth())
+            .and_then(|auth_manager| auth_manager.get_api_auth_mode())
+            .is_some_and(codex_models_manager::AuthMode::has_chatgpt_account)
     }
 
     fn auth(&self) -> ModelProviderFuture<'_, codex_protocol::error::Result<Option<CodexAuth>>> {
         Box::pin(async move {
             match self.auth_manager.as_ref() {
-                Some(auth_manager) => Ok(auth_manager.auth().await?),
+                Some(auth_manager)
+                    if self.info.requires_openai_auth || self.info.has_command_auth() =>
+                {
+                    Ok(auth_manager.auth().await?)
+                }
+                Some(auth_manager) => Ok(auth_manager.auth_cached()),
                 None => Ok(None),
             }
         })
@@ -371,6 +376,16 @@ mod tests {
         }
     }
 
+    fn required_external_auth_manager_without_cached_auth() -> Arc<AuthManager> {
+        let auth_manager = AuthManager::external_bearer_only(
+            provider_info_with_command_auth()
+                .auth
+                .expect("command auth should be configured"),
+        );
+        auth_manager.set_external_auth(Arc::new(FailingRequiredExternalAuth));
+        auth_manager
+    }
+
     fn test_codex_home() -> std::path::PathBuf {
         std::env::temp_dir().join(format!("codex-model-provider-test-{}", std::process::id()))
     }
@@ -411,6 +426,36 @@ mod tests {
         assert!(provider.api_auth().await.is_err());
     }
 
+    #[tokio::test]
+    async fn unauthenticated_provider_does_not_resolve_required_openai_auth() {
+        let auth_manager = required_external_auth_manager_without_cached_auth();
+        let provider = create_model_provider(
+            provider_for("https://example.test/v1".to_string()),
+            Some(auth_manager),
+        );
+
+        assert_eq!(provider.auth().await.unwrap(), None);
+        assert!(
+            provider
+                .api_auth()
+                .await
+                .unwrap()
+                .to_auth_headers()
+                .is_empty()
+        );
+    }
+
+    #[test]
+    fn required_external_chatgpt_auth_supports_attestation_before_resolution() {
+        let auth_manager = required_external_auth_manager_without_cached_auth();
+        let provider = create_model_provider(
+            ModelProviderInfo::create_openai_provider(/*base_url*/ None),
+            Some(auth_manager),
+        );
+
+        assert!(provider.supports_attestation());
+    }
+
     fn remote_model(slug: &str) -> ModelInfo {
         serde_json::from_value(json!({
             "slug": slug,

codex-rs/workload-identity-spiffe/src/lib.rs

@@ -83,7 +83,7 @@ mod tests {
     async fn rejects_tcp_endpoint() {
         let source = SpiffeSubjectTokenProvider::new(
             Some("tcp://127.0.0.1:8081".to_string()),
-            None,
+            /*spiffe_id*/ None,
             "openai-audience".to_string(),
         );
 

codex-rs/workload-identity/Cargo.toml

@@ -16,11 +16,14 @@ thiserror = { workspace = true }
 tokio = { workspace = true, features = ["fs", "io-util", "sync"] }
 url = { workspace = true }
 
+[target.'cfg(unix)'.dependencies]
+libc = { workspace = true }
+
 [dev-dependencies]
 anyhow = { workspace = true }
 pretty_assertions = { workspace = true }
 tempfile = { workspace = true }
-tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
+tokio = { workspace = true, features = ["macros", "rt-multi-thread", "time"] }
 wiremock = { workspace = true }
 
 [lib]

codex-rs/workload-identity/src/source.rs

@@ -152,13 +152,28 @@ impl FileSubjectTokenSource {
 
 impl SubjectTokenProvider for FileSubjectTokenSource {
     async fn subject_token(&self) -> Result<SubjectToken, SubjectTokenError> {
-        let file = tokio::fs::File::open(&self.path).await.map_err(|error| {
-            SubjectTokenError::ReadFile {
+        let metadata =
+            tokio::fs::metadata(&self.path)
+                .await
+                .map_err(|error| SubjectTokenError::ReadFile {
+                    provider: self.source,
+                    path: self.path.clone(),
+                    kind: error.kind(),
+                })?;
+        validate_file_metadata(self.source, &self.path, &metadata)?;
+
+        let mut options = tokio::fs::OpenOptions::new();
+        options.read(true);
+        #[cfg(unix)]
+        options.custom_flags(libc::O_NONBLOCK);
+        let file = options
+            .open(&self.path)
+            .await
+            .map_err(|error| SubjectTokenError::ReadFile {
                 provider: self.source,
                 path: self.path.clone(),
                 kind: error.kind(),
-            }
-        })?;
+            })?;
         let metadata = file
             .metadata()
             .await
@@ -167,17 +182,7 @@ impl SubjectTokenProvider for FileSubjectTokenSource {
                 path: self.path.clone(),
                 kind: error.kind(),
             })?;
-        if !metadata.is_file() {
-            return Err(SubjectTokenError::NotAFile {
-                provider: self.source,
-                path: self.path.clone(),
-            });
-        }
-        if metadata.len() > MAX_SUBJECT_TOKEN_BYTES as u64 {
-            return Err(SubjectTokenError::TooLarge {
-                provider: self.source,
-            });
-        }
+        validate_file_metadata(self.source, &self.path, &metadata)?;
         let mut bytes = Vec::new();
         file.take(MAX_SUBJECT_TOKEN_BYTES as u64 + 1)
             .read_to_end(&mut bytes)
@@ -201,6 +206,23 @@ impl SubjectTokenProvider for FileSubjectTokenSource {
     }
 }
 
+fn validate_file_metadata(
+    source: &'static str,
+    path: &std::path::Path,
+    metadata: &std::fs::Metadata,
+) -> Result<(), SubjectTokenError> {
+    if !metadata.is_file() {
+        return Err(SubjectTokenError::NotAFile {
+            provider: source,
+            path: path.to_path_buf(),
+        });
+    }
+    if metadata.len() > MAX_SUBJECT_TOKEN_BYTES as u64 {
+        return Err(SubjectTokenError::TooLarge { provider: source });
+    }
+    Ok(())
+}
+
 #[derive(Clone, Debug)]
 pub struct UnavailableSubjectTokenSource {
     source: &'static str,

codex-rs/workload-identity/src/source_tests.rs

@@ -1,4 +1,6 @@
 use pretty_assertions::assert_eq;
+#[cfg(unix)]
+use std::time::Duration;
 
 use super::*;
 
@@ -15,6 +17,35 @@ async fn file_source_rereads_rotated_token() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[cfg(unix)]
+#[tokio::test]
+async fn file_source_rejects_fifo_without_blocking() -> anyhow::Result<()> {
+    use std::ffi::CString;
+    use std::os::unix::ffi::OsStrExt;
+
+    let temp_dir = tempfile::tempdir()?;
+    let path = temp_dir.path().join("subject-token");
+    let c_path = CString::new(path.as_os_str().as_bytes())?;
+    // SAFETY: `c_path` is a valid, nul-terminated path and `mkfifo` does not
+    // retain the pointer after the call.
+    let result = unsafe { libc::mkfifo(c_path.as_ptr(), 0o600) };
+    assert_eq!(result, 0);
+    let source = FileSubjectTokenSource::new(path.clone());
+
+    let result = tokio::time::timeout(Duration::from_secs(1), source.subject_token())
+        .await
+        .expect("FIFO credential validation should not block");
+
+    assert!(matches!(
+        result,
+        Err(SubjectTokenError::NotAFile {
+            provider: "file",
+            path: error_path,
+        }) if error_path == path
+    ));
+    Ok(())
+}
+
 #[tokio::test]
 async fn environment_source_captures_startup_value() -> anyhow::Result<()> {
     const VARIABLE: &str = "CODEX_WIF_SOURCE_CAPTURE_TEST";