fix: stop job tokens minting credentials that shed their provenance

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
hugocasa
2026-08-06 10:23:17 +02:00
parent 21218f8f6d
commit d6caffe9fc
5 changed files with 150 additions and 27 deletions
+5
View File
@@ -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')
+92 -15
View File
@@ -323,9 +323,12 @@ async fn test_wm_token_rejected_by_require_devops_role(db: Pool<Postgres>) -> an
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?;
let resp = authed(
client().get(format!("{base}/service_logs/list_files")),
&sa_wm,
)
.send()
.await?;
assert_eq!(
resp.status(),
401,
@@ -423,12 +426,10 @@ async fn test_wm_token_rejected_by_instance_admin_gates(db: Pool<Postgres>) -> a
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?;
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();
@@ -466,12 +467,30 @@ async fn test_wm_token_rejected_by_instance_admin_gates(db: Pool<Postgres>) -> a
resp.text().await?
);
// 3. 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?;
// 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}"
@@ -494,3 +513,61 @@ async fn test_wm_token_rejected_by_instance_admin_gates(db: Pool<Postgres>) -> a
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<Postgres>,
) -> 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?
);
Ok(())
}
+27
View File
@@ -354,6 +354,33 @@ 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<uuid::Uuid>,
) -> 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(())
}
pub fn check_scopes<F>(authed: &ApiAuthed, required: F) -> error::Result<()>
where
F: FnOnce() -> String,
@@ -1,9 +1,8 @@
use windmill_api_auth::{check_scopes, is_instance_admin, 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};
@@ -43,7 +42,9 @@ async fn list_concurrency_groups(
authed: ApiAuthed,
Extension(db): Extension<DB>,
) -> JsonResult<Vec<ConcurrencyGroups>> {
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",
@@ -284,7 +285,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<UnifiedJob> = if lq.success.is_none() && lq.resolved != Some(true) {
let running_jobs_db: Vec<UnifiedJob> = if lq.success.is_none() && lq.resolved != Some(true)
{
sqlx::query_as(&sql_q).fetch_all(&db).await?
} else {
vec![]
+20 -8
View File
@@ -27,7 +27,9 @@ 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_superadmin_job_token, require_super_admin, OptJobAuthed,
};
use windmill_common::usernames::{
generate_instance_wide_unique_username, get_instance_username_or_create_pending,
};
@@ -2312,12 +2314,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?;
@@ -2762,6 +2762,18 @@ async fn refresh_token(
authed: ApiAuthed,
cookies: Cookies,
) -> Result<String> {
// 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)
@@ -2905,7 +2917,7 @@ async fn create_token(
OptJobAuthed { job_id, .. }: OptJobAuthed,
Json(token_config): Json<NewToken>,
) -> 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,