fix(exec): keep file URI sandboxing after URL guard refactor
This commit is contained in:
parent
1c62971dc3
commit
bdef3e411f
2 changed files with 64 additions and 1 deletions
|
|
@ -5,6 +5,7 @@ import (
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"net/url"
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
|
@ -84,6 +85,9 @@ var (
|
||||||
// A separate boundary check is applied before treating a match as a filesystem path.
|
// A separate boundary check is applied before treating a match as a filesystem path.
|
||||||
absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`)
|
absolutePathPattern = regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`)
|
||||||
|
|
||||||
|
// fileURIPathPattern matches file:// URIs that may point at local or UNC paths.
|
||||||
|
fileURIPathPattern = regexp.MustCompile(`file://[^\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.
|
||||||
|
|
@ -376,6 +380,23 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fileURIMatches := fileURIPathPattern.FindAllString(cmd, -1)
|
||||||
|
for _, rawURI := range fileURIMatches {
|
||||||
|
p, ok := fileURIToPath(rawURI, cwdPath)
|
||||||
|
if !ok || safePaths[p] {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
rel, err := filepath.Rel(cwdPath, p)
|
||||||
|
if err != nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
if strings.HasPrefix(rel, "..") {
|
||||||
|
return "Command blocked by safety guard (path outside working dir)"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Web URL schemes whose path components (starting with //) should be exempt
|
// Web URL schemes whose path components (starting with //) should be exempt
|
||||||
// from workspace sandbox checks. file: is intentionally excluded so that
|
// from workspace sandbox checks. file: is intentionally excluded so that
|
||||||
// file:// URIs are still validated against the workspace boundary.
|
// file:// URIs are still validated against the workspace boundary.
|
||||||
|
|
@ -447,6 +468,45 @@ func isPathBoundary(command string, start int) bool {
|
||||||
return strings.ContainsRune(`"'=<>|&;()[]{},`, r)
|
return strings.ContainsRune(`"'=<>|&;()[]{},`, r)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func fileURIToPath(rawURI, cwdPath string) (string, bool) {
|
||||||
|
u, err := url.Parse(rawURI)
|
||||||
|
if err != nil || u.Scheme != "file" {
|
||||||
|
return "", false
|
||||||
|
}
|
||||||
|
|
||||||
|
path := u.Path
|
||||||
|
if path == "" {
|
||||||
|
path = u.Opaque
|
||||||
|
}
|
||||||
|
|
||||||
|
if path == "" {
|
||||||
|
return "", false
|
||||||
|
}
|
||||||
|
|
||||||
|
if runtime.GOOS == "windows" && len(path) >= 3 && path[0] == '/' && path[2] == ':' {
|
||||||
|
path = path[1:]
|
||||||
|
}
|
||||||
|
|
||||||
|
if u.Host != "" && u.Host != "localhost" {
|
||||||
|
path = "//" + u.Host + path
|
||||||
|
}
|
||||||
|
|
||||||
|
path = filepath.FromSlash(path)
|
||||||
|
|
||||||
|
if runtime.GOOS == "windows" &&
|
||||||
|
filepath.VolumeName(path) == "" &&
|
||||||
|
(strings.HasPrefix(path, `\`) || strings.HasPrefix(path, `/`)) {
|
||||||
|
path = filepath.VolumeName(cwdPath) + path
|
||||||
|
}
|
||||||
|
|
||||||
|
absPath, err := filepath.Abs(path)
|
||||||
|
if err != nil {
|
||||||
|
return "", false
|
||||||
|
}
|
||||||
|
|
||||||
|
return absPath, true
|
||||||
|
}
|
||||||
|
|
||||||
func (t *ExecTool) SetTimeout(timeout time.Duration) {
|
func (t *ExecTool) SetTimeout(timeout time.Duration) {
|
||||||
t.timeout = timeout
|
t.timeout = timeout
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -528,7 +528,10 @@ func TestShellTool_GuardCommand_BlocksAbsolutePathOutsideWorkspace(t *testing.T)
|
||||||
t.Fatalf("unable to configure exec tool: %s", err)
|
t.Fatalf("unable to configure exec tool: %s", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if got := tool.guardCommand(`cat "`+outsideFile+`"`, workspace); !strings.Contains(got, "path outside working dir") {
|
if got := tool.guardCommand(
|
||||||
|
`cat "`+outsideFile+`"`,
|
||||||
|
workspace,
|
||||||
|
); !strings.Contains(got, "path outside working dir") {
|
||||||
t.Fatalf("guardCommand should block outside path, got %q", got)
|
t.Fatalf("guardCommand should block outside path, got %q", got)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue