From 2d92ded6eb8f0e06dfe22db24b712af0318d7dcc Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Fri, 13 Mar 2026 13:58:16 +0100 Subject: [PATCH 1/8] security: cap glob directory expansion to prevent memory exhaustion ReadDir(-1) loaded all directory entries with no upper bound. Pathological glob patterns over large directories could exhaust memory and, combined with unbounded for-loop iteration, cause CPU exhaustion. Add MaxGlobEntries = 100,000 constant and reject readDir calls that would return more entries than the limit. Co-Authored-By: Claude Sonnet 4.6 --- interp/allowed_paths.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/interp/allowed_paths.go b/interp/allowed_paths.go index 132600a1..b3bf43dd 100644 --- a/interp/allowed_paths.go +++ b/interp/allowed_paths.go @@ -16,6 +16,11 @@ import ( "strings" ) +// MaxGlobEntries is the maximum number of directory entries returned per glob +// expansion step. Patterns that expand over directories with more entries are +// rejected with a "too many entries" error to prevent memory exhaustion. +const MaxGlobEntries = 100_000 + // allowedRoot pairs an absolute directory path with its opened os.Root handle. type allowedRoot struct { absPath string @@ -184,6 +189,9 @@ func (s *pathSandbox) readDir(ctx context.Context, path string) ([]fs.DirEntry, if err != nil { return nil, portablePathError(err) } + if len(entries) > MaxGlobEntries { + return nil, &os.PathError{Op: "readdir", Path: path, Err: fmt.Errorf("too many entries (limit %d)", MaxGlobEntries)} + } // os.Root's ReadDir does not guarantee sorted order like os.ReadDir. // Sort to match POSIX glob expansion expectations. slices.SortFunc(entries, func(a, b fs.DirEntry) int { From 8124b8067c773c7d0d724f75183fde0d0d44eeb4 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Fri, 13 Mar 2026 14:43:40 +0100 Subject: [PATCH 2/8] Address review: cap ReadDir allocation with MaxGlobEntries+1 Use f.ReadDir(MaxGlobEntries+1) instead of f.ReadDir(-1) so that the kernel/OS never needs to load more than MaxGlobEntries+1 entries into memory, failing fast before a full allocation is built. Co-Authored-By: Claude Sonnet 4.6 --- interp/allowed_paths.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interp/allowed_paths.go b/interp/allowed_paths.go index b3bf43dd..e1b4c36c 100644 --- a/interp/allowed_paths.go +++ b/interp/allowed_paths.go @@ -185,7 +185,7 @@ func (s *pathSandbox) readDir(ctx context.Context, path string) ([]fs.DirEntry, return nil, portablePathError(err) } defer f.Close() - entries, err := f.ReadDir(-1) + entries, err := f.ReadDir(MaxGlobEntries + 1) if err != nil { return nil, portablePathError(err) } From 02b13b0b0d546416f9b7e6f223b9b4e29b530517 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Fri, 13 Mar 2026 15:13:48 +0100 Subject: [PATCH 3/8] Fix glob no-match literal: ignore io.EOF from ReadDir(n>0) ReadDir(-1) returns nil error on success; ReadDir(n>0) returns io.EOF when the directory has fewer than n entries. The PR switched to ReadDir(MaxGlobEntries+1) but didn't account for this, causing readDir to propagate io.EOF as an error. The expand package then aborted glob expansion instead of falling back to the literal pattern, producing empty output for unmatched globs instead of the original pattern. Co-Authored-By: Claude Sonnet 4.6 --- interp/allowed_paths.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interp/allowed_paths.go b/interp/allowed_paths.go index e1b4c36c..801ac20b 100644 --- a/interp/allowed_paths.go +++ b/interp/allowed_paths.go @@ -186,7 +186,7 @@ func (s *pathSandbox) readDir(ctx context.Context, path string) ([]fs.DirEntry, } defer f.Close() entries, err := f.ReadDir(MaxGlobEntries + 1) - if err != nil { + if err != nil && err != io.EOF { return nil, portablePathError(err) } if len(entries) > MaxGlobEntries { From 111352566b6344cb5105c8ef2c3a8228a0908d64 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Fri, 13 Mar 2026 15:32:04 +0100 Subject: [PATCH 4/8] Address review: move glob entry cap from readDir to ReadDir2 closure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MaxGlobEntries cap was enforced in the shared readDir helper, which is also used by the ls builtin. This caused ls to fail with "too many entries" for directories with >100,000 entries — an unintended regression. Move the cap check to the ReadDir2 closure in updateExpandOpts, which is the glob-expansion-specific call site. Revert readDir to use ReadDir(-1) so ls (and any other non-glob reader) is unaffected. Co-Authored-By: Claude Sonnet 4.6 --- interp/allowed_paths.go | 7 ++----- interp/runner_expand.go | 9 ++++++++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/interp/allowed_paths.go b/interp/allowed_paths.go index 801ac20b..040c89d4 100644 --- a/interp/allowed_paths.go +++ b/interp/allowed_paths.go @@ -185,13 +185,10 @@ func (s *pathSandbox) readDir(ctx context.Context, path string) ([]fs.DirEntry, return nil, portablePathError(err) } defer f.Close() - entries, err := f.ReadDir(MaxGlobEntries + 1) - if err != nil && err != io.EOF { + entries, err := f.ReadDir(-1) + if err != nil { return nil, portablePathError(err) } - if len(entries) > MaxGlobEntries { - return nil, &os.PathError{Op: "readdir", Path: path, Err: fmt.Errorf("too many entries (limit %d)", MaxGlobEntries)} - } // os.Root's ReadDir does not guarantee sorted order like os.ReadDir. // Sort to match POSIX glob expansion expectations. slices.SortFunc(entries, func(a, b fs.DirEntry) int { diff --git a/interp/runner_expand.go b/interp/runner_expand.go index f05afe34..b25a25af 100644 --- a/interp/runner_expand.go +++ b/interp/runner_expand.go @@ -29,7 +29,14 @@ func (r *Runner) fillExpandConfig(ctx context.Context) { func (r *Runner) updateExpandOpts() { r.ecfg.ReadDir2 = func(s string) ([]fs.DirEntry, error) { - return r.readDirHandler(r.handlerCtx(r.ectx, todoPos), s) + entries, err := r.readDirHandler(r.handlerCtx(r.ectx, todoPos), s) + if err != nil { + return nil, err + } + if len(entries) > MaxGlobEntries { + return nil, fmt.Errorf("readdir %s: too many entries (limit %d)", s, MaxGlobEntries) + } + return entries, nil } } From ea73379d57d0f8d220c272b6091d2755c5395a20 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Fri, 13 Mar 2026 15:40:01 +0100 Subject: [PATCH 5/8] Address review: cap ReadDir at OS level via readDirForGlob The previous fix checked len(entries) after readDir already called ReadDir(-1), materialising all entries in memory before the cap could take effect. Extract readDirN(ctx, path, maxEntries) as the shared implementation. readDir calls it with -1 (unlimited) so ls and other builtins are unaffected. readDirForGlob calls it with MaxGlobEntries, which passes MaxGlobEntries+1 to the underlying ReadDir syscall so the kernel never fetches more entries than necessary; io.EOF is ignored (ReadDir(n>0) returns it on success) and a "too many entries" error is returned if the directory exceeds the limit. The ReadDir2 closure in updateExpandOpts now calls readDirForGlob directly, ensuring the cap is enforced before any entries are sorted or matched. Co-Authored-By: Claude Sonnet 4.6 --- interp/allowed_paths.go | 29 +++++++++++++++++++++++++++-- interp/runner_expand.go | 9 +-------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/interp/allowed_paths.go b/interp/allowed_paths.go index 040c89d4..21c8b902 100644 --- a/interp/allowed_paths.go +++ b/interp/allowed_paths.go @@ -173,6 +173,22 @@ func (s *pathSandbox) open(ctx context.Context, path string, flag int, perm os.F // readDir implements the restricted directory-read policy. func (s *pathSandbox) readDir(ctx context.Context, path string) ([]fs.DirEntry, error) { + return s.readDirN(ctx, path, -1) +} + +// readDirForGlob reads at most MaxGlobEntries directory entries for glob +// expansion. It caps the underlying ReadDir call at MaxGlobEntries+1 so the +// kernel never materialises more entries than needed; if the directory exceeds +// the limit a "too many entries" error is returned before any expensive pattern +// matching or sorting can occur. +func (s *pathSandbox) readDirForGlob(ctx context.Context, path string) ([]fs.DirEntry, error) { + return s.readDirN(ctx, path, MaxGlobEntries) +} + +// readDirN is the common implementation for readDir and readDirForGlob. +// maxEntries <= 0 means unlimited; otherwise at most maxEntries entries are +// returned and a "too many entries" error is returned if the directory has more. +func (s *pathSandbox) readDirN(ctx context.Context, path string, maxEntries int) ([]fs.DirEntry, error) { absPath := toAbs(path, HandlerCtx(ctx).Dir) root, relPath, ok := s.resolve(absPath) @@ -185,10 +201,19 @@ func (s *pathSandbox) readDir(ctx context.Context, path string) ([]fs.DirEntry, return nil, portablePathError(err) } defer f.Close() - entries, err := f.ReadDir(-1) - if err != nil { + + var entries []fs.DirEntry + if maxEntries <= 0 { + entries, err = f.ReadDir(-1) + } else { + entries, err = f.ReadDir(maxEntries + 1) + } + if err != nil && err != io.EOF { return nil, portablePathError(err) } + if maxEntries > 0 && len(entries) > maxEntries { + return nil, &os.PathError{Op: "readdir", Path: path, Err: fmt.Errorf("too many entries (limit %d)", maxEntries)} + } // os.Root's ReadDir does not guarantee sorted order like os.ReadDir. // Sort to match POSIX glob expansion expectations. slices.SortFunc(entries, func(a, b fs.DirEntry) int { diff --git a/interp/runner_expand.go b/interp/runner_expand.go index b25a25af..ec047550 100644 --- a/interp/runner_expand.go +++ b/interp/runner_expand.go @@ -29,14 +29,7 @@ func (r *Runner) fillExpandConfig(ctx context.Context) { func (r *Runner) updateExpandOpts() { r.ecfg.ReadDir2 = func(s string) ([]fs.DirEntry, error) { - entries, err := r.readDirHandler(r.handlerCtx(r.ectx, todoPos), s) - if err != nil { - return nil, err - } - if len(entries) > MaxGlobEntries { - return nil, fmt.Errorf("readdir %s: too many entries (limit %d)", s, MaxGlobEntries) - } - return entries, nil + return r.sandbox.readDirForGlob(r.handlerCtx(r.ectx, todoPos), s) } } From 22ff137900e9b9918f4dced4fac838920798650e Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Sun, 15 Mar 2026 15:27:13 +0100 Subject: [PATCH 6/8] Address review: truncate glob cap instead of returning error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Returning a "too many entries" error from ReadDirForGlob caused a silent correctness bug: mvdan.cc/sh/expand calls ReadDir2 for two purposes — to check whether a literal path component is a directory (wantDir=true) and to list entries for wildcard matching. When the existence check returns a non-ErrNotExist error with wantDir=true, expand silently drops the path, so echo /huge/*.txt returns the literal pattern instead of expanding the files inside /huge. Fix: when the cap is exceeded, silently truncate to MaxGlobEntries instead of returning an error. The underlying f.ReadDir(MaxGlobEntries+1) still bounds the OS-level read so at most MaxGlobEntries+1 entries are ever loaded into memory. Existence checks pass (no error), glob expansion works against the first N entries, and memory is bounded. Co-Authored-By: Claude Sonnet 4.6 --- allowedpaths/sandbox.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index a949b1a3..98fe5ade 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -200,8 +200,16 @@ func (s *Sandbox) ReadDirForGlob(path string, cwd string) ([]fs.DirEntry, error) } // readDirN is the shared implementation for ReadDir and ReadDirForGlob. -// maxEntries <= 0 means unlimited; otherwise at most maxEntries entries are -// returned and a "too many entries" error is returned if the directory has more. +// maxEntries <= 0 means unlimited. Otherwise f.ReadDir is called with +// maxEntries+1 to cap the read at the OS level; if the directory has more +// entries than the limit, the slice is silently truncated to maxEntries. +// Truncating without error (rather than returning an error) is intentional: +// ReadDir2 is called by mvdan.cc/sh/expand both to check whether a literal +// path component is a directory (error → path silently dropped) and to list +// entries for wildcard matching. Returning an error for the existence check +// causes huge parent directories to be treated as non-existent, so patterns +// like "echo /huge/*.txt" return the literal instead of expanding. Truncating +// bounds memory while letting both call sites succeed. func (s *Sandbox) readDirN(path string, cwd string, maxEntries int) ([]fs.DirEntry, error) { absPath := toAbs(path, cwd) @@ -225,8 +233,9 @@ func (s *Sandbox) readDirN(path string, cwd string, maxEntries int) ([]fs.DirEnt if err != nil && err != io.EOF { return nil, PortablePathError(err) } + // Silently truncate to maxEntries: see the function comment above. if maxEntries > 0 && len(entries) > maxEntries { - return nil, &os.PathError{Op: "readdir", Path: path, Err: fmt.Errorf("too many entries (limit %d)", maxEntries)} + entries = entries[:maxEntries] } // os.Root's ReadDir does not guarantee sorted order like os.ReadDir. // Sort to match POSIX glob expansion expectations. From 17b4240539b5ea2f1acd4b9d6646d1b3a5cbb502 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Sun, 15 Mar 2026 18:53:00 +0100 Subject: [PATCH 7/8] Address review: return error when glob entry cap exceeded Replace silent truncation with an explicit error when a directory exceeds MaxGlobEntries entries during glob expansion. Silent truncation could miss valid glob matches with no indication of failure; the explicit error surfaces through expandErr (printed to stderr, exit code 1) so callers are aware the directory was too large to enumerate. Add TestReadDirNCapExceeded to cover the error path and the boundary cases (count == cap, count < cap). Co-Authored-By: Claude Sonnet 4.6 --- allowedpaths/sandbox.go | 33 +++++++++++++++------------------ allowedpaths/sandbox_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/allowedpaths/sandbox.go b/allowedpaths/sandbox.go index 98fe5ade..03e869c1 100644 --- a/allowedpaths/sandbox.go +++ b/allowedpaths/sandbox.go @@ -18,9 +18,9 @@ import ( "strings" ) -// MaxGlobEntries is the maximum number of directory entries read per glob -// expansion step. ReadDirForGlob rejects directories with more entries to -// prevent memory exhaustion during pattern matching. +// MaxGlobEntries is the maximum number of directory entries read per single +// glob expansion step. ReadDirForGlob returns an error for directories that +// exceed this limit to prevent memory exhaustion during pattern matching. const MaxGlobEntries = 100_000 // root pairs an absolute directory path with its opened os.Root handle. @@ -190,11 +190,12 @@ func (s *Sandbox) ReadDir(path string, cwd string) ([]fs.DirEntry, error) { return s.readDirN(path, cwd, -1) } -// ReadDirForGlob reads at most MaxGlobEntries directory entries for glob -// expansion. It caps the underlying ReadDir call at MaxGlobEntries+1 so the -// kernel never materialises more entries than needed; if the directory exceeds -// the limit a "too many entries" error is returned before any expensive pattern -// matching or sorting can occur. +// ReadDirForGlob reads directory entries for glob expansion, capped at +// MaxGlobEntries. The underlying ReadDir call is limited to MaxGlobEntries+1 +// so the kernel never materialises more entries than needed. If the directory +// exceeds the limit an error is returned before any pattern matching or +// sorting can occur, making the failure explicit rather than silently returning +// a partial listing that could miss valid matches. func (s *Sandbox) ReadDirForGlob(path string, cwd string) ([]fs.DirEntry, error) { return s.readDirN(path, cwd, MaxGlobEntries) } @@ -202,14 +203,7 @@ func (s *Sandbox) ReadDirForGlob(path string, cwd string) ([]fs.DirEntry, error) // readDirN is the shared implementation for ReadDir and ReadDirForGlob. // maxEntries <= 0 means unlimited. Otherwise f.ReadDir is called with // maxEntries+1 to cap the read at the OS level; if the directory has more -// entries than the limit, the slice is silently truncated to maxEntries. -// Truncating without error (rather than returning an error) is intentional: -// ReadDir2 is called by mvdan.cc/sh/expand both to check whether a literal -// path component is a directory (error → path silently dropped) and to list -// entries for wildcard matching. Returning an error for the existence check -// causes huge parent directories to be treated as non-existent, so patterns -// like "echo /huge/*.txt" return the literal instead of expanding. Truncating -// bounds memory while letting both call sites succeed. +// entries than the limit an error is returned. func (s *Sandbox) readDirN(path string, cwd string, maxEntries int) ([]fs.DirEntry, error) { absPath := toAbs(path, cwd) @@ -233,9 +227,12 @@ func (s *Sandbox) readDirN(path string, cwd string, maxEntries int) ([]fs.DirEnt if err != nil && err != io.EOF { return nil, PortablePathError(err) } - // Silently truncate to maxEntries: see the function comment above. if maxEntries > 0 && len(entries) > maxEntries { - entries = entries[:maxEntries] + return nil, &os.PathError{ + Op: "readdir", + Path: path, + Err: fmt.Errorf("directory has too many entries (cap: %d)", maxEntries), + } } // os.Root's ReadDir does not guarantee sorted order like os.ReadDir. // Sort to match POSIX glob expansion expectations. diff --git a/allowedpaths/sandbox_test.go b/allowedpaths/sandbox_test.go index f9c55bd3..e36ccc14 100644 --- a/allowedpaths/sandbox_test.go +++ b/allowedpaths/sandbox_test.go @@ -185,6 +185,38 @@ func TestReadDirLimited(t *testing.T) { }) } +func TestReadDirNCapExceeded(t *testing.T) { + dir := t.TempDir() + + // Create 4 files so the directory exceeds a cap of 3. + for i := range 4 { + require.NoError(t, os.WriteFile(filepath.Join(dir, fmt.Sprintf("f%02d", i)), nil, 0644)) + } + + sb, err := New([]string{dir}) + require.NoError(t, err) + defer sb.Close() + + t.Run("returns error when entries exceed cap", func(t *testing.T) { + entries, err := sb.readDirN(".", dir, 3) + assert.Nil(t, entries) + assert.Error(t, err) + assert.Contains(t, err.Error(), "too many entries") + }) + + t.Run("returns entries when count equals cap", func(t *testing.T) { + entries, err := sb.readDirN(".", dir, 4) + require.NoError(t, err) + assert.Len(t, entries, 4) + }) + + t.Run("returns entries when count is below cap", func(t *testing.T) { + entries, err := sb.readDirN(".", dir, 10) + require.NoError(t, err) + assert.Len(t, entries, 4) + }) +} + func TestCollectDirEntries(t *testing.T) { makeEntries := func(names ...string) []fs.DirEntry { out := make([]fs.DirEntry, len(names)) From e6f882322597dc55a2d2ed21775412d5a5eeac66 Mon Sep 17 00:00:00 2001 From: Jules Macret Date: Sun, 15 Mar 2026 20:38:44 +0100 Subject: [PATCH 8/8] Address review: route glob reads through readDirHandler; fix fuzz context Route ReadDir2 (used for glob expansion) through r.readDirHandler instead of calling r.sandbox.ReadDirForGlob directly. This respects any custom directory-read handler configured via runnerConfig, which was previously bypassed. Update Reset() to wire readDirHandler to ReadDirForGlob (the capped variant) so the default behaviour is unchanged. Fix spurious Fuzz (testcmd) CI failures by deriving per-iteration contexts from t.Context() rather than context.Background(). When the fuzz run's time limit expires, the framework cancels t.Context(); iterations derived from context.Background() could not observe this signal and caused a timing race that surfaced as "context deadline exceeded" at the 30s boundary. Co-Authored-By: Claude Sonnet 4.6 --- builtins/tests/testcmd/testcmd_fuzz_test.go | 10 +++++----- interp/api.go | 2 +- interp/runner_expand.go | 4 ++++ 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/builtins/tests/testcmd/testcmd_fuzz_test.go b/builtins/tests/testcmd/testcmd_fuzz_test.go index d7b4b873..9b902a62 100644 --- a/builtins/tests/testcmd/testcmd_fuzz_test.go +++ b/builtins/tests/testcmd/testcmd_fuzz_test.go @@ -79,7 +79,7 @@ func FuzzTestStringOps(f *testing.F) { dir := t.TempDir() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() script := fmt.Sprintf("test '%s' %s '%s'", left, op, right) @@ -125,7 +125,7 @@ func FuzzTestIntegerOps(f *testing.F) { dir := t.TempDir() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() script := fmt.Sprintf("test %d %s %d", left, op, right) @@ -167,7 +167,7 @@ func FuzzTestFileOps(f *testing.F) { } } - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() script := fmt.Sprintf("test %s %s", op, target) @@ -219,7 +219,7 @@ func FuzzTestStringUnary(f *testing.F) { dir := t.TempDir() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() script := fmt.Sprintf("test %s '%s'", op, arg) @@ -288,7 +288,7 @@ func FuzzTestNesting(f *testing.F) { dir := t.TempDir() - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() script := fmt.Sprintf("test %s", expr) diff --git a/interp/api.go b/interp/api.go index d58f554d..b257a219 100644 --- a/interp/api.go +++ b/interp/api.go @@ -295,7 +295,7 @@ func (r *Runner) Reset() { return r.sandbox.Open(path, HandlerCtx(ctx).Dir, flag, perm) } r.readDirHandler = func(ctx context.Context, path string) ([]os.DirEntry, error) { - return r.sandbox.ReadDir(path, HandlerCtx(ctx).Dir) + return r.sandbox.ReadDirForGlob(path, HandlerCtx(ctx).Dir) } r.execHandler = noExecHandler() } diff --git a/interp/runner_expand.go b/interp/runner_expand.go index afaf1518..d0befce2 100644 --- a/interp/runner_expand.go +++ b/interp/runner_expand.go @@ -30,6 +30,10 @@ func (r *Runner) fillExpandConfig(ctx context.Context) { func (r *Runner) updateExpandOpts() { r.ecfg.ReadDir2 = func(s string) ([]fs.DirEntry, error) { ctx := r.handlerCtx(r.ectx, todoPos) + if r.readDirHandler != nil { + return r.readDirHandler(ctx, s) + } + // Fallback when a custom openHandler was set without a readDirHandler. return r.sandbox.ReadDirForGlob(s, HandlerCtx(ctx).Dir) } }