diff --git a/backend/windmill-api-flows/src/flows.rs b/backend/windmill-api-flows/src/flows.rs index 546bda32f0..c63d53171c 100644 --- a/backend/windmill-api-flows/src/flows.rs +++ b/backend/windmill-api-flows/src/flows.rs @@ -16,7 +16,8 @@ use axum::{ }; use windmill_api_auth::{ auth::{list_tokens_internal, TruncatedTokenWithEmail}, - check_scopes, maybe_refresh_folders, require_owner_of_path, ApiAuthed, + build_scope_path_predicate, check_scopes, maybe_refresh_folders, require_owner_of_path, + ApiAuthed, }; use windmill_common::workspaces::{check_deploy_rules, RuleCheckResult}; use windmill_common::{ @@ -108,9 +109,10 @@ async fn list_search_flows( let n = 3; let mut tx = user_db.begin(&authed).await?; + let allowed = build_scope_path_predicate(&authed, "flows", "read"); let rows = sqlx::query_as::<_, SearchFlow>( "SELECT flow.path, flow_version.value - FROM flow + FROM flow LEFT JOIN flow_version ON flow_version.id = flow.versions[array_upper(flow.versions, 1)] WHERE flow.workspace_id = $1 LIMIT $2", ) @@ -119,6 +121,7 @@ async fn list_search_flows( .fetch_all(&mut *tx) .await? .into_iter() + .filter(|r| allowed(&r.path)) .collect::>(); tx.commit().await?; Ok(Json(rows)) @@ -212,9 +215,13 @@ async fn list_flows( let sql = sqlb.sql().map_err(|e| Error::internal_err(e.to_string()))?; let mut tx = user_db.begin(&authed).await?; + let allowed = build_scope_path_predicate(&authed, "flows", "read"); let rows = sqlx::query_as::<_, ListableFlow>(&sql) .fetch_all(&mut *tx) - .await?; + .await? + .into_iter() + .filter(|r| allowed(&r.path)) + .collect::>(); tx.commit().await?; Ok(Json(rows)) } diff --git a/backend/windmill-api-integration-tests/tests/flows.rs b/backend/windmill-api-integration-tests/tests/flows.rs index ff3f86bf2d..b6075c8e69 100644 --- a/backend/windmill-api-integration-tests/tests/flows.rs +++ b/backend/windmill-api-integration-tests/tests/flows.rs @@ -259,12 +259,10 @@ async fn test_flow_endpoints(db: Pool) -> anyhow::Result<()> { // ===== Hub endpoints (require external network, expect 500 or 200) ===== // --- hub/list --- - let resp = authed(client().get(format!( - "http://localhost:{port}/api/flows/hub/list" - ))) - .send() - .await - .unwrap(); + let resp = authed(client().get(format!("http://localhost:{port}/api/flows/hub/list"))) + .send() + .await + .unwrap(); assert!( resp.status() == 200 || resp.status() == 500, "hub/list: unexpected status {}", @@ -272,12 +270,10 @@ async fn test_flow_endpoints(db: Pool) -> anyhow::Result<()> { ); // --- hub/get --- - let resp = authed(client().get(format!( - "http://localhost:{port}/api/flows/hub/get/1" - ))) - .send() - .await - .unwrap(); + let resp = authed(client().get(format!("http://localhost:{port}/api/flows/hub/get/1"))) + .send() + .await + .unwrap(); assert!( resp.status() == 200 || resp.status() == 500, "hub/get: unexpected status {}", @@ -286,3 +282,98 @@ async fn test_flow_endpoints(db: Pool) -> anyhow::Result<()> { Ok(()) } + +/// Regression test for GHSA-2ppx-66jv-wpw5: a path-scoped token must only see +/// the flows within its scope when listing, even though the route-level scope +/// check only validates `domain:action`. Before the fix, `list_search` returned +/// `path` + the full flow `value` for every flow the underlying user could see, +/// leaking out-of-scope flow definitions to narrowly-scoped tokens. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_list_search_scope_filtering(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api/w/test-workspace/flows"); + + // Create two folders and one flow in each, as the (super-admin) test user. + for folder in ["allowed", "private"] { + let resp = authed(client().post(format!( + "http://localhost:{port}/api/w/test-workspace/folders/create" + ))) + .json(&json!({ "name": folder })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200, "create folder: {}", resp.text().await?); + } + + for path in ["f/allowed/foo", "f/private/bar"] { + let resp = authed(client().post(format!("{base}/create"))) + .json(&new_flow(path, "summary")) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 201, "create {path}: {}", resp.text().await?); + } + + // Helper: GET /list_search with an arbitrary bearer token, returning the set + // of flow paths visible to that token. + async fn list_search_paths(port: u16, token: &str) -> Vec { + let resp = client() + .get(format!( + "http://localhost:{port}/api/w/test-workspace/flows/list_search" + )) + .header("Authorization", format!("Bearer {token}")) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200); + resp.json::>() + .await + .unwrap() + .into_iter() + .map(|s| s["path"].as_str().unwrap().to_string()) + .collect() + } + + // Insert three tokens for the same super-admin user, differing only by scope. + sqlx::query( + "INSERT INTO token (token_hash, token_prefix, token, email, label, super_admin, scopes) VALUES + (encode(sha256('SCOPED_TOKEN'::bytea), 'hex'), 'SCOPED_TOK', 'SCOPED_TOKEN', 'test@windmill.dev', 'scoped', true, ARRAY['flows:read:f/allowed/*']), + (encode(sha256('BROAD_TOKEN'::bytea), 'hex'), 'BROAD_TOK', 'BROAD_TOKEN', 'test@windmill.dev', 'broad', true, ARRAY['flows:read']), + (encode(sha256('TAG_TOKEN'::bytea), 'hex'), 'TAG_TOK', 'TAG_TOKEN', 'test@windmill.dev', 'tag-only', true, ARRAY['if_jobs:filter_tags:default'])", + ) + .execute(&db) + .await?; + + // Path-scoped token: only sees flows within `f/allowed/*`. + let scoped = list_search_paths(port, "SCOPED_TOKEN").await; + assert!( + scoped.contains(&"f/allowed/foo".to_string()), + "scoped token should see f/allowed/foo, got: {scoped:?}" + ); + assert!( + !scoped.contains(&"f/private/bar".to_string()), + "scoped token must NOT see f/private/bar, got: {scoped:?}" + ); + + // Broad `flows:read` token: still sees every RLS-visible flow. + let broad = list_search_paths(port, "BROAD_TOKEN").await; + assert!(broad.contains(&"f/allowed/foo".to_string())); + assert!( + broad.contains(&"f/private/bar".to_string()), + "broad flows:read token should see all flows, got: {broad:?}" + ); + + // Tag-filter-only token is not scope-restricted: unchanged, sees all. + let tag_only = list_search_paths(port, "TAG_TOKEN").await; + assert!(tag_only.contains(&"f/allowed/foo".to_string())); + assert!(tag_only.contains(&"f/private/bar".to_string())); + + // Unscoped token (no scopes column set): unchanged, sees all. + let unscoped = list_search_paths(port, "SECRET_TOKEN").await; + assert!(unscoped.contains(&"f/allowed/foo".to_string())); + assert!(unscoped.contains(&"f/private/bar".to_string())); + + Ok(()) +} diff --git a/backend/windmill-api-integration-tests/tests/scripts.rs b/backend/windmill-api-integration-tests/tests/scripts.rs index f5e78f880f..c374b757a4 100644 --- a/backend/windmill-api-integration-tests/tests/scripts.rs +++ b/backend/windmill-api-integration-tests/tests/scripts.rs @@ -463,3 +463,107 @@ async fn test_auto_parent_resolves_parent_hash(db: Pool) -> anyhow::Re Ok(()) } + +/// Regression test for GHSA-2ppx-66jv-wpw5: a path-scoped token must only see +/// the scripts within its scope when listing, even though the route-level scope +/// check only validates `domain:action`. Before the fix, `list_search` (and +/// `list`) returned `path` + full `content` for every script the underlying +/// user could see, leaking out-of-scope script source to narrowly-scoped tokens. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_list_search_scope_filtering(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api/w/test-workspace/scripts"); + + // Create two folders and one script in each, as the (super-admin) test user. + for folder in ["allowed", "private"] { + let resp = authed(client().post(format!( + "http://localhost:{port}/api/w/test-workspace/folders/create" + ))) + .json(&json!({ "name": folder })) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200, "create folder: {}", resp.text().await?); + } + + for (path, content) in [ + ( + "f/allowed/foo", + "export async function main() { return 'allowed'; }", + ), + ( + "f/private/bar", + "export async function main() { return 'secret'; }", + ), + ] { + let resp = authed(client().post(format!("{base}/create"))) + .json(&new_script(path, "summary", content)) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 201, "create {path}: {}", resp.text().await?); + } + + // Helper: GET /list_search with an arbitrary bearer token, returning the set + // of script paths visible to that token. + async fn list_search_paths(port: u16, token: &str) -> Vec { + let resp = client() + .get(format!( + "http://localhost:{port}/api/w/test-workspace/scripts/list_search" + )) + .header("Authorization", format!("Bearer {token}")) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200); + resp.json::>() + .await + .unwrap() + .into_iter() + .map(|s| s["path"].as_str().unwrap().to_string()) + .collect() + } + + // Insert three tokens for the same super-admin user, differing only by scope. + sqlx::query( + "INSERT INTO token (token_hash, token_prefix, token, email, label, super_admin, scopes) VALUES + (encode(sha256('SCOPED_TOKEN'::bytea), 'hex'), 'SCOPED_TOK', 'SCOPED_TOKEN', 'test@windmill.dev', 'scoped', true, ARRAY['scripts:read:f/allowed/*']), + (encode(sha256('BROAD_TOKEN'::bytea), 'hex'), 'BROAD_TOK', 'BROAD_TOKEN', 'test@windmill.dev', 'broad', true, ARRAY['scripts:read']), + (encode(sha256('TAG_TOKEN'::bytea), 'hex'), 'TAG_TOK', 'TAG_TOKEN', 'test@windmill.dev', 'tag-only', true, ARRAY['if_jobs:filter_tags:default'])", + ) + .execute(&db) + .await?; + + // Path-scoped token: only sees scripts within `f/allowed/*`. + let scoped = list_search_paths(port, "SCOPED_TOKEN").await; + assert!( + scoped.contains(&"f/allowed/foo".to_string()), + "scoped token should see f/allowed/foo, got: {scoped:?}" + ); + assert!( + !scoped.contains(&"f/private/bar".to_string()), + "scoped token must NOT see f/private/bar, got: {scoped:?}" + ); + + // Broad `scripts:read` token: still sees every RLS-visible script. + let broad = list_search_paths(port, "BROAD_TOKEN").await; + assert!(broad.contains(&"f/allowed/foo".to_string())); + assert!( + broad.contains(&"f/private/bar".to_string()), + "broad scripts:read token should see all scripts, got: {broad:?}" + ); + + // Tag-filter-only token is not scope-restricted: unchanged, sees all. + let tag_only = list_search_paths(port, "TAG_TOKEN").await; + assert!(tag_only.contains(&"f/allowed/foo".to_string())); + assert!(tag_only.contains(&"f/private/bar".to_string())); + + // Unscoped token (no scopes column set): unchanged, sees all. + let unscoped = list_search_paths(port, "SECRET_TOKEN").await; + assert!(unscoped.contains(&"f/allowed/foo".to_string())); + assert!(unscoped.contains(&"f/private/bar".to_string())); + + Ok(()) +} diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index 0dc86bfda8..c783f18d97 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -9,7 +9,8 @@ use axum::extract::Multipart; use windmill_api_auth::{ auth::{list_tokens_internal, AuthCache, TruncatedTokenWithEmail}, - check_scopes, maybe_refresh_folders, require_owner_of_path, ApiAuthed, + build_scope_path_predicate, check_scopes, maybe_refresh_folders, require_owner_of_path, + ApiAuthed, }; use windmill_common::{ utils::{BulkDeleteRequest, WithStarredInfoQuery, HTTP_CLIENT}, @@ -275,6 +276,7 @@ async fn list_search_scripts( #[cfg(not(feature = "enterprise"))] let n = 10; + let allowed = build_scope_path_predicate(&authed, "scripts", "read"); let rows = sqlx::query_as!( SearchScript, "SELECT path, content from script WHERE workspace_id = $1 AND archived = false LIMIT $2", @@ -284,6 +286,7 @@ async fn list_search_scripts( .fetch_all(&mut *tx) .await? .into_iter() + .filter(|r| allowed(&r.path)) .collect::>(); tx.commit().await?; Ok(Json(rows)) @@ -438,9 +441,13 @@ async fn list_scripts( let sql = sqlb.sql().map_err(|e| Error::internal_err(e.to_string()))?; let mut tx = user_db.begin(&authed).await?; + let allowed = build_scope_path_predicate(&authed, "scripts", "read"); let rows = sqlx::query_as::<_, ListableScript>(&sql) .fetch_all(&mut *tx) - .await?; + .await? + .into_iter() + .filter(|r| allowed(&r.path)) + .collect::>(); tx.commit().await?; Ok(Json(rows)) }