From 1e0321616132d468aec28e75868dbbed01c36085 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 19 Apr 2026 09:31:41 -0400 Subject: [PATCH] feat(server): add IP allowlist middleware with GitHub webhook support Introduces a configurable IP allowlist applied to the main API router and the GitHub webhook listener. Supports CIDR literals plus a `github_meta_hooks` keyword that resolves live against GitHub's meta API for the webhooks override. Adds trusted-proxy handling for X-Forwarded-For, validation that rejects Unix socket listeners without a trusted proxy count, and deep-merge logic for the new server.ip_allowlist and per-integration override layers. --- Cargo.lock | 6 + lib/crates/fabro-config/Cargo.toml | 1 + lib/crates/fabro-config/src/merge.rs | 101 +++- lib/crates/fabro-config/src/resolve/server.rs | 173 ++++++- .../fabro-config/tests/resolve_server.rs | 165 +++++- lib/crates/fabro-server/Cargo.toml | 1 + .../fabro-server/src/github_webhooks.rs | 82 ++- lib/crates/fabro-server/src/ip_allowlist.rs | 487 ++++++++++++++++++ lib/crates/fabro-server/src/lib.rs | 1 + lib/crates/fabro-server/src/serve.rs | 65 ++- lib/crates/fabro-server/src/server.rs | 14 +- lib/crates/fabro-server/src/tls.rs | 8 +- .../fabro-server/tests/it/api/routing.rs | 66 +++ lib/crates/fabro-server/tests/it/api/tls.rs | 52 +- lib/crates/fabro-types/Cargo.toml | 1 + lib/crates/fabro-types/src/settings/mod.rs | 11 +- lib/crates/fabro-types/src/settings/server.rs | 59 ++- 17 files changed, 1236 insertions(+), 57 deletions(-) create mode 100644 lib/crates/fabro-server/src/ip_allowlist.rs diff --git a/Cargo.lock b/Cargo.lock index 65bff1221..a1804ed1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1655,6 +1655,7 @@ dependencies = [ "dirs", "fabro-types", "fabro-util", + "ipnet", "serde", "serde_json", "strsim 0.11.1", @@ -1952,6 +1953,7 @@ dependencies = [ "httpmock", "hyper", "hyper-util", + "ipnet", "jsonwebtoken", "mime_guess", "multer", @@ -2113,6 +2115,7 @@ dependencies = [ "fabro-model", "fabro-util", "hex", + "ipnet", "serde", "serde_json", "sha2", @@ -3317,6 +3320,9 @@ name = "ipnet" version = "2.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "469fb0b9cefa57e3ef31275ee7cacb78f2fdca44e4765491884a2b119d4eb130" +dependencies = [ + "serde", +] [[package]] name = "iri-string" diff --git a/lib/crates/fabro-config/Cargo.toml b/lib/crates/fabro-config/Cargo.toml index b2d701afc..8034688c0 100644 --- a/lib/crates/fabro-config/Cargo.toml +++ b/lib/crates/fabro-config/Cargo.toml @@ -23,6 +23,7 @@ chrono.workspace = true fabro-types = { path = "../fabro-types" } fabro-util = { path = "../fabro-util" } dirs.workspace = true +ipnet = "2.11.0" serde.workspace = true serde_json.workspace = true strsim = "0.11" diff --git a/lib/crates/fabro-config/src/merge.rs b/lib/crates/fabro-config/src/merge.rs index a72aa2f60..7256c6149 100644 --- a/lib/crates/fabro-config/src/merge.rs +++ b/lib/crates/fabro-config/src/merge.rs @@ -22,8 +22,11 @@ use fabro_types::settings::run::{ StringOrSplice, }; use fabro_types::settings::server::{ - ServerArtifactsLayer, ServerAuthLayer, ServerIntegrationsLayer, ServerLayer, ServerListenLayer, - ServerSchedulerLayer, ServerSlateDbLayer, ServerStorageLayer, ServerWebLayer, + DiscordIntegrationLayer, GithubIntegrationLayer, IntegrationWebhooksLayer, + ServerArtifactsLayer, ServerAuthLayer, ServerIntegrationsLayer, ServerIpAllowlistLayer, + ServerIpAllowlistOverrideLayer, ServerLayer, ServerListenLayer, ServerSchedulerLayer, + ServerSlateDbLayer, ServerStorageLayer, ServerWebLayer, SlackIntegrationLayer, + TeamsIntegrationLayer, }; use fabro_types::settings::workflow::WorkflowLayer; @@ -404,6 +407,11 @@ fn combine_server(lower: ServerLayer, higher: ServerLayer) -> ServerLayer { api: higher.api.or(lower.api), web: merge_option(lower.web, higher.web, combine_server_web), auth: merge_option(lower.auth, higher.auth, combine_server_auth), + ip_allowlist: merge_option( + lower.ip_allowlist, + higher.ip_allowlist, + combine_server_ip_allowlist, + ), storage: merge_option(lower.storage, higher.storage, combine_server_storage), artifacts: merge_option(lower.artifacts, higher.artifacts, combine_server_artifacts), slatedb: merge_option(lower.slatedb, higher.slatedb, combine_server_slatedb), @@ -436,6 +444,16 @@ fn combine_server_auth(lower: ServerAuthLayer, higher: ServerAuthLayer) -> Serve } } +fn combine_server_ip_allowlist( + lower: ServerIpAllowlistLayer, + higher: ServerIpAllowlistLayer, +) -> ServerIpAllowlistLayer { + ServerIpAllowlistLayer { + entries: higher.entries.or(lower.entries), + trusted_proxy_count: higher.trusted_proxy_count.or(lower.trusted_proxy_count), + } +} + fn combine_server_storage( lower: ServerStorageLayer, higher: ServerStorageLayer, @@ -485,10 +503,81 @@ fn combine_server_integrations( higher: ServerIntegrationsLayer, ) -> ServerIntegrationsLayer { ServerIntegrationsLayer { - github: higher.github.or(lower.github), - slack: higher.slack.or(lower.slack), - discord: higher.discord.or(lower.discord), - teams: higher.teams.or(lower.teams), + github: merge_option(lower.github, higher.github, combine_github_integration), + slack: merge_option(lower.slack, higher.slack, combine_slack_integration), + discord: merge_option(lower.discord, higher.discord, combine_discord_integration), + teams: merge_option(lower.teams, higher.teams, combine_teams_integration), + } +} + +fn combine_github_integration( + lower: GithubIntegrationLayer, + higher: GithubIntegrationLayer, +) -> GithubIntegrationLayer { + GithubIntegrationLayer { + enabled: higher.enabled.or(lower.enabled), + strategy: higher.strategy.or(lower.strategy), + app_id: higher.app_id.or(lower.app_id), + client_id: higher.client_id.or(lower.client_id), + slug: higher.slug.or(lower.slug), + permissions: merge_string_map_sticky(lower.permissions, higher.permissions), + webhooks: merge_option( + lower.webhooks, + higher.webhooks, + combine_integration_webhooks, + ), + } +} + +fn combine_integration_webhooks( + lower: IntegrationWebhooksLayer, + higher: IntegrationWebhooksLayer, +) -> IntegrationWebhooksLayer { + IntegrationWebhooksLayer { + strategy: higher.strategy.or(lower.strategy), + ip_allowlist: merge_option( + lower.ip_allowlist, + higher.ip_allowlist, + combine_server_ip_allowlist_override, + ), + } +} + +fn combine_server_ip_allowlist_override( + lower: ServerIpAllowlistOverrideLayer, + higher: ServerIpAllowlistOverrideLayer, +) -> ServerIpAllowlistOverrideLayer { + ServerIpAllowlistOverrideLayer { + entries: higher.entries.or(lower.entries), + trusted_proxy_count: higher.trusted_proxy_count.or(lower.trusted_proxy_count), + } +} + +fn combine_slack_integration( + lower: SlackIntegrationLayer, + higher: SlackIntegrationLayer, +) -> SlackIntegrationLayer { + SlackIntegrationLayer { + enabled: higher.enabled.or(lower.enabled), + default_channel: higher.default_channel.or(lower.default_channel), + } +} + +fn combine_discord_integration( + lower: DiscordIntegrationLayer, + higher: DiscordIntegrationLayer, +) -> DiscordIntegrationLayer { + DiscordIntegrationLayer { + enabled: higher.enabled.or(lower.enabled), + } +} + +fn combine_teams_integration( + lower: TeamsIntegrationLayer, + higher: TeamsIntegrationLayer, +) -> TeamsIntegrationLayer { + TeamsIntegrationLayer { + enabled: higher.enabled.or(lower.enabled), } } diff --git a/lib/crates/fabro-config/src/resolve/server.rs b/lib/crates/fabro-config/src/resolve/server.rs index 6d0be64dd..addd12cc8 100644 --- a/lib/crates/fabro-config/src/resolve/server.rs +++ b/lib/crates/fabro-config/src/resolve/server.rs @@ -1,16 +1,21 @@ +use std::net::IpAddr; + use fabro_types::settings::InterpString; use fabro_types::settings::server::{ - DiscordIntegrationSettings, GithubIntegrationSettings, IntegrationWebhooksSettings, - ObjectStoreLocalLayer, ObjectStoreProvider, ObjectStoreS3Layer, ObjectStoreSettings, - ServerApiLayer, ServerApiSettings, ServerArtifactsLayer, ServerArtifactsSettings, - ServerAuthGithubSettings, ServerAuthLayer, ServerAuthMethod, ServerAuthSettings, - ServerIntegrationsLayer, ServerIntegrationsSettings, ServerLayer, ServerListenLayer, - ServerListenSettings, ServerListenTlsLayer, ServerLoggingSettings, ServerSchedulerSettings, - ServerSettings, ServerSlateDbLayer, ServerSlateDbSettings, ServerStorageLayer, - ServerStorageSettings, ServerWebLayer, ServerWebSettings, SlackIntegrationSettings, - TeamsIntegrationSettings, TlsConfig, + DiscordIntegrationSettings, GithubIntegrationSettings, IntegrationWebhooksLayer, + IntegrationWebhooksSettings, IpAllowEntry, ObjectStoreLocalLayer, ObjectStoreProvider, + ObjectStoreS3Layer, ObjectStoreSettings, ServerApiLayer, ServerApiSettings, + ServerArtifactsLayer, ServerArtifactsSettings, ServerAuthGithubSettings, ServerAuthLayer, + ServerAuthMethod, ServerAuthSettings, ServerIntegrationsLayer, ServerIntegrationsSettings, + ServerIpAllowlistLayer, ServerIpAllowlistOverrideLayer, ServerIpAllowlistOverrideSettings, + ServerIpAllowlistSettings, ServerLayer, ServerListenLayer, ServerListenSettings, + ServerListenTlsLayer, ServerLoggingSettings, ServerSchedulerSettings, ServerSettings, + ServerSlateDbLayer, ServerSlateDbSettings, ServerStorageLayer, ServerStorageSettings, + ServerWebLayer, ServerWebSettings, SlackIntegrationSettings, TeamsIntegrationSettings, + TlsConfig, }; use fabro_util::Home; +use ipnet::IpNet; use super::{ResolveError, default_interp, parse_socket_addr, require_interp}; @@ -19,6 +24,9 @@ pub fn resolve_server(layer: &ServerLayer, errors: &mut Vec) -> Se let (listen, _valid_tls) = resolve_listen(layer.listen.as_ref(), errors); let web = resolve_web(layer.api.as_ref(), layer.web.as_ref()); let auth = resolve_auth(layer.auth.as_ref(), errors); + let ip_allowlist = resolve_ip_allowlist(layer.ip_allowlist.as_ref(), errors); + validate_ip_allowlist_for_listen(&listen, &ip_allowlist, errors); + let integrations = resolve_integrations(layer.integrations.as_ref(), errors); ServerSettings { listen, @@ -27,6 +35,7 @@ pub fn resolve_server(layer: &ServerLayer, errors: &mut Vec) -> Se }, web, auth, + ip_allowlist, storage: storage.clone(), artifacts: resolve_artifacts(layer.artifacts.as_ref(), &storage.root, errors), slatedb: resolve_slatedb(layer.slatedb.as_ref(), &storage.root, errors), @@ -43,7 +52,7 @@ pub fn resolve_server(layer: &ServerLayer, errors: &mut Vec) -> Se .as_ref() .and_then(|logging| logging.level.clone()), }, - integrations: resolve_integrations(layer.integrations.as_ref()), + integrations, } } @@ -149,6 +158,120 @@ fn resolve_auth( } } +fn resolve_ip_allowlist( + layer: Option<&ServerIpAllowlistLayer>, + errors: &mut Vec, +) -> ServerIpAllowlistSettings { + let entries = layer + .and_then(|allowlist| allowlist.entries.as_ref()) + .map(|entries| { + resolve_ip_allow_entries(entries, "server.ip_allowlist.entries", false, errors) + }) + .unwrap_or_default(); + + ServerIpAllowlistSettings { + entries, + trusted_proxy_count: layer + .and_then(|allowlist| allowlist.trusted_proxy_count) + .unwrap_or(0), + } +} + +fn resolve_ip_allowlist_override( + layer: Option<&ServerIpAllowlistOverrideLayer>, + path: &str, + allow_github_meta_hooks: bool, + errors: &mut Vec, +) -> Option { + layer.map(|allowlist| ServerIpAllowlistOverrideSettings { + entries: allowlist.entries.as_ref().map(|entries| { + resolve_ip_allow_entries( + entries, + &format!("{path}.entries"), + allow_github_meta_hooks, + errors, + ) + }), + trusted_proxy_count: allowlist.trusted_proxy_count, + }) +} + +fn resolve_ip_allow_entries( + entries: &[String], + path: &str, + allow_github_meta_hooks: bool, + errors: &mut Vec, +) -> Vec { + entries + .iter() + .enumerate() + .filter_map(|(index, entry)| { + resolve_ip_allow_entry( + entry, + &format!("{path}[{index}]"), + allow_github_meta_hooks, + errors, + ) + }) + .collect() +} + +fn resolve_ip_allow_entry( + entry: &str, + path: &str, + allow_github_meta_hooks: bool, + errors: &mut Vec, +) -> Option { + if entry == IpAllowEntry::GITHUB_META_HOOKS_KEYWORD { + if allow_github_meta_hooks { + return Some(IpAllowEntry::GitHubMetaHooks); + } + + errors.push(ResolveError::Invalid { + path: path.to_string(), + reason: format!( + "`{}` is only valid in server.integrations.github.webhooks.ip_allowlist.entries", + IpAllowEntry::GITHUB_META_HOOKS_KEYWORD + ), + }); + return None; + } + + match parse_ip_net(entry) { + Ok(net) => Some(IpAllowEntry::Literal(net)), + Err(reason) => { + errors.push(ResolveError::ParseFailure { + path: path.to_string(), + reason, + }); + None + } + } +} + +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, + errors: &mut Vec, +) { + if matches!(listen, ServerListenSettings::Unix { .. }) + && !ip_allowlist.entries.is_empty() + && ip_allowlist.trusted_proxy_count == 0 + { + errors.push(ResolveError::Invalid { + path: "server.ip_allowlist.trusted_proxy_count".to_string(), + reason: "must be greater than 0 when using a Unix socket listener with a non-empty IP allowlist".to_string(), + }); + } +} + fn resolve_artifacts( layer: Option<&ServerArtifactsLayer>, storage_root: &InterpString, @@ -255,7 +378,10 @@ fn object_store_default_root(storage_root: &InterpString, domain: &str) -> Inter InterpString::parse(&format!("{root}/objects/{domain}")) } -fn resolve_integrations(layer: Option<&ServerIntegrationsLayer>) -> ServerIntegrationsSettings { +fn resolve_integrations( + layer: Option<&ServerIntegrationsLayer>, + errors: &mut Vec, +) -> ServerIntegrationsSettings { ServerIntegrationsSettings { github: layer .and_then(|integrations| integrations.github.as_ref()) @@ -266,12 +392,9 @@ fn resolve_integrations(layer: Option<&ServerIntegrationsLayer>) -> ServerIntegr client_id: github.client_id.clone(), slug: github.slug.clone(), permissions: github.permissions.clone(), - webhooks: github - .webhooks - .as_ref() - .map(|webhooks| IntegrationWebhooksSettings { - strategy: webhooks.strategy, - }), + webhooks: github.webhooks.as_ref().map(|webhooks| { + resolve_github_webhooks(webhooks, "server.integrations.github.webhooks", errors) + }), }) .unwrap_or_default(), slack: layer @@ -295,3 +418,19 @@ fn resolve_integrations(layer: Option<&ServerIntegrationsLayer>) -> ServerIntegr .unwrap_or_default(), } } + +fn resolve_github_webhooks( + layer: &IntegrationWebhooksLayer, + path: &str, + errors: &mut Vec, +) -> IntegrationWebhooksSettings { + IntegrationWebhooksSettings { + strategy: layer.strategy, + ip_allowlist: resolve_ip_allowlist_override( + layer.ip_allowlist.as_ref(), + &format!("{path}.ip_allowlist"), + true, + errors, + ), + } +} diff --git a/lib/crates/fabro-config/tests/resolve_server.rs b/lib/crates/fabro-config/tests/resolve_server.rs index 0c53c062e..9f40e67f5 100644 --- a/lib/crates/fabro-config/tests/resolve_server.rs +++ b/lib/crates/fabro-config/tests/resolve_server.rs @@ -1,6 +1,6 @@ use fabro_config::parse_settings_layer; use fabro_types::settings::server::{ - GithubIntegrationStrategy, ObjectStoreSettings, ServerListenSettings, + GithubIntegrationStrategy, IpAllowEntry, ObjectStoreSettings, ServerListenSettings, }; use fabro_types::settings::{InterpString, SettingsLayer}; use fabro_util::Home; @@ -213,3 +213,166 @@ disk_cache = true assert!(settings.slatedb.disk_cache); } + +#[test] +fn resolves_empty_ip_allowlist_by_default() { + let settings = fabro_config::resolve_server_from_file(&SettingsLayer::default()) + .expect("empty settings should resolve"); + + assert!(settings.ip_allowlist.entries.is_empty()); + assert_eq!(settings.ip_allowlist.trusted_proxy_count, 0); +} + +#[test] +fn resolves_global_ip_allowlist_entries_and_proxy_count() { + let file = parse( + r#" +_version = 1 + +[server.ip_allowlist] +entries = ["10.0.0.0/8", "2001:db8::/32", "192.0.2.42"] +trusted_proxy_count = 2 +"#, + ); + + let settings = + fabro_config::resolve_server_from_file(&file).expect("server settings should resolve"); + + assert_eq!(settings.ip_allowlist.entries, vec![ + IpAllowEntry::parse_literal("10.0.0.0/8").unwrap(), + IpAllowEntry::parse_literal("2001:db8::/32").unwrap(), + IpAllowEntry::parse_literal("192.0.2.42").unwrap(), + ]); + assert_eq!(settings.ip_allowlist.trusted_proxy_count, 2); +} + +#[test] +fn resolves_github_webhook_ip_allowlist_overlay_with_inheritance() { + let file = parse( + r#" +_version = 1 + +[server.ip_allowlist] +entries = ["10.0.0.0/8"] +trusted_proxy_count = 2 + +[server.integrations.github.webhooks.ip_allowlist] +entries = ["github_meta_hooks"] +"#, + ); + + let settings = + fabro_config::resolve_server_from_file(&file).expect("server settings should resolve"); + let webhook_allowlist = settings + .integrations + .github + .webhooks + .expect("github webhooks settings should resolve") + .ip_allowlist + .expect("github webhook ip allowlist overlay should resolve"); + + assert_eq!( + webhook_allowlist.entries, + Some(vec![IpAllowEntry::GitHubMetaHooks]) + ); + assert_eq!(webhook_allowlist.trusted_proxy_count, None); +} + +#[test] +fn resolves_github_webhook_ip_allowlist_override_proxy_count() { + let file = parse( + r#" +_version = 1 + +[server.ip_allowlist] +entries = ["10.0.0.0/8"] +trusted_proxy_count = 2 + +[server.integrations.github.webhooks.ip_allowlist] +trusted_proxy_count = 3 +"#, + ); + + let settings = + fabro_config::resolve_server_from_file(&file).expect("server settings should resolve"); + let webhook_allowlist = settings + .integrations + .github + .webhooks + .expect("github webhooks settings should resolve") + .ip_allowlist + .expect("github webhook ip allowlist overlay should resolve"); + + assert_eq!(webhook_allowlist.entries, None); + assert_eq!(webhook_allowlist.trusted_proxy_count, Some(3)); +} + +#[test] +fn rejects_invalid_ip_allowlist_entry() { + let file = parse( + r#" +_version = 1 + +[server.ip_allowlist] +entries = ["10.0.0.0/33"] +"#, + ); + + let errors = + fabro_config::resolve_server_from_file(&file).expect_err("invalid CIDR should fail"); + let rendered = errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n"); + + assert!(rendered.contains("server.ip_allowlist.entries[0]")); +} + +#[test] +fn rejects_github_meta_hooks_in_global_scope() { + let file = parse( + r#" +_version = 1 + +[server.ip_allowlist] +entries = ["github_meta_hooks"] +"#, + ); + + let errors = fabro_config::resolve_server_from_file(&file) + .expect_err("github_meta_hooks should be rejected outside github webhooks"); + let rendered = errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n"); + + assert!(rendered.contains("server.ip_allowlist.entries[0]")); +} + +#[test] +fn rejects_unix_socket_allowlist_without_trusted_proxy() { + let file = parse( + r#" +_version = 1 + +[server.listen] +type = "unix" +path = "/tmp/fabro.sock" + +[server.ip_allowlist] +entries = ["10.0.0.0/8"] +"#, + ); + + let errors = fabro_config::resolve_server_from_file(&file) + .expect_err("unix allowlist without trusted proxies should fail"); + let rendered = errors + .iter() + .map(ToString::to_string) + .collect::>() + .join("\n"); + + assert!(rendered.contains("server.ip_allowlist.trusted_proxy_count")); +} diff --git a/lib/crates/fabro-server/Cargo.toml b/lib/crates/fabro-server/Cargo.toml index 698818086..c5d10f4a7 100644 --- a/lib/crates/fabro-server/Cargo.toml +++ b/lib/crates/fabro-server/Cargo.toml @@ -78,6 +78,7 @@ semver.workspace = true walkdir.workspace = true multer = "3" thiserror.workspace = true +ipnet = "2.11.0" [build-dependencies] chrono = { workspace = true } diff --git a/lib/crates/fabro-server/src/github_webhooks.rs b/lib/crates/fabro-server/src/github_webhooks.rs index ca556dc99..97c7fb471 100644 --- a/lib/crates/fabro-server/src/github_webhooks.rs +++ b/lib/crates/fabro-server/src/github_webhooks.rs @@ -1,8 +1,11 @@ -use axum::Router; +use std::net::SocketAddr; +use std::sync::Arc; + use axum::body::Bytes; use axum::extract::State; use axum::http::{HeaderMap, StatusCode}; use axum::routing::post; +use axum::{Router, middleware}; use hmac::{Hmac, Mac}; use sha2::Sha256; use tokio::net::TcpListener; @@ -10,6 +13,8 @@ use tokio::process::Command; use tokio::sync::oneshot; use tracing::{debug, error, info, warn}; +use crate::ip_allowlist::{IpAllowlistConfig, ip_allowlist_middleware}; + type HmacSha256 = Hmac; /// Verify a GitHub webhook HMAC-SHA256 signature. @@ -119,24 +124,34 @@ impl WebhookListener { } /// Spawn the webhook HTTP listener on a random port (127.0.0.1 only). -pub async fn spawn_webhook_listener(secret: Vec) -> anyhow::Result { +pub async fn spawn_webhook_listener( + secret: Vec, + ip_allowlist: Arc, +) -> anyhow::Result { let listener = TcpListener::bind("127.0.0.1:0").await?; let port = listener.local_addr()?.port(); let state = WebhookState { secret }; let router = Router::new() .route("/webhooks/github", post(webhook_handler)) - .with_state(state); + .with_state(state) + .layer(middleware::from_fn_with_state( + ip_allowlist, + ip_allowlist_middleware, + )); let (shutdown_tx, shutdown_rx) = oneshot::channel::<()>(); tokio::spawn(async move { - axum::serve(listener, router) - .with_graceful_shutdown(async { - let _ = shutdown_rx.await; - }) - .await - .ok(); + axum::serve( + listener, + router.into_make_service_with_connect_info::(), + ) + .with_graceful_shutdown(async { + let _ = shutdown_rx.await; + }) + .await + .ok(); }); info!(port = port, "Webhook listener started"); @@ -156,8 +171,9 @@ impl WebhookManager { secret: Vec, app_id: &str, private_key_pem: &str, + ip_allowlist: Arc, ) -> anyhow::Result { - let listener = spawn_webhook_listener(secret).await?; + let listener = spawn_webhook_listener(secret, ip_allowlist).await?; let port = listener.port(); // Enable Tailscale funnel @@ -286,11 +302,19 @@ mod tests { use tower::ServiceExt; use super::*; + use crate::ip_allowlist::{IpAllowlist, IpAllowlistConfig}; fn test_http_client() -> fabro_http::HttpClient { fabro_http::test_http_client().unwrap() } + fn empty_allowlist_config() -> Arc { + Arc::new(IpAllowlistConfig { + allowlist: IpAllowlist::default(), + trusted_proxy_count: 0, + }) + } + // ----------------------------------------------------------------------- // verify_signature // ----------------------------------------------------------------------- @@ -346,6 +370,10 @@ mod tests { Router::new() .route("/webhooks/github", post(webhook_handler)) .with_state(state) + .layer(middleware::from_fn_with_state( + empty_allowlist_config(), + ip_allowlist_middleware, + )) } #[tokio::test] @@ -405,7 +433,9 @@ mod tests { #[tokio::test] async fn spawn_listener_serves_route() { let secret = b"integration-secret"; - let listener = spawn_webhook_listener(secret.to_vec()).await.unwrap(); + let listener = spawn_webhook_listener(secret.to_vec(), empty_allowlist_config()) + .await + .unwrap(); let port = listener.port(); // Valid request should return 200 @@ -433,4 +463,34 @@ mod tests { listener.shutdown(); } + + #[tokio::test] + async fn spawn_listener_blocks_non_allowlisted_ip() { + let secret = b"integration-secret"; + let listener = spawn_webhook_listener( + secret.to_vec(), + Arc::new(IpAllowlistConfig { + allowlist: IpAllowlist::new(vec!["10.0.0.0/8".parse().unwrap()]), + trusted_proxy_count: 0, + }), + ) + .await + .unwrap(); + let port = listener.port(); + + let body = b"{}"; + let sig = compute_signature(secret, body); + + let client = test_http_client(); + let resp = client + .post(format!("http://127.0.0.1:{port}/webhooks/github")) + .header("x-hub-signature-256", sig) + .body(body.to_vec()) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 403); + + listener.shutdown(); + } } diff --git a/lib/crates/fabro-server/src/ip_allowlist.rs b/lib/crates/fabro-server/src/ip_allowlist.rs new file mode 100644 index 000000000..b05d8b70d --- /dev/null +++ b/lib/crates/fabro-server/src/ip_allowlist.rs @@ -0,0 +1,487 @@ +use std::net::{IpAddr, SocketAddr}; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use anyhow::{Context, Result, anyhow, bail}; +use axum::extract::{ConnectInfo, Request, State}; +use axum::http::Request as HttpRequest; +use axum::middleware::Next; +use axum::response::{IntoResponse, Response}; +use fabro_types::settings::server::{ + IpAllowEntry, ServerIpAllowlistOverrideSettings, ServerIpAllowlistSettings, +}; +use fabro_util::Home; +use ipnet::IpNet; +use serde::{Deserialize, Serialize}; +use tracing::warn; + +use crate::ApiError; + +const GITHUB_META_URL: &str = "https://api.github.com/meta"; + +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct IpAllowlist { + entries: Vec, +} + +impl IpAllowlist { + pub fn new(entries: Vec) -> Self { + Self { entries } + } + + pub fn is_empty(&self) -> bool { + self.entries.is_empty() + } + + pub fn contains(&self, ip: &IpAddr) -> bool { + let ip = normalize_ip(*ip); + self.entries.iter().any(|entry| entry.contains(&ip)) + } +} + +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct IpAllowlistConfig { + pub allowlist: IpAllowlist, + pub trusted_proxy_count: u32, +} + +#[derive(Clone)] +pub struct GitHubMetaResolver { + client: HttpClient, + meta_url: String, + cache_path: PathBuf, +} + +impl GitHubMetaResolver { + pub fn new(client: HttpClient, meta_url: String, cache_path: PathBuf) -> Self { + Self { + client, + meta_url, + cache_path, + } + } + + pub fn from_home() -> Result { + Ok(Self::new( + fabro_http::http_client().context("building GitHub meta HTTP client")?, + GITHUB_META_URL.to_string(), + github_meta_cache_path(Home::from_env().root()), + )) + } + + async fn resolve_hooks(&self) -> Result> { + let cached = self.load_cache()?; + let mut request = self + .client + .get(&self.meta_url) + .header("Accept", "application/vnd.github+json") + .header("User-Agent", "fabro"); + + if let Some(etag) = cached.as_ref().and_then(|cache| cache.etag.as_deref()) { + request = request.header("If-None-Match", etag); + } + + let response = request.send().await.context("fetching GitHub /meta")?; + if response.status() == fabro_http::StatusCode::NOT_MODIFIED { + let cached = cached.ok_or_else(|| { + anyhow!("GitHub /meta returned 304 Not Modified but no usable cache was present") + })?; + return parse_ip_nets(&cached.hooks); + } + + if !response.status().is_success() { + bail!("GitHub /meta returned {}", response.status()); + } + + let etag = response + .headers() + .get("etag") + .and_then(|value| value.to_str().ok()) + .map(ToOwned::to_owned); + let payload: GitHubMetaResponse = response + .json() + .await + .context("parsing GitHub /meta response")?; + let hooks = parse_ip_nets(&payload.hooks)?; + self.store_cache(&GitHubMetaCache { + etag, + hooks: payload.hooks, + })?; + Ok(hooks) + } + + fn load_cache(&self) -> Result> { + match std::fs::read(&self.cache_path) { + Ok(contents) => match serde_json::from_slice(&contents) { + Ok(cache) => Ok(Some(cache)), + Err(error) => { + warn!( + path = %self.cache_path.display(), + error = %error, + "Ignoring invalid GitHub meta cache" + ); + Ok(None) + } + }, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(error) => { + Err(error).with_context(|| format!("reading {}", self.cache_path.display())) + } + } + } + + fn store_cache(&self, cache: &GitHubMetaCache) -> Result<()> { + if let Some(parent) = self.cache_path.parent() { + std::fs::create_dir_all(parent) + .with_context(|| format!("creating {}", parent.display()))?; + } + + let contents = serde_json::to_vec(cache).context("serializing GitHub meta cache")?; + std::fs::write(&self.cache_path, contents) + .with_context(|| format!("writing {}", self.cache_path.display()))?; + Ok(()) + } +} + +type HttpClient = fabro_http::HttpClient; + +#[derive(Debug, Deserialize)] +struct GitHubMetaResponse { + hooks: Vec, +} + +#[derive(Debug, Serialize, Deserialize)] +struct GitHubMetaCache { + etag: Option, + hooks: Vec, +} + +pub fn effective_ip_allowlist_settings( + global: &ServerIpAllowlistSettings, + overlay: Option<&ServerIpAllowlistOverrideSettings>, +) -> ServerIpAllowlistSettings { + let Some(overlay) = overlay else { + return global.clone(); + }; + + ServerIpAllowlistSettings { + entries: overlay + .entries + .clone() + .unwrap_or_else(|| global.entries.clone()), + trusted_proxy_count: overlay + .trusted_proxy_count + .unwrap_or(global.trusted_proxy_count), + } +} + +pub async fn resolve_ip_allowlist_config( + global: &ServerIpAllowlistSettings, + overlay: Option<&ServerIpAllowlistOverrideSettings>, + github_meta_resolver: &GitHubMetaResolver, +) -> Result { + let effective = effective_ip_allowlist_settings(global, overlay); + let allowlist = expand_ip_allow_entries(&effective.entries, github_meta_resolver).await?; + + Ok(IpAllowlistConfig { + allowlist: IpAllowlist::new(allowlist), + trusted_proxy_count: effective.trusted_proxy_count, + }) +} + +pub fn extract_client_ip(request: &HttpRequest, trusted_proxy_count: u32) -> Option { + if trusted_proxy_count == 0 { + return request + .extensions() + .get::>() + .map(|connect_info| normalize_ip(connect_info.0.ip())); + } + + let header = request.headers().get("x-forwarded-for")?.to_str().ok()?; + let entries = header + .split(',') + .map(str::trim) + .filter(|entry| !entry.is_empty()) + .collect::>(); + let trusted_proxy_count = trusted_proxy_count as usize; + let client_index = entries.len().checked_sub(trusted_proxy_count + 1)?; + + entries[client_index] + .parse::() + .ok() + .map(normalize_ip) +} + +pub async fn ip_allowlist_middleware( + State(config): State>, + request: Request, + next: Next, +) -> Response { + if config.allowlist.is_empty() || request.uri().path() == "/health" { + return next.run(request).await; + } + + let path = request.uri().path().to_string(); + match extract_client_ip(&request, config.trusted_proxy_count) { + Some(client_ip) if config.allowlist.contains(&client_ip) => next.run(request).await, + Some(client_ip) => { + warn!(client_ip = %client_ip, path = %path, "request rejected: IP not in allowlist"); + ApiError::forbidden().into_response() + } + None => { + warn!(path = %path, "request rejected: IP not in allowlist"); + ApiError::forbidden().into_response() + } + } +} + +async fn expand_ip_allow_entries( + entries: &[IpAllowEntry], + github_meta_resolver: &GitHubMetaResolver, +) -> Result> { + let github_hooks = if entries + .iter() + .any(|entry| matches!(entry, IpAllowEntry::GitHubMetaHooks)) + { + Some(github_meta_resolver.resolve_hooks().await?) + } else { + None + }; + + 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()); + } + } + } + + Ok(expanded) +} + +fn parse_ip_nets(values: &[String]) -> Result> { + values + .iter() + .map(|value| { + value + .parse::() + .with_context(|| format!("invalid IP range `{value}` in GitHub /meta hooks")) + }) + .collect() +} + +fn normalize_ip(ip: IpAddr) -> IpAddr { + match ip { + IpAddr::V4(_) => ip, + IpAddr::V6(address) => address + .to_ipv4_mapped() + .map_or(IpAddr::V6(address), IpAddr::V4), + } +} + +pub fn github_meta_cache_path(home: &Path) -> PathBuf { + home.join("cache/github-meta-hooks.json") +} + +#[cfg(test)] +mod tests { + use std::net::Ipv4Addr; + + use axum::body::Body; + use axum::http::{Request, StatusCode}; + use axum::routing::get; + use axum::{Router, middleware}; + use httpmock::MockServer; + use tower::ServiceExt; + + use super::*; + + fn literal(value: &str) -> IpAllowEntry { + IpAllowEntry::parse_literal(value).unwrap() + } + + #[test] + fn effective_scope_inherits_global_fields_and_prefers_override_values() { + let global = ServerIpAllowlistSettings { + entries: vec![literal("10.0.0.0/8")], + trusted_proxy_count: 1, + }; + let overlay = ServerIpAllowlistOverrideSettings { + entries: Some(vec![IpAllowEntry::GitHubMetaHooks]), + trusted_proxy_count: None, + }; + + let effective = effective_ip_allowlist_settings(&global, Some(&overlay)); + + assert_eq!(effective.entries, vec![IpAllowEntry::GitHubMetaHooks]); + assert_eq!(effective.trusted_proxy_count, 1); + } + + #[test] + fn extract_client_ip_uses_connect_info_without_trusted_proxies() { + let request = Request::builder() + .uri("/api/v1/runs") + .body(Body::empty()) + .unwrap(); + let mut request = request; + request + .extensions_mut() + .insert(ConnectInfo(SocketAddr::from(( + Ipv4Addr::new(192, 0, 2, 42), + 8080, + )))); + + assert_eq!( + extract_client_ip(&request, 0), + Some(IpAddr::V4(Ipv4Addr::new(192, 0, 2, 42))) + ); + } + + #[test] + fn extract_client_ip_uses_rightmost_minus_trusted_proxy_count_from_x_forwarded_for() { + let request = Request::builder() + .uri("/api/v1/runs") + .header( + "x-forwarded-for", + "198.51.100.10, 203.0.113.20, 203.0.113.30", + ) + .body(Body::empty()) + .unwrap(); + + assert_eq!( + extract_client_ip(&request, 2), + Some(IpAddr::V4(Ipv4Addr::new(198, 51, 100, 10))) + ); + } + + #[test] + fn extract_client_ip_fails_closed_when_x_forwarded_for_chain_is_too_short() { + let request = Request::builder() + .uri("/api/v1/runs") + .header("x-forwarded-for", "198.51.100.10") + .body(Body::empty()) + .unwrap(); + + assert_eq!(extract_client_ip(&request, 1), None); + } + + #[test] + fn ip_allowlist_matches_ipv4_mapped_ipv6_addresses_against_ipv4_ranges() { + let allowlist = IpAllowlist::new(vec!["10.0.0.0/8".parse().unwrap()]); + + assert!(allowlist.contains(&"::ffff:10.1.2.3".parse().unwrap())); + } + + #[tokio::test] + async fn resolve_ip_allowlist_config_expands_github_meta_hooks() { + let mock_server = MockServer::start_async().await; + mock_server + .mock_async(|when, then| { + when.method("GET").path("/meta"); + then.status(200) + .header("content-type", "application/json") + .header("etag", "\"meta-v1\"") + .body(r#"{"hooks":["192.30.252.0/22","185.199.108.0/22"]}"#); + }) + .await; + + let resolver = GitHubMetaResolver::new( + fabro_http::test_http_client().unwrap(), + format!("{}/meta", mock_server.url("")), + tempfile::tempdir().unwrap().path().join("github-meta.json"), + ); + let global = ServerIpAllowlistSettings { + entries: vec![IpAllowEntry::GitHubMetaHooks], + trusted_proxy_count: 1, + }; + + let config = resolve_ip_allowlist_config(&global, None, &resolver) + .await + .unwrap(); + + assert!(config.allowlist.contains(&"192.30.252.45".parse().unwrap())); + assert!(config.allowlist.contains(&"185.199.109.1".parse().unwrap())); + assert_eq!(config.trusted_proxy_count, 1); + } + + #[tokio::test] + async fn resolve_ip_allowlist_config_reuses_cached_github_meta_on_not_modified() { + let mock_server = MockServer::start_async().await; + mock_server + .mock_async(|when, then| { + when.method("GET") + .path("/meta") + .header("if-none-match", "\"meta-v1\""); + then.status(304); + }) + .await; + + let cache_dir = tempfile::tempdir().unwrap(); + std::fs::write( + cache_dir.path().join("github-meta.json"), + r#"{"etag":"\"meta-v1\"","hooks":["192.30.252.0/22"]}"#, + ) + .unwrap(); + + let resolver = GitHubMetaResolver::new( + fabro_http::test_http_client().unwrap(), + format!("{}/meta", mock_server.url("")), + cache_dir.path().join("github-meta.json"), + ); + let global = ServerIpAllowlistSettings { + entries: vec![IpAllowEntry::GitHubMetaHooks], + trusted_proxy_count: 0, + }; + + let config = resolve_ip_allowlist_config(&global, None, &resolver) + .await + .unwrap(); + + assert!(config.allowlist.contains(&"192.30.252.42".parse().unwrap())); + } + + #[tokio::test] + async fn middleware_allows_health_and_blocks_non_allowlisted_requests() { + let config = Arc::new(IpAllowlistConfig { + allowlist: IpAllowlist::new(vec!["10.0.0.0/8".parse().unwrap()]), + trusted_proxy_count: 0, + }); + let app = Router::new() + .route("/health", get(|| async { StatusCode::OK })) + .route("/api/v1/runs", get(|| async { StatusCode::OK })) + .layer(middleware::from_fn_with_state( + Arc::clone(&config), + ip_allowlist_middleware, + )); + + let health_response = app + .clone() + .oneshot(request_with_connect_info( + "/health", + IpAddr::V4(Ipv4Addr::new(203, 0, 113, 10)), + )) + .await + .unwrap(); + assert_eq!(health_response.status(), StatusCode::OK); + + let blocked_response = app + .oneshot(request_with_connect_info( + "/api/v1/runs", + IpAddr::V4(Ipv4Addr::new(203, 0, 113, 10)), + )) + .await + .unwrap(); + assert_eq!(blocked_response.status(), StatusCode::FORBIDDEN); + } + + fn request_with_connect_info(path: &str, ip: IpAddr) -> Request { + let request = Request::builder().uri(path).body(Body::empty()).unwrap(); + let mut request = request; + request + .extensions_mut() + .insert(ConnectInfo(SocketAddr::new(ip, 8080))); + request + } +} diff --git a/lib/crates/fabro-server/src/lib.rs b/lib/crates/fabro-server/src/lib.rs index a89f61e31..a1547dc86 100644 --- a/lib/crates/fabro-server/src/lib.rs +++ b/lib/crates/fabro-server/src/lib.rs @@ -10,6 +10,7 @@ mod demo; pub mod diagnostics; pub mod error; pub mod github_webhooks; +pub mod ip_allowlist; pub mod jwt_auth; mod run_manifest; pub mod security_headers; diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index 6f87f03e1..79f089452 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -1,3 +1,4 @@ +use std::net::SocketAddr; use std::path::{Path, PathBuf}; use std::sync::{Arc, RwLock}; use std::time::Duration; @@ -25,6 +26,7 @@ use tracing::{error, info, warn}; use crate::bind::{self, Bind, BindRequest}; use crate::github_webhooks::WebhookManager; +use crate::ip_allowlist::{GitHubMetaResolver, resolve_ip_allowlist_config}; use crate::jwt_auth::resolve_auth_mode_with_lookup; use crate::server::{ AppStateConfig, RouterOptions, build_app_state, build_router_with_options, @@ -329,6 +331,7 @@ where (auth_mode, max_concurrent_runs) }; let web_enabled = router_web_enabled(&resolved_server_settings); + let github_meta_resolver = GitHubMetaResolver::from_home()?; let (object_store, slatedb_prefix, flush_interval, disk_cache) = build_slatedb_store(&resolved_server_settings)?; @@ -366,8 +369,21 @@ where ); } spawn_scheduler(Arc::clone(&state)); - let router = - build_router_with_options(Arc::clone(&state), auth_mode, RouterOptions { web_enabled }); + let default_ip_allowlist = Arc::new( + resolve_ip_allowlist_config( + &resolved_server_settings.ip_allowlist, + None, + &github_meta_resolver, + ) + .await + .context("resolving server IP allowlist")?, + ); + let router = build_router_with_options( + Arc::clone(&state), + auth_mode, + Arc::clone(&default_ip_allowlist), + RouterOptions { web_enabled }, + ); // Optionally start webhook listener let webhook_manager = match resolved_server_settings.integrations.github.strategy { @@ -396,16 +412,40 @@ where if let (Some(secret), Some(fabro_github::GitHubCredentials::App(github_app))) = (secret, github_app) { - match WebhookManager::start( - secret.into_bytes(), - &app_id, - &github_app.private_key_pem, + match resolve_ip_allowlist_config( + &resolved_server_settings.ip_allowlist, + resolved_server_settings + .integrations + .github + .webhooks + .as_ref() + .and_then(|webhooks| webhooks.ip_allowlist.as_ref()), + &github_meta_resolver, ) .await { - Ok(manager) => Some(manager), + Ok(webhook_ip_allowlist) => { + let webhook_ip_allowlist = Arc::new(webhook_ip_allowlist); + match WebhookManager::start( + secret.into_bytes(), + &app_id, + &github_app.private_key_pem, + webhook_ip_allowlist, + ) + .await + { + Ok(manager) => Some(manager), + Err(err) => { + error!(error = %err, "Failed to start webhook listener"); + None + } + } + } Err(err) => { - error!(error = %err, "Failed to start webhook listener"); + error!( + error = %err, + "Failed to resolve GitHub webhook IP allowlist" + ); None } } @@ -546,9 +586,12 @@ where .await?; } else { announce_server_ready(&bind_addr, styles); - axum::serve(listener, router) - .with_graceful_shutdown(wait_for_shutdown(shutdown_rx.clone())) - .await?; + axum::serve( + listener, + router.into_make_service_with_connect_info::(), + ) + .with_graceful_shutdown(wait_for_shutdown(shutdown_rx.clone())) + .await?; } } } diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 824978703..dd7a3183c 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -110,6 +110,7 @@ use ulid::Ulid; use crate::bind::Bind; use crate::error::ApiError; +use crate::ip_allowlist::{IpAllowlistConfig, ip_allowlist_middleware}; use crate::jwt_auth::{ AuthMode, AuthenticatedService, AuthenticatedSubject, authenticate_service_parts, }; @@ -866,7 +867,12 @@ fn start_optional_slack_service(state: &Arc) { /// Build the axum Router with all run endpoints and embedded static assets. pub fn build_router(state: Arc, auth_mode: AuthMode) -> Router { - build_router_with_options(state, auth_mode, RouterOptions::default()) + build_router_with_options( + state, + auth_mode, + Arc::new(IpAllowlistConfig::default()), + RouterOptions::default(), + ) } #[derive(Clone, Copy, Debug)] @@ -888,6 +894,7 @@ fn removed_web_route(path: &str) -> bool { pub fn build_router_with_options( state: Arc, auth_mode: AuthMode, + ip_allowlist_config: Arc, options: RouterOptions, ) -> Router { start_optional_slack_service(&state); @@ -977,6 +984,11 @@ pub fn build_router_with_options( )); } + router = router.layer(middleware::from_fn_with_state( + ip_allowlist_config, + ip_allowlist_middleware, + )); + router .layer(middleware::from_fn(security_headers::layer)) .layer(trace_layer) diff --git a/lib/crates/fabro-server/src/tls.rs b/lib/crates/fabro-server/src/tls.rs index e8eafbcfb..07da9e745 100644 --- a/lib/crates/fabro-server/src/tls.rs +++ b/lib/crates/fabro-server/src/tls.rs @@ -4,6 +4,7 @@ use std::pin::Pin; use std::sync::Arc; use anyhow::Context; +use axum::extract::ConnectInfo; use fabro_types::settings::{InterpString, TlsConfig}; use rustls::ServerConfig; use rustls_pki_types::{CertificateDer, PrivateKeyDer}; @@ -75,9 +76,12 @@ where let io = TokioIo::new(tls_stream); - let service = service_fn(move |req: hyper::Request| { + let service = service_fn(move |mut req: hyper::Request| { let mut router = router.clone(); - async move { router.call(req).await } + async move { + req.extensions_mut().insert(ConnectInfo(remote_addr)); + router.call(req).await + } }); if let Err(e) = builder.serve_connection(io, service).await { diff --git a/lib/crates/fabro-server/tests/it/api/routing.rs b/lib/crates/fabro-server/tests/it/api/routing.rs index 6c56b56ef..2923353e0 100644 --- a/lib/crates/fabro-server/tests/it/api/routing.rs +++ b/lib/crates/fabro-server/tests/it/api/routing.rs @@ -1,6 +1,11 @@ +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; +use std::sync::Arc; + use axum::body::{Body, to_bytes}; +use axum::extract::ConnectInfo; use axum::http::{Method, Request, StatusCode}; use fabro_config::parse_settings_layer; +use fabro_server::ip_allowlist::{IpAllowlist, IpAllowlistConfig}; use fabro_server::jwt_auth::AuthMode; use fabro_server::server::{ RouterOptions, build_router, build_router_with_options, create_app_state, @@ -286,6 +291,7 @@ enabled = false let app = build_router_with_options( create_app_state_with_options(settings, 5), AuthMode::Disabled, + Arc::new(IpAllowlistConfig::default()), RouterOptions { web_enabled: false }, ); @@ -344,6 +350,7 @@ enabled = false let app = build_router_with_options( create_app_state_with_options(settings, 5), AuthMode::Disabled, + Arc::new(IpAllowlistConfig::default()), RouterOptions { web_enabled: false }, ); let run_id = "01ARZ3NDEKTSV4RRFFQ69G5FAV"; @@ -358,3 +365,62 @@ enabled = false let response = app.oneshot(request).await.unwrap(); assert_eq!(response.status(), StatusCode::NOT_FOUND); } + +#[tokio::test] +async fn allowlist_blocks_non_allowlisted_api_requests() { + let app = build_router_with_options( + create_app_state(), + AuthMode::Disabled, + Arc::new(IpAllowlistConfig { + allowlist: IpAllowlist::new(vec!["10.0.0.0/8".parse().unwrap()]), + trusted_proxy_count: 0, + }), + RouterOptions::default(), + ); + + let response = app + .oneshot(request_with_connect_info( + "/api/v1/runs", + IpAddr::V4(Ipv4Addr::new(203, 0, 113, 10)), + )) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); +} + +#[tokio::test] +async fn allowlist_exempts_health_checks() { + let app = build_router_with_options( + create_app_state(), + AuthMode::Disabled, + Arc::new(IpAllowlistConfig { + allowlist: IpAllowlist::new(vec!["10.0.0.0/8".parse().unwrap()]), + trusted_proxy_count: 0, + }), + RouterOptions::default(), + ); + + let response = app + .oneshot(request_with_connect_info( + "/health", + IpAddr::V4(Ipv4Addr::new(203, 0, 113, 10)), + )) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); +} + +fn request_with_connect_info(path: &str, ip: IpAddr) -> Request { + let request = Request::builder() + .method("GET") + .uri(path) + .body(Body::empty()) + .unwrap(); + let mut request = request; + request + .extensions_mut() + .insert(ConnectInfo(SocketAddr::new(ip, 8080))); + request +} diff --git a/lib/crates/fabro-server/tests/it/api/tls.rs b/lib/crates/fabro-server/tests/it/api/tls.rs index a666b548d..8b719eeef 100644 --- a/lib/crates/fabro-server/tests/it/api/tls.rs +++ b/lib/crates/fabro-server/tests/it/api/tls.rs @@ -1,7 +1,11 @@ use std::path::{Path, PathBuf}; +use std::sync::Arc; +use fabro_server::ip_allowlist::{IpAllowlist, IpAllowlistConfig}; use fabro_server::jwt_auth::{AuthMode, ConfiguredAuth}; -use fabro_server::server::{build_router, create_app_state}; +use fabro_server::server::{ + RouterOptions, build_router, build_router_with_options, create_app_state, +}; use fabro_server::tls::build_rustls_config; use fabro_types::settings::{InterpString, ServerAuthMethod, TlsConfig}; use tokio::net::TcpListener; @@ -45,6 +49,28 @@ async fn start_tls_server(tls_settings: &TlsConfig, auth_mode: AuthMode) -> std: addr } +async fn start_tls_server_with_allowlist( + tls_settings: &TlsConfig, + auth_mode: AuthMode, + ip_allowlist: Arc, +) -> std::net::SocketAddr { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + + let rustls_config = build_rustls_config(tls_settings).unwrap(); + let tls_acceptor = tokio_rustls::TlsAcceptor::from(rustls_config); + + let state = create_app_state(); + let router = + build_router_with_options(state, auth_mode, ip_allowlist, RouterOptions::default()); + + tokio::spawn(async move { + let _ = fabro_server::tls::serve_tls(listener, tls_acceptor, router).await; + }); + + addr +} + fn build_client(ca_cert_path: &Path) -> fabro_http::HttpClient { let ca_pem = std::fs::read(ca_cert_path).unwrap(); let ca_cert = fabro_http::tls::Certificate::from_pem(&ca_pem).unwrap(); @@ -103,3 +129,27 @@ async fn tls_dev_token_auth_does_not_require_client_cert() { let authorized = client.get(url).bearer_auth(dev_token).send().await.unwrap(); assert_eq!(authorized.status(), 200); } + +#[tokio::test] +async fn tls_ip_allowlist_uses_connect_info() { + install_crypto_provider(); + let pki = fixture_pki(); + let addr = start_tls_server_with_allowlist( + &tls_settings(&pki), + AuthMode::Disabled, + Arc::new(IpAllowlistConfig { + allowlist: IpAllowlist::new(vec!["10.0.0.0/8".parse().unwrap()]), + trusted_proxy_count: 0, + }), + ) + .await; + let client = build_client(&pki.ca_cert); + + let response = client + .get(format!("https://127.0.0.1:{}{}", addr.port(), api("/runs"))) + .send() + .await + .unwrap(); + + assert_eq!(response.status(), 403); +} diff --git a/lib/crates/fabro-types/Cargo.toml b/lib/crates/fabro-types/Cargo.toml index 5684113ff..9c1257f70 100644 --- a/lib/crates/fabro-types/Cargo.toml +++ b/lib/crates/fabro-types/Cargo.toml @@ -24,6 +24,7 @@ fabro-macros = { path = "../fabro-macros" } fabro-model = { path = "../fabro-model" } fabro-util = { path = "../fabro-util" } hex.workspace = true +ipnet = { version = "2.11.0", features = ["serde"] } serde.workspace = true serde_json.workspace = true sha2.workspace = true diff --git a/lib/crates/fabro-types/src/settings/mod.rs b/lib/crates/fabro-types/src/settings/mod.rs index 344505e21..3f839aa72 100644 --- a/lib/crates/fabro-types/src/settings/mod.rs +++ b/lib/crates/fabro-types/src/settings/mod.rs @@ -47,11 +47,12 @@ pub use run::{ }; pub use server::{ DiscordIntegrationSettings, GithubIntegrationSettings, IntegrationWebhooksSettings, - ObjectStoreSettings, ServerApiSettings, ServerArtifactsSettings, ServerAuthGithubSettings, - ServerAuthMethod, ServerAuthSettings, ServerIntegrationsSettings, ServerLayer, - ServerListenSettings, ServerLoggingSettings, ServerSchedulerSettings, ServerSettings, - ServerSlateDbSettings, ServerStorageSettings, ServerWebSettings, SlackIntegrationSettings, - TeamsIntegrationSettings, TlsConfig, + IpAllowEntry, ObjectStoreSettings, ServerApiSettings, ServerArtifactsSettings, + ServerAuthGithubSettings, ServerAuthMethod, ServerAuthSettings, ServerIntegrationsSettings, + ServerIpAllowlistLayer, ServerIpAllowlistOverrideLayer, ServerIpAllowlistOverrideSettings, + ServerIpAllowlistSettings, ServerLayer, ServerListenSettings, ServerLoggingSettings, + ServerSchedulerSettings, ServerSettings, ServerSlateDbSettings, ServerStorageSettings, + ServerWebSettings, SlackIntegrationSettings, TeamsIntegrationSettings, TlsConfig, }; pub use size::{ParseSizeError, Size}; pub use splice_array::{SPLICE_MARKER, SpliceArray, SpliceArrayError}; diff --git a/lib/crates/fabro-types/src/settings/server.rs b/lib/crates/fabro-types/src/settings/server.rs index 42c80ae6e..ba7895e25 100644 --- a/lib/crates/fabro-types/src/settings/server.rs +++ b/lib/crates/fabro-types/src/settings/server.rs @@ -9,6 +9,7 @@ use std::collections::HashMap; use std::net::SocketAddr; use std::time::Duration as StdDuration; +use ipnet::IpNet; use serde::{Deserialize, Serialize, Serializer}; use super::duration::Duration as DurationLayer; @@ -21,6 +22,7 @@ pub struct ServerSettings { pub api: ServerApiSettings, pub web: ServerWebSettings, pub auth: ServerAuthSettings, + pub ip_allowlist: ServerIpAllowlistSettings, pub storage: ServerStorageSettings, pub artifacts: ServerArtifactsSettings, pub slatedb: ServerSlateDbSettings, @@ -112,6 +114,36 @@ pub struct ServerAuthGithubSettings { pub allowed_usernames: Vec, } +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)] +pub struct ServerIpAllowlistSettings { + pub entries: Vec, + pub trusted_proxy_count: u32, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)] +pub struct ServerIpAllowlistOverrideSettings { + pub entries: Option>, + pub trusted_proxy_count: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub enum IpAllowEntry { + Literal(IpNet), + GitHubMetaHooks, +} + +impl IpAllowEntry { + pub const GITHUB_META_HOOKS_KEYWORD: &str = "github_meta_hooks"; + + pub fn parse_literal(value: &str) -> Result { + value + .parse::() + .or_else(|_| value.parse::().map(IpNet::from)) + .map_err(|error| error.to_string()) + .map(Self::Literal) + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct ServerStorageSettings { pub root: InterpString, @@ -229,7 +261,8 @@ pub struct TeamsIntegrationSettings { #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)] pub struct IntegrationWebhooksSettings { - pub strategy: Option, + pub strategy: Option, + pub ip_allowlist: Option, } fn serialize_socket_addr(value: &SocketAddr, serializer: S) -> Result @@ -259,6 +292,8 @@ pub struct ServerLayer { #[serde(default, skip_serializing_if = "Option::is_none")] pub auth: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + pub ip_allowlist: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] pub storage: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub artifacts: Option, @@ -339,6 +374,24 @@ pub struct ServerAuthGithubLayer { pub allowed_usernames: Vec, } +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ServerIpAllowlistLayer { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub entries: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub trusted_proxy_count: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ServerIpAllowlistOverrideLayer { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub entries: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub trusted_proxy_count: Option, +} + /// `[server.storage]` — single managed local disk root. #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] @@ -494,7 +547,9 @@ pub struct TeamsIntegrationLayer { #[serde(deny_unknown_fields)] pub struct IntegrationWebhooksLayer { #[serde(default, skip_serializing_if = "Option::is_none")] - pub strategy: Option, + pub strategy: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub ip_allowlist: Option, } #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)]