diff --git a/codex-rs/app-server/src/request_processors/account_processor/workspace_routing.rs b/codex-rs/app-server/src/request_processors/account_processor/workspace_routing.rs index 90c8781b5b..e5028e3370 100644 --- a/codex-rs/app-server/src/request_processors/account_processor/workspace_routing.rs +++ b/codex-rs/app-server/src/request_processors/account_processor/workspace_routing.rs @@ -6,6 +6,8 @@ use super::*; use codex_app_server_protocol::AccountRoutingOverride; use codex_backend_client::AccountEntry; +use codex_config::ConfigLoadError; +use codex_http_client::NetworkPolicyDenied; use codex_login::WorkspaceRouting; use codex_login::WorkspaceRoutingRequest; use codex_login::WorkspaceRoutingResolver; @@ -54,11 +56,12 @@ pub(super) enum AccountReadError { Routing(#[from] WorkspaceRoutingError), } -/// Routing failure categories carry no account, URL, or backend data. +/// Routing failures carry no account, URL, or backend data. Config failures retain +/// only a location or structured error category, never the raw config error. #[derive(Debug, PartialEq, Eq, thiserror::Error)] pub(super) enum WorkspaceRoutingError { - #[error("failed to load workspace requirements")] - RequirementsLoad, + #[error("failed to load workspace requirements: {0}")] + RequirementsLoad(String), #[error("workspace routing requires a ChatGPT account id")] MissingAccountId, #[error("workspace routing discovery cancelled")] @@ -73,8 +76,8 @@ pub(super) enum WorkspaceRoutingError { MissingWorkspace, #[error("duplicate workspace in routing discovery")] DuplicateWorkspace, - #[error("failed to reload workspace requirements")] - RequirementsReload, + #[error("failed to reload workspace requirements: {0}")] + RequirementsReload(String), #[error("configuration changed during workspace routing discovery; retry account/read")] ConfigurationChanged, #[error("invalid model provider URL")] @@ -103,6 +106,77 @@ pub(super) enum WorkspaceRoutingError { InvalidBackendOrigin, } +fn workspace_config_load_reason(error: &io::Error) -> String { + // Parser messages and cloud bundle errors can quote secrets, even without + // their source snippets. Retain only structured diagnostics before these + // errors reach JSON-RPC clients, login notifications, or tracing. + let mut current = error + .get_ref() + .map(|source| source as &(dyn std::error::Error + 'static)); + while let Some(source) = current { + if let Some(config) = source.downcast_ref::() { + let config = config.config_error(); + let start = config.range.start; + // Non-file diagnostics store a display name, which can contain + // backend-supplied policy names and IDs, in this field. + if !config.path.is_absolute() { + return format!( + "invalid non-file configuration at line {}, column {}", + start.line, start.column + ); + } + return format!( + "invalid configuration at {}:{}:{}", + config.path.display(), + start.line, + start.column + ); + } + if let Some(cloud) = source.downcast_ref::() { + let reason = match cloud.code() { + CloudConfigBundleLoadErrorCode::Timeout => "request timed out", + CloudConfigBundleLoadErrorCode::Auth + | CloudConfigBundleLoadErrorCode::RequestFailed => { + // Cloud loading currently erases the typed denial. Match + // only known safe policy messages, never forward raw text. + let message = cloud.to_string(); + if let Some(policy) = [ + NetworkPolicyDenied::Unavailable, + NetworkPolicyDenied::Destination, + NetworkPolicyDenied::Revoked, + NetworkPolicyDenied::UnsupportedTransport, + ] + .into_iter() + .find(|policy| policy.to_string() == message) + { + return policy.to_string(); + } + if cloud.code() == CloudConfigBundleLoadErrorCode::Auth { + "authentication failed" + } else { + "request failed" + } + } + CloudConfigBundleLoadErrorCode::InvalidBundle => "invalid bundle", + CloudConfigBundleLoadErrorCode::Internal => "internal error", + }; + return match cloud.status_code() { + Some(status) => format!("cloud config: {reason} (HTTP {status})"), + None => format!("cloud config: {reason}"), + }; + } + if let Some(policy) = source.downcast_ref::() { + return policy.to_string(); + } + current = source.source(); + } + // Use the OS error or kind, never an untyped error's potentially secret message. + match error.raw_os_error() { + Some(code) => io::Error::from_raw_os_error(code).to_string(), + None => error.kind().to_string(), + } +} + impl WorkspaceRoutingResolver for AccountRequestProcessor { fn resolve( &self, @@ -237,7 +311,12 @@ impl AccountRequestProcessor { { self.config.as_ref().clone() } - Err(_) => return Err(WorkspaceRoutingError::RequirementsLoad.into()), + Err(error) => { + return Err(WorkspaceRoutingError::RequirementsLoad( + workspace_config_load_reason(&error), + ) + .into()); + } }; let auth = self.auth_manager.auth_cached(); let provider = create_model_provider( @@ -353,9 +432,11 @@ impl AccountRequestProcessor { if accounts.next().is_some() { return Err(WorkspaceRoutingError::DuplicateWorkspace.into()); } - let latest_config = load_config() - .await - .map_err(|_| WorkspaceRoutingError::RequirementsReload)?; + let latest_config = load_config().await.map_err(|error| { + WorkspaceRoutingError::RequirementsReload(workspace_config_load_reason( + &error, + )) + })?; if latest_config.chatgpt_base_url != config.chatgpt_base_url || latest_config.model_provider != config.model_provider || latest_config diff --git a/codex-rs/app-server/src/request_processors/account_processor/workspace_routing_tests.rs b/codex-rs/app-server/src/request_processors/account_processor/workspace_routing_tests.rs index cc06d5086e..7ae5fff4da 100644 --- a/codex-rs/app-server/src/request_processors/account_processor/workspace_routing_tests.rs +++ b/codex-rs/app-server/src/request_processors/account_processor/workspace_routing_tests.rs @@ -5,6 +5,27 @@ use pretty_assertions::assert_eq; use serde_json::Value; use test_case::test_case; +#[test_case(CloudConfigBundleLoadErrorCode::RequestFailed, "cloud config: request failed"; "request")] +#[test_case(CloudConfigBundleLoadErrorCode::Auth, "cloud config: authentication failed"; "refresh")] +fn cloud_failures_preserve_policy_denials_but_redact_other_messages( + code: CloudConfigBundleLoadErrorCode, + redacted: &str, +) { + let denied = NetworkPolicyDenied::Destination.to_string(); + for (message, expected) in [ + (denied.clone(), denied), + ( + "destination denied by application network policy: sk-test-private-value".into(), + redacted.into(), + ), + ] { + let error = io::Error::other(CloudConfigBundleLoadError::new( + code, /*status_code*/ None, message, + )); + assert_eq!(workspace_config_load_reason(&error), expected); + } +} + #[test_case(None; "signed_out")] #[test_case(Some("dummy"); "api_key")] #[tokio::test] @@ -119,15 +140,23 @@ async fn background_routing_skips_config_until_chatgpt_sign_in( ); assert!(loads.load(Ordering::SeqCst) > initial_loads); - std::fs::write(home.path().join("config.toml"), "invalid = [")?; - assert!(matches!( - processor - .read_account_for_background_workspace_routing() - .await, - Err(AccountReadError::Routing( - WorkspaceRoutingError::RequirementsLoad - )) - )); + let config_path = home.path().join("config.toml"); + std::fs::write(&config_path, "invalid = [ 'sk-test-private-value',")?; + let Err(error) = processor + .read_account_for_background_workspace_routing() + .await + else { + panic!("invalid config must prevent workspace routing"); + }; + let message = error.to_string(); + assert!( + message.starts_with(&format!( + "failed to load workspace requirements: invalid configuration at {}:1:", + config_path.display(), + )), + "{message}" + ); + assert!(!message.contains("sk-test-private-value"), "{message}"); Ok(()) } diff --git a/codex-rs/app-server/tests/suite/v2/workspace_routing.rs b/codex-rs/app-server/tests/suite/v2/workspace_routing.rs index 67f2041f49..a36df8e062 100644 --- a/codex-rs/app-server/tests/suite/v2/workspace_routing.rs +++ b/codex-rs/app-server/tests/suite/v2/workspace_routing.rs @@ -395,19 +395,32 @@ async fn api_only_login_does_not_discover_chatgpt_routing() -> Result<()> { Ok(()) } +#[test_case( + json!({"requirements_toml": {"enterprise_managed": [{ + "id": "invalid", "name": "Invalid requirements", "contents": "application = 'sk-test-private-value'", + }]}}), + "failed to load workspace requirements: cloud config: invalid bundle"; + "invalid_requirements" +)] +#[test_case( + json!({"config_toml": {"enterprise_managed": [{ + "id": "sk-test-private-id", "name": "sk-test-private-name", "contents": "model = 123", + }]}}), + "failed to load workspace requirements: invalid non-file configuration at line 1, column 9"; + "invalid_non_file_config" +)] #[tokio::test] -async fn failed_workspace_requirements_do_not_fall_back_to_startup_config() -> Result<()> { +async fn failed_workspace_requirements_do_not_fall_back_to_startup_config( + bundle: Value, + expected_message: &str, +) -> Result<()> { let backend = MockServer::start().await; let home = TempDir::new()?; config(&home, &backend).await?; let mut server = start(&home).await?; backend.reset().await; Mock::given(path("/backend-api/wham/config/bundle")) - .respond_with(ResponseTemplate::new(200).set_body_json( - json!({"requirements_toml": {"enterprise_managed": [{ - "id": "invalid", "name": "Invalid requirements", "contents": "application = true", - }]}}), - )) + .respond_with(ResponseTemplate::new(200).set_body_json(bundle)) .mount(&backend) .await; Mock::given(path("/backend-api/wham/accounts/check")) @@ -421,7 +434,10 @@ async fn failed_workspace_requirements_do_not_fall_back_to_startup_config() -> R server.read_stream_until_notification_message("account/login/completed"), ) .await??; - assert_eq!(notification.params.unwrap()["success"], false); + assert_eq!( + notification.params.expect("account/login/completed params")["success"], + false + ); let request = server .send_get_account_request(GetAccountParams { refresh_token: false, @@ -432,7 +448,8 @@ async fn failed_workspace_requirements_do_not_fall_back_to_startup_config() -> R server.read_stream_until_error_message(RequestId::Integer(request)), ) .await??; - assert_eq!(error.error.message, "failed to load workspace requirements"); + assert_eq!(error.error.code, -32603); + assert_eq!(error.error.message, expected_message); backend.verify().await; Ok(()) }