From 1e09f7e59f6ce2aa3ad23b289a5ed653ebe2326f Mon Sep 17 00:00:00 2001 From: niko4244 <112509534+niko4244@users.noreply.github.com> Date: Fri, 8 May 2026 15:01:59 -0600 Subject: [PATCH] Harden workspace-scoped route access --- src/app/api/agents/[id]/heartbeat/route.ts | 22 +- src/app/api/agents/[id]/memory/route.ts | 19 +- src/app/api/tasks/[id]/comments/route.ts | 23 +- src/app/api/tasks/[id]/route.ts | 30 ++- src/app/api/tasks/queue/route.ts | 10 +- src/app/api/tasks/route.ts | 30 ++- src/app/api/webhooks/route.ts | 17 +- .../agent-memory-route-security.test.ts | 174 ++++++++++++++ .../heartbeat-route-security.test.ts | 94 ++++++++ .../task-comments-route-security.test.ts | 87 +++++++ .../task-queue-route-security.test.ts | 53 +++++ src/lib/__tests__/task-route-security.test.ts | 93 ++++++++ .../__tests__/webhooks-route-security.test.ts | 84 +++++++ src/lib/__tests__/workspace-scope.test.ts | 221 ++++++++++++++++++ src/lib/auth.ts | 3 + src/lib/enforcement/workspace-scope.ts | 116 +++++++++ src/lib/webhooks.ts | 28 ++- 17 files changed, 1066 insertions(+), 38 deletions(-) create mode 100644 src/lib/__tests__/agent-memory-route-security.test.ts create mode 100644 src/lib/__tests__/heartbeat-route-security.test.ts create mode 100644 src/lib/__tests__/task-comments-route-security.test.ts create mode 100644 src/lib/__tests__/task-queue-route-security.test.ts create mode 100644 src/lib/__tests__/task-route-security.test.ts create mode 100644 src/lib/__tests__/webhooks-route-security.test.ts create mode 100644 src/lib/__tests__/workspace-scope.test.ts create mode 100644 src/lib/enforcement/workspace-scope.ts diff --git a/src/app/api/agents/[id]/heartbeat/route.ts b/src/app/api/agents/[id]/heartbeat/route.ts index 27d9620749..59082699ae 100644 --- a/src/app/api/agents/[id]/heartbeat/route.ts +++ b/src/app/api/agents/[id]/heartbeat/route.ts @@ -4,6 +4,7 @@ import { requireRole } from '@/lib/auth'; import { agentHeartbeatLimiter } from '@/lib/rate-limit'; import { logger } from '@/lib/logger'; import { resolveTaskImplementationTarget } from '@/lib/task-routing'; +import { requireAgentSelfAccess, requireWorkspaceId } from '@/lib/enforcement/workspace-scope'; /** * GET /api/agents/[id]/heartbeat - Agent heartbeat check @@ -26,8 +27,14 @@ export async function GET( const db = getDatabase(); const resolvedParams = await params; const agentId = resolvedParams.id; - const workspaceId = auth.user.workspace_id ?? 1; - + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; + + // Agent key resource scoping: an agent key may only read its own heartbeat. + const selfDeny = requireAgentSelfAccess(auth.user, agentId); + if (selfDeny) return selfDeny; + // Get agent by ID or name let agent: any; if (isNaN(Number(agentId))) { @@ -37,7 +44,7 @@ export async function GET( // Lookup by ID agent = db.prepare('SELECT * FROM agents WHERE id = ? AND workspace_id = ?').get(Number(agentId), workspaceId); } - + if (!agent) { return NextResponse.json({ error: 'Agent not found' }, { status: 404 }); } @@ -195,6 +202,11 @@ export async function POST( const rateLimited = agentHeartbeatLimiter(request); if (rateLimited) return rateLimited; + // Agent key resource scoping: an agent key may only update its own heartbeat. + const routeParams = await params; + const selfDeny = requireAgentSelfAccess(auth.user, routeParams.id); + if (selfDeny) return selfDeny; + let body: any = {}; try { body = await request.json(); @@ -205,7 +217,9 @@ export async function POST( const { connection_id, token_usage } = body; const db = getDatabase(); const now = Math.floor(Date.now() / 1000); - const workspaceId = auth.user.workspace_id ?? 1; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; // Update direct connection heartbeat if connection_id provided if (connection_id) { diff --git a/src/app/api/agents/[id]/memory/route.ts b/src/app/api/agents/[id]/memory/route.ts index 0e14bb76ec..4be01383a2 100644 --- a/src/app/api/agents/[id]/memory/route.ts +++ b/src/app/api/agents/[id]/memory/route.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from 'next/server'; import { getDatabase, db_helpers } from '@/lib/db'; import { requireRole } from '@/lib/auth'; import { logger } from '@/lib/logger'; +import { requireAgentSelfAccess, requireWorkspaceId } from '@/lib/enforcement/workspace-scope'; import { statSync, mkdirSync, writeFileSync } from 'node:fs'; import { dirname } from 'node:path'; import { resolveWithin } from '@/lib/paths'; @@ -43,7 +44,11 @@ export async function GET( const db = getDatabase(); const resolvedParams = await params; const agentId = resolvedParams.id; - const workspaceId = auth.user.workspace_id ?? 1; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; + const selfDeny = requireAgentSelfAccess(auth.user, agentId); + if (selfDeny) return selfDeny; const agent = getAgentByIdOrName(db, agentId, workspaceId); if (!agent) { @@ -117,7 +122,11 @@ export async function PUT( const db = getDatabase(); const resolvedParams = await params; const agentId = resolvedParams.id; - const workspaceId = auth.user.workspace_id ?? 1; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; + const selfDeny = requireAgentSelfAccess(auth.user, agentId); + if (selfDeny) return selfDeny; const body = await request.json(); const { working_memory, append } = body; @@ -220,7 +229,11 @@ export async function DELETE( const db = getDatabase(); const resolvedParams = await params; const agentId = resolvedParams.id; - const workspaceId = auth.user.workspace_id ?? 1; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; + const selfDeny = requireAgentSelfAccess(auth.user, agentId); + if (selfDeny) return selfDeny; const agent = getAgentByIdOrName(db, agentId, workspaceId); if (!agent) { diff --git a/src/app/api/tasks/[id]/comments/route.ts b/src/app/api/tasks/[id]/comments/route.ts index dbbebf9c4c..88c467002f 100644 --- a/src/app/api/tasks/[id]/comments/route.ts +++ b/src/app/api/tasks/[id]/comments/route.ts @@ -5,6 +5,7 @@ import { validateBody, createCommentSchema } from '@/lib/validation'; import { mutationLimiter } from '@/lib/rate-limit'; import { logger } from '@/lib/logger'; import { resolveMentionRecipients } from '@/lib/mentions'; +import { requireAgentTaskAccess, requireWorkspaceId } from '@/lib/enforcement/workspace-scope'; /** * GET /api/tasks/[id]/comments - Get all comments for a task @@ -20,7 +21,9 @@ export async function GET( const db = getDatabase(); const resolvedParams = await params; const taskId = parseInt(resolvedParams.id); - const workspaceId = auth.user.workspace_id ?? 1; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; if (isNaN(taskId)) { return NextResponse.json({ error: 'Invalid task ID' }, { status: 400 }); @@ -28,12 +31,15 @@ export async function GET( // Verify task exists const task = db - .prepare('SELECT id FROM tasks WHERE id = ? AND workspace_id = ?') - .get(taskId, workspaceId); + .prepare('SELECT id, assigned_to FROM tasks WHERE id = ? AND workspace_id = ?') + .get(taskId, workspaceId) as { id: number; assigned_to: string | null } | undefined; if (!task) { return NextResponse.json({ error: 'Task not found' }, { status: 404 }); } - + + const taskDeny = requireAgentTaskAccess(auth.user, task.assigned_to); + if (taskDeny) return taskDeny; + // Get comments ordered by creation time const stmt = db.prepare(` SELECT * FROM comments @@ -101,7 +107,9 @@ export async function POST( const db = getDatabase(); const resolvedParams = await params; const taskId = parseInt(resolvedParams.id); - const workspaceId = auth.user.workspace_id ?? 1; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; if (isNaN(taskId)) { return NextResponse.json({ error: 'Invalid task ID' }, { status: 400 }); @@ -141,7 +149,10 @@ export async function POST( if (!task) { return NextResponse.json({ error: 'Task not found' }, { status: 404 }); } - + + const taskDeny = requireAgentTaskAccess(auth.user, task.assigned_to ?? null); + if (taskDeny) return taskDeny; + // Verify parent comment exists if specified if (parent_id) { const parentComment = db diff --git a/src/app/api/tasks/[id]/route.ts b/src/app/api/tasks/[id]/route.ts index 46de0c6281..07e4519c77 100644 --- a/src/app/api/tasks/[id]/route.ts +++ b/src/app/api/tasks/[id]/route.ts @@ -11,6 +11,7 @@ import { reconcileDeferredTaskCompletions } from '@/lib/task-dispatch'; import { syncTaskOutbound } from '@/lib/github-sync-engine'; import { removeTaskFromGnap } from '@/lib/gnap-sync'; import { config } from '@/lib/config'; +import { requireAgentTaskAccess, requireWorkspaceId } from '@/lib/enforcement/workspace-scope'; function formatTicketRef(prefix?: string | null, num?: number | null): string | undefined { if (!prefix || typeof num !== 'number' || !Number.isFinite(num) || num <= 0) return undefined @@ -54,7 +55,9 @@ export async function GET( const db = getDatabase(); const resolvedParams = await params; const taskId = parseInt(resolvedParams.id); - const workspaceId = auth.user.workspace_id ?? 1; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; if (isNaN(taskId)) { return NextResponse.json({ error: 'Invalid task ID' }, { status: 400 }); @@ -77,10 +80,13 @@ export async function GET( if (!task) { return NextResponse.json({ error: 'Task not found' }, { status: 404 }); } - + + const taskDeny = requireAgentTaskAccess(auth.user, (task as Task).assigned_to ?? null); + if (taskDeny) return taskDeny; + // Parse JSON fields const taskWithParsedData = mapTaskRow(task); - + return NextResponse.json({ task: taskWithParsedData }); } catch (error) { logger.error({ err: error }, 'GET /api/tasks/[id] error'); @@ -105,7 +111,9 @@ export async function PUT( const db = getDatabase(); const resolvedParams = await params; const taskId = parseInt(resolvedParams.id); - const workspaceId = auth.user.workspace_id ?? 1; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; const validated = await validateBody(request, updateTaskSchema); if ('error' in validated) return validated.error; const body = validated.data; @@ -122,7 +130,10 @@ export async function PUT( if (!currentTask) { return NextResponse.json({ error: 'Task not found' }, { status: 404 }); } - + + const taskDeny = requireAgentTaskAccess(auth.user, currentTask.assigned_to ?? null); + if (taskDeny) return taskDeny; + const { title, description, @@ -430,7 +441,9 @@ export async function DELETE( const db = getDatabase(); const resolvedParams = await params; const taskId = parseInt(resolvedParams.id); - const workspaceId = auth.user.workspace_id ?? 1; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; if (isNaN(taskId)) { return NextResponse.json({ error: 'Invalid task ID' }, { status: 400 }); @@ -444,7 +457,10 @@ export async function DELETE( if (!task) { return NextResponse.json({ error: 'Task not found' }, { status: 404 }); } - + + const taskDeny = requireAgentTaskAccess(auth.user, (task as Task).assigned_to ?? null); + if (taskDeny) return taskDeny; + // Delete task (cascades will handle comments) const stmt = db.prepare('DELETE FROM tasks WHERE id = ? AND workspace_id = ?'); stmt.run(taskId, workspaceId); diff --git a/src/app/api/tasks/queue/route.ts b/src/app/api/tasks/queue/route.ts index d70f4a3809..b323737a16 100644 --- a/src/app/api/tasks/queue/route.ts +++ b/src/app/api/tasks/queue/route.ts @@ -3,6 +3,7 @@ import { getDatabase } from '@/lib/db' import { requireRole } from '@/lib/auth' import { agentTaskLimiter } from '@/lib/rate-limit' import { logger } from '@/lib/logger' +import { requireWorkspaceId } from '@/lib/enforcement/workspace-scope' type QueueReason = 'continue_current' | 'assigned' | 'at_capacity' | 'no_tasks_available' @@ -51,7 +52,9 @@ export async function GET(request: NextRequest) { try { const db = getDatabase() - const workspaceId = auth.user.workspace_id + const wsResult = requireWorkspaceId(auth.user) + if (!('workspaceId' in wsResult)) return wsResult.response + const { workspaceId } = wsResult const { searchParams } = new URL(request.url) const agent = @@ -62,6 +65,11 @@ export async function GET(request: NextRequest) { return NextResponse.json({ error: 'Missing agent. Provide ?agent=... or x-agent-name header.' }, { status: 400 }) } + // Agent keys (non-admin) may only queue for themselves + if (auth.user.agent_name && auth.user.role !== 'admin' && agent !== auth.user.agent_name) { + return NextResponse.json({ error: 'Access denied: agent key may only queue tasks for itself.' }, { status: 403 }) + } + const maxCapacityRaw = searchParams.get('max_capacity') || '1' if (!/^\d+$/.test(maxCapacityRaw)) { return NextResponse.json({ error: 'Invalid max_capacity. Expected integer 1..20.' }, { status: 400 }) diff --git a/src/app/api/tasks/route.ts b/src/app/api/tasks/route.ts index 605afb2b22..6357190dd8 100644 --- a/src/app/api/tasks/route.ts +++ b/src/app/api/tasks/route.ts @@ -11,6 +11,7 @@ import { reconcileDeferredTaskCompletions } from '@/lib/task-dispatch'; import { pushTaskToGitHub, syncTaskOutbound } from '@/lib/github-sync-engine'; import { pushTaskToGnap } from '@/lib/gnap-sync'; import { config } from '@/lib/config'; +import { requireWorkspaceId } from '@/lib/enforcement/workspace-scope'; function formatTicketRef(prefix?: string | null, num?: number | null): string | undefined { if (!prefix || typeof num !== 'number' || !Number.isFinite(num) || num <= 0) return undefined @@ -69,7 +70,9 @@ export async function GET(request: NextRequest) { try { const db = getDatabase(); - const workspaceId = auth.user.workspace_id; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; const { searchParams } = new URL(request.url); // Parse query parameters @@ -102,11 +105,19 @@ export async function GET(request: NextRequest) { params.push(status); } - if (assigned_to) { + // Agent keys (non-admin) may only list their own tasks + const agentScope = auth.user.agent_name && auth.user.role !== 'admin' + ? auth.user.agent_name + : null; + + if (agentScope) { + query += ' AND t.assigned_to = ?'; + params.push(agentScope); + } else if (assigned_to) { query += ' AND t.assigned_to = ?'; params.push(assigned_to); } - + if (priority) { query += ' AND t.priority = ?'; params.push(priority); @@ -133,7 +144,10 @@ export async function GET(request: NextRequest) { countQuery += ' AND status = ?'; countParams.push(status); } - if (assigned_to) { + if (agentScope) { + countQuery += ' AND assigned_to = ?'; + countParams.push(agentScope); + } else if (assigned_to) { countQuery += ' AND assigned_to = ?'; countParams.push(assigned_to); } @@ -166,7 +180,9 @@ export async function POST(request: NextRequest) { try { const db = getDatabase(); - const workspaceId = auth.user.workspace_id; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; const validated = await validateBody(request, createTaskSchema); if ('error' in validated) return validated.error; const body = validated.data; @@ -352,7 +368,9 @@ export async function PUT(request: NextRequest) { try { const db = getDatabase(); - const workspaceId = auth.user.workspace_id; + const wsResult = requireWorkspaceId(auth.user); + if (!('workspaceId' in wsResult)) return wsResult.response; + const { workspaceId } = wsResult; const validated = await validateBody(request, bulkUpdateTaskStatusSchema); if ('error' in validated) return validated.error; const { tasks } = validated.data; diff --git a/src/app/api/webhooks/route.ts b/src/app/api/webhooks/route.ts index 235ce605ab..d33729b962 100644 --- a/src/app/api/webhooks/route.ts +++ b/src/app/api/webhooks/route.ts @@ -5,6 +5,7 @@ import { randomBytes } from 'crypto' import { mutationLimiter } from '@/lib/rate-limit' import { logger } from '@/lib/logger' import { validateBody, createWebhookSchema } from '@/lib/validation' +import { requireWorkspaceId } from '@/lib/enforcement/workspace-scope' const WEBHOOK_BLOCKED_HOSTNAMES = new Set([ 'localhost', '127.0.0.1', '::1', '0.0.0.0', @@ -41,7 +42,9 @@ export async function GET(request: NextRequest) { try { const db = getDatabase() - const workspaceId = auth.user.workspace_id ?? 1 + const wsResult = requireWorkspaceId(auth.user) + if (!('workspaceId' in wsResult)) return wsResult.response + const { workspaceId } = wsResult const webhooks = db.prepare(` SELECT w.*, (SELECT COUNT(*) FROM webhook_deliveries wd WHERE wd.webhook_id = w.id AND wd.workspace_id = w.workspace_id) as total_deliveries, @@ -82,7 +85,9 @@ export async function POST(request: NextRequest) { try { const db = getDatabase() - const workspaceId = auth.user.workspace_id ?? 1 + const wsResult = requireWorkspaceId(auth.user) + if (!('workspaceId' in wsResult)) return wsResult.response + const { workspaceId } = wsResult const validated = await validateBody(request, createWebhookSchema) if ('error' in validated) return validated.error const body = validated.data @@ -127,7 +132,9 @@ export async function PUT(request: NextRequest) { try { const db = getDatabase() - const workspaceId = auth.user.workspace_id ?? 1 + const wsResult = requireWorkspaceId(auth.user) + if (!('workspaceId' in wsResult)) return wsResult.response + const { workspaceId } = wsResult const body = await request.json() const { id, name, url, events, enabled, regenerate_secret, reset_circuit } = body @@ -195,7 +202,9 @@ export async function DELETE(request: NextRequest) { try { const db = getDatabase() - const workspaceId = auth.user.workspace_id ?? 1 + const wsResult = requireWorkspaceId(auth.user) + if (!('workspaceId' in wsResult)) return wsResult.response + const { workspaceId } = wsResult let body: any try { body = await request.json() } catch { return NextResponse.json({ error: 'Request body required' }, { status: 400 }) } const id = body.id diff --git a/src/lib/__tests__/agent-memory-route-security.test.ts b/src/lib/__tests__/agent-memory-route-security.test.ts new file mode 100644 index 0000000000..c62e804f16 --- /dev/null +++ b/src/lib/__tests__/agent-memory-route-security.test.ts @@ -0,0 +1,174 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { NextRequest } from 'next/server' + +const requireRoleMock = vi.fn() +const prepareMock = vi.fn() + +vi.mock('@/lib/auth', () => ({ requireRole: requireRoleMock })) +vi.mock('@/lib/logger', () => ({ + logger: { error: vi.fn(), warn: vi.fn(), info: vi.fn() }, +})) +vi.mock('@/lib/db', () => ({ + getDatabase: vi.fn(() => ({ prepare: prepareMock })), + db_helpers: { + logActivity: vi.fn(), + }, +})) +vi.mock('@/lib/agent-workspace', () => ({ + getAgentWorkspaceCandidates: vi.fn(() => []), + readAgentWorkspaceFile: vi.fn(() => ({ exists: false, path: null, content: '' })), +})) +vi.mock('@/lib/paths', () => ({ + resolveWithin: vi.fn((base: string, name: string) => `${base}/${name}`), +})) + +describe('agent memory route security', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + }) + + it('GET fails closed when workspace_id is missing', async () => { + requireRoleMock.mockReturnValue({ + user: { username: 'agent-a', role: 'viewer', agent_name: 'agent-a' }, + }) + + const { GET } = await import('@/app/api/agents/[id]/memory/route') + const response = await GET( + new NextRequest('http://localhost/api/agents/agent-a/memory'), + { params: Promise.resolve({ id: 'agent-a' }) }, + ) + + expect(response.status).toBe(400) + await expect(response.json()).resolves.toEqual({ error: 'Workspace context required' }) + expect(prepareMock).not.toHaveBeenCalled() + }) + + it('GET allows an agent key to access its own memory route', async () => { + prepareMock.mockImplementation((sql: string) => { + if (sql.includes('SELECT * FROM agents')) { + return { + get: vi.fn(() => ({ + id: 42, + name: 'agent-a', + role: 'operator', + updated_at: 123, + config: null, + })), + } + } + + if (sql.includes('SELECT working_memory, updated_at FROM agents')) { + return { + get: vi.fn(() => ({ + working_memory: 'hello', + updated_at: 123, + })), + } + } + + return { get: vi.fn(() => undefined) } + }) + + requireRoleMock.mockReturnValue({ + user: { + username: 'agent-a', + role: 'viewer', + workspace_id: 7, + agent_name: 'agent-a', + agent_id: 42, + }, + }) + + const { GET } = await import('@/app/api/agents/[id]/memory/route') + const response = await GET( + new NextRequest('http://localhost/api/agents/42/memory'), + { params: Promise.resolve({ id: '42' }) }, + ) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toMatchObject({ + agent: { id: 42, name: 'agent-a' }, + working_memory: 'hello', + source: 'database', + }) + }) + + it('GET denies same-workspace agent overreach before reading another agent memory', async () => { + requireRoleMock.mockReturnValue({ + user: { + username: 'agent-a', + role: 'viewer', + workspace_id: 7, + agent_name: 'agent-a', + agent_id: 42, + }, + }) + + const { GET } = await import('@/app/api/agents/[id]/memory/route') + const response = await GET( + new NextRequest('http://localhost/api/agents/99/memory'), + { params: Promise.resolve({ id: '99' }) }, + ) + + expect(response.status).toBe(403) + await expect(response.json()).resolves.toEqual({ + error: 'Access denied: agent key may only access its own agent.', + }) + expect(prepareMock).not.toHaveBeenCalled() + }) + + it('PUT denies same-workspace agent overreach before updating another agent memory', async () => { + requireRoleMock.mockReturnValue({ + user: { + username: 'agent-a', + role: 'operator', + workspace_id: 7, + agent_name: 'agent-a', + agent_id: 42, + }, + }) + + const { PUT } = await import('@/app/api/agents/[id]/memory/route') + const response = await PUT( + new NextRequest('http://localhost/api/agents/99/memory', { + method: 'PUT', + body: JSON.stringify({ working_memory: 'secret' }), + headers: { 'content-type': 'application/json' }, + }), + { params: Promise.resolve({ id: '99' }) }, + ) + + expect(response.status).toBe(403) + await expect(response.json()).resolves.toEqual({ + error: 'Access denied: agent key may only access its own agent.', + }) + expect(prepareMock).not.toHaveBeenCalled() + }) + + it('DELETE denies same-workspace agent overreach before clearing another agent memory', async () => { + requireRoleMock.mockReturnValue({ + user: { + username: 'agent-a', + role: 'operator', + workspace_id: 7, + agent_name: 'agent-a', + agent_id: 42, + }, + }) + + const { DELETE } = await import('@/app/api/agents/[id]/memory/route') + const response = await DELETE( + new NextRequest('http://localhost/api/agents/99/memory', { + method: 'DELETE', + }), + { params: Promise.resolve({ id: '99' }) }, + ) + + expect(response.status).toBe(403) + await expect(response.json()).resolves.toEqual({ + error: 'Access denied: agent key may only access its own agent.', + }) + expect(prepareMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/lib/__tests__/heartbeat-route-security.test.ts b/src/lib/__tests__/heartbeat-route-security.test.ts new file mode 100644 index 0000000000..25ceea9ce0 --- /dev/null +++ b/src/lib/__tests__/heartbeat-route-security.test.ts @@ -0,0 +1,94 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { NextRequest } from 'next/server' + +const requireRoleMock = vi.fn() +const getDatabaseMock = vi.fn() +const agentHeartbeatLimiterMock = vi.fn(() => null) + +vi.mock('@/lib/auth', () => ({ + requireRole: requireRoleMock, +})) + +vi.mock('@/lib/db', () => ({ + getDatabase: getDatabaseMock, + db_helpers: { + getUnreadNotifications: vi.fn(() => []), + updateAgentStatus: vi.fn(), + logActivity: vi.fn(), + }, +})) + +vi.mock('@/lib/rate-limit', () => ({ + agentHeartbeatLimiter: agentHeartbeatLimiterMock, +})) + +vi.mock('@/lib/logger', () => ({ + logger: { + error: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + }, +})) + +vi.mock('@/lib/task-routing', () => ({ + resolveTaskImplementationTarget: vi.fn(() => ({})), +})) + +describe('heartbeat route security', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + getDatabaseMock.mockReturnValue({ prepare: vi.fn() }) + agentHeartbeatLimiterMock.mockReturnValue(null) + }) + + it('GET fails closed when workspace_id is missing', async () => { + requireRoleMock.mockReturnValue({ + user: { username: 'agent-a', role: 'viewer', agent_name: 'agent-a' }, + }) + + const { GET } = await import('@/app/api/agents/[id]/heartbeat/route') + const response = await GET( + new NextRequest('http://localhost/api/agents/agent-a/heartbeat'), + { params: Promise.resolve({ id: 'agent-a' }) }, + ) + + expect(response.status).toBe(400) + await expect(response.json()).resolves.toEqual({ error: 'Workspace context required' }) + }) + + it('POST fails closed when workspace_id is missing', async () => { + requireRoleMock.mockReturnValue({ + user: { username: 'agent-a', role: 'operator', agent_name: 'agent-a' }, + }) + + const { POST } = await import('@/app/api/agents/[id]/heartbeat/route') + const response = await POST( + new NextRequest('http://localhost/api/agents/agent-a/heartbeat', { method: 'POST' }), + { params: Promise.resolve({ id: 'agent-a' }) }, + ) + + expect(response.status).toBe(400) + await expect(response.json()).resolves.toEqual({ error: 'Workspace context required' }) + }) + + it('GET denies agent overreach before reading agent records', async () => { + const prepareMock = vi.fn() + getDatabaseMock.mockReturnValue({ prepare: prepareMock }) + requireRoleMock.mockReturnValue({ + user: { username: 'agent-a', role: 'viewer', agent_name: 'agent-a', workspace_id: 7 }, + }) + + const { GET } = await import('@/app/api/agents/[id]/heartbeat/route') + const response = await GET( + new NextRequest('http://localhost/api/agents/agent-b/heartbeat'), + { params: Promise.resolve({ id: 'agent-b' }) }, + ) + + expect(response.status).toBe(403) + await expect(response.json()).resolves.toEqual({ + error: 'Access denied: agent key may only access its own agent.', + }) + expect(prepareMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/lib/__tests__/task-comments-route-security.test.ts b/src/lib/__tests__/task-comments-route-security.test.ts new file mode 100644 index 0000000000..ed62e27d1f --- /dev/null +++ b/src/lib/__tests__/task-comments-route-security.test.ts @@ -0,0 +1,87 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { NextRequest } from 'next/server' + +const requireRoleMock = vi.fn() +const prepareMock = vi.fn() + +vi.mock('@/lib/auth', () => ({ requireRole: requireRoleMock })) +vi.mock('@/lib/rate-limit', () => ({ mutationLimiter: vi.fn(() => null) })) +vi.mock('@/lib/validation', () => ({ + validateBody: vi.fn(), + createCommentSchema: {}, +})) +vi.mock('@/lib/mentions', () => ({ resolveMentionRecipients: vi.fn(() => ({ resolved: [], recipients: [], tokens: [], unresolved: [] })) })) +vi.mock('@/lib/logger', () => ({ logger: { error: vi.fn(), warn: vi.fn(), info: vi.fn() } })) +vi.mock('@/lib/db', () => ({ + getDatabase: vi.fn(() => ({ prepare: prepareMock })), + db_helpers: { + logActivity: vi.fn(), + ensureTaskSubscription: vi.fn(), + createNotification: vi.fn(), + getTaskSubscribers: vi.fn(() => []), + }, +})) + +describe('task comments route security', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + }) + + it('GET fails closed when workspace_id is missing', async () => { + requireRoleMock.mockReturnValue({ + user: { username: 'tester', role: 'viewer' }, + }) + + const { GET } = await import('@/app/api/tasks/[id]/comments/route') + const response = await GET( + new NextRequest('http://localhost/api/tasks/42/comments'), + { params: Promise.resolve({ id: '42' }) }, + ) + + expect(response.status).toBe(400) + await expect(response.json()).resolves.toEqual({ error: 'Workspace context required' }) + expect(prepareMock).not.toHaveBeenCalled() + }) + + it('POST fails closed when workspace_id is missing', async () => { + requireRoleMock.mockReturnValue({ + user: { username: 'tester', role: 'operator' }, + }) + + const { POST } = await import('@/app/api/tasks/[id]/comments/route') + const response = await POST( + new NextRequest('http://localhost/api/tasks/42/comments', { + method: 'POST', + body: JSON.stringify({ content: 'hello' }), + headers: { 'content-type': 'application/json' }, + }), + { params: Promise.resolve({ id: '42' }) }, + ) + + expect(response.status).toBe(400) + await expect(response.json()).resolves.toEqual({ error: 'Workspace context required' }) + }) + + it('GET denies agent overreach on comments for another agent task', async () => { + const getMock = vi.fn(() => ({ + id: 42, + assigned_to: 'agent-b', + })) + prepareMock.mockReturnValue({ get: getMock, all: vi.fn(() => []) }) + requireRoleMock.mockReturnValue({ + user: { username: 'agent-a', role: 'viewer', workspace_id: 7, agent_name: 'agent-a' }, + }) + + const { GET } = await import('@/app/api/tasks/[id]/comments/route') + const response = await GET( + new NextRequest('http://localhost/api/tasks/42/comments'), + { params: Promise.resolve({ id: '42' }) }, + ) + + expect(response.status).toBe(403) + await expect(response.json()).resolves.toEqual({ + error: 'Access denied: agent key may only access its own tasks.', + }) + }) +}) diff --git a/src/lib/__tests__/task-queue-route-security.test.ts b/src/lib/__tests__/task-queue-route-security.test.ts new file mode 100644 index 0000000000..2bf8d95435 --- /dev/null +++ b/src/lib/__tests__/task-queue-route-security.test.ts @@ -0,0 +1,53 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { NextRequest } from 'next/server' + +const requireRoleMock = vi.fn() +const agentTaskLimiterMock = vi.fn(() => null) +const prepareMock = vi.fn() + +vi.mock('@/lib/auth', () => ({ requireRole: requireRoleMock })) +vi.mock('@/lib/rate-limit', () => ({ agentTaskLimiter: agentTaskLimiterMock })) +vi.mock('@/lib/logger', () => ({ logger: { error: vi.fn(), warn: vi.fn(), info: vi.fn() } })) +vi.mock('@/lib/db', () => ({ + getDatabase: vi.fn(() => ({ prepare: prepareMock })), +})) + +describe('task queue route security', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + agentTaskLimiterMock.mockReturnValue(null) + }) + + it('GET fails closed when workspace_id is missing', async () => { + requireRoleMock.mockReturnValue({ + user: { username: 'agent-a', role: 'operator', agent_name: 'agent-a' }, + }) + + const { GET } = await import('@/app/api/tasks/queue/route') + const response = await GET( + new NextRequest('http://localhost/api/tasks/queue?agent=agent-a'), + ) + + expect(response.status).toBe(400) + await expect(response.json()).resolves.toEqual({ error: 'Workspace context required' }) + expect(prepareMock).not.toHaveBeenCalled() + }) + + it('GET denies agent overreach when queueing for another agent', async () => { + requireRoleMock.mockReturnValue({ + user: { username: 'agent-a', role: 'operator', agent_name: 'agent-a', workspace_id: 7 }, + }) + + const { GET } = await import('@/app/api/tasks/queue/route') + const response = await GET( + new NextRequest('http://localhost/api/tasks/queue?agent=agent-b'), + ) + + expect(response.status).toBe(403) + await expect(response.json()).resolves.toEqual({ + error: 'Access denied: agent key may only queue tasks for itself.', + }) + expect(prepareMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/lib/__tests__/task-route-security.test.ts b/src/lib/__tests__/task-route-security.test.ts new file mode 100644 index 0000000000..49211db4cd --- /dev/null +++ b/src/lib/__tests__/task-route-security.test.ts @@ -0,0 +1,93 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { NextRequest } from 'next/server' + +const requireRoleMock = vi.fn() +const prepareMock = vi.fn() + +vi.mock('@/lib/auth', () => ({ requireRole: requireRoleMock })) +vi.mock('@/lib/rate-limit', () => ({ mutationLimiter: vi.fn(() => null) })) +vi.mock('@/lib/validation', () => ({ + validateBody: vi.fn(), + updateTaskSchema: {}, +})) +vi.mock('@/lib/mentions', () => ({ resolveMentionRecipients: vi.fn(() => ({ recipients: [], unresolved: [] })) })) +vi.mock('@/lib/task-status', () => ({ normalizeTaskUpdateStatus: vi.fn() })) +vi.mock('@/lib/event-bus', () => ({ eventBus: { broadcast: vi.fn() } })) +vi.mock('@/lib/github-sync-engine', () => ({ syncTaskOutbound: vi.fn() })) +vi.mock('@/lib/gnap-sync', () => ({ removeTaskFromGnap: vi.fn() })) +vi.mock('@/lib/config', () => ({ config: { gnap: { enabled: false, autoSync: false, repoPath: '' } } })) +vi.mock('@/lib/logger', () => ({ logger: { error: vi.fn(), warn: vi.fn(), info: vi.fn() } })) +vi.mock('@/lib/db', () => ({ + getDatabase: vi.fn(() => ({ prepare: prepareMock })), + db_helpers: { + createNotification: vi.fn(), + ensureTaskSubscription: vi.fn(), + logActivity: vi.fn(), + }, +})) + +describe('task detail route security', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + }) + + it('GET fails closed when workspace_id is missing', async () => { + requireRoleMock.mockReturnValue({ + user: { username: 'tester', role: 'viewer' }, + }) + + const { GET } = await import('@/app/api/tasks/[id]/route') + const response = await GET( + new NextRequest('http://localhost/api/tasks/42'), + { params: Promise.resolve({ id: '42' }) }, + ) + + expect(response.status).toBe(400) + await expect(response.json()).resolves.toEqual({ error: 'Workspace context required' }) + expect(prepareMock).not.toHaveBeenCalled() + }) + + it('GET denies cross-workspace access by scoping lookup to the caller workspace', async () => { + const getMock = vi.fn(() => undefined) + prepareMock.mockReturnValue({ get: getMock }) + requireRoleMock.mockReturnValue({ + user: { username: 'tester', role: 'viewer', workspace_id: 7 }, + }) + + const { GET } = await import('@/app/api/tasks/[id]/route') + const response = await GET( + new NextRequest('http://localhost/api/tasks/42'), + { params: Promise.resolve({ id: '42' }) }, + ) + + expect(response.status).toBe(404) + await expect(response.json()).resolves.toEqual({ error: 'Task not found' }) + expect(getMock).toHaveBeenCalledWith(42, 7) + }) + + it('GET denies agent overreach on another agent task', async () => { + const getMock = vi.fn(() => ({ + id: 42, + workspace_id: 7, + assigned_to: 'agent-b', + tags: '[]', + metadata: '{}', + })) + prepareMock.mockReturnValue({ get: getMock }) + requireRoleMock.mockReturnValue({ + user: { username: 'agent-a', role: 'viewer', workspace_id: 7, agent_name: 'agent-a' }, + }) + + const { GET } = await import('@/app/api/tasks/[id]/route') + const response = await GET( + new NextRequest('http://localhost/api/tasks/42'), + { params: Promise.resolve({ id: '42' }) }, + ) + + expect(response.status).toBe(403) + await expect(response.json()).resolves.toEqual({ + error: 'Access denied: agent key may only access its own tasks.', + }) + }) +}) diff --git a/src/lib/__tests__/webhooks-route-security.test.ts b/src/lib/__tests__/webhooks-route-security.test.ts new file mode 100644 index 0000000000..4e50434c2b --- /dev/null +++ b/src/lib/__tests__/webhooks-route-security.test.ts @@ -0,0 +1,84 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { NextRequest } from 'next/server' + +const requireRoleMock = vi.fn() +const mutationLimiterMock = vi.fn(() => null) +const prepareMock = vi.fn() + +vi.mock('@/lib/auth', () => ({ requireRole: requireRoleMock })) +vi.mock('@/lib/rate-limit', () => ({ mutationLimiter: mutationLimiterMock })) +vi.mock('@/lib/validation', () => ({ + validateBody: vi.fn(), + createWebhookSchema: {}, +})) +vi.mock('@/lib/logger', () => ({ logger: { error: vi.fn(), warn: vi.fn(), info: vi.fn() } })) +vi.mock('@/lib/db', () => ({ + getDatabase: vi.fn(() => ({ prepare: prepareMock })), +})) + +describe('webhooks route security', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + mutationLimiterMock.mockReturnValue(null) + }) + + it('PUT fails closed when workspace_id is missing', async () => { + requireRoleMock.mockReturnValue({ + user: { username: 'admin', role: 'admin' }, + }) + + const { PUT } = await import('@/app/api/webhooks/route') + const response = await PUT( + new NextRequest('http://localhost/api/webhooks', { + method: 'PUT', + body: JSON.stringify({ id: 22 }), + headers: { 'content-type': 'application/json' }, + }), + ) + + expect(response.status).toBe(400) + await expect(response.json()).resolves.toEqual({ error: 'Workspace context required' }) + expect(prepareMock).not.toHaveBeenCalled() + }) + + it('DELETE fails closed when workspace_id is missing', async () => { + requireRoleMock.mockReturnValue({ + user: { username: 'admin', role: 'admin' }, + }) + + const { DELETE } = await import('@/app/api/webhooks/route') + const response = await DELETE( + new NextRequest('http://localhost/api/webhooks', { + method: 'DELETE', + body: JSON.stringify({ id: 22 }), + headers: { 'content-type': 'application/json' }, + }), + ) + + expect(response.status).toBe(400) + await expect(response.json()).resolves.toEqual({ error: 'Workspace context required' }) + expect(prepareMock).not.toHaveBeenCalled() + }) + + it('PUT scopes webhook lookup to the caller workspace', async () => { + const getMock = vi.fn(() => undefined) + prepareMock.mockReturnValue({ get: getMock }) + requireRoleMock.mockReturnValue({ + user: { username: 'admin', role: 'admin', workspace_id: 7 }, + }) + + const { PUT } = await import('@/app/api/webhooks/route') + const response = await PUT( + new NextRequest('http://localhost/api/webhooks', { + method: 'PUT', + body: JSON.stringify({ id: 22 }), + headers: { 'content-type': 'application/json' }, + }), + ) + + expect(response.status).toBe(404) + await expect(response.json()).resolves.toEqual({ error: 'Webhook not found' }) + expect(getMock).toHaveBeenCalledWith(22, 7) + }) +}) diff --git a/src/lib/__tests__/workspace-scope.test.ts b/src/lib/__tests__/workspace-scope.test.ts new file mode 100644 index 0000000000..a34ed2fe30 --- /dev/null +++ b/src/lib/__tests__/workspace-scope.test.ts @@ -0,0 +1,221 @@ +import { describe, it, expect } from 'vitest' +import { + resolveWorkspaceId, + requireWorkspaceId, + isInWorkspace, + enforceWorkspaceBoundary, + requireAgentSelfAccess, + requireAgentTaskAccess, +} from '@/lib/enforcement/workspace-scope' +import type { User } from '@/lib/auth' + +function makeUser(overrides: Partial = {}): User { + return { + id: 1, + username: 'test', + display_name: 'Test', + role: 'operator', + workspace_id: 10, + tenant_id: 1, + created_at: 0, + updated_at: 0, + last_login_at: null, + ...overrides, + } +} + +// --------------------------------------------------------------------------- +// resolveWorkspaceId +// --------------------------------------------------------------------------- + +describe('resolveWorkspaceId', () => { + it('returns workspace_id when valid', () => { + expect(resolveWorkspaceId(makeUser({ workspace_id: 5 }))).toBe(5) + }) + + it('returns null for workspace_id = 0', () => { + expect(resolveWorkspaceId(makeUser({ workspace_id: 0 }))).toBeNull() + }) + + it('returns null for negative workspace_id', () => { + expect(resolveWorkspaceId(makeUser({ workspace_id: -1 }))).toBeNull() + }) + + it('returns null for NaN workspace_id', () => { + expect(resolveWorkspaceId(makeUser({ workspace_id: NaN }))).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// requireWorkspaceId +// --------------------------------------------------------------------------- + +describe('requireWorkspaceId', () => { + it('returns workspaceId for a valid user', () => { + const result = requireWorkspaceId(makeUser({ workspace_id: 7 })) + if (!('workspaceId' in result)) throw new Error('expected workspaceId') + expect(result.workspaceId).toBe(7) + }) + + it('returns a 400 response when workspace_id is missing', () => { + const result = requireWorkspaceId(makeUser({ workspace_id: 0 })) + if ('workspaceId' in result) throw new Error('expected response') + expect(result.response.status).toBe(400) + }) +}) + +// --------------------------------------------------------------------------- +// isInWorkspace +// --------------------------------------------------------------------------- + +describe('isInWorkspace', () => { + it('returns true when user workspace matches resource workspace', () => { + expect(isInWorkspace(makeUser({ workspace_id: 3 }), 3)).toBe(true) + }) + + it('returns false when workspaces differ', () => { + expect(isInWorkspace(makeUser({ workspace_id: 3 }), 99)).toBe(false) + }) + + it('returns false when user workspace_id is invalid', () => { + expect(isInWorkspace(makeUser({ workspace_id: 0 }), 1)).toBe(false) + }) + + it('user cannot read another workspace task (cross-workspace denied)', () => { + const user = makeUser({ workspace_id: 1 }) + expect(isInWorkspace(user, 2)).toBe(false) + }) +}) + +// --------------------------------------------------------------------------- +// enforceWorkspaceBoundary +// --------------------------------------------------------------------------- + +describe('enforceWorkspaceBoundary', () => { + it('returns null (allow) when workspaces match', () => { + const result = enforceWorkspaceBoundary(makeUser({ workspace_id: 4 }), 4) + expect(result).toBeNull() + }) + + it('returns 403 response when workspaces differ', () => { + const result = enforceWorkspaceBoundary(makeUser({ workspace_id: 4 }), 9) + expect(result).not.toBeNull() + expect(result!.status).toBe(403) + }) + + it('user cannot update another workspace task — boundary enforced', () => { + const user = makeUser({ workspace_id: 1 }) + const deny = enforceWorkspaceBoundary(user, 2) + expect(deny).not.toBeNull() + expect(deny!.status).toBe(403) + }) +}) + +// --------------------------------------------------------------------------- +// requireAgentSelfAccess +// --------------------------------------------------------------------------- + +describe('requireAgentSelfAccess — human user (no agent_name)', () => { + it('allows human user to access any agent', () => { + const user = makeUser({ agent_name: null }) + expect(requireAgentSelfAccess(user, 'repo-steward')).toBeNull() + expect(requireAgentSelfAccess(user, 'skill-intake')).toBeNull() + }) +}) + +describe('requireAgentSelfAccess — admin-scoped key', () => { + it('allows admin-scoped agent key to access any agent', () => { + const user = makeUser({ agent_name: 'repo-steward', role: 'admin' }) + expect(requireAgentSelfAccess(user, 'skill-intake')).toBeNull() + expect(requireAgentSelfAccess(user, 'other-agent')).toBeNull() + }) +}) + +describe('requireAgentSelfAccess — agent key (name-based path)', () => { + it('allows agent key to access its own agent by name', () => { + const user = makeUser({ agent_name: 'repo-steward', role: 'operator' }) + expect(requireAgentSelfAccess(user, 'repo-steward')).toBeNull() + }) + + it('blocks agent key from accessing another agent by name', () => { + const user = makeUser({ agent_name: 'repo-steward', role: 'operator' }) + const result = requireAgentSelfAccess(user, 'skill-intake') + expect(result).not.toBeNull() + expect(result!.status).toBe(403) + }) + + it('valid agent key accessing another agent fails — enforced', () => { + const user = makeUser({ agent_name: 'agent-a', role: 'operator' }) + const deny = requireAgentSelfAccess(user, 'agent-b') + expect(deny).not.toBeNull() + expect(deny!.status).toBe(403) + }) +}) + +describe('requireAgentSelfAccess — agent key (numeric ID path)', () => { + it('allows agent key by numeric id when agent_id matches', () => { + const user = makeUser({ agent_name: 'repo-steward', agent_id: 42, role: 'operator' }) + expect(requireAgentSelfAccess(user, '42')).toBeNull() + }) + + it('blocks agent key by numeric id when agent_id differs', () => { + const user = makeUser({ agent_name: 'repo-steward', agent_id: 42, role: 'operator' }) + const result = requireAgentSelfAccess(user, '99') + expect(result).not.toBeNull() + expect(result!.status).toBe(403) + }) + + it('allows numeric path when agent_id is not set (falls through to workspace filter)', () => { + const user = makeUser({ agent_name: 'repo-steward', agent_id: null, role: 'operator' }) + expect(requireAgentSelfAccess(user, '42')).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// requireAgentTaskAccess +// --------------------------------------------------------------------------- + +describe('requireAgentTaskAccess — human user', () => { + it('allows human user to access any task regardless of assignment', () => { + const user = makeUser({ agent_name: null }) + expect(requireAgentTaskAccess(user, 'other-agent')).toBeNull() + expect(requireAgentTaskAccess(user, null)).toBeNull() + }) +}) + +describe('requireAgentTaskAccess — admin-scoped key', () => { + it('allows admin-scoped key to access any task', () => { + const user = makeUser({ agent_name: 'repo-steward', role: 'admin' }) + expect(requireAgentTaskAccess(user, 'other-agent')).toBeNull() + expect(requireAgentTaskAccess(user, null)).toBeNull() + }) +}) + +describe('requireAgentTaskAccess — agent key', () => { + it('allows agent key to access its own assigned task', () => { + const user = makeUser({ agent_name: 'repo-steward', role: 'operator' }) + expect(requireAgentTaskAccess(user, 'repo-steward')).toBeNull() + }) + + it('blocks agent key from accessing another agent task — enforced', () => { + const user = makeUser({ agent_name: 'repo-steward', role: 'operator' }) + const deny = requireAgentTaskAccess(user, 'skill-intake') + expect(deny).not.toBeNull() + expect(deny!.status).toBe(403) + }) + + it('blocks agent key when task has no assignment (null)', () => { + const user = makeUser({ agent_name: 'repo-steward', role: 'operator' }) + const deny = requireAgentTaskAccess(user, null) + expect(deny).not.toBeNull() + expect(deny!.status).toBe(403) + }) + + it('admin/user auth behavior remains unchanged', () => { + const admin = makeUser({ agent_name: 'admin-agent', role: 'admin' }) + expect(requireAgentTaskAccess(admin, 'any-agent')).toBeNull() + + const human = makeUser({ role: 'operator', agent_name: null }) + expect(requireAgentTaskAccess(human, 'any-agent')).toBeNull() + }) +}) diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 92c7a42a97..e17e9dacc6 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -69,6 +69,8 @@ export interface User { last_login_at: number | null /** Agent name when request is made on behalf of a specific agent (via X-Agent-Name header) */ agent_name?: string | null + /** Numeric agent DB id — set only when authenticated via an agent-scoped API key */ + agent_id?: number | null } export interface UserSession { @@ -541,6 +543,7 @@ export function getUserFromRequest(request: Request): User | null { updated_at: now, last_login_at: now, agent_name: agent.name, + agent_id: agent.id, } } } diff --git a/src/lib/enforcement/workspace-scope.ts b/src/lib/enforcement/workspace-scope.ts new file mode 100644 index 0000000000..ea02293c41 --- /dev/null +++ b/src/lib/enforcement/workspace-scope.ts @@ -0,0 +1,116 @@ +/** + * Workspace Scope Enforcement + * + * Central helpers for deriving and enforcing workspace_id boundaries. + * Routes must not access resources from other workspaces; this module + * makes that check reusable and fail-closed. + */ + +import { NextResponse } from 'next/server' +import type { User } from '@/lib/auth' + +/** + * Derive workspace_id from an authenticated user. + * Returns null (fail-closed) if workspace_id is missing, non-numeric, or ≤ 0. + */ +export function resolveWorkspaceId(user: User): number | null { + const id = user.workspace_id + if (typeof id !== 'number' || !Number.isFinite(id) || id <= 0) return null + return id +} + +/** + * Require a valid workspace_id from an authenticated user. + * Returns `{ workspaceId }` on success or `{ response }` (400) when missing. + * Check with `'workspaceId' in result` to discriminate. + */ +export function requireWorkspaceId( + user: User, +): { workspaceId: number } | { response: NextResponse } { + const workspaceId = resolveWorkspaceId(user) + if (workspaceId === null) { + return { + response: NextResponse.json({ error: 'Workspace context required' }, { status: 400 }), + } + } + return { workspaceId } +} + +/** + * Check that a resource's workspace_id matches the caller's workspace. + */ +export function isInWorkspace(user: User, resourceWorkspaceId: number): boolean { + const id = resolveWorkspaceId(user) + if (id === null) return false + return id === resourceWorkspaceId +} + +/** + * Enforce workspace boundary for a specific resource. + * Returns null when access is allowed, or a 403 NextResponse to return directly. + * + * Usage: + * const deny = enforceWorkspaceBoundary(auth.user, task.workspace_id) + * if (deny) return deny + */ +export function enforceWorkspaceBoundary(user: User, resourceWorkspaceId: number): NextResponse | null { + if (!isInWorkspace(user, resourceWorkspaceId)) { + return NextResponse.json({ error: 'Access denied' }, { status: 403 }) + } + return null +} + +/** + * Enforce that an agent API key can only access its own agent. + * + * Accepts the URL path segment which may be a name ("repo-steward") or a + * numeric DB id ("42"). Compares by agent_id when numeric, by agent_name + * when a name string. Falls through (allows) when type cannot be determined. + * + * Human users (no agent_name) and admin-scoped keys are not restricted. + * + * Returns null when access is allowed, or a 403 NextResponse to return directly. + */ +export function requireAgentSelfAccess(user: User, targetAgentIdOrName: string): NextResponse | null { + if (!user.agent_name) return null + if (user.role === 'admin') return null + + const targetNumeric = Number(targetAgentIdOrName) + if (Number.isFinite(targetNumeric) && targetNumeric > 0) { + // Numeric ID path: compare by agent_id when available + if (user.agent_id != null && user.agent_id !== targetNumeric) { + return NextResponse.json( + { error: 'Access denied: agent key may only access its own agent.' }, + { status: 403 }, + ) + } + return null + } + + // Name-based path + if (user.agent_name !== targetAgentIdOrName) { + return NextResponse.json( + { error: 'Access denied: agent key may only access its own agent.' }, + { status: 403 }, + ) + } + return null +} + +/** + * Enforce that an agent API key can only access tasks assigned to itself. + * + * Human users (no agent_name) and admin-scoped keys are not restricted. + * Agent-scoped keys (agent_name set, non-admin) may only access tasks + * where assigned_to matches the key's agent_name. + * + * Returns null when access is allowed, or a 403 NextResponse to return directly. + */ +export function requireAgentTaskAccess(user: User, taskAssignedTo: string | null): NextResponse | null { + if (!user.agent_name) return null + if (user.role === 'admin') return null + if (taskAssignedTo !== user.agent_name) { + return NextResponse.json({ error: 'Access denied: agent key may only access its own tasks.' }, { status: 403 }) + } + return null +} diff --git a/src/lib/webhooks.ts b/src/lib/webhooks.ts index 0cf998645e..f06232f588 100644 --- a/src/lib/webhooks.ts +++ b/src/lib/webhooks.ts @@ -231,6 +231,20 @@ async function deliverWebhook( try { const { getDatabase } = await import('./db') const db = getDatabase() + const workspaceId = + typeof webhook.workspace_id === 'number' && + Number.isFinite(webhook.workspace_id) && + webhook.workspace_id > 0 + ? webhook.workspace_id + : null + + if (workspaceId === null) { + logger.error( + { webhookId: webhook.id, name: webhook.name }, + 'Webhook delivery bookkeeping skipped: workspace context required', + ) + return { success, status_code: statusCode, response_body: responseBody, error, duration_ms: durationMs, delivery_id: deliveryId } + } const insertResult = db.prepare(` INSERT INTO webhook_deliveries (webhook_id, event_type, payload, status_code, response_body, error, duration_ms, attempt, is_retry, parent_delivery_id, workspace_id) @@ -246,7 +260,7 @@ async function deliverWebhook( attempt, attempt > 0 ? 1 : 0, parentDeliveryId, - webhook.workspace_id ?? 1 + workspaceId ) deliveryId = Number(insertResult.lastInsertRowid) @@ -254,16 +268,16 @@ async function deliverWebhook( db.prepare(` UPDATE webhooks SET last_fired_at = unixepoch(), last_status = ?, updated_at = unixepoch() WHERE id = ? AND workspace_id = ? - `).run(statusCode ?? -1, webhook.id, webhook.workspace_id ?? 1) + `).run(statusCode ?? -1, webhook.id, workspaceId) // Circuit breaker + retry scheduling (skip for test deliveries) if (allowRetry) { if (success) { // Reset consecutive failures on success - db.prepare(`UPDATE webhooks SET consecutive_failures = 0 WHERE id = ? AND workspace_id = ?`).run(webhook.id, webhook.workspace_id ?? 1) + db.prepare(`UPDATE webhooks SET consecutive_failures = 0 WHERE id = ? AND workspace_id = ?`).run(webhook.id, workspaceId) } else { // Increment consecutive failures - db.prepare(`UPDATE webhooks SET consecutive_failures = consecutive_failures + 1 WHERE id = ? AND workspace_id = ?`).run(webhook.id, webhook.workspace_id ?? 1) + db.prepare(`UPDATE webhooks SET consecutive_failures = consecutive_failures + 1 WHERE id = ? AND workspace_id = ?`).run(webhook.id, workspaceId) if (attempt < MAX_RETRIES - 1) { // Schedule retry @@ -272,9 +286,9 @@ async function deliverWebhook( db.prepare(`UPDATE webhook_deliveries SET next_retry_at = ? WHERE id = ?`).run(nextRetryAt, deliveryId) } else { // Exhausted retries — trip circuit breaker - const wh = db.prepare(`SELECT consecutive_failures FROM webhooks WHERE id = ? AND workspace_id = ?`).get(webhook.id, webhook.workspace_id ?? 1) as { consecutive_failures: number } | undefined + const wh = db.prepare(`SELECT consecutive_failures FROM webhooks WHERE id = ? AND workspace_id = ?`).get(webhook.id, workspaceId) as { consecutive_failures: number } | undefined if (wh && wh.consecutive_failures >= MAX_RETRIES) { - db.prepare(`UPDATE webhooks SET enabled = 0, updated_at = unixepoch() WHERE id = ? AND workspace_id = ?`).run(webhook.id, webhook.workspace_id ?? 1) + db.prepare(`UPDATE webhooks SET enabled = 0, updated_at = unixepoch() WHERE id = ? AND workspace_id = ?`).run(webhook.id, workspaceId) logger.warn({ webhookId: webhook.id, name: webhook.name }, 'Webhook circuit breaker tripped — disabled after exhausting retries') } } @@ -287,7 +301,7 @@ async function deliverWebhook( WHERE webhook_id = ? AND workspace_id = ? AND id NOT IN ( SELECT id FROM webhook_deliveries WHERE webhook_id = ? AND workspace_id = ? ORDER BY created_at DESC LIMIT 200 ) - `).run(webhook.id, webhook.workspace_id ?? 1, webhook.id, webhook.workspace_id ?? 1) + `).run(webhook.id, workspaceId, webhook.id, workspaceId) } catch (logErr) { logger.error({ err: logErr, webhookId: webhook.id }, 'Webhook delivery logging/pruning failed') }