fix(security): authorize aggregated prompts by canonical owner and stop profile enumeration on a deleted pin - #1227
Open
Dumbris wants to merge 3 commits into
Open
fix(security): authorize aggregated prompts by canonical owner and stop profile enumeration on a deleted pin#1227Dumbris wants to merge 3 commits into
Dumbris wants to merge 3 commits into
Conversation
…op profile enumeration on a deleted pin
Two agent-token scope leaks on the HTTP MCP surfaces.
Aggregated prompts (Spec 105 FR-006). filterAggregatedPromptsForAuth
re-parsed each published "server__prompt" display name on its first "__"
to find the owning server, while the registered handler dispatched to the
server captured at publication. With servers "a" and "a__b" both serving
"greeting", "a__b"'s prompt is published as "a__b__greeting", the re-parse
claims owner "a", and an agent token scoped to "a" alone could both list
and fetch a prompt that dispatches to "a__b". The fix records the
canonical owner inside the registered prompt's _meta at publication
("app.mcpproxy/server", stamped by buildAggregatedServerPrompts) and
authorizes list and get against that stamp, read from the same snapshot
mcp-go hands the filter, so owner and prompt can never skew across a
refresh. An upstream prompt with no stamp is dropped for scoped callers
(fail closed). The stamp is stripped from every prompt returned so the
client-visible _meta is exactly what the upstream sent. Every aggregated
prompt handler additionally runs authorizeAggregatedPromptServer against
its OWN server before contacting the upstream, returning mcp-go's exact
unregistered-name wording, so a scope change between the filter and the
handler within one request still cannot dispatch. Upstream prompts are
sorted by qualified name before collision resolution so the display-name
winner no longer depends on map iteration order.
Profile enumeration (Spec 105 FR-004). When the profile an agent token is
pinned to has been deleted, both the profile-URL middleware
(/mcp/p/<slug>, 404 body "available": [...]) and the set_profile tool
("unknown profile 'x' (available: ...)") fell into the generic unknown-slug
branch and enumerated every remaining profile — profiles the resolver
treats as unselectable for that token (a deleted pin is deny-all). Both
now omit the list for pinned callers; unpinned callers and administrators
keep the discovery affordance unchanged.
Tests: TestAggregatedPrompt_ScopeUsesCanonicalOwner (real "a"/"a__b"
upstreams over streamable-http, prompts/list and prompts/get through
HandleMessage, admin parity, no stamp on the wire);
TestBuildAggregatedServerPrompts_HandlerAuthorizesCanonicalServer;
TestFilterAggregatedPromptsForAuth_UnstampedFailsClosed;
TestStripAggregatedPromptServer_PreservesUpstreamMeta; owner-stamp
assertions in the existing buildAggregatedServerPrompts tests;
TestProfile_DeletedPinDoesNotEnumerateProfiles (HTTP, pinned vs admin);
TestHandleSetProfile_DeletedPinDoesNotEnumerateProfiles. The replaced
TestFilterAggregatedPromptsForAuth_KeepsUnparseableName encoded the
leak (keep-on-unparseable) and is superseded by the fail-closed test.
Spec 105 (agent-token scope hardening) is the acceptance contract for
this change; it lives on branch 105-agent-scope-hardening and is not yet
merged. The code comments cite the Spec 104 FR-016g / FR-016b lineage the
fix was built against; Spec 105 FR-006 / FR-004 restate those points.
Frozen tool-surface goldens are untouched.
Deploying mcpproxy-docs with
|
| Latest commit: |
b1a4864
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3775fc9a.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://claude-eager-kirch-10f6c2.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 34234236233 --repo smart-mcp-proxy/mcpproxy-go
|
…dler gate is wired Round-1 review of the aggregated-prompt owner stamp (PR #1227): - stripAggregatedPromptServer rebuilt the client-visible _meta from the stamped copy, so an upstream `_meta: {}` came back absent and an upstream value under our own key was lost — an administrator-visible wire change the code comment denied. The stamp is now a private struct that carries the upstream's own *Meta, and strip hands it back unchanged (nil stays nil, `{}` stays `{}`, progress tokens and foreign values survive). A string an upstream sends under our key is no longer mistaken for a stamp. Table-driven test compares marshalled output against the unstamped prompt for every shape. - TestBuildAggregatedServerPrompts_CollisionKeepsFirst fed the builder an input that was already in sorted order, so the new deterministic sort was never exercised; both input orders now assert the same winner, owner and log fields. - TestAggregatedPrompt_ScopeUsesCanonicalOwner passed with the production authorize hook unwired because mcp-go's filter alone denied the get. It now invokes the registered handler directly (bypassing the filter) and asserts the scoped caller gets the not-found sentinel while an admin is served, and asserts code + message parity between a hidden and a nonexistent prompt on prompts/get.
…tion-time enable_prompts
Cross-review round 2 on the prompt-scope change.
R2-1 (P2, applied): the prompt filter was installed only when enable_prompts
was true at CONSTRUCTION (mcp.go for p.server, initRoutingModeServers for
the routing-mode servers), while RefreshPrompts publishes from the LIVE
snapshot on every servers.changed / config.reloaded / prompts-changed event
and mcp-go registers the prompts capability implicitly on the first
SetPrompts. Boot with prompts off, enable them at runtime, and every
routing-mode server served the aggregated prompts with no scope filter and
with the internal owner stamp serialized as "_meta":{"app.mcpproxy/server":{}}
for every caller. The filter is now bound unconditionally on all four
servers; it is a no-op while nothing is registered. The capability
advertisement stays gated as before. Regression:
TestAggregatedPrompt_LateEnableStillFiltered (fails before, passes after,
on all four servers).
R2-2 (P3, applied): the stamp doc no longer claims SetPrompts publishes and
filters from ONE atomic snapshot; mcp-go clears and re-adds under separate
lock acquisitions. The per-prompt binding is what the guard relies on and
that is what the comment now states.
R2-3 (P3, applied): the "unpinned callers keep the discovery affordance"
half of TestHandleSetProfile_DeletedPinDoesNotEnumerateProfiles ran with no
auth context (administrator-shaped). It now runs with an unpinned agent
identity (ProfilePin "", AllowedServers ["*"]) and, separately, with no
auth context. Verified to bite: a guard that hides the list from every
agent fails the new assertion and passed the old one vacuously.
R2-4 (pre-existing, outside the diff, already a PR follow-up): declined.
Test helper: createTestProxyWithRuntimeCfg adds a pre-construction config
hook; createTestProxyWithRuntime is unchanged in behaviour.
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
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.
Part of Spec 105 (agent-token scope hardening).
Summary
Two agent-token scope leaks on the HTTP MCP surfaces, both fail-closed now.
Aggregated prompts (Spec 105 FR-006).
filterAggregatedPromptsForAuthre-parsed each publishedserver__promptdisplay name on its first__to find the owner, while the registered handler dispatched to the server captured at publication. With serversaanda__bboth servinggreeting,a__b's prompt is published asa__b__greeting; the re-parse claims ownera, so an agent token scoped toaalone could list and fetch a prompt that dispatches toa__b. The fix:buildAggregatedServerPromptsstamps the canonical owner into the registered prompt's_meta(app.mcpproxy/server). The stamp is a private struct carrying the owner and the upstream's original_metapointer, so an upstream-supplied string under that key is never accepted as a stamp. The filter authorizes list and get against that stamp, read from the same registered prompt mcp-go hands it, so owner and prompt cannot skew across a refresh. The stamp is stripped from every prompt returned (admins included) by restoring the upstream_metaverbatim (nil→nil,{}→{}, progress token and foreign keys untouched), so client-visible_metais exactly what the upstream sent.authorizeAggregatedPromptServeragainst its own server beforegetPrompt, returning mcp-go's exact unregistered-name wording (prompt '<name>' not found: prompt not found), so a scope change between the filter and the handler inside one request still cannot dispatch.enable_prompts.RefreshPromptspublishes from the live config snapshot on everyservers.changed/config.reloaded/ prompts-changed event and mcp-go registers the prompts capability implicitly on the firstSetPrompts, so a core started with prompts off and hot-reloaded to on would otherwise serve every upstream prompt unfiltered, with the internal stamp on the wire (cross-review round 2).Profile enumeration on a deleted pin (Spec 105 FR-004). When the profile an agent token is pinned to has been deleted, both the profile-URL middleware (
/mcp/p/<slug>404 with"available": [...]) and theset_profiletool (unknown profile 'x' (available: ...)) fell into the generic unknown-slug branch and enumerated every remaining profile — profiles the resolver treats as unselectable for that token (a deleted pin is deny-all). Both now omit the list for pinned callers; unpinned callers and administrators are unchanged.What changed
internal/server/mcp_routing.gobuildAggregatedServerPromptsgains anauthorizehook consulted by every upstream prompt handler before dispatch; stamps the canonical owner into_metaas a privateaggregatedPromptStamp{server, upstream}; sorts upstream prompts by qualified name (slices.SortStableFunc) before collision resolution. HelpersstampAggregatedPromptServer/aggregatedPromptStampOf/aggregatedPromptServer/stripAggregatedPromptServer(strip restores the upstream_metapointer verbatim).RefreshPromptswiresp.authorizeAggregatedPromptServer;initRoutingModeServersbindsWithPromptFilteron the direct / code / call servers regardless ofEnablePrompts.internal/server/mcp.goNewMCPProxyServerbindsWithPromptFilter(filterAggregatedPromptsForAuth)on the default server unconditionally (was insideif config.EnablePrompts);registerPromptsstays gated.internal/server/mcp_direct_scope.gofilterAggregatedPromptsForAuthauthorizes against the_metastamp instead of re-parsing the display name; unstamped upstream prompts dropped for scoped callers; stamp stripped from all output. NewpromptServerAllowed(single predicate shared by filter and handler gate),authorizeAggregatedPromptServer,errPromptNotFound(=mcpserver.ErrPromptNotFound).internal/server/server.goprofileMiddlewareunknown-slug 404 omitsavailablewhen the request carries a profile pin.internal/server/profile_tool.gohandleSetProfileunknown-slug error omits(available: ...)when the caller is pinned.internal/server/mcp_prompt_scope_test.goTestAggregatedPrompt_ScopeUsesCanonicalOwner(with refusal-shape parity + direct handler invocation),TestAggregatedPrompt_LateEnableStillFiltered,TestFilterAggregatedPromptsForAuth_UnstampedFailsClosed, table-drivenTestStripAggregatedPromptServer_PreservesUpstreamMeta; removed..._KeepsUnparseableName(it encoded the leak).internal/server/mcp_routing_test.goTestBuildAggregatedServerPrompts_HandlerAuthorizesCanonicalServer;_CollisionKeepsFirstruns both input orders.internal/server/mcp_block_tools_test.gocreateTestProxyWithRuntimeCfg—createTestProxyWithRuntimewith a pre-construction config hook (used by the late-enable test).internal/server/profile_integration_test.goTestProfile_DeletedPinDoesNotEnumerateProfiles(HTTP: pinned token gets no list, admin still does).internal/server/profile_tool_test.goTestHandleSetProfile_DeletedPinDoesNotEnumerateProfiles(pinned: no list; unpinned agent token and no-auth admin: list unchanged).No config, API-surface, or tool-definition golden changes.
Tests
TestAggregatedPrompt_ScopeUsesCanonicalOwner— realaanda__bupstreams (streamable-http test servers),prompts/list+prompts/getthroughHandleMessageas ana-only token:a__greetingvisible/fetchable,a__b__greetinghidden and denied; admin sees both; no_metastamp on the wire for either caller. Asserts JSON-RPC code + message parity (modulo the echoed name) between the hiddena__b__greetingget and a nonexistenta__nonexistentget, and invokes the registered handler directly (bypassing mcp-go's filter) requiringerrPromptNotFoundfor thea-only context while the admin context is served.TestAggregatedPrompt_LateEnableStillFiltered— proxy built withenable_prompts=false, both prompt flags flipped in the live snapshot,RefreshPrompts, thenprompts/listas ana-only agent on all four servers: hidden prompt absent, no_metastamp on any entry.TestBuildAggregatedServerPrompts_HandlerAuthorizesCanonicalServer— each handler asks about its canonical server (a__b, nota); a denied handler never callsgetPrompt.TestBuildAggregatedServerPrompts_CollisionKeepsFirst— both input orders yield the same winner/owner/description and the same dropped-collision log fields.TestStripAggregatedPromptServer_PreservesUpstreamMeta— table:nil/{}/ fields / progress token / upstream value under our key, each comparingjson.Marshalof stripped vs as-sent; plus a forged-string stamp case.TestFilterAggregatedPromptsForAuth_UnstampedFailsClosed, stamp assertions inTestBuildAggregatedServerPrompts.TestProfile_DeletedPinDoesNotEnumerateProfiles,TestHandleSetProfile_DeletedPinDoesNotEnumerateProfiles.Bite checks (each restored afterwards): PR-head
stripAggregatedPromptServer→ stripemptyandour_keysubtests fail;SortStableFuncremoved → collisionreversed_orderfails;authorizepassed asnilinRefreshPrompts→ integration wiring assertion fails; filter bound only underEnablePrompts→LateEnableStillFilteredfails on all four servers; a guard hiding the profile list from every agent →profile_tool_test.go:138fails.Gates run locally (CI invocation), green on every commit:
Frozen tool-surface goldens under
internal/server/testdatapass unregenerated.Verification
Live verification on an isolated personal-edition instance (
127.0.0.1:18201, fresh--data-dir+ scratch--config, socket and Web UI disabled). Branchda05678f(the original fix commit; the two review commits below change the_metarestore and the filter binding, neither of which alters the probes shown) vs baselineorigin/main c93f79423, same config on both binaries.Rig. Upstreams
aanda__bare bothnpx -y @modelcontextprotocol/server-everything(stdio; 4 prompts each:args-prompt,completable-prompt,resource-prompt,simple-prompt). Profilesp-doomed→["a"],p-other→["a__b"].enable_prompts: true,aggregate_upstream_prompts: true,quarantine_enabled: false, both serversquarantined: false, bothconnected: truebefore any probe. Tokens minted viaPOST /api/v1/tokens(X-API-Key):tok-a-only(allowed_servers: ["a"], read),tok-wild(["*"]),tok-pinned(["*"],profile_pin: "p-doomed").Request shape —
initialize(protocolVersion 2025-03-26) → captureMcp-Session-Id→ secondPOSTwith the probe method:Scenario 1 —
a-only agent token vs prompts owned bya__borigin/main)tok-a-onlyprompts/lista__b__args-prompt,a__b__completable-prompt,a__b__resource-prompt,a__b__simple-prompta__args-prompt,a__completable-prompt,a__resource-prompt,a__simple-prompt,setup-new-mcp-server,troubleshoot-mcp-server— noa__b__*, no_metatok-a-onlyprompts/get a__b__args-promptresult.messages[0].content.text = "What's weather in Vilnius?"(upstreama__bwas contacted){"error":{"code":-32602,"message":"prompt 'a__b__args-prompt' not found: prompt not found"}}tok-a-onlyprompts/get totally-unknown-prompt(reference)-32602 "prompt 'totally-unknown-prompt' not found: prompt not found"tok-a-onlyprompts/get a__args-prompt(own server)"What's weather in Vilnius?"tok-wildprompts/get a__b__args-prompt(positive control)"What's weather in Vilnius?"tok-wild/ adminprompts/list(positive control)_metaon any promptRefusal-shape parity on the branch (request
idstripped, caller-supplied name placeholdered):An
a__bprompt is indistinguishable from an unregistered name to thea-only caller; the only name in the refusal is the one the caller itself sent.Scenario 2 — pinned token whose pinned profile was deleted
Deletion = remove
p-doomedfromprofiles[]in the scratch config; the watcher hot-reloaded within 2 s (GET /api/v1/profiles→["p-other"]) on both binaries.tok-pinned, pinp-doomed)initializevia/mcp/p/p-doomedafter deletion{"available":["p-other"],"error":"unknown profile 'p-doomed'"}— leak{"error":"unknown profile 'p-doomed'"}tools/call set_profile {"profile":"p-doomed"}on/mcp"unknown profile 'p-doomed' (available: p-other)"— leak"unknown profile 'p-doomed'"set_profile {"profile":"nope-nope"}"agent token is pinned to profile 'p-doomed' and cannot switch to 'nope-nope'"(pin-mismatch check fires first on both)set_profile {"profile":""}{"active_profile":"p-doomed","servers":[]}— deny-all, no server names/mcp/p/p-other(foreign profile)"agent token is pinned to profile 'p-doomed' and cannot access profile 'p-other'"/mcp/p/p-doomedbefore deletion (positive control)prompts/list=a__*×4 + built-insUnpinned admin references are unchanged by design on both binaries:
/mcp/p/nope-nope→ 404 withavailable: ["p-other"];set_profile nope-nope→"unknown profile 'nope-nope' (available: p-other)";set_profile p-other→{"active_profile":"p-other","servers":["a__b"]}.grep -c -E 'p-other|available|a__b'over every branch pinned-token body → 0.Caveats from the run:
prompts/getrefusal does name the requested prompt, but only by echoing the caller's own input; it is byte-identical in shape to the refusal for a never-registered name. Read "without naming it" as "without confirming it exists".set_profileenumeration is only reachable when the pinned token names its own deleted slug; a foreign bad name is short-circuited by the pin-mismatch check on both binaries. The own-slug path is the one shown closed above../cmd/mcpfixtureexposes no prompts, so both upstreams areserver-everything— identical prompt names on both servers, which is exactly the first-__parse trap.availablelist (Spec 105 admin-withholding point, listed under follow-ups).server-everythingchildren;8080/~/.mcpproxyuntouched.Cross-model review
Reviewer: opencode 1.18.29, model
github-copilot/gpt-6-astra. Rounds run: 3 (one attempt each, every round returned a verdict of FINDINGS). Fix rounds: 2 (round 1 →8466860ec, round 2 →b1a4864b5, both pushed); round 3 produced no genuine defect in the diff, so nothing was pushed and the PR stands atb1a4864b5. Every finding was verified against the code before being applied or declined. The reviewer was given Spec 105 (staged under the worktree's uncommitted.review-tmp/) as the acceptance contract.Round 1 — 6 findings (all P3); fixed in
8466860ecmcp_routing.gostripAggregatedPromptServerchanged admin-visible upstream_meta: an upstream_meta: {}(non-nil empty map) went absent on the wire, and an upstream value underapp.mcpproxy/serverwas overwritten at stamp time and deleted at strip time — contradicting the "exactly what the upstream sent" claim.mcp.Meta.MarshalJSONemits{}for a non-nil empty map). Stamp is now a privateaggregatedPromptStamp{server, upstream *mcp.Meta}; strip restoresstamp.upstreamverbatim (exact pre-PR pointer parity); an upstream string under our key is no longer accepted as a stamp.TestStripAggregatedPromptServer_PreservesUpstreamMetarewritten table-driven;emptyandour_keysubtests fail on the PR-head strip.mcp_direct_scope.goenforceis true; admin / API-key callers still list and fetch them; FR-006 + SC-005 require withholding from everyone.buildAggregatedServerPromptsis stamped by construction and the only unstamped prompts are built-ins. Admin withholding is the FR-006/SC-005 contract point already listed as a follow-up; the test atmcp_prompt_scope_test.go:150documents current behaviour for that follow-up to flip.mcp_routing.gostrings.Cut("a:", ":")succeeds, so an empty upstream prompt name is stamped with owneraand registered asa__; FR-006/SC-005 require withholding it from everyone.!okgate onstrings.Cutand the manager'sname + ":" + promptqualification predate the PR; the diff only stamps the (correct) owner onto what the gate admits. Ana-authorized caller listinga__is not a scope leak; withholding empty-name registrations is the first FR-006 follow-up.mcp_direct_scope.go-32603(INTERNAL_ERROR) while the unregistered-name / filter-hidden paths return-32602(INVALID_PARAMS); message identical.handleGetPrompt: anyfinalHandlererror is unconditionally wrapped asINTERNAL_ERROR— no handler-controllable code. Reachable only via a scope change inside one request; already documented as a residual in the code and under follow-ups. The reachable path is now pinned to-32602+ identical message by the new parity assertion.mcp_prompt_scope_test.go,mcp_routing_test.go,profile_tool_test.goUnstampedFailsClosedis vacuous for the explicit!stampedbranch; (b)CollisionKeepsFirstis vacuous for the newSortStableFunc(input already lexically ordered); (c) the "unpinned" comparison inHandleSetProfile_DeletedPin…runs with a nil auth context; (d)ScopeUsesCanonicalOwnerasserts onlyNotNil(denied["error"])and passes with the authorize hook removed.SortStableFuncfailsreversed_order);ScopeUsesCanonicalOwnerasserts code + message parity vs a nonexistent name and invokes the registered handler directly with thea-only context (passingnilforauthorizefails). (a) declined:auth.CanAccessServer("")andProfileScope.Allows("")both return false for every enforcing caller, so the!stampedbranch is redundant defence in depth and no fixture can make it deciding; the test still bites for the contract it names (it fails if the filter re-parses the display name). (c) declined in round 1 (an unpinned-agent comparison would codify the(available: …)enumeration that the FR-003/FR-004 follow-up removes) — then reconsidered and applied in round 2 as R2-3, since the comparison pins the admin/unpinned affordance unchanged by this diff, which is what the PR claims.server.go,profile_tool.goprofilePinFromContext(...) != ""only, so an unpinned restricted token on an unknown slug still receives every profile name and can initialize through a profile disjoint from its grant. Reviewer rated P1 against FR-003/FR-004.Round 2 — 4 findings (1 P2, 3 P3); fixed in
b1a4864b5mcp_routing.go,mcp.goEnablePrompts, butRefreshPromptsgates on the live snapshot and runs on everyconfig.reloaded; mcp-goAddPromptsimplicitly registers the capability. Result: unfilteredprompts/listfor every caller, with"_meta":{"app.mcpproxy/server":{}}on the wire. The filter-less listing hole predates the diff; the diff added the stamp to that path.enable_prompts=falseat construction, both flags flipped live,RefreshPrompts,prompts/listas ana-only agent on all four servers → hidden prompt listed + stamp serialized). Filter now bound unconditionally on all four servers (no-op while nothing is registered; capability advertisement stays gated).TestAggregatedPrompt_LateEnableStillFilteredfails before, passes after; helpercreateTestProxyWithRuntimeCfgadded. The handler-side authorize hook already blockedprompts/geton this path, so the diff never widened fetch.mcp_routing.goSetPromptsclears underpromptsMu, unlocks, thenAddPromptsre-locks per batch, so a concurrent list can observe an empty/partial set. No cross-owner bypass follows (each prompt carries its own owner).server.go:914-919); comment rewritten to state the per-prompt binding the guard relies on and to acknowledge the non-atomic clear/re-add. No behaviour change.profile_tool_test.gosetProfileCtxinstalls no auth context for pin"", so the "unpinned" comparison was administrator-shaped.AuthTypeAgentcontext (ProfilePin "",AllowedServers ["*"]) and separately with no auth context. A mutant guard hiding the list from every agent fails the new assertion and passed the old fixture vacuously.server.gono profiles configured(checked before the pin check), another profile present → 404unknown profile '<pin>'; empty-slug/mcp/pgets 404 vs 403.no profiles configuredordering is identical onmain); the diff only removes the name list. Already the FR-004 status/body-parity follow-up.Round 3 — 2 findings (both P2, both recommended as follow-ups by the reviewer); no code change
server.goprofileMiddleware: once a slug exists and passes the pin check, theProfileScopeis built without intersecting the profile's servers with the agent's allowed servers, so an unpinneda-only token initializes through ab-only (or empty) profile while a nonexistent slug 404s — an existence oracle. Downstream filtering still blocksbcontent.git diff origin/main -- internal/server/server.gohas exactly two hunks (doc comment + the FR-009 404 body); theProfileScopebuild is byte-identical onmainandhandleSetProfileequally lacks the intersection. Reframing of declined R1-6; listed as follow-up per the reviewer's own recommendation.mcp_prompt_scope_test.go__aserver /reviewprompt fixture, no retained-old-registration-across-replacement test, no overlapping-refresh test; collision-owner change is tested at builder level only. Implementation appears correct.__aservingreviewover streamable-http, published byRefreshPrompts, JSON-RPCprompts/list+prompts/getviaHandleMessage: registered as__a__review; listed and fetchable for an unrestricted agent and an admin; withheld from ana-only token on list, on get (-32602, same shape as an absent prompt) and at the registered handler (errPromptNotFound).ParseDirectToolName("__a__review")returnsok=false, somain's "unidentifiable owner → keep" rule would have leaked it; the canonical-owner stamp is what makes it pass. Recorded under follow-ups.Gates per fix round (identical invocation to the Tests section): build both editions OK;
go test -race -count=1 …/internal/server/...ok (227 s / 226 s / 246 s); golangci-lint v2 0 issues; goldens unregenerated (git statusclean underinternal/server/testdataafter each run);gofmt/go vetclean; pre-commit and pre-push hooks passed. Commits made by explicit path only;.review-tmp/never staged.Follow-ups / Spec 105 gaps (not in this PR)
a:→ registereda__,mcp_routing.gostamp path),:xstamping an empty owner, and any unstamped display entry (mcp_direct_scope.go, unreachable in production today but the filter only drops it underenforce). Spec names both as administrator exceptions that must be withheld from everyone. (R1-2, R1-3.)__awith promptreview(unparseable display name__a__review) — listed+fetchable for an unrestricted agent and admin, withheld from ana-only token. Verified by hand in review round 3 (R3-2); the committed test is still missing.SetPromptsreplacement and for interleaved/overlappingRefreshPrompts(edge case "Prompt refresh interleaving") — mcp-go v1.0.0SetPromptsclears under lock thenAddPromptsre-locks per batch, and the four routing-mode servers are updated sequentially, so empty/mixed windows exist (R2-2); plus collision-owner change exercised through authorization + dispatch, not only at builder level (both input orders at builder level and the refusal-shape parity assertion inTestAggregatedPrompt_ScopeUsesCanonicalOwnerlanded in8466860ec). (R3-2.)prompts/list+prompts/getmatrix across/mcp,/mcp/all,/mcp/code,/mcp/call,/mcp/p/<slug>with the differential sentinel fixtures — the committed prompt tests inject the auth context directly intoproxy.server.HandleMessage, andTestAggregatedPrompt_LateEnableStillFilteredcovers list on all four server instances but not get.server.go:2335), "no profiles configured" (distinct 404, evaluated before the pin check atserver.go:2317— so a deleted-pin caller can still tell an empty fleet from a non-empty one, and on empty-slug/mcp/pgets 404 vs 403; R2-4), and "profile exists but not selectable" (200/proceeds); only the deleted-pin case stops enumerating here./mcp/p/<slug>andset_profile; today an unpinned restricted token on an unknown slug still receives the fullavailablelist (server.go:2358,profile_tool.go:98),set_profilestill says(available: ...), and/mcp/p/<slug>initializes through an existing profile whose server set is disjoint from (or empty against) the token's grant while a nonexistent slug 404s — an existence oracle (R1-6, R3-1). Add the required fixture pair: initialize through a selectable AND a non-selectable existing profile URL with an unpinned restricted token (the new deleted-pin test covers only a matching deleted slug with another profile remaining)./mcp/pand/mcp/p/(empty slug) with tests; for an unpinned token the empty slug falls into the enumerating unknown-profile branch.INTERNAL_ERROR(-32603) vsINVALID_PARAMS(-32602) for an unregistered name — reachable only inside the filter→handler window (scope resolved per check, not per request); message identical; no handler-controllable code in mcp-go v1.0.0 (R1-4). The reachable path is pinned to -32602 + message parity byTestAggregatedPrompt_ScopeUsesCanonicalOwner.gpt-6-astraround to completion so the PR carries a real cross-review verdict — done, three rounds with verdicts (see Cross-model review).Notes
claude/eager-kirch-10f6c2(this one) andclaude/xenodochial-lumiere-5abe75both editinternal/server/profile_tool.go(handleSetProfile); expect a merge conflict in that function with whichever lands second.105-agent-scope-hardeningand is not yet onmain; code comments cite the Spec 104 FR-016g / FR-016b lineage the fix was built against, which Spec 105 FR-006 / FR-004 restate.