diff --git a/README.md b/README.md index 09aebcdff..30f965c87 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,8 @@ 🧠 **Smart routing**: Rule-based model routing — simple queries go to lightweight models, saving API costs. +šŸ›”ļø **Hardened Multi-User Isolation**: Built-in [Tenant Isolation](docs/configuration.md#šŸ”’-multi-tenant-agent-isolation) for shared infrastructure (Azure/ACA) — automatically partitions workspaces, memory, and tools (including MCP) per-user session. + _*Recent builds may use 10-20MB due to rapid PR merges. Resource optimization is planned. Boot speed comparison based on 0.8GHz single-core benchmarks (see table below)._
diff --git a/cmd/picoclaw/internal/onboard/command_test.go b/cmd/picoclaw/internal/onboard/command_test.go index 56936190b..eb2c57f3d 100644 --- a/cmd/picoclaw/internal/onboard/command_test.go +++ b/cmd/picoclaw/internal/onboard/command_test.go @@ -28,5 +28,10 @@ func TestNewOnboardCommand(t *testing.T) { encFlag := cmd.Flags().Lookup("enc") require.NotNil(t, encFlag, "expected --enc flag to be registered") assert.Equal(t, "false", encFlag.DefValue, "--enc should default to false") - assert.False(t, cmd.HasSubCommands()) + yesFlag := cmd.Flags().Lookup("yes") + require.NotNil(t, yesFlag, "expected --yes flag to be registered") + assert.Equal(t, "false", yesFlag.DefValue, "--yes should default to false") + assert.True(t, cmd.HasSubCommands()) + assert.Len(t, cmd.Commands(), 1) + assert.Equal(t, "purge", cmd.Commands()[0].Name()) } diff --git a/cmd/picoclaw/internal/onboard/purge.go b/cmd/picoclaw/internal/onboard/purge.go index 456ee22db..76138e0f4 100644 --- a/cmd/picoclaw/internal/onboard/purge.go +++ b/cmd/picoclaw/internal/onboard/purge.go @@ -35,7 +35,7 @@ func NewPurgeCommand() *cobra.Command { } fmt.Println("Purging...") - + if err := os.RemoveAll(workspace); err != nil { fmt.Printf("Error deleting workspace: %v\n", err) } else { @@ -47,7 +47,7 @@ func NewPurgeCommand() *cobra.Command { } else { fmt.Println("āœ“ Logs deleted") } - + fmt.Println("Purge complete.") }, } diff --git a/cmd/picoclaw/main.go b/cmd/picoclaw/main.go index 434917c0b..57c303501 100644 --- a/cmd/picoclaw/main.go +++ b/cmd/picoclaw/main.go @@ -86,6 +86,7 @@ func main() { cmd := NewPicoclawCommand() if err := cmd.Execute(); err != nil { + fmt.Fprintf(os.Stderr, "\nāŒ FATAL: %v\n", err) os.Exit(1) } } diff --git a/config/config.json.azure b/config/config.json.azure new file mode 100644 index 000000000..9a7ff3397 --- /dev/null +++ b/config/config.json.azure @@ -0,0 +1,568 @@ +{ + "session": { + "dm_scope": "per-channel-peer" + }, + "version": 1, + "agents": { + "defaults": { + "workspace": "", + "restrict_to_workspace": true, + "allow_read_outside_workspace": false, + "provider": "openai", + "model_name": "azure-grok", + "max_tokens": 32768, + "max_tool_iterations": 50, + "summarize_message_threshold": 20, + "summarize_token_percent": 75, + "steering_mode": "one-at-a-time", + "subturn": { + "max_depth": 10, + "max_concurrent": 5, + "default_timeout_minutes": 20, + "default_token_budget": 100000, + "concurrency_timeout_sec": 10 + }, + "tool_feedback": { + "enabled": true, + "max_args_length": 300 + } + } + }, + "channels": { + "whatsapp": { + "enabled": false, + "bridge_url": "ws://localhost:3001", + "use_native": false, + "session_store_path": "", + "allow_from": [], + "reasoning_channel_id": "" + }, + "telegram": { + "enabled": false, + "base_url": "", + "proxy": "", + "allow_from": [], + "group_trigger": {}, + "typing": { + "enabled": true + }, + "placeholder": { + "enabled": true, + "text": "Thinking... šŸ’­" + }, + "streaming": { + "enabled": true, + "throttle_seconds": 3, + "min_growth_chars": 200 + }, + "reasoning_channel_id": "", + "use_markdown_v2": false + }, + "feishu": { + "enabled": false, + "app_id": "", + "allow_from": [], + "group_trigger": {}, + "placeholder": {}, + "reasoning_channel_id": "", + "random_reaction_emoji": null, + "is_lark": false + }, + "discord": { + "enabled": false, + "proxy": "", + "allow_from": [], + "mention_only": false, + "group_trigger": {}, + "typing": {}, + "placeholder": {}, + "reasoning_channel_id": "" + }, + "maixcam": { + "enabled": false, + "host": "0.0.0.0", + "port": 18790, + "allow_from": [], + "reasoning_channel_id": "" + }, + "qq": { + "enabled": false, + "app_id": "", + "allow_from": [], + "group_trigger": {}, + "max_message_length": 2000, + "max_base64_file_size_mib": 0, + "send_markdown": false, + "reasoning_channel_id": "" + }, + "dingtalk": { + "enabled": false, + "client_id": "", + "allow_from": [], + "group_trigger": {}, + "reasoning_channel_id": "" + }, + "slack": { + "enabled": false, + "allow_from": [], + "group_trigger": {}, + "typing": {}, + "placeholder": {}, + "reasoning_channel_id": "" + }, + "matrix": { + "enabled": false, + "homeserver": "https://matrix.org", + "user_id": "", + "join_on_invite": true, + "allow_from": [], + "group_trigger": { + "mention_only": true + }, + "placeholder": { + "enabled": true, + "text": "Thinking... šŸ’­" + }, + "reasoning_channel_id": "" + }, + "line": { + "enabled": false, + "webhook_host": "0.0.0.0", + "webhook_port": 18791, + "webhook_path": "/webhook/line", + "allow_from": [], + "group_trigger": { + "mention_only": true + }, + "typing": {}, + "placeholder": {}, + "reasoning_channel_id": "" + }, + "onebot": { + "enabled": false, + "ws_url": "ws://127.0.0.1:3001", + "reconnect_interval": 5, + "group_trigger_prefix": null, + "allow_from": [], + "group_trigger": {}, + "typing": {}, + "placeholder": {}, + "reasoning_channel_id": "" + }, + "wecom": { + "enabled": false, + "webhook_url": "", + "webhook_host": "0.0.0.0", + "webhook_port": 18793, + "webhook_path": "/webhook/wecom", + "allow_from": [], + "reply_timeout": 5, + "group_trigger": {}, + "reasoning_channel_id": "" + }, + "wecom_app": { + "enabled": false, + "corp_id": "", + "agent_id": 0, + "webhook_host": "0.0.0.0", + "webhook_port": 18792, + "webhook_path": "/webhook/wecom-app", + "allow_from": [], + "reply_timeout": 5, + "group_trigger": {}, + "reasoning_channel_id": "" + }, + "wecom_aibot": { + "enabled": false, + "webhook_path": "/webhook/wecom-aibot", + "allow_from": [], + "reply_timeout": 5, + "max_steps": 10, + "welcome_message": "Hello! I'm your AI assistant. How can I help you today?", + "processing_message": "ā³ Processing, please wait. The results will be sent shortly.", + "reasoning_channel_id": "" + }, + "weixin": { + "enabled": false, + "base_url": "https://ilinkai.weixin.qq.com/", + "cdn_base_url": "https://novac2c.cdn.weixin.qq.com/c2c", + "proxy": "", + "allow_from": [], + "reasoning_channel_id": "" + }, + "pico": { + "enabled": false, + "ping_interval": 30, + "read_timeout": 60, + "write_timeout": 10, + "max_connections": 100, + "allow_from": [], + "placeholder": {} + }, + "pico_client": { + "enabled": false, + "url": "", + "token": "", + "allow_from": null + }, + "irc": { + "enabled": false, + "server": "", + "tls": false, + "nick": "", + "sasl_user": "", + "channels": null, + "allow_from": null, + "group_trigger": {}, + "typing": {}, + "reasoning_channel_id": "" + } + }, + "model_list": [ + { + "model_name": "glm-4.7", + "model": "zhipu/glm-4.7", + "api_base": "https://open.bigmodel.cn/api/paas/v4" + }, + { + "model_name": "gpt-5.4", + "model": "openai/gpt-5.4", + "api_base": "https://api.openai.com/v1" + }, + { + "model_name": "claude-sonnet-4.6", + "model": "anthropic/claude-sonnet-4.6", + "api_base": "https://api.anthropic.com/v1" + }, + { + "model_name": "deepseek-chat", + "model": "deepseek/deepseek-chat", + "api_base": "https://api.deepseek.com/v1" + }, + { + "model_name": "gemini-2.0-flash", + "model": "gemini/gemini-2.0-flash-exp", + "api_base": "https://generativelanguage.googleapis.com/v1beta" + }, + { + "model_name": "qwen-plus", + "model": "qwen/qwen-plus", + "api_base": "https://dashscope.aliyuncs.com/compatible-mode/v1" + }, + { + "model_name": "moonshot-v1-8k", + "model": "moonshot/moonshot-v1-8k", + "api_base": "https://api.moonshot.cn/v1" + }, + { + "model_name": "llama-3.3-70b", + "model": "groq/llama-3.3-70b-versatile", + "api_base": "https://api.groq.com/openai/v1" + }, + { + "model_name": "openrouter-auto", + "model": "openrouter/auto", + "api_base": "https://openrouter.ai/api/v1" + }, + { + "model_name": "openrouter-gpt-5.4", + "model": "openrouter/openai/gpt-5.4", + "api_base": "https://openrouter.ai/api/v1" + }, + { + "model_name": "nemotron-4-340b", + "model": "nvidia/nemotron-4-340b-instruct", + "api_base": "https://integrate.api.nvidia.com/v1" + }, + { + "model_name": "azure-grok", + "model": "openai/grok-4-fast-non-reasoning", + "api_base": "https://TestSJF.openai.azure.com/openai/v1/", + "api_key": "REDACTED" + }, + { + "model_name": "cerebras-llama-3.3-70b", + "model": "cerebras/llama-3.3-70b", + "api_base": "https://api.cerebras.ai/v1" + }, + { + "model_name": "vivgrid-auto", + "model": "vivgrid/auto", + "api_base": "https://api.vivgrid.com/v1" + }, + { + "model_name": "ark-code-latest", + "model": "volcengine/ark-code-latest", + "api_base": "https://ark.cn-beijing.volces.com/api/v3" + }, + { + "model_name": "doubao-pro", + "model": "volcengine/doubao-pro-32k", + "api_base": "https://ark.cn-beijing.volces.com/api/v3" + }, + { + "model_name": "deepseek-v3", + "model": "shengsuanyun/deepseek-v3", + "api_base": "https://api.shengsuanyun.com/v1" + }, + { + "model_name": "gemini-flash", + "model": "antigravity/gemini-3-flash", + "auth_method": "oauth" + }, + { + "model_name": "copilot-gpt-5.4", + "model": "github-copilot/gpt-5.4", + "api_base": "http://localhost:4321", + "auth_method": "oauth" + }, + { + "model_name": "llama3", + "model": "ollama/llama3", + "api_base": "http://localhost:11434/v1" + }, + { + "model_name": "mistral-small", + "model": "mistral/mistral-small-latest", + "api_base": "https://api.mistral.ai/v1" + }, + { + "model_name": "deepseek-v3.2", + "model": "avian/deepseek/deepseek-v3.2", + "api_base": "https://api.avian.io/v1" + }, + { + "model_name": "kimi-k2.5", + "model": "avian/moonshotai/kimi-k2.5", + "api_base": "https://api.avian.io/v1" + }, + { + "model_name": "MiniMax-M2.5", + "model": "minimax/MiniMax-M2.5", + "api_base": "https://api.minimaxi.com/v1", + "extra_body": { + "reasoning_split": true + } + }, + { + "model_name": "LongCat-Flash-Thinking", + "model": "longcat/LongCat-Flash-Thinking", + "api_base": "https://api.longcat.chat/openai" + }, + { + "model_name": "modelscope-qwen", + "model": "modelscope/Qwen/Qwen3-235B-A22B-Instruct-2507", + "api_base": "https://api-inference.modelscope.cn/v1" + }, + { + "model_name": "local-model", + "model": "vllm/custom-model", + "api_base": "http://localhost:8000/v1" + }, + { + "model_name": "azure-gpt5", + "model": "azure/my-gpt5-deployment", + "api_base": "https://your-resource.openai.azure.com" + } + ], + "gateway": { + "host": "0.0.0.0", + "port": 18790, + "chat_enabled": true, + "hot_reload": true, + "log_level": "info", + "api_key": "picoclaw-secret-123" + }, + "hooks": { + "enabled": true, + "defaults": { + "observer_timeout_ms": 500, + "interceptor_timeout_ms": 5000, + "approval_timeout_ms": 60000 + } + }, + "tools": { + "filter_sensitive_data": true, + "filter_min_length": 8, + "allow_read_paths": null, + "allow_write_paths": null, + "deny_read_paths": [ + "^skills(/.*)?$" + ], + "deny_write_paths": [ + "^skills(/.*)?$" + ], + "web": { + "enabled": true, + "brave": { + "enabled": false, + "max_results": 5 + }, + "tavily": { + "enabled": false, + "base_url": "", + "max_results": 5 + }, + "duckduckgo": { + "enabled": true, + "max_results": 5 + }, + "perplexity": { + "enabled": false, + "max_results": 5 + }, + "searxng": { + "enabled": false, + "base_url": "", + "max_results": 5 + }, + "glm_search": { + "enabled": false, + "base_url": "https://open.bigmodel.cn/api/paas/v4/web_search", + "search_engine": "search_std", + "max_results": 5 + }, + "baidu_search": { + "enabled": false, + "base_url": "https://qianfan.baidubce.com/v2/ai_search/web_search", + "max_results": 10 + }, + "prefer_native": true, + "fetch_limit_bytes": 10485760, + "format": "plaintext" + }, + "cron": { + "enabled": true, + "exec_timeout_minutes": 5, + "allow_command": true + }, + "exec": { + "enabled": true, + "enable_deny_patterns": true, + "allow_remote": true, + "custom_deny_patterns": null, + "custom_allow_patterns": null, + "timeout_seconds": 60 + }, + "skills": { + "whitelist_enabled": true, + "whitelist": [ + "weather", + "summarize" + ], + "enabled": true, + "registries": { + "clawhub": { + "enabled": true, + "base_url": "https://clawhub.ai", + "search_path": "", + "skills_path": "", + "download_path": "", + "timeout": 0, + "max_zip_size": 0, + "max_response_size": 0 + } + }, + "github": {}, + "max_concurrent_searches": 2, + "search_cache": { + "max_size": 50, + "ttl_seconds": 300 + } + }, + "media_cleanup": { + "enabled": true, + "max_age_minutes": 30, + "interval_minutes": 5 + }, + "mcp": { + "enabled": true, + "discovery": { + "enabled": false, + "ttl": 5, + "max_search_results": 5, + "use_bm25": true, + "use_regex": false + }, + "servers": {} + }, + "whitelist": [ + "spawn", + "subagent", + "read_file", + "list_dir", + "write_file", + "edit_file", + "append_file", + "message", + "weather", + "summarize", + "github", + "search_tool" + ], + "whitelist_enabled": true, + "append_file": { + "enabled": true + }, + "edit_file": { + "enabled": true + }, + "find_skills": { + "enabled": true + }, + "i2c": { + "enabled": false + }, + "install_skill": { + "enabled": true + }, + "list_dir": { + "enabled": true + }, + "message": { + "enabled": true + }, + "read_file": { + "enabled": true, + "max_read_file_size": 65536 + }, + "send_file": { + "enabled": true + }, + "spawn": { + "enabled": true + }, + "spawn_status": { + "enabled": false + }, + "spi": { + "enabled": false + }, + "subagent": { + "enabled": true + }, + "web_fetch": { + "enabled": true + }, + "write_file": { + "enabled": true + } + }, + "heartbeat": { + "enabled": true, + "interval": 30 + }, + "devices": { + "enabled": false, + "monitor_usb": true + }, + "voice": { + "echo_transcription": false + }, + "build_info": { + "version": "0.1.0", + "git_commit": "054b55fd", + "build_time": "2026-03-23T10:15:13+0100", + "go_version": "go1.26.1" + } +} \ No newline at end of file diff --git a/docker/Dockerfile.full b/docker/Dockerfile.full index 30e1680d5..aa85ee4cc 100644 --- a/docker/Dockerfile.full +++ b/docker/Dockerfile.full @@ -37,7 +37,18 @@ RUN curl -LsSf https://astral.sh/uv/install.sh | sh && \ # Copy binary COPY --from=builder /src/build/picoclaw /usr/local/bin/picoclaw -# Create picoclaw home directory +# Create non-root user and group +# node image already has a 'node' user with UID 1000, so we remove it first +RUN deluser --remove-home node || true && \ + addgroup -g 1000 picoclaw && \ + adduser -D -u 1000 -G picoclaw picoclaw + +# Switch to non-root user +USER picoclaw +WORKDIR /home/picoclaw + +# Run onboard to create initial directories and config +# HOME will be /home/picoclaw RUN /usr/local/bin/picoclaw onboard ENTRYPOINT ["picoclaw"] diff --git a/docs/configuration.md b/docs/configuration.md index e94374160..fc1cc061b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -77,7 +77,7 @@ When an incoming message includes a **ChatID** (passed in the `/chat` API or ext 1. **Isolated Workspace:** The agent's operations are restricted to `workspace/sessions/{isolationID}/workspace`. 2. **Isolated Memory:** Long-term memory (`MEMORY.md`) is stored and read from the isolated session path. -3. **Isolated Tools:** Tools like `read_file` and `write_file` are automatically pointed to the isolated workspace, preventing any tenant from accessing another's files or the global base workspace. +3. **Isolated Tools:** Tools like `read_file` and `write_file` are automatically pointed to the isolated workspace. Additionally, **MCP server tools** (e.g., Harvest, Monday) and discovery search tools are dynamically registered to each isolated instance, ensuring they inherit the same security boundaries. #### Tenant Identification (Inbound Integration) @@ -98,6 +98,33 @@ If no `ChatID` is detected, the request is routed to the **Global Agent** contex - **Session Agents:** Every request with a `chatID` creates a transient isolated agent instance that "routes" all file and memory operations into its session-specific subdirectory. This mechanism is transparent to the end-user and the AI agent itself, ensuring a secure and portable multi-user environment out-of-the-box. + +### šŸš€ Onboarding & Automation + +For automated deployments (like Azure Container Apps or CI/CD), the `onboard` command supports non-interactive execution and environment cleanup. + +#### Automated Setup + +Use the `--yes` (or `-y`) flag to skip all interactive prompts and automatically generate default credentials/keys: + +```bash +picoclaw onboard --yes +``` + +#### Environment Purge + +If you need to reset an environment (e.g., before a clean redeploy), use the `purge` subcommand. This removes existing workspaces, logs, and generated keys: + +```bash +# Safe purge (checks if files exist) +picoclaw onboard purge + +# Force purge (no confirmation) +picoclaw onboard purge --force +``` + +> [!WARNING] +> The `purge` command is destructive. It will delete your local session history, memory, and encrypted secrets. Only use it when you are prepared to start from a clean slate. ### Skill Sources diff --git a/docs/docker.md b/docs/docker.md index 6c32879a6..69cff013b 100644 --- a/docs/docker.md +++ b/docs/docker.md @@ -67,6 +67,21 @@ docker compose -f docker/docker-compose.yml pull docker compose -f docker/docker-compose.yml --profile gateway up -d ``` +### šŸ”’ Hardened & Non-Root Deployment + +For production environments (like Azure Container Apps or Kubernetes), use the **full hardened image** (`docker/Dockerfile.full`). + +This image provides several security and reliability enhancements: +- **Non-Root Execution**: Runs as the `picoclaw` user (UID 1000) instead of root, meeting strict security requirements. +- **Volume Compatibility**: Fixed UID 1000 ensures compatibility with Azure Files and other cloud volume mounts without manual `chown` hacks. +- **Self-Contained**: Includes the full system suite (Node.js, Python, etc.) required for all tools. +- **Automated Onboarding**: The image entrypoint automatically triggers `picoclaw onboard --yes` if the environment is not initialized. + +To build it manually: +```bash +docker build -f docker/Dockerfile.full -t picoclaw-full:latest . +``` + ### šŸš€ Quick Start > [!TIP] diff --git a/docs/tools_configuration.md b/docs/tools_configuration.md index 5a4b5bb28..045127c9c 100644 --- a/docs/tools_configuration.md +++ b/docs/tools_configuration.md @@ -37,6 +37,34 @@ See [Sensitive Data Filtering](../sensitive_data_filtering.md) for full document | `filter_sensitive_data` | bool | `true` | Enable/disable filtering | | `filter_min_length` | int | `8` | Minimum content length to trigger filtering | +## File Paths & Workspace Security + +PicoClaw provides path-level security for all filesystem-related tools (`read_file`, `write_file`, `list_dir`, `edit_file`, `append_file`). This allows you to restrict the agent's access to specific patterns or block sensitive directories (like a `skills/` folder) even if they are inside the workspace. + +| Config | Type | Default | Description | +|--------|------|---------|-------------| +| `allow_read_paths` | array | `[]` | Explicit regex patterns to allow reading from (even outside workspace) | +| `allow_write_paths` | array | `[]` | Explicit regex patterns to allow writing to (even outside workspace) | +| `deny_read_paths` | array | `[]` | Regex patterns to explicitly block from reading (overrides workspace access) | +| `deny_write_paths` | array | `[]` | Regex patterns to explicitly block from writing (overrides workspace access) | + +### Path Deny Patterns + +Deny patterns are useful for "hardening" a workspace. For example, to prevent an agent from manually tampering with its own skill configuration (the `skills/` directory), you can apply global block rules. + +**Blocking the skills directory:** + +```json +{ + "tools": { + "deny_read_paths": ["^skills(/.*)?$"], + "deny_write_paths": ["^skills(/.*)?$"] + } +} +``` + +> **Note:** Deny patterns apply to the relative path within the workspace (when restricted) or the absolute path (when unrestricted). They take precedence over workspace access and whitelist patterns. + ## Web Tools Web tools are used for web search and fetching. diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index 73d90dac5..f4c9a27ed 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -73,6 +73,8 @@ func NewAgentInstance( // Compile path whitelist patterns from config. allowReadPaths := buildAllowReadPatterns(cfg) allowWritePaths := compilePatterns(cfg.Tools.AllowWritePaths) + denyReadPaths := compilePatterns(cfg.Tools.DenyReadPaths) + denyWritePaths := compilePatterns(cfg.Tools.DenyWritePaths) toolsRegistry := tools.NewToolRegistry() @@ -80,16 +82,16 @@ func NewAgentInstance( maxReadFileSize := cfg.Tools.ReadFile.MaxReadFileSize switch cfg.Tools.ReadFile.EffectiveMode() { case config.ReadFileModeLines: - toolsRegistry.Register(tools.NewReadFileLinesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths)) + toolsRegistry.Register(tools.NewReadFileLinesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths, denyReadPaths)) default: - toolsRegistry.Register(tools.NewReadFileBytesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths)) + toolsRegistry.Register(tools.NewReadFileBytesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths, denyReadPaths)) } } if cfg.Tools.IsToolEnabled("write_file") { - toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict, allowWritePaths)) + toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict, allowWritePaths, denyWritePaths)) } if cfg.Tools.IsToolEnabled("list_dir") { - toolsRegistry.Register(tools.NewListDirTool(workspace, readRestrict, allowReadPaths)) + toolsRegistry.Register(tools.NewListDirTool(workspace, readRestrict, allowReadPaths, denyReadPaths)) } if cfg.Tools.IsToolEnabled("exec") { execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg, allowReadPaths) @@ -102,10 +104,10 @@ func NewAgentInstance( } if cfg.Tools.IsToolEnabled("edit_file") { - toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict, allowWritePaths)) + toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict, allowWritePaths, denyWritePaths)) } if cfg.Tools.IsToolEnabled("append_file") { - toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict, allowWritePaths)) + toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict, allowWritePaths, denyWritePaths)) } // Use main agent workspace (no isolation) for sessions so that session history diff --git a/pkg/agent/instance_test.go b/pkg/agent/instance_test.go index 93649f8ec..209477a50 100644 --- a/pkg/agent/instance_test.go +++ b/pkg/agent/instance_test.go @@ -190,7 +190,7 @@ func TestNewAgentInstance_PreservesDistinctLimiterIdentityForSharedResolvedModel }, } - agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, &mockProvider{}) + agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, &mockProvider{}, "") if len(agent.Candidates) != 2 { t.Fatalf("len(Candidates) = %d, want 2", len(agent.Candidates)) } @@ -319,7 +319,7 @@ func TestNewAgentInstance_ReadFileModeSelectsSchema(t *testing.T) { }, } - agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, &mockProvider{}) + agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, &mockProvider{}, "") readTool, ok := agent.Tools.Get("read_file") if !ok { t.Fatal("read_file tool not registered") diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 864e5ecc2..b01038a8d 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -178,6 +178,7 @@ func registerSharedTools( provider providers.LLMProvider, ) { allowReadPaths := buildAllowReadPatterns(cfg) + denyReadPaths := compilePatterns(cfg.Tools.DenyReadPaths) var ttsProvider tts.TTSProvider if cfg.Tools.IsToolEnabled("send_tts") { ttsProvider = tts.DetectTTS(cfg) @@ -296,14 +297,15 @@ func registerSharedTools( agent.Workspace, cfg.Agents.Defaults.RestrictToWorkspace, cfg.Agents.Defaults.GetMaxMediaSize(), - nil, + al.mediaStore, allowReadPaths, + denyReadPaths, ) agent.Tools.Register(sendFileTool) } if ttsProvider != nil { - agent.Tools.Register(tools.NewSendTTSTool(ttsProvider, nil)) + agent.Tools.Register(tools.NewSendTTSTool(ttsProvider, al.mediaStore)) } if cfg.Tools.IsToolEnabled("load_image") { @@ -467,6 +469,9 @@ func registerSharedTools( } else if (spawnEnabled || spawnStatusEnabled) && !cfg.Tools.IsToolEnabled("subagent") { logger.WarnCF("agent", "spawn/spawn_status tools require subagent to be enabled", nil) } + // Register MCP and discovery tools to this agent + al.RegisterMCPToolsToAgent(agentID, agent) + // Apply global tools whitelist agent.Tools.Filter(cfg.Tools.Whitelist, cfg.Tools.WhitelistEnabled) } @@ -1144,6 +1149,12 @@ func (al *AgentLoop) GetConfig() *config.Config { } // SetMediaStore injects a MediaStore for media lifecycle management. +func (al *AgentLoop) GetMediaStore() media.MediaStore { + al.mu.RLock() + defer al.mu.RUnlock() + return al.mediaStore +} + func (al *AgentLoop) SetMediaStore(s media.MediaStore) { al.mediaStore = s diff --git a/pkg/agent/loop_mcp.go b/pkg/agent/loop_mcp.go index 83cdb2756..b00a9d8a0 100644 --- a/pkg/agent/loop_mcp.go +++ b/pkg/agent/loop_mcp.go @@ -8,7 +8,6 @@ package agent import ( "context" - "fmt" "sync" "github.com/sipeed/picoclaw/pkg/config" @@ -57,6 +56,12 @@ func (r *mcpRuntime) hasManager() bool { return r.manager != nil } +func (r *mcpRuntime) getManager() *mcp.Manager { + r.mu.Lock() + defer r.mu.Unlock() + return r.manager +} + // ensureMCPInitialized loads MCP servers/tools once so both Run() and direct // agent mode share the same initialization path. func (al *AgentLoop) EnsureMCPInitialized(ctx context.Context) error { @@ -103,110 +108,100 @@ func (al *AgentLoop) EnsureMCPInitialized(ctx context.Context) error { return } - // Register MCP tools for all agents - servers := mcpManager.GetServers() - uniqueTools := 0 - totalRegistrations := 0 - agentIDs := al.registry.ListAgentIDs() - agentCount := len(agentIDs) - - for serverName, conn := range servers { - uniqueTools += len(conn.Tools) - - // Determine whether this server's tools should be deferred (hidden). - // Per-server "deferred" field takes precedence over the global Discovery.Enabled. - serverCfg := al.cfg.Tools.MCP.Servers[serverName] - registerAsHidden := serverIsDeferred(al.cfg.Tools.MCP.Discovery.Enabled, serverCfg) - - for _, tool := range conn.Tools { - for _, agentID := range agentIDs { - agent, ok := al.registry.GetAgent(agentID) - if !ok { - continue - } - - mcpTool := tools.NewMCPTool(mcpManager, serverName, tool) - - if registerAsHidden { - agent.Tools.RegisterHidden(mcpTool) - } else { - agent.Tools.Register(mcpTool) - } - - totalRegistrations++ - logger.DebugCF("agent", "Registered MCP tool", - map[string]any{ - "agent_id": agentID, - "server": serverName, - "tool": tool.Name, - "name": mcpTool.Name(), - "deferred": registerAsHidden, - }) - } - } - } - logger.InfoCF("agent", "MCP tools registered successfully", - map[string]any{ - "server_count": len(servers), - "unique_tools": uniqueTools, - "total_registrations": totalRegistrations, - "agent_count": agentCount, - }) - - // Initializes Discovery Tools only if enabled by configuration - if al.cfg.Tools.MCP.Enabled && al.cfg.Tools.MCP.Discovery.Enabled { - useBM25 := al.cfg.Tools.MCP.Discovery.UseBM25 - useRegex := al.cfg.Tools.MCP.Discovery.UseRegex - - // Fail fast: If discovery is enabled but no search method is turned on - if !useBM25 && !useRegex { - al.mcp.setInitErr(fmt.Errorf( - "tool discovery is enabled but neither 'use_bm25' nor 'use_regex' is set to true in the configuration", - )) - if closeErr := mcpManager.Close(); closeErr != nil { - logger.ErrorCF("agent", "Failed to close MCP manager", - map[string]any{ - "error": closeErr.Error(), - }) - } - return - } - - ttl := al.cfg.Tools.MCP.Discovery.TTL - if ttl <= 0 { - ttl = 5 // Default value - } - - maxSearchResults := al.cfg.Tools.MCP.Discovery.MaxSearchResults - if maxSearchResults <= 0 { - maxSearchResults = 5 // Default value - } - - logger.InfoCF("agent", "Initializing tool discovery", map[string]any{ - "bm25": useBM25, "regex": useRegex, "ttl": ttl, "max_results": maxSearchResults, - }) - - for _, agentID := range agentIDs { - agent, ok := al.registry.GetAgent(agentID) - if !ok { - continue - } - - if useRegex { - agent.Tools.Register(tools.NewRegexSearchTool(agent.Tools, ttl, maxSearchResults)) - } - if useBM25 { - agent.Tools.Register(tools.NewBM25SearchTool(agent.Tools, ttl, maxSearchResults)) - } - } - } - al.mcp.setManager(mcpManager) + + // Register MCP and discovery tools for all currently known agents + agentIDs := al.registry.ListAgentIDs() + for _, agentID := range agentIDs { + agent, ok := al.registry.GetAgent(agentID) + if !ok { + continue + } + al.RegisterMCPToolsToAgent(agentID, agent) + } + + logger.InfoCF("agent", "MCP initialization complete", + map[string]any{ + "server_count": len(mcpManager.GetServers()), + "agent_count": len(agentIDs), + }) }) return al.mcp.getInitErr() } +// RegisterMCPToolsToAgent registers all currently active MCP tools and discovery tools to the given agent instance. +func (al *AgentLoop) RegisterMCPToolsToAgent(agentID string, agent *AgentInstance) { + if !al.cfg.Tools.MCP.Enabled { + return + } + + mcpManager := al.mcp.getManager() + if mcpManager == nil { + return + } + + // 1. Register MCP server tools + servers := mcpManager.GetServers() + uniqueTools := 0 + totalRegistrations := 0 + + for serverName, conn := range servers { + uniqueTools += len(conn.Tools) + + serverCfg := al.cfg.Tools.MCP.Servers[serverName] + registerAsHidden := serverIsDeferred(al.cfg.Tools.MCP.Discovery.Enabled, serverCfg) + + for _, tool := range conn.Tools { + mcpTool := tools.NewMCPTool(mcpManager, serverName, tool) + + if registerAsHidden { + agent.Tools.RegisterHidden(mcpTool) + } else { + agent.Tools.Register(mcpTool) + } + totalRegistrations++ + } + } + + if totalRegistrations > 0 { + logger.DebugCF("agent", "Registered MCP tools to agent", + map[string]any{ + "agent_id": agentID, + "server_count": len(servers), + "tool_count": totalRegistrations, + }) + } + + // 2. Initializes Discovery Tools only if enabled by configuration + if al.cfg.Tools.MCP.Discovery.Enabled { + useBM25 := al.cfg.Tools.MCP.Discovery.UseBM25 + useRegex := al.cfg.Tools.MCP.Discovery.UseRegex + + if useBM25 || useRegex { + ttl := al.cfg.Tools.MCP.Discovery.TTL + if ttl <= 0 { + ttl = 5 + } + maxSearchResults := al.cfg.Tools.MCP.Discovery.MaxSearchResults + if maxSearchResults <= 0 { + maxSearchResults = 5 + } + + if useRegex { + agent.Tools.Register(tools.NewRegexSearchTool(agent.Tools, ttl, maxSearchResults)) + } + if useBM25 { + agent.Tools.Register(tools.NewBM25SearchTool(agent.Tools, ttl, maxSearchResults)) + } + + logger.DebugCF("agent", "Initialized tool discovery for agent", map[string]any{ + "agent_id": agentID, "bm25": useBM25, "regex": useRegex, + }) + } + } +} + // serverIsDeferred reports whether an MCP server's tools should be registered // as hidden (deferred/discovery mode). // diff --git a/pkg/agent/multiuser_mcp_test.go b/pkg/agent/multiuser_mcp_test.go new file mode 100644 index 000000000..0358d68bd --- /dev/null +++ b/pkg/agent/multiuser_mcp_test.go @@ -0,0 +1,55 @@ +package agent + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/config" + mcp_pkg "github.com/sipeed/picoclaw/pkg/mcp" +) + +func TestMultiUserMCPPropagation(t *testing.T) { + cfg := &config.Config{} + cfg.Agents.Defaults.Workspace = t.TempDir() + cfg.Tools.MCP.Enabled = true + cfg.Tools.MCP.Servers = map[string]config.MCPServerConfig{ + "test-server": {Enabled: true}, + } + + msgBus := bus.NewMessageBus() + provider := &mockProvider{} + al := NewAgentLoop(cfg, msgBus, provider) + + // Mock initialized MCP manager + mcpManager := mcp_pkg.NewManager() + al.mcp.setManager(mcpManager) + + // 1. Create a transient agent instance + agent := NewAgentInstance(&config.AgentConfig{ID: "test"}, &cfg.Agents.Defaults, cfg, provider, "user-123") + require.NotNil(t, agent) + + // 2. Register tools initially (should be nothing) + al.RegisterMCPToolsToAgent("test", agent) + + // Verify no MCP tools yet + _, ok := agent.Tools.Get("mcp_test_tool") + assert.False(t, ok) + + // 3. Test Discovery tools registration + cfg.Tools.MCP.Discovery.Enabled = true + cfg.Tools.MCP.Discovery.UseRegex = true + + t.Logf("Config before registration: MCP.Enabled=%v, Discovery.Enabled=%v, UseRegex=%v", + cfg.Tools.MCP.Enabled, cfg.Tools.MCP.Discovery.Enabled, cfg.Tools.MCP.Discovery.UseRegex) + + // Call registration again - it should now add the discovery tool + al.RegisterMCPToolsToAgent("test", agent) + + t.Logf("Registered tools: %v", agent.Tools.List()) + + _, ok = agent.Tools.Get("tool_search_tool_regex") + assert.True(t, ok, "Discovery tool (tool_search_tool_regex) should be registered after enabling it") +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 172929b81..f84c949ee 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -640,8 +640,8 @@ type ModelConfig struct { func (c *ModelConfig) UnmarshalJSON(data []byte) error { type Alias ModelConfig aux := &struct { - APIKey string `json:"api_key"` - APIKeys []string `json:"api_keys"` + APIKey string `json:"api_key"` + APIKeys FlexibleStringSlice `json:"api_keys"` *Alias }{ Alias: (*Alias)(c), @@ -651,7 +651,7 @@ func (c *ModelConfig) UnmarshalJSON(data []byte) error { return err } - c.APIKeys = toSecureStrings(mergeAPIKeys(aux.APIKey, aux.APIKeys)) + c.APIKeys = toSecureStrings(mergeAPIKeys(aux.APIKey, []string(aux.APIKeys))) return nil } @@ -687,8 +687,6 @@ func (c *ModelConfig) SetAPIKey(value string) { } } - - type ToolDiscoveryConfig struct { Enabled bool `json:"enabled" env:"PICOCLAW_TOOLS_DISCOVERY_ENABLED"` TTL int `json:"ttl" env:"PICOCLAW_TOOLS_DISCOVERY_TTL"` @@ -843,8 +841,8 @@ type SkillsToolsConfig struct { Github SkillsGithubConfig `yaml:"github,omitempty" json:"github"` MaxConcurrentSearches int `yaml:"-" json:"max_concurrent_searches" env:"PICOCLAW_TOOLS_SKILLS_MAX_CONCURRENT_SEARCHES"` SearchCache SearchCacheConfig `yaml:"-" json:"search_cache"` - Whitelist FlexibleStringSlice `json:"whitelist,omitempty" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST"` - WhitelistEnabled bool `json:"whitelist_enabled,omitempty" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST_ENABLED"` + Whitelist FlexibleStringSlice `json:"whitelist,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST"` + WhitelistEnabled bool `json:"whitelist_enabled,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST_ENABLED"` } type MediaCleanupConfig struct { @@ -878,6 +876,8 @@ func (c ReadFileToolConfig) EffectiveMode() string { type ToolsConfig struct { AllowReadPaths []string `json:"allow_read_paths" yaml:"-" env:"PICOCLAW_TOOLS_ALLOW_READ_PATHS"` AllowWritePaths []string `json:"allow_write_paths" yaml:"-" env:"PICOCLAW_TOOLS_ALLOW_WRITE_PATHS"` + DenyReadPaths []string `json:"deny_read_paths" yaml:"-" env:"PICOCLAW_TOOLS_DENY_READ_PATHS"` + DenyWritePaths []string `json:"deny_write_paths" yaml:"-" env:"PICOCLAW_TOOLS_DENY_WRITE_PATHS"` // FilterSensitiveData controls whether to filter sensitive values (API keys, // tokens, secrets) from tool results before sending to the LLM. // Default: true (enabled) @@ -885,31 +885,31 @@ type ToolsConfig struct { // FilterMinLength is the minimum content length required for filtering. // Content shorter than this will be returned unchanged for performance. // Default: 8 - FilterMinLength int `json:"filter_min_length" yaml:"-" env:"PICOCLAW_TOOLS_FILTER_MIN_LENGTH"` - Web WebToolsConfig `json:"web" yaml:"web,omitempty"` - Cron CronToolsConfig `json:"cron" yaml:"-"` - Exec ExecConfig `json:"exec" yaml:"-"` - Skills SkillsToolsConfig `json:"skills" yaml:"skills,omitempty"` - MediaCleanup MediaCleanupConfig `json:"media_cleanup" yaml:"-"` + FilterMinLength int `json:"filter_min_length" yaml:"-" env:"PICOCLAW_TOOLS_FILTER_MIN_LENGTH"` + Web WebToolsConfig `json:"web" yaml:"web,omitempty"` + Cron CronToolsConfig `json:"cron" yaml:"-"` + Exec ExecConfig `json:"exec" yaml:"-"` + Skills SkillsToolsConfig `json:"skills" yaml:"skills,omitempty"` + MediaCleanup MediaCleanupConfig `json:"media_cleanup" yaml:"-"` Whitelist FlexibleStringSlice `json:"whitelist,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_WHITELIST"` WhitelistEnabled bool `json:"whitelist_enabled,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_WHITELIST_ENABLED"` - MCP MCPConfig `json:"mcp" yaml:"-"` - AppendFile ToolConfig `json:"append_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_APPEND_FILE_"` - EditFile ToolConfig `json:"edit_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_EDIT_FILE_"` - FindSkills ToolConfig `json:"find_skills" yaml:"-" envPrefix:"PICOCLAW_TOOLS_FIND_SKILLS_"` - I2C ToolConfig `json:"i2c" yaml:"-" envPrefix:"PICOCLAW_TOOLS_I2C_"` - InstallSkill ToolConfig `json:"install_skill" yaml:"-" envPrefix:"PICOCLAW_TOOLS_INSTALL_SKILL_"` - ListDir ToolConfig `json:"list_dir" yaml:"-" envPrefix:"PICOCLAW_TOOLS_LIST_DIR_"` - Message ToolConfig `json:"message" yaml:"-" envPrefix:"PICOCLAW_TOOLS_MESSAGE_"` - ReadFile ReadFileToolConfig `json:"read_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_READ_FILE_"` - SendFile ToolConfig `json:"send_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SEND_FILE_"` - SendTTS ToolConfig `json:"send_tts" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SEND_TTS_"` - Spawn ToolConfig `json:"spawn" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_"` - SpawnStatus ToolConfig `json:"spawn_status" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_STATUS_"` - SPI ToolConfig `json:"spi" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPI_"` - Subagent ToolConfig `json:"subagent" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SUBAGENT_"` - WebFetch ToolConfig `json:"web_fetch" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WEB_FETCH_"` - WriteFile ToolConfig `json:"write_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WRITE_FILE_"` + MCP MCPConfig `json:"mcp" yaml:"-"` + AppendFile ToolConfig `json:"append_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_APPEND_FILE_"` + EditFile ToolConfig `json:"edit_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_EDIT_FILE_"` + FindSkills ToolConfig `json:"find_skills" yaml:"-" envPrefix:"PICOCLAW_TOOLS_FIND_SKILLS_"` + I2C ToolConfig `json:"i2c" yaml:"-" envPrefix:"PICOCLAW_TOOLS_I2C_"` + InstallSkill ToolConfig `json:"install_skill" yaml:"-" envPrefix:"PICOCLAW_TOOLS_INSTALL_SKILL_"` + ListDir ToolConfig `json:"list_dir" yaml:"-" envPrefix:"PICOCLAW_TOOLS_LIST_DIR_"` + Message ToolConfig `json:"message" yaml:"-" envPrefix:"PICOCLAW_TOOLS_MESSAGE_"` + ReadFile ReadFileToolConfig `json:"read_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_READ_FILE_"` + SendFile ToolConfig `json:"send_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SEND_FILE_"` + SendTTS ToolConfig `json:"send_tts" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SEND_TTS_"` + Spawn ToolConfig `json:"spawn" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_"` + SpawnStatus ToolConfig `json:"spawn_status" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPAWN_STATUS_"` + SPI ToolConfig `json:"spi" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SPI_"` + Subagent ToolConfig `json:"subagent" yaml:"-" envPrefix:"PICOCLAW_TOOLS_SUBAGENT_"` + WebFetch ToolConfig `json:"web_fetch" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WEB_FETCH_"` + WriteFile ToolConfig `json:"write_file" yaml:"-" envPrefix:"PICOCLAW_TOOLS_WRITE_FILE_"` } // IsFilterSensitiveDataEnabled returns true if sensitive data filtering is enabled diff --git a/pkg/config/config_old.go b/pkg/config/config_old.go index 150275aac..f120d56d3 100644 --- a/pkg/config/config_old.go +++ b/pkg/config/config_old.go @@ -832,9 +832,12 @@ type braveConfigV0 struct { } func toSecureStrings(keys []string) SecureStrings { - apikeys := make(SecureStrings, len(keys)) - for i, key := range keys { - apikeys[i] = NewSecureString(key) + var apikeys SecureStrings + for _, key := range keys { + if key == "[NOT_HERE]" { + continue + } + apikeys = append(apikeys, NewSecureString(key)) } return apikeys } diff --git a/pkg/config/config_struct.go b/pkg/config/config_struct.go index 0b8dd85c8..ac2632000 100644 --- a/pkg/config/config_struct.go +++ b/pkg/config/config_struct.go @@ -144,13 +144,19 @@ func (s *SecureStrings) UnmarshalJSON(value []byte) error { if string(value) == notHere { return nil } + // Try []string first var v []*SecureString - err := json.Unmarshal(value, &v) - if err != nil { - return err + if err := json.Unmarshal(value, &v); err == nil { + *s = v + return nil } - *s = v - return nil + // Fallback to single string + var single *SecureString + if err := json.Unmarshal(value, &single); err == nil { + *s = []*SecureString{single} + return nil + } + return json.Unmarshal(value, &v) // Return original error } // SecureString the string value that can be decrypted or resolved diff --git a/pkg/config/gateway.go b/pkg/config/gateway.go index 30e6f4204..06df7e5bb 100644 --- a/pkg/config/gateway.go +++ b/pkg/config/gateway.go @@ -18,7 +18,6 @@ type GatewayConfig struct { LogLevel string `json:"log_level,omitempty" env:"PICOCLAW_LOG_LEVEL"` } - func canonicalGatewayLogLevel(level logger.LogLevel) string { switch level { case logger.DEBUG: diff --git a/pkg/gateway/gateway.go b/pkg/gateway/gateway.go index 0f20f79b4..397091d30 100644 --- a/pkg/gateway/gateway.go +++ b/pkg/gateway/gateway.go @@ -111,28 +111,39 @@ func (p *startupBlockedProvider) GetDefaultModel() string { // Run starts the gateway runtime using the configuration loaded from configPath. func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error { + fmt.Printf("šŸš€ PicoClaw Gateway starting...\n") + fmt.Printf("šŸ“‚ Home Path: %s\n", homePath) + fmt.Printf("šŸ“„ Config Path: %s\n", configPath) + panicPath := filepath.Join(homePath, logPath, panicFile) + fmt.Printf("šŸ”§ Initializing panic log: %s\n", panicPath) panicFunc, err := logger.InitPanic(panicPath) if err != nil { - return fmt.Errorf("error initializing panic log: %w", err) + fmt.Printf("āš ļø Warning: error initializing panic log (continuing): %v\n", err) + } else if panicFunc != nil { + defer panicFunc() + fmt.Println("āœ“ Panic log initialized") } - defer panicFunc() - if err = logger.EnableFileLogging(filepath.Join(homePath, logPath, logFile)); err != nil { - logger.Fatal(fmt.Sprintf("error enabling file logging: %v", err)) + logFilePath := filepath.Join(homePath, logPath, logFile) + fmt.Printf("šŸ”§ Enabling file logging: %s\n", logFilePath) + if err = logger.EnableFileLogging(logFilePath); err != nil { + fmt.Printf("āš ļø Warning: error initializing file logging (continuing): %v\n", err) + } else { + defer logger.DisableFileLogging() + fmt.Println("āœ“ File logging enabled") + } + + fmt.Println("šŸ” Loading configuration...") + cfg, err := config.LoadConfig(configPath) + if err != nil { + return fmt.Errorf("error loading config: %w", err) } - defer logger.DisableFileLogging() if debug { logger.SetLevel(logger.DEBUG) } else { - logger.SetLevelFromString(config.ResolveGatewayLogLevel(configPath)) - } - -<<<<<<< HEAD - cfg, err := config.LoadConfig(configPath) - if err != nil { - logger.Fatalf("error loading config: %v", err) + logger.SetLevelFromString(cfg.Gateway.LogLevel) } if err = preCheckConfig(cfg); err != nil { @@ -156,9 +167,7 @@ func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error } defer pid.RemovePidFile(homePath) -======= fmt.Printf("šŸ” Creating startup provider for model: %s (allow empty: %v)\n", cfg.Agents.Defaults.GetModelName(), allowEmptyStartup) ->>>>>>> 46dc6e5 (Synchronize hardening: added onboard purge, non-interactive mode, and diagnostic startup logs) provider, modelID, err := createStartupProvider(cfg, allowEmptyStartup) if err != nil { fmt.Printf("āŒ Error creating provider: %v\n", err) @@ -194,7 +203,6 @@ func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error return err } - // Setup manual reload channel for /reload endpoint manualReloadChan := make(chan struct{}, 1) runningServices.manualReloadChan = manualReloadChan diff --git a/pkg/health/server.go b/pkg/health/server.go index 2ff2dae6e..c09b31440 100644 --- a/pkg/health/server.go +++ b/pkg/health/server.go @@ -13,6 +13,11 @@ import ( "github.com/sipeed/picoclaw/pkg/logger" ) +// Mux defines the interface required for registering health handlers. +type Mux interface { + HandleFunc(pattern string, handler func(http.ResponseWriter, *http.Request)) +} + // ChatRequest is the JSON body for POST /chat. type ChatRequest struct { Message string `json:"message"` @@ -36,7 +41,6 @@ type chatStatus struct { } type Server struct { - server *http.Server mu sync.RWMutex ready bool @@ -50,7 +54,6 @@ type Server struct { chatResultsMu sync.RWMutex } - type Check struct { Name string `json:"name"` Status string `json:"status"` @@ -271,7 +274,7 @@ func (s *Server) readyHandler(w http.ResponseWriter, r *http.Request) { // RegisterOnMux registers /health, /ready, /reload and /chat handlers onto the // given mux. This allows the health endpoints to be served by a shared HTTP server. -func (s *Server) RegisterOnMux(mux *http.ServeMux) { +func (s *Server) RegisterOnMux(mux Mux) { mux.HandleFunc("/health", s.healthHandler) mux.HandleFunc("/ready", s.readyHandler) mux.HandleFunc("/reload", s.reloadHandler) @@ -346,6 +349,8 @@ func (s *Server) handlePostChat(w http.ResponseWriter, r *http.Request) { // These are ordered by specificity/reliability headers := []string{ "X-PicoClaw-Chat-ID", + "X-MS-CONVERSATION-ID", // Teams Conversation ID + "X-MS-TENANT-ID", // Teams Tenant ID "X-User-ID", "X-Session-ID", "X-MS-CLIENT-PRINCIPAL-ID", // Azure App Service / Container Apps (EasyAuth) diff --git a/pkg/logger/panic.go b/pkg/logger/panic.go index 0a9125dda..f8df39268 100644 --- a/pkg/logger/panic.go +++ b/pkg/logger/panic.go @@ -17,7 +17,7 @@ func InitPanic(filePath string) (func(), error) { } writer := initPanicFile(filePath) if writer == nil { - return nil, fmt.Errorf("failed to create log file: %s", filePath) + return nil, nil } if panicWriter != nil { _ = panicWriter.Close() diff --git a/pkg/logger/panic_unix.go b/pkg/logger/panic_unix.go index 48f393b45..1a3745d33 100644 --- a/pkg/logger/panic_unix.go +++ b/pkg/logger/panic_unix.go @@ -13,10 +13,13 @@ import ( func initPanicFile(panicFile string) io.WriteCloser { file, err := os.OpenFile(panicFile, os.O_WRONLY|os.O_CREATE|os.O_APPEND|os.O_SYNC, 0o600) if err != nil { - panic(fmt.Sprintf("error in open panic: %v", err)) + fmt.Fprintf(os.Stdout, "Failed to open panic log file %s: %v\n", panicFile, err) + return nil } if err = unix.Dup2(int(file.Fd()), int(os.Stderr.Fd())); err != nil { - panic(fmt.Sprintf("error in syscall.Dup2: %v", err)) + fmt.Fprintf(os.Stdout, "Failed to dup2 panic log: %v\n", err) + file.Close() + return nil } return file } diff --git a/pkg/providers/factory_provider.go b/pkg/providers/factory_provider.go index 653d8732f..ddad48a94 100644 --- a/pkg/providers/factory_provider.go +++ b/pkg/providers/factory_provider.go @@ -217,7 +217,7 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err } return provider, modelID, nil - case "litellm", "lmstudio", "openrouter", "groq", "zhipu", "gemini", "nvidia", "venice", + case "litellm", "lmstudio", "openrouter", "groq", "zhipu", "gemini", "venice", "ollama", "moonshot", "shengsuanyun", "deepseek", "cerebras", "vivgrid", "volcengine", "vllm", "qwen", "qwen-intl", "qwen-international", "dashscope-intl", "qwen-us", "dashscope-us", "mistral", "avian", "longcat", "modelscope", "novita", @@ -250,6 +250,7 @@ func CreateProviderFromConfig(cfg *config.ModelConfig) (LLMProvider, string, err apiBase, cfg.Proxy, cfg.MaxTokensField, + userAgent, cfg.RequestTimeout, cfg.ExtraBody, ) diff --git a/pkg/providers/http_provider.go b/pkg/providers/http_provider.go index 6df03c606..0e197d754 100644 --- a/pkg/providers/http_provider.go +++ b/pkg/providers/http_provider.go @@ -51,12 +51,16 @@ func NewAzureAIProvider(apiKey, apiBase, proxy string, requestTimeoutSeconds int apiKey, apiBase, proxy, - openai_compat.WithAzureHeaders(), + openai_compat.WithAzureHeaders(true), openai_compat.WithRequestTimeout(time.Duration(requestTimeoutSeconds)*time.Second), ), } } +func (p *HTTPProvider) SetUseAzureHeaders(use bool) { + p.delegate.SetUseAzureHeaders(use) +} + func (p *HTTPProvider) Chat( ctx context.Context, messages []Message, @@ -84,10 +88,6 @@ func (p *HTTPProvider) GetDefaultModel() string { return "" } -func (p *HTTPProvider) SetUseAzureHeaders(use bool) { - p.delegate.SetUseAzureHeaders(use) -} - func (p *HTTPProvider) SupportsNativeSearch() bool { return p.delegate.SupportsNativeSearch() } diff --git a/pkg/providers/openai_compat/provider.go b/pkg/providers/openai_compat/provider.go index 279b518f5..02a41a344 100644 --- a/pkg/providers/openai_compat/provider.go +++ b/pkg/providers/openai_compat/provider.go @@ -14,7 +14,6 @@ import ( "sync" "time" - "github.com/sipeed/picoclaw/pkg/providers/common" "github.com/sipeed/picoclaw/pkg/providers/protocoltypes" ) @@ -43,33 +42,30 @@ type Provider struct { mu sync.RWMutex // Protect useAzureHeaders } - - type Option func(*Provider) const defaultRequestTimeout = common.DefaultRequestTimeout var stripModelPrefixProviders = map[string]struct{}{ - "litellm": {}, - "venice": {}, - "moonshot": {}, - "nvidia": {}, - "groq": {}, - "ollama": {}, - "deepseek": {}, - "google": {}, - "openrouter": {}, - "zhipu": {}, - "mistral": {}, - "vivgrid": {}, - "minimax": {}, + "litellm": {}, + "venice": {}, + "moonshot": {}, + "nvidia": {}, + "groq": {}, + "ollama": {}, + "deepseek": {}, + "google": {}, + "openrouter": {}, + "zhipu": {}, + "mistral": {}, + "vivgrid": {}, + "minimax": {}, "novita": {}, "lmstudio": {}, "azure-ai": {}, "azure-foundry": {}, } - func WithMaxTokensField(maxTokensField string) Option { return func(p *Provider) { p.maxTokensField = maxTokensField @@ -108,7 +104,6 @@ func (p *Provider) SetUseAzureHeaders(use bool) { p.useAzureHeaders = use } - func NewProvider(apiKey, apiBase, proxy string, opts ...Option) *Provider { p := &Provider{ apiKey: apiKey, diff --git a/pkg/tools/edit.go b/pkg/tools/edit.go index d5bebf4a2..e84481c94 100644 --- a/pkg/tools/edit.go +++ b/pkg/tools/edit.go @@ -16,12 +16,12 @@ type EditFileTool struct { } // NewEditFileTool creates a new EditFileTool with optional directory restriction. -func NewEditFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *EditFileTool { - var patterns []*regexp.Regexp - if len(allowPaths) > 0 { - patterns = allowPaths[0] +func NewEditFileTool(workspace string, restrict bool, allowPaths []*regexp.Regexp, denyPaths ...[]*regexp.Regexp) *EditFileTool { + var denyPatterns []*regexp.Regexp + if len(denyPaths) > 0 { + denyPatterns = denyPaths[0] } - return &EditFileTool{fs: buildFs(workspace, restrict, patterns)} + return &EditFileTool{fs: buildFs(workspace, restrict, allowPaths, denyPatterns)} } func (t *EditFileTool) Name() string { @@ -79,12 +79,12 @@ type AppendFileTool struct { fs fileSystem } -func NewAppendFileTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *AppendFileTool { - var patterns []*regexp.Regexp - if len(allowPaths) > 0 { - patterns = allowPaths[0] +func NewAppendFileTool(workspace string, restrict bool, allowPaths []*regexp.Regexp, denyPaths ...[]*regexp.Regexp) *AppendFileTool { + var denyPatterns []*regexp.Regexp + if len(denyPaths) > 0 { + denyPatterns = denyPaths[0] } - return &AppendFileTool{fs: buildFs(workspace, restrict, patterns)} + return &AppendFileTool{fs: buildFs(workspace, restrict, allowPaths, denyPatterns)} } func (t *AppendFileTool) Name() string { diff --git a/pkg/tools/edit_test.go b/pkg/tools/edit_test.go index 83a7e778c..a950a6566 100644 --- a/pkg/tools/edit_test.go +++ b/pkg/tools/edit_test.go @@ -16,7 +16,7 @@ func TestEditTool_EditFile_Success(t *testing.T) { testFile := filepath.Join(tmpDir, "test.txt") os.WriteFile(testFile, []byte("Hello World\nThis is a test"), 0o644) - tool := NewEditFileTool(tmpDir, true) + tool := NewEditFileTool(tmpDir, true, nil) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -60,7 +60,7 @@ func TestEditTool_EditFile_NotFound(t *testing.T) { tmpDir := t.TempDir() testFile := filepath.Join(tmpDir, "nonexistent.txt") - tool := NewEditFileTool(tmpDir, true) + tool := NewEditFileTool(tmpDir, true, nil) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -87,7 +87,7 @@ func TestEditTool_EditFile_OldTextNotFound(t *testing.T) { testFile := filepath.Join(tmpDir, "test.txt") os.WriteFile(testFile, []byte("Hello World"), 0o644) - tool := NewEditFileTool(tmpDir, true) + tool := NewEditFileTool(tmpDir, true, nil) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -114,7 +114,7 @@ func TestEditTool_EditFile_MultipleMatches(t *testing.T) { testFile := filepath.Join(tmpDir, "test.txt") os.WriteFile(testFile, []byte("test test test"), 0o644) - tool := NewEditFileTool(tmpDir, true) + tool := NewEditFileTool(tmpDir, true, nil) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -142,7 +142,7 @@ func TestEditTool_EditFile_OutsideAllowedDir(t *testing.T) { testFile := filepath.Join(otherDir, "test.txt") os.WriteFile(testFile, []byte("content"), 0o644) - tool := NewEditFileTool(tmpDir, true) // Restrict to tmpDir + tool := NewEditFileTool(tmpDir, true, nil) // Restrict to tmpDir ctx := context.Background() args := map[string]any{ "path": testFile, @@ -169,7 +169,7 @@ func TestEditTool_EditFile_OutsideAllowedDir(t *testing.T) { // TestEditTool_EditFile_MissingPath verifies error handling for missing path func TestEditTool_EditFile_MissingPath(t *testing.T) { - tool := NewEditFileTool("", false) + tool := NewEditFileTool("", false, nil) ctx := context.Background() args := map[string]any{ "old_text": "old", @@ -186,7 +186,7 @@ func TestEditTool_EditFile_MissingPath(t *testing.T) { // TestEditTool_EditFile_MissingOldText verifies error handling for missing old_text func TestEditTool_EditFile_MissingOldText(t *testing.T) { - tool := NewEditFileTool("", false) + tool := NewEditFileTool("", false, nil) ctx := context.Background() args := map[string]any{ "path": "/tmp/test.txt", @@ -203,7 +203,7 @@ func TestEditTool_EditFile_MissingOldText(t *testing.T) { // TestEditTool_EditFile_MissingNewText verifies error handling for missing new_text func TestEditTool_EditFile_MissingNewText(t *testing.T) { - tool := NewEditFileTool("", false) + tool := NewEditFileTool("", false, nil) ctx := context.Background() args := map[string]any{ "path": "/tmp/test.txt", @@ -224,7 +224,7 @@ func TestEditTool_AppendFile_Success(t *testing.T) { testFile := filepath.Join(tmpDir, "test.txt") os.WriteFile(testFile, []byte("Initial content"), 0o644) - tool := NewAppendFileTool("", false) + tool := NewAppendFileTool("", false, nil) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -264,7 +264,7 @@ func TestEditTool_AppendFile_Success(t *testing.T) { // TestEditTool_AppendFile_MissingPath verifies error handling for missing path func TestEditTool_AppendFile_MissingPath(t *testing.T) { - tool := NewAppendFileTool("", false) + tool := NewAppendFileTool("", false, nil) ctx := context.Background() args := map[string]any{ "content": "test", @@ -280,7 +280,7 @@ func TestEditTool_AppendFile_MissingPath(t *testing.T) { // TestEditTool_AppendFile_MissingContent verifies error handling for missing content func TestEditTool_AppendFile_MissingContent(t *testing.T) { - tool := NewAppendFileTool("", false) + tool := NewAppendFileTool("", false, nil) ctx := context.Background() args := map[string]any{ "path": "/tmp/test.txt", @@ -348,7 +348,7 @@ func TestReplaceEditContent(t *testing.T) { // This exercises the errors.Is(err, fs.ErrNotExist) path in appendFileWithRW + rootRW. func TestAppendFileTool_AppendToNonExistent_Restricted(t *testing.T) { workspace := t.TempDir() - tool := NewAppendFileTool(workspace, true) + tool := NewAppendFileTool(workspace, true, nil) ctx := context.Background() args := map[string]any{ @@ -378,7 +378,7 @@ func TestAppendFileTool_Restricted_Success(t *testing.T) { err := os.WriteFile(filepath.Join(workspace, testFile), []byte("initial"), 0o644) assert.NoError(t, err) - tool := NewAppendFileTool(workspace, true) + tool := NewAppendFileTool(workspace, true, nil) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -402,7 +402,7 @@ func TestEditFileTool_Restricted_InPlaceEdit(t *testing.T) { err := os.WriteFile(filepath.Join(workspace, testFile), []byte("Hello World"), 0o644) assert.NoError(t, err) - tool := NewEditFileTool(workspace, true) + tool := NewEditFileTool(workspace, true, nil) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -423,7 +423,7 @@ func TestEditFileTool_Restricted_InPlaceEdit(t *testing.T) { // error message when the target file does not exist. func TestEditFileTool_Restricted_FileNotFound(t *testing.T) { workspace := t.TempDir() - tool := NewEditFileTool(workspace, true) + tool := NewEditFileTool(workspace, true, nil) ctx := context.Background() args := map[string]any{ "path": "no_such_file.txt", diff --git a/pkg/tools/filesystem.go b/pkg/tools/filesystem.go index 0b9a16950..84e5a6388 100644 --- a/pkg/tools/filesystem.go +++ b/pkg/tools/filesystem.go @@ -256,6 +256,19 @@ func isWithinWorkspace(candidate, workspace string) bool { return err == nil && (rel == "." || filepath.IsLocal(rel)) } +func isDeniedPath(path string, patterns []*regexp.Regexp) bool { + if len(patterns) == 0 { + return false + } + cleaned := filepath.Clean(path) + for _, pattern := range patterns { + if pattern.MatchString(cleaned) { + return true + } + } + return false +} + type ReadFileTool struct { fs fileSystem maxSize int64 @@ -270,11 +283,15 @@ func NewReadFileTool( workspace string, restrict bool, maxReadFileSize int, - allowPaths ...[]*regexp.Regexp, + configs ...[]*regexp.Regexp, ) *ReadFileTool { - var patterns []*regexp.Regexp - if len(allowPaths) > 0 { - patterns = allowPaths[0] + var allowPatterns []*regexp.Regexp + var denyPatterns []*regexp.Regexp + if len(configs) > 0 { + allowPatterns = configs[0] + } + if len(configs) > 1 { + denyPatterns = configs[1] } maxSize := int64(maxReadFileSize) @@ -283,7 +300,7 @@ func NewReadFileTool( } return &ReadFileTool{ - fs: buildFs(workspace, restrict, patterns), + fs: buildFs(workspace, restrict, allowPatterns, denyPatterns), maxSize: maxSize, } } @@ -292,20 +309,24 @@ func NewReadFileBytesTool( workspace string, restrict bool, maxReadFileSize int, - allowPaths ...[]*regexp.Regexp, + configs ...[]*regexp.Regexp, ) *ReadFileTool { - return NewReadFileTool(workspace, restrict, maxReadFileSize, allowPaths...) + return NewReadFileTool(workspace, restrict, maxReadFileSize, configs...) } func NewReadFileLinesTool( workspace string, restrict bool, maxReadFileSize int, - allowPaths ...[]*regexp.Regexp, + configs ...[]*regexp.Regexp, ) *ReadFileLinesTool { - var patterns []*regexp.Regexp - if len(allowPaths) > 0 { - patterns = allowPaths[0] + var allowPatterns []*regexp.Regexp + var denyPatterns []*regexp.Regexp + if len(configs) > 0 { + allowPatterns = configs[0] + } + if len(configs) > 1 { + denyPatterns = configs[1] } maxSize := int64(maxReadFileSize) @@ -314,7 +335,7 @@ func NewReadFileLinesTool( } return &ReadFileLinesTool{ - fs: buildFs(workspace, restrict, patterns), + fs: buildFs(workspace, restrict, allowPatterns, denyPatterns), maxSize: maxSize, } } @@ -853,16 +874,16 @@ type WriteFileTool struct { fs fileSystem } -func NewWriteFileTool( - workspace string, - restrict bool, - allowPaths ...[]*regexp.Regexp, -) *WriteFileTool { - var patterns []*regexp.Regexp - if len(allowPaths) > 0 { - patterns = allowPaths[0] +func NewWriteFileTool(workspace string, restrict bool, configs ...[]*regexp.Regexp) *WriteFileTool { + var allowPatterns []*regexp.Regexp + var denyPatterns []*regexp.Regexp + if len(configs) > 0 { + allowPatterns = configs[0] } - return &WriteFileTool{fs: buildFs(workspace, restrict, patterns)} + if len(configs) > 1 { + denyPatterns = configs[1] + } + return &WriteFileTool{fs: buildFs(workspace, restrict, allowPatterns, denyPatterns)} } func (t *WriteFileTool) Name() string { @@ -927,12 +948,16 @@ type ListDirTool struct { fs fileSystem } -func NewListDirTool(workspace string, restrict bool, allowPaths ...[]*regexp.Regexp) *ListDirTool { - var patterns []*regexp.Regexp - if len(allowPaths) > 0 { - patterns = allowPaths[0] +func NewListDirTool(workspace string, restrict bool, configs ...[]*regexp.Regexp) *ListDirTool { + var allowPatterns []*regexp.Regexp + var denyPatterns []*regexp.Regexp + if len(configs) > 0 { + allowPatterns = configs[0] } - return &ListDirTool{fs: buildFs(workspace, restrict, patterns)} + if len(configs) > 1 { + denyPatterns = configs[1] + } + return &ListDirTool{fs: buildFs(workspace, restrict, allowPatterns, denyPatterns)} } func (t *ListDirTool) Name() string { @@ -991,9 +1016,14 @@ type fileSystem interface { } // hostFs is an unrestricted fileReadWriter that operates directly on the host filesystem. -type hostFs struct{} +type hostFs struct { + denyPatterns []*regexp.Regexp +} func (h *hostFs) ReadFile(path string) ([]byte, error) { + if isDeniedPath(path, h.denyPatterns) { + return nil, fmt.Errorf("access denied: path is blocked by security policy") + } content, err := os.ReadFile(path) if err != nil { if os.IsNotExist(err) { @@ -1008,16 +1038,25 @@ func (h *hostFs) ReadFile(path string) ([]byte, error) { } func (h *hostFs) ReadDir(path string) ([]os.DirEntry, error) { + if isDeniedPath(path, h.denyPatterns) { + return nil, fmt.Errorf("access denied: path is blocked by security policy") + } return os.ReadDir(path) } func (h *hostFs) WriteFile(path string, data []byte) error { + if isDeniedPath(path, h.denyPatterns) { + return fmt.Errorf("access denied: path is blocked by security policy") + } // Use unified atomic write utility with explicit sync for flash storage reliability. // Using 0o600 (owner read/write only) for secure default permissions. return fileutil.WriteFileAtomic(path, data, 0o600) } func (h *hostFs) Open(path string) (fs.File, error) { + if isDeniedPath(path, h.denyPatterns) { + return nil, fmt.Errorf("access denied: path is blocked by security policy") + } f, err := os.Open(path) if err != nil { if os.IsNotExist(err) { @@ -1033,7 +1072,8 @@ func (h *hostFs) Open(path string) (fs.File, error) { // sandboxFs is a sandboxed fileSystem that operates within a strictly defined workspace using os.Root. type sandboxFs struct { - workspace string + workspace string + denyPatterns []*regexp.Regexp } func (r *sandboxFs) execute(path string, fn func(root *os.Root, relPath string) error) error { @@ -1052,6 +1092,10 @@ func (r *sandboxFs) execute(path string, fn func(root *os.Root, relPath string) return err } + if isDeniedPath(relPath, r.denyPatterns) { + return fmt.Errorf("access denied: path is blocked by security policy") + } + return fn(root, relPath) } @@ -1204,13 +1248,13 @@ func (w *whitelistFs) Open(path string) (fs.File, error) { // buildFs returns the appropriate fileSystem implementation based on restriction // settings and optional path whitelist patterns. -func buildFs(workspace string, restrict bool, patterns []*regexp.Regexp) fileSystem { +func buildFs(workspace string, restrict bool, allowPatterns, denyPatterns []*regexp.Regexp) fileSystem { if !restrict { - return &hostFs{} + return &hostFs{denyPatterns: denyPatterns} } - sandbox := &sandboxFs{workspace: workspace} - if len(patterns) > 0 { - return &whitelistFs{sandbox: sandbox, patterns: patterns} + sandbox := &sandboxFs{workspace: workspace, denyPatterns: denyPatterns} + if len(allowPatterns) > 0 { + return &whitelistFs{sandbox: sandbox, patterns: allowPatterns} } return sandbox } @@ -1236,3 +1280,36 @@ func getSafeRelPath(workspace, path string) (string, error) { return rel, nil } + +// validatePathWithConfigs returns the resolved absolute path if it is allowed +// by the given workspace, restriction setting, and path whitelist/blacklist. +func validatePathWithConfigs(path, workspace string, restrict bool, allowPatterns, denyPatterns []*regexp.Regexp) (string, error) { + cleaned := filepath.Clean(path) + var resolved string + + if !filepath.IsAbs(cleaned) { + resolved = filepath.Join(workspace, cleaned) + } else { + resolved = cleaned + } + + // 1. Check blacklist first + if isDeniedPath(resolved, denyPatterns) { + return "", fmt.Errorf("access to %s is denied by policy", path) + } + + // 2. Check whitelist (explicit allow) + if isAllowedPath(resolved, allowPatterns) { + return resolved, nil + } + + // 3. Check workspace sandbox if restricted + if restrict { + rel, err := filepath.Rel(workspace, resolved) + if err != nil || !filepath.IsLocal(rel) { + return "", fmt.Errorf("path %s is outside workspace and not whitelisted", path) + } + } + + return resolved, nil +} diff --git a/pkg/tools/filesystem_test.go b/pkg/tools/filesystem_test.go index bfbc1f46e..9b2494d9c 100644 --- a/pkg/tools/filesystem_test.go +++ b/pkg/tools/filesystem_test.go @@ -94,7 +94,7 @@ func TestFilesystemTool_WriteFile_Success(t *testing.T) { tmpDir := t.TempDir() testFile := filepath.Join(tmpDir, "newfile.txt") - tool := NewWriteFileTool("", false) + tool := NewWriteFileTool("", false, nil) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -133,7 +133,7 @@ func TestFilesystemTool_WriteFile_CreateDir(t *testing.T) { tmpDir := t.TempDir() testFile := filepath.Join(tmpDir, "subdir", "newfile.txt") - tool := NewWriteFileTool("", false) + tool := NewWriteFileTool("", false, nil) ctx := context.Background() args := map[string]any{ "path": testFile, @@ -159,7 +159,7 @@ func TestFilesystemTool_WriteFile_CreateDir(t *testing.T) { // TestFilesystemTool_WriteFile_MissingPath verifies error handling for missing path func TestFilesystemTool_WriteFile_MissingPath(t *testing.T) { - tool := NewWriteFileTool("", false) + tool := NewWriteFileTool("", false, nil) ctx := context.Background() args := map[string]any{ "content": "test", @@ -175,7 +175,7 @@ func TestFilesystemTool_WriteFile_MissingPath(t *testing.T) { // TestFilesystemTool_WriteFile_MissingContent verifies error handling for missing content func TestFilesystemTool_WriteFile_MissingContent(t *testing.T) { - tool := NewWriteFileTool("", false) + tool := NewWriteFileTool("", false, nil) ctx := context.Background() args := map[string]any{ "path": "/tmp/test.txt", @@ -202,7 +202,7 @@ func TestFilesystemTool_WriteFile_OverwriteDefaultBlocked(t *testing.T) { testFile := filepath.Join(tmpDir, "existing.txt") os.WriteFile(testFile, []byte("original"), 0o644) - tool := NewWriteFileTool("", false) + tool := NewWriteFileTool("", false, nil) result := tool.Execute(context.Background(), map[string]any{ "path": testFile, "content": "new content", @@ -225,7 +225,7 @@ func TestFilesystemTool_WriteFile_OverwriteExplicitAllowed(t *testing.T) { testFile := filepath.Join(tmpDir, "existing.txt") os.WriteFile(testFile, []byte("original"), 0o644) - tool := NewWriteFileTool("", false) + tool := NewWriteFileTool("", false, nil) result := tool.Execute(context.Background(), map[string]any{ "path": testFile, "content": "replaced", @@ -245,7 +245,7 @@ func TestFilesystemTool_WriteFile_NewFileNoOverwriteFlag(t *testing.T) { tmpDir := t.TempDir() testFile := filepath.Join(tmpDir, "newfile.txt") - tool := NewWriteFileTool("", false) + tool := NewWriteFileTool("", false, nil) result := tool.Execute(context.Background(), map[string]any{ "path": testFile, "content": "brand new", @@ -265,7 +265,7 @@ func TestFilesystemTool_WriteFile_OverwriteFalseExplicitBlocked(t *testing.T) { testFile := filepath.Join(tmpDir, "existing.txt") os.WriteFile(testFile, []byte("original"), 0o644) - tool := NewWriteFileTool("", false) + tool := NewWriteFileTool("", false, nil) result := tool.Execute(context.Background(), map[string]any{ "path": testFile, "content": "new content", @@ -287,7 +287,7 @@ func TestFilesystemTool_WriteFile_OverwriteSandboxed(t *testing.T) { testFile := "file.txt" os.WriteFile(filepath.Join(workspace, testFile), []byte("original"), 0o644) - tool := NewWriteFileTool(workspace, true) + tool := NewWriteFileTool(workspace, true, nil) // Without overwrite=true → blocked result := tool.Execute(context.Background(), map[string]any{ @@ -322,7 +322,7 @@ func TestFilesystemTool_ListDir_Success(t *testing.T) { os.WriteFile(filepath.Join(tmpDir, "file2.txt"), []byte("content"), 0o644) os.Mkdir(filepath.Join(tmpDir, "subdir"), 0o755) - tool := NewListDirTool("", false) + tool := NewListDirTool("", false, nil) ctx := context.Background() args := map[string]any{ "path": tmpDir, @@ -347,7 +347,7 @@ func TestFilesystemTool_ListDir_Success(t *testing.T) { // TestFilesystemTool_ListDir_NotFound verifies error handling for non-existent directory func TestFilesystemTool_ListDir_NotFound(t *testing.T) { - tool := NewListDirTool("", false) + tool := NewListDirTool("", false, nil) ctx := context.Background() args := map[string]any{ "path": "/nonexistent_directory_12345", @@ -373,7 +373,7 @@ func TestFilesystemTool_ListDir_NotFound(t *testing.T) { // TestFilesystemTool_ListDir_DefaultPath verifies default to current directory func TestFilesystemTool_ListDir_DefaultPath(t *testing.T) { - tool := NewListDirTool("", false) + tool := NewListDirTool("", false, nil) ctx := context.Background() args := map[string]any{} @@ -403,7 +403,7 @@ func TestFilesystemTool_ReadFile_RejectsSymlinkEscape(t *testing.T) { t.Skipf("symlink not supported in this environment: %v", err) } - tool := NewReadFileTool(workspace, true, MaxReadFileSize) + tool := NewReadFileTool(workspace, true, MaxReadFileSize, nil) result := tool.Execute(context.Background(), map[string]any{ "path": link, }) @@ -422,7 +422,7 @@ func TestFilesystemTool_ReadFile_RejectsSymlinkEscape(t *testing.T) { } func TestFilesystemTool_EmptyWorkspace_AccessDenied(t *testing.T) { - tool := NewReadFileTool("", true, MaxReadFileSize) // restrict=true but workspace="" + tool := NewReadFileTool("", true, MaxReadFileSize, nil) // restrict=true but workspace="" // Try to read a sensitive file (simulated by a temp file outside workspace) tmpDir := t.TempDir() @@ -485,7 +485,7 @@ func TestRootMkdirAll(t *testing.T) { func TestFilesystemTool_WriteFile_Restricted_CreateDir(t *testing.T) { workspace := t.TempDir() - tool := NewWriteFileTool(workspace, true) + tool := NewWriteFileTool(workspace, true, nil) ctx := context.Background() testFile := "deep/nested/path/to/file.txt" @@ -763,7 +763,7 @@ func TestReadFileTool_ChunkedReading(t *testing.T) { t.Fatalf("Failed to write test file: %v", err) } - tool := NewReadFileTool(tmpDir, false, MaxReadFileSize) + tool := NewReadFileTool(tmpDir, false, MaxReadFileSize, nil) ctx := context.Background() // --- Step 1: Read the first chunk (10 bytes) --- @@ -841,7 +841,7 @@ func TestReadFileTool_OffsetBeyondEOF(t *testing.T) { t.Fatalf("Failed to write test file: %v", err) } - tool := NewReadFileTool(tmpDir, false, MaxReadFileSize) + tool := NewReadFileTool(tmpDir, false, MaxReadFileSize, nil) ctx := context.Background() args := map[string]any{ @@ -1236,3 +1236,66 @@ func TestReadFileLinesTool_ExactByteBudgetBoundaryIncludesPrefix(t *testing.T) { t.Fatalf("expected continuation at line 2, got: %s", result.ForLLM) } } + +func TestFileSystem_DenyPatterns(t *testing.T) { + tmpDir := t.TempDir() + ctx := context.Background() + + // Create a simulated skills directory + skillsDir := filepath.Join(tmpDir, "skills", "secret-skill") + os.MkdirAll(skillsDir, 0o755) + skillFile := filepath.Join(skillsDir, "SKILL.md") + os.WriteFile(skillFile, []byte("forbidden content"), 0o644) + + // Create a normal file + normalFile := filepath.Join(tmpDir, "report.txt") + os.WriteFile(normalFile, []byte("allowed content"), 0o644) + + // Test with deny patterns: block anything under skills/ + denyPatterns := []*regexp.Regexp{regexp.MustCompile(`^skills(/.*)?$`)} + + t.Run("WriteFile blocked", func(t *testing.T) { + tool := NewWriteFileTool(tmpDir, true, nil, denyPatterns) + args := map[string]any{ + "path": "skills/new-skill.md", + "content": "hacker stuff", + } + result := tool.Execute(ctx, args) + if !result.IsError { + t.Fatal("Expected error when writing to denied path, but got success") + } + if !strings.Contains(result.ForLLM, "access denied") { + t.Errorf("Expected 'access denied' error, got: %s", result.ForLLM) + } + }) + + t.Run("ReadFile blocked", func(t *testing.T) { + tool := NewReadFileTool(tmpDir, true, 0, nil, denyPatterns) + args := map[string]any{"path": "skills/secret-skill/SKILL.md"} + result := tool.Execute(ctx, args) + if !result.IsError { + t.Fatal("Expected error when reading from denied path, but got success") + } + }) + + t.Run("ListDir blocked", func(t *testing.T) { + tool := NewListDirTool(tmpDir, true, nil, denyPatterns) + args := map[string]any{"path": "skills"} + result := tool.Execute(ctx, args) + if !result.IsError { + t.Fatal("Expected error when listing denied path, but got success") + } + }) + + t.Run("Normal file allowed", func(t *testing.T) { + tool := NewReadFileTool(tmpDir, true, 0, nil, denyPatterns) + args := map[string]any{"path": "report.txt"} + result := tool.Execute(ctx, args) + if result.IsError { + t.Fatalf("Expected success for normal file, got error: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "allowed content") { + t.Errorf("Got unexpected content: %s", result.ForLLM) + } + }) +} diff --git a/pkg/tools/registry.go b/pkg/tools/registry.go index bb179509d..b8e9bd3e2 100644 --- a/pkg/tools/registry.go +++ b/pkg/tools/registry.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "sort" + "strings" "sync" "sync/atomic" "time" @@ -440,7 +441,22 @@ func (r *ToolRegistry) Filter(whitelist []string, enabled bool) { removed := 0 for name := range r.tools { - if _, allowed := whitelistMap[name]; !allowed { + allowed := false + if _, exact := whitelistMap[name]; exact { + allowed = true + } else { + // Check for prefix matches (e.g. "monday" matches "mcp_monday_...") + for _, w := range whitelist { + // Match exact (redundant but safe) or prefix with underscore + // We also check for "mcp_" prefix specifically to support MCP tool grouping + if strings.HasPrefix(name, "mcp_"+w+"_") || strings.HasPrefix(name, "tool_"+w+"_") || strings.HasPrefix(name, w+"_") { + allowed = true + break + } + } + } + + if !allowed { delete(r.tools, name) removed++ } diff --git a/pkg/tools/registry_test.go b/pkg/tools/registry_test.go index 16bd30928..3ca4cee4b 100644 --- a/pkg/tools/registry_test.go +++ b/pkg/tools/registry_test.go @@ -759,3 +759,42 @@ func TestToolRegistry_ExecuteWithContext_SanitizesInlineMediaWithoutStore(t *tes t.Fatalf("expected inline media omission note, got %q", result.ForLLM) } } + +func TestToolRegistry_Filter_SupportsPrefix(t *testing.T) { + r := NewToolRegistry() + r.Register(newMockTool("read_file", "core tool")) + r.Register(newMockTool("write_file", "core tool")) + r.Register(newMockTool("mcp_monday_get_items", "mcp tool")) + r.Register(newMockTool("mcp_harvest_get_entries", "mcp tool")) + r.Register(newMockTool("tool_search_regex", "discovery tool")) + + whitelist := []string{"read_file", "monday", "search"} + r.Filter(whitelist, true) + + // expected: read_file (exact), mcp_monday_get_items (mcp_monday_ prefix), tool_search_regex (tool_search_ prefix) + if r.Count() != 3 { + t.Errorf("expected 3 tools after filtering, got %d: %v", r.Count(), r.List()) + } + + allowed := r.List() + expected := map[string]bool{ + "read_file": true, + "mcp_monday_get_items": true, + "tool_search_regex": true, + } + + for _, name := range allowed { + if !expected[name] { + t.Errorf("tool %q should have been filtered out", name) + } + delete(expected, name) + } + + if len(expected) > 0 { + var missing []string + for m := range expected { + missing = append(missing, m) + } + t.Errorf("missing expected tools after filter: %v", missing) + } +} diff --git a/pkg/tools/send_file.go b/pkg/tools/send_file.go index 44198381e..6afc4b09d 100644 --- a/pkg/tools/send_file.go +++ b/pkg/tools/send_file.go @@ -23,6 +23,7 @@ type SendFileTool struct { maxFileSize int mediaStore media.MediaStore allowPaths []*regexp.Regexp + denyPaths []*regexp.Regexp defaultChannel string defaultChatID string @@ -33,21 +34,26 @@ func NewSendFileTool( restrict bool, maxFileSize int, store media.MediaStore, - allowPaths ...[]*regexp.Regexp, + configs ...[]*regexp.Regexp, ) *SendFileTool { if maxFileSize <= 0 { maxFileSize = config.DefaultMaxMediaSize } - var patterns []*regexp.Regexp - if len(allowPaths) > 0 { - patterns = allowPaths[0] + var allowPatterns []*regexp.Regexp + var denyPatterns []*regexp.Regexp + if len(configs) > 0 { + allowPatterns = configs[0] + } + if len(configs) > 1 { + denyPatterns = configs[1] } return &SendFileTool{ workspace: workspace, restrict: restrict, maxFileSize: maxFileSize, mediaStore: store, - allowPaths: patterns, + allowPaths: allowPatterns, + denyPaths: denyPatterns, } } @@ -105,7 +111,7 @@ func (t *SendFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe return ErrorResult("media store not configured") } - resolved, err := validatePathWithAllowPaths(path, t.workspace, t.restrict, t.allowPaths) + resolved, err := validatePathWithConfigs(path, t.workspace, t.restrict, t.allowPaths, t.denyPaths) if err != nil { return ErrorResult(fmt.Sprintf("invalid path: %v", err)) } diff --git a/web/backend/api/skills.go b/web/backend/api/skills.go index 608672172..481a52858 100644 --- a/web/backend/api/skills.go +++ b/web/backend/api/skills.go @@ -609,7 +609,7 @@ func findWorkspaceSkillByDirectory(cfg *config.Config, directory string) *skillS } func findWorkspaceSkillInfoByDirectory(workspace, directory string) *skills.SkillInfo { - loader := skills.NewSkillsLoader(workspace, "", "") + loader := skills.NewSkillsLoader(workspace, "", "", "", nil, false) for _, skill := range loader.ListSkills() { if skill.Source != "workspace" { continue