fix(rbac): correct the non-CRUD verbs, and make the mapping testable
Closes 134/164. The five agentic-teams resources had RBAC slots but the
non-CRUD verbs had no cases, so they fell through to the method default and
every one of them read as `create:<resource>`. That is wrong in both
directions: it let anyone who may CREATE a task also ACCEPT one — the human gate
that fires a merge — and it refused a reviewer who may drive tasks but not
create them.
Corrected, following the verb the codebase already uses for "use this resource
to do work" (llms/infer, agents/chat, projects/chat):
- POST /tasks/:id/{transition,accept,reject,verdicts} → run:tasks:<name>
- {GET,POST} /gitproviders/:id/{repos,import,probe} → run:gitproviders:<name>.
Discovery calls OUT to the forge with its stored token, so holding
view:gitproviders should let you see that a forge is registered, not use its
credential.
- POST /epics/:id/{status,tradeoffs,criteria} → edit:epics:<name>. These rewrite
the vision record.
- GET /epics/:id/roadmap → view:epics:<name>
- {POST,DELETE} /epictasks/:id/edges → edit:epictasks:<name>
mapUrlToPermission moved from main.ts to middleware/url-permission.ts. main.ts
calls main() at module load, so nothing in it is importable from a test — an
authorization mapping that cannot be tested is an authorization mapping nobody
has checked, which is exactly how these verbs shipped mis-scoped.
15 tests now assert the mapping rather than the mere presence of the strings,
including that a bare create grant does NOT cover the gated verbs, that work
items and the inference queue never resolve to the same resource for any method,
and that the pre-existing mappings (llm infer, agent chat, operations, the
personalities/skills piggybacks) still hold.
Local: build clean, workspace 2478 passed (+15), lint back to the repo's
baseline 794 errors after making the five new match guards explicit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
This commit is contained in:
Michal
@@ -2829,9 +2829,9 @@
|
||||
"131",
|
||||
"133"
|
||||
],
|
||||
"status": "pending",
|
||||
"status": "done",
|
||||
"subtasks": [],
|
||||
"updatedAt": "2026-08-08T08:55:58.627Z"
|
||||
"updatedAt": "2026-08-09T20:55:31.457Z"
|
||||
},
|
||||
{
|
||||
"id": "135",
|
||||
@@ -3272,9 +3272,9 @@
|
||||
"dependencies": [
|
||||
"134"
|
||||
],
|
||||
"status": "pending",
|
||||
"status": "done",
|
||||
"subtasks": [],
|
||||
"updatedAt": "2026-08-09T19:36:34.052Z"
|
||||
"updatedAt": "2026-08-09T20:55:31.468Z"
|
||||
},
|
||||
{
|
||||
"id": "165",
|
||||
@@ -3419,12 +3419,12 @@
|
||||
],
|
||||
"metadata": {
|
||||
"version": "1.0.0",
|
||||
"lastModified": "2026-08-09T19:44:20.538Z",
|
||||
"taskCount": 173,
|
||||
"completedCount": 127,
|
||||
"lastModified": "2026-08-09T20:55:31.468Z",
|
||||
"taskCount": 174,
|
||||
"completedCount": 129,
|
||||
"tags": [
|
||||
"master"
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -56,7 +56,7 @@ import { PersonalityService } from './services/personality.service.js';
|
||||
import { registerPersonalityRoutes } from './routes/personalities.js';
|
||||
import { registerWebUi } from './routes/web-ui.js';
|
||||
import { bootstrapSystemProject } from './bootstrap/system-project.js';
|
||||
import { SET_OWN_PASSWORD_OPERATION } from './bootstrap/self-password-permission.js';
|
||||
import { mapUrlToPermission } from './middleware/url-permission.js';
|
||||
import { registerLogRoutes } from './routes/logs.js';
|
||||
import { errorLogBuffer } from './services/error-log-buffer.js';
|
||||
import { bootstrapSystemSkills } from './bootstrap/system-skills.js';
|
||||
@@ -84,7 +84,6 @@ import {
|
||||
AuditEventService,
|
||||
McpTokenService,
|
||||
} from './services/index.js';
|
||||
import type { RbacAction } from './services/index.js';
|
||||
import type { UpdateRbacDefinitionInput } from './validation/rbac-definition.schema.js';
|
||||
import { createAuthMiddleware } from './middleware/auth.js';
|
||||
import {
|
||||
@@ -138,196 +137,6 @@ import type { BackupKind } from './services/backup/yaml-serializer.js';
|
||||
import { ResourceRuleRegistry } from './validation/resource-rules.js';
|
||||
import { systemPromptVarsRule } from './validation/rules/system-prompt-vars.js';
|
||||
|
||||
type PermissionCheck =
|
||||
| { kind: 'resource'; resource: string; action: RbacAction; resourceName?: string }
|
||||
| { kind: 'operation'; operation: string }
|
||||
| { kind: 'skip' };
|
||||
|
||||
/**
|
||||
* Map an HTTP method + URL to a permission check.
|
||||
* Returns 'skip' for URLs that should not be RBAC-checked.
|
||||
*/
|
||||
function mapUrlToPermission(method: string, url: string): PermissionCheck {
|
||||
const match = url.match(/^\/api\/v1\/([a-z-]+)/);
|
||||
if (!match) return { kind: 'skip' };
|
||||
|
||||
const segment = match[1] as string;
|
||||
|
||||
// Recent mcpd error logs — read-only operator view, gated by `logs`.
|
||||
if (url.startsWith('/api/v1/logs')) {
|
||||
return { kind: 'operation', operation: 'logs' };
|
||||
}
|
||||
|
||||
// Self-service password change — gated by the `set-own-password` operation
|
||||
// (a default, admin-revocable permission), NOT the broad edit:users that an
|
||||
// admin reset of another user needs. Must precede the generic users mapping.
|
||||
if (url.startsWith('/api/v1/users/me/password')) {
|
||||
return { kind: 'operation', operation: SET_OWN_PASSWORD_OPERATION };
|
||||
}
|
||||
|
||||
// Operations (non-resource endpoints)
|
||||
if (segment === 'backup') return { kind: 'operation', operation: 'backup' };
|
||||
if (segment === 'restore') return { kind: 'operation', operation: 'restore' };
|
||||
if (segment === 'audit-logs' && method === 'DELETE') return { kind: 'operation', operation: 'audit-purge' };
|
||||
// /api/v1/secrets/migrate is a bulk cross-backend operation — treat as op, not a plain secret write.
|
||||
if (url.startsWith('/api/v1/secrets/migrate')) return { kind: 'operation', operation: 'migrate-secrets' };
|
||||
// /api/v1/secretbackends/:id/rotate — manual rotation trigger. Operation so
|
||||
// only explicitly-granted callers can force it (the loop itself bypasses
|
||||
// RBAC by calling the rotator in-process).
|
||||
if (/^\/api\/v1\/secretbackends\/[^/?]+\/rotate/.test(url)) {
|
||||
return { kind: 'operation', operation: 'rotate-secretbackend' };
|
||||
}
|
||||
|
||||
// /api/v1/llms/:name/infer → `run:llms:<name>` (not the default create:llms).
|
||||
const inferMatch = url.match(/^\/api\/v1\/llms\/([^/?]+)\/infer/);
|
||||
if (inferMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'llms', action: 'run', resourceName: inferMatch[1] };
|
||||
}
|
||||
|
||||
// /api/v1/agents/:name/chat or /threads* → `run:agents:<name>`.
|
||||
// Driving a turn or managing its history is a "run" on the agent — listing
|
||||
// and CRUD continue to fall through to the default mapping below.
|
||||
const agentRunMatch = url.match(/^\/api\/v1\/agents\/([^/?]+)\/(chat|threads)/);
|
||||
if (agentRunMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'agents', action: 'run', resourceName: agentRunMatch[1] };
|
||||
}
|
||||
|
||||
// /api/v1/threads/:id/messages → `view:agents` (we don't carry the agent
|
||||
// name in the URL; the service-level owner check enforces fine-grained
|
||||
// access on top).
|
||||
if (url.startsWith('/api/v1/threads/')) {
|
||||
return { kind: 'resource', resource: 'agents', action: 'view' };
|
||||
}
|
||||
|
||||
const resourceMap: Record<string, string | undefined> = {
|
||||
'servers': 'servers',
|
||||
'instances': 'instances',
|
||||
'secrets': 'secrets',
|
||||
'secretbackends': 'secretbackends',
|
||||
'projects': 'projects',
|
||||
'templates': 'templates',
|
||||
'users': 'users',
|
||||
'groups': 'groups',
|
||||
'rbac': 'rbac',
|
||||
'audit-logs': 'rbac',
|
||||
'mcp': 'servers',
|
||||
'prompts': 'prompts',
|
||||
'promptrequests': 'promptrequests',
|
||||
// PR-2: revisions/proposals piggyback on the prompts permission for now.
|
||||
// Anyone with view:prompts can read history; anyone with edit:prompts can
|
||||
// approve/reject proposals. PR-7 may split these out if RBAC granularity
|
||||
// becomes useful (e.g., a "reviewer" role).
|
||||
'revisions': 'prompts',
|
||||
'proposals': 'prompts',
|
||||
// PR-3: skills follow prompts for RBAC. A "skills" RBAC slot can be
|
||||
// split out later if the operator wants to scope skill writes more
|
||||
// tightly than prompt writes — for now, a senior reviewer who can
|
||||
// edit prompts can edit skills.
|
||||
'skills': 'prompts',
|
||||
'mcptokens': 'mcptokens',
|
||||
'llms': 'llms',
|
||||
// v5: durable inference task queue. Same default action mapping as
|
||||
// any other resource (GET=view, POST=create, DELETE=delete). The
|
||||
// route layer enforces owner-scoping on top — a caller without
|
||||
// resource-wide grant only sees their own tasks.
|
||||
//
|
||||
// Maps to its own `inference-tasks` resource, NOT `tasks` — agentic-teams
|
||||
// work items own `tasks` now. Pre-split grants are rewritten by
|
||||
// backfillInferenceTaskBindings at startup.
|
||||
'inference-tasks': 'inference-tasks',
|
||||
'agents': 'agents',
|
||||
// Personalities inherit the agent's RBAC: managing a personality
|
||||
// requires view/edit/create/delete on the `agents` resource.
|
||||
'personalities': 'agents',
|
||||
// ── Agentic teams ──
|
||||
'epics': 'epics',
|
||||
'epictasks': 'epictasks',
|
||||
// Work items. NOT inference-tasks — see the rbac-tasks-backfill bootstrap.
|
||||
'tasks': 'tasks',
|
||||
'repos': 'repos',
|
||||
'gitproviders': 'gitproviders',
|
||||
};
|
||||
|
||||
const resource = resourceMap[segment];
|
||||
if (resource === undefined) return { kind: 'skip' };
|
||||
|
||||
// Special case: /api/v1/promptrequests/:id/approve → needs both delete+promptrequests and create+prompts
|
||||
// We check delete on promptrequests (the harder permission); create on prompts is checked in the service layer
|
||||
const approveMatch = url.match(/^\/api\/v1\/promptrequests\/([^/?]+)\/approve/);
|
||||
if (approveMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'promptrequests', action: 'delete', resourceName: approveMatch[1] };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/mcptokens/:id/revoke → treated as 'delete' on the token.
|
||||
const revokeMatch = url.match(/^\/api\/v1\/mcptokens\/([^/?]+)\/revoke/);
|
||||
if (revokeMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'mcptokens', action: 'delete', resourceName: revokeMatch[1] };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:name/(chat|threads) → run:projects:<name>.
|
||||
// Driving a project chat turn or managing its threads is a "run" on the
|
||||
// project; per-thread ownership + admin-delete are enforced in the service.
|
||||
const projectRunMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/(chat|threads)/);
|
||||
if (projectRunMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'projects', action: 'run', resourceName: projectRunMatch[1] };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:name/prompts/visible → view prompts
|
||||
const visiblePromptsMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/prompts\/visible/);
|
||||
if (visiblePromptsMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'prompts', action: 'view' };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:name/promptrequests → create promptrequests
|
||||
const projectPromptrequestsMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/promptrequests/);
|
||||
if (projectPromptrequestsMatch?.[1] && method === 'POST') {
|
||||
return { kind: 'resource', resource: 'promptrequests', action: 'create' };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:id/instructions → view projects
|
||||
const instructionsMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/instructions/);
|
||||
if (instructionsMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'projects', action: 'view', resourceName: instructionsMatch[1] };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:id/mcp-config → requires 'expose' permission
|
||||
const mcpConfigMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/mcp-config/);
|
||||
if (mcpConfigMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'projects', action: 'expose', resourceName: mcpConfigMatch[1] };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:id/servers — attach/detach requires 'edit'
|
||||
const projectServersMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/servers/);
|
||||
if (projectServersMatch?.[1] && method !== 'GET') {
|
||||
return { kind: 'resource', resource: 'projects', action: 'edit', resourceName: projectServersMatch[1] };
|
||||
}
|
||||
|
||||
// Map HTTP method to action
|
||||
let action: RbacAction;
|
||||
switch (method) {
|
||||
case 'GET':
|
||||
case 'HEAD':
|
||||
action = 'view';
|
||||
break;
|
||||
case 'POST':
|
||||
action = 'create';
|
||||
break;
|
||||
case 'DELETE':
|
||||
action = 'delete';
|
||||
break;
|
||||
default: // PUT, PATCH
|
||||
action = 'edit';
|
||||
break;
|
||||
}
|
||||
|
||||
// Extract resource name/ID from URL (3rd segment: /api/v1/servers/:nameOrId)
|
||||
const nameMatch = url.match(/^\/api\/v1\/[a-z-]+\/([^/?]+)/);
|
||||
const resourceName = nameMatch?.[1];
|
||||
|
||||
const check: PermissionCheck = { kind: 'resource', resource, action };
|
||||
if (resourceName !== undefined) (check as { resourceName: string }).resourceName = resourceName;
|
||||
return check;
|
||||
}
|
||||
|
||||
/** Operations an admin-equivalent user should always be able to run. */
|
||||
const ADMIN_OPS = [
|
||||
|
||||
247
src/mcpd/src/middleware/url-permission.ts
Normal file
247
src/mcpd/src/middleware/url-permission.ts
Normal file
@@ -0,0 +1,247 @@
|
||||
/**
|
||||
* HTTP method + URL → RBAC permission check.
|
||||
*
|
||||
* Extracted from main.ts so it can be tested directly: main.ts calls main() at
|
||||
* module load, so nothing there is importable from a test, and an
|
||||
* authorization mapping that cannot be tested is an authorization mapping
|
||||
* nobody has checked.
|
||||
*/
|
||||
import { SET_OWN_PASSWORD_OPERATION } from '../bootstrap/self-password-permission.js';
|
||||
import type { RbacAction } from '../services/index.js';
|
||||
|
||||
export type PermissionCheck =
|
||||
| { kind: 'resource'; resource: string; action: RbacAction; resourceName?: string }
|
||||
| { kind: 'operation'; operation: string }
|
||||
| { kind: 'skip' };
|
||||
|
||||
/**
|
||||
* Map an HTTP method + URL to a permission check.
|
||||
* Returns 'skip' for URLs that should not be RBAC-checked.
|
||||
*/
|
||||
export function mapUrlToPermission(method: string, url: string): PermissionCheck {
|
||||
const match = url.match(/^\/api\/v1\/([a-z-]+)/);
|
||||
if (!match) return { kind: 'skip' };
|
||||
|
||||
const segment = match[1] as string;
|
||||
|
||||
// Recent mcpd error logs — read-only operator view, gated by `logs`.
|
||||
if (url.startsWith('/api/v1/logs')) {
|
||||
return { kind: 'operation', operation: 'logs' };
|
||||
}
|
||||
|
||||
// Self-service password change — gated by the `set-own-password` operation
|
||||
// (a default, admin-revocable permission), NOT the broad edit:users that an
|
||||
// admin reset of another user needs. Must precede the generic users mapping.
|
||||
if (url.startsWith('/api/v1/users/me/password')) {
|
||||
return { kind: 'operation', operation: SET_OWN_PASSWORD_OPERATION };
|
||||
}
|
||||
|
||||
// Operations (non-resource endpoints)
|
||||
if (segment === 'backup') return { kind: 'operation', operation: 'backup' };
|
||||
if (segment === 'restore') return { kind: 'operation', operation: 'restore' };
|
||||
if (segment === 'audit-logs' && method === 'DELETE') return { kind: 'operation', operation: 'audit-purge' };
|
||||
// /api/v1/secrets/migrate is a bulk cross-backend operation — treat as op, not a plain secret write.
|
||||
if (url.startsWith('/api/v1/secrets/migrate')) return { kind: 'operation', operation: 'migrate-secrets' };
|
||||
// /api/v1/secretbackends/:id/rotate — manual rotation trigger. Operation so
|
||||
// only explicitly-granted callers can force it (the loop itself bypasses
|
||||
// RBAC by calling the rotator in-process).
|
||||
if (/^\/api\/v1\/secretbackends\/[^/?]+\/rotate/.test(url)) {
|
||||
return { kind: 'operation', operation: 'rotate-secretbackend' };
|
||||
}
|
||||
|
||||
// /api/v1/llms/:name/infer → `run:llms:<name>` (not the default create:llms).
|
||||
const inferMatch = url.match(/^\/api\/v1\/llms\/([^/?]+)\/infer/);
|
||||
if (inferMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'llms', action: 'run', resourceName: inferMatch[1] };
|
||||
}
|
||||
|
||||
// /api/v1/agents/:name/chat or /threads* → `run:agents:<name>`.
|
||||
// Driving a turn or managing its history is a "run" on the agent — listing
|
||||
// and CRUD continue to fall through to the default mapping below.
|
||||
const agentRunMatch = url.match(/^\/api\/v1\/agents\/([^/?]+)\/(chat|threads)/);
|
||||
if (agentRunMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'agents', action: 'run', resourceName: agentRunMatch[1] };
|
||||
}
|
||||
|
||||
// ── Agentic teams: non-CRUD verbs ──
|
||||
//
|
||||
// Without these the default method mapping applies and every one of these
|
||||
// POSTs reads as `create:<resource>`, which is wrong in both directions:
|
||||
// it lets anyone who may create a task also accept one, and it refuses a
|
||||
// reviewer who may drive tasks but not create them.
|
||||
//
|
||||
// `run` is the existing verb for "use this resource to do work" (llms/infer,
|
||||
// agents/chat, projects/chat), so driving a task's lifecycle and exercising a
|
||||
// forge's credential both belong to it. Mutations of the vision record itself
|
||||
// stay `edit`.
|
||||
|
||||
// Driving a work item: transition, the human accept gate, reject, and
|
||||
// recording a stage verdict. Separable from `edit:tasks`, which is field edits.
|
||||
const taskRunMatch = url.match(/^\/api\/v1\/tasks\/([^/?]+)\/(transition|accept|reject|verdicts)/);
|
||||
if (taskRunMatch?.[1] !== undefined) {
|
||||
return { kind: 'resource', resource: 'tasks', action: 'run', resourceName: taskRunMatch[1] };
|
||||
}
|
||||
|
||||
// Discovery and import call OUT to the forge using its stored credential, so
|
||||
// they are `run`, not `view`/`create` — holding view:gitproviders should not
|
||||
// let you exercise the token.
|
||||
const forgeRunMatch = url.match(/^\/api\/v1\/gitproviders\/([^/?]+)\/(repos|import|probe)/);
|
||||
if (forgeRunMatch?.[1] !== undefined) {
|
||||
return { kind: 'resource', resource: 'gitproviders', action: 'run', resourceName: forgeRunMatch[1] };
|
||||
}
|
||||
|
||||
// Marking an Epic delivered, recording an accepted tradeoff, or ticking a
|
||||
// criterion all rewrite the vision record → `edit`, same as editing content.
|
||||
const epicEditMatch = url.match(/^\/api\/v1\/epics\/([^/?]+)\/(status|tradeoffs|criteria)/);
|
||||
if (epicEditMatch?.[1] !== undefined) {
|
||||
return { kind: 'resource', resource: 'epics', action: 'edit', resourceName: epicEditMatch[1] };
|
||||
}
|
||||
|
||||
// The roadmap is a read of the epic's graph.
|
||||
const roadmapMatch = url.match(/^\/api\/v1\/epics\/([^/?]+)\/roadmap/);
|
||||
if (roadmapMatch?.[1] !== undefined) {
|
||||
return { kind: 'resource', resource: 'epics', action: 'view', resourceName: roadmapMatch[1] };
|
||||
}
|
||||
|
||||
// Chaining roadmap nodes edits the graph.
|
||||
const edgeMatch = url.match(/^\/api\/v1\/epictasks\/([^/?]+)\/edges/);
|
||||
if (edgeMatch?.[1] !== undefined) {
|
||||
return { kind: 'resource', resource: 'epictasks', action: 'edit', resourceName: edgeMatch[1] };
|
||||
}
|
||||
|
||||
// /api/v1/threads/:id/messages → `view:agents` (we don't carry the agent
|
||||
// name in the URL; the service-level owner check enforces fine-grained
|
||||
// access on top).
|
||||
if (url.startsWith('/api/v1/threads/')) {
|
||||
return { kind: 'resource', resource: 'agents', action: 'view' };
|
||||
}
|
||||
|
||||
const resourceMap: Record<string, string | undefined> = {
|
||||
'servers': 'servers',
|
||||
'instances': 'instances',
|
||||
'secrets': 'secrets',
|
||||
'secretbackends': 'secretbackends',
|
||||
'projects': 'projects',
|
||||
'templates': 'templates',
|
||||
'users': 'users',
|
||||
'groups': 'groups',
|
||||
'rbac': 'rbac',
|
||||
'audit-logs': 'rbac',
|
||||
'mcp': 'servers',
|
||||
'prompts': 'prompts',
|
||||
'promptrequests': 'promptrequests',
|
||||
// PR-2: revisions/proposals piggyback on the prompts permission for now.
|
||||
// Anyone with view:prompts can read history; anyone with edit:prompts can
|
||||
// approve/reject proposals. PR-7 may split these out if RBAC granularity
|
||||
// becomes useful (e.g., a "reviewer" role).
|
||||
'revisions': 'prompts',
|
||||
'proposals': 'prompts',
|
||||
// PR-3: skills follow prompts for RBAC. A "skills" RBAC slot can be
|
||||
// split out later if the operator wants to scope skill writes more
|
||||
// tightly than prompt writes — for now, a senior reviewer who can
|
||||
// edit prompts can edit skills.
|
||||
'skills': 'prompts',
|
||||
'mcptokens': 'mcptokens',
|
||||
'llms': 'llms',
|
||||
// v5: durable inference task queue. Same default action mapping as
|
||||
// any other resource (GET=view, POST=create, DELETE=delete). The
|
||||
// route layer enforces owner-scoping on top — a caller without
|
||||
// resource-wide grant only sees their own tasks.
|
||||
//
|
||||
// Maps to its own `inference-tasks` resource, NOT `tasks` — agentic-teams
|
||||
// work items own `tasks` now. Pre-split grants are rewritten by
|
||||
// backfillInferenceTaskBindings at startup.
|
||||
'inference-tasks': 'inference-tasks',
|
||||
'agents': 'agents',
|
||||
// Personalities inherit the agent's RBAC: managing a personality
|
||||
// requires view/edit/create/delete on the `agents` resource.
|
||||
'personalities': 'agents',
|
||||
// ── Agentic teams ──
|
||||
'epics': 'epics',
|
||||
'epictasks': 'epictasks',
|
||||
// Work items. NOT inference-tasks — see the rbac-tasks-backfill bootstrap.
|
||||
'tasks': 'tasks',
|
||||
'repos': 'repos',
|
||||
'gitproviders': 'gitproviders',
|
||||
};
|
||||
|
||||
const resource = resourceMap[segment];
|
||||
if (resource === undefined) return { kind: 'skip' };
|
||||
|
||||
// Special case: /api/v1/promptrequests/:id/approve → needs both delete+promptrequests and create+prompts
|
||||
// We check delete on promptrequests (the harder permission); create on prompts is checked in the service layer
|
||||
const approveMatch = url.match(/^\/api\/v1\/promptrequests\/([^/?]+)\/approve/);
|
||||
if (approveMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'promptrequests', action: 'delete', resourceName: approveMatch[1] };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/mcptokens/:id/revoke → treated as 'delete' on the token.
|
||||
const revokeMatch = url.match(/^\/api\/v1\/mcptokens\/([^/?]+)\/revoke/);
|
||||
if (revokeMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'mcptokens', action: 'delete', resourceName: revokeMatch[1] };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:name/(chat|threads) → run:projects:<name>.
|
||||
// Driving a project chat turn or managing its threads is a "run" on the
|
||||
// project; per-thread ownership + admin-delete are enforced in the service.
|
||||
const projectRunMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/(chat|threads)/);
|
||||
if (projectRunMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'projects', action: 'run', resourceName: projectRunMatch[1] };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:name/prompts/visible → view prompts
|
||||
const visiblePromptsMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/prompts\/visible/);
|
||||
if (visiblePromptsMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'prompts', action: 'view' };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:name/promptrequests → create promptrequests
|
||||
const projectPromptrequestsMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/promptrequests/);
|
||||
if (projectPromptrequestsMatch?.[1] && method === 'POST') {
|
||||
return { kind: 'resource', resource: 'promptrequests', action: 'create' };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:id/instructions → view projects
|
||||
const instructionsMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/instructions/);
|
||||
if (instructionsMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'projects', action: 'view', resourceName: instructionsMatch[1] };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:id/mcp-config → requires 'expose' permission
|
||||
const mcpConfigMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/mcp-config/);
|
||||
if (mcpConfigMatch?.[1]) {
|
||||
return { kind: 'resource', resource: 'projects', action: 'expose', resourceName: mcpConfigMatch[1] };
|
||||
}
|
||||
|
||||
// Special case: /api/v1/projects/:id/servers — attach/detach requires 'edit'
|
||||
const projectServersMatch = url.match(/^\/api\/v1\/projects\/([^/?]+)\/servers/);
|
||||
if (projectServersMatch?.[1] && method !== 'GET') {
|
||||
return { kind: 'resource', resource: 'projects', action: 'edit', resourceName: projectServersMatch[1] };
|
||||
}
|
||||
|
||||
// Map HTTP method to action
|
||||
let action: RbacAction;
|
||||
switch (method) {
|
||||
case 'GET':
|
||||
case 'HEAD':
|
||||
action = 'view';
|
||||
break;
|
||||
case 'POST':
|
||||
action = 'create';
|
||||
break;
|
||||
case 'DELETE':
|
||||
action = 'delete';
|
||||
break;
|
||||
default: // PUT, PATCH
|
||||
action = 'edit';
|
||||
break;
|
||||
}
|
||||
|
||||
// Extract resource name/ID from URL (3rd segment: /api/v1/servers/:nameOrId)
|
||||
const nameMatch = url.match(/^\/api\/v1\/[a-z-]+\/([^/?]+)/);
|
||||
const resourceName = nameMatch?.[1];
|
||||
|
||||
const check: PermissionCheck = { kind: 'resource', resource, action };
|
||||
if (resourceName !== undefined) (check as { resourceName: string }).resourceName = resourceName;
|
||||
return check;
|
||||
}
|
||||
137
src/mcpd/tests/url-permission.test.ts
Normal file
137
src/mcpd/tests/url-permission.test.ts
Normal file
@@ -0,0 +1,137 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
|
||||
import { mapUrlToPermission } from '../src/middleware/url-permission.js';
|
||||
|
||||
/**
|
||||
* The agentic-teams resources shipped with five new RBAC slots and a namespace
|
||||
* split, and nothing tested that the mapping was right — only that the strings
|
||||
* existed. These assert the mapping itself, which is what actually decides
|
||||
* whether a request is allowed.
|
||||
*
|
||||
* The non-CRUD verbs matter most. Without an explicit case they fall through to
|
||||
* the method default, so every POST reads as `create:<resource>` — which both
|
||||
* lets anyone who may create a task also ACCEPT one, and refuses a reviewer who
|
||||
* may drive tasks but not create them.
|
||||
*/
|
||||
const perm = (method: string, url: string) => mapUrlToPermission(method, url);
|
||||
|
||||
describe('agentic-teams CRUD mapping', () => {
|
||||
it('maps each resource to its own slot', () => {
|
||||
for (const [segment, resource] of [
|
||||
['epics', 'epics'],
|
||||
['epictasks', 'epictasks'],
|
||||
['tasks', 'tasks'],
|
||||
['repos', 'repos'],
|
||||
['gitproviders', 'gitproviders'],
|
||||
] as const) {
|
||||
expect(perm('GET', `/api/v1/${segment}`)).toMatchObject({ kind: 'resource', resource, action: 'view' });
|
||||
}
|
||||
});
|
||||
|
||||
it('maps HTTP methods to the usual actions', () => {
|
||||
expect(perm('GET', '/api/v1/epics')).toMatchObject({ action: 'view' });
|
||||
expect(perm('POST', '/api/v1/epics')).toMatchObject({ action: 'create' });
|
||||
expect(perm('PUT', '/api/v1/epics/e1')).toMatchObject({ action: 'edit' });
|
||||
expect(perm('DELETE', '/api/v1/epics/e1')).toMatchObject({ action: 'delete' });
|
||||
});
|
||||
|
||||
it('carries the resource name so name-scoped grants can match', () => {
|
||||
expect(perm('GET', '/api/v1/epics/the-car')).toMatchObject({ resourceName: 'the-car' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('the tasks namespace split', () => {
|
||||
it('routes work items to `tasks`', () => {
|
||||
expect(perm('GET', '/api/v1/tasks')).toMatchObject({ resource: 'tasks' });
|
||||
});
|
||||
|
||||
it('routes the inference queue to `inference-tasks`, NOT `tasks`', () => {
|
||||
// Before the split both landed on `tasks`, so a grant for one silently
|
||||
// authorized the other.
|
||||
const p = perm('GET', '/api/v1/inference-tasks');
|
||||
expect(p).toMatchObject({ resource: 'inference-tasks' });
|
||||
expect(p).not.toMatchObject({ resource: 'tasks' });
|
||||
});
|
||||
|
||||
it('keeps the two apart for every method', () => {
|
||||
for (const m of ['GET', 'POST', 'DELETE']) {
|
||||
const work = perm(m, '/api/v1/tasks');
|
||||
const infer = perm(m, '/api/v1/inference-tasks');
|
||||
expect((work as { resource: string }).resource).not.toBe((infer as { resource: string }).resource);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('non-CRUD verbs', () => {
|
||||
it('treats driving a task as `run`, not `create`', () => {
|
||||
// Accepting a task fires a merge. That must be grantable separately from
|
||||
// "may create tasks", and separately from "may edit task fields".
|
||||
for (const verb of ['transition', 'accept', 'reject', 'verdicts']) {
|
||||
expect(perm('POST', `/api/v1/tasks/wire-cli/${verb}`)).toEqual({
|
||||
kind: 'resource', resource: 'tasks', action: 'run', resourceName: 'wire-cli',
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it('treats forge discovery and import as `run` — they exercise the credential', () => {
|
||||
// view:gitproviders should let you see that a forge is registered, not use
|
||||
// its token to call out.
|
||||
for (const verb of ['repos', 'import', 'probe']) {
|
||||
expect(perm('POST', `/api/v1/gitproviders/mysources/${verb}`)).toEqual({
|
||||
kind: 'resource', resource: 'gitproviders', action: 'run', resourceName: 'mysources',
|
||||
});
|
||||
}
|
||||
expect(perm('GET', '/api/v1/gitproviders/mysources/repos')).toMatchObject({ action: 'run' });
|
||||
});
|
||||
|
||||
it('treats rewriting the vision record as `edit`', () => {
|
||||
for (const verb of ['status', 'tradeoffs', 'criteria/builds']) {
|
||||
expect(perm('POST', `/api/v1/epics/the-car/${verb}`)).toMatchObject({
|
||||
resource: 'epics', action: 'edit', resourceName: 'the-car',
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it('treats the roadmap as a read', () => {
|
||||
expect(perm('GET', '/api/v1/epics/the-car/roadmap')).toMatchObject({
|
||||
resource: 'epics', action: 'view',
|
||||
});
|
||||
});
|
||||
|
||||
it('treats chaining roadmap nodes as an edit of the graph', () => {
|
||||
expect(perm('POST', '/api/v1/epictasks/chassis/edges')).toMatchObject({
|
||||
resource: 'epictasks', action: 'edit', resourceName: 'chassis',
|
||||
});
|
||||
expect(perm('DELETE', '/api/v1/epictasks/chassis/edges')).toMatchObject({
|
||||
resource: 'epictasks', action: 'edit',
|
||||
});
|
||||
});
|
||||
|
||||
it('does NOT let a bare create grant cover the gated verbs', () => {
|
||||
// The regression this whole block exists to prevent.
|
||||
const create = perm('POST', '/api/v1/tasks');
|
||||
const accept = perm('POST', '/api/v1/tasks/wire-cli/accept');
|
||||
expect((create as { action: string }).action).toBe('create');
|
||||
expect((accept as { action: string }).action).toBe('run');
|
||||
});
|
||||
});
|
||||
|
||||
describe('existing mappings still hold', () => {
|
||||
it('leaves the established run-verbs alone', () => {
|
||||
expect(perm('POST', '/api/v1/llms/vllm-current/infer')).toMatchObject({ resource: 'llms', action: 'run' });
|
||||
expect(perm('POST', '/api/v1/agents/reviewer/chat')).toMatchObject({ resource: 'agents', action: 'run' });
|
||||
expect(perm('POST', '/api/v1/projects/sre/chat')).toMatchObject({ resource: 'projects', action: 'run' });
|
||||
});
|
||||
|
||||
it('still gates operations and skips unknown paths', () => {
|
||||
expect(perm('GET', '/api/v1/logs')).toMatchObject({ kind: 'operation', operation: 'logs' });
|
||||
expect(perm('POST', '/api/v1/backup')).toMatchObject({ kind: 'operation', operation: 'backup' });
|
||||
expect(perm('GET', '/healthz')).toEqual({ kind: 'skip' });
|
||||
expect(perm('GET', '/api/v1/unknownthing')).toEqual({ kind: 'skip' });
|
||||
});
|
||||
|
||||
it('keeps personalities and skills piggybacked where they were', () => {
|
||||
expect(perm('GET', '/api/v1/personalities')).toMatchObject({ resource: 'agents' });
|
||||
expect(perm('GET', '/api/v1/skills')).toMatchObject({ resource: 'prompts' });
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user