diff --git a/crates/gitforge-api/src/middleware.rs b/crates/gitforge-api/src/middleware.rs index 1ad25048..f1f52b79 100644 --- a/crates/gitforge-api/src/middleware.rs +++ b/crates/gitforge-api/src/middleware.rs @@ -19,7 +19,7 @@ const PUBLIC_PATHS: &[&str] = &["/health", "/metrics", "/swagger-ui", "/api-docs /// instance the aggregate counters leak activity to anonymous parties, so /// the default is authenticated access. `/health` stays public either way /// — a liveness probe carries no instance data (F5). -fn telemetry_is_public(raw: Option<&str>) -> bool { +pub(crate) fn telemetry_is_public(raw: Option<&str>) -> bool { matches!(raw, Some(v) if v.trim() == "1") } diff --git a/crates/gitforge-api/src/server.rs b/crates/gitforge-api/src/server.rs index 7c8d701f..c713d14d 100644 --- a/crates/gitforge-api/src/server.rs +++ b/crates/gitforge-api/src/server.rs @@ -103,19 +103,25 @@ impl ApiServer { .allow_headers(Any) }; - // Public routes (no auth required) + // Public routes (no auth required). Telemetry surfaces are added + // only when the operator opted in; otherwise they ride behind the + // auth middleware via the protected router (F5). + let telemetry_public = crate::middleware::telemetry_is_public( + std::env::var("GITFORGE_PUBLIC_TELEMETRY").ok().as_deref(), + ); + let metrics_router = Router::new().route("/metrics", get(metrics_handler)); let health = Router::new().route("/health", get(health_check)); - let metrics = Router::new().route("/metrics", get(metrics_handler)); let swagger = Router::new() .route("/swagger-ui", get(crate::openapi::swagger_ui)) .route("/api-docs/openapi.json", get(crate::openapi::openapi_spec)); - let dashboard = crate::dashboard::dashboard_routes(); let mut public_routes = Router::new(); public_routes = public_routes.merge(health); - public_routes = public_routes.merge(metrics); - public_routes = public_routes.merge(dashboard); public_routes = public_routes.merge(swagger); + if telemetry_public { + let dashboard = crate::dashboard::dashboard_routes(); + public_routes = public_routes.merge(metrics_router.clone()).merge(dashboard); + } // Runner registration is unauthenticated by design (the runner // proves itself with its registration token inside the handler), so // it is the other public write surface and gets its own rate-limit @@ -159,7 +165,8 @@ impl ApiServer { .merge(ssh_key_routes()) .merge(artifact_routes()) .merge(webhook_routes()) - .merge(review_routes()) + .merge(review_routes()); + let protected_routes = protected_routes // Authenticate once at the protected-route boundary. Individual // handlers may still apply resource authorization, but token // parsing and validation must not depend on every handler @@ -168,9 +175,12 @@ impl ApiServer { .layer(Extension(Arc::new(auth.clone()))) .layer(Extension(pool_arc.clone())); - // Metrics layer for automatic request recording + // Telemetry surfaces stay at top level either way; when the + // operator has not opted into public access they are merged in + // behind their own auth-middleware layer (F5). They cannot ride + // the /api-nested protected router — nesting would move them to + // /api/metrics and leave the canonical paths unrouted. let metrics_layer = MetricsLayer::new(metrics_arc.clone()); - let app = public_routes .layer(cors) .layer(metrics_layer) @@ -178,6 +188,17 @@ impl ApiServer { .layer(Extension(metrics_arc)) .merge(auth_routes) .nest("/api", protected_routes); + let app = if telemetry_public { + app + } else { + let dashboard = crate::dashboard::dashboard_routes(); + let telemetry = metrics_router + .merge(dashboard) + .layer(middleware::from_fn(crate::middleware::auth_middleware)) + .layer(Extension(Arc::new(auth.clone()))) + .layer(Extension(pool_arc)); + app.merge(telemetry) + }; Self { router: app, diff --git a/crates/gitforge-api/tests/integration.rs b/crates/gitforge-api/tests/integration.rs index 9f7068d2..d3f53c07 100644 --- a/crates/gitforge-api/tests/integration.rs +++ b/crates/gitforge-api/tests/integration.rs @@ -26,6 +26,12 @@ use gitforge_storage::{Artifact, ArtifactStore}; use std::sync::Arc; use tower::ServiceExt; +/// Serializes the tests that mutate GITFORGE_PUBLIC_TELEMETRY. The +/// decision is read at router-build time from the process env, which is +/// shared by every test thread in this binary — without the lock the +/// default-posture tests race the opt-in test and randomly invert. +static TELEMETRY_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + async fn webhook_fixture( config: serde_json::Value, ) -> ( @@ -142,9 +148,16 @@ async fn test_health_check_endpoint() { #[tokio::test] async fn test_metrics_endpoint() { + // Opt-in telemetry: with GITFORGE_PUBLIC_TELEMETRY unset, /metrics is + // served behind the auth middleware (F5) and must reject anonymous + // access. The wiring boundary (route placement, not just the + // middleware decision) is what this test pins. let pool = Pool::memory().await.unwrap(); + let _guard = TELEMETRY_ENV_LOCK.lock().unwrap(); + unsafe { std::env::remove_var("GITFORGE_PUBLIC_TELEMETRY") }; let server = ApiServer::new("test-secret", pool); let app = server.into_router(); + drop(_guard); // Request metrics let response = app @@ -157,7 +170,7 @@ async fn test_metrics_endpoint() { .await .unwrap(); - assert_eq!(response.status(), StatusCode::OK); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); } #[tokio::test] @@ -675,9 +688,36 @@ async fn test_delete_nonexistent_repo() { #[tokio::test] async fn test_dashboard_endpoint() { + // Default posture: authenticated only (F5 wiring boundary). + let pool = Pool::memory().await.unwrap(); + let _guard = TELEMETRY_ENV_LOCK.lock().unwrap(); + unsafe { std::env::remove_var("GITFORGE_PUBLIC_TELEMETRY") }; + let server = ApiServer::new("test-secret", pool); + let app = server.into_router(); + drop(_guard); + + let response = app + .oneshot( + Request::builder() + .uri("/dashboard") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); +} + +#[tokio::test] +async fn test_dashboard_endpoint_public_opt_in() { + // Operator opt-in keeps both telemetry surfaces public. let pool = Pool::memory().await.unwrap(); + let _guard = TELEMETRY_ENV_LOCK.lock().unwrap(); + unsafe { std::env::set_var("GITFORGE_PUBLIC_TELEMETRY", "1") }; let server = ApiServer::new("test-secret", pool); let app = server.into_router(); + drop(_guard); let response = app .oneshot( @@ -690,6 +730,7 @@ async fn test_dashboard_endpoint() { .unwrap(); assert_eq!(response.status(), StatusCode::OK); + unsafe { std::env::remove_var("GITFORGE_PUBLIC_TELEMETRY") }; } #[tokio::test]