This commit is contained in:
Chris Crawford 2026-05-15 19:02:10 +02:00 committed by GitHub
commit 9e2c5987ae
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 53 additions and 17 deletions

View file

@ -98,7 +98,13 @@ 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\"']+`) // Unix: require "/" to follow start or a shell delimiter so "archive/SKILL.md" does not
// match "/SKILL.md" (which Abs would treat as under filesystem root).
absolutePathPattern = regexp.MustCompile(
// Include "/" as a delimiter so file:///etc/passwd matches "/etc/passwd" (third slash)
// while archive/SKILL.md still does not (slash is preceded by "e", not a delimiter).
`(?:([A-Za-z]:\\[^\\\"']+)|(?:^|[\s"'=><|;&(\[{/])(/[^\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
@ -1063,32 +1069,36 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
return "" return ""
} }
// Web URL schemes whose path components (starting with //) should be exempt // Web URL schemes: path segments after the scheme should be exempt from workspace
// from workspace sandbox checks. file: is intentionally excluded so that // sandbox checks. file: is intentionally excluded so file:// URIs are still validated.
// file:// URIs are still validated against the workspace boundary.
webSchemes := []string{"http:", "https:", "ftp:", "ftps:", "sftp:", "ssh:", "git:"} webSchemes := []string{"http:", "https:", "ftp:", "ftps:", "sftp:", "ssh:", "git:"}
matchIndices := absolutePathPattern.FindAllStringIndex(cmd, -1) for _, m := range absolutePathPattern.FindAllStringSubmatchIndex(cmd, -1) {
var raw string
var pathStart int
switch {
case m[2] >= 0 && m[3] >= 0:
raw = cmd[m[2]:m[3]]
pathStart = m[2]
case m[4] >= 0 && m[5] >= 0:
raw = cmd[m[4]:m[5]]
pathStart = m[4]
default:
continue
}
for _, loc := range matchIndices { // Skip path-like substrings that belong to a web URL (e.g. "//host", "/user/repo",
raw := cmd[loc[0]:loc[1]] // or "/api/..." after "https://"). Trim trailing slashes so "https://" → "https:" for
// HasSuffix(scheme) checks. file:/// leaves before as "file:" which is not in webSchemes.
// Skip URL path components that look like they're from web URLs. if pathStart > 0 {
// When a URL like "https://github.com" is parsed, the regex captures before := strings.TrimSpace(strings.TrimRight(cmd[:pathStart], "/"))
// "//github.com" as a match (the path portion after "https:").
// 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 isWebURL := false
for _, scheme := range webSchemes { for _, scheme := range webSchemes {
if strings.HasSuffix(before, scheme) { if strings.HasSuffix(before, scheme) {
isWebURL = true isWebURL = true
break break
} }
} }
if isWebURL { if isWebURL {
continue continue
} }

View file

@ -394,6 +394,32 @@ func TestShellTool_AllowRemoteBypassesChannelCheck(t *testing.T) {
} }
} }
// TestShellTool_RelativePathWithSlashNotTreatedAsAbsolute verifies that a relative path
// containing a slash (e.g. archive/SKILL.md) is not parsed as the absolute path /SKILL.md.
func TestShellTool_RelativePathWithSlashNotTreatedAsAbsolute(t *testing.T) {
tmpDir := t.TempDir()
archive := filepath.Join(tmpDir, "archive")
if err := os.MkdirAll(archive, 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
if err := os.WriteFile(filepath.Join(archive, "SKILL.md"), []byte("x\n"), 0o644); err != nil {
t.Fatalf("write file: %v", err)
}
tool, err := NewExecTool(tmpDir, true)
if err != nil {
t.Fatalf("NewExecTool: %v", err)
}
result := tool.Execute(context.Background(), map[string]any{
"action": "run",
"command": "wc -l archive/SKILL.md",
})
if result.IsError && strings.Contains(result.ForLLM, "path outside working dir") {
t.Fatalf("relative path wrongly treated as absolute root path: %s", result.ForLLM)
}
}
// TestShellTool_RestrictToWorkspace verifies workspace restriction // TestShellTool_RestrictToWorkspace verifies workspace restriction
func TestShellTool_RestrictToWorkspace(t *testing.T) { func TestShellTool_RestrictToWorkspace(t *testing.T) {
tmpDir := t.TempDir() tmpDir := t.TempDir()