-
Notifications
You must be signed in to change notification settings - Fork 362
Fix Add-MachinePath to preserve registry value type and unexpanded variables #819
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…riables Refactor Add-MachinePath to avoid unintended modifications to the PATH registry value: - Use GetValue() with DoNotExpandEnvironmentNames to preserve unexpanded environment variables (e.g., %SystemRoot%) - Preserve original registry value type (REG_SZ vs REG_EXPAND_SZ) using GetValueKind() - Keep original PATH entry values unchanged; normalization (expansion + backslash trimming) only used for duplicate detection - Improve ShouldProcess implementation with proper message/prompt/description parameters for better -WhatIf/-Confirm support - Enhance duplicate detection to compare expanded values, catching duplicates like C:\Windows\System32 vs %SystemRoot%\System32
|
@microsoft-github-policy-service agree |
There was a problem hiding this 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 the Add-MachinePath function to properly preserve the PATH registry value type and unexpanded environment variables when adding new paths to the machine PATH.
Key Changes:
- Uses
GetValue()withDoNotExpandEnvironmentNamesflag to preserve unexpanded variables (e.g.,%SystemRoot%) - Preserves the original registry value type (
REG_SZvsREG_EXPAND_SZ) when updating PATH - Implements path normalization (expansion + backslash trimming) for duplicate detection without modifying original entries
- Adds proper
ShouldProcesssupport for-WhatIfand-Confirmfunctionality
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
tgauth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @emilvissing!
PR Summary
Fix
Add-MachinePathfunction to preserve PATH registry value type and unexpanded environment variables when adding new paths.PR Context
Existing implementation has several issues:
%SystemRoot%becomesC:\Windows)REG_SZvsREG_EXPAND_SZ)Changes
GetValue()withDoNotExpandEnvironmentNamesto preserve unexpanded variablesShouldProcessfor-WhatIf/-ConfirmsupportSet-ItemPropertyparameter (see first commit)