From 58eccf0a802dd3e22b88996fc79e1bdb9a132702 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 4 Sep 2026 16:38:41 +0200 Subject: [PATCH] fix: strip one guest bearer prefix and bound the raw bearer trim_start_matches stripped every jwt_guest_ prefix, so a repeated-prefix bearer shrank to a valid short token that verified and was then cached under the full oversized bearer key. Strip exactly one prefix, and bound the raw bearer length (the auth cache keys on it) before verifying or caching. The refusal test now mints a valid signed token over the cap (which would otherwise verify, the extra claim ignored) and a repeated-prefix bearer, so it fails if either guard regresses. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01VF3v6LA9399gNphmZaHYG3 --- backend/tests/app_guest_jwt_entry.rs | 21 +++++++++++++++++++-- backend/windmill-api-auth/src/auth.rs | 10 +++++++++- backend/windmill-common/src/guest_jwt.rs | 2 +- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/backend/tests/app_guest_jwt_entry.rs b/backend/tests/app_guest_jwt_entry.rs index 84664fb772..9f39b3a863 100644 --- a/backend/tests/app_guest_jwt_entry.rs +++ b/backend/tests/app_guest_jwt_entry.rs @@ -305,8 +305,24 @@ async fn guest_jwt_refusals(db: Pool) -> anyhow::Result<()> { c.app_path = "u/test-user/*".to_string(); let wildcard_app_path = bearer(&c, PRIV1, Algorithm::ES256); - // a token past the length cap is refused before any signature work or caching. - let oversized_token = format!("jwt_guest_{}", "a".repeat(9000)); + // a valid, signed token past the length cap: without the cap it would deserialize into + // GuestJwtClaims (the extra claim ignored) and verify, so this pins the length check. + let mut payload = serde_json::to_value(Claims::valid()).unwrap(); + payload["padding"] = serde_json::json!("a".repeat(9000)); + let big_jwt = encode( + &Header::new(Algorithm::ES256), + &payload, + &EncodingKey::from_ec_pem(PRIV1.as_bytes()).unwrap(), + ) + .unwrap(); + let oversized_token = format!("jwt_guest_{big_jwt}"); + + // a repeated prefix must not strip down to a valid short token that verifies and is then + // cached under the full bearer key (trim_start_matches would; strip_prefix must not). + let repeated_prefix = format!( + "jwt_guest_{}", + bearer(&Claims::valid(), PRIV1, Algorithm::ES256) + ); for (label, token) in [ ("wrong workspace", wrong_ws), @@ -321,6 +337,7 @@ async fn guest_jwt_refusals(db: Pool) -> anyhow::Result<()> { ("oversized email", oversized_email), ("wildcard app_path", wildcard_app_path), ("oversized token", oversized_token), + ("repeated prefix", repeated_prefix), ] { let resp = whoami(port, ws, &token).send().await?; assert_eq!(resp.status(), 401, "{label} must be refused"); diff --git a/backend/windmill-api-auth/src/auth.rs b/backend/windmill-api-auth/src/auth.rs index 6f88ed9a4e..6d96112ec5 100644 --- a/backend/windmill-api-auth/src/auth.rs +++ b/backend/windmill-api-auth/src/auth.rs @@ -222,7 +222,15 @@ impl AuthCache { let Some(w_id) = w_id.as_deref() else { return None; }; - let jwt = token.trim_start_matches(windmill_common::guest_jwt::BEARER_PREFIX); + // The cache keys on the whole bearer, so bound it here before verify or caching; + // strip exactly one prefix, or repeated prefixes would shrink an oversized + // bearer past the length check while it is still cached at full size. + if token.len() > windmill_common::guest_jwt::MAX_GUEST_JWT_LEN { + return None; + } + let jwt = token + .strip_prefix(windmill_common::guest_jwt::BEARER_PREFIX) + .unwrap_or(token); let claims = match windmill_common::guest_jwt::verify_for_workspace(&self.db, w_id, jwt) .await diff --git a/backend/windmill-common/src/guest_jwt.rs b/backend/windmill-common/src/guest_jwt.rs index 49496ff7db..f84e8394ba 100644 --- a/backend/windmill-common/src/guest_jwt.rs +++ b/backend/windmill-common/src/guest_jwt.rs @@ -274,7 +274,7 @@ const MAX_GUEST_PEM_LEN: usize = 8 * 1024; /// A guest JWT is refused past this before any signature work or caching: the auth cache keys /// on the bearer, so an oversized token (unauthenticated at this point) would otherwise be /// decoded and, if it verified, cached at its full size. A real JWT is well under this. -const MAX_GUEST_JWT_LEN: usize = 8 * 1024; +pub const MAX_GUEST_JWT_LEN: usize = 8 * 1024; /// Fetch a JWKS, keeping only the keys usable here. The URL was set by a workspace /// admin, so it is validated against private ranges and the connect is pinned to the