From f662828a30b0c53df9cb00122d3c3e108e70b68b Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 15 Oct 2026 16:23:12 +0200 Subject: [PATCH] test: fix backend integration tests broken since #11777 (#11791) * test: expect app scoping to refuse a foreign app_script id in app_preview_auth #11777 scopes run-mode app_script ids to their app, so another app's id is now refused with 401 before the content-hash pin check is reached. Co-Authored-By: Claude Opus 5.5 * test: run end_user_email inline scripts through the deployed app_script id #11777 runs inline code only through the app_script rows the deployed lite value references once it exists, so id-less raw_code is refused. Shape the fixtures like real apps (nested inlineScript) and run by the deployed id. Co-Authored-By: Claude Opus 5.5 * test: wait for both dedicated worker jobs on one listener The second completed-job listener was opened only after the first job completed, so a second job that finished first was never seen and the test hung until the 90s worker timeout. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- backend/tests/app_preview_auth.rs | 20 +++-- backend/tests/dedicated_workers.rs | 47 ++++++----- backend/tests/end_user_email.rs | 123 ++++++++++++++++------------- 3 files changed, 99 insertions(+), 91 deletions(-) diff --git a/backend/tests/app_preview_auth.rs b/backend/tests/app_preview_auth.rs index cd84e6f278..f8e90ce144 100644 --- a/backend/tests/app_preview_auth.rs +++ b/backend/tests/app_preview_auth.rs @@ -312,13 +312,11 @@ async fn test_app_preview_authorization(db: Pool) -> anyhow::Result<() "run-mode unpinned raw_code must be rejected for any caller, not just operators (got {status}): {body}" ); - // 11. The pin requirement also covers `raw_code` carrying an `app_script` - // `id`. The id resolves to `rawscript/` of any app_script row - // by number (no app scoping), so without the pin a caller could run a - // script belonging to another app against this Viewer app. Here `999777` - // belongs to `u/test-user/private`, not to the targeted `vapp`, and its - // sha is absent from `vapp`'s empty `triggerables_v2` — it must be - // rejected, not fall back to the Viewer default. + // 11. `raw_code` carrying an `app_script` `id` from another app must not run + // against this Viewer app: the row carries its own modules and lock, which + // would run under `vapp`'s identity. Here `999777` belongs to + // `u/test-user/private`, not to the targeted `vapp`, so the id lookup + // (scoped to the app) refuses it before any Viewer default applies. let resp = authed( client().post(format!("{base}/u/test-user/vapp")), "OPERATOR_TOKEN", @@ -338,12 +336,12 @@ async fn test_app_preview_authorization(db: Pool) -> anyhow::Result<() let status = resp.status(); let body = resp.text().await?; assert_eq!( - status, 400, - "run-mode raw_code with an unpinned app_script id must be rejected against a Viewer app (got {status}): {body}" + status, 401, + "run-mode raw_code with another app's app_script id must be rejected against a Viewer app (got {status}): {body}" ); assert!( - body.contains("forbidden by policy"), - "rejection must be the content-hash pin (unpinned app_script sha), got: {body}" + body.contains("does not belong to app u/test-user/vapp"), + "rejection must be the app scoping of the app_script id, got: {body}" ); // 12. Path confinement: the scope picker mints `apps:run:` / diff --git a/backend/tests/dedicated_workers.rs b/backend/tests/dedicated_workers.rs index 5e9c371f90..d0ba3e5e63 100644 --- a/backend/tests/dedicated_workers.rs +++ b/backend/tests/dedicated_workers.rs @@ -23,6 +23,20 @@ mod dedicated_worker_tests { WorkspacedPath { workspace_id: workspace.to_string(), path: path.to_string() } } + /// Waits on one listener for every job: the jobs may complete in any order, and a listener + /// opened after one completes never sees that notification. + async fn wait_for_all( + mut listener: impl futures::Stream + Unpin, + jobs: &[uuid::Uuid], + ) { + use futures::StreamExt; + let mut pending: std::collections::HashSet<_> = jobs.iter().copied().collect(); + while !pending.is_empty() { + let id = listener.next().await.expect("completed job listener ended"); + pending.remove(&id); + } + } + /// Assert that a completed job (or any descendant job) was executed /// by a dedicated worker process with the expected mode by checking job logs /// for the "dedicated worker {mode}:" prefix. @@ -280,10 +294,7 @@ mod dedicated_worker_tests { in_test_worker_dedicated( db.clone(), async { - // Wait for both jobs to complete - listener.find(&uuid_a).await; - let listener2 = listen_for_completed_jobs(&db).await; - listener2.find(&uuid_b).await; + wait_for_all(listener, &[uuid_a, uuid_b]).await; }, port, vec![ @@ -533,9 +544,7 @@ mod dedicated_worker_tests { in_test_worker_dedicated( db.clone(), async { - listener.find(&uuid_a).await; - let listener2 = listen_for_completed_jobs(&db).await; - listener2.find(&uuid_b).await; + wait_for_all(listener, &[uuid_a, uuid_b]).await; }, port, vec![ @@ -659,9 +668,7 @@ mod dedicated_worker_tests { in_test_worker_dedicated( db.clone(), async { - listener.find(&uuid_standalone).await; - let listener2 = listen_for_completed_jobs(&db).await; - listener2.find(&uuid_flow).await; + wait_for_all(listener, &[uuid_standalone, uuid_flow]).await; }, port, vec![ @@ -824,9 +831,7 @@ mod dedicated_worker_tests { in_test_worker_dedicated( db.clone(), async { - listener.find(&uuid_a).await; - let listener2 = listen_for_completed_jobs(&db).await; - listener2.find(&uuid_b).await; + wait_for_all(listener, &[uuid_a, uuid_b]).await; }, port, vec![ @@ -898,9 +903,7 @@ mod dedicated_worker_tests { in_test_worker_dedicated( db.clone(), async { - listener.find(&uuid_pre).await; - let listener2 = listen_for_completed_jobs(&db).await; - listener2.find(&uuid_other).await; + wait_for_all(listener, &[uuid_pre, uuid_other]).await; }, port, vec![ @@ -971,9 +974,7 @@ mod dedicated_worker_tests { in_test_worker_dedicated( db.clone(), async { - listener.find(&uuid_branch).await; - let listener2 = listen_for_completed_jobs(&db).await; - listener2.find(&uuid_default).await; + wait_for_all(listener, &[uuid_branch, uuid_default]).await; }, port, vec![wp("test-workspace", "flow/f/system/dedicated_branch_flow")], @@ -1198,9 +1199,7 @@ mod dedicated_worker_tests { in_test_worker_dedicated( db.clone(), async { - listener.find(&uuid_pre).await; - let listener2 = listen_for_completed_jobs(&db).await; - listener2.find(&uuid_other).await; + wait_for_all(listener, &[uuid_pre, uuid_other]).await; }, port, vec![ @@ -1281,9 +1280,7 @@ mod dedicated_worker_tests { in_test_worker_dedicated( db.clone(), async { - listener.find(&uuid_ws1).await; - let listener2 = listen_for_completed_jobs(&db).await; - listener2.find(&uuid_ws2).await; + wait_for_all(listener, &[uuid_ws1, uuid_ws2]).await; }, port, vec![ diff --git a/backend/tests/end_user_email.rs b/backend/tests/end_user_email.rs index 15eb9a64f1..7fad5ab5ef 100644 --- a/backend/tests/end_user_email.rs +++ b/backend/tests/end_user_email.rs @@ -17,6 +17,9 @@ const SAME_WS_EMAIL: &str = "test@windmill.dev"; const OTHER_WS_EMAIL: &str = "other-ws@windmill.dev"; const NO_WS_EMAIL: &str = "no-ws@windmill.dev"; +const GET_EMAIL_CONTENT: &str = + "export function main() { return process.env.WM_END_USER_EMAIL || \"\"; }"; + fn client() -> reqwest::Client { reqwest::Client::new() } @@ -38,9 +41,11 @@ async fn create_app_with_inline_script(port: u16, path: &str) -> anyhow::Result< "subgrids": {}, "hiddenInlineScripts": [{ "name": "get_email", - "language": "bun", - "content": "export function main() { return process.env.WM_END_USER_EMAIL || \"\"; }", - "path": "f/test/email_app/get_email" + "inlineScript": { + "language": "bun", + "content": GET_EMAIL_CONTENT, + "path": "f/test/email_app/get_email" + } }] }, "policy": { @@ -82,11 +87,13 @@ async fn create_raw_app_with_inline_script(port: u16, path: &str) -> anyhow::Res "value": { "type": "rawapp", "css": "", - "inlineScripts": [{ - "name": "get_email", - "language": "bun", - "content": "export function main() { return process.env.WM_END_USER_EMAIL || \"\"; }" - }] + "runnables": { + "get_email": { + "name": "get_email", + "type": "inline", + "inlineScript": { "language": "bun", "content": GET_EMAIL_CONTENT } + } + } }, "policy": { "execution_mode": "anonymous", @@ -183,7 +190,39 @@ async fn run_app_flow( wait_for_job_result(port, token, &job_id).await } -async fn run_app_inline_script( +fn find_inline_script_id(value: &serde_json::Value) -> Option { + match value { + serde_json::Value::Object(m) => m + .get("inlineScript") + .and_then(|s| s.get("id")) + .and_then(|id| id.as_i64()) + .or_else(|| m.values().find_map(find_inline_script_id)), + serde_json::Value::Array(a) => a.iter().find_map(find_inline_script_id), + _ => None, + } +} + +/// Once the app's dependency job writes the deployed lite value, inline code runs only through +/// the `app_script` rows it references, so the run must carry that row's id. +async fn deployed_inline_script_id(port: u16, app_path: &str) -> anyhow::Result { + let url = format!( + "http://localhost:{}/api/w/test-workspace/apps/get/lite/{}", + port, app_path + ); + for _ in 0..300 { + let resp = authed(client().get(&url), SAME_WS_TOKEN).send().await?; + if resp.status().is_success() { + let app = resp.json::().await?; + if let Some(id) = find_inline_script_id(&app["value"]) { + return Ok(id); + } + } + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + anyhow::bail!("timeout waiting for the deployed app_script id of {app_path}") +} + +async fn run_inline_script( port: u16, token: &str, app_path: &str, @@ -196,9 +235,10 @@ async fn run_app_inline_script( let mut payload = json!({ "args": {}, "component": "get_email", + "id": deployed_inline_script_id(port, app_path).await?, "raw_code": { "language": "bun", - "content": "export function main() { return process.env.WM_END_USER_EMAIL || \"\"; }", + "content": GET_EMAIL_CONTENT, "path": format!("{}/get_email", app_path) } }); @@ -220,42 +260,6 @@ async fn run_app_inline_script( wait_for_job_result(port, token, &job_id).await } -async fn run_raw_app_inline_script( - port: u16, - token: &str, - app_path: &str, - force_viewer: bool, -) -> anyhow::Result { - let url = format!( - "http://localhost:{}/api/w/test-workspace/apps_u/execute_component/{}", - port, app_path - ); - let mut payload = json!({ - "args": {}, - "component": "get_email", - "raw_code": { - "language": "bun", - "content": "export function main() { return process.env.WM_END_USER_EMAIL || \"\"; }" - } - }); - if force_viewer { - payload["force_viewer_static_fields"] = json!({}); - } - let resp = authed(client().post(&url), token) - .json(&payload) - .send() - .await?; - if !resp.status().is_success() { - anyhow::bail!( - "raw app inline script run failed: {} - {}", - resp.status(), - resp.text().await? - ); - } - let job_id = resp.text().await?; - wait_for_job_result(port, token, &job_id).await -} - async fn wait_for_job_result(port: u16, token: &str, job_id: &str) -> anyhow::Result { let url = format!( "http://localhost:{}/api/w/test-workspace/jobs_u/completed/get_result/{}", @@ -399,21 +403,21 @@ async fn test_app_wm_end_user_email(db: Pool) -> anyhow::Result<()> { create_app_with_inline_script(port, app_path).await?; // Same workspace user (force_viewer mode works for workspace members) - let result = run_app_inline_script(port, SAME_WS_TOKEN, app_path, true).await?; + let result = run_inline_script(port, SAME_WS_TOKEN, app_path, true).await?; assert_eq!( result, SAME_WS_EMAIL, "same workspace user should get their email" ); // Other workspace user (uses app's anonymous policy + token lookup) - let result = run_app_inline_script(port, OTHER_WS_TOKEN, app_path, false).await?; + let result = run_inline_script(port, OTHER_WS_TOKEN, app_path, false).await?; assert_eq!( result, OTHER_WS_EMAIL, "other workspace user should get their email" ); // No workspace user (uses app's anonymous policy + token lookup) - let result = run_app_inline_script(port, NO_WS_TOKEN, app_path, false).await?; + let result = run_inline_script(port, NO_WS_TOKEN, app_path, false).await?; assert_eq!( result, NO_WS_EMAIL, "no workspace user should get their email" @@ -481,21 +485,21 @@ async fn test_raw_app_wm_end_user_email(db: Pool) -> anyhow::Result<() create_raw_app_with_inline_script(port, app_path).await?; // Same workspace user (force_viewer mode works for workspace members) - let result = run_raw_app_inline_script(port, SAME_WS_TOKEN, app_path, true).await?; + let result = run_inline_script(port, SAME_WS_TOKEN, app_path, true).await?; assert_eq!( result, SAME_WS_EMAIL, "same workspace user should get their email" ); // Other workspace user (uses app's anonymous policy + token lookup) - let result = run_raw_app_inline_script(port, OTHER_WS_TOKEN, app_path, false).await?; + let result = run_inline_script(port, OTHER_WS_TOKEN, app_path, false).await?; assert_eq!( result, OTHER_WS_EMAIL, "other workspace user should get their email" ); // No workspace user (uses app's anonymous policy + token lookup) - let result = run_raw_app_inline_script(port, NO_WS_TOKEN, app_path, false).await?; + let result = run_inline_script(port, NO_WS_TOKEN, app_path, false).await?; assert_eq!( result, NO_WS_EMAIL, "no workspace user should get their email" @@ -550,7 +554,11 @@ async fn test_app_viewer_data_access_is_stamped_on_inline_runnables_only( })) .send() .await?; - anyhow::ensure!(resp.status().is_success(), "create app: {}", resp.text().await?); + anyhow::ensure!( + resp.status().is_success(), + "create app: {}", + resp.text().await? + ); let execute = |payload: serde_json::Value| async move { let resp = authed( @@ -562,7 +570,11 @@ async fn test_app_viewer_data_access_is_stamped_on_inline_runnables_only( .json(&payload) .send() .await?; - anyhow::ensure!(resp.status().is_success(), "execute: {}", resp.text().await?); + anyhow::ensure!( + resp.status().is_success(), + "execute: {}", + resp.text().await? + ); Ok::<_, anyhow::Error>(resp.text().await?.trim_matches('"').parse::()?) }; let as_viewer = |job_id: uuid::Uuid| { @@ -588,7 +600,8 @@ async fn test_app_viewer_data_access_is_stamped_on_inline_runnables_only( .await?; assert!(as_viewer(inline).await?); - let path = execute(json!({ "args": {}, "component": "run_script", "path": script_path })).await?; + let path = + execute(json!({ "args": {}, "component": "run_script", "path": script_path })).await?; assert!( !as_viewer(path).await?, "a path runnable connected as the viewer because its policy entry said so"