mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix: require board authority for agent budget policies
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
0cb3fc7db5
commit
f970f6b01f
6 files changed
+95
-14
No files matched your search
@@ -54,7 +54,7 @@ An unfinished record keeps compatibility status `paused` with a
|
|||||||
`setup` hold and cannot receive API keys or authenticate with keys or JWTs.
|
`setup` hold and cannot receive API keys or authenticate with keys or JWTs.
|
||||||
Its standard grants apply at hire. Authorized completion retries recover the
|
Its standard grants apply at hire. Authorized completion retries recover the
|
||||||
same record and approval without changing the submitted configuration.
|
same record and approval without changing the submitted configuration.
|
||||||
Normal agent-authored hiring keeps its existing approval policy.
|
Normal agent-authored hiring keeps its existing approval policy. A pending hire may propose a budget, but only board approval activates that policy. Direct creation or an immediate hire with a positive budget requires a board actor.
|
||||||
|
|
||||||
An agent-owned plugin may prepare the draft and leave its lifecycle in
|
An agent-owned plugin may prepare the draft and leave its lifecycle in
|
||||||
`verifying` while that setup hold remains. Only bounded Core-issued setup probes
|
`verifying` while that setup hold remains. Only bounded Core-issued setup probes
|
||||||
@@ -132,6 +132,8 @@ CLI.
|
|||||||
Policy: Agent creation/configuration uses action grants, resource policies and change
|
Policy: Agent creation/configuration uses action grants, resource policies and change
|
||||||
consent; some controls require a board actor.
|
consent; some controls require a board actor.
|
||||||
|
|
||||||
|
The legacy `budgetMonthlyCents` field requires a board actor for updates or budget-changing rollbacks. Agent-authored rollbacks with an unchanged amount do not write budget policy or policy audit. Board mutations retain the actual operator identity.
|
||||||
|
|
||||||
The roster intent preserves unrelated settings, rejects stale model/connection
|
The roster intent preserves unrelated settings, rejects stale model/connection
|
||||||
previews and harness changes, and checks the full execution configuration under
|
previews and harness changes, and checks the full execution configuration under
|
||||||
the lifecycle write lock. It uses the same configuration, company, credential,
|
the lifecycle write lock. It uses the same configuration, company, credential,
|
||||||
|
|||||||
@@ -0,0 +1,19 @@
|
|||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import type { Db } from "@paperclipai/db";
|
||||||
|
import type { AgentConfigurationRecord } from "../lib/agent-records.js";
|
||||||
|
import { prepareAgentConfiguration } from "../services/agent-configuration.js";
|
||||||
|
|
||||||
|
const existing = { id: "agent", companyId: "company", status: "idle", budgetMonthlyCents: 500 } as AgentConfigurationRecord;
|
||||||
|
describe("budget authority in configuration preparation", () => {
|
||||||
|
it("rejects agent-originated budget changes before persistence or policy audit", async () => {
|
||||||
|
const db = {} as Db;
|
||||||
|
await expect(prepareAgentConfiguration(db, existing, { budgetMonthlyCents: 5000 }, { recordRevision: { createdByAgentId: "ceo" } })).rejects.toMatchObject({ status: 403 });
|
||||||
|
await expect(prepareAgentConfiguration(db, existing, { budgetMonthlyCents: 0 }, { recordRevision: { createdByAgentId: "ceo" } })).rejects.toMatchObject({ status: 403 });
|
||||||
|
});
|
||||||
|
it("removes an unchanged legacy budget from agent rollback so it cannot rewrite policy or invent a board actor", async () => {
|
||||||
|
expect(await prepareAgentConfiguration({} as Db, existing, { budgetMonthlyCents: 500 }, { recordRevision: { createdByAgentId: "ceo", source: "rollback" } })).toEqual({});
|
||||||
|
});
|
||||||
|
it("preserves the actual board-authorized amount for the existing policy writer", async () => {
|
||||||
|
expect(await prepareAgentConfiguration({} as Db, existing, { budgetMonthlyCents: 5000 }, { recordRevision: { createdByUserId: "operator-user" } })).toEqual({ budgetMonthlyCents: 5000 });
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -185,11 +185,11 @@ describeEmbeddedPostgres("agent hire idempotency within a run", () => {
|
|||||||
// Same identity, corrected configuration: the agent meant a different hire.
|
// Same identity, corrected configuration: the agent meant a different hire.
|
||||||
const corrected = await request(app)
|
const corrected = await request(app)
|
||||||
.post(`/api/companies/${company.id}/agent-hires`)
|
.post(`/api/companies/${company.id}/agent-hires`)
|
||||||
.send({ name: "Sam", role: "engineer", adapterType: "process", budgetMonthlyCents: 5000 });
|
.send({ name: "Sam", role: "engineer", adapterType: "process", title: "Store Builder" });
|
||||||
expect(corrected.status, JSON.stringify(corrected.body)).toBe(201);
|
expect(corrected.status, JSON.stringify(corrected.body)).toBe(201);
|
||||||
expect(corrected.body.idempotent).toBeUndefined();
|
expect(corrected.body.idempotent).toBeUndefined();
|
||||||
expect(corrected.body.agent?.id).not.toBe(first.body.agent?.id);
|
expect(corrected.body.agent?.id).not.toBe(first.body.agent?.id);
|
||||||
expect(corrected.body.agent?.budgetMonthlyCents).toBe(5000);
|
expect(corrected.body.agent?.title).toBe("Store Builder");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("still creates a distinct agent for a different hire in the same run", async () => {
|
it("still creates a distinct agent for a different hire in the same run", async () => {
|
||||||
|
|||||||
@@ -2343,6 +2343,48 @@ describe("agent permission routes", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it.each([0, 5000])("budget authority: rejects agent policy changes through the legacy field (%i)", async amount => {
|
||||||
|
const app = createApp({ type: "agent", agentId, companyId, source: "agent_key", runId: "run-1" });
|
||||||
|
const response = await requestApp(app, url => request(url).patch(`/api/agents/${agentId}`).send({ budgetMonthlyCents: amount }));
|
||||||
|
expect(response.status).toBe(403);
|
||||||
|
expect(mockAgentService.update).not.toHaveBeenCalled();
|
||||||
|
expect(mockBudgetService.upsertPolicy).not.toHaveBeenCalled();
|
||||||
|
expect(mockLogActivity).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
it.each(["agents", "agent-hires"])("budget authority: rejects agent immediate creation budgets through %s", async route => {
|
||||||
|
mockAccessService.hasPermission.mockResolvedValue(true);
|
||||||
|
const app = createApp({ type: "agent", agentId, companyId, source: "agent_key" });
|
||||||
|
const response = await requestApp(app, url => request(url).post(`/api/companies/${companyId}/${route}`).send({ name: "New employee", role: "engineer", adapterType: "process", budgetMonthlyCents: 5000 }));
|
||||||
|
expect(response.status).toBe(403);
|
||||||
|
expect(mockAgentService.create).not.toHaveBeenCalled();
|
||||||
|
expect(mockBudgetService.upsertPolicy).not.toHaveBeenCalled();
|
||||||
|
expect(mockLogActivity).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
it("budget authority: preserves a proposed budget without applying a policy before approval", async () => {
|
||||||
|
mockAccessService.hasPermission.mockResolvedValue(true);
|
||||||
|
mockAgentService.create.mockResolvedValue({ ...baseAgent, status: "pending_approval", budgetMonthlyCents: 5000 });
|
||||||
|
mockApprovalService.create.mockResolvedValue({ id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", status: "pending", type: "hire_agent", companyId });
|
||||||
|
const app = createApp({ type: "agent", agentId, companyId, source: "agent_key" }, { requireBoardApprovalForNewAgents: true });
|
||||||
|
const response = await requestApp(app, url => request(url).post(`/api/companies/${companyId}/agent-hires`).send({ name: "Proposed employee", role: "engineer", adapterType: "process", budgetMonthlyCents: 5000 }));
|
||||||
|
expect(response.status).toBe(201);
|
||||||
|
expect(response.body.approval.status).toBe("pending");
|
||||||
|
expect(mockApprovalService.create).toHaveBeenCalledWith(companyId, expect.objectContaining({ payload: expect.objectContaining({ budgetMonthlyCents: 5000 }) }));
|
||||||
|
expect(mockBudgetService.upsertPolicy).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
it("budget authority: forwards the actual board identity on legacy updates", async () => {
|
||||||
|
const app = createApp({ type: "board", userId: "operator-user", source: "session", companyIds: [companyId] });
|
||||||
|
const response = await requestApp(app, url => request(url).patch(`/api/agents/${agentId}`).send({ budgetMonthlyCents: 5000 }));
|
||||||
|
expect(response.status).toBe(200);
|
||||||
|
expect(mockAgentService.update).toHaveBeenCalledWith(agentId, expect.objectContaining({ budgetMonthlyCents: 5000 }), expect.objectContaining({ recordRevision: expect.objectContaining({ createdByUserId: "operator-user", createdByAgentId: null }) }));
|
||||||
|
expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ actorType: "user", actorId: "operator-user", agentId: null }));
|
||||||
|
});
|
||||||
|
it.each(["agents", "agent-hires"])("budget authority: records the real board actor for immediate %s budgets", async route => {
|
||||||
|
mockAgentService.create.mockResolvedValue({ ...baseAgent, budgetMonthlyCents: 5000 });
|
||||||
|
const app = createApp({ type: "board", userId: "operator-user", source: "local_implicit", companyIds: [companyId] });
|
||||||
|
const response = await requestApp(app, url => request(url).post(`/api/companies/${companyId}/${route}`).send({ name: "New employee", role: "engineer", adapterType: "process", budgetMonthlyCents: 5000 }));
|
||||||
|
expect(response.status).toBe(201);
|
||||||
|
expect(mockBudgetService.upsertPolicy).toHaveBeenCalledWith(companyId, expect.objectContaining({ scopeType: "agent", scopeId: agentId, amount: 5000 }), "operator-user");
|
||||||
|
});
|
||||||
it("rejects heartbeat cancellation outside the caller company scope", async () => {
|
it("rejects heartbeat cancellation outside the caller company scope", async () => {
|
||||||
mockHeartbeatService.getRun.mockResolvedValue({
|
mockHeartbeatService.getRun.mockResolvedValue({
|
||||||
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
|
||||||
|
|||||||
+24
-10
@@ -4206,6 +4206,9 @@ export function agentRoutes(
|
|||||||
if (!rollbackConfig) {
|
if (!rollbackConfig) {
|
||||||
throw unprocessable("Invalid revision snapshot");
|
throw unprocessable("Invalid revision snapshot");
|
||||||
}
|
}
|
||||||
|
if (typeof rollbackConfig.budgetMonthlyCents === "number" && rollbackConfig.budgetMonthlyCents !== existing.budgetMonthlyCents) {
|
||||||
|
assertBoard(req);
|
||||||
|
}
|
||||||
assertProviderTraceSettingTransition(
|
assertProviderTraceSettingTransition(
|
||||||
req,
|
req,
|
||||||
rollbackConfig.runtimeConfig,
|
rollbackConfig.runtimeConfig,
|
||||||
@@ -4338,6 +4341,17 @@ export function agentRoutes(
|
|||||||
const handleAgentHire = async (req: Request, res: Response) => {
|
const handleAgentHire = async (req: Request, res: Response) => {
|
||||||
const companyId = req.params.companyId as string;
|
const companyId = req.params.companyId as string;
|
||||||
await assertCanCreateAgentsForCompany(req, companyId);
|
await assertCanCreateAgentsForCompany(req, companyId);
|
||||||
|
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 && req.body.budgetMonthlyCents > 0) assertBoard(req);
|
||||||
const sourceIssueIds = parseSourceIssueIds(req.body);
|
const sourceIssueIds = parseSourceIssueIds(req.body);
|
||||||
const {
|
const {
|
||||||
desiredSkills: requestedDesiredSkills,
|
desiredSkills: requestedDesiredSkills,
|
||||||
@@ -4501,16 +4515,6 @@ export function agentRoutes(
|
|||||||
allowedSandboxProviders: allowedSandboxProvidersForAgent(hireInput.adapterType),
|
allowedSandboxProviders: allowedSandboxProvidersForAgent(hireInput.adapterType),
|
||||||
});
|
});
|
||||||
|
|
||||||
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;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Idempotency within a run: if this run already created a hire from this
|
// Idempotency within a run: if this run already created a hire from this
|
||||||
// exact request, return that hire instead of creating a duplicate. The
|
// exact request, return that hire instead of creating a duplicate. The
|
||||||
// creating agent cannot pause or delete its own hire (board-only), so a
|
// creating agent cannot pause or delete its own hire (board-only), so a
|
||||||
@@ -4597,6 +4601,14 @@ export function agentRoutes(
|
|||||||
|
|
||||||
let approval: Awaited<ReturnType<typeof approvalsSvc.getById>> | null = null;
|
let approval: Awaited<ReturnType<typeof approvalsSvc.getById>> | null = null;
|
||||||
const actor = getActorInfo(req);
|
const actor = getActorInfo(req);
|
||||||
|
if (!requiresApproval && agent.budgetMonthlyCents > 0) {
|
||||||
|
await budgets.upsertPolicy(companyId, {
|
||||||
|
scopeType: "agent",
|
||||||
|
scopeId: agent.id,
|
||||||
|
amount: agent.budgetMonthlyCents,
|
||||||
|
windowKind: "calendar_month_utc",
|
||||||
|
}, actor.actorId);
|
||||||
|
}
|
||||||
|
|
||||||
if (requiresApproval) {
|
if (requiresApproval) {
|
||||||
const requestedAdapterType = normalizedHireInput.adapterType ?? agent.adapterType;
|
const requestedAdapterType = normalizedHireInput.adapterType ?? agent.adapterType;
|
||||||
@@ -4722,6 +4734,7 @@ export function agentRoutes(
|
|||||||
router.post("/companies/:companyId/agents", validate(createAgentSchema), async (req, res) => {
|
router.post("/companies/:companyId/agents", validate(createAgentSchema), async (req, res) => {
|
||||||
const companyId = req.params.companyId as string;
|
const companyId = req.params.companyId as string;
|
||||||
await assertCanCreateAgentsForCompany(req, companyId);
|
await assertCanCreateAgentsForCompany(req, companyId);
|
||||||
|
if (req.body.budgetMonthlyCents > 0) assertBoard(req);
|
||||||
|
|
||||||
const company = await db
|
const company = await db
|
||||||
.select()
|
.select()
|
||||||
@@ -5390,6 +5403,7 @@ export function agentRoutes(
|
|||||||
const id = req.params.id as string;
|
const id = req.params.id as string;
|
||||||
const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found");
|
const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found");
|
||||||
if (!existing) return;
|
if (!existing) return;
|
||||||
|
if (hasOwn(req.body as object, "budgetMonthlyCents")) assertBoard(req);
|
||||||
|
|
||||||
if (hasOwn(req.body as object, "permissions")) {
|
if (hasOwn(req.body as object, "permissions")) {
|
||||||
res.status(422).json({ error: "Use /api/agents/:id/permissions for permission changes" });
|
res.status(422).json({ error: "Use /api/agents/:id/permissions for permission changes" });
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ import { and, eq } from "drizzle-orm";
|
|||||||
import { assets, agentConfigRevisions, agentTaskSessions, agentRuntimeState, type Db } from "@paperclipai/db";
|
import { assets, agentConfigRevisions, agentTaskSessions, agentRuntimeState, type Db } from "@paperclipai/db";
|
||||||
import { normalizeAgentUrlKey } from "@paperclipai/shared";
|
import { normalizeAgentUrlKey } from "@paperclipai/shared";
|
||||||
import { normalizePaperclipRunnerAdapterConfig } from "@paperclipai/adapter-utils/server-utils";
|
import { normalizePaperclipRunnerAdapterConfig } from "@paperclipai/adapter-utils/server-utils";
|
||||||
import { conflict, notFound, unprocessable } from "../errors.js";
|
import { conflict, forbidden, notFound, unprocessable } from "../errors.js";
|
||||||
import { agentRecordQueries, type UpdateAgentOptions, type AgentConfigurationPatch, type AgentConfigurationRecord, isPlainRecord, buildConfigSnapshot,
|
import { agentRecordQueries, type UpdateAgentOptions, type AgentConfigurationPatch, type AgentConfigurationRecord, isPlainRecord, buildConfigSnapshot,
|
||||||
hasConfigPatchFields, changedPendingApprovalConfigFields, diffConfigSnapshot } from "../lib/agent-records.js";
|
hasConfigPatchFields, changedPendingApprovalConfigFields, diffConfigSnapshot } from "../lib/agent-records.js";
|
||||||
import { normalizeAgentPermissions } from "../lib/agent-permissions.js";
|
import { normalizeAgentPermissions } from "../lib/agent-permissions.js";
|
||||||
@@ -13,6 +13,9 @@ import { clearPrimaryAgent } from "./primary-agent.js";
|
|||||||
import { budgetServiceInTransaction } from "./budgets.js";
|
import { budgetServiceInTransaction } from "./budgets.js";
|
||||||
|
|
||||||
export async function prepareAgentConfiguration(db: Db, existing: AgentConfigurationRecord, data: AgentConfigurationPatch, options?: UpdateAgentOptions): Promise<AgentConfigurationPatch> {
|
export async function prepareAgentConfiguration(db: Db, existing: AgentConfigurationRecord, data: AgentConfigurationPatch, options?: UpdateAgentOptions): Promise<AgentConfigurationPatch> {
|
||||||
|
if (options?.recordRevision?.createdByAgentId && data.budgetMonthlyCents !== undefined && data.budgetMonthlyCents !== existing.budgetMonthlyCents) {
|
||||||
|
throw forbidden("Only board operators can change agent budgets");
|
||||||
|
}
|
||||||
const id = existing.id;
|
const id = existing.id;
|
||||||
const { ensureManager, assertNoCycle, assertCompanyShortnameAvailable, assertBuiltInAgentMetadataMutationAllowed } = agentRecordQueries(db);
|
const { ensureManager, assertNoCycle, assertCompanyShortnameAvailable, assertBuiltInAgentMetadataMutationAllowed } = agentRecordQueries(db);
|
||||||
if (existing.status === "pending_approval" && !options?.allowPendingApprovalConfigUpdate) {
|
if (existing.status === "pending_approval" && !options?.allowPendingApprovalConfigUpdate) {
|
||||||
@@ -55,6 +58,7 @@ export async function prepareAgentConfiguration(db: Db, existing: AgentConfigura
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
const normalizedPatch = { ...data } as AgentConfigurationPatch;
|
const normalizedPatch = { ...data } as AgentConfigurationPatch;
|
||||||
|
if (options?.recordRevision?.createdByAgentId) delete normalizedPatch.budgetMonthlyCents;
|
||||||
if (data.permissions !== undefined) {
|
if (data.permissions !== undefined) {
|
||||||
normalizedPatch.permissions = normalizeAgentPermissions(data.permissions);
|
normalizedPatch.permissions = normalizeAgentPermissions(data.permissions);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user