From 61fdcdcb8122a5a1b894d0da8533a8b7f8b7ed66 Mon Sep 17 00:00:00 2001 From: Duy /zuey/ Date: Sun, 24 May 2026 13:52:23 +0700 Subject: [PATCH] feat(tools): add command keyword allowlist Closes #42 --- cmd/gateway_lifecycle_shell_deny_groups.go | 6 +- ...ateway_lifecycle_shell_deny_groups_test.go | 31 +++ cmd/gateway_setup.go | 4 +- docs/03-tools-system.md | 28 ++- docs/project-changelog.md | 15 ++ internal/config/config_channels.go | 164 ++++++++------- internal/tools/command_keyword_allowlist.go | 187 ++++++++++++++++++ internal/tools/credentialed_exec.go | 17 +- internal/tools/credentialed_exec_test.go | 109 ++++++++++ internal/tools/shell.go | 65 ++++-- ui/web/src/i18n/locales/en/config.json | 4 + ui/web/src/i18n/locales/vi/config.json | 4 + ui/web/src/i18n/locales/zh/config.json | 4 + .../config/sections/tools-exec-section.tsx | 59 +++++- 14 files changed, 600 insertions(+), 97 deletions(-) create mode 100644 internal/tools/command_keyword_allowlist.go diff --git a/cmd/gateway_lifecycle_shell_deny_groups.go b/cmd/gateway_lifecycle_shell_deny_groups.go index 28c8be7f..46e0e0df 100644 --- a/cmd/gateway_lifecycle_shell_deny_groups.go +++ b/cmd/gateway_lifecycle_shell_deny_groups.go @@ -30,6 +30,10 @@ func subscribeShellDenyGroupsReload(msgBus *bus.MessageBus, toolsReg *tools.Regi return } et.SetGlobalShellDenyGroups(updatedCfg.Tools.ShellDenyGroups) - slog.Info("shell deny groups reloaded via pub/sub", "groups", len(updatedCfg.Tools.ShellDenyGroups)) + et.SetCommandKeywordAllowlist(updatedCfg.Tools.CommandKeywordAllowlist) + slog.Info("shell deny groups reloaded via pub/sub", + "groups", len(updatedCfg.Tools.ShellDenyGroups), + "command_keyword_allowlist_rules", len(updatedCfg.Tools.CommandKeywordAllowlist), + ) }) } diff --git a/cmd/gateway_lifecycle_shell_deny_groups_test.go b/cmd/gateway_lifecycle_shell_deny_groups_test.go index 13368dd6..e66be5c2 100644 --- a/cmd/gateway_lifecycle_shell_deny_groups_test.go +++ b/cmd/gateway_lifecycle_shell_deny_groups_test.go @@ -35,6 +35,37 @@ func TestShellDenyGroupsConfigReload_UpdatesGlobal(t *testing.T) { if v, ok := got["package_install"]; !ok || v != true { t.Fatalf("expected pub/sub to set global package_install=true, got %v", got) } + rules := execTool.CommandKeywordAllowlistForTest() + if len(rules) != 0 { + t.Fatalf("expected empty command keyword allowlist, got %v", rules) + } +} + +func TestShellDenyGroupsConfigReload_UpdatesCommandKeywordAllowlist(t *testing.T) { + msgBus := bus.New() + defer msgBus.Unsubscribe("shell-deny-groups-config-reload") + + toolsReg := tools.NewRegistry() + execTool := tools.NewExecTool("/tmp", false) + toolsReg.Register(execTool) + + subscribeShellDenyGroupsReload(msgBus, toolsReg) + + msgBus.Broadcast(bus.Event{ + Name: bus.TopicConfigChanged, + Payload: &config.Config{ + Tools: config.ToolsConfig{ + CommandKeywordAllowlist: []config.CommandKeywordAllowlistRule{ + {ID: "github-content", Command: "gh", Args: []string{"--body"}, Keywords: []string{"secret"}}, + }, + }, + }, + }) + + rules := execTool.CommandKeywordAllowlistForTest() + if len(rules) != 1 || rules[0].ID != "github-content" { + t.Fatalf("expected command keyword allowlist to reload, got %v", rules) + } } // TestShellDenyGroupsConfigReload_IgnoresOtherEvents: subscriber must guard diff --git a/cmd/gateway_setup.go b/cmd/gateway_setup.go index 491bd680..aba394db 100644 --- a/cmd/gateway_setup.go +++ b/cmd/gateway_setup.go @@ -14,11 +14,11 @@ import ( "github.com/nextlevelbuilder/goclaw/internal/bootstrap" "github.com/nextlevelbuilder/goclaw/internal/bus" "github.com/nextlevelbuilder/goclaw/internal/config" + "github.com/nextlevelbuilder/goclaw/internal/edition" mcpbridge "github.com/nextlevelbuilder/goclaw/internal/mcp" "github.com/nextlevelbuilder/goclaw/internal/permissions" "github.com/nextlevelbuilder/goclaw/internal/providers" "github.com/nextlevelbuilder/goclaw/internal/sandbox" - "github.com/nextlevelbuilder/goclaw/internal/edition" "github.com/nextlevelbuilder/goclaw/internal/skills" "github.com/nextlevelbuilder/goclaw/internal/store" "github.com/nextlevelbuilder/goclaw/internal/store/pg" @@ -214,6 +214,7 @@ func setupToolRegistry( // Apply global shell deny-group toggles before any request can arrive. // Per-agent overrides via store.WithShellDenyGroups still win per-key. et.SetGlobalShellDenyGroups(cfg.Tools.ShellDenyGroups) + et.SetCommandKeywordAllowlist(cfg.Tools.CommandKeywordAllowlist) et.DenyPaths(dataDir, ".goclaw/") // Allow skills execution: master-tenant skills-store + all tenant-scoped skills-store dirs. et.AllowPathExemptions( @@ -603,4 +604,3 @@ func setupSkillsSystem( return skillsLoader, skillSearchTool, globalSkillsDir, bundledSkillsDir, builtinSkillsDir } - diff --git a/docs/03-tools-system.md b/docs/03-tools-system.md index ea7a5d2d..c7940b6d 100644 --- a/docs/03-tools-system.md +++ b/docs/03-tools-system.md @@ -246,6 +246,32 @@ User-facing parameter schemas for the most commonly configured tools. ``` Available presets: `gh`, `gcloud`, `aws`, `kubectl`, `terraform`. +### Credentialed CLI keyword allowlist + +`config.tools.commandKeywordAllowlist` lets operators allow specific product or security vocabulary inside selected credentialed CLI content arguments without disabling `deny_args`. + +Example: + +```json +{ + "tools": { + "commandKeywordAllowlist": [ + { + "id": "github-content", + "command": "gh", + "subcommands": ["issue create", "issue edit", "pr create", "pr comment"], + "args": ["--body", "--title"], + "argPositions": [], + "keywords": ["secret", "secrets", "token", "credential"], + "reason": "Allow security vocabulary in GitHub issue and PR prose" + } + ] + } +} +``` + +The rule above allows `gh issue create --body "secret rotation notes"` but still blocks command paths like `gh secret set TOKEN`. `argPositions` are 0-based after the matched subcommand. The scanner evaluates command arguments only; it does not read the contents of files passed through arguments such as `--body-file`. + --- ## 6. Interception Layer @@ -406,7 +432,7 @@ Current adopters: `web_search`, `web_fetch`, `tts`, `create_image`, `read_image` - Per-key: agent value takes precedence over global value - Multi-tenant invariant: each tenant's config is isolated -**Live reload:** Changes to `config.tools.shellDenyGroups` propagate via `bus.TopicConfigChanged` pub/sub. Next agent turn automatically applies new toggles. +**Live reload:** Changes to `config.tools.shellDenyGroups` and `config.tools.commandKeywordAllowlist` propagate via `bus.TopicConfigChanged` pub/sub. Next agent turn automatically applies new toggles. **Deny-group classes** (from `internal/tools/shell_deny_groups.go` — all denied by default): diff --git a/docs/project-changelog.md b/docs/project-changelog.md index 68fcb9fd..1e234716 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -6,6 +6,21 @@ Significant changes, features, and fixes in reverse chronological order. ## 2026-05-24 +### Command keyword allowlist + +**Features** + +- Added scoped credentialed CLI keyword allowlist config for content arguments and positional arguments, with runtime reload and web config editing. + +**Fixes** + +- Kept credentialed CLI `deny_args` active for command paths such as `gh secret set` while allowing approved GitHub issue/PR prose to mention security vocabulary. +- Added security audit logging for real allowlisted pass-throughs without logging full argument values. + +**Tests** + +- Added regression coverage for scoped keyword masking, disabled rules, unsafe positional rules, config reload, and race-safe policy snapshots. + ### Browser cookie sync and config UI **Features** diff --git a/internal/config/config_channels.go b/internal/config/config_channels.go index a125d50e..6a9c5c70 100644 --- a/internal/config/config_channels.go +++ b/internal/config/config_channels.go @@ -24,25 +24,25 @@ type ChannelsConfig struct { } type TelegramConfig struct { - Enabled bool `json:"enabled"` - Token string `json:"token"` - Proxy string `json:"proxy,omitempty"` - APIServer string `json:"api_server,omitempty"` // custom Telegram Bot API server URL (e.g. "http://localhost:8081") - AllowFrom FlexibleStringSlice `json:"allow_from"` - DMPolicy string `json:"dm_policy,omitempty"` // "pairing" (default), "allowlist", "open", "disabled" - GroupPolicy string `json:"group_policy,omitempty"` // "open" (default), "allowlist", "disabled" - RequireMention *bool `json:"require_mention,omitempty"` // require @bot mention in groups (default true) - MentionMode string `json:"mention_mode,omitempty"` // "strict" (default) = only respond when mentioned; "yield" = respond unless another bot is mentioned - HistoryLimit int `json:"history_limit,omitempty"` // max pending group messages for context (default 50, 0=disabled) - DMStream *bool `json:"dm_stream,omitempty"` // enable streaming for DMs (default false) — edits placeholder progressively - GroupStream *bool `json:"group_stream,omitempty"` // enable streaming for groups (default false) — sends new message, edits progressively - DraftTransport *bool `json:"draft_transport,omitempty"` // use sendMessageDraft for DM streaming (default true) — stealth preview, no notifications per edit - ReasoningStream *bool `json:"reasoning_stream,omitempty"` // show reasoning as separate message when provider emits thinking events (default true) - ReactionLevel string `json:"reaction_level,omitempty"` // "off" (default), "minimal", "full" — status emoji reactions - MediaMaxBytes int64 `json:"media_max_bytes,omitempty"` // max media download size in bytes (default 20MB) - LinkPreview *bool `json:"link_preview,omitempty"` // enable URL previews in messages (default true) - BlockReply *bool `json:"block_reply,omitempty"` // override gateway block_reply (nil = inherit) - ForceIPv4 bool `json:"force_ipv4,omitempty"` // force IPv4 for all Telegram API requests (use when IPv6 routing is broken) + Enabled bool `json:"enabled"` + Token string `json:"token"` + Proxy string `json:"proxy,omitempty"` + APIServer string `json:"api_server,omitempty"` // custom Telegram Bot API server URL (e.g. "http://localhost:8081") + AllowFrom FlexibleStringSlice `json:"allow_from"` + DMPolicy string `json:"dm_policy,omitempty"` // "pairing" (default), "allowlist", "open", "disabled" + GroupPolicy string `json:"group_policy,omitempty"` // "open" (default), "allowlist", "disabled" + RequireMention *bool `json:"require_mention,omitempty"` // require @bot mention in groups (default true) + MentionMode string `json:"mention_mode,omitempty"` // "strict" (default) = only respond when mentioned; "yield" = respond unless another bot is mentioned + HistoryLimit int `json:"history_limit,omitempty"` // max pending group messages for context (default 50, 0=disabled) + DMStream *bool `json:"dm_stream,omitempty"` // enable streaming for DMs (default false) — edits placeholder progressively + GroupStream *bool `json:"group_stream,omitempty"` // enable streaming for groups (default false) — sends new message, edits progressively + DraftTransport *bool `json:"draft_transport,omitempty"` // use sendMessageDraft for DM streaming (default true) — stealth preview, no notifications per edit + ReasoningStream *bool `json:"reasoning_stream,omitempty"` // show reasoning as separate message when provider emits thinking events (default true) + ReactionLevel string `json:"reaction_level,omitempty"` // "off" (default), "minimal", "full" — status emoji reactions + MediaMaxBytes int64 `json:"media_max_bytes,omitempty"` // max media download size in bytes (default 20MB) + LinkPreview *bool `json:"link_preview,omitempty"` // enable URL previews in messages (default true) + BlockReply *bool `json:"block_reply,omitempty"` // override gateway block_reply (nil = inherit) + ForceIPv4 bool `json:"force_ipv4,omitempty"` // force IPv4 for all Telegram API requests (use when IPv6 routing is broken) // Optional STT (Speech-to-Text) pipeline for voice/audio inbound messages. // When stt_proxy_url is set, audio/voice messages are transcribed before being forwarded to the agent. @@ -133,7 +133,7 @@ type SlackConfig struct { type WhatsAppConfig struct { Enabled bool `json:"enabled"` - AuthDir string `json:"auth_dir,omitempty"` // optional: SQLite auth dir override (desktop) + AuthDir string `json:"auth_dir,omitempty"` // optional: SQLite auth dir override (desktop) AllowFrom FlexibleStringSlice `json:"allow_from"` DMPolicy string `json:"dm_policy,omitempty"` // "pairing" (default for DB instances), "open", "allowlist", "disabled" GroupPolicy string `json:"group_policy,omitempty"` // "pairing" (default for DB instances), "open" (default for config), "allowlist", "disabled" @@ -196,25 +196,25 @@ type FeishuConfig struct { // ProvidersConfig maps provider name to its config. type ProvidersConfig struct { - Anthropic ProviderConfig `json:"anthropic"` - OpenAI ProviderConfig `json:"openai"` - OpenRouter ProviderConfig `json:"openrouter"` - Groq ProviderConfig `json:"groq"` - Gemini ProviderConfig `json:"gemini"` - DeepSeek ProviderConfig `json:"deepseek"` - Mistral ProviderConfig `json:"mistral"` - XAI ProviderConfig `json:"xai"` - MiniMax ProviderConfig `json:"minimax"` - Cohere ProviderConfig `json:"cohere"` - Perplexity ProviderConfig `json:"perplexity"` - DashScope ProviderConfig `json:"dashscope"` - Bailian ProviderConfig `json:"bailian"` - Zai ProviderConfig `json:"zai"` - ZaiCoding ProviderConfig `json:"zai_coding"` - Ollama OllamaConfig `json:"ollama"` // local Ollama instance (no API key needed) - OllamaCloud ProviderConfig `json:"ollama_cloud"` // Ollama Cloud (API key required) - ClaudeCLI ClaudeCLIConfig `json:"claude_cli"` - ACP ACPConfig `json:"acp"` + Anthropic ProviderConfig `json:"anthropic"` + OpenAI ProviderConfig `json:"openai"` + OpenRouter ProviderConfig `json:"openrouter"` + Groq ProviderConfig `json:"groq"` + Gemini ProviderConfig `json:"gemini"` + DeepSeek ProviderConfig `json:"deepseek"` + Mistral ProviderConfig `json:"mistral"` + XAI ProviderConfig `json:"xai"` + MiniMax ProviderConfig `json:"minimax"` + Cohere ProviderConfig `json:"cohere"` + Perplexity ProviderConfig `json:"perplexity"` + DashScope ProviderConfig `json:"dashscope"` + Bailian ProviderConfig `json:"bailian"` + Zai ProviderConfig `json:"zai"` + ZaiCoding ProviderConfig `json:"zai_coding"` + Ollama OllamaConfig `json:"ollama"` // local Ollama instance (no API key needed) + OllamaCloud ProviderConfig `json:"ollama_cloud"` // Ollama Cloud (API key required) + ClaudeCLI ClaudeCLIConfig `json:"claude_cli"` + ACP ACPConfig `json:"acp"` Novita ProviderConfig `json:"novita"` // Novita AI (OpenAI-compatible endpoint) BytePlus ProviderConfig `json:"byteplus"` // BytePlus ModelArk (Seed 2.0) BytePlusCoding ProviderConfig `json:"byteplus_coding"` // BytePlus ModelArk Coding Plan @@ -362,16 +362,16 @@ type QuotaConfig struct { // GatewayConfig controls the gateway server. type GatewayConfig struct { - Host string `json:"host"` - Port int `json:"port"` - Token string `json:"token,omitempty"` // bearer token for WS/HTTP auth - OwnerIDs []string `json:"owner_ids,omitempty"` // sender IDs considered "owner" - AllowedOrigins []string `json:"allowed_origins,omitempty"` // WebSocket CORS whitelist (empty = allow all) - MaxMessageChars int `json:"max_message_chars,omitempty"` // max user message characters (default 32000) - RateLimitRPM int `json:"rate_limit_rpm,omitempty"` // rate limit: requests per minute per user (default 20, 0 = disabled) - InjectionAction string `json:"injection_action,omitempty"` // prompt injection action: "log", "warn" (default), "block", "off" - InboundDebounceMs int `json:"inbound_debounce_ms,omitempty"` // merge rapid channel/Web Chat messages from same sender/session (0 = no wait) - Quota *QuotaConfig `json:"quota,omitempty"` // per-user/group request quotas + Host string `json:"host"` + Port int `json:"port"` + Token string `json:"token,omitempty"` // bearer token for WS/HTTP auth + OwnerIDs []string `json:"owner_ids,omitempty"` // sender IDs considered "owner" + AllowedOrigins []string `json:"allowed_origins,omitempty"` // WebSocket CORS whitelist (empty = allow all) + MaxMessageChars int `json:"max_message_chars,omitempty"` // max user message characters (default 32000) + RateLimitRPM int `json:"rate_limit_rpm,omitempty"` // rate limit: requests per minute per user (default 20, 0 = disabled) + InjectionAction string `json:"injection_action,omitempty"` // prompt injection action: "log", "warn" (default), "block", "off" + InboundDebounceMs int `json:"inbound_debounce_ms,omitempty"` // merge rapid channel/Web Chat messages from same sender/session (0 = no wait) + Quota *QuotaConfig `json:"quota,omitempty"` // per-user/group request quotas BlockReply *bool `json:"block_reply,omitempty"` // deliver intermediate text during tool iterations (default false) ToolStatus *bool `json:"tool_status,omitempty"` // show tool name in streaming preview during tool execution (default true) TaskRecoveryIntervalSec int `json:"task_recovery_interval_sec,omitempty"` // team task recovery ticker interval in seconds (default 300 = 5min) @@ -381,18 +381,32 @@ type GatewayConfig struct { // ToolsConfig controls tool availability, policy, and web search. type ToolsConfig struct { - Profile string `json:"profile,omitempty"` // global profile: "minimal", "coding", "messaging", "full" - Allow []string `json:"allow,omitempty"` // global allow list (tool names or "group:xxx") - Deny []string `json:"deny,omitempty"` // global deny list - AlsoAllow []string `json:"alsoAllow,omitempty"` // additive: adds without removing existing - ByProvider map[string]*ToolPolicySpec `json:"byProvider,omitempty"` // per-provider overrides - ShellDenyGroups map[string]bool `json:"shellDenyGroups,omitempty"` // global shell deny-group toggles (group name -> denied); per-agent overrides win per-key - ExecApproval ExecApprovalCfg `json:"execApproval"` // exec command approval settings - WebFetch WebFetchPolicyConfig `json:"web_fetch"` // domain policy for URL fetching - Browser BrowserToolConfig `json:"browser"` - RateLimitPerHour int `json:"rate_limit_per_hour,omitempty"` // max tool executions per hour per session (0 = disabled) - ScrubCredentials *bool `json:"scrub_credentials,omitempty"` // auto-redact API keys/tokens in tool output (default true) - McpServers map[string]*MCPServerConfig `json:"mcp_servers,omitempty"` // external MCP server connections + Profile string `json:"profile,omitempty"` // global profile: "minimal", "coding", "messaging", "full" + Allow []string `json:"allow,omitempty"` // global allow list (tool names or "group:xxx") + Deny []string `json:"deny,omitempty"` // global deny list + AlsoAllow []string `json:"alsoAllow,omitempty"` // additive: adds without removing existing + ByProvider map[string]*ToolPolicySpec `json:"byProvider,omitempty"` // per-provider overrides + ShellDenyGroups map[string]bool `json:"shellDenyGroups,omitempty"` // global shell deny-group toggles (group name -> denied); per-agent overrides win per-key + CommandKeywordAllowlist []CommandKeywordAllowlistRule `json:"commandKeywordAllowlist,omitempty"` // scoped bypass for credentialed CLI content keywords + ExecApproval ExecApprovalCfg `json:"execApproval"` // exec command approval settings + WebFetch WebFetchPolicyConfig `json:"web_fetch"` // domain policy for URL fetching + Browser BrowserToolConfig `json:"browser"` + RateLimitPerHour int `json:"rate_limit_per_hour,omitempty"` // max tool executions per hour per session (0 = disabled) + ScrubCredentials *bool `json:"scrub_credentials,omitempty"` // auto-redact API keys/tokens in tool output (default true) + McpServers map[string]*MCPServerConfig `json:"mcp_servers,omitempty"` // external MCP server connections +} + +// CommandKeywordAllowlistRule scopes product/security vocabulary that may appear +// in credentialed CLI content args without disabling command-path deny patterns. +type CommandKeywordAllowlistRule struct { + ID string `json:"id,omitempty"` + Command string `json:"command"` + Subcommands []string `json:"subcommands,omitempty"` + Args []string `json:"args,omitempty"` + ArgPositions []int `json:"argPositions,omitempty"` // 0-based after matched subcommand + Keywords []string `json:"keywords,omitempty"` + Reason string `json:"reason,omitempty"` + Enabled *bool `json:"enabled,omitempty"` // nil means enabled } // MCPServerConfig configures a single external MCP server connection. @@ -429,24 +443,24 @@ type WebFetchPolicyConfig struct { // BrowserToolConfig controls the browser automation tool. type BrowserToolConfig struct { - Enabled bool `json:"enabled"` // enable the browser tool (default false) - Headless bool `json:"headless,omitempty"` // run Chrome in headless mode (ignored when RemoteURL is set) - RemoteURL string `json:"remote_url,omitempty"` // CDP endpoint for remote Chrome sidecar, e.g. "ws://chrome:9222" - ActionTimeoutMs int `json:"action_timeout_ms,omitempty"` // per-action timeout in ms (default 30000) - IdleTimeoutMs int `json:"idle_timeout_ms,omitempty"` // idle page auto-close in ms (default 600000, 0=disabled) - MaxPages int `json:"max_pages,omitempty"` // max open pages per tenant (default 5) - CookieSyncEnabled bool `json:"cookie_sync_enabled"` // apply selected synced cookies to scoped browser sessions + Enabled bool `json:"enabled"` // enable the browser tool (default false) + Headless bool `json:"headless,omitempty"` // run Chrome in headless mode (ignored when RemoteURL is set) + RemoteURL string `json:"remote_url,omitempty"` // CDP endpoint for remote Chrome sidecar, e.g. "ws://chrome:9222" + ActionTimeoutMs int `json:"action_timeout_ms,omitempty"` // per-action timeout in ms (default 30000) + IdleTimeoutMs int `json:"idle_timeout_ms,omitempty"` // idle page auto-close in ms (default 600000, 0=disabled) + MaxPages int `json:"max_pages,omitempty"` // max open pages per tenant (default 5) + CookieSyncEnabled bool `json:"cookie_sync_enabled"` // apply selected synced cookies to scoped browser sessions } // ToolPolicySpec defines a tool policy at any level (global, per-agent, per-provider). type ToolPolicySpec struct { - Profile string `json:"profile,omitempty"` - Allow []string `json:"allow,omitempty"` - Deny []string `json:"deny,omitempty"` - AlsoAllow []string `json:"alsoAllow,omitempty"` - ByProvider map[string]*ToolPolicySpec `json:"byProvider,omitempty"` - Wait *WaitToolPolicy `json:"wait,omitempty"` - ToolCallPrefix string `json:"toolCallPrefix,omitempty"` // prefix to strip from model's tool call names before registry lookup + Profile string `json:"profile,omitempty"` + Allow []string `json:"allow,omitempty"` + Deny []string `json:"deny,omitempty"` + AlsoAllow []string `json:"alsoAllow,omitempty"` + ByProvider map[string]*ToolPolicySpec `json:"byProvider,omitempty"` + Wait *WaitToolPolicy `json:"wait,omitempty"` + ToolCallPrefix string `json:"toolCallPrefix,omitempty"` // prefix to strip from model's tool call names before registry lookup } // WaitToolPolicy configures per-agent safety bounds for the wait tool. diff --git a/internal/tools/command_keyword_allowlist.go b/internal/tools/command_keyword_allowlist.go new file mode 100644 index 00000000..b03490eb --- /dev/null +++ b/internal/tools/command_keyword_allowlist.go @@ -0,0 +1,187 @@ +package tools + +import ( + "regexp" + "strconv" + "strings" + + "github.com/nextlevelbuilder/goclaw/internal/config" +) + +type commandKeywordAllowAudit struct { + RuleID string + Command string + Subcommand string + Arg string + Keyword string + Reason string +} + +func applyCommandKeywordAllowlist(command string, args []string, rules []config.CommandKeywordAllowlistRule) ([]string, []commandKeywordAllowAudit) { + if len(args) == 0 || len(rules) == 0 { + return args, nil + } + + var out []string + var audits []commandKeywordAllowAudit + for _, rule := range rules { + if !commandKeywordAllowlistRuleEnabled(rule) { + continue + } + if normalizeBinaryName(rule.Command) != normalizeBinaryName(command) { + continue + } + subcommand, argStart, ok := matchCommandKeywordSubcommand(args, rule.Subcommands) + if !ok { + continue + } + argNames := commandKeywordSet(rule.Args) + argPositions := commandKeywordPositionSet(rule.ArgPositions) + keywords := commandKeywordSet(rule.Keywords) + if (len(argNames) == 0 && len(argPositions) == 0) || len(keywords) == 0 { + continue + } + if len(argPositions) > 0 && subcommand == "" { + continue + } + if out == nil { + out = slicesClone(args) + } + for i := argStart; i < len(out); i++ { + argName, valueIndex, inlineValue, ok := commandKeywordArgValue(out, i, argNames) + if !ok { + if _, positional := argPositions[i-argStart]; positional { + argName = "argPositions[" + strconv.Itoa(i-argStart) + "]" + valueIndex = i + inlineValue = out[i] + ok = true + } + } + if !ok { + continue + } + value := inlineValue + if valueIndex != i { + value = out[valueIndex] + } + masked, hits := maskCommandKeywords(value, keywords) + if len(hits) == 0 { + continue + } + if valueIndex == i { + prefix := out[i][:strings.Index(out[i], "=")+1] + out[i] = prefix + masked + } else { + out[valueIndex] = masked + i = valueIndex + } + for _, keyword := range hits { + audits = append(audits, commandKeywordAllowAudit{ + RuleID: rule.ID, + Command: normalizeBinaryName(command), + Subcommand: subcommand, + Arg: argName, + Keyword: keyword, + Reason: rule.Reason, + }) + } + } + } + if out == nil { + return args, audits + } + return out, audits +} + +func commandKeywordAllowlistRuleEnabled(rule config.CommandKeywordAllowlistRule) bool { + return rule.Enabled == nil || *rule.Enabled +} + +func matchCommandKeywordSubcommand(args []string, subcommands []string) (string, int, bool) { + if len(subcommands) == 0 { + return "", 0, true + } + for _, subcommand := range subcommands { + parts := strings.Fields(strings.ToLower(strings.TrimSpace(subcommand))) + if len(parts) == 0 || len(parts) > len(args) { + continue + } + matched := true + for i, part := range parts { + if strings.ToLower(args[i]) != part { + matched = false + break + } + } + if matched { + return strings.Join(parts, " "), len(parts), true + } + } + return "", 0, false +} + +func commandKeywordSet(values []string) map[string]struct{} { + if len(values) == 0 { + return nil + } + set := make(map[string]struct{}, len(values)) + for _, value := range values { + value = strings.ToLower(strings.TrimSpace(value)) + if value != "" { + set[value] = struct{}{} + } + } + return set +} + +func commandKeywordPositionSet(values []int) map[int]struct{} { + if len(values) == 0 { + return nil + } + set := make(map[int]struct{}, len(values)) + for _, value := range values { + if value >= 0 { + set[value] = struct{}{} + } + } + return set +} + +func commandKeywordArgValue(args []string, i int, argNames map[string]struct{}) (string, int, string, bool) { + arg := args[i] + if eq := strings.Index(arg, "="); eq > 0 { + name := strings.ToLower(arg[:eq]) + if _, ok := argNames[name]; ok { + return name, i, arg[eq+1:], true + } + return "", 0, "", false + } + name := strings.ToLower(arg) + if _, ok := argNames[name]; !ok || i+1 >= len(args) { + return "", 0, "", false + } + return name, i + 1, "", true +} + +func maskCommandKeywords(value string, keywords map[string]struct{}) (string, []string) { + if value == "" || len(keywords) == 0 { + return value, nil + } + var hits []string + masked := value + for keyword := range keywords { + re := regexp.MustCompile(`(?i)\b` + regexp.QuoteMeta(keyword) + `\b`) + if !re.MatchString(masked) { + continue + } + hits = append(hits, keyword) + masked = re.ReplaceAllString(masked, "__allowlisted_keyword__") + } + return masked, hits +} + +func slicesClone[T any](src []T) []T { + dst := make([]T, len(src)) + copy(dst, src) + return dst +} diff --git a/internal/tools/credentialed_exec.go b/internal/tools/credentialed_exec.go index b811ca22..a15b5dbc 100644 --- a/internal/tools/credentialed_exec.go +++ b/internal/tools/credentialed_exec.go @@ -381,7 +381,8 @@ func (t *ExecTool) executeCredentialed(ctx context.Context, cred *store.SecureCL } // Step 3: Per-binary deny check (deny_args) - if p := matchesBinaryDeny(args, cred.DenyArgs); p != "" { + denyArgs, allowAudits := applyCommandKeywordAllowlist(binary, args, t.commandKeywordAllowlistSnapshot()) + if p := matchesBinaryDeny(denyArgs, cred.DenyArgs); p != "" { return credentialedDenyError(binary, args, p) } // Per-binary verbose deny check (deny_verbose) — per-arg start-anchored match @@ -389,6 +390,20 @@ func (t *ExecTool) executeCredentialed(ctx context.Context, cred *store.SecureCL if p := matchesBinaryVerbose(args, cred.DenyVerbose); p != "" { return credentialedDenyError(binary, args, p) } + for _, audit := range allowAudits { + slog.Info("security.command_keyword_allowlist", + "binary", audit.Command, + "subcommand", audit.Subcommand, + "arg", audit.Arg, + "keyword", audit.Keyword, + "rule_id", audit.RuleID, + "reason", audit.Reason, + "agent_id", store.AgentIDFromContext(ctx), + "user_id", store.UserIDFromContext(ctx), + "credential_user_id", store.CredentialUserIDFromContext(ctx), + "tenant_id", store.TenantIDFromContext(ctx), + ) + } // Step 4: Decrypt env vars from store (already decrypted by store layer). // Per-user env overrides take priority over binary/grant env. diff --git a/internal/tools/credentialed_exec_test.go b/internal/tools/credentialed_exec_test.go index 484d3dbe..6390dc18 100644 --- a/internal/tools/credentialed_exec_test.go +++ b/internal/tools/credentialed_exec_test.go @@ -5,6 +5,8 @@ import ( "os" "path/filepath" "testing" + + "github.com/nextlevelbuilder/goclaw/internal/config" ) func TestDetectShellOperators(t *testing.T) { @@ -208,6 +210,113 @@ func TestMatchesBinaryDenyJoinedArgs(t *testing.T) { } } +func TestApplyCommandKeywordAllowlistScopesContentArgs(t *testing.T) { + ghPatterns, _ := json.Marshal([]string{`auth\s+`, `repo\s+delete`, `secret\s+`, `token\s+`}) + rules := []config.CommandKeywordAllowlistRule{ + { + ID: "github-content", + Command: "gh", + Subcommands: []string{"issue create", "pr create"}, + Args: []string{"--body", "--title"}, + ArgPositions: []int{0}, + Keywords: []string{"secret", "token"}, + Reason: "GitHub issue and PR prose may discuss security terms.", + }, + } + + tests := []struct { + name string + args []string + wantHit bool + wantAudit int + }{ + { + name: "issue body content allowed", + args: []string{"issue", "create", "--body", "secret rotation details"}, + wantHit: false, + wantAudit: 1, + }, + { + name: "pr title content allowed", + args: []string{"pr", "create", "--title=token handling notes"}, + wantHit: false, + wantAudit: 1, + }, + { + name: "positional content allowed", + args: []string{"issue", "create", "token handling notes"}, + wantHit: false, + wantAudit: 1, + }, + { + name: "command path stays blocked", + args: []string{"secret", "set", "TOKEN"}, + wantHit: true, + }, + { + name: "non-allowlisted arg stays blocked", + args: []string{"issue", "create", "--label", "secret incident"}, + wantHit: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sanitized, audits := applyCommandKeywordAllowlist("gh", tt.args, rules) + got := matchesBinaryDeny(sanitized, ghPatterns) + if (got != "") != tt.wantHit { + t.Fatalf("matchesBinaryDeny(%v) after allowlist = %q, wantHit=%v", sanitized, got, tt.wantHit) + } + if len(audits) != tt.wantAudit { + t.Fatalf("audit count = %d, want %d", len(audits), tt.wantAudit) + } + }) + } +} + +func TestApplyCommandKeywordAllowlistIgnoresDisabledRules(t *testing.T) { + ghPatterns, _ := json.Marshal([]string{`secret\s+`}) + enabled := false + rules := []config.CommandKeywordAllowlistRule{ + { + ID: "disabled-github-content", + Command: "gh", + Subcommands: []string{"issue create"}, + Args: []string{"--body"}, + Keywords: []string{"secret"}, + Enabled: &enabled, + }, + } + + sanitized, audits := applyCommandKeywordAllowlist("gh", []string{"issue", "create", "--body", "secret notes"}, rules) + if got := matchesBinaryDeny(sanitized, ghPatterns); got == "" { + t.Fatalf("disabled rule bypassed deny_args; sanitized args = %v", sanitized) + } + if len(audits) != 0 { + t.Fatalf("disabled rule emitted audit records: %v", audits) + } +} + +func TestApplyCommandKeywordAllowlistRequiresSubcommandForPositions(t *testing.T) { + ghPatterns, _ := json.Marshal([]string{`secret\s+`}) + rules := []config.CommandKeywordAllowlistRule{ + { + ID: "unsafe-position", + Command: "gh", + ArgPositions: []int{0}, + Keywords: []string{"secret"}, + }, + } + + sanitized, audits := applyCommandKeywordAllowlist("gh", []string{"secret", "set", "TOKEN"}, rules) + if got := matchesBinaryDeny(sanitized, ghPatterns); got == "" { + t.Fatalf("position rule without subcommand bypassed command-path deny; sanitized args = %v", sanitized) + } + if len(audits) != 0 { + t.Fatalf("position rule without subcommand emitted audit records: %v", audits) + } +} + func TestResolveAndMatchBinaryUsesConfiguredExecutablePath(t *testing.T) { t.Setenv("PATH", "/usr/bin") binDir := t.TempDir() diff --git a/internal/tools/shell.go b/internal/tools/shell.go index 6be167cd..4e07182f 100644 --- a/internal/tools/shell.go +++ b/internal/tools/shell.go @@ -13,8 +13,10 @@ import ( "regexp" "runtime" "strings" + "sync" "time" + "github.com/nextlevelbuilder/goclaw/internal/config" "github.com/nextlevelbuilder/goclaw/internal/sandbox" "github.com/nextlevelbuilder/goclaw/internal/store" "golang.org/x/text/unicode/norm" @@ -35,27 +37,56 @@ func DefaultDenyPatterns() []*regexp.Regexp { // ExecTool executes shell commands, optionally inside a sandbox container. type ExecTool struct { - workspace string - timeout time.Duration - 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 + workspace string + timeout time.Duration + 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 + policyMu sync.RWMutex + commandKeywordAllowlist []config.CommandKeywordAllowlistRule // globalDenyGroups holds global shell deny-group toggles from config.tools. // Per-agent overrides from context (store.WithShellDenyGroups) win per-key. // Updated at startup and via TopicConfigChanged pub/sub for runtime reload. globalDenyGroups map[string]bool } +// SetCommandKeywordAllowlist replaces the scoped credentialed CLI keyword +// allowlist. The slice is defensively copied so config reload callers cannot +// mutate the running tool policy after assignment. +func (t *ExecTool) SetCommandKeywordAllowlist(rules []config.CommandKeywordAllowlistRule) { + t.policyMu.Lock() + defer t.policyMu.Unlock() + if len(rules) == 0 { + t.commandKeywordAllowlist = nil + return + } + t.commandKeywordAllowlist = slicesClone(rules) +} + +// CommandKeywordAllowlistForTest exposes the effective global allowlist for +// cross-package config reload tests. Not for production callers. +func (t *ExecTool) CommandKeywordAllowlistForTest() []config.CommandKeywordAllowlistRule { + return t.commandKeywordAllowlistSnapshot() +} + +func (t *ExecTool) commandKeywordAllowlistSnapshot() []config.CommandKeywordAllowlistRule { + t.policyMu.RLock() + defer t.policyMu.RUnlock() + return slicesClone(t.commandKeywordAllowlist) +} + // SetGlobalShellDenyGroups replaces the global shell deny-group toggles. The // caller's map is defensively copied so later mutations cannot leak into the // tool's internal state. Passing nil or an empty map clears the global config // (per-agent context overrides, if any, still apply on their own). func (t *ExecTool) SetGlobalShellDenyGroups(groups map[string]bool) { + t.policyMu.Lock() + defer t.policyMu.Unlock() if len(groups) == 0 { t.globalDenyGroups = nil return @@ -70,14 +101,20 @@ func (t *ExecTool) SetGlobalShellDenyGroups(groups map[string]bool) { // empty, the other is returned directly (no allocation). func (t *ExecTool) effectiveDenyGroups(ctx context.Context) map[string]bool { agent := store.ShellDenyGroupsFromContext(ctx) - if len(t.globalDenyGroups) == 0 { + t.policyMu.RLock() + global := t.globalDenyGroups + if len(global) > 0 { + global = maps.Clone(global) + } + t.policyMu.RUnlock() + if len(global) == 0 { return agent } if len(agent) == 0 { - return t.globalDenyGroups + return global } - merged := make(map[string]bool, len(t.globalDenyGroups)+len(agent)) - maps.Copy(merged, t.globalDenyGroups) + merged := make(map[string]bool, len(global)+len(agent)) + maps.Copy(merged, global) // agent wins per-key maps.Copy(merged, agent) return merged diff --git a/ui/web/src/i18n/locales/en/config.json b/ui/web/src/i18n/locales/en/config.json index a1059e77..f22f94bc 100644 --- a/ui/web/src/i18n/locales/en/config.json +++ b/ui/web/src/i18n/locales/en/config.json @@ -208,6 +208,10 @@ "tools.execAskMode": "Ask Mode", "tools.execAskModeTip": "When to prompt the user for approval before executing a command.", "tools.execAllowlistLabel": "Allowed Commands (one pattern per line)", + "tools.commandKeywordAllowlist": "Command Keyword Allowlist", + "tools.commandKeywordAllowlistTip": "JSON rules that allow configured keywords only inside selected credentialed CLI content arguments. Command-path deny rules still apply.", + "tools.commandKeywordAllowlistJsonError": "Invalid JSON.", + "tools.commandKeywordAllowlistArrayError": "Use a JSON array of rules.", "tools.scrubCredentials": "Scrub Credentials", "browser.title": "Browser Automation", diff --git a/ui/web/src/i18n/locales/vi/config.json b/ui/web/src/i18n/locales/vi/config.json index b0c6a77d..aaace75b 100644 --- a/ui/web/src/i18n/locales/vi/config.json +++ b/ui/web/src/i18n/locales/vi/config.json @@ -208,6 +208,10 @@ "tools.execAskMode": "Chế độ hỏi", "tools.execAskModeTip": "Khi nào nhắc người dùng phê duyệt trước khi thực thi lệnh.", "tools.execAllowlistLabel": "Lệnh được phép (mỗi dòng một mẫu)", + "tools.commandKeywordAllowlist": "Allowlist từ khóa theo lệnh", + "tools.commandKeywordAllowlistTip": "Rule JSON cho phép từ khóa đã cấu hình chỉ trong argument nội dung của CLI có credential. Deny rule theo command path vẫn áp dụng.", + "tools.commandKeywordAllowlistJsonError": "JSON không hợp lệ.", + "tools.commandKeywordAllowlistArrayError": "Dùng mảng JSON các rule.", "tools.scrubCredentials": "Ẩn thông tin xác thực", "browser.title": "Tự động hóa trình duyệt", diff --git a/ui/web/src/i18n/locales/zh/config.json b/ui/web/src/i18n/locales/zh/config.json index 1994f1df..21fe8e1d 100644 --- a/ui/web/src/i18n/locales/zh/config.json +++ b/ui/web/src/i18n/locales/zh/config.json @@ -208,6 +208,10 @@ "tools.execAskMode": "询问模式", "tools.execAskModeTip": "在执行命令之前何时提示用户审批。", "tools.execAllowlistLabel": "允许的命令（每行一个模式）", + "tools.commandKeywordAllowlist": "命令关键词允许列表", + "tools.commandKeywordAllowlistTip": "JSON 规则只允许配置的关键词出现在指定的凭证 CLI 内容参数中。命令路径拒绝规则仍然生效。", + "tools.commandKeywordAllowlistJsonError": "JSON 无效。", + "tools.commandKeywordAllowlistArrayError": "请使用规则 JSON 数组。", "tools.scrubCredentials": "擦除凭证", "browser.title": "浏览器自动化", diff --git a/ui/web/src/pages/config/sections/tools-exec-section.tsx b/ui/web/src/pages/config/sections/tools-exec-section.tsx index a0caf55a..7486f094 100644 --- a/ui/web/src/pages/config/sections/tools-exec-section.tsx +++ b/ui/web/src/pages/config/sections/tools-exec-section.tsx @@ -19,7 +19,6 @@ import { } from "@/components/ui/select"; import { InfoLabel } from "@/components/shared/info-label"; - type ToolsData = Record; interface Props { @@ -31,10 +30,14 @@ interface Props { export function ToolsExecSection({ data, onSave, saving }: Props) { const { t } = useTranslation("config"); const [draft, setDraft] = useState(data ?? {}); + const [allowlistText, setAllowlistText] = useState(""); + const [allowlistError, setAllowlistError] = useState(null); const [dirty, setDirty] = useState(false); useEffect(() => { setDraft(data ?? {}); + setAllowlistText(JSON.stringify(data?.commandKeywordAllowlist ?? [], null, 2)); + setAllowlistError(null); setDirty(false); }, [data]); @@ -46,6 +49,30 @@ export function ToolsExecSection({ data, onSave, saving }: Props) { setDirty(true); }; + const updateCommandKeywordAllowlist = (value: string) => { + setAllowlistText(value); + setDirty(true); + + const trimmed = value.trim(); + if (!trimmed) { + setAllowlistError(null); + setDraft((prev) => ({ ...prev, commandKeywordAllowlist: [] })); + return; + } + + try { + const parsed = JSON.parse(trimmed); + if (!Array.isArray(parsed)) { + setAllowlistError(t("tools.commandKeywordAllowlistArrayError")); + return; + } + setAllowlistError(null); + setDraft((prev) => ({ ...prev, commandKeywordAllowlist: parsed })); + } catch { + setAllowlistError(t("tools.commandKeywordAllowlistJsonError")); + } + }; + if (!data) return null; const exec = draft.execApproval ?? {}; @@ -91,15 +118,41 @@ export function ToolsExecSection({ data, onSave, saving }: Props) { allowlist: e.target.value.split("\n").filter(Boolean), }) } - className="min-h-[80px] font-mono text-xs" + className="min-h-[80px] font-mono text-base md:text-sm" placeholder="git * npm * ls *" /> )} +
+ + {t("tools.commandKeywordAllowlist")} + +