fix(quarantine): server admission stays human under the default trust tier

CHANGELOG.md

@@ -28,9 +28,11 @@ Releases follow [Semantic Versioning](https://semver.org/).
   changes behind an unchanged definition. **Migration:** set `trust_mode: "manual"` on any server whose
   tool changes you want a person to review; nothing changes for servers with an explicit `trust_mode` or
   `auto_approve_tool_changes`. A record whose stored annotation snapshot differs from the live hints in
-  dispatch tier only (a server last seen before hints were captured) is held once for review. New servers
-  added with no trust mode are quarantined, scanned and released automatically on a clean verdict, as
-  `scan` always did.
+  dispatch tier only (a server last seen before hints were captured) is held once for review. Server
+  admission stays with a person: a new server added with no trust mode is quarantined and stays
+  quarantined until someone approves it. Only an explicit `trust_mode: "scan"` lets a clean admission
+  scan release a server, and the approval re-checks that under the commit lock. The Web UI add form sends
+  a trust mode only when one is picked.
 
 - **connect / disconnect:** `mcpproxy disconnect`, `DELETE /api/v1/connect/{client}`, and the Web UI
   and tray Disconnect now also revoke the client's credential (`client-<id>`), so a secret copied out

cmd/mcpproxy/packaged_install_test.go

@@ -172,7 +172,14 @@ func TestLoadersOffLinuxKeepCreatingHomeConfig(t *testing.T) {
 	home := t.TempDir()
 	t.Setenv("HOME", home)
 	t.Setenv("USERPROFILE", home) // os.UserHomeDir on Windows
-	t.Chdir(t.TempDir())
+	// Not t.TempDir: on Windows CI the former cwd can stay locked for a moment
+	// after t.Chdir restores it, and TempDir's RemoveAll then fails the test.
+	cwd, err := os.MkdirTemp("", "mcpproxy-cwd-")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.RemoveAll(cwd) }) // best effort; runs after the chdir is undone
+	t.Chdir(cwd)
 
 	if _, err := loadUpstreamConfig(); err != nil {
 		t.Fatalf("load: %v", err)

cmd/mcpproxy/upstream_cmd.go

@@ -379,7 +379,7 @@ func init() {
 	upstreamAddCmd.Flags().StringVar(&upstreamAddTransport, "transport", "", "Transport type: http or stdio (auto-detected if not specified)")
 	upstreamAddCmd.Flags().BoolVar(&upstreamAddIfNotExists, "if-not-exists", false, "Don't error if server already exists")
 	upstreamAddCmd.Flags().BoolVar(&upstreamAddNoQuarantine, "no-quarantine", false, "Don't quarantine the new server (use with caution)")
-	upstreamAddCmd.Flags().StringVar(&upstreamAddTrustMode, "trust-mode", "", "Per-server trust tier governing admission AND tool-change approval: auto (approve without scanning), scan (new tools and changed tools are auto-approved only when the offline TPA scan is clean; findings are held for review), manual (human reviews every change). Unset inherits the default (scan)")
+	upstreamAddCmd.Flags().StringVar(&upstreamAddTrustMode, "trust-mode", "", "Per-server trust tier governing admission AND tool-change approval: auto (approve without scanning), scan (new tools and changed tools are auto-approved only when the offline TPA scan is clean; findings are held for review), manual (human reviews every change). Unset inherits the default (scan for tool changes; the server stays quarantined until approved — only an explicit scan releases it on a clean scan)")
 	upstreamAddCmd.Flags().StringArrayVar(&upstreamAddSecretEnvs, "secret-env", nil, "Environment variable to store in the OS keyring instead of the config, in KEY=value format (repeatable, FR-065)")
 	upstreamAddCmd.Flags().StringArrayVar(&upstreamAddSecretHeaders, "secret-header", nil, "HTTP header to store in the OS keyring instead of the config, in 'Name: value' format (repeatable, FR-065)")
 

docs/configuration.md

@@ -453,7 +453,7 @@ the error names the field.
 | `oauth` | object | No | OAuth configuration (see [OAuth Configuration](#oauth-configuration)) |
 | `isolation` | object | No | Per-server Docker isolation settings (see [Docker Isolation](#docker-isolation)) |
 | `enabled` | boolean | No | Enable/disable server (default: `true`) |
-| `quarantined` | boolean | No | Security quarantine status. The default depends on how the server arrives. A server added through the UI, CLI, REST API or an AI agent follows its [trust mode](features/security-quarantine.md#trust-modes-auto--scan--manual) at add time (quarantined under the default `scan` mode, then released automatically on a clean scan). A server you add by editing this file, with no `quarantined` key and no prior `config.db` record, is **held for review** on first load when quarantine is enabled and its trust mode is not `auto` (no `trust_mode` means `scan`, unless a legacy `auto_approve_tool_changes` resolves it — `true` to `auto`, `false` to `manual` — or `skip_quarantine: true` resolves it to `auto`). An explicit `"quarantined": false` admits it, and an explicit `true` holds it. A server that is already recorded in `config.db` keeps its recorded state. See [Servers added by hand-editing `mcp_config.json`](features/security-quarantine.md#servers-added-by-hand-editing-mcp_configjson) for the full admission rules. |
+| `quarantined` | boolean | No | Security quarantine status. The default depends on how the server arrives. A server added through the UI, CLI, REST API or an AI agent follows its [trust mode](features/security-quarantine.md#trust-modes-auto--scan--manual) at add time (quarantined under the default tier until you approve it; an explicit `trust_mode: scan` releases it automatically on a clean scan). A server you add by editing this file, with no `quarantined` key and no prior `config.db` record, is **held for review** on first load when quarantine is enabled and its trust mode is not `auto` (no `trust_mode` means `scan`, unless a legacy `auto_approve_tool_changes` resolves it — `true` to `auto`, `false` to `manual` — or `skip_quarantine: true` resolves it to `auto`). An explicit `"quarantined": false` admits it, and an explicit `true` holds it. A server that is already recorded in `config.db` keeps its recorded state. See [Servers added by hand-editing `mcp_config.json`](features/security-quarantine.md#servers-added-by-hand-editing-mcp_configjson) for the full admission rules. |
 | `reconnect_on_use` | boolean | No | When `true`, tool calls to a disconnected server trigger an immediate reconnect attempt (15s timeout) before failing (default: `false`) |
 | `expose_prompts` | boolean | No | Per-server override for whether this server's MCP prompts are aggregated into mcpproxy's `prompts/list`. Only takes effect when the global `aggregate_upstream_prompts` master switch is on. Omit to expose prompts whenever the server advertises `Capabilities.Prompts`; `false` opts this server out even if it does. |
 | `toon_output` | string | No | Per-server override for the global [`toon_output`](#toon-output-adaptive-result-encoding): `off`, `adaptive`, or `always`. Non-empty value wins over the global for this server's tools; omit to inherit. See [TOON Output](features/toon-output.md). |

docs/features/security-quarantine.md

@@ -593,7 +593,7 @@ new-server admission and tool-change approval (superseding the binary
 | Mode | Add time | Tool changes |
 |------|----------|--------------|
 | `auto` | Admitted without quarantine or scanning | Trusted without scanning (rug-pull risk) |
-| `scan` (default) | Quarantined, then a fail-closed automatic TPA scan admits it on a clean verdict | New tools and **changed** tool contracts are auto-approved when the offline scan verdict is clean with full coverage (spec 119) **and** the structural gate finds nothing (spec 120): any safety-hint change in either direction, a new required parameter, a type change, an enum member added or an enum dropped, a loosened or removed constraint, a default or const added or changed, a new parameter that carries a default, const or enum, an output-schema change, or a dispatch-tier change with an equal hash is held whatever the verdict ([details](#structural-gate)). A finding or an uncovered tool is held for review with the evidence attached. A paraphrased rug pull that scans clean and changes nothing structural is admitted: that is the accepted residual risk of the default tier — use `manual` on servers where it is not acceptable |
+| `scan` (default) | Quarantined. Set **explicitly**, a fail-closed automatic TPA scan admits it on a clean verdict. Left unset (the default tier), the server stays quarantined until a person approves it: server release stays human unless you opt in | New tools and **changed** tool contracts are auto-approved when the offline scan verdict is clean with full coverage (spec 119) **and** the structural gate finds nothing (spec 120): any safety-hint change in either direction, a new required parameter, a type change, an enum member added or an enum dropped, a loosened or removed constraint, a default or const added or changed, a new parameter that carries a default, const or enum, an output-schema change, or a dispatch-tier change with an equal hash is held whatever the verdict ([details](#structural-gate)). A finding or an uncovered tool is held for review with the evidence attached. A paraphrased rug pull that scans clean and changes nothing structural is admitted: that is the accepted residual risk of the default tier — use `manual` on servers where it is not acceptable |
 | `manual` | Quarantined for human review | Every change held for review, including input-schema-only changes, an added output schema, an added description and annotation-hint flips (see [What quarantine does and does not cover](#what-quarantine-does-and-does-not-cover)); a clean scan verdict is recorded as evidence and approves nothing |
 
 Config field: per-server `trust_mode`; REST: `trust_mode` on

docs/intro.md

@@ -32,7 +32,7 @@ Instead of loading all tools from all servers into your AI client's context, MCP
 
 ### Security Quarantine
 
-New MCP servers are quarantined on add and their tools are withheld from agents in the meantime. Under the default `scan` trust mode a server is released automatically when its offline scan is clean; under `manual` it waits for your approval. This reduces exposure to Tool Poisoning Attacks, where a malicious server tries to manipulate AI behavior through crafted tool descriptions. A clean scan is not proof of safety: `scan` and `auto` admit servers or changes without a human review, so use `manual` on servers where that is not acceptable.
+New MCP servers are quarantined on add and their tools are withheld from agents in the meantime. By default a new server waits for your approval; set `trust_mode: scan` on a server to release it automatically when its offline scan is clean. This reduces exposure to Tool Poisoning Attacks, where a malicious server tries to manipulate AI behavior through crafted tool descriptions. A clean scan is not proof of safety: an explicit `scan` and `auto` admit servers, and the default tier admits clean tool changes, without a human review, so use `manual` on servers where that is not acceptable.
 
 ### Docker Isolation
 

frontend/src/components/ManualServerForm.vue

@@ -115,7 +115,7 @@ import TrustModeSelector from '@/components/TrustModeSelector.vue'
 import { resolveSecretFields, rollbackSecrets } from '@/composables/useSecretFields'
 import { useServersStore } from '@/stores/servers'
 import { serverDetailPath } from '@/utils/serverRoute'
-import { DEFAULT_TRUST_MODE, type TrustMode } from '@/utils/trustMode'
+import type { TrustMode } from '@/utils/trustMode'
 
 const props = withDefaults(defineProps<{
   navigateAfterAdd?: boolean
@@ -140,7 +140,10 @@ const errorEl = ref<HTMLElement | null>(null)
 
 const name = ref('')
 const protocol = ref<'stdio' | 'http'>('stdio')
-const trustMode = ref<TrustMode>(DEFAULT_TRUST_MODE)
+// Unset until the operator picks a mode: an unset trust_mode is the default
+// tier, which never releases the server from quarantine on a clean scan; an
+// explicit Scan does.
+const trustMode = ref<TrustMode | undefined>(undefined)
 const trustModeConfirmationPending = ref(false)
 const url = ref('')
 const command = ref('')
@@ -191,8 +194,8 @@ async function handleSubmit() {
       name: name.value,
       protocol: protocol.value,
       enabled: true,
-      trust_mode: trustMode.value,
     }
+    if (trustMode.value) serverData.trust_mode = trustMode.value
     if (protocol.value === 'http') {
       serverData.url = url.value
       if (Object.keys(resolved.headers).length > 0) serverData.headers_json = JSON.stringify(resolved.headers)

frontend/src/components/TrustModeSelector.vue

@@ -17,7 +17,7 @@
           class="radio radio-primary radio-sm mt-0.5 shrink-0"
           :name="groupName"
           :value="m.mode"
-          :checked="displayedMode === m.mode"
+          :checked="checkedMode === m.mode"
           @change="onSelect(m.mode)"
         />
         <span class="flex-1 min-w-0">
@@ -118,6 +118,15 @@ const pendingWarnMode = ref<TrustMode | null>(null)
 /** Tentative selection while a warning is open; otherwise the effective mode. */
 const displayedMode = computed<TrustMode>(() => pendingWarnMode.value ?? state.value.effective)
 
+/**
+ * The checked radio. An unset value (the default tier) checks none: it is not
+ * the same as an explicit Scan, which also releases a quarantined server on a
+ * clean scan, so the operator must be able to pick Scan to opt in.
+ */
+const checkedMode = computed<TrustMode | null>(() =>
+  pendingWarnMode.value ?? (state.value.isDefault ? null : state.value.effective)
+)
+
 // A value arriving from the parent (save, re-load, SSE refresh) supersedes any
 // selection still awaiting acknowledgement.
 watch(
@@ -129,7 +138,7 @@ watch(
 )
 
 function onSelect(mode: TrustMode) {
-  if (mode === state.value.effective) {
+  if (mode === state.value.effective && !state.value.isDefault) {
     // Already in effect — nothing to warn about and nothing to save.
     pendingWarnMode.value = null
     emit('confirmation-pending', false)

frontend/src/utils/trustMode.ts

@@ -6,8 +6,9 @@
  *    contracts auto-approved only on a clean offline scan verdict (otherwise
  *    held with the findings), manual = every change held;
  *  - new-server admission: auto = admitted without quarantine or scan,
- *    scan = quarantined on add with a fail-closed automatic scan (the
- *    default), manual = quarantined on add for human review.
+ *    scan = quarantined on add and released by a clean automatic scan, but
+ *    only when set EXPLICITLY; an unset mode (the default tier) is quarantined
+ *    until a person approves it, manual = quarantined on add for human review.
  *
  * The raw value delivered by `GET /api/v1/servers` may be absent (unset), a
  * value migrated from the legacy `auto_approve_tool_changes` / `skip_quarantine`
@@ -94,7 +95,7 @@ export const TRUST_MODES: readonly TrustModeMeta[] = [
     description:
       'New tools and changes to approved tools are approved automatically when the offline security scan returns a clean verdict. Anything the scan flags — or cannot cover — is held for your review with the findings attached.',
     admissionNote:
-      'At add time the server is quarantined and scanned automatically; a scan that is not clean — or cannot complete — keeps it quarantined (fail closed). The default.',
+      'At add time the server is quarantined and scanned automatically. Chosen explicitly, a clean scan releases it; a scan that is not clean — or cannot complete — keeps it quarantined (fail closed). Left unset (the default), the server stays quarantined until you approve it.',
     warnsOnSelect: false,
   },
   {

frontend/tests/unit/add-server-manual.spec.ts

@@ -91,7 +91,6 @@ describe('ManualServerForm', () => {
       name: 'fs-server',
       protocol: 'stdio',
       enabled: true,
-      trust_mode: 'scan',
       command: 'npx',
       args_json: JSON.stringify(['-y', '@scope/server', '/tmp']),
     })
@@ -115,7 +114,6 @@ describe('ManualServerForm', () => {
       name: 'remote',
       protocol: 'http',
       enabled: true,
-      trust_mode: 'scan',
       url: 'https://api.example.com/mcp',
     })
     expect(wrapper.emitted('added')).toEqual([['remote']])
@@ -146,6 +144,23 @@ describe('ManualServerForm', () => {
     expect(JSON.parse(payload.env_json as string)).toEqual({ API_TOKEN: 'fake-secret-value' })
   })
 
+  it('sends trust_mode scan only when the operator picks it', async () => {
+    vi.mocked(api.callTool).mockResolvedValue({ success: true, data: {} })
+    vi.mocked(api.getServers).mockResolvedValue({ success: true, data: { servers: [] } })
+    const wrapper = await mountManual()
+
+    await wrapper.find('[data-test="manual-name-input"]').setValue('scan-server')
+    await wrapper.find('[data-test="manual-command-input"]').setValue('npx')
+    await wrapper.find('[data-test="trust-mode-option-scan"] input').setValue(true)
+    await wrapper.find('[data-test="manual-server-form"]').trigger('submit')
+    await flushPromises()
+
+    expect(api.callTool).toHaveBeenCalledWith('upstream_servers', expect.objectContaining({
+      name: 'scan-server',
+      trust_mode: 'scan',
+    }))
+  })
+
   it('requires confirmation before adding with auto trust mode and submits the chosen mode', async () => {
     vi.mocked(api.callTool).mockResolvedValue({ success: true, data: {} })
     vi.mocked(api.getServers).mockResolvedValue({ success: true, data: { servers: [] } })

frontend/tests/unit/server-detail-auto-approve.spec.ts

@@ -104,9 +104,9 @@ describe('ServerDetail — trust-mode control (Spec 088 US1, T009)', () => {
     expect(wrapper.findAll('[data-test^="trust-mode-option-"]')).toHaveLength(3)
   })
 
-  it('presents scan as the current default when no trust mode is configured (spec 119)', async () => {
+  it('marks scan as the current default, with no mode checked, when no trust mode is configured (spec 119)', async () => {
     const { wrapper } = await mountDetail('config')
-    expect(isChecked(wrapper, 'scan')).toBe(true)
+    expect(isChecked(wrapper, 'scan')).toBe(false)
     expect(wrapper.find('[data-test="trust-mode-default-note"]').exists()).toBe(true)
   })
 

frontend/tests/unit/trust-mode-selector.spec.ts

@@ -70,9 +70,9 @@ describe('TrustModeSelector — rendering (FR-002)', () => {
 })
 
 describe('TrustModeSelector — current selection from the raw value (FR-001)', () => {
-  it('unset (field omitted) selects scan and indicates it is the default (spec 119)', () => {
+  it('unset (field omitted) checks no mode and marks scan as the current default (spec 119)', () => {
     const wrapper = mountSelector(undefined)
-    expect(isChecked(wrapper, 'scan')).toBe(true)
+    expect(isChecked(wrapper, 'scan')).toBe(false)
     expect(isChecked(wrapper, 'auto')).toBe(false)
     expect(isChecked(wrapper, 'manual')).toBe(false)
     const note = wrapper.find('[data-test="trust-mode-default-note"]')
@@ -83,7 +83,7 @@ describe('TrustModeSelector — current selection from the raw value (FR-001)',
 
   it('empty string is also treated as unset (default scan)', () => {
     const wrapper = mountSelector('')
-    expect(isChecked(wrapper, 'scan')).toBe(true)
+    expect(isChecked(wrapper, 'scan')).toBe(false)
     expect(wrapper.find('[data-test="trust-mode-default-note"]').exists()).toBe(true)
     expect(wrapper.find('[data-test="trust-mode-invalid-note"]').exists()).toBe(false)
   })
@@ -155,6 +155,12 @@ describe('TrustModeSelector — selecting a safe mode emits immediately', () =>
     expect(wrapper.find('[data-test="trust-mode-auto-confirm"]').exists()).toBe(false)
   })
 
+  it('picking scan while unset emits scan (an explicit opt-in to clean-scan admission)', async () => {
+    const wrapper = mountSelector(undefined)
+    await select(wrapper, 'scan')
+    expect(wrapper.emitted('update:modelValue')![0]).toEqual(['scan'])
+  })
+
   it('re-selecting the mode already in effect does not emit', async () => {
     const wrapper = mountSelector('scan')
     await select(wrapper, 'scan')