fix: bound the guest JWT email; serve stale JWKS during refresh

- Cap the email claim at 254 bytes as well as requiring a plain address. An
  over-255-byte email fit the signature but overflowed the guest_activity.email
  column: the activity write and the login_guest audit both failed while the
  guest was still admitted, so a guest could enter uncounted and unaudited. Now
  refused before authentication.
- Serve a stale-but-good JWKS entry while a refresh runs off the request path,
  so a slow or hanging issuer no longer stalls guest requests for the fetch
  timeout at each 15-minute TTL boundary. Only a cold or negative entry blocks,
  still under the single-flight lock; the background refresh no-ops when a fetch
  is already in flight and keeps the stale keys on failure.
- Tests: an oversized email is refused alongside the group-shaped and
  slash-in-email cases.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VF3v6LA9399gNphmZaHYG3

backend/tests/app_guest_jwt_entry.rs

@@ -288,6 +288,12 @@ async fn guest_jwt_refusals(db: Pool<Postgres>) -> anyhow::Result<()> {
     c.email = "u/x@example.com".to_string();
     let slash_in_email = bearer(&c, PRIV1, Algorithm::ES256);
 
+    // an email longer than the `guest_activity.email` column: refused before auth, so a
+    // guest is never admitted without the activity row and audit event the count needs.
+    let mut c = Claims::valid();
+    c.email = format!("{}@example.com", "a".repeat(250));
+    let oversized_email = bearer(&c, PRIV1, Algorithm::ES256);
+
     for (label, token) in [
         ("wrong workspace", wrong_ws),
         ("wrong key", wrong_key),
@@ -299,6 +305,7 @@ async fn guest_jwt_refusals(db: Pool<Postgres>) -> anyhow::Result<()> {
         ("over the 24h lifetime cap", over_lifetime_cap),
         ("group-shaped email", group_shaped_email),
         ("slash in email", slash_in_email),
+        ("oversized email", oversized_email),
     ] {
         let resp = whoami(port, ws, &token).send().await?;
         assert_eq!(resp.status(), 401, "{label} must be refused");

backend/windmill-common/src/guest_jwt.rs

@@ -162,14 +162,17 @@ pub fn verify(
     }
     // The email becomes the guest's username, and `username_to_permissioned_as` reads a
     // name with no `@` (or the `group-` prefix) as a `u/<user>` or `g/<group>`
-    // principal. Require a plain email address so a guest's name can only ever be its
-    // own principal, never a user's or a group's.
+    // principal. Require a plain, bounded email: so a guest's name can only ever be its
+    // own principal, never a user's or a group's, and so it fits the
+    // `guest_activity.email` column (a longer one fails that insert while the guest is
+    // admitted uncounted).
     if !claims.email.contains('@')
         || claims.email.contains('/')
         || claims.email.chars().any(char::is_whitespace)
+        || claims.email.len() > 254
     {
         return Err(Error::NotAuthorized(
-            "guest JWT refused: email is not a plain email address".to_string(),
+            "guest JWT refused: email is not a plain, bounded email address".to_string(),
         ));
     }
     Ok(claims)
@@ -263,20 +266,58 @@ fn servable(entry: Arc<JwksEntry>) -> Result<Arc<JwksEntry>> {
     }
 }
 
-/// The workspace's JWKS, from cache when fresh. A cold or stale entry is refetched
-/// under a per-URL lock, so a burst triggers one fetch, not one per request; a failed
-/// refetch serves the last good keys, or a short-lived empty entry that reads as
-/// "issuer unreachable" rather than "kid not found", so an unreachable issuer is hit at
-/// most once per `JWKS_NEGATIVE_TTL`. Fetches follow a schedule, never a per-request,
+/// Refresh a URL's JWKS off the request path, under the single-flight lock. A held
+/// lock means a refresh is already running, so this is a no-op. A failed refresh
+/// leaves the served stale keys in place rather than dropping them.
+fn spawn_jwks_refresh(url: String) {
+    tokio::spawn(async move {
+        let lock = JWKS_FETCH_LOCKS
+            .get_or_insert_with(url.as_str(), || Ok::<_, ()>(Arc::new(tokio::sync::Mutex::new(()))))
+            .unwrap();
+        let Ok(_guard) = lock.try_lock() else { return };
+        match fetch_jwks(&url).await {
+            Ok(keys) => {
+                JWKS_CACHE.insert(
+                    url,
+                    Arc::new(JwksEntry {
+                        keys: Arc::new(keys),
+                        expires_at: Instant::now() + JWKS_TTL,
+                    }),
+                );
+            }
+            Err(e) => tracing::warn!("guest JWKS background refresh failed for {url}: {e:#}"),
+        }
+    });
+}
+
+/// The workspace's JWKS. A fresh entry is served directly; a stale-but-good one is
+/// served while a refresh runs off the request path, so a slow issuer never stalls a
+/// request. Only a cold or negative entry blocks, under a per-URL lock so a burst
+/// triggers one fetch; a failed fetch serves the last good keys, or a short-lived empty
+/// entry that reads as "issuer unreachable", so an unreachable issuer is hit at most
+/// once per `JWKS_NEGATIVE_TTL`. Fetches follow a schedule, never a per-request,
 /// attacker-chosen `kid`.
 async fn cached_jwks(url: &str) -> Result<Arc<JwksEntry>> {
     if let Some(entry) = JWKS_CACHE.get(url) {
         if entry.expires_at > Instant::now() {
             return servable(entry);
         }
+        // Stale but still holds good keys: serve them now and refresh off the request
+        // path, so a slow or hanging issuer adds no latency. Bump the entry first so the
+        // refresh window does not spawn a task per request. A negative (empty) entry
+        // falls through to the blocking refresh below.
+        if !entry.keys.is_empty() {
+            let served = Arc::new(JwksEntry {
+                keys: entry.keys.clone(),
+                expires_at: Instant::now() + JWKS_NEGATIVE_TTL,
+            });
+            JWKS_CACHE.insert(url.to_string(), served.clone());
+            spawn_jwks_refresh(url.to_string());
+            return Ok(served);
+        }
     }
-    // Single-flight: hold the per-URL lock across the fetch. `get_or_insert_with`
-    // creates the lock atomically, so two cold requests share one.
+    // Cold or negative entry, nothing good to serve: block on a single-flight refresh.
+    // `get_or_insert_with` creates the lock atomically, so two cold requests share one.
     let lock = JWKS_FETCH_LOCKS
         .get_or_insert_with(url, || Ok::<_, ()>(Arc::new(tokio::sync::Mutex::new(()))))
         .unwrap();