Skip to content

[codex] Tighten cloud task backend handling #22213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
bookholt-oai wants to merge 1 commit into
base: main
Choose a base branch
from
+100 −28
Draft

[codex] Tighten cloud task backend handling #22213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions codex-rs/cloud-tasks/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,12 @@ async fn init_backend(user_agent_suffix: &str) -> anyhow::Result<BackendContext>
std::process::exit(1);
}

let auth_provider = codex_model_provider::auth_provider_from_auth(&auth);
http = http.with_auth_provider(auth_provider);
if let Some(acc) = auth.get_account_id() {
append_error_log(format!("auth: set ChatGPT-Account-Id header: {acc}"));
if util::should_attach_chatgpt_auth(&base_url) {
let auth_provider = codex_model_provider::auth_provider_from_auth(&auth);
http = http.with_auth_provider(auth_provider);
if let Some(acc) = auth.get_account_id() {
append_error_log(format!("auth: set ChatGPT-Account-Id header: {acc}"));
}
}

Ok(BackendContext {
Expand Down Expand Up @@ -185,7 +187,7 @@ async fn resolve_environment_id(ctx: &BackendContext, requested: &str) -> anyhow
return Err(anyhow!("environment id must not be empty"));
}
let normalized = util::normalize_base_url(&ctx.base_url);
let headers = util::build_chatgpt_headers().await;
let headers = util::build_chatgpt_headers(&normalized).await;
let environments = crate::env_detect::list_environments(&normalized, &headers).await?;
if environments.is_empty() {
return Err(anyhow!(
Expand Down Expand Up @@ -841,7 +843,7 @@ pub async fn run_main(cli: Cli, _codex_linux_sandbox_exe: Option<PathBuf>) -> an
&std::env::var("CODEX_CLOUD_TASKS_BASE_URL")
.unwrap_or_else(|_| "https://chatgpt.com/backend-api".to_string()),
);
let headers = util::build_chatgpt_headers().await;
let headers = util::build_chatgpt_headers(&base_url).await;
let res = crate::env_detect::list_environments(&base_url, &headers).await;
let _ = tx.send(app::AppEvent::EnvironmentsLoaded(res));
});
Expand All @@ -857,7 +859,7 @@ pub async fn run_main(cli: Cli, _codex_linux_sandbox_exe: Option<PathBuf>) -> an
.unwrap_or_else(|_| "https://chatgpt.com/backend-api".to_string()),
);
// Build headers: UA + ChatGPT auth if available
let headers = util::build_chatgpt_headers().await;
let headers = util::build_chatgpt_headers(&base_url).await;

// Run autodetect. If it fails, we keep using "All".
let res = crate::env_detect::autodetect_environment_id(
Expand Down Expand Up @@ -1082,7 +1084,8 @@ pub async fn run_main(cli: Cli, _codex_linux_sandbox_exe: Option<PathBuf>) -> an
&std::env::var("CODEX_CLOUD_TASKS_BASE_URL")
.unwrap_or_else(|_| "https://chatgpt.com/backend-api".to_string()),
);
let headers = crate::util::build_chatgpt_headers().await;
let headers =
crate::util::build_chatgpt_headers(&base_url).await;
let res = crate::env_detect::list_environments(&base_url, &headers).await;
let _ = tx.send(app::AppEvent::EnvironmentsLoaded(res));
});
Expand Down Expand Up @@ -1465,7 +1468,7 @@ pub async fn run_main(cli: Cli, _codex_linux_sandbox_exe: Option<PathBuf>) -> an
let tx = tx.clone();
tokio::spawn(async move {
let base_url = crate::util::normalize_base_url(&std::env::var("CODEX_CLOUD_TASKS_BASE_URL").unwrap_or_else(|_| "https://chatgpt.com/backend-api".to_string()));
let headers = crate::util::build_chatgpt_headers().await;
let headers = crate::util::build_chatgpt_headers(&base_url).await;
let res = crate::env_detect::list_environments(&base_url, &headers).await;
let _ = tx.send(app::AppEvent::EnvironmentsLoaded(res));
});
Expand Down Expand Up @@ -1654,7 +1657,8 @@ pub async fn run_main(cli: Cli, _codex_linux_sandbox_exe: Option<PathBuf>) -> an
&std::env::var("CODEX_CLOUD_TASKS_BASE_URL")
.unwrap_or_else(|_| "https://chatgpt.com/backend-api".to_string()),
);
let headers = crate::util::build_chatgpt_headers().await;
let headers =
crate::util::build_chatgpt_headers(&base_url).await;
let res = crate::env_detect::list_environments(&base_url, &headers).await;
let _ = tx.send(app::AppEvent::EnvironmentsLoaded(res));
});
Expand Down Expand Up @@ -1830,7 +1834,8 @@ pub async fn run_main(cli: Cli, _codex_linux_sandbox_exe: Option<PathBuf>) -> an
let tx = tx.clone();
tokio::spawn(async move {
let base_url = crate::util::normalize_base_url(&std::env::var("CODEX_CLOUD_TASKS_BASE_URL").unwrap_or_else(|_| "https://chatgpt.com/backend-api".to_string()));
let headers = crate::util::build_chatgpt_headers().await;
let headers =
crate::util::build_chatgpt_headers(&base_url).await;
let res = crate::env_detect::list_environments(&base_url, &headers).await;
let _ = tx.send(app::AppEvent::EnvironmentsLoaded(res));
});
Expand Down
59 changes: 53 additions & 6 deletions codex-rs/cloud-tasks/src/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use chrono::DateTime;
use chrono::Local;
use chrono::Utc;
use codex_client::is_allowed_chatgpt_url;
use reqwest::header::HeaderMap;

use codex_core::config::Config;
Expand Down Expand Up @@ -32,15 +33,18 @@ pub fn normalize_base_url(input: &str) -> String {
while base_url.ends_with('/') {
base_url.pop();
}
if (base_url.starts_with("https://chatgpt.com")
|| base_url.starts_with("https://chat.openai.com"))
&& !base_url.contains("/backend-api")
{
if should_attach_chatgpt_auth(&base_url) && !base_url.contains("/backend-api") {
base_url = format!("{base_url}/backend-api");
}
base_url
}

pub(crate) fn should_attach_chatgpt_auth(base_url: &str) -> bool {
reqwest::Url::parse(base_url)
.ok()
.is_some_and(|url| is_allowed_chatgpt_url(&url))
}

pub async fn load_auth_manager(chatgpt_base_url: Option<String>) -> Option<AuthManager> {
// TODO: pass in cli overrides once cloud tasks properly support them.
let config = Config::load_with_cli_overrides(Vec::new()).await.ok()?;
Expand All @@ -57,7 +61,7 @@ pub async fn load_auth_manager(chatgpt_base_url: Option<String>) -> Option<AuthM

/// Build headers for ChatGPT-backed requests: `User-Agent`, optional `Authorization`,
/// and optional `ChatGPT-Account-Id`.
pub async fn build_chatgpt_headers() -> HeaderMap {
pub async fn build_chatgpt_headers(base_url: &str) -> HeaderMap {
use reqwest::header::HeaderValue;
use reqwest::header::USER_AGENT;

Expand All @@ -68,7 +72,8 @@ pub async fn build_chatgpt_headers() -> HeaderMap {
USER_AGENT,
HeaderValue::from_str(&ua).unwrap_or(HeaderValue::from_static("codex-cli")),
);
if let Some(am) = load_auth_manager(/*chatgpt_base_url*/ None).await
if should_attach_chatgpt_auth(base_url)
&& let Some(am) = load_auth_manager(/*chatgpt_base_url*/ None).await
&& let Some(auth) = am.auth().await
&& auth.uses_codex_backend()
{
Expand Down Expand Up @@ -115,3 +120,45 @@ pub fn format_relative_time(reference: DateTime<Utc>, ts: DateTime<Utc>) -> Stri
pub fn format_relative_time_now(ts: DateTime<Utc>) -> String {
format_relative_time(Utc::now(), ts)
}

#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;

#[test]
fn normalize_base_url_only_rewrites_allowed_chatgpt_origins() {
assert_eq!(
normalize_base_url("https://chatgpt.com"),
"https://chatgpt.com/backend-api"
);
assert_eq!(
normalize_base_url("https://chat.openai.com/"),
"https://chat.openai.com/backend-api"
);
assert_eq!(
normalize_base_url("https://chatgpt.com.fromspeech.ai/"),
"https://chatgpt.com.fromspeech.ai"
);
assert_eq!(
normalize_base_url("http://chatgpt.com/"),
"http://chatgpt.com"
);
}

#[test]
fn allowed_chatgpt_base_urls_require_https_and_exact_first_party_hosts() {
assert!(should_attach_chatgpt_auth(
"https://chatgpt.com/backend-api"
));
assert!(should_attach_chatgpt_auth(
"https://foo.chatgpt.com/backend-api"
));
assert!(!should_attach_chatgpt_auth(
"https://chatgpt.com.fromspeech.ai/backend-api"
));
assert!(!should_attach_chatgpt_auth(
"http://chatgpt.com/backend-api"
));
}
}
13 changes: 2 additions & 11 deletions codex-rs/codex-client/src/chatgpt_cloudflare_cookies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use reqwest::cookie::CookieStore;
use reqwest::cookie::Jar;
use reqwest::header::HeaderValue;

use crate::chatgpt_hosts::is_allowed_chatgpt_host;
use crate::chatgpt_hosts::is_allowed_chatgpt_url;

// WARNING: this store is process-global and may be shared across auth contexts.
// It must only ever contain Cloudflare infrastructure cookies. Never extend this
Expand Down Expand Up @@ -56,16 +56,7 @@ pub fn with_chatgpt_cloudflare_cookie_store(
}

fn is_chatgpt_cookie_url(url: &reqwest::Url) -> bool {
match url.scheme() {
"https" => {}
_ => return false,
}

let Some(host) = url.host_str() else {
return false;
};

is_allowed_chatgpt_host(host)
is_allowed_chatgpt_url(url)
}

fn is_allowed_cloudflare_set_cookie_header(header: &HeaderValue) -> bool {
Expand Down
28 changes: 28 additions & 0 deletions codex-rs/codex-client/src/chatgpt_hosts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ pub fn is_allowed_chatgpt_host(host: &str) -> bool {
.any(|suffix| host.ends_with(suffix))
}

/// Returns whether `url` is an HTTPS URL targeting a first-party ChatGPT host.
pub fn is_allowed_chatgpt_url(url: &reqwest::Url) -> bool {
url.scheme() == "https" && url.host_str().is_some_and(is_allowed_chatgpt_host)
}

#[cfg(test)]
mod tests {
use super::*;
Expand All @@ -36,4 +41,27 @@ mod tests {
assert!(!is_allowed_chatgpt_host(host));
}
}

#[test]
fn recognizes_chatgpt_urls_without_origin_confusion() {
for url in [
"https://chatgpt.com/backend-api",
"https://foo.chatgpt.com/backend-api",
"https://chat.openai.com/backend-api",
"https://api.chatgpt-staging.com/backend-api",
] {
let parsed = reqwest::Url::parse(url).expect("test URL should parse");
assert!(is_allowed_chatgpt_url(&parsed));
}

for url in [
"http://chatgpt.com/backend-api",
"https://chatgpt.com.fromspeech.ai/backend-api",
"https://chat.openai.com.evil.example/backend-api",
"https://api.openai.com/v1/responses",
] {
let parsed = reqwest::Url::parse(url).expect("test URL should parse");
assert!(!is_allowed_chatgpt_url(&parsed));
}
}
}
1 change: 1 addition & 0 deletions codex-rs/codex-client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ mod transport;

pub use crate::chatgpt_cloudflare_cookies::with_chatgpt_cloudflare_cookie_store;
pub use crate::chatgpt_hosts::is_allowed_chatgpt_host;
pub use crate::chatgpt_hosts::is_allowed_chatgpt_url;
pub use crate::custom_ca::BuildCustomCaTransportError;
/// Test-only subprocess hook for custom CA coverage.
///
Expand Down
Loading