diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index f6575c95db..1c7d363896 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -dff61d6da80d15f8327af99d322c00cc91f784ff +d30af67d38954f9012f7bad08da23e347344b4c6 diff --git a/backend/tests/fixtures/preserve_on_behalf_of.sql b/backend/tests/fixtures/preserve_on_behalf_of.sql index ff8428a1c5..7c6c4fee57 100644 --- a/backend/tests/fixtures/preserve_on_behalf_of.sql +++ b/backend/tests/fixtures/preserve_on_behalf_of.sql @@ -36,6 +36,11 @@ INSERT INTO password(email, password_hash, login_type, super_admin, verified, na VALUES ('test2@windmill.dev', 'not-a-real-hash', 'password', false, true, 'Test User 2') ON CONFLICT DO NOTHING; +-- Instance devops user (not a superadmin): the tier a token mint must not launder +INSERT INTO password(email, password_hash, login_type, super_admin, devops, verified, name) + VALUES ('devops@windmill.dev', 'not-a-real-hash', 'password', false, true, true, 'Devops User') +ON CONFLICT DO NOTHING; + -- Deployer user (non-admin but in wm_deployers group) INSERT INTO password(email, password_hash, login_type, super_admin, verified, name) VALUES ('deployer@windmill.dev', 'not-a-real-hash', 'password', false, true, 'Deployer User') diff --git a/backend/tests/postgres_trigger_scope.rs b/backend/tests/postgres_trigger_scope.rs index cf98a1e6de..e8ba36ebb9 100644 --- a/backend/tests/postgres_trigger_scope.rs +++ b/backend/tests/postgres_trigger_scope.rs @@ -24,6 +24,7 @@ fn scoped_authed(scopes: Vec<&str>) -> ApiAuthed { is_session_token: false, token_prefix: None, read_only: false, + job_id: None, } } diff --git a/backend/tests/preserve_on_behalf_of.rs b/backend/tests/preserve_on_behalf_of.rs index e696de44ea..2a1f3895ca 100644 --- a/backend/tests/preserve_on_behalf_of.rs +++ b/backend/tests/preserve_on_behalf_of.rs @@ -2810,3 +2810,168 @@ async fn test_schedule_permissions_superadmin_not_in_workspace( Ok(()) } + +// ============================================================================ +// Forged-superadmin on_behalf_of guard (GHSA-hfh4-cx4h-3fcr) +// ============================================================================ + +/// Reserved internal sentinel identities are rejected by name at deploy time on +/// every entity that stores a preserved on_behalf_of. Real identities stay +/// deployable by a `wm_deployers` member — including a real superadmin, and even +/// their email pinned onto an unrelated principal — because that escalation is +/// closed at execution by the job-token cap, not by restricting what is stored. +#[sqlx::test(fixtures("preserve_on_behalf_of"))] +async fn test_reject_reserved_sentinel_on_behalf_of(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api/w/test-workspace"); + + // Reserved internal sentinel (grants is_super_admin at execution by email). + const SENTINEL: &str = "superadmin_secret@windmill.dev"; + // Reserved sentinel matched on permissioned_as. + const SYNC_SENTINEL: &str = "superadmin_sync@windmill.dev"; + // Real instance superadmin, present only in `password` (not a workspace member). + const REAL_SA: &str = "superadmin-external@windmill.dev"; + + // App: deployer cannot pin the sentinel email. + let resp = authed( + client().post(format!("{base}/apps/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_app_with_on_behalf_of( + "u/deployer-user/app_sentinel", + Some("u/original-user"), + Some(SENTINEL), + true, + )) + .send() + .await?; + assert_eq!( + resp.status(), + 400, + "deployer must not pin the sentinel as an app on_behalf_of_email: {}", + resp.text().await? + ); + + // App: a real superadmin on_behalf_of is *allowed* at deploy (deployers may + // deploy on behalf of any real user). The escalation is closed at execution + // by the job-token cap, not by restricting what can be stored, so even a + // superadmin email pinned onto an unrelated principal deploys fine here. + let resp = authed( + client().post(format!("{base}/apps/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_app_with_on_behalf_of( + "u/deployer-user/app_real_sa", + Some("u/original-user"), + Some(REAL_SA), + true, + )) + .send() + .await?; + assert_eq!( + resp.status(), + 201, + "a real superadmin on_behalf_of is allowed at deploy (capped at execution): {}", + resp.text().await? + ); + + // App: a consistently named real superadmin identity is likewise allowed. + let resp = authed( + client().post(format!("{base}/apps/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_app_with_on_behalf_of( + "u/deployer-user/app_consistent_sa", + Some("u/superadmin-external"), + Some(REAL_SA), + true, + )) + .send() + .await?; + assert_eq!( + resp.status(), + 201, + "deployer may preserve a consistently named superadmin identity: {}", + resp.text().await? + ); + + // Flow: deployer cannot pin the sentinel email. + let resp = authed( + client().post(format!("{base}/flows/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_flow_with_on_behalf_of( + "u/deployer-user/flow_sentinel", + Some(SENTINEL), + true, + )) + .send() + .await?; + assert_eq!( + resp.status(), + 400, + "deployer must not pin the sentinel as a flow on_behalf_of_email: {}", + resp.text().await? + ); + + // Script: deployer cannot pin the sentinel email. + let resp = authed( + client().post(format!("{base}/scripts/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_script_with_on_behalf_of( + "u/deployer-user/script_sentinel", + Some(SENTINEL), + true, + )) + .send() + .await?; + assert_eq!( + resp.status(), + 400, + "deployer must not pin the sentinel as a script on_behalf_of_email: {}", + resp.text().await? + ); + + // Schedule: deployer cannot preserve the sync sentinel as permissioned_as. + let resp = authed( + client().post(format!("{base}/scripts/create")), + "DEPLOYER_TOKEN", + ) + .json(&new_script_with_on_behalf_of( + "u/deployer-user/sched_guard_script", + None, + false, + )) + .send() + .await?; + assert_eq!(resp.status(), 201, "{}", resp.text().await?); + + let resp = authed( + client().post(format!("{base}/schedules/create")), + "DEPLOYER_TOKEN", + ) + .json(&json!({ + "path": "u/deployer-user/schedule_sync_sentinel", + "schedule": "0 0 */6 * * *", + "timezone": "UTC", + "script_path": "u/deployer-user/sched_guard_script", + "is_flow": false, + "enabled": false, + "permissioned_as": SYNC_SENTINEL, + "preserve_permissioned_as": true + })) + .send() + .await?; + assert_eq!( + resp.status(), + 400, + "deployer must not preserve the sync sentinel as a schedule permissioned_as: {}", + resp.text().await? + ); + + Ok(()) +} diff --git a/backend/tests/trigger_listener_queries.rs b/backend/tests/trigger_listener_queries.rs index 52088557ac..1463138167 100644 --- a/backend/tests/trigger_listener_queries.rs +++ b/backend/tests/trigger_listener_queries.rs @@ -177,6 +177,7 @@ fn make_authed() -> windmill_api_auth::ApiAuthed { is_session_token: false, token_prefix: None, read_only: false, + job_id: None, } } diff --git a/backend/tests/wm_token_superadmin_guard.rs b/backend/tests/wm_token_superadmin_guard.rs index 650cf384d4..8823cb8707 100644 --- a/backend/tests/wm_token_superadmin_guard.rs +++ b/backend/tests/wm_token_superadmin_guard.rs @@ -204,3 +204,654 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool) -> any Ok(()) } + +/// A WM_TOKEN running as a superadmin must be rejected by *any* `require_super_admin` +/// route, not just the handful that call `forbid_superadmin_job_token`. `GET +/// /api/settings/list_global` is gated solely by `require_super_admin`, so it +/// exercises the token-layer guard (GHSA-hfh4-cx4h-3fcr). +#[sqlx::test(fixtures("preserve_on_behalf_of"))] +async fn test_wm_token_rejected_by_require_super_admin(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api"); + + // The exact token a deployer obtains via an app on_behalf_of pointed at a superadmin. + let sa_wm = wm_token("test@windmill.dev", true).await; + let resp = authed(client().get(format!("{base}/settings/list_global")), &sa_wm) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not reach a require_super_admin route: {}", + resp.text().await? + ); + + // No false positive: a real superadmin API token (no job_id) still reaches it. + let resp = authed( + client().get(format!("{base}/settings/list_global")), + "SECRET_TOKEN", + ) + .send() + .await?; + assert_eq!( + resp.status(), + 200, + "a real superadmin token must still reach the route: {}", + resp.text().await? + ); + + Ok(()) +} + +/// Direct `is_super_admin_email` authorization gates (not routed through +/// `require_super_admin`) must also reject a superadmin `WM_TOKEN`. Covers the two +/// bypass classes the CI review flagged: destructive `delete_workspace`, and the +/// `CUSTOM_INSTANCE_DB` credential lookup whose guard must read the *authenticated* +/// `job_id`, not the caller-supplied `?job_id` query param (GHSA-hfh4-cx4h-3fcr). +#[sqlx::test(fixtures("preserve_on_behalf_of"))] +async fn test_wm_token_rejected_by_direct_super_admin_gates( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().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; + + // 1. Global workspace deletion (destructive) — must be forbidden. + let resp = authed( + client().delete(format!("{base}/workspaces/delete/test-workspace")), + &sa_wm, + ) + .send() + .await?; + assert_eq!( + resp.status(), + 403, + "superadmin WM_TOKEN must not delete a workspace: {}", + resp.text().await? + ); + // The workspace must still exist. + let exists: bool = + sqlx::query_scalar("SELECT EXISTS(SELECT 1 FROM workspace WHERE id = 'test-workspace')") + .fetch_one(&db) + .await?; + assert!( + exists, + "rejected delete must not have removed the workspace" + ); + + // 2. CUSTOM_INSTANCE_DB credential lookup, WITHOUT the ?job_id query param — + // the guard must reject based on the authenticated token's job_id. + let resp = authed( + client().get(format!( + "{base}/w/test-workspace/resources/get_value_interpolated/CUSTOM_INSTANCE_DB/anydb" + )), + &sa_wm, + ) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not resolve CUSTOM_INSTANCE_DB (no creds leak): {}", + resp.text().await? + ); + + Ok(()) +} + +/// The instance-level `devops` role must be capped like superadmin. +/// `is_devops_email` returns true for superadmin emails, so every +/// `require_devops_role` route (worker management, instance config, service logs) +/// is reachable by exactly the same superadmin `WM_TOKEN` unless it is capped too +/// (GHSA-hfh4-cx4h-3fcr). +#[sqlx::test(fixtures("preserve_on_behalf_of"))] +async fn test_wm_token_rejected_by_require_devops_role(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().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}/service_logs/list_files")), + &sa_wm, + ) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not reach a require_devops_role route: {}", + resp.text().await? + ); + + // No false positive: a real superadmin API token (no job_id) still reaches it. + let resp = authed( + client().get(format!("{base}/service_logs/list_files")), + "SECRET_TOKEN", + ) + .send() + .await?; + assert_eq!( + resp.status(), + 200, + "a real superadmin token must still reach the devops route: {}", + resp.text().await? + ); + + // The advisory's own PoC route: the full user directory, gated solely by + // `require_super_admin` with no per-route job-token denylist. + let resp = authed( + client().get(format!("{base}/users/list_as_super_admin")), + &sa_wm, + ) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not list all users: {}", + resp.text().await? + ); + + Ok(()) +} + +/// A job token must not clear an *admin-or-devops* gate via the devops branch. +/// `require_admin_or_devops` (the EE critical-alerts endpoints) grants when the +/// caller is a workspace admin OR an instance `devops`; since `is_devops_email` +/// is true for superadmins, a WM_TOKEN running on-behalf of a superadmin who is +/// NOT a member of the target workspace would otherwise gain workspace-scoped +/// devops access to a workspace it has no admin rights in (GHSA-hfh4-cx4h-3fcr). +/// The workspace-admin branch stays allowed — that is the cap ceiling. +#[cfg(feature = "enterprise")] +#[sqlx::test(fixtures("preserve_on_behalf_of"))] +async fn test_wm_token_rejected_by_admin_or_devops_gate(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api/w/test-workspace/workspaces"); + + // superadmin-external is a superadmin but not a member of test-workspace, so + // its workspace-level is_admin is false — the exact exploit precondition. + let sa_wm = wm_token("superadmin-external@windmill.dev", false).await; + let resp = authed(client().get(format!("{base}/critical_alerts")), &sa_wm) + .send() + .await?; + assert_eq!( + resp.status(), + 403, + "superadmin WM_TOKEN must not clear the admin-or-devops gate on a workspace it isn't admin of: {}", + resp.text().await? + ); + + // No false positive: the same superadmin's real API token (not a job token) + // still clears the gate via the devops branch. + let resp = authed( + client().get(format!("{base}/critical_alerts")), + "EXTERNAL_SUPERADMIN_TOKEN", + ) + .send() + .await?; + assert_ne!( + resp.status(), + 403, + "a real superadmin token must still clear the admin-or-devops gate: {}", + resp.text().await? + ); + + Ok(()) +} + +/// Instance-global routes with no workspace binding that gate on the caller's own +/// `is_admin` claim must reject a WM_TOKEN — `is_admin` is a workspace-admin claim +/// (also true for superadmins), and a job token is capped at workspace admin, so it +/// must not wield that claim as instance authorization (GHSA-hfh4-cx4h-3fcr). +#[sqlx::test(fixtures("preserve_on_behalf_of"))] +async fn test_wm_token_rejected_by_instance_admin_gates(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + + // A worker-group config carrying a static env value that must stay masked. + sqlx::query("INSERT INTO config (name, config) VALUES ('worker__wm2082grp', $1)") + .bind(json!({ "env_vars_static": { "LEAKY": "supersecretvalue" } })) + .execute(&db) + .await?; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api"); + + // The exact token a deployer obtains via an app on_behalf_of pointed at a + // superadmin: is_admin=true, but carrying a job_id. + let sa_wm = wm_token("test@windmill.dev", true).await; + + // 1. Arbitrary workspace unarchive (mutation on any workspace by id). + let resp = authed( + client().post(format!("{base}/workspaces/unarchive/test-workspace")), + &sa_wm, + ) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not unarchive an arbitrary workspace: {}", + resp.text().await? + ); + + // 2. Global concurrency-group pruning. + let resp = authed( + client().delete(format!("{base}/concurrency_groups/prune/anykey")), + &sa_wm, + ) + .send() + .await?; + assert_eq!( + resp.status(), + 403, + "superadmin WM_TOKEN must not prune a global concurrency group: {}", + resp.text().await? + ); + + // 3. The sibling listing spans every workspace's concurrency keys, so it is + // gated the same way as the prune above. + let resp = authed( + client().get(format!("{base}/concurrency_groups/list")), + &sa_wm, + ) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not list global concurrency groups: {}", + resp.text().await? + ); + + // 4. Worker-group config: the static env value must be masked for a job token. + let body = authed( + client().get(format!("{base}/configs/list_worker_groups")), + &sa_wm, + ) + .send() + .await? + .text() + .await?; + assert!( + !body.contains("supersecretvalue"), + "superadmin WM_TOKEN must get the obfuscated worker-group view: {body}" + ); + + // No false positive: a real superadmin API token (no job_id) still sees the + // unobfuscated value — the cap keys off the job token, not the identity. + let body = authed( + client().get(format!("{base}/configs/list_worker_groups")), + "SECRET_TOKEN", + ) + .send() + .await? + .text() + .await?; + assert!( + body.contains("supersecretvalue"), + "a real superadmin token must still see the unobfuscated worker-group config: {body}" + ); + + Ok(()) +} + +/// Capping a `WM_TOKEN` at the gates is only durable if the token cannot trade +/// itself for one without the `job_id` those gates key off. Both credential-minting +/// routes must therefore refuse an elevated job token: `refresh_token` (which mints +/// a database-backed session token and returns it in `Set-Cookie`) and +/// `tokens/create` for the `devops` tier, whose routes are capped just like +/// superadmin's (GHSA-hfh4-cx4h-3fcr). +#[sqlx::test(fixtures("preserve_on_behalf_of"))] +async fn test_wm_token_cannot_mint_a_provenance_free_credential( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api/users"); + + // 1. Session refresh: a superadmin-identity job token must not obtain a session + // token, which would authenticate with no job provenance at all. + let sa_wm = wm_token("test@windmill.dev", true).await; + let resp = authed(client().get(format!("{base}/refresh_token")), &sa_wm) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not refresh into a session token: {}", + resp.text().await? + ); + + // No false positive: a real superadmin API token still refreshes. + let resp = authed( + client().get(format!("{base}/refresh_token")), + "SECRET_TOKEN", + ) + .send() + .await?; + assert_eq!( + resp.status(), + 200, + "a real superadmin token must still refresh: {}", + resp.text().await? + ); + + // 2. Token mint, devops tier: `require_devops_role` rejects this job token, so + // minting one that would pass it by email must be refused too. + let devops_wm = wm_token("devops@windmill.dev", false).await; + let resp = authed(client().post(format!("{base}/tokens/create")), &devops_wm) + .json(&json!({ "label": "from-script" })) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "devops WM_TOKEN must not mint a token: {}", + resp.text().await? + ); + + // 3. Choosing the password of the elevated account it runs as would let the + // holder log in for a session that carries no job provenance at all. + let resp = authed(client().post(format!("{base}/setpassword")), &devops_wm) + .json(&json!({ "password": "hunter2" })) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "devops WM_TOKEN must not set its account password: {}", + resp.text().await? + ); + + Ok(()) +} + +/// The MCP OAuth approval is a third credential mint: the code it stores is +/// exchanged for a database token holding only an email, so an elevated job token +/// approving a client would obtain a credential with no `job_id` and re-enter the +/// API through the gateway uncapped (GHSA-hfh4-cx4h-3fcr). The guard sits in the +/// shared inner fn, ahead of client validation, so it fires without a registered +/// client. +#[cfg(feature = "mcp")] +#[sqlx::test(fixtures("preserve_on_behalf_of"))] +async fn test_wm_token_cannot_mint_via_mcp_oauth_approval( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api"); + + let approval = json!({ + "client_id": "wm2082-client", + "redirect_uri": "http://localhost/callback", + "scope": "mcp:all", + "state": "s", + "code_challenge": "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + "code_challenge_method": "S256", + }); + + let sa_wm = wm_token("test@windmill.dev", true).await; + let resp = authed( + client().post(format!("{base}/w/test-workspace/mcp/oauth/server/approve")), + &sa_wm, + ) + .json(&approval) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not approve an MCP OAuth client: {}", + resp.text().await? + ); + + // The gateway route reaches the same mint and must be capped identically. + let mut gateway_approval = approval.clone(); + gateway_approval["workspace_id"] = json!("test-workspace"); + let resp = authed( + client().post(format!("{base}/mcp/gateway/oauth/server/approve")), + &sa_wm, + ) + .json(&gateway_approval) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not approve through the MCP gateway: {}", + resp.text().await? + ); + + Ok(()) +} + +/// The two links that let a narrowly-scoped mint become a general credential: the +/// sandboxed app embed mint (a 12h database token with no job provenance) and +/// `tokens/update_scopes`, which an unscoped job token could use to clear the +/// scopes of any token sharing its email (GHSA-hfh4-cx4h-3fcr). +#[sqlx::test(fixtures("preserve_on_behalf_of"))] +async fn test_wm_token_cannot_mint_or_widen_an_app_embed_token( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + + // A sandboxed app the superadmin identity can read — the mint's precondition. + sqlx::query( + "INSERT INTO app (id, workspace_id, path, summary, versions, policy, extra_perms) + VALUES (9001, 'test-workspace', 'u/test-user/embedded', 'Embedded', '{}', + '{\"execution_mode\": \"viewer\", \"sandbox\": true}', '{}')", + ) + .execute(&db) + .await?; + sqlx::query( + "INSERT INTO app_version (id, app_id, value, created_by, created_at) + VALUES (9001, 9001, '{\"grid\": []}', 'test-user', NOW())", + ) + .execute(&db) + .await?; + sqlx::query("UPDATE app SET versions = ARRAY[9001::bigint] WHERE id = 9001") + .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/apps/embed_token/p/u/test-user/embedded" + )), + &sa_wm, + ) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not mint an app embed token: {}", + resp.text().await? + ); + + // Even a token minted some other way must stay narrow: widening is refused. + let resp = authed( + client().post(format!("{base}/users/tokens/update_scopes/SECRET_T")), + &sa_wm, + ) + .json(&json!({ "scopes": serde_json::Value::Null })) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "superadmin WM_TOKEN must not widen a token's scopes: {}", + resp.text().await? + ); + + Ok(()) +} + +/// Destroying the account or credentials of the identity a job runs as is never the +/// runnable's work, and a `wm_deployers` member may point `on_behalf_of` at any real +/// user — so these reject every job token, elevated or not (GHSA-hfh4-cx4h-3fcr). +#[sqlx::test(fixtures("preserve_on_behalf_of"))] +async fn test_wm_token_cannot_destroy_its_on_behalf_account( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api/users"); + + // An ordinary member's identity: the cap here does not depend on elevation. + let user_wm = wm_token("test2@windmill.dev", false).await; + + let resp = authed(client().post(format!("{base}/leave_instance")), &user_wm) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "WM_TOKEN must not delete the account it runs as: {}", + resp.text().await? + ); + + // The prefix of that identity's real fixture token, so without the guard the + // delete would land rather than silently match nothing. + let resp = authed( + client().delete(format!("{base}/tokens/delete/SECRET_TOK")), + &user_wm, + ) + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "WM_TOKEN must not revoke that identity's tokens: {}", + 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'") + .fetch_one(&db) + .await?; + assert_eq!(account_rows, 1, "the password row must survive"); + let token_rows: i64 = + sqlx::query_scalar("SELECT COUNT(*) FROM token WHERE email = 'test2@windmill.dev'") + .fetch_one(&db) + .await?; + assert!(token_rows > 0, "the identity's tokens must survive"); + + 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, +) -> 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(()) +} diff --git a/backend/tests/worker.rs b/backend/tests/worker.rs index 2ecb266787..419be6fdf3 100644 --- a/backend/tests/worker.rs +++ b/backend/tests/worker.rs @@ -5524,25 +5524,25 @@ async fn test_fork_marker_tag_admission_through_lineage(db: Pool) -> a "bare(test-workspace)".to_string(), ]))); - // test2 is not a superadmin, who would bypass the scope check entirely. - let email = "test2@windmill.dev"; + // A non-superadmin caller (a superadmin would bypass the scope check entirely). + let is_super_admin = false; for (w_id, tag) in [("test-workspace", "bare"), ("test-workspace", "forky")] { assert!( - check_tag_available_for_workspace_internal(&db, w_id, tag, email, None) + check_tag_available_for_workspace_internal(&db, w_id, tag, is_super_admin, None) .await .is_ok(), "{tag} should be available in the workspace it names" ); } assert!( - check_tag_available_for_workspace_internal(&db, fork, "forky", email, None) + check_tag_available_for_workspace_internal(&db, fork, "forky", is_super_admin, None) .await .is_ok(), "a `*` tag must be granted to a fork through its parent lineage" ); assert!( - check_tag_available_for_workspace_internal(&db, fork, "bare", email, None) + check_tag_available_for_workspace_internal(&db, fork, "bare", is_super_admin, None) .await .is_err(), "an unmarked tag must not reach a fork of the workspace it names" diff --git a/backend/windmill-api-assets/tests/dbt_pinned_graph.rs b/backend/windmill-api-assets/tests/dbt_pinned_graph.rs index 82e32797d0..7f4979a96f 100644 --- a/backend/windmill-api-assets/tests/dbt_pinned_graph.rs +++ b/backend/windmill-api-assets/tests/dbt_pinned_graph.rs @@ -32,6 +32,7 @@ fn outsider() -> ApiAuthed { is_session_token: false, token_prefix: None, read_only: false, + job_id: None, } } @@ -417,9 +418,16 @@ async fn an_editor_graph_renders_only_through_its_own_job(db: Pool) { // Through the PATH — which is what the workspace graph and every run of the // deployed version ask for — a buffer parse must not appear at all. It // describes an editor's unsaved state, not what the script owns. - let workspace = asset_graph_for(&admin, WS, UserDB::new(db.clone()), db.clone(), query(), None) - .await - .unwrap(); + let workspace = asset_graph_for( + &admin, + WS, + UserDB::new(db.clone()), + db.clone(), + query(), + None, + ) + .await + .unwrap(); let workspace = serde_json::to_value(&workspace.0).unwrap().to_string(); assert!( !workspace.contains("u/a/wh/analytics/draft"), diff --git a/backend/windmill-api-auth/src/auth.rs b/backend/windmill-api-auth/src/auth.rs index b3ad811b7f..dead5fecca 100644 --- a/backend/windmill-api-auth/src/auth.rs +++ b/backend/windmill-api-auth/src/auth.rs @@ -137,6 +137,20 @@ impl AuthCache { &self, w_id: Option, token: &str, + ) -> Option { + let mut opt_job_authed = self.get_opt_job_authed_inner(w_id, token).await?; + // Single source of truth: mirror the resolved job_id onto the authed so + // every consumer (require_super_admin, ...) sees that this identity came + // from a job's WM_TOKEN, even on an AUTH_CACHE hit whose cached authed + // predates this field. + opt_job_authed.authed.job_id = opt_job_authed.job_id; + Some(opt_job_authed) + } + + async fn get_opt_job_authed_inner( + &self, + w_id: Option, + token: &str, ) -> Option { // In no-auth mode there are no real tokens: resolve directly as the // admin superadmin so direct cache callers (e.g. get_all_runnables, @@ -218,8 +232,21 @@ impl AuthCache { is_session_token, token_prefix: claims.audit_span, read_only: false, + job_id: None, + }; + // Fail closed: a `job_id` claim that does not parse must reject + // the token rather than resolve to `None`, which would clear the + // job provenance and uncap the token (GHSA-hfh4-cx4h-3fcr). + let job_id = match claims.job_id { + Some(j) => match uuid::Uuid::from_str(&j) { + Ok(job_id) => Some(job_id), + Err(_) => { + tracing::error!("JWT auth error: job_id claim is not a uuid"); + return None; + } + }, + None => None, }; - let job_id = claims.job_id.and_then(|j| uuid::Uuid::from_str(&j).ok()); AUTH_CACHE.insert( key, ExpiringAuthCache { @@ -319,6 +346,7 @@ impl AuthCache { is_session_token, token_prefix: Some(safe_token_prefix(token)), read_only, + job_id: None, }) } else { tracing::warn!( @@ -371,6 +399,7 @@ impl AuthCache { is_session_token, token_prefix: Some(safe_token_prefix(token)), read_only, + job_id: None, }) } else { tracing::warn!( @@ -446,6 +475,7 @@ impl AuthCache { is_session_token, token_prefix: Some(safe_token_prefix(token)), read_only, + job_id: None, }) } None if super_admin => { @@ -469,6 +499,7 @@ impl AuthCache { is_session_token, token_prefix: Some(safe_token_prefix(token)), read_only, + job_id: None, }), Err(e) => { tracing::error!( @@ -494,6 +525,7 @@ impl AuthCache { is_session_token, token_prefix: Some(safe_token_prefix(token)), read_only, + job_id: None, }) } } @@ -531,6 +563,7 @@ impl AuthCache { is_session_token: false, token_prefix: Some(safe_token_prefix(token)), read_only: false, + job_id: None, }; Some(OptJobAuthed { authed, job_id: None }) } else { @@ -740,6 +773,7 @@ fn no_auth_admin_authed() -> ApiAuthed { is_session_token: false, token_prefix: None, read_only: false, + job_id: None, } } diff --git a/backend/windmill-api-auth/src/lib.rs b/backend/windmill-api-auth/src/lib.rs index 88eb11093a..ecd193165e 100644 --- a/backend/windmill-api-auth/src/lib.rs +++ b/backend/windmill-api-auth/src/lib.rs @@ -73,6 +73,11 @@ pub struct ApiAuthed { pub is_session_token: bool, pub token_prefix: Option, pub read_only: bool, + /// Set when this authed was resolved from a job's `WM_TOKEN`. Such a token's + /// identity is derived from an app/flow `on_behalf_of` that a `wm_deployers` + /// member can point at a superadmin, so it must never be trusted as a global + /// superadmin (`require_super_admin`), GHSA-hfh4-cx4h-3fcr. + pub job_id: Option, } impl ApiAuthed { @@ -159,6 +164,7 @@ impl From for ApiAuthed { is_session_token: false, token_prefix: value.token_prefix, read_only: false, + job_id: None, } } } @@ -247,7 +253,10 @@ impl windmill_mcp::server::McpAuth for ApiAuthed { // ------------ Utility functions ------------ -pub async fn require_super_admin(db: &DB, email: &str) -> error::Result<()> { +/// Assert the *email* belongs to a superadmin. Prefer [`require_super_admin`], +/// which also rejects job tokens (`WM_TOKEN`); use this only where no `ApiAuthed` +/// is available and the caller has separately guaranteed it is not a job token. +pub async fn require_super_admin_email(db: &DB, email: &str) -> error::Result<()> { let is_admin = is_super_admin_email(db, email).await?; if !is_admin { @@ -259,6 +268,66 @@ pub async fn require_super_admin(db: &DB, email: &str) -> error::Result<()> { } } +/// Assert the caller is a superadmin acting under their own credentials. +/// +/// A job's `WM_TOKEN` runs as the runnable's `on_behalf_of` identity, which a +/// non-superadmin `wm_deployers` member can point at a superadmin — so a job +/// token must never satisfy a global superadmin gate regardless of whose email +/// it carries (GHSA-hfh4-cx4h-3fcr). A real superadmin needing this from a script +/// uses a dedicated superadmin token instead of `$WM_TOKEN`. +pub async fn require_super_admin(db: &DB, authed: &ApiAuthed) -> error::Result<()> { + if authed.job_id.is_some() { + return Err(Error::NotAuthorized( + "This endpoint cannot be called with a job token ($WM_TOKEN). If a script \ + genuinely needs to do this, create a dedicated superadmin token from the User \ + settings drawer (the 'Tokens' section), store it as a secret, and use that token \ + explicitly instead of $WM_TOKEN." + .to_owned(), + )); + } + require_super_admin_email(db, &authed.email).await +} + +/// Job-token-aware superadmin predicate for the many boolean `is_super_admin_email` +/// authorization branches (workspace deletion, fork drops, SSRF exemptions, ...). +/// A job's `WM_TOKEN` is never a superadmin regardless of whose email it carries +/// (GHSA-hfh4-cx4h-3fcr), so callers naturally fall through to the restricted path. +pub async fn is_super_admin_authed(db: &DB, authed: &ApiAuthed) -> error::Result { + if authed.job_id.is_some() { + return Ok(false); + } + is_super_admin_email(db, &authed.email).await +} + +/// Instance-global admin predicate, job-token-aware. `ApiAuthed::is_admin` is a +/// *workspace*-admin claim (also true for superadmins), and a `WM_TOKEN` is capped +/// at workspace admin (GHSA-hfh4-cx4h-3fcr). Routes with no workspace binding that +/// treat `is_admin` as instance authorization (worker-group config, arbitrary +/// workspace unarchive, global concurrency pruning) must use this instead of the +/// raw `authed.is_admin`, so a job token can't wield a workspace-admin claim as an +/// instance action. Interactive admins are unaffected. +pub fn is_instance_admin(authed: &ApiAuthed) -> bool { + authed.is_admin && authed.job_id.is_none() +} + +/// Hard-gate variant of [`is_instance_admin`] for instance-global routes: rejects +/// a job token (`WM_TOKEN`) explicitly, then requires admin. +pub fn require_instance_admin(authed: &ApiAuthed) -> error::Result<()> { + if authed.job_id.is_some() { + return Err(Error::NotAuthorized( + "This endpoint cannot be called with a job token ($WM_TOKEN): it is an \ + instance-global admin action and a job token is capped at workspace admin. \ + If a script genuinely needs this, create a dedicated token from the User \ + settings drawer and use it explicitly instead of $WM_TOKEN." + .to_owned(), + )); + } + if !authed.is_admin { + return Err(Error::RequireAdmin(authed.username.clone())); + } + Ok(()) +} + /// Forbid sensitive global user/token management when authenticated as a /// superadmin *via a job token* (`WM_TOKEN`). /// @@ -286,6 +355,53 @@ pub async fn forbid_superadmin_job_token( Ok(()) } +/// Forbid *minting a durable credential* from a job token that carries an elevated +/// instance identity (superadmin or `devops`; [`is_devops_email`] covers both). +/// +/// The gates that cap `$WM_TOKEN` key off `ApiAuthed::job_id`, which only a job +/// token carries. A token minted from one is an ordinary database-backed token with +/// no such provenance, so it passes every one of those gates by email alone — the +/// cap would last only until the script exchanged its token for a fresh one +/// (GHSA-hfh4-cx4h-3fcr). Narrower than rejecting all job tokens: a script running +/// as an unprivileged identity has nothing to launder and still mints freely. +pub async fn forbid_elevated_job_token( + db: &DB, + email: &str, + job_id: Option, +) -> error::Result<()> { + if job_id.is_some() && is_devops_email(db, email).await? { + return Err(Error::NotAuthorized( + "A job token ($WM_TOKEN) running as a superadmin or devops user cannot mint a new \ + token, which would carry that identity without the job provenance that caps it. \ + If a script genuinely needs this, create a dedicated token from the User settings \ + drawer (the 'Tokens' section), store it as a secret, and use that token explicitly \ + instead of $WM_TOKEN." + .to_owned(), + )); + } + Ok(()) +} + +/// Forbid an irreversible action against the *account* a job token runs as. +/// +/// A job token borrows an `on_behalf_of` identity to do the runnable's work, and a +/// `wm_deployers` member may point that at any real user. Destroying the account or +/// its credentials is never that work, and unlike the privilege gates the damage +/// does not depend on the identity being elevated — so this rejects every job +/// token, not just superadmin/devops ones (GHSA-hfh4-cx4h-3fcr). +pub fn forbid_job_token_account_destruction(authed: &ApiAuthed) -> error::Result<()> { + if authed.job_id.is_some() { + return Err(Error::NotAuthorized( + "This endpoint cannot be called with a job token ($WM_TOKEN): it would destroy the \ + account or credentials of the identity the job runs as. If this is genuinely \ + intended, do it from the User settings drawer, or with a dedicated token created \ + there and used explicitly instead of $WM_TOKEN." + .to_owned(), + )); + } + Ok(()) +} + pub fn check_scopes(authed: &ApiAuthed, required: F) -> error::Result<()> where F: FnOnce() -> String, @@ -651,10 +767,24 @@ pub fn build_scope_path_filter(authed: &ApiAuthed, domain: &str, action: &str) - ScopePathFilter::Restricted { exact, prefix } } -pub async fn require_devops_role(db: &DB, email: &str) -> error::Result<()> { - let is_devops = is_devops_email(db, email).await?; - - if is_devops { +/// Assert the caller holds the instance-level `devops` role under their own +/// credentials. +/// +/// `devops` is instance-level and [`is_devops_email`] is also true for +/// superadmins, so this gate is reachable by the same job token that +/// [`require_super_admin`] rejects, and is capped the same way +/// (GHSA-hfh4-cx4h-3fcr). +pub async fn require_devops_role(db: &DB, authed: &ApiAuthed) -> error::Result<()> { + if authed.job_id.is_some() { + return Err(Error::NotAuthorized( + "This endpoint cannot be called with a job token ($WM_TOKEN). If a script \ + genuinely needs this, create a dedicated token from the User settings drawer \ + (the 'Tokens' section), store it as a secret, and use that token explicitly \ + instead of $WM_TOKEN." + .to_owned(), + )); + } + if is_devops_email(db, &authed.email).await? { Ok(()) } else { Err(Error::NotAuthorized( @@ -913,6 +1043,7 @@ pub async fn fetch_api_authed_from_permissioned_as( is_session_token: false, token_prefix: authed.token_prefix, read_only: false, + job_id: None, }; API_AUTHED_CACHE.insert( diff --git a/backend/windmill-api-configs/src/lib.rs b/backend/windmill-api-configs/src/lib.rs index f776712063..7ad99996d5 100644 --- a/backend/windmill-api-configs/src/lib.rs +++ b/backend/windmill-api-configs/src/lib.rs @@ -23,7 +23,7 @@ use windmill_common::{ DB, }; -use windmill_api_auth::{require_devops_role, ApiAuthed}; +use windmill_api_auth::{is_instance_admin, require_devops_role, ApiAuthed}; pub fn global_service() -> Router { Router::new() @@ -75,7 +75,10 @@ async fn list_worker_groups( } } } - let configs = if !authed.is_admin { + // Worker-group configs are instance-global and expose env_vars_static (may hold + // secrets); a job token (capped at workspace admin) gets the obfuscated view even + // when its identity is a superadmin. See is_instance_admin (GHSA-hfh4-cx4h-3fcr). + let configs = if !is_instance_admin(&authed) { let mut obfuscated_configs: Vec = vec![]; for config in configs_raw { let config_value_opt = config.config.as_object().map(|obj| obj.to_owned()); @@ -117,7 +120,7 @@ async fn get_config( Path(name): Path, Extension(db): Extension, ) -> error::JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let config = sqlx::query_as!(Config, "SELECT name, config FROM config WHERE name = $1", name) .fetch_optional(&db) @@ -133,7 +136,7 @@ async fn update_config( authed: ApiAuthed, Json(config): Json, ) -> error::Result { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; #[cfg(not(feature = "enterprise"))] let config = if name.starts_with("worker__") { @@ -212,7 +215,7 @@ async fn delete_config( Extension(db): Extension, authed: ApiAuthed, ) -> error::Result { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let mut tx = db.begin().await?; @@ -280,7 +283,7 @@ async fn native_kubernetes_autoscaling_healthcheck( authed: ApiAuthed, Extension(db): Extension, ) -> Result<(), windmill_autoscaling::kubernetes_integration_ee::KubeError> { - require_devops_role(&db, &authed.email).await.map_err(|e| { + require_devops_role(&db, &authed).await.map_err(|e| { windmill_autoscaling::kubernetes_integration_ee::KubeError::Other(e.to_string()) })?; @@ -317,7 +320,7 @@ async fn list_configs( authed: ApiAuthed, Extension(db): Extension, ) -> error::JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let configs = sqlx::query_as!(Config, "SELECT name, config FROM config") .fetch_all(&db) .await?; @@ -342,7 +345,7 @@ async fn list_all_workspace_dependencies( authed: ApiAuthed, Extension(db): Extension, ) -> error::JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let deps = sqlx::query!( r#"SELECT workspace_id, name, language AS "language: windmill_common::scripts::ScriptLang" FROM workspace_dependencies @@ -374,7 +377,7 @@ async fn list_all_dedicated_with_deps( authed: ApiAuthed, Extension(db): Extension, ) -> error::JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let rows = sqlx::query!( r#"SELECT DISTINCT ON (workspace_id, path) diff --git a/backend/windmill-api-flows/src/flows.rs b/backend/windmill-api-flows/src/flows.rs index 3c37523082..6353cd429a 100644 --- a/backend/windmill-api-flows/src/flows.rs +++ b/backend/windmill-api-flows/src/flows.rs @@ -612,8 +612,7 @@ async fn create_flow( // Apply folder default_permissioned_as on create when the caller did not // explicitly preserve a value and the user can preserve. - let explicit_preserve = (nf.on_behalf_of_email.is_some() - || nf.on_behalf_of.is_some()) + let explicit_preserve = (nf.on_behalf_of_email.is_some() || nf.on_behalf_of.is_some()) && nf.preserve_on_behalf_of.unwrap_or(false) && windmill_common::can_preserve_on_behalf_of(&authed); if !explicit_preserve && windmill_common::can_preserve_on_behalf_of(&authed) { @@ -633,16 +632,15 @@ async fn create_flow( check_schedule_conflict(&mut tx, &w_id, &nf.path).await?; let schema_str = nf.schema.and_then(|x| serde_json::to_string(&x.0).ok()); - let resolved_on_behalf_of = - windmill_common::resolve_on_behalf_of( - nf.on_behalf_of_email.as_deref(), - nf.on_behalf_of.as_deref(), - nf.preserve_on_behalf_of.unwrap_or(false), - &authed, - &w_id, - &db, - ) - .await?; + let resolved_on_behalf_of = windmill_common::resolve_on_behalf_of( + nf.on_behalf_of_email.as_deref(), + nf.on_behalf_of.as_deref(), + nf.preserve_on_behalf_of.unwrap_or(false), + &authed, + &w_id, + &db, + ) + .await?; // Written beside the principal only while a worker that still reads it may be live. let legacy_on_behalf_of_email = windmill_common::legacy_on_behalf_of_email(resolved_on_behalf_of.as_deref(), &w_id, &db) @@ -1160,16 +1158,15 @@ async fn update_flow( let old_dep_job = not_found_if_none(old_dep_job, "Flow", flow_path)?; let is_new_path = nf.path != flow_path; let schema_str = schema.and_then(|x| serde_json::to_string(&x).ok()); - let resolved_on_behalf_of = - windmill_common::resolve_on_behalf_of( - nf.on_behalf_of_email.as_deref(), - nf.on_behalf_of.as_deref(), - nf.preserve_on_behalf_of.unwrap_or(false), - &authed, - &w_id, - &db, - ) - .await?; + let resolved_on_behalf_of = windmill_common::resolve_on_behalf_of( + nf.on_behalf_of_email.as_deref(), + nf.on_behalf_of.as_deref(), + nf.preserve_on_behalf_of.unwrap_or(false), + &authed, + &w_id, + &db, + ) + .await?; // Written beside the principal only while a worker that still reads it may be live. let legacy_on_behalf_of_email = windmill_common::legacy_on_behalf_of_email(resolved_on_behalf_of.as_deref(), &w_id, &db) diff --git a/backend/windmill-api-groups/src/groups.rs b/backend/windmill-api-groups/src/groups.rs index 69b50e4b83..a15b9823d3 100644 --- a/backend/windmill-api-groups/src/groups.rs +++ b/backend/windmill-api-groups/src/groups.rs @@ -299,7 +299,7 @@ async fn create_igroup( ) -> Result { use uuid::Uuid; - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx = db.begin().await?; let normalized_name = convert_name(&ng.name); @@ -464,7 +464,7 @@ async fn update_igroup( Path(name): Path, Json(igroup_update): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx: Transaction<'_, Postgres> = db.begin().await?; let exists_opt = sqlx::query("SELECT 1 FROM instance_group WHERE name = $1") @@ -656,7 +656,7 @@ async fn delete_igroup( Extension(db): Extension, Path(name): Path, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx: Transaction<'_, Postgres> = db.begin().await?; // FOR UPDATE: the group row is the group-level mutex, taken before the workspace @@ -970,7 +970,7 @@ async fn add_user_igroup( Path(name): Path, Json(Email { email }): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx: Transaction<'_, Postgres> = db.begin().await?; @@ -1189,7 +1189,7 @@ async fn remove_user_igroup( Path(name): Path, Json(Email { email }): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx = db.begin().await?; // FOR UPDATE: the group row is the group-level mutex, taken before the workspace @@ -1330,7 +1330,7 @@ async fn export_igroups( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx = db.begin().await?; let igroups = sqlx::query_as!( ExportedIGroup, @@ -1366,7 +1366,7 @@ async fn overwrite_igroups( Extension(db): Extension, Json(igroups): Json>, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut tx = db.begin().await?; // The import replaces the whole group catalog, so the whole-table lock is its diff --git a/backend/windmill-api-integration-tests/tests/native_triggers.rs b/backend/windmill-api-integration-tests/tests/native_triggers.rs index c86ba3154f..fe80e5fdf4 100644 --- a/backend/windmill-api-integration-tests/tests/native_triggers.rs +++ b/backend/windmill-api-integration-tests/tests/native_triggers.rs @@ -62,6 +62,7 @@ fn test_authed() -> ApiAuthed { is_session_token: false, token_prefix: None, read_only: false, + job_id: None, } } diff --git a/backend/windmill-api-jobs/src/concurrency_groups.rs b/backend/windmill-api-jobs/src/concurrency_groups.rs index b3692ca910..4040d41bd2 100644 --- a/backend/windmill-api-jobs/src/concurrency_groups.rs +++ b/backend/windmill-api-jobs/src/concurrency_groups.rs @@ -1,9 +1,8 @@ -use windmill_api_auth::{check_scopes, ApiAuthed}; +use windmill_api_auth::{check_scopes, is_instance_admin, require_instance_admin, ApiAuthed}; use windmill_common::{ db::{UserDB, DB}, error::Error::PermissionDenied, error::{self, JsonResult}, - utils::require_admin, }; use crate::query::{filter_list_completed_query, filter_list_queue_query}; @@ -44,7 +43,9 @@ async fn list_concurrency_groups( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult> { - require_admin(authed.is_admin, &authed.username)?; + // Instance-global: the listing spans every workspace's concurrency keys, so a job + // token's workspace-admin claim must not reach it (mirrors the prune route below). + require_instance_admin(&authed)?; let concurrency_counts = sqlx::query_as::<_, (String, i64)>( "SELECT concurrency_id, (select COUNT(*) from jsonb_object_keys(job_uuids)) as n_job_uuids FROM concurrency_counter", @@ -67,7 +68,9 @@ async fn prune_concurrency_group( Extension(db): Extension, Path(concurrency_key): Path, ) -> JsonResult<()> { - if !authed.is_admin { + // Global concurrency-group pruning gated on the caller's own is_admin claim, + // so a job token (capped at workspace admin) must not pass. + if !is_instance_admin(&authed) { return Err(PermissionDenied( "Only administrators can delete concurrency groups".to_string(), )); @@ -283,7 +286,8 @@ async fn get_concurrent_intervals( // This second transaction uses the db, so it will fetch information // potentially forbidden to the user. It must be obscured before // returning it - let running_jobs_db: Vec = if lq.success.is_none() && lq.resolved != Some(true) { + let running_jobs_db: Vec = if lq.success.is_none() && lq.resolved != Some(true) + { sqlx::query_as(&sql_q).fetch_all(&db).await? } else { vec![] diff --git a/backend/windmill-api-jobs/src/execution.rs b/backend/windmill-api-jobs/src/execution.rs index ebee1bcbbf..fbfd656cdf 100644 --- a/backend/windmill-api-jobs/src/execution.rs +++ b/backend/windmill-api-jobs/src/execution.rs @@ -56,7 +56,9 @@ pub async fn check_tag_available_for_workspace( ) -> error::Result<()> { if let Some(tag) = tag.as_deref().filter(|t| !t.is_empty()) { let tags = get_scope_tags(authed); - check_tag_available_for_workspace_internal(db, w_id, tag, &authed.email, tags).await + // Job-aware: a WM_TOKEN running as a superadmin must not unlock restricted tags. + let is_super_admin = windmill_api_auth::is_super_admin_authed(db, authed).await?; + check_tag_available_for_workspace_internal(db, w_id, tag, is_super_admin, tags).await } else { Ok(()) } diff --git a/backend/windmill-api-schedule/src/lib.rs b/backend/windmill-api-schedule/src/lib.rs index ec790ec060..6ae902e984 100644 --- a/backend/windmill-api-schedule/src/lib.rs +++ b/backend/windmill-api-schedule/src/lib.rs @@ -339,6 +339,13 @@ async fn create_schedule( ) .await?; + // Reject a forged superadmin run identity in a preserved permissioned_as + // (the sentinel guard; the email is derived from it so it always belongs). + windmill_common::auth::validate_on_behalf_of( + Some(&resolved_permissioned_as), + Some(&resolved_email), + )?; + let mut tx: Transaction<'_, Postgres> = user_db.begin(&authed).await?; check_path_conflict(&mut tx, &w_id, &ns.path).await?; @@ -571,6 +578,13 @@ async fn edit_schedule( authed.email.clone() }; + // Reject a forged superadmin run identity in a preserved permissioned_as + // (the sentinel guard; the email is derived from it so it always belongs). + windmill_common::auth::validate_on_behalf_of( + Some(&resolved_permissioned_as), + Some(&resolved_email), + )?; + let before = trigger_history::snapshot_row(&mut *tx, "schedule", &w_id, path).await?; let schedule = sqlx::query_as!( @@ -1413,7 +1427,7 @@ async fn set_default_error_handler( Path(w_id): Path, Json(payload): Json, ) -> Result<()> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let (key, value) = match payload.handler_type { HandlerType::Error => { let key = format!("default_error_handler_{}", w_id); diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index 7c2453d848..a9bc601591 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -50,7 +50,6 @@ use windmill_common::secret_backend::{ AwsSecretsManagerSettings, AzureKeyVaultSettings, SecretMigrationReport, VaultSettings, }; use windmill_common::{ - auth::is_super_admin_email, ee_oss::{get_license_plan, LicensePlan}, email_oss::{send_email_plain_text, SMTP_ENABLED}, error::{self, pg_error_message, JsonResult, Result}, @@ -236,7 +235,7 @@ pub async fn test_email( authed: ApiAuthed, Json(test_email): Json, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; if !SMTP_ENABLED { return Err(error::Error::Generic( axum::http::StatusCode::NOT_IMPLEMENTED, @@ -290,7 +289,7 @@ pub async fn test_s3_bucket( // local-filesystem surface (see validate_object_storage_test). On self-hosted instances the // object store usually lives on the local/private network and all authenticated users are // trusted, so testing there stays unrestricted. Super admins keep the unrestricted path too. - let is_super_admin = is_super_admin_email(&db, &authed.email).await?; + let is_super_admin = windmill_api_auth::is_super_admin_authed(&db, &authed).await?; let restrict = !is_super_admin && *CLOUD_HOSTED; if restrict { validate_object_storage_test(&test_s3_bucket).await?; @@ -590,7 +589,7 @@ async fn get_object_storage_usage( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; Ok(Json(storage_usage::get_status(&db).await?)) } @@ -599,7 +598,7 @@ async fn compute_object_storage_usage( Extension(db): Extension, authed: ApiAuthed, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; storage_usage::try_start(&db).await?; storage_usage::spawn_compute(db.clone()); Ok(axum::http::StatusCode::ACCEPTED) @@ -610,7 +609,7 @@ async fn run_log_cleanup( Extension(db): Extension, authed: ApiAuthed, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; log_cleanup::try_start(&db).await?; log_cleanup::spawn_cleanup(db.clone()); Ok(axum::http::StatusCode::ACCEPTED) @@ -621,7 +620,7 @@ async fn log_cleanup_status( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; Ok(Json(log_cleanup::get_status(&db).await?)) } @@ -630,7 +629,7 @@ async fn audit_logs_s3_status( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; Ok(Json(audit_logs_s3::get_status(&db).await?)) } @@ -640,7 +639,7 @@ async fn run_audit_logs_s3_backfill( authed: ApiAuthed, Json(req): Json, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; if !matches!(get_license_plan().await, LicensePlan::Enterprise) { return Err(error::Error::BadRequest( "Audit log export to object storage is an Enterprise feature".to_string(), @@ -656,7 +655,7 @@ async fn audit_logs_s3_backfill_status( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; Ok(Json(audit_logs_s3_backfill::get_status(&db).await?)) } @@ -670,7 +669,7 @@ pub async fn test_license_key( authed: ApiAuthed, Json(TestKey { license_key }): Json, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let (_, expired, _offline_meta) = validate_license_key(license_key, Some(&db)).await?; if expired { @@ -691,7 +690,7 @@ pub async fn get_offline_license_status( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let offline = (**windmill_common::ee_oss::LICENSE_OFFLINE_METADATA.load()).clone(); let is_offline = matches!(&offline, Some(m) if m.is_offline()); @@ -717,7 +716,7 @@ pub async fn get_instance_hash( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; #[cfg(feature = "enterprise")] let hash = windmill_common::ee_oss::compute_instance_hash(&db) .await @@ -731,7 +730,7 @@ pub async fn get_local_settings( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let mut settings = serde_json::Map::new(); for key in ENV_SETTINGS.iter() { @@ -790,7 +789,7 @@ pub async fn set_global_setting( Path(key): Path, Json(value): Json, ) -> error::Result<()> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; set_global_setting_internal(&db, key, value.value.unwrap_or(serde_json::Value::Null)).await } @@ -1149,7 +1148,7 @@ async fn get_instance_config( Extension(db): Extension, authed: ApiAuthed, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let config = InstanceConfig::from_db(&db) .await .map_err(|e| error::Error::internal_err(e.to_string()))?; @@ -1160,7 +1159,7 @@ async fn get_instance_config_yaml( Extension(db): Extension, authed: ApiAuthed, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let config = InstanceConfig::from_db(&db) .await .map_err(|e| error::Error::internal_err(e.to_string()))?; @@ -1178,7 +1177,7 @@ async fn set_instance_config( authed: ApiAuthed, Json(desired): Json, ) -> error::Result<()> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let current = InstanceConfig::from_db(&db) .await @@ -1277,7 +1276,7 @@ pub async fn get_global_setting( && key != HTTP_ROUTE_WORKSPACED_ROUTE_SETTING && key != WS_BASE_URL_SETTING { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; } let value = sqlx::query!("SELECT value FROM global_settings WHERE name = $1", key) .fetch_optional(&db) @@ -1301,7 +1300,7 @@ async fn github_app_stale_webhooks( Extension(_db): Extension, authed: ApiAuthed, ) -> JsonResult { - require_super_admin(&_db, &authed.email).await?; + require_super_admin(&_db, &authed).await?; #[cfg(all(feature = "enterprise", feature = "private"))] { let stale = windmill_common::git_sync_ee::stale_webhook_repos(&_db).await?; @@ -1318,7 +1317,7 @@ async fn list_global_settings( Extension(db): Extension, authed: ApiAuthed, ) -> JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let settings = sqlx::query_as!(GlobalSetting, "SELECT name, value FROM global_settings") .fetch_all(&db) .await?; @@ -1334,7 +1333,7 @@ async fn list_global_settings() -> JsonResult { } pub async fn send_stats(Extension(db): Extension, authed: ApiAuthed) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::stats_oss::send_stats( &HTTP_CLIENT, &db, @@ -1351,7 +1350,7 @@ async fn restart_worker_group( authed: ApiAuthed, Path(worker_group): Path, ) -> error::Result { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; sqlx::query!( "INSERT INTO notify_event (channel, payload) VALUES ('restart_worker_group', $1)", @@ -1376,7 +1375,7 @@ pub async fn get_stats( Extension(db): Extension, authed: ApiAuthed, ) -> error::JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let stats = windmill_common::stats_oss::get_stats_payload( &db, &windmill_common::stats_oss::SendStatsReason::Manual, @@ -1406,7 +1405,7 @@ pub async fn get_latest_key_renewal_attempt( Extension(db): Extension, authed: ApiAuthed, ) -> JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let last_attempt = sqlx::query!( "SELECT value, created_at FROM metrics WHERE id = $1 ORDER BY created_at DESC LIMIT 1", @@ -1449,7 +1448,7 @@ pub async fn renew_license_key( Query(LicenseQuery { license_key }): Query, authed: ApiAuthed, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let result = windmill_common::ee_oss::renew_license_key( &HTTP_CLIENT, &db, @@ -1495,7 +1494,7 @@ pub async fn test_critical_channels( authed: ApiAuthed, Json(test_critical_channels): Json>, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; #[cfg(feature = "enterprise")] send_critical_alert( @@ -1519,7 +1518,7 @@ pub async fn get_critical_alerts( authed: ApiAuthed, Query(params): Query, ) -> JsonResult { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; windmill_alerting::get_critical_alerts(db, params, None).await } @@ -1535,7 +1534,7 @@ pub async fn acknowledge_critical_alert( authed: ApiAuthed, Path(id): Path, ) -> error::Result { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; windmill_alerting::acknowledge_critical_alert(db, None, id).await } @@ -1549,7 +1548,7 @@ pub async fn acknowledge_all_critical_alerts( Extension(db): Extension, authed: ApiAuthed, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_alerting::acknowledge_all_critical_alerts(db, None).await } @@ -1607,7 +1606,7 @@ async fn list_custom_instance_pg_databases( )) })?; - if is_super_admin_email(&db, &authed.email).await? { + if windmill_api_auth::is_super_admin_authed(&db, &authed).await? { // Enrich each database with the list of workspaces referencing it through // either a ducklake catalog or a datatable database whose resource_type is // 'instance'. Not stored in DB to avoid drift. @@ -1657,7 +1656,7 @@ async fn refresh_custom_instance_user_pwd( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult<()> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::utils::refresh_custom_instance_user_pwd(&db).await?; windmill_common::utils::refresh_custom_instance_replication_user_pwd(&db).await?; Ok(Json(())) @@ -1696,7 +1695,7 @@ async fn setup_custom_instance_pg_database_inner( dbname: &str, logs: &mut CustomInstanceDbLogs, ) -> Result<()> { - require_super_admin(db, &authed.email).await?; + require_super_admin(db, &authed).await?; logs.super_admin = "OK".to_string(); let wmill_pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; logs.database_credentials = "OK".to_string(); @@ -1812,7 +1811,7 @@ async fn drop_custom_instance_pg_database( Extension(db): Extension, Path(dbname): Path, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::drop_custom_instance_database(&db, &dbname).await?; @@ -1835,7 +1834,7 @@ pub async fn test_secret_backend( authed: ApiAuthed, Json(settings): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::secret_backend::test_vault_connection(&settings, Some(&db)).await?; @@ -1855,7 +1854,7 @@ pub async fn migrate_secrets_to_vault( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_to_vault(&db, &settings).await?; @@ -1875,7 +1874,7 @@ pub async fn migrate_secrets_to_database( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_to_database(&db, &settings).await?; @@ -1892,7 +1891,7 @@ pub async fn test_azure_kv_backend( authed: ApiAuthed, Json(settings): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::secret_backend::test_azure_kv_connection(&settings).await?; @@ -1908,7 +1907,7 @@ pub async fn migrate_secrets_to_azure_kv( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_to_azure_kv(&db, &settings).await?; @@ -1925,7 +1924,7 @@ pub async fn migrate_secrets_from_azure_kv( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_from_azure_kv(&db, &settings).await?; @@ -1940,7 +1939,7 @@ pub async fn test_aws_sm_backend( authed: ApiAuthed, Json(settings): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; windmill_common::secret_backend::test_aws_sm_connection(&settings).await?; Ok("Successfully connected to AWS Secrets Manager".to_string()) } @@ -1952,7 +1951,7 @@ pub async fn migrate_secrets_to_aws_sm( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_to_aws_sm(&db, &settings).await?; Ok(Json(report)) } @@ -1964,7 +1963,7 @@ pub async fn migrate_secrets_from_aws_sm( authed: ApiAuthed, Json(settings): Json, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let report = windmill_common::secret_backend::migrate_secrets_from_aws_sm(&db, &settings).await?; Ok(Json(report)) @@ -2063,7 +2062,7 @@ async fn sync_cached_resource_types( authed: ApiAuthed, Query(SyncResourceTypesQuery { name }): Query, ) -> error::Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; use windmill_common::worker::HUB_RT_CACHE_DIR; let cache_path = format!("{}/resource_types.json", *HUB_RT_CACHE_DIR); diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index 67e1721109..0c83d5f6ec 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -27,7 +27,10 @@ use axum::{ Json, Router, }; use hyper::{header::LOCATION, StatusCode}; -use windmill_api_auth::{forbid_superadmin_job_token, require_super_admin, OptJobAuthed}; +use windmill_api_auth::{ + forbid_elevated_job_token, forbid_job_token_account_destruction, forbid_superadmin_job_token, + require_super_admin, OptJobAuthed, +}; use windmill_common::usernames::{ generate_instance_wide_unique_username, get_instance_username_or_create_pending, }; @@ -427,7 +430,7 @@ async fn list_addable_instance_users( Path(w_id): Path, Query(AddableInstanceUsersQuery { search, per_page }): Query, ) -> JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let per_page = per_page.unwrap_or(10).clamp(1, 100); // An absent search yields '%%', which matches every row. let search = format!( @@ -500,7 +503,7 @@ async fn list_users_as_super_admin( Query(pagination): Query, Query(ActiveUsersOnly { active_only }): Query, ) -> JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let per_page = pagination.per_page.unwrap_or(10000).max(1); let offset = (pagination.page.unwrap_or(1).max(1) - 1) * per_page; @@ -1231,6 +1234,7 @@ async fn join_workspace<'c>( } async fn leave_instance(Extension(db): Extension, authed: ApiAuthed) -> Result { + forbid_job_token_account_destruction(&authed)?; let mut tx = db.begin().await?; sqlx::query!("DELETE FROM password WHERE email = $1", &authed.email) .execute(&mut *tx) @@ -1471,7 +1475,7 @@ async fn update_user( Extension(db): Extension, Json(eu): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; @@ -1647,7 +1651,7 @@ async fn delete_user( Path(email_to_delete): Path, Extension(db): Extension, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; @@ -1728,7 +1732,7 @@ async fn change_user_email( Extension(db): Extension, Json(ce): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; // The target is matched verbatim (accounts predating email normalization can hold uppercase), @@ -2297,12 +2301,10 @@ async fn change_user_email( // Read back inside the transaction: the address is derived at dispatch through a cache // that nothing else evicts, so without this a job pushed in the next 60s would resolve // the old address and with it the wrong superadmin flag and instance groups. - let memberships = sqlx::query_scalar!( - "SELECT workspace_id FROM usr WHERE email = $1", - &new_email - ) - .fetch_all(&mut *tx) - .await?; + let memberships = + sqlx::query_scalar!("SELECT workspace_id FROM usr WHERE email = $1", &new_email) + .fetch_all(&mut *tx) + .await?; tx.commit().await?; @@ -2604,7 +2606,7 @@ async fn set_login_type( OptJobAuthed { job_id, .. }: OptJobAuthed, Json(et): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; @@ -2747,6 +2749,18 @@ async fn refresh_token( authed: ApiAuthed, cookies: Cookies, ) -> Result { + // The session token minted below is database-backed and carries no job provenance, + // so a job token that exchanged itself for one would shed the `job_id` every + // `$WM_TOKEN` cap keys off (GHSA-hfh4-cx4h-3fcr). Only a browser session refreshes. + if authed.job_id.is_some() { + return Err(Error::NotAuthorized( + "This endpoint cannot be called with a job token ($WM_TOKEN). If a script \ + genuinely needs a token of its own, create a dedicated token from the User \ + settings drawer (the 'Tokens' section), store it as a secret, and use that \ + token explicitly instead of $WM_TOKEN." + .to_string(), + )); + } if let Some(thresh_s) = query.if_expiring_in_less_than_s { let t_hash = windmill_common::auth::hash_token(&token); let not_expired = sqlx::query_scalar!("SELECT true FROM token WHERE token_hash = $1 and expiration IS NOT NULL and expiration > now() + $2::int * '1 sec'::interval", &t_hash, thresh_s) @@ -2890,7 +2904,7 @@ async fn create_token( OptJobAuthed { job_id, .. }: OptJobAuthed, Json(token_config): Json, ) -> Result<(StatusCode, String)> { - forbid_superadmin_job_token(&db, &authed.email, job_id).await?; + forbid_elevated_job_token(&db, &authed.email, job_id).await?; check_token_create_rate_limit(&authed.username)?; // `username_override_from_label` trusts a server-minted label to name the entity acting, @@ -2934,7 +2948,7 @@ async fn impersonate( } else { Some(&token) }; - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; if new_token.impersonate_email.is_none() { @@ -3089,6 +3103,7 @@ async fn delete_token( authed: ApiAuthed, Path(token_prefix): Path, ) -> Result { + forbid_job_token_account_destruction(&authed)?; let mut tx = db.begin().await?; let tokens_deleted: Vec = sqlx::query_scalar( @@ -3133,6 +3148,11 @@ async fn update_token_scopes( Path(token_prefix): Path, Json(req): Json, ) -> Result { + // Widening is what makes a narrowly-scoped mint (app embed, raw-app SDK, MCP + // OAuth) recoverable as a general credential: a job token is unscoped, so the + // caller check below would let it clear the scopes of any token sharing its + // email (GHSA-hfh4-cx4h-3fcr). + forbid_elevated_job_token(&db, &authed.email, authed.job_id).await?; windmill_api_auth::ensure_scopes_within_caller(&authed, req.scopes.as_deref())?; let mut tx = db.begin().await?; @@ -3261,6 +3281,7 @@ async fn leave_workspace( Path(w_id): Path, authed: ApiAuthed, ) -> Result { + forbid_job_token_account_destruction(&authed)?; let mut tx = db.begin().await?; sqlx::query!( "DELETE FROM usr WHERE workspace_id = $1 AND username = $2", @@ -3402,11 +3423,11 @@ struct WorkspaceUsernameInfo { username: String, } async fn get_instance_username_info( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Path(user_email): Path, Extension(db): Extension, ) -> JsonResult { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; let mut tx = db.begin().await?; let instance_username = match sqlx::query_scalar!( "SELECT username FROM password WHERE email = $1", @@ -3476,7 +3497,7 @@ async fn export_global_users( authed: ApiAuthed, OptJobAuthed { job_id, .. }: OptJobAuthed, ) -> JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; let users = sqlx::query_as!( @@ -3516,7 +3537,7 @@ async fn overwrite_global_users( OptJobAuthed { job_id, .. }: OptJobAuthed, Json(users): Json>, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; sqlx::query!("DELETE FROM password") diff --git a/backend/windmill-api-workers/src/lib.rs b/backend/windmill-api-workers/src/lib.rs index f33e12470f..6ee55ed003 100644 --- a/backend/windmill-api-workers/src/lib.rs +++ b/backend/windmill-api-workers/src/lib.rs @@ -103,7 +103,7 @@ async fn list_worker_pings( Extension(user_db): Extension, Query(query): Query, ) -> JsonResult> { - let has_devops_role = require_devops_role(&db, &authed.email).await.is_ok(); + let has_devops_role = require_devops_role(&db, &authed).await.is_ok(); if *HIDE_WORKERS_FOR_NON_ADMINS && !has_devops_role { return Ok(Json(vec![])); } @@ -159,7 +159,7 @@ async fn exists_workers_with_tags( // When TAGS_ARE_SENSITIVE is enabled, filter tags based on workspace visibility if *TAGS_ARE_SENSITIVE { - let has_devops_role = require_devops_role(&db, &authed.email).await.is_ok(); + let has_devops_role = require_devops_role(&db, &authed).await.is_ok(); if !has_devops_role { if let Some(ref workspace) = tags_query.workspace { // This route is global, so the workspace is an unauthorized query param: check @@ -229,7 +229,7 @@ async fn get_custom_tags( return Ok(Json(all_tags)); } if *TAGS_ARE_SENSITIVE { - let has_devops_role = require_devops_role(&db, &authed.email).await.is_ok(); + let has_devops_role = require_devops_role(&db, &authed).await.is_ok(); if !has_devops_role { return Ok(Json(vec![])); } @@ -268,7 +268,7 @@ async fn get_queue_metrics( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let queue_metrics = sqlx::query_as!( QueueMetric, @@ -293,7 +293,7 @@ async fn get_queue_counts( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let queue_counts = windmill_common::queue::get_queue_counts(&db).await; Ok(Json(queue_counts)) } @@ -302,7 +302,7 @@ async fn get_queue_running_counts( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; let queue_running_counts = windmill_common::queue::get_queue_running_counts(&db).await; Ok(Json(queue_running_counts)) } @@ -327,7 +327,7 @@ async fn get_workspace_fairness_events( authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult> { - require_devops_role(&db, &authed.email).await?; + require_devops_role(&db, &authed).await?; // No cloud-host gate — workspace fairness is an Enterprise feature // available on any multi-tenant EE deployment. Non-EE / non-enabled diff --git a/backend/windmill-api-workspaces/src/datatable_migrations.rs b/backend/windmill-api-workspaces/src/datatable_migrations.rs index 5b1dde395a..14c6d49132 100644 --- a/backend/windmill-api-workspaces/src/datatable_migrations.rs +++ b/backend/windmill-api-workspaces/src/datatable_migrations.rs @@ -777,7 +777,7 @@ async fn datatable_migrations_status( /// Only workspace admins and super admins may opt a data table in or out of /// migrations. async fn require_datatable_migrations_manager(db: &DB, authed: &ApiAuthed) -> Result<()> { - if authed.is_admin || require_super_admin(db, &authed.email).await.is_ok() { + if authed.is_admin || require_super_admin(db, &authed).await.is_ok() { Ok(()) } else { Err(Error::BadRequest( diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 9bc4890fe6..b25b4a18f5 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -7,8 +7,8 @@ */ use windmill_api_auth::{ - build_scope_path_predicate, check_scopes, require_devops_role, require_is_writer, - require_super_admin, ApiAuthed, + build_scope_path_predicate, check_scopes, require_devops_role, require_instance_admin, + require_is_writer, require_super_admin, ApiAuthed, }; use windmill_api_users::users::WorkspaceInvite; use windmill_common::email_oss::send_email_if_possible; @@ -2846,7 +2846,7 @@ async fn create_pg_database( windmill_common::validate_dbname(&req.target_dbname)?; // Non-superadmin: restrict dbname to wm_fork_ prefix - if !windmill_common::auth::is_super_admin_email(&db, &authed.email).await? { + if !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { if !req.target_dbname.starts_with("wm_fork_") { return Err(Error::BadRequest( "Non-superadmin users can only create databases with names starting with 'wm_fork_'" @@ -2963,7 +2963,7 @@ async fn import_pg_database( resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.target).await?; if let Some(ref override_dbname) = req.target_dbname_override { - if !windmill_common::auth::is_super_admin_email(&db, &authed.email).await? { + if !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { if !override_dbname.starts_with("wm_fork_") { return Err(Error::BadRequest( "Non-superadmin users can only override target dbname with names starting with 'wm_fork_'" @@ -3037,7 +3037,7 @@ async fn edit_ducklake_config( Json(new_config): Json, ) -> Result { require_admin(is_admin, &username)?; - let is_superadmin = require_super_admin(&db, &email).await.is_ok(); + let is_superadmin = require_super_admin(&db, &authed).await.is_ok(); // Lake names end up interpolated in `ATTACH 'ducklake://'`, // generated maintenance SQL and the reserved maintenance schedule path @@ -3130,11 +3130,11 @@ async fn edit_datatable_config( authed: ApiAuthed, Extension(db): Extension, Path(w_id): Path, - ApiAuthed { is_admin, username, email, .. }: ApiAuthed, + ApiAuthed { is_admin, username, .. }: ApiAuthed, Json(mut new_config): Json, ) -> Result { require_admin(is_admin, &username)?; - let is_superadmin = require_super_admin(&db, &email).await.is_ok(); + let is_superadmin = require_super_admin(&db, &authed).await.is_ok(); let mut tx = db.begin().await?; @@ -4783,7 +4783,7 @@ async fn set_encryption_key( Path(w_id): Path, Json(request): Json, ) -> Result<()> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; if !WORKSPACE_KEY_REGEXP.is_match(request.new_key.as_str()) { return Err(Error::BadRequest( @@ -4939,7 +4939,7 @@ async fn get_workspace_as_superadmin( Extension(db): Extension, Path(w_id): Path, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let workspace = sqlx::query_as!( Workspace, "SELECT @@ -4970,9 +4970,8 @@ async fn list_workspaces_as_super_admin( Extension(db): Extension, Extension(user_db): Extension, Query(pagination): Query, - ApiAuthed { email, .. }: ApiAuthed, ) -> JsonResult> { - require_devops_role(&db, &email).await?; + require_devops_role(&db, &authed).await?; let (per_page, offset) = paginate(pagination); let mut tx = user_db.begin(&authed).await?; @@ -5044,7 +5043,7 @@ struct SessionWorkspaceStatusRequest { /// lingering, so it is deliberately not treated as unreachable. async fn session_workspace_status( Extension(db): Extension, - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Json(req): Json, ) -> JsonResult> { if req.workspace_ids.len() > 1000 { @@ -5052,7 +5051,8 @@ async fn session_workspace_status( "Too many workspace ids (max 1000)".to_string(), )); } - let is_superadmin = windmill_common::auth::is_super_admin_email(&db, &email).await?; + let email = &authed.email; + let is_superadmin = windmill_api_auth::is_super_admin_authed(&db, &authed).await?; let rows = sqlx::query!( // A missing workspace row must be caught before the membership arm: for a // superadmin the two arms below both fall through, and a hard-deleted workspace @@ -5254,7 +5254,7 @@ async fn create_workspace( Json(nw): Json, ) -> Result { if *CREATE_WORKSPACE_REQUIRE_SUPERADMIN { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; } #[cfg(not(feature = "enterprise"))] @@ -6837,7 +6837,7 @@ async fn create_workspace_fork_branch( } if *DISABLE_WORKSPACE_FORK { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; } if let RuleCheckResult::Blocked(msg) = check_user_against_rule( &w_id, @@ -7249,7 +7249,7 @@ async fn create_workspace_fork( _check_nb_of_workspaces(&db).await?; if *DISABLE_WORKSPACE_FORK { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; } if let RuleCheckResult::Blocked(msg) = check_user_against_rule( &parent_workspace_id, @@ -7592,7 +7592,7 @@ async fn attach_dev_workspace( .fetch_optional(&db) .await? .unwrap_or(false); - if !is_admin_of_dev && !windmill_common::auth::is_super_admin_email(&db, &authed.email).await? { + if !is_admin_of_dev && !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { return Err(Error::PermissionDenied(format!( "Attaching workspace '{dev_w_id}' as a dev requires being an admin of it (or a superadmin)" ))); @@ -8095,9 +8095,7 @@ async fn archive_workspace( .fetch_optional(&db) .await? .unwrap_or(false); - if !is_prod_admin - && !windmill_common::auth::is_super_admin_email(&db, &authed.email).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)" ))); @@ -8172,6 +8170,7 @@ async fn leave_workspace( Path(w_id): Path, authed: ApiAuthed, ) -> Result { + 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", @@ -8201,7 +8200,9 @@ async fn unarchive_workspace( Path(w_id): Path, authed: ApiAuthed, ) -> Result { - require_admin(authed.is_admin, &authed.username)?; + // Global route (unarchives any workspace by id) gated on the caller's own + // is_admin claim, so it must reject a job token — see require_instance_admin. + require_instance_admin(&authed)?; // Unarchiving re-activates a soft-deleted workspace, so it must respect the // same CE workspace-count cap as creating one. The archived workspace is @@ -10222,7 +10223,7 @@ async fn compare_workspaces( // source AND the fork (superadmin satisfies both), which guarantees full // visibility of every item on every side. `fork_authed.is_admin` already folds // in superadmin; `authed.is_admin` (source side) does not, so OR it in. - let is_super_admin = windmill_common::auth::is_super_admin_email(&db, &authed.email).await?; + let is_super_admin = windmill_api_auth::is_super_admin_authed(&db, &authed).await?; let sees_all_items = is_super_admin || (authed.is_admin && fork_authed.is_admin); let all_ahead_items_visible = all_ahead_items_visible || sees_all_items; let all_behind_items_visible = all_behind_items_visible || sees_all_items; @@ -10624,8 +10625,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 @@ -10650,6 +10654,7 @@ async fn load_workspace_authed( is_session_token: base_authed.is_session_token, token_prefix: base_authed.token_prefix.clone(), read_only: base_authed.read_only, + job_id: base_authed.job_id, }); }; @@ -10681,6 +10686,7 @@ async fn load_workspace_authed( is_session_token: base_authed.is_session_token, token_prefix: base_authed.token_prefix.clone(), read_only: base_authed.read_only, + job_id: base_authed.job_id, }) } diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 55f7c4f15d..1968437e40 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -21,7 +21,6 @@ use windmill_audit::ActionKind; use windmill_common::worker::CLOUD_HOSTED; use windmill_common::{ - auth::is_super_admin_email, db::UserDB, error::{Error, Result}, utils::require_admin, @@ -43,14 +42,14 @@ pub(crate) async fn change_workspace_id( Extension(db): Extension, Json(rw): Json, ) -> Result { - if *CLOUD_HOSTED && !is_super_admin_email(&db, &authed.email).await? { + if *CLOUD_HOSTED && !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { return Err(Error::BadRequest( "This feature is not available on the cloud".to_string(), )); } if *CREATE_WORKSPACE_REQUIRE_SUPERADMIN { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; } else { require_admin(authed.is_admin, &authed.username)?; } @@ -929,7 +928,7 @@ pub(crate) async fn delete_workspace( let mut tx = db.begin().await?; if !(is_fork && is_workspace_owner(&authed, &w_id, &mut tx).await?) - && !is_super_admin_email(&db, &authed.email).await? + && !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { return Err(Error::PermissionDenied( "Deleting this workspace requires being the fork's owner or a superadmin".to_string(), @@ -1297,7 +1296,7 @@ pub async fn drop_forked_datatable_databases( let is_fork = workspace_is_fork(&db, &w_id).await?; let mut tx = db.begin().await?; if !(is_fork && is_workspace_owner(&authed, &w_id, &mut tx).await?) - && !is_super_admin_email(&db, &authed.email).await? + && !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { return Err(Error::PermissionDenied( "Dropping forked datatable databases requires being the fork's owner or a superadmin" @@ -1454,7 +1453,7 @@ pub async fn drop_forked_ducklake_namespaces( let is_fork = workspace_is_fork(&db, &w_id).await?; let mut tx = db.begin().await?; if !(is_fork && is_workspace_owner(&authed, &w_id, &mut tx).await?) - && !is_super_admin_email(&db, &authed.email).await? + && !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { return Err(Error::PermissionDenied( "Dropping forked ducklake namespaces requires being the fork's owner or a superadmin" @@ -1959,7 +1958,7 @@ async fn require_prod_admin_for_dev_workspace( .fetch_optional(db) .await? .unwrap_or(false); - if !is_prod_admin && !is_super_admin_email(db, &authed.email).await? { + if !is_prod_admin && !windmill_api_auth::is_super_admin_authed(db, &authed).await? { return Err(Error::PermissionDenied(format!( "Destroying dev workspace '{w_id}' or its data requires being an admin of its parent prod workspace '{prod}' (or a superadmin)" ))); diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 3b0fc60d79..4a581443ad 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -84,7 +84,7 @@ use windmill_object_store::object_store_reexports::{Attribute, Attributes}; use windmill_store::resources::get_resource_value_interpolated_internal; use windmill_api_auth::{ - create_token_internal, ensure_scopes_within_caller, forbid_superadmin_job_token, NewToken, + create_token_internal, ensure_scopes_within_caller, forbid_elevated_job_token, NewToken, OptJobAuthed, }; use windmill_git_sync::{handle_deployment_metadata, DeployedObject}; @@ -1338,7 +1338,9 @@ async fn mint_raw_app_sdk_token( ) -> Result<(String, chrono::DateTime)> { // This credential outlives the request, so an ephemeral job token must not be // able to launder itself into one — the reason `users/tokens/create` refuses. - forbid_superadmin_job_token(db, &authed.email, job_id).await?; + // The minted scopes do not contain it: `users/tokens/update_scopes` can widen + // any token of the same email. + forbid_elevated_job_token(db, &authed.email, job_id).await?; // An embed token represents untrusted app JS; it must not bootstrap a // broader SDK credential (same guard as `mint_app_embed_token`). if windmill_api_auth::scopes::has_app_embed_sentinel(authed.scopes.as_deref()) { @@ -1411,7 +1413,7 @@ pub async fn build_embed_token_response( _ => (None, None), } } else if policy.sandbox { - let resp = mint_app_embed_token(db, w_id, app_path, opt_authed).await?; + let resp = mint_app_embed_token(db, w_id, app_path, opt_authed, job_id).await?; (resp.token, resp.expiration) } else { (None, None) @@ -1535,8 +1537,13 @@ pub async fn mint_app_embed_token( w_id: &str, app_path: &str, opt_authed: Option<&ApiAuthed>, + job_id: Option, ) -> Result { let token_and_exp = if let Some(authed) = opt_authed { + // This credential outlives the request and its narrow scopes are not the + // boundary — `users/tokens/update_scopes` can widen any same-email token — + // so an elevated job token must not mint one (GHSA-hfh4-cx4h-3fcr). + forbid_elevated_job_token(db, &authed.email, job_id).await?; // An app embed token represents untrusted app JS in the sandboxed iframe; it // must never reach this mint path to renew itself. The 12h expiry is the // blast-radius cap on a leaked embed token, and `ensure_scopes_within_caller` @@ -2186,6 +2193,14 @@ async fn create_app_internal<'a>( } } + // Reject a forged superadmin run identity in the (possibly preserved) policy. + // Done on the non-RLS pool before the transaction below, like the resolution + // above, to avoid holding a second connection while `tx` is checked out. + windmill_common::auth::validate_on_behalf_of( + app.policy.on_behalf_of.as_deref(), + app.policy.on_behalf_of_email.as_deref(), + )?; + let mut tx = user_db.clone().begin(&authed).await?; let path = app.path.clone(); if &app.path == "" { @@ -3074,6 +3089,22 @@ async fn update_app_internal<'a>( check_scopes(&authed, || format!("apps:write:{}", npath))?; } + // Reject a forged superadmin run identity in a preserved policy. Mirror the + // `should_preserve` gate below (only a preserved value is caller-controlled; + // otherwise the policy is rewritten to the deployer's own identity) and run + // it on the non-RLS pool before the transaction to avoid a second connection. + if let Some(npolicy) = ns.policy.as_ref() { + let should_preserve = ns.preserve_on_behalf_of.unwrap_or(false) + && windmill_common::can_preserve_on_behalf_of(&authed) + && npolicy.on_behalf_of.is_some(); + if should_preserve { + windmill_common::auth::validate_on_behalf_of( + npolicy.on_behalf_of.as_deref(), + npolicy.on_behalf_of_email.as_deref(), + )?; + } + } + let mut tx = user_db.clone().begin(&authed).await?; // `app_version.raw_app` is set by whichever endpoint writes the version, so a @@ -5109,6 +5140,18 @@ fn get_on_behalf_of(policy: &Policy) -> Result<(String, String)> { ) })? .to_string(); + // Defence in depth against a policy that already carries a forged superadmin + // sentinel (deployed before validation existed, or copied verbatim by a + // workspace fork): the sentinels are internal-only and never a legitimate app + // run identity, so refuse to execute rather than mint a superadmin token. + if windmill_common::auth::is_reserved_on_behalf_of_identity( + Some(&permissioned_as), + Some(&email), + ) { + return Err(Error::BadRequest( + "app on_behalf_of is a reserved internal identity and cannot be executed".to_string(), + )); + } Ok((permissioned_as, email)) } diff --git a/backend/windmill-api/src/db_health.rs b/backend/windmill-api/src/db_health.rs index 0b2e86567b..02b2efd182 100644 --- a/backend/windmill-api/src/db_health.rs +++ b/backend/windmill-api/src/db_health.rs @@ -197,10 +197,10 @@ struct SlowQueriesQuery { } async fn get_db_health( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, ) -> JsonResult { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; let (database_size, connection_pool, table_maintenance, slow_queries, datatables) = tokio::try_join!( fetch_database_size(&db), @@ -220,11 +220,11 @@ async fn get_db_health( } async fn get_db_health_jobs( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Query(query): Query, ) -> JsonResult { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; let scan_limit = query.scan_limit.unwrap_or(10_000).clamp(1_000, 1_000_000); @@ -237,20 +237,20 @@ async fn get_db_health_jobs( } async fn get_slow_queries( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Query(query): Query, ) -> JsonResult> { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; let sort = query.sort.unwrap_or(SlowQuerySort::Total); Ok(Json(fetch_slow_queries(&db, sort).await?)) } async fn reset_slow_queries( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, ) -> windmill_common::error::Result { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; sqlx::query("SELECT pg_stat_statements_reset()") .execute(&db) .await diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 584c44bbec..513341b6c1 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -26,8 +26,6 @@ use tokio::io::AsyncReadExt; use tower::ServiceBuilder; use url::Url; use windmill_common::assets::AssetUsageAccessType; -#[cfg(all(feature = "enterprise", feature = "instance_smtp"))] -use windmill_common::auth::is_super_admin_email; use windmill_common::auth::TOKEN_PREFIX_LEN; #[cfg(feature = "run_inline")] use windmill_common::client::AuthedClient; @@ -2621,7 +2619,7 @@ async fn send_email_with_instance_smtp( let is_handler_job = authed.email == EMAIL_ERROR_HANDLER_USER_EMAIL || authed.email == SCHEDULE_ERROR_HANDLER_USER_EMAIL; - if !is_handler_job && !is_super_admin_email(&db, &authed.email).await? { + if !is_handler_job && !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { return Err(Error::NotAuthorized( "Only super admin or whitelisted token can access email workspace error handler feature" .to_string(), @@ -8904,7 +8902,7 @@ async fn add_batch_jobs( Path((w_id, n)): Path<(String, i32)>, Json(batch_info): Json, ) -> error::JsonResult> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let ( hash, @@ -10880,11 +10878,11 @@ struct TagCount { } async fn count_by_tag( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Query(query): Query, ) -> JsonResult> { - require_super_admin(&db, &email).await?; + require_super_admin(&db, &authed).await?; let horizon = query.horizon_secs.unwrap_or(3600); // Default to 1 hour if not specified let counts = sqlx::query_as!( @@ -11618,6 +11616,7 @@ mod approval_view_gate_tests { is_session_token: false, token_prefix: None, read_only: false, + job_id: None, } } diff --git a/backend/windmill-api/src/lib.rs b/backend/windmill-api/src/lib.rs index d2e0ea8e78..9963219993 100644 --- a/backend/windmill-api/src/lib.rs +++ b/backend/windmill-api/src/lib.rs @@ -374,6 +374,7 @@ async fn inject_agent_authed( is_session_token: false, token_prefix: None, read_only: false, + job_id: None, }, job_id: None, }); diff --git a/backend/windmill-api/src/mcp/oauth_server.rs b/backend/windmill-api/src/mcp/oauth_server.rs index c5142c3031..d7c8c35586 100644 --- a/backend/windmill-api/src/mcp/oauth_server.rs +++ b/backend/windmill-api/src/mcp/oauth_server.rs @@ -18,6 +18,7 @@ use windmill_common::{ }; use crate::db::ApiAuthed; +use windmill_api_auth::forbid_elevated_job_token; use windmill_mcp::parse_mcp_scopes; /// Token expiration for MCP OAuth tokens (1 week in seconds) @@ -763,6 +764,13 @@ async fn oauth_approve_inner( workspace_id: &str, form: ApprovalForm, ) -> Result> { + // The code approved here is exchanged for a database token carrying only this + // email, so an elevated job token would launder its identity into a credential + // with no `job_id` — and the MCP gateway would then re-enter the API uncapped + // (GHSA-hfh4-cx4h-3fcr). Guarded at the shared inner fn: both the workspaced and + // the gateway approve route reach the exchange through here. + forbid_elevated_job_token(db, &authed.email, authed.job_id).await?; + // Verify user is a member of the workspace let is_member = sqlx::query_scalar!( "SELECT EXISTS(SELECT 1 FROM usr WHERE workspace_id = $1 AND email = $2 AND NOT disabled)", diff --git a/backend/windmill-api/src/mcp/utils.rs b/backend/windmill-api/src/mcp/utils.rs index 3af0498ff3..6dd2823959 100644 --- a/backend/windmill-api/src/mcp/utils.rs +++ b/backend/windmill-api/src/mcp/utils.rs @@ -634,11 +634,22 @@ pub async fn create_http_request( .map_err(|e| ErrorData::internal_error(format!("Invalid proxied URL: {}", e), None))?; let scopes = jwt_scopes_for_proxied_route(api_authed.scopes.as_deref(), method, parsed.path())?; - // Add authorization header + // Add authorization header. Carry the caller's job provenance into the proxy + // JWT: a job's WM_TOKEN is capped at workspace admin (GHSA-hfh4-cx4h-3fcr), and + // dropping `job_id` here would re-mint an uncapped token that satisfies + // require_super_admin / require_devops_role on the proxied route. let authed = Authed::from(api_authed.clone()); - let token = create_jwt_token(authed, workspace_id, 3600, None, None, None, scopes) - .await - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let token = create_jwt_token( + authed, + workspace_id, + 3600, + api_authed.job_id, + None, + None, + scopes, + ) + .await + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; request_builder = request_builder.header("Authorization", format!("Bearer {}", token)); // Add body if present @@ -1102,4 +1113,102 @@ mod tests { Some("((o.path = 'f/a_b' OR o.path LIKE 'f/a\\_b/%' ESCAPE '\\'))".to_string()) ); } + + fn test_api_authed(job_id: Option) -> ApiAuthed { + ApiAuthed { + email: "admin@windmill.dev".to_string(), + username: "admin".to_string(), + is_admin: true, + is_operator: false, + groups: vec![], + folders: vec![], + scopes: None, + username_override: None, + username_override_is_token_label: false, + is_session_token: false, + token_prefix: None, + read_only: false, + job_id, + } + } + + /// Capture the `Authorization` header of the single request `create_http_request` + /// proxies, decode the minted JWT, and return its `job_id` claim. + async fn proxied_jwt_job_id(caller: &ApiAuthed) -> Option { + use axum::{extract::State, routing::get, Router}; + use std::sync::{Arc, Mutex}; + use windmill_common::auth::JWTAuthClaims; + + // The internal JWT secret must be non-empty for encode/decode to round-trip. + windmill_common::jwt::JWT_SECRET.store(Arc::new("mytestsecret".to_string())); + + let captured: Arc>> = Arc::new(Mutex::new(None)); + let app = Router::new() + .route( + "/", + get( + |State(state): State>>>, + headers: axum::http::HeaderMap| async move { + if let Some(auth) = headers.get(axum::http::header::AUTHORIZATION) { + *state.lock().unwrap() = + Some(auth.to_str().unwrap_or_default().to_string()); + } + "ok" + }, + ), + ) + .with_state(captured.clone()); + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + + let url = format!("http://{addr}/"); + create_http_request("GET", &url, "test-workspace", caller, None) + .await + .expect("proxied request should succeed"); + + server.abort(); + + let header = captured + .lock() + .unwrap() + .clone() + .expect("no auth header captured"); + let token = header.strip_prefix("Bearer ").unwrap().to_string(); + let jwt = token + .strip_prefix("jwt_") + .expect("expected an internal jwt_ token"); + let claims: JWTAuthClaims = windmill_common::jwt::decode_with_internal_secret(jwt) + .await + .unwrap(); + claims.job_id + } + + /// Regression for GHSA-hfh4-cx4h-3fcr: the MCP proxy must carry the caller's + /// job provenance into the JWT it mints, otherwise a job's WM_TOKEN — capped at + /// workspace admin — would be re-minted uncapped and pass require_super_admin / + /// require_devops_role on the proxied route (e.g. listWorkers). + #[tokio::test] + async fn create_http_request_preserves_job_id_provenance() { + let job_id = uuid::Uuid::from_u128(0x0123_4567_89ab_cdef_0123_4567_89ab_cdef); + assert_eq!( + proxied_jwt_job_id(&test_api_authed(Some(job_id))).await, + Some(job_id.to_string()), + "a job-token caller's job_id must be preserved in the proxied JWT" + ); + } + + /// The mirror invariant: a non-job caller must not gain a spurious job_id (which + /// would wrongly cap a legitimate interactive/superadmin MCP token). + #[tokio::test] + async fn create_http_request_keeps_non_job_caller_unstamped() { + assert_eq!( + proxied_jwt_job_id(&test_api_authed(None)).await, + None, + "a non-job caller must not be stamped with a job_id" + ); + } } diff --git a/backend/windmill-api/src/offboarding.rs b/backend/windmill-api/src/offboarding.rs index f7f5831d0f..0d9529c0ce 100644 --- a/backend/windmill-api/src/offboarding.rs +++ b/backend/windmill-api/src/offboarding.rs @@ -463,7 +463,7 @@ pub(crate) async fn global_offboard_preview( Extension(db): Extension, Path(email): Path, ) -> JsonResult { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let workspaces = sqlx::query!( "SELECT workspace_id, username FROM usr WHERE email = $1", @@ -492,7 +492,7 @@ pub(crate) async fn offboard_global_user( Path(email): Path, Json(req): Json, ) -> Result> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let workspaces = sqlx::query!( diff --git a/backend/windmill-api/src/service_logs.rs b/backend/windmill-api/src/service_logs.rs index b1902f7c1a..99dc7c41a8 100644 --- a/backend/windmill-api/src/service_logs.rs +++ b/backend/windmill-api/src/service_logs.rs @@ -43,12 +43,12 @@ pub struct LogFile { pub json_fmt: bool, } async fn list_files( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Query(pagination): Query, Query(lq): Query, ) -> JsonResult> { - require_devops_role(&db, &email).await?; + require_devops_role(&db, &authed).await?; let (per_page, offset) = windmill_common::utils::paginate(pagination); let mut sqlb = sql_builder::SqlBuilder::select_from("log_file") @@ -89,13 +89,13 @@ async fn list_files( } async fn get_log_file( - ApiAuthed { email, .. }: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, Path(path): Path, ) -> windmill_common::error::Result { use windmill_common::tracing_init::TMP_WINDMILL_LOGS_SERVICE; - require_devops_role(&db, &email).await?; + require_devops_role(&db, &authed).await?; let path = path.to_path(); if path.contains("..") { return Err(Error::BadRequest("Invalid path".to_string())); diff --git a/backend/windmill-api/src/users.rs b/backend/windmill-api/src/users.rs index 3ac80d6b00..37946f68a4 100644 --- a/backend/windmill-api/src/users.rs +++ b/backend/windmill-api/src/users.rs @@ -21,7 +21,9 @@ use axum::{ }; use hyper::StatusCode; use serde::Deserialize; -use windmill_api_auth::{forbid_superadmin_job_token, require_super_admin}; +use windmill_api_auth::{ + forbid_elevated_job_token, forbid_superadmin_job_token, require_super_admin, +}; use windmill_audit::audit_oss::audit_log; use windmill_audit::ActionKind; use windmill_common::audit::AuditAuthor; @@ -115,7 +117,7 @@ async fn list_ext_jwt_tokens( Extension(db): Extension, Query(query): Query, ) -> Result>> { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; let (per_page, offset) = windmill_common::utils::paginate(windmill_common::utils::Pagination { page: query.page, @@ -146,7 +148,10 @@ async fn set_password( OptJobAuthed { job_id, .. }: OptJobAuthed, Json(ep): Json, ) -> Result { - forbid_superadmin_job_token(&db, &authed.email, job_id).await?; + // Choosing the password of the elevated account this job runs as is a credential + // mint by another name: logging in with it yields a session with no `job_id` + // (GHSA-hfh4-cx4h-3fcr). + forbid_elevated_job_token(&db, &authed.email, job_id).await?; let email = authed.email.clone(); crate::users_oss::set_password(db, argon2, authed, &email, ep).await } @@ -159,7 +164,7 @@ async fn set_password_of_user( OptJobAuthed { job_id, .. }: OptJobAuthed, Json(ep): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; crate::users_oss::set_password(db, argon2, authed, &email, ep).await } @@ -176,7 +181,7 @@ async fn rename_user( Extension(db): Extension, Json(ru): Json, ) -> Result { - require_super_admin(&db, &authed.email).await?; + require_super_admin(&db, &authed).await?; forbid_superadmin_job_token(&db, &authed.email, job_id).await?; let mut tx = db.begin().await?; diff --git a/backend/windmill-api/src/workspaces.rs b/backend/windmill-api/src/workspaces.rs index 1e26a20e05..23ce68fb80 100644 --- a/backend/windmill-api/src/workspaces.rs +++ b/backend/windmill-api/src/workspaces.rs @@ -214,7 +214,7 @@ pub async fn get_critical_alerts( authed: ApiAuthed, Query(params): Query, ) -> JsonResult { - require_admin_or_devops(authed.is_admin, &authed.username, &authed.email, &db).await?; + require_admin_or_devops(authed.is_admin, &authed.username, &authed.email, authed.job_id.is_some(), &db).await?; crate::utils::get_critical_alerts(db, params, Some(w_id)).await } @@ -230,7 +230,7 @@ pub async fn acknowledge_critical_alert( Path((w_id, id)): Path<(String, i32)>, authed: ApiAuthed, ) -> Result { - require_admin_or_devops(authed.is_admin, &authed.username, &authed.email, &db).await?; + require_admin_or_devops(authed.is_admin, &authed.username, &authed.email, authed.job_id.is_some(), &db).await?; crate::utils::acknowledge_critical_alert(db, Some(w_id), id).await } diff --git a/backend/windmill-common/src/auth.rs b/backend/windmill-common/src/auth.rs index 8dfc3fc324..09887455e6 100644 --- a/backend/windmill-common/src/auth.rs +++ b/backend/windmill-common/src/auth.rs @@ -325,6 +325,49 @@ pub async fn is_super_admin_email<'c>(db: impl sqlx::PgExecutor<'c>, email: &str Ok(is_admin) } +/// The three reserved internal identities that grant instance-superadmin at +/// execution: `superadmin_secret@` / `superadmin_notification@` (matched on the +/// email) and `superadmin_sync@` (matched on `permissioned_as`). They belong to +/// no real user, so a stored `on_behalf_of` (app policy, flow/script, +/// schedule, trigger) must never carry one as either field — it would be a +/// forged superadmin run identity. Mirror of the `is_super_admin` derivation in +/// [`fetch_authed_from_permissioned_as_inner`]. +pub fn is_reserved_on_behalf_of_identity( + permissioned_as: Option<&str>, + on_behalf_of_email: Option<&str>, +) -> bool { + const RESERVED: [&str; 3] = [ + SUPERADMIN_SECRET_EMAIL, + SUPERADMIN_NOTIFICATION_EMAIL, + SUPERADMIN_SYNC_EMAIL, + ]; + [permissioned_as, on_behalf_of_email] + .into_iter() + .flatten() + .any(|v| RESERVED.contains(&v)) +} + +/// Guard a caller-supplied `on_behalf_of` before it is persisted on a deployable +/// object (app policy, flow/script, schedule, trigger): reject the reserved +/// internal sentinels, which no legitimate deploy ever carries. The actual +/// escalation is closed at execution by the job-token cap in +/// [`require_super_admin`] — even a superadmin `on_behalf_of` yields a token +/// capped at workspace admin — so this is a cheap, non-breaking early guard, not +/// the primary defense. It deliberately does *not* restrict deploying on behalf +/// of a real user (including a real superadmin, e.g. git-sync of +/// superadmin-authored content), which is the intended `wm_deployers` capability. +pub fn validate_on_behalf_of( + permissioned_as: Option<&str>, + on_behalf_of_email: Option<&str>, +) -> Result<()> { + if is_reserved_on_behalf_of_identity(permissioned_as, on_behalf_of_email) { + return Err(Error::BadRequest( + "on_behalf_of cannot be a reserved internal identity".to_string(), + )); + } + Ok(()) +} + pub async fn is_devops_email(db: &DB, email: &str) -> Result { if is_super_admin_email(db, email).await? { return Ok(true); @@ -727,8 +770,11 @@ pub mod aws { #[cfg(test)] mod tests { - use super::is_user_token; + use super::{is_reserved_on_behalf_of_identity, is_user_token}; use super::{job_token_remaining_lifetime_secs, JWTAuthClaims, JOB_TOKEN_REFRESH_MARGIN_SECS}; + use crate::users::{ + SUPERADMIN_NOTIFICATION_EMAIL, SUPERADMIN_SECRET_EMAIL, SUPERADMIN_SYNC_EMAIL, + }; fn job_jwt(exp_offset_secs: i64) -> String { let claims = JWTAuthClaims { @@ -769,6 +815,36 @@ mod tests { assert!(job_token_remaining_lifetime_secs("").is_none()); } + #[test] + fn reserved_on_behalf_of_identity_matches_every_sentinel_in_either_field() { + // Matched on the email (secret / notification) or on permissioned_as (sync). + assert!(is_reserved_on_behalf_of_identity( + None, + Some(SUPERADMIN_SECRET_EMAIL) + )); + assert!(is_reserved_on_behalf_of_identity( + None, + Some(SUPERADMIN_NOTIFICATION_EMAIL) + )); + assert!(is_reserved_on_behalf_of_identity( + Some(SUPERADMIN_SYNC_EMAIL), + None + )); + // A sentinel smuggled as a raw-email permissioned_as (schedules/triggers + // derive the email from it) is caught too. + assert!(is_reserved_on_behalf_of_identity( + Some(SUPERADMIN_SECRET_EMAIL), + None + )); + // Ordinary identities pass. + assert!(!is_reserved_on_behalf_of_identity(None, None)); + assert!(!is_reserved_on_behalf_of_identity( + Some("u/alice"), + Some("alice@example.com") + )); + assert!(!is_reserved_on_behalf_of_identity(Some("g/team"), None)); + } + #[test] fn user_tokens_are_editable() { assert!(is_user_token(None)); // no label diff --git a/backend/windmill-common/src/jobs.rs b/backend/windmill-common/src/jobs.rs index 3546381885..5541615838 100644 --- a/backend/windmill-common/src/jobs.rs +++ b/backend/windmill-common/src/jobs.rs @@ -10,7 +10,6 @@ use tokio::io::AsyncReadExt; pub use windmill_types::jobs::*; use crate::{ - auth::is_super_admin_email, client::AuthedClient, db::{AuthedRef, UserDbWithAuthed, DB}, error::{self, to_anyhow, Error}, @@ -335,11 +334,14 @@ lazy_static::lazy_static! { ).unwrap_or(false); } +// `is_super_admin` is passed in (not derived from an email here) so callers can +// make it job-token-aware: a job's WM_TOKEN must never count as superadmin +// (GHSA-hfh4-cx4h-3fcr). See `is_super_admin_authed` at the request wrapper. pub async fn check_tag_available_for_workspace_internal( db: &DB, w_id: &str, tag: &str, - email: &str, + is_super_admin: bool, scope_tags: Option>, ) -> error::Result<()> { let mut is_tag_in_scope_tags = None; @@ -372,7 +374,7 @@ pub async fn check_tag_available_for_workspace_internal( _ => {} } - if !is_super_admin_email(db, email).await? { + if !is_super_admin { if scope_tags.is_some() && is_tag_in_scope_tags.is_some() { return Err(Error::BadRequest(format!( "Tag {tag} is not available in your scope" diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index b72d0ae1d9..7129ae0e48 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -29,10 +29,9 @@ use sqlx::{Acquire, Postgres}; pub mod agent_workers; pub mod apps; pub mod assets; -pub mod azure_workload_identity; -pub mod dbt_manifest; pub mod audit; pub mod auth; +pub mod azure_workload_identity; #[cfg(feature = "benchmark")] pub mod bench; pub mod cache; @@ -44,6 +43,7 @@ mod db_entra_ee; #[cfg(all(feature = "enterprise", feature = "private"))] mod db_iam_ee; pub mod db_params; +pub mod dbt_manifest; pub mod deploy_origin; #[cfg(feature = "private")] pub mod deployment_requests_ee; @@ -236,6 +236,10 @@ pub async fn resolve_on_behalf_of( if !(preserve && can_preserve_on_behalf_of(authed)) { return reject_unenqueueable(users::username_to_permissioned_as(authed.username())); } + // Reserved superadmin sentinels are rejected by name, before resolution: the lookups + // below only reject them while no account holds their address, and the runtime grants + // superadmin on these emails by string comparison alone. + auth::validate_on_behalf_of(on_behalf_of, on_behalf_of_email)?; let permissioned_as = match on_behalf_of { Some(permissioned_as) => { // The principal wins, but a caller that also names a contradictory address has a @@ -1760,7 +1764,10 @@ pub async fn on_behalf_of_from_permissioned_as( // processes, so a cached read would keep minting jobs under an address the account no longer // holds for up to a minute after it moves. let email = users::get_email_from_permissioned_as_uncached(permissioned_as, w_id, db).await?; - Ok(Some(jobs::OnBehalfOf { email, permissioned_as: permissioned_as.to_string() })) + Ok(Some(jobs::OnBehalfOf { + email, + permissioned_as: permissioned_as.to_string(), + })) } impl ScriptHashInfo { diff --git a/backend/windmill-common/src/utils.rs b/backend/windmill-common/src/utils.rs index db0de177db..f2ae31bcc4 100644 --- a/backend/windmill-common/src/utils.rs +++ b/backend/windmill-common/src/utils.rs @@ -330,10 +330,16 @@ pub async fn require_admin_or_devops( is_admin: bool, username: &str, email: &str, + // True when the caller is a job token (`$WM_TOKEN`). `devops` is instance-level + // and `is_devops_email` is true for superadmins, so a job token whose on_behalf_of + // a `wm_deployers` member pointed at a superadmin would otherwise clear the devops + // branch on a workspace it isn't admin of (GHSA-hfh4-cx4h-3fcr). Workspace admin + // (`is_admin`) stays allowed — that is the cap ceiling. + is_job_token: bool, db: &DB, ) -> Result<()> { if !is_admin { - if !is_devops_email(db, email).await? { + if is_job_token || !is_devops_email(db, email).await? { return Err(Error::RequireAdmin(username.to_string())); } } diff --git a/backend/windmill-queue/src/schedule.rs b/backend/windmill-queue/src/schedule.rs index 10f4a2b408..fa495c9cd2 100644 --- a/backend/windmill-queue/src/schedule.rs +++ b/backend/windmill-queue/src/schedule.rs @@ -513,11 +513,12 @@ pub async fn push_scheduled_job<'c>( }; if let Some(tag) = tag.as_deref().filter(|t| !t.is_empty()) { + let is_super_admin = windmill_common::auth::is_super_admin_email(db, &email).await?; check_tag_available_for_workspace_internal( db, &schedule.workspace_id, &tag, - &email, + is_super_admin, None, // no token for schedules so no scopes so no scope_tags ) .warn_after_seconds_with_sql(1, "check_tag_available_for_workspace_internal".to_string()) diff --git a/backend/windmill-store/src/resources.rs b/backend/windmill-store/src/resources.rs index 421d147bdf..0f209b0381 100644 --- a/backend/windmill-store/src/resources.rs +++ b/backend/windmill-store/src/resources.rs @@ -13,7 +13,7 @@ use std::sync::LazyLock; use windmill_api_auth::{ build_scope_path_predicate, check_scopes, maybe_refresh_folders, require_owner_of_path, - require_super_admin, ApiAuthed, Tokened, + require_super_admin_email, ApiAuthed, Tokened, }; use windmill_common::db::DB; use windmill_common::per_minute_counter::PerMinuteCounter; @@ -724,7 +724,16 @@ pub async fn get_resource_value_interpolated_internal<'a>( ) -> Result> { // This is a special syntax to help debugging custom instance databases if let Some(dbname) = path.strip_prefix("CUSTOM_INSTANCE_DB/") { - require_super_admin(db_with_opt_authed.db(), &db_with_opt_authed.email()).await?; + // A job's WM_TOKEN must never reach this superadmin-only path even if it + // runs on behalf of a superadmin (GHSA-hfh4-cx4h-3fcr). Read the job + // provenance from the *authenticated* identity, never the caller-supplied + // `job_id` param (which comes from an untrusted query string). + if db_with_opt_authed.authed().and_then(|a| a.job_id).is_some() { + return Err(Error::NotAuthorized( + "CUSTOM_INSTANCE_DB cannot be resolved from a job token ($WM_TOKEN)".to_string(), + )); + } + require_super_admin_email(db_with_opt_authed.db(), &db_with_opt_authed.email()).await?; let mut pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; pg_creds.dbname = dbname.to_string(); let pg_creds = serde_json::to_value(&pg_creds) diff --git a/backend/windmill-trigger/src/handler.rs b/backend/windmill-trigger/src/handler.rs index 00d8cbf828..c68fa7fce6 100644 --- a/backend/windmill-trigger/src/handler.rs +++ b/backend/windmill-trigger/src/handler.rs @@ -572,6 +572,14 @@ async fn create_trigger( } } + // Reject a forged superadmin run identity in a preserved permissioned_as + // (the sentinel guard; a trigger's email is derived from it at execution). + let resolved_permissioned_as = new_trigger.base.resolve_permissioned_as(&authed); + windmill_common::auth::validate_on_behalf_of( + Some(&resolved_permissioned_as), + None, + )?; + let on_behalf_of_info = windmill_common::check_on_behalf_of_preservation( new_trigger.base.permissioned_as.as_deref(), new_trigger.base.preserve_permissioned_as.unwrap_or(false), @@ -825,6 +833,15 @@ async fn update_trigger( let new_path = edit_trigger.base.path.to_string(); let labels = edit_trigger.base.labels.clone(); + + // Reject a forged superadmin run identity in a preserved permissioned_as + // (the sentinel guard; a trigger's email is derived from it at execution). + let resolved_permissioned_as = edit_trigger.base.resolve_permissioned_as(&authed); + windmill_common::auth::validate_on_behalf_of( + Some(&resolved_permissioned_as), + None, + )?; + let on_behalf_of_info = windmill_common::check_on_behalf_of_preservation( edit_trigger.base.permissioned_as.as_deref(), edit_trigger.base.preserve_permissioned_as.unwrap_or(false), diff --git a/backend/windmill-worker/src/worker_flow.rs b/backend/windmill-worker/src/worker_flow.rs index 42246c6358..bd39afa8c7 100644 --- a/backend/windmill-worker/src/worker_flow.rs +++ b/backend/windmill-worker/src/worker_flow.rs @@ -4421,11 +4421,13 @@ async fn push_next_flow_job( .as_deref() .filter(|t| !t.is_empty() && *t != flow_job.tag.as_str()) { + let is_super_admin = + windmill_common::auth::is_super_admin_email(db, email).await?; check_tag_available_for_workspace_internal( db, &flow_job.workspace_id, tag_str, - email, + is_super_admin, None, // no token for flow substeps so no scopes so no scope_tags ) .warn_after_seconds_with_sql(