fix: tighten exec url workspace checks
This commit is contained in:
parent
456a302b36
commit
dc9da009b6
2 changed files with 107 additions and 7 deletions
|
|
@ -79,10 +79,6 @@ var (
|
||||||
// absolutePathPattern matches absolute file paths in commands (Unix and Windows).
|
// absolutePathPattern matches absolute file paths in commands (Unix and Windows).
|
||||||
absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`)
|
absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`)
|
||||||
|
|
||||||
// httpURLPattern matches HTTP(S) URLs so they can be excluded from path-based
|
|
||||||
// workspace checks. URLs are command arguments, not local filesystem paths.
|
|
||||||
httpURLPattern = regexp.MustCompile(`(?i)\bhttps?://[^\s"'<>]+`)
|
|
||||||
|
|
||||||
// safePaths are kernel pseudo-devices that are always safe to reference in
|
// safePaths are kernel pseudo-devices that are always safe to reference in
|
||||||
// commands, regardless of workspace restriction. They contain no user data
|
// commands, regardless of workspace restriction. They contain no user data
|
||||||
// and cannot cause destructive writes.
|
// and cannot cause destructive writes.
|
||||||
|
|
@ -369,9 +365,89 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
}
|
}
|
||||||
|
|
||||||
func stripHTTPURLs(command string) string {
|
func stripHTTPURLs(command string) string {
|
||||||
return httpURLPattern.ReplaceAllStringFunc(command, func(match string) string {
|
sanitized := []byte(command)
|
||||||
return strings.Repeat(" ", len(match))
|
quote := byte(0)
|
||||||
})
|
|
||||||
|
for i := 0; i < len(command); i++ {
|
||||||
|
ch := command[i]
|
||||||
|
|
||||||
|
switch quote {
|
||||||
|
case '\'':
|
||||||
|
if ch == '\'' {
|
||||||
|
quote = 0
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
case '"':
|
||||||
|
if ch == '\\' && i+1 < len(command) {
|
||||||
|
i++
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if ch == '"' {
|
||||||
|
quote = 0
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
default:
|
||||||
|
if ch == '\\' && i+1 < len(command) {
|
||||||
|
i++
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if ch == '\'' || ch == '"' {
|
||||||
|
quote = ch
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if !hasHTTPURLPrefix(command, i) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
end := findHTTPURLEnd(command, i, quote)
|
||||||
|
for j := i; j < end; j++ {
|
||||||
|
sanitized[j] = ' '
|
||||||
|
}
|
||||||
|
i = end - 1
|
||||||
|
}
|
||||||
|
|
||||||
|
return string(sanitized)
|
||||||
|
}
|
||||||
|
|
||||||
|
func hasHTTPURLPrefix(command string, start int) bool {
|
||||||
|
return len(command)-start >= len("http://") &&
|
||||||
|
(strings.EqualFold(command[start:start+len("http://")], "http://") ||
|
||||||
|
(len(command)-start >= len("https://") &&
|
||||||
|
strings.EqualFold(command[start:start+len("https://")], "https://")))
|
||||||
|
}
|
||||||
|
|
||||||
|
func findHTTPURLEnd(command string, start int, quote byte) int {
|
||||||
|
if quote != 0 {
|
||||||
|
for i := start; i < len(command); i++ {
|
||||||
|
if quote == '"' && command[i] == '\\' && i+1 < len(command) {
|
||||||
|
i++
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if command[i] == quote {
|
||||||
|
return i
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return len(command)
|
||||||
|
}
|
||||||
|
|
||||||
|
for i := start; i < len(command); i++ {
|
||||||
|
if strings.ContainsRune(" \t\r\n", rune(command[i])) || isShellURLDelimiter(command[i]) {
|
||||||
|
return i
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return len(command)
|
||||||
|
}
|
||||||
|
|
||||||
|
func isShellURLDelimiter(ch byte) bool {
|
||||||
|
switch ch {
|
||||||
|
case ';', '|', '&', '<', '>', '(', ')', '`':
|
||||||
|
return true
|
||||||
|
default:
|
||||||
|
return false
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (t *ExecTool) SetTimeout(timeout time.Duration) {
|
func (t *ExecTool) SetTimeout(timeout time.Duration) {
|
||||||
|
|
|
||||||
|
|
@ -325,6 +325,8 @@ func TestShellTool_RestrictToWorkspace_AllowsHTTPURLs(t *testing.T) {
|
||||||
`echo "http://example.com"`,
|
`echo "http://example.com"`,
|
||||||
`echo "https://test"`,
|
`echo "https://test"`,
|
||||||
`echo "https://example.com/path/../still-a-url"`,
|
`echo "https://example.com/path/../still-a-url"`,
|
||||||
|
`echo "https://example.com?a=1&b=/still-a-url"`,
|
||||||
|
`echo 'https://example.com;/bin/sh'`,
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, cmd := range commands {
|
for _, cmd := range commands {
|
||||||
|
|
@ -335,6 +337,28 @@ func TestShellTool_RestrictToWorkspace_AllowsHTTPURLs(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestShellTool_RestrictToWorkspace_URLDoesNotBypassPathChecks verifies that
|
||||||
|
// URLs do not hide real paths that should still be blocked by the workspace guard.
|
||||||
|
func TestShellTool_RestrictToWorkspace_URLDoesNotBypassPathChecks(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
tool, err := NewExecTool(tmpDir, true)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("unable to configure exec tool: %s", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
commands := []string{
|
||||||
|
`echo https://example.com /etc/passwd`,
|
||||||
|
`echo https://x;/bin/sh -c whoami`,
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, cmd := range commands {
|
||||||
|
result := tool.Execute(context.Background(), map[string]any{"command": cmd})
|
||||||
|
if !result.IsError || !strings.Contains(result.ForLLM, "blocked") {
|
||||||
|
t.Fatalf("expected command to stay blocked, command=%q output=%s", cmd, result.ForLLM)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestShellTool_DevNullAllowed verifies that /dev/null redirections are not blocked (issue #964).
|
// TestShellTool_DevNullAllowed verifies that /dev/null redirections are not blocked (issue #964).
|
||||||
func TestShellTool_DevNullAllowed(t *testing.T) {
|
func TestShellTool_DevNullAllowed(t *testing.T) {
|
||||||
tmpDir := t.TempDir()
|
tmpDir := t.TempDir()
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue