mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 01:41:42 +02:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
5a8ee74616
commit
9adf4a0287
3 files changed
+136
-12
No files matched your search
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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<string, unknown>) {
|
||||
async function createApp(actor: Record<string, unknown>, dbOptions: { requireBoardApprovalForNewAgents?: boolean } = {}) {
|
||||
const [{ errorHandler }, { agentRoutes }] = await Promise.all([
|
||||
vi.importActual<typeof import("../middleware/index.js")>("../middleware/index.js"),
|
||||
vi.importActual<typeof import("../routes/agents.js")>("../routes/agents.js"),
|
||||
@@ -146,7 +148,7 @@ async function createApp(actor: Record<string, unknown>) {
|
||||
(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",
|
||||
|
||||
@@ -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<string, unknown>,
|
||||
);
|
||||
const requestedAdapterConfig = applyCreateDefaultsByAdapterType(
|
||||
createInput.adapterType,
|
||||
((createInput.adapterConfig ?? {}) as Record<string, unknown>),
|
||||
|
||||
Reference in new issue
Block a user