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

cd fails for directory without read permission, displays Permission denied #10432

Open
TheJonny opened this issue Apr 9, 2024 · 3 comments
Open

Comments

@TheJonny
Copy link

TheJonny commented Apr 9, 2024

cd or pushd into a directory without read permission (but with x permission) does not work in fish.

jonny@charon /> sh -c 'env HOME=$(mktemp -d) XDG_CONFIG_HOME= fish'
Welcome to fish, the friendly interactive shell
Type help for instructions on how to use fish
jonny@charon /> echo $version
3.7.1
jonny@charon /> ls -ld /srv
drwxr-x--x 1 root root 48 14. Jan 23:51 /srv/
jonny@charon /> cd /srv
cd: Permission denied: '/srv'
jonny@charon / [1]> bash
[jonny@charon /]$ cd /srv
[jonny@charon srv]$ pwd
/srv

This transscript was on archlinux. The same behaviour on debian stable with fish 3.6.0

it is possible to cd through the directory, if I have read permission in the target, e.g. cd /srv/www.

@TheJonny
Copy link
Author

TheJonny commented Apr 9, 2024

here an strace.

strace.log

the only EACCES occurs in line 2947, followed by printing the error message:

2947 openat(AT_FDCWD, "/srv", O_RDONLY|O_CLOEXEC) = -1 EACCES (Permission denied)
2948 write(2, "cd: Permission denied: '/srv'\n", 30) = 30

@TheJonny
Copy link
Author

TheJonny commented Apr 9, 2024

builtins/cd.rs calls fds::wopen_dir, which calls fds::open_dir, which sets O_RDONLY. changing it to O_PATH works for me on Linux, but is neither portable nor correct for a function called open_dir

But it looks like open_dir is only used in cd and in the parser to initialize LibraryData::cwd_fd.
It also seems like cwd_fd is never read (grep does not find usages, and it still builds when changing it to pub(crate) in struct LibraryData.)

so can we just remove the functions, skip the filedescriptor and directly chdir? this would be portable to platforms without O_PATH (like it seems, macos) or O_SEARCH (all platforms, even if this is specified by posix)

@TheJonny
Copy link
Author

TheJonny commented Apr 9, 2024

If we want to keep cwd_fd, we can first chdir and then open(".", ...) (as done in the Parser's constructor).
As cwd_fd is an Option, this would be ok to fail.

The flag can be O_SEARCH, O_SEARCH or O_RDONLY, whatever is supported on the platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant