Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 39 additions & 24 deletions bundle/permissions/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,43 +9,58 @@ import (
"github.com/databricks/cli/libs/diag"
)

type validateSharedRootPermissions struct{}
type validateWorkspacePermissions struct{}

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

func (*validateSharedRootPermissions) Name() string {
return "ValidateSharedRootPermissions"
func (*validateWorkspacePermissions) Name() string {
return "ValidateWorkspacePermissions"
}

func (*validateSharedRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
if libraries.IsWorkspaceSharedPath(b.Config.Workspace.RootPath) {
return isUsersGroupPermissionSet(b)
}

return nil
}

// isUsersGroupPermissionSet checks that top-level permissions set for bundle contain group_name: users with CAN_MANAGE permission.
func isUsersGroupPermissionSet(b *bundle.Bundle) diag.Diagnostics {
func (*validateWorkspacePermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
var diags diag.Diagnostics

allUsers := false
for _, p := range b.Config.Permissions {
if p.GroupName == "users" && p.Level == CAN_MANAGE {
allUsers = true
break
}
rootPath := b.Config.Workspace.RootPath
statePath := b.Config.Workspace.StatePath
rootIsShared := libraries.IsWorkspaceSharedPath(rootPath)
usersCanManage := hasUsersGroupManagePermission(b)

// root_path is in /Workspace/Shared without users CAN_MANAGE.
if rootIsShared && !usersCanManage {
diags = diags.Append(diag.Diagnostic{
Severity: diag.Warning,
Summary: fmt.Sprintf("the bundle root path %s is writable by all workspace users", rootPath),
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>.",
})
}

if !allUsers {
// state_path is in /Workspace/Shared without users CAN_MANAGE. Skip when
// root_path is also shared, since the warning above already covers the entire
// bundle tree including the state directory.
if libraries.IsWorkspaceSharedPath(statePath) && !rootIsShared && !usersCanManage {
diags = diags.Append(diag.Diagnostic{
Severity: diag.Warning,
Summary: fmt.Sprintf("the bundle root path %s is writable by all workspace users", b.Config.Workspace.RootPath),
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>.",
Summary: fmt.Sprintf("the bundle state path %s is writable by all workspace users", statePath),
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>.",
})
}

return diags
}

// hasUsersGroupManagePermission returns true if the top-level permissions include
// group_name: users with CAN_MANAGE level.
func hasUsersGroupManagePermission(b *bundle.Bundle) bool {
for _, p := range b.Config.Permissions {
if p.GroupName == "users" && p.Level == CAN_MANAGE {
return true
}
}
return false
}
103 changes: 79 additions & 24 deletions bundle/permissions/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,18 @@ import (
"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/databricks-sdk-go/experimental/mocks"
"github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestValidateSharedRootPermissionsForShared(t *testing.T) {
func applyValidate(t *testing.T, b *bundle.Bundle) diag.Diagnostics {
t.Helper()
m := mocks.NewMockWorkspaceClient(t)
b.SetWorkpaceClient(m.WorkspaceClient)
return bundle.Apply(t.Context(), b, ValidateWorkspacePermissions())
}

func TestValidateWorkspacePermissions_RootShared_NoWarningWithUsersManage(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
Expand All @@ -21,23 +28,13 @@ func TestValidateSharedRootPermissionsForShared(t *testing.T) {
Permissions: []resources.Permission{
{Level: CAN_MANAGE, GroupName: "users"},
},
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job_1": {JobSettings: jobs.JobSettings{Name: "job_1"}},
"job_2": {JobSettings: jobs.JobSettings{Name: "job_2"}},
},
},
},
}

m := mocks.NewMockWorkspaceClient(t)
b.SetWorkpaceClient(m.WorkspaceClient)

diags := bundle.Apply(t.Context(), b, ValidateSharedRootPermissions())
diags := applyValidate(t, b)
require.Empty(t, diags)
}

func TestValidateSharedRootPermissionsForSharedError(t *testing.T) {
func TestValidateWorkspacePermissions_RootShared_WarnWithoutUsersManage(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
Expand All @@ -46,20 +43,78 @@ func TestValidateSharedRootPermissionsForSharedError(t *testing.T) {
Permissions: []resources.Permission{
{Level: CAN_MANAGE, UserName: "foo@bar.test"},
},
Resources: config.Resources{
Jobs: map[string]*resources.Job{
"job_1": {JobSettings: jobs.JobSettings{Name: "job_1"}},
"job_2": {JobSettings: jobs.JobSettings{Name: "job_2"}},
},
},
}
diags := applyValidate(t, b)
require.Len(t, diags, 1)
assert.Equal(t, diag.Warning, diags[0].Severity)
assert.Contains(t, diags[0].Summary, "root path")
assert.Contains(t, diags[0].Summary, "/Workspace/Shared/foo/bar")
}

func TestValidateWorkspacePermissions_StateShared_WarnWithoutUsersManage(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
RootPath: "/Workspace/Users/user@example.test",
StatePath: "/Workspace/Shared/state",
},
Permissions: []resources.Permission{
{Level: CAN_MANAGE, UserName: "user@example.test"},
},
},
}
diags := applyValidate(t, b)
require.Len(t, diags, 1)
assert.Equal(t, diag.Warning, diags[0].Severity)
assert.Contains(t, diags[0].Summary, "state path")
assert.Contains(t, diags[0].Summary, "/Workspace/Shared/state")
}

m := mocks.NewMockWorkspaceClient(t)
b.SetWorkpaceClient(m.WorkspaceClient)
func TestValidateWorkspacePermissions_StateShared_NoWarningWithUsersManage(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
RootPath: "/Workspace/Users/user@example.test",
StatePath: "/Workspace/Shared/state",
},
Permissions: []resources.Permission{
{Level: CAN_MANAGE, GroupName: "users"},
},
},
}
diags := applyValidate(t, b)
require.Empty(t, diags)
}

diags := bundle.Apply(t.Context(), b, ValidateSharedRootPermissions())
func TestValidateWorkspacePermissions_BothShared_OnlyOneWarning(t *testing.T) {
// root_path is also in Shared — the root warning covers the whole tree, so the
// state warning must not fire.
b := &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
RootPath: "/Workspace/Shared/root",
StatePath: "/Workspace/Shared/state",
},
Permissions: []resources.Permission{
{Level: CAN_MANAGE, UserName: "user@example.test"},
},
},
}
diags := applyValidate(t, b)
require.Len(t, diags, 1)
require.Equal(t, "the bundle root path /Workspace/Shared/foo/bar is writable by all workspace users", diags[0].Summary)
require.Equal(t, diag.Warning, diags[0].Severity)
assert.Contains(t, diags[0].Summary, "root path")
}

func TestValidateWorkspacePermissions_NoSharedPaths_NoWarning(t *testing.T) {
b := &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
RootPath: "/Workspace/Users/user@example.test/bundle",
StatePath: "/Workspace/Users/user@example.test/other-state",
},
},
}
diags := applyValidate(t, b)
require.Empty(t, diags)
}
2 changes: 1 addition & 1 deletion bundle/permissions/workspace_root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) {
WorkspaceObjectType: "directories",
}).Return(nil, nil)

diags := bundle.ApplySeq(t.Context(), b, ValidateSharedRootPermissions(), ApplyWorkspaceRootPermissions())
diags := bundle.ApplySeq(t.Context(), b, ValidateWorkspacePermissions(), ApplyWorkspaceRootPermissions())
require.Empty(t, diags)
}

Expand Down
8 changes: 4 additions & 4 deletions bundle/phases/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,10 @@ func Initialize(ctx context.Context, b *bundle.Bundle) {
apps.Validate(),

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

// Annotate resources with "deployment" metadata.
//
Expand Down
Loading