fix: RBAC name-scoped access — CUID resolution + list filtering
Some checks failed
CI / lint (pull_request) Has been cancelled
CI / typecheck (pull_request) Has been cancelled
CI / test (pull_request) Has been cancelled
CI / build (pull_request) Has been cancelled
CI / package (pull_request) Has been cancelled

Two bugs fixed:
- GET /api/v1/servers/:cuid now resolves CUID→name before RBAC check,
  so name-scoped bindings match correctly
- List endpoints now filter responses via preSerialization hook using
  getAllowedScope(), so name-scoped users only see their resources

Also adds fulldeploy.sh orchestrator script.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Michal
2026-02-23 12:26:37 +00:00
parent da14bb8c23
commit 604bd76d60
7 changed files with 736 additions and 1 deletions

View File

@@ -0,0 +1,444 @@
/**
* Integration tests reproducing RBAC name-scoped access bugs.
*
* Bug 1: `mcpctl get servers` shows ALL servers despite user only having
* view:servers+name:my-home-assistant
* Bug 2: `mcpctl get server my-home-assistant -o yaml` returns 403 because
* CLI resolves name→CUID, and RBAC compares CUID against binding name
*
* These tests spin up a full Fastify app with auth + RBAC hooks + server routes,
* exactly like main.ts, to catch regressions at the HTTP level.
*/
import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
import Fastify from 'fastify';
import type { FastifyInstance } from 'fastify';
import { registerMcpServerRoutes } from '../src/routes/mcp-servers.js';
import { McpServerService } from '../src/services/mcp-server.service.js';
import { InstanceService } from '../src/services/instance.service.js';
import { RbacService } from '../src/services/rbac.service.js';
import { errorHandler } from '../src/middleware/error-handler.js';
import type { IMcpServerRepository, IMcpInstanceRepository } from '../src/repositories/interfaces.js';
import type { IRbacDefinitionRepository } from '../src/repositories/rbac-definition.repository.js';
import type { McpOrchestrator } from '../src/services/orchestrator.js';
import type { McpServer, RbacDefinition, PrismaClient } from '@prisma/client';
// ── Test data ──
const SERVERS: McpServer[] = [
{ id: 'clxyz000000001', name: 'my-home-assistant', description: 'HA server', transport: 'STDIO', packageName: null, dockerImage: null, repositoryUrl: null, externalUrl: null, command: null, containerPort: null, replicas: 1, env: [], healthCheck: null, version: 1, createdAt: new Date(), updatedAt: new Date() },
{ id: 'clxyz000000002', name: 'slack-server', description: 'Slack MCP', transport: 'STDIO', packageName: null, dockerImage: null, repositoryUrl: null, externalUrl: null, command: null, containerPort: null, replicas: 1, env: [], healthCheck: null, version: 1, createdAt: new Date(), updatedAt: new Date() },
{ id: 'clxyz000000003', name: 'github-server', description: 'GitHub MCP', transport: 'STDIO', packageName: null, dockerImage: null, repositoryUrl: null, externalUrl: null, command: null, containerPort: null, replicas: 1, env: [], healthCheck: null, version: 1, createdAt: new Date(), updatedAt: new Date() },
];
// User tokens → userId mapping
const SESSIONS: Record<string, { userId: string }> = {
'scoped-token': { userId: 'user-scoped' },
'admin-token': { userId: 'user-admin' },
'multi-scoped-token': { userId: 'user-multi' },
'secrets-only-token': { userId: 'user-secrets' },
'edit-scoped-token': { userId: 'user-edit-scoped' },
};
// User email mapping
const USERS: Record<string, { email: string }> = {
'user-scoped': { email: 'scoped@example.com' },
'user-admin': { email: 'admin@example.com' },
'user-multi': { email: 'multi@example.com' },
'user-secrets': { email: 'secrets@example.com' },
'user-edit-scoped': { email: 'editscoped@example.com' },
};
// RBAC definitions
const RBAC_DEFS: RbacDefinition[] = [
{
id: 'rbac-scoped', name: 'scoped-view', version: 1, createdAt: new Date(), updatedAt: new Date(),
subjects: [{ kind: 'User', name: 'scoped@example.com' }],
roleBindings: [{ role: 'view', resource: 'servers', name: 'my-home-assistant' }],
},
{
id: 'rbac-admin', name: 'admin-all', version: 1, createdAt: new Date(), updatedAt: new Date(),
subjects: [{ kind: 'User', name: 'admin@example.com' }],
roleBindings: [{ role: 'edit', resource: '*' }],
},
{
id: 'rbac-multi', name: 'multi-scoped', version: 1, createdAt: new Date(), updatedAt: new Date(),
subjects: [{ kind: 'User', name: 'multi@example.com' }],
roleBindings: [
{ role: 'view', resource: 'servers', name: 'my-home-assistant' },
{ role: 'view', resource: 'servers', name: 'slack-server' },
],
},
{
id: 'rbac-secrets', name: 'secrets-only', version: 1, createdAt: new Date(), updatedAt: new Date(),
subjects: [{ kind: 'User', name: 'secrets@example.com' }],
roleBindings: [{ role: 'view', resource: 'secrets' }],
},
{
id: 'rbac-edit-scoped', name: 'edit-scoped', version: 1, createdAt: new Date(), updatedAt: new Date(),
subjects: [{ kind: 'User', name: 'editscoped@example.com' }],
roleBindings: [{ role: 'edit', resource: 'servers', name: 'my-home-assistant' }],
},
];
// ── Mock factories ──
function mockServerRepo(): IMcpServerRepository {
return {
findAll: vi.fn(async () => [...SERVERS]),
findById: vi.fn(async (id: string) => SERVERS.find((s) => s.id === id) ?? null),
findByName: vi.fn(async (name: string) => SERVERS.find((s) => s.name === name) ?? null),
create: vi.fn(async () => SERVERS[0]!),
update: vi.fn(async () => SERVERS[0]!),
delete: vi.fn(async () => {}),
};
}
function mockRbacRepo(): IRbacDefinitionRepository {
return {
findAll: vi.fn(async () => [...RBAC_DEFS]),
findById: vi.fn(async () => null),
findByName: vi.fn(async () => null),
create: vi.fn(async () => RBAC_DEFS[0]!),
update: vi.fn(async () => RBAC_DEFS[0]!),
delete: vi.fn(async () => {}),
};
}
function mockPrisma(): PrismaClient {
return {
user: {
findUnique: vi.fn(async ({ where }: { where: { id: string } }) => {
const u = USERS[where.id];
return u ? { email: u.email } : null;
}),
},
groupMember: {
findMany: vi.fn(async () => []),
},
} as unknown as PrismaClient;
}
function stubInstanceRepo(): IMcpInstanceRepository {
return {
findAll: vi.fn(async () => []),
findById: vi.fn(async () => null),
findByContainerId: vi.fn(async () => null),
create: vi.fn(async (data) => ({
id: 'inst-stub', serverId: data.serverId, containerId: null,
status: data.status ?? 'STOPPED', port: null, metadata: {},
healthStatus: null, lastHealthCheck: null, events: [],
version: 1, createdAt: new Date(), updatedAt: new Date(),
}) as never),
updateStatus: vi.fn(async () => ({}) as never),
delete: vi.fn(async () => {}),
};
}
function stubOrchestrator(): McpOrchestrator {
return {
ping: vi.fn(async () => true),
pullImage: vi.fn(async () => {}),
createContainer: vi.fn(async () => ({ containerId: 'ctr', name: 'stub', state: 'running' as const, port: 3000, createdAt: new Date() })),
stopContainer: vi.fn(async () => {}),
removeContainer: vi.fn(async () => {}),
inspectContainer: vi.fn(async () => ({ containerId: 'ctr', name: 'stub', state: 'running' as const, createdAt: new Date() })),
getContainerLogs: vi.fn(async () => ({ stdout: '', stderr: '' })),
};
}
// ── App setup (replicates main.ts hooks) ──
import { normalizeResource } from '../src/validation/rbac-definition.schema.js';
import type { RbacAction } from '../src/services/rbac.service.js';
type PermissionCheck =
| { kind: 'resource'; resource: string; action: RbacAction; resourceName?: string }
| { kind: 'operation'; operation: string }
| { kind: 'skip' };
function mapUrlToPermission(method: string, url: string): PermissionCheck {
const match = url.match(/^\/api\/v1\/([a-z-]+)/);
if (!match) return { kind: 'skip' };
const segment = match[1] as string;
if (segment === 'backup') return { kind: 'operation', operation: 'backup' };
if (segment === 'restore') return { kind: 'operation', operation: 'restore' };
if (segment === 'audit-logs' && method === 'DELETE') return { kind: 'operation', operation: 'audit-purge' };
const resourceMap: Record<string, string | undefined> = {
servers: 'servers', instances: 'instances', secrets: 'secrets',
projects: 'projects', templates: 'templates', users: 'users',
groups: 'groups', rbac: 'rbac', 'audit-logs': 'rbac', mcp: 'servers',
};
const resource = resourceMap[segment];
if (resource === undefined) return { kind: 'skip' };
let action: RbacAction;
switch (method) {
case 'GET': case 'HEAD': action = 'view'; break;
case 'POST': action = 'create'; break;
case 'DELETE': action = 'delete'; break;
default: action = 'edit'; break;
}
const nameMatch = url.match(/^\/api\/v1\/[a-z-]+\/([^/?]+)/);
const resourceName = nameMatch?.[1];
const check: PermissionCheck = { kind: 'resource', resource, action };
if (resourceName !== undefined) (check as { resourceName: string }).resourceName = resourceName;
return check;
}
let app: FastifyInstance;
afterEach(async () => {
if (app) await app.close();
});
async function createTestApp() {
const serverRepo = mockServerRepo();
const rbacRepo = mockRbacRepo();
const prisma = mockPrisma();
const rbacService = new RbacService(rbacRepo, prisma);
const CUID_RE = /^c[^\s-]{8,}$/i;
const nameResolvers: Record<string, { findById(id: string): Promise<{ name: string } | null> }> = {
servers: serverRepo,
};
app = Fastify({ logger: false });
app.setErrorHandler(errorHandler);
// Auth hook (mock)
app.addHook('preHandler', async (request, reply) => {
const url = request.url;
if (url.startsWith('/api/v1/auth/') || url === '/healthz') return;
if (!url.startsWith('/api/v1/')) return;
const header = request.headers.authorization;
if (!header?.startsWith('Bearer ')) {
reply.code(401).send({ error: 'Unauthorized' });
return;
}
const token = header.slice(7);
const session = SESSIONS[token];
if (!session) {
reply.code(401).send({ error: 'Invalid token' });
return;
}
request.userId = session.userId;
});
// RBAC hook (replicates main.ts)
app.addHook('preHandler', async (request, reply) => {
if (reply.sent) return;
const url = request.url;
if (url.startsWith('/api/v1/auth/') || url === '/healthz') return;
if (!url.startsWith('/api/v1/')) return;
if (request.userId === undefined) return;
const check = mapUrlToPermission(request.method, url);
if (check.kind === 'skip') return;
let allowed: boolean;
if (check.kind === 'operation') {
allowed = await rbacService.canRunOperation(request.userId, check.operation);
} else {
// CUID→name resolution
if (check.resourceName !== undefined && CUID_RE.test(check.resourceName)) {
const resolver = nameResolvers[check.resource];
if (resolver) {
const entity = await resolver.findById(check.resourceName);
if (entity) check.resourceName = entity.name;
}
}
allowed = await rbacService.canAccess(request.userId, check.action, check.resource, check.resourceName);
// Compute scope for list filtering
if (allowed && check.resourceName === undefined) {
request.rbacScope = await rbacService.getAllowedScope(request.userId, check.action, check.resource);
}
}
if (!allowed) {
reply.code(403).send({ error: 'Forbidden' });
}
});
// Routes
const serverService = new McpServerService(serverRepo);
const instanceService = new InstanceService(stubInstanceRepo(), serverRepo, stubOrchestrator());
serverService.setInstanceService(instanceService);
registerMcpServerRoutes(app, serverService, instanceService);
// preSerialization hook (list filtering)
app.addHook('preSerialization', async (request, _reply, payload) => {
if (!request.rbacScope || request.rbacScope.wildcard) return payload;
if (!Array.isArray(payload)) return payload;
return (payload as Array<Record<string, unknown>>).filter((item) => {
const name = item['name'];
return typeof name === 'string' && request.rbacScope!.names.has(name);
});
});
await app.ready();
return app;
}
// ── Tests ──
describe('RBAC name-scoped integration (reproduces mcpctl bugs)', () => {
beforeEach(async () => {
await createTestApp();
});
describe('Bug 1: mcpctl get servers (list filtering)', () => {
it('name-scoped user sees ONLY their permitted server', async () => {
const res = await app.inject({
method: 'GET',
url: '/api/v1/servers',
headers: { authorization: 'Bearer scoped-token' },
});
expect(res.statusCode).toBe(200);
const servers = res.json<Array<{ name: string }>>();
expect(servers).toHaveLength(1);
expect(servers[0]!.name).toBe('my-home-assistant');
});
it('wildcard user sees ALL servers', async () => {
const res = await app.inject({
method: 'GET',
url: '/api/v1/servers',
headers: { authorization: 'Bearer admin-token' },
});
expect(res.statusCode).toBe(200);
const servers = res.json<Array<{ name: string }>>();
expect(servers).toHaveLength(3);
});
it('user with multiple name-scoped bindings sees only those servers', async () => {
const res = await app.inject({
method: 'GET',
url: '/api/v1/servers',
headers: { authorization: 'Bearer multi-scoped-token' },
});
expect(res.statusCode).toBe(200);
const servers = res.json<Array<{ name: string }>>();
expect(servers).toHaveLength(2);
const names = servers.map((s) => s.name);
expect(names).toContain('my-home-assistant');
expect(names).toContain('slack-server');
expect(names).not.toContain('github-server');
});
it('user with no server permissions gets 403', async () => {
const res = await app.inject({
method: 'GET',
url: '/api/v1/servers',
headers: { authorization: 'Bearer secrets-only-token' },
});
expect(res.statusCode).toBe(403);
});
});
describe('Bug 2: mcpctl get server NAME (CUID resolution)', () => {
it('allows access when URL contains CUID matching a name-scoped binding', async () => {
// CLI resolves my-home-assistant → clxyz000000001
const res = await app.inject({
method: 'GET',
url: '/api/v1/servers/clxyz000000001',
headers: { authorization: 'Bearer scoped-token' },
});
expect(res.statusCode).toBe(200);
expect(res.json<{ name: string }>().name).toBe('my-home-assistant');
});
it('denies access when CUID resolves to server NOT in binding', async () => {
const res = await app.inject({
method: 'GET',
url: '/api/v1/servers/clxyz000000002',
headers: { authorization: 'Bearer scoped-token' },
});
expect(res.statusCode).toBe(403);
});
it('passes RBAC when URL has human-readable name (route 404 is expected)', async () => {
// Human name in URL: RBAC passes (matches binding directly),
// but the route only does findById, so it 404s.
// CLI always resolves name→CUID first, so this doesn't happen in practice.
// The important thing: it does NOT return 403.
const res = await app.inject({
method: 'GET',
url: '/api/v1/servers/my-home-assistant',
headers: { authorization: 'Bearer scoped-token' },
});
expect(res.statusCode).toBe(404); // Not 403!
});
it('handles nonexistent CUID gracefully (403)', async () => {
const res = await app.inject({
method: 'GET',
url: '/api/v1/servers/cnonexistent12345678',
headers: { authorization: 'Bearer scoped-token' },
});
expect(res.statusCode).toBe(403);
});
it('wildcard user can access any server by CUID', async () => {
const res = await app.inject({
method: 'GET',
url: '/api/v1/servers/clxyz000000002',
headers: { authorization: 'Bearer admin-token' },
});
expect(res.statusCode).toBe(200);
expect(res.json<{ name: string }>().name).toBe('slack-server');
});
});
describe('name-scoped write operations', () => {
it('name-scoped edit user can DELETE their named server by CUID', async () => {
const res = await app.inject({
method: 'DELETE',
url: '/api/v1/servers/clxyz000000001',
headers: { authorization: 'Bearer edit-scoped-token' },
});
expect(res.statusCode).toBe(204);
});
it('name-scoped edit user CANNOT delete other servers', async () => {
const res = await app.inject({
method: 'DELETE',
url: '/api/v1/servers/clxyz000000002',
headers: { authorization: 'Bearer edit-scoped-token' },
});
expect(res.statusCode).toBe(403);
});
it('name-scoped view user CANNOT delete their named server', async () => {
const res = await app.inject({
method: 'DELETE',
url: '/api/v1/servers/clxyz000000001',
headers: { authorization: 'Bearer scoped-token' },
});
expect(res.statusCode).toBe(403);
});
});
describe('preSerialization edge cases', () => {
it('single-object responses pass through unmodified', async () => {
const res = await app.inject({
method: 'GET',
url: '/api/v1/servers/clxyz000000001',
headers: { authorization: 'Bearer scoped-token' },
});
expect(res.statusCode).toBe(200);
expect(res.json<{ name: string }>().name).toBe('my-home-assistant');
});
it('unauthenticated requests get 401', async () => {
const res = await app.inject({
method: 'GET',
url: '/api/v1/servers',
});
expect(res.statusCode).toBe(401);
});
});
});

View File

@@ -681,6 +681,199 @@ describe('RbacService', () => {
});
});
describe('getAllowedScope', () => {
describe('unscoped binding → wildcard', () => {
it('returns wildcard:true for matching resource', async () => {
const repo = mockRepo([
makeDef({
subjects: [{ kind: 'User', name: 'alice@example.com' }],
roleBindings: [{ role: 'view', resource: 'servers' }],
}),
]);
const prisma = mockPrisma({
user: { findUnique: vi.fn(async () => ({ email: 'alice@example.com' })) },
groupMember: { findMany: vi.fn(async () => []) },
});
const service = new RbacService(repo, prisma);
const scope = await service.getAllowedScope('user-1', 'view', 'servers');
expect(scope.wildcard).toBe(true);
expect(scope.names.size).toBe(0);
});
it('returns wildcard:true with wildcard resource binding', async () => {
const repo = mockRepo([
makeDef({
subjects: [{ kind: 'User', name: 'alice@example.com' }],
roleBindings: [{ role: 'edit', resource: '*' }],
}),
]);
const prisma = mockPrisma({
user: { findUnique: vi.fn(async () => ({ email: 'alice@example.com' })) },
groupMember: { findMany: vi.fn(async () => []) },
});
const service = new RbacService(repo, prisma);
const scope = await service.getAllowedScope('user-1', 'view', 'servers');
expect(scope.wildcard).toBe(true);
});
});
describe('name-scoped binding → restricted', () => {
let service: RbacService;
beforeEach(() => {
const repo = mockRepo([
makeDef({
subjects: [{ kind: 'User', name: 'alice@example.com' }],
roleBindings: [{ role: 'view', resource: 'servers', name: 'my-ha' }],
}),
]);
const prisma = mockPrisma({
user: { findUnique: vi.fn(async () => ({ email: 'alice@example.com' })) },
groupMember: { findMany: vi.fn(async () => []) },
});
service = new RbacService(repo, prisma);
});
it('returns names containing the scoped name', async () => {
const scope = await service.getAllowedScope('user-1', 'view', 'servers');
expect(scope.wildcard).toBe(false);
expect(scope.names).toEqual(new Set(['my-ha']));
});
it('returns empty names for wrong resource', async () => {
const scope = await service.getAllowedScope('user-1', 'view', 'secrets');
expect(scope.wildcard).toBe(false);
expect(scope.names.size).toBe(0);
});
it('returns empty names for wrong action', async () => {
const scope = await service.getAllowedScope('user-1', 'edit', 'servers');
expect(scope.wildcard).toBe(false);
expect(scope.names.size).toBe(0);
});
});
describe('multiple name-scoped bindings → union of names', () => {
it('collects names from multiple bindings', async () => {
const repo = mockRepo([
makeDef({
id: 'def-1',
name: 'rbac-a',
subjects: [{ kind: 'User', name: 'alice@example.com' }],
roleBindings: [{ role: 'view', resource: 'servers', name: 'server-a' }],
}),
makeDef({
id: 'def-2',
name: 'rbac-b',
subjects: [{ kind: 'User', name: 'alice@example.com' }],
roleBindings: [{ role: 'view', resource: 'servers', name: 'server-b' }],
}),
]);
const prisma = mockPrisma({
user: { findUnique: vi.fn(async () => ({ email: 'alice@example.com' })) },
groupMember: { findMany: vi.fn(async () => []) },
});
const service = new RbacService(repo, prisma);
const scope = await service.getAllowedScope('user-1', 'view', 'servers');
expect(scope.wildcard).toBe(false);
expect(scope.names).toEqual(new Set(['server-a', 'server-b']));
});
});
describe('mixed scoped + unscoped → wildcard wins', () => {
it('returns wildcard:true when any binding is unscoped', async () => {
const repo = mockRepo([
makeDef({
subjects: [{ kind: 'User', name: 'alice@example.com' }],
roleBindings: [
{ role: 'view', resource: 'servers', name: 'my-ha' },
{ role: 'view', resource: 'servers' },
],
}),
]);
const prisma = mockPrisma({
user: { findUnique: vi.fn(async () => ({ email: 'alice@example.com' })) },
groupMember: { findMany: vi.fn(async () => []) },
});
const service = new RbacService(repo, prisma);
const scope = await service.getAllowedScope('user-1', 'view', 'servers');
expect(scope.wildcard).toBe(true);
});
});
describe('no matching permissions → empty', () => {
it('returns wildcard:false with empty names', async () => {
const repo = mockRepo([]);
const prisma = mockPrisma();
const service = new RbacService(repo, prisma);
const scope = await service.getAllowedScope('unknown', 'view', 'servers');
expect(scope.wildcard).toBe(false);
expect(scope.names.size).toBe(0);
});
});
describe('edit role grants view scope', () => {
let service: RbacService;
beforeEach(() => {
const repo = mockRepo([
makeDef({
subjects: [{ kind: 'User', name: 'alice@example.com' }],
roleBindings: [{ role: 'edit', resource: 'servers', name: 'my-ha' }],
}),
]);
const prisma = mockPrisma({
user: { findUnique: vi.fn(async () => ({ email: 'alice@example.com' })) },
groupMember: { findMany: vi.fn(async () => []) },
});
service = new RbacService(repo, prisma);
});
it('returns names for view action', async () => {
const scope = await service.getAllowedScope('user-1', 'view', 'servers');
expect(scope.wildcard).toBe(false);
expect(scope.names).toEqual(new Set(['my-ha']));
});
it('returns names for create action', async () => {
const scope = await service.getAllowedScope('user-1', 'create', 'servers');
expect(scope.wildcard).toBe(false);
expect(scope.names).toEqual(new Set(['my-ha']));
});
it('returns names for delete action', async () => {
const scope = await service.getAllowedScope('user-1', 'delete', 'servers');
expect(scope.wildcard).toBe(false);
expect(scope.names).toEqual(new Set(['my-ha']));
});
});
describe('operation bindings are ignored', () => {
it('returns empty names when only operation bindings exist', async () => {
const repo = mockRepo([
makeDef({
subjects: [{ kind: 'User', name: 'alice@example.com' }],
roleBindings: [{ role: 'run', action: 'logs' }],
}),
]);
const prisma = mockPrisma({
user: { findUnique: vi.fn(async () => ({ email: 'alice@example.com' })) },
groupMember: { findMany: vi.fn(async () => []) },
});
const service = new RbacService(repo, prisma);
const scope = await service.getAllowedScope('user-1', 'view', 'servers');
expect(scope.wildcard).toBe(false);
expect(scope.names.size).toBe(0);
});
});
});
describe('unknown/legacy roles are denied', () => {
let service: RbacService;