Slim down shell extension and elevate-shim (#15327)

This simplifies the code (from the perspective of the CPU) by doing
some miniscule-feels-good optimizations like replacing `snprintf` with
regular string concatenation and by doing an actual optimization by
removing the remaining calls to the WinRT `ApplicationModel` namespace.

More importantly however it fixes a bug: The only reason `elevate-shim`
worked at all is because the shell extension passed "wrong" parameters
to `CreateProcess`. Instead of repeating the application path in the
command line argument again, as is convention in C and on Windows, and
getting the 2nd and following parameters as an argument to `wWinMain`,
it used `GetCommandLineW` to get the original, broken command line.
This fixes the issue by passing the application path as the first
argument, which allows `elevate-shim` to be called like any other app.

## Validation Steps Performed
* Deploy WT and restart explorer
* Clicking "Open in Terminal (Dev)" works ✅
* Clicking "Open in Terminal (Dev)" while holding Ctrl+Shift
  opens WT as admin ✅

src/cascadia/ElevateShim/elevate-shim.cpp

@@ -1,16 +1,13 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-#include <string>
+#define WIN32_LEAN_AND_MEAN
+
 #include <filesystem>
 
-#define WIN32_LEAN_AND_MEAN
-#include <windows.h>
 #include <wil/stl.h>
-#include <wil/resource.h>
 #include <wil/win32_helpers.h>
-#include <gsl/gsl_util>
-#include <gsl/pointers>
+
 #include <shellapi.h>
 #include <appmodel.h>
 
@@ -27,10 +24,13 @@
 // wants elevated. We'll hang around until ShellExecute is finished, so that the
 // process can successfully elevate.
 
-#pragma warning(suppress : 26461) // we can't change the signature of wWinMain
-int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int)
+// we can't change the signature of wWinMain
+#pragma warning(suppress : 26461) // The pointer argument 'cmdline' for function 'wWinMain' can be marked as a pointer to const (con.3).
+int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR cmdline, int)
 {
-    // This will invoke an elevated terminal in two possible ways. See GH#14501
+    // This will invoke an elevated terminal in two possible ways. We need to do this,
+    // because ShellExecuteExW() fails to work if we're running elevated and
+    // the given executable path is a packaged application. See GH#14501.
     // In both scenarios, it passes the entire cmdline as-is to the new process.
     //
     // #1 discover and invoke the app using the GetCurrentApplicationUserModelId
@@ -42,38 +42,26 @@ int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int)
     //    cmd:    {same path as this binary}\WindowsTerminal.exe
     //    params: new-tab -p {guid}
 
-    // see if we're a store app we can invoke with shell:AppsFolder
-    std::wstring appUserModelId;
-    const auto result = wil::AdaptFixedSizeToAllocatedResult<std::wstring, APPLICATION_USER_MODEL_ID_MAX_LENGTH>(
-        appUserModelId, [&](PWSTR value, size_t valueLength, gsl::not_null<size_t*> valueLengthNeededWithNull) noexcept -> HRESULT {
-            UINT32 length = gsl::narrow_cast<UINT32>(valueLength);
-            const LONG rc = GetCurrentApplicationUserModelId(&length, value);
-            switch (rc)
-            {
-            case ERROR_SUCCESS:
-                *valueLengthNeededWithNull = length;
-                return S_OK;
+    std::wstring cmd;
 
-            case ERROR_INSUFFICIENT_BUFFER:
-                *valueLengthNeededWithNull = length;
-                return S_FALSE; // trigger allocation loop
-
-            case APPMODEL_ERROR_NO_APPLICATION:
-                return E_FAIL; // we are not running as a store app
-
-            default:
-                return E_UNEXPECTED;
-            }
-        });
-    LOG_IF_FAILED(result);
-
-    std::wstring cmd = {};
-    if (result == S_OK && appUserModelId.length() > 0)
+    // scenario #1
     {
-        // scenario #1
-        cmd = L"shell:AppsFolder\\" + appUserModelId;
+#pragma warning(suppress : 26494) // Variable 'buffer' is uninitialized. Always initialize an object (type.5).
+        wchar_t buffer[APPLICATION_USER_MODEL_ID_MAX_LENGTH];
+        // "On input, the size of the applicationUserModelId buffer, in wide characters."
+        UINT32 length = APPLICATION_USER_MODEL_ID_MAX_LENGTH;
+
+        const auto result = GetCurrentApplicationUserModelId(&length, &buffer[0]);
+        if (result == ERROR_SUCCESS)
+        {
+            cmd.append(L"shell:AppsFolder\\");
+            // "On success, the size of the buffer used, including the null terminator."
+            // --> Remove the null terminator
+            cmd.append(&buffer[0], length - 1);
+        }
     }
-    else
+
+    if (cmd.empty())
     {
         // scenario #2
         // Get the path to WindowsTerminal.exe, which should live next to us.
@@ -87,20 +75,14 @@ int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int)
 
     // Go!
 
-    // The cmdline argument passed to WinMain is stripping the first argument.
-    // Using GetCommandLine() instead for lParameters
-
     // disable warnings from SHELLEXECUTEINFOW struct. We can't fix that.
-#pragma warning(push)
-#pragma warning(disable : 26476) // Macro uses naked union over variant.
-    SHELLEXECUTEINFOW seInfo{ 0 };
-#pragma warning(pop)
-
+#pragma warning(suppress : 26476) // Expression/symbol '{seInfo.<unnamed-tag>.hIcon = 0}' uses a naked union 'union ' with multiple type pointers: Use variant instead (type.7).
+    SHELLEXECUTEINFOW seInfo{};
     seInfo.cbSize = sizeof(seInfo);
     seInfo.fMask = SEE_MASK_DEFAULT;
     seInfo.lpVerb = L"runas"; // This asks the shell to elevate the process
     seInfo.lpFile = cmd.c_str(); // This is `shell:AppsFolder\...` or `...\WindowsTerminal.exe`
-    seInfo.lpParameters = GetCommandLine(); // This is `new-tab -p {guid}`
+    seInfo.lpParameters = cmdline; // This is `new-tab -p {guid}`
     seInfo.nShow = SW_SHOWNORMAL;
     LOG_IF_WIN32_BOOL_FALSE(ShellExecuteExW(&seInfo));
 

src/cascadia/ShellExtension/OpenTerminalHere.cpp

@@ -49,17 +49,17 @@ try
         siEx.StartupInfo.wShowWindow = SW_SHOWNORMAL;
 
         std::filesystem::path modulePath{ wil::GetModuleFileNameW<std::wstring>(wil::GetModuleInstanceHandle()) };
+        modulePath.replace_filename(runElevated ? ElevateShimExe : WindowsTerminalExe);
+
         std::wstring cmdline;
-        if (runElevated)
-        {
-            RETURN_IF_FAILED(wil::str_printf_nothrow(cmdline, LR"-(-d %s)-", QuoteAndEscapeCommandlineArg(pszName.get()).c_str()));
-        }
-        else
-        {
-            RETURN_IF_FAILED(wil::str_printf_nothrow(cmdline, LR"-("%s" -d %s)-", GetWtExePath().c_str(), QuoteAndEscapeCommandlineArg(pszName.get()).c_str()));
-        }
+        cmdline.reserve(256); // The WindowsTerminal.exe path is ~110 characters long
+        cmdline.push_back(L'"');
+        cmdline.append(modulePath.native());
+        cmdline.append(LR"(" -d )");
+        QuoteAndEscapeCommandlineArg(pszName.get(), cmdline);
+
         RETURN_IF_WIN32_BOOL_FALSE(CreateProcessW(
-            runElevated ? modulePath.replace_filename(ElevateShimExe).c_str() : nullptr, // if elevation requested pass the elevate-shim.exe as the application name
+            modulePath.c_str(),
             cmdline.data(),
             nullptr, // lpProcessAttributes
             nullptr, // lpThreadAttributes
@@ -132,12 +132,11 @@ HRESULT OpenTerminalHere::GetIcon(IShellItemArray* /*psiItemArray*/,
 try
 {
     std::filesystem::path modulePath{ wil::GetModuleFileNameW<std::wstring>(wil::GetModuleInstanceHandle()) };
-    modulePath.replace_filename(WindowsTerminalExe);
     // WindowsTerminal.exe,-101 will be the first icon group in WT
     // We're using WindowsTerminal here explicitly, and not wt (from GetWtExePath), because
     // WindowsTerminal is the only one built with the right icons.
-    const auto resource{ modulePath.wstring() + L",-101" };
-    return SHStrDupW(resource.c_str(), ppszIcon);
+    modulePath.replace_filename(L"WindowsTerminal.exe,-101");
+    return SHStrDupW(modulePath.c_str(), ppszIcon);
 }
 CATCH_RETURN();
 

src/cascadia/ShellExtension/pch.h

@@ -21,13 +21,11 @@
 #undef GetCurrentTime
 #endif
 
-#include <unknwn.h>
-#include <wil/cppwinrt.h>
-#include <winrt/Windows.Foundation.h>
-#include <winrt/Windows.ApplicationModel.h>
+#include <winrt/base.h>
 
-#include <Shobjidl.h>
-#include <shlwapi.h>
+#include <Unknwn.h>
+#include <ShObjIdl.h>
+#include <Shlwapi.h>
 
 #include <wrl.h>
 #include <wrl/module.h>

src/cascadia/WinRTUtils/inc/WtExeUtils.h

@@ -4,6 +4,7 @@ constexpr std::wstring_view WindowsTerminalExe{ L"WindowsTerminal.exe" };
 constexpr std::wstring_view LocalAppDataAppsPath{ L"%LOCALAPPDATA%\\Microsoft\\WindowsApps\\" };
 constexpr std::wstring_view ElevateShimExe{ L"elevate-shim.exe" };
 
+#ifdef WINRT_Windows_ApplicationModel_H
 _TIL_INLINEPREFIX bool IsPackaged()
 {
     static const auto isPackaged = []() -> bool {
@@ -105,6 +106,7 @@ _TIL_INLINEPREFIX const std::wstring& GetWtExePath()
     }();
     return exePath;
 }
+#endif
 
 // Method Description:
 // - Quotes and escapes the given string so that it can be used as a command-line arg.
@@ -114,10 +116,8 @@ _TIL_INLINEPREFIX const std::wstring& GetWtExePath()
 // - arg - the command-line argument to quote and escape.
 // Return Value:
 // - the quoted and escaped command-line argument.
-_TIL_INLINEPREFIX std::wstring QuoteAndEscapeCommandlineArg(const std::wstring_view& arg)
+inline void QuoteAndEscapeCommandlineArg(const std::wstring_view& arg, std::wstring& out)
 {
-    std::wstring out;
-    out.reserve(arg.size() + 2);
     out.push_back(L'"');
 
     size_t backslashes = 0;
@@ -140,5 +140,12 @@ _TIL_INLINEPREFIX std::wstring QuoteAndEscapeCommandlineArg(const std::wstring_v
 
     out.append(backslashes, L'\\');
     out.push_back(L'"');
+}
+
+_TIL_INLINEPREFIX std::wstring QuoteAndEscapeCommandlineArg(const std::wstring_view& arg)
+{
+    std::wstring out;
+    out.reserve(arg.size() + 2);
+    QuoteAndEscapeCommandlineArg(arg, out);
     return out;
 }