fix: exec guard treats agent CLI slash commands as non-paths
Agent CLI tools (claude, codex, gemini) use slash commands like "/review" that look like absolute paths. Instead of generic quote handling, detect these tools specifically and check os.Stat before blocking — if the path does not exist, it is a slash command. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
16b4ab6262
commit
fc4ff76a9e
2 changed files with 46 additions and 41 deletions
|
|
@ -291,11 +291,17 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
}
|
}
|
||||||
|
|
||||||
// Token-based absolute path detection.
|
// Token-based absolute path detection.
|
||||||
// Uses shellTokenize to respect quoted strings (e.g., "/review ..."
|
// Uses strings.Fields so relative paths (e.g., "tests/cold/file.py")
|
||||||
// is a single argument, not a file path).
|
// are not falsely flagged.
|
||||||
// Flags like -I/usr/local/include are naturally skipped because
|
// Flags like -I/usr/local/include are naturally skipped because
|
||||||
// filepath.IsAbs returns false for tokens starting with "-".
|
// filepath.IsAbs returns false for tokens starting with "-".
|
||||||
for _, token := range shellTokenize(cmd) {
|
//
|
||||||
|
// Agent CLI tools (claude, codex, gemini) accept slash commands
|
||||||
|
// (e.g., "/review") that look like absolute paths but are not.
|
||||||
|
// For these tools we check whether the token is an existing path
|
||||||
|
// before blocking.
|
||||||
|
agentCLI := isAgentCLICommand(cmd)
|
||||||
|
for _, token := range strings.Fields(cmd) {
|
||||||
token = strings.Trim(token, "\"'")
|
token = strings.Trim(token, "\"'")
|
||||||
|
|
||||||
if !filepath.IsAbs(token) {
|
if !filepath.IsAbs(token) {
|
||||||
|
|
@ -313,6 +319,13 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
if isExecutable(p) {
|
if isExecutable(p) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
// Agent CLI slash commands: skip non-existent paths
|
||||||
|
// (e.g., "/review" is a command, not a file).
|
||||||
|
if agentCLI {
|
||||||
|
if _, statErr := os.Stat(p); os.IsNotExist(statErr) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
}
|
||||||
return "Command blocked by safety guard (path outside working dir)"
|
return "Command blocked by safety guard (path outside working dir)"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -321,40 +334,23 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
// shellTokenize splits a command string into tokens while respecting
|
// agentCLINames lists agent CLI tools that use slash commands
|
||||||
// single and double quotes. Quoted substrings are returned as a single
|
// (e.g., "/review", "/help") which look like absolute paths.
|
||||||
// token (with the quotes still attached so the caller can trim them).
|
var agentCLINames = []string{"claude", "codex", "gemini"}
|
||||||
// This prevents false positives where a quoted argument like
|
|
||||||
// "/review skip-git-repo-check" would be split into "/review" and
|
// isAgentCLICommand returns true if the command invokes an agent CLI tool.
|
||||||
// "skip-git-repo-check" by strings.Fields.
|
func isAgentCLICommand(cmd string) bool {
|
||||||
func shellTokenize(s string) []string {
|
fields := strings.Fields(cmd)
|
||||||
var tokens []string
|
if len(fields) == 0 {
|
||||||
var cur strings.Builder
|
return false
|
||||||
var quote byte // 0 = none, '\'' or '"'
|
}
|
||||||
for i := 0; i < len(s); i++ {
|
base := filepath.Base(fields[0])
|
||||||
ch := s[i]
|
for _, name := range agentCLINames {
|
||||||
switch {
|
if base == name {
|
||||||
case quote != 0:
|
return true
|
||||||
cur.WriteByte(ch)
|
|
||||||
if ch == quote {
|
|
||||||
quote = 0
|
|
||||||
}
|
|
||||||
case ch == '\'' || ch == '"':
|
|
||||||
cur.WriteByte(ch)
|
|
||||||
quote = ch
|
|
||||||
case ch == ' ' || ch == '\t':
|
|
||||||
if cur.Len() > 0 {
|
|
||||||
tokens = append(tokens, cur.String())
|
|
||||||
cur.Reset()
|
|
||||||
}
|
|
||||||
default:
|
|
||||||
cur.WriteByte(ch)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if cur.Len() > 0 {
|
return false
|
||||||
tokens = append(tokens, cur.String())
|
|
||||||
}
|
|
||||||
return tokens
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// isExecutable checks if a path points to an executable file.
|
// isExecutable checks if a path points to an executable file.
|
||||||
|
|
|
||||||
|
|
@ -487,21 +487,30 @@ func TestGuardCommand_CdWithAbsoluteWorkspacePath(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestGuardCommand_QuotedSlashArgNotBlocked(t *testing.T) {
|
func TestGuardCommand_AgentCLISlashCommand(t *testing.T) {
|
||||||
workspace := t.TempDir()
|
workspace := t.TempDir()
|
||||||
tool := NewExecTool(workspace, true)
|
tool := NewExecTool(workspace, true)
|
||||||
|
|
||||||
// A quoted argument starting with "/" is not a file path — it's a
|
// Agent CLI slash commands (e.g., "/review") are not file paths.
|
||||||
// command argument that happens to contain a slash.
|
// They should be allowed because they don't exist on disk.
|
||||||
cmds := []string{
|
cmds := []string{
|
||||||
`codex exec --yolo "/review skip-git-repo-check"`,
|
`codex exec --yolo "/review skip-git-repo-check"`,
|
||||||
`echo '/hello world'`,
|
`claude "/review"`,
|
||||||
`grep "/etc/passwd" file.txt`,
|
`gemini "/help"`,
|
||||||
}
|
}
|
||||||
for _, cmd := range cmds {
|
for _, cmd := range cmds {
|
||||||
result := tool.guardCommand(cmd, workspace)
|
result := tool.guardCommand(cmd, workspace)
|
||||||
if result != "" {
|
if result != "" {
|
||||||
t.Errorf("Quoted argument should not be blocked: %q → %s", cmd, result)
|
t.Errorf("Agent CLI slash command should not be blocked: %q → %s", cmd, result)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Non-agent commands with absolute paths should still be blocked.
|
||||||
|
if runtime.GOOS != "windows" {
|
||||||
|
blocked := `cat /etc/hosts`
|
||||||
|
result := tool.guardCommand(blocked, workspace)
|
||||||
|
if result == "" {
|
||||||
|
t.Errorf("Non-agent command with absolute path should be blocked: %q", blocked)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue