diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 568f9275673..da60b3bfd61 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -933,6 +933,11 @@ No Docker or external database is required for this mode. - `off`: skip the predicate and shadow logging Grant checks read current database state; there is no positive privacy cache. + +A run-history list is one read operation. It checks each distinct company, +run scope, and task binding once, then applies that decision to the related +rows in the same response. Every new list operation reads permissions again. + Sharing a task grants access to that task and its private descendants. Parents and siblings need separate grants. An agent's access is intersected with its run's responsible user's current access. diff --git a/server/src/__tests__/heartbeat-run-privacy-routes.test.ts b/server/src/__tests__/heartbeat-run-privacy-routes.test.ts index 32df5430679..6051e11d8c3 100644 --- a/server/src/__tests__/heartbeat-run-privacy-routes.test.ts +++ b/server/src/__tests__/heartbeat-run-privacy-routes.test.ts @@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto"; import { eq } from "drizzle-orm"; import express from "express"; import request from "supertest"; -import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { agents, agentRuntimeState, @@ -27,6 +27,8 @@ import { import { errorHandler } from "../middleware/index.js"; import { activityRoutes } from "../routes/activity.js"; import { agentRoutes } from "../routes/agents.js"; +import { authorizationService } from "../services/authorization.js"; +import { getHeartbeatRunReadDecisions } from "../services/heartbeat-run-privacy.js"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; @@ -43,6 +45,7 @@ describeEmbeddedPostgres("heartbeat run privacy routes", { concurrent: false }, }, 120_000); afterEach(async () => { + vi.restoreAllMocks(); await db.delete(activityLog); await db.delete(workspaceOperations); await db.delete(heartbeatRunEvents); @@ -424,4 +427,103 @@ describeEmbeddedPostgres("heartbeat run privacy routes", { concurrent: false }, process.env.PAPERCLIP_ISSUE_PRIVACY_MODE = "enforce"; } }); + it.each(["owner", "outsider"])("checks concurrent duplicate issue runs once for the %s", async (viewer) => { + const fixture = await seedFixture(); + const access = authorizationService(db); + const decide = vi.spyOn(access, "decide"); + const actor = { + type: "agent" as const, + companyId: fixture.companyId, + agentId: viewer === "owner" ? fixture.ownerAgentId : fixture.otherAgentId, + }; + const binding = { companyId: fixture.companyId, scopeKind: "issue" as const, issueId: fixture.issueId }; + const decisions = await getHeartbeatRunReadDecisions(db, access, actor, Array.from({ length: 200 }, () => ({ ...binding }))); + expect(decisions).toEqual(Array(200).fill(viewer === "owner")); + expect(decide).toHaveBeenCalledTimes(1); + decide.mockRestore(); + }); + + it("keeps company scope, issue scope, and company identity separate", async () => { + const fixture = await seedFixture(); + const access = authorizationService(db); + const actor = { type: "agent" as const, companyId: fixture.companyId, agentId: fixture.ownerAgentId }; + const binding = { companyId: fixture.companyId, scopeKind: "issue" as const, issueId: fixture.issueId }; + expect(await getHeartbeatRunReadDecisions(db, access, actor, [binding, { ...binding, companyId: randomUUID() }])).toEqual([true, false]); + expect(await getHeartbeatRunReadDecisions(db, access, { ...actor, agentId: fixture.otherAgentId }, [ + { ...binding, scopeKind: "company" }, binding, { ...binding, scopeKind: null }, + { companyId: fixture.companyId, scopeKind: "issue" }, + ])).toEqual([true, false, false, false]); + }); + + it("uses the captured binding if a caller changes its input while checks are pending", async () => { + const fixture = await seedFixture(); + const access = authorizationService(db); + const decide = vi.spyOn(access, "decide"); + const actor = { type: "agent" as const, companyId: fixture.companyId, agentId: fixture.ownerAgentId }; + const binding = { companyId: fixture.companyId, scopeKind: "issue" as const, issueId: fixture.issueId }; + const pending = getHeartbeatRunReadDecisions(db, access, actor, [binding, binding]); + binding.issueId = randomUUID(); + expect(await pending).toEqual([true, true]); + expect(decide).toHaveBeenCalledExactlyOnceWith(expect.objectContaining({ + resource: expect.objectContaining({ companyId: fixture.companyId, issueId: fixture.issueId }), + })); + }); + + it("does not share decisions between actors", async () => { + const fixture = await seedFixture(); + const access = authorizationService(db); + const binding = { companyId: fixture.companyId, scopeKind: "issue" as const, issueId: fixture.issueId }; + const actor = { type: "agent" as const, companyId: fixture.companyId, agentId: fixture.ownerAgentId }; + expect(await getHeartbeatRunReadDecisions(db, access, actor, [binding])).toEqual([true]); + expect(await getHeartbeatRunReadDecisions(db, access, { ...actor, agentId: fixture.otherAgentId }, [binding])).toEqual([false]); + }); + + it("rechecks a revoked grant on the next history request", async () => { + const fixture = await seedFixture(); + const duplicateId = randomUUID(); + await db.insert(heartbeatRuns).values({ + id: duplicateId, companyId: fixture.companyId, agentId: fixture.ownerAgentId, + scopeKind: "issue", issueId: fixture.issueId, invocationSource: "on_demand", status: "succeeded", + resultJson: { summary: "PRIVATE_HISTORY_CANARY" }, + }); + const app = createApp(fixture.companyId, fixture.grantedAgentId); + const path = `/api/companies/${fixture.companyId}/heartbeat-runs`; + const permitted = await request(app).get(path); + expect(permitted.status).toBe(200); + expect(permitted.body.find((run: { id: string }) => run.id === duplicateId).resultJson.summary).toBe("PRIVATE_HISTORY_CANARY"); + await db.update(issueAccessGrants).set({ revokedAt: new Date() }).where(eq(issueAccessGrants.subjectId, fixture.grantedAgentId)); + for (const suffix of ["", "?summary=true"]) { + const denied = await request(app).get(path + suffix); + expect(denied.status).toBe(200); + for (const id of [fixture.privateRunId, duplicateId]) { + expect(denied.body.find((run: { id: string }) => run.id === id)).toMatchObject({ id, redacted: true }); + } + expect(JSON.stringify(denied.body)).not.toContain("PRIVATE_HISTORY_CANARY"); + expect(JSON.stringify(denied.body)).not.toContain("private@example.test"); + } + }); + + it("fails closed when a shared decision rejects and can recover in a new request", async () => { + const fixture = await seedFixture(); + const access = authorizationService(db); + const error = new Error("authorization unavailable"); + const decide = vi.spyOn(access, "decide").mockRejectedValue(error); + const actor = { type: "agent" as const, companyId: fixture.companyId, agentId: fixture.ownerAgentId }; + const binding = { companyId: fixture.companyId, scopeKind: "issue" as const, issueId: fixture.issueId }; + await expect(getHeartbeatRunReadDecisions(db, access, actor, Array(20).fill(binding))).rejects.toBe(error); + expect(decide).toHaveBeenCalledTimes(1); + decide.mockRestore(); + expect(await getHeartbeatRunReadDecisions(db, access, actor, [binding])).toEqual([true]); + }); + + it("rejects an issue deleted before the next request", async () => { + const fixture = await seedFixture(); + const access = authorizationService(db); + const actor = { type: "agent" as const, companyId: fixture.companyId, agentId: fixture.ownerAgentId }; + const binding = { companyId: fixture.companyId, scopeKind: "issue" as const, issueId: fixture.issueId }; + expect(await getHeartbeatRunReadDecisions(db, access, actor, [binding])).toEqual([true]); + await db.delete(issues).where(eq(issues.id, fixture.issueId)); + expect(await getHeartbeatRunReadDecisions(db, access, actor, [binding])).toEqual([false]); + }); + }); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 5eb5eece028..c7864216599 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -273,6 +273,7 @@ import { } from "../services/change-consent-gate.js"; import { canActorReadHeartbeatRun, + getHeartbeatRunReadDecisions, canActorReadWorkspaceOperation, redactHeartbeatRunListRow, } from "../services/heartbeat-run-privacy.js"; @@ -800,12 +801,6 @@ export function agentRoutes( return false; } - async function serializeRunListRow< - T extends Record & RunReadBinding & { id: string }, - >(req: Request, run: T) { - return await actorCanReadRun(req, run) ? run : redactHeartbeatRunListRow(run); - } - // The company-scoped adapter login-session service. It runs the device-login // flow in a fresh trusted sandbox and holds the one-time prompt in memory. The // process owns one instance, so the in-memory prompt and the cancellation @@ -6616,7 +6611,9 @@ export function agentRoutes( const limit = limitParam ? Math.max(1, Math.min(1000, parseInt(limitParam, 10) || 200)) : undefined; const summary = req.query.summary === "true" || req.query.summary === "1"; const runs = await heartbeat.list(companyId, agentId, limit, { summary }); - res.json(await runRedactions.redactForRuns(companyId, await Promise.all(runs.map(run => serializeRunListRow(req, run))), + const readDecisions = await getHeartbeatRunReadDecisions(db, access, req.actor, runs); + res.json(await runRedactions.redactForRuns(companyId, runs.map((run, index) => + readDecisions[index] ? run : redactHeartbeatRunListRow(run)), req.actor.type === "board" ? getActorInfo(req).actorId : null)); }); diff --git a/server/src/services/heartbeat-run-privacy.ts b/server/src/services/heartbeat-run-privacy.ts index 64027132012..0564f93c427 100644 --- a/server/src/services/heartbeat-run-privacy.ts +++ b/server/src/services/heartbeat-run-privacy.ts @@ -55,6 +55,25 @@ export async function canActorReadHeartbeatRun( return decision.allowed; } +export async function getHeartbeatRunReadDecisions( + db: Db, + access: Parameters[1], + actor: AuthorizationActor, + runs: readonly RunIssueBinding[], +): Promise { + const decisions = new Map>(); + return Promise.all(runs.map(run => { + const binding = { companyId: run.companyId, scopeKind: run.scopeKind, issueId: run.issueId }; + const key = JSON.stringify([binding.companyId, binding.scopeKind === "company", binding.issueId ?? null]); + let decision = decisions.get(key); + if (!decision) { + decision = canActorReadHeartbeatRun(db, access, actor, binding); + decisions.set(key, decision); + } + return decision; + })); +} + function asRecord(value: unknown): Record | null { return value && typeof value === "object" && !Array.isArray(value) ? value as Record