fix: deny job tokens a foreign-workspace admin claim and workspace ejection

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

backend/tests/wm_token_superadmin_guard.rs

@@ -759,6 +759,32 @@ async fn test_wm_token_cannot_destroy_its_on_behalf_account(
         resp.text().await?
     );
 
+    // Ejecting the identity from a workspace is the same primitive, on both routes
+    // that expose it (one keyed by username, one by email).
+    for route in [
+        "w/test-workspace/users/leave",
+        "w/test-workspace/workspaces/leave",
+    ] {
+        let resp = authed(
+            client().post(format!("http://localhost:{port}/api/{route}")),
+            &user_wm,
+        )
+        .send()
+        .await?;
+        assert_eq!(
+            resp.status(),
+            401,
+            "WM_TOKEN must not leave a workspace as {route}: {}",
+            resp.text().await?
+        );
+    }
+    let membership: i64 = sqlx::query_scalar(
+        "SELECT COUNT(*) FROM usr WHERE workspace_id = 'test-workspace' AND email = 'test2@windmill.dev'",
+    )
+    .fetch_one(&db)
+    .await?;
+    assert_eq!(membership, 1, "the workspace membership must survive");
+
     // The account and its credentials are untouched, not merely the response refused.
     let account_rows: i64 =
         sqlx::query_scalar("SELECT COUNT(*) FROM password WHERE email = 'test2@windmill.dev'")
@@ -773,3 +799,59 @@ async fn test_wm_token_cannot_destroy_its_on_behalf_account(
 
     Ok(())
 }
+
+/// `load_workspace_authed` grants an admin claim in a workspace the caller may have
+/// no relationship with, and carries `job_id` into the result — so deriving it from
+/// the on-behalf email would hand a WM_TOKEN admin over every workspace on the
+/// instance, and with it the cross-workspace diff (GHSA-hfh4-cx4h-3fcr).
+#[sqlx::test(fixtures("preserve_on_behalf_of"))]
+async fn test_wm_token_gets_no_admin_claim_in_a_foreign_workspace(
+    db: Pool<Postgres>,
+) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    set_jwt_secret().await;
+
+    // A workspace the superadmin identity is not a member of.
+    sqlx::query(
+        "INSERT INTO workspace (id, name, owner) VALUES ('other-workspace', 'Other', 'test-user')",
+    )
+    .execute(&db)
+    .await?;
+
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+    let base = format!("http://localhost:{port}/api");
+
+    let sa_wm = wm_token("test@windmill.dev", true).await;
+    let resp = authed(
+        client().get(format!(
+            "{base}/w/test-workspace/workspaces/compare/other-workspace"
+        )),
+        &sa_wm,
+    )
+    .send()
+    .await?;
+    assert_ne!(
+        resp.status(),
+        200,
+        "superadmin WM_TOKEN must not diff a workspace it does not belong to"
+    );
+
+    // No false positive: a real superadmin token still holds the claim.
+    let resp = authed(
+        client().get(format!(
+            "{base}/w/test-workspace/workspaces/compare/other-workspace"
+        )),
+        "SECRET_TOKEN",
+    )
+    .send()
+    .await?;
+    assert_eq!(
+        resp.status(),
+        200,
+        "a real superadmin token must still diff across workspaces: {}",
+        resp.text().await?
+    );
+
+    Ok(())
+}

backend/windmill-api-users/src/users.rs

@@ -3296,6 +3296,7 @@ async fn leave_workspace(
     Path(w_id): Path<String>,
     authed: ApiAuthed,
 ) -> Result<String> {
+    forbid_job_token_account_destruction(&authed)?;
     let mut tx = db.begin().await?;
     sqlx::query!(
         "DELETE FROM usr WHERE workspace_id = $1 AND username = $2",

backend/windmill-api-workspaces/src/workspaces.rs

@@ -7822,9 +7822,7 @@ async fn archive_workspace(
         .fetch_optional(&db)
         .await?
         .unwrap_or(false);
-        if !is_prod_admin
-            && !windmill_api_auth::is_super_admin_authed(&db, &authed).await?
-        {
+        if !is_prod_admin && !windmill_api_auth::is_super_admin_authed(&db, &authed).await? {
             return Err(Error::PermissionDenied(format!(
                 "Archiving dev workspace '{w_id}' requires being an admin of its parent prod workspace '{prod}' (or a superadmin)"
             )));
@@ -7897,6 +7895,7 @@ async fn leave_workspace(
     Path(w_id): Path<String>,
     authed: ApiAuthed,
 ) -> Result<String> {
+    windmill_api_auth::forbid_job_token_account_destruction(&authed)?;
     let mut tx = db.begin().await?;
     sqlx::query!(
         "DELETE FROM usr WHERE workspace_id = $1 AND email = $2",
@@ -10107,8 +10106,11 @@ async fn load_workspace_authed(
         .await
         .map_err(|e| Error::internal_err(e.to_string()))?;
 
-    let is_super_admin =
-        windmill_common::auth::is_super_admin_email(db, &base_authed.email).await?;
+    // Job-aware: this grants an admin claim in a workspace the caller may have no
+    // relationship with, and `job_id` is carried into the result — so a `WM_TOKEN`
+    // whose on-behalf identity is a superadmin would hold admin everywhere
+    // (GHSA-hfh4-cx4h-3fcr). It then falls through to its real membership below.
+    let is_super_admin = windmill_api_auth::is_super_admin_authed(db, base_authed).await?;
 
     let user_row = sqlx::query!(
         "SELECT username, is_admin, operator FROM usr

docs/followup-onbehalf-execution-privilege-hardening.md

@@ -65,6 +65,69 @@ job's *preserved on-behalf email* rather than from a request `ApiAuthed`, so the
   `is_super_admin`/`is_job_token` bool from callers. Only independently authenticated
   superadmins should get the exemption.
 
+### 3. Two credential mints still reachable by a job token
+
+Both hand back a database-backed token, which authenticates with `job_id: None` and so
+sheds the cap. Neither is an *elevation* on its own, which is why they are here rather
+than in #10124, but both let a job token trade itself for a credential it should not have.
+
+- `impersonate_service_account` (`windmill-api-users/src/users_ee.rs`) gates only on the
+  raw `require_admin(authed.is_admin, ..)` boolean, which a `WM_TOKEN` satisfies — it is
+  capped *at* workspace admin. It mints a 24h, **unscoped**, `workspace_id`-NULL token for
+  the service account and returns it in `Set-Cookie`. Unscoped means the route-scope
+  middleware does not confine it, so it holds whatever the service account's `usr` row
+  grants, including a workspace-admin claim with no job provenance.
+- `new_webhook_token` (`windmill-native-triggers/src/handler.rs`) reaches
+  `create_token_internal` from trigger create/update and from the rename path with no job
+  guard. `create_token_internal` copies `super_admin` from the caller's `password` row, and
+  `webhook_token_expiration()` is `None` for GitHub and Nextcloud, so the result is a
+  permanent superadmin-flagged token — embedded in the webhook URL handed to the external
+  service, and readable by whoever controls that repo. Its sibling `rotate_webhook_token`
+  (`windmill-native-triggers/src/lib.rs`) copies `email`/`super_admin`/`owner` verbatim from
+  the old row, so guarding only the fresh-mint path leaves the update route open.
+  The `jobs:run:<kind>:<path>` scopes do bound this one, and scopes are enforced globally at
+  auth extraction — unlike the impersonation token above.
+
+Fix both with `forbid_elevated_job_token`, as the other mints do.
+
+### 4. Destructive self-service against the borrowed identity
+
+A different threat model from the GHSA: not escalation, but a `wm_deployers` member
+damaging an arbitrary victim through a runnable that preserves their identity. #10124
+closed `leave_instance`, `tokens/delete`, `users/leave`, and `workspaces/leave`; these
+remain, ranked by damage:
+
+- `update_draft` (`windmill-api/src/drafts.rs`) deliberately **skips**
+  `require_can_write_path` when the request is a self-discard, and honours `force`, so a
+  job token can destroy the victim's unsaved script/flow/app work at any path. Draft paths
+  are enumerable first through `drafts/list` with the same token.
+- `decline_invite` (`windmill-api-users/src/users.rs`) takes `workspace_id` from the request
+  **body**, not the path, escaping the token's workspace binding: it can delete the victim's
+  pending invite to any workspace on the instance, including admin invites.
+- `accept_invite` consumes an invite (irreversible, same body-supplied workspace) and, when
+  `AUTOMATE_USERNAME_CREATION` is off, lets the caller choose the victim's username in that
+  workspace.
+- `delete_input` / `update_input` (`windmill-api-inputs/src/lib.rs`) delete or rewrite the
+  victim's saved inputs; flipping `is_public` to true leaks their saved arguments.
+- `unstar` (`windmill-api/src/favorite.rs`) — cosmetic.
+
+`forbid_job_token_account_destruction` is the right guard for the first three. Decide
+deliberately where the line sits for the tail: a job legitimately acts *as* the identity,
+so not every self-service write should be refused.
+
+Note `leave_workspace` (both routes) did a *bare* `DELETE FROM usr`, unlike
+`delete_workspace_user_internal`, which also strips `extra_perms`, folder owners, drafts,
+favorites, inputs and captures. Even now that a job token cannot reach it, a user leaving a
+workspace still leaves dangling `u/<username>` ACLs behind — worth fixing separately.
+
+### 5. Missing workspace binding on `get_github_app_token`
+
+`git_sync_ee.rs` gates on `require_admin(authed.is_admin, ..)` but never checks that
+`authed`'s workspace matches the `workspace_id` claim inside the supplied job JWT — the JWT
+alone drives the installation lookup. Exploiting it requires already holding a git-sync job
+JWT from the other workspace, so it is not an escalation path on its own, but the binding
+should be explicit.
+
 ## Guiding principle
 
 `on_behalf_of` is attacker-influenced (a `wm_deployers` member sets it). It must never