From 9adf4a0287a651cafbfda1bf3c108f7694f340a2 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 20 Apr 2026 08:12:07 -0500 Subject: [PATCH] Secure direct agent creation Route direct agent creation through the agents:create gate and reject the legacy direct route when new-agent approvals are required. Add regression coverage for missing permissions, allowed direct creation, and approval-required rejection. Co-Authored-By: Paperclip --- .../agent-adapter-validation-routes.test.ts | 14 ++- .../agent-permissions-routes.test.ts | 109 ++++++++++++++++-- server/src/routes/agents.ts | 25 +++- 3 files changed, 136 insertions(+), 12 deletions(-) diff --git a/server/src/__tests__/agent-adapter-validation-routes.test.ts b/server/src/__tests__/agent-adapter-validation-routes.test.ts index 1d54ff0ff24..6fabde6d420 100644 --- a/server/src/__tests__/agent-adapter-validation-routes.test.ts +++ b/server/src/__tests__/agent-adapter-validation-routes.test.ts @@ -131,7 +131,19 @@ async function createApp() { }; next(); }); - app.use("/api", agentRoutes({} as any)); + const db = { + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(async () => [ + { + id: "company-1", + requireBoardApprovalForNewAgents: false, + }, + ]), + })), + })), + }; + app.use("/api", agentRoutes(db as any)); app.use(errorHandler); return app; } diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 978e7c2e244..f1f903073b9 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -119,23 +119,25 @@ function registerModuleMocks() { })); } -function createDbStub() { +function createDbStub(options: { requireBoardApprovalForNewAgents?: boolean } = {}) { return { select: vi.fn().mockReturnValue({ from: vi.fn().mockReturnValue({ where: vi.fn().mockReturnValue({ - then: vi.fn().mockResolvedValue([{ - id: companyId, - name: "Paperclip", - requireBoardApprovalForNewAgents: false, - }]), + then: vi.fn((resolve) => + Promise.resolve(resolve([{ + id: companyId, + name: "Paperclip", + requireBoardApprovalForNewAgents: options.requireBoardApprovalForNewAgents ?? false, + }])), + ), }), }), }), }; } -async function createApp(actor: Record) { +async function createApp(actor: Record, dbOptions: { requireBoardApprovalForNewAgents?: boolean } = {}) { const [{ errorHandler }, { agentRoutes }] = await Promise.all([ vi.importActual("../middleware/index.js"), vi.importActual("../routes/agents.js"), @@ -146,7 +148,7 @@ async function createApp(actor: Record) { (req as any).actor = actor; next(); }); - app.use("/api", agentRoutes(createDbStub() as any)); + app.use("/api", agentRoutes(createDbStub(dbOptions) as any)); app.use(errorHandler); return app; } @@ -398,6 +400,97 @@ describe("agent permission routes", () => { expect(mockLogActivity).not.toHaveBeenCalled(); }); + it("blocks direct agent creation for authenticated company members without agent create permission", async () => { + mockAccessService.canUser.mockResolvedValue(false); + + const app = await createApp({ + type: "board", + userId: "member-user", + source: "session", + isInstanceAdmin: false, + companyIds: [companyId], + }); + + const res = await request(app) + .post(`/api/companies/${companyId}/agents`) + .send({ + name: "Backdoor", + role: "engineer", + adapterType: "process", + adapterConfig: {}, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("agents:create"); + expect(mockAgentService.create).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it("allows direct agent creation for authenticated board users with agent create permission when approval is not required", async () => { + mockAccessService.canUser.mockResolvedValue(true); + + const app = await createApp({ + type: "board", + userId: "agent-admin-user", + source: "session", + isInstanceAdmin: false, + companyIds: [companyId], + }); + + const res = await request(app) + .post(`/api/companies/${companyId}/agents`) + .send({ + name: "Builder", + role: "engineer", + adapterType: "process", + adapterConfig: {}, + }); + + expect(res.status).toBe(201); + expect(mockAgentService.create).toHaveBeenCalledWith( + companyId, + expect.objectContaining({ + status: "idle", + }), + ); + expect(mockAccessService.setPrincipalPermission).toHaveBeenCalledWith( + companyId, + "agent", + agentId, + "tasks:assign", + true, + "agent-admin-user", + ); + }); + + it("rejects direct agent creation when new agents require board approval", async () => { + const app = await createApp( + { + type: "board", + userId: "board-user", + source: "local_implicit", + isInstanceAdmin: true, + companyIds: [companyId], + }, + { requireBoardApprovalForNewAgents: true }, + ); + + const res = await request(app) + .post(`/api/companies/${companyId}/agents`) + .send({ + name: "Builder", + role: "engineer", + adapterType: "process", + adapterConfig: {}, + }); + + expect(res.status).toBe(409); + expect(res.body.error).toContain("/agent-hires"); + expect(mockAgentService.create).not.toHaveBeenCalled(); + expect(mockApprovalService.create).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + it("grants tasks:assign by default when board creates a new agent", async () => { const app = await createApp({ type: "board", diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 9d0eddc6a04..cad3130f603 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -1552,10 +1552,21 @@ export function agentRoutes(db: Db) { router.post("/companies/:companyId/agents", validate(createAgentSchema), async (req, res) => { const companyId = req.params.companyId as string; - assertCompanyAccess(req, companyId); + await assertCanCreateAgentsForCompany(req, companyId); - if (req.actor.type === "agent") { - assertBoard(req); + const company = await db + .select() + .from(companies) + .where(eq(companies.id, companyId)) + .then((rows) => rows[0] ?? null); + if (!company) { + res.status(404).json({ error: "Company not found" }); + return; + } + if (company.requireBoardApprovalForNewAgents) { + throw conflict( + "Direct agent creation requires board approval. Use POST /api/companies/:companyId/agent-hires to create a pending hire approval.", + ); } const { @@ -1563,6 +1574,14 @@ export function agentRoutes(db: Db) { ...createInput } = req.body; createInput.adapterType = assertKnownAdapterType(createInput.adapterType); + assertNoAgentHostWorkspaceCommandMutation( + req, + collectAgentAdapterWorkspaceCommandPaths(createInput.adapterConfig), + ); + assertNoAgentInstructionsConfigMutation( + req, + (createInput.adapterConfig ?? {}) as Record, + ); const requestedAdapterConfig = applyCreateDefaultsByAdapterType( createInput.adapterType, ((createInput.adapterConfig ?? {}) as Record),