Skip to content

Commit 472314b

Browse files
Dumbrisclaude
andauthored
feat(prompts): finish prompt surfaces — rug-pull approve ops + F9 tool arg (#1011)
Completes the prompt-aggregation feature at the MCP/REST level for release. - Rug-pull approve surface (spec 100 FR-7): quarantine_security gains inspect_prompts, approve_prompt, approve_all_prompts operations, delegating to the ApprovePrompt/ApproveAllPrompts mutators. A held (pending/changed) prompt is now approvable by an agent/operator via any MCP client instead of only programmatically or by flipping the server to trust:auto. - F9: the upstream_servers MCP tool gains the expose_prompts arg (add/patch), so the per-server prompt-aggregation override is reachable from the tool, not just the REST PATCH shipped in #1006. - Regenerated the three frozen tool-surface goldens for both schema additions (merge-base via the write-goldens env hook; pre099 via byte-preserving splice; prefeature via map update). - Docs: security-quarantine.md documents the prompt rug-pull baseline and the new quarantine_security prompt operations. Tests: quarantine_security prompt op handlers (inspect/approve/approve-all + missing-arg error) and the F9 patch-builder expose_prompts mapping. Fast-follow (Web UI): a per-server expose_prompts toggle and a rug-pull "N prompts need approval" banner in ServerDetail.vue (the latter needs REST twins). The feature is complete via MCP + REST; the operator UI is polish. Claude-Session: https://claude.ai/code/session_01H7J8Yv5zr4tMQZaY3ot3Za Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 2378d37 commit 472314b

11 files changed

Lines changed: 337 additions & 39 deletions

‎docs/features/security-quarantine.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,24 @@ built-in `tpa-descriptions` scanner. Its findings appear in the scan report
152152
`threat_level`, `confidence`, and the contributing check `signals`. See
153153
[Tool Scanner](/features/tool-scanner) for the full rule reference.
154154

155+
## Prompt rug-pull baseline (spec 100)
156+
157+
When upstream **prompt aggregation** is enabled (`aggregate_upstream_prompts: true`, off by default), mcpproxy keeps a per-prompt approval baseline that mirrors the tool rug-pull machinery. A trusted server that ships a benign prompt and later mutates its **advertised metadata** (name, description, or argument descriptions) has that changed prompt **withheld from `prompts/list`** until it is approved — closing the gap where a subtler injection that passes the poison scanner could still slip in via a later edit.
158+
159+
- **Scope — metadata only.** The baseline hashes the prompt's advertised metadata, not its `prompts/get` message content (which is materialised fresh per call and has no list-time artifact to baseline). Content is defended separately by output sanitisation and size caps. This is an inherent limit, not a shortcut.
160+
- **Enforcement is by withholding.** A held prompt is simply not registered, so it is absent from `prompts/list` and `prompts/get` on it fails natively. There is no separate runtime gate.
161+
- **Trust parity.** A server with `trust_mode: auto` (or `auto_approve_tool_changes: true`) auto-approves its prompt changes; `manual` holds them. Disabling quarantine globally (`quarantine_enabled: false`) auto-approves.
162+
163+
Manage held prompts with the `quarantine_security` MCP tool:
164+
165+
```jsonc
166+
// see what is held (all servers, or one via "name")
167+
{ "operation": "inspect_prompts", "name": "github" }
168+
// approve one held prompt, or all for a server
169+
{ "operation": "approve_prompt", "name": "github", "prompt_name": "summarize_pr" }
170+
{ "operation": "approve_all_prompts", "name": "github" }
171+
```
172+
155173
## Managing Quarantine
156174

157175
### View Quarantined Servers

‎internal/server/mcp.go‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1152,6 +1152,9 @@ func (p *MCPProxyServer) buildManagementTools() []mcpserver.ServerTool {
11521152
mcp.Description("Per-server trust tier governing new-server admission AND tool-change approval (spec 086): 'auto' = approve without scanning; 'scan' = auto-approve only when the fast offline TPA scan is green, else hold for review; 'manual' = human reviews every change. Empty → manual (secure default). Used with add/update/patch."),
11531153
mcp.Enum("auto", "scan", "manual"),
11541154
),
1155+
mcp.WithBoolean("expose_prompts",
1156+
mcp.Description("Per-server prompt-aggregation override (F9): true = include this server's MCP prompts in mcpproxy's aggregated prompts/list; false = exclude them regardless of capability. Omit to leave unchanged (patch) / inherit the default (aggregate if advertised). Only meaningful when aggregate_upstream_prompts is enabled globally. Used with add/update/patch."),
1157+
),
11551158
)
11561159
tools = append(tools, mcpserver.ServerTool{Tool: upstreamServersTool, Handler: p.handleUpstreamServers})
11571160
}
@@ -1166,15 +1169,18 @@ func (p *MCPProxyServer) buildManagementTools() []mcpserver.ServerTool {
11661169
mcp.WithOpenWorldHintAnnotation(false),
11671170
mcp.WithString("operation",
11681171
mcp.Required(),
1169-
mcp.Description("Security operation: list_quarantined, inspect_quarantined, quarantine_server, inspect_tools, approve_tool, approve_all_tools, block_tool, block_all_tools, enable_tool, disable_tool. 'block_tool'/'block_all_tools' atomically approve AND disable a tool (acknowledge it but keep it hidden) — all-or-nothing so a tool is never left approved+enabled."),
1170-
mcp.Enum("list_quarantined", "inspect_quarantined", "quarantine_server", "inspect_tools", "approve_tool", "approve_all_tools", "block_tool", "block_all_tools", "enable_tool", "disable_tool"),
1172+
mcp.Description("Security operation: list_quarantined, inspect_quarantined, quarantine_server, inspect_tools, approve_tool, approve_all_tools, block_tool, block_all_tools, enable_tool, disable_tool, inspect_prompts, approve_prompt, approve_all_prompts. 'block_tool'/'block_all_tools' atomically approve AND disable a tool (acknowledge it but keep it hidden) — all-or-nothing so a tool is never left approved+enabled. The prompt operations (spec 100) manage aggregated upstream prompts held by the metadata rug-pull baseline: a prompt whose advertised metadata changed since approval is withheld from prompts/list until approved."),
1173+
mcp.Enum("list_quarantined", "inspect_quarantined", "quarantine_server", "inspect_tools", "approve_tool", "approve_all_tools", "block_tool", "block_all_tools", "enable_tool", "disable_tool", "inspect_prompts", "approve_prompt", "approve_all_prompts"),
11711174
),
11721175
mcp.WithString("name",
1173-
mcp.Description("Server name (required for inspect_quarantined, quarantine_server, inspect_tools, approve_tool, approve_all_tools, block_tool, block_all_tools)"),
1176+
mcp.Description("Server name (required for inspect_quarantined, quarantine_server, inspect_tools, approve_tool, approve_all_tools, block_tool, block_all_tools, approve_prompt, approve_all_prompts)"),
11741177
),
11751178
mcp.WithString("tool_name",
11761179
mcp.Description("Tool name (required for approve_tool and block_tool operations)"),
11771180
),
1181+
mcp.WithString("prompt_name",
1182+
mcp.Description("Prompt name (required for approve_prompt; spec 100)"),
1183+
),
11781184
)
11791185
tools = append(tools, mcpserver.ServerTool{Tool: quarantineSecurityTool, Handler: p.handleQuarantineSecurity})
11801186
}
@@ -3410,6 +3416,12 @@ func (p *MCPProxyServer) handleQuarantineSecurity(ctx context.Context, request m
34103416
result, opErr = p.handleSetToolEnabledByName(request, true)
34113417
case "disable_tool":
34123418
result, opErr = p.handleSetToolEnabledByName(request, false)
3419+
case "inspect_prompts":
3420+
result, opErr = p.handleInspectPromptApprovals(request)
3421+
case "approve_prompt":
3422+
result, opErr = p.handleApprovePromptByName(request)
3423+
case "approve_all_prompts":
3424+
result, opErr = p.handleApproveAllPromptsByServer(request)
34133425
default:
34143426
p.emitActivityInternalToolCall("quarantine_security", "", "", "", sessionID, requestID, "error", fmt.Sprintf("Unknown quarantine operation: %s", operation), time.Since(startTime).Milliseconds(), args, nil, nil, "")
34153427
return mcp.NewToolResultError(fmt.Sprintf("Unknown quarantine operation: %s", operation)), nil
@@ -4603,6 +4615,16 @@ func (p *MCPProxyServer) handleAddUpstream(ctx context.Context, request mcp.Call
46034615
TrustMode: trustMode, // spec 086: carry the per-server trust_mode through on create
46044616
}
46054617

4618+
// F9: optional per-server expose_prompts override on add. GetBool can't tell
4619+
// absent from false, so probe the raw args for presence.
4620+
if rawArgs := request.GetArguments(); rawArgs != nil {
4621+
if raw, ok := rawArgs["expose_prompts"]; ok {
4622+
if b, ok := raw.(bool); ok {
4623+
serverConfig.ExposePrompts = &b
4624+
}
4625+
}
4626+
}
4627+
46064628
// Save to storage
46074629
if err := p.storage.SaveUpstreamServer(serverConfig); err != nil {
46084630
return mcp.NewToolResultError(fmt.Sprintf("Failed to add upstream: %v", err)), nil
@@ -5119,6 +5141,19 @@ func (p *MCPProxyServer) buildPatchConfigFromRequest(request mcp.CallToolRequest
51195141
patch.InitTimeout = &v
51205142
}
51215143

5144+
// F9: per-server expose_prompts override. GetBool collapses absent→false, so
5145+
// detect key presence in the raw args and only set the pointer when the caller
5146+
// actually provided it (nil = leave unchanged for MergeServerConfig).
5147+
if args := request.GetArguments(); args != nil {
5148+
if raw, ok := args["expose_prompts"]; ok {
5149+
b, ok := raw.(bool)
5150+
if !ok {
5151+
return nil, opts, fmt.Errorf("invalid expose_prompts: must be a boolean, got %T", raw)
5152+
}
5153+
patch.ExposePrompts = &b
5154+
}
5155+
}
5156+
51225157
// Handle oauth JSON string - deep merge for nested config
51235158
if oauthJSON := request.GetString("oauth_json", ""); oauthJSON != "" {
51245159
// Check for explicit null removal

‎internal/server/prompt_quarantine.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,3 +338,80 @@ func (p *MCPProxyServer) ApproveAllPrompts(serverName, approvedBy string) (int,
338338
}
339339
return approved, nil
340340
}
341+
342+
// --- MCP quarantine_security prompt operations (spec 100 FR-7) ---
343+
344+
// handleInspectPromptApprovals returns the prompt approval records for a server
345+
// (all servers when 'name' is omitted), with pending/changed counts so an agent
346+
// can see which prompts the rug-pull baseline is withholding.
347+
func (p *MCPProxyServer) handleInspectPromptApprovals(request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
348+
if p.storage == nil {
349+
return mcp.NewToolResultError("storage unavailable"), nil
350+
}
351+
serverName := request.GetString("name", "")
352+
recs, err := p.storage.ListPromptApprovals(serverName)
353+
if err != nil {
354+
return mcp.NewToolResultError(fmt.Sprintf("Failed to list prompt approvals: %v", err)), nil
355+
}
356+
type promptView struct {
357+
Server string `json:"server"`
358+
Prompt string `json:"prompt"`
359+
Status string `json:"status"`
360+
ChangedFrom string `json:"changed_from,omitempty"`
361+
}
362+
var pending, changed int
363+
views := make([]promptView, 0, len(recs))
364+
for _, r := range recs {
365+
switch r.Status {
366+
case promptStatusPending:
367+
pending++
368+
case promptStatusChanged:
369+
changed++
370+
}
371+
v := promptView{Server: r.ServerName, Prompt: r.PromptName, Status: r.Status}
372+
if r.Status == promptStatusChanged && r.PreviousDescription != "" {
373+
v.ChangedFrom = r.PreviousDescription
374+
}
375+
views = append(views, v)
376+
}
377+
payload := map[string]interface{}{
378+
"prompts": views,
379+
"pending_count": pending,
380+
"changed_count": changed,
381+
"action_required": pending + changed,
382+
}
383+
b, err := json.MarshalIndent(payload, "", " ")
384+
if err != nil {
385+
return mcp.NewToolResultError(fmt.Sprintf("Failed to encode: %v", err)), nil
386+
}
387+
return mcp.NewToolResultText(string(b)), nil
388+
}
389+
390+
// handleApprovePromptByName approves one held prompt (re-baselines it).
391+
func (p *MCPProxyServer) handleApprovePromptByName(request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
392+
serverName := request.GetString("name", "")
393+
if serverName == "" {
394+
return mcp.NewToolResultError("Missing required parameter 'name' (server name)"), nil
395+
}
396+
promptName := request.GetString("prompt_name", "")
397+
if promptName == "" {
398+
return mcp.NewToolResultError("Missing required parameter 'prompt_name'"), nil
399+
}
400+
if err := p.ApprovePrompt(serverName, promptName, "mcp"); err != nil {
401+
return mcp.NewToolResultError(fmt.Sprintf("Failed to approve prompt '%s': %v", promptName, err)), nil
402+
}
403+
return mcp.NewToolResultText(fmt.Sprintf("Prompt '%s' on server '%s' has been approved.", promptName, serverName)), nil
404+
}
405+
406+
// handleApproveAllPromptsByServer approves every held prompt for a server.
407+
func (p *MCPProxyServer) handleApproveAllPromptsByServer(request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
408+
serverName := request.GetString("name", "")
409+
if serverName == "" {
410+
return mcp.NewToolResultError("Missing required parameter 'name' (server name)"), nil
411+
}
412+
n, err := p.ApproveAllPrompts(serverName, "mcp")
413+
if err != nil {
414+
return mcp.NewToolResultError(fmt.Sprintf("Failed to approve prompts for '%s': %v", serverName, err)), nil
415+
}
416+
return mcp.NewToolResultText(fmt.Sprintf("Approved %d prompt(s) on server '%s'.", n, serverName)), nil
417+
}

‎internal/server/prompt_quarantine_test.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,3 +162,47 @@ func TestApproveAllPrompts(t *testing.T) {
162162

163163
// Guard: the storage record type is what we expect (compile-time contract).
164164
var _ = storage.PromptApprovalRecord{}
165+
166+
// --- MCP quarantine_security prompt op handlers ---
167+
168+
func TestHandlePromptApprovalOps(t *testing.T) {
169+
proxy := manualTrustProxy(t)
170+
proxy.checkPromptApprovals([]mcp.Prompt{{Name: "srv:a", Description: "1"}, {Name: "srv:b", Description: "2"}})
171+
172+
// inspect_prompts reports the two held prompts.
173+
insp, err := proxy.handleInspectPromptApprovals(mcp.CallToolRequest{Params: mcp.CallToolParams{
174+
Arguments: map[string]interface{}{"name": "srv"},
175+
}})
176+
require.NoError(t, err)
177+
require.False(t, insp.IsError)
178+
body := insp.Content[0].(mcp.TextContent).Text
179+
assert.Contains(t, body, "\"pending_count\": 2")
180+
assert.Contains(t, body, "\"action_required\": 2")
181+
182+
// approve_prompt approves one.
183+
ap, err := proxy.handleApprovePromptByName(mcp.CallToolRequest{Params: mcp.CallToolParams{
184+
Arguments: map[string]interface{}{"name": "srv", "prompt_name": "a"},
185+
}})
186+
require.NoError(t, err)
187+
assert.False(t, ap.IsError)
188+
rec, _ := proxy.storage.GetPromptApproval("srv", "a")
189+
assert.Equal(t, promptStatusApproved, rec.Status)
190+
191+
// approve_all_prompts approves the rest.
192+
aa, err := proxy.handleApproveAllPromptsByServer(mcp.CallToolRequest{Params: mcp.CallToolParams{
193+
Arguments: map[string]interface{}{"name": "srv"},
194+
}})
195+
require.NoError(t, err)
196+
assert.Contains(t, aa.Content[0].(mcp.TextContent).Text, "Approved 1 prompt")
197+
recB, _ := proxy.storage.GetPromptApproval("srv", "b")
198+
assert.Equal(t, promptStatusApproved, recB.Status)
199+
}
200+
201+
func TestHandleApprovePromptByName_MissingArgs(t *testing.T) {
202+
proxy := manualTrustProxy(t)
203+
res, err := proxy.handleApprovePromptByName(mcp.CallToolRequest{Params: mcp.CallToolParams{
204+
Arguments: map[string]interface{}{"name": "srv"},
205+
}})
206+
require.NoError(t, err)
207+
assert.True(t, res.IsError, "missing prompt_name is an error result")
208+
}

0 commit comments

Comments
 (0)