Skip to content

Write the full path of the dotnet tools directory in /etc/paths.d/dotnet-cli-tools #35411

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

0xced
Copy link

@0xced 0xced commented Sep 14, 2023

Fixes #23165 and #9415

Also remove the instructions to manually tweak the PATH when using zsh since they are no longer necessary.

@ghost ghost added Area-Tools untriaged Request triage from a team member labels Sep 14, 2023
@0xced 0xced force-pushed the fix-dotnet-tools-path branch from 3aaf9ed to c57c8f1 Compare September 14, 2023 20:04
This is no longer necessary when the full path is specified in /etc/paths.d/dotnet-cli-tools as per previous commit.

Also rename OsxBashEnvironmentPath into OsxEnvironmentPath since it's not shell specific.
@jrdodds
Copy link

jrdodds commented Sep 14, 2023

The /etc/paths.d/ directory and the dotnet-cli-tools file are global for all users. Writing the full path of a specific user in this file should not be done.

@b4ux1t3
Copy link

b4ux1t3 commented Sep 14, 2023

The /etc/paths.d/ directory and the dotnet-cli-tools file are global for all users. Writing the full path of a specific user in this file should not be done.

I'm trying to understand the issue here.

On the one hand, it could expose some of a user's home folder structure.

However, it's generally (and by that I mean I would be surprised if you found even one counter-example) going to be the very well-documented dotnet tools path, and as such isn't likely to cause any raised eyebrows.

For most users, even if there is another user's home path in that global path, there won't be any issues, as they likely won't have permissions to that other user's path and, as such, path resolution will just skip that path as "empty".

There are no reasonable security concerns here. There are no performance concerns that aren't inherent with any path lookup. There are, however, very real concerns of users not being able to use global dotnet tools at all.

@cicorias
Copy link
Member

cicorias commented Mar 1, 2024

The /etc/paths.d/ directory and the dotnet-cli-tools file are global for all users. Writing the full path of a specific user in this file should not be done.

This is such a rare situation that a dotnet sdk developer will have a device that other users are also using. Even if it did, the comment that @b4ux1t3 indicates -- it won't affect other users -- is true as well -- lack of perms.

@jrdodds
Copy link

jrdodds commented Mar 1, 2024

The root issue is that "global" tools are not installed globally. They are installed only for a specific user.

If the global tools were installed in a folder under /usr/local/share/dotnet, that tools path could be shared in /etc/paths.d/.

Or 'global' actually means user scoped and the tools path in the user directory needs to be added to the PATH in a user scoped file (e.g. .zprofile) and not via /etc/paths.d/.

Also note that it appears that the dotnet-cli-tools file is overwritten and prior values are not preserved. If that is the case and if UserA installs a global tool and then UserB installs the same tool globally, UserA will be unable to use the tool.

@cicorias
Copy link
Member

cicorias commented Mar 1, 2024

again, macOS, dotnet sdk -- can't fathom a multi-user situation in reality

Perfect is the enemy of good

@jrdodds
Copy link

jrdodds commented Mar 1, 2024

macOS is a multi-user operating system. I use fast user switching with family members. My son took a course that used C# and Unity. For me, although rare, it's not unfathomable to imagine this issue in reality.

How many places to the left of the decimal point are needed to show a stock share price? 3? A share of Berkshire Hathaway Class A is 6 figures. Berkshire Hathaway is a very rare situation but I think a stock ticker display that doesn't handle share prices that large is simply not good. 'Perfect' is not relevant. In a similar way, the behavior of the defect is not good.

This PR is, In essence, doubling down on the original defect and that is arguably worse than the current situation. If it were my decision (and to be clear it is very much not), I would reject this PR.

@b4ux1t3
Copy link

b4ux1t3 commented Mar 6, 2024

@jrdodds So, you're saying if they allowed for a user scope for dotnet tools, adding further complexity, that would be better than fixing it for a vast majority of use cases without adding any additional complexity?

@jrdodds
Copy link

jrdodds commented Mar 7, 2024

@jrdodds So, you're saying if they allowed for a user scope for dotnet tools, adding further complexity, that would be better than fixing it for a vast majority of use cases without adding any additional complexity?

No. I'm not saying that. I don't think another 'scope' should be added.

@jrdodds
Copy link

jrdodds commented Mar 7, 2024

For dotnet tool, the "global" scope is the scope of a specific user. It does not mean global to all users. It means global to all projects for a specific user.

On Windows the install folder for a global tool is %USERPROFILE%\.dotnet\tools. This is added to the PATH in the user environment variables.

On macOS the install folder for a global tool is $HOME/.dotnet/tools. It follows that the PATH should be updated in a manner roughly equivalent to the Windows user environment variables. That would be $HOME/.zprofile or another file in the user home directory.

But instead the "All Users" /etc/paths.d/ mechanism is used. This is a defect.

OS macOS Windows
All Users /etc/paths.d/ System Env Var PATH
Specific User $HOME/.zprofile User Env Var PATH

The message about a one time user action to add the tools directory to the ~/.zprofile is correctly addressing the defect.

An improvement over the current message would be to extend the dotnet tool to have an equivalent to Homebrew's brew shellenv.

(Note that shell command completion for dotnet is already implemented with a dotnet complete command.)

@b4ux1t3
Copy link

b4ux1t3 commented Mar 7, 2024

There is a marked difference between the concept of environment variables on Windows and *nix (Including, for this case, Mac).

I do not allow third party tools to edit my shell configuration files, such as .zprofile or .zshrc. I use my dotfiles on multiple systems with multiple operating systems (including sharing it between Linux and Mac).

This is not an uncommon use of those files. They are not machine-specific configurations, they are user-specific configurations, which must maintain usability wherever that user uses them, even on other machines.

Conversely, /etc/paths.d is a machine-specific configuration, just like the Windows environment variable setup.

How Windows separates the User Environment from the System Environment is a Windows-specific detail, and should not be applied to other platforms. The User Environment is a configuration for that User on that machine. Yes, it can be synchronized with a federated windows login. That isn't how it is generally used.

When you're working on different platforms, you have to take the conventions of the platforms into account. For many *nix developers, both on Linux and Mac, their shell configuration is their shell configuration. It isn't a dumping ground for whatever nonsense other applications want to put there.

That's what paths.d is for. That, in this case, it happens to include paths that aren't accessible to all users isn't a defect, it's a side effect, and it doesn't hurt anything.

@KalleOlaviNiemitalo
Copy link
Contributor

I can imagine some ways for a reference into another user's home directory to hurt something:

  • If the home directory is automounted from a network server that is not reachable, causing command lookups to take a long time before timing out. If this actually happens, it will be easy enough to fix.
  • If the owner of the directory deliberately makes the directory accessible to other users and installs malware to it. Then the question is how such an untrustworthy user was able to edit paths.d in the first place. It could happen if a trusted user first installs .NET and then their user account is deleted and a new user account with the same home directory path is created for a malicious user. Seems rather farfetched.
  • If there is some sort of audit system that generates an alarm when a user attempts to access the home directory of another.

@b4ux1t3
Copy link

b4ux1t3 commented Mar 7, 2024

@KalleOlaviNiemitalo You're absolutely right. Those are concerns. I just don't believe that they're reasonable concerns (I don't know if you're arguing they are, just clarifying my position).

What is a reasonable concern, I think, is allowing a piece of software to change my known-good shell configuration just because Windows and *nix systems work differently.

@jrdodds
Copy link

jrdodds commented Mar 7, 2024

I very strongly agree with:

When you're working on different platforms, you have to take the conventions of the platforms into account. For many *nix developers, both on Linux and Mac, their shell configuration is their shell configuration. It isn't a dumping ground for whatever nonsense other applications want to put there.

I disagree with:

That's what paths.d is for. That, in this case, it happens to include paths that aren't accessible to all users isn't a defect, it's a side effect, and it doesn't hurt anything.

When you dig into it, paths.d is not for user specific configuration. One reason is that paths.d is in /etc and user configuration in /etc breaks convention. (That paths.d 'works' with the bash shell is a side effect of how bash handles the tilde.)

There are tools that respect both that paths.d is not for user specific configuration and that dotfiles should not be edited by a third party. Usually this comes down to the tool needing an action on the user's part to update a dotfile, as dotnet currently does.

There are 'replacement' versions of path_helper written by others that extend the feature to support per user paths by adding support for a user level paths.d folder, e.g. $HOME/Library/Paths/paths.d.

We agree that the conventions and rules of a platform have to be taken into account. It's not okay to break a rule because the rule is inconvenient. We disagree about the rules for /etc/paths.d.

@KalleOlaviNiemitalo
Copy link
Contributor

@b4ux1t3, to clarify my position: I'm averse to adding per-user directories to system-wide $PATH and would not do such a thing in any software I maintain; but each scenario in which it actually matters seems to be unlikely or easily mitigated.

@b4ux1t3
Copy link

b4ux1t3 commented Mar 7, 2024

@KalleOlaviNiemitalo Yeah, I wouldn't do it in most situations.

In this specific situation, I'm saying that it makes sense given the oddities of the system at play.

Specifically, "global" not meaning system-wide in .NET tooling combined with the convention of not messing with a user's shell configuration.

There are obviously other ways to resolve this; all of them require adding a more complexity and could take a lot more time.

My ideal fix would be to install global dotnet tools to a shared directory, no differently to how other tools' global packages work.

Of course, that would require users to install global packages with root access. . .which is a thing I explicitly dislike about said tools.

But, in the mean time, the functionality doesn't exist at all out of the box for some users.

@cicorias
Copy link
Member

cicorias commented Mar 7, 2024

well, first I'd opt for to stop mucking with /etc/path.d altogether and just have the installer emit a clear message -- or the dotnet tool itself provide a warning of some thype that "someting is misssing"..

it's tiresome to keep telling macOS folks to change that file on every SDK update.

@jrdodds
Copy link

jrdodds commented Mar 7, 2024

it's tiresome to keep telling macOS folks to change that file on every SDK update.

@cicorias Are you telling people to change the /etc/paths.d/dotnet-cli-tools file?

For zsh, the following could be added to .zprofile:

# replace ~ with $HOME
export PATH=${PATH//\~/$HOME}

@cicorias
Copy link
Member

cicorias commented Mar 7, 2024

it's tiresome to keep telling macOS folks to change that file on every SDK update.

@cicorias Are you telling people to change the /etc/paths.d/dotnet-cli-tools file?

For zsh, the following could be added to .zprofile:

# replace ~ with $HOME
export PATH=${PATH//\~/$HOME}

I was -- however, I'll try this -- tx.

@cicorias
Copy link
Member

cicorias commented Mar 8, 2024

it's tiresome to keep telling macOS folks to change that file on every SDK update.

@cicorias Are you telling people to change the /etc/paths.d/dotnet-cli-tools file?
For zsh, the following could be added to .zprofile:

# replace ~ with $HOME
export PATH=${PATH//\~/$HOME}

I was -- however, I'll try this -- tx.

@jrdodds thanks so much. This works well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Tools untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

macOS dotnet CLI path for global tools is bad - Installer issue
5 participants