fix(safety-guard): use exact match position to prevent URL exemption bypass
Using strings.Index(cmd, raw) always returned the first occurrence of the matched substring, allowing a bypass where the same //path appeared both inside a URL and as a standalone shell path (e.g. echo https://etc/passwd && cat //etc/passwd would skip the second match). Switch to FindAllStringIndex so each match is evaluated at its actual position in the command string. Adds TestShellTool_URLBypassPrevented to cover the exploit scenario.
This commit is contained in:
parent
21cd92efe9
commit
9182e7197a
2 changed files with 49 additions and 24 deletions
|
|
@ -336,36 +336,35 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
|||
return ""
|
||||
}
|
||||
|
||||
matches := absolutePathPattern.FindAllString(cmd, -1)
|
||||
// Web URL schemes whose path components (starting with //) should be exempt
|
||||
// from workspace sandbox checks. file: is intentionally excluded so that
|
||||
// file:// URIs are still validated against the workspace boundary.
|
||||
webSchemes := []string{"http:", "https:", "ftp:", "ftps:", "sftp:", "ssh:", "git:"}
|
||||
|
||||
matchIndices := absolutePathPattern.FindAllStringIndex(cmd, -1)
|
||||
|
||||
for _, loc := range matchIndices {
|
||||
raw := cmd[loc[0]:loc[1]]
|
||||
|
||||
for _, raw := range matches {
|
||||
// Skip URL path components that look like they're from web URLs.
|
||||
// When a URL like "https://github.com" is parsed, the regex captures
|
||||
// "//github.com" as a match (the path portion after "https:").
|
||||
// These double-slash prefixes indicate URL paths, not file system paths.
|
||||
// However, we must NOT skip file:// URIs as they could escape the sandbox.
|
||||
// Only skip if preceded by a web URL scheme (http:, https:, ftp:, etc.).
|
||||
if strings.HasPrefix(raw, "//") {
|
||||
// Check if this // path is preceded by a web URL scheme
|
||||
// by looking for patterns like "http://", "https://", "ftp://" before the match
|
||||
idx := strings.Index(cmd, raw)
|
||||
if idx > 0 {
|
||||
// Look for the scheme prefix (e.g., "https:") before the //
|
||||
before := cmd[:idx]
|
||||
// Check if it ends with a web URL scheme followed by colon
|
||||
// Web schemes: http, https, ftp, ftps, sftp, ssh, git
|
||||
webSchemes := []string{"http:", "https:", "ftp:", "ftps:", "sftp:", "ssh:", "git:"}
|
||||
isWebURL := false
|
||||
for _, scheme := range webSchemes {
|
||||
if strings.HasSuffix(before, scheme) {
|
||||
isWebURL = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if isWebURL {
|
||||
continue
|
||||
// Use the exact match position (loc[0]) so that duplicate //path substrings
|
||||
// in the same command are each evaluated at their own position.
|
||||
if strings.HasPrefix(raw, "//") && loc[0] > 0 {
|
||||
before := cmd[:loc[0]]
|
||||
isWebURL := false
|
||||
|
||||
for _, scheme := range webSchemes {
|
||||
if strings.HasSuffix(before, scheme) {
|
||||
isWebURL = true
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if isWebURL {
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
p, err := filepath.Abs(raw)
|
||||
|
|
|
|||
|
|
@ -515,3 +515,29 @@ func TestShellTool_FileURISandboxing(t *testing.T) {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestShellTool_URLBypassPrevented verifies that a command cannot bypass the workspace
|
||||
// sandbox by smuggling a real path after a URL that contains the same //path substring.
|
||||
// e.g. "echo https://etc/passwd && cat //etc/passwd" must still be blocked.
|
||||
func TestShellTool_URLBypassPrevented(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
tool, err := NewExecTool(tmpDir, true)
|
||||
if err != nil {
|
||||
t.Fatalf("unable to configure exec tool: %s", err)
|
||||
}
|
||||
|
||||
// The path //etc/passwd appears twice: once as the host part of an https URL
|
||||
// and once as a real (escaped) absolute path. The guard must block the command
|
||||
// because the second occurrence is a genuine out-of-workspace path.
|
||||
blockedCommands := []string{
|
||||
"echo https://etc/passwd && cat //etc/passwd",
|
||||
"curl https://host/file && ls //etc",
|
||||
}
|
||||
|
||||
for _, cmd := range blockedCommands {
|
||||
result := tool.Execute(context.Background(), map[string]any{"command": cmd})
|
||||
if !result.IsError || !strings.Contains(result.ForLLM, "path outside working dir") {
|
||||
t.Errorf("bypass attempt should be blocked: %q\n got: %s", cmd, result.ForLLM)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue