Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions cmd/mcpproxy/security_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,15 @@ func GetSecurityCommand() *cobra.Command {
Long: `Commands for managing security scanners, scanning MCP servers,
and reviewing scan results.

Security scanners run as Docker containers and analyze upstream MCP servers
for vulnerabilities, tool poisoning attacks, and other security issues.
Scanning works out of the box: the offline baseline scanner is built into
mcpproxy, runs in-process on every scan, and needs no Docker and no setup. It
analyzes tool descriptions and schemas for tool poisoning attacks (TPAs),
prompt injection, and data exfiltration.

Deep scanners are the optional extra layer. They run as Docker containers for
source and dependency analysis (CVEs, secrets), so they need Docker plus
"deep scan" enabled — and when they are unavailable they are skipped, never
blocking the baseline verdict.

Examples:
mcpproxy security scanners
Expand Down
22 changes: 22 additions & 0 deletions docs/features/security-quarantine.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,28 @@ Manage held prompts with the `quarantine_security` MCP tool:

## Managing Quarantine

### Scan a Server for TPAs (MCP)

The `quarantine_security` tool can also run and read the TPA scan, so an agent
reviewing a held server does not have to leave for the CLI or web UI:

```jsonc
// run the offline baseline scan (in-process, no Docker required)
{ "operation": "scan_server", "name": "github" }
// read the latest verdict + findings
{ "operation": "get_scan_report", "name": "github" }
```

`scan_server` answers with the verdict when the scan settles quickly, otherwise
with the job id and `"status": "scan started"` — poll `get_scan_report` for the
result. Every `list_quarantined`, `inspect_quarantined` and `inspect_tools`
response also carries a one-line `scan_status`, so a server nobody ever scanned
reads as `never scanned — run scan_server first` instead of looking clean.

The optional Docker-based deep scanners are a separate layer: they run only when
[deep scan](/features/security-scanner-plugins) is enabled, and when they are
unavailable they are skipped without changing the baseline verdict.

### View Quarantined Servers

**Web UI:**
Expand Down
25 changes: 15 additions & 10 deletions frontend/src/views/Security.vue
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@
<p class="text-base-content/70 mt-1">Configure security scanner plugins and review scan results</p>
</div>
<div class="flex gap-2">
<div
v-if="(overview?.scanners_enabled ?? overview?.scanners_installed ?? 0) > 0"
class="tooltip"
:data-tip="!overview?.docker_available ? 'Docker is required to run security scanners' : ''"
>
<button @click="startScanAll" :disabled="loading || scanAllRunning || !overview?.docker_available" class="btn btn-primary">
<!-- The offline baseline scanner is built in and always runs, so this
action is never gated on Docker or on an installed deep scanner
(mirrors the per-server Scan Now button, spec 088 FR-016). -->
<div class="tooltip" :data-tip="scanAllTooltip(overview?.docker_available)">
<button
@click="startScanAll"
:disabled="loading || scanAllRunning"
class="btn btn-primary"
data-test="scan-all-button"
>
<span v-if="scanAllRunning" class="loading loading-spinner loading-sm"></span>
{{ scanAllRunning ? 'Scanning...' : 'Scan All Servers' }}
</button>
Expand Down Expand Up @@ -151,12 +155,13 @@
</div>
</div>

<!-- Docker unavailable warning (only after overview has loaded) -->
<div v-if="overviewLoaded && overview.docker_available === false" class="alert alert-warning">
<!-- Docker unavailable warning (only after overview has loaded). Scanning
still works: only the optional deep scanners need Docker. -->
<div v-if="overviewLoaded && overview.docker_available === false" class="alert alert-warning" data-test="docker-unavailable-alert">
<svg class="w-5 h-5 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-2.5L13.732 4c-.77-.833-1.964-.833-2.732 0L4.082 16.5c-.77.833.192 2.5 1.732 2.5z" />
</svg>
<span>Docker is not running. Security scanners require Docker to analyze MCP servers.</span>
<span>Docker is not running, so the optional deep scanners are skipped. The built-in offline baseline scan still runs.</span>
</div>

<!-- Docker isolation nudge: show only when Docker is available, global
Expand Down Expand Up @@ -513,7 +518,7 @@ import { refreshSecurityScannerStatus } from '@/composables/useSecurityScannerSt
import { useSystemStore } from '@/stores/system'
import { scanReportPath } from '@/utils/serverRoute'
import { formatSignatureBundle } from '@/utils/signatureBundle'
import { deepScanSummary, enabledDockerScanners, scannerWontRun } from './security/deepScanState'
import { deepScanSummary, enabledDockerScanners, scanAllTooltip, scannerWontRun } from './security/deepScanState'

const systemStore = useSystemStore()

Expand Down
17 changes: 17 additions & 0 deletions frontend/src/views/security/deepScanState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,23 @@ export function enabledDockerScanners(scanners: ScannerLike[]): number {
return scanners.filter(s => isDockerScanner(s) && isScannerEnabled(String(s.status ?? ''))).length
}

/**
* Tooltip for the page-level "Scan All Servers" action.
*
* Docker is NOT a precondition for scanning: the offline baseline scanner is
* built into mcpproxy and runs in-process for every server. Only the optional
* deep scanners need Docker. The old copy ("Docker is required to run security
* scanners") came with a hard `disabled` on the button, which left a default
* install — no Docker, no installed scanner — with no way to scan at all, the
* same bug already fixed for the per-server Scan Now button (spec 088 FR-016).
*/
export function scanAllTooltip(dockerAvailable: boolean | null | undefined): string {
if (dockerAvailable === false) {
return 'Optional deep scanners need Docker; the offline baseline scan is built in'
}
return 'Runs the built-in offline baseline scan on every server'
}

/**
* The master card's one-line status. Names what a scan will do right now,
* from the operator's side of the screen.
Expand Down
109 changes: 109 additions & 0 deletions frontend/tests/unit/security-scan-all-ungate.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { describe, it, expect, beforeEach, vi } from 'vitest'
import { mount, flushPromises } from '@vue/test-utils'
import { createPinia, setActivePinia } from 'pinia'
import { createRouter, createWebHistory } from 'vue-router'
import { scanAllTooltip } from '@/views/security/deepScanState'

// "Scan All Servers" was unreachable on exactly the installs that had never
// scanned anything: it only rendered when the overview reported an installed
// scanner (the always-on in-process baseline was not counted, so a fresh
// install reported zero), and it was disabled without Docker — the same false
// gate already fixed for the per-server Scan Now button (spec 088 FR-016).
//
// Every fixture here is that default install: zero scanners, no Docker.

let overviewPayload: Record<string, unknown>

vi.mock('@/services/api', () => {
const ok = (data: unknown = {}) => Promise.resolve({ success: true, data })
return {
default: {
getSecurityOverview: vi.fn(() => ok(overviewPayload)),
listScanners: vi.fn(() => ok([])),
listScanHistory: vi.fn(() => ok({ scans: [], total: 0 })),
getQueueProgress: vi.fn(() => ok({ status: 'idle' })),
getConfig: vi.fn(() => ok({ docker_isolation: { enabled: false }, security: { deep_scan: { enabled: false } } })),
getServers: vi.fn(() => ok({ servers: [] })),
scanAll: vi.fn(() => ok({ status: 'running', total: 1, completed: 0, running: 1 })),
installScanner: vi.fn(() => ok({})),
removeScanner: vi.fn(() => ok({})),
configureScanner: vi.fn(() => ok({})),
cancelAllScans: vi.fn(() => ok({})),
patchConfig: vi.fn(() => ok({})),
updateConfig: vi.fn(() => ok({})),
},
}
})

async function mountSecurity() {
const api = (await import('@/services/api')).default
const Security = (await import('@/views/Security.vue')).default
const router = createRouter({
history: createWebHistory(),
routes: [
{ path: '/', component: { template: '<div/>' } },
{ path: '/settings', component: { template: '<div/>' } },
{ path: '/security/scans/:jobId', component: { template: '<div/>' } },
],
})
await router.push('/')
await router.isReady()
const wrapper = mount(Security, { global: { plugins: [createPinia(), router] } })
await flushPromises()
await flushPromises()
return { wrapper, api }
}

beforeEach(() => {
setActivePinia(createPinia())
vi.clearAllMocks()
// A fresh install as the backend used to report it: nothing persisted.
overviewPayload = { scanners_enabled: 0, scanners_installed: 0, docker_available: false }
})

describe('Security page — Scan All is reachable on a default install', () => {
it('renders the Scan All button with zero reported scanners', async () => {
const { wrapper } = await mountSecurity()
expect(wrapper.find('[data-test="scan-all-button"]').exists()).toBe(true)
wrapper.unmount()
})

it('leaves it enabled when Docker is unavailable', async () => {
const { wrapper } = await mountSecurity()
expect(wrapper.find('[data-test="scan-all-button"]').attributes('disabled')).toBeUndefined()
wrapper.unmount()
})

it('starts a batch scan from the button on a default install', async () => {
const { wrapper, api } = await mountSecurity()
await wrapper.find('[data-test="scan-all-button"]').trigger('click')
await flushPromises()
expect(api.scanAll).toHaveBeenCalled()
wrapper.unmount()
})

it('explains Docker as a deep-scan-only requirement, not a scanning blocker', async () => {
const { wrapper } = await mountSecurity()
const tip = wrapper.find('[data-test="scan-all-button"]').element.parentElement?.getAttribute('data-tip') ?? ''
expect(tip).toContain('deep scanners need Docker')
expect(tip).toContain('baseline')
expect(tip).not.toContain('Docker is required')

const alert = wrapper.find('[data-test="docker-unavailable-alert"]')
expect(alert.exists()).toBe(true)
expect(alert.text()).toContain('baseline scan still runs')
wrapper.unmount()
})
})

describe('scanAllTooltip', () => {
it('names deep scanners — not scanning itself — as what Docker gates', () => {
expect(scanAllTooltip(false)).toBe('Optional deep scanners need Docker; the offline baseline scan is built in')
})

it('describes the baseline scan when Docker is present or unknown', () => {
expect(scanAllTooltip(true)).toContain('offline baseline scan')
expect(scanAllTooltip(undefined)).toContain('offline baseline scan')
expect(scanAllTooltip(null)).toContain('offline baseline scan')
})
})
113 changes: 113 additions & 0 deletions internal/security/scanner/overview_baseline_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package scanner

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
)

// newFreshInstallService builds a service over the REAL bundled registry and an
// empty storage — i.e. exactly what a user gets on first launch, before any
// Docker scanner has ever been installed.
func newFreshInstallService(t *testing.T) *Service {
t.Helper()
logger := zap.NewNop()
dir := t.TempDir()
return NewService(newMockStorage(), NewRegistry(dir, logger), NewDockerRunner(logger), dir, logger)
}

// TestGetOverview_CountsAlwaysOnBaselineOnFreshInstall pins the fix for the
// invisible "Scan All Servers" button: the overview counted only scanners
// persisted in BBolt, so a fresh install reported scanners_enabled=0 even
// though the in-process tpa-descriptions baseline is always installed and runs
// on every scan. The web UI hides its scan-trigger button on 0.
func TestGetOverview_CountsAlwaysOnBaselineOnFreshInstall(t *testing.T) {
svc := newFreshInstallService(t)

overview, err := svc.GetOverview(context.Background())
require.NoError(t, err)

assert.GreaterOrEqual(t, overview.ScannersEnabled, 1,
"the always-on in-process baseline must count as enabled on a fresh install")
assert.GreaterOrEqual(t, overview.ScannersInstalled, overview.ScannersEnabled,
"installed is a superset of enabled")

// The count is the in-process baseline, not the Docker scanners: those load
// as "available" and must stay uncounted until their image is pulled.
inProcessEnabled := 0
for _, sc := range svc.registry.List() {
if sc.InProcess && (sc.Status == ScannerStatusInstalled || sc.Status == ScannerStatusConfigured) {
inProcessEnabled++
}
}
assert.Equal(t, inProcessEnabled, overview.ScannersEnabled,
"only the in-process baseline is enabled before any Docker scanner is installed")
}

// TestGetOverview_DoesNotDoubleCountPersistedBaseline guards the other half:
// once the baseline IS persisted (older builds saved it, and the healing path
// in syncRegistryFromStorage rewrites it), the registry pass must not count it
// a second time.
func TestGetOverview_DoesNotDoubleCountPersistedBaseline(t *testing.T) {
logger := zap.NewNop()
dir := t.TempDir()
store := newMockStorage()
registry := NewRegistry(dir, logger)

baseline, err := registry.Get(inProcessTPAScannerID)
require.NoError(t, err)
persisted := *baseline
persisted.Status = ScannerStatusInstalled
require.NoError(t, store.SaveScanner(&persisted))

svc := NewService(store, registry, NewDockerRunner(logger), dir, logger)
overview, err := svc.GetOverview(context.Background())
require.NoError(t, err)

assert.Equal(t, 1, overview.ScannersEnabled, "the persisted baseline is counted exactly once")
assert.Equal(t, 1, overview.ScannersInstalled, "the persisted baseline is counted exactly once")
}

// TestGetOverview_ConcurrentScannerStatusUpdateIsRaceFree guards the read seam
// the baseline count added. Registry.List() hands out the live *ScannerPlugin
// records the registry keeps, and UpdateStatus mutates Status on exactly those
// records under the registry lock — so counting by reading reg.Status outside
// the lock is a data race with any concurrent install/pull. GetOverview must
// resolve the predicate inside the registry instead. Run with -race.
func TestGetOverview_ConcurrentScannerStatusUpdateIsRaceFree(t *testing.T) {
svc := newFreshInstallService(t)

done := make(chan struct{})
go func() {
defer close(done)
for i := 0; i < 200; i++ {
status := ScannerStatusInstalled
if i%2 == 0 {
status = ScannerStatusAvailable
}
_ = svc.registry.UpdateStatus(inProcessTPAScannerID, status)
}
}()

for i := 0; i < 200; i++ {
_, err := svc.GetOverview(context.Background())
require.NoError(t, err)
}
<-done
}

// TestGetOverview_ToleratesNilRegistry pins the nil-registry contract: other
// Service methods already guard for it, and the overview must not be the one
// call that panics on a service built without a registry.
func TestGetOverview_ToleratesNilRegistry(t *testing.T) {
logger := zap.NewNop()
dir := t.TempDir()
svc := NewService(newMockStorage(), nil, NewDockerRunner(logger), dir, logger)

overview, err := svc.GetOverview(context.Background())
require.NoError(t, err)
assert.Equal(t, 0, overview.ScannersEnabled)
}
28 changes: 28 additions & 0 deletions internal/security/scanner/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,34 @@ func (r *Registry) List() []*ScannerPlugin {
return result
}

// InProcessRunnableIDs returns the IDs of in-process scanners whose status
// means the engine will actually run them (installed or configured), sorted.
//
// It exists so a caller can count the always-on in-process baseline without
// reading ScannerPlugin fields outside the registry lock: List() hands out the
// live pointers the registry keeps, and UpdateStatus mutates Status on exactly
// those records, so an unsynchronized read of reg.Status races with any
// concurrent install/pull. Resolving the predicate here keeps the read under
// the same lock as the write.
func (r *Registry) InProcessRunnableIDs() []string {
if r == nil {
return nil
}
r.mu.RLock()
defer r.mu.RUnlock()
ids := make([]string, 0, len(r.scanners))
for id, s := range r.scanners {
if s == nil || !s.InProcess {
continue
}
if s.Status == ScannerStatusInstalled || s.Status == ScannerStatusConfigured {
ids = append(ids, id)
}
}
sort.Strings(ids)
return ids
}

// Get returns a scanner by ID
func (r *Registry) Get(id string) (*ScannerPlugin, error) {
r.mu.RLock()
Expand Down
Loading
Loading