fix: actionable error when a custom_path is taken by an app in another workspace (#9190)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-08-18 16:02:10 +00:00 · 2026-05-16 08:29:53 +00:00
parent 52960ca30a
commit dfeed9c5c2
6 changed files with 392 additions and 48 deletions
@@ -0,0 +1,31 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "SELECT workspace_id, path FROM app WHERE custom_path = $1 AND ($2::TEXT IS NULL OR workspace_id = $2) AND NOT (path = $3 AND workspace_id = $4) LIMIT 1",
+  "describe": {
+    "columns": [
+      {
+        "ordinal": 0,
+        "name": "workspace_id",
+        "type_info": "Varchar"
+      },
+      {
+        "ordinal": 1,
+        "name": "path",
+        "type_info": "Varchar"
+      }
+    ],
+    "parameters": {
+      "Left": [
+        "Text",
+        "Text",
+        "Text",
+        "Text"
+      ]
+    },
+    "nullable": [
+      false,
+      false
+    ]
+  },
+  "hash": "5347ec9ab6de69a99e8823d2199757b244b280e1262dcbccd2fc5189c7b3d25b"
+}
@@ -0,0 +1,29 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "SELECT workspace_id, path FROM app WHERE custom_path = $1 AND ($2::TEXT IS NULL OR workspace_id = $2) LIMIT 1",
+  "describe": {
+    "columns": [
+      {
+        "ordinal": 0,
+        "name": "workspace_id",
+        "type_info": "Varchar"
+      },
+      {
+        "ordinal": 1,
+        "name": "path",
+        "type_info": "Varchar"
+      }
+    ],
+    "parameters": {
+      "Left": [
+        "Text",
+        "Text"
+      ]
+    },
+    "nullable": [
+      false,
+      false
+    ]
+  },
+  "hash": "debd7470025cf64cd1191e604fed9ae0ab27c8a76302a2570473046e28c91fdd"
+}
@@ -1,25 +0,0 @@
-{
-  "db_name": "PostgreSQL",
-  "query": "SELECT EXISTS(SELECT 1 FROM app WHERE custom_path = $1 AND ($2::TEXT IS NULL OR workspace_id = $2) AND NOT (path = $3 AND workspace_id = $4))",
-  "describe": {
-    "columns": [
-      {
-        "ordinal": 0,
-        "name": "exists",
-        "type_info": "Bool"
-      }
-    ],
-    "parameters": {
-      "Left": [
-        "Text",
-        "Text",
-        "Text",
-        "Text"
-      ]
-    },
-    "nullable": [
-      null
-    ]
-  },
-  "hash": "fb1399c1dc171ec6bb24fee3477a1d606d9725e20e9c2d76a0887fadfd87f8df"
-}
@@ -0,0 +1,120 @@
+//! Regression test for the cross-workspace custom_path conflict.
+//!
+//! When custom paths are instance-global (CLOUD_HOSTED unset and
+//! `app_workspaced_route` off — the default for dedicated instances), a
+//! custom_path is a single global route slot. The uniqueness check correctly
+//! blocks two apps from claiming it, including the same logical app deployed
+//! to two workspaces (staging/prod, git-sync). The bug was that the error
+//! ("App with custom path <x> already exists") gave the operator no idea
+//! where the conflicting copy lived. This test pins down:
+//!   - a single-workspace edit keeping its own custom_path still succeeds
+//!     (the app's own row is excluded),
+//!   - a real conflict is still rejected, and
+//!   - the error now names the conflicting app's path and workspace.
+
+use serde_json::json;
+use sqlx::{Pool, Postgres};
+use windmill_test_utils::*;
+
+fn client() -> reqwest::Client {
+    reqwest::Client::new()
+}
+
+fn authed(builder: reqwest::RequestBuilder, token: &str) -> reqwest::RequestBuilder {
+    builder.header("Authorization", format!("Bearer {}", token))
+}
+
+fn new_app(path: &str, custom_path: &str) -> serde_json::Value {
+    json!({
+        "path": path,
+        "summary": "Test app",
+        "value": { "type": "rawapp", "inline_script": null },
+        "policy": { "execution_mode": "anonymous", "triggerables": {} },
+        "custom_path": custom_path
+    })
+}
+
+#[sqlx::test(fixtures("app_custom_path_cross_workspace"))]
+async fn test_custom_path_cross_workspace_deploy(db: Pool<Postgres>) -> anyhow::Result<()> {
+    initialize_tracing().await;
+
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+    let ws_a = format!("http://localhost:{port}/api/w/test-workspace");
+    let ws_b = format!("http://localhost:{port}/api/w/test-workspace-2");
+
+    let app_path = "f/Newsletter/newsletter_composer";
+    let custom_path = "newsletter";
+
+    // 1. Create the app with a custom path in workspace A.
+    let resp = authed(client().post(format!("{ws_a}/apps/create")), "SECRET_TOKEN")
+        .json(&new_app(app_path, custom_path))
+        .send()
+        .await?;
+    assert_eq!(
+        resp.status(),
+        201,
+        "create app in ws A should succeed: {}",
+        resp.text().await?
+    );
+
+    // 2. Editing the app in its own workspace, keeping the same custom path,
+    //    must still succeed — the app's own row is excluded from the check.
+    //    (This is the common single-workspace deploy; it must not regress.)
+    let resp = authed(
+        client().post(format!("{ws_a}/apps/update/{app_path}")),
+        "SECRET_TOKEN",
+    )
+    .json(&json!({
+        "summary": "Test app (edited)",
+        "value": { "type": "rawapp", "inline_script": null },
+        "policy": { "execution_mode": "anonymous", "triggerables": {} },
+        "custom_path": custom_path
+    }))
+    .send()
+    .await?;
+    assert_eq!(
+        resp.status(),
+        200,
+        "editing an app in its own workspace keeping its custom path must succeed: {}",
+        resp.text().await?
+    );
+
+    // 3. Deploying the same app (same path) to a second workspace is a real
+    //    conflict in global mode (one global route slot). It must be rejected,
+    //    and the error must name the conflicting workspace + app so the
+    //    operator knows what to resolve.
+    let resp = authed(client().post(format!("{ws_b}/apps/create")), "SECRET_TOKEN")
+        .json(&new_app(app_path, custom_path))
+        .send()
+        .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(
+        status, 400,
+        "same custom path in another workspace is a global conflict: {body}"
+    );
+    assert!(
+        body.contains("test-workspace") && body.contains(app_path),
+        "error must name the conflicting workspace and app, got: {body}"
+    );
+
+    // 4. A genuinely different app claiming the in-use custom path is still
+    //    rejected, with the same actionable message.
+    let resp = authed(client().post(format!("{ws_a}/apps/create")), "SECRET_TOKEN")
+        .json(&new_app("f/Other/other_app", custom_path))
+        .send()
+        .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(
+        status, 400,
+        "a different app must not steal an in-use custom path: {body}"
+    );
+    assert!(
+        body.contains(app_path),
+        "error must name the app already using the custom path, got: {body}"
+    );
+
+    Ok(())
+}
@@ -0,0 +1,153 @@
+-- Fixture for app_custom_path_cross_workspace regression test.
+-- Two workspaces sharing the same admin user, so the same logical app
+-- (same `path`) can be deployed to both — exercising the instance-global
+-- custom_path uniqueness behavior (CLOUD_HOSTED unset and
+-- app_workspaced_route off, the default for dedicated instances).
+
+INSERT INTO workspace
+            (id,               name,             owner)
+     VALUES ('test-workspace', 'test-workspace', 'test-user');
+
+INSERT INTO usr(workspace_id, email, username, is_admin, role) VALUES
+	('test-workspace', 'test@windmill.dev', 'test-user', true, 'Admin');
+
+INSERT INTO workspace_key(workspace_id, kind, key) VALUES
+	('test-workspace', 'cloud', 'test-key');
+
+INSERT INTO workspace_settings (workspace_id) VALUES
+	('test-workspace');
+
+INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES
+	('test-workspace', 'all', 'All users', '{}');
+
+INSERT INTO password(email, password_hash, login_type, super_admin, verified, name, username)
+    VALUES ('test@windmill.dev', 'not-a-real-hash', 'password', true, true, 'Test User', 'test-user');
+
+-- Second workspace, same admin user. Lets us deploy the same app path to
+-- two workspaces, which is what triggered the custom_path conflict.
+INSERT INTO workspace (id, name, owner) VALUES
+	('test-workspace-2', 'test-workspace-2', 'test-user');
+
+INSERT INTO usr(workspace_id, email, username, is_admin, role) VALUES
+	('test-workspace-2', 'test@windmill.dev', 'test-user', true, 'Admin');
+
+INSERT INTO workspace_key(workspace_id, kind, key) VALUES
+	('test-workspace-2', 'cloud', 'test-key-2');
+
+INSERT INTO workspace_settings (workspace_id) VALUES
+	('test-workspace-2');
+
+INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES
+	('test-workspace-2', 'all', 'All users', '{}');
+
+-- super_admin token so custom_path edits pass require_admin in both workspaces.
+INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin)
+VALUES (encode(sha256('SECRET_TOKEN'::bytea), 'hex'), 'SECRET_TOK', 'SECRET_TOKEN', 'test@windmill.dev', 'test token', true);
+
+GRANT ALL PRIVILEGES ON TABLE workspace_key TO windmill_admin;
+GRANT ALL PRIVILEGES ON TABLE workspace_key TO windmill_user;
+
+CREATE FUNCTION "notify_insert_on_completed_job" ()
+RETURNS TRIGGER AS $$
+BEGIN
+    PERFORM pg_notify('completed', NEW.id::text);
+    RETURN NEW;
+END;
+$$ LANGUAGE PLPGSQL;
+
+  CREATE TRIGGER "notify_insert_on_completed_job"
+ AFTER INSERT ON "v2_job_completed"
+    FOR EACH ROW
+EXECUTE FUNCTION "notify_insert_on_completed_job" ();
+
+
+CREATE FUNCTION "notify_queue" ()
+RETURNS TRIGGER AS $$
+BEGIN
+    PERFORM pg_notify('queued', NEW.id::text);
+    RETURN NEW;
+END;
+$$ LANGUAGE PLPGSQL;
+
+  CREATE TRIGGER "notify_queue_after_insert"
+ AFTER INSERT ON "v2_job_queue"
+    FOR EACH ROW
+EXECUTE FUNCTION "notify_queue" ();
+
+  CREATE TRIGGER "notify_queue_after_flow_status_update"
+ AFTER UPDATE ON "v2_job_status"
+    FOR EACH ROW
+            WHEN (NEW.flow_status IS DISTINCT FROM OLD.flow_status)
+EXECUTE FUNCTION "notify_queue" ();
+
+-- Apply phase 4:
+DROP FUNCTION IF EXISTS v2_job_after_update CASCADE;
+DROP FUNCTION IF EXISTS v2_job_completed_before_insert CASCADE;
+DROP FUNCTION IF EXISTS v2_job_completed_before_update CASCADE;
+DROP FUNCTION IF EXISTS v2_job_queue_after_insert CASCADE;
+DROP FUNCTION IF EXISTS v2_job_queue_before_insert CASCADE;
+DROP FUNCTION IF EXISTS v2_job_queue_before_update CASCADE;
+DROP FUNCTION IF EXISTS v2_job_runtime_before_insert CASCADE;
+DROP FUNCTION IF EXISTS v2_job_runtime_before_update CASCADE;
+DROP FUNCTION IF EXISTS v2_job_status_before_insert CASCADE;
+DROP FUNCTION IF EXISTS v2_job_status_before_update CASCADE;
+
+DROP VIEW IF EXISTS completed_job, completed_job_view, job, queue, queue_view CASCADE;
+
+ALTER TABLE v2_job_queue
+    DROP COLUMN IF EXISTS __parent_job CASCADE,
+    DROP COLUMN IF EXISTS __created_by CASCADE,
+    DROP COLUMN IF EXISTS __script_hash CASCADE,
+    DROP COLUMN IF EXISTS __script_path CASCADE,
+    DROP COLUMN IF EXISTS __args CASCADE,
+    DROP COLUMN IF EXISTS __logs CASCADE,
+    DROP COLUMN IF EXISTS __raw_code CASCADE,
+    DROP COLUMN IF EXISTS __canceled CASCADE,
+    DROP COLUMN IF EXISTS __last_ping CASCADE,
+    DROP COLUMN IF EXISTS __job_kind CASCADE,
+    DROP COLUMN IF EXISTS __env_id CASCADE,
+    DROP COLUMN IF EXISTS __schedule_path CASCADE,
+    DROP COLUMN IF EXISTS __permissioned_as CASCADE,
+    DROP COLUMN IF EXISTS __flow_status CASCADE,
+    DROP COLUMN IF EXISTS __raw_flow CASCADE,
+    DROP COLUMN IF EXISTS __is_flow_step CASCADE,
+    DROP COLUMN IF EXISTS __language CASCADE,
+    DROP COLUMN IF EXISTS __same_worker CASCADE,
+    DROP COLUMN IF EXISTS __raw_lock CASCADE,
+    DROP COLUMN IF EXISTS __pre_run_error CASCADE,
+    DROP COLUMN IF EXISTS __email CASCADE,
+    DROP COLUMN IF EXISTS __visible_to_owner CASCADE,
+    DROP COLUMN IF EXISTS __mem_peak CASCADE,
+    DROP COLUMN IF EXISTS __root_job CASCADE,
+    DROP COLUMN IF EXISTS __leaf_jobs CASCADE,
+    DROP COLUMN IF EXISTS __concurrent_limit CASCADE,
+    DROP COLUMN IF EXISTS __concurrency_time_window_s CASCADE,
+    DROP COLUMN IF EXISTS __timeout CASCADE,
+    DROP COLUMN IF EXISTS __flow_step_id CASCADE,
+    DROP COLUMN IF EXISTS __cache_ttl CASCADE;
+
+LOCK TABLE v2_job_queue IN ACCESS EXCLUSIVE MODE;
+ALTER TABLE v2_job_completed
+    DROP COLUMN IF EXISTS __parent_job CASCADE,
+    DROP COLUMN IF EXISTS __created_by CASCADE,
+    DROP COLUMN IF EXISTS __created_at CASCADE,
+    DROP COLUMN IF EXISTS __success CASCADE,
+    DROP COLUMN IF EXISTS __script_hash CASCADE,
+    DROP COLUMN IF EXISTS __script_path CASCADE,
+    DROP COLUMN IF EXISTS __args CASCADE,
+    DROP COLUMN IF EXISTS __logs CASCADE,
+    DROP COLUMN IF EXISTS __raw_code CASCADE,
+    DROP COLUMN IF EXISTS __canceled CASCADE,
+    DROP COLUMN IF EXISTS __job_kind CASCADE,
+    DROP COLUMN IF EXISTS __env_id CASCADE,
+    DROP COLUMN IF EXISTS __schedule_path CASCADE,
+    DROP COLUMN IF EXISTS __permissioned_as CASCADE,
+    DROP COLUMN IF EXISTS __raw_flow CASCADE,
+    DROP COLUMN IF EXISTS __is_flow_step CASCADE,
+    DROP COLUMN IF EXISTS __language CASCADE,
+    DROP COLUMN IF EXISTS __is_skipped CASCADE,
+    DROP COLUMN IF EXISTS __raw_lock CASCADE,
+    DROP COLUMN IF EXISTS __email CASCADE,
+    DROP COLUMN IF EXISTS __visible_to_owner CASCADE,
+    DROP COLUMN IF EXISTS __tag CASCADE,
+    DROP COLUMN IF EXISTS __priority CASCADE;
@@ -1213,6 +1213,32 @@ async fn create_app(
    Ok((StatusCode::CREATED, path))
 }

+/// Actionable error when a custom path is already taken. In global mode (not
+/// CLOUD_HOSTED and `app_workspaced_route` off) custom paths are unique across
+/// the whole instance, so the conflicting copy may live in another workspace
+/// (e.g. the same app deployed/git-synced to staging and prod) — name it so
+/// the operator knows exactly what to remove.
+fn custom_path_conflict_error(
+    custom_path: &str,
+    conflict_path: &str,
+    conflict_workspace: &str,
+    scoped: bool,
+) -> Error {
+    if scoped {
+        Error::BadRequest(format!(
+            "Custom path '{}' is already used by app '{}' in this workspace",
+            custom_path, conflict_path
+        ))
+    } else {
+        Error::BadRequest(format!(
+            "Custom path '{}' is already used by app '{}' in workspace '{}'. \
+             Custom paths must be unique across the whole instance unless the \
+             'app_workspaced_route' instance setting is enabled.",
+            custom_path, conflict_path, conflict_workspace
+        ))
+    }
+}
+
 async fn create_app_internal<'a>(
    authed: ApiAuthed,
    db: sqlx::Pool<sqlx::Postgres>,
@@ -1284,21 +1310,24 @@ async fn create_app_internal<'a>(
    }
    if let Some(custom_path) = &app.custom_path {
        require_admin(authed.is_admin, &authed.username)?;
-        let as_workspaced_route = APP_WORKSPACED_ROUTE.load(std::sync::atomic::Ordering::Relaxed);
+        let scoped =
+            *CLOUD_HOSTED || APP_WORKSPACED_ROUTE.load(std::sync::atomic::Ordering::Relaxed);

-        let exists = sqlx::query_scalar!(
-            "SELECT EXISTS(SELECT 1 FROM app WHERE custom_path = $1 AND ($2::TEXT IS NULL OR workspace_id = $2))",
+        let conflict = sqlx::query!(
+            "SELECT workspace_id, path FROM app WHERE custom_path = $1 AND ($2::TEXT IS NULL OR workspace_id = $2) LIMIT 1",
            custom_path,
-            if *CLOUD_HOSTED || as_workspaced_route { Some(w_id) } else { None }
+            if scoped { Some(w_id) } else { None }
        )
-        .fetch_one(&mut *tx)
-        .await?.unwrap_or(false);
+        .fetch_optional(&mut *tx)
+        .await?;

-        if exists {
-            return Err(Error::BadRequest(format!(
-                "App with custom path {} already exists",
-                custom_path
-            )));
+        if let Some(conflict) = conflict {
+            return Err(custom_path_conflict_error(
+                custom_path,
+                &conflict.path,
+                &conflict.workspace_id,
+                scoped,
+            ));
        }
    }
    sqlx::query!(
@@ -1781,27 +1810,34 @@ async fn update_app_internal<'a>(

        if let Some(ncustom_path) = &ns.custom_path {
            require_admin(authed.is_admin, &authed.username)?;
-            let as_workspaced_route =
-                APP_WORKSPACED_ROUTE.load(std::sync::atomic::Ordering::Relaxed);
+            let scoped =
+                *CLOUD_HOSTED || APP_WORKSPACED_ROUTE.load(std::sync::atomic::Ordering::Relaxed);

            if ncustom_path.is_empty() {
                sqlb.set("custom_path", "NULL");
            } else {
-                let exists = sqlx::query_scalar!(
-                    "SELECT EXISTS(SELECT 1 FROM app WHERE custom_path = $1 AND ($2::TEXT IS NULL OR workspace_id = $2) AND NOT (path = $3 AND workspace_id = $4))",
+                // Same predicate as before (the check is correct): the app's
+                // own row in this workspace is excluded, so a single-workspace
+                // edit still works. In global mode a copy of this app in
+                // another workspace is a genuine conflict (one global route) —
+                // surface which workspace so it can be resolved.
+                let conflict = sqlx::query!(
+                    "SELECT workspace_id, path FROM app WHERE custom_path = $1 AND ($2::TEXT IS NULL OR workspace_id = $2) AND NOT (path = $3 AND workspace_id = $4) LIMIT 1",
                    ncustom_path,
-                    if *CLOUD_HOSTED || as_workspaced_route { Some(w_id) } else { None },
+                    if scoped { Some(w_id) } else { None },
                    path,
                    w_id
                )
-                .fetch_one(&mut *tx)
-                .await?.unwrap_or(false);
+                .fetch_optional(&mut *tx)
+                .await?;

-                if exists {
-                    return Err(Error::BadRequest(format!(
-                        "App with custom path {} already exists",
-                        ncustom_path
-                    )));
+                if let Some(conflict) = conflict {
+                    return Err(custom_path_conflict_error(
+                        ncustom_path,
+                        &conflict.path,
+                        &conflict.workspace_id,
+                        scoped,
+                    ));
                }
                sqlb.set_str("custom_path", ncustom_path);
            }