feat(quarantine): rescan automatically after a tool definition change (Spec 118)

CLAUDE.md

@@ -146,6 +146,7 @@ tail -f ~/Library/Logs/mcpproxy/main.log  # main log (macOS; Linux: ~/.mcpproxy/
 - **Windows installer**: [docs/github-actions-windows-wix-research.md](docs/github-actions-windows-wix-research.md). **Prerelease** (`next` branch + `v*-rc.*` tags, opt-in, off stable channels): [docs/prerelease-builds.md](docs/prerelease-builds.md).
 
 ## Recent Changes
+- 118-auto-rescan-on-change: Go 1.26, small Web UI change — after a tool definition changes (or a tool is added after the baseline) in `manual`/`scan` trust mode the in-process TPA scan of its current definition is recorded as hold evidence (`held_reason: awaiting_review` + `held_verdict: clean` for a clean verdict; findings/coverage as before) and a debounced informational server rescan (`scan_informational.go`, fed by `Runtime.AddToolDefinitionsChangedObserver`) brings the review scan line back to "current". Switch: `security.auto_rescan_on_change` (default on, env `MCPPROXY_AUTO_RESCAN_ON_CHANGE`) AND `auto_baseline_scan`. RT-02 untouched: a clean verdict never approves a change. Docs: `docs/features/security-quarantine.md` "Automatic informational baseline scan". **No new dependencies.**
 - quarantine-approval-pilot: Go 1.26, tests + CI only — independent policy model `internal/qapilot` (no production imports), deterministic scenario inventory S01–S26 + 896-cell dispatch table + REST authz table + counting-upstream dispatch check, check ledger (`QAPILOT_REPORT_DIR`) gated by `go run ./cmd/qapilot report`, versioned mutation set `tests/qapilot/mutations.json` run by `qapilot mutate` in disposable worktrees; workflow `.github/workflows/quarantine-pilot.yml` (3 PR checks + scheduled rapid exploration). Docs: `docs/development/quarantine-approval-pilot.md`. **No new dependencies.**
 - 117-review-bound-approvals PR-3 (#1565): Go 1.26, backend-only — every upstream `tools/list` carries a `managed.InventoryCapture{Seq, Epoch, Client}` (process-global sequence stamped by the flight leader; `ListToolsCaptured`/`ListToolsFreshCaptured`; `Client.Retire`/`CaptureIsCurrent`; the manager retires replaced/removed clients; `Manager.DiscoverToolsReportCaptured` gives the sweep one capture per server). The runtime admits each inventory under the server's approval lock (`admitInventoryHeld`, per-server `applied`/`definitions` in `toolApprovalServerLock`): an older, retired or reconnected capture writes nothing (records, index, last-good snapshot + its capture, StateView); a lenient empty result is an observation fence; the approval reindex/sweep fallback re-apply the snapshot only while its capture is current, else re-list/skip. Docs: `docs/features/security-quarantine.md` "Tool lists are applied in the order they were captured". **No new dependencies.**
 - 116-config-commit-consistency PR2: Go 1.26 + a small Web UI change — removal purges the whole name-keyed purge set in the commit's bbolt transaction (`internal/storage/purge.go`, exact OAuth key match), in-memory purge, incarnation-bound approval writers (`internal/runtime/approval_incarnation.go`, REST `expected_incarnation` → 409 `server_replaced`), incarnation-conditional scan/identity/history writes. **No new dependencies.**

cmd/mcpproxy/tools_cmd.go

@@ -601,6 +601,10 @@ func formatToolHold(t map[string]interface{}) string {
 		// A hold with no signals means the scan itself could not be trusted
 		// (degraded coverage / missing bundle) — still worth naming.
 		if reason := getStringField(t, "held_reason"); reason != "" {
+			// A clean verdict held only for review (Spec 118) is not a scan hold.
+			if reason == "awaiting_review" {
+				return "clean"
+			}
 			return reason
 		}
 		return "-"

cmd/mcpproxy/tools_cmd_test.go

@@ -452,6 +452,8 @@ func TestFormatToolHold_BackCompat(t *testing.T) {
 	assert.Equal(t, "-", formatToolHold(map[string]interface{}{"name": "create_issue", "approval_status": "changed"}))
 	assert.Equal(t, "-", formatToolHold(map[string]interface{}{"held_signals": []interface{}{}}))
 	assert.Equal(t, "scan_coverage", formatToolHold(map[string]interface{}{"held_reason": "scan_coverage"}))
+	// Spec 118: a clean verdict held only for review shows the verdict.
+	assert.Equal(t, "clean", formatToolHold(map[string]interface{}{"held_reason": "awaiting_review", "held_verdict": "clean"}))
 }
 
 // TestOutputGlobalTools_HeldColumn verifies the global table renders the HELD

docs/configuration.md

@@ -932,6 +932,7 @@ block** — off by default, best-effort, and unable to change the baseline verdi
     "scanner_registry_url": "",
     "tpa_bundle_path": "",
     "auto_baseline_scan": true,
+    "auto_rescan_on_change": true,
     "deep_scan": {
       "enabled": false,
       "fetch_package_source": true,
@@ -946,6 +947,7 @@ block** — off by default, best-effort, and unable to change the baseline verdi
 |-------|------|---------|-------------|
 | `tpa_bundle_path` | string | `""` (embedded) | Filesystem path to the tpa-db `scanner-bundle.json` the offline TPA scanner runs. Empty uses the corpus embedded in the build. Env override: `MCPPROXY_TPA_BUNDLE_PATH`, which wins over this field on every path (loader, hot-reload, `/api/v1/config/apply`). Re-read on config hot-reload and honoured in every transport, stdio included. A bundle that fails to read/parse/version-check/compile — or that contributes zero runnable rules — is refused and the previously active corpus stays live; the reason is surfaced as `signature_bundle.load_error` in `GET /api/v1/security/overview` and in `mcpproxy security overview`. |
 | `auto_baseline_scan` | boolean | `true` | Kill switch for the **automatic informational baseline scan**. When on (the default), every newly added server gets one free in-process Pass-1 TPA scan, and once per installation a background sweep scans pre-existing enabled servers that have never been scanned (marker persisted in BBolt, so it runs exactly once and never delays startup). The result only populates the security badge / scan summary: it **never** quarantines, approves, or otherwise gates a server. Disabled servers are skipped, and so are `trust_mode: "scan"` servers — that mode's own admission gate scans them and auto-approves on a clean verdict, so the informational path stays out of it entirely rather than risk feeding that gate; this flag does not affect that separate path. Set to `false` to suppress all automatic scans; manual scans keep working. Hot-reloadable — the flag is read live at each decision point. Env override: `MCPPROXY_AUTO_BASELINE_SCAN` (`true`/`1`/`false`/`0`), which wins over this field on every path. |
+| `auto_rescan_on_change` | boolean | `true` | Finer switch for the **automatic rescan after a tool definition changes** (Spec 118), in every trust mode except `auto`. When on, a changed tool, a tool added after the baseline, or a changed definition that moves again gets the in-process TPA scan of its current definition recorded as hold evidence (`held_reason`/`held_verdict`/`held_signals`, so the review screen shows a per-tool verdict without a click), and a debounced (about 5 s, coalesced per server) informational rescan of the server brings the review screen's scan line back to "current". Evidence and freshness only: a clean verdict **never** approves a changed or new tool, and no approval rule changes. It only takes effect while `auto_baseline_scan` is also on, so set this to `false` to keep the admission scan and turn off per-change rescans. The `trust_mode: "scan"` gate's own admission-time scan of new tools is not affected. Hot-reloadable. Env override: `MCPPROXY_AUTO_RESCAN_ON_CHANGE` (`true`/`1`/`false`/`0`), which wins over this field on every path. |
 | `deep_scan.enabled` | boolean | `false` | Master opt-in for the heavy layer. When `false`, no Docker scanner runs and no source extraction is attempted — only the in-process baseline scanner executes. |
 | `deep_scan.fetch_package_source` | boolean | `true` (when deep scan is on) | Whether the scanner fetches (never executes) the published source of `npx`/`uvx` package-runner servers when no local source is available. Set `false` for air-gapped deployments. |
 | `deep_scan.disable_no_new_privileges` | boolean | `false` | Omits `--security-opt no-new-privileges` from scanner container runs (snap-docker/AppArmor escape hatch). |

docs/features/security-quarantine.md

@@ -609,10 +609,32 @@ so every server ends up with a security verdict instead of an empty badge:
   serialized, never delays startup, is cancelled on shutdown, and a persisted
   marker keeps it one-shot.
 
+- **After a tool definition changes** — when a tool's definition changes, or a
+  tool is added to a server that already has an approved baseline, in every trust
+  mode except `auto`, two things run without a click:
+  - the in-process scan of that tool's *current* definition, recorded on the tool
+    (see [Hold evidence in the Web UI](#hold-evidence-in-the-web-ui-spec-088)).
+    In `scan` mode this scan is the gate's own; in `manual` mode it is run for
+    evidence only. It re-runs each time the held definition changes again.
+  - a debounced (about 5 seconds, coalesced per server) informational rescan of
+    the server, so the review screen's scan line returns to **current** instead
+    of **stale** ("Scan out of date: N tool definitions changed after the last
+    scan"). A change that lands while a scan of that server is queued or running
+    is retried, not dropped. Disabled servers, `trust_mode: "auto"` servers and
+    quarantined `trust_mode: "scan"` servers (the admission gate owns those) are
+    skipped. Failed starts share the per-server retry cap of the admission scan.
+
 These scans are **informational**: the verdict fills in the scan summary and the
-UI badge and never quarantines, approves, or blocks anything. Disable with
-`security.auto_baseline_scan: false` (env: `MCPPROXY_AUTO_BASELINE_SCAN`).
-The `trust_mode: "scan"` gate above is a separate path and is unaffected.
+UI badge and never quarantines, approves, or blocks anything. **A clean verdict
+never approves a changed or new tool**: a changed contract is held for review in
+`scan` and `manual` modes whatever the scan says (RT-02); the scan only adds
+evidence and freshness. Disable everything above with
+`security.auto_baseline_scan: false` (env: `MCPPROXY_AUTO_BASELINE_SCAN`), or
+keep the admission scan and turn off only the per-change rescans with
+`security.auto_rescan_on_change: false` (env: `MCPPROXY_AUTO_RESCAN_ON_CHANGE`).
+Manual scans keep working either way. The `trust_mode: "scan"` gate above is a
+separate path: its admission-time scan of new tools is not switched off by either
+setting.
 
 ### Signature bundle (offline TPA corpus)
 
@@ -663,6 +685,17 @@ The quarantine banner distinguishes four states: scan running, scan verdict
 blocked automatic approval, scan could not complete (retry offered), and
 awaiting manual review.
 
+`held_reason` has a third value, `awaiting_review`, recorded in `manual` and
+`scan` modes when the automatic scan of a changed or newly pending tool's current
+definition came back **clean** (`held_verdict: "clean"`, no signals). It is
+evidence, not a finding and not a precaution: the tool is still `changed` or
+`pending` and stays blocked until you approve it, because a clean verdict never
+approves a changed contract. The Web UI shows it as "Scan clean, awaiting your
+review", the review screen shows the tool's scan verdict as `clean` (the tool still starts
+unchecked there: the in-process verdict never widens what is pre-selected), and `mcpproxy tools list` shows
+`clean` in the HELD column. The server banner is derived from the server scan,
+not from this field, and is unchanged.
+
 ### Reviewing a held change {#reviewing-a-held-change}
 
 Every changed tool contract is held, in every trust mode except `auto`. To keep that cheap, the Web review screen and `mcpproxy review show <server>` show what differs from the approved definition: the description, parameters added, removed, retyped or newly required, output schema changes, and flips of the four safety hints (`readOnlyHint`, `destructiveHint`, `idempotentHint`, `openWorldHint`). **Approve update (N changed tools)** on the Web review screen approves exactly those tools in one click. The approval is bound to the definitions and the server incarnation on screen, so if a tool changes again, or the server is removed and re-added, between viewing and clicking, the request is refused with 409 and nothing is approved; reload and review again.

frontend/src/types/api.ts

@@ -370,7 +370,7 @@ export interface GlobalTool {
   // on tools the trust gate refused to auto-approve. GET /api/v1/tools emits
   // these alongside the approval status (internal/httpapi/server.go); records
   // predating Spec 086 omit them entirely, which must render unchanged.
-  held_reason?: string     // "scan_findings" (threat) | "scan_coverage" (precaution)
+  held_reason?: string     // "scan_findings" (threat) | "scan_coverage" (precaution) | "awaiting_review" (clean, held for review)
   held_verdict?: string    // "dangerous" | "warnings" | "clean"
   held_signals?: string[]  // matched deterministic check ids, producer order, ≤16
   // Spec 116 FR-017: the incarnation of the server this row was read for; a
@@ -497,7 +497,7 @@ export interface ToolApproval {
   // them on from GET /api/v1/servers/{id}/tools; the diff endpoint returns them
   // directly. Absent on records that are not currently held by the scan gate
   // and on every record written before spec 086, which must render unchanged.
-  //   held_reason:  'scan_findings' | 'scan_coverage'
+  //   held_reason:  'scan_findings' | 'scan_coverage' | 'awaiting_review'
   //   held_verdict: 'dangerous' | 'warnings' | 'clean'
   //   held_signals: matched check ids, e.g. 'tpa.TPA-2026-0001.hidden_instruction'
   held_reason?: string

frontend/src/utils/holdEvidence.ts

@@ -7,8 +7,12 @@
 //                  verdict) or "scan_coverage" (the scan itself could not be
 //                  trusted, so the gate failed closed). A coverage hold is a
 //                  PRECAUTION and must never be presented as a threat verdict.
+//                  "awaiting_review" (Spec 118) records a CLEAN scan verdict on a
+//                  tool that stays held for your review anyway: evidence, not a
+//                  finding and not a precaution. A clean verdict never approves
+//                  a changed or new tool.
 //   held_verdict — the baseline verdict at hold time (dangerous | warnings |
-//                  clean — "clean" accompanies a coverage hold).
+//                  clean — "clean" accompanies a coverage or awaiting-review hold).
 //   held_signals — matched deterministic check ids, e.g.
 //                  "tpa.TPA-2026-0001.hidden_instruction" or "phrase.injection".
 //
@@ -28,6 +32,7 @@ import type { ToolApproval } from '@/types'
 /** Backend hold-reason constants (internal/storage/models.go). */
 export const HOLD_REASON_SCAN_FINDINGS = 'scan_findings'
 export const HOLD_REASON_SCAN_COVERAGE = 'scan_coverage'
+export const HOLD_REASON_AWAITING_REVIEW = 'awaiting_review'
 
 /** The raw payload fields carrying hold evidence, on any surface that has them. */
 export interface HoldEvidenceFields {
@@ -61,6 +66,8 @@ export interface HoldEvidence {
   isThreat: boolean
   /** The scan could not complete: style as a precaution, never a threat. */
   isPrecaution: boolean
+  /** The scan came back clean; the tool is held only because it awaits review. */
+  isAwaitingReview: boolean
 }
 
 /**
@@ -141,6 +148,7 @@ export function parseHoldEvidence(
     heuristics,
     isThreat: reason === HOLD_REASON_SCAN_FINDINGS,
     isPrecaution: reason === HOLD_REASON_SCAN_COVERAGE,
+    isAwaitingReview: reason === HOLD_REASON_AWAITING_REVIEW,
   }
 }
 
@@ -221,6 +229,15 @@ export function reasonPresentation(
     }
   }
 
+  if (evidence.isAwaitingReview) {
+    return {
+      label: 'Scan clean, awaiting your review',
+      tone: 'neutral',
+      description:
+        'The offline security scan found nothing in this tool definition. It is still held for your review: a clean scan never approves a changed or new tool.',
+    }
+  }
+
   // Unknown reason from a newer backend: name it, claim nothing.
   return {
     label: `Held for review (${evidence.reason})`,

frontend/tests/unit/hold-evidence.spec.ts

@@ -67,6 +67,19 @@ describe('parseHoldEvidence (spec 088 FR-008/FR-012)', () => {
     expect(evidence!.signals).toEqual([])
   })
 
+  it('marks an awaiting_review + clean hold as neither a threat nor a precaution (Spec 118)', () => {
+    const evidence = parseHoldEvidence({ held_reason: 'awaiting_review', held_verdict: 'clean' })
+    expect(evidence).not.toBeNull()
+    expect(evidence!.isThreat).toBe(false)
+    expect(evidence!.isPrecaution).toBe(false)
+    expect(evidence!.isAwaitingReview).toBe(true)
+    expect(evidence!.signals).toEqual([])
+    const reason = reasonPresentation(evidence)
+    expect(reason?.tone).toBe('neutral')
+    expect(reason?.label).toBe('Scan clean, awaiting your review')
+    expect(reason?.description).toMatch(/never approves/)
+  })
+
   it('does not fabricate a threat claim for an unrecognized reason', () => {
     const evidence = parseHoldEvidence({ held_reason: 'future_reason' })
     expect(evidence).not.toBeNull()

internal/config/auto_rescan_on_change_test.go

@@ -0,0 +1,76 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestIsAutoRescanOnChangeEnabled(t *testing.T) {
+	boolPtr := func(b bool) *bool { return &b }
+
+	t.Run("nil and unset default to enabled", func(t *testing.T) {
+		var sec *SecurityConfig
+		assert.True(t, sec.IsAutoRescanOnChangeEnabled())
+		assert.True(t, (&SecurityConfig{}).IsAutoRescanOnChangeEnabled())
+	})
+
+	t.Run("explicit values", func(t *testing.T) {
+		assert.False(t, (&SecurityConfig{AutoRescanOnChange: boolPtr(false)}).IsAutoRescanOnChangeEnabled())
+		assert.True(t, (&SecurityConfig{AutoRescanOnChange: boolPtr(true)}).IsAutoRescanOnChangeEnabled())
+	})
+
+	t.Run("env override outranks the file value", func(t *testing.T) {
+		t.Setenv(EnvAutoRescanOnChange, "false")
+		assert.False(t, (&SecurityConfig{AutoRescanOnChange: boolPtr(true)}).IsAutoRescanOnChangeEnabled())
+		t.Setenv(EnvAutoRescanOnChange, "0")
+		assert.False(t, (&SecurityConfig{}).IsAutoRescanOnChangeEnabled())
+		t.Setenv(EnvAutoRescanOnChange, "true")
+		assert.True(t, (&SecurityConfig{AutoRescanOnChange: boolPtr(false)}).IsAutoRescanOnChangeEnabled())
+		t.Setenv(EnvAutoRescanOnChange, "1")
+		assert.True(t, (&SecurityConfig{AutoRescanOnChange: boolPtr(false)}).IsAutoRescanOnChangeEnabled())
+	})
+
+	t.Run("unrecognized env value is ignored", func(t *testing.T) {
+		t.Setenv(EnvAutoRescanOnChange, "maybe")
+		assert.False(t, (&SecurityConfig{AutoRescanOnChange: boolPtr(false)}).IsAutoRescanOnChangeEnabled())
+		assert.True(t, (&SecurityConfig{}).IsAutoRescanOnChangeEnabled())
+	})
+}
+
+// IsAutoChangeScanEnabled is the combined gate: the coarse
+// auto_baseline_scan switch AND the finer auto_rescan_on_change switch.
+func TestIsAutoChangeScanEnabled(t *testing.T) {
+	boolPtr := func(b bool) *bool { return &b }
+	var nilSec *SecurityConfig
+	assert.True(t, nilSec.IsAutoChangeScanEnabled())
+	assert.True(t, (&SecurityConfig{}).IsAutoChangeScanEnabled())
+	assert.False(t, (&SecurityConfig{AutoBaselineScan: boolPtr(false)}).IsAutoChangeScanEnabled())
+	assert.False(t, (&SecurityConfig{AutoRescanOnChange: boolPtr(false)}).IsAutoChangeScanEnabled())
+	assert.True(t, (&SecurityConfig{AutoBaselineScan: boolPtr(true), AutoRescanOnChange: boolPtr(true)}).IsAutoChangeScanEnabled())
+
+	t.Setenv(EnvAutoBaselineScan, "false")
+	assert.False(t, (&SecurityConfig{AutoRescanOnChange: boolPtr(true)}).IsAutoChangeScanEnabled())
+}
+
+func TestApplyEnvOverrides_AutoRescanOnChange(t *testing.T) {
+	boolPtr := func(b bool) *bool { return &b }
+
+	t.Run("recognized value materializes the field", func(t *testing.T) {
+		t.Setenv(EnvAutoRescanOnChange, "false")
+		cfg := &Config{}
+		applyTLSEnvOverrides(cfg)
+		require.NotNil(t, cfg.Security)
+		require.NotNil(t, cfg.Security.AutoRescanOnChange)
+		assert.False(t, *cfg.Security.AutoRescanOnChange)
+	})
+
+	t.Run("unrecognized value does not touch the file value", func(t *testing.T) {
+		t.Setenv(EnvAutoRescanOnChange, "yes")
+		cfg := &Config{Security: &SecurityConfig{AutoRescanOnChange: boolPtr(true)}}
+		applyTLSEnvOverrides(cfg)
+		require.NotNil(t, cfg.Security.AutoRescanOnChange)
+		assert.True(t, *cfg.Security.AutoRescanOnChange)
+	})
+}