shell(ls): decide operand errors with lstat, not the O_DIRECTORY errno

Merged
RO
3mo ago
+169-28

Problem

  • The ls builtin opens each operand with O_DIRECTORY and reads ENOTDIR as "the operand is a file". The kernel also returns ENOTDIR when a path component is a file, so ls f/x prints f/x and exits 0, and ls -l f/x prints a row of ?. coreutils: ls: cannot access 'f/x': Not a directory, exit 2.
  • Every other open error is fatal, so a dangling or self-referencing symlink operand (ENOENT, ELOOP) is reported as an error. GNU and BSD ls list the link by name, exit 0.
  • The operand went through the -a/-A dotfile filter, so ls .hidden and ls ./f print nothing, exit 0.
  • Cause for all three: ShellLsTask::run_from_thread_pool, src/runtime/shell/builtin/ls.rs:459.

Fix

  • When the open fails, apply the rule coreutils uses for a command-line operand: stat it, and on ENOENT or ELOOP fall back to lstat. A directory that would not open keeps the open error (EACCES, also through a symlink). A failed stat is the error to report (f/x, nx). Anything else that exists is a non-directory and is listed by name. lstat describes it, so -l shows a symlink as a link.
  • Operands bypass the dotfile filter, which only applies to directory entries.
  • shell_lstatat in interpreter.rs mirrors shell_statat so Windows operands get the same path rewrite as the open above.
  • Verified: test/js/bun/shell/commands/ls.test.ts (5 new or updated tests fail on the release build, plus a symlink case in the permission test). The f/x test also carries the ls -d f/x and ls f f/x cases from #39725. Also bunshell.test.ts.

Supersedes #39725, which only fixes the f/x case. Its stat check sits inside the ENOTDIR arm, so it cannot list a dangling link, and it keeps the dotfile filter on operands. Its four test cases pass on this branch. The two this branch did not already have are folded in.

Background

  • The ls builtin runs one ShellLsTask per operand on the thread pool. A task either lists a directory's entries or appends the operand itself to its output. A task that sets err makes the builtin print ls: <path>: <strerror> and exit 1.
  • lstat reports a symlink as itself, stat reports its target. A dangling link exists for lstat and does not exist for stat.
Notes

Cells from the conformance matrix, release 1.4.0 vs this branch vs bash -c, run as a non-root user. f is a regular file, dl -> nonexist, loop -> loop, sub is a directory, restricted is a mode 000 directory, symrestricted -> restricted.

command1.4.0this branchcoreutils 8.32
ls f/x0 f/x1 ls: f/x: Not a directory2 cannot access 'f/x': Not a directory
ls -l f/x0 ?????????? ? ? ? ? ? f/x1 same error2 same error
ls f/x/y/z0 f/x/y/z1 Not a directory2 Not a directory
ls dl1 No such file or directory0 dl0 dl
ls -l dl10 lrwxrwxrwx ... dl0 lrwxrwxrwx ... dl -> nonexist
ls loop1 Too many levels of symbolic links0 loop0 loop
ls loop/x1 ELOOP1 ELOOP2 ELOOP
ls .hidden0 (empty)0 .hidden0 .hidden
ls ./f0 (empty)0 ./f0 ./f
ls sub f/x0 sub:\na\nf/x1 sub:\na + error2 same
ls restricted, ls symrestricted, ls restricted/x1 Permission denied1 Permission denied2 Permission denied
ls f, ls symf, ls symdir, ls nx, ls -d funchangedunchangedmatch

The first version of this branch decided with lstat alone. Review caught that a symlink to an unreadable directory then passed as "exists, not a directory" and swallowed the EACCES. The stat first, lstat on ENOENT/ELOOP order is what coreutils does (gobble_file, DEREF_COMMAND_LINE_SYMLINK_TO_DIR), and the permission test now covers the symlink.

Not changed here:

  • The exit code stays 1. The builtin uses 1 for every error, coreutils uses 2 for a failed operand and 1 for a failed entry.
  • -l does not print -> target for symlinks.
  • ls -l symdir and ls -d symdir still follow a command-line symlink to a directory. coreutils does not with -l or -d.

The two existing broken symlink tests expected the error. They came in with #20531 (a memory refactor) and pinned the behavior of the time. They now expect the coreutils output.

The chmod based permission denied tests fail in any environment that runs as root, with or without this change (#39232). They pass here as nobody.

cargo check -p bun_runtime --target x86_64-pc-windows-msvc passes.


no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/shell/commands/ls.test.ts