From d8d7332eb6d92f7a55b82890de5de4196039b40d Mon Sep 17 00:00:00 2001 From: hugocasa Date: Mon, 14 Sep 2026 11:36:01 +0200 Subject: [PATCH 1/2] feat: add per-route CORS origin allowlist for HTTP triggers (#10833) * feat: add per-route CORS origin allowlist for HTTP triggers Co-Authored-By: Claude Opus 5 * fix: fail closed on cold router cache and invalid origin input Co-Authored-By: Claude Opus 5 * fix: resolve CORS route from the decoded path like the request handler Co-Authored-By: Claude Opus 5 * feat: add instance-wide default allowed origins for HTTP routes Co-Authored-By: Claude Opus 5 * fix: let non-superadmins read the default allowed origins setting Co-Authored-By: Claude Opus 5 * feat: badge the advanced section when a route's origins are restricted Co-Authored-By: Claude Opus 5 * fix: state inherited origins on the control and use one hint row Co-Authored-By: Claude Opus 5 * fix: trim the origins tooltip and relabel the toggle when a default exists Co-Authored-By: Claude Opus 5 * fix: keep the origins format hint visible until an entry is wrong Co-Authored-By: Claude Opus 5 * fix: state the at-least-one requirement in the origins hint Co-Authored-By: Claude Opus 5 * fix: import the origins validator in the trigger-http tests Co-Authored-By: Claude Opus 5 * fix: make an empty allowlist deny rather than fall back to the default Co-Authored-By: Claude Opus 5 * fix: address review nits on origin validation and the CORS editor * fix: derive the origins error from the stored list and tighten host validation * fix: parse real IPv6 hosts and refuse a newly emptied allowlist * refactor: make origin validation advisory except for null and non-ascii * feat: let an empty allowlist be saved as deny every origin * docs: document the empty allowlist as deny every origin * fix: bound allowlists, reject commas, and decide cors after the handler * chore: revert unrelated rustfmt churn in windmill-common tests * chore: revert unrelated rustfmt churn in windmill-common * chore: drop the route types the cors restructure replaced * fix: take the stricter cors decision from before and after the handler * fix: strip runnable cors headers when the routers are unavailable * docs: document the allowlist bounds in the openapi schema * fix: let an unavailable cors read defer to one that resolved * refactor: carry the resolved cors policy from the handler to the middleware * docs: describe why an unavailable read fails closed on the paths that reach it * fix: validate the default origins on the declarative settings path * test: keep the webhook doc comment with the test it describes * fix: warn on impossible schemes and ports, and validate the instance setting * feat: treat an empty allowlist as unset at both levels * perf: decode the cors path only when the fallback needs it * docs: document the empty allowlist as unset in the api schema * docs: describe an empty allowlist as unset in the frontend comments * docs: say what a null allowlist resolves to, not what it meant before the default existed * docs: state what the validator refuses and why methods stay broad * feat: exempt static asset routes from the origin allowlist * fix: hide the origin control for every static target, not just websites * fix: exempt only static websites, not single-file static assets * fix: warn on an unclosed ipv6 host in the origins advisory * fix: require assets present, not just the static website flag --------- Co-authored-by: Claude Opus 5 --- ...871ca8667a657dfdfc2fa65e9ff1a3c0d2908.json | 87 ++++++ ...cbc2bc7235c63ed4f0a885d412793f3a3a3fc.json | 84 ++++++ ...22e8a9147d36a4b56ebd30a1aaa960156b598.json | 86 ++++++ ...ef9db3a0579c889bcae4e8bf33467ce5cebd1.json | 16 + ...1eaf40862b464af7113d2632c626e43c35c04.json | 185 ++++++++++++ ...5019_http_trigger_allowed_origins.down.sql | 2 + ...105019_http_trigger_allowed_origins.up.sql | 2 + backend/src/main.rs | 11 +- backend/src/monitor.rs | 45 ++- backend/summarized_schema.txt | 2 +- backend/tests/instance_config.rs | 59 +++- backend/windmill-api-settings/src/lib.rs | 24 +- .../windmill-api-workspaces/src/workspaces.rs | 4 +- backend/windmill-api/openapi.yaml | 24 ++ .../windmill-api/src/triggers/http/handler.rs | 284 +++++++++++++++++- .../windmill-common/src/global_settings.rs | 120 ++++++++ .../windmill-common/src/instance_config.rs | 12 +- backend/windmill-trigger-http/src/handler.rs | 91 +++--- backend/windmill-trigger-http/src/lib.rs | 218 +++++++++++++- cli/src/guidance/skills.gen.ts | 15 + .../copilot/chat/workspaceToolsZod.gen.ts | 1 + .../src/lib/components/instanceSettings.ts | 17 ++ .../triggers/http/RouteCorsOption.svelte | 132 ++++++++ .../triggers/http/RouteEditorInner.svelte | 51 +++- .../src/lib/components/triggers/http/utils.ts | 141 +++++++++ .../schemas/http_trigger.schema.yaml | 15 + 26 files changed, 1659 insertions(+), 69 deletions(-) create mode 100644 backend/.sqlx/query-2ead4c5e0fec64dfdc24431d2f4871ca8667a657dfdfc2fa65e9ff1a3c0d2908.json create mode 100644 backend/.sqlx/query-47e0f46fddb3ad1c854deb9bdbdcbc2bc7235c63ed4f0a885d412793f3a3a3fc.json create mode 100644 backend/.sqlx/query-ab752dd133b20103800554b3f6622e8a9147d36a4b56ebd30a1aaa960156b598.json create mode 100644 backend/.sqlx/query-c1976ac63f5d763b2a747ff92f5ef9db3a0579c889bcae4e8bf33467ce5cebd1.json create mode 100644 backend/.sqlx/query-c7a78d3db99e7f709479c9520471eaf40862b464af7113d2632c626e43c35c04.json create mode 100644 backend/migrations/20260825105019_http_trigger_allowed_origins.down.sql create mode 100644 backend/migrations/20260825105019_http_trigger_allowed_origins.up.sql create mode 100644 frontend/src/lib/components/triggers/http/RouteCorsOption.svelte diff --git a/backend/.sqlx/query-2ead4c5e0fec64dfdc24431d2f4871ca8667a657dfdfc2fa65e9ff1a3c0d2908.json b/backend/.sqlx/query-2ead4c5e0fec64dfdc24431d2f4871ca8667a657dfdfc2fa65e9ff1a3c0d2908.json new file mode 100644 index 0000000000..99c8262860 --- /dev/null +++ b/backend/.sqlx/query-2ead4c5e0fec64dfdc24431d2f4871ca8667a657dfdfc2fa65e9ff1a3c0d2908.json @@ -0,0 +1,87 @@ +{ + "db_name": "PostgreSQL", + "query": "\n UPDATE\n http_trigger\n SET\n route_path = $1,\n route_path_key = $2,\n workspaced_route = $3,\n wrap_body = $4,\n raw_string = $5,\n allowed_origins = $6,\n authentication_resource_path = $7,\n script_path = $8,\n path = $9,\n is_flow = $10,\n mode = $11,\n http_method = $12,\n static_asset_config = $13,\n edited_by = $14,\n permissioned_as = $15,\n request_type = $16,\n authentication_method = $17,\n summary = $18,\n description = $19,\n edited_at = now(),\n is_static_website = $20,\n error_handler_path = $21,\n error_handler_args = $22,\n retry = $23\n WHERE\n workspace_id = $24 AND\n path = $25\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Varchar", + "Bool", + "Bool", + "Bool", + "TextArray", + "Varchar", + "Varchar", + "Varchar", + "Bool", + { + "Custom": { + "name": "trigger_mode", + "kind": { + "Enum": [ + "enabled", + "disabled", + "suspended" + ] + } + } + }, + { + "Custom": { + "name": "http_method", + "kind": { + "Enum": [ + "get", + "post", + "put", + "delete", + "patch" + ] + } + } + }, + "Jsonb", + "Varchar", + "Varchar", + { + "Custom": { + "name": "request_type", + "kind": { + "Enum": [ + "sync", + "async", + "sync_sse" + ] + } + } + }, + { + "Custom": { + "name": "authentication_method", + "kind": { + "Enum": [ + "none", + "windmill", + "api_key", + "basic_http", + "custom_script", + "signature" + ] + } + } + }, + "Varchar", + "Text", + "Bool", + "Varchar", + "Jsonb", + "Jsonb", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "2ead4c5e0fec64dfdc24431d2f4871ca8667a657dfdfc2fa65e9ff1a3c0d2908" +} diff --git a/backend/.sqlx/query-47e0f46fddb3ad1c854deb9bdbdcbc2bc7235c63ed4f0a885d412793f3a3a3fc.json b/backend/.sqlx/query-47e0f46fddb3ad1c854deb9bdbdcbc2bc7235c63ed4f0a885d412793f3a3a3fc.json new file mode 100644 index 0000000000..6bec80ccb2 --- /dev/null +++ b/backend/.sqlx/query-47e0f46fddb3ad1c854deb9bdbdcbc2bc7235c63ed4f0a885d412793f3a3a3fc.json @@ -0,0 +1,84 @@ +{ + "db_name": "PostgreSQL", + "query": "\n UPDATE\n http_trigger\n SET\n wrap_body = $1,\n raw_string = $2,\n allowed_origins = $3,\n authentication_resource_path = $4,\n script_path = $5,\n path = $6,\n is_flow = $7,\n mode = $8,\n http_method = $9,\n static_asset_config = $10,\n edited_by = $11,\n permissioned_as = $12,\n request_type = $13,\n authentication_method = $14,\n summary = $15,\n description = $16,\n edited_at = now(),\n is_static_website = $17,\n error_handler_path = $18,\n error_handler_args = $19,\n retry = $20\n WHERE\n workspace_id = $21 AND\n path = $22\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Bool", + "Bool", + "TextArray", + "Varchar", + "Varchar", + "Varchar", + "Bool", + { + "Custom": { + "name": "trigger_mode", + "kind": { + "Enum": [ + "enabled", + "disabled", + "suspended" + ] + } + } + }, + { + "Custom": { + "name": "http_method", + "kind": { + "Enum": [ + "get", + "post", + "put", + "delete", + "patch" + ] + } + } + }, + "Jsonb", + "Varchar", + "Varchar", + { + "Custom": { + "name": "request_type", + "kind": { + "Enum": [ + "sync", + "async", + "sync_sse" + ] + } + } + }, + { + "Custom": { + "name": "authentication_method", + "kind": { + "Enum": [ + "none", + "windmill", + "api_key", + "basic_http", + "custom_script", + "signature" + ] + } + } + }, + "Varchar", + "Text", + "Bool", + "Varchar", + "Jsonb", + "Jsonb", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "47e0f46fddb3ad1c854deb9bdbdcbc2bc7235c63ed4f0a885d412793f3a3a3fc" +} diff --git a/backend/.sqlx/query-ab752dd133b20103800554b3f6622e8a9147d36a4b56ebd30a1aaa960156b598.json b/backend/.sqlx/query-ab752dd133b20103800554b3f6622e8a9147d36a4b56ebd30a1aaa960156b598.json new file mode 100644 index 0000000000..5a9e5da58e --- /dev/null +++ b/backend/.sqlx/query-ab752dd133b20103800554b3f6622e8a9147d36a4b56ebd30a1aaa960156b598.json @@ -0,0 +1,86 @@ +{ + "db_name": "PostgreSQL", + "query": "\n INSERT INTO http_trigger (\n workspace_id,\n path,\n route_path,\n route_path_key,\n workspaced_route,\n authentication_resource_path,\n wrap_body,\n raw_string,\n allowed_origins,\n script_path,\n summary,\n description,\n is_flow,\n mode,\n request_type,\n authentication_method,\n http_method,\n static_asset_config,\n edited_by,\n permissioned_as,\n edited_at,\n is_static_website,\n error_handler_path,\n error_handler_args,\n retry\n )\n VALUES (\n $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, now(), $21, $22, $23, $24\n )\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Varchar", + "Varchar", + "Varchar", + "Bool", + "Varchar", + "Bool", + "Bool", + "TextArray", + "Varchar", + "Varchar", + "Text", + "Bool", + { + "Custom": { + "name": "trigger_mode", + "kind": { + "Enum": [ + "enabled", + "disabled", + "suspended" + ] + } + } + }, + { + "Custom": { + "name": "request_type", + "kind": { + "Enum": [ + "sync", + "async", + "sync_sse" + ] + } + } + }, + { + "Custom": { + "name": "authentication_method", + "kind": { + "Enum": [ + "none", + "windmill", + "api_key", + "basic_http", + "custom_script", + "signature" + ] + } + } + }, + { + "Custom": { + "name": "http_method", + "kind": { + "Enum": [ + "get", + "post", + "put", + "delete", + "patch" + ] + } + } + }, + "Jsonb", + "Varchar", + "Varchar", + "Bool", + "Varchar", + "Jsonb", + "Jsonb" + ] + }, + "nullable": [] + }, + "hash": "ab752dd133b20103800554b3f6622e8a9147d36a4b56ebd30a1aaa960156b598" +} diff --git a/backend/.sqlx/query-c1976ac63f5d763b2a747ff92f5ef9db3a0579c889bcae4e8bf33467ce5cebd1.json b/backend/.sqlx/query-c1976ac63f5d763b2a747ff92f5ef9db3a0579c889bcae4e8bf33467ce5cebd1.json new file mode 100644 index 0000000000..ed6a4f9b5e --- /dev/null +++ b/backend/.sqlx/query-c1976ac63f5d763b2a747ff92f5ef9db3a0579c889bcae4e8bf33467ce5cebd1.json @@ -0,0 +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, allowed_origins, 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, allowed_origins, 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", + "Bool" + ] + }, + "nullable": [] + }, + "hash": "c1976ac63f5d763b2a747ff92f5ef9db3a0579c889bcae4e8bf33467ce5cebd1" +} diff --git a/backend/.sqlx/query-c7a78d3db99e7f709479c9520471eaf40862b464af7113d2632c626e43c35c04.json b/backend/.sqlx/query-c7a78d3db99e7f709479c9520471eaf40862b464af7113d2632c626e43c35c04.json new file mode 100644 index 0000000000..7bc28f0f39 --- /dev/null +++ b/backend/.sqlx/query-c7a78d3db99e7f709479c9520471eaf40862b464af7113d2632c626e43c35c04.json @@ -0,0 +1,185 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT\n path,\n script_path,\n is_flow,\n route_path,\n authentication_resource_path,\n workspace_id,\n request_type AS \"request_type: _\",\n authentication_method AS \"authentication_method: _\",\n edited_by,\n permissioned_as,\n static_asset_config AS \"static_asset_config: _\",\n wrap_body,\n raw_string,\n allowed_origins,\n workspaced_route,\n is_static_website,\n error_handler_path,\n error_handler_args as \"error_handler_args: _\",\n retry as \"retry: _\",\n mode as \"mode: _\"\n FROM\n http_trigger\n WHERE\n http_method = $1 AND\n (mode = 'enabled'::TRIGGER_MODE OR mode = 'suspended'::TRIGGER_MODE)\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "path", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "script_path", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "is_flow", + "type_info": "Bool" + }, + { + "ordinal": 3, + "name": "route_path", + "type_info": "Varchar" + }, + { + "ordinal": 4, + "name": "authentication_resource_path", + "type_info": "Varchar" + }, + { + "ordinal": 5, + "name": "workspace_id", + "type_info": "Varchar" + }, + { + "ordinal": 6, + "name": "request_type: _", + "type_info": { + "Custom": { + "name": "request_type", + "kind": { + "Enum": [ + "sync", + "async", + "sync_sse" + ] + } + } + } + }, + { + "ordinal": 7, + "name": "authentication_method: _", + "type_info": { + "Custom": { + "name": "authentication_method", + "kind": { + "Enum": [ + "none", + "windmill", + "api_key", + "basic_http", + "custom_script", + "signature" + ] + } + } + } + }, + { + "ordinal": 8, + "name": "edited_by", + "type_info": "Varchar" + }, + { + "ordinal": 9, + "name": "permissioned_as", + "type_info": "Varchar" + }, + { + "ordinal": 10, + "name": "static_asset_config: _", + "type_info": "Jsonb" + }, + { + "ordinal": 11, + "name": "wrap_body", + "type_info": "Bool" + }, + { + "ordinal": 12, + "name": "raw_string", + "type_info": "Bool" + }, + { + "ordinal": 13, + "name": "allowed_origins", + "type_info": "TextArray" + }, + { + "ordinal": 14, + "name": "workspaced_route", + "type_info": "Bool" + }, + { + "ordinal": 15, + "name": "is_static_website", + "type_info": "Bool" + }, + { + "ordinal": 16, + "name": "error_handler_path", + "type_info": "Varchar" + }, + { + "ordinal": 17, + "name": "error_handler_args: _", + "type_info": "Jsonb" + }, + { + "ordinal": 18, + "name": "retry: _", + "type_info": "Jsonb" + }, + { + "ordinal": 19, + "name": "mode: _", + "type_info": { + "Custom": { + "name": "trigger_mode", + "kind": { + "Enum": [ + "enabled", + "disabled", + "suspended" + ] + } + } + } + } + ], + "parameters": { + "Left": [ + { + "Custom": { + "name": "http_method", + "kind": { + "Enum": [ + "get", + "post", + "put", + "delete", + "patch" + ] + } + } + } + ] + }, + "nullable": [ + false, + false, + false, + false, + true, + false, + false, + false, + false, + false, + true, + false, + false, + true, + false, + false, + true, + true, + true, + false + ] + }, + "hash": "c7a78d3db99e7f709479c9520471eaf40862b464af7113d2632c626e43c35c04" +} diff --git a/backend/migrations/20260825105019_http_trigger_allowed_origins.down.sql b/backend/migrations/20260825105019_http_trigger_allowed_origins.down.sql new file mode 100644 index 0000000000..bee3e6034c --- /dev/null +++ b/backend/migrations/20260825105019_http_trigger_allowed_origins.down.sql @@ -0,0 +1,2 @@ +-- Add down migration script here +ALTER TABLE http_trigger DROP COLUMN allowed_origins; diff --git a/backend/migrations/20260825105019_http_trigger_allowed_origins.up.sql b/backend/migrations/20260825105019_http_trigger_allowed_origins.up.sql new file mode 100644 index 0000000000..444fc46535 --- /dev/null +++ b/backend/migrations/20260825105019_http_trigger_allowed_origins.up.sql @@ -0,0 +1,2 @@ +-- Add up migration script here +ALTER TABLE http_trigger ADD COLUMN allowed_origins TEXT[]; diff --git a/backend/src/main.rs b/backend/src/main.rs index ac1d286d6e..7f2ce3cd82 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -46,7 +46,8 @@ use windmill_common::{ CUSTOM_TAGS_SETTING, DEFAULT_TAGS_PER_WORKSPACE_SETTING, DEFAULT_TAGS_WORKSPACES_SETTING, DISABLE_PASSWORD_LOGIN_SETTING, EMAIL_DOMAIN_SETTING, ENV_SETTINGS, EXPOSE_DEBUG_METRICS_SETTING, EXPOSE_METRICS_SETTING, EXTRA_PIP_INDEX_URL_SETTING, - FORK_WORKSPACE_TAG_APPEND_FORK_SUFFIX_SETTING, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, + FORK_WORKSPACE_TAG_APPEND_FORK_SUFFIX_SETTING, + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, HUB_API_SECRET_SETTING, HUB_BASE_URL_SETTING, INDEXER_SETTING, INSTANCE_EVENTS_WEBHOOK_SETTING, INSTANCE_PYTHON_VERSION_SETTING, JOB_DEFAULT_TIMEOUT_SECS_SETTING, JOB_ISOLATION_SETTING, JWT_SECRET_SETTING, @@ -135,7 +136,8 @@ use crate::monitor::{ reload_bun_install_min_release_age_setting, reload_bunfig_install_scopes_setting, reload_critical_alert_mute_ui_setting, reload_critical_alert_mute_zombie_job_restart_setting, reload_critical_alerts_on_token_expiry_setting, reload_critical_error_channels_setting, - reload_extra_pip_index_url_setting, reload_http_route_workspaced_route_setting, + reload_extra_pip_index_url_setting, reload_http_route_default_allowed_origins_setting, + reload_http_route_workspaced_route_setting, reload_hub_api_secret_setting, reload_hub_base_url_setting, reload_instance_events_webhook_setting, reload_job_default_timeout_setting, reload_job_isolation_setting, reload_jwt_secret_setting, reload_license_key, @@ -2145,6 +2147,11 @@ async fn process_notify_event( tracing::error!(error = %e, "Could not reload app workspaced route setting"); } } + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING => { + if let Err(e) = reload_http_route_default_allowed_origins_setting(db).await { + tracing::error!(error = %e, "Could not reload http route default allowed origins setting"); + } + } HTTP_ROUTE_WORKSPACED_ROUTE_SETTING => { if let Err(e) = reload_http_route_workspaced_route_setting(db).await { tracing::error!(error = %e, "Could not reload http route workspaced route setting"); diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 9f8605c17a..81571bc942 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -106,8 +106,9 @@ use windmill_common::{ use windmill_common::{ client::AuthedClient, global_settings::{ - APP_WORKSPACED_ROUTE_SETTING, HTTP_ROUTE_WORKSPACED_ROUTE, - HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, + parse_allowed_origins_setting, APP_WORKSPACED_ROUTE_SETTING, + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS, HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, + HTTP_ROUTE_WORKSPACED_ROUTE, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, }, queue_metrics::{ QueueSample, QUEUE_COUNT_PREFIX, QUEUE_DELAY_PREFIX, QUEUE_DELAY_SAME_HEAD_SECS, @@ -428,6 +429,18 @@ pub async fn initial_load( pass.setting(APP_WORKSPACED_ROUTE_SETTING, false, |v| async move { apply_app_workspaced_route_setting(v) }); + pass.setting( + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, + false, + |v| async move { + if let Err(e) = apply_http_route_default_allowed_origins_setting(v) { + tracing::error!( + "Error reloading http route default allowed origins: {:?}", + e + ) + } + }, + ); pass.setting( HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, false, @@ -7002,6 +7015,34 @@ pub fn apply_app_workspaced_route_setting(app_workspaced_route: Option error::Result<()> { + let v = + load_value_from_global_settings(conn, HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING).await?; + apply_http_route_default_allowed_origins_setting(v) +} + +pub fn apply_http_route_default_allowed_origins_setting( + value: Option, +) -> error::Result<()> { + // A bad value leaves whatever is already loaded in place rather than + // reverting to no restriction. On the boot path that is still the empty + // default, so what keeps a stored typo from widening CORS instance-wide is + // write-time validation, not this. + let origins = match parse_allowed_origins_setting(value.as_ref()) { + Ok(origins) => origins, + Err(err) => { + tracing::error!( + "Invalid {} setting, keeping the previous value: {err:#}", + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING + ); + return Ok(()); + } + }; + + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS.store(std::sync::Arc::new(origins)); + Ok(()) +} + pub async fn reload_http_route_workspaced_route_setting(conn: &DB) -> error::Result<()> { let v = load_value_from_global_settings(conn, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING).await?; apply_http_route_workspaced_route_setting(conn, v).await diff --git a/backend/summarized_schema.txt b/backend/summarized_schema.txt index d0f9d63647..df3e6ddbf8 100644 --- a/backend/summarized_schema.txt +++ b/backend/summarized_schema.txt @@ -120,7 +120,7 @@ group_: workspace_id(char), name(char), summary(text), extra_perms(jsonb) group_permission_history: id(bigint), workspace_id(char), group_name(char), changed_by(char), changed_at(ts), change_type(char), member_affected(char) FK: (workspace_id, group_name) -> group_(workspace_id, name) healthchecks: id(bigint), check_type(text), healthy(bool), created_at(ts) -http_trigger: path(char), route_path(char), route_path_key(char), script_path(char), is_flow(bool), workspace_id(char), edited_by(char), email(char), edited_at(ts), extra_perms(jsonb), authentication_method(authentication_method), http_method(http_method), static_asset_config(jsonb), is_static_website(bool), workspaced_route(bool), wrap_body(bool), raw_string(bool), authentication_resource_path(char), summary(char), description(text), error_handler_path(char), error_handler_args(jsonb), retry(jsonb), request_type(request_type), mode(trigger_mode), labels(text[]) +http_trigger: path(char), route_path(char), route_path_key(char), script_path(char), is_flow(bool), workspace_id(char), edited_by(char), email(char), edited_at(ts), extra_perms(jsonb), authentication_method(authentication_method), http_method(http_method), static_asset_config(jsonb), is_static_website(bool), workspaced_route(bool), wrap_body(bool), raw_string(bool), allowed_origins(text[]), authentication_resource_path(char), summary(char), description(text), error_handler_path(char), error_handler_args(jsonb), retry(jsonb), request_type(request_type), mode(trigger_mode), labels(text[]) input: id(uuid), workspace_id(char), runnable_id(char), runnable_type(runnable_type), name(text), args(jsonb), created_at(ts), created_by(char), is_public(bool) FK: (workspace_id) -> workspace(id) instance_group: name(char), summary(char), id(char), scim_display_name(char), external_id(char) diff --git a/backend/tests/instance_config.rs b/backend/tests/instance_config.rs index 76102656a5..64d7854173 100644 --- a/backend/tests/instance_config.rs +++ b/backend/tests/instance_config.rs @@ -1442,7 +1442,10 @@ async fn declarative_sync_rejects_an_unusable_webhook_base_url(db: Pool "the other settings in the same apply must not have been written either" ); } + +#[sqlx::test(fixtures("base"))] +async fn declarative_sync_rejects_an_unusable_default_allowed_origins(db: Pool) { + // The declarative writers (the sync-config CLI, the operator's ConfigMap + // sync) do not run the HTTP layer's pre-write hook, so an origin list that + // cannot be parsed would persist here, be dropped at boot, and leave the + // instance with no restriction at all. + clear_settings_and_configs(&db).await; + let before = count_global_settings(&db).await; + + for bad in [ + serde_json::json!([""]), + serde_json::json!(["https://a.example,https://b.example"]), + serde_json::json!("null"), + ] { + let mut desired = BTreeMap::new(); + desired.insert( + "http_route_default_allowed_origins".to_string(), + bad.clone(), + ); + let err = windmill_common::instance_config::sync_global_settings_declarative( + &db, + &BTreeMap::new(), + &desired, + ) + .await + .expect_err(&format!("{bad} must fail the sync")); + assert!( + err.to_string() + .contains("http_route_default_allowed_origins"), + "the error should name the offending setting, got: {err}" + ); + } + + assert_eq!( + count_global_settings(&db).await, + before, + "a rejected sync must not have persisted anything" + ); + + // A usable list still syncs. + let mut desired = BTreeMap::new(); + desired.insert( + "http_route_default_allowed_origins".to_string(), + serde_json::json!(["https://app.example.com"]), + ); + windmill_common::instance_config::sync_global_settings_declarative( + &db, + &BTreeMap::new(), + &desired, + ) + .await + .expect("a valid origin list must sync"); +} diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index 0b975fda98..0fe358da1c 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -58,12 +58,13 @@ use windmill_common::{ AI_CONFIG_SETTING, APP_WORKSPACED_ROUTE_SETTING, AUTOMATE_USERNAME_CREATION_SETTING, CRITICAL_ALERT_MUTE_UI_SETTING, CUSTOM_TAGS_SETTING, DEFAULT_TAGS_WORKSPACES_SETTING, DISABLE_HUB_SETTING, EMAIL_DOMAIN_SETTING, ENV_SETTINGS, - GITHUB_APP_WEBHOOK_BASE_URL_SETTING, HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, - HUB_ACCESSIBLE_URL_SETTING, HUB_BASE_URL_SETTING, INSTANCE_BANNER_SETTING, - MAX_RETENTION_OVERRIDE_WORKSPACES, RETENTION_PERIOD_SECS_OVERRIDES_SETTING, - RUFF_CONFIG_SETTING, UNIQUE_ID_SETTING, WORKSPACE_FAIRNESS_DURATION_SECS_SETTING, - WORKSPACE_FAIRNESS_ENABLED_SETTING, WORKSPACE_FAIRNESS_MAX_PERCENT_SETTING, - WORKSPACE_FAIRNESS_MIN_TOTAL_SETTING, WS_BASE_URL_SETTING, + GITHUB_APP_WEBHOOK_BASE_URL_SETTING, HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, + HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, HUB_ACCESSIBLE_URL_SETTING, HUB_BASE_URL_SETTING, + INSTANCE_BANNER_SETTING, MAX_RETENTION_OVERRIDE_WORKSPACES, + RETENTION_PERIOD_SECS_OVERRIDES_SETTING, RUFF_CONFIG_SETTING, UNIQUE_ID_SETTING, + WORKSPACE_FAIRNESS_DURATION_SECS_SETTING, WORKSPACE_FAIRNESS_ENABLED_SETTING, + WORKSPACE_FAIRNESS_MAX_PERCENT_SETTING, WORKSPACE_FAIRNESS_MIN_TOTAL_SETTING, + WS_BASE_URL_SETTING, }, instance_config::{self, ApplyMode, InstanceConfig}, server::Smtp, @@ -1047,6 +1048,12 @@ async fn run_setting_pre_write_hook( } } } + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING => { + // Rejected at write time rather than at boot: a mistyped origin + // matches no request, so it would silently block the very app it + // names with nothing but a log line to go on. + windmill_common::global_settings::parse_allowed_origins_setting(Some(value))?; + } HTTP_ROUTE_WORKSPACED_ROUTE_SETTING => { let serde_json::Value::Bool(workspaced_route) = value else { return Err(error::Error::BadRequest(format!( @@ -1329,6 +1336,11 @@ pub async fn get_global_setting( && key != EMAIL_DOMAIN_SETTING && key != APP_WORKSPACED_ROUTE_SETTING && key != HTTP_ROUTE_WORKSPACED_ROUTE_SETTING + // The route editor shows the inherited default to whoever is editing a + // trigger, who is usually not a superadmin. Not a secret either: any + // browser discovers the list by reading Access-Control-Allow-Origin off + // a response. + && key != HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING && key != WS_BASE_URL_SETTING && key != INSTANCE_BANNER_SETTING { diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 879e739dc0..3a2b1d715a 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -6056,7 +6056,7 @@ async fn clone_triggers_and_schedules( path, route_path, route_path_key, script_path, is_flow, workspace_id, edited_by, edited_at, extra_perms, authentication_method, http_method, static_asset_config, is_static_website, workspaced_route, wrap_body, - raw_string, authentication_resource_path, summary, description, + raw_string, allowed_origins, authentication_resource_path, summary, description, error_handler_path, error_handler_args, retry, request_type, mode, permissioned_as, labels ) @@ -6064,7 +6064,7 @@ async fn clone_triggers_and_schedules( path, route_path, route_path_key, script_path, is_flow, $1, edited_by, edited_at, extra_perms, authentication_method, http_method, static_asset_config, is_static_website, workspaced_route, wrap_body, - raw_string, authentication_resource_path, summary, description, + raw_string, allowed_origins, authentication_resource_path, summary, description, error_handler_path, error_handler_args, retry, request_type, 'disabled'::TRIGGER_MODE, permissioned_as, labels FROM http_trigger diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 9690a10663..ba4eddd97e 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -30999,6 +30999,14 @@ components: raw_string: type: boolean description: If true, passes the request body as a raw string instead of parsing as JSON + allowed_origins: + type: array + nullable: true + maxItems: 100 + items: + type: string + maxLength: 256 + description: "Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. When set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin the runnable returns via wm_headers. Use ['*'] to opt out of any restriction, including the http_route_default_allowed_origins instance setting. An empty list is not a configuration and resolves exactly as null does. When null, the instance setting applies, or Access-Control-Allow-Origin: * if it is unset. Ignored on a static website, which has no authentication of its own and so hands out public files: restricting which browsers may read them protects nothing while breaking cross-origin webfonts and fetches. A single-file static asset is not exempt, since it can carry an authentication_method." error_handler_path: type: string description: Path to a script to run when the triggered job fails. A bare @@ -31090,6 +31098,14 @@ components: raw_string: type: boolean description: If true, passes the request body as a raw string instead of parsing as JSON + allowed_origins: + type: array + nullable: true + maxItems: 100 + items: + type: string + maxLength: 256 + description: "Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. When set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin the runnable returns via wm_headers. Use ['*'] to opt out of any restriction, including the http_route_default_allowed_origins instance setting. An empty list is not a configuration and resolves exactly as null does. When null, the instance setting applies, or Access-Control-Allow-Origin: * if it is unset. Ignored on a static website, which has no authentication of its own and so hands out public files: restricting which browsers may read them protects nothing while breaking cross-origin webfonts and fetches. A single-file static asset is not exempt, since it can carry an authentication_method." error_handler_path: type: string description: Path to a script to run when the triggered job fails. A bare @@ -31188,6 +31204,14 @@ components: raw_string: type: boolean description: If true, passes the request body as a raw string instead of parsing as JSON + allowed_origins: + type: array + nullable: true + maxItems: 100 + items: + type: string + maxLength: 256 + description: "Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. When set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin the runnable returns via wm_headers. Use ['*'] to opt out of any restriction, including the http_route_default_allowed_origins instance setting. An empty list is not a configuration and resolves exactly as null does. When null, the instance setting applies, or Access-Control-Allow-Origin: * if it is unset. Ignored on a static website, which has no authentication of its own and so hands out public files: restricting which browsers may read them protects nothing while breaking cross-origin webfonts and fetches. A single-file static asset is not exempt, since it can carry an authentication_method." error_handler_path: type: string description: Path to a script to run when the triggered job fails. A bare diff --git a/backend/windmill-api/src/triggers/http/handler.rs b/backend/windmill-api/src/triggers/http/handler.rs index e58aab44a2..387ba56589 100644 --- a/backend/windmill-api/src/triggers/http/handler.rs +++ b/backend/windmill-api/src/triggers/http/handler.rs @@ -1,6 +1,7 @@ use super::{ - http_trigger_args::RawHttpTriggerArgs, refresh_routers, AuthenticationMethod, HttpMethod, - RequestType, TriggerRoute, HTTP_ACCESS_CACHE, HTTP_AUTH_CACHE, HTTP_ROUTERS_CACHE, + effective_allowed_origins, http_trigger_args::RawHttpTriggerArgs, match_origin, + refresh_routers, AuthenticationMethod, HttpMethod, RequestType, TriggerRoute, + HTTP_ACCESS_CACHE, HTTP_AUTH_CACHE, HTTP_ROUTERS_CACHE, }; use crate::{ auth::{AuthCache, OptTokened}, @@ -24,6 +25,7 @@ use std::{collections::HashMap, sync::Arc}; use windmill_common::{ db::UserDB, error::{Error, Result}, + global_settings::HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS, jobs::JobTriggerKind, triggers::{TriggerKind, TriggerMetadata}, utils::{not_found_if_none, StripPath}, @@ -37,12 +39,222 @@ use { windmill_object_store::build_object_store_client, }; +/// Which router a request's CORS decision must be looked up in. +/// +/// A preflight names the method it is asking about in +/// `Access-Control-Request-Method`; the routers are keyed by method, so without +/// that header there is nothing to look up. +fn cors_lookup_method(req: &axum::extract::Request) -> Option { + let method = req.method(); + if method == http::Method::OPTIONS { + req.headers() + .get(http::header::ACCESS_CONTROL_REQUEST_METHOD) + .and_then(|method| method.to_str().ok()) + .and_then(|method| http::Method::try_from(method).ok()) + .as_ref() + .and_then(routable_method) + } else { + routable_method(method) + } +} + +/// The router key a request method maps to. `HEAD` resolves the `GET` route it +/// mirrors, and does so for a preflight naming it too: browsers send +/// `Access-Control-Request-Method: HEAD` when the HEAD carries a non-safelisted +/// header, and answering that preflight from a different route than the request +/// itself resolves is how the two come to disagree. +fn routable_method(method: &http::Method) -> Option { + if method == http::Method::HEAD { + Some(HttpMethod::Get) + } else { + HttpMethod::try_from(method).ok() + } +} + +/// The key to look a request up by, matching what `route_job` resolves it to. +/// +/// `Path` percent-decodes before `get_http_route_trigger` builds its +/// lookup key, so decoding here is what keeps the two agreeing: on the raw path, +/// `/us%65rs` misses the trigger registered at `/users` that goes on to serve the +/// request, and the response would carry the permissive default instead of that +/// trigger's allowlist. +fn cors_lookup_path(raw_path: &str) -> Option { + let decoded = urlencoding::decode(raw_path).ok()?; + // `StripPath::to_path` strips one leading slash and the handler trims + // trailing ones, before a single `/` is prefixed back on. + let stripped = decoded.strip_prefix('/').unwrap_or(&decoded); + Some(format!("/{}", stripped.trim_end_matches('/'))) +} + +/// What the middleware should stamp, decided while the routers guard is held. +/// +/// Deliberately small and owned: the allowlist itself never leaves the guard, +/// so a large one is scanned in place instead of being copied per request onto +/// a path an unauthenticated preflight can reach. +#[derive(Clone)] +enum CorsDecision { + /// No allowlist applies, so the permissive default stands. + Unrestricted, + /// An allowlist applies. `allow_origin` is the value to echo, present only + /// when the request's own `Origin` is on the list. + Restricted { route_method: Option, allow_origin: Option }, + /// The routers could not be read, so nothing is known about this path. + Unavailable, +} + +/// Whether a route actually serves a static website, rather than merely saying +/// it does. +/// +/// `is_static_website` is a caller-set flag that validation ties to nothing: a +/// route can carry it while having no assets configured and a `script_path` +/// that `route_job` runs regardless. Keying the exemption off the flag alone +/// would let one boolean disable a route's allowlist and hand its runnable back +/// the `wm_headers` escape hatch, so the assets have to be there too. +fn serves_a_static_website(trigger: &TriggerRoute) -> bool { + trigger.is_static_website && trigger.static_asset_config.is_some() +} + +/// A static website is never subject to an allowlist, its own included. It has +/// no authentication of its own — the editor does not offer any — so it hands +/// out public files that any non-browser client can already fetch, and +/// restricting which browsers may read them protects nothing while breaking the +/// cross-origin uses that do consult CORS: a webfont, a `crossorigin` asset, a +/// `fetch`. +/// +/// A single-file static asset is not exempt. That one can carry an +/// `authentication_method`, so its content need not be public, and an allowlist +/// is what keeps another origin from reading a response its own credentials +/// would not have obtained. +/// +/// The CORS verdict for a request, published by whoever resolved its trigger. +/// +/// The middleware stamps headers after the handler returns, but only the +/// handler knows which trigger it actually served. Re-deriving that from the +/// routers cache is a second lookup which can disagree with the first when a +/// route is edited, deleted or widened mid-request, and every ordering of the +/// two is wrong in some case. So the verdict travels with the request instead +/// of being worked out twice. +#[derive(Clone, Default)] +struct ResolvedCorsPolicy(std::sync::Arc>); + +impl ResolvedCorsPolicy { + /// Record what the trigger being served allows. Called once, where the + /// route is resolved, so the answer cannot drift from the response. + fn publish(&self, trigger: &TriggerRoute, method: Option, headers: &HeaderMap) { + let decision = if serves_a_static_website(trigger) { + CorsDecision::Unrestricted + } else { + let instance_default = HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS.load(); + match effective_allowed_origins( + trigger.allowed_origins.as_deref(), + instance_default.as_slice(), + ) { + None => CorsDecision::Unrestricted, + Some(allowed_origins) => CorsDecision::Restricted { + route_method: method, + allow_origin: match_origin(allowed_origins, headers.get(http::header::ORIGIN)), + }, + } + }; + let _ = self.0.set(decision); + } + + fn published(&self) -> Option { + self.0.get().cloned() + } +} + +/// Decide the CORS answer from the routers cache, for a request no handler +/// published a verdict for: a preflight, an unknown path, or any failure ahead +/// of the publish — authentication included, which runs after the route itself +/// resolves. +/// +/// Loads the routers when the cache is cold, the way `get_http_route_trigger` +/// does, so a preflight is answered from the same view of the routes as the +/// request that follows it. +async fn resolve_cors_decision( + db: &DB, + http_method: HttpMethod, + requested_path: &str, + origin: Option<&http::HeaderValue>, +) -> CorsDecision { + let routers_cache = HTTP_ROUTERS_CACHE.read().await; + + let routers_cache = if routers_cache.routers.is_empty() { + drop(routers_cache); + match refresh_routers(db, false).await { + Ok((_, routers_cache)) => routers_cache, + Err(err) => { + tracing::error!("Could not load HTTP routers to resolve CORS: {err:#}"); + return CorsDecision::Unavailable; + } + } + } else { + routers_cache + }; + + let Some(router) = routers_cache.routers.get(&http_method) else { + return CorsDecision::Unavailable; + }; + + let route = router.at(requested_path).ok(); + if route + .as_ref() + .is_some_and(|trigger| serves_a_static_website(trigger.value)) + { + return CorsDecision::Unrestricted; + } + let route_allowed_origins = route + .as_ref() + .and_then(|trigger| trigger.value.allowed_origins.as_deref()); + + let instance_default = HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS.load(); + match effective_allowed_origins(route_allowed_origins, instance_default.as_slice()) { + None => CorsDecision::Unrestricted, + Some(allowed_origins) => CorsDecision::Restricted { + route_method: route.map(|_| http_method), + allow_origin: match_origin(allowed_origins, origin), + }, + } +} + async fn conditional_cors_middleware( - req: axum::extract::Request, + Extension(db): Extension, + mut req: axum::extract::Request, next: axum::middleware::Next, ) -> Response { + let origin = req.headers().get(http::header::ORIGIN).cloned(); + // Owned before `next.run` consumes the request. `&Request` is not `Send` + // (`Body` is not `Sync`), so nothing borrowed from it can cross the await. + // The URI is carried rather than the decoded path: cloning it is a refcount + // bump, while decoding allocates, and only the fallback below ever needs it. + let lookup_method = cors_lookup_method(&req); + let uri = req.uri().clone(); + + let resolved = ResolvedCorsPolicy::default(); + req.extensions_mut().insert(resolved.clone()); + let mut response = next.run(req).await; + let decision = match resolved.published() { + // The handler resolved a trigger and said what it served under. That is + // the policy this response was produced with, so nothing else can be + // more authoritative. + Some(decision) => decision, + // No verdict was published: a preflight, an unknown path, or a request + // that failed before reaching the publish, authentication included. No + // runnable produced this body, so reading the cache cannot contradict + // anything. + None => match lookup_method.zip(cors_lookup_path(uri.path())) { + Some((method, path)) => { + resolve_cors_decision(&db, method, &path, origin.as_ref()).await + } + // Not a preflight, not a routable method, or a path that does not + // decode. + None => CorsDecision::Unrestricted, + }, + }; + let headers = response.headers_mut(); // Check existing headers first to determine what not to insert @@ -67,18 +279,65 @@ async fn conditional_cors_middleware( } } - // Insert only the missing headers - if !not_insert_origin { - headers.insert( - http::header::ACCESS_CONTROL_ALLOW_ORIGIN, - http::HeaderValue::from_static("*"), - ); + match &decision { + CorsDecision::Restricted { allow_origin, .. } => { + // A configured allowlist decides, overriding any `wm_headers` value + // the runnable set. The preflight is answered before any code runs, + // so config is the only thing it can consult; letting the response + // widen what the preflight advertised would make the two disagree + // and leave the allowlist bounding nothing. A route escapes a + // stricter instance default — `wm_headers` included — by setting + // its own list to `*`. + match allow_origin { + Some(value) => { + headers.insert(http::header::ACCESS_CONTROL_ALLOW_ORIGIN, value.clone()) + } + // No match: omit the header entirely so the browser blocks the + // read, and drop any value the runnable set. + None => headers.remove(http::header::ACCESS_CONTROL_ALLOW_ORIGIN), + }; + // Appended, not inserted: the answer now depends on the request's + // Origin, and a shared cache that ignores it would hand one + // origin's response to another. + headers.append(http::header::VARY, http::HeaderValue::from_static("origin")); + } + // The routers could not be read, so nothing is known about this path; + // only a preflight or an unresolved request reaches here. Answering a + // preflight permissively would let a disallowed origin go on to invoke + // a runnable whose purpose may be a side effect. + CorsDecision::Unavailable => { + headers.remove(http::header::ACCESS_CONTROL_ALLOW_ORIGIN); + } + CorsDecision::Unrestricted => { + if !not_insert_origin { + headers.insert( + http::header::ACCESS_CONTROL_ALLOW_ORIGIN, + http::HeaderValue::from_static("*"), + ); + } + } } if !not_insert_methods { + // A route accepts exactly one method, so advertising all seven + // overstates it. Only a route under an allowlist gets the narrower + // answer; an unrestricted one advertises the full supported set, since + // narrowing it would say something about a route the response is not + // otherwise willing to disclose. + let restricted_method = match &decision { + CorsDecision::Restricted { route_method, .. } => *route_method, + _ => None, + }; headers.insert( http::header::ACCESS_CONTROL_ALLOW_METHODS, - http::HeaderValue::from_static("GET, POST, PUT, DELETE, PATCH, HEAD, OPTIONS"), + http::HeaderValue::from_static(match restricted_method { + Some(HttpMethod::Get) => "GET, OPTIONS", + Some(HttpMethod::Post) => "POST, OPTIONS", + Some(HttpMethod::Put) => "PUT, OPTIONS", + Some(HttpMethod::Delete) => "DELETE, OPTIONS", + Some(HttpMethod::Patch) => "PATCH, OPTIONS", + None => "GET, POST, PUT, DELETE, PATCH, HEAD, OPTIONS", + }), ); } @@ -237,6 +496,7 @@ async fn route_job( Extension(db): Extension, Extension(user_db): Extension, Extension(auth_cache): Extension>, + Extension(cors_policy): Extension, OptTokened { token }: OptTokened, Path(route_path): Path, headers: HeaderMap, @@ -255,6 +515,10 @@ async fn route_job( .await .map_err(|e| e.into_response())?; + // Publish before anything else can fail: the CORS middleware stamps this + // response either way, and it must reflect the trigger actually served. + cors_policy.publish(&trigger, routable_method(&args.0.metadata.method), &headers); + if trigger.script_path.is_empty() && trigger.static_asset_config.is_none() { return Err(Error::NotFound(format!( "Runnable path of HTTP route at path: {}", diff --git a/backend/windmill-common/src/global_settings.rs b/backend/windmill-common/src/global_settings.rs index 1d590e38bf..c0ed63cd53 100644 --- a/backend/windmill-common/src/global_settings.rs +++ b/backend/windmill-common/src/global_settings.rs @@ -118,6 +118,7 @@ pub const OTEL_TRACING_PROXY_SETTING: &str = "otel_tracing_proxy"; pub const OTEL_TRACES_RETENTION_SECS_SETTING: &str = "otel_traces_retention_secs"; pub const APP_WORKSPACED_ROUTE_SETTING: &str = "app_workspaced_route"; pub const HTTP_ROUTE_WORKSPACED_ROUTE_SETTING: &str = "http_route_workspaced_route"; +pub const HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING: &str = "http_route_default_allowed_origins"; pub const SECRET_BACKEND_SETTING: &str = "secret_backend"; pub const MIN_KEEP_ALIVE_VERSION_SETTING: &str = "min_keep_alive_version"; pub const GITHUB_ENTERPRISE_APP_SETTING: &str = "github_enterprise_app"; @@ -362,6 +363,125 @@ use std::sync::atomic::AtomicBool; lazy_static::lazy_static! { pub static ref HTTP_ROUTE_WORKSPACED_ROUTE: AtomicBool = AtomicBool::new(false); pub static ref DISABLE_PASSWORD_LOGIN: AtomicBool = AtomicBool::new(false); + /// Origins HTTP routes allow cross-origin when they configure none of their + /// own. Empty means unset, which keeps the historical `*`. + pub static ref HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS: arc_swap::ArcSwap> = + arc_swap::ArcSwap::from_pointee(vec![]); +} + +/// Whether an allowlist places no restriction at all. +/// +/// `*` is the explicit "open on purpose" entry, and a route carrying it behaves +/// exactly as an unconfigured one: it is how a route opts out of a stricter +/// instance default, including back into the `wm_headers` escape hatch. +pub fn allows_any_origin(allowed_origins: &[String]) -> bool { + allowed_origins.iter().any(|allowed| allowed == "*") +} + +/// An allowlist is scanned on every request to a restricted route, including +/// the unauthenticated preflight, so its size is a request cost anyone can +/// trigger. +pub const MAX_ALLOWED_ORIGINS: usize = 100; +pub const MAX_ALLOWED_ORIGIN_LEN: usize = 256; + +/// Reject allowlist entries that cannot be compared, stored, or safely allowed. +/// +/// The stored string is only ever an operand: `match_origin` echoes the +/// request's own `Origin` back, never this value, so a malformed entry matches +/// nothing and fails closed. Shapes that merely cannot match are the editor's +/// business to warn about, not this function's to refuse. What is left are the +/// three cases where permissiveness costs something: `null` is what every +/// sandboxed iframe sends, so allowing it would admit any page that can open +/// one; a comma cannot survive the editor's comma-separated field, which would +/// silently split one entry into two and widen the list; and an unbounded list +/// makes every preflight pay for it. +pub fn validate_allowed_origins(allowed_origins: &[String]) -> crate::error::Result<()> { + if allowed_origins.len() > MAX_ALLOWED_ORIGINS { + return Err(crate::error::Error::BadRequest(format!( + "At most {} allowed origins, got {}.", + MAX_ALLOWED_ORIGINS, + allowed_origins.len() + ))); + } + + for origin in allowed_origins { + if origin == "*" { + continue; + } + + let invalid = |reason: &str| { + crate::error::Error::BadRequest(format!( + "Invalid allowed origin '{}': {}.", + origin, reason + )) + }; + + if origin.is_empty() { + return Err(invalid("must not be empty")); + } + if origin.len() > MAX_ALLOWED_ORIGIN_LEN { + return Err(invalid("is longer than any origin a browser sends")); + } + // The editor edits the whole list as one comma-separated field, so an + // entry carrying a comma comes back as two and widens the list. + if origin.contains(',') { + return Err(invalid("must not contain a comma, which separates entries")); + } + if origin.eq_ignore_ascii_case("null") { + return Err(invalid( + "'null' is what a sandboxed iframe sends, so allowing it would allow any page that can open one", + )); + } + // An Origin header is always visible ASCII, so a value outside it can + // never be the string this is compared against. + if !origin.chars().all(|c| c.is_ascii_graphic()) { + return Err(invalid( + "must contain only visible ASCII, with no whitespace", + )); + } + } + + Ok(()) +} + +/// Read [`HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING`] from its stored value. +/// +/// Accepts the comma-separated string the settings UI writes, or a JSON array +/// for anything setting it through the API directly. +pub fn parse_allowed_origins_setting( + value: Option<&serde_json::Value>, +) -> crate::error::Result> { + let origins = match value { + None | Some(serde_json::Value::Null) => vec![], + Some(serde_json::Value::String(raw)) => raw + .split(',') + .map(|origin| origin.trim().to_string()) + .filter(|origin| !origin.is_empty()) + .collect(), + Some(serde_json::Value::Array(entries)) => entries + .iter() + .map(|entry| match entry { + serde_json::Value::String(origin) => Ok(origin.trim().to_string()), + _ => Err(crate::error::Error::BadRequest(format!( + "{} entries must be strings", + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING + ))), + }) + // Not filtered for empties, unlike the string form: there a + // trailing separator naturally yields an empty token, whereas an + // empty array entry is something the caller wrote and validation + // should reject rather than silently drop. + .collect::>>()?, + Some(_) => { + return Err(crate::error::Error::BadRequest(format!( + "{} expected to be a comma-separated string or an array of strings", + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING + ))) + } + }; + + validate_allowed_origins(&origins)?; + Ok(origins) } pub const ENV_SETTINGS: &[&str] = &[ diff --git a/backend/windmill-common/src/instance_config.rs b/backend/windmill-common/src/instance_config.rs index e28bf139cf..1dd3396809 100644 --- a/backend/windmill-common/src/instance_config.rs +++ b/backend/windmill-common/src/instance_config.rs @@ -1288,8 +1288,9 @@ pub fn diff_worker_configs( ConfigsDiff { upserts, deletes } } -/// Declaratively replace the global settings, rejecting a `github_app_webhook_base_url` -/// the API would reject. +/// Declaratively replace the global settings, rejecting a +/// `github_app_webhook_base_url` or `http_route_default_allowed_origins` the +/// API would reject. /// /// Every declarative writer (the `sync-config` CLI, the Kubernetes operator's /// ConfigMap sync) MUST go through this rather than calling @@ -1348,6 +1349,13 @@ pub async fn sync_global_settings_declarative( .map_err(|e| anyhow::anyhow!("{banner_key}: {e}"))?, } + // An origin list that cannot be parsed is dropped at boot, leaving the + // empty default — which is no restriction at all. Rejecting it here is what + // keeps a typo in a ConfigMap from silently widening CORS instance-wide. + let origins_key = crate::global_settings::HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING; + crate::global_settings::parse_allowed_origins_setting(desired.get(origins_key)) + .map_err(|e| anyhow::anyhow!("{origins_key}: {e}"))?; + let diff = diff_global_settings(current, desired, ApplyMode::Replace); apply_settings_diff(db, &diff).await?; diff --git a/backend/windmill-trigger-http/src/handler.rs b/backend/windmill-trigger-http/src/handler.rs index 68986bb6f9..01ddfd4a81 100644 --- a/backend/windmill-trigger-http/src/handler.rs +++ b/backend/windmill-trigger-http/src/handler.rs @@ -9,7 +9,7 @@ use sqlx::PgConnection; use std::collections::HashSet; use windmill_api_auth::{check_scopes, ApiAuthed}; use windmill_audit::{audit_oss::audit_log, ActionKind}; -use windmill_common::global_settings::HTTP_ROUTE_WORKSPACED_ROUTE; +use windmill_common::global_settings::{validate_allowed_origins, HTTP_ROUTE_WORKSPACED_ROUTE}; use windmill_common::{ db::UserDB, error::{Error, Result}, @@ -189,6 +189,7 @@ pub async fn insert_new_trigger_into_db( authentication_resource_path, wrap_body, raw_string, + allowed_origins, script_path, summary, description, @@ -207,7 +208,7 @@ pub async fn insert_new_trigger_into_db( retry ) VALUES ( - $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, now(), $20, $21, $22, $23 + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, now(), $21, $22, $23, $24 ) "#, w_id, @@ -218,6 +219,7 @@ pub async fn insert_new_trigger_into_db( trigger.config.authentication_resource_path, trigger.config.wrap_body.unwrap_or(false), trigger.config.raw_string.unwrap_or(false), + trigger.config.allowed_origins.as_deref(), trigger.base.script_path, trigger.config.summary, trigger.config.description, @@ -444,6 +446,7 @@ impl TriggerCrud for HttpTrigger { "workspaced_route", "wrap_body", "raw_string", + "allowed_origins", ]; fn get_deployed_object(path: String, parent_path: Option) -> DeployedObject { @@ -474,6 +477,8 @@ impl TriggerCrud for HttpTrigger { validate_authentication_method(new.authentication_method, new.raw_string)?; + validate_allowed_origins(new.allowed_origins.as_deref().unwrap_or_default())?; + Ok(()) } @@ -492,6 +497,8 @@ impl TriggerCrud for HttpTrigger { validate_authentication_method(edit.authentication_method, edit.raw_string)?; + validate_allowed_origins(edit.allowed_origins.as_deref().unwrap_or_default())?; + Ok(()) } @@ -554,33 +561,35 @@ impl TriggerCrud for HttpTrigger { workspaced_route = $3, wrap_body = $4, raw_string = $5, - authentication_resource_path = $6, - script_path = $7, - path = $8, - is_flow = $9, - mode = $10, - http_method = $11, - static_asset_config = $12, - edited_by = $13, - permissioned_as = $14, - request_type = $15, - authentication_method = $16, - summary = $17, - description = $18, + allowed_origins = $6, + authentication_resource_path = $7, + script_path = $8, + path = $9, + is_flow = $10, + mode = $11, + http_method = $12, + static_asset_config = $13, + edited_by = $14, + permissioned_as = $15, + request_type = $16, + authentication_method = $17, + summary = $18, + description = $19, edited_at = now(), - is_static_website = $19, - error_handler_path = $20, - error_handler_args = $21, - retry = $22 + is_static_website = $20, + error_handler_path = $21, + error_handler_args = $22, + retry = $23 WHERE - workspace_id = $23 AND - path = $24 + workspace_id = $24 AND + path = $25 "#, route_path, &route_path_key, Some(effective_workspaced), trigger.config.wrap_body.unwrap_or(false), trigger.config.raw_string.unwrap_or(false), + trigger.config.allowed_origins.as_deref(), trigger.config.authentication_resource_path, trigger.base.script_path, trigger.base.path, @@ -613,30 +622,32 @@ impl TriggerCrud for HttpTrigger { SET wrap_body = $1, raw_string = $2, - authentication_resource_path = $3, - script_path = $4, - path = $5, - is_flow = $6, - mode = $7, - http_method = $8, - static_asset_config = $9, - edited_by = $10, - permissioned_as = $11, - request_type = $12, - authentication_method = $13, - summary = $14, - description = $15, + allowed_origins = $3, + authentication_resource_path = $4, + script_path = $5, + path = $6, + is_flow = $7, + mode = $8, + http_method = $9, + static_asset_config = $10, + edited_by = $11, + permissioned_as = $12, + request_type = $13, + authentication_method = $14, + summary = $15, + description = $16, edited_at = now(), - is_static_website = $16, - error_handler_path = $17, - error_handler_args = $18, - retry = $19 + is_static_website = $17, + error_handler_path = $18, + error_handler_args = $19, + retry = $20 WHERE - workspace_id = $20 AND - path = $21 + workspace_id = $21 AND + path = $22 "#, trigger.config.wrap_body.unwrap_or(false), trigger.config.raw_string.unwrap_or(false), + trigger.config.allowed_origins.as_deref(), trigger.config.authentication_resource_path, trigger.base.script_path, trigger.base.path, diff --git a/backend/windmill-trigger-http/src/lib.rs b/backend/windmill-trigger-http/src/lib.rs index b78276c28d..40bab102a5 100644 --- a/backend/windmill-trigger-http/src/lib.rs +++ b/backend/windmill-trigger-http/src/lib.rs @@ -8,7 +8,7 @@ use tokio::sync::{RwLock, RwLockReadGuard}; use windmill_common::{ error::{Error, Result}, flows::Retry, - global_settings::HTTP_ROUTE_WORKSPACED_ROUTE, + global_settings::{allows_any_origin, HTTP_ROUTE_WORKSPACED_ROUTE}, utils::ExpiringCacheEntry, worker::CLOUD_HOSTED, DB, @@ -51,6 +51,7 @@ pub struct TriggerRoute { pub workspaced_route: bool, pub wrap_body: bool, pub raw_string: bool, + pub allowed_origins: Option>, pub error_handler_path: Option, pub error_handler_args: Option>>, pub retry: Option>, @@ -127,6 +128,7 @@ pub struct HttpConfig { pub workspaced_route: bool, pub wrap_body: bool, pub raw_string: bool, + pub allowed_origins: Option>, } #[derive(Debug, Clone, Serialize)] @@ -144,6 +146,7 @@ pub struct HttpConfigRequest { pub workspaced_route: Option, pub wrap_body: Option, pub raw_string: Option, + pub allowed_origins: Option>, } #[derive(Deserialize)] @@ -162,6 +165,7 @@ struct HttpConfigRequestHelper { workspaced_route: Option, wrap_body: Option, raw_string: Option, + allowed_origins: Option>, } impl<'de> Deserialize<'de> for HttpConfigRequest { @@ -197,6 +201,7 @@ impl<'de> Deserialize<'de> for HttpConfigRequest { workspaced_route: helper.workspaced_route, wrap_body: helper.wrap_body, raw_string: helper.raw_string, + allowed_origins: helper.allowed_origins, }) } } @@ -216,6 +221,50 @@ pub struct RouteExists { pub workspaced_route: Option, } +/// The allowlist that governs a route: its own when it has one, otherwise the +/// instance-wide default. `None` means nothing is configured at either level, so +/// the route keeps the historical permissive behaviour. +/// +/// A list containing `*` is treated as no restriction, which is how a route opts +/// out of a stricter instance default. +pub fn effective_allowed_origins<'a>( + route_allowed_origins: Option<&'a [String]>, + instance_default: &'a [String], +) -> Option<&'a [String]> { + // An empty list is not a configuration. It reads exactly as never having set + // one, so such a route still inherits the instance default rather than + // skipping it, which is what would make `[]` more permissive than `NULL`. + match route_allowed_origins.filter(|list| !list.is_empty()) { + // `*` is the opt-out, including out of a stricter instance default. + Some(list) if allows_any_origin(list) => None, + Some(list) => Some(list), + None => (!instance_default.is_empty() && !allows_any_origin(instance_default)) + .then_some(instance_default), + } +} + +/// Resolve the `Access-Control-Allow-Origin` value for a request, or `None` to +/// omit the header so the browser blocks the read. +/// +/// The request's `Origin` is echoed back only on a match against the allowlist. +/// Reflecting it unchecked is the classic way this feature turns into no +/// restriction at all. +/// +/// The comparison ignores ASCII case because a browser lowercases the scheme and +/// host it sends, so a configured `https://App.Example.com` would otherwise name +/// a real origin and still match nothing. +pub fn match_origin( + allowed_origins: &[String], + origin: Option<&http::HeaderValue>, +) -> Option { + let origin = origin?; + let origin_str = origin.to_str().ok()?; + allowed_origins + .iter() + .any(|allowed| allowed.eq_ignore_ascii_case(origin_str)) + .then(|| origin.clone()) +} + pub fn validate_authentication_method( authentication_method: AuthenticationMethod, raw_string: Option, @@ -276,6 +325,7 @@ pub async fn refresh_routers( static_asset_config AS "static_asset_config: _", wrap_body, raw_string, + allowed_origins, workspaced_route, is_static_website, error_handler_path, @@ -384,6 +434,10 @@ pub struct HttpTrigger; #[cfg(test)] mod tests { use super::*; + // Not used by the lib itself, only exercised here. + use windmill_common::global_settings::{ + validate_allowed_origins, MAX_ALLOWED_ORIGINS, MAX_ALLOWED_ORIGIN_LEN, + }; #[test] fn test_request_type_backward_compatibility() { @@ -578,6 +632,168 @@ mod tests { assert!(validate_authentication_method(AuthenticationMethod::Signature, None).is_ok()); } + // --- CORS allowed origins --- + + fn origin(value: &str) -> http::HeaderValue { + http::HeaderValue::from_str(value).unwrap() + } + + #[test] + fn test_match_origin_exact_match_echoes_request_origin() { + let allowed = vec!["https://a.com".to_string(), "https://b.com".to_string()]; + assert_eq!( + match_origin(&allowed, Some(&origin("https://b.com"))), + Some(origin("https://b.com")) + ); + } + + #[test] + fn test_match_origin_ignores_case() { + let allowed = vec!["https://App.Example.com".to_string()]; + assert_eq!( + match_origin(&allowed, Some(&origin("https://app.example.com"))), + Some(origin("https://app.example.com")) + ); + } + + #[test] + fn test_match_origin_no_match_omits_header() { + let allowed = vec!["https://a.com".to_string()]; + assert_eq!( + match_origin(&allowed, Some(&origin("https://evil.com"))), + None + ); + // A prefix of an allowed origin must not match: https://a.com.evil.com + // is a different site entirely. + assert_eq!( + match_origin(&allowed, Some(&origin("https://a.com.evil.com"))), + None + ); + } + + #[test] + fn test_wildcard_entry_means_unrestricted() { + // `*` is handled before matching: it means "no restriction", which is + // how a route opts out of a stricter instance default. + assert!(allows_any_origin(&["*".to_string()])); + assert!(allows_any_origin(&[ + "https://a.com".to_string(), + "*".to_string() + ])); + assert!(!allows_any_origin(&["https://a.com".to_string()])); + assert_eq!( + effective_allowed_origins(Some(&["*".to_string()]), &[]), + None + ); + } + + #[test] + fn test_effective_allowed_origins_prefers_the_route() { + let route = ["https://a.com".to_string()]; + let default = ["https://default.com".to_string()]; + assert_eq!( + effective_allowed_origins(Some(&route), &default), + Some(&route[..]) + ); + // No route list: the instance default applies. + assert_eq!( + effective_allowed_origins(None, &default), + Some(&default[..]) + ); + // No route list and no instance default: nothing is restricted, so the + // historical permissive behaviour is kept. + assert_eq!(effective_allowed_origins(None, &[]), None); + // A route opting out with `*` escapes a stricter instance default. + assert_eq!( + effective_allowed_origins(Some(&["*".to_string()]), &default), + None + ); + // An empty route list is not a configuration: it resolves exactly as + // `NULL` does, so it inherits the instance default rather than skipping + // it and becoming more permissive than an unset one. + assert_eq!( + effective_allowed_origins(Some(&[]), &default), + Some(&default[..]) + ); + assert_eq!(effective_allowed_origins(Some(&[]), &[]), None); + } + + #[test] + fn test_match_origin_missing_origin_header_omits_header() { + let allowed = vec!["https://a.com".to_string()]; + assert_eq!(match_origin(&allowed, None), None); + } + + #[test] + fn test_validate_allowed_origins_accepts_anything_comparable() { + // A shape that cannot match simply matches nothing, so it is the + // editor's job to warn and not this one's to refuse. What is refused is + // narrower: `null`, values that are not header-comparable, entries that + // cannot round-trip the editor's comma-separated field, and lists past + // the size a request can afford to scan. + let allowed = vec![ + "https://app.example.com".to_string(), + "http://localhost:3000".to_string(), + "http://[::1]:8080".to_string(), + "chrome-extension://mhjfbmdgcfjbbpaeojofohoefgiehjai".to_string(), + // Never matches, but that is the caller's problem, not an error. + "https://app.example.com/".to_string(), + "https://app.example.com:99999".to_string(), + "not-an-origin".to_string(), + "*".to_string(), + ]; + assert!(validate_allowed_origins(&allowed).is_ok()); + assert!(validate_allowed_origins(&[]).is_ok()); + } + + #[test] + fn test_parse_allowed_origins_setting_rejects_empty_array_entries() { + use windmill_common::global_settings::parse_allowed_origins_setting; + // A trailing separator in the string form is a typing artifact and is + // dropped; an empty array entry is something the caller wrote, so it + // must reach validation rather than be filtered away into an empty + // (and therefore unrestricted) default. + assert!(parse_allowed_origins_setting(Some(&serde_json::json!("https://a.com,"))).is_ok()); + assert!(parse_allowed_origins_setting(Some(&serde_json::json!([""]))).is_err()); + assert!( + parse_allowed_origins_setting(Some(&serde_json::json!(["https://a.com", ""]))).is_err() + ); + } + + #[test] + fn test_validate_allowed_origins_bounds_the_list() { + // An allowlist is scanned on every request to a restricted route, the + // unauthenticated preflight included, so its size is a cost anyone can + // trigger. + let too_many = vec!["https://a.com".to_string(); MAX_ALLOWED_ORIGINS + 1]; + assert!(validate_allowed_origins(&too_many).is_err()); + assert!(validate_allowed_origins(&too_many[..MAX_ALLOWED_ORIGINS]).is_ok()); + let too_long = format!("https://{}.com", "a".repeat(MAX_ALLOWED_ORIGIN_LEN)); + assert!(validate_allowed_origins(&[too_long]).is_err()); + } + + #[test] + fn test_validate_allowed_origins_rejects_null_and_uncomparable() { + for invalid in [ + // Every sandboxed iframe sends `Origin: null`, so allowing it would + // grant access to any page that can open one. + "null", + "NULL", // Cannot be the string an Origin header is compared against. + "https://a b.com", + "https://app.example.com ", + "https://exämple.com", + // The editor edits the list as one comma-separated field, so an + // entry carrying a comma would come back as two and widen the list. + "https://a.com,https://b.com", + "", + ] { + assert!( + validate_allowed_origins(&[invalid.to_string()]).is_err(), + "expected {invalid} to be rejected" + ); + } + } + // --- Route path regex --- #[test] diff --git a/cli/src/guidance/skills.gen.ts b/cli/src/guidance/skills.gen.ts index 0c7b91887d..b8b71b1de9 100644 --- a/cli/src/guidance/skills.gen.ts +++ b/cli/src/guidance/skills.gen.ts @@ -8525,6 +8525,21 @@ properties: type: boolean description: If true, passes the request body as a raw string instead of parsing as JSON + allowed_origins: + type: array + items: + type: string + description: 'Origins allowed to call this route cross-origin, matched against + the request''s Origin header (ignoring case) and echoed back on a match. When + set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin + the runnable returns via wm_headers. Use [''*''] to opt out of any restriction, + including the http_route_default_allowed_origins instance setting. An empty + list is not a configuration and resolves exactly as null does. When null, the + instance setting applies, or Access-Control-Allow-Origin: * if it is unset. + Ignored on a static website, which has no authentication of its own and so hands + out public files: restricting which browsers may read them protects nothing + while breaking cross-origin webfonts and fetches. A single-file static asset + is not exempt, since it can carry an authentication_method.' error_handler_path: type: string description: Path to a script to run when the triggered job fails. A bare path, diff --git a/frontend/src/lib/components/copilot/chat/workspaceToolsZod.gen.ts b/frontend/src/lib/components/copilot/chat/workspaceToolsZod.gen.ts index 42773873c2..fc019ee983 100644 --- a/frontend/src/lib/components/copilot/chat/workspaceToolsZod.gen.ts +++ b/frontend/src/lib/components/copilot/chat/workspaceToolsZod.gen.ts @@ -69,6 +69,7 @@ export const httpTriggerRequestSchema = z.object({ "wrap_body": z.boolean().describe("If true, wraps the request body in a 'body' parameter").optional(), "mode": z.enum(["enabled", "disabled", "suspended"]).describe("job trigger mode").optional(), "raw_string": z.boolean().describe("If true, passes the request body as a raw string instead of parsing as JSON").optional(), + "allowed_origins": z.array(z.string()).describe("Origins allowed to call this route cross-origin, matched against the request's Origin header (ignoring case) and echoed back on a match. When set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin the runnable returns via wm_headers. Use ['*'] to opt out of any restriction, including the http_route_default_allowed_origins instance setting. An empty list is not a configuration and resolves exactly as null does. When null, the instance setting applies, or Access-Control-Allow-Origin: * if it is unset. Ignored on a static website, which has no authentication of its own and so hands out public files: restricting which browsers may read them protects nothing while breaking cross-origin webfonts and fetches. A single-file static asset is not exempt, since it can carry an authentication_method.").nullable().optional(), "error_handler_path": z.string().describe("Path to a script to run when the triggered job fails. A bare path, without the script/ or flow/ prefix a schedule error handler takes; it cannot be a flow.").optional(), "error_handler_args": z.record(z.string(), z.any()).describe("Arguments to pass to the error handler").optional(), "retry": z.object({ diff --git a/frontend/src/lib/components/instanceSettings.ts b/frontend/src/lib/components/instanceSettings.ts index 0548d3ded5..0677648978 100644 --- a/frontend/src/lib/components/instanceSettings.ts +++ b/frontend/src/lib/components/instanceSettings.ts @@ -1,4 +1,5 @@ import type { ButtonType } from './common/button/model' +import { allowedOriginsSettingError } from './triggers/http/utils' import { z } from 'zod' import { instanceBannerFormError } from './instanceBanner' import { writable } from 'svelte/store' @@ -281,6 +282,22 @@ export const settings: Record = { ee_only: '', hideInQuickSetup: true }, + { + label: 'HTTP route default allowed origins', + description: + 'Origins that HTTP routes allow to call them from a browser when the route sets none of its own. A route overrides this with its own list, and opts out entirely by setting its allowed origins to *. Leave unset for no instance-wide default, so every route is callable from any origin unless it restricts itself.', + key: 'http_route_default_allowed_origins', + fieldType: 'text', + placeholder: 'https://app.example.com, https://admin.example.com', + storage: 'setting', + error: + 'Each origin must be visible ASCII with no comma, and there can be at most 100 of them. null is not allowed, since every sandboxed iframe sends it.', + // The same check the API applies, so a value it would refuse cannot be + // saved here and then silently drop to no restriction at the next boot. + isValid: (value: unknown) => allowedOriginsSettingError(value) === undefined, + ee_only: '', + hideInQuickSetup: true + }, { label: 'Audit log retention (days)', key: 'audit_log_retention_days', diff --git a/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte b/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte new file mode 100644 index 0000000000..50b09a108d --- /dev/null +++ b/frontend/src/lib/components/triggers/http/RouteCorsOption.svelte @@ -0,0 +1,132 @@ + + + + diff --git a/frontend/src/lib/components/triggers/http/RouteEditorInner.svelte b/frontend/src/lib/components/triggers/http/RouteEditorInner.svelte index 5a32120ca2..eb046f380e 100644 --- a/frontend/src/lib/components/triggers/http/RouteEditorInner.svelte +++ b/frontend/src/lib/components/triggers/http/RouteEditorInner.svelte @@ -12,6 +12,7 @@ import ScriptPicker from '$lib/components/ScriptPicker.svelte' import { HttpTriggerService, + SettingService, VariableService, type AuthenticationMethod, type ErrorHandler, @@ -46,9 +47,18 @@ import ResourcePicker from '$lib/components/ResourcePicker.svelte' import ItemPicker from '../../ItemPicker.svelte' import { Popover } from '$lib/components/meltComponents' - import { HUB_SCRIPT_ID, saveHttpRouteFromCfg, SECRET_KEY_PATH } from './utils' + import { + HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING, + HUB_SCRIPT_ID, + allowedOriginsError, + isOriginRestricted, + parseAllowedOriginsSetting, + saveHttpRouteFromCfg, + SECRET_KEY_PATH + } from './utils' import { HubFlow } from '$lib/hub' import RouteBodyTransformerOption from './RouteBodyTransformerOption.svelte' + import RouteCorsOption from './RouteCorsOption.svelte' import TestingBadge from '../testingBadge.svelte' import TriggerEditorToolbar from '../TriggerEditorToolbar.svelte' import PermissionedAsLine from '../PermissionedAsLine.svelte' @@ -110,6 +120,27 @@ let workspaced_route = $state(false) let raw_string = $state(false) let wrap_body = $state(false) + let allowed_origins = $state(undefined) + // Derived from the stored list, not reported by the field: the field only + // exists on the request-options tab, so an error owned by it would keep Save + // disabled from a screen that cannot show why. An empty list is not an error + // either, since it resolves as an unset one, so only what the API refuses + // blocks the save. + const originsError = $derived(allowedOriginsError(allowed_origins)) + // Fetched once here rather than in RouteCorsOption so the Advanced badge can + // show an inherited restriction without the section being expanded. + let instanceDefaultOrigins = $state([]) + async function loadInstanceDefaultOrigins() { + try { + const setting = await SettingService.getGlobal({ + key: HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING + }) + instanceDefaultOrigins = parseAllowedOriginsSetting(setting) + } catch { + instanceDefaultOrigins = [] + } + } + loadInstanceDefaultOrigins() let drawerLoading = $state(true) let showLoader = $state(false) let authentication_resource_path = $state('') @@ -160,6 +191,7 @@ !can_write || pathError != '' || !isValid || + originsError != undefined || (!static_asset_config && emptyString(script_path)) || !hasChanged ) @@ -295,6 +327,7 @@ signature_options_type = defaultValues?.signature_options_type ?? 'custom_signature' raw_string = defaultValues?.raw_string ?? false wrap_body = defaultValues?.wrap_body ?? false + allowed_origins = defaultValues?.allowed_origins ?? undefined summary = defaultValues?.summary ?? '' routeDescription = defaultValues?.description ?? '' error_handler_path = defaultValues?.error_handler_path ?? undefined @@ -323,6 +356,7 @@ workspaced_route = cfg?.workspaced_route ?? false wrap_body = cfg?.wrap_body ?? false raw_string = cfg?.raw_string ?? false + allowed_origins = cfg?.allowed_origins ?? undefined summary = cfg?.summary ?? '' mode = cfg?.mode ?? 'enabled' routeDescription = cfg?.description ?? '' @@ -423,6 +457,7 @@ mode, wrap_body, raw_string, + allowed_origins, authentication_resource_path, authentication_method: auth_method, static_asset_config, @@ -758,7 +793,11 @@ extraBadges={[ { name: 'Async', active: request_type === 'async' }, { name: 'SSE', active: request_type === 'sync_sse' }, - { name: 'Authentication', active: authentication_method !== 'none' } + { name: 'Authentication', active: authentication_method !== 'none' }, + { + name: 'CORS', + active: isOriginRestricted(allowed_origins, instanceDefaultOrigins) + } ]} /> {/snippet} @@ -961,6 +1000,14 @@ disabled={!can_write} {testingBadge} /> + + {:else} origin.trim()) + .filter((origin) => origin !== '') +} + +/** + * Entries the API refuses, mirroring `validate_allowed_origins`. + * + * Deliberately short: a stored origin is only ever compared against the + * request's `Origin`, so a shape that cannot match is dead config rather than a + * risk. `null` is the exception, since it is what every sandboxed iframe sends. + */ +export function allowedOriginRejection(origin: string): string | undefined { + // Same order as `validate_allowed_origins`, so the same entry draws the same + // message on both sides rather than only the same verdict. + if (origin === '*') return undefined + if (origin === '') return 'An origin must not be empty' + if (origin.length > MAX_ALLOWED_ORIGIN_LEN) + return `'${origin.slice(0, 40)}…' is longer than any origin a browser sends` + if (origin.includes(',')) + return `'${origin}' must not contain a comma, which separates entries` + if (origin.toLowerCase() === 'null') + return `'null' is what a sandboxed iframe sends, so it would allow any page that can open one` + if (!/^[\x21-\x7e]+$/.test(origin)) + return `'${origin}' must contain only visible ASCII, with no whitespace` + return undefined +} + +/** Kept in step with `MAX_ALLOWED_ORIGIN{,S}` in windmill-common. */ +export const MAX_ALLOWED_ORIGINS = 100 +export const MAX_ALLOWED_ORIGIN_LEN = 256 + +/** + * The first entry the API would refuse, if any. Derived from the stored list + * rather than the field, so it stays correct while the editor is on another tab + * and the field is not mounted. An empty list is not an error: it resolves as an + * unset one, so there is nothing in it to refuse. + * + * A comma-bearing entry does reach this, through the settings path where a list + * can be given as an array. It cannot arrive from the origins field, which + * splits on commas before this ever sees it. + */ +export function allowedOriginsError(allowed_origins: string[] | undefined): string | undefined { + if (allowed_origins !== undefined && allowed_origins.length > MAX_ALLOWED_ORIGINS) + return `At most ${MAX_ALLOWED_ORIGINS} origins, got ${allowed_origins.length}` + return allowed_origins?.map(allowedOriginRejection).find((message) => message !== undefined) +} + +/** + * Shapes that save fine but can never equal an `Origin` header, so the route + * would read as configured while allowing nothing. + * + * Advisory only. What a browser sends is the caller's to know, so this points + * at the usual slips rather than deciding which origins are legitimate. + */ +export function allowedOriginWarning(origin: string): string | undefined { + if (origin === '*' || allowedOriginRejection(origin) !== undefined) return undefined + const separator = origin.indexOf('://') + if (separator <= 0) return `'${origin}' has no scheme, such as https://` + const rest = origin.slice(separator + 3) + if (rest === '') return `'${origin}' has no host` + if (/[/?#]/.test(rest)) + return `'${origin}' should be scheme://host[:port], with no path, query or fragment` + if (rest.includes('@')) return `'${origin}' should not contain userinfo` + // Only the port is checked past this point. The host is left alone on + // purpose: browsers send origins this cannot anticipate, `chrome-extension` + // and IPv6 literals among them, and a warning that cries wolf on a working + // origin is worse than one that stays quiet. + if (rest.startsWith(':')) return `'${origin}' has no host` + // An unclosed bracket would otherwise leave `portStart` at zero, which reads + // as "no port" and lets the entry through unremarked. + if (rest.startsWith('[') && !rest.includes(']')) + return `'${origin}' has an unclosed IPv6 host` + const portStart = rest.startsWith('[') ? rest.indexOf(']') + 1 : rest.indexOf(':') + // A trailing colon is a port, an empty one — distinct from having none. + const port = portStart > 0 && rest[portStart] === ':' ? rest.slice(portStart + 1) : undefined + if (port !== undefined && !(/^[0-9]{1,5}$/.test(port) && Number(port) <= 65535)) + return `'${origin}' has a port no browser can send` + return undefined +} + +/** + * What the settings API would refuse, mirroring `parse_allowed_origins_setting` + * in windmill-common. + * + * Distinct from reading the setting for display: that drops entries it cannot + * use, while this has to report them, or a shape only the YAML editor can + * produce would pass here and come back as a 400 on save. + */ +export function allowedOriginsSettingError(setting: unknown): string | undefined { + let origins: string[] + if (setting == null || typeof setting === 'string') { + origins = parseAllowedOrigins(typeof setting === 'string' ? setting : '') + } else if (Array.isArray(setting)) { + if (setting.some((entry) => typeof entry !== 'string')) return 'Entries must be strings' + // Not filtered for empties, unlike the comma-separated form, where a + // trailing separator is a typing artifact rather than an entry. + origins = setting.map((entry) => (entry as string).trim()) + } else { + return 'Expected a comma-separated string or a list of strings' + } + return allowedOriginsError(origins) +} + +/** + * Read the instance-default setting, mirroring `parse_allowed_origins_setting` + * in windmill-common: the settings UI writes a comma-separated string, but the + * API accepts an array too. + */ +export function parseAllowedOriginsSetting(setting: unknown): string[] { + if (typeof setting === 'string') return parseAllowedOrigins(setting) + if (Array.isArray(setting)) + return setting + .filter((origin): origin is string => typeof origin === 'string') + .map((origin) => origin.trim()) + .filter((origin) => origin !== '') + return [] +} + +/** + * Whether a route is restricted to specific origins, mirroring + * `effective_allowed_origins` in windmill-trigger-http: a route with a non-empty + * list of its own restricts, `*` in it is the opt-out, and anything else — an + * empty list included, since that is not a configuration — falls back to the + * instance default. + */ +export function isOriginRestricted( + allowed_origins: string[] | undefined, + instanceDefaultOrigins: string[] +): boolean { + if (allowed_origins !== undefined && allowed_origins.length > 0) + return !allowed_origins.includes('*') + return instanceDefaultOrigins.length > 0 && !instanceDefaultOrigins.includes('*') +} + export const SECRET_KEY_PATH = 'secret_key_path' export const HUB_SCRIPT_ID = 19670 export const SIGNATURE_TEMPLATE_SCRIPT_HUB_PATH: string = `hub/${HUB_SCRIPT_ID}` @@ -56,6 +196,7 @@ export async function saveHttpRouteFromCfg( authentication_resource_path: routeCfg.authentication_resource_path, wrap_body: routeCfg.wrap_body, raw_string: routeCfg.raw_string, + allowed_origins: routeCfg.allowed_origins, description: routeCfg.description, summary: routeCfg.summary, error_handler_path: routeCfg.error_handler_path, diff --git a/system_prompts/auto-generated/schemas/http_trigger.schema.yaml b/system_prompts/auto-generated/schemas/http_trigger.schema.yaml index 14628d4001..f484366783 100644 --- a/system_prompts/auto-generated/schemas/http_trigger.schema.yaml +++ b/system_prompts/auto-generated/schemas/http_trigger.schema.yaml @@ -95,6 +95,21 @@ properties: type: boolean description: If true, passes the request body as a raw string instead of parsing as JSON + allowed_origins: + type: array + items: + type: string + description: 'Origins allowed to call this route cross-origin, matched against + the request''s Origin header (ignoring case) and echoed back on a match. When + set, the list governs both the preflight and the response, overriding any Access-Control-Allow-Origin + the runnable returns via wm_headers. Use [''*''] to opt out of any restriction, + including the http_route_default_allowed_origins instance setting. An empty + list is not a configuration and resolves exactly as null does. When null, the + instance setting applies, or Access-Control-Allow-Origin: * if it is unset. + Ignored on a static website, which has no authentication of its own and so hands + out public files: restricting which browsers may read them protects nothing + while breaking cross-origin webfonts and fetches. A single-file static asset + is not exempt, since it can carry an authentication_method.' error_handler_path: type: string description: Path to a script to run when the triggered job fails. A bare path, From 80eba80d6ed51753cfaa67310f1a0f5dd5ce0484 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Mon, 14 Sep 2026 11:42:57 +0200 Subject: [PATCH 2/2] feat(git-sync): gate GitHub PRs on Windmill CI test results (WIN-2051) (#10096) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs: add design doc for automatic git-to-windmill pull sync * docs: add migration plan and implementation phases to git-sync pull design * feat(git-sync): add auto_pull settings schema and pull enqueue primitive Adds AutoPullSettings/AutoPullMode/AutoPullStatus on GitRepositorySettings (workspace_settings.git_sync JSONB), the GIT_SYNC_PULL_SCRIPT_PATH constant, and should_pull/effective_poll_interval_s helpers with unit tests. Exports the EE enqueue_git_pull_job primitive. Foundation for repo→Windmill auto-pull. Co-Authored-By: Claude Opus 4.8 (1M context) * feat(git-sync): poll repos and auto-pull new commits into the workspace Phase 1 of automatic repo → Windmill sync. A monitor task (EE-licensed, single-replica via advisory lock) git ls-remotes each auto-pull-enabled repository ~every minute and enqueues a pull when the tracked branch moves, reusing the {workspace_id}:git_sync concurrency key so pulls serialize with in-flight push commits. - windmill-store: background (no-authed) resolver get_git_repo_head_for_autopull that resolves the repo resource (incl. $var: refs) and ls-remotes; GitHub-App repos are skipped here and will sync via webhooks (phase 2). - monitor.rs: poll/reconcile/persist with optimistic sha advance and failure status; targeted jsonb update so concurrent settings edits aren't clobbered. - edit_git_sync_repository: preserve server-owned auto_pull state on UI save. - openapi: AutoPullSettings/AutoPullMode/AutoPullStatus + auto_pull field. - frontend: per-repo "Automatically deploy changes from Git" toggle with last sync status; demote the GitHub Actions link to an advanced CI option. Co-Authored-By: Claude Opus 4.8 (1M context) * feat(git-sync): wire webhook lifecycle + receiver; share reconcile logic OSS side of phase 2 auto-pull webhooks: - edit_git_sync_repository creates/removes the repo webhook on save (EE-gated, best-effort → falls back to polling). - monitor poller now delegates to the shared windmill_git_sync reconcile/persist helpers (also used by the webhook receiver), removing duplicated logic. - export the shared reconcile/persist/failure helpers; bump EE ref. Co-Authored-By: Claude Opus 4.8 (1M context) * chore(git-sync): bump EE ref for phase 3 in-app PR creation Co-Authored-By: Claude Opus 4.8 (1M context) * feat(git-sync): show webhook vs polling status on the auto-pull toggle When a repo has an active webhook (auto_pull.webhook_id set), the status line reads "instant via webhook"; otherwise it reads the ~1-minute polling cadence. Co-Authored-By: Claude Opus 4.8 (1M context) * feat(git-sync): post PR diff check on dry-run completion (phase 4) Worker completion hook in process_completed_job: when a DeploymentCallback job carrying the __git_sync_pr_check marker finishes, parse the dry-run SyncResponse and patch the GitHub check run with the diff summary (success/neutral/failure). Export enqueue_git_pull_dry_run; bump EE ref. Co-Authored-By: Claude Opus 4.8 (1M context) * chore(git-sync): bump EE ref (drop unused GHES webhook_secret) Co-Authored-By: Claude Opus 4.8 (1M context) * revert(git-sync): defer phase 4 PR diff checks (OSS side) Remove the worker completion hook that posted the PR check run, drop the enqueue_git_pull_dry_run re-export and the orphaned sqlx cache, bump EE ref. Phases 1-3 (polling, webhooks, in-app PR creation) are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) * Revert "revert(git-sync): defer phase 4 PR diff checks (OSS side)" This reverts commit 0137d3ca489983b22870e15f442613bd55e79e99. * chore(git-sync): point EE ref at restored phase 4 commit Co-Authored-By: Claude Opus 4.8 (1M context) * chore(git-sync): bump EE ref for clone_ref dry-run Co-Authored-By: Claude Opus 4.8 (1M context) * chore(git-sync): bump init-repository hub script to v28784 Picks up the clone_ref param (windmill-integrations#158) so the phase 4 PR-check dry-run can clone the PR head. Backward compatible; manual pull/push and the automated pull/poller/webhook all move to the same published version. Co-Authored-By: Claude Opus 4.8 (1M context) * chore(git-sync): bump EE ref for auto-pull admin-permissioning fix Co-Authored-By: Claude Opus 4.8 (1M context) * chore(git-sync): bump EE ref for superadmin pull fallback Co-Authored-By: Claude Opus 4.8 (1M context) * fix(git-sync): refresh auto-pull tooltip; bump EE ref for webhook secret encryption The auto-pull toggle tooltip claimed GitHub App repos would sync via webhooks "in a future update"; webhook delivery now works, so describe the webhook-vs-polling behavior accurately. Bump the EE ref to pick up encrypting the webhook HMAC secret at rest. Co-Authored-By: Claude Opus 4.8 * fix(git-sync): poll app-backed repos in auto/polling mode The auto-pull poller skipped app-backed repos (the ls-remote head check can't authenticate a tokenless URL), so auto- and polling-mode app repos never synced when their webhook wasn't live. Wire the poller to fetch the head via the GitHub API for app repos and reconcile. Bump the EE ref. Co-Authored-By: Claude Opus 4.8 * feat(git-sync): auto-pull UI — direction split, delivery mode, fallback notice Reorganize the repository card into two clearly labeled directions: "Push to Git on deploy (Windmill → Git)" and "Pull from Git (Git → Windmill)". In the pull section: - new connections default to auto-pull enabled (webhook with polling fallback); existing repos load with auto-pull off and are unchanged - a Delivery selector chooses "Webhook with polling fallback" or "Polling only (air-gapped)" - a notice surfaces webhook_error when delivery falls back to polling - a reminder to remove any pre-existing GitHub Action that pushed into Windmill, to avoid conflicting double-syncs Adds the webhook_error field to AutoPullSettings (+ openapi) and bumps the EE ref. Co-Authored-By: Claude Opus 4.8 * feat(git-sync): clearer push indicator + gate webhook delivery to app repos - Push-on-deploy is shown with a check icon + concise line (via the shared GitSyncModeDisplay, restyled from the oversized "Sync:" text); the setup wizard reuses it without the check (pre-save preview). - The delivery-mode selector only shows for GitHub App-backed repos; token-based repos show a "webhooks require the GitHub App (managed or GHES)" note with a docs link and poll instead. Bumps the EE ref. Co-Authored-By: Claude Opus 4.8 * feat(git-sync): fork auto-sync (phase 5) + live deploy check (phase 6) Phase 5 — fork auto-sync configured at the parent (replaces the *-to-forks GitHub Actions): - Add fork_open_prs + fork_pull_sync to GitRepositorySettings (openapi + UI). - UI: two "Forks of this workspace" toggles in the repo card, gated on app-backed and not-a-fork; serialize the flags on save. - On fork creation, strip the inherited auto_pull block (and fork_* flags) from the copied git_sync repo: a fork must not carry the parent's webhook id (it would delete the parent's hook on disable) or self-poll on top of the parent's fan-out. Push-direction config + installation are still inherited unchanged. Phase 6 — live deploy status check on the commit (Cloudflare-style): an in-progress "Windmill" check on the head commit that flips to "Deployed N changes"; completion handled by the generalized git-sync check hook. Bump EE ref for the phase 5-6 EE implementation. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * chore(git-sync): bump EE ref for PAT auto-pull mode normalization Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): address PR review findings - webhook_secret: redact from the settings API response and Debug output (still persisted encrypted); it's a server-only HMAC key the UI never needs. - poller: honor each repo's effective poll interval (relaxed ~10 min when a webhook is live) instead of probing every ~60s tick. - settings save: roll back a just-created webhook if the settings transaction doesn't commit, so a failed save can't orphan a hook. - auto-pull head check: fail SSH remotes with an actionable message (background polling has no SSH identity) instead of a confusing ls-remote error. - deploy/PR check summary: a pull result carrying neither changes nor a settings diff now falls back to the unsummarized path instead of a false "in sync". - UI: reset isGithubApp on resource change / failed fetch so webhook + fork controls can't show for the wrong repo. - tests: cover parse_git_sync_changes and format_change_list edge cases. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): correct feature gating for OSS builds - monitor.rs: keep the AUTO_PULL_LAST_POLL static, slack const, and poll_git_auto_pull_inner all behind #[cfg(feature = "private")] (an inserted static had split the cfg off the function, ungating it in OSS builds). - edit_git_sync_repository: the webhook create/rollback block references windmill_common::git_sync_ee (private module), so gate it on all(enterprise, private) instead of enterprise only. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * chore(sqlx): cache workspace_diff query pulled in from origin/main Re-merged origin/main (advanced past the earlier merge); regenerate the offline sqlx entry for the new workspace_comparison test query so SQLX_OFFLINE builds (cargo_test) pass. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): address Codex review findings (webhook cleanup on delete) - Deleting a git-sync repository now tears down its managed GitHub webhook (deletion bypassed the sync_repo_webhook lifecycle, orphaning the hook so GitHub kept delivering to the instance). - Worker completion hook rolls back the optimistic auto-pull sha on job failure (OSS side of the EE change) + caches the new marker query. Bump EE ref. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): delete repo webhook after the removal commits Codex re-review nits: - delete_git_sync_repository deleted the webhook before the settings transaction committed; a failed save would then leave the repo pointing at a hook that no longer exists (sync_repo_webhook treats a set webhook_id as live and won't recreate it). Capture the hook id, commit the DB removal, then delete the hook. - Reword a fork-copy comment to drop drafting-history wording per AGENTS.md. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): reconcile the edit-path webhook after the settings commit Codex nit: edit_git_sync_repository ran sync_repo_webhook before the transaction committed. The rollback only covered created hooks, but sync_repo_webhook also deletes a hook on disable/switch-to-polling — a commit failure then left the DB with a webhook_id whose hook was already gone (and it wouldn't be recreated). Save + commit first, then reconcile the webhook against the durable config and persist any hook id/secret change (best-effort). Bump EE ref. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): preserve webhook secret on whole-config save + default on visible add Codex nits: - edit_git_sync_config saved the client config verbatim, so the webhook_secret redacted from the GET response would be dropped (breaking delivery). Preserve server-owned auto-pull state (webhook id/secret, synced sha, last status) per repo from the existing settings, matching edit_git_sync_repository. - addSyncRepository (the visible add path) didn't set the auto_pull default, so new sync repos added from the UI came up with auto-deploy off. Match addRepository's default (webhook + polling fallback). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * refactor(git-sync): drop fork_pull_sync (parent-level keep-forks-in-sync) Removes the "Keep forks in sync with the tracked branch" toggle and its fan-out. Pulling the tracked branch straight into every fork was the inconsistent piece; the consistent model is per-fork branch sync (each fork tracks its own wm-fork/** branch), which is a separate follow-up. fork_open_prs is kept. Also tightens the fork toggle-section spacing. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): detect dev workspaces in CLI fork branch derivation isForkWorkspace / computeGitSyncDeployBranch keyed off the wm-fork- id prefix. Dev workspaces are forks with a custom, prefix-less id, so their wm-fork/** branch was never derived or created. Detect them via parent_workspace_id too (which the backend already passes), mirroring the backend's `parent.is_some() || wm-fork- prefix` rule. Pairs with the hub-script clone-flag fix (windmill-integrations#163); both take effect once the CLI is released and the pinned version is bumped. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): reconcile webhooks on full-config save edit_git_sync_config preserved server-owned webhook fields but never created or deleted the managed GitHub webhook, so enabling auto-pull through the whole-config endpoint only polled, and disabling or removing a repo left an orphan hook still delivering. Mirror the per-repository endpoint: after the commit is durable, reconcile every saved repo's webhook (sync_repo_webhook) and delete the hooks of repos the save removed, including the clear-whole-config case. Addresses the Codex nit. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): address Codex nits (webhook orphan on cleared auto_pull, fork detection) - edit_git_sync_config: also delete a repo's old webhook when the save drops the repo OR clears its auto_pull. Webhook fields are only preserved onto a Some auto_pull, so a save that present-but-clears a repo would otherwise orphan its hook. - GitSyncRepositoryCard: isFork now uses parent_workspace_id OR the wm-fork- prefix (was AND), matching the backend/CLI rule, so prefix-less dev workspaces are detected as forks and don't show the parent fork-PR toggle. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * docs(git-sync): update design doc for the dropped fork_pull_sync Phase 5 documented "Keep forks in sync with the tracked branch" (fork_pull_sync) and its fan-out as implemented; that feature was removed. Rewrite the section to reflect what ships (fork_open_prs), note the drop + the per-fork-branch follow-up, and remove the stale fan-out mentions elsewhere. Addresses the Codex nit. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): redact webhook secrets from workspace export; fix doc endpoints - Export (P1): strip the server-owned auto_pull state (webhook secret/id/error + synced sha + last pull status) from git_sync before it is written into an export's settings.json for both settings formats. The HMAC webhook secret must never leave the server (matching the GET-settings redaction), and a re-imported workspace must not inherit another install's hook/sync state. - Docs: the webhook receiver is a single per-workspace endpoint /api/w/{workspace}/github_app/webhook (host-aware for managed + self-managed); update the stale push_webhook/{id} and instance-global /api/github_app/webhook references. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): skip deleted/archived workspaces in the auto-pull poller The poller scanned workspace_settings directly, so an archived (soft-deleted) or renamed-away workspace — whose settings row persists — kept polling and could enqueue a pull into a dead workspace. Join workspace and require NOT deleted. The EE webhook receiver gets the same filter (ee ref bumped). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): never trust client-supplied server-owned auto-pull fields Both write endpoints (edit_git_sync_repository, edit_git_sync_config) persisted caller-supplied auto_pull.webhook_id / webhook_secret / webhook_error / last_synced_sha / last_pull_status when adding a repo or newly enabling auto-pull, letting a client inject a webhook id/secret or fake sync state. Strip those server-owned fields from the request up front; existing repos re-derive them from the DB (carried over), new ones start clean and the server (re)creates the webhook. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): merge webhook fields post-commit instead of clobbering the row The post-commit webhook reconcile in edit_git_sync_repository and edit_git_sync_config wrote the whole pre-reconcile git_sync snapshot back after the main save committed. A concurrent git-sync edit or poller status write that landed in the gap could then be dropped by the stale snapshot. Re-read the current row and merge only the reconciled webhook id/secret/error for the repos the reconcile actually changed. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * feat(git-sync): parent-managed fork sync + PR-on-deploy toggles Fork sync (push-on-merge-to-forks parity): a parent-level auto_pull.sync_forks toggle routes changes on each fork's wm-fork/** branch into that fork workspace, via the parent's existing webhook and one extra fork-heads listing per poll tick (git ls-remote pattern for token repos, git/matching-refs for app-backed). Fork state is a server-written status-only auto_pull blob on the fork's own repo entry; the fork's card shows a read-only "managed in the parent workspace" line with its branch and last pull status. Dev workspaces (prefix-less ids) use the same branch parsing (unit-tested in windmill-common). PR-on-deploy: opening PRs for Windmill-pushed branches moves into the deploy pipeline, per repo toggle (promotion_open_prs on the promotion repo; parent-level fork_open_prs for fork deploys). The push job carries a marker and the job-completion hook derives the pushed branch (helper unit-tested against the CLI formula) and opens the PR outbound, so it works without inbound webhooks; the webhook-side wm_deploy PR arm is removed. The documented open-pr-* GitHub Actions remain valid alternatives (PR creation is idempotent). Fork guards: promotion mode, enabled auto-pull, and fork_open_prs are rejected on fork workspaces (they are parent-managed; a fork's deploys always target its wm-fork/** branch) and the promotion card is hidden in a fork's settings. Enabling auto-pull now also requires EE, and the post-commit webhook reconcile persists the normalized delivery mode. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * feat(git-sync): dev workspaces sync with their environment-label branch A dev workspace's git branch is its environment label verbatim (dev/ staging, default dev) — a first-class env branch like the documented push-on-merge-staging layout — instead of the wm-fork/** form. The label rides the deploy job args (backend → hub script → CLI --dev-workspace-label), the PR completion hook derives the same head, the webhook/poller route label branches into the matching dev-workspace child (poller lists them alongside wm-fork/* via extra ls-remote refs / per-label API lookups), and manual pulls from the UI pass clone_ref accordingly. The CLI refuses to deploy when the label branch equals the checked-out tracked branch, which would otherwise commit fork content straight to it. Because the branch is keyed on the label, the label is now immutable after creation: set at create/attach only, the set_dev_workspace_label endpoint is removed and the settings tab shows it read-only. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * feat(git-sync): nested fork routing + fork-of-dev branch rooting A fork of a dev workspace now roots its wm-fork/** branch on the dev's environment-label branch (the content it diverged from) and its PR merges back into that branch: the backend passes parent_dev_workspace_label with the deploy (parent row joined in both enqueue paths), the CLI gains --parent-dev-workspace-label and checks it before the wm-fork- prefix fallback when rooting a fork-of-a-fork branch, and the PR completion hook uses it as the PR base. Fork sync routing covers the whole live descendant chain of the webhook/poller workspace (recursive, depth-capped) instead of direct children only, and fork_open_prs is resolved at the root ancestor — only the root can hold auto-pull config, so grandchild forks sync through it. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * feat(git-sync): PR deploy-preview comment, clearer check copy, app-only hints - The PR diff completion hook maintains one managed comment on the PR (Cloudflare deploy-preview style: workspace, status, commit, collapsible change list), upserted per synchronize via a hidden marker. The check run stays for required-check gating. - A settings difference in the diff summary is worded by cause: the PR changes wmill.yaml, vs pre-existing drift between the repo's wmill.yaml and the workspace, vs undetermined (neutral wording). - Deploy-status check titles name the target workspace ("Deployed 2 change(s) to staging"), since GitHub shows a head commit's checks on any PR containing it and a bare "Deployed" read as if the PR had deployed. - Token-based repos see a hint pointing at the open-pr-on-commit / open-pr-on-fork-commit workflows where the app-only PR toggles would be; an API-set toggle on a non-app repo now logs a warning naming the fallback; the design doc lists app-only features and their degradation. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): EE-gate auto-pull UI, fork pull clone_ref, no-op push PR gate - CE: the auto-pull and fork-PR toggles are disabled with an EE badge, and new sync repos only default them on when licensed (basic git sync is available on CE since #8493, but auto-pull is EE and the backend rejects it) - The pull modal passes clone_ref for wm-fork- forks (wm-fork//) so a manual pull fetches the fork branch instead of the tracked branch head - PR-on-deploy skips no-op pushes: when the push script reports pushed=false (e.g. the deploy was caused by an auto-pull), the completion hook no longer ensures a PR, so closed PRs aren't recreated by the sync loop Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * chore: refresh package-lock after main merge (windmill-utils-internal 1.8.2) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * test: auto-pull e2e integration tests; fix PR comment table formatting Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): runtime license gate for auto-pull saves; user/group promotion-branch parity Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): explain in-sync PR verdicts with the repo's sync filter scope A PR that only touches files outside the repository's include paths gets "In sync", which reads as a wrong verdict; the check summary (and managed comment) now name the filters, e.g. "Only files matching this repository's sync filters deploy on merge: `f/**` (excluding `f/pat/**`)." Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * feat(git-sync): clearer card copy/structure; surface PR-creation failures - Fork sync toggle renamed and kept in the pull section; the fork PR toggle moves to the push section with a note that push settings apply to forks - Fork/dev workspaces' push section names their actual branch instead of the tracked-branch line; promotion repos hide the pull direction (promotion pushes deploy branches on top of a sync-mode setup) - Promotion mode line describes the wm_deploy/** branch + merge-to-promote flow; workflow-fallback hints lead with the how-to and link to the docs; test connection button demoted from accent per brand guidelines - New server-owned open_pr_error on repo settings: the deploy completion hook records why a PR couldn't be opened (e.g. app permission not yet approved) and clears it on the next success; shown as a warning under the PR toggles Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix: cfg-gate scope-note helper (dead code on OSS builds) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): license-gate preserved auto-pull; attach strips parent-only settings - edit_git_sync_repository re-checks the runtime Enterprise gate against the EFFECTIVE repo state after preservation: the older-client arm copies the existing auto_pull back, which the request-side check never saw - attach_dev_workspace now mirrors the fork-creation copy on the attached workspace's own git sync: promotion repos dropped, auto_pull/fork PRs/PR error stripped, and any managed webhook deleted after commit (the attached workspace is parent-managed and must not keep pulling its old tracked branch) - integration test: attaching an auto-pull-enabled workspace strips it Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): detach clears standalone parent; reject label == tracked branch - detach_dev_workspace clears parent_workspace_id for prefix-less (attached standalone) workspaces so they stop classifying as forks and deploying to wm-fork/** branches; wm-fork- re-designated forks keep their parent; cache invalidations mirror attach - dev-workspace create/attach reject an environment label that equals a git-sync repository's tracked branch (prod's or the candidate's): deploys would target the very branch the repo syncs from, and the CLI guard would fail every push job after the fact - CLI unit tests: prefix-less fork beats wm_deploy derivation; isForkWorkspace parent-id argument Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * chore(git-sync): bump hub script pins (push 28786, pull 28785) Published from windmill-integrations #163 with windmill-cli@1.753.1-gitsync.0: dev-workspace label deploys, fork-of-dev rooting, fork checkout on the existing remote branch, and the pushed-flag result. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): no parent-only defaults on fork repos; rename strips webhook state - addSyncRepository skips the auto_pull/fork_open_prs defaults on fork/dev workspaces where the backend rejects them (saving a new sync repo from an EE fork 400'd deterministically) - change_workspace_id strips webhook id/secret/error from the copied git_sync and deletes the stale GitHub hooks post-commit: they deliver to the old (archived) workspace URL, so the new workspace would report a live webhook while polling at the relaxed interval; next save re-registers cleanly - EE: PR diff checks for contributor-fork PRs clone the synthetic pull//head ref (head.ref doesn't exist in the base repo) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * chore(git-sync): bump pull script pin to hub/28787 (synthetic PR ref support) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): targeted jsonb update for open_pr_error (no full-blob clobber) The full read-modify-write raced the poller's concurrent last_synced_sha / last_pull_status writes on the same column; mirror the EE status writer and update only the matching repository element's open_pr_error key. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * style(git-sync): inline EE badge on gated toggles (matches settings nav) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * copy(git-sync): both directions in page/card descriptions; clearer promotion flow - Page header and sync-card description mention the pull direction, not only push-on-deploy - Promotion description walks the actual flow (wm_deploy/** branch, merge to promote, sync the target workspace) and points at the PR toggle / workflow; the Git Promotion docs link now also shows on configured cards, not only in the empty state Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): resolve branch-less resources' default branch for fork sync A git resource without an explicit branch polled as the bare "HEAD" ref, which the fork/dev-label fan-out cannot scope (wm-fork//*), so fork sync silently never ran on polling-only repos. Resolve the remote's default branch name with `ls-remote --symref HEAD` (one call for name + head sha); "HEAD" only remains when resolution fails. The polling e2e test now uses a branch-less resource to cover this shape. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): runtime license gate for in-app PR creation promotion_open_prs/fork_open_prs are rejected on save without an Enterprise plan (like auto_pull), and the deploy completion hook re-checks the plan before opening PRs so flags stored while licensed stop driving GitHub calls after a lapse. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * feat(git-sync): app-aware pull defaults, always webhook delivery, token-repo guidance - Pull-from-Git defaults on only for app-backed repos (applied when the selected resource resolves); polling is opt-in for token repositories, with a warning alert recommending the GitHub App (instant pull + in-app PRs) or the sync GitHub workflow - App repos always use webhook delivery with polling fallback: the delivery selector is gone and a stored polling mode is normalized back to auto - Post-save modal reflects the auto-pull state instead of telling the user to turn on a toggle that is already on - Non-app PR hints recommend the GitHub App explicitly Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * copy(git-sync): single info box for token-repo pull guidance Merges the instant-pull recommendation with the GitHub Action conflict note, shown only for non-app repos; app repos need neither, and the redundant 'instant webhook sync requires' line is gone. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * copy(git-sync): keep the GitHub Action conflict note on app repos Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * copy(git-sync): anchor docs links to their exact sections GitHub App references point at integrations/git_repository#github-app, the workflow hints at deploy_gh_gl#github-actions-setup, and the sync workflow at git_sync#github-actions (all anchors verified against the live docs). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * copy(git-sync): fork workflow hint links to git_sync#github-actions open-pr-on-fork-commit is documented on the git_sync page, not deploy_gh_gl. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * fix(git-sync): fork PRs are opt-in on new connections too Only auto-pull and fork sync default on for new app-backed connections; opening pull requests stays a deliberate per-repo decision. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * feat(git-sync): default the managed PR on for new app-backed promotion repos A promotion deploy's wm_deploy/** branch exists to be merged; without a PR it's an orphaned branch. Fork PRs stay opt-in. Also scope the sync-repo auto-pull default to sync mode so promotion repos can't pick it up. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * docs(git-sync): GHES self-managed app permission setup Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * docs(git-sync): frame permission update against GitHub Actions, not polling Existing installations don't have polling; their git-to-Windmill direction runs on GitHub Actions today, so the approval text describes the update as replacing those workflows and notes every feature is opt-in. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm * feat(git-sync): gate GitHub PRs on Windmill CI test results (WIN-2051) Add a "Windmill CI tests" GitHub check run on any PR against the tracked branch, so a workspace-fork customer can mark it a required status check and have Windmill's own CI tests (the `// test:` annotation) block the PR — replacing the GitHub Action that polls the CI-test results API. Driven by the pull_request webhook (same event as the Phase 4 diff check): a new git_sync_ci_test_check table tracks one check per (fork workspace, PR head commit); a per-ci_test-job completion hook (result_processor) and the git-sync poller sweep (monitor) conclude it. Reuses the Checks: write grant from #9552; token repos keep the documented Action. See docs Phase 7. EE logic lives in windmill-ee-private (git_sync_ee.rs); see ee-repo-ref.txt. Co-Authored-By: Claude Opus 4.8 (1M context) * chore(sqlx): drop two cache entries orphaned by the merge Both queries were rewritten on main; nothing in the sources produces them. Co-Authored-By: Claude Fable 5.1 * docs(migration): describe the CI-test check as webhook-opened, not deploy-opened Co-Authored-By: Claude Fable 5.1 * chore: bump ee-repo-ref (skipped CI-test check on unmapped PRs) Co-Authored-By: Claude Fable 5.1 * docs(git-sync): skipped check for unmapped PRs; correct the installation rationale Co-Authored-By: Claude Fable 5.1 * fix(git-sync): cascade CI-test check rows with their workspaces; review fixes (EE ref) Deleting a fork (the normal end of the one-fork-per-PR flow) or its parent must not fail on the check table's foreign keys. Co-Authored-By: Claude Fable 5.1 * fix(git-sync): CI-test check waits for the head's deploy; ignore retired tests (EE ref) Design doc: GitLab scope, pending-while-deploying and retired-test rules. Co-Authored-By: Claude Fable 5.1 * fix(git-sync): CI-test verdict matches jobs against current test references (EE ref) Co-Authored-By: Claude Fable 5.1 * fix(git-sync): record the pushed head as synced; CI-test check waits for it (EE ref) The push completion hook records the commit the deploy push script reports as the branch's synced sha, mirroring what a pull records. The check row stores head_ref. Requires the hub push script version that reports the sha; the pin is bumped once it is published. Co-Authored-By: Claude Fable 5.1 * fix(git-sync): CI-test check row keeps the newest check run when creates race (EE ref) Co-Authored-By: Claude Fable 5.1 * fix(ci-tests): queue CI tests before a deploy or dependency job completes The dispatch was detached, so a finished deploy did not yet imply its tests existed. The git-sync PR check reads "deploy settled" as "tests queued", so the dispatch is awaited at all three sites; errors are still only logged. Co-Authored-By: Claude Fable 5.1 * chore(ci-tests): sqlx cache for the pull-success readiness clause; bump EE ref Co-Authored-By: Claude Fable 5.1 * docs(ci-tests): readiness waits for pull success, failed deploy fails the check; sqlx cache; EE ref Co-Authored-By: Claude Fable 5.1 * fix(ci-tests): queue resource-triggered CI tests before the update is acknowledged Same ordering guarantee as scripts and flows: the git-sync PR check treats a finished pull as "the deploy's tests exist". Also carries the sqlx cache for the tested-item liveness filter and the EE ref. Co-Authored-By: Claude Fable 5.1 * fix(ci-tests): dispatch on flow/resource create, sweep outside the auto-pull lock, pushed-head map - create_flow and create_resource queue CI tests the way the update paths do - the check sweep runs after the advisory unlock so GitHub calls never extend the auto-pull tick - AutoPullSettings gains last_pushed_sha (settings round-trips keep it); the EE push hook writes it instead of last_synced_sha so auto-pull still pulls a commit someone else pushed under ours Co-Authored-By: Claude Fable 5.1 * fix(git-sync): last_pushed_sha is server-owned auto-pull state Clear it from client-supplied settings, carry it over from the existing entry on save, and keep it out of workspace exports, like the other sync state fields. Correct the two doc comments that still described the pushed head as the synced head. Co-Authored-By: Claude Fable 5.1 * feat(git-sync): say which pull toggles the PR checks depend on The repo webhook created by the pull toggle is what delivers pull request events, and fork sync is what lets a fork PR's externally pushed commits reach the fork workspace before its CI tests check is read. Co-Authored-By: Claude Fable 5.1 * refactor(git-sync): CI check readiness reads a synced-head event log New git_sync_synced_head table: one row per commit a workspace reflects on a branch, written by the pull completion hook on success (from the pull marker's branch and sha) and by the push completion hook from the push script's reported sha. The check reads that instead of inferring sync state from the auto-pull settings jsonb, so AutoPullSettings gets no pushed-sha field and the settings gates for it go away. Pull success now also runs the auto-pull completion hook. Co-Authored-By: Claude Fable 5.1 * fix(ci-tests): queue CI tests before the deploy push is enqueued; schema summary The synced-head row a push writes is what makes the PR check read the fork's results, so the tests must be queued before the push job exists. Reordered at the script create, lockfile, and resource sites (flows already push from their dependency job). Lists the two new tables in summarized_schema.txt. Co-Authored-By: Claude Fable 5.1 * docs(ci-tests): newest synced head decides readiness; EE ref Co-Authored-By: Claude Fable 5.1 * docs(ci-tests): newest synced head decides readiness Co-Authored-By: Claude Fable 5.1 * fix(git-sync): synced heads keyed by repository; check row keeps the repo resource path Both tables are unreleased, so their migrations gain the column in place. The push hook reads the push job's repository from its args again. Co-Authored-By: Claude Fable 5.1 * fix(git-sync): CI check rows keyed by repository; EE ref Co-Authored-By: Claude Fable 5.1 * chore(git-sync): pin the deploy push script to hub/28956 The version whose main returns the push outcome {pushed, sha, branch}, which the completion hook records as the branch's synced head. Co-Authored-By: Claude Fable 5.1 * chore(ci-tests): tooltips name GitHub for the CI check; drop the sanitization test; docs and EE ref The sanitization test guarded a field that no longer exists and now only re-exercised pre-existing behavior. The design doc carries the repository key and the row-before-create order. Co-Authored-By: Claude Fable 5.1 * docs(ci-tests): re-fire resets the run id; pinned-script reason on timeout; EE ref Co-Authored-By: Claude Fable 5.1 * fix(git-sync): record the commit a pull checked out; skip a rebased push The pull script now reports the sha and branch its clone landed on, so the synced head is what was applied rather than the head observed when the pull was enqueued (the marker stays the fallback). A push the script had to rebase sits on commits the workspace has not pulled, so it is not recorded; the pull those commits trigger records the head. Co-Authored-By: Claude Fable 5.1 * chore(git-sync): pin the push script to hub/28958 and the pull script to hub/28957 The versions whose results report a rebased push and the commit a pull checked out. Co-Authored-By: Claude Fable 5.1 * docs(ci-tests): queued-pull wait and kept failure on re-fire; EE ref Co-Authored-By: Claude Fable 5.1 * chore(ci-tests): sqlx cache for the prune and skipped-guard queries; EE ref Co-Authored-By: Claude Fable 5.1 * docs(ci-tests): a failed head passes only on newer runs; EE ref Co-Authored-By: Claude Fable 5.1 * refactor(ci-tests): the PR check runs the head's own suite The synced-head row records the CI test runs dispatched for the head (tests_dispatched_at, ci_test_job_ids); the check reads exactly those. windmill-git-sync now owns the check lifecycle (it can enqueue jobs) and depends on windmill-dep-map for the dispatcher. The deploy-site changes that ordered per-item dispatch before the push existed only for the old workspace-wide verdict and are restored to main's form; flow and resource creation keep the per-item dispatch they lacked. Co-Authored-By: Claude Fable 5.1 * fix(ci-tests): suite runs are not debounced, run as the workspace admin, and a dead dispatch claim is retaken EE ref f30eaf8: the head's suite no longer shares debounce keys with deploy-triggered runs (a superseded run dropped out of the verdict), runs as the workspace admin an auto pull resolves rather than the sync job's identity (which retention could remove), and a dispatch claim that never recorded ids is retaken after 5 minutes. sqlx cache and design doc updated. Co-Authored-By: Claude Fable 5.1 * fix(ci-tests): the head's suite expands wildcard references EE ref 53d77c3; sqlx cache updated for the reference and script queries. Co-Authored-By: Claude Fable 5.1 * fix(ci-tests): bound the failed-deployment gate; keep a current head's check row EE ref 63523d5; sqlx cache updated for the two changed queries. Co-Authored-By: Claude Fable 5.1 * fix(ci-tests): the head's suite runs as the fork's owner; poster_workspace_id EE ref a74af94. The unreleased check-table migration renames github_workspace_id to poster_workspace_id (the workspace whose credential posts the check); sqlx cache, schema summary and design doc follow. Co-Authored-By: Claude Fable 5.1 * docs(git-sync): the PR check's suite runs as the fork's owner Co-Authored-By: Claude Fable 5.1 * fix(ci-tests): CE-only build, UI pull-script pin, detached sweep - The four check-lifecycle re-exports exist only on enterprise builds; gating them on private alone broke the CE image (private without enterprise). - hubPaths.json's gitInitRepo follows the server's pull-script pin (28957), as every previous bump did and as the git-sync integration test asserts. - The monitor spawns the CI-check sweep detached (EE ref f96aa31). Co-Authored-By: Claude Fable 5.1 * chore: bump EE ref to a1d7a1e Co-Authored-By: Claude Fable 5.1 * fix(ci-tests): advance the PR check off the worker's completion loop Concluding a check calls GitHub; the completion loop finishes jobs serially, so the evaluation is spawned detached (idempotent, retried by the poller). Co-Authored-By: Claude Fable 5.1 * docs(git-sync): check-row-before-create order, retarget events; EE ref 40079e0 Co-Authored-By: Claude Fable 5.1 * chore: update ee-repo-ref to a4da009a5eae72bd55f34de41ba7929b53d53c9b This commit updates the EE repository reference after PR #662 was merged in windmill-ee-private. Previous ee-repo-ref: 4845e823bfaa28d3258b4c2fb1e9e2781dde382f New ee-repo-ref: a4da009a5eae72bd55f34de41ba7929b53d53c9b Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: windmill-internal-app[bot] --- ...99107f989d50ea651546e964670cf99fd2de6.json | 19 +++ ...a8605c3851b48bab4964c380538ada93f06a9.json | 40 +++++ ...be02fe144a2da1ebf446b0df92944da791fa7.json | 28 ++++ ...3599f083dee7415acdbe4410412309dfa2ca1.json | 23 +++ ...884932eab27f53171c12291cca15a6ec33586.json | 15 ++ ...c0fa50b81d46845c400d569d6d8e49e503b59.json | 17 ++ ...3a456eef081d16c2a58e7262f86d80289c2da.json | 26 ++++ ...09de2846c43ee48f047e0f2ea9ee7a6502c81.json | 25 +++ ...fe4c1c745457d463d8ea61627ebe61ba8ab2c.json | 17 ++ ...2193e094ca44642342778fb4281a711263385.json | 18 +++ ...864e09c7a5a421ba99647570c2b557d53aa51.json | 17 ++ ...0c4e1612928737a5fb2b4ff1336d8827f788b.json | 37 +++++ ...ce00cc1dbd4921a9ccee8f906515286190b8f.json | 25 +++ ...6dd175aeab87f6377f6ac7b51956ce6d5f039.json | 17 ++ ...2cc25020b11882fb359b136466c82b040f8a1.json | 14 ++ ...447eff928c4518fd2f6800145bf1211086352.json | 28 ++++ ...6c920aa2e14365e63ef8c28c7f65da7c6c9ab.json | 17 ++ ...d203ea689c1e4b094a2c90eb90c8e4b6e0dff.json | 24 +++ ...87a438b619eb1919c028c6c80e972b4ae438d.json | 25 +++ ...3b36afde836fbc6db046279b015bea6f70201.json | 22 +++ ...8257c07785c8b3123f13d59ee361b4ee0bc0a.json | 22 +++ ...b95a942f71651a4f34e671f25ba6816a506c8.json | 22 +++ ...20ddfc414c7e741a171fb80673c23644e3619.json | 50 ++++++ ...ca5a99addffab3dd44529ce9f40de115b0a3c.json | 14 ++ ...c67efbe6b0559adb891dba65d8b8e1430f357.json | 22 +++ ...05879d507b9697eed7e5e232f8e6cb95c2bd0.json | 19 +++ ...f84d0ec7b6cb9375e30b10415605cba9b2fcb.json | 66 ++++++++ backend/Cargo.lock | 1 + backend/ee-repo-ref.txt | 2 +- ...142042_add_git_sync_ci_test_check.down.sql | 1 + ...14142042_add_git_sync_ci_test_check.up.sql | 39 +++++ ...09092950_add_git_sync_synced_head.down.sql | 1 + ...0909092950_add_git_sync_synced_head.up.sql | 24 +++ backend/src/monitor.rs | 8 + backend/summarized_schema.txt | 5 + backend/windmill-api-flows/src/flows.rs | 23 +++ backend/windmill-common/src/workspaces.rs | 9 +- backend/windmill-dep-map/src/ci_tests.rs | 10 ++ backend/windmill-git-sync/Cargo.toml | 3 +- backend/windmill-git-sync/src/lib.rs | 11 +- backend/windmill-store/src/resources.rs | 19 ++- .../windmill-worker/src/result_processor.rs | 145 +++++++++++++++++- docs/git-sync-pull-design.md | 87 +++++++++++ .../git_sync/GitSyncRepositoryCard.svelte | 4 +- frontend/src/lib/hubPaths.json | 2 +- 45 files changed, 1047 insertions(+), 16 deletions(-) create mode 100644 backend/.sqlx/query-16c6e24ae06b52feed597a0c3d299107f989d50ea651546e964670cf99fd2de6.json create mode 100644 backend/.sqlx/query-18ba139acef81d4de18bf21755fa8605c3851b48bab4964c380538ada93f06a9.json create mode 100644 backend/.sqlx/query-1b5f6620d35dd74b32ce6325891be02fe144a2da1ebf446b0df92944da791fa7.json create mode 100644 backend/.sqlx/query-35783f52031d7ba14142108480b3599f083dee7415acdbe4410412309dfa2ca1.json create mode 100644 backend/.sqlx/query-422490f2f91b4d97331e87da135884932eab27f53171c12291cca15a6ec33586.json create mode 100644 backend/.sqlx/query-45d0e716fa402a63b0bf6877c21c0fa50b81d46845c400d569d6d8e49e503b59.json create mode 100644 backend/.sqlx/query-4dde939e92f5b8a9cc165c9ea383a456eef081d16c2a58e7262f86d80289c2da.json create mode 100644 backend/.sqlx/query-4e4b31e97f0cc946f26cc0faf9a09de2846c43ee48f047e0f2ea9ee7a6502c81.json create mode 100644 backend/.sqlx/query-5fcaf17e24fa00ffafdc5f0f425fe4c1c745457d463d8ea61627ebe61ba8ab2c.json create mode 100644 backend/.sqlx/query-6e3cd83ad7eef0dddacf9359f662193e094ca44642342778fb4281a711263385.json create mode 100644 backend/.sqlx/query-7a0ddb6821d8f628bcf85f786e5864e09c7a5a421ba99647570c2b557d53aa51.json create mode 100644 backend/.sqlx/query-89a7f413f6f37aeb7e777faeebb0c4e1612928737a5fb2b4ff1336d8827f788b.json create mode 100644 backend/.sqlx/query-8efe5509034327c202cb3fdd409ce00cc1dbd4921a9ccee8f906515286190b8f.json create mode 100644 backend/.sqlx/query-98b036be15cbd5efbaf2420feb56dd175aeab87f6377f6ac7b51956ce6d5f039.json create mode 100644 backend/.sqlx/query-a7d5a7b6b3bb88f5f7926da577f2cc25020b11882fb359b136466c82b040f8a1.json create mode 100644 backend/.sqlx/query-a970bbf4d3d064614bc47d438a0447eff928c4518fd2f6800145bf1211086352.json create mode 100644 backend/.sqlx/query-b7c72ecebf6818d4e60a02edb986c920aa2e14365e63ef8c28c7f65da7c6c9ab.json create mode 100644 backend/.sqlx/query-bafee32cbff8bb7fff26a241d9ad203ea689c1e4b094a2c90eb90c8e4b6e0dff.json create mode 100644 backend/.sqlx/query-c38a1cf8d2a8fd89008a98f03ab87a438b619eb1919c028c6c80e972b4ae438d.json create mode 100644 backend/.sqlx/query-cfbe6784b3d108f935ba884fe2d3b36afde836fbc6db046279b015bea6f70201.json create mode 100644 backend/.sqlx/query-d644b9cd3407e58f235cc2e97558257c07785c8b3123f13d59ee361b4ee0bc0a.json create mode 100644 backend/.sqlx/query-dfffd6573a1eab11c0515805f85b95a942f71651a4f34e671f25ba6816a506c8.json create mode 100644 backend/.sqlx/query-e64ec4941cbbee016c14d958b7220ddfc414c7e741a171fb80673c23644e3619.json create mode 100644 backend/.sqlx/query-e6f2a6fa47bf3b5c774d6bc6060ca5a99addffab3dd44529ce9f40de115b0a3c.json create mode 100644 backend/.sqlx/query-ea397add5eb6555457883e5b6bdc67efbe6b0559adb891dba65d8b8e1430f357.json create mode 100644 backend/.sqlx/query-f1282393a95b499f1a9fce5939205879d507b9697eed7e5e232f8e6cb95c2bd0.json create mode 100644 backend/.sqlx/query-f6a2a8fbc22c69fd5da86626372f84d0ec7b6cb9375e30b10415605cba9b2fcb.json create mode 100644 backend/migrations/20260714142042_add_git_sync_ci_test_check.down.sql create mode 100644 backend/migrations/20260714142042_add_git_sync_ci_test_check.up.sql create mode 100644 backend/migrations/20260909092950_add_git_sync_synced_head.down.sql create mode 100644 backend/migrations/20260909092950_add_git_sync_synced_head.up.sql diff --git a/backend/.sqlx/query-16c6e24ae06b52feed597a0c3d299107f989d50ea651546e964670cf99fd2de6.json b/backend/.sqlx/query-16c6e24ae06b52feed597a0c3d299107f989d50ea651546e964670cf99fd2de6.json new file mode 100644 index 0000000000..f5bb8e14d1 --- /dev/null +++ b/backend/.sqlx/query-16c6e24ae06b52feed597a0c3d299107f989d50ea651546e964670cf99fd2de6.json @@ -0,0 +1,19 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO git_sync_ci_test_check\n (workspace_id, poster_workspace_id, head_sha, head_ref, repo_url,\n repo_resource_path, check_run_id,\n created_at, concluded, conclusion, concluded_at, github_posted)\n VALUES ($1, $2, $3, $4, $5, $6, NULL, now(), false, NULL, NULL, false)\n ON CONFLICT (workspace_id, repo_resource_path, head_sha) DO UPDATE SET\n poster_workspace_id = EXCLUDED.poster_workspace_id,\n head_ref = EXCLUDED.head_ref,\n repo_url = EXCLUDED.repo_url,\n check_run_id = NULL,\n created_at = now(),\n concluded = false,\n conclusion = NULL,\n concluded_at = NULL,\n github_posted = false", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Varchar", + "Varchar", + "Varchar", + "Text", + "Varchar" + ] + }, + "nullable": [] + }, + "hash": "16c6e24ae06b52feed597a0c3d299107f989d50ea651546e964670cf99fd2de6" +} diff --git a/backend/.sqlx/query-18ba139acef81d4de18bf21755fa8605c3851b48bab4964c380538ada93f06a9.json b/backend/.sqlx/query-18ba139acef81d4de18bf21755fa8605c3851b48bab4964c380538ada93f06a9.json new file mode 100644 index 0000000000..8647442530 --- /dev/null +++ b/backend/.sqlx/query-18ba139acef81d4de18bf21755fa8605c3851b48bab4964c380538ada93f06a9.json @@ -0,0 +1,40 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT test_script_path, tested_item_path, tested_item_kind, has_wildcard AS \"has_wildcard!\" FROM ci_test_reference WHERE workspace_id = $1 ORDER BY test_script_path, tested_item_kind, tested_item_path", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "test_script_path", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "tested_item_path", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "tested_item_kind", + "type_info": "Varchar" + }, + { + "ordinal": 3, + "name": "has_wildcard!", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + false, + false, + true + ] + }, + "hash": "18ba139acef81d4de18bf21755fa8605c3851b48bab4964c380538ada93f06a9" +} diff --git a/backend/.sqlx/query-1b5f6620d35dd74b32ce6325891be02fe144a2da1ebf446b0df92944da791fa7.json b/backend/.sqlx/query-1b5f6620d35dd74b32ce6325891be02fe144a2da1ebf446b0df92944da791fa7.json new file mode 100644 index 0000000000..a78e343072 --- /dev/null +++ b/backend/.sqlx/query-1b5f6620d35dd74b32ce6325891be02fe144a2da1ebf446b0df92944da791fa7.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT u.username, u.email FROM workspace w JOIN usr u ON u.workspace_id = w.id AND u.email = w.owner WHERE w.id = $1 AND NOT u.disabled", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "username", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "email", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + false + ] + }, + "hash": "1b5f6620d35dd74b32ce6325891be02fe144a2da1ebf446b0df92944da791fa7" +} diff --git a/backend/.sqlx/query-35783f52031d7ba14142108480b3599f083dee7415acdbe4410412309dfa2ca1.json b/backend/.sqlx/query-35783f52031d7ba14142108480b3599f083dee7415acdbe4410412309dfa2ca1.json new file mode 100644 index 0000000000..9f9cf036fb --- /dev/null +++ b/backend/.sqlx/query-35783f52031d7ba14142108480b3599f083dee7415acdbe4410412309dfa2ca1.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT r->>'script_path' as \"script_path\"\n FROM workspace_settings ws,\n jsonb_array_elements(\n CASE WHEN jsonb_typeof(ws.git_sync->'repositories') = 'array'\n THEN ws.git_sync->'repositories' END\n ) r\n WHERE ws.workspace_id = $1\n AND r->>'git_repo_resource_path' IN ($2, '$res:' || $2)\n LIMIT 1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "script_path", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "35783f52031d7ba14142108480b3599f083dee7415acdbe4410412309dfa2ca1" +} diff --git a/backend/.sqlx/query-422490f2f91b4d97331e87da135884932eab27f53171c12291cca15a6ec33586.json b/backend/.sqlx/query-422490f2f91b4d97331e87da135884932eab27f53171c12291cca15a6ec33586.json new file mode 100644 index 0000000000..2d0bb40c18 --- /dev/null +++ b/backend/.sqlx/query-422490f2f91b4d97331e87da135884932eab27f53171c12291cca15a6ec33586.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE git_sync_ci_test_check\n SET concluded = true, github_posted = true, concluded_at = now(),\n conclusion = COALESCE(conclusion, 'failure')\n WHERE (check_run_id IS NULL AND NOT concluded\n AND created_at < now() - make_interval(secs => $1))\n OR (concluded AND NOT github_posted\n AND concluded_at < now() - make_interval(secs => $2))", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Float8", + "Float8" + ] + }, + "nullable": [] + }, + "hash": "422490f2f91b4d97331e87da135884932eab27f53171c12291cca15a6ec33586" +} diff --git a/backend/.sqlx/query-45d0e716fa402a63b0bf6877c21c0fa50b81d46845c400d569d6d8e49e503b59.json b/backend/.sqlx/query-45d0e716fa402a63b0bf6877c21c0fa50b81d46845c400d569d6d8e49e503b59.json new file mode 100644 index 0000000000..526f3f5537 --- /dev/null +++ b/backend/.sqlx/query-45d0e716fa402a63b0bf6877c21c0fa50b81d46845c400d569d6d8e49e503b59.json @@ -0,0 +1,17 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE git_sync_synced_head SET tests_dispatched_at = NULL\n WHERE workspace_id = $1 AND repo_resource_path = $2 AND branch = $3 AND sha = $4", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "45d0e716fa402a63b0bf6877c21c0fa50b81d46845c400d569d6d8e49e503b59" +} diff --git a/backend/.sqlx/query-4dde939e92f5b8a9cc165c9ea383a456eef081d16c2a58e7262f86d80289c2da.json b/backend/.sqlx/query-4dde939e92f5b8a9cc165c9ea383a456eef081d16c2a58e7262f86d80289c2da.json new file mode 100644 index 0000000000..8c784b664e --- /dev/null +++ b/backend/.sqlx/query-4dde939e92f5b8a9cc165c9ea383a456eef081d16c2a58e7262f86d80289c2da.json @@ -0,0 +1,26 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE git_sync_synced_head SET tests_dispatched_at = now()\n WHERE workspace_id = $1 AND repo_resource_path = $2 AND branch = $3 AND sha = $4\n AND (tests_dispatched_at IS NULL\n OR (ci_test_job_ids IS NULL\n AND tests_dispatched_at < now() - make_interval(secs => $5)))\n RETURNING true as \"claimed!\"", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "claimed!", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + "Text", + "Float8" + ] + }, + "nullable": [ + null + ] + }, + "hash": "4dde939e92f5b8a9cc165c9ea383a456eef081d16c2a58e7262f86d80289c2da" +} diff --git a/backend/.sqlx/query-4e4b31e97f0cc946f26cc0faf9a09de2846c43ee48f047e0f2ea9ee7a6502c81.json b/backend/.sqlx/query-4e4b31e97f0cc946f26cc0faf9a09de2846c43ee48f047e0f2ea9ee7a6502c81.json new file mode 100644 index 0000000000..c8b8b94da4 --- /dev/null +++ b/backend/.sqlx/query-4e4b31e97f0cc946f26cc0faf9a09de2846c43ee48f047e0f2ea9ee7a6502c81.json @@ -0,0 +1,25 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT (\n SELECT h.sha FROM git_sync_synced_head h\n WHERE h.workspace_id = $1 AND h.repo_resource_path = $4 AND h.branch = $3\n ORDER BY h.synced_at DESC LIMIT 1\n ) = $2 AND NOT EXISTS (\n SELECT 1\n FROM v2_job_queue q\n JOIN v2_job j ON j.id = q.id\n WHERE q.workspace_id = $1\n AND j.kind = 'deploymentcallback'\n AND j.args->'__git_sync_auto_pull'->>'branch' = $3\n AND j.args->'__git_sync_auto_pull'->>'repo_resource_path'\n IN ($4, '$res:' || $4)\n ) AND NOT EXISTS (\n SELECT 1\n FROM v2_job_queue q\n JOIN v2_job j ON j.id = q.id\n WHERE q.workspace_id = $1\n AND j.kind IN ('dependencies', 'flowdependencies', 'appdependencies')\n ) as \"ready\"", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "ready", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "4e4b31e97f0cc946f26cc0faf9a09de2846c43ee48f047e0f2ea9ee7a6502c81" +} diff --git a/backend/.sqlx/query-5fcaf17e24fa00ffafdc5f0f425fe4c1c745457d463d8ea61627ebe61ba8ab2c.json b/backend/.sqlx/query-5fcaf17e24fa00ffafdc5f0f425fe4c1c745457d463d8ea61627ebe61ba8ab2c.json new file mode 100644 index 0000000000..53a3b04d6b --- /dev/null +++ b/backend/.sqlx/query-5fcaf17e24fa00ffafdc5f0f425fe4c1c745457d463d8ea61627ebe61ba8ab2c.json @@ -0,0 +1,17 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE git_sync_synced_head SET ci_test_job_ids = NULL, tests_dispatched_at = NULL\n WHERE workspace_id = $1 AND repo_resource_path = $2 AND branch = $3 AND sha = $4", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "5fcaf17e24fa00ffafdc5f0f425fe4c1c745457d463d8ea61627ebe61ba8ab2c" +} diff --git a/backend/.sqlx/query-6e3cd83ad7eef0dddacf9359f662193e094ca44642342778fb4281a711263385.json b/backend/.sqlx/query-6e3cd83ad7eef0dddacf9359f662193e094ca44642342778fb4281a711263385.json new file mode 100644 index 0000000000..427ba00075 --- /dev/null +++ b/backend/.sqlx/query-6e3cd83ad7eef0dddacf9359f662193e094ca44642342778fb4281a711263385.json @@ -0,0 +1,18 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE git_sync_synced_head SET ci_test_job_ids = $5\n WHERE workspace_id = $1 AND repo_resource_path = $2 AND branch = $3 AND sha = $4", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + "Text", + "UuidArray" + ] + }, + "nullable": [] + }, + "hash": "6e3cd83ad7eef0dddacf9359f662193e094ca44642342778fb4281a711263385" +} diff --git a/backend/.sqlx/query-7a0ddb6821d8f628bcf85f786e5864e09c7a5a421ba99647570c2b557d53aa51.json b/backend/.sqlx/query-7a0ddb6821d8f628bcf85f786e5864e09c7a5a421ba99647570c2b557d53aa51.json new file mode 100644 index 0000000000..b7cccbfad4 --- /dev/null +++ b/backend/.sqlx/query-7a0ddb6821d8f628bcf85f786e5864e09c7a5a421ba99647570c2b557d53aa51.json @@ -0,0 +1,17 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE git_sync_ci_test_check SET github_posted = true\n WHERE workspace_id = $1 AND repo_resource_path = $4 AND head_sha = $2\n AND check_run_id = $3", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Int8", + "Text" + ] + }, + "nullable": [] + }, + "hash": "7a0ddb6821d8f628bcf85f786e5864e09c7a5a421ba99647570c2b557d53aa51" +} diff --git a/backend/.sqlx/query-89a7f413f6f37aeb7e777faeebb0c4e1612928737a5fb2b4ff1336d8827f788b.json b/backend/.sqlx/query-89a7f413f6f37aeb7e777faeebb0c4e1612928737a5fb2b4ff1336d8827f788b.json new file mode 100644 index 0000000000..e0d369fd12 --- /dev/null +++ b/backend/.sqlx/query-89a7f413f6f37aeb7e777faeebb0c4e1612928737a5fb2b4ff1336d8827f788b.json @@ -0,0 +1,37 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE git_sync_ci_test_check\n SET concluded = true, conclusion = $3, concluded_at = now()\n WHERE workspace_id = $1 AND repo_resource_path = $4 AND head_sha = $2 AND NOT concluded\n RETURNING check_run_id, poster_workspace_id, repo_url", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "check_run_id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "poster_workspace_id", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "repo_url", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + "Text" + ] + }, + "nullable": [ + true, + false, + false + ] + }, + "hash": "89a7f413f6f37aeb7e777faeebb0c4e1612928737a5fb2b4ff1336d8827f788b" +} diff --git a/backend/.sqlx/query-8efe5509034327c202cb3fdd409ce00cc1dbd4921a9ccee8f906515286190b8f.json b/backend/.sqlx/query-8efe5509034327c202cb3fdd409ce00cc1dbd4921a9ccee8f906515286190b8f.json new file mode 100644 index 0000000000..db3362bf0c --- /dev/null +++ b/backend/.sqlx/query-8efe5509034327c202cb3fdd409ce00cc1dbd4921a9ccee8f906515286190b8f.json @@ -0,0 +1,25 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT ci_test_job_ids\n FROM git_sync_synced_head\n WHERE workspace_id = $1 AND repo_resource_path = $2 AND branch = $3 AND sha = $4", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "ci_test_job_ids", + "type_info": "UuidArray" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + "Text" + ] + }, + "nullable": [ + true + ] + }, + "hash": "8efe5509034327c202cb3fdd409ce00cc1dbd4921a9ccee8f906515286190b8f" +} diff --git a/backend/.sqlx/query-98b036be15cbd5efbaf2420feb56dd175aeab87f6377f6ac7b51956ce6d5f039.json b/backend/.sqlx/query-98b036be15cbd5efbaf2420feb56dd175aeab87f6377f6ac7b51956ce6d5f039.json new file mode 100644 index 0000000000..b63a9d294e --- /dev/null +++ b/backend/.sqlx/query-98b036be15cbd5efbaf2420feb56dd175aeab87f6377f6ac7b51956ce6d5f039.json @@ -0,0 +1,17 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE git_sync_synced_head SET tests_dispatched_at = NULL\n WHERE workspace_id = $1 AND repo_resource_path = $2 AND branch = $3 AND sha = $4", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "98b036be15cbd5efbaf2420feb56dd175aeab87f6377f6ac7b51956ce6d5f039" +} diff --git a/backend/.sqlx/query-a7d5a7b6b3bb88f5f7926da577f2cc25020b11882fb359b136466c82b040f8a1.json b/backend/.sqlx/query-a7d5a7b6b3bb88f5f7926da577f2cc25020b11882fb359b136466c82b040f8a1.json new file mode 100644 index 0000000000..26301c39b2 --- /dev/null +++ b/backend/.sqlx/query-a7d5a7b6b3bb88f5f7926da577f2cc25020b11882fb359b136466c82b040f8a1.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM git_sync_ci_test_check c\n WHERE c.concluded AND c.github_posted\n AND c.concluded_at < now() - make_interval(secs => $1)\n AND NOT EXISTS (\n SELECT 1 FROM git_sync_synced_head h\n WHERE h.workspace_id = c.workspace_id\n AND h.repo_resource_path = c.repo_resource_path\n AND h.sha = c.head_sha\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Float8" + ] + }, + "nullable": [] + }, + "hash": "a7d5a7b6b3bb88f5f7926da577f2cc25020b11882fb359b136466c82b040f8a1" +} diff --git a/backend/.sqlx/query-a970bbf4d3d064614bc47d438a0447eff928c4518fd2f6800145bf1211086352.json b/backend/.sqlx/query-a970bbf4d3d064614bc47d438a0447eff928c4518fd2f6800145bf1211086352.json new file mode 100644 index 0000000000..258ee9eaa7 --- /dev/null +++ b/backend/.sqlx/query-a970bbf4d3d064614bc47d438a0447eff928c4518fd2f6800145bf1211086352.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT repo_resource_path, head_sha FROM git_sync_ci_test_check\n WHERE workspace_id = $1 AND NOT concluded", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "repo_resource_path", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "head_sha", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + false + ] + }, + "hash": "a970bbf4d3d064614bc47d438a0447eff928c4518fd2f6800145bf1211086352" +} diff --git a/backend/.sqlx/query-b7c72ecebf6818d4e60a02edb986c920aa2e14365e63ef8c28c7f65da7c6c9ab.json b/backend/.sqlx/query-b7c72ecebf6818d4e60a02edb986c920aa2e14365e63ef8c28c7f65da7c6c9ab.json new file mode 100644 index 0000000000..2991d30d1c --- /dev/null +++ b/backend/.sqlx/query-b7c72ecebf6818d4e60a02edb986c920aa2e14365e63ef8c28c7f65da7c6c9ab.json @@ -0,0 +1,17 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE git_sync_ci_test_check\n SET github_posted = github_posted\n AND check_run_id IS NOT DISTINCT FROM GREATEST(check_run_id, $3),\n check_run_id = GREATEST(check_run_id, $3)\n WHERE workspace_id = $1 AND repo_resource_path = $4 AND head_sha = $2", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Int8", + "Text" + ] + }, + "nullable": [] + }, + "hash": "b7c72ecebf6818d4e60a02edb986c920aa2e14365e63ef8c28c7f65da7c6c9ab" +} diff --git a/backend/.sqlx/query-bafee32cbff8bb7fff26a241d9ad203ea689c1e4b094a2c90eb90c8e4b6e0dff.json b/backend/.sqlx/query-bafee32cbff8bb7fff26a241d9ad203ea689c1e4b094a2c90eb90c8e4b6e0dff.json new file mode 100644 index 0000000000..31c628c1a8 --- /dev/null +++ b/backend/.sqlx/query-bafee32cbff8bb7fff26a241d9ad203ea689c1e4b094a2c90eb90c8e4b6e0dff.json @@ -0,0 +1,24 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT EXISTS (\n SELECT 1 FROM git_sync_ci_test_check\n WHERE poster_workspace_id = $1 AND repo_resource_path = $2 AND head_sha = $3\n ) as \"exists!\"", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "exists!", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "bafee32cbff8bb7fff26a241d9ad203ea689c1e4b094a2c90eb90c8e4b6e0dff" +} diff --git a/backend/.sqlx/query-c38a1cf8d2a8fd89008a98f03ab87a438b619eb1919c028c6c80e972b4ae438d.json b/backend/.sqlx/query-c38a1cf8d2a8fd89008a98f03ab87a438b619eb1919c028c6c80e972b4ae438d.json new file mode 100644 index 0000000000..c904dc72fd --- /dev/null +++ b/backend/.sqlx/query-c38a1cf8d2a8fd89008a98f03ab87a438b619eb1919c028c6c80e972b4ae438d.json @@ -0,0 +1,25 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT COUNT(*) as \"count!\"\n FROM git_sync_synced_head h\n JOIN v2_job_completed pc ON pc.id = h.job_id\n JOIN v2_job j ON j.workspace_id = h.workspace_id\n AND j.kind IN ('dependencies', 'flowdependencies', 'appdependencies')\n AND j.created_at >= pc.started_at\n AND j.created_at <= COALESCE(h.tests_dispatched_at, now())\n JOIN v2_job_completed c ON c.id = j.id AND c.status IN ('failure', 'canceled')\n WHERE h.workspace_id = $1 AND h.repo_resource_path = $4\n AND h.branch = $3 AND h.sha = $2\n AND h.source = 'pull'", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "count!", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "c38a1cf8d2a8fd89008a98f03ab87a438b619eb1919c028c6c80e972b4ae438d" +} diff --git a/backend/.sqlx/query-cfbe6784b3d108f935ba884fe2d3b36afde836fbc6db046279b015bea6f70201.json b/backend/.sqlx/query-cfbe6784b3d108f935ba884fe2d3b36afde836fbc6db046279b015bea6f70201.json new file mode 100644 index 0000000000..cb87d3b3f6 --- /dev/null +++ b/backend/.sqlx/query-cfbe6784b3d108f935ba884fe2d3b36afde836fbc6db046279b015bea6f70201.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT args->>'repo_url_resource_path' FROM v2_job WHERE id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "?column?", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Uuid" + ] + }, + "nullable": [ + null + ] + }, + "hash": "cfbe6784b3d108f935ba884fe2d3b36afde836fbc6db046279b015bea6f70201" +} diff --git a/backend/.sqlx/query-d644b9cd3407e58f235cc2e97558257c07785c8b3123f13d59ee361b4ee0bc0a.json b/backend/.sqlx/query-d644b9cd3407e58f235cc2e97558257c07785c8b3123f13d59ee361b4ee0bc0a.json new file mode 100644 index 0000000000..70fd38193c --- /dev/null +++ b/backend/.sqlx/query-d644b9cd3407e58f235cc2e97558257c07785c8b3123f13d59ee361b4ee0bc0a.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT DISTINCT path FROM script WHERE workspace_id = $1 AND deleted = false AND archived = false", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "path", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "d644b9cd3407e58f235cc2e97558257c07785c8b3123f13d59ee361b4ee0bc0a" +} diff --git a/backend/.sqlx/query-dfffd6573a1eab11c0515805f85b95a942f71651a4f34e671f25ba6816a506c8.json b/backend/.sqlx/query-dfffd6573a1eab11c0515805f85b95a942f71651a4f34e671f25ba6816a506c8.json new file mode 100644 index 0000000000..9129b657d3 --- /dev/null +++ b/backend/.sqlx/query-dfffd6573a1eab11c0515805f85b95a942f71651a4f34e671f25ba6816a506c8.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT COUNT(*) as \"count!\" FROM v2_job WHERE id = ANY($1::uuid[])", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "count!", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [ + "UuidArray" + ] + }, + "nullable": [ + null + ] + }, + "hash": "dfffd6573a1eab11c0515805f85b95a942f71651a4f34e671f25ba6816a506c8" +} diff --git a/backend/.sqlx/query-e64ec4941cbbee016c14d958b7220ddfc414c7e741a171fb80673c23644e3619.json b/backend/.sqlx/query-e64ec4941cbbee016c14d958b7220ddfc414c7e741a171fb80673c23644e3619.json new file mode 100644 index 0000000000..cf4d577dbb --- /dev/null +++ b/backend/.sqlx/query-e64ec4941cbbee016c14d958b7220ddfc414c7e741a171fb80673c23644e3619.json @@ -0,0 +1,50 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT workspace_id, poster_workspace_id, head_sha, repo_url, repo_resource_path,\n check_run_id\n FROM git_sync_ci_test_check\n WHERE NOT concluded OR NOT github_posted", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "workspace_id", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "poster_workspace_id", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "head_sha", + "type_info": "Varchar" + }, + { + "ordinal": 3, + "name": "repo_url", + "type_info": "Text" + }, + { + "ordinal": 4, + "name": "repo_resource_path", + "type_info": "Varchar" + }, + { + "ordinal": 5, + "name": "check_run_id", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + false, + false, + false, + false, + false, + true + ] + }, + "hash": "e64ec4941cbbee016c14d958b7220ddfc414c7e741a171fb80673c23644e3619" +} diff --git a/backend/.sqlx/query-e6f2a6fa47bf3b5c774d6bc6060ca5a99addffab3dd44529ce9f40de115b0a3c.json b/backend/.sqlx/query-e6f2a6fa47bf3b5c774d6bc6060ca5a99addffab3dd44529ce9f40de115b0a3c.json new file mode 100644 index 0000000000..f8c42256bb --- /dev/null +++ b/backend/.sqlx/query-e6f2a6fa47bf3b5c774d6bc6060ca5a99addffab3dd44529ce9f40de115b0a3c.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM git_sync_synced_head h\n WHERE h.synced_at < now() - make_interval(secs => $1)\n AND EXISTS (\n SELECT 1 FROM git_sync_synced_head n\n WHERE n.workspace_id = h.workspace_id\n AND n.repo_resource_path = h.repo_resource_path\n AND n.branch = h.branch\n AND n.synced_at > h.synced_at\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Float8" + ] + }, + "nullable": [] + }, + "hash": "e6f2a6fa47bf3b5c774d6bc6060ca5a99addffab3dd44529ce9f40de115b0a3c" +} diff --git a/backend/.sqlx/query-ea397add5eb6555457883e5b6bdc67efbe6b0559adb891dba65d8b8e1430f357.json b/backend/.sqlx/query-ea397add5eb6555457883e5b6bdc67efbe6b0559adb891dba65d8b8e1430f357.json new file mode 100644 index 0000000000..5d0e560ced --- /dev/null +++ b/backend/.sqlx/query-ea397add5eb6555457883e5b6bdc67efbe6b0559adb891dba65d8b8e1430f357.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT COALESCE(jc.status::text, 'running') as \"status!\"\n FROM unnest($1::uuid[]) AS run(id)\n LEFT JOIN v2_job_completed jc ON jc.id = run.id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "status!", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "UuidArray" + ] + }, + "nullable": [ + null + ] + }, + "hash": "ea397add5eb6555457883e5b6bdc67efbe6b0559adb891dba65d8b8e1430f357" +} diff --git a/backend/.sqlx/query-f1282393a95b499f1a9fce5939205879d507b9697eed7e5e232f8e6cb95c2bd0.json b/backend/.sqlx/query-f1282393a95b499f1a9fce5939205879d507b9697eed7e5e232f8e6cb95c2bd0.json new file mode 100644 index 0000000000..7020c21b0d --- /dev/null +++ b/backend/.sqlx/query-f1282393a95b499f1a9fce5939205879d507b9697eed7e5e232f8e6cb95c2bd0.json @@ -0,0 +1,19 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO git_sync_synced_head\n (workspace_id, repo_resource_path, branch, sha, source, job_id)\n VALUES ($1, $2, $3, $4, $5, $6)\n ON CONFLICT (workspace_id, repo_resource_path, branch, sha)\n DO UPDATE SET source = EXCLUDED.source, job_id = EXCLUDED.job_id, synced_at = now()", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Varchar", + "Varchar", + "Varchar", + "Varchar", + "Uuid" + ] + }, + "nullable": [] + }, + "hash": "f1282393a95b499f1a9fce5939205879d507b9697eed7e5e232f8e6cb95c2bd0" +} diff --git a/backend/.sqlx/query-f6a2a8fbc22c69fd5da86626372f84d0ec7b6cb9375e30b10415605cba9b2fcb.json b/backend/.sqlx/query-f6a2a8fbc22c69fd5da86626372f84d0ec7b6cb9375e30b10415605cba9b2fcb.json new file mode 100644 index 0000000000..dda976de02 --- /dev/null +++ b/backend/.sqlx/query-f6a2a8fbc22c69fd5da86626372f84d0ec7b6cb9375e30b10415605cba9b2fcb.json @@ -0,0 +1,66 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT repo_url, check_run_id, poster_workspace_id, conclusion,\n created_at, concluded, github_posted, head_ref\n FROM git_sync_ci_test_check\n WHERE workspace_id = $1 AND repo_resource_path = $3 AND head_sha = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "repo_url", + "type_info": "Text" + }, + { + "ordinal": 1, + "name": "check_run_id", + "type_info": "Int8" + }, + { + "ordinal": 2, + "name": "poster_workspace_id", + "type_info": "Varchar" + }, + { + "ordinal": 3, + "name": "conclusion", + "type_info": "Text" + }, + { + "ordinal": 4, + "name": "created_at", + "type_info": "Timestamptz" + }, + { + "ordinal": 5, + "name": "concluded", + "type_info": "Bool" + }, + { + "ordinal": 6, + "name": "github_posted", + "type_info": "Bool" + }, + { + "ordinal": 7, + "name": "head_ref", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [ + false, + true, + false, + true, + false, + false, + false, + false + ] + }, + "hash": "f6a2a8fbc22c69fd5da86626372f84d0ec7b6cb9375e30b10415605cba9b2fcb" +} diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 665ac56988..a1a358c4e4 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -15727,6 +15727,7 @@ dependencies = [ "tracing", "uuid", "windmill-common", + "windmill-dep-map", "windmill-queue", ] diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index cdd93efb54..9f9388b41a 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -be7262ca144933128cc7924e418c88ffe4e5a6ef +a4da009a5eae72bd55f34de41ba7929b53d53c9b diff --git a/backend/migrations/20260714142042_add_git_sync_ci_test_check.down.sql b/backend/migrations/20260714142042_add_git_sync_ci_test_check.down.sql new file mode 100644 index 0000000000..b21e2bb474 --- /dev/null +++ b/backend/migrations/20260714142042_add_git_sync_ci_test_check.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS git_sync_ci_test_check; diff --git a/backend/migrations/20260714142042_add_git_sync_ci_test_check.up.sql b/backend/migrations/20260714142042_add_git_sync_ci_test_check.up.sql new file mode 100644 index 0000000000..59fabb890c --- /dev/null +++ b/backend/migrations/20260714142042_add_git_sync_ci_test_check.up.sql @@ -0,0 +1,39 @@ +-- One "Windmill CI tests" GitHub check run per (fork workspace, repository, PR head commit): +-- the pull_request webhook opens the check in_progress and it is concluded once +-- the fork's CI tests settle, so the results can gate a GitHub PR. +CREATE TABLE git_sync_ci_test_check ( + -- The fork workspace whose CI tests gate the PR: keys the row, and its `ci_test` + -- jobs are what the check reflects. + workspace_id VARCHAR(50) NOT NULL REFERENCES workspace(id) ON DELETE CASCADE, + head_sha VARCHAR(64) NOT NULL, + -- The PR's head branch: the check waits until the fork's synced state for this + -- branch (written by its pushes and pulls alike) names `head_sha`. + head_ref VARCHAR(255) NOT NULL, + -- The workspace whose git host credential posts the check: the one that received + -- the pull request webhook (the parent owning the repo hook). + poster_workspace_id VARCHAR(50) NOT NULL REFERENCES workspace(id) ON DELETE CASCADE, + repo_url TEXT NOT NULL, + -- The fork's copy of the repository resource: keys the synced-head lookup, since a + -- fork syncing two repositories names its branch identically in both. + repo_resource_path VARCHAR(255) NOT NULL, + -- NULL when the GitHub check-run creation failed; the poller retries the create. + check_run_id BIGINT, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + concluded BOOLEAN NOT NULL DEFAULT false, + conclusion TEXT, + concluded_at TIMESTAMPTZ, + -- Decoupled from `concluded` so a failed check-run PATCH is retried by the + -- poller instead of hanging a required check on GitHub. + github_posted BOOLEAN NOT NULL DEFAULT false, + PRIMARY KEY (workspace_id, repo_resource_path, head_sha) +); + +-- Rows still needing action (create retry, conclusion, timeout, delivery retry). +-- A row drops out only once it is both concluded and delivered to GitHub, so the +-- per-job conclusion hook and the poller sweeper both scan a small live set. +CREATE INDEX idx_git_sync_ci_test_check_pending + ON git_sync_ci_test_check (workspace_id) + WHERE NOT concluded OR NOT github_posted; + +GRANT ALL ON git_sync_ci_test_check TO windmill_user; +GRANT ALL ON git_sync_ci_test_check TO windmill_admin; diff --git a/backend/migrations/20260909092950_add_git_sync_synced_head.down.sql b/backend/migrations/20260909092950_add_git_sync_synced_head.down.sql new file mode 100644 index 0000000000..a089085444 --- /dev/null +++ b/backend/migrations/20260909092950_add_git_sync_synced_head.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS git_sync_synced_head; diff --git a/backend/migrations/20260909092950_add_git_sync_synced_head.up.sql b/backend/migrations/20260909092950_add_git_sync_synced_head.up.sql new file mode 100644 index 0000000000..a0c81d2671 --- /dev/null +++ b/backend/migrations/20260909092950_add_git_sync_synced_head.up.sql @@ -0,0 +1,24 @@ +-- One row per commit a workspace has come to reflect on a branch, written when a +-- pull of that commit succeeds or a deploy push produces it. The "Windmill CI +-- tests" PR check reads it to know when a workspace reflects a PR head, and +-- records the head's CI test runs on it. Kept apart from `workspace_settings.git_sync.auto_pull.last_synced_sha`, +-- which decides whether the next poll pulls and is client-round-tripped settings. +CREATE TABLE git_sync_synced_head ( + workspace_id VARCHAR(50) NOT NULL REFERENCES workspace(id) ON DELETE CASCADE, + -- Repository resource path without its `$res:` prefix. + repo_resource_path VARCHAR(255) NOT NULL, + branch VARCHAR(255) NOT NULL, + sha VARCHAR(64) NOT NULL, + -- 'pull' rows name the pull job; 'push' rows the deploy push job. + source VARCHAR(4) NOT NULL CHECK (source IN ('pull', 'push')), + job_id UUID, + synced_at TIMESTAMPTZ NOT NULL DEFAULT now(), + -- The head's own CI test suite, dispatched once the workspace reflects it and its + -- dependency jobs settled: the "Windmill CI tests" check reads exactly these runs. + tests_dispatched_at TIMESTAMPTZ, + ci_test_job_ids UUID[], + PRIMARY KEY (workspace_id, repo_resource_path, branch, sha) +); + +GRANT ALL ON git_sync_synced_head TO windmill_user; +GRANT ALL ON git_sync_synced_head TO windmill_admin; diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 81571bc942..dc8072d684 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -4711,6 +4711,14 @@ pub async fn poll_git_auto_pull(db: &Pool) { { tracing::error!("git auto-pull: advisory unlock failed: {e:#}"); } + + // Backstop for the "Windmill CI tests" checks: retry a failed GitHub create or + // delivery, conclude checks whose tests settled, time out stuck ones, prune old + // rows. Detached and outside the advisory lock: its writes are guarded (claimed + // conclude, greatest-id upsert), it is single-flight, and its GitHub calls must not + // count against the monitor pass's budget. + let db = db.clone(); + tokio::spawn(async move { windmill_git_sync::sweep_ci_test_checks(&db).await }); } #[cfg(feature = "private")] diff --git a/backend/summarized_schema.txt b/backend/summarized_schema.txt index df3e6ddbf8..41dd70ca93 100644 --- a/backend/summarized_schema.txt +++ b/backend/summarized_schema.txt @@ -113,6 +113,11 @@ folder: name(char), workspace_id(char), display_name(char), owners(char), extra_ folder_permission_history: id(bigint), workspace_id(char), folder_name(char), changed_by(char), changed_at(ts), change_type(char), affected(char) FK: (workspace_id, folder_name) -> folder(workspace_id, name) gcp_trigger: gcp_resource_path(char), topic_id(char), subscription_id(char), delivery_type(delivery_mode), delivery_config(jsonb), path(char), script_path(char), is_flow(bool), workspace_id(char), edited_by(char), email(char), edited_at(ts), extra_perms(jsonb), server_id(char), last_server_ping(ts), error(text), subscription_mode(gcp_subscription_mode), error_handler_path(char), error_handler_args(jsonb), retry(jsonb), auto_acknowledge_msg(bool), ack_deadline(int), mode(trigger_mode), labels(text[]) +git_sync_ci_test_check: workspace_id(char), head_sha(char), head_ref(char), poster_workspace_id(char), repo_url(text), repo_resource_path(char), check_run_id(bigint), created_at(timestamptz), concluded(bool), conclusion(text), concluded_at(timestamptz), github_posted(bool) + FK: (workspace_id) -> workspace(id) + FK: (poster_workspace_id) -> workspace(id) +git_sync_synced_head: workspace_id(char), repo_resource_path(char), branch(char), sha(char), source(char), job_id(uuid), synced_at(timestamptz), tests_dispatched_at(timestamptz), ci_test_job_ids(uuid[]) + FK: (workspace_id) -> workspace(id) global_settings: name(char), value(jsonb), updated_at(ts) guest_activity: email(char), workspace_id(char), day(date), last_seen_at(timestamptz), jwt_entry(bool) group_: workspace_id(char), name(char), summary(text), extra_perms(jsonb) diff --git a/backend/windmill-api-flows/src/flows.rs b/backend/windmill-api-flows/src/flows.rs index d985078e71..2093ac5fec 100644 --- a/backend/windmill-api-flows/src/flows.rs +++ b/backend/windmill-api-flows/src/flows.rs @@ -827,6 +827,29 @@ async fn create_flow( WebhookMessage::CreateFlow { workspace: w_id.clone(), path: nf.path.clone() }, ); + // Trigger CI tests for items that reference this flow + { + let db2 = db.clone(); + let w_id2 = w_id.clone(); + let flow_path2 = nf.path.clone(); + let email2 = authed.email.clone(); + let username2 = authed.username.clone(); + tokio::spawn(async move { + if let Err(e) = windmill_dep_map::ci_tests::trigger_ci_tests_for_item( + &db2, + &w_id2, + &flow_path2, + "flow", + &email2, + &username2, + ) + .await + { + tracing::error!(%e, "error triggering CI tests after flow creation"); + } + }); + } + Ok((StatusCode::CREATED, nf.path.to_string())) } diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 45c8695585..ad040c5905 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -191,7 +191,7 @@ pub const LATEST_GIT_SYNC_SCRIPT_PATH: &str = "hub/28958/sync-script-to-git-repo /// ignores the slug, so the slug is kept free of characters that would be /// percent-encoded into the run URL (a `:` becomes `%3A`, which some hardened /// reverse proxies reject as double-encoding when the client re-encodes it). -pub const GIT_SYNC_PULL_SCRIPT_PATH: &str = "hub/28948/git-sync-init-repository-windmill"; +pub const GIT_SYNC_PULL_SCRIPT_PATH: &str = "hub/28957/git-sync-init-repository-windmill"; /// Prefix used to identify fork workspaces. A workspace whose id starts with this string is a /// fork of another workspace. @@ -559,9 +559,10 @@ impl AutoPullSettings { /// Whether a freshly observed `(git_ref, head_sha)` warrants enqueuing a pull. /// /// A trigger (poll or webhook) is only a hint: we pull when auto-pull is - /// enabled and the observed head differs from the last sha we synced for - /// that ref. Re-observing the same head (e.g. a redundant poll, or the - /// commit our own deploy callback just pushed back) is a no-op. + /// enabled and the observed head differs from the last sha we pulled for + /// that ref. Re-observing the same head (a redundant poll) is a no-op. A + /// commit our own deploy pushed is not: pushes never write here, so the pull + /// it triggers picks up anything pushed under it. pub fn should_pull(&self, git_ref: &str, head_sha: &str) -> bool { self.enabled && self.last_synced_sha.get(git_ref).map(String::as_str) != Some(head_sha) } diff --git a/backend/windmill-dep-map/src/ci_tests.rs b/backend/windmill-dep-map/src/ci_tests.rs index 79f1b7e30b..8f56111b35 100644 --- a/backend/windmill-dep-map/src/ci_tests.rs +++ b/backend/windmill-dep-map/src/ci_tests.rs @@ -19,3 +19,13 @@ pub async fn trigger_ci_tests_for_item( ) -> error::Result> { Ok(vec![]) } + +#[cfg(not(feature = "private"))] +pub async fn trigger_all_ci_tests( + _db: &sqlx::Pool, + _w_id: &str, + _email: &str, + _username: &str, +) -> error::Result> { + Ok(vec![]) +} diff --git a/backend/windmill-git-sync/Cargo.toml b/backend/windmill-git-sync/Cargo.toml index 148746dcca..f74581ba67 100644 --- a/backend/windmill-git-sync/Cargo.toml +++ b/backend/windmill-git-sync/Cargo.toml @@ -9,7 +9,7 @@ name = "windmill_git_sync" path = "./src/lib.rs" [features] -private = ["windmill-common/private"] +private = ["windmill-common/private", "windmill-dep-map/private"] enterprise = ["windmill-queue/enterprise", "windmill-common/enterprise"] all_sqlx_features = ["enterprise"] default = [] @@ -22,5 +22,6 @@ serde_json.workspace = true tracing.workspace = true windmill-common = { workspace = true, default-features = false } windmill-queue.workspace = true +windmill-dep-map.workspace = true regex = "1.10.3" tokio = { workspace = true, features = ["full"] } \ No newline at end of file diff --git a/backend/windmill-git-sync/src/lib.rs b/backend/windmill-git-sync/src/lib.rs index ccb14e9028..d65e230988 100644 --- a/backend/windmill-git-sync/src/lib.rs +++ b/backend/windmill-git-sync/src/lib.rs @@ -17,7 +17,16 @@ pub use git_sync_ee::{ clear_auto_pull_failure, enqueue_git_pull_dry_run, enqueue_git_pull_job, handle_deployment_metadata, handle_deployment_metadata_batch, handle_fork_branch_creation, persist_auto_pull_state, reconcile_and_enqueue_pull, reconcile_fork_branch_pull, - record_auto_pull_failure, tally_deployed_object_changes, + record_auto_pull_failure, record_synced_head, sweep_ci_test_checks, + tally_deployed_object_changes, +}; + +// The CI-test check exists only on enterprise builds; `private` alone (the CE image) +// compiles git_sync_ee without them. +#[cfg(all(feature = "private", feature = "enterprise"))] +pub use git_sync_ee::{ + ensure_ci_test_check_for_pr, evaluate_and_conclude_ci_test_checks, + post_ci_test_check_not_applicable, resolve_pr_head_workspace, }; #[cfg(not(feature = "private"))] diff --git a/backend/windmill-store/src/resources.rs b/backend/windmill-store/src/resources.rs index 72513d2d63..126cf97106 100644 --- a/backend/windmill-store/src/resources.rs +++ b/backend/windmill-store/src/resources.rs @@ -1297,9 +1297,26 @@ async fn create_resource( webhook.send_message( w_id.clone(), - WebhookMessage::CreateResource { workspace: w_id, path: resource.path.clone() }, + WebhookMessage::CreateResource { workspace: w_id.clone(), path: resource.path.clone() }, ); + // Trigger CI tests for items that reference this resource + { + let db2 = db.clone(); + let path2 = resource.path.clone(); + let email2 = authed.email.clone(); + let username2 = authed.username.clone(); + tokio::spawn(async move { + if let Err(e) = windmill_dep_map::ci_tests::trigger_ci_tests_for_item( + &db2, &w_id, &path2, "resource", &email2, &username2, + ) + .await + { + tracing::error!(%e, "error triggering CI tests after resource creation"); + } + }); + } + Ok(( StatusCode::CREATED, format!("resource {} created", resource.path), diff --git a/backend/windmill-worker/src/result_processor.rs b/backend/windmill-worker/src/result_processor.rs index fd4f08f5fb..21b83b32ff 100644 --- a/backend/windmill-worker/src/result_processor.rs +++ b/backend/windmill-worker/src/result_processor.rs @@ -983,19 +983,21 @@ mod git_sync_check_tests { } } -/// When an auto-pull job (carrying `__git_sync_auto_pull`) fails, roll the +/// When an auto-pull job (carrying `__git_sync_auto_pull`) completes: on success, +/// record the commit as a head the workspace reflects; on failure, roll the /// optimistic `last_synced_sha` advance back to the pre-pull value so the commit /// is retried instead of being silently treated as synced, and record the failure. +/// The recorded commit is the one the pull script reports having checked out +/// (`{sha, branch}` in its result): the branch can move between the observation +/// the marker holds and the clone. A result without it falls back to the marker. #[cfg(all(feature = "enterprise", feature = "private"))] async fn maybe_reconcile_git_sync_auto_pull( db: &DB, job_id: &uuid::Uuid, workspace_id: &str, success: bool, + result: &str, ) { - if success { - return; // the optimistic synced state is already correct - } let marker: Option = match sqlx::query_scalar!( "SELECT args->'__git_sync_auto_pull' FROM v2_job WHERE id = $1", job_id @@ -1015,12 +1017,50 @@ async fn maybe_reconcile_git_sync_auto_pull( #[derive(serde::Deserialize)] struct AutoPullMarker { repo_resource_path: String, + branch: Option, + head_sha: Option, #[serde(default)] prev_synced: std::collections::HashMap, } let Ok(m) = serde_json::from_value::(marker) else { return; }; + if success { + // The optimistic synced state is already correct; record that the workspace + // now reflects the commit, which the PR CI-test check waits for. + #[derive(serde::Deserialize)] + struct PullResult { + sha: Option, + branch: Option, + } + let applied = serde_json::from_str::(result).ok(); + let branch = applied + .as_ref() + .and_then(|r| r.branch.as_deref()) + .or(m.branch.as_deref()); + let sha = applied + .as_ref() + .and_then(|r| r.sha.as_deref()) + .or(m.head_sha.as_deref()); + if let (Some(branch), Some(sha)) = (branch, sha) { + if let Err(e) = windmill_git_sync::record_synced_head( + db, + workspace_id, + &m.repo_resource_path, + branch, + sha, + "pull", + Some(*job_id), + ) + .await + { + tracing::warn!( + "git auto-pull: failed to record synced head {sha} on {branch}: {e:#}" + ); + } + } + return; + } windmill_git_sync::record_auto_pull_failure( db, workspace_id, @@ -1109,6 +1149,70 @@ fn git_sync_push_result_pushed(result: &str) -> Option { .as_bool() } +/// When a git-sync push job pushed a commit, record it as a head the workspace +/// reflects, the way a successful pull records the commit it applied. The PR +/// CI-test check waits for that record. Best-effort: failures are logged, never +/// propagated. +#[cfg(all(feature = "enterprise", feature = "private"))] +async fn maybe_record_git_sync_pushed_head( + db: &DB, + job_id: &uuid::Uuid, + workspace_id: &str, + result: &str, +) { + #[derive(serde::Deserialize)] + struct PushResult { + pushed: bool, + sha: Option, + branch: Option, + #[serde(default)] + rebased: bool, + } + let Ok(PushResult { pushed: true, sha: Some(sha), branch: Some(branch), rebased }) = + serde_json::from_str::(result) + else { + return; + }; + // A push that had to rebase sits on commits this workspace has not pulled, so the + // pushed head is not something it reflects yet; the pull those commits trigger + // records the head once they are in. + if rebased { + tracing::info!( + "git sync push: {sha} on {branch} was rebased onto unpulled commits; not recording it as synced for {workspace_id}" + ); + return; + } + let repo_path = match sqlx::query_scalar!( + "SELECT args->>'repo_url_resource_path' FROM v2_job WHERE id = $1", + job_id + ) + .fetch_optional(db) + .await + { + Ok(Some(Some(p))) => p, + Ok(_) => return, + Err(e) => { + tracing::error!("git sync push: failed to read job args: {e:#}"); + return; + } + }; + if let Err(e) = windmill_git_sync::record_synced_head( + db, + workspace_id, + &repo_path, + &branch, + &sha, + "push", + Some(*job_id), + ) + .await + { + tracing::warn!( + "git sync push: failed to record pushed head {sha} on {branch} for {workspace_id}/{repo_path}: {e:#}" + ); + } +} + /// When a git-sync push job carrying `__git_sync_open_pr` succeeds, open (or /// reopen) the PR for the branch it pushed: `wm-fork//` for a fork /// deploy, `wm_deploy/**` for a promotion deploy. Runs outbound with the @@ -1707,8 +1811,26 @@ pub async fn process_completed_job( #[cfg(all(feature = "enterprise", feature = "private"))] if job.kind == JobKind::DeploymentCallback { maybe_post_git_sync_check(db, &job_id, &workspace_id, true, result.get()).await; + maybe_reconcile_git_sync_auto_pull(db, &job_id, &workspace_id, true, result.get()) + .await; + maybe_record_git_sync_pushed_head(db, &job_id, &workspace_id, result.get()).await; maybe_open_git_sync_deploy_pr(db, &job_id, &workspace_id, result.get()).await; } + // A CI test job just finished: advance any open "Windmill CI tests" PR check for + // its workspace. Detached, since concluding a check calls GitHub and this loop + // completes jobs serially; the evaluation is idempotent and the poller retries. + #[cfg(all(feature = "enterprise", feature = "private"))] + if job + .trigger_kind + .as_ref() + .is_some_and(|k| k.is(windmill_common::jobs::JobTriggerKind::CiTest)) + { + let db = db.clone(); + let w_id = workspace_id.clone(); + tokio::spawn(async move { + windmill_git_sync::evaluate_and_conclude_ci_test_checks(&db, &w_id).await + }); + } // Asset-trigger fan-out: best-effort, never propagates errors. // Internal eligibility checks gate to top-level Script/Preview runs; @@ -1819,7 +1941,20 @@ pub async fn process_completed_job( #[cfg(all(feature = "enterprise", feature = "private"))] if job.kind == JobKind::DeploymentCallback { maybe_post_git_sync_check(db, &job.id, &job.workspace_id, false, result.get()).await; - maybe_reconcile_git_sync_auto_pull(db, &job.id, &job.workspace_id, false).await; + maybe_reconcile_git_sync_auto_pull(db, &job.id, &job.workspace_id, false, "").await; + } + // A failed CI test job also settles its check; same detached advance as on success. + #[cfg(all(feature = "enterprise", feature = "private"))] + if job + .trigger_kind + .as_ref() + .is_some_and(|k| k.is(windmill_common::jobs::JobTriggerKind::CiTest)) + { + let db = db.clone(); + let w_id = job.workspace_id.clone(); + tokio::spawn(async move { + windmill_git_sync::evaluate_and_conclude_ci_test_checks(&db, &w_id).await + }); } if job.is_flow_step() { if let Some(parent_job) = job.parent_job { diff --git a/docs/git-sync-pull-design.md b/docs/git-sync-pull-design.md index 8961b0f047..5f77a3f117 100644 --- a/docs/git-sync-pull-design.md +++ b/docs/git-sync-pull-design.md @@ -618,6 +618,93 @@ repo's **Environments** timeline ("Production → Deployed"). Needs opt-in / later. The check-run version is the cheap default and matches the visual Cloudflare parity without a new permission. +### Phase 7 — CI test results check (WIN-2051) — implemented + +Surfaces Windmill's own CI tests (the `// test: script/...` annotation) as a +**"Windmill CI tests"** check run on **any PR** against the tracked branch, so a customer +can mark it a **required status check** and have Windmill CI results gate the PR — +replacing the documented GitHub Action that polls `ci_test_results_batch`. GitHub App-backed +only; reuses the Phase 4 `Checks: write` grant, so no new permission. Token repos keep the +Action, and GitLab merge requests get no CI-test surface for the same reason the Phase 4 +preview lives in a note there (a commit status would fail the project's own pipeline). + +Driven by the **`pull_request` webhook** — the same event Phase 4 already reacts to — +rather than the deploy push/pull, so it's uniform across how the PR's commit came to exist +(a fork deploy that pushes `wm-fork/**` and opens the PR, or an external push that gets +pulled in). CI tests run as separate async `ci_test` jobs in the **fork workspace** the PR +corresponds to; the check reflects that fork's current results on the PR head. + +- **State** — `git_sync_ci_test_check(workspace_id, repo_resource_path, head_sha)` (new + table). `workspace_id` is the **fork** whose `ci_test` jobs the check reflects; + `repo_resource_path` the repository (a fork can sync several, and two can hold the same + commit); `poster_workspace_id` is the **parent** whose GitHub-App installation posts the + run (the workspace that received the webhook and owns the repo hook). Plus `repo_url`, + `head_ref`, `check_run_id` (NULL until the create succeeds, and reset to NULL by a re-fired event + for the same head: the row is written first so a create that never gets recorded cannot + strand an in-progress run, and the poller retries any row without an id), `created_at`, + `concluded`, `conclusion`, `concluded_at`, `github_posted`. + Partial index `(workspace_id) WHERE NOT concluded OR NOT github_posted` (the live set the + hook + poller scan). +- **Open** — in the `pull_request` handler (opened/synchronize/reopened, or edited with a + base change, base = tracked): when the head lives in the base repo, resolve the fork + workspace from the head ref (reusing the fork-branch routing; + `resolve_pr_head_workspace`), persist the intent row with a null check-run id, then + `create_check_run` in_progress on `head_sha` via the parent's installation and adopt the + id (the poller retries the create from the row if it failed), then evaluate. An earlier head's open check is left to conclude on its + own (fork verdict or timeout): a late-delivered event for an old head must never touch + the current head's check. +- **Conclude** — the verdict is the head's own suite. Once the fork reflects the head (below) + and its dependency jobs settled, every CI test the fork declares is dispatched once, one + run per `ci_test_reference` row the way a deploy of that item would (`trigger_all_ci_tests`, + as the fork's owner, the user who created it, with no more reach than they have; a + missing or disabled owner concludes the check as failure; and without the per-item + debounce so a deploy-triggered run of the same test cannot supersede a suite run), and the job ids are + recorded on the synced-head row (`ci_test_job_ids`; `tests_dispatched_at` claims the + dispatch so the per-job hook and the poller queue it once, and a claim that never recorded + ids is retaken after 5 min). The verdict is exactly those runs: fail-fast on any + failed/canceled; `success` once all settle ("No CI tests" when the fork declares none); + `skipped` ignored. Nothing older, newer or workspace-wide stands in + for a head's runs, so a re-fired event reads the same runs and gets the same answer, a + test-only change is run because the suite runs on every head, and a deploy in flight in + the fork cannot feed another head's check. Runs purged by job retention reset the row so + the head is re-tested. +- **Readiness** — the suite is dispatched only once the fork reflects the head, so it does + not matter which webhook GitHub delivers first. The evidence is `git_sync_synced_head`: the + pull completion hook writes a row when a pull job succeeds (the pull script reports the + commit its clone checked out; the enqueue-time marker is the fallback), and the push + completion hook writes one from the deploy push script's `{pushed, sha, branch, rebased}` + result (a rebased push sits on unpulled commits and is not recorded). The head is ready + when the repository branch's newest row names it, so a branch reset to an older commit + waits for its re-pull; the prune keeps each repository branch's newest row so a PR reopened + at an unchanged head stays ready. This is a sync event log, deliberately apart from + `auto_pull.last_synced_sha`: that map decides whether the next poll pulls (a push must + never write it, or a commit someone else pushed under ours would be skipped) and it is + client-round-tripped settings. The check row stores `head_ref` for the lookup. Dispatch + also waits while a dependency job in the fork or a pull of the repository branch is queued + (a deploy push lands on whatever the remote held when it cloned, so a commit pushed there + from outside is in the workspace only once its pull ran), and the check fails outright if + a dependency job failed after the head's pull started (the item deployed nothing + runnable). A commit the fork never comes to reflect times out; a timeout on a repository + pinned to a sync script older than the one that reports pushed commits names that as the + reason. Needs the hub script versions that report the sha (`LATEST_GIT_SYNC_SCRIPT_PATH`, + `GIT_SYNC_PULL_SCRIPT_PATH`). +- **Drivers** — a per-`ci_test`-job completion hook (low latency) and the git-sync poller + (the backstop: retries the GitHub create/deliver, times stuck checks out after 30 min, + prunes old rows; runs after the auto-pull advisory lock is released so its GitHub calls + never extend the tick). Both call one idempotent `evaluate_and_conclude`, which claims the + decision with a guarded `UPDATE ... WHERE NOT concluded RETURNING` (exactly-once) and + decouples GitHub delivery via `github_posted` so a failed PATCH is retried, not hung. + +Invariants: only a head in the base repo can map to a workspace (a contributor fork's +branch names mean nothing here); the webhook's workspace posts through its own +installation; the timeout stops a hung test job from blocking a required check forever; +rows cascade away with either workspace. A plain feature-branch or +contributor-fork PR resolves to no fork workspace and gets an already-concluded `skipped` +check (branch protection counts `skipped` as passing, so requiring the check does not block +those PRs). A timeout on a repository pinned to a sync script older than the one that reports +pushed commits names that as the reason. Known limit (accepted for v1): the fork's status is workspace-wide (all its +tested items), which for the one-fork-per-PR model equals the PR's scope. + ## 16. Alternatives considered **Portal as webhook proxy (the rejected "option 2").** Subscribe the managed app diff --git a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte index 515f57ea01..4b4bff2168 100644 --- a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte +++ b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte @@ -949,7 +949,7 @@ options={{ right: 'Automatically deploy changes from Git', rightTooltip: - 'Windmill deploys new commits from the tracked branch into this workspace. Repositories Windmill holds a credential for sync instantly via webhooks with a polling fallback; other token-based repositories are checked about every minute.' + 'Windmill deploys new commits from the tracked branch into this workspace. Repositories Windmill holds a credential for sync instantly via webhooks with a polling fallback; other token-based repositories are checked about every minute. The webhook also delivers pull request events, which is what posts the diff check on pull requests and, for GitHub repositories, the Windmill CI tests check.' }} on:change={(e) => setAutoPullEnabled(e.detail)} > @@ -964,7 +964,7 @@ options={{ right: 'Automatically sync forks with git branches', rightTooltip: repo.auto_pull?.enabled - ? "When a fork's wm-fork/** branch changes in the repository (for example after merging the tracked branch into it), Windmill deploys those commits into the fork workspace. Configured once here, applied to every fork of this workspace." + ? "When a fork's wm-fork/** branch changes in the repository (for example after merging the tracked branch into it), Windmill deploys those commits into the fork workspace. On GitHub, needed for the Windmill CI tests check on a fork pull request whose commits were pushed outside Windmill. Configured once here, applied to every fork of this workspace." : 'Requires automatic deploy from Git to be enabled above.' }} on:change={(e) => setSyncForks(e.detail)} diff --git a/frontend/src/lib/hubPaths.json b/frontend/src/lib/hubPaths.json index 7439ad7ada..dfa0be57c1 100644 --- a/frontend/src/lib/hubPaths.json +++ b/frontend/src/lib/hubPaths.json @@ -1,6 +1,6 @@ { "gitSyncTest": "hub/28950/git-repo-test-read-write-windmill", - "gitInitRepo": "hub/28948/git-sync-init-repository-windmill", + "gitInitRepo": "hub/28957/git-sync-init-repository-windmill", "slackErrorHandler": "hub/28794/workspace-or-schedule-error-handler-slack", "emailErrorHandler": "hub/19795/workspace-or-error-handler-email", "slackRecoveryHandler": "hub/28791/slack/schedule-recovery-handler-slack",