The audit log is best-effort in both apps while the docs say all actions are logged #3571

Hi,

I run independent verification of AI-built codebases (Trenyx; the published engagements are at https://trenyx.io/audits.html) and did a pre-registered static read of trycompai/comp at 2b25584, the plan hashed and anchored before reading. The authorization story held everywhere I looked: the organization comes from the session and a live membership check at every entry (apps/api/src/auth/hybrid-auth.guard.ts:186-222, apps/app/src/actions/safe-action.ts:150-170), PermissionGuard re-derives the role server-side per route (apps/api/src/auth/permission.guard.ts:47-176), integration credentials sit under AES-256-GCM with a key the process refuses to run without (apps/api/src/integration-platform/services/credential-vault.service.ts, apps/api/src/secrets/encryption.util.ts:26-28), the MCP tools proxy the same guarded REST endpoints, and no fallback secret exists in code. Nothing here is a security report. This is about the audit trail, which for a compliance-evidence product is the record itself.

The audit write is fire-and-forget on both surfaces. In the API, AuditLogInterceptor runs void this.persist(...).catch(err => this.logger.error(...)) inside the response tap (apps/api/src/audit/audit-log.interceptor.ts:284-300), so the client has its success before the row is attempted, and a failed write is a log line with no counter, alert, retry or caller signal. The interceptor's own spec pins that behaviour (audit-log.interceptor.spec.ts:548-567, "should handle db errors gracefully without throwing"). Every server action in the Next.js app does the same: apps/app/src/actions/safe-action.ts:207-221 wraps db.auditLog.create in a try/catch, logs, and continues to next(). The interceptor is registered globally, and a handful of @RequirePermission mutations opt out of it with @SkipAuditLog (the email send routes, the assistant-chat history routes); authActionClient is the base of every Next.js action with no equivalent opt-out. So the pattern covers the large majority of mutations in the product.

I read this as a posture rather than an oversight: four other sites call their audit writes best-effort in their own comments (apps/api/src/admin-organizations/purge-organization.service.ts:134, apps/api/src/security-penetration-tests/pentest-credits.service.ts:306 and :368, apps/api/src/vendors/vendors.service.ts:464, apps/api/src/trigger/vendor/vendor-risk-assessment-task.ts:534). The gap is between that posture and what a customer is told: packages/docs/mcp-server.mdx:295 says "All actions are logged." Under a transient database error the mutation lands, the audit row does not, the caller sees success, and nothing counts the miss. Actor attribution is not the issue; the actor comes from the session on both paths.

Two suggestions, either of which closes it. Count and alert on a failed audit write (a metric next to the existing error log, so an operator can see a day with missing rows), or offer a fail-closed mode for tenants who need the trail to be complete, where a failed audit write fails the mutation. And qualify the docs line to match whichever you choose.

Three smaller things from the same read, none of them findings:

1. apps/portal/src/actions/accept-policies.ts is an unreferenced server action with no session check and a caller-supplied memberId; the live path is apps/portal/src/app/api/portal/accept-policies/route.ts:14-38, which verifies the member belongs to the session user. Worth deleting the dead file before someone imports it.
2. Webhook signature verification is gated on webhookConfig.secretHeader && webhookConfig.signatureAlgorithm (apps/api/src/integration-platform/controllers/webhook.controller.ts:102), and both fields are optional in WebhookConfigSchema (packages/integration-platform/src/types.ts:228-230). No shipped manifest declares a webhook today, so nothing is exposed; a future manifest that omits either field would accept unsigned posts for its connection. Making both required whenever the webhook capability is declared closes that in the schema.
3. apps/app/src/app/api/revalidate/path/route.ts:9 compares REVALIDATION_SECRET with !==, while webhook.controller.ts:44-49 uses crypto.timingSafeEqual. Hygiene only, the secret is long and random.

Happy to re-check whatever you ship. The pre-registration hash and the full record publish only after that, and the write-up says you fixed it.

SK
Trenyx