Skip to content

bug: MCP/REST update, patch, quarantine and enable bypass the config commit lock (resurrect removed servers, revert security fields) #1560

Closed

Description

@Dumbris

Problem

Several single-server writers read and write storage outside configCommitMu, so they race with removal and with reconcile:

  • MCP upstream_servers update/patch (handleUpdateUpstream internal/server/mcp.go:6146, handlePatchUpstream :6266; storage write + upstreamManager.AddServer at :6212, :6328).
  • REST Server.UpdateServer (internal/server/server.go:2030, upsert at :2122) and REST PATCH (internal/httpapi/server.go:2682), which resolves omitted fields and map merges from an earlier snapshot.
  • Runtime.QuarantineServer (lifecycle.go:2175) and EnableServer, which write the storage flag before SaveConfiguration acquires the commit lock.

Failure scenarios

  1. PATCH reads A, pauses; A is removed; PATCH upserts A and reconnects it; its save republishes the removed server.
  2. An unrelated PATCH paused after its snapshot read reverts a later trust_mode: manual to auto, restores a removed forwarding allowlist, or re-enables a disabled server.
  3. Reconcile captures an explicit quarantined:false; concurrently QuarantineServer(name,true) persists true; reconcile saves its old explicit false (the storage quarantine guard allows it because of the explicit bit) and the quarantine request returns success with the server unquarantined.
  4. Update/patch accepts a name that is already removed from live config but awaiting async cleanup and resurrects it.

Suggested direction

Route update/patch/quarantine/enable through one commit helper that takes configCommitMu, re-reads the record, resolves partial updates inside the lock and re-validates existence (including pending-removal). Tests: pause each real path after its read, then remove / change security fields / quarantine, then resume.

Where Sol found it

ux01-r6a.txt #2, ux01-r6b.txt #2, ux01-r8-A.txt #1, ux01-r9-C.txt #1 and #2.

Related: #1558, #1559


Found by OpenCode Sol 6.1 cross-model review of the (now split) oversized PRs #1553 (UX-01) and #1555 (UX-02); verified still present on origin/main (704795c) by reading the cited sites. Pre-existing, deliberately out of scope of the minimal core PRs. Review transcripts: docs/qa/0.70.0-ux-2026-10-10/remediation/reviews/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/proxyCore MCP proxy engine and tool routingkind/bugSomething isn't workingpriority/highShould be addressed in the current or next release

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions