mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
fix(routines): agents can only create authenticated webhook triggers (#21006)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Routines can have webhook triggers that outside services call > - Agents can create and change routine triggers through the API > - The choice of webhook authentication belongs to a person > - This pull request lets agents create authenticated webhook triggers only, and keeps every choice open to people on the board > - The benefit is that agents can only create authenticated webhook triggers ## Linked Issues or Issue Description **What happened** In a chat eval, an agent set up a routine webhook trigger with a weaker authentication choice than the default. **Expected behavior** Agents can only create authenticated webhook triggers. When a sender cannot authenticate, the agent asks a person, who decides on the board. **Steps to reproduce** 1. Ask an agent in chat to give a routine a webhook for a test service. 2. Read the trigger that it creates. ## What Changed - `server/src/services/routines.ts`: trigger creation, trigger updates and revision restores refuse an agent that does not keep the webhook trigger authenticated. The error tells the agent to use the default or a signed mode, or to ask a person. Board users keep every option. An agent can still edit other fields of a trigger that a person set up. - `skills/paperclip/references/routines.md`: says agents can only create authenticated webhook triggers. - Test: new case in `routines-service.test.ts`. ## Verification - `cd server && npx vitest run src/__tests__/routines-service.test.ts`: 104 passed. The new test fails without the service change (red) and passes with it (green). - `npx vitest run src/__tests__/routines-routes.test.ts src/__tests__/company-portability.test.ts`: passed. - The chat eval `routines.webhook-trigger` already requires one authenticated trigger. It runs again on a combined throwaway branch; results are in a PR comment. ## Risks - An agent-run company import that would add such a trigger now fails with the same error and rolls back. A person can run that import from the board. - Agents must use the default or a signed mode. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution in Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
5a7378af56
commit
1bb4dcbe4b
3 files changed
+48
-6
No files matched your search
@@ -251,6 +251,26 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => {
|
||||
expect(f.wakeups).toEqual([]);
|
||||
});
|
||||
|
||||
it("lets only a person choose an unauthenticated public webhook trigger", async () => {
|
||||
const f = await seedFixture();
|
||||
const webhook = { kind: "webhook", enabled: true, replayWindowSec: 300 } as const;
|
||||
const agent = { agentId: f.agentId };
|
||||
await expect(f.svc.createTrigger(f.routine.id, { ...webhook, signingMode: "none" }, agent))
|
||||
.rejects.toMatchObject({ status: 403, message: expect.stringContaining("Agents can only create authenticated webhook triggers") });
|
||||
const signed = await f.svc.createTrigger(f.routine.id, { ...webhook, signingMode: "bearer" }, agent);
|
||||
expect(signed.trigger.signingMode).toBe("bearer");
|
||||
await expect(f.svc.updateTrigger(signed.trigger.id, { signingMode: "none" }, agent)).rejects.toMatchObject({ status: 403 });
|
||||
const open = await f.svc.createTrigger(f.routine.id, { ...webhook, signingMode: "none" }, { userId: "board-user" });
|
||||
expect(open.trigger.signingMode).toBe("none");
|
||||
await expect(f.svc.updateTrigger(open.trigger.id, { label: "Inbound", signingMode: "none" }, agent))
|
||||
.resolves.toMatchObject({ trigger: { label: "Inbound", signingMode: "none" } });
|
||||
await f.svc.updateTrigger(signed.trigger.id, { signingMode: "none" }, { userId: "board-user" });
|
||||
const unsafeRevision = (await f.svc.listRevisions(f.routine.id))[0]!.id;
|
||||
await f.svc.updateTrigger(signed.trigger.id, { signingMode: "bearer" }, { userId: "board-user" });
|
||||
await expect(f.svc.restoreRevision(f.routine.id, unsafeRevision, agent)).rejects.toMatchObject({ status: 403 });
|
||||
expect((await f.svc.getTrigger(signed.trigger.id))!.signingMode).toBe("bearer");
|
||||
});
|
||||
|
||||
it("preserves creation ancestry through regular webhooks, child tasks, and standalone follow-ups", async () => {
|
||||
vi.stubEnv("PAPERCLIP_AGENT_JWT_SECRET", randomUUID().repeat(2));
|
||||
const f = await seedFixture();
|
||||
|
||||
@@ -163,6 +163,13 @@ type Actor = {
|
||||
actorResponsibleUserId?: string | null;
|
||||
runId?: string | null;
|
||||
};
|
||||
|
||||
export const AGENT_UNAUTHENTICATED_WEBHOOK_ERROR =
|
||||
"Agents can only create authenticated webhook triggers. Use signingMode \"bearer\" (the default) or a signed mode such as \"hmac_sha256\". If the sender cannot authenticate its requests, ask a person to choose an unauthenticated webhook on the board.";
|
||||
|
||||
function assertAgentWebhookAuthenticated(actor: Actor, signingMode: string | null | undefined) {
|
||||
if (actor.agentId && signingMode === "none") throw forbidden(AGENT_UNAUTHENTICATED_WEBHOOK_ERROR);
|
||||
}
|
||||
type RoutineRow = typeof routines.$inferSelect;
|
||||
type RoutineTriggerRow = typeof routineTriggers.$inferSelect;
|
||||
|
||||
@@ -2537,6 +2544,7 @@ export function routineService(
|
||||
}
|
||||
|
||||
if (input.kind === "webhook") {
|
||||
assertAgentWebhookAuthenticated(actor, input.signingMode);
|
||||
publicId = crypto.randomBytes(12).toString("hex");
|
||||
const created = await createWebhookSecret(routine.companyId, routine.id, actor);
|
||||
secretId = created.secret.id;
|
||||
@@ -2623,8 +2631,17 @@ export function routineService(
|
||||
|
||||
const result = await db.transaction(async (tx) => {
|
||||
const txDb = tx as unknown as Db;
|
||||
await notifyDeliveryWork(txDb, DELIVERY_QUEUES.routineSchedules);
|
||||
await tx.execute(sql`select id from ${routines} where ${routines.id} = ${existing.routineId} for update`);
|
||||
const current = await txDb
|
||||
.select({ signingMode: routineTriggers.signingMode })
|
||||
.from(routineTriggers)
|
||||
.where(eq(routineTriggers.id, id))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!current) return null;
|
||||
if (existing.kind === "webhook" && current.signingMode !== "none") {
|
||||
assertAgentWebhookAuthenticated(actor, patch.signingMode);
|
||||
}
|
||||
await notifyDeliveryWork(txDb, DELIVERY_QUEUES.routineSchedules);
|
||||
const [updated] = await txDb
|
||||
.update(routineTriggers)
|
||||
.set({
|
||||
@@ -2635,7 +2652,7 @@ export function routineService(
|
||||
cronExpression,
|
||||
timezone,
|
||||
nextRunAt,
|
||||
signingMode: patch.signingMode === undefined ? existing.signingMode : patch.signingMode,
|
||||
signingMode: patch.signingMode === undefined ? current.signingMode : patch.signingMode,
|
||||
replayWindowSec: patch.replayWindowSec === undefined ? existing.replayWindowSec : patch.replayWindowSec,
|
||||
updatedByAgentId: actor.agentId ?? null,
|
||||
updatedByUserId: actor.userId ?? null,
|
||||
@@ -2779,10 +2796,8 @@ export function routineService(
|
||||
const snapshot = routineRevisionSnapshotSchema.parse(targetRevision.snapshot) as RoutineRevisionSnapshotV1;
|
||||
const routineSnapshot = snapshot.routine;
|
||||
await assertRestorableAssignee(existingRoutine.companyId, routineSnapshot.assigneeAgentId, actor);
|
||||
|
||||
const result = await db.transaction(async (tx) => {
|
||||
const txDb = tx as unknown as Db;
|
||||
await notifyDeliveryWork(txDb, DELIVERY_QUEUES.routineSchedules);
|
||||
await tx.execute(sql`select id from ${routines} where ${routines.id} = ${existingRoutine.id} for update`);
|
||||
const locked = await txDb
|
||||
.select()
|
||||
@@ -2797,9 +2812,16 @@ export function routineService(
|
||||
}
|
||||
|
||||
const currentTriggers = await txDb
|
||||
.select({ id: routineTriggers.id })
|
||||
.select({ id: routineTriggers.id, signingMode: routineTriggers.signingMode })
|
||||
.from(routineTriggers)
|
||||
.where(and(eq(routineTriggers.companyId, locked.companyId), eq(routineTriggers.routineId, locked.id)));
|
||||
const unauthenticatedTriggerIds = new Set(currentTriggers.filter((trigger) => trigger.signingMode === "none").map((trigger) => trigger.id));
|
||||
for (const trigger of snapshot.triggers) {
|
||||
if (trigger.kind === "webhook" && !unauthenticatedTriggerIds.has(trigger.id)) {
|
||||
assertAgentWebhookAuthenticated(actor, trigger.signingMode);
|
||||
}
|
||||
}
|
||||
await notifyDeliveryWork(txDb, DELIVERY_QUEUES.routineSchedules);
|
||||
const currentTriggerIds = new Set(currentTriggers.map((trigger) => trigger.id));
|
||||
const missingWebhookTriggers = snapshot.triggers
|
||||
.filter((trigger) => trigger.kind === "webhook" && !currentTriggerIds.has(trigger.id));
|
||||
|
||||
@@ -157,7 +157,7 @@ POST /api/routines/{routineId}/triggers
|
||||
}
|
||||
```
|
||||
|
||||
- `signingMode`: `bearer` (default) or `hmac_sha256`
|
||||
- `signingMode`: `bearer` (default) or `hmac_sha256`. Agents can only create authenticated webhook triggers; if a sender cannot authenticate, ask a person to set that trigger up on the board.
|
||||
- `replayWindowSec`: 30-86400 (default 300)
|
||||
- Response includes the webhook URL (`publicId`-based) and the signing secret
|
||||
- Fire externally: `POST /api/routine-triggers/public/{publicId}/fire`
|
||||
|
||||
Reference in new issue
Block a user