diff --git a/completions/mcpctl.bash b/completions/mcpctl.bash index eb27698..acc6c38 100644 --- a/completions/mcpctl.bash +++ b/completions/mcpctl.bash @@ -185,10 +185,10 @@ _mcpctl() { COMPREPLY=($(compgen -W "--data --force -h --help" -- "$cur")) ;; llm) - COMPREPLY=($(compgen -W "--type --model --url --tier --description --api-key-ref --extra --pool-name --force --skip-auth-check -h --help" -- "$cur")) + COMPREPLY=($(compgen -W "--type --model --url --tier --description --api-key-ref --extra --pool-name --visibility --force --skip-auth-check -h --help" -- "$cur")) ;; agent) - COMPREPLY=($(compgen -W "--llm --project --description --system-prompt --system-prompt-file --proxy-model --default-temperature --default-top-p --default-top-k --default-max-tokens --default-seed --default-stop --default-extra --default-params-file --force -h --help" -- "$cur")) + COMPREPLY=($(compgen -W "--llm --project --description --system-prompt --system-prompt-file --proxy-model --default-temperature --default-top-p --default-top-k --default-max-tokens --default-seed --default-stop --default-extra --default-params-file --visibility --force -h --help" -- "$cur")) ;; secretbackend) COMPREPLY=($(compgen -W "--type --description --default --url --namespace --mount --path-prefix --auth --token-secret --role --auth-mount --sa-token-path --config --wizard --setup-token --policy-name --token-role --no-promote-default --force -h --help" -- "$cur")) diff --git a/completions/mcpctl.fish b/completions/mcpctl.fish index e4162a2..80c131f 100644 --- a/completions/mcpctl.fish +++ b/completions/mcpctl.fish @@ -335,6 +335,7 @@ complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l description -d 'Des complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l api-key-ref -d 'API key reference in SECRET/KEY form (e.g. anthropic-key/token)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l extra -d 'Extra config key=value (repeat)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l pool-name -d 'Stack with other Llms sharing this pool name; agents pinned to any member dispatch across the pool' -x +complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l visibility -d 'Visibility scope: public (everyone) or private (only owner + name-grants)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l force -d 'Update if already exists' complete -c mcpctl -n "__mcpctl_subcmd_active create llm" -l skip-auth-check -d 'Skip the upstream auth probe (for offline registration before infra exists)' @@ -353,6 +354,7 @@ complete -c mcpctl -n "__mcpctl_subcmd_active create agent" -l default-seed -d ' complete -c mcpctl -n "__mcpctl_subcmd_active create agent" -l default-stop -d 'Default stop sequence (repeat for multiple)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create agent" -l default-extra -d 'Default provider-specific knob k=v (repeat)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create agent" -l default-params-file -d 'Read defaultParams from a JSON file' -x +complete -c mcpctl -n "__mcpctl_subcmd_active create agent" -l visibility -d 'Visibility scope: public (everyone) or private (only owner + name-grants)' -x complete -c mcpctl -n "__mcpctl_subcmd_active create agent" -l force -d 'Update if already exists' # create secretbackend options diff --git a/docs/virtual-llms.md b/docs/virtual-llms.md index d8fd354..6f5b458 100644 --- a/docs/virtual-llms.md +++ b/docs/virtual-llms.md @@ -431,10 +431,100 @@ mid-task reverts the row to pending instead of failing the caller. See [inference-tasks.md](./inference-tasks.md) for the full data model, async API, lifecycle, RBAC, and CLI surface. +## Visibility scope (v7) + +Virtual Llms and Agents now carry an explicit **visibility** field that +decides who can see the row in listings. + +| Visibility | Meaning | +|-------------|----------------------------------------------------------------------------------| +| `public` | Visible to anyone with `view:llms` / `view:agents`. Default for hand-created Llms. | +| `private` | Only the **owner** plus principals with a name-scoped grant can see it. Default for virtual Llms and Agents on first publish. | + +The owner is whichever user authenticated the publishing +`POST /api/v1/llms/_provider-register` (or `mcpctl create llm`). For +mcplocal that's whichever `~/.mcpctl/credentials` token is on disk. +Legacy rows from before v7 default to `visibility=public, ownerId=NULL`, +so the upgrade is a no-op for everything that already exists. + +### Who skips the filter? + +Two principals see every row regardless of visibility: + +1. The **row owner** (`ownerId === request.userId`). +2. Anyone with a **cross-resource admin** grant — RBAC binding + `{ resource: '*' }`. Operationally this is the SRE / cluster admin. + +A plain `view:llms` resource grant is *not* the same as admin: it's a +RBAC wildcard for name-scoping (you can name any Llm), but the +visibility filter still applies on top. This is the v7 split that +prevents a user with `view:llms` from enumerating every developer's +private virtual Llm. + +### Granting a single-row exception + +When alice wants bob to see her private virtual Llm `alice-vllm-local` +without making it public, she binds: + +```sh +mcpctl create rbac bob view:llms --name alice-vllm-local +``` + +Same shape as any other name-scoped binding. Removing the binding +flips bob back to "row not found". + +### Publishing as private from mcplocal + +mcplocal defaults to `private` for every published provider and agent. +Override per-row in `~/.mcpctl/config.json`: + +```jsonc +{ + "llm": { + "providers": [ + { "name": "vllm-local", "type": "vllm", "model": "...", "publish": true, + "visibility": "private" }, // default; explicit for clarity + { "name": "shared-qwen", "type": "vllm", "model": "...", "publish": true, + "visibility": "public" } // every team member can chat with it + ] + }, + "agents": [ + { "name": "local-coder", "llm": "vllm-local", + "visibility": "private" } // private agents pinned to private Llms + ] +} +``` + +On a sticky reconnect (`providerSessionId` matches an existing row) +the visibility is **only** updated when the publisher explicitly sends +it — leaving the field off keeps whatever the row already has, +including any field admin set out-of-band. + +### Hand-created Llms + +`mcpctl create llm` defaults to `public` (matches pre-v7 behavior). +Pass `--visibility private` to opt in: + +```sh +mcpctl create llm my-key --type openai --model gpt-4o \ + --api-key-ref my-secret/key --visibility private +``` + +The same `--visibility` flag is on `mcpctl create agent`. + +### CLI surface + +`mcpctl get llm` and `mcpctl get agent` show a `VISIBILITY` column. +YAML round-trips cleanly: `mcpctl get llm X -o yaml | mcpctl apply -f -` +preserves visibility, and `ownerId` is stripped from the apply doc +because it's server-side state (the apply re-stamps the ownerId of the +authenticated caller, not the original creator). + ## Roadmap (later stages) -(LB pool by name landed in v4; durable task queue landed in v5.) -- **v6** — multi-instance mcpd via pg `LISTEN/NOTIFY` (replaces the +(LB pool by name landed in v4; durable task queue landed in v5; +visibility scope landed in v7.) +- **v8** — multi-instance mcpd via pg `LISTEN/NOTIFY` (replaces the per-instance EventEmitter wakeup), per-session worker capacity, remote cancel protocol over the SSE channel. diff --git a/src/cli/src/commands/create.ts b/src/cli/src/commands/create.ts index d0f4d69..95a35d3 100644 --- a/src/cli/src/commands/create.ts +++ b/src/cli/src/commands/create.ts @@ -264,6 +264,7 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { .option('--api-key-ref ', 'API key reference in SECRET/KEY form (e.g. anthropic-key/token)') .option('--extra ', 'Extra config key=value (repeat)', collect, []) .option('--pool-name ', 'Stack with other Llms sharing this pool name; agents pinned to any member dispatch across the pool') + .option('--visibility ', 'Visibility scope: public (everyone) or private (only owner + name-grants)', 'public') .option('--force', 'Update if already exists') .option('--skip-auth-check', 'Skip the upstream auth probe (for offline registration before infra exists)') .action(async (name: string, opts) => { @@ -276,6 +277,12 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { if (opts.url) body.url = opts.url; if (opts.description !== undefined) body.description = opts.description; if (opts.poolName !== undefined) body.poolName = opts.poolName; + if (opts.visibility !== undefined) { + if (opts.visibility !== 'public' && opts.visibility !== 'private') { + throw new Error(`Invalid --visibility '${opts.visibility as string}'. Expected 'public' or 'private'`); + } + body.visibility = opts.visibility; + } if (opts.apiKeyRef) { const slashIdx = (opts.apiKeyRef as string).indexOf('/'); if (slashIdx < 1) throw new Error(`Invalid --api-key-ref '${opts.apiKeyRef as string}'. Expected SECRET_NAME/KEY_NAME`); @@ -333,6 +340,7 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { .option('--default-stop ', 'Default stop sequence (repeat for multiple)', collect, []) .option('--default-extra ', 'Default provider-specific knob k=v (repeat)', collect, []) .option('--default-params-file ', 'Read defaultParams from a JSON file') + .option('--visibility ', 'Visibility scope: public (everyone) or private (only owner + name-grants)', 'public') .option('--force', 'Update if already exists') .action(async (name: string, opts) => { const body: Record = { @@ -341,6 +349,12 @@ export function createCreateCommand(deps: CreateCommandDeps): Command { }; if (opts.project) body.project = { name: opts.project }; if (opts.description !== undefined) body.description = opts.description; + if (opts.visibility !== undefined) { + if (opts.visibility !== 'public' && opts.visibility !== 'private') { + throw new Error(`Invalid --visibility '${opts.visibility as string}'. Expected 'public' or 'private'`); + } + body.visibility = opts.visibility; + } let systemPrompt = opts.systemPrompt as string | undefined; if (systemPrompt === undefined && opts.systemPromptFile !== undefined) { diff --git a/src/cli/src/commands/get.ts b/src/cli/src/commands/get.ts index 445bc6d..5c1ca16 100644 --- a/src/cli/src/commands/get.ts +++ b/src/cli/src/commands/get.ts @@ -138,6 +138,10 @@ interface LlmRow { status?: 'active' | 'inactive' | 'hibernating'; // v4: explicit pool key. NULL = solo Llm (effective pool = its own name). poolName?: string | null; + // v7: visibility scope. Legacy public rows omit it; mcpd defaults missing + // values to 'public' on serialization. + visibility?: 'public' | 'private'; + ownerId?: string | null; } // v4: POOL column placed right after NAME so an operator can't miss @@ -148,6 +152,7 @@ const llmColumns: Column[] = [ { header: 'POOL', key: (r) => (r.poolName !== null && r.poolName !== undefined && r.poolName !== '') ? r.poolName : '-', width: 18 }, { header: 'KIND', key: (r) => r.kind ?? 'public', width: 8 }, { header: 'STATUS', key: (r) => r.status ?? 'active', width: 12 }, + { header: 'VISIBILITY', key: (r) => r.visibility ?? 'public', width: 11 }, { header: 'TYPE', key: 'type', width: 12 }, { header: 'MODEL', key: 'model', width: 28 }, { header: 'TIER', key: 'tier', width: 8 }, @@ -214,12 +219,15 @@ interface AgentRow { // AgentService as the publishing mcplocal heartbeats and disconnects. kind?: 'public' | 'virtual'; status?: 'active' | 'inactive'; + // v7: visibility — same semantics as Llm. Public legacy agents omit it. + visibility?: 'public' | 'private'; } const agentColumns: Column[] = [ { header: 'NAME', key: 'name' }, { header: 'KIND', key: (r) => r.kind ?? 'public', width: 8 }, { header: 'STATUS', key: (r) => r.status ?? 'active', width: 10 }, + { header: 'VISIBILITY', key: (r) => r.visibility ?? 'public', width: 11 }, { header: 'LLM', key: (r) => r.llm.name, width: 24 }, { header: 'PROJECT', key: (r) => r.project?.name ?? '-', width: 20 }, { header: 'DESCRIPTION', key: (r) => truncate(r.description, 50) || '-', width: 50 }, diff --git a/src/db/prisma/migrations/20260428233954_add_visibility_v7/migration.sql b/src/db/prisma/migrations/20260428233954_add_visibility_v7/migration.sql new file mode 100644 index 0000000..f577bdb --- /dev/null +++ b/src/db/prisma/migrations/20260428233954_add_visibility_v7/migration.sql @@ -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"); diff --git a/src/db/prisma/schema.prisma b/src/db/prisma/schema.prisma index 4289ae0..1c70d95 100644 --- a/src/db/prisma/schema.prisma +++ b/src/db/prisma/schema.prisma @@ -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:`, 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:`, `run:llms:`). 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) ── diff --git a/src/mcpd/src/main.ts b/src/mcpd/src/main.ts index d53828f..8269e7c 100644 --- a/src/mcpd/src/main.ts +++ b/src/mcpd/src/main.ts @@ -747,14 +747,48 @@ async function main(): Promise { 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>).filter((item) => { - const name = item['name']; - return typeof name === 'string' && request.rbacScope!.names.has(name); - }); + let items = payload as Array>; + // 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 diff --git a/src/mcpd/src/middleware/auth.ts b/src/mcpd/src/middleware/auth.ts index 0bc3a3c..741ad66 100644 --- a/src/mcpd/src/middleware/auth.ts +++ b/src/mcpd/src/middleware/auth.ts @@ -33,7 +33,14 @@ export interface AuthDeps { declare module 'fastify' { interface FastifyRequest { userId?: string; - rbacScope?: { wildcard: boolean; names: Set }; + /** + * 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 }; /** Set by the auth hook when the caller authenticated via a McpToken bearer (prefix `mcpctl_pat_`). */ mcpToken?: McpTokenPrincipal; } diff --git a/src/mcpd/src/repositories/agent.repository.ts b/src/mcpd/src/repositories/agent.repository.ts index 9f83541..271ef25 100644 --- a/src/mcpd/src/repositories/agent.repository.ts +++ b/src/mcpd/src/repositories/agent.repository.ts @@ -11,6 +11,8 @@ export interface CreateAgentRepoInput { defaultParams?: Record; extras?: Record; 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; extras?: Record; + // 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; diff --git a/src/mcpd/src/repositories/llm.repository.ts b/src/mcpd/src/repositories/llm.repository.ts index 8942e19..b10bd64 100644 --- a/src/mcpd/src/repositories/llm.repository.ts +++ b/src/mcpd/src/repositories/llm.repository.ts @@ -12,6 +12,11 @@ export interface CreateLlmInput { extraConfig?: Record; // 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; 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; diff --git a/src/mcpd/src/routes/agents.ts b/src/mcpd/src/routes/agents.ts index 5e3e633..d2fbf09 100644 --- a/src/mcpd/src/routes/agents.ts +++ b/src/mcpd/src/routes/agents.ts @@ -6,21 +6,33 @@ * — the resource is `agents`. The chat endpoints live in `agent-chat.ts` and * map to `run:agents:`. */ -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); } diff --git a/src/mcpd/src/routes/llms.ts b/src/mcpd/src/routes/llms.ts index b153a9a..740e8d1 100644 --- a/src/mcpd/src/routes/llms.ts +++ b/src/mcpd/src/routes/llms.ts @@ -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 ` 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); } diff --git a/src/mcpd/src/routes/virtual-llms.ts b/src/mcpd/src/routes/virtual-llms.ts index fbde2ba..be86d1f 100644 --- a/src/mcpd/src/routes/virtual-llms.ts +++ b/src/mcpd/src/routes/virtual-llms.ts @@ -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; } + // 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; 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; } diff --git a/src/mcpd/src/services/agent.service.ts b/src/mcpd/src/services/agent.service.ts index 601d0dc..440d4d2 100644 --- a/src/mcpd/src/services/agent.service.ts +++ b/src/mcpd/src/services/agent.service.ts @@ -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; +} + +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; extras?: Record; + /** + * 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 { + async list(viewer: AgentViewer | null = null): Promise { 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 { + async listByProject(projectName: string, viewer: AgentViewer | null = null): Promise { 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 { + async getById(id: string, viewer: AgentViewer | null = null): Promise { 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 { + async getByName(name: string, viewer: AgentViewer | null = null): Promise { 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', diff --git a/src/mcpd/src/services/llm.service.ts b/src/mcpd/src/services/llm.service.ts index af37176..1ee2149 100644 --- a/src/mcpd/src/services/llm.service.ts +++ b/src/mcpd/src/services/llm.service.ts @@ -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; +} + +/** + * 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 { + async list(viewer: Viewer | null = null): Promise { 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 { + async getById(id: string, viewer: Viewer | null = null): Promise { 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 { + async getByName(name: string, viewer: Viewer | null = null): Promise { 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, poolName: row.poolName, + ownerId: row.ownerId, + visibility: (row.visibility === 'private' ? 'private' : 'public') as 'public' | 'private', kind: row.kind, status: row.status, lastHeartbeatAt: row.lastHeartbeatAt, diff --git a/src/mcpd/src/services/rbac.service.ts b/src/mcpd/src/services/rbac.service.ts index ddc7fe2..db76359 100644 --- a/src/mcpd/src/services/rbac.service.ts +++ b/src/mcpd/src/services/rbac.service.ts @@ -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; } @@ -97,6 +107,8 @@ export class RbacService { const permissions = await this.getPermissions(userId, serviceAccountName, mcpTokenSha); const normalized = normalizeResource(resource); const names = new Set(); + 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 }; } /** diff --git a/src/mcpd/src/services/virtual-llm.service.ts b/src/mcpd/src/services/virtual-llm.service.ts index 7a6a8b2..594435f 100644 --- a/src/mcpd/src/services/virtual-llm.service.ts +++ b/src/mcpd/src/services/virtual-llm.service.ts @@ -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; + register(input: { providerSessionId?: string | null; providers: RegisterProviderInput[]; ownerId?: string }): Promise; heartbeat(providerSessionId: string): Promise; bindSession(providerSessionId: string, handle: VirtualSessionHandle): void; unbindSession(providerSessionId: string): Promise; @@ -193,7 +199,7 @@ export class VirtualLlmService implements IVirtualLlmService { private readonly resolveOwner: () => string = () => 'system', ) {} - async register(input: { providerSessionId?: string | null; providers: RegisterProviderInput[] }): Promise { + async register(input: { providerSessionId?: string | null; providers: RegisterProviderInput[]; ownerId?: string }): Promise { 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, diff --git a/src/mcpd/tests/virtual-llm-routes.test.ts b/src/mcpd/tests/virtual-llm-routes.test.ts index a4477bc..1e4c47f 100644 --- a/src/mcpd/tests/virtual-llm-routes.test.ts +++ b/src/mcpd/tests/virtual-llm-routes.test.ts @@ -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' }); }); diff --git a/src/mcpd/tests/visibility-filter.test.ts b/src/mcpd/tests/visibility-filter.test.ts new file mode 100644 index 0000000..8758e9b --- /dev/null +++ b/src/mcpd/tests/visibility-filter.test.ts @@ -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); + }); +}); diff --git a/src/mcplocal/src/http/config.ts b/src/mcplocal/src/http/config.ts index b99ebb9..3290280 100644 --- a/src/mcplocal/src/http/config.ts +++ b/src/mcplocal/src/http/config.ts @@ -105,6 +105,14 @@ export interface LlmProviderFileEntry { * logical pool that auto-grows as more workers come online. */ poolName?: string; + /** + * v7: per-user RBAC scoping. mcplocal-published virtuals default to + * 'private' on register — the publishing user owns the row and other + * users don't see it without an explicit `view:llms:` grant. + * Set to 'public' here to opt into org-wide sharing for this + * provider. + */ + visibility?: 'public' | 'private'; } export type WakeRecipe = @@ -136,6 +144,8 @@ export interface AgentFileEntry { project?: string; defaultParams?: Record; extras?: Record; + /** v7: see LlmProviderFileEntry.visibility — same default ('private'). */ + visibility?: 'public' | 'private'; } /** diff --git a/src/mcplocal/src/main.ts b/src/mcplocal/src/main.ts index 3dc842a..892b95f 100644 --- a/src/mcplocal/src/main.ts +++ b/src/mcplocal/src/main.ts @@ -233,6 +233,10 @@ async function maybeStartVirtualLlmRegistrar( if (entry.wake !== undefined) item.wake = entry.wake; if (entry.poolName !== undefined) item.poolName = entry.poolName; if (wireName !== provider.name) item.publishName = wireName; + // v7: pass visibility through; registrar already defaults to + // 'private' when omitted, and the per-provider override flows + // straight through to the register payload. + if (entry.visibility !== undefined) item.visibility = entry.visibility; published.push(item); } // v3: forward locally-declared agents alongside the providers. We @@ -255,6 +259,7 @@ async function maybeStartVirtualLlmRegistrar( if (a.project !== undefined) item.project = a.project; if (a.defaultParams !== undefined) item.defaultParams = a.defaultParams; if (a.extras !== undefined) item.extras = a.extras; + if (a.visibility !== undefined) item.visibility = a.visibility; publishedAgents.push(item); } diff --git a/src/mcplocal/src/providers/registrar.ts b/src/mcplocal/src/providers/registrar.ts index 5e2b17e..4069040 100644 --- a/src/mcplocal/src/providers/registrar.ts +++ b/src/mcplocal/src/providers/registrar.ts @@ -72,6 +72,14 @@ export interface RegistrarPublishedProvider { * `publishName ?? provider.name` everywhere. */ publishName?: string; + /** + * v7: per-user RBAC scoping. mcplocal-published virtuals default to + * 'private' (visible only to the publishing user) — workstations + * shouldn't broadcast their models org-wide unless explicitly + * shared. The publisher can override per provider with + * `"visibility": "public"` in their mcplocal config. + */ + visibility?: 'public' | 'private'; } /** @@ -88,6 +96,8 @@ export interface RegistrarPublishedAgent { project?: string; defaultParams?: Record; extras?: Record; + /** v7: per-user RBAC scoping, defaults to 'private' on register. */ + visibility?: 'public' | 'private'; } export interface RegistrarOptions { @@ -207,6 +217,10 @@ export class VirtualLlmRegistrar { ...(p.tier !== undefined ? { tier: p.tier } : {}), ...(p.description !== undefined ? { description: p.description } : {}), ...(p.poolName !== undefined ? { poolName: p.poolName } : {}), + // v7: virtuals default to private. Operators who want their + // workstation model org-visible set "visibility": "public" per + // provider in mcplocal config. + visibility: p.visibility ?? 'private', initialStatus, }; })); @@ -224,6 +238,9 @@ export class VirtualLlmRegistrar { ...(a.project !== undefined ? { project: a.project } : {}), ...(a.defaultParams !== undefined ? { defaultParams: a.defaultParams } : {}), ...(a.extras !== undefined ? { extras: a.extras } : {}), + // v7: forward visibility to mcpd. Defaults to 'private' for + // virtual agents on the server side when omitted. + visibility: a.visibility ?? 'private', })); } diff --git a/src/mcplocal/tests/smoke/virtual-llm-visibility.smoke.test.ts b/src/mcplocal/tests/smoke/virtual-llm-visibility.smoke.test.ts new file mode 100644 index 0000000..a41c0e4 --- /dev/null +++ b/src/mcplocal/tests/smoke/virtual-llm-visibility.smoke.test.ts @@ -0,0 +1,208 @@ +/** + * Smoke: v7 visibility round-trip. + * + * Publishes two virtual Llms via the registrar — one explicitly public, + * one explicitly private — and verifies the GET /api/v1/llms response + * carries the visibility + ownerId fields end-to-end. The cross-user + * filter (private rows hidden from non-owner non-admin) is fully + * covered by mcpd's visibility-filter unit tests; smoke only proves + * the new fields make the round-trip from registrar → mcpd → list + * payload without dropping or being defaulted away. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import http from 'node:http'; +import https from 'node:https'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + VirtualLlmRegistrar, + type RegistrarPublishedProvider, +} from '../../src/providers/registrar.js'; +import type { LlmProvider, CompletionResult } from '../../src/providers/types.js'; + +const MCPD_URL = process.env.MCPD_URL ?? 'https://mcpctl.ad.itaz.eu'; +const SUFFIX = Date.now().toString(36); +const PUBLIC_NAME = `smoke-vis-public-${SUFFIX}`; +const PRIVATE_NAME = `smoke-vis-private-${SUFFIX}`; + +function makeFakeProvider(name: string): LlmProvider { + return { + name, + async complete(): Promise { + return { + content: 'ok', + toolCalls: [], + usage: { promptTokens: 1, completionTokens: 1, totalTokens: 2 }, + finishReason: 'stop', + }; + }, + async listModels() { return []; }, + async isAvailable() { return true; }, + }; +} + +function healthz(url: string, timeoutMs = 5000): Promise { + return new Promise((resolve) => { + const parsed = new URL(`${url.replace(/\/$/, '')}/healthz`); + const driver = parsed.protocol === 'https:' ? https : http; + const req = driver.get( + { + hostname: parsed.hostname, + port: parsed.port || (parsed.protocol === 'https:' ? 443 : 80), + path: parsed.pathname, + timeout: timeoutMs, + }, + (res) => { resolve((res.statusCode ?? 500) < 500); res.resume(); }, + ); + req.on('error', () => resolve(false)); + req.on('timeout', () => { req.destroy(); resolve(false); }); + }); +} + +function readToken(): string | null { + try { + const home = process.env.HOME ?? ''; + const path = `${home}/.mcpctl/credentials`; + // eslint-disable-next-line @typescript-eslint/no-require-imports + const fs = require('node:fs') as typeof import('node:fs'); + if (!fs.existsSync(path)) return null; + const raw = fs.readFileSync(path, 'utf-8'); + const parsed = JSON.parse(raw) as { token?: string }; + return parsed.token ?? null; + } catch { + return null; + } +} + +interface HttpResponse { status: number; body: string } + +function httpRequest(method: string, urlStr: string): Promise { + return new Promise((resolve, reject) => { + const tokenRaw = readToken(); + const parsed = new URL(urlStr); + const driver = parsed.protocol === 'https:' ? https : http; + const headers: Record = { + Accept: 'application/json', + ...(tokenRaw !== null ? { Authorization: `Bearer ${tokenRaw}` } : {}), + }; + const req = driver.request({ + hostname: parsed.hostname, + port: parsed.port || (parsed.protocol === 'https:' ? 443 : 80), + path: parsed.pathname + parsed.search, + method, + headers, + timeout: 30_000, + }, (res) => { + const chunks: Buffer[] = []; + res.on('data', (c: Buffer) => chunks.push(c)); + res.on('end', () => { + resolve({ status: res.statusCode ?? 0, body: Buffer.concat(chunks).toString('utf-8') }); + }); + }); + req.on('error', reject); + req.on('timeout', () => { req.destroy(); reject(new Error(`httpRequest timeout: ${method} ${urlStr}`)); }); + req.end(); + }); +} + +let mcpdUp = false; +let registrar: VirtualLlmRegistrar | null = null; +let tempDir: string; + +interface LlmListRow { + id: string; + name: string; + visibility?: 'public' | 'private'; + ownerId?: string | null; +} + +describe('virtual-LLM smoke — visibility (v7)', () => { + beforeAll(async () => { + mcpdUp = await healthz(MCPD_URL); + if (!mcpdUp) { + // eslint-disable-next-line no-console + console.warn(`\n ○ visibility smoke: skipped — ${MCPD_URL}/healthz unreachable.\n`); + return; + } + if (readToken() === null) { + mcpdUp = false; + // eslint-disable-next-line no-console + console.warn('\n ○ visibility smoke: skipped — no ~/.mcpctl/credentials.\n'); + return; + } + tempDir = mkdtempSync(join(tmpdir(), 'mcpctl-vis-smoke-')); + }, 20_000); + + afterAll(async () => { + if (registrar !== null) registrar.stop(); + if (tempDir !== undefined) rmSync(tempDir, { recursive: true, force: true }); + if (mcpdUp) { + const list = await httpRequest('GET', `${MCPD_URL}/api/v1/llms`); + if (list.status === 200) { + const rows = JSON.parse(list.body) as LlmListRow[]; + for (const target of [PUBLIC_NAME, PRIVATE_NAME]) { + const row = rows.find((r) => r.name === target); + if (row !== undefined) { + await httpRequest('DELETE', `${MCPD_URL}/api/v1/llms/${row.id}`); + } + } + } + } + }); + + it('publishes one public + one private virtual Llm and the list payload reflects both', async () => { + if (!mcpdUp) return; + const token = readToken(); + if (token === null) return; + const published: RegistrarPublishedProvider[] = [ + { provider: makeFakeProvider(PUBLIC_NAME), type: 'openai', model: 'fake-vis', tier: 'fast', visibility: 'public' }, + { provider: makeFakeProvider(PRIVATE_NAME), type: 'openai', model: 'fake-vis', tier: 'fast', visibility: 'private' }, + ]; + registrar = new VirtualLlmRegistrar({ + mcpdUrl: MCPD_URL, + token, + publishedProviders: published, + sessionFilePath: join(tempDir, 'session'), + log: { info: () => {}, warn: () => {}, error: () => {} }, + heartbeatIntervalMs: 60_000, + }); + await registrar.start(); + expect(registrar.getSessionId()).not.toBeNull(); + await new Promise((r) => setTimeout(r, 400)); + + const res = await httpRequest('GET', `${MCPD_URL}/api/v1/llms`); + expect(res.status).toBe(200); + const rows = JSON.parse(res.body) as LlmListRow[]; + + const pub = rows.find((r) => r.name === PUBLIC_NAME); + expect(pub, `${PUBLIC_NAME} must be visible to its owner`).toBeDefined(); + expect(pub!.visibility).toBe('public'); + // ownerId is the auth principal that ran register; non-empty proves + // mcpd actually stamped it on the row (otherwise the v7 register + // path would have left it NULL = legacy public). + expect(typeof pub!.ownerId).toBe('string'); + expect((pub!.ownerId ?? '').length).toBeGreaterThan(0); + + const priv = rows.find((r) => r.name === PRIVATE_NAME); + expect(priv, `${PRIVATE_NAME} must be visible to its owner (visibility filter is owner-bypass)`).toBeDefined(); + expect(priv!.visibility).toBe('private'); + expect(typeof priv!.ownerId).toBe('string'); + expect((priv!.ownerId ?? '').length).toBeGreaterThan(0); + + // Same publisher, same session — both rows must share the same owner. + expect(priv!.ownerId).toBe(pub!.ownerId); + }, 30_000); + + it('GET /api/v1/llms/ returns the row to its owner without 404', async () => { + if (!mcpdUp) return; + // Owner is calling — visibility filter must let the row through. A + // 404 here would indicate the service-layer filter is wrongly hiding + // it from the very user who created it. + const res = await httpRequest('GET', `${MCPD_URL}/api/v1/llms/${PRIVATE_NAME}`); + expect(res.status).toBe(200); + const row = JSON.parse(res.body) as LlmListRow; + expect(row.name).toBe(PRIVATE_NAME); + expect(row.visibility).toBe('private'); + }, 30_000); +});