From cdd51c84c7c20fd98a5b16df4e54d82136005a22 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 21 Sep 2026 10:18:05 +0300 Subject: [PATCH 1/2] fix(scope): OAuth users are scope-restricted like agent tokens on the direct surface (Spec 105 PR G) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/server/mcp_describe_direct.go | 4 +- internal/server/mcp_describe_direct_test.go | 22 ++++++ internal/server/mcp_direct_callability.go | 32 +++++--- .../server/mcp_direct_callability_test.go | 47 ++++++++++++ internal/server/mcp_direct_catalog_test.go | 47 ++++++++++++ internal/server/mcp_direct_scope.go | 73 ++++++++++++------- internal/server/mcp_prompt_scope_test.go | 21 ++++++ 7 files changed, 205 insertions(+), 41 deletions(-) diff --git a/internal/server/mcp_describe_direct.go b/internal/server/mcp_describe_direct.go index 6124e6704..3f13db5db 100644 --- a/internal/server/mcp_describe_direct.go +++ b/internal/server/mcp_describe_direct.go @@ -95,9 +95,9 @@ func (p *MCPProxyServer) resolveDirectDescribeIDIn(ctx context.Context, cat *dir func (p *MCPProxyServer) directEntryVisibleToSession(ctx context.Context, entry *directCatalogEntry) bool { authCtx := auth.AuthContextFromContext(ctx) _, profileScope := p.resolveActiveProfile(ctx) - isScopedAgent := authCtx != nil && authCtx.Type == auth.AuthTypeAgent + isScopeRestricted := isScopeRestrictedCaller(authCtx) - if !directEntryInScope(authCtx, profileScope, isScopedAgent, entry) { + if !directEntryInScope(authCtx, profileScope, isScopeRestricted, entry) { return false } return p.directEntryCallable(authCtx, entry) diff --git a/internal/server/mcp_describe_direct_test.go b/internal/server/mcp_describe_direct_test.go index e05649d1c..90e9616b0 100644 --- a/internal/server/mcp_describe_direct_test.go +++ b/internal/server/mcp_describe_direct_test.go @@ -299,6 +299,28 @@ func TestDescribeDirect_ServerScopeGate(t *testing.T) { assert.Equal(t, describeErrNotFound, byID["github__read_file"]["error"]) } +// TestDescribeDirect_ServerScopeGate_UserType is TestDescribeDirect_ServerScopeGate's +// Spec 105 PR G regression: describe's isScopeRestrictedCaller gate must +// scope-check a server-edition OAuth "user" exactly like an agent token, not +// wave it through as an unrestricted (admin-like) caller — see +// directEntryVisibleToSession, which must stay in parity with the listing +// gate this same fix landed in (mcp_direct_scope.go). +func TestDescribeDirect_ServerScopeGate_UserType(t *testing.T) { + p := newDirectDescribeProxy(t) + + elsewhere := auth.WithAuthContext(context.Background(), &auth.AuthContext{ + Type: auth.AuthTypeUser, + UserID: "u-other", + AllowedServers: []string{"gitlab"}, + }) + + resp := callDescribeDirect(t, p, elsewhere, []interface{}{"github__read_file"}) + assert.Empty(t, resp.Definitions) + byID := describeErrorsByID(resp) + require.Contains(t, byID, "github__read_file") + assert.Equal(t, describeErrNotFound, byID["github__read_file"]["error"]) +} + // T044: catalog divergence. A tool that is pending approval is still LISTED for // a non-agent session, so it must still describe — from the catalog snapshot. // An index-backed resolver answers not_found here, which would make deferral diff --git a/internal/server/mcp_direct_callability.go b/internal/server/mcp_direct_callability.go index f94a09ced..4ecdd36ad 100644 --- a/internal/server/mcp_direct_callability.go +++ b/internal/server/mcp_direct_callability.go @@ -42,17 +42,22 @@ func newDirectCallabilityEvaluator(proxy *MCPProxyServer) *directCallabilityEval } } -// filterDirectToolsForAgentCallability hides direct-mode tools that an agent -// token cannot actually invoke because they are disabled, quarantined, pending -// approval, or changed since approval. Non-agent contexts keep the existing -// operator-visible discovery behavior. +// filterDirectToolsForAgentCallability hides direct-mode tools that a +// scope-restricted caller (an agent token or an OAuth-authenticated user — +// see isScopeRestrictedCaller) cannot actually invoke because they are +// disabled, quarantined, pending approval, or changed since approval. +// Administrator contexts keep the existing operator-visible discovery +// behavior: server-edition-multiuser-auth.md documents the admin role as the +// one that "sees all activity, manages users" and reviews pending/quarantined +// tools, which a plain "user" account is not (it is caller-bounded exactly +// like an agent token — see cache_authz.go's CallerKindUser case). func (p *MCPProxyServer) filterDirectToolsForAgentCallability(ctx context.Context, tools []mcp.Tool) []mcp.Tool { if len(tools) == 0 { return tools } authCtx := auth.AuthContextFromContext(ctx) - if authCtx == nil || authCtx.Type != auth.AuthTypeAgent { + if !isScopeRestrictedCaller(authCtx) { return tools } @@ -107,19 +112,22 @@ func (p *MCPProxyServer) filterDirectToolsForAgentCallability(ctx context.Contex return filtered } -// directEntryCallable is the agent-callability half of the direct listing gate, +// directEntryCallable is the callability half of the direct listing gate, // for callers that already hold a resolved catalog entry (Spec 102 US2). // -// Non-agent sessions are unfiltered here, exactly as the loop above leaves them: -// the direct listing deliberately RETAINS tool-level pending/changed/disabled -// states for an operator, and describe_tool must therefore keep describing them -// — a listed tool is never undescribable (SC-007). Only agent tokens, which -// cannot see those tools in their own listing, are gated. +// Administrator sessions are unfiltered here, exactly as the loop above +// leaves them: the direct listing deliberately RETAINS tool-level +// pending/changed/disabled states for an operator, and describe_tool must +// therefore keep describing them — a listed tool is never undescribable +// (SC-007). Only scope-restricted callers (agent tokens and OAuth +// users — see isScopeRestrictedCaller), which cannot see those tools in +// their own listing, are gated. This MUST stay in parity with +// filterDirectToolsForAgentCallability above — the same SC-007 invariant. func (p *MCPProxyServer) directEntryCallable(authCtx *auth.AuthContext, entry *directCatalogEntry) bool { if entry == nil { return false } - if authCtx == nil || authCtx.Type != auth.AuthTypeAgent { + if !isScopeRestrictedCaller(authCtx) { return true } return newDirectCallabilityEvaluator(p).evaluate(entry.ServerName, entry.ToolName).callable diff --git a/internal/server/mcp_direct_callability_test.go b/internal/server/mcp_direct_callability_test.go index 993d79d3c..572184d6f 100644 --- a/internal/server/mcp_direct_callability_test.go +++ b/internal/server/mcp_direct_callability_test.go @@ -224,6 +224,53 @@ func TestFilterDirectToolsForAgentCallability_AgentOnly(t *testing.T) { assert.Equal(t, tools, proxy.filterDirectToolsForAgentCallability(context.Background(), tools)) } +// TestFilterDirectToolsForAgentCallability_UserTypeIsScopeRestrictedToo is the +// Spec 105 PR G regression for this gate: it used to key on +// `authCtx.Type == auth.AuthTypeAgent`, which let a server-edition OAuth +// "user" context fall through to the operator-visible branch (unfiltered, +// same as admin) and see pending/disabled tools it cannot actually call. A +// "user" is not an administrator — server-edition-multiuser-auth.md reserves +// "sees all activity, manages users" for the admin role — so it must be +// gated exactly like an agent token here too (isScopeRestrictedCaller). +func TestFilterDirectToolsForAgentCallability_UserTypeIsScopeRestrictedToo(t *testing.T) { + proxy := createTestMCPProxyServer(t) + require.NoError(t, proxy.storage.SaveUpstreamServer(&config.ServerConfig{Name: "github", Enabled: true})) + require.NoError(t, proxy.storage.SaveToolApproval(&storage.ToolApprovalRecord{ + ServerName: "github", + ToolName: "allowed", + Status: storage.ToolApprovalStatusApproved, + })) + require.NoError(t, proxy.storage.SaveToolApproval(&storage.ToolApprovalRecord{ + ServerName: "github", + ToolName: "pending", + Status: storage.ToolApprovalStatusPending, + })) + + tools := []mcp.Tool{ + {Name: FormatDirectToolName("github", "allowed")}, + {Name: FormatDirectToolName("github", "pending")}, + } + publishPermsCatalog(proxy, map[string]string{ + FormatDirectToolName("github", "allowed"): auth.PermRead, + FormatDirectToolName("github", "pending"): auth.PermRead, + }) + + userCtx := auth.WithAuthContext(context.Background(), &auth.AuthContext{ + Type: auth.AuthTypeUser, + UserID: "u1", + AllowedServers: []string{"github"}, + }) + + filtered := proxy.filterDirectToolsForAgentCallability(userCtx, tools) + assert.Equal(t, []string{FormatDirectToolName("github", "allowed")}, directCallabilityToolNamesForTest(filtered), + "a scoped OAuth user must not see a tool pending approval, same as an equivalently-scoped agent token") + + // Positive control: an OAuth admin_user keeps the operator-visible view. + adminUserCtx := auth.WithAuthContext(context.Background(), auth.AdminUserContext("a1", "admin@example.com", "Admin", "google")) + assert.Equal(t, tools, proxy.filterDirectToolsForAgentCallability(adminUserCtx, tools), + "an OAuth admin user keeps the operator-visible discovery behavior, like api-key admin") +} + func directCallabilityToolNamesForTest(tools []mcp.Tool) []string { names := make([]string, 0, len(tools)) for _, tool := range tools { diff --git a/internal/server/mcp_direct_catalog_test.go b/internal/server/mcp_direct_catalog_test.go index 203599112..d00cf8aed 100644 --- a/internal/server/mcp_direct_catalog_test.go +++ b/internal/server/mcp_direct_catalog_test.go @@ -259,3 +259,50 @@ func TestFilterDirectModeToolsForAuth_EmptyToolNameIsScopeChecked(t *testing.T) assert.Containsf(t, names, "describe_tool", "%s: real built-ins stay visible", name) } } + +// TestFilterDirectModeToolsForAuth_UserTypeIsScopeRestricted is the Spec 105 +// PR G regression: filterDirectModeToolsForAuth used to key its scope check +// on `authCtx.Type == auth.AuthTypeAgent`, so a server-edition OAuth "user" +// context — which is NOT an administrator (AuthContext.IsAdmin() is false for +// it) but also isn't an agent token — fell through to the unrestricted +// branch and saw every configured server's tools regardless of its own +// AllowedServers. isScopeRestrictedCaller (keyed on IsAdmin()) closes that: +// a "user" token scoped to "we" must not see "hostile"'s tools, exactly like +// an equivalently-scoped agent token. +func TestFilterDirectModeToolsForAuth_UserTypeIsScopeRestricted(t *testing.T) { + tools := []*config.ToolMetadata{ + {ServerName: "hostile", Name: "steal_secrets", Description: "Should stay hidden", ParamsJSON: `{"type":"object"}`, Hash: "h-steal", + Annotations: &config.ToolAnnotations{ReadOnlyHint: boolPtr(true)}}, + {ServerName: "we", Name: "solo", Description: "Fine", ParamsJSON: `{"type":"object"}`, Hash: "h-solo", + Annotations: &config.ToolAnnotations{ReadOnlyHint: boolPtr(true)}}, + } + p := &MCPProxyServer{} + p.publishDirectCatalog(buildDirectCatalog(tools, nil)) + + rawTools := []mcp.Tool{{Name: "hostile__steal_secrets"}, {Name: "we__solo"}} + + scopedUser := auth.WithAuthContext(context.Background(), &auth.AuthContext{ + Type: auth.AuthTypeUser, + UserID: "u1", + AllowedServers: []string{"we"}, + Permissions: []string{auth.PermRead}, + }) + + filtered := p.filterDirectModeToolsForAuth(scopedUser, rawTools) + names := make([]string, 0, len(filtered)) + for _, tool := range filtered { + names = append(names, tool.Name) + } + assert.NotContains(t, names, "hostile__steal_secrets", "a user token scoped to \"we\" must not see \"hostile\"'s tools") + assert.Contains(t, names, "we__solo", "the user token's own server stays visible") + + // Positive control: an admin_user (the real "operator" identity in the + // server edition) is unrestricted, same as api-key admin. + adminUser := auth.WithAuthContext(context.Background(), auth.AdminUserContext("a1", "admin@example.com", "Admin", "google")) + filtered = p.filterDirectModeToolsForAuth(adminUser, rawTools) + names = names[:0] + for _, tool := range filtered { + names = append(names, tool.Name) + } + assert.ElementsMatch(t, []string{"hostile__steal_secrets", "we__solo"}, names, "an OAuth admin user must see every server, like api-key admin") +} diff --git a/internal/server/mcp_direct_scope.go b/internal/server/mcp_direct_scope.go index 239db6615..4c2af61fb 100644 --- a/internal/server/mcp_direct_scope.go +++ b/internal/server/mcp_direct_scope.go @@ -118,6 +118,23 @@ func stripDirectToolStampFilter(_ context.Context, tools []mcp.Tool) []mcp.Tool return out } +// isScopeRestrictedCaller reports whether authCtx must have its server access +// checked against its own AllowedServers/Permissions, rather than getting the +// unrestricted view an administrator gets. +// +// This is deliberately keyed on IsAdmin(), not on Type == AuthTypeAgent: a +// server-edition OAuth "user" is a real, scoped identity too (Role: "user", +// its own AllowedServers via Spec 107 PR-C group grants — see +// cache_authz.go's CallerKindUser case, which documents the same "caller- +// bounded exactly like an Agent" rule for the cache read/write gate). A +// Type == AuthTypeAgent check alone treats a "user" context as unrestricted +// the way only "admin"/"admin_user" should be — a scoped user token would +// see and reach every configured server through this surface, not only the +// ones it was granted. +func isScopeRestrictedCaller(authCtx *auth.AuthContext) bool { + return authCtx != nil && !authCtx.IsAdmin() +} + // directIdentityInScope is directEntryInScope's identity-only twin: the same // scope+tier predicate, evaluated against a bare (owner, tier) pair rather // than a *directCatalogEntry, so a caller resolving through a stamp (Spec 105 @@ -125,13 +142,13 @@ func stripDirectToolStampFilter(_ context.Context, tools []mcp.Tool) []mcp.Tool func directIdentityInScope( authCtx *auth.AuthContext, profileScope *profile.ProfileScope, - isScopedAgent bool, + isScopeRestricted bool, owner, tier string, ) bool { if !profileScope.Allows(owner) { return false } - if !isScopedAgent { + if !isScopeRestricted { return true } if !authCtx.CanAccessServer(owner) { @@ -152,14 +169,15 @@ func requiredPermissionForDirectTool(annotations *config.ToolAnnotations) string return contracts.ToolVariantToOperationType[contracts.DeriveCallWith(annotations)] } -// filterDirectModeToolsForAuth filters tools/list for scoped agent tokens and -// for any request with an active profile. +// filterDirectModeToolsForAuth filters tools/list for scope-restricted +// callers (agent tokens and OAuth-authenticated users — see +// isScopeRestrictedCaller) and for any request with an active profile. // // Direct mode registers upstream tools globally as server__tool. Without this -// filter, scoped agent tokens prevent execution but still disclose tool names, -// descriptions, and schemas for servers outside their scope. Call-time auth is -// still authoritative; this filter only removes tools that the current token -// could not call from discovery responses. +// filter, a scope-restricted caller's server access prevents execution but +// still discloses tool names, descriptions, and schemas for servers outside +// its scope. Call-time auth is still authoritative; this filter only removes +// tools that the current caller could not call from discovery responses. // // The profile filter (Spec 057) is applied to EVERY auth type, not just agent // tokens: an unauthenticated /mcp/p/ connection runs as an admin context @@ -174,7 +192,7 @@ func (p *MCPProxyServer) filterDirectModeToolsForAuth(ctx context.Context, tools authCtx := auth.AuthContextFromContext(ctx) _, profileScope := p.resolveActiveProfile(ctx) - isScopedAgent := authCtx != nil && authCtx.Type == auth.AuthTypeAgent + isScopeRestricted := isScopeRestrictedCaller(authCtx) // Spec 105 FR-008 (FR008-G7): a tool with no registration identity is // withheld from EVERY caller, administrators included, so this filter can @@ -193,7 +211,7 @@ func (p *MCPProxyServer) filterDirectModeToolsForAuth(ctx context.Context, tools // empty raw name, so a stamp should never carry one. continue } - if !directIdentityInScope(authCtx, profileScope, isScopedAgent, stamp.owner, stamp.tier) { + if !directIdentityInScope(authCtx, profileScope, isScopeRestricted, stamp.owner, stamp.tier) { continue } filtered = append(filtered, tool) @@ -247,7 +265,7 @@ func (p *MCPProxyServer) filterDirectModeToolsForAuth(ctx context.Context, tools if !profileScope.Allows(serverName) { continue } - if isScopedAgent { + if isScopeRestricted { // With no catalog there is no permission tier to check, and the // retired directToolPermissions map behaved identically — a // missing tier DROPPED the tool. Failing closed on an unknown @@ -260,7 +278,7 @@ func (p *MCPProxyServer) filterDirectModeToolsForAuth(ctx context.Context, tools case directResolveFound: } - if !directEntryInScope(authCtx, profileScope, isScopedAgent, entry) { + if !directEntryInScope(authCtx, profileScope, isScopeRestricted, entry) { continue } @@ -281,7 +299,7 @@ func (p *MCPProxyServer) filterDirectModeToolsForAuth(ctx context.Context, tools func directEntryInScope( authCtx *auth.AuthContext, profileScope *profile.ProfileScope, - isScopedAgent bool, + isScopeRestricted bool, entry *directCatalogEntry, ) bool { if entry == nil { @@ -293,7 +311,7 @@ func directEntryInScope( // essentially every tool — and hide the catalog from read- and // write-scoped tokens while dispatch happily allowed the same calls // (D13 rule 3). - return directIdentityInScope(authCtx, profileScope, isScopedAgent, entry.ServerName, entry.RequiredPermission) + return directIdentityInScope(authCtx, profileScope, isScopeRestricted, entry.ServerName, entry.RequiredPermission) } // builtinPromptNames is the set of prompt display names mcpproxy serves itself @@ -306,13 +324,14 @@ var builtinPromptNames = map[string]struct{}{ troubleshootServerPrompt().Name: {}, } -// filterAggregatedPromptsForAuth filters prompts/list AND prompts/get for scoped -// agent tokens and for any request with an active profile. It is the prompt -// analogue of filterDirectModeToolsForAuth and the list-side half of the -// aggregated-prompt gate (the handler-side half is -// authorizeAggregatedPromptServer): without it a scoped agent token could -// discover any upstream server's prompt even when the tool filters hid that -// server (PR #973 review, finding F1). mcp-go enforces this on both list and +// filterAggregatedPromptsForAuth filters prompts/list AND prompts/get for +// scope-restricted callers (agent tokens and OAuth-authenticated users — see +// isScopeRestrictedCaller) and for any request with an active profile. It is +// the prompt analogue of filterDirectModeToolsForAuth and the list-side half +// of the aggregated-prompt gate (the handler-side half is +// authorizeAggregatedPromptServer): without it a scope-restricted caller +// could discover any upstream server's prompt even when the tool filters hid +// that server (PR #973 review, finding F1). mcp-go enforces this on both list and // get (server.go filteredPrompts / passesPromptFilters, v1.0.0), so a prompt // dropped here is neither discoverable nor retrievable. // @@ -341,8 +360,8 @@ func (p *MCPProxyServer) filterAggregatedPromptsForAuth(ctx context.Context, pro authCtx := auth.AuthContextFromContext(ctx) _, profileScope := p.resolveActiveProfile(ctx) - isScopedAgent := authCtx != nil && authCtx.Type == auth.AuthTypeAgent - enforce := isScopedAgent || profileScope != nil + isScopeRestricted := isScopeRestrictedCaller(authCtx) + enforce := isScopeRestricted || profileScope != nil allowed := promptServerAllowed(authCtx, profileScope) filtered := make([]mcp.Prompt, 0, len(prompts)) @@ -376,18 +395,18 @@ func (p *MCPProxyServer) filterAggregatedPromptsForAuth(ctx context.Context, pro } // promptServerAllowed returns the per-server access predicate for one caller: -// profile scope (Allows) plus, for scoped agent tokens, server scope +// profile scope (Allows) plus, for scope-restricted callers, server scope // (CanAccessServer). profileScope.Allows tolerates a nil receiver (returns -// true), so the scoped-agent-without-profile case falls through correctly. +// true), so the scope-restricted-without-profile case falls through correctly. // It is the ONE definition of "may this caller touch prompts on server X", // shared by the list/get filter and by every aggregated prompt handler. func promptServerAllowed(authCtx *auth.AuthContext, profileScope *profile.ProfileScope) func(serverName string) bool { - isScopedAgent := authCtx != nil && authCtx.Type == auth.AuthTypeAgent + isScopeRestricted := isScopeRestrictedCaller(authCtx) return func(serverName string) bool { if !profileScope.Allows(serverName) { return false } - return !isScopedAgent || authCtx.CanAccessServer(serverName) + return !isScopeRestricted || authCtx.CanAccessServer(serverName) } } diff --git a/internal/server/mcp_prompt_scope_test.go b/internal/server/mcp_prompt_scope_test.go index 1f72e0893..61a7f8e47 100644 --- a/internal/server/mcp_prompt_scope_test.go +++ b/internal/server/mcp_prompt_scope_test.go @@ -68,6 +68,13 @@ func TestFilterAggregatedPromptsForAuth(t *testing.T) { profile.NewProfileScope("dev", servers), ) } + userCtx := func(servers ...string) context.Context { + return auth.WithAuthContext(context.Background(), &auth.AuthContext{ + Type: auth.AuthTypeUser, + UserID: "u1", + AllowedServers: servers, + }) + } tests := []struct { name string @@ -94,6 +101,20 @@ func TestFilterAggregatedPromptsForAuth(t *testing.T) { ctx: agentCtx("*"), want: []string{builtinSetup, builtinTrbl, githubPrompt, gitlabPrompt}, }, + { + // Regression: Spec 105 PR G. isScopeRestrictedCaller must key off + // IsAdmin(), not Type == AuthTypeAgent — a server-edition OAuth + // "user" is scope-restricted too (cache_authz.go's CallerKindUser), + // and a Type-only check let it see every server's prompts. + name: "scoped OAuth user sees only its server's prompts plus built-ins", + ctx: userCtx("github"), + want: []string{builtinSetup, builtinTrbl, githubPrompt}, + }, + { + name: "OAuth admin user sees everything", + ctx: auth.WithAuthContext(context.Background(), auth.AdminUserContext("a1", "admin@example.com", "Admin", "google")), + want: []string{builtinSetup, builtinTrbl, githubPrompt, gitlabPrompt}, + }, { name: "profile-scoped session sees only in-profile prompts plus built-ins", ctx: profileCtx("gitlab"), From b667ea4183307acbaf64e4cb79d3cb6f900dda08 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 21 Sep 2026 12:53:49 +0300 Subject: [PATCH 2/2] test(scope): cover directEntryCallable's AuthTypeUser fix at describe time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/server/mcp_describe_direct_test.go | 41 +++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/internal/server/mcp_describe_direct_test.go b/internal/server/mcp_describe_direct_test.go index 4bb2a2557..96eddcb5b 100644 --- a/internal/server/mcp_describe_direct_test.go +++ b/internal/server/mcp_describe_direct_test.go @@ -324,6 +324,47 @@ func TestDescribeDirect_ServerScopeGate_UserType(t *testing.T) { assert.Equal(t, describeErrNotFound, byID["github__read_file"]["error"]) } +// TestDescribeDirect_CallabilityGate_UserType is the describe-time half of +// TestFilterDirectToolsForAgentCallability_UserTypeIsScopeRestrictedToo +// (mcp_direct_callability_test.go), which only proved the listing side of +// the fix. directEntryVisibleToSession runs directEntryInScope BEFORE +// directEntryCallable, so a caller scoped OUT of the server (as +// TestDescribeDirect_ServerScopeGate_UserType uses) never reaches the +// callability check at all — it is not a regression test for +// directEntryCallable's own isScopeRestrictedCaller fix. This one scopes the +// OAuth user INTO "github" so the pending tool must be refused by +// callability specifically, with an approved sibling on the same server as +// the positive control (codex gpt-5.6-sol cross-review, round 1: the +// describe-side gate had no non-vacuous AuthTypeUser coverage). +func TestDescribeDirect_CallabilityGate_UserType(t *testing.T) { + p := newDirectDescribeProxy(t) + require.NoError(t, p.storage.SaveToolApproval(&storage.ToolApprovalRecord{ + ServerName: "github", + ToolName: "read_file", + Status: storage.ToolApprovalStatusPending, + })) + + scopedUser := auth.WithAuthContext(context.Background(), &auth.AuthContext{ + Type: auth.AuthTypeUser, + UserID: "u1", + AllowedServers: []string{"github"}, + Permissions: []string{auth.PermRead, auth.PermWrite, auth.PermDestructive}, + }) + + resp := callDescribeDirect(t, p, scopedUser, []interface{}{"github__read_file", "github__create_issue"}) + byID := describeErrorsByID(resp) + require.Contains(t, byID, "github__read_file", + "a pending tool must be refused by callability for a scoped OAuth user, exactly like an agent token") + assert.Equal(t, describeErrNotFound, byID["github__read_file"]["error"]) + + defsByName := map[string]bool{} + for _, def := range resp.Definitions { + defsByName[def["name"].(string)] = true + } + assert.True(t, defsByName["github__create_issue"], + "positive control: an approved sibling on the same in-scope server must still describe") +} + // T044: catalog divergence. A tool that is pending approval is still LISTED for // a non-agent session, so it must still describe — from the catalog snapshot. // An index-backed resolver answers not_found here, which would make deferral