fix: safety guard incorrectly blocks commands with URLs (#1254)

* fix: safety guard incorrectly blocks commands with URLs

The absolutePathPattern regex was matching URL path components like
//github.com as file system paths, causing commands containing URLs
to be incorrectly blocked by the workspace restriction safety guard.

For example, 'agent-browser open https://github.com' would be blocked
because //github.com was treated as an absolute file path outside
the working directory.

The fix adds a check to skip any path match that starts with '//',
as these are URL path components, not file system paths.

Fixes #1203

* fix: handle file:// URIs correctly in safety guard

The previous fix skipped all paths starting with '//', which incorrectly
also skipped file:// URIs that could escape the workspace sandbox.

Changes:
- Only skip '//' paths when preceded by web URL schemes (http:, https:, ftp:, etc.)
- file:// URIs are now properly checked against workspace boundaries
- Added TestShellTool_FileURISandboxing to verify the fix

Fixes security issue raised by @alexhoshina in PR #1254

* style: fix gofumpt formatting

* 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.

pkg/tools/shell.go

@@ -373,9 +373,37 @@ 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]]
+
+			// 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]]
+				isWebURL := false
+
+				for _, scheme := range webSchemes {
+					if strings.HasSuffix(before, scheme) {
+						isWebURL = true
+						break
+					}
+				}
+
+				if isWebURL {
+					continue
+				}
+			}
 
-		for _, raw := range matches {
 			p, err := filepath.Abs(raw)
 			if err != nil {
 				continue

pkg/tools/shell_test.go

@@ -522,3 +522,101 @@ func TestShellTool_CustomAllowPatterns(t *testing.T) {
 		t.Errorf("'git push upstream main' should still be blocked by deny pattern")
 	}
 }
+
+// TestShellTool_URLsNotBlocked verifies that commands containing URLs are not
+// incorrectly blocked by the workspace restriction safety guard (issue #1203).
+func TestShellTool_URLsNotBlocked(t *testing.T) {
+	tmpDir := t.TempDir()
+	tool, err := NewExecTool(tmpDir, true)
+	if err != nil {
+		t.Fatalf("unable to configure exec tool: %s", err)
+	}
+
+	// These commands contain URLs and should NOT be blocked by workspace restriction.
+	// The URL path components (e.g., "//github.com") should be recognized as URLs,
+	// not as file system paths.
+	commands := []string{
+		"agent-browser open https://github.com",
+		"curl https://api.example.com/data",
+		"wget http://example.com/file",
+		"browser open https://github.com/user/repo",
+		"fetch ftp://ftp.example.com/file.txt",
+		"git clone https://github.com/sipeed/picoclaw.git",
+	}
+
+	for _, cmd := range commands {
+		result := tool.Execute(context.Background(), map[string]any{"command": cmd})
+		if result.IsError && strings.Contains(result.ForLLM, "path outside working dir") {
+			t.Errorf("command with URL should not be blocked by workspace check: %s\n  error: %s", cmd, result.ForLLM)
+		}
+	}
+}
+
+// TestShellTool_FileURISandboxing verifies that file:// URIs that escape the
+// workspace are still blocked, even though other URLs are allowed (issue #1254).
+func TestShellTool_FileURISandboxing(t *testing.T) {
+	tmpDir := t.TempDir()
+	tool, err := NewExecTool(tmpDir, true)
+	if err != nil {
+		t.Fatalf("unable to configure exec tool: %s", err)
+	}
+
+	// These file:// URIs should be blocked if they reference paths outside the workspace.
+	// Unlike web URLs (http://, https://, ftp://), file:// URIs can be used to escape the sandbox.
+	blockedCommands := []string{
+		"cat file:///etc/passwd",
+		"cat file:///etc/hosts",
+		"cat file:///root/.ssh/id_rsa",
+	}
+
+	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("file:// URI outside workspace should be blocked: %s", cmd)
+		}
+	}
+
+	// These file:// URIs should be allowed if they reference paths inside the workspace.
+	// Create a test file inside the temp directory
+	testFile := filepath.Join(tmpDir, "test.txt")
+	if err := os.WriteFile(testFile, []byte("test content"), 0o644); err != nil {
+		t.Fatalf("failed to create test file: %s", err)
+	}
+
+	allowedCommands := []string{
+		"cat file://" + testFile,
+	}
+
+	for _, cmd := range allowedCommands {
+		result := tool.Execute(context.Background(), map[string]any{"command": cmd})
+		if result.IsError && strings.Contains(result.ForLLM, "path outside working dir") {
+			t.Errorf("file:// URI inside workspace should be allowed: %s\n  error: %s", cmd, result.ForLLM)
+		}
+	}
+}
+
+// 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)
+		}
+	}
+}