diff --git a/codex-rs/app-server/tests/suite/v2/plugin_install.rs b/codex-rs/app-server/tests/suite/v2/plugin_install.rs index 87999efce1..ae19ff3b50 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -1183,6 +1183,7 @@ async fn plugin_install_preserves_status_when_remote_bundle_error_body_is_too_la let server = MockServer::start().await; let bundle_url = mount_remote_plugin_bundle(&server, /*status_code*/ 503, vec![b'x'; 8 * 1024 + 1]).await; + let bundle_url = format!("{bundle_url}?sig=signed-token"); configure_remote_plugin_test(codex_home.path(), &server)?; mount_remote_plugin_detail(&server, REMOTE_PLUGIN_ID, "1.2.3", Some(&bundle_url)).await; mount_empty_remote_installed_plugins(&server).await; @@ -1218,6 +1219,7 @@ async fn plugin_install_preserves_status_when_remote_bundle_error_body_is_too_la 8192 ); assert!(!err.error.message.contains("exceeded maximum size")); + assert!(!err.error.message.contains("signed-token")); wait_for_remote_plugin_request_count( &server, "GET", diff --git a/codex-rs/core-plugins/src/remote_bundle.rs b/codex-rs/core-plugins/src/remote_bundle.rs index c6fbb4b38d..cb884aa40f 100644 --- a/codex-rs/core-plugins/src/remote_bundle.rs +++ b/codex-rs/core-plugins/src/remote_bundle.rs @@ -37,7 +37,7 @@ const REMOTE_PLUGIN_INSTALL_STAGING_DIR: &str = "plugins/.remote-plugin-install- const TEST_ALLOW_LOOPBACK_HTTP_REMOTE_PLUGIN_BUNDLES_ENV: &str = "CODEX_TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS"; -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct ValidatedRemotePluginBundle { pub plugin_id: PluginId, pub plugin_version: String, @@ -46,6 +46,17 @@ pub struct ValidatedRemotePluginBundle { bundle_download_url: String, } +impl std::fmt::Debug for ValidatedRemotePluginBundle { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("ValidatedRemotePluginBundle") + .field("plugin_id", &self.plugin_id) + .field("plugin_version", &self.plugin_version) + .field("remote_plugin_id", &self.remote_plugin_id) + .finish_non_exhaustive() + } +} + #[derive(Debug, thiserror::Error)] pub enum RemotePluginBundleInstallError { #[error("backend did not return a release version for remote plugin `{remote_plugin_id}`")] @@ -63,11 +74,10 @@ pub enum RemotePluginBundleInstallError { MissingBundleDownloadUrl { remote_plugin_id: String }, #[error( - "backend returned an invalid download URL for remote plugin `{remote_plugin_id}`: {url}" + "backend returned an invalid download URL for remote plugin `{remote_plugin_id}`: {source}" )] InvalidBundleDownloadUrl { remote_plugin_id: String, - url: String, #[source] source: url::ParseError, }, @@ -89,32 +99,34 @@ pub enum RemotePluginBundleInstallError { source: PluginIdError, }, - #[error("failed to send remote plugin bundle download request to {url}: {source}")] + #[error("failed to send remote plugin bundle download request to {host}: {source}")] DownloadRequest { - url: String, + host: String, #[source] source: RouteAwareRequestError, }, - #[error("remote plugin bundle download from {url} failed with status {status}: {body}")] + #[error("remote plugin bundle download from {host} failed with status {status}: {body}")] DownloadStatus { - url: String, + host: String, status: StatusCode, body: String, }, - #[error("failed to read remote plugin bundle download response from {url}: {source}")] + #[error("failed to read remote plugin bundle download response from {host}: {source}")] DownloadBody { - url: String, + host: String, #[source] source: codex_http_client::HttpError, }, - #[error("remote plugin bundle download from {url} exceeded maximum size of {max_bytes} bytes")] - DownloadTooLarge { url: String, max_bytes: u64 }, + #[error("remote plugin bundle download from {host} exceeded maximum size of {max_bytes} bytes")] + DownloadTooLarge { host: String, max_bytes: u64 }, - #[error("remote plugin bundle download from {url} redirected to unsupported URL {final_url}")] - UnsupportedBundleDownloadFinalUrl { url: String, final_url: String }, + #[error( + "remote plugin bundle download from {host} resolved to unsupported URL scheme {scheme}" + )] + UnsupportedBundleDownloadFinalUrl { host: String, scheme: String }, #[error( "remote plugin bundle extracted size would be {bytes} bytes, exceeding the maximum total size of {max_bytes} bytes" @@ -201,7 +213,6 @@ pub fn validate_remote_plugin_bundle( let parsed_bundle_url = Url::parse(&bundle_download_url).map_err(|source| { RemotePluginBundleInstallError::InvalidBundleDownloadUrl { remote_plugin_id: remote_plugin_id.to_string(), - url: bundle_download_url.clone(), source, } })?; @@ -311,23 +322,27 @@ async fn download_remote_plugin_bundle_with_limit( .send() .await .map_err(|source| RemotePluginBundleInstallError::DownloadRequest { - url: bundle_download_url.to_string(), - source, + host: Url::parse(bundle_download_url) + .ok() + .and_then(|url| url.host_str().map(str::to_owned)) + .unwrap_or_else(|| "unknown-host".to_string()), + source: source.without_url(), })?; let final_url = response.url().clone(); + // Keep only the host in URL error fields. + let host = final_url.host_str().unwrap_or("unknown-host").to_string(); // 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 { - url: bundle_download_url.to_string(), - final_url: final_url.to_string(), + host, + scheme: final_url.scheme().to_string(), }, ); } - let url = final_url.to_string(); let status = response.status(); if !status.is_success() { let mut response = response; @@ -339,7 +354,7 @@ async fn download_remote_plugin_bundle_with_limit( Ok(Some(chunk)) => chunk, Ok(None) => break, Err(source) => { - body_read_error = Some(source); + body_read_error = Some(source.without_url()); break; } }; @@ -361,19 +376,19 @@ async fn download_remote_plugin_bundle_with_limit( if let Some(source) = body_read_error { body.push_str(&format!("\n[failed to read response body: {source}]")); } - return Err(RemotePluginBundleInstallError::DownloadStatus { url, status, body }); + return Err(RemotePluginBundleInstallError::DownloadStatus { host, status, body }); } - read_response_body_with_limit(response, &url, max_bytes).await + read_response_body_with_limit(response, &host, max_bytes).await } async fn read_response_body_with_limit( mut response: HttpResponse, - url: &str, + host: &str, max_bytes: u64, ) -> Result, RemotePluginBundleInstallError> { if let Some(content_length) = response.content_length() { - enforce_download_size_limit(url, content_length, max_bytes)?; + enforce_download_size_limit(host, content_length, max_bytes)?; } let mut body = Vec::new(); @@ -382,12 +397,12 @@ async fn read_response_body_with_limit( .chunk() .await .map_err(|source| RemotePluginBundleInstallError::DownloadBody { - url: url.to_string(), - source, + host: host.to_string(), + source: source.without_url(), })? { let next_len = body.len() as u64 + chunk.len() as u64; - enforce_download_size_limit(url, next_len, max_bytes)?; + enforce_download_size_limit(host, next_len, max_bytes)?; body.extend_from_slice(&chunk); } @@ -395,13 +410,13 @@ async fn read_response_body_with_limit( } fn enforce_download_size_limit( - url: &str, + host: &str, bytes: u64, max_bytes: u64, ) -> Result<(), RemotePluginBundleInstallError> { if bytes > max_bytes { return Err(RemotePluginBundleInstallError::DownloadTooLarge { - url: url.to_string(), + host: host.to_string(), max_bytes, }); } @@ -654,6 +669,7 @@ mod tests { use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path; + use wiremock::matchers::query_param; const REMOTE_PLUGIN_ID: &str = "plugins~Plugin_00000000000000000000000000000000"; @@ -680,7 +696,7 @@ mod tests { "openai-curated-remote", "linear", Some("1.2.3"), - Some("https://example.com/linear.tar.gz"), + Some("https://example.com/linear.tar.gz?sig=signed-token"), /*app_manifest*/ None, ) .expect("valid install plan"); @@ -690,8 +706,9 @@ mod tests { assert_eq!(bundle.plugin_version, "1.2.3"); assert_eq!( bundle.bundle_download_url.as_str(), - "https://example.com/linear.tar.gz" + "https://example.com/linear.tar.gz?sig=signed-token" ); + assert!(!format!("{bundle:?}").contains("signed-token")); } #[test] @@ -768,12 +785,9 @@ mod tests { #[test] fn download_size_limit_rejects_oversized_bundle() { - let err = enforce_download_size_limit( - "https://example.com/linear.tar.gz", - /*bytes*/ 5, - /*max_bytes*/ 4, - ) - .expect_err("oversized bundle download should fail"); + let err = + enforce_download_size_limit("example.com", /*bytes*/ 5, /*max_bytes*/ 4) + .expect_err("oversized bundle download should fail"); assert!(matches!( err, @@ -786,6 +800,7 @@ mod tests { let server = MockServer::start().await; Mock::given(method("GET")) .and(path("/signed/plugin-bundle")) + .and(query_param("sig", "signed-token")) .respond_with(ResponseTemplate::new(200).set_body_bytes(b"bundle")) .expect(1) .mount(&server) @@ -803,6 +818,8 @@ mod tests { err, RemotePluginBundleInstallError::UnsupportedBundleDownloadFinalUrl { .. } )); + assert!(!err.to_string().contains("signed-token")); + assert!(!format!("{err:?}").contains("signed-token")); assert_eq!( recorded_http_client_urls(&selected_urls), vec![download_url]