fix(scope): OAuth users are scope-restricted like agent tokens on the direct surface - #1334
Merged
Merged
Conversation
… direct surface (Spec 105 PR G) Several direct-mode gates keyed "is this caller scope-restricted" on authCtx.Type == auth.AuthTypeAgent. A server-edition OAuth "user" context is not an administrator (IsAdmin() is false) but also isn't an agent token, so it fell through to the unrestricted branch: a user token scoped to one server could see and reach every configured server through tools/list, describe_tool, prompts/list+get, and the direct callability filters. Replaces the Type-only check with isScopeRestrictedCaller (authCtx != nil && !authCtx.IsAdmin()), consistent with the existing auth.IsScopedCaller helper and with cache_authz.go's own "a User is caller-bounded exactly like an Agent" rule. Fixed in mcp_direct_scope.go (filterDirectModeToolsForAuth, filterAggregatedPromptsForAuth, promptServerAllowed), mcp_describe_direct.go (directEntryVisibleToSession) and mcp_direct_callability.go (filterDirectToolsForAgentCallability + directEntryCallable, kept in parity per the SC-007 listing/describe invariant). Audited but left unchanged (verified not bugs): cache_authz.go, mcp.go's getAuthMetadata, audit_funnel.go, profile_resolver.go's profilePinFromContext, and internal/httpapi/sse_scope.go — each either already handles AuthTypeUser correctly or checks something other than caller scope (a data-model fact or an audit-metadata field). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Deploying mcpproxy-docs with
|
| Latest commit: |
b667ea4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0bf967a4.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://claude-eager-jackson-badeab.mcpproxy-docs.pages.dev |
…aude/eager-jackson-badeab # Conflicts: # internal/server/mcp_describe_direct.go # internal/server/mcp_direct_scope.go
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 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 35585824309 --repo smart-mcp-proxy/mcpproxy-go
|
… time codex gpt-5.6-sol cross-review (round 1) of PR #1334: the existing TestDescribeDirect_ServerScopeGate_UserType scopes its OAuth user OUT of the target server, so directEntryInScope rejects the tool before directEntryVisibleToSession ever reaches directEntryCallable — it does not exercise that function's own isScopeRestrictedCaller fix. Adds a test that scopes the user INTO the server with a pending tool, proving describe_tool refuses it via callability (with an approved sibling as positive control). Verified non-vacuous: fails against the pre-fix Type == AuthTypeAgent check, passes against isScopeRestrictedCaller. 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
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
Continues Spec 105 scope hardening (PR G). Several direct-mode authorization gates decided "is this caller scope-restricted" with
authCtx.Type == auth.AuthTypeAgent. A server-edition OAuth user context is not an administrator (IsAdmin()isfalse) but also isn't an agent token, so it fell through to the unrestricted branch — a user token scoped to one server could see and reach every configured server via:tools/liston the direct surface (filterDirectModeToolsForAuth)describe_tool's direct-surface id resolution (directEntryVisibleToSession)prompts/listandprompts/get(filterAggregatedPromptsForAuth,promptServerAllowed)filterDirectToolsForAgentCallability,directEntryCallable)Fix
Replaced the
Type-only check withisScopeRestrictedCaller(authCtx) = authCtx != nil && !authCtx.IsAdmin(), matching the existingauth.IsScopedCallerhelper andcache_authz.go's own documented rule that "a User is caller-bounded exactly like an Agent." The listing (filterDirectToolsForAgentCallability) and describe-time (directEntryCallable) callability gates were fixed together to preserve the SC-007 listing/describe parity invariant, cross-checked againstdocs/development/server-edition-multiuser-auth.md(only the admin role is meant to be operator-visible into pending/quarantined tools).Audited, left unchanged (verified not bugs)
cache_authz.go— already has its own correctly-scopedAuthTypeUsercasemcp.go'sgetAuthMetadata— copies agent-only fields into audit metadata, not an access decisionaudit_funnel.go— aswitchwith an explicit case per auth type, not a binary gateprofile_resolver.go'sprofilePinFromContext—ProfilePinis a field only ever set on agent-token contexts (data-model fact, not an authz gate)internal/httpapi/sse_scope.go— the real SSE scope gate (eventVisibleToCaller) already usesauth.IsScopedCaller; the flagged line only selects a token-refresh mechanism, and OAuth session principals are already routed correctly via an earlier branchTest plan
AuthTypeUserfixtures (plusAdminUserContextpositive controls) inmcp_direct_catalog_test.go,mcp_describe_direct_test.go,mcp_direct_callability_test.go,mcp_prompt_scope_test.gogo build ./...andgo build -tags server ./...go vet ./internal/server/... ./internal/httpapi/...go test ./internal/httpapi/... ./internal/auth/...go test -timeout 15m -skip "E2E|Binary|MCPProtocol|TestInfoEndpoint|TestGracefulShutdownNoPanic|TestSocketInfoEndpoint" ./internal/server/...(CI's skip pattern — bare runs hang on known binary-spawning tests, unrelated to this change)go test -raceon the full touched-surface test set (73 tests, no race flags)🤖 Generated with Claude Code