Fix Windows ownership resolution using Win32_UserProfile SID translation#68
Merged
travis-hoover-glean merged 3 commits intoApr 14, 2026
Merged
Conversation
resolveProfileOwner() was using Get-Acl to read the ACL owner of the user's home directory, which returns NT AUTHORITY\SYSTEM on provisioned machines where the profile directory is owned by SYSTEM. Since this is a truthy value, the ?? username fallback never triggers, causing all config files to be owned by SYSTEM instead of the target user. Replace with Win32_UserProfile CIM query that resolves the user SID from the Windows profile registry and translates it to an NTAccount name. This is the authoritative source for profile-to-user mapping regardless of directory ACL ownership. Fixes: PROD-23734
The E2E config test was using Get-Acl on the home directory to determine the expected file owner, which returns NT AUTHORITY\SYSTEM on CI runners. Update to use Win32_UserProfile SID translation, matching the production code fix. Also add unit tests for resolveProfileOwner to guard against regression back to the Get-Acl approach.
Export setOwnerWindowsBatch for testing and add 9 new test cases: resolveProfileOwner: - Verify home directory path is passed in the PowerShell command - Return null when no Win32_UserProfile matches the path - Return null on PowerShell timeout (ETIMEDOUT) setOwnerWindowsBatch: - No-op when paths array is empty - Construct correct Set-Acl command for a single path - Batch all paths into a single PowerShell call - Escape single quotes in owner name - Escape single quotes in file paths - Log warning without throwing when PowerShell fails
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
resolveProfileOwner()was usingGet-Aclto read the ACL owner of the user's home directory, which returnsNT AUTHORITY\SYSTEMon provisioned/managed machines where the profile directory is owned by SYSTEM. Since this is a truthy value, the?? usernamefallback never triggers, causing all config files and directories to be owned by SYSTEM instead of the target user.Win32_UserProfileCIM query that resolves the user SID from the Windows profile registry and translates it to an NTAccount name.Before:
After:
Test plan
TRAVISHOOVE8C46\thoov