fix(frontend): key tool-quarantine banner off changed, suppress for baseline pending (#641)

The per-server Tool-Quarantine banner / "N tool(s) require approval" + per-tool
Approve UI in ServerDetail.vue surfaced for every non-approved tool, including
freshly-`pending` baseline tools. Under the confirmed trust model (parent
MCP-2081), approving a *server* promotes its baseline `pending` tools to
`approved`, so a baseline `pending` tool is not a reason to nag the operator.

- Extract selectQuarantinedTools() (frontend/src/utils/toolQuarantine.ts): the
  banner keys off `status=changed` (rug-pull) plus any residual `pending` left
  once a change has surfaced, NOT all non-approved tools.
- While the server-level Security Quarantine banner is showing
  (server.quarantined=true), suppress the Tool-Quarantine banner entirely — the
  operator approves the server first; the two banners never appear at once.
- Add data-test hooks (security-quarantine-banner, tool-quarantine-banner,
  tool-quarantine-list) for Playwright.
- Unit tests lock the selection logic; Playwright sweep (3 states) + HTML report
  under specs/041-quarantine-invariants/verification/mcp-2101/.
- Docs: update docs/features/tool-quarantine.md Web UI section.

Related MCP-2101, MCP-2081 (Spec 032)

Author: Dumbris

docs/features/tool-quarantine.md

@@ -205,7 +205,17 @@ curl -H "X-API-Key: your-key" \
 1. Open the MCPProxy dashboard
 2. Click on a server in the server list
 3. Navigate to the **Tools** tab in the server detail view
-4. Review pending/changed tools and click **Approve** or **Approve All**
+4. Review changed (and any residual pending) tools and click **Approve** or **Approve All**
+
+The server detail view's **Tool Quarantine** banner is shown only when a tool's
+status is `changed` (a rug-pull). Once a change has surfaced, any residual
+`pending` tools are listed alongside it so they can be cleared in the same pass.
+Freshly-`pending` baseline tools do **not** raise the banner on their own:
+approving the **server** (lifting the server-level Security Quarantine) promotes
+its baseline `pending` tools to `approved`. While the server-level **Security
+Quarantine** banner is showing, the Tool-Quarantine banner is suppressed
+entirely — the operator approves the server first, and the two banners never
+appear at once.
 
 The server list page shows a quarantine badge with the count of pending/changed tools for each server.
 

frontend/src/utils/toolQuarantine.ts

@@ -0,0 +1,29 @@
+import type { ToolApproval } from '@/types'
+
+/**
+ * Selects the tools that warrant the per-server Tool-Quarantine banner / list
+ * (Spec 032, parent MCP-2081, MCP-2101).
+ *
+ * Trust model (confirmed on MCP-2081): when an operator approves a *server*
+ * (lifting the server-level Security Quarantine), the backend promotes that
+ * server's baseline `pending` tools to `approved`. A freshly-`pending` baseline
+ * tool is therefore NOT a reason to nag the operator with a tool-level banner —
+ * only a `changed` tool (a rug-pull) is.
+ *
+ * Rules:
+ *  - While the server is quarantined, suppress the tool banner entirely. The
+ *    server-level Security Quarantine banner already covers it and the operator
+ *    must approve the server first — never show two banners at once.
+ *  - Otherwise the banner keys off `status === 'changed'`. Only once a change
+ *    has surfaced do we also include any residual `pending` tools so the
+ *    operator can clear them in the same approval pass.
+ */
+export function selectQuarantinedTools(
+  toolApprovals: ToolApproval[],
+  serverQuarantined: boolean,
+): ToolApproval[] {
+  if (serverQuarantined) return []
+  const hasChanged = toolApprovals.some((t) => t.status === 'changed')
+  if (!hasChanged) return []
+  return toolApprovals.filter((t) => t.status === 'changed' || t.status === 'pending')
+}

frontend/src/views/ServerDetail.vue

@@ -202,7 +202,7 @@
           </div>
         </div>
 
-        <div v-if="server.quarantined" class="alert alert-warning">
+        <div v-if="server.quarantined" data-test="security-quarantine-banner" class="alert alert-warning">
           <svg class="w-6 h-6" fill="none" stroke="currentColor" viewBox="0 0 24 24">
             <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-2.5L13.732 4c-.77-.833-1.732-.833-2.5 0L3.732 16.5c-.77.833.192 2.5 1.732 2.5z" />
           </svg>
@@ -331,7 +331,7 @@
 
           <div v-else class="space-y-4">
             <!-- Tool Quarantine Panel (Spec 032) -->
-            <div v-if="quarantinedTools.length > 0" class="alert alert-warning shadow-lg mb-4">
+            <div v-if="quarantinedTools.length > 0" data-test="tool-quarantine-banner" class="alert alert-warning shadow-lg mb-4">
               <svg class="w-6 h-6 shrink-0" fill="none" stroke="currentColor" viewBox="0 0 24 24">
                 <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z" />
               </svg>
@@ -352,7 +352,7 @@
             </div>
 
             <!-- Quarantined Tools List -->
-            <div v-if="quarantinedTools.length > 0" class="space-y-3 mb-6">
+            <div v-if="quarantinedTools.length > 0" data-test="tool-quarantine-list" class="space-y-3 mb-6">
               <div
                 v-for="tool in quarantinedTools"
                 :key="'q-' + tool.tool_name"
@@ -1224,6 +1224,7 @@ import type { Server, Tool, ToolApproval, SecurityScanReport } from '@/types'
 import api from '@/services/api'
 import { useSecurityScannerStatus } from '@/composables/useSecurityScannerStatus'
 import { serverDisplayName } from '@/utils/serverRoute'
+import { selectQuarantinedTools } from '@/utils/toolQuarantine'
 import { oauthSignInState } from '@/utils/health'
 import { computeToolDiffSections } from '@/utils/toolDiff'
 
@@ -1302,8 +1303,12 @@ const toolToggleLoading = ref<Record<string, boolean>>({})
 // they're mutually exclusive with each other and with any per-tool toggle.
 const bulkToolToggleLoading = ref(false)
 
+// MCP-2101 (Spec 032): the Tool-Quarantine banner / list keys off `changed`
+// (rug-pull) tools, NOT freshly-`pending` baseline tools, and is suppressed
+// entirely while the server-level Security Quarantine banner is showing.
+// See selectQuarantinedTools for the trust-model rationale.
 const quarantinedTools = computed(() => {
-  return toolApprovals.value.filter(t => t.status === 'pending' || t.status === 'changed')
+  return selectQuarantinedTools(toolApprovals.value, server.value?.quarantined ?? false)
 })
 
 const blockedToolCount = computed(() => {

frontend/tests/unit/tool-quarantine-banner.spec.ts

@@ -0,0 +1,76 @@
+import { describe, it, expect } from 'vitest'
+import { selectQuarantinedTools } from '@/utils/toolQuarantine'
+import type { ToolApproval } from '@/types'
+
+// MCP-2101 (Spec 032, parent MCP-2081): the per-server Tool-Quarantine banner
+// must stop nagging for freshly-`pending` baseline tools and must never show
+// alongside the server-level Security Quarantine banner.
+//
+// Trust model (confirmed on MCP-2081): approving a *server* promotes its
+// baseline `pending` tools to `approved` on the backend. So a baseline
+// `pending` tool is NOT a reason to surface a tool-level banner — only a
+// `changed` tool (a rug-pull) is.
+
+function approval(over: Partial<ToolApproval>): ToolApproval {
+  return {
+    server_name: 's',
+    tool_name: 't',
+    status: 'approved',
+    hash: 'h',
+    description: 'd',
+    ...over,
+  }
+}
+
+describe('selectQuarantinedTools (MCP-2101)', () => {
+  it('suppresses the tool banner entirely while the server is quarantined', () => {
+    // Even a rug-pull `changed` tool must not surface a SECOND banner while the
+    // server-level Security Quarantine banner is up — operator approves the
+    // server first, then the backend promotes baseline pending→approved.
+    const tools = [
+      approval({ tool_name: 'a', status: 'changed' }),
+      approval({ tool_name: 'b', status: 'pending' }),
+    ]
+    expect(selectQuarantinedTools(tools, true)).toEqual([])
+  })
+
+  it('does NOT surface freshly-pending baseline tools (the core fix)', () => {
+    // Not quarantined, no changed tool → baseline `pending` tools alone must
+    // not raise the banner. Pre-fix this returned the pending tools.
+    const tools = [
+      approval({ tool_name: 'a', status: 'pending' }),
+      approval({ tool_name: 'b', status: 'pending' }),
+      approval({ tool_name: 'c', status: 'approved' }),
+    ]
+    expect(selectQuarantinedTools(tools, false)).toEqual([])
+  })
+
+  it('surfaces a `changed` (rug-pull) tool when the server is not quarantined', () => {
+    const changed = approval({ tool_name: 'rugpull', status: 'changed' })
+    const tools = [changed, approval({ tool_name: 'fine', status: 'approved' })]
+    expect(selectQuarantinedTools(tools, false)).toEqual([changed])
+  })
+
+  it('once a change has surfaced, also includes residual pending tools', () => {
+    const changed = approval({ tool_name: 'rugpull', status: 'changed' })
+    const pending = approval({ tool_name: 'leftover', status: 'pending' })
+    const tools = [changed, pending, approval({ tool_name: 'fine', status: 'approved' })]
+    const result = selectQuarantinedTools(tools, false)
+    expect(result).toContain(changed)
+    expect(result).toContain(pending)
+    expect(result).not.toContainEqual(approval({ tool_name: 'fine', status: 'approved' }))
+  })
+
+  it('returns empty when every tool is approved', () => {
+    const tools = [
+      approval({ tool_name: 'a', status: 'approved' }),
+      approval({ tool_name: 'b', status: 'approved' }),
+    ]
+    expect(selectQuarantinedTools(tools, false)).toEqual([])
+  })
+
+  it('returns empty for an empty approval list', () => {
+    expect(selectQuarantinedTools([], false)).toEqual([])
+    expect(selectQuarantinedTools([], true)).toEqual([])
+  })
+})

specs/041-quarantine-invariants/verification/mcp-2101/01-stateA-security-only.png

@@ -0,0 +1,51 @@
+import { test, expect } from '@playwright/test';
+
+const BASE = 'http://127.0.0.1:18082';
+const KEY = 'uitest';
+const SERVER = 'rugpull';
+const STATE = process.env.STATE || 'B';
+const OUT = process.env.OUT || `/tmp/mcp-2101-uitest/pw/shot-${STATE}.png`;
+
+async function api(request: any, method: 'GET' | 'POST', path: string) {
+  const r = await request.fetch(`${BASE}${path}`, {
+    method,
+    headers: { 'X-API-Key': KEY, 'Content-Type': 'application/json' },
+    data: method === 'POST' ? '{}' : undefined,
+  });
+  return r;
+}
+
+test(`tool-quarantine banner — state ${STATE}`, async ({ page, request }) => {
+  // Drive the server-level quarantine flag for the state under test.
+  if (STATE === 'A') {
+    await api(request, 'POST', `/api/v1/servers/${SERVER}/quarantine`);
+  } else {
+    await api(request, 'POST', `/api/v1/servers/${SERVER}/unquarantine`);
+  }
+
+  await page.goto(`${BASE}/ui/servers/${SERVER}?apikey=${KEY}&tab=tools`);
+  await page.waitForLoadState('domcontentloaded');
+  // Give the view time to fetch server + tool-approval data and render.
+  await page.waitForTimeout(2500);
+
+  const securityBanner = page.locator('[data-test="security-quarantine-banner"]');
+  const toolBanner = page.locator('[data-test="tool-quarantine-banner"]');
+
+  if (STATE === 'A') {
+    // Quarantined: ONLY the server-level Security Quarantine banner. The
+    // tool-level banner must be suppressed even though tools are non-approved.
+    await expect(securityBanner).toBeVisible();
+    await expect(toolBanner).toHaveCount(0);
+  } else if (STATE === 'B') {
+    // Not quarantined, baseline pending tools, NO changed tool: neither banner.
+    await expect(securityBanner).toHaveCount(0);
+    await expect(toolBanner).toHaveCount(0);
+  } else if (STATE === 'C') {
+    // Not quarantined, a `changed` (rug-pull) tool exists: the tool-level
+    // banner appears; the server-level Security banner does not.
+    await expect(securityBanner).toHaveCount(0);
+    await expect(toolBanner).toBeVisible();
+  }
+
+  await page.screenshot({ path: OUT, fullPage: true });
+});

specs/041-quarantine-invariants/verification/mcp-2101/02-stateB-no-tool-banner.png

@@ -0,0 +1,120 @@
+#!/usr/bin/env python3
+"""Build a self-contained HTML verification report for MCP-2101.
+
+Base64-embeds each screenshot so the report opens offline with no external
+assets. Run from this directory: `python3 build-report.py`.
+"""
+import base64
+import os
+
+HERE = os.path.dirname(os.path.abspath(__file__))
+
+SCENARIOS = [
+    {
+        "id": "A",
+        "title": "Server quarantined → only the Security-Quarantine banner",
+        "img": "01-stateA-security-only.png",
+        "expected": "Server has quarantined=true with two non-approved (pending) tools. "
+                    "Exactly ONE banner shows — the server-level 'Security Quarantine' — "
+                    "and the tool-level 'Tool Quarantine' banner is suppressed entirely "
+                    "(never two banners at once).",
+        "observed": "Only the 'Security Quarantine' banner renders. No 'Tool Quarantine' "
+                    "banner despite both tools being pending. (data-test=tool-quarantine-banner "
+                    "asserted count 0; security-quarantine-banner visible.)",
+        "pass": True,
+    },
+    {
+        "id": "B",
+        "title": "Not quarantined, baseline pending tools → NO tool banner (core fix)",
+        "img": "02-stateB-no-tool-banner.png",
+        "expected": "Server is Active (not quarantined) with only freshly-pending baseline "
+                    "tools and no changed tool. Neither banner shows — a freshly-pending "
+                    "baseline tool must not nag the operator. (Pre-fix this showed "
+                    "'2 tool(s) require approval'.)",
+        "observed": "No banner of either kind. Tools list 'echo'/'get_time' with informational "
+                    "'new' badges only. (Both banner data-tests asserted count 0.)",
+        "pass": True,
+    },
+    {
+        "id": "C",
+        "title": "A changed (rug-pull) tool exists → tool banner appears",
+        "img": "03-stateC-tool-banner.png",
+        "expected": "Server is Active (not quarantined); two tools flipped to 'changed' "
+                    "(rug-pull) plus a residual 'pending' new tool. The 'Tool Quarantine' "
+                    "banner appears keyed off the changed status, listing changed + residual "
+                    "pending tools with per-tool Approve UI and before/after diffs.",
+        "observed": "'Tool Quarantine' banner shows '3 tool(s) require approval'. echo/get_time "
+                    "show 'changed' with before/after diffs; steal_data shows 'pending'. "
+                    "(tool-quarantine-banner visible; security-quarantine-banner count 0.)",
+        "pass": True,
+    },
+]
+
+
+def b64(path):
+    with open(os.path.join(HERE, path), "rb") as f:
+        return base64.b64encode(f.read()).decode("ascii")
+
+
+def main():
+    passed = sum(1 for s in SCENARIOS if s["pass"])
+    total = len(SCENARIOS)
+    cards = []
+    for s in SCENARIOS:
+        badge = "PASS" if s["pass"] else "FAIL"
+        color = "#16a34a" if s["pass"] else "#dc2626"
+        cards.append(f"""
+        <details open>
+          <summary><span class="badge" style="background:{color}">{badge}</span>
+            <strong>State {s['id']}.</strong> {s['title']}</summary>
+          <div class="body">
+            <p><span class="lbl">Expected</span> {s['expected']}</p>
+            <p><span class="lbl">Observed</span> {s['observed']}</p>
+            <img src="data:image/png;base64,{b64(s['img'])}" alt="State {s['id']}" />
+          </div>
+        </details>""")
+
+    html = f"""<!doctype html><html><head><meta charset="utf-8">
+<title>MCP-2101 — Tool-Quarantine banner verification</title>
+<style>
+  body {{ font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif;
+         margin: 0; background:#f8fafc; color:#0f172a; }}
+  .wrap {{ max-width: 1100px; margin: 0 auto; padding: 32px 20px 80px; }}
+  h1 {{ font-size: 22px; margin: 0 0 4px; }}
+  .sub {{ color:#64748b; margin: 0 0 24px; }}
+  .summary {{ background:#fff; border:1px solid #e2e8f0; border-radius:12px;
+             padding:20px 24px; margin-bottom:24px; box-shadow:0 1px 2px rgba(0,0,0,.04); }}
+  .count {{ font-size: 30px; font-weight: 700; color:#16a34a; }}
+  details {{ background:#fff; border:1px solid #e2e8f0; border-radius:12px;
+            margin-bottom:16px; overflow:hidden; box-shadow:0 1px 2px rgba(0,0,0,.04); }}
+  summary {{ cursor:pointer; padding:16px 20px; font-size:15px; list-style:none; }}
+  summary::-webkit-details-marker {{ display:none; }}
+  .badge {{ color:#fff; font-size:11px; font-weight:700; padding:2px 8px;
+           border-radius:999px; margin-right:10px; letter-spacing:.04em; }}
+  .body {{ padding: 4px 20px 20px; }}
+  .lbl {{ display:inline-block; min-width:78px; font-weight:600; color:#475569;
+         font-size:12px; text-transform:uppercase; letter-spacing:.04em; }}
+  img {{ width:100%; border:1px solid #e2e8f0; border-radius:8px; margin-top:12px; }}
+  p {{ line-height:1.5; }}
+  code {{ background:#f1f5f9; padding:1px 5px; border-radius:4px; font-size:13px; }}
+</style></head><body><div class="wrap">
+  <h1>MCP-2101 — Suppress tool-quarantine banner for baseline pending tools</h1>
+  <p class="sub">Spec 032 · parent MCP-2081 · <code>frontend/src/views/ServerDetail.vue</code></p>
+  <div class="summary">
+    <div class="count">{passed}/{total} scenarios pass</div>
+    <p>The Tool-Quarantine banner now keys off <code>status=changed</code> (plus residual
+    <code>pending</code> once a change has surfaced) and is suppressed entirely while the
+    server-level Security-Quarantine banner is showing. Captured live against a fresh
+    mcpproxy + the <code>echo-rugpull</code> test server via Playwright (pinned Chromium 1217).</p>
+  </div>
+  {''.join(cards)}
+</div></body></html>"""
+
+    out = os.path.join(HERE, "report.html")
+    with open(out, "w") as f:
+        f.write(html)
+    print(f"wrote {out} ({passed}/{total} pass)")
+
+
+if __name__ == "__main__":
+    main()