diff --git a/.taskmaster/tasks/tasks.json b/.taskmaster/tasks/tasks.json index 1e48a80..d525bf0 100644 --- a/.taskmaster/tasks/tasks.json +++ b/.taskmaster/tasks/tasks.json @@ -3454,19 +3454,47 @@ "dependencies": [ "166" ], + "status": "done", + "subtasks": [], + "updatedAt": "2026-08-15T22:54:16.432Z" + }, + { + "id": "178", + "title": "Agent authority: run an agent unattended on a schedule", + "description": "Mode 2. The agent acts under its OWN authority - every project it is a member of - with no human in the loop.", + "details": "The authority resolver already implements this (kind: agent returns all memberships without consulting any user, unit-tested). What is missing is anything that can INVOKE it.\n\nBlockers, all verified:\n- ChatService.chat has exactly two call sites, both HTTP handlers. No cron, no job runner, no queue that can re-enter prepareContext (InferenceTask carries raw completion bodies: no agent, no project, no tools).\n- AgentSession is schema-only: no repository, no service, no route, no credential column. Its own comment promises a GC sweep that does not exist.\n- The mcpctl toolset fails closed without a caller token by design, so a scheduled run of a system agent would throw.\n\nNeeds a scheduler, a run record, and a decision on what a scheduled run may do that a caller-authority run may not.", + "testStrategy": "See details.", + "priority": "medium", + "dependencies": [ + "166" + ], "status": "pending", "subtasks": [], - "updatedAt": "2026-08-09T22:34:45.085Z" + "updatedAt": "2026-08-15T22:54:36.185Z" + }, + { + "id": "179", + "title": "Service-account authority: an authenticated non-human identity", + "description": "Mode 3. An agent runs under an explicit service account whose permission set may be smaller OR larger than any human's.", + "details": "The resolver refuses this with a 501 rather than honouring it, deliberately. Two things must exist first, both verified absent:\n\n1. A ServiceAccount is currently a forgeable header. main.ts reads x-service-account with no lookup, signature or credential; there is no ServiceAccount table. tests/security.test.ts already documents this as an escalation hole.\n2. Smaller-than-a-human is not expressible at all. middleware/auth.ts sets request.userId = ownerId unconditionally, so getPermissions always unions the owner's grants in. An McpToken with --rbac=empty is the owner at full privilege; docs/mcptoken-implementation.md claims the opposite and is wrong. A reduced principal needs that inheritance broken, not a restriction layered on top.\n\nAlso fix while there: middleware/rbac.ts route-level guards check the user only, never the SA or token, so enforcement differs by which path a route uses.", + "testStrategy": "See details.", + "priority": "medium", + "dependencies": [ + "177" + ], + "status": "pending", + "subtasks": [], + "updatedAt": "2026-08-15T22:54:36.185Z" } ], "metadata": { "version": "1.0.0", - "lastModified": "2026-08-15T22:10:49.722Z", + "lastModified": "2026-08-15T22:54:16.433Z", "taskCount": 177, - "completedCount": 137, + "completedCount": 138, "tags": [ "master" ] } } -} \ No newline at end of file +} diff --git a/src/mcpd/src/main.ts b/src/mcpd/src/main.ts index 9e948be..3e7b470 100644 --- a/src/mcpd/src/main.ts +++ b/src/mcpd/src/main.ts @@ -136,6 +136,7 @@ import { GitTaskMerger } from './services/task-merger.js'; import { GitProviderService } from './services/git-provider.service.js'; import { GitProviderRegistry } from './services/git/registry.js'; import { McpctlToolProvider } from './services/mcpctl-tools.js'; +import { RunAuthorityResolver } from './services/run-authority.js'; import { SkillService } from './services/skill.service.js'; import { GitBackupService } from './services/backup/git-backup.service.js'; import type { BackupKind } from './services/backup/yaml-serializer.js'; @@ -591,6 +592,9 @@ async function main(): Promise { // Loopback to this same mcpd, so the mcpctl system agent's calls go through // the normal auth + RBAC path carrying the caller's own token. new McpctlToolProvider(`http://127.0.0.1:${String(config.port)}`), + // Decides which projects' TOOLS a run may call. Prompts are never gated by + // it — knowledge travels with the agent, action does not. + new RunAuthorityResolver(rbacService), ); registerAgentChatRoutes(app, chatService); registerProjectChatRoutes(app, chatService, rbacService); diff --git a/src/mcpd/src/routes/agent-chat.ts b/src/mcpd/src/routes/agent-chat.ts index 39cdbb3..fc4f861 100644 --- a/src/mcpd/src/routes/agent-chat.ts +++ b/src/mcpd/src/routes/agent-chat.ts @@ -48,10 +48,18 @@ export function registerAgentChatRoutes( ? authHeader.slice(7) : undefined; + // A token bound to one project must not drive an agent into another's + // tools. `project` arrives in the BODY, so the URL-based token scope + // check in main.ts never sees it — the ceiling has to travel with the call. + const mcpTokenProjectName = request.mcpToken?.projectName; + const args = { agentName: request.params.name, ownerId, ...(callerToken !== undefined ? { callerToken } : {}), + ...(mcpTokenProjectName !== undefined && mcpTokenProjectName !== null + ? { mcpTokenProjectName } + : {}), ...(threadId !== undefined ? { threadId } : {}), ...(message !== undefined ? { userMessage: message } : {}), ...(messagesOverride !== undefined diff --git a/src/mcpd/src/services/chat-tool-dispatcher.ts b/src/mcpd/src/services/chat-tool-dispatcher.ts index 2602998..d6716b3 100644 --- a/src/mcpd/src/services/chat-tool-dispatcher.ts +++ b/src/mcpd/src/services/chat-tool-dispatcher.ts @@ -15,6 +15,7 @@ */ import type { McpProxyService } from './mcp-proxy-service.js'; import type { IProjectRepository } from '../repositories/project.repository.js'; +import type { RunAuthority } from './run-authority.js'; import type { ChatTool, ChatToolDispatcher } from './chat.service.js'; import { TOOL_NAME_SEPARATOR } from './chat.service.js'; @@ -38,8 +39,11 @@ export interface ChatToolDispatcherDeps { export class ChatToolDispatcherImpl implements ChatToolDispatcher { constructor(private readonly deps: ChatToolDispatcherDeps) {} - async listTools(projectId: string | null): Promise { + async listTools(projectId: string | null, authority?: RunAuthority): Promise { if (projectId === null) return []; + // Not authorised for this project's tools: hand back nothing. `callTool` + // enforces the same rule independently — see the note there. + if (authority !== undefined && !authority.toolProjects.has(projectId)) return []; const project = await this.deps.projects.findById(projectId); if (project === null) return []; const out: ChatTool[] = []; @@ -77,7 +81,17 @@ export class ChatToolDispatcherImpl implements ChatToolDispatcher { serverName: string; toolName: string; args: Record; + authority?: RunAuthority; }): Promise { + // Checked here as well as in listTools, deliberately. Withholding a tool + // from the list is presentation, not enforcement — a model can invent a + // tool name it was never shown, and this is the call that would run it. + if (args.authority !== undefined && !args.authority.toolProjects.has(args.projectId)) { + throw new Error( + `Not authorised to use tools in this project. The agent has knowledge of it, but acting there ` + + `needs run:projects on it.`, + ); + } const project = await this.deps.projects.findById(args.projectId); if (project === null) { throw new Error(`Project ${args.projectId} not found`); diff --git a/src/mcpd/src/services/chat.service.ts b/src/mcpd/src/services/chat.service.ts index 45a7e4d..3251c4f 100644 --- a/src/mcpd/src/services/chat.service.ts +++ b/src/mcpd/src/services/chat.service.ts @@ -38,6 +38,7 @@ import type { RbacService } from './rbac.service.js'; import type { OpenAiChatRequest, OpenAiMessage } from './llm/types.js'; import type { AgentChatParams } from '../validation/agent.schema.js'; import { NotFoundError } from './mcp-server.service.js'; +import { restrictedContextNotice, unscopedAuthority, type RunAuthority, type RunAuthorityResolver } from './run-authority.js'; import type { McpctlToolProvider } from './mcpctl-tools.js'; /** Minimal secret-read surface the project-chat `get_secret` tool needs. */ @@ -82,13 +83,14 @@ export interface ChatTool { export interface ChatToolDispatcher { /** List tools available to an agent's project. Empty if no project. */ - listTools(projectId: string | null): Promise; + listTools(projectId: string | null, authority?: RunAuthority): Promise; /** Execute a tool call. Throws on error. */ callTool(args: { projectId: string; serverName: string; toolName: string; args: Record; + authority?: RunAuthority; }): Promise; } @@ -149,6 +151,14 @@ export interface ChatRequestArgs { * AS the caller — see services/mcpctl-tools.ts. Absent for every other agent. */ callerToken?: string; + /** + * When the request arrived on an McpToken bound to a project, that project's + * name. It is a hard ceiling on which projects' tools this run may use: + * `project` travels in the request BODY, so the URL-based token scope check + * in main.ts cannot see it, and a token for project A could otherwise drive + * an agent into project B's tools. + */ + mcpTokenProjectName?: string; threadId?: string; userMessage?: string; /** Optional full-history override; if set, threadId history is ignored. */ @@ -211,6 +221,8 @@ interface PreparedChatContext { allowSecrets: boolean; /** 'project' (MCP servers) or 'mcpctl' (operate mcpctl as the caller). */ toolset: string; + /** Whose authority this run acts under; decides which projects' tools it may call. */ + authority: RunAuthority; /** Caller's bearer token, forwarded only for the 'mcpctl' toolset. */ callerToken?: string; } @@ -241,6 +253,8 @@ export class ChatService { * compile; without it a system agent simply gets no tools. */ private readonly mcpctlTools?: McpctlToolProvider, + /** Decides which projects' tools a run may call. Absent → knowledge only. */ + private readonly authorities?: RunAuthorityResolver, ) {} async createThread(agentName: string, ownerId: string, title?: string): Promise<{ id: string }> { @@ -321,7 +335,7 @@ export class ChatService { tool_calls: choice.tool_calls, }); for (const call of choice.tool_calls) { - const toolResult = await this.dispatchTool(call.function.name, call.function.arguments, ctx.projectId, ctx.allowSecrets, { toolset: ctx.toolset, ...(ctx.callerToken !== undefined ? { callerToken: ctx.callerToken } : {}) }); + const toolResult = await this.dispatchTool(call.function.name, call.function.arguments, ctx.projectId, ctx.allowSecrets, { toolset: ctx.toolset, authority: ctx.authority, ...(ctx.callerToken !== undefined ? { callerToken: ctx.callerToken } : {}) }); const resultMsg = await this.chatRepo.appendMessage({ threadId: ctx.threadId, role: 'tool', @@ -449,7 +463,7 @@ export class ChatService { for (const call of accumulated.toolCalls) { yield { type: 'tool_call', toolName: call.name, args: safeParseJson(call.argumentsJson) as Record }; try { - const result = await this.dispatchTool(call.name, call.argumentsJson, ctx.projectId, ctx.allowSecrets, { toolset: ctx.toolset, ...(ctx.callerToken !== undefined ? { callerToken: ctx.callerToken } : {}) }); + const result = await this.dispatchTool(call.name, call.argumentsJson, ctx.projectId, ctx.allowSecrets, { toolset: ctx.toolset, authority: ctx.authority, ...(ctx.callerToken !== undefined ? { callerToken: ctx.callerToken } : {}) }); const resultStr = typeof result === 'string' ? result : JSON.stringify(result); await this.chatRepo.appendMessage({ threadId: ctx.threadId, @@ -777,22 +791,43 @@ export class ChatService { history.push({ role: 'user', content: args.userMessage }); } + // Whose authority this run acts under. Knowledge (the prompts assembled + // above) is NOT gated by it — a QA reviewer that forgets the project's + // conventions when a different person asks is not a reviewer. Acting is. + const authority = this.authorities !== undefined && projectId !== null && membership !== null + ? await this.authorities.resolve({ + kind: 'caller', + projects: [{ id: projectId, name: membership.projectName }], + ...(args.ownerId !== undefined ? { userId: args.ownerId } : {}), + ...(args.mcpTokenProjectName !== undefined + ? { mcpTokenProjectName: args.mcpTokenProjectName } + : {}), + }) + : unscopedAuthority(args.ownerId); + // A system agent operating mcpctl gets ONLY the mcpctl tools: no project // MCP servers, and no project prompts in the system block above. Managing // mcpctl is the whole job, and an unrelated project's context would just // be noise it might act on. const toolList = agent.toolset === 'mcpctl' && this.mcpctlTools !== undefined ? this.mcpctlTools.listTools() - : await this.tools.listTools(projectId); + : await this.tools.listTools(projectId, authority); const allowed = mergedParams.tools_allowlist; const filteredTools = allowed === undefined ? toolList : toolList.filter((t) => allowed.includes(t.name)); + // Tell the model when it is carrying context the caller cannot reach on + // their own. This is guidance to a persona, not an enforced boundary — the + // enforcement is that the tools simply are not there. + const finalSystemBlock = authority.restrictedProjects.length > 0 + ? `${systemBlock}\n\n${restrictedContextNotice(authority.restrictedProjects)}` + : systemBlock; + return { threadId, history, - systemBlock, + systemBlock: finalSystemBlock, poolCandidates, primaryLlm: pinned.name, mergedParams, @@ -802,6 +837,7 @@ export class ChatService { maxIterations: resolveMaxIterations(agent.extras), allowSecrets: false, toolset: agent.toolset, + authority, ...(args.callerToken !== undefined ? { callerToken: args.callerToken } : {}), }; } @@ -918,6 +954,14 @@ export class ChatService { // Project chat never gets the mcpctl toolset — that surface belongs to // the system agent alone. toolset: 'project', + // The route gate for this path is already `run:projects:`, so the + // caller is authorised for exactly this project and nothing else. + authority: { + kind: 'caller' as const, + userId: args.ownerId, + toolProjects: new Set([project.id]), + restrictedProjects: [], + }, }; } @@ -1042,6 +1086,13 @@ export class ChatService { if (args.threadId !== undefined) { const existing = await this.chatRepo.findThread(args.threadId); if (existing === null) throw new NotFoundError(`Thread not found: ${args.threadId}`); + // Ownership check, mirroring resolveProjectThreadId. Without it anyone + // holding `run:agents:` could append to another user's thread by + // guessing its id — and read the history back in the reply. 404 rather + // than 403 so an id cannot be probed for existence. + if (existing.ownerId !== args.ownerId) { + throw new NotFoundError(`Thread not found: ${args.threadId}`); + } return existing.id; } const created = await this.chatRepo.createThread({ agentId, ownerId: args.ownerId }); @@ -1113,7 +1164,7 @@ export class ChatService { argsJson: string, projectId: string | null, allowSecrets: boolean, - ctx?: { toolset: string; callerToken?: string }, + ctx?: { toolset: string; callerToken?: string; authority?: RunAuthority }, ): Promise { // mcpctl system tools are checked FIRST: they need no project, and they // execute as the caller rather than through an MCP server. @@ -1147,7 +1198,13 @@ export class ChatService { const serverName = toolWireName.slice(0, sep); const toolName = toolWireName.slice(sep + TOOL_NAME_SEPARATOR.length); const parsed = safeParseJson(argsJson) as Record; - return this.tools.callTool({ projectId, serverName, toolName, args: parsed }); + return this.tools.callTool({ + projectId, + serverName, + toolName, + args: parsed, + ...(ctx?.authority !== undefined ? { authority: ctx.authority } : {}), + }); } } diff --git a/src/mcpd/src/services/run-authority.ts b/src/mcpd/src/services/run-authority.ts new file mode 100644 index 0000000..b3968d3 --- /dev/null +++ b/src/mcpd/src/services/run-authority.ts @@ -0,0 +1,144 @@ +import type { RbacService } from './rbac.service.js'; + +/** + * Whose authority a chat turn runs under. + * + * An agent carries a project's prompts AND its MCP tools. Those two are not the + * same kind of thing, and this splits them: + * + * - **Knowledge travels with the agent.** Prompts and skills load regardless of + * who is asking — that is what makes an agent a QA reviewer rather than a + * generic bot. A reviewer that forgets the project's conventions when a + * different person asks is not a reviewer. + * - **Action is bounded by the authority.** Every MCP tool call is gated. The + * agent's membership is no longer, by itself, a capability. + * + * Authority 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. + */ +export type AuthorityKind = 'caller' | 'agent' | 'service-account'; + +export interface RunAuthority { + kind: AuthorityKind; + /** The human this run acts as. Present only for `caller`. */ + userId?: string; + /** Project ids whose TOOLS this run may call. Prompt loading ignores this. */ + toolProjects: Set; + /** + * Names of projects contributing context the caller could not reach on their + * own. Surfaced to the model so a persona can apply the knowledge without + * reciting it — guidance, not an enforced boundary, and described as such. + */ + restrictedProjects: string[]; +} + +/** One project the agent belongs to. */ +export interface AuthorityProject { + id: string; + name: string; +} + +export interface ResolveAuthorityInput { + kind: AuthorityKind; + /** Projects the agent is a member of (plus any legacy pinned project). */ + projects: AuthorityProject[]; + /** Required for `caller`. */ + userId?: string; + /** + * When the request arrived on an McpToken bound to a project, that project's + * name. The token's binding is a ceiling: a token for project A must not + * drive an agent into project B's tools, which the URL-based scope check in + * main.ts cannot see because `project` travels in the request body. + */ + mcpTokenProjectName?: string; +} + +export class RunAuthorityResolver { + constructor(private readonly rbac?: RbacService) {} + + async resolve(input: ResolveAuthorityInput): Promise { + switch (input.kind) { + case 'agent': + // The agent's own memberships ARE the grant. No human is consulted + // because there is no human — this is the scheduled-work authority. + return { + kind: 'agent', + toolProjects: new Set(input.projects.map((p) => p.id)), + restrictedProjects: [], + }; + + case 'service-account': + // Deliberately unreachable today. A ServiceAccount is currently a + // forgeable `x-service-account` header with no credential behind it, so + // honouring it here would grant tool access on an unauthenticated + // string. Refuse loudly rather than quietly degrading to something + // weaker or stronger than asked for. + throw Object.assign( + new Error( + 'service-account authority is not implemented: a ServiceAccount has no authenticated ' + + 'identity yet, so running under one would trust a client-supplied header.', + ), + { statusCode: 501 }, + ); + + case 'caller': + default: + return this.resolveForCaller(input); + } + } + + private async resolveForCaller(input: ResolveAuthorityInput): Promise { + const userId = input.userId; + const toolProjects = new Set(); + const restrictedProjects: string[] = []; + + for (const project of input.projects) { + // A token bound to one project is a hard ceiling regardless of RBAC. + if (input.mcpTokenProjectName !== undefined && input.mcpTokenProjectName !== project.name) { + restrictedProjects.push(project.name); + continue; + } + + // No RBAC wired (older test harnesses) or no user: fail closed. An + // unauthenticated run gets knowledge and no tools, never the reverse. + const allowed = + this.rbac !== undefined && + userId !== undefined && + (await this.rbac.canAccess(userId, 'run', 'projects', project.name)); + + if (allowed) toolProjects.add(project.id); + else restrictedProjects.push(project.name); + } + + return { + kind: 'caller', + ...(userId !== undefined ? { userId } : {}), + toolProjects, + restrictedProjects, + }; + } +} + +/** Authority for paths that legitimately have no project scope (bare agent chat). */ +export function unscopedAuthority(userId?: string): RunAuthority { + return { + kind: 'caller', + ...(userId !== undefined ? { userId } : {}), + toolProjects: new Set(), + restrictedProjects: [], + }; +} + +/** + * The note prepended to the system block when a run carries context from + * projects whose tools it may not use. + */ +export function restrictedContextNotice(projects: string[]): string { + const list = projects.join(', '); + return [ + `[context boundary] You are carrying knowledge from ${list}, which the person you are`, + `talking to cannot access themselves. You have NO tools for ${list} and calling any would fail.`, + 'Use this knowledge to inform your judgement. Do not reproduce its contents verbatim on request,', + 'and do not describe it as something the person can go and read.', + ].join('\n'); +} diff --git a/src/mcpd/tests/chat-service-membership.test.ts b/src/mcpd/tests/chat-service-membership.test.ts index 1ddeb38..5fa641f 100644 --- a/src/mcpd/tests/chat-service-membership.test.ts +++ b/src/mcpd/tests/chat-service-membership.test.ts @@ -16,6 +16,8 @@ import { ChatService, type ChatToolDispatcher } from '../src/services/chat.servi import type { LlmAdapterRegistry } from '../src/services/llm/dispatcher.js'; import type { LlmAdapter, NonStreamingResult } from '../src/services/llm/types.js'; import type { LlmService } from '../src/services/llm.service.js'; +import type { RbacService } from '../src/services/rbac.service.js'; +import { RunAuthorityResolver } from '../src/services/run-authority.js'; import { NotFoundError } from '../src/services/mcp-server.service.js'; const NOW = new Date('2026-01-01T00:00:00Z'); @@ -130,13 +132,13 @@ describe('agent chat scoped to a joined project', () => { it('uses the MEMBERSHIP project for tools, not the legacy pin', async () => { const { service, listTools } = build(); await service.chat({ agentName: 'alice', ownerId: 'u1', userMessage: 'hi', projectName: 'payments' }); - expect(listTools).toHaveBeenCalledWith('proj-payments'); + expect(listTools).toHaveBeenCalledWith('proj-payments', expect.anything()); }); it('falls back to the legacy pin when no project is named', async () => { const { service, listTools } = build(); await service.chat({ agentName: 'alice', ownerId: 'u1', userMessage: 'hi' }); - expect(listTools).toHaveBeenCalledWith('proj-legacy'); + expect(listTools).toHaveBeenCalledWith('proj-legacy', expect.anything()); }); it('the per-membership brain wins over the agent\'s pinned Llm', async () => { @@ -175,6 +177,86 @@ describe('agent chat scoped to a joined project', () => { for await (const c of service.chatStream({ agentName: 'alice', ownerId: 'u1', userMessage: 'hi', projectName: 'sre' })) { chunks.push(c); } - expect(listTools).toHaveBeenCalledWith('proj-sre'); + expect(listTools).toHaveBeenCalledWith('proj-sre', expect.anything()); + }); +}); + +describe('tool authority — knowledge travels, action is bounded', () => { + /** Same wiring as build(), plus an authority resolver with a real RBAC stub. */ + function buildWithAuthority(canReach: string[]) { + const listTools = vi.fn(async () => []); + const tools = { listTools, callTool: vi.fn(async () => ({})) } as unknown as ChatToolDispatcher; + const adapter: LlmAdapter = { + kind: 'scripted', + infer: vi.fn(async () => reply('ok')), + stream: async function*() { yield { data: '[DONE]', done: true }; }, + } as unknown as LlmAdapter; + const rbac = { + canAccess: vi.fn(async (_u: string, action: string, resource: string, name?: string) => + action === 'run' && resource === 'projects' && name !== undefined && canReach.includes(name), + ), + } as unknown as RbacService; + + const service = new ChatService( + mockAgents(), mockLlms(), { get: () => adapter } as unknown as LlmAdapterRegistry, + mockChatRepo(), mockPromptRepo(), tools, + undefined, undefined, undefined, undefined, undefined, undefined, + new RunAuthorityResolver(rbac), + ); + + const systemSent = (): string => { + const body = (adapter.infer as ReturnType).mock.calls[0]?.[0].body as + { messages: Array<{ role: string; content: string }> }; + return body.messages.find((m) => m.role === 'system')?.content ?? ''; + }; + return { service, listTools, systemSent }; + } + + it('withholds the tools when the caller cannot reach the project', async () => { + // The motivating hole: run:agents: was the whole check, so talking to + // an agent inherited tool access to every project it had joined. + const { service, listTools } = buildWithAuthority([]); + await service.chat({ agentName: 'alice', ownerId: 'u1', userMessage: 'hi', projectName: 'payments' }); + + expect(listTools).toHaveBeenCalledWith( + 'proj-payments', + expect.objectContaining({ toolProjects: new Set() }), + ); + }); + + it('tells the model it is carrying context the caller cannot reach', async () => { + const { service, systemSent } = buildWithAuthority([]); + await service.chat({ agentName: 'alice', ownerId: 'u1', userMessage: 'hi', projectName: 'payments' }); + + const system = systemSent(); + expect(system).toContain('[context boundary]'); + expect(system).toContain('payments'); + expect(system).toMatch(/NO tools/); + }); + + it('grants the tools when the caller can reach the project', async () => { + const { service, listTools, systemSent } = buildWithAuthority(['payments']); + await service.chat({ agentName: 'alice', ownerId: 'u1', userMessage: 'hi', projectName: 'payments' }); + + expect(listTools).toHaveBeenCalledWith( + 'proj-payments', + expect.objectContaining({ toolProjects: new Set(['proj-payments']) }), + ); + expect(systemSent()).not.toContain('[context boundary]'); + }); + + it('honours an McpToken binding as a ceiling over RBAC', async () => { + // `project` arrives in the body, so main.ts's URL-based token scope check + // cannot see it. + const { service, listTools } = buildWithAuthority(['payments']); + await service.chat({ + agentName: 'alice', ownerId: 'u1', userMessage: 'hi', + projectName: 'payments', mcpTokenProjectName: 'sre', + }); + + expect(listTools).toHaveBeenCalledWith( + 'proj-payments', + expect.objectContaining({ toolProjects: new Set() }), + ); }); }); diff --git a/src/mcpd/tests/chat-service-project.test.ts b/src/mcpd/tests/chat-service-project.test.ts index 886a0e5..6bb21ec 100644 --- a/src/mcpd/tests/chat-service-project.test.ts +++ b/src/mcpd/tests/chat-service-project.test.ts @@ -168,7 +168,7 @@ describe('ChatService — project chat', () => { adapter: scriptedAdapter([toolCall('srv__do', { x: 1 }), text('done')]), }); await service.chatProject({ projectName: 'sre', userMessage: 'go', ownerId: 'owner-1' }); - expect(callTool).toHaveBeenCalledWith({ projectId: 'proj-1', serverName: 'srv', toolName: 'do', args: { x: 1 } }); + expect(callTool).toHaveBeenCalledWith(expect.objectContaining({ projectId: 'proj-1', serverName: 'srv', toolName: 'do', args: { x: 1 } })); }); it('does NOT offer get_secret without --allow-secrets', async () => { diff --git a/src/mcpd/tests/chat-service.test.ts b/src/mcpd/tests/chat-service.test.ts index ef5f9aa..9d02bf4 100644 --- a/src/mcpd/tests/chat-service.test.ts +++ b/src/mcpd/tests/chat-service.test.ts @@ -258,12 +258,12 @@ describe('ChatService', () => { }); expect(result.assistant).toBe('the answer is 42'); - expect(tools.callTool).toHaveBeenCalledWith({ + expect(tools.callTool).toHaveBeenCalledWith(expect.objectContaining({ projectId: 'proj-1', serverName: 'grafana', toolName: 'query', args: { q: 'cpu' }, - }); + })); const stored = chatRepo._msgs.filter((m) => m.threadId === result.threadId); expect(stored.map((m) => m.role)).toEqual(['user', 'assistant', 'tool', 'assistant']); // No `pending` rows leaked. diff --git a/src/mcpd/tests/services/run-authority.test.ts b/src/mcpd/tests/services/run-authority.test.ts new file mode 100644 index 0000000..0074c4c --- /dev/null +++ b/src/mcpd/tests/services/run-authority.test.ts @@ -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:` — 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:` 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); + }); +});