From d794fdd97907de9ae1849ca5ea468e222cef8fdc Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 17 Jul 2026 02:38:11 +0000 Subject: [PATCH] fix(audit): log only successful sensitive reads Co-Authored-By: Paperclip --- .../agent-instructions-routes.test.ts | 8 +++++++ .../__tests__/agent-live-run-routes.test.ts | 8 ++++++- .../__tests__/issue-attachment-routes.test.ts | 4 ++++ server/src/__tests__/tool-gateway.test.ts | 23 +++++++++++++++++++ server/src/routes/issues.ts | 4 ++-- server/src/routes/tool-gateway.ts | 2 +- 6 files changed, 45 insertions(+), 4 deletions(-) diff --git a/server/src/__tests__/agent-instructions-routes.test.ts b/server/src/__tests__/agent-instructions-routes.test.ts index 5fca425834d..a7c0adb8966 100644 --- a/server/src/__tests__/agent-instructions-routes.test.ts +++ b/server/src/__tests__/agent-instructions-routes.test.ts @@ -67,6 +67,10 @@ vi.mock("../services/environments.js", () => ({ environmentService: () => mockEnvironmentService, })); +vi.mock("../services/activity-log.js", () => ({ + logActivity: mockLogActivity, +})); + vi.mock("../adapters/index.js", () => ({ findServerAdapter: mockFindServerAdapter, listAdapterModels: vi.fn(), @@ -98,6 +102,10 @@ function registerModuleMocks() { environmentService: () => mockEnvironmentService, })); + vi.doMock("../services/activity-log.js", () => ({ + logActivity: mockLogActivity, + })); + vi.doMock("../adapters/index.js", () => ({ findServerAdapter: mockFindServerAdapter, listAdapterModels: vi.fn(), diff --git a/server/src/__tests__/agent-live-run-routes.test.ts b/server/src/__tests__/agent-live-run-routes.test.ts index f611dfa66ef..ed29911411e 100644 --- a/server/src/__tests__/agent-live-run-routes.test.ts +++ b/server/src/__tests__/agent-live-run-routes.test.ts @@ -28,6 +28,8 @@ const mockInstanceSettingsService = vi.hoisted(() => ({ listCompanyIds: vi.fn(), })); +const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined)); + const routeAgentId = "11111111-1111-4111-8111-111111111111"; function registerModuleMocks() { @@ -49,6 +51,10 @@ function registerModuleMocks() { issueService: () => mockIssueService, })); + vi.doMock("../services/activity-log.js", () => ({ + logActivity: mockLogActivity, + })); + vi.doMock("../services/index.js", () => ({ agentService: () => mockAgentService, agentInstructionsService: () => ({}), @@ -69,7 +75,7 @@ function registerModuleMocks() { heartbeatService: () => mockHeartbeatService, issueApprovalService: () => ({}), issueService: () => mockIssueService, - logActivity: vi.fn(), + logActivity: mockLogActivity, secretService: () => ({}), syncInstructionsBundleConfigFromFilePath: vi.fn((_agent, config) => config), workspaceOperationService: () => ({}), diff --git a/server/src/__tests__/issue-attachment-routes.test.ts b/server/src/__tests__/issue-attachment-routes.test.ts index 2103aa58809..948fc1d3533 100644 --- a/server/src/__tests__/issue-attachment-routes.test.ts +++ b/server/src/__tests__/issue-attachment-routes.test.ts @@ -597,6 +597,10 @@ describe("issue attachment routes", () => { expect(res.status).toBe(416); expect(res.headers["content-range"]).toBe("bytes */4"); expect(storage.getObject).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ action: "attachment.downloaded" }), + ); }); it("rejects cross-company attachment content reads", async () => { diff --git a/server/src/__tests__/tool-gateway.test.ts b/server/src/__tests__/tool-gateway.test.ts index 5a7d4cc0a36..ee52d730140 100644 --- a/server/src/__tests__/tool-gateway.test.ts +++ b/server/src/__tests__/tool-gateway.test.ts @@ -3265,6 +3265,29 @@ rl.on("line", (line) => { ]); }); + it("does not audit rejected tool gateway audit filters as successful reads", async () => { + const company = await createCompany(db); + const app = createGatewayRouteApp(db, createTestToolGatewayService(db), { + type: "board", + userId: "instance-admin", + source: "session", + companyIds: [company.id], + memberships: [{ companyId: company.id, membershipRole: "owner", status: "active" }], + isInstanceAdmin: true, + }); + + const response = await request(app) + .get("/api/tool-gateway/audit") + .query({ companyId: company.id, app: "not-a-uuid" }); + + expect(response.status).toBe(400); + const rows = await db + .select({ action: activityLog.action }) + .from(activityLog) + .where(and(eq(activityLog.companyId, company.id), eq(activityLog.action, "tool.audit_read"))); + expect(rows).toEqual([]); + }); + it("rejects durable sessions after the heartbeat run is no longer active", async () => { const company = await createCompany(db); const agent = await createAgent(db, company.id); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index e1955c8fc79..c31a98a8ebd 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -10719,8 +10719,6 @@ export function issueRoutes( } if (!(await assertIssueReadAllowed(req, res, issue))) return; - await logRouteActivity(db, req, { companyId: attachment.companyId, action: "attachment.downloaded", entityType: "issue_attachment", entityId: attachment.id, issueId: attachment.issueId, details: { byteSize: attachment.byteSize, contentType: attachment.contentType } }); - const contentLength = attachment.byteSize; const range = parseAttachmentRangeHeader( typeof req.headers.range === "string" ? req.headers.range : undefined, @@ -10733,6 +10731,8 @@ export function issueRoutes( return; } + await logRouteActivity(db, req, { companyId: attachment.companyId, action: "attachment.downloaded", entityType: "issue_attachment", entityId: attachment.id, issueId: attachment.issueId, details: { byteSize: attachment.byteSize, contentType: attachment.contentType } }); + const object = await storage.getObject( attachment.companyId, attachment.objectKey, diff --git a/server/src/routes/tool-gateway.ts b/server/src/routes/tool-gateway.ts index 226fa96b82e..5355dcc81dd 100644 --- a/server/src/routes/tool-gateway.ts +++ b/server/src/routes/tool-gateway.ts @@ -651,7 +651,6 @@ export function toolGatewayRoutes(db: Db, toolGateway: ToolGatewayService) { const windowFilter = typeof req.query.window === "string" ? req.query.window.trim() : "24h"; const searchRaw = typeof req.query.search === "string" ? req.query.search.trim() : null; const cursorRaw = typeof req.query.cursor === "string" ? req.query.cursor.trim() : null; - await logRouteActivity(db, req, { companyId, action: "tool.audit_read", entityType: "company", entityId: companyId, details: { limit } }); if (appFilter && !uuidPattern.test(appFilter)) { res.status(400).json({ error: "app must be an applicationId or connectionId UUID" }); return; @@ -819,6 +818,7 @@ export function toolGatewayRoutes(db: Db, toolGateway: ToolGatewayService) { }); const last = visible.at(-1)?.row; + await logRouteActivity(db, req, { companyId, action: "tool.audit_read", entityType: "company", entityId: companyId, details: { limit } }); res.json({ events, nextCursor: hasMore && last ? encodeAuditCursor({ createdAt: last.createdAt, id: last.id }) : null,