Trait history, billing lifecycle events, and ingestion credential scrubbing#538
Conversation
new execution suite compiles all registered builders and EXPLAINs them against ClickHouse, catching analyzer errors that compile-only tests cannot; ci gains a clickhouse service and schema init so the suite enforces instead of skipping. Fixes interesting_sessions, which the sweep caught failing on the production server (correlated CTE references and unaliased qualified projections).
getContentHash now uses node:crypto so sri.spec.ts can run under Playwright's node runner, unblocking the deploy test gate.
… single-use helpers
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📄 Knowledge review🆕 New pages1 new page was drafted from this PR.
✏️ Documentation updates1 page was updated by changes in this PR.
📝 SDK Usage and Configuration — changes@@ -33,7 +33,23 @@
| `maskPatterns` | string[] | — | Glob patterns to mask sensitive paths. |
| `filter` | function | — | Filter function to conditionally skip events. |
-All analytics are anonymized by default. No cookies, fingerprinting, or PII are collected. The SDK is safe for SSR/React Server Components and only injects the script on the client side.
+All analytics are anonymized by default. No cookies, fingerprinting, or PII are collected. The SDK is safe for SSR/React Server Components and only injects the script on the client side.
+
+### Automatic Credential Scrubbing
+
+Databuddy automatically redacts sensitive query parameters at ingestion time to prevent credential leaks. URL, path, referrer, and href fields have sensitive query parameters (password, token, api_key, email, otp, cvv, ssn, etc.) replaced with `REDACTED` before storage.
+
+This applies to both query strings and hash fragments (including OAuth implicit flow tokens). Parameter keys remain visible so teams can identify leaky forms, but values are protected:
+
+```
+https://app.com/login?email=user@example.com&password=hunter2
+→ https://app.com/login?email=REDACTED&password=REDACTED
+
+https://app.com/callback#access_token=abc123&state=xyz
+→ https://app.com/callback#access_token=REDACTED&state=xyz
+```
+
+No configuration is required — credential scrubbing is applied automatically to protect against accidental credential exposure in URLs.
[Reference](https://github.com/databuddy-analytics/Databuddy/blob/78f406152290b45d3ace08be460e270ee0a01ffd/packages/sdk/README.md#L11-L118)
## SDK Methods
@@ -143,6 +159,27 @@
```js
window.databuddy.identify("user_123", { email: "jo@acme.com" });
```
+
+#### Tracking Billing Lifecycle Events
+
+Traits can be used to track subscription and billing state. By setting traits like `plan`, `has_active_subscription`, and `billing_scope`, you enable lifecycle event tracking (subscription_started, plan_upgraded, plan_downgraded, subscription_renewed, subscription_canceled, etc.).
+
+```ts
+import { identify, setGlobalProperties } from "@databuddy/sdk";
+
+// On login, set billing traits
+identify(user.id, {
+ email: user.email,
+ plan: "pro",
+ has_active_subscription: true,
+ billing_scope: "organization" // or "personal"
+});
+
+// Attach plan to all future events
+setGlobalProperties({ plan: "pro" });
+```
+
+Server-side systems (webhooks, billing providers) can update traits and emit lifecycle events by calling `identify()` with updated traits. The dashboard tracks when traits change and what they were at any point in time.
### setTraits
|
There was a problem hiding this comment.
2 issues found across 31 files
Confidence score: 2/5
packages/ai/src/query/builders/sessions.tsregresses thepaths_for_top/names_for_topCTE scope by aggregating across all sessions in range instead of just paginatedtop_sessions, which can skew top-session results and significantly increase query cost under load — restore the prefilter join/constraint totop_sessionsbefore merging.packages/services/src/identity.tsleaves a race inupsertProfilefor first-time identifies becauseSELECT ... FOR UPDATEdoes not lock a row that does not yet exist, so concurrent requests on the same(websiteId, profile...)can create duplicates or fail unpredictably — add a create-path concurrency guard (e.g., unique-key upsert/retry or advisory lock) and verify with a concurrent test before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/ai/src/query/builders/sessions.ts">
<violation number="1" location="packages/ai/src/query/builders/sessions.ts:304">
P1: The `paths_for_top` and `names_for_top` CTEs lost the prefilter that limited aggregation to the paginated `top_sessions` set. As a result, they now `GROUP BY session_id` across every non-empty session in the date range (potentially millions) instead of just the selected top sessions (default 10), then discard the surplus in the final LEFT JOIN. This significantly inflates scan volume and memory/CPU for those two aggregations and raises timeout risk for the `interesting_sessions` endpoint. Restoring `AND session_id IN (SELECT session_id FROM top_sessions)` in both CTEs keeps the query scoped to the small result set it actually needs.</violation>
</file>
<file name="packages/services/src/identity.ts">
<violation number="1" location="packages/services/src/identity.ts:193">
P1: The `SELECT ... FOR UPDATE` inside the `upsertProfile` transaction only protects against concurrent updates when the profile row already exists. For two concurrent first-time identifies on the same `(websiteId, profileId)`, neither transaction acquires a lock (no row to lock), both read `{}` as `existingTraits`, and the second transaction's `profileTrait_changes` record will have incorrect `oldValue`s because it never saw the first transaction's insert. This breaks trait-history correctness for the bootstrap case.
To prevent this, acquire a transaction-scoped advisory lock before the SELECT, consistent with the existing pattern in `packages/rpc/src/routers/feedback.ts`, so that only one first-time upsert proceeds at a time per profile.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Tracker as Client Tracker (SDK)
participant Basket as Basket Ingestion API
participant Autumn as Autumn (Billing Webhook)
participant API as Databuddy Query API
participant SDK as SDK / Dashboard Client
participant Postgres as Postgres (RDS)
participant ClickHouse as ClickHouse (Analytics)
participant Services as @databuddy/services
participant AI as AI Agent / MCP
Note over Tracker,ClickHouse: Sensor/Sensitive Query Param Scrubbing (All Write Paths)
Tracker->>Basket: POST /track, /identify, /outlink, /error, /vital
Basket->>Basket: sanitizeUrl() → redactSensitiveQueryParams() on url/path/referrer/href
Basket->>Basket: Redact denylist params (password, token, api_key, email, otp, ...) + hash fragments
Basket->>ClickHouse: store scrubbed event (REDACTED values, keys preserved)
Note over Postgres,ClickHouse: Profile Trait History (Postgres, Transactional)
Basket->>Postgres: upsertProfile(websiteId, profileId, splitTraits)
Postgres->>Postgres: SELECT ... FOR UPDATE on existing traits
Postgres->>Postgres: applyTraits() → compute diff (old vs new for each key)
Postgres->>Postgres: INSERT INTO profile_trait_changes (full snapshot + changes JSONB)
Postgres-->>Basket: return { changes, traits }
Note over Autumn,Postgres: Billing Lifecycle Events (Dogfooding)
Autumn->>API: POST /webhooks/autumn (customer.products.updated, includes scenario)
API->>Services: recordPlanChange({ customerId, planId, scenario })
alt SELF_ANALYTICS_WEBSITE_ID set
Services->>Postgres: upsertProfile(websiteId, customerId, { plan }, source="billing")
Postgres->>Postgres: Apply trait changes, record in profile_trait_changes
Services->>ClickHouse: INSERT INTO custom_events (event_name, profile_id=customerId, source=billing)
Note over Services,ClickHouse: e.g. plan_upgraded, subscription_canceled
else env var unset / no-op
Services-->>API: return (skip)
end
API-->>Autumn: { success: true }
Note over API,SDK: Profile Enrichment in Query API (PII-safe)
SDK->>API: GET /v1/query (profile_list type)
API->>ClickHouse: Execute query, get results with profile_id
API->>Postgres: attachProfileIdentities() → SELECT displayName, email FROM profiles WHERE profile_id IN (...)
Postgres-->>API: identity list
API->>API: revealPii() on displayName, email (decrypt if encrypted)
API->>SDK: results with display_name, email attached (null if no identity)
SDK->>SDK: Render user name/email directly (no useProfileIdentities hook needed)
Note over AI,Postgres: Profile History Tool (Agent / MCP)
AI->>AI: get_profile_history tool invoked (agent or MCP)
AI->>API: (internal) query with websiteId, profileId
API->>Postgres: SELECT changes, traits, source, changedAt FROM profile_trait_changes ORDER BY createdAt DESC LIMIT N
Postgres-->>API: history entries
API-->>AI: { history: [...], count: N }
AI->>AI: Answer "when did plan change" / "traits at time T"
Note over SDK,ClickHouse: Dashboard Billing Identity
SDK->>SDK: IdentifyBillingTraits component
SDK->>SDK: identify(userId, { plan, has_active_subscription, billing_scope })
SDK->>SDK: setGlobalProperties({ plan }) → attaches plan to all future events
Tracker->>Basket: subsequent events include global plan property
Basket->>ClickHouse: store event with plan property
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| AND time >= toDateTime({startDate:String}) | ||
| AND time <= toDateTime({endDate:String}) | ||
| AND session_id IN (SELECT session_id FROM top_sessions) | ||
| AND session_id != '' |
There was a problem hiding this comment.
P1: The paths_for_top and names_for_top CTEs lost the prefilter that limited aggregation to the paginated top_sessions set. As a result, they now GROUP BY session_id across every non-empty session in the date range (potentially millions) instead of just the selected top sessions (default 10), then discard the surplus in the final LEFT JOIN. This significantly inflates scan volume and memory/CPU for those two aggregations and raises timeout risk for the interesting_sessions endpoint. Restoring AND session_id IN (SELECT session_id FROM top_sessions) in both CTEs keeps the query scoped to the small result set it actually needs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ai/src/query/builders/sessions.ts, line 304:
<comment>The `paths_for_top` and `names_for_top` CTEs lost the prefilter that limited aggregation to the paginated `top_sessions` set. As a result, they now `GROUP BY session_id` across every non-empty session in the date range (potentially millions) instead of just the selected top sessions (default 10), then discard the surplus in the final LEFT JOIN. This significantly inflates scan volume and memory/CPU for those two aggregations and raises timeout risk for the `interesting_sessions` endpoint. Restoring `AND session_id IN (SELECT session_id FROM top_sessions)` in both CTEs keeps the query scoped to the small result set it actually needs.</comment>
<file context>
@@ -301,7 +301,7 @@ export const SessionsBuilders: Record<string, SimpleQueryConfig> = {
AND time >= toDateTime({startDate:String})
AND time <= toDateTime({endDate:String})
- AND session_id IN (SELECT session_id FROM top_sessions)
+ AND session_id != ''
GROUP BY session_id
),
</file context>
| ) | ||
| ) | ||
| .limit(1) | ||
| .for("update"); |
There was a problem hiding this comment.
P1: The SELECT ... FOR UPDATE inside the upsertProfile transaction only protects against concurrent updates when the profile row already exists. For two concurrent first-time identifies on the same (websiteId, profileId), neither transaction acquires a lock (no row to lock), both read {} as existingTraits, and the second transaction's profileTrait_changes record will have incorrect oldValues because it never saw the first transaction's insert. This breaks trait-history correctness for the bootstrap case.
To prevent this, acquire a transaction-scoped advisory lock before the SELECT, consistent with the existing pattern in packages/rpc/src/routers/feedback.ts, so that only one first-time upsert proceeds at a time per profile.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/services/src/identity.ts, line 193:
<comment>The `SELECT ... FOR UPDATE` inside the `upsertProfile` transaction only protects against concurrent updates when the profile row already exists. For two concurrent first-time identifies on the same `(websiteId, profileId)`, neither transaction acquires a lock (no row to lock), both read `{}` as `existingTraits`, and the second transaction's `profileTrait_changes` record will have incorrect `oldValue`s because it never saw the first transaction's insert. This breaks trait-history correctness for the bootstrap case.
To prevent this, acquire a transaction-scoped advisory lock before the SELECT, consistent with the existing pattern in `packages/rpc/src/routers/feedback.ts`, so that only one first-time upsert proceeds at a time per profile.</comment>
<file context>
@@ -99,33 +160,74 @@ export async function upsertProfile(
+ )
+ )
+ .limit(1)
+ .for("update");
+ const existingTraits = (existingRows[0]?.traits ?? {}) as Record<
+ string,
</file context>
Greptile SummaryThis PR ships three interconnected features: credential scrubbing at ingestion (
Confidence Score: 3/5The credential-scrubbing and session-ID hardening are straightforward and well-tested. The trait-history and billing-lifecycle code works correctly in the single-writer case but has two structural gaps that can produce silently wrong data in production. The
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Tracker as Browser Tracker
participant Basket as basket ingestion
participant PG as Postgres
participant CH as ClickHouse
participant Autumn as Autumn Webhook
participant API as api service
Tracker->>Basket: POST /identify with traits
Basket->>Basket: sanitizeUrl redacts sensitive params
Basket->>PG: BEGIN tx SELECT profile FOR UPDATE
PG-->>Basket: existingTraits
Basket->>Basket: applyTraits compute diff
Basket->>PG: UPSERT profile row
Basket->>PG: INSERT profile_trait_changes if changed
PG-->>Basket: commit
Basket-->>Tracker: 200 OK
Autumn->>API: POST /webhooks/autumn products.updated
API->>PG: UPSERT profile with plan trait
API->>PG: INSERT profile_trait_changes
API->>CH: INSERT custom_events lifecycle event
API-->>Autumn: 200 OK
API->>PG: SELECT profile_trait_changes get_profile_history
PG-->>API: trait snapshots and diffs
%%{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 Tracker as Browser Tracker
participant Basket as basket ingestion
participant PG as Postgres
participant CH as ClickHouse
participant Autumn as Autumn Webhook
participant API as api service
Tracker->>Basket: POST /identify with traits
Basket->>Basket: sanitizeUrl redacts sensitive params
Basket->>PG: BEGIN tx SELECT profile FOR UPDATE
PG-->>Basket: existingTraits
Basket->>Basket: applyTraits compute diff
Basket->>PG: UPSERT profile row
Basket->>PG: INSERT profile_trait_changes if changed
PG-->>Basket: commit
Basket-->>Tracker: 200 OK
Autumn->>API: POST /webhooks/autumn products.updated
API->>PG: UPSERT profile with plan trait
API->>PG: INSERT profile_trait_changes
API->>CH: INSERT custom_events lifecycle event
API-->>Autumn: 200 OK
API->>PG: SELECT profile_trait_changes get_profile_history
PG-->>API: trait snapshots and diffs
Reviews (1): Last reviewed commit: "refactor(services): tighten billing life..." | Re-trigger Greptile |
| // latest row with createdAt <= T, no diff folding needed. | ||
| traits: jsonb().$type<Record<string, unknown>>().default({}).notNull(), | ||
| changes: jsonb() | ||
| .$type< | ||
| Record< | ||
| string, | ||
| { | ||
| old: string | number | boolean | null; | ||
| new: string | number | boolean | null; | ||
| } | ||
| > | ||
| >() | ||
| .default({}) | ||
| .notNull(), | ||
| source: text().default("identify").notNull(), | ||
| createdAt: timestamp({ precision: 3, withTimezone: true }) | ||
| .defaultNow() | ||
| .notNull(), | ||
| }, | ||
| (table) => [ | ||
| index("profile_trait_changes_profile_idx").on( | ||
| table.websiteId, |
There was a problem hiding this comment.
Missing FK from
profileId to profiles undermines the stated GDPR story
The profileTraitChanges table has a FK from websiteId to websites (with cascade delete) but no FK from profileId to profiles. A GDPR delete that removes a row from profiles will not cascade to profile_trait_changes, leaving behind trait history for that user even though their profile is gone. The PR description calls out "trivial GDPR deletes" as a motivation for choosing Postgres over ClickHouse, so an explicit cascading FK on profileId seems necessary to deliver on that promise.
| const existingRows = await tx | ||
| .select({ traits: profiles.traits }) | ||
| .from(profiles) | ||
| .where( | ||
| and( | ||
| eq(profiles.websiteId, websiteId), | ||
| eq(profiles.profileId, profileId) | ||
| ) | ||
| ) | ||
| .limit(1) | ||
| .for("update"); |
There was a problem hiding this comment.
SELECT … FOR UPDATE doesn't lock on a non-existent row — first-time profiles can produce duplicate / inconsistent history
When a profile doesn't exist yet, SELECT … FOR UPDATE returns no rows and acquires no predicate lock in Postgres. Two concurrent first-time upsertProfile calls for the same (websiteId, profileId) pair will both see existingTraits = {}, both call applyTraits({}, rest, []), and both attempt to insert into profileTraitChanges. The INSERT ON CONFLICT DO UPDATE on profiles is safe (one wins, the other updates), but profileTraitChanges has no uniqueness guard, so both writes succeed, leaving duplicate — and potentially contradictory — history rows: if the two calls carry different trait maps (e.g. back-to-back identify events during sign-up), a history row for a transient intermediate state that was immediately overwritten can appear as a permanent entry.
| @@ -313,7 +313,7 @@ export const SessionsBuilders: Record<string, SimpleQueryConfig> = { | |||
| website_id = {websiteId:String} | |||
| AND timestamp >= toDateTime({startDate:String}) | |||
| AND timestamp <= toDateTime({endDate:String}) | |||
| AND session_id IN (SELECT session_id FROM top_sessions) | |||
| AND session_id != '' | |||
There was a problem hiding this comment.
Replacing
session_id IN (SELECT … top_sessions) with session_id != '' expands the scan to all sessions in the time window
The two CTEs (custom_events_per_session and names_for_top) previously scoped their reads to only the sessions that would appear in the final result via the correlated subquery. The new predicate session_id != '' removes that scoping — ClickHouse now reads every custom event and every page-name row for the entire requested time range before the outer LEFT JOIN discards non-top-sessions rows. For high-traffic websites this could be orders-of-magnitude more data than before, potentially causing query timeouts. The final SELECT output is still correct (the LEFT JOIN on ts.session_id bounds it), but the intermediate scan is no longer bounded.
| "cvv", | ||
| "ssn", | ||
| "email", | ||
| "e-mail", | ||
| "phone", | ||
| "tel", | ||
| "username", | ||
| ]); | ||
|
|
||
| const REDACTED_VALUE = "REDACTED"; | ||
|
|
||
| function redactParams(query: string): string { | ||
| const params = new URLSearchParams(query); | ||
| let changed = false; | ||
| for (const key of [...params.keys()]) { | ||
| if (SENSITIVE_QUERY_PARAMS.has(key.toLowerCase())) { |
There was a problem hiding this comment.
URLSearchParams.toString() silently re-encodes non-sensitive params when any sensitive param is found
When changed is true, the function returns params.toString(), which percent-encodes the entire query string using URLSearchParams rules. This re-encodes space characters as + instead of %20, normalises %2B to +, and may change other application-specific encodings for non-sensitive keys that the caller never intended to alter. The problem only manifests when at least one sensitive param is present in the same query string, making it inconsistent: /search?q=hello%20world&utm=foo is stored unchanged, but /search?q=hello%20world&token=abc is stored as /search?q=hello+world&token=REDACTED, silently losing the %20→+ distinction for q.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| useEffect(() => { | ||
| if (isDashboardE2E || isLoading || !(userId && currentPlanId)) { | ||
| return; | ||
| } | ||
| identify(userId, { | ||
| plan: currentPlanId, | ||
| has_active_subscription: hasActiveSubscription, | ||
| billing_scope: isOrganizationBilling ? "organization" : "personal", | ||
| }); | ||
| setGlobalProperties({ plan: currentPlanId }); | ||
| }, [ | ||
| userId, | ||
| currentPlanId, | ||
| hasActiveSubscription, | ||
| isOrganizationBilling, | ||
| isLoading, |
There was a problem hiding this comment.
identify fires a full Postgres transaction on every page navigation
The useEffect runs whenever isLoading transitions, which happens on every route change that remounts this provider. Because rest = { plan, has_active_subscription, billing_scope } is never empty, upsertProfile always falls into the hasTraitUpdates path and runs a full transaction (SELECT … FOR UPDATE + INSERT ON CONFLICT DO UPDATE + possibly INSERT INTO profile_trait_changes), even when the traits are identical to what was last stored. Adding a client-side guard (e.g., comparing values before calling identify, or moving the call to a stable singleton) would prevent redundant DB round-trips for every page load.
Summary
Ships the identity/trait-history work plus the credential-leak hardening from this week, alongside the tracker/dashboard fixes already on staging.
Security: sensitive query params scrubbed at ingestion
password,token,api_key,email,otp, ...) fromurl/path/referrer/hrefon every write path, including hash fragments (OAuth implicit flow). Values becomeREDACTED, keys stay visible so leaky customer forms remain detectable.Profile trait history (Postgres)
profile_trait_changestable: each row is a full post-change trait snapshot plus a{key: {old, new}}diff, written transactionally insideupsertProfile(SELECT ... FOR UPDATE). Answers "when did the plan change" and "what were the traits at time T" without diff folding.planat event time so point-in-time joins aren't needed.Billing lifecycle events
customer.products.updatedwebhook now callsrecordPlanChange: plan trait upsert (source: "billing") + lifecycle custom events (plan_upgraded,subscription_canceled, ...) withprofile_idattached. Gated onSELF_ANALYTICS_WEBSITE_ID(unset = no-op); scenario type is compile-checked against the webhook's zod enum.Agent/MCP
get_profile_historytool added tobuildProfileTools— available to both the in-app agent and MCP with website-access enforcement.Dogfooding
plan/has_active_subscription/billing_scopetraits and stampsplanon every event via the newly exportedsetGlobalProperties.Deploy notes
SELF_ANALYTICS_WEBSITE_IDmust be set on the API service for lifecycle recording to activate (safe no-op until then).profile_trait_changesis already applied to production Postgres.Testing
Summary by cubic
Adds Postgres-backed trait history and billing lifecycle recording, and scrubs sensitive query params at ingestion to stop credential leaks. Also enriches profile lists with identity info and adds small tracker/CI fixes.
New Features
password,token,api_key,email,otp, hash fragments). Values become REDACTED; keys stay.profile_trait_changes) with full snapshots and diffs, written transactionally inupsertProfile; newget_profile_historytool.@databuddy/services(recordPlanChange): setplantrait (source: "billing") and emit custom events (e.g.,plan_upgraded). Gated bySELF_ANALYTICS_WEBSITE_ID.display_nameandemailtoprofile_listresults (PII-safe), and the dashboard users page uses these to avoid flicker.planproperty;@databuddy/sdknow exportssetGlobalProperties.Bug Fixes
interesting_sessionsbuilder and tighten session queries.sess_...session IDs; reject malformed URL overrides; use sha256 in deploy utils.Written for commit 4422b2e. Summary will update on new commits.