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 11b35516a0..b500ecdf77 100644 --- a/codex-rs/app-server/src/request_processors/apps_processor.rs +++ b/codex-rs/app-server/src/request_processors/apps_processor.rs @@ -90,7 +90,7 @@ impl AppsRequestProcessor { .await .map_err(|err| internal_error(format!("failed to resolve auth: {err}")))? } else { - self.auth_manager.auth_cached() + self.auth_manager.auth_for_optional_use().await }; if !config .features diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index ace8ec1573..790388f9fd 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -340,6 +340,53 @@ async fn workload_credential_file_is_denied_even_with_danger_full_access() -> an Ok(()) } +#[tokio::test] +async fn workload_credential_deny_preserves_an_existing_user_deny() -> anyhow::Result<()> { + let codex_home = TempDir::new()?; + let token_file = codex_home.path().join("subject-token"); + let token_path = AbsolutePathBuf::try_from(token_file.clone())?; + let user_deny = FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: token_path.clone(), + }, + access: FileSystemAccessMode::Deny, + }; + let permission_profile = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![user_deny.clone()]), + NetworkSandboxPolicy::Restricted, + ); + let cfg = ConfigToml { + workload_identity: Some(WorkloadIdentityConfig { + identity_provider_id: "idp_example".to_string(), + identity_provider_mapping_id: "idpm_example".to_string(), + audience: "openai-audience".to_string(), + token_url: "https://auth.openai.com/oauth/token".to_string(), + credential_source: CredentialSourceConfig::File { path: token_file }, + }), + ..Default::default() + }; + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides { + permission_profile: Some(permission_profile), + ..Default::default() + }, + codex_home.abs(), + ) + .await?; + + assert!( + config + .permissions + .file_system_sandbox_policy() + .entries + .contains(&user_deny) + ); + assert!(config.workload_identity_credential_deny_paths.is_empty()); + Ok(()) +} + #[tokio::test] async fn workload_identity_rejects_forced_api_key_login() -> anyhow::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 25a966e5cc..21dac489d2 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -810,8 +810,8 @@ pub struct Config { /// Workload identity federation configuration for external ChatGPT auth. pub workload_identity: Option, - /// Runtime-only paths added to the sandbox for workload credential isolation. - /// These enforcement details must not be exposed in model-visible context. + /// Runtime-only paths added to the sandbox solely for workload credential isolation. + /// Pre-existing user or managed denies are excluded so they remain model-visible. #[doc(hidden)] pub workload_identity_credential_deny_paths: Vec, @@ -2370,6 +2370,21 @@ fn apply_workload_identity_filesystem_constraints( .preserve_deny_read_restrictions_from(&FileSystemSandboxPolicy::restricted(entries)); } +fn file_system_policy_has_exact_deny_path( + file_system_sandbox_policy: &FileSystemSandboxPolicy, + path: &AbsolutePathBuf, +) -> bool { + file_system_sandbox_policy.entries.iter().any(|entry| { + entry.access == codex_protocol::permissions::FileSystemAccessMode::Deny + && matches!( + &entry.path, + codex_protocol::permissions::FileSystemPath::Path { + path: denied_path, + } if denied_path == path + ) + }) +} + fn workload_identity_credential_deny_paths( workload_identity: &WorkloadIdentityConfig, ) -> Vec { @@ -3642,11 +3657,21 @@ impl Config { })?; let credential_deny_paths = workload_identity_credential_deny_paths(workload_identity); + let enforcement_only_paths = credential_deny_paths + .iter() + .filter(|path| { + !file_system_policy_has_exact_deny_path( + &effective_file_system_sandbox_policy, + path, + ) + }) + .cloned() + .collect(); apply_workload_identity_filesystem_constraints( &mut effective_file_system_sandbox_policy, &credential_deny_paths, ); - credential_deny_paths + enforcement_only_paths } else { Vec::new() }; diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 7e4b9ecf50..c9aa7b12e3 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -607,7 +607,7 @@ impl Session { let auth = if config_for_mcp.model_provider.requires_openai_auth { auth_manager_clone.auth_result().await? } else { - auth_manager_clone.auth_cached() + auth_manager_clone.auth_for_optional_use().await }; let mcp_config = mcp_manager_for_mcp .runtime_config_for_thread(&config_for_mcp, mcp_thread_init_for_startup) diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 586736a8ad..0815c1d9de 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -8278,6 +8278,33 @@ async fn turn_context_item_redacts_enforcement_only_paths_from_split_policy() { ); } +#[tokio::test] +async fn turn_context_item_preserves_user_deny_overlapping_workload_credential() { + let (_session, mut turn_context) = make_session_and_context().await; + let credential_path = turn_context.config.codex_home.join("workload-token"); + let mut file_system_policy = file_system_policy_with_unreadable_glob(&turn_context); + file_system_policy.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: credential_path.clone(), + }, + access: FileSystemAccessMode::Deny, + }); + turn_context.permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( + turn_context.permission_profile.enforcement(), + &file_system_policy, + turn_context.network_sandbox_policy(), + ); + + let item = turn_context.to_turn_context_item(); + + assert_eq!(item.file_system_sandbox_policy, Some(file_system_policy)); + 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/tests/suite/openai_file_mcp.rs b/codex-rs/core/tests/suite/openai_file_mcp.rs index edc14cead0..02134acffc 100644 --- a/codex-rs/core/tests/suite/openai_file_mcp.rs +++ b/codex-rs/core/tests/suite/openai_file_mcp.rs @@ -2,9 +2,11 @@ use std::fs; use std::path::Path; +use std::sync::Arc; use anyhow::Context; use anyhow::Result; +use codex_config::CONFIG_TOML_FILE; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_utils_path_uri::PathUri; @@ -16,6 +18,7 @@ use core_test_support::apps_test_server::SEARCH_CALENDAR_EXTRACT_TEXT_TOOL as DO use core_test_support::apps_test_server::SEARCH_CALENDAR_NAMESPACE as DOCUMENT_EXTRACT_NAMESPACE; use core_test_support::apps_test_server::apps_enabled_builder; use core_test_support::apps_test_server::recorded_apps_tool_call_by_name; +use core_test_support::apps_test_server::recorded_apps_tool_calls; use core_test_support::hooks::trust_discovered_hooks; use core_test_support::responses::ResponseMock; use core_test_support::responses::ev_assistant_message; @@ -29,6 +32,7 @@ use core_test_support::test_codex::TestCodex; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; +use tempfile::TempDir; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -136,8 +140,8 @@ async fn mount_file_upload_mocks(server: &MockServer, file_size_bytes: u64) { .await; } -async fn run_extract_turn(test: &TestCodex, server: &MockServer) -> Result { - let mock = mount_sse_sequence( +async fn mount_extract_responses(server: &MockServer) -> ResponseMock { + mount_sse_sequence( server, vec![ sse(vec![ @@ -157,12 +161,20 @@ async fn run_extract_turn(test: &TestCodex, server: &MockServer) -> Result Result { + let mock = mount_extract_responses(server).await; test.submit_turn_with_approval_and_permission_profile( "Extract the report text with the app tool.", AskForApproval::Never, - PermissionProfile::Disabled, + permission_profile, ) .await?; @@ -187,7 +199,7 @@ async fn codex_apps_file_params_upload_environment_files_before_mcp_tool_call() Ok(()) }); let test = builder.build_with_remote_env(&server).await?; - let mock = run_extract_turn(&test, &server).await?; + let mock = run_extract_turn(&test, &server, PermissionProfile::Disabled).await?; let requests = mock.requests(); let body = requests[0].body_json(); @@ -243,7 +255,7 @@ async fn codex_apps_file_params_pass_uploaded_file_to_post_tool_use_hook() -> Re }); let test = builder.build(&server).await?; tokio::fs::write(test.cwd.path().join("report.txt"), b"hello world").await?; - let _responses = run_extract_turn(&test, &server).await?; + let _responses = run_extract_turn(&test, &server, PermissionProfile::Disabled).await?; let hook_inputs = read_post_tool_use_hook_inputs(test.codex_home_path())?; assert_eq!(hook_inputs.len(), 1); @@ -255,3 +267,78 @@ async fn codex_apps_file_params_pass_uploaded_file_to_post_tool_use_hook() -> Re server.verify().await; Ok(()) } + +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn codex_apps_file_params_reject_wif_credential_symlink_before_upload() -> Result<()> { + let server = start_mock_server().await; + let apps_server = AppsTestServer::mount(&server).await?; + let home = Arc::new(TempDir::new()?); + let credential_path = home.path().join("subject-token"); + let credential_contents = "external.subject.token.must-not-leak"; + fs::write(&credential_path, credential_contents)?; + + let credential_path_for_config = credential_path.clone(); + let mut builder = apps_enabled_builder(apps_server.chatgpt_base_url.clone()) + .with_home(Arc::clone(&home)) + .with_pre_build_hook(move |home| { + let config = format!( + r#"[workload_identity] +identity_provider_id = "idp_example" +identity_provider_mapping_id = "idpm_example" +audience = "openai-audience" +token_url = "https://auth.openai.com/oauth/token" + +[workload_identity.credential_source] +type = "file" +path = "{}" + +[apps.calendar] +default_tools_approval_mode = "approve" +"#, + credential_path_for_config.display() + ); + fs::write(home.join(CONFIG_TOML_FILE), config).expect("write workload identity config"); + }); + let test = builder.build(&server).await?; + let upload_path = test.cwd.path().join("report.txt"); + std::os::unix::fs::symlink(&credential_path, &upload_path)?; + + assert!( + test.config + .workload_identity_credential_deny_paths + .iter() + .any(|path| path.as_path() == credential_path) + ); + let responses = run_extract_turn( + &test, + &server, + test.config.permissions.permission_profile().clone(), + ) + .await?; + + let output = responses + .function_call_output_text("extract-call-1") + .context("Apps file tool failure should be returned to the model")?; + assert!(output.contains("path is not readable under the current sandbox policy")); + assert!(!output.contains(credential_contents)); + assert!(!output.contains(credential_path.to_string_lossy().as_ref())); + assert_eq!(recorded_apps_tool_calls(&server).await, Vec::::new()); + + let upload_requests = server + .received_requests() + .await + .unwrap_or_default() + .into_iter() + .filter(|request| { + matches!( + request.url.path(), + "/files" | "/upload/file_123" | "/files/file_123/uploaded" + ) + }) + .collect::>(); + assert!(upload_requests.is_empty(), "unexpected file upload request"); + + server.verify().await; + Ok(()) +} diff --git a/codex-rs/login/src/auth/auth_tests.rs b/codex-rs/login/src/auth/auth_tests.rs index 3c4fcc29db..b8dc9a9daa 100644 --- a/codex-rs/login/src/auth/auth_tests.rs +++ b/codex-rs/login/src/auth/auth_tests.rs @@ -801,6 +801,30 @@ async fn auth_result_preserves_external_auth_resolution_errors() { assert!(manager.auth().await.is_err()); } +#[tokio::test] +async fn optional_auth_does_not_resolve_required_external_auth() { + let cached = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let manager = AuthManager::from_auth_for_testing(cached.clone()); + manager.set_external_auth(Arc::new(FailingExternalChatgptAuth( + "required external auth must not run", + ))); + + let resolved = manager + .auth_for_optional_use() + .await + .expect("cached ChatGPT auth should remain available"); + + assert_eq!(resolved.get_token().unwrap(), cached.get_token().unwrap()); + assert_eq!( + manager + .auth() + .await + .expect_err("normal auth should still require external resolution") + .to_string(), + "required external auth must not run" + ); +} + #[tokio::test] async fn replace_non_required_external_auth_preserves_required_provider() { let manager = AuthManager::shared( diff --git a/codex-rs/login/src/auth/manager.rs b/codex-rs/login/src/auth/manager.rs index 7067a1f1a6..514441d8e4 100644 --- a/codex-rs/login/src/auth/manager.rs +++ b/codex-rs/login/src/auth/manager.rs @@ -1731,6 +1731,30 @@ impl AuthManager { Ok(self.auth_cached()) } + /// Resolves auth for optional OpenAI-backed features while another model provider is active. + /// + /// A required external auth source belongs to the primary model provider and must not be + /// invoked merely to initialize an optional feature. Managed ChatGPT auth and optional + /// external auth still use the normal resolution path, including proactive token refresh. + pub async fn auth_for_optional_use(&self) -> Option { + if self + .external_auth() + .is_some_and(|external_auth| external_auth.requires_successful_resolution()) + { + return self.auth_cached(); + } + + match self.auth().await { + Ok(auth) => auth, + Err(error) => { + // The external auth registration can race with the check above. Optional features + // should keep using the last managed credential instead of failing startup. + tracing::error!("Failed to resolve optional feature auth: {error}"); + self.auth_cached() + } + } + } + /// Backwards-compatible explicit name for callers that already handle auth resolution errors. pub async fn auth_result(&self) -> std::io::Result> { self.auth().await diff --git a/codex-rs/login/tests/suite/auth_refresh.rs b/codex-rs/login/tests/suite/auth_refresh.rs index ea2b8688e0..4cab2e9792 100644 --- a/codex-rs/login/tests/suite/auth_refresh.rs +++ b/codex-rs/login/tests/suite/auth_refresh.rs @@ -180,6 +180,16 @@ async fn refresh_token_refreshes_when_auth_is_unchanged() -> Result<()> { #[serial_test::serial(auth_env)] #[tokio::test] async fn auth_refreshes_when_access_token_is_near_expiry() -> Result<()> { + assert_auth_refreshes_when_access_token_is_near_expiry(/*optional*/ false).await +} + +#[serial_test::serial(auth_env)] +#[tokio::test] +async fn optional_auth_refreshes_when_access_token_is_near_expiry() -> Result<()> { + assert_auth_refreshes_when_access_token_is_near_expiry(/*optional*/ true).await +} + +async fn assert_auth_refreshes_when_access_token_is_near_expiry(optional: bool) -> Result<()> { skip_if_no_network!(Ok(())); let server = MockServer::start().await; @@ -208,12 +218,18 @@ async fn auth_refreshes_when_access_token_is_near_expiry() -> Result<()> { }; ctx.write_auth(&initial_auth).await?; - let cached_auth = ctx - .auth_manager - .auth() - .await - .context("auth should resolve")? - .context("auth should be cached")?; + let cached_auth = if optional { + ctx.auth_manager + .auth_for_optional_use() + .await + .context("optional auth should be cached")? + } else { + ctx.auth_manager + .auth() + .await + .context("auth should resolve")? + .context("auth should be cached")? + }; let refreshed_tokens = TokenData { access_token: "new-access-token".to_string(),