feat(tools): opt-in shell tool without interpreter: allowlist per chain element, no expansions/redirections, workspace-only, timeout, output limit, scrubbed env

cmd/byteclaw/main.go

@@ -63,6 +63,14 @@ func run(cfgPath string, log *slog.Logger) error {
 		reg.Register(tools.FileWrite{Workspace: ws})
 		reg.Register(tools.FileList{Workspace: ws})
 	}
+	if sc := cfg.Tools.Shell; sc.Enabled {
+		sh, err := tools.NewShell(cfg.Workspace, sc.AllowedCommands, time.Duration(sc.TimeoutSec)*time.Second, sc.MaxOutputBytes)
+		if err != nil {
+			return err
+		}
+		reg.Register(sh)
+		log.Warn("shell tool enabled", "allowed", sc.AllowedCommands)
+	}
 	prov := llm.NewOpenAI(cfg.LLM.BaseURL, cfg.LLM.APIKey, cfg.LLM.Model, cfg.LLM.Temperature, time.Duration(cfg.LLM.TimeoutSec)*time.Second)
 	ag := &agent.Agent{LLM: prov, Tools: reg, Store: st, SystemPrompt: cfg.Agent.SystemPrompt,
 		MaxIterations: cfg.Agent.MaxIterations, HistoryLimit: cfg.Agent.HistoryLimit}

config.example.yaml

@@ -29,5 +29,10 @@ tools:
     max_bytes: 524288
     timeout_sec: 15
     allow_private: false        # true allows localhost/LAN targets (SSRF risk)
+  shell:                        # opt-in; runs WITHOUT a shell interpreter, only in the workspace
+    enabled: false
+    allowed_commands: []        # e.g. [ls, cat, wc, head, tail, grep, date]; sh/bash/env/xargs/python/... are refused
+    timeout_sec: 30
+    max_output_bytes: 65536
   files_enabled: true
   file_max_bytes: 1048576

internal/config/config.go

@@ -37,7 +37,16 @@ type WebFetch struct {
 	AllowPrivate bool `yaml:"allow_private" json:"allow_private"`
 }
 
+// Shell configures the opt-in shell tool (default off).
+type Shell struct {
+	Enabled         bool     `yaml:"enabled" json:"enabled"`
+	AllowedCommands []string `yaml:"allowed_commands" json:"allowed_commands"`
+	TimeoutSec      int      `yaml:"timeout_sec" json:"timeout_sec"`
+	MaxOutputBytes  int      `yaml:"max_output_bytes" json:"max_output_bytes"`
+}
+
 type Tools struct {
+	Shell    Shell    `yaml:"shell" json:"shell"`
 	WebFetch WebFetch `yaml:"web_fetch" json:"web_fetch"`
 	// FilesEnabled enables read/write/list tools confined to the workspace directory.
 	FilesEnabled bool `yaml:"files_enabled" json:"files_enabled"`
@@ -75,6 +84,7 @@ func Default() Config {
 		Tools: Tools{
 			WebFetch:     WebFetch{Enabled: true, MaxBytes: 512 * 1024, TimeoutSec: 15},
 			FilesEnabled: true,
+			Shell:        Shell{Enabled: false, TimeoutSec: 30, MaxOutputBytes: 64 * 1024},
 			FileMaxBytes: 1 << 20,
 		},
 	}

internal/tools/shell.go

@@ -0,0 +1,333 @@
+package tools
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"sync"
+	"time"
+
+	"github.com/crypto/byteclaw/internal/llm"
+)
+
+// Shell runs allowlisted commands inside the workspace WITHOUT a shell
+// interpreter: the command line is parsed here, every command of a chain
+// (&&, ||, ;, |) is checked against the allowlist, and pipes are wired
+// directly between processes. Expansions ($, `...`, <(...)), redirections,
+// background jobs and escapes are rejected. The environment is replaced
+// by a minimal one, so no API keys or tokens leak into child processes.
+type Shell struct {
+	Dir       string
+	Allowed   map[string]bool
+	Timeout   time.Duration
+	MaxOutput int
+}
+
+// NeverAllowed commands would re-open an interpreter or escape the sandbox; they
+// cannot be allowlisted.
+var NeverAllowed = map[string]bool{
+	"sh": true, "bash": true, "zsh": true, "dash": true, "ksh": true, "fish": true, "busybox": true,
+	"env": true, "xargs": true, "sudo": true, "su": true, "doas": true, "nohup": true, "timeout": true,
+	"eval": true, "exec": true, "command": true, "nice": true, "chroot": true, "setsid": true, "watch": true,
+	"python": true, "python3": true, "perl": true, "ruby": true, "node": true, "php": true, "lua": true, "awk": true,
+	"gawk": true, "sed": true, "ssh": true, "scp": true, "curl": true, "wget": true, "nc": true, "socat": true,
+	"git": true, "make": true, "vim": true, "vi": true, "less": true, "more": true, "man": true,
+}
+
+const safePath = "/usr/local/bin:/usr/bin:/bin"
+
+func NewShell(dir string, allowed []string, timeout time.Duration, maxOutput int) (*Shell, error) {
+	m := map[string]bool{}
+	for _, a := range allowed {
+		a = strings.TrimSpace(a)
+		if a == "" {
+			continue
+		}
+		if strings.ContainsRune(a, '/') || NeverAllowed[a] {
+			return nil, fmt.Errorf("command %q cannot be allowlisted", a)
+		}
+		m[a] = true
+	}
+	if len(m) == 0 {
+		return nil, errors.New("shell tool enabled but allowed_commands is empty")
+	}
+	return &Shell{Dir: dir, Allowed: m, Timeout: timeout, MaxOutput: maxOutput}, nil
+}
+
+func (s *Shell) Spec() llm.ToolSpec {
+	names := make([]string, 0, len(s.Allowed))
+	for n := range s.Allowed {
+		names = append(names, n)
+	}
+	return llm.ToolSpec{
+		Name: "shell",
+		Description: "Run a command in the workspace directory (no shell interpreter). Allowed commands: " +
+			strings.Join(sortStrings(names), ", ") + ". Supports && || ; and |. No $, backticks, redirections or absolute paths.",
+		Parameters: obj(map[string]any{"command": map[string]any{"type": "string"}}, "command"),
+	}
+}
+
+func sortStrings(s []string) []string {
+	for i := 1; i < len(s); i++ {
+		for j := i; j > 0 && s[j] < s[j-1]; j-- {
+			s[j], s[j-1] = s[j-1], s[j]
+		}
+	}
+	return s
+}
+
+// ---- parsing
+
+type step struct {
+	op       string     // operator preceding this pipeline: "", "&&", "||", ";"
+	pipeline [][]string // commands connected with |
+}
+
+var ErrRejected = errors.New("rejected")
+
+func rejectf(format string, a ...any) error {
+	return fmt.Errorf("%w: %s", ErrRejected, fmt.Sprintf(format, a...))
+}
+
+// Parse splits a command line into checked steps.
+func (s *Shell) Parse(line string) ([]step, error) {
+	if strings.TrimSpace(line) == "" {
+		return nil, rejectf("empty command")
+	}
+	for _, bad := range []string{"$", "`", "<", ">", "\\", "\n", "\r", "\x00"} {
+		if strings.Contains(line, bad) {
+			return nil, rejectf("forbidden character %q (no expansion, substitution or redirection)", bad)
+		}
+	}
+	var (
+		steps   []step
+		cur     []string // current command tokens
+		pipe    [][]string
+		tok     strings.Builder
+		inTok   bool
+		quote   rune
+		nextOp  = ""
+		runes   = []rune(line)
+	)
+	flushTok := func() {
+		if inTok {
+			cur = append(cur, tok.String())
+			tok.Reset()
+			inTok = false
+		}
+	}
+	endCmd := func() error {
+		flushTok()
+		if len(cur) == 0 {
+			return rejectf("empty command in chain")
+		}
+		pipe = append(pipe, cur)
+		cur = nil
+		return nil
+	}
+	endPipeline := func(op string) error {
+		if err := endCmd(); err != nil {
+			return err
+		}
+		steps = append(steps, step{op: nextOp, pipeline: pipe})
+		pipe, nextOp = nil, op
+		return nil
+	}
+	for i := 0; i < len(runes); i++ {
+		c := runes[i]
+		if quote != 0 {
+			if c == quote {
+				quote = 0
+			} else {
+				tok.WriteRune(c)
+			}
+			continue
+		}
+		switch {
+		case c == '\'' || c == '"':
+			quote, inTok = c, true
+		case c == ' ' || c == '\t':
+			flushTok()
+		case c == '&':
+			if i+1 < len(runes) && runes[i+1] == '&' {
+				if err := endPipeline("&&"); err != nil {
+					return nil, err
+				}
+				i++
+			} else {
+				return nil, rejectf("background jobs (&) are not allowed")
+			}
+		case c == '|':
+			if i+1 < len(runes) && runes[i+1] == '|' {
+				if err := endPipeline("||"); err != nil {
+					return nil, err
+				}
+				i++
+			} else if err := endCmd(); err != nil {
+				return nil, err
+			}
+		case c == ';':
+			if err := endPipeline(";"); err != nil {
+				return nil, err
+			}
+		case c == '(' || c == ')' || c == '{' || c == '}':
+			return nil, rejectf("subshells and groups are not allowed")
+		default:
+			tok.WriteRune(c)
+			inTok = true
+		}
+	}
+	if quote != 0 {
+		return nil, rejectf("unterminated quote")
+	}
+	if err := endPipeline(""); err != nil {
+		return nil, err
+	}
+	for _, st := range steps {
+		for _, argv := range st.pipeline {
+			if err := s.check(argv); err != nil {
+				return nil, err
+			}
+		}
+	}
+	return steps, nil
+}
+
+func (s *Shell) check(argv []string) error {
+	name := argv[0]
+	if strings.ContainsRune(name, '/') || !s.Allowed[name] || NeverAllowed[name] {
+		return rejectf("command %q is not allowed", name)
+	}
+	for _, a := range argv[1:] {
+		if strings.HasPrefix(a, "/") || strings.HasPrefix(a, "~") || strings.Contains(a, "=/") || strings.Contains(a, "=~") {
+			return rejectf("absolute or home paths are not allowed: %q", a)
+		}
+		for _, part := range strings.FieldsFunc(a, func(r rune) bool { return r == '/' || r == '=' || r == ':' || r == ',' }) {
+			if part == ".." {
+				return rejectf("path traversal is not allowed: %q", a)
+			}
+		}
+		if name == "find" && (strings.HasPrefix(a, "-exec") || strings.HasPrefix(a, "-ok") || a == "-delete" || strings.HasPrefix(a, "-fprint")) {
+			return rejectf("find %s is not allowed", a)
+		}
+	}
+	return nil
+}
+
+// ---- execution
+
+type limitedBuf struct {
+	mu        sync.Mutex
+	buf       bytes.Buffer
+	max       int
+	truncated bool
+}
+
+func (l *limitedBuf) Write(p []byte) (int, error) {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	if room := l.max - l.buf.Len(); room < len(p) {
+		if room > 0 {
+			l.buf.Write(p[:room])
+		}
+		l.truncated = true
+		return len(p), nil
+	}
+	return l.buf.Write(p)
+}
+
+func lookPath(name string) (string, error) {
+	for _, dir := range filepath.SplitList(safePath) {
+		p := filepath.Join(dir, name)
+		if fi, err := os.Stat(p); err == nil && !fi.IsDir() && fi.Mode()&0o111 != 0 {
+			return p, nil
+		}
+	}
+	return "", fmt.Errorf("command %q not found", name)
+}
+
+func (s *Shell) env() []string {
+	return []string{"PATH=" + safePath, "HOME=" + s.Dir, "LANG=C.UTF-8", "LC_ALL=C.UTF-8", "TERM=dumb"}
+}
+
+func (s *Shell) Call(ctx context.Context, raw json.RawMessage) (string, error) {
+	var a struct{ Command string `json:"command"` }
+	if err := json.Unmarshal(raw, &a); err != nil {
+		return "", err
+	}
+	steps, err := s.Parse(a.Command)
+	if err != nil {
+		return "", err
+	}
+	if err := os.MkdirAll(s.Dir, 0o750); err != nil {
+		return "", err
+	}
+	ctx, cancel := context.WithTimeout(ctx, s.Timeout)
+	defer cancel()
+	out := &limitedBuf{max: s.MaxOutput}
+	code := 0
+	for _, st := range steps {
+		if (st.op == "&&" && code != 0) || (st.op == "||" && code == 0) {
+			continue
+		}
+		code, err = s.runPipeline(ctx, st.pipeline, out)
+		if err != nil {
+			return "", err
+		}
+	}
+	res := out.buf.String()
+	if out.truncated {
+		res += "\n[output truncated]"
+	}
+	if ctx.Err() == context.DeadlineExceeded {
+		return res + fmt.Sprintf("\n[timeout after %s]", s.Timeout), nil
+	}
+	return fmt.Sprintf("%s\n[exit %d]", strings.TrimRight(res, "\n"), code), nil
+}
+
+func (s *Shell) runPipeline(ctx context.Context, pipeline [][]string, out io.Writer) (int, error) {
+	cmds := make([]*exec.Cmd, len(pipeline))
+	for i, argv := range pipeline {
+		bin, err := lookPath(argv[0])
+		if err != nil {
+			return 127, nil
+		}
+		c := exec.CommandContext(ctx, bin, argv[1:]...)
+		c.Dir, c.Env, c.Stderr = s.Dir, s.env(), out
+		c.WaitDelay = time.Second
+		cmds[i] = c
+	}
+	for i := 0; i < len(cmds)-1; i++ {
+		r, err := cmds[i].StdoutPipe()
+		if err != nil {
+			return 0, err
+		}
+		cmds[i+1].Stdin = r
+	}
+	cmds[len(cmds)-1].Stdout = out
+	for _, c := range cmds {
+		if err := c.Start(); err != nil {
+			return 126, nil
+		}
+	}
+	code := 0
+	for i, c := range cmds {
+		err := c.Wait()
+		if i == len(cmds)-1 {
+			var ee *exec.ExitError
+			if errors.As(err, &ee) {
+				code = ee.ExitCode()
+			} else if err != nil {
+				code = 1
+			}
+		}
+	}
+	return code, nil
+}

internal/tools/shell_test.go

@@ -0,0 +1,108 @@
+package tools
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+func newTestShell(t *testing.T) *Shell {
+	t.Helper()
+	s, err := NewShell(t.TempDir(), []string{"echo", "ls", "cat", "wc", "printenv", "sleep", "false", "true", "head", "find"}, 2*time.Second, 200)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return s
+}
+
+func run(t *testing.T, s *Shell, cmd string) (string, error) {
+	t.Helper()
+	b, _ := json.Marshal(map[string]string{"command": cmd})
+	return s.Call(context.Background(), b)
+}
+
+func TestShellRejectsBypassAttempts(t *testing.T) {
+	s := newTestShell(t)
+	for _, c := range []string{
+		"rm -rf x", "echo hi && rm x", "echo hi; rm x", "echo hi | sh", "false || bash",
+		"echo $(id)", "echo `id`", "cat <(id)", "echo ${HOME}", "echo $HOME", "echo hi > f", "cat < /etc/passwd",
+		"cat /etc/passwd", "cat ../secret", "cat a/../../secret", "ls ~", "cat --file=/etc/passwd",
+		"sleep 10 &", "(echo hi)", "{ echo hi; }", "/bin/echo hi", "./echo hi", "echo hi\nid",
+		"echo 'unterminated", "echo a\\;id", "find . -exec id ;", "find . -delete", "", "echo hi &&", "| echo",
+		"e''cho hi; r''m x",
+	} {
+		if _, err := s.Parse(c); err == nil || !errors.Is(err, ErrRejected) {
+			t.Errorf("%q should be rejected, got %v", c, err)
+		}
+	}
+}
+
+func TestShellNeverAllowed(t *testing.T) {
+	for _, c := range []string{"sh", "bash", "env", "xargs", "python3", "/bin/ls"} {
+		if _, err := NewShell(t.TempDir(), []string{c}, time.Second, 10); err == nil {
+			t.Errorf("%q must not be allowlistable", c)
+		}
+	}
+	if _, err := NewShell(t.TempDir(), nil, time.Second, 10); err == nil {
+		t.Error("empty allowlist must fail")
+	}
+}
+
+func TestShellRunsChainsAndPipes(t *testing.T) {
+	s := newTestShell(t)
+	os.WriteFile(filepath.Join(s.Dir, "f.txt"), []byte("a\nb\nc\n"), 0o600)
+	out, err := run(t, s, `cat f.txt | wc -l && echo "ok yes" ; false || echo fallback`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(out, "3\nok yes\nfallback") || !strings.HasSuffix(out, "[exit 0]") {
+		t.Fatalf("%q", out)
+	}
+	out, _ = run(t, s, "false && echo never")
+	if strings.Contains(out, "never") || !strings.HasSuffix(out, "[exit 1]") {
+		t.Fatalf("%q", out)
+	}
+	// metacharacters inside quotes are literal arguments, not operators
+	out, _ = run(t, s, `echo "a && b | c ; d"`)
+	if !strings.HasPrefix(out, "a && b | c ; d") {
+		t.Fatalf("%q", out)
+	}
+}
+
+func TestShellEnvironmentIsScrubbed(t *testing.T) {
+	t.Setenv("BYTECLAW_AUTH_TOKEN", "supersecret")
+	t.Setenv("DAHL_API_KEY", "dahl_secret")
+	s := newTestShell(t)
+	out, _ := run(t, s, "printenv")
+	if strings.Contains(out, "secret") || !strings.Contains(out, "HOME="+s.Dir) {
+		t.Fatalf("env leaked or wrong: %q", out)
+	}
+}
+
+func TestShellRunsInWorkspace(t *testing.T) {
+	s := newTestShell(t)
+	os.WriteFile(filepath.Join(s.Dir, "inside.txt"), nil, 0o600)
+	if out, _ := run(t, s, "ls"); !strings.Contains(out, "inside.txt") {
+		t.Fatalf("%q", out)
+	}
+}
+
+func TestShellTimeoutAndOutputLimit(t *testing.T) {
+	s := newTestShell(t)
+	s.Timeout = 300 * time.Millisecond
+	start := time.Now()
+	out, _ := run(t, s, "sleep 5")
+	if time.Since(start) > 3*time.Second || !strings.Contains(out, "timeout") {
+		t.Fatalf("timeout not enforced: %q after %s", out, time.Since(start))
+	}
+	s.Timeout = 2 * time.Second
+	out, _ = run(t, s, "echo "+strings.Repeat("x", 500))
+	if !strings.Contains(out, "[output truncated]") || len(out) > 300 {
+		t.Fatalf("limit not enforced: %d", len(out))
+	}
+}