diff --git a/cmd/chat_subcommand_shell_test.go b/cmd/chat_subcommand_shell_test.go index b7f5081be..d4de22e95 100644 --- a/cmd/chat_subcommand_shell_test.go +++ b/cmd/chat_subcommand_shell_test.go @@ -71,6 +71,21 @@ func TestRunSubcommand_BashToolBlocksEnvDumpEvenWhenBypassed(t *testing.T) { } } +func TestRunSubcommand_AllowsNamedGoDiagnostics(t *testing.T) { + m := newTestChatModel() + m.session.PermSvc().SetAutonomy(safety.AutonomyYOLO) + + result, _ := (&runSubcommand{}).Handle(m, []string{"go", "env", "GOTOOLCHAIN"}, "/run go env GOTOOLCHAIN") + cm := requireChatModel(t, result) + if len(cm.messages) == 0 { + t.Fatal("expected display message") + } + last := cm.messages[len(cm.messages)-1] + if last.role == "error" || strings.Contains(last.content, "blocked:") || !strings.Contains(last.content, "$ go env GOTOOLCHAIN") { + t.Fatalf("safe Go diagnostic failed in CLI: role=%s output=%q", last.role, last.content) + } +} + func TestTestSubcommand_DetectsNonzeroExit(t *testing.T) { m := newTestChatModel() m.session.PermSvc().SetAutonomy(safety.AutonomyYOLO) diff --git a/internal/engine/branching/snowball.go b/internal/engine/branching/snowball.go index 9e2d29a76..7125ace27 100644 --- a/internal/engine/branching/snowball.go +++ b/internal/engine/branching/snowball.go @@ -16,6 +16,7 @@ type SnowballDetector struct { } // NewSnowballDetector creates a detector with the given absolute token ceiling. +// A non-positive ceiling leaves token limits to the configured session budget. // The default growth threshold is 2.0x (last 3 turns vs first 3 turns). func NewSnowballDetector(maxTokens int) *SnowballDetector { return &SnowballDetector{ @@ -31,13 +32,8 @@ func (sd *SnowballDetector) RecordTurn(tokens int, progress float64) { sd.turnProgress = append(sd.turnProgress, progress) } -// IsSnowballing returns true if the last 3 turns consumed threshold-x+ tokens -// compared to the first 3 turns, i.e. token consumption is accelerating without -// a proportional bound. The progress-per-token dimension is intentionally not -// used here: with the coarse progress signal available per turn it collapses -// to a pure growth check and adds no independent information (see RecordTurn -// call sites). Keeping this a honest growth-rate detector avoids a false sense -// of a second, stricter signal. +// IsSnowballing reports rising usage accompanied by declining activity. +// Growing prompts alone are normal during productive work and must not stop it. func (sd *SnowballDetector) IsSnowballing() bool { n := len(sd.turnTokens) if n < 6 { @@ -54,14 +50,14 @@ func (sd *SnowballDetector) IsSnowballing() bool { } growthRate := float64(lastAvg) / float64(firstAvg) - return growthRate >= sd.threshold + return growthRate >= sd.threshold && sd.progressTrend() == "declining" } // ShouldAbort returns true if total tokens exceed maxTokens or the growth rate -// exceeds 3x between the first and last 3-turn windows. +// exceeds 3x between the first and last 3-turn windows while activity declines. func (sd *SnowballDetector) ShouldAbort() bool { total := sumInt(sd.turnTokens) - if total > sd.maxTokens { + if sd.maxTokens > 0 && total > sd.maxTokens { return true } @@ -76,7 +72,16 @@ func (sd *SnowballDetector) ShouldAbort() bool { return false } - return float64(lastAvg)/float64(firstAvg) >= 3.0 + return float64(lastAvg)/float64(firstAvg) >= 3.0 && sd.progressTrend() == "declining" +} + +// StopReason explains the active stop condition without exposing debug output +// or claiming that a large cumulative input count means context is full. +func (sd *SnowballDetector) StopReason() string { + if sd.maxTokens > 0 && sumInt(sd.turnTokens) > sd.maxTokens { + return fmt.Sprintf("Token limit reached (%s/%s total input and output tokens).", formatTokenCount(sumInt(sd.turnTokens)), formatTokenCount(sd.maxTokens)) + } + return "Token usage increased sharply while tool activity declined." } // Summary returns a human-readable summary of the snowball state. @@ -91,7 +96,7 @@ func (sd *SnowballDetector) Summary() string { firstAvg := avgInt(sd.turnTokens[:3]) lastAvg := avgInt(sd.turnTokens[n-3:]) if firstAvg > 0 { - growthStr = fmt.Sprintf("growing %.1fx/turn", float64(lastAvg)/float64(firstAvg)) + growthStr = fmt.Sprintf("%.1fx initial usage", float64(lastAvg)/float64(firstAvg)) } else { growthStr = "no baseline" } diff --git a/internal/engine/branching/snowball_test.go b/internal/engine/branching/snowball_test.go index ad5fc82eb..57ac91d77 100644 --- a/internal/engine/branching/snowball_test.go +++ b/internal/engine/branching/snowball_test.go @@ -90,6 +90,24 @@ func TestSnowballDetector_ShouldAbort_GrowthRate3x(t *testing.T) { } } +func TestSnowballDetectorContinuesProductiveLargeWork(t *testing.T) { + for _, maxTokens := range []int{0, -1} { + sd := NewSnowballDetector(maxTokens) + for range 3 { + sd.RecordTurn(10000, 1) + } + for range 30 { + sd.RecordTurn(40000, 1) + } + if sd.ShouldAbort() || sd.IsSnowballing() { + t.Fatalf("productive work hit a hidden limit: %s", sd.Summary()) + } + if strings.Contains(sd.Summary(), "x/turn") { + t.Fatalf("window comparison mislabeled as per-turn growth: %s", sd.Summary()) + } + } +} + func TestSnowballDetector_Summary(t *testing.T) { sd := NewSnowballDetector(100000) diff --git a/internal/engine/lifecycle_service.go b/internal/engine/lifecycle_service.go index db6ebbd43..55ad00cce 100644 --- a/internal/engine/lifecycle_service.go +++ b/internal/engine/lifecycle_service.go @@ -97,7 +97,7 @@ func NewLifecycleService(log *logger.Logger) *LifecycleService { return &LifecycleService{ limits: lifecycle.NewLimitTracker(lifecycle.DefaultLimits()), loopDet: control.NewLoopDetector(10, control.DoomLoopThreshold), - snowball: branching.NewSnowballDetector(500000), + snowball: branching.NewSnowballDetector(0), beliefs: NewBeliefState(), backtrack: control.NewBacktrackEngine(), lifecycle: nil, // constructed in New() with cwd diff --git a/internal/engine/stream.go b/internal/engine/stream.go index 9b753651a..c427344b3 100644 --- a/internal/engine/stream.go +++ b/internal/engine/stream.go @@ -234,7 +234,9 @@ func (s *Session) agentLoop(ctx context.Context, ch chan<- StreamEvent) { toolTurns := 0 // turns that used tools (for skill distillation) var toolsUsedSet map[string]bool var filesModifiedSet map[string]bool - snowball := branching.NewSnowballDetector(500000) // 500K token ceiling + // Explicit session budgets own token limits. Re-reading conversation + // history across turns must not impose an unrelated lifetime ceiling. + snowball := branching.NewSnowballDetector(0) loopDet := control.NewLoopDetector(10, control.DoomLoopThreshold) // 10-step window, 3 repeats = doom loop for { diff --git a/internal/engine/stream_guards.go b/internal/engine/stream_guards.go index 0c37c24ed..cd578fc2a 100644 --- a/internal/engine/stream_guards.go +++ b/internal/engine/stream_guards.go @@ -31,7 +31,7 @@ func (s *Session) checkGuardConditions(ctx context.Context, ch chan<- StreamEven } if snowball.ShouldAbort() { - ch <- StreamEvent{Type: "content", Content: "\n\n" + snowball.Summary()} + ch <- StreamEvent{Type: "content", Content: "\n\nStopped: " + snowball.StopReason() + " Send a follow-up to continue from this conversation."} ch <- StreamEvent{Type: "done"} return false } diff --git a/internal/engine/stream_review_test.go b/internal/engine/stream_review_test.go new file mode 100644 index 000000000..bd026ac99 --- /dev/null +++ b/internal/engine/stream_review_test.go @@ -0,0 +1,64 @@ +package engine + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + "github.com/GrayCodeAI/reflect/internal/engine/branching" + "github.com/GrayCodeAI/reflect/internal/engine/control" + "github.com/GrayCodeAI/reflect/internal/engine/safety" + "github.com/GrayCodeAI/reflect/internal/tool" + "github.com/GrayCodeAI/reflect/internal/types" +) + +func TestReviewContinuesBeyondOldCumulativeTokenCeiling(t *testing.T) { + client := &conversationalStreamClient{resilienceBoundaryClient: &resilienceBoundaryClient{}} + for i := range 3 { + client.streams = append(client.streams, []types.RelayStreamEvent{ + {Type: "tool_call", ToolCall: &types.ToolCall{ID: fmt.Sprint(i), Name: "Read", Arguments: map[string]interface{}{"id": i}}}, + {Type: "done", StopReason: "tool_use", Usage: &types.RelayUsage{PromptTokens: 200000, CompletionTokens: 10}}, + }) + } + client.streams = append(client.streams, []types.RelayStreamEvent{{Type: "content", Content: "Review complete."}, {Type: "done", StopReason: "end_turn"}}) + s := NewSessionWithClient(client, "test", "test-model", "", tool.NewRegistry(&countedReadTool{}), false) + s.PermSvc().SetAutonomy(safety.AutonomyYOLO) + s.AddUser("Review the code") + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + events, err := s.Stream(ctx) + if err != nil { + t.Fatal(err) + } + var reply strings.Builder + for event := range events { + if event.Type == "error" { + t.Errorf("review failed: %s", event.Content) + } + if event.Type == "content" { + reply.WriteString(event.Content) + } + } + if !strings.Contains(reply.String(), "Review complete.") || len(client.options) != 4 { + t.Fatalf("review stopped early after %d requests: %s", len(client.options), reply.String()) + } +} + +func TestReviewStopExplainsReasonInCLI(t *testing.T) { + s := newMockSession(newMockClient()) + detector := branching.NewSnowballDetector(100) + detector.RecordTurn(101, 1) + ch := make(chan StreamEvent, 2) + if s.checkGuardConditions(t.Context(), ch, 0, detector, control.NewLoopDetector(10, control.DoomLoopThreshold)) { + t.Fatal("explicit token limit not enforced") + } + event := <-ch + if event.Type != "content" || !strings.Contains(event.Content, "Token limit reached") || !strings.Contains(event.Content, "follow-up") || strings.Contains(event.Content, "Recommendation:") || strings.Contains(event.Content, "fresh context") { + t.Fatalf("unhelpful stop message: %+v", event) + } + if event := <-ch; event.Type != "done" { + t.Fatalf("missing terminal event: %+v", event) + } +} diff --git a/internal/tool/grep.go b/internal/tool/grep.go index 5f77ff335..d8e7b40af 100644 --- a/internal/tool/grep.go +++ b/internal/tool/grep.go @@ -1,22 +1,35 @@ package tool import ( + "bufio" + "bytes" "context" "encoding/json" "fmt" + "io" "io/fs" "os" "path/filepath" "regexp" "strings" + "unicode/utf8" +) + +const ( + maxGrepMatches = 200 + maxGrepOutputBytes = 32 << 10 + maxGrepLineBytes = 64 << 10 + maxGrepPreviewBytes = 4 << 10 ) type GrepTool struct{} -func (GrepTool) Name() string { return "Grep" } -func (GrepTool) RiskLevel() string { return "low" } -func (GrepTool) Aliases() []string { return []string{"grep"} } -func (GrepTool) Description() string { return "Search for a regex pattern in files." } +func (GrepTool) Name() string { return "Grep" } +func (GrepTool) RiskLevel() string { return "low" } +func (GrepTool) Aliases() []string { return []string{"grep"} } +func (GrepTool) Description() string { + return "Search for a regex pattern in text files. Skips sensitive files, binaries, and files larger than 10 MiB. Results are bounded; narrow the pattern or use include for more specific results." +} // GrepInput is the typed input for GrepTool. type GrepInput struct { @@ -47,6 +60,9 @@ func (GrepTool) Parameters() map[string]interface{} { var grepSchema = GrepTool{}.Schema() func (GrepTool) Execute(ctx context.Context, input json.RawMessage) (string, error) { + if err := ctx.Err(); err != nil { + return "", err + } p, err := DecodeInput[GrepInput]("Grep", input) if err != nil { return "", err @@ -67,42 +83,164 @@ func (GrepTool) Execute(ctx context.Context, input json.RawMessage) (string, err if err != nil { return "", err } + if reason := IsSensitivePath(rootAbs); reason != "" { + return "", fmt.Errorf("blocked: %s", reason) + } rootHandle, err := os.OpenRoot(rootAbs) if err != nil { return "", fmt.Errorf("open search root: %w", err) } defer func() { _ = rootHandle.Close() }() - var results []string - _ = fs.WalkDir(rootHandle.FS(), ".", func(path string, d fs.DirEntry, err error) error { - if err != nil || d.IsDir() { - if d != nil && d.IsDir() && (d.Name() == ".git" || d.Name() == "node_modules") { + results := grepResults{} + skipped := map[string]int{} + err = fs.WalkDir(rootHandle.FS(), ".", func(path string, d fs.DirEntry, err error) error { + if err := ctx.Err(); err != nil { + return err + } + if err != nil { + skipped["unreadable"]++ + return nil + } + fullPath := filepath.Join(rootAbs, filepath.FromSlash(path)) + if IsSensitivePath(fullPath) != "" { + skipped["sensitive"]++ + if d != nil && d.IsDir() { + return fs.SkipDir + } + return nil + } + if d.IsDir() { + if d.Name() == ".git" || d.Name() == "node_modules" { return filepath.SkipDir } return nil } + // Do not follow links or open devices, sockets, or named pipes. + if !d.Type().IsRegular() { + return nil + } if p.Include != "" { if matched, _ := filepath.Match(p.Include, d.Name()); !matched { return nil } } - data, err := fs.ReadFile(rootHandle.FS(), path) + reason, err := grepFile(ctx, rootHandle, path, fullPath, re, &results) if err != nil { - return nil + return err } - lines := strings.Split(string(data), "\n") - for i, line := range lines { - if re.MatchString(line) { - results = append(results, fmt.Sprintf("%s:%d: %s", filepath.Join(rootAbs, filepath.FromSlash(path)), i+1, line)) - if len(results) >= 200 { - return fmt.Errorf("limit") - } - } + if reason != "" { + skipped[reason]++ + } + if results.truncated { + return fs.SkipAll } return nil }) - if len(results) == 0 { - return "No matches found", nil + if err != nil { + return "", err + } + if err := ctx.Err(); err != nil { + return "", err + } + out := strings.Join(results.lines, "\n") + if len(results.lines) == 0 { + out = "No matches found" + } + if results.truncated { + out += "\n[Results truncated; narrow the pattern or use include.]" + } + var notes []string + for _, reason := range []string{"sensitive", "binary", "oversized", "unreadable", "with long lines"} { + if count := skipped[reason]; count > 0 { + notes = append(notes, fmt.Sprintf("%d %s", count, reason)) + } + } + if len(notes) > 0 { + out += "\n[Skipped files: " + strings.Join(notes, ", ") + ".]" + } + return out, nil +} + +type grepResults struct { + lines []string + bytes int + truncated bool +} + +func (r *grepResults) add(path string, line int, text string) { + entry := fmt.Sprintf("%s:%d: %s", path, line, grepPreview(text)) + if r.bytes+len(entry)+1 > maxGrepOutputBytes { + r.truncated = true + return + } + r.lines = append(r.lines, entry) + r.bytes += len(entry) + 1 + r.truncated = len(r.lines) >= maxGrepMatches +} + +func grepPreview(text string) string { + text = strings.ToValidUTF8(text, "�") + if len(text) <= maxGrepPreviewBytes { + return text + } + end := maxGrepPreviewBytes - len("…") + for end > 0 && !utf8.RuneStart(text[end]) { + end-- + } + return text[:end] + "…" +} + +func grepFile(ctx context.Context, root *os.Root, path, displayPath string, re *regexp.Regexp, results *grepResults) (string, error) { + if err := ctx.Err(); err != nil { + return "", err + } + f, err := root.Open(path) + if err != nil { + return "unreadable", nil + } + defer func() { _ = f.Close() }() + info, err := f.Stat() + if err != nil || !info.Mode().IsRegular() { + return "unreadable", nil + } + // Reject a link swapped in after the walk and sensitive-path checks. + entry, err := root.Lstat(path) + if err != nil || !os.SameFile(info, entry) { + return "unreadable", nil + } + if info.Size() > maxFileSize { + return "oversized", nil + } + // Bound the read even if the file grows after Stat. + data, err := io.ReadAll(io.LimitReader(f, maxFileSize+1)) + if ctxErr := ctx.Err(); ctxErr != nil { + return "", ctxErr + } + if err != nil { + return "unreadable", nil + } + if len(data) > maxFileSize { + return "oversized", nil + } + if IsBinaryContent(data) { + return "binary", nil + } + scanner := bufio.NewScanner(bytes.NewReader(data)) + scanner.Buffer(make([]byte, 4096), maxGrepLineBytes) + for line := 1; scanner.Scan(); line++ { + if err := ctx.Err(); err != nil { + return "", err + } + if re.Match(scanner.Bytes()) { + results.add(displayPath, line, scanner.Text()) + if results.truncated { + return "", nil + } + } + } + if scanner.Err() != nil { + return "with long lines", nil } - return strings.Join(results, "\n"), nil + return "", nil } diff --git a/internal/tool/grep_test.go b/internal/tool/grep_test.go new file mode 100644 index 000000000..5ac678ab6 --- /dev/null +++ b/internal/tool/grep_test.go @@ -0,0 +1,142 @@ +package tool + +import ( + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "unicode/utf8" +) + +func runGrepFixture(t *testing.T, ctx context.Context, dir, pattern string) (string, error) { + t.Helper() + input, err := json.Marshal(GrepInput{Path: dir, Pattern: pattern}) + if err != nil { + t.Fatal(err) + } + return (GrepTool{}).Execute(WithToolContext(ctx, &ToolContext{WorkingDir: dir}), input) +} + +func TestGrepExcludesSensitiveFilesAndLinks(t *testing.T) { + dir := t.TempDir() + for _, name := range []string{".env", ".env.production", "credentials.json", "id_rsa"} { + if err := os.WriteFile(filepath.Join(dir, name), []byte("needle-secret"), 0o600); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(dir, "source.go"), []byte("package main\n// needle-source\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(dir, ".env"), filepath.Join(dir, "readme.txt")); err != nil { + t.Fatal(err) + } + out, err := runGrepFixture(t, t.Context(), dir, "needle") + if err != nil { + t.Fatal(err) + } + if strings.Contains(out, "needle-secret") || !strings.Contains(out, "source.go:2: // needle-source") { + t.Fatalf("search leaked sensitive data or lost source matches: %q", out) + } + if !strings.Contains(out, "sensitive") { + t.Fatalf("missing skipped-file notice: %q", out) + } +} + +func TestGrepSkipsBinaryOversizedAndLongLineFiles(t *testing.T) { + dir := t.TempDir() + for name, data := range map[string]string{"binary": "\x7fELF\x00needle-binary", "long.txt": strings.Repeat("x", maxGrepLineBytes) + "needle-long", "source.go": "// needle-source\n"} { + if err := os.WriteFile(filepath.Join(dir, name), []byte(data), 0o600); err != nil { + t.Fatal(err) + } + } + f, err := os.Create(filepath.Join(dir, "large.txt")) + if err != nil { + t.Fatal(err) + } + if _, err := f.WriteString("needle-large"); err != nil { + t.Fatal(err) + } + if err := f.Truncate(maxFileSize + 1); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + out, err := runGrepFixture(t, t.Context(), dir, "needle") + if err != nil { + t.Fatal(err) + } + for _, unexpected := range []string{"needle-binary", "needle-large", "needle-long"} { + if strings.Contains(out, unexpected) { + t.Errorf("returned skipped file contents: %q", out) + } + } + for _, want := range []string{"needle-source", "1 binary", "1 oversized", "1 with long lines"} { + if !strings.Contains(out, want) { + t.Errorf("missing %q in %q", want, out) + } + } +} + +func TestGrepCancellation(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "source.go"), []byte(strings.Repeat("needle\n", 100)), 0o600); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(t.Context()) + cancel() + if out, err := runGrepFixture(t, ctx, dir, "needle"); !errors.Is(err, context.Canceled) || out != "" { + t.Fatalf("cancelled search returned %q, %v", out, err) + } + // Cancel once a scan is underway, rather than only before Execute. + ctx, cancel = context.WithCancel(t.Context()) + defer cancel() + counted := &grepCancelContext{Context: ctx, cancel: cancel, remaining: 8} + if out, err := runGrepFixture(t, counted, dir, "needle"); !errors.Is(err, context.Canceled) || out != "" { + t.Fatalf("search ignored cancellation during scan: %q, %v", out, err) + } +} + +type grepCancelContext struct { + context.Context + cancel context.CancelFunc + remaining int +} + +func (c *grepCancelContext) Err() error { + c.remaining-- + if c.remaining == 0 { + c.cancel() + } + return c.Context.Err() +} + +func TestGrepBoundsMatchCountAndOutput(t *testing.T) { + for _, tc := range []struct { + name, text string + want int + }{ + {name: "match count", text: strings.Repeat("needle\n", maxGrepMatches+10), want: maxGrepMatches}, + {name: "output bytes", text: strings.Repeat("needle "+strings.Repeat("界", 2000)+"\n", 20)}, + } { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "source.go"), []byte(tc.text), 0o600); err != nil { + t.Fatal(err) + } + out, err := runGrepFixture(t, t.Context(), dir, "needle") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "Results truncated") || len(out) > maxGrepOutputBytes+256 || !utf8.ValidString(out) { + t.Fatalf("unbounded or invalid output: bytes=%d, tail=%q", len(out), out[max(0, len(out)-100):]) + } + if tc.want > 0 && strings.Count(out, "needle") != tc.want { + t.Fatalf("matches=%d, want %d", strings.Count(out, "needle"), tc.want) + } + }) + } +} diff --git a/internal/tool/shell_safety.go b/internal/tool/shell_safety.go index 8b9ee1b0d..bbe28cb76 100644 --- a/internal/tool/shell_safety.go +++ b/internal/tool/shell_safety.go @@ -96,7 +96,7 @@ func validateShellCommand(ctx context.Context, command string) error { if procEnvironRe.MatchString(command) { return fmt.Errorf("blocked: /proc/*/environ access can expose environment variables") } - if envDumpRe.MatchString(command) { + if hasBlockedEnvironmentDump(command) { return fmt.Errorf("blocked: dumping environment variables can expose API keys") } if reflectEnvReadRe.MatchString(command) { @@ -129,6 +129,45 @@ func validateShellCommand(ctx context.Context, command string) error { return nil } +// A named Go diagnostic is not an environment dump. Keep the exception +// literal and segment-local so a second command or expansion cannot use it +// to bypass the environment protection. Proxy and authentication settings +// are deliberately excluded because their values can contain credentials. +func hasBlockedEnvironmentDump(command string) bool { + for _, segment := range splitShellSegments(command) { + if envDumpRe.MatchString(segment) && !isNamedGoDiagnostic(segment) { + return true + } + } + return false +} + +func isNamedGoDiagnostic(command string) bool { + if strings.ContainsAny(command, "$`\\()<>") { + return false + } + args := tokenizeShellWords(command) + if len(args) < 3 || commandBase(args[0]) != "go" || args[1] != "env" { + return false + } + args = args[2:] + if args[0] == "-json" { + args = args[1:] + } + if len(args) == 0 { + return false + } + for _, name := range args { + switch name { + case "GOTOOLCHAIN", "GOFLAGS", "GOROOT", "GOPATH", "GOMOD", "GOWORK", "GOVERSION", + "GOOS", "GOARCH", "GOHOSTOS", "GOHOSTARCH", "GOCACHE", "GOMODCACHE", "GOBIN", "CGO_ENABLED": + default: + return false + } + } + return true +} + // validateTerminalInput applies the destructive-command subset of the shell // safety stack to input sent to a persistent terminal. It deliberately omits // the interactive-hostile checks (carriage return, quote obfuscation) that diff --git a/internal/tool/shell_safety_test.go b/internal/tool/shell_safety_test.go new file mode 100644 index 000000000..20b14f664 --- /dev/null +++ b/internal/tool/shell_safety_test.go @@ -0,0 +1,34 @@ +package tool + +import "testing" + +func TestShellSafetyAllowsNamedGoDiagnostics(t *testing.T) { + for _, command := range []string{ + "go env GOTOOLCHAIN", + "go env GOTOOLCHAIN GOFLAGS GOROOT", + "go env -json GOPATH GOMOD GOWORK", + "cd /tmp && go env GOCACHE; go version", + "export PATH=\"$HOME/.local/go/bin:$PATH\"; cd /tmp && go env GOTOOLCHAIN", + } { + t.Run(command, func(t *testing.T) { + if err := validateShellCommand(t.Context(), command); err != nil { + t.Fatalf("safe diagnostic blocked: %v", err) + } + }) + } +} + +func TestShellSafetyStillBlocksEnvironmentDumps(t *testing.T) { + for _, command := range []string{ + "env", "printenv", "go env", "go env -json", "go env GOPROXY", "go env GOAUTH", + "go env -w GOFLAGS=-x", "go env -u GOFLAGS", "go env GOTOOLCHAIN && env", + "go env GOTOOLCHAIN | printenv", "go env GOTOOLCHAIN; printenv OPENAI_API_KEY", + "go env GOTOOLCHAIN $(printenv)", "go env GOTOOLCHAIN\nprintenv", "echo 'go env GOTOOLCHAIN'", + } { + t.Run(command, func(t *testing.T) { + if err := validateShellCommand(t.Context(), command); err == nil { + t.Fatal("environment dump protection was bypassed") + } + }) + } +}