Skip to content

Commit b703045

Browse files
shreyas-goenkashreyas-goenka
committed
bundle: warn when a workspace path is in /Workspace/Shared without users CAN_MANAGE
Renames ValidateSharedRootPermissions to ValidateWorkspaceSharedPermissions 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 only when state_path is nested under root_path, since the root warning already covers it. When state_path is a separate shared folder, both warnings fire. Co-authored-by: Shreyas Goenka <shreyas.goenka@databricks.com>
1 parent e92f999 commit b703045

9 files changed

Lines changed: 181 additions & 54 deletions

File tree

‎acceptance/bundle/resources/jobs/shared-root-path/output.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
>>> [CLI] bundle deploy
33
Warning: the bundle root path /Workspace/Shared/[USERNAME]/.bundle/[UNIQUE_NAME] is writable by all workspace users
44

5-
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>.
5+
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>.
66

77
Uploading bundle files to /Workspace/Shared/[USERNAME]/.bundle/[UNIQUE_NAME]/files...
88
Deploying resources...
@@ -12,7 +12,7 @@ Deployment complete!
1212
>>> [CLI] bundle destroy --auto-approve
1313
Warning: the bundle root path /Workspace/Shared/[USERNAME]/.bundle/[UNIQUE_NAME] is writable by all workspace users
1414

15-
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>.
15+
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>.
1616

1717
The following resources will be deleted:
1818
delete resources.jobs.foo

‎acceptance/bundle/validate/workspace_shared_permissions/databricks.yml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
bundle:
2+
name: test-bundle
3+
4+
targets:
5+
# state_path defaults to a directory under root_path, so only the root path
6+
# warning fires.
7+
one_warning:
8+
workspace:
9+
root_path: /Workspace/Shared/test-bundle/one
10+
11+
# state_path is a separate folder under /Workspace/Shared (not nested under
12+
# root_path), so both the root path and state path warnings fire.
13+
two_warnings:
14+
workspace:
15+
root_path: /Workspace/Shared/test-bundle/two
16+
state_path: /Workspace/Shared/test-bundle-state

‎acceptance/bundle/validate/workspace_shared_permissions/out.test.toml‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎acceptance/bundle/validate/workspace_shared_permissions/output.txt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
2+
>>> [CLI] bundle validate -t one_warning
3+
Warning: the bundle root path /Workspace/Shared/test-bundle/one is writable by all workspace users
4+
5+
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>.
6+
7+
Name: test-bundle
8+
Target: one_warning
9+
Workspace:
10+
User: [USERNAME]
11+
Path: /Workspace/Shared/test-bundle/one
12+
13+
Found 1 warning
14+
15+
>>> [CLI] bundle validate -t two_warnings
16+
Warning: the bundle root path /Workspace/Shared/test-bundle/two is writable by all workspace users
17+
18+
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>.
19+
20+
Warning: the bundle state path /Workspace/Shared/test-bundle-state is writable by all workspace users
21+
22+
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>.
23+
24+
Name: test-bundle
25+
Target: two_warnings
26+
Workspace:
27+
User: [USERNAME]
28+
Path: /Workspace/Shared/test-bundle/two
29+
30+
Found 2 warnings

‎acceptance/bundle/validate/workspace_shared_permissions/script‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
trace $CLI bundle validate -t one_warning
2+
trace $CLI bundle validate -t two_warnings

‎bundle/permissions/validate.go‎

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,49 +3,86 @@ package permissions
33
import (
44
"context"
55
"fmt"
6+
"strings"
67

78
"github.com/databricks/cli/bundle"
89
"github.com/databricks/cli/bundle/libraries"
910
"github.com/databricks/cli/libs/diag"
1011
)
1112

12-
type validateSharedRootPermissions struct{}
13+
type validateWorkspaceSharedPermissions struct{}
1314

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

18-
func (*validateSharedRootPermissions) Name() string {
19-
return "ValidateSharedRootPermissions"
23+
func (*validateWorkspaceSharedPermissions) Name() string {
24+
return "ValidateWorkspaceSharedPermissions"
2025
}
2126

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 {
27+
func (*validateWorkspaceSharedPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
3228
var diags diag.Diagnostics
3329

34-
allUsers := false
30+
rootPath := b.Config.Workspace.RootPath
31+
statePath := b.Config.Workspace.StatePath
32+
rootIsShared := libraries.IsWorkspaceSharedPath(rootPath)
33+
34+
// Whether the top-level permissions grant group_name: users CAN_MANAGE, i.e.
35+
// the broad /Workspace/Shared access is intentional and declared.
36+
usersCanManage := false
3537
for _, p := range b.Config.Permissions {
3638
if p.GroupName == "users" && p.Level == CAN_MANAGE {
37-
allUsers = true
39+
usersCanManage = true
3840
break
3941
}
4042
}
4143

42-
if !allUsers {
44+
// root_path is in /Workspace/Shared without users CAN_MANAGE.
45+
if rootIsShared && !usersCanManage {
4346
diags = diags.Append(diag.Diagnostic{
4447
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>.",
48+
Summary: fmt.Sprintf("the bundle root path %s is writable by all workspace users", rootPath),
49+
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>.",
50+
})
51+
}
52+
53+
// state_path is in /Workspace/Shared without users CAN_MANAGE. Skip only when
54+
// state_path is nested under root_path, since the root warning above already
55+
// covers it. When state_path is a separate folder, warn about it on its own.
56+
if libraries.IsWorkspaceSharedPath(statePath) && !statePathUnderRootPath(rootPath, statePath) && !usersCanManage {
57+
diags = diags.Append(diag.Diagnostic{
58+
Severity: diag.Warning,
59+
Summary: fmt.Sprintf("the bundle state path %s is writable by all workspace users", statePath),
60+
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>.",
4761
})
4862
}
4963

5064
return diags
5165
}
66+
67+
// statePathUnderRootPath returns true when statePath is nested under rootPath, in
68+
// which case permissions applied to root_path also cover the state directory.
69+
//
70+
// By default state_path lives under root_path (it defaults to "${root_path}/state"),
71+
// so we treat it as nested unless both paths are set and root_path is genuinely not a
72+
// prefix of state_path. That keeps us from emitting a separate state warning for the
73+
// common case.
74+
//
75+
// Both paths are /Workspace-normalized by PrependWorkspacePrefix before this mutator
76+
// runs, so the prefix comparison here is reliable.
77+
func statePathUnderRootPath(rootPath, statePath string) bool {
78+
if rootPath == "" || statePath == "" {
79+
return true
80+
}
81+
if statePath == rootPath {
82+
return true
83+
}
84+
if !strings.HasSuffix(rootPath, "/") {
85+
rootPath += "/"
86+
}
87+
return strings.HasPrefix(statePath, rootPath)
88+
}

‎bundle/permissions/validate_test.go‎

Lines changed: 66 additions & 27 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, ValidateWorkspaceSharedPermissions())
20+
}
21+
22+
func TestValidateWorkspaceSharedPermissions_RootShared_NoWarningWithUsersManage(t *testing.T) {
1623
b := &bundle.Bundle{
1724
Config: config.Root{
1825
Workspace: config.Workspace{
@@ -21,45 +28,77 @@ 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 TestValidateWorkspaceSharedPermissions_StateShared_WarnWithoutUsersManage(t *testing.T) {
4138
b := &bundle.Bundle{
4239
Config: config.Root{
4340
Workspace: config.Workspace{
44-
RootPath: "/Workspace/Shared/foo/bar",
41+
RootPath: "/Workspace/Users/user@example.test",
42+
StatePath: "/Workspace/Shared/state",
4543
},
4644
Permissions: []resources.Permission{
47-
{Level: CAN_MANAGE, UserName: "foo@bar.test"},
45+
{Level: CAN_MANAGE, UserName: "user@example.test"},
46+
},
47+
},
48+
}
49+
diags := applyValidate(t, b)
50+
require.Len(t, diags, 1)
51+
assert.Equal(t, diag.Warning, diags[0].Severity)
52+
assert.Contains(t, diags[0].Summary, "state path")
53+
assert.Contains(t, diags[0].Summary, "/Workspace/Shared/state")
54+
}
55+
56+
func TestValidateWorkspaceSharedPermissions_StateShared_NoWarningWithUsersManage(t *testing.T) {
57+
b := &bundle.Bundle{
58+
Config: config.Root{
59+
Workspace: config.Workspace{
60+
RootPath: "/Workspace/Users/user@example.test",
61+
StatePath: "/Workspace/Shared/state",
4862
},
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-
},
63+
Permissions: []resources.Permission{
64+
{Level: CAN_MANAGE, GroupName: "users"},
5465
},
5566
},
5667
}
68+
diags := applyValidate(t, b)
69+
require.Empty(t, diags)
70+
}
5771

58-
m := mocks.NewMockWorkspaceClient(t)
59-
b.SetWorkpaceClient(m.WorkspaceClient)
72+
func TestValidateWorkspaceSharedPermissions_NoSharedPaths_NoWarning(t *testing.T) {
73+
b := &bundle.Bundle{
74+
Config: config.Root{
75+
Workspace: config.Workspace{
76+
RootPath: "/Workspace/Users/user@example.test/bundle",
77+
StatePath: "/Workspace/Users/user@example.test/other-state",
78+
},
79+
},
80+
}
81+
diags := applyValidate(t, b)
82+
require.Empty(t, diags)
83+
}
6084

61-
diags := bundle.Apply(t.Context(), b, ValidateSharedRootPermissions())
62-
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)
85+
func TestStatePathUnderRootPath(t *testing.T) {
86+
cases := []struct {
87+
name string
88+
rootPath string
89+
statePath string
90+
want bool
91+
}{
92+
{name: "default nested state", rootPath: "/Workspace/Users/me/bundle", statePath: "/Workspace/Users/me/bundle/state", want: true},
93+
{name: "equal paths", rootPath: "/Workspace/Users/me/bundle", statePath: "/Workspace/Users/me/bundle", want: true},
94+
{name: "separate folder", rootPath: "/Workspace/Users/me/bundle", statePath: "/Workspace/Shared/state", want: false},
95+
{name: "sibling prefix is not nested", rootPath: "/Workspace/Users/me/bundle", statePath: "/Workspace/Users/me/bundle-2/state", want: false},
96+
{name: "empty state defaults to nested", rootPath: "/Workspace/Users/me/bundle", statePath: "", want: true},
97+
{name: "empty root defaults to nested", rootPath: "", statePath: "/Workspace/Shared/state", want: true},
98+
}
99+
for _, tc := range cases {
100+
t.Run(tc.name, func(t *testing.T) {
101+
assert.Equal(t, tc.want, statePathUnderRootPath(tc.rootPath, tc.statePath))
102+
})
103+
}
65104
}

‎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, ValidateWorkspaceSharedPermissions(), 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.ValidateWorkspaceSharedPermissions(),
198198

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

0 commit comments

Comments
 (0)