[Feature] Ask for administrator permissions on update if not able to write to target folder

[XanatosX]

Problem

Is your feature request related to a problem? Please describe.

If I try to update the application without starting it as an administrator the update fails and deletes the exe as well. This forces me to download the program again. I do run the program on a windows machine.

Suggestions

Describe the solution you'd like

Could you try to write some data to the target folder before installing and if the task fails request administration permissions. This should solve the error, I think.

Not sure how the patching works exactly but I think ckan starts a "patch app" which downloads the newest build and replaces the old files. The check if you can write into the folder could be placed here. If the permission is not there you could start the patch app as administrator which should give a popup for the request. I do not think if other os require this as well.

Describe alternatives you've considered

Check if the program runs with administrator privileges and if this is not the case prevent the update.

[DasSkelett]

I'd like to keep permission handling out of CKAN as much as possible. Running with elevated permissions is a big no-go for the ckan.exe, maybe less so for the auto updater, but I still see tons of new bugs and cross-platform portability problems appearing.

If I try to update the application without starting it as an administrator the update fails and deletes the exe as well.

Hm, did this happen with one of the latest updates, i.e. to v1.30.0 or later? We did fix a rare possibility that the auto-updater could remove the old ckan.exe without moving the new one in #3250 if it failed 7 times but succeeded the 8th and final time. Currently it could theoretically still happen if the auto updater is somehow allowed to remove the old ckan.exe, but doesn't have permissions to move the new file in place. This should only occur if the permissions are somehow messed up.

What speaks against moving ckan.exe to a place where it and the auto updater has write access? Somewhere in your user directory, for example.

[HebaruSan]

The auto-updater is already started with Verb = "runas", which is supposed to launch with requested escalated permissions:

https://github.com/KSP-CKAN/CKAN/blob/62a0bc7c32686df333e9319082349fd332aa7c25/Core/Net/AutoUpdate.cs#L164-L175

However, dotnet/runtime#63784 states "You need to set ProcessStartInfo.UseShellExecute = true, for ProcessStartInfo.Verb to have any effect. The default is true on .NET Framework but changed to false in .NET Core." Currently we set this to false.

This important quirk is mentioned nowhere in the actual documentation:

• https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.start
• https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo
• https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.useshellexecute
• https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.verb

Changing UseShellExecute would have some surprising side effects:

But in our case, we're using RedirectStandardInput rather than RedirectStandardOutput, so we should be OK. 🤞

[HebaruSan]

But in our case, we're using RedirectStandardInput rather than RedirectStandardOutput, so we should be OK. 🤞

Elsewhere, this amazingly clear, complete, and consistent documentation suggests otherwise:

So far the autoupdater isn't working for me with UseShellExecute=true. ~~Don't know yet whether it's not running, or failing to do the update.~~ Removing RedirectStandardInput=true made it run, but the newly launched CKAN process also runs as admin, which trips the admin check! Adding --asroot wouldn't be a very good example for our official updater component to set, so ideally we would de-escalate privileges here, but so far that looks very hard to do...

[HebaruSan]

As of #3997, the updater now launches with escalated permissions and runs the new ckan.exe with --asroot. We may revisit this if we find a way to do the former without the latter, but for now that should address this issue.