diff --git a/cmd/mcpproxy/tools_approval.go b/cmd/mcpproxy/tools_approval.go new file mode 100644 index 000000000..8509969c8 --- /dev/null +++ b/cmd/mcpproxy/tools_approval.go @@ -0,0 +1,200 @@ +package main + +import ( + "context" + "fmt" + "os" + "sort" + "strings" + "time" + + "github.com/spf13/cobra" +) + +var ( + // tools approve/reject flags + toolsApprovalServer string // --server scoping + toolsApprovalAll bool // --all (requires --server) +) + +func newToolsApproveCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "approve [:...]", + Short: "Approve tools pending tool-level quarantine (Spec 032)", + Long: `Approve one or more tools that are pending or changed in tool-level +quarantine, clearing them for use without the Web UI or MCP. + +Targets may be given as : pairs, or as bare tool names when +scoped with --server. Use --server --all to approve every pending or +changed tool for a server. + +Examples: + mcpproxy tools approve github:create_issue + mcpproxy tools approve github:create_issue github:list_repos + mcpproxy tools approve --server github create_issue list_repos + mcpproxy tools approve --server github --all`, + RunE: func(_ *cobra.Command, args []string) error { + return runToolsApproval(args, false) + }, + } + cmd.Flags().StringVarP(&toolsApprovalServer, "server", "s", "", "Scope bare tool names to this server (required with --all)") + cmd.Flags().BoolVar(&toolsApprovalAll, "all", false, "Approve all pending/changed tools for --server") + return cmd +} + +func newToolsRejectCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "reject [:...]", + Short: "Reject (block) tools pending tool-level quarantine (Spec 032)", + Long: `Reject one or more tools pending tool-level quarantine. Reject maps to the +"block" action: the tool is atomically approved AND disabled (hidden), so it is +never left in the approved+enabled state. Mirrors the Web UI "Block" button. + +Targets may be given as : pairs, or as bare tool names when +scoped with --server. Use --server --all to reject every pending or +changed tool for a server. + +Examples: + mcpproxy tools reject github:delete_repo + mcpproxy tools reject --server github delete_repo force_push + mcpproxy tools reject --server github --all`, + RunE: func(_ *cobra.Command, args []string) error { + return runToolsApproval(args, true) + }, + } + cmd.Flags().StringVarP(&toolsApprovalServer, "server", "s", "", "Scope bare tool names to this server (required with --all)") + cmd.Flags().BoolVar(&toolsApprovalAll, "all", false, "Reject all pending/changed tools for --server") + return cmd +} + +// resolveToolApprovalTargets validates approve/reject invocation flags and +// returns the per-server tool grouping. +// +// - With all=true: requires a non-empty server and no positional args, and +// returns {server: nil} with allMode=true (caller issues a single +// approve-all/block-all call for that server). +// - Otherwise: each positional arg is either an explicit ":" +// pair (the colon form takes precedence) or a bare tool name scoped to the +// --server flag. Bare names without --server are rejected. +func resolveToolApprovalTargets(args []string, server string, all bool) (groups map[string][]string, allMode bool, err error) { + if all { + if server == "" { + return nil, false, fmt.Errorf("--all requires --server ") + } + if len(args) > 0 { + return nil, false, fmt.Errorf("--all cannot be combined with explicit : targets") + } + return map[string][]string{server: nil}, true, nil + } + + if len(args) == 0 { + return nil, false, fmt.Errorf("no targets specified: provide : args, or use --server --all") + } + + groups = make(map[string][]string) + for _, arg := range args { + if strings.Contains(arg, ":") { + srv, tool, parseErr := parseServerTool(arg) + if parseErr != nil { + return nil, false, parseErr + } + groups[srv] = append(groups[srv], tool) + continue + } + // Bare tool name — must be scoped via --server. + if server == "" { + return nil, false, fmt.Errorf("invalid target %q: use : format or scope bare names with --server", arg) + } + groups[server] = append(groups[server], arg) + } + return groups, false, nil +} + +// runToolsApproval implements the `tools approve` and `tools reject` commands. +// When block is true it calls the block endpoint (approve+disable); otherwise +// it approves. Each server group is processed independently; the command exits +// non-zero if any group fails but still attempts the rest. +func runToolsApproval(args []string, block bool) error { + groups, allMode, err := resolveToolApprovalTargets(args, toolsApprovalServer, toolsApprovalAll) + if err != nil { + return err + } + + client, _, err := newSecurityCLIClient() + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + verb := "approve" + pastTense := "approved" + if block { + verb = "reject" + pastTense = "blocked" + } + + // Deterministic ordering for stable output and JSON results. + servers := make([]string, 0, len(groups)) + for srv := range groups { + servers = append(servers, srv) + } + sort.Strings(servers) + + type serverResult struct { + Server string `json:"server"` + All bool `json:"all,omitempty"` + Tools []string `json:"tools,omitempty"` + Count int `json:"count"` + Error string `json:"error,omitempty"` + } + + var results []serverResult + anyFailed := false + for _, srv := range servers { + tools := groups[srv] + var count int + var callErr error + if block { + count, callErr = client.BlockTools(ctx, srv, tools, allMode) + } else { + count, callErr = client.ApproveTools(ctx, srv, tools, allMode) + } + res := serverResult{Server: srv, All: allMode, Tools: tools, Count: count} + if callErr != nil { + anyFailed = true + res.Error = callErr.Error() + } + results = append(results, res) + } + + format := ResolveOutputFormat() + if format == "json" || format == "yaml" { + if ferr := formatAndPrint(format, results); ferr != nil { + return ferr + } + if anyFailed { + return fmt.Errorf("one or more servers failed to %s", verb) + } + return nil + } + + // Table / human-readable output: one line per server group. + for _, res := range results { + if res.Error != "" { + fmt.Fprintf(os.Stderr, "FAILED %s: %s\n", res.Server, res.Error) + continue + } + scope := fmt.Sprintf("%d tool(s)", res.Count) + if res.All { + scope = fmt.Sprintf("all pending/changed (%d tool(s))", res.Count) + } + fmt.Printf("OK %s: %s %s\n", res.Server, pastTense, scope) + } + + if anyFailed { + return fmt.Errorf("one or more servers failed to %s (see above)", verb) + } + return nil +} diff --git a/cmd/mcpproxy/tools_approval_test.go b/cmd/mcpproxy/tools_approval_test.go new file mode 100644 index 000000000..61e978cd1 --- /dev/null +++ b/cmd/mcpproxy/tools_approval_test.go @@ -0,0 +1,82 @@ +package main + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestResolveToolApprovalTargets_ServerTool verifies that positional +// : args are parsed and grouped per server. +func TestResolveToolApprovalTargets_ServerTool(t *testing.T) { + groups, allMode, err := resolveToolApprovalTargets( + []string{"github:create_issue", "github:list_repos", "gitlab:merge"}, + "", false) + require.NoError(t, err) + assert.False(t, allMode) + assert.Len(t, groups, 2) + assert.ElementsMatch(t, []string{"create_issue", "list_repos"}, groups["github"]) + assert.ElementsMatch(t, []string{"merge"}, groups["gitlab"]) +} + +// TestResolveToolApprovalTargets_BareToolsWithServer verifies that bare tool +// names are scoped to the --server flag. +func TestResolveToolApprovalTargets_BareToolsWithServer(t *testing.T) { + groups, allMode, err := resolveToolApprovalTargets( + []string{"create_issue", "list_repos"}, "github", false) + require.NoError(t, err) + assert.False(t, allMode) + assert.Len(t, groups, 1) + assert.ElementsMatch(t, []string{"create_issue", "list_repos"}, groups["github"]) +} + +// TestResolveToolApprovalTargets_AllRequiresServer verifies that --all without +// --server is rejected. +func TestResolveToolApprovalTargets_AllRequiresServer(t *testing.T) { + _, _, err := resolveToolApprovalTargets(nil, "", true) + require.Error(t, err) + assert.Contains(t, err.Error(), "--server") +} + +// TestResolveToolApprovalTargets_All verifies that --all --server yields a +// single server entry in all-mode with an empty tool list. +func TestResolveToolApprovalTargets_All(t *testing.T) { + groups, allMode, err := resolveToolApprovalTargets(nil, "github", true) + require.NoError(t, err) + assert.True(t, allMode) + assert.Len(t, groups, 1) + assert.Empty(t, groups["github"]) +} + +// TestResolveToolApprovalTargets_AllRejectsPositional verifies that mixing +// positional targets with --all is rejected. +func TestResolveToolApprovalTargets_AllRejectsPositional(t *testing.T) { + _, _, err := resolveToolApprovalTargets([]string{"github:create_issue"}, "github", true) + require.Error(t, err) +} + +// TestResolveToolApprovalTargets_NoTargets verifies that an empty invocation +// (no args, no --all) is rejected with guidance. +func TestResolveToolApprovalTargets_NoTargets(t *testing.T) { + _, _, err := resolveToolApprovalTargets(nil, "", false) + require.Error(t, err) +} + +// TestResolveToolApprovalTargets_BareToolNoServer verifies that a bare tool +// name without --server (and no colon) is rejected. +func TestResolveToolApprovalTargets_BareToolNoServer(t *testing.T) { + _, _, err := resolveToolApprovalTargets([]string{"create_issue"}, "", false) + require.Error(t, err) + assert.Contains(t, err.Error(), "create_issue") +} + +// TestResolveToolApprovalTargets_MixedColonAndServerFlag verifies that explicit +// server:tool args take precedence over the --server flag. +func TestResolveToolApprovalTargets_MixedColonAndServerFlag(t *testing.T) { + groups, _, err := resolveToolApprovalTargets( + []string{"gitlab:merge", "bare_tool"}, "github", false) + require.NoError(t, err) + assert.ElementsMatch(t, []string{"merge"}, groups["gitlab"]) + assert.ElementsMatch(t, []string{"bare_tool"}, groups["github"]) +} diff --git a/cmd/mcpproxy/tools_cmd.go b/cmd/mcpproxy/tools_cmd.go index df4f31ca3..7bc723a6e 100644 --- a/cmd/mcpproxy/tools_cmd.go +++ b/cmd/mcpproxy/tools_cmd.go @@ -193,6 +193,8 @@ func init() { toolsCmd.AddCommand(toolsListCmd) toolsCmd.AddCommand(toolsEnableCmd) toolsCmd.AddCommand(toolsDisableCmd) + toolsCmd.AddCommand(newToolsApproveCmd()) + toolsCmd.AddCommand(newToolsRejectCmd()) initToolsFlags() } diff --git a/docs/cli-management-commands.md b/docs/cli-management-commands.md index 9d085e225..186dbeb90 100644 --- a/docs/cli-management-commands.md +++ b/docs/cli-management-commands.md @@ -670,6 +670,74 @@ echo "exit: $?" # non-zero if any failed --- +### `mcpproxy tools approve [...]` + +Approve tools pending tool-level quarantine (Spec 032), clearing `pending` / +`changed` tools for use without the Web UI or MCP. Requires daemon. + +**Usage:** +```bash +mcpproxy tools approve [...] +mcpproxy tools approve --server [...] +mcpproxy tools approve --server --all +``` + +**Flags:** +- `-s, --server ` - Scope bare tool names to this server (required with `--all`) +- `--all` - Approve every pending/changed tool for `--server` + +**Examples:** +```bash +mcpproxy tools approve github:create_issue +mcpproxy tools approve github:create_issue github:list_repos +mcpproxy tools approve --server github create_issue list_repos +mcpproxy tools approve --server github --all +mcpproxy tools approve --server github --all -o json +``` + +**Behavior:** +- Targets are either `:` pairs (the colon form wins) or bare tool + names scoped via `--server`; bare names without `--server` are rejected +- Targets are grouped per server; each server group is processed independently +- `--all` requires `--server` and cannot be combined with explicit targets +- Prints `OK : approved N tool(s)` per server group; exits non-zero if + any server group failed +- `-o json|yaml` (or `MCPPROXY_OUTPUT`) emits a structured per-server result array + +--- + +### `mcpproxy tools reject [...]` + +Reject (block) tools pending tool-level quarantine (Spec 032). Reject maps to +the **block** action: the tool is atomically approved **and** disabled (hidden), +so it is never left in the approved+enabled state — mirroring the Web UI "Block" +button. Requires daemon. + +**Usage:** +```bash +mcpproxy tools reject [...] +mcpproxy tools reject --server [...] +mcpproxy tools reject --server --all +``` + +**Flags:** +- `-s, --server ` - Scope bare tool names to this server (required with `--all`) +- `--all` - Reject every pending/changed tool for `--server` + +**Examples:** +```bash +mcpproxy tools reject github:delete_repo +mcpproxy tools reject --server github delete_repo force_push +mcpproxy tools reject --server github --all +``` + +**Behavior:** +- Same target parsing, per-server grouping, exit-code, and output-format + semantics as `tools approve` +- Prints `OK : blocked N tool(s)` per server group + +--- + ## Exit Codes - `0` - Success diff --git a/internal/cliclient/client.go b/internal/cliclient/client.go index a51afb928..e6901764f 100644 --- a/internal/cliclient/client.go +++ b/internal/cliclient/client.go @@ -1710,6 +1710,67 @@ func (c *Client) ApproveTools(ctx context.Context, serverName string, toolNames return apiResp.Data.Approved, nil } +// BlockTools blocks specific tools or all pending/changed tools for a server +// (Spec 032). Block atomically approves AND disables the tool so it is never +// left in the approved+enabled state — mirroring the Web UI "Block" action. +func (c *Client) BlockTools(ctx context.Context, serverName string, toolNames []string, blockAll bool) (int, error) { + url := fmt.Sprintf("%s/api/v1/servers/%s/tools/block", c.baseURL, serverName) + + reqBody := struct { + Tools []string `json:"tools,omitempty"` + BlockAll bool `json:"block_all,omitempty"` + }{ + Tools: toolNames, + BlockAll: blockAll, + } + + bodyBytes, err := json.Marshal(reqBody) + if err != nil { + return 0, fmt.Errorf("failed to marshal request: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(bodyBytes)) + if err != nil { + return 0, fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + c.prepareRequest(ctx, req) + + resp, err := c.httpClient.Do(req) + if err != nil { + return 0, fmt.Errorf("failed to call block tools API: %w", err) + } + defer resp.Body.Close() + + respBytes, err := io.ReadAll(resp.Body) + if err != nil { + return 0, fmt.Errorf("failed to read response: %w", err) + } + + if resp.StatusCode != http.StatusOK { + return 0, fmt.Errorf("API returned status %d: %s", resp.StatusCode, string(respBytes)) + } + + var apiResp struct { + Success bool `json:"success"` + Data struct { + Blocked int `json:"blocked"` + } `json:"data"` + Error string `json:"error"` + RequestID string `json:"request_id"` + } + + if err := json.Unmarshal(respBytes, &apiResp); err != nil { + return 0, fmt.Errorf("failed to parse response: %w", err) + } + + if !apiResp.Success { + return 0, parseAPIError(apiResp.Error, apiResp.RequestID) + } + + return apiResp.Data.Blocked, nil +} + // ListRegistries returns the MCP server registries known to the daemon // (spec 070). Mirrors GetServers: GET /api/v1/registries → data.registries. func (c *Client) ListRegistries(ctx context.Context) ([]map[string]interface{}, error) { diff --git a/internal/cliclient/client_block_tools_test.go b/internal/cliclient/client_block_tools_test.go new file mode 100644 index 000000000..99a92af17 --- /dev/null +++ b/internal/cliclient/client_block_tools_test.go @@ -0,0 +1,110 @@ +package cliclient + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" +) + +// TestClient_BlockTools_Specific verifies BlockTools posts the tool list to the +// block endpoint and returns the "blocked" count from the API envelope. +func TestClient_BlockTools_Specific(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/api/v1/servers/github/tools/block", r.URL.Path) + assert.Equal(t, http.MethodPost, r.Method) + + body, _ := io.ReadAll(r.Body) + var req struct { + Tools []string `json:"tools"` + BlockAll bool `json:"block_all"` + } + require.NoError(t, json.Unmarshal(body, &req)) + assert.Equal(t, []string{"create_issue", "delete_repo"}, req.Tools) + assert.False(t, req.BlockAll) + + response := map[string]interface{}{ + "success": true, + "data": map[string]interface{}{ + "blocked": 2, + }, + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(response) + })) + defer ts.Close() + + client := NewClient(ts.URL, zap.NewNop().Sugar()) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + count, err := client.BlockTools(ctx, "github", []string{"create_issue", "delete_repo"}, false) + require.NoError(t, err) + assert.Equal(t, 2, count) +} + +// TestClient_BlockTools_All verifies BlockTools sends block_all=true (and no +// tools array) when blockAll is set. +func TestClient_BlockTools_All(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/api/v1/servers/github/tools/block", r.URL.Path) + + body, _ := io.ReadAll(r.Body) + var req struct { + Tools []string `json:"tools"` + BlockAll bool `json:"block_all"` + } + require.NoError(t, json.Unmarshal(body, &req)) + assert.True(t, req.BlockAll) + assert.Empty(t, req.Tools) + + response := map[string]interface{}{ + "success": true, + "data": map[string]interface{}{ + "blocked": 5, + }, + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(response) + })) + defer ts.Close() + + client := NewClient(ts.URL, zap.NewNop().Sugar()) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + count, err := client.BlockTools(ctx, "github", nil, true) + require.NoError(t, err) + assert.Equal(t, 5, count) +} + +// TestClient_BlockTools_APIError verifies a non-200 response surfaces an error. +func TestClient_BlockTools_APIError(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + response := map[string]interface{}{ + "success": false, + "error": "Server not found", + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(w).Encode(response) + })) + defer ts.Close() + + client := NewClient(ts.URL, zap.NewNop().Sugar()) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + _, err := client.BlockTools(ctx, "nonexistent", []string{"foo"}, false) + require.Error(t, err) + assert.Contains(t, err.Error(), "Server not found") +}