Skip to content

fix(ls): report readlink failures in long format and exit 1 - #707

Open
astuyve wants to merge 2 commits into
mainfrom
patina/ls-readlink-error
Open

astuyve wants to merge 2 commits into
mainfrom
patina/ls-readlink-error

Conversation

@astuyve

@astuyve astuyve commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator

What

ls -l called ReadlinkFile at two places (file operands, and entries while listing a directory) and dropped the error. A symlink whose target couldn't be read showed up with no -> target, nothing on stderr, and exit 0.

It now matches GNU ls: it prints ls: cannot read symbolic link 'NAME': ERR on stderr, still lists the entry, and exits 1.

Credit

Found by Jacob Hayes with his deterministic-simulation tool patina (finding #2: SILENT oracle, random EIO leg, about 7.8k runs; replays deterministically with --fs-error-at read_link:2:EIO). Findings thread: https://dd.slack.com/archives/D0AC8RLB21J/p1793475631273749

Tests

There's no native way to make readlink fail, so builtins/ls/readlink_error_test.go injects the failure through CallContext.ReadlinkFile:

  • TestLsLongReadlinkFailureInDirectory
  • TestLsLongReadlinkFailureOnOperand

Both fail on main and pass with this change. go test ./... passes except TestPSPentestLargePIDListDoesNotHang in builtins/ps, which fails the same way on clean main on this host.

🤖 Generated with Claude Code

astuyve and others added 2 commits October 31, 2026 17:45
`ls -l` ignored errors from ReadlinkFile, at both the operand and the
directory-entry call sites, so a symlink whose target could not be read
was listed without ` -> target`, with nothing on stderr and exit 0.
Match GNU ls: print `ls: cannot read symbolic link 'NAME': ERR`, still
list the entry, and exit 1.

Found by Jacob Hayes with patina (https://github.com/JacobHayes/patina),
by injecting EIO into readlink.

Environment: Datadog workspace

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ls quotes operands with SafeOperand, which escapes backslashes, and joins
directory entries with "/", so build the expected path the same way.

Environment: Datadog workspace

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@astuyve
astuyve marked this pull request as ready for review November 1, 2026 02:05
@astuyve
astuyve requested review from a team as code owners November 1, 2026 02:05
@astuyve
astuyve requested a review from pducolin November 1, 2026 02:05

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

1 participant