michal/mcpctl
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

feat(mcpd+db): visibility scope + ownership for Llms and Agents (v7 Stage 1)

Browse Source 
Adds the schema + service-layer machinery for per-user RBAC scoping
of virtual Llms and Agents. Without this, anyone with `view:llms`
sees every other user's published model — fine for a single-user
homelab, wrong for org use where workstation-published models or
paid keys aren't meant to be broadcast.

Schema:
  - Llm: new `ownerId String?` + `visibility String @default("public")`.
    NULL ownerId on legacy rows is treated as public for back-compat.
  - Agent: `visibility String @default("public")` (Agent already has
    `ownerId`, required).
  - Composite index `(visibility, ownerId)` on both tables for the
    list-filter hot path.
  - Migration backfills both columns to 'public' so pre-v7 setups
    behave identically post-deploy.

Service layer:
  - New `Viewer` / `AgentViewer` shape: `{ userId, wildcard, allowedNames }`.
    The route layer computes this from `request.userId` +
    `RbacService.getAllowedScope` and passes it down. NULL viewer =
    skip the filter (internal callers — cron sweeps, audit, tests).
  - `isLlmVisibleTo` / `isAgentVisibleTo` pure predicates encode the
    decision tree:
      visibility=public → visible (RBAC layer above already passed)
      viewer=null OR wildcard → visible
      ownerId === viewer.userId → visible
      row.name in viewer.allowedNames → visible
      else → hidden
  - LlmService.list/getById/getByName + AgentService equivalents
    accept an optional Viewer arg and apply the predicate. Get-style
    methods 404 (not 403) on hidden rows so name enumeration via
    differential status is impossible.

Repositories: CreateInput/UpdateInput types gained `ownerId`/
`visibility` (Llm) and `visibility` (Agent). Update is in place;
ownerId is set-once at create time.

Tests:
  - 13 unit tests on the predicate covering every branch (null
    viewer, public, wildcard, owner, name-scoped grant, foreign
    private, legacy null-ownerId).
  - mcpd 908/908 (was 893; +15 across the merge windows + this PR).

Stage 2 (next): route plumbing — every list/get endpoint needs to
build the Viewer from the request and pass it through. mcplocal
virtuals default to visibility=private on register. CLI adds a
VISIBILITY column and a --visibility flag. yaml round-trip preserves
the field.
This commit is contained in:
Michal
2026-04-29 00:46:06 +01:00
parent 3071bcee8e
commit 21f8bede2e
7 changed files with 287 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
src/db/prisma/migrations/20260428233954_add_visibility_v7/migration.sql Normal file
View File

@@ -0,0 +1,25 @@
-- v7: per-user RBAC scoping for virtual Llms and Agents.
--
-- `Llm.ownerId` is new — we don't have a record of who created legacy
-- rows, so existing data is left NULL (treated as "no owner, public").
-- The list/get filter in the service layer handles NULL ownerId
-- correctly: a NULL-owner public row stays visible to everyone.
--
-- `Llm.visibility` and `Agent.visibility` default to 'public' so the
-- backfill is automatic — pre-v7 setups continue to behave identically.
-- New rows created post-deploy carry the value the service writes
-- (mcplocal virtuals → 'private'; CLI `mcpctl create llm` → 'public'
-- by default unless `--visibility private` is passed).
ALTER TABLE "Llm"
ADD COLUMN "ownerId" TEXT,
ADD COLUMN "visibility" TEXT NOT NULL DEFAULT 'public';
ALTER TABLE "Agent"
ADD COLUMN "visibility" TEXT NOT NULL DEFAULT 'public';
-- Composite index supports the list-filter hot path:
-- `WHERE visibility='public' OR ownerId=$1`
-- on tables that may grow as more publishers / users come online.
CREATE INDEX "Llm_visibility_ownerId_idx" ON "Llm"("visibility", "ownerId");
CREATE INDEX "Agent_visibility_ownerId_idx" ON "Agent"("visibility", "ownerId");

37
src/db/prisma/schema.prisma
View File

@@ -225,6 +225,30 @@ model Llm {
lastHeartbeatAt DateTime? // bumped on every publisher heartbeat
status LlmStatus @default(active)
inactiveSince DateTime? // when status flipped from active; used for 4-h GC
// ── Per-user RBAC scoping (v7) ──
// `ownerId` records who created/published the row. NULL on legacy rows
// (those created before the v7 migration) — those continue to behave
// as `visibility=public` for back-compat. New rows always carry an
// ownerId set by the service layer (`User.id` of the authenticated
// caller, or the publishing mcplocal user for virtuals).
//
// `visibility` controls who can see / use the row:
// - 'public' : anyone with the resource grant (`view:llms`,
// `run:llms:<name>`, etc.) sees it. Legacy default
// mirrors today's behavior — explicit
// `mcpctl create llm` calls keep this default.
// - 'private' : only the owner sees it by default; other users need
// an explicit name-scoped RBAC binding
// (`view:llms:<name>`, `run:llms:<name>`). The list
// endpoint hides foreign-private rows from
// unauthorized callers; get/describe returns 404 to
// prevent name enumeration.
//
// mcplocal-published virtual Llms default to 'private' on register —
// a workstation-published model isn't typically meant for the whole
// org until the publisher explicitly shares it. See docs/virtual-llms.md.
ownerId String?
visibility String @default("public")
version Int @default(1)
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt
@@ -238,6 +262,10 @@ model Llm {
@@index([kind, status])
@@index([providerSessionId])
@@index([poolName])
// List filter on the hot path: "rows visible to caller X" decomposes
// into `visibility='public' OR ownerId=X` + an RBAC join. Composite
// index keeps the predicate fast even on a large table.
@@index([visibility, ownerId])
}
// ── Groups ──
@@ -495,6 +523,14 @@ model Agent {
status LlmStatus @default(active)
inactiveSince DateTime?
ownerId String
// v7: per-user RBAC scoping. Mirrors `Llm.visibility` semantics —
// 'public' (default, today's behavior) lets anyone with the resource
// grant see the agent; 'private' restricts to owner + explicit
// name-scoped RBAC bindings. mcplocal-published virtual agents
// default to 'private' on register so a workstation-published persona
// isn't broadcast to the whole org until shared explicitly. Existing
// rows backfill to 'public' so pre-v7 setups keep working unchanged.
visibility String @default("public")
version Int @default(1)
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt
@@ -514,6 +550,7 @@ model Agent {
@@index([defaultPersonalityId])
@@index([kind, status])
@@index([providerSessionId])
@@index([visibility, ownerId])
}
// ── Personalities (named overlay bundles of prompts on top of an Agent) ──

7
src/mcpd/src/repositories/agent.repository.ts
View File

@@ -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
src/mcpd/src/repositories/llm.repository.ts
View File

@@ -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;

42
src/mcpd/src/services/agent.service.ts
View File

@@ -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;
@@ -63,26 +88,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 +229,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,

69
src/mcpd/src/services/llm.service.ts
View File

@@ -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,

105
src/mcpd/tests/visibility-filter.test.ts Normal file
View 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);
});
});