chore: Add ObjectID to ComputerStatus

[MikeX777]

Description

Adding ObjectId to ComputerStatus

Motivation and Context

Resolves: BED-6568

How Has This Been Tested?

This has been tested by creating a build of SharpHound with the updated library, and using running a collection.

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
  • Status outputs now include an Object ID for computers, adding a new column to CSV exports for improved traceability across processors (LDAP, SMB, SPN, Local Groups, Sessions, URA, Cert Abuse).
• Breaking Changes
  • CSV format updated: status rows gain an additional Object ID column.
  • Some scans now accept an extra Object ID/SID parameter.
• Tests
  • Unit tests updated to reflect new method signatures and CSV output structure.

[coderabbitai]

Walkthrough

Adds ObjectId to CSVComputerStatus and propagates it across multiple processors and status reports. Updates CSV serialization to include ObjectId. Changes method signatures for DCLdapProcessor.Scan, SmbProcessor.Scan, and ComputerAvailability.IsComputerAvailable to accept/passthrough identifiers. Updates related unit tests to match new signatures.

Changes

Cohort / File(s)	Summary
Data model: CSVComputerStatus src/CommonLib/CSVComputerStatus.cs	Added public property ObjectId; ToCsv now outputs four cells including ObjectId.
Processor status payloads updated src/CommonLib/Processors/CertAbuseProcessor.cs, .../ComputerSessionProcessor.cs, .../DCLdapProcessor.cs, .../LdapPropertyProcessor.cs, .../LocalGroupProcessor.cs, .../SPNProcessors.cs, .../SmbProcessor.cs, .../UserRightsAssignmentProcessor.cs	Included ObjectId in all CSVComputerStatus emissions; no control-flow changes.
Public API signature changes src/CommonLib/Processors/DCLdapProcessor.cs, src/CommonLib/Processors/SmbProcessor.cs, src/CommonLib/Processors/ComputerAvailability.cs	DCLdapProcessor.Scan(string computerName, string computerObjectId); SmbProcessor.Scan(string host, string securityIdentifier); IsComputerAvailable(..., string objectId = null). Propagates ObjectId through status reporting.
Unit tests updated for new signatures test/unit/DCLdapProcessorTest.cs, test/unit/SmbProcessorTest.cs	Updated Scan calls to pass an extra string argument (e.g., "").

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant Processor
  participant CSVComputerStatus as CSV Status
  participant StatusSink as Status Channel

  Caller->>Processor: Scan/IsComputerAvailable(..., objectId)
  rect rgba(230,245,255,0.6)
    note right of Processor: Prepare status with ObjectId
    Processor->>CSVComputerStatus: new {..., ObjectId}
    CSVComputerStatus-->>Processor: ToCsv() now includes ObjectId
  end
  Processor->>StatusSink: Send(status CSV with ObjectId)
  StatusSink-->>Caller: Acknowledgement/Result

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I thump my paws with merry pride,
A fourth cell joined the CSV ride;
From burrow logs to forest IDs,
ObjectId hops on every breeze.
Tests nibble new args, all aligned—
Carrots counted, neatly signed. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.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 title succinctly conveys the main modification by stating the addition of ObjectID to ComputerStatus and follows conventional commit format without unnecessary details.
Description Check	✅ Passed	The pull request description adheres closely to the repository template, providing a clear description, linking the resolved issue in Motivation, detailing testing procedures, specifying change types, and completing the checklist; optional sections like screenshots are appropriately omitted.

✨ 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 mcuomo/BED-6568

---

📜 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 1ec7cba and fccb713.

📒 Files selected for processing (12)

• src/CommonLib/CSVComputerStatus.cs (1 hunks)
• src/CommonLib/Processors/CertAbuseProcessor.cs (2 hunks)
• src/CommonLib/Processors/ComputerAvailability.cs (5 hunks)
• src/CommonLib/Processors/ComputerSessionProcessor.cs (8 hunks)
• src/CommonLib/Processors/DCLdapProcessor.cs (2 hunks)
• src/CommonLib/Processors/LdapPropertyProcessor.cs (2 hunks)
• src/CommonLib/Processors/LocalGroupProcessor.cs (8 hunks)
• src/CommonLib/Processors/SPNProcessors.cs (1 hunks)
• src/CommonLib/Processors/SmbProcessor.cs (3 hunks)
• src/CommonLib/Processors/UserRightsAssignmentProcessor.cs (4 hunks)
• test/unit/DCLdapProcessorTest.cs (2 hunks)
• test/unit/SmbProcessorTest.cs (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (7)

src/CommonLib/Processors/SmbProcessor.cs (6)

src/CommonLib/Processors/ComputerSessionProcessor.cs (4)

• Task (54-185)
• Task (196-287)
• Task (289-336)
• Task (338-340)

src/CommonLib/Processors/DCLdapProcessor.cs (5)

• Task (48-114)
• Task (130-142)
• Task (152-168)
• Task (176-227)
• Task (229-231)

src/CommonLib/Processors/LocalGroupProcessor.cs (3)

• Task (344-350)
• Task (352-386)
• Task (388-391)

src/CommonLib/Processors/UserRightsAssignmentProcessor.cs (2)

• Task (203-210)
• Task (213-215)

src/CommonLib/OutputTypes/APIResult.cs (4)

• APIResult (2-5)
• APIResult (7-28)
• APIResult (10-15)
• APIResult (17-22)

src/CommonLib/OutputTypes/Computer.cs (2)

• SmbInfo (61-63)
• CSVComputerStatus (80-86)

src/CommonLib/CSVComputerStatus.cs (2)

src/CommonLib/Processors/ComputerSessionProcessor.cs (4)

• Task (54-185)
• Task (196-287)
• Task (289-336)
• Task (338-340)

src/CommonLib/Processors/ComputerAvailability.cs (3)

• Task (44-51)
• Task (65-131)
• Task (147-149)

src/CommonLib/Processors/LocalGroupProcessor.cs (4)

src/CommonLib/Processors/CertAbuseProcessor.cs (7)

• Task (42-152)
• Task (162-195)
• Task (197-213)
• Task (314-347)
• Task (349-397)
• Task (399-449)
• Task (462-465)

src/CommonLib/Processors/ComputerSessionProcessor.cs (4)

• Task (54-185)
• Task (196-287)
• Task (289-336)
• Task (338-340)

src/CommonLib/Processors/ComputerAvailability.cs (3)

• Task (44-51)
• Task (65-131)
• Task (147-149)

src/CommonLib/CSVComputerStatus.cs (1)

• CSVComputerStatus (6-47)

src/CommonLib/Processors/CertAbuseProcessor.cs (2)

src/CommonLib/Processors/LocalGroupProcessor.cs (3)

• Task (344-350)
• Task (352-386)
• Task (388-391)

src/CommonLib/LdapUtils.cs (12)

• Task (105-109)
• Task (111-140)
• Task (142-197)
• Task (199-246)
• Task (248-264)
• Task (266-287)
• Task (289-312)
• Task (314-330)
• Task (332-385)
• Task (387-427)
• Task (429-482)
• Task (579-606)

src/CommonLib/Processors/UserRightsAssignmentProcessor.cs (3)

src/CommonLib/Processors/CertAbuseProcessor.cs (7)

• Task (42-152)
• Task (162-195)
• Task (197-213)
• Task (314-347)
• Task (349-397)
• Task (399-449)
• Task (462-465)

src/CommonLib/Processors/ComputerSessionProcessor.cs (4)

• Task (54-185)
• Task (196-287)
• Task (289-336)
• Task (338-340)

src/CommonLib/Processors/LocalGroupProcessor.cs (3)

• Task (344-350)
• Task (352-386)
• Task (388-391)

src/CommonLib/Processors/ComputerAvailability.cs (3)

src/CommonLib/Processors/ComputerSessionProcessor.cs (4)

• Task (54-185)
• Task (196-287)
• Task (289-336)
• Task (338-340)

src/CommonLib/OutputTypes/Computer.cs (2)

• ComputerStatus (71-87)
• CSVComputerStatus (80-86)

src/CommonLib/CSVComputerStatus.cs (1)

• CSVComputerStatus (6-47)

src/CommonLib/Processors/DCLdapProcessor.cs (5)

src/CommonLib/Processors/ComputerSessionProcessor.cs (4)

• Task (54-185)
• Task (196-287)
• Task (289-336)
• Task (338-340)

src/CommonLib/Processors/LocalGroupProcessor.cs (3)

• Task (344-350)
• Task (352-386)
• Task (388-391)

src/CommonLib/Processors/SmbProcessor.cs (2)

• Task (28-68)
• Task (70-72)

src/CommonLib/Processors/UserRightsAssignmentProcessor.cs (2)

• Task (203-210)
• Task (213-215)

src/CommonLib/OutputTypes/Computer.cs (2)

• LdapService (33-58)
• CSVComputerStatus (80-86)

🔇 Additional comments (20)

src/CommonLib/CSVComputerStatus.cs (2)

12-12: LGTM: ObjectId property added.

The new public property follows the existing pattern and supports the PR objective of propagating object identifiers through status reports.

---

20-20: LGTM: CSV serialization extended to include ObjectId.

The ToCsv() method correctly appends ObjectId as the fourth CSV cell, and StringToCsvCell handles null values appropriately (lines 30-31).

src/CommonLib/Processors/SPNProcessors.cs (1)

58-63: LGTM: ObjectId propagation in SPN target resolution.

The ObjectId is correctly populated with the resolved host SID when an SPN target is successfully resolved. The host variable is validated to start with "S-1" before this point (line 56), ensuring a valid SID is propagated.

test/unit/DCLdapProcessorTest.cs (1)

46-46: LGTM: Test updated to match new Scan signature.

The test calls have been correctly updated to pass an additional empty string argument, aligning with the updated DCLdapProcessor.Scan method signature that now accepts a computerObjectId parameter.

Also applies to: 74-74

src/CommonLib/Processors/ComputerSessionProcessor.cs (4)

86-105: LGTM: ObjectId propagation in NetSessionEnum status reports.

The ObjectId field is correctly populated with computerSid in both failure (lines 90-91) and success (lines 103-104) status reports for NetSessionEnum operations.

---

145-150: LGTM: ObjectId propagation for resolved session hosts.

When resolving session hostnames to SIDs within NetSessionEnum processing, the status reports correctly include the resolvedComputerSID as ObjectId. The resolved SID is validated to start with "S-1" before usage (line 155).

Also applies to: 168-173

---

228-247: LGTM: ObjectId propagation in NetWkstaUserEnum status reports.

The ObjectId field is correctly populated with computerSid in both failure (lines 232-233) and success (lines 245-246) status reports for privileged session enumeration.

---

296-303: LGTM: ObjectId propagation in RegistrySessionEnum status reports.

The ObjectId field is correctly populated with computerSid in both success (lines 301-302) and failure (lines 329-330) status reports for registry-based session enumeration.

Also applies to: 326-331

src/CommonLib/Processors/UserRightsAssignmentProcessor.cs (1)

60-65: LGTM: ObjectId propagation in URA status reports.

The ObjectId field is correctly populated with computerObjectId across all User Rights Assignment status reports, including LSAOpenPolicy (lines 63-64), LSAGetMachineSID (lines 82-83), and LSAEnumerateAccountsWithUserRight operations (lines 111-112, 127-128).

Also applies to: 79-84, 108-113, 124-129

src/CommonLib/Processors/CertAbuseProcessor.cs (1)

361-367: LGTM: ObjectId propagation in certificate abuse processor status reports.

The ObjectId field is correctly populated with computerObjectId in both SamConnect failure (lines 365-366) and GetMachineSid failure (lines 380-381) status reports.

Also applies to: 376-382

test/unit/SmbProcessorTest.cs (1)

44-44: LGTM: Test updated to match new Scan signature.

The test call has been correctly updated to pass an additional empty string argument, aligning with the updated SmbProcessor.Scan method signature that now accepts a securityIdentifier parameter.

src/CommonLib/Processors/SmbProcessor.cs (2)

28-28: LGTM: Method signature updated to accept securityIdentifier.

The Scan method signature correctly adds a securityIdentifier parameter to support ObjectId propagation in status reports.

---

32-37: LGTM: ObjectId propagation in SMB scan status reports.

The ObjectId field is correctly populated with securityIdentifier across all SMB scan status reporting paths: failure (lines 35-36), null result (lines 47-48), and success (lines 58-59).

Also applies to: 44-49, 55-60

src/CommonLib/Processors/LocalGroupProcessor.cs (1)

76-82: LGTM! Consistent ObjectId propagation across all status emissions.

All eight status emission sites in this processor consistently propagate computerObjectId to CSVComputerStatus.ObjectId. The changes are mechanical and align with the PR objective to add ObjectId tracking to computer status reports.

Also applies to: 96-102, 121-127, 143-149, 164-170, 197-203, 219-225, 235-241

src/CommonLib/Processors/LdapPropertyProcessor.cs (2)

268-273: LGTM! ObjectId correctly populated from resolved host SID.

The ObjectId assignment uses resolvedHost.SecurityIdentifier after successfully resolving the host via ResolveHostToSid(). The guard condition ensures the ObjectId is only populated when resolution succeeds and returns a valid SID.

---

384-389: LGTM! Consistent ObjectId propagation pattern.

This follows the same pattern as ReadUserProperties (lines 268-273), correctly assigning the resolved host's security identifier to ObjectId after successful resolution.

src/CommonLib/Processors/DCLdapProcessor.cs (2)

48-48: LGTM! Method signature updated to accept ObjectId.

The Scan method now accepts computerObjectId as a second parameter, enabling ObjectId tracking throughout the DC LDAP scanning process. This aligns with the broader PR changes to propagate ObjectId through status reports.

---

63-68: LGTM! Consistent ObjectId propagation across all DC LDAP scan status emissions.

All four status emissions (two for IsSigningRequired checks, two for IsChannelBindingDisabled checks) consistently propagate computerObjectId to CSVComputerStatus.ObjectId. The implementation correctly handles both success and failure paths.

Also applies to: 71-76, 80-85, 88-93

src/CommonLib/Processors/ComputerAvailability.cs (2)

63-66: LGTM! Optional ObjectId parameter maintains backward compatibility.

The method signature correctly adds objectId as an optional parameter (defaulting to null), which maintains backward compatibility while enabling ObjectId tracking when available. The overload at line 50 ensures result.ObjectId is passed when using ResolvedSearchResult.

---

70-75: LGTM! Consistent ObjectId propagation with proper null-safety.

All five status emissions consistently propagate the objectId parameter (which may be null) to CSVComputerStatus.ObjectId. The CSV serialization layer (see CSVComputerStatus.StringToCsvCell) correctly handles null values, so this is safe.

Also applies to: 86-91, 106-111, 120-125

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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