From cc244519e0473730603f5d98d6dee5c006fc386c Mon Sep 17 00:00:00 2001
From: Travis Thieman <travis.thieman@gmail.com>
Date: Mon, 9 Mar 2026 14:19:54 -0400
Subject: [PATCH 1/2] Enforce builtin command structure and import allowlist

- Introduced self-registration via init() and a Register() function,
  replacing the manually maintained central registry in builtins.go;
  each command implementation now owns its own registration

- Moved all command implementations (cat, echo, exit, true/false,
  break/continue) from interp/builtins/ into a new interp/builtins/cmds/
  package, keeping the framework (CallContext, Result, Register, Lookup)
  isolated in interp/builtins/

- Changed CallContext.Stdin from *os.File to io.Reader to eliminate the
  dangerous *os.File method surface from command implementations; guarded
  the assignment in runner_exec.go to avoid typed-nil interface issues

- Added tests/import_allowlist_test.go with a symbol-level import
  allowlist for interp/builtins/cmds/; every package and pkg.Symbol
  used by a command implementation must be explicitly listed, including
  internal packages; reflect and unsafe are permanently banned

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 interp/builtins/builtins.go                  |  19 +--
 interp/builtins/{ => cmds}/break_continue.go |  25 ++--
 interp/builtins/{ => cmds}/cat.go            |  16 ++-
 interp/builtins/{ => cmds}/echo.go           |  16 ++-
 interp/builtins/{ => cmds}/exit.go           |  12 +-
 interp/builtins/cmds/true_false.go           |  25 ++++
 interp/builtins/true_false.go                |  16 ---
 interp/runner_exec.go                        |   5 +-
 tests/import_allowlist_test.go               | 141 +++++++++++++++++++
 9 files changed, 228 insertions(+), 47 deletions(-)
 rename interp/builtins/{ => cmds}/break_continue.go (57%)
 rename interp/builtins/{ => cmds}/cat.go (69%)
 rename interp/builtins/{ => cmds}/echo.go (57%)
 rename interp/builtins/{ => cmds}/exit.go (78%)
 create mode 100644 interp/builtins/cmds/true_false.go
 delete mode 100644 interp/builtins/true_false.go
 create mode 100644 tests/import_allowlist_test.go

diff --git a/interp/builtins/builtins.go b/interp/builtins/builtins.go
index cafd6d00..05c92e1f 100644
--- a/interp/builtins/builtins.go
+++ b/interp/builtins/builtins.go
@@ -20,7 +20,7 @@ type HandlerFunc func(ctx context.Context, callCtx *CallContext, args []string)
 type CallContext struct {
 	Stdout io.Writer
 	Stderr io.Writer
-	Stdin  *os.File
+	Stdin  io.Reader
 
 	// InLoop is true when the builtin runs inside a for loop.
 	InLoop bool
@@ -65,14 +65,15 @@ type Result struct {
 	ContinueN int
 }
 
-var registry = map[string]HandlerFunc{
-	"true":     builtinTrue,
-	"false":    builtinFalse,
-	"echo":     builtinEcho,
-	"cat":      builtinCat,
-	"exit":     builtinExit,
-	"break":    builtinBreak,
-	"continue": builtinContinue,
+var registry = map[string]HandlerFunc{}
+
+// Register adds a builtin command to the registry.
+// It panics if name is already registered, catching duplicate registrations at startup.
+func Register(name string, fn HandlerFunc) {
+	if _, exists := registry[name]; exists {
+		panic("builtin already registered: " + name)
+	}
+	registry[name] = fn
 }
 
 // Lookup returns the handler for a builtin command.
diff --git a/interp/builtins/break_continue.go b/interp/builtins/cmds/break_continue.go
similarity index 57%
rename from interp/builtins/break_continue.go
rename to interp/builtins/cmds/break_continue.go
index a987e2fa..fb4502fc 100644
--- a/interp/builtins/break_continue.go
+++ b/interp/builtins/cmds/break_continue.go
@@ -3,25 +3,32 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/).
 // Copyright 2026-present Datadog, Inc.
 
-package builtins
+package cmds
 
 import (
 	"context"
 	"strconv"
+
+	"github.com/DataDog/rshell/interp/builtins"
 )
 
-func builtinBreak(_ context.Context, callCtx *CallContext, args []string) Result {
+func init() {
+	builtins.Register("break", builtinBreak)
+	builtins.Register("continue", builtinContinue)
+}
+
+func builtinBreak(_ context.Context, callCtx *builtins.CallContext, args []string) builtins.Result {
 	return loopControl(callCtx, "break", args)
 }
 
-func builtinContinue(_ context.Context, callCtx *CallContext, args []string) Result {
+func builtinContinue(_ context.Context, callCtx *builtins.CallContext, args []string) builtins.Result {
 	return loopControl(callCtx, "continue", args)
 }
 
-func loopControl(callCtx *CallContext, name string, args []string) Result {
+func loopControl(callCtx *builtins.CallContext, name string, args []string) builtins.Result {
 	if !callCtx.InLoop {
 		callCtx.Errf("%s is only useful in a loop\n", name)
-		return Result{}
+		return builtins.Result{}
 	}
 
 	n := 1
@@ -31,19 +38,19 @@ func loopControl(callCtx *CallContext, name string, args []string) Result {
 		parsed, err := strconv.Atoi(args[0])
 		if err != nil {
 			callCtx.Errf("%s: %s: numeric argument required\n", name, args[0])
-			return Result{Code: 128, Exiting: true}
+			return builtins.Result{Code: 128, Exiting: true}
 		}
 		if parsed < 1 {
 			callCtx.Errf("%s: %s: loop count out of range\n", name, args[0])
-			return Result{Code: 1, BreakN: 1}
+			return builtins.Result{Code: 1, BreakN: 1}
 		}
 		n = parsed
 	default:
 		callCtx.Errf("%s: too many arguments\n", name)
-		return Result{Code: 1, BreakN: 1}
+		return builtins.Result{Code: 1, BreakN: 1}
 	}
 
-	var r Result
+	var r builtins.Result
 	if name == "break" {
 		r.BreakN = n
 	} else {
diff --git a/interp/builtins/cat.go b/interp/builtins/cmds/cat.go
similarity index 69%
rename from interp/builtins/cat.go
rename to interp/builtins/cmds/cat.go
index ceaefbee..d76baed7 100644
--- a/interp/builtins/cat.go
+++ b/interp/builtins/cmds/cat.go
@@ -3,15 +3,21 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/).
 // Copyright 2026-present Datadog, Inc.
 
-package builtins
+package cmds
 
 import (
 	"context"
 	"io"
 	"os"
+
+	"github.com/DataDog/rshell/interp/builtins"
 )
 
-func builtinCat(ctx context.Context, callCtx *CallContext, args []string) Result {
+func init() {
+	builtins.Register("cat", builtinCat)
+}
+
+func builtinCat(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result {
 	if len(args) == 0 {
 		args = []string{"-"}
 	}
@@ -23,12 +29,12 @@ func builtinCat(ctx context.Context, callCtx *CallContext, args []string) Result
 		}
 	}
 	if failed {
-		return Result{Code: 1}
+		return builtins.Result{Code: 1}
 	}
-	return Result{}
+	return builtins.Result{}
 }
 
-func catFile(ctx context.Context, callCtx *CallContext, path string) error {
+func catFile(ctx context.Context, callCtx *builtins.CallContext, path string) error {
 	var rc io.ReadCloser
 	if path == "-" {
 		if callCtx.Stdin == nil {
diff --git a/interp/builtins/echo.go b/interp/builtins/cmds/echo.go
similarity index 57%
rename from interp/builtins/echo.go
rename to interp/builtins/cmds/echo.go
index 26b03e76..7b1cf4e9 100644
--- a/interp/builtins/echo.go
+++ b/interp/builtins/cmds/echo.go
@@ -3,11 +3,19 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/).
 // Copyright 2026-present Datadog, Inc.
 
-package builtins
+package cmds
 
-import "context"
+import (
+	"context"
 
-func builtinEcho(_ context.Context, callCtx *CallContext, args []string) Result {
+	"github.com/DataDog/rshell/interp/builtins"
+)
+
+func init() {
+	builtins.Register("echo", builtinEcho)
+}
+
+func builtinEcho(_ context.Context, callCtx *builtins.CallContext, args []string) builtins.Result {
 	for i, arg := range args {
 		if i > 0 {
 			callCtx.Out(" ")
@@ -15,5 +23,5 @@ func builtinEcho(_ context.Context, callCtx *CallContext, args []string) Result
 		callCtx.Out(arg)
 	}
 	callCtx.Out("\n")
-	return Result{}
+	return builtins.Result{}
 }
diff --git a/interp/builtins/exit.go b/interp/builtins/cmds/exit.go
similarity index 78%
rename from interp/builtins/exit.go
rename to interp/builtins/cmds/exit.go
index a38de127..d002a97a 100644
--- a/interp/builtins/exit.go
+++ b/interp/builtins/cmds/exit.go
@@ -3,15 +3,21 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/).
 // Copyright 2026-present Datadog, Inc.
 
-package builtins
+package cmds
 
 import (
 	"context"
 	"strconv"
+
+	"github.com/DataDog/rshell/interp/builtins"
 )
 
-func builtinExit(_ context.Context, callCtx *CallContext, args []string) Result {
-	var r Result
+func init() {
+	builtins.Register("exit", builtinExit)
+}
+
+func builtinExit(_ context.Context, callCtx *builtins.CallContext, args []string) builtins.Result {
+	var r builtins.Result
 	if len(args) > 0 && args[0] == "--" {
 		args = args[1:]
 	}
diff --git a/interp/builtins/cmds/true_false.go b/interp/builtins/cmds/true_false.go
new file mode 100644
index 00000000..bd7a0635
--- /dev/null
+++ b/interp/builtins/cmds/true_false.go
@@ -0,0 +1,25 @@
+// 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 cmds
+
+import (
+	"context"
+
+	"github.com/DataDog/rshell/interp/builtins"
+)
+
+func init() {
+	builtins.Register("true", builtinTrue)
+	builtins.Register("false", builtinFalse)
+}
+
+func builtinTrue(_ context.Context, _ *builtins.CallContext, _ []string) builtins.Result {
+	return builtins.Result{}
+}
+
+func builtinFalse(_ context.Context, _ *builtins.CallContext, _ []string) builtins.Result {
+	return builtins.Result{Code: 1}
+}
diff --git a/interp/builtins/true_false.go b/interp/builtins/true_false.go
deleted file mode 100644
index 4889c7fc..00000000
--- a/interp/builtins/true_false.go
+++ /dev/null
@@ -1,16 +0,0 @@
-// 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 builtins
-
-import "context"
-
-func builtinTrue(_ context.Context, _ *CallContext, _ []string) Result {
-	return Result{}
-}
-
-func builtinFalse(_ context.Context, _ *CallContext, _ []string) Result {
-	return Result{Code: 1}
-}
diff --git a/interp/runner_exec.go b/interp/runner_exec.go
index 0854a843..d84f949e 100644
--- a/interp/runner_exec.go
+++ b/interp/runner_exec.go
@@ -16,6 +16,7 @@ import (
 	"mvdan.cc/sh/v3/syntax"
 
 	"github.com/DataDog/rshell/interp/builtins"
+	_ "github.com/DataDog/rshell/interp/builtins/cmds"
 )
 
 func (r *Runner) stmt(ctx context.Context, st *syntax.Stmt) {
@@ -203,7 +204,6 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) {
 		call := &builtins.CallContext{
 			Stdout:       r.stdout,
 			Stderr:       r.stderr,
-			Stdin:        r.stdin,
 			InLoop:       r.inLoop,
 			LastExitCode: r.lastExit.code,
 			OpenFile: func(ctx context.Context, path string, flags int, mode os.FileMode) (io.ReadWriteCloser, error) {
@@ -211,6 +211,9 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string) {
 			},
 			PortableErr: portableErrMsg,
 		}
+		if r.stdin != nil { // do not assign a typed nil into the io.Reader interface
+			call.Stdin = r.stdin
+		}
 		result := fn(ctx, call, args[1:])
 		r.exit.code = result.Code
 		r.exit.exiting = result.Exiting
diff --git a/tests/import_allowlist_test.go b/tests/import_allowlist_test.go
new file mode 100644
index 00000000..37c273fb
--- /dev/null
+++ b/tests/import_allowlist_test.go
@@ -0,0 +1,141 @@
+// 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 tests
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// builtinAllowedSymbols lists every "importpath.Symbol" that may be used by
+// files in interp/builtins/cmds/. Each entry must be in "importpath.Symbol"
+// form, where importpath is the full Go import path.
+//
+// To use a new symbol, add a single line here.
+//
+// Permanently banned (cannot be added):
+//   - reflect  — reflection defeats static safety analysis
+//   - unsafe   — bypasses Go's type and memory safety guarantees
+//
+// All packages not listed here are implicitly banned, including third-party
+// and other internal module packages.
+var builtinAllowedSymbols = []string{
+	"context.Context",
+	"github.com/DataDog/rshell/interp/builtins.CallContext",
+	"github.com/DataDog/rshell/interp/builtins.Register",
+	"github.com/DataDog/rshell/interp/builtins.Result",
+	"io.Copy",
+	"io.NopCloser",
+	"io.ReadCloser",
+	"os.O_RDONLY",
+	"strconv.Atoi",
+}
+
+// permanentlyBanned lists packages that may never be imported by builtins,
+// regardless of what symbols they export.
+var permanentlyBanned = map[string]string{
+	"reflect": "reflection defeats static safety analysis",
+	"unsafe":  "bypasses Go's type and memory safety guarantees",
+}
+
+// TestBuiltinImportAllowlist enforces symbol-level import restrictions on all
+// .go files in interp/builtins/cmds/. Every imported package must appear in
+// builtinAllowedSymbols, and every pkg.Symbol reference must be explicitly
+// listed there.
+func TestBuiltinImportAllowlist(t *testing.T) {
+	// Build lookup sets from the allowlist.
+	allowedSymbols := make(map[string]bool, len(builtinAllowedSymbols))
+	allowedPackages := make(map[string]bool)
+	for _, entry := range builtinAllowedSymbols {
+		dot := strings.LastIndexByte(entry, '.')
+		if dot <= 0 {
+			t.Fatalf("malformed allowlist entry (no dot): %q", entry)
+		}
+		allowedSymbols[entry] = true
+		allowedPackages[entry[:dot]] = true
+	}
+
+	root := repoRoot(t)
+	cmdsDir := filepath.Join(root, "interp", "builtins", "cmds")
+
+	entries, err := os.ReadDir(cmdsDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	fset := token.NewFileSet()
+	for _, entry := range entries {
+		name := entry.Name()
+		if !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
+			continue
+		}
+		path := filepath.Join(cmdsDir, name)
+		f, err := parser.ParseFile(fset, path, nil, 0)
+		if err != nil {
+			t.Errorf("%s: parse error: %v", name, err)
+			continue
+		}
+
+		// Build a map from local package name → import path and validate each import.
+		localToPath := make(map[string]string)
+		for _, imp := range f.Imports {
+			importPath := strings.Trim(imp.Path.Value, `"`)
+
+			if reason, banned := permanentlyBanned[importPath]; banned {
+				t.Errorf("%s: import of %q is permanently banned (%s)", name, importPath, reason)
+				continue
+			}
+
+			// Determine the local name used to reference this package.
+			var localName string
+			if imp.Name != nil {
+				localName = imp.Name.Name
+			} else {
+				parts := strings.Split(importPath, "/")
+				localName = parts[len(parts)-1]
+			}
+
+			if localName == "_" || localName == "." {
+				t.Errorf("%s: blank/dot import of %q is not allowed", name, importPath)
+				continue
+			}
+
+			if !allowedPackages[importPath] {
+				t.Errorf("%s: import of %q is not in the allowlist", name, importPath)
+				continue
+			}
+
+			localToPath[localName] = importPath
+		}
+
+		// Walk all selector expressions and verify each pkg.Symbol is allowed.
+		ast.Inspect(f, func(n ast.Node) bool {
+			sel, ok := n.(*ast.SelectorExpr)
+			if !ok {
+				return true
+			}
+			ident, ok := sel.X.(*ast.Ident)
+			if !ok {
+				return true
+			}
+			importPath, ok := localToPath[ident.Name]
+			if !ok {
+				return true // not a package-level selector
+			}
+			key := importPath + "." + sel.Sel.Name
+			if !allowedSymbols[key] {
+				pos := fset.Position(sel.Pos())
+				t.Errorf("%s:%d: %s is not in the allowlist", name, pos.Line, key)
+			}
+			return true
+		})
+	}
+}

From ac22b009b051fbaf48e9bc7551f2f7c8a472baaf Mon Sep 17 00:00:00 2001
From: Travis Thieman <travis.thieman@gmail.com>
Date: Mon, 9 Mar 2026 14:38:08 -0400
Subject: [PATCH 2/2] Collapse cmds/ back into builtins package, make register
 private
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Renamed Register → register (unexported); the Go compiler now enforces
  that only files within package builtins can call it, making the
  AST-based TestBuiltinRegisterOnlyFromCmds test redundant — removed it

- Moved command implementations back from interp/builtins/cmds/ into
  interp/builtins/ (same package), removing the need for the blank import
  in runner_exec.go and the internal-package allowlist entries

- Restored the builtins.go exclusion in TestBuiltinImportAllowlist; this
  is safe because Go's per-file import system ensures that imports in
  builtins.go do not bleed into command implementation files

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 interp/builtins/{cmds => }/break_continue.go | 24 ++++++------
 interp/builtins/builtins.go                  |  4 +-
 interp/builtins/{cmds => }/cat.go            | 14 +++----
 interp/builtins/cmds/true_false.go           | 25 -------------
 interp/builtins/{cmds => }/echo.go           | 14 +++----
 interp/builtins/{cmds => }/exit.go           | 10 ++---
 interp/builtins/true_false.go                | 21 +++++++++++
 interp/runner_exec.go                        |  1 -
 tests/import_allowlist_test.go               | 39 +++++++++++---------
 9 files changed, 71 insertions(+), 81 deletions(-)
 rename interp/builtins/{cmds => }/break_continue.go (58%)
 rename interp/builtins/{cmds => }/cat.go (71%)
 delete mode 100644 interp/builtins/cmds/true_false.go
 rename interp/builtins/{cmds => }/echo.go (60%)
 rename interp/builtins/{cmds => }/exit.go (79%)
 create mode 100644 interp/builtins/true_false.go

diff --git a/interp/builtins/cmds/break_continue.go b/interp/builtins/break_continue.go
similarity index 58%
rename from interp/builtins/cmds/break_continue.go
rename to interp/builtins/break_continue.go
index fb4502fc..586cdb7d 100644
--- a/interp/builtins/cmds/break_continue.go
+++ b/interp/builtins/break_continue.go
@@ -3,32 +3,30 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/).
 // Copyright 2026-present Datadog, Inc.
 
-package cmds
+package builtins
 
 import (
 	"context"
 	"strconv"
-
-	"github.com/DataDog/rshell/interp/builtins"
 )
 
 func init() {
-	builtins.Register("break", builtinBreak)
-	builtins.Register("continue", builtinContinue)
+	register("break", builtinBreak)
+	register("continue", builtinContinue)
 }
 
-func builtinBreak(_ context.Context, callCtx *builtins.CallContext, args []string) builtins.Result {
+func builtinBreak(_ context.Context, callCtx *CallContext, args []string) Result {
 	return loopControl(callCtx, "break", args)
 }
 
-func builtinContinue(_ context.Context, callCtx *builtins.CallContext, args []string) builtins.Result {
+func builtinContinue(_ context.Context, callCtx *CallContext, args []string) Result {
 	return loopControl(callCtx, "continue", args)
 }
 
-func loopControl(callCtx *builtins.CallContext, name string, args []string) builtins.Result {
+func loopControl(callCtx *CallContext, name string, args []string) Result {
 	if !callCtx.InLoop {
 		callCtx.Errf("%s is only useful in a loop\n", name)
-		return builtins.Result{}
+		return Result{}
 	}
 
 	n := 1
@@ -38,19 +36,19 @@ func loopControl(callCtx *builtins.CallContext, name string, args []string) buil
 		parsed, err := strconv.Atoi(args[0])
 		if err != nil {
 			callCtx.Errf("%s: %s: numeric argument required\n", name, args[0])
-			return builtins.Result{Code: 128, Exiting: true}
+			return Result{Code: 128, Exiting: true}
 		}
 		if parsed < 1 {
 			callCtx.Errf("%s: %s: loop count out of range\n", name, args[0])
-			return builtins.Result{Code: 1, BreakN: 1}
+			return Result{Code: 1, BreakN: 1}
 		}
 		n = parsed
 	default:
 		callCtx.Errf("%s: too many arguments\n", name)
-		return builtins.Result{Code: 1, BreakN: 1}
+		return Result{Code: 1, BreakN: 1}
 	}
 
-	var r builtins.Result
+	var r Result
 	if name == "break" {
 		r.BreakN = n
 	} else {
diff --git a/interp/builtins/builtins.go b/interp/builtins/builtins.go
index 05c92e1f..6daf4897 100644
--- a/interp/builtins/builtins.go
+++ b/interp/builtins/builtins.go
@@ -67,9 +67,9 @@ type Result struct {
 
 var registry = map[string]HandlerFunc{}
 
-// Register adds a builtin command to the registry.
+// register adds a builtin command to the registry.
 // It panics if name is already registered, catching duplicate registrations at startup.
-func Register(name string, fn HandlerFunc) {
+func register(name string, fn HandlerFunc) {
 	if _, exists := registry[name]; exists {
 		panic("builtin already registered: " + name)
 	}
diff --git a/interp/builtins/cmds/cat.go b/interp/builtins/cat.go
similarity index 71%
rename from interp/builtins/cmds/cat.go
rename to interp/builtins/cat.go
index d76baed7..9cfe4552 100644
--- a/interp/builtins/cmds/cat.go
+++ b/interp/builtins/cat.go
@@ -3,21 +3,19 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/).
 // Copyright 2026-present Datadog, Inc.
 
-package cmds
+package builtins
 
 import (
 	"context"
 	"io"
 	"os"
-
-	"github.com/DataDog/rshell/interp/builtins"
 )
 
 func init() {
-	builtins.Register("cat", builtinCat)
+	register("cat", builtinCat)
 }
 
-func builtinCat(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result {
+func builtinCat(ctx context.Context, callCtx *CallContext, args []string) Result {
 	if len(args) == 0 {
 		args = []string{"-"}
 	}
@@ -29,12 +27,12 @@ func builtinCat(ctx context.Context, callCtx *builtins.CallContext, args []strin
 		}
 	}
 	if failed {
-		return builtins.Result{Code: 1}
+		return Result{Code: 1}
 	}
-	return builtins.Result{}
+	return Result{}
 }
 
-func catFile(ctx context.Context, callCtx *builtins.CallContext, path string) error {
+func catFile(ctx context.Context, callCtx *CallContext, path string) error {
 	var rc io.ReadCloser
 	if path == "-" {
 		if callCtx.Stdin == nil {
diff --git a/interp/builtins/cmds/true_false.go b/interp/builtins/cmds/true_false.go
deleted file mode 100644
index bd7a0635..00000000
--- a/interp/builtins/cmds/true_false.go
+++ /dev/null
@@ -1,25 +0,0 @@
-// 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 cmds
-
-import (
-	"context"
-
-	"github.com/DataDog/rshell/interp/builtins"
-)
-
-func init() {
-	builtins.Register("true", builtinTrue)
-	builtins.Register("false", builtinFalse)
-}
-
-func builtinTrue(_ context.Context, _ *builtins.CallContext, _ []string) builtins.Result {
-	return builtins.Result{}
-}
-
-func builtinFalse(_ context.Context, _ *builtins.CallContext, _ []string) builtins.Result {
-	return builtins.Result{Code: 1}
-}
diff --git a/interp/builtins/cmds/echo.go b/interp/builtins/echo.go
similarity index 60%
rename from interp/builtins/cmds/echo.go
rename to interp/builtins/echo.go
index 7b1cf4e9..7f2211c7 100644
--- a/interp/builtins/cmds/echo.go
+++ b/interp/builtins/echo.go
@@ -3,19 +3,15 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/).
 // Copyright 2026-present Datadog, Inc.
 
-package cmds
+package builtins
 
-import (
-	"context"
-
-	"github.com/DataDog/rshell/interp/builtins"
-)
+import "context"
 
 func init() {
-	builtins.Register("echo", builtinEcho)
+	register("echo", builtinEcho)
 }
 
-func builtinEcho(_ context.Context, callCtx *builtins.CallContext, args []string) builtins.Result {
+func builtinEcho(_ context.Context, callCtx *CallContext, args []string) Result {
 	for i, arg := range args {
 		if i > 0 {
 			callCtx.Out(" ")
@@ -23,5 +19,5 @@ func builtinEcho(_ context.Context, callCtx *builtins.CallContext, args []string
 		callCtx.Out(arg)
 	}
 	callCtx.Out("\n")
-	return builtins.Result{}
+	return Result{}
 }
diff --git a/interp/builtins/cmds/exit.go b/interp/builtins/exit.go
similarity index 79%
rename from interp/builtins/cmds/exit.go
rename to interp/builtins/exit.go
index d002a97a..c36d233b 100644
--- a/interp/builtins/cmds/exit.go
+++ b/interp/builtins/exit.go
@@ -3,21 +3,19 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/).
 // Copyright 2026-present Datadog, Inc.
 
-package cmds
+package builtins
 
 import (
 	"context"
 	"strconv"
-
-	"github.com/DataDog/rshell/interp/builtins"
 )
 
 func init() {
-	builtins.Register("exit", builtinExit)
+	register("exit", builtinExit)
 }
 
-func builtinExit(_ context.Context, callCtx *builtins.CallContext, args []string) builtins.Result {
-	var r builtins.Result
+func builtinExit(_ context.Context, callCtx *CallContext, args []string) Result {
+	var r Result
 	if len(args) > 0 && args[0] == "--" {
 		args = args[1:]
 	}
diff --git a/interp/builtins/true_false.go b/interp/builtins/true_false.go
new file mode 100644
index 00000000..c27ae736
--- /dev/null
+++ b/interp/builtins/true_false.go
@@ -0,0 +1,21 @@
+// 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 builtins
+
+import "context"
+
+func init() {
+	register("true", builtinTrue)
+	register("false", builtinFalse)
+}
+
+func builtinTrue(_ context.Context, _ *CallContext, _ []string) Result {
+	return Result{}
+}
+
+func builtinFalse(_ context.Context, _ *CallContext, _ []string) Result {
+	return Result{Code: 1}
+}
diff --git a/interp/runner_exec.go b/interp/runner_exec.go
index d84f949e..4c3b4870 100644
--- a/interp/runner_exec.go
+++ b/interp/runner_exec.go
@@ -16,7 +16,6 @@ import (
 	"mvdan.cc/sh/v3/syntax"
 
 	"github.com/DataDog/rshell/interp/builtins"
-	_ "github.com/DataDog/rshell/interp/builtins/cmds"
 )
 
 func (r *Runner) stmt(ctx context.Context, st *syntax.Stmt) {
diff --git a/tests/import_allowlist_test.go b/tests/import_allowlist_test.go
index 37c273fb..a30785be 100644
--- a/tests/import_allowlist_test.go
+++ b/tests/import_allowlist_test.go
@@ -16,8 +16,8 @@ import (
 )
 
 // builtinAllowedSymbols lists every "importpath.Symbol" that may be used by
-// files in interp/builtins/cmds/. Each entry must be in "importpath.Symbol"
-// form, where importpath is the full Go import path.
+// command implementation files in interp/builtins/. Each entry must be in
+// "importpath.Symbol" form, where importpath is the full Go import path.
 //
 // To use a new symbol, add a single line here.
 //
@@ -25,13 +25,10 @@ import (
 //   - reflect  — reflection defeats static safety analysis
 //   - unsafe   — bypasses Go's type and memory safety guarantees
 //
-// All packages not listed here are implicitly banned, including third-party
-// and other internal module packages.
+// All packages not listed here are implicitly banned, including all
+// third-party packages and other internal module packages.
 var builtinAllowedSymbols = []string{
 	"context.Context",
-	"github.com/DataDog/rshell/interp/builtins.CallContext",
-	"github.com/DataDog/rshell/interp/builtins.Register",
-	"github.com/DataDog/rshell/interp/builtins.Result",
 	"io.Copy",
 	"io.NopCloser",
 	"io.ReadCloser",
@@ -39,17 +36,17 @@ var builtinAllowedSymbols = []string{
 	"strconv.Atoi",
 }
 
-// permanentlyBanned lists packages that may never be imported by builtins,
-// regardless of what symbols they export.
+// permanentlyBanned lists packages that may never be imported by builtin
+// command implementations, regardless of what symbols they export.
 var permanentlyBanned = map[string]string{
 	"reflect": "reflection defeats static safety analysis",
 	"unsafe":  "bypasses Go's type and memory safety guarantees",
 }
 
-// TestBuiltinImportAllowlist enforces symbol-level import restrictions on all
-// .go files in interp/builtins/cmds/. Every imported package must appear in
-// builtinAllowedSymbols, and every pkg.Symbol reference must be explicitly
-// listed there.
+// TestBuiltinImportAllowlist enforces symbol-level import restrictions on
+// command implementation files in interp/builtins/. builtins.go is exempt as
+// the package framework. Every other file's imports and pkg.Symbol references
+// must be explicitly listed in builtinAllowedSymbols.
 func TestBuiltinImportAllowlist(t *testing.T) {
 	// Build lookup sets from the allowlist.
 	allowedSymbols := make(map[string]bool, len(builtinAllowedSymbols))
@@ -64,20 +61,25 @@ func TestBuiltinImportAllowlist(t *testing.T) {
 	}
 
 	root := repoRoot(t)
-	cmdsDir := filepath.Join(root, "interp", "builtins", "cmds")
+	builtinsDir := filepath.Join(root, "interp", "builtins")
 
-	entries, err := os.ReadDir(cmdsDir)
+	entries, err := os.ReadDir(builtinsDir)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	fset := token.NewFileSet()
+	checked := 0
 	for _, entry := range entries {
 		name := entry.Name()
-		if !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
+		// builtins.go is the package framework (CallContext, Result, register,
+		// Lookup) and is exempt. Only command implementation files are checked.
+		if !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") || name == "builtins.go" {
 			continue
 		}
-		path := filepath.Join(cmdsDir, name)
+		checked++
+
+		path := filepath.Join(builtinsDir, name)
 		f, err := parser.ParseFile(fset, path, nil, 0)
 		if err != nil {
 			t.Errorf("%s: parse error: %v", name, err)
@@ -138,4 +140,7 @@ func TestBuiltinImportAllowlist(t *testing.T) {
 			return true
 		})
 	}
+	if checked == 0 {
+		t.Fatal("no command implementation files found in interp/builtins/ — builtins.go may have been moved or the directory is empty")
+	}
 }