mirror of
https://github.com/sipeed/picoclaw.git
synced 2026-06-12 18:08:54 +00:00
fix(tools): allow workspace relative exec paths
This commit is contained in:
+61
-1
@@ -1201,7 +1201,7 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||
}
|
||||
}
|
||||
|
||||
p, err := filepath.Abs(raw)
|
||||
p, err := commandPathAbs(commandPathTextFromMatch(cmd, loc[0], loc[1]), cwdPath)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
@@ -1243,6 +1243,66 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
|
||||
return ""
|
||||
}
|
||||
|
||||
func commandPathAbs(pathText, cwdPath string) (string, error) {
|
||||
if filepath.IsAbs(pathText) {
|
||||
return filepath.Abs(pathText)
|
||||
}
|
||||
return filepath.Abs(filepath.Join(cwdPath, pathText))
|
||||
}
|
||||
|
||||
func commandPathTextFromMatch(cmd string, start, end int) string {
|
||||
raw := cmd[start:end]
|
||||
if !strings.HasPrefix(raw, "/") || isUnixAbsolutePathMatchStart(cmd, start) {
|
||||
return raw
|
||||
}
|
||||
|
||||
tokenStart, tokenEnd := shellTokenBounds(cmd, start)
|
||||
prefix := cmd[tokenStart:start]
|
||||
// For --flag=rel/path, validate the value. For ambiguous attached option
|
||||
// forms like -isystem/path, keep the slash-starting path conservative.
|
||||
if eq := strings.IndexByte(prefix, '='); eq >= 0 {
|
||||
return cmd[tokenStart+eq+1 : tokenEnd]
|
||||
}
|
||||
if strings.HasPrefix(prefix, "-") {
|
||||
return raw
|
||||
}
|
||||
return cmd[tokenStart:tokenEnd]
|
||||
}
|
||||
|
||||
func shellTokenBounds(cmd string, idx int) (int, int) {
|
||||
start := idx
|
||||
for start > 0 && !isShellTokenBoundary(cmd[start-1]) {
|
||||
start--
|
||||
}
|
||||
end := idx
|
||||
for end < len(cmd) && !isShellTokenBoundary(cmd[end]) {
|
||||
end++
|
||||
}
|
||||
return start, end
|
||||
}
|
||||
|
||||
// isUnixAbsolutePathMatchStart returns true when a regex match beginning with
|
||||
// "/" is actually an absolute path token, not the separator inside a relative
|
||||
// path such as "skills/foo.py".
|
||||
func isUnixAbsolutePathMatchStart(cmd string, idx int) bool {
|
||||
if idx <= 0 {
|
||||
return true
|
||||
}
|
||||
|
||||
prev := cmd[idx-1]
|
||||
if isShellTokenBoundary(prev) || prev == '=' || prev == ',' || prev == '(' || prev == '[' || prev == '{' {
|
||||
return true
|
||||
}
|
||||
|
||||
j := idx - 1
|
||||
for j >= 0 && !isShellTokenBoundary(cmd[j]) {
|
||||
j--
|
||||
}
|
||||
prefix := cmd[j+1 : idx]
|
||||
|
||||
return strings.HasPrefix(prefix, "-") && !strings.Contains(prefix, "=")
|
||||
}
|
||||
|
||||
// isShellTokenBoundary returns true when b is a byte that separates
|
||||
// tokens in a shell command (space, tab, colon, semicolon, pipe, etc.).
|
||||
func isShellTokenBoundary(b byte) bool {
|
||||
|
||||
@@ -426,6 +426,90 @@ func TestShellTool_RestrictToWorkspace(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestShellTool_RelativePathWithSlashAllowed verifies that local relative paths
|
||||
// under the workspace are not mistaken for absolute paths by the safety guard.
|
||||
func TestShellTool_RelativePathWithSlashAllowed(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
scriptDir := filepath.Join(tmpDir, "skills", "calendar-query", "scripts")
|
||||
if err := os.MkdirAll(scriptDir, 0o755); err != nil {
|
||||
t.Fatalf("failed to create script dir: %v", err)
|
||||
}
|
||||
scriptPath := filepath.Join(scriptDir, "query_calendar.py")
|
||||
if err := os.WriteFile(scriptPath, []byte("calendar ok\n"), 0o644); err != nil {
|
||||
t.Fatalf("failed to create script: %v", err)
|
||||
}
|
||||
|
||||
tool, err := NewExecTool(tmpDir, true)
|
||||
if err != nil {
|
||||
t.Fatalf("unable to configure exec tool: %s", err)
|
||||
}
|
||||
|
||||
result := tool.Execute(context.Background(), map[string]any{
|
||||
"action": "run",
|
||||
"command": "cat skills/calendar-query/scripts/query_calendar.py",
|
||||
})
|
||||
|
||||
if result.IsError {
|
||||
t.Fatalf("relative workspace script path should be allowed, got: %s", result.ForLLM)
|
||||
}
|
||||
if !strings.Contains(result.ForLLM, "calendar ok") {
|
||||
t.Fatalf("expected script output, got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
func TestShellTool_AttachedAbsolutePathsStillBlocked(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{
|
||||
"cat --file=/etc/passwd",
|
||||
"cc -I/etc main.c",
|
||||
"echo -isystem/usr/include",
|
||||
}
|
||||
|
||||
for _, cmd := range commands {
|
||||
result := tool.Execute(context.Background(), map[string]any{"action": "run", "command": cmd})
|
||||
if !result.IsError || !strings.Contains(result.ForLLM, "path outside working dir") {
|
||||
t.Errorf("attached absolute path should be blocked: %q\n got: %s", cmd, result.ForLLM)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestShellTool_OptionValueRelativeSymlinkEscapeBlocked(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
workspace := filepath.Join(root, "workspace")
|
||||
outsideDir := filepath.Join(root, "outside")
|
||||
if err := os.MkdirAll(workspace, 0o755); err != nil {
|
||||
t.Fatalf("failed to create workspace: %v", err)
|
||||
}
|
||||
if err := os.MkdirAll(outsideDir, 0o755); err != nil {
|
||||
t.Fatalf("failed to create outside dir: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(outsideDir, "passwd"), []byte("secret\n"), 0o644); err != nil {
|
||||
t.Fatalf("failed to create outside file: %v", err)
|
||||
}
|
||||
if err := os.Symlink(outsideDir, filepath.Join(workspace, "link")); err != nil {
|
||||
t.Skipf("symlinks not supported in this environment: %v", err)
|
||||
}
|
||||
|
||||
tool, err := NewExecTool(workspace, true)
|
||||
if err != nil {
|
||||
t.Fatalf("unable to configure exec tool: %s", err)
|
||||
}
|
||||
|
||||
result := tool.Execute(context.Background(), map[string]any{
|
||||
"action": "run",
|
||||
"command": "echo --config=link/passwd",
|
||||
})
|
||||
|
||||
if !result.IsError || !strings.Contains(result.ForLLM, "path outside working dir") {
|
||||
t.Fatalf("option value symlink escape should be blocked, got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestShellTool_DevNullAllowed verifies that /dev/null redirections are not blocked (issue #964).
|
||||
func TestShellTool_DevNullAllowed(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
Reference in New Issue
Block a user