From ad571c62262dad9b2211f442d9e94c0a044ac167 Mon Sep 17 00:00:00 2001 From: AlexRV12 <71396855+AlexRV12@users.noreply.github.com> Date: Wed, 28 Oct 2026 12:10:05 +0100 Subject: [PATCH 01/26] fix: scope rename's group membership update to the user's workspace (#12043) --- ...155e10e909bd02c91082341dd73bb1d6d62e1.json | 15 ------ ...d708f200a52aaf55c7cf40f7b210f97688c15.json | 16 ++++++ .../tests/users.rs | 52 +++++++++++++++++++ backend/windmill-api/src/users.rs | 5 +- 4 files changed, 71 insertions(+), 17 deletions(-) delete mode 100644 backend/.sqlx/query-2f0de5417088df098a200a67197155e10e909bd02c91082341dd73bb1d6d62e1.json create mode 100644 backend/.sqlx/query-af0bcc6aaf455f75cdf44a503eed708f200a52aaf55c7cf40f7b210f97688c15.json diff --git a/backend/.sqlx/query-2f0de5417088df098a200a67197155e10e909bd02c91082341dd73bb1d6d62e1.json b/backend/.sqlx/query-2f0de5417088df098a200a67197155e10e909bd02c91082341dd73bb1d6d62e1.json deleted file mode 100644 index 85f3b49b2d..0000000000 --- a/backend/.sqlx/query-2f0de5417088df098a200a67197155e10e909bd02c91082341dd73bb1d6d62e1.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "UPDATE usr_to_group SET usr = $1 WHERE usr = $2", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Varchar", - "Text" - ] - }, - "nullable": [] - }, - "hash": "2f0de5417088df098a200a67197155e10e909bd02c91082341dd73bb1d6d62e1" -} diff --git a/backend/.sqlx/query-af0bcc6aaf455f75cdf44a503eed708f200a52aaf55c7cf40f7b210f97688c15.json b/backend/.sqlx/query-af0bcc6aaf455f75cdf44a503eed708f200a52aaf55c7cf40f7b210f97688c15.json new file mode 100644 index 0000000000..4c04e5eb0e --- /dev/null +++ b/backend/.sqlx/query-af0bcc6aaf455f75cdf44a503eed708f200a52aaf55c7cf40f7b210f97688c15.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE usr_to_group SET usr = $1 WHERE usr = $2 AND workspace_id = $3", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "af0bcc6aaf455f75cdf44a503eed708f200a52aaf55c7cf40f7b210f97688c15" +} diff --git a/backend/windmill-api-integration-tests/tests/users.rs b/backend/windmill-api-integration-tests/tests/users.rs index 3a52e94e4a..cf9d2c39d5 100644 --- a/backend/windmill-api-integration-tests/tests/users.rs +++ b/backend/windmill-api-integration-tests/tests/users.rs @@ -1160,3 +1160,55 @@ async fn test_set_password_revokes_mcp_oauth_grants(db: Pool) -> anyho Ok(()) } + +/// Without automatic usernames, a username is only unique within a workspace, so renaming one +/// account must leave the memberships of a like-named account elsewhere alone. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_rename_keeps_other_workspaces_like_named_memberships( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + sqlx::raw_sql( + "INSERT INTO workspace (id, name, owner) VALUES ('other-workspace', 'other', 'test-user'); + INSERT INTO usr (workspace_id, email, username, is_admin, role) + VALUES ('other-workspace', 'test3@windmill.dev', 'test-user-2', false, 'User'); + INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES + ('test-workspace', 'ops', '', '{}'), ('other-workspace', 'finance', '', '{}'); + INSERT INTO usr_to_group (workspace_id, group_, usr) VALUES + ('test-workspace', 'ops', 'test-user-2'), + ('other-workspace', 'finance', 'test-user-2');", + ) + .execute(&db) + .await?; + + let resp = authed(client().post(format!( + "http://localhost:{port}/api/users/rename/test2@windmill.dev" + ))) + .json(&json!({ "new_username": "alice" })) + .send() + .await?; + assert_eq!(resp.status(), 200, "rename: {}", resp.text().await?); + + let memberships: Vec<(String, String, String)> = sqlx::query_as( + "SELECT workspace_id, group_, usr FROM usr_to_group + WHERE group_ IN ('ops', 'finance') ORDER BY workspace_id", + ) + .fetch_all(&db) + .await?; + assert_eq!( + memberships, + vec![ + ( + "other-workspace".into(), + "finance".into(), + "test-user-2".into() + ), + ("test-workspace".into(), "ops".into(), "alice".into()), + ] + ); + + Ok(()) +} diff --git a/backend/windmill-api/src/users.rs b/backend/windmill-api/src/users.rs index ecf77b6d5f..4b8d3481c3 100644 --- a/backend/windmill-api/src/users.rs +++ b/backend/windmill-api/src/users.rs @@ -443,9 +443,10 @@ async fn update_username_in_workpsace<'c>( .await?; sqlx::query!( - "UPDATE usr_to_group SET usr = $1 WHERE usr = $2", + "UPDATE usr_to_group SET usr = $1 WHERE usr = $2 AND workspace_id = $3", new_username, - old_username + old_username, + w_id ) .execute(&mut **tx) .await?; From d3869fcdb28a7053644582afe822f3d52d6c2ace Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 28 Oct 2026 13:35:44 +0100 Subject: [PATCH 02/26] feat: gate guest apps by their mode and the protection rule alone (#12061) * feat: gate guest apps by their mode and the protection rule alone Co-Authored-By: Claude Opus 5.5 * fix: keep apps that admitted no guest closed on upgrade Co-Authored-By: Claude Opus 5.5 * chore: update ee-repo-ref to 0605eac866b49f0de913ab41c1168b79eec31ca0 This commit updates the EE repository reference after PR #931 was merged in windmill-ee-private. Previous ee-repo-ref: 1f643a1aaa697578b36d67982358299ee20af7f4 New ee-repo-ref: 0605eac866b49f0de913ab41c1168b79eec31ca0 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: windmill-internal-app[bot] --- ...ce61a985c7c2eed72544acd05316bc002234.json} | 4 +- ...7e9481881fd99c5be43a4e58ce95a3baefeb6.json | 15 ++ ...539dc7e9c440cd1625054d6ee1b67a785418.json} | 14 +- ...2909be7dc7eb9b6deac7ea439eb637d728477.json | 15 -- ...cd21d12fa1207b58bfc46249c250cdcdb5363.json | 232 ------------------ ...7f41c3a72838c5d4c5f795638506084561894.json | 15 -- ...fdddbfdb63c021a4ec93c371c1ae38cb72be.json} | 16 +- ...53aba57d5e8f19a316f76e01922559425d81b.json | 15 -- backend/ee-repo-ref.txt | 2 +- ...1028111747_close_inert_guest_apps.down.sql | 2 + ...261028111747_close_inert_guest_apps.up.sql | 15 ++ backend/tests/app_guest_allowance.rs | 7 - backend/tests/app_guest_cloud_hosted.rs | 21 +- backend/tests/app_guest_execution_mode.rs | 226 ++++------------- backend/tests/app_guest_jwt_allowance.rs | 13 +- backend/tests/app_guest_jwt_entry.rs | 60 ----- backend/windmill-api-auth/src/auth.rs | 20 +- .../windmill-api-workspaces/src/workspaces.rs | 64 +---- .../src/workspaces_extra.rs | 2 +- backend/windmill-api/openapi.yaml | 73 +----- backend/windmill-api/src/apps.rs | 9 +- .../src/mcp/auto_generated_endpoints.rs | 4 +- .../windmill-api/src/mcp/session/deploy.rs | 7 +- .../src/mcp/session/gen/instructions.json | 2 +- .../windmill-api/src/mcp/session/workspace.rs | 13 +- .../src/workspace_settings_document.rs | 62 ++--- .../windmill-common/src/global_settings.rs | 4 - backend/windmill-common/src/workspaces.rs | 79 ++---- cli/src/commands/sync/sync.ts | 6 +- cli/src/core/settings.ts | 6 +- cli/src/guidance/skills.gen.ts | 6 +- docs/operator-builder-rights.md | 4 +- .../components/SuperadminSettingsInner.svelte | 1 - .../apps/editor/AppEditorHeaderDeploy.svelte | 17 +- .../copilot/chat/global/core.test.ts | 57 +---- .../components/copilot/chat/global/core.ts | 21 +- .../instanceSettings/GuestActivityList.svelte | 59 +---- frontend/src/lib/mcpEndpointTools.ts | 4 +- .../(logged)/workspace_settings/+page.svelte | 49 +--- system_prompts/auto-generated/prompts.ts | 2 +- .../skills/workspace-settings/SKILL.md | 6 +- system_prompts/base/workspace-settings.md | 6 +- 42 files changed, 252 insertions(+), 1003 deletions(-) rename backend/.sqlx/{query-06af616fe4fc61a3b0dcd996a2a1e0e9ac63fc8756aeafac8d279841db117eac.json => query-27aed1f0ee2c6f0ec4f2d7e64a67ce61a985c7c2eed72544acd05316bc002234.json} (76%) create mode 100644 backend/.sqlx/query-3ab4e59067a97b74636320f6e237e9481881fd99c5be43a4e58ce95a3baefeb6.json rename backend/.sqlx/{query-af37e5579b729a1aedcd63ab70d189c7e0376ca1f0071ae8360183ed1300d2bb.json => query-3b98b506c544162e630641147490539dc7e9c440cd1625054d6ee1b67a785418.json} (92%) delete mode 100644 backend/.sqlx/query-76c0331b18eed478a50572e35642909be7dc7eb9b6deac7ea439eb637d728477.json delete mode 100644 backend/.sqlx/query-8ebe054b41793f1a7b85f1f8d29cd21d12fa1207b58bfc46249c250cdcdb5363.json delete mode 100644 backend/.sqlx/query-9dabc87e05f72fbd4b586c80d9c7f41c3a72838c5d4c5f795638506084561894.json rename backend/.sqlx/{query-e6e31fdf705896c81f9a0f45d47c1b93db0406aaf278bccba07be00e9f937e2a.json => query-d8b38538f21b533eccc147be0a24fdddbfdb63c021a4ec93c371c1ae38cb72be.json} (76%) delete mode 100644 backend/.sqlx/query-f5022831ab610ab22ef1af1441753aba57d5e8f19a316f76e01922559425d81b.json create mode 100644 backend/migrations/20261028111747_close_inert_guest_apps.down.sql create mode 100644 backend/migrations/20261028111747_close_inert_guest_apps.up.sql diff --git a/backend/.sqlx/query-06af616fe4fc61a3b0dcd996a2a1e0e9ac63fc8756aeafac8d279841db117eac.json b/backend/.sqlx/query-27aed1f0ee2c6f0ec4f2d7e64a67ce61a985c7c2eed72544acd05316bc002234.json similarity index 76% rename from backend/.sqlx/query-06af616fe4fc61a3b0dcd996a2a1e0e9ac63fc8756aeafac8d279841db117eac.json rename to backend/.sqlx/query-27aed1f0ee2c6f0ec4f2d7e64a67ce61a985c7c2eed72544acd05316bc002234.json index 843207dad7..ac0e47054f 100644 --- a/backend/.sqlx/query-06af616fe4fc61a3b0dcd996a2a1e0e9ac63fc8756aeafac8d279841db117eac.json +++ b/backend/.sqlx/query-27aed1f0ee2c6f0ec4f2d7e64a67ce61a985c7c2eed72544acd05316bc002234.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n (SELECT MIN(day) FROM guest_activity) AS since,\n (SELECT COUNT(DISTINCT email) FROM guest_activity\n WHERE day > CURRENT_DATE - 30)::INT AS \"guest_count!\",\n (SELECT COUNT(DISTINCT email) FROM guest_activity\n WHERE jwt_entry AND day > CURRENT_DATE - 30)::INT AS \"guest_jwt_count!\",\n (SELECT COUNT(DISTINCT workspace_id) FROM guest_activity\n WHERE day > CURRENT_DATE - 30)::INT AS \"guest_workspace_count!\",\n (SELECT COUNT(*) FROM workspace_settings ws JOIN workspace w ON w.id = ws.workspace_id\n WHERE ws.guest_access_enabled AND NOT w.deleted)::INT AS \"guest_enabled_workspace_count!\",\n (SELECT COUNT(*) FROM workspace WHERE NOT deleted)::INT AS \"workspace_count!\"\n ", + "query": "\n SELECT\n (SELECT MIN(day) FROM guest_activity) AS since,\n (SELECT COUNT(DISTINCT email) FROM guest_activity\n WHERE day > CURRENT_DATE - 30)::INT AS \"guest_count!\",\n (SELECT COUNT(DISTINCT email) FROM guest_activity\n WHERE jwt_entry AND day > CURRENT_DATE - 30)::INT AS \"guest_jwt_count!\",\n (SELECT COUNT(DISTINCT workspace_id) FROM guest_activity\n WHERE day > CURRENT_DATE - 30)::INT AS \"guest_workspace_count!\",\n (SELECT COUNT(DISTINCT a.workspace_id) FROM app a JOIN workspace w ON w.id = a.workspace_id\n WHERE a.policy->>'execution_mode' = 'guest' AND NOT w.deleted)::INT AS \"guest_enabled_workspace_count!\",\n (SELECT COUNT(*) FROM workspace WHERE NOT deleted)::INT AS \"workspace_count!\"\n ", "describe": { "columns": [ { @@ -46,5 +46,5 @@ null ] }, - "hash": "06af616fe4fc61a3b0dcd996a2a1e0e9ac63fc8756aeafac8d279841db117eac" + "hash": "27aed1f0ee2c6f0ec4f2d7e64a67ce61a985c7c2eed72544acd05316bc002234" } diff --git a/backend/.sqlx/query-3ab4e59067a97b74636320f6e237e9481881fd99c5be43a4e58ce95a3baefeb6.json b/backend/.sqlx/query-3ab4e59067a97b74636320f6e237e9481881fd99c5be43a4e58ce95a3baefeb6.json new file mode 100644 index 0000000000..82ebd7ab22 --- /dev/null +++ b/backend/.sqlx/query-3ab4e59067a97b74636320f6e237e9481881fd99c5be43a4e58ce95a3baefeb6.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO workspace_settings (workspace_id, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, role_overrides, teams_command_script, teams_team_id, teams_team_name, git_app_installations, git_credentials, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, slack_signing_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_jwt_public_key, guest_jwt_jwks_url, add_admins_and_developers_to_forks, remote_deploy_target, remote_deploy_target_changed_at, variable_expiration_handler) SELECT $1, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, role_overrides, teams_command_script, teams_team_id, teams_team_name, git_app_installations, git_credentials, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, slack_signing_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_jwt_public_key, guest_jwt_jwks_url, add_admins_and_developers_to_forks, remote_deploy_target, remote_deploy_target_changed_at, variable_expiration_handler FROM workspace_settings WHERE workspace_id = $2", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Text" + ] + }, + "nullable": [] + }, + "hash": "3ab4e59067a97b74636320f6e237e9481881fd99c5be43a4e58ce95a3baefeb6" +} diff --git a/backend/.sqlx/query-af37e5579b729a1aedcd63ab70d189c7e0376ca1f0071ae8360183ed1300d2bb.json b/backend/.sqlx/query-3b98b506c544162e630641147490539dc7e9c440cd1625054d6ee1b67a785418.json similarity index 92% rename from backend/.sqlx/query-af37e5579b729a1aedcd63ab70d189c7e0376ca1f0071ae8360183ed1300d2bb.json rename to backend/.sqlx/query-3b98b506c544162e630641147490539dc7e9c440cd1625054d6ee1b67a785418.json index c832fdeeda..fd43bec176 100644 --- a/backend/.sqlx/query-af37e5579b729a1aedcd63ab70d189c7e0376ca1f0071ae8360183ed1300d2bb.json +++ b/backend/.sqlx/query-3b98b506c544162e630641147490539dc7e9c440cd1625054d6ee1b67a785418.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n workspace_id,\n slack_team_id,\n teams_team_id,\n teams_team_name,\n teams_team_guid,\n slack_name,\n slack_command_script,\n teams_command_script,\n slack_email,\n slack_oauth_client_id,\n slack_oauth_client_secret,\n customer_id,\n plan,\n webhook,\n ai_config,\n dbt_warehouses,\n large_file_storage,\n datatable,\n ducklake,\n git_sync,\n deploy_ui,\n default_app,\n default_scripts,\n mute_critical_alerts,\n color,\n operator_settings,\n git_app_installations,\n auto_invite,\n error_handler,\n success_handler,\n variable_expiration_handler,\n public_app_execution_limit_per_minute,\n error_handler_fallback_to_instance_alerts,\n guest_access_enabled,\n guest_jwt_public_key,\n guest_jwt_jwks_url,\n add_admins_and_developers_to_forks\n FROM\n workspace_settings\n WHERE\n workspace_id = $1\n ", + "query": "\n SELECT\n workspace_id,\n slack_team_id,\n teams_team_id,\n teams_team_name,\n teams_team_guid,\n slack_name,\n slack_command_script,\n teams_command_script,\n slack_email,\n slack_oauth_client_id,\n slack_oauth_client_secret,\n customer_id,\n plan,\n webhook,\n ai_config,\n dbt_warehouses,\n large_file_storage,\n datatable,\n ducklake,\n git_sync,\n deploy_ui,\n default_app,\n default_scripts,\n mute_critical_alerts,\n color,\n operator_settings,\n git_app_installations,\n auto_invite,\n error_handler,\n success_handler,\n variable_expiration_handler,\n public_app_execution_limit_per_minute,\n error_handler_fallback_to_instance_alerts,\n guest_jwt_public_key,\n guest_jwt_jwks_url,\n add_admins_and_developers_to_forks\n FROM\n workspace_settings\n WHERE\n workspace_id = $1\n ", "describe": { "columns": [ { @@ -170,21 +170,16 @@ }, { "ordinal": 33, - "name": "guest_access_enabled", - "type_info": "Bool" - }, - { - "ordinal": 34, "name": "guest_jwt_public_key", "type_info": "Text" }, { - "ordinal": 35, + "ordinal": 34, "name": "guest_jwt_jwks_url", "type_info": "Text" }, { - "ordinal": 36, + "ordinal": 35, "name": "add_admins_and_developers_to_forks", "type_info": "Bool" } @@ -228,11 +223,10 @@ true, true, false, - false, true, true, false ] }, - "hash": "af37e5579b729a1aedcd63ab70d189c7e0376ca1f0071ae8360183ed1300d2bb" + "hash": "3b98b506c544162e630641147490539dc7e9c440cd1625054d6ee1b67a785418" } diff --git a/backend/.sqlx/query-76c0331b18eed478a50572e35642909be7dc7eb9b6deac7ea439eb637d728477.json b/backend/.sqlx/query-76c0331b18eed478a50572e35642909be7dc7eb9b6deac7ea439eb637d728477.json deleted file mode 100644 index ebd492abf3..0000000000 --- a/backend/.sqlx/query-76c0331b18eed478a50572e35642909be7dc7eb9b6deac7ea439eb637d728477.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "UPDATE workspace_settings SET guest_access_enabled = $1 WHERE workspace_id = $2", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Bool", - "Text" - ] - }, - "nullable": [] - }, - "hash": "76c0331b18eed478a50572e35642909be7dc7eb9b6deac7ea439eb637d728477" -} diff --git a/backend/.sqlx/query-8ebe054b41793f1a7b85f1f8d29cd21d12fa1207b58bfc46249c250cdcdb5363.json b/backend/.sqlx/query-8ebe054b41793f1a7b85f1f8d29cd21d12fa1207b58bfc46249c250cdcdb5363.json deleted file mode 100644 index 7a822a8d6a..0000000000 --- a/backend/.sqlx/query-8ebe054b41793f1a7b85f1f8d29cd21d12fa1207b58bfc46249c250cdcdb5363.json +++ /dev/null @@ -1,232 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n SELECT\n workspace_id,\n slack_team_id,\n teams_team_id,\n teams_team_name,\n teams_team_guid,\n slack_name,\n slack_command_script,\n teams_command_script,\n slack_email,\n slack_oauth_client_id,\n slack_oauth_client_secret,\n customer_id,\n plan,\n webhook,\n ai_config,\n dbt_warehouses,\n large_file_storage,\n datatable,\n ducklake,\n git_sync,\n deploy_ui,\n default_app,\n default_scripts,\n mute_critical_alerts,\n color,\n operator_settings,\n git_app_installations,\n auto_invite,\n error_handler,\n success_handler,\n public_app_execution_limit_per_minute,\n error_handler_fallback_to_instance_alerts,\n guest_access_enabled,\n guest_jwt_public_key,\n guest_jwt_jwks_url,\n add_admins_and_developers_to_forks\n FROM\n workspace_settings\n WHERE\n workspace_id = $1\n ", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "workspace_id", - "type_info": "Varchar" - }, - { - "ordinal": 1, - "name": "slack_team_id", - "type_info": "Varchar" - }, - { - "ordinal": 2, - "name": "teams_team_id", - "type_info": "Text" - }, - { - "ordinal": 3, - "name": "teams_team_name", - "type_info": "Text" - }, - { - "ordinal": 4, - "name": "teams_team_guid", - "type_info": "Text" - }, - { - "ordinal": 5, - "name": "slack_name", - "type_info": "Varchar" - }, - { - "ordinal": 6, - "name": "slack_command_script", - "type_info": "Varchar" - }, - { - "ordinal": 7, - "name": "teams_command_script", - "type_info": "Text" - }, - { - "ordinal": 8, - "name": "slack_email", - "type_info": "Varchar" - }, - { - "ordinal": 9, - "name": "slack_oauth_client_id", - "type_info": "Varchar" - }, - { - "ordinal": 10, - "name": "slack_oauth_client_secret", - "type_info": "Varchar" - }, - { - "ordinal": 11, - "name": "customer_id", - "type_info": "Varchar" - }, - { - "ordinal": 12, - "name": "plan", - "type_info": "Varchar" - }, - { - "ordinal": 13, - "name": "webhook", - "type_info": "Text" - }, - { - "ordinal": 14, - "name": "ai_config", - "type_info": "Jsonb" - }, - { - "ordinal": 15, - "name": "dbt_warehouses", - "type_info": "Jsonb" - }, - { - "ordinal": 16, - "name": "large_file_storage", - "type_info": "Jsonb" - }, - { - "ordinal": 17, - "name": "datatable", - "type_info": "Jsonb" - }, - { - "ordinal": 18, - "name": "ducklake", - "type_info": "Jsonb" - }, - { - "ordinal": 19, - "name": "git_sync", - "type_info": "Jsonb" - }, - { - "ordinal": 20, - "name": "deploy_ui", - "type_info": "Jsonb" - }, - { - "ordinal": 21, - "name": "default_app", - "type_info": "Varchar" - }, - { - "ordinal": 22, - "name": "default_scripts", - "type_info": "Jsonb" - }, - { - "ordinal": 23, - "name": "mute_critical_alerts", - "type_info": "Bool" - }, - { - "ordinal": 24, - "name": "color", - "type_info": "Varchar" - }, - { - "ordinal": 25, - "name": "operator_settings", - "type_info": "Jsonb" - }, - { - "ordinal": 26, - "name": "git_app_installations", - "type_info": "Jsonb" - }, - { - "ordinal": 27, - "name": "auto_invite", - "type_info": "Jsonb" - }, - { - "ordinal": 28, - "name": "error_handler", - "type_info": "Jsonb" - }, - { - "ordinal": 29, - "name": "success_handler", - "type_info": "Jsonb" - }, - { - "ordinal": 30, - "name": "public_app_execution_limit_per_minute", - "type_info": "Int4" - }, - { - "ordinal": 31, - "name": "error_handler_fallback_to_instance_alerts", - "type_info": "Bool" - }, - { - "ordinal": 32, - "name": "guest_access_enabled", - "type_info": "Bool" - }, - { - "ordinal": 33, - "name": "guest_jwt_public_key", - "type_info": "Text" - }, - { - "ordinal": 34, - "name": "guest_jwt_jwks_url", - "type_info": "Text" - }, - { - "ordinal": 35, - "name": "add_admins_and_developers_to_forks", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Text" - ] - }, - "nullable": [ - false, - true, - true, - true, - true, - true, - true, - true, - false, - true, - true, - true, - true, - true, - true, - true, - true, - true, - true, - true, - true, - true, - true, - true, - true, - true, - false, - true, - true, - true, - true, - false, - false, - true, - true, - false - ] - }, - "hash": "8ebe054b41793f1a7b85f1f8d29cd21d12fa1207b58bfc46249c250cdcdb5363" -} diff --git a/backend/.sqlx/query-9dabc87e05f72fbd4b586c80d9c7f41c3a72838c5d4c5f795638506084561894.json b/backend/.sqlx/query-9dabc87e05f72fbd4b586c80d9c7f41c3a72838c5d4c5f795638506084561894.json deleted file mode 100644 index 6557494459..0000000000 --- a/backend/.sqlx/query-9dabc87e05f72fbd4b586c80d9c7f41c3a72838c5d4c5f795638506084561894.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO workspace_settings (workspace_id, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, role_overrides, teams_command_script, teams_team_id, teams_team_name, git_app_installations, git_credentials, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, slack_signing_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_access_enabled, guest_jwt_public_key, guest_jwt_jwks_url, add_admins_and_developers_to_forks, remote_deploy_target, remote_deploy_target_changed_at, variable_expiration_handler) SELECT $1, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, role_overrides, teams_command_script, teams_team_id, teams_team_name, git_app_installations, git_credentials, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, slack_signing_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_access_enabled, guest_jwt_public_key, guest_jwt_jwks_url, add_admins_and_developers_to_forks, remote_deploy_target, remote_deploy_target_changed_at, variable_expiration_handler FROM workspace_settings WHERE workspace_id = $2", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Varchar", - "Text" - ] - }, - "nullable": [] - }, - "hash": "9dabc87e05f72fbd4b586c80d9c7f41c3a72838c5d4c5f795638506084561894" -} diff --git a/backend/.sqlx/query-e6e31fdf705896c81f9a0f45d47c1b93db0406aaf278bccba07be00e9f937e2a.json b/backend/.sqlx/query-d8b38538f21b533eccc147be0a24fdddbfdb63c021a4ec93c371c1ae38cb72be.json similarity index 76% rename from backend/.sqlx/query-e6e31fdf705896c81f9a0f45d47c1b93db0406aaf278bccba07be00e9f937e2a.json rename to backend/.sqlx/query-d8b38538f21b533eccc147be0a24fdddbfdb63c021a4ec93c371c1ae38cb72be.json index 7462224850..6bf69c90b1 100644 --- a/backend/.sqlx/query-e6e31fdf705896c81f9a0f45d47c1b93db0406aaf278bccba07be00e9f937e2a.json +++ b/backend/.sqlx/query-d8b38538f21b533eccc147be0a24fdddbfdb63c021a4ec93c371c1ae38cb72be.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n workspace_id,\n slack_team_id,\n slack_name,\n teams_team_id,\n teams_team_name,\n teams_team_guid,\n mute_critical_alerts,\n guest_access_enabled,\n add_admins_and_developers_to_forks,\n deploy_ui,\n large_file_storage,\n datatable\n FROM\n workspace_settings\n WHERE\n workspace_id = $1\n ", + "query": "\n SELECT\n workspace_id,\n slack_team_id,\n slack_name,\n teams_team_id,\n teams_team_name,\n teams_team_guid,\n mute_critical_alerts,\n add_admins_and_developers_to_forks,\n deploy_ui,\n large_file_storage,\n datatable\n FROM\n workspace_settings\n WHERE\n workspace_id = $1\n ", "describe": { "columns": [ { @@ -40,26 +40,21 @@ }, { "ordinal": 7, - "name": "guest_access_enabled", - "type_info": "Bool" - }, - { - "ordinal": 8, "name": "add_admins_and_developers_to_forks", "type_info": "Bool" }, { - "ordinal": 9, + "ordinal": 8, "name": "deploy_ui", "type_info": "Jsonb" }, { - "ordinal": 10, + "ordinal": 9, "name": "large_file_storage", "type_info": "Jsonb" }, { - "ordinal": 11, + "ordinal": 10, "name": "datatable", "type_info": "Jsonb" } @@ -78,11 +73,10 @@ true, true, false, - false, true, true, true ] }, - "hash": "e6e31fdf705896c81f9a0f45d47c1b93db0406aaf278bccba07be00e9f937e2a" + "hash": "d8b38538f21b533eccc147be0a24fdddbfdb63c021a4ec93c371c1ae38cb72be" } diff --git a/backend/.sqlx/query-f5022831ab610ab22ef1af1441753aba57d5e8f19a316f76e01922559425d81b.json b/backend/.sqlx/query-f5022831ab610ab22ef1af1441753aba57d5e8f19a316f76e01922559425d81b.json deleted file mode 100644 index 3b0dbb2f41..0000000000 --- a/backend/.sqlx/query-f5022831ab610ab22ef1af1441753aba57d5e8f19a316f76e01922559425d81b.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO workspace_settings (workspace_id, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, role_overrides, teams_command_script, teams_team_id, teams_team_name, git_app_installations, git_credentials, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, slack_signing_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_access_enabled, guest_jwt_public_key, guest_jwt_jwks_url, add_admins_and_developers_to_forks, remote_deploy_target, remote_deploy_target_changed_at) SELECT $1, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, role_overrides, teams_command_script, teams_team_id, teams_team_name, git_app_installations, git_credentials, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, slack_signing_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_access_enabled, guest_jwt_public_key, guest_jwt_jwks_url, add_admins_and_developers_to_forks, remote_deploy_target, remote_deploy_target_changed_at FROM workspace_settings WHERE workspace_id = $2", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Varchar", - "Text" - ] - }, - "nullable": [] - }, - "hash": "f5022831ab610ab22ef1af1441753aba57d5e8f19a316f76e01922559425d81b" -} diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 548dec8dc1..fe6faf655f 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -bdc4ba4af27f57dc978a7ddca2dccd5b0d67f627 +0605eac866b49f0de913ab41c1168b79eec31ca0 diff --git a/backend/migrations/20261028111747_close_inert_guest_apps.down.sql b/backend/migrations/20261028111747_close_inert_guest_apps.down.sql new file mode 100644 index 0000000000..6fe6281b43 --- /dev/null +++ b/backend/migrations/20261028111747_close_inert_guest_apps.down.sql @@ -0,0 +1,2 @@ +-- Which apps were moved to `publisher` is not recorded, so there is nothing to restore. +SELECT 1; diff --git a/backend/migrations/20261028111747_close_inert_guest_apps.up.sql b/backend/migrations/20261028111747_close_inert_guest_apps.up.sql new file mode 100644 index 0000000000..fbffb8f3df --- /dev/null +++ b/backend/migrations/20261028111747_close_inert_guest_apps.up.sql @@ -0,0 +1,15 @@ +-- Guest apps are now gated by their execution mode alone. An app stored as `guest` where the +-- workspace switch was off, or the instance switch was set, admitted no guest; leave it +-- admitting none by moving it to `publisher`, which runs the same way for members. +UPDATE app +SET policy = jsonb_set(policy, '{execution_mode}', '"publisher"') +FROM workspace_settings ws +WHERE ws.workspace_id = app.workspace_id + AND app.policy->>'execution_mode' = 'guest' + AND ( + NOT ws.guest_access_enabled + OR COALESCE( + (SELECT (value #>> '{}')::boolean FROM global_settings WHERE name = 'guest_access_disabled'), + false + ) + ); diff --git a/backend/tests/app_guest_allowance.rs b/backend/tests/app_guest_allowance.rs index 88626b1c81..5c2e99f46f 100644 --- a/backend/tests/app_guest_allowance.rs +++ b/backend/tests/app_guest_allowance.rs @@ -57,13 +57,6 @@ async fn the_allowance_caps_strangers_and_meters_an_enterprise_plan( let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - authed( - client().post(format!("{ws}/workspaces/edit_guest_access")), - ADMIN_TOKEN, - ) - .json(&json!({ "guest_access_enabled": true })) - .send() - .await?; let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN) .json(&json!({ "path": APP_PATH, diff --git a/backend/tests/app_guest_cloud_hosted.rs b/backend/tests/app_guest_cloud_hosted.rs index 7ab4145a65..1b59538aef 100644 --- a/backend/tests/app_guest_cloud_hosted.rs +++ b/backend/tests/app_guest_cloud_hosted.rs @@ -32,20 +32,6 @@ async fn the_cloud_admits_no_guest(db: Pool) -> anyhow::Result<()> { let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - // The workspace switch cannot be turned on, so no policy can lean on it. - let resp = authed( - client().post(format!("{ws}/workspaces/edit_guest_access")), - ADMIN_TOKEN, - ) - .json(&json!({ "guest_access_enabled": true })) - .send() - .await?; - assert_eq!(resp.status(), 400); - assert!( - resp.text().await?.contains("self-hosted"), - "the refusal must name what guests need" - ); - let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN) .json(&json!({ "path": APP_PATH, @@ -56,6 +42,10 @@ async fn the_cloud_admits_no_guest(db: Pool) -> anyhow::Result<()> { .send() .await?; assert_eq!(resp.status(), 400, "an app cannot be deployed to guests"); + assert!( + resp.text().await?.contains("self-hosted"), + "the refusal must name what guests need" + ); // Nor can a key be configured for the JWT way in — the refusal lands before the // outbound JWKS fetch it would otherwise make. @@ -96,9 +86,6 @@ async fn the_cloud_admits_no_guest(db: Pool) -> anyhow::Result<()> { .bind(APP_PATH) .execute(&db) .await?; - sqlx::query("UPDATE workspace_settings SET guest_access_enabled = true WHERE workspace_id = 'test-workspace'") - .execute(&db) - .await?; // Deploying it again is not refused: only widening an app into guests is, so a // git-sync push of one already stored that way keeps working (and keeps being inert). diff --git a/backend/tests/app_guest_execution_mode.rs b/backend/tests/app_guest_execution_mode.rs index 105458847a..f67757c6ed 100644 --- a/backend/tests/app_guest_execution_mode.rs +++ b/backend/tests/app_guest_execution_mode.rs @@ -35,19 +35,6 @@ fn authed(builder: reqwest::RequestBuilder, token: &str) -> reqwest::RequestBuil builder.header("Authorization", format!("Bearer {}", token)) } -async fn enable_guests(port: u16, ws: &str) -> anyhow::Result<()> { - authed( - client().post(format!( - "http://localhost:{port}/api/w/{ws}/workspaces/edit_guest_access" - )), - ADMIN_TOKEN, - ) - .json(&json!({ "guest_access_enabled": true })) - .send() - .await?; - Ok(()) -} - fn guest_scopes() -> Vec { vec![ "guest".to_string(), @@ -86,7 +73,6 @@ async fn guest_session_is_confined_to_its_app(db: Pool) -> anyhow::Res let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - enable_guests(port, "test-workspace").await?; insert_guest_token(&db, "test-workspace").await?; // Its own identity resolves, and reports the role rather than falling through to @@ -166,9 +152,7 @@ async fn guest_token_does_not_cross_workspaces(db: Pool) -> anyhow::Re } #[sqlx::test(fixtures("base"))] -async fn guest_entry_needs_both_the_app_mode_and_the_workspace_switch( - db: Pool, -) -> anyhow::Result<()> { +async fn guest_entry_follows_the_app_mode(db: Pool) -> anyhow::Result<()> { initialize_tracing().await; let server = ApiServer::start(db.clone()).await?; let port = server.addr.port(); @@ -194,25 +178,6 @@ async fn guest_entry_needs_both_the_app_mode_and_the_workspace_switch( .text() .await?; - // The app says guest, the workspace has not opted in: inert. - let resp = client() - .get(format!("{ws}/apps_u/guest_entry/{secret}")) - .send() - .await?; - assert_eq!( - resp.status(), - 404, - "a guest app in a workspace that has not enabled guests must not advertise entry" - ); - - authed( - client().post(format!("{ws}/workspaces/edit_guest_access")), - ADMIN_TOKEN, - ) - .json(&json!({ "guest_access_enabled": true })) - .send() - .await?; - // Unauthenticated on purpose: this is what a signed-out visitor reads. let resp = client() .get(format!("{ws}/apps_u/guest_entry/{secret}")) @@ -222,15 +187,15 @@ async fn guest_entry_needs_both_the_app_mode_and_the_workspace_switch( let entry: serde_json::Value = resp.json().await?; assert_eq!(entry["app_path"], json!(APP_PATH)); - // Turning the switch back off closes the door again even though the app's own - // policy is unchanged. - authed( - client().post(format!("{ws}/workspaces/edit_guest_access")), + // Back to members only: the door closes. + let resp = authed( + client().post(format!("{ws}/apps/update/{APP_PATH}")), ADMIN_TOKEN, ) - .json(&json!({ "guest_access_enabled": false })) + .json(&json!({ "policy": { "execution_mode": "publisher", "triggerables": {} } })) .send() .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); let resp = client() .get(format!("{ws}/apps_u/guest_entry/{secret}")) .send() @@ -238,7 +203,7 @@ async fn guest_entry_needs_both_the_app_mode_and_the_workspace_switch( assert_eq!( resp.status(), 404, - "turning guests off must stop advertising entry for an app already set to guest" + "an app taken out of guest mode must stop advertising entry" ); Ok(()) @@ -363,12 +328,11 @@ fn execute(port: u16, ws: &str, app: &str, token: &str) -> reqwest::RequestBuild })) } -/// The workspace switch is enforced at the auth door for every guest request, not -/// remembered per handler. This is what stands between a `guest` policy pushed by -/// git-sync and execution once an admin has turned guests off — and it closes the -/// app to sessions already issued. +/// Taking an app out of guest mode is what closes it to guests, sessions already issued +/// included: the mode is re-read on every run, not when the session was minted. This is +/// what stands between a guest and an app an admin has closed again. #[sqlx::test(fixtures("base"))] -async fn the_door_re_checks_the_workspace_switch(db: Pool) -> anyhow::Result<()> { +async fn leaving_guest_mode_closes_issued_sessions(db: Pool) -> anyhow::Result<()> { initialize_tracing().await; let server = ApiServer::start(db.clone()).await?; let port = server.addr.port(); @@ -381,38 +345,40 @@ async fn the_door_re_checks_the_workspace_switch(db: Pool) -> anyhow:: assert_eq!(resp.status(), 201, "{}", resp.text().await?); insert_guest_token(&db, "test-workspace").await?; - // Switch off: the session does not authenticate at all, even though the app's - // policy says guest and the session was (in this fixture) issued regardless. On - // the authed route that is a 401; on the optional-auth run route the rejected - // token reads as no token, and a guest-mode app then refuses the anonymous - // caller — a denial either way. - let resp = authed(client().get(format!("{ws}/users/whoami")), GUEST_TOKEN) - .send() - .await?; - assert_eq!( - resp.status(), - 401, - "a guest must not authenticate while guests are off" - ); - let resp = execute(port, "test-workspace", APP_PATH, GUEST_TOKEN) - .send() - .await?; - assert!( - resp.status().is_client_error() && resp.status() != 404, - "a guest must not run while guests are off, got {}", - resp.status() - ); - - // Switch on: through the door. What follows the run is the runnable lookup, - // which fails on the nonexistent script — the point is that it is no longer a - // denial. - enable_guests(port, "test-workspace").await?; + // In guest mode: through the door. What follows the run is the runnable lookup, + // which fails on the nonexistent script; the point is that it is not a denial. let resp = execute(port, "test-workspace", APP_PATH, GUEST_TOKEN) .send() .await?; assert!( resp.status() != 401 && resp.status() != 403, - "with guests on, the door must let the run through: {}", + "a guest must reach its guest-mode app: {}", + resp.status() + ); + + let resp = authed( + client().post(format!("{ws}/apps/update/{APP_PATH}")), + ADMIN_TOKEN, + ) + .json(&app_with_runnable(APP_PATH, "publisher", false)) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + // Evict the cached policy as the server's notify poller would. + for e in windmill_common::notify_events::poll_notify_events(&db, 0).await? { + if e.channel == "notify_app_policy_change" { + if let Some((ws, path)) = e.payload.split_once(':') { + windmill_api::invalidate_app_policy_cache(ws, path); + } + } + } + + let resp = execute(port, "test-workspace", APP_PATH, GUEST_TOKEN) + .send() + .await?; + assert!( + resp.status() == 401 || resp.status() == 403, + "an issued guest session must not run an app taken out of guest mode: {}", resp.status() ); @@ -428,13 +394,6 @@ async fn guest_cannot_run_another_guest_app(db: Pool) -> anyhow::Resul let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - authed( - client().post(format!("{ws}/workspaces/edit_guest_access")), - ADMIN_TOKEN, - ) - .json(&json!({ "guest_access_enabled": true })) - .send() - .await?; let other = "u/test-user/other_guest_app"; let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN) .json(&guest_app_with_runnable(other, false)) @@ -510,7 +469,6 @@ async fn a_guest_cannot_read_a_job_it_did_not_launch(db: Pool) -> anyh let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - enable_guests(port, "test-workspace").await?; insert_guest_token(&db, "test-workspace").await?; let resp = authed(client().post(format!("{ws}/scripts/create")), ADMIN_TOKEN) .json(&json!({ @@ -611,15 +569,14 @@ async fn guests_mode_needs_a_scopable_path(db: Pool) -> anyhow::Result Ok(()) } -/// Renaming a workspace copies its settings; the guest switch and the guest JWT key must -/// travel with them, or the rename silently shuts every guest app or drops the key. +/// Renaming a workspace copies its settings; the guest JWT key must travel with them, +/// or the rename silently drops the key. #[sqlx::test(fixtures("base"))] -async fn a_workspace_rename_keeps_the_guest_switch(db: Pool) -> anyhow::Result<()> { +async fn a_workspace_rename_keeps_the_guest_jwt_key(db: Pool) -> anyhow::Result<()> { initialize_tracing().await; let server = ApiServer::start(db.clone()).await?; let port = server.addr.port(); - enable_guests(port, "test-workspace").await?; sqlx::query( "INSERT INTO guest_activity (email, workspace_id, day) VALUES ('guest@example.com', 'test-workspace', CURRENT_DATE)", @@ -641,12 +598,6 @@ async fn a_workspace_rename_keeps_the_guest_switch(db: Pool) -> anyhow .send() .await?; assert_eq!(resp.status(), 200, "{}", resp.text().await?); - let enabled: bool = sqlx::query_scalar( - "SELECT guest_access_enabled FROM workspace_settings WHERE workspace_id = 'test-workspace-2'", - ) - .fetch_one(&db) - .await?; - assert!(enabled, "the guest switch travels with the workspace"); let jwt_key: Option = sqlx::query_scalar( "SELECT guest_jwt_public_key FROM workspace_settings WHERE workspace_id = 'test-workspace-2'", ) @@ -668,70 +619,6 @@ async fn a_workspace_rename_keeps_the_guest_switch(db: Pool) -> anyhow Ok(()) } -/// The superadmin switch sits above every workspace's: off, no guest session stands and -/// no app discovers as open, whatever the workspace and the app say. -#[sqlx::test(fixtures("base"))] -async fn the_instance_switch_closes_every_workspace(db: Pool) -> anyhow::Result<()> { - initialize_tracing().await; - let server = ApiServer::start(db.clone()).await?; - let port = server.addr.port(); - let ws = format!("http://localhost:{port}/api/w/test-workspace"); - - enable_guests(port, "test-workspace").await?; - let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN) - .json(&guest_app_with_runnable(APP_PATH, false)) - .send() - .await?; - assert_eq!(resp.status(), 201, "{}", resp.text().await?); - insert_guest_token(&db, "test-workspace").await?; - let set_instance_switch = |disabled: bool| { - authed( - client().post(format!( - "http://localhost:{port}/api/settings/global/guest_access_disabled" - )), - ADMIN_TOKEN, - ) - .json(&json!({ "value": disabled })) - .send() - }; - - let secret: String = authed( - client().get(format!("{ws}/apps/secret_of/{APP_PATH}")), - ADMIN_TOKEN, - ) - .send() - .await? - .text() - .await?; - - set_instance_switch(true).await?.error_for_status()?; - let resp = authed(client().get(format!("{ws}/users/whoami")), GUEST_TOKEN) - .send() - .await?; - assert_eq!( - resp.status(), - 401, - "the instance switch closes an issued session" - ); - let resp = client() - .get(format!("{ws}/apps_u/guest_entry/{secret}")) - .send() - .await?; - assert_eq!( - resp.status(), - 404, - "and nothing discovers as open to guests" - ); - - set_instance_switch(false).await?.error_for_status()?; - let resp = authed(client().get(format!("{ws}/users/whoami")), GUEST_TOKEN) - .send() - .await?; - assert_eq!(resp.status(), 200, "back on, the session stands again"); - - Ok(()) -} - /// An account holder is never a guest, and that holds after the mint too: a session /// minted before the account existed ends at the door the moment one does, so an /// account provisioned in a race with the mint cannot outlive the rule. @@ -742,7 +629,6 @@ async fn an_account_ends_the_guest_session(db: Pool) -> anyhow::Result let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - enable_guests(port, "test-workspace").await?; insert_guest_token(&db, "test-workspace").await?; let resp = authed(client().get(format!("{ws}/users/whoami")), GUEST_TOKEN) .send() @@ -778,7 +664,6 @@ async fn a_guest_cannot_upload_outside_a_policy(db: Pool) -> anyhow::R let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - enable_guests(port, "test-workspace").await?; let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN) .json(&guest_app_with_runnable(APP_PATH, false)) // no `s3_inputs` .send() @@ -825,7 +710,6 @@ async fn a_guest_uses_an_anonymous_app_as_itself(db: Pool) -> anyhow:: let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - enable_guests(port, "test-workspace").await?; let resp = authed(client().post(format!("{ws}/scripts/create")), ADMIN_TOKEN) .json(&json!({ "path": "u/test-user/noop", @@ -878,7 +762,6 @@ async fn a_guest_minted_embed_token_stays_a_guest(db: Pool) -> anyhow: let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - enable_guests(port, "test-workspace").await?; let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN) .json(&guest_app_with_runnable(APP_PATH, true)) .send() @@ -937,13 +820,13 @@ async fn a_guest_minted_embed_token_stays_a_guest(db: Pool) -> anyhow: let me: serde_json::Value = resp.json().await?; assert_eq!(me["role"], json!("guest")); - // Governed: the workspace switch closes it at the door, iframe or not. - authed( - client().post(format!("{ws}/workspaces/edit_guest_access")), - ADMIN_TOKEN, + // Governed: the door's no-account rule closes it, iframe or not, once the guest's + // email is promoted to an account. + sqlx::query( + "INSERT INTO password (email, password_hash, login_type, super_admin, verified, name) + VALUES ('guest@example.com', 'not-a-real-hash', 'password', false, true, 'Guest')", ) - .json(&json!({ "guest_access_enabled": false })) - .send() + .execute(&db) .await?; let resp = authed(client().get(format!("{ws}/users/whoami")), &embed) .send() @@ -951,7 +834,7 @@ async fn a_guest_minted_embed_token_stays_a_guest(db: Pool) -> anyhow: assert_eq!( resp.status(), 401, - "turning guests off must stop a guest's embed token authenticating" + "an account for the guest's email must stop its embed token authenticating" ); let resp = execute(port, "test-workspace", APP_PATH, &embed) .send() @@ -961,7 +844,6 @@ async fn a_guest_minted_embed_token_stays_a_guest(db: Pool) -> anyhow: "and running components, got {}", resp.status() ); - enable_guests(port, "test-workspace").await?; // And its scopes are not something the guest's email can later rewrite. The // guest session itself cannot reach `/users/*` (workspace pin), so model the real @@ -1006,7 +888,6 @@ async fn a_guest_label_is_governed_without_the_sentinel(db: Pool) -> a let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - enable_guests(port, "test-workspace").await?; let scopes: Vec = guest_scopes() .into_iter() .filter(|s| s != "guest") @@ -1051,13 +932,6 @@ async fn a_disabled_account_cannot_become_a_guest(db: Pool) -> anyhow: let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - authed( - client().post(format!("{ws}/workspaces/edit_guest_access")), - ADMIN_TOKEN, - ) - .json(&json!({ "guest_access_enabled": true })) - .send() - .await?; let resp = authed(client().post(format!("{ws}/apps/create")), ADMIN_TOKEN) .json(&guest_app_with_runnable(APP_PATH, false)) .send() diff --git a/backend/tests/app_guest_jwt_allowance.rs b/backend/tests/app_guest_jwt_allowance.rs index 85cb51d39e..c6f929776c 100644 --- a/backend/tests/app_guest_jwt_allowance.rs +++ b/backend/tests/app_guest_jwt_allowance.rs @@ -67,13 +67,6 @@ async fn a_guest_jwt_is_capped_like_a_signed_in_guest(db: Pool) -> any let port = server.addr.port(); let ws = format!("http://localhost:{port}/api/w/test-workspace"); - authed( - client().post(format!("{ws}/workspaces/edit_guest_access")), - ADMIN_TOKEN, - ) - .json(&json!({ "guest_access_enabled": true })) - .send() - .await?; let resp = authed( client().post(format!("{ws}/workspaces/edit_guest_jwt_key")), ADMIN_TOKEN, @@ -109,7 +102,11 @@ async fn a_guest_jwt_is_capped_like_a_signed_in_guest(db: Pool) -> any ) .send() .await?; - assert_eq!(resp.status(), 401, "a stranger's JWT is refused past the cap"); + assert_eq!( + resp.status(), + 401, + "a stranger's JWT is refused past the cap" + ); let resp = authed( client().get(format!("{ws}/users/whoami")), diff --git a/backend/tests/app_guest_jwt_entry.rs b/backend/tests/app_guest_jwt_entry.rs index 7b84130a04..e196c914e4 100644 --- a/backend/tests/app_guest_jwt_entry.rs +++ b/backend/tests/app_guest_jwt_entry.rs @@ -85,20 +85,6 @@ fn bearer(claims: &Claims, priv_pem: &str, alg: Algorithm) -> String { format!("jwt_guest_{jwt}") } -async fn enable_guests(port: u16, ws: &str, on: bool) -> anyhow::Result<()> { - let resp = authed( - client().post(format!( - "http://localhost:{port}/api/w/{ws}/workspaces/edit_guest_access" - )), - ADMIN_TOKEN, - ) - .json(&json!({ "guest_access_enabled": on })) - .send() - .await?; - assert_eq!(resp.status(), 200, "{}", resp.text().await?); - Ok(()) -} - async fn set_guest_jwt_pem(port: u16, ws: &str, pem: &str) -> anyhow::Result<()> { let resp = authed( client().post(format!( @@ -157,7 +143,6 @@ async fn a_valid_guest_jwt_opens_its_app(db: Pool) -> anyhow::Result<( let port = server.addr.port(); let ws = "test-workspace"; - enable_guests(port, ws, true).await?; set_guest_jwt_pem(port, ws, PUB1).await?; let resp = authed( client().post(format!("http://localhost:{port}/api/w/{ws}/scripts/create")), @@ -239,7 +224,6 @@ async fn guest_jwt_refusals(db: Pool) -> anyhow::Result<()> { let port = server.addr.port(); let ws = "test-workspace"; - enable_guests(port, ws, true).await?; set_guest_jwt_pem(port, ws, PUB1).await?; create_app(port, ws, app(APP_PATH, "guest", false)).await?; create_app(port, ws, app("u/test-user/members_app", "publisher", false)).await?; @@ -346,48 +330,6 @@ async fn guest_jwt_refusals(db: Pool) -> anyhow::Result<()> { Ok(()) } -/// The workspace switch gates a JWT guest exactly as it gates a signed-in one, at the -/// auth door, so turning guests off closes the JWT entry too. -#[sqlx::test(fixtures("base"))] -async fn guest_jwt_needs_the_workspace_switch(db: Pool) -> anyhow::Result<()> { - initialize_tracing().await; - let server = ApiServer::start(db.clone()).await?; - let port = server.addr.port(); - let ws = "test-workspace"; - - set_guest_jwt_pem(port, ws, PUB1).await?; - create_app(port, ws, app(APP_PATH, "guest", false)).await?; - let token = bearer(&Claims::valid(), PRIV1, Algorithm::ES256); - - // Switch off (the default): refused. - let resp = whoami(port, ws, &token).send().await?; - assert_eq!( - resp.status(), - 401, - "a JWT guest must be refused while guests are off" - ); - - // Switch on: through. - enable_guests(port, ws, true).await?; - let resp = whoami(port, ws, &token).send().await?; - assert_eq!( - resp.status(), - 200, - "with guests on, the JWT guest is admitted" - ); - - // Off again: closed on the next request. - enable_guests(port, ws, false).await?; - let resp = whoami(port, ws, &token).send().await?; - assert_eq!( - resp.status(), - 401, - "turning guests off closes the JWT guest again" - ); - - Ok(()) -} - /// A guest JWT is pinned to the workspace its claim names, so it authenticates on no /// workspace-less route: the arm has no workspace to check the claim against. #[sqlx::test(fixtures("base"))] @@ -397,7 +339,6 @@ async fn guest_jwt_rejected_on_workspaceless_route(db: Pool) -> anyhow let port = server.addr.port(); let ws = "test-workspace"; - enable_guests(port, ws, true).await?; set_guest_jwt_pem(port, ws, PUB1).await?; create_app(port, ws, app(APP_PATH, "guest", false)).await?; let token = bearer(&Claims::valid(), PRIV1, Algorithm::ES256); @@ -427,7 +368,6 @@ async fn a_guest_jwt_derived_embed_token_is_capped(db: Pool) -> anyhow let port = server.addr.port(); let ws = "test-workspace"; - enable_guests(port, ws, true).await?; set_guest_jwt_pem(port, ws, PUB1).await?; create_app(port, ws, app(APP_PATH, "guest", true)).await?; let secret: String = authed( diff --git a/backend/windmill-api-auth/src/auth.rs b/backend/windmill-api-auth/src/auth.rs index e278b644d5..a4a9d41a50 100644 --- a/backend/windmill-api-auth/src/auth.rs +++ b/backend/windmill-api-auth/src/auth.rs @@ -171,10 +171,10 @@ impl AuthCache { // from a job's WM_TOKEN, even on an AUTH_CACHE hit whose cached authed // predates this field. opt_job_authed.authed.job_id = opt_job_authed.job_id; - // The workspace's guest switch is enforced here, once, for every guest request - // — not per handler, where each guest-reachable route would have to remember - // it. Uncached, so turning guests off takes effect on the next request of every - // guest session and every token derived from one. + // The no-account rule is enforced here, once, for every guest request — not per + // handler, where each guest-reachable route would have to remember it. Uncached, + // so an account provisioned after the mint ends the guest session, and every + // token derived from one, on its next request. if crate::scopes::has_guest_sentinel(opt_job_authed.authed.scopes.as_deref()) { let Some(w_id) = w_id else { return None }; let email = &opt_job_authed.authed.email; @@ -277,11 +277,9 @@ impl AuthCache { return None; } }; - // The workspace switch, the instance switch and the app being in guest - // mode, in one answer (guest_app_admits). The door re-reads the switches - // and the no-account rule per request through the sentinel below - // (guest_session_stands), so turning any of them off stops a cached JWT - // session on its next call. + // The app being in guest mode (guest_app_admits). The door re-reads the + // no-account rule per request through the sentinel below + // (guest_session_stands), and the on-behalf resolver re-reads the mode. match windmill_common::workspaces::guest_app_admits( &self.db, w_id, @@ -811,8 +809,8 @@ impl AuthCache { } /// How long a guest JWT resolves from the auth cache before the arm re-runs (and -/// re-reads the key). A guest JWT is not revocable except by the workspace switch or -/// by rotating the key, so the entry must be short enough that a rotated key bites +/// re-reads the key). A guest JWT is not revocable except by taking its app out of +/// guest mode or by rotating the key, so the entry must be short enough that a rotated key bites /// soon, unlike a normal token whose row can be deleted. Also what makes the /// day-keyed activity dedupe below reachable across a midnight. const GUEST_JWT_CACHE_TTL: chrono::Duration = chrono::Duration::minutes(5); diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 4f93c4f02d..b909220ec4 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -163,7 +163,6 @@ pub fn workspaced_service() -> Router { ) .route("/edit_deploy_ui_config", post(edit_deploy_ui_config)) .route("/edit_default_app", post(edit_default_app)) - .route("/edit_guest_access", post(edit_guest_access)) .route( "/edit_add_admins_and_developers_to_forks", post(edit_add_admins_and_developers_to_forks), @@ -350,13 +349,10 @@ pub struct WorkspaceSettings { #[serde(skip_serializing_if = "Option::is_none")] pub public_app_execution_limit_per_minute: Option, pub error_handler_fallback_to_instance_alerts: bool, - /// Whether this workspace admits guest sessions (`ExecutionMode::Guest`). An app's - /// own `execution_mode: guest` is inert while this is off. - pub guest_access_enabled: bool, /// The key a guest JWT is verified against: a PEM public key, or a JWKS URL, at most /// one (a DB CHECK enforces it). Public material, not a secret, so it is admin- /// readable here. `None`/`None` falls back to the instance issuer (`JWT_EXT_JWKS_URL`) - /// off cloud, or accepts no JWT guest if none is set; `guest_access_enabled` is the switch. + /// off cloud, or accepts no JWT guest if none is set. #[serde(skip_serializing_if = "Option::is_none")] pub guest_jwt_public_key: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -384,9 +380,6 @@ pub struct WorkspacePublicSettings { pub teams_team_guid: Option, #[serde(skip_serializing_if = "Option::is_none")] pub mute_critical_alerts: Option, - /// Not sensitive, and the app editor needs it to say whether the guest rung is - /// live -- an app can be set to `guest` while the workspace has guests off. - pub guest_access_enabled: bool, /// Read by the fork dialog, which tells the forker who else the fork will include. pub add_admins_and_developers_to_forks: bool, #[serde(skip_serializing_if = "Option::is_none")] @@ -1214,7 +1207,6 @@ async fn get_settings( variable_expiration_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, - guest_access_enabled, guest_jwt_public_key, guest_jwt_jwks_url, add_admins_and_developers_to_forks @@ -1258,7 +1250,6 @@ async fn get_public_settings( teams_team_name, teams_team_guid, mute_critical_alerts, - guest_access_enabled, add_admins_and_developers_to_forks, deploy_ui, large_file_storage, @@ -5947,53 +5938,6 @@ async fn edit_default_app( )); } -#[derive(Deserialize)] -struct EditGuestAccess { - guest_access_enabled: bool, -} - -/// Turn guest sessions on or off for this workspace. Off by default, and off is -/// authoritative and immediate: the switch is re-read where a guest session is -/// minted (`guest_app_admits`) and at the auth door on every guest request, so an app -/// whose policy already says `guest` — pushed by git-sync, say — closes to guests on -/// the next request, sessions already issued included. -async fn edit_guest_access( - authed: ApiAuthed, - Extension(db): Extension, - Path(w_id): Path, - Json(EditGuestAccess { guest_access_enabled }): Json, -) -> Result { - require_admin(authed.is_admin, &authed.username)?; - if guest_access_enabled { - windmill_common::workspaces::require_guest_support()?; - } - - let mut tx = db.begin().await?; - sqlx::query!( - "UPDATE workspace_settings SET guest_access_enabled = $1 WHERE workspace_id = $2", - guest_access_enabled, - &w_id - ) - .execute(&mut *tx) - .await?; - - audit_log( - &mut *tx, - &authed, - "workspaces.edit_guest_access", - ActionKind::Update, - &w_id, - Some(&guest_access_enabled.to_string()), - None, - ) - .await?; - tx.commit().await?; - - Ok(format!( - "Guest access set to {guest_access_enabled} for workspace {w_id}" - )) -} - #[derive(Deserialize)] struct EditAddAdminsAndDevelopersToForks { add_admins_and_developers_to_forks: bool, @@ -6039,13 +5983,13 @@ async fn edit_add_admins_and_developers_to_forks( struct EditGuestJwtKey { /// A PEM public key (RS or ES family), or a JWKS URL, at most one. Both empty clears the /// workspace key; verification then falls back to the instance issuer (`JWT_EXT_JWKS_URL`) - /// off cloud, or refuses the JWT if none is set. The off-switch is `guest_access_enabled`. + /// off cloud, or refuses the JWT if none is set. public_key: Option, jwks_url: Option, } /// Configure the key a guest JWT (`jwt_guest_`) is verified against for this workspace. -/// Workspace-admin gated, like the guest switch: guests are free up to the instance +/// Workspace-admin gated: guests are free up to the instance /// allowance on any plan, so configuring their key needs no licence. The key is /// validated before it is stored so a typo is refused here, not silently on every guest /// later: a PEM must parse as an RS/ES public key (HS* has no PEM form and is @@ -15820,7 +15764,6 @@ pub enum SettingsEdit { Ducklake, DbtWarehouses, DeployUi, - GuestAccess, GuestJwtKey, PublicAppRateLimit, AddAdminsAndDevelopersToForks, @@ -15892,7 +15835,6 @@ pub async fn apply_settings_edit( DeployUi => edit_deploy_ui_config(a, d, p, settings_edit_body(edit, body)?).await?, #[cfg(not(feature = "enterprise"))] DeployUi => edit_deploy_ui_config(a, d, p).await?, - GuestAccess => edit_guest_access(a, d, p, settings_edit_body(edit, body)?).await?, GuestJwtKey => edit_guest_jwt_key(a, d, p, settings_edit_body(edit, body)?).await?, PublicAppRateLimit => { edit_public_app_rate_limit(d, p, a, settings_edit_body(edit, body)?).await? diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index aea887624d..a159935f3c 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -135,7 +135,7 @@ pub(crate) async fn change_workspace_id( // Duplicate workspace settings (keep copy in old workspace for reference) info!("Duplicating workspace_settings table"); sqlx::query!( - "INSERT INTO workspace_settings (workspace_id, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, role_overrides, teams_command_script, teams_team_id, teams_team_name, git_app_installations, git_credentials, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, slack_signing_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_access_enabled, guest_jwt_public_key, guest_jwt_jwks_url, add_admins_and_developers_to_forks, remote_deploy_target, remote_deploy_target_changed_at, variable_expiration_handler) SELECT $1, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, role_overrides, teams_command_script, teams_team_id, teams_team_name, git_app_installations, git_credentials, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, slack_signing_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_access_enabled, guest_jwt_public_key, guest_jwt_jwks_url, add_admins_and_developers_to_forks, remote_deploy_target, remote_deploy_target_changed_at, variable_expiration_handler FROM workspace_settings WHERE workspace_id = $2", + "INSERT INTO workspace_settings (workspace_id, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, role_overrides, teams_command_script, teams_team_id, teams_team_name, git_app_installations, git_credentials, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, slack_signing_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_jwt_public_key, guest_jwt_jwks_url, add_admins_and_developers_to_forks, remote_deploy_target, remote_deploy_target_changed_at, variable_expiration_handler) SELECT $1, slack_team_id, slack_name, slack_command_script, slack_email, customer_id, plan, webhook, ai_config, large_file_storage, git_sync, default_app, default_scripts, deploy_ui, mute_critical_alerts, color, operator_settings, role_overrides, teams_command_script, teams_team_id, teams_team_name, git_app_installations, git_credentials, ducklake, dbt_warehouses, slack_oauth_client_id, slack_oauth_client_secret, slack_signing_secret, datatable, teams_team_guid, auto_invite, error_handler, success_handler, public_app_execution_limit_per_minute, error_handler_fallback_to_instance_alerts, guest_jwt_public_key, guest_jwt_jwks_url, add_admins_and_developers_to_forks, remote_deploy_target, remote_deploy_target_changed_at, variable_expiration_handler FROM workspace_settings WHERE workspace_id = $2", &rw.new_id, &old_id ) diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 40d4d650ec..0cedbeddee 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -4304,15 +4304,11 @@ paths: $ref: "#/components/schemas/WorkspaceDeployUISettings" mute_critical_alerts: type: boolean - guest_access_enabled: - type: boolean - description: Whether this workspace admits guest sessions. An app's own `guest` execution mode is inert while this is false. add_admins_and_developers_to_forks: type: boolean description: Whether every new fork of this workspace starts with its admins and developers as members, keeping their role. required: - workspace_id - - guest_access_enabled - add_admins_and_developers_to_forks /w/{workspace}/workspaces/get_settings: @@ -4397,9 +4393,6 @@ paths: error_handler_fallback_to_instance_alerts: type: boolean description: Report failed jobs to the instance critical alert channels when no workspace error handler is set. - guest_access_enabled: - type: boolean - description: Whether this workspace admits guest sessions. An app's own `guest` execution mode is inert while this is false. guest_jwt_public_key: type: string description: PEM public key a guest JWT (`jwt_guest_`) is verified against for this workspace. Mutually exclusive with `guest_jwt_jwks_url`. @@ -6888,41 +6881,6 @@ paths: schema: type: string - /w/{workspace}/workspaces/edit_guest_access: - post: - summary: enable or disable guest sessions for this workspace - description: >- - Guests are people the identity provider authenticates who have no Windmill - account; the `guest` app execution mode admits them. Off by default. Re-read - where a guest session is minted and at the auth door on every guest request, so - turning it off takes effect immediately, for sessions already issued and for - apps whose policy already says `guest`. Turning it *on* is refused with a 400 - where guests are unavailable (the shared cloud); turning it off always works. - operationId: editGuestAccess - tags: - - workspace - parameters: - - $ref: "#/components/parameters/WorkspaceId" - requestBody: - description: Whether guest sessions are admitted - required: true - content: - application/json: - schema: - type: object - properties: - guest_access_enabled: - type: boolean - required: - - guest_access_enabled - responses: - "200": - description: status - content: - text/plain: - schema: - type: string - /w/{workspace}/workspaces/edit_add_admins_and_developers_to_forks: post: summary: choose whether new forks of this workspace start with its admins and developers @@ -6964,7 +6922,7 @@ paths: verified against this key: a PEM public key (RS/ES family, HS* refused) or a JWKS URL, at most one. Both empty clears the workspace key; off cloud, verification then falls back to the instance issuer (`JWT_EXT_JWKS_URL`) if one is set, else no guest - JWT is accepted (`guest_access_enabled` is the on/off switch). Workspace-admin gated. + JWT is accepted. Workspace-admin gated. The key is validated before it is stored. Setting a key is refused with a 400 where guests are unavailable (the shared cloud); clearing one always works. operationId: editGuestJwtKey @@ -10584,9 +10542,8 @@ paths: summary: whether the app behind a custom path admits guests description: >- The custom-path counterpart of `getGuestEntry`. Unauthenticated; 404 unless - the app's execution mode is `guest` AND its workspace has - `guest_access_enabled` AND the instance has not set `guest_access_disabled`, - and never on a deployment where guests are unavailable (the shared cloud). + the app's execution mode is `guest`, and never on a deployment where guests + are unavailable (the shared cloud). Returns the workspace too, since a custom URL may not carry it. operationId: getGuestEntryByCustomPath tags: @@ -15656,11 +15613,9 @@ paths: summary: whether the app behind a share secret admits guests description: >- Unauthenticated: what a signed-out visitor reads to learn that signing in - would let them in. 404 unless the app's execution mode is `guest` AND the - workspace has `guest_access_enabled` AND the instance has not set the - `guest_access_disabled` global setting, and never on a deployment where guests - are unavailable (the shared cloud), so it says nothing about apps that are not - open to guests. Discloses only the app path, to a caller already + would let them in. 404 unless the app's execution mode is `guest`, and never + on a deployment where guests are unavailable (the shared cloud), so it says + nothing about apps that are not open to guests. Discloses only the app path, to a caller already holding the share secret. operationId: getGuestEntry tags: @@ -32598,16 +32553,12 @@ components: Guests are free up to `free_allowance` distinct emails over the trailing `window_days`. Past that an Enterprise plan meters them (`metered`, four guests to one seat: `billable_guests`, `guest_seats`); every other plan and build - admits no new email until the count drops. `instance_enabled` is the superadmin - switch (`guest_access_disabled` global setting) every workspace switch sits under. - `available` is whether this deployment can have guests at all: false on the shared - cloud, where guest access requires a self-hosted or dedicated deployment, and every - other field and switch is then moot. + admits no new email until the count drops. `available` is whether this + deployment can have guests at all: false on the shared cloud, where guest access + requires a self-hosted or dedicated deployment, and every other field is then moot. properties: available: type: boolean - instance_enabled: - type: boolean guest_count: type: integer format: int64 @@ -32626,7 +32577,6 @@ components: format: int64 required: - available - - instance_enabled - guest_count - window_days - free_allowance @@ -37703,9 +37653,8 @@ components: requires an authenticated viewer), while updating one keeps the mode the app is already deployed under. Neither `anonymous`, which makes the app publicly executable, nor `guest`, which opens it to anyone the - identity provider authenticates, is ever assumed. A guest is only - admitted where the workspace also has `guest_access_enabled`, which is - checked when the session is minted and again on every guest request + identity provider authenticates, is ever assumed. Widening an app to + `guest` is gated by the `RestrictGuestAppDeployment` protection rule on_behalf_of: type: string description: The user or group the app runs as in anonymous or publisher mode (e.g. 'u/admin' or 'g/mygroup'). The authority for the app's identity. diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 1b684cdefc..c7b61a1211 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -306,9 +306,8 @@ pub enum ExecutionMode { /// publisher exactly as in [`ExecutionMode::Publisher`]. Such a viewer holds a /// guest session: an identity with no account at all (no `password` row, no `usr` /// row anywhere), which is what keeps it off every row-based seat counter; what a - /// guest costs instead is the allowance in `windmill_common::workspaces`. Honored - /// only where `workspace_settings.guest_access_enabled` is on, re-read at the auth - /// door on every guest request. + /// guest costs instead is the allowance in `windmill_common::workspaces`. Widening + /// an app into it is gated by `RestrictGuestAppDeployment`. Guest, /// Default for a policy that omits `execution_mode`. It MUST stay a mode /// that requires an authenticated viewer: an omitted field must never be @@ -386,8 +385,8 @@ fn refuse_guest_mode_where_unavailable( /// A guest is authorized by its token's scope and never by an ACL probe: it holds no /// `usr` row, so RLS finds nothing for it and every guest would read as having no /// access. That scope is also what keeps a guest session to the one app it was minted -/// for, even though the mode itself admits anyone signed in. The workspace's guest -/// switch is not checked here: `AuthCache` enforces it for every guest request. +/// for, even though the mode itself admits anyone signed in. The no-account rule is not +/// checked here: `AuthCache` enforces it for every guest request. pub fn authorize_non_member_viewer( mode: ExecutionMode, app_path: &str, diff --git a/backend/windmill-api/src/mcp/auto_generated_endpoints.rs b/backend/windmill-api/src/mcp/auto_generated_endpoints.rs index 9add0792ee..079ee5e519 100644 --- a/backend/windmill-api/src/mcp/auto_generated_endpoints.rs +++ b/backend/windmill-api/src/mcp/auto_generated_endpoints.rs @@ -1332,7 +1332,7 @@ is, a different one moves it there and archives the old path"), }, "execution_mode": { "type": "string", - "description": "Who may open the app, and who its runnables execute as. Optional, and what omitting it means depends on the operation: creating an app defaults it to `publisher` (runs on behalf of the app's publisher and requires an authenticated viewer), while updating one keeps the mode the app is already deployed under. Neither `anonymous`, which makes the app publicly executable, nor `guest`, which opens it to anyone the identity provider authenticates, is ever assumed. A guest is only admitted where the workspace also has `guest_access_enabled`, which is checked when the session is minted and again on every guest request. Possible values: viewer, publisher, guest, anonymous" + "description": "Who may open the app, and who its runnables execute as. Optional, and what omitting it means depends on the operation: creating an app defaults it to `publisher` (runs on behalf of the app's publisher and requires an authenticated viewer), while updating one keeps the mode the app is already deployed under. Neither `anonymous`, which makes the app publicly executable, nor `guest`, which opens it to anyone the identity provider authenticates, is ever assumed. Widening an app to `guest` is gated by the `RestrictGuestAppDeployment` protection rule. Possible values: viewer, publisher, guest, anonymous" }, "on_behalf_of": { "type": "string", @@ -1449,7 +1449,7 @@ is, a different one moves it there and archives the old path"), }, "execution_mode": { "type": "string", - "description": "Who may open the app, and who its runnables execute as. Optional, and what omitting it means depends on the operation: creating an app defaults it to `publisher` (runs on behalf of the app's publisher and requires an authenticated viewer), while updating one keeps the mode the app is already deployed under. Neither `anonymous`, which makes the app publicly executable, nor `guest`, which opens it to anyone the identity provider authenticates, is ever assumed. A guest is only admitted where the workspace also has `guest_access_enabled`, which is checked when the session is minted and again on every guest request. Possible values: viewer, publisher, guest, anonymous" + "description": "Who may open the app, and who its runnables execute as. Optional, and what omitting it means depends on the operation: creating an app defaults it to `publisher` (runs on behalf of the app's publisher and requires an authenticated viewer), while updating one keeps the mode the app is already deployed under. Neither `anonymous`, which makes the app publicly executable, nor `guest`, which opens it to anyone the identity provider authenticates, is ever assumed. Widening an app to `guest` is gated by the `RestrictGuestAppDeployment` protection rule. Possible values: viewer, publisher, guest, anonymous" }, "on_behalf_of": { "type": "string", diff --git a/backend/windmill-api/src/mcp/session/deploy.rs b/backend/windmill-api/src/mcp/session/deploy.rs index 5512c92913..1dc8b1453e 100644 --- a/backend/windmill-api/src/mcp/session/deploy.rs +++ b/backend/windmill-api/src/mcp/session/deploy.rs @@ -317,15 +317,10 @@ async fn undeployed_runnable_targets(ctx: &Ctx<'_>, runnables: &Map) -> bool { - let (Ok(usage), Ok(settings)) = ( - ctx.get(&ctx.ws("workspaces/guest_usage")).await, - ctx.get(&ctx.ws("workspaces/get_public_settings")).await, - ) else { + let Ok(usage) = ctx.get(&ctx.ws("workspaces/guest_usage")).await else { return false; }; truthy(usage.get("available")) - && truthy(usage.get("instance_enabled")) - && truthy(settings.get("guest_access_enabled")) } /// Deploys a raw app draft from its sources: the server bundles them on a worker and diff --git a/backend/windmill-api/src/mcp/session/gen/instructions.json b/backend/windmill-api/src/mcp/session/gen/instructions.json index 33ba824a4d..f5c932d4cd 100644 --- a/backend/windmill-api/src/mcp/session/gen/instructions.json +++ b/backend/windmill-api/src/mcp/session/gen/instructions.json @@ -261,6 +261,6 @@ "datatable.python3": "# Data tables: schema changes, migrations and permissions\n\nA data table is a PostgreSQL database managed by Windmill and shared by the whole workspace: scripts, flows and apps reach it by name, `main` being the default. It is configured in the workspace settings (Data Tables), never created by SQL.\n\n## Changing the schema\n\nA data table with migrations enabled keeps its schema as an ordered list of versioned SQL migrations, each an `up` and an optional `down`. That list is what git sync and a workspace merge carry, and what rebuilds the schema in another workspace. So first find out whether migrations are enabled on the data table:\n\n- **Enabled**: make every schema change (CREATE, ALTER, DROP, anything but row data) as a new migration. A change made with plain SQL exists in that one database only, and the history no longer describes it.\n- **Not enabled**: schema changes are plain SQL. Enabling migrations is a workspace admin's decision, made in the Data Tables settings, not a step to take on the way to something else.\n\nWriting a migration:\n\n- Wrap the statements in `BEGIN; ... END;` so it applies atomically. Leave the frame out only for a statement that cannot run in a transaction, such as `CREATE INDEX CONCURRENTLY`: such a migration can fail half applied.\n- Give it a `down` that undoes the `up` whenever the change is reversible. Without one it cannot be rolled back.\n- Declare the schemas it changes on a leading line of its `up`, `-- schemas sales, billing`, and qualify every name with its schema (`sales.orders`). A migration waits only on earlier migrations whose schemas overlap its own. One that declares none applies to the whole database and every later migration waits for it, so leave the line out only for a change that really spans the database, such as an extension.\n- Migrations apply oldest first. One that is applied is not edited: correct it with a new migration.\n- Row data is not schema. Insert, update and delete rows with plain SQL, migrations or not.\n\n## A migration is not a draft\n\nA migration is saved to the workspace and changes the database as soon as it is applied, for everything that reads those tables. To try a schema out without touching tables already in use:\n\n- put the new tables in a schema of their own, created by the same migration (`CREATE SCHEMA IF NOT EXISTS`), and point what you are building at it. A raw app names its schema in its `data` config, which is why a new app gets a new schema by default;\n- or work in a workspace fork whose data table was cloned when the fork was created (\"Clone schema only\" or \"Clone schema and data\"): migrations applied there change that copy alone, and reach the parent workspace when the fork is merged. A fork that kept the original data table points at the parent's database, so a migration applied in it changes the parent's tables at once.\n\nAltering or dropping a table other scripts or apps use is neither of those: say what depends on it before doing it.\n\n## Roles and permissions\n\nData table roles are a self-hosted Enterprise Edition feature, for data tables on managed Postgres (the instance database or the external managed instance). Without them, and always on a data table backed by a Postgres resource, everyone who can use the data table reaches all of it: there is no per-user, per-schema or per-row access to promise.\n\nA data table under roles is reached as a Postgres role, and Postgres enforces what that role was granted:\n\n- roles are assigned, per data table, to users, groups or folders. `admin` is the connection the data table had before roles: it owns the existing objects and ignores grants;\n- a connection either names a role (`wmill.datatable('main', { role: 'analyst' })`, `role='analyst'` in Python, `datatable://main?role=analyst`) and holds that role alone, or names none and is resolved from the data table's default role;\n- `permission denied` means the role in use lacks a grant. Say which role and which object: the grant is a data table admin's to give, in the roles drawer of the Database Manager. Do not switch to `admin` to get past it;\n- a migration runs as the role its SQL declares on its first line, `-- role `, the same one in its `down`. One that declares none runs as `admin`, which only those who may use `admin` can do.\n\nRead the Permissions section of https://www.windmill.dev/docs/core_concepts/persistent_storage/data_tables before designing who reaches what: it covers tenants, grants, the default role and the roles a connection that names none ends up holding, rows that differ per viewer through row-level security, and how forks and raw apps behave under roles.\n\n\n# Datatable SQL SDK reference\n\nDatatables are workspace-scoped managed PostgreSQL databases. In chat, explore and shape them with the datatable tools you were given. The reference below is for code you author inside runnables (inline app runnables, scripts, or flow rawscript modules) that reads or writes datatable data at runtime.\n\n- A runnable accesses a datatable via `wmill.datatable()` (the default \"main\") or `wmill.datatable('')`, referencing tables as `schema.table`.\n- Use parameterized queries (the tagged template in TypeScript, `$1`/`$2` placeholders in Python) — never interpolate untrusted values into SQL strings.\n\n## Python Datatable API (wmill)\n\nImport: `import wmill`\n\n# Get a DataTable client for SQL queries.\n# \n# Args:\n# name: Database name (default: \"main\")\n# role: Connect as this data table role instead of the data table's default one.\n# \n# Returns:\n# DataTableClient instance\ndef datatable(name: str = 'main', *, role: Optional[str] = None) -> DataTableClient\n\n# Client for executing SQL queries against Windmill DataTables.\nclass DataTableClient:\n # Initialize DataTableClient.\n # \n # Args:\n # client: Windmill client instance\n # name: DataTable name\n # role: Data table role to connect as, or None for the data table's default\n def __init__(client: Windmill, name: str, role: Optional[str] = None)\n\n # Execute a SQL query against the DataTable.\n # \n # Args:\n # sql: SQL query string with $1, $2, etc. placeholders\n # *args: Positional arguments to bind to query placeholders\n # \n # Returns:\n # SqlQuery instance for fetching results\n def query(sql: str, *args) -> SqlQuery\n\n\n# Query result handler for DataTable and DuckLake queries.\nclass SqlQuery:\n # Initialize SqlQuery.\n # \n # Args:\n # sql: SQL query string\n # fetch_fn: Function to execute the query\n def __init__(sql: str, fetch_fn)\n\n # Execute query and fetch results.\n # \n # Args:\n # result_collection: Optional result collection mode\n # \n # Returns:\n # Query results\n def fetch(result_collection: str | None = None)\n\n # Execute query and fetch first row of results.\n # \n # Returns:\n # First row of query results\n def fetch_one()\n\n # Execute query and fetch first row of results. Return result as a scalar value.\n # \n # Returns:\n # First row of query result as a scalar value\n def fetch_one_scalar()\n\n # Execute query and don't return any results.\n # \n def execute()\n\n\n", "datatable.go": "# Data tables: schema changes, migrations and permissions\n\nA data table is a PostgreSQL database managed by Windmill and shared by the whole workspace: scripts, flows and apps reach it by name, `main` being the default. It is configured in the workspace settings (Data Tables), never created by SQL.\n\n## Changing the schema\n\nA data table with migrations enabled keeps its schema as an ordered list of versioned SQL migrations, each an `up` and an optional `down`. That list is what git sync and a workspace merge carry, and what rebuilds the schema in another workspace. So first find out whether migrations are enabled on the data table:\n\n- **Enabled**: make every schema change (CREATE, ALTER, DROP, anything but row data) as a new migration. A change made with plain SQL exists in that one database only, and the history no longer describes it.\n- **Not enabled**: schema changes are plain SQL. Enabling migrations is a workspace admin's decision, made in the Data Tables settings, not a step to take on the way to something else.\n\nWriting a migration:\n\n- Wrap the statements in `BEGIN; ... END;` so it applies atomically. Leave the frame out only for a statement that cannot run in a transaction, such as `CREATE INDEX CONCURRENTLY`: such a migration can fail half applied.\n- Give it a `down` that undoes the `up` whenever the change is reversible. Without one it cannot be rolled back.\n- Declare the schemas it changes on a leading line of its `up`, `-- schemas sales, billing`, and qualify every name with its schema (`sales.orders`). A migration waits only on earlier migrations whose schemas overlap its own. One that declares none applies to the whole database and every later migration waits for it, so leave the line out only for a change that really spans the database, such as an extension.\n- Migrations apply oldest first. One that is applied is not edited: correct it with a new migration.\n- Row data is not schema. Insert, update and delete rows with plain SQL, migrations or not.\n\n## A migration is not a draft\n\nA migration is saved to the workspace and changes the database as soon as it is applied, for everything that reads those tables. To try a schema out without touching tables already in use:\n\n- put the new tables in a schema of their own, created by the same migration (`CREATE SCHEMA IF NOT EXISTS`), and point what you are building at it. A raw app names its schema in its `data` config, which is why a new app gets a new schema by default;\n- or work in a workspace fork whose data table was cloned when the fork was created (\"Clone schema only\" or \"Clone schema and data\"): migrations applied there change that copy alone, and reach the parent workspace when the fork is merged. A fork that kept the original data table points at the parent's database, so a migration applied in it changes the parent's tables at once.\n\nAltering or dropping a table other scripts or apps use is neither of those: say what depends on it before doing it.\n\n## Roles and permissions\n\nData table roles are a self-hosted Enterprise Edition feature, for data tables on managed Postgres (the instance database or the external managed instance). Without them, and always on a data table backed by a Postgres resource, everyone who can use the data table reaches all of it: there is no per-user, per-schema or per-row access to promise.\n\nA data table under roles is reached as a Postgres role, and Postgres enforces what that role was granted:\n\n- roles are assigned, per data table, to users, groups or folders. `admin` is the connection the data table had before roles: it owns the existing objects and ignores grants;\n- a connection either names a role (`wmill.datatable('main', { role: 'analyst' })`, `role='analyst'` in Python, `datatable://main?role=analyst`) and holds that role alone, or names none and is resolved from the data table's default role;\n- `permission denied` means the role in use lacks a grant. Say which role and which object: the grant is a data table admin's to give, in the roles drawer of the Database Manager. Do not switch to `admin` to get past it;\n- a migration runs as the role its SQL declares on its first line, `-- role `, the same one in its `down`. One that declares none runs as `admin`, which only those who may use `admin` can do.\n\nRead the Permissions section of https://www.windmill.dev/docs/core_concepts/persistent_storage/data_tables before designing who reaches what: it covers tenants, grants, the default role and the roles a connection that names none ends up holding, rows that differ per viewer through row-level security, and how forks and raw apps behave under roles.\n\n\n# Datatable SQL SDK reference\n\nDatatables are workspace-scoped managed PostgreSQL databases. In chat, explore and shape them with the datatable tools you were given. The reference below is for code you author inside runnables (inline app runnables, scripts, or flow rawscript modules) that reads or writes datatable data at runtime.\n\n- A runnable accesses a datatable via `wmill.datatable()` (the default \"main\") or `wmill.datatable('')`, referencing tables as `schema.table`.\n- Use parameterized queries (the tagged template in TypeScript, `$1`/`$2` placeholders in Python) — never interpolate untrusted values into SQL strings.\n\n## TypeScript Datatable API (windmill-client)\n\nImport: `import * as wmill from 'windmill-client'`\n\nSQL statement object with query content, arguments, and execution methods\n```typescript\ntype SqlStatement = {\n /** Raw SQL content with formatted arguments */\n content: string;\n\n /** Argument values keyed by parameter name */\n args: Record;\n\n /**\n * Execute the SQL query and return results\n * @param params - Optional parameters including result collection mode\n * @returns Query results based on the result collection mode\n */\n fetch(\n params?: FetchParams // The union is for auto-completion\n ): Promise>;\n\n /**\n * Execute the SQL query and return only the first row\n * @param params - Optional parameters\n * @returns First row of the query result\n */\n fetchOne(\n params?: Omit, \"resultCollection\">\n ): Promise>;\n\n /**\n * Execute the SQL query and return only the first row as a scalar value\n * @param params - Optional parameters\n * @returns First row of the query result\n */\n fetchOneScalar(\n params?: Omit<\n FetchParams<\"last_statement_first_row_scalar\">,\n \"resultCollection\"\n >\n ): Promise>;\n\n /**\n * Execute the SQL query without fetching rows\n * @param params - Optional parameters\n */\n execute(\n params?: Omit, \"resultCollection\">\n ): Promise;\n};\n```\n\n```typescript\n// Template tag function: sql`SELECT * FROM table WHERE id = ${id}`.fetch()\ninterface DatatableSqlTemplateFunction {\n // Tagged template usage:\n (strings: TemplateStringsArray, ...values: any[]): SqlStatement;\n query(sql: string, ...params: any[]): SqlStatement;\n};\n```\n\nCreate a SQL template function for PostgreSQL/datatable queries\n@param name - Database/datatable name (default: \"main\")\n@param opts.role - Connect as this data table role instead of the data table's default one.\n Only meaningful on a data table under roles, and only for a role you are a tenant of.\n@returns SQL template function for building parameterized queries\n@example\nlet sql = wmill.datatable()\nlet name = 'Robin'\nlet age = 21\nawait sql`\n SELECT * FROM friends\n WHERE name = ${name} AND age = ${age}::int\n`.fetch()\n@example\n// Read through a restricted role\nlet sql = wmill.datatable(\"main\", { role: \"analytics\" })\n```typescript\nfunction datatable(name: string = \"main\",\n opts?: DatatableOptions): DatatableSqlTemplateFunction\n```\n\n\n## Python Datatable API (wmill)\n\nImport: `import wmill`\n\n# Get a DataTable client for SQL queries.\n# \n# Args:\n# name: Database name (default: \"main\")\n# role: Connect as this data table role instead of the data table's default one.\n# \n# Returns:\n# DataTableClient instance\ndef datatable(name: str = 'main', *, role: Optional[str] = None) -> DataTableClient\n\n# Client for executing SQL queries against Windmill DataTables.\nclass DataTableClient:\n # Initialize DataTableClient.\n # \n # Args:\n # client: Windmill client instance\n # name: DataTable name\n # role: Data table role to connect as, or None for the data table's default\n def __init__(client: Windmill, name: str, role: Optional[str] = None)\n\n # Execute a SQL query against the DataTable.\n # \n # Args:\n # sql: SQL query string with $1, $2, etc. placeholders\n # *args: Positional arguments to bind to query placeholders\n # \n # Returns:\n # SqlQuery instance for fetching results\n def query(sql: str, *args) -> SqlQuery\n\n\n# Query result handler for DataTable and DuckLake queries.\nclass SqlQuery:\n # Initialize SqlQuery.\n # \n # Args:\n # sql: SQL query string\n # fetch_fn: Function to execute the query\n def __init__(sql: str, fetch_fn)\n\n # Execute query and fetch results.\n # \n # Args:\n # result_collection: Optional result collection mode\n # \n # Returns:\n # Query results\n def fetch(result_collection: str | None = None)\n\n # Execute query and fetch first row of results.\n # \n # Returns:\n # First row of query results\n def fetch_one()\n\n # Execute query and fetch first row of results. Return result as a scalar value.\n # \n # Returns:\n # First row of query result as a scalar value\n def fetch_one_scalar()\n\n # Execute query and don't return any results.\n # \n def execute()\n\n\n", "pipeline": "# Data pipeline authoring\n\nA **data pipeline** is NOT a flow. A flow is one runnable that orchestrates steps internally. A data pipeline is a set of **independent scripts**, each deployed on its own, that form a DAG by reading and writing shared **storage assets** (DuckLake tables, data tables, S3 objects, volumes, resources) and by declaring execution **triggers**. The pipeline is visualized and edited at `/pipeline/`; every node is a normal workspace script that happens to carry pipeline annotations. When the user asks for a \"data pipeline\" (or to \"ingest / transform / materialize\" data across steps), build pipeline-annotated scripts — do NOT build a flow.\n\n## Default to DuckDB + DuckLake\n\nA pipeline node that produces a table should almost always be a **`duckdb`** node that materializes its output into a **DuckLake** table with `-- materialize ducklake:///` (in a DuckDB node the annotation uses SQL `--` comment syntax; write the body as a bare `SELECT` and let the runtime do the write). DuckLake is the default lakehouse store for pipelines and is the shape the pipeline editor is built around, so prefer it unless the work specifically calls for something else:\n\n- `postgresql` / data tables — only for row-level, OLTP-style mutations against an existing Postgres data table (frequent single-row upserts/updates, transactional reads that an app queries live).\n- `bun` / `python3` — only for work that doesn't map to SQL: calling an external API, wrangling files, arbitrary glue. When such a node produces tabular data for downstream steps, write it **straight into DuckLake** (or a data table) from the same script — see \"Ingesting from Python / TypeScript\" below — rather than inventing a parallel store.\n\nDo not spread a pipeline across postgres, S3, and DuckLake when one DuckLake lake would do; a consistent DuckLake lakehouse is the goal.\n\n## Ingesting from Python / TypeScript\n\nA node that fetches data in code (an API, a SaaS export, a scraper) writes the rows **directly** into their DuckLake table with `wmill.ducklake()`, or into a data table with `wmill.datatable()`. DuckLake already stores every table as parquet files in the workspace object storage, so **never stage the raw payload first** — no intermediate JSON / CSV / parquet file in S3 and no second \"load the raw file\" node. One ingestion node, one table.\n\nPass the rows as a query argument (a list of dicts / array of objects is sent as JSON) and let SQL unnest them; declare the column types so the table gets a real schema:\n\n```python\n# pipeline\n# on schedule\nimport wmill\n\ndef main():\n rows = fetch_charges() # list[dict]\n lake = wmill.ducklake()\n lake.query(\"\"\"\n CREATE OR REPLACE TABLE stripe_charges AS\n SELECT unnest(from_json($rows, '[{\"id\":\"VARCHAR\",\"amount\":\"BIGINT\",\"created\":\"BIGINT\"}]'), recursive := true)\n \"\"\", rows=rows).fetch()\n```\n\n```ts\n// pipeline\n// on schedule\nimport * as wmill from \"windmill-client\"\n\nexport async function main() {\n const rows = await fetchCharges()\n const lake = wmill.ducklake()\n await lake`CREATE OR REPLACE TABLE stripe_charges AS\n SELECT unnest(from_json(${rows}, '[{\"id\":\"VARCHAR\",\"amount\":\"BIGINT\",\"created\":\"BIGINT\"}]'), recursive := true)`.fetch()\n}\n```\n\n- Use `INSERT INTO
SELECT ...` instead of `CREATE OR REPLACE` to append (an incremental pull), with a one-time `CREATE TABLE IF NOT EXISTS` first.\n- Assign the client to a variable (`lake = wmill.ducklake()`) and keep the SQL a **string literal** (Python) or a template without `sql.raw` (TS): that is what lets the graph record the node's write to `ducklake://main/
`. A chained `wmill.ducklake().query(...)` or SQL built in a variable records no output, so the downstream edge never forms.\n- `wmill.ducklake(\"\")` targets a non-default catalog; `wmill.datatable(\"\")` writes a Postgres data table the same way (Python `db.query(sql, *args)` with `$1` placeholders, TS the same template form) — use it only when the data belongs in Postgres (see above).\n- The downstream node triggers on the table itself: `-- on ducklake://main/stripe_charges`.\n\n## Storage prerequisites\n\nA DuckLake pipeline only runs once the workspace has **object storage** (S3 / Azure Blob / GCS) **and a DuckLake catalog** configured — DuckLake tables and `s3://` assets can't be materialized or read without it. Check which DuckLake catalogs the workspace has before you build.\nDrafting the annotated scripts does not require storage, but the pipeline can't ingest, materialize, or read its assets until it exists. So if there is none (or the user hits \"storage not configured\" errors), say so and give the right next step **by role**:\n\n- a workspace **admin** sets it up in Workspace settings → Object Storage (add an S3/Azure/GCS storage), then adds a DuckLake catalog on top of it;\n- anyone **without admin rights** should ask a workspace admin to configure object storage + a DuckLake catalog.\n\nNever hand back a DuckLake pipeline that cannot run without flagging the missing storage and pointing to who sets it up.\n\n## What makes a script a pipeline node\n\nA script joins the pipeline when its source begins with the `pipeline` annotation as a top-of-file comment, **written in the script's own comment syntax** — `//` for TS/JS (bun), `--` for SQL (DuckDB/Postgres), `#` for Python/Bash. So it's `-- pipeline` in a DuckDB node, `# pipeline` in a Python node, `// pipeline` in a bun node. Every annotation below uses that same prefix (the `//` shown is the TS form). All other wiring is expressed as annotation comments near the top of the file:\n\n- `// on ` — declares an execution-DAG **input** (what triggers/feeds this node). `` is either:\n - an **asset URI** (the node runs when that asset is produced upstream): `ducklake://main/orders`, `datatable://main/users`, `$res:f/folder/my_resource`, `volume://name/path`, or an S3 object (see the S3 storage-form rule below).\n - a **native trigger kind**: `schedule`, `webhook`, `email`, `kafka`, `mqtt`, `amqp`, `nats`, `postgres`, `sqs`, `gcp`, or `data_upload` (a user-uploaded S3 file). For these the actual trigger row (cron, topic, …) is created separately; the annotation only declares the binding. **`data_upload` is special**: there is no trigger row — the node instead declares an **`S3Object` input parameter** fed by the auto-generated upload picker; it never hard-codes a key. Any language can be the `data_upload` node:\n - Python (has `import wmill`): `def main(file: wmill.S3Object):` then `wmill.load_s3_file(file)`; TS (has `import * as wmill from \"windmill-client\"`): `export async function main(file: wmill.S3Object)`. Qualify the type as `wmill.S3Object` (or add `from wmill import S3Object` / `import { S3Object } from \"windmill-client\"`) — a bare `S3Object` is undefined.\n - **DuckDB** takes the s3object arg via a `-- $ (s3object)` declaration and reads it directly, so a single DuckDB node can ingest **and** materialize:\n ```\n -- pipeline\n -- on data_upload\n -- materialize ducklake://main/raw_uploads\n -- $file (s3object)\n SELECT * FROM read_csv($file)\n ```\n- **Outputs** are inferred from what the body writes — a `CREATE TABLE`, a `wmill.writeS3File(...)`, a DuckLake/datatable write. To declare a managed output explicitly, use `// materialize `.\n- Optional badges: `// partitioned `, `// freshness ` (e.g. `1h`), `// tag `, `// retry [delay]`, `// data_test ...` (managed DuckLake targets only — deploy rejects it beside a `dbt://` one), `// measure = [where ]` and `// dimension = ` (see \"Declared metrics\" below).\n\n## S3 object wiring (storage form matters)\n\nAn `s3://` URI's first slashes select the **storage**, not part of the key — get this wrong and the producer/consumer edge silently won't connect:\n\n- `s3:///` (**triple** slash, empty first segment) = the **default** workspace storage. A downstream node reading or triggering on that object uses `s3:///` — e.g. DuckDB `-- on s3:///orders/2024.parquet` and `read_parquet('s3:///orders/2024.parquet')`.\n- `s3:///` (**double** slash, non-empty first segment) = a **named secondary** storage called `` — so `s3://ingest/x` means storage `ingest`, key `x`, NOT key `ingest/x`. Only use this when the object genuinely lives in a configured secondary storage; never invent a bucket/storage name for a default-storage object (it breaks the edge).\n\nTo make the producer side visible to lineage, a Python/TS node MUST pass the **`S3Object` form**, not a bare key string: Python `wmill.write_s3_file(wmill.S3Object(s3=\"\"), data)` (or the import-free dict `{\"s3\": \"\"}`), TS `wmill.writeS3File({ s3: \"\" }, data)`. That records the default-storage asset `/`, which a downstream `s3:///` reader connects to (same key both sides). A bare `write_s3_file(\"\", ...)` records **no** asset and produces **no** edge. Add `storage=\"\"` only for a named secondary storage.\n\nThe key must be a **string literal** — the graph parser is static and cannot follow a variable, f-string, or computed path, so `write_s3_file(wmill.S3Object(s3=key_var), ...)` records no edge. Inline the literal (`s3=\"events/user_events.parquet\"`) on both the writing and reading node. The same rule applies to every asset URI in an annotation or SDK call (`ducklake://`, `datatable://`, `s3://`): write them literally, not via a variable.\n\n## Materialize (the managed output)\n\n> **A MANAGED `// materialize` is DuckDB-only**, and its target must be a DuckLake table (`ducklake:///
`). Deploy **rejects** a `ducklake://` `// materialize` on any other language (`python3`, `bun`, `postgresql`). For a non-DuckDB node writing the lake, do **not** use `// materialize` — write the table itself with `wmill.ducklake()` (see \"Ingesting from Python / TypeScript\") and let the output be inferred. Use `duckdb` when a node should materialize a DuckLake table.\n>\n> The one target ANY language may declare (except a dbt script, whose writes come from its manifest) is a **warehouse relation**: `// materialize manual dbt:////`, where `` is a warehouse the workspace configures under Settings → dbt. `manual` is the only mode it has — nothing generates warehouse DDL, so the node issues its own write (a postgresql `CREATE TABLE` / `INSERT`, an SDK load, …) and the annotation records the outcome. Use it on an ingestion node whose output a dbt project reads as a `source`: the declared relation and the dbt model land on ONE graph node, and a downstream `// on dbt:////` fires when the ingestion node completes.\n\nA managed `// materialize ducklake:///
` tells the runtime to write the node's output table **for you**: write the body as a single `SELECT` and the runtime wraps it in the create/replace — do **not** also write your own `CREATE TABLE` / `INSERT`. (The opposite holds for the `dbt://` target above: there the node writes its own DDL and the strategies below do not apply.) Write strategy:\n\n- no option → **replace** the whole table each run (full refresh; the only mode whose output columns may change);\n- `// materialize append` → INSERT-append rows (incremental);\n- `// materialize key=` → merge/upsert on ``.\n\n`// materialize manual ` opts **out** of managed writes — the script writes its own DDL and the annotation only records the output asset for lineage.\n\n`materialize` pairs with partitioning for incremental pipelines: a `// partitioned ` node runs **once per partition** (append/merge into a fixed-schema table). The `{partition}` token, usable in any asset URI **and** in the body SQL, is replaced at run time by the current partition's **identity string**:\n\n- To filter the source to the active slice on a time grain, use the runtime-injected macro: `WHERE wm_partition() = {partition}`. `wm_partition(ts)` buckets a timestamp in exactly the identity format the runtime uses for daily/hourly/weekly/monthly, so it always matches; never hand-write a `strftime` format.\n- Do NOT write `= TIMESTAMP {partition}`: the identity string is not a valid timestamp literal for hourly/weekly/monthly and errors at run time.\n- For `dynamic` partitioning the identity is the caller-supplied key (not a timestamp, no macro), so filter on it directly: `WHERE = {partition}`.\n\n`materialize` is an output **declaration** on a node — not a command. There is no \"materialize run\".\n\n## Declared metrics (`measure` / `dimension`)\n\nOn a node that materializes a DuckLake table, `// measure = [where ]` names the canonical way to aggregate that table (e.g. `// measure revenue = sum(amount) where not is_refund`), and `// dimension = ` names a way to slice it (e.g. `// dimension region = region`, `// dimension month = date_trunc('month', ordered_at)`). They execute nothing: they are catalogued at deploy so the editor and other agents reuse the definition instead of re-deriving it and silently disagreeing.\n\n- Keep the predicate in the `where` clause rather than folding it into the aggregate: it is rendered as ` FILTER (WHERE )`, which is what lets two measures with different predicates share one GROUP BY.\n- DuckLake-only, and only meaningful next to `// materialize`.\n- Declare one when a number carries a judgement call someone else would get wrong (refunds excluded, test rows dropped, which column is the amount); do NOT blanket every table with measures — an obvious `count(*)` earns nothing.\n- To use a metric another node declares, read that node and reuse its exact expression rather than guessing it.\n\n## How to build one\n\n1. Put every node in the **same folder**: `f//`. The folder is the pipeline.\n2. Write each node as its own script. Default to `duckdb` materializing into DuckLake (see \"Default to DuckDB + DuckLake\" above); pick `postgresql`, `bun`, or `python3` only when that section says the work calls for it.\n3. Start each body with `// pipeline`, then the `// on` input declarations, then the transform that writes the output.\n4. **Chain nodes by asset URI**: read an upstream node's output asset, then `// on ` in the downstream node so the edge forms. Reuse exact asset paths from existing nodes rather than inventing parallel ones.\n5. Don't deploy nodes unless the user asks to. A pipeline only \"runs\" once its scripts are deployed and their triggers exist.\n\n## Example (DuckDB → DuckLake, scheduled ingest + downstream transform)\n\nNode `f/sales/orders_ingest` (runs on a schedule, materializes a DuckLake table):\n\n```sql\n-- pipeline\n-- on schedule\n-- materialize ducklake://main/orders\nSELECT * FROM read_csv('s3:///raw/orders/*.csv')\n```\n\nNode `f/sales/orders_daily` (runs when `orders` is produced, writes a rollup):\n\n```sql\n-- pipeline\n-- on ducklake://main/orders\n-- materialize ducklake://main/orders_daily\nSELECT date_trunc('day', ts) AS day, count(*) AS n\nFROM ducklake.main.orders GROUP BY 1\n```\n", - "workspace_settings": "# Workspace settings\n\nEverything an admin sets on the Workspace settings page is one document: a mapping of top-level\nkeys, the same layout as the `settings.yaml` file `wmill sync` keeps. Only a workspace admin can\nread or change it.\n\n## Applying a document\n\n- A partial document changes only the keys it states; every key it omits is left as it is.\n- Each key is replaced whole, so read its current value and send it back edited: a nested object\n given with one field replaces the stored object with that one field.\n- A key set to `null` clears that setting; an on/off switch cleared this way reads as `false`.\n- Each setting is written through the same path the settings page uses, with the same validation\n and audit log. The writes are not one transaction: when one fails the apply stops, and the\n error says what was already written.\n- A dry run reports the changes, as `before` / `after` per key, without writing anything.\n- `name` is never applied: rename a workspace on the settings page. `color` is applied only when\n set to a value, so `null` does not clear it.\n- A key the server does not know is ignored and named in the warnings: check them, since a typo\n otherwise reads as \"nothing to change\".\n- A setting this edition does not offer (Enterprise only) is refused, by the dry run too.\n\n## Secrets\n\n`slack_oauth_client_secret` is the one secret. A read shows `__SENSITIVE_AND_UNCHANGED__` in place\nof a set secret; sending that placeholder back keeps the stored value. A value starting with\n`$encrypted:` is decrypted with the workspace key, and one encrypted with another workspace's key\nis left unchanged with a warning; any other value is taken as plaintext. Never\nwrite a secret you were not given: keep the placeholder.\n\n## Keys\n\n| Key | Value |\n|---|---|\n| `auto_invite` | `{ enabled, mode: invite \\| add, operator, instance_groups?, instance_groups_roles? }`. The instance groups are managed only when `instance_groups` is stated; clearing a group removes the members it added. |\n| `error_handler` / `success_handler` | `{ path, extra_args?, muted_on_cancel?, muted_on_user_path? }`, `path` being `script/` or `flow/`. |\n| `error_handler_fallback_to_instance_alerts` | Report failures to the instance critical alert channels when no error handler is set. |\n| `webhook` | URL POSTed a JSON event on every change to the workspace's items. |\n| `ai_config` | Providers, default and completion models, custom prompts, `copilot_disabled`. |\n| `large_file_storage` | Workspace object storage: `{ type, s3_resource_path \\| azure_blob_resource_path \\| gcs_resource_path, public_resource?, secondary_storage?, advanced_permissions? }`. |\n| `git_sync` | `{ repositories: [...] }`, each with `git_repo_resource_path`, `script_path` and `settings` (include/exclude paths and types). Server-owned fields (webhook secrets, sync state, credentials) are never read nor written. |\n| `datatable` | `{ datatables: { : { database: { resource_type, resource_path } } } }`, both required: `resource_type: postgresql` with `resource_path` a Postgres resource's path, or `instance`, or `external_instance` (Enterprise only), with `resource_path` the name of a database a superadmin set up. A fork's entry can instead be `{ reference: { workspace_id, datatable } }`, pointing at its parent's data table; keep it as is. `main` is the one scripts use by default. Role permissions are `datatable_permissions`. |\n| `datatable_permissions` | `{ : { default_role, roles: { : [tenants] } } }`: who may connect to each data table as which role. Tenants are `u/`, `g/`, `f/` or `*`; `admin` is always a role, and `default_role: null` means no default. A data table the value omits is no longer under roles. Only a data table on a cluster Windmill manages can be. |\n| `protection_rules` | `{ : { rules: [...], bypass_groups: [...], bypass_users: [...], fork_role_mapping?: { : } } }`, rules among `DisableDirectDeployment`, `DisableWorkspaceForking`, `RestrictDeployToDeployers`, `RestrictAnonymousAppDeployment`, `RestrictPublicRunSharing`, `RestrictGuestAppDeployment`, `RequireIsolatedForks`. A rule the value omits is deleted. The dev workspace lock is not listed and cannot be set here. |\n| `ducklake` | `{ ducklakes: { : { catalog, storage, ... } } }`. |\n| `dbt_warehouses` | dbt warehouse connections. |\n| `default_app` | Path of the app shown as the workspace home. |\n| `default_scripts` | `{ order?, hidden?, default_script_content? }` for the script language picker. |\n| `deploy_ui` | `{ include_path?, include_type? }`: what the deploy-to-workspace UI offers. |\n| `operator_settings` | What operators see and do: page visibility flags (`runs`, `schedules`, `resources`, `variables`, `assets`, `audit_logs`, `groups`, `folders`, `workers`, `triggers`) and write rights (`manage_schedules`, `manage_triggers`, `builder_flows`, `builder_apps`). A write right the value omits keeps its stored value. |\n| `mute_critical_alerts` | Mute the workspace's critical alerts. |\n| `slack_team_id`, `slack_name` | Read only: set by connecting Slack on the settings page. |\n| `slack_command_script` / `teams_command_script` | Script or flow handling the Slack / Teams command. |\n| `slack_oauth_client_id` / `slack_oauth_client_secret` | The workspace's own Slack OAuth app. Clearing either removes both. |\n| `public_app_execution_limit_per_minute` | Rate limit for public apps, `null` for none. |\n| `guest_access_enabled`, `guest_jwt_public_key`, `guest_jwt_jwks_url` | Guest access to public apps and the key that signs guest JWTs. |\n| `add_admins_and_developers_to_forks` | Add the workspace's admins and developers to each fork. |\n| `color` | Workspace color, `#rrggbb`. |\n" + "workspace_settings": "# Workspace settings\n\nEverything an admin sets on the Workspace settings page is one document: a mapping of top-level\nkeys, the same layout as the `settings.yaml` file `wmill sync` keeps. Only a workspace admin can\nread or change it.\n\n## Applying a document\n\n- A partial document changes only the keys it states; every key it omits is left as it is.\n- Each key is replaced whole, so read its current value and send it back edited: a nested object\n given with one field replaces the stored object with that one field.\n- A key set to `null` clears that setting; an on/off switch cleared this way reads as `false`.\n- Each setting is written through the same path the settings page uses, with the same validation\n and audit log. The writes are not one transaction: when one fails the apply stops, and the\n error says what was already written.\n- A dry run reports the changes, as `before` / `after` per key, without writing anything.\n- `name` is never applied: rename a workspace on the settings page. `color` is applied only when\n set to a value, so `null` does not clear it.\n- A key the server does not know is ignored and named in the warnings: check them, since a typo\n otherwise reads as \"nothing to change\".\n- A setting this edition does not offer (Enterprise only) is refused, by the dry run too.\n\n## Secrets\n\n`slack_oauth_client_secret` is the one secret. A read shows `__SENSITIVE_AND_UNCHANGED__` in place\nof a set secret; sending that placeholder back keeps the stored value. A value starting with\n`$encrypted:` is decrypted with the workspace key, and one encrypted with another workspace's key\nis left unchanged with a warning; any other value is taken as plaintext. Never\nwrite a secret you were not given: keep the placeholder.\n\n## Keys\n\n| Key | Value |\n|---|---|\n| `auto_invite` | `{ enabled, mode: invite \\| add, operator, instance_groups?, instance_groups_roles? }`. The instance groups are managed only when `instance_groups` is stated; clearing a group removes the members it added. |\n| `error_handler` / `success_handler` | `{ path, extra_args?, muted_on_cancel?, muted_on_user_path? }`, `path` being `script/` or `flow/`. |\n| `error_handler_fallback_to_instance_alerts` | Report failures to the instance critical alert channels when no error handler is set. |\n| `webhook` | URL POSTed a JSON event on every change to the workspace's items. |\n| `ai_config` | Providers, default and completion models, custom prompts, `copilot_disabled`. |\n| `large_file_storage` | Workspace object storage: `{ type, s3_resource_path \\| azure_blob_resource_path \\| gcs_resource_path, public_resource?, secondary_storage?, advanced_permissions? }`. |\n| `git_sync` | `{ repositories: [...] }`, each with `git_repo_resource_path`, `script_path` and `settings` (include/exclude paths and types). Server-owned fields (webhook secrets, sync state, credentials) are never read nor written. |\n| `datatable` | `{ datatables: { : { database: { resource_type, resource_path } } } }`, both required: `resource_type: postgresql` with `resource_path` a Postgres resource's path, or `instance`, or `external_instance` (Enterprise only), with `resource_path` the name of a database a superadmin set up. A fork's entry can instead be `{ reference: { workspace_id, datatable } }`, pointing at its parent's data table; keep it as is. `main` is the one scripts use by default. Role permissions are `datatable_permissions`. |\n| `datatable_permissions` | `{ : { default_role, roles: { : [tenants] } } }`: who may connect to each data table as which role. Tenants are `u/`, `g/`, `f/` or `*`; `admin` is always a role, and `default_role: null` means no default. A data table the value omits is no longer under roles. Only a data table on a cluster Windmill manages can be. |\n| `protection_rules` | `{ : { rules: [...], bypass_groups: [...], bypass_users: [...], fork_role_mapping?: { : } } }`, rules among `DisableDirectDeployment`, `DisableWorkspaceForking`, `RestrictDeployToDeployers`, `RestrictAnonymousAppDeployment`, `RestrictPublicRunSharing`, `RestrictGuestAppDeployment`, `RequireIsolatedForks`. A rule the value omits is deleted. The dev workspace lock is not listed and cannot be set here. |\n| `ducklake` | `{ ducklakes: { : { catalog, storage, ... } } }`. |\n| `dbt_warehouses` | dbt warehouse connections. |\n| `default_app` | Path of the app shown as the workspace home. |\n| `default_scripts` | `{ order?, hidden?, default_script_content? }` for the script language picker. |\n| `deploy_ui` | `{ include_path?, include_type? }`: what the deploy-to-workspace UI offers. |\n| `operator_settings` | What operators see and do: page visibility flags (`runs`, `schedules`, `resources`, `variables`, `assets`, `audit_logs`, `groups`, `folders`, `workers`, `triggers`) and write rights (`manage_schedules`, `manage_triggers`, `builder_flows`, `builder_apps`). A write right the value omits keeps its stored value. |\n| `mute_critical_alerts` | Mute the workspace's critical alerts. |\n| `slack_team_id`, `slack_name` | Read only: set by connecting Slack on the settings page. |\n| `slack_command_script` / `teams_command_script` | Script or flow handling the Slack / Teams command. |\n| `slack_oauth_client_id` / `slack_oauth_client_secret` | The workspace's own Slack OAuth app. Clearing either removes both. |\n| `public_app_execution_limit_per_minute` | Rate limit for public apps, `null` for none. |\n| `guest_jwt_public_key`, `guest_jwt_jwks_url` | The key that signs guest JWTs for apps set to Guests. |\n| `add_admins_and_developers_to_forks` | Add the workspace's admins and developers to each fork. |\n| `color` | Workspace color, `#rrggbb`. |\n" } } diff --git a/backend/windmill-api/src/mcp/session/workspace.rs b/backend/windmill-api/src/mcp/session/workspace.rs index ef31947243..720dbbac76 100644 --- a/backend/windmill-api/src/mcp/session/workspace.rs +++ b/backend/windmill-api/src/mcp/session/workspace.rs @@ -800,20 +800,15 @@ pub(super) async fn describe_app_exposure(ctx: &Ctx<'_>, mode: Option<&Value>) - if mode.and_then(Value::as_str) != Some("guest") { return mode.cloned(); } - let usage = ctx.get(&ctx.ws("workspaces/guest_usage")).await; - let settings = ctx.get(&ctx.ws("workspaces/get_public_settings")).await; - // A switch that could not be read proves nothing either way: report the mode bare. - let (Ok(usage), Ok(settings)) = (usage, settings) else { + // A deployment that could not be read proves nothing either way: report the mode bare. + let Ok(usage) = ctx.get(&ctx.ws("workspaces/guest_usage")).await else { return mode.cloned(); }; - let live = truthy(usage.get("available")) - && truthy(usage.get("instance_enabled")) - && truthy(settings.get("guest_access_enabled")); - if live { + if truthy(usage.get("available")) { mode.cloned() } else { Some(json!( - "guest, but inert: the instance or workspace admits no guest, so this app admits members only" + "guest, but inert: this deployment admits no guest, so this app admits members only" )) } } diff --git a/backend/windmill-api/src/workspace_settings_document.rs b/backend/windmill-api/src/workspace_settings_document.rs index cb0c2e67aa..5f0f062d10 100644 --- a/backend/windmill-api/src/workspace_settings_document.rs +++ b/backend/windmill-api/src/workspace_settings_document.rs @@ -81,7 +81,6 @@ struct SettingsRow { deploy_ui: Option, teams_command_script: Option, public_app_execution_limit_per_minute: Option, - guest_access_enabled: Option, guest_jwt_public_key: Option, guest_jwt_jwks_url: Option, error_handler_fallback_to_instance_alerts: Option, @@ -272,16 +271,18 @@ async fn protection_rules_document(conn: &mut PgConnection, w_id: &str) -> Resul .await?; Ok(Value::Object( rows.into_iter() - .map(|(name, bits, bypass_groups, bypass_users, fork_role_mapping)| { - let rules = ProtectionRules::from_bits_truncate(bits); - let mut rule = json!({ - "rules": rule_kinds(rules), - "bypass_groups": bypass_groups, - "bypass_users": bypass_users, - }); - with_fork_role_mapping(&mut rule, fork_role_mapping); - (name, rule) - }) + .map( + |(name, bits, bypass_groups, bypass_users, fork_role_mapping)| { + let rules = ProtectionRules::from_bits_truncate(bits); + let mut rule = json!({ + "rules": rule_kinds(rules), + "bypass_groups": bypass_groups, + "bypass_users": bypass_users, + }); + with_fork_role_mapping(&mut rule, fork_role_mapping); + (name, rule) + }, + ) .collect(), )) } @@ -312,7 +313,7 @@ pub async fn load_settings_document( git_sync, default_app, default_scripts, workspace.name AS name, mute_critical_alerts, color, operator_settings, datatable, slack_team_id, slack_name, slack_command_script, slack_oauth_client_id, slack_oauth_client_secret, ducklake, dbt_warehouses, deploy_ui, - teams_command_script, public_app_execution_limit_per_minute, guest_access_enabled, + teams_command_script, public_app_execution_limit_per_minute, guest_jwt_public_key, guest_jwt_jwks_url, error_handler_fallback_to_instance_alerts, add_admins_and_developers_to_forks FROM workspace_settings @@ -360,7 +361,7 @@ pub async fn load_settings_document( // In the order `wmill sync pull` has always written settings.yaml. A key that predates the // document and is not marked `always` is written only when set, as it always was; a key // added since is always written, so a pulled file states it and from then on manages it. - let entries: [(&str, Option, bool); 32] = [ + let entries: [(&str, Option, bool); 31] = [ ("auto_invite", row.auto_invite, false), ("webhook", row.webhook.map(Value::String), false), ("error_handler", row.error_handler, true), @@ -418,11 +419,6 @@ pub async fn load_settings_document( row.public_app_execution_limit_per_minute.map(|n| json!(n)), true, ), - ( - "guest_access_enabled", - Some(json!(row.guest_access_enabled.unwrap_or(false))), - true, - ), ( "guest_jwt_public_key", row.guest_jwt_public_key.map(Value::String), @@ -565,7 +561,6 @@ fn same(a: Option<&Value>, b: Option<&Value>) -> bool { /// Switches the endpoint stores as `false` when cleared, compared as `false` when unset. const FALSE_WHEN_UNSET: &[&str] = &[ "mute_critical_alerts", - "guest_access_enabled", "add_admins_and_developers_to_forks", "error_handler_fallback_to_instance_alerts", ]; @@ -690,6 +685,10 @@ fn resolve_secret( } } +/// Keys earlier versions wrote that no longer configure anything. A synced `settings.yaml` +/// keeps them until its next pull, so they are dropped without the unknown-key warning. +const RETIRED_KEYS: &[&str] = &["guest_access_enabled"]; + const KNOWN_KEYS: &[&str] = &[ "auto_invite", "webhook", @@ -716,7 +715,6 @@ const KNOWN_KEYS: &[&str] = &[ "deploy_ui", "teams_command_script", "public_app_execution_limit_per_minute", - "guest_access_enabled", "guest_jwt_public_key", "guest_jwt_jwks_url", "error_handler_fallback_to_instance_alerts", @@ -1072,7 +1070,7 @@ fn plan_document( let unknown: Vec<&str> = desired .keys() .map(String::as_str) - .filter(|k| !KNOWN_KEYS.contains(k)) + .filter(|k| !KNOWN_KEYS.contains(k) && !RETIRED_KEYS.contains(k)) .collect(); if !unknown.is_empty() { warnings.push(format!("Ignored unknown settings: {}", unknown.join(", "))); @@ -1540,14 +1538,6 @@ fn plan_document( |v| json!({ "public_app_execution_limit_per_minute": v }), ); - let guest_access = null_as_false(p.managed("guest_access_enabled")); - p.simple( - "guest_access_enabled", - guest_access, - Target::Edit(SettingsEdit::GuestAccess), - |v| json!({ "guest_access_enabled": v.as_bool().unwrap_or(false) }), - ); - let (public_key, jwks_url) = ( p.managed("guest_jwt_public_key"), p.managed("guest_jwt_jwks_url"), @@ -2008,7 +1998,10 @@ mod tests { #[test] fn unset_switches_compare_as_false() { let mc = WorkspaceCrypt::new("k"); - let d = doc(json!({ "mute_critical_alerts": false, "guest_access_enabled": null })); + let d = doc(json!({ + "mute_critical_alerts": false, + "add_admins_and_developers_to_forks": null + })); assert!( plan_document(current(), d, false, &mc, &DatatableNames::default()) .unwrap() @@ -2017,6 +2010,14 @@ mod tests { ); } + #[test] + fn retired_keys_are_dropped_without_a_warning() { + let mc = WorkspaceCrypt::new("k"); + let d = doc(json!({ "guest_access_enabled": true })); + let p = plan_document(current(), d, false, &mc, &DatatableNames::default()).unwrap(); + assert!(p.changes.is_empty() && p.writes.is_empty() && p.warnings.is_empty()); + } + #[test] fn stating_one_guest_key_clears_the_other() { let mc = WorkspaceCrypt::new("k"); @@ -2033,7 +2034,6 @@ mod tests { fn mistyped_values_are_refused_rather_than_coerced() { let mc = WorkspaceCrypt::new("k"); for d in [ - json!({ "guest_access_enabled": "true" }), json!({ "add_admins_and_developers_to_forks": 1 }), json!({ "slack_oauth_client_id": 123 }), json!({ "error_handler": { "path": "f/x", "muted_on_cancel": "yes" } }), diff --git a/backend/windmill-common/src/global_settings.rs b/backend/windmill-common/src/global_settings.rs index bc1af69353..371eff43dd 100644 --- a/backend/windmill-common/src/global_settings.rs +++ b/backend/windmill-common/src/global_settings.rs @@ -76,10 +76,6 @@ pub const EXPOSE_METRICS_SETTING: &str = "expose_metrics"; pub const EXPOSE_DEBUG_METRICS_SETTING: &str = "expose_debug_metrics"; pub const KEEP_JOB_DIR_SETTING: &str = "keep_job_dir"; pub const REQUIRE_PREEXISTING_USER_FOR_OAUTH_SETTING: &str = "require_preexisting_user_for_oauth"; -/// Superadmin switch over guest sessions for the whole instance, above the per-workspace -/// one. Read from the table, uncached, by the same gates that read the workspace switch; -/// the superadmin Guests list writes it through `/settings/global/{key}` by this name. -pub const GUEST_ACCESS_DISABLED_SETTING: &str = "guest_access_disabled"; pub const JOB_ISOLATION_SETTING: &str = "job_isolation"; pub const NSJAIL_TMPFS_SIZE_MB_SETTING: &str = "nsjail_tmpfs_size_mb"; pub const NSJAIL_TMP_BACKING_SETTING: &str = "nsjail_tmp_backing"; diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 2a266ba813..7f4c60faa3 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -906,12 +906,8 @@ pub async fn guest_count_in_window<'c, E: sqlx::Executor<'c, Database = sqlx::Po #[derive(Clone, Debug, Serialize)] pub struct GuestUsage { /// Whether this deployment can admit guests at all ([`instance_supports_guests`]). - /// Off, every other field is moot and no switch below can turn guests on. + /// Off, every other field is moot and an app in `guest` mode admits members only. pub available: bool, - /// The superadmin switch (`GUEST_ACCESS_DISABLED_SETTING`), which every workspace - /// switch sits under. Reported as stored, so a superadmin sees what they set even - /// where `available` overrules it. - pub instance_enabled: bool, /// Distinct guest emails over the trailing `window_days`. pub guest_count: i64, pub window_days: i32, @@ -931,8 +927,8 @@ pub const GUESTS_UNAVAILABLE_MESSAGE: &str = /// Whether guests can exist on this deployment at all. They cannot on the shared cloud: /// a guest is an identity Windmill itself never vouched for, admitted on the say-so of /// whoever runs the instance, which is not a call a multi-tenant deployment can make for -/// its tenants. Folded into every guest gate below, so a workspace switch or an app -/// policy left saying `guest` is inert rather than honored. +/// its tenants. Folded into every guest gate below, so an app policy left saying `guest` +/// is inert rather than honored. pub fn instance_supports_guests() -> bool { !*crate::worker::CLOUD_HOSTED } @@ -947,31 +943,7 @@ pub fn require_guest_support() -> Result<()> { } } -/// SQL for the superadmin switch alone, absent meaning on. The setting is read as text -/// before the cast so `true` and `"true"` both count. -fn instance_switch_sql() -> String { - format!( - "NOT COALESCE((SELECT (value #>> '{{}}')::boolean FROM global_settings \ - WHERE name = '{}'), false)", - crate::global_settings::GUEST_ACCESS_DISABLED_SETTING - ) -} - -/// SQL for "the instance admits guests": the superadmin switch, under -/// [`instance_supports_guests`]. -fn instance_admits_guests_sql() -> String { - if !instance_supports_guests() { - return "false".to_string(); - } - instance_switch_sql() -} - pub async fn guest_usage(db: &crate::DB) -> Result { - let instance_switch = instance_switch_sql(); - let instance_enabled: bool = sqlx::query_scalar(&format!("SELECT {instance_switch}")) - .fetch_one(db) - .await - .map_err(|e| Error::internal_err(format!("reading the instance guest switch: {e:#}")))?; let guest_count = guest_count_in_window(db).await?; let metered = guests_are_metered().await; let billable_guests = if metered { @@ -981,7 +953,6 @@ pub async fn guest_usage(db: &crate::DB) -> Result { }; Ok(GuestUsage { available: instance_supports_guests(), - instance_enabled, guest_count, window_days: GUEST_WINDOW_DAYS, free_allowance: FREE_GUESTS_PER_WINDOW, @@ -1025,19 +996,19 @@ pub async fn guest_admission(conn: &mut sqlx::PgConnection, email: &str) -> Resu ))) } -/// Whether a guest session for `email` in `w_id` still stands: the instance and the -/// workspace admit guests, and the email still has no account. Read at the auth door on -/// every guest request, so turning either switch off, or an account provisioned after -/// the mint (or racing it), ends the session on its next request. +/// Whether a guest session for `email` in `w_id` still stands: the deployment admits +/// guests, and the email still has no account. Read at the auth door on every guest +/// request, so an account provisioned after the mint (or racing it) ends the session on +/// its next request. Whether its app still admits guests is the on-behalf resolver's call. pub async fn guest_session_stands(db: &crate::DB, w_id: &str, email: &str) -> Result { - let instance_admits = instance_admits_guests_sql(); - let stands: Option = sqlx::query_scalar(&format!( - "SELECT guest_access_enabled - AND {instance_admits} - AND NOT EXISTS(SELECT 1 FROM password WHERE email = $2) + if !instance_supports_guests() { + return Ok(false); + } + let stands: Option = sqlx::query_scalar( + "SELECT NOT EXISTS(SELECT 1 FROM password WHERE email = $2) AND NOT EXISTS(SELECT 1 FROM usr WHERE email = $2) - FROM workspace_settings WHERE workspace_id = $1" - )) + FROM workspace_settings WHERE workspace_id = $1", + ) .bind(w_id) .bind(email) .fetch_optional(db) @@ -1046,27 +1017,23 @@ pub async fn guest_session_stands(db: &crate::DB, w_id: &str, email: &str) -> Re Ok(stands.unwrap_or(false)) } -/// Every switch at once: the instance's, the workspace's, and `app_path` being in -/// `guest` execution mode. The single answer to "may a guest session be minted for this -/// app", used by the mint itself and by the sign-in branch that decides whether to call -/// it. A missing app or a policy with no stated mode reads as "no". The allowance is -/// `guest_admission`. +/// Whether the deployment admits guests and `app_path` is in `guest` execution mode. +/// The single answer to "may a guest session be minted for this app", used by the mint +/// itself and by the sign-in branch that decides whether to call it. A missing app or a +/// policy with no stated mode reads as "no". The allowance is `guest_admission`. pub async fn guest_app_admits<'c, E: sqlx::Executor<'c, Database = sqlx::Postgres>>( executor: E, w_id: &str, app_path: &str, ) -> Result { // The mint refuses a path it cannot scope, so discovery must not advertise one. - if !crate::auth::is_scope_literal_path(app_path) { + if !instance_supports_guests() || !crate::auth::is_scope_literal_path(app_path) { return Ok(false); } - let instance_admits = instance_admits_guests_sql(); - let admits: Option = sqlx::query_scalar(&format!( - "SELECT COALESCE(ws.guest_access_enabled AND app.policy->>'execution_mode' = 'guest', false) - AND {instance_admits} - FROM app JOIN workspace_settings ws ON ws.workspace_id = app.workspace_id - WHERE app.workspace_id = $1 AND app.path = $2" - )) + let admits: Option = sqlx::query_scalar( + "SELECT COALESCE(policy->>'execution_mode' = 'guest', false) + FROM app WHERE workspace_id = $1 AND path = $2", + ) .bind(w_id) .bind(app_path) .fetch_optional(executor) diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index cdf5a1ba1d..f4962d3a65 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -14,7 +14,7 @@ import { colors } from "@cliffy/ansi/colors"; import { Command } from "@cliffy/command"; import { Confirm } from "@cliffy/prompt/confirm"; import * as log from "../../core/log.ts"; -import { SETTINGS_MANAGED_ONLY_WHEN_PRESENT } from "../../core/settings.ts"; +import { RETIRED_SETTINGS, SETTINGS_MANAGED_ONLY_WHEN_PRESENT } from "../../core/settings.ts"; import * as path from "node:path"; import { sep as SEP } from "node:path"; import { @@ -2606,6 +2606,10 @@ const isWorkspaceSettingsFile = (p: string) => function stripUnappliedSettingsFields(local: any, remote: any) { delete local?.name; delete remote?.name; + for (const key of RETIRED_SETTINGS) { + delete local?.[key]; + delete remote?.[key]; + } for (const key of ["operator_settings", ...SETTINGS_MANAGED_ONLY_WHEN_PRESENT]) { if (local?.[key] === undefined) delete remote?.[key]; } diff --git a/cli/src/core/settings.ts b/cli/src/core/settings.ts index 94875d2659..9891440dd4 100644 --- a/cli/src/core/settings.ts +++ b/cli/src/core/settings.ts @@ -72,7 +72,6 @@ export interface SimplifiedSettings { deploy_ui?: any; teams_command_script?: string | null; public_app_execution_limit_per_minute?: number | null; - guest_access_enabled?: boolean; guest_jwt_public_key?: string | null; guest_jwt_jwks_url?: string | null; error_handler_fallback_to_instance_alerts?: boolean; @@ -89,7 +88,6 @@ export const SETTINGS_MANAGED_ONLY_WHEN_PRESENT = [ "deploy_ui", "teams_command_script", "public_app_execution_limit_per_minute", - "guest_access_enabled", "guest_jwt_public_key", "guest_jwt_jwks_url", "error_handler_fallback_to_instance_alerts", @@ -97,6 +95,10 @@ export const SETTINGS_MANAGED_ONLY_WHEN_PRESENT = [ "variable_expiration_handler", ] as const; +/** Keys earlier versions wrote that no longer configure anything; the server ignores them, + * so a file that still carries one must not show as differing from the workspace. */ +export const RETIRED_SETTINGS = ["guest_access_enabled"] as const; + // Legacy settings interface for reading old settings.yaml files interface LegacySimplifiedSettings { auto_invite_enabled?: boolean; diff --git a/cli/src/guidance/skills.gen.ts b/cli/src/guidance/skills.gen.ts index ce715fe46a..cdd283b904 100644 --- a/cli/src/guidance/skills.gen.ts +++ b/cli/src/guidance/skills.gen.ts @@ -8509,7 +8509,7 @@ write a secret you were not given: keep the placeholder. | \`slack_command_script\` / \`teams_command_script\` | Script or flow handling the Slack / Teams command. | | \`slack_oauth_client_id\` / \`slack_oauth_client_secret\` | The workspace's own Slack OAuth app. Clearing either removes both. | | \`public_app_execution_limit_per_minute\` | Rate limit for public apps, \`null\` for none. | -| \`guest_access_enabled\`, \`guest_jwt_public_key\`, \`guest_jwt_jwks_url\` | Guest access to public apps and the key that signs guest JWTs. | +| \`guest_jwt_public_key\`, \`guest_jwt_jwks_url\` | The key that signs guest JWTs for apps set to Guests. | | \`add_admins_and_developers_to_forks\` | Add the workspace's admins and developers to each fork. | | \`color\` | Workspace color, \`#rrggbb\`. | @@ -8527,8 +8527,8 @@ With \`includeSettings: true\` in \`wmill.yaml\`, \`wmill sync pull\` writes the \`wmill sync push\` applies it. Unlike a partial apply, a push treats the file as the whole document: a setting the file omits is cleared, except \`operator_settings\`, \`color\`, the auto-invite instance groups, and the keys \`ducklake\`, \`dbt_warehouses\`, \`deploy_ui\`, \`teams_command_script\`, -\`public_app_execution_limit_per_minute\`, \`guest_access_enabled\`, \`guest_jwt_public_key\`, -\`guest_jwt_jwks_url\`, \`error_handler_fallback_to_instance_alerts\` and +\`public_app_execution_limit_per_minute\`, \`guest_jwt_public_key\`, \`guest_jwt_jwks_url\`, +\`error_handler_fallback_to_instance_alerts\` and \`add_admins_and_developers_to_forks\`, which are managed only when the file states them. \`datatable_permissions\` is pushed only with \`includeDatatablePermissions: true\` (or \`--include-datatable-permissions\`). A push ignores \`protection_rules\`: \`protection-rules.yaml\` and diff --git a/docs/operator-builder-rights.md b/docs/operator-builder-rights.md index d4dc42b2d3..08ce69b6ba 100644 --- a/docs/operator-builder-rights.md +++ b/docs/operator-builder-rights.md @@ -202,8 +202,8 @@ already authors elsewhere must not be charged again. A change that adds no seat - A builder may make an app public (`ExecutionMode::Anonymous`): anyone with the URL runs the runnables its policy names, as the builder. Refusing it is the workspace's call, through the `RestrictAnonymousAppDeployment` protection rule. A `Guest` app is the same with a login in - front: anyone the instance's identity provider admits, member or not, once guest access is - enabled. `RestrictGuestAppDeployment` refuses it. + front: anyone the instance's identity provider admits, member or not. + `RestrictGuestAppDeployment` refuses it. - A builder raw app may set `allow_user_resources` on a runnable's inputs, and the bundle picks the `$res:` path it sends. `build_args` resolves that path as the *viewer*, with no prompt, into a job that runs as the builder. Getting a value out takes a guessable path and a readable script diff --git a/frontend/src/lib/components/SuperadminSettingsInner.svelte b/frontend/src/lib/components/SuperadminSettingsInner.svelte index eb5559a2aa..4173045853 100644 --- a/frontend/src/lib/components/SuperadminSettingsInner.svelte +++ b/frontend/src/lib/components/SuperadminSettingsInner.svelte @@ -813,7 +813,6 @@ loading={guestLoading} onLoadMore={() => loadGuestPage(Math.floor((guestList?.guests.length ?? 0) / guestPerPage) + 1)} - onInstanceSwitch={() => loadGuestPage(1)} /> {/if} diff --git a/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte b/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte index 6118f6626d..9037aec2d4 100644 --- a/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte +++ b/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte @@ -122,10 +122,6 @@ ? 'guest' : 'publisher' ) - // Undefined until loaded. An app can be set to `guest` while the workspace has - // guests off, in which case the mode is stored but inert -- say so rather than - // letting the publisher believe the app is open. - let guestAccessEnabled: boolean | undefined = $state(undefined) let guestUsage: GuestUsage | undefined = $state(undefined) // Whether the deployment can have guests at all; off, the mode is not on offer. The // backend decides; the hostname stands in until it has answered, the shared cloud @@ -136,9 +132,6 @@ const ws = opWs if (ws === undefined) return untrack(() => { - WorkspaceService.getPublicSettings({ workspace: ws }) - .then((s) => (guestAccessEnabled = s.guest_access_enabled)) - .catch(() => (guestAccessEnabled = undefined)) WorkspaceService.getGuestUsage({ workspace: ws }) .then((u) => (guestUsage = u)) .catch(() => (guestUsage = undefined)) @@ -553,14 +546,6 @@ {#if !guestsAvailable} Guests are not available on Windmill Cloud, so this app still admits members only. They require a self-hosted instance or a dedicated Windmill Cloud deployment. - {:else if guestUsage && !guestUsage.instance_enabled} - A superadmin has turned guests off for this instance, so this app still admits members - only. - {:else if guestAccessEnabled === undefined} - Checking whether this workspace allows guests… - {:else if guestAccessEnabled === false} - Guests are turned off for this workspace, so this app still admits members only. A - workspace admin can turn them on in the workspace settings. {:else} Anyone your identity provider authenticates can open this app without a Windmill account. They join no workspace. Members of this workspace can open it too. @@ -614,7 +599,7 @@ - {#if embedMode && policy.execution_mode == 'guest' && guestAccessEnabled && guestJwtBase && guestsAvailable} + {#if embedMode && policy.execution_mode == 'guest' && guestJwtBase && guestsAvailable}
Embed for your own authenticated users (guest JWT) diff --git a/frontend/src/lib/components/copilot/chat/global/core.test.ts b/frontend/src/lib/components/copilot/chat/global/core.test.ts index ff686b6de7..66d105a216 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -260,10 +260,9 @@ vi.mock('$lib/gen', async () => { WorkerService: wrapService(actual.WorkerService, { listWorkers: vi.fn(async () => []) }), - // Guests off by default, as a fresh workspace has them. + // Guests unavailable by default, as on the shared cloud. WorkspaceService: wrapService(actual.WorkspaceService, { - getGuestUsage: vi.fn(async () => ({ available: false, instance_enabled: false })), - getPublicSettings: vi.fn(async () => ({ guest_access_enabled: false })) + getGuestUsage: vi.fn(async () => ({ available: false })) }), FolderService: wrapService(actual.FolderService, { createFolder: vi.fn(async () => 'created') @@ -3933,11 +3932,7 @@ describe('global AI tools', () => { } vi.mocked(WorkspaceService.getGuestUsage).mockResolvedValueOnce({ - available: true, - instance_enabled: true - } as any) - vi.mocked(WorkspaceService.getPublicSettings).mockResolvedValueOnce({ - guest_access_enabled: true + available: true } as any) expect(await readMode(guestApp)).toBe('guest') @@ -3951,19 +3946,8 @@ describe('global AI tools', () => { }) ).toBe('anonymous') - // The same app, with each switch crossed in turn: either one alone admits nobody, so - // reporting the mode bare would name an exposure the server refuses. Both off needs - // no case of its own — whichever half of the check were dropped, one of these two - // still catches it. - vi.mocked(WorkspaceService.getPublicSettings).mockResolvedValueOnce({ - guest_access_enabled: true - } as any) - expect(await readMode(guestApp)).toContain('inert') - - vi.mocked(WorkspaceService.getGuestUsage).mockResolvedValueOnce({ - available: true, - instance_enabled: true - } as any) + // The same app where the deployment admits no guest: reporting the mode bare would + // name an exposure the server refuses. expect(await readMode(guestApp)).toContain('inert') }) @@ -4734,36 +4718,19 @@ describe('global AI tools', () => { // anyone outside the deployers group — so the note must not name an identity. expect(open.message).not.toContain('u/alice') - // Guest is a real widening only where the deployment, the instance and the - // workspace all admit guests; stored below that, it is inert. + // Guest is a real widening only where the deployment admits guests; stored on one + // that does not, it is inert and the note would announce an exposure that does not + // exist. vi.mocked(WorkspaceService.getGuestUsage).mockResolvedValueOnce({ - available: true, - instance_enabled: true - } as any) - vi.mocked(WorkspaceService.getPublicSettings).mockResolvedValueOnce({ - guest_access_enabled: true + available: true } as any) const guestOn = await deployApp('f/apps/guest', { execution_mode: 'guest' }) expect(guestOn.success).toBe(true) expect(guestOn.message).toContain('identity provider authenticates') - // Each switch crossed in turn, because either one alone admits nobody and the note - // would then announce an exposure that does not exist. Both off needs no case of its - // own: whichever half of the check were dropped, one of these two still catches it. - vi.mocked(WorkspaceService.getPublicSettings).mockResolvedValueOnce({ - guest_access_enabled: true - } as any) - const instanceOff = await deployApp('f/apps/guest_inst_off', { execution_mode: 'guest' }) - expect(instanceOff.success).toBe(true) - expect(instanceOff.message).not.toContain('identity provider') - - vi.mocked(WorkspaceService.getGuestUsage).mockResolvedValueOnce({ - available: true, - instance_enabled: true - } as any) - const workspaceOff = await deployApp('f/apps/guest_ws_off', { execution_mode: 'guest' }) - expect(workspaceOff.success).toBe(true) - expect(workspaceOff.message).not.toContain('identity provider') + const unavailable = await deployApp('f/apps/guest_unavailable', { execution_mode: 'guest' }) + expect(unavailable.success).toBe(true) + expect(unavailable.message).not.toContain('identity provider') }) it('forwards preserve_on_behalf_of when the deployed policy carries an on_behalf_of', async () => { diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 50fde9e552..bc57802dbf 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -2519,18 +2519,15 @@ const triggerServices: Record = { } /** Whether the server would admit a guest here: the deployment has to support guests at - * all, and the instance and workspace switches both have to be on. `guest` is stored on - * an app even when none of that holds, so the mode alone never settles who can open it. - * `undefined` when a switch read fails — neither proven live nor proven inert. */ + * all. `guest` is stored on an app even where it does not, so the mode alone never + * settles who can open it. `undefined` when the read fails — neither proven live nor + * proven inert. */ async function guestAccessIsLive(workspace: string): Promise { - const [usage, settings] = await Promise.all([ - WorkspaceService.getGuestUsage({ workspace }).catch(() => undefined), - WorkspaceService.getPublicSettings({ workspace }).catch(() => undefined) - ]) - if (usage === undefined || settings === undefined) { + const usage = await WorkspaceService.getGuestUsage({ workspace }).catch(() => undefined) + if (usage === undefined) { return undefined } - return !!(usage.available && usage.instance_enabled && settings.guest_access_enabled) + return !!usage.available } /** Who may open an app, from the mode stored on it. An app sits in `guest` mode whether @@ -2544,7 +2541,7 @@ async function describeAppExposure( if (mode !== 'guest' || (await guestAccessIsLive(workspace)) !== false) { return mode } - return 'guest, but inert: the instance or workspace admits no guest, so this app admits members only' + return 'guest, but inert: this deployment admits no guest, so this app admits members only' } async function readWorkspaceItem( @@ -6040,7 +6037,9 @@ function createVariableToDraftState( // `null` clears and omitted keeps the base's. The draft stores a cleared expiry as // `undefined`, since a stored draft goes through a JSON round-trip. value_valid_until: - args.value_valid_until === null ? undefined : (args.value_valid_until ?? base?.value_valid_until) + args.value_valid_until === null + ? undefined + : (args.value_valid_until ?? base?.value_valid_until) } } diff --git a/frontend/src/lib/components/instanceSettings/GuestActivityList.svelte b/frontend/src/lib/components/instanceSettings/GuestActivityList.svelte index d55c2f9ce6..cde5e340f3 100644 --- a/frontend/src/lib/components/instanceSettings/GuestActivityList.svelte +++ b/frontend/src/lib/components/instanceSettings/GuestActivityList.svelte @@ -3,10 +3,8 @@ import Head from '$lib/components/table/Head.svelte' import Cell from '$lib/components/table/Cell.svelte' import SettingsPageHeader from '$lib/components/settings/SettingsPageHeader.svelte' - import Toggle from '$lib/components/Toggle.svelte' import { Alert } from '$lib/components/common' - import { SettingService, type GuestActivity, type GuestUsage } from '$lib/gen' - import { sendUserToast } from '$lib/toast' + import type { GuestActivity, GuestUsage } from '$lib/gen' interface Props { usage: GuestUsage @@ -14,44 +12,10 @@ hasMore: boolean loading: boolean onLoadMore: () => void - /** The instance switch was written; the caller re-reads usage and says whether - * that read succeeded, so the toggle can show what is actually stored. */ - onInstanceSwitch: () => Promise } - let { usage, guests, hasMore, loading, onLoadMore, onInstanceSwitch }: Props = $props() + let { usage, guests, hasMore, loading, onLoadMore }: Props = $props() const loadMoreSize = 50 - // One write at a time, and the toggle always ends on what is stored: the reloaded - // value when the reload succeeds, else the write's outcome. - let switchPending = $state(false) - let switchOn = $state(usage.instance_enabled) - $effect(() => { - switchOn = usage.instance_enabled - }) - - async function setInstanceSwitch(enabled: boolean) { - switchPending = true - let written = false - try { - await SettingService.setGlobal({ - key: 'guest_access_disabled', - requestBody: { value: !enabled } - }) - written = true - sendUserToast( - enabled - ? 'Guests can sign in again where a workspace allows them' - : 'Guests can no longer sign in anywhere on this instance' - ) - } catch (e) { - sendUserToast(`Could not change the instance guest switch: ${e}`, true) - } - const reloaded = await onInstanceSwitch() - if (!reloaded) { - switchOn = written ? enabled : usage.instance_enabled - } - switchPending = false - } // A capped instance refuses the next stranger as soon as the allowance is used up. let pastAllowance = $derived( usage.metered @@ -68,26 +32,11 @@ {#if !usage.available}
- No guest can sign in here, whatever a workspace or an app says. Guests require a self-hosted - instance or a dedicated Windmill Cloud deployment. + No guest can sign in here, whatever an app says. Guests require a self-hosted instance or a + dedicated Windmill Cloud deployment.
{:else} -
- {#key usage} - setInstanceSwitch(e.detail)} - options={{ - right: 'Allow guests on this instance', - rightTooltip: - 'Off, no guest can sign in anywhere, whatever a workspace or an app says, and sessions already issued stop on their next request.' - }} - /> - {/key} -
-
('pem') @@ -257,8 +255,8 @@ pem: guestJwtKeyType === 'pem' ? guestJwtPublicKey.trim() : '', jwks: guestJwtKeyType === 'jwks' ? guestJwtJwksUrl.trim() : '' }) - // Whether the deployment can have guests at all; off, the card offers no switch to - // turn on. The backend decides; the hostname stands in until it has answered. + // Whether the deployment can have guests at all; off, the card offers no guest key to + // configure. The backend decides; the hostname stands in until it has answered. let guestsAvailable = $derived.by(() => guestUsage?.available ?? !isCloudHosted()) let initialPublicAppRateLimitPerMinute: number | undefined = $state(undefined) @@ -697,11 +695,8 @@ } async function saveDefaultAppSettings(): Promise { - // Guest access and the guest JWT key are the writes of this card available on every plan; - // save them first so a refused Enterprise-only write after cannot swallow them. - if (guestAccessEnabled !== initialGuestAccessEnabled) { - await editGuestAccess() - } + // The guest JWT key is the write of this card available on every plan; save it first so + // a refused Enterprise-only write after cannot swallow it. if ( effectiveGuestJwt.pem !== initialGuestJwtPublicKey || effectiveGuestJwt.jwks !== initialGuestJwtJwksUrl @@ -743,19 +738,6 @@ sendUserToast('Guest JWT key cleared') } - async function editGuestAccess(): Promise { - await WorkspaceService.editGuestAccess({ - workspace: $workspaceStore!, - requestBody: { guest_access_enabled: guestAccessEnabled } - }) - initialGuestAccessEnabled = guestAccessEnabled - sendUserToast( - guestAccessEnabled - ? 'Guests can now open apps set to Guests in this workspace' - : 'Guests can no longer sign in to this workspace' - ) - } - async function loadWorkspaceEncryptionKey(): Promise { let resp = await WorkspaceService.getWorkspaceEncryptionKey({ workspace: $workspaceStore! @@ -841,8 +823,6 @@ initialCriticalAlertUIMuted = settings.mute_critical_alerts publicAppRateLimitPerMinute = settings.public_app_execution_limit_per_minute ?? undefined initialPublicAppRateLimitPerMinute = settings.public_app_execution_limit_per_minute ?? undefined - guestAccessEnabled = settings.guest_access_enabled ?? false - initialGuestAccessEnabled = settings.guest_access_enabled ?? false guestJwtPublicKey = settings.guest_jwt_public_key ?? '' guestJwtJwksUrl = settings.guest_jwt_jwks_url ?? '' initialGuestJwtPublicKey = guestJwtPublicKey @@ -1351,14 +1331,12 @@ savedValue: { defaultAppPath: initialWorkspaceDefaultAppPath, publicAppRateLimitPerMinute: initialPublicAppRateLimitPerMinute, - guestAccessEnabled: initialGuestAccessEnabled, guestJwtPem: initialGuestJwtPublicKey, guestJwtJwks: initialGuestJwtJwksUrl }, modifiedValue: { defaultAppPath: workspaceDefaultAppPath, publicAppRateLimitPerMinute: publicAppRateLimitPerMinute, - guestAccessEnabled: guestAccessEnabled, guestJwtPem: effectiveGuestJwt.pem, guestJwtJwks: effectiveGuestJwt.jwks } @@ -1369,7 +1347,6 @@ function discardDefaultAppSettingsChanges() { workspaceDefaultAppPath = initialWorkspaceDefaultAppPath publicAppRateLimitPerMinute = initialPublicAppRateLimitPerMinute - guestAccessEnabled = initialGuestAccessEnabled guestJwtPublicKey = initialGuestJwtPublicKey guestJwtJwksUrl = initialGuestJwtJwksUrl guestJwtKeyType = initialGuestJwtJwksUrl ? 'jwks' : 'pem' @@ -2018,7 +1995,9 @@ onclick={saveSlackSigningSecret} disabled={!slackSigningSecret.trim() && !slackSigningSecretSet} > - {slackSigningSecret.trim() || !slackSigningSecretSet ? 'Save' : 'Remove'} + {slackSigningSecret.trim() || !slackSigningSecretSet + ? 'Save' + : 'Remove'}
{#if !guestsAvailable} @@ -2723,16 +2702,7 @@ export async function main(
{/if} {:else} - - {#if guestUsage && !guestUsage.instance_enabled} - - A superadmin has turned guests off for this instance, so this switch has no - effect until they are allowed again. - - {:else if guestUsage} + {#if guestUsage} {guestUsage.guest_count} of {guestUsage.free_allowance} free guests used across this instance in the last {guestUsage.window_days} days. @@ -2802,7 +2772,6 @@ export async function main( onDiscard={discardDefaultAppSettingsChanges} saveLabel="Save app settings" disabled={!$enterpriseLicense && - guestAccessEnabled === initialGuestAccessEnabled && effectiveGuestJwt.pem === initialGuestJwtPublicKey && effectiveGuestJwt.jwks === initialGuestJwtJwksUrl} /> diff --git a/system_prompts/auto-generated/prompts.ts b/system_prompts/auto-generated/prompts.ts index 9965d9ae63..9ab3b7ecd6 100644 --- a/system_prompts/auto-generated/prompts.ts +++ b/system_prompts/auto-generated/prompts.ts @@ -1617,7 +1617,7 @@ write a secret you were not given: keep the placeholder. | \`slack_command_script\` / \`teams_command_script\` | Script or flow handling the Slack / Teams command. | | \`slack_oauth_client_id\` / \`slack_oauth_client_secret\` | The workspace's own Slack OAuth app. Clearing either removes both. | | \`public_app_execution_limit_per_minute\` | Rate limit for public apps, \`null\` for none. | -| \`guest_access_enabled\`, \`guest_jwt_public_key\`, \`guest_jwt_jwks_url\` | Guest access to public apps and the key that signs guest JWTs. | +| \`guest_jwt_public_key\`, \`guest_jwt_jwks_url\` | The key that signs guest JWTs for apps set to Guests. | | \`add_admins_and_developers_to_forks\` | Add the workspace's admins and developers to each fork. | | \`color\` | Workspace color, \`#rrggbb\`. | `; diff --git a/system_prompts/auto-generated/skills/workspace-settings/SKILL.md b/system_prompts/auto-generated/skills/workspace-settings/SKILL.md index 73f276c255..f739f95aa7 100644 --- a/system_prompts/auto-generated/skills/workspace-settings/SKILL.md +++ b/system_prompts/auto-generated/skills/workspace-settings/SKILL.md @@ -58,7 +58,7 @@ write a secret you were not given: keep the placeholder. | `slack_command_script` / `teams_command_script` | Script or flow handling the Slack / Teams command. | | `slack_oauth_client_id` / `slack_oauth_client_secret` | The workspace's own Slack OAuth app. Clearing either removes both. | | `public_app_execution_limit_per_minute` | Rate limit for public apps, `null` for none. | -| `guest_access_enabled`, `guest_jwt_public_key`, `guest_jwt_jwks_url` | Guest access to public apps and the key that signs guest JWTs. | +| `guest_jwt_public_key`, `guest_jwt_jwks_url` | The key that signs guest JWTs for apps set to Guests. | | `add_admins_and_developers_to_forks` | Add the workspace's admins and developers to each fork. | | `color` | Workspace color, `#rrggbb`. | @@ -76,8 +76,8 @@ With `includeSettings: true` in `wmill.yaml`, `wmill sync pull` writes the docum `wmill sync push` applies it. Unlike a partial apply, a push treats the file as the whole document: a setting the file omits is cleared, except `operator_settings`, `color`, the auto-invite instance groups, and the keys `ducklake`, `dbt_warehouses`, `deploy_ui`, `teams_command_script`, -`public_app_execution_limit_per_minute`, `guest_access_enabled`, `guest_jwt_public_key`, -`guest_jwt_jwks_url`, `error_handler_fallback_to_instance_alerts` and +`public_app_execution_limit_per_minute`, `guest_jwt_public_key`, `guest_jwt_jwks_url`, +`error_handler_fallback_to_instance_alerts` and `add_admins_and_developers_to_forks`, which are managed only when the file states them. `datatable_permissions` is pushed only with `includeDatatablePermissions: true` (or `--include-datatable-permissions`). A push ignores `protection_rules`: `protection-rules.yaml` and diff --git a/system_prompts/base/workspace-settings.md b/system_prompts/base/workspace-settings.md index 145cd89c66..f5e4c7e7d1 100644 --- a/system_prompts/base/workspace-settings.md +++ b/system_prompts/base/workspace-settings.md @@ -53,7 +53,7 @@ write a secret you were not given: keep the placeholder. | `slack_command_script` / `teams_command_script` | Script or flow handling the Slack / Teams command. | | `slack_oauth_client_id` / `slack_oauth_client_secret` | The workspace's own Slack OAuth app. Clearing either removes both. | | `public_app_execution_limit_per_minute` | Rate limit for public apps, `null` for none. | -| `guest_access_enabled`, `guest_jwt_public_key`, `guest_jwt_jwks_url` | Guest access to public apps and the key that signs guest JWTs. | +| `guest_jwt_public_key`, `guest_jwt_jwks_url` | The key that signs guest JWTs for apps set to Guests. | | `add_admins_and_developers_to_forks` | Add the workspace's admins and developers to each fork. | | `color` | Workspace color, `#rrggbb`. | @@ -72,8 +72,8 @@ With `includeSettings: true` in `wmill.yaml`, `wmill sync pull` writes the docum `wmill sync push` applies it. Unlike a partial apply, a push treats the file as the whole document: a setting the file omits is cleared, except `operator_settings`, `color`, the auto-invite instance groups, and the keys `ducklake`, `dbt_warehouses`, `deploy_ui`, `teams_command_script`, -`public_app_execution_limit_per_minute`, `guest_access_enabled`, `guest_jwt_public_key`, -`guest_jwt_jwks_url`, `error_handler_fallback_to_instance_alerts` and +`public_app_execution_limit_per_minute`, `guest_jwt_public_key`, `guest_jwt_jwks_url`, +`error_handler_fallback_to_instance_alerts` and `add_admins_and_developers_to_forks`, which are managed only when the file states them. `datatable_permissions` is pushed only with `includeDatatablePermissions: true` (or `--include-datatable-permissions`). A push ignores `protection_rules`: `protection-rules.yaml` and From 499c7569ffd49131cb6de9588b8691f062c5d91e Mon Sep 17 00:00:00 2001 From: hugocasa Date: Wed, 28 Oct 2026 13:37:14 +0100 Subject: [PATCH 03/26] fix: honor a step's timeout when it is the only step of a loop (#12058) Co-authored-by: Claude Opus 5.5 --- backend/windmill-worker/src/worker_flow.rs | 27 +++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/backend/windmill-worker/src/worker_flow.rs b/backend/windmill-worker/src/worker_flow.rs index e0c4026804..3356793b84 100644 --- a/backend/windmill-worker/src/worker_flow.rs +++ b/backend/windmill-worker/src/worker_flow.rs @@ -6166,6 +6166,7 @@ pub(super) fn is_simple_modules( && modules[0].suspend.is_none() && modules[0].cache_ttl.is_none() && modules[0].retry.is_none() + && modules[0].timeout.is_none() && modules[0].stop_after_if.is_none() && modules[0].stop_after_all_iters_if.is_none() && modules[0].skip_if.is_none() @@ -6710,8 +6711,32 @@ pub async fn get_previous_job_result( #[cfg(test)] mod tests { - use super::{effective_flow_step_timeout, extract_chat_message_from_flow_result}; + use super::{ + effective_flow_step_timeout, extract_chat_message_from_flow_result, is_simple_modules, + }; use serde_json::{json, value::to_raw_value}; + use windmill_common::flows::FlowModule; + + // The simple-loop fast path reads step settings off the loop module, so a loop body whose + // step has its own timeout must take the full path or that timeout is dropped. + #[test] + fn loop_body_with_step_timeout_is_not_simple() { + let step = |extra: serde_json::Value| -> FlowModule { + let mut m = json!({ + "id": "b", + "value": {"type": "rawscript", "language": "python3", "content": "", "input_transforms": {}} + }); + m.as_object_mut() + .unwrap() + .extend(extra.as_object().unwrap().clone()); + serde_json::from_value(m).unwrap() + }; + assert!(is_simple_modules(&vec![step(json!({}))], None)); + assert!(!is_simple_modules( + &vec![step(json!({"timeout": {"type": "static", "value": 300}}))], + None + )); + } // A `<= 0` step timeout override must behave as "no override" and inherit the referenced // script's timeout, not collapse to a 0-second (instant-kill) timeout. A positive override From 77c0a26515bf4405b2e0918910aa8a84eb1a55ea Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 28 Oct 2026 17:28:41 +0100 Subject: [PATCH 04/26] feat: add ai review to drafts, draft diffs and session review (#12063) * feat: add ai review to the drafts page, draft diffs and the session review Co-Authored-By: Claude Opus 5.5 * fix: drop an ai review when its workspace changes Co-Authored-By: Claude Opus 5.5 * fix: follow the diff version picker in ai review and review only your own draft from the badge Co-Authored-By: Claude Opus 5.5 * fix: give the drafts page ai review the full width Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- cli/src/commands/review/aiReviewCore.gen.ts | 27 ++- .../src/lib/components/CompareDrafts.svelte | 84 ++++++---- .../lib/components/CompareWorkspaces.svelte | 9 +- frontend/src/lib/components/DiffDrawer.svelte | 53 +++++- frontend/src/lib/components/DraftBadge.svelte | 7 +- .../src/lib/components/FlowBuilder.svelte | 5 +- .../src/lib/components/ScriptBuilder.svelte | 5 +- .../apps/editor/AppEditorHeader.svelte | 8 +- .../deploymentRequest/AiReview.svelte | 98 ++++++++--- .../DeploymentRequestPanel.svelte | 9 +- .../deploymentRequest/aiReview.test.ts | 46 +++++- .../components/deploymentRequest/aiReview.ts | 155 +++++++++++++----- .../deploymentRequest/aiReviewCore.ts | 27 ++- .../deploymentRequest/draftReviewItem.ts | 21 +++ frontend/src/lib/components/diff_drawer.ts | 11 ++ .../raw_apps/RawAppEditorHeader.svelte | 8 +- .../sessions/SessionDiffDrawer.svelte | 1 + .../sessions/WorkspaceDiffDrawer.svelte | 29 ++++ 18 files changed, 476 insertions(+), 127 deletions(-) create mode 100644 frontend/src/lib/components/deploymentRequest/draftReviewItem.ts diff --git a/cli/src/commands/review/aiReviewCore.gen.ts b/cli/src/commands/review/aiReviewCore.gen.ts index 34d98da18f..546b82f124 100644 --- a/cli/src/commands/review/aiReviewCore.gen.ts +++ b/cli/src/commands/review/aiReviewCore.gen.ts @@ -74,7 +74,7 @@ export function formatFolderInstructions( } // What the review asks of the model and how its answer is read, for every place it runs: the -// deployment request panel here, and `wmill review` on a git sync pull request, which carries +// deployment request panel and the drafts review here, and `wmill review` on a git sync pull request, which carries // this module through cli/generate-ai-review.ts. Keep it free of `$lib` imports; the // staleness test in cli/test/ai_review_bundle_unit.test.ts fails when the CLI's copy drifts. @@ -109,32 +109,47 @@ export const TOTAL_CHAR_BUDGET = 160_000 // Under the Anthropic SDK's non-streaming pre-flight max_tokens limit. export const MAX_OUTPUT_TOKENS = 8_000 -/** What is being reviewed: a fork's deployment request, or a pull request on a git sync repo. */ -export type ReviewSubject = 'deployment_request' | 'pull_request' +/** + * What is being reviewed: a fork's deployment request, a workspace's undeployed drafts, or a + * pull request on a git sync repo. + */ +export type ReviewSubject = 'deployment_request' | 'draft' | 'pull_request' const INTRO: Record = { deployment_request: 'You review a Windmill deployment request: a set of changes made in a fork workspace that will be merged into its parent workspace. Each changed item is given with its parent version (what is deployed now) and its fork version (what will be deployed).', + draft: + 'You review drafts in a Windmill workspace: changes saved to items of the workspace but not deployed yet. Each changed item is given with its deployed version (what is deployed now) and its draft version (what will be deployed).', pull_request: "You review a pull request on a git repository that Windmill's git sync keeps in step with a workspace: merging it deploys its changes. Each changed item is given with the files that make it up in the repository, each as a unified diff of the base branch (what is deployed now) against the pull request (what will be deployed). Every diff line starts with its line number: R for line n of the pull request's version of the file, L for line n of the base version, then + for an added line, - for a removed one and a space for unchanged context." } +const ITEM_ANSWER_SHAPE = + '{"summary": "", "findings": [{"kind": "", "path": "", "severity": "high" | "medium" | "low", "comment": ""}]}' + const ANSWER_SHAPE: Record = { - deployment_request: - '{"summary": "", "findings": [{"kind": "", "path": "", "severity": "high" | "medium" | "low", "comment": ""}]}', + deployment_request: ITEM_ANSWER_SHAPE, + draft: ITEM_ANSWER_SHAPE, pull_request: '{"summary": "", "findings": [{"kind": "", "path": "", "file": "", "line": "", "severity": "high" | "medium" | "low", "comment": ""}]}' } const SOURCE: Record = { deployment_request: 'The parent workspace', + draft: 'The workspace', pull_request: "The repository's base branch" } +const APPROVED: Record = { + deployment_request: 'the merge', + draft: 'the deployment', + pull_request: 'the merge' +} + export function reviewSystemPrompt(subject: ReviewSubject): string { return `${INTRO[subject]} An item can be a script, flow, app, schedule, trigger, folder, resource type, datatable migration, variable or resource. -Report only issues a reviewer should act on before approving the merge: +Report only issues a reviewer should act on before approving ${APPROVED[subject]}: - bugs and logic errors introduced by the change - breaking changes: removed or renamed inputs, changed input types, changed outputs that callers may rely on - risky operational changes: schedules retimed, triggers pointed at a different script or flow, folder owners or permissions, app policies, destructive migrations diff --git a/frontend/src/lib/components/CompareDrafts.svelte b/frontend/src/lib/components/CompareDrafts.svelte index 2dc0b1fa59..a4e9495ce3 100644 --- a/frontend/src/lib/components/CompareDrafts.svelte +++ b/frontend/src/lib/components/CompareDrafts.svelte @@ -42,6 +42,9 @@ import { userStore, userWorkspaces } from '$lib/stores' import { capabilitiesIn } from '$lib/roleCapabilities' import { childWorkspaceNoun } from '$lib/utils/devWorkspaceLabel' + import { copilotInfo } from '$lib/aiStore' + import AiReview from './deploymentRequest/AiReview.svelte' + import { draftReviewItem } from './deploymentRequest/draftReviewItem' interface Props { currentWorkspaceId: string @@ -456,6 +459,13 @@ selectedItems = [] } + // Your own drafts only: another user's draft is view-only, and reading one returns yours. + const reviewItems = $derived( + visibleItems + .filter((i) => selectedItems.includes(i.key) && i.mine && i.draftKind !== 'data_pipeline') + .map((i) => draftReviewItem(i.draftKind, i.path, currentWorkspaceId, i.draft_only)) + ) + // --- Diff --- let diffDrawer: DiffDrawer | undefined = $state(undefined) let isFlow = $state(false) @@ -480,7 +490,13 @@ mode: 'simple', original: deployed as any, current: draft as any, - title: 'Deployed → Draft' + title: 'Deployed → Draft', + aiReview: { + workspaceId: currentWorkspaceId, + kind: item.draftKind, + path: item.path, + added: item.draft_only + } }) } @@ -912,35 +928,45 @@ {/snippet} {#snippet footer()} -
-
- - +
+
+
+ + +
+ {#if !deployPerm.ok} + {deployPerm.reason} + {:else if undeployableSelectedCount > 0} + + {undeployableSelectedCount} selected draft{undeployableSelectedCount !== 1 + ? 's' + : ''} + can't be deployed (no write permission on the path) but can still be discarded + + {/if}
- {#if !deployPerm.ok} - {deployPerm.reason} - {:else if undeployableSelectedCount > 0} - - {undeployableSelectedCount} selected draft{undeployableSelectedCount !== 1 ? 's' : ''} - can't be deployed (no write permission on the path) but can still be discarded - + {#if $copilotInfo.enabled} + {/if}
{/snippet} diff --git a/frontend/src/lib/components/CompareWorkspaces.svelte b/frontend/src/lib/components/CompareWorkspaces.svelte index 8868813958..06b691679c 100644 --- a/frontend/src/lib/components/CompareWorkspaces.svelte +++ b/frontend/src/lib/components/CompareWorkspaces.svelte @@ -1854,9 +1854,12 @@ {#if mergeIntoParent && $copilotInfo.enabled && !hasOpenDeploymentRequest && !deploymentRequestPanel?.isDialogOpen()} {/if} diff --git a/frontend/src/lib/components/DiffDrawer.svelte b/frontend/src/lib/components/DiffDrawer.svelte index 2e5810e036..b7b5aa8922 100644 --- a/frontend/src/lib/components/DiffDrawer.svelte +++ b/frontend/src/lib/components/DiffDrawer.svelte @@ -14,7 +14,10 @@ import { sendUserToast } from '$lib/toast' import type { Script } from '$lib/gen' import Select from './select/Select.svelte' - import type { DiffVersionOption } from './diff_drawer' + import type { DiffAiReview, DiffVersionOption } from './diff_drawer' + import { copilotInfo } from '$lib/aiStore' + import AiReview from './deploymentRequest/AiReview.svelte' + import type { ReviewScope } from './deploymentRequest/aiReview' type DiffData = { lang?: string @@ -36,6 +39,25 @@ let diffViewer: Drawer | undefined = $state(undefined) + let aiReviewScope: ReviewScope | undefined = $state(undefined) + /** Normal mode: what the review is rebuilt from when the version picker swaps the left side. */ + let aiReviewTarget: { review: DiffAiReview; current: Value } | undefined = undefined + + function reviewScope(review: DiffAiReview, before: Value, after: Value): ReviewScope { + return { + kind: 'drafts', + workspaceId: review.workspaceId, + items: [ + { + kind: review.kind, + path: review.path, + change: review.added ? 'added' : 'modified', + load: async () => ({ before, after }) + } + ] + } + } + interface Props { restoreDeployed?: () => Promise isFlow?: boolean @@ -77,6 +99,8 @@ if (token == null) openingToken++ data = undefined diffType = undefined + aiReviewScope = undefined + aiReviewTarget = undefined diffViewer?.openDrawer() } @@ -111,6 +135,8 @@ loadingVersion = false data = undefined diffType = undefined + aiReviewScope = undefined + aiReviewTarget = undefined diffViewer?.closeDrawer() } @@ -209,6 +235,11 @@ deployed: prepareDiff(value), deployedLabel: opt?.isHead ? headLabel : opt?.label } + // The review covers the version on screen; a new scope also drops a result for the + // previous one. + if (aiReviewTarget) { + aiReviewScope = reviewScope(aiReviewTarget.review, value, aiReviewTarget.current) + } } catch (e: any) { // The snap-back would otherwise be the only sign the version never loaded. if (generation === versionLoadGeneration) { @@ -270,6 +301,7 @@ current: Value defaultDiffType?: 'deployed' | 'draft' button?: { text: string; onClick: () => void } + aiReview?: DiffAiReview } | { mode: 'simple' @@ -277,6 +309,7 @@ current: Value title: string button?: { text: string; onClick: () => void } + aiReview?: DiffAiReview }, token?: number ) { @@ -284,6 +317,8 @@ // caller with none is taking the drawer, so it claims one. if (token != null && token !== openingToken) return if (token == null) openingToken++ + aiReviewScope = undefined + aiReviewTarget = undefined if (diff.mode === 'normal') { const { deployed, @@ -326,6 +361,13 @@ // The draft-vs-current view (and its tab) is obsolete — always show the // deployed-vs-current diff. diffType = 'deployed' + if (diff.aiReview) { + aiReviewTarget = { + review: { ...diff.aiReview, added: diff.aiReview.added || !!deployed.draft_only }, + current + } + aiReviewScope = reviewScope(aiReviewTarget.review, deployed, current) + } } else { const { original, current, title, button } = diff data = { @@ -336,6 +378,7 @@ button } diffType = 'custom' + if (diff.aiReview) aiReviewScope = reviewScope(diff.aiReview, original, current) } } @@ -343,6 +386,14 @@
+ {#if data && aiReviewScope && $copilotInfo.enabled} +
+ + {#key aiReviewScope} + + {/key} +
+ {/if} {#if data} + + {#snippet trigger()} + {error !== undefined && 
{error}
} - {result !== undefined && 
{JSON.stringify(result, null, 2)}
} + {result !== undefined && 
{${outputOnly ? `typeof result === 'string' ? result : ` : ''}JSON.stringify(result, null, 2)}
} ) } @@ -651,7 +656,7 @@ function createRawApp(opts: { path: string summary: string | undefined schema: Record | undefined - runType: 'script' | 'flow' + runType: 'script' | 'flow' | 'agent' }): RawAppImport { const { path, summary, schema, runType } = opts const runnableId = runnableIdFromPath(path) @@ -677,6 +682,7 @@ function createRawApp(opts: { return { summary: title, + openInAiSession: true, value: { files: { ...react19Template, @@ -684,7 +690,8 @@ function createRawApp(opts: { runnableId, fields, title, - subtitle: `${runType} · ${path}` + subtitle: `${runType} · ${path}`, + outputOnly: runType === 'agent' }), '/index.css': indexCss }, @@ -718,3 +725,448 @@ export function createRawAppFromFlow( ): RawAppImport { return createRawApp({ path, summary, schema, runType: 'flow' }) } + +/** For an agent without managed memory, which answers each message on its own: a form over the + * message. Attachments are left out, a raw app having no upload for them. */ +export function createRawAppFromAgent(path: string): RawAppImport { + const { user_message } = AGENT_CHAT_SCHEMA.properties + return createRawApp({ + path, + summary: undefined, + schema: { ...AGENT_CHAT_SCHEMA, properties: { user_message } }, + runType: 'agent' + }) +} + +const chatAppTsx = (chatName: string) => `import React, { useEffect, useRef, useState } from 'react' +import { useWindmillChat } from '@windmill-labs/chat-sdk/react' +import * as wmill from './wmill' +import './index.css' + +const CHAT_NAME = ${JSON.stringify(chatName)} +document.title = CHAT_NAME + +export default function App() { + // Each turn runs through the \`chat\` runnable, as this app. Conversations are + // kept in the viewer's browser. + const chat = useWindmillChat({ wmill, runnable: 'chat' }) + const [input, setInput] = useState('') + const [showList, setShowList] = useState(false) + const end = useRef(null) + const busy = chat.status === 'submitted' || chat.status === 'streaming' + const last = chat.messages[chat.messages.length - 1] + + useEffect(() => { + end.current?.scrollIntoView({ block: 'end' }) + }, [chat.messages]) + + function send() { + const text = input.trim() + if (!text || busy) return + setInput('') + // A message refused before it starts leaves the transcript as it was: give it back. + chat.sendMessage(text).catch(() => setInput(text)) + } + + function open(id?: string) { + if (id) chat.selectConversation(id) + else chat.newConversation() + setShowList(false) + } + + return ( +
+ + {showList &&
setShowList(false)} />} + +
+
+ +

{CHAT_NAME}

+
+ +
+
+ {chat.messages.length === 0 && ( +

Ask {CHAT_NAME} anything.

+ )} + {chat.messages.map((m) => + m.tool ? ( +
+ {m.tool.name} + {m.tool.status === 'running' ? '…' : m.tool.status === 'error' ? ' failed' : ''} +
+ ) : m.role === 'user' || m.role === 'assistant' ? ( +
+ {m.content || (m.pending && Thinking…)} +
+ ) : null + )} + {busy && last?.role === 'user' && ( +
+ Thinking… +
+ )} +
+
+
+ +
+ {chat.status === 'error' && chat.error && ( +
{chat.error.message}
+ )} +
{ + e.preventDefault() + send() + }} + > +