diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 911c873f5f..f6f21761a0 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2796,7 +2796,6 @@ dependencies = [ "libc", "pretty_assertions", "regex", - "reqwest 0.12.28", "semver", "serde", "serde_json", diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index b7c023eaa9..a56a5e376f 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1583,6 +1583,7 @@ impl PluginRequestProcessor { })?; let result = codex_core_plugins::remote_bundle::download_and_install_remote_plugin_bundle( + &remote_plugin_service_config, config.codex_home.to_path_buf(), validated_bundle, ) diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index 5b8d0e6cce..ade250db06 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -39,7 +39,6 @@ chrono = { workspace = true } dirs = { workspace = true } flate2 = { workspace = true } http = { workspace = true } -reqwest = { workspace = true } regex = { workspace = true } semver = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index c8595b6174..f5d3fc7663 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -14,8 +14,10 @@ use codex_app_server_protocol::SkillInterface; use codex_http_client::ClientRouteClass; use codex_http_client::HttpClientFactory; use codex_http_client::RouteAwareClientPool; +use codex_http_client::RouteAwareRequestBuilder; +use codex_http_client::RouteAwareRequestError; use codex_login::CodexAuth; -use codex_login::default_client::build_reqwest_client; +use codex_login::default_client::default_headers; use codex_plugin::AppConnectorId; use codex_plugin::AppDeclaration; use codex_plugin::PluginCapabilitySummary; @@ -23,7 +25,8 @@ use codex_plugin::PluginId; use codex_plugin::app_connector_ids_from_declarations; use codex_plugin::prompt_safe_plugin_description; use codex_utils_absolute_path::AbsolutePathBuf; -use reqwest::RequestBuilder; +use http::Method; +use http::StatusCode; use serde::Deserialize; use serde::Serialize; use serde_json::Value as JsonValue; @@ -147,6 +150,12 @@ impl RemotePluginServiceConfig { http_clients: Arc::new(http_clients), } } + + pub(crate) fn http_request(&self, method: Method, url: &str) -> RouteAwareRequestBuilder { + self.http_clients + .request(method, url) + .headers(default_headers()) + } } impl PartialEq for RemotePluginServiceConfig { @@ -158,7 +167,6 @@ impl PartialEq for RemotePluginServiceConfig { } impl Eq for RemotePluginServiceConfig {} - #[derive(Debug, Clone, PartialEq, Eq)] pub struct RemotePluginUninstallTarget { pub plugin_id: PluginId, @@ -348,13 +356,13 @@ pub enum RemotePluginCatalogError { Request { url: String, #[source] - source: reqwest::Error, + source: RouteAwareRequestError, }, #[error("remote plugin catalog request to {url} failed with status {status}: {body}")] UnexpectedStatus { url: String, - status: reqwest::StatusCode, + status: StatusCode, body: String, }, @@ -892,11 +900,12 @@ pub async fn fetch_recommended_plugins( ) -> Result { let auth = ensure_chatgpt_auth(auth)?; let base_url = config.chatgpt_base_url.trim_end_matches('/'); - let url = format!("{base_url}/ps/plugins/suggested"); - let client = build_reqwest_client(); - let request = authenticated_request(client.get(&url), auth)? - .timeout(RECOMMENDED_PLUGINS_TIMEOUT) - .query(&[("scope", "GLOBAL")]); + let mut url = Url::parse(&format!("{base_url}/ps/plugins/suggested")) + .map_err(RemotePluginCatalogError::InvalidBaseUrl)?; + url.query_pairs_mut().append_pair("scope", "GLOBAL"); + let url = url.to_string(); + let request = authenticated_request(config.http_request(Method::GET, &url), auth) + .timeout(RECOMMENDED_PLUGINS_TIMEOUT); let response: RecommendedPluginsResponse = send_and_decode(request, &url).await?; Ok(recommended_plugins_mode(response)) } @@ -1203,8 +1212,7 @@ pub async fn fetch_remote_plugin_skill_detail( } let url = remote_plugin_skill_detail_url(config, plugin_id, skill_name)?; - let client = build_reqwest_client(); - let request = authenticated_request(client.get(&url), auth)?; + let request = authenticated_request(config.http_request(Method::GET, &url), auth); let response: RemotePluginSkillDetailResponse = send_and_decode(request, &url).await?; if response.plugin_id != plugin_id { return Err(RemotePluginCatalogError::UnexpectedPluginId { @@ -1359,14 +1367,12 @@ pub async fn install_remote_plugin( // marketplace name is not validated before sending the install mutation. let base_url = config.chatgpt_base_url.trim_end_matches('/'); - let url = format!("{base_url}/ps/plugins/{plugin_id}/install"); - let client = build_reqwest_client(); - let request = authenticated_request( - client - .post(&url) - .query(&[("includeAppsNeedingAuth", "true")]), - auth, - )?; + let mut url = Url::parse(&format!("{base_url}/ps/plugins/{plugin_id}/install")) + .map_err(RemotePluginCatalogError::InvalidBaseUrl)?; + url.query_pairs_mut() + .append_pair("includeAppsNeedingAuth", "true"); + let url = url.to_string(); + let request = authenticated_request(config.http_request(Method::POST, &url), auth); let response: RemotePluginMutationResponse = send_and_decode(request, &url).await?; if response.id != plugin_id { return Err(RemotePluginCatalogError::UnexpectedPluginId { @@ -1453,8 +1459,7 @@ pub async fn uninstall_remote_plugin( let base_url = config.chatgpt_base_url.trim_end_matches('/'); let url = format!("{base_url}/ps/plugins/{remote_plugin_id}/uninstall"); - let client = build_reqwest_client(); - let request = authenticated_request(client.post(&url), auth)?; + let request = authenticated_request(config.http_request(Method::POST, &url), auth); let response: RemotePluginMutationResponse = send_and_decode(request, &url).await?; if response.id != remote_plugin_id { return Err(RemotePluginCatalogError::UnexpectedPluginId { @@ -1873,17 +1878,19 @@ async fn get_remote_plugin_list_page( collection: Option<&str>, ) -> Result { let base_url = config.chatgpt_base_url.trim_end_matches('/'); - let url = format!("{base_url}/ps/plugins/list"); - let client = build_reqwest_client(); - let mut request = authenticated_request(client.get(&url), auth)?; - request = request.query(&[("scope", scope.api_value())]); - request = request.query(&[("limit", REMOTE_PLUGIN_LIST_PAGE_LIMIT)]); + let mut url = Url::parse(&format!("{base_url}/ps/plugins/list")) + .map_err(RemotePluginCatalogError::InvalidBaseUrl)?; + url.query_pairs_mut() + .append_pair("scope", scope.api_value()) + .append_pair("limit", &REMOTE_PLUGIN_LIST_PAGE_LIMIT.to_string()); if let Some(collection) = collection { - request = request.query(&[("collection", collection)]); + url.query_pairs_mut().append_pair("collection", collection); } if let Some(page_token) = page_token { - request = request.query(&[("pageToken", page_token)]); + url.query_pairs_mut().append_pair("pageToken", page_token); } + let url = url.to_string(); + let request = authenticated_request(config.http_request(Method::GET, &url), auth); send_and_decode(request, &url).await } @@ -1893,13 +1900,15 @@ async fn get_remote_shared_workspace_plugins_page( page_token: Option<&str>, ) -> Result { let base_url = config.chatgpt_base_url.trim_end_matches('/'); - let url = format!("{base_url}/ps/plugins/workspace/shared"); - let client = build_reqwest_client(); - let mut request = authenticated_request(client.get(&url), auth)?; - request = request.query(&[("limit", REMOTE_PLUGIN_LIST_PAGE_LIMIT)]); + let mut url = Url::parse(&format!("{base_url}/ps/plugins/workspace/shared")) + .map_err(RemotePluginCatalogError::InvalidBaseUrl)?; + url.query_pairs_mut() + .append_pair("limit", &REMOTE_PLUGIN_LIST_PAGE_LIMIT.to_string()); if let Some(page_token) = page_token { - request = request.query(&[("pageToken", page_token)]); + url.query_pairs_mut().append_pair("pageToken", page_token); } + let url = url.to_string(); + let request = authenticated_request(config.http_request(Method::GET, &url), auth); send_and_decode(request, &url).await } @@ -1911,16 +1920,19 @@ async fn get_remote_plugin_installed_page( include_download_urls: bool, ) -> Result { let base_url = config.chatgpt_base_url.trim_end_matches('/'); - let url = format!("{base_url}/ps/plugins/installed"); - let client = build_reqwest_client(); - let mut request = authenticated_request(client.get(&url), auth)?; - request = request.query(&[("scope", scope.api_value())]); + let mut url = Url::parse(&format!("{base_url}/ps/plugins/installed")) + .map_err(RemotePluginCatalogError::InvalidBaseUrl)?; + url.query_pairs_mut() + .append_pair("scope", scope.api_value()); if include_download_urls { - request = request.query(&[("includeDownloadUrls", true)]); + url.query_pairs_mut() + .append_pair("includeDownloadUrls", "true"); } if let Some(page_token) = page_token { - request = request.query(&[("pageToken", page_token)]); + url.query_pairs_mut().append_pair("pageToken", page_token); } + let url = url.to_string(); + let request = authenticated_request(config.http_request(Method::GET, &url), auth); send_and_decode(request, &url).await } @@ -1931,12 +1943,14 @@ async fn fetch_plugin_detail( include_download_urls: bool, ) -> Result { let base_url = config.chatgpt_base_url.trim_end_matches('/'); - let url = format!("{base_url}/ps/plugins/{plugin_id}"); - let client = build_reqwest_client(); - let mut request = authenticated_request(client.get(&url), auth)?; + let mut url = Url::parse(&format!("{base_url}/ps/plugins/{plugin_id}")) + .map_err(RemotePluginCatalogError::InvalidBaseUrl)?; if include_download_urls { - request = request.query(&[("includeDownloadUrls", true)]); + url.query_pairs_mut() + .append_pair("includeDownloadUrls", "true"); } + let url = url.to_string(); + let request = authenticated_request(config.http_request(Method::GET, &url), auth); send_and_decode(request, &url).await } @@ -1972,17 +1986,17 @@ fn ensure_chatgpt_auth(auth: Option<&CodexAuth>) -> Result<&CodexAuth, RemotePlu } fn authenticated_request( - request: RequestBuilder, + request: RouteAwareRequestBuilder, auth: &CodexAuth, -) -> Result { - Ok(request +) -> RouteAwareRequestBuilder { + request .timeout(REMOTE_PLUGIN_CATALOG_TIMEOUT) .headers(codex_model_provider::auth_provider_from_auth(auth).to_auth_headers()) - .header(OAI_PRODUCT_SKU_HEADER, CODEX_PRODUCT_SKU)) + .header(OAI_PRODUCT_SKU_HEADER, CODEX_PRODUCT_SKU) } async fn send_and_decode Deserialize<'de>>( - request: RequestBuilder, + request: RouteAwareRequestBuilder, url: &str, ) -> Result { let response = request diff --git a/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs b/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs index e2a665beee..361707cfd5 100644 --- a/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs +++ b/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs @@ -257,6 +257,7 @@ pub async fn sync_remote_installed_plugin_bundles_once( }; match crate::remote_bundle::download_and_install_remote_plugin_bundle( + config, codex_home.clone(), bundle, ) diff --git a/codex-rs/core-plugins/src/remote/share.rs b/codex-rs/core-plugins/src/remote/share.rs index 44644c9137..5fba63458d 100644 --- a/codex-rs/core-plugins/src/remote/share.rs +++ b/codex-rs/core-plugins/src/remote/share.rs @@ -1,17 +1,18 @@ use super::*; use crate::plugin_bundle_archive::PluginBundlePackError; use crate::plugin_bundle_archive::pack_plugin_bundle_tar_gz; +use codex_http_client::RouteAwareRequestBuilder; use codex_login::CodexAuth; -use codex_login::default_client::build_reqwest_client; use codex_utils_absolute_path::AbsolutePathBuf; -use reqwest::RequestBuilder; -use reqwest::StatusCode; +use http::Method; +use http::StatusCode; use serde::Deserialize; use serde::Serialize; use std::collections::BTreeMap; use std::io; use std::path::Path; use tracing::warn; +use url::Url; mod checkout; mod local_paths; @@ -164,7 +165,7 @@ pub async fn save_remote_plugin_share( let etag = upload .etag .ok_or(RemotePluginCatalogError::MissingUploadEtag)?; - put_workspace_plugin_upload(&upload.upload_url, archive_bytes).await?; + put_workspace_plugin_upload(config, &upload.upload_url, archive_bytes).await?; let share_targets = access_policy.share_targets; let share_targets = ensure_unlisted_workspace_target(auth, access_policy.discoverability, share_targets)?; @@ -280,8 +281,7 @@ pub async fn delete_remote_plugin_share( let auth = ensure_chatgpt_auth(auth)?; let base_url = config.chatgpt_base_url.trim_end_matches('/'); let url = format!("{base_url}/public/plugins/workspace/{remote_plugin_id}"); - let client = build_reqwest_client(); - let request = authenticated_request(client.delete(&url), auth)?; + let request = authenticated_request(config.http_request(Method::DELETE, &url), auth); send_and_expect_status(request, &url, &[StatusCode::NO_CONTENT]).await?; if let Err(err) = local_paths::remove_plugin_share_local_path(codex_home, remote_plugin_id) { warn!( @@ -314,8 +314,7 @@ pub async fn update_remote_plugin_share_targets( .unwrap_or_default(); let base_url = config.chatgpt_base_url.trim_end_matches('/'); let url = format!("{base_url}/ps/plugins/{remote_plugin_id}/shares"); - let client = build_reqwest_client(); - let request = authenticated_request(client.put(&url), auth)?.json( + let request = authenticated_request(config.http_request(Method::PUT, &url), auth).json( &RemotePluginShareUpdateTargetsRequest { discoverability, targets, @@ -379,13 +378,15 @@ async fn get_created_workspace_plugins_page( page_token: Option<&str>, ) -> Result { let base_url = config.chatgpt_base_url.trim_end_matches('/'); - let url = format!("{base_url}/ps/plugins/workspace/created"); - let client = build_reqwest_client(); - let mut request = authenticated_request(client.get(&url), auth)?; - request = request.query(&[("limit", REMOTE_PLUGIN_LIST_PAGE_LIMIT)]); + let mut url = Url::parse(&format!("{base_url}/ps/plugins/workspace/created")) + .map_err(RemotePluginCatalogError::InvalidBaseUrl)?; + url.query_pairs_mut() + .append_pair("limit", &REMOTE_PLUGIN_LIST_PAGE_LIMIT.to_string()); if let Some(page_token) = page_token { - request = request.query(&[("pageToken", page_token)]); + url.query_pairs_mut().append_pair("pageToken", page_token); } + let url = url.to_string(); + let request = authenticated_request(config.http_request(Method::GET, &url), auth); send_and_decode(request, &url).await } @@ -398,8 +399,7 @@ async fn create_workspace_plugin_upload( ) -> Result { let base_url = config.chatgpt_base_url.trim_end_matches('/'); let url = format!("{base_url}/public/plugins/workspace/upload-url"); - let client = build_reqwest_client(); - let request = authenticated_request(client.post(&url), auth)?.json( + let request = authenticated_request(config.http_request(Method::POST, &url), auth).json( &RemoteWorkspacePluginUploadUrlRequest { filename, mime_type: "application/gzip", @@ -411,12 +411,12 @@ async fn create_workspace_plugin_upload( } async fn put_workspace_plugin_upload( + config: &RemotePluginServiceConfig, upload_url: &str, archive_bytes: Vec, ) -> Result<(), RemotePluginCatalogError> { - let client = build_reqwest_client(); - let request = client - .put(upload_url) + let request = config + .http_request(Method::PUT, upload_url) .timeout(REMOTE_PLUGIN_CATALOG_TIMEOUT) .header("x-ms-blob-type", "BlockBlob") .header("Content-Type", "application/gzip") @@ -452,8 +452,7 @@ async fn finalize_workspace_plugin_upload( } else { format!("{base_url}/public/plugins/workspace") }; - let client = build_reqwest_client(); - let request = authenticated_request(client.post(&url), auth)?.json(&body); + let request = authenticated_request(config.http_request(Method::POST, &url), auth).json(&body); send_and_decode(request, &url).await } @@ -491,7 +490,7 @@ fn archive_plugin_for_upload_with_limit( } async fn send_and_expect_status( - request: RequestBuilder, + request: RouteAwareRequestBuilder, url_for_error: &str, expected_statuses: &[StatusCode], ) -> Result<(), RemotePluginCatalogError> { diff --git a/codex-rs/core-plugins/src/remote/share/checkout.rs b/codex-rs/core-plugins/src/remote/share/checkout.rs index 6b12d2588d..f846a9b90e 100644 --- a/codex-rs/core-plugins/src/remote/share/checkout.rs +++ b/codex-rs/core-plugins/src/remote/share/checkout.rs @@ -93,6 +93,7 @@ pub async fn checkout_remote_plugin_share( )) })?; crate::remote_bundle::download_and_extract_remote_plugin_bundle_to_path( + config, bundle, local_plugin_path.clone(), ) diff --git a/codex-rs/core-plugins/src/remote/share/tests.rs b/codex-rs/core-plugins/src/remote/share/tests.rs index baaffb335a..c6346d0aba 100644 --- a/codex-rs/core-plugins/src/remote/share/tests.rs +++ b/codex-rs/core-plugins/src/remote/share/tests.rs @@ -1,4 +1,6 @@ use super::*; +use crate::test_support::recorded_http_client_urls; +use crate::test_support::recording_remote_plugin_service_config; use codex_app_server_protocol::PluginAuthPolicy; use codex_app_server_protocol::PluginInstallPolicy; use codex_app_server_protocol::PluginInterface; @@ -175,7 +177,8 @@ async fn save_remote_plugin_share_creates_workspace_plugin() { .unwrap() .len(); let server = MockServer::start().await; - let config = test_config(&server); + let (config, selected_urls) = + recording_remote_plugin_service_config(format!("{}/backend-api", server.uri())); let auth = test_auth(); Mock::given(method("POST")) @@ -261,6 +264,17 @@ async fn save_remote_plugin_share_creates_workspace_plugin() { local_paths::load_plugin_share_local_paths(codex_home.path()).unwrap(), BTreeMap::from([("plugins_123".to_string(), plugin_path)]) ); + assert_eq!( + recorded_http_client_urls(&selected_urls), + vec![ + format!( + "{}/backend-api/public/plugins/workspace/upload-url", + server.uri() + ), + format!("{}/upload/file_123", server.uri()), + format!("{}/backend-api/public/plugins/workspace", server.uri()), + ] + ); let requests = server.received_requests().await.unwrap_or_default(); let upload_request = requests diff --git a/codex-rs/core-plugins/src/remote_bundle.rs b/codex-rs/core-plugins/src/remote_bundle.rs index 7a302f2f36..482c737591 100644 --- a/codex-rs/core-plugins/src/remote_bundle.rs +++ b/codex-rs/core-plugins/src/remote_bundle.rs @@ -1,18 +1,20 @@ use crate::plugin_bundle_archive::PluginBundleUnpackError; use crate::plugin_bundle_archive::unpack_plugin_bundle_tar_gz; use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; +use crate::remote::RemotePluginServiceConfig; use crate::store::PluginInstallResult; use crate::store::PluginStore; use crate::store::PluginStoreError; use crate::store::error_context_sub_error_type; use crate::store::validate_plugin_version_segment; -use codex_login::default_client::build_reqwest_client; +use codex_http_client::HttpResponse; +use codex_http_client::RouteAwareRequestError; use codex_plugin::PluginId; use codex_plugin::PluginIdError; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::find_plugin_manifest_path; -use reqwest::Response; -use reqwest::StatusCode; +use http::Method; +use http::StatusCode; use serde_json::Value as JsonValue; use std::fs; use std::io; @@ -87,7 +89,7 @@ pub enum RemotePluginBundleInstallError { DownloadRequest { url: String, #[source] - source: reqwest::Error, + source: RouteAwareRequestError, }, #[error("remote plugin bundle download from {url} failed with status {status}: {body}")] @@ -101,7 +103,7 @@ pub enum RemotePluginBundleInstallError { DownloadBody { url: String, #[source] - source: reqwest::Error, + source: codex_http_client::HttpError, }, #[error("remote plugin bundle download from {url} exceeded maximum size of {max_bytes} bytes")] @@ -247,10 +249,12 @@ fn is_loopback_url(url: &Url) -> bool { } pub async fn download_and_install_remote_plugin_bundle( + config: &RemotePluginServiceConfig, codex_home: PathBuf, bundle: ValidatedRemotePluginBundle, ) -> Result { let bundle_bytes = download_remote_plugin_bundle_with_limit( + config, &bundle.bundle_download_url, /*max_bytes*/ REMOTE_PLUGIN_BUNDLE_MAX_DOWNLOAD_BYTES, ) @@ -267,10 +271,12 @@ pub async fn download_and_install_remote_plugin_bundle( } pub(crate) async fn download_and_extract_remote_plugin_bundle_to_path( + config: &RemotePluginServiceConfig, bundle: ValidatedRemotePluginBundle, destination: AbsolutePathBuf, ) -> Result { let bundle_bytes = download_remote_plugin_bundle_with_limit( + config, &bundle.bundle_download_url, /*max_bytes*/ REMOTE_PLUGIN_BUNDLE_MAX_DOWNLOAD_BYTES, ) @@ -287,12 +293,12 @@ pub(crate) async fn download_and_extract_remote_plugin_bundle_to_path( } async fn download_remote_plugin_bundle_with_limit( + config: &RemotePluginServiceConfig, bundle_download_url: &str, max_bytes: u64, ) -> Result, RemotePluginBundleInstallError> { - let client = build_reqwest_client(); - let response = client - .get(bundle_download_url) + let response = config + .http_request(Method::GET, bundle_download_url) .timeout(REMOTE_PLUGIN_BUNDLE_DOWNLOAD_TIMEOUT) .send() .await @@ -302,8 +308,8 @@ async fn download_remote_plugin_bundle_with_limit( })?; let final_url = response.url().clone(); - // reqwest may already have followed redirects here. For backend-issued bundle URLs, keep the - // shared client policy and fail unsupported final schemes before caching. + // The shared client has already followed redirects here. Reject an unsupported final scheme + // before caching a backend-issued bundle. if !is_allowed_bundle_download_url(&final_url, allow_test_loopback_http_bundle_downloads()) { return Err( RemotePluginBundleInstallError::UnsupportedBundleDownloadFinalUrl { @@ -354,7 +360,7 @@ async fn download_remote_plugin_bundle_with_limit( } async fn read_response_body_with_limit( - mut response: Response, + mut response: HttpResponse, url: &str, max_bytes: u64, ) -> Result, RemotePluginBundleInstallError> { @@ -623,11 +629,18 @@ fn is_standard_plugin_root(path: &Path) -> bool { #[cfg(test)] mod tests { use super::*; + use crate::test_support::recorded_http_client_urls; + use crate::test_support::recording_remote_plugin_service_config; use flate2::Compression; use flate2::write::GzEncoder; use pretty_assertions::assert_eq; use std::io::Write; use tempfile::tempdir; + use wiremock::Mock; + use wiremock::MockServer; + use wiremock::ResponseTemplate; + use wiremock::matchers::method; + use wiremock::matchers::path; const REMOTE_PLUGIN_ID: &str = "plugins~Plugin_00000000000000000000000000000000"; @@ -739,6 +752,34 @@ mod tests { )); } + #[tokio::test] + async fn bundle_download_routes_the_backend_supplied_url() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/signed/plugin-bundle")) + .respond_with(ResponseTemplate::new(200).set_body_bytes(b"bundle")) + .expect(1) + .mount(&server) + .await; + let (config, selected_urls) = + recording_remote_plugin_service_config(format!("{}/backend-api", server.uri())); + let download_url = format!("{}/signed/plugin-bundle?sig=signed-token", server.uri()); + + let err = + download_remote_plugin_bundle_with_limit(&config, &download_url, /*max_bytes*/ 64) + .await + .expect_err("plain HTTP final URL should remain unsupported"); + + assert!(matches!( + err, + RemotePluginBundleInstallError::UnsupportedBundleDownloadFinalUrl { .. } + )); + assert_eq!( + recorded_http_client_urls(&selected_urls), + vec![download_url] + ); + } + #[test] fn install_rejects_invalid_tar_gz_bundle() { let codex_home = tempdir().expect("tempdir"); diff --git a/codex-rs/core-plugins/src/remote_legacy.rs b/codex-rs/core-plugins/src/remote_legacy.rs index 137c33753b..5701637aae 100644 --- a/codex-rs/core-plugins/src/remote_legacy.rs +++ b/codex-rs/core-plugins/src/remote_legacy.rs @@ -1,7 +1,9 @@ use crate::remote::RemotePluginServiceConfig; +use codex_http_client::RouteAwareRequestError; use codex_login::CodexAuth; -use codex_login::default_client::build_reqwest_client; use codex_protocol::protocol::Product; +use http::Method; +use http::StatusCode; use serde::Deserialize; use std::time::Duration; use url::Url; @@ -39,13 +41,13 @@ pub enum RemotePluginMutationError { Request { url: String, #[source] - source: reqwest::Error, + source: RouteAwareRequestError, }, #[error("remote plugin mutation failed with status {status} from {url}: {body}")] UnexpectedStatus { url: String, - status: reqwest::StatusCode, + status: StatusCode, body: String, }, @@ -73,17 +75,20 @@ pub enum RemotePluginMutationError { #[derive(Debug, thiserror::Error)] pub enum RemotePluginFetchError { + #[error("invalid chatgpt base url for remote featured plugin request: {0}")] + InvalidBaseUrl(#[source] url::ParseError), + #[error("failed to send remote featured plugin request to {url}: {source}")] Request { url: String, #[source] - source: reqwest::Error, + source: RouteAwareRequestError, }, #[error("remote featured plugin request to {url} failed with status {status}: {body}")] UnexpectedStatus { url: String, - status: reqwest::StatusCode, + status: StatusCode, body: String, }, @@ -101,14 +106,15 @@ pub async fn fetch_remote_featured_plugin_ids( product: Option, ) -> Result, RemotePluginFetchError> { let base_url = config.chatgpt_base_url.trim_end_matches('/'); - let url = format!("{base_url}/plugins/featured"); - let client = build_reqwest_client(); - let mut request = client - .get(&url) - .query(&[( - "platform", - product.unwrap_or(Product::Codex).to_app_platform(), - )]) + let mut url = Url::parse(&format!("{base_url}/plugins/featured")) + .map_err(RemotePluginFetchError::InvalidBaseUrl)?; + url.query_pairs_mut().append_pair( + "platform", + product.unwrap_or(Product::Codex).to_app_platform(), + ); + let url = url.to_string(); + let mut request = config + .http_request(Method::GET, &url) .timeout(REMOTE_FEATURED_PLUGIN_FETCH_TIMEOUT); if let Some(auth) = auth.filter(|auth| auth.uses_codex_backend()) { @@ -173,9 +179,8 @@ async fn post_remote_plugin_mutation( ) -> Result { let auth = ensure_codex_backend_auth(auth)?; let url = remote_plugin_mutation_url(config, plugin_id, action)?; - let client = build_reqwest_client(); - let request = client - .post(url.clone()) + let request = config + .http_request(Method::POST, &url) .timeout(REMOTE_PLUGIN_MUTATION_TIMEOUT) .headers(codex_model_provider::auth_provider_from_auth(auth).to_auth_headers()); diff --git a/codex-rs/core-plugins/src/remote_tests.rs b/codex-rs/core-plugins/src/remote_tests.rs index cd6831a2ea..839e683342 100644 --- a/codex-rs/core-plugins/src/remote_tests.rs +++ b/codex-rs/core-plugins/src/remote_tests.rs @@ -1,5 +1,50 @@ use super::*; +use crate::test_support::recorded_http_client_urls; +use crate::test_support::recording_remote_plugin_service_config; use pretty_assertions::assert_eq; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::header_exists; +use wiremock::matchers::method; +use wiremock::matchers::path; + +#[tokio::test] +async fn remote_plugin_list_routes_the_complete_query_url() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/list")) + .and(header_exists("user-agent")) + .and(header_exists("originator")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "plugins": [], + "pagination": {"next_page_token": null}, + }))) + .expect(1) + .mount(&server) + .await; + let (config, selected_urls) = + recording_remote_plugin_service_config(format!("{}/backend-api", server.uri())); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + + get_remote_plugin_list_page( + &config, + &auth, + RemotePluginScope::Global, + Some("next page/+"), + Some("vertical & special"), + ) + .await + .expect("plugin list request should succeed"); + + assert_eq!( + recorded_http_client_urls(&selected_urls), + vec![format!( + "{}/backend-api/ps/plugins/list?scope=GLOBAL&limit=200&collection=vertical+%26+special&pageToken=next+page%2F%2B", + server.uri() + )] + ); +} #[test] fn build_remote_marketplace_preserves_directory_order_and_appends_installed_only_plugins() { diff --git a/codex-rs/core-plugins/src/startup_sync/http_client.rs b/codex-rs/core-plugins/src/startup_sync/http_client.rs index 15fcc452a8..b84813e41e 100644 --- a/codex-rs/core-plugins/src/startup_sync/http_client.rs +++ b/codex-rs/core-plugins/src/startup_sync/http_client.rs @@ -2,9 +2,9 @@ //! //! Curated plugin startup sync normally uses git, so its HTTP path is also a recovery path for //! machines where git is unavailable or fails. Under `ReqwestDefault`, that recovery path must -//! preserve the legacy `codex_login::default_client::build_reqwest_client()` behavior: invalid -//! custom-CA configuration is logged and falls back to a normal reqwest client instead of making -//! HTTP sync fail as well. +//! preserve the legacy `codex_login::default_client::create_client_without_request_logging()` +//! behavior: invalid custom-CA configuration is logged and falls back to a normal client instead +//! of making HTTP sync fail as well. //! //! When `RespectSystemProxy` is enabled, however, every concrete request URL—including download //! URLs returned by another endpoint—must be routed through `RouteAwareClientPool` so PAC and @@ -20,23 +20,28 @@ use std::time::Duration; use crate::http_client_selector::HttpClientSelector; use codex_http_client::ClientRouteClass; +use codex_http_client::HttpClient; use codex_http_client::HttpClientFactory; +use codex_http_client::HttpResponse; use codex_http_client::OutboundProxyPolicy; +use codex_http_client::RequestBuilder; use codex_http_client::RouteAwareClientPool; use codex_http_client::RouteAwareRequestBuilder; -use codex_login::default_client::build_reqwest_client; +use codex_login::default_client::create_client_without_request_logging; use http::HeaderMap; use http::Method; pub(super) enum StartupSyncHttpClient { - Default(reqwest::Client), + Default(HttpClient), RouteAware(Arc), } impl StartupSyncHttpClient { pub(super) fn new(http_client_factory: &HttpClientFactory) -> Self { match http_client_factory.outbound_proxy_policy() { - OutboundProxyPolicy::ReqwestDefault => Self::Default(build_reqwest_client()), + OutboundProxyPolicy::ReqwestDefault => { + Self::Default(create_client_without_request_logging()) + } OutboundProxyPolicy::RespectSystemProxy => { let http_clients = RouteAwareClientPool::with_chatgpt_cloudflare_cookies_without_request_logging( @@ -66,7 +71,7 @@ impl StartupSyncHttpClient { } pub(super) enum StartupSyncRequestBuilder { - Default(reqwest::RequestBuilder), + Default(RequestBuilder), RouteAware(RouteAwareRequestBuilder), } @@ -92,7 +97,7 @@ impl StartupSyncRequestBuilder { } } - pub(super) async fn send(self) -> Result { + pub(super) async fn send(self) -> Result { match self { Self::Default(request) => request.send().await.map_err(|err| err.to_string()), Self::RouteAware(request) => request.send().await.map_err(|err| err.to_string()), diff --git a/codex-rs/core-plugins/src/test_support.rs b/codex-rs/core-plugins/src/test_support.rs index 32bf1f92b4..da3a4aa670 100644 --- a/codex-rs/core-plugins/src/test_support.rs +++ b/codex-rs/core-plugins/src/test_support.rs @@ -7,6 +7,7 @@ use crate::OPENAI_API_CURATED_MARKETPLACE_NAME; use crate::OPENAI_CURATED_MARKETPLACE_NAME; use crate::PluginsConfigInput; use crate::http_client_selector::HttpClientSelector; +use crate::remote::RemotePluginServiceConfig; use codex_config::LoaderOverrides; use codex_config::NoopThreadConfigLoader; use codex_config::loader::load_config_layers_state; @@ -58,12 +59,24 @@ impl HttpClientSelector for RecordingHttpClientSelector { } self.delegate.request(method, url) } - fn outbound_proxy_policy(&self) -> OutboundProxyPolicy { self.delegate.outbound_proxy_policy() } } +pub(crate) fn recording_remote_plugin_service_config( + chatgpt_base_url: String, +) -> (RemotePluginServiceConfig, Arc>>) { + let (http_clients, selected_urls) = RecordingHttpClientSelector::new(); + ( + RemotePluginServiceConfig { + chatgpt_base_url, + http_clients, + }, + selected_urls, + ) +} + pub(crate) fn recorded_http_client_urls(selected_urls: &Mutex>) -> Vec { match selected_urls.lock() { Ok(selected_urls) => selected_urls.clone(), diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index a52838b2ac..24849a22f2 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -1712,6 +1712,15 @@ respect_system_proxy = true config.http_client_factory().outbound_proxy_policy(), codex_http_client::OutboundProxyPolicy::RespectSystemProxy ); + assert_eq!( + config.plugins_config_input().remote_plugin_service_config(), + codex_core_plugins::remote::RemotePluginServiceConfig::new( + config.chatgpt_base_url, + codex_http_client::HttpClientFactory::new( + codex_http_client::OutboundProxyPolicy::RespectSystemProxy, + ), + ) + ); Ok(()) } diff --git a/codex-rs/deny.toml b/codex-rs/deny.toml index f4f60f0a83..5f27f5542a 100644 --- a/codex-rs/deny.toml +++ b/codex-rs/deny.toml @@ -243,7 +243,6 @@ deny = [ "codex-api", "codex-app-server", "codex-core", - "codex-core-plugins", "codex-exec-server", "codex-lmstudio", "codex-login", diff --git a/codex-rs/http-client/src/default_client.rs b/codex-rs/http-client/src/default_client.rs index 99cbe39d7d..08cf8d3987 100644 --- a/codex-rs/http-client/src/default_client.rs +++ b/codex-rs/http-client/src/default_client.rs @@ -31,9 +31,9 @@ impl HttpClient { /// Creates a client that suppresses request URL and response-header diagnostics. /// - /// Use this for authentication endpoints whose URLs or headers may contain credentials that - /// are redacted by the caller above the HTTP transport boundary. - pub(crate) fn new_without_request_logging(inner: reqwest::Client) -> Self { + /// Use this for endpoints whose URLs or headers may contain credentials that are redacted by + /// the caller above the HTTP transport boundary. + pub fn new_without_request_logging(inner: reqwest::Client) -> Self { Self { inner, request_logging: RequestLogging::Disabled, diff --git a/codex-rs/login/src/auth/default_client.rs b/codex-rs/login/src/auth/default_client.rs index 6e1df1002e..630bcdb6e1 100644 --- a/codex-rs/login/src/auth/default_client.rs +++ b/codex-rs/login/src/auth/default_client.rs @@ -225,6 +225,15 @@ pub fn create_client() -> HttpClient { HttpClient::new(inner) } +/// Create the default HTTP client without request URL or response-header diagnostics. +/// +/// This preserves the default client's legacy custom-CA fallback and reqwest proxy behavior while +/// avoiding diagnostics that could expose credentials embedded in request URLs or headers. +pub fn create_client_without_request_logging() -> HttpClient { + let inner = build_reqwest_client(); + HttpClient::new_without_request_logging(inner) +} + /// Builds the default reqwest client used for ordinary Codex HTTP traffic. /// /// This starts from the standard Codex user agent, default headers, and sandbox-specific proxy