From 112abe12f242fea3ff31fb4e1f93f0fb211b0986 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 1 Aug 2026 11:09:43 +0000 Subject: [PATCH 1/2] fix(api): guard OpenAPI auth fallback origin Co-authored-by: Patrick Mill --- romm-api/src/client/openapi.rs | 63 +++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/romm-api/src/client/openapi.rs b/romm-api/src/client/openapi.rs index 4894ec3..0d1af35 100644 --- a/romm-api/src/client/openapi.rs +++ b/romm-api/src/client/openapi.rs @@ -2,6 +2,7 @@ use std::time::Instant; use crate::config::normalize_romm_origin; use crate::error::ApiError; +use reqwest::{header::HeaderMap, Url}; use super::response::{ api_error_from_response_truncated, read_error_response_text, version_from_heartbeat_json, @@ -75,7 +76,7 @@ impl RommClient { let urls = openapi_spec_urls(&root); let mut failures = Vec::new(); for url in &urls { - match self.fetch_openapi_json_once(url).await { + match self.fetch_openapi_json_once(url, &root).await { Ok(body) => return Ok(body), Err(e) => failures.push(openapi_failure_summary(url, &e)), } @@ -87,8 +88,16 @@ impl RommClient { ))) } - async fn fetch_openapi_json_once(&self, url: &str) -> Result { - let headers = self.build_headers()?; + async fn fetch_openapi_json_once( + &self, + url: &str, + auth_origin: &str, + ) -> Result { + let headers = if should_send_auth_to_openapi_url(url, auth_origin) { + self.build_headers()? + } else { + HeaderMap::new() + }; let t0 = Instant::now(); let resp = self.http.get(url).headers(headers).send().await?; @@ -113,6 +122,19 @@ impl RommClient { } } +fn should_send_auth_to_openapi_url(url: &str, auth_origin: &str) -> bool { + let Ok(openapi_url) = Url::parse(url) else { + return false; + }; + let Ok(auth_origin) = Url::parse(&normalize_romm_origin(auth_origin)) else { + return false; + }; + + openapi_url.scheme() == auth_origin.scheme() + && openapi_url.host_str() == auth_origin.host_str() + && openapi_url.port_or_known_default() == auth_origin.port_or_known_default() +} + /// Reject empty/HTML/non-OpenAPI 200 bodies so callers can try the next URL or fall back. fn validate_openapi_json_body(body: &str) -> Result<(), ApiError> { let trimmed = body.trim(); @@ -134,8 +156,9 @@ fn validate_openapi_json_body(body: &str) -> Result<(), ApiError> { #[cfg(test)] mod tests { use crate::config::{AuthConfig, Config, ExtrasDefaults}; + use reqwest::header::AUTHORIZATION; use wiremock::matchers::{method, path}; - use wiremock::{Mock, MockServer, ResponseTemplate}; + use wiremock::{Match, Mock, MockServer, Request, ResponseTemplate}; use super::*; @@ -159,6 +182,14 @@ mod tests { .expect("client") } + struct MissingHeader(&'static reqwest::header::HeaderName); + + impl Match for MissingHeader { + fn matches(&self, request: &Request) -> bool { + !request.headers.contains_key(self.0) + } + } + #[tokio::test] async fn fetch_openapi_json_rejects_empty_200_body() { let server = MockServer::start().await; @@ -249,4 +280,28 @@ mod tests { assert!(!log_msg.contains("url-secret"), "{log_msg}"); assert!(!log_msg.contains("query-secret"), "{log_msg}"); } + + #[tokio::test] + async fn fetch_openapi_json_once_omits_auth_for_off_origin_url() { + let server = MockServer::start().await; + let body = r#"{"openapi":"3.0.0","info":{"version":"1.0.0"},"paths":{}}"#; + Mock::given(method("GET")) + .and(path("/openapi.json")) + .and(MissingHeader(&AUTHORIZATION)) + .respond_with(ResponseTemplate::new(200).set_body_string(body)) + .expect(1) + .mount(&server) + .await; + + let client = client_for("http://romm.example/api"); + let got = client + .fetch_openapi_json_once( + &format!("{}/openapi.json", server.uri()), + "http://romm.example", + ) + .await + .expect("off-origin OpenAPI fetch should still succeed without auth"); + + assert!(got.contains("\"paths\"")); + } } From 64cefe37a5e0465ef4560d9cca37b68fa951058f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 1 Aug 2026 11:16:02 +0000 Subject: [PATCH 2/2] fix(api): block OpenAPI redirect credential leaks Co-authored-by: Patrick Mill --- romm-api/src/client/openapi.rs | 69 +++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/romm-api/src/client/openapi.rs b/romm-api/src/client/openapi.rs index 0d1af35..f7d61fc 100644 --- a/romm-api/src/client/openapi.rs +++ b/romm-api/src/client/openapi.rs @@ -2,12 +2,12 @@ use std::time::Instant; use crate::config::normalize_romm_origin; use crate::error::ApiError; -use reqwest::{header::HeaderMap, Url}; +use reqwest::{header::HeaderMap, redirect::Policy, Client as HttpClient, Url}; use super::response::{ api_error_from_response_truncated, read_error_response_text, version_from_heartbeat_json, }; -use super::RommClient; +use super::{http_user_agent, RommClient}; /// Returns the browser-style origin for RomM (no `/api` suffix). pub fn api_root_url(base_url: &str) -> String { @@ -99,8 +99,9 @@ impl RommClient { HeaderMap::new() }; + let http = openapi_http_client()?; let t0 = Instant::now(); - let resp = self.http.get(url).headers(headers).send().await?; + let resp = http.get(url).headers(headers).send().await?; let status = resp.status(); if self.verbose { @@ -135,6 +136,13 @@ fn should_send_auth_to_openapi_url(url: &str, auth_origin: &str) -> bool { && openapi_url.port_or_known_default() == auth_origin.port_or_known_default() } +fn openapi_http_client() -> Result { + Ok(HttpClient::builder() + .user_agent(http_user_agent()) + .redirect(Policy::none()) + .build()?) +} + /// Reject empty/HTML/non-OpenAPI 200 bodies so callers can try the next URL or fall back. fn validate_openapi_json_body(body: &str) -> Result<(), ApiError> { let trimmed = body.trim(); @@ -156,21 +164,27 @@ fn validate_openapi_json_body(body: &str) -> Result<(), ApiError> { #[cfg(test)] mod tests { use crate::config::{AuthConfig, Config, ExtrasDefaults}; - use reqwest::header::AUTHORIZATION; use wiremock::matchers::{method, path}; use wiremock::{Match, Mock, MockServer, Request, ResponseTemplate}; use super::*; fn client_for(base_url: &str) -> RommClient { + client_for_auth( + base_url, + AuthConfig::Bearer { + token: "secret".to_string(), + }, + ) + } + + fn client_for_auth(base_url: &str, auth: AuthConfig) -> RommClient { RommClient::new( &Config { base_url: base_url.to_string(), download_dir: ".".to_string(), use_https: false, - auth: Some(AuthConfig::Bearer { - token: "secret".to_string(), - }), + auth: Some(auth), extras_defaults: ExtrasDefaults::default(), save_sync: Default::default(), roms_layout: Default::default(), @@ -182,7 +196,7 @@ mod tests { .expect("client") } - struct MissingHeader(&'static reqwest::header::HeaderName); + struct MissingHeader(&'static str); impl Match for MissingHeader { fn matches(&self, request: &Request) -> bool { @@ -287,7 +301,7 @@ mod tests { let body = r#"{"openapi":"3.0.0","info":{"version":"1.0.0"},"paths":{}}"#; Mock::given(method("GET")) .and(path("/openapi.json")) - .and(MissingHeader(&AUTHORIZATION)) + .and(MissingHeader("authorization")) .respond_with(ResponseTemplate::new(200).set_body_string(body)) .expect(1) .mount(&server) @@ -304,4 +318,41 @@ mod tests { assert!(got.contains("\"paths\"")); } + + #[tokio::test] + async fn fetch_openapi_json_once_does_not_follow_off_origin_redirect_with_api_key_auth() { + let origin = MockServer::start().await; + let redirected = MockServer::start().await; + let body = r#"{"openapi":"3.0.0","info":{"version":"1.0.0"},"paths":{}}"#; + + Mock::given(method("GET")) + .and(path("/openapi.json")) + .respond_with( + ResponseTemplate::new(302) + .insert_header("Location", format!("{}/openapi.json", redirected.uri())), + ) + .expect(1) + .mount(&origin) + .await; + Mock::given(method("GET")) + .and(path("/openapi.json")) + .respond_with(ResponseTemplate::new(200).set_body_string(body)) + .expect(0) + .mount(&redirected) + .await; + + let client = client_for_auth( + &origin.uri(), + AuthConfig::ApiKey { + header: "X-API-Key".to_string(), + key: "secret".to_string(), + }, + ); + let err = client + .fetch_openapi_json_once(&format!("{}/openapi.json", origin.uri()), &origin.uri()) + .await + .expect_err("off-origin OpenAPI redirect should not be followed"); + + assert!(err.to_string().contains("302"), "{err}"); + } }