bundle: warn when a workspace path is in /Workspace/Shared without users CAN_MANAGE

bundle/permissions/validate.go

@@ -3,49 +3,86 @@ package permissions
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/libraries"
 	"github.com/databricks/cli/libs/diag"
 )
 
-type validateSharedRootPermissions struct{}
+type validateWorkspaceSharedPermissions struct{}
 
-func ValidateSharedRootPermissions() bundle.Mutator {
-	return &validateSharedRootPermissions{}
+// ValidateWorkspaceSharedPermissions 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 ValidateWorkspaceSharedPermissions() bundle.Mutator {
+	return &validateWorkspaceSharedPermissions{}
 }
 
-func (*validateSharedRootPermissions) Name() string {
-	return "ValidateSharedRootPermissions"
+func (*validateWorkspaceSharedPermissions) Name() string {
+	return "ValidateWorkspaceSharedPermissions"
 }
 
-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 (*validateWorkspaceSharedPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	var diags diag.Diagnostics
 
-	allUsers := false
+	rootPath := b.Config.Workspace.RootPath
+	statePath := b.Config.Workspace.StatePath
+	rootIsShared := libraries.IsWorkspaceSharedPath(rootPath)
+
+	// Whether the top-level permissions grant group_name: users CAN_MANAGE, i.e.
+	// the broad /Workspace/Shared access is intentional and declared.
+	usersCanManage := false
 	for _, p := range b.Config.Permissions {
 		if p.GroupName == "users" && p.Level == CAN_MANAGE {
-			allUsers = true
+			usersCanManage = true
 			break
 		}
 	}
 
-	if !allUsers {
+	// 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", 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 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>.",
+		})
+	}
+
+	// state_path is in /Workspace/Shared without users CAN_MANAGE. Skip only when
+	// state_path is nested under root_path, since the root warning above already
+	// covers it. When state_path is a separate folder, warn about it on its own.
+	if libraries.IsWorkspaceSharedPath(statePath) && !statePathUnderRootPath(rootPath, statePath) && !usersCanManage {
+		diags = diags.Append(diag.Diagnostic{
+			Severity: diag.Warning,
+			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
 }
+
+// statePathUnderRootPath returns true when statePath is nested under rootPath, in
+// which case permissions applied to root_path also cover the state directory.
+//
+// By default state_path lives under root_path (it defaults to "${root_path}/state"),
+// so we treat it as nested unless both paths are set and root_path is genuinely not a
+// prefix of state_path. That keeps us from emitting a separate state warning for the
+// common case.
+//
+// Both paths are /Workspace-normalized by PrependWorkspacePrefix before this mutator
+// runs, so the prefix comparison here is reliable.
+func statePathUnderRootPath(rootPath, statePath string) bool {
+	if rootPath == "" || statePath == "" {
+		return true
+	}
+	if statePath == rootPath {
+		return true
+	}
+	if !strings.HasSuffix(rootPath, "/") {
+		rootPath += "/"
+	}
+	return strings.HasPrefix(statePath, rootPath)
+}

bundle/permissions/validate_test.go

@@ -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, ValidateWorkspaceSharedPermissions())
+}
+
+func TestValidateWorkspaceSharedPermissions_RootShared_NoWarningWithUsersManage(t *testing.T) {
 	b := &bundle.Bundle{
 		Config: config.Root{
 			Workspace: config.Workspace{
@@ -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 TestValidateWorkspaceSharedPermissions_RootShared_WarnWithoutUsersManage(t *testing.T) {
 	b := &bundle.Bundle{
 		Config: config.Root{
 			Workspace: config.Workspace{
@@ -46,20 +43,119 @@ 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 TestValidateWorkspaceSharedPermissions_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 TestValidateWorkspaceSharedPermissions_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 TestValidateWorkspaceSharedPermissions_StateUnderSharedRoot_OnlyRootWarning(t *testing.T) {
+	// state_path is nested under root_path — the root warning already covers it,
+	// so the state warning must not fire.
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				RootPath:  "/Workspace/Shared/bundle",
+				StatePath: "/Workspace/Shared/bundle/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 TestValidateWorkspaceSharedPermissions_BothSharedSeparateFolders_TwoWarnings(t *testing.T) {
+	// root_path and state_path are both shared but state_path is not under root_path,
+	// so both warnings fire.
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Workspace: config.Workspace{
+				RootPath:  "/Workspace/Shared/bundle",
+				StatePath: "/Workspace/Shared/separate-state",
+			},
+			Permissions: []resources.Permission{
+				{Level: CAN_MANAGE, UserName: "user@example.test"},
+			},
+		},
+	}
+	diags := applyValidate(t, b)
+	require.Len(t, diags, 2)
+	assert.Contains(t, diags[0].Summary, "root path")
+	assert.Contains(t, diags[1].Summary, "state path")
+}
+
+func TestValidateWorkspaceSharedPermissions_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)
+}
+
+func TestStatePathUnderRootPath(t *testing.T) {
+	cases := []struct {
+		name      string
+		rootPath  string
+		statePath string
+		want      bool
+	}{
+		{name: "default nested state", rootPath: "/Workspace/Users/me/bundle", statePath: "/Workspace/Users/me/bundle/state", want: true},
+		{name: "equal paths", rootPath: "/Workspace/Users/me/bundle", statePath: "/Workspace/Users/me/bundle", want: true},
+		{name: "separate folder", rootPath: "/Workspace/Users/me/bundle", statePath: "/Workspace/Shared/state", want: false},
+		{name: "sibling prefix is not nested", rootPath: "/Workspace/Users/me/bundle", statePath: "/Workspace/Users/me/bundle-2/state", want: false},
+		{name: "empty state defaults to nested", rootPath: "/Workspace/Users/me/bundle", statePath: "", want: true},
+		{name: "empty root defaults to nested", rootPath: "", statePath: "/Workspace/Shared/state", want: true},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.want, statePathUnderRootPath(tc.rootPath, tc.statePath))
+		})
+	}
 }

bundle/permissions/workspace_root_test.go

@@ -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, ValidateWorkspaceSharedPermissions(), ApplyWorkspaceRootPermissions())
 	require.Empty(t, diags)
 }
 

bundle/phases/initialize.go

@@ -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.ValidateWorkspaceSharedPermissions(),
 
 		// Annotate resources with "deployment" metadata.
 		//