From 02b880a88a0c767f2118c41b0dbcf2588f9b1e71 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 2 Sep 2026 15:34:34 +0000 Subject: [PATCH] fix: a guest upload needs an app policy; a missing app does not skip the confinement --- backend/tests/app_guest_execution_mode.rs | 47 +++++++++++++++++++++++ backend/windmill-api/src/apps.rs | 20 +++++++--- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/backend/tests/app_guest_execution_mode.rs b/backend/tests/app_guest_execution_mode.rs index 8c8317463a..247fbd26ac 100644 --- a/backend/tests/app_guest_execution_mode.rs +++ b/backend/tests/app_guest_execution_mode.rs @@ -456,6 +456,53 @@ async fn guest_cannot_run_another_guest_app(db: Pool) -> anyhow::Resul Ok(()) } +/// An upload goes through an app's `s3_inputs` policy or not at all for a guest: the +/// legacy branch for an app without one uploads with the caller's own standing, which a +/// guest has none of, and an app path with no row must not slip past the confinement. +#[cfg(feature = "parquet")] +#[sqlx::test(fixtures("base"))] +async fn a_guest_cannot_upload_outside_a_policy(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"); + + enable_guests(port, "test-workspace").await?; + let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN) + .json(&guest_app_with_runnable(APP_PATH, false)) // no `s3_inputs` + .send() + .await?; + assert_eq!(resp.status(), 201, "{}", resp.text().await?); + insert_guest_token(&db, "test-workspace").await?; + + let upload = |app: &str| { + authed( + client().post(format!( + "{ws}/apps_u/upload_s3_file/{app}?file_key=anything" + )), + GUEST_TOKEN, + ) + .body("x") + .send() + }; + let resp = upload("u/test-user/no_such_app").await?; + assert_eq!( + resp.status(), + 403, + "a path with no app must not escape the guest's confinement: {}", + resp.text().await? + ); + let resp = upload(APP_PATH).await?; + assert_eq!( + resp.status(), + 400, + "without an upload policy a guest is refused like an anonymous caller: {}", + resp.text().await? + ); + + Ok(()) +} + /// An anonymous app is open to anyone, a guest included, and the guest uses it as /// itself: the component run and the result read that follows are one identity, so /// the read's launched-by-me grant matches. Acting as nobody for the run and as the diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index b5d9b16934..b228bbb09d 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -4516,10 +4516,14 @@ async fn upload_s3_file_from_app( .map(|p| serde_json::from_value::(p).map_err(to_anyhow)) .transpose()? }; - let opt_authed = match policy.as_ref() { - Some(policy) => guest_caller_for_mode(opt_authed, policy.execution_mode(), path.to_path())?, - None => opt_authed, - }; + let opt_authed = guest_caller_for_mode( + opt_authed, + policy + .as_ref() + .map(Policy::execution_mode) + .unwrap_or_default(), + path.to_path(), + )?; let user_db = UserDB::new(db.clone()); @@ -4677,8 +4681,12 @@ async fn upload_s3_file_from_app( } } else { // backward compatibility (no policy) - // if no policy but logged in, use the user's auth to get the s3 resource - if let Some(authed) = opt_authed { + // if no policy but logged in, use the user's auth to get the s3 resource. A guest + // has no standing of its own to upload with, so without a policy it is refused + // exactly as an anonymous caller is. + if let Some(authed) = opt_authed + .filter(|a| !windmill_api_auth::scopes::has_guest_sentinel(a.scopes.as_deref())) + { let file_key = query .file_key .unwrap_or_else(|| get_random_file_name(query.file_extension));