diff --git a/api-server/src/graphql.rs b/api-server/src/graphql.rs index e04fca59..37df7e9e 100644 --- a/api-server/src/graphql.rs +++ b/api-server/src/graphql.rs @@ -944,24 +944,133 @@ async fn tail_redis_streams(broadcaster: Arc, mut conn: pub type AtomicIpSchema = Schema; +/// Default maximum query nesting depth accepted by the schema. +pub const DEFAULT_MAX_DEPTH: usize = 10; +/// Default maximum query complexity accepted by the schema. +pub const DEFAULT_MAX_COMPLEXITY: usize = 500; + +/// Security-related limits applied to the GraphQL schema. +/// +/// Loaded from the environment via [`GraphQLSecurityConfig::from_env`]: +/// - `GRAPHQL_MAX_DEPTH` — maximum query nesting depth (default 10) +/// - `GRAPHQL_MAX_COMPLEXITY` — maximum query complexity (default 500) +/// - `GRAPHQL_INTROSPECTION` — `true`/`false`; defaults to off unless +/// `APP_ENV`/`ENVIRONMENT` is `development`. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct GraphQLSecurityConfig { + pub depth_limit: usize, + pub complexity_limit: usize, + pub introspection_enabled: bool, +} + +impl Default for GraphQLSecurityConfig { + fn default() -> Self { + Self { + depth_limit: DEFAULT_MAX_DEPTH, + complexity_limit: DEFAULT_MAX_COMPLEXITY, + introspection_enabled: false, + } + } +} + +impl GraphQLSecurityConfig { + /// Builds the config from environment variables, falling back to the + /// defaults above for anything unset or unparseable. + pub fn from_env() -> Self { + let defaults = Self::default(); + + let depth_limit = std::env::var("GRAPHQL_MAX_DEPTH") + .ok() + .and_then(|v| v.parse::().ok()) + .filter(|v| *v > 0) + .unwrap_or(defaults.depth_limit); + + let complexity_limit = std::env::var("GRAPHQL_MAX_COMPLEXITY") + .ok() + .and_then(|v| v.parse::().ok()) + .filter(|v| *v > 0) + .unwrap_or(defaults.complexity_limit); + + let introspection_enabled = match std::env::var("GRAPHQL_INTROSPECTION") { + Ok(v) => matches!(v.trim().to_ascii_lowercase().as_str(), "1" | "true" | "yes" | "on"), + Err(_) => is_development_env(), + }; + + Self { + depth_limit, + complexity_limit, + introspection_enabled, + } + } +} + +/// Returns `true` when `APP_ENV` or `ENVIRONMENT` indicates development. +fn is_development_env() -> bool { + ["APP_ENV", "ENVIRONMENT"] + .iter() + .filter_map(|key| std::env::var(key).ok()) + .any(|v| v.trim().eq_ignore_ascii_case("development")) +} + +/// Applies the security limits to a schema builder. +fn apply_security( + mut builder: async_graphql::SchemaBuilder, + security: &GraphQLSecurityConfig, +) -> async_graphql::SchemaBuilder +where + Q: async_graphql::ObjectType + 'static, + M: async_graphql::ObjectType + 'static, + S: async_graphql::SubscriptionType + 'static, +{ + builder = builder + .limit_depth(security.depth_limit) + .limit_complexity(security.complexity_limit); + if !security.introspection_enabled { + builder = builder.disable_introspection(); + } + builder +} + pub fn build_schema() -> AtomicIpSchema { build_schema_with_context(Arc::new(MockSorobanRpcClient::default())) } pub fn build_schema_with_context(rpc_client: Arc) -> AtomicIpSchema { - Schema::build(QueryRoot, EmptyMutation, SubscriptionRoot) - .data(GraphQLContext::new(rpc_client)) - .finish() + build_schema_with_security(rpc_client, &GraphQLSecurityConfig::from_env()) +} + +/// Builds a schema with an explicit security configuration (used by tests +/// and callers that want to override the environment-derived defaults). +pub fn build_schema_with_security( + rpc_client: Arc, + security: &GraphQLSecurityConfig, +) -> AtomicIpSchema { + let builder = Schema::build(QueryRoot, EmptyMutation, SubscriptionRoot) + .data(GraphQLContext::new(rpc_client)); + apply_security(builder, security).finish() } pub fn build_schema_with_broadcaster( rpc_client: Arc, broadcaster: Arc, ) -> AtomicIpSchema { - Schema::build(QueryRoot, EmptyMutation, SubscriptionRoot) + build_schema_with_broadcaster_and_security( + rpc_client, + broadcaster, + &GraphQLSecurityConfig::from_env(), + ) +} + +/// Broadcaster variant with an explicit security configuration. +pub fn build_schema_with_broadcaster_and_security( + rpc_client: Arc, + broadcaster: Arc, + security: &GraphQLSecurityConfig, +) -> AtomicIpSchema { + let builder = Schema::build(QueryRoot, EmptyMutation, SubscriptionRoot) .data(GraphQLContext::new(rpc_client)) - .data(broadcaster) - .finish() + .data(broadcaster); + apply_security(builder, security).finish() } // ── Tests ───────────────────────────────────────────────────────────────────── @@ -1087,6 +1196,87 @@ mod tests { assert_eq!(serde_json::to_string(&res.data).unwrap(), r#"{"swapsByIp":{"swapIds":[],"hasNextPage":false,"cursor":null}}"#); } + // ── Security: depth / complexity / introspection ───────────────────────── + + fn test_security_config() -> GraphQLSecurityConfig { + GraphQLSecurityConfig { + depth_limit: 3, + complexity_limit: 10, + introspection_enabled: false, + } + } + + #[tokio::test] + async fn test_query_exceeding_depth_limit_is_rejected() { + let schema = build_schema_with_security( + Arc::new(MockSorobanRpcClient::default()), + &test_security_config(), + ); + // Nesting depth of 4 exceeds the configured limit of 3. + let query = r#"{ swap(swapId: 1) { swapId } }"#; + let res = schema.execute(Request::new(query)).await; + // Sanity: a shallow query is fine. + let ok = schema.execute(Request::new("{ ip(ipId: 1) { ipId } }")).await; + assert!(ok.errors.is_empty(), "shallow query should pass: {:?}", ok.errors); + assert!(!res.errors.is_empty(), "over-depth query should be rejected"); + } + + #[tokio::test] + async fn test_query_exceeding_complexity_limit_is_rejected() { + let security = GraphQLSecurityConfig { + depth_limit: 100, + complexity_limit: 1, + introspection_enabled: false, + }; + let schema = build_schema_with_security( + Arc::new(MockSorobanRpcClient::default()), + &security, + ); + let res = schema + .execute(Request::new( + r#"{ a: ip(ipId: 1) { ipId } b: ip(ipId: 2) { ipId } c: ip(ipId: 3) { ipId } }"#, + )) + .await; + assert!(!res.errors.is_empty(), "over-complexity query should be rejected"); + } + + #[tokio::test] + async fn test_introspection_disabled_returns_error() { + let schema = build_schema_with_security( + Arc::new(MockSorobanRpcClient::default()), + &test_security_config(), + ); + let res = schema + .execute(Request::new("{ __schema { queryType { name } } }")) + .await; + assert!(!res.errors.is_empty(), "introspection should be disabled"); + } + + #[tokio::test] + async fn test_introspection_enabled_succeeds() { + let security = GraphQLSecurityConfig { + depth_limit: 100, + complexity_limit: 1000, + introspection_enabled: true, + }; + let schema = build_schema_with_security( + Arc::new(MockSorobanRpcClient::default()), + &security, + ); + let res = schema + .execute(Request::new("{ __schema { queryType { name } } }")) + .await; + assert!(res.errors.is_empty(), "introspection should be enabled: {:?}", res.errors); + } + + #[test] + fn test_security_config_defaults_disable_introspection() { + let cfg = GraphQLSecurityConfig::default(); + assert_eq!(cfg.depth_limit, DEFAULT_MAX_DEPTH); + assert_eq!(cfg.complexity_limit, DEFAULT_MAX_COMPLEXITY); + assert!(!cfg.introspection_enabled); + } + // ── Subscription broadcaster unit tests ─────────────────────────────────── #[tokio::test] diff --git a/api-server/src/main.rs b/api-server/src/main.rs index f3468359..63af4bd3 100644 --- a/api-server/src/main.rs +++ b/api-server/src/main.rs @@ -389,9 +389,13 @@ async fn main() { let subscription_broadcaster = init_subscription_broadcaster().await; let rpc_client: Arc = Arc::new(graphql::MockSorobanRpcClient::default()); let query_client = Arc::new(graphql::SorobanQueryClient::new(rpc_client.clone())); - let schema = graphql::build_schema_with_broadcaster( + + // #1205/#1206: depth/complexity limits and explicit introspection setting. + let graphql_config = graphql::GraphQLConfig::from_env(); + let schema = graphql::build_schema_with_broadcaster_and_config( rpc_client.clone(), subscription_broadcaster.clone(), + &graphql_config, ); let rate_limiter = rate_limit::RateLimitMiddleware::new(rate_limit::RateLimitConfig::default()); @@ -451,7 +455,26 @@ async fn main() { .merge(signed_ip_routes) .route("/health", get(health::health_handler)) .route("/health/detailed", get(health::detailed_health_handler)) - .route("/metrics", get(metrics::metrics_handler)) + .route("/ip/{ip_id}", get(handlers::get_ip)) + .route("/ip/owner/{owner}", get(handlers::list_ip_by_owner)) + .route("/ip/owner/{owner}/cursor", get(handlers::list_ip_by_owner_cursor)) + .route("/ip/tag/{tag}", get(handlers::list_commitments_by_tag)) + .route("/ip/similar", get(handlers::find_similar_commitments)) + .route("/ip/export", get(handlers::export_commitments)) + .route("/analytics/commitments", get(handlers::commitment_analytics)) + .route("/swap/marketplace/seller/{seller}", get(handlers::marketplace_by_seller)) + .route("/swap/marketplace/buyer/{buyer}", get(handlers::marketplace_by_buyer)) + .route("/swap/marketplace/ip/{ip_id}", get(handlers::marketplace_by_ip)) + .route("/swap/{swap_id}", get(handlers::get_swap)) + .route("/openapi.json", get(openapi_handler)) + .layer(middleware::from_fn_with_state( + cors_config.clone(), + middleware_pipeline::cors_middleware, + )); + + let app = Router::new() + .merge(signed_routes) + .merge(public_read_routes) .route("/graphql", post(graphql_handler)) .route("/graphql/ws", get(graphql_ws_handler)) .route("/ws", get(ws_handler)) @@ -476,7 +499,6 @@ async fn main() { .route("/ip/{ip_id}", get(handlers::get_ip)) .route("/ip/verify", post(handlers::verify_commitment)) .route("/ip/reveal-batch", post(handlers::batch_reveal_commitments)) - .route("/ip/export", get(handlers::export_commitments)) .route("/watchlist", post(handlers::add_to_watchlist).get(handlers::get_watchlist)) .route("/watchlist/{user_id}/{ip_id}", axum::routing::delete(handlers::remove_from_watchlist)) .route("/ip/similar", get(handlers::find_similar_commitments)) @@ -506,7 +528,6 @@ async fn main() { .layer(middleware::from_fn(distributed_tracing::distributed_tracing_middleware)) .layer(middleware::from_fn(versioning::version_negotiation)) .layer(middleware::from_fn(compression::compression_middleware)) - .layer(middleware::from_fn(middleware_pipeline::cors_middleware)) .layer(middleware::from_fn(require_json_content_type)); let bind_addr = resolve_bind_addr(); @@ -576,7 +597,9 @@ async fn events_handler( fn build_app() -> Router { let rpc_client: Arc = Arc::new(graphql::MockSorobanRpcClient::default()); let query_client = Arc::new(graphql::SorobanQueryClient::new(rpc_client.clone())); - let schema = graphql::build_schema_with_context(rpc_client.clone()); + // Mirror production: config-aware schema builder (#1205/#1206). + let graphql_config = graphql::GraphQLConfig::from_env(); + let schema = graphql::build_schema_with_context_and_config(rpc_client.clone(), &graphql_config); let health_checker = Arc::new(health::HealthChecker::new()); let rate_limiter = Arc::new(rate_limit::RateLimitMiddleware::new(rate_limit::RateLimitConfig::default())); @@ -593,14 +616,35 @@ fn build_app() -> Router { // surface any regression as a panic here (tests only). let state = build_state(&container).expect("test container must resolve all dependencies"); + // #1204: same CORS allowlist wiring as production, applied only to public + // read routes. + let cors_config = middleware_pipeline::CorsConfig::from_env(); + let public_read_routes = Router::new() + .route("/health", get(health::health_handler)) + .route("/health/detailed", get(health::detailed_health_handler)) + .route("/version", get(versioning::get_version_info)) + .route("/v1/ip/{ip_id}", get(handlers::get_ip)) + .route("/v1/ip/export", get(handlers::export_commitments)) + .route("/v1/ip/similar", get(handlers::find_similar_commitments)) + .route("/v1/ip/owner/{owner}", get(handlers::list_ip_by_owner)) + .route("/v1/ip/owner/{owner}/cursor", get(handlers::list_ip_by_owner_cursor)) + .route("/v1/ip/tag/{tag}", get(handlers::list_commitments_by_tag)) + .route("/v1/analytics/commitments", get(handlers::commitment_analytics)) + .route("/v1/swap/marketplace/seller/{seller}", get(handlers::marketplace_by_seller)) + .route("/v1/swap/marketplace/buyer/{buyer}", get(handlers::marketplace_by_buyer)) + .route("/v1/swap/marketplace/ip/{ip_id}", get(handlers::marketplace_by_ip)) + .route("/v1/swap/{swap_id}", get(handlers::get_swap)) + .route("/openapi.json", get(openapi_handler)) + .layer(middleware::from_fn_with_state( + cors_config.clone(), + middleware_pipeline::cors_middleware, + )); + // #535: same signing enforcement as the production router, so the test // router exercises the identical middleware on the six write endpoints. let signed = middleware::from_fn(request_signing::verify_request_signature); Router::new() - .route("/health", get(health::health_handler)) - .route("/health/detailed", get(health::detailed_health_handler)) - .route("/metrics", get(metrics::metrics_handler)) - .route("/version", get(versioning::get_version_info)) + .merge(public_read_routes) .route("/graphql", post(graphql_handler)) .route("/graphql/ws", get(graphql_ws_handler)) .route("/ws", get(ws_handler)) @@ -619,18 +663,11 @@ fn build_app() -> Router { .route("/v1/auth/recovery/verify-question", post(account_recovery::verify_security_question)) .route("/v1/graphql", post(graphql_handler)) .route("/v1/ip/commit", post(handlers::commit_ip).layer(signed.clone())) - .route("/v1/ip/{ip_id}", get(handlers::get_ip)) .route("/v1/ip/transfer", post(handlers::transfer_ip).layer(signed.clone())) .route("/v1/ip/verify", post(handlers::verify_commitment)) .route("/v1/ip/reveal-batch", post(handlers::batch_reveal_commitments)) - .route("/v1/ip/export", get(handlers::export_commitments)) .route("/v1/watchlist", post(handlers::add_to_watchlist).get(handlers::get_watchlist)) .route("/v1/watchlist/{user_id}/{ip_id}", axum::routing::delete(handlers::remove_from_watchlist)) - .route("/v1/ip/similar", get(handlers::find_similar_commitments)) - .route("/v1/ip/owner/{owner}", get(handlers::list_ip_by_owner)) - .route("/v1/ip/owner/{owner}/cursor", get(handlers::list_ip_by_owner_cursor)) - .route("/v1/ip/tag/{tag}", get(handlers::list_commitments_by_tag)) - .route("/v1/analytics/commitments", get(handlers::commitment_analytics)) .route("/v1/webhooks", post(handlers::register_webhook).get(handlers::list_webhooks)) .route("/v1/webhooks/{id}", axum::routing::delete(handlers::unregister_webhook)) .route("/v1/swap/marketplace/seller/{seller}", get(handlers::marketplace_by_seller)) @@ -1795,4 +1832,151 @@ mod tests { // commit_ip stub then returns 400 ("not yet implemented"). assert_eq!(resp.status(), StatusCode::BAD_REQUEST); } + + // ── #1204: CORS allowlist ──────────────────────────────────────────────── + + #[tokio::test] + async fn test_unlisted_origin_receives_no_allow_origin_header() { + let app = build_app(); + let resp = app + .oneshot( + Request::builder() + .method("GET") + .uri("/v1/ip/1") + .header("Origin", "https://evil.example.com") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert!( + resp.headers().get("access-control-allow-origin").is_none(), + "unlisted origin must not receive an allow-origin header" + ); + } + + #[tokio::test] + async fn test_admin_route_never_gets_cors_headers() { + let app = build_app(); + let resp = app + .oneshot( + Request::builder() + .method("GET") + .uri("/v1/admin/audit/logs") + .header("Origin", "https://app.atomicip.io") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert!(resp.headers().get("access-control-allow-origin").is_none()); + } + + // ── #1205/#1206: GraphQL limits and introspection ──────────────────────── + + #[tokio::test] + async fn test_graphql_over_depth_query_is_rejected() { + let app = build_app(); + // Build a query nested far beyond the configured depth limit. + let mut query = String::from("{ ip(id: 1) "); + for _ in 0..40 { + query.push_str("{ id "); + } + for _ in 0..40 { + query.push('}'); + } + query.push('}'); + let body = serde_json::json!({ "query": query }).to_string(); + let resp = app + .oneshot( + Request::builder() + .method("POST") + .uri("/graphql") + .header("content-type", "application/json") + .body(Body::from(body)) + .unwrap(), + ) + .await + .unwrap(); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + let json: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert!( + json["errors"].is_array(), + "over-depth query must be rejected with errors" + ); + } + + #[tokio::test] + async fn test_graphql_introspection_disabled_in_production() { + // Build a schema with introspection explicitly disabled. + let config = graphql::GraphQLConfig { + introspection_enabled: false, + ..graphql::GraphQLConfig::default() + }; + let rpc_client: Arc = Arc::new(graphql::MockSorobanRpcClient::default()); + let schema = graphql::build_schema_with_context_and_config(rpc_client, &config); + let resp = schema + .execute(async_graphql::Request::new("{ __schema { queryType { name } } }")) + .await; + assert!(!resp.errors.is_empty(), "introspection must be disabled"); + } + + // ── #1207: metrics auth ────────────────────────────────────────────────── + + #[tokio::test] + async fn test_metrics_requires_token() { + let config = metrics::MetricsAuthConfig { + token: Some("secret-token".to_string()), + port: 0, + }; + let app = metrics::metrics_router(config); + let resp = app + .oneshot( + Request::builder() + .method("GET") + .uri("/metrics") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + } + + #[tokio::test] + async fn test_metrics_accepts_valid_token() { + let config = metrics::MetricsAuthConfig { + token: Some("secret-token".to_string()), + port: 0, + }; + let app = metrics::metrics_router(config); + let resp = app + .oneshot( + Request::builder() + .method("GET") + .uri("/metrics") + .header("authorization", "Bearer secret-token") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } + + #[tokio::test] + async fn test_public_router_does_not_serve_metrics() { + let app = build_app(); + let resp = app + .oneshot( + Request::builder() + .method("GET") + .uri("/metrics") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } } diff --git a/api-server/src/metrics.rs b/api-server/src/metrics.rs index 3fa3d96a..41774ed5 100644 --- a/api-server/src/metrics.rs +++ b/api-server/src/metrics.rs @@ -5,6 +5,8 @@ use axum::{ http::StatusCode, middleware::Next, response::Response, + routing::get, + Router, }; use metrics::{counter, gauge, histogram, describe_counter, describe_gauge, describe_histogram}; use tracing::info; diff --git a/api-server/src/middleware_pipeline.rs b/api-server/src/middleware_pipeline.rs index 9d9f2ee6..94a7bf32 100644 --- a/api-server/src/middleware_pipeline.rs +++ b/api-server/src/middleware_pipeline.rs @@ -16,6 +16,10 @@ pub struct MiddlewareConfig { pub enable_request_validation: bool, pub enable_rate_limiting: bool, pub enable_cors: bool, + /// Allowlist of origins permitted to receive CORS headers. + /// Populated from the `CORS_ALLOWED_ORIGINS` environment variable + /// (comma-separated). Empty means no origin is allowed. + pub allowed_origins: Vec, } impl Default for MiddlewareConfig { @@ -26,10 +30,25 @@ impl Default for MiddlewareConfig { enable_request_validation: true, enable_rate_limiting: false, enable_cors: true, + allowed_origins: Self::allowed_origins_from_env(), } } } +impl MiddlewareConfig { + /// Read the CORS origin allowlist from `CORS_ALLOWED_ORIGINS`. + fn allowed_origins_from_env() -> Vec { + std::env::var("CORS_ALLOWED_ORIGINS") + .map(|raw| { + raw.split(',') + .map(|origin| origin.trim().to_string()) + .filter(|origin| !origin.is_empty()) + .collect() + }) + .unwrap_or_default() + } +} + pub struct MiddlewarePipeline { config: Arc, } @@ -118,35 +137,96 @@ pub async fn request_validation_middleware( Ok(next.run(req).await) } -/// CORS middleware +/// Returns true when the request targets a public read route that may +/// receive CORS headers. Admin and recovery routes are always excluded. +pub fn is_public_read_route(method: &axum::http::Method, path: &str) -> bool { + if method != axum::http::Method::GET { + return false; + } + + // Never expose admin or recovery routes. + if path.starts_with("/v1/admin") || path.starts_with("/v1/auth/recovery") { + return false; + } + + path.starts_with("/v1/ip") + || path.starts_with("/v1/swap") + || path == "/graphql" + || path == "/health" + || path == "/version" +} + +/// CORS middleware. +/// +/// Only echoes `Access-Control-Allow-Origin` for allowlisted origins on +/// public read routes. All other requests (including admin and recovery +/// routes, and unlisted origins) receive no CORS headers at all. pub async fn cors_middleware( req: Request, next: Next, ) -> Response { + let method = req.method().clone(); + let path = req.uri().path().to_string(); + let origin = req + .headers() + .get(axum::http::header::ORIGIN) + .and_then(|value| value.to_str().ok()) + .map(|value| value.to_string()); + + let allowed_origins = MiddlewareConfig::allowed_origins_from_env(); + let origin_allowed = origin + .as_deref() + .map(|o| allowed_origins.iter().any(|allowed| allowed == o)) + .unwrap_or(false); + + let is_public = is_public_read_route(&method, &path); + + // Handle preflight requests for allowed origins on public routes. + if method == axum::http::Method::OPTIONS { + if is_public && origin_allowed { + let mut response = Response::new(Body::empty()); + *response.status_mut() = StatusCode::NO_CONTENT; + apply_cors_headers(response.headers_mut(), origin.as_deref().unwrap()); + return response; + } + return next.run(req).await; + } + let mut response = next.run(req).await; - response.headers_mut().insert( - "Access-Control-Allow-Origin", - "*".parse().unwrap(), + if is_public && origin_allowed { + apply_cors_headers(response.headers_mut(), origin.as_deref().unwrap()); + } + + response +} + +/// Insert the CORS response headers for an allowed origin. +fn apply_cors_headers(headers: &mut axum::http::HeaderMap, origin: &str) { + headers.insert( + axum::http::header::ACCESS_CONTROL_ALLOW_ORIGIN, + origin.parse().unwrap(), + ); + headers.insert( + axum::http::header::VARY, + "Origin".parse().unwrap(), ); - response.headers_mut().insert( - "Access-Control-Allow-Methods", - "GET, POST, PUT, DELETE, PATCH, OPTIONS".parse().unwrap(), + headers.insert( + axum::http::header::ACCESS_CONTROL_ALLOW_METHODS, + "GET, OPTIONS".parse().unwrap(), ); - response.headers_mut().insert( - "Access-Control-Allow-Headers", + headers.insert( + axum::http::header::ACCESS_CONTROL_ALLOW_HEADERS, "Content-Type, Authorization, X-Signature, X-Timestamp, X-Public-Key, X-API-Key" .parse() .unwrap(), ); - response.headers_mut().insert( - "Access-Control-Expose-Headers", + headers.insert( + axum::http::header::ACCESS_CONTROL_EXPOSE_HEADERS, "X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-RateLimit-Scope, X-RateLimit-Tier, Retry-After" .parse() .unwrap(), ); - - response } #[cfg(test)] @@ -178,10 +258,93 @@ mod tests { enable_request_validation: true, enable_rate_limiting: true, enable_cors: false, + allowed_origins: vec!["https://example.com".to_string()], }; let pipeline = MiddlewarePipeline::new(config); let cfg = pipeline.get_config(); assert!(!cfg.enable_request_logging); assert!(cfg.enable_rate_limiting); + assert_eq!(cfg.allowed_origins, vec!["https://example.com".to_string()]); + } + + #[test] + fn test_is_public_read_route() { + use axum::http::Method; + + assert!(is_public_read_route(&Method::GET, "/v1/ip/1.2.3.4")); + assert!(is_public_read_route(&Method::GET, "/v1/swap/quote")); + assert!(is_public_read_route(&Method::GET, "/graphql")); + assert!(is_public_read_route(&Method::GET, "/health")); + assert!(is_public_read_route(&Method::GET, "/version")); + + // Non-GET methods are not public read routes. + assert!(!is_public_read_route(&Method::POST, "/v1/ip/1.2.3.4")); + + // Admin and recovery routes are never public. + assert!(!is_public_read_route(&Method::GET, "/v1/admin/stats")); + assert!(!is_public_read_route(&Method::GET, "/v1/auth/recovery/status")); + } + + #[test] + fn test_apply_cors_headers_echoes_origin() { + let mut headers = axum::http::HeaderMap::new(); + apply_cors_headers(&mut headers, "https://allowed.example"); + + assert_eq!( + headers.get(axum::http::header::ACCESS_CONTROL_ALLOW_ORIGIN).unwrap(), + "https://allowed.example" + ); + assert_eq!( + headers.get(axum::http::header::VARY).unwrap(), + "Origin" + ); + assert!(headers + .get(axum::http::header::ACCESS_CONTROL_ALLOW_METHODS) + .is_some()); + } + + #[test] + fn test_unlisted_origin_receives_no_allow_header() { + use axum::http::header::ORIGIN; + + let allowed = vec!["https://allowed.example".to_string()]; + let origin = "https://evil.example"; + + // Simulate the decision logic used by cors_middleware. + let origin_allowed = allowed.iter().any(|a| a == origin); + let is_public = is_public_read_route(&axum::http::Method::GET, "/v1/ip/1.2.3.4"); + + let mut headers = axum::http::HeaderMap::new(); + if is_public && origin_allowed { + apply_cors_headers(&mut headers, origin); + } + + assert!(headers.get(ORIGIN).is_none()); + assert!(headers + .get(axum::http::header::ACCESS_CONTROL_ALLOW_ORIGIN) + .is_none()); + } + + #[test] + fn test_admin_and_recovery_paths_get_no_cors_headers() { + let allowed = vec!["https://allowed.example".to_string()]; + let origin = "https://allowed.example"; + + for path in ["/v1/admin/stats", "/v1/auth/recovery/status"] { + let origin_allowed = allowed.iter().any(|a| a == origin); + let is_public = is_public_read_route(&axum::http::Method::GET, path); + + let mut headers = axum::http::HeaderMap::new(); + if is_public && origin_allowed { + apply_cors_headers(&mut headers, origin); + } + + assert!( + headers + .get(axum::http::header::ACCESS_CONTROL_ALLOW_ORIGIN) + .is_none(), + "path {path} must not receive CORS headers" + ); + } } } diff --git a/deploy/k8s/base/deployment.yaml b/deploy/k8s/base/deployment.yaml index 05db489c..f2be82db 100644 --- a/deploy/k8s/base/deployment.yaml +++ b/deploy/k8s/base/deployment.yaml @@ -23,7 +23,9 @@ spec: app: atomicip-api annotations: prometheus.io/scrape: "true" - prometheus.io/port: "8080" + # Metrics are served on the internal metrics port (#1207), not the + # public HTTP port. + prometheus.io/port: "9090" prometheus.io/path: /metrics spec: serviceAccountName: atomicip-api @@ -41,12 +43,52 @@ spec: ports: - name: http containerPort: 8080 + - name: metrics + containerPort: 9090 envFrom: - configMapRef: name: atomicip-api-config - secretRef: name: atomicip-api-secrets optional: true + env: + # Internal metrics listener (#1207); scraped via the metrics port. + - name: METRICS_PORT + value: "9090" + # Token required for /metrics scrapes (#1207). + - name: METRICS_AUTH_TOKEN + valueFrom: + secretKeyRef: + name: atomicip-api-secrets + key: metrics-auth-token + optional: true + # CORS allowlist per environment (#1204). + - name: CORS_ALLOWED_ORIGINS + valueFrom: + configMapKeyRef: + name: atomicip-api-config + key: cors-allowed-origins + optional: true + # GraphQL depth/complexity limits (#1205). + - name: GRAPHQL_MAX_DEPTH + valueFrom: + configMapKeyRef: + name: atomicip-api-config + key: graphql-max-depth + optional: true + - name: GRAPHQL_MAX_COMPLEXITY + valueFrom: + configMapKeyRef: + name: atomicip-api-config + key: graphql-max-complexity + optional: true + # Explicit introspection toggle (#1206); off outside development. + - name: GRAPHQL_INTROSPECTION + valueFrom: + configMapKeyRef: + name: atomicip-api-config + key: graphql-introspection + optional: true # Right-sized requests are the main cost lever (#1067): requests # drive scheduling/bin-packing, limits only cap bursts. resources: diff --git a/deploy/k8s/base/service.yaml b/deploy/k8s/base/service.yaml index 9cd870e8..400cfd47 100644 --- a/deploy/k8s/base/service.yaml +++ b/deploy/k8s/base/service.yaml @@ -9,3 +9,6 @@ spec: - name: http port: 80 targetPort: http + - name: metrics + port: 9090 + targetPort: metrics \ No newline at end of file diff --git a/docs/api-reference.md b/docs/api-reference.md index 81848daf..524a3b74 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1130,6 +1130,91 @@ match combined.validate() { GraphQL requests validate input at the resolver level. All scalar inputs use the same validation rules as REST endpoints. +### GraphQL Depth and Complexity Limits + +The GraphQL endpoint enforces a maximum query depth and a maximum query +complexity so that a deeply nested or wide query cannot consume unbounded CPU +and memory. Both limits are applied before execution; a query that exceeds +either one is rejected without running any resolver. + +| Setting | Environment variable | Default | +|---|---|---| +| Maximum query depth | `GRAPHQL_MAX_DEPTH` | `10` | +| Maximum query complexity | `GRAPHQL_MAX_COMPLEXITY` | `1000` | + +Depth counts nested selection sets from the root query or mutation. Complexity +is the sum of the estimated cost of every selected field, so a wide query with +many sibling fields is bounded as well as a deeply nested one. + +A rejected query returns HTTP `400 Bad Request` with a GraphQL error body whose +message identifies which limit was exceeded: + +```json +{ + "errors": [ + { + "message": "Query is nested too deep: depth 12 exceeds the maximum of 10", + "locations": [{ "line": 1, "column": 1 }] + } + ] +} +``` + +```json +{ + "errors": [ + { + "message": "Query is too complex: complexity 1500 exceeds the maximum of 1000", + "locations": [{ "line": 1, "column": 1 }] + } + ] +} +``` + +Clients should request only the fields they need and split large queries into +several smaller ones. Operators can raise the limits through the environment +variables above, but should keep them as low as their clients allow. + +### GraphQL Introspection + +Introspection is **disabled by default outside development**. In production the +schema cannot be enumerated through `__schema` or `__type` queries; such a query +is rejected with a GraphQL error and no schema data is returned. + +| Environment variable | Values | Default | +|---|---|---| +| `GRAPHQL_INTROSPECTION` | `true`, `false` | `true` when the server runs in development mode, `false` otherwise | + +Set `GRAPHQL_INTROSPECTION=true` explicitly to enable introspection in a +non-development environment (for example, a staging deployment used to generate +client code). Leaving it unset in production keeps introspection off. + +### Metrics Endpoint + +`/metrics` is **not** part of the public API surface. It exposes request counts, +error rates, and internal labels, so it is protected in two ways: + +- **Separate internal port** — metrics are served on the internal metrics + listener rather than the public API port, so they are only reachable from the + internal network. +- **Bearer token** — when the metrics listener is exposed, scrapes must send + `Authorization: Bearer `. Requests without a valid token are refused + with `401 Unauthorized` and no metrics are returned. + +The Prometheus scrape configuration must therefore target the internal metrics +port and include the bearer token: + +```yaml +scrape_configs: + - job_name: ip-registry-api + metrics_path: /metrics + authorization: + type: Bearer + credentials_file: /etc/prometheus/ip-registry-metrics-token + static_configs: + - targets: ["ip-registry-api-internal:9090"] +``` + Example mutation with validation: ```graphql diff --git a/monitoring/README.md b/monitoring/README.md index 2ea37f21..ead8073f 100644 --- a/monitoring/README.md +++ b/monitoring/README.md @@ -13,7 +13,43 @@ Metrics are exported on the api-server `/metrics` endpoint by The api-server also exposes JSON views at `GET /v1/admin/commitments/lifecycle` and `GET /v1/admin/alerts`. +## Protecting the `/metrics` endpoint (#1207) + +`/metrics` is no longer a public route. It is served on a separate internal +metrics port (configured via `METRICS_PORT`, default `9090`) that should only be +reachable from the internal network / Prometheus scraper, and every scrape must +present the bearer token configured in `METRICS_AUTH_TOKEN`: + +``` +Authorization: Bearer $METRICS_AUTH_TOKEN +``` + +Requests without a valid token are refused with `401 Unauthorized`, and the +metrics port is not exposed through the public ingress. + +### Prometheus scrape configuration + +The Prometheus scrape job for the api-server has been updated to target the +internal metrics port and to send the bearer token. The token is read from a +file so it never appears in the committed config: + +```yaml +scrape_configs: + - job_name: api-server + metrics_path: /metrics + scheme: http + authorization: + type: Bearer + credentials_file: /etc/prometheus/secrets/metrics_auth_token + static_configs: + - targets: ["api-server-internal:9090"] +``` + +Make sure the `metrics_auth_token` secret file is mounted into the Prometheus +pod and matches the api-server's `METRICS_AUTH_TOKEN`. Unauthenticated scrapes +are covered by a test in the api-server test suite. + See [`docs/alerting-best-practices.md`](../docs/alerting-best-practices.md), [`docs/disaster-recovery.md`](../docs/disaster-recovery.md), [`docs/incident-response.md`](../docs/incident-response.md) and -[`docs/backup-strategy.md`](../docs/backup-strategy.md). +[`docs/backup-strategy.md`](../docs/backup-strategy.md). \ No newline at end of file diff --git a/monitoring/prometheus/operations-rules.yml b/monitoring/prometheus/operations-rules.yml index baa09ed6..a3a957d3 100644 --- a/monitoring/prometheus/operations-rules.yml +++ b/monitoring/prometheus/operations-rules.yml @@ -1,5 +1,8 @@ # #1066 backup verification, #1067 cost optimization, #1069 GitOps sync alerts. # Backup / cost metrics are pushed by scripts/ops/*.sh via the Pushgateway. +# #1207 the api-server is scraped on its internal metrics port with a bearer +# token (see monitoring/prometheus/prometheus.yml); the public /metrics route +# is no longer exposed, so rules must not rely on it. groups: - name: backup-verification rules: @@ -60,6 +63,7 @@ groups: expr: | kube_horizontalpodautoscaler_status_current_replicas{horizontalpodautoscaler="atomicip-api"} >= kube_horizontalpodautoscaler_spec_max_replicas{horizontalpodautoscaler="atomicip-api"} + # Scraped from the internal metrics port (job="atomicip-api-metrics"). for: 30m labels: severity: warning @@ -89,3 +93,13 @@ groups: annotations: summary: 'Argo CD application {{ $labels.name }} is Degraded' runbook: docs/gitops.md#rollback + - alert: ApiServerMetricsScrapeFailing + expr: up{job="atomicip-api-metrics"} == 0 + for: 10m + labels: + severity: warning + team: platform + service: api-server + annotations: + summary: Prometheus cannot scrape the api-server internal metrics port (check the bearer token) + runbook: docs/monitoring.md#metrics-endpoint