diff --git a/server/src/__tests__/agent-commentary.integration.test.ts b/server/src/__tests__/agent-commentary.integration.test.ts index 16585c554af..9b2c6ae23ae 100644 --- a/server/src/__tests__/agent-commentary.integration.test.ts +++ b/server/src/__tests__/agent-commentary.integration.test.ts @@ -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>; @@ -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) => { diff --git a/server/src/routes/agent-commentary.ts b/server/src/routes/agent-commentary.ts index 41d8910ee63..a834d74dff3 100644 --- a/server/src/routes/agent-commentary.ts +++ b/server/src/routes/agent-commentary.ts @@ -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 { + 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");