NotificationsYou must be signed in to change notification settings
Fork225
Star1.1k

Add local privilege collection#184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

ncha-syn wants to merge2 commits intoSpecterOps:2.X

base:2.X

Choose a base branch

fromncha-syn:2.X

Open

Add local privilege collection#184

ncha-syn wants to merge2 commits intoSpecterOps:2.Xfromncha-syn:2.X

Conversation

Copy link

ncha-syn commentedOct 24, 2025•
edited by coderabbitaibot
Loading

Description

This pull request adds the ability to SharpHound to collect user privileges using GPOs and increases of the scope of the collected privileges.

Motivation and Context

This PR followsSpecterOps/BloodHound#1999.

How Has This Been Tested?

The changes have been tested manually on a lab environment.

Screenshots (if appropriate):

Types of changes

Chore (a change that does not modify the application functionality)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

Documentation updates are needed, and have been made accordingly.
I have added and/or updated tests to cover my changes.
All new and existing tests passed.
My changes include a database migration.

Summary by CodeRabbit

New Features
- Added GPOUserRights as a new collection method option
- Available in command-line interface help text and PowerShell parameters
- Updated documentation and configurations to support the new method
- Extends available collection methods: Container, Group, LocalGroup, GPOLocalGroup, and GPOUserRights

ncha-syn added2 commits

October 24, 2025 14:42

add local privilege collection

baedd72

Update Template.ps1

e4c0d99

Copy link

coderabbitaibot commentedOct 24, 2025•
edited
Loading

Walkthrough

This PR adds support for a new collection method called GPOUserRights. The changes include updating the enum definition, CLI help text, PowerShell parameter validation, and runtime processing logic to handle this new collection method, with specific processing for OU objects.

Changes

Cohort / File(s)	Change Summary
Enum Definition `src/Client/Enums.cs`	Added new enum member`GPOUserRights` to`CollectionMethodOptions`, inserted after`GPOLocalGroup`.
Configuration & Help Text `README.md`,`src/Options.cs`	Updated CLI help text and collection methods option list to include`GPOUserRights`, and added mapping from`CollectionMethodOptions.GPOUserRights` to`CollectionMethod.GPOUserRights` in the resolution switch.
PowerShell Module `src/PowerShell/Template.ps1`	Extended`CollectionMethods` parameter allowed values in`Invoke-BloodHound` to include`GPOUserRights`.
Runtime Processing `src/Runtime/ObjectProcessors.cs`	Added`GPOUserRightsAssignmentProcessor` field and initialization in constructor; introduced processing hook for OU objects to fetch and assign GPO user rights via the new processor.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

enhancement,blocked by SHC PR

Suggested reviewers

ktstrader
definitelynotagoblin

Poem

🐰 A new collection method hops into view,
GPOUserRights now part of the queue,
From enums to PowerShell, the changes align,
Runtime processors handle OU's design—
BloodHound's reach grows ever so fine! 🎯

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run`@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The PR title "Add local privilege collection" is related to the main change in the pull request. The changeset adds GPOUserRights as a new collection method across multiple components (enums, CLI options, PowerShell templates, and object processors), enabling SharpHound to collect user privileges via GPOs. The title appropriately summarizes this primary change, though it could be more specific about GPO-based collection. The title is clear and avoids vague terminology, making it sufficiently descriptive for scanning commit history.
Description Check	✅ Passed	The PR description follows the required template structure with all major sections addressed. The Description section clearly states the purpose, the Motivation and Context section provides a link to a related BloodHound PR for context, the testing section indicates manual lab environment testing, and a screenshot is included. The author appropriately selected "New feature" for the type of change. While the testing section is relatively brief and lacks extensive detail, it is sufficiently filled out for evaluation, and the unchecked checklist items appear to be reasonable author judgment rather than oversights, as documentation updates, formal tests, and database migrations may not apply to this feature addition.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for usingCodeRabbit! 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.}

coderabbitaibot reviewed

Oct 24, 2025

View reviewed changes

Copy link

coderabbitaibot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/Runtime/ObjectProcessors.cs (1)

365-365:Remove commented-out code.

These commented-out code blocks should be removed to maintain code cleanliness. If these represent alternative implementations being evaluated, consider using feature flags or creating a separate development branch.

Apply this diff to remove the commented-out code:

             if (_methods.HasFlag(CollectionMethod.SmbInfo)) {                 ret.SmbInfo = await _smbProcessor.Scan(apiName, resolvedSearchResult.DomainSid);-                //ret.SmbInfo = await _smbProcessor.Scan(apiName);             }

                 if (ldapServices.IsSigningRequired.Collected) {                     ret.Properties.Add("ldapsigning", ldapServices.IsSigningRequired.Result);                 }-                //var ldapServices = await dcLdapProcessor.Scan(resolvedSearchResult.DisplayName);-                //ret.Properties.Add("ldapavailable", ldapServices.HasLdap);-                //ret.Properties.Add("ldapsavailable", ldapServices.HasLdaps);-                //if (ldapServices.IsChannelBindingDisabled.Collected) {-                //    ret.Properties.Add("ldapsepa", !ldapServices.IsChannelBindingDisabled.Result);-                //}--                //if (ldapServices.IsSigningRequired.Collected) {-                //    ret.Properties.Add("ldapsigning", ldapServices.IsSigningRequired.Result);-                //}             }

Also applies to: 431-440

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and betweendc420a6 ande4c0d99.

📒 Files selected for processing (5)

README.md (1 hunks)
src/Client/Enums.cs (1 hunks)
src/Options.cs (2 hunks)
src/PowerShell/Template.ps1 (1 hunks)
src/Runtime/ObjectProcessors.cs (6 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/Options.cs (3)

src/BaseContext.cs (1)
CollectionMethod (95-102)
src/Client/Context.cs (1)
CollectionMethod (75-75)
src/Extensions.cs (1)
CollectionMethod (114-119)

🔇 Additional comments (7)

src/Client/Enums.cs (1)
22-22:LGTM! Enum member positioned logically.
The newGPOUserRights enum member is appropriately placed alongside other GPO-related collection methods.
README.md (1)
38-38:LGTM! Documentation updated correctly.
The CLI help text accurately reflects the new collection method.
src/PowerShell/Template.ps1 (1)
30-30:LGTM! PowerShell parameter documentation updated.
The description clearly explains the new collection method's purpose.
src/Options.cs (2)
17-17:LGTM! Help text includes new collection method.
The help text correctly lists GPOUserRights among available collection methods.
199-199:Mapping is correct; composite method verification is external to this file.
The enum mapping at line 199 correctly mapsCollectionMethodOptions.GPOUserRights toCollectionMethod.GPOUserRights, consistent with all other option mappings in the switch expression.
However, verification of whetherGPOUserRights is appropriately included in composite methods (Default,All,DCOnly) is not applicable here. Those composite values are defined in the externalSharpHoundCommonLib.Enums.CollectionMethod enum, not in this file. The code inOptions.cs simply maps individual user-specified options to their corresponding enum values using bitwise OR operations—it does not construct or define composite methods.
The mapping itself contains no issues: it handles the newGPOUserRights option consistently with existing collection methods.
src/Runtime/ObjectProcessors.cs (2)
35-35:LGTM! Processor field and initialization follow established pattern.
TheGPOUserRightsAssignmentProcessor is correctly declared and initialized, mirroring the approach used forGPOLocalGroupProcessor.
Also applies to: 60-60
615-617:Verify OU class has GPOUserRights property.
The implementation correctly processes GPO user rights for OUs when the collection method is enabled. Ensure that theOU class (in SharpHoundCommonLib.OutputTypes) has a correspondingGPOUserRights property defined to receive this data.

Labels

None yet

Movatterモバイル変換

Add local privilege collection#184

Are you sure you want to change the base?

Add local privilege collection#184

Uh oh!

Conversation

ncha-syn commentedOct 24, 2025• edited by coderabbitaibotLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Summary by CodeRabbit

Uh oh!

coderabbitaibot commentedOct 24, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitaibot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ncha-syn commentedOct 24, 2025•
edited by coderabbitaibot
Loading

coderabbitaibot commentedOct 24, 2025•
edited
Loading