openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-31 19:28:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Keep remote plugin bundle URLs out of diagnostics (#58165)

Browse Source 
## Why

Remote plugin bundle download URLs can contain signature tokens in query parameters. Including the full URL in errors or debug output exposes those tokens.

## What changed

Omit the download URL from `ValidatedRemotePluginBundle` debug output. Report download hosts instead of full URLs, strip URLs from underlying HTTP errors, and report only the scheme for unsupported final URLs. Invalid URL errors retain the parse error without echoing the input.

## Testing

Extend tests to check that signature tokens stay out of bundle debug output, unsupported final URL errors, and app-server installation errors while preserving signed request URLs and HTTP status diagnostics.

GitOrigin-RevId: 8edda4847feae4f2bcb12fbb414a3a25bde53814
This commit is contained in:
Matthew Zeng
2026-10-29 19:49:03 +00:00
committed by copyberry
parent 4bc746c94f
commit f5fbe7accd
2 changed files with 56 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core-plugins/src/remote_bundle.rs
+54 -37
View File
@@ -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<Vec<u8>, 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]