Skip to content

Change issues with empty SID and Exception calling "Translate" with "1" argument(s): "Some or all identity references could not be translated."#257

Closed
mstraessner wants to merge 5 commits intojakehildreth:testingfrom
mstraessner:Testing
Closed

Change issues with empty SID and Exception calling "Translate" with "1" argument(s): "Some or all identity references could not be translated."#257
mstraessner wants to merge 5 commits intojakehildreth:testingfrom
mstraessner:Testing

Conversation

@mstraessner
Copy link
Copy Markdown

@mstraessner mstraessner commented Jul 3, 2025

Your ESC analysis script has helped my customer, my company, and me a lot – thank you!

Yesterday, I encountered the following error in a German AD environment:
Exception calling "Translate" with "1" argument(s): "Some or all identity references could not be translated."
Today I tested it in my demo lab.

I found this:

image
image

After applying my changes, the issue was resolved.

image

I didn’t catch the "CA Unavailable" issue in the script – I didn’t dive too deep into error handling there.

jakehildreth and others added 2 commits July 2, 2025 21:13
@jakehildreth
Copy link
Copy Markdown
Owner

Thank you! This is wonderful! I'll review over the weekend.

@jakehildreth
Copy link
Copy Markdown
Owner

Oops! Forgot to review. Soon!

@jakehildreth jakehildreth changed the base branch from main to testing July 7, 2025 19:58
@mstraessner
Copy link
Copy Markdown
Author

It happens 😉 I’m looking forward to your feedback and result!

@jakehildreth jakehildreth self-assigned this Jul 22, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a PowerShell translation error that occurs when trying to convert empty or invalid SIDs in a German AD environment. The changes prevent the "Some or all identity references could not be translated" exception by adding proper validation checks.

  • Adds SID validation to prevent empty SID processing in Set-RiskRating.ps1
  • Adds domain filtering to only process relevant administrators in Find-ESC7.ps1
  • Minor documentation formatting change

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
Private/Set-RiskRating.ps1 Adds null check for SID before processing ESC7 issues
Private/Find-ESC7.ps1 Adds domain filtering for admin processing to avoid invalid SIDs
Docs/Flowcharts/ESC1.md Adds blank line at beginning of file

$IdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $SID } | Select-Object objectClass

# Check to make sure $SID isn't null.
if (!$SID) {
Copy link

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

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

The null check logic is inverted. This condition will skip processing when SID is null/empty, but the code inside the block expects to process a valid SID. The condition should be 'if ($SID)' to only process when SID has a value.

Suggested change
if (!$SID) {
if ($SID) {

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@mstraessner this one actually seems correct. Can you explain your use of negation here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In some customer environments and in my test Active Directory environment, the $SID variable was empty, which caused the translation error.

Comment thread Private/Find-ESC7.ps1
Comment thread Private/Find-ESC7.ps1
@jakehildreth
Copy link
Copy Markdown
Owner

After spending some time reviewing this PR, I am going to send it back to the drawing board. Can you first focus your efforts on updating Convert-IdentityReferenceToSid (called in Find-ESC7) so it returns the correct information? If you'd like to have a call to discuss what I'm talking about, please let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants