fix(auth): stale agent-token ProfilePin resolves to deny-all scope on the session path - #997
Merged
Merged
Conversation
… the session path
An agent token can carry a `profile_pin` — a profile the operator binds the
credential to so it can never operate outside that subset of upstream servers.
If the operator later DELETED that profile, the live MCP session resolver
(resolveActiveProfile) warn-skipped the pin and fell through to the next
precedence tier (URL scope, session set_profile, then "none"). "None" means no
profile filtering, so the session silently ran with the token's own — wider —
server scope. Deleting a profile, an act that reads as a restriction, therefore
GREW what the pinned token could see and call, and did so with only a debug-log
trace.
Spec 098 already resolved this for the preflight path: resolvePreflightScope
intersects an unresolvable pin against an empty server set, i.e. deny-all
(internal/server/preflight_glue.go). This makes the session path match, so a
preflight's `not_found` for a stale pin can never be contradicted by a live
session that happily dispatches the same tool.
BEHAVIOR CHANGE: a session whose token names a deleted profile now sees no
upstream tools and can call none, where it previously saw everything in its
token scope. The warn log is kept and now names the deny-all outcome. Recovery
is to re-create the profile or re-mint the token against a live one. This is
the security-correct reading: privilege widening was the bug, not the fix.
Auditing the other surfaces that resolve a profile turned up three more places
where a pin — stale or live — was not enforced at all:
* code_execution read only the URL-injected scope (ProfileScopeFromContext), so
a pinned token on the base /mcp endpoint ran the sandbox under its full token
scope. It now resolves through resolveActiveProfile
(applyProfileScopeToExecution), keeping RestrictToAllowed set so an empty
allow-list denies instead of meaning "allow all".
* direct-routing mode enforced allowed_servers and permissions but no profile
at all, in both tools/list (filterDirectModeToolsForAuth) and dispatch
(makeDirectModeHandler) — a pinned token could discover and call every server
in its token scope. Both now apply the active profile; dispatch emits the
same profile_scope policy decision call_tool_* emits.
* set_profile("") on a pinned token answered with the FULL server list even
though the pin still bounded the session. It now reports the pinned scope
(empty when the pin is stale), so the response cannot advertise a reach the
resolver denies. The tool's own description text is left untouched on
purpose: it is snapshotted by the spec-098 FR-015 tools/list goldens, whose
value is that they were captured from the pre-098 merge base, so an
MCP-surface reword belongs in its own change rather than in this one. The
gap is recorded in a comment on buildSetProfileTool.
retrieve_tools additionally skips index.Manager.ForProfile for a deny-all scope:
that call lazily CREATES an on-disk per-profile index, and a request allowed to
see nothing must not leave one behind for a profile that may not exist.
Tests: resolver unit test for stale-pin deny-all; a session/preflight parity
test asserting both paths answer a deleted pin identically (and that they agree
while the profile still exists); per-surface tests for the sandbox, direct mode
and set_profile, each exercised with the profile present AND deleted so a
surface that ignores profiles cannot pass for the wrong reason.
Docs/spec updated: agent-tokens, profiles and tools-preflight now describe
deny-all and list every surface that resolves the pin; spec 028's warn-skip
answer is amended with the reason it changed.
Related #969
Deploying mcpproxy-docs with
|
| Latest commit: |
d41d657
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://10804bc0.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-stale-profile-pin-deny.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 31913007548 --repo smart-mcp-proxy/mcpproxy-go
|
Dumbris
enabled auto-merge (squash)
August 15, 2026 23:08
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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.
The bug: deleting a profile widened a pinned token's scope
An agent token can carry a
profile_pin— a profile the operator binds the credential to so it can never operate outside that subset of upstream servers.If the operator later deleted that profile, the live MCP session resolver (
resolveActiveProfile,internal/server/profile_resolver.go) warn-skipped the pin and fell through to the next precedence tier — URL scope, sessionset_profile, then "none". "None" means no profile filtering, so the session ran with the token's own, wider server scope.Deleting a profile — an operator action that reads as a restriction — therefore grew what the pinned token could see and call, and did so with only a warning in the log. Privilege widening was the bug; the pin is the restriction and it must fail closed.
The precedent: preflight already got this right
Spec 098 resolved exactly this on the preflight path.
resolvePreflightScope(internal/server/preflight_glue.go) keeps an unresolvable pin's name as a restriction over an empty server set, which intersects to deny-all:This PR makes the session path match, so a preflight's
not_foundfor a stale pin can never be contradicted by a live session that happily dispatches the same tool.Behavior change (CHANGELOG-worthy)
A session whose token names a deleted profile now sees no upstream tools and can call none, where it previously saw everything in its token scope. The warn log is kept and now names the deny-all outcome. Recovery: re-create the profile, or re-mint the token against a live one.
Additional surfaces found while auditing
A pin is only a boundary if every dispatch surface resolves it. Three did not — for stale and live pins:
code_executionsandboxProfileScopeFromContext), so a pinned token on the base/mcpran with its full token scoperesolveActiveProfile(applyProfileScopeToExecution), keepingRestrictToAllowedset so an empty allow-list denies instead of meaning "allow all"allowed_servers+ permissions but no profile at all, in bothtools/listand dispatchfilterDirectModeToolsForAuthandmakeDirectModeHandlerapply the active profile; dispatch emits the sameprofile_scopepolicy decisioncall_tool_*emitsset_profile("")retrieve_toolsadditionally skipsindex.Manager.ForProfilefor a deny-all scope: that call lazily creates an on-disk per-profile index, and a request allowed to see nothing must not leave one behind for a profile that may not exist. (Same reasoning as the deliberateForProfileomission documented inpreflight_glue.go.)ProfileScopeFromContextis now documented as the URL tier only, andresolveActiveProfileis its single production caller.Tests
TestResolveActiveProfile_StalePinDeniesAll— the resolver unit: stale pin → non-nil deny-all scope that keeps the removed profile's name, and neither a session selection nor a URL scope may re-widen it.TestStaleTokenPinDeniesOnBothSessionAndPreflightPaths— session/preflight parity: both paths agree while the profile exists, and both deny after it is deleted.internal/server/profile_pin_enforcement_test.go— per-surface coverage for the sandbox, direct mode andset_profile. Each case runs twice, with the pinned profile present (it narrows) and deleted (it denies), because a surface that ignores profiles entirely would otherwise pass the deny assertion for the wrong reason.No existing test pinned the old fall-through behavior. Docs (
agent-tokens,profiles,tools-preflight) and spec 028's warn-skip Q&A / FR-021 are updated, the spec with an explicit note on why the original answer changed.Deliberately not done
The
set_profiletool description still says clearing goes "back to all servers", which understates the pinned case. That string is snapshotted byte-for-byte by the spec-098 FR-015 tools/list goldens, whose value is that they were captured from the pre-098 merge base — rewording the MCP surface deserves its own change rather than riding a security fix. The gap is recorded in a comment onbuildSetProfileTool.Verification
CI="" go test -race ./internal/server/— pass (full package)CI="" go test ./internal/server/ ./internal/profile/ ./internal/preflight/ ./internal/jsruntime/— passgolangci-lint run --config .github/.golangci.yml ./internal/...— 0 issuesset_profilegaps (all fixed here), round 2 found two LOW items (nil-safety, stale wording — both fixed), round 3 found nothing blocking after the final comment corrections.Related #969