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

fix: confine job tokens to workspace-scoped API routes (#10631)

Browse Source 
* fix: confine job tokens to workspace-scoped API routes

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

* fix: keep the object-storage connection test reachable from a job token

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

* fix: keep the workspace-exists check the CLI makes reachable from a job

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

* test: reconcile the job-token caps after #10124

The workspace-confinement middleware answers a workspace-less route before the
privilege gate behind it runs, so the cases #10124 added on those routes now see
403 rather than 401. Rejection is what they assert, but two of them needed more
than a status change:

- `list_worker_groups` asserted only that the response body omits the static env
  value, which an error body satisfies for the wrong reason. It now asserts the
  status, keeping the secret check as a second assertion.
- `require_super_admin` lost its only unshadowed route. `GET
  /api/w/{workspace}/users/list_addable` is gated solely by that call and names a
  workspace, so it reaches the gate and pins it at 401.

The module doc states the two-layer rule once; the file covers both caps, so it
is no longer named for either one alone.

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

* docs: correct the workspaces/exists rationale and the parquet gate note

`workspace` carries no row-level security, so `exists_workspace` running through
`user_db` does not filter by membership as the comment claimed. State what the
route actually discloses — whether a workspace id is taken.

The object-storage case explained why a 404 would satisfy the assertion for the
wrong reason, which described the earlier `assert_ne!(403)`; against the 422 it
now asserts, a 404 fails. Say instead why the case is gated on the feature.

* fix: let a job token keep the workspace-less routes that carry no workspace

Confinement refused every route outside the allowlist, including ones that
answer purely from the caller's own account or from the request body. Those
cross no workspace boundary, so refusing them buys nothing:

- `users/email` returns a value already inside the token, and
  `workspaces/allowed_domain_auto_invite` tests the caller's own address against
  a static list. Neither opens a transaction.
- `users/usage` reads the caller's own row; `users/tutorial_progress` reads and
  upserts a UI bitfield keyed on the same email.
- `schedules/preview` takes no `ApiAuthed` at all — it computes the occurrences
  of the cron expression in the body and returns nothing the caller did not send.

The rule, not the list, is what the doc comment states: answers from the caller's
own account, the request body, or content identical for every workspace; never
naming another workspace, never instance configuration. The candidates it
excludes are written down with their reasons, since `users/list_invites` reads as
caller-scoped until you notice the response carries a workspace id per invite.

Regression covers both directions — the new entries answer, and the rejected
caller-scoped reads stay refused.

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

* test: reach the privilege gates confinement hides

`require_devops_role` and `require_instance_admin` gate only workspace-less
routes, so confinement answers every request that would reach them: no HTTP case
can tell whether they still cap job tokens, and both could lose that check with
this suite green. `require_super_admin` has a workspace-scoped route to reach it;
these two have none, so call them directly instead.

The identity used is the fixture's real superadmin, so the passing half proves
the rejection keys off `job_id` rather than off the user.

Also separate two claims the write-allowlist doc had merged into one sentence:
no entry writes outside the caller's own account, but what each may read differs,
and the workspace-existence check answers for any id.

* docs: say that the object-storage probe writes

The write-allowlist lead claimed no entry writes state outside the caller's own
account. `test_s3_bucket` puts an object into the store the body names and
deletes it again, so it does write; a failure between the two leaves the object
behind. The invariant that holds is about Windmill state.

Say so in the lead, and describe the put/delete in the entry itself rather than
leaving "acts only on the store the request body describes" to imply a read.

* test: cover the last two job-token gates confinement hides

Seven guards key on `ApiAuthed::job_id`. Three keep a workspace-scoped route and
are exercised over HTTP; the other four are reachable only through workspace-less
routes, which confinement now answers first, so nothing observed whether they
still cap job tokens.

`require_devops_role` and `require_instance_admin` were already called directly.
Add the two that were not: `forbid_superadmin_job_token`, and
`forbid_elevated_job_token`, whose call sites are `create_token`,
`update_token_scopes` and `set_password` — all workspace-less.

Both key on two conditions rather than one, so all three combinations are pinned:
neither fires without job provenance, and neither fires for an unelevated
identity. The second matters — collapsing either into a blanket job-token refusal
would stop ordinary users creating tokens, and no other case would catch it.

The doc comment records which of the seven each route covers.

* docs: correct which job-token gates have no observable route

The previous commit put `forbid_elevated_job_token` among the guards reachable
only through workspace-less routes, and its message named three call sites. It
has six, and two are workspaced: `mint_app_embed_token` and
`mint_raw_app_sdk_token`. Its superadmin branch is therefore already exercised
over HTTP — the 401 the embed-token case asserts is this gate.

So three of the seven lack an observable route, not four. Its direct assertions
stay: the embed-token case only ever reaches it with an elevated identity, and
the unelevated-negative case is what would catch the gate being collapsed into a
blanket job-token refusal.

* test: pin is_instance_admin, and stop enumerating gates in prose

`is_instance_admin` is `authed.is_admin && authed.job_id.is_none()`, so a census
built by searching for `job_id.is_some()` could not see it. Both its call sites
are workspace-less, and it returns a bool that selects obfuscation rather than
refusing — a job token reading `true` leaks `env_vars_static` instead of being
turned away. Pin both directions.

The doc comment tried to account for every job-token guard and which route
exercised it. It was wrong three times running: the count, the call sites of
`forbid_elevated_job_token`, and the claim that the CUSTOM_INSTANCE_DB case
covers `is_super_admin_authed` when that path tests `job_id` inline. A table that
has to be rederived from six crates to stay true does not belong in a comment, so
it now states only why these calls are direct.

* docs: name the right is_instance_admin caller

The comment credited "the concurrency-group listing" with obfuscating rows. The
second caller is `prune_concurrency_group`, which returns PermissionDenied; the
obfuscating one is `list_worker_groups`. Keep the claim to that single caller,
which is what makes this guard fail by leaking `env_vars_static` rather than by
admitting a request.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
hugocasa
2026-08-21 11:55:42 +02:00
committed by GitHub
co-authored by Claude Opus 5
parent 3c8e4b43fd
commit 9022dc9d44
3 changed files with 455 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/tests/wm_token_superadmin_guard.rs → backend/tests/wm_token_confinement.rs
+358 -48
View File
@@ -1,17 +1,32 @@
//! A WM_TOKEN (job JWT) running as a superadmin must not be able to perform
//! global user/token management — promotion, password reset, user creation,
//! token creation/impersonation, offboarding, or exporting the user table.
//! A non-admin `wm_deployers` member can mint
//! such a token implicitly via an app/flow `on_behalf_of`, so trusting it would
//! let them establish *persistent* superadmin. A real superadmin who needs this
//! from a script must use a dedicated superadmin API token (which only a real
//! superadmin can create), not `$WM_TOKEN`.
//! A WM_TOKEN (job JWT) is minted for one job in one workspace and carries that
//! job's full user privileges. Two independent caps hold it there, and this file
//! covers both:
//!
//! - Confinement to the job's workspace, enforced in the auth middleware. A route
//! that names no workspace is instance-wide, so reaching one would trade an
//! ephemeral, workspace-bound credential for a permanent one (`tokens/create`
//! mints a workspace-less API token that never expires), for instance
//! configuration, or for global user management. Refusals are `403`.
//! - A ceiling of workspace admin whatever identity the token borrows, enforced at
//! the privilege gates themselves (`require_super_admin`, `require_devops_role`,
//! `require_instance_admin`, GHSA-hfh4-cx4h-3fcr). Refusals are `401`.
//!
//! The middleware runs first, so on a workspace-less route it answers before the
//! gate behind it ever runs: those cases pin the outer cap, and the gates are
//! pinned by the workspace-scoped cases, which the middleware lets through.
//!
//! A non-admin `wm_deployers` member can mint such a token implicitly via an
//! app/flow `on_behalf_of`, so the identity it carries need not be their own. A
//! real superadmin who needs a global endpoint from a script must use a
//! dedicated API token (which only a real superadmin can create), not
//! `$WM_TOKEN`.
//!
//! The fixture provides `test@windmill.dev` (instance superadmin, token
//! `SECRET_TOKEN`) and `test2@windmill.dev` (non-superadmin, `SECRET_TOKEN_2`).
use serde_json::json;
use sqlx::{Pool, Postgres};
use windmill_api_auth::ApiAuthed;
use windmill_common::auth::create_jwt_token;
use windmill_common::db::Authed;
use windmill_test_utils::*;
@@ -51,7 +66,7 @@ async fn wm_token(email: &str, is_admin: bool) -> String {
}
#[sqlx::test(fixtures("preserve_on_behalf_of"))]
async fn test_wm_token_cannot_manage_superadmin_users(db: Pool<Postgres>) -> anyhow::Result<()> {
async fn test_wm_token_is_confined_to_its_workspace(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
// The server decodes WM_TOKENs with the same in-process JWT secret, so
// setting it once lets us mint a valid one below.
@@ -59,11 +74,14 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool<Postgres>) -> any
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
let base = format!("http://localhost:{port}/api/users");
let api = format!("http://localhost:{port}/api");
let base = format!("{api}/users");
// A superadmin-capable WM_TOKEN — the exact thing a deployer obtains via an
// app on_behalf_of pointed at a superadmin.
let sa_wm = wm_token("test@windmill.dev", true).await;
// ...and one for a plain user: neither may leave its workspace.
let user_wm = wm_token("test2@windmill.dev", false).await;
// 1. Cannot mint a (superadmin) token.
let resp = authed(client().post(format!("{base}/tokens/create")), &sa_wm)
@@ -72,7 +90,7 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool<Postgres>) -> any
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not create tokens: {}",
resp.text().await?
);
@@ -84,7 +102,7 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool<Postgres>) -> any
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not impersonate: {}",
resp.text().await?
);
@@ -99,7 +117,7 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool<Postgres>) -> any
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not promote users: {}",
resp.text().await?
);
@@ -111,7 +129,7 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool<Postgres>) -> any
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not reset passwords: {}",
resp.text().await?
);
@@ -125,7 +143,7 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool<Postgres>) -> any
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not delete users: {}",
resp.text().await?
);
@@ -140,7 +158,7 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool<Postgres>) -> any
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not change login type: {}",
resp.text().await?
);
@@ -156,7 +174,7 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool<Postgres>) -> any
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not offboard users: {}",
resp.text().await?
);
@@ -167,13 +185,38 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool<Postgres>) -> any
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not export global users: {}",
resp.text().await?
);
// 5. Escape hatch / no false positive: a real superadmin API token
// (SECRET_TOKEN, no job_id) can still create tokens.
// 5. Not only user management: any workspace-less route is out of reach,
// including one open to every authenticated user.
let resp = authed(client().get(format!("{api}/workers/list")), &user_wm)
.send()
.await?;
assert_eq!(
resp.status(),
403,
"WM_TOKEN must not enumerate instance workers: {}",
resp.text().await?
);
// 6. A plain user's WM_TOKEN cannot mint itself a permanent, workspace-less
// token either — the confinement does not depend on being a superadmin.
let resp = authed(client().post(format!("{base}/tokens/create")), &user_wm)
.json(&json!({ "label": "from-script" }))
.send()
.await?;
assert_eq!(
resp.status(),
403,
"WM_TOKEN must not create tokens: {}",
resp.text().await?
);
// 7. Escape hatch / no false positive: a real API token (SECRET_TOKEN, no
// job_id) still reaches both.
let resp = authed(
client().post(format!("{base}/tokens/create")),
"SECRET_TOKEN",
@@ -187,28 +230,137 @@ async fn test_wm_token_cannot_manage_superadmin_users(db: Pool<Postgres>) -> any
"a real superadmin token must still create tokens: {}",
resp.text().await?
);
// 6. No collateral: a non-superadmin WM_TOKEN can still create its own
// token — the guard only fires for superadmin-capable job tokens.
let user_wm = wm_token("test2@windmill.dev", false).await;
let resp = authed(client().post(format!("{base}/tokens/create")), &user_wm)
.json(&json!({ "label": "from-script" }))
let resp = authed(client().get(format!("{api}/workers/list")), "SECRET_TOKEN")
.send()
.await?;
assert_eq!(
resp.status(),
201,
"non-superadmin WM_TOKEN must still create its own token: {}",
200,
"a real token must still list workers: {}",
resp.text().await?
);
// 8. No collateral on the routes a job legitimately needs: its own workspace,
// and the workspace-less endpoint the clients' `whoami()` calls.
let resp = authed(client().get(format!("{base}/whoami")), &user_wm)
.send()
.await?;
assert_eq!(
resp.status(),
200,
"WM_TOKEN must still resolve its own identity: {}",
resp.text().await?
);
let resp = authed(
client().get(format!("{api}/w/test-workspace/scripts/list")),
&user_wm,
)
.send()
.await?;
assert_eq!(
resp.status(),
200,
"WM_TOKEN must still work inside its own workspace: {}",
resp.text().await?
);
// 9. ...and the writes it keeps. `wmill workspace add` checks this before it will
// accept the credentials it was given, so a job that points the CLI at its own
// instance depends on it.
let resp = authed(client().post(format!("{api}/workspaces/exists")), &user_wm)
.json(&json!({ "id": "test-workspace" }))
.send()
.await?;
assert_eq!(
resp.status(),
200,
"WM_TOKEN must still reach the workspace-exists check `wmill workspace add` makes: {}",
resp.text().await?
);
// The resource editor's object-storage "Test connection" runs as a preview job that
// POSTs its config here. The body is deliberately not a valid `ObjectSettings`, so
// reaching the handler's own extractors is exactly a 422 — a 403 means confinement
// refused it. The route is only mounted under `parquet`, which would make this a 404,
// so the case is gated on the feature rather than left to fail where it can't run.
#[cfg(feature = "parquet")]
{
let resp = authed(
client().post(format!("{api}/settings/test_object_storage_config")),
&user_wm,
)
.json(&json!({}))
.send()
.await?;
assert_eq!(
resp.status(),
422,
"WM_TOKEN must still reach the object-storage connection test: {}",
resp.text().await?
);
}
// 10. The rest of the allowlist: routes that answer from the caller's own account or
// from the request body alone.
for route in [
"users/email",
"users/usage",
"users/tutorial_progress",
"workspaces/allowed_domain_auto_invite",
] {
let resp = authed(client().get(format!("{api}/{route}")), &user_wm)
.send()
.await?;
assert_eq!(
resp.status(),
200,
"WM_TOKEN must still read its own {route}: {}",
resp.text().await?
);
}
let resp = authed(client().post(format!("{api}/schedules/preview")), &user_wm)
.json(&json!({ "schedule": "0 0 12 * * *", "timezone": "UTC" }))
.send()
.await?;
assert_eq!(
resp.status(),
200,
"WM_TOKEN must still preview a cron expression: {}",
resp.text().await?
);
let resp = authed(client().post(format!("{base}/tutorial_progress")), &user_wm)
.json(&json!({ "progress": 1, "skipped_all": false }))
.send()
.await?;
assert_eq!(
resp.status(),
200,
"WM_TOKEN must still record its own tutorial progress: {}",
resp.text().await?
);
// 11. ...and the caller-scoped reads deliberately left out of it, each because it
// names another workspace or the identity's credentials.
for route in ["users/list_invites", "users/tokens/list", "workspaces/list"] {
let resp = authed(client().get(format!("{api}/{route}")), &user_wm)
.send()
.await?;
assert_eq!(
resp.status(),
403,
"{route} must stay confined: {}",
resp.text().await?
);
}
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).
/// route, not just the handful that call `forbid_superadmin_job_token`
/// (GHSA-hfh4-cx4h-3fcr). Both shapes of such a route are covered: a workspace-less
/// one, which workspace confinement answers first, and a workspace-scoped one, which
/// reaches the gate itself.
#[sqlx::test(fixtures("preserve_on_behalf_of"))]
async fn test_wm_token_rejected_by_require_super_admin(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
@@ -225,7 +377,7 @@ async fn test_wm_token_rejected_by_require_super_admin(db: Pool<Postgres>) -> an
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not reach a require_super_admin route: {}",
resp.text().await?
);
@@ -244,6 +396,35 @@ async fn test_wm_token_rejected_by_require_super_admin(db: Pool<Postgres>) -> an
resp.text().await?
);
// `GET /api/w/{workspace}/users/list_addable` is gated solely by
// `require_super_admin` too, but names a workspace, so the request runs the gate
// instead of stopping at confinement.
let resp = authed(
client().get(format!("{base}/w/test-workspace/users/list_addable")),
&sa_wm,
)
.send()
.await?;
assert_eq!(
resp.status(),
401,
"superadmin WM_TOKEN must not clear require_super_admin inside its own workspace: {}",
resp.text().await?
);
let resp = authed(
client().get(format!("{base}/w/test-workspace/users/list_addable")),
"SECRET_TOKEN",
)
.send()
.await?;
assert_eq!(
resp.status(),
200,
"a real superadmin token must still clear the gate: {}",
resp.text().await?
);
Ok(())
}
@@ -331,7 +512,7 @@ async fn test_wm_token_rejected_by_require_devops_role(db: Pool<Postgres>) -> an
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not reach a require_devops_role route: {}",
resp.text().await?
);
@@ -360,7 +541,7 @@ async fn test_wm_token_rejected_by_require_devops_role(db: Pool<Postgres>) -> an
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not list all users: {}",
resp.text().await?
);
@@ -448,7 +629,7 @@ async fn test_wm_token_rejected_by_instance_admin_gates(db: Pool<Postgres>) -> a
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not unarchive an arbitrary workspace: {}",
resp.text().await?
);
@@ -477,23 +658,30 @@ async fn test_wm_token_rejected_by_instance_admin_gates(db: Pool<Postgres>) -> a
.await?;
assert_eq!(
resp.status(),
401,
403,
"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(
// 4. Worker-group config: refused outright, so the static env value it would
// otherwise obfuscate never reaches a job token. Asserting the status rather
// than the absence of the secret keeps the case honest — an error body
// trivially satisfies "does not contain the secret".
let resp = authed(
client().get(format!("{base}/configs/list_worker_groups")),
&sa_wm,
)
.send()
.await?
.text()
.await?;
let status = resp.status();
let body = resp.text().await?;
assert_eq!(
status, 403,
"superadmin WM_TOKEN must not read the worker-group config: {body}"
);
assert!(
!body.contains("supersecretvalue"),
"superadmin WM_TOKEN must get the obfuscated worker-group view: {body}"
"the refusal must not carry the static env value: {body}"
);
// No false positive: a real superadmin API token (no job_id) still sees the
@@ -539,7 +727,7 @@ async fn test_wm_token_cannot_mint_a_provenance_free_credential(
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not refresh into a session token: {}",
resp.text().await?
);
@@ -567,7 +755,7 @@ async fn test_wm_token_cannot_mint_a_provenance_free_credential(
.await?;
assert_eq!(
resp.status(),
401,
403,
"devops WM_TOKEN must not mint a token: {}",
resp.text().await?
);
@@ -580,7 +768,7 @@ async fn test_wm_token_cannot_mint_a_provenance_free_credential(
.await?;
assert_eq!(
resp.status(),
401,
403,
"devops WM_TOKEN must not set its account password: {}",
resp.text().await?
);
@@ -642,7 +830,7 @@ async fn test_wm_token_cannot_mint_via_mcp_oauth_approval(
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not approve through the MCP gateway: {}",
resp.text().await?
);
@@ -709,7 +897,7 @@ async fn test_wm_token_cannot_mint_or_widen_an_app_embed_token(
.await?;
assert_eq!(
resp.status(),
401,
403,
"superadmin WM_TOKEN must not widen a token's scopes: {}",
resp.text().await?
);
@@ -739,7 +927,7 @@ async fn test_wm_token_cannot_destroy_its_on_behalf_account(
.await?;
assert_eq!(
resp.status(),
401,
403,
"WM_TOKEN must not delete the account it runs as: {}",
resp.text().await?
);
@@ -754,7 +942,7 @@ async fn test_wm_token_cannot_destroy_its_on_behalf_account(
.await?;
assert_eq!(
resp.status(),
401,
403,
"WM_TOKEN must not revoke that identity's tokens: {}",
resp.text().await?
);
@@ -855,3 +1043,125 @@ async fn test_wm_token_gets_no_admin_claim_in_a_foreign_workspace(
Ok(())
}
/// The guards below cap a job token on routes that name no workspace, which confinement
/// now answers first — so no request can reach them and no HTTP case would notice if they
/// stopped capping. They are called directly for that reason; deleting them because the
/// suite is green elsewhere would leave the inner cap unpinned (GHSA-hfh4-cx4h-3fcr).
#[sqlx::test(fixtures("preserve_on_behalf_of"))]
async fn test_privilege_gates_reject_a_job_token_directly(
db: Pool<Postgres>,
) -> anyhow::Result<()> {
fn authed_as(email: &str, job_id: Option<uuid::Uuid>) -> ApiAuthed {
ApiAuthed {
email: email.to_string(),
username: "runner".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,
}
}
let job = authed_as("test@windmill.dev", Some(uuid::Uuid::new_v4()));
let not_job = authed_as("test@windmill.dev", None);
assert!(
windmill_api_auth::require_devops_role(&db, &job)
.await
.is_err(),
"a job token must not hold the devops role"
);
assert!(
windmill_api_auth::require_instance_admin(&job).is_err(),
"a job token must not hold instance admin"
);
// The identity is a real superadmin, so without the job provenance both gates pass —
// proving the rejections above key off `job_id` and not the fixture's user.
assert!(
windmill_api_auth::require_devops_role(&db, &not_job)
.await
.is_ok(),
"a superadmin API token must still hold the devops role"
);
assert!(
windmill_api_auth::require_instance_admin(&not_job).is_ok(),
"a superadmin API token must still hold instance admin"
);
// The boolean sibling, which `list_worker_groups` consults to decide whether to
// obfuscate rather than to refuse: reading `true` there returns `env_vars_static` in
// the clear, so this one fails by leaking rather than by letting a request through.
assert!(
!windmill_api_auth::is_instance_admin(&job),
"a job token must not read as instance admin"
);
assert!(
windmill_api_auth::is_instance_admin(&not_job),
"an admin API token must still read as instance admin"
);
// `forbid_superadmin_job_token` guards the same class of route but keys on two things
// at once, so all three combinations are worth pinning: it fires only for a job token
// whose identity is a superadmin.
assert!(
windmill_api_auth::forbid_superadmin_job_token(&db, &job.email, job.job_id)
.await
.is_err(),
"a superadmin job token must be forbidden"
);
assert!(
windmill_api_auth::forbid_superadmin_job_token(&db, &not_job.email, None)
.await
.is_ok(),
"a superadmin API token carries no job provenance to forbid"
);
assert!(
windmill_api_auth::forbid_superadmin_job_token(
&db,
"test2@windmill.dev",
Some(uuid::Uuid::new_v4())
)
.await
.is_ok(),
"a job token running as a non-superadmin is not what this gate withholds"
);
// `forbid_elevated_job_token` is the same shape one tier wider — `is_devops_email` is
// true for superadmins too. The embed-token case above reaches it, but only ever with
// an elevated identity; these pin the other two combinations, so collapsing the gate
// into a blanket job-token refusal would be caught here rather than by whoever next
// creates a token from a script.
assert!(
windmill_api_auth::forbid_elevated_job_token(&db, "devops@windmill.dev", job.job_id)
.await
.is_err(),
"a devops job token must not mint a credential"
);
assert!(
windmill_api_auth::forbid_elevated_job_token(&db, &job.email, job.job_id)
.await
.is_err(),
"a superadmin job token must not mint a credential"
);
assert!(
windmill_api_auth::forbid_elevated_job_token(
&db,
"test2@windmill.dev",
Some(uuid::Uuid::new_v4())
)
.await
.is_ok(),
"an unelevated job token is left to the confinement check, not refused here"
);
Ok(())
}
backend/windmill-api-auth/src/auth.rs
+7 -1
View File
@@ -826,9 +826,15 @@ pub async fn resolve_opt_job_authed(
if let Some(mut opt_job_authed) =
cache.get_opt_job_authed(workspace_id.clone(), &token).await
{
let authed = &mut opt_job_authed.authed;
let path = original_uri.path();
let method = parts.method.as_str();
if workspace_id.is_none() && opt_job_authed.job_id.is_some() {
if let Err(err) = crate::scopes::check_job_token_for_global_route(path, method)
{
return Err((err, parts));
}
}
let authed = &mut opt_job_authed.authed;
if authed.scopes.is_some() {
transform_old_scope_to_new_scope(authed.scopes.as_mut());
backend/windmill-api-auth/src/scopes.rs
+90 -1
View File
@@ -982,6 +982,91 @@ fn scope_grants_access(
Ok(true)
}
/// The workspace-less routes a job token (`$WM_TOKEN`) may still read. A route qualifies
/// only when it answers from the caller's own account, from the request body, or with
/// content identical for every workspace (the Hub proxy, the documentation) — never
/// naming another workspace, and never disclosing instance configuration. `usage` reads
/// the caller's own row; `email` and `allowed_domain_auto_invite` are derived from the
/// token itself and touch no table.
///
/// Deliberately absent, as each crosses that line: `users/list_invites` (returns the
/// workspace ids the identity was invited to), `users/tokens/list` (credential metadata
/// of the borrowed identity), `users/exists/{email}` (an oracle over arbitrary
/// addresses, not the caller's own), and `workspaces/list` / `workspaces/users`.
///
/// Read methods only — a mutating handler added on one of these paths must be
/// reconsidered rather than inherit the grant.
fn is_global_read_open_to_job_token(route_path: &str) -> bool {
matches!(
route_path,
"/api/users/whoami"
| "/api/users/email"
| "/api/users/usage"
| "/api/users/tutorial_progress"
| "/api/workspaces/allowed_domain_auto_invite"
| "/api/docs/search"
| "/api/docs/page"
| "/api/integrations/hub/list"
| "/api/embeddings/query_hub_scripts"
) || route_path.starts_with("/api/scripts/hub/")
|| route_path.starts_with("/api/flows/hub/")
|| route_path.starts_with("/api/apps/hub/")
}
/// The workspace-less POSTs a job token keeps. Each takes a `POST` for the sake of a
/// request body rather than to commit anything of consequence: none writes Windmill state
/// outside the caller's own account. The object-storage probe does write to the store the
/// body names — see its entry. What each may *read* is bounded per entry below — the
/// workspace-existence check answers for any id, the rest only from the body or the
/// caller's own row:
/// - a resource editor's object-storage "Test connection" runs as a preview job that POSTs
/// the storage config (`TestConnection.svelte`). It puts and deletes an object to prove
/// the credentials work, so it does write — but only to the store the body names, and a
/// failure between the two can leave that object behind. No Windmill state of any
/// workspace is touched.
/// - `wmill workspace add`, how a job points the CLI at its own instance, checks the
/// workspace exists before accepting the credentials — the git-sync hub scripts run
/// exactly this. `workspace` carries no row-level security, so the bare boolean it
/// answers is instance-wide rather than membership-filtered; what it discloses is
/// only whether a workspace id is taken.
/// - the cron preview computes the next occurrences of the expression in the body. It
/// takes no `ApiAuthed` and opens no transaction, so it returns nothing the caller did
/// not send.
/// - tutorial progress upserts a UI bitfield keyed on the caller's own email. Its path
/// serves a `GET` too, which the read list above carries.
const GLOBAL_WRITES_OPEN_TO_JOB_TOKEN: [&str; 4] = [
"/api/settings/test_object_storage_config",
"/api/workspaces/exists",
"/api/schedules/preview",
"/api/users/tutorial_progress",
];
/// Confines a job token (`$WM_TOKEN`) to routes that name a workspace. It is minted
/// for one job in one workspace yet carries that job's full user privileges, so on an
/// instance-wide route it would mint a permanent workspace-less API token, read
/// worker-group configuration, or manage global users. The token lookup already
/// rejects a workspace-bound API token on those routes; this is the same rule for
/// job tokens.
///
/// Keyed on the job token specifically: an MCP token is workspace-bound too, but
/// deliberately publishes instance-wide tools. Callers pass only routes whose path
/// carries no workspace.
pub fn check_job_token_for_global_route(route_path: &str, http_method: &str) -> Result<()> {
let is_read = map_http_method_to_action(http_method, route_path) == ScopeAction::Read;
if (is_read && is_global_read_open_to_job_token(route_path))
|| (http_method.eq_ignore_ascii_case("POST")
&& GLOBAL_WRITES_OPEN_TO_JOB_TOKEN.contains(&route_path))
{
Ok(())
} else {
Err(Error::PermissionDenied(format!(
"A job token ($WM_TOKEN) is confined to the workspace of its job and cannot be used \
on {route_path}, which is not workspace-scoped. Use an API token created for the \
user instead."
)))
}
}
/// Enforces a token's `read_only` flag: only methods classified as `Read`
/// (GET/HEAD/OPTIONS) are allowed. Run actions and mutating methods are
/// rejected. Independent of `scopes`.
@@ -1227,7 +1312,11 @@ mod tests {
// A kind-only scope confines to that kind, at any path.
let kind_only = job_read_run_confinement(Some(&["jobs:run:scripts".to_string()])).unwrap();
assert!(run_confinement_admits(&kind_only, "scripts", "u/admin/anything"));
assert!(run_confinement_admits(
&kind_only,
"scripts",
"u/admin/anything"
));
assert!(!run_confinement_admits(&kind_only, "flows", "f/team/etl"));
// Scopes that grant job reads in their own right leave reads unconfined.