diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 0322ea4a90f..fd25491f0d4 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -2885,6 +2885,10 @@ Sandbox-provider driver lookup filters plugin manifests by driver key and kind in Postgres before returning rows. Install order, status and worker checks still select the provider. Each lookup reads current registry state without a cache. +Run-to-issue links authorize through `getRunReadAccess`, which reads only the run +ID, company ID, scope kind, and issue ID. The existing visibility policy then +filters linked issues without loading the run's prompt, output, or result. + Run the deterministic native event replay with the normal server test runner: ```sh diff --git a/server/src/__tests__/activity-routes.test.ts b/server/src/__tests__/activity-routes.test.ts index 4d654036ecd..578c8a2738a 100644 --- a/server/src/__tests__/activity-routes.test.ts +++ b/server/src/__tests__/activity-routes.test.ts @@ -12,6 +12,7 @@ const mockActivityService = vi.hoisted(() => ({ const mockHeartbeatService = vi.hoisted(() => ({ getRun: vi.fn(), + getRunReadAccess: vi.fn(), })); const mockIssueService = vi.hoisted(() => ({ @@ -67,6 +68,7 @@ async function createApp( source: "session", isInstanceAdmin: false, }, + database: unknown = {}, ) { vi.resetModules(); const [{ errorHandler }, { activityRoutes }] = await Promise.all([ @@ -82,7 +84,7 @@ async function createApp( }; next(); }); - app.use("/api", activityRoutes({} as any)); + app.use("/api", activityRoutes(database as any)); app.use(errorHandler); return app; } @@ -118,6 +120,9 @@ describe("activity routes", () => { beforeEach(() => { for (const mock of Object.values(mockActivityService)) mock.mockReset(); for (const mock of Object.values(mockHeartbeatService)) mock.mockReset(); + mockHeartbeatService.getRun.mockImplementation(() => { + throw new Error("Run issue links must use the narrow authorization read"); + }); for (const mock of Object.values(mockIssueService)) mock.mockReset(); mockAccessService.decide.mockReset(); mockAccessService.canUser.mockReset(); @@ -344,13 +349,15 @@ describe("activity routes", () => { }); it("passes task read filtering when listing links for a readable company run", async () => { - mockHeartbeatService.getRun.mockResolvedValue({ + mockHeartbeatService.getRunReadAccess.mockResolvedValue({ id: "run-1", companyId: "company-1", scopeKind: "company", issueId: null, }); mockActivityService.issuesForRun.mockResolvedValue([{ issueId: "public-issue", title: "Visible task" }]); const app = await createApp({ type: "agent", companyId: "company-1", agentId: "agent-1" }); const res = await requestApp(app, baseUrl => request(baseUrl).get("/api/heartbeat-runs/run-1/issues")); expect(res.status).toBe(200); + expect(mockHeartbeatService.getRunReadAccess).toHaveBeenCalledWith("run-1"); + expect(mockHeartbeatService.getRun).not.toHaveBeenCalled(); const { issueReadSqlCondition } = await import("../services/authorization.js"); expect(issueReadSqlCondition).toHaveBeenCalledWith({}, expect.objectContaining({ type: "agent", companyId: "company-1", agentId: "agent-1", @@ -360,7 +367,7 @@ describe("activity routes", () => { }); it("returns 200 [] (not 404) when listing issues for another company's run, preserving API contract and the cross-tenant oracle", async () => { - mockHeartbeatService.getRun.mockResolvedValue({ + mockHeartbeatService.getRunReadAccess.mockResolvedValue({ id: "run-2", companyId: "company-2", }); @@ -374,7 +381,7 @@ describe("activity routes", () => { }); it("returns 200 [] (not 404) for a non-existent heartbeat run, matching the cross-tenant response", async () => { - mockHeartbeatService.getRun.mockResolvedValue(null); + mockHeartbeatService.getRunReadAccess.mockResolvedValue(null); const app = await createApp(); const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/heartbeat-runs/missing-run/issues")); @@ -390,6 +397,42 @@ describe("activity routes", () => { expect(res.status).toBe(401); expect(mockHeartbeatService.getRun).not.toHaveBeenCalled(); + expect(mockHeartbeatService.getRunReadAccess).not.toHaveBeenCalled(); expect(mockActivityService.issuesForRun).not.toHaveBeenCalled(); }); + + it.each([true, false])("preserves issue-scoped run authorization when read permission is %s", async (allowed) => { + mockHeartbeatService.getRunReadAccess.mockResolvedValue({ + id: "run-1", companyId: "company-1", scopeKind: "issue", issueId: "issue-private", + }); + mockAccessService.decide.mockImplementation(async ({ action }) => ({ + allowed: action === "issue:read" ? allowed : true, + })); + mockActivityService.issuesForRun.mockResolvedValue([{ issueId: "visible-issue" }]); + const db = { + select: () => ({ from: () => ({ where: async () => [{ id: "issue-private" }] }) }), + }; + const app = await createApp({ type: "agent", companyId: "company-1", agentId: "agent-1" }, db); + const res = await requestApp(app, baseUrl => request(baseUrl).get("/api/heartbeat-runs/run-1/issues")); + expect(res.status).toBe(200); + expect(res.body).toEqual(allowed ? [{ issueId: "visible-issue" }] : []); + expect(mockAccessService.decide).toHaveBeenCalledWith(expect.objectContaining({ + action: "issue:read", resource: expect.objectContaining({ companyId: "company-1", issueId: "issue-private" }), + })); + expect(mockHeartbeatService.getRunReadAccess).toHaveBeenCalledWith("run-1"); + expect(mockHeartbeatService.getRun).not.toHaveBeenCalled(); + expect(mockActivityService.issuesForRun).toHaveBeenCalledTimes(allowed ? 1 : 0); + }); + + it("does not read issue links for a run without a trusted issue binding", async () => { + mockHeartbeatService.getRunReadAccess.mockResolvedValue({ + id: "run-1", companyId: "company-1", scopeKind: "issue", issueId: null, + }); + const app = await createApp(); + const res = await requestApp(app, baseUrl => request(baseUrl).get("/api/heartbeat-runs/run-1/issues")); + expect(res.status).toBe(200); + expect(res.body).toEqual([]); + expect(mockActivityService.issuesForRun).not.toHaveBeenCalled(); + expect(mockHeartbeatService.getRun).not.toHaveBeenCalled(); + }); }); diff --git a/server/src/routes/activity.ts b/server/src/routes/activity.ts index 61e2e8b8756..5239562f7a6 100644 --- a/server/src/routes/activity.ts +++ b/server/src/routes/activity.ts @@ -372,7 +372,7 @@ export function activityRoutes(db: Db) { router.get("/heartbeat-runs/:runId/issues", async (req, res) => { assertAuthenticated(req); const runId = req.params.runId as string; - const run = await heartbeat.getRun(runId); + const run = await heartbeat.getRunReadAccess(runId); if (!run || !hasCompanyAccess(req, run.companyId)) { // Return `200 []` for both "doesn't exist" and "cross-tenant" — preserves the // legacy API contract while keeping the cross-tenant existence oracle closed