mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
[codex-core] skip refresh for revoked access tokens [ci changed_files]
This commit is contained in:
@@ -150,6 +150,8 @@ const RESPONSES_COMPACT_ENDPOINT: &str = "/responses/compact";
|
||||
// period between stream events.
|
||||
const COMPACT_REQUEST_TIMEOUT_IDLE_MULTIPLIER: u32 = 4;
|
||||
const MEMORIES_SUMMARIZE_ENDPOINT: &str = "/memories/trace_summarize";
|
||||
const TOKEN_INVALIDATED_ERROR_CODE: &str = "token_invalidated";
|
||||
const TOKEN_REVOKED_ERROR_CODE: &str = "token_revoked";
|
||||
#[cfg(test)]
|
||||
pub(crate) const WEBSOCKET_CONNECT_TIMEOUT: Duration =
|
||||
Duration::from_millis(DEFAULT_WEBSOCKET_CONNECT_TIMEOUT_MS);
|
||||
@@ -1979,7 +1981,12 @@ async fn handle_unauthorized(
|
||||
session_telemetry: &SessionTelemetry,
|
||||
) -> Result<UnauthorizedRecoveryExecution> {
|
||||
let debug = extract_response_debug_context(&transport);
|
||||
if let Some(recovery) = auth_recovery
|
||||
let revocation_recovery_reason = access_token_revocation_recovery_reason(
|
||||
debug.auth_error_code.as_deref(),
|
||||
debug.auth_error_type.as_deref(),
|
||||
);
|
||||
if revocation_recovery_reason.is_none()
|
||||
&& let Some(recovery) = auth_recovery
|
||||
&& recovery.has_next()
|
||||
{
|
||||
let mode = recovery.mode_name();
|
||||
@@ -2057,13 +2064,18 @@ async fn handle_unauthorized(
|
||||
};
|
||||
}
|
||||
|
||||
let (mode, phase, recovery_reason) = match auth_recovery.as_ref() {
|
||||
Some(recovery) => (
|
||||
let (mode, phase, recovery_reason) = match (auth_recovery.as_ref(), revocation_recovery_reason)
|
||||
{
|
||||
(Some(recovery), Some(reason)) => {
|
||||
(recovery.mode_name(), recovery.step_name(), Some(reason))
|
||||
}
|
||||
(None, Some(reason)) => ("none", "none", Some(reason)),
|
||||
(Some(recovery), None) => (
|
||||
recovery.mode_name(),
|
||||
recovery.step_name(),
|
||||
Some(recovery.unavailable_reason()),
|
||||
),
|
||||
None => ("none", "none", Some("auth_manager_missing")),
|
||||
(None, None) => ("none", "none", Some("auth_manager_missing")),
|
||||
};
|
||||
session_telemetry.record_auth_recovery(
|
||||
mode,
|
||||
@@ -2089,6 +2101,21 @@ async fn handle_unauthorized(
|
||||
Err(map_api_error(ApiError::Transport(transport)))
|
||||
}
|
||||
|
||||
fn access_token_revocation_recovery_reason(
|
||||
auth_error_code: Option<&str>,
|
||||
auth_error_type: Option<&str>,
|
||||
) -> Option<&'static str> {
|
||||
match (auth_error_code, auth_error_type) {
|
||||
(Some(TOKEN_INVALIDATED_ERROR_CODE), _) | (_, Some(TOKEN_INVALIDATED_ERROR_CODE)) => {
|
||||
Some("access_token_invalidated")
|
||||
}
|
||||
(Some(TOKEN_REVOKED_ERROR_CODE), _) | (_, Some(TOKEN_REVOKED_ERROR_CODE)) => {
|
||||
Some("access_token_revoked")
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn api_error_http_status(error: &ApiError) -> Option<u16> {
|
||||
match error {
|
||||
ApiError::Transport(TransportError::Http { status, .. }) => Some(status.as_u16()),
|
||||
|
||||
@@ -7,11 +7,14 @@ use super::X_CODEX_PARENT_THREAD_ID_HEADER;
|
||||
use super::X_CODEX_TURN_METADATA_HEADER;
|
||||
use super::X_CODEX_WINDOW_ID_HEADER;
|
||||
use super::X_OPENAI_SUBAGENT_HEADER;
|
||||
use super::handle_unauthorized;
|
||||
use crate::AttestationContext;
|
||||
use crate::AttestationProvider;
|
||||
use crate::GenerateAttestationFuture;
|
||||
use base64::Engine;
|
||||
use codex_api::ApiError;
|
||||
use codex_api::ResponseEvent;
|
||||
use codex_api::TransportError;
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
use codex_login::AuthManager;
|
||||
use codex_login::CodexAuth;
|
||||
@@ -23,6 +26,7 @@ use codex_model_provider_info::create_oss_provider_with_base_url;
|
||||
use codex_otel::SessionTelemetry;
|
||||
use codex_protocol::SessionId;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::error::CodexErr;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
@@ -37,6 +41,9 @@ use codex_rollout_trace::RolloutTrace;
|
||||
use codex_rollout_trace::TraceWriter;
|
||||
use codex_rollout_trace::replay_bundle;
|
||||
use futures::StreamExt;
|
||||
use http::HeaderMap;
|
||||
use http::HeaderValue;
|
||||
use http::StatusCode;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::collections::BTreeMap;
|
||||
@@ -581,3 +588,76 @@ async fn non_chatgpt_codex_endpoints_omit_attestation_generation() {
|
||||
);
|
||||
assert_eq!(attestation_calls.load(Ordering::Relaxed), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn token_invalidated_401_skips_unauthorized_recovery() {
|
||||
let manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing());
|
||||
let mut recovery = Some(manager.unauthorized_recovery());
|
||||
let x_error_json = base64::engine::general_purpose::STANDARD
|
||||
.encode(r#"{"error":{"code":"token_invalidated"}}"#);
|
||||
let mut headers = HeaderMap::new();
|
||||
headers.insert(
|
||||
"x-error-json",
|
||||
HeaderValue::from_str(&x_error_json).expect("valid x-error-json header"),
|
||||
);
|
||||
|
||||
let err = handle_unauthorized(
|
||||
TransportError::Http {
|
||||
status: StatusCode::UNAUTHORIZED,
|
||||
url: Some("https://chatgpt.com/backend-api/codex/responses".to_string()),
|
||||
headers: Some(headers),
|
||||
body: Some(r#"{"error":{"message":"revoked"}}"#.to_string()),
|
||||
},
|
||||
&mut recovery,
|
||||
&test_session_telemetry(),
|
||||
)
|
||||
.await
|
||||
.expect_err("revoked access tokens should not enter refresh recovery");
|
||||
|
||||
let CodexErr::UnexpectedStatus(unexpected) = err else {
|
||||
panic!("expected unauthorized response to bubble up, got {err:?}");
|
||||
};
|
||||
assert_eq!(
|
||||
unexpected.identity_error_code.as_deref(),
|
||||
Some("token_invalidated")
|
||||
);
|
||||
assert_eq!(
|
||||
recovery
|
||||
.as_ref()
|
||||
.expect("recovery state should remain available")
|
||||
.step_name(),
|
||||
"reload"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn token_invalidated_error_type_401_skips_unauthorized_recovery() {
|
||||
let manager =
|
||||
AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing());
|
||||
let mut recovery = Some(manager.unauthorized_recovery());
|
||||
|
||||
let err = handle_unauthorized(
|
||||
TransportError::Http {
|
||||
status: StatusCode::UNAUTHORIZED,
|
||||
url: Some("https://chatgpt.com/backend-api/codex/responses".to_string()),
|
||||
headers: None,
|
||||
body: Some(r#"{"error":{"type":"token_invalidated"}}"#.to_string()),
|
||||
},
|
||||
&mut recovery,
|
||||
&test_session_telemetry(),
|
||||
)
|
||||
.await
|
||||
.expect_err("invalidated access tokens should not enter refresh recovery");
|
||||
|
||||
let CodexErr::UnexpectedStatus(_) = err else {
|
||||
panic!("expected unauthorized response to bubble up, got {err:?}");
|
||||
};
|
||||
assert_eq!(
|
||||
recovery
|
||||
.as_ref()
|
||||
.expect("recovery state should remain available")
|
||||
.step_name(),
|
||||
"reload"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1064,7 +1064,8 @@ enum UnauthorizedRecoveryMode {
|
||||
}
|
||||
|
||||
// UnauthorizedRecovery is a state machine that handles an attempt to refresh the authentication when requests
|
||||
// to API fail with 401 status code.
|
||||
// to API fail with a refreshable 401 status code. Callers must leave explicit access-token revocation signals
|
||||
// outside this flow.
|
||||
// The client calls next() every time it encounters a 401 error, one time per retry.
|
||||
// For API key based authentication, we don't do anything and let the error bubble to the user.
|
||||
//
|
||||
|
||||
@@ -14,15 +14,13 @@ pub struct ResponseDebugContext {
|
||||
pub cf_ray: Option<String>,
|
||||
pub auth_error: Option<String>,
|
||||
pub auth_error_code: Option<String>,
|
||||
pub auth_error_type: Option<String>,
|
||||
}
|
||||
|
||||
pub fn extract_response_debug_context(transport: &TransportError) -> ResponseDebugContext {
|
||||
let mut context = ResponseDebugContext::default();
|
||||
|
||||
let TransportError::Http {
|
||||
headers, body: _, ..
|
||||
} = transport
|
||||
else {
|
||||
let TransportError::Http { headers, body, .. } = transport else {
|
||||
return context;
|
||||
};
|
||||
|
||||
@@ -38,21 +36,46 @@ pub fn extract_response_debug_context(transport: &TransportError) -> ResponseDeb
|
||||
extract_header(REQUEST_ID_HEADER).or_else(|| extract_header(OAI_REQUEST_ID_HEADER));
|
||||
context.cf_ray = extract_header(CF_RAY_HEADER);
|
||||
context.auth_error = extract_header(AUTH_ERROR_HEADER);
|
||||
context.auth_error_code = extract_header(X_ERROR_JSON_HEADER).and_then(|encoded| {
|
||||
let decoded = base64::engine::general_purpose::STANDARD
|
||||
.decode(encoded)
|
||||
.ok()?;
|
||||
let parsed = serde_json::from_slice::<serde_json::Value>(&decoded).ok()?;
|
||||
parsed
|
||||
.get("error")
|
||||
.and_then(|error| error.get("code"))
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(str::to_string)
|
||||
});
|
||||
let header_error = extract_header(X_ERROR_JSON_HEADER)
|
||||
.and_then(|encoded| parse_x_error_json(encoded.as_str()));
|
||||
let body_error = body
|
||||
.as_deref()
|
||||
.and_then(|body| serde_json::from_str::<serde_json::Value>(body).ok());
|
||||
context.auth_error_code = header_error
|
||||
.as_ref()
|
||||
.and_then(|error| extract_error_field(error, "code"))
|
||||
.or_else(|| {
|
||||
body_error
|
||||
.as_ref()
|
||||
.and_then(|error| extract_error_field(error, "code"))
|
||||
});
|
||||
context.auth_error_type = header_error
|
||||
.as_ref()
|
||||
.and_then(|error| extract_error_field(error, "type"))
|
||||
.or_else(|| {
|
||||
body_error
|
||||
.as_ref()
|
||||
.and_then(|error| extract_error_field(error, "type"))
|
||||
});
|
||||
|
||||
context
|
||||
}
|
||||
|
||||
fn parse_x_error_json(encoded: &str) -> Option<serde_json::Value> {
|
||||
let decoded = base64::engine::general_purpose::STANDARD
|
||||
.decode(encoded)
|
||||
.ok()?;
|
||||
serde_json::from_slice::<serde_json::Value>(&decoded).ok()
|
||||
}
|
||||
|
||||
fn extract_error_field(error: &serde_json::Value, field: &str) -> Option<String> {
|
||||
error
|
||||
.get("error")
|
||||
.and_then(|error| error.get(field))
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(str::to_string)
|
||||
}
|
||||
|
||||
pub fn extract_response_debug_context_from_api_error(error: &ApiError) -> ResponseDebugContext {
|
||||
match error {
|
||||
ApiError::Transport(transport) => extract_response_debug_context(transport),
|
||||
@@ -127,6 +150,43 @@ mod tests {
|
||||
cf_ray: Some("ray-auth".to_string()),
|
||||
auth_error: Some("missing_authorization_header".to_string()),
|
||||
auth_error_code: Some("token_expired".to_string()),
|
||||
auth_error_type: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_response_debug_context_reads_identity_error_code_from_body() {
|
||||
let context = extract_response_debug_context(&TransportError::Http {
|
||||
status: StatusCode::UNAUTHORIZED,
|
||||
url: Some("https://chatgpt.com/backend-api/codex/models".to_string()),
|
||||
headers: None,
|
||||
body: Some(r#"{"error":{"code":"token_revoked"}}"#.to_string()),
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
context,
|
||||
ResponseDebugContext {
|
||||
auth_error_code: Some("token_revoked".to_string()),
|
||||
..ResponseDebugContext::default()
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_response_debug_context_reads_identity_error_type_from_body() {
|
||||
let context = extract_response_debug_context(&TransportError::Http {
|
||||
status: StatusCode::UNAUTHORIZED,
|
||||
url: Some("https://chatgpt.com/backend-api/codex/models".to_string()),
|
||||
headers: None,
|
||||
body: Some(r#"{"error":{"type":"token_invalidated"}}"#.to_string()),
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
context,
|
||||
ResponseDebugContext {
|
||||
auth_error_type: Some("token_invalidated".to_string()),
|
||||
..ResponseDebugContext::default()
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user