feat: Profile identification & traits - #593
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughAdds visitor identification through public analytics endpoints, ClickHouse profile aliases and traits, canonicalized analytics queries, JavaScript and Node tracker APIs, dashboard rendering, cleanup flows, and documentation. ChangesVisitor identity linking
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Visitor
participant Tracker
participant IdentifyAPI
participant AnalyticsService
participant AnalyticsStore
Visitor->>Tracker: identify(profileId, traits)
Tracker->>IdentifyAPI: POST identify
IdentifyAPI->>AnalyticsService: validate request and resolve identities
AnalyticsService->>AnalyticsStore: persist alias and traits
IdentifyAPI-->>Tracker: return canonical profileId
Tracker-->>Visitor: use identified profile for tracking
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
backend/apps/cloud/src/analytics/analytics.service.ts (1)
2466-2499: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider negative caching for unlinked anon lookups.
getUserProfileForAnononly writes to Redis on a positive hit (Line 2494-2496). The overwhelmingly common case — an anonymous profile that was never identified — falls through to aprofile_aliasesClickHouse query on every call. This runs on hot read paths (getSessionDetails,resolveProfileIdentity), so a short-lived negative cache (e.g. a sentinel value with a small TTL) would remove a per-request CH round-trip.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/analytics.service.ts` around lines 2466 - 2499, Update getUserProfileForAnon to cache unlinked lookups using a dedicated sentinel value with a short TTL, return null when that sentinel is read, and continue returning cached userProfileId values normally. Store the sentinel when ClickHouse finds no userProfileId while preserving the existing positive-cache behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/apps/cloud/src/analytics/analytics.controller.ts`:
- Around line 3308-3320: Add the same rate-limit and bot checks used by the
other public ingestion routes to the identify handlers in
backend/apps/cloud/src/analytics/analytics.controller.ts (lines 3308-3320) and
backend/apps/community/src/analytics/analytics.controller.ts (lines 2799-2811),
applying them before validation or the profile-linking flow in identify. Ensure
both public endpoints invoke checkRateLimit and checkBot consistently.
---
Nitpick comments:
In `@backend/apps/cloud/src/analytics/analytics.service.ts`:
- Around line 2466-2499: Update getUserProfileForAnon to cache unlinked lookups
using a dedicated sentinel value with a short TTL, return null when that
sentinel is read, and continue returning cached userProfileId values normally.
Store the sentinel when ClickHouse finds no userProfileId while preserving the
existing positive-cache behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c03edfbe-1647-4222-9c36-f7fe7ee91ce5
📒 Files selected for processing (17)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/cloud/src/analytics/dto/identify.dto.tsbackend/apps/community/src/analytics/analytics.controller.tsbackend/apps/community/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/dto/identify.dto.tsbackend/migrations/clickhouse/2026_07_17_profile_aliases.jsbackend/migrations/clickhouse/initialise_database.jsdocs/content/docs/analytics-dashboard/profiles-and-sessions.mdxdocs/content/docs/api/events.mdxdocs/content/docs/script-reference.mdxdocs/content/docs/visitor-identification.mdxpackages/tracker-js/README.mdpackages/tracker-js/src/Lib.tspackages/tracker-js/src/index.tspackages/tracker-node/README.mdpackages/tracker-node/src/index.ts
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/apps/cloud/src/analytics/analytics.service.ts (1)
4010-4017: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse
buildProfileAliasMapCTE()instead of re-inlining the same SQL.
buildProfileAliasMapCTE()is defined specifically to centralize theprofile_alias_mapCTE, andgetProfilesListcorrectly calls it, butgetFunnelSessionsList,getJourneySessionsList,getSessionsList, andgetSessionReplaysListeach inline an identical copy of the same SQL text instead. This is exact duplication of the alias-resolution logic across 4 call sites — a future fix/tweak to how aliases are resolved (e.g. adding a filter, changing the aggregation) is easy to apply in one place and silently miss the other three.
CTE mapping anonymous profile IDs to the identified profiles they are linked to. LEFT JOIN it on profileId and coalesce(nullIf(pam.userProfileId, ''), profileId) to attribute pre-identification events to the identified profile.♻️ Proposed fix (apply to all 4 sites)
const query = ` - WITH profile_alias_map AS ( - SELECT - anonProfileId, - argMin(userProfileId, created) AS userProfileId - FROM profile_aliases - WHERE pid = {pid:FixedString(12)} - GROUP BY anonProfileId - ), + WITH ${this.buildProfileAliasMapCTE()}, funnel_qualified AS (Also applies to: 4179-4186, 8076-8083, 8332-8339
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/apps/cloud/src/analytics/analytics.service.ts` around lines 4010 - 4017, Replace the duplicated profile_alias_map CTE SQL in getFunnelSessionsList, getJourneySessionsList, getSessionsList, and getSessionReplaysList with calls to buildProfileAliasMapCTE(). Preserve each query’s existing alias-map LEFT JOIN and coalesce(nullIf(pam.userProfileId, ''), profileId) attribution behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/apps/cloud/src/analytics/dto/identify.dto.ts`:
- Around line 36-58: Update TraitsValueType.validate in both
backend/apps/cloud/src/analytics/dto/identify.dto.ts (lines 36-58) and
backend/apps/community/src/analytics/dto/identify.dto.ts (lines 36-58) to reject
string values matching UNPRINTABLE_REGEX, while retaining the existing
non-string validation.
- Around line 83-106: Prevent prototype-polluting trait keys in transformTraits
by rejecting __proto__, constructor, and prototype before assigning transformed
values; apply this change in both
backend/apps/cloud/src/analytics/dto/identify.dto.ts (lines 83-106) and
backend/apps/community/src/analytics/dto/identify.dto.ts (lines 83-106).
Preserve the existing normalization behavior for all other keys.
In `@docs/content/docs/script-reference.mdx`:
- Around line 196-201: Document that profileId is trimmed before storage: update
docs/content/docs/script-reference.mdx lines 196-201 to say the trimmed ID is
prefixed and displayed; update docs/content/docs/visitor-identification.mdx
lines 146-149 and 212-217 to replace exact-storage wording with normalized
behavior, clarifying that “as provided” excludes surrounding whitespace; and
update packages/tracker-node/README.md line 444 to mention trimming beside the
Node API example.
---
Outside diff comments:
In `@backend/apps/cloud/src/analytics/analytics.service.ts`:
- Around line 4010-4017: Replace the duplicated profile_alias_map CTE SQL in
getFunnelSessionsList, getJourneySessionsList, getSessionsList, and
getSessionReplaysList with calls to buildProfileAliasMapCTE(). Preserve each
query’s existing alias-map LEFT JOIN and coalesce(nullIf(pam.userProfileId, ''),
profileId) attribution behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7260ab6f-e16b-4cb2-bd55-168566b83527
📒 Files selected for processing (29)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/cloud/src/analytics/dto/get-profile.dto.tsbackend/apps/cloud/src/analytics/dto/identify.dto.tsbackend/apps/cloud/src/analytics/v2/dto/entities.dto.tsbackend/apps/cloud/src/project/project.controller.tsbackend/apps/cloud/src/project/project.service.tsbackend/apps/community/src/analytics/analytics.controller.tsbackend/apps/community/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/dto/get-profile.dto.tsbackend/apps/community/src/analytics/dto/identify.dto.tsbackend/apps/community/src/analytics/v2/dto/entities.dto.tsbackend/apps/community/src/project/project.controller.tsbackend/apps/community/src/user/user.controller.tsbackend/migrations/clickhouse/2026_07_28_profile_traits.jsbackend/migrations/clickhouse/initialise_database.jsdocs/.gitignoredocs/content/docs/analytics-dashboard/profiles-and-sessions.mdxdocs/content/docs/api/events.mdxdocs/content/docs/script-reference.mdxdocs/content/docs/visitor-identification.mdxpackages/tracker-js/README.mdpackages/tracker-js/src/Lib.tspackages/tracker-js/src/index.tspackages/tracker-node/README.mdpackages/tracker-node/src/index.tsweb/app/lib/models/Project.tsweb/app/pages/Project/tabs/Profiles/ProfileDetails.tsxweb/public/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (7)
- backend/migrations/clickhouse/initialise_database.js
- docs/content/docs/analytics-dashboard/profiles-and-sessions.mdx
- docs/content/docs/api/events.mdx
- packages/tracker-js/README.md
- backend/apps/community/src/analytics/analytics.controller.ts
- backend/apps/cloud/src/analytics/analytics.controller.ts
- backend/apps/community/src/analytics/analytics.service.ts
| function transformTraits(value: any): any { | ||
| if (!value || typeof value !== 'object' || Array.isArray(value)) { | ||
| return value | ||
| } | ||
|
|
||
| const transformed: Record<string, any> = {} | ||
|
|
||
| for (const key of _keys(value)) { | ||
| const trait = value[key] | ||
|
|
||
| if (trait === null || trait === undefined) { | ||
| transformed[key.trim()] = '' | ||
| } else if (typeof trait === 'number' || typeof trait === 'boolean') { | ||
| transformed[key.trim()] = String(trait) | ||
| } else if (typeof trait === 'string') { | ||
| transformed[key.trim()] = trait.trim() | ||
| } else { | ||
| // Objects and arrays are left as-is for the validation below to reject | ||
| transformed[key.trim()] = trait | ||
| } | ||
| } | ||
|
|
||
| return transformed | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg 'backend/apps/(cloud|community)/src/analytics/dto/identify.dto.ts$' || true
echo
echo "Cloud DTO outline:"
ast-grep outline backend/apps/cloud/src/analytics/dto/identify.dto.ts || true
echo
echo "Community DTO outline:"
ast-grep outline backend/apps/community/src/analytics/dto/identify.dto.ts || true
echo
echo "Relevant cloud DTO lines:"
sed -n '1,180p' backend/apps/cloud/src/analytics/dto/identify.dto.ts
echo
echo "Relevant community DTO lines:"
sed -n '1,180p' backend/apps/community/src/analytics/dto/identify.dto.ts
echo
echo "Search for DTO decorators/usages:"
rg -n "TraitsKeysQuantity|TraitsKeyFormat|TraitsValueType|TraitsSizeLimit|identify\\.dto|identify dto|Dto" backend/apps/cloud backend/apps/community -g '*.ts' | head -200Repository: Swetrix/swetrix
Length of output: 30218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search exact IdentifyDto references:"
rg -n "IdentifyDto|identifyDto|identify\\(" backend/apps backend -g '*.ts' | head -200
echo
echo "Git diff stat/name-only:"
git diff --stat
git diff --name-only
echo
echo "Package/class-validator versions if present:"
for f in package.json backend/apps/cloud/package.json backend/apps/community/package.json; do
[ -f "$f" ] && { echo "--- $f"; jq '.dependencies, .devDependencies' "$f" | rg '"(class-validator|class-transformer|lodash)"' || true; }
done
echo
echo "Behavioral prototype pollution probe (no repository execution):"
node - <<'JS'
const _keys = require('lodash/keys');
function transformTraits(value) {
if (!value || typeof value !== 'object' || Array.isArray(value)) {
return value
}
const transformed = {};
for (const key of _keys(value)) {
const trait = value[key];
if (trait === null || trait === undefined) {
transformed[key.trim()] = '';
} else if (typeof trait === 'number' || typeof trait === 'boolean') {
transformed[key.trim()] = String(trait);
} else if (typeof trait === 'string') {
transformed[key.trim()] = trait.trim();
} else {
transformed[key.trim()] = trait;
}
}
return transformed;
}
const input = JSON.parse('{"__proto__":{"bad":true},"safe":"x"}');
const result = transformTraits(input);
console.log(JSON.stringify({
ownKeys: Object.keys(result),
hasOwnBad: Object.prototype.hasOwnProperty.call(result, 'bad'),
hasProtoSet: Object.prototype.hasOwnProperty.call(Object.prototype, 'bad'),
protoHasBad: Object.getPrototypeOf(result).bad,
resultBadValue: result.bad,
keysLength: _keys(result).length
}));
JSRepository: Swetrix/swetrix
Length of output: 4869
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Analytic controller identify route signatures:"
sed -n '3180,3340p' backend/apps/cloud/src/analytics/analytics.controller.ts
sed -n '2780,2830p' backend/apps/community/src/analytics/analytics.controller.ts
echo
echo "Prototype pollution probe without lodash:"
node - <<'JS'
function ownKeysSimple(obj) {
return Object.keys(obj);
}
function transformTraits(value) {
if (!value || typeof value !== 'object' || Array.isArray(value)) return value;
const transformed = {};
for (const key of ownKeysSimple(value)) {
const trait = value[key];
if (trait === null || trait === undefined) transformed[key.trim()] = '';
else if (typeof trait === 'number' || typeof trait === 'boolean') transformed[key.trim()] = String(trait);
else if (typeof trait === 'string') transformed[key.trim()] = trait.trim();
else transformed[key.trim()] = trait;
}
return transformed;
}
const result = transformTraits({ ['__proto__']: { bad: true }, safe: 'x' });
console.log(JSON.stringify({
ownKeys: Object.keys(result),
hasOwnBad: Object.prototype.hasOwnProperty.call(result, 'bad'),
hasProtoBadOnObjectProto: Object.prototype.hasOwnProperty.call(Object.prototype, 'bad'),
prototypeHasBad: Object.getPrototypeOf(result).bad,
resultBadValue: result.bad,
keysLength: ownKeysSimple(result).length
}));
JSRepository: Swetrix/swetrix
Length of output: 5891
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Trait validation after prototype pollution (Node, ownKeys implementation) probe:"
node - <<'JS'
const MAX_TRAITS_KEYS = 50;
const MAX_TRAIT_KEY_LENGTH = 128;
const MAX_TRAITS_TOTAL_LENGTH = 2000;
const UNPRINTABLE_REGEX = /[\p{Cc}\p{Cf}]/u;
function ownKeysSimple(obj) {
return Object.keys(obj);
}
function valuesSimple(obj) {
const keys = Object.keys(obj);
const vals = [];
for (let i = 0; i < keys.length; i++) vals.push(obj[keys[i]]);
return vals;
}
function transformTraits(value) {
if (!value || typeof value !== 'object' || Array.isArray(value)) return value;
const transformed = {};
for (const key of ownKeysSimple(value)) {
const trait = value[key];
if (trait === null || trait === undefined) transformed[key.trim()] = '';
else if (typeof trait === 'number' || typeof trait === 'boolean') transformed[key.trim()] = String(trait);
else if (typeof trait === 'string') transformed[key.trim()] = trait.trim();
else transformed[key.trim()] = trait;
}
return transformed;
}
const input = { `__proto__`: { bad: true }, safe: 'x' };
const result = transformTraits(input);
const keysQuantityPass = ownKeysSimple(result).length <= MAX_TRAITS_KEYS;
const keyFormatPass = (() => {
const keys = ownKeysSimple(result);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
if (!key.trim() || key.length > MAX_TRAIT_KEY_LENGTH || UNPRINTABLE_REGEX.test(key)) return false;
}
return true;
})();
const valueTypePass = (() => {
const vals = valuesSimple(result);
for (let i = 0; i < vals.length; i++) {
if (typeof vals[i] !== 'string') return false;
}
return true;
})();
let totalSize = 0;
for (const key of ownKeysSimple(result)) {
const value = result[key];
totalSize += key.length + (typeof value === 'string' ? value.length : 0);
if (totalSize > MAX_TRAITS_TOTAL_LENGTH) totalSize = Infinity;
}
const sizeLimitPass = totalSize <= MAX_TRAITS_TOTAL_LENGTH;
console.log(JSON.stringify({
ownKeys: Object.keys(result),
keysLength: ownKeysSimple(result).length,
resultBadValue: result.bad,
prototypeHasBad: Object.getPrototypeOf(result).bad,
validators: {
keysQuantity: keysQuantityPass,
keyFormat: keyFormatPass,
valueType: valueTypePass,
sizeLimit: sizeLimitPass
}
}));
JSRepository: Swetrix/swetrix
Length of output: 993
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Trait validation after prototype pollution (Node, safe literal) probe:"
node - <<'JS'
const MAX_TRAITS_KEYS = 50;
const MAX_TRAIT_KEY_LENGTH = 128;
const MAX_TRAITS_TOTAL_LENGTH = 2000;
const UNPRINTABLE_REGEX = /[\p{Cc}\p{Cf}]/u;
function ownKeysSimple(obj) {
return Object.keys(obj);
}
function valuesSimple(obj) {
const keys = Object.keys(obj);
const vals = [];
for (let i = 0; i < keys.length; i++) vals.push(obj[keys[i]]);
return vals;
}
function transformTraits(value) {
if (!value || typeof value !== 'object' || Array.isArray(value)) return value;
const transformed = {};
for (const key of ownKeysSimple(value)) {
const trait = value[key];
if (trait === null || trait === undefined) transformed[key.trim()] = '';
else if (typeof trait === 'number' || typeof trait === 'boolean') transformed[key.trim()] = String(trait);
else if (typeof trait === 'string') transformed[key.trim()] = trait.trim();
else transformed[key.trim()] = trait;
}
return transformed;
}
const input = { ['__proto__']: { bad: true }, safe: 'x' };
const result = transformTraits(input);
const keysQuantityPass = ownKeysSimple(result).length <= MAX_TRAITS_KEYS;
const keyFormatPass = (() => {
const keys = ownKeysSimple(result);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
if (!key.trim() || key.length > MAX_TRAIT_KEY_LENGTH || UNPRINTABLE_REGEX.test(key)) return false;
}
return true;
})();
const valueTypePass = (() => {
const vals = valuesSimple(result);
for (let i = 0; i < vals.length; i++) {
if (typeof vals[i] !== 'string') return false;
}
return true;
})();
let totalSize = 0;
for (const key of ownKeysSimple(result)) {
const value = result[key];
totalSize += key.length + (typeof value === 'string' ? value.length : 0);
if (totalSize > MAX_TRAITS_TOTAL_LENGTH) totalSize = Infinity;
}
const sizeLimitPass = totalSize <= MAX_TRAITS_TOTAL_LENGTH;
console.log(JSON.stringify({
ownKeys: Object.keys(result),
keysLength: ownKeysSimple(result).length,
resultBadValue: result.bad,
prototypeHasBad: Object.getPrototypeOf(result).bad,
validators: {
keysQuantity: keysQuantityPass,
keyFormat: keyFormatPass,
valueType: valueTypePass,
sizeLimit: sizeLimitPass
}
}));
JSRepository: Swetrix/swetrix
Length of output: 391
Other (CWE-1321): Improperly Controlled Modification of Object Prototype Attributes ('Prototype Pollution')
Reachability: External
● Entry
backend/apps/cloud/src/analytics/analytics.controller.ts:3313
identify: Flip the visitor's current session (if any) to the identified profile.
│
▼
● Sink
backend/apps/cloud/src/analytics/dto/identify.dto.ts
Prevent trait-key prototype pollution in both DTOs. transformTraits rebuilds traits into a fresh {} and assigns with transformed[key.trim()] = ..., so a crafted __proto__ object key can mutate Object.prototype for the transformed payload. That bypasses the subsequent trait validators and can carry injected properties into identity storage; reject dangerous keys like __proto__, constructor, and prototype before assigning.
📍 Affects 2 files
backend/apps/cloud/src/analytics/dto/identify.dto.ts#L83-L106(this comment)backend/apps/community/src/analytics/dto/identify.dto.ts#L83-L106
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/apps/cloud/src/analytics/dto/identify.dto.ts` around lines 83 - 106,
Prevent prototype-polluting trait keys in transformTraits by rejecting
__proto__, constructor, and prototype before assigning transformed values; apply
this change in both backend/apps/cloud/src/analytics/dto/identify.dto.ts (lines
83-106) and backend/apps/community/src/analytics/dto/identify.dto.ts (lines
83-106). Preserve the existing normalization behavior for all other keys.
Changes
If applicable, please describe what changes were made in this pull request.
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit
New Features
/analytics/identify//log/identifysupport with bot-safe handling and rate limiting.Documentation
identify/setTraits/resetguidance, profile ID rules, and trait semantics.