Skip to content
New issue

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

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

Already on GitHub? # to your account

.nonadmin files in pre-menuinst v2 conda installations #150

Closed
jaimergp opened this issue Aug 24, 2023 · 6 comments · Fixed by #156
Closed

.nonadmin files in pre-menuinst v2 conda installations #150

jaimergp opened this issue Aug 24, 2023 · 6 comments · Fixed by #156
Labels
locked [bot] locked due to inactivity

Comments

@jaimergp
Copy link
Contributor

This is an issue for both constructor and menuinst I guess, but I'll keep it here because I think it has to do more with menuinst (even if the fix is in constructor proper).


The context:

  • On Windows, menuinst v1 has some super user permissions elevation logic to enable the creation of shortcut items in the Start Menu for "All Users" installations.
  • The way menuinst anticipates whether it needs superuser permissions or not is by checking the presence of a .nonadmin file in the root conda installation (aka %PREFIX%\.nonadmin). The presence of the file dictates that no extra permisions are needed. The absence means superuser was needed at installation time.
    • This file is placed there by the constructor-based installer when the user installs Miniconda, Miniforge, etc.
    • It is primarily there to notify the uninstaller whether superuser is needed or not, but menuinst also uses it because well, it's there.
  • Again, this is a Windows-only thing! .nonadmin doesn't exist in Linux or macOS, even if no special permissions were needed to install conda.

The problem:

  • menuinst v2 extends shortcut creation to all platforms, Linux and macOS included.
  • It still checks for permissions needed or not via the presence of this file.
  • However, existing Unix installations (all of those provided by a constructor-based installer built with constructor 3.4.5 or earlier), will NOT have the .nonadmin file even if no special permissions were needed.
  • This will make menuinst believe that, since .nonadmin is absent, it indeed NEEDS superuser permissions, and will request sudo powers, when it doesn't technically need them.

Possible solutions:

  • Add a post-link.sh script to the menuinst v2 package to add the .nonadmin file to $CONDA_ROOT in Unix systems. The logic would be something like "if it's not there, try to add it; if we could do it, it was a nonadmin install; if we failed, that's ok, it was an admin install".
  • Change how we approach the "superuser permissions needed" on Unix systems. Maybe we can ignore the .nonadmin file and instead check whether the CONDA_ROOT path is in a system-wide location (/opt, /usr/local...), outside of the current user home (~/), while handling if we are sudo already or not. This might have a few rough edges though; e.g. what if the user installed to a non-standard location like /my-stuff/conda. Is that user-only? All users? Maybe it's world writable but, does that mean that the shortcuts I am processing should be available to all users?

I feel we need to rethink how this is approached outside Windows, specially because HPC users will probably run Linux and in those setups you can find system-wide conda installations with per-user environments that should NOT ask for superuser permissions to process their own shortcuts.

Maybe it's all way simpler: if the installation is running with superuser permissions already, then install to an "All users" location. If it's not, we'll install to current user directories. But we should not depend on the state of the $CONDA_ROOT folder.

Thoughts? Tagging a few folks that have shown interest in the project or the superuser elevation issue before: @aganders3 @mrclary @marcoesters. Thanks!

@mrclary
Copy link

mrclary commented Aug 24, 2023

  • Add a post-link.sh script to the menuinst v2 package to add the .nonadmin file to $CONDA_ROOT in Unix systems. The logic would be something like "if it's not there, try to add it; if we could do it, it was a nonadmin install; if we failed, that's ok, it was an admin install".

Does any other package use .nonadmin? Or does constructor add this for the sole benefit of menuinst?

  • Change how we approach the "superuser permissions needed" on Unix systems. Maybe we can ignore the .nonadmin file and instead check whether the CONDA_ROOT path is in a system-wide location (/opt, /usr/local...), outside of the current user home (~/), while handling if we are sudo already or not. This might have a few rough edges though; e.g. what if the user installed to a non-standard location like /my-stuff/conda. Is that user-only? All users? Maybe it's world writable but, does that mean that the shortcuts I am processing should be available to all users?

Maybe we consider two possible solutions:

  • Provide user input to menuinst specifying user|all user treatment. This would allow users to dictate behavior under in any situation.
  • Put .nonadmin in the package environment, not necessarily the root environment. This seems more natural to me anyway, since shortcuts are created for a package in a particular environment. This could address circumstances where root environments are "all users" and user environments are not.

@marcoesters
Copy link
Contributor

I personally dislike the "admin unless a file has been found" approach and would rather see a ".admin" file instead. But that may be too much of a breaking change since it also affects how constructor works.

Adding a user|all option is a good idea in either case.

The tricky part for Linux is indeed an HPC environment where there is an all-users installation but every user has a shortcut that they can remove. I wonder if a check like is_sudo or UserID == FileOwnerID could be a solution here.

Put .nonadmin in the package environment, not necessarily the root environment.

This could be a promising approach, but which package environment? The .nonadmin file is created by constructor, so it needs to be in a place that both constructor and menuinst can easily find.

@jaimergp
Copy link
Contributor Author

Provide user input to menuinst specifying user|all user treatment. This would allow users to dictate behavior under in any situation.

The problem is that most of the time menuinst is called by conda behind the scenes; it's not the user dictating that directly. So we need some kind of heuristic to decide without user intervention.

Put .nonadmin in the package environment, not necessarily the root environment.

That's promising, but it's breaking the previous assumptions for Windows (where root env is checked). Maybe we would check package env first and then only base if absent, but since absence could be both meaningful (is admin) and lack of signal (we didn't try to establish whether admin is needed or not), it'll be ambiguous.

The more I think about it the more I believe that in Unix systems we can get away with a different strategy. In Windows, requesting super user permissions is more involved than prepending sudo to the command. It's usually done from the UI, right clicking on the shortcut, or pre-invoking a CMD instance with admin permissions. So it's way more cumbersome and hence why I think previous engineers working on it decided for the file sentinel for auto-elevation.

On Unix, users could just rerun the command with sudo in-front if they reaaally need an all-users shortcut. If they are installing something for all users, that probably requires sudo anyway at that point. So I'd say we check for the user id and if it's root we go for all users; otherwise, we default to the current user location.

I wonder if a check like is_sudo or UserID == FileOwnerID could be a solution here.

Yea, this (or a similar variation) is essentially what I think we should do.

@mrclary
Copy link

mrclary commented Aug 25, 2023

Provide user input to menuinst specifying user|all user treatment. This would allow users to dictate behavior under in any situation.

The problem is that most of the time menuinst is called by conda behind the scenes; it's not the user dictating that directly. So we need some kind of heuristic to decide without user intervention.

Conceded.

Put .nonadmin in the package environment, not necessarily the root environment.

That's promising, but it's breaking the previous assumptions for Windows (where root env is checked). Maybe we would check package env first and then only base if absent, but since absence could be both meaningful (is admin) and lack of signal (we didn't try to establish whether admin is needed or not), it'll be ambiguous.

What if we disambiguated by always creating a file, i.e. .nonadmin and .admin. If there is no file, then check base environment (for old paradigm), otherwise it is affirmatively declaring to create shortcuts for all users or just this user.

@jaimergp
Copy link
Contributor Author

jaimergp commented Sep 6, 2023

What if we disambiguated by always creating a file, i.e. .nonadmin and .admin.

That can work for new environments, but we have to figure out a way for old environments too.

I am going to start prototyping something in a PR and we'll see how ugly that looks :D

@jaimergp
Copy link
Contributor Author

jaimergp commented Sep 7, 2023

#156 is now ready for review implementing the discussed approach (sans .admin markers).

@github-project-automation github-project-automation bot moved this from 🆕 New to 🏁 Done in 🧭 Planning Sep 13, 2023
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Sep 13, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
locked [bot] locked due to inactivity
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants