merge: integrate v7-rbac-visibility on top of skills feature
Brings together the two unmerged feature lines onto one integration branch: - skills/review/proposals/revisions (via fix/mcpd-instance-health-and-retry, which contains the full skills-1..7 chain + mcpd env/retry/readiness fix) - v7 visibility scope + ownership for Llms and Agents Auto-merge resolved schema.prisma, mcpd main.ts, mcplocal config, CLI create, and completions with no conflicts. Both Prisma migrations coexist. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -787,14 +787,48 @@ async function main(): Promise<void> {
|
||||
registerGitBackupRoutes(app, gitBackup);
|
||||
|
||||
// ── RBAC list filtering hook ──
|
||||
// Filters array responses to only include resources the user is allowed to see.
|
||||
//
|
||||
// Two filters compose here, in order:
|
||||
//
|
||||
// 1. RBAC name-scope (existing): when a caller has only name-scoped
|
||||
// grants (no resource-wide), only items whose name is in their
|
||||
// grants set pass through. wildcard=true skips this step.
|
||||
//
|
||||
// 2. v7 visibility filter (new): private rows are hidden from
|
||||
// callers who aren't the owner and don't have a name-scoped
|
||||
// grant. Skipped for `*`-resource admins (isAdmin=true) so
|
||||
// org-wide audit/list operations still work. Importantly, this
|
||||
// runs even when wildcard=true — that's how a regular `view:llms`
|
||||
// grant stops broadcasting private virtuals across the org while
|
||||
// still letting `*`-grant admins see everything.
|
||||
//
|
||||
// Items without a `visibility` field (today: every resource other
|
||||
// than Llm and Agent) pass through this step unchanged.
|
||||
app.addHook('preSerialization', async (request, _reply, payload) => {
|
||||
if (!request.rbacScope || request.rbacScope.wildcard) return payload;
|
||||
if (!request.rbacScope) 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);
|
||||
});
|
||||
let items = payload as Array<Record<string, unknown>>;
|
||||
// Step 1: RBAC name-scope.
|
||||
if (!request.rbacScope.wildcard) {
|
||||
items = items.filter((item) => {
|
||||
const name = item['name'];
|
||||
return typeof name === 'string' && request.rbacScope!.names.has(name);
|
||||
});
|
||||
}
|
||||
// Step 2: visibility (only meaningful when the resource carries it).
|
||||
if (!request.rbacScope.isAdmin) {
|
||||
const userId = request.userId;
|
||||
items = items.filter((item) => {
|
||||
const visibility = item['visibility'];
|
||||
if (visibility !== 'private') return true;
|
||||
const ownerId = item['ownerId'];
|
||||
if (typeof ownerId === 'string' && ownerId === userId) return true;
|
||||
const name = item['name'];
|
||||
if (typeof name === 'string' && request.rbacScope!.names.has(name)) return true;
|
||||
return false;
|
||||
});
|
||||
}
|
||||
return items;
|
||||
});
|
||||
|
||||
// Web UI: served from /ui (static SPA bundle). Falls through to API
|
||||
|
||||
@@ -33,7 +33,14 @@ export interface AuthDeps {
|
||||
declare module 'fastify' {
|
||||
interface FastifyRequest {
|
||||
userId?: string;
|
||||
rbacScope?: { wildcard: boolean; names: Set<string> };
|
||||
/**
|
||||
* v7: extended with `isAdmin` to distinguish `*` (cross-resource
|
||||
* admin) grants from plain `view:llms` resource-wide grants. The
|
||||
* preSerialization filter and the route-level Viewer use this to
|
||||
* decide whether to skip the visibility filter (admins bypass;
|
||||
* regular wildcard does not).
|
||||
*/
|
||||
rbacScope?: { wildcard: boolean; isAdmin: boolean; names: Set<string> };
|
||||
/** Set by the auth hook when the caller authenticated via a McpToken bearer (prefix `mcpctl_pat_`). */
|
||||
mcpToken?: McpTokenPrincipal;
|
||||
}
|
||||
|
||||
@@ -11,6 +11,8 @@ export interface CreateAgentRepoInput {
|
||||
defaultParams?: Record<string, unknown>;
|
||||
extras?: Record<string, unknown>;
|
||||
ownerId: string;
|
||||
// v7: optional visibility scope (default 'public' if omitted).
|
||||
visibility?: 'public' | 'private';
|
||||
// Virtual-agent lifecycle (omit for kind=public).
|
||||
kind?: LlmKind;
|
||||
providerSessionId?: string | null;
|
||||
@@ -28,6 +30,9 @@ export interface UpdateAgentRepoInput {
|
||||
proxyModelName?: string | null;
|
||||
defaultParams?: Record<string, unknown>;
|
||||
extras?: Record<string, unknown>;
|
||||
// v7: visibility is mutable (operator can flip a private virtual to
|
||||
// public for org-wide sharing without re-creating).
|
||||
visibility?: 'public' | 'private';
|
||||
// Virtual-agent lifecycle. AgentService is the only public writer; the
|
||||
// VirtualAgentService methods (Stage 2) bypass the public CRUD path.
|
||||
kind?: LlmKind;
|
||||
@@ -87,6 +92,7 @@ export class AgentRepository implements IAgentRepository {
|
||||
defaultParams: (data.defaultParams ?? {}) as Prisma.InputJsonValue,
|
||||
extras: (data.extras ?? {}) as Prisma.InputJsonValue,
|
||||
ownerId: data.ownerId,
|
||||
...(data.visibility !== undefined ? { visibility: data.visibility } : {}),
|
||||
...(data.kind !== undefined ? { kind: data.kind } : {}),
|
||||
...(data.providerSessionId !== undefined ? { providerSessionId: data.providerSessionId } : {}),
|
||||
...(data.status !== undefined ? { status: data.status } : {}),
|
||||
@@ -122,6 +128,7 @@ export class AgentRepository implements IAgentRepository {
|
||||
if (data.extras !== undefined) {
|
||||
updateData.extras = data.extras as Prisma.InputJsonValue;
|
||||
}
|
||||
if (data.visibility !== undefined) updateData.visibility = data.visibility;
|
||||
if (data.kind !== undefined) updateData.kind = data.kind;
|
||||
if (data.providerSessionId !== undefined) updateData.providerSessionId = data.providerSessionId;
|
||||
if (data.status !== undefined) updateData.status = data.status;
|
||||
|
||||
@@ -12,6 +12,11 @@ export interface CreateLlmInput {
|
||||
extraConfig?: Record<string, unknown>;
|
||||
// v4: optional pool key. NULL = "pool of 1" (effective key falls back to `name`).
|
||||
poolName?: string | null;
|
||||
// v7: per-user RBAC scoping. ownerId is set by the service layer to
|
||||
// the authenticated caller's User.id; visibility defaults to 'public'
|
||||
// and gets flipped to 'private' for mcplocal-published virtuals.
|
||||
ownerId?: string | null;
|
||||
visibility?: 'public' | 'private';
|
||||
// Virtual-provider lifecycle (omit for kind=public).
|
||||
kind?: LlmKind;
|
||||
providerSessionId?: string | null;
|
||||
@@ -30,6 +35,10 @@ export interface UpdateLlmInput {
|
||||
apiKeySecretKey?: string | null;
|
||||
extraConfig?: Record<string, unknown>;
|
||||
poolName?: string | null;
|
||||
// v7: ownerId immutable at update time (use a separate transfer flow if
|
||||
// ever needed). Visibility is mutable so an operator can flip a
|
||||
// virtual Llm to public for org-wide sharing without re-creating it.
|
||||
visibility?: 'public' | 'private';
|
||||
// Virtual-provider lifecycle. VirtualLlmService is the only writer for
|
||||
// these in v1; the public CRUD path leaves them undefined.
|
||||
kind?: LlmKind;
|
||||
@@ -108,6 +117,8 @@ export class LlmRepository implements ILlmRepository {
|
||||
apiKeySecretKey: data.apiKeySecretKey ?? null,
|
||||
extraConfig: (data.extraConfig ?? {}) as Prisma.InputJsonValue,
|
||||
...(data.poolName !== undefined ? { poolName: data.poolName } : {}),
|
||||
...(data.ownerId !== undefined ? { ownerId: data.ownerId } : {}),
|
||||
...(data.visibility !== undefined ? { visibility: data.visibility } : {}),
|
||||
...(data.kind !== undefined ? { kind: data.kind } : {}),
|
||||
...(data.providerSessionId !== undefined ? { providerSessionId: data.providerSessionId } : {}),
|
||||
...(data.status !== undefined ? { status: data.status } : {}),
|
||||
@@ -132,6 +143,7 @@ export class LlmRepository implements ILlmRepository {
|
||||
if (data.apiKeySecretKey !== undefined) updateData.apiKeySecretKey = data.apiKeySecretKey;
|
||||
if (data.extraConfig !== undefined) updateData.extraConfig = data.extraConfig as Prisma.InputJsonValue;
|
||||
if (data.poolName !== undefined) updateData.poolName = data.poolName;
|
||||
if (data.visibility !== undefined) updateData.visibility = data.visibility;
|
||||
if (data.kind !== undefined) updateData.kind = data.kind;
|
||||
if (data.providerSessionId !== undefined) updateData.providerSessionId = data.providerSessionId;
|
||||
if (data.status !== undefined) updateData.status = data.status;
|
||||
|
||||
@@ -6,21 +6,33 @@
|
||||
* — the resource is `agents`. The chat endpoints live in `agent-chat.ts` and
|
||||
* map to `run:agents:<name>`.
|
||||
*/
|
||||
import type { FastifyInstance } from 'fastify';
|
||||
import type { AgentService } from '../services/agent.service.js';
|
||||
import type { FastifyInstance, FastifyRequest } from 'fastify';
|
||||
import type { AgentService, AgentViewer } from '../services/agent.service.js';
|
||||
import { NotFoundError, ConflictError } from '../services/mcp-server.service.js';
|
||||
|
||||
/** v7: thread the request's RBAC scope into the service so foreign-private rows 404. */
|
||||
function viewerFromRequest(request: FastifyRequest): AgentViewer | null {
|
||||
if (request.userId === undefined || request.rbacScope === undefined) return null;
|
||||
return {
|
||||
userId: request.userId,
|
||||
wildcard: request.rbacScope.isAdmin,
|
||||
allowedNames: request.rbacScope.names,
|
||||
};
|
||||
}
|
||||
|
||||
export function registerAgentRoutes(
|
||||
app: FastifyInstance,
|
||||
service: AgentService,
|
||||
): void {
|
||||
app.get('/api/v1/agents', async () => {
|
||||
// List filter is applied by the preSerialization hook (visibility +
|
||||
// RBAC name-scope); service stays viewer-blind for internal callers.
|
||||
return service.list();
|
||||
});
|
||||
|
||||
app.get<{ Params: { id: string } }>('/api/v1/agents/:id', async (request, reply) => {
|
||||
try {
|
||||
return await getByIdOrName(service, request.params.id);
|
||||
return await getByIdOrName(service, request.params.id, viewerFromRequest(request));
|
||||
} catch (err) {
|
||||
if (err instanceof NotFoundError) {
|
||||
reply.code(404);
|
||||
@@ -51,7 +63,7 @@ export function registerAgentRoutes(
|
||||
|
||||
app.put<{ Params: { id: string } }>('/api/v1/agents/:id', async (request, reply) => {
|
||||
try {
|
||||
const target = await getByIdOrName(service, request.params.id);
|
||||
const target = await getByIdOrName(service, request.params.id, viewerFromRequest(request));
|
||||
return await service.update(target.id, request.body);
|
||||
} catch (err) {
|
||||
if (err instanceof NotFoundError) {
|
||||
@@ -64,7 +76,7 @@ export function registerAgentRoutes(
|
||||
|
||||
app.delete<{ Params: { id: string } }>('/api/v1/agents/:id', async (request, reply) => {
|
||||
try {
|
||||
const target = await getByIdOrName(service, request.params.id);
|
||||
const target = await getByIdOrName(service, request.params.id, viewerFromRequest(request));
|
||||
await service.delete(target.id);
|
||||
reply.code(204);
|
||||
return null;
|
||||
@@ -84,7 +96,7 @@ export function registerAgentRoutes(
|
||||
'/api/v1/projects/:projectName/agents',
|
||||
async (request, reply) => {
|
||||
try {
|
||||
return await service.listByProject(request.params.projectName);
|
||||
return await service.listByProject(request.params.projectName, viewerFromRequest(request));
|
||||
} catch (err) {
|
||||
if (err instanceof NotFoundError) {
|
||||
reply.code(404);
|
||||
@@ -98,9 +110,9 @@ export function registerAgentRoutes(
|
||||
|
||||
const CUID_RE = /^c[a-z0-9]{24}/i;
|
||||
|
||||
async function getByIdOrName(service: AgentService, idOrName: string) {
|
||||
async function getByIdOrName(service: AgentService, idOrName: string, viewer: AgentViewer | null = null) {
|
||||
if (CUID_RE.test(idOrName)) {
|
||||
return service.getById(idOrName);
|
||||
return service.getById(idOrName, viewer);
|
||||
}
|
||||
return service.getByName(idOrName);
|
||||
return service.getByName(idOrName, viewer);
|
||||
}
|
||||
|
||||
@@ -1,13 +1,32 @@
|
||||
import type { FastifyInstance } from 'fastify';
|
||||
import type { LlmService, LlmView } from '../services/llm.service.js';
|
||||
import type { FastifyInstance, FastifyRequest } from 'fastify';
|
||||
import type { LlmService, LlmView, Viewer } from '../services/llm.service.js';
|
||||
import { LlmAuthVerificationError, effectivePoolName } from '../services/llm.service.js';
|
||||
import { NotFoundError, ConflictError } from '../services/mcp-server.service.js';
|
||||
|
||||
/**
|
||||
* v7: build a Viewer from the request's RBAC scope so the service
|
||||
* applies the visibility filter consistently. The list endpoint relies
|
||||
* on the preSerialization hook for the same logic; for get-by-name/id
|
||||
* the service does the filter itself and 404s on a hidden row.
|
||||
*/
|
||||
function viewerFromRequest(request: FastifyRequest): Viewer | null {
|
||||
if (request.userId === undefined || request.rbacScope === undefined) return null;
|
||||
return {
|
||||
userId: request.userId,
|
||||
wildcard: request.rbacScope.isAdmin, // only admins skip the visibility filter
|
||||
allowedNames: request.rbacScope.names,
|
||||
};
|
||||
}
|
||||
|
||||
export function registerLlmRoutes(
|
||||
app: FastifyInstance,
|
||||
service: LlmService,
|
||||
): void {
|
||||
app.get('/api/v1/llms', async () => {
|
||||
// List goes through the preSerialization hook which applies both
|
||||
// RBAC name-scope and v7 visibility filter — service stays
|
||||
// viewer-blind here so internal callers (audit, sweeps) keep
|
||||
// working without a request context.
|
||||
return service.list();
|
||||
});
|
||||
|
||||
@@ -16,7 +35,7 @@ export function registerLlmRoutes(
|
||||
// hands over the user-facing name to avoid an extra round-trip).
|
||||
app.get<{ Params: { id: string } }>('/api/v1/llms/:id', async (request, reply) => {
|
||||
try {
|
||||
return await getByIdOrName(service, request.params.id);
|
||||
return await getByIdOrName(service, request.params.id, viewerFromRequest(request));
|
||||
} catch (err) {
|
||||
if (err instanceof NotFoundError) {
|
||||
reply.code(404);
|
||||
@@ -96,8 +115,20 @@ export function registerLlmRoutes(
|
||||
// size + activeCount.
|
||||
app.get<{ Params: { name: string } }>('/api/v1/llms/:name/members', async (request, reply) => {
|
||||
try {
|
||||
const anchor = await getByIdOrName(service, request.params.name);
|
||||
const members = await service.listPoolMembers(effectivePoolName(anchor));
|
||||
const viewer = viewerFromRequest(request);
|
||||
const anchor = await getByIdOrName(service, request.params.name, viewer);
|
||||
const allMembers = await service.listPoolMembers(effectivePoolName(anchor));
|
||||
// v7: filter pool members by visibility too — without this, a
|
||||
// pool with private members would leak their names through the
|
||||
// /members endpoint even though the list endpoint hides them.
|
||||
const members = viewer === null
|
||||
? allMembers
|
||||
: allMembers.filter((m) => {
|
||||
if (m.visibility !== 'private') return true;
|
||||
if (m.ownerId !== null && m.ownerId === viewer.userId) return true;
|
||||
if (viewer.allowedNames.has(m.name)) return true;
|
||||
return viewer.wildcard;
|
||||
});
|
||||
return {
|
||||
poolName: effectivePoolName(anchor),
|
||||
explicitPoolName: anchor.poolName,
|
||||
@@ -131,11 +162,12 @@ const CUID_RE = /^c[a-z0-9]{24}/i;
|
||||
/**
|
||||
* Look up by CUID first; if the input doesn't look like one, fall back to
|
||||
* findByName. Lets the same URL serve both `mcpctl describe llm <name>` and
|
||||
* the FailoverRouter's name-based RBAC check.
|
||||
* the FailoverRouter's name-based RBAC check. v7: the optional viewer
|
||||
* threads through to the service so foreign-private rows 404 cleanly.
|
||||
*/
|
||||
async function getByIdOrName(service: LlmService, idOrName: string) {
|
||||
async function getByIdOrName(service: LlmService, idOrName: string, viewer: Viewer | null = null) {
|
||||
if (CUID_RE.test(idOrName)) {
|
||||
return service.getById(idOrName);
|
||||
return service.getById(idOrName, viewer);
|
||||
}
|
||||
return service.getByName(idOrName);
|
||||
return service.getByName(idOrName, viewer);
|
||||
}
|
||||
|
||||
@@ -45,9 +45,14 @@ export function registerVirtualLlmRoutes(
|
||||
const agentsInput = Array.isArray(body.agents) ? body.agents : null;
|
||||
|
||||
try {
|
||||
// v7: ownerId from the authenticated request lands on every
|
||||
// newly-created virtual Llm row (sticky reconnects update the
|
||||
// existing row's ownerId too — same publisher, same session).
|
||||
const ownerId = request.userId ?? 'system';
|
||||
const result = await service.register({
|
||||
providerSessionId: body.providerSessionId ?? null,
|
||||
providers: providers.map(coerceProviderInput),
|
||||
ownerId,
|
||||
});
|
||||
// v3: atomically publish virtual agents tied to the same session.
|
||||
// If the caller didn't include an agents array, skip silently.
|
||||
@@ -189,6 +194,10 @@ function coerceAgentInput(raw: unknown): VirtualAgentInput {
|
||||
if (o['extras'] !== null && typeof o['extras'] === 'object') {
|
||||
out.extras = o['extras'] as Record<string, unknown>;
|
||||
}
|
||||
// v7: optional visibility scope (defaults to 'private' on first publish).
|
||||
if (o['visibility'] === 'public' || o['visibility'] === 'private') {
|
||||
out.visibility = o['visibility'];
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
@@ -202,6 +211,7 @@ function coerceProviderInput(raw: unknown): {
|
||||
extraConfig?: Record<string, unknown>;
|
||||
initialStatus?: 'active' | 'hibernating';
|
||||
poolName?: string;
|
||||
visibility?: 'public' | 'private';
|
||||
} {
|
||||
if (raw === null || typeof raw !== 'object') {
|
||||
throw Object.assign(new Error('provider entry must be an object'), { statusCode: 400 });
|
||||
@@ -234,5 +244,10 @@ function coerceProviderInput(raw: unknown): {
|
||||
if (typeof o['poolName'] === 'string' && /^[a-z0-9-]+$/.test(o['poolName']) && o['poolName'].length >= 1 && o['poolName'].length <= 100) {
|
||||
out.poolName = o['poolName'];
|
||||
}
|
||||
// v7: visibility. Only 'public' or 'private'; unknown values fall
|
||||
// through to the service default ('private' for virtuals).
|
||||
if (o['visibility'] === 'public' || o['visibility'] === 'private') {
|
||||
out.visibility = o['visibility'];
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
@@ -21,6 +21,29 @@ import {
|
||||
} from '../validation/agent.schema.js';
|
||||
import { NotFoundError, ConflictError } from './mcp-server.service.js';
|
||||
|
||||
/**
|
||||
* v7: visibility scope for the current request, mirrors LlmService.Viewer.
|
||||
* Same semantics — null = no filter; wildcard = full grant; else
|
||||
* filter to public-or-owned-or-name-scoped.
|
||||
*/
|
||||
export interface AgentViewer {
|
||||
userId: string;
|
||||
wildcard: boolean;
|
||||
allowedNames: Set<string>;
|
||||
}
|
||||
|
||||
export function isAgentVisibleTo(
|
||||
row: { name: string; ownerId: string; visibility: string },
|
||||
viewer: AgentViewer | null,
|
||||
): boolean {
|
||||
if (viewer === null) return true;
|
||||
if (viewer.wildcard) return true;
|
||||
if (row.visibility !== 'private') return true;
|
||||
if (row.ownerId === viewer.userId) return true;
|
||||
if (viewer.allowedNames.has(row.name)) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
/** Shape returned by the API layer — embeds llm + project metadata. */
|
||||
export interface AgentView {
|
||||
id: string;
|
||||
@@ -39,6 +62,8 @@ export interface AgentView {
|
||||
lastHeartbeatAt: Date | null;
|
||||
inactiveSince: Date | null;
|
||||
ownerId: string;
|
||||
/** v7: per-user RBAC scoping. mcplocal-published virtuals default to 'private'. */
|
||||
visibility: 'public' | 'private';
|
||||
version: number;
|
||||
createdAt: Date;
|
||||
updatedAt: Date;
|
||||
@@ -53,6 +78,12 @@ export interface VirtualAgentInput {
|
||||
project?: string;
|
||||
defaultParams?: Record<string, unknown>;
|
||||
extras?: Record<string, unknown>;
|
||||
/**
|
||||
* v7: per-user RBAC scoping. When omitted, virtual agents default to
|
||||
* 'private' on register — same shape as virtual Llms. The publisher
|
||||
* can override per-agent in mcplocal config.
|
||||
*/
|
||||
visibility?: 'public' | 'private';
|
||||
}
|
||||
|
||||
export class AgentService {
|
||||
@@ -63,26 +94,30 @@ export class AgentService {
|
||||
private readonly personalities?: IPersonalityRepository,
|
||||
) {}
|
||||
|
||||
async list(): Promise<AgentView[]> {
|
||||
async list(viewer: AgentViewer | null = null): Promise<AgentView[]> {
|
||||
const rows = await this.repo.findAll();
|
||||
return Promise.all(rows.map((r) => this.toView(r)));
|
||||
const visible = rows.filter((r) => isAgentVisibleTo(r, viewer));
|
||||
return Promise.all(visible.map((r) => this.toView(r)));
|
||||
}
|
||||
|
||||
async listByProject(projectName: string): Promise<AgentView[]> {
|
||||
async listByProject(projectName: string, viewer: AgentViewer | null = null): Promise<AgentView[]> {
|
||||
const project = await this.projects.resolveAndGet(projectName);
|
||||
const rows = await this.repo.findByProjectId(project.id);
|
||||
return Promise.all(rows.map((r) => this.toView(r)));
|
||||
const visible = rows.filter((r) => isAgentVisibleTo(r, viewer));
|
||||
return Promise.all(visible.map((r) => this.toView(r)));
|
||||
}
|
||||
|
||||
async getById(id: string): Promise<AgentView> {
|
||||
async getById(id: string, viewer: AgentViewer | null = null): Promise<AgentView> {
|
||||
const row = await this.repo.findById(id);
|
||||
if (row === null) throw new NotFoundError(`Agent not found: ${id}`);
|
||||
if (!isAgentVisibleTo(row, viewer)) throw new NotFoundError(`Agent not found: ${id}`);
|
||||
return this.toView(row);
|
||||
}
|
||||
|
||||
async getByName(name: string): Promise<AgentView> {
|
||||
async getByName(name: string, viewer: AgentViewer | null = null): Promise<AgentView> {
|
||||
const row = await this.repo.findByName(name);
|
||||
if (row === null) throw new NotFoundError(`Agent not found: ${name}`);
|
||||
if (!isAgentVisibleTo(row, viewer)) throw new NotFoundError(`Agent not found: ${name}`);
|
||||
return this.toView(row);
|
||||
}
|
||||
|
||||
@@ -200,6 +235,7 @@ export class AgentService {
|
||||
lastHeartbeatAt: row.lastHeartbeatAt,
|
||||
inactiveSince: row.inactiveSince,
|
||||
ownerId: row.ownerId,
|
||||
visibility: (row.visibility === 'private' ? 'private' : 'public') as 'public' | 'private',
|
||||
version: row.version,
|
||||
createdAt: row.createdAt,
|
||||
updatedAt: row.updatedAt,
|
||||
@@ -262,6 +298,10 @@ export class AgentService {
|
||||
projectId,
|
||||
...(a.defaultParams !== undefined ? { defaultParams: a.defaultParams } : {}),
|
||||
...(a.extras !== undefined ? { extras: a.extras } : {}),
|
||||
// v7: only update visibility on sticky reconnect when the
|
||||
// publisher explicitly sent it — operators may have flipped
|
||||
// a virtual agent to public manually via `mcpctl edit agent`.
|
||||
...(a.visibility !== undefined ? { visibility: a.visibility } : {}),
|
||||
kind: 'virtual',
|
||||
providerSessionId: sessionId,
|
||||
status: 'active',
|
||||
@@ -279,6 +319,8 @@ export class AgentService {
|
||||
projectId,
|
||||
...(a.defaultParams !== undefined ? { defaultParams: a.defaultParams } : {}),
|
||||
...(a.extras !== undefined ? { extras: a.extras } : {}),
|
||||
// v7: virtual agents default to private on first publish.
|
||||
visibility: a.visibility ?? 'private',
|
||||
kind: 'virtual',
|
||||
providerSessionId: sessionId,
|
||||
status: 'active',
|
||||
|
||||
@@ -57,6 +57,17 @@ export interface LlmView {
|
||||
* expands at request time.
|
||||
*/
|
||||
poolName: string | null;
|
||||
/**
|
||||
* v7: per-user RBAC scoping. NULL `ownerId` on legacy rows means
|
||||
* "no recorded owner"; treated as public for visibility checks.
|
||||
*/
|
||||
ownerId: string | null;
|
||||
/**
|
||||
* v7: visibility scope. 'public' = anyone with the resource grant;
|
||||
* 'private' = owner + explicit name-scoped RBAC bindings only.
|
||||
* mcplocal virtuals default to 'private' on register.
|
||||
*/
|
||||
visibility: 'public' | 'private';
|
||||
// Virtual-provider lifecycle (kind defaults to 'public' for legacy rows).
|
||||
kind: 'public' | 'virtual';
|
||||
status: 'active' | 'inactive' | 'hibernating';
|
||||
@@ -77,6 +88,49 @@ export function effectivePoolName(row: { name: string; poolName: string | null }
|
||||
return row.poolName !== null && row.poolName !== '' ? row.poolName : row.name;
|
||||
}
|
||||
|
||||
/**
|
||||
* v7: visibility scope for the current request. The route layer
|
||||
* computes this from `request.userId` + `RbacService.getAllowedScope`
|
||||
* and passes it down to LlmService for filtering. When `wildcard` is
|
||||
* true (admin / resource-wide `view:llms`), no filter applies — every
|
||||
* row is visible regardless of owner / private flag. When false, the
|
||||
* filter is `visibility='public' OR ownerId=userId OR name in allowedNames`.
|
||||
*
|
||||
* `null` Viewer means "skip the v7 filter entirely" — used by internal
|
||||
* callers (cron sweeps, audit collectors) and tests that don't have a
|
||||
* request context.
|
||||
*/
|
||||
export interface Viewer {
|
||||
userId: string;
|
||||
/** True when the caller has resource-wide `view:llms` (or admin). */
|
||||
wildcard: boolean;
|
||||
/** Name-scoped grants the caller holds (e.g. `view:llms:vllm-alice`). */
|
||||
allowedNames: Set<string>;
|
||||
}
|
||||
|
||||
/**
|
||||
* v7: shared predicate. A row is visible to the viewer when:
|
||||
* - visibility is public AND row passes RBAC layer above (caller already has resource grant), OR
|
||||
* - viewer is null (internal call, no filter), OR
|
||||
* - viewer has wildcard grant, OR
|
||||
* - viewer is the owner, OR
|
||||
* - the row's name is in viewer.allowedNames (name-scoped grant).
|
||||
*
|
||||
* Pure function so service tests can exercise it directly without a
|
||||
* full mock RbacService.
|
||||
*/
|
||||
export function isLlmVisibleTo(
|
||||
row: { name: string; ownerId: string | null; visibility: string },
|
||||
viewer: Viewer | null,
|
||||
): boolean {
|
||||
if (viewer === null) return true;
|
||||
if (viewer.wildcard) return true;
|
||||
if (row.visibility !== 'private') return true;
|
||||
if (row.ownerId !== null && row.ownerId === viewer.userId) return true;
|
||||
if (viewer.allowedNames.has(row.name)) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
export class LlmService {
|
||||
constructor(
|
||||
private readonly repo: ILlmRepository,
|
||||
@@ -84,20 +138,25 @@ export class LlmService {
|
||||
private readonly verifyDeps: LlmServiceDeps = {},
|
||||
) {}
|
||||
|
||||
async list(): Promise<LlmView[]> {
|
||||
async list(viewer: Viewer | null = null): Promise<LlmView[]> {
|
||||
const rows = await this.repo.findAll();
|
||||
return Promise.all(rows.map((r) => this.toView(r)));
|
||||
const visible = rows.filter((r) => isLlmVisibleTo(r, viewer));
|
||||
return Promise.all(visible.map((r) => this.toView(r)));
|
||||
}
|
||||
|
||||
async getById(id: string): Promise<LlmView> {
|
||||
async getById(id: string, viewer: Viewer | null = null): Promise<LlmView> {
|
||||
const row = await this.repo.findById(id);
|
||||
if (row === null) throw new NotFoundError(`Llm not found: ${id}`);
|
||||
// 404 (not 403) on a foreign-private row prevents id-enumeration —
|
||||
// identical shape to the chat-thread + inference-task routes.
|
||||
if (!isLlmVisibleTo(row, viewer)) throw new NotFoundError(`Llm not found: ${id}`);
|
||||
return this.toView(row);
|
||||
}
|
||||
|
||||
async getByName(name: string): Promise<LlmView> {
|
||||
async getByName(name: string, viewer: Viewer | null = null): Promise<LlmView> {
|
||||
const row = await this.repo.findByName(name);
|
||||
if (row === null) throw new NotFoundError(`Llm not found: ${name}`);
|
||||
if (!isLlmVisibleTo(row, viewer)) throw new NotFoundError(`Llm not found: ${name}`);
|
||||
return this.toView(row);
|
||||
}
|
||||
|
||||
@@ -326,6 +385,8 @@ export class LlmService {
|
||||
apiKeyRef,
|
||||
extraConfig: row.extraConfig as Record<string, unknown>,
|
||||
poolName: row.poolName,
|
||||
ownerId: row.ownerId,
|
||||
visibility: (row.visibility === 'private' ? 'private' : 'public') as 'public' | 'private',
|
||||
kind: row.kind,
|
||||
status: row.status,
|
||||
lastHeartbeatAt: row.lastHeartbeatAt,
|
||||
|
||||
@@ -24,7 +24,17 @@ export interface OperationPermission {
|
||||
export type Permission = ResourcePermission | OperationPermission;
|
||||
|
||||
export interface AllowedScope {
|
||||
/** True iff the user has a resource-wide grant for this resource (no name constraint). */
|
||||
wildcard: boolean;
|
||||
/**
|
||||
* v7: true iff the user has `*` (cross-resource admin) grant. The
|
||||
* visibility filter (per-user RBAC scoping for virtual Llms/Agents)
|
||||
* skips itself when this is set — admins see private rows from any
|
||||
* owner, just like before v7. Plain `view:llms` resource-wide grants
|
||||
* set `wildcard=true` but `isAdmin=false`, so private rows from other
|
||||
* users are still hidden in the list view.
|
||||
*/
|
||||
isAdmin: boolean;
|
||||
names: Set<string>;
|
||||
}
|
||||
|
||||
@@ -97,6 +107,8 @@ export class RbacService {
|
||||
const permissions = await this.getPermissions(userId, serviceAccountName, mcpTokenSha);
|
||||
const normalized = normalizeResource(resource);
|
||||
const names = new Set<string>();
|
||||
let wildcard = false;
|
||||
let isAdmin = false;
|
||||
|
||||
for (const perm of permissions) {
|
||||
if (!('resource' in perm)) continue;
|
||||
@@ -105,12 +117,22 @@ export class RbacService {
|
||||
if (!actions.includes(action)) continue;
|
||||
const permResource = normalizeResource(perm.resource);
|
||||
if (permResource !== '*' && permResource !== normalized) continue;
|
||||
// Unscoped binding → wildcard access to this resource
|
||||
if (perm.name === undefined) return { wildcard: true, names: new Set() };
|
||||
names.add(perm.name);
|
||||
// Unscoped binding → wildcard access to this resource. v7: also
|
||||
// record whether the binding came from a `*` (cross-resource
|
||||
// admin) grant — that's the only one that bypasses the
|
||||
// visibility filter for private rows from other owners.
|
||||
if (perm.name === undefined) {
|
||||
wildcard = true;
|
||||
if (permResource === '*') isAdmin = true;
|
||||
// Don't return early — keep scanning so isAdmin can flip true
|
||||
// even if a more-specific binding matched first.
|
||||
} else {
|
||||
names.add(perm.name);
|
||||
}
|
||||
}
|
||||
|
||||
return { wildcard: false, names };
|
||||
if (wildcard) return { wildcard: true, isAdmin, names: new Set() };
|
||||
return { wildcard: false, isAdmin: false, names };
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -58,6 +58,12 @@ export interface RegisterProviderInput {
|
||||
* shares the `poolName` with siblings.
|
||||
*/
|
||||
poolName?: string;
|
||||
/**
|
||||
* v7: per-user RBAC scoping. When omitted, virtuals default to
|
||||
* 'private' (visible only to the publishing user). Set explicitly
|
||||
* to 'public' for org-wide sharing.
|
||||
*/
|
||||
visibility?: 'public' | 'private';
|
||||
}
|
||||
|
||||
export interface RegisterResult {
|
||||
@@ -134,7 +140,7 @@ export interface EnqueueInferOptions {
|
||||
}
|
||||
|
||||
export interface IVirtualLlmService {
|
||||
register(input: { providerSessionId?: string | null; providers: RegisterProviderInput[] }): Promise<RegisterResult>;
|
||||
register(input: { providerSessionId?: string | null; providers: RegisterProviderInput[]; ownerId?: string }): Promise<RegisterResult>;
|
||||
heartbeat(providerSessionId: string): Promise<void>;
|
||||
bindSession(providerSessionId: string, handle: VirtualSessionHandle): void;
|
||||
unbindSession(providerSessionId: string): Promise<void>;
|
||||
@@ -193,7 +199,7 @@ export class VirtualLlmService implements IVirtualLlmService {
|
||||
private readonly resolveOwner: () => string = () => 'system',
|
||||
) {}
|
||||
|
||||
async register(input: { providerSessionId?: string | null; providers: RegisterProviderInput[] }): Promise<RegisterResult> {
|
||||
async register(input: { providerSessionId?: string | null; providers: RegisterProviderInput[]; ownerId?: string }): Promise<RegisterResult> {
|
||||
const sessionId = input.providerSessionId ?? randomUUID();
|
||||
const now = new Date();
|
||||
const llms: Llm[] = [];
|
||||
@@ -210,6 +216,12 @@ export class VirtualLlmService implements IVirtualLlmService {
|
||||
description: p.description ?? '',
|
||||
...(p.extraConfig !== undefined ? { extraConfig: p.extraConfig } : {}),
|
||||
...(p.poolName !== undefined ? { poolName: p.poolName } : {}),
|
||||
// v7: virtuals default to private ownership. The publisher
|
||||
// (passed through from the route's authenticated userId)
|
||||
// owns the row; mcplocal's defaulting + the publisher's
|
||||
// explicit override land here.
|
||||
ownerId: input.ownerId ?? null,
|
||||
visibility: p.visibility ?? 'private',
|
||||
kind: 'virtual',
|
||||
providerSessionId: sessionId,
|
||||
status: initialStatus,
|
||||
@@ -244,6 +256,11 @@ export class VirtualLlmService implements IVirtualLlmService {
|
||||
...(p.description !== undefined ? { description: p.description } : {}),
|
||||
...(p.extraConfig !== undefined ? { extraConfig: p.extraConfig } : {}),
|
||||
...(p.poolName !== undefined ? { poolName: p.poolName } : {}),
|
||||
// v7: only update visibility on sticky reconnect when the
|
||||
// publisher explicitly sent it — operators may have flipped a
|
||||
// virtual to public manually via `mcpctl edit llm`, and we
|
||||
// don't want a routine reconnect to clobber that.
|
||||
...(p.visibility !== undefined ? { visibility: p.visibility } : {}),
|
||||
kind: 'virtual',
|
||||
providerSessionId: sessionId,
|
||||
status: initialStatus,
|
||||
|
||||
@@ -75,6 +75,7 @@ describe('POST /api/v1/llms/_provider-register', () => {
|
||||
expect(register).toHaveBeenCalledWith({
|
||||
providerSessionId: 'sess-xyz',
|
||||
providers: [{ name: 'vllm-local', type: 'openai', model: 'm', tier: 'fast', extraConfig: { gpu: 1 } }],
|
||||
ownerId: 'system',
|
||||
});
|
||||
expect(res.json()).toMatchObject({ providerSessionId: 'sess-xyz' });
|
||||
});
|
||||
|
||||
105
src/mcpd/tests/visibility-filter.test.ts
Normal file
105
src/mcpd/tests/visibility-filter.test.ts
Normal file
@@ -0,0 +1,105 @@
|
||||
/**
|
||||
* v7 Stage 1 — pure-function tests for the visibility predicate.
|
||||
* Lives separately from the service-level tests because the predicate
|
||||
* is the single source of truth for "can user X see this row" and is
|
||||
* called from both LlmService.list and AgentService.list. We exercise
|
||||
* every branch of the decision tree to lock the semantics in.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { isLlmVisibleTo, type Viewer } from '../src/services/llm.service.js';
|
||||
import { isAgentVisibleTo, type AgentViewer } from '../src/services/agent.service.js';
|
||||
|
||||
const llmRow = (overrides: { name?: string; ownerId?: string | null; visibility?: string } = {}): { name: string; ownerId: string | null; visibility: string } => ({
|
||||
name: 'vllm-alice',
|
||||
ownerId: 'alice',
|
||||
visibility: 'private',
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const agentRow = (overrides: { name?: string; ownerId?: string; visibility?: string } = {}): { name: string; ownerId: string; visibility: string } => ({
|
||||
name: 'reviewer',
|
||||
ownerId: 'alice',
|
||||
visibility: 'private',
|
||||
...overrides,
|
||||
});
|
||||
|
||||
describe('isLlmVisibleTo (v7)', () => {
|
||||
it('null viewer skips the filter — internal callers see everything', () => {
|
||||
// Cron sweeps, audit collectors, and tests without a request
|
||||
// context get a null viewer. The visibility filter is then a
|
||||
// no-op, which matches the pre-v7 behavior of those code paths.
|
||||
expect(isLlmVisibleTo(llmRow(), null)).toBe(true);
|
||||
});
|
||||
|
||||
it('public rows are visible to anyone with the resource grant', () => {
|
||||
const v: Viewer = { userId: 'bob', wildcard: false, allowedNames: new Set() };
|
||||
expect(isLlmVisibleTo(llmRow({ visibility: 'public' }), v)).toBe(true);
|
||||
});
|
||||
|
||||
it('wildcard viewer (admin) sees private rows owned by others', () => {
|
||||
const v: Viewer = { userId: 'admin', wildcard: true, allowedNames: new Set() };
|
||||
expect(isLlmVisibleTo(llmRow({ visibility: 'private', ownerId: 'alice' }), v)).toBe(true);
|
||||
});
|
||||
|
||||
it('owner sees their own private row', () => {
|
||||
const v: Viewer = { userId: 'alice', wildcard: false, allowedNames: new Set() };
|
||||
expect(isLlmVisibleTo(llmRow({ visibility: 'private', ownerId: 'alice' }), v)).toBe(true);
|
||||
});
|
||||
|
||||
it('non-owner without name-scoped grant cannot see a private row', () => {
|
||||
const v: Viewer = { userId: 'bob', wildcard: false, allowedNames: new Set() };
|
||||
expect(isLlmVisibleTo(llmRow({ visibility: 'private', ownerId: 'alice' }), v)).toBe(false);
|
||||
});
|
||||
|
||||
it('non-owner WITH name-scoped grant can see a private row', () => {
|
||||
// alice published vllm-alice as private; alice ran
|
||||
// `mcpctl create rbac binding view:llms:vllm-alice --user bob`,
|
||||
// so bob now sees the row in his list output.
|
||||
const v: Viewer = { userId: 'bob', wildcard: false, allowedNames: new Set(['vllm-alice']) };
|
||||
expect(isLlmVisibleTo(llmRow({ name: 'vllm-alice', visibility: 'private', ownerId: 'alice' }), v)).toBe(true);
|
||||
});
|
||||
|
||||
it('treats null ownerId as no-owner (legacy rows pre-v7 backfill stay visible if public)', () => {
|
||||
// The migration sets visibility='public' for legacy rows, so they
|
||||
// pass the public-visibility check before the ownerId branch is
|
||||
// ever reached. A row with NULL ownerId AND visibility='private'
|
||||
// is unreachable via normal flows, but we still want the predicate
|
||||
// to behave: no owner + bob viewing = not visible.
|
||||
const v: Viewer = { userId: 'bob', wildcard: false, allowedNames: new Set() };
|
||||
expect(isLlmVisibleTo(llmRow({ ownerId: null, visibility: 'private' }), v)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isAgentVisibleTo (v7)', () => {
|
||||
// Same shape as Llm; agents always have a non-null ownerId because
|
||||
// `Agent.ownerId` is required, so we don't need the legacy-null
|
||||
// branch test.
|
||||
it('null viewer = visible (internal calls bypass filter)', () => {
|
||||
expect(isAgentVisibleTo(agentRow(), null)).toBe(true);
|
||||
});
|
||||
|
||||
it('public agents visible to anyone with resource grant', () => {
|
||||
const v: AgentViewer = { userId: 'bob', wildcard: false, allowedNames: new Set() };
|
||||
expect(isAgentVisibleTo(agentRow({ visibility: 'public' }), v)).toBe(true);
|
||||
});
|
||||
|
||||
it('owner sees own private agent', () => {
|
||||
const v: AgentViewer = { userId: 'alice', wildcard: false, allowedNames: new Set() };
|
||||
expect(isAgentVisibleTo(agentRow({ visibility: 'private', ownerId: 'alice' }), v)).toBe(true);
|
||||
});
|
||||
|
||||
it('non-owner without grant blocked from private agent', () => {
|
||||
const v: AgentViewer = { userId: 'bob', wildcard: false, allowedNames: new Set() };
|
||||
expect(isAgentVisibleTo(agentRow({ visibility: 'private', ownerId: 'alice' }), v)).toBe(false);
|
||||
});
|
||||
|
||||
it('non-owner WITH name-scoped grant can see private agent', () => {
|
||||
const v: AgentViewer = { userId: 'bob', wildcard: false, allowedNames: new Set(['reviewer']) };
|
||||
expect(isAgentVisibleTo(agentRow({ name: 'reviewer', visibility: 'private', ownerId: 'alice' }), v)).toBe(true);
|
||||
});
|
||||
|
||||
it('wildcard viewer sees private agent owned by another user', () => {
|
||||
const v: AgentViewer = { userId: 'admin', wildcard: true, allowedNames: new Set() };
|
||||
expect(isAgentVisibleTo(agentRow({ visibility: 'private', ownerId: 'alice' }), v)).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user