feat(observability): extend audit log, PostHog, and storage metering coverage#5269
feat(observability): extend audit log, PostHog, and storage metering coverage#5269waleedlatif1 wants to merge 13 commits into
Conversation
…coverage Instruments previously-uncaptured resources and actions across the three observability layers (audit log, PostHog product analytics, usage metering): - Exfiltration audit: file downloads (workspace/public-share/v1), table, workflow, and workspace exports. - Credential access audited at the token-issuance boundary (success-only). - Full revenue trail: invoice paid/failed, overage billed, disputes, credit fulfillment, subscription lifecycle, plan/seat changes. - Session/login lifecycle audit via Better Auth hooks (login, blocked sign-in, logout, session revoke, account delete). - v1/admin programmatic surface and copilot tool handlers instrumented. - Dead audit/PostHog constants wired (lock/unlock, table, custom tool, etc.). - Storage metering extended to KB documents and copilot files. - PostHog person identify + workspace/organization group hygiene. Audit package hardened to null the actor FK for system actors (admin-api) and adds an awaitable recordAuditNow for pre-delete hooks. All instrumentation is fire-and-forget and never blocks or breaks the primary operation.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Better Auth gains before/after hooks for blocked sign-in/sign-up, successful logins (gated to real sign-in paths), sign-out, session revocation, and an awaited PostHog adds typed revenue/org/credential/copilot events and enriches client workspace/organization group properties in Storage metering now checks quota and increments on KB document create (post-commit), decrements in the same transaction as hard deletes via Reviewed by Cursor Bugbot for commit f0a18ef. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR extends three observability layers — audit log, PostHog analytics, and storage metering — to cover previously uninstrumented surfaces including file downloads, credential access, billing events, and session lifecycle. It also hardens the audit package to null actor FKs for system/unknown actors instead of FK-violating silently, and introduces
Confidence Score: 5/5Safe to merge — all instrumentation is fire-and-forget and the primary operations are unchanged; the storage metering path is atomic and idempotent. Every audit write is non-blocking (fire-and-forget recordAudit, or a best-effort try/catch wrapper), so a failure in any of the new observability paths cannot break payment processing, file operations, or auth flows. The storage metering changes are the most structurally significant: releaseDeletedFileStorage uses an idempotency claim row to prevent double-decrements, and decrementStorageUsageInTx keeps the counter and the row deletion in the same transaction. The actor FK nulling fix for system actors is well-tested with new regression tests. The previously-flagged issues (GET audit ordering, actor misattribution on public shares, copilot delete ordering, catch-branch FK violation) are all resolved in this revision. No files require special attention — all changed files are observability additions over existing, well-tested business logic. Important Files Changed
Reviews (12): Last reviewed commit: "fix(audit): null actor FK when the user ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Address Cursor Bugbot review: - GET OAuth token: emit CREDENTIAL_ACCESSED/credential_used after refreshTokenIfNeeded succeeds (matches POST), not before. - File export: audit on each actual success exit via a shared helper — including the non-markdown serve redirect (previously unaudited) and after the zip is generated (previously before asset fetch).
Address Greptile P1: setting actorId to the file owner made every anonymous external download read as a self-download, undermining the exfiltration trail. recordAudit now accepts a null actor; the public content/inline routes record actorId=null with the owner in metadata.sharedByUserId and rely on ip/user-agent for the forensic trail. The misleading owner-attributed file_downloaded PostHog event is dropped on these anonymous paths.
|
@greptile |
|
@cursor review |
- Admin workflow/workspace ZIP exports now audit only after the archive is built (via a local helper), so a zip/build failure no longer logs an export. - Remove redundant 'success-only placement' inline comments and tighten the remaining design-rationale notes (export helper now TSDoc).
|
@greptile |
|
@cursor review |
Address Cursor review: - handleDisputeClosed now records CHARGE_DISPUTE_CLOSED for every closed dispute (won/lost/warning_closed), unblocking only on favorable outcomes. dispute.status in the metadata distinguishes the outcome, so lost chargebacks are no longer missing from the trail. - Threshold overage now emits OVERAGE_BILLED + overage_billed even when credits fully cover the overage (settledVia: 'credits' vs 'stripe'), so credit-settled overages are audited instead of silently returning null.
Address Greptile P1: deleting the metadata row before the decrement meant a decrement failure left the quota permanently inflated with no record to retry from. Decrement first; only remove the metadata row once it succeeds.
|
@greptile |
|
@cursor review |
…tion - Copilot file delete now releases storage via a single transaction (releaseDeletedFileStorage): the soft-delete is the idempotency claim and the decrement shares the transaction, so neither a partial failure (inflated counter) nor a retry (double-decrement) can desync the quota. Resolves the conflicting Greptile/Cursor ordering findings. - Policy-blocked sign-ups now record USER_SIGNUP_BLOCKED instead of USER_SIGNIN_BLOCKED, so account-lifecycle events aren't mislabeled.
|
@cursor review |
Address Cursor review: when every requested workflow fails to load, the admin export still returned 200 but recorded a successful export of zero workflows. Guard auditExport so an empty result records nothing.
|
@cursor review |
Address Cursor review: the blob was removed before releaseDeletedFileStorage, so a release failure left the counter inflated and the metadata active with the blob gone. Now the atomic soft-delete + decrement runs first and the blob is deleted only if it succeeds, so a failure leaves the file fully intact and retryable.
|
@cursor review |
|
@cursor review |
Address Cursor review: hardDeleteDocuments deleted the rows then decremented best-effort, so a decrement failure left billed storage inflated with no row to reconcile. Resolve each owner's subscription up front, then decrement inside the same transaction that deletes the embeddings/documents (decrementStorageUsageInTx, also now shared by releaseDeletedFileStorage), so the counter and the content commit or roll back together. Connector docs remain excluded.
|
@cursor review |
Address Cursor review: /verify-email could emit USER_LOGIN when verification mints or refreshes a session, mislabeling (or double-counting) a non-sign-in as a login. Restrict isLoginPath to genuine sign-in entrypoints.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 59ee511. Configure here.
Address Greptile P1: the catch branch left the original actorId, so a system actor like 'admin-api' (or a since-deleted user) would FK-violate the insert and lose the row when the existence lookup errored. Mirror the not-found branch — null the FK with a readable label — so the audit row always persists.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f0a18ef. Configure here.
1 participant
Summary
Extends the three observability layers — audit log, PostHog product analytics, and usage metering — to cover resources and actions that were previously uninstrumented. Found via a codebase-wide coverage audit; every gap below was a real blind spot.
Audit log
CREDENTIAL_ACCESSED), success-only.v1/adminprogrammatic surface (member/role/export ops) and copilot tool handlers (skills, custom tools, tables) — both previously bypassed audit entirely.AuditActionconstants (lock/unlock, table update/delete, etc.).PostHog
Metering
Hardening
admin-api) with a readable label instead of silently FK-failing; adds an awaitablerecordAuditNowfor pre-delete hooks.Type of Change
Testing
check:api-validationpass.Checklist