From ad454e62f5a395a0bda5c1df990fc9fc9b9f95bb Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 7 Jul 2026 21:40:17 +0000 Subject: [PATCH] Freeze pending approval agent config Co-Authored-By: Paperclip --- .../agents-pending-approval-config.test.ts | 158 ++++++++++++++++++ .../src/__tests__/approvals-service.test.ts | 2 +- server/src/__tests__/built-in-agents.test.ts | 20 +++ server/src/routes/agents.ts | 8 +- server/src/services/agents.ts | 79 ++++++++- server/src/services/approvals.ts | 2 +- 6 files changed, 263 insertions(+), 6 deletions(-) create mode 100644 server/src/__tests__/agents-pending-approval-config.test.ts diff --git a/server/src/__tests__/agents-pending-approval-config.test.ts b/server/src/__tests__/agents-pending-approval-config.test.ts new file mode 100644 index 00000000000..3bb906334a6 --- /dev/null +++ b/server/src/__tests__/agents-pending-approval-config.test.ts @@ -0,0 +1,158 @@ +import { randomUUID } from "node:crypto"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { eq } from "drizzle-orm"; +import { + agents, + approvals, + activityLog, + budgetPolicies, + companies, + createDb, +} from "@paperclipai/db"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; +import { agentService } from "../services/agents.ts"; +import { approvalService } from "../services/approvals.ts"; + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +function issuePrefix(id: string) { + return `T${id.replace(/-/g, "").slice(0, 6).toUpperCase()}`; +} + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping embedded Postgres pending approval agent tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`, + ); +} + +describeEmbeddedPostgres("pending approval agent config integrity", () => { + let db!: ReturnType; + let tempDb: Awaited> | null = null; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-pending-agent-config-"); + db = createDb(tempDb.connectionString); + }, 20_000); + + afterEach(async () => { + await db.delete(activityLog); + await db.delete(budgetPolicies); + await db.delete(approvals); + await db.delete(agents); + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + async function seedCompany() { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: issuePrefix(companyId), + requireBoardApprovalForNewAgents: true, + }); + return companyId; + } + + it("freezes generic pending hire config and reapplies the approval snapshot on activation", async () => { + const companyId = await seedCompany(); + const agentSvc = agentService(db); + const approvalSvc = approvalService(db); + const pending = await agentSvc.create(companyId, { + name: "Pending Coder", + role: "engineer", + title: "Software Engineer", + icon: "code", + capabilities: "Writes code", + adapterType: "process", + adapterConfig: { command: "echo safe" }, + runtimeConfig: { maxConcurrentRuns: 1 }, + budgetMonthlyCents: 1234, + metadata: { source: "hire-form" }, + status: "pending_approval", + spentMonthlyCents: 0, + permissions: {}, + lastHeartbeatAt: null, + }); + const approval = await approvalSvc.create(companyId, { + type: "hire_agent", + requestedByAgentId: null, + requestedByUserId: "board-user", + status: "pending", + payload: { + name: "Pending Coder", + role: "engineer", + title: "Software Engineer", + icon: "code", + reportsTo: null, + capabilities: "Writes code", + adapterType: "process", + adapterConfig: { command: "echo safe" }, + runtimeConfig: { maxConcurrentRuns: 1 }, + budgetMonthlyCents: 1234, + metadata: { source: "hire-form" }, + agentId: pending.id, + }, + decisionNote: null, + decidedByUserId: null, + decidedAt: null, + updatedAt: new Date(), + }); + + await expect(agentSvc.update(pending.id, { + name: "Tampered Coder", + adapterConfig: { command: "echo malicious" }, + runtimeConfig: { maxConcurrentRuns: 99 }, + })).rejects.toMatchObject({ + status: 409, + details: { + code: "pending_approval_agent_config_frozen", + agentId: pending.id, + fields: ["name", "adapterConfig", "runtimeConfig"], + }, + }); + await expect(agentSvc.updatePermissions(pending.id, { + canCreateAgents: true, + })).rejects.toMatchObject({ + status: 409, + details: { + code: "pending_approval_agent_config_frozen", + agentId: pending.id, + fields: ["permissions"], + }, + }); + + await db + .update(agents) + .set({ + name: "Tampered Coder", + adapterConfig: { command: "echo malicious" }, + runtimeConfig: { maxConcurrentRuns: 99 }, + metadata: { source: "tampered" }, + }) + .where(eq(agents.id, pending.id)); + + await approvalSvc.approve(approval.id, "board-user", "Approved generic hire"); + + await expect(agentSvc.getById(pending.id)).resolves.toMatchObject({ + status: "idle", + name: "Pending Coder", + role: "engineer", + title: "Software Engineer", + icon: "code", + capabilities: "Writes code", + adapterType: "process", + adapterConfig: { command: "echo safe" }, + runtimeConfig: { maxConcurrentRuns: 1 }, + budgetMonthlyCents: 1234, + metadata: { source: "hire-form" }, + }); + }); +}); diff --git a/server/src/__tests__/approvals-service.test.ts b/server/src/__tests__/approvals-service.test.ts index aebff09f59a..e3411d89dd2 100644 --- a/server/src/__tests__/approvals-service.test.ts +++ b/server/src/__tests__/approvals-service.test.ts @@ -101,7 +101,7 @@ describe("approvalService resolution idempotency", () => { const result = await svc.approve("approval-1", "board", "ship it"); expect(result.applied).toBe(true); - expect(mockAgentService.activatePendingApproval).toHaveBeenCalledWith("agent-1"); + expect(mockAgentService.activatePendingApproval).toHaveBeenCalledWith("agent-1", approved.payload); expect(mockNotifyHireApproved).toHaveBeenCalledTimes(1); }); diff --git a/server/src/__tests__/built-in-agents.test.ts b/server/src/__tests__/built-in-agents.test.ts index ed47f6c04a3..c925ac7ace4 100644 --- a/server/src/__tests__/built-in-agents.test.ts +++ b/server/src/__tests__/built-in-agents.test.ts @@ -180,6 +180,26 @@ describeEmbeddedPostgres("built-in agents", () => { details: { code: "built_in_agent_not_configured", status: "pending_approval" }, }); + await expect(agentService(db).update(result.state.agentId!, { + adapterType: "process", + adapterConfig: { command: "echo tampered" }, + })).rejects.toMatchObject({ + status: 409, + details: { + code: "pending_approval_agent_config_frozen", + agentId: result.state.agentId, + fields: ["adapterConfig"], + }, + }); + + await db + .update(agents) + .set({ + adapterType: "process", + adapterConfig: { command: "echo tampered" }, + }) + .where(eq(agents.id, result.state.agentId!)); + await approvalService(db).approve(result.approval!.id, "board-user", "Approved built-in agent"); await expect(builtIns.get(companyId, "briefs")).resolves.toMatchObject({ diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index d3bbdcee526..beb6c712e60 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -1271,7 +1271,9 @@ export function agentRoutes( delete nextAdapterConfig.bootstrapPromptTemplate; if (!hadLegacyPrompt) return agent; - const updated = await svc.update(agent.id, { adapterConfig: nextAdapterConfig }); + const updated = await svc.update(agent.id, { adapterConfig: nextAdapterConfig }, { + allowPendingApprovalConfigUpdate: true, + }); return (updated as T | null) ?? { ...agent, adapterConfig: nextAdapterConfig }; } @@ -1286,7 +1288,9 @@ export function agentRoutes( delete nextAdapterConfig.promptTemplate; delete nextAdapterConfig.bootstrapPromptTemplate; - const updated = await svc.update(agent.id, { adapterConfig: nextAdapterConfig }); + const updated = await svc.update(agent.id, { adapterConfig: nextAdapterConfig }, { + allowPendingApprovalConfigUpdate: true, + }); return (updated as T | null) ?? { ...agent, adapterConfig: nextAdapterConfig }; } diff --git a/server/src/services/agents.ts b/server/src/services/agents.ts index 44ce39c5760..72683199141 100644 --- a/server/src/services/agents.ts +++ b/server/src/services/agents.ts @@ -47,6 +47,7 @@ const CONFIG_REVISION_FIELDS = [ "name", "role", "title", + "icon", "reportsTo", "capabilities", "adapterType", @@ -70,6 +71,7 @@ interface RevisionMetadata { interface UpdateAgentOptions { recordRevision?: RevisionMetadata; allowBuiltInAgentMetadata?: boolean; + allowPendingApprovalConfigUpdate?: boolean; } interface CreateAgentOptions { @@ -113,6 +115,7 @@ function buildConfigSnapshot( name: row.name, role: row.role, title: row.title, + icon: row.icon, reportsTo: row.reportsTo, capabilities: row.capabilities, adapterType: row.adapterType, @@ -135,6 +138,47 @@ function hasConfigPatchFields(data: Partial) { return CONFIG_REVISION_FIELDS.some((field) => Object.prototype.hasOwnProperty.call(data, field)); } +function changedPendingApprovalConfigFields( + existing: typeof agents.$inferSelect, + data: Partial, +) { + return CONFIG_REVISION_FIELDS.filter((field) => + Object.prototype.hasOwnProperty.call(data, field) && !jsonEqual(data[field], existing[field]), + ); +} + +function configPatchFromApprovalPayload(payload: Record) { + const patch: Partial = {}; + if (typeof payload.name === "string") patch.name = payload.name; + if (typeof payload.role === "string") patch.role = payload.role; + if (Object.prototype.hasOwnProperty.call(payload, "title")) { + patch.title = typeof payload.title === "string" ? payload.title : null; + } + if (Object.prototype.hasOwnProperty.call(payload, "icon")) { + patch.icon = typeof payload.icon === "string" ? payload.icon : null; + } + if (Object.prototype.hasOwnProperty.call(payload, "reportsTo")) { + patch.reportsTo = typeof payload.reportsTo === "string" ? payload.reportsTo : null; + } + if (Object.prototype.hasOwnProperty.call(payload, "capabilities")) { + patch.capabilities = typeof payload.capabilities === "string" ? payload.capabilities : null; + } + if (typeof payload.adapterType === "string") patch.adapterType = payload.adapterType; + if (isPlainRecord(payload.adapterConfig)) patch.adapterConfig = payload.adapterConfig; + if (isPlainRecord(payload.runtimeConfig)) patch.runtimeConfig = payload.runtimeConfig; + if (Object.prototype.hasOwnProperty.call(payload, "defaultEnvironmentId")) { + patch.defaultEnvironmentId = + typeof payload.defaultEnvironmentId === "string" ? payload.defaultEnvironmentId : null; + } + if (typeof payload.budgetMonthlyCents === "number") { + patch.budgetMonthlyCents = payload.budgetMonthlyCents; + } + if (Object.prototype.hasOwnProperty.call(payload, "metadata")) { + patch.metadata = isPlainRecord(payload.metadata) ? payload.metadata : null; + } + return patch; +} + function parseFiniteNumberLike(value: unknown): number | null { if (typeof value === "number" && Number.isFinite(value)) return value; if (typeof value !== "string") return null; @@ -433,6 +477,16 @@ export function agentService(db: Db) { ) { throw conflict("Pending approval agents cannot be activated directly"); } + if (existing.status === "pending_approval" && !options?.allowPendingApprovalConfigUpdate) { + const changedFields = changedPendingApprovalConfigFields(existing as typeof agents.$inferSelect, data); + if (changedFields.length > 0) { + throw conflict("Pending approval agent configuration cannot be changed before board approval", { + code: "pending_approval_agent_config_frozen", + agentId: id, + fields: changedFields, + }); + } + } if (data.reportsTo !== undefined) { if (data.reportsTo) { @@ -712,12 +766,26 @@ export function agentService(db: Db) { }); }, - activatePendingApproval: async (id: string) => { + activatePendingApproval: async (id: string, approvedPayload?: Record | null) => { const activatedAgent = await db.transaction(async (tx) => { const txDb = tx as unknown as Db; + const existing = await agentService(txDb).getById(id); + if (!existing || existing.status !== "pending_approval") return null; + const approvedPatch = approvedPayload ? configPatchFromApprovalPayload(approvedPayload) : {}; + let patch = { ...approvedPatch } as Partial; + if ( + Object.prototype.hasOwnProperty.call(patch, "adapterConfig") && + isPlainRecord(patch.adapterConfig) + ) { + patch.adapterConfig = await secretService(txDb).normalizeAdapterConfigForPersistence( + existing.companyId, + patch.adapterConfig, + { adapterType: (patch.adapterType ?? existing.adapterType) as string }, + ); + } const updated = await tx .update(agents) - .set({ status: "idle", updatedAt: new Date() }) + .set({ ...patch, status: "idle", updatedAt: new Date() }) .where(and(eq(agents.id, id), eq(agents.status, "pending_approval"))) .returning() .then((rows) => rows[0] ?? null); @@ -741,6 +809,13 @@ export function agentService(db: Db) { updatePermissions: async (id: string, permissions: Record & { canCreateAgents: boolean }) => { const existing = await getById(id); if (!existing) return null; + if (existing.status === "pending_approval") { + throw conflict("Pending approval agent permissions cannot be changed before board approval", { + code: "pending_approval_agent_config_frozen", + agentId: id, + fields: ["permissions"], + }); + } const updated = await db .update(agents) diff --git a/server/src/services/approvals.ts b/server/src/services/approvals.ts index 5917270cad7..832f0dce7e0 100644 --- a/server/src/services/approvals.ts +++ b/server/src/services/approvals.ts @@ -128,7 +128,7 @@ export function approvalService(db: Db) { const payload = updated.payload as Record; const payloadAgentId = typeof payload.agentId === "string" ? payload.agentId : null; if (payloadAgentId) { - await agentsSvc.activatePendingApproval(payloadAgentId); + await agentsSvc.activatePendingApproval(payloadAgentId, payload); hireApprovedAgentId = payloadAgentId; } else { const created = await agentsSvc.create(updated.companyId, {