mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 08:01:26 +00:00
fix: add apps:run to the token scope picker and confine path-scoped app tokens (#10428)
* fix: expose apps:run in the token scope picker and let apps:write grant it Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: confine path-scoped app run/write tokens to the app they name Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: let apps:run read back its own app's S3 files, condense scope comments Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: let apps:write mint apps:run and extend run read-back to app S3 display routes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: drop stale embed-token wording from the app S3 helper summary Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -22,7 +22,9 @@
|
||||
//! - run mode against a deployed Viewer app rejects caller-supplied inline
|
||||
//! `raw_code` whose sha is not publisher-pinned (CVE-2026-22683 residual:
|
||||
//! the Viewer default-triggerable fallback let any caller / an operator run
|
||||
//! arbitrary code as themselves, bypassing the content-hash pin).
|
||||
//! arbitrary code as themselves, bypassing the content-hash pin), and
|
||||
//! - a path-qualified `apps:run|write:<path>` token reaches only that app
|
||||
//! (the route layer is resource-blind, so the handler must path-check).
|
||||
|
||||
use serde_json::json;
|
||||
use sqlx::{Pool, Postgres};
|
||||
@@ -344,5 +346,47 @@ async fn test_app_preview_authorization(db: Pool<Postgres>) -> anyhow::Result<()
|
||||
"rejection must be the content-hash pin (unpinned app_script sha), got: {body}"
|
||||
);
|
||||
|
||||
// 12. Path confinement: the scope picker mints `apps:run:<path>` /
|
||||
// `apps:write:<path>`, but the route layer matches domain + action only, so the
|
||||
// handler is the only place the path is enforced. A token scoped to `vapp` must
|
||||
// not execute another app's components — those run under that app's identity.
|
||||
for token in ["APPS_RUN_VAPP_TOKEN", "APPS_WRITE_VAPP_TOKEN"] {
|
||||
let resp = authed(client().post(format!("{base}/u/test-user/private")), token)
|
||||
.json(&json!({ "args": {}, "component": "comp", "path": "script/u/test-user/private" }))
|
||||
.send()
|
||||
.await?;
|
||||
let status = resp.status();
|
||||
let body = resp.text().await?;
|
||||
assert_eq!(
|
||||
status, 403,
|
||||
"{token} must not execute an app outside its scope path (got {status}): {body}"
|
||||
);
|
||||
assert!(
|
||||
body.contains("apps:run:u/test-user/private"),
|
||||
"rejection must be the app path scope gate, got: {body}"
|
||||
);
|
||||
}
|
||||
|
||||
// 13. ...and must not over-block the app it IS scoped to: both tokens clear the
|
||||
// scope gate for `vapp` and reach the policy (which then rejects the unpinned
|
||||
// code, as in step 9). `apps:write` covering run is what keeps an app-editor
|
||||
// token working without it also holding `apps:run`.
|
||||
for token in ["APPS_RUN_VAPP_TOKEN", "APPS_WRITE_VAPP_TOKEN"] {
|
||||
let resp = authed(client().post(format!("{base}/u/test-user/vapp")), token)
|
||||
.json(&run_mode_raw_code)
|
||||
.send()
|
||||
.await?;
|
||||
let status = resp.status();
|
||||
let body = resp.text().await?;
|
||||
assert_eq!(
|
||||
status, 400,
|
||||
"{token} must clear the scope gate for its own app and reach the policy (got {status}): {body}"
|
||||
);
|
||||
assert!(
|
||||
body.contains("forbidden by policy"),
|
||||
"{token} must be stopped by the policy, not the scope gate, got: {body}"
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
+7
@@ -19,6 +19,13 @@ INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin) VA
|
||||
INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin, scopes) VALUES
|
||||
(encode(sha256('APPS_RUN_TOKEN'::bytea), 'hex'), 'APPS_RUN_T', 'APPS_RUN_TOKEN', 'test2@windmill.dev', 'apps:run scoped token', false, '{apps:run}');
|
||||
|
||||
-- Path-qualified app scopes, as the token scope picker mints them. Both must be
|
||||
-- confined to `u/test-user/vapp` on the execution route (`apps:write` covers run
|
||||
-- for the same app, never for another one).
|
||||
INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin, scopes) VALUES
|
||||
(encode(sha256('APPS_RUN_VAPP_TOKEN'::bytea), 'hex'), 'APPS_RUN_V', 'APPS_RUN_VAPP_TOKEN', 'test2@windmill.dev', 'apps:run path-scoped token', false, '{apps:run:u/test-user/vapp}'),
|
||||
(encode(sha256('APPS_WRITE_VAPP_TOKEN'::bytea), 'hex'), 'APPS_WRIT_', 'APPS_WRITE_VAPP_TOKEN', 'test2@windmill.dev', 'apps:write path-scoped token', false, '{apps:write:u/test-user/vapp}');
|
||||
|
||||
-- A private app owned by `test-user` with a persisted inline script. Used to
|
||||
-- assert that `test-user-2` cannot preview-execute another app's app_script id.
|
||||
INSERT INTO app (id, workspace_id, path, summary, policy, versions) VALUES
|
||||
|
||||
@@ -440,6 +440,9 @@ fn scope_contains(caller: &ScopeDefinition, requested: &ScopeDefinition) -> bool
|
||||
// write subsumes read; otherwise the action must match exactly.
|
||||
match (caller.action.as_str(), requested.action.as_str()) {
|
||||
(c, r) if c == r || (c == "write" && r == "read") => {}
|
||||
// Apps only: `write` covers `run` (see `ScopeDefinition::includes`), so an
|
||||
// app-editor token can mint the narrower run-only credential.
|
||||
("write", "run") if caller.domain == "apps" => {}
|
||||
_ => return false,
|
||||
}
|
||||
|
||||
@@ -1570,6 +1573,25 @@ mod tests {
|
||||
opt_scopes(Some(vec!["users:read", "if_jobs:filter_tags:default"])).as_deref()
|
||||
)
|
||||
.is_ok());
|
||||
// Apps `write` covers `run`, so an app-editor token can mint the run-only
|
||||
// credential for the same app — but only within its own resource subtree,
|
||||
// and the equivalence stays Apps-only.
|
||||
let app_editor = authed_with_scopes(Some(vec!["apps:write:u/me/a", "jobs:write"]));
|
||||
assert!(ensure_scopes_within_caller(
|
||||
&app_editor,
|
||||
opt_scopes(Some(vec!["apps:run:u/me/a"])).as_deref()
|
||||
)
|
||||
.is_ok());
|
||||
assert!(ensure_scopes_within_caller(
|
||||
&app_editor,
|
||||
opt_scopes(Some(vec!["apps:run:u/me/b"])).as_deref()
|
||||
)
|
||||
.is_err());
|
||||
assert!(ensure_scopes_within_caller(
|
||||
&app_editor,
|
||||
opt_scopes(Some(vec!["jobs:run"])).as_deref()
|
||||
)
|
||||
.is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -120,6 +120,10 @@ impl ScopeDefinition {
|
||||
|
||||
match (self.action.as_str(), other.action.as_str()) {
|
||||
(a, b) if (a == "write" && b == "read") || (a == b) => {}
|
||||
// Apps only: `write` can rewrite the app and its policy, so it also covers
|
||||
// running its components. Not general — `jobs:write` must not grant
|
||||
// `jobs:run`. The resource check below still confines it to the same app.
|
||||
("write", "run") if self.domain == "apps" => {}
|
||||
_ => return false,
|
||||
}
|
||||
|
||||
@@ -802,6 +806,15 @@ fn scope_grants_access(
|
||||
&& route_path.is_some_and(resource_metadata_route_allowed));
|
||||
}
|
||||
|
||||
// Apps `write` covers `run` (see `ScopeDefinition::includes`). Like every domain
|
||||
// here this layer is resource-blind; the Run handlers path-check the app.
|
||||
if scope_domain == ScopeDomain::Apps
|
||||
&& scope_action == ScopeAction::Write
|
||||
&& required_action == ScopeAction::Run
|
||||
{
|
||||
return Ok(true);
|
||||
}
|
||||
|
||||
if !scope_action.includes(&required_action)
|
||||
&& !(scope_domain == ScopeDomain::Jobs
|
||||
&& required_action == ScopeAction::Read
|
||||
@@ -1016,6 +1029,29 @@ mod tests {
|
||||
assert!(check_route_access(&sc, "/api/w/test/data_metrics/list", "GET").is_err());
|
||||
}
|
||||
|
||||
/// `apps_u/execute_component` (and the S3 upload the same components drive) is a
|
||||
/// Run action, so a scoped token needs `apps:run`. `apps:write` must keep reaching
|
||||
/// it too: it can rewrite the app and its policy, so withholding execution from it
|
||||
/// protects nothing while breaking every app-scoped token.
|
||||
#[test]
|
||||
fn apps_run_routes_accept_run_and_write_scopes() {
|
||||
let execute = "/api/w/test/apps_u/execute_component/u/admin/app";
|
||||
for scope in ["apps:run", "apps:write"] {
|
||||
assert!(
|
||||
check_route_access(&[scope.to_string()], execute, "POST").is_ok(),
|
||||
"{scope} must reach execute_component"
|
||||
);
|
||||
}
|
||||
assert!(check_route_access(&["apps:read".to_string()], execute, "POST").is_err());
|
||||
// The write-satisfies-run allowance is confined to the apps domain.
|
||||
assert!(check_route_access(
|
||||
&["jobs:write".to_string()],
|
||||
"/api/w/test/jobs/run/p/u/admin/script",
|
||||
"POST"
|
||||
)
|
||||
.is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_new_domain_parsing() {
|
||||
// Test that new domains are properly parsed
|
||||
|
||||
@@ -2837,15 +2837,12 @@ async fn execute_component(
|
||||
Json(mut payload): Json<ExecuteApp>,
|
||||
) -> Result<String> {
|
||||
let path = path.to_path();
|
||||
// Authorize FIRST, before touching the payload: confine the app embed token (the
|
||||
// only credential handed to untrusted app JS, carrying `apps:run:<own path>`) to
|
||||
// the app it was minted for. The route layer can't path-check the apps domain, so
|
||||
// enforce it here. Scoped to embed tokens only — other callers (anonymous, cookie,
|
||||
// plain external JWT) keep their existing access; the run is still policy-gated.
|
||||
// Authorize before touching the payload: the route layer is resource-blind, so a
|
||||
// path-scoped caller (app embed token, or a picker-minted `apps:run|write:<path>`)
|
||||
// is confined to its own app only here. No-op for unscoped callers; anonymous ones
|
||||
// are policy-gated below.
|
||||
if let Some(authed) = opt_authed.as_ref() {
|
||||
if windmill_api_auth::scopes::has_app_embed_sentinel(authed.scopes.as_deref()) {
|
||||
check_scopes(authed, || format!("apps:run:{}", path))?;
|
||||
}
|
||||
check_scopes(authed, || format!("apps:run:{}", path))?;
|
||||
}
|
||||
// Only honor temp_script_refs for the inline-script preview path:
|
||||
// preview/editor mode (force_viewer_static_fields set, == `is_preview`),
|
||||
@@ -3372,14 +3369,10 @@ async fn upload_s3_file_from_app(
|
||||
Query(query): Query<UploadFileToS3Query>,
|
||||
request: axum::extract::Request,
|
||||
) -> JsonResult<AppUploadFileResponse> {
|
||||
// Confine an app embed token (untrusted app JS) to uploading for its OWN app.
|
||||
// The route is reachable with `apps:run` (RUN_PATH_ACTIONS), so without this a
|
||||
// token minted for app A could drive app B's upload policy. Mirrors
|
||||
// execute_component / download_s3_file; other callers are unaffected.
|
||||
// Same path confinement as `execute_component`: without it a token scoped to app A
|
||||
// could drive app B's upload policy.
|
||||
if let Some(authed) = opt_authed.as_ref() {
|
||||
if windmill_api_auth::scopes::has_app_embed_sentinel(authed.scopes.as_deref()) {
|
||||
check_scopes(authed, || format!("apps:run:{}", path.to_path()))?;
|
||||
}
|
||||
check_scopes(authed, || format!("apps:run:{}", path.to_path()))?;
|
||||
}
|
||||
let policy = if let Some(file_key_regex) = query.force_viewer_file_key_regex {
|
||||
// `force_viewer_*` lets the caller supply a synthetic upload policy that
|
||||
@@ -3991,6 +3984,19 @@ struct AppS3FileQueryWithForceViewerAllowedS3Keys {
|
||||
pub force_viewer_allowed_s3_keys: Option<String>,
|
||||
}
|
||||
|
||||
/// Confine a scoped caller to THIS app's S3 files, or it could read another app's
|
||||
/// through that app's on-behalf policy. Either grant reads them: `apps:read:<path>`
|
||||
/// (app embed tokens) or `apps:run:<path>` (a run-scoped token fetching back what its
|
||||
/// own runs produced). Unscoped/anonymous callers fall through to the provenance gate.
|
||||
#[cfg(feature = "parquet")]
|
||||
fn check_app_s3_read_scope(opt_authed: &Option<ApiAuthed>, path: &str) -> Result<()> {
|
||||
let Some(authed) = opt_authed.as_ref() else {
|
||||
return Ok(());
|
||||
};
|
||||
check_scopes(authed, || format!("apps:run:{}", path))
|
||||
.or_else(|_| check_scopes(authed, || format!("apps:read:{}", path)))
|
||||
}
|
||||
|
||||
#[cfg(feature = "parquet")]
|
||||
async fn download_s3_file_from_app(
|
||||
OptAuthed(opt_authed): OptAuthed,
|
||||
@@ -4002,14 +4008,7 @@ async fn download_s3_file_from_app(
|
||||
|
||||
let path = path.to_path();
|
||||
|
||||
// Authorize the app path first: a scoped caller (notably an app embed token,
|
||||
// which carries `apps:read:<own path>`) may only download files for the app it
|
||||
// was minted for — otherwise it could read another app's S3 files via that app's
|
||||
// on-behalf policy. Unscoped sessions / anonymous callers pass through (the
|
||||
// latter still gated by the policy allowlist in `check_if_allowed_...`).
|
||||
if let Some(authed) = opt_authed.as_ref() {
|
||||
check_scopes(authed, || format!("apps:read:{}", path))?;
|
||||
}
|
||||
check_app_s3_read_scope(&opt_authed, path)?;
|
||||
|
||||
let force_viewer_allowed_s3_keys = if let Some(force_viewer_allowed_s3_keys) =
|
||||
query.force_viewer_allowed_s3_keys.clone()
|
||||
@@ -4087,8 +4086,8 @@ fn app_s3_file_query(s3: String, storage: Option<String>, sig: AppS3Sig) -> AppS
|
||||
}
|
||||
}
|
||||
|
||||
/// Shared entry for every app-scoped (`apps_u/*`) S3 display op: scope-confine an
|
||||
/// app embed token, resolve the on-behalf identity per `execution_mode`, then run
|
||||
/// Shared entry for every app-scoped (`apps_u/*`) S3 display op: confine a scoped
|
||||
/// caller to this app, resolve the on-behalf identity per `execution_mode`, then run
|
||||
/// the provenance gate (`check_if_allowed_to_access_s3_file_from_app`) once before
|
||||
/// dispatching to the S3 helpers.
|
||||
#[cfg(feature = "parquet")]
|
||||
@@ -4099,9 +4098,7 @@ async fn app_s3_on_behalf_and_provenance(
|
||||
opt_authed: &Option<ApiAuthed>,
|
||||
file_query: &AppS3FileQuery,
|
||||
) -> Result<crate::db::OptJobAuthed> {
|
||||
if let Some(authed) = opt_authed.as_ref() {
|
||||
check_scopes(authed, || format!("apps:read:{}", path))?;
|
||||
}
|
||||
check_app_s3_read_scope(opt_authed, path)?;
|
||||
let (on_behalf_authed, policy) =
|
||||
get_on_behalf_authed_from_app(db, path, w_id, opt_authed, None).await?;
|
||||
let read_authed = match check_if_allowed_to_access_s3_file_from_app(
|
||||
|
||||
@@ -143,14 +143,8 @@ fn build_standard_scope_domains() -> Vec<ScopeDomain> {
|
||||
|
||||
STANDARD_DOMAINS
|
||||
.iter()
|
||||
.map(|(key, name, desc, req)| ScopeDomain {
|
||||
name: name.to_string(),
|
||||
description: if desc.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(desc.to_string())
|
||||
},
|
||||
scopes: vec![
|
||||
.map(|(key, name, desc, req)| {
|
||||
let mut scopes = vec![
|
||||
ScopeOption {
|
||||
value: format!("{key}:read"),
|
||||
label: "Read".to_string(),
|
||||
@@ -161,7 +155,26 @@ fn build_standard_scope_domains() -> Vec<ScopeDomain> {
|
||||
label: "Write".to_string(),
|
||||
requires_resource_path: *req,
|
||||
},
|
||||
],
|
||||
];
|
||||
// `apps_u/execute_component` and `apps_u/upload_s3_file` are classified as
|
||||
// Run actions, so running a deployed app's components needs `apps:run`:
|
||||
// without it here no supported token can be granted that access.
|
||||
if *key == "apps" {
|
||||
scopes.push(ScopeOption {
|
||||
value: "apps:run".to_string(),
|
||||
label: "Run".to_string(),
|
||||
requires_resource_path: *req,
|
||||
});
|
||||
}
|
||||
ScopeDomain {
|
||||
name: name.to_string(),
|
||||
description: if desc.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(desc.to_string())
|
||||
},
|
||||
scopes,
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
@@ -258,6 +271,22 @@ mod tests {
|
||||
assert!(!values.contains(&"docs:write"), "docs has no write surface");
|
||||
}
|
||||
|
||||
/// Running a deployed app's components is enforced as a Run action, so `apps:run`
|
||||
/// must be selectable here or no supported token can be granted that access.
|
||||
#[test]
|
||||
fn apps_run_scope_is_exposed_and_path_selectable() {
|
||||
let apps = ALL_SCOPES
|
||||
.iter()
|
||||
.find(|d| d.name == "Apps")
|
||||
.expect("Apps domain must exist");
|
||||
let opt = apps
|
||||
.scopes
|
||||
.iter()
|
||||
.find(|s| s.value == "apps:run")
|
||||
.expect("apps:run must be selectable");
|
||||
assert!(opt.requires_resource_path, "apps:run is path-scoped");
|
||||
}
|
||||
|
||||
/// The `data_metrics` route enforces its own scope domain, so `data_metrics:read`
|
||||
/// must be grantable here or no token can ever reach it. It is read-only (the
|
||||
/// catalog is written at deploy) and path-selectable.
|
||||
|
||||
Reference in New Issue
Block a user