feat(rbac): bound what an agent may DO by whoever invoked it
Closes 177. An agent carries a project's prompts and its MCP tools, and the only check on a chat turn was run:agents:<name> - so anyone who could talk to an agent inherited tool access to every project that agent had joined. Membership was the capability. AgentProject widened it from one pinned project to many. The rule now: knowledge travels with the agent, action is bounded by the authority of the run. Prompts and skills still load regardless of who is asking - a QA reviewer that forgets the project's conventions when a different person asks is not a reviewer. Every MCP tool call is gated. Authority is resolved once per run (services/run-authority.ts) and is per-RUN, not per-agent: the same agent answers a human under that human's authority and, once a scheduler exists, runs unattended under its own. `caller` intersects the agent's memberships with the caller's run:projects - reusing the exact grant the project-chat route already demands, rather than inventing a parallel permission. `agent` takes every membership without consulting a user. Both are implemented; `service-account` throws 501 rather than honouring a forgeable x-service-account header, and task 179 records what it needs first. Gated in BOTH listTools and callTool, deliberately: withholding a tool from the list is presentation, not enforcement - a model can invent a tool name it was never shown, and callTool is what would run it. The accepted consequence is stated plainly rather than papered over: a user can ask about a project they cannot reach and the agent will answer from its knowledge. The system block tells the model when it is holding such context so a persona can apply it without reciting it. That is guidance to a model, not a boundary - the boundary is that the tools are absent. Two sibling holes found while exploring and closed here: - Thread hijack. resolveThreadId on the agent path had no owner check, so anyone with run:agents:<name> could append to another user's thread by id and read its history back. The project path already did this correctly; mirrored, 404 not 403 so ids cannot be probed. - McpToken project escape. `project` travels in the request BODY, so the URL-based token scope check in main.ts never saw it - a token bound to project A could drive an agent into B's tools. The binding is now a ceiling. Tests: 19 authority tests (intersection, fail-closed with no user and with no RBAC wired, token ceiling, agent authority bypassing RBAC entirely, the 501) and 4 through the real chat path asserting prompts load while tools do not. Three existing assertions updated for the new dispatcher signature - a deliberate interface change, not tests bent to fit. Local: build clean, workspace 2608 passed (+16), lint unchanged at 794 errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p
This commit is contained in:
133
src/mcpd/tests/services/run-authority.test.ts
Normal file
133
src/mcpd/tests/services/run-authority.test.ts
Normal file
@@ -0,0 +1,133 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
|
||||
import type { RbacService } from '../../src/services/rbac.service.js';
|
||||
import {
|
||||
RunAuthorityResolver,
|
||||
restrictedContextNotice,
|
||||
unscopedAuthority,
|
||||
} from '../../src/services/run-authority.js';
|
||||
|
||||
/**
|
||||
* An agent carries a project's prompts AND its MCP tools. Before this, the only
|
||||
* check on a chat turn was `run:agents:<name>` — so anyone who could talk to an
|
||||
* agent inherited tool access to every project that agent had joined.
|
||||
*
|
||||
* The rule these pin: knowledge travels with the agent, action is bounded by
|
||||
* whoever invoked it.
|
||||
*/
|
||||
function rbac(allowed: string[]): RbacService {
|
||||
return {
|
||||
canAccess: vi.fn(async (_u: string, action: string, resource: string, name?: string) =>
|
||||
action === 'run' && resource === 'projects' && name !== undefined && allowed.includes(name),
|
||||
),
|
||||
} as unknown as RbacService;
|
||||
}
|
||||
|
||||
const SRE = { id: 'p-sre', name: 'sre' };
|
||||
const PAY = { id: 'p-pay', name: 'payments' };
|
||||
|
||||
describe('caller authority', () => {
|
||||
it('grants tools only for projects the caller can reach', async () => {
|
||||
const r = new RunAuthorityResolver(rbac(['sre']));
|
||||
const a = await r.resolve({ kind: 'caller', userId: 'u1', projects: [SRE, PAY] });
|
||||
|
||||
expect([...a.toolProjects]).toEqual(['p-sre']);
|
||||
expect(a.restrictedProjects).toEqual(['payments']);
|
||||
});
|
||||
|
||||
it('still lists the unreachable project as restricted, not absent', async () => {
|
||||
// The distinction matters: the prompts DO load, so the model has to be told
|
||||
// it is holding context it cannot act on.
|
||||
const r = new RunAuthorityResolver(rbac([]));
|
||||
const a = await r.resolve({ kind: 'caller', userId: 'u1', projects: [SRE, PAY] });
|
||||
|
||||
expect(a.toolProjects.size).toBe(0);
|
||||
expect(a.restrictedProjects).toEqual(['sre', 'payments']);
|
||||
});
|
||||
|
||||
it('gives full tool access when the caller can reach everything', async () => {
|
||||
const r = new RunAuthorityResolver(rbac(['sre', 'payments']));
|
||||
const a = await r.resolve({ kind: 'caller', userId: 'u1', projects: [SRE, PAY] });
|
||||
expect(a.toolProjects.size).toBe(2);
|
||||
expect(a.restrictedProjects).toEqual([]);
|
||||
});
|
||||
|
||||
it('asks RBAC the same question the project-chat route asks', async () => {
|
||||
// Reusing `run:projects:<name>` rather than inventing a parallel permission
|
||||
// is the point — one grant means the same thing on both paths.
|
||||
const svc = rbac(['sre']);
|
||||
await new RunAuthorityResolver(svc).resolve({ kind: 'caller', userId: 'u1', projects: [SRE] });
|
||||
expect(svc.canAccess).toHaveBeenCalledWith('u1', 'run', 'projects', 'sre');
|
||||
});
|
||||
|
||||
it('fails closed with no user', async () => {
|
||||
const r = new RunAuthorityResolver(rbac(['sre']));
|
||||
const a = await r.resolve({ kind: 'caller', projects: [SRE] });
|
||||
expect(a.toolProjects.size).toBe(0);
|
||||
});
|
||||
|
||||
it('fails closed when RBAC is not wired', async () => {
|
||||
// Older harnesses construct ChatService without it. Knowledge and no tools
|
||||
// is the safe degradation; the reverse would be a silent grant.
|
||||
const a = await new RunAuthorityResolver().resolve({ kind: 'caller', userId: 'u1', projects: [SRE] });
|
||||
expect(a.toolProjects.size).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('McpToken project ceiling', () => {
|
||||
it('refuses projects outside the token binding even when RBAC would allow', async () => {
|
||||
// `project` travels in the request BODY, so the URL-based token scope check
|
||||
// in main.ts never sees it. Without this a token for sre could drive an
|
||||
// agent into payments' tools.
|
||||
const r = new RunAuthorityResolver(rbac(['sre', 'payments']));
|
||||
const a = await r.resolve({
|
||||
kind: 'caller', userId: 'u1', projects: [SRE, PAY], mcpTokenProjectName: 'sre',
|
||||
});
|
||||
|
||||
expect([...a.toolProjects]).toEqual(['p-sre']);
|
||||
expect(a.restrictedProjects).toEqual(['payments']);
|
||||
});
|
||||
|
||||
it('does not widen a token beyond what its owner may reach', async () => {
|
||||
const r = new RunAuthorityResolver(rbac([]));
|
||||
const a = await r.resolve({
|
||||
kind: 'caller', userId: 'u1', projects: [SRE], mcpTokenProjectName: 'sre',
|
||||
});
|
||||
expect(a.toolProjects.size).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('agent authority', () => {
|
||||
it('takes every membership without consulting any user', async () => {
|
||||
const svc = rbac([]);
|
||||
const a = await new RunAuthorityResolver(svc).resolve({ kind: 'agent', projects: [SRE, PAY] });
|
||||
|
||||
expect(a.toolProjects.size).toBe(2);
|
||||
expect(a.restrictedProjects).toEqual([]);
|
||||
expect(svc.canAccess).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('service-account authority', () => {
|
||||
it('refuses rather than running on a forgeable identity', async () => {
|
||||
// A ServiceAccount today is an unauthenticated `x-service-account` header.
|
||||
// Honouring it would grant tool access on a client-supplied string, so this
|
||||
// fails loudly until there is a real credential behind it.
|
||||
await expect(
|
||||
new RunAuthorityResolver(rbac(['sre'])).resolve({ kind: 'service-account', projects: [SRE] }),
|
||||
).rejects.toThrow(/not implemented|authenticated identity/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the boundary notice', () => {
|
||||
it('names the projects and says the tools are absent', () => {
|
||||
const n = restrictedContextNotice(['payments', 'sre']);
|
||||
expect(n).toContain('payments, sre');
|
||||
expect(n).toMatch(/NO tools/);
|
||||
expect(n).toMatch(/not reproduce its contents verbatim/i);
|
||||
});
|
||||
|
||||
it('unscoped authority grants nothing', () => {
|
||||
expect(unscopedAuthority('u1').toolProjects.size).toBe(0);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user