Handle scope better

[denelon]

Description of the new feature / enhancement

Several packages have been reported to not properly handle "--scope".

The most likely reasons are due to the installer determining scope based on whether the process is elevated, or the user is a member of the local Administrators group.

One example is Git.Git reported at twitter by @woodyinwoodley

The 2.40.1 manifest is specifying an "inno" installer, but it doesn't appear to support the /CURRENTUSER or /ALLUSERS switch.

The manifest does have entries for both "user" and "machine", but those keys don't actually drive the intended behavior.

The default behavior in the installer checks to see if the user is a member of the local Administrators group to determine if the installer will be installed machine wide or per user.

; Note that we write the Registry values below either to HKLM or to HKCU depending on whether the user running the installer
; is a member of the local Administrators group or not (see the "Check" argument).

; Install under HKEY_LOCAL_MACHINE if an administrator is installing.

https://github.com/git-for-windows/build-extra/blob/6fe3f63731fb39e20f7a668fd54566514ecc83af/installer/install.iss#L214

; Install under HKEY_CURRENT_USER if a non-administrator is installing

https://github.com/git-for-windows/build-extra/blob/6fe3f63731fb39e20f7a668fd54566514ecc83af/installer/install.iss#L221

Proposed technical implementation details

We could add another YAML key to indicate that the scope arguments either aren't properly supported, or we could attempt to provide some kind of error messaging to help users understand why their request can't be honored by WinGet.

We could potentially also check to see if the user is in the local Administrator group to determine what output to display.

[Trenly]

We could add another YAML key to indicate that the scope arguments either aren't properly supported

This probably wouldn't even need a new key. It would just need Scope and the associated implementation to be added to the enum for UnsupportedArguments

winget-cli/schemas/JSON/manifests/v1.4.0/manifest.installer.1.4.0.json

Lines 456 to 468 in 005f765

	"UnsupportedArguments": {
	"type": [ "array", "null" ],
	"uniqueItems": true,
	"items": {
	"type": "string",
	"title": "UnsupportedArgument",
	"enum": [
	"log",
	"location"
	]
	},
	"description": "List of winget arguments the installer does not support"
	},

or we could attempt to provide some kind of error messaging to help users understand why their request can't be honored by WinGet.

This is also already kind of in place, although it could likely be improved.

winget-cli/src/AppInstallerCLICore/Workflows/InstallFlow.cpp

Line 317 in 005f765

context.Reporter.Warn() << Resource::String::UnsupportedArgument << std::endl;

winget-cli/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw

Lines 1446 to 1448 in 005f765

	<data name="UnsupportedArgument" xml:space="preserve">
	<value>A provided argument is not supported for this package</value>
	</data>

[florelis]

I think for cases where it depends on whether it runs as admin or not, we could set elevationRequirement=elevationProhibited for user scope and elevationRequirement=requiresElevation for machine scope; then enlighten winget to launch the installers with the right access. We could spin off a separate process with the right permissions and use it only to run the installers from the right context. That could probably also help us get a single UAC from winget during upgrade --all / import, instead of multiple ones from each installer.

[yao-msft]

I think for cases where it depends on whether it runs as admin or not, we could set elevationRequirement=elevationProhibited for user scope and elevationRequirement=requiresElevation for machine scope;

I think for this particular installer, fixing from manifest is ideal. And works in downlevel.

[Trenly]

I think for cases where it depends on whether it runs as admin or not, we could set elevationRequirement=elevationProhibited for user scope and elevationRequirement=requiresElevation for machine scope;

I think for this particular installer, fixing from manifest is ideal. And works in downlevel.

I dont think that works in this case. Git.git doesn’t depend on elevation status. If an administrator is running the installer - even in a non-elevated context - it will install with machine scope; If a non-admin is running - even in an elevated context - I believe it installs as user scope (depends on how IsAdminLoggedOn is handled for inno)

[yao-msft]

I dont think that works in this case. Git.git doesn’t depend on elevation status. If an administrator is running the installer - even in a non-elevated context - it will install with machine scope; If a non-admin is running - even in an elevated context - I believe it installs as user scope (depends on how IsAdminLoggedOn is handled for inno)

Ah, thanks for the clarification. I must have mis-read something. I feel to truly support this case, there may be changes required to the installer itself.

[Trenly]

I dont think that works in this case. Git.git doesn’t depend on elevation status. If an administrator is running the installer - even in a non-elevated context - it will install with machine scope; If a non-admin is running - even in an elevated context - I believe it installs as user scope (depends on how IsAdminLoggedOn is handled for inno)

Ah, thanks for the clarification. I must have mis-read something. I feel to truly support this case, there may be changes required to the installer itself.

I agree that changing the installer would probably be the best here, but its likely a breaking change for git for windows, which leads me to believe it won’t happen anytime soon. Of course anything is possible, but there are likely other installers out there which don’t respect intended scope, so in my opinion it would be good to have at least some method of declaring this in the manifest

[yao-msft]

I agree that changing the installer would probably be the best here, but its likely a breaking change for git for windows, which leads me to believe it won’t happen anytime soon. Of course anything is possible, but there are likely other installers out there which don’t respect intended scope, so in my opinion it would be good to have at least some method of declaring this in the manifest

Yes, it's certainly doable but unfortunately it does not solve the actual issue. Declaring Log | InstallLocation as unsupported means the installer does not support custom log writing | installing to custom location. While in this case, declaring scope as unsupported is sort of hiding the issue. The installer does support "scope", but in a custom way. Making it unsupported seems not helping user with their goal, but failing gracefully. winget install git.git --scope machine will just fail gracefully with "scope is not supported by this package"

[Trenly]

While in this case, declaring scope as unsupported is sort of hiding the issue.

I hadn't considered it that way. That's a good way of looking at it. There certainly is a tradeoff between accuracy and ease of use

[denelon]

Yeah, that's been my concern as well. I think we might need to fail if we can't honor "--scope" as passed in, and provide the appropriate error with what we can determine. That's why I was thinking we might also need to have something that means: *

• "scope for this installer is determined by running the command as user vs administrator"
  and
• "scope for this installer is determined by user being a member of administrators group"

At least that way we don't do something against the wishes of the user arguments, and they can be informed.

[joaompinto]

As an enterprise user with limited privileges I am allowed to install software which meets the following requirements:

1. Is available only for my user (Scope: User)
2. Does not require elevated privileges to be installed/executed

Some software support the above requirement using a very clear approach (e.g. VSCode, User Installer vs System Installer), others do not support it at all (e.g standard Python installer) others require a very custom set of installer parameters (eg. Miniforge3).

While I understand now (from Issue #3286) that "Scope" and "ElevationRequirement" might be divergent, it is my understanding that the expectation from most users is that Scope: User also implies "No elevation required".

I see two potential options:

a) Determine that packages with "Scope: User" MUST 1. install to a user scoped location AND 2. not require Elevation
b) Provide clear documentation and different properties to separate the distinction between "Scope" and ""ElevationRequirement"

I would strongly prefer option a) because in my experience it is clear to understand matching the most commonly found use case.

If there are exceptional cases where an applications need to be installed with elevated privileges but the user would like to have it installed to the a user scoped location I believe it should be handled with some CLI override options.