terminal icon indicating copy to clipboard operation
terminal copied to clipboard

Launch elevated instances via shell:AppFolder

Open jboelter opened this issue 2 years ago • 5 comments

Summary of the Pull Request

Launch elevated instances via shell:AppsFolder

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.

References

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

PR Checklist

  • [X] Closes #14501
  • [X] CLA signed. If not, go over here and sign the CLA
  • [ ] Tests added/passed
  • [ ] Documentation updated.
  • [ ] Schema updated.
  • [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.

Detailed Description of the Pull Request / Additional comments

The fallback path (scenario 2 in elevate-shim) doesn't appear to be used, as I can't directly launch WindowsTerminal.exe without an error. It's not clear if this can be removed entirely (implying this would be entirely reliant on elevation via shell:AppsFolder).

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. It wasn't obvious where/if any existing tests exist around elevation. The fallback path (scenario 2 in elevate-shim) was not tested as it appears to be dead code.

jboelter avatar Jan 05 '23 22:01 jboelter

@microsoft-github-policy-service agree

jboelter avatar Jan 05 '23 22:01 jboelter

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view or the :scroll:action log for details.

Unrecognized words (2)

appfolder wargv

Previously acknowledged words that are now absent Hirots inthread reingest :arrow_right:
To accept :heavy_check_mark: these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the [email protected]:jboelter/terminal.git repository on the bug/14501-elevate-dllpath branch (:information_source: how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/3850963644/attempts/1'
:pencil2: Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

:warning: The command is written for posix shells. If it doesn't work for you, you can manually add (one word per line) / remove items to expect.txt and the excludes.txt files.

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/allow/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/allow/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

:microscope: You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. :wink:

If the flagged items are :exploding_head: false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it, try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

github-actions[bot] avatar Jan 05 '23 22:01 github-actions[bot]

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view or the :scroll:action log for details.

Unrecognized words (2)

appfolder wargv

Previously acknowledged words that are now absent Hirots inthread reingest :arrow_right:
To accept :heavy_check_mark: these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the [email protected]:jboelter/terminal.git repository on the bug/14501-elevate-dllpath branch (:information_source: how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/3851038261/attempts/1'
:pencil2: Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

:warning: The command is written for posix shells. If it doesn't work for you, you can manually add (one word per line) / remove items to expect.txt and the excludes.txt files.

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/allow/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/allow/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

:microscope: You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. :wink:

If the flagged items are :exploding_head: false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it, try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

github-actions[bot] avatar Jan 05 '23 23:01 github-actions[bot]

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view or the :scroll:action log for details.

Unrecognized words (1)

wargv

Previously acknowledged words that are now absent Hirots inthread reingest :arrow_right:
To accept :heavy_check_mark: these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the [email protected]:jboelter/terminal.git repository on the bug/14501-elevate-dllpath branch (:information_source: how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/3851066294/attempts/1'
:pencil2: Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

:warning: The command is written for posix shells. If it doesn't work for you, you can manually add (one word per line) / remove items to expect.txt and the excludes.txt files.

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/allow/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/allow/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

:microscope: You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. :wink:

If the flagged items are :exploding_head: false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it, try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

github-actions[bot] avatar Jan 05 '23 23:01 github-actions[bot]

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.

jboelter avatar Jan 06 '23 17:01 jboelter

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view or the :scroll:action log for details.

Unrecognized words (1)

teh

Previously acknowledged words that are now absent Hirots inthread reingest :arrow_right:
To accept :heavy_check_mark: these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the [email protected]:jboelter/terminal.git repository on the bug/14501-elevate-dllpath branch (:information_source: how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/3864172494/attempts/1'
:pencil2: Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

:warning: The command is written for posix shells. If it doesn't work for you, you can manually add (one word per line) / remove items to expect.txt and the excludes.txt files.

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/allow/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/allow/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

:microscope: You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. :wink:

If the flagged items are :exploding_head: false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it, try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

github-actions[bot] avatar Jan 07 '23 22:01 github-actions[bot]

@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.

jboelter avatar Jan 11 '23 19:01 jboelter

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.

KalleOlaviNiemitalo avatar Jan 18 '23 08:01 KalleOlaviNiemitalo

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.

ghost avatar Jan 19 '23 23:01 ghost

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 :)

DHowett avatar Jan 19 '23 23:01 DHowett

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

Handy links:

ghost avatar Jan 24 '23 18:01 ghost

:tada: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 avatar Jan 27 '23 20:01 ghost

:tada: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 avatar Jan 27 '23 20:01 ghost

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

AnrDaemon avatar Jan 31 '23 07:01 AnrDaemon