From be186e6fdd006dc2d102d2bd1bf2c84af663a4d1 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Tue, 1 Sep 2026 23:10:53 +0200 Subject: [PATCH] fix(datatables): the objects a revoke names do not answer for its scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ownership guard took the named-objects branch whatever the scope, and returned. A revoke naming one table the caller owns, scoped to all tables, therefore passed a check over that one table and planned a statement that names the whole schema — the objects never reach the SQL there. --- backend/ee-repo-ref.txt | 2 +- .../src/datatable_acl.rs | 25 +++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index fef2825da4..3d406e6fbe 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -9a61895fa24f1e5a13ee174353ccbe2ad76a24f5 +7cb71003d15a2f459596681dbd5d6392dd603a7a diff --git a/backend/windmill-api-workspaces/src/datatable_acl.rs b/backend/windmill-api-workspaces/src/datatable_acl.rs index 0e69561882..62ef3b65df 100644 --- a/backend/windmill-api-workspaces/src/datatable_acl.rs +++ b/backend/windmill-api-workspaces/src/datatable_acl.rs @@ -338,8 +338,10 @@ async fn first_unmanageable_object( return Ok(None); }; - // A revoke that names its objects reaches exactly those. - if let AclChange::Revoke { objects, .. } = change { + // A revoke that names its objects reaches exactly those — but only at + // `Target` scope, which is the only one the planner writes them into. Any + // other scope names the schema, so it is the classes below that answer. + if let AclChange::Revoke { objects, scope: GrantScope::Target, .. } = change { if !objects.is_empty() { let named: Vec = objects .iter() @@ -1129,5 +1131,24 @@ mod tests { reached_object_classes(&table(), &AclChange::SetOwner { role: "analyst".to_string() }), None ); + + // A revoke naming objects still answers for its scope: the planner + // writes those objects only at `Target`, so any other scope reaches the + // schema whatever the request listed. + let revoke = |scope| AclChange::Revoke { + role: "analyst".to_string(), + privileges: vec!["SELECT".to_string()], + scope, + objects: vec![AclObject { + name: "mine".to_string(), + kind: "TABLE".to_string(), + args: None, + }], + }; + assert_eq!( + reached_object_classes(&schema(), &revoke(GrantScope::AllTables)), + Some((["r", "p", "v", "m", "f"].as_slice(), false)) + ); + assert_eq!(reached_object_classes(&schema(), &revoke(GrantScope::Target)), None); } }