feat(server): add permission-gated agent action audit API#9735
feat(server): add permission-gated agent action audit API#9735cryppadotta wants to merge 7 commits into
Conversation
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
✅ All checks passing — ready for Greptile review and maintainer approval. — commitperclip |
|
@greptile review |
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Security finding: information disclosure via audit entity enrichment in server/src/services/agent-action-audit.ts lines 87 to 126. The enrichment pass keys entirely on entityId and never applies the hidden issue visibility gate before returning issue, comment, or document snippets. I reproduced two cases against the PR head in a scratch worktree.
Blast radius: any board principal with audit:view_agent_actions can recover hidden issue or comment metadata from same-company audit rows, and the feed can misattribute unrelated rows to issue, comment, or document snippets. Concrete fix:
I checked the other scoped areas during review. Company scoping, explicit board permission gating, cursor decoding, redaction reuse, and legacy responsible-user fallback looked consistent with the surrounding route and service patterns. |
Greptile SummaryThis PR adds a permission-gated
Confidence Score: 5/5Safe to merge — all previously flagged issues (N+1 settings queries, cursor precision loss, cursor UUID validation, ZodError 500 responses) are resolved in this version, and the permission gate correctly rejects agent actors and unpermissioned board users. The new audit route is correctly gated, uses typed errors throughout, and the cursor encoding now preserves microsecond timestamps so pagination cannot silently drop rows. The bulk enrichment queries are clean — one settings fetch per request, three bounded IN queries per page — and the comprehensive test suite exercises the key correctness and security paths end-to-end. No files require special attention; the logic in agent-action-audit.ts and the refactored activity-log.ts are consistent with each other and with the existing codebase patterns. Important Files Changed
Reviews (6): Last reviewed commit: "fix(server): preserve audit cursor preci..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR adds a permission-gated
Confidence Score: 3/5The authorization boundary and pagination logic look correct; the main concern is a per-row database call to the instance settings table that fires on every audit request with populated details fields. The redactActivityDetails call inside Promise.all issues a fresh SELECT against instanceSettings for each row with non-null details — instanceSettingsService(db).getGeneral() has no caching and hits the database on every invocation. With up to 200 rows per page this creates a burst of identical queries on every audit request. This should be fixed before the endpoint sees production load. server/src/services/agent-action-audit.ts — the Promise.all / redactActivityDetails interaction and the responsibleUserId COALESCE filter both warrant a close look. Important Files Changed
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
server/src/services/agent-action-audit.ts:111-129
**N+1 DB queries for instance settings per page item**
`redactActivityDetails` calls `instanceSettingsService(db).getGeneral()` which issues a raw `SELECT` against the `instanceSettings` table on every invocation — there is no caching in that service. Inside `Promise.all`, this fires up to N concurrent identical SELECT queries (one per row with non-null `details`) for the same singleton row on every audit request. Fetch the setting once before the map and pass it inline.
```suggestion
const { censorUsernameInLogs } = await instanceSettingsService(db).getGeneral();
const items = page.map((row) => {
const comment = comments.get(row.entityId);
const issue = issueMap.get(row.entityId);
const document = documents.get(row.entityId);
const issueSnippet = comment
? { id: comment.issueId, identifier: comment.identifier, title: comment.title }
: document
? { id: document.issueId, identifier: document.identifier, title: document.title }
: issue ? { id: issue.id, identifier: issue.identifier, title: issue.title } : null;
const redactedDetails = row.details
? redactCurrentUserValue(sanitizeRecord(row.details), { enabled: censorUsernameInLogs })
: null;
return {
...row,
details: redactedDetails,
entity: {
issue: issueSnippet,
comment: comment ? { id: comment.id, excerpt: excerpt(comment.body) } : null,
document: document ? { id: document.documentId, key: document.key } : null,
},
};
});
```
### Issue 2 of 3
server/src/routes/activity.ts:119
**ZodError from query parsing returns 500, not 400**
`agentActionAuditQuerySchema.parse(req.query)` sits outside the `try/catch` block, so any `ZodError` (e.g., a non-integer `limit` or an invalid UUID for `agentId`) propagates uncaught to the global error handler. If that handler doesn't recognise `ZodError`, the caller gets a 500 instead of a 400. The existing `POST` route avoids this by using the `validate(schema)` middleware — the same approach should be applied here, or the `parse` call moved inside the `try/catch`.
### Issue 3 of 3
server/src/services/agent-action-audit.ts:52
**`responsibleUserId` filter on COALESCE expression prevents index use**
`eq(effectiveResponsibleUserId, filters.responsibleUserId)` puts a computed SQL expression in the `WHERE` clause. PostgreSQL cannot use a plain B-tree index on `activity_log.responsible_user_id` to satisfy this filter and will fall back to a sequential scan over all agent-attributed rows for the company. On large tables this query can be slow. Consider filtering directly on `activityLog.responsibleUserId` (and accepting that legacy rows without a stored value are excluded from this filter path), or adding a partial functional index if fallback attribution is required.
Reviews (2): Last reviewed commit: "chore: retrigger PR metadata review" | Re-trigger Greptile |
Co-Authored-By: Paperclip <noreply@paperclip.ing>
![superagent-security[bot]](https://avatars.githubusercontent.com/in/3287076?s=60&v=4){kind=small}
superagent-security
Bot
added
the
pr:flagged
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Addressed the security finding in
Focused verification passes: server typecheck and 5/5 audit-route tests. |
superagent-security
Bot
removed
the
pr:flagged
|
Security review on head I re-checked the scoped risk areas after the
I did not find a remaining cross-company or hidden-issue disclosure on this head. Current merge readiness from a security perspective:
Residual risk:
|
|
Security finding: information disclosure via hidden-issue details on The entity-enrichment fix in
Attack / code path:
Blast radius: any same-company board user with Concrete fix:
As of July 17, 2026 at 02:30 UTC, the latest PR head is still not review-ready anyway: |
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Security follow-upFixed the remaining hidden-issue
A fresh SecurityEngineer re-review is queued on the exact current head. |
|
@greptile review |
1 participant
Thinking Path
Linked Issues or Issue Description
Subsystem affected
server/— REST API and orchestration services, plus the shared permission-key contract.Problem or motivation
Board operators cannot retrieve a unified feed of agent actions with responsible-user attribution and entity context. Existing activity reads lack the dedicated permission boundary, cursor pagination, filter set, and render-ready comment/issue/document snippets needed by audit views.
Proposed solution
Add
GET /api/companies/:companyId/audit/agent-actions, gated byaudit:view_agent_actions, with stable cursor pagination, agent/responsible-user/run/entity/action/time/actor filters, read-time redaction, heartbeat-run fallback attribution, and bulk entity enrichment.Alternatives considered
Extending the broad company activity endpoint would preserve overly broad authorization semantics and still require UI-side N+1 fetches. A separate audit route makes the sensitive permission boundary explicit.
Roadmap alignment
Supports the roadmap direction toward durable audit trails and explicit review workflows; no duplicate open pull request was found.
Additional context
The response is designed for a board audit UI that needs to render entries such as an agent commenting on an issue without issuing follow-up entity requests.
What Changed
audit:view_agent_actionspermission key.(createdAt,id)cursor-paginated audit query with all requested filters.heartbeat_runsfor legacy rows.Verification
pnpm --filter @paperclipai/server typecheckpnpm exec vitest run server/src/__tests__/agent-action-audit-routes.test.tsRisks
Model Used
Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template#NNN/github.com/paperclipai/paperclipURLs)docs/...,fix/...) and contains no internal Paperclip ticket id or instance-derived details