REG_EXPAND_SZ environment variables not properly expanded for shells

[chrisdjali-wrld3d]

Windows Terminal version (or Windows build number)

1.7.572.0

Other Software

No response

Steps to reproduce

1. Set a system environment variable including %USERPROFILE%, e.g. TestVariable set to %USERPROFILE%\bin.
2. In Regedit, confirm that the type is REG_EXPAND_SZ by finding it under HKLM\SYSTEM\CurrentControlSet\Control\Session Manager\Environment.
3. Start PowerShell or CMD normally, e.g. from the WinX menu, or Shift+RMB in Windows Explorer.
4. Print the variable's value, e.g. echo $env:TestVariable.
5. Observe that %USERPROFILE% has been expanded, e.g. C:\Users\Chris\bin is printed.
6. Open Windows Terminal and create a new PowerShell or CMD tab.
7. Print the variable's value, e.g. echo $env:TestVariable.
8. Observe that %USERPROFILE% has not been expanded, e.g. %USERPROFILE%\bin is printed.

Expected Behavior

Like with standalone PowerShell or CMD via CONHOST, the environment variable is properly expanded.

Actual Behavior

The unexpanded value is used.

This isn't happening with every environment variable - others get expanded correctly, like windir being %SystemRoot% becoming C:\WINDOWS. My suspicion is that it's just variables not defined in the system scope not being expanded if they're used in the system scope.

I'm pretty sure this is Windows Terminal doing something weird rather than Windows itself as when viewing the WindowsTerminal.exe process' environment in Process Explorer, the variables are properly expanded, but when doing the same for the actual shell process, they aren't. If I had to guess, I'd say maybe Windows Terminal is attempting to read the up-to-date environment from the registry when starting new shells instead of letting them inherit its potentially outdated environment, but is doing so inconsistently with how Windows does it. If that's actually what's happening, it may be sensible to use Chocolatey's RefreshEnv batch script or PowerShell module as a reference implementation as that gets the same result as Windows.

This is happening in MSYS-esque Bash shells, too, but the Windows-to-Unix path conversion seems to be doing another round of expansion that masks the symptoms.

[DHowett]

So, this isn’t the first issue with our environment block. Thanks for reporting it!

We’re using the Win32 API that generates a new environment block and not doing anything special on top of that. It is rather alarming, but somewhat par for the course, that it doesn’t work quite right.

We probably need to do more things manually instead of fewer things, because the API fixtures available to us do those things wrong.

[eryksun]

A system variable can't reliably refer to a user variable. This is going to be unreliable in many cases. It works with the shell APIs private shell32!RegenerateUserEnvironment() function because it's explicitly for the current user. But other cases -- in particular CreateEnvironmentBlock() without inheritance -- load the environment in a systematic way that does not allow a system variable to refer to a user variable.

First load system variables:

• inherit SystemRoot and SystemDrive from the current process
• load ProgramData, ALLUSERSPROFILE, and PUBLIC from the registry
• load REG_SZ values from "HKLM\SYSTEM\CurrentControlSet\Control\Session Manager\Environment"
• load REG_EXPAND_SZ values from the above key
• set COMPUTERNAME from GetComputerNameExW()
• load ProgramFiles and CommonProgramFiles (and x86 and W6432 variants) from the registry

Next load user variables:

• load USERPROFILE, APPDATA, and LOCALAPPDATA from the registry based on the user SID
• load REG_SZ values from "HKU\<SID string>\Environment"
• load REG_EXPAND_SZ values from "HKU\<SID string>\Environment"
• append user Path, LibPath, and Os2LibPath variables to corresponding system value, if any
• replace TEMP and TMP with short paths via GetShortPathNameW()
• load volatile values for the logon session from "HKU\<SID string>\Volatile Environment": LOGONSERVER, USERNAME, USERDOMAIN, HOMEDRIVE, HOMEPATH, USERPROFILE, APPDATA, LOCALAPPDATA
• load volatile values for the Windows session from "HKU\<SID string>\Volatile Environment\<session ID>": SESSION_NAME, CLIENT_NAME
• set USERNAME from GetUserNameExW() (e.g. SYSTEM logons use "%COMPUTERNAME%$", not "SYSTEM")

At each step it's only reliable to refer to variables that were loaded in a previous step. For example user REG_EXPAND_SZ variables can reliably refer to system variables, user REG_SZ variables, USERPROFILE, APPDATA, and LOCALAPPDATA. They cannot reliably refer to other user REG_EXPAND_SZ variables (registry key enumeration order is arbitrary -- I think in creation order) or volatile variables such as USERNAME and HOMEDRIVE.

These limitations have to be abided if you want the user's environment to be correct in contexts such as a process spawned with "runas.exe" or Task Scheduler, and various other cases that use CreateProcessWithLogonW(), CreateProcessAsUserW(), and so on, that need to create a new environment block.

I'd prefer a more reasonable and useful load order. It's always annoyed me that I can't reliably use ProgramFiles in system environment variables or USERNAME in user environment variables. For system variables, I'd load COMPUTERNAME, ProgramFiles, CommonProgramFiles (and variants) before loading the system REG_SZ and REG_EXPAND_SZ values from the system "Environment" key. For user variables, I'd load the user "Volatile Environment" before the persistent user "Environment". That said, I would never expect to be able to reliably use USERPROFILE in a system variable.

[chrisdjali-wrld3d]

When I originally set this up, the goal was to have a user-specific bin directory prepended onto the path, which can only be done via the system path as the user path gets appended. I don't think I necessarily expected it to work, but nothing that came up in a Google search implied it wouldn't (although it was the kind of search where it's a specific query and all the results are generic guides to the basics), and it did when I tried. It wasn't inconceivable that everything would be loaded from the registry before any substitution was done, and therefore this was expected to work.

As this is a single-user machine, I can easily work around the problem by hardcoding the path, but I imagine there are other cases where people have done something similar without knowing it only worked because they'd only tried it in the large subset of cases where it didn't matter. I guess whether Windows Terminal 'fixes' this should depend on whether the goal is to make everything that used to work in CONHOST work or whether anyone doing anything sketchy should be forced to start doing it properly.

[eryksun]

I guess whether Windows Terminal 'fixes' this should depend on whether the goal is to make everything that used to work in CONHOST work or whether anyone doing anything sketchy should be forced to start doing it properly.

I don't know the reason why they need a new environment block as opposed to creating a copy of the current environment that's returned by GetEnvironmentStrings.

It wasn't inconceivable that everything would be loaded from the registry before any substitution was done, and therefore this was expected to work.

Unfortunately, the order in which environment variables are loaded and expanded is not documented and not consistent in the system.

Check that the value is set as expected with a fresh boot, and that it's not nonsense in system processes that inherit and extend the environment as the system boots (smss.exe, csrss.exe, wininit.exe, services.exe, winlogon.exe). Also, make sure that the desktop shell reloads the right value after setting a new value -- i.e. that it's compatible with RegenerateUserEnvironment in shell32.dll. And make sure that it works with CreateEnvironmentBlock, the documented way to create a new environment block. The latter can be checked with runas.exe /user:%username% "cmd.exe /k set".

When I originally set this up, the goal was to have a user-specific bin directory prepended onto the path, which can only be done via the system path as the user path gets appended.

Or rather as close as one can get to 'prepended', reliably in practice. The default search paths for CreateProcessW() and SearchPathW() always check the application directory and system directories before PATH.

[chrisdjali-wrld3d]

I guess whether Windows Terminal 'fixes' this should depend on whether the goal is to make everything that used to work in CONHOST work or whether anyone doing anything sketchy should be forced to start doing it properly.

I don't know the reason why they need a new environment block as opposed to creating a copy of the current environment that's returned by GetEnvironmentStrings.

There's one Windows Terminal process that's the parent of all the tabs, so GetEnvironmentStrings would make all the shells have whichever environment variables were set when the parent terminal was launched, not what was set when each shell was launched. Presumably someone decided this was worse than having it refresh them before launching each shell.

Check that the value is set as expected with a fresh boot, and that it's not nonsense in system processes that inherit and extend the environment as the system boots (smss.exe, csrss.exe, wininit.exe, services.exe, winlogon.exe).

It's survived at least a year of reboots without me noticing problems outside Windows Terminal, so it's at least consistent nonsense.

Also, make sure that the desktop shell reloads the right value after setting a new value -- i.e. that it's compatible with RegenerateUserEnvironment in shell32.dll.

Yep, if I edit the value in Advanced System Settings and launch a fresh PowerShell, the variable gets expanded 'properly'.

And make sure that it works with CreateEnvironmentBlock, the documented way to create a new environment block. The latter can be checked with runas.exe /user:%username% "cmd.exe /k set".

I see the same behaviour as Windows Terminal, i.e. %USERPROFILE% is not expanded. I also notice that the %PROGRAMFILES% in PSModulePath isn't expanded, either, but PowerShell seems to do that itself when it's launched, either directly via runas instead of CMD or from within the CMD shell.

When I originally set this up, the goal was to have a user-specific bin directory prepended onto the path, which can only be done via the system path as the user path gets appended.

Or rather as close as one can get to 'prepended', reliably in practice. The default search paths for CreateProcessW() and SearchPathW() always check the application directory and system directories before PATH.

Being Windows, it's more complicated than even that - things like the registry's list of known DLLs get involved before searching is even attempted. For what I needed, what I did was enough, though.

Anyhow, while looking into this, I noticed that I've also got %USERPROFILE%\.dnx\bin in my system path, and looking at its position, it looks like it was put there by the Visual Studio 2015 installer. I can be pretty confident of that as this machine was mostly set up by a script. It's never mattered as it either didn't install DNVM or installed it elsewhere, but it's evidence that I'm not the only person who's tried doing this.

[DHowett]

I don't know the reason why they need a new environment block as opposed to creating a copy of the current environment that's returned by GetEnvironmentStrings.

Users continually complain that shells spawned by Terminal do not reflect the most up-to-date system environment. We don't usually do work without reason 😉

[eryksun]

GetEnvironmentStrings would make all the shells have whichever environment variables were set when the parent terminal was launched, not what was set when each shell was launched.

I should have seen that this was the reason.

If calling the undocumented function RegenerateUserEnvironment is off limits, then I don't see an easy way to make it agree with Explorer.

CreateEnvironmentBlock, the documented way to create a new environment block. The latter can be checked with runas.exe /user:%username% "cmd.exe /k set".

I see the same behaviour as Windows Terminal, i.e. %USERPROFILE% is not expanded.

Yes, both use CreateEnvironmentBlock. I meant this as a general test to quickly see if a variable will be okay in such cases.

I'd prefer a new CreateEnvironmentBlockEx function that's at least more consistent with RegenerateUserEnvironment. A creation flag could be added to get a new environment block with CreateProcessW, CreateProcessAsUserW, and CreateProcessWithLogonW when lpEnvironment is NULL.

%PROGRAMFILES% in PSModulePath isn't expanded, either, but PowerShell seems to do that itself

Since PowerShell modifes the value of PSModulePath, it doesn't surprise me that it expands it as well. However, it doesn't expand every variable in the entire environment.

Being Windows, it's more complicated than even that - things like the registry's list of known DLLs get involved before searching is even attempted. For what I needed, what I did was enough, though.

I didn't think about DLLs. It has been a while since I used PATH for that.

[DHowett]

The current behavior is definitely by design for when we regenerate a new environment block, but if other applications seem to handle this then I think we should take a crack at it.

For reference, linking this up to #9233, #7418, #7204 for environment shenanigans. I suspect that we'll have to do something about them sooner rather than later.