diff --git a/cmd/gateway.go b/cmd/gateway.go index 6611d857..1893d33b 100644 --- a/cmd/gateway.go +++ b/cmd/gateway.go @@ -240,6 +240,17 @@ func runGateway() { } else { toolsReg.SetRateLimiter(nil) } + + // Re-apply user-configured allowed paths for the same reason as the rate + // limiter above: setupToolRegistry wired the filesystem tools' AllowPaths + // from the JSON5 default before ApplySystemConfigs overlaid + // system_configs['allowed_paths'], so DB-driven paths never reached the tools. + // Re-run now that cfg reflects the DB value. Safe: server has not started, no + // in-flight tool calls. + if paths := cfg.Agents.Defaults.AllowedPaths; len(paths) > 0 { + applyUserAllowedPaths(toolsReg, paths) + slog.Info("filesystem allowed paths reapplied from system_configs", "paths", len(paths)) + } setupMemoryEmbeddings(pgStores, providerRegistry) usageCapSvc := usagecaps.NewService(pgStores.UsageCaps, pgStores.Providers) diff --git a/cmd/gateway_tools_wiring.go b/cmd/gateway_tools_wiring.go index bf782399..547fdc46 100644 --- a/cmd/gateway_tools_wiring.go +++ b/cmd/gateway_tools_wiring.go @@ -97,47 +97,31 @@ func wireExtraTools( if pgStores.Skills != nil { skillsAllowPaths = append(skillsAllowPaths, pgStores.Skills.Dirs()...) } - // Expand user-configured allowed paths (for cross-drive access on Windows). - // These paths are validated per-request in resolvePath for tenant isolation. - var userAllowPaths []string - for _, p := range agentCfg.AllowedPaths { - expanded := config.ExpandHome(p) - if expanded != "" { - userAllowPaths = append(userAllowPaths, expanded) - } - } - if readTool, ok := toolsReg.Get("read_file"); ok { if pa, ok := readTool.(tools.PathAllowable); ok { pa.AllowPaths(skillsAllowPaths...) pa.AllowPaths(filepath.Join(dataDir, "cli-workspaces")) - pa.AllowPaths(userAllowPaths...) } } if listTool, ok := toolsReg.Get("list_files"); ok { if pa, ok := listTool.(tools.PathAllowable); ok { pa.AllowPaths(skillsAllowPaths...) - pa.AllowPaths(userAllowPaths...) - } - } - // Write and edit tools also get user-configured allowed paths for cross-drive access. - if writeTool, ok := toolsReg.Get("write_file"); ok { - if pa, ok := writeTool.(tools.PathAllowable); ok { - pa.AllowPaths(userAllowPaths...) - } - } - if editTool, ok := toolsReg.Get("edit"); ok { - if pa, ok := editTool.(tools.PathAllowable); ok { - pa.AllowPaths(userAllowPaths...) } } if sendFileTool, ok := toolsReg.Get("send_file"); ok { if pa, ok := sendFileTool.(tools.PathAllowable); ok { pa.AllowPaths(skillsAllowPaths...) - pa.AllowPaths(userAllowPaths...) } } + // User-configured allowed paths (config agents.defaults.allowed_paths, for + // cross-drive access on Windows and shared dirs outside the workspace). + // Applied via a helper so cmd/gateway.go can re-apply it after + // ApplySystemConfigs overlays system_configs['allowed_paths'] — see + // applyUserAllowedPaths. Paths are validated per-request in resolvePath for + // tenant isolation. + applyUserAllowedPaths(toolsReg, agentCfg.AllowedPaths) + // Memory tools are PG-backed; always available. hasMemory = true @@ -162,6 +146,39 @@ func wireExtraTools( return heartbeatTool, hasMemory } +// fsAllowPathTools are the filesystem tools that honour user-configured allowed +// paths beyond the agent workspace (config agents.defaults.allowed_paths). +var fsAllowPathTools = []string{"read_file", "list_files", "write_file", "edit", "send_file"} + +// applyUserAllowedPaths grants the user-configured allowed paths to the +// filesystem tools. ExpandHome resolves a leading "~". +// +// Called twice during startup: once while tools are wired (from config.json), +// and again from cmd/gateway.go after ApplySystemConfigs overlays +// system_configs['allowed_paths']. Tool wiring runs before that overlay, so +// without the re-apply DB-driven allowed paths never reach the tools — the +// AllowPaths analogue of the rate-limiter re-apply (#1111). Safe to call +// repeatedly: AllowPaths is additive and the prefix check is membership-based, +// so a duplicated prefix is harmless. +func applyUserAllowedPaths(toolsReg *tools.Registry, allowedPaths []string) { + var paths []string + for _, p := range allowedPaths { + if expanded := config.ExpandHome(p); expanded != "" { + paths = append(paths, expanded) + } + } + if len(paths) == 0 { + return + } + for _, name := range fsAllowPathTools { + if t, ok := toolsReg.Get(name); ok { + if pa, ok := t.(tools.PathAllowable); ok { + pa.AllowPaths(paths...) + } + } + } +} + // wireWorkstationTools registers workstation_exec and claude_remote tools (Standard edition only). // Phase 6: wires the real AllowlistChecker permission check replacing the deny-all sentinel. // Phase 7: wires the activity sink for exec audit logging. diff --git a/cmd/gateway_tools_wiring_test.go b/cmd/gateway_tools_wiring_test.go new file mode 100644 index 00000000..e774d432 --- /dev/null +++ b/cmd/gateway_tools_wiring_test.go @@ -0,0 +1,86 @@ +package cmd + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/tools" +) + +// applyUserAllowedPaths must grant the filesystem tools access to paths outside +// the agent workspace. cmd/gateway.go relies on this to re-apply +// system_configs['allowed_paths'] after the overlay (tool wiring runs first) — +// the AllowPaths analogue of the rate-limiter re-apply in #1111. +func TestApplyUserAllowedPaths_GrantsExternalPathToReadFile(t *testing.T) { + ws := t.TempDir() + ext := t.TempDir() + extFile := filepath.Join(ext, "kb.md") + if err := os.WriteFile(extFile, []byte("knowledge"), 0o644); err != nil { + t.Fatal(err) + } + + reg := tools.NewRegistry() + reg.Register(tools.NewReadFileTool(ws, true)) + read, ok := reg.Get("read_file") + if !ok { + t.Fatal("read_file not registered") + } + + // Before granting: a path outside the workspace is denied. + if res := read.Execute(context.Background(), map[string]any{"path": extFile}); !res.IsError { + t.Fatalf("expected access denied before allow, got: %s", res.ForLLM) + } + + applyUserAllowedPaths(reg, []string{ext}) + + // After granting: the external path is readable. + if res := read.Execute(context.Background(), map[string]any{"path": extFile}); res.IsError { + t.Fatalf("expected success after allow, got error: %s", res.ForLLM) + } + + // The grant is scoped — an unrelated external path stays denied. + other := t.TempDir() + otherFile := filepath.Join(other, "secret.md") + if err := os.WriteFile(otherFile, []byte("nope"), 0o644); err != nil { + t.Fatal(err) + } + if res := read.Execute(context.Background(), map[string]any{"path": otherFile}); !res.IsError { + t.Fatalf("expected unrelated external path to stay denied, got: %s", res.ForLLM) + } +} + +// Applying twice (initial wiring + the gateway re-apply after ApplySystemConfigs) +// must keep the grant working and must not error. +func TestApplyUserAllowedPaths_RepeatedApplyIsSafe(t *testing.T) { + ws := t.TempDir() + ext := t.TempDir() + extFile := filepath.Join(ext, "kb.md") + if err := os.WriteFile(extFile, []byte("knowledge"), 0o644); err != nil { + t.Fatal(err) + } + + reg := tools.NewRegistry() + reg.Register(tools.NewReadFileTool(ws, true)) + read, ok := reg.Get("read_file") + if !ok { + t.Fatal("read_file not registered") + } + + applyUserAllowedPaths(reg, []string{ext}) // initial wiring (from config.json) + applyUserAllowedPaths(reg, []string{ext}) // re-apply after system_configs overlay + + if res := read.Execute(context.Background(), map[string]any{"path": extFile}); res.IsError { + t.Fatalf("expected success after repeated allow, got error: %s", res.ForLLM) + } +} + +// An empty allow list is a no-op and must not panic (the common case when no +// allowed_paths are configured). +func TestApplyUserAllowedPaths_EmptyIsNoop(t *testing.T) { + reg := tools.NewRegistry() + reg.Register(tools.NewReadFileTool(t.TempDir(), true)) + applyUserAllowedPaths(reg, nil) + applyUserAllowedPaths(reg, []string{}) +}