feat: Add visualisation of Active Directory site data #2031
Conversation
* Add Site, SiteSubnet and SiteServer nodes * Add GenericAll, GenericWrite and WriteGPLink edges on Site objects * Add gPLink edges from GPOs to site objects * Add Site objects to default high value targets
WalkthroughThis PR implements comprehensive support for Active Directory Sites, SiteServers, and SiteSubnets across the full stack—graph schema, data models, backend APIs, frontend UI, data quality metrics, entity endpoints, GPO affiliation paths, and abuse documentation—enabling representation and analysis of site-based objects and ACLs. Changes
Sequence DiagramsequenceDiagram
participant Collector as AD Collector
participant Ingest as Ingest Service
participant Graph as Graph DB
participant API as API Backend
participant UI as Frontend UI
Collector->>Ingest: Send Site/SiteServer/SiteSubnet data
Ingest->>Ingest: convertSiteData, convertSiteServerData, convertSiteSubnetData
Ingest->>Graph: Create nodes + parse ACE/container relationships
Ingest->>Graph: Record statistics (Sites, SiteServers, SiteSubnets)
UI->>API: GET /api/v2/sites/{id}
API->>Graph: FetchSiteEntityInfo + SelectSitesCandidateFilter
API->>UI: Return Site entity + controllers count
UI->>API: GET /api/v2/gpos/{id}/sites
API->>Graph: FetchGPOAffectedSitePaths
API->>UI: Return paths terminating at Site nodes
UI->>API: GET /api/v2/sites/{id}/siteservers
API->>Graph: CreateSiteContainedPathDelegate(ad.SiteServer)
API->>UI: Return outbound Contains relationships
UI->>UI: Render Site node with icons + abuse documentation
UI->>UI: Display data quality metrics (sites, siteservers, sitesubnets)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring additional attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (11)
cmd/ui/src/ducks/graph/graphutils.ts (1)
239-241: LGTM! Consider differentiating icons for better visual distinction.The icon mappings are correctly implemented and maintain type safety. All three new Site-related node types currently share the 'fa-clipboard-check' icon, which is also used by IssuancePolicy. While this may be intentional for visual grouping, consider using more semantically distinct icons to improve user experience:
- Site: 'fa-map-marker-alt' or 'fa-building' (represents physical location)
- SiteServer: 'fa-server' (if not conflicting) or 'fa-hdd' (represents server infrastructure)
- SiteSubnet: 'fa-network-wired' or 'fa-project-diagram' (represents network topology)
packages/javascript/bh-shared-ui/src/components/HelpTexts/GPLink/LinuxAbuse.tsx (2)
32-39: Fix indentation consistency.Line 32 has extra leading whitespace before the opening
<Typography>tag, which is inconsistent with the rest of the file's indentation.Apply this diff:
- <Typography variant='body2'> + <Typography variant='body2'>
66-71: Remove extra whitespace around tool name.Line 68 has inconsistent spacing around
pyGPOAbuse.pywithin the Link tags, with extra space after the closing tag.Apply this diff:
- Alternatively, <Link target='_blank' rel='noopener noreferrer' href='https://github.com/Hackndo/pyGPOAbuse'> - pyGPOAbuse.py - </Link>{' '} - can be used for that purpose. + Alternatively, <Link target='_blank' rel='noopener noreferrer' href='https://github.com/Hackndo/pyGPOAbuse'> + pyGPOAbuse.py + </Link>{' '} + can be used for that purpose.packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/WindowsAbuse.tsx (1)
22-132: Consider refactoring to reduce code duplication.The three cases (Domain, OU, Site) share 95%+ identical content with only minor text variations. This duplication makes maintenance harder and increases the risk of inconsistencies (like the issue flagged in Line 86).
Consider extracting a helper function that accepts the target type and returns the appropriate content with target-specific strings interpolated.
Example approach:
const getAbuseContent = (targetType: string) => { const targetLabel = targetType === 'OU' ? 'OU' : targetType.toLowerCase(); const targetObject = `${targetType}${targetType === 'OU' ? '' : ' object'}`; return { targetLabel, targetObject, }; };Then use these values in a single template rather than duplicating the entire structure three times.
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/LinuxAbuse.tsx (1)
22-187: Consider refactoring to reduce code duplication.Similar to WindowsAbuse.tsx, the three cases share 95%+ identical content with only minor variations in tool parameters and text. This duplication increases maintenance burden and the risk of inconsistencies.
Consider extracting a helper function that generates the appropriate content based on target type, with target-specific parameters and labels.
Example parameters that vary:
- Target label: "domain" vs "OU" vs "site"
- Target object reference: "domain object", "OU", "Site"
- Example DN for the
-oparameter in the link commandThese could be parameterized in a single content template instead of three separate case blocks.
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/LinuxAbuse.tsx (1)
700-763: Clarify the Server objects reference.The documentation for the Site case is comprehensive and technically sound. However, Line 710 contains unclear phrasing: "Note that Server objects associated with the Site should be located in the Site."
The phrase "should be located in the Site" is ambiguous. Consider revising to clarify what this means—for example, if you're indicating that domain controllers (Server objects) associated with a Site should be considered as affected targets, make this explicit.
Additionally, Line 754's example DN (
CN=Default-First-Site-Name,...) is specific to the default site. Consider adding a brief note that users should substitute their actual site name.Consider revising Line 710 for clarity:
- Note that Server objects associated with the Site should be located in the Site. + Note that Server objects (such as domain controllers) associated with the Site will be affected by GPOs linked to that Site.And optionally add clarification for Line 754:
<Typography component={'pre'}> { - 'python3 gpb.py links link -d "corp.com" --dc "dc.corp.com" -u "user" -p "password" -o "CN=Default-First-Site-Name,CN=Sites,CN=Configuration,DC=corp,DC=com" -n "TARGETGPO"' + '# Replace Default-First-Site-Name with your actual site name\npython3 gpb.py links link -d "corp.com" --dc "dc.corp.com" -u "user" -p "password" -o "CN=Default-First-Site-Name,CN=Sites,CN=Configuration,DC=corp,DC=com" -n "TARGETGPO"' } </Typography>cmd/api/src/api/bloodhoundgraph/bloodhoundgraph.go (1)
261-272: Consider adding background colors for new node types.The icon mappings are correctly added for Site, SiteServer, and SiteSubnet. However, the
SetBackground()function (starting at line 290) doesn't include cases for these new node types, so they'll receive the default color"#EEE".Consider adding explicit color cases in
SetBackground()to ensure consistent visual representation with other AD node types.packages/javascript/bh-shared-ui/src/views/DataQuality/DomainInfo.tsx (1)
57-59: Minor inconsistency in displayText for SiteSubnets.Line 59 uses the singular form
'SiteSubnet'for displayText, while lines 57-58 use plural forms ('Sites','SiteServers'). For consistency with the majority of other entries in the DomainMap (e.g., 'Users', 'Groups', 'Computers', 'GPOs'), consider changing it to'SiteSubnets'.- sitesubnets: { displayText: 'SiteSubnet', kind: ActiveDirectoryNodeKind.SiteSubnet }, + sitesubnets: { displayText: 'SiteSubnets', kind: ActiveDirectoryNodeKind.SiteSubnet },cmd/api/src/api/v2/ad_related_entity.go (1)
208-210: GPO→Sites list delegate will page per-link, not across the aggregateUsing CreateGPOAffectedIntermediariesListDelegate with SelectSitesCandidateFilter returns only the Site end node per GPLink; skip/limit apply inside each per-link traversal. Consider a small, specialized list delegate that collects distinct end Sites across all GPLinks and applies paging once at the end for deterministic results (mirrors your bespoke FetchGPOAffectedSitePaths). I can draft this helper if wanted.
packages/go/analysis/ad/queries.go (1)
730-747: DRY opportunity: unify OU/Site contained delegatesCreateSiteContained{List,Path}Delegate duplicate the OU versions except for the function name. Consider a single CreateContained{List,Path}Delegate(kind) used by both callers to reduce surface area. No behavior change.
Also applies to: 749-764
packages/javascript/js-client-library/src/client.ts (1)
1850-1863: Route parity verified with backendAll requested endpoints are registered in the API layer:
/api/v2/gpos/{id}/sites✓/api/v2/sites/{id}/siteservers✓/api/v2/sites/{id}/sitesubnets✓/api/v2/siteservers/{id}✓/api/v2/siteservers/{id}/controllers✓/api/v2/sitesubnets/{id}✓/api/v2/sitesubnets/{id}/controllers✓Optionally, consider a small utility function for the recurring
{skip, limit, type}param objects to reduce repetition across the Site, SiteServer, and SiteSubnet methods.Also applies to: 2634-2744
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
cmd/api/src/analysis/ad/queries.go(1 hunks)cmd/api/src/api/bloodhoundgraph/bloodhoundgraph.go(1 hunks)cmd/api/src/api/registration/v2.go(2 hunks)cmd/api/src/api/v2/ad_entity.go(2 hunks)cmd/api/src/api/v2/ad_related_entity.go(2 hunks)cmd/api/src/database/migration/migrations/v8.4.0.sql(1 hunks)cmd/api/src/model/adquality.go(2 hunks)cmd/api/src/model/ingest/ingest.go(3 hunks)cmd/api/src/services/graphify/convertors.go(1 hunks)cmd/api/src/services/graphify/ingest.go(1 hunks)cmd/ui/src/ducks/graph/graphutils.ts(1 hunks)cmd/ui/src/ducks/graph/types.ts(1 hunks)packages/cue/bh/ad/ad.cue(2 hunks)packages/go/analysis/ad/filters.go(1 hunks)packages/go/analysis/ad/queries.go(6 hunks)packages/go/ein/incoming_models.go(1 hunks)packages/go/graphschema/ad/ad.go(3 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GPLink/LinuxAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GPLink/WindowsAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/LinuxAbuse.tsx(5 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/WindowsAbuse.tsx(5 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/LinuxAbuse.tsx(6 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/WindowsAbuse.tsx(5 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/General.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/LinuxAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/WindowsAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/graphSchema.ts(2 hunks)packages/javascript/bh-shared-ui/src/utils/content.ts(5 hunks)packages/javascript/bh-shared-ui/src/utils/icons.ts(2 hunks)packages/javascript/bh-shared-ui/src/views/DataQuality/DomainInfo.tsx(1 hunks)packages/javascript/js-client-library/src/client.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-06T20:45:32.705Z
Learnt from: JimSycurity
Repo: SpecterOps/BloodHound PR: 1761
File: packages/javascript/bh-shared-ui/src/components/HelpTexts/ProtectAdminGroups/References.tsx:22-43
Timestamp: 2025-08-06T20:45:32.705Z
Learning: The SpecterOps URL https://specterops.io/resources/adminsdholder in the ProtectAdminGroups References.tsx component is a placeholder link created by marketing that will become active when JimSycurity's AdminSDHolder whitepaper is published. This is intentional and should not be flagged as a broken link.
Applied to files:
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/References.tsxpackages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/General.tsxpackages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/LinuxAbuse.tsxpackages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/LinuxAbuse.tsxpackages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/References.tsxpackages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/References.tsx
📚 Learning: 2025-06-18T08:27:18.317Z
Learnt from: JonasBK
Repo: SpecterOps/BloodHound PR: 1434
File: packages/go/analysis/ad/gpos.go:102-126
Timestamp: 2025-06-18T08:27:18.317Z
Learning: In Active Directory's containment hierarchy, each user/computer has exactly one direct parent container, forming a tree structure. When processing GPO edges in packages/go/analysis/ad/gpos.go, the fetchDirectChildUsersAndComputers function only returns direct children, ensuring each user/computer is processed exactly once by its immediate parent container, eliminating the need for deduplication logic.
Applied to files:
cmd/api/src/api/v2/ad_entity.gopackages/go/analysis/ad/queries.go
📚 Learning: 2025-06-06T23:12:14.181Z
Learnt from: elikmiller
Repo: SpecterOps/BloodHound PR: 1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Applied to files:
packages/cue/bh/ad/ad.cue
📚 Learning: 2025-08-06T20:42:40.256Z
Learnt from: JimSycurity
Repo: SpecterOps/BloodHound PR: 1761
File: packages/javascript/bh-shared-ui/src/components/HelpTexts/ProtectAdminGroups/General.tsx:0-0
Timestamp: 2025-08-06T20:42:40.256Z
Learning: In Active Directory security mechanisms, conventional knowledge and Microsoft documentation about SDProp being the mechanism that performs AdminSDHolder protection is incorrect and has been wrong for decades. The actual mechanism is the ProtectAdminGroups background task, which is distinct from SDProp. This is important tribal knowledge when working with AdminSDHolder-related functionality in BloodHound.
Applied to files:
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/LinuxAbuse.tsxpackages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/WindowsAbuse.tsx
🧬 Code graph analysis (16)
packages/go/analysis/ad/filters.go (2)
packages/go/ein/incoming_models.go (1)
Site(177-180)packages/go/graphschema/ad/ad.go (1)
Site(44-44)
cmd/api/src/services/graphify/convertors.go (4)
packages/go/ein/incoming_models.go (4)
Site(177-180)IngestBase(83-90)SiteServer(182-182)SiteSubnet(184-184)packages/go/graphschema/ad/ad.go (3)
Site(44-44)SiteServer(45-45)SiteSubnet(46-46)cmd/api/src/services/graphify/models.go (1)
ConvertedData(26-29)packages/go/ein/ad.go (4)
ConvertObjectToNode(46-52)ParseACEData(433-647)ParseObjectContainer(298-319)ParseGpLinks(826-846)
cmd/api/src/analysis/ad/queries.go (2)
packages/go/ein/incoming_models.go (3)
Site(177-180)SiteServer(182-182)SiteSubnet(184-184)packages/go/graphschema/ad/ad.go (3)
Site(44-44)SiteServer(45-45)SiteSubnet(46-46)
packages/go/ein/incoming_models.go (1)
packages/go/graphschema/ad/ad.go (1)
GPLink(58-58)
cmd/api/src/api/v2/ad_related_entity.go (5)
cmd/api/src/api/v2/model.go (1)
Resources(105-118)packages/go/analysis/ad/queries.go (4)
FetchGPOAffectedSitePaths(380-402)CreateGPOAffectedIntermediariesListDelegate(222-264)CreateSiteContainedPathDelegate(749-764)CreateSiteContainedListDelegate(730-747)packages/go/analysis/ad/filters.go (1)
SelectSitesCandidateFilter(157-159)packages/go/ein/incoming_models.go (2)
SiteServer(182-182)SiteSubnet(184-184)packages/go/graphschema/ad/ad.go (2)
SiteServer(45-45)SiteSubnet(46-46)
cmd/api/src/api/v2/ad_entity.go (3)
packages/go/analysis/ad/queries.go (2)
CreateGPOAffectedIntermediariesListDelegate(222-264)FetchInboundADEntityControllers(1418-1437)packages/go/analysis/ad/filters.go (1)
SelectSitesCandidateFilter(157-159)packages/go/graphschema/ad/ad.go (3)
Site(44-44)SiteServer(45-45)SiteSubnet(46-46)
cmd/api/src/services/graphify/ingest.go (1)
cmd/api/src/model/ingest/ingest.go (3)
DataTypeSite(112-112)DataTypeSiteServer(113-113)DataTypeSiteSubnet(114-114)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/LinuxAbuse.tsx (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (1)
EdgeInfoProps(142-151)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/WindowsAbuse.tsx (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (1)
EdgeInfoProps(142-151)
cmd/api/src/model/ingest/ingest.go (3)
packages/go/ein/incoming_models.go (3)
Site(177-180)SiteServer(182-182)SiteSubnet(184-184)packages/go/graphschema/ad/ad.go (3)
Site(44-44)SiteServer(45-45)SiteSubnet(46-46)cmd/api/src/api/v2/helpers.go (1)
DataType(34-34)
packages/go/graphschema/ad/ad.go (1)
packages/go/ein/incoming_models.go (3)
Site(177-180)SiteServer(182-182)SiteSubnet(184-184)
packages/javascript/bh-shared-ui/src/components/HelpTexts/GPLink/WindowsAbuse.tsx (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (1)
EdgeInfoProps(142-151)
cmd/api/src/api/registration/v2.go (1)
cmd/api/src/api/constant.go (1)
URIPathVariableObjectID(65-65)
packages/go/analysis/ad/queries.go (3)
packages/go/ein/incoming_models.go (4)
Domain(263-270)OU(368-374)Site(177-180)Container(241-245)packages/go/graphschema/ad/ad.go (6)
Domain(35-35)OU(33-33)Site(44-44)Nodes(1149-1151)Container(34-34)Contains(57-57)packages/go/analysis/analysis.go (2)
ListDelegate(154-154)PathDelegate(153-153)
packages/javascript/bh-shared-ui/src/utils/content.ts (1)
packages/javascript/js-client-library/src/requests.ts (1)
RequestOptions(30-30)
packages/javascript/js-client-library/src/client.ts (1)
packages/javascript/js-client-library/src/requests.ts (1)
RequestOptions(30-30)
🔇 Additional comments (40)
cmd/ui/src/ducks/graph/types.ts (1)
52-54: LGTM!The enum additions are correctly implemented, following the established pattern of string enums with values matching their keys. The new node types (Site, SiteServer, SiteSubnet) are properly integrated into the GraphNodeTypes enum and will work seamlessly with the existing type system.
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/General.tsx (1)
31-35: LGTM!The explanation clearly describes the security risk and provides a concrete attack example using scheduled tasks.
packages/javascript/bh-shared-ui/src/components/HelpTexts/GPLink/WindowsAbuse.tsx (1)
17-17: LGTM!The addition of
Linkto the imports is necessary for the external reference added later in the component.packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/WindowsAbuse.tsx (1)
19-21: LGTM!The refactoring to accept
EdgeInfoPropsand usetargetTypefor conditional rendering is well-implemented and aligns with the broader PR objective of supporting Site-specific help text.packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/LinuxAbuse.tsx (1)
19-21: LGTM!The component refactoring follows the same pattern as WindowsAbuse.tsx and correctly implements the prop-driven approach for Site support.
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/LinuxAbuse.tsx (2)
458-507: Excellent documentation improvement.The GPO case has been substantially enhanced with concrete tooling examples and detailed module configuration. The addition of the GroupPolicyBackdoor.py workflow with the actual INI-style configuration (lines 475-489) and command examples provides clear, actionable guidance. The alternative reference to pyGPOAbuse.py gives users options. This is a significant improvement over the previous version.
547-608: Consistent documentation pattern established.The OU case updates mirror the improvements made to the Domain and Site cases, establishing a consistent documentation pattern across all three object types that support gPLink manipulation. The parallel structure makes it easy for users to understand the attack vector regardless of which object type they're working with.
packages/go/graphschema/ad/ad.go (1)
44-46: LGTM! Site node kinds properly integrated.The three new node kinds (Site, SiteServer, SiteSubnet) are correctly declared and consistently added to both
Nodes()andNodeKinds()functions, following the established pattern for other AD node types.Also applies to: 1150-1150, 1179-1179
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/References.tsx (1)
125-130: LGTM! Relevant reference properly added.The Synacktiv publication link about AD sites enumeration and attacks is appropriately added and follows the existing reference formatting pattern.
packages/javascript/bh-shared-ui/src/utils/icons.ts (1)
146-159: Verify icon color choices for accessibility.The icon mappings are correctly added, but the colors used are quite light:
- Site:
'#bababdff'(light grey)- SiteServer:
'#dcdce6ff'(lighter grey)- SiteSubnet:
'#fdfdfdff'(nearly white)These pale colors may create visibility issues on light backgrounds. Please verify that these color choices provide adequate contrast for accessibility and align with the design intent.
cmd/api/src/model/adquality.go (1)
37-39: LGTM! Site-related fields properly added.The new site-related fields (Sites, SiteServers, SiteSubnets) are consistently added to both
ADDataQualityStatandADDataQualityAggregationstructs with proper JSON and GORM tags, following the established pattern.Also applies to: 64-66
cmd/api/src/analysis/ad/queries.go (1)
173-184: LGTM! Site counting logic correctly implemented.The new cases for Site, SiteServer, and SiteSubnet follow the established pattern for domain-scoped statistics counting, with proper mutex locking and aggregation updates.
packages/javascript/bh-shared-ui/src/graphSchema.ts (1)
33-35: LGTM! Enum values properly synchronized with backend.The new Site-related enum members are correctly added to
ActiveDirectoryNodeKindwith corresponding display name mappings, maintaining consistency with the Go backend definitions.Also applies to: 71-76
cmd/api/src/database/migration/migrations/v8.4.0.sql (1)
47-53: LGTM! Database migration is safe and well-structured.The migration safely adds site-related columns using
ADD COLUMN IF NOT EXISTSwith appropriate defaults. The default selector insertion for Sites is properly guarded with existence checks to prevent duplicates and follows the established pattern for Tier Zero selectors.Also applies to: 57-99
cmd/api/src/services/graphify/ingest.go (1)
342-344: LGTM! Site-related ingest handlers follow established patterns.The three new basicHandlers for Site, SiteServer, and SiteSubnet are correctly wired using
defaultBasicHandlerwith their respective conversion functions, consistent with existing handlers in the map.packages/cue/bh/ad/ad.cue (1)
1264-1299: LGTM! CUE schema definitions are consistent.The three new Site-related node kinds (Site, SiteServer, SiteSubnet) are correctly defined following the established pattern for Active Directory kinds, and properly included in the NodeKinds aggregate.
Based on learnings: These CUE definitions will generate corresponding Go code when
just prepare-for-codereviewis run, which is the expected workflow.cmd/api/src/api/v2/ad_entity.go (2)
163-163: LGTM! Sites correctly added to GPO affected intermediaries.Adding the "sites" query using
SelectSitesCandidateFilterproperly extends GPO affiliation tracking to include Site objects, consistent with the PR's objective to represent GPOs affecting Sites.
287-315: LGTM! New Site entity handlers follow established patterns.The three new entity info handlers (GetSiteEntityInfo, GetSiteServerEntityInfo, GetSiteSubnetEntityInfo) are correctly implemented following the same pattern as other minimal entity handlers like GetContainerEntityInfo, each providing a "controllers" count query.
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/WindowsAbuse.tsx (4)
142-168: LGTM! Improved GPO abuse documentation.The updated GPO case provides clearer guidance, including GPO application timing details (90 minutes for standard objects, 5 minutes for DCs) and mentions both GPMC and DRSAT tools for editing GPOs.
271-311: LGTM! OU case documentation improved for consistency.The rewording clarifies gPLink manipulation mechanics, prerequisites, and attack execution steps, maintaining consistency with the Domain and Site cases.
313-354: LGTM! Domain case aligned with OU and Site patterns.The Domain case documentation now follows the same structure and terminology as the OU and Site cases, improving overall consistency.
407-451: LGTM! Comprehensive Site abuse documentation added.The new Site case provides thorough documentation explaining the Site-specific attack surface, correctly noting that affected objects are computers with IP addresses in the site's subnets (or the default site) and users connecting to those computers. The documentation structure mirrors the Domain and OU cases for consistency.
cmd/api/src/api/registration/v2.go (2)
279-279: LGTM! GPO sites endpoint properly registered.The new
/api/v2/gpos/{object_id}/sitesendpoint is correctly registered with GraphDBRead permissions, following the pattern of existing GPO relationship endpoints.
347-360: LGTM! Complete REST API surface for Site entities.The new Site, SiteServer, and SiteSubnet entity endpoints are properly registered following established patterns:
- Entity info endpoints for all three types
- Controllers endpoints for all three types
- Site-specific linked servers and subnets endpoints
All routes correctly require GraphDBRead permissions and use the standard
URIPathVariableObjectIDpath variable.cmd/api/src/model/ingest/ingest.go (3)
82-88: LGTM! MatchKind() correctly handles new Site data types.The three new cases properly map DataTypeSite, DataTypeSiteServer, and DataTypeSiteSubnet to their corresponding Active Directory graph kinds, following the established pattern.
112-114: LGTM! New DataType constants follow naming conventions.The three new DataType constants are correctly defined with appropriate string values following the established plural naming pattern.
137-139: LGTM! AllIngestDataTypes() updated to include Site types.The new data types are properly added to the AllIngestDataTypes() slice, ensuring they're recognized as valid ingest data types for validation purposes.
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/LinuxAbuse.tsx (4)
183-230: LGTM! GPO case updated with GroupPolicyBackdoor.py guidance.The GPO case now references modern tooling (GroupPolicyBackdoor.py) with a comprehensive module configuration example demonstrating how to inject a scheduled task with item-level targeting. The timing details (90 minutes for standard objects, 5 minutes for DCs) provide helpful context.
241-300: LGTM! OU case aligned with modern tooling.The updated OU case properly references both OUned.py for the gPLink manipulation vector and GroupPolicyBackdoor.py for controlled GPO scenarios, with concrete command examples showing the inject and link workflow.
301-360: LGTM! Domain case follows OU pattern.The Domain case documentation mirrors the OU case structure with appropriate tool references and command examples, maintaining consistency across target types.
413-476: LGTM! Comprehensive Site abuse documentation for Linux.The new Site case provides thorough Linux-specific guidance, correctly explaining that Site-targeted GPOs affect computers in the site's subnets and users connecting to them. The command examples demonstrate the complete workflow using GroupPolicyBackdoor.py, including the proper distinguished name format for Site objects (
CN=Default-First-Site-Name,CN=Sites,CN=Configuration,DC=corp,DC=com).cmd/api/src/api/v2/ad_related_entity.go (2)
208-210: LGTM: path delegate for graph and list delegate pairing follows existing patternFetchGPOAffectedSitePaths for graph + list delegate for list/count aligns with v2 conventions.
248-254: Routes and query parameter support verifiedThe routes are correctly mapped at registration/v2.go:350-351:
- GET /api/v2/sites/{id}/siteservers → ListADSiteLinkedServers
- GET /api/v2/sites/{id}/sitesubnets → ListADSiteLinkedSubnets
Both endpoints support
type=list|count|graphquery parameters through the standard query handling chain:handleAdRelatedEntityQuery→BuildEntityQueryParams→GetRequestedType, which parses the type parameter and defaults to "list". Feature parity with peer endpoints likeListADGPOAffectedSitesis confirmed by identical implementation patterns.packages/go/analysis/ad/queries.go (5)
213-214: GPLink end includes Site — correct expansionAllowing ad.Site as a GPLink end makes the GPO queries site-aware. Looks good.
337-340: Correct to exclude Sites from container-affected traversalSites aren’t processed in the OU/container descent; skipping here avoids double-handling.
380-402: Direct GPO→Site paths are minimal and accurateNot traversing beyond the Site and ignoring enforcement is appropriate since inheritance doesn’t apply to Sites. Consider deduping by end.ID if duplicate GPLink edges are possible in ingest, but PathSet likely handles this; up to you.
556-556: Including Site in reverse-walk filter avoids premature stop for Site targetsThis keeps enforcement computation valid when the target is a Site without treating it as inheritance-blocking.
614-616: ACL inheritance: adding Site to start kinds enables Site ACL ancestry visualizationMatches new object model; good.
packages/javascript/bh-shared-ui/src/utils/content.ts (2)
164-168: Entity info mapping for Site entities — consistent with client surfaceMappings to getSiteV2/getSiteServerV2/getSiteSubnetV2 look correct.
904-908: Code wiring verified; confirm backend handles all query types in both list and graph modes
- All 5 query types (gpo-sites, site-linked_siteservers, site-linked_sitesubnets, siteserver-inbound_object_control, sitesubnet-inbound_object_control) properly wired in UI, handlers (lines 1784-1851), and backend client methods (js-client-library).
- Type parameter correctly threaded through entire stack for graph mode support.
- Implementation follows existing patterns and peers.
Verify backend implementations return data for these query types in both list (
typeunspecified) and graph (type='graph') modes before merging.
| Be mindful of the number of users and computers that are in the given OU as they all will | ||
| attempt to fetch and apply the malicious GPO. |
There was a problem hiding this comment.
Verify the warning text context.
Lines 449-450 state "Be mindful of the number of users and computers that are in the given OU" in the Domain case. This appears to be a copy-paste artifact from the OU case—it should reference "domain" rather than "OU" in this context.
Apply this diff:
- Be mindful of the number of users and computers that are in the given OU as they all will
+ Be mindful of the number of users and computers that are in the domain as they all will
attempt to fetch and apply the malicious GPO.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Be mindful of the number of users and computers that are in the given OU as they all will | |
| attempt to fetch and apply the malicious GPO. | |
| Be mindful of the number of users and computers that are in the domain as they all will | |
| attempt to fetch and apply the malicious GPO. |
🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/LinuxAbuse.tsx
around lines 449 to 450, the warning text incorrectly references "OU" in the
Domain case; update the text to reference "domain" instead (e.g., replace "in
the given OU" with "in the domain") so the message accurately reflects the
Domain context.
| <Link | ||
| target='_blank' | ||
| rel='noopener noreferrer' | ||
| href='https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites'> | ||
| https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites | ||
| </Link> |
There was a problem hiding this comment.
Add missing line break after the new link
Line 213 renders the new reference on the same line as the previous link because the usual <br /> separator is missing right after it. Please restore the line break so the list formatting stays consistent with the rest of the component.
<Link
target='_blank'
rel='noopener noreferrer'
href='https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites'>
https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites
</Link>
+ <br />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Link | |
| target='_blank' | |
| rel='noopener noreferrer' | |
| href='https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites'> | |
| https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites | |
| </Link> | |
| <Link | |
| target='_blank' | |
| rel='noopener noreferrer' | |
| href='https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites'> | |
| https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites | |
| </Link> | |
| <br /> |
🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/References.tsx
around lines 210 to 215, the newly added Link at line 213 is missing the
following line break so it renders inline with the previous link; add a <br />
immediately after that Link component (preserving indentation and existing
props) to restore the list's line-break formatting consistent with the other
entries.
| If you control a GPO linked to a target object, you may make modifications to that GPO in order to inject malicious configurations into it. | ||
| You could for instance add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies, | ||
| thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing | ||
| to precisely target a specific object. | ||
| GPOs are applied every 90 minutes for standard objects (with a random offset of 0 to 30 minutes), and every 5 minutes for domain controllers. |
There was a problem hiding this comment.
Fix grammatical errors in user-facing text.
The text contains minor grammatical issues:
- Line 26: "You could for instance" should be "You could, for instance," (add commas)
- Line 27: "allowing to precisely target" should be "allowing you to precisely target"
Apply this diff to fix the grammar:
- If you control a GPO linked to a target object, you may make modifications to that GPO in order to inject malicious configurations into it.
- You could for instance add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies,
- thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing
- to precisely target a specific object.
+ If you control a GPO linked to a target object, you may make modifications to that GPO in order to inject malicious configurations into it.
+ You could, for instance, add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies,
+ thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing you
+ to precisely target a specific object.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| If you control a GPO linked to a target object, you may make modifications to that GPO in order to inject malicious configurations into it. | |
| You could for instance add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies, | |
| thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing | |
| to precisely target a specific object. | |
| GPOs are applied every 90 minutes for standard objects (with a random offset of 0 to 30 minutes), and every 5 minutes for domain controllers. | |
| If you control a GPO linked to a target object, you may make modifications to that GPO in order to inject malicious configurations into it. | |
| You could, for instance, add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies, | |
| thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing you | |
| to precisely target a specific object. | |
| GPOs are applied every 90 minutes for standard objects (with a random offset of 0 to 30 minutes), and every 5 minutes for domain controllers. |
🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/HelpTexts/GPLink/LinuxAbuse.tsx
around lines 25 to 29, fix two grammar issues in the user-facing paragraph: add
commas around the parenthetical phrase so "You could for instance" becomes "You
could, for instance," and insert the missing pronoun so "allowing to precisely
target" becomes "allowing you to precisely target"; update the sentence
punctuation as needed to preserve readability.
| <Typography variant='body2'> | ||
| With full control of a GPO, you may make modifications to that GPO which will then apply to the users | ||
| and computers affected by the GPO. Select the target object you wish to push an evil policy down to, | ||
| then use the gpedit GUI to modify the GPO, using an evil policy that allows item-level targeting, such | ||
| as a new immediate scheduled task. Then wait for the group policy client to pick up and execute the new | ||
| evil policy. See the references tab for a more detailed write up on this abuse. | ||
| If you control a GPO linked to a target object, you may make modifications to that GPO in order to inject malicious configurations into it. | ||
| You could for instance add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies, | ||
| thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing | ||
| to precisely target a specific object. | ||
| GPOs are applied every 90 minutes for standard objects (with a random offset of 0 to 30 minutes), and every 5 minutes for domain controllers. | ||
| See the references tab for a more detailed write up on this abuse. | ||
| </Typography> |
There was a problem hiding this comment.
Fix grammatical error in user-facing text.
Line 28 contains a grammatical error: "allowing to precisely target" is missing the subject pronoun.
Apply this diff to fix the grammar:
- thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing
- to precisely target a specific object.
+ thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing
+ you to precisely target a specific object.Optionally, consider adding a comma for better readability on line 26:
- You could for instance add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies,
+ You could, for instance, add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Typography variant='body2'> | |
| With full control of a GPO, you may make modifications to that GPO which will then apply to the users | |
| and computers affected by the GPO. Select the target object you wish to push an evil policy down to, | |
| then use the gpedit GUI to modify the GPO, using an evil policy that allows item-level targeting, such | |
| as a new immediate scheduled task. Then wait for the group policy client to pick up and execute the new | |
| evil policy. See the references tab for a more detailed write up on this abuse. | |
| If you control a GPO linked to a target object, you may make modifications to that GPO in order to inject malicious configurations into it. | |
| You could for instance add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies, | |
| thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing | |
| to precisely target a specific object. | |
| GPOs are applied every 90 minutes for standard objects (with a random offset of 0 to 30 minutes), and every 5 minutes for domain controllers. | |
| See the references tab for a more detailed write up on this abuse. | |
| </Typography> | |
| <Typography variant='body2'> | |
| If you control a GPO linked to a target object, you may make modifications to that GPO in order to inject malicious configurations into it. | |
| You could for instance add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies, | |
| thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing | |
| you to precisely target a specific object. | |
| GPOs are applied every 90 minutes for standard objects (with a random offset of 0 to 30 minutes), and every 5 minutes for domain controllers. | |
| See the references tab for a more detailed write up on this abuse. | |
| </Typography> |
| <Typography variant='body2'> | |
| With full control of a GPO, you may make modifications to that GPO which will then apply to the users | |
| and computers affected by the GPO. Select the target object you wish to push an evil policy down to, | |
| then use the gpedit GUI to modify the GPO, using an evil policy that allows item-level targeting, such | |
| as a new immediate scheduled task. Then wait for the group policy client to pick up and execute the new | |
| evil policy. See the references tab for a more detailed write up on this abuse. | |
| If you control a GPO linked to a target object, you may make modifications to that GPO in order to inject malicious configurations into it. | |
| You could for instance add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies, | |
| thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing | |
| to precisely target a specific object. | |
| GPOs are applied every 90 minutes for standard objects (with a random offset of 0 to 30 minutes), and every 5 minutes for domain controllers. | |
| See the references tab for a more detailed write up on this abuse. | |
| </Typography> | |
| <Typography variant='body2'> | |
| If you control a GPO linked to a target object, you may make modifications to that GPO in order to inject malicious configurations into it. | |
| You could, for instance, add a Scheduled Task that will then be executed by all of the computers and/or users to which the GPO applies, | |
| thus compromising them. Note that some configurations (such as Scheduled Tasks) implement item-level targeting, allowing | |
| to precisely target a specific object. | |
| GPOs are applied every 90 minutes for standard objects (with a random offset of 0 to 30 minutes), and every 5 minutes for domain controllers. | |
| See the references tab for a more detailed write up on this abuse. | |
| </Typography> |
🤖 Prompt for AI Agents
packages/javascript/bh-shared-ui/src/components/HelpTexts/GPLink/WindowsAbuse.tsx
lines 24-31: fix the grammatical error "allowing to precisely target" by adding
the missing subject (e.g., change to "allowing you to precisely target" or
"allowing precise targeting of a specific object"), and optionally add a comma
after "for instance" on line 26 for readability.
| <Typography variant='body2'> | ||
| On a domain-joined Windows machine, the native Group Policy Management Console (GPMC) may be used to edit GPOs. | ||
| On a non-domain joined Windows Machine, the{' '} | ||
| <Link | ||
| target='_blank' | ||
| rel='noopener noreferrer' | ||
| href='https://github.com/CCob/DRSAT'> | ||
| DRSAT (Disconnected RSAT) | ||
| </Link> tool can be used. | ||
| </Typography> |
There was a problem hiding this comment.
Fix capitalization inconsistency in user-facing text.
Line 35 uses "Machine" (capitalized), while line 34 uses "machine" (lowercase). Maintain consistency by using lowercase throughout.
Apply this diff:
- On a non-domain joined Windows Machine, the{' '}
+ On a non-domain joined Windows machine, the{' '}The Link component is properly configured with security attributes (target='_blank' and rel='noopener noreferrer').
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Typography variant='body2'> | |
| On a domain-joined Windows machine, the native Group Policy Management Console (GPMC) may be used to edit GPOs. | |
| On a non-domain joined Windows Machine, the{' '} | |
| <Link | |
| target='_blank' | |
| rel='noopener noreferrer' | |
| href='https://github.com/CCob/DRSAT'> | |
| DRSAT (Disconnected RSAT) | |
| </Link> tool can be used. | |
| </Typography> | |
| <Typography variant='body2'> | |
| On a domain-joined Windows machine, the native Group Policy Management Console (GPMC) may be used. | |
| On a non-domain joined Windows machine, the{' '} | |
| <Link | |
| target='_blank' | |
| rel='noopener noreferrer' | |
| href='https://github.com/CCob/DRSAT'> | |
| DRSAT (Disconnected RSAT) | |
| </Link> tool can be used. | |
| </Typography> |
🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/HelpTexts/GPLink/WindowsAbuse.tsx
around lines 33 to 42, there's a capitalization inconsistency: change
"non-domain joined Windows Machine" to "non-domain joined Windows machine" so
both occurrences of "machine" are lowercase; keep the Link component and its
security attributes unchanged.
| Note that Server objects associated with the Site should be located in the Site. | ||
| </Typography> |
There was a problem hiding this comment.
Clarify the Server objects statement.
The phrase "Server objects associated with the Site should be located in the Site" is ambiguous and may confuse users:
- Does "Server objects" refer to the new SiteServer node type introduced in this PR, or to server computers in general?
- What does "should be located in the Site" mean? Does it mean their IP addresses should be within the site's subnets, or something else?
- The phrasing sounds prescriptive ("should be") rather than descriptive of how the system works.
Consider rephrasing for clarity, for example:
"Note that server computers defined as site servers are also affected by GPOs linked to the site."
| <Typography variant='body2'> | ||
| Be mindful of the number of users and computers that are in the given OU as they all will | ||
| attempt to fetch and apply the malicious GPO. | ||
| </Typography> |
There was a problem hiding this comment.
Fix incorrect scope reference in warning message.
Lines 70-71 say "in the given OU" but this is in the Domain case. It should say "in the given domain" to match the target type.
Apply this diff:
<Typography variant='body2'>
- Be mindful of the number of users and computers that are in the given OU as they all will
+ Be mindful of the number of users and computers that are in the given domain as they all will
attempt to fetch and apply the malicious GPO.
</Typography>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Typography variant='body2'> | |
| Be mindful of the number of users and computers that are in the given OU as they all will | |
| attempt to fetch and apply the malicious GPO. | |
| </Typography> | |
| <Typography variant='body2'> | |
| Be mindful of the number of users and computers that are in the given domain as they all will | |
| attempt to fetch and apply the malicious GPO. | |
| </Typography> |
🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/LinuxAbuse.tsx
around lines 69 to 72, the warning text incorrectly references "OU" for the
Domain target; update the copy to say "in the given domain" (or otherwise
replace "OU" with "domain") so the message matches the Domain case and intent.
| <Link | ||
| target='_blank' | ||
| rel='noopener noreferrer' | ||
| href='https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites'> | ||
| https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites | ||
| </Link> |
There was a problem hiding this comment.
🧩 Analysis chain
Add missing line break before the new link.
The new reference link follows the correct pattern and is relevant to the PR's Active Directory Sites feature. However, there's a missing <br /> tag between the second link (ending at line 35) and this new third link. This will cause inconsistent spacing compared to the existing links.
Apply this diff to add the missing line break:
https://www.synacktiv.com/publications/ounedpy-exploiting-hidden-organizational-units-acl-attack-vectors-in-active-directory
</Link>
+ <br />
<Link
target='_blank'
rel='noopener noreferrer'Additionally, verify that the Synacktiv URL is accessible and correctly points to the intended article about AD Sites.
🏁 Script executed:
#!/bin/bash
# Verify that the Synacktiv URL is accessible
curl -I -L "https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites" 2>&1 | head -20Length of output: 532
---
🏁 Script executed:
```shell
#!/bin/bash
# View the actual file content around the referenced lines
cat packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/References.tsx | sed -n '23,45p'
Length of output: 1068
Add missing line break and fix the broken reference URL.
The new reference link has two issues:
-
Missing
<br />separator: The third link lacks a line break before it, unlike the spacing after the first link (line 29). Add<br />between line 35 and 36 for consistency. -
Broken URL (404 error): The Synacktiv publication URL returns an HTTP 404 error and is inaccessible. Either the URL is incorrect or the resource no longer exists. Verify the correct publication link and update it.
Apply this fix:
https://www.synacktiv.com/publications/ounedpy-exploiting-hidden-organizational-units-acl-attack-vectors-in-active-directory
</Link>
+ <br />
<Link
target='_blank'
rel='noopener noreferrer'
href='https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites'>Verify the correct Synacktiv publication URL for "Site-Unseen: Enumerating and Attacking Active Directory Sites" and ensure it's accessible before merging.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Link | |
| target='_blank' | |
| rel='noopener noreferrer' | |
| href='https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites'> | |
| https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites | |
| </Link> | |
| <br /> | |
| <Link | |
| target='_blank' | |
| rel='noopener noreferrer' | |
| href='https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites'> | |
| https://www.synacktiv.com/publications/site-unseen-enumerating-and-attacking-active-directory-sites | |
| </Link> |
🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/References.tsx
around lines 36 to 41, add the missing <br /> separator before the Synacktiv
link to match the spacing used for other links, and replace the broken Synacktiv
URL with the correct, accessible publication URL for "Site-Unseen: Enumerating
and Attacking Active Directory Sites" (verify the canonical Synacktiv
publication page or archived location and update the href accordingly); ensure
the Link keeps target='_blank' and rel='noopener noreferrer' and that the
visible text matches the updated URL or a short descriptive label.
| Be mindful of the number of users and computers that are in the given domain as they all will | ||
| attempt to fetch and apply the malicious GPO. |
There was a problem hiding this comment.
Fix incorrect scope reference in warning message.
Line 86 says "in the given domain" but this is in the OU case. It should say "in the given OU" to match the target type.
Apply this diff:
<Typography variant='body2'>
- Be mindful of the number of users and computers that are in the given domain as they all will
+ Be mindful of the number of users and computers that are in the given OU as they all will
attempt to fetch and apply the malicious GPO.
</Typography>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Be mindful of the number of users and computers that are in the given domain as they all will | |
| attempt to fetch and apply the malicious GPO. | |
| Be mindful of the number of users and computers that are in the given OU as they all will | |
| attempt to fetch and apply the malicious GPO. |
🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteGPLink/WindowsAbuse.tsx
around lines 86 to 87, the warning text incorrectly says "in the given domain"
while this help text is for the OU case; change the string to say "in the given
OU" (or otherwise replace "domain" with "OU") so the scope matches the target
type, and update any nearby punctuation or line wrapping to preserve formatting.
1 participant
Description
Hi Bloodhound's team! This pull request contains a proposition to implement the visualisation of data and ACLs related to Active Directory Sites into Bloodhound. A more complete description on why we thought this could be a good idea is described in details in the following article that we just released: https://www.synacktiv.com/en/publications/site-unseen-enumerating-and-attacking-active-directory-sites
In a nutshell, Active Directory sites can be affected by several exploitable ACLs, namely:
Successful exploitation might be rather critical, as the compromise of a site results in the compromise of one or several domain controller(s) acting as the site's server(s). Which is why the pull requests suggest placing the site objects to the default high value targets.
Two pull requests for the SharpHound and SharpHoundCommon were also submitted, in order to provide the necessary data to visualize through this pull request:
As described in the linked issue (see below), the following additions are mainly implemented:
Motivation and Context
Resolves #2030
Motivation and context are explained in the linked article.
How Has This Been Tested?
This has for the moment been tested in local lab instances. The lab instance was composed of various Active Directory site configurations:
Automated test cases have not yet been added to the project. This is something that I could implement in the next few days if you wish for me to do so.
Screenshots (optional):
Types of changes
Checklist:
Additional information
This PR is an implementation suggestion, I remain of course open to modifications (for the visuals or the implementation itself). Please do not hesitate if you need me to make specific changes, I'Il be glad to discuss the choices I made, or change them.
As was mentioned above, the automated testing and documentation are currently not included in the PR.
I decided to submit this pull request before implementing the tests and documentation in order to have your preliminary opinion on the proposed implementation, and then adapt test cases / documentation.
I can start working on it in the next few days if you want!
Cheers,
Summary by CodeRabbit