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

Cannot cd into directory when permissions are granted via ACLs instead of traditional unix permissions #9046

Open
andkit opened this issue Apr 28, 2023 · 7 comments · May be fixed by #12507
Labels
coreutils-uutils Changes relating to coreutils/uutils file-system Related to commands and core nushell behavior around the file system platform-specific platform-specific issues

Comments

@andkit
Copy link

andkit commented Apr 28, 2023

I have a couple of directories that are owned by other users, but where I have permissions via ACLs
Something like this:

^mkdir -m 0770 foo
setfacl -m u:myuser:rwX -m d:u:myuser:rwX foo

cd /tmp/foo results in Cannot change directory to /tmp/foo: You are neither the owner, in the group, nor the super user and do not have permission

however exec sh -c 'cd /tmp/foo; exec nu' works fine and running ls in the resulting shell works just as well

I had a brief look at the code and it seems that on windows have_permission() tries to actually read_dir() the directory that we want to cd into, but only linux it tries to guess whether we have access from the metadata by looking at owner, group and the respective permission bits.

This can lead to false negatives (as in my case), and maybe false positives as well when things like selinux come into play (I haven't had much contact with that, but I vaguely recall something like that is possible)

As the current error messages are more helpful to inexperienced users, I think my preferred approach to improve this would be:

  • try read_dir() first, akin to whats done on windows
  • If that works, don't perform any other checks and allow cd to proceed (fixes my false negative)
  • If it doesn't, do the current look-at-metadata-thing
  • Return any error just as it does now (keep helpfull error messages)
  • If the metadata looks like it should have worked, pass on the error message from the original read_dir (instead of the current OK return value, to catch eventual false positives)

But I'd also be fine with something like cd --force or anything else that just lets me get on with it ;)

@sholderbach sholderbach added platform-specific platform-specific issues file-system Related to commands and core nushell behavior around the file system labels Apr 29, 2023
@sholderbach
Copy link
Member

Thanks for including potential strategies to resolve that!

Would you mind adding your current platform and the build info from the bug report template for future reference:

version | transpose key value | to md --pretty

@andkit
Copy link
Author

andkit commented Apr 30, 2023

Yep, here it is:

key value
version 0.79.0
branch makepkg
commit_hash a1b7261
build_os linux-x86_64
build_target x86_64-unknown-linux-gnu
rust_version rustc 1.69.0 (84c898d65 2023-04-16) (Arch Linux rust 1:1.69.0-2)
cargo_version cargo 1.69.0
build_time 2023-04-27 07:32:56 +00:00
build_rust_channel release
features default, zip
installed_plugins

@crabdancing
Copy link

crabdancing commented Dec 17, 2023

I ran into this on NixOS. Version:

key value
version 0.87.1
branch
commit_hash
build_os linux-x86_64
build_target x86_64-unknown-linux-gnu
rust_version rustc 1.73.0 (cc66ad468 2023-10-03) (built from a source tarball)
cargo_version cargo 1.73.0
build_time 1980-01-01 00:00:00 +00:00
build_rust_channel release
allocator mimalloc
features dataframe, default, extra, sqlite, trash, which, zip
installed_plugins

(er... the 1980 thing is because the nix build system resets the apparent system time for all build processes to ensure reproduciblity)

Anyways, this makes Nushell practically unusable on ACL-heavy systems. :(

I would be willing to write a patch if this will take a long time for others to get to, but I need to know what solution y'all would deem acceptable. Would trying to read the directory, catching the failure, and then printing the error be acceptable?

@krzysztof-bochm
Copy link

due to this bug i have also found out that udisks2 or udiskie ( idk which one is the actual backend and handles mounting ) also uses acl based access to /run/media/* so you cannot cd into ur drives folder

u can still cd to drive folder but that is annoying

key value
version 0.89.0
branch makepkg
commit_hash 2c1560e
build_os linux-x86_64
build_target x86_64-unknown-linux-gnu
rust_version rustc 1.75.0 (82e1608df 2023-12-21) (Arch Linux rust 1:1.75.0-1)
cargo_version cargo 1.75.0
build_time 2024-01-09 23:05:05 +00:00
build_rust_channel release
allocator mimalloc
features dataframe, default, extra, sqlite, trash, which, zip
installed_plugins

@sholderbach sholderbach added the coreutils-uutils Changes relating to coreutils/uutils label Feb 21, 2024
@sholderbach
Copy link
Member

sholderbach commented Feb 21, 2024

This is certainly limiting Nushell on more complex systems.

We plan on moving the core of our file-system related commands over to rely on the implementation from https://github.com/uutils/coreutils #11549.

While cd is not part of the coreutils and responsibility of the shell, we can possibly learn from their general permission management code to deal with this consistently.

@crabdancing
Copy link

The fix seems pretty simple. It's easier to ask forgiveness than permission. Actually try to cd into the directory, and see if anything Err's, no?

andkit added a commit to andkit/nushell that referenced this issue Apr 13, 2024
The unix implementation of cd fails with a permission error when
classical unix permissions deny access to a directory, but additional mechanisms like
ACLs actually grant the user access

Fix this by miroring the windows implementation: if we can read_dir()
without error => allow the cd to proceed.

Addtionally we also handle the case where it is the other way round:
unix permissions grant access but additional security modules (SElinux,
apparmor, etc.) deny the current user/context.
andkit added a commit to andkit/nushell that referenced this issue Apr 13, 2024
The unix implementation of cd fails with a permission error when
classical unix permissions deny access to a directory, but additional mechanisms like
ACLs actually grant the user access

Fix this by miroring the windows implementation: if we can read_dir()
without error => allow the cd to proceed.

Addtionally we also handle the case where it is the other way round:
unix permissions grant access but additional security modules (SElinux,
apparmor, etc.) deny the current user/context.
@andkit
Copy link
Author

andkit commented Apr 13, 2024

As there seems some interest in this by other people I just gave fixing this a go, maybe it'll help someone, even if it gets replaced by something more elaborate in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coreutils-uutils Changes relating to coreutils/uutils file-system Related to commands and core nushell behavior around the file system platform-specific platform-specific issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants