Conversation
Deploying mcpproxy-docs with
|
| Latest commit: |
4ba12d3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5d117417.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://105-d-profiles.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 35305209675 --repo smart-mcp-proxy/mcpproxy-go
|
Dumbris
force-pushed
the
105-d-profiles
branch
from
September 16, 2026 15:48
431b35b to
bb27556
Compare
…n /mcp/p and set_profile (FR-003/004, FR003-G1..G8) Failing tests for Phase 4 (T035-T040), all RED on HEAD by assertion: - T035: generalise mintPinnedToken → mintAgentToken(t, env, name, allowed, perms, pin); mintPinnedToken stays as a thin wrapper for the pre-105 pin tests (H1 reuses the generalised minter). - T036 (G1/G2/G5): TestProfile_ScopedUnpinnedRefusalUniform — research-srv-only unpinned token: /mcp/p/deploy (HEAD 200), /mcp/p/nonexistent, /mcp/p, /mcp/p/ and deleted /mcp/p/deploy (HEAD 404 + `available`) must share one 404 status+body with no `available`; /mcp/p/research positive control. - T037 (G3/G4): TestProfile_PinnedRefusalUniform (HEAD 403/403/404) and TestProfile_PinnedRefusalIndependentOfFleet (HEAD "no profiles configured" before the gate); anonymous no-profiles branch kept as admin control. - T038 (G6): TestHandleSetProfile_URLScopeGovernsReportedServers — set_profile on a URL-scoped endpoint stores the selection but reports URL profile ∩ token. - T039 (G7): TestSetProfileClearPinnedReportsEmptyActiveProfile — clearing a pinned selection reports active_profile == "". - T040 (G8/D1): TestHandleSetProfile_PinnedZeroReachRefusedLikeDeletedPin (empty/ghost/disjoint pin refused with the deleted-pin body, no mutation) and TestProfile_PinnedZeroReachURLRefusedUniformly (/mcp/p/<zero-reach pin> uniform 404; HEAD 200). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nd set_profile (FR-003/004, FR003-G1..G8, D1)
profileMiddleware (T041): scoped callers (auth.IsScopedCaller — agent tokens
and server-edition users) are admitted through /mcp/p/<slug>, /mcp/p and
/mcp/p/ only when the slug is in selectableProfileNames; every other outcome
— missing, deleted, configured-but-unreachable, empty profile, pin mismatch,
zero-reach pin, slug-less path, empty fleet — is answered by ONE constructor
(profileNotSelectable: 404 {"error":"unknown profile '<slug>'"}, no
`available`, no pin). The no-profiles branch now runs after the gate; the
pre-105 403 pin branch is gone (a pinned agent is always a scoped caller).
Admin / anonymous callers keep the two pre-105 404 branches (SC-005).
handleSetProfile (T042): one selectable check for every non-empty slug;
`servers` is the effective scope after the update via resolveActiveProfile
(pin > URL > session) ∩ token — on a URL-scoped endpoint the URL profile,
not the stored selection; `active_profile` is the stored selection, so a
cleared pinned selection reports "". selectableProfileNames' pin branch now
requires reach (D1): an empty/ghost/disjoint pin is refused with the
deleted-pin body and the session is not mutated.
Docs (T043): docs/features/profiles.md set_profile payload semantics and the
scoped /mcp/p refusal; docs/features/agent-tokens.md pin section (404, not
403; zero-reach pin ≡ deleted pin).
Inverted pinned tests (T044), never deleted: TestProfile_PinnedTokenURLEnforcement
(403+pin text → uniform 404 without pin), TestSetProfileClearReportsPinnedScope
and TestHandleSetProfile_PinnedTokenClearIntersectsAllowedServers (active_profile
"" on clear), TestHandleSetProfile_PinnedTokenSelectsDisjointPin (→ refuses,
no mutation). Admin controls kept: TestProfile_404NoProfiles/404UnknownSlug,
TestHandleSetProfile_AdminUnchanged, the anonymous `available` control in
TestProfile_DeletedPinDoesNotEnumerateProfiles.
Verified: go test -race (CI -skip regex) ./internal/server/... ok;
go test -tags server -race ./internal/serveredition/... ok; both editions build;
gofmt/vet clean on touched files; goldens untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…snapshot resolver, operator log on profile-URL refusals, fail-closed nil-config auth Findings confirmed and fixed (red-test-first; verdict table in .review-tmp/critique-r1.md): - A1/A2 (SC-005): handleSetProfile rendered `servers` from ProfileScope's map-backed set — nondeterministic order, duplicates dropped — for every caller, and applied FR-003's URL-precedence reporting to administrators too. Administrators now short-circuit to the pre-105 payload (selected profile's EffectiveServers / allServerNames on clear, byte-for-byte); scoped callers render the effective scope in profile-declared order. Admin controls now require.Equal exact slices on a 5-server fixture and an admin URL-scope leg is added. - S4/N7: resolveActiveProfileIn(ctx, cfg) / profileScopeForSlugIn(cfg, slug) — the handler admits, stores and reports from ONE config snapshot. - S1: profileMiddleware logs every scoped refusal (agent_name, profile, remote_addr) — the gate answers before the logging handler, so this is the only trace a token probing /mcp/p/<slug> leaves. - N5: mcpAuthMiddleware refuses (503) an agent token when no configuration is published instead of forwarding it with no AuthContext (absent context reads as administrator downstream). - Tests: assertUniformProfileRefusal pins the error text; vacuous NotContains removed; duplicate pin-clear test folded into the inverted original. - Docs: server-edition-user over-claim removed; deleted-pin warning attributed correctly; pin-mismatch message documented. Deferred with rationale: resolver collapse of a zero-reach pin (PR C fixture), pin-naming mismatch text (PR G), client-choosable session id (Spec 058 US3). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e predicate selectableProfileNames returned on the first reachable pin but walked the whole profile slice for a deleted or zero-reach pin. profileMiddleware and handleSetProfile consult it before issuing the uniform refusal, so a pinned caller could tell "pin alive, slug mismatched" from "pin gone / zero reach" by the work its own refusal cost — a timing-class oracle on pin existence (spec Definitions: non-disclosing = status, body AND timing class). The predicate is now forEachProfileSelectable: it visits every configured profile in order, computes reach for each one whenever the caller is scoped or pinned, applies the pin as a filter and never returns early; selectableProfileNames accumulates through it into a pre-sized slice so even the result allocation is constant. Administrator semantics are unchanged (every configured profile, empty and ghost ones included — SC-005); the orphaned profileNames helper is removed. Tests: TestSelectableProfileNames_PinOutcomesDoSameWork (AllocsPerRun parity across reachable / zero-reach / deleted pin on an equal-sized fleet — 2/1/0 before, equal after) and TestForEachProfileSelectable_VisitsEveryProfileRegardlessOfOutcome (traversal counter over eight caller kinds, picks == selectableProfileNames). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The uniform /mcp/p/<slug> refusal computed the whole selectable-profile
list (one reach pass per configured profile) before refusing, so a scoped
token's refusal cost zero iterations over an empty fleet and one per
profile over a populated one — same 404 body, work proportional to the
number of hidden profiles (FR-004 timing class; 1.7 µs vs 0.9 ms at
10 000 profiles through the middleware).
- profileIndex: immutable slug → profile map per config snapshot, cached
by snapshot pointer (profileIndexCache, atomic.Pointer on Server).
- profileIndex.selectable evaluates the selectable rule for the requested
slug (and the pin) only: same lookups and one allocation-free reach
computation (profileHasReach, shared with forEachProfileSelectable) on
every branch — absent, not selectable, deleted pin, zero-reach pin, pin
mismatch, no profiles.
- profileMiddleware → serveProfileURL(w, r, cfg, next): the gate after the
snapshot read, so fleet tests run on a bare Server (a live runtime over
4 097 profiles spends the test building per-profile indexes). set_profile
keeps selectableProfileNames; the URL gate never calls it.
- Tests: refusal allocation parity over 0/1/4 097-profile fleets; lookup-
hook traversal seam (gate resolves ⊆ {slug, pin}, mutation-tested);
wiring through the seam; pure-predicate zero-alloc and list-equivalence;
cache-per-snapshot.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…set_profile refusal, warmed index
Codex round 3 on Spec 105 PR D (FR-003/FR-004, research D1): three findings,
all confirmed by trace and throwaway timing probe, fixed red-test-first.
1. The warmed URL gate still told a deleted pin from an existing zero-reach
one by work: profileHasReach scanned the candidate's declared list for
every configured server (nil for a missing profile, the real list for an
existing one) — 9.3 ms vs 3.2 µs over 4 096 servers. Reach is now
precomputed per profile when the per-snapshot index is built (one bitset
over the configured servers' positions, plus an all-zero placeholder for a
slug the snapshot lacks); profileIndex.reach reads one bit and runs the
credential check unconditionally per configured server, so a missing,
deleted, empty or 4 096-server candidate costs the same (24.7 µs each).
2. set_profile decided the requested slug through the whole selectable list
(one reach computation per configured profile) — 0.3 µs over one profile,
63 µs over 4 097, byte-identical body. It now decides the requested slug
alone through the same index (profileIndexFor → the main Server's cache in
production); a scoped caller's refusal is the list-free
`unknown profile '<slug>'`, administrators keep the pre-105 `available:`
affordance (SC-005). Traversal-counter seam: every scoped refusal over a
1- or 4 097-profile fleet resolves ⊆ {slug, pin}.
3. Prior item: the index was still built lazily by the first request after
startup or a reload (one insertion per hidden profile). Server now warms
it at construction and on every config.saved / config.reloaded /
servers.changed event; the lazy build stays as the fallback for the
event-delivery window.
Tests: TestProfileIndex_ReachIsPrecomputedAtBuild,
TestHandleSetProfile_ScopedRefusalTouchesOnlySlugAndPin,
TestHandleSetProfile_AdminUnknownSlugKeepsAvailableList,
TestProfileIndex_WarmedBeforeFirstRequest; the list-carrying scoped
assertions inverted per SC-007 (never deleted). Docs: profiles.md set_profile
bullet + §404 responses; tasks.md T041/T042; verdict table in
.review-tmp/critique-r1.md "Codex round 3".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…er the fleet profileIndex.reach walked every configured server and ran the credential check on each, so a scoped token's uniform refusal on /mcp/p/<slug> and on set_profile cost one iteration over a one-server fleet and 4 096 over a fleet with 4 095 hidden servers — same 404, same zero allocations (so the allocation-parity guards were blind), 36 ns vs 25 µs: the operator's server population was a timing oracle (FR-004; spec Definitions: non-disclosing = status, body AND timing class; codex review round 4). Reach is now O(|reader grant|): the index precomputes per snapshot a server name → position map, the per-profile membership bitset and a per-profile non-empty flag. A reader that may see every server (administrator, absent context, wildcard entry) does one test — is the candidate's set non-empty; a restricted reader walks its OWN allowed_servers and does one O(1) membership test per entry against the candidate's precomputed set, never returning early, so the count is the grant size whatever the fleet, the candidate (present, absent, empty, 4 098-server) or the outcome. After: 29–31 ns over 1 and 4 096 hidden servers alike. Traversal-counter seam profileIndex.reachHook (one call per membership test). Red tests: TestProfileMiddleware_RefusalReachCostsTheGrantNotTheFleet over the new "4096 hidden servers" fleet shape in profile_url_gate_test.go (the round-2 allocation-parity and lookup-seam tests replay over it too), TestProfileIndex_ReachCostsTheGrantNotTheFleet, and the set_profile leg TestHandleSetProfile_ScopedRefusalReachCostsTheGrantNotTheFleet. Docs: profiles.md (set_profile and profile-URL cost statements), tasks.md T041. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tion; two-slot cache no request can roll back Prior item P (round 3/4 residual): the profile index was warmed from the config EVENT, which configsvc delivers after snapshot.Store, so a request in the publication-to-event window built the fleet-sized index inline — a window a token with server-write permission can open itself. configsvc gains AddPrePublishObserver, a read-only observer list distinct from the #937 admission gate's single SetPrePublishHook slot: observers run inside updateLocked after the gate hook and before snapshot.Store, on the exact *Config that will be published. NewServer registers the index build there; the construction-time warm (the initial snapshot is stored without observers) and the event warm (belt-and-braces) stay. Finding 1: profileIndexCache.For stored whatever snapshot its caller captured, so a stalled request holding an older snapshot evicted the warmed index for the current one and the next request rebuilt 4 096 hidden profiles. The cache is now two-slot: warm (written only by the warm path) and lazy (For's fallback, which never touches warm); the previous warm index is demoted into lazy so a request that captured its snapshot just before a publication still finds it. Tests: configsvc observer ordering (sees the gated config before Current() moves and before subscribers are notified; nil-safe), zero lazy builds across N reloads over a live runtime with an observer-order seam, and the rollback regression. research.md D17 records the decision. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… its snapshot as one pair; observers may register observers Codex round 6 on the Spec 105 PR D profile work (two findings). 1. MUST-FIX: the two-slot index cache still let a request build a fleet-sized profile index inline — a request that captured snapshot A and paused across two publications (warm C, lazy B) rebuilt A on resume, while the empty-fleet equivalent built nothing (FR-004 timing class). Structural fix: the request path never captures a config snapshot of its own. profileIndexCache.Current() hands out the warm index, which carries the config it was built from (idx.cfg); profileMiddleware passes that index to serveProfileURL and handleSetProfile takes profileIndexCurrent() and decides over its cfg — admission, server list and effective scope alike. The previous-warm demotion is gone (a request holds the index it was handed); For stays as the bare-test-server build slot only, never writing the warm slot. Because the pre-publish observer warms the slot before snapshot.Store, a request may hold the index of the gated config about to be published, one publication ahead of runtime.Config() for microseconds — documented as acceptable. 2. SHOULD: runPrePublishObservers held observersMu.RLock across the callbacks under updateMu, so an observer registering another observer deadlocked. The list is now cloned under the read lock and released before observers run; AddPrePublishObserver documents that observers run under the update mutex and must not publish (Update / UpdateIfCurrent / ReloadFromFile re-enter by design). Tests (red first): TestProfileRequests_ServeTheIndexAboutToBePublished (URL gate and set_profile, from inside a publication, admit the profile that exists only in the config being published; zero builds), TestProfileIndexCache_PausedRequestKeepsTheIndexItWasHanded, TestProfileRequests_NeverBuildTheIndexOverARuntime (lazyBuilds seam stays 0 on both caches across every request entry and three reloads), TestService_PrePublishObserverMayRegisterAnObserver. research.md D17 updated: the request path takes the index+snapshot pair; no request builds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s the published snapshot, not the one ahead of it The warm (index, cfg) pair became request-visible the instant the pre-publish observer ran, before configsvc stored the snapshot, so a request could be admitted against the NEXT config while resolveActiveProfile's pin tier — reading runtime.Config() independently downstream — still answered the previous one: admission and the effective scope could split across two snapshots within one request. profileIndexCache now keeps the last two prepared pairs (warm, previous) and Published(cfg) returns whichever matches the caller's own runtime.Config() read, never the unconditional latest one; profileMiddleware and profileIndexCurrent call it instead of Current(). serveProfileURL also injects the admitted pair's cfg into the request context, and resolveActiveProfile reads it in preference to a fresh live read, so a reload landing between admission and a downstream pin resolution can't split the two either. TestProfileRequests_ServeTheIndexAboutToBePublished is rewritten (and renamed) to assert the opposite of its old behaviour; TestProfileMiddleware_InjectsAdmittedSnapshotForDownstreamResolution is new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e tests run The profile-gate fixture returned while the runtime's background initialization was still writing index.bleve under the test's TempDir, so shutdown and the TempDir cleanup raced it (shuffle lane: "directory not empty"; Windows: "file is being used by another process"). Wait for PhaseReady first, as server_logs_missing_file_test already does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ss-wide noise testing.AllocsPerRun counts every malloc in the process, so a goroutine still winding down from an earlier test inflated one case (CI read 48 against 12). Build the index outside the window and take the minimum over a few samples of the predicate alone — noise only ever adds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…te's admitted (index, snapshot) pair, and a pin mismatch refuses like any other non-selectable profile
MUST-FIX 1: handleSetProfile called profileIndexCurrent(), which read
runtime.Config() independently even on a /mcp/p/<slug> request whose
gate had already admitted a specific (index, cfg) pair and injected
only cfg into the context. A reload landing between admission and the
set_profile call could hand set_profile a different snapshot than the
one the URL gate used for the same request. serveProfileURL now
injects the whole admitted *profileIndex pair
(withProfileRequestIndex); profileIndexCurrent(ctx) and
resolveActiveProfile both prefer it outright — no build, no
Published()/runtime.Config() read — falling back to the existing
Published/For chain only for the base /mcp endpoint, which admits no
snapshot of its own.
MUST-FIX 2: a profile-pinned token asking for a different, otherwise
selectable profile was refused by an early branch
("agent token is pinned to profile '<pin>' and cannot switch to
'<slug>'") before any index lookup, distinguishable in body and work
from the uniform "unknown profile '<slug>'" a deleted, zero-reach or
disjoint profile receives. That branch let a caller confirm from the
wording alone that it is pinned, and to what. It is removed; a pin
mismatch now falls through to profileIndex.selectable, the same
predicate and body as every other non-selectable outcome
(contracts/refusals.md's `set_profile <not selectable>` row already
documented this as one format string).
Tests: TestProfileMiddleware_SetProfileDecidesWithTheAdmittedIndexNotAFreshRead
(new — a profile present only at admission stays selectable after a
reload removes it live, and a profile present only in the reload stays
refused, both within the one admitted request);
TestHandleSetProfile_PinnedRejectsOtherSlug and
TestHandleSetProfile_ScopedRefusalTouchesOnlySlugAndPin updated for the
uniform body; TestHandleSetProfile_ScopedRefusalReachCostsTheGrantNotTheFleet
extended with the pin-mismatch case. docs/features/profiles.md and
specs/105-agent-scope-hardening/research.md (D1, D17) updated to match.
Verification: gofmt clean; go build (both editions, -o /dev/null);
GOOS=linux/windows go vet ./internal/server/...; go test -race
-count=1 on ./internal/server/... (standard -skip regex, 400s),
./internal/runtime/... (5 packages) and
-tags server ./internal/serveredition/... (7 packages) — all green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…mically, never a stale read-then-match profileMiddleware and set_profile's base-/mcp path (profileIndexCurrent) each read runtime.Config()/currentConfig() once and only then called Published(cfg) against that frozen read, falling back to For(cfg) on a miss. A request paused across two publications between those two steps missed both the warm and previous prepared pairs and fell back to a fleet-sized inline build — the same timing-class disclosure D17 exists to close, just relocated from the admission race (rounds 7-9) to the acquisition race between a request's own config read and the cache lookup. profileIndexCache.Acquire(readCfg) closes it structurally: it loops a bounded number of times (8), re-reading readCfg() and matching it via Published on every iteration instead of freezing one read that may already be stale by the time it is used. Each iteration is O(1), so even a publication storm costs O(retries), never a build. A genuine miss (nil) is left for the caller: serveProfileURL and profileIndexCurrent refuse a scoped caller uniformly and O(1) on nil (no lookup, no disclosure, no build) and fall back to a fresh For build for an administrator-shaped caller, whose timing is not contract-bound (SC-005). For/Current remain only the bare-test-server fallback and the caches' own warm-path mechanics. research.md D17 extended with the round-11 acquisition rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ost finding with a file:line trace and measurement The round-13 reviewer finding claimed a write-capable agent's config mutation (upstream_servers add) builds the profile index synchronously inside the pre-publish observer, making its latency scale with hidden profiles/servers — a D17/SC-005 non-disclosing-refusal concern. Traced instead: Runtime.applyConfigLocked already runs DetectConfigChanges (walks and JSON-diffs the whole Servers slice, twice, sometimes three times), config.SaveConfig (marshals the entire config to disk) and upstreamManager.SetGlobalConfig (walks every live client) before configSvc.Update ever runs the pre-publish observer that builds the profile index. The config-publishing path was already O(fleet) end to end; the observer adds a bounded constant factor to a timing class that predates this PR. Measured at 4096 profiles/servers: newProfileIndex (the observer) = 437 µs/op vs SaveConfig = 19.42 ms/op and one DetectConfigChanges pass = 9.81 ms/op (called 2-3x per apply) — well under 1.1% of the pre-existing work, so the maintainer's move-the-bitset-out-of-the- observer branch does not fire. Also confirmed the profiles×servers bitset is not a droppable dead artifact: idx.reach reads it via an O(1) bit test on every scoped call, the mechanism that keeps profile selectability O(reader grant) rather than O(fleet) (D1) — so it stays. No production code changed. research.md D17 records the refutation verbatim; .review-tmp/critique-r1.md carries the full file:line trace and benchmark; .review-tmp/pr-d-body.md carries the same "Retained effects" text for the eventual PR body. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ex, never a fleet walk (Spec 105 PR D review round 14 — MUST-FIX) After O(1) admission, successful scoped reads still materialized their effective server set by rebuilding a fleet-sized "known servers" set on every call: /mcp/p/<slug>, resolveActiveProfileIn's pin/session tiers (every scoped retrieve_tools/describe_tool/call_tool_*/code_execution call under a pinned or session profile), and set_profile's success payload (including the cleared-session case, which additionally walked every configured server a second time to apply the credential). profileIndex.EffectiveServersFor(profileName, allowed) intersects the reader's own grant against the index's precomputed serverPos/members data instead: O(len(allowed)) for a restricted grant, never O(len(cfg. Servers)). All four cited call sites now use it (or its int-keyed sibling effectiveServersForCandidate, reusing an admission lookup the gate already paid for); administrators keep the unchanged, fleet-proportional path (SC-005). scopeServersIn/callerVisibleServers are now dead and removed. profileIndex.position also gained a bounds check against idx.cfg. Profiles' CURRENT length: two existing tests mutate *config.Config in place after an index is built/cached, which the new EffectiveServersFor call sites exposed as an out-of-range panic on a stale cached position. The check is a no-op for any snapshot that is never mutated in place. research.md D17 records the round; golangci-lint (stuck on Go 1.25 since round 6) upgraded to 2.13.2 so the v2 lint pass could actually run locally. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Dumbris
force-pushed
the
105-d-profiles
branch
from
September 17, 2026 19:12
edbdd23 to
c8d41d9
Compare
…air (Spec 105 PR D review round 15 — MUST-FIX) resolveActiveProfile extracted only injected.cfg from the context-carried (index, snapshot) pair and handed it to resolveActiveProfileIn, which resolved the index a SECOND, independent time via profileIndexFor(cfg) — an O(1) Published(cfg) match that falls back to a fleet-sized For(cfg) rebuild on a miss. A request admitted with pair A that then paused while two more publications landed would find A evicted from both the warm and previous cache slots by the time resolveActiveProfile ran, rebuilding the whole fleet inline for what must stay an O(1) decision — exactly the pair-acquisition bypass rounds 11/13 closed on the admission path (profileIndexCurrent/Acquire), reopened here on the downstream resolution path a paused request reaches next. resolveActiveProfile now calls resolveActiveProfileFromIndex directly with the already-resolved pair when profileRequestIndexFromContext succeeds, bypassing resolveActiveProfileIn/profileIndexFor(cfg) entirely on that path; the profileIndexFor(cfg) route is reached only for the no-injected-pair case (plain /mcp, no URL admission), unchanged. A second round-12 finding argued resolveActiveProfileIn's pin/session tiers should intersect the profile's declared membership with the caller's own AllowedServers instead of a hardcoded wildcard, to bound the per-call cost by the caller's grant rather than the profile's (possibly much larger) declared population. Refuted: round 14 already tried and reverted exactly this change (recorded in its own research.md D17 entry), and reproducing it this round breaks the same tested contract (TestResolveActiveProfile_PinHighestPrecedence, a pin context with no AllowedServers asserting full pin-profile reach) — the reproduction and the full downstream-consumer trace are recorded in research.md D17 and .review-tmp/critique-r1.md under "Review round 15"; no production code changed for that finding. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…wn grant with its profile (Spec 105 PR D review round 17 — MUST-FIX)
cacheAuthorizationWith stamped cache.Authorization.ProfileServers from
the resolver's wildcard-derived FULL profile membership, unintersected
with the caller's own AllowedServers. CouldHaveProduced/coversAll then
compares that stamped set on redemption, so a profile member entirely
outside an agent token's own grant — a server it never had, and never
will have, access to — entered the comparison: removing that hidden
member later revoked the SAME token's cache access to its own,
still-authorized server, turning the token's own cache hit/miss into
an observable side-channel for an unrelated server's continued
existence in the profile (SC-005-class disclosure through the cache
layer; round 16's trace of a consumer round 15's refutation did not
reach).
Fix: a new resolveActiveProfileWithIndex seam returns the (index,
snapshot) pair a (name, scope) pair was resolved against, so
cacheAuthorizationWith can stamp an agent caller's ProfileServers via
idx.EffectiveServersFor(profileName, ac.AllowedServers) — the same
O(len(caller-grant)) helper handleSetProfile's own scoped-visible path
already uses — instead of re-deriving a possibly different snapshot's
index or walking the profile's declared size. Non-agent scoped callers
(no AllowedServers of their own) keep the resolver's full-membership
semantic; resolveActiveProfileIn/resolveActiveProfileFromIndex's own
ProfileScope construction is untouched, per round 15's standing
verdict.
Red tests first: a reproduction of the reviewer's exact scenario
(token grant {github}, profile {github,weather}, narrowed to {github}
after caching — access must NOT be revoked) and a companion (the
token's own grant server removed from the profile — access must still
be revoked).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
Spec 105 (agent-token scope hardening), planning artifacts and PR A merged in #1279. This PR closes FR003-G1..G8 · roadmap
scope-selectable-profile-predicate· research D1. Gap ids, evidence and test sketches:specs/105-agent-scope-hardening/gap-map.md§1; the refusal shapes it must satisfy:contracts/refusals.md.Commits
c03e5685bfix(scope): PR D critique round 1 — admin set_profile parity, single-snapshot resolver, operator log on profile-URL refusals, fail-closed nil-config authc18eef4e3feat(scope): Spec 105 PR D — selectable-profile predicate on /mcp/p and set_profile (FR-003/004, FR003-G1..G8, D1)553d6f0c3test(scope): Spec 105 PR D red phase — selectable-profile predicate on /mcp/p and set_profile (FR-003/004, FR003-G1..G8)Tests
Red-first per gap id (each confirmed failing on
origin/mainby assertion), pinned pre-105 tests inverted (never deleted), admin controls kept. Full list in the red-phase commit message and the test names above.Verification
go buildboth editions ·go test -raceon the touched packages,internal/server(CI skip regex),-tags serverserveredition · goldens byte-identical to main · gofmt/vet ·./scripts/test-api-e2e.sh65/65.golangci-lintv2 could not run locally (binary built with go1.25 vs the go 1.26 target) — CI covers it..review-tmp/critique-r1.md.gpt-5.6-solrounds run after this draft opens (Copilot quota for opencode astra is exhausted until 2026-10-01); the PR leaves draft when the reviewer returns CLEAN.Follow-ups / Spec 105 gaps
See
gap-map.md§8 (REST replay has no tool gate; UI active profile bounds no MCP session). Remaining Spec 105 PRs: C → F → G → H1 after this batch.🤖 Generated with Claude Code