diff --git a/backend/.sqlx/query-fe464b8b3ade86743d82c5e3fb14f457e07f07e44c7b693d5d755899d4210dee.json b/backend/.sqlx/query-aa0a2f90d15a642ad3caaa3876d9cb4a5391ff8663da61b48bfeb73bfa005bbe.json similarity index 85% rename from backend/.sqlx/query-fe464b8b3ade86743d82c5e3fb14f457e07f07e44c7b693d5d755899d4210dee.json rename to backend/.sqlx/query-aa0a2f90d15a642ad3caaa3876d9cb4a5391ff8663da61b48bfeb73bfa005bbe.json index 71bb00c25e..7e985b63ec 100644 --- a/backend/.sqlx/query-fe464b8b3ade86743d82c5e3fb14f457e07f07e44c7b693d5d755899d4210dee.json +++ b/backend/.sqlx/query-aa0a2f90d15a642ad3caaa3876d9cb4a5391ff8663da61b48bfeb73bfa005bbe.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT EXISTS(\n SELECT 1\n FROM http_trigger\n WHERE\n ((workspaced_route IS TRUE AND workspace_id || '/' || route_path_key = $1)\n OR (workspaced_route IS FALSE AND route_path_key = $1))\n AND http_method = $2\n AND ($3::TEXT IS NULL OR path != $3)\n )\n ", + "query": "\n SELECT EXISTS(\n SELECT 1\n FROM http_trigger\n WHERE\n ((workspaced_route IS TRUE AND workspace_id || '/' || route_path_key = $1)\n OR (workspaced_route IS FALSE AND route_path_key = $1))\n AND http_method = $2\n AND ($3::TEXT IS NULL OR NOT (workspace_id = $4 AND path = $3))\n )\n ", "describe": { "columns": [ { @@ -26,6 +26,7 @@ } } }, + "Text", "Text" ] }, @@ -33,5 +34,5 @@ null ] }, - "hash": "fe464b8b3ade86743d82c5e3fb14f457e07f07e44c7b693d5d755899d4210dee" + "hash": "aa0a2f90d15a642ad3caaa3876d9cb4a5391ff8663da61b48bfeb73bfa005bbe" } diff --git a/backend/.sqlx/query-94901314288c2b37c942b35d88b6de6cfa0f18fc191f1b1d90dfacd4ea8d2f60.json b/backend/.sqlx/query-aee9bf16e37a5361f96d6b35dddffa6cc37346cdb6b38ecbf6e1530eef3a57bd.json similarity index 83% rename from backend/.sqlx/query-94901314288c2b37c942b35d88b6de6cfa0f18fc191f1b1d90dfacd4ea8d2f60.json rename to backend/.sqlx/query-aee9bf16e37a5361f96d6b35dddffa6cc37346cdb6b38ecbf6e1530eef3a57bd.json index 72b6026aa9..6a93511b46 100644 --- a/backend/.sqlx/query-94901314288c2b37c942b35d88b6de6cfa0f18fc191f1b1d90dfacd4ea8d2f60.json +++ b/backend/.sqlx/query-aee9bf16e37a5361f96d6b35dddffa6cc37346cdb6b38ecbf6e1530eef3a57bd.json @@ -1,15 +1,16 @@ { "db_name": "PostgreSQL", - "query": "INSERT INTO http_trigger (\n path, route_path, route_path_key, script_path, is_flow, workspace_id,\n edited_by, edited_at, extra_perms, authentication_method, http_method,\n static_asset_config, is_static_website, workspaced_route, wrap_body,\n raw_string, authentication_resource_path, summary, description,\n error_handler_path, error_handler_args, retry, request_type, mode,\n permissioned_as, labels\n )\n SELECT\n path, route_path, route_path_key, script_path, is_flow, $1,\n edited_by, edited_at, extra_perms, authentication_method, http_method,\n static_asset_config, is_static_website, workspaced_route, wrap_body,\n raw_string, authentication_resource_path, summary, description,\n error_handler_path, error_handler_args, retry, request_type, 'disabled'::TRIGGER_MODE,\n permissioned_as, labels\n FROM http_trigger WHERE workspace_id = $2", + "query": "INSERT INTO http_trigger (\n path, route_path, route_path_key, script_path, is_flow, workspace_id,\n edited_by, edited_at, extra_perms, authentication_method, http_method,\n static_asset_config, is_static_website, workspaced_route, wrap_body,\n raw_string, authentication_resource_path, summary, description,\n error_handler_path, error_handler_args, retry, request_type, mode,\n permissioned_as, labels\n )\n SELECT\n path, route_path, route_path_key, script_path, is_flow, $1,\n edited_by, edited_at, extra_perms, authentication_method, http_method,\n static_asset_config, is_static_website, workspaced_route, wrap_body,\n raw_string, authentication_resource_path, summary, description,\n error_handler_path, error_handler_args, retry, request_type, 'disabled'::TRIGGER_MODE,\n permissioned_as, labels\n FROM http_trigger\n WHERE workspace_id = $2\n AND (workspaced_route IS TRUE OR $3)", "describe": { "columns": [], "parameters": { "Left": [ "Varchar", - "Text" + "Text", + "Bool" ] }, "nullable": [] }, - "hash": "94901314288c2b37c942b35d88b6de6cfa0f18fc191f1b1d90dfacd4ea8d2f60" + "hash": "aee9bf16e37a5361f96d6b35dddffa6cc37346cdb6b38ecbf6e1530eef3a57bd" } diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index f4a7a835a0..e4277b7ba5 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -30,6 +30,7 @@ use uuid::Uuid; use windmill_audit::audit_oss::{audit_log, AuditAuthorable}; use windmill_audit::ActionKind; use windmill_common::db::UserDB; +use windmill_common::global_settings::HTTP_ROUTE_WORKSPACED_ROUTE; use windmill_common::users::username_to_permissioned_as; use windmill_common::variables::{ build_crypt, decrypt, encrypt, SECRET_SALT, WORKSPACE_CRYPT_CACHE, @@ -3825,6 +3826,15 @@ async fn clone_triggers_and_schedules( .execute(&mut **tx) .await?; + // Skip non-workspaced HTTP triggers: their URL has no workspace prefix, so + // a clone would collide with the parent's row at runtime (matchit::Router + // silently drops one of two duplicates) and `route_path_key_exists` would + // also fail to spot the cross-workspace conflict cleanly. The instance + // settings `CLOUD_HOSTED` and `HTTP_ROUTE_WORKSPACED_ROUTE` force every + // route to be workspace-prefixed regardless of the column, so when either + // is on we clone everything. + let force_workspaced = + *CLOUD_HOSTED || HTTP_ROUTE_WORKSPACED_ROUTE.load(std::sync::atomic::Ordering::Relaxed); sqlx::query!( r#"INSERT INTO http_trigger ( path, route_path, route_path_key, script_path, is_flow, workspace_id, @@ -3841,9 +3851,12 @@ async fn clone_triggers_and_schedules( raw_string, authentication_resource_path, summary, description, error_handler_path, error_handler_args, retry, request_type, 'disabled'::TRIGGER_MODE, permissioned_as, labels - FROM http_trigger WHERE workspace_id = $2"#, + FROM http_trigger + WHERE workspace_id = $2 + AND (workspaced_route IS TRUE OR $3)"#, target_workspace_id, source_workspace_id, + force_workspaced, ) .execute(&mut **tx) .await?; diff --git a/backend/windmill-trigger-http/src/handler.rs b/backend/windmill-trigger-http/src/handler.rs index ad50e9811f..9a42957ff9 100644 --- a/backend/windmill-trigger-http/src/handler.rs +++ b/backend/windmill-trigger-http/src/handler.rs @@ -62,7 +62,8 @@ pub async fn route_path_key_exists( .await? .unwrap_or(false) } else { - let http_route_workspaced = HTTP_ROUTE_WORKSPACED_ROUTE.load(std::sync::atomic::Ordering::Relaxed); + let http_route_workspaced = + HTTP_ROUTE_WORKSPACED_ROUTE.load(std::sync::atomic::Ordering::Relaxed); let effective_workspaced = workspaced_route.unwrap_or(false) || http_route_workspaced; let route_path_key = if effective_workspaced { std::borrow::Cow::Owned(format!("{}/{}", w_id, route_path_key.trim_matches('/'))) @@ -70,6 +71,10 @@ pub async fn route_path_key_exists( std::borrow::Cow::Borrowed(route_path_key) }; + // Self-exclusion is by `(workspace_id, path)` not just `path`: workspace + // forks clone trigger rows verbatim, so the same trigger path can exist + // in multiple workspaces. Excluding by path alone would silently mask a + // real collision against the parent's row. sqlx::query_scalar!( r#" SELECT EXISTS( @@ -79,12 +84,13 @@ pub async fn route_path_key_exists( ((workspaced_route IS TRUE AND workspace_id || '/' || route_path_key = $1) OR (workspaced_route IS FALSE AND route_path_key = $1)) AND http_method = $2 - AND ($3::TEXT IS NULL OR path != $3) + AND ($3::TEXT IS NULL OR NOT (workspace_id = $4 AND path = $3)) ) "#, &route_path_key, http_method as &HttpMethod, - trigger_path + trigger_path, + w_id ) .fetch_one(db) .await? @@ -145,7 +151,8 @@ async fn require_admin_for_instance_wide_route( is_admin: bool, workspaced_route: Option, ) -> Result { - let http_route_workspaced = HTTP_ROUTE_WORKSPACED_ROUTE.load(std::sync::atomic::Ordering::Relaxed); + let http_route_workspaced = + HTTP_ROUTE_WORKSPACED_ROUTE.load(std::sync::atomic::Ordering::Relaxed); let effective_workspaced = workspaced_route.unwrap_or(false) || http_route_workspaced; if !is_admin && !effective_workspaced { return Err(Error::NotAuthorized( @@ -465,7 +472,8 @@ impl TriggerCrud for HttpTrigger { let resolved_edited_by = trigger.base.resolve_edited_by(authed); let resolved_permissioned_as = trigger.base.resolve_permissioned_as(authed); - let http_route_workspaced = HTTP_ROUTE_WORKSPACED_ROUTE.load(std::sync::atomic::Ordering::Relaxed); + let http_route_workspaced = + HTTP_ROUTE_WORKSPACED_ROUTE.load(std::sync::atomic::Ordering::Relaxed); let effective_workspaced = trigger.config.workspaced_route.unwrap_or(false) || http_route_workspaced; diff --git a/docs/fork-triggers.md b/docs/fork-triggers.md index e172ed4713..f2856a2356 100644 --- a/docs/fork-triggers.md +++ b/docs/fork-triggers.md @@ -32,6 +32,18 @@ with two invariants: cloned**. Those triggers manage external webhook state we don't want duplicated. +**Non-workspaced HTTP triggers are also skipped.** A row with +`workspaced_route=false` (and where neither `CLOUD_HOSTED` nor the +`HTTP_ROUTE_WORKSPACED_ROUTE` instance setting is on) has a runtime URL +without any workspace prefix. A clone would collide with the parent's row at +the matchit router level, where duplicate inserts are silently dropped — one +trigger would invisibly hijack the other. There is no namespacing escape +hatch for these (the whole point of `workspaced_route=false` is to skip the +prefix), so the clone filter excludes them. The fork user can re-create one +manually if they need it. When `CLOUD_HOSTED` or `HTTP_ROUTE_WORKSPACED_ROUTE` +is on, every route is workspace-prefixed at runtime regardless of the column, +and the clone copies all rows. + ## Merge-direction filter (always on) Whenever the source workspace ID matches `wm-fork-*`, the tarball export at