fix(studio-mcp): enforce API key permissions and bind session identity

betterclever · claude · betterclever · commit 4377d6f875db · 2026-02-13T13:35:40.000+04:00
Address Codex P1 review findings:

1. Permission gating: All workflow/run MCP tools now check
   auth.apiKeyPermissions before executing. Keys with restricted
   permissions (e.g. workflows.run=false) are properly denied.
   Component tools remain ungated (static metadata).

2. Session identity binding: MCP sessions store the creator's
   userId + organizationId. Subsequent requests verify the caller
   matches the session creator, returning 403 on mismatch to
   prevent cross-tenant session hijacking.

3. Extended AuthContext with optional apiKeyPermissions field,
   populated during API key authentication in AuthGuard.

Tests: 26 pass (19 service + 7 controller), 61 assertions.

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
Signed-off-by: betterclever &lt;paliwal.pranjal83@gmail.com&gt;
diff --git a/backend/src/auth/auth.guard.ts b/backend/src/auth/auth.guard.ts
@@ -124,6 +124,7 @@ export class AuthGuard implements CanActivate {
       roles: ['MEMBER'], // API keys have MEMBER role by default
       isAuthenticated: true,
       provider: 'api-key',
+      apiKeyPermissions: apiKey.permissions,
     };
   }
 }
diff --git a/backend/src/auth/types.ts b/backend/src/auth/types.ts
@@ -1,11 +1,18 @@
 export type AuthRole = 'ADMIN' | 'MEMBER';
 
+export interface ApiKeyPermissions {
+  workflows: { run: boolean; list: boolean; read: boolean };
+  runs: { read: boolean; cancel: boolean };
+}
+
 export interface AuthContext {
   userId: string | null;
   organizationId: string | null;
   roles: AuthRole[];
   isAuthenticated: boolean;
   provider: string;
+  /** Present only when authenticated via API key. */
+  apiKeyPermissions?: ApiKeyPermissions;
 }
 
 export const DEFAULT_ROLES: AuthRole[] = ['ADMIN', 'MEMBER'];
diff --git a/backend/src/studio-mcp/__tests__/studio-mcp.controller.spec.ts b/backend/src/studio-mcp/__tests__/studio-mcp.controller.spec.ts
@@ -0,0 +1,222 @@
+import { describe, it, expect, beforeEach, jest } from 'bun:test';
+import { StudioMcpController } from '../studio-mcp.controller';
+import type { StudioMcpService } from '../studio-mcp.service';
+import type { AuthContext } from '../../auth/types';
+import type { Request, Response } from 'express';
+import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
+
+// Access private sessions map for assertions
+type SessionsMap = Map<
+  string,
+  { transport: unknown; userId: string | null; organizationId: string | null }
+>;
+function getSessions(controller: StudioMcpController): SessionsMap {
+  return (controller as unknown as { sessions: SessionsMap }).sessions;
+}
+
+function createMockRes(): Response & { _status: number; _json: unknown } {
+  const res = {
+    _status: 200,
+    _json: null,
+    status(code: number) {
+      res._status = code;
+      return res;
+    },
+    json(body: unknown) {
+      res._json = body;
+      return res;
+    },
+    on: jest.fn(),
+  } as unknown as Response & { _status: number; _json: unknown };
+  return res;
+}
+
+function createMockReq(
+  overrides: Partial<Request> & { auth?: AuthContext } = {},
+): Request & { auth?: AuthContext } {
+  return {
+    method: 'POST',
+    headers: {},
+    header: jest.fn().mockReturnValue(undefined),
+    body: {},
+    ...overrides,
+  } as unknown as Request & { auth?: AuthContext };
+}
+
+describe('StudioMcpController', () => {
+  let controller: StudioMcpController;
+  let mcpService: StudioMcpService;
+
+  const authUser1: AuthContext = {
+    userId: 'user-1',
+    organizationId: 'org-1',
+    roles: ['MEMBER'],
+    isAuthenticated: true,
+    provider: 'api-key',
+    apiKeyPermissions: {
+      workflows: { run: true, list: true, read: true },
+      runs: { read: true, cancel: true },
+    },
+  };
+
+  const authUser2: AuthContext = {
+    userId: 'user-2',
+    organizationId: 'org-2',
+    roles: ['MEMBER'],
+    isAuthenticated: true,
+    provider: 'api-key',
+    apiKeyPermissions: {
+      workflows: { run: true, list: true, read: true },
+      runs: { read: true, cancel: true },
+    },
+  };
+
+  beforeEach(() => {
+    mcpService = {
+      createServer: jest.fn().mockReturnValue(new McpServer({ name: 'test', version: '1.0.0' })),
+    } as unknown as StudioMcpService;
+
+    controller = new StudioMcpController(mcpService);
+  });
+
+  it('rejects unauthenticated requests with 401', async () => {
+    const req = createMockReq({ auth: undefined });
+    const res = createMockRes();
+
+    await controller.handleMcp(req, res);
+
+    expect(res._status).toBe(401);
+    expect(res._json).toEqual({
+      error: 'Authentication required. Use Bearer sk_live_* API key.',
+    });
+  });
+
+  it('rejects requests without session ID and without initialize body with 400', async () => {
+    const req = createMockReq({
+      auth: authUser1,
+      method: 'POST',
+      headers: {},
+      body: { jsonrpc: '2.0', method: 'tools/list', id: 1 },
+    });
+    const res = createMockRes();
+
+    await controller.handleMcp(req, res);
+
+    expect(res._status).toBe(400);
+  });
+
+  it('returns 404 for unknown session ID', async () => {
+    const req = createMockReq({
+      auth: authUser1,
+      headers: { 'mcp-session-id': 'nonexistent-session' },
+    });
+    const res = createMockRes();
+
+    await controller.handleMcp(req, res);
+
+    expect(res._status).toBe(404);
+    expect(res._json).toEqual({ error: 'Session not found or expired' });
+  });
+
+  describe('session identity binding', () => {
+    it('rejects session reuse from different user with 403', async () => {
+      // Manually insert a session owned by user-1
+      const sessions = getSessions(controller);
+      const mockTransport = { handleRequest: jest.fn() };
+      sessions.set('test-session-id', {
+        transport: mockTransport,
+        userId: authUser1.userId,
+        organizationId: authUser1.organizationId,
+      });
+
+      // User-2 tries to use user-1's session
+      const req = createMockReq({
+        auth: authUser2,
+        method: 'POST',
+        headers: { 'mcp-session-id': 'test-session-id' },
+        body: { jsonrpc: '2.0', method: 'tools/list', id: 1 },
+      });
+      const res = createMockRes();
+
+      await controller.handleMcp(req, res);
+
+      expect(res._status).toBe(403);
+      expect(res._json).toEqual({ error: 'Session belongs to a different principal' });
+      expect(mockTransport.handleRequest).not.toHaveBeenCalled();
+    });
+
+    it('rejects session reuse from different org with 403', async () => {
+      const sessions = getSessions(controller);
+      const mockTransport = { handleRequest: jest.fn() };
+      sessions.set('test-session-id', {
+        transport: mockTransport,
+        userId: authUser1.userId,
+        organizationId: authUser1.organizationId,
+      });
+
+      // Same user ID but different org
+      const crossOrgAuth: AuthContext = {
+        ...authUser1,
+        organizationId: 'different-org',
+      };
+      const req = createMockReq({
+        auth: crossOrgAuth,
+        method: 'POST',
+        headers: { 'mcp-session-id': 'test-session-id' },
+        body: { jsonrpc: '2.0', method: 'tools/list', id: 1 },
+      });
+      const res = createMockRes();
+
+      await controller.handleMcp(req, res);
+
+      expect(res._status).toBe(403);
+      expect(mockTransport.handleRequest).not.toHaveBeenCalled();
+    });
+
+    it('allows session reuse from same principal', async () => {
+      const sessions = getSessions(controller);
+      const mockTransport = { handleRequest: jest.fn() };
+      sessions.set('test-session-id', {
+        transport: mockTransport,
+        userId: authUser1.userId,
+        organizationId: authUser1.organizationId,
+      });
+
+      const req = createMockReq({
+        auth: authUser1,
+        method: 'POST',
+        headers: { 'mcp-session-id': 'test-session-id' },
+        body: { jsonrpc: '2.0', method: 'tools/list', id: 1 },
+      });
+      const res = createMockRes();
+
+      await controller.handleMcp(req, res);
+
+      // Should have forwarded to the transport, not returned an error
+      expect(res._status).toBe(200); // not changed to 403 or 404
+      expect(mockTransport.handleRequest).toHaveBeenCalled();
+    });
+
+    it('cleans up session on DELETE from same principal', async () => {
+      const sessions = getSessions(controller);
+      const mockTransport = { handleRequest: jest.fn() };
+      sessions.set('test-session-id', {
+        transport: mockTransport,
+        userId: authUser1.userId,
+        organizationId: authUser1.organizationId,
+      });
+
+      const req = createMockReq({
+        auth: authUser1,
+        method: 'DELETE',
+        headers: { 'mcp-session-id': 'test-session-id' },
+      });
+      const res = createMockRes();
+
+      await controller.handleMcp(req, res);
+
+      expect(sessions.has('test-session-id')).toBe(false);
+      expect(mockTransport.handleRequest).toHaveBeenCalled();
+    });
+  });
+});
diff --git a/backend/src/studio-mcp/__tests__/studio-mcp.service.spec.ts b/backend/src/studio-mcp/__tests__/studio-mcp.service.spec.ts
@@ -207,6 +207,125 @@ describe('StudioMcpService Unit Tests', () => {
       expect(getResult).toBeDefined();
     });
 
+    describe('API key permission gating', () => {
+      const restrictedAuth: AuthContext = {
+        userId: 'api-key-id',
+        organizationId: 'test-org-id',
+        roles: ['MEMBER'],
+        isAuthenticated: true,
+        provider: 'api-key',
+        apiKeyPermissions: {
+          workflows: { run: false, list: true, read: true },
+          runs: { read: true, cancel: false },
+        },
+      };
+
+      it('allows list_workflows when workflows.list is true', async () => {
+        const server = service.createServer(restrictedAuth);
+        const tools = getRegisteredTools(server);
+        const result = (await tools['list_workflows'].handler({})) as { isError?: boolean };
+        expect(result.isError).toBeUndefined();
+      });
+
+      it('denies run_workflow when workflows.run is false', async () => {
+        const server = service.createServer(restrictedAuth);
+        const tools = getRegisteredTools(server);
+        const result = (await tools['run_workflow'].handler({
+          workflowId: '11111111-1111-4111-8111-111111111111',
+        })) as { isError?: boolean; content: { text: string }[] };
+        expect(result.isError).toBe(true);
+        expect(result.content[0].text).toContain('workflows.run');
+      });
+
+      it('denies cancel_run when runs.cancel is false', async () => {
+        const server = service.createServer(restrictedAuth);
+        const tools = getRegisteredTools(server);
+        const result = (await tools['cancel_run'].handler({
+          runId: 'test-run-id',
+        })) as { isError?: boolean; content: { text: string }[] };
+        expect(result.isError).toBe(true);
+        expect(result.content[0].text).toContain('runs.cancel');
+      });
+
+      it('allows get_run_status when runs.read is true', async () => {
+        const server = service.createServer(restrictedAuth);
+        const tools = getRegisteredTools(server);
+        const result = (await tools['get_run_status'].handler({
+          runId: 'test-run-id',
+        })) as { isError?: boolean };
+        expect(result.isError).toBeUndefined();
+      });
+
+      it('allows all tools when no apiKeyPermissions (non-API-key auth)', async () => {
+        const server = service.createServer(mockAuthContext); // no apiKeyPermissions
+        const tools = getRegisteredTools(server);
+
+        // All workflow/run tools should work without permission errors
+        const listResult = (await tools['list_workflows'].handler({})) as { isError?: boolean };
+        expect(listResult.isError).toBeUndefined();
+
+        const runResult = (await tools['run_workflow'].handler({
+          workflowId: '11111111-1111-4111-8111-111111111111',
+        })) as { isError?: boolean };
+        expect(runResult.isError).toBeUndefined();
+
+        const cancelResult = (await tools['cancel_run'].handler({
+          runId: 'test-run-id',
+        })) as { isError?: boolean };
+        expect(cancelResult.isError).toBeUndefined();
+      });
+
+      it('component tools are always allowed regardless of permissions', async () => {
+        const noPermsAuth: AuthContext = {
+          ...restrictedAuth,
+          apiKeyPermissions: {
+            workflows: { run: false, list: false, read: false },
+            runs: { read: false, cancel: false },
+          },
+        };
+        const server = service.createServer(noPermsAuth);
+        const tools = getRegisteredTools(server);
+
+        const listResult = (await tools['list_components'].handler({})) as { isError?: boolean };
+        expect(listResult.isError).toBeUndefined();
+
+        const getResult = (await tools['get_component'].handler({
+          componentId: 'core.workflow.entrypoint',
+        })) as { isError?: boolean };
+        expect(getResult.isError).toBeUndefined();
+      });
+
+      it('denies all 7 gated tools when all permissions are false', async () => {
+        const noPermsAuth: AuthContext = {
+          ...restrictedAuth,
+          apiKeyPermissions: {
+            workflows: { run: false, list: false, read: false },
+            runs: { read: false, cancel: false },
+          },
+        };
+        const server = service.createServer(noPermsAuth);
+        const tools = getRegisteredTools(server);
+
+        const gatedTools = [
+          'list_workflows',
+          'get_workflow',
+          'run_workflow',
+          'list_runs',
+          'get_run_status',
+          'get_run_result',
+          'cancel_run',
+        ];
+
+        for (const toolName of gatedTools) {
+          const result = (await tools[toolName].handler({
+            workflowId: '11111111-1111-4111-8111-111111111111',
+            runId: 'test-run-id',
+          })) as { isError?: boolean };
+          expect(result.isError).toBe(true);
+        }
+      });
+    });
+
     it('each server instance has isolated auth context', async () => {
       const authContext1: AuthContext = {
         userId: 'user-1',
diff --git a/backend/src/studio-mcp/studio-mcp.controller.ts b/backend/src/studio-mcp/studio-mcp.controller.ts
diff --git a/backend/src/studio-mcp/studio-mcp.service.ts b/backend/src/studio-mcp/studio-mcp.service.ts

Original file line number	Diff line number	Diff line change
`@@ -124,6 +124,7 @@ export class AuthGuard implements CanActivate {`
`124`	`124`	`roles: ['MEMBER'], // API keys have MEMBER role by default`
`125`	`125`	`isAuthenticated: true,`
`126`	`126`	`provider: 'api-key',`
	`127`	`+ apiKeyPermissions: apiKey.permissions,`
`127`	`128`	`};`
`128`	`129`	`}`
`129`	`130`	`}`