From 98e8a594f2d265a6e72af03a5dbcaa44ba5714d1 Mon Sep 17 00:00:00 2001
From: Ruben Fiszel <ruben@windmill.dev>
Date: Thu, 3 Sep 2026 09:08:25 +0000
Subject: [PATCH] fix: single-flight the JWKS refresh; design-system textarea;
 doc placement

- Refresh a workspace's JWKS under a per-URL lock, so a burst against a cold or
  stale entry triggers one fetch, not one per request (single-flight). The "at
  most once per interval" bound now holds while a fetch is in flight, not only
  after it lands.
- A cached entry with no keys is a remembered failure; serving it reported an
  unreachable issuer as an unknown `kid`. Map an empty entry to an
  issuer-unreachable error instead.
- Use the design-system TextInput (textarea variant) for the PEM key field
  rather than a raw <textarea>.
- Move `has_any_account` above `username_to_permissioned_as` so it no longer
  sits between that function's doc comment and its body.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VF3v6LA9399gNphmZaHYG3
---
 backend/windmill-common/src/guest_jwt.rs      | 44 +++++++++++++++----
 backend/windmill-common/src/users.rs          | 10 ++---
 .../(logged)/workspace_settings/+page.svelte  | 12 +++--
 3 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/backend/windmill-common/src/guest_jwt.rs b/backend/windmill-common/src/guest_jwt.rs
index de5eba567a..248c8d4686 100644
--- a/backend/windmill-common/src/guest_jwt.rs
+++ b/backend/windmill-common/src/guest_jwt.rs
@@ -174,6 +174,9 @@ struct JwksEntry {
 
 lazy_static::lazy_static! {
     static ref JWKS_CACHE: Cache<String, Arc<JwksEntry>> = Cache::new(200);
+    /// Per-URL fetch lock: only one refresh per URL is in flight at a time, so a cold
+    /// or stale entry under a burst triggers one fetch, not one per request.
+    static ref JWKS_FETCH_LOCKS: Cache<String, Arc<tokio::sync::Mutex<()>>> = Cache::new(200);
 }
 
 /// How long a good key set is served before a refresh; also the lag before a
@@ -236,15 +239,40 @@ pub async fn fetch_jwks(url: &str) -> Result<HashMap<String, Jwk>> {
     Ok(keys)
 }
 
+/// A cached entry that holds no keys is a remembered failure; serving it would report
+/// an unreachable issuer as an unknown `kid`. Map it to an issuer-unreachable error.
+fn servable(entry: Arc<JwksEntry>) -> Result<Arc<JwksEntry>> {
+    if entry.keys.is_empty() {
+        Err(Error::NotAuthorized(
+            "guest JWT refused: the JWKS issuer is unreachable".to_string(),
+        ))
+    } else {
+        Ok(entry)
+    }
+}
+
 /// The workspace's JWKS, from cache when fresh. A cold or stale entry is refetched
-/// once; a failed refetch serves the last good keys (or an empty set) for
-/// `JWKS_NEGATIVE_TTL`, so an unreachable issuer is fetched at most once per that
-/// interval whatever the guest-JWT traffic. Fetches thus follow a schedule, never a
-/// per-request, attacker-chosen `kid`.
+/// 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,
+/// 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 Ok(entry);
+            return servable(entry);
+        }
+    }
+    // Single-flight: hold the per-URL lock across the fetch. `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();
+    let _guard = lock.lock().await;
+    // Another task may have refreshed while we waited for the lock.
+    if let Some(entry) = JWKS_CACHE.get(url) {
+        if entry.expires_at > Instant::now() {
+            return servable(entry);
         }
     }
     match fetch_jwks(url).await {
@@ -256,13 +284,11 @@ async fn cached_jwks(url: &str) -> Result<Arc<JwksEntry>> {
         }
         Err(e) => {
             let keys = JWKS_CACHE.get(url).map(|stale| stale.keys.clone()).unwrap_or_default();
-            let empty = keys.is_empty();
             let entry =
                 Arc::new(JwksEntry { keys, expires_at: Instant::now() + JWKS_NEGATIVE_TTL });
             JWKS_CACHE.insert(url.to_string(), entry.clone());
-            // Nothing good was ever cached: surface the fetch error rather than an
-            // empty key set that reads as "kid not found".
-            if empty {
+            // Nothing good was ever cached: surface the fetch error itself.
+            if entry.keys.is_empty() {
                 return Err(e);
             }
             Ok(entry)
diff --git a/backend/windmill-common/src/users.rs b/backend/windmill-common/src/users.rs
index 3ea7843558..3d00c5d312 100644
--- a/backend/windmill-common/src/users.rs
+++ b/backend/windmill-common/src/users.rs
@@ -31,11 +31,6 @@ pub const USERNAME_GROUP_PREFIX: &str = "group-";
 /// columns runnables and triggers store one in.
 pub const PERMISSIONED_AS_MAX_LEN: usize = 55;
 
-/// An email-shaped username is its own principal, which is how a superadmin acting without a
-/// `usr` row is named (`usr.username` is constrained to `[\w-]+`, so a member never is). It is
-/// decided before the group convention — an address is never a group's username — and one
-/// containing `/` is prefixed, since readers split on the first `/` and would otherwise take
-/// `g/alice@example.com` for a group.
 /// Whether any account exists for `email`: a `password` row (deactivated ones
 /// included, since the sign-in path filters `disabled = false` and a re-enabled
 /// account must not read as absent) or a `usr` row in any workspace (what a service
@@ -59,6 +54,11 @@ pub async fn has_any_account<'c, E: sqlx::Executor<'c, Database = sqlx::Postgres
     .map_err(|e| crate::error::Error::internal_err(format!("checking account for {email}: {e:#}")))
 }
 
+/// An email-shaped username is its own principal, which is how a superadmin acting without a
+/// `usr` row is named (`usr.username` is constrained to `[\w-]+`, so a member never is). It is
+/// decided before the group convention — an address is never a group's username — and one
+/// containing `/` is prefixed, since readers split on the first `/` and would otherwise take
+/// `g/alice@example.com` for a group.
 pub fn username_to_permissioned_as(user: &str) -> String {
     if user.contains('@') {
         return if user.contains('/') {
diff --git a/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte b/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte
index ee248538f0..17c82afcf6 100644
--- a/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte
+++ b/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte
@@ -2258,11 +2258,15 @@ export async function main(
 											{/snippet}
 										</ToggleButtonGroup>
 										{#if guestJwtKeyType === 'pem'}
-											<textarea
-												class="w-full h-32 font-mono text-xs p-2 border rounded resize-y bg-surface text-primary"
-												placeholder={'-----BEGIN PUBLIC KEY-----\n...\n-----END PUBLIC KEY-----'}
+											<TextInput
+												underlyingInputEl="textarea"
+												class="font-mono text-xs"
+												autosizeParams={{ minHeight: 128 }}
+												inputProps={{
+													placeholder: '-----BEGIN PUBLIC KEY-----\n...\n-----END PUBLIC KEY-----'
+												}}
 												bind:value={guestJwtPublicKey}
-											></textarea>
+											/>
 										{:else}
 											<TextInput
 												inputProps={{ placeholder: 'https://issuer.example.com/.well-known/jwks.json' }}