From 82c37e09906c2a07888555d2847725b930b9c36f Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 4 Jun 2026 13:50:02 +0200 Subject: [PATCH] bundle: add telemetry for workspace.state_path permission scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three fields to BundleDeployExperimental: - state_path_scope_exceeds_permissions: state_path is under /Workspace/Shared (all workspace users can read/write) but the permissions section does not grant group_name: users CAN_MANAGE — the effective scope exceeds the declared one. - state_path_is_shared: state_path is under /Workspace/Shared. - state_path_outside_root_path: state_path is not nested under root_path. The signals are computed in a small self-contained helper so this telemetry change is independent of the workspace permission validation mutators. Co-authored-by: Shreyas Goenka --- bundle/phases/telemetry.go | 70 ++++++++++++++++++----- bundle/phases/telemetry_workspace_test.go | 67 ++++++++++++++++++++++ libs/telemetry/protos/bundle_deploy.go | 15 +++++ 3 files changed, 138 insertions(+), 14 deletions(-) create mode 100644 bundle/phases/telemetry_workspace_test.go diff --git a/bundle/phases/telemetry.go b/bundle/phases/telemetry.go index e64b736fc72..eb6e2e16784 100644 --- a/bundle/phases/telemetry.go +++ b/bundle/phases/telemetry.go @@ -4,11 +4,13 @@ import ( "cmp" "context" "slices" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/bundle/metrics" + "github.com/databricks/cli/bundle/permissions" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/telemetry" @@ -37,6 +39,41 @@ func getExecutionTimes(b *bundle.Bundle) []protos.IntMapEntry { // Maximum length of the error message included in telemetry. const maxErrorMessageLength = 500 +// statePathScopeSignals computes telemetry describing how workspace.state_path's +// permission scope relates to the declared permissions. It is intentionally +// self-contained so the telemetry stays independent of the validation mutators. +// +// Returns: +// - isShared: state_path is under /Workspace/Shared (all workspace users get read/write). +// - outsideRoot: state_path is not nested under root_path (a separate permission scope). +// - scopeExceedsPermissions: state_path is shared but the permissions section does not +// grant group_name: users CAN_MANAGE, so the effective scope exceeds the declared one. +func statePathScopeSignals(b *bundle.Bundle) (isShared, outsideRoot, scopeExceedsPermissions bool) { + statePath := b.Config.Workspace.StatePath + rootPath := b.Config.Workspace.RootPath + + isShared = libraries.IsWorkspaceSharedPath(statePath) + + usersGroupCanManage := false + for _, p := range b.Config.Permissions { + if p.GroupName == "users" && p.Level == permissions.CAN_MANAGE { + usersGroupCanManage = true + break + } + } + scopeExceedsPermissions = isShared && !usersGroupCanManage + + if statePath != "" && rootPath != "" { + prefix := rootPath + if !strings.HasSuffix(prefix, "/") { + prefix += "/" + } + outsideRoot = !strings.HasPrefix(statePath, prefix) + } + + return isShared, outsideRoot, scopeExceedsPermissions +} + // LogDeployTelemetry logs a telemetry event for a bundle deploy command. func LogDeployTelemetry(ctx context.Context, b *bundle.Bundle, errMsg string) { errMsg = scrubForTelemetry(errMsg) @@ -177,6 +214,8 @@ func LogDeployTelemetry(ctx context.Context, b *bundle.Bundle, errMsg string) { experimentalConfig = &config.Experimental{} } + statePathIsShared, statePathOutsideRootPath, statePathScopeExceedsPermissions := statePathScopeSignals(b) + telemetry.Log(ctx, protos.DatabricksCliLog{ BundleDeployEvent: &protos.BundleDeployEvent{ BundleUuid: bundleUuid, @@ -203,20 +242,23 @@ func LogDeployTelemetry(ctx context.Context, b *bundle.Bundle, errMsg string) { ResourceDashboardIDs: dashboardIds, Experimental: &protos.BundleDeployExperimental{ - BundleMode: mode, - ConfigurationFileCount: b.Metrics.ConfigurationFileCount, - TargetCount: b.Metrics.TargetCount, - WorkspaceArtifactPathType: artifactPathType, - BoolValues: b.Metrics.BoolValues, - LocalCacheMeasurementsMs: b.Metrics.LocalCacheMeasurementsMs, - PythonAddedResourcesCount: b.Metrics.PythonAddedResourcesCount, - PythonUpdatedResourcesCount: b.Metrics.PythonUpdatedResourcesCount, - PythonResourceLoadersCount: int64(len(experimentalConfig.Python.Resources)), - PythonResourceMutatorsCount: int64(len(experimentalConfig.Python.Mutators)), - VariableCount: int64(variableCount), - ComplexVariableCount: complexVariableCount, - LookupVariableCount: lookupVariableCount, - BundleMutatorExecutionTimeMs: getExecutionTimes(b), + BundleMode: mode, + ConfigurationFileCount: b.Metrics.ConfigurationFileCount, + TargetCount: b.Metrics.TargetCount, + WorkspaceArtifactPathType: artifactPathType, + BoolValues: b.Metrics.BoolValues, + LocalCacheMeasurementsMs: b.Metrics.LocalCacheMeasurementsMs, + PythonAddedResourcesCount: b.Metrics.PythonAddedResourcesCount, + PythonUpdatedResourcesCount: b.Metrics.PythonUpdatedResourcesCount, + PythonResourceLoadersCount: int64(len(experimentalConfig.Python.Resources)), + PythonResourceMutatorsCount: int64(len(experimentalConfig.Python.Mutators)), + VariableCount: int64(variableCount), + ComplexVariableCount: complexVariableCount, + LookupVariableCount: lookupVariableCount, + BundleMutatorExecutionTimeMs: getExecutionTimes(b), + StatePathScopeExceedsPermissions: statePathScopeExceedsPermissions, + StatePathIsShared: statePathIsShared, + StatePathOutsideRootPath: statePathOutsideRootPath, }, }, }) diff --git a/bundle/phases/telemetry_workspace_test.go b/bundle/phases/telemetry_workspace_test.go new file mode 100644 index 00000000000..286cde53de9 --- /dev/null +++ b/bundle/phases/telemetry_workspace_test.go @@ -0,0 +1,67 @@ +package phases + +import ( + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/permissions" + "github.com/stretchr/testify/assert" +) + +func TestStatePathScopeSignals(t *testing.T) { + cases := []struct { + name string + rootPath string + statePath string + permissions []resources.Permission + isShared bool + outsideRoot bool + scopeExceedsPermissions bool + }{ + { + name: "state under root, not shared", + rootPath: "/Workspace/Users/me@example.test/bundle", + statePath: "/Workspace/Users/me@example.test/bundle/state", + }, + { + name: "state outside root, not shared", + rootPath: "/Workspace/Users/me@example.test/bundle", + statePath: "/Workspace/Users/me@example.test/other-state", + outsideRoot: true, + }, + { + name: "state shared without users manage", + rootPath: "/Workspace/Users/me@example.test/bundle", + statePath: "/Workspace/Shared/state", + isShared: true, + outsideRoot: true, + scopeExceedsPermissions: true, + }, + { + name: "state shared with users manage", + rootPath: "/Workspace/Users/me@example.test/bundle", + statePath: "/Workspace/Shared/state", + permissions: []resources.Permission{{Level: permissions.CAN_MANAGE, GroupName: "users"}}, + isShared: true, + outsideRoot: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + b := &bundle.Bundle{Config: config.Root{ + Workspace: config.Workspace{ + RootPath: tc.rootPath, + StatePath: tc.statePath, + }, + Permissions: tc.permissions, + }} + isShared, outsideRoot, scopeExceeds := statePathScopeSignals(b) + assert.Equal(t, tc.isShared, isShared, "isShared") + assert.Equal(t, tc.outsideRoot, outsideRoot, "outsideRoot") + assert.Equal(t, tc.scopeExceedsPermissions, scopeExceeds, "scopeExceedsPermissions") + }) + } +} diff --git a/libs/telemetry/protos/bundle_deploy.go b/libs/telemetry/protos/bundle_deploy.go index d9439437d9b..77e918e6357 100644 --- a/libs/telemetry/protos/bundle_deploy.go +++ b/libs/telemetry/protos/bundle_deploy.go @@ -86,6 +86,21 @@ type BundleDeployExperimental struct { // Local cache measurements in milliseconds (compute duration, potential savings, etc.) LocalCacheMeasurementsMs []IntMapEntry `json:"local_cache_measurements_ms,omitempty"` + + // Whether workspace.state_path grants broader access than the top-level + // permissions section declares. True when state_path is under /Workspace/Shared + // (all workspace users can read/write) but the permissions section does not + // grant group_name: users CAN_MANAGE. + StatePathScopeExceedsPermissions bool `json:"state_path_scope_exceeds_permissions,omitempty"` + + // Whether workspace.state_path is under /Workspace/Shared, making deployment + // state readable and writable by all workspace users. + StatePathIsShared bool `json:"state_path_is_shared,omitempty"` + + // Whether workspace.state_path is not nested under workspace.root_path, + // meaning it may carry a different (potentially broader) permission scope + // than the rest of the bundle. + StatePathOutsideRootPath bool `json:"state_path_outside_root_path,omitempty"` } type BoolMapEntry struct {