Skip to content

Commit a1101fb

Browse files
authored
fix: first-run docs, changelog, upgrade-start log noise and set_profile reach (#1482)
Docs, changelog and verified fixes from a first-run, upgrade and user-test pass on `main` (335477e). ## Docs - `docs/features/profiles.md`: profiles are optional. Without an effective profile, no profile-level restriction applies (a scoped credential's own grant still does). The quick start is presented as being for scoped access. - `docs/configuration.md` (`quarantined` row): explains the real default. A server added through the UI, CLI or API follows its trust mode. A first-seen config-file server with no `quarantined` key and no `config.db` record is held for review. An explicit value wins, and a recorded server keeps its state. Links to the admission rules. - `docs/features/security-quarantine.md`: the hand-editing section names the extra conditions: quarantine is enabled, and the trust mode is not `auto` (including legacy `auto_approve_tool_changes`/`skip_quarantine` resolving to `auto`), matching `ServerConfig.EffectiveTrustMode`. - `docs/getting-started/quick-start.mdx`: a plain note that a server added by editing the file is held for review until approved. - `CHANGELOG.md`: the security, review, catalog, telemetry, CLI and first-run fixes merged since v0.69.0 (#1463, #1467–#1473, #1481) plus this PR's. ## Fixes (each reproduced in an isolated instance, regression test fails on main) - **Upgrade start advisory repeated.** `reportPreFixAdmissions` ran on every gate pass (initial gate, `LoadConfiguredServers`, every publish), so an upgrade start logged the "predate the config-load admission gate" advisory 2–5 times. It is now reported once per server per process. Gate decisions are unchanged. - **Benign startup race logged as ERROR.** Both the supervisor reconcile and `LoadConfiguredServers` → `AddServer` asked a not-yet-ready client to connect. The second call was correctly refused with "connection already in progress or established (state: Connecting)" but logged at ERROR, and server identity registration was skipped. A new `managed.ErrConnectAlreadyActive` sentinel (same message text) lets `AddServer` treat it as success at debug level. - **`set_profile` under-reported a switchable client's reach.** After a successful switch, `resolveEffectiveProfileForJustSetSlug` treated the client binding as authoritative and reported the binding's servers instead of the selected profile's. ## Verification - Cross-model review: codex gpt-6.1-sol, chunked. Three low findings: two doc-wording findings are applied, and the third (the per-process advisory set is never cleared) is a deliberate trade-off. - Live re-verification on isolated instances: each symptom was reproduced on a main build and is gone on this branch. The upgrade case was run over a v0.52.1 data dir.
1 parent 335477e commit a1101fb

13 files changed

Lines changed: 257 additions & 5 deletions

‎CHANGELOG.md‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,51 @@ Releases follow [Semantic Versioning](https://semver.org/).
167167

168168
### Bug Fixes
169169

170+
- **security/quarantine:** a server added by editing `mcp_config.json` (no `quarantined` key,
171+
first seen) stayed held for review only until its first restart. Restarting it (by hand, after a
172+
secret change, or through the automatic baseline scan's connect) wrote the raw file entry over
173+
the recorded quarantine, so the next config write or core restart admitted it and auto-approved
174+
its tools. The restart path now runs the entry through the config-load admission gate, and storage
175+
never lowers a recorded quarantine unless the operator sets `"quarantined": false` or approves the
176+
server. Present since v0.53.0. ([#1463](https://github.com/smart-mcp-proxy/mcpproxy-go/pull/1463))
177+
- **security/cli:** `mcpproxy doctor` no longer prints the admin API key (it was embedded in the Web
178+
UI URL) in any output format, and URL credentials are masked in its output. ([#1472](https://github.com/smart-mcp-proxy/mcpproxy-go/pull/1472))
179+
- **review:** the Review screen starts fail-closed: read tools are pre-selected, write, destructive
180+
and unannotated tools are not, the button states the exact count ("Approve server (3 of 9
181+
tools)"), and approving everything is an explicit action. The same defaults apply on macOS and
182+
in `mcpproxy review approve`. ([#1481](https://github.com/smart-mcp-proxy/mcpproxy-go/pull/1481))
183+
- **review:** the scan banner no longer says "clean" for definitions that were never scanned or
184+
changed after the scan, an approved server shows its approved state instead of per-tool
185+
Approve/Reject buttons, and tool definitions are captured automatically when a quarantined
186+
server is imported. ([#1470](https://github.com/smart-mcp-proxy/mcpproxy-go/pull/1470))
187+
- **catalog:** search finds the server you mean: a typed query fetches enough results from each
188+
registry, versions of one server collapse to a single entry, an exact owner or name match ranks
189+
first (searching "github" now returns GitHub's own server first), and "Verified" means the
190+
publisher owns the source repository. ([#1469](https://github.com/smart-mcp-proxy/mcpproxy-go/pull/1469))
191+
- **telemetry:** when telemetry is disabled by `MCPPROXY_TELEMETRY=false`, `DO_NOT_TRACK` or `CI`,
192+
the setup wizard, Home banner, Settings and the macOS app say so and lock the control instead of
193+
showing the opt-out notice; `GET /api/v1/status` reports the effective state and its source.
194+
macOS Settings shows the listen address of the core it is connected to. ([#1471](https://github.com/smart-mcp-proxy/mcpproxy-go/pull/1471))
195+
- **cli:** the global `-c/--config` and `-d/--data-dir` flags apply to every management command
196+
(`upstream`, `registry`, `catalog` included) in any position, and no command silently creates a
197+
default config in `~/.mcpproxy` when one was given; errors are printed once; a locked client
198+
credential calling `set_profile` is told that profile changes are locked. ([#1472](https://github.com/smart-mcp-proxy/mcpproxy-go/pull/1472))
199+
- **web:** first-run fixes — the setup wizard shows one completion state after importing servers,
200+
previewing an empty client config no longer fails, secret values are masked while you type,
201+
the status pill says "awaiting review" for quarantined servers instead of "0 online", and the
202+
profile editor's Try it uses your unsaved edits and shows readable reasons. ([#1473](https://github.com/smart-mcp-proxy/mcpproxy-go/pull/1473))
203+
- **profiles:** a call refused inside `code_execution` records the same `block_reason` as a
204+
top-level refusal, and `mcpproxy access explain` names the destination profile in its move-client
205+
fix. ([#1468](https://github.com/smart-mcp-proxy/mcpproxy-go/pull/1468))
206+
- **web:** token savings are labelled as an estimate on Home and in the macOS app, Activity's
207+
"Clear filters" on a server/tool conflict reloads the list, "Needs review" health badges are
208+
orange, and a brand-new install shows a getting-started card instead of "All clear". ([#1467](https://github.com/smart-mcp-proxy/mcpproxy-go/pull/1467))
209+
- **logging:** an upgrade start no longer repeats the "predate the config-load admission gate"
210+
advisory on every config pass (once per server per process), and the benign startup race
211+
"connection already in progress or established" is logged at debug instead of error.
212+
- **profiles:** `set_profile` from a switchable client credential reports the servers of the
213+
profile it just selected instead of the binding's narrower set.
214+
170215
- **web/settings:** toggles for nullable settings (`quarantine_enabled`, `telemetry.enabled` and the
171216
`audit_log.*` booleans) show the value the core actually applies instead of OFF when the key is
172217
absent, and the page header now says to press Save changes instead of "Changes save instantly".
@@ -235,6 +280,10 @@ Releases follow [Semantic Versioning](https://semver.org/).
235280

236281
### Documentation
237282

283+
- **docs:** profiles are described as optional (without one, nothing profile-level restricts a
284+
caller), and the `quarantined` default for servers added by editing `mcp_config.json` is
285+
explained, with the full admission rules in Security Quarantine and a first-run note in the Quick
286+
Start.
238287
- **051:** README hero — frosted-tiles banner + demo GIF (#488) ([#488](https://github.com/smart-mcp-proxy/mcpproxy-go/pull/488)) ([`25731da`](https://github.com/smart-mcp-proxy/mcpproxy-go/commit/25731da5e1a26753ee90a173a8ea03a317e82666))
239288

240289
## [0.33.1] - 2026-05-20

‎docs/configuration.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ it and none can double-report it.
415415
| `oauth` | object | No | OAuth configuration (see [OAuth Configuration](#oauth-configuration)) |
416416
| `isolation` | object | No | Per-server Docker isolation settings (see [Docker Isolation](#docker-isolation)) |
417417
| `enabled` | boolean | No | Enable/disable server (default: `true`) |
418-
| `quarantined` | boolean | No | Security quarantine status (default: `false` for manually added servers, `true` for LLM-added servers) |
418+
| `quarantined` | boolean | No | Security quarantine status. The default depends on how the server arrives. A server added through the UI, CLI, REST API or an AI agent follows its [trust mode](features/security-quarantine.md#trust-modes-auto--scan--manual) at add time (quarantined under the default `manual` mode). A server you add by editing this file, with no `quarantined` key and no prior `config.db` record, is **held for review** on first load when quarantine is enabled and its trust mode is not `auto` (no `trust_mode` means `manual`, unless a legacy `auto_approve_tool_changes: true` or `skip_quarantine: true` resolves it to `auto`). An explicit `"quarantined": false` admits it, and an explicit `true` holds it. A server that is already recorded in `config.db` keeps its recorded state. See [Servers added by hand-editing `mcp_config.json`](features/security-quarantine.md#servers-added-by-hand-editing-mcp_configjson) for the full admission rules. |
419419
| `reconnect_on_use` | boolean | No | When `true`, tool calls to a disconnected server trigger an immediate reconnect attempt (15s timeout) before failing (default: `false`) |
420420
| `expose_prompts` | boolean | No | Per-server override for whether this server's MCP prompts are aggregated into mcpproxy's `prompts/list`. Only takes effect when the global `aggregate_upstream_prompts` master switch is on. Omit to expose prompts whenever the server advertises `Capabilities.Prompts`; `false` opts this server out even if it does. |
421421
| `toon_output` | string | No | Per-server override for the global [`toon_output`](#toon-output-adaptive-result-encoding): `off`, `adaptive`, or `always`. Non-empty value wins over the global for this server's tools; omit to inherit. See [TOON Output](features/toon-output.md). |

‎docs/features/profiles.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ description: "Named views over your upstream servers with a tool policy: tier ca
66

77
# Profiles
88

9+
:::note Profiles are optional
10+
You do not need a profile to use MCPProxy. Without an effective profile (no profiles configured, or a caller that none of them applies to), no profile-level restriction applies to that caller: it can reach every configured server, unless its own credential is scoped (for example an agent token with an allowed-servers list). Use profiles when you need to scope access, for example to give one client or token read-only access to a few servers.
11+
:::
12+
913
A **profile** is a named view over your upstream servers plus a **tool policy**. It decides which servers a caller reaches, which of their tools the caller can discover and call, and whether the caller gets code execution and the management tools. The same profile is used by every surface that lets you work with MCPProxy: the config file, the Web UI and macOS app (**Profiles** in the sidebar), the CLI (`mcpproxy profile ...`), the MCP `profiles` tool and the REST API.
1014

1115
A profile is enforced by the core, on every path a tool can be discovered or called through. "Work Read-only" really is read-only: a session under it cannot find, describe or call a write, destructive, denied or unclassified tool, whichever way it connected.
@@ -18,6 +22,8 @@ Three ideas fit together:
1822

1923
## Quick start
2024

25+
This quick start is for users who need scoped access. If you do not, you can skip profiles entirely.
26+
2127
```json
2228
{
2329
"require_mcp_auth": true,

‎docs/features/security-quarantine.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ A config-file server is held for review when **both** of the following are true:
4444
is an explicit operator statement and is obeyed), **and**
4545
- the server is not yet recorded in `config.db`.
4646

47+
Quarantine must also be enabled (`quarantine_enabled`, on by default), and the
48+
server's [trust mode](#trust-modes-auto--scan--manual) must not be `auto`. A
49+
server with no `trust_mode` is `manual` unless a legacy setting resolves to
50+
`auto` (`"auto_approve_tool_changes": true`, or the older
51+
`"skip_quarantine": true`), so by default a first-seen config-file server is
52+
held; a server whose trust mode is `auto` is admitted.
53+
4754
The second condition is what makes upgrading safe: every server you are already
4855
running has a `config.db` record, so **upgrading never re-quarantines a server
4956
you have already vetted**. The boundary is that a server present in a

‎docs/getting-started/quick-start.mdx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,10 @@ Edit `~/.mcpproxy/mcp_config.json`:
233233
}
234234
```
235235

236+
:::note First-time config-file servers are held for review
237+
A server you add by editing this file for the first time is **held for review** (quarantined) until you approve it in the **Review queue**. The examples above omit the `quarantined` key on purpose. Writing `"quarantined": false` admits a server right away, so use it only for servers you have already vetted. A server that MCPProxy already knows keeps its recorded state. See [Servers added by hand-editing `mcp_config.json`](/features/security-quarantine#servers-added-by-hand-editing-mcp_configjson).
238+
:::
239+
236240
## 5. Approve It Out of Quarantine
237241

238242
However you added it, the server is now **quarantined** - its tools are withheld

‎internal/runtime/config_load_admission_gate.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,25 @@ func (r *Runtime) reportPreFixAdmissions(names []string) {
224224
if len(names) == 0 {
225225
return
226226
}
227+
// The gate runs several times per process (gateInitialConfig, then
228+
// LoadConfiguredServers, then the pre-publish hook on every publish); name
229+
// each affected server once per process.
230+
r.preFixMu.Lock()
231+
if r.preFixReported == nil {
232+
r.preFixReported = make(map[string]struct{})
233+
}
234+
fresh := names[:0:0]
235+
for _, n := range names {
236+
if _, seen := r.preFixReported[n]; !seen {
237+
r.preFixReported[n] = struct{}{}
238+
fresh = append(fresh, n)
239+
}
240+
}
241+
r.preFixMu.Unlock()
242+
if len(fresh) == 0 {
243+
return
244+
}
245+
names = fresh
227246
r.logger.Warn("Configured servers predate the config-load admission gate and have never been explicitly reviewed",
228247
zap.Strings("servers", names),
229248
zap.String("action", "review them in the quarantine UI, or record the decision with an explicit \"quarantined\" value in mcp_config.json (issue #937)"))
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package runtime
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
"go.uber.org/zap"
8+
"go.uber.org/zap/zaptest/observer"
9+
10+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
11+
"github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime/configsvc"
12+
)
13+
14+
// Item 4a: on an upgrade start the pre-fix advisory must be emitted once per
15+
// process, not once per gate pass. Startup runs the gate in gateInitialConfig
16+
// (before the supervisor starts) and again in LoadConfiguredServers
17+
// (backgroundInitialization), and every later configsvc publish runs it again
18+
// through the pre-publish hook. None of those passes changes the affected set.
19+
func TestConfigLoadAdmissionGate_PreFixAdvisoryOncePerProcess(t *testing.T) {
20+
core, logs := observer.New(zap.WarnLevel)
21+
rt, _, _ := gateEnvAt(t, []map[string]any{
22+
{"name": "pre-fix", "command": "./poison", "protocol": "stdio", "enabled": true},
23+
{"name": "reviewed", "command": "./ok", "protocol": "stdio", "enabled": true, "quarantined": false},
24+
}, nil, zap.New(core))
25+
26+
// What an older release left behind: both servers known to config.db, live.
27+
for _, name := range []string{"pre-fix", "reviewed"} {
28+
require.NoError(t, rt.storageManager.SaveUpstreamServer(&config.ServerConfig{
29+
Name: name, Command: "./x", Protocol: "stdio", Enabled: true,
30+
}))
31+
}
32+
33+
// Production startup order (lifecycle.go): hook + initial gate, then
34+
// backgroundInitialization -> LoadConfiguredServers(nil).
35+
rt.installAdmissionGateHook()
36+
rt.gateInitialConfig()
37+
require.NoError(t, rt.LoadConfiguredServers(nil))
38+
39+
// An unrelated publish (e.g. a hot reload of an unchanged file).
40+
cur := rt.configSvc.Current().Config
41+
require.NoError(t, rt.configSvc.Update(cur, configsvc.UpdateTypeReload, "test_reload"))
42+
43+
require.Len(t, logs.FilterMessageSnippet("predate").All(), 1,
44+
"the same pre-fix set must be reported once per process, not once per gate pass")
45+
}

‎internal/runtime/runtime.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,12 @@ type Runtime struct {
127127
selfWriteMu sync.Mutex
128128
recentSelfWrites []selfWriteEntry
129129

130+
// preFixReported holds the servers the "predate the config-load admission
131+
// gate" advisory has already named in this process, so the gate passes at
132+
// startup and on every later publish do not repeat it.
133+
preFixMu sync.Mutex
134+
preFixReported map[string]struct{}
135+
130136
statusMu sync.RWMutex
131137
status Status
132138
statusCh chan Status

‎internal/server/profile_resolver.go‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,16 @@ func (p *MCPProxyServer) resolveActiveProfileWithSourceFromIndex(ctx context.Con
336336
// review round 2). idx.position is the O(1) existence check the pin/slug
337337
// tiers actually need.
338338
func (p *MCPProxyServer) resolveEffectiveProfileForJustSetSlug(ctx context.Context, idx *profileIndex, slug string) string {
339-
if pin := profilePinFromContext(ctx); pin != "" {
339+
// A SWITCHABLE client credential's ProfilePin is its BINDING (the base for
340+
// switchable_to), which FR-020 ranks BELOW the url and session tiers. It is
341+
// authoritative only while no selection is in effect, so a just-written
342+
// slug must be reported instead of the binding. A locked credential and a
343+
// regular agent-token pin stay authoritative.
344+
switchableBinding := ""
345+
if pin, mode, ok := clientCredentialFromContext(ctx); ok && mode != auth.ProfileModeLocked {
346+
switchableBinding = pin
347+
}
348+
if pin := profilePinFromContext(ctx); pin != "" && pin != switchableBinding {
340349
if idx != nil && idx.position(pin) >= 0 {
341350
return pin
342351
}
@@ -354,10 +363,12 @@ func (p *MCPProxyServer) resolveEffectiveProfileForJustSetSlug(ctx context.Conte
354363
if urlScope := profile.ProfileScopeFromContext(ctx); urlScope != nil {
355364
return urlScope.Name
356365
}
357-
if slug != "" && idx != nil && idx.position(slug) >= 0 {
366+
if slug != "" && idx != nil && idx.position(slug) >= 0 && (switchableBinding == "" || idx.position(switchableBinding) >= 0) {
358367
return slug
359368
}
360-
return ""
369+
// No selection in effect: a switchable credential falls back to its binding
370+
// (a dangling binding stays deny-all, as resolveV3Base resolves it).
371+
return switchableBinding
361372
}
362373

363374
// profileScopeFromIndex builds the ProfileScope for slug's FULL membership
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package server
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/mark3labs/mcp-go/mcp"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
// set_profile must report the servers the session can actually reach after the
12+
// switch. For a SWITCHABLE client credential the bound profile is only the base
13+
// (profile.SourceBinding): the session selection outranks it, so the reported
14+
// list is the selected profile's reach, not the binding's.
15+
func TestSetProfileV3SwitchableClientReportsSelectedProfileServers(t *testing.T) {
16+
proxy, _ := newProfilesV3Fixture(t)
17+
18+
setProfile := func(t *testing.T, sid, bindingMode, slug string) (active string, servers []string) {
19+
t.Helper()
20+
ctx := sessionCtx(clientCtx("laptop", "work-readonly", bindingMode), sid)
21+
request := mcp.CallToolRequest{}
22+
request.Params.Arguments = map[string]interface{}{"profile": slug}
23+
result, err := proxy.handleSetProfile(ctx, request)
24+
require.NoError(t, err)
25+
require.False(t, result.IsError, resultText(t, result))
26+
var payload struct {
27+
Active string `json:"active_profile"`
28+
Servers []string `json:"servers"`
29+
}
30+
require.NoError(t, json.Unmarshal([]byte(resultText(t, result)), &payload))
31+
return payload.Active, payload.Servers
32+
}
33+
34+
t.Run("switching to a wider profile reports the wider reach", func(t *testing.T) {
35+
active, servers := setProfile(t, "sw-wide", "switchable", "work-full")
36+
require.Equal(t, "work-full", active)
37+
require.ElementsMatch(t, []string{"github", "notion", "filesystem"}, servers)
38+
})
39+
40+
t.Run("re-selecting the bound profile reports the binding's reach", func(t *testing.T) {
41+
active, servers := setProfile(t, "sw-base", "switchable", "work-readonly")
42+
require.Equal(t, "work-readonly", active)
43+
require.ElementsMatch(t, []string{"github", "notion"}, servers)
44+
})
45+
46+
t.Run("clearing the selection falls back to the binding", func(t *testing.T) {
47+
active, servers := setProfile(t, "sw-clear", "switchable", "")
48+
require.Equal(t, "", active)
49+
require.ElementsMatch(t, []string{"github", "notion"}, servers)
50+
})
51+
}

0 commit comments

Comments
 (0)