diff --git a/bun.lock b/bun.lock index 92687108..634a3944 100644 --- a/bun.lock +++ b/bun.lock @@ -49,7 +49,8 @@ "@hono/node-server": ">=1.19.13", "ajv": ">=8.18.0", "express-rate-limit": ">=8.2.2", - "hono": ">=4.12.14", + "fast-uri": ">=3.1.2", + "hono": ">=4.12.18", "ip-address": ">=10.1.1", "minimatch": ">=9.0.7", "path-to-regexp": ">=8.4.0", @@ -479,7 +480,7 @@ "fast-deep-equal": ["fast-deep-equal@3.1.3", "", {}, "sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q=="], - "fast-uri": ["fast-uri@3.1.0", "", {}, "sha512-iPeeDKJSWf4IEOasVVrknXpaBV0IApz/gp7S2bb7Z4Lljbl2MGJRqInZiUrQwV16cpzw/D3S5j5Julj/gT52AA=="], + "fast-uri": ["fast-uri@3.1.2", "", {}, "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ=="], "fetch-blob": ["fetch-blob@3.2.0", "", { "dependencies": { "node-domexception": "^1.0.0", "web-streams-polyfill": "^3.0.3" } }, "sha512-7yAQpD2UMJzLi1Dqv7qFYnPbaPx7ZfFK6PiIxQ4PfkGPyNyl2Ugx+a/umUonmKqjhM4DnfbMvdX6otXq83soQQ=="], @@ -517,7 +518,7 @@ "hi-base32": ["hi-base32@0.5.1", "", {}, "sha512-EmBBpvdYh/4XxsnUybsPag6VikPYnN30td+vQk+GI3qpahVEG9+gTkG0aXVxTjBqQ5T6ijbWIu77O+C5WFWsnA=="], - "hono": ["hono@4.12.14", "", {}, "sha512-am5zfg3yu6sqn5yjKBNqhnTX7Cv+m00ox+7jbaKkrLMRJ4rAdldd1xPd/JzbBWspqaQv6RSTrgFN95EsfhC+7w=="], + "hono": ["hono@4.12.18", "", {}, "sha512-RWzP96k/yv0PQfyXnWjs6zot20TqfpfsNXhOnev8d1InAxubW93L11/oNUc3tQqn2G0bSdAOBpX+2uDFHV7kdQ=="], "http-errors": ["http-errors@2.0.1", "", { "dependencies": { "depd": "~2.0.0", "inherits": "~2.0.4", "setprototypeof": "~1.2.0", "statuses": "~2.0.2", "toidentifier": "~1.0.1" } }, "sha512-4FbRdAX+bSdmo4AUFuS0WNiPz8NgFt+r8ThgNWmlrjQjt1Q7ZR9+zTlce2859x4KSXrwIsaeTqDoKQmtP8pLmQ=="], diff --git a/package.json b/package.json index 0003f29d..2b2e9156 100644 --- a/package.json +++ b/package.json @@ -103,7 +103,8 @@ "minimatch": ">=9.0.7", "ajv": ">=8.18.0", "qs": ">=6.14.2", - "hono": ">=4.12.14", + "fast-uri": ">=3.1.2", + "hono": ">=4.12.18", "@hono/node-server": ">=1.19.13", "express-rate-limit": ">=8.2.2", "path-to-regexp": ">=8.4.0", diff --git a/server/__tests__/workflow-tool-handler.test.ts b/server/__tests__/workflow-tool-handler.test.ts new file mode 100644 index 00000000..ccc327ab --- /dev/null +++ b/server/__tests__/workflow-tool-handler.test.ts @@ -0,0 +1,153 @@ +import { Database } from 'bun:sqlite'; +import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test'; +import type { WorkflowEdge, WorkflowNode } from '../../shared/types'; +import { createAgent } from '../db/agents'; +import { runMigrations } from '../db/schema'; +import { createWorkflow, createWorkflowRun, updateWorkflow } from '../db/workflows'; +import type { McpToolContext } from '../mcp/tool-handlers/types'; +import { handleManageWorkflow } from '../mcp/tool-handlers/workflow'; + +// ── Helpers ──────────────────────────────────────────────────────────── + +function extractText(result: { content: Array<{ type: string; text?: string }> }): string { + const first = result.content[0]; + return first && 'text' in first ? (first.text ?? '') : ''; +} + +function isError(result: { isError?: boolean }): boolean { + return result.isError === true; +} + +const startNode: WorkflowNode = { id: 'start-1', type: 'start', label: 'Start', config: {} }; +const endNode: WorkflowNode = { id: 'end-1', type: 'end', label: 'End', config: {} }; +const sampleEdges: WorkflowEdge[] = [{ id: 'e1', sourceNodeId: 'start-1', targetNodeId: 'end-1' }]; + +// ── Setup ────────────────────────────────────────────────────────────── + +let db: Database; +let agentAId: string; +let agentBId: string; + +function makeCtx(agentId: string): McpToolContext { + return { + agentId, + db, + agentMessenger: {} as McpToolContext['agentMessenger'], + agentDirectory: {} as McpToolContext['agentDirectory'], + agentWalletService: {} as McpToolContext['agentWalletService'], + workflowService: { + triggerWorkflow: mock(() => Promise.resolve({ id: 'run-1', status: 'running', currentNodeIds: ['start-1'] })), + } as unknown as McpToolContext['workflowService'], + } as McpToolContext; +} + +beforeEach(() => { + db = new Database(':memory:'); + db.exec('PRAGMA foreign_keys = ON'); + runMigrations(db); + agentAId = createAgent(db, { name: 'Agent A', model: 'sonnet' }).id; + agentBId = createAgent(db, { name: 'Agent B', model: 'sonnet' }).id; +}); + +afterEach(() => { + db.close(); +}); + +// ── Cross-agent isolation tests ──────────────────────────────────────── + +describe('handleManageWorkflow — cross-agent isolation', () => { + test("get: agent cannot read another agent's workflow", async () => { + const wf = createWorkflow(db, { agentId: agentAId, name: 'Wf A', nodes: [startNode, endNode], edges: sampleEdges }); + const result = await handleManageWorkflow(makeCtx(agentBId), { action: 'get', workflow_id: wf.id }); + expect(isError(result)).toBe(true); + expect(extractText(result)).toContain('not found'); + }); + + test('get: agent can read its own workflow', async () => { + const wf = createWorkflow(db, { agentId: agentAId, name: 'Wf A', nodes: [startNode, endNode], edges: sampleEdges }); + const result = await handleManageWorkflow(makeCtx(agentAId), { action: 'get', workflow_id: wf.id }); + expect(isError(result)).toBe(false); + expect(extractText(result)).toContain('Wf A'); + }); + + test("activate: agent cannot activate another agent's workflow", async () => { + const wf = createWorkflow(db, { agentId: agentAId, name: 'Wf A', nodes: [startNode, endNode], edges: sampleEdges }); + const result = await handleManageWorkflow(makeCtx(agentBId), { action: 'activate', workflow_id: wf.id }); + expect(isError(result)).toBe(true); + expect(extractText(result)).toContain('not found'); + }); + + test("pause: agent cannot pause another agent's workflow", async () => { + const wf = createWorkflow(db, { agentId: agentAId, name: 'Wf A', nodes: [startNode, endNode], edges: sampleEdges }); + updateWorkflow(db, wf.id, { status: 'active' }); + const result = await handleManageWorkflow(makeCtx(agentBId), { action: 'pause', workflow_id: wf.id }); + expect(isError(result)).toBe(true); + expect(extractText(result)).toContain('not found'); + }); + + test("trigger: agent cannot trigger another agent's workflow", async () => { + const wf = createWorkflow(db, { agentId: agentAId, name: 'Wf A', nodes: [startNode, endNode], edges: sampleEdges }); + updateWorkflow(db, wf.id, { status: 'active' }); + const result = await handleManageWorkflow(makeCtx(agentBId), { action: 'trigger', workflow_id: wf.id }); + expect(isError(result)).toBe(true); + expect(extractText(result)).toContain('not found'); + }); + + test("runs: with workflow_id, agent cannot list runs for another agent's workflow", async () => { + const wf = createWorkflow(db, { agentId: agentAId, name: 'Wf A', nodes: [startNode, endNode], edges: sampleEdges }); + const result = await handleManageWorkflow(makeCtx(agentBId), { action: 'runs', workflow_id: wf.id }); + expect(isError(result)).toBe(true); + expect(extractText(result)).toContain('not found'); + }); + + test('runs: without workflow_id, agent only sees its own runs', async () => { + const wfA = createWorkflow(db, { + agentId: agentAId, + name: 'Wf A', + nodes: [startNode, endNode], + edges: sampleEdges, + }); + const wfB = createWorkflow(db, { + agentId: agentBId, + name: 'Wf B', + nodes: [startNode, endNode], + edges: sampleEdges, + }); + + createWorkflowRun(db, wfA.id, agentAId, {}, { nodes: [startNode, endNode], edges: sampleEdges }); + createWorkflowRun(db, wfB.id, agentBId, {}, { nodes: [startNode, endNode], edges: sampleEdges }); + + const resultA = await handleManageWorkflow(makeCtx(agentAId), { action: 'runs' }); + const textA = extractText(resultA); + expect(isError(resultA)).toBe(false); + expect(textA).toContain(wfA.id.slice(0, 8)); + expect(textA).not.toContain(wfB.id.slice(0, 8)); + }); + + test("run_status: agent cannot view another agent's run", async () => { + const wf = createWorkflow(db, { agentId: agentAId, name: 'Wf A', nodes: [startNode, endNode], edges: sampleEdges }); + const run = createWorkflowRun(db, wf.id, agentAId, {}, { nodes: [startNode, endNode], edges: sampleEdges }); + const result = await handleManageWorkflow(makeCtx(agentBId), { action: 'run_status', run_id: run.id }); + expect(isError(result)).toBe(true); + expect(extractText(result)).toContain('not found'); + }); + + test('run_status: agent can view its own run', async () => { + const wf = createWorkflow(db, { agentId: agentAId, name: 'Wf A', nodes: [startNode, endNode], edges: sampleEdges }); + const run = createWorkflowRun(db, wf.id, agentAId, {}, { nodes: [startNode, endNode], edges: sampleEdges }); + const result = await handleManageWorkflow(makeCtx(agentAId), { action: 'run_status', run_id: run.id }); + expect(isError(result)).toBe(false); + expect(extractText(result)).toContain('Workflow Run:'); + }); + + test('list: agent only sees its own workflows', async () => { + createWorkflow(db, { agentId: agentAId, name: 'Wf A', nodes: [startNode, endNode], edges: sampleEdges }); + createWorkflow(db, { agentId: agentBId, name: 'Wf B', nodes: [startNode, endNode], edges: sampleEdges }); + + const result = await handleManageWorkflow(makeCtx(agentAId), { action: 'list' }); + const text = extractText(result); + expect(isError(result)).toBe(false); + expect(text).toContain('Wf A'); + expect(text).not.toContain('Wf B'); + }); +}); diff --git a/server/db/workflows.ts b/server/db/workflows.ts index bab995d9..76f565b9 100644 --- a/server/db/workflows.ts +++ b/server/db/workflows.ts @@ -208,6 +208,7 @@ export function listWorkflowRuns( workflowId?: string, limit = 50, tenantId: string = DEFAULT_TENANT_ID, + agentId?: string, ): WorkflowRun[] { if (workflowId) { const { query, bindings } = withTenantFilter( @@ -216,6 +217,13 @@ export function listWorkflowRuns( ); return (db.query(query).all(workflowId, ...bindings, limit) as Record[]).map(rowToRun); } + if (agentId) { + const { query, bindings } = withTenantFilter( + 'SELECT * FROM workflow_runs WHERE agent_id = ? ORDER BY started_at DESC LIMIT ?', + tenantId, + ); + return (db.query(query).all(agentId, ...bindings, limit) as Record[]).map(rowToRun); + } const { query, bindings } = withTenantFilter( 'SELECT * FROM workflow_runs ORDER BY started_at DESC LIMIT ?', tenantId, diff --git a/server/mcp/tool-handlers/workflow.ts b/server/mcp/tool-handlers/workflow.ts index 5b833d29..ac0396b6 100644 --- a/server/mcp/tool-handlers/workflow.ts +++ b/server/mcp/tool-handlers/workflow.ts @@ -94,7 +94,7 @@ export async function handleManageWorkflow( case 'get': { if (!args.workflow_id) return errorResult('workflow_id is required'); const workflow = getWorkflow(ctx.db, args.workflow_id); - if (!workflow) return errorResult('Workflow not found'); + if (!workflow || workflow.agentId !== ctx.agentId) return errorResult('Workflow not found'); const nodeList = workflow.nodes.map((n) => ` - ${n.id}: ${n.type} "${n.label}"`).join('\n'); const edgeList = workflow.edges @@ -112,6 +112,8 @@ export async function handleManageWorkflow( case 'activate': { if (!args.workflow_id) return errorResult('workflow_id is required'); + const existing = getWorkflow(ctx.db, args.workflow_id); + if (!existing || existing.agentId !== ctx.agentId) return errorResult('Workflow not found'); const updated = updateWorkflow(ctx.db, args.workflow_id, { status: 'active' }); if (!updated) return errorResult('Workflow not found'); return textResult(`Workflow "${updated.name}" activated. It can now be triggered.`); @@ -119,6 +121,8 @@ export async function handleManageWorkflow( case 'pause': { if (!args.workflow_id) return errorResult('workflow_id is required'); + const existing = getWorkflow(ctx.db, args.workflow_id); + if (!existing || existing.agentId !== ctx.agentId) return errorResult('Workflow not found'); const updated = updateWorkflow(ctx.db, args.workflow_id, { status: 'paused' }); if (!updated) return errorResult('Workflow not found'); return textResult(`Workflow "${updated.name}" paused.`); @@ -127,6 +131,8 @@ export async function handleManageWorkflow( case 'trigger': { if (!args.workflow_id) return errorResult('workflow_id is required'); if (!ctx.workflowService) return errorResult('Workflow service not available'); + const existing = getWorkflow(ctx.db, args.workflow_id); + if (!existing || existing.agentId !== ctx.agentId) return errorResult('Workflow not found'); const run = await ctx.workflowService.triggerWorkflow(args.workflow_id, args.input ?? {}); return textResult( @@ -138,7 +144,17 @@ export async function handleManageWorkflow( } case 'runs': { - const runs = listWorkflowRuns(ctx.db, args.workflow_id, 20); + if (args.workflow_id) { + const workflow = getWorkflow(ctx.db, args.workflow_id); + if (!workflow || workflow.agentId !== ctx.agentId) return errorResult('Workflow not found'); + } + const runs = listWorkflowRuns( + ctx.db, + args.workflow_id, + 20, + undefined, + args.workflow_id ? undefined : ctx.agentId, + ); if (runs.length === 0) return textResult('No workflow runs found.'); const lines = runs.map( (r) => @@ -150,7 +166,7 @@ export async function handleManageWorkflow( case 'run_status': { if (!args.run_id) return errorResult('run_id is required'); const run = getWorkflowRun(ctx.db, args.run_id); - if (!run) return errorResult('Run not found'); + if (!run || run.agentId !== ctx.agentId) return errorResult('Run not found'); const nodeLines = run.nodeRuns.map( (nr) => diff --git a/specs/mcp/tools/tool-handlers.spec.md b/specs/mcp/tools/tool-handlers.spec.md index f9240555..ad729768 100644 --- a/specs/mcp/tools/tool-handlers.spec.md +++ b/specs/mcp/tools/tool-handlers.spec.md @@ -181,8 +181,9 @@ Implements every `corvid_*` MCP tool handler. Each exported function takes an `M 6. **All handlers return `CallToolResult`**: Every handler returns `{ content: [{ type: 'text', text }] }` via `textResult()` or `errorResult()` helpers 7. **Service availability checks**: Handlers check for optional services (e.g. `ctx.workTaskService`, `ctx.schedulerService`) before use and return error results if missing 8. **Status emission**: Long-running handlers call `ctx.emitStatus?.()` to provide progress updates to the UI -9. **Agent identity signature**: GitHub write operations (`handleGitHubCreatePr`, `handleGitHubCreateIssue`, `handleGitHubCommentOnPr`, `handleGitHubReviewPr`) append an agent identity footer to the body via `buildAgentSignature()`. If the agent cannot be resolved, no signature is appended (fail-open). PR and issue creation tools accept an optional `requested_by` parameter to attribute human collaborators — resolved via the contacts DB and displayed as `Requested by: @username` in the footer -10. **Issue body template**: `handleGitHubCreateIssue` wraps plain-text bodies in a structured template via `formatIssueBody()` (from `github-issue-body.ts`). Bug issues get Description/Steps to Reproduce/Expected Behavior/Actual Behavior sections; feature requests get Description/Motivation/Proposed Solution sections. Bodies already containing markdown `##` headers are passed through unchanged (backward compatible). An explicit `type` parameter overrides auto-detection; `type="unknown"` skips wrapping entirely +9. **Workflow agent isolation**: `handleManageWorkflow` enforces per-agent ownership for `get`, `activate`, `pause`, `trigger`, `runs`, and `run_status` actions. After fetching a workflow or run, the handler verifies `agentId` matches `ctx.agentId` and returns a "not found" error if it doesn't, preventing cross-agent access. The `runs` action without a `workflow_id` filters by `ctx.agentId` so agents only see their own runs. +10. **Agent identity signature**: GitHub write operations (`handleGitHubCreatePr`, `handleGitHubCreateIssue`, `handleGitHubCommentOnPr`, `handleGitHubReviewPr`) append an agent identity footer to the body via `buildAgentSignature()`. If the agent cannot be resolved, no signature is appended (fail-open). PR and issue creation tools accept an optional `requested_by` parameter to attribute human collaborators — resolved via the contacts DB and displayed as `Requested by: @username` in the footer +11. **Issue body template**: `handleGitHubCreateIssue` wraps plain-text bodies in a structured template via `formatIssueBody()` (from `github-issue-body.ts`). Bug issues get Description/Steps to Reproduce/Expected Behavior/Actual Behavior sections; feature requests get Description/Motivation/Proposed Solution sections. Bodies already containing markdown `##` headers are passed through unchanged (backward compatible). An explicit `type` parameter overrides auto-detection; `type="unknown"` skips wrapping entirely ## Behavioral Examples