Conversation
…lLimit to recent stats request. Update loadRecentSignUpStats function to include email normalization checks. Adjust tests to reflect new return structure.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR enhances the risk-scoring system by adding normalized email tracking to sign-up risk assessment. It introduces fields to capture and count sign-ups with the same normalized email address within a configurable time window, supported by a new parallel database query. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR extends the sign-up risk assessment system by adding a new Key changes:
Issues found:
Confidence Score: 4/5Safe to merge functionally, but missing a DB index will cause query performance regressions at scale on every sign-up. The logic change is correct and well-tested. The single blocking concern is a missing composite index for the new signUpEmailNormalized query — without it, the query falls back to a table scan on each sign-up, matching the P1 severity pattern of the existing sibling indexes. apps/backend/prisma/schema.prisma — needs a new index for signUpEmailNormalized to match the pattern of existing signUpIp and signUpEmailBase indexes. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Sign-up request] --> B[calculateSignUpRiskAssessment]
B --> C[signUpRiskEngine.calculateRiskAssessment]
C --> D[loadRecentSignUpStats]
D --> E{signUpIp != null & sameIpLimit > 0?}
D --> F{signUpEmailNormalized != null & sameEmailLimit > 0?}
D --> G{signUpEmailBase != null & similarEmailLimit > 0?}
E -->|Yes| H[Query: sameIpRows - index: signUpIp_recent_idx]
E -->|No| I[return empty]
F -->|Yes| J[Query: sameEmailRows - index: MISSING]
F -->|No| K[return empty]
G -->|Yes| L[Query: similarEmailRows - index: signUpEmailBase_recent_idx]
G -->|No| M[return empty]
H & J & L --> N[Promise.all]
I & K & M --> N
N --> O[Return: sameIpCount, sameEmailCount, similarEmailCount]
O --> C
|
There was a problem hiding this comment.
Pull request overview
Enhances the backend sign-up risk assessment plumbing by extending “recent sign-up stats” to include same-normalized-email counts/limits, and updates dependency lockfile entries likely due to a submodule/dependency refresh.
Changes:
- Extend recent stats request/response types to include
signUpEmailNormalized,sameEmailLimit, andsameEmailCount. - Add a DB query to count recent sign-ups matching the same normalized email (bounded by
sameEmailLimit). - Update the in-source Vitest stubbed dependency return shape to match the new stats structure.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates dependency resolution (notably Nitro now pulling in rolldown) and removes an importer entry. |
| apps/backend/src/lib/risk-scores.tsx | Adds same-normalized-email recent stats fields and query; updates test stub return shape. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SELECT 1 AS "matched" | ||
| FROM ${sqlQuoteIdent(schema)}."ProjectUser" | ||
| WHERE "tenancyId" = ${tenancy.id}::UUID | ||
| AND "isAnonymous" = false | ||
| AND "signedUpAt" >= ${windowStart} | ||
| AND "signUpEmailNormalized" = ${request.signUpEmailNormalized} | ||
| LIMIT ${request.sameEmailLimit} |
There was a problem hiding this comment.
The new recent-stats query filters on "signUpEmailNormalized" and "signedUpAt" (plus tenancyId/isAnonymous), but the DB migration set currently only adds a recent index for signUpIp and signUpEmailBase (no signUpEmailNormalized). This will likely result in sequential scans on ProjectUser for tenants with many users. Consider adding a concurrent index similar to ProjectUser_signUpEmailBase_recent_idx, e.g. (tenancyId, isAnonymous, signUpEmailNormalized, signedUpAt), or otherwise ensuring this query is backed by an index.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/lib/risk-scores.tsx`:
- Around line 83-93: The query in apps/backend/src/lib/risk-scores.tsx that uses
request.signUpEmailNormalized (inside the prisma.$replica().$queryRaw block that
checks sameEmailLimit) needs a corresponding DB index to avoid full table scans;
add the index to migration 20260308000002_finalize_signup_fraud_protection.sql
(CREATE INDEX CONCURRENTLY IF NOT EXISTS
"ProjectUser_signUpEmailNormalized_recent_idx" ON "ProjectUser"("tenancyId",
"isAnonymous", "signUpEmailNormalized", "signedUpAt")) and add the matching
Prisma index annotation to the ProjectUser model (@@index([tenancyId,
isAnonymous, signUpEmailNormalized, signedUpAt], name:
"ProjectUser_signUpEmailNormalized_recent_idx")) so the signUpEmailNormalized
filter in the sameEmailLimit query can use the index.
In `@apps/backend/src/private/implementation`:
- Line 1: Verify the private submodule update by checking the commit range
between a93d7ea0c0a91d7a4dfbc97c0032c9c9c68ec4d6 and
576f383b69a9593a9cff8d755c64c810aeeae239: run git fetch in the private repo and
git log --oneline a93d7ea..576f383b6 to enumerate commits, then run git diff
--name-status a93d7ea0c..576f383b6 to ensure only the intended risk-score and
signup-stat files were changed (no other files or code paths), inspect each
commit message and diff for breaking/security-impacting edits, run the private
repo’s test suite and any linters/security scanners on the new commits, and if
anything else appears, revert or request the correct commit range before
merging.
🪄 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: 1840880f-79f3-4e1f-95d1-fd9e84e37754
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
apps/backend/src/lib/risk-scores.tsxapps/backend/src/private/implementation
| request.signUpEmailNormalized == null || request.sameEmailLimit === 0 | ||
| ? [] | ||
| : prisma.$replica().$queryRaw<{ matched: number }[]>` | ||
| SELECT 1 AS "matched" | ||
| FROM ${sqlQuoteIdent(schema)}."ProjectUser" | ||
| WHERE "tenancyId" = ${tenancy.id}::UUID | ||
| AND "isAnonymous" = false | ||
| AND "signedUpAt" >= ${windowStart} | ||
| AND "signUpEmailNormalized" = ${request.signUpEmailNormalized} | ||
| LIMIT ${request.sameEmailLimit} | ||
| `, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that no index exists for signUpEmailNormalized
echo "=== Checking Prisma schema for signUpEmailNormalized index ==="
rg -n 'signUpEmailNormalized' apps/backend/prisma/schema.prisma
echo ""
echo "=== Checking migrations for signUpEmailNormalized index ==="
rg -rn 'signUpEmailNormalized' apps/backend/prisma/migrations/Repository: stack-auth/stack-auth
Length of output: 345
🏁 Script executed:
#!/bin/bash
# Get the full ProjectUser model definition to see all indexes
echo "=== Full ProjectUser model definition ==="
sed -n '/^model ProjectUser/,/^}/p' apps/backend/prisma/schema.prisma | head -100Repository: stack-auth/stack-auth
Length of output: 4513
🏁 Script executed:
#!/bin/bash
# Read the specific migration file to see what was actually created
echo "=== Checking migration file for signup fraud protection ==="
find apps/backend/prisma/migrations -name "*signup*fraud*" -o -name "*20260308*" | head -5Repository: stack-auth/stack-auth
Length of output: 478
🏁 Script executed:
#!/bin/bash
# Check for all indexes on ProjectUser in schema
echo "=== All @@index definitions in ProjectUser model ==="
sed -n '/^model ProjectUser/,/^}/p' apps/backend/prisma/schema.prisma | grep -n "@@index"Repository: stack-auth/stack-auth
Length of output: 1001
🏁 Script executed:
#!/bin/bash
# Read the main signup fraud protection migration
echo "=== Migration 20260308000000 ==="
cat apps/backend/prisma/migrations/20260308000000_add_signup_fraud_protection/migration.sqlRepository: stack-auth/stack-auth
Length of output: 1237
🏁 Script executed:
#!/bin/bash
# Read the finalize migration
echo "=== Migration 20260308000002 ==="
cat apps/backend/prisma/migrations/20260308000002_finalize_signup_fraud_protection/migration.sqlRepository: stack-auth/stack-auth
Length of output: 1879
🏁 Script executed:
#!/bin/bash
# Verify the actual code in risk-scores.tsx around lines 83-93
echo "=== Risk scores query code ==="
sed -n '75,100p' apps/backend/src/lib/risk-scores.tsxRepository: stack-auth/stack-auth
Length of output: 1157
Add missing database index for signUpEmailNormalized to prevent full table scans.
The query on line 90 filters by signUpEmailNormalized, but no index exists for this column. The sibling queries on signUpIp and signUpEmailBase have corresponding indexes (ProjectUser_signUpIp_recent_idx and ProjectUser_signUpEmailBase_recent_idx), but signUpEmailNormalized was omitted. Without an index, this query will perform a sequential scan on the ProjectUser table, degrading sign-up latency as the table grows.
Add the index to migration 20260308000002_finalize_signup_fraud_protection.sql:
CREATE INDEX CONCURRENTLY IF NOT EXISTS "ProjectUser_signUpEmailNormalized_recent_idx"
ON "ProjectUser"("tenancyId", "isAnonymous", "signUpEmailNormalized", "signedUpAt");And add it to the Prisma schema:
@@index([tenancyId, isAnonymous, signUpEmailNormalized, signedUpAt], name: "ProjectUser_signUpEmailNormalized_recent_idx")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/src/lib/risk-scores.tsx` around lines 83 - 93, The query in
apps/backend/src/lib/risk-scores.tsx that uses request.signUpEmailNormalized
(inside the prisma.$replica().$queryRaw block that checks sameEmailLimit) needs
a corresponding DB index to avoid full table scans; add the index to migration
20260308000002_finalize_signup_fraud_protection.sql (CREATE INDEX CONCURRENTLY
IF NOT EXISTS "ProjectUser_signUpEmailNormalized_recent_idx" ON
"ProjectUser"("tenancyId", "isAnonymous", "signUpEmailNormalized",
"signedUpAt")) and add the matching Prisma index annotation to the ProjectUser
model (@@index([tenancyId, isAnonymous, signUpEmailNormalized, signedUpAt],
name: "ProjectUser_signUpEmailNormalized_recent_idx")) so the
signUpEmailNormalized filter in the sameEmailLimit query can use the index.
| @@ -1 +1 @@ | |||
| Subproject commit a93d7ea0c0a91d7a4dfbc97c0032c9c9c68ec4d6 | |||
| Subproject commit 576f383b69a9593a9cff8d755c64c810aeeae239 | |||
There was a problem hiding this comment.
❓ Verification inconclusive
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify submodule pointer and inspect exact delta
git submodule status -- apps/backend/src/private/implementation || true
# Ensure submodule metadata is available
git submodule update --init --recursive -- apps/backend/src/private/implementation
OLD="a93d7ea0c0a91d7a4dfbc97c0032c9c9c68ec4d6"
NEW="576f383b69a9593a9cff8d755c64c810aeeae239"
SUB="apps/backend/src/private/implementation"
echo "== Commit range in submodule =="
git -C "$SUB" log --oneline --no-merges "${OLD}..${NEW}"
echo
echo "== Files changed in submodule range =="
git -C "$SUB" diff --name-status "${OLD}..${NEW}"
echo
echo "== High-signal grep for risk-score/signup/email changes =="
git -C "$SUB" diff --name-only "${OLD}..${NEW}" | rg -n "risk|signup|sign-up|email|stats|score" || trueRepository: stack-auth/stack-auth
Length of output: 1239
The private submodule cannot be verified from this environment—manual review required.
Line 1 updates the gitlink to commit 576f383b69a9593a9cff8d755c64c810aeeae239, but the private repository (https://github.com/stack-auth/private.git) is not accessible for automated verification. Before merging, confirm that the commit range from a93d7ea0c0a91d7a4dfbc97c0032c9c9c68ec4d6 to 576f383b69a9593a9cff8d755c64c810aeeae239 contains only the intended risk-score and signup-stat changes, with no unrelated breaking or security-impacting updates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/src/private/implementation` at line 1, Verify the private
submodule update by checking the commit range between
a93d7ea0c0a91d7a4dfbc97c0032c9c9c68ec4d6 and
576f383b69a9593a9cff8d755c64c810aeeae239: run git fetch in the private repo and
git log --oneline a93d7ea..576f383b6 to enumerate commits, then run git diff
--name-status a93d7ea0c..576f383b6 to ensure only the intended risk-score and
signup-stat files were changed (no other files or code paths), inspect each
commit message and diff for breaking/security-impacting edits, run the private
repo’s test suite and any linters/security scanners on the new commits, and if
anything else appears, revert or request the correct commit range before
merging.
Summary by CodeRabbit