diff --git a/Cargo.lock b/Cargo.lock index ab88afb..29ca5ce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2044,9 +2044,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.40" +version = "0.23.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef86cd5876211988985292b91c96a8f2d298df24e75989a43a3c73f2d4d8168b" +checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" dependencies = [ "once_cell", "ring", @@ -2089,9 +2089,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.103.13" +version = "0.103.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" +checksum = "f3c3cf1d8b1e7d4927e2d154c3fcb02979afb9939629c62cd9048d4f07b60ac2" dependencies = [ "ring", "rustls-pki-types", diff --git a/crates/trapfalld/src/auth.rs b/crates/trapfalld/src/auth.rs index d9102e9..4ff239d 100644 --- a/crates/trapfalld/src/auth.rs +++ b/crates/trapfalld/src/auth.rs @@ -237,17 +237,31 @@ pub async fn change_password( // ── Middleware ────────────────────────────────────────────────────────── +/// Exact public routes (no authentication required). +const PUBLIC_PATHS: &[&str] = + &["/health", "/metrics", "/api/0/config", "/api/0/setup", "/api/0/auth/login", "/api/0/auth/logout"]; + +/// Whether `path` is public. Uses exact matching (never suffix/substring), so +/// project slugs like `setup` or `envelope` cannot bypass authentication. +/// The ingest route `/api/{project_id}/envelope/` is matched by path segments. +pub(crate) fn is_public_path(path: &str) -> bool { + if PUBLIC_PATHS.contains(&path) { + return true; + } + // Ingest: exactly api / {id} / envelope, with an optional trailing slash. + let trimmed = path.strip_suffix('/').unwrap_or(path); + let mut segs = trimmed.split('/'); + matches!( + (segs.next(), segs.next(), segs.next(), segs.next(), segs.next(), segs.next()), + (Some(""), Some("api"), Some(id), Some("envelope"), None, None) if !id.is_empty() + ) +} + /// Auth middleware — extracts session cookie, validates, injects user. pub async fn require_auth(State(state): State, mut req: Request, next: Next) -> Response { // Public routes that don't require authentication let path = req.uri().path(); - let is_public = path.ends_with("/setup") - || path.ends_with("/auth/login") - || path.ends_with("/auth/logout") - || path == "/health" - || path == "/metrics" - || path == "/api/0/config" - || path.contains("/envelope/"); + let is_public = is_public_path(path); if is_public { return next.run(req).await; } @@ -310,3 +324,39 @@ pub fn extract_session_token(headers: &axum::http::HeaderMap) -> Option } None } + +#[cfg(test)] +mod public_path_tests { + use super::is_public_path; + + #[test] + fn exact_public_routes() { + for p in ["/health", "/metrics", "/api/0/config", "/api/0/setup", "/api/0/auth/login", "/api/0/auth/logout"] { + assert!(is_public_path(p), "{p}"); + } + } + + #[test] + fn ingest_route_matched_by_segments() { + assert!(is_public_path("/api/42/envelope/")); + assert!(is_public_path("/api/42/envelope")); + assert!(!is_public_path("/api/0/projects/envelope/issues")); + assert!(!is_public_path("/api/42/envelope/extra")); + assert!(!is_public_path("/api//envelope/")); + assert!(!is_public_path("/x/api/42/envelope/")); + } + + #[test] + fn slug_lookalikes_are_not_public() { + for p in [ + "/api/0/projects/setup", + "/api/0/projects/x/setup", + "/api/0/projects/auth/login", + "/api/0/projects/x/auth/logout", + "/api/0/projects/envelope/issues", + "/api/0/projects/x/health", + ] { + assert!(!is_public_path(p), "{p}"); + } + } +} diff --git a/crates/trapfalld/src/server.rs b/crates/trapfalld/src/server.rs index 2dda053..183ee23 100644 --- a/crates/trapfalld/src/server.rs +++ b/crates/trapfalld/src/server.rs @@ -127,6 +127,44 @@ fn default_slowest_limit() -> i64 { 5 } +/// Slugs that collide with route segments or public paths. +const RESERVED_SLUGS: &[&str] = &[ + "setup", + "envelope", + "login", + "logout", + "auth", + "config", + "health", + "metrics", + "api", + "admin", + "me", + "ws", + "projects", + "issues", + "events", + "rules", + "attachments", + "archive", + "search", +]; + +/// Reject empty, path-like or reserved project slugs. +fn validate_project_slug(slug: &str) -> Result<(), (StatusCode, String)> { + let bad = |m: String| Err((StatusCode::BAD_REQUEST, m)); + if slug.trim().is_empty() { + return bad("Project slug must not be empty".into()); + } + if slug.contains('/') { + return bad("Project slug must not contain '/'".into()); + } + if RESERVED_SLUGS.contains(&slug.to_ascii_lowercase().as_str()) { + return bad(format!("Project slug '{slug}' is reserved; choose a different slug")); + } + Ok(()) +} + /// Build the Axum router. pub fn router(state: AppState) -> Router { // All API routes flat — no .nest() to avoid Axum 0.8 routing quirks. @@ -217,9 +255,10 @@ async fn create_project( _user: AuthenticatedUser, headers: axum::http::HeaderMap, Json(req): Json, -) -> Result<(StatusCode, Json), StatusCode> { +) -> Result<(StatusCode, Json), (StatusCode, String)> { let store = state.store.clone(); let slug = req.slug.unwrap_or_else(|| req.name.to_lowercase().replace(' ', "-")); + validate_project_slug(&slug)?; // Prefer configured `public_url` (TRAPFALL_PUBLIC_URL) for DSN generation. // Fall back to the request Host header so local dev keeps working without // extra config (e.g. user accesses via http://localhost:9090). @@ -229,7 +268,7 @@ async fn create_project( .unwrap_or_else(|| headers.get("host").and_then(|v| v.to_str().ok()).unwrap_or("localhost:9090").to_string()); let project = store.create_project_with_host(&slug, &req.name, &host).await.map_err(|e| { tracing::warn!("Create project failed: {e}"); - StatusCode::CONFLICT + (StatusCode::CONFLICT, "Project could not be created (slug may already exist)".to_string()) })?; Ok((StatusCode::CREATED, Json(project))) } @@ -255,15 +294,19 @@ async fn update_project( State(state): State, Path(slug): Path, Json(req): Json, -) -> Result, StatusCode> { +) -> Result, (StatusCode, String)> { + validate_project_slug(&slug)?; let store = state.store.clone(); let project = store .get_project_by_slug(&slug) .await - .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)? - .ok_or(StatusCode::NOT_FOUND)?; + .map_err(|_| (StatusCode::INTERNAL_SERVER_ERROR, "Internal error".to_string()))? + .ok_or((StatusCode::NOT_FOUND, "Project not found".to_string()))?; if let Some(name) = req.name { - let updated = store.update_project(&project.id, &name).await.map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + let updated = store + .update_project(&project.id, &name) + .await + .map_err(|_| (StatusCode::INTERNAL_SERVER_ERROR, "Internal error".to_string()))?; Ok(Json(updated)) } else { Ok(Json(project)) diff --git a/crates/trapfalld/tests/integration.rs b/crates/trapfalld/tests/integration.rs index 72543e5..16d5291 100644 --- a/crates/trapfalld/tests/integration.rs +++ b/crates/trapfalld/tests/integration.rs @@ -818,3 +818,54 @@ async fn parse_transaction_envelope() { assert_eq!(result.transactions.len(), 1, "expected 1 transaction parsed, got {}", result.transactions.len()); assert_eq!(result.transactions[0].transaction, "GET /api/health"); } + +// ── Auth bypass regression (#329) ────────────────────────────────────── + +async fn status_of(app: &Router, method: &str, uri: &str, cookie: Option<&str>, body: &str) -> StatusCode { + let mut b = Request::builder().method(method).uri(uri).header("content-type", "application/json"); + if let Some(c) = cookie { + b = b.header("cookie", c); + } + app.clone().oneshot(b.body(Body::from(body.to_string())).unwrap()).await.unwrap().status() +} + +#[tokio::test] +// Note: unmatched paths (e.g. /api/0/projects/x/setup) hit the SPA fallback, outside +// route_layer; `is_public_path` unit tests cover those lookalikes. +async fn slug_lookalike_paths_require_auth() { + let app = router(make_state(test_store().await, RateLimiter::default())); + for uri in [ + "/api/0/projects/setup", + "/api/0/projects/envelope/issues", + "/api/0/projects/setup/rules", + "/api/0/projects/envelope/search", + ] { + assert_eq!(status_of(&app, "GET", uri, None, "").await, StatusCode::UNAUTHORIZED, "{uri}"); + } +} + +#[tokio::test] +async fn real_public_routes_stay_reachable() { + let app = router(make_state(test_store().await, RateLimiter::default())); + for uri in ["/health", "/api/0/config", "/api/0/setup"] { + assert_eq!(status_of(&app, "GET", uri, None, "").await, StatusCode::OK, "{uri}"); + } + // Login/logout are reachable (not blocked by auth middleware with 401 "Not authenticated"). + assert_ne!(status_of(&app, "POST", "/api/0/auth/logout", None, "").await, StatusCode::UNAUTHORIZED); +} + +#[tokio::test] +async fn reserved_slug_create_returns_400() { + let app = router(make_state(test_store().await, RateLimiter::default())); + let cookie = setup_and_get_cookie(&app).await; + for slug in ["setup", "envelope", "login", "Auth"] { + let body = format!(r#"{{"name":"P","slug":"{slug}"}}"#); + assert_eq!( + status_of(&app, "POST", "/api/0/projects", Some(&cookie), &body).await, + StatusCode::BAD_REQUEST, + "{slug}" + ); + } + let ok = status_of(&app, "POST", "/api/0/projects", Some(&cookie), r#"{"name":"Fine","slug":"fine"}"#).await; + assert_eq!(ok, StatusCode::CREATED); +} diff --git a/web/package-lock.json b/web/package-lock.json index b17ad41..90ebfd1 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -1556,10 +1556,11 @@ } }, "node_modules/devalue": { - "version": "5.8.1", - "resolved": "https://registry.npmjs.org/devalue/-/devalue-5.8.1.tgz", - "integrity": "sha512-4CXDYRBGqN+57wVJkuXBYmpAVUSg3L6JAQa/DFqm238G73E1wuyc/JhGQJzN7vUf/CMphYau2zXbfWzDR5aTEw==", - "dev": true + "version": "5.9.4", + "resolved": "https://registry.npmjs.org/devalue/-/devalue-5.9.4.tgz", + "integrity": "sha512-sPAT4pztbu6586/hrhOnMKS17IJrvg12mXiSPSS3W5qDeN2RGgvZ0diZCm31dBbnevfVmujNO3IM2wrS4Y2Rhg==", + "dev": true, + "license": "MIT" }, "node_modules/dom-accessibility-api": { "version": "0.6.3", @@ -2481,10 +2482,11 @@ } }, "node_modules/source-map-js": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/source-map-js/-/source-map-js-1.2.1.tgz", - "integrity": "sha512-UXWMKhLOwVKb728IUtQPXxfYU+usdybtUrK/8uGE8CQMvrhOpwvzDBwj0QhSL7MQc7vIsISBG8VQ8+IDQxpfQA==", + "version": "1.2.2", + "resolved": "https://registry.npmjs.org/source-map-js/-/source-map-js-1.2.2.tgz", + "integrity": "sha512-KGj/8Y43x35aZVDtt+J4mK1hoLGHULMYfSkODJNQjNDC3oW1PqPoxMwo0pLUsWM/UEGzON/NxeHywEfNXNP3Vw==", "dev": true, + "license": "BSD-3-Clause", "engines": { "node": ">=0.10.0" } @@ -2800,9 +2802,9 @@ } }, "node_modules/undici": { - "version": "7.29.0", - "resolved": "https://registry.npmjs.org/undici/-/undici-7.29.0.tgz", - "integrity": "sha512-IDxfleLmmbSskfWSUATiN1nfn2rDuvnMOqb5CWR92iIfojA0Ud+ulOAAEQ57LPr9rWmsreUyf5lwyao+7GNNVw==", + "version": "7.30.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.30.0.tgz", + "integrity": "sha512-dkrQXeHSaoamnItlYbmzG0wFYrM0ZwDxCIg0A7aKjTyyhh9svRzCNFEzV+Vm05/yehjCzjDZ31KXfGEjYSztDQ==", "dev": true, "license": "MIT", "engines": {