diff --git a/backend/.sqlx/query-75d1618e12c4dfe5e9632ba8dc45b5aeb572e51ebd0732e6ebf80197acb3e577.json b/backend/.sqlx/query-6b23ae17cd480bf420145b82e46a2bf109d9115775509fb2f1646215094e6e70.json similarity index 58% rename from backend/.sqlx/query-75d1618e12c4dfe5e9632ba8dc45b5aeb572e51ebd0732e6ebf80197acb3e577.json rename to backend/.sqlx/query-6b23ae17cd480bf420145b82e46a2bf109d9115775509fb2f1646215094e6e70.json index 817a13cb2f..e69d9bcd3e 100644 --- a/backend/.sqlx/query-75d1618e12c4dfe5e9632ba8dc45b5aeb572e51ebd0732e6ebf80197acb3e577.json +++ b/backend/.sqlx/query-6b23ae17cd480bf420145b82e46a2bf109d9115775509fb2f1646215094e6e70.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT EXISTS (\n SELECT 1 FROM v2_job_completed c JOIN v2_job j USING (id)\n WHERE j.workspace_id = $2\n AND c.started_at > now() - interval '3 hours'\n AND c.result @> ('{\"s3\":\"' || $1 || '\"}')::jsonb\n AND j.trigger_kind = 'app'\n AND j.trigger = $3\n AND j.created_by = $4\n )", + "query": "SELECT EXISTS (\n SELECT 1 FROM v2_job_completed c JOIN v2_job j USING (id)\n WHERE j.workspace_id = $2\n AND c.started_at > now() - interval '3 hours'\n AND jsonb_path_exists(c.result, '$.** ? (@.s3 == $k)', jsonb_build_object('k', $1::text))\n AND j.trigger_kind = 'app'\n AND j.trigger = $3\n AND j.created_by = $4\n )", "describe": { "columns": [ { @@ -21,5 +21,5 @@ null ] }, - "hash": "75d1618e12c4dfe5e9632ba8dc45b5aeb572e51ebd0732e6ebf80197acb3e577" + "hash": "6b23ae17cd480bf420145b82e46a2bf109d9115775509fb2f1646215094e6e70" } diff --git a/backend/Cargo.lock b/backend/Cargo.lock index e145f1135c..39cf00da8d 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -13900,6 +13900,7 @@ dependencies = [ "flate2", "futures", "git-version", + "globset", "hex", "hmac", "http 1.4.2", diff --git a/backend/tests/app_s3_onbehalf.rs b/backend/tests/app_s3_onbehalf.rs index 2e1085edd3..8473a69a5e 100644 --- a/backend/tests/app_s3_onbehalf.rs +++ b/backend/tests/app_s3_onbehalf.rs @@ -519,3 +519,186 @@ async fn test_preview_is_not_app_provenanced(db: Pool) -> anyhow::Resu Ok(()) } + +/// Seed a completed app-marked job (`trigger_kind='app'` + `trigger=`) by +/// `created_by`, whose result is the caller-supplied JSON — used to seed s3 objects +/// nested inside the result rather than at the top level. +async fn seed_completed_job_with_result( + db: &Pool, + created_by: &str, + app_trigger: &str, + result_json: &str, +) -> anyhow::Result<()> { + sqlx::query( + r#" + WITH j AS ( + INSERT INTO v2_job (id, workspace_id, kind, runnable_path, created_by, + permissioned_as, trigger_kind, trigger) + VALUES (gen_random_uuid(), 'test-workspace', 'script', 'u/test-user/query_to_s3', + $1, 'u/test-user', 'app'::job_trigger_kind, $2) + RETURNING id + ) + INSERT INTO v2_job_completed (id, workspace_id, duration_ms, status, result, started_at) + SELECT id, 'test-workspace', 1, 'success', $3::jsonb, now() FROM j + "#, + ) + .bind(created_by) + .bind(app_trigger) + .bind(result_json) + .execute(db) + .await?; + Ok(()) +} + +/// A runnable may return its s3 object nested (e.g. `[{"s3":..}]` for a query result, +/// or under a field) rather than as the top-level value. The provenance gate must +/// still match the produced key wherever it sits in the result, otherwise the +/// viewer's own app run is denied "File restricted". +#[sqlx::test(fixtures("base"))] +async fn test_deployed_app_s3_nested_result_provenance(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let ws = format!("http://localhost:{port}/api/w/test-workspace"); + + const NESTED_APP: &str = "u/test-user/s3nested"; + const ARRAY_KEY: &str = "results/nested_array.parquet"; + const OBJECT_KEY: &str = "results/nested_object.parquet"; + const UNPRODUCED_KEY: &str = "results/never_produced.parquet"; + + let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN) + .json(&json!({ + "path": NESTED_APP, + "summary": "s3 nested-result provenance test", + "value": {}, + "policy": { "execution_mode": "anonymous", "triggerables": {} } + })) + .send() + .await?; + assert_eq!(resp.status(), 201, "app create: {}", resp.text().await?); + + // The viewer's own app runs produced these keys, nested in the result. + seed_completed_job_with_result( + &db, + "test-user-2", + NESTED_APP, + &format!(r#"[{{"s3":"{ARRAY_KEY}"}}]"#), + ) + .await?; + seed_completed_job_with_result( + &db, + "test-user-2", + NESTED_APP, + &format!(r#"{{"file":{{"s3":"{OBJECT_KEY}"}}}}"#), + ) + .await?; + + let get = |route: String, token: &'static str| { + let url = format!("{ws}/apps_u/{route}"); + authed(client().get(url), token).send() + }; + let denied = |body: &str| body.contains("File restricted"); + + let body = get( + format!("download_s3_file/{NESTED_APP}?s3={ARRAY_KEY}"), + USER_TOKEN, + ) + .await? + .text() + .await?; + assert!( + !denied(&body), + "s3 key nested in an array result must clear the gate: {body}" + ); + + let body = get( + format!("download_s3_file/{NESTED_APP}?s3={OBJECT_KEY}"), + USER_TOKEN, + ) + .await? + .text() + .await?; + assert!( + !denied(&body), + "s3 key nested in an object result must clear the gate: {body}" + ); + + // A key that no job produced stays denied — deep matching does not weaken the gate. + let body = get( + format!("download_s3_file/{NESTED_APP}?s3={UNPRODUCED_KEY}"), + USER_TOKEN, + ) + .await? + .text() + .await?; + assert!( + denied(&body), + "a key no job produced must stay denied: {body}" + ); + + Ok(()) +} + +/// `s3_read_scopes` lets an app author declare globs of keys the app may read +/// on-behalf for files it displays but does not produce in a viewer's session (e.g. +/// results a separate job persisted to S3). In-scope keys clear the confused-deputy +/// gate; out-of-scope keys stay denied. (The read is still bounded by the on-behalf +/// identity's S3 permissions downstream — not exercised here, which asserts the gate.) +#[sqlx::test(fixtures("base"))] +async fn test_deployed_app_s3_read_scopes(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let ws = format!("http://localhost:{port}/api/w/test-workspace"); + + const SCOPED_APP: &str = "u/test-user/s3scoped"; + const IN_SCOPE_KEY: &str = "f/sensitive/results/query.csv"; + const OUT_OF_SCOPE_KEY: &str = "f/other/secret.csv"; + + let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN) + .json(&json!({ + "path": SCOPED_APP, + "summary": "s3 read-scopes test", + "value": {}, + "policy": { + "execution_mode": "anonymous", + "triggerables": {}, + "s3_read_scopes": [{ "path_glob": "f/sensitive/**" }] + } + })) + .send() + .await?; + assert_eq!(resp.status(), 201, "app create: {}", resp.text().await?); + + let get = |route: String| { + let url = format!("{ws}/apps_u/{route}"); + authed(client().get(url), USER_TOKEN).send() + }; + let denied = |body: &str| body.contains("File restricted"); + + // In-scope key clears the gate even though no job produced it. + let body = get(format!("download_s3_file/{SCOPED_APP}?s3={IN_SCOPE_KEY}")) + .await? + .text() + .await?; + assert!( + !denied(&body), + "a key matching a read scope must clear the gate: {body}" + ); + + // A key outside the declared scope stays denied. + let body = get(format!( + "download_s3_file/{SCOPED_APP}?s3={OUT_OF_SCOPE_KEY}" + )) + .await? + .text() + .await?; + assert!( + denied(&body), + "a key outside every read scope must be denied: {body}" + ); + + Ok(()) +} diff --git a/backend/windmill-api/Cargo.toml b/backend/windmill-api/Cargo.toml index fa9ed3f2e5..070f27327b 100644 --- a/backend/windmill-api/Cargo.toml +++ b/backend/windmill-api/Cargo.toml @@ -87,6 +87,7 @@ tower-cookies.workspace = true tower-http.workspace = true hyper.workspace = true itertools.workspace = true +globset.workspace = true reqwest.workspace = true serde.workspace = true sqlx.workspace = true diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 7f9ff752d1..1c18d1784c 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -29167,6 +29167,23 @@ components: type: string resource: type: string + s3_read_scopes: + type: array + description: > + Author-declared literal globs (globset syntax, e.g. `f/sensitive/**`) of S3 + keys a deployed app may read on-behalf of its on_behalf_of identity for files + it displays but does not produce in a viewer's session. Unlike the workspace + advanced-permission rules, template variables (`{folder_read}` etc.) are not + expanded, and `storage` is matched exactly (a scope with no storage covers the + primary storage). Reads remain bounded by the on_behalf_of identity's advanced + S3 permissions. + items: + type: object + properties: + path_glob: + type: string + storage: + type: string execution_mode: type: string enum: [viewer, publisher, anonymous] diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 596d28ed64..69e7e6ddbb 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -307,6 +307,23 @@ pub struct S3Key { storage: Option, } +/// Author-declared surface of S3 keys a deployed app may read on-behalf of its +/// `on_behalf_of` identity, for files the app displays but does NOT produce within +/// a viewer's session (e.g. results a separate job persisted to S3). `path_glob` is a +/// literal glob (globset syntax: `*` within a path segment, `**` across segments — e.g. +/// `f/sensitive/**`); unlike the workspace advanced-permission rules it does NOT expand +/// `{folder_read}`/`{username}`/`{group}` templates. `storage` is matched exactly, so a +/// scope with no `storage` covers the primary storage only. This only widens the +/// confused-deputy provenance gate; the read is still executed on-behalf of +/// `on_behalf_of` and bounded by that identity's advanced S3 permissions downstream, so +/// a scope can never grant more than the on-behalf identity already has. +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct S3ReadScope { + path_glob: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + storage: Option, +} + #[derive(Serialize, Deserialize, Debug, Clone, Default)] pub struct Policy { pub on_behalf_of: Option, @@ -322,6 +339,8 @@ pub struct Policy { pub execution_mode: ExecutionMode, pub s3_inputs: Option>, pub allowed_s3_keys: Option>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub s3_read_scopes: Option>, // WIN-2006: publisher opt-in to iframe sandbox isolation (alpha). When true the // app is isolated from each viewer's Windmill session: low-code renders in an // opaque-origin iframe with a scoped embed token, raw renders its bundle in an @@ -3398,6 +3417,7 @@ async fn upload_s3_file_from_app( .unwrap_or_default(), }]), allowed_s3_keys: None, + s3_read_scopes: None, sandbox: None, }) } else { @@ -3800,6 +3820,7 @@ async fn get_on_behalf_authed_from_app( on_behalf_of_email: None, s3_inputs: None, allowed_s3_keys: Some(force_allowed_s3_keys), + s3_read_scopes: None, sandbox: None, } } else { @@ -3823,6 +3844,7 @@ async fn get_on_behalf_authed_from_app( on_behalf_of_email: None, s3_inputs: None, allowed_s3_keys: None, + s3_read_scopes: None, sandbox: None, }) }; @@ -3875,12 +3897,24 @@ async fn check_if_allowed_to_access_s3_file_from_app( // tokens are excluded (untrusted app JS stays confined below). Ok(()) } else { - // Author-mode/embed: confine reads to the app's declared keys or files THIS - // app produced, else a viewer could launder the author's S3 perms via an - // arbitrary file_key (confused deputy). Provenance is the un-forgeable - // app-origination marker (`trigger_kind='app'` + `trigger=`); - // `created_by=` is ANDed only as a per-viewer isolation filter (it - // can narrow — one viewer can't read another's result — never forge). + // Author-mode/embed: confine reads to the app's declared keys/scopes or files + // THIS app produced, else a viewer could launder the author's S3 perms via an + // arbitrary file_key (confused deputy). Three ways a key clears — all still run + // the read on-behalf of `on_behalf_of`, bounded by that identity's advanced S3 + // permissions downstream, so none can grant more than the on-behalf identity has: + // 1. `allowed_s3_keys`: exact static keys (image/pdf/download components). + // 2. `s3_read_scopes`: author-declared globs, for files the app displays but + // does not produce in the viewer's session (e.g. results a separate job + // persisted to S3). + // 3. Provenance: an un-forgeable app-origination marker (`trigger_kind='app'` + + // `trigger=`) on a recent completed job; `created_by=` is + // ANDed only as a per-viewer isolation filter (it can narrow — one viewer + // can't read another's result — never forge). The s3 key is matched anywhere + // in the result (a runnable may return it nested, e.g. `[{"s3":..}]`), not + // only at the top level. Because the match is deep, a component that echoes + // viewer-controlled input into its result can surface any key that appears + // there — the on-behalf identity's advanced-permission bound above is the + // backstop that keeps this within that identity's own grants. let creator = opt_authed .as_ref() .map(|authed| authed.username.clone()) @@ -3888,13 +3922,22 @@ async fn check_if_allowed_to_access_s3_file_from_app( let allowed = policy.allowed_s3_keys.as_ref().is_some_and(|keys| { keys.iter() .any(|key| key.s3_path == file_query.s3 && key.storage == file_query.storage) + }) || policy.s3_read_scopes.as_ref().is_some_and(|scopes| { + scopes.iter().any(|scope| { + scope.storage == file_query.storage + && globset::Glob::new(&scope.path_glob) + .map_err(|e| { + tracing::error!("Invalid s3_read_scope glob {}: {e}", scope.path_glob) + }) + .is_ok_and(|g| g.compile_matcher().is_match(&file_query.s3)) + }) }) || { sqlx::query_scalar!( r#"SELECT EXISTS ( SELECT 1 FROM v2_job_completed c JOIN v2_job j USING (id) WHERE j.workspace_id = $2 AND c.started_at > now() - interval '3 hours' - AND c.result @> ('{"s3":"' || $1 || '"}')::jsonb + AND jsonb_path_exists(c.result, '$.** ? (@.s3 == $k)', jsonb_build_object('k', $1::text)) AND j.trigger_kind = 'app' AND j.trigger = $3 AND j.created_by = $4 diff --git a/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte b/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte index 880c24810d..d3c8850ae4 100644 --- a/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte +++ b/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte @@ -1,9 +1,9 @@