From da89ff08380c072eadf3a25f54e522d8fd4883b7 Mon Sep 17 00:00:00 2001 From: AJ Stuyvenberg Date: Sat, 31 Oct 2026 17:45:09 -0400 Subject: [PATCH 1/2] fix(ls): report readlink failures in long format and exit 1 `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 --- builtins/ls/ls.go | 8 +++ builtins/ls/readlink_error_test.go | 81 ++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 builtins/ls/readlink_error_test.go diff --git a/builtins/ls/ls.go b/builtins/ls/ls.go index ef4a0a9cc..dbb619bcf 100644 --- a/builtins/ls/ls.go +++ b/builtins/ls/ls.go @@ -237,6 +237,10 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { if opts.longFmt && info.Mode()&iofs.ModeSymlink != 0 { if t, err := callCtx.ReadlinkFile(ctx, p); err == nil { pa.linkTarget = t + } else { + // GNU ls still lists the entry, without " -> target". + callCtx.Errf("ls: cannot read symbolic link '%s': %s\n", builtins.SafeOperand(p), callCtx.PortableErr(err)) + failed = true } // Stat the target for -F/-p indicators (nil for dangling links). if ti, err := callCtx.StatFile(ctx, p); err == nil { @@ -397,6 +401,10 @@ func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opt fullPath := joinPath(dir, name) if t, err := callCtx.ReadlinkFile(ctx, fullPath); err == nil { ei.linkTarget = t + } else { + // GNU ls still lists the entry, without " -> target". + callCtx.Errf("ls: cannot read symbolic link '%s': %s\n", builtins.SafeOperand(fullPath), callCtx.PortableErr(err)) + failed = true } // Stat the target for -F/-p indicators (nil for dangling links). if ti, err := callCtx.StatFile(ctx, fullPath); err == nil { diff --git a/builtins/ls/readlink_error_test.go b/builtins/ls/readlink_error_test.go new file mode 100644 index 000000000..e165cf690 --- /dev/null +++ b/builtins/ls/readlink_error_test.go @@ -0,0 +1,81 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package ls + +import ( + "bytes" + "context" + "errors" + iofs "io/fs" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/spf13/pflag" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/DataDog/rshell/builtins" +) + +// runLsWithReadlinkFault runs ls with ReadlinkFile failing with EIO-like +// errors. A symlink target cannot fail to read natively, so the failure has to +// be injected through the CallContext. +func runLsWithReadlinkFault(t *testing.T, args ...string) (stdout, stderr string, code uint8) { + t.Helper() + var out, errOut bytes.Buffer + callCtx := &builtins.CallContext{ + Stdout: &out, + Stderr: &errOut, + Now: time.Now(), + PortableErr: func(err error) string { return err.Error() }, + LstatFile: func(_ context.Context, p string) (iofs.FileInfo, error) { return os.Lstat(p) }, + StatFile: func(_ context.Context, p string) (iofs.FileInfo, error) { return os.Stat(p) }, + ReadDir: func(_ context.Context, p string) ([]iofs.DirEntry, error) { return os.ReadDir(p) }, + ReadlinkFile: func(_ context.Context, p string) (string, error) { + return "", &os.PathError{Op: "readlink", Path: p, Err: errors.New("input/output error")} + }, + } + fs := pflag.NewFlagSet("ls", pflag.ContinueOnError) + handler := registerFlags(fs) + require.NoError(t, fs.Parse(args)) + res := handler(context.Background(), callCtx, fs.Args()) + return out.String(), errOut.String(), res.Code +} + +func TestLsLongReadlinkFailureInDirectory(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "target"), nil, 0o644)) + if err := os.Symlink("target", filepath.Join(dir, "link")); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + + stdout, stderr, code := runLsWithReadlinkFault(t, "-l", dir) + assert.Equal(t, uint8(1), code) + assert.Contains(t, stderr, "ls: cannot read symbolic link '"+filepath.Join(dir, "link")+"': ") + assert.Contains(t, stderr, "input/output error") + // The entry is still listed, without its target. + assert.Contains(t, stdout, " link\n") + assert.NotContains(t, stdout, "->") +} + +func TestLsLongReadlinkFailureOnOperand(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "target"), nil, 0o644)) + link := filepath.Join(dir, "link") + if err := os.Symlink("target", link); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + + stdout, stderr, code := runLsWithReadlinkFault(t, "-l", link) + assert.Equal(t, uint8(1), code) + assert.Contains(t, stderr, "ls: cannot read symbolic link '"+link+"': ") + assert.Contains(t, stderr, "input/output error") + assert.True(t, strings.HasSuffix(strings.TrimRight(stdout, "\n"), " "+link), "entry not listed: %q", stdout) + assert.NotContains(t, stdout, "->") +} From 73742afff230badc110004b4aae20cf38e061683 Mon Sep 17 00:00:00 2001 From: AJ Stuyvenberg Date: Sat, 31 Oct 2026 20:00:09 -0400 Subject: [PATCH 2/2] test(ls): expect the escaped, slash-joined path on Windows 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 --- builtins/ls/readlink_error_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtins/ls/readlink_error_test.go b/builtins/ls/readlink_error_test.go index e165cf690..d04b6b9d6 100644 --- a/builtins/ls/readlink_error_test.go +++ b/builtins/ls/readlink_error_test.go @@ -57,7 +57,7 @@ func TestLsLongReadlinkFailureInDirectory(t *testing.T) { stdout, stderr, code := runLsWithReadlinkFault(t, "-l", dir) assert.Equal(t, uint8(1), code) - assert.Contains(t, stderr, "ls: cannot read symbolic link '"+filepath.Join(dir, "link")+"': ") + assert.Contains(t, stderr, "ls: cannot read symbolic link '"+builtins.SafeOperand(joinPath(dir, "link"))+"': ") assert.Contains(t, stderr, "input/output error") // The entry is still listed, without its target. assert.Contains(t, stdout, " link\n") @@ -74,7 +74,7 @@ func TestLsLongReadlinkFailureOnOperand(t *testing.T) { stdout, stderr, code := runLsWithReadlinkFault(t, "-l", link) assert.Equal(t, uint8(1), code) - assert.Contains(t, stderr, "ls: cannot read symbolic link '"+link+"': ") + assert.Contains(t, stderr, "ls: cannot read symbolic link '"+builtins.SafeOperand(link)+"': ") assert.Contains(t, stderr, "input/output error") assert.True(t, strings.HasSuffix(strings.TrimRight(stdout, "\n"), " "+link), "entry not listed: %q", stdout) assert.NotContains(t, stdout, "->")