fix(sandbox): Windows-appropriate suggestions when blocking interactive commands#414
Conversation
…ractive commands DetectInteractiveCommand already receives goos, but its remediation text for pagers (less/more/most), process monitors (top/htop/btop/ btm), and `tail -f` hardcoded POSIX-only alternatives (cat, head, tail -n N, ps aux) regardless of platform. On Windows none of those exist either, so blocking e.g. `type file | more` correctly stopped the interactive command but then pointed the model at three more commands cmd.exe doesn't have — directly undermining the guidance this check exists to give. Reported in the wild (screenshot showing `more` blocked with a "use cat, head, or tail -n N" suggestion on a Windows session). Adds an optional windowsSuggestion per program/segment, used in place of the POSIX suggestion when goos == "windows" — `type` for pagers, `tasklist` for process monitors, the read_file tool for tail -f. Non-Windows suggestions are unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
WalkthroughThis PR adds platform-aware remediation text to the interactive command blocker. ChangesWindows-specific interactive command suggestions
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant DetectInteractiveCommand
participant interactivePrograms
participant interactiveSegments
Caller->>DetectInteractiveCommand: command string, goos
DetectInteractiveCommand->>interactiveSegments: match segment
alt segment matched
interactiveSegments-->>DetectInteractiveCommand: suggestion, windowsSuggestion
else program matched
DetectInteractiveCommand->>interactivePrograms: match program
interactivePrograms-->>DetectInteractiveCommand: suggestion, windowsSuggestion
end
DetectInteractiveCommand->>DetectInteractiveCommand: select windowsSuggestion if goos=="windows" and non-empty
DetectInteractiveCommand-->>Caller: InteractiveCommandResult.Suggestion
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/sandbox/safe_command_test.go (1)
91-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend Windows suggestion test coverage.
The table exercises
more,less,htop, andtail -f, butmost,top,btop, andbtmalso gained newwindowsSuggestionoverrides insafe_command.goand aren't verified here.✅ Suggested additional test cases
{command: "more file.txt", avoid: []string{"cat", "head", "tail"}, want: "type"}, {command: "less file.txt", avoid: []string{"cat", "head", "tail"}, want: "type"}, + {command: "most file.txt", avoid: []string{"cat", "head", "tail"}, want: "type"}, {command: "htop", avoid: []string{"ps aux"}, want: "tasklist"}, + {command: "top", avoid: []string{"ps aux"}, want: "tasklist"}, + {command: "btop", avoid: []string{"ps aux"}, want: "tasklist"}, + {command: "btm", avoid: []string{"ps aux"}, want: "tasklist"}, {command: "tail -f app.log", avoid: []string{"tail -n"}, want: "read_file"},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sandbox/safe_command_test.go` around lines 91 - 121, `TestDetectInteractiveCommandSuggestsWindowsAlternativesOnWindows` does not cover all Windows-specific overrides added in `DetectInteractiveCommand`/`windowsSuggestion` in `safe_command.go`. Extend the existing table-driven cases to include `most`, `top`, `btop`, and `btm`, and assert their Windows suggestions mention the expected non-POSIX alternatives while still keeping the Linux behavior check unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/sandbox/safe_command_test.go`:
- Around line 91-121:
`TestDetectInteractiveCommandSuggestsWindowsAlternativesOnWindows` does not
cover all Windows-specific overrides added in
`DetectInteractiveCommand`/`windowsSuggestion` in `safe_command.go`. Extend the
existing table-driven cases to include `most`, `top`, `btop`, and `btm`, and
assert their Windows suggestions mention the expected non-POSIX alternatives
while still keeping the Linux behavior check unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 61a93c06-373a-472d-9371-1f558f7eaeac
📒 Files selected for processing (2)
internal/sandbox/safe_command.gointernal/sandbox/safe_command_test.go
Summary
DetectInteractiveCommandalready receivesgoos, but its remediation text for pagers (less/more/most), process monitors (top/htop/btop/btm), andtail -fhardcoded POSIX-only alternatives (cat,head,tail -n N,ps aux) regardless of platform. On Windows none of those exist either (Zero's bash tool executes through native cmd.exe there), so blocking e.g.type file | morecorrectly stopped the interactive command but then pointed the model at three more commands cmd.exe doesn't have — undermining the guidance this check exists to give. Reported in the wild.Adds an optional
windowsSuggestionper program/segment, used in place of the POSIX suggestion whengoos == "windows":type <file>(or theread_filetool) for pagers/tail -f,tasklistfor process monitors. Non-Windows suggestions are unchanged. Scoped to the cases where the POSIX suggestion was the sole remediation offered — leftvim/vi/etc.'s secondarysed -imention alone sinceedit_file/write_fileis already their correct primary suggestion.Linked issue
Fixes #413
Checklist
issue-approvedlabel.go build ./...,go vet ./..., andgo test ./...pass locally.gofmtclean.-racewhere relevant).Verification notes
go build ./.../go vet ./...clean.TestDetectInteractiveCommandSuggestsWindowsAlternativesOnWindows(internal/sandbox/safe_command_test.go), coveringmore/less→type,htop→tasklist,tail -f→read_file, and confirming Linux keeps the original POSIX suggestion unchanged.go test ./internal/sandbox/...passes in full, including all pre-existing interactive-command tests.gofmt -lon the touched files: pre-existing repo-wide CRLF flag (core.autocrlf=trueon Windows), not introduced by this change — same as prior PRs against this repo.Summary by CodeRabbit
New Features
Bug Fixes
Tests