Skip to content

Introduce tool:list verb for ingress and tool:read for egress filtering #4050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ghaskins wants to merge 1 commit into
base: main
Choose a base branch
from
+82 −15
Open

Introduce tool:list verb for ingress and tool:read for egress filtering #4050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions pkg/authz/authorizers/cedar/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,42 @@ func (a *Authorizer) authorizeToolCall(
return a.IsAuthorized(principal, action, resource, contextMap, entities)
}

// authorizeToolRead authorizes a tool read operation.
// This method is used when filtering a tools list response to determine
// which tools a client is permitted to see.
func (a *Authorizer) authorizeToolRead(
clientID, toolName string,
claimsMap map[string]interface{},
attrsMap map[string]interface{},
) (bool, error) {
// Extract principal from client ID
principal := fmt.Sprintf("Client::%s", clientID)

// Action is to read a tool
action := "Action::read_tool"

// Resource is the tool being read
resource := fmt.Sprintf("Tool::%s", toolName)

// Create attributes for the entities
attributes := mergeContexts(map[string]interface{}{
"name": toolName,
"operation": "read",
"feature": "tool",
}, attrsMap)

// Create Cedar entities
entities, err := a.entityFactory.CreateEntitiesForRequest(principal, action, resource, claimsMap, attributes)
if err != nil {
return false, fmt.Errorf("failed to create Cedar entities: %w", err)
}

contextMap := mergeContexts(claimsMap, attrsMap)

// Check authorization with entities
return a.IsAuthorized(principal, action, resource, contextMap, entities)
}

// authorizePromptGet authorizes a prompt get operation.
// This method is used when a client tries to get a specific prompt.
// It checks if the client is authorized to access the prompt with the given context.
Expand Down Expand Up @@ -608,6 +644,10 @@ func (a *Authorizer) AuthorizeWithJWTClaims(
// Use the authorizeToolCall function for tool call operations
return a.authorizeToolCall(ctx, clientID, resourceID, processedClaims, processedArgs)

case feature == authorizers.MCPFeatureTool && operation == authorizers.MCPOperationRead:
// Use the authorizeToolRead function for tool read (list filtering) operations
return a.authorizeToolRead(clientID, resourceID, processedClaims, processedArgs)

case feature == authorizers.MCPFeaturePrompt && operation == authorizers.MCPOperationGet:
// Use the authorizePromptGet function for prompt get operations
return a.authorizePromptGet(clientID, resourceID, processedClaims, processedArgs)
Expand Down
11 changes: 8 additions & 3 deletions pkg/authz/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,17 @@ func TestIntegrationListFiltering(t *testing.T) {
// Create a realistic Cedar authorizer with role-based policies
authorizer, err := cedar.NewCedarAuthorizer(cedar.ConfigOptions{
Policies: []string{
// All principals can perform list operations (results are filtered per role)
`permit(principal, action == Action::"list_tools", resource);`,
`permit(principal, action == Action::"list_prompts", resource);`,
`permit(principal, action == Action::"list_resources", resource);`,

// Basic users can only access weather and news tools
`permit(principal, action == Action::"call_tool", resource == Tool::"weather") when { principal.claim_role == "user" };`,
`permit(principal, action == Action::"call_tool", resource == Tool::"news") when { principal.claim_role == "user" };`,
`permit(principal, action == Action::"read_tool", resource == Tool::"weather") when { principal.claim_role == "user" };`,
`permit(principal, action == Action::"read_tool", resource == Tool::"news") when { principal.claim_role == "user" };`,

// Admin users can access all tools
`permit(principal, action == Action::"call_tool", resource) when { principal.claim_role == "admin" };`,
`permit(principal, action == Action::"read_tool", resource) when { principal.claim_role == "admin" };`,

// Basic users can only access public prompts
`permit(principal, action == Action::"get_prompt", resource == Prompt::"greeting") when { principal.claim_role == "user" };`,
Expand Down
16 changes: 14 additions & 2 deletions pkg/authz/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ var MCPMethodToFeatureOperation = map[string]struct {
"resources/unsubscribe": {Feature: authorizers.MCPFeatureResource, Operation: authorizers.MCPOperationRead},

// Discovery and capability methods - always allowed
"features/list": {Feature: "", Operation: authorizers.MCPOperationList}, // Capability discovery
"roots/list": {Feature: "", Operation: ""}, // Root directory discovery
"features/list": {Feature: "", Operation: ""}, // Capability discovery - always allowed
"roots/list": {Feature: "", Operation: ""}, // Root directory discovery

// Logging and client preferences - always allowed
"logging/setLevel": {Feature: "", Operation: ""}, // Client preference for server logging
Expand Down Expand Up @@ -209,6 +209,18 @@ func Middleware(a authorizers.Authorizer, next http.Handler) http.Handler {

// Handle list operations differently - allow them through but filter the response
if featureOp.Operation == authorizers.MCPOperationList {
authorized, err := a.AuthorizeWithJWTClaims(
r.Context(),
featureOp.Feature,
authorizers.MCPOperationList,
"metadata",
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For list operations, the middleware is passing a hard-coded resourceID ("metadata") into AuthorizeWithJWTClaims. This changes the resource/mcp.resource_id seen by non-Cedar authorizers (e.g., the HTTP PDP authorizer will emit mrn:mcp:<server>:tool:metadata for tools/list), which is likely unintended and makes list authorization harder to reason about. Prefer passing an empty resourceID (or the parsed cursor if you intentionally want pagination-aware decisions).

Suggested change
"metadata",
"",

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@ghaskins ghaskins Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yrobla I left this one, as the assignment was intentional. The rationale was that this presents a verb of "tool:list" with a resource MRN of "mrn:...:tool:metadata", which makes sense to me, i.e. we are listing the tool's metadata (name, prompts, etc). Thoughts?

nil, // No arguments for the authorization check
)
// Handle unauthorized requests
if err != nil || !authorized {
handleUnauthorized(w, parsedRequest.ID, err)
return
}

// Create a response filtering writer to intercept and filter the response
filteringWriter := NewResponseFilteringWriter(w, a, r, parsedRequest.Method)
Expand Down
21 changes: 15 additions & 6 deletions pkg/authz/middleware_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ func TestMiddleware(t *testing.T) {
`permit(principal, action == Action::"call_tool", resource == Tool::"weather");`,
`permit(principal, action == Action::"get_prompt", resource == Prompt::"greeting");`,
`permit(principal, action == Action::"read_resource", resource == Resource::"data");`,
`permit(principal, action == Action::"list_tools", resource);`,
`permit(principal, action == Action::"list_prompts", resource);`,
`permit(principal, action == Action::"list_resources", resource);`,
},
EntitiesJSON: `[]`,
})
Expand Down Expand Up @@ -695,7 +698,8 @@ func TestMiddlewareToolsListTestkit(t *testing.T) {
testkit.WithJSONClientType(),
},
policies: []string{
`permit(principal, action == Action::"call_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"read_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"list_tools", resource);`,
},
expected: []any{
map[string]any{"name": "foo", "description": "A test tool"},
Expand All @@ -711,7 +715,8 @@ func TestMiddlewareToolsListTestkit(t *testing.T) {
testkit.WithJSONClientType(),
},
policies: []string{
`permit(principal, action == Action::"call_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"read_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"list_tools", resource);`,
},
expected: []any{
map[string]any{"name": "foo", "description": "A test tool"},
Expand All @@ -725,7 +730,8 @@ func TestMiddlewareToolsListTestkit(t *testing.T) {
testkit.WithJSONClientType(),
},
policies: []string{
`permit(principal, action == Action::"call_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"read_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"list_tools", resource);`,
},
expected: []any{},
},
Expand All @@ -739,7 +745,8 @@ func TestMiddlewareToolsListTestkit(t *testing.T) {
testkit.WithSSEClientType(),
},
policies: []string{
`permit(principal, action == Action::"call_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"read_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"list_tools", resource);`,
},
expected: []any{
map[string]any{"name": "foo", "description": "A test tool"},
Expand All @@ -755,7 +762,8 @@ func TestMiddlewareToolsListTestkit(t *testing.T) {
testkit.WithSSEClientType(),
},
policies: []string{
`permit(principal, action == Action::"call_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"read_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"list_tools", resource);`,
},
expected: []any{
map[string]any{"name": "foo", "description": "A test tool"},
Expand All @@ -769,7 +777,8 @@ func TestMiddlewareToolsListTestkit(t *testing.T) {
testkit.WithSSEClientType(),
},
policies: []string{
`permit(principal, action == Action::"call_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"read_tool", resource == Tool::"foo");`,
`permit(principal, action == Action::"list_tools", resource);`,
},
expected: []any{},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/authz/response_filter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,11 @@ func (rfw *ResponseFilteringWriter) filterToolsResponse(response *jsonrpc2.Respo
// This is basically defensive programming, but for clients.
filteredTools := []mcp.Tool{}
for _, tool := range listResult.Tools {
// Check if the user is authorized to call this tool
// Check if the user is authorized to read this tool
authorized, err := rfw.authorizer.AuthorizeWithJWTClaims(
rfw.request.Context(),
authorizers.MCPFeatureTool,
authorizers.MCPOperationCall,
authorizers.MCPOperationRead,
tool.Name,
nil, // No arguments for the authorization check
)
Expand Down
4 changes: 2 additions & 2 deletions pkg/authz/response_filter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestResponseFilteringWriter(t *testing.T) {
// Create a Cedar authorizer with specific tool permissions
authorizer, err := cedar.NewCedarAuthorizer(cedar.ConfigOptions{
Policies: []string{
`permit(principal, action == Action::"call_tool", resource == Tool::"weather");`,
`permit(principal, action == Action::"read_tool", resource == Tool::"weather");`,
`permit(principal, action == Action::"get_prompt", resource == Prompt::"greeting");`,
`permit(principal, action == Action::"read_resource", resource == Resource::"data");`,
},
Expand Down Expand Up @@ -329,7 +329,7 @@ func TestResponseFilteringWriter_ContentLengthMismatch(t *testing.T) {
// The backend will return 3 tools, so filtering will shrink the response.
authorizer, err := cedar.NewCedarAuthorizer(cedar.ConfigOptions{
Policies: []string{
`permit(principal, action == Action::"call_tool", resource == Tool::"weather");`,
`permit(principal, action == Action::"read_tool", resource == Tool::"weather");`,
},
EntitiesJSON: `[]`,
})
Expand Down
1 change: 1 addition & 0 deletions pkg/vmcp/auth/factory/authz_not_wired_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func TestNewIncomingAuthMiddleware_AuthzApproveAndBlock(t *testing.T) {
Type: "cedar",
Policies: []string{
`permit(principal, action == Action::"list_tools", resource);`,
`permit(principal, action == Action::"list_prompts", resource);`,
`forbid(principal, action == Action::"call_tool", resource);`,
},
},
Expand Down
Loading