Release: user identity, encryption at rest, env schema enforcement#536
Merged
Conversation
profiles + profile_aliases Postgres tables, profile_id columns on events/custom_events/revenue with idempotent migrations, canonical visitor-key expressions in clickhouse/identity.ts, agent allowlist entries, and drift tests binding schema, migrations, and allowlist.
profileId (1-128 chars) on analytics and custom-event schemas, traits capped at 50 scalar keys / 2KB serialized with null-deletes-key.
splitTraits (email/username/name promotion), no-op-skipping profile upsert with jsonb trait merge, and race-safe alias upsert.
profileId flows through all ingestion paths into profile_id (never salted), /identify supports website-origin and API-key auth with track:events website scope, rate limits, bot checks, and evlog; Stripe and Paddle webhooks extract profile ids for revenue attribution.
identify/setTraits/clearProfile/getProfileId on the tracker and window globals, did_profile persistence, profileId on every payload, one identify request per session unless traits change.
SSR-safe identify wrappers with pre-load getProfileId fallback, node client identify() with API-key auth and websiteId fallback, profileId on custom event inputs.
getByIds (batched, max 100) and get-with-aliases behind website read permissions; integration coverage for member access, cross-org FORBIDDEN, alias resolution, and unknown profiles.
profile builders group by the canonical visitor key so identified users collapse across devices, expose profile_id, resolve activity in tables without the column via device sets, allow profile_id filters, and teach the agent prompt identity counting patterns.
users list shows display names and emails via batched profile lookup, detail header enriched with identity, Identified preset filter.
profile row with copy action in the identity section and profile storage keys in the inspector.
concepts, browser/script/node examples, trait semantics and limits, revenue attribution metadata keys, API reference.
identity.ts was missing from the drizzle-kit schema list so db:push skipped the new tables; also drops the stale admin.ts entry. Adds a drift test asserting every pgTable file is registered and every registered file exists.
identify API across core, react, and node entrypoints.
the prompt module's import-time consistency check throws when an AGENT_TABLE_COLUMNS entry is missing from the table docs, which crashed the api at boot after revenue gained identity columns.
IdentifyUser provider calls identify/clearProfile from the auth session, and Stripe checkout metadata carries databuddy_profile_id for revenue attribution.
dedupe marker is now written only after a successful identify so transient failures retry within the session; page_exit beacons carry profileId; identify enforces the documented 128-char cap; empty trait objects no longer bypass dedupe; clear() reuses clearProfile(); e2e waits are method-filtered and deterministic.
stripe/paddle profile ids now pass through sanitizeString with the 128-char cap like every other ingest path; track schema reuses profileIdSchema; identify payload rejects empty anonymousId and allows trait values up to the property limit.
visitor_ids includes the requested id so span-only activity is not dropped; profile_detail resolves the most recent non-empty identity; agent guidance notes the nullable fallback for custom_events/revenue.
AES-256-GCM via DATABUDDY_ENCRYPTION_KEY with a deterministic HMAC email hash for equality lookups; rpc decrypts on read so the API contract is unchanged; custom traits stay plaintext for filtering; keyless deployments fall back to plaintext with a boot warning.
DATABUDDY_ENCRYPTION_KEY is enforced by the basket env schema, so a production boot without it fails validation instead of silently storing profile PII unencrypted; development keeps a boot warning. Basket now consumes the validated env at startup.
api now validates its env at boot (schema previously existed but was never imported) and reads PORT from it; basket declares its redpanda, ip-privacy, and resend vars with IP_HASH_SALT required in production; slack requires the encryption key in production; insights declares its worker and railway vars; drops the orphaned better-admin and database schemas.
six identical window-guard try/catch wrappers become callTracker with typed closures and no assertions; env schemas share optionalString from base instead of two local copies.
Contributor
|
Too many files changed for review. ( Bypass the limit by tagging |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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 |
|
The latest updates on your projects. Learn more about Unkey Deploy
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Shadow auto-approve: would require human review. Major production release with end-to-end identity, encryption at rest, and new auth discovery endpoints; requires thorough human review.
Re-trigger cubic
📄 Knowledge review✏️ Documentation updates1 page was updated by changes in this PR.
📝 SDK Usage and Configuration — changes@@ -38,6 +38,8 @@
## SDK Methods
+For comprehensive documentation on user identification, traits, encryption, and best practices, see [Identifying Users](/docs/sdk/identify-users).
+
### track
Tracks a custom event with an event name and optional properties. Events are batched and sent efficiently to minimize network overhead. Safe to call on the server (no-op) or before the tracker loads.
@@ -80,16 +82,13 @@
### clear
-Clears the current user session and generates new anonymous/session IDs. Use after logout to ensure the next user gets a fresh identity.
+Clears the current user session and generates new anonymous/session IDs. Use `clearProfile()` instead if you only want to forget the user identity on logout. `clear()` resets the entire session, including the anonymous ID, so subsequent activity starts fresh.
```ts
import { clear } from "@databuddy/sdk";
-async function handleLogout() {
- await signOut();
- clear(); // Reset tracking identity
- router.push("/login");
-}
+// Full reset (less common, for complete identity change)
+clear();
```
### flush
@@ -124,6 +123,64 @@
}
```
+### identify
+
+Links the browser to a user ID from your system. Persists across sessions and is safe to call on every page load (deduplicated per session when called with the same ID and no traits).
+
+```ts
+import { identify } from "@databuddy/sdk";
+
+// On login or signup
+identify("user_123", {
+ email: "jo@acme.com",
+ name: "Jo Smith",
+ plan: "pro"
+});
+```
+
+When using the script tag, `identify` is available on the global `window.databuddy` object:
+
+```js
+window.databuddy.identify("user_123", { email: "jo@acme.com" });
+```
+
+### setTraits
+
+Merges traits into the identified user's profile. Requires a prior `identify()` call.
+
+```ts
+import { setTraits } from "@databuddy/sdk";
+
+setTraits({ plan: "enterprise", trial_ends_at: null });
+```
+
+### clearProfile
+
+Forgets the identified user (call on logout). The anonymous ID is kept so subsequent activity is tracked anonymously again.
+
+```ts
+import { clearProfile } from "@databuddy/sdk";
+
+async function handleLogout() {
+ await signOut();
+ clearProfile(); // Forget user identity but keep anonymous tracking
+ router.push("/login");
+}
+```
+
+### getProfileId
+
+Returns the currently identified user ID, or `null` when anonymous.
+
+```ts
+import { getProfileId } from "@databuddy/sdk";
+
+const userId = getProfileId();
+if (userId) {
+ console.log("User is identified:", userId);
+}
+```
+
## Standard Events
Databuddy supports standard events such as:
@@ -210,6 +267,32 @@
await analytics.flush();
```
+The Node.js SDK also supports identifying users server-side:
+
+```javascript
+await analytics.identify({
+ profileId: user.id,
+ anonymousId: cookies.get("did"), // Optional: link to client's device
+ traits: {
+ email: user.email,
+ name: user.name,
+ plan: 'pro',
+ },
+});
+```
+
+Server-tracked events can include a `profileId` field for attribution:
+
+```javascript
+await analytics.track({
+ name: 'subscription_started',
+ profileId: user.id,
+ properties: {
+ plan: 'enterprise',
+ },
+});
+```
+
### Important Notes
- **Flush in serverless environments**: Call `flush()` in serverless functions or background jobs to ensure all events are sent before the function or process terminates.
@@ -221,7 +304,7 @@
- **Initialize early**: Add the `<Databuddy />` component as high as possible in your React/Next.js layout to capture all events.
- **Use batching**: Enable batching (`enableBatching`, `batchSize`, `batchTimeout`) to reduce network overhead.
- **Flush before navigation**: Call `flush()` before navigating away or redirecting to ensure all events are sent.
-- **Clear on logout**: Call `clear()` after user logout to reset tracking identity.
+- **Clear on logout**: Call `clearProfile()` after user logout to forget the user identity while keeping anonymous tracking. Use `clear()` only if you need to reset the entire session (less common).
- **Debugging**: Enable debug logging with the `debug` option for troubleshooting.
- **Disable in development**: Use the `disabled` option to turn off tracking in non-production environments.
- **Privacy**: No cookies, fingerprinting, or PII are collected. All analytics are anonymized by default.
|
health-check and dashboard-e2e workflows now supply AI_API_KEY, CLICKHOUSE_URL, BETTER_AUTH_URL, and DATABASE_URL so the api's new boot-time env validation passes in CI; the docs middleware.ts added for root markdown negotiation is merged into proxy.ts since Next 16 allows only one, reusing the shared accept-markdown helper.
There was a problem hiding this comment.
26 issues found and verified against the latest diff
Confidence score: 2/5
- The highest-risk issue is in
apps/api/src/routes/discovery.ts: hardcodingSITE_URLto production can emit wrong discovery/auth URLs in non-prod or self-hosted environments, so clients may follow invalid endpoints after merge — switch this to env/config sourcing (likeconfig.urls.api) before merging. - Identity integrity is at risk across
packages/db/src/drizzle/schema/identity.tsandapps/basket/src/routes/identify.ts: aliases can drift without a(website_id, profile_id)FK, and inactive websites can still mutate identity via API key, which can corrupt or revive data that should be frozen — add the composite FK and enforcewebsite.status === ACTIVEin the API-key gate. - There are concrete regression risks in client/runtime behavior:
packages/devtools/src/core/types.tsmissesprofileIdinidentityChanged(stale UI state), andpackages/sdk/src/node/index.tsnow fails valid API-key identify calls whenwebsiteIdis omitted (breaking documented optional flow) — restore optional server-side resolution and includeprofileIdin change detection before release. - Docs/platform routes need consistency hardening:
apps/docs/app/.well-known/api-catalog/route.tsHEAD/GET header mismatch,apps/docs/app/openapi.json/route.tsunhandled fetch failures, and discovery manifest drift inpackages/shared/src/agent-discovery.tscan break tooling/crawlers and produce confusing failures — align HEAD headers with GET, catch fetch errors with clear responses, and keep advertised prompts in sync.
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/db/src/drizzle/schema/identity.ts">
<violation number="1" location="packages/db/src/drizzle/schema/identity.ts:60">
P2: Alias rows can drift from actual profiles because `profile_aliases.profileId` is not constrained to `profiles`. Adding a composite foreign key on `(website_id, profile_id)` would preserve identity-graph integrity and prevent dangling aliases.</violation>
</file>
<file name="apps/basket/src/routes/identify.ts">
<violation number="1" location="apps/basket/src/routes/identify.ts:41">
P2: API-key `/identify` can still mutate identity data for inactive websites because `denyApiKeyIdentify` only checks existence/scope/org and skips `website.status`. Consider rejecting non-`ACTIVE` websites in this gate and keeping `/identify` and `/track` website-status behavior aligned.
(Based on your team's feedback about keeping ingest website-status checks aligned across /identify and /track.) [FEEDBACK_USED].</violation>
<violation number="2" location="apps/basket/src/routes/identify.ts:109">
P2: `/identify` rate limiting is keyed by an IP value that comes from `extractIpFromRequest`, so spoofable forwarded-IP input can generate new limiter principals and weaken throttling. Using trusted client IP extraction (with a stable fallback) for the key would make abuse limits harder to evade.</violation>
</file>
<file name="apps/docs/app/.well-known/mcp.json/route.ts">
<violation number="1" location="apps/docs/app/.well-known/mcp.json/route.ts:1">
P2: This new `.well-known/mcp.json` route is a duplicate of `apps/docs/app/mcp.json/route.ts` — both serve the same MCP manifest via `createMcpManifest()` with identical caching. If the intent is to offer the manifest at the standard well-known path, consider replacing the original route with a redirect or keeping only one to avoid serving duplicate content that could drift.</violation>
</file>
<file name="packages/devtools/src/core/types.ts">
<violation number="1" location="packages/devtools/src/core/types.ts:140">
P2: The `identityChanged` function in `store.ts` doesn't check `profileId`, so the UI won't re-render when the profile ID changes. Add `a.profileId !== b.profileId ||` to the comparison to keep change detection working for the new field.</violation>
</file>
<file name="packages/rpc/src/routers/profiles.ts">
<violation number="1" location="packages/rpc/src/routers/profiles.ts:124">
P3: Profile response shaping is duplicated across `getByIds` and `get`, so future edits to profile fields or PII handling can easily diverge between endpoints. Consider extracting a shared profile select/format helper so both routes stay aligned.</violation>
</file>
<file name="apps/docs/app/llms.txt/route.ts">
<violation number="1" location="apps/docs/app/llms.txt/route.ts:21">
P3: The "sdk" section heading will render as lowercase "sdk" instead of "SDK" — the SECTION_LABELS entry is missing. The parallel `llms-full.txt` route already has `sdk: "SDK"`; add it here too for consistency.</violation>
</file>
<file name="apps/docs/components/structured-data.tsx">
<violation number="1" location="apps/docs/components/structured-data.tsx:256">
P2: Structured data now always advertises speakable selectors, but many pages using `StructuredData` do not render `#hero`, `#agent-summary`, or `#faq`. This can generate invalid/low-quality JSON-LD on those routes; making `speakable` page-specific (or omitting it by default) would keep markup accurate.</violation>
</file>
<file name="packages/sdk/src/core/types.ts">
<violation number="1" location="packages/sdk/src/core/types.ts:153">
P3: Profile trait typing is now duplicated across core and node type modules, which raises drift risk between SDK entry points. A shared `ProfileTraits` source (or re-export) would keep browser/node contracts aligned as identity fields evolve.</violation>
</file>
<file name="apps/api/src/routes/query.ts">
<violation number="1" location="apps/api/src/routes/query.ts:344">
P3: WWW-Authenticate header value is duplicated across `createAuthFailedResponse` and `createErrorResponse`. If the OAuth resource metadata URL or header format changes, both sites need updating independently.
Since `createAuthFailedResponse` always returns 401 with the same body shape as `createErrorResponse`, consider consolidating: `createAuthFailedResponse` could delegate to `createErrorResponse("Authentication required", "AUTH_REQUIRED", 401, requestId)`. At minimum, add a cross-reference comment between the two functions so future edits to one prompt review of the other.</violation>
</file>
<file name="apps/docs/app/developers.md/route.ts">
<violation number="1" location="apps/docs/app/developers.md/route.ts:1">
P2: This new route at `/developers.md` duplicates the identical route at `/developer.md` (singular). Both export the same handler calling `createScopedLlmsText("developers")`. Having two URLs serving the exact same content introduces a maintenance liability — future changes must update both files in sync, and it's unclear which path is canonical. If the plural path is the intended canonical route, consider removing or redirecting from the singular `/developer.md` instead of keeping both.</violation>
</file>
<file name="packages/ai/src/mcp/http.ts">
<violation number="1" location="packages/ai/src/mcp/http.ts:247">
P3: Tool UI metadata is now defined in both runtime registration and shared discovery generation, which increases drift risk when one side is updated without the other. A shared helper/constant for the `_meta` payload would keep MCP manifest output and live tool registration aligned.</violation>
</file>
<file name="packages/sdk/src/node/index.ts">
<violation number="1" location="packages/sdk/src/node/index.ts:189">
P2: Identify now fails locally whenever `websiteId` is omitted, which blocks valid API-key flows that rely on server-side website resolution and is inconsistent with the optional `websiteId` contract in SDK/server types. Consider allowing `identify()` to send without `websiteId` (like `track`) and only include it when provided.</violation>
</file>
<file name="apps/docs/app/.well-known/api-catalog/route.ts">
<violation number="1" location="apps/docs/app/.well-known/api-catalog/route.ts:9">
P2: The HEAD handler returns different headers than GET, which violates HTTP semantics (RFC 9110 §9.3.2: HEAD must have the same headers as GET). It also breaks the established codebase pattern — all other well-known routes let Next.js handle HEAD by calling GET and stripping the body. The Link header pointing to itself with an unregistered `rel="api-catalog"` relation type provides no discovery value that the well-known URI itself doesn't already provide. Recommend removing the HEAD export so Next.js auto-handles HEAD requests.</violation>
</file>
<file name="packages/sdk/tests/node-client.test.ts">
<violation number="1" location="packages/sdk/tests/node-client.test.ts:229">
P3: This test validates the websiteId override in the request body but never checks `result.success`. Since the mock always returns a successful response, the test won't currently fail — but if the implementation changed to silently return an error (e.g., a validation short-circuit or auth failure), this test would still pass and miss the regression. Store the result and add `expect(result.success).toBe(true)` to make the assertion complete.</violation>
</file>
<file name="apps/docs/app/openapi.json/route.ts">
<violation number="1" location="apps/docs/app/openapi.json/route.ts:6">
P2: Unhandled fetch rejection on network errors: if the API is unreachable (DNS failure, connection refused, timeout), the `fetch` throws and Next.js returns a generic 500 instead of a meaningful user-facing message. Wrap the fetch in a try-catch and return the same 502 JSON error on failure so transient network issues are surfaced gracefully.</violation>
</file>
<file name="apps/basket/src/routes/track-event-schema.ts">
<violation number="1" location="apps/basket/src/routes/track-event-schema.ts:55">
P2: profileId should accept null like sibling schemas for consistency</violation>
</file>
<file name="apps/docs/app/(home)/about/page.tsx">
<violation number="1" location="apps/docs/app/(home)/about/page.tsx:22">
P2: The Open Graph metadata references `/og-image.png` but this file doesn't exist in the public directory. Social platforms (Twitter, LinkedIn, Discord) will show a broken preview image when this page is shared. Either add an `og-image.png` to `apps/docs/public/` or remove the `images` entry until the asset is available.</violation>
</file>
<file name="apps/docs/app/ask/route.ts">
<violation number="1" location="apps/docs/app/ask/route.ts:7">
P3: `revalidate = 0` is a no-op on this POST-only route. Next.js only applies this config to statically-rendered GET routes and pages. Since this file only exports `POST`, the constant has no effect on caching or freshness — it suggests caching intent that isn't wired to anything.</violation>
<violation number="2" location="apps/docs/app/ask/route.ts:26">
P2: `Cache-Control: public, max-age=300, must-revalidate` on a POST response is unlikely to work as intended. Standard shared caches require a `Content-Location` header to cache POST responses keyed to the request URI, and they don't vary on the request body. Without it, if a CDN does honor the directive (e.g. via custom cache rules), a cached response from one query could be served for a different query — the response includes the query text, so subsequent callers would see the wrong query echoed back.</violation>
</file>
<file name="apps/docs/app/(home)/developers/page.tsx">
<violation number="1" location="apps/docs/app/(home)/developers/page.tsx:34">
P2: `datePublished` in the documentation structured-data element uses a dynamic build-time timestamp (`new Date().toISOString()`) instead of a fixed original publication date. Schema.org specifies that `datePublished` should be the date of first publication — changing it on every build misleads search engines about content age and can hurt freshness signals in SERP. Set `datePublished` to a hardcoded string (e.g., the release date of this page) and reserve the dynamic value for `dateModified` only.</violation>
<violation number="2" location="apps/docs/app/(home)/developers/page.tsx:58">
P3: The `page` prop passed to `StructuredData` omits `datePublished` and `dateModified`, so the WebPage node in the JSON-LD graph will have no date metadata. The documentation element inside `elements` does carry dates, creating an inconsistency. Consider passing the same dates to the `page` prop for complete WebPage metadata.</violation>
</file>
<file name="packages/shared/src/agent-discovery.ts">
<violation number="1" location="packages/shared/src/agent-discovery.ts:276">
P2: MCP prompt discovery is inconsistent: `mcp.json` advertises 3 prompts, but server-card/runtime support also includes `flag_rollout_check`. Manifest-only clients can miss a supported workflow; keep the prompt list aligned across discovery outputs.</violation>
</file>
<file name="packages/db/src/clickhouse/schema.ts">
<violation number="1" location="packages/db/src/clickhouse/schema.ts:356">
P3: ClickHouse ALTER TABLE migrations for custom_events and revenue add `profile_id` without the `CODEC(ZSTD(1))` clause, while the CREATE TABLE definitions specify ZSTD compression. Fresh tables will use ZSTD(1) but existing tables migrated in-place will get the default compression codec (typically LZ4). The preferred codec should match across both paths so that the storage layer stays uniform regardless of when the table was created.</violation>
</file>
<file name="apps/api/src/routes/discovery.ts">
<violation number="1" location="apps/api/src/routes/discovery.ts:21">
P1: The `SITE_URL` constant is hardcoded to `"https://www.databuddy.cc"` but should be read from the env config, similar to how `API_URL` is sourced from `config.urls.api`. All discovery endpoints (agent.json, auth.md, MCP manifest, developer resources, sandbox, OAuth metadata) will construct URLs with this hardcoded value. For self-hosted deployments, every generated URL will point to the wrong domain, breaking agent discovery and user-facing documentation links. Add a `site` URL to the env schema and use it here.</violation>
</file>
<file name="packages/services/src/identity.ts">
<violation number="1" location="packages/services/src/identity.ts:16">
P3: `revealPii` identifies encrypted values by checking whether the value starts with the literal prefix `"v1:"`. If a plaintext display name or email happens to start with `"v1:"` and no `DATABUDDY_ENCRYPTION_KEY` is configured, the value is incorrectly classified as encrypted. Since `identitySecret()` returns `""` when no key is set, the function hits the `if (!secret) { return null; }` branch and silently returns `null` — the original plaintext is lost. This affects any deployment running without the encryption key (development, CI, or self-hosted setups before the env var is configured). Wrapping the encrypted payload with an envelope that cannot collide with valid UTF-8 text (e.g., a binary marker or length-prefixed format) would eliminate the ambiguity.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| import { config } from "@databuddy/env/app"; | ||
| import { Elysia } from "elysia"; | ||
|
|
||
| const SITE_URL = "https://www.databuddy.cc"; |
There was a problem hiding this comment.
P1: The SITE_URL constant is hardcoded to "https://www.databuddy.cc" but should be read from the env config, similar to how API_URL is sourced from config.urls.api. All discovery endpoints (agent.json, auth.md, MCP manifest, developer resources, sandbox, OAuth metadata) will construct URLs with this hardcoded value. For self-hosted deployments, every generated URL will point to the wrong domain, breaking agent discovery and user-facing documentation links. Add a site URL to the env schema and use it here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/routes/discovery.ts, line 21:
<comment>The `SITE_URL` constant is hardcoded to `"https://www.databuddy.cc"` but should be read from the env config, similar to how `API_URL` is sourced from `config.urls.api`. All discovery endpoints (agent.json, auth.md, MCP manifest, developer resources, sandbox, OAuth metadata) will construct URLs with this hardcoded value. For self-hosted deployments, every generated URL will point to the wrong domain, breaking agent discovery and user-facing documentation links. Add a `site` URL to the env schema and use it here.</comment>
<file context>
@@ -0,0 +1,186 @@
+import { config } from "@databuddy/env/app";
+import { Elysia } from "elysia";
+
+const SITE_URL = "https://www.databuddy.cc";
+const API_URL = config.urls.api;
+
</file context>
| }, | ||
| (table) => [ | ||
| primaryKey({ columns: [table.websiteId, table.anonymousId] }), | ||
| index("profile_aliases_profile_idx").on(table.websiteId, table.profileId), |
There was a problem hiding this comment.
P2: Alias rows can drift from actual profiles because profile_aliases.profileId is not constrained to profiles. Adding a composite foreign key on (website_id, profile_id) would preserve identity-graph integrity and prevent dangling aliases.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/src/drizzle/schema/identity.ts, line 60:
<comment>Alias rows can drift from actual profiles because `profile_aliases.profileId` is not constrained to `profiles`. Adding a composite foreign key on `(website_id, profile_id)` would preserve identity-graph integrity and prevent dangling aliases.</comment>
<file context>
@@ -0,0 +1,62 @@
+ },
+ (table) => [
+ primaryKey({ columns: [table.websiteId, table.anonymousId] }),
+ index("profile_aliases_profile_idx").on(table.websiteId, table.profileId),
+ ]
+);
</file context>
|
|
||
| return { | ||
| websiteId: clientId, | ||
| rateLimitPrincipal: `identify:${clientId}:${ip}`, |
There was a problem hiding this comment.
P2: /identify rate limiting is keyed by an IP value that comes from extractIpFromRequest, so spoofable forwarded-IP input can generate new limiter principals and weaken throttling. Using trusted client IP extraction (with a stable fallback) for the key would make abuse limits harder to evade.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/basket/src/routes/identify.ts, line 109:
<comment>`/identify` rate limiting is keyed by an IP value that comes from `extractIpFromRequest`, so spoofable forwarded-IP input can generate new limiter principals and weaken throttling. Using trusted client IP extraction (with a stable fallback) for the key would make abuse limits harder to evade.</comment>
<file context>
@@ -0,0 +1,166 @@
+
+ return {
+ websiteId: clientId,
+ rateLimitPrincipal: `identify:${clientId}:${ip}`,
+ rateLimitPerMinute: 60,
+ };
</file context>
| if (!hasWebsiteScope(apiKey, websiteId, "track:events")) { | ||
| return "missing_scope"; | ||
| } | ||
| if (!website) { |
There was a problem hiding this comment.
P2: API-key /identify can still mutate identity data for inactive websites because denyApiKeyIdentify only checks existence/scope/org and skips website.status. Consider rejecting non-ACTIVE websites in this gate and keeping /identify and /track website-status behavior aligned.
(Based on your team's feedback about keeping ingest website-status checks aligned across /identify and /track.) .
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/basket/src/routes/identify.ts, line 41:
<comment>API-key `/identify` can still mutate identity data for inactive websites because `denyApiKeyIdentify` only checks existence/scope/org and skips `website.status`. Consider rejecting non-`ACTIVE` websites in this gate and keeping `/identify` and `/track` website-status behavior aligned.
(Based on your team's feedback about keeping ingest website-status checks aligned across /identify and /track.) .</comment>
<file context>
@@ -0,0 +1,166 @@
+ if (!hasWebsiteScope(apiKey, websiteId, "track:events")) {
+ return "missing_scope";
+ }
+ if (!website) {
+ return "website_not_found";
+ }
</file context>
| @@ -0,0 +1,7 @@ | |||
| import { agentJsonResponse, createMcpManifest } from "@/lib/agent-discovery"; | |||
There was a problem hiding this comment.
P2: This new .well-known/mcp.json route is a duplicate of apps/docs/app/mcp.json/route.ts — both serve the same MCP manifest via createMcpManifest() with identical caching. If the intent is to offer the manifest at the standard well-known path, consider replacing the original route with a redirect or keeping only one to avoid serving duplicate content that could drift.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/app/.well-known/mcp.json/route.ts, line 1:
<comment>This new `.well-known/mcp.json` route is a duplicate of `apps/docs/app/mcp.json/route.ts` — both serve the same MCP manifest via `createMcpManifest()` with identical caching. If the intent is to offer the manifest at the standard well-known path, consider replacing the original route with a redirect or keeping only one to avoid serving duplicate content that could drift.</comment>
<file context>
@@ -0,0 +1,7 @@
+import { agentJsonResponse, createMcpManifest } from "@/lib/agent-discovery";
+
+export const revalidate = 3600;
</file context>
| }); | ||
| }); | ||
|
|
||
| it("prefers per-call websiteId over the config default", async () => { |
There was a problem hiding this comment.
P3: This test validates the websiteId override in the request body but never checks result.success. Since the mock always returns a successful response, the test won't currently fail — but if the implementation changed to silently return an error (e.g., a validation short-circuit or auth failure), this test would still pass and miss the regression. Store the result and add expect(result.success).toBe(true) to make the assertion complete.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/sdk/tests/node-client.test.ts, line 229:
<comment>This test validates the websiteId override in the request body but never checks `result.success`. Since the mock always returns a successful response, the test won't currently fail — but if the implementation changed to silently return an error (e.g., a validation short-circuit or auth failure), this test would still pass and miss the regression. Store the result and add `expect(result.success).toBe(true)` to make the assertion complete.</comment>
<file context>
@@ -201,3 +201,73 @@ describe("Databuddy Node client", () => {
+ });
+ });
+
+ it("prefers per-call websiteId over the config default", async () => {
+ const calls = mockFetch(() =>
+ jsonResponse({ status: "success", type: "identify" })
</file context>
| parseNlwebAskBody, | ||
| } from "@/lib/agent-discovery"; | ||
|
|
||
| export const revalidate = 0; |
There was a problem hiding this comment.
P3: revalidate = 0 is a no-op on this POST-only route. Next.js only applies this config to statically-rendered GET routes and pages. Since this file only exports POST, the constant has no effect on caching or freshness — it suggests caching intent that isn't wired to anything.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/app/ask/route.ts, line 7:
<comment>`revalidate = 0` is a no-op on this POST-only route. Next.js only applies this config to statically-rendered GET routes and pages. Since this file only exports `POST`, the constant has no effect on caching or freshness — it suggests caching intent that isn't wired to anything.</comment>
<file context>
@@ -0,0 +1,36 @@
+ parseNlwebAskBody,
+} from "@/lib/agent-discovery";
+
+export const revalidate = 0;
+
+async function readBody(request: Request) {
</file context>
Suggested change
| export const revalidate = 0; | |
| // (remove line) |
| }, | ||
| }, | ||
| ]} | ||
| page={{ |
There was a problem hiding this comment.
P3: The page prop passed to StructuredData omits datePublished and dateModified, so the WebPage node in the JSON-LD graph will have no date metadata. The documentation element inside elements does carry dates, creating an inconsistency. Consider passing the same dates to the page prop for complete WebPage metadata.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/app/(home)/developers/page.tsx, line 58:
<comment>The `page` prop passed to `StructuredData` omits `datePublished` and `dateModified`, so the WebPage node in the JSON-LD graph will have no date metadata. The documentation element inside `elements` does carry dates, creating an inconsistency. Consider passing the same dates to the `page` prop for complete WebPage metadata.</comment>
<file context>
@@ -0,0 +1,107 @@
+ },
+ },
+ ]}
+ page={{
+ title,
+ description,
</file context>
|
|
||
| anonymous_id Nullable(String) CODEC(ZSTD(1)), | ||
| profile_id String DEFAULT '' CODEC(ZSTD(1)), |
There was a problem hiding this comment.
P3: ClickHouse ALTER TABLE migrations for custom_events and revenue add profile_id without the CODEC(ZSTD(1)) clause, while the CREATE TABLE definitions specify ZSTD compression. Fresh tables will use ZSTD(1) but existing tables migrated in-place will get the default compression codec (typically LZ4). The preferred codec should match across both paths so that the storage layer stays uniform regardless of when the table was created.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/src/clickhouse/schema.ts, line 356:
<comment>ClickHouse ALTER TABLE migrations for custom_events and revenue add `profile_id` without the `CODEC(ZSTD(1))` clause, while the CREATE TABLE definitions specify ZSTD compression. Fresh tables will use ZSTD(1) but existing tables migrated in-place will get the default compression codec (typically LZ4). The preferred codec should match across both paths so that the storage layer stays uniform regardless of when the table was created.</comment>
<file context>
@@ -348,16 +351,18 @@ CREATE TABLE IF NOT EXISTS ${DATABASES.ANALYTICS}.custom_events (
-
+
anonymous_id Nullable(String) CODEC(ZSTD(1)),
+ profile_id String DEFAULT '' CODEC(ZSTD(1)),
session_id Nullable(String) CODEC(ZSTD(1)),
</file context>
| return secret ? encrypt(value, secret) : value; | ||
| } | ||
|
|
||
| export function revealPii(value: string | null): string | null { |
There was a problem hiding this comment.
P3: revealPii identifies encrypted values by checking whether the value starts with the literal prefix "v1:". If a plaintext display name or email happens to start with "v1:" and no DATABUDDY_ENCRYPTION_KEY is configured, the value is incorrectly classified as encrypted. Since identitySecret() returns "" when no key is set, the function hits the if (!secret) { return null; } branch and silently returns null — the original plaintext is lost. This affects any deployment running without the encryption key (development, CI, or self-hosted setups before the env var is configured). Wrapping the encrypted payload with an envelope that cannot collide with valid UTF-8 text (e.g., a binary marker or length-prefixed format) would eliminate the ambiguity.
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 16:
<comment>`revealPii` identifies encrypted values by checking whether the value starts with the literal prefix `"v1:"`. If a plaintext display name or email happens to start with `"v1:"` and no `DATABUDDY_ENCRYPTION_KEY` is configured, the value is incorrectly classified as encrypted. Since `identitySecret()` returns `""` when no key is set, the function hits the `if (!secret) { return null; }` branch and silently returns `null` — the original plaintext is lost. This affects any deployment running without the encryption key (development, CI, or self-hosted setups before the env var is configured). Wrapping the encrypted payload with an envelope that cannot collide with valid UTF-8 text (e.g., a binary marker or length-prefixed format) would eliminate the ambiguity.</comment>
<file context>
@@ -0,0 +1,145 @@
+ return secret ? encrypt(value, secret) : value;
+}
+
+export function revealPii(value: string | null): string | null {
+ if (!(value && value.startsWith(ENCRYPTED_PREFIX))) {
+ return value;
</file context>
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Shadow auto-approve: would not auto-approve. Auto-approval blocked by 26 unresolved issues from previous reviews.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes staging to production.
Highlights:
Databases are already migrated (additive, idempotent); encryption keys are already set on the affected services.
Summary by cubic
Ships end-to-end user identity with encrypted profile fields at rest and strict env schema validation. Adds agent discovery endpoints and OpenAPI exposure; updates ingestion, SDKs (
@databuddy/sdk@2.5.0), dashboard, queries, and devtools to support identified users.New Features
/identifyingestion with API-key support and website scope; addsprofile_idto events/custom_events/revenue; rate limited and validated.@databuddy/sdk@2.5.0addsidentify,setTraits,clearProfile, andgetProfileId(web and node); tracker persistsdid_profile.profile_id.profile_idfilters, MCP server and manifests published, API 401s includeWWW-Authenticatediscovery hints, and docs expose/.well-known/*,openapi.json, andllms.txt.Migration
DATABUDDY_ENCRYPTION_KEYandIP_HASH_SALTin production; per-app env schemas now validate at boot.@databuddy/sdk@2.5.0and callidentify(userId, { email, name })on login/sign-up; callclearProfile()on logout.databuddy_profile_idmetadata for revenue attribution.profile_idin analytics filters when needed; an “Identified” preset is available in the dashboard.AI_API_KEY,CLICKHOUSE_URL,BETTER_AUTH_URL, andDATABASE_URLset to pass boot-time validation.Written for commit 7ba7a5e. Summary will update on new commits.