mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 10:13:11 +02:00
Fix low-trust promotion authorization
This commit is contained in:
1 parent
d78bd3b9a5
commit
af975b4d5d
4 files changed
+291
-19
No files matched your search
@@ -194,26 +194,43 @@ function makeAgent(id: string, overrides: Record<string, unknown> = {}) {
|
||||
};
|
||||
}
|
||||
|
||||
function createRunContextDb(contextSnapshot: Record<string, unknown> = {}) {
|
||||
function createRunContextDb(
|
||||
contextSnapshot: Record<string, unknown> = {},
|
||||
runAgentId: string = ownerAgentId,
|
||||
runId: string = ownerRunId,
|
||||
) {
|
||||
return {
|
||||
transaction: async (callback: (tx: Record<string, never>) => Promise<unknown>) => callback({}),
|
||||
select: vi.fn(() => ({
|
||||
select: vi.fn((selection: Record<string, unknown> = {}) => ({
|
||||
from: vi.fn(() => ({
|
||||
where: vi.fn(() => ({
|
||||
then: async (resolve: (rows: unknown[]) => unknown) =>
|
||||
resolve([{
|
||||
id: ownerRunId,
|
||||
companyId,
|
||||
agentId: ownerAgentId,
|
||||
contextSnapshot,
|
||||
}]),
|
||||
then: async (resolve: (rows: unknown[]) => unknown) => {
|
||||
const keys = Object.keys(selection);
|
||||
if (keys.includes("contextSnapshot")) {
|
||||
return resolve([{
|
||||
id: runId,
|
||||
companyId,
|
||||
agentId: runAgentId,
|
||||
contextSnapshot,
|
||||
}]);
|
||||
}
|
||||
if (keys.includes("permissions")) {
|
||||
return resolve([{ id: runAgentId, companyId, permissions: {}, role: "engineer", reportsTo: null }]);
|
||||
}
|
||||
return resolve([{ id: runAgentId, companyId, permissions: {}, role: "engineer", reportsTo: null }]);
|
||||
},
|
||||
})),
|
||||
})),
|
||||
})),
|
||||
};
|
||||
}
|
||||
|
||||
async function createApp(actor: Record<string, unknown>, db: unknown = createRunContextDb()) {
|
||||
async function createApp(actor: Record<string, unknown>, db?: unknown) {
|
||||
const routeDb = db ?? createRunContextDb(
|
||||
{},
|
||||
typeof actor.agentId === "string" ? actor.agentId : ownerAgentId,
|
||||
typeof actor.runId === "string" ? actor.runId : ownerRunId,
|
||||
);
|
||||
const [{ errorHandler }, { issueRoutes }] = await Promise.all([
|
||||
vi.importActual<typeof import("../middleware/index.js")>("../middleware/index.js"),
|
||||
vi.importActual<typeof import("../routes/issues.js")>("../routes/issues.js"),
|
||||
@@ -224,7 +241,7 @@ async function createApp(actor: Record<string, unknown>, db: unknown = createRun
|
||||
(req as any).actor = actor;
|
||||
next();
|
||||
});
|
||||
app.use("/api", issueRoutes(db as any, mockStorageService as any));
|
||||
app.use("/api", issueRoutes(routeDb as any, mockStorageService as any));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
@@ -280,10 +297,26 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
mockAccessService.canUser.mockReset();
|
||||
mockAccessService.decide.mockReset();
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: input.action === "tasks:assign",
|
||||
allowed:
|
||||
input.action === "tasks:assign" ||
|
||||
input.action === "issue:read" ||
|
||||
input.action === "issue:mutate" ||
|
||||
input.action === "company_scope:read",
|
||||
action: input.action,
|
||||
reason: input.action === "tasks:assign" ? "allow_explicit_grant" : "deny_missing_grant",
|
||||
explanation: input.action === "tasks:assign" ? "Allowed by test assignment default." : "Missing permission.",
|
||||
reason:
|
||||
input.action === "tasks:assign" ||
|
||||
input.action === "issue:read" ||
|
||||
input.action === "issue:mutate" ||
|
||||
input.action === "company_scope:read"
|
||||
? "allow_explicit_grant"
|
||||
: "deny_missing_grant",
|
||||
explanation:
|
||||
input.action === "tasks:assign" ||
|
||||
input.action === "issue:read" ||
|
||||
input.action === "issue:mutate" ||
|
||||
input.action === "company_scope:read"
|
||||
? "Allowed by test default."
|
||||
: "Missing permission.",
|
||||
}));
|
||||
mockAccessService.hasPermission.mockReset();
|
||||
mockAgentService.getById.mockReset();
|
||||
@@ -486,6 +519,16 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
request(app).put(`/api/issues/${issueId}/documents/plan`).send({ format: "markdown", body: "# blocked" }),
|
||||
],
|
||||
["work product update", (app: express.Express) => request(app).patch("/api/work-products/product-1").send({ title: "Blocked" })],
|
||||
[
|
||||
"low-trust promotion",
|
||||
(app: express.Express) =>
|
||||
request(app).post(`/api/issues/${issueId}/low-trust/promotions`).send({
|
||||
sourceArtifactKind: "comment",
|
||||
sourceArtifactId: recoveryActionId,
|
||||
title: "Promoted artifact",
|
||||
summary: "Sanitized output",
|
||||
}),
|
||||
],
|
||||
[
|
||||
"attachment upload",
|
||||
(app: express.Express) =>
|
||||
@@ -503,6 +546,7 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
expect(mockIssueService.update).not.toHaveBeenCalled();
|
||||
expect(mockIssueService.addComment).not.toHaveBeenCalled();
|
||||
expect(mockDocumentService.upsertIssueDocument).not.toHaveBeenCalled();
|
||||
expect(mockWorkProductService.createForIssue).not.toHaveBeenCalled();
|
||||
expect(mockWorkProductService.update).not.toHaveBeenCalled();
|
||||
expect(mockStorageService.putFile).not.toHaveBeenCalled();
|
||||
expect(mockStorageService.deleteObject).not.toHaveBeenCalled();
|
||||
@@ -542,6 +586,16 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
],
|
||||
["work product update", (app: express.Express) => request(app).patch("/api/work-products/product-1").send({ title: "Blocked" })],
|
||||
["work product delete", (app: express.Express) => request(app).delete("/api/work-products/product-1")],
|
||||
[
|
||||
"low-trust promotion",
|
||||
(app: express.Express) =>
|
||||
request(app).post(`/api/issues/${issueId}/low-trust/promotions`).send({
|
||||
sourceArtifactKind: "comment",
|
||||
sourceArtifactId: recoveryActionId,
|
||||
title: "Promoted artifact",
|
||||
summary: "Sanitized output",
|
||||
}),
|
||||
],
|
||||
[
|
||||
"attachment upload",
|
||||
(app: express.Express) =>
|
||||
@@ -693,10 +747,18 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
|
||||
it("allows agents with the active-checkout management grant to mutate active checkouts", async () => {
|
||||
mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: input.action === "tasks:manage_active_checkouts",
|
||||
allowed: input.action === "issue:mutate" || input.action === "tasks:manage_active_checkouts",
|
||||
action: input.action,
|
||||
reason: input.action === "tasks:manage_active_checkouts" ? "allow_explicit_grant" : "deny_missing_grant",
|
||||
explanation: input.action === "tasks:manage_active_checkouts" ? "Allowed by checkout management grant." : "Missing permission.",
|
||||
reason:
|
||||
input.action === "issue:mutate" || input.action === "tasks:manage_active_checkouts"
|
||||
? "allow_explicit_grant"
|
||||
: "deny_missing_grant",
|
||||
explanation:
|
||||
input.action === "tasks:manage_active_checkouts"
|
||||
? "Allowed by checkout management grant."
|
||||
: input.action === "issue:mutate"
|
||||
? "Allowed by test boundary default."
|
||||
: "Missing permission.",
|
||||
}));
|
||||
|
||||
const res = await request(await createApp(peerActor())).patch(`/api/issues/${issueId}`).send({ title: "Managed update" });
|
||||
@@ -846,6 +908,15 @@ describe("agent issue mutation checkout ownership", () => {
|
||||
reason: "deny_policy_restricted",
|
||||
explanation: "Target agent requires approval before task assignment.",
|
||||
}));
|
||||
decide.mockImplementation(async (input: { action: string }) => ({
|
||||
allowed: input.action === "issue:mutate",
|
||||
action: input.action,
|
||||
reason: input.action === "issue:mutate" ? "allow_self" : "deny_policy_restricted",
|
||||
explanation:
|
||||
input.action === "issue:mutate"
|
||||
? "Allowed because the actor owns the assigned issue."
|
||||
: "Target agent requires approval before task assignment.",
|
||||
}));
|
||||
(mockAccessService as any).decide = decide;
|
||||
mockIssueService.getById.mockResolvedValue(makeIssue({ assigneeAgentId: ownerAgentId }));
|
||||
mockAgentService.resolveByReference.mockResolvedValue({
|
||||
|
||||
@@ -1,12 +1,23 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { agents, companies, createDb, heartbeatRuns, issues } from "@paperclipai/db";
|
||||
import { LOW_TRUST_REVIEW_PRESET } from "@paperclipai/shared";
|
||||
import {
|
||||
LOW_TRUST_QUARANTINED_BODY,
|
||||
buildPromotedSourceTrust,
|
||||
isLowTrustQuarantined,
|
||||
redactQuarantinedBodyForHigherTrust,
|
||||
resolveActorSourceTrustForIssue,
|
||||
sanitizeQuarantinedCommentForHigherTrust,
|
||||
} from "../services/source-trust.js";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
const quarantinedSourceTrust = {
|
||||
preset: LOW_TRUST_REVIEW_PRESET,
|
||||
@@ -79,3 +90,166 @@ describe("source trust quarantine helpers", () => {
|
||||
expect(isLowTrustQuarantined(promoted)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describeEmbeddedPostgres("resolveActorSourceTrustForIssue", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-source-trust-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
await db.delete(heartbeatRuns);
|
||||
await db.delete(issues);
|
||||
await db.delete(agents);
|
||||
await db.delete(companies);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
async function createCompany() {
|
||||
return db
|
||||
.insert(companies)
|
||||
.values({
|
||||
name: `Source trust ${randomUUID()}`,
|
||||
issuePrefix: `ST${randomUUID().slice(0, 6).toUpperCase()}`,
|
||||
})
|
||||
.returning()
|
||||
.then((rows) => rows[0]!);
|
||||
}
|
||||
|
||||
async function createAgent(companyId: string) {
|
||||
return db
|
||||
.insert(agents)
|
||||
.values({
|
||||
companyId,
|
||||
name: `Agent ${randomUUID()}`,
|
||||
role: "engineer",
|
||||
adapterType: "process",
|
||||
adapterConfig: {},
|
||||
runtimeConfig: {},
|
||||
permissions: {},
|
||||
})
|
||||
.returning()
|
||||
.then((rows) => rows[0]!);
|
||||
}
|
||||
|
||||
function lowTrustExecutionPolicy(companyId: string, rootIssueId: string) {
|
||||
return {
|
||||
authorizationPolicy: {
|
||||
trustBoundary: {
|
||||
mode: LOW_TRUST_REVIEW_PRESET,
|
||||
companyId,
|
||||
rootIssueId,
|
||||
},
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
it("uses the heartbeat run execution-policy snapshot after live issue policy changes", async () => {
|
||||
const company = await createCompany();
|
||||
const agent = await createAgent(company.id);
|
||||
const [issue] = await db
|
||||
.insert(issues)
|
||||
.values({
|
||||
companyId: company.id,
|
||||
title: "Run-scoped low-trust issue",
|
||||
status: "in_progress",
|
||||
priority: "high",
|
||||
assigneeAgentId: agent.id,
|
||||
})
|
||||
.returning();
|
||||
const executionPolicy = lowTrustExecutionPolicy(company.id, issue!.id);
|
||||
const [run] = await db
|
||||
.insert(heartbeatRuns)
|
||||
.values({
|
||||
companyId: company.id,
|
||||
agentId: agent.id,
|
||||
status: "running",
|
||||
contextSnapshot: {
|
||||
issueId: issue!.id,
|
||||
executionPolicy,
|
||||
},
|
||||
})
|
||||
.returning();
|
||||
|
||||
await db.update(issues).set({ executionPolicy: null }).where(eq(issues.id, issue!.id));
|
||||
|
||||
const sourceTrust = await resolveActorSourceTrustForIssue({
|
||||
db,
|
||||
issue: {
|
||||
id: issue!.id,
|
||||
companyId: company.id,
|
||||
projectId: null,
|
||||
executionPolicy: null,
|
||||
},
|
||||
actor: {
|
||||
actorType: "agent",
|
||||
actorId: agent.id,
|
||||
agentId: agent.id,
|
||||
runId: run!.id,
|
||||
},
|
||||
});
|
||||
|
||||
expect(sourceTrust).toMatchObject({
|
||||
preset: LOW_TRUST_REVIEW_PRESET,
|
||||
disposition: "quarantined",
|
||||
sourceIssueId: issue!.id,
|
||||
sourceRunId: run!.id,
|
||||
sourceAgentId: agent.id,
|
||||
});
|
||||
});
|
||||
|
||||
it("fails closed when the supplied run id does not belong to the acting agent", async () => {
|
||||
const company = await createCompany();
|
||||
const actorAgent = await createAgent(company.id);
|
||||
const runOwnerAgent = await createAgent(company.id);
|
||||
const [issue] = await db
|
||||
.insert(issues)
|
||||
.values({
|
||||
companyId: company.id,
|
||||
title: "Standard issue",
|
||||
status: "in_progress",
|
||||
priority: "high",
|
||||
assigneeAgentId: actorAgent.id,
|
||||
})
|
||||
.returning();
|
||||
const [run] = await db
|
||||
.insert(heartbeatRuns)
|
||||
.values({
|
||||
companyId: company.id,
|
||||
agentId: runOwnerAgent.id,
|
||||
status: "running",
|
||||
contextSnapshot: { issueId: issue!.id },
|
||||
})
|
||||
.returning();
|
||||
|
||||
const sourceTrust = await resolveActorSourceTrustForIssue({
|
||||
db,
|
||||
issue: {
|
||||
id: issue!.id,
|
||||
companyId: company.id,
|
||||
projectId: null,
|
||||
executionPolicy: null,
|
||||
},
|
||||
actor: {
|
||||
actorType: "agent",
|
||||
actorId: actorAgent.id,
|
||||
agentId: actorAgent.id,
|
||||
runId: run!.id,
|
||||
},
|
||||
});
|
||||
|
||||
expect(sourceTrust).toMatchObject({
|
||||
preset: LOW_TRUST_REVIEW_PRESET,
|
||||
disposition: "quarantined",
|
||||
sourceIssueId: issue!.id,
|
||||
sourceRunId: run!.id,
|
||||
sourceAgentId: actorAgent.id,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -3578,6 +3578,9 @@ export function issueRoutes(
|
||||
return;
|
||||
}
|
||||
assertCompanyAccess(req, issue.companyId);
|
||||
if (!(await assertIssueReadAllowed(req, res, issue))) return;
|
||||
if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return;
|
||||
if (!(await assertDeliverableMutationAllowedByRunContext(req, res, issue))) return;
|
||||
const actor = getActorInfo(req);
|
||||
if (await sourceTrustForActorWrite(issue, actor)) {
|
||||
res.status(403).json({ error: "Low-trust actors cannot promote quarantined output" });
|
||||
|
||||
@@ -69,6 +69,12 @@ export function buildLowTrustSourceTrust(input: {
|
||||
};
|
||||
}
|
||||
|
||||
function readObject(value: unknown): Record<string, unknown> | null {
|
||||
return typeof value === "object" && value !== null && !Array.isArray(value)
|
||||
? value as Record<string, unknown>
|
||||
: null;
|
||||
}
|
||||
|
||||
export function buildPromotedSourceTrust(input: {
|
||||
sourceIssueId: string;
|
||||
sourceArtifactKind: "comment" | "document" | "work_product" | "issue";
|
||||
@@ -122,6 +128,8 @@ export async function resolveActorSourceTrustForIssue(input: {
|
||||
? input.db
|
||||
.select({
|
||||
companyId: heartbeatRuns.companyId,
|
||||
agentId: heartbeatRuns.agentId,
|
||||
contextSnapshot: heartbeatRuns.contextSnapshot,
|
||||
})
|
||||
.from(heartbeatRuns)
|
||||
.where(and(eq(heartbeatRuns.id, input.actor.runId), eq(heartbeatRuns.companyId, input.issue.companyId)))
|
||||
@@ -129,6 +137,17 @@ export async function resolveActorSourceTrustForIssue(input: {
|
||||
: Promise.resolve(null),
|
||||
]);
|
||||
|
||||
if (input.actor.runId && (!run || run.agentId !== input.actor.agentId)) {
|
||||
return buildLowTrustSourceTrust({
|
||||
issueId: input.issue.id,
|
||||
runId: input.actor.runId,
|
||||
agentId: input.actor.agentId,
|
||||
});
|
||||
}
|
||||
|
||||
const runContext = readObject(run?.contextSnapshot);
|
||||
const runExecutionPolicy = readObject(runContext?.executionPolicy);
|
||||
|
||||
const resolution = resolveCoreTrustPreset({
|
||||
companyId: input.issue.companyId,
|
||||
agent,
|
||||
@@ -137,7 +156,12 @@ export async function resolveActorSourceTrustForIssue(input: {
|
||||
companyId: input.issue.companyId,
|
||||
executionPolicy: input.issue.executionPolicy,
|
||||
},
|
||||
run,
|
||||
run: run
|
||||
? {
|
||||
companyId: run.companyId,
|
||||
executionPolicy: runExecutionPolicy,
|
||||
}
|
||||
: null,
|
||||
});
|
||||
|
||||
if (resolution.kind !== "low_trust_review") return null;
|
||||
|
||||
Reference in new issue
Block a user