fix(audit): log only successful sensitive reads

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

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(),

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: () => ({}),

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 () => {

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);

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,

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,