mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-09 00:04:10 +00:00
fix: cap app embed/SDK mints and scope widening at the elevated-job-token gate
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
e7f7f19607
commit
c6de975d42
@@ -649,3 +649,70 @@ async fn test_wm_token_cannot_mint_via_mcp_oauth_approval(
|
||||
|
||||
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<Postgres>,
|
||||
) -> 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(())
|
||||
}
|
||||
|
||||
@@ -3160,6 +3160,11 @@ async fn update_token_scopes(
|
||||
Path(token_prefix): Path<String>,
|
||||
Json(req): Json<UpdateTokenScopesRequest>,
|
||||
) -> Result<String> {
|
||||
// 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?;
|
||||
|
||||
@@ -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};
|
||||
@@ -1310,7 +1310,9 @@ async fn mint_raw_app_sdk_token(
|
||||
) -> Result<(String, chrono::DateTime<chrono::Utc>)> {
|
||||
// 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()) {
|
||||
@@ -1383,7 +1385,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)
|
||||
@@ -1507,8 +1509,13 @@ pub async fn mint_app_embed_token(
|
||||
w_id: &str,
|
||||
app_path: &str,
|
||||
opt_authed: Option<&ApiAuthed>,
|
||||
job_id: Option<uuid::Uuid>,
|
||||
) -> Result<EmbedTokenResponse> {
|
||||
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`
|
||||
|
||||
Reference in New Issue
Block a user