Skip to content

feat(observability): extend audit log, PostHog, and storage metering coverage#5269

Open
waleedlatif1 wants to merge 29 commits into
stagingfrom
worktree-audit-posthog-coverage
Open

feat(observability): extend audit log, PostHog, and storage metering coverage#5269
waleedlatif1 wants to merge 29 commits into
stagingfrom
worktree-audit-posthog-coverage

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

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

  • Exfiltration trail (none existed): file downloads (workspace, public-share, v1 API), and table / workflow / workspace exports.
  • Credential access audited at the token-issuance boundary (CREDENTIAL_ACCESSED), success-only.
  • Full revenue/financial trail: invoice paid/failed, overage billed, charge disputes, credit fulfillment, subscription create/cancel/transfer, enterprise provisioning, plan/seat changes.
  • Session/account lifecycle via Better Auth hooks: login, blocked sign-in, logout, session revoke, account delete.
  • The entire v1/admin programmatic surface (member/role/export ops) and copilot tool handlers (skills, custom tools, tables) — both previously bypassed audit entirely.
  • Wired several defined-but-dead AuditAction constants (lock/unlock, table update/delete, etc.).

PostHog

  • Revenue events (payments, overage, credits, disputes, plan conversion, enterprise), org/seat lifecycle, KB search, copilot/skill/tool events.
  • Client-side workspace + organization group association so events roll up by account.

Metering

  • Storage now counted for KB documents and copilot files (was workspace-files only). Connector-synced KB docs remain correctly unmetered on both ingest and delete.

Hardening

  • Audit package nulls the actor FK for system actors (admin-api) with a readable label instead of silently FK-failing; adds an awaitable recordAuditNow for pre-delete hooks.
  • Every billing-webhook actor lookup is guarded so instrumentation can never fail payment processing.
  • All instrumentation is fire-and-forget and never blocks or breaks the primary operation. Ephemeral execution-file storage is intentionally not metered (no decrement path → would leak).

Type of Change

  • New feature (observability coverage)

Testing

  • Full typecheck, biome, and check:api-validation pass.
  • 2,617 touched-area tests + 22 audit-package tests pass.
  • Validated by a multi-agent adversarial review across every changed domain (FK safety, fire-and-forget, no double-emit, storage symmetry, behavior-preserving refactors).

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)
…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.
@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 30, 2026 2:41am

Request Review

@cursor

cursor Bot commented Jun 29, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Large additive surface across credentials, billing webhooks, and storage accounting; changes are mostly fire-and-forget, but KB storage quota/decrement in transactions and billing instrumentation still warrant careful review for double-counting or missed rollbacks.

Overview
Adds audit log and PostHog instrumentation across previously unlogged paths: OAuth/credential token issuance, file downloads and exports (workspace, public share, v1 API, admin bulk export), table exports, workflow public API and lock toggles, org/billing lifecycle (plan switch, seat reconcile, subscription webhooks, threshold overage, disputes, credits), admin v1 APIs, copilot chat, and copilot table/skill/custom-tool handlers. Realtime debounced workflow variable writes now record WORKFLOW_VARIABLES_UPDATED with the last editor as actor.

Audit package accepts actorId: null for anonymous events and nulls the user FK for missing users or admin-api, with display labels instead of FK failures.

Storage metering for knowledge-base uploads checks quota before insert and increments after commit; hard deletes decrement bytes in the same transaction via new decrementStorageUsageInTx (connector-synced docs still excluded). Table rename/delete/archive and import column adds gain actor-aware audit hooks.

PostHog event map grows (revenue, org/seat, credentials, schedules, etc.); WorkspaceScopeSync sets organization and workspace groups together once workspace metadata loads. Seat reconciliation and invite acceptance pass actorId into centralized seat audit/analytics.

Reviewed by Cursor Bugbot for commit 2afd114. Configure here.

Comment thread apps/sim/app/api/auth/oauth/token/route.ts
Comment thread apps/sim/app/api/files/export/[id]/route.ts Outdated
Comment thread apps/sim/app/api/files/export/[id]/route.ts Outdated
@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends the three observability layers — audit log, PostHog analytics, and storage metering — to cover a comprehensive set of previously uninstrumented events: billing webhooks (invoices, disputes, subscriptions, enterprise), credential access, copilot tool/skill/table management, file downloads/exports, and the entire v1 admin API surface. The audit package is also hardened to null actor FKs for system actors and on user-lookup failures, with a new recordAuditNow helper and corresponding tests.

  • Audit + PostHog coverage: Dozens of new event types added across billing (invoice paid/failed, overage, disputes, subscription lifecycle), organization/seat management, credential access, copilot operations, and workflow lock/export/deploy actions.
  • Storage metering for KB documents: hardDeleteDocuments now runs decrementStorageUsageInTx inside the delete transaction using pre-resolved subscriptions, so storage counters stay atomic with hard-deletes; connector-synced documents remain correctly unmetered.
  • Audit package hardening: actorId is nulled (with a readable actorName label) whenever the user-table lookup returns no row or throws, preventing silent FK violations; recordAuditNow provides an awaitable variant for pre-delete hooks.

Confidence Score: 5/5

Safe to merge — all instrumentation is fire-and-forget, all actor-resolution helpers have internal try/catch so they cannot throw into critical billing paths, and the storage accounting changes use existing transaction boundaries.

The billing-webhook instrumentation blocks that were previously identified as potentially blocking critical operations (payment-failed user-blocking, public-share actor misattribution, actor FK violations on catch paths) have all been addressed with try/catch guards, null-actor paths, and regression tests. Storage metering for KB document deletes is correctly atomic with the delete transaction via decrementStorageUsageInTx. The GET-handler credential audit is correctly placed after refreshTokenIfNeeded succeeds. No new unguarded awaits were introduced on critical webhook paths.

No files require special attention — the most sensitive paths (billing webhooks, storage counters, audit FK safety) were reviewed in depth and follow the defensive patterns described in the PR.

Important Files Changed

Filename Overview
packages/audit/src/log.ts FK-null hardening for system actors: catch branch now mirrors the not-found branch, nulling actorId and assigning a readable label; both code paths and the new actorId: null API for anonymous events are covered by tests.
apps/sim/lib/knowledge/documents/service.ts Storage accounting for KB documents added; createDocumentRecords/createSingleDocument call checkStorageQuota inside the transaction and incrementStorageUsage outside; hardDeleteDocuments resolves subscriptions before the transaction and calls decrementStorageUsageInTx inside the delete transaction, keeping counters atomic with row deletes; connector-synced docs correctly excluded from metering.
apps/sim/lib/billing/webhooks/invoices.ts payment_succeeded instrumentation added after business logic; payment_failed instrumentation wrapped in its own try/catch so a DB error during actor resolution can never skip the user-blocking block that follows; resolveBillingActorId has internal try/catch and never throws.
apps/sim/lib/billing/webhooks/subscription.ts Subscription create/cancel lifecycle now audited; resolveSubscriptionActorId has internal try/catch so it can never throw at the call site; for personal subscriptions referenceId IS the user ID so the query finds no org member and correctly falls back to the user ID.
apps/sim/lib/billing/webhooks/disputes.ts Dispute-open and dispute-close are now both audited regardless of outcome; shouldUnblock logic preserved correctly, close is audited even when dispute is lost; getOrganizationOwnerId guarded with internal try/catch.
apps/sim/lib/billing/storage/tracking.ts New decrementStorageUsageInTx for in-transaction storage counter updates, uses GREATEST(0,...) to prevent negative values, correctly branches on org vs personal scope.
apps/sim/app/api/auth/oauth/token/route.ts GET handler audit correctly placed inside try block after refreshTokenIfNeeded succeeds; POST service-account path uses emitServiceAccountAccess closure called only on success; all credential-access audit paths are success-only.
apps/sim/app/workspace/[workspaceId]/providers/workspace-scope-sync.tsx Adds org group association to PostHog; resetGroups() is called before re-setting workspace group for personal workspaces, correctly clearing stale org group; effect deferred until activeWorkspace metadata loads to avoid transient null organizationId mismatching groups.
apps/sim/lib/billing/organizations/seats.ts Seat audit and analytics moved into reconcileOrganizationSeats itself; audit fires only when actorId is provided (user-initiated reconciles); PostHog event fires for all reconciles.
apps/sim/lib/billing/threshold-billing.ts Overage billing refactored to return a structured result from the transaction; audit and PostHog emitted outside the transaction using that result, correctly attributed to the org owner or user.
packages/audit/src/log.test.ts Tests updated to cover the new FK-null behavior: lookup-throws path, user-not-found path, and admin-api label all verified.
apps/sim/lib/posthog/events.ts 13 new event type definitions added for billing/org/credential/workflow events; several existing events relaxed workspace_id to optional to avoid phantom empty-string values.
apps/sim/lib/invitations/core.ts Seat reconciliation audit moved into reconcileOrganizationSeats (actorId now passed in); org_member_added audit added on successful invite acceptance; old manual audit block cleanly removed.
apps/sim/lib/copilot/tools/handlers/management/manage-skill.ts SKILL_CREATED/UPDATED/DELETED audit and PostHog events added after each successful operation; all instrumentation is fire-and-forget.
apps/sim/lib/copilot/tools/handlers/management/manage-custom-tool.ts CUSTOM_TOOL_CREATED/UPDATED/DELETED audit and PostHog events added; resourceId is correctly optional when the newly-created tool is not found in the result set.
apps/realtime/src/handlers/variables.ts WORKFLOW_VARIABLES_UPDATED audit correctly deferred to after the coalesced write succeeds; actorId tracks the most recent writer of each pending variable update.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Stripe as Stripe Webhook
    participant Handler as Webhook Handler
    participant ActorResolver as resolveBillingActorId
    participant AuditLog as recordAudit (fire-and-forget)
    participant PostHog as captureServerEvent (fire-and-forget)
    participant DB as Database

    Stripe->>Handler: event (invoice / subscription / dispute)
    Handler->>Handler: run critical business logic
    Handler->>ActorResolver: resolve actor (org owner lookup)
    ActorResolver->>DB: "SELECT member WHERE role='owner'"
    DB-->>ActorResolver: userId OR referenceId (fallback)
    ActorResolver-->>Handler: actorId (never throws)
    Handler->>AuditLog: "recordAudit({ actorId, action, ... })"
    AuditLog->>DB: "SELECT user WHERE id=actorId"
    alt user found
        DB-->>AuditLog: name, email
    else user not found or lookup error
        AuditLog->>AuditLog: "null actorId, label=Admin API/System"
    end
    AuditLog->>DB: INSERT audit_log (async, non-blocking)
    Handler->>PostHog: captureServerEvent(actorId, event, props)
    Handler-->>Stripe: 200 OK
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Stripe as Stripe Webhook
    participant Handler as Webhook Handler
    participant ActorResolver as resolveBillingActorId
    participant AuditLog as recordAudit (fire-and-forget)
    participant PostHog as captureServerEvent (fire-and-forget)
    participant DB as Database

    Stripe->>Handler: event (invoice / subscription / dispute)
    Handler->>Handler: run critical business logic
    Handler->>ActorResolver: resolve actor (org owner lookup)
    ActorResolver->>DB: "SELECT member WHERE role='owner'"
    DB-->>ActorResolver: userId OR referenceId (fallback)
    ActorResolver-->>Handler: actorId (never throws)
    Handler->>AuditLog: "recordAudit({ actorId, action, ... })"
    AuditLog->>DB: "SELECT user WHERE id=actorId"
    alt user found
        DB-->>AuditLog: name, email
    else user not found or lookup error
        AuditLog->>AuditLog: "null actorId, label=Admin API/System"
    end
    AuditLog->>DB: INSERT audit_log (async, non-blocking)
    Handler->>PostHog: captureServerEvent(actorId, event, props)
    Handler-->>Stripe: 200 OK
Loading

Reviews (28): Last reviewed commit: "chore(observability): trim verbose inlin..." | Re-trigger Greptile

Comment thread apps/sim/app/api/files/public/[token]/content/route.ts
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.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/api/v1/admin/workflows/export/route.ts Outdated
Comment thread apps/sim/lib/knowledge/documents/service.ts
- 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).
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/billing/webhooks/disputes.ts
Comment thread apps/sim/lib/billing/threshold-billing.ts
Comment thread apps/sim/lib/uploads/contexts/copilot/copilot-file-manager.ts Outdated
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.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/api/table/[tableId]/export/route.ts Outdated
Comment thread apps/sim/lib/auth/auth.ts Outdated
Comment thread apps/sim/lib/uploads/contexts/copilot/copilot-file-manager.ts Outdated
…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.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Address Cursor review: addImportColumns (async createColumns import path) now
threads the importing userId into auditTableColumnsAdded instead of falling back
to table.createdBy, so column additions are attributed to the actual member who
ran the import rather than the table creator.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/api/files/download/route.ts
Per design decision: copilot files are working/conversational artifacts, so
gating their materialization on storage quota would fail an agent operation
mid-flow when a user is over limit, and metering them would inflate usage enough
to block KB/workspace uploads indirectly. Remove copilot quota gates + copilot
ingest metering entirely (revert copilot-file-manager, metadata, and the upload
route's copilot branch to baseline) and drop the now-unused releaseDeletedFileStorage.
KB document metering + quota enforcement (the deliberate, persistent storage path)
is unchanged.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Address Cursor review: gate the group-sync effect on workspace metadata being
loaded (activeWorkspace present) so the workspace and organization groups always
update atomically. Acting during the load window paired the new workspace group
with the previous workspace's org group; until metadata loads, events stay
consistently attributed to the previous workspace.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/app/api/table/[tableId]/export-async/route.ts
Address Cursor review: async exports only emitted TABLE_EXPORTED when the
background job reached 'ready', so an authorized export whose job later failed or
was abandoned left no audit trail — inconsistent with the sync export route,
which audits before streaming. Move the audit + analytics to the async route's
authorization point (after the job is claimed/dispatched) and remove it from the
runner. Drop the now-unused userId from TableExportPayload.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 f5f8a00. Configure here.

Comment thread apps/sim/lib/billing/webhooks/invoices.ts Outdated
…king

Address Greptile P1: the payment_failed audit hoisted an unguarded
isSubscriptionOrgScoped DB read (for entity_type) directly before the
attempt-count user-blocking block. A transient failure of that read would throw
out of the handler and skip blocking. Wrap the whole audit/analytics block in
try/catch (best-effort), and let the blocking compute its own isSubscriptionOrgScoped
as it did originally — instrumentation can no longer abort payment processing.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 2afd114. Configure here.

@waleedlatif1 waleedlatif1 deleted the branch staging July 1, 2026 05:43
@waleedlatif1 waleedlatif1 reopened this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant