Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Put restic binary in a user-writable location on Windows #549

Open
homandr opened this issue Nov 11, 2024 · 5 comments
Open

Put restic binary in a user-writable location on Windows #549

homandr opened this issue Nov 11, 2024 · 5 comments
Labels
enhancement New feature or request p2 planned

Comments

@homandr
Copy link
Contributor

homandr commented Nov 11, 2024

I would like to suggest that maybe Backrest should store restic binary not in C:\Program Files\Backrest as it does currently, but in a user-writable location. For example, %localappdata%. This is what Chrome browser uses as an example, and Firefox when UAC is cancelled during the installation. C:\Program Files on Windows requires UAC elevation to write even if the current user is an administrator. This creates several issues:

  1. Backrest must be launched with elevated privileges on the first run in order to download restic binary. It is mentioned in the documentation but makes Backrest less user-friendly for non-technical users.
  2. If restic self-update through Backrest is ever implemented (would be really nice), this UAC requirement makes things more difficult, requiring that Backrest is either executed elevated for each update or always run elevated.
  3. A user wanting to use Backrest installer currently must have admin permissions, even if technically it is not needed to use the tool. I know one can just download and unpack zip archive anywhere, but then the user needs to set up shortcuts, basically doing manually what the installer does.

I think even Backrest itself could be stored in %localappdata%. It would eliminate all issues listed above and also make Backrest self-updates (if ever implemented) on Windows easier.

@homandr homandr added the enhancement New feature or request label Nov 11, 2024
@garethgeorge
Copy link
Owner

garethgeorge commented Nov 11, 2024

Hey -- really interesting and thanks for the fairly detailed input on considering locations (and reasons). This is something I've struggled with a bit on Windows as my understanding of correct packaging for the platform is relatively weak.

I like the idea re: keeping backrest in %localappdata%, assuming the installer should still be able to configure it to launch on startup from there -- and I imagine that should be the case. Doing that, backrest installs restic using a relative path on windows so it would install next to the backrest binary it in the same directory.

Interestingly I also see a C:\Users\gareth\AppData\Local\Programs folder on my system where a few programs are storing binaries on my system, I'll learn a bit more about what the conventions are here. Edit: this stackoverflow talks a bit about it https://superuser.com/questions/1482669/what-exactly-is-c-users-myuser-appdata-local-programs

Would using appdata be a problem for multiuser installs of backrest? Unsure.

@homandr
Copy link
Contributor Author

homandr commented Nov 16, 2024

Just a quick note - I found out Backrest is actually storing restic binary in the working directory, not necessarily the same directory as Backrest binary. It's just that currently the installer uses C:\Program Files\Backrest in the startup shortcut properties. When I changed that, it re-downloaded restic to the new location.

For multiuser installs you don't want to use %localappdata%. There is %programdata% that's accessible to all users, but not sure if it's appropriate to install binaries there.

Currently the installer creates the shortcuts in the current user's Startup and Desktop folders anyway, so as it stands, it is targeting the current user.

@homandr
Copy link
Contributor Author

homandr commented Nov 17, 2024

I've been researching this subject and I propose we make Windows installer ask the user if they want to install only for the current user or for all users. I haven't worked with NSIS before but looked at some examples and documentation, and I think I can do it.

When "for current user" mode is selected, Backrest will be installed to %localappdata%\Programs\Backrest. No admins rights are required. It will also take care of restic download since that location is user-writable.

When "for everyone" mode is selected, Backrest will be installed to %programfiles%\Backrest. It will ask for privileges elevation like it does now. We need to figure out what to do with the restic location. Currently if the user leaves the installer checkbox "Run application" at the end enabled (default), the download works because Backrest is launched elevated (this one time only), inheriting the elevation from the installer. If the checkbox is unchecked, and Backrest is launched manually afterwards, it can't download restic. What's worse, the Web UI doesn't even come up because it's trying to download restic first.

I see several potential options here:

  1. Leave the existing restic location for machine-wide installation and maybe add a note next that final checkbox, something like "Run application (recommended)" or "(don't uncheck unless you know what you are doing)". It should be sufficient to keep most users on the easy path.
  2. Change the default working directory in the shortcut - where restic is downloaded - to another location. Backrest itself will still be installed to %programfiles%\Backrest. Need to decide what location to use for restic. I see on my PC that %programdata% has .exe binaries for Windows Defender, so I guess it's not strictly for data.

I'm willing to take on the Windows installer work if you want. I already started to work on fixing some issues with the existing installer outside of the changes I'm proposing above; will submit a PR in the near future.

@garethgeorge
Copy link
Owner

Hey -- really appreciate the thought you're giving to this. Contributions re: how Windows is packaged and improving the polish there are hugely appreciated, Backrest has quite a large usage on the platform.

Thinking through the footguns, a few more to mention are

  • Insecure login on LAN #560 -- I think it's also the case that in a multi-user install, the various backrest installs may conflict when trying to bind to the network interface if they aren't correctly shutdown on logout. This isn't something that needs fixing immediately / is in scope for the improvements we're talking about here! But is good to be aware of.
  • Auto-Updater for Windows #539 -- I very much agree with this user's FR that an auto-update feature would be nice. Though this may also end up looking like adding the package to winget. This is something I'll probably support on Linux and Windows (possibly out of scope for MacOS) and is (likely?) easier if the binary is user writable as well.

Thinking through your recommendations, they make sense to me and especially re: the "warning" if installing in a central (program files) type location that updates may be problematic / that the "local user" install is recommended.

Re: workarounds for a centralized install and restic management -- I recently switched to working directory relative installs just for windows to allow for portable installs. I think the working directory via shortcut sounds like a good way to manage that, but minimally I think the warning would cover it reasonably well. With a central install, one will have to redownload and run the installer for each update anyway so backrest should (on first run) have administrator privileges.

To provide a bit of help with untangling the NSIS installer build scripts, the scripts for this are somewhat messy at the moment but are dockerized which makes them relatively easy to try out. See https://github.com/garethgeorge/backrest/blob/main/scripts/generate-installers.sh . These are runnable on a unix system:

So testing the NSIS script is:

# commands to run in the repo root (e.g. top level source directory)
# build backrest as a snapshot distribution , matches settings used by release pipeline
goreleaser release --snapshot --clean
# generate installers
mkdir -p ./installers 
./scripts/generate-installers.sh ./installers

I believe nsis would be runnable directly on Windows and goreleaser is also available there or I can help with testing.

Thanks for thinking on this :) overall I think your recommendations sound like an improvement and I have seen bugs related to install permissions as a pain point that this will help address.

@homandr
Copy link
Contributor Author

homandr commented Nov 18, 2024

I would like to provide some clarity around what user-specific vs machine-wide installations mean. It's really all about these things:

  1. Start Menu and Desktop shortcuts in user's profile vs public (for all users).
  2. The same applies to Startup shortcut. If it's in user's profile, the app will start only when that user logs in. If it's in %programdata%, the app will start for any user who logs in.
  3. Application binaries location. When installed in user's profile, it is not accessible to other users unless they have admin rights and forcefully change NTFS permissions on another user's profile.

One of the issues with the current installer that I'm fixing is that it installs binaries to "Program Files". Yet all shortcuts and registry uninstall entries are for current user only.

Usually, a system-wide installation means just a single copy on disk for desktop applications. Each user has their own running instance and app settings except for global things like license etc. This approach doesn't play well with Backrest due to the port issue like you mentioned. Also, all Backrest configuration is user-specific anyway, there is nothing global. Except for the binaries, which are too small to worry about disk space. If userA configures repos and backup plans, userB won't have any of that configuration even when Backrest is installed in "Program Files" for all users.

A system-wide installation doesn't imply "multi-user", which could mean different things. One scenario is a shared home PC with several users but never concurrent. If each users logs off before the next one, there is no problem, it will just work. But in Windows a user can log in without logging off another user, even if that other user can't use the PC. This is where things get a bit complicated.

Backrest in its default configuration won't work when multiple instances are launched because of the port conflict. And it doesn't matter whether two users launch Backrest from their own profile binary or from the same "Program Files" readable by both users.

It means in order to have multiple running instances, the user will have to configure environment variable with a custom port. I believe this is something I can do in the installer, asking user to override the default port. The installer would take care of the shortcuts too. But the system tray link at the moment appears to be hard-coded and doesn't pick up the variable.

In my mind a true system-wide installation means a single running instance, single listening port, shared system-wide Backrest configuration. More like a server scenario although not necessarily accessible outside of this system. Another example is an office PC. No matter what user logs in, they get access to the same Backrest instance and configuration. This is usually deployed as a service. I can leverage Task Scheduler and NT AUTHORITY\SYSTEM user for autostarting the process (more or less equivalent to running as root on Linux). Or use some of the tools that allow to run any executable as a service.

To sum up, I would propose the following approach for the installer:

  1. Current user installation with no admin rights required. If another user wants to install another instance, the port has to be changed for the second and all subsequent installations.
  2. System-wide installation described above. One downside is with this approach the system tray icon won't be visible since it's no longer running under the current user login.

Everything I suggested above can be accomplished without any Backrest code changes, except for the systray port issue that has nothing to do with my suggestion. Both scenarios would allow Backrest and restic self-updates because now user-specific instance will be running from a user-writable location, and system-wide instance will be running with admin rights.

Please let me know what you think.

homandr added a commit to homandr/backrest that referenced this issue Nov 19, 2024
Various fixes before decisions are made regarding multi-purpose installer (garethgeorge#549)

1. Changed installation location to user %localappdata%\programs since the installer already installed all icons and made registry entry only for the current user.
2. Fixed blurry installer text on high DPI screens.
3. Fixed icon for Start Menu, Desktop shortcuts, and Add/Remove programs entry.
4. Fixed uninstaller not terminating running application and not fully cleaning up installed files.

Users upgrading from the previous installer should uninstall it first to avoid conflicts. User configuration will stay intact.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request p2 planned
Projects
None yet
Development

No branches or pull requests

2 participants