Repository navigation
fix(security): don't admit a server v0.69 held in quarantine on upgrade (RC-UPG-001) - #1485
Merged
Merged
Conversation
…ne on upgrade (RC-UPG-001) v0.69's restart path (including the baseline scan shortly after startup) re-read mcp_config.json and wrote a keyless server's ungated entry over its recorded quarantine in config.db, while v0.69 kept holding it in memory. On upgrade, the config-load admission gate treated "known to config.db" as already admitted, and auto-baseline approved all of the server's tools. The gate now also requires an approval baseline: a known, unquarantined server whose config never states `quarantined` and whose trust mode would hold it is quarantined again unless at least one of its tools was ever approved (approved now, or carrying an approved hash after a rug pull). Servers with approved tools stay live and still only get the pre-fix advisory. When approval records cannot be read the gate keeps the old rule. This fails closed for vetted servers with no tool records (zero-tool servers, servers never discovered while live, servers last vetted before v0.21): they are held once on upgrade; one approval or an explicit "quarantined": false releases them. Documented in security-quarantine.md and the changelog.
Deploying mcpproxy-docs with
|
| Latest commit: |
c0a727e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://166ed547.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://claude-fix-upgrade-admission.mcpproxy-docs.pages.dev |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
3 tasks done
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 3, 2026
…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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
Fixes RC-UPG-001, the release blocker in the v0.70.0-rc.2 test report. On upgrade from v0.69, a server that v0.69 held in quarantine was admitted, and all of its tools were auto-approved.
Root cause
lookupServerConfigForRestart) re-readmcp_config.json. A server with noquarantinedkey reads asfalse, and that path wrote the entry to config.db ungated.quarantined:false.Fix (
internal/runtime/config_load_admission_gate.go)quarantined;ApprovedHash, such as a rug-pulled "changed" record.Deliberate trade-off (fail-closed). Some vetted servers have no tool records, and nothing in storage distinguishes them from the stale v0.69 record. They are held once on upgrade:
One approval releases each, and so does an explicit
"quarantined": falseset before upgrading. This is documented indocs/features/security-quarantine.mdand the changelog.Tests
config_load_admission_gate_upgrade_test.gocovers:falseandtrust_mode: autostay live;Testing
Upgrade replay with the real binaries, on an isolated instance: v0.69 with a keyless server and a vetted server, then a restart of the keyless one, then an upgrade.
The race suite with
-tags serverpasses for runtime, config, storage, server and httpapi, and lint is clean. The cross-model review (zcode) found one MEDIUM, the population above. It is kept as a documented trade-off, and round 2 came back clean.🤖 Generated with Claude Code