From 0ad85be50b1e6a6c27c90c213d1563052e97335f Mon Sep 17 00:00:00 2001 From: Roman Chernyak Date: Sat, 23 May 2026 15:26:30 +0200 Subject: [PATCH] fix(webui): surface tool-level quarantine on the Servers list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Servers grid only renders the big orange "Server is quarantined" banner + Approve button when `server.quarantined === true`. Servers that are trusted at the server level but ship tools blocked by Spec 032 tool-level quarantine (pending / changed) had only a tiny `stat-desc` under the tool count — easy to miss, and shadowed entirely by the `X disabled` line when any tool was also disabled, because both lines shared a `v-if / v-else-if` chain. The asymmetry is confusing: the detail page DOES escalate tool-level quarantine to a yellow alert panel with an Approve-All button, so users who hit the "tool requires approval" error from the daemon find clear remediation on Details but nothing on the list view. They report the list as broken or inconsistent. This commit: * Adds a second `alert-warning` banner (`v-else-if="quarantineToolCount > 0"`) with a Review link that routes to `/servers/?tab=tools`. Server and tool quarantine remain mutually exclusive on the card — the server-level banner already implies the tools won't run, so we don't stack two warnings. * Splits the stat-desc `v-if / v-else-if` chain so `X disabled` and `N pending approval` can render side-by-side. Token-size hint stays the "nothing wrong" fallback. * Computes a context-aware summary string: - "All N tools pending security approval" when every tool is pending - "N of M tools pending security approval" when partial - "N tools changed since approval — re-review needed" for rug-pull - mixed pending+changed when both apply No backend change: the `quarantine.pending_count` / `changed_count` / `blocked_count` block is already populated by `enrichServersWithQuarantineStats` (REST + SSE). Extends the existing ServerCard test with five new cases covering the partial / full / changed / mixed-with-server-level / mixed-with-disabled scenarios. --- frontend/src/components/ServerCard.vue | 55 +++++++++- .../components/__tests__/ServerCard.test.ts | 102 ++++++++++++++++++ 2 files changed, 154 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/ServerCard.vue b/frontend/src/components/ServerCard.vue index 7ebeed7d..69ddc2e8 100644 --- a/frontend/src/components/ServerCard.vue +++ b/frontend/src/components/ServerCard.vue @@ -35,13 +35,13 @@ {{ blockedToolCount }} disabled -
+
{{ quarantineToolCount }} pending approval
-
+
{{ server.tool_list_token_size.toLocaleString() }} tokens
@@ -106,7 +106,9 @@ {{ server.last_error }}
- +
@@ -114,6 +116,25 @@ Server is quarantined
+ +
+ + + + {{ toolQuarantineSummary }} + + Review + +
+
@@ -408,6 +429,34 @@ const blockedToolCount = computed(() => { return q.blocked_count ?? 0 }) +// Human-readable summary for the tool-quarantine banner. Differentiates +// fully-quarantined (every tool needs approval) from partially-quarantined, +// and surfaces "changed" tools separately because they indicate a rug-pull +// rather than a first-time review. +const toolQuarantineSummary = computed(() => { + const q = props.server.quarantine + if (!q) return '' + const pending = q.pending_count ?? 0 + const changed = q.changed_count ?? 0 + const total = pending + changed + if (total === 0) return '' + const toolCount = props.server.tool_count ?? 0 + const noun = (n: number) => (n === 1 ? 'tool' : 'tools') + if (changed > 0 && pending > 0) { + return `${pending} ${noun(pending)} pending, ${changed} changed — approval needed` + } + if (changed > 0) { + return `${changed} ${noun(changed)} changed since approval — re-review needed` + } + if (toolCount > 0 && pending === toolCount) { + return `All ${pending} ${noun(pending)} pending security approval` + } + if (toolCount > 0) { + return `${pending} of ${toolCount} ${noun(toolCount)} pending security approval` + } + return `${pending} ${noun(pending)} pending security approval` +}) + // Security scan badge (Spec 039) const securityScanStatus = computed(() => { return props.server.security_scan?.status || 'not_scanned' diff --git a/frontend/src/components/__tests__/ServerCard.test.ts b/frontend/src/components/__tests__/ServerCard.test.ts index 4092dc56..770ff950 100644 --- a/frontend/src/components/__tests__/ServerCard.test.ts +++ b/frontend/src/components/__tests__/ServerCard.test.ts @@ -61,4 +61,106 @@ describe('ServerCard', () => { expect(wrapper.text()).toContain('disabled-server') expect(wrapper.text()).toContain('Disabled') }) + + it('shows tool-quarantine banner with Review link when tools are pending and server is not quarantined', () => { + const server = { + name: 'partial-server', + protocol: 'stdio' as const, + enabled: true, + connected: true, + quarantined: false, + tool_count: 10, + quarantine: { pending_count: 3, changed_count: 0, blocked_count: 0 } + } + + const wrapper = mount(ServerCard, { + props: { server }, + global: { plugins: [pinia, router] } + }) + + expect(wrapper.text()).toContain('3 of 10 tools pending security approval') + const review = wrapper.find('a.btn-warning') + expect(review.exists()).toBe(true) + expect(review.attributes('href')).toBe('/servers/partial-server?tab=tools') + // The server-level "Server is quarantined" banner must NOT render here + expect(wrapper.text()).not.toContain('Server is quarantined') + }) + + it('says "All N pending" when every tool is pending', () => { + const server = { + name: 'fully-pending', + protocol: 'stdio' as const, + enabled: true, + connected: true, + quarantined: false, + tool_count: 4, + quarantine: { pending_count: 4, changed_count: 0, blocked_count: 0 } + } + + const wrapper = mount(ServerCard, { + props: { server }, + global: { plugins: [pinia, router] } + }) + + expect(wrapper.text()).toContain('All 4 tools pending security approval') + }) + + it('flags rug-pull-style changed tools separately', () => { + const server = { + name: 'rugpull', + protocol: 'stdio' as const, + enabled: true, + connected: true, + quarantined: false, + tool_count: 5, + quarantine: { pending_count: 0, changed_count: 2, blocked_count: 0 } + } + + const wrapper = mount(ServerCard, { + props: { server }, + global: { plugins: [pinia, router] } + }) + + expect(wrapper.text()).toContain('2 tools changed since approval') + }) + + it('does not double up: server-level banner wins over tool-level banner', () => { + const server = { + name: 'srv-quarantined', + protocol: 'stdio' as const, + enabled: true, + connected: false, + quarantined: true, + tool_count: 4, + quarantine: { pending_count: 4, changed_count: 0, blocked_count: 0 } + } + + const wrapper = mount(ServerCard, { + props: { server }, + global: { plugins: [pinia, router] } + }) + + expect(wrapper.text()).toContain('Server is quarantined') + expect(wrapper.text()).not.toContain('pending security approval') + }) + + it('shows both disabled and pending counts when both apply', () => { + const server = { + name: 'mixed', + protocol: 'stdio' as const, + enabled: true, + connected: true, + quarantined: false, + tool_count: 10, + quarantine: { pending_count: 2, changed_count: 0, blocked_count: 3 } + } + + const wrapper = mount(ServerCard, { + props: { server }, + global: { plugins: [pinia, router] } + }) + + expect(wrapper.text()).toContain('3 disabled') + expect(wrapper.text()).toContain('2 pending approval') + }) }) \ No newline at end of file