Skip to content

Conversation

@martinsohn
Copy link
Contributor

@martinsohn martinsohn commented Oct 7, 2025

Description

  • Remove highPrivilegedRoleDisplayNameRegex AZRole clause from ´Shortest paths from Entra Users to Tier Zero / High Value targets` as the query is targeting Tier Zero, not just privileged roles that are Tier Zero
  • Include AZRole 'Privileged Role Administrator' to highPrivilegedRoleDisplayNameRegex as the is highly privileged / Tier Zero

Motivation and Context

Resolves BED-6591

Why is this change required? What problem does it solve?

Originates from an issue on the BloodHound Query Library repo: SpecterOps/BloodHoundQueryLibrary#31

How Has This Been Tested?

Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc.

Built BHCE and ran all three affected queries with Entra sample data and confirmed they return the correct data.

Screenshots (optional):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

Summary by CodeRabbit

  • New Features
    • High-privileged role detection now includes the Privileged Role Administrator role, improving identification of elevated accounts.
    • Shortest-path searches to Tier Zero / High Value targets return broader, more comprehensive results by removing role-name filtering and relying on tier tags and basic path constraints.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 7, 2025

Walkthrough

Expanded the highPrivilegedRoleDisplayNameRegex to include "Privileged Role Administrator" and removed name-based role filtering from shortest-path queries to Tier Zero / High Value targets in both AGI and AGT common searches; exported/public signatures unchanged.

Changes

Cohort / File(s) Summary of Changes
Regex and shortest-path query updates
packages/javascript/bh-shared-ui/src/commonSearchesAGI.ts, packages/javascript/bh-shared-ui/src/commonSearchesAGT.ts
- Added Privileged Role Administrator to highPrivilegedRoleDisplayNameRegex.
- Removed name-based privilege filtering from the "Shortest paths from Entra Users to Tier Zero / High Value targets" queries; queries now rely on tier tags and s <> t only.
- No changes to exported/public entity signatures.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant UI as Shared UI
    participant Search as Common Searches (AGI/AGT)
    participant DB as Graph DB (Cypher)

    Note over UI,Search: Trigger shortest-path / privileged-role searches
    UI->>Search: requestShortestPaths(tier=TierZero/HighValue)
    Search->>DB: MATCH shortestPath... WHERE t.tier = "TierZero" AND s <> t
    Note right of DB: Role-name filter removed\nhighPrivilegedRoleDisplayNameRegex updated
    DB-->>Search: return paths
    Search-->>UI: results
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • BED 5606 - PIM Small Tasks #1589 — Modifies privileged-role handling and adds PIM/AZRole eligible/approver edges; overlaps with regex and role-filtering changes in commonSearchesAGI/AGT.

Suggested labels

bug, user interface

Suggested reviewers

  • neumachen
  • mvlipka
  • urangel

Poem

A rabbit tweaks regex with a hop and a grin,
Drops name-filters gently so paths may run in.
One role more welcomed into the fold,
Queries breathe freer — boldness behold!
🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely summarizes the primary change by referencing the ticket and describing the query fix for Entra privileged roles without unnecessary details.
Description Check ✅ Passed The description follows the repository template with populated Description, Motivation and Context, How Has This Been Tested, Types of changes, and Checklist sections, providing clear details on the changes, their rationale, and the testing performed even though some template prompts remain.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch BED-6591

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 784cea9 and d92d66a.

📒 Files selected for processing (2)
  • packages/javascript/bh-shared-ui/src/commonSearchesAGI.ts (2 hunks)
  • packages/javascript/bh-shared-ui/src/commonSearchesAGT.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/javascript/bh-shared-ui/src/commonSearchesAGT.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/javascript/bh-shared-ui/src/commonSearchesAGI.ts (1)
packages/javascript/bh-shared-ui/src/constants.ts (1)
  • TIER_ZERO_TAG (42-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Build BloodHound Container Image / Build and Package Container
  • GitHub Check: build-ui
  • GitHub Check: run-tests
  • GitHub Check: run-analysis
  • GitHub Check: run-tests
🔇 Additional comments (2)
packages/javascript/bh-shared-ui/src/commonSearchesAGI.ts (2)

27-28: LGTM! Privileged Role Administrator added correctly.

The addition of "Privileged Role Administrator" to the high-privileged roles regex is correctly formatted and aligns with the PR objective to include this Tier Zero role.


397-399: LGTM! highPrivilegedRoleDisplayNameRegex is identical in both AGI and AGT, and the shortestPath query in each file now removes the privileged-role filter and uses its respective Tier Zero tag constant.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@martinsohn martinsohn self-assigned this Oct 7, 2025
Copy link
Contributor

@maffkipp maffkipp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great!

@irshadaj irshadaj merged commit 53b12ec into main Oct 15, 2025
9 checks passed
@irshadaj irshadaj deleted the BED-6591 branch October 15, 2025 17:54
@github-actions github-actions bot locked and limited conversation to collaborators Oct 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants