make ls return "Permission denied" for CWD instead of empty results (#14310)

Fixes #14265

# User-Facing Changes

`ls` without a path argument now errors when the current working
directory is unreadable due to missing permissions:

```diff
mkdir foo
chmod 100 foo
cd foo
ls | to nuon
-[]
+Error:   × Permission denied
```
This commit is contained in:
Solomon 2024-11-14 21:09:02 -07:00 committed by GitHub
parent a84d410f11
commit a04c90e22d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 40 additions and 51 deletions

View File

@ -287,28 +287,10 @@ fn ls_for_one_pattern(
nu_path::expand_path_with(pat.item.as_ref(), &cwd, pat.item.is_expand()); nu_path::expand_path_with(pat.item.as_ref(), &cwd, pat.item.is_expand());
// Avoid checking and pushing "*" to the path when directory (do not show contents) flag is true // Avoid checking and pushing "*" to the path when directory (do not show contents) flag is true
if !directory && tmp_expanded.is_dir() { if !directory && tmp_expanded.is_dir() {
if permission_denied(&tmp_expanded) { if read_dir(&tmp_expanded, p_tag, use_threads)?
#[cfg(unix)] .next()
let error_msg = format!( .is_none()
"The permissions of {:o} do not allow access for this user", {
tmp_expanded
.metadata()
.expect("this shouldn't be called since we already know there is a dir")
.permissions()
.mode()
& 0o0777
);
#[cfg(not(unix))]
let error_msg = String::from("Permission denied");
return Err(ShellError::GenericError {
error: "Permission denied".into(),
msg: error_msg,
span: Some(p_tag),
help: None,
inner: vec![],
});
}
if is_empty_dir(&tmp_expanded) {
return Ok(Value::test_nothing().into_pipeline_data()); return Ok(Value::test_nothing().into_pipeline_data());
} }
just_read_dir = !(pat.item.is_expand() && pat.item.as_ref().contains(GLOB_CHARS)); just_read_dir = !(pat.item.is_expand() && pat.item.as_ref().contains(GLOB_CHARS));
@ -327,7 +309,7 @@ fn ls_for_one_pattern(
// Avoid pushing "*" to the default path when directory (do not show contents) flag is true // Avoid pushing "*" to the default path when directory (do not show contents) flag is true
if directory { if directory {
(NuGlob::Expand(".".to_string()), false) (NuGlob::Expand(".".to_string()), false)
} else if is_empty_dir(&cwd) { } else if read_dir(&cwd, p_tag, use_threads)?.next().is_none() {
return Ok(Value::test_nothing().into_pipeline_data()); return Ok(Value::test_nothing().into_pipeline_data());
} else { } else {
(NuGlob::Expand("*".to_string()), false) (NuGlob::Expand("*".to_string()), false)
@ -339,7 +321,7 @@ fn ls_for_one_pattern(
let path = pattern_arg.into_spanned(p_tag); let path = pattern_arg.into_spanned(p_tag);
let (prefix, paths) = if just_read_dir { let (prefix, paths) = if just_read_dir {
let expanded = nu_path::expand_path_with(path.item.as_ref(), &cwd, path.item.is_expand()); let expanded = nu_path::expand_path_with(path.item.as_ref(), &cwd, path.item.is_expand());
let paths = read_dir(&expanded, use_threads)?; let paths = read_dir(&expanded, p_tag, use_threads)?;
// just need to read the directory, so prefix is path itself. // just need to read the directory, so prefix is path itself.
(Some(expanded), paths) (Some(expanded), paths)
} else { } else {
@ -492,20 +474,6 @@ fn ls_for_one_pattern(
.into_pipeline_data(call_span, signals.clone())) .into_pipeline_data(call_span, signals.clone()))
} }
fn permission_denied(dir: impl AsRef<Path>) -> bool {
match dir.as_ref().read_dir() {
Err(e) => matches!(e.kind(), std::io::ErrorKind::PermissionDenied),
Ok(_) => false,
}
}
fn is_empty_dir(dir: impl AsRef<Path>) -> bool {
match dir.as_ref().read_dir() {
Err(_) => true,
Ok(mut s) => s.next().is_none(),
}
}
fn is_hidden_dir(dir: impl AsRef<Path>) -> bool { fn is_hidden_dir(dir: impl AsRef<Path>) -> bool {
#[cfg(windows)] #[cfg(windows)]
{ {
@ -979,9 +947,25 @@ mod windows_helper {
#[allow(clippy::type_complexity)] #[allow(clippy::type_complexity)]
fn read_dir( fn read_dir(
f: &Path, f: &Path,
span: Span,
use_threads: bool, use_threads: bool,
) -> Result<Box<dyn Iterator<Item = Result<PathBuf, ShellError>> + Send>, ShellError> { ) -> Result<Box<dyn Iterator<Item = Result<PathBuf, ShellError>> + Send>, ShellError> {
let items = f.read_dir()?.map(|d| { let items = f
.read_dir()
.map_err(|error| {
if error.kind() == std::io::ErrorKind::PermissionDenied {
return ShellError::GenericError {
error: "Permission denied".into(),
msg: "The permissions may not allow access for this user".into(),
span: Some(span),
help: None,
inner: vec![],
};
}
error.into()
})?
.map(|d| {
d.map(|r| r.path()) d.map(|r| r.path())
.map_err(|e| ShellError::IOError { msg: e.to_string() }) .map_err(|e| ShellError::IOError { msg: e.to_string() })
}); });

View File

@ -378,32 +378,37 @@ fn glob_with_hidden_directory() {
#[test] #[test]
#[cfg(unix)] #[cfg(unix)]
fn fails_with_ls_to_dir_without_permission() { fn fails_with_permission_denied() {
Playground::setup("ls_test_1", |dirs, sandbox| { Playground::setup("ls_test_1", |dirs, sandbox| {
sandbox sandbox
.within("dir_a") .within("dir_a")
.with_files(&[EmptyFile("yehuda.11.txt"), EmptyFile("jt10.txt")]); .with_files(&[EmptyFile("yehuda.11.txt"), EmptyFile("jt10.txt")]);
let actual = nu!( let actual_with_path_arg = nu!(
cwd: dirs.test(), pipeline( cwd: dirs.test(), pipeline(
" "
chmod 000 dir_a; ls dir_a chmod 000 dir_a; ls dir_a
" "
)); ));
let check_not_root = nu!( let actual_in_cwd = nu!(
cwd: dirs.test(), pipeline(
"
chmod 100 dir_a; cd dir_a; ls
"
));
let get_uid = nu!(
cwd: dirs.test(), pipeline( cwd: dirs.test(), pipeline(
" "
id -u id -u
" "
)); ));
let is_root = get_uid.out == "0";
assert!( assert!(actual_with_path_arg.err.contains("Permission denied") || is_root);
actual
.err assert!(actual_in_cwd.err.contains("Permission denied") || is_root);
.contains("The permissions of 0 do not allow access for this user")
|| check_not_root.out == "0"
);
}) })
} }