starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-09-08 08:04:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): a WM_TOKEN job token can never be a global superadmin (GHSA-hfh4-cx4h-3fcr) (#10124)

Browse Source 
* fix(security): a WM_TOKEN job token can never be a global superadmin (GHSA-hfh4-cx4h-3fcr)

Privilege escalation: an app/flow/schedule/trigger execution policy's `on_behalf_of`
(which a `wm_deployers` member can set) could point at a superadmin email. The
resulting job `WM_TOKEN` then passed the email-based superadmin checks, granting
instance superadmin. `forbid_superadmin_job_token` only guarded ~15 of ~75 routes.

Fix at the token layer: a WM_TOKEN must never satisfy a superadmin gate,
regardless of whose email it runs as (sentinel OR a real superadmin).

- `ApiAuthed` gains a `job_id` field, stamped once in `AuthCache::get_opt_job_authed`
  from the resolved token's job_id (correct even on cache hits).
- `require_super_admin(db, email)` -> `require_super_admin(db, &ApiAuthed)`, rejects
  `authed.job_id.is_some()`. `require_super_admin_email` kept for the few internal
  callers without an ApiAuthed.
- `is_super_admin_authed(db, &ApiAuthed)` for the boolean `is_super_admin_email`
  authorization branches on request handlers (workspace deletion, fork drops,
  dev-workspace attach/archive, object-storage SSRF exemption, custom dbname, EE GHES
  + connected repositories, ...). Migrate ~75 sites (OSS + EE).
- CUSTOM_INSTANCE_DB reads the *authenticated* job_id, not the caller-supplied
  `?job_id` query param. Worker-tag check takes a precomputed job-aware `is_super_admin`
  on the request path.

Execution-time on-behalf checks (scheduled/flow worker-tag, Cloud enqueue quota,
is_devops_email) are hardened in a follow-up — see
docs/followup-onbehalf-execution-privilege-hardening.md.

Regression tests: a superadmin-email WM_TOKEN is rejected on `require_super_admin`
routes, on `DELETE /workspaces/delete/{w}` (403, workspace preserved), and on the
CUSTOM_INSTANCE_DB lookup with no `?job_id` (401); real superadmin tokens still succeed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: cap devops role at workspace admin and reject reserved on_behalf_of identities

Extends the job-token cap with three pieces:

- `require_devops_role` takes `&ApiAuthed` and rejects job tokens.
  `is_devops_email` is true for superadmin emails, so every worker-management,
  instance-config and service-log route was reachable by the same superadmin
  `WM_TOKEN` that `require_super_admin` already rejects.
- A `job_id` claim that does not parse as a uuid rejects the token rather than
  resolving to `None`, which would clear the job provenance and uncap it. Applies
  to the internal JWT and the external `jwt_ext_` path.
- Defense in depth at store time: `validate_on_behalf_of` refuses the reserved
  internal sentinels as an `on_behalf_of` on apps/flows/scripts/schedules/triggers,
  and app execution refuses a policy carrying one — covering already-persisted and
  forked-app rows that predate the cap. Deploying on behalf of a real user,
  including a real superadmin, stays allowed; the cap handles that at execution.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(mcp): preserve job-token provenance when minting the proxy JWT

The MCP endpoint-tool proxy re-mints a JWT from the caller's ApiAuthed to
forward the proxied request, but passed job_id: None. A job's WM_TOKEN is
capped at workspace admin (GHSA-hfh4-cx4h-3fcr); dropping the job_id here
re-minted an uncapped token that satisfies require_super_admin /
require_devops_role on the proxied route (e.g. listWorkers exposing worker
IPs, job/workspace IDs, and sensitive tags).

Carry api_authed.job_id into create_jwt_token. Adds an in-module regression
that decodes the forwarded JWT and asserts the job_id is preserved for a job
caller and absent for a non-job caller.

Reported by Codex CI review (P1) on #10124.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix: cap the admin-or-devops gate at workspace admin for job tokens

require_admin_or_devops (the EE critical-alerts endpoints) grants when the
caller is a workspace admin OR an instance devops. is_devops_email is true
for superadmins, so a WM_TOKEN running on-behalf of a superadmin who is not a
member of the target workspace could clear the devops branch and read/ack that
workspace's critical alerts (GHSA-hfh4-cx4h-3fcr). This gate takes a bare
email, not an ApiAuthed, so the token-layer cap could not see it.

Thread the caller's job-token provenance and reject the devops branch for job
tokens, matching require_devops_role. The workspace-admin branch stays allowed
— that is the cap ceiling. Adds an enterprise-gated regression proving the
bypass is closed and a real superadmin token still clears the gate.

Found while auditing the PR for bare-email gates the choke-point cap misses.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix: cap instance-global is_admin gates at workspace admin for job tokens

Three instance-global routes gate on the caller's own `is_admin` claim, which
`ApiAuthed.is_admin` carries into a WM_TOKEN (it is a workspace-admin claim,
true for superadmins too). A job token is capped at workspace admin
(GHSA-hfh4-cx4h-3fcr), so its is_admin claim must not authorize instance
actions on a route with no workspace binding:

- `unarchive_workspace` — unarchive an arbitrary workspace by id
- `prune_concurrency_group` — delete a global concurrency group
- `list_worker_groups` — return unobfuscated `env_vars_static` (may hold secrets)

Add job-token-aware `is_instance_admin` / `require_instance_admin` helpers (the
same shape as `require_super_admin` / `require_devops_role`) and use them at
these three sites. Workspace-scoped `require_admin(authed.is_admin, ...)` gates
are intentionally left unchanged — a workspace-admin job token is within the
cap there. Regression added covering all three; verified it lets a WM_TOKEN
unarchive/leak without the fix and is blocked with it.

Reported by Codex CI review (P1) on #10124.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(mcp): drop orphaned path_field_renames from EndpointTool test helper

The merge with main adopted main's mcp path-substitution refactor (#10162),
which removed the `path_field_renames` field from `EndpointTool` and its
consumer (`substitute_path_params` no longer takes per-field path renames).
main's `runner.rs` `ep` test helper still constructed the struct with
`path_field_renames: None`, so the workspace test build (cargo test --all,
which compiles windmill-mcp's own #[cfg(test)] module under the `server`
feature) failed with E0560. A plain `cargo check` does not compile that test
module, so it only surfaced in CI's cargo_test.

Remove the orphaned field to match the struct.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: describe the sentinel-rejection policy the forged-identity test asserts

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

* fix: complete ApiAuthed initializers in feature-gated tests after merge

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

* fix: stop job tokens minting credentials that shed their provenance

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

* fix: cap the MCP OAuth approval mint at the same elevated-job-token gate

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

* fix: cap the self-service password reset at the elevated-job-token gate

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

* fix: cap app embed/SDK mints and scope widening at the elevated-job-token gate

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

* fix: keep job tokens from destroying the account they run on behalf of

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

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

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

* docs: keep the follow-up inventory in the PR instead of the repo

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

* fix: make the session workspace status gate job-token aware

session_workspace_status derived its superadmin branch from a bare email
check, so a job token carrying a superadmin identity resolved the existence
of workspaces it has no relationship with rather than seeing them as
deleted. Switch to is_super_admin_authed, matching every other instance
gate reached from a request ApiAuthed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* revert: leave the global concurrency-group listing on the plain admin gate

The listing exposes concurrency keys across workspaces, which is metadata
rather than a capability, and it 401s rather than degrading. Keep the guard
on the prune route next to it, which is the destructive one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: keep the instance-admin gate on the global concurrency listing

The listing spans every workspace's concurrency keys, and the gate rejects
only job tokens: the !is_admin branch is the pre-existing check, so
workspaced tokens and interactive admins are unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore: update ee-repo-ref to d30af67d38954f9012f7bad08da23e347344b4c6

This commit updates the EE repository reference after PR #664 was merged in windmill-ee-private.

Previous ee-repo-ref: 7870573dbc3360f99bada143f094c67dce0d9e9c

New ee-repo-ref: d30af67d38954f9012f7bad08da23e347344b4c6

Automated by sync-ee-ref workflow.

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: hugocasa <hugo@casademont.ch>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
Ruben Fiszel
2026-08-19 22:33:46 +02:00
committed by GitHub
co-authored by Claude Opus 5 hugocasa windmill-internal-app[bot]
parent ed2ff6c5e7
commit c2deea13b7
41 changed files with 1535 additions and 208 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/windmill-api-workspaces/src/workspaces.rs
+30 -24
View File
@@ -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<EditDucklakeConfig>,
) -> Result<String> {
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://<name>'`,
// generated maintenance SQL and the reserved maintenance schedule path
@@ -3130,11 +3130,11 @@ async fn edit_datatable_config(
authed: ApiAuthed,
Extension(db): Extension<DB>,
Path(w_id): Path<String>,
ApiAuthed { is_admin, username, email, .. }: ApiAuthed,
ApiAuthed { is_admin, username, .. }: ApiAuthed,
Json(mut new_config): Json<EditDataTableConfig>,
) -> Result<String> {
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<String>,
Json(request): Json<SetEncryptionKeyRequest>,
) -> 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<DB>,
Path(w_id): Path<String>,
) -> JsonResult<Workspace> {
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<DB>,
Extension(user_db): Extension<UserDB>,
Query(pagination): Query<Pagination>,
ApiAuthed { email, .. }: ApiAuthed,
) -> JsonResult<Vec<Workspace>> {
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<DB>,
ApiAuthed { email, .. }: ApiAuthed,
authed: ApiAuthed,
Json(req): Json<SessionWorkspaceStatusRequest>,
) -> JsonResult<HashMap<String, String>> {
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<CreateWorkspace>,
) -> Result<String> {
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<String>,
authed: ApiAuthed,
) -> Result<String> {
windmill_api_auth::forbid_job_token_account_destruction(&authed)?;
let mut tx = db.begin().await?;
sqlx::query!(
"DELETE FROM usr WHERE workspace_id = $1 AND email = $2",
@@ -8201,7 +8200,9 @@ async fn unarchive_workspace(
Path(w_id): Path<String>,
authed: ApiAuthed,
) -> Result<String> {
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,
})
}