fix(security): scope upstream_servers tail_log to the caller's authorized servers

[Dumbris]

Part of Spec 105 (agent-token scope hardening) — FR-007, authorize-before-lookup with a non-disclosing refusal. Spec 105 is the acceptance contract on branch 105-agent-scope-hardening (not yet merged); the fix follows the Spec 104 FR-016h lineage.

Summary

The leak. upstream_servers tail_log is a per-server read that agent tokens may invoke (it is deliberately absent from auth.agentDeniedServerOps), but handleTailLog never consulted the caller's scope. It went straight to storage.GetUpstreamServer(name) and, on success, returned the server's log lines, its enabled/quarantined flags and its live connection_status. A token restricted to server a — by AllowedServers, by a profile pin, or by an explicit /mcp/p/<slug> URL profile — could tail hidden server b's log, and could use "not found" vs. a served response as an existence oracle for any server name it cared to probe.

The fix. The named server is authorized against the caller's effective scope before the lookup, using the same predicate list filters by and the call_tool_* variants enforce: serverInScope(authCtx, profileScope, name) with profileScope from resolveActiveProfile(ctx), so pin > URL profile > session selection are all honoured and a stale pin resolves to deny-all rather than widening to the token's own server list. The refusal is rendered by one shared helper, tailLogNotFound, which is now also used for the genuine storage.ErrUpstreamNotFound path, so an out-of-scope server and a nonexistent server produce byte-identical responses (modulo the echoed name): no existence, no status, no logs. Admin / OS-socket callers skip the AllowedServers check, so unscoped administrators are unaffected; an explicit /mcp/p/<slug> URL profile or session-selected profile bounds administrators too, exactly as list and call_tool_* already do on that surface (Spec 057 FR-004) — see review finding R1-1 below.

What changed

File	Change
internal/server/mcp.go	handleTailLog now takes ctx; adds the scope check ahead of the storage lookup; adds tailLogNotFound(name) as the single refusal shape and routes the ErrUpstreamNotFound branch through it. Other storage errors keep their previous wording. The scope-gate comment states the actual administrator policy (review R1-1).
internal/server/mcp_tail_log_scope_test.go	New. Five tests driving the real upstream_servers dispatcher with operation=tail_log.

Net: +229 / −1 over three commits (2d8728cd1 fix + tests, 1b6bbb68f test-fixture cleanup, 202ee1957 review comment correction), no new dependencies, no golden regeneration.

Tests

Fixture: two upstreams (github, secret) with per-server log files carrying a canary line, clients registered (never connected) so a served response includes connection_status — otherwise the "status not disclosed" assertions would pass vacuously — and a profile gh containing only github. Every hidden case is compared byte-for-byte (modulo name) against the unrestricted response for a server that truly does not exist (ghost).

• TestTailLog_ServerRestrictedToken_HidesOutOfScopeServer — AllowedServers=[github] token: secret hidden, github served.
• TestTailLog_ProfilePinnedToken_HidesServerOutsideProfile — AllowedServers=[*] + ProfilePin=gh: secret hidden, github served.
• TestTailLog_StaleProfilePin_DeniesAll — pin to a removed profile hides both servers (deny-all, no widening).
• TestTailLog_AdminUnchanged — admin context, no context, and an admin with a populated AllowedServers are all served both servers.
• TestTailLog_URLProfileScope_AppliesToAllCallers — /mcp/p/gh scope bounds admin and anonymous callers alike (Spec 057 FR-004).

Gates run locally on ./internal/server/...:

• go build ./cmd/mcpproxy and go build -tags server -o /dev/null ./cmd/mcpproxy — OK
• go test -race -skip 'E2E|Binary|MCPProtocol|TestInfoEndpoint|TestGracefulShutdownNoPanic|TestSocketInfoEndpoint' ./internal/server/... — ok (327s)
• /opt/homebrew/bin/golangci-lint run --config .github/.golangci.yml ./internal/server/... — 0 issues
• Frozen tool-surface goldens under internal/server/testdata pass unregenerated (tool definitions are untouched).

Verification

Live, against two locally built binaries (baseline = origin/main c93f79423; branch = 2d8728cd1), each on a fresh --data-dir + --config, port 18221, personal edition. Two stdio upstreams a and b (./cmd/mcpfixture -transport stdio, quarantine_enabled:false), both connected:true with server-a.log / server-b.log present. Token minted via POST /api/v1/tokens: {"name":"a-only","allowed_servers":["a"],"permissions":["read"]}. On both binaries this token's upstream_servers list returns only ["a"], i.e. b is hidden from it.

Request (after initialize / notifications/initialized, Authorization: Bearer <a-only>):

{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"upstream_servers","arguments":{"operation":"tail_log","name":"b","lines":5}}}

Before (baseline, origin/main) — the leak. tail_log name=b with the a-only token is served:

{"result":{"content":[{"type":"text","text":"{\"connection_status\":{\"connected\":true,...,\"state\":\"Ready\"},\"lines_requested\":5,\"lines_returned\":5,\"log_lines\":[\"... | Disconnecting from server | {\"server\": \"b\", ...}\", ...

Five real log lines of b, plus server_status.enabled/quarantined and live connection_status, for a server the same token's list hides. tail_log name=nonexistent with the same token returns Server 'nonexistent' not found: upstream not found (isError:true) — cmp of the two bodies: different, so served-vs-not-found is an existence oracle.

After (this branch) — refusal is byte-identical to not-found.

tail_log name=b            → {"content":[{"type":"text","text":"Server 'b' not found: upstream not found"}],"isError":true}
tail_log name=nonexistent  → {"content":[{"type":"text","text":"Server 'nonexistent' not found: upstream not found"}],"isError":true}

cmp after normalising the echoed name: identical. Neither body contains log_lines, server_status or connection_status. The hidden-b refusal is also identical (name-normalised) to the baseline's genuine not-found body, so the not-found wording did not change across the fix.

Positive controls (branch). a-only token, tail_log name=a → 5 lines of a, connection_status.state: Ready. Admin X-API-Key, tail_log name=b → 5 lines of b, server_status {enabled:true, quarantined:false}, state: Ready (unchanged from baseline). Wildcard token (allowed_servers:["*"], read), tail_log name=b lines=3 → lines_returned:3, status and state present.

CI round 1 (2d8728cd1) — red, fixed in 1b6bbb68f. Build Binaries linux/amd64 failed 3 of the 5 new tests (ServerRestrictedToken, AdminUnchanged, URLProfileScope) with TempDir RemoveAll cleanup: directory not empty and a lumberjack write error: can't open new logfile: .../server-secret.log — the fixture listed github/secret as enabled in the main server's config (so profile gh would expand), which makes the runtime connect them in a background goroutine that keeps writing server-<name>.log after Shutdown returns. The other matrix jobs (darwin amd64/arm64, windows amd64, linux arm64) were fail-fast cancelled, not failed — darwin/arm64's test step had already passed. Fix (test-only): the two entries are now Enabled: false; EffectiveServers only needs the names, so the profile still expands, the runtime spawns no connection, and the enabled flag a served response reports still comes from proxy.storage. Focused run go test -race -count=10 -run 'TestTailLog_' ./internal/server/ → ok (28s); lint → 0 issues.

Gating in the worktree at 1b6bbb68f: go test -race -count=1 -skip 'E2E|Binary|MCPProtocol|TestInfoEndpoint|TestGracefulShutdownNoPanic|TestSocketInfoEndpoint' ./internal/server/... → ok (225s; internal/server/tokens 5.7s), all five new tests plus the pre-existing TestTailLog_ScrubsLogLines pass; goldens under internal/server/testdata untouched; golangci-lint (v2, .github/.golangci.yml) → 0 issues. Live verification above was run at 2d8728cd1; the follow-up commit touches only the test fixture. Both instances killed by pattern and verified gone; port 18221 free; the tray core on :8080 was never touched.

Caveats: only the AllowedServers dimension was exercised live — profile pin, stale pin and /mcp/p/<slug> URL scope are covered by the unit tests, not by the running instance. ld: warning: ... malformed LC_DYSYMTAB during the -race test build is a macOS toolchain warning, unrelated. /api/v1/status reports version: null for both unstamped local builds (cosmetic).

Cross-model review

Reviewer: opencode CLI (1.18.29), model github-copilot/gpt-6-astra, run in the worktree with the diff, a brief and a copy of Spec 105. Two rounds, one fix round between them.

Round 1 (1b6bbb68f) — FINDINGS, 2.

• R1-1 (P2, internal/server/mcp.go) — applied, comment-only (202ee1957). The reviewer pointed out that profile-scoped administrators lose a previously available capability: handleTailLog applies the profile half of serverInScope to admin callers, so an API-key admin on /mcp/p/gh (or with a session-selected profile) gets tailLogNotFound for an existing server outside the profile, where origin/main served it; TestTailLog_URLProfileScope_AppliesToAllCallers pins this. The finding is genuine as a description defect: the diff's comment claimed "Admin / OS-socket callers carry no restriction and are unaffected", which is only true for unscoped administrators. The comment now states the actual policy — admins skip the AllowedServers check but not the profile check, exactly as list (handleListUpstreams) and call_tool_* (handleCallToolVariant profile gate) already bound every caller on the same surface (Spec 057 FR-004) — and names it as an administrator-visible change on profile-scoped connections. Runtime behaviour deliberately unchanged: exempting admins would make tail_log the only named-server operation on /mcp/p/<slug> that ignores the URL profile, re-splitting the three predicates this PR unifies. Spec 105 FR-007 (line 40) defines effective authorization as including the resolved profile scope for every caller kind and line 113 applies it "to profile-scoped administrators too"; the tension is with SC-005's parity clause, which is a spec-text reconciliation (add profile-scoped tail_log to the SC-005 exception list) — tracked below, not implemented here.
• R1-2 (P1, internal/server/mcp.go) — declined: pre-existing, outside the diff. Name authorization does not prevent cross-server disclosure through shared log files: sanitizeServerLogName maps a/b and a_b to the same server-a_b.log and ReadUpstreamServerLogTail returns the unfiltered last N lines, so an a_b-only token still reads hidden a/b records. Reproduced with a throwaway test in internal/logs (sanitizeServerLogName("a/b") == sanitizeServerLogName("a_b") == "a_b"; deleted, not committed). Both functions are untouched by the PR, which only adds a name-scope gate before the lookup and neither introduces nor widens the shared-log disclosure. The fix needs per-record canonical attribution at write time plus filter-before-limit and an authorized-tail lines_returned (Spec 105 FR-007) — the reviewer itself noted it is the first follow-up below; the no-widening rule keeps it there.

Gates after the fix round (202ee1957): both edition builds OK; go test -race -count=1 -skip '…' ./internal/server/... → ok (241s; tokens 5.5s); golangci-lint v2 → 0 issues; goldens under internal/server/testdata unregenerated and passing; pre-commit hooks passed. Pushed fast-forward from 1b6bbb68f.

Round 2 (202ee1957) — CLEAN, 0 findings. The reviewer re-read the diff against Spec 105 and returned no defects in the change; it listed the remaining FR-007 / SC-005 contract points as spec gaps, which are merged into the checklist below.

Bite check (own verification). With the serverInScope gate deleted from handleTailLog (_, _ = authCtx, profileScope), 4 of the 5 new tests fail (ServerRestrictedToken, ProfilePinnedToken, StaleProfilePin, URLProfileScope); TestTailLog_AdminUnchanged still passes, as a regression guard should. The name-normalised byte comparison is not vacuous: the reference response requires isError and not found from the real storage path, and connection_status is asserted present on served responses.

Own observations not applied (with reasons):

• Session set_profile scope (tier 3 of resolveActiveProfile) is honoured by construction because the gate calls the same resolver, but no test drives a session-selected profile. Left as a follow-up; adding it would widen the test fixture beyond this PR.
• The scope refusal returns before lines parsing and the BBolt read, so it is marginally faster than the genuine not-found path. Both are in-memory fast-fail paths with no file I/O — same timing class under the spec's definition; no timing assertion exists.
• Code comments cite "Spec 104 FR-016h" rather than Spec 105 FR-007. Spec 105 is not on main yet, so the reference stays; re-point it when 105 merges.

Follow-ups / Spec 105 gaps

This PR covers only the authorize-before-lookup + non-disclosing-refusal half of FR-007. The remaining contract points — from the initial audit and both reviewer rounds — are listed, not implemented, here:

• Canonical log-file ownership for colliding names: sanitizeServerLogName maps a/b and a_b (any non [A-Za-z0-9._-] rune) to the same server-a_b.log, so an a_b-only token still reads hidden a/b's lines; no per-record canonical-writer attribution exists (internal/logs/logger.go ~297-308, ~468-498). (Reviewer R1-2, P1 — pre-existing.)
• Uniform policy independent of hidden co-owners, and withholding of legacy unattributed records for scoped callers — the whole file is returned once the name passes scope. Note the PR's own fixture serves an unattributed plain-text canary (mcp_tail_log_scope_test.go ~63-64, ~114), which will have to change when attribution lands.
• Ownership filtering must precede tail limiting (ReadUpstreamServerLogTail tails all records first, logger.go ~495-511, so a foreign interleaved line can displace an authorized one from the returned window) and lines_returned must count the authorized tail — today it is len(logLines) of the raw tail (mcp.go ~5745).
• Fixture for case-only / sanitised name collisions (a/b vs a_b vs case variants), plus interleaved-line fixtures with and without a hidden co-owner (attributable / foreign / legacy records) and the differential hidden-co-owner cases.
• Subject-bound attribution for shared-service producers: OAuth callback shutdown still logs through the manager's last-installed logger rather than the target callback server's recorded logger (internal/oauth/config.go ~1692-1732); interleaved a/b OAuth fixture.
• Canonical container ownership in Docker cleanup: matching is by name prefix (mcpproxy-a- also matches mcpproxy-a-b-…, internal/upstream/core/docker.go ~349-415) and matched containers are logged under the requesting client; foreign container ids must never be written to a's log, legacy foreign records withheld; a vs a-b cleanup fixture.
• Forced-rotation fixture pinning the retained shared rotation/retention effect.
• Test asserting the non-disclosing refusal's timing class (currently argued by construction only).
• Session-selected profile (set_profile) test for tail_log.
• Audit of the other per-server reads FR-007 names ("any per-server operation it may invoke"). Reviewer confirmed no other agent-invokable named operation currently exists in the upstream_servers switch (mutations are denied by AuthorizeServerOp, quarantine ops reject non-admins), so there is no second leak today — but there is no generalized pre-lookup scope gate, and operation/config refusals (read-only mode, DisableManagement, AuthorizeServerOp, mcp.go ~3370-3406, ~3482-3486) still precede name-scope resolution rather than FR-010's scope-first ordering. FR-010 precedence is not demonstrated across them.
• SC-005 administrator parity: profile-scoped administrators (URL /mcp/p/<slug> or session set_profile) are now refused tail_log for servers outside the profile, which is not on Spec 105's SC-005 exception list (reviewer R1-1). Reconcile the spec text — add profile-scoped tail_log to the exception list, consistent with FR-007 line 40 / line 113 — rather than exempting admins in handleTailLog; also add the SC-005 administrator outcomes for the collision / interleaving / container fixtures above.
• Test-strength gaps: no empty-AllowedServers agent-token case; assertTailLogServed checks only the presence of server_status / connection_status, not that the flag values match storage or full pre-feature response parity; no test drives the real HTTP anonymous-auth middleware / transport path (the scope tests call the dispatcher with a constructed context).

Notes

• Branch was fast-forwarded onto origin/main (c93f79423) before commit; no conflicts.
• This branch touches only internal/server/mcp.go (the handleTailLog region) and a new test file. It does not edit internal/server/profile_tool.go, so no merge conflict is expected with the two profile-related branches claude/eager-kirch-10f6c2 and claude/xenodochial-lumiere-5abe75; those two conflict with each other, not with this one.

[cloudflare-workers-and-pages]

Deploying mcpproxy-docs with    Cloudflare Pages

Latest commit:	202ee19
Status:	✅  Deploy successful!
Preview URL:	https://2b8d49ce.mcpproxy-docs.pages.dev
Branch Preview URL:	https://claude-agitated-brattain-c21.mcpproxy-docs.pages.dev

View logs

[codecov-commenter]

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

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

📦 Build Artifacts

Workflow Run: View Run
Branch: claude/agitated-brattain-c21223

Available Artifacts

• archive-darwin-amd64 (29 MB)
• archive-darwin-arm64 (26 MB)
• archive-linux-amd64 (17 MB)
• archive-linux-arm64 (16 MB)
• archive-windows-amd64 (29 MB)
• archive-windows-arm64 (26 MB)
• frontend-dist-pr (0 MB)
• installer-dmg-darwin-amd64 (23 MB)
• installer-dmg-darwin-arm64 (21 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 34229958295 --repo smart-mcp-proxy/mcpproxy-go

Note: Artifacts expire in 14 days.