Merge origin/license-app-only-implicit-promotion into guest-jwt-app-entry

Adopt the parent's app-path guard for scopes: windmill_common::auth::
is_scope_literal_path (refuses only `:`, `,`, `*` and a leading `/`) rather than
check_proper_path, since app paths may carry spaces and `@`. The relocated
guest_session_scopes and the JWT arm's verify() both use it. The parent's
re-added local guest_session_scopes in users.rs stays relocated to
windmill_api_auth::scopes, its guard preserved there.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VF3v6LA9399gNphmZaHYG3

backend/tests/app_guest_execution_mode.rs

@@ -8,9 +8,10 @@
 //!     could type into `users/tokens/create`;
 //!   * the confinement — a guest reaches the one app it was let in for and nothing
 //!     else;
-//!   * the two gates — an app's own `execution_mode: guest` is inert unless the
-//!     workspace switch is on, checked at the door rather than only where a policy
-//!     is written (git-sync and the CLI push policies past every UI).
+//!   * the switches — an app's own `execution_mode: guest` is inert unless the
+//!     workspace and the instance allow guests, checked at the door rather than only
+//!     where a policy is written (git-sync and the CLI push policies past every UI);
+//!     the allowance on top of them has a binary of its own.
 //!
 //! The token is inserted directly: how a guest session is minted is the identity
 //! provider's business (EE), what one can do is this file's.
@@ -454,33 +455,48 @@ async fn guest_cannot_run_another_guest_app(db: Pool<Postgres>) -> anyhow::Resul
     Ok(())
 }
 
-/// The app path is spliced into the session's scopes, whose parser splits resources on
-/// `,` and reads `*` as a wildcard: a path carrying either would scope the guest to more
-/// than the one app it was let in for, so the mint refuses it before anything else.
+/// The app path is spliced into the session's scopes, whose grammar reserves `:`, `,`
+/// and `*`: a path carrying one would scope the guest to more than the one app it was
+/// let in for, so the mint refuses it before anything else. Anything else in a path
+/// (spaces, `@`) is literal to that grammar and stays admissible.
 #[sqlx::test(fixtures("base"))]
 async fn a_scope_metacharacter_in_the_app_path_is_refused(
     db: Pool<Postgres>,
 ) -> anyhow::Result<()> {
     initialize_tracing().await;
+    let mint = |path: &'static str| {
+        let db = db.clone();
+        async move {
+            let mut tx = db.begin().await?;
+            let minted = windmill_api_users::users::create_guest_session_token(
+                "guest@example.com",
+                "test-workspace",
+                path,
+                &mut tx,
+                tower_cookies::Cookies::default(),
+            )
+            .await;
+            anyhow::Ok(minted)
+        }
+    };
     for path in [
         "u/test-user/entry,u/test-user/hidden",
         "u/test-user/*",
-        "u/test-user/a b",
+        "u/test-user/entry:run",
     ] {
-        let mut tx = db.begin().await?;
-        let minted = windmill_api_users::users::create_guest_session_token(
-            "guest@example.com",
-            "test-workspace",
-            path,
-            &mut tx,
-            tower_cookies::Cookies::default(),
-        )
-        .await;
+        let minted = mint(path).await?;
         assert!(
-            matches!(minted, Err(windmill_common::error::Error::BadRequest(ref m)) if m.contains("Invalid path")),
+            matches!(minted, Err(windmill_common::error::Error::BadRequest(ref m)) if m.contains("cannot be scoped")),
             "{path}: {minted:?}"
         );
     }
+    for path in ["u/test-user/My App", "u/admin@windmill.dev/x"] {
+        let minted = mint(path).await?;
+        assert!(
+            !matches!(minted, Err(windmill_common::error::Error::BadRequest(ref m)) if m.contains("cannot be scoped")),
+            "{path} is literal to the scope grammar and must get past the guard: {minted:?}"
+        );
+    }
     Ok(())
 }
 
@@ -533,6 +549,51 @@ async fn a_guest_cannot_read_a_job_it_did_not_launch(db: Pool<Postgres>) -> anyh
     Ok(())
 }
 
+/// Guests mode cannot land on a path the scope grammar cannot hold, however it gets
+/// there: set at creation, set on update, or a rename of an app already in that mode.
+#[sqlx::test(fixtures("base"))]
+async fn guests_mode_needs_a_scopable_path(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");
+
+    let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN)
+        .json(&guest_app_with_runnable("u/test-user/a:b", false))
+        .send()
+        .await?;
+    assert_eq!(resp.status(), 400, "created into Guests on a `:` path");
+
+    let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN)
+        .json(&guest_app_with_runnable(APP_PATH, false))
+        .send()
+        .await?;
+    assert_eq!(resp.status(), 201, "{}", resp.text().await?);
+    let resp = authed(
+        client().post(format!("{ws}/apps/update/{APP_PATH}")),
+        ADMIN_TOKEN,
+    )
+    .json(&json!({ "path": "u/test-user/a,b" }))
+    .send()
+    .await?;
+    assert_eq!(resp.status(), 400, "renamed to a `,` path while in Guests");
+    let resp = authed(
+        client().post(format!("{ws}/apps/update/{APP_PATH}")),
+        ADMIN_TOKEN,
+    )
+    .json(&json!({ "path": "u/test-user/My App" }))
+    .send()
+    .await?;
+    assert_eq!(
+        resp.status(),
+        200,
+        "a space is literal: {}",
+        resp.text().await?
+    );
+
+    Ok(())
+}
+
 /// The superadmin switch sits above every workspace's: off, no guest session stands and
 /// no app discovers as open, whatever the workspace and the app say.
 #[sqlx::test(fixtures("base"))]

backend/windmill-api-auth/src/scopes.rs

@@ -798,9 +798,13 @@ pub fn with_guest_sentinel(mut scopes: Vec<String>) -> Vec<String> {
 /// grants. A guest has no `usr` row, so this list is the whole of what it can do. The
 /// single source both the mint (a signed-in guest) and the JWT auth arm build from.
 pub fn guest_session_scopes(app_path: &str) -> windmill_common::error::Result<Vec<String>> {
-    // The path is spliced into a scope, whose parser reads `,` as a resource separator
-    // and `*` as a wildcard; a canonical path carries neither.
-    windmill_common::utils::check_proper_path(app_path)?;
+    // The path is spliced into a scope, whose grammar reserves `:`, `,`, `*` and a leading
+    // `/`; app paths may otherwise carry spaces and `@`, so guard only those reserved chars.
+    if !windmill_common::auth::is_scope_literal_path(app_path) {
+        return Err(windmill_common::error::Error::BadRequest(format!(
+            "app path {app_path} cannot be scoped: `:`, `,` and `*` are reserved in scopes"
+        )));
+    }
     Ok(vec![
         GUEST_SENTINEL.to_string(),
         "jobs:read".to_string(),

backend/windmill-api/src/apps.rs

@@ -329,6 +329,19 @@ fn deployment_rule_for_mode(mode: ExecutionMode) -> Option<ProtectionRuleKind> {
     }
 }
 
+/// A guest session is scoped to its app by path, so an app whose path the scope
+/// grammar cannot hold as one literal (`is_scope_literal_path`) can never admit a
+/// guest; refuse the mode at deploy time rather than advertise an app nobody enters.
+/// `path` is where the app ends up: on a rename, the destination.
+fn refuse_unscopable_guest_app(path: &str, mode: ExecutionMode) -> Result<()> {
+    if matches!(mode, ExecutionMode::Guest) && !windmill_common::auth::is_scope_literal_path(path) {
+        return Err(Error::BadRequest(format!(
+            "app {path} cannot be set to Guests: `:`, `,` and `*` in a path cannot be scoped"
+        )));
+    }
+    Ok(())
+}
+
 /// Gate a viewer on the app's `execution_mode`, as far as can be decided without an
 /// ACL probe. `Ok(true)` means already authorized — anonymous admits anyone, guest
 /// admits anyone signed in; `Ok(false)` means the caller is a member and still owes
@@ -2503,6 +2516,7 @@ async fn create_app_internal<'a>(
     // Pin the mode the app is created under, so the stored policy states one
     // even when the caller did not.
     app.policy.set_execution_mode(app.policy.execution_mode());
+    refuse_unscopable_guest_app(&app.path, app.policy.execution_mode())?;
     if let Some(rule) = deployment_rule_for_mode(app.policy.execution_mode()) {
         if let RuleCheckResult::Blocked(msg) = check_user_against_rule(
             w_id,
@@ -3345,6 +3359,24 @@ async fn update_app_internal<'a>(
     // the token's write scope, not just the source path.
     if let Some(npath) = ns.path.as_deref() {
         check_scopes(&authed, || format!("apps:write:{}", npath))?;
+        // The destination is what a guest session would be scoped to; a rename that
+        // carries no policy keeps the deployed mode.
+        let mode = match ns.policy.as_ref().and_then(|p| p.stated_execution_mode()) {
+            Some(mode) => mode,
+            None => sqlx::query_scalar::<_, Option<String>>(
+                "SELECT policy->>'execution_mode' FROM app WHERE workspace_id = $1 AND path = $2",
+            )
+            .bind(w_id)
+            .bind(path)
+            .fetch_optional(&db)
+            .await?
+            .flatten()
+            .and_then(|m| {
+                serde_json::from_value::<ExecutionMode>(serde_json::Value::String(m)).ok()
+            })
+            .unwrap_or_default(),
+        };
+        refuse_unscopable_guest_app(npath, mode)?;
     }
 
     if raw_app {
@@ -3524,6 +3556,10 @@ async fn update_app_internal<'a>(
                         .unwrap_or_default(),
                 );
             }
+            refuse_unscopable_guest_app(
+                ns.path.as_deref().unwrap_or(path),
+                npolicy.execution_mode(),
+            )?;
             if let Some(rule) =
                 deployment_rule_for_mode(npolicy.execution_mode()).filter(|_| !authed.is_admin)
             {

backend/windmill-common/src/auth.rs

@@ -81,6 +81,16 @@ pub fn is_guest_session_label(label: Option<&str>) -> bool {
     label == Some(GUEST_SESSION_LABEL)
 }
 
+/// Whether `path` can be spliced into a scope as one literal resource. The scope
+/// grammar reserves three characters: `:` separates the parts, `,` separates
+/// resources, `*` is a wildcard. App paths are otherwise free-form (spaces, `@`). A
+/// leading `/` is refused too: routes strip it, so the scope would never match.
+pub fn is_scope_literal_path(path: &str) -> bool {
+    !path.is_empty()
+        && !path.starts_with('/')
+        && !path.chars().any(|c| matches!(c, ':' | ',' | '*'))
+}
+
 /// Whether `label` is the one minted for a browser session at login. [`is_server_minted_label`]
 /// stops a member minting it directly, but `/users/refresh_token` hands one to any authenticated
 /// caller, so this attributes a request to the UI without proving it: never gate authority on it.

backend/windmill-common/src/guest_jwt.rs

@@ -133,7 +133,7 @@ pub fn jwk_algorithms(jwk: &Jwk) -> Option<Vec<Algorithm>> {
 /// Verify `token` against `key`, honouring only the accepted `algorithms`, and check
 /// every claim rule that needs no database: signature, `exp` (mandatory), `nbf` and
 /// `iat` when present, the lifetime cap, that the token names `w_id`, that `email` is a
-/// valid address bounded to 254 bytes, and that `app_path` is a canonical path.
+/// valid address bounded to 254 bytes, and that `app_path` carries no scope metacharacter.
 pub fn verify(
     token: &str,
     key: &DecodingKey,
@@ -180,10 +180,13 @@ pub fn verify(
         ));
     }
     // The app path is spliced into `apps:read:<path>` and `apps:run:<path>` scopes, whose
-    // parser reads `,` as a separator and `*` as a wildcard; require a canonical path, the
-    // same guard `guest_session_scopes` applies at the mint.
-    crate::utils::check_proper_path(&claims.app_path)
-        .map_err(|e| Error::NotAuthorized(format!("guest JWT refused: {e}")))?;
+    // grammar reserves `:`, `,`, `*` and a leading `/`; refuse those, the same guard
+    // `guest_session_scopes` applies at the mint. App paths may carry spaces and `@`.
+    if !crate::auth::is_scope_literal_path(&claims.app_path) {
+        return Err(Error::NotAuthorized(
+            "guest JWT refused: app_path contains a character reserved in scopes".to_string(),
+        ));
+    }
     Ok(claims)
 }
 

backend/windmill-common/src/workspaces.rs

@@ -934,7 +934,7 @@ pub async fn guest_app_admits<'c, E: sqlx::Executor<'c, Database = sqlx::Postgre
     app_path: &str,
 ) -> Result<bool> {
     // The mint refuses a path it cannot scope, so discovery must not advertise one.
-    if crate::utils::check_proper_path(app_path).is_err() {
+    if !crate::auth::is_scope_literal_path(app_path) {
         return Ok(false);
     }
     let instance_admits = instance_admits_guests_sql();