diff --git a/lib/crates/fabro-config/src/resolve/server.rs b/lib/crates/fabro-config/src/resolve/server.rs index d9d479ebb..5d4f5a439 100644 --- a/lib/crates/fabro-config/src/resolve/server.rs +++ b/lib/crates/fabro-config/src/resolve/server.rs @@ -1,5 +1,3 @@ -use std::net::IpAddr; - use fabro_types::settings::InterpString; use fabro_types::settings::server::{ DiscordIntegrationSettings, GithubIntegrationSettings, IntegrationWebhooksLayer, @@ -15,7 +13,6 @@ use fabro_types::settings::server::{ TlsConfig, }; use fabro_util::Home; -use ipnet::IpNet; use super::{ResolveError, default_interp, parse_socket_addr, require_interp}; @@ -257,8 +254,8 @@ fn resolve_ip_allow_entry( return None; } - match parse_ip_net(entry) { - Ok(net) => Some(IpAllowEntry::Literal(net)), + match IpAllowEntry::parse_literal(entry) { + Ok(parsed) => Some(parsed), Err(reason) => { errors.push(ResolveError::ParseFailure { path: path.to_string(), @@ -269,13 +266,6 @@ fn resolve_ip_allow_entry( } } -fn parse_ip_net(value: &str) -> Result { - value - .parse::() - .or_else(|_| value.parse::().map(IpNet::from)) - .map_err(|error| error.to_string()) -} - fn validate_ip_allowlist_for_listen( listen: &ServerListenSettings, ip_allowlist: &ServerIpAllowlistSettings, diff --git a/lib/crates/fabro-server/src/ip_allowlist.rs b/lib/crates/fabro-server/src/ip_allowlist.rs index a94e260f1..aaa7ce4d2 100644 --- a/lib/crates/fabro-server/src/ip_allowlist.rs +++ b/lib/crates/fabro-server/src/ip_allowlist.rs @@ -279,18 +279,16 @@ async fn expand_ip_allow_entries( .iter() .any(|entry| matches!(entry, IpAllowEntry::GitHubMetaHooks)) { - Some(github_meta_resolver.resolve_hooks().await?) + github_meta_resolver.resolve_hooks().await? } else { - None + Vec::new() }; let mut expanded = Vec::new(); for entry in entries { match entry { IpAllowEntry::Literal(net) => expanded.push(*net), - IpAllowEntry::GitHubMetaHooks => { - expanded.extend(github_hooks.clone().unwrap_or_default()); - } + IpAllowEntry::GitHubMetaHooks => expanded.extend(github_hooks.iter().copied()), } } @@ -512,6 +510,36 @@ mod tests { assert!(config.allowlist.contains(&"192.30.252.42".parse().unwrap())); } + #[tokio::test] + async fn middleware_reads_client_ip_from_x_forwarded_for_when_trusted_proxy_count_is_set() { + let config = Arc::new(IpAllowlistConfig { + allowlist: IpAllowlist::new(vec!["10.0.0.0/8".parse().unwrap()]), + trusted_proxy_count: 1, + }); + let app = Router::new() + .route("/api/v1/runs", get(|| async { StatusCode::OK })) + .layer(middleware::from_fn_with_state( + Arc::clone(&config), + ip_allowlist_middleware, + )); + + let allowed_request = Request::builder() + .uri("/api/v1/runs") + .header("x-forwarded-for", "10.0.0.1, 198.51.100.1") + .body(Body::empty()) + .unwrap(); + let allowed_response = app.clone().oneshot(allowed_request).await.unwrap(); + assert_eq!(allowed_response.status(), StatusCode::OK); + + let blocked_request = Request::builder() + .uri("/api/v1/runs") + .header("x-forwarded-for", "203.0.113.1, 198.51.100.1") + .body(Body::empty()) + .unwrap(); + let blocked_response = app.oneshot(blocked_request).await.unwrap(); + assert_eq!(blocked_response.status(), StatusCode::FORBIDDEN); + } + #[tokio::test] async fn middleware_allows_health_and_blocks_non_allowlisted_requests() { let config = Arc::new(IpAllowlistConfig {