Skip to content

Commit cc47397

Browse files
bundle: warn when a workspace path is in /Workspace/Shared without users CAN_MANAGE
Renames ValidateSharedRootPermissions to ValidateWorkspacePermissions and extends it to also cover workspace.state_path. It warns when root_path or state_path is in /Workspace/Shared — granting read/write to all workspace users — but the top-level permissions section does not declare that access via group_name: users CAN_MANAGE. The state_path warning is suppressed when root_path is also shared, since the root warning already covers the entire bundle tree. Co-authored-by: Shreyas Goenka <shreyas.goenka@databricks.com>
1 parent e92f999 commit cc47397

4 files changed

Lines changed: 123 additions & 53 deletions

File tree

bundle/permissions/validate.go

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,43 +9,58 @@ import (
99
"github.com/databricks/cli/libs/diag"
1010
)
1111

12-
type validateSharedRootPermissions struct{}
12+
type validateWorkspacePermissions struct{}
1313

14-
func ValidateSharedRootPermissions() bundle.Mutator {
15-
return &validateSharedRootPermissions{}
14+
// ValidateWorkspacePermissions warns when a workspace path is configured under
15+
// /Workspace/Shared — which grants read/write access to all workspace users —
16+
// without the top-level permissions section declaring that broad access via
17+
// group_name: users with CAN_MANAGE.
18+
func ValidateWorkspacePermissions() bundle.Mutator {
19+
return &validateWorkspacePermissions{}
1620
}
1721

18-
func (*validateSharedRootPermissions) Name() string {
19-
return "ValidateSharedRootPermissions"
22+
func (*validateWorkspacePermissions) Name() string {
23+
return "ValidateWorkspacePermissions"
2024
}
2125

22-
func (*validateSharedRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
23-
if libraries.IsWorkspaceSharedPath(b.Config.Workspace.RootPath) {
24-
return isUsersGroupPermissionSet(b)
25-
}
26-
27-
return nil
28-
}
29-
30-
// isUsersGroupPermissionSet checks that top-level permissions set for bundle contain group_name: users with CAN_MANAGE permission.
31-
func isUsersGroupPermissionSet(b *bundle.Bundle) diag.Diagnostics {
26+
func (*validateWorkspacePermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
3227
var diags diag.Diagnostics
3328

34-
allUsers := false
35-
for _, p := range b.Config.Permissions {
36-
if p.GroupName == "users" && p.Level == CAN_MANAGE {
37-
allUsers = true
38-
break
39-
}
29+
rootPath := b.Config.Workspace.RootPath
30+
statePath := b.Config.Workspace.StatePath
31+
rootIsShared := libraries.IsWorkspaceSharedPath(rootPath)
32+
usersCanManage := hasUsersGroupManagePermission(b)
33+
34+
// root_path is in /Workspace/Shared without users CAN_MANAGE.
35+
if rootIsShared && !usersCanManage {
36+
diags = diags.Append(diag.Diagnostic{
37+
Severity: diag.Warning,
38+
Summary: fmt.Sprintf("the bundle root path %s is writable by all workspace users", rootPath),
39+
Detail: "The bundle root path is in /Workspace/Shared, giving read/write access to all workspace users that is not reflected in the permissions section. If this is intentional, add CAN_MANAGE for 'group_name: users' to your bundle permissions. Otherwise, move the bundle to a restricted path such as /Workspace/Users/<username>.",
40+
})
4041
}
4142

42-
if !allUsers {
43+
// state_path is in /Workspace/Shared without users CAN_MANAGE. Skip when
44+
// root_path is also shared, since the warning above already covers the entire
45+
// bundle tree including the state directory.
46+
if libraries.IsWorkspaceSharedPath(statePath) && !rootIsShared && !usersCanManage {
4347
diags = diags.Append(diag.Diagnostic{
4448
Severity: diag.Warning,
45-
Summary: fmt.Sprintf("the bundle root path %s is writable by all workspace users", b.Config.Workspace.RootPath),
46-
Detail: "The bundle is configured to use /Workspace/Shared, which will give read/write access to all users. If this is intentional, add CAN_MANAGE for 'group_name: users' permission to your bundle configuration. If the deployment should be restricted, move it to a restricted folder such as /Workspace/Users/<username or principal name>.",
49+
Summary: fmt.Sprintf("the bundle state path %s is writable by all workspace users", statePath),
50+
Detail: "The bundle state path is in /Workspace/Shared, giving read/write access to all workspace users that is not reflected in the permissions section. If this is intentional, add CAN_MANAGE for 'group_name: users' to your bundle permissions. Otherwise, move the state path to a restricted location such as /Workspace/Users/<username>.",
4751
})
4852
}
4953

5054
return diags
5155
}
56+
57+
// hasUsersGroupManagePermission returns true if the top-level permissions include
58+
// group_name: users with CAN_MANAGE level.
59+
func hasUsersGroupManagePermission(b *bundle.Bundle) bool {
60+
for _, p := range b.Config.Permissions {
61+
if p.GroupName == "users" && p.Level == CAN_MANAGE {
62+
return true
63+
}
64+
}
65+
return false
66+
}

bundle/permissions/validate_test.go

Lines changed: 79 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,18 @@ import (
88
"github.com/databricks/cli/bundle/config/resources"
99
"github.com/databricks/cli/libs/diag"
1010
"github.com/databricks/databricks-sdk-go/experimental/mocks"
11-
"github.com/databricks/databricks-sdk-go/service/jobs"
11+
"github.com/stretchr/testify/assert"
1212
"github.com/stretchr/testify/require"
1313
)
1414

15-
func TestValidateSharedRootPermissionsForShared(t *testing.T) {
15+
func applyValidate(t *testing.T, b *bundle.Bundle) diag.Diagnostics {
16+
t.Helper()
17+
m := mocks.NewMockWorkspaceClient(t)
18+
b.SetWorkpaceClient(m.WorkspaceClient)
19+
return bundle.Apply(t.Context(), b, ValidateWorkspacePermissions())
20+
}
21+
22+
func TestValidateWorkspacePermissions_RootShared_NoWarningWithUsersManage(t *testing.T) {
1623
b := &bundle.Bundle{
1724
Config: config.Root{
1825
Workspace: config.Workspace{
@@ -21,23 +28,13 @@ func TestValidateSharedRootPermissionsForShared(t *testing.T) {
2128
Permissions: []resources.Permission{
2229
{Level: CAN_MANAGE, GroupName: "users"},
2330
},
24-
Resources: config.Resources{
25-
Jobs: map[string]*resources.Job{
26-
"job_1": {JobSettings: jobs.JobSettings{Name: "job_1"}},
27-
"job_2": {JobSettings: jobs.JobSettings{Name: "job_2"}},
28-
},
29-
},
3031
},
3132
}
32-
33-
m := mocks.NewMockWorkspaceClient(t)
34-
b.SetWorkpaceClient(m.WorkspaceClient)
35-
36-
diags := bundle.Apply(t.Context(), b, ValidateSharedRootPermissions())
33+
diags := applyValidate(t, b)
3734
require.Empty(t, diags)
3835
}
3936

40-
func TestValidateSharedRootPermissionsForSharedError(t *testing.T) {
37+
func TestValidateWorkspacePermissions_RootShared_WarnWithoutUsersManage(t *testing.T) {
4138
b := &bundle.Bundle{
4239
Config: config.Root{
4340
Workspace: config.Workspace{
@@ -46,20 +43,78 @@ func TestValidateSharedRootPermissionsForSharedError(t *testing.T) {
4643
Permissions: []resources.Permission{
4744
{Level: CAN_MANAGE, UserName: "foo@bar.test"},
4845
},
49-
Resources: config.Resources{
50-
Jobs: map[string]*resources.Job{
51-
"job_1": {JobSettings: jobs.JobSettings{Name: "job_1"}},
52-
"job_2": {JobSettings: jobs.JobSettings{Name: "job_2"}},
53-
},
46+
},
47+
}
48+
diags := applyValidate(t, b)
49+
require.Len(t, diags, 1)
50+
assert.Equal(t, diag.Warning, diags[0].Severity)
51+
assert.Contains(t, diags[0].Summary, "root path")
52+
assert.Contains(t, diags[0].Summary, "/Workspace/Shared/foo/bar")
53+
}
54+
55+
func TestValidateWorkspacePermissions_StateShared_WarnWithoutUsersManage(t *testing.T) {
56+
b := &bundle.Bundle{
57+
Config: config.Root{
58+
Workspace: config.Workspace{
59+
RootPath: "/Workspace/Users/user@example.test",
60+
StatePath: "/Workspace/Shared/state",
61+
},
62+
Permissions: []resources.Permission{
63+
{Level: CAN_MANAGE, UserName: "user@example.test"},
5464
},
5565
},
5666
}
67+
diags := applyValidate(t, b)
68+
require.Len(t, diags, 1)
69+
assert.Equal(t, diag.Warning, diags[0].Severity)
70+
assert.Contains(t, diags[0].Summary, "state path")
71+
assert.Contains(t, diags[0].Summary, "/Workspace/Shared/state")
72+
}
5773

58-
m := mocks.NewMockWorkspaceClient(t)
59-
b.SetWorkpaceClient(m.WorkspaceClient)
74+
func TestValidateWorkspacePermissions_StateShared_NoWarningWithUsersManage(t *testing.T) {
75+
b := &bundle.Bundle{
76+
Config: config.Root{
77+
Workspace: config.Workspace{
78+
RootPath: "/Workspace/Users/user@example.test",
79+
StatePath: "/Workspace/Shared/state",
80+
},
81+
Permissions: []resources.Permission{
82+
{Level: CAN_MANAGE, GroupName: "users"},
83+
},
84+
},
85+
}
86+
diags := applyValidate(t, b)
87+
require.Empty(t, diags)
88+
}
6089

61-
diags := bundle.Apply(t.Context(), b, ValidateSharedRootPermissions())
90+
func TestValidateWorkspacePermissions_BothShared_OnlyOneWarning(t *testing.T) {
91+
// root_path is also in Shared — the root warning covers the whole tree, so the
92+
// state warning must not fire.
93+
b := &bundle.Bundle{
94+
Config: config.Root{
95+
Workspace: config.Workspace{
96+
RootPath: "/Workspace/Shared/root",
97+
StatePath: "/Workspace/Shared/state",
98+
},
99+
Permissions: []resources.Permission{
100+
{Level: CAN_MANAGE, UserName: "user@example.test"},
101+
},
102+
},
103+
}
104+
diags := applyValidate(t, b)
62105
require.Len(t, diags, 1)
63-
require.Equal(t, "the bundle root path /Workspace/Shared/foo/bar is writable by all workspace users", diags[0].Summary)
64-
require.Equal(t, diag.Warning, diags[0].Severity)
106+
assert.Contains(t, diags[0].Summary, "root path")
107+
}
108+
109+
func TestValidateWorkspacePermissions_NoSharedPaths_NoWarning(t *testing.T) {
110+
b := &bundle.Bundle{
111+
Config: config.Root{
112+
Workspace: config.Workspace{
113+
RootPath: "/Workspace/Users/user@example.test/bundle",
114+
StatePath: "/Workspace/Users/user@example.test/other-state",
115+
},
116+
},
117+
}
118+
diags := applyValidate(t, b)
119+
require.Empty(t, diags)
65120
}

bundle/permissions/workspace_root_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) {
7272
WorkspaceObjectType: "directories",
7373
}).Return(nil, nil)
7474

75-
diags := bundle.ApplySeq(t.Context(), b, ValidateSharedRootPermissions(), ApplyWorkspaceRootPermissions())
75+
diags := bundle.ApplySeq(t.Context(), b, ValidateWorkspacePermissions(), ApplyWorkspaceRootPermissions())
7676
require.Empty(t, diags)
7777
}
7878

bundle/phases/initialize.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,10 @@ func Initialize(ctx context.Context, b *bundle.Bundle) {
191191
apps.Validate(),
192192

193193
resourcemutator.ValidateTargetMode(),
194-
// Reads (typed): b.Config.Workspace.RootPath (checks if path is in shared workspace)
195-
// Reads (typed): b.Config.Permissions (checks if users group has CAN_MANAGE permission)
196-
// Validates that when using a shared workspace path, appropriate permissions are configured
197-
permissions.ValidateSharedRootPermissions(),
194+
// Reads (typed): b.Config.Workspace.{RootPath,StatePath} and b.Config.Permissions.
195+
// Warns when a workspace path is in /Workspace/Shared without the permissions
196+
// section granting all workspace users CAN_MANAGE.
197+
permissions.ValidateWorkspacePermissions(),
198198

199199
// Annotate resources with "deployment" metadata.
200200
//

0 commit comments

Comments
 (0)