fix: let admins share any connection, serialize service removal with trigger creation

An admin can already read every connection, so restricting sharing to their own
added nothing. Removing a service now takes each connection's lock and keeps
everything if a trigger was created meanwhile.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

.agents/skills/native-trigger/SKILL.md

@@ -131,7 +131,7 @@ accounts) are many per service. Each trigger acts through one connection, named
 A user may use a connection when they can read its variable (checked with a user transaction,
 so the variable's row-level security decides): `resolve_usable_connection` /
 `list_usable_connections` in `lib.rs`. The exception is the **workspace connection**: an admin
-may share one connection they made per service (`workspace_integrations.resource_path`, set by
+may share one connection per service (`workspace_integrations.resource_path`, set by
 `POST .../workspace_connection`), and every member may then use it without reading its token.
 Renames carry the marker along; disconnecting clears it. Background work (sync, renewal, rename re-registration,
 webhook handling) uses the stored `connection_path` directly.

backend/.sqlx/query-1966a1c4776492599de66d6d189d399c37957f832485156f703f59bd39cdc2bb.json

@@ -1,6 +1,6 @@
 {
   "db_name": "PostgreSQL",
-  "query": "SELECT EXISTS(SELECT 1 FROM variable v\n             JOIN account a ON a.workspace_id = v.workspace_id AND a.id = v.account\n             JOIN resource r ON r.workspace_id = v.workspace_id AND r.path = v.path\n             WHERE v.workspace_id = $1 AND v.path = $2 AND a.client = $3\n               AND a.is_workspace_integration = true AND r.created_by = $4)",
+  "query": "SELECT EXISTS(SELECT 1 FROM variable v\n             JOIN account a ON a.workspace_id = v.workspace_id AND a.id = v.account\n             WHERE v.workspace_id = $1 AND v.path = $2 AND a.client = $3\n               AND a.is_workspace_integration = true)",
   "describe": {
     "columns": [
       {
@@ -11,7 +11,6 @@
     ],
     "parameters": {
       "Left": [
-        "Text",
         "Text",
         "Text",
         "Text"
@@ -21,5 +20,5 @@
       null
     ]
   },
-  "hash": "72c123eb6a0d66f4e57d86694581772154b09f4a89903462875e57c9760a19e8"
+  "hash": "1966a1c4776492599de66d6d189d399c37957f832485156f703f59bd39cdc2bb"
 }

backend/.sqlx/query-78954c7a6d38ae9d094f77bb255ae235c9b3083b4c179f819c927342bc58651f.json

@@ -0,0 +1,34 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "SELECT EXISTS(SELECT 1 FROM native_trigger WHERE workspace_id = $1 AND service_name = $2)",
+  "describe": {
+    "columns": [
+      {
+        "ordinal": 0,
+        "name": "exists",
+        "type_info": "Bool"
+      }
+    ],
+    "parameters": {
+      "Left": [
+        "Text",
+        {
+          "Custom": {
+            "name": "native_trigger_service",
+            "kind": {
+              "Enum": [
+                "nextcloud",
+                "google",
+                "github"
+              ]
+            }
+          }
+        }
+      ]
+    },
+    "nullable": [
+      null
+    ]
+  },
+  "hash": "78954c7a6d38ae9d094f77bb255ae235c9b3083b4c179f819c927342bc58651f"
+}

backend/.sqlx/query-d79ba22a53c9f96c96fcd4b4cf0e6cc17f767055de943709192ef1ff5106eac5.json

@@ -0,0 +1,23 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "SELECT v.path FROM variable v\n         JOIN account a ON a.workspace_id = v.workspace_id AND a.id = v.account\n         WHERE v.workspace_id = $1 AND a.client = $2 AND a.is_workspace_integration = true",
+  "describe": {
+    "columns": [
+      {
+        "ordinal": 0,
+        "name": "path",
+        "type_info": "Varchar"
+      }
+    ],
+    "parameters": {
+      "Left": [
+        "Text",
+        "Text"
+      ]
+    },
+    "nullable": [
+      false
+    ]
+  },
+  "hash": "d79ba22a53c9f96c96fcd4b4cf0e6cc17f767055de943709192ef1ff5106eac5"
+}

backend/windmill-native-triggers/src/lib.rs

@@ -1189,6 +1189,8 @@ pub async fn delete_token_by_hash<'c, E: sqlx::Executor<'c, Database = Postgres>
     Ok(deleted > 0)
 }
 
+/// Callers must hold [`lock_connection`] on `connection_path` in the same transaction and check
+/// the connection still exists, or a concurrent disconnect can leave this trigger stranded.
 pub async fn store_native_trigger<'c, E: sqlx::Executor<'c, Database = Postgres>, C: Serialize>(
     db: E,
     workspace_id: &str,

backend/windmill-native-triggers/src/workspace_integrations.rs

@@ -475,8 +475,40 @@ async fn delete_integration(
 
     let mut tx = user_db.begin(&authed).await?;
 
+    // Same serialization as a single disconnect: a trigger created meanwhile either committed
+    // before these locks, and is caught below, or waits and then finds its connection gone.
+    let mut paths = sqlx::query_scalar!(
+        "SELECT v.path FROM variable v
+         JOIN account a ON a.workspace_id = v.workspace_id AND a.id = v.account
+         WHERE v.workspace_id = $1 AND a.client = $2 AND a.is_workspace_integration = true",
+        workspace_id,
+        service_name.as_str(),
+    )
+    .fetch_all(&db)
+    .await?;
+    paths.sort();
+    for path in &paths {
+        lock_connection(&mut tx, &workspace_id, path).await?;
+    }
+
     cleanup_service_connections(&mut *tx, &workspace_id, service_name).await?;
 
+    let created_meanwhile = sqlx::query_scalar!(
+        "SELECT EXISTS(SELECT 1 FROM native_trigger WHERE workspace_id = $1 AND service_name = $2)",
+        workspace_id,
+        service_name as ServiceName,
+    )
+    .fetch_one(&mut *tx)
+    .await?
+    .unwrap_or(false);
+    if created_meanwhile {
+        tx.rollback().await?;
+        return Err(Error::BadRequest(format!(
+            "A {service_name} trigger was created while removing the integration, so it was kept. \
+             Remove it again."
+        )));
+    }
+
     let deleted = delete_workspace_integration(&mut *tx, &workspace_id, service_name).await?;
 
     if !deleted {
@@ -1088,26 +1120,22 @@ async fn set_workspace_connection(
 ) -> JsonResult<String> {
     require_admin(authed.is_admin, &workspace_id)?;
 
-    // Sharing lets everyone act as the account behind it, so only the one who connected it may.
     if let Some(path) = path.as_deref() {
-        let own = sqlx::query_scalar!(
+        let exists = sqlx::query_scalar!(
             "SELECT EXISTS(SELECT 1 FROM variable v
              JOIN account a ON a.workspace_id = v.workspace_id AND a.id = v.account
-             JOIN resource r ON r.workspace_id = v.workspace_id AND r.path = v.path
              WHERE v.workspace_id = $1 AND v.path = $2 AND a.client = $3
-               AND a.is_workspace_integration = true AND r.created_by = $4)",
+               AND a.is_workspace_integration = true)",
             workspace_id,
             path,
             service_name.as_str(),
-            authed.username,
         )
         .fetch_one(&db)
         .await?
         .unwrap_or(false);
-        if !own {
+        if !exists {
             return Err(Error::BadRequest(format!(
-                "No {service_name} connection you made at {path}. You can only share an account \
-                 you connected yourself."
+                "No {service_name} connection at {path}"
             )));
         }
     }
@@ -1213,9 +1241,10 @@ async fn delete_connection(
     let account = cleanup_connection(&mut *tx, &workspace_id, &path).await?;
 
     // A trigger moved or created meanwhile was skipped above and still acts through this
-    // connection, so removing the connection would strand it. Checked after the delete: a create
-    // holds the connection's variable until it commits, so the delete waits for it and this
-    // check then sees its trigger.
+    // connection, so removing the connection would strand it. `lock_connection` is what makes
+    // this check sound: a create stores its trigger under the same lock, so it either committed
+    // before this transaction took the lock, and is seen here, or it waits and finds the
+    // connection gone.
     let still_used = sqlx::query_scalar!(
         "SELECT EXISTS(SELECT 1 FROM native_trigger
          WHERE workspace_id = $1 AND service_name = $2 AND connection_path = $3)",

frontend/src/lib/components/triggers/native/NativeConnectionPicker.svelte

@@ -18,8 +18,6 @@
 		/** An existing trigger keeps the connection its webhook was registered under. */
 		locked?: boolean
 		disabled?: boolean
-		/** Only offer the caller's own connections. */
-		ownOnly?: boolean
 		/** Select the first connection when none is chosen yet. */
 		autoSelect?: boolean
 		clearable?: boolean
@@ -35,7 +33,6 @@
 		value = $bindable(),
 		locked = false,
 		disabled = false,
-		ownOnly = false,
 		autoSelect = true,
 		clearable = false,
 		placeholder,
@@ -60,8 +57,8 @@
 			connections = []
 			sendUserToast(`Failed to load ${displayName} connections: ${err.body ?? err.message}`, true)
 		}
-		if (autoSelect && !locked && value === undefined && offered.length > 0) {
-			select(offered[0].path)
+		if (autoSelect && !locked && value === undefined && connections.length > 0) {
+			select(connections[0].path)
 		}
 	}
 
@@ -119,12 +116,9 @@
 		return !c.workspace && !!c.owner && c.owner === $userStore?.username
 	}
 
-	const offered = $derived(
-		(connections ?? []).filter((c) => !ownOnly || c.owner === $userStore?.username)
-	)
 	const items = $derived.by(() => {
-		const list = offered.map((c) => ({
-			label: c.workspace && !ownOnly ? `${c.path} (workspace connection)` : c.path,
+		const list = (connections ?? []).map((c) => ({
+			label: c.workspace ? `${c.path} (workspace connection)` : c.path,
 			value: c.path
 		}))
 		if (value && !list.some((i) => i.value === value)) {
@@ -133,7 +127,7 @@
 		return list
 	})
 	const unusable = $derived(
-		!ownOnly && connections !== undefined && !!value && !connections.some((c) => c.path === value)
+		connections !== undefined && !!value && !connections.some((c) => c.path === value)
 	)
 </script>
 

frontend/src/lib/components/workspaceSettings/WorkspaceIntegrations.svelte

@@ -158,10 +158,12 @@
 
 		const displayName = supportedServices[serviceName]?.displayName ?? serviceName
 		if (path) {
+			const owner = connections[serviceName]?.find((c) => c.path === path)?.owner
+			const account = owner ? `this account, connected by ${owner}` : 'this account'
 			const confirmed = await confirmationModal.ask({
 				title: `Share ${path} with the workspace?`,
 				confirmationText: 'Share',
-				children: `Every member will be able to create and edit ${displayName} triggers that act as this account. They will not see its token. It replaces any connection shared before.`
+				children: `Every member will be able to create and edit ${displayName} triggers that act as ${account}. They will not see its token. It replaces any connection shared before.`
 			})
 			if (!confirmed) return
 		} else {
@@ -356,7 +358,6 @@
 							<NativeConnectionPicker
 								service={serviceName as NativeServiceName}
 								value={workspaceConnection}
-								ownOnly
 								autoSelect={false}
 								clearable
 								placeholder="No workspace connection"