fix(security): don't re-quarantine servers added while running (rc.3 release gate) (#1488)

# Pull Request

## Description

Fixes the `v0.70.0-rc.3` release-gate failure (`matrix/oauth`: *server
not ready, connected=true tools=0*). That failure was a regression from
#1485 (RC-UPG-001).

**Cause**
- #1485 re-quarantines a server that is known to config.db but has no
approved tool baseline.
- `Server.AddServer` saves a new server to config.db **before** it
publishes the config, and the admission gate runs on every publish.
- So a server added at runtime looked "known but never approved" and was
held before its tools could be discovered.
- An OAuth server added with `quarantined: false` has no tools until
sign-in, which is exactly the gate's oauth cell.

**Fix**
- The no-baseline rule now applies only to servers that config.db held
the first time the gate read it in this process
(`Runtime.bootKnownServers`). Those are the only records an older
release could have left behind.
- A `quarantined` value stated at add time is now recorded as an
operator statement (`MarkQuarantineExplicitlySet`). It is written to
`mcp_config.json` and obeyed after a restart. This covers:
  - REST add;
  - the `upstream_servers` MCP add;
  - CLI `upstream add --no-quarantine` without a running daemon;
  - imports with `skip_quarantine` (REST and CLI).

The last two were found by an add-path completeness sweep during review.
The CLI direct-file case is pre-existing.

## Testing
- [x] I have tested these changes locally
- [x] I have added/updated tests that prove my fix is effective or my
feature works
- [x] All existing tests pass

| Check | rc.3 (`dc851c8f5`) | this branch |
|---|---|---|
| `release-gate matrix --cells oauth` (real gate driver, run locally) |
fail, same error as CI | pass |
| `release-gate matrix --cells stdio,http,sse` | — | pass |
| v0.69 → upgrade replay (keyless server restarted under v0.69) | — |
keyless server held, vetted server live |

- **New tests:**
-
`TestConfigLoadAdmissionGate_ServerAddedWhileRunningIsNotRequarantined`;
  - an explicit-bit assertion in `TestImport_SkipQuarantine`.
- **Suites:** the race suite with `-tags server` passes for runtime,
httpapi, server and config. `configimport` and `cmd/mcpproxy` tests
pass.
- **Lint:** clean on the changed files.
- **Review:** zcode (GLM) found nothing on the diff. The add-path
sweep's two confirmed gaps are fixed here; a third, theoretical
bootKnown edge was refuted on verification.

After merge, the next RC will be `v0.70.0-rc.4`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Author: Dumbris

CHANGELOG.md

@@ -169,6 +169,8 @@ Releases follow [Semantic Versioning](https://semver.org/).
 
 - **Upgrade no longer admits a server an older release held in quarantine (RC-UPG-001).** A v0.69 restart (including the baseline security scan shortly after startup) wrote a keyless server's un-gated config over its recorded quarantine, while v0.69 kept holding it in memory. On upgrade the config-load admission gate read that record as already admitted and auto-baseline approved all of its tools. The gate now also requires an approval baseline: a server recorded as unquarantined whose tools were never approved is quarantined again for review. Servers with approved tools are unaffected; vetted servers with no tool records (zero-tool servers, servers that never connected while live, servers last vetted before v0.21) are also held once — approve them, or set `"quarantined": false` before upgrading.
 
+- **A server added while mcpproxy runs is no longer re-quarantined before its tools are discovered.** The RC-UPG-001 rule now applies only to servers `config.db` already held at startup; adding a server saves it to `config.db` before publishing its config, so an OAuth server added with `quarantined: false` was held again before sign-in (v0.70.0-rc.3 release gate). Adding a server with an explicit `quarantined` value — REST, the `upstream_servers` tool, `upstream add --no-quarantine` without a running daemon, or an import with `skip_quarantine` — now records it as an operator statement in `mcp_config.json`, so it is honoured after a restart.
+
 - **security/quarantine:** a server added by editing `mcp_config.json` (no `quarantined` key,
   first seen) stayed held for review only until its first restart. Restarting it (by hand, after a
   secret change, or through the automatic baseline scan's connect) wrote the raw file entry over

cmd/mcpproxy/upstream_cmd.go

@@ -1717,6 +1717,12 @@ func runUpstreamAddConfigMode(req *cliclient.AddServerRequest, globalConfig *con
 		Quarantined: quarantined,
 		TrustMode:   req.TrustMode,
 	}
+	if req.Quarantined != nil {
+		// A stated --quarantine/--no-quarantine is written to the file as an
+		// operator statement; otherwise SaveConfig drops quarantined:false
+		// and the admission gate holds the server on the next start.
+		newServer.MarkQuarantineExplicitlySet(true)
+	}
 
 	// Add to config
 	globalConfig.Servers = append(globalConfig.Servers, newServer)

internal/configimport/import.go

@@ -149,6 +149,11 @@ func Import(content []byte, opts *ImportOptions) (*ImportResult, error) {
 		// Override quarantine if SkipQuarantine is set
 		if opts.SkipQuarantine {
 			serverConfig.Quarantined = false
+			// The caller stated the value: keep it as an operator statement
+			// so it is written to mcp_config.json and the admission gate
+			// obeys it after a restart, instead of re-holding a server
+			// whose tools were never discovered.
+			serverConfig.MarkQuarantineExplicitlySet(true)
 		}
 
 		// FR-040: classify env/header fields and build the second-line

internal/configimport/import_test.go

@@ -292,6 +292,12 @@ func TestImport_SkipQuarantine(t *testing.T) {
 			if s.Server.Quarantined {
 				t.Errorf("server %s should NOT be quarantined when SkipQuarantine=true", s.Server.Name)
 			}
+			// RC-UPG-001 follow-up: the stated value must persist as an
+			// operator statement, or the boot gate re-holds the server after
+			// a restart when its tools were never discovered.
+			if !s.Server.QuarantineExplicitlySet() {
+				t.Errorf("server %s: SkipQuarantine must be recorded as an explicit quarantine statement", s.Server.Name)
+			}
 		}
 	})
 }

internal/httpapi/server.go

@@ -2513,6 +2513,12 @@ func (s *Server) handleAddServer(w http.ResponseWriter, r *http.Request) {
 		Enabled:     enabled,
 		Quarantined: quarantined,
 	}
+	if req.Quarantined != nil {
+		// The caller stated the value: record it as an operator statement so
+		// the admission gate obeys it on later loads (it is written to
+		// mcp_config.json instead of being dropped as a default).
+		serverConfig.MarkQuarantineExplicitlySet(true)
+	}
 	if req.ReconnectOnUse != nil {
 		serverConfig.ReconnectOnUse = *req.ReconnectOnUse
 	}

internal/runtime/config_load_admission_gate.go

@@ -51,7 +51,11 @@ import (
 // records: zero-tool (prompts/resources-only) servers, servers never
 // discovered while live (e.g. disabled since they were vetted), and servers
 // last vetted before tool approvals were stored (v0.21). Each is held once
-// on upgrade; one approval releases it and records the decision. When the
+// on upgrade; one approval releases it and records the decision. The rule
+// applies only to servers config.db already held when this process started
+// (bootKnownServers): AddServer saves a new server to config.db before it
+// publishes the config, so a server added while running must not be read as
+// "known but never approved" before its tools are even discovered. When the
 // approval records cannot be read the gate keeps the old rule: unknown is not
 // the same as never admitted.
 //
@@ -148,6 +152,13 @@ func (r *Runtime) applyConfigLoadAdmissionGate(cfg *config.Config, stored map[st
 		return cfg, false
 	}
 
+	r.bootKnownOnce.Do(func() {
+		r.bootKnownServers = make(map[string]struct{}, len(stored))
+		for name := range stored {
+			r.bootKnownServers[name] = struct{}{}
+		}
+	})
+
 	decisions := make(map[int]admissionDecision)
 	var preFix []string
 
@@ -173,6 +184,13 @@ func (r *Runtime) applyConfigLoadAdmissionGate(cfg *config.Config, stored map[st
 			// the fix looks like. Upgrade safety says leave it alone; honesty
 			// says say so.
 			if !sc.QuarantineExplicitlySet() && cfg.QuarantineDefaultForServer(sc) {
+				_, bootKnown := r.bootKnownServers[sc.Name]
+				if !bootKnown {
+					// Added while this process runs: AddServer saves it to
+					// config.db before publishing, so "known" here only means
+					// "just added" — the add path already decided admission.
+					continue
+				}
 				if vetted, ok := r.hasApprovalBaseline(sc.Name); ok && !vetted {
 					decisions[i] = admissionDecision{
 						server: sc.Name,

internal/runtime/config_load_admission_gate_upgrade_test.go

@@ -126,3 +126,27 @@ func TestConfigLoadAdmissionGate_RequarantineSurvivesRestart(t *testing.T) {
 	require.NoError(t, rt.LoadConfiguredServers(reloaded))
 	assert.True(t, storedServer(t, rt, "legacy-keyless").Quarantined)
 }
+
+// v0.70.0-rc.3 release gate (matrix/oauth): AddServer saves a new server to
+// config.db BEFORE publishing the config, so the publish-time gate saw it as
+// "known, unquarantined, never approved" and re-held it before its tools
+// (an OAuth server has none until sign-in) could be discovered. The
+// no-baseline rule is for records an older release left behind, so it only
+// applies to servers config.db held when this process started.
+func TestConfigLoadAdmissionGate_ServerAddedWhileRunningIsNotRequarantined(t *testing.T) {
+	rt, cfg, _ := gateEnvAt(t, []map[string]any{
+		{"name": "boot", "command": "./x", "protocol": "stdio", "enabled": true, "quarantined": false},
+	}, nil, zap.NewNop())
+	// Boot: the first gate pass records what config.db held at start.
+	require.NoError(t, rt.LoadConfiguredServers(cfg))
+
+	// Runtime add, in AddServer's order: storage first, then the config.
+	added := &config.ServerConfig{Name: "added-later", URL: "http://127.0.0.1:1/mcp", Protocol: "http", Enabled: true}
+	require.NoError(t, rt.storageManager.SaveUpstreamServer(added))
+	next := *rt.Config()
+	next.Servers = append(append([]*config.ServerConfig{}, rt.Config().Servers...), config.CopyServerConfig(added))
+	require.NoError(t, rt.LoadConfiguredServers(&next))
+
+	assert.False(t, storedServer(t, rt, "added-later").Quarantined,
+		"a server added while running was admitted by its add path; the upgrade rule must not re-hold it")
+}

internal/runtime/runtime.go

@@ -133,6 +133,14 @@ type Runtime struct {
 	preFixMu       sync.Mutex
 	preFixReported map[string]struct{}
 
+	// bootKnownServers is the set of servers config.db held the first time
+	// the admission gate read it in this process (RC-UPG-001). Only those can
+	// carry a stale record from an older release; a server added while this
+	// process runs is saved to config.db before its config is published, so
+	// the gate must not read it as "known but never approved".
+	bootKnownOnce    sync.Once
+	bootKnownServers map[string]struct{}
+
 	statusMu sync.RWMutex
 	status   Status
 	statusCh chan Status

internal/server/mcp.go

@@ -5797,6 +5797,11 @@ func (p *MCPProxyServer) handleAddUpstream(ctx context.Context, request mcp.Call
 	// F9: optional per-server expose_prompts override on add. GetBool can't tell
 	// absent from false, so probe the raw args for presence.
 	if rawArgs := request.GetArguments(); rawArgs != nil {
+		// A stated `quarantined` is an operator statement the admission gate
+		// must obey on later loads, not a default to drop on save.
+		if _, stated := rawArgs["quarantined"]; stated {
+			serverConfig.MarkQuarantineExplicitlySet(true)
+		}
 		if raw, ok := rawArgs["expose_prompts"]; ok {
 			if b, ok := raw.(bool); ok {
 				serverConfig.ExposePrompts = &b