fix: the deploy-time guest path guard checks the destination of a rename and refuses a leading slash

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BayTppRCstWX6qTf3LMco5

backend/tests/app_guest_execution_mode.rs

@@ -549,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/src/apps.rs

@@ -321,9 +321,18 @@ impl ExecutionMode {
 /// The protection rule gating a *transition into* `mode`, if any. Anonymous and
 /// guest each widen who may open an app past the workspace's own members, so each
 /// carries its own rule; the two member-only modes are ungated.
+fn deployment_rule_for_mode(mode: ExecutionMode) -> Option<ProtectionRuleKind> {
+    match mode {
+        ExecutionMode::Anonymous => Some(ProtectionRuleKind::RestrictAnonymousAppDeployment),
+        ExecutionMode::Guest => Some(ProtectionRuleKind::RestrictGuestAppDeployment),
+        ExecutionMode::Publisher | ExecutionMode::Viewer => None,
+    }
+}
+
 /// 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!(
@@ -333,14 +342,6 @@ fn refuse_unscopable_guest_app(path: &str, mode: ExecutionMode) -> Result<()> {
     Ok(())
 }
 
-fn deployment_rule_for_mode(mode: ExecutionMode) -> Option<ProtectionRuleKind> {
-    match mode {
-        ExecutionMode::Anonymous => Some(ProtectionRuleKind::RestrictAnonymousAppDeployment),
-        ExecutionMode::Guest => Some(ProtectionRuleKind::RestrictGuestAppDeployment),
-        ExecutionMode::Publisher | ExecutionMode::Viewer => None,
-    }
-}
-
 /// 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
@@ -3354,6 +3355,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 {
@@ -3533,7 +3552,10 @@ async fn update_app_internal<'a>(
                         .unwrap_or_default(),
                 );
             }
-            refuse_unscopable_guest_app(path, npolicy.execution_mode())?;
+            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

@@ -83,9 +83,12 @@ pub fn is_guest_session_label(label: Option<&str>) -> bool {
 
 /// 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, `@`).
+/// 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.chars().any(|c| matches!(c, ':' | ',' | '*'))
+    !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`]