bundle: warn when state_path uses /Workspace/Shared and add telemetry for state path scope#5428
Draft
shreyas-goenka wants to merge 5 commits into
Draft
bundle: warn when state_path uses /Workspace/Shared and add telemetry for state path scope#5428shreyas-goenka wants to merge 5 commits into
shreyas-goenka wants to merge 5 commits into
Conversation
… for state path scope Extends ValidateSharedRootPermissions to also warn when workspace.state_path is under /Workspace/Shared but workspace.root_path is not, since the existing root-path warning would not cover that case. Adds two telemetry fields to BundleDeployExperimental: - state_path_is_shared: state_path is under /Workspace/Shared - state_path_outside_root_path: state_path is not nested under root_path, meaning it may carry a broader permission scope than the rest of the bundle. Co-authored-by: Shreyas Goenka <shreyas.goenka@databricks.com>
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
…cePermissions Replaces ValidateSharedRootPermissions with a single, clearly documented ValidateWorkspacePermissions mutator that covers three distinct static checks: 1. workspace.root_path in /Workspace/Shared without users CAN_MANAGE 2. workspace.state_path in /Workspace/Shared (when root_path is not) without users CAN_MANAGE — deployment state separately exposed to all workspace users 3. workspace.state_path outside workspace.root_path — state stored in a separate folder with an independently managed permission scope Cases 2 and 3 suppress each other when they would produce redundant warnings for the same misconfiguration (state_path in Shared and outside root_path). Co-authored-by: Shreyas Goenka <shreyas.goenka@databricks.com>
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
…red permissions The original goal was to statically detect when workspace.state_path grants broader access than the top-level permissions section declares. The prior fields captured raw facts (path is shared, path is outside root) but not this mismatch signal itself. Reworks validation so each warning is backed by a named, reusable predicate, and emits those predicates as telemetry: - StatePathScopeExceedsPermissions: state_path is in /Workspace/Shared (all users can read/write) but permissions do not grant users CAN_MANAGE — the effective scope exceeds the declared one. This is the new telemetry field. - RootPathScopeExceedsPermissions: same check for root_path, now backing the existing root warning. Co-authored-by: Shreyas Goenka <shreyas.goenka@databricks.com>
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
…y only Storing workspace.state_path outside workspace.root_path is a valid configuration, not a misconfiguration, so it should not produce a warning. ValidateWorkspacePermissions now warns only when a path's effective access exceeds the declared permissions (cases 1 and 2). The StatePathOutsideRootPath predicate and its telemetry field remain as an informational signal. Co-authored-by: Shreyas Goenka <shreyas.goenka@databricks.com>
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
|
Commit: e7e9e83 |
…he bundle's ValidateFolderPermissions already compares the live workspace ACL against the declared permissions, but it only runs during `bundle validate`. This brings the same check to `bundle deploy` without adding any API latency: ApplyWorkspaceRoot- Permissions already calls SetPermissions on each workspace path prefix (root_path and, when separate, state_path), and the response carries the resulting ACL. Reusing that response, we compare against the declared permissions. Because the Set replaces the folder's direct ACL with the declared set, any principal still showing higher access is inherited from a parent folder — the broader access that actually persists after deploy, which is the scope mismatch worth surfacing. No extra GetPermissions round trip is made. The check is skipped for /Workspace/ Shared paths, consistent with the existing behavior. Co-authored-by: Shreyas Goenka <shreyas.goenka@databricks.com>
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Detects when a workspace path grants broader access than the bundle's top-level
permissionssection declares, and surfaces it in bothvalidateanddeploy— plus telemetry for the static signals.Two kinds of detection
Static (config-only), in
ValidateWorkspacePermissions— warns when a path is in/Workspace/Shared(so all workspace users can read/write) but the permissions section doesn't grantusers: CAN_MANAGE:root_pathscope exceeds declared permissionsstate_pathscope exceeds declared permissions (suppressed when root_path is also shared)Storing
state_pathoutsideroot_pathis informational only (recorded as telemetry, no warning).Live ACL comparison (any path, not just
/Shared) — compares the actual workspace folder ACL against the declared permissions viaWorkspacePathPermissions.Compare, warning when the folder grants a principal more access than the bundle declares:bundle validate: already done byValidateFolderPermissions(issues aGetPermissionscall per path).bundle deploy(new): done without adding latency.ApplyWorkspaceRootPermissionsalready callsSetPermissionson each path prefix (root_path and, when separate, state_path); we reuse that response's ACL for the comparison instead of issuing a separateGetPermissions. Because the Set replaces the direct ACL with the declared set, any principal still showing higher access is inherited from a parent folder — the broader access that persists after deploy.Telemetry (
BundleDeployExperimental)state_path_scope_exceeds_permissions— state_path's effective access is broader than declaredstate_path_is_shared— state_path uses/Workspace/Sharedstate_path_outside_root_path— state_path is outside root_path (informational)