diff --git a/internal/config/config.go b/internal/config/config.go index fe760443..4070cd48 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -230,7 +230,9 @@ type ServerConfig struct { // when the server is configured with both Command and an HTTP/SSE URL — i.e., // mcpproxy starts the process AND connects via network. Stdio servers ignore // this field. Zero or unset → 30s default. - LauncherWaitTimeout Duration `json:"launcher_wait_timeout,omitempty" mapstructure:"launcher_wait_timeout" swaggertype:"string"` + LauncherWaitTimeout Duration `json:"launcher_wait_timeout,omitempty" mapstructure:"launcher_wait_timeout" swaggertype:"string"` + EnabledTools []string `json:"enabled_tools,omitempty" mapstructure:"enabled_tools"` // Allowlist: only these tools are exposed; mutually exclusive with disabled_tools + DisabledTools []string `json:"disabled_tools,omitempty" mapstructure:"disabled_tools"` // Denylist: these tools are hidden; mutually exclusive with enabled_tools } // OAuthConfig represents OAuth configuration for a server @@ -1000,6 +1002,14 @@ func (c *Config) ValidateDetailed() []ValidationError { // Note: OAuth configuration is optional. client_id is optional (uses Dynamic Client Registration RFC 7591 if empty). // ClientSecret can be a secret reference, so we don't validate it as empty. + + // enabled_tools and disabled_tools are mutually exclusive + if len(server.EnabledTools) > 0 && len(server.DisabledTools) > 0 { + errors = append(errors, ValidationError{ + Field: fieldPrefix + ".enabled_tools", + Message: "enabled_tools and disabled_tools are mutually exclusive; use one or the other", + }) + } } // Validate DataDir exists (if specified and not empty). diff --git a/internal/config/validation_test.go b/internal/config/validation_test.go index b3893abc..caaf83bf 100644 --- a/internal/config/validation_test.go +++ b/internal/config/validation_test.go @@ -236,6 +236,64 @@ func TestValidateDetailed(t *testing.T) { expectedErrors: 0, errorFields: []string{}, }, + { + name: "enabled_tools and disabled_tools are mutually exclusive", + config: &Config{ + Listen: ":8080", + ToolsLimit: 15, + ToolResponseLimit: 1000, + CallToolTimeout: Duration(60000000000), + Servers: []*ServerConfig{ + { + Name: "test", + Protocol: "http", + URL: "https://api.example.com/mcp", + EnabledTools: []string{"read_file"}, + DisabledTools: []string{"write_file"}, + }, + }, + }, + expectedErrors: 1, + errorFields: []string{"mcpServers[0].enabled_tools"}, + }, + { + name: "enabled_tools alone is valid", + config: &Config{ + Listen: ":8080", + ToolsLimit: 15, + ToolResponseLimit: 1000, + CallToolTimeout: Duration(60000000000), + Servers: []*ServerConfig{ + { + Name: "test", + Protocol: "http", + URL: "https://api.example.com/mcp", + EnabledTools: []string{"read_file", "list_dir"}, + }, + }, + }, + expectedErrors: 0, + errorFields: []string{}, + }, + { + name: "disabled_tools alone is valid", + config: &Config{ + Listen: ":8080", + ToolsLimit: 15, + ToolResponseLimit: 1000, + CallToolTimeout: Duration(60000000000), + Servers: []*ServerConfig{ + { + Name: "test", + Protocol: "http", + URL: "https://api.example.com/mcp", + DisabledTools: []string{"delete_file", "execute_code"}, + }, + }, + }, + expectedErrors: 0, + errorFields: []string{}, + }, } for _, tt := range tests { diff --git a/internal/runtime/lifecycle.go b/internal/runtime/lifecycle.go index 3270295c..9284e830 100644 --- a/internal/runtime/lifecycle.go +++ b/internal/runtime/lifecycle.go @@ -497,6 +497,13 @@ func (r *Runtime) applyDifferentialToolUpdate(ctx context.Context, serverName st approvalResult = &ToolApprovalResult{BlockedTools: make(map[string]bool)} } + // Sync enabled_tools / disabled_tools from server config into BBolt + if err := r.applyConfigToolFilter(serverName, newTools); err != nil { + r.logger.Warn("Failed to apply config tool filter", + zap.String("server", serverName), + zap.Error(err)) + } + // Query existing tools from the index existingTools, err := r.indexManager.GetToolsByServer(serverName) if err != nil { diff --git a/internal/runtime/tool_config_filter_test.go b/internal/runtime/tool_config_filter_test.go new file mode 100644 index 00000000..ebc41b66 --- /dev/null +++ b/internal/runtime/tool_config_filter_test.go @@ -0,0 +1,177 @@ +package runtime + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/storage" +) + +func setupConfigFilterRuntime(t *testing.T, servers []*config.ServerConfig) *Runtime { + t.Helper() + tempDir := t.TempDir() + cfg := &config.Config{ + DataDir: tempDir, + Listen: "127.0.0.1:0", + Servers: servers, + } + rt, err := New(cfg, "", zap.NewNop()) + require.NoError(t, err) + t.Cleanup(func() { _ = rt.Close() }) + return rt +} + +// TestApplyConfigToolFilter_EnabledTools_DisablesNonListedTools verifies that +// when a server has enabled_tools set, tools not in that list are disabled in +// BBolt so they are hidden from MCP clients. +func TestApplyConfigToolFilter_EnabledTools_DisablesNonListedTools(t *testing.T) { + rt := setupConfigFilterRuntime(t, []*config.ServerConfig{ + { + Name: "github", + Enabled: true, + EnabledTools: []string{"list_issues", "get_issue"}, + }, + }) + + tools := []*config.ToolMetadata{ + {ServerName: "github", Name: "list_issues", Description: "List issues", ParamsJSON: `{}`}, + {ServerName: "github", Name: "get_issue", Description: "Get issue", ParamsJSON: `{}`}, + {ServerName: "github", Name: "create_issue", Description: "Create issue", ParamsJSON: `{}`}, + {ServerName: "github", Name: "delete_issue", Description: "Delete issue", ParamsJSON: `{}`}, + } + + err := rt.applyConfigToolFilter("github", tools) + require.NoError(t, err) + + // Allowed tools should remain enabled (no record or Disabled=false) + for _, allowed := range []string{"list_issues", "get_issue"} { + record, err := rt.storageManager.GetToolApproval("github", allowed) + if err == nil && record != nil { + assert.False(t, record.Disabled, "tool %q should be enabled", allowed) + } + // ErrToolApprovalNotFound is also acceptable (means enabled by default) + } + + // Non-listed tools must be explicitly disabled + for _, blocked := range []string{"create_issue", "delete_issue"} { + record, err := rt.storageManager.GetToolApproval("github", blocked) + require.NoError(t, err, "expected approval record for %q", blocked) + assert.True(t, record.Disabled, "tool %q should be disabled", blocked) + } +} + +// TestApplyConfigToolFilter_DisabledTools_DisablesListedTools verifies that +// when a server has disabled_tools set, only those specific tools are disabled. +func TestApplyConfigToolFilter_DisabledTools_DisablesListedTools(t *testing.T) { + rt := setupConfigFilterRuntime(t, []*config.ServerConfig{ + { + Name: "github", + Enabled: true, + DisabledTools: []string{"delete_repo", "force_push"}, + }, + }) + + tools := []*config.ToolMetadata{ + {ServerName: "github", Name: "list_repos", Description: "List repos", ParamsJSON: `{}`}, + {ServerName: "github", Name: "delete_repo", Description: "Delete repo", ParamsJSON: `{}`}, + {ServerName: "github", Name: "force_push", Description: "Force push", ParamsJSON: `{}`}, + } + + err := rt.applyConfigToolFilter("github", tools) + require.NoError(t, err) + + // Listed tools must be disabled + for _, blocked := range []string{"delete_repo", "force_push"} { + record, err := rt.storageManager.GetToolApproval("github", blocked) + require.NoError(t, err, "expected approval record for %q", blocked) + assert.True(t, record.Disabled, "tool %q should be disabled", blocked) + } + + // Non-listed tools should remain enabled + record, err := rt.storageManager.GetToolApproval("github", "list_repos") + if err == nil && record != nil { + assert.False(t, record.Disabled, "tool %q should be enabled", "list_repos") + } +} + +// TestApplyConfigToolFilter_NoFilter_NoChanges verifies that when neither +// enabled_tools nor disabled_tools is set, no records are written. +func TestApplyConfigToolFilter_NoFilter_NoChanges(t *testing.T) { + rt := setupConfigFilterRuntime(t, []*config.ServerConfig{ + {Name: "github", Enabled: true}, + }) + + tools := []*config.ToolMetadata{ + {ServerName: "github", Name: "list_issues", Description: "List issues", ParamsJSON: `{}`}, + } + + err := rt.applyConfigToolFilter("github", tools) + require.NoError(t, err) + + // No approval record should have been written + _, err = rt.storageManager.GetToolApproval("github", "list_issues") + assert.ErrorIs(t, err, storage.ErrToolApprovalNotFound) +} + +// TestApplyConfigToolFilter_EnabledTools_ReEnablesTool verifies that a tool +// previously disabled (e.g. by the API) is re-enabled if it appears in +// enabled_tools on the next applyConfigToolFilter call. +func TestApplyConfigToolFilter_EnabledTools_ReEnablesTool(t *testing.T) { + rt := setupConfigFilterRuntime(t, []*config.ServerConfig{ + { + Name: "github", + Enabled: true, + EnabledTools: []string{"list_issues", "get_issue"}, + }, + }) + + // Manually mark get_issue as disabled (simulating a prior API call) + require.NoError(t, rt.storageManager.SaveToolApproval(&storage.ToolApprovalRecord{ + ServerName: "github", + ToolName: "get_issue", + Status: storage.ToolApprovalStatusApproved, + Disabled: true, + })) + + tools := []*config.ToolMetadata{ + {ServerName: "github", Name: "list_issues", Description: "List issues", ParamsJSON: `{}`}, + {ServerName: "github", Name: "get_issue", Description: "Get issue", ParamsJSON: `{}`}, + } + + err := rt.applyConfigToolFilter("github", tools) + require.NoError(t, err) + + // get_issue is in the enabled_tools list — must be re-enabled + record, err := rt.storageManager.GetToolApproval("github", "get_issue") + require.NoError(t, err) + assert.False(t, record.Disabled, "get_issue should be re-enabled by config") +} + +// TestApplyDifferentialToolUpdate_RespectsEnabledToolsConfig is an integration +// test verifying that applyDifferentialToolUpdate honours enabled_tools from the +// server config: tools not in the list end up with Disabled=true in storage. +func TestApplyDifferentialToolUpdate_RespectsEnabledToolsConfig(t *testing.T) { + rt := setupConfigFilterRuntime(t, []*config.ServerConfig{ + { + Name: "github", + Enabled: true, + EnabledTools: []string{"list_issues"}, + }, + }) + + tools := []*config.ToolMetadata{ + {ServerName: "github", Name: "list_issues", Description: "List issues", ParamsJSON: `{}`}, + {ServerName: "github", Name: "create_issue", Description: "Create issue", ParamsJSON: `{}`}, + } + + err := rt.applyDifferentialToolUpdate(t.Context(), "github", tools) + require.NoError(t, err) + + blocked, err := rt.storageManager.GetToolApproval("github", "create_issue") + require.NoError(t, err) + assert.True(t, blocked.Disabled, "create_issue should be disabled by enabled_tools config") +} \ No newline at end of file diff --git a/internal/runtime/tool_quarantine.go b/internal/runtime/tool_quarantine.go index e159ad0d..a4ff96ef 100644 --- a/internal/runtime/tool_quarantine.go +++ b/internal/runtime/tool_quarantine.go @@ -921,3 +921,72 @@ func (r *Runtime) emitToolQuarantineEvent(serverName, toolName, action, oldHash, } r.publishEvent(newEvent(EventTypeActivityToolQuarantineChange, payload)) } + +// applyConfigToolFilter syncs the enabled_tools / disabled_tools lists from +// the server's static config into BBolt ToolApprovalRecord.Disabled flags. +// It is called from applyDifferentialToolUpdate so the config-declared filter +// is enforced on every connect / tool-refresh cycle. +// +// - enabled_tools (allowlist): every tool NOT in the list is disabled. +// - disabled_tools (denylist): every tool IN the list is disabled; others +// are (re-)enabled so a tool removed from the denylist becomes visible. +// +// When neither field is set the function is a no-op — no records are written. +func (r *Runtime) applyConfigToolFilter(serverName string, tools []*config.ToolMetadata) error { + if r.storageManager == nil { + return nil + } + + // Read from the in-memory config snapshot (EnabledTools/DisabledTools are + // static declarations, not runtime state — no BBolt read needed). + var serverCfg *config.ServerConfig + for _, sc := range r.Config().Servers { + if sc.Name == serverName { + serverCfg = sc + break + } + } + if serverCfg == nil { + return nil + } + + hasAllowList := len(serverCfg.EnabledTools) > 0 + hasDenyList := len(serverCfg.DisabledTools) > 0 + + if !hasAllowList && !hasDenyList { + return nil + } + + allowSet := make(map[string]bool, len(serverCfg.EnabledTools)) + for _, t := range serverCfg.EnabledTools { + allowSet[t] = true + } + denySet := make(map[string]bool, len(serverCfg.DisabledTools)) + for _, t := range serverCfg.DisabledTools { + denySet[t] = true + } + + for _, tool := range tools { + toolName := tool.Name + if idx := strings.Index(toolName, ":"); idx != -1 { + toolName = toolName[idx+1:] + } + + var shouldEnable bool + if hasAllowList { + shouldEnable = allowSet[toolName] + } else { + shouldEnable = !denySet[toolName] + } + + if _, err := r.setToolEnabledNoEmit(serverName, toolName, shouldEnable, "config"); err != nil { + r.logger.Warn("applyConfigToolFilter: failed to set tool enabled state", + zap.String("server", serverName), + zap.String("tool", toolName), + zap.Bool("enabled", shouldEnable), + zap.Error(err)) + } + } + + return nil +}