fix(auth): deduplicate run history permission checks (#16242)

Share matching run-history permission checks within one list operation while preserving current authorization, redaction, and fresh checks on subsequent requests.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

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.

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]);
+  });
+
 });

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<string, unknown> & 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));
   });
 

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<typeof canActorReadHeartbeatRun>[1],
+  actor: AuthorizationActor,
+  runs: readonly RunIssueBinding[],
+): Promise<boolean[]> {
+  const decisions = new Map<string, Promise<boolean>>();
+  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<string, unknown> | null {
   return value && typeof value === "object" && !Array.isArray(value)
     ? value as Record<string, unknown>