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.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR extends the sign-up risk-scoring system to track duplicate sign-ups by normalized email address. It adds fields to risk-scoring data structures, updates the database schema with a new composite index for efficient querying, creates the corresponding migration, and updates a git submodule reference. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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 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 enhances the signup risk assessment system by adding same-email detection alongside the existing same-IP and similar-email-base checks. It introduces a
Confidence Score: 5/5Safe to merge — changes are additive, consistent with existing patterns, and carry no breaking changes. All changes follow established patterns (cost-capped LIMIT queries, concurrent IF NOT EXISTS index creation, null-guarding). No logic errors, security issues, or migration safety concerns were found. The test mock is correctly updated. No P1 or P0 findings. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant calculateSignUpRiskAssessment
participant loadRecentSignUpStats
participant DB as DB (replica)
Caller->>calculateSignUpRiskAssessment: context (email, ip, ...)
calculateSignUpRiskAssessment->>loadRecentSignUpStats: SignUpRiskRecentStatsRequest
par same-IP query
loadRecentSignUpStats->>DB: SELECT 1 WHERE signUpIp = ? LIMIT sameIpLimit
DB-->>loadRecentSignUpStats: sameIpRows
and same-normalized-email query (NEW)
loadRecentSignUpStats->>DB: SELECT 1 WHERE signUpEmailNormalized = ? LIMIT sameEmailLimit
DB-->>loadRecentSignUpStats: sameEmailRows
and similar-email-base query
loadRecentSignUpStats->>DB: SELECT 1 WHERE signUpEmailBase = ? LIMIT similarEmailLimit
DB-->>loadRecentSignUpStats: similarEmailRows
end
loadRecentSignUpStats-->>calculateSignUpRiskAssessment: { sameIpCount, sameEmailCount, similarEmailCount }
calculateSignUpRiskAssessment-->>Caller: SignUpRiskAssessment (scores + heuristicFacts)
Reviews (3): Last reviewed commit: "Add index for normalized sign-up email i..." | Re-trigger Greptile |
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.
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.
- Introduced a new index on the ProjectUser model for the signUpEmailNormalized field, enhancing query performance related to user sign-up activities. - Updated the migration SQL to create the corresponding index in the database, ensuring efficient data retrieval for analytics and fraud protection measures.
apps/backend/prisma/migrations/20260308000002_finalize_signup_fraud_protection/migration.sql
Outdated
Show resolved
Hide resolved
- Removed the existing index for signUpEmailNormalized from the finalize_signup_fraud_protection migration. - Created a new migration to add the signUpEmailNormalized index, ensuring better organization and clarity in the migration history. - This change enhances the database schema by maintaining a clear separation of index creation across migrations.
Summary by CodeRabbit
New Features
Performance