fix: Process objectguid as common property - BED-7397

[StephenHinck]

Description

Adds ObjectGUID processing support as a common property

Motivation and Context

Resolves: BED-7397

How Has This Been Tested?

Help me team!

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

• Bug Fixes

  • GUID handling for LDAP properties is more consistent: duplicate extraction removed, malformed GUID bytes skipped, and object GUIDs are recorded reliably—reducing related errors.

• Tests

  • Two unit tests updated to run only on Windows environments to reflect platform-specific behavior.

[coderabbitai]

Walkthrough

Centralizes ObjectGUID extraction into GetCommonProps in LdapPropertyProcessor; when a 16-byte ObjectGUID is present it is converted to an uppercase GUID string and stored; malformed byte arrays are ignored. Also narrows two unit tests to Windows-only.

Changes

Cohort / File(s)	Summary
Ldap GUID handling src/CommonLib/Processors/LdapPropertyProcessor.cs	Added centralized parsing of 16-byte ObjectGUID in GetCommonProps to construct an uppercase GUID string and store it; removed duplicated extraction from ReadComputerProperties; malformed GUID byte arrays are skipped.
Windows-only tests test/unit/CommonLibHelperTests.cs	Replaced [Theory]/[Fact] with [WindowsOnlyTheory]/[WindowsOnlyFact] on two SID-related tests to restrict them to Windows environments.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Bed 6619 collection of objectguid #253: Modifies LDAP ObjectGUID handling in LdapPropertyProcessor, directly related to GUID parsing/storage changes.

Suggested labels

bug

Suggested reviewers

• MikeX777

Poem

🐇 I munched through bytes and found a clue,
A GUID in order, shiny and new.
I hopped it to common ground with cheer,
Now every prop can read it clear.
Hooray for tidy hops and code — nibble, nibble, woo!

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description covers most required sections but has a critical gap: the testing section states 'Help me team!' instead of providing actual test details, leaving testing validation incomplete.	Provide detailed testing information explaining how ObjectGUID processing was verified, what test cases were executed, and confirm the specific tests that passed.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: centralizing ObjectGUID processing as a common property in the LDAP processor.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into v4

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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-7397

---

No actionable comments were generated in the recent review. 🎉

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/CommonLib/Processors/LdapPropertyProcessor.cs`:
- Around line 56-63: The code is invoking a dictionary entry as a function at
ret["objectguid"](LDAPProperties.ObjectGUID, guid.ToString().ToUpper()); —
replace that with a direct assignment to the "objectguid" dictionary key so the
value stores the GUID string (formatted with ToUpper()) and includes the
LDAPProperties.ObjectGUID info as the value payload as your dictionary type
expects; locate this in LdapPropertyProcessor inside the
entry.TryGetByteProperty(LDAPProperties.objectguid, out var objectguid) block
and change the invocation to a proper assignment (keeping the key "objectguid"
unchanged so downstream consumers like SharpHound rely on the same key).

[StephenHinck]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.