fix(#1606): Remove console window for GUI apps, without changing the manifests.

[spider2048]

Description

Added 2 functions to get the subsystem of the shimmed PE binaries and change the subsystem of the shim executable while installing.

Motivation and Context

Closes #1606 as it is annoying to deal with the Console Window.

How Has This Been Tested?

I've checked my code changes by installing packages: nvim, vim, wireshark, firefox.
Every GUI app from the above packages (nvim-qt, gvim, gvimdiff, wireshark, firefox) doesn't pop-up a console window and works fine.
The changed code only affects the subsystem value in the PE Optional Headers of the shim exe.

Checklist:

• I have read the Contributing Guide.
• I have ensured that I am targeting the develop branch.
• I have updated the documentation accordingly.
• I have updated the tests accordingly.
• I have added an entry in the CHANGELOG.

[spider2048]

Hey, all! Umm.. any reasons for rejecting/keeping this pull request stale?

[rashil2000]

Hey @spider2048!

I apologize for letting this PR get stale - I recently switched jobs so I haven't been active on GitHub for a long while.
This has been a very long standing issue in Scoop, so thank you for taking the time to work on it.

I just have a couple of hunches - post which I'll merge this ASAP.

[rashil2000]

Also, could you provide a brief list of validation steps to test this PR?

Thanks again!

[spider2048]

Also, could you provide a brief list of validation steps to test this PR?

Thanks again!

Hello @rashil2000!
The way you validate this PR is by installing a package which contains a GUI app which has to be shimmed.

For example, if you have neovim/firefox/brave installed, force updating them using this PR would patch their shim, which removes the console window.

I've fixed the parts of the code you pointed out.

[rashil2000]

Great! I have tested it with Notepad++ and Paint.NET.

Final step: please add an entry in the Changelog.md

[spider2048]

Hello @rashil2000, I've added an entry in the changelog.md as you requested.
Is the PR good to go now?

[HUMORCE]

~\Desktop
❯ ni 1app.exe

    Directory: C:\Users\humorce\Desktop

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
-a---           10/9/2023  4:11 AM              0 1app.exe

~\Desktop
❯ scoop shim add 1app .\1app.exe
Adding local shim 1app...
MethodInvocationException: C:\Users\humorce\scoop\apps\scoop\current\lib\core.ps1:12
Line |
  12 |          $peOffset = $binaryReader.ReadInt32()
     |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     | Exception calling "ReadInt32" with "0" argument(s): "Unable to read beyond the end of the stream."

This is not a bug technically, but consider this scene.

[spider2048]

Hello, @HUMORCE!
Thanks for pointing out the bug.
Please consider looking at #5684, I've fixed the issue along with a few minor changes.
I think it is good to go.

[r15ch13]

So, I just installed syncthingtray which includes cli executable on a fresh installation of Win11 and Windows Defender doesn't seem to like this implementation.

Maybe compiling a cli version of shim.exe and using the corresponding shim after reading the subsystem is a better approach to this issue.

[spider2048]

Hello @r15ch13!
I am able to reproduce the issue on my PC as well. As you said, windows defender didn't like the application which was installed and not the shim. The following image describes it.

Also, if we look at the error message:

InvalidOperation: D:\Sources\Scoop\lib\core.ps1:19
Line |
  19 |          $binaryReader.Close()
     |          ~~~~~~~~~~~~~~~~~~~~~
     | You cannot call a method on a null-valued expression.
InvalidOperation: D:\Sources\Scoop\lib\core.ps1:20
Line |
  20 |          $fileStream.Close()
     |          ~~~~~~~~~~~~~~~~~~~
     | You cannot call a method on a null-valued expression.

The error occurs in the Get-PESubsystem function (L1-23) because windows defender prevents the executable file from being opened. It actually has nothing to do with the shim.

So, compiling the shim wouldn't really fix this issue as the user would try to run the false positive anyway.
I think the maintainers of the application should take care of this issue.

Edit:
It is also worth noting that, using the link on their github releases page triggers my browser.

https://github.com/Martchus/syncthingtray/releases/download/v1.4.7/syncthingtray-1.4.7-x86_64-w64-mingw32.exe.zip

Look at the VT Report of the file with which Windows Defender is unhappy with.

[brian6932]

Another program that kinda has some weird behavior with this PR is rufus, which has a cli mode pbatard/rufus#111 (comment) but it can't be accessed due to:

Creating shim for 'rufus'.
Making ~\scoop\shims\rufus.exe a GUI binary.

You can ofc get around this with & (scoop which rufus)

[spider2048]

Hello, @brian6932!

Rufus' CLI developement has a very low priority. As a result, we have the following:

1. The Rufus executable is compiled with the subsystem windows.
2. To implement CLI features, the application tries to check if the calling process has a valid console handle and tries to attach to it.
3. If Rufus attached to the parent processes' console, it would extract a rufus.com in the current folder, which Rufus calls the Hogger.
4. Here comes the best part. Rufus tries to simulate keystrokes rufus.com<CR> to the console (to execute it).
5. The rufus.com executable waits for the global mutex creted by the main Rufus.exe process.
6. After finishing, Rufus tries to delete the rufus.com file, which didn't really happen in my case.

So, when an application (like Rufus) compiled with the windows subsystem wants a console window, they use AllocConsole to get a new console window. But the developers already consider the disadvantage of the floating console window. But, Rufus, on the other hand tries to attach to the existing console window using AttachConsole. If the application was executed normally in a shell (using its actual path), Rufus can obtain a console handle directly.

If we use the CLI version of the shim (without the PR), Rufus would think that it is operating from a shell which wouldn't be the case everytime. Also, it creates some weird lag because the main Rufus application blocks the console and the keystrokes events get processed after the application quits (creating weird lag). And because I use powershell:

spider@Sp1d3R 0 0.074s
% rufus

spider@Sp1d3R 0 6.403s (command took 6 seconds even I closed the Rufus' main window within 2 seconds)
% rufus.com
rufus.com: The term 'rufus.com' is not recognized as a name of a cmdlet, function, script file, or executable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.

Suggestion [3,General]: The command rufus.com was not found, but does exist in the current location. PowerShell does not load commands from the current location by default. If you trust this command, instead type: ".\rufus.com". See "get-help about_Command_Precedence" for more details.

... which is not a good experience for a user who needs to use Rufus' GUI.

If we wanted to use the CLI features:

spider@Sp1d3R 0 0.007s
% rufus -h

Usage: rufus [-x] [-g] [-h] [-f FILESYSTEM] [-i PATH] [-l LOCALE] [-w TIMEOUT]
  -x, --extra-devs
     List extra devices, such as USB HDDs
  -g, --gui
     Start in GUI mode (disable the 'rufus.com' commandline hogger)
  -i PATH, --iso=PATH
     Select the ISO image pointed by PATH to be used on startup
  -l LOCALE, --locale=LOCALE
     Select the locale to be used on startup
  -f FILESYSTEM, --filesystem=FILESYSTEM
     Preselect the file system to be preferred when formatting
  -w TIMEOUT, --wait=TIMEOUT
     Wait TIMEOUT tens of seconds for the global application mutex to be released.
     Used when launching a newer version of Rufus from a running application.
  -h, --help
     This usage guide.

spider@Sp1d3R 0 3.667s
% rufus.com
rufus.com: The term 'rufus.com' is not recognized as a name of a cmdlet, function, script file, or executable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.

Suggestion [3,General]: The command rufus.com was not found, but does exist in the current location. PowerShell does not load commands from the current location by default. If you trust this command, instead type: ".\rufus.com". See "get-help about_Command_Precedence" for more details.

... which is an okay-ish experience (~3.5 seconds for showing the help).

On the other hand, using the GUI version of the shim (with the PR), Rufus won't be able to attach to the console. This would not create any weird lags if the user had to use only the GUI features. We can't use the CLI features using this shim. So, if we wanted the CLI features, we would prefer to use the workaround you proposed. But the GUI features work perfectly.

spider@Sp1d3R 0 0.001s
% rufus

spider@Sp1d3R 0 0.052s
%

... which is a good experience (~0.2s to show the UAC prompt)

So, I would like to conclude that:

1. The maintainers of Rufus should rethink the way they implement the CLI features (I feel it that way)
2. Using this PR, Rufus' GUI features work perfectly - No weird lags, but using the CLI shim would create some lag to use both the GUI and CLI features.

[brian6932]

Yea I agree that Rufus' CLI implementation is scuffed, I just wanted to point out what I witnessed, as I don't doubt there could be other GUI+CLI bins that experience similar issues. Potentially a manual setting for GUI/CLI bin shims could be required for certain programs.

[spider2048]

Yes, I agree with you. I think we can probably adjust the manifests and add a command line flag, if we experience issues. (for packages like Rufus). Choosing between AttachConsole and AllocConsole is totally up to the developers.

[melMass]

Is there a way to enforce this feature? I'm trying to create a manifest for the rio terminal, but the shim creates a visible terminal running the shim that stays open but at least can be closed, but I tried to make a shortcut and this creates a "conhost.exe" in the background that I have to force quit.

[spider2048]

Hello @melMass!

I've tried to install rio using your manifest. This is what I observed after installing it:

1. Using this PR, running rio from the command line didn't create any "visible terminal". The only window which was created was the console window of Rio.
2. The shim doesn't start an additional conhost.exe process. It just starts the target process. The target process is responsible for starting a conhost.exe if it is using a console.

The above picture is the process tree after running rio from the command line. The second process is the main rio process, which starts conhost.

On the other hand, if you didn't use this PR, you would see a blank console window (if you run the shim using the shortcut in the startmenu) along with the Rio's main window.

I would also like to point out that, using the shortcut to the actual rio.exe (not the shim) would panic and create huge amounts of lag: (even when the PR is not used)

It's because the shortcut uses the variable %HOMEPATH% which expands to \Users\spider (in my case) without the drive name. It should be changed to %HOMEDRIVE%%HOMEPATH% which includes the drive letter as well. Now, making this fix in the shortcut didn't give me an extra conhost.exe process which I had to force quit.

Now, there's something I am worried about: here, which would not work if we are using this PR, but I will make a fix for it soon.

[melMass]

It's because the shortcut uses the variable %HOMEPATH% which expands to \Users\spider (in my case) without the drive name. It should be changed to %HOMEDRIVE%%HOMEPATH% which includes the drive letter as well. Now, making this fix in the shortcut didn't give me an extra conhost.exe process which I had to force quit.

Now, there's something I am worried about: here, which would not work if we are using this PR, but I will make a fix for it soon.

Thanks a lot for this answer, I will try out more based on it