mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Prevent custom providers from inheriting ambient auth (#39214)
## Why Custom model providers that do not require OpenAI authentication could inherit ambient authentication headers, including the bearer token and `ChatGPT-Account-ID`, and send them to the custom provider endpoint. ## What changed - Resolve providers without `requires_openai_auth` or configured `auth` as unauthenticated. - Continue to honor an explicit `experimental_bearer_token` or command-resolved provider authentication. - Preserve ambient authentication for OpenAI providers. ## Testing Added unit and request-level coverage for custom, explicitly authenticated, and OpenAI provider authentication behavior. GitOrigin-RevId: d89a79604338bc660a3edb2af0068cfa9b694fa2
This commit is contained in:
@@ -3,6 +3,8 @@ use codex_login::CodexAuth;
|
||||
use codex_login::ExternalAuth;
|
||||
use codex_login::ExternalAuthFuture;
|
||||
use codex_login::ExternalAuthRefreshContext;
|
||||
use codex_model_provider_info::WireApi;
|
||||
use codex_model_provider_info::create_oss_provider_with_base_url;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_sse_once;
|
||||
@@ -75,6 +77,10 @@ async fn header_auth_is_attached_to_responses_requests() -> anyhow::Result<()> {
|
||||
.await;
|
||||
let mut headers = HeaderMap::new();
|
||||
headers.insert(AUTHORIZATION, HeaderValue::from_static("Bearer external"));
|
||||
headers.insert(
|
||||
"ChatGPT-Account-ID",
|
||||
HeaderValue::from_static("account-123"),
|
||||
);
|
||||
headers.insert("x-external-auth", HeaderValue::from_static("enabled"));
|
||||
let mut builder = test_codex().with_auth(CodexAuth::Headers(AuthHeaders::new(headers)));
|
||||
let test = builder.build_with_auto_env(&server).await?;
|
||||
@@ -90,6 +96,82 @@ async fn header_auth_is_attached_to_responses_requests() -> anyhow::Result<()> {
|
||||
request.header("x-external-auth").as_deref(),
|
||||
Some("enabled")
|
||||
);
|
||||
assert_eq!(
|
||||
request.header("chatgpt-account-id").as_deref(),
|
||||
Some("account-123")
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn custom_provider_does_not_receive_ambient_auth_headers() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let response_mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
)
|
||||
.await;
|
||||
let mut headers = HeaderMap::new();
|
||||
headers.insert(AUTHORIZATION, HeaderValue::from_static("Bearer ambient"));
|
||||
headers.insert(
|
||||
"ChatGPT-Account-ID",
|
||||
HeaderValue::from_static("account-123"),
|
||||
);
|
||||
let provider =
|
||||
create_oss_provider_with_base_url(&format!("{}/v1", server.uri()), WireApi::Responses);
|
||||
let mut builder = test_codex()
|
||||
.with_auth(CodexAuth::Headers(AuthHeaders::new(headers)))
|
||||
.with_config(move |config| {
|
||||
config.model_provider_id = provider.name.clone();
|
||||
config.model_provider = provider;
|
||||
});
|
||||
let test = builder.build_with_auto_env(&server).await?;
|
||||
|
||||
test.submit_turn("hello").await?;
|
||||
|
||||
let request = response_mock.single_request();
|
||||
assert_eq!(request.header("authorization"), None);
|
||||
assert_eq!(request.header("chatgpt-account-id"), None);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn custom_provider_uses_explicit_bearer_without_ambient_account() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let response_mock = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
)
|
||||
.await;
|
||||
let mut headers = HeaderMap::new();
|
||||
headers.insert(AUTHORIZATION, HeaderValue::from_static("Bearer ambient"));
|
||||
headers.insert(
|
||||
"ChatGPT-Account-ID",
|
||||
HeaderValue::from_static("account-123"),
|
||||
);
|
||||
let mut provider =
|
||||
create_oss_provider_with_base_url(&format!("{}/v1", server.uri()), WireApi::Responses);
|
||||
provider.experimental_bearer_token = Some("provider-token".to_string());
|
||||
let mut builder = test_codex()
|
||||
.with_auth(CodexAuth::Headers(AuthHeaders::new(headers)))
|
||||
.with_config(move |config| {
|
||||
config.model_provider_id = provider.name.clone();
|
||||
config.model_provider = provider;
|
||||
});
|
||||
let test = builder.build_with_auto_env(&server).await?;
|
||||
|
||||
test.submit_turn("hello").await?;
|
||||
|
||||
let request = response_mock.single_request();
|
||||
assert_eq!(
|
||||
request.header("authorization").as_deref(),
|
||||
Some("Bearer provider-token")
|
||||
);
|
||||
assert_eq!(request.header("chatgpt-account-id"), None);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -198,16 +198,20 @@ pub(crate) fn resolve_provider_auth(
|
||||
auth: Option<&CodexAuth>,
|
||||
provider: &ModelProviderInfo,
|
||||
) -> codex_protocol::error::Result<SharedAuthProvider> {
|
||||
if let Some(auth) = bearer_auth_for_provider(provider)? {
|
||||
return Ok(Arc::new(auth));
|
||||
}
|
||||
|
||||
if !provider.requires_openai_auth && provider.auth.is_none() {
|
||||
return Ok(unauthenticated_auth_provider());
|
||||
}
|
||||
|
||||
if matches!(auth, Some(CodexAuth::BedrockApiKey(_))) {
|
||||
return Err(CodexErr::UnsupportedOperation(
|
||||
BEDROCK_API_KEY_UNSUPPORTED_MESSAGE.to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
if let Some(auth) = bearer_auth_for_provider(provider)? {
|
||||
return Ok(Arc::new(auth));
|
||||
}
|
||||
|
||||
Ok(match auth {
|
||||
Some(auth) => auth_provider_from_auth(auth),
|
||||
None => unauthenticated_auth_provider(),
|
||||
@@ -341,10 +345,12 @@ mod tests {
|
||||
use codex_model_provider_info::WireApi;
|
||||
use codex_model_provider_info::create_oss_provider_with_base_url;
|
||||
use codex_protocol::account::PlanType;
|
||||
use codex_protocol::config_types::ModelProviderAuthInfo;
|
||||
use codex_protocol::error::CodexErrorDetails;
|
||||
use http::header::AUTHORIZATION;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::num::NonZeroU64;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
@@ -466,6 +472,109 @@ mod tests {
|
||||
assert!(auth.to_auth_headers().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn custom_provider_does_not_inherit_ambient_auth_headers() {
|
||||
let provider =
|
||||
create_oss_provider_with_base_url("http://localhost:11434/v1", WireApi::Responses);
|
||||
let mut ambient_headers = HeaderMap::new();
|
||||
ambient_headers.insert(
|
||||
AUTHORIZATION,
|
||||
HeaderValue::from_static("Bearer ambient-token"),
|
||||
);
|
||||
ambient_headers.insert(
|
||||
"ChatGPT-Account-ID",
|
||||
HeaderValue::from_static("account-123"),
|
||||
);
|
||||
let ambient_auth = CodexAuth::Headers(AuthHeaders::new(ambient_headers));
|
||||
|
||||
let auth =
|
||||
resolve_provider_auth(Some(&ambient_auth), &provider).expect("auth should resolve");
|
||||
|
||||
assert!(auth.to_auth_headers().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn custom_provider_does_not_inherit_ambient_bedrock_auth() {
|
||||
let provider =
|
||||
create_oss_provider_with_base_url("http://localhost:11434/v1", WireApi::Responses);
|
||||
let ambient_auth = CodexAuth::BedrockApiKey(BedrockApiKeyAuth {
|
||||
api_key: "bedrock-api-key-test".to_string(),
|
||||
region: "us-east-1".to_string(),
|
||||
});
|
||||
|
||||
let auth =
|
||||
resolve_provider_auth(Some(&ambient_auth), &provider).expect("auth should resolve");
|
||||
|
||||
assert!(auth.to_auth_headers().is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn custom_provider_uses_explicit_bearer_instead_of_ambient_auth() {
|
||||
let mut provider =
|
||||
create_oss_provider_with_base_url("http://localhost:11434/v1", WireApi::Responses);
|
||||
provider.experimental_bearer_token = Some("provider-token".to_string());
|
||||
let ambient_auth = CodexAuth::BedrockApiKey(BedrockApiKeyAuth {
|
||||
api_key: "bedrock-api-key-test".to_string(),
|
||||
region: "us-east-1".to_string(),
|
||||
});
|
||||
|
||||
let headers = resolve_provider_auth(Some(&ambient_auth), &provider)
|
||||
.expect("auth should resolve")
|
||||
.to_auth_headers();
|
||||
|
||||
assert_eq!(
|
||||
headers.get(AUTHORIZATION),
|
||||
Some(&HeaderValue::from_static("Bearer provider-token"))
|
||||
);
|
||||
assert_eq!(headers.len(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn custom_provider_uses_command_resolved_auth() {
|
||||
let mut provider =
|
||||
create_oss_provider_with_base_url("http://localhost:11434/v1", WireApi::Responses);
|
||||
provider.auth = Some(ModelProviderAuthInfo {
|
||||
command: "print-token".to_string(),
|
||||
args: Vec::new(),
|
||||
timeout_ms: NonZeroU64::new(5_000).expect("timeout should be non-zero"),
|
||||
refresh_interval_ms: 300_000,
|
||||
cwd: std::env::current_dir()
|
||||
.expect("current directory should be available")
|
||||
.try_into()
|
||||
.expect("current directory should be absolute"),
|
||||
});
|
||||
let command_auth = CodexAuth::from_api_key("command-token");
|
||||
|
||||
let headers = resolve_provider_auth(Some(&command_auth), &provider)
|
||||
.expect("auth should resolve")
|
||||
.to_auth_headers();
|
||||
|
||||
assert_eq!(
|
||||
headers.get(AUTHORIZATION),
|
||||
Some(&HeaderValue::from_static("Bearer command-token"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn openai_provider_preserves_ambient_auth_headers() {
|
||||
let provider = ModelProviderInfo::create_openai_provider(/*base_url*/ None);
|
||||
let mut expected = HeaderMap::new();
|
||||
expected.insert(
|
||||
AUTHORIZATION,
|
||||
HeaderValue::from_static("Bearer ambient-token"),
|
||||
);
|
||||
expected.insert(
|
||||
"ChatGPT-Account-ID",
|
||||
HeaderValue::from_static("account-123"),
|
||||
);
|
||||
let ambient_auth = CodexAuth::Headers(AuthHeaders::new(expected.clone()));
|
||||
|
||||
let auth =
|
||||
resolve_provider_auth(Some(&ambient_auth), &provider).expect("auth should resolve");
|
||||
|
||||
assert_eq!(auth.to_auth_headers(), expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn header_auth_adds_predefined_headers() {
|
||||
let mut expected = HeaderMap::new();
|
||||
|
||||
Reference in New Issue
Block a user