fix(tools): exec guard must not treat relative paths as root-absolute
The path scanner used /[^\s"']+ anywhere in the command string, so archive/SKILL.md matched /SKILL.md, Abs resolved it to the filesystem root, and Rel produced .. chains that tripped the workspace check. Require Unix-looking absolutes to follow a shell delimiter (including / so file:/// paths still match). Parse with FindAllStringSubmatchIndex. Extend web URL exemption: after adding / as a delimiter, https://host can match /host with a single leading slash; detect via scheme suffix on the trimmed prefix before the match. Add TestShellTool_RelativePathWithSlashNotTreatedAsAbsolute. Made-with: Cursor
This commit is contained in:
parent
a94ba82181
commit
aa006db59e
2 changed files with 53 additions and 17 deletions
|
|
@ -98,7 +98,13 @@ var (
|
|||
}
|
||||
|
||||
// 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
|
||||
// commands, regardless of workspace restriction. They contain no user data
|
||||
|
|
@ -1063,32 +1069,36 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
|||
return ""
|
||||
}
|
||||
|
||||
// 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.
|
||||
// Web URL schemes: path segments after the scheme should be exempt from workspace
|
||||
// sandbox checks. file: is intentionally excluded so file:// URIs are still validated.
|
||||
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 {
|
||||
raw := cmd[loc[0]:loc[1]]
|
||||
|
||||
// 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:").
|
||||
// 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]]
|
||||
// Skip path-like substrings that belong to a web URL (e.g. "//host", "/user/repo",
|
||||
// or "/api/..." after "https://"). Trim trailing slashes so "https://" → "https:" for
|
||||
// HasSuffix(scheme) checks. file:/// leaves before as "file:" which is not in webSchemes.
|
||||
if pathStart > 0 {
|
||||
before := strings.TrimSpace(strings.TrimRight(cmd[:pathStart], "/"))
|
||||
isWebURL := false
|
||||
|
||||
for _, scheme := range webSchemes {
|
||||
if strings.HasSuffix(before, scheme) {
|
||||
isWebURL = true
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if isWebURL {
|
||||
continue
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
func TestShellTool_RestrictToWorkspace(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue