mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
feat(commentary): agents read only their own commentary unless granted commentary:view
Product rule: an agent cannot read other agents' commentary unless it holds commentary:view, which is not a default agent grant. An agent may read its own entries by asking for them explicitly. Without the grant, listing all commentary or another agent's entries is a 403 that names the human page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
4279d57aa9
commit
0fc6a7a635
2 files changed
+63
-22
No files matched your search
@@ -7,7 +7,7 @@ import { fileURLToPath } from "node:url";
|
||||
import { and, eq, sql } from "drizzle-orm";
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { activityLog, agentApiKeys, agentCommentary, agents, authUsers, companies, companyMemberships, heartbeatRuns, issues, nativeRunResults } from "@paperclipai/db";
|
||||
import { activityLog, agentApiKeys, agentCommentary, agents, authUsers, companies, companyMemberships, heartbeatRuns, issues, nativeRunResults, principalPermissionGrants } from "@paperclipai/db";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { createLocalAgentJwt } from "../agent-auth-jwt.js";
|
||||
import { submitAgentCommentary } from "../services/agent-commentary.js";
|
||||
@@ -19,6 +19,7 @@ import { agentCommentaryRoutes } from "../routes/agent-commentary.js";
|
||||
import { errorHandler } from "../middleware/error-handler.js";
|
||||
import { agentService } from "../services/agents.js";
|
||||
import { companyService } from "../services/companies.js";
|
||||
import { NEW_STANDARD_AGENT_DEFAULT_GRANT_KEYS } from "../lib/agent-permissions.js";
|
||||
|
||||
describe("internal agent commentary through both transports", () => {
|
||||
let server: Awaited<ReturnType<typeof startRunnerApiTestServer>>;
|
||||
@@ -125,18 +126,36 @@ describe("internal agent commentary through both transports", () => {
|
||||
expect(await rows(f)).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("tells an agent denied a commentary read where a person can see it, for its own company only", async () => {
|
||||
it("lets an agent read only its own commentary unless granted commentary:view, and names the human page when denied", async () => {
|
||||
const f = await legacy();
|
||||
expect((await post(f, { ...input, kind: "complaint" })).status).toBe(201);
|
||||
const [own] = await rows(f);
|
||||
const [company] = await server.db.select({ issuePrefix: companies.issuePrefix }).from(companies).where(eq(companies.id, f.companyId));
|
||||
const read = (companyId: string) => fetch(`${server.apiUrl}/api/companies/${companyId}/agent-commentary`, { headers: { Authorization: `Bearer ${f.token}` } });
|
||||
const own = await read(f.companyId);
|
||||
expect(own.status).toBe(403);
|
||||
const body = await own.json() as { error: string; remediation?: string };
|
||||
expect(body.error).toBe("Board access required");
|
||||
expect(body.remediation).toContain(`/${company!.issuePrefix}/activity/commentary`);
|
||||
const foreign = await read(f.foreignCompanyId);
|
||||
const get = (path: string, companyId = f.companyId) => fetch(`${server.apiUrl}/api/companies/${companyId}/agent-commentary${path}`, { headers: { Authorization: `Bearer ${f.token}` } });
|
||||
const denied = async (response: Response) => {
|
||||
expect(response.status).toBe(403);
|
||||
const body = await response.json() as { error: string; remediation?: string };
|
||||
expect(body.error).toBe("Board access required");
|
||||
expect(body.remediation).toContain(`/${company!.issuePrefix}/activity/commentary`);
|
||||
};
|
||||
// Reading other agents' commentary is never a default agent grant.
|
||||
expect(NEW_STANDARD_AGENT_DEFAULT_GRANT_KEYS).not.toContain("commentary:view");
|
||||
// Default: other agents' feedback and the full list are denied; its own is readable.
|
||||
await denied(await get(""));
|
||||
await denied(await get(`?agentId=${randomUUID()}`));
|
||||
await denied(await get(`/${randomUUID()}`));
|
||||
const mine = await get(`?agentId=${f.agentId}`);
|
||||
expect(mine.status).toBe(200);
|
||||
expect(((await mine.json()) as { items: Array<{ id: string }> }).items.map(row => row.id)).toEqual([own!.id]);
|
||||
expect((await get(`/${own!.id}`)).status).toBe(200);
|
||||
// Another company's request fails on company access, with no page path.
|
||||
const foreign = await get("", f.foreignCompanyId);
|
||||
expect(foreign.status).toBe(403);
|
||||
expect(((await foreign.json()) as { remediation?: string }).remediation).toBeUndefined();
|
||||
// The explicit grant allows reading everyone's commentary.
|
||||
await server.db.insert(companyMemberships).values({ companyId: f.companyId, principalType: "agent", principalId: f.agentId, status: "active", membershipRole: "member" }).onConflictDoNothing();
|
||||
await server.db.insert(principalPermissionGrants).values({ companyId: f.companyId, principalType: "agent", principalId: f.agentId, permissionKey: "commentary:view" });
|
||||
expect((await get("")).status).toBe(200);
|
||||
});
|
||||
|
||||
it.each(["cancelled", "succeeded", "failed"])("rejects submissions and replay after the run is %s", async (status) => {
|
||||
|
||||
@@ -14,16 +14,22 @@ export function agentCommentaryRoutes(db: Db) {
|
||||
const reader = agentCommentaryReader(db);
|
||||
const access = accessService(db);
|
||||
|
||||
async function authorizeRead(req: Request, companyId: string) {
|
||||
assertCompanyAccess(req, companyId);
|
||||
if (req.actor.type !== "board") {
|
||||
// Agents can submit commentary but never read it. Name the human page so
|
||||
// an agent can send the person somewhere useful instead of a dead end.
|
||||
const [company] = await db.select({ issuePrefix: companies.issuePrefix }).from(companies).where(eq(companies.id, companyId)).limit(1);
|
||||
throw forbidden("Board access required", company ? {
|
||||
remediation: `Agent commentary is readable only by people with commentary access. Send the person to /${company.issuePrefix}/activity/commentary.`,
|
||||
} : undefined);
|
||||
}
|
||||
// Agents can submit commentary. They read only their own entries unless they
|
||||
// hold commentary:view, which no agent has by default. A denial names the
|
||||
// human page so the agent can send the person somewhere useful.
|
||||
async function denyAgentRead(companyId: string): Promise<never> {
|
||||
const [company] = await db.select({ issuePrefix: companies.issuePrefix }).from(companies).where(eq(companies.id, companyId)).limit(1);
|
||||
throw forbidden("Board access required", company ? {
|
||||
remediation: `Agents read only their own commentary unless granted commentary:view. People with commentary access can read it at /${company.issuePrefix}/activity/commentary.`,
|
||||
} : undefined);
|
||||
}
|
||||
|
||||
async function agentCanReadAll(req: Request, companyId: string) {
|
||||
return req.actor.type === "agent" && !!req.actor.agentId
|
||||
&& await access.hasPermission(companyId, "agent", req.actor.agentId, "commentary:view");
|
||||
}
|
||||
|
||||
async function authorizeBoardRead(req: Request, companyId: string) {
|
||||
assertBoard(req);
|
||||
if (req.actor.source !== "local_implicit" && !req.actor.isInstanceAdmin
|
||||
&& (!req.actor.userId || !await access.canUser(companyId, req.actor.userId, "commentary:view"))) {
|
||||
@@ -34,16 +40,32 @@ export function agentCommentaryRoutes(db: Db) {
|
||||
|
||||
router.get("/companies/:companyId/agent-commentary", async (req, res) => {
|
||||
const companyId = req.params.companyId as string;
|
||||
const readCondition = await authorizeRead(req, companyId);
|
||||
assertCompanyAccess(req, companyId);
|
||||
const query = agentCommentaryQuerySchema.safeParse(req.query);
|
||||
if (!query.success) throw badRequest("Invalid agent commentary filters", query.error.issues);
|
||||
res.json(await reader.list(companyId, query.data, readCondition));
|
||||
if (req.actor.type === "agent") {
|
||||
// Without the grant, only an explicit request for its own entries is allowed.
|
||||
if (!await agentCanReadAll(req, companyId) && query.data.agentId !== req.actor.agentId) await denyAgentRead(companyId);
|
||||
res.json(await reader.list(companyId, query.data, await issueReadSqlCondition(db, req.actor)));
|
||||
return;
|
||||
}
|
||||
if (req.actor.type !== "board") await denyAgentRead(companyId);
|
||||
res.json(await reader.list(companyId, query.data, await authorizeBoardRead(req, companyId)));
|
||||
});
|
||||
|
||||
router.get("/companies/:companyId/agent-commentary/:id", async (req, res) => {
|
||||
const companyId = req.params.companyId as string;
|
||||
const readCondition = await authorizeRead(req, companyId);
|
||||
assertCompanyAccess(req, companyId);
|
||||
const id = req.params.id as string;
|
||||
if (req.actor.type === "agent") {
|
||||
const entry = isUuidLike(id) ? await reader.get(companyId, id, await issueReadSqlCondition(db, req.actor)) : null;
|
||||
if (!await agentCanReadAll(req, companyId) && entry?.agent.id !== req.actor.agentId) await denyAgentRead(companyId);
|
||||
if (!entry) throw notFound("Commentary not found");
|
||||
res.json(entry);
|
||||
return;
|
||||
}
|
||||
if (req.actor.type !== "board") await denyAgentRead(companyId);
|
||||
const readCondition = await authorizeBoardRead(req, companyId);
|
||||
if (!isUuidLike(id)) throw notFound("Commentary not found");
|
||||
const entry = await reader.get(companyId, id, readCondition);
|
||||
if (!entry) throw notFound("Commentary not found");
|
||||
|
||||
Reference in new issue
Block a user