From a1237f9560f6d02056afb4ded29954af295c2020 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 17 Jul 2026 02:18:08 +0000 Subject: [PATCH] fix(server): address agent audit review findings Co-Authored-By: Paperclip --- .../agent-action-audit-routes.test.ts | 9 +++++++++ server/src/routes/activity.ts | 9 ++++++--- server/src/services/activity-log.ts | 12 +++++++++--- server/src/services/agent-action-audit.ts | 19 +++++++++++++------ 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/server/src/__tests__/agent-action-audit-routes.test.ts b/server/src/__tests__/agent-action-audit-routes.test.ts index ac1a53b4540..0dc49d239d2 100644 --- a/server/src/__tests__/agent-action-audit-routes.test.ts +++ b/server/src/__tests__/agent-action-audit-routes.test.ts @@ -128,6 +128,15 @@ describePostgres("agent action audit routes", () => { expect(boardResponse.body.error).toContain("audit:view_agent_actions"); }); + it("returns a client error for invalid audit query parameters", async () => { + const { company } = await seed(); + const response = await request(await createApp(db, { + type: "board", userId: "local-board", companyIds: [company.id], source: "local_implicit", isInstanceAdmin: false, + })).get(`/api/companies/${company.id}/audit/agent-actions?limit=invalid`); + expect(response.status).toBe(400); + expect(response.body.error).toBe("Invalid agent action audit query"); + }); + it("paginates, filters, enriches entities, and falls back to the run responsible user", async () => { const { company, agent, otherAgent, issue, comment, issueDocument, run } = await seed(); const app = await createApp(db, { type: "board", userId: "local-board", companyIds: [company.id], source: "local_implicit", isInstanceAdmin: false }); diff --git a/server/src/routes/activity.ts b/server/src/routes/activity.ts index 88b1f0928c1..c3dc2951cd9 100644 --- a/server/src/routes/activity.ts +++ b/server/src/routes/activity.ts @@ -7,7 +7,7 @@ import { activityService, normalizeActivityLimit } from "../services/activity.js import { assertAuthenticated, assertBoard, assertCompanyAccess, getAccessibleResource, hasCompanyAccess } from "./authz.js"; import { accessService, heartbeatService, issueService } from "../services/index.js"; import { sanitizeRecord } from "../redaction.js"; -import { forbidden } from "../errors.js"; +import { badRequest, forbidden } from "../errors.js"; import { agentActionAuditService } from "../services/agent-action-audit.js"; const createActivitySchema = z.object({ @@ -116,9 +116,12 @@ export function activityRoutes(db: Db) { router.get("/companies/:companyId/audit/agent-actions", async (req, res) => { const companyId = req.params.companyId as string; await assertAgentAuditPermission(req, companyId); - const query = agentActionAuditQuerySchema.parse(req.query); + const parsedQuery = agentActionAuditQuerySchema.safeParse(req.query); + if (!parsedQuery.success) { + throw badRequest("Invalid agent action audit query", parsedQuery.error.issues); + } try { - res.json(await agentAudit.list({ companyId, ...query })); + res.json(await agentAudit.list({ companyId, ...parsedQuery.data })); } catch (error) { if (error instanceof Error && error.message === "Invalid audit cursor") { res.status(400).json({ error: error.message }); diff --git a/server/src/services/activity-log.ts b/server/src/services/activity-log.ts index eb821f23011..981d7ce153f 100644 --- a/server/src/services/activity-log.ts +++ b/server/src/services/activity-log.ts @@ -65,12 +65,18 @@ export interface LogActivityInput { details?: Record | null; } -export async function redactActivityDetails(db: Db, details: Record | null) { - if (!details) return null; +export async function createActivityDetailsRedactor(db: Db) { const currentUserRedactionOptions = { enabled: (await instanceSettingsService(db).getGeneral()).censorUsernameInLogs, }; - return redactCurrentUserValue(sanitizeRecord(details), currentUserRedactionOptions); + return (details: Record | null) => ( + details ? redactCurrentUserValue(sanitizeRecord(details), currentUserRedactionOptions) : null + ); +} + +export async function redactActivityDetails(db: Db, details: Record | null) { + if (!details) return null; + return (await createActivityDetailsRedactor(db))(details); } function readNonEmptyString(value: unknown) { diff --git a/server/src/services/agent-action-audit.ts b/server/src/services/agent-action-audit.ts index 312b84a8687..93f812b458e 100644 --- a/server/src/services/agent-action-audit.ts +++ b/server/src/services/agent-action-audit.ts @@ -1,7 +1,7 @@ -import { and, desc, eq, gte, inArray, isNotNull, lt, lte, or, sql } from "drizzle-orm"; +import { and, desc, eq, gte, inArray, isNotNull, isNull, lt, lte, or, sql } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { activityLog, heartbeatRuns, issueComments, issueDocuments, issues } from "@paperclipai/db"; -import { redactActivityDetails } from "./activity-log.js"; +import { createActivityDetailsRedactor } from "./activity-log.js"; export interface AgentActionAuditFilters { companyId: string; @@ -49,7 +49,13 @@ export function agentActionAuditService(db: Db) { const effectiveResponsibleUserId = sql`coalesce(${activityLog.responsibleUserId}, ${heartbeatRuns.responsibleUserId})`; const conditions = [eq(activityLog.companyId, filters.companyId), isNotNull(activityLog.agentId)]; if (filters.agentId) conditions.push(eq(activityLog.agentId, filters.agentId)); - if (filters.responsibleUserId) conditions.push(eq(effectiveResponsibleUserId, filters.responsibleUserId)); + if (filters.responsibleUserId) conditions.push(or( + eq(activityLog.responsibleUserId, filters.responsibleUserId), + and( + isNull(activityLog.responsibleUserId), + eq(heartbeatRuns.responsibleUserId, filters.responsibleUserId), + ), + )!); if (filters.runId) conditions.push(eq(activityLog.runId, filters.runId)); if (filters.entityType) conditions.push(eq(activityLog.entityType, filters.entityType)); if (filters.entityId) conditions.push(eq(activityLog.entityId, filters.entityId)); @@ -108,7 +114,8 @@ export function agentActionAuditService(db: Db) { documents.set(row.documentId, row); } - const items = await Promise.all(page.map(async (row) => { + const redactDetails = await createActivityDetailsRedactor(db); + const items = page.map((row) => { const comment = comments.get(row.entityId); const issue = issueMap.get(row.entityId); const document = documents.get(row.entityId); @@ -119,14 +126,14 @@ export function agentActionAuditService(db: Db) { : issue ? { id: issue.id, identifier: issue.identifier, title: issue.title } : null; return { ...row, - details: await redactActivityDetails(db, row.details), + details: redactDetails(row.details), entity: { issue: issueSnippet, comment: comment ? { id: comment.id, excerpt: excerpt(comment.body) } : null, document: document ? { id: document.documentId, key: document.key } : null, }, }; - })); + }); const last = page.at(-1); return { items,