feat(permissions-analysis): workspace identity changes detection + report robustness fixes - #387
Open
andyweaves wants to merge 9 commits into
Open
andyweaves wants to merge 9 commits into
andyweaves wants to merge 9 commits into
Conversation
…er AIM groups App report endpoints (shared-to-account, privileged-non-idp, denylist-candidates) selected all rows with no LIMIT, and exec_query_df read only the first inline result chunk — so a large account could overflow the ~25 MB inline Statement Execution cap and surface as a false "no findings" in a security tool. Now: - exec_query_df follows result chunks so multi-chunk results aren't truncated - the three report queries are bounded (LIMIT REPORT_ROW_CAP + 1) and the endpoints flag truncation; a shared UI banner shows "showing first N rows" - the endpoints re-raise NoAccessError so a missing UC read grant renders the access banner instead of a misleading "run the job" hint Notebook 07 (denylist candidates) emitted zero-member IdP groups as candidates, but under Automatic Identity Management a populated external group returns empty members via SCIM — indistinguishable from a genuinely empty group — so active groups could be flagged "safe to deny". Zero-member groups are now excluded by default behind an opt-in include_zero_member_groups widget. Co-authored-by: Isaac
…nges New notebook 08_workspace_identity_changes.py + app tab + job wiring that reads system.access.audit to flag two governance gaps under Automatic Identity Management, distinguishing human actions from the AIM sync process via the request_params.endpoint = 'autoUserCreation' tag: - Ungoverned identity changes: a human (not the AIM sync) creates/changes a user/group/SP, adds a group member, or grants account admin — identities that should originate from the IdP (add, createGroup, addPrincipalToGroup(s), setAdmin). - Non-IdP workspace assignments: a principal without an externalId assigned to a workspace (updatePermissionAssignment / changeDatabricksWorkspaceAcl). Assigning IdP-managed identities to workspaces is the expected, governed path and is recorded but not flagged. Each event's principal is enriched from account SCIM (type / name / IdP-managed). Opt-in remediation removes flagged non-IdP workspace assignments via the account-scoped permissionassignments DELETE (removes workspace access only, account identity untouched) — reusing notebook 06's cross-workspace-safe pattern. Ungoverned identity creations are report-only (removal is left manual). Findings land in brickhound_workspace_identity_changes (run_id snapshots). The new read-only "Workspace Identity Changes" app tab renders flagged-first with the bounded-query + truncation-banner + NoAccessError handling used by the other report tabs. Weekly job (Sun 6 AM) added to terraform/common and the DABS template, remediate=no by default. All audit action names and request_params keys were verified against a live system.access.audit table before writing the queries. Co-authored-by: Isaac
…" + deep-link flagged principals
- Drop the ampersand: "Workspace & Identity Changes" -> "Workspace Identity
Changes" across the notebook title, app tab (title/desc/loader/header/error),
and the Terraform + DABS job names.
- notebook 08 console_url now deep-links to the specific principal's
account-console detail page (/user-management/{segment}/{id}?account_id=...),
matching notebooks 06/07, instead of the generic section landing page. The
app renders this value, so flagged rows now link straight to the user/group/SP.
Co-authored-by: Isaac
…ntity change rows Each flagged/recorded row in the Workspace Identity Changes tab now carries an explicit principal-type chip (👤 User / 👥 Group / 🤖 Service Principal / ❔ Unknown) next to the principal name, so an "Identity created" finding shows at a glance whether the created identity is a user or a service principal. Co-authored-by: Isaac
…on Workspace Identity Changes tab Workspace Identity Changes is an account-level detection page with its own run_id and per-report coverage block, like the other 05/06/07 tabs — it should not show the global graph-collection "Data Collection Date & Time" run selector (which switches BrickHound graph runs, irrelevant here). Add 'workspaceidentity' to hideStatsBarPages so it matches the other account-level tabs and avoids showing two conflicting coverage widgets. Co-authored-by: Isaac
…ector, and remediated filter
Notebook 08 (Workspace Identity Changes):
- New opt-in `disable_identities` widget (default no): deactivates flagged Users /
Service Principals added outside the AIM sync — either created in the account
(identity_created) or assigned to a workspace as non-IdP — via account SCIM
PATCH active=false (reversible; groups excluded, SCIM active doesn't apply).
Independent of the existing `remediate` (assignment-removal) widget.
- New `remediation_action` column records what was done per finding
(assignment_removed and/or identity_disabled); auto_remediated now means "any
action applied". Job definitions (TF + DABS) pass disable_identities=no.
App (Shared to Account, Privileged Non-IdP, Workspace Identity Changes):
- Per-run history selector: each report endpoint accepts ?run_id= (validated) and
returns the recent run list; a dropdown lets you view past detection runs
(default latest). Denylist unchanged.
- Remediated filter (All / Remediated / Not remediated): client-side toggle over
the rendered rows, hiding empty groups.
- Workspace Identity Changes rows show the specific remediation_action
("Assignment removed" / "Identity disabled") instead of a generic "Remediated".
Co-authored-by: Isaac
…he global header filters Render the account-tab run selector + remediated filter as stats-header-section cards with small uppercase labels and a divider (the same classes the global graph-collection header uses), instead of a plain inline label+select row. Co-authored-by: Isaac
…d Recorded section, refresh docs - Notebook 08: drop audit events with a blank principal_id. Creating an SP or group emits a secondary event whose id lands in a different field (a second scim `add` with empty targetUserId, or a createGroup with endpoint=permissionAssignment carrying the id in targetUserId), leaving our extracted principal_id empty — a duplicate of the real creation event that surfaced as spurious "Unknown type" rows. Dropping them removes the noise without losing any genuine finding (real creation events keep their id and resolve). Genuinely unresolvable ids (deleted/old identities) still show Unknown. - App: rename the Workspace Identity Changes "Recorded — governed baseline" section to "Recorded — review discretionary" with a note that it holds non-flagged context (governed baseline + removals, including this tool's own assignment removals). - Docs: brickhound README lists all four account-level detection tables and the 08 disable_identities remediation + app run selector/filter; CLAUDE.md storage section lists the account-level detection tables. Co-authored-by: Isaac
…workspace identity changes Deploying a Databricks App auto-creates a service principal via the audit `endpoint=DatabricksApps` path. These are platform-managed, not ungoverned human onboarding — flagging them as findings (and, with disable_identities=yes, deactivating them) would take down live apps. Drop `endpoint=DatabricksApps` events from detection entirely, so app SPs never appear in the report, are never flagged, and are never remediated. Also removes an intermittent "Unknown type" row (the app-SP creation event). Docs updated. Co-authored-by: Isaac
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
1 participant
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
Adds a new BrickHound / Permissions Analysis detection+remediation surface, plus a robustness fix to the existing report tabs (bundled as a separate documented commit).
New feature — Workspace & Identity Changes (
08)Motivated by an AIM governance concern: a workspace admin can pull Entra identities into a workspace, and identities can be created outside the IdP sync. New
notebooks/brickhound/08_workspace_identity_changes.pyreadssystem.access.auditand flags, distinguishing human actions from the AIM sync via therequest_params.endpoint = 'autoUserCreation'tag:add/createGroup/addPrincipalToGroup(s)/setAdmin.updatePermissionAssignment(workspace admin) /changeDatabricksWorkspaceAcl(account admin) where the assigned principal has noexternalId. Assigning IdP-managed identities is the governed baseline — recorded, not flagged.Each principal is enriched from account SCIM (type / name / IdP-managed). Opt-in remediation removes flagged non-IdP workspace assignments via the account-scoped
permissionassignmentsDELETE (workspace access only; account identity untouched), reusing notebook 06's cross-workspace-safe pattern. Findings land inbrickhound_workspace_identity_changes(run_id snapshots) and render in a new read-only Workspace Identity Changes app tab. Weekly job (Sun 6 AM) added toterraform/commonand the DABS template,remediate=noby default.All audit
action_names andrequest_paramskeys were verified against a livesystem.access.audittable before writing the queries.Bundled fix (commit
4913d9d) — report robustnessexec_query_dfnow follows result chunks (no silent truncation of multi-chunk results).LIMIT REPORT_ROW_CAP + 1) with a truncation banner, so a large account can't overflow the ~25 MB inline result and show a false "no findings".NoAccessErrorso a missing UC grant shows the access banner.07excludes zero-member IdP groups by default (opt-ininclude_zero_member_groups) — they are AIM false positives.Testing (live, on an AWS workspace)
Unknownfallback for a since-deleted principal).07now returns 0 zero-member false positives.This pull request and its description were written by Isaac.