diff --git a/interp/builtins/break/break.go b/interp/builtins/break/break.go new file mode 100644 index 00000000..ec7a070a --- /dev/null +++ b/interp/builtins/break/break.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 breakcmd + +import ( + "context" + + "github.com/DataDog/rshell/interp/builtins" + "github.com/DataDog/rshell/interp/builtins/internal/loopctl" +) + +func init() { + builtins.Register("break", run) +} + +func run(_ context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { + return loopctl.LoopControl(callCtx, "break", args) +} diff --git a/interp/builtins/builtins.go b/interp/builtins/builtins.go index 6daf4897..05c92e1f 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/cat.go b/interp/builtins/cat/cat.go similarity index 72% rename from interp/builtins/cat.go rename to interp/builtins/cat/cat.go index 9cfe4552..5d6d7c24 100644 --- a/interp/builtins/cat.go +++ b/interp/builtins/cat/cat.go @@ -3,19 +3,21 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2026-present Datadog, Inc. -package builtins +package cat import ( "context" "io" "os" + + "github.com/DataDog/rshell/interp/builtins" ) func init() { - register("cat", builtinCat) + builtins.Register("cat", run) } -func builtinCat(ctx context.Context, callCtx *CallContext, args []string) Result { +func run(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { if len(args) == 0 { args = []string{"-"} } @@ -27,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/continue/continue.go b/interp/builtins/continue/continue.go new file mode 100644 index 00000000..863f645c --- /dev/null +++ b/interp/builtins/continue/continue.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 continuecmd + +import ( + "context" + + "github.com/DataDog/rshell/interp/builtins" + "github.com/DataDog/rshell/interp/builtins/internal/loopctl" +) + +func init() { + builtins.Register("continue", run) +} + +func run(_ context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { + return loopctl.LoopControl(callCtx, "continue", args) +} diff --git a/interp/builtins/echo.go b/interp/builtins/echo/echo.go similarity index 61% rename from interp/builtins/echo.go rename to interp/builtins/echo/echo.go index 7f2211c7..183e08fc 100644 --- a/interp/builtins/echo.go +++ b/interp/builtins/echo/echo.go @@ -3,15 +3,19 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2026-present Datadog, Inc. -package builtins +package echo -import "context" +import ( + "context" + + "github.com/DataDog/rshell/interp/builtins" +) func init() { - register("echo", builtinEcho) + builtins.Register("echo", run) } -func builtinEcho(_ context.Context, callCtx *CallContext, args []string) Result { +func run(_ context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { for i, arg := range args { if i > 0 { callCtx.Out(" ") @@ -19,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/exit/exit.go similarity index 81% rename from interp/builtins/exit.go rename to interp/builtins/exit/exit.go index c36d233b..980a7b69 100644 --- a/interp/builtins/exit.go +++ b/interp/builtins/exit/exit.go @@ -3,19 +3,21 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2026-present Datadog, Inc. -package builtins +package exit import ( "context" "strconv" + + "github.com/DataDog/rshell/interp/builtins" ) func init() { - register("exit", builtinExit) + builtins.Register("exit", run) } -func builtinExit(_ context.Context, callCtx *CallContext, args []string) Result { - var r Result +func run(_ 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/false/false.go b/interp/builtins/false/false.go new file mode 100644 index 00000000..f43b12e9 --- /dev/null +++ b/interp/builtins/false/false.go @@ -0,0 +1,20 @@ +// 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 falsecmd + +import ( + "context" + + "github.com/DataDog/rshell/interp/builtins" +) + +func init() { + builtins.Register("false", run) +} + +func run(_ context.Context, _ *builtins.CallContext, _ []string) builtins.Result { + return builtins.Result{Code: 1} +} diff --git a/interp/builtins/head.go b/interp/builtins/head/head.go similarity index 78% rename from interp/builtins/head.go rename to interp/builtins/head/head.go index badaaaa2..97de7ac8 100644 --- a/interp/builtins/head.go +++ b/interp/builtins/head/head.go @@ -3,7 +3,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2026-present Datadog, Inc. -// Package builtins implements safe shell builtin commands. +// Package head implements the head builtin command. // // head — output the first part of files // @@ -40,14 +40,13 @@ // // Memory safety: // -// Line mode uses a streaming scanner with a per-line cap of maxHeadLineBytes +// Line mode uses a streaming scanner with a per-line cap of MaxLineBytes // (1 MiB). Lines that exceed this cap cause an error rather than an // unbounded allocation. Byte mode reads in fixed-size chunks; it never // allocates proportionally to user-supplied N. All loops check ctx.Err() // at each iteration to honour the shell's execution timeout and to support // graceful cancellation. - -package builtins +package head import ( "bufio" @@ -58,22 +57,24 @@ import ( "strconv" "github.com/spf13/pflag" + + "github.com/DataDog/rshell/interp/builtins" ) func init() { - register("head", builtinHead) + builtins.Register("head", run) } -// maxHeadCount is the maximum accepted line or byte count. Values above this +// MaxCount is the maximum accepted line or byte count. Values above this // are clamped. This prevents huge theoretical allocations while remaining // larger than any practical file. -const maxHeadCount = 1<<31 - 1 // 2 147 483 647 +const MaxCount = 1<<31 - 1 // 2 147 483 647 -// maxHeadLineBytes is the per-line buffer cap for the line scanner. Lines +// MaxLineBytes is the per-line buffer cap for the line scanner. Lines // longer than this are reported as an error instead of being buffered. -const maxHeadLineBytes = 1 << 20 // 1 MiB +const MaxLineBytes = 1 << 20 // 1 MiB -func builtinHead(ctx context.Context, callCtx *CallContext, args []string) Result { +func run(ctx context.Context, callCtx *builtins.CallContext, args []string) builtins.Result { fs := pflag.NewFlagSet("head", pflag.ContinueOnError) fs.SetOutput(io.Discard) @@ -87,14 +88,14 @@ func builtinHead(ctx context.Context, callCtx *CallContext, args []string) Resul // command line. pflag calls Set() in parse order, so the last flag Set // gets the highest pos value — no raw arg scanning required. var modeSeq int - linesFlag := newHeadModeFlag(&modeSeq, "10") - bytesFlag := newHeadModeFlag(&modeSeq, "") + linesFlag := newModeFlag(&modeSeq, "10") + bytesFlag := newModeFlag(&modeSeq, "") fs.VarP(linesFlag, "lines", "n", "print the first N lines instead of the first 10") fs.VarP(bytesFlag, "bytes", "c", "print the first N bytes instead of lines") if err := fs.Parse(args); err != nil { callCtx.Errf("head: %v\n", err) - return Result{Code: 1} + return builtins.Result{Code: 1} } if *help { @@ -103,7 +104,7 @@ func builtinHead(ctx context.Context, callCtx *CallContext, args []string) Resul callCtx.Out("With no FILE, or when FILE is -, read standard input.\n\n") fs.SetOutput(callCtx.Stdout) fs.PrintDefaults() - return Result{} + return builtins.Result{} } // --silent is an alias for --quiet. @@ -124,10 +125,10 @@ func builtinHead(ctx context.Context, callCtx *CallContext, args []string) Resul modeLabel = "bytes" } - count, ok := headParseCount(countStr) + count, ok := parseCount(countStr) if !ok { callCtx.Errf("head: invalid number of %s: %q\n", modeLabel, countStr) - return Result{Code: 1} + return builtins.Result{Code: 1} } // Collect file arguments; default to stdin. @@ -148,7 +149,7 @@ func builtinHead(ctx context.Context, callCtx *CallContext, args []string) Resul if ctx.Err() != nil { break } - if err := headProcessFile(ctx, callCtx, file, i, printHeaders, useBytesMode, count); err != nil { + if err := processFile(ctx, callCtx, file, i, printHeaders, useBytesMode, count); err != nil { name := file if file == "-" { name = "standard input" @@ -159,13 +160,13 @@ func builtinHead(ctx context.Context, callCtx *CallContext, args []string) Resul } if failed { - return Result{Code: 1} + return builtins.Result{Code: 1} } - return Result{} + return builtins.Result{} } -// headProcessFile opens and processes one file (or stdin for "-"). -func headProcessFile(ctx context.Context, callCtx *CallContext, file string, idx int, printHeaders, useBytesMode bool, count int64) error { +// processFile opens and processes one file (or stdin for "-"). +func processFile(ctx context.Context, callCtx *builtins.CallContext, file string, idx int, printHeaders, useBytesMode bool, count int64) error { var rc io.ReadCloser name := file if file == "-" { @@ -200,17 +201,17 @@ func headProcessFile(ctx context.Context, callCtx *CallContext, file string, idx } if useBytesMode { - return headBytes(ctx, callCtx, rc, count) + return readBytes(ctx, callCtx, rc, count) } - return headLines(ctx, callCtx, rc, count) + return readLines(ctx, callCtx, rc, count) } -// headLines writes the first count lines of r to callCtx.Stdout, preserving +// readLines writes the first count lines of r to callCtx.Stdout, preserving // line endings exactly (including a missing final newline). -func headLines(ctx context.Context, callCtx *CallContext, r io.Reader, count int64) error { +func readLines(ctx context.Context, callCtx *builtins.CallContext, r io.Reader, count int64) error { sc := bufio.NewScanner(r) buf := make([]byte, 4096) - sc.Buffer(buf, maxHeadLineBytes) + sc.Buffer(buf, MaxLineBytes) sc.Split(scanLinesPreservingNewline) var emitted int64 @@ -226,11 +227,11 @@ func headLines(ctx context.Context, callCtx *CallContext, r io.Reader, count int return sc.Err() } -// headBytes writes the first count bytes of r to callCtx.Stdout. It reads +// readBytes writes the first count bytes of r to callCtx.Stdout. It reads // in fixed-size chunks; the buffer is capped at chunkSize but shrunk to // count when count is smaller, avoiding unnecessary allocation for small // byte requests (e.g. head -c 5). -func headBytes(ctx context.Context, callCtx *CallContext, r io.Reader, count int64) error { +func readBytes(ctx context.Context, callCtx *builtins.CallContext, r io.Reader, count int64) error { if count == 0 { return nil } @@ -259,10 +260,10 @@ func headBytes(ctx context.Context, callCtx *CallContext, r io.Reader, count int return nil } -// headParseCount parses a line or byte count string. A leading '+' is +// parseCount parses a line or byte count string. A leading '+' is // accepted (treated as a positive sign by strconv.ParseInt, matching GNU // head behavior). Returns (count, true) on success, (0, false) on failure. -func headParseCount(s string) (int64, bool) { +func parseCount(s string) (int64, bool) { if s == "" { return 0, false } @@ -270,35 +271,35 @@ func headParseCount(s string) (int64, bool) { if err != nil || n < 0 { return 0, false } - if n > maxHeadCount { - n = maxHeadCount + if n > MaxCount { + n = MaxCount } return n, true } -// headModeFlag is a pflag.Value implementation for -n/--lines and -c/--bytes. -// Two headModeFlag values share a *seq counter; each call to Set increments +// modeFlag is a pflag.Value implementation for -n/--lines and -c/--bytes. +// Two modeFlag values share a *seq counter; each call to Set increments // the counter and records the new value in pos. After pflag.Parse, comparing // pos fields reveals which flag appeared last on the command line — without // scanning raw args or inspecting individual characters of flag tokens. -type headModeFlag struct { +type modeFlag struct { val string seq *int // shared per-invocation counter; incremented on every Set call pos int // counter value when Set was last called; 0 means never set } -func newHeadModeFlag(seq *int, defaultVal string) *headModeFlag { - return &headModeFlag{val: defaultVal, seq: seq} +func newModeFlag(seq *int, defaultVal string) *modeFlag { + return &modeFlag{val: defaultVal, seq: seq} } -func (f *headModeFlag) String() string { return f.val } -func (f *headModeFlag) Set(s string) error { +func (f *modeFlag) String() string { return f.val } +func (f *modeFlag) Set(s string) error { f.val = s *f.seq++ f.pos = *f.seq return nil } -func (f *headModeFlag) Type() string { return "string" } +func (f *modeFlag) Type() string { return "string" } // scanLinesPreservingNewline is a bufio.SplitFunc that includes the line // terminator (\n) in the returned token. Unlike bufio.ScanLines, it does not diff --git a/interp/builtins/tests/head/head_test.go b/interp/builtins/head/head_test.go similarity index 100% rename from interp/builtins/tests/head/head_test.go rename to interp/builtins/head/head_test.go diff --git a/interp/builtins/tests/head/head_unix_test.go b/interp/builtins/head/head_unix_test.go similarity index 100% rename from interp/builtins/tests/head/head_unix_test.go rename to interp/builtins/head/head_unix_test.go diff --git a/interp/builtins/tests/head/head_windows_test.go b/interp/builtins/head/head_windows_test.go similarity index 100% rename from interp/builtins/tests/head/head_windows_test.go rename to interp/builtins/head/head_windows_test.go diff --git a/interp/builtins/break_continue.go b/interp/builtins/internal/loopctl/loopctl.go similarity index 56% rename from interp/builtins/break_continue.go rename to interp/builtins/internal/loopctl/loopctl.go index 586cdb7d..05b2fc97 100644 --- a/interp/builtins/break_continue.go +++ b/interp/builtins/internal/loopctl/loopctl.go @@ -3,30 +3,19 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2026-present Datadog, Inc. -package builtins +package loopctl import ( - "context" "strconv" -) - -func init() { - register("break", builtinBreak) - register("continue", builtinContinue) -} -func builtinBreak(_ context.Context, callCtx *CallContext, args []string) Result { - return loopControl(callCtx, "break", args) -} - -func builtinContinue(_ context.Context, callCtx *CallContext, args []string) Result { - return loopControl(callCtx, "continue", args) -} + "github.com/DataDog/rshell/interp/builtins" +) -func loopControl(callCtx *CallContext, name string, args []string) Result { +// LoopControl implements the shared logic for the break and continue builtins. +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 @@ -36,19 +25,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/true/true.go b/interp/builtins/true/true.go new file mode 100644 index 00000000..9c8832b3 --- /dev/null +++ b/interp/builtins/true/true.go @@ -0,0 +1,20 @@ +// 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 truecmd + +import ( + "context" + + "github.com/DataDog/rshell/interp/builtins" +) + +func init() { + builtins.Register("true", run) +} + +func run(_ context.Context, _ *builtins.CallContext, _ []string) builtins.Result { + return builtins.Result{} +} diff --git a/interp/builtins/true_false.go b/interp/builtins/true_false.go deleted file mode 100644 index c27ae736..00000000 --- a/interp/builtins/true_false.go +++ /dev/null @@ -1,21 +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 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 4c3b4870..e767ec5e 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -16,6 +16,14 @@ import ( "mvdan.cc/sh/v3/syntax" "github.com/DataDog/rshell/interp/builtins" + _ "github.com/DataDog/rshell/interp/builtins/break" + _ "github.com/DataDog/rshell/interp/builtins/cat" + _ "github.com/DataDog/rshell/interp/builtins/continue" + _ "github.com/DataDog/rshell/interp/builtins/echo" + _ "github.com/DataDog/rshell/interp/builtins/exit" + _ "github.com/DataDog/rshell/interp/builtins/false" + _ "github.com/DataDog/rshell/interp/builtins/head" + _ "github.com/DataDog/rshell/interp/builtins/true" ) 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 f05cbbb7..4df9afd3 100644 --- a/tests/import_allowlist_test.go +++ b/tests/import_allowlist_test.go @@ -71,26 +71,46 @@ func TestBuiltinImportAllowlist(t *testing.T) { root := repoRoot(t) builtinsDir := filepath.Join(root, "interp", "builtins") - entries, err := os.ReadDir(builtinsDir) + // Collect all .go files in builtin sub-packages (each builtin lives + // in its own subdirectory, e.g. cat/cat.go, head/head.go). Internal + // shared packages (internal/) are also checked. + var goFiles []string + err := filepath.Walk(builtinsDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil + } + if !strings.HasSuffix(info.Name(), ".go") || strings.HasSuffix(info.Name(), "_test.go") { + return nil + } + rel, _ := filepath.Rel(builtinsDir, path) + // builtins.go is the package framework (CallContext, Result, Register, + // Lookup) and is exempt. Only command implementation files are checked. + if rel == "builtins.go" { + return nil + } + // Only check files inside subdirectories (the per-builtin packages). + if !strings.Contains(rel, string(filepath.Separator)) { + return nil + } + goFiles = append(goFiles, path) + return nil + }) if err != nil { t.Fatal(err) } fset := token.NewFileSet() checked := 0 - for _, entry := range entries { - name := entry.Name() - // 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 - } + for _, path := range goFiles { + rel, _ := filepath.Rel(builtinsDir, path) 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) + t.Errorf("%s: parse error: %v", rel, err) continue } @@ -100,7 +120,14 @@ func TestBuiltinImportAllowlist(t *testing.T) { 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) + t.Errorf("%s: import of %q is permanently banned (%s)", rel, importPath, reason) + continue + } + + // The parent builtins package and sibling internal packages are + // always allowed — they are part of the builtins module. + if importPath == "github.com/DataDog/rshell/interp/builtins" || + strings.HasPrefix(importPath, "github.com/DataDog/rshell/interp/builtins/internal/") { continue } @@ -114,12 +141,12 @@ func TestBuiltinImportAllowlist(t *testing.T) { } if localName == "_" || localName == "." { - t.Errorf("%s: blank/dot import of %q is not allowed", name, importPath) + t.Errorf("%s: blank/dot import of %q is not allowed", rel, importPath) continue } if !allowedPackages[importPath] { - t.Errorf("%s: import of %q is not in the allowlist", name, importPath) + t.Errorf("%s: import of %q is not in the allowlist", rel, importPath) continue } @@ -143,12 +170,12 @@ func TestBuiltinImportAllowlist(t *testing.T) { 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) + t.Errorf("%s:%d: %s is not in the allowlist", rel, pos.Line, key) } 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") + t.Fatal("no command implementation files found in interp/builtins/ sub-packages") } }