From 0b122cc542c2f282dd7afa85d238de146e7a38ab Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Fri, 24 Jul 2026 01:11:35 -0500 Subject: [PATCH] fix(fetchers): enforce release API URL policy --- .../fetchkit/src/fetchers/github_release.rs | 141 +++++++++++++++++- specs/fetchers.md | 6 +- 2 files changed, 145 insertions(+), 2 deletions(-) diff --git a/crates/fetchkit/src/fetchers/github_release.rs b/crates/fetchkit/src/fetchers/github_release.rs index df6eef8..fed585b 100644 --- a/crates/fetchkit/src/fetchers/github_release.rs +++ b/crates/fetchkit/src/fetchers/github_release.rs @@ -5,7 +5,7 @@ use crate::client::FetchOptions; use crate::error::FetchError; use crate::fetchers::default::{read_full_body, transport_request}; -use crate::fetchers::Fetcher; +use crate::fetchers::{url_matches_policy_prefix, Fetcher}; use crate::types::{FetchRequest, FetchResponse}; use crate::DEFAULT_USER_AGENT; use async_trait::async_trait; @@ -99,6 +99,7 @@ impl Fetcher for GitHubReleaseFetcher { "https://api.github.com/repos/{owner}/{repo}/releases/tags/{tag}" )) .map_err(|_| FetchError::InvalidUrlScheme)?; + validate_secondary_api_url(&api_url, options)?; let mut headers = HeaderMap::new(); headers.insert( @@ -155,6 +156,31 @@ impl Fetcher for GitHubReleaseFetcher { } } +// THREAT[TM-SSRF-011]: Specialized fetchers that derive secondary URLs must apply +// the same operator egress policy as direct user-requested URLs before transport. +fn validate_secondary_api_url(url: &Url, options: &FetchOptions) -> Result<(), FetchError> { + options.validate_url(url)?; + + if !options.allow_prefixes.is_empty() + && !options + .allow_prefixes + .iter() + .any(|prefix| url_matches_policy_prefix(url, prefix)) + { + return Err(FetchError::BlockedUrl); + } + + if options + .block_prefixes + .iter() + .any(|prefix| url_matches_policy_prefix(url, prefix)) + { + return Err(FetchError::BlockedUrl); + } + + Ok(()) +} + fn format_release(release: &Release) -> String { let title = release.name.as_deref().unwrap_or(&release.tag_name); let mut out = format!( @@ -204,6 +230,52 @@ fn truncate(mut value: String, max: usize) -> (String, bool) { #[cfg(test)] mod tests { use super::*; + use crate::dns::DnsPolicy; + use crate::transport::{HttpTransport, TransportError, TransportRequest, TransportResponse}; + use bytes::Bytes; + use std::sync::{Arc, Mutex}; + + struct RecordingTransport { + calls: Mutex>, + } + + #[async_trait] + impl HttpTransport for RecordingTransport { + async fn execute( + &self, + req: TransportRequest, + ) -> Result { + self.calls.lock().unwrap().push(req.url.to_string()); + let body = Bytes::from( + r#"{ + "name":"Fetchkit 1.0", + "tag_name":"v1.0.0", + "body":"Notable changes.", + "html_url":"https://github.com/owner/repo/releases/tag/v1.0.0", + "draft":false, + "prerelease":false, + "author":{"login":"octocat"}, + "created_at":"2026-01-01T00:00:00Z", + "published_at":"2026-01-02T00:00:00Z", + "assets":[] + }"#, + ); + Ok(TransportResponse { + status: 200, + url: req.url, + headers: vec![("content-type".into(), "application/json".into())], + body: Box::pin(futures::stream::once(async move { Ok(body) })), + }) + } + } + + fn options_with_transport(transport: Arc) -> FetchOptions { + FetchOptions { + dns_policy: DnsPolicy::allow_all(), + transport: Some(transport), + ..Default::default() + } + } #[test] fn parses_release_url_and_preserves_encoded_tag() { @@ -225,6 +297,73 @@ mod tests { } } + #[test] + fn validates_secondary_api_url_against_host_port_and_prefix_policy() { + let api_url = + Url::parse("https://api.github.com/repos/owner/repo/releases/tags/v1.0.0").unwrap(); + + let options = FetchOptions { + blocked_hosts: vec!["api.github.com".into()], + ..Default::default() + }; + assert!(matches!( + validate_secondary_api_url(&api_url, &options), + Err(FetchError::BlockedUrl) + )); + + let options = FetchOptions { + allowed_ports: vec![80], + ..Default::default() + }; + assert!(matches!( + validate_secondary_api_url(&api_url, &options), + Err(FetchError::BlockedUrl) + )); + + let options = FetchOptions { + allow_prefixes: vec!["https://github.com/owner/repo/releases/tag/".into()], + ..Default::default() + }; + assert!(matches!( + validate_secondary_api_url(&api_url, &options), + Err(FetchError::BlockedUrl) + )); + + let options = FetchOptions { + allow_prefixes: vec!["https://api.github.com/repos/owner/repo/releases/tags/".into()], + block_prefixes: vec![ + "https://api.github.com/repos/owner/repo/releases/tags/v1.0.0".into(), + ], + ..Default::default() + }; + assert!(matches!( + validate_secondary_api_url(&api_url, &options), + Err(FetchError::BlockedUrl) + )); + } + + #[tokio::test] + async fn blocks_disallowed_secondary_api_url_before_transport() { + let transport = Arc::new(RecordingTransport { + calls: Mutex::new(Vec::new()), + }); + let mut options = options_with_transport(transport.clone()); + options.allow_prefixes = vec!["https://github.com/owner/repo/releases/tag/".into()]; + + let result = GitHubReleaseFetcher::new() + .fetch( + &FetchRequest { + url: "https://github.com/owner/repo/releases/tag/v1.0.0".into(), + ..Default::default() + }, + &options, + ) + .await; + + assert!(matches!(result, Err(FetchError::BlockedUrl))); + assert!(transport.calls.lock().unwrap().is_empty()); + } + #[test] fn formats_metadata_notes_and_assets() { let release = Release { diff --git a/specs/fetchers.md b/specs/fetchers.md index 1034836..9faa620 100644 --- a/specs/fetchers.md +++ b/specs/fetchers.md @@ -24,7 +24,8 @@ Central dispatcher that: 3. Falls back to default fetcher if none match 4. Provides `register()` for adding custom fetchers 5. Validates URL scheme and allow/block lists before dispatching -6. Provides `fetch_to_file()` that dispatches to matched fetcher's `fetch_to_file()` +6. Requires specialized fetchers to apply the same host, port, allow-prefix, and block-prefix policy to any secondary URL they derive before transport +7. Provides `fetch_to_file()` that dispatches to matched fetcher's `fetch_to_file()` ### Built-in Fetchers @@ -62,6 +63,7 @@ Central dispatcher that: - Matches: `https://github.com/{owner}/{repo}/releases/tag/{tag}` - Behavior: Fetches tagged release metadata, notes, and assets through the public GitHub API - Enforces the configured maximum response body size +- Applies configured host, port, allow-prefix, and block-prefix policy to the derived `api.github.com` request before transport - Response format field: `"github_release"` #### GitHubRepoFetcher @@ -223,6 +225,8 @@ All fetchers perform their outbound HTTP exclusively through a pluggable it never follows redirects and never performs DNS policy resolution. fetchkit owns URL validation, DNS policy (resolve-then-check, producing `TransportRequest.pinned_addrs`), manual per-hop redirect following, bot-auth signing, and body-size/timeout caps; +specialized fetchers that derive secondary URLs must re-apply host, port, +allow-prefix, and block-prefix policy to those derived URLs before transport; only the socket-level send is delegated. `FetchOptions.transport` selects the implementation (`None` => default