From a04c90e22d7bc9ca8b2d023d5d771255d1a6e759 Mon Sep 17 00:00:00 2001 From: Solomon Date: Thu, 14 Nov 2024 21:09:02 -0700 Subject: [PATCH] make `ls` return "Permission denied" for CWD instead of empty results (#14310) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ``` --- crates/nu-command/src/filesystem/ls.rs | 68 ++++++++++---------------- crates/nu-command/tests/commands/ls.rs | 23 +++++---- 2 files changed, 40 insertions(+), 51 deletions(-) diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs index b1b231a4a5..c4944c6531 100644 --- a/crates/nu-command/src/filesystem/ls.rs +++ b/crates/nu-command/src/filesystem/ls.rs @@ -287,28 +287,10 @@ fn ls_for_one_pattern( 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 if !directory && tmp_expanded.is_dir() { - if permission_denied(&tmp_expanded) { - #[cfg(unix)] - let error_msg = format!( - "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) { + if read_dir(&tmp_expanded, p_tag, use_threads)? + .next() + .is_none() + { return Ok(Value::test_nothing().into_pipeline_data()); } 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 if directory { (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()); } else { (NuGlob::Expand("*".to_string()), false) @@ -339,7 +321,7 @@ fn ls_for_one_pattern( let path = pattern_arg.into_spanned(p_tag); let (prefix, paths) = if just_read_dir { 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. (Some(expanded), paths) } else { @@ -492,20 +474,6 @@ fn ls_for_one_pattern( .into_pipeline_data(call_span, signals.clone())) } -fn permission_denied(dir: impl AsRef) -> 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) -> bool { - match dir.as_ref().read_dir() { - Err(_) => true, - Ok(mut s) => s.next().is_none(), - } -} - fn is_hidden_dir(dir: impl AsRef) -> bool { #[cfg(windows)] { @@ -979,12 +947,28 @@ mod windows_helper { #[allow(clippy::type_complexity)] fn read_dir( f: &Path, + span: Span, use_threads: bool, ) -> Result> + Send>, ShellError> { - let items = f.read_dir()?.map(|d| { - d.map(|r| r.path()) - .map_err(|e| ShellError::IOError { msg: e.to_string() }) - }); + 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()) + .map_err(|e| ShellError::IOError { msg: e.to_string() }) + }); if !use_threads { let mut collected = items.collect::>(); collected.sort_by(|a, b| { diff --git a/crates/nu-command/tests/commands/ls.rs b/crates/nu-command/tests/commands/ls.rs index 4140ea64bc..416a6aaf7f 100644 --- a/crates/nu-command/tests/commands/ls.rs +++ b/crates/nu-command/tests/commands/ls.rs @@ -378,32 +378,37 @@ fn glob_with_hidden_directory() { #[test] #[cfg(unix)] -fn fails_with_ls_to_dir_without_permission() { +fn fails_with_permission_denied() { Playground::setup("ls_test_1", |dirs, sandbox| { sandbox .within("dir_a") .with_files(&[EmptyFile("yehuda.11.txt"), EmptyFile("jt10.txt")]); - let actual = nu!( + let actual_with_path_arg = nu!( cwd: dirs.test(), pipeline( " 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( " id -u " )); + let is_root = get_uid.out == "0"; - assert!( - actual - .err - .contains("The permissions of 0 do not allow access for this user") - || check_not_root.out == "0" - ); + assert!(actual_with_path_arg.err.contains("Permission denied") || is_root); + + assert!(actual_in_cwd.err.contains("Permission denied") || is_root); }) }