Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
118 changes: 118 additions & 0 deletions packages/mcp-server/src/tools/__tests__/policy.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { describe, expect, it } from 'vitest';
import { buildAuthContext, type McpAuthContext } from '../../auth/identity.js';
import type { AuthPrincipal } from '../../auth/token.js';
import { authorizeToolCall, evaluateToolPolicy } from '../policy.js';
import type { ToolAuthPolicy, ToolDefinition } from '../registry.js';

function authContext(
memberships: Array<{ businessId: string; roleId: string }>,
scopes: string[] = [],
): McpAuthContext {
const principal: AuthPrincipal = {
subject: 'user-1',
issuer: 'https://tenant.auth0.com/',
audience: 'aud',
scopes,
email: null,
expiresAt: undefined,
claims: { sub: 'user-1' },
};
return buildAuthContext(principal, memberships);
}

const M = (businessId: string) => ({ businessId, roleId: 'accountant' });

const scopedPolicy: ToolAuthPolicy = {
requiresBusinessScope: true,
dataClassification: 'business',
};

describe('evaluateToolPolicy — business scope', () => {
it('allows and defaults to all memberships when no scope is requested', () => {
const decision = evaluateToolPolicy({
policy: scopedPolicy,
auth: authContext([M('b1'), M('b2')]),
});
expect(decision).toEqual({ allowed: true, readScope: { businessIds: ['b1', 'b2'] } });
});

it('allows a caller-narrowed subset', () => {
const decision = evaluateToolPolicy({
policy: scopedPolicy,
auth: authContext([M('b1'), M('b2'), M('b3')]),
requestedBusinessIds: ['b3', 'b1'],
});
expect(decision).toEqual({ allowed: true, readScope: { businessIds: ['b3', 'b1'] } });
});

it('denies a requested scope outside the memberships', () => {
const decision = evaluateToolPolicy({
policy: scopedPolicy,
auth: authContext([M('b1')]),
requestedBusinessIds: ['b1', 'bX'],
});
expect(decision.allowed).toBe(false);
if (!decision.allowed) {
expect(decision.error.code).toBe('AUTHORIZATION_ERROR');
expect(decision.error.message).toMatch(/outside your authorized memberships/);
}
});

it('denies when the tool requires business scope but the caller has none', () => {
const decision = evaluateToolPolicy({ policy: scopedPolicy, auth: authContext([]) });
expect(decision.allowed).toBe(false);
if (!decision.allowed) {
expect(decision.error.message).toMatch(/No authorized business scope/);
}
});
});

describe('evaluateToolPolicy — roles', () => {
const roleScopedPolicy: ToolAuthPolicy = {
requiredRoles: ['read:reports', 'admin'],
requiresBusinessScope: false,
dataClassification: 'business',
};

it('allows when the caller holds one of the required roles (any-of)', () => {
const decision = evaluateToolPolicy({
policy: roleScopedPolicy,
auth: authContext([M('b1')], ['read:reports']),
});
expect(decision.allowed).toBe(true);
});

it('denies when the caller holds none of the required roles', () => {
const decision = evaluateToolPolicy({
policy: roleScopedPolicy,
auth: authContext([M('b1')], ['read:charges']),
});
expect(decision.allowed).toBe(false);
if (!decision.allowed) {
expect(decision.error.message).toMatch(/missing a required role/);
}
});

it('applies no role gate when requiredRoles is omitted', () => {
const decision = evaluateToolPolicy({
policy: { requiresBusinessScope: false, dataClassification: 'public' },
auth: authContext([], []),
});
expect(decision).toEqual({ allowed: true, readScope: { businessIds: [] } });
});
});

describe('authorizeToolCall', () => {
it('evaluates the policy of a registered tool', () => {
const tool = {
name: 't',
description: 'd',
inputSchema: undefined as never,
policy: scopedPolicy,
handler: () => ({ content: [] }),
} as unknown as ToolDefinition;

const decision = authorizeToolCall(tool, authContext([M('b1')]), ['b1']);
expect(decision).toEqual({ allowed: true, readScope: { businessIds: ['b1'] } });
});
Comment on lines +115 to +117

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Add a test case to verify that authorizeToolCall correctly denies requests when the input arguments contain a businessId outside the resolved read scope.

    const decision = authorizeToolCall(tool, authContext([M('b1')]), ['b1']);
    expect(decision).toEqual({ allowed: true, readScope: { businessIds: ['b1'] } });
  });

  it('denies when the input arguments contain a businessId outside the resolved read scope', () => {
    const tool = {
      name: 't',
      description: 'd',
      inputSchema: undefined as never,
      policy: scopedPolicy,
      handler: () => ({ content: [] }),
    } as unknown as ToolDefinition;

    const decision = authorizeToolCall(tool, authContext([M('b1')]), ['b1'], { businessId: 'b2' });
    expect(decision.allowed).toBe(false);
    if (!decision.allowed) {
      expect(decision.error.message).toMatch(/outside your authorized read scope/);
    }
  });

});
82 changes: 82 additions & 0 deletions packages/mcp-server/src/tools/policy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import {
resolveRequestedReadScope,
type AuthorizedReadScope,
type McpAuthContext,
} from '../auth/identity.js';
import type { ToolAuthPolicy, ToolDefinition } from './registry.js';

/**
* Per-tool authorization policy evaluator (spec §7).
*
* Runs BEFORE a tool handler executes. It enforces the tool's required roles
* and business-scope constraints against the caller's auth context, and applies
* optional caller-provided scope narrowing — but only ever as a subset of the
* caller's authorized memberships. Any request outside those memberships is
* denied with a deterministic AUTHORIZATION_ERROR (never silently narrowed).
*/

/** Deterministic authorization error payload (spec §10.2). */
export interface AuthorizationError {
code: 'AUTHORIZATION_ERROR';
message: string;
}

export type PolicyDecision =
{ allowed: true; readScope: AuthorizedReadScope } | { allowed: false; error: AuthorizationError };

function deny(message: string): PolicyDecision {
return { allowed: false, error: { code: 'AUTHORIZATION_ERROR', message } };
}

export interface EvaluatePolicyParams {
policy: ToolAuthPolicy;
auth: McpAuthContext;
/** Optional caller-provided business-scope narrowing (subset of memberships). */
requestedBusinessIds?: readonly string[];
}

/**
* Evaluate a tool's policy for a caller. Returns the resolved read scope when
* allowed, or an AUTHORIZATION_ERROR when denied.
*
* Rules:
* - Required roles (any-of): the caller must hold at least one when the policy
* lists any. No list ⇒ no role gate.
* - Requested scope narrowing must be a subset of the caller's memberships;
* otherwise the request is denied (not silently dropped).
* - When the tool requires business scope, the resolved scope must be
* non-empty (a caller with no memberships cannot use it).
*/
export function evaluateToolPolicy(params: EvaluatePolicyParams): PolicyDecision {
const { policy, auth, requestedBusinessIds } = params;

// 1. Role gate (any-of).
if (policy.requiredRoles && policy.requiredRoles.length > 0) {
const held = new Set(auth.roles);
if (!policy.requiredRoles.some(role => held.has(role))) {
return deny('Caller is missing a required role for this tool');
}
}

// 2. Resolve requested scope as a subset of authorized memberships.
const readScope = resolveRequestedReadScope(auth, requestedBusinessIds);
if (readScope === null) {
return deny('Requested business scope is outside your authorized memberships');
}

// 3. Business-scope requirement.
if (policy.requiresBusinessScope && readScope.businessIds.length === 0) {
return deny('No authorized business scope for this request');
}

return { allowed: true, readScope };
}

/** Convenience: evaluate the policy of a registered tool. */
export function authorizeToolCall(
tool: ToolDefinition,
auth: McpAuthContext,
requestedBusinessIds?: readonly string[],
): PolicyDecision {
return evaluateToolPolicy({ policy: tool.policy, auth, requestedBusinessIds });
}
Comment on lines +76 to +82

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-high high

Security Concern: Potential Tenant Isolation Bypass

If a tool handler accepts a businessId (or business_id) in its input arguments, there is currently no automatic validation ensuring that the requested input.businessId is within the resolved context.readScope.businessIds.

If a developer forgets to manually perform this check inside the tool's handler, a caller could pass an arbitrary businessId in the tool arguments and bypass tenant isolation, even if requestedBusinessIds was correctly narrowed or restricted at the policy level.

Recommendation

Implement an automatic validation step in authorizeToolCall that automatically rejects the request if the validated input contains a businessId or business_id field that is not present in the resolved readScope.businessIds (when requiresBusinessScope is true).

Suggested change
export function authorizeToolCall(
tool: ToolDefinition,
auth: McpAuthContext,
requestedBusinessIds?: readonly string[],
): PolicyDecision {
return evaluateToolPolicy({ policy: tool.policy, auth, requestedBusinessIds });
}
export function authorizeToolCall(
tool: ToolDefinition,
auth: McpAuthContext,
requestedBusinessIds?: readonly string[],
inputArgs?: Record<string, unknown>,
): PolicyDecision {
const decision = evaluateToolPolicy({ policy: tool.policy, auth, requestedBusinessIds });
if (!decision.allowed) {
return decision;
}
if (tool.policy.requiresBusinessScope && inputArgs) {
const targetBusinessId = inputArgs.businessId ?? inputArgs.business_id;
if (typeof targetBusinessId === 'string' && !decision.readScope.businessIds.includes(targetBusinessId)) {
return deny(`Requested businessId '${targetBusinessId}' is outside your authorized read scope`);
}
}
return decision;
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Declining — the tenant-isolation bypass is already prevented at the execution layer (added in the charges/report steps). executeRegisteredTool derives the requested scope from a businessId/businessIds input and passes it to evaluateToolPolicy, which narrows via narrowReadScope and returns AUTHORIZATION_ERROR when any requested id is outside the caller's memberships. So a tool that reads input.businessId can never receive one outside scope — the call is denied before the handler runs.

This is covered by tests: charges "denies a requested business outside the memberships" and reports "denies a business outside the caller memberships". Adding the check to authorizeToolCall would duplicate logic that isn't on the execution path (the executor calls evaluateToolPolicy directly). Keeping enforcement in one place.


Generated by Claude Code

Loading