feat(quarantine): atomic Block (approve+disable) endpoint for tools (MCP-2198)#653
Conversation
Deploying mcpproxy-docs with
|
| Latest commit: |
5f9b1b4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://85e3dc6a.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://feat-mcp-2198-block-tools.mcpproxy-docs.pages.dev |
…MCP-2198)
Add an atomic "block" = approve+disable for quarantined tools, performed
server-side as a single all-or-nothing write per tool so a tool is never
observably left in the approved+enabled state.
- internal/runtime/tool_quarantine.go: BlockTools / BlockAllTools — promote
status to approved (ReasonUserApprove) AND set Disabled in one
SaveToolApproval; emit tool_blocked activity + tools_blocked SSE. Block only
ever disables, so config-denied tools need no special handling.
- internal/httpapi/server.go: POST /api/v1/servers/{id}/tools/block
(handleBlockTools), mirrors handleApproveTools — {tools:[...]} or
{block_all:true}; 400 if neither.
- internal/server/mcp.go: block_tool / block_all_tools ops on quarantine_security.
- internal/server/server.go: controller passthroughs.
- Tests: httpapi + runtime (end state approved && Disabled; specific/all/empty).
Backend lane of GH #632 (parent MCP-2196).
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Dumbris
force-pushed
the
feat/mcp-2198-block-tools
branch
from
7e9699e to
8c8b2a3
Compare
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27488135448 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
Critic (Gemini Flash Lite) review — feat(quarantine): atomic Block (approve+disable) endpoint for tools (MCP-2198) PR #653 / request_changes
|
|
Critic (Gemini Flash Lite) review — feat(quarantine): atomic Block (approve+disable) endpoint for tools (MCP-2198) PR #653 / accept |
QA Gate — PASS ✅ (head
|
…ests Addresses CodexReviewer request_changes on PR #653: 1. The /servers/{id}/tools/block @description swag annotation was split across multiple comment lines, so swag truncated the generated oas/swagger.yaml description at the first line. Collapse it to a single line and regenerate (make swagger) so the full sentence is emitted. 2. Add internal/server coverage for the new quarantine_security MCP ops block_tool / block_all_tools: dispatch routing, required-arg errors (missing name / tool_name), and the atomic approve+disable success path (RV-3). A runtime-backed proxy helper shares storage with the runtime so the handlers' runtime.BlockTools path executes for real. Related #653
![mcpproxy-gatekeeper[bot]](https://avatars.githubusercontent.com/u/216428285?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Gatekeeper approval — Codex review verdict: ACCEPT.
This approval is posted automatically by the MCPProxy Gatekeeper App on behalf of the Codex reviewer (verdict of record lives in the Paperclip review thread). Author≠approver satisfied; QA + CI gates enforced separately.
Auto-approved per Model B (MCP-1249).
2 participants
Summary
Backend lane of GH #632 (parent MCP-2196). Adds an atomic "block" = approve + disable for quarantined tools, performed server-side as a single all-or-nothing write per tool, so a tool is never observably left in the approved+enabled state — the invariant this endpoint exists to guarantee.
What changed
internal/runtime/tool_quarantine.go—BlockTools/BlockAllTools: per tool, enforce the quarantine invariant (ReasonUserApprove), then promote status → approved and setDisabledin a singleSaveToolApproval. Emitstool_blockedactivity + onetools_blockedSSE per call. Block only ever disables, so config-denied tools (enabled_tools/disabled_tools) need no special handling.internal/httpapi/server.go—POST /api/v1/servers/{id}/tools/block(handleBlockTools), mirrorshandleApproveTools: accepts{"tools":[...]}or{"block_all":true};400if neither. Response{blocked:int, tools?:[], message}.internal/server/mcp.go—block_tool/block_all_toolsoperations added to thequarantine_securityMCP tool enum + handlers →runtime.BlockTools(...,"mcp").internal/server/server.go— controller passthroughs.Why a single combined write
ApproveToolsfollowed bySetToolEnabled(disabled)is two sequential saves — a crash/IO error between them yields an approved+enabled tool, the exact state callers want to avoid. Folding both into one write removes that window.Tests (TDD)
internal/runtime/tool_quarantine_test.go—TestBlockTools_ApprovedAndDisabled,TestBlockAllTools,TestBlockTools_ConfigDeniedToolNeverEnabled,TestBlockTools_MissingRecordSkipped(end stateapproved && Disabled; config-denied never enabled).internal/httpapi/tool_quarantine_test.go— specific / block_all / empty-error / invalid-JSON / error paths.Docs / OAS
oas/swagger.yaml+oas/docs.goregenerated (make swagger);make swagger-verifyclean.docs/api/rest-api.md,docs/features/security-quarantine.md,CLAUDE.mdendpoint table +quarantine_securityop list updated.Verification
go build ./...✅ ·make swagger-verify✅ clean./scripts/run-linter.sh→ 0 issues ✅go test ./internal/runtime/ ./internal/httpapi/ -run Block→ 11/11 pass ✅go test ./internal/storage/(field-coverage canary) ✅Closes #632. Related parent MCP-2196.