fix(security): keep implicit quarantine across server restarts and config writes (Spec fix-quarantine-restart)

[Dumbris]

Summary

Security fix. Restarting a server that the config-load admission gate (#937) had quarantined silently cleared its quarantine, and the next unrelated config write, reload or process restart then admitted it and auto-approved its tools. This is not a Spec 108/109 task, so no spec documents change and no names or contracts change on any surface (REST, CLI, MCP, Web UI, macOS app are unaffected apart from the corrected behavior).

Root cause

lookupServerConfigForRestart in internal/runtime/lifecycle.go is the #467 "disk-first restart" helper. It re-reads mcp_config.json, writes the raw file entry to config.db with SaveUpstreamServer, and hands the same entry to the upstream manager. A server entry that never stated quarantined decodes as not quarantined with no explicit bit. The admission gate had quarantined it at load and recorded that in config.db, so the restart overwrote the recorded quarantine with false. The gate's durability depends on config.db: its "known server" branch only re-holds a server when the stored record is quarantined and the config states nothing. After the overwrite, the next SaveConfiguration (approving or disabling a different server, a PATCH, enable/disable) rebuilt the file and snapshot from storage without the quarantine, and any later reload or reboot treated the server as known, live and vetted. The restarted client was also created as trusted until the next reconcile.

Affected paths

Everything that reaches Runtime.RestartServer: REST restart and restart_all, mcpproxy upstream restart (including --all), MCP upstream_servers operation=restart, the tray restart, secret changes that restart affected servers, the scanner's EnsureConnected (manual scan and the automatic baseline sweep shortly after startup), and the server-edition admin handler. The bug has existed since the gate was added in #944, which did not gate this disk read.

Fix

1. Restart gates the disk entry. Only the restarted server is run through the existing admission gate as a one-server config, so other servers do not re-log the "predate the gate" warning or emit duplicate first-seen activity. When storage is unreadable the gate would abstain, so restart fails closed instead: a server that states nothing inherits the quarantine of the currently published (already gated) entry, or the trust-mode default if the runtime has never seen it, and nothing is written to the unreadable storage.
2. Defense in depth in storage. SaveUpstreamServer and the async saveServerSync never lower a stored quarantine unless the incoming config states quarantined explicitly. The read-check-write happens inside one bbolt transaction, so a concurrent QuarantineUpstreamServer cannot be lost. The guard changes only the persisted record, never the caller's struct (which may be a published snapshot pointer), and logs a warning naming the server.
3. REST PATCH carries the explicit bit. The PATCH handler marks the quarantine decision explicit only when the body contains quarantined, and UpdateServer applies the value only then. Previously an unrelated PATCH could copy a stale false from a config snapshot over the stored record.

Decisions

• An explicit "quarantined": false in the file, or an explicit PATCH, is still obeyed (operator statement keeps its bug: mcp_config.json file-watcher hot-reload doesn't propagate env (and likely headers) changes to running upstreams #467 and security: servers loaded from mcp_config.json bypass the trust-mode admission gate entirely #937 meaning).
• The legitimate un-quarantine doors are unchanged: Runtime.QuarantineServer(name, false) (security approve, tray, REST unquarantine, scan-mode auto-approve) goes through QuarantineUpstreamServer, which the guard does not touch.
• Add paths need no change: with no prior record the guard is a no-op.
• Installs already hit by this bug have a stored false and are not re-quarantined (same upgrade rule as security: servers loaded from mcp_config.json bypass the trust-mode admission gate entirely #937); they surface in the existing "predate the config-load admission gate" warning. docs/features/security-quarantine.md now says so.
• No new dependencies.

Tests

• internal/runtime/restart_quarantine_test.go: restart keeps implicit quarantine (storage, published config and the written file, including after approving a different server), keeps it across a simulated reboot with no tool approvals promoted, gates a first-seen server restarted from disk, obeys an explicit false on disk, does not over-quarantine under trust_mode: auto, and fails closed when storage is unreadable.
• internal/storage/quarantine_guard_test.go and async_ops_test.go: the guard never lowers without an explicit decision (Go-built and JSON-decoded explicit false both lower), new servers and raises are unaffected, the caller's struct is not mutated, QuarantineUpstreamServer still un-quarantines, and a concurrent quarantine is not lost under -race.
• internal/httpapi/patch_server_test.go: the explicit bit is set only when the body carries quarantined.
• internal/server/update_server_quarantine_test.go: an update that omits quarantine keeps the stored value; an explicit false un-quarantines and is written to the file.
• Existing tests that un-quarantined by re-saving a plain struct now mark the decision explicit instead of weakening the guard.
• Verified locally with go test -race on runtime, storage, httpapi, server, management, security and config, the server-edition lane with the CI skip regex, the server E2E tests, and golangci-lint (bare and --build-tags server).

Follow-up (medium/low, not in this PR)

REST PATCH quarantined:false still bypasses the baseline-approve and activity event that QuarantineServer performs; consider routing it through QuarantineServer.

[cloudflare-workers-and-pages]

Deploying mcpproxy-docs with    Cloudflare Pages

Latest commit:	0edf3da
Status:	✅  Deploy successful!
Preview URL:	https://89fd7812.mcpproxy-docs.pages.dev
Branch Preview URL:	https://fix-quarantine-restart-bypas.mcpproxy-docs.pages.dev

View logs

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 92.30769% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/storage/bbolt.go	76.47%	2 Missing and 2 partials ⚠️
internal/runtime/lifecycle.go	75.00%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

📦 Build Artifacts

Workflow Run: View Run
Branch: fix-quarantine-restart-bypass

Available Artifacts

• archive-darwin-amd64 (31 MB)
• archive-darwin-arm64 (28 MB)
• archive-linux-amd64 (18 MB)
• archive-linux-arm64 (17 MB)
• archive-windows-amd64 (31 MB)
• archive-windows-arm64 (27 MB)
• frontend-dist-pr (0 MB)
• installer-dmg-darwin-amd64 (26 MB)
• installer-dmg-darwin-arm64 (24 MB)
• smart-mcp-proxymcpproxy-goVVN9KL.dockerbuild (0 MB)

How to Download

Option 1: GitHub Web UI (easiest)

1. Go to the workflow run page linked above
2. Scroll to the bottom "Artifacts" section
3. Click on the artifact you want to download

Option 2: GitHub CLI

gh run download 36994351547 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.

[github-actions]

Approved (Model B): Paperclip review verdicts = ACCEPT and qa-gate green at this head SHA. Arming auto-merge; GitHub merges when all required checks pass.