From 2b01b05bb9600fb3b847d083ef63eafb461b3f2b Mon Sep 17 00:00:00 2001 From: Colin Young Date: Mon, 30 Mar 2026 16:21:42 -0700 Subject: [PATCH] [Codex][Codex CLI] Emit auth-failure Sentry telemetry without feedback Add automatic auth-failure Sentry events for request-level and refresh-token failures, including the early /oauth/token path hit by short-lived CLI commands. Also allow a local DSN override so the non-/feedback path can be validated end to end with a real CLI 401 repro. Co-authored-by: Codex --- codex-rs/app-server/src/lib.rs | 2 + codex-rs/core/src/client.rs | 107 +++---- codex-rs/core/src/models_manager/manager.rs | 40 +-- .../core/src/models_manager/manager_tests.rs | 81 ++++- codex-rs/core/src/util.rs | 39 +++ codex-rs/core/src/util_tests.rs | 71 ++++- codex-rs/exec/src/lib.rs | 19 +- codex-rs/feedback/src/lib.rs | 294 +++++++++++++++++- codex-rs/login/src/auth/auth_tests.rs | 133 ++++++++ codex-rs/login/src/auth/manager.rs | 84 +++++ codex-rs/login/src/lib.rs | 2 + codex-rs/tui/src/lib.rs | 2 + 12 files changed, 785 insertions(+), 89 deletions(-) diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index d2bae7ccd7..e4d62843fc 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -500,6 +500,7 @@ pub async fn run_main_with_transport( let feedback_layer = feedback.logger_layer(); let feedback_metadata_layer = feedback.metadata_layer(); + let feedback_auth_event_layer = feedback.auth_event_layer(); let log_db = codex_state::StateRuntime::init( config.sqlite_home.clone(), config.model_provider_id.clone(), @@ -516,6 +517,7 @@ pub async fn run_main_with_transport( .with(stderr_fmt) .with(feedback_layer) .with(feedback_metadata_layer) + .with(feedback_auth_event_layer) .with(log_db_layer) .with(otel_logger_layer) .with(otel_tracing_layer) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 57e5db1d0d..f2d1c92fe4 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -112,6 +112,7 @@ use crate::response_debug_context::telemetry_transport_error_message; use crate::util::FeedbackRequestTags; use crate::util::emit_feedback_auth_recovery_tags; use crate::util::emit_feedback_request_tags_with_auth_env; +use crate::util::emit_sentry_auth_failure_event_with_auth_env; pub const OPENAI_BETA_HEADER: &str = "OpenAI-Beta"; pub const X_CODEX_TURN_STATE_HEADER: &str = "x-codex-turn-state"; @@ -1732,32 +1733,33 @@ impl RequestTelemetry for ApiTelemetry { debug.auth_error.as_deref(), debug.auth_error_code.as_deref(), ); - emit_feedback_request_tags_with_auth_env( - &FeedbackRequestTags { - endpoint: self.request_route_telemetry.endpoint, - auth_header_attached: self.auth_context.auth_header_attached, - auth_header_name: self.auth_context.auth_header_name, - auth_mode: self.auth_context.auth_mode, - auth_retry_after_unauthorized: Some(self.auth_context.retry_after_unauthorized), - auth_recovery_mode: self.auth_context.recovery_mode, - auth_recovery_phase: self.auth_context.recovery_phase, - auth_connection_reused: None, - auth_request_id: debug.request_id.as_deref(), - auth_cf_ray: debug.cf_ray.as_deref(), - auth_error: debug.auth_error.as_deref(), - auth_error_code: debug.auth_error_code.as_deref(), - auth_recovery_followup_success: self - .auth_context - .retry_after_unauthorized - .then_some(error.is_none()), - auth_recovery_followup_status: self - .auth_context - .retry_after_unauthorized - .then_some(status) - .flatten(), - }, - &self.auth_env_telemetry, - ); + let feedback_tags = FeedbackRequestTags { + endpoint: self.request_route_telemetry.endpoint, + auth_header_attached: self.auth_context.auth_header_attached, + auth_header_name: self.auth_context.auth_header_name, + auth_mode: self.auth_context.auth_mode, + auth_retry_after_unauthorized: Some(self.auth_context.retry_after_unauthorized), + auth_recovery_mode: self.auth_context.recovery_mode, + auth_recovery_phase: self.auth_context.recovery_phase, + auth_connection_reused: None, + auth_request_id: debug.request_id.as_deref(), + auth_cf_ray: debug.cf_ray.as_deref(), + auth_error: debug.auth_error.as_deref(), + auth_error_code: debug.auth_error_code.as_deref(), + auth_recovery_followup_success: self + .auth_context + .retry_after_unauthorized + .then_some(error.is_none()), + auth_recovery_followup_status: self + .auth_context + .retry_after_unauthorized + .then_some(status) + .flatten(), + }; + emit_feedback_request_tags_with_auth_env(&feedback_tags, &self.auth_env_telemetry); + if status == Some(http::StatusCode::UNAUTHORIZED.as_u16()) { + emit_sentry_auth_failure_event_with_auth_env(&feedback_tags, &self.auth_env_telemetry); + } } } @@ -1786,32 +1788,33 @@ impl WebsocketTelemetry for ApiTelemetry { error_message.as_deref(), connection_reused, ); - emit_feedback_request_tags_with_auth_env( - &FeedbackRequestTags { - endpoint: self.request_route_telemetry.endpoint, - auth_header_attached: self.auth_context.auth_header_attached, - auth_header_name: self.auth_context.auth_header_name, - auth_mode: self.auth_context.auth_mode, - auth_retry_after_unauthorized: Some(self.auth_context.retry_after_unauthorized), - auth_recovery_mode: self.auth_context.recovery_mode, - auth_recovery_phase: self.auth_context.recovery_phase, - auth_connection_reused: Some(connection_reused), - auth_request_id: debug.request_id.as_deref(), - auth_cf_ray: debug.cf_ray.as_deref(), - auth_error: debug.auth_error.as_deref(), - auth_error_code: debug.auth_error_code.as_deref(), - auth_recovery_followup_success: self - .auth_context - .retry_after_unauthorized - .then_some(error.is_none()), - auth_recovery_followup_status: self - .auth_context - .retry_after_unauthorized - .then_some(status) - .flatten(), - }, - &self.auth_env_telemetry, - ); + let feedback_tags = FeedbackRequestTags { + endpoint: self.request_route_telemetry.endpoint, + auth_header_attached: self.auth_context.auth_header_attached, + auth_header_name: self.auth_context.auth_header_name, + auth_mode: self.auth_context.auth_mode, + auth_retry_after_unauthorized: Some(self.auth_context.retry_after_unauthorized), + auth_recovery_mode: self.auth_context.recovery_mode, + auth_recovery_phase: self.auth_context.recovery_phase, + auth_connection_reused: Some(connection_reused), + auth_request_id: debug.request_id.as_deref(), + auth_cf_ray: debug.cf_ray.as_deref(), + auth_error: debug.auth_error.as_deref(), + auth_error_code: debug.auth_error_code.as_deref(), + auth_recovery_followup_success: self + .auth_context + .retry_after_unauthorized + .then_some(error.is_none()), + auth_recovery_followup_status: self + .auth_context + .retry_after_unauthorized + .then_some(status) + .flatten(), + }; + emit_feedback_request_tags_with_auth_env(&feedback_tags, &self.auth_env_telemetry); + if status == Some(http::StatusCode::UNAUTHORIZED.as_u16()) { + emit_sentry_auth_failure_event_with_auth_env(&feedback_tags, &self.auth_env_telemetry); + } } fn on_ws_event( diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index aa22186f98..462ec2268f 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -19,6 +19,7 @@ use crate::response_debug_context::extract_response_debug_context; use crate::response_debug_context::telemetry_transport_error_message; use crate::util::FeedbackRequestTags; use crate::util::emit_feedback_request_tags_with_auth_env; +use crate::util::emit_sentry_auth_failure_event_with_auth_env; use codex_api::ModelsClient; use codex_api::RequestTelemetry; use codex_api::ReqwestTransport; @@ -114,25 +115,26 @@ impl RequestTelemetry for ModelsRequestTelemetry { auth.error_code = response_debug.auth_error_code.as_deref(), auth.mode = self.auth_mode.as_deref(), ); - emit_feedback_request_tags_with_auth_env( - &FeedbackRequestTags { - endpoint: MODELS_ENDPOINT, - auth_header_attached: self.auth_header_attached, - auth_header_name: self.auth_header_name, - auth_mode: self.auth_mode.as_deref(), - auth_retry_after_unauthorized: None, - auth_recovery_mode: None, - auth_recovery_phase: None, - auth_connection_reused: None, - auth_request_id: response_debug.request_id.as_deref(), - auth_cf_ray: response_debug.cf_ray.as_deref(), - auth_error: response_debug.auth_error.as_deref(), - auth_error_code: response_debug.auth_error_code.as_deref(), - auth_recovery_followup_success: None, - auth_recovery_followup_status: None, - }, - &self.auth_env, - ); + let feedback_tags = FeedbackRequestTags { + endpoint: MODELS_ENDPOINT, + auth_header_attached: self.auth_header_attached, + auth_header_name: self.auth_header_name, + auth_mode: self.auth_mode.as_deref(), + auth_retry_after_unauthorized: None, + auth_recovery_mode: None, + auth_recovery_phase: None, + auth_connection_reused: None, + auth_request_id: response_debug.request_id.as_deref(), + auth_cf_ray: response_debug.cf_ray.as_deref(), + auth_error: response_debug.auth_error.as_deref(), + auth_error_code: response_debug.auth_error_code.as_deref(), + auth_recovery_followup_success: None, + auth_recovery_followup_status: None, + }; + emit_feedback_request_tags_with_auth_env(&feedback_tags, &self.auth_env); + if status == Some(http::StatusCode::UNAUTHORIZED.as_u16()) { + emit_sentry_auth_failure_event_with_auth_env(&feedback_tags, &self.auth_env); + } } } diff --git a/codex-rs/core/src/models_manager/manager_tests.rs b/codex-rs/core/src/models_manager/manager_tests.rs index 3c9add309a..0618e5cbee 100644 --- a/codex-rs/core/src/models_manager/manager_tests.rs +++ b/codex-rs/core/src/models_manager/manager_tests.rs @@ -215,6 +215,7 @@ impl Visit for TagCollectorVisitor { #[derive(Clone)] struct TagCollectorLayer { + target: &'static str, tags: Arc>>, } @@ -223,7 +224,7 @@ where S: Subscriber + for<'a> LookupSpan<'a>, { fn on_event(&self, event: &Event<'_>, _ctx: Context<'_, S>) { - if event.metadata().target() != "feedback_tags" { + if event.metadata().target() != self.target { return; } let mut visitor = TagCollectorVisitor::default(); @@ -741,7 +742,10 @@ async fn refresh_available_models_skips_network_without_chatgpt_auth() { fn models_request_telemetry_emits_auth_env_feedback_tags_on_failure() { let tags = Arc::new(Mutex::new(BTreeMap::new())); let _guard = tracing_subscriber::registry() - .with(TagCollectorLayer { tags: tags.clone() }) + .with(TagCollectorLayer { + target: "feedback_tags", + tags: tags.clone(), + }) .set_default(); let telemetry = ModelsRequestTelemetry { @@ -835,6 +839,79 @@ fn models_request_telemetry_emits_auth_env_feedback_tags_on_failure() { ); } +#[test] +fn models_request_telemetry_emits_sentry_auth_failure_event_on_401() { + let tags = Arc::new(Mutex::new(BTreeMap::new())); + let _guard = tracing_subscriber::registry() + .with(TagCollectorLayer { + target: crate::util::SENTRY_AUTH_FAILURES_TARGET, + tags: tags.clone(), + }) + .set_default(); + + let telemetry = ModelsRequestTelemetry { + auth_mode: Some(TelemetryAuthMode::Chatgpt.to_string()), + auth_header_attached: true, + auth_header_name: Some("authorization"), + auth_env: crate::auth_env_telemetry::AuthEnvTelemetry { + openai_api_key_env_present: false, + codex_api_key_env_present: false, + codex_api_key_env_enabled: false, + provider_env_key_name: Some("configured".to_string()), + provider_env_key_present: Some(false), + refresh_token_url_override_present: false, + }, + }; + let mut headers = HeaderMap::new(); + headers.insert("x-request-id", "req-models-401".parse().unwrap()); + headers.insert("cf-ray", "ray-models-401".parse().unwrap()); + headers.insert( + "x-openai-authorization-error", + "missing_authorization_header".parse().unwrap(), + ); + headers.insert( + "x-error-json", + base64::engine::general_purpose::STANDARD + .encode(r#"{"error":{"code":"token_expired"}}"#) + .parse() + .unwrap(), + ); + telemetry.on_request( + 1, + Some(StatusCode::UNAUTHORIZED), + Some(&TransportError::Http { + status: StatusCode::UNAUTHORIZED, + url: Some("https://example.test/models".to_string()), + headers: Some(headers), + body: Some("plain text error".to_string()), + }), + Duration::from_millis(17), + ); + + let tags = tags.lock().unwrap().clone(); + assert_eq!( + tags.get("report_kind").map(String::as_str), + Some("auth_failure_auto") + ); + assert_eq!(tags.get("endpoint").map(String::as_str), Some("/models")); + assert_eq!( + tags.get("auth_header_attached").map(String::as_str), + Some("true") + ); + assert_eq!( + tags.get("auth_header_name").map(String::as_str), + Some("authorization") + ); + assert_eq!( + tags.get("auth_request_id").map(String::as_str), + Some("req-models-401") + ); + assert_eq!( + tags.get("auth_cf_ray").map(String::as_str), + Some("ray-models-401") + ); +} + #[test] fn build_available_models_picks_default_after_hiding_hidden_models() { let codex_home = tempdir().expect("temp dir"); diff --git a/codex-rs/core/src/util.rs b/codex-rs/core/src/util.rs index 04d973c86b..2433c77074 100644 --- a/codex-rs/core/src/util.rs +++ b/codex-rs/core/src/util.rs @@ -11,6 +11,7 @@ use crate::parse_command::shlex_join; const INITIAL_DELAY_MS: u64 = 200; const BACKOFF_FACTOR: f64 = 2.0; +pub(crate) const SENTRY_AUTH_FAILURES_TARGET: &str = "sentry_auth_failures"; /// Emit structured feedback metadata as key/value pairs. /// @@ -201,6 +202,44 @@ pub(crate) fn emit_feedback_auth_recovery_tags( ); } +pub(crate) fn emit_sentry_auth_failure_event_with_auth_env( + tags: &FeedbackRequestTags<'_>, + auth_env: &AuthEnvTelemetry, +) { + tracing::event!( + target: SENTRY_AUTH_FAILURES_TARGET, + tracing::Level::ERROR, + report_kind = "auth_failure_auto", + endpoint = tags.endpoint, + auth_header_attached = tags.auth_header_attached, + auth_header_name = tags.auth_header_name.unwrap_or(""), + auth_mode = tags.auth_mode.unwrap_or(""), + auth_retry_after_unauthorized = tags + .auth_retry_after_unauthorized + .unwrap_or(false), + auth_recovery_mode = tags.auth_recovery_mode.unwrap_or(""), + auth_recovery_phase = tags.auth_recovery_phase.unwrap_or(""), + auth_connection_reused = tags.auth_connection_reused.unwrap_or(false), + auth_request_id = tags.auth_request_id.unwrap_or(""), + auth_cf_ray = tags.auth_cf_ray.unwrap_or(""), + auth_error = tags.auth_error.unwrap_or(""), + auth_error_code = tags.auth_error_code.unwrap_or(""), + auth_recovery_followup_success = tags + .auth_recovery_followup_success + .unwrap_or(false), + auth_recovery_followup_status = tags + .auth_recovery_followup_status + .unwrap_or_default(), + auth_env_openai_api_key_present = auth_env.openai_api_key_env_present, + auth_env_codex_api_key_present = auth_env.codex_api_key_env_present, + auth_env_codex_api_key_enabled = auth_env.codex_api_key_env_enabled, + auth_env_provider_key_name = auth_env.provider_env_key_name.as_deref().unwrap_or(""), + auth_env_provider_key_present = auth_env.provider_env_key_present.unwrap_or(false), + auth_env_refresh_token_url_override_present = auth_env.refresh_token_url_override_present, + cli_version = env!("CARGO_PKG_VERSION"), + ); +} + pub fn backoff(attempt: u64) -> Duration { let exp = BACKOFF_FACTOR.powi(attempt.saturating_sub(1) as i32); let base = (INITIAL_DELAY_MS as f64 * exp) as u64; diff --git a/codex-rs/core/src/util_tests.rs b/codex-rs/core/src/util_tests.rs index 61bf70284a..b0e276510e 100644 --- a/codex-rs/core/src/util_tests.rs +++ b/codex-rs/core/src/util_tests.rs @@ -44,6 +44,7 @@ impl Visit for TagCollectorVisitor { #[derive(Clone)] struct TagCollectorLayer { + target: &'static str, tags: Arc>>, event_count: Arc>, } @@ -53,7 +54,7 @@ where S: Subscriber + for<'a> LookupSpan<'a>, { fn on_event(&self, event: &Event<'_>, _ctx: Context<'_, S>) { - if event.metadata().target() != "feedback_tags" { + if event.metadata().target() != self.target { return; } let mut visitor = TagCollectorVisitor::default(); @@ -69,6 +70,7 @@ fn emit_feedback_request_tags_records_sentry_feedback_fields() { let event_count = Arc::new(Mutex::new(0)); let _guard = tracing_subscriber::registry() .with(TagCollectorLayer { + target: "feedback_tags", tags: tags.clone(), event_count: event_count.clone(), }) @@ -172,6 +174,7 @@ fn emit_feedback_auth_recovery_tags_preserves_401_specific_fields() { let event_count = Arc::new(Mutex::new(0)); let _guard = tracing_subscriber::registry() .with(TagCollectorLayer { + target: "feedback_tags", tags: tags.clone(), event_count: event_count.clone(), }) @@ -213,6 +216,7 @@ fn emit_feedback_auth_recovery_tags_clears_stale_401_fields() { let event_count = Arc::new(Mutex::new(0)); let _guard = tracing_subscriber::registry() .with(TagCollectorLayer { + target: "feedback_tags", tags: tags.clone(), event_count: event_count.clone(), }) @@ -260,6 +264,7 @@ fn emit_feedback_request_tags_preserves_latest_auth_fields_after_unauthorized() let event_count = Arc::new(Mutex::new(0)); let _guard = tracing_subscriber::registry() .with(TagCollectorLayer { + target: "feedback_tags", tags: tags.clone(), event_count: event_count.clone(), }) @@ -307,12 +312,76 @@ fn emit_feedback_request_tags_preserves_latest_auth_fields_after_unauthorized() assert_eq!(*event_count.lock().unwrap(), 1); } +#[test] +fn emit_sentry_auth_failure_event_records_non_feedback_fields() { + let tags = Arc::new(Mutex::new(BTreeMap::new())); + let event_count = Arc::new(Mutex::new(0)); + let _guard = tracing_subscriber::registry() + .with(TagCollectorLayer { + target: SENTRY_AUTH_FAILURES_TARGET, + tags: tags.clone(), + event_count: event_count.clone(), + }) + .set_default(); + + let auth_env = AuthEnvTelemetry { + openai_api_key_env_present: true, + codex_api_key_env_present: false, + codex_api_key_env_enabled: true, + provider_env_key_name: Some("configured".to_string()), + provider_env_key_present: Some(true), + refresh_token_url_override_present: true, + }; + + emit_sentry_auth_failure_event_with_auth_env( + &FeedbackRequestTags { + endpoint: "/responses", + auth_header_attached: true, + auth_header_name: Some("authorization"), + auth_mode: Some("chatgpt"), + auth_retry_after_unauthorized: Some(false), + auth_recovery_mode: Some("managed"), + auth_recovery_phase: Some("reload"), + auth_connection_reused: Some(false), + auth_request_id: Some("req-123"), + auth_cf_ray: Some("ray-123"), + auth_error: Some("missing_authorization_header"), + auth_error_code: Some("token_expired"), + auth_recovery_followup_success: None, + auth_recovery_followup_status: None, + }, + &auth_env, + ); + + let tags = tags.lock().unwrap().clone(); + assert_eq!( + tags.get("report_kind").map(String::as_str), + Some("auth_failure_auto") + ); + assert_eq!(tags.get("endpoint").map(String::as_str), Some("/responses")); + assert_eq!( + tags.get("auth_header_attached").map(String::as_str), + Some("true") + ); + assert_eq!( + tags.get("auth_header_name").map(String::as_str), + Some("authorization") + ); + assert_eq!( + tags.get("auth_request_id").map(String::as_str), + Some("req-123") + ); + assert_eq!(tags.get("auth_cf_ray").map(String::as_str), Some("ray-123")); + assert_eq!(*event_count.lock().unwrap(), 1); +} + #[test] fn emit_feedback_request_tags_preserves_auth_env_fields_for_legacy_emitters() { let tags = Arc::new(Mutex::new(BTreeMap::new())); let event_count = Arc::new(Mutex::new(0)); let _guard = tracing_subscriber::registry() .with(TagCollectorLayer { + target: "feedback_tags", tags: tags.clone(), event_count: event_count.clone(), }) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index ae1285be4d..bce3d6e3b8 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -66,6 +66,7 @@ use codex_core::config_loader::format_config_error_with_source; use codex_core::format_exec_policy_error_with_source; use codex_core::path_utils; use codex_feedback::CodexFeedback; +use codex_feedback::upload_auth_failure_event_tags; use codex_git_utils::get_git_repo_root; use codex_otel::set_parent_from_context; use codex_otel::traceparent_context_from_env; @@ -90,6 +91,7 @@ use std::io::IsTerminal; use std::io::Read; use std::path::Path; use std::path::PathBuf; +use std::sync::Arc; use supports_color::Stream; use tokio::sync::mpsc; use tracing::Instrument; @@ -256,6 +258,14 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result } }; + let feedback = CodexFeedback::new(); + let _auth_failure_reporter_guard = + codex_core::auth::set_auth_failure_reporter(Arc::new(|tags| { + if let Err(err) = upload_auth_failure_event_tags(tags) { + tracing::warn!(error = %err, "failed to upload auth failure event"); + } + })); + #[allow(clippy::print_stderr)] let config_toml = match load_config_as_toml_with_cli_overrides( &codex_home, @@ -410,8 +420,15 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result let otel_tracing_layer = otel.as_ref().and_then(|o| o.tracing_layer()); + let feedback_layer = feedback.logger_layer(); + let feedback_metadata_layer = feedback.metadata_layer(); + let feedback_auth_event_layer = feedback.auth_event_layer(); + let _ = tracing_subscriber::registry() .with(fmt_layer) + .with(feedback_layer) + .with(feedback_metadata_layer) + .with(feedback_auth_event_layer) .with(otel_tracing_layer) .with(otel_logger_layer) .try_init(); @@ -436,7 +453,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result cli_overrides: run_cli_overrides, loader_overrides: run_loader_overrides, cloud_requirements: run_cloud_requirements, - feedback: CodexFeedback::new(), + feedback, config_warnings, session_source: SessionSource::Exec, enable_codex_api_key_env: true, diff --git a/codex-rs/feedback/src/lib.rs b/codex-rs/feedback/src/lib.rs index 19c6abffdf..1cfa9e76a6 100644 --- a/codex-rs/feedback/src/lib.rs +++ b/codex-rs/feedback/src/lib.rs @@ -28,8 +28,11 @@ pub mod feedback_diagnostics; const DEFAULT_MAX_BYTES: usize = 4 * 1024 * 1024; // 4 MiB const SENTRY_DSN: &str = "https://ae32ed50620d7a7792c1ce5df38b3e3e@o33249.ingest.us.sentry.io/4510195390611458"; +const SENTRY_DSN_OVERRIDE_ENV_VAR: &str = "CODEX_SENTRY_DSN_OVERRIDE"; const UPLOAD_TIMEOUT_SECS: u64 = 10; const FEEDBACK_TAGS_TARGET: &str = "feedback_tags"; +const SENTRY_AUTH_FAILURES_TARGET: &str = "sentry_auth_failures"; +const AUTH_FAILURE_REPORT_KIND: &str = "auth_failure_auto"; const MAX_FEEDBACK_TAGS: usize = 64; #[derive(Clone)] @@ -93,6 +96,18 @@ impl CodexFeedback { .with_filter(Targets::new().with_target(FEEDBACK_TAGS_TARGET, Level::TRACE)) } + /// Returns a [`tracing_subscriber`] layer that uploads lightweight auth-failure Sentry events. + /// + /// Events with `target: "sentry_auth_failures"` are converted into searchable Sentry issues + /// without relying on a manual `/feedback` upload. + pub fn auth_event_layer(&self) -> impl Layer + Send + Sync + 'static + where + S: tracing::Subscriber + for<'a> LookupSpan<'a>, + { + AuthFailureSentryLayer + .with_filter(Targets::new().with_target(SENTRY_AUTH_FAILURES_TARGET, Level::TRACE)) + } + pub fn snapshot(&self, session_id: Option) -> FeedbackSnapshot { let bytes = { let guard = self.inner.ring.lock().expect("mutex poisoned"); @@ -252,25 +267,13 @@ impl FeedbackSnapshot { session_source: Option, logs_override: Option>, ) -> Result<()> { - use std::collections::BTreeMap; - use std::str::FromStr; - use std::sync::Arc; - - use sentry::Client; - use sentry::ClientOptions; use sentry::protocol::Envelope; use sentry::protocol::EnvelopeItem; use sentry::protocol::Event; use sentry::protocol::Level; - use sentry::transports::DefaultTransportFactory; - use sentry::types::Dsn; + use std::collections::BTreeMap; - // Build Sentry client - let client = Client::from_config(ClientOptions { - dsn: Some(Dsn::from_str(SENTRY_DSN).map_err(|e| anyhow!("invalid DSN: {e}"))?), - transport: Some(Arc::new(DefaultTransportFactory {})), - ..Default::default() - }); + let client = build_sentry_client()?; let cli_version = env!("CARGO_PKG_VERSION"); let mut tags = BTreeMap::from([ @@ -398,6 +401,29 @@ impl FeedbackSnapshot { } } +fn build_sentry_client() -> Result { + build_sentry_client_with_dsn_override( + std::env::var(SENTRY_DSN_OVERRIDE_ENV_VAR).ok().as_deref(), + ) +} + +fn build_sentry_client_with_dsn_override(dsn_override: Option<&str>) -> Result { + use std::str::FromStr; + use std::sync::Arc; + + use sentry::Client; + use sentry::ClientOptions; + use sentry::transports::DefaultTransportFactory; + use sentry::types::Dsn; + + let dsn = dsn_override.unwrap_or(SENTRY_DSN); + Ok(Client::from_config(ClientOptions { + dsn: Some(Dsn::from_str(dsn).map_err(|e| anyhow!("invalid DSN: {e}"))?), + transport: Some(Arc::new(DefaultTransportFactory {})), + ..Default::default() + })) +} + fn display_classification(classification: &str) -> String { match classification { "bug" => "Bug".to_string(), @@ -440,6 +466,30 @@ where } } +#[derive(Clone, Copy)] +struct AuthFailureSentryLayer; + +impl Layer for AuthFailureSentryLayer +where + S: tracing::Subscriber + for<'a> LookupSpan<'a>, +{ + fn on_event(&self, event: &Event<'_>, _ctx: tracing_subscriber::layer::Context<'_, S>) { + if event.metadata().target() != SENTRY_AUTH_FAILURES_TARGET { + return; + } + + let mut visitor = FeedbackTagsVisitor::default(); + event.record(&mut visitor); + if visitor.tags.is_empty() { + return; + } + + if let Err(err) = upload_auth_failure_event_tags(visitor.tags) { + tracing::warn!(error = %err, "failed to upload auth failure event"); + } + } +} + #[derive(Default)] struct FeedbackTagsVisitor { tags: BTreeMap, @@ -477,10 +527,94 @@ impl Visit for FeedbackTagsVisitor { } } +fn finalize_auth_failure_tags(mut tags: BTreeMap) -> BTreeMap { + tags.retain(|_, value| !value.is_empty()); + tags.insert( + String::from("report_kind"), + AUTH_FAILURE_REPORT_KIND.to_string(), + ); + tags +} + +fn auth_failure_grouping_key(tags: &BTreeMap) -> Vec { + let endpoint = tags + .get("endpoint") + .cloned() + .unwrap_or_else(|| "unknown".to_string()); + let error_code = tags + .get("auth_error_code") + .filter(|value| !value.is_empty()) + .or_else(|| tags.get("auth_error").filter(|value| !value.is_empty())) + .cloned() + .unwrap_or_else(|| "unknown".to_string()); + let auth_header_attached = tags + .get("auth_header_attached") + .cloned() + .unwrap_or_else(|| "unknown".to_string()); + vec![ + "codex".to_string(), + AUTH_FAILURE_REPORT_KIND.to_string(), + endpoint, + error_code, + auth_header_attached, + ] +} + +fn build_auth_failure_event(tags: BTreeMap) -> sentry::protocol::Event<'static> { + use std::borrow::Cow; + + use sentry::protocol::Event; + use sentry::protocol::Level; + + let endpoint = tags + .get("endpoint") + .cloned() + .unwrap_or_else(|| "unknown".to_string()); + let fingerprint = auth_failure_grouping_key(&tags) + .into_iter() + .map(Cow::Owned) + .collect::>>(); + Event { + level: Level::Error, + message: Some(format!("Codex client auth failure on {endpoint}")), + fingerprint: fingerprint.into(), + tags, + ..Default::default() + } +} + +pub fn upload_auth_failure_event_tags(tags: BTreeMap) -> Result<()> { + upload_auth_failure_event_with_dsn_override( + finalize_auth_failure_tags(tags), + std::env::var(SENTRY_DSN_OVERRIDE_ENV_VAR).ok().as_deref(), + ) +} + +fn upload_auth_failure_event_with_dsn_override( + tags: BTreeMap, + dsn_override: Option<&str>, +) -> Result<()> { + use sentry::protocol::Envelope; + use sentry::protocol::EnvelopeItem; + + let client = build_sentry_client_with_dsn_override(dsn_override)?; + let mut envelope = Envelope::new(); + envelope.add_item(EnvelopeItem::Event(build_auth_failure_event(tags))); + client.send_envelope(envelope); + client.flush(Some(Duration::from_secs(UPLOAD_TIMEOUT_SECS))); + Ok(()) +} + #[cfg(test)] mod tests { use std::ffi::OsStr; use std::fs; + use std::io::Read; + use std::io::Write; + use std::net::TcpListener; + use std::sync::mpsc; + use std::thread; + use std::time::Duration as StdDuration; use super::*; use feedback_diagnostics::FeedbackDiagnostic; @@ -515,6 +649,138 @@ mod tests { pretty_assertions::assert_eq!(snap.tags.get("cached").map(String::as_str), Some("true")); } + #[test] + fn finalize_auth_failure_tags_adds_report_kind_and_drops_empty_values() { + let tags = finalize_auth_failure_tags(BTreeMap::from([ + (String::from("endpoint"), String::from("/responses")), + (String::from("auth_request_id"), String::new()), + ])); + + assert_eq!( + tags.get("report_kind").map(String::as_str), + Some(AUTH_FAILURE_REPORT_KIND) + ); + assert_eq!(tags.get("endpoint").map(String::as_str), Some("/responses")); + assert!(!tags.contains_key("auth_request_id")); + } + + #[test] + fn auth_failure_grouping_key_uses_endpoint_code_and_header_state() { + let grouping_key = auth_failure_grouping_key(&BTreeMap::from([ + (String::from("endpoint"), String::from("/responses")), + ( + String::from("auth_error_code"), + String::from("token_expired"), + ), + (String::from("auth_header_attached"), String::from("true")), + ])); + + assert_eq!( + grouping_key, + vec![ + "codex".to_string(), + AUTH_FAILURE_REPORT_KIND.to_string(), + "/responses".to_string(), + "token_expired".to_string(), + "true".to_string(), + ] + ); + } + + #[test] + fn build_auth_failure_event_sets_stable_message_and_tags() { + let event = build_auth_failure_event(BTreeMap::from([ + ( + String::from("report_kind"), + AUTH_FAILURE_REPORT_KIND.to_string(), + ), + (String::from("endpoint"), String::from("/responses")), + (String::from("auth_header_attached"), String::from("true")), + ])); + + assert_eq!( + event.message.as_deref(), + Some("Codex client auth failure on /responses") + ); + assert_eq!( + event.tags.get("report_kind").map(String::as_str), + Some(AUTH_FAILURE_REPORT_KIND) + ); + assert_eq!( + event.tags.get("auth_header_attached").map(String::as_str), + Some("true") + ); + } + + #[test] + fn auth_failure_upload_posts_envelope_to_overridden_dsn() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind local listener"); + listener + .set_nonblocking(false) + .expect("listener should stay blocking"); + let addr = listener.local_addr().expect("local addr"); + let (tx, rx) = mpsc::channel(); + + let server = thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept envelope request"); + let mut buffer = Vec::new(); + let mut headers_end = None; + while headers_end.is_none() { + let mut chunk = [0_u8; 4096]; + let read = stream.read(&mut chunk).expect("read request headers"); + buffer.extend_from_slice(&chunk[..read]); + headers_end = buffer.windows(4).position(|window| window == b"\r\n\r\n"); + } + let headers_end = headers_end.expect("headers terminator should exist") + 4; + let headers = String::from_utf8_lossy(&buffer[..headers_end]); + let content_length = headers + .lines() + .find_map(|line| { + let (name, value) = line.split_once(':')?; + if name.eq_ignore_ascii_case("content-length") { + Some(value.trim().parse::().expect("content-length")) + } else { + None + } + }) + .expect("content-length header"); + while buffer.len() < headers_end + content_length { + let mut chunk = [0_u8; 4096]; + let read = stream.read(&mut chunk).expect("read request body"); + buffer.extend_from_slice(&chunk[..read]); + } + stream + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n") + .expect("write response"); + tx.send(buffer).expect("capture request"); + }); + + let dsn = format!("http://public@127.0.0.1:{}/1", addr.port()); + upload_auth_failure_event_with_dsn_override( + finalize_auth_failure_tags(BTreeMap::from([ + (String::from("endpoint"), String::from("/oauth/token")), + ( + String::from("auth_error_code"), + String::from("refresh_token_reused"), + ), + (String::from("auth_header_attached"), String::from("true")), + ])), + Some(&dsn), + ) + .expect("upload auth failure event"); + + let request = rx + .recv_timeout(StdDuration::from_secs(5)) + .expect("receive envelope request"); + server.join().expect("server thread should exit"); + + let request_text = String::from_utf8_lossy(&request); + assert!(request_text.contains("POST /api/1/envelope/")); + assert!(request_text.contains("\"report_kind\":\"auth_failure_auto\"")); + assert!(request_text.contains("\"endpoint\":\"/oauth/token\"")); + assert!(request_text.contains("\"auth_error_code\":\"refresh_token_reused\"")); + } + #[test] fn feedback_attachments_gate_connectivity_diagnostics() { let extra_filename = format!("codex-feedback-extra-{}.jsonl", ThreadId::new()); diff --git a/codex-rs/login/src/auth/auth_tests.rs b/codex-rs/login/src/auth/auth_tests.rs index 53580c72d3..662475bdbb 100644 --- a/codex-rs/login/src/auth/auth_tests.rs +++ b/codex-rs/login/src/auth/auth_tests.rs @@ -10,11 +10,20 @@ use base64::Engine; use codex_protocol::config_types::ForcedLoginMethod; use codex_protocol::config_types::ModelProviderAuthInfo; use pretty_assertions::assert_eq; +use reqwest::header::HeaderMap; +use reqwest::header::HeaderValue; use serde::Serialize; use serde_json::json; +use std::collections::BTreeMap; use std::sync::Arc; use tempfile::TempDir; +use std::sync::Mutex; use tempfile::tempdir; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; #[tokio::test] async fn refresh_without_id_token() { @@ -789,3 +798,127 @@ fn missing_plan_type_maps_to_unknown() { pretty_assertions::assert_eq!(auth.account_plan_type(), Some(AccountPlanType::Unknown)); } + +#[test] +fn refresh_token_reused_builds_sentry_auth_failure_fields() { + let mut headers = HeaderMap::new(); + headers.insert("x-request-id", HeaderValue::from_static("req_refresh_123")); + headers.insert("cf-ray", HeaderValue::from_static("abc123-SJC")); + + assert_eq!( + refresh_failure_sentry_auth_fields( + &headers, + r#"{"error":{"code":"refresh_token_reused","message":"refresh token already used"}}"# + ), + BTreeMap::from([ + ("report_kind".to_string(), "auth_failure_auto".to_string()), + ("endpoint".to_string(), "/oauth/token".to_string()), + ("auth_header_attached".to_string(), "true".to_string()), + ("auth_header_name".to_string(), "authorization".to_string()), + ("auth_mode".to_string(), "Chatgpt".to_string()), + ("auth_request_id".to_string(), "req_refresh_123".to_string()), + ("auth_cf_ray".to_string(), "abc123-SJC".to_string()), + ( + "auth_error".to_string(), + "refresh token already used".to_string() + ), + ( + "auth_error_code".to_string(), + "refresh_token_reused".to_string() + ), + ( + "cli_version".to_string(), + env!("CARGO_PKG_VERSION").to_string() + ), + ]) + ); +} + +#[tokio::test] +#[serial(codex_api_key)] +async fn refresh_token_reused_reports_fields_through_auth_failure_reporter() { + let codex_home = tempdir().unwrap(); + write_auth_file( + AuthFileParams { + openai_api_key: None, + chatgpt_plan_type: Some("pro".to_string()), + chatgpt_account_id: Some("org_mine".to_string()), + }, + codex_home.path(), + ) + .expect("failed to write auth file"); + + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/oauth/token")) + .respond_with( + ResponseTemplate::new(401) + .insert_header("x-request-id", "req_refresh_123") + .insert_header("cf-ray", "abc123-SJC") + .set_body_json(json!({ + "error": { + "code": "refresh_token_reused", + "message": "refresh token already used" + } + })), + ) + .mount(&server) + .await; + let _refresh_url = EnvVarGuard::set( + REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR, + &format!("{}/oauth/token", server.uri()), + ); + + let reported = Arc::new(Mutex::new(Vec::new())); + let _reporter_guard = set_auth_failure_reporter({ + let reported = Arc::clone(&reported); + Arc::new(move |fields| { + reported + .lock() + .expect("report collector poisoned") + .push(fields); + }) + }); + + let manager = AuthManager::shared( + codex_home.path().to_path_buf(), + false, + AuthCredentialsStoreMode::File, + ); + + let err = manager + .refresh_token_from_authority() + .await + .expect_err("refresh should fail permanently"); + assert_eq!( + err.failed_reason(), + Some(RefreshTokenFailedReason::Exhausted) + ); + + let reported = reported.lock().expect("report collector poisoned"); + assert_eq!(reported.len(), 1); + assert_eq!( + reported[0], + BTreeMap::from([ + ("report_kind".to_string(), "auth_failure_auto".to_string()), + ("endpoint".to_string(), "/oauth/token".to_string()), + ("auth_header_attached".to_string(), "true".to_string()), + ("auth_header_name".to_string(), "authorization".to_string()), + ("auth_mode".to_string(), "Chatgpt".to_string()), + ("auth_request_id".to_string(), "req_refresh_123".to_string()), + ("auth_cf_ray".to_string(), "abc123-SJC".to_string()), + ( + "auth_error".to_string(), + "refresh token already used".to_string() + ), + ( + "auth_error_code".to_string(), + "refresh_token_reused".to_string() + ), + ( + "cli_version".to_string(), + env!("CARGO_PKG_VERSION").to_string() + ), + ]) + ); +} diff --git a/codex-rs/login/src/auth/manager.rs b/codex-rs/login/src/auth/manager.rs index d13106cdd3..324219755a 100644 --- a/codex-rs/login/src/auth/manager.rs +++ b/codex-rs/login/src/auth/manager.rs @@ -1,10 +1,13 @@ use async_trait::async_trait; use chrono::Utc; +use once_cell::sync::Lazy; use reqwest::StatusCode; +use reqwest::header::HeaderMap; use serde::Deserialize; use serde::Serialize; #[cfg(test)] use serial_test::serial; +use std::collections::BTreeMap; use std::env; use std::fmt::Debug; use std::path::Path; @@ -83,6 +86,10 @@ const REFRESH_TOKEN_UNKNOWN_MESSAGE: &str = const REFRESH_TOKEN_ACCOUNT_MISMATCH_MESSAGE: &str = "Your access token could not be refreshed because you have since logged out or signed in to another account. Please sign in again."; const REFRESH_TOKEN_URL: &str = "https://auth.openai.com/oauth/token"; pub const REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR: &str = "CODEX_REFRESH_TOKEN_URL_OVERRIDE"; +const SENTRY_AUTH_FAILURES_TARGET: &str = "sentry_auth_failures"; +static AUTH_FAILURE_REPORTER: Lazy< + RwLock) + Send + Sync + 'static>>>, +> = Lazy::new(|| RwLock::new(None)); #[derive(Debug, Error)] pub enum RefreshTokenError { @@ -92,6 +99,25 @@ pub enum RefreshTokenError { Transient(#[from] std::io::Error), } +pub struct AuthFailureReporterGuard; + +impl Drop for AuthFailureReporterGuard { + fn drop(&mut self) { + if let Ok(mut reporter) = AUTH_FAILURE_REPORTER.write() { + *reporter = None; + } + } +} + +pub fn set_auth_failure_reporter( + reporter: Arc) + Send + Sync + 'static>, +) -> AuthFailureReporterGuard { + if let Ok(mut current) = AUTH_FAILURE_REPORTER.write() { + *current = Some(reporter); + } + AuthFailureReporterGuard +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct ExternalAuthTokens { pub access_token: String, @@ -681,9 +707,11 @@ async fn request_chatgpt_token_refresh( .map_err(|err| RefreshTokenError::Transient(std::io::Error::other(err)))?; Ok(refresh_response) } else { + let headers = response.headers().clone(); let body = response.text().await.unwrap_or_default(); tracing::error!("Failed to refresh token: {status}: {body}"); if status == StatusCode::UNAUTHORIZED { + emit_sentry_auth_failure_event_for_refresh_failure(&headers, &body); let failed = classify_refresh_token_failure(&body); Err(RefreshTokenError::Permanent(failed)) } else { @@ -695,6 +723,62 @@ async fn request_chatgpt_token_refresh( } } +fn emit_sentry_auth_failure_event_for_refresh_failure(headers: &HeaderMap, body: &str) { + let fields = refresh_failure_sentry_auth_fields(headers, body); + if let Ok(reporter) = AUTH_FAILURE_REPORTER.read() + && let Some(reporter) = reporter.as_ref() + { + reporter(fields); + return; + } + + tracing::event!( + target: SENTRY_AUTH_FAILURES_TARGET, + tracing::Level::ERROR, + report_kind = fields.get("report_kind").map(String::as_str).unwrap_or(""), + endpoint = fields.get("endpoint").map(String::as_str).unwrap_or(""), + auth_header_attached = fields + .get("auth_header_attached") + .is_some_and(|value| value == "true"), + auth_header_name = fields.get("auth_header_name").map(String::as_str).unwrap_or(""), + auth_mode = fields.get("auth_mode").map(String::as_str).unwrap_or(""), + auth_request_id = fields.get("auth_request_id").map(String::as_str).unwrap_or(""), + auth_cf_ray = fields.get("auth_cf_ray").map(String::as_str).unwrap_or(""), + auth_error = fields.get("auth_error").map(String::as_str).unwrap_or(""), + auth_error_code = fields.get("auth_error_code").map(String::as_str).unwrap_or(""), + cli_version = fields.get("cli_version").map(String::as_str).unwrap_or(""), + ); +} + +fn refresh_failure_sentry_auth_fields(headers: &HeaderMap, body: &str) -> BTreeMap { + let auth_request_id = headers + .get("x-request-id") + .and_then(|value| value.to_str().ok()) + .unwrap_or(""); + let auth_cf_ray = headers + .get("cf-ray") + .and_then(|value| value.to_str().ok()) + .unwrap_or(""); + let auth_error_code = extract_refresh_token_error_code(body).unwrap_or_default(); + let auth_error = try_parse_error_message(body); + + BTreeMap::from([ + ("report_kind".to_string(), "auth_failure_auto".to_string()), + ("endpoint".to_string(), "/oauth/token".to_string()), + ("auth_header_attached".to_string(), "true".to_string()), + ("auth_header_name".to_string(), "authorization".to_string()), + ("auth_mode".to_string(), "Chatgpt".to_string()), + ("auth_request_id".to_string(), auth_request_id.to_string()), + ("auth_cf_ray".to_string(), auth_cf_ray.to_string()), + ("auth_error".to_string(), auth_error), + ("auth_error_code".to_string(), auth_error_code), + ( + "cli_version".to_string(), + env!("CARGO_PKG_VERSION").to_string(), + ), + ]) +} + fn classify_refresh_token_failure(body: &str) -> RefreshTokenFailedError { let code = extract_refresh_token_error_code(body); diff --git a/codex-rs/login/src/lib.rs b/codex-rs/login/src/lib.rs index a8033cf888..fdd1b48434 100644 --- a/codex-rs/login/src/lib.rs +++ b/codex-rs/login/src/lib.rs @@ -18,6 +18,7 @@ pub use server::run_login_server; pub use auth::AuthConfig; pub use auth::AuthCredentialsStoreMode; pub use auth::AuthDotJson; +pub use auth::AuthFailureReporterGuard; pub use auth::AuthManager; pub use auth::CLIENT_ID; pub use auth::CODEX_API_KEY_ENV_VAR; @@ -38,5 +39,6 @@ pub use auth::login_with_api_key; pub use auth::logout; pub use auth::read_openai_api_key_from_env; pub use auth::save_auth; +pub use auth::set_auth_failure_reporter; pub use codex_app_server_protocol::AuthMode; pub use token_data::TokenData; diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 02b743fa8a..e9e4422dd9 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -842,6 +842,7 @@ pub async fn run_main( let feedback = codex_feedback::CodexFeedback::new(); let feedback_layer = feedback.logger_layer(); let feedback_metadata_layer = feedback.metadata_layer(); + let feedback_auth_event_layer = feedback.auth_event_layer(); if cli.oss && model_provider_override.is_some() { // We're in the oss section, so provider_id should be Some @@ -895,6 +896,7 @@ pub async fn run_main( .with(file_layer) .with(feedback_layer) .with(feedback_metadata_layer) + .with(feedback_auth_event_layer) .with(log_db_layer) .with(otel_logger_layer) .with(otel_tracing_layer)