diff --git a/lib/crates/fabro-server/src/install.rs b/lib/crates/fabro-server/src/install.rs index 60e5aa09d..c6c5b88f7 100644 --- a/lib/crates/fabro-server/src/install.rs +++ b/lib/crates/fabro-server/src/install.rs @@ -1179,6 +1179,51 @@ fn install_http_client_for_url(base_url: &str) -> Result Result { + let url = fabro_http::Url::parse(raw).map_err(|err| err.to_string())?; + match url.scheme() { + "http" | "https" => {} + other => { + return Err(format!( + "install upstream URL must use http or https, got {other}" + )); + } + } + if url.host_str().is_none() { + return Err("install upstream URL must include a host".to_string()); + } + Ok(url) +} + +/// Append `segments` as new path segments to a validated base URL. +/// +/// Each segment is percent-encoded by `url`, so caller-controlled values +/// (e.g. a GitHub manifest `code`) cannot insert additional path components, +/// alter the host, or redirect the request to a different URL scheme. +fn install_upstream_endpoint(base_url: &str, segments: &[&str]) -> Result { + let mut url = parse_install_upstream_url(base_url)?; + { + let mut path = url + .path_segments_mut() + .map_err(|()| "install upstream URL cannot be a base".to_string())?; + for segment in segments { + path.push(segment); + } + } + Ok(url) +} + async fn validate_llm_provider( state: &InstallAppState, input: &InstallLlmTestInput, @@ -1200,9 +1245,10 @@ async fn validate_llm_provider( }; let base_url = provider_base_url(state, input.provider); + let endpoint = install_upstream_endpoint(&base_url, &["models"])?; let client = install_http_client_for_url(&base_url)?; let mut request = client - .get(format!("{base_url}/models")) + .get(endpoint) .header(auth_header, auth_value) .header("User-Agent", "fabro-server"); if matches!(input.provider, Provider::Anthropic) { @@ -1256,9 +1302,10 @@ async fn validate_github_token(state: &InstallAppState, token: &str) -> Result Result { + if !is_valid_github_manifest_code(code) { + return Err("install GitHub manifest code is not in the expected format".to_string()); + } let base_url = state .upstreams .github_api_base_url .clone() .unwrap_or_else(|| DEFAULT_INSTALL_GITHUB_API_BASE_URL.to_string()); + let endpoint = install_upstream_endpoint(&base_url, &["app-manifests", code, "conversions"])?; let client = install_http_client_for_url(&base_url)?; let response = client - .post(format!("{base_url}/app-manifests/{code}/conversions")) + .post(endpoint) .header("Accept", "application/vnd.github+json") .header("User-Agent", "fabro-server") .send() @@ -1297,6 +1348,18 @@ async fn exchange_github_app_manifest_code( response.json().await.map_err(|err| err.to_string()) } +/// GitHub's manifest-conversion `code` is short, unpadded-base64url by +/// construction. Reject anything outside that alphabet so a malicious +/// browser callback cannot smuggle extra path segments, host overrides, or +/// query parameters into the request. +fn is_valid_github_manifest_code(code: &str) -> bool { + !code.is_empty() + && code.len() <= 256 + && code + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') +} + async fn write_artifact_store_metadata( settings: &SettingsLayer, storage_dir: &Path,