mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 20:46:28 +01:00
fix(routines): retain authenticated agent responsibility on creation (#19222)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Routines save work for later execution under a responsible person's authority. > - An agent can create a routine through its normal REST or native API tools. > - The create route kept the agent creator but dropped the authenticated responsible user. > - The service then selected a parent task's user or a company default, even when the source run had a different user. > - This pull request passes the authenticated responsible user separately from creator attribution. > - Future automatic runs then use the authenticated responsible user saved in the routine revision. ## Linked Issues or Issue Description **What happened?** An authenticated native agent created a routine without a parent task. The stored responsible user became a different company owner. An unrelated parent task could also replace the authenticated user. The defect changes the identity saved for future automatic work. **Expected behavior** Creation must use the server-resolved responsible user from the authenticated run or API key. The agent remains the creator. A person who clicks Run now still supplies that manual run's identity. **Steps to reproduce** 1. Create an active company member and a different company owner. 2. Initialize a native agent run with the member's identity. 3. Use `call_api` to create a paused, self-assigned routine without a parent. 4. Read the routine and its first revision. Before this fix, both use the company owner. **Paperclip version or commit** Reproduced on master `56517dda6c5b32bc901b7dbf1605eaaa041c5fba`. **Deployment mode** Local source integration with real HTTP routes, agent authentication, native tool authority, and embedded PostgreSQL. No provider turn is needed for the reproduction. Related work: [#19128](https://github.com/paperclipai/paperclip/pull/19128) adds board controls for saved responsibility. [#15207](https://github.com/paperclipai/paperclip/pull/15207) repairs synthetic identities from built-in installers. Neither fixes this authenticated create path. ## What Changed - Pass the authenticated agent's responsible user as a separate internal actor field. - Select the board user first, then the agent’s server-resolved responsible user, before the existing parent and company fallback. - Keep creator attribution, initial revision attribution, manual dispatch identity, and existing records intact. - Add real native API, HTTP, and PostgreSQL regressions for identity selection, replay, client spoofing, stale claims, revoked membership, cancellation, cross-company calls, API keys, and fallback behavior. - Document the creation and manual dispatch identity rules in the execution contract. Correct the obsolete responsibility sentence in the agent access map. - Make the existing live-retry route fixture wait for its committed comment’s durable wake before draining. Keep all process and saved-owner assertions. ## Verification - Captured a failing native `call_api` test before changing production code. The persisted responsible user was the unrelated company owner. - The same real route and database boundary passes after the fix. - Initial affected routine and identity checks: 131 passed. The existing pending-AI-card fence also passed. - Follow-up routine checks: 118 passed. The owning scheduled-retry suite passed all 31 tests. - Initial CI serialized shard 7/9 failed with a `23503` wake-to-agent foreign key error during test cleanup. A controlled real-route/PostgreSQL pause reproduced that exact error on unchanged base `56517dda6c`. The drain returned before the route registered its deferred comment wake. The same pause passed after the fixture waited for the actual commentId’s durable wake receipt. The original failure is preserved. - Affected server typecheck after the fixture query change: passed. - `pnpm -r typecheck`: passed. - `pnpm build`: passed. - `pnpm test:run`: still running as supporting coverage. The worktree changed during the run: map cleanup, then the six-line fixture wait. Production and routine-test files stayed unchanged. This is mixed-working-tree coverage, not an immutable full-suite pass for a commit. Exact next-head CI remains required. - `node scripts/check-maps.mjs` and `git diff --check`: passed. - Exact head `a12e3f6c42b8ad5826fd58faa3cd9553ea251e09`: all CI gates passed. Greptile and the native substantive review both gave 5/5 with no open findings. The native reviewer did not run tests or exercise shadow-mode authorization. - Live provider execution remains unverified. This change does not grant access to another person's AI account. ## Risks New agent-created routines now use their authenticated responsible user before a parent or company default. This is the intended behavior change. The service keeps its fallback when no bound identity exists. Missing agent-key identity still fails authentication. Missing personal AI configuration still blocks execution. Existing routines, revisions, tasks, grants, and manual dispatch behavior do not change. The fixture waits for a persisted wake record before draining; its original live-process and saved-owner assertions remain. ## Model Used OpenAI `gpt-6.1-sol` with reasoning, tool use, and code execution. The host did not expose the context window size. ## 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 - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
3a090b815b
commit
47027e7ba5
7 files changed
+139
-6
No files matched your search
@@ -2878,6 +2878,14 @@ or grant access to the other task. An explicit pending workspace choice takes pr
|
||||
|
||||
### Operator identity and permission for manual dispatch
|
||||
|
||||
Routine creation keeps the authenticated responsible user separate from creator
|
||||
attribution. An agent-created routine uses the server-resolved on-behalf-of user
|
||||
from its run or API key; the agent remains the creator. Without a bound identity,
|
||||
the service retains its parent-task and company-default fallback. Automatic
|
||||
routine executions use the saved revision's responsible user. A person clicking
|
||||
Run now supplies a new execution identity, so that manual task uses the clicking
|
||||
person's authority and AI account selection rather than the routine creator's.
|
||||
|
||||
A legacy queued-message Interrupt is a new instruction from the user who clicks
|
||||
it. The new run uses that user's execution identity, including when someone else
|
||||
wrote the queued messages. Message bodies and historical authors stay unchanged.
|
||||
|
||||
@@ -39,8 +39,8 @@ Assert the relevant denied actor, owner, company or mode from Policy. Review the
|
||||
remaining operations before qualifying the whole capability.
|
||||
|
||||
Gaps: No dedicated production manage_routine binding; generic `call_api` provides
|
||||
the permitted REST path. Creation needs a responsible user resolved from the parent
|
||||
issue or company default.
|
||||
the permitted REST path. Creation resolves responsibility from the authenticated
|
||||
agent's on-behalf-of user, then the parent issue or company default.
|
||||
|
||||
## `triggers`
|
||||
|
||||
|
||||
@@ -689,6 +689,12 @@ describeEmbeddedPostgres("issue scheduled retry routes", () => {
|
||||
: await request(app).patch(`/api/issues/${f.issueId}`).send({ comment: "Verified source recovery; continue the original work." });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(method === "post" ? 201 : 200);
|
||||
const [comment] = await db.select({ id: issueComments.id }).from(issueComments)
|
||||
.where(eq(issueComments.issueId, f.issueId));
|
||||
expect(comment).toBeDefined();
|
||||
await expect.poll(async () => db.select({ id: agentWakeupRequests.id }).from(agentWakeupRequests)
|
||||
.where(and(eq(agentWakeupRequests.companyId, f.companyId),
|
||||
sql`${agentWakeupRequests.payload}->>'commentId' = ${comment!.id}`))).toHaveLength(1);
|
||||
await heartbeat.drainActiveRunExecutions();
|
||||
const progress = once(child, "message", { signal: AbortSignal.timeout(5_000) });
|
||||
child.send("continue-work");
|
||||
|
||||
@@ -33,7 +33,7 @@ import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { envConfigSchema, type Routine } from "@paperclipai/shared";
|
||||
import { createRoutineSchema, envConfigSchema, type Routine } from "@paperclipai/shared";
|
||||
import { issueService } from "../services/issues.ts";
|
||||
import { instanceSettingsService } from "../services/instance-settings.ts";
|
||||
import * as providerRegistry from "../secrets/provider-registry.ts";
|
||||
@@ -1602,6 +1602,30 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it.each(["parent", "company", "missing", "board"] as const)(
|
||||
"preserves routine creation responsibility fallback: %s", async source => {
|
||||
const { companyId, agentId, routine, svc } = await seedFixture();
|
||||
const parentUserId = randomUUID();
|
||||
const boardUserId = randomUUID();
|
||||
const [parent] = await db.insert(issues).values({ companyId, title: "Parent context", status: "backlog",
|
||||
responsibleUserId: parentUserId }).returning();
|
||||
if (source === "missing") await db.update(companies).set({ defaultResponsibleUserId: null }).where(eq(companies.id, companyId));
|
||||
const creation = svc.create(companyId, createRoutineSchema.parse({
|
||||
title: "Responsibility fallback", assigneeAgentId: agentId, status: "paused",
|
||||
...(source === "parent" ? { parentIssueId: parent.id } : {}),
|
||||
}), source === "board" ? { userId: boardUserId, actorResponsibleUserId: "other-user" } : { agentId });
|
||||
if (source === "missing") {
|
||||
await expect(creation).rejects.toMatchObject({ status: 422 });
|
||||
} else {
|
||||
expect(await creation).toMatchObject({
|
||||
responsibleUserId: source === "board" ? boardUserId : source === "parent" ? parentUserId : routine.responsibleUserId,
|
||||
createdByAgentId: source === "board" ? null : agentId,
|
||||
createdByUserId: source === "board" ? boardUserId : null,
|
||||
});
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("retains manual runner attribution without adding fresh routine issues to Mine", async () => {
|
||||
const { companyId, agentId, issueSvc, routine, svc } = await seedFixture();
|
||||
const userId = randomUUID();
|
||||
|
||||
@@ -161,6 +161,7 @@ export function routineRoutes(
|
||||
const created = await svc.create(companyId, req.body, {
|
||||
agentId: req.actor.type === "agent" ? req.actor.agentId : null,
|
||||
userId: req.actor.type === "board" ? req.actor.userId ?? "board" : null,
|
||||
...(req.actor.type === "agent" ? { actorResponsibleUserId: req.actor.onBehalfOfUserId ?? null } : {}),
|
||||
runId: req.actor.runId ?? null,
|
||||
});
|
||||
const actor = getActorInfo(req);
|
||||
|
||||
@@ -3,7 +3,7 @@ import { createHash, randomUUID } from "node:crypto";
|
||||
import { chmod, writeFile, symlink, mkdir, open } from "node:fs/promises";
|
||||
import { join, dirname } from "node:path";
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import { agents, agentWakeupRequests, assets, documents, heartbeatRuns, issues, projects, routineDocuments, routines, runnerApiResponseReservations, companySkills, companySkillVersions, companySkillTestRuns, companyMemberships, principalPermissionGrants } from "@paperclipai/db";
|
||||
import { agents, agentApiKeys, agentWakeupRequests, assets, authUsers, documents, heartbeatRuns, issues, projects, routineDocuments, routineRevisions, routines, runnerApiResponseReservations, companySkills, companySkillVersions, companySkillTestRuns, companyMemberships, principalPermissionGrants } from "@paperclipai/db";
|
||||
import { initializeRunIdentity } from "../run-identity.js";
|
||||
import { beforeAll, afterAll, beforeEach, afterEach, describe, expect, it, vi } from "vitest";
|
||||
import * as nativeSetupReadiness from "./setup-readiness.js";
|
||||
@@ -243,6 +243,90 @@ else if(m.id!==undefined) send({id:m.id,result:{}});
|
||||
.toMatchObject({ status: "blocked", billingCode: "REST-PARITY" });
|
||||
});
|
||||
|
||||
it.each(["company owner", "unrelated parent", "no fallback"])(
|
||||
"keeps the authenticated native run responsible user over %s", async fallback => {
|
||||
const fixture = await server.fixture({ connectionScenario: "fresh", disableWakeOnDemand: true });
|
||||
const ownerUserId = randomUUID();
|
||||
if (fallback !== "no fallback") {
|
||||
await server.db.insert(authUsers).values({ id: ownerUserId, name: "Company owner", email: `${ownerUserId}@fixture.invalid`,
|
||||
emailVerified: true, createdAt: new Date(), updatedAt: new Date() });
|
||||
await server.db.insert(companyMemberships).values({ companyId: fixture.companyId, principalType: "user",
|
||||
principalId: ownerUserId, status: "active", membershipRole: "owner" });
|
||||
}
|
||||
if (fallback === "unrelated parent") {
|
||||
await server.db.update(issues).set({ responsibleUserId: ownerUserId }).where(eq(issues.id, fixture.blockerId));
|
||||
}
|
||||
const call = { tool: "call_api", callId: "bound-routine", arguments: {
|
||||
operationId: "POST /api/companies/{companyId}/routines", body: {
|
||||
title: "Native run owned routine", assigneeAgentId: fixture.agentId, status: "paused",
|
||||
...(fallback === "unrelated parent" ? { parentIssueId: fixture.blockerId } : {}),
|
||||
actorResponsibleUserId: ownerUserId, responsibleUserId: ownerUserId, createdByUserId: ownerUserId,
|
||||
},
|
||||
} };
|
||||
const created = await fixture.authority.execute(call);
|
||||
expect(created).toMatchObject({ ok: true, status: 201 });
|
||||
expect(await new PaperclipRunnerToolAuthority(server.db, fixture).execute(call)).toEqual(created);
|
||||
const [routine] = await server.db.select({ id: routines.id, responsibleUserId: routines.responsibleUserId,
|
||||
createdByAgentId: routines.createdByAgentId, createdByUserId: routines.createdByUserId }).from(routines)
|
||||
.where(eq(routines.companyId, fixture.companyId));
|
||||
expect(routine).toEqual({ id: expect.any(String), responsibleUserId: fixture.responsibleUserId,
|
||||
createdByAgentId: fixture.agentId, createdByUserId: null });
|
||||
const revisions = await server.db.select({ responsibleUserId: routineRevisions.responsibleUserId,
|
||||
createdByRunId: routineRevisions.createdByRunId }).from(routineRevisions)
|
||||
.where(eq(routineRevisions.routineId, routine.id));
|
||||
expect(revisions).toEqual([{ responsibleUserId: fixture.responsibleUserId, createdByRunId: fixture.runId }]);
|
||||
await expect(fixture.authority.execute({ ...call, callId: "foreign-routine", arguments: {
|
||||
...call.arguments, pathParams: { companyId: fixture.foreignCompanyId },
|
||||
} })).rejects.toThrow("another company");
|
||||
await server.db.update(heartbeatRuns).set({ status: "cancelled" }).where(eq(heartbeatRuns.id, fixture.runId));
|
||||
await expect(fixture.authority.execute({ ...call, callId: "cancelled-routine" })).rejects.toThrow("binding_not_authorized");
|
||||
expect(await server.db.select({ id: routines.id }).from(routines).where(eq(routines.companyId, fixture.companyId)))
|
||||
.toEqual([{ id: routine.id }]);
|
||||
},
|
||||
);
|
||||
|
||||
it("uses the current run identity when an agent JWT carries a stale user claim", async () => {
|
||||
const fixture = await server.fixture({ connectionScenario: "fresh", disableWakeOnDemand: true });
|
||||
const token = createLocalAgentJwt(fixture.agentId, fixture.companyId, "paperclip_runner", fixture.runId, "stale-user");
|
||||
const response = await fetch(`${fixture.apiUrl}/api/companies/${fixture.companyId}/routines`, {
|
||||
method: "POST", headers: { Authorization: `Bearer ${token}`, "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ title: "Current identity routine", assigneeAgentId: fixture.agentId, status: "paused" }),
|
||||
});
|
||||
expect(response.status).toBe(201);
|
||||
expect(await response.json()).toMatchObject({ responsibleUserId: fixture.responsibleUserId,
|
||||
createdByAgentId: fixture.agentId, createdByUserId: null });
|
||||
});
|
||||
|
||||
it("rejects routine creation after the bound responsible user loses company membership", async () => {
|
||||
const fixture = await server.fixture({ connectionScenario: "fresh", disableWakeOnDemand: true });
|
||||
await server.db.delete(companyMemberships).where(and(eq(companyMemberships.companyId, fixture.companyId),
|
||||
eq(companyMemberships.principalId, fixture.responsibleUserId!)));
|
||||
await expect(fixture.authority.execute({ tool: "call_api", callId: "revoked-user-routine", arguments: {
|
||||
operationId: "POST /api/companies/{companyId}/routines", body: {
|
||||
title: "Revoked authority routine", assigneeAgentId: fixture.agentId, status: "paused",
|
||||
},
|
||||
} })).rejects.toMatchObject({ status: 403 });
|
||||
expect(await server.db.select({ id: routines.id }).from(routines).where(eq(routines.companyId, fixture.companyId))).toEqual([]);
|
||||
});
|
||||
|
||||
it.each([true, false])("keeps agent API-key responsibility binding (present=%s)", async bound => {
|
||||
const fixture = await server.fixture({ connectionScenario: "fresh", disableWakeOnDemand: true });
|
||||
const token = `pcp_fixture_${randomUUID()}`;
|
||||
await server.db.insert(agentApiKeys).values({ companyId: fixture.companyId, agentId: fixture.agentId,
|
||||
name: "Routine API fixture", keyHash: createHash("sha256").update(token).digest("hex"),
|
||||
responsibleUserId: bound ? fixture.responsibleUserId : null });
|
||||
const response = await fetch(`${fixture.apiUrl}/api/companies/${fixture.companyId}/routines`, {
|
||||
method: "POST", headers: { Authorization: `Bearer ${token}`, "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ title: "API-key routine", assigneeAgentId: fixture.agentId, status: "paused" }),
|
||||
});
|
||||
expect(response.status).toBe(bound ? 201 : 403);
|
||||
const rows = await server.db.select({ responsibleUserId: routines.responsibleUserId,
|
||||
createdByAgentId: routines.createdByAgentId, createdByUserId: routines.createdByUserId }).from(routines)
|
||||
.where(eq(routines.companyId, fixture.companyId));
|
||||
expect(rows).toEqual(bound ? [{ responsibleUserId: fixture.responsibleUserId,
|
||||
createdByAgentId: fixture.agentId, createdByUserId: null }] : []);
|
||||
});
|
||||
|
||||
it("manages owned routines and triggers with durable replay, attribution and owner denials", async () => {
|
||||
const fixture = await server.fixture({ disableWakeOnDemand: true, conversation: true });
|
||||
await server.db.update(issues).set({ responsibleUserId: fixture.responsibleUserId }).where(eq(issues.id, fixture.issueId));
|
||||
|
||||
@@ -154,7 +154,12 @@ async function resolveRoutineResponsibleUserId(db: Db, companyId: string, actorU
|
||||
return resolveCompanyDefaultResponsibleUserId(db, companyId);
|
||||
}
|
||||
|
||||
type Actor = { agentId?: string | null; userId?: string | null; runId?: string | null };
|
||||
type Actor = {
|
||||
agentId?: string | null;
|
||||
userId?: string | null;
|
||||
actorResponsibleUserId?: string | null;
|
||||
runId?: string | null;
|
||||
};
|
||||
type RoutineRow = typeof routines.$inferSelect;
|
||||
type RoutineTriggerRow = typeof routineTriggers.$inferSelect;
|
||||
|
||||
@@ -2217,7 +2222,12 @@ export function routineService(
|
||||
);
|
||||
assertRoutineVariableDefinitions(variables);
|
||||
const status = normalizeDraftRoutineStatus(input.status, input.assigneeAgentId);
|
||||
const responsibleUserId = await resolveRoutineResponsibleUserId(db, companyId, actor.userId, input.parentIssueId ?? null);
|
||||
const responsibleUserId = await resolveRoutineResponsibleUserId(
|
||||
db,
|
||||
companyId,
|
||||
actor.userId ?? actor.actorResponsibleUserId,
|
||||
input.parentIssueId ?? null,
|
||||
);
|
||||
if (!responsibleUserId) {
|
||||
throw unprocessable("Routine requires a responsible user");
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user