Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/fetchkit/src/fetchers/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,8 @@ pub(crate) async fn transport_request(
pin_host: &str,
pin_port: u16,
) -> Result<TransportResponse, FetchError> {
super::validate_policy_url(&url, options)?;

let transport = options.transport();
let transport_method = if method == reqwest::Method::HEAD {
TransportMethod::Head
Expand Down
30 changes: 30 additions & 0 deletions crates/fetchkit/src/fetchers/github_actions_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,36 @@ mod tests {
}
}

#[tokio::test]
async fn enforces_policy_on_api_requests_before_transport() {
let fetcher = GitHubActionsRunFetcher::new();
let request = FetchRequest::new("http://github.com/o/r/actions/runs/123");
let options = FetchOptions {
allow_prefixes: vec!["http://github.com/o/r/actions/runs".to_string()],
block_prefixes: vec!["https://api.github.com/".to_string()],
allowed_ports: vec![80],
blocked_hosts: vec!["api.github.com".to_string()],
dns_policy: crate::DnsPolicy::allow_all(),
transport: Some(std::sync::Arc::new(PanicTransport)),
..Default::default()
};

let error = fetcher.fetch(&request, &options).await.unwrap_err();
assert!(matches!(error, FetchError::BlockedUrl));
}

struct PanicTransport;

#[async_trait::async_trait]
impl crate::transport::HttpTransport for PanicTransport {
async fn execute(
&self,
_req: crate::transport::TransportRequest,
) -> Result<crate::transport::TransportResponse, crate::transport::TransportError> {
panic!("policy must block before transport execution");
}
}

#[test]
fn formats_run_jobs_and_failed_steps() {
let run = WorkflowRun {
Expand Down
54 changes: 30 additions & 24 deletions crates/fetchkit/src/fetchers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_policy_url(&parsed_url, options)?;

for fetcher in &self.fetchers {
if fetcher.matches(&parsed_url) {
Expand Down Expand Up @@ -304,6 +281,35 @@ async fn preflight_save_path<'a>(
// before comparison, preventing bypasses via encoding, case, or trailing dots.
// THREAT[TM-INPUT-007]: Compares parsed URL components (scheme, host, path) instead of
// raw strings, so "http://internal.example.com" won't match "http://internal.example.com.evil.com".
pub(crate) fn validate_policy_url(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 not in allow list");
return Err(FetchError::BlockedUrl);
}
}

if options
.block_prefixes
.iter()
.any(|prefix| url_matches_policy_prefix(url, prefix))
{
debug!(%url, "URL matched block list");
return Err(FetchError::BlockedUrl);
}

Ok(())
}

fn url_matches_policy_prefix(url: &Url, prefix: &str) -> bool {
let Ok(prefix_url) = Url::parse(prefix) else {
tracing::warn!(
Expand Down
Loading