diff --git a/crates/fetchkit/src/fetchers/gitlab.rs b/crates/fetchkit/src/fetchers/gitlab.rs index a0af176..2e8b6c0 100644 --- a/crates/fetchkit/src/fetchers/gitlab.rs +++ b/crates/fetchkit/src/fetchers/gitlab.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::{enforce_url_policy, Fetcher}; use crate::types::{FetchRequest, FetchResponse}; use crate::DEFAULT_USER_AGENT; use async_trait::async_trait; @@ -112,6 +112,7 @@ impl Fetcher for GitLabFetcher { let resource = Self::parse_url(&page_url) .ok_or_else(|| FetchError::FetcherError("Not a supported GitLab URL".into()))?; let (api_url, format, raw) = api_url(&resource)?; + enforce_url_policy(&api_url, options)?; let mut headers = HeaderMap::new(); headers.insert( USER_AGENT, diff --git a/crates/fetchkit/src/fetchers/mod.rs b/crates/fetchkit/src/fetchers/mod.rs index 2d1d02e..36de4c8 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); - } + enforce_url_policy(&parsed_url, options)?; for fetcher in &self.fetchers { if fetcher.matches(&parsed_url) { @@ -300,6 +277,36 @@ async fn preflight_save_path<'a>( Ok(Some(path)) } +/// Enforce all caller-supplied outbound URL policy for a concrete request URL. +pub(crate) fn enforce_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-002]: URL-aware prefix matching normalizes both the URL and the prefix // before comparison, preventing bypasses via encoding, case, or trailing dots. // THREAT[TM-INPUT-007]: Compares parsed URL components (scheme, host, path) instead of diff --git a/crates/fetchkit/tests/transport.rs b/crates/fetchkit/tests/transport.rs index 5dfaa20..ec0df70 100644 --- a/crates/fetchkit/tests/transport.rs +++ b/crates/fetchkit/tests/transport.rs @@ -6,6 +6,7 @@ use async_trait::async_trait; use bytes::Bytes; +use fetchkit::fetchers::GitLabFetcher; use fetchkit::{ ArXivFetcher, DefaultFetcher, DnsPolicy, DocsSiteFetcher, FetchError, FetchOptions, FetchRequest, Fetcher, GitHubCodeFetcher, GitHubIssueFetcher, GitHubRepoFetcher, @@ -102,6 +103,35 @@ fn options_with(transport: Arc) -> FetchOptions { } } +#[tokio::test] +async fn gitlab_fetcher_enforces_policy_on_api_subrequest() { + let mock = Arc::new(MockTransport::new().route( + "gitlab.com/api/v4/projects/group%2Fproject", + 200, + "application/json", + br#"{"name":"project"}"#, + )); + let options = FetchOptions { + allow_prefixes: vec!["http://gitlab.com/group/project".into()], + block_prefixes: vec!["https://gitlab.com/api/v4".into()], + allowed_ports: vec![80], + dns_policy: DnsPolicy::allow_all(), + transport: Some(mock.clone()), + ..Default::default() + }; + + let error = GitLabFetcher::new() + .fetch( + &FetchRequest::new("http://gitlab.com/group/project"), + &options, + ) + .await + .unwrap_err(); + + assert!(matches!(error, FetchError::BlockedUrl)); + assert!(mock.calls().is_empty()); +} + #[tokio::test] async fn default_fetcher_get_uses_injected_transport() { let mock = Arc::new(MockTransport::new().route( diff --git a/specs/fetchers.md b/specs/fetchers.md index 1034836..1fd01a1 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. Re-applies URL policy to any specialized fetcher API URL before transport execution +7. Provides `fetch_to_file()` that dispatches to matched fetcher's `fetch_to_file()` ### Built-in Fetchers @@ -222,8 +223,9 @@ All fetchers perform their outbound HTTP exclusively through a pluggable `HttpTransport` (see `transport.rs`). The transport is a single-hop socket adapter: 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; -only the socket-level send is delegated. +manual per-hop redirect following, specialized-fetcher API subrequest policy +checks, bot-auth signing, and body-size/timeout caps; only the socket-level send +is delegated. `FetchOptions.transport` selects the implementation (`None` => default `ReqwestTransport`). A host application can supply its own transport to route