fix: a guest upload needs an app policy; a missing app does not skip the confinement

backend/tests/app_guest_execution_mode.rs

@@ -456,6 +456,53 @@ async fn guest_cannot_run_another_guest_app(db: Pool<Postgres>) -> 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<Postgres>) -> 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

backend/windmill-api/src/apps.rs

@@ -4516,10 +4516,14 @@ async fn upload_s3_file_from_app(
             .map(|p| serde_json::from_value::<Policy>(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));