Launch elevated instances via shell:AppFolder #14637

jboelter · 2023-01-05T22:45:53Z

This uses shell:AppsFolder to launch elevated instances of the app via
ShellExecuteEx and runas in elevate-shim.exe. The app to launch is
discovered via the GetCurrentApplicationUserModelId API.

e.g. shell:AppsFolder\WindowsTerminalDev_8wekyb3d8bbwe!App

This will fallback to launching WindowsTerminal.exe if it fails to
discover the app user model id to launch.

This also fixes a bug in elevate-shim where the first argument of
WinMain was lost (e.g. new-tab).

Curiously, AppLogic::RunAsUwp() is never called and
AppLogic::IsUwp() is always false when running debug builds locally
(e.g. WindowsTerminalDev). It's not clear if this is an artifact of
development packages or something else.

Validation Steps Performed

Various manual debug/execution scenarios.

Verified the fallback path by running the unbundled app by extracting
the CascadiaPackage_0.0.1.0_x64.msix from the 'drop' build artifact.

Fixes #14501

Fixes microsoft#14501 This uses `shell:AppsFolder` to launch elevated instances of the app via `ShellExecuteEx` and `runas` in elevate-shim.exe. The app to launch is discovered via the `GetCurrentApplicationUserModelId` API. e.g. `shell:AppsFolder\WindowsTerminalDev_8wekyb3d8bbwe!App` This will fallback to launching `WindowsTerminal.exe` if it fails to discover the app user model id to launch. This also fixes an innocuous bug in elevate-shim where the first argument of WinMain was lost (e.g. `new-tab`).

jboelter · 2023-01-05T22:47:19Z

@microsoft-github-policy-service agree

jboelter · 2023-01-06T17:25:42Z

I'm not particularly attached to the specifics of the implementation; it was an iterative process to get here.

There may be a potential for (un)escaping issues in the command line conversion when it removes (replace w/ empty string) shell:AppsFolder... from the cmdline args. i.e. arg0 from __wargv is not the same as what is found in the full cmd line string.

~~This is also assuming the string returned from GetCurrentApplicationUserModelId doesn't have spaces in it.~~

edit: change was moved to elevate-shim; no need to parse the cmdline

It also wasn't clear if terminal is always an AppX package, in which case the fallback to invoking WindowsTerminal.exe isn't necessary.

Reverts the changes to AppLogic.* Minimizes scope of changes to TerminalPage and elevate-shim Use WIL for logging and win32 api interaction

jboelter · 2023-01-11T19:26:03Z

@zadjii-msft

Just had an idea while working on something entirely unrelated. Turns out that elevate-shim.exe can successfully call GetCurrentApplicationUserModelId. This means this change can be isolated to the shim and not have to deal with any command line argument parsing. It significantly simplifies the change to only a decision on how it generates the cmd to pass to shell execute.

lhecker · 2023-01-17T11:50:31Z

src/cascadia/ElevateShim/elevate-shim.cpp

-    seInfo.lpFile = module.c_str(); // This is `...\WindowsTerminal.exe`
-    seInfo.lpParameters = pCmdLine; // This is `new-tab -p {guid}`
+    seInfo.lpFile = cmd.c_str(); // This is `shell:AppsFolder\...` or `...\WindowsTerminal.exe`
+    seInfo.lpParameters = GetCommandLine(); // This is `new-tab -p {guid}`


Why not use the previous pCmdLine?

I found an innocuous bug in how the first parameter, new-tab, was dropped when passed to elevate-shim.exe by wWinMain given how it's launched by CreateProcess. It appears wWinMain is assuming param zero is the executable path even if it's not.

However, the elevate-shim process is launched with a command line that doesn't include the executable path.

terminal/src/cascadia/TerminalApp/TerminalPage.cpp

Lines 4118 to 4135 in 8e04169

auto cmdline{

fmt::format(L"new-tab {}", newTerminalArgs.ToCommandline().c_str())

};

wil::unique_process_information pi;

STARTUPINFOW si{};

si.cb = sizeof(si);

LOG_IF_WIN32_BOOL_FALSE(CreateProcessW(exePath.c_str(),

cmdline.data(),

nullptr,

nullptr,

FALSE,

0,

nullptr,

nullptr,

&si,

&pi));

This is the !peb from elevate-shim.exe when launched from a dev build of terminal as an appx (I setup Windbg Preview as the post-mortem debugger and put a DebugBreak() at the top of elevate-shim to break in easily). Notice the CommandLine is missing the executable filepath as the first argument. You can also see this in Process Monitor.

CurrentDirectory: 'C:\windows\system32\' WindowTitle: 'C:\source\github.com.jboelter.terminal\src\cascadia\CascadiaPackage\bin\x64\Debug\AppX\elevate-shim.exe' ImageFile: 'C:\source\github.com.jboelter.terminal\src\cascadia\CascadiaPackage\bin\x64\Debug\AppX\elevate-shim.exe' CommandLine: ' new-tab --profile "{0caa0dad-35be-5f56-a8ff-afceeeaa6101}"' DllPath: 'C:\source\github.com.jboelter.terminal\src\cascadia\CascadiaPackage\bin\x64\Debug\AppX;C:\Program Files\WindowsApps\Microsoft.VCLibs.140.00.Debug.UWPDesktop_14.0.30704.0_x64__8wekyb3d8bbwe;'

Yeeeeeeeep that's totally on me. We probably could fix that down in TerminalPage.cpp, but 🤷

KalleOlaviNiemitalo · 2023-01-18T08:36:33Z

Is shell:AppsFolder documented as an API? It's described in Find the Application User Model ID of an installed app for interactive use though, and there is FOLDERID_AppsFolder in KNOWNFOLDERID.

zadjii-msft

Every day I'm amazed and horrified. Let's give this a shot for 1.17. Somehow I have a feeling that this is going to be way more reliable, based on the fact that it's 500% crazier than what we had before 😆

DHowett

Thanks so much for doing this. It's clever and thoughtfully-implemented 😄

ghost · 2023-01-19T23:39:52Z

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

DHowett · 2023-01-19T23:43:35Z

Is shell:AppsFolder documented as an API? It's described in Find the Application User Model ID of an installed app for interactive use though, and there is FOLDERID_AppsFolder in KNOWNFOLDERID.

I know now (thanks to jboelter!) that PowerToys uses it, but I don't know whether it's been publicly documented. That's something we should definitely pursue. Thank you :)

ghost · 2023-01-24T18:18:43Z

🎉Windows Terminal Preview v1.17.1023 has been released which incorporates this pull request.:tada:

Handy links:

This uses `shell:AppsFolder` to launch elevated instances of the app via `ShellExecuteEx` and `runas` in elevate-shim.exe. The app to launch is discovered via the `GetCurrentApplicationUserModelId` API. e.g. `shell:AppsFolder\WindowsTerminalDev_8wekyb3d8bbwe!App` This will fallback to launching `WindowsTerminal.exe` if it fails to discover the app user model id to launch. This also fixes a bug in elevate-shim where the first argument of WinMain was lost (e.g. `new-tab`). Curiously, `AppLogic::RunAsUwp()` is never called and `AppLogic::IsUwp()` is always false when running debug builds locally (e.g. WindowsTerminalDev). It's not clear if this is an artifact of development packages or something else. ## Validation Steps Performed Various manual debug/execution scenarios. Verified the fallback path by running the unbundled app by extracting the `CascadiaPackage_0.0.1.0_x64.msix` from the 'drop' build artifact. Fixes #14501

This uses `shell:AppsFolder` to launch elevated instances of the app via `ShellExecuteEx` and `runas` in elevate-shim.exe. The app to launch is discovered via the `GetCurrentApplicationUserModelId` API. e.g. `shell:AppsFolder\WindowsTerminalDev_8wekyb3d8bbwe!App` This will fallback to launching `WindowsTerminal.exe` if it fails to discover the app user model id to launch. This also fixes a bug in elevate-shim where the first argument of WinMain was lost (e.g. `new-tab`). Curiously, `AppLogic::RunAsUwp()` is never called and `AppLogic::IsUwp()` is always false when running debug builds locally (e.g. WindowsTerminalDev). It's not clear if this is an artifact of development packages or something else. ## Validation Steps Performed Various manual debug/execution scenarios. Verified the fallback path by running the unbundled app by extracting the `CascadiaPackage_0.0.1.0_x64.msix` from the 'drop' build artifact. Fixes #14501 (cherry picked from commit eab1c23) Service-Card-Id: 87690177 Service-Version: 1.16

ghost · 2023-01-27T20:13:00Z

🎉Windows Terminal v1.16.1026 (1.16.10261.0 and 1.16.10262.0) has been released which incorporates this pull request.:tada:

Handy links:

ghost · 2023-01-27T20:15:54Z

🎉Windows Terminal v1.16.1026 (1.16.10261.0 and 1.16.10262.0) has been released which incorporates this pull request.:tada:

Handy links:

AnrDaemon · 2023-01-31T07:19:54Z

"Release notes" link in bot posts is 404'd.

ghost added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. labels Jan 5, 2023

This comment has been minimized.

Sign in to view

length() -> size()

145dd52

This comment has been minimized.

Sign in to view

make spell check happy

039307f

This comment has been minimized.

Sign in to view

jboelter added 2 commits January 5, 2023 15:40

make argc const to satisfy audit mode build

9c794e6

add wargv to recognized words

f69a69e

zadjii-msft mentioned this pull request Jan 6, 2023

The new 'Run as Administrator' feature returns a 'cannot find WindowsTerminal.exe' error #12369

Open

Minimized scope of changes and use WIL

74c4b0c

Reverts the changes to AppLogic.* Minimizes scope of changes to TerminalPage and elevate-shim Use WIL for logging and win32 api interaction

This comment has been minimized.

Sign in to view

fix spelling

1aa94bb

zadjii-msft added this to the Terminal v1.17 milestone Jan 10, 2023

jboelter added 2 commits January 11, 2023 21:52

simplification to only change elevate-shim

405e89e

make audit mode happy

f94b4e9

carlos-zamora added a commit that referenced this pull request Jan 14, 2023

PRE-MERGE #14637 Launch elevated instances via shell:AppFolder

3dd18f9

lhecker approved these changes Jan 17, 2023

View reviewed changes

zadjii-msft approved these changes Jan 19, 2023

View reviewed changes

DHowett approved these changes Jan 19, 2023

View reviewed changes

DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 19, 2023

ghost merged commit eab1c23 into microsoft:main Jan 19, 2023

zadjii-msft mentioned this pull request Jan 24, 2023

Powershell as Admin - Ignoring system path variable #14686

Closed

ghost mentioned this pull request Jan 24, 2023

WindowsTerminal fails to launch elevate=true profiles on new install; elevate-shim does not set dllpath #14501

Closed

zadjii-msft mentioned this pull request Jan 25, 2023

Cannot start after updating to 1.16 - likely due to elevate:true #14729

Closed

carlos-zamora mentioned this pull request Jan 25, 2023

Windows Terminal doesn't start from folder right-click menu #14369

Open

zadjii-msft mentioned this pull request Feb 5, 2023

windows terminal (as well as windows terminal preview) only runs in admin mode #14788

Closed

msherian mentioned this pull request Sep 4, 2023

System performance extremely poor when Windows Terminal is open during Teams screen sharing session #15928

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Launch elevated instances via shell:AppFolder #14637

Launch elevated instances via shell:AppFolder #14637

jboelter commented Jan 5, 2023 •

edited by DHowett

Loading

jboelter commented Jan 5, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jboelter commented Jan 6, 2023 •

edited

Loading

This comment has been minimized.

jboelter commented Jan 11, 2023 •

edited

Loading

lhecker Jan 17, 2023

jboelter Jan 18, 2023 •

edited

Loading

zadjii-msft Jan 19, 2023

KalleOlaviNiemitalo commented Jan 18, 2023

zadjii-msft left a comment

DHowett left a comment

ghost commented Jan 19, 2023

DHowett commented Jan 19, 2023 •

edited

Loading

ghost commented Jan 24, 2023

ghost commented Jan 27, 2023

ghost commented Jan 27, 2023

AnrDaemon commented Jan 31, 2023

	auto cmdline{
	fmt::format(L"new-tab {}", newTerminalArgs.ToCommandline().c_str())
	};

	wil::unique_process_information pi;
	STARTUPINFOW si{};
	si.cb = sizeof(si);

	LOG_IF_WIN32_BOOL_FALSE(CreateProcessW(exePath.c_str(),
	cmdline.data(),
	nullptr,
	nullptr,
	FALSE,
	0,
	nullptr,
	nullptr,
	&si,
	&pi));

Launch elevated instances via shell:AppFolder #14637

Launch elevated instances via shell:AppFolder #14637

Conversation

jboelter commented Jan 5, 2023 • edited by DHowett Loading

Validation Steps Performed

jboelter commented Jan 5, 2023

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jboelter commented Jan 6, 2023 • edited Loading

This comment has been minimized.

jboelter commented Jan 11, 2023 • edited Loading

lhecker Jan 17, 2023

Choose a reason for hiding this comment

jboelter Jan 18, 2023 • edited Loading

Choose a reason for hiding this comment

zadjii-msft Jan 19, 2023

Choose a reason for hiding this comment

KalleOlaviNiemitalo commented Jan 18, 2023

zadjii-msft left a comment

Choose a reason for hiding this comment

DHowett left a comment

Choose a reason for hiding this comment

ghost commented Jan 19, 2023

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

DHowett commented Jan 19, 2023 • edited Loading

ghost commented Jan 24, 2023

ghost commented Jan 27, 2023

ghost commented Jan 27, 2023

AnrDaemon commented Jan 31, 2023

jboelter commented Jan 5, 2023 •

edited by DHowett

Loading

jboelter commented Jan 6, 2023 •

edited

Loading

jboelter commented Jan 11, 2023 •

edited

Loading

jboelter Jan 18, 2023 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

DHowett commented Jan 19, 2023 •

edited

Loading