From 70c2aa1aa4b4179fb4c578836349ade1c1c059b0 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 1 Apr 2026 16:40:28 -0400 Subject: [PATCH 1/4] feat(ls): display symlink targets in long format output ls -l now shows `-> target` for symbolic links, matching GNU ls behavior. Adds Sandbox.Readlink and CallContext.ReadlinkFile to read symlink destinations through the sandbox. Co-Authored-By: Claude Opus 4.6 (1M context) --- allowedpaths/sandbox.go | 16 ++++++ builtins/builtins.go | 4 ++ builtins/ls/ls.go | 55 +++++++++++++------ interp/runner_exec.go | 6 ++ .../cmd/ls/long_format/symlink_target.yaml | 19 +++++++ .../ls/long_format/symlink_target_in_dir.yaml | 20 +++++++ 6 files changed, 102 insertions(+), 18 deletions(-) create mode 100644 tests/scenarios/cmd/ls/long_format/symlink_target.yaml create mode 100644 tests/scenarios/cmd/ls/long_format/symlink_target_in_dir.yaml diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index f83c2b8e..4da1f32b 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -407,6 +407,22 @@ func (s *Sandbox) Lstat(path string, cwd string) (fs.FileInfo, error) { return info, nil } +// Readlink returns the destination of a symbolic link within the sandbox. +func (s *Sandbox) Readlink(path string, cwd string) (string, error) { + absPath := toAbs(path, cwd) + + root, relPath, ok := s.resolve(absPath) + if !ok { + return "", &os.PathError{Op: "readlink", Path: path, Err: os.ErrPermission} + } + + target, err := root.Readlink(relPath) + if err != nil { + return "", PortablePathError(err) + } + return target, nil +} + // Close releases all os.Root file descriptors. It is safe to call multiple times. func (s *Sandbox) Close() error { if s == nil { diff --git a/builtins/builtins.go b/builtins/builtins.go index 3fec7afa..6d78301b 100644 --- a/builtins/builtins.go +++ b/builtins/builtins.go @@ -115,6 +115,10 @@ type CallContext struct { // LstatFile returns file info within the shell's path restrictions (does not follow symlinks). LstatFile func(ctx context.Context, path string) (fs.FileInfo, error) + // ReadlinkFile returns the destination of a symbolic link within the + // shell's path restrictions. + ReadlinkFile func(ctx context.Context, path string) (string, error) + // AccessFile checks whether the file at path is accessible with the given mode // within the shell's path restrictions. Mode: 0x04=read, 0x02=write, 0x01=execute. AccessFile func(ctx context.Context, path string, mode uint32) error diff --git a/builtins/ls/ls.go b/builtins/ls/ls.go index 5d8d1cd0..37760a9a 100644 --- a/builtins/ls/ls.go +++ b/builtins/ls/ls.go @@ -233,7 +233,13 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { } } if !isDir || opts.dirOnly { - files = append(files, pathArg{name: p, info: info}) + pa := pathArg{name: p, info: info} + if opts.longFmt && info.Mode()&iofs.ModeSymlink != 0 { + if t, err := callCtx.ReadlinkFile(ctx, p); err == nil { + pa.linkTarget = t + } + } + files = append(files, pa) } else { dirs = append(dirs, pathArg{name: p, info: info}) } @@ -251,7 +257,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { cw = computeColWidths(files, func(a pathArg) iofs.FileInfo { return a.info }, func(a pathArg) (string, string, string) { return a.owner, a.group, a.nlink }, opts) } for _, f := range files { - printEntry(callCtx, f.name, f.info, f.owner, f.group, f.nlink, opts, now, cw) + printEntry(callCtx, f.name, f.info, f.owner, f.group, f.nlink, f.linkTarget, opts, now, cw) } } @@ -297,11 +303,12 @@ type options struct { } type pathArg struct { - name string - info iofs.FileInfo - owner string // cached by fileOwner (populated when longFmt) - group string - nlink string + name string + info iofs.FileInfo + owner string // cached by fileOwner (populated when longFmt) + group string + nlink string + linkTarget string // symlink destination (populated when longFmt + symlink) } func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opts *options, depth int, now time.Time) error { @@ -347,12 +354,13 @@ func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opt // Get FileInfo for sorting (if needed) and for long format. type entryInfo struct { - name string - info iofs.FileInfo - isSymlink bool - owner string // cached by fileOwner (populated when longFmt) - group string - nlink string + name string + info iofs.FileInfo + isSymlink bool + owner string // cached by fileOwner (populated when longFmt) + group string + nlink string + linkTarget string // symlink destination (populated when longFmt + symlink) } failed := false @@ -372,11 +380,17 @@ func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opt failed = true continue } - infoEntries = append(infoEntries, entryInfo{ + ei := entryInfo{ name: name, info: info, isSymlink: e.Type()&iofs.ModeSymlink != 0, - }) + } + if opts.longFmt && ei.isSymlink { + if t, err := callCtx.ReadlinkFile(ctx, joinPath(dir, name)); err == nil { + ei.linkTarget = t + } + } + infoEntries = append(infoEntries, ei) } // Synthesize . and .. for -a (os.ReadDir never includes them). @@ -432,7 +446,7 @@ func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opt if ctx.Err() != nil { break } - printEntry(callCtx, ei.name, ei.info, ei.owner, ei.group, ei.nlink, opts, now, cw) + printEntry(callCtx, ei.name, ei.info, ei.owner, ei.group, ei.nlink, ei.linkTarget, opts, now, cw) } // Only warn on implicit truncation (no explicit --offset/--limit). @@ -525,7 +539,7 @@ func computeColWidths[T any](entries []T, getInfo func(T) iofs.FileInfo, getOwne return w } -func printEntry(callCtx *builtins.CallContext, name string, info iofs.FileInfo, owner, group, nlink string, opts *options, now time.Time, cw colWidths) { +func printEntry(callCtx *builtins.CallContext, name string, info iofs.FileInfo, owner, group, nlink, linkTarget string, opts *options, now time.Time, cw colWidths) { if opts.longFmt { mode := formatMode(info) size := info.Size() @@ -538,12 +552,17 @@ func printEntry(callCtx *builtins.CallContext, name string, info iofs.FileInfo, sizeStr = fmt.Sprintf("%d", size) } + suffix := indicator(info, opts) + if linkTarget != "" { + suffix = " -> " + linkTarget + } + timeStr := formatTime(modTime, now) callCtx.Outf("%s %*s %-*s %-*s %*s %s %s%s\n", mode, cw.nlink, nlink, cw.owner, owner, cw.group, group, cw.size, sizeStr, timeStr, - name, indicator(info, opts)) + name, suffix) } else { callCtx.Outf("%s%s\n", name, indicator(info, opts)) } diff --git a/interp/runner_exec.go b/interp/runner_exec.go index dfb9868e..62734ee4 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -304,6 +304,9 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { LstatFile: func(ctx context.Context, path string) (fs.FileInfo, error) { return r.sandbox.Lstat(path, dir) }, + ReadlinkFile: func(ctx context.Context, path string) (string, error) { + return r.sandbox.Readlink(path, dir) + }, AccessFile: func(ctx context.Context, path string, mode uint32) error { return r.sandbox.Access(path, dir, mode) }, @@ -363,6 +366,9 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) { LstatFile: func(ctx context.Context, path string) (fs.FileInfo, error) { return r.sandbox.Lstat(path, HandlerCtx(r.handlerCtx(ctx, todoPos)).Dir) }, + ReadlinkFile: func(ctx context.Context, path string) (string, error) { + return r.sandbox.Readlink(path, HandlerCtx(r.handlerCtx(ctx, todoPos)).Dir) + }, AccessFile: func(ctx context.Context, path string, mode uint32) error { return r.sandbox.Access(path, HandlerCtx(r.handlerCtx(ctx, todoPos)).Dir, mode) }, diff --git a/tests/scenarios/cmd/ls/long_format/symlink_target.yaml b/tests/scenarios/cmd/ls/long_format/symlink_target.yaml new file mode 100644 index 00000000..d57dcf54 --- /dev/null +++ b/tests/scenarios/cmd/ls/long_format/symlink_target.yaml @@ -0,0 +1,19 @@ +description: ls -l shows symlink target with -> arrow +# skip: rshell displays numeric UID/GID while GNU ls resolves usernames. +skip_assert_against_bash: true +setup: + files: + - path: file.txt + content: "hello" + chmod: 0644 + - path: link.txt + symlink: file.txt +input: + allowed_paths: ["$DIR"] + script: |+ + ls -l link.txt +expect: + stdout_contains: + - "link.txt -> file.txt" + stderr: "" + exit_code: 0 diff --git a/tests/scenarios/cmd/ls/long_format/symlink_target_in_dir.yaml b/tests/scenarios/cmd/ls/long_format/symlink_target_in_dir.yaml new file mode 100644 index 00000000..e02c384d --- /dev/null +++ b/tests/scenarios/cmd/ls/long_format/symlink_target_in_dir.yaml @@ -0,0 +1,20 @@ +description: ls -l in a directory shows symlink targets with -> arrow +# skip: rshell displays numeric UID/GID while GNU ls resolves usernames. +skip_assert_against_bash: true +setup: + files: + - path: dir/file.txt + content: "hello" + chmod: 0644 + - path: dir/link.txt + symlink: file.txt +input: + allowed_paths: ["$DIR"] + script: |+ + ls -l dir/ +expect: + stdout_contains: + - "link.txt -> file.txt" + - "file.txt" + stderr: "" + exit_code: 0 From dbff91ca17ff228ac79ccb68ceacc39db863b5a3 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 1 Apr 2026 16:52:41 -0400 Subject: [PATCH 2/4] fix(ls): preserve classification indicators on symlink targets ls -lF now shows indicators on the symlink target (e.g. dirlink -> subdir/, filelink -> file.txt*), matching GNU ls behavior. The target's FileInfo is obtained via StatFile and passed to indicator(). Dangling symlinks show no target indicator, also matching GNU ls. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/ls/ls.go | 51 ++++++++++++------- .../cmd/ls/long_format/symlink_classify.yaml | 24 +++++++++ 2 files changed, 57 insertions(+), 18 deletions(-) create mode 100644 tests/scenarios/cmd/ls/long_format/symlink_classify.yaml diff --git a/builtins/ls/ls.go b/builtins/ls/ls.go index 37760a9a..c34d5642 100644 --- a/builtins/ls/ls.go +++ b/builtins/ls/ls.go @@ -238,6 +238,10 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { if t, err := callCtx.ReadlinkFile(ctx, p); err == nil { pa.linkTarget = t } + // Stat the target for -F/-p indicators (nil for dangling links). + if ti, err := callCtx.StatFile(ctx, p); err == nil { + pa.linkTargetInfo = ti + } } files = append(files, pa) } else { @@ -257,7 +261,7 @@ func registerFlags(fs *builtins.FlagSet) builtins.HandlerFunc { cw = computeColWidths(files, func(a pathArg) iofs.FileInfo { return a.info }, func(a pathArg) (string, string, string) { return a.owner, a.group, a.nlink }, opts) } for _, f := range files { - printEntry(callCtx, f.name, f.info, f.owner, f.group, f.nlink, f.linkTarget, opts, now, cw) + printEntry(callCtx, f.name, f.info, f.owner, f.group, f.nlink, f.linkTarget, f.linkTargetInfo, opts, now, cw) } } @@ -303,12 +307,13 @@ type options struct { } type pathArg struct { - name string - info iofs.FileInfo - owner string // cached by fileOwner (populated when longFmt) - group string - nlink string - linkTarget string // symlink destination (populated when longFmt + symlink) + name string + info iofs.FileInfo + owner string // cached by fileOwner (populated when longFmt) + group string + nlink string + linkTarget string // symlink destination (populated when longFmt + symlink) + linkTargetInfo iofs.FileInfo // target's FileInfo for indicator (may be nil for dangling links) } func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opts *options, depth int, now time.Time) error { @@ -354,13 +359,14 @@ func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opt // Get FileInfo for sorting (if needed) and for long format. type entryInfo struct { - name string - info iofs.FileInfo - isSymlink bool - owner string // cached by fileOwner (populated when longFmt) - group string - nlink string - linkTarget string // symlink destination (populated when longFmt + symlink) + name string + info iofs.FileInfo + isSymlink bool + owner string // cached by fileOwner (populated when longFmt) + group string + nlink string + linkTarget string // symlink destination (populated when longFmt + symlink) + linkTargetInfo iofs.FileInfo // target's FileInfo for indicator (may be nil for dangling links) } failed := false @@ -386,9 +392,14 @@ func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opt isSymlink: e.Type()&iofs.ModeSymlink != 0, } if opts.longFmt && ei.isSymlink { - if t, err := callCtx.ReadlinkFile(ctx, joinPath(dir, name)); err == nil { + fullPath := joinPath(dir, name) + if t, err := callCtx.ReadlinkFile(ctx, fullPath); err == nil { ei.linkTarget = t } + // Stat the target for -F/-p indicators (nil for dangling links). + if ti, err := callCtx.StatFile(ctx, fullPath); err == nil { + ei.linkTargetInfo = ti + } } infoEntries = append(infoEntries, ei) } @@ -446,7 +457,7 @@ func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opt if ctx.Err() != nil { break } - printEntry(callCtx, ei.name, ei.info, ei.owner, ei.group, ei.nlink, ei.linkTarget, opts, now, cw) + printEntry(callCtx, ei.name, ei.info, ei.owner, ei.group, ei.nlink, ei.linkTarget, ei.linkTargetInfo, opts, now, cw) } // Only warn on implicit truncation (no explicit --offset/--limit). @@ -539,7 +550,7 @@ func computeColWidths[T any](entries []T, getInfo func(T) iofs.FileInfo, getOwne return w } -func printEntry(callCtx *builtins.CallContext, name string, info iofs.FileInfo, owner, group, nlink, linkTarget string, opts *options, now time.Time, cw colWidths) { +func printEntry(callCtx *builtins.CallContext, name string, info iofs.FileInfo, owner, group, nlink, linkTarget string, linkTargetInfo iofs.FileInfo, opts *options, now time.Time, cw colWidths) { if opts.longFmt { mode := formatMode(info) size := info.Size() @@ -554,7 +565,11 @@ func printEntry(callCtx *builtins.CallContext, name string, info iofs.FileInfo, suffix := indicator(info, opts) if linkTarget != "" { - suffix = " -> " + linkTarget + targetIndicator := "" + if linkTargetInfo != nil { + targetIndicator = indicator(linkTargetInfo, opts) + } + suffix = " -> " + linkTarget + targetIndicator } timeStr := formatTime(modTime, now) diff --git a/tests/scenarios/cmd/ls/long_format/symlink_classify.yaml b/tests/scenarios/cmd/ls/long_format/symlink_classify.yaml new file mode 100644 index 00000000..2c37e6f9 --- /dev/null +++ b/tests/scenarios/cmd/ls/long_format/symlink_classify.yaml @@ -0,0 +1,24 @@ +description: ls -lF shows classification indicators on symlink targets +# skip: rshell displays numeric UID/GID while GNU ls resolves usernames. +skip_assert_against_bash: true +setup: + files: + - path: dir/file.txt + content: "hello" + chmod: 0644 + - path: dir/subdir/.keep + content: "" + - path: dir/dirlink + symlink: subdir + - path: dir/filelink + symlink: file.txt +input: + allowed_paths: ["$DIR"] + script: |+ + ls -lF dir/ +expect: + stdout_contains: + - "dirlink -> subdir/" + - "filelink -> file.txt" + stderr: "" + exit_code: 0 From 20bcb577e73301ec254ed5015b7847f5da292678 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Wed, 1 Apr 2026 17:02:36 -0400 Subject: [PATCH 3/4] fix(ls): robust symlink detection and re-enable bash assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use info.Mode() as fallback for symlink detection when DirEntry.Type() returns 0 (DT_UNKNOWN on some filesystems). - Remove skip_assert_against_bash from symlink scenarios — they use stdout_contains so UID/GID differences don't affect the comparison. Verified all three pass against bash via docker. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/ls/ls.go | 8 +++++--- tests/scenarios/cmd/ls/long_format/symlink_classify.yaml | 2 -- tests/scenarios/cmd/ls/long_format/symlink_target.yaml | 2 -- .../cmd/ls/long_format/symlink_target_in_dir.yaml | 2 -- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/builtins/ls/ls.go b/builtins/ls/ls.go index c34d5642..21b0f7af 100644 --- a/builtins/ls/ls.go +++ b/builtins/ls/ls.go @@ -387,9 +387,11 @@ func listDir(ctx context.Context, callCtx *builtins.CallContext, dir string, opt continue } ei := entryInfo{ - name: name, - info: info, - isSymlink: e.Type()&iofs.ModeSymlink != 0, + name: name, + info: info, + // Check both Type() and info.Mode() — Type() can be 0 on + // filesystems that report DT_UNKNOWN. + isSymlink: e.Type()&iofs.ModeSymlink != 0 || info.Mode()&iofs.ModeSymlink != 0, } if opts.longFmt && ei.isSymlink { fullPath := joinPath(dir, name) diff --git a/tests/scenarios/cmd/ls/long_format/symlink_classify.yaml b/tests/scenarios/cmd/ls/long_format/symlink_classify.yaml index 2c37e6f9..3e09d049 100644 --- a/tests/scenarios/cmd/ls/long_format/symlink_classify.yaml +++ b/tests/scenarios/cmd/ls/long_format/symlink_classify.yaml @@ -1,6 +1,4 @@ description: ls -lF shows classification indicators on symlink targets -# skip: rshell displays numeric UID/GID while GNU ls resolves usernames. -skip_assert_against_bash: true setup: files: - path: dir/file.txt diff --git a/tests/scenarios/cmd/ls/long_format/symlink_target.yaml b/tests/scenarios/cmd/ls/long_format/symlink_target.yaml index d57dcf54..a0dec9f5 100644 --- a/tests/scenarios/cmd/ls/long_format/symlink_target.yaml +++ b/tests/scenarios/cmd/ls/long_format/symlink_target.yaml @@ -1,6 +1,4 @@ description: ls -l shows symlink target with -> arrow -# skip: rshell displays numeric UID/GID while GNU ls resolves usernames. -skip_assert_against_bash: true setup: files: - path: file.txt diff --git a/tests/scenarios/cmd/ls/long_format/symlink_target_in_dir.yaml b/tests/scenarios/cmd/ls/long_format/symlink_target_in_dir.yaml index e02c384d..65950a1e 100644 --- a/tests/scenarios/cmd/ls/long_format/symlink_target_in_dir.yaml +++ b/tests/scenarios/cmd/ls/long_format/symlink_target_in_dir.yaml @@ -1,6 +1,4 @@ description: ls -l in a directory shows symlink targets with -> arrow -# skip: rshell displays numeric UID/GID while GNU ls resolves usernames. -skip_assert_against_bash: true setup: files: - path: dir/file.txt From e5b0a338dcec3d635417694dc1e16c222ffc6eaa Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Thu, 2 Apr 2026 09:03:02 -0400 Subject: [PATCH 4/4] fix(ls): only apply -F indicators to symlink targets, not -p GNU ls applies classification indicators to symlink targets only with -F (classify), not -p (append-slash). For example, ls -lF shows "link -> dir/" but ls -lp shows "link -> dir" without the trailing slash. Guard the target indicator call with opts.classify. Co-Authored-By: Claude Opus 4.6 (1M context) --- builtins/ls/ls.go | 5 ++++- .../cmd/ls/long_format/symlink_append_slash.yaml | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 tests/scenarios/cmd/ls/long_format/symlink_append_slash.yaml diff --git a/builtins/ls/ls.go b/builtins/ls/ls.go index 21b0f7af..1b0bdf67 100644 --- a/builtins/ls/ls.go +++ b/builtins/ls/ls.go @@ -568,7 +568,10 @@ func printEntry(callCtx *builtins.CallContext, name string, info iofs.FileInfo, suffix := indicator(info, opts) if linkTarget != "" { targetIndicator := "" - if linkTargetInfo != nil { + // GNU ls only applies -F (classify) indicators to symlink + // targets, not -p (append-slash-only). For example, + // ls -lF shows "link -> dir/" but ls -lp shows "link -> dir". + if linkTargetInfo != nil && opts.classify { targetIndicator = indicator(linkTargetInfo, opts) } suffix = " -> " + linkTarget + targetIndicator diff --git a/tests/scenarios/cmd/ls/long_format/symlink_append_slash.yaml b/tests/scenarios/cmd/ls/long_format/symlink_append_slash.yaml new file mode 100644 index 00000000..19dc875d --- /dev/null +++ b/tests/scenarios/cmd/ls/long_format/symlink_append_slash.yaml @@ -0,0 +1,16 @@ +description: ls -lp does not append slash to symlink targets (only -F does) +setup: + files: + - path: dir/subdir/.keep + content: "" + - path: dir/dirlink + symlink: subdir +input: + allowed_paths: ["$DIR"] + script: |+ + ls -lp dir/ +expect: + stdout_contains: + - "dirlink -> subdir" + - "subdir/" + exit_code: 0