Always create a new environment block before we spawn a process (#7243)

This commit ensures that we always furnish a new process with the
cleanest, most up-to-date environment variables we can. There is a minor
cost here in that WT will no longer pass environment variables that it
itself inherited to its child processes.

This could be considered a reasonable sacrifice. It will also remove
somebody else's TERM, TERM_PROGRAM and TERM_PROGRAM_VERSION from the
environment, which could be considered a win.

I validated  that GetCurrentProcessToken returns a token we're
_technically able_ to use with this API; it is roughly equivalent to
OpenProcessToken(GetCurrentProcess) in that it returns the current
active _access token_ (which is what CreateEnvironmentBlock wants.)

There's been discussion about doing a 3-way merge between WT's
environment and the new one. This will be complicated and I'd like to
scream test the 0-way merge first ;P

Related to #1125 (but it does not close it or resolve any of the other
issues it calls out.)

Fixes #7239
Fixes #7204 ("App Paths" value creeping into wt's environment)

.github/actions/spell-check/dictionary/apis.txt

@@ -4,6 +4,7 @@ alignof
 bitfield
 bitfields
 CLASSNOTAVAILABLE
+environstrings
 EXPCMDFLAGS
 EXPCMDSTATE
 fullkbd
@@ -46,3 +47,4 @@ STDCPP
 syscall
 tmp
 tx
+userenv

src/cascadia/TerminalConnection/ConptyConnection.cpp

@@ -10,6 +10,7 @@
 #include "ConptyConnection.h"
 
 #include <windows.h>
+#include <userenv.h>
 
 #include "ConptyConnection.g.cpp"
 
@@ -97,8 +98,11 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
             environment.clear();
         });
 
-        // Populate the environment map with the current environment.
-        RETURN_IF_FAILED(Utils::UpdateEnvironmentMapW(environment));
+        {
+            const auto newEnvironmentBlock{ Utils::CreateEnvironmentBlock() };
+            // Populate the environment map with the current environment.
+            RETURN_IF_FAILED(Utils::UpdateEnvironmentMapW(environment, newEnvironmentBlock.get()));
+        }
 
         {
             // Convert connection Guid to string and ignore the enclosing '{}'.

src/types/Environment.cpp

@@ -9,30 +9,42 @@ using namespace ::Microsoft::Console::Utils;
 // We cannot use spand or not_null because we're dealing with \0\0-terminated buffers of unknown length
 #pragma warning(disable : 26481 26429)
 
+// Function Description:
+// - Wraps win32's CreateEnvironmentBlock to return a smart pointer.
+EnvironmentBlockPtr Microsoft::Console::Utils::CreateEnvironmentBlock()
+{
+    void* newEnvironmentBlock{ nullptr };
+    if (!::CreateEnvironmentBlock(&newEnvironmentBlock, GetCurrentProcessToken(), FALSE))
+    {
+        return nullptr;
+    }
+    return EnvironmentBlockPtr{ newEnvironmentBlock };
+}
+
 // Function Description:
 // - Updates an EnvironmentVariableMapW with the current process's unicode
 //   environment variables ignoring ones already set in the provided map.
 // Arguments:
 // - map: The map to populate with the current processes's environment variables.
+// - environmentBlock: Optional environment block to use when filling map. If omitted,
+//   defaults to the current environment.
 // Return Value:
 // - S_OK if we succeeded, or an appropriate HRESULT for failing
-HRESULT Microsoft::Console::Utils::UpdateEnvironmentMapW(EnvironmentVariableMapW& map) noexcept
+HRESULT Microsoft::Console::Utils::UpdateEnvironmentMapW(EnvironmentVariableMapW& map, void* environmentBlock) noexcept
 try
 {
-    LPWCH currentEnvVars{};
-    auto freeCurrentEnv = wil::scope_exit([&] {
-        if (currentEnvVars)
-        {
-            FreeEnvironmentStringsW(currentEnvVars);
-            currentEnvVars = nullptr;
-        }
-    });
+    wchar_t const* activeEnvironmentBlock{ static_cast<wchar_t const*>(environmentBlock) };
 
-    currentEnvVars = ::GetEnvironmentStringsW();
-    RETURN_HR_IF_NULL(E_OUTOFMEMORY, currentEnvVars);
+    wil::unique_environstrings_ptr currentEnvVars;
+    if (!activeEnvironmentBlock)
+    {
+        currentEnvVars.reset(::GetEnvironmentStringsW());
+        RETURN_HR_IF_NULL(E_OUTOFMEMORY, currentEnvVars);
+        activeEnvironmentBlock = currentEnvVars.get();
+    }
 
     // Each entry is NULL-terminated; block is guaranteed to be double-NULL terminated at a minimum.
-    for (wchar_t const* lastCh{ currentEnvVars }; *lastCh != '\0'; ++lastCh)
+    for (wchar_t const* lastCh{ activeEnvironmentBlock }; *lastCh != '\0'; ++lastCh)
     {
         // Copy current entry into temporary map.
         const size_t cchEntry{ ::wcslen(lastCh) };

src/types/inc/Environment.hpp

@@ -21,9 +21,12 @@ namespace Microsoft::Console::Utils
         }
     };
 
+    using EnvironmentBlockPtr = wil::unique_any<void*, decltype(::DestroyEnvironmentBlock), ::DestroyEnvironmentBlock>;
+    [[nodiscard]] EnvironmentBlockPtr CreateEnvironmentBlock();
+
     using EnvironmentVariableMapW = std::map<std::wstring, std::wstring, WStringCaseInsensitiveCompare>;
 
-    [[nodiscard]] HRESULT UpdateEnvironmentMapW(EnvironmentVariableMapW& map) noexcept;
+    [[nodiscard]] HRESULT UpdateEnvironmentMapW(EnvironmentVariableMapW& map, void* environmentBlock = nullptr) noexcept;
 
     [[nodiscard]] HRESULT EnvironmentMapToEnvironmentStringsW(EnvironmentVariableMapW& map,
                                                               std::vector<wchar_t>& newEnvVars) noexcept;

src/types/precomp.h

@@ -29,6 +29,7 @@ Abstract:
 
 // Windows Header Files:
 #include <windows.h>
+#include <userenv.h>
 #include <combaseapi.h>
 #include <UIAutomation.h>
 #include <objbase.h>