From a7e398981c7df992d327e9e52d128a1e03a0d3a8 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 7 Jan 2026 11:03:52 +0100 Subject: [PATCH 1/5] Improve folder permissions warning to be more actionable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update the warning message shown when workspace folder has permissions not configured in the bundle. The new message provides clear guidance on how to resolve the issue. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- bundle/config/validate/folder_permissions_test.go | 10 +++++++--- bundle/permissions/workspace_path_permissions.go | 9 +++++++-- .../permissions/workspace_path_permissions_test.go | 14 ++++++++++---- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/bundle/config/validate/folder_permissions_test.go b/bundle/config/validate/folder_permissions_test.go index 40906ce6f7..e41d731b3e 100644 --- a/bundle/config/validate/folder_permissions_test.go +++ b/bundle/config/validate/folder_permissions_test.go @@ -118,9 +118,13 @@ func TestValidateFolderPermissionsFailsOnMissingBundlePermission(t *testing.T) { b.SetWorkpaceClient(m.WorkspaceClient) diags := ValidateFolderPermissions().Apply(context.Background(), b) require.Len(t, diags, 1) - require.Equal(t, "untracked permissions apply to target workspace path", diags[0].Summary) + require.Equal(t, "workspace folder has permissions not configured in bundle", diags[0].Summary) require.Equal(t, diag.Warning, diags[0].Severity) - require.Equal(t, "The following permissions apply to the workspace folder at \"/Workspace/Users/foo@bar.com\" but are not configured in the bundle:\n- level: CAN_MANAGE, user_name: foo2@bar.com\n", diags[0].Detail) + expectedDetail := "The following permissions apply to the workspace folder at \"/Workspace/Users/foo@bar.com\" " + + "but are not configured in the bundle:\n- level: CAN_MANAGE, user_name: foo2@bar.com\n\n" + + "Add them to your bundle permissions or remove them from the folder.\n" + + "See https://docs.databricks.com/dev-tools/bundles/permissions" + require.Equal(t, expectedDetail, diags[0].Detail) } func TestValidateFolderPermissionsFailsOnPermissionMismatch(t *testing.T) { @@ -162,7 +166,7 @@ func TestValidateFolderPermissionsFailsOnPermissionMismatch(t *testing.T) { b.SetWorkpaceClient(m.WorkspaceClient) diags := ValidateFolderPermissions().Apply(context.Background(), b) require.Len(t, diags, 1) - require.Equal(t, "untracked permissions apply to target workspace path", diags[0].Summary) + require.Equal(t, "workspace folder has permissions not configured in bundle", diags[0].Summary) require.Equal(t, diag.Warning, diags[0].Severity) } diff --git a/bundle/permissions/workspace_path_permissions.go b/bundle/permissions/workspace_path_permissions.go index e6ef16540c..9e15a3658b 100644 --- a/bundle/permissions/workspace_path_permissions.go +++ b/bundle/permissions/workspace_path_permissions.go @@ -43,8 +43,13 @@ func (p WorkspacePathPermissions) Compare(perms []resources.Permission) diag.Dia if !ok { diags = diags.Append(diag.Diagnostic{ Severity: diag.Warning, - Summary: "untracked permissions apply to target workspace path", - Detail: fmt.Sprintf("The following permissions apply to the workspace folder at %q but are not configured in the bundle:\n%s", p.Path, toString(missing)), + Summary: "workspace folder has permissions not configured in bundle", + Detail: fmt.Sprintf( + "The following permissions apply to the workspace folder at %q "+ + "but are not configured in the bundle:\n%s\n"+ + "Add them to your bundle permissions or remove them from the folder.\n"+ + "See https://docs.databricks.com/dev-tools/bundles/permissions", + p.Path, toString(missing)), }) } diff --git a/bundle/permissions/workspace_path_permissions_test.go b/bundle/permissions/workspace_path_permissions_test.go index eaefad906c..73beba62c3 100644 --- a/bundle/permissions/workspace_path_permissions_test.go +++ b/bundle/permissions/workspace_path_permissions_test.go @@ -85,8 +85,11 @@ func TestWorkspacePathPermissionsCompare(t *testing.T) { expected: diag.Diagnostics{ { Severity: diag.Warning, - Summary: "untracked permissions apply to target workspace path", - Detail: "The following permissions apply to the workspace folder at \"path\" but are not configured in the bundle:\n- level: CAN_MANAGE, group_name: foo\n", + Summary: "workspace folder has permissions not configured in bundle", + Detail: "The following permissions apply to the workspace folder at \"path\" " + + "but are not configured in the bundle:\n- level: CAN_MANAGE, group_name: foo\n\n" + + "Add them to your bundle permissions or remove them from the folder.\n" + + "See https://docs.databricks.com/dev-tools/bundles/permissions", }, }, }, @@ -105,8 +108,11 @@ func TestWorkspacePathPermissionsCompare(t *testing.T) { expected: diag.Diagnostics{ { Severity: diag.Warning, - Summary: "untracked permissions apply to target workspace path", - Detail: "The following permissions apply to the workspace folder at \"path\" but are not configured in the bundle:\n- level: CAN_MANAGE, user_name: foo2@bar.com\n", + Summary: "workspace folder has permissions not configured in bundle", + Detail: "The following permissions apply to the workspace folder at \"path\" " + + "but are not configured in the bundle:\n- level: CAN_MANAGE, user_name: foo2@bar.com\n\n" + + "Add them to your bundle permissions or remove them from the folder.\n" + + "See https://docs.databricks.com/dev-tools/bundles/permissions", }, }, }, From dab074dc4b59e05234c25081cc04ed30cbde897e Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 7 Jan 2026 14:57:16 +0100 Subject: [PATCH 2/5] Fix false positive warnings for folder permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes several issues with the folder permissions validation: 1. Permission hierarchy: Bundle CAN_MANAGE now properly supersedes workspace CAN_VIEW for the same principal (no false warning) 2. Deduplication: When the API returns both inherited and explicit permissions for the same principal, keep only the highest level 3. Current user groups: Suppress warnings for groups the current user belongs to when the user is already tracked in the bundle Also moves PermissionOrder and helpers to resources package for reuse. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../resourcemutator/fix_permissions.go | 59 +----------- .../resourcemutator/fix_permissions_test.go | 29 +++--- bundle/config/resources/permission.go | 62 +++++++++++- bundle/config/validate/folder_permissions.go | 2 +- .../permissions/workspace_path_permissions.go | 67 +++++++++++-- .../workspace_path_permissions_test.go | 94 ++++++++++++++++++- 6 files changed, 233 insertions(+), 80 deletions(-) diff --git a/bundle/config/mutator/resourcemutator/fix_permissions.go b/bundle/config/mutator/resourcemutator/fix_permissions.go index a15e905fe9..7f685015e2 100644 --- a/bundle/config/mutator/resourcemutator/fix_permissions.go +++ b/bundle/config/mutator/resourcemutator/fix_permissions.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/iamutil" @@ -170,7 +171,7 @@ func useMaximumLevel(permissions dyn.Value, resourceType string) (dyn.Value, err principalIndex[principal] = ind principals = append(principals, principal) } - levelPerPrincipal[principal] = getMaxLevel(levelPerPrincipal[principal], level) + levelPerPrincipal[principal] = resources.GetMaxLevel(levelPerPrincipal[principal], level) } var newPermissions []dyn.Value @@ -182,62 +183,6 @@ func useMaximumLevel(permissions dyn.Value, resourceType string) (dyn.Value, err return dyn.V(newPermissions), nil } -// Unified permission order map -// Based on https://docs.databricks.com/aws/en/security/auth/access-control -var PermissionOrder = map[string]int{ - "": -1, - "CAN_VIEW": 2, - "CAN_READ": 3, - "CAN_VIEW_METADATA": 4, - "CAN_RUN": 5, - "CAN_QUERY": 6, - "CAN_USE": 7, - "CAN_EDIT": 8, - "CAN_EDIT_METADATA": 9, - "CAN_CREATE": 10, - "CAN_ATTACH_TO": 11, - "CAN_RESTART": 12, - "CAN_MONITOR": 13, - "CAN_MANAGE_RUN": 14, - "CAN_MANAGE_STAGING_VERSIONS": 15, - "CAN_MANAGE_PRODUCTION_VERSIONS": 16, - "CAN_MANAGE": 17, - "IS_OWNER": 18, - // One known exception from this order: for SQL Warehouses, CAN_USE and CAN_RUN cannot be ordered and must be upgraded to CAN_MONITOR. - // We're not doing that currently. -} - -func getLevelScore(a string) int { - score, ok := PermissionOrder[a] - if ok { - return score - } - maxPrefixLength := 0 - for levelName, levelScore := range PermissionOrder { - if strings.HasPrefix(a, levelName) && len(levelName) > maxPrefixLength { - score = levelScore - maxPrefixLength = len(levelName) - } - } - return score -} - -func compareLevels(a, b string) int { - s1 := getLevelScore(a) - s2 := getLevelScore(b) - if s1 == s2 { - return strings.Compare(a, b) - } - return s1 - s2 -} - -func getMaxLevel(a, b string) string { - if compareLevels(a, b) >= 0 { - return a - } - return b -} - func createPermission(user, level string) dyn.Value { permission := map[string]dyn.Value{ "level": dyn.V(level), diff --git a/bundle/config/mutator/resourcemutator/fix_permissions_test.go b/bundle/config/mutator/resourcemutator/fix_permissions_test.go index 9ff42ca9a5..507096c3fd 100644 --- a/bundle/config/mutator/resourcemutator/fix_permissions_test.go +++ b/bundle/config/mutator/resourcemutator/fix_permissions_test.go @@ -3,27 +3,28 @@ package resourcemutator import ( "testing" + "github.com/databricks/cli/bundle/config/resources" "github.com/stretchr/testify/assert" ) func TestGetLevelScore(t *testing.T) { - assert.Equal(t, 17, getLevelScore("CAN_MANAGE")) - assert.Equal(t, 0, getLevelScore("UNKNOWN_PERMISSION")) - assert.Equal(t, getLevelScore("CAN_MANAGE"), getLevelScore("CAN_MANAGE_SOMETHING_ELSE")) - assert.Equal(t, getLevelScore("CAN_MANAGE_RUN"), getLevelScore("CAN_MANAGE_RUN1")) + assert.Equal(t, 17, resources.GetLevelScore("CAN_MANAGE")) + assert.Equal(t, 0, resources.GetLevelScore("UNKNOWN_PERMISSION")) + assert.Equal(t, resources.GetLevelScore("CAN_MANAGE"), resources.GetLevelScore("CAN_MANAGE_SOMETHING_ELSE")) + assert.Equal(t, resources.GetLevelScore("CAN_MANAGE_RUN"), resources.GetLevelScore("CAN_MANAGE_RUN1")) } func TestGetMaxLevel(t *testing.T) { - assert.Equal(t, "IS_OWNER", getMaxLevel("IS_OWNER", "CAN_MANAGE")) - assert.Equal(t, "IS_OWNER", getMaxLevel("CAN_MANAGE", "IS_OWNER")) - assert.Equal(t, "CAN_MANAGE", getMaxLevel("CAN_MANAGE", "CAN_EDIT")) - assert.Equal(t, "CAN_EDIT", getMaxLevel("CAN_READ", "CAN_EDIT")) + assert.Equal(t, "IS_OWNER", resources.GetMaxLevel("IS_OWNER", "CAN_MANAGE")) + assert.Equal(t, "IS_OWNER", resources.GetMaxLevel("CAN_MANAGE", "IS_OWNER")) + assert.Equal(t, "CAN_MANAGE", resources.GetMaxLevel("CAN_MANAGE", "CAN_EDIT")) + assert.Equal(t, "CAN_EDIT", resources.GetMaxLevel("CAN_READ", "CAN_EDIT")) - assert.Equal(t, "CAN_MANAGE", getMaxLevel("CAN_MANAGE", "CAN_MANAGE")) - assert.Equal(t, "CAN_READ", getMaxLevel("CAN_READ", "")) - assert.Equal(t, "CAN_READ", getMaxLevel("", "CAN_READ")) - assert.Equal(t, "", getMaxLevel("", "")) + assert.Equal(t, "CAN_MANAGE", resources.GetMaxLevel("CAN_MANAGE", "CAN_MANAGE")) + assert.Equal(t, "CAN_READ", resources.GetMaxLevel("CAN_READ", "")) + assert.Equal(t, "CAN_READ", resources.GetMaxLevel("", "CAN_READ")) + assert.Equal(t, "", resources.GetMaxLevel("", "")) - assert.Equal(t, "UNKNOWN_B", getMaxLevel("UNKNOWN_A", "UNKNOWN_B")) - assert.Equal(t, "UNKNOWN_B", getMaxLevel("UNKNOWN_B", "UNKNOWN_A")) + assert.Equal(t, "UNKNOWN_B", resources.GetMaxLevel("UNKNOWN_A", "UNKNOWN_B")) + assert.Equal(t, "UNKNOWN_B", resources.GetMaxLevel("UNKNOWN_B", "UNKNOWN_A")) } diff --git a/bundle/config/resources/permission.go b/bundle/config/resources/permission.go index d7c3f6fca9..b13c81e5e9 100644 --- a/bundle/config/resources/permission.go +++ b/bundle/config/resources/permission.go @@ -1,6 +1,9 @@ package resources -import "fmt" +import ( + "fmt" + "strings" +) // Permission holds the permission level setting for a single principal. type Permission struct { @@ -221,3 +224,60 @@ func (p SqlWarehousePermission) GetLevel() string { return string func (p SqlWarehousePermission) GetUserName() string { return p.UserName } func (p SqlWarehousePermission) GetServicePrincipalName() string { return p.ServicePrincipalName } func (p SqlWarehousePermission) GetGroupName() string { return p.GroupName } + +// PermissionOrder defines the hierarchy of permission levels. +// Higher numbers mean more permissive access. +// Based on https://docs.databricks.com/aws/en/security/auth/access-control +var PermissionOrder = map[string]int{ + "": -1, + "CAN_VIEW": 2, + "CAN_READ": 3, + "CAN_VIEW_METADATA": 4, + "CAN_RUN": 5, + "CAN_QUERY": 6, + "CAN_USE": 7, + "CAN_EDIT": 8, + "CAN_EDIT_METADATA": 9, + "CAN_CREATE": 10, + "CAN_ATTACH_TO": 11, + "CAN_RESTART": 12, + "CAN_MONITOR": 13, + "CAN_MANAGE_RUN": 14, + "CAN_MANAGE_STAGING_VERSIONS": 15, + "CAN_MANAGE_PRODUCTION_VERSIONS": 16, + "CAN_MANAGE": 17, + "IS_OWNER": 18, + // One known exception from this order: for SQL Warehouses, CAN_USE and CAN_RUN cannot be ordered and must be upgraded to CAN_MONITOR. + // We're not doing that currently. +} + +func GetLevelScore(a string) int { + score, ok := PermissionOrder[a] + if ok { + return score + } + maxPrefixLength := 0 + for levelName, levelScore := range PermissionOrder { + if strings.HasPrefix(a, levelName) && len(levelName) > maxPrefixLength { + score = levelScore + maxPrefixLength = len(levelName) + } + } + return score +} + +func CompareLevels(a, b string) int { + s1 := GetLevelScore(a) + s2 := GetLevelScore(b) + if s1 == s2 { + return strings.Compare(a, b) + } + return s1 - s2 +} + +func GetMaxLevel(a, b string) string { + if CompareLevels(a, b) >= 0 { + return a + } + return b +} diff --git a/bundle/config/validate/folder_permissions.go b/bundle/config/validate/folder_permissions.go index 575702c34c..5f9f19b2b2 100644 --- a/bundle/config/validate/folder_permissions.go +++ b/bundle/config/validate/folder_permissions.go @@ -68,7 +68,7 @@ func checkFolderPermission(ctx context.Context, b *bundle.Bundle, folderPath str } p := permissions.ObjectAclToResourcePermissions(folderPath, objPermissions.AccessControlList) - return p.Compare(b.Config.Permissions) + return p.Compare(b.Config.Permissions, b.Config.Workspace.CurrentUser) } func getClosestExistingObject(ctx context.Context, w workspace.WorkspaceInterface, folderPath string) (*workspace.ObjectInfo, error) { diff --git a/bundle/permissions/workspace_path_permissions.go b/bundle/permissions/workspace_path_permissions.go index 9e15a3658b..18422c90ef 100644 --- a/bundle/permissions/workspace_path_permissions.go +++ b/bundle/permissions/workspace_path_permissions.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/workspace" @@ -22,9 +23,18 @@ func ObjectAclToResourcePermissions(path string, acl []workspace.WorkspaceObject continue } + // Find the highest permission level for this principal (handles inherited + explicit permissions) + var highestLevel string for _, pl := range a.AllPermissions { + level := convertWorkspaceObjectPermissionLevel(pl.PermissionLevel) + if resources.GetLevelScore(level) > resources.GetLevelScore(highestLevel) { + highestLevel = level + } + } + + if highestLevel != "" { permissions = append(permissions, resources.Permission{ - Level: convertWorkspaceObjectPermissionLevel(pl.PermissionLevel), + Level: highestLevel, GroupName: a.GroupName, UserName: a.UserName, ServicePrincipalName: a.ServicePrincipalName, @@ -35,11 +45,11 @@ func ObjectAclToResourcePermissions(path string, acl []workspace.WorkspaceObject return &WorkspacePathPermissions{Permissions: permissions, Path: path} } -func (p WorkspacePathPermissions) Compare(perms []resources.Permission) diag.Diagnostics { +func (p WorkspacePathPermissions) Compare(perms []resources.Permission, currentUser *config.User) diag.Diagnostics { var diags diag.Diagnostics // Check the permissions in the workspace and see if they are all set in the bundle. - ok, missing := containsAll(p.Permissions, perms) + ok, missing := containsAll(p.Permissions, perms, currentUser) if !ok { diags = diags.Append(diag.Diagnostic{ Severity: diag.Warning, @@ -56,17 +66,38 @@ func (p WorkspacePathPermissions) Compare(perms []resources.Permission) diag.Dia return diags } -// containsAll checks if permA contains all permissions in permB. -func containsAll(permA, permB []resources.Permission) (bool, []resources.Permission) { +// samePrincipal checks if two permissions refer to the same user/group/service principal. +func samePrincipal(a, b resources.Permission) bool { + return a.UserName == b.UserName && + a.GroupName == b.GroupName && + a.ServicePrincipalName == b.ServicePrincipalName +} + +// containsAll checks if all permissions in permA (workspace) are accounted for in permB (bundle). +// A workspace permission is considered accounted for if the bundle has the same principal +// with an equal or higher permission level, OR if the permission is for a group that belongs +// to the current deployment user and the current user is already tracked in the bundle. +func containsAll(permA, permB []resources.Permission, currentUser *config.User) (bool, []resources.Permission) { var missing []resources.Permission for _, a := range permA { found := false + + // Check if bundle has same principal with adequate permission for _, b := range permB { - if a == b { + if samePrincipal(a, b) && resources.GetLevelScore(b.Level) >= resources.GetLevelScore(a.Level) { found = true break } } + + // If not found directly, check if this is a group of the current user + // and the current user is already tracked in the bundle + if !found && a.GroupName != "" && currentUser != nil { + if isCurrentUserGroup(a.GroupName, currentUser) && currentUserInBundle(permB, currentUser) { + found = true + } + } + if !found { missing = append(missing, a) } @@ -74,6 +105,30 @@ func containsAll(permA, permB []resources.Permission) (bool, []resources.Permiss return len(missing) == 0, missing } +func isCurrentUserGroup(groupName string, currentUser *config.User) bool { + for _, g := range currentUser.Groups { + // Check both Display (preferred) and Value (fallback) fields + if g.Display == groupName || g.Value == groupName { + return true + } + } + return false +} + +func currentUserInBundle(perms []resources.Permission, currentUser *config.User) bool { + for _, p := range perms { + // Check direct user match + if p.UserName == currentUser.UserName || p.ServicePrincipalName == currentUser.UserName { + return true + } + // Check if any bundle group contains the current user + if p.GroupName != "" && isCurrentUserGroup(p.GroupName, currentUser) { + return true + } + } + return false +} + // convertWorkspaceObjectPermissionLevel converts matching object permission levels to bundle ones. // If there is no matching permission level, it returns permission level as is, for example, CAN_EDIT. func convertWorkspaceObjectPermissionLevel(level workspace.WorkspaceObjectPermissionLevel) string { diff --git a/bundle/permissions/workspace_path_permissions_test.go b/bundle/permissions/workspace_path_permissions_test.go index 73beba62c3..66b818fa0e 100644 --- a/bundle/permissions/workspace_path_permissions_test.go +++ b/bundle/permissions/workspace_path_permissions_test.go @@ -120,7 +120,99 @@ func TestWorkspacePathPermissionsCompare(t *testing.T) { for _, tc := range testCases { wp := ObjectAclToResourcePermissions("path", tc.acl) - diags := wp.Compare(tc.perms) + diags := wp.Compare(tc.perms, nil) require.Equal(t, tc.expected, diags) } } + +func TestWorkspacePathPermissionsCompareWithHierarchy(t *testing.T) { + testCases := []struct { + name string + perms []resources.Permission + acl []workspace.WorkspaceObjectAccessControlResponse + expected diag.Diagnostics + }{ + { + name: "bundle grants higher permission than workspace - no warning", + perms: []resources.Permission{ + {Level: CAN_MANAGE, UserName: "foo@bar.com"}, + }, + acl: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_READ"}, + }, + }, + }, + expected: nil, + }, + { + name: "bundle grants lower permission than workspace - warning", + perms: []resources.Permission{ + {Level: CAN_VIEW, UserName: "foo@bar.com"}, + }, + acl: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + expected: diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "workspace folder has permissions not configured in bundle", + Detail: "The following permissions apply to the workspace folder at \"path\" " + + "but are not configured in the bundle:\n- level: CAN_MANAGE, user_name: foo@bar.com\n\n" + + "Add them to your bundle permissions or remove them from the folder.\n" + + "See https://docs.databricks.com/dev-tools/bundles/permissions", + }, + }, + }, + { + name: "bundle grants same permission as workspace - no warning", + perms: []resources.Permission{ + {Level: CAN_MANAGE, UserName: "foo@bar.com"}, + }, + acl: []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_MANAGE"}, + }, + }, + }, + expected: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + wp := ObjectAclToResourcePermissions("path", tc.acl) + diags := wp.Compare(tc.perms, nil) + require.Equal(t, tc.expected, diags) + }) + } +} + +func TestWorkspacePathPermissionsDeduplication(t *testing.T) { + // User has both inherited CAN_VIEW and explicit CAN_MANAGE + acl := []workspace.WorkspaceObjectAccessControlResponse{ + { + UserName: "foo@bar.com", + AllPermissions: []workspace.WorkspaceObjectPermission{ + {PermissionLevel: "CAN_READ"}, // inherited + {PermissionLevel: "CAN_MANAGE"}, // explicit + }, + }, + } + + wp := ObjectAclToResourcePermissions("path", acl) + + // Should only have one permission entry with the highest level + require.Len(t, wp.Permissions, 1) + require.Equal(t, CAN_MANAGE, wp.Permissions[0].Level) + require.Equal(t, "foo@bar.com", wp.Permissions[0].UserName) +} From cc1459522af78c593f8753dd4dc301b757f8d1d3 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 7 Jan 2026 15:11:38 +0100 Subject: [PATCH 3/5] Simplify permission comparison logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove untested current user group tracking code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- bundle/config/validate/folder_permissions.go | 2 +- .../permissions/workspace_path_permissions.go | 45 ++----------------- .../workspace_path_permissions_test.go | 4 +- 3 files changed, 7 insertions(+), 44 deletions(-) diff --git a/bundle/config/validate/folder_permissions.go b/bundle/config/validate/folder_permissions.go index 5f9f19b2b2..575702c34c 100644 --- a/bundle/config/validate/folder_permissions.go +++ b/bundle/config/validate/folder_permissions.go @@ -68,7 +68,7 @@ func checkFolderPermission(ctx context.Context, b *bundle.Bundle, folderPath str } p := permissions.ObjectAclToResourcePermissions(folderPath, objPermissions.AccessControlList) - return p.Compare(b.Config.Permissions, b.Config.Workspace.CurrentUser) + return p.Compare(b.Config.Permissions) } func getClosestExistingObject(ctx context.Context, w workspace.WorkspaceInterface, folderPath string) (*workspace.ObjectInfo, error) { diff --git a/bundle/permissions/workspace_path_permissions.go b/bundle/permissions/workspace_path_permissions.go index 18422c90ef..cabf4117f6 100644 --- a/bundle/permissions/workspace_path_permissions.go +++ b/bundle/permissions/workspace_path_permissions.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/workspace" @@ -45,11 +44,11 @@ func ObjectAclToResourcePermissions(path string, acl []workspace.WorkspaceObject return &WorkspacePathPermissions{Permissions: permissions, Path: path} } -func (p WorkspacePathPermissions) Compare(perms []resources.Permission, currentUser *config.User) diag.Diagnostics { +func (p WorkspacePathPermissions) Compare(perms []resources.Permission) diag.Diagnostics { var diags diag.Diagnostics // Check the permissions in the workspace and see if they are all set in the bundle. - ok, missing := containsAll(p.Permissions, perms, currentUser) + ok, missing := containsAll(p.Permissions, perms) if !ok { diags = diags.Append(diag.Diagnostic{ Severity: diag.Warning, @@ -75,29 +74,17 @@ func samePrincipal(a, b resources.Permission) bool { // containsAll checks if all permissions in permA (workspace) are accounted for in permB (bundle). // A workspace permission is considered accounted for if the bundle has the same principal -// with an equal or higher permission level, OR if the permission is for a group that belongs -// to the current deployment user and the current user is already tracked in the bundle. -func containsAll(permA, permB []resources.Permission, currentUser *config.User) (bool, []resources.Permission) { +// with an equal or higher permission level. +func containsAll(permA, permB []resources.Permission) (bool, []resources.Permission) { var missing []resources.Permission for _, a := range permA { found := false - - // Check if bundle has same principal with adequate permission for _, b := range permB { if samePrincipal(a, b) && resources.GetLevelScore(b.Level) >= resources.GetLevelScore(a.Level) { found = true break } } - - // If not found directly, check if this is a group of the current user - // and the current user is already tracked in the bundle - if !found && a.GroupName != "" && currentUser != nil { - if isCurrentUserGroup(a.GroupName, currentUser) && currentUserInBundle(permB, currentUser) { - found = true - } - } - if !found { missing = append(missing, a) } @@ -105,30 +92,6 @@ func containsAll(permA, permB []resources.Permission, currentUser *config.User) return len(missing) == 0, missing } -func isCurrentUserGroup(groupName string, currentUser *config.User) bool { - for _, g := range currentUser.Groups { - // Check both Display (preferred) and Value (fallback) fields - if g.Display == groupName || g.Value == groupName { - return true - } - } - return false -} - -func currentUserInBundle(perms []resources.Permission, currentUser *config.User) bool { - for _, p := range perms { - // Check direct user match - if p.UserName == currentUser.UserName || p.ServicePrincipalName == currentUser.UserName { - return true - } - // Check if any bundle group contains the current user - if p.GroupName != "" && isCurrentUserGroup(p.GroupName, currentUser) { - return true - } - } - return false -} - // convertWorkspaceObjectPermissionLevel converts matching object permission levels to bundle ones. // If there is no matching permission level, it returns permission level as is, for example, CAN_EDIT. func convertWorkspaceObjectPermissionLevel(level workspace.WorkspaceObjectPermissionLevel) string { diff --git a/bundle/permissions/workspace_path_permissions_test.go b/bundle/permissions/workspace_path_permissions_test.go index 66b818fa0e..df2f86a8a8 100644 --- a/bundle/permissions/workspace_path_permissions_test.go +++ b/bundle/permissions/workspace_path_permissions_test.go @@ -120,7 +120,7 @@ func TestWorkspacePathPermissionsCompare(t *testing.T) { for _, tc := range testCases { wp := ObjectAclToResourcePermissions("path", tc.acl) - diags := wp.Compare(tc.perms, nil) + diags := wp.Compare(tc.perms) require.Equal(t, tc.expected, diags) } } @@ -191,7 +191,7 @@ func TestWorkspacePathPermissionsCompareWithHierarchy(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { wp := ObjectAclToResourcePermissions("path", tc.acl) - diags := wp.Compare(tc.perms, nil) + diags := wp.Compare(tc.perms) require.Equal(t, tc.expected, diags) }) } From aaaf403eaf620738a7d20297ce1279d95096d025 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Thu, 8 Jan 2026 09:50:43 +0100 Subject: [PATCH 4/5] Add NEXT_CHANGELOG entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- NEXT_CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 6dec8b7cd6..511ebf466e 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -10,6 +10,7 @@ ### Bundles * Add interactive SQL warehouse picker to `default-sql` and `dbt-sql` bundle templates ([#4170](https://github.com/databricks/cli/pull/4170)) +* Fix false positive folder permission warnings and make them more actionable ([#4216](https://github.com/databricks/cli/pull/4216)) ### Dependency updates From f80e15dd143283393f8b76682ccd7eb075051e9c Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Sun, 11 Jan 2026 17:34:59 +0100 Subject: [PATCH 5/5] Remove unrelated changelog entry --- NEXT_CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index cee52146ac..ff8b2d38fc 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -9,7 +9,6 @@ To disable this, set the environment variable DATABRICKS_CACHE_ENABLED to false. ### CLI ### Bundles -* Add interactive SQL warehouse picker to `default-sql` and `dbt-sql` bundle templates ([#4170](https://github.com/databricks/cli/pull/4170)) * Enable caching user identity by default ([#4202](https://github.com/databricks/cli/pull/4202)) * Fix false positive folder permission warnings and make them more actionable ([#4216](https://github.com/databricks/cli/pull/4216)) * Pass additional Azure DevOps system variables ([#4236](https://github.com/databricks/cli/pull/4236))