fix: make cd, ls, rg and /dev/null behave like the real thing

bash.go

@@ -16,10 +16,10 @@ import (
 	"github.com/mark3labs/go-bash/command"
 	gbfs "github.com/mark3labs/go-bash/fs"
 	"github.com/mark3labs/go-bash/fs/memfs"
-	bashinterp "github.com/mark3labs/go-bash/interp"
 	gbalias "github.com/mark3labs/go-bash/internal/alias"
 	"github.com/mark3labs/go-bash/internal/ringbuf"
 	"github.com/mark3labs/go-bash/internal/runtimestate"
+	bashinterp "github.com/mark3labs/go-bash/interp"
 	"github.com/mark3labs/go-bash/network"
 	"github.com/mark3labs/go-bash/parser"
 	"github.com/mark3labs/go-bash/transform"
@@ -154,11 +154,12 @@ func New(opts BashOptions) (*Bash, error) {
 	} else {
 		b.fs = memfs.New()
 	}
-	if len(opts.Files) > 0 {
-		if err := seedFiles(b.fs, opts.Files); err != nil {
-			return nil, err
-		}
-	}
+	// /dev/null must discard writes and read as empty for every
+	// consumer — shell redirections and built-ins alike. This is
+	// independent of the default layout below: the wrapper matches on
+	// path, so /dev/null works even when the caller manages its own
+	// layout and no /dev directory exists.
+	b.fs = gbfs.WithNullDevice(b.fs)
 	// Command registry. CustomCommands register first so name
 	// collisions with later (Phase 10) built-in registrations resolve
 	// in favor of the custom entry — the built-in bootstrap will skip
@@ -214,6 +215,11 @@ func New(opts BashOptions) (*Bash, error) {
 	// non-fatal: a read-only or restricted FileSystem may reject some
 	// of the writes, but New should still succeed so the caller can
 	// inspect / repair the FS post-construction.
+	if len(opts.Files) > 0 {
+		if err := seedFiles(b.fs, opts.Files); err != nil {
+			return nil, err
+		}
+	}
 	useDefaultLayout := opts.Cwd == "" && len(opts.Files) == 0
 	if useDefaultLayout {
 		_ = applyDefaultLayout(b.fs, b.procInfo, b.registry)
@@ -249,7 +255,7 @@ func New(opts BashOptions) (*Bash, error) {
 
 // FS returns the virtual filesystem this Bash is bound to. Useful for
 // host-side inspection or post-Exec assertions in tests.
-func (b *Bash) FS() gbfs.FileSystem { return b.fs }
+func (b *Bash) FS() gbfs.FileSystem { return gbfs.UnwrapNullDevice(b.fs) }
 
 // Registry returns the command dispatch registry. The returned
 // pointer is the live registry consulted by every Exec call; mutating
@@ -521,6 +527,26 @@ func (b *Bash) execLocked(ctx context.Context, script string, opts ExecOptions)
 				})
 			}
 		}
+		// mvdan/sh's own `cd` and `pwd` are unusable inside the sandbox:
+		// cd gates on unix.Access() against the HOST filesystem (see
+		// builtins/cd), and pwd reads the $PWD variable that only that
+		// broken cd maintains. Both are shadowed by gobash built-ins
+		// that work off the VFS, but mvdan/sh dispatches its builtins
+		// before the exec handler ever sees the name — so redirect them
+		// here, by path, which routes through lookupCommand's /bin/
+		// basename fallback into the registry.
+		//
+		// A user-defined shell function of the same name still wins:
+		// r.Funcs is consulted after the CallHandler, and rewriting the
+		// word would hide the function.
+		if runnerRef != nil {
+			if _, isFunc := runnerRef.Funcs[args[0]]; !isFunc {
+				switch args[0] {
+				case "cd", "pwd":
+					args = append([]string{"/bin/" + args[0]}, args[1:]...)
+				}
+			}
+		}
 		cmd := cmdCount.Add(1)
 		if cmd > int64(limits.MaxCommandCount) {
 			return nil, trip(&ExecutionLimitError{

builtins/cd/cd.go

@@ -1,11 +1,12 @@
-// Package cd implements the `cd` shell built-in. Real bash
-// semantics: change the working directory; mvdan/sh ships its own `cd`
-// that mutates the runner's Dir. Our registration is reachable via
-// /bin/cd and CANNOT mutate the runner's Dir (it has no back-channel),
-// so it functions as a path-validation + diagnostic stub: it resolves
-// the target via c.FS.Stat and emits a clean error if missing.
+// Package cd implements the `cd` shell built-in.
 //
-// Documented shadow in DECISIONS.md (Phase 11).
+// mvdan/sh ships its own `cd`, but it is unusable here: after the stat
+// succeeds it calls unix.Access(path, X_OK) against the HOST
+// filesystem, which no virtual path satisfies, so every cd inside the
+// sandbox fails with "permission denied". gobash therefore intercepts
+// `cd` in the CallHandler (see bash.go) and routes it here, where the
+// target is resolved against the VFS and applied through
+// Context.SetCwd.
 package cd
 
 import (
@@ -75,8 +76,14 @@ done:
 			return builtinutil.Errorf(c.Stderr, "cd", 1, "%s: not a directory", args[len(args)-1])
 		}
 	}
-	// Best-effort mutation: c.Env update is in-place but not
-	// propagated to mvdan/sh's runner.
+	// Apply the move. Without the back-channel there is no interpreter
+	// to move, so report that rather than silently succeeding.
+	if c.SetCwd == nil {
+		return builtinutil.Errorf(c.Stderr, "cd", 1, "cannot change directory in this context")
+	}
+	if err := c.SetCwd(dir); err != nil {
+		return builtinutil.Errorf(c.Stderr, "cd", 1, "%v", err)
+	}
 	if c.Env != nil {
 		c.Env["OLDPWD"] = c.Cwd
 		c.Env["PWD"] = dir

builtins/cd/cd_test.go

@@ -10,13 +10,44 @@ import (
 	"github.com/mark3labs/go-bash/fs/memfs"
 )
 
+// captureCwd stands in for the interpreter back-channel the dispatcher
+// normally supplies, and records the directory cd asked to move to.
+func captureCwd(got *string) func(string) error {
+	return func(dir string) error {
+		*got = dir
+		return nil
+	}
+}
+
 func TestCdValidDir(t *testing.T) {
 	fs := memfs.New()
 	_ = fs.MkdirAll("/tmp/x", 0o755)
-	r := New().Execute(context.Background(), []string{"cd", "/tmp/x"}, &command.Context{FS: fs, Env: map[string]string{}})
+	var moved string
+	r := New().Execute(context.Background(), []string{"cd", "/tmp/x"},
+		&command.Context{FS: fs, Env: map[string]string{}, SetCwd: captureCwd(&moved)})
 	if r.ExitCode != 0 {
 		t.Errorf("exit=%d", r.ExitCode)
 	}
+	if moved != "/tmp/x" {
+		t.Errorf("cwd moved to %q, want /tmp/x", moved)
+	}
+}
+
+// TestCdWithoutBackChannel pins the fail-closed path: with no SetCwd
+// there is no interpreter to move, and cd must say so rather than
+// report a success it did not perform.
+func TestCdWithoutBackChannel(t *testing.T) {
+	fs := memfs.New()
+	_ = fs.MkdirAll("/tmp/x", 0o755)
+	var e bytes.Buffer
+	r := New().Execute(context.Background(), []string{"cd", "/tmp/x"},
+		&command.Context{FS: fs, Env: map[string]string{}, Stderr: &e})
+	if r.ExitCode == 0 {
+		t.Errorf("exit=%d, want non-zero", r.ExitCode)
+	}
+	if !strings.Contains(e.String(), "cd:") {
+		t.Errorf("stderr=%q", e.String())
+	}
 }
 
 func TestCdMissing(t *testing.T) {
@@ -31,10 +62,15 @@ func TestCdMissing(t *testing.T) {
 func TestCdHome(t *testing.T) {
 	fs := memfs.New()
 	_ = fs.MkdirAll("/home/user", 0o755)
-	r := New().Execute(context.Background(), []string{"cd"}, &command.Context{FS: fs, Env: map[string]string{"HOME": "/home/user"}})
+	var moved string
+	r := New().Execute(context.Background(), []string{"cd"},
+		&command.Context{FS: fs, Env: map[string]string{"HOME": "/home/user"}, SetCwd: captureCwd(&moved)})
 	if r.ExitCode != 0 {
 		t.Errorf("exit=%d", r.ExitCode)
 	}
+	if moved != "/home/user" {
+		t.Errorf("cwd moved to %q, want /home/user", moved)
+	}
 }
 
 func TestCdHelp(t *testing.T) {

builtins/ls/ls.go

@@ -16,7 +16,7 @@ import (
 	"github.com/mark3labs/go-bash/internal/builtinutil"
 )
 
-const usage = "ls [-laAdFhiR1tSrLnp] [--color[=WHEN]] [PATH...]"
+const usage = "ls [-laAdFhiR1CxmtSrLnp] [--color[=WHEN]] [PATH...]"
 const helpText = `Usage: ls [OPTION]... [FILE]...
 List information about the FILEs (the current directory by default).
 
@@ -28,7 +28,9 @@ List information about the FILEs (the current directory by default).
   -h            with -l, print human-readable sizes
   -i            print the index number of each file
   -R            list subdirectories recursively
-  -1            list one file per line
+  -1            list one file per line (the default: stdout is not a TTY)
+  -C, -x        list entries in columns on a single line
+  -m            list entries separated by ", "
   -t            sort by modification time, newest first
   -S            sort by file size, largest first
   -r            reverse order while sorting
@@ -41,6 +43,10 @@ type opts struct {
 	long, all, almostAll, dirOnly, classify, human, inode bool
 	recursive, oneLine, byTime, bySize, reverse           bool
 	deref, numeric, slashDir                              bool
+	// columns forces the multi-column layout (-C/-x) and commas the
+	// comma-separated one (-m). Neither is the default: with no TTY,
+	// real ls falls back to one-entry-per-line, and so do we.
+	columns, commas bool
 }
 
 // New returns the ls command.
@@ -74,6 +80,10 @@ func run(_ context.Context, args []string, c *command.Context) command.Result {
 			o.recursive = true
 		case a == "-1":
 			o.oneLine = true
+		case a == "-C", a == "-x":
+			o.columns = true
+		case a == "-m":
+			o.commas = true
 		case a == "-t":
 			o.byTime = true
 		case a == "-S":
@@ -146,6 +156,10 @@ func bundleLsOpts(a string, o *opts) bool {
 			o.recursive = true
 		case '1':
 			o.oneLine = true
+		case 'C', 'x':
+			o.columns = true
+		case 'm':
+			o.commas = true
 		case 't':
 			o.byTime = true
 		case 'S':
@@ -263,22 +277,39 @@ func writeEntries(c *command.Context, items []entry, o *opts) {
 		}
 		return
 	}
-	if o.oneLine {
-		for _, e := range items {
-			writeShortLine(c.Stdout, e, o)
+	if o.commas {
+		for i, e := range items {
+			if i > 0 {
+				_, _ = io.WriteString(c.Stdout, ", ")
+			}
+			_, _ = io.WriteString(c.Stdout, decorate(e, o))
+		}
+		if len(items) > 0 {
+			_, _ = io.WriteString(c.Stdout, "\n")
 		}
 		return
 	}
-	// Default: space-separated on one line, then newline. Real ls
-	// uses column layout; we keep it simple to match a CLI sandbox.
-	for i, e := range items {
-		if i > 0 {
-			_, _ = io.WriteString(c.Stdout, "  ")
+	if o.columns {
+		// Explicit -C/-x: space-separated on one line. Real ls computes a
+		// column grid from the terminal width; there is no terminal here,
+		// so a single row is the honest approximation.
+		for i, e := range items {
+			if i > 0 {
+				_, _ = io.WriteString(c.Stdout, "  ")
+			}
+			_, _ = io.WriteString(c.Stdout, decorate(e, o))
+		}
+		if len(items) > 0 {
+			_, _ = io.WriteString(c.Stdout, "\n")
 		}
-		_, _ = io.WriteString(c.Stdout, decorate(e, o))
+		return
 	}
-	if len(items) > 0 {
-		_, _ = io.WriteString(c.Stdout, "\n")
+	// Default. Stdout is never a terminal in this sandbox, and real ls
+	// switches to one-entry-per-line whenever it is not writing to a
+	// TTY -- which is what keeps `ls | wc -l` and `for f in $(ls)`
+	// honest. Multi-column output is opt-in via -C/-x.
+	for _, e := range items {
+		writeShortLine(c.Stdout, e, o)
 	}
 }