fix: keep a folded grant row revocable only if every object is

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DsU2Lf6wYQJ9o8ASKRgCmK

backend/windmill-api-workspaces/src/datatable_acl.rs

@@ -1531,15 +1531,64 @@ mod tests {
         }
     }
 
-    /// A kind of object the list misses stays with its old owner while the schema changes hands,
-    /// which only a real catalog shows.
-    #[sqlx::test(migrations = false)]
-    async fn a_schemas_owner_change_takes_every_object_in_it(pool: sqlx::PgPool) {
+    /// A connection to the test's own database, the way the handlers reach a data table's.
+    async fn catalog_client(pool: &sqlx::PgPool) -> tokio_postgres::Client {
         let mut config: tokio_postgres::Config =
             std::env::var("DATABASE_URL").unwrap().parse().unwrap();
         config.dbname(pool.connect_options().get_database().unwrap());
         let (client, connection) = config.connect(tokio_postgres::NoTls).await.unwrap();
         tokio::spawn(connection);
+        client
+    }
+
+    /// What a revoke takes back is read from the catalog, per object and source, and only the
+    /// privileges it asks for: the planner renders exactly this, and refuses an empty read.
+    #[sqlx::test(migrations = false)]
+    async fn a_revoke_reads_back_only_what_it_asks_for(pool: sqlx::PgPool) {
+        let client = catalog_client(&pool).await;
+        // A predefined role, so that nothing is granted outside the test's own database.
+        client
+            .batch_execute(
+                "CREATE SCHEMA granted;
+                 CREATE TABLE granted.g (id int);
+                 GRANT SELECT, INSERT ON granted.g TO pg_read_all_data;",
+            )
+            .await
+            .unwrap();
+        let target = AclTarget::Table { schema: "granted".to_string(), table: "g".to_string() };
+        let revoked = read_revoked_grants(
+            &client,
+            "db",
+            &target,
+            GrantScope::Target,
+            &[],
+            &["select".to_string()],
+            "pg_read_all_data",
+        )
+        .await
+        .unwrap();
+        assert_eq!(revoked.len(), 1, "{revoked:?}");
+        assert_eq!(revoked[0].object, None);
+        assert_eq!(revoked[0].privileges, ["SELECT"]);
+        let held_none = read_revoked_grants(
+            &client,
+            "db",
+            &target,
+            GrantScope::Target,
+            &[],
+            &["update".to_string()],
+            "pg_read_all_data",
+        )
+        .await
+        .unwrap();
+        assert!(held_none.is_empty(), "{held_none:?}");
+    }
+
+    /// A kind of object the list misses stays with its old owner while the schema changes hands,
+    /// which only a real catalog shows.
+    #[sqlx::test(migrations = false)]
+    async fn a_schemas_owner_change_takes_every_object_in_it(pool: sqlx::PgPool) {
+        let client = catalog_client(&pool).await;
         client
             .batch_execute(
                 "CREATE SCHEMA moved;

frontend/src/lib/components/datatableAcl/aclScopes.test.ts

@@ -121,6 +121,30 @@ describe('revoke of a row', () => {
 		expect(revokeScopeOf({ ...row(), objects: [{ name: 'mood', kind: 'TYPE' }] })).toBeUndefined()
 	})
 
+	// Whether a grant can be taken back depends on its object, so a row folding several objects is
+	// only revocable if each of its grants is.
+	it('offers none for a folded row with a source out of reach on any of its objects', () => {
+		const grants: AclGrant[] = [
+			{
+				grantee: 'analytics',
+				privileges: ['SELECT'],
+				object: table('orders'),
+				sources: from('admin')
+			},
+			{
+				grantee: 'analytics',
+				privileges: ['SELECT'],
+				object: table('salaries'),
+				sources: [{ role: 'admin', reachable: false }]
+			}
+		]
+		const [folded] = groupGrants(grants)
+		expect(folded.objects).toHaveLength(2)
+		expect(revokeScopeOf(folded)).toBeUndefined()
+		// Folding reads the grants, never rewrites them.
+		expect(grants[0].sources[0].reachable).toBe(true)
+	})
+
 	// Postgres takes a grant back only through its source: offering the revoke would promise what
 	// the plan then refuses.
 	it('offers none for a row with a source out of reach', () => {

frontend/src/lib/components/datatableAcl/aclScopes.ts

@@ -121,9 +121,11 @@ export function groupGrants(grants: AclGrant[]): GroupedGrant[] {
 		if (existing) {
 			existing.objects.push(grant.object!)
 			for (const source of grant.sources) {
-				if (!existing.sources.some((s) => s.role === source.role)) {
-					existing.sources.push(source)
-				}
+				const known = existing.sources.find((s) => s.role === source.role)
+				// Whether a role's grant can be taken back depends on the object it is on, so a row
+				// holds a source as reachable only if it is on every object the row folds.
+				if (known) known.reachable &&= source.reachable
+				else existing.sources.push({ ...source })
 			}
 		} else {
 			rows.push({
@@ -131,7 +133,7 @@ export function groupGrants(grants: AclGrant[]): GroupedGrant[] {
 				privileges: grant.privileges,
 				objects: grant.object ? [grant.object] : [],
 				future: grant.future,
-				sources: [...grant.sources]
+				sources: grant.sources.map((s) => ({ ...s }))
 			})
 		}
 	}