From 82aca633a23d64896dc087474958da0b1a4b161e Mon Sep 17 00:00:00 2001 From: Claude Code Date: Sat, 28 Feb 2026 19:20:14 +0200 Subject: [PATCH] fix: correct quarantine API docs and wire tray unquarantine menu Fix B: Documentation showed incorrect curl examples using POST /quarantine with {"quarantined": false} body. Actual API uses separate POST /quarantine and POST /unquarantine endpoints with no body. Fixed in rest-api.md, security-quarantine.md, cli-management-commands.md, and CLAUDE.md. Fix C: Tray unquarantine menu returned "not yet supported via API". Added QuarantineServer() and UnquarantineServer() methods to tray API client and wired adapter to use them. Added 5 tests (27 total pass). Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 3 +- cmd/mcpproxy-tray/internal/api/adapter.go | 13 +-- .../internal/api/adapter_test.go | 99 ++++++++++++++++--- cmd/mcpproxy-tray/internal/api/client.go | 32 ++++++ docs/api/rest-api.md | 11 +-- docs/cli-management-commands.md | 6 +- docs/features/security-quarantine.md | 6 +- 7 files changed, 136 insertions(+), 34 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3639aa59..1a39d2ee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -214,7 +214,8 @@ See [docs/configuration.md](docs/configuration.md) for complete reference. | `GET /api/v1/status` | Server status and statistics | | `GET /api/v1/servers` | List all upstream servers | | `POST /api/v1/servers/{name}/enable` | Enable/disable server | -| `POST /api/v1/servers/{name}/quarantine` | Quarantine/unquarantine server | +| `POST /api/v1/servers/{name}/quarantine` | Quarantine a server | +| `POST /api/v1/servers/{name}/unquarantine` | Unquarantine a server | | `GET /api/v1/tools` | Search tools across servers | | `GET /api/v1/activity` | List activity records with filtering | | `GET /api/v1/activity/{id}` | Get activity record details | diff --git a/cmd/mcpproxy-tray/internal/api/adapter.go b/cmd/mcpproxy-tray/internal/api/adapter.go index b111e2be..ca5183da 100644 --- a/cmd/mcpproxy-tray/internal/api/adapter.go +++ b/cmd/mcpproxy-tray/internal/api/adapter.go @@ -16,6 +16,8 @@ type ClientInterface interface { GetServers() ([]Server, error) GetInfo() (map[string]interface{}, error) EnableServer(serverName string, enabled bool) error + QuarantineServer(serverName string) error + UnquarantineServer(serverName string) error TriggerOAuthLogin(serverName string) error StatusChannel() <-chan StatusUpdate } @@ -210,9 +212,7 @@ func (a *ServerAdapter) GetQuarantinedServers() ([]map[string]interface{}, error // UnquarantineServer removes a server from quarantine func (a *ServerAdapter) UnquarantineServer(serverName string) error { - // This functionality is not available in the current API - // Would need to be added to the API first - return fmt.Errorf("UnquarantineServer not yet supported via API for %s", serverName) + return a.client.UnquarantineServer(serverName) } // EnableServer enables or disables a server @@ -222,9 +222,10 @@ func (a *ServerAdapter) EnableServer(serverName string, enabled bool) error { // QuarantineServer sets quarantine status for a server func (a *ServerAdapter) QuarantineServer(serverName string, quarantined bool) error { - // This functionality is not available in the current API - // Would need to be added to the API first - return fmt.Errorf("QuarantineServer not yet supported via API for %s (quarantined=%t)", serverName, quarantined) + if quarantined { + return a.client.QuarantineServer(serverName) + } + return a.client.UnquarantineServer(serverName) } // GetAllServers returns all servers diff --git a/cmd/mcpproxy-tray/internal/api/adapter_test.go b/cmd/mcpproxy-tray/internal/api/adapter_test.go index a01eeb76..2dc0c2de 100644 --- a/cmd/mcpproxy-tray/internal/api/adapter_test.go +++ b/cmd/mcpproxy-tray/internal/api/adapter_test.go @@ -14,21 +14,25 @@ import ( // MockClient implements ClientInterface for testing type MockClient struct { - servers []Server - serversErr error - info map[string]interface{} - infoErr error - enableErr error - oauthErr error - enabledServers map[string]bool // tracks enable/disable calls - oauthTriggered []string // tracks OAuth login calls - statusCh chan StatusUpdate + servers []Server + serversErr error + info map[string]interface{} + infoErr error + enableErr error + quarantineErr error + oauthErr error + enabledServers map[string]bool // tracks enable/disable calls + quarantinedServers map[string]bool // tracks quarantine calls + unquarantinedServers []string // tracks unquarantine calls + oauthTriggered []string // tracks OAuth login calls + statusCh chan StatusUpdate } func NewMockClient() *MockClient { return &MockClient{ - enabledServers: make(map[string]bool), - statusCh: make(chan StatusUpdate, 10), + enabledServers: make(map[string]bool), + quarantinedServers: make(map[string]bool), + statusCh: make(chan StatusUpdate, 10), } } @@ -54,6 +58,23 @@ func (m *MockClient) EnableServer(serverName string, enabled bool) error { return nil } +func (m *MockClient) QuarantineServer(serverName string) error { + if m.quarantineErr != nil { + return m.quarantineErr + } + m.quarantinedServers[serverName] = true + return nil +} + +func (m *MockClient) UnquarantineServer(serverName string) error { + if m.quarantineErr != nil { + return m.quarantineErr + } + m.unquarantinedServers = append(m.unquarantinedServers, serverName) + delete(m.quarantinedServers, serverName) + return nil +} + func (m *MockClient) TriggerOAuthLogin(serverName string) error { if m.oauthErr != nil { return m.oauthErr @@ -466,6 +487,62 @@ func TestServerAdapter_TriggerOAuthLogin_Error(t *testing.T) { assert.Error(t, err) } +// ============================================================================= +// ServerAdapter.UnquarantineServer Tests +// ============================================================================= + +func TestServerAdapter_UnquarantineServer_Success(t *testing.T) { + mock := NewMockClient() + adapter := NewServerAdapter(mock) + + err := adapter.UnquarantineServer("suspicious-server") + require.NoError(t, err) + assert.Contains(t, mock.unquarantinedServers, "suspicious-server") +} + +func TestServerAdapter_UnquarantineServer_Error(t *testing.T) { + mock := NewMockClient() + mock.quarantineErr = errors.New("server not found") + adapter := NewServerAdapter(mock) + + err := adapter.UnquarantineServer("missing-server") + assert.Error(t, err) +} + +// ============================================================================= +// ServerAdapter.QuarantineServer Tests +// ============================================================================= + +func TestServerAdapter_QuarantineServer_Quarantine(t *testing.T) { + mock := NewMockClient() + adapter := NewServerAdapter(mock) + + err := adapter.QuarantineServer("test-server", true) + require.NoError(t, err) + assert.True(t, mock.quarantinedServers["test-server"]) +} + +func TestServerAdapter_QuarantineServer_Unquarantine(t *testing.T) { + mock := NewMockClient() + mock.quarantinedServers["test-server"] = true + adapter := NewServerAdapter(mock) + + err := adapter.QuarantineServer("test-server", false) + require.NoError(t, err) + assert.Contains(t, mock.unquarantinedServers, "test-server") + _, stillQuarantined := mock.quarantinedServers["test-server"] + assert.False(t, stillQuarantined) +} + +func TestServerAdapter_QuarantineServer_Error(t *testing.T) { + mock := NewMockClient() + mock.quarantineErr = errors.New("server not found") + adapter := NewServerAdapter(mock) + + err := adapter.QuarantineServer("missing-server", true) + assert.Error(t, err) +} + // ============================================================================= // Integration Test: Health Data Flow Verification // ============================================================================= diff --git a/cmd/mcpproxy-tray/internal/api/client.go b/cmd/mcpproxy-tray/internal/api/client.go index d1791422..fa668122 100644 --- a/cmd/mcpproxy-tray/internal/api/client.go +++ b/cmd/mcpproxy-tray/internal/api/client.go @@ -677,6 +677,38 @@ func (c *Client) GetServerTools(serverName string) ([]Tool, error) { return result, nil } +// QuarantineServer places a server in quarantine +func (c *Client) QuarantineServer(serverName string) error { + endpoint := fmt.Sprintf("/api/v1/servers/%s/quarantine", serverName) + + resp, err := c.makeRequest("POST", endpoint, nil) + if err != nil { + return err + } + + if !resp.Success { + return fmt.Errorf("API error: %s", resp.Error) + } + + return nil +} + +// UnquarantineServer removes a server from quarantine +func (c *Client) UnquarantineServer(serverName string) error { + endpoint := fmt.Sprintf("/api/v1/servers/%s/unquarantine", serverName) + + resp, err := c.makeRequest("POST", endpoint, nil) + if err != nil { + return err + } + + if !resp.Success { + return fmt.Errorf("API error: %s", resp.Error) + } + + return nil +} + // SearchTools searches for tools // GetInfo fetches server information from /api/v1/info endpoint func (c *Client) GetInfo() (map[string]interface{}, error) { diff --git a/docs/api/rest-api.md b/docs/api/rest-api.md index 65fb0562..669a47ad 100644 --- a/docs/api/rest-api.md +++ b/docs/api/rest-api.md @@ -177,14 +177,11 @@ Disable a server. #### POST /api/v1/servers/{name}/quarantine -Set quarantine status. +Place a server in quarantine to prevent tool execution. No request body required. -**Request Body:** -```json -{ - "quarantined": true -} -``` +#### POST /api/v1/servers/{name}/unquarantine + +Remove a server from quarantine to allow tool execution. No request body required. #### POST /api/v1/servers/{name}/restart diff --git a/docs/cli-management-commands.md b/docs/cli-management-commands.md index 92297daf..6afce77a 100644 --- a/docs/cli-management-commands.md +++ b/docs/cli-management-commands.md @@ -497,10 +497,8 @@ mcpproxy upstream list # 🔒 notion http 0 Pending approval Approve in Web UI # Approve in web UI or via API: -curl -X POST "http://localhost:8080/api/v1/servers/notion/quarantine" \ - -H "X-API-Key: your-key" \ - -H "Content-Type: application/json" \ - -d '{"quarantined": false}' +curl -X POST "http://localhost:8080/api/v1/servers/notion/unquarantine" \ + -H "X-API-Key: your-key" ``` ### Bulk Operations Warning diff --git a/docs/features/security-quarantine.md b/docs/features/security-quarantine.md index 01eff3b5..08943c73 100644 --- a/docs/features/security-quarantine.md +++ b/docs/features/security-quarantine.md @@ -102,9 +102,7 @@ mcpproxy upstream list ```bash curl -X POST \ -H "X-API-Key: your-key" \ - -H "Content-Type: application/json" \ - -d '{"quarantined": false}' \ - http://127.0.0.1:8080/api/v1/servers/server-name/quarantine + http://127.0.0.1:8080/api/v1/servers/server-name/unquarantine ``` **Config File:** @@ -132,8 +130,6 @@ If you need to quarantine a previously approved server: ```bash curl -X POST \ -H "X-API-Key: your-key" \ - -H "Content-Type: application/json" \ - -d '{"quarantined": true}' \ http://127.0.0.1:8080/api/v1/servers/server-name/quarantine ```