diff --git a/.github/pr-assets/748/after-worktree.txt b/.github/pr-assets/748/after-worktree.txt new file mode 100644 index 00000000..f4f3d2a3 --- /dev/null +++ b/.github/pr-assets/748/after-worktree.txt @@ -0,0 +1,25 @@ +=== RUN TestSetupToolRegistryExecWorkspacePaths +2026/04/07 21:24:13 INFO memory + knowledge graph tools registered (PG-backed) +2026/04/07 21:24:13 INFO web_search tool enabled +2026/04/07 21:24:13 INFO web_fetch tool enabled policy="" blocked=0 +2026/04/07 21:24:13 INFO tts enabled provider=edge auto=off +2026/04/07 21:24:13 INFO tool rate limiting enabled per_hour=150 +2026/04/07 21:24:13 INFO exec approval enabled security=full ask=off +=== RUN TestSetupToolRegistryExecWorkspacePaths/personal_workspace_prefixed_uploads_allowed +2026/04/07 21:24:13 WARN secure_cli.lookup: store is nil, skipping credentialed exec command="printf '%s' file=@\"/var/folders/t9/0cyfylls7d1gdy_6qp8kn2qc0000gn/T/TestSetupToolRegistryExecWorkspacePaths3385685647/001/personal/.uploads/Quarterly Report.png\"" +=== RUN TestSetupToolRegistryExecWorkspacePaths/team_workspace_quoted_input_allowed +2026/04/07 21:24:13 WARN secure_cli.lookup: store is nil, skipping credentialed exec command="printf '%s' --input=\"/var/folders/t9/0cyfylls7d1gdy_6qp8kn2qc0000gn/T/TestSetupToolRegistryExecWorkspacePaths3385685647/001/teams/team-123/Quarterly Report.png\"" +=== RUN TestSetupToolRegistryExecWorkspacePaths/legacy_dotgoclaw_uploads_layout_allowed +2026/04/07 21:24:13 WARN secure_cli.lookup: store is nil, skipping credentialed exec command="cp \"/var/folders/t9/0cyfylls7d1gdy_6qp8kn2qc0000gn/T/TestSetupToolRegistryExecWorkspacePaths3385685647/001/.goclaw/goclaw-workspace/ws/system/uploads/Quarterly Report.png\" \"/var/folders/t9/0cyfylls7d1gdy_6qp8kn2qc0000gn/T/TestSetupToolRegistryExecWorkspacePaths3385685647/002/partner.png\"" +=== RUN TestSetupToolRegistryExecWorkspacePaths/team_workspace_symlink_escape_denied +=== RUN TestSetupToolRegistryExecWorkspacePaths/unrelated_data_dir_path_denied +=== RUN TestSetupToolRegistryExecWorkspacePaths/workspace_local_dotgoclaw_denied +--- PASS: TestSetupToolRegistryExecWorkspacePaths (0.02s) + --- PASS: TestSetupToolRegistryExecWorkspacePaths/personal_workspace_prefixed_uploads_allowed (0.01s) + --- PASS: TestSetupToolRegistryExecWorkspacePaths/team_workspace_quoted_input_allowed (0.00s) + --- PASS: TestSetupToolRegistryExecWorkspacePaths/legacy_dotgoclaw_uploads_layout_allowed (0.01s) + --- PASS: TestSetupToolRegistryExecWorkspacePaths/team_workspace_symlink_escape_denied (0.00s) + --- PASS: TestSetupToolRegistryExecWorkspacePaths/unrelated_data_dir_path_denied (0.00s) + --- PASS: TestSetupToolRegistryExecWorkspacePaths/workspace_local_dotgoclaw_denied (0.00s) +PASS +ok github.com/nextlevelbuilder/goclaw/cmd 0.041s diff --git a/.github/pr-assets/748/before-claw.txt b/.github/pr-assets/748/before-claw.txt new file mode 100644 index 00000000..72d4dbde --- /dev/null +++ b/.github/pr-assets/748/before-claw.txt @@ -0,0 +1,94 @@ +go: downloading github.com/golang-migrate/migrate/v4 v4.19.1 +go: downloading github.com/jackc/pgx/v5 v5.6.0 +go: downloading github.com/google/uuid v1.6.0 +go: downloading github.com/gorilla/websocket v1.5.4-0.20250319132907-e064f32e3674 +go: downloading github.com/charmbracelet/huh v0.8.0 +go: downloading github.com/spf13/cobra v1.10.2 +go: downloading github.com/bwmarrin/discordgo v0.29.0 +go: downloading golang.org/x/sync v0.19.0 +go: downloading github.com/slack-go/slack v0.19.0 +go: downloading github.com/mattn/go-runewidth v0.0.16 +go: downloading github.com/mymmrac/telego v1.6.0 +go: downloading github.com/fsnotify/fsnotify v1.9.0 +go: downloading github.com/titanous/json5 v1.0.0 +go: downloading github.com/disintegration/imaging v1.6.2 +go: downloading golang.org/x/image v0.27.0 +go: downloading golang.org/x/time v0.14.0 +go: downloading github.com/mark3labs/mcp-go v0.44.0 +go: downloading github.com/adhocore/gronx v1.19.6 +go: downloading github.com/go-rod/rod v0.116.2 +go: downloading github.com/lib/pq v1.10.9 +go: downloading github.com/mattn/go-shellwords v1.0.12 +go: downloading golang.org/x/net v0.49.0 +go: downloading golang.org/x/text v0.33.0 +go: downloading github.com/rivo/uniseg v0.4.7 +go: downloading github.com/catppuccin/go v0.3.0 +go: downloading github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 +go: downloading github.com/charmbracelet/bubbletea v1.3.6 +go: downloading github.com/charmbracelet/lipgloss v1.1.0 +go: downloading github.com/charmbracelet/x/cellbuf v0.0.13 +go: downloading github.com/charmbracelet/x/exp/strings v0.0.0-20240722160745-212f7b056ed0 +go: downloading github.com/mitchellh/hashstructure/v2 v2.0.2 +go: downloading github.com/spf13/pflag v1.0.9 +go: downloading golang.org/x/crypto v0.47.0 +go: downloading golang.org/x/sys v0.42.0 +go: downloading github.com/valyala/fasthttp v1.69.0 +go: downloading github.com/invopop/jsonschema v0.13.0 +go: downloading github.com/spf13/cast v1.7.1 +go: downloading github.com/yosida95/uritemplate/v3 v3.0.2 +go: downloading github.com/dustin/go-humanize v1.0.1 +go: downloading github.com/charmbracelet/x/ansi v0.9.3 +go: downloading github.com/charmbracelet/x/term v0.2.1 +go: downloading github.com/atotto/clipboard v0.1.4 +go: downloading github.com/grbit/go-json v0.11.0 +go: downloading github.com/valyala/fastjson v1.6.7 +go: downloading github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 +go: downloading github.com/muesli/cancelreader v0.2.2 +go: downloading github.com/muesli/termenv v0.16.0 +go: downloading github.com/wk8/go-ordered-map/v2 v2.1.8 +go: downloading github.com/ysmood/goob v0.4.0 +go: downloading github.com/ysmood/got v0.40.0 +go: downloading github.com/ysmood/gson v0.7.3 +go: downloading github.com/ysmood/fetchup v0.2.3 +go: downloading github.com/ysmood/leakless v0.9.0 +go: downloading github.com/andybalholm/brotli v1.2.0 +go: downloading github.com/klauspost/compress v1.18.2 +go: downloading github.com/valyala/bytebufferpool v1.0.0 +go: downloading github.com/jackc/pgpassfile v1.0.0 +go: downloading github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 +go: downloading github.com/jackc/puddle/v2 v2.2.2 +go: downloading github.com/lucasb-eyer/go-colorful v1.2.0 +go: downloading github.com/bahlo/generic-list-go v0.2.0 +go: downloading github.com/buger/jsonparser v1.1.1 +go: downloading github.com/mailru/easyjson v0.7.7 +go: downloading gopkg.in/yaml.v3 v3.0.1 +go: downloading github.com/aymanbagabas/go-osc52/v2 v2.0.1 +go: downloading github.com/mattn/go-isatty v0.0.20 +go: downloading github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc +go: downloading github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e +=== RUN TestSetupToolRegistryExecWorkspacePaths +2026/04/08 01:24:40 INFO memory + knowledge graph tools registered (PG-backed) +2026/04/08 01:24:40 INFO web_search tool enabled +2026/04/08 01:24:40 INFO web_fetch tool enabled policy="" blocked=0 +2026/04/08 01:24:40 INFO tts enabled provider=edge auto=off +2026/04/08 01:24:40 INFO tool rate limiting enabled per_hour=150 +2026/04/08 01:24:40 INFO exec approval enabled security=full ask=off +=== RUN TestSetupToolRegistryExecWorkspacePaths/personal_workspace_prefixed_uploads_allowed + gateway_setup_exec_workspace_test.go:133: denied = true, want false; output = command denied by safety policy: matches pattern /tmp/TestSetupToolRegistryExecWorkspacePaths1657441769/001 +=== RUN TestSetupToolRegistryExecWorkspacePaths/team_workspace_quoted_input_allowed + gateway_setup_exec_workspace_test.go:133: denied = true, want false; output = command denied by safety policy: matches pattern /tmp/TestSetupToolRegistryExecWorkspacePaths1657441769/001 +=== RUN TestSetupToolRegistryExecWorkspacePaths/legacy_dotgoclaw_uploads_layout_allowed + gateway_setup_exec_workspace_test.go:133: denied = true, want false; output = command denied by safety policy: matches pattern /tmp/TestSetupToolRegistryExecWorkspacePaths1657441769/001 +=== RUN TestSetupToolRegistryExecWorkspacePaths/team_workspace_symlink_escape_denied +=== RUN TestSetupToolRegistryExecWorkspacePaths/unrelated_data_dir_path_denied +=== RUN TestSetupToolRegistryExecWorkspacePaths/workspace_local_dotgoclaw_denied +--- FAIL: TestSetupToolRegistryExecWorkspacePaths (0.00s) + --- FAIL: TestSetupToolRegistryExecWorkspacePaths/personal_workspace_prefixed_uploads_allowed (0.00s) + --- FAIL: TestSetupToolRegistryExecWorkspacePaths/team_workspace_quoted_input_allowed (0.00s) + --- FAIL: TestSetupToolRegistryExecWorkspacePaths/legacy_dotgoclaw_uploads_layout_allowed (0.00s) + --- PASS: TestSetupToolRegistryExecWorkspacePaths/team_workspace_symlink_escape_denied (0.00s) + --- PASS: TestSetupToolRegistryExecWorkspacePaths/unrelated_data_dir_path_denied (0.00s) + --- PASS: TestSetupToolRegistryExecWorkspacePaths/workspace_local_dotgoclaw_denied (0.00s) +FAIL +FAIL github.com/nextlevelbuilder/goclaw/cmd 0.019s +FAIL diff --git a/.github/pr-assets/748/claw-live-glm-thuc-bo-before.png b/.github/pr-assets/748/claw-live-glm-thuc-bo-before.png new file mode 100644 index 00000000..b940d58c Binary files /dev/null and b/.github/pr-assets/748/claw-live-glm-thuc-bo-before.png differ diff --git a/.github/pr-assets/748/index.html b/.github/pr-assets/748/index.html new file mode 100644 index 00000000..f48afca2 --- /dev/null +++ b/.github/pr-assets/748/index.html @@ -0,0 +1,228 @@ + + + + + +PR #748 — fix(exec): allow uploaded files in active workspaces + + + + + +

fix(exec): allow uploaded files in active workspaces

+

PR #748 — visual proof pack for issue #739

+ +
+
+

Bug

+

Active workspace files under deny-roots were rejected by exec even when they belonged to the current upload/team scope.

+
+
+

Root Cause

+

Path exemptions were static and non-shell-aware, so quoted, prefixed, and legacy screenshot-era upload paths still matched the deny root.

+
+
+

Fix

+

Shell-aware parsing + canonical path checks now exempt the active personal .uploads subtree, the legacy uploads subtree, and the active team workspace.

+
+
+ +
+ Before source: stable claw:3000 backed by the unfixed /opt/goclaw deployment (v2.7.1). After source: temporary PR deployment on claw:3001, backed by goclaw-pr748:local built from the fixed code commit 83fa1342. Both UI captures use fresh chats with the same user and GLM thục bô. Callout border marks the reviewed exec row. +
+ +
+

UI Before / After

+
+
+
Before
+
+ Before: stable claw blocks absolute .goclaw upload path +
+

Stable claw release, fresh chat with GLM thục bô: the agent runs the exact absolute /app/.goclaw/.../.uploads/...jpg copy command and the exec row fails with the safety-policy .goclaw/ denial.

+
+
+
After
+
+ After: fixed temporary claw deployment allows absolute .goclaw upload path +
+

Temporary fixed deployment on claw:3001, fresh chat with the same agent: the same absolute /app/.goclaw/.../.uploads/...jpg copy command now completes successfully and returns /tmp/partner-abs.png.

+
+
+
+ +
+

Gateway Reproduction

+
+
+
Before
+
+
$ ssh claw 'docker run --rm -v /tmp/goclaw-739-before:/src -w /src golang:1.26 sh -lc "/usr/local/go/bin/go test ./cmd -run TestSetupToolRegistryExecWorkspacePaths -count=1 -v"'
+=== RUN   TestSetupToolRegistryExecWorkspacePaths
+=== RUN   TestSetupToolRegistryExecWorkspacePaths/personal_workspace_prefixed_uploads_allowed
+gateway_setup_exec_workspace_test.go:133: denied = true, want false; output = command denied by safety policy: matches pattern /tmp/TestSetupToolRegistryExecWorkspacePaths.../001
+=== RUN   TestSetupToolRegistryExecWorkspacePaths/team_workspace_quoted_input_allowed
+gateway_setup_exec_workspace_test.go:133: denied = true, want false; output = command denied by safety policy: matches pattern /tmp/TestSetupToolRegistryExecWorkspacePaths.../001
+=== RUN   TestSetupToolRegistryExecWorkspacePaths/legacy_dotgoclaw_uploads_layout_allowed
+gateway_setup_exec_workspace_test.go:133: denied = true, want false; output = command denied by safety policy: matches pattern /tmp/TestSetupToolRegistryExecWorkspacePaths.../001
+FAIL
+
+

Baseline source on claw: the exact gateway wiring still rejects all three representative workspace-path forms.

+
+
+
After
+
+
$ go test ./cmd -run TestSetupToolRegistryExecWorkspacePaths -count=1 -v
+=== RUN   TestSetupToolRegistryExecWorkspacePaths
+=== RUN   TestSetupToolRegistryExecWorkspacePaths/personal_workspace_prefixed_uploads_allowed
+=== RUN   TestSetupToolRegistryExecWorkspacePaths/team_workspace_quoted_input_allowed
+=== RUN   TestSetupToolRegistryExecWorkspacePaths/legacy_dotgoclaw_uploads_layout_allowed
+--- PASS: TestSetupToolRegistryExecWorkspacePaths (0.01s)
+    --- PASS: personal_workspace_prefixed_uploads_allowed
+    --- PASS: team_workspace_quoted_input_allowed
+    --- PASS: legacy_dotgoclaw_uploads_layout_allowed
+PASS
+
+

Current worktree: the same gateway path handling now passes for personal uploads, team workspace paths, and the screenshot-era legacy .goclaw/.../uploads layout.

+
+
+
+ +
+

Supporting Live Recreation on claw

+
+ Live claw recreation with GLM thục bô +
+

+ This earlier live run documents the same production agent and workspace context on stable claw. The report’s primary Before/After pair above is the clean isolated proof you asked for; this screenshot is kept as supporting context for the reproduction journey. +

+
+ +
+

What This Proves

+ +
+ + diff --git a/.github/pr-assets/748/pr748-after-absolute-goclaw-allowed.png b/.github/pr-assets/748/pr748-after-absolute-goclaw-allowed.png new file mode 100644 index 00000000..2e36d929 Binary files /dev/null and b/.github/pr-assets/748/pr748-after-absolute-goclaw-allowed.png differ diff --git a/.github/pr-assets/748/pr748-before-absolute-goclaw-blocked.png b/.github/pr-assets/748/pr748-before-absolute-goclaw-blocked.png new file mode 100644 index 00000000..6e73455d Binary files /dev/null and b/.github/pr-assets/748/pr748-before-absolute-goclaw-blocked.png differ diff --git a/cmd/gateway_setup_exec_workspace_test.go b/cmd/gateway_setup_exec_workspace_test.go new file mode 100644 index 00000000..6fc79ba8 --- /dev/null +++ b/cmd/gateway_setup_exec_workspace_test.go @@ -0,0 +1,144 @@ +package cmd + +import ( + "context" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/config" + "github.com/nextlevelbuilder/goclaw/internal/providers" + "github.com/nextlevelbuilder/goclaw/internal/tools" +) + +func testExecToolFromGatewaySetup(t *testing.T, workspace, dataDir string) *tools.ExecTool { + t.Helper() + + cfg := config.Default() + cfg.DataDir = dataDir + cfg.Agents.Defaults.Workspace = workspace + cfg.Tools.Browser.Enabled = false + + providerRegistry := providers.NewRegistry(nil) + toolsReg, _, _, _, _, _, _, _, _, _, _ := setupToolRegistry(cfg, workspace, providerRegistry) + + execToolAny, ok := toolsReg.Get("exec") + if !ok { + t.Fatal("exec tool not registered") + } + execTool, ok := execToolAny.(*tools.ExecTool) + if !ok { + t.Fatalf("exec tool type = %T, want *tools.ExecTool", execToolAny) + } + return execTool +} + +func TestSetupToolRegistryExecWorkspacePaths(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink setup requires elevated privileges on Windows") + } + dataDir := t.TempDir() + workspace := filepath.Join(dataDir, "personal") + teamWorkspace := filepath.Join(dataDir, "teams", "team-123") + for _, dir := range []string{workspace, teamWorkspace} { + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatalf("MkdirAll(%q) error = %v", dir, err) + } + } + + execTool := testExecToolFromGatewaySetup(t, workspace, dataDir) + + uploadPath := filepath.Join(workspace, ".uploads", "Quarterly Report.png") + if err := os.MkdirAll(filepath.Dir(uploadPath), 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + if err := os.WriteFile(uploadPath, []byte("ok"), 0644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + teamFilePath := filepath.Join(teamWorkspace, "Quarterly Report.png") + if err := os.WriteFile(teamFilePath, []byte("ok"), 0644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + protectedPath := filepath.Join(dataDir, "config.json") + if err := os.WriteFile(protectedPath, []byte("secret"), 0644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + symlinkPath := filepath.Join(teamWorkspace, "leak.txt") + if err := os.Symlink(protectedPath, symlinkPath); err != nil { + t.Fatalf("Symlink() error = %v", err) + } + legacyWorkspace := filepath.Join(dataDir, ".goclaw", "goclaw-workspace", "ws", "system") + legacyUploadPath := filepath.Join(legacyWorkspace, "uploads", "Quarterly Report.png") + legacyCopyTarget := filepath.Join(t.TempDir(), "partner.png") + if err := os.MkdirAll(filepath.Dir(legacyUploadPath), 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + if err := os.WriteFile(legacyUploadPath, []byte("ok"), 0644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + tests := []struct { + name string + ctx context.Context + command string + wantDenied bool + wantPath string + }{ + { + name: "personal_workspace_prefixed_uploads_allowed", + ctx: tools.WithToolWorkspace(context.Background(), workspace), + command: "printf '%s' file=@\"" + uploadPath + "\"", + }, + { + name: "team_workspace_quoted_input_allowed", + ctx: tools.WithToolTeamWorkspace(tools.WithToolWorkspace(context.Background(), workspace), teamWorkspace), + command: "printf '%s' --input=\"" + teamFilePath + "\"", + }, + { + name: "legacy_dotgoclaw_uploads_layout_allowed", + ctx: tools.WithToolWorkspace(context.Background(), legacyWorkspace), + command: "cp \"" + legacyUploadPath + "\" \"" + legacyCopyTarget + "\"", + wantPath: legacyCopyTarget, + }, + { + name: "team_workspace_symlink_escape_denied", + ctx: tools.WithToolTeamWorkspace(tools.WithToolWorkspace(context.Background(), workspace), teamWorkspace), + command: "printf '%s' " + symlinkPath, + wantDenied: true, + }, + { + name: "unrelated_data_dir_path_denied", + ctx: tools.WithToolWorkspace(context.Background(), workspace), + command: "printf '%s' " + protectedPath, + wantDenied: true, + }, + { + name: "workspace_local_dotgoclaw_denied", + ctx: tools.WithToolWorkspace(context.Background(), workspace), + command: "printf '%s' " + filepath.Join(workspace, ".goclaw", "secrets.json"), + wantDenied: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := execTool.Execute(tc.ctx, map[string]any{ + "command": tc.command, + }) + + denied := strings.Contains(result.ForLLM, "command denied by safety policy") + if denied != tc.wantDenied { + t.Fatalf("denied = %v, want %v; output = %s", denied, tc.wantDenied, result.ForLLM) + } + if tc.wantPath != "" { + if _, err := os.Stat(tc.wantPath); err != nil { + t.Fatalf("expected copied file at %q, got stat error: %v", tc.wantPath, err) + } + } else if !tc.wantDenied && !strings.Contains(result.ForLLM, "Quarterly Report.png") { + t.Fatalf("expected output to contain quoted file path, got: %s", result.ForLLM) + } + }) + } +} diff --git a/internal/tools/shell.go b/internal/tools/shell.go index d4834a84..b4bc5402 100644 --- a/internal/tools/shell.go +++ b/internal/tools/shell.go @@ -6,11 +6,14 @@ import ( "errors" "fmt" "log/slog" + "os" "os/exec" + "path/filepath" "regexp" "strings" "time" + shellwords "github.com/mattn/go-shellwords" "github.com/nextlevelbuilder/goclaw/internal/sandbox" "github.com/nextlevelbuilder/goclaw/internal/store" "golang.org/x/text/unicode/norm" @@ -31,30 +34,31 @@ func DefaultDenyPatterns() []*regexp.Regexp { // ExecTool executes shell commands, optionally inside a sandbox container. type ExecTool struct { - workspace string + workspace string timeout time.Duration - pathDenyPatterns []*regexp.Regexp // always-on path-based denials (DenyPaths) - denyExemptions []string // substrings that exempt a command from deny + pathDenyPatterns []*regexp.Regexp // always-on path-based denials (DenyPaths) + pathDenyRoots []string // raw deny roots for nested workspace exemptions + denyExemptions []string // substrings that exempt a command from deny restrict bool sandboxMgr sandbox.Manager // nil = no sandbox, execute on host approvalMgr *ExecApprovalManager // nil = no approval needed agentID string // for approval request context - secureCLIStore store.SecureCLIStore // nil = no credentialed exec + secureCLIStore store.SecureCLIStore // nil = no credentialed exec } // NewExecTool creates an exec tool that runs commands directly on the host. func NewExecTool(workspace string, restrict bool) *ExecTool { return &ExecTool{ workspace: workspace, - timeout: 60 * time.Second, - restrict: restrict, + timeout: 60 * time.Second, + restrict: restrict, } } // NewSandboxedExecTool creates an exec tool that routes commands through a sandbox container. func NewSandboxedExecTool(workspace string, restrict bool, mgr sandbox.Manager) *ExecTool { return &ExecTool{ - workspace: workspace, + workspace: workspace, timeout: 300 * time.Second, // sandbox allows longer timeout restrict: restrict, sandboxMgr: mgr, @@ -70,6 +74,7 @@ func (t *ExecTool) DenyPaths(paths ...string) { for _, p := range paths { escaped := regexp.QuoteMeta(p) t.pathDenyPatterns = append(t.pathDenyPatterns, regexp.MustCompile(escaped)) + t.pathDenyRoots = append(t.pathDenyRoots, p) } } @@ -96,6 +101,256 @@ func normalizeCommand(s string) string { return s } +func (t *ExecTool) dynamicPathExemptions(ctx context.Context) []string { + var exemptions []string + seen := make(map[string]struct{}, 4) + workspace := ToolWorkspaceFromCtx(ctx) + teamWorkspace := ToolTeamWorkspaceFromCtx(ctx) + + var dirs []string + if teamWorkspace != "" { + dirs = append(dirs, teamWorkspace) + } + if workspace != "" && filepath.Clean(workspace) != filepath.Clean(teamWorkspace) { + dirs = append(dirs, filepath.Join(workspace, ".uploads")) + dirs = append(dirs, filepath.Join(workspace, "uploads")) + } + + for _, dir := range dirs { + if dir == "" || strings.Contains(dir, "..") { + continue + } + for _, variant := range pathAliasVariants(filepath.Clean(dir)) { + if !t.isNestedUnderDeniedRoot(variant) { + continue + } + for _, ex := range []string{variant, variant + string(filepath.Separator)} { + if _, ok := seen[ex]; ok { + continue + } + seen[ex] = struct{}{} + exemptions = append(exemptions, ex) + } + } + } + return exemptions +} + +func pathAliasVariants(path string) []string { + variants := []string{path} + for _, mapping := range [][2]string{ + {"/app/workspace", "/app/.goclaw"}, + {"/app/.goclaw", "/app/workspace"}, + } { + from, to := mapping[0], mapping[1] + if path == from { + variants = append(variants, to) + continue + } + if strings.HasPrefix(path, from+string(filepath.Separator)) { + variants = append(variants, to+strings.TrimPrefix(path, from)) + } + } + return variants +} + +func (t *ExecTool) isNestedUnderDeniedRoot(path string) bool { + for _, root := range t.pathDenyRoots { + cleanRoot := filepath.Clean(root) + if cleanRoot == "." || cleanRoot == string(filepath.Separator) { + continue + } + if !filepath.IsAbs(cleanRoot) { + marker := string(filepath.Separator) + cleanRoot + string(filepath.Separator) + if strings.Contains(path, marker) { + return true + } + continue + } + if path == cleanRoot { + continue + } + if strings.HasPrefix(path, cleanRoot+string(filepath.Separator)) { + return true + } + } + return false +} + +func matchesPathExemption(path string, exemptions []string) bool { + sep := string(filepath.Separator) + for _, ex := range exemptions { + if ex == "" { + continue + } + if path == ex { + return true + } + if strings.HasSuffix(ex, sep) { + if strings.HasPrefix(path, ex) { + return true + } + continue + } + if strings.HasPrefix(path, ex+sep) { + return true + } + } + return false +} + +func parseExecCommandWords(command string) []string { + var words []string + for _, segment := range splitExecCommandSegments(command) { + parser := shellwords.NewParser() + parser.ParseBacktick = false + parser.ParseEnv = false + + segmentWords, err := parser.Parse(segment) + if err != nil || len(segmentWords) == 0 { + words = append(words, strings.Fields(segment)...) + continue + } + words = append(words, segmentWords...) + } + if len(words) == 0 { + return strings.Fields(command) + } + return words +} + +func splitExecCommandSegments(command string) []string { + var segments []string + start := 0 + inSingle := false + inDouble := false + + for i := 0; i < len(command); i++ { + ch := command[i] + switch { + case inSingle: + if ch == '\'' { + inSingle = false + } + case inDouble: + if ch == '\\' && i+1 < len(command) { + i++ + } else if ch == '"' { + inDouble = false + } + default: + switch ch { + case '\\': + if i+1 < len(command) { + i++ + } + case '\'': + inSingle = true + case '"': + inDouble = true + case ';', '|', '&', '<', '>', '\n', '\r': + if segment := strings.TrimSpace(command[start:i]); segment != "" { + segments = append(segments, segment) + } + start = i + 1 + } + } + } + + if tail := strings.TrimSpace(command[start:]); tail != "" { + segments = append(segments, tail) + } + return segments +} + +func extractPathCandidates(word string) []string { + if word == "" { + return nil + } + + queue := []string{word} + seen := make(map[string]struct{}, 4) + var out []string + + for len(queue) > 0 { + current := queue[0] + queue = queue[1:] + if current == "" { + continue + } + if _, ok := seen[current]; ok { + continue + } + seen[current] = struct{}{} + if looksLikePathCandidate(current) { + out = append(out, current) + } + for _, sep := range []string{"=", "@"} { + if idx := strings.Index(current, sep); idx >= 0 && idx+1 < len(current) { + queue = append(queue, current[idx+1:]) + } + } + } + return out +} + +func looksLikePathCandidate(s string) bool { + if s == "" { + return false + } + if filepath.IsAbs(s) { + return true + } + return strings.HasPrefix(s, "./") || + strings.HasPrefix(s, "../") || + strings.HasPrefix(s, ".uploads/") || + strings.HasPrefix(s, ".goclaw/") || + strings.HasPrefix(s, "teams/") || + strings.HasPrefix(s, "tenants/") || + strings.HasPrefix(s, "~/") || + strings.Contains(s, string(filepath.Separator)) +} + +func canonicalizeExecPath(path, baseDir string) (string, error) { + if strings.HasPrefix(path, "~/") { + homeDir, err := os.UserHomeDir() + if err != nil { + return "", err + } + path = filepath.Join(homeDir, strings.TrimPrefix(path, "~/")) + } + if !filepath.IsAbs(path) { + path = filepath.Join(baseDir, path) + } + absPath, _ := filepath.Abs(filepath.Clean(path)) + if real, err := filepath.EvalSymlinks(absPath); err == nil { + return real, nil + } + return resolveThroughExistingAncestors(absPath) +} + +func matchesAnyPathExemption(word string, exemptions []string, baseDir string) bool { + for _, candidate := range extractPathCandidates(word) { + if strings.Contains(candidate, "..") { + continue + } + realCandidate, err := canonicalizeExecPath(candidate, baseDir) + if err != nil { + continue + } + for _, exemption := range exemptions { + realExemption, err := canonicalizeExecPath(exemption, baseDir) + if err != nil { + continue + } + if matchesPathExemption(realCandidate, []string{realExemption}) { + return true + } + } + } + return false +} + // SetApprovalManager sets the exec approval manager for this tool. func (t *ExecTool) SetApprovalManager(mgr *ExecApprovalManager, agentID string) { t.approvalMgr = mgr @@ -155,8 +410,15 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *Result { allPatterns := make([]*regexp.Regexp, 0, len(groupPatterns)+len(t.pathDenyPatterns)) allPatterns = append(allPatterns, groupPatterns...) allPatterns = append(allPatterns, t.pathDenyPatterns...) + exemptions := append([]string{}, t.denyExemptions...) + exemptions = append(exemptions, t.dynamicPathExemptions(ctx)...) // Check for dangerous commands (applies to both host and sandbox). + wordFields := parseExecCommandWords(normalizedCommand) + pathBaseDir := ToolWorkspaceFromCtx(ctx) + if pathBaseDir == "" { + pathBaseDir = t.workspace + } for _, pattern := range allPatterns { if pattern.MatchString(normalizedCommand) { // Check if exemption applies. Only exempt if EVERY field that @@ -167,23 +429,20 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *Result { // path traversal ("..") to prevent exemption escape. exempt := false trimmed := strings.TrimSpace(normalizedCommand) - fields := strings.Fields(trimmed) + fields := wordFields + if len(fields) == 0 { + fields = strings.Fields(trimmed) + } matchingFields := 0 exemptFields := 0 for _, field := range fields { - clean := strings.Trim(field, `"'`) + clean := strings.TrimSpace(field) if !pattern.MatchString(clean) { continue // field doesn't trigger this deny pattern } matchingFields++ - if strings.Contains(clean, "..") { - continue // path traversal — never exempt - } - for _, ex := range t.denyExemptions { - if strings.HasPrefix(clean, ex) { - exemptFields++ - break - } + if matchesAnyPathExemption(clean, exemptions, pathBaseDir) { + exemptFields++ } } // Exempt only if at least one field matched AND all matched fields are exempt. diff --git a/internal/tools/shell_deny_test.go b/internal/tools/shell_deny_test.go index 17b100cd..d32a58ab 100644 --- a/internal/tools/shell_deny_test.go +++ b/internal/tools/shell_deny_test.go @@ -2,7 +2,10 @@ package tools import ( "context" + "os" + "path/filepath" "regexp" + "slices" "strings" "testing" ) @@ -19,10 +22,10 @@ func TestBase64DecodeShellDeny(t *testing.T) { } allowed := []string{ - "base64 -w0 file.txt", // encode, no pipe to shell - "base64 -d file.txt", // decode without pipe to shell - "echo hello | base64", // encode - "base64 --decode file.txt", // decode without pipe to shell + "base64 -w0 file.txt", // encode, no pipe to shell + "base64 -d file.txt", // decode without pipe to shell + "echo hello | base64", // encode + "base64 --decode file.txt", // decode without pipe to shell } for _, cmd := range denied { @@ -98,7 +101,7 @@ func TestDestructiveOpsGaps(t *testing.T) { ) mustAllow(t, patterns, - "halting the process", // "halt" inside word + "halting the process", // "halt" inside word "initialize", // "init" inside word "initial setup", // "init" inside word "init_db", // no space+digit after init @@ -171,12 +174,18 @@ func TestExecute_RejectsNULByte(t *testing.T) { } func TestPathExemptions(t *testing.T) { + dataDir := t.TempDir() + workspace := filepath.Join(t.TempDir(), "workspace") + if err := os.MkdirAll(workspace, 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + tool := &ExecTool{ - workspace: "/workspace", + workspace: workspace, restrict: false, } - tool.DenyPaths("/app/data", ".goclaw/") - tool.AllowPathExemptions(".goclaw/skills-store/", "/app/data/skills-store/") + tool.DenyPaths(dataDir, ".goclaw/") + tool.AllowPathExemptions(".goclaw/skills-store/", filepath.Join(dataDir, "skills-store")+"/") cases := []struct { name string @@ -334,30 +343,24 @@ func TestPathExemptions(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - normalizedCmd := normalizeCommand(tc.cmd) + normalizedCmd := strings.ReplaceAll(normalizeCommand(tc.cmd), "/app/data", dataDir) denied := false for _, pattern := range allPatterns { if !pattern.MatchString(normalizedCmd) { continue } // Replicate per-field exemption logic from Execute() - fields := strings.Fields(strings.TrimSpace(normalizedCmd)) + fields := parseExecCommandWords(strings.TrimSpace(normalizedCmd)) matchingFields := 0 exemptFields := 0 for _, field := range fields { - clean := strings.Trim(field, `"'`) + clean := strings.TrimSpace(field) if !pattern.MatchString(clean) { continue } matchingFields++ - if strings.Contains(clean, "..") { - continue // traversal — never exempt - } - for _, ex := range tool.denyExemptions { - if strings.HasPrefix(clean, ex) { - exemptFields++ - break - } + if matchesAnyPathExemption(clean, tool.denyExemptions, workspace) { + exemptFields++ } } exempt := matchingFields > 0 && exemptFields == matchingFields @@ -381,11 +384,16 @@ func TestPathExemptions(t *testing.T) { // path and an exempt path in different arguments is correctly denied. // Per-field matching ensures the non-exempt field causes denial. func TestPathExemptions_MixedArgs(t *testing.T) { + dataDir := t.TempDir() + workspace := filepath.Join(t.TempDir(), "workspace") + if err := os.MkdirAll(workspace, 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } tool := &ExecTool{} - tool.DenyPaths("/app/data") - tool.AllowPathExemptions("/app/data/skills-store/") + tool.DenyPaths(dataDir) + tool.AllowPathExemptions(filepath.Join(dataDir, "skills-store") + "/") - cmd := "cat /app/data/config.json /app/data/skills-store/tool.py" + cmd := "cat " + filepath.Join(dataDir, "config.json") + " " + filepath.Join(dataDir, "skills-store", "tool.py") normalizedCmd := normalizeCommand(cmd) denied := false @@ -393,23 +401,17 @@ func TestPathExemptions_MixedArgs(t *testing.T) { if !pattern.MatchString(normalizedCmd) { continue } - fields := strings.Fields(strings.TrimSpace(normalizedCmd)) + fields := parseExecCommandWords(strings.TrimSpace(normalizedCmd)) matchingFields := 0 exemptFields := 0 for _, field := range fields { - clean := strings.Trim(field, `"'`) + clean := strings.TrimSpace(field) if !pattern.MatchString(clean) { continue } matchingFields++ - if strings.Contains(clean, "..") { - continue - } - for _, ex := range tool.denyExemptions { - if strings.HasPrefix(clean, ex) { - exemptFields++ - break - } + if matchesAnyPathExemption(clean, tool.denyExemptions, workspace) { + exemptFields++ } } if matchingFields == 0 || exemptFields != matchingFields { @@ -422,6 +424,195 @@ func TestPathExemptions_MixedArgs(t *testing.T) { } } +func TestExecute_AllowsCurrentWorkspaceNestedUnderDeniedRoot(t *testing.T) { + dataDir := t.TempDir() + workspace := filepath.Join(dataDir, "teams", "team-123") + if err := os.MkdirAll(workspace, 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + tool := NewExecTool("/workspace", false) + tool.DenyPaths(dataDir) + + target := filepath.Join(workspace, ".uploads", "report.png") + ctx := WithToolWorkspace(context.Background(), workspace) + result := tool.Execute(ctx, map[string]any{ + "command": "printf '%s' " + target, + }) + + if strings.Contains(result.ForLLM, "command denied by safety policy") { + t.Fatalf("expected nested workspace path to bypass dataDir deny, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, target) { + t.Fatalf("expected command output to include workspace path, got: %s", result.ForLLM) + } +} + +func TestExecute_AllowsCurrentTeamWorkspaceNestedUnderDeniedRoot(t *testing.T) { + dataDir := t.TempDir() + teamWorkspace := filepath.Join(dataDir, "tenants", "acme", "teams", "team-123") + if err := os.MkdirAll(teamWorkspace, 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + tool := NewExecTool("/workspace", false) + tool.DenyPaths(dataDir) + + target := filepath.Join(teamWorkspace, "report.png") + ctx := WithToolWorkspace(context.Background(), t.TempDir()) + ctx = WithToolTeamWorkspace(ctx, teamWorkspace) + result := tool.Execute(ctx, map[string]any{ + "command": "printf '%s' " + target, + }) + + if strings.Contains(result.ForLLM, "command denied by safety policy") { + t.Fatalf("expected nested team workspace path to bypass dataDir deny, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, target) { + t.Fatalf("expected command output to include team workspace path, got: %s", result.ForLLM) + } +} + +func TestExecute_DoesNotExemptOtherDataDirPaths(t *testing.T) { + dataDir := t.TempDir() + workspace := filepath.Join(dataDir, "teams", "team-123") + if err := os.MkdirAll(workspace, 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + tool := NewExecTool("/workspace", false) + tool.DenyPaths(dataDir) + + ctx := WithToolWorkspace(context.Background(), workspace) + result := tool.Execute(ctx, map[string]any{ + "command": "printf '%s' " + filepath.Join(dataDir, "config.json"), + }) + + if !strings.Contains(result.ForLLM, "command denied by safety policy") { + t.Fatalf("expected unrelated dataDir path to remain denied, got: %s", result.ForLLM) + } +} + +func TestExecute_DoesNotExemptWorkspaceLocalDotGoclawPaths(t *testing.T) { + dataDir := t.TempDir() + workspace := filepath.Join(dataDir, "teams", "team-123") + if err := os.MkdirAll(workspace, 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + tool := NewExecTool("/workspace", false) + tool.DenyPaths(dataDir, ".goclaw/") + + ctx := WithToolWorkspace(context.Background(), workspace) + result := tool.Execute(ctx, map[string]any{ + "command": "printf '%s' " + filepath.Join(workspace, ".goclaw", "secrets.json"), + }) + + if !strings.Contains(result.ForLLM, "command denied by safety policy") { + t.Fatalf("expected workspace-local .goclaw path to remain denied, got: %s", result.ForLLM) + } +} + +func TestExecute_AllowsQuotedAndPrefixedUploadArguments(t *testing.T) { + dataDir := t.TempDir() + workspace := filepath.Join(dataDir, "teams", "team-123") + if err := os.MkdirAll(filepath.Join(workspace, ".uploads"), 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + target := filepath.Join(workspace, ".uploads", "Quarterly Report.png") + if err := os.WriteFile(target, []byte("ok"), 0644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + tool := NewExecTool("/workspace", false) + tool.DenyPaths(dataDir) + + ctx := WithToolWorkspace(context.Background(), workspace) + result := tool.Execute(ctx, map[string]any{ + "command": "printf '%s' file=@\"" + target + "\"", + }) + if strings.Contains(result.ForLLM, "command denied by safety policy") { + t.Fatalf("expected quoted/prefixed upload argument to bypass deny, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "file=@"+target) { + t.Fatalf("expected output to contain prefixed path, got: %s", result.ForLLM) + } +} + +func TestExecute_DoesNotExemptSymlinkEscapeInsideTeamWorkspace(t *testing.T) { + if err := os.MkdirAll(t.TempDir(), 0755); err != nil { + t.Fatalf("TempDir setup error = %v", err) + } + dataDir := t.TempDir() + workspace := filepath.Join(dataDir, "personal") + teamWorkspace := filepath.Join(dataDir, "teams", "team-123") + if err := os.MkdirAll(workspace, 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + if err := os.MkdirAll(teamWorkspace, 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + + protected := filepath.Join(dataDir, "config.json") + if err := os.WriteFile(protected, []byte("secret"), 0644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + linkPath := filepath.Join(teamWorkspace, "leak.txt") + if err := os.Symlink(protected, linkPath); err != nil { + t.Fatalf("Symlink() error = %v", err) + } + + tool := NewExecTool("/workspace", false) + tool.DenyPaths(dataDir) + + ctx := WithToolWorkspace(context.Background(), workspace) + ctx = WithToolTeamWorkspace(ctx, teamWorkspace) + result := tool.Execute(ctx, map[string]any{ + "command": "printf '%s' " + linkPath, + }) + + if !strings.Contains(result.ForLLM, "command denied by safety policy") { + t.Fatalf("expected symlink escape to remain denied, got: %s", result.ForLLM) + } +} + +func TestExecute_AllowsLegacyWorkspaceUploadsLayout(t *testing.T) { + dataDir := t.TempDir() + workspace := filepath.Join(dataDir, ".goclaw", "goclaw-workspace", "ws", "system") + legacyUploads := filepath.Join(workspace, "uploads") + if err := os.MkdirAll(legacyUploads, 0755); err != nil { + t.Fatalf("MkdirAll() error = %v", err) + } + target := filepath.Join(legacyUploads, "Quarterly Report.png") + if err := os.WriteFile(target, []byte("ok"), 0644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + tool := NewExecTool("/workspace", false) + tool.DenyPaths(dataDir, ".goclaw/") + + ctx := WithToolWorkspace(context.Background(), workspace) + result := tool.Execute(ctx, map[string]any{ + "command": "cp \"" + target + "\" /tmp/partner.png", + }) + + if strings.Contains(result.ForLLM, "command denied by safety policy") { + t.Fatalf("expected legacy uploads layout to bypass deny, got: %s", result.ForLLM) + } +} + +func TestPathAliasVariants_AppWorkspaceMirror(t *testing.T) { + got := pathAliasVariants("/app/workspace/glm-thuc-bo/ws/user/.uploads") + want := "/app/.goclaw/glm-thuc-bo/ws/user/.uploads" + if !slices.Contains(got, want) { + t.Fatalf("expected mirror variant %q in %v", want, got) + } +} + +func TestIsNestedUnderDeniedRoot_RelativeDotGoclaw(t *testing.T) { + tool := &ExecTool{pathDenyRoots: []string{".goclaw/"}} + if !tool.isNestedUnderDeniedRoot("/app/.goclaw/glm-thuc-bo/ws/user/.uploads") { + t.Fatal("expected absolute .goclaw path to be treated as nested under relative deny root") + } +} + func TestLimitedBuffer(t *testing.T) { t.Run("under limit", func(t *testing.T) { lb := &limitedBuffer{max: 100}