Conversation
- Added logic to conditionally update the user PATH variable only when not running as an admin. - Implemented a new function to execute PowerShell commands as the original user to ensure proper PATH updates. - Updated registry settings to prevent multiple installations from incorrectly modifying the PATH for elevated users.
There was a problem hiding this comment.
PR Summary:
This PR enhances environment variable handling in pyRevit installer scripts to properly manage PATH updates when installers run with elevated admin privileges. Key changes:
- Adds conditional logic to skip Registry-based PATH updates for admin users
- Implements
ExecAsOriginalUserto execute PowerShell commands as the original user - Prevents elevated installations from incorrectly modifying the admin user's PATH instead of the actual user's PATH
Review Summary:
Reviewed installer scripts for proper PATH handling with admin privileges. The approach of using ExecAsOriginalUser for elevated installations is correct and addresses a real issue where admin-elevated installers would modify the wrong user's environment. However, identified critical PATH duplication bugs that will cause repeated PATH entries on reinstalls/updates, plus silent failure handling that could confuse users.
Critical issues found:
- Both PowerShell and Registry PATH updates lack duplicate checking, causing PATH pollution on every reinstall
- Silent failures provide no user notification when PATH updates fail
All comments include actionable fixes with code examples.
Follow-up suggestions:
There was a problem hiding this comment.
Pull request overview
This pull request addresses a critical issue where per-user installers (pyRevit_*_signed.exe and pyRevit_CLI_*_signed.exe) incorrectly update the PATH environment variable when run with administrator privileges. When elevated, HKCU points to the administrator's registry instead of the actual user's, causing the pyrevit command to be unavailable in non-elevated command prompts.
Changes:
- Modified installer scripts to conditionally update PATH based on elevation status
- Added PowerShell-based PATH update mechanism for elevated installations using
ExecAsOriginalUser - Introduced
NotAdmincheck function to prevent registry writes when running elevated
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| release/pyrevit.iss | Added NotAdmin check to Registry PATH entry, implemented CurStepChanged procedure to update PATH via PowerShell when running as admin |
| release/pyrevit-cli.iss | Identical changes to pyrevit.iss - added NotAdmin check and PowerShell-based PATH update for elevated installations |
- Renamed function to improve clarity and added logic to check if the PATH variable already includes the application path before updating. - Enhanced error handling to notify users if the installer fails to update the user PATH environment variable. - Updated registry settings to ensure proper PATH modifications only occur when not running as an admin.
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1433-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1538-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1543-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1553-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1612-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1624-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1738-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1743-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1829-wip |
|
📦 New work-in-progress (wip) builds are available for 5.3.1.26032+1937-wip |
|
📦 New work-in-progress (wip) builds are available for 6.0.0.26032+1956-wip |
|
📦 New work-in-progress (wip) builds are available for 6.0.0.26032+2005-wip |
|
📦 New work-in-progress (wip) builds are available for 6.0.0.26032+2008-wip |
|
📦 New public release are available for 6.0.0.26032+2040 |
|
📦 New public release are available for 6.0.0.26032+2040 |
Per-user installers (
pyRevit_*_signed.exeandpyRevit_CLI_*_signed.exe) write the install path toHKCU\Environment\Path. When the user runs the installer "Run as administrator", the process runs elevated and HKCU refers to the elevated user's registry, so PATH is updated only for the admin account andpyrevitis not found in a normal (non-admin) command prompt.Changes
Check: NotAdminto the[Registry]PATH entry so the HKCU write runs only when the installer is not elevated. When the installer is elevated (IsAdmin),CurStepChanged(ssPostInstall)usesExecAsOriginalUserto run PowerShell and append{app}\binto the original (logged-in) user's PATH. Added a short comment in the Registry section explaining this.Result
Running the per-user CLI or full pyRevit installer as administrator now updates PATH for the user who started the installer, so
pyrevitworks in a regular (non-admin) cmd. Normal (non-elevated) installs are unchanged.