fix(exec): allow quoted heredoc markdown bodies
This commit is contained in:
parent
223ebdf0c7
commit
deab3ab09d
2 changed files with 95 additions and 1 deletions
|
|
@ -112,8 +112,70 @@ var (
|
||||||
"/dev/stdout": true,
|
"/dev/stdout": true,
|
||||||
"/dev/stderr": true,
|
"/dev/stderr": true,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
quotedHeredocStartPattern = regexp.MustCompile(`<<-?\s*(?:'([^'\s]+)'|"([^"\s]+)")`)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
func stripQuotedHeredocBodies(command string) string {
|
||||||
|
sanitized := command
|
||||||
|
searchFrom := 0
|
||||||
|
|
||||||
|
for {
|
||||||
|
loc := quotedHeredocStartPattern.FindStringSubmatchIndex(sanitized[searchFrom:])
|
||||||
|
if loc == nil {
|
||||||
|
return sanitized
|
||||||
|
}
|
||||||
|
|
||||||
|
matchStart := searchFrom + loc[0]
|
||||||
|
matchEnd := searchFrom + loc[1]
|
||||||
|
delimStart := searchFrom + loc[2]
|
||||||
|
delimEnd := searchFrom + loc[3]
|
||||||
|
if delimStart == searchFrom-1 || delimEnd == searchFrom-1 {
|
||||||
|
delimStart = searchFrom + loc[4]
|
||||||
|
delimEnd = searchFrom + loc[5]
|
||||||
|
}
|
||||||
|
delim := sanitized[delimStart:delimEnd]
|
||||||
|
allowTabs := strings.Contains(sanitized[matchStart:matchEnd], "<<-")
|
||||||
|
|
||||||
|
newlineRel := strings.IndexByte(sanitized[matchEnd:], '\n')
|
||||||
|
if newlineRel == -1 {
|
||||||
|
return sanitized
|
||||||
|
}
|
||||||
|
bodyStart := matchEnd + newlineRel + 1
|
||||||
|
lineStart := bodyStart
|
||||||
|
foundEnd := false
|
||||||
|
|
||||||
|
for lineStart <= len(sanitized) {
|
||||||
|
lineEnd := len(sanitized)
|
||||||
|
if nextNewline := strings.IndexByte(sanitized[lineStart:], '\n'); nextNewline >= 0 {
|
||||||
|
lineEnd = lineStart + nextNewline
|
||||||
|
}
|
||||||
|
|
||||||
|
line := sanitized[lineStart:lineEnd]
|
||||||
|
compare := strings.TrimSuffix(line, "\r")
|
||||||
|
if allowTabs {
|
||||||
|
compare = strings.TrimLeft(compare, "\t")
|
||||||
|
}
|
||||||
|
if compare == delim {
|
||||||
|
sanitized = sanitized[:bodyStart] + "[quoted heredoc omitted]\n" + sanitized[lineStart:]
|
||||||
|
searchFrom = bodyStart + len("[quoted heredoc omitted]\n")
|
||||||
|
foundEnd = true
|
||||||
|
break
|
||||||
|
}
|
||||||
|
|
||||||
|
if lineEnd == len(sanitized) {
|
||||||
|
lineStart = len(sanitized) + 1
|
||||||
|
} else {
|
||||||
|
lineStart = lineEnd + 1
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if !foundEnd {
|
||||||
|
return sanitized
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func NewExecTool(workingDir string, restrict bool, allowPaths ...[]*regexp.Regexp) (*ExecTool, error) {
|
func NewExecTool(workingDir string, restrict bool, allowPaths ...[]*regexp.Regexp) (*ExecTool, error) {
|
||||||
return NewExecToolWithConfig(workingDir, restrict, nil, allowPaths...)
|
return NewExecToolWithConfig(workingDir, restrict, nil, allowPaths...)
|
||||||
}
|
}
|
||||||
|
|
@ -1022,6 +1084,7 @@ func (t *ExecTool) executeSendKeys(args map[string]any) *ToolResult {
|
||||||
func (t *ExecTool) guardCommand(command, cwd string) string {
|
func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
cmd := strings.TrimSpace(command)
|
cmd := strings.TrimSpace(command)
|
||||||
lower := strings.ToLower(cmd)
|
lower := strings.ToLower(cmd)
|
||||||
|
lowerForDeny := strings.ToLower(stripQuotedHeredocBodies(cmd))
|
||||||
|
|
||||||
// Custom allow patterns exempt a command from deny checks.
|
// Custom allow patterns exempt a command from deny checks.
|
||||||
explicitlyAllowed := false
|
explicitlyAllowed := false
|
||||||
|
|
@ -1034,7 +1097,7 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
|
|
||||||
if !explicitlyAllowed {
|
if !explicitlyAllowed {
|
||||||
for _, pattern := range t.denyPatterns {
|
for _, pattern := range t.denyPatterns {
|
||||||
if pattern.MatchString(lower) {
|
if pattern.MatchString(lowerForDeny) {
|
||||||
return "Command blocked by safety guard (dangerous pattern detected)"
|
return "Command blocked by safety guard (dangerous pattern detected)"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -181,6 +181,37 @@ func TestShellTool_DangerousCommand_KillBlocked(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestShellTool_BackticksInsideQuotedHeredocAreAllowed(t *testing.T) {
|
||||||
|
tool, err := NewExecTool("", false)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
command := "gh pr comment 2763 --body-file - <<'TXT'\n" +
|
||||||
|
"Fixed `pkg/tools/integration/web_test.go` and `brave` is now expected.\n" +
|
||||||
|
"TXT"
|
||||||
|
|
||||||
|
guardError := tool.guardCommand(command, "")
|
||||||
|
if guardError != "" {
|
||||||
|
t.Fatalf("quoted heredoc body should not be blocked, got: %s", guardError)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestShellTool_BackticksOutsideQuotedHeredocRemainBlocked(t *testing.T) {
|
||||||
|
tool, err := NewExecTool("", false)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
result := tool.Execute(context.Background(), map[string]any{
|
||||||
|
"action": "run",
|
||||||
|
"command": "echo `whoami`",
|
||||||
|
})
|
||||||
|
|
||||||
|
if !result.IsError {
|
||||||
|
t.Fatal("expected raw backtick command substitution to be blocked")
|
||||||
|
}
|
||||||
|
if !strings.Contains(result.ForLLM, "blocked") {
|
||||||
|
t.Fatalf("expected blocked message, got: %s", result.ForLLM)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestShellTool_MissingCommand verifies error handling for missing command
|
// TestShellTool_MissingCommand verifies error handling for missing command
|
||||||
func TestShellTool_MissingCommand(t *testing.T) {
|
func TestShellTool_MissingCommand(t *testing.T) {
|
||||||
tool, err := NewExecTool("", false)
|
tool, err := NewExecTool("", false)
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue