fix(config): persist shell deny group disables

Squash merge PR #112 after resolving the project changelog conflict with current dev. Local checks passed: config gateway tests, provider/http/tools deny-pattern tests, go build ./..., and go build -tags sqliteonly ./.... PR CI run 26704832350 passed release-versioning, go, and web.
This commit is contained in:
Duy /zuey/ authored and GitHub committed 2026-05-31 13:08:24 +07:00
1 parent 9ab3d6dfe5
commit 990fabf94b
15 files changed
+527 -92

No files matched your search

+1 -1
View File
@@ -383,7 +383,7 @@ func runGateway() {
httpapi.InitGatewayNoAuthFallbackAllowed(config.GatewayNoAuthFallbackAllowed(cfg.Gateway))
exportTokenStore := httpapi.InitExportTokenStore()
defer exportTokenStore.Stop()
agentsH, skillsH, tracesH, mcpH, channelInstancesH, providersH, builtinToolsH, pendingMessagesH, teamEventsH, secureCLIH, secureCLIGrantH, mcpUserCredsH := wireHTTP(pgStores, cfg.Agents.Defaults.Workspace, dataDir, bundledSkillsDir, msgBus, toolsReg, providerRegistry, modelReg, permPE.IsOwner, gatewayAddr, mcpToolLister, usageCapSvc, cfg.Skills)
agentsH, skillsH, tracesH, mcpH, channelInstancesH, providersH, builtinToolsH, pendingMessagesH, teamEventsH, secureCLIH, secureCLIGrantH, mcpUserCredsH := wireHTTP(pgStores, cfg.Agents.Defaults.Workspace, dataDir, bundledSkillsDir, msgBus, toolsReg, providerRegistry, modelReg, permPE.IsOwner, gatewayAddr, mcpToolLister, usageCapSvc, cfg, cfg.Skills)
// Wire dependencies for system prompt preview parity.
if agentsH != nil {
+6 -1
View File
@@ -11,7 +11,7 @@ import (
)
// wireHTTP creates HTTP handlers (agents + skills + traces + MCP + channel instances + providers + builtin tools + pending messages).
func wireHTTP(stores *store.Stores, defaultWorkspace, dataDir, bundledSkillsDir string, msgBus *bus.MessageBus, toolsReg *tools.Registry, providerReg *providers.Registry, modelReg providers.ModelRegistry, isOwner func(string) bool, gatewayAddr string, mcpToolLister httpapi.MCPToolLister, usageCapSvc *usagecaps.Service, skillUploadConfig config.SkillsConfig) (*httpapi.AgentsHandler, *httpapi.SkillsHandler, *httpapi.TracesHandler, *httpapi.MCPHandler, *httpapi.ChannelInstancesHandler, *httpapi.ProvidersHandler, *httpapi.BuiltinToolsHandler, *httpapi.PendingMessagesHandler, *httpapi.TeamEventsHandler, *httpapi.SecureCLIHandler, *httpapi.SecureCLIGrantHandler, *httpapi.MCPUserCredentialsHandler) {
func wireHTTP(stores *store.Stores, defaultWorkspace, dataDir, bundledSkillsDir string, msgBus *bus.MessageBus, toolsReg *tools.Registry, providerReg *providers.Registry, modelReg providers.ModelRegistry, isOwner func(string) bool, gatewayAddr string, mcpToolLister httpapi.MCPToolLister, usageCapSvc *usagecaps.Service, appCfg *config.Config, skillUploadConfig config.SkillsConfig) (*httpapi.AgentsHandler, *httpapi.SkillsHandler, *httpapi.TracesHandler, *httpapi.MCPHandler, *httpapi.ChannelInstancesHandler, *httpapi.ProvidersHandler, *httpapi.BuiltinToolsHandler, *httpapi.PendingMessagesHandler, *httpapi.TeamEventsHandler, *httpapi.SecureCLIHandler, *httpapi.SecureCLIGrantHandler, *httpapi.MCPUserCredentialsHandler) {
var agentsH *httpapi.AgentsHandler
var skillsH *httpapi.SkillsHandler
var tracesH *httpapi.TracesHandler
@@ -68,6 +68,11 @@ func wireHTTP(stores *store.Stores, defaultWorkspace, dataDir, bundledSkillsDir
providersH = httpapi.NewProvidersHandler(stores.Providers, stores.ConfigSecrets, providerReg, gatewayAddr)
providersH.SetMessageBus(msgBus)
providersH.SetUsageCapService(usageCapSvc)
if appCfg != nil {
providersH.SetShellDenyGroupsSource(func() map[string]bool {
return appCfg.ShellDenyGroupsSnapshot()
})
}
if modelReg != nil {
providersH.SetModelRegistry(modelReg)
}
+7
View File
@@ -88,6 +88,13 @@ func (d *gatewayDeps) runLifecycle(
// Reload global shell deny-group toggles on config changes via pub/sub
// so /config edits apply without a process restart.
subscribeShellDenyGroupsReload(d.msgBus, d.toolsReg)
var providerStore store.ProviderStore
var mcpStore store.MCPServerStore
if d.pgStores != nil {
providerStore = d.pgStores.Providers
mcpStore = d.pgStores.MCP
}
subscribeProviderShellDenyGroupsReload(d.msgBus, d.providerRegistry, providerStore, mcpStore)
// Reload TTS providers on config changes via pub/sub.
d.msgBus.Subscribe("tts-config-reload", func(evt bus.Event) {
+55 -4
View File
@@ -1,10 +1,13 @@
package cmd
import (
"context"
"log/slog"
"github.com/nextlevelbuilder/goclaw/internal/bus"
"github.com/nextlevelbuilder/goclaw/internal/config"
"github.com/nextlevelbuilder/goclaw/internal/providers"
"github.com/nextlevelbuilder/goclaw/internal/store"
"github.com/nextlevelbuilder/goclaw/internal/tools"
)
@@ -21,6 +24,7 @@ func subscribeShellDenyGroupsReload(msgBus *bus.MessageBus, toolsReg *tools.Regi
if !ok {
return
}
snapshot := updatedCfg.Clone()
execTool, ok := toolsReg.Get("exec")
if !ok {
return
@@ -29,11 +33,58 @@ func subscribeShellDenyGroupsReload(msgBus *bus.MessageBus, toolsReg *tools.Regi
if !ok {
return
}
et.SetGlobalShellDenyGroups(updatedCfg.Tools.ShellDenyGroups)
et.SetCommandKeywordAllowlist(updatedCfg.Tools.CommandKeywordAllowlist)
et.SetGlobalShellDenyGroups(snapshot.Tools.ShellDenyGroups)
et.SetCommandKeywordAllowlist(snapshot.Tools.CommandKeywordAllowlist)
slog.Info("shell deny groups reloaded via pub/sub",
"groups", len(updatedCfg.Tools.ShellDenyGroups),
"command_keyword_allowlist_rules", len(updatedCfg.Tools.CommandKeywordAllowlist),
"groups", len(snapshot.Tools.ShellDenyGroups),
"command_keyword_allowlist_rules", len(snapshot.Tools.CommandKeywordAllowlist),
)
})
}
func subscribeProviderShellDenyGroupsReload(msgBus *bus.MessageBus, providerReg *providers.Registry, provStore store.ProviderStore, mcpStore store.MCPServerStore) {
if msgBus == nil || providerReg == nil {
return
}
msgBus.Subscribe("shell-deny-provider-policy-reload", func(evt bus.Event) {
if evt.Name != bus.TopicConfigChanged {
return
}
updatedCfg, ok := evt.Payload.(*config.Config)
if !ok {
return
}
reloadShellDenyProviderPolicies(providerReg, provStore, mcpStore, updatedCfg)
})
}
func reloadShellDenyProviderPolicies(providerReg *providers.Registry, provStore store.ProviderStore, mcpStore store.MCPServerStore, cfg *config.Config) {
if providerReg == nil || cfg == nil {
return
}
snapshot := cfg.Clone()
registerClaudeCLIFromConfig(providerReg, snapshot)
if snapshot.Providers.ACP.Binary != "" {
registerACPFromConfig(providerReg, snapshot.Providers.ACP, snapshot.ShellDenyGroupsSnapshot())
}
if provStore == nil {
return
}
dbProviders, err := provStore.ListAllProviders(context.Background())
if err != nil {
slog.Warn("shell deny provider policy reload: failed to load providers from DB", "error", err)
return
}
gatewayAddr := loopbackAddr(snapshot.Gateway.Host, snapshot.Gateway.Port)
for _, p := range dbProviders {
if !p.Enabled {
continue
}
switch p.ProviderType {
case store.ProviderClaudeCLI:
registerClaudeCLIFromDB(providerReg, p, gatewayAddr, snapshot.Gateway.Token, mcpStore, snapshot)
case store.ProviderACP:
registerACPFromDB(providerReg, p, snapshot.ShellDenyGroupsSnapshot())
}
}
}
@@ -2,10 +2,18 @@ package cmd
import (
"context"
"errors"
"os"
"path/filepath"
"regexp"
"testing"
"github.com/google/uuid"
"github.com/nextlevelbuilder/goclaw/internal/bus"
"github.com/nextlevelbuilder/goclaw/internal/config"
"github.com/nextlevelbuilder/goclaw/internal/providers"
"github.com/nextlevelbuilder/goclaw/internal/store"
"github.com/nextlevelbuilder/goclaw/internal/tools"
)
@@ -119,3 +127,153 @@ func TestShellDenyGroupsConfigReload_IgnoresWrongPayload(t *testing.T) {
t.Fatalf("expected wrong-payload event to be ignored; package_install changed to %v", v)
}
}
func TestShellDenyGroupsConfigReload_ReplacesConfigClaudeCLIProvider(t *testing.T) {
msgBus := bus.New()
defer msgBus.Unsubscribe("shell-deny-provider-policy-reload")
providerReg := providers.NewRegistry(store.TenantIDFromContext)
defer providerReg.Close()
initial := config.Default()
initial.Providers.ClaudeCLI.CLIPath = "claude"
initial.Tools.ShellDenyGroups = map[string]bool{"package_install": true}
reloadShellDenyProviderPolicies(providerReg, nil, nil, initial)
before, err := providerReg.Get(context.Background(), "claude-cli")
if err != nil {
t.Fatal(err)
}
beforeCLI, ok := before.(*providers.ClaudeCLIProvider)
if !ok {
t.Fatalf("expected ClaudeCLI provider, got %T", before)
}
updated := config.Default()
updated.Providers.ClaudeCLI.CLIPath = "claude"
updated.Tools.ShellDenyGroups = map[string]bool{"package_install": false}
subscribeProviderShellDenyGroupsReload(msgBus, providerReg, nil, nil)
msgBus.Broadcast(bus.Event{Name: bus.TopicConfigChanged, Payload: updated})
after, err := providerReg.Get(context.Background(), "claude-cli")
if err != nil {
t.Fatal(err)
}
afterCLI, ok := after.(*providers.ClaudeCLIProvider)
if !ok {
t.Fatalf("expected ClaudeCLI provider after reload, got %T", after)
}
if beforeCLI == afterCLI {
t.Fatal("expected config change to replace Claude CLI provider runtime")
}
}
func TestShellDenyGroupsConfigReload_ReplacesDBClaudeCLIProvider(t *testing.T) {
providerReg := providers.NewRegistry(store.TenantIDFromContext)
defer providerReg.Close()
tenantID := uuid.New()
binary := writeTestExecutable(t)
provStore := &shellDenyGroupsProviderStore{providers: []store.LLMProviderData{
{
TenantID: tenantID,
Name: "tenant-claude",
ProviderType: store.ProviderClaudeCLI,
APIBase: binary,
Enabled: true,
},
}}
initial := config.Default()
initial.Tools.ShellDenyGroups = map[string]bool{"package_install": true}
reloadShellDenyProviderPolicies(providerReg, provStore, nil, initial)
before, err := providerReg.GetForTenant(tenantID, "tenant-claude")
if err != nil {
t.Fatal(err)
}
beforeCLI, ok := before.(*providers.ClaudeCLIProvider)
if !ok {
t.Fatalf("expected ClaudeCLI provider, got %T", before)
}
updated := config.Default()
updated.Tools.ShellDenyGroups = map[string]bool{"package_install": false}
reloadShellDenyProviderPolicies(providerReg, provStore, nil, updated)
after, err := providerReg.GetForTenant(tenantID, "tenant-claude")
if err != nil {
t.Fatal(err)
}
afterCLI, ok := after.(*providers.ClaudeCLIProvider)
if !ok {
t.Fatalf("expected ClaudeCLI provider after reload, got %T", after)
}
if beforeCLI == afterCLI {
t.Fatal("expected config change to replace DB Claude CLI provider runtime")
}
}
func TestConfiguredShellDenyPatternsDropsDisabledPackageInstall(t *testing.T) {
cfg := config.Default()
cfg.Tools.ShellDenyGroups = map[string]bool{"package_install": false}
patterns := configuredShellDenyPatterns(cfg)
if matchesAny(patterns, "pip install requests") {
t.Fatal("package_install=false should remove package-install deny patterns")
}
if !matchesAny(patterns, "env") {
t.Fatal("unrelated default deny patterns should remain active")
}
}
type shellDenyGroupsProviderStore struct {
providers []store.LLMProviderData
}
func (s *shellDenyGroupsProviderStore) CreateProvider(context.Context, *store.LLMProviderData) error {
return errors.New("not implemented")
}
func (s *shellDenyGroupsProviderStore) GetProvider(context.Context, uuid.UUID) (*store.LLMProviderData, error) {
return nil, errors.New("not implemented")
}
func (s *shellDenyGroupsProviderStore) GetProviderByName(context.Context, string) (*store.LLMProviderData, error) {
return nil, errors.New("not implemented")
}
func (s *shellDenyGroupsProviderStore) ListProviders(context.Context) ([]store.LLMProviderData, error) {
return s.providers, nil
}
func (s *shellDenyGroupsProviderStore) ListAllProviders(context.Context) ([]store.LLMProviderData, error) {
return s.providers, nil
}
func (s *shellDenyGroupsProviderStore) UpdateProvider(context.Context, uuid.UUID, map[string]any) error {
return errors.New("not implemented")
}
func (s *shellDenyGroupsProviderStore) DeleteProvider(context.Context, uuid.UUID) error {
return errors.New("not implemented")
}
func writeTestExecutable(t *testing.T) string {
t.Helper()
binary := filepath.Join(t.TempDir(), "claude-test")
if err := os.WriteFile(binary, []byte("#!/bin/sh\nexit 0\n"), 0755); err != nil {
t.Fatal(err)
}
return binary
}
func matchesAny(patterns []*regexp.Regexp, command string) bool {
for _, pattern := range patterns {
if pattern.MatchString(command) {
return true
}
}
return false
}
+1 -1
View File
@@ -701,7 +701,7 @@ func wireExtras(
}
providerReg.UnregisterForTenant(tenantID, p.Name)
if p.Enabled {
registerACPFromDB(providerReg, *p)
registerACPFromDB(providerReg, *p, configuredShellDenyGroups(appCfg))
}
})
+71 -52
View File
@@ -7,6 +7,7 @@ import (
"net"
"os/exec"
"path/filepath"
"regexp"
"strconv"
"time"
@@ -195,33 +196,11 @@ func registerProviders(registry *providers.Registry, cfg *config.Config, modelRe
}
}
// Claude CLI provider (subscription-based, no API key needed)
if cfg.Providers.ClaudeCLI.CLIPath != "" {
cliPath := cfg.Providers.ClaudeCLI.CLIPath
var opts []providers.ClaudeCLIOption
if cfg.Providers.ClaudeCLI.Model != "" {
opts = append(opts, providers.WithClaudeCLIModel(cfg.Providers.ClaudeCLI.Model))
}
if cfg.Providers.ClaudeCLI.BaseWorkDir != "" {
opts = append(opts, providers.WithClaudeCLIWorkDir(cfg.Providers.ClaudeCLI.BaseWorkDir))
}
if cfg.Providers.ClaudeCLI.PermMode != "" {
opts = append(opts, providers.WithClaudeCLIPermMode(cfg.Providers.ClaudeCLI.PermMode))
}
// Build per-session MCP config: external MCP servers + GoClaw bridge
gatewayAddr := loopbackAddr(cfg.Gateway.Host, cfg.Gateway.Port)
mcpData := providers.BuildCLIMCPConfigData(cfg.Tools.McpServers, gatewayAddr, cfg.Gateway.Token)
opts = append(opts, providers.WithClaudeCLIMCPConfigData(mcpData))
// Enable GoClaw security hooks (shell deny patterns, path restrictions)
opts = append(opts, providers.WithClaudeCLISecurityHooks(
cfg.Providers.ClaudeCLI.BaseWorkDir, true))
registry.Register(providers.NewClaudeCLIProvider(cliPath, opts...))
slog.Info("registered provider", "name", "claude-cli")
}
registerClaudeCLIFromConfig(registry, cfg)
// ACP provider (config-based) — orchestrates any ACP-compatible agent binary
if cfg.Providers.ACP.Binary != "" {
registerACPFromConfig(registry, cfg.Providers.ACP)
registerACPFromConfig(registry, cfg.Providers.ACP, configuredShellDenyGroups(cfg))
}
}
@@ -303,34 +282,12 @@ func registerProvidersFromDB(registry *providers.Registry, provStore store.Provi
continue
}
if p.ProviderType == store.ProviderClaudeCLI {
cliPath := p.APIBase // reuse APIBase field for CLI path
if cliPath == "" {
cliPath = "claude"
}
// Validate: only accept "claude" or absolute path
if cliPath != "claude" && !filepath.IsAbs(cliPath) {
slog.Warn("security.claude_cli: invalid path from DB, using default", "path", cliPath)
cliPath = "claude"
}
if _, err := exec.LookPath(cliPath); err != nil {
slog.Warn("claude-cli: binary not found, skipping", "path", cliPath, "error", err)
continue
}
var cliOpts []providers.ClaudeCLIOption
cliOpts = append(cliOpts, providers.WithClaudeCLIName(p.Name))
cliOpts = append(cliOpts, providers.WithClaudeCLISecurityHooks("", true))
if gatewayAddr != "" {
mcpData := providers.BuildCLIMCPConfigData(nil, gatewayAddr, gatewayToken)
mcpData.AgentMCPLookup = buildMCPServerLookup(mcpStore)
cliOpts = append(cliOpts, providers.WithClaudeCLIMCPConfigData(mcpData))
}
registry.RegisterForTenant(p.TenantID, providers.NewClaudeCLIProvider(cliPath, cliOpts...))
slog.Info("registered provider from DB", "name", p.Name)
registerClaudeCLIFromDB(registry, p, gatewayAddr, gatewayToken, mcpStore, cfg)
continue
}
// ACP provider — no API key needed (agents manage their own auth).
if p.ProviderType == store.ProviderACP {
registerACPFromDB(registry, p)
registerACPFromDB(registry, p, configuredShellDenyGroups(cfg))
continue
}
// Local Ollama requires no API key — handle before the key guard (same pattern as ClaudeCLI).
@@ -455,8 +412,59 @@ func registerProvidersFromDB(registry *providers.Registry, provStore store.Provi
}
}
func registerClaudeCLIFromConfig(registry *providers.Registry, cfg *config.Config) {
if cfg == nil || cfg.Providers.ClaudeCLI.CLIPath == "" {
return
}
cliPath := cfg.Providers.ClaudeCLI.CLIPath
var opts []providers.ClaudeCLIOption
if cfg.Providers.ClaudeCLI.Model != "" {
opts = append(opts, providers.WithClaudeCLIModel(cfg.Providers.ClaudeCLI.Model))
}
if cfg.Providers.ClaudeCLI.BaseWorkDir != "" {
opts = append(opts, providers.WithClaudeCLIWorkDir(cfg.Providers.ClaudeCLI.BaseWorkDir))
}
if cfg.Providers.ClaudeCLI.PermMode != "" {
opts = append(opts, providers.WithClaudeCLIPermMode(cfg.Providers.ClaudeCLI.PermMode))
}
gatewayAddr := loopbackAddr(cfg.Gateway.Host, cfg.Gateway.Port)
mcpData := providers.BuildCLIMCPConfigData(cfg.Tools.McpServers, gatewayAddr, cfg.Gateway.Token)
opts = append(opts, providers.WithClaudeCLIMCPConfigData(mcpData))
opts = append(opts, providers.WithClaudeCLISecurityHooks(
cfg.Providers.ClaudeCLI.BaseWorkDir, true, configuredShellDenyPatterns(cfg)))
registry.Register(providers.NewClaudeCLIProvider(cliPath, opts...))
slog.Info("registered provider", "name", "claude-cli")
}
func registerClaudeCLIFromDB(registry *providers.Registry, p store.LLMProviderData, gatewayAddr, gatewayToken string, mcpStore store.MCPServerStore, cfg *config.Config) bool {
cliPath := p.APIBase // reuse APIBase field for CLI path
if cliPath == "" {
cliPath = "claude"
}
// Validate: only accept "claude" or absolute path
if cliPath != "claude" && !filepath.IsAbs(cliPath) {
slog.Warn("security.claude_cli: invalid path from DB, using default", "path", cliPath)
cliPath = "claude"
}
if _, err := exec.LookPath(cliPath); err != nil {
slog.Warn("claude-cli: binary not found, skipping", "path", cliPath, "error", err)
return false
}
var cliOpts []providers.ClaudeCLIOption
cliOpts = append(cliOpts, providers.WithClaudeCLIName(p.Name))
cliOpts = append(cliOpts, providers.WithClaudeCLISecurityHooks("", true, configuredShellDenyPatterns(cfg)))
if gatewayAddr != "" {
mcpData := providers.BuildCLIMCPConfigData(nil, gatewayAddr, gatewayToken)
mcpData.AgentMCPLookup = buildMCPServerLookup(mcpStore)
cliOpts = append(cliOpts, providers.WithClaudeCLIMCPConfigData(mcpData))
}
registry.RegisterForTenant(p.TenantID, providers.NewClaudeCLIProvider(cliPath, cliOpts...))
slog.Info("registered provider from DB", "name", p.Name)
return true
}
// registerACPFromConfig registers an ACP provider from config file settings.
func registerACPFromConfig(registry *providers.Registry, cfg config.ACPConfig) {
func registerACPFromConfig(registry *providers.Registry, cfg config.ACPConfig, shellDenyGroups map[string]bool) {
if _, err := exec.LookPath(cfg.Binary); err != nil {
slog.Warn("acp: binary not found, skipping", "binary", cfg.Binary, "error", err)
return
@@ -479,13 +487,13 @@ func registerACPFromConfig(registry *providers.Registry, cfg config.ACPConfig) {
opts = append(opts, providers.WithACPPermMode(cfg.PermMode))
}
registry.Register(providers.NewACPProvider(
cfg.Binary, cfg.Args, workDir, idleTTL, tools.DefaultDenyPatterns(), opts...,
cfg.Binary, cfg.Args, workDir, idleTTL, tools.ResolveDenyPatterns(shellDenyGroups), opts...,
))
slog.Info("registered provider", "name", "acp", "binary", cfg.Binary)
}
// registerACPFromDB registers an ACP provider from a DB provider row.
func registerACPFromDB(registry *providers.Registry, p store.LLMProviderData) {
func registerACPFromDB(registry *providers.Registry, p store.LLMProviderData, shellDenyGroups map[string]bool) {
binary := p.APIBase // repurpose api_base as binary path
if binary == "" {
slog.Warn("acp: no binary specified in DB provider", "name", p.Name)
@@ -522,13 +530,24 @@ func registerACPFromDB(registry *providers.Registry, p store.LLMProviderData) {
workDir = defaultACPWorkDir()
}
registry.RegisterForTenant(p.TenantID, providers.NewACPProvider(
binary, settings.Args, workDir, idleTTL, tools.DefaultDenyPatterns(),
binary, settings.Args, workDir, idleTTL, tools.ResolveDenyPatterns(shellDenyGroups),
providers.WithACPName(p.Name),
providers.WithACPModel(p.Name),
))
slog.Info("registered provider from DB", "name", p.Name, "type", "acp")
}
func configuredShellDenyGroups(cfg *config.Config) map[string]bool {
if cfg == nil {
return nil
}
return cfg.ShellDenyGroupsSnapshot()
}
func configuredShellDenyPatterns(cfg *config.Config) []*regexp.Regexp {
return tools.ResolveDenyPatterns(configuredShellDenyGroups(cfg))
}
// defaultACPWorkDir returns the default workspace directory for ACP agents.
func defaultACPWorkDir() string {
return filepath.Join(config.ResolvedDataDirFromEnv(), "acp-workspaces")