[wix-devs] Burn support for WM_SETTINGSCHANGED to refresh environment variables

Rob Mensching rob at firegiant.com
Thu Jul 7 11:59:34 PDT 2022 

> If we are deciding when to check the registry, then we are effectively declining the issue since it was requesting we update the environment whenever WM_SETTINGCHANGE is received.

The important part of the feature request is to allow "A chained package could modify the environment and if Burn refreshed its environment variables, subsequent executable packages could be created with the updated variables." https://github.com/wixtoolset/issues/issues/5981

WM_SETTINGCHANGE was the solution I proposed since my memory said that is what MSI broadcasts after it updates environment variables. I'm totally cool if the following (or something else) is a better solution:

> > A more targeted approach might be to add the ability to specify a 
> > specific environment variable that gets overwritten in the current 
> > process by the default after a package has run.


> I don't want to acknowledge that the user environment variables in the elevated process is something we should worry about.

Okay. I'm still uncertain what the guidelines are for elevated processes updating their environment block post-CreateProcess() but if others aren't concerned then I'll worry less.



-----Original Message-----
From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of Sean Hall via wix-devs
Sent: Thursday, July 7, 2022 9:25 AM
To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
Cc: Sean Hall <r.sean.hall at gmail.com>
Subject: Re: [wix-devs] Burn support for WM_SETTINGSCHANGED to refresh environment variables

I don't want to acknowledge that the user environment variables in the elevated process is something we should worry about. That would require more mitigation actions than we are currently doing such as purging the user environment variables from the process. If Windows includes user environment variables in the process when created from ShellExecute with "runas" then I don't think there's anything wrong with us updating them.

This feature was always about reading the new values from the registry. The WM_SETTINGCHANGE message with the "Environment" parameter contains no information except that the sender of that message thought that *something* changed in environment variables.

I find it interesting that you're starting to talk yourself into declining your own feature request. If we are deciding when to check the registry, then we are effectively declining the issue since it was requesting we update the environment whenever WM_SETTINGCHANGE is received.

On Thu, Jul 7, 2022 at 10:09 AM Rob Mensching <rob at firegiant.com> wrote:

> I was thinking about this a bit more and I think the elevated process 
> should only be updated with changes to the system environment changes.
> Otherwise, if (for some reason) a per-machine package in the bundle 
> used an environment variable for a "secure" purposes, a user 
> environment variable change could sneak into the per-machine execution.
>
> I think that means this feature boils down to using the Environment 
> registry key to update environment variables in the currently 
> executing Burn process. Right?
>
> If so, I like the proposal of having a list of "runtime updatable"
> Environment variables defined in the bundle. We could also exclude a 
> list of "important" environments variables (but that may be a slippery 
> slope to maintain).
>
> I think the "list of runtime updatable Environment variables" also 
> helps mitigate the issues of env var override behavior. If an 
> Environment variable is specified to be updatable, it'll be updated 
> during the execution of the bundle (overwriting any specially crafted 
> environment block when the Burn process was started).
>
> The "list of runtime updatable Environment variables" could also be 
> specified per-package to further narrow down the impact of changes. 
> This would really narrow how often the Environment registry key is checked.
>
>
> -----Original Message-----
> From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of 
> Sean Hall via wix-devs
> Sent: Monday, June 27, 2022 11:36 AM
> To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
> Cc: Sean Hall <r.sean.hall at gmail.com>
> Subject: Re: [wix-devs] Burn support for WM_SETTINGSCHANGED to refresh 
> environment variables
>
> I don't have a concrete scenario off the top of my head. It's the way 
> environment variables are supposed to work. System environment 
> variables are the defaults. User environment variables are layered on 
> top, overriding system environment variables if they are defined in 
> both places (with the exception of a small number of path variables 
> that get merged instead). New process environment blocks are supposed 
> to be created with those defaults and then the process can override 
> them if it wants to. If the process modified an environment variable 
> then in general that modified value shouldn't be overwritten if the default changed.
>
> On Mon, Jun 27, 2022 at 12:45 PM Rob Mensching <rob at firegiant.com> wrote:
>
> > > For example, let's say `MYVAR` has a default value of `1` but in 
> > > the
> > current process `MYVAR` has a local value of `2`. After receiving 
> > the `WM_SETTINGCHANGE` message, the default value is now `3`. There 
> > are going to be some cases where the local value of `2` needs to 
> > continue to override the default, while in other cases the new 
> > default of `3` is supposed to overwrite the local value.
> >
> > I was trying to think of situations where Burn should not accept a 
> > WM_SETTINGCHANGE update to its environment but wasn't sure what 
> > would be considered "bad". Do you have a concrete scenario where 
> > this is a
> problem?
> >
> >
> > -----Original Message-----
> > From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of 
> > Sean Hall via wix-devs
> > Sent: Friday, June 24, 2022 2:08 PM
> > To: WiX Toolset Developer Mailing List 
> > <wix-devs at lists.wixtoolset.org>
> > Cc: Sean Hall <r.sean.hall at gmail.com>
> > Subject: Re: [wix-devs] Burn support for WM_SETTINGSCHANGED to 
> > refresh environment variables
> >
> > This came across my radar when figuring out my options for working 
> > around https://github.com/dotnet/runtime/issues/68180. There are 
> > various other projects that have looked into keeping the environment 
> > block of the current process up to date:
> >
> > https://github.com/chocolatey/choco/blob/f924d47fb4177a9a34ff0c2bf99
> > 59 38b5c12800b/src/chocolatey.resources/redirects/RefreshEnv.cmd
> > ,
> > https://github.com/Maximus5/ConEmu/issues/468,
> > https://github.com/microsoft/terminal/issues/1125,
> > https://github.com/microsoft/terminal/issues/9741. There's basically 
> > two options - throw away the current environment block and get a 
> > fresh one or merge the changes that were made with the current one.
> >
> > The first option of getting a fresh one has a fatal flaw - there's 
> > no documented way to get a fresh one with all the variables that 
> > would normally be set when launching from explorer. 
> > ::CreateEnvironmentBlock is the documented way to create a fresh one 
> > but it doesn't include
> everything.
> > If you get this wrong then parts of Windows might not work 
> > correctly, for example [The case of the 
> > SHGet­Folder­Path(CSIDL_COMMON_DOCUMENTS)
> > that returned ERROR_PATH_NOT_FOUND]( 
> > https://devblogs.microsoft.com/oldnewthing/20200520-00/?p=103775).
> > There is an undocumented shell32 function 
> > `RegenerateUserEnvironment` that is explained at
> https://textslashplain.com/2018/10/11/shellexecute-doesnt/.
> >
> > The other problem with the fresh option is that it's probably not 
> > the right thing to do. The user may have set an environment variable 
> > that it wanted to propagate for the entire lifetime of the process 
> > but this will undo it.
> >
> > The second option of merging the changes has the same issue. For 
> > example, let's say `MYVAR` has a default value of `1` but in the 
> > current process `MYVAR` has a local value of `2`. After receiving 
> > the `WM_SETTINGCHANGE` message, the default value is now `3`. There 
> > are going to be some cases where the local value of `2` needs to 
> > continue to override the default, while in other cases the new 
> > default of `3` is supposed to overwrite the local value.
> >
> > There is theoretically a third option here - add an attribute to 
> > executable packages to make Burn provide a fresh environment block 
> > to ::CreateProcessW. Of course, this has the same problem as the 
> > first option in that there's no documented way to get a fresh 
> > environment block. And it would mean that the executable would never 
> > get environment variables from the Burn process, though that could 
> > be mitigated if users could explicitly pass certain environment 
> > variables (by implementing https://github.com/wixtoolset/issues/issues/2697).
> > Note that this only would help executable packages and not 
> > `MsiPackage`
> or `MspPackage`.
> >
> > A more targeted approach might be to add the ability to specify a 
> > specific environment variable that gets overwritten in the current 
> > process by the default after a package has run.
> >
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant 
> http://www.firegiant.com/
>
____________________________________________________________________
WiX Toolset Developer Mailing List provided by FireGiant http://www.firegiant.com/

More information about the wix-devs mailing list