diff --git a/crates/fetchkit/src/fetchers/github_commit.rs b/crates/fetchkit/src/fetchers/github_commit.rs index 807794b..a5cf18e 100644 --- a/crates/fetchkit/src/fetchers/github_commit.rs +++ b/crates/fetchkit/src/fetchers/github_commit.rs @@ -3,7 +3,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::{validate_url_policy, Fetcher}; use crate::types::{FetchRequest, FetchResponse}; use crate::DEFAULT_USER_AGENT; use async_trait::async_trait; @@ -176,6 +176,7 @@ impl Fetcher for GitHubCommitFetcher { ), }; let api_url = api_url(owner, repo, endpoint, &label)?; + validate_url_policy(&api_url, options)?; let response = transport_request( api_url, reqwest::Method::GET, diff --git a/crates/fetchkit/src/fetchers/mod.rs b/crates/fetchkit/src/fetchers/mod.rs index 2d1d02e..1f757d1 100644 --- a/crates/fetchkit/src/fetchers/mod.rs +++ b/crates/fetchkit/src/fetchers/mod.rs @@ -210,30 +210,7 @@ impl FetcherRegistry { let parsed_url = Url::parse(&request.url).map_err(|_| FetchError::InvalidUrlScheme)?; - options.validate_url(&parsed_url)?; - - // THREAT[TM-INPUT-002]: Normalize URL before prefix matching to prevent - // encoding-based bypasses (case, trailing dots, default ports) - // THREAT[TM-INPUT-007]: URL-aware prefix matching prevents subdomain tricks - if !options.allow_prefixes.is_empty() { - let allowed = options - .allow_prefixes - .iter() - .any(|prefix| url_matches_policy_prefix(&parsed_url, prefix)); - if !allowed { - debug!(url = %request.url, "URL not in allow list"); - return Err(FetchError::BlockedUrl); - } - } - - if options - .block_prefixes - .iter() - .any(|prefix| url_matches_policy_prefix(&parsed_url, prefix)) - { - debug!(url = %request.url, "URL matched block list"); - return Err(FetchError::BlockedUrl); - } + validate_url_policy(&parsed_url, options)?; for fetcher in &self.fetchers { if fetcher.matches(&parsed_url) { @@ -277,6 +254,39 @@ impl FetcherRegistry { } } +/// Validate all operator URL policy for a request destination. +/// +/// Specialized fetchers that derive secondary API URLs must call this before +/// transport_request so allow/block/host/port policy applies to every egress hop. +pub(crate) fn validate_url_policy(url: &Url, options: &FetchOptions) -> Result<(), FetchError> { + options.validate_url(url)?; + + // THREAT[TM-INPUT-002]: Normalize URL before prefix matching to prevent + // encoding-based bypasses (case, trailing dots, default ports) + // THREAT[TM-INPUT-007]: URL-aware prefix matching prevents subdomain tricks + if !options.allow_prefixes.is_empty() { + let allowed = options + .allow_prefixes + .iter() + .any(|prefix| url_matches_policy_prefix(url, prefix)); + if !allowed { + debug!(url = %url, "URL not in allow list"); + return Err(FetchError::BlockedUrl); + } + } + + if options + .block_prefixes + .iter() + .any(|prefix| url_matches_policy_prefix(url, prefix)) + { + debug!(url = %url, "URL matched block list"); + return Err(FetchError::BlockedUrl); + } + + Ok(()) +} + // THREAT[TM-INPUT-010]: Invalid file destinations must fail before any outbound request. // Mitigation: reject blank paths and invoke the adapter's preflight validation first. async fn preflight_save_path<'a>( diff --git a/crates/fetchkit/tests/transport.rs b/crates/fetchkit/tests/transport.rs index 5dfaa20..94e6bfd 100644 --- a/crates/fetchkit/tests/transport.rs +++ b/crates/fetchkit/tests/transport.rs @@ -7,8 +7,8 @@ use async_trait::async_trait; use bytes::Bytes; use fetchkit::{ - ArXivFetcher, DefaultFetcher, DnsPolicy, DocsSiteFetcher, FetchError, FetchOptions, - FetchRequest, Fetcher, GitHubCodeFetcher, GitHubIssueFetcher, GitHubRepoFetcher, + fetch_with_options, ArXivFetcher, DefaultFetcher, DnsPolicy, DocsSiteFetcher, FetchError, + FetchOptions, FetchRequest, Fetcher, GitHubCodeFetcher, GitHubIssueFetcher, GitHubRepoFetcher, HackerNewsFetcher, HttpMethod, HttpTransport, PackageRegistryFetcher, RSSFeedFetcher, StackOverflowFetcher, TransportError, TransportMethod, TransportRequest, TransportResponse, TwitterFetcher, WikipediaFetcher, YouTubeFetcher, @@ -300,6 +300,22 @@ async fn default_fetcher_enforces_body_cap_on_transport_stream() { assert_eq!(response.size, Some(10)); } +#[tokio::test] +async fn github_commit_fetcher_enforces_policy_on_api_subrequest() { + let mock = Arc::new(MockTransport::new()); + let mut options = options_with(mock.clone()); + options.allow_prefixes = vec!["http://github.com".to_string()]; + options.block_prefixes = vec!["https://api.github.com".to_string()]; + options.blocked_hosts = vec!["api.github.com".to_string()]; + options.allowed_ports = vec![80]; + + let request = FetchRequest::new("http://github.com/everruns/fetchkit/commit/deadbeef"); + let result = fetch_with_options(request, options).await; + + assert!(matches!(result, Err(FetchError::BlockedUrl))); + assert!(mock.calls().is_empty()); +} + #[tokio::test] async fn wikipedia_fetcher_uses_injected_transport() { let summary = br#"{"title":"Rust","extract":"A language.","description":"PL"}"#; diff --git a/specs/fetchers.md b/specs/fetchers.md index 1034836..dd4a5bf 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. Provides shared URL-policy validation for fetchers that derive secondary API URLs; every outbound destination must satisfy the same host, port, allow-prefix, and block-prefix policy before transport execution +7. Provides `fetch_to_file()` that dispatches to matched fetcher's `fetch_to_file()` ### Built-in Fetchers @@ -47,6 +48,7 @@ Central dispatcher that: - Uses GitHub's REST API for commit metadata, authorship, signature verification, changed-file statistics, and patches - Comparison responses include ahead/behind counts, merge base, and included commits - Limits each patch excerpt to 2,000 Unicode characters and enforces the configured overall response limit +- Enforces full URL policy on the derived `https://api.github.com` request before transport execution - Response format field: `"github_commit"` or `"github_compare"` #### GitHubActionsRunFetcher