bot protection#1231
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA sign-up risk scoring system is introduced, adding risk evaluation capabilities for bot activity and free trial abuse. The implementation spans database schema changes, a new risk calculation module, integration into sign-up flows and rule evaluation, and corresponding UI components for rule testing and user management. Changes
Sequence DiagramsequenceDiagram
actor User
participant Client
participant SignUpAPI as Sign-up API
participant RiskModule as Risk Scoring
participant RuleEngine as Rule Engine
participant Database
participant Dashboard
User->>Client: Initiate sign-up (email, auth method)
Client->>SignUpAPI: POST /sign-up with auth details
SignUpAPI->>RiskModule: calculateSignUpRiskScores(email, authMethod, ipAddress)
RiskModule->>RiskModule: Evaluate bot & free-trial-abuse risk
RiskModule-->>SignUpAPI: Return riskScores {bot, freeTrialAbuse}
SignUpAPI->>RuleEngine: evaluateCelExpression(context + riskScores)
RuleEngine->>RuleEngine: Check conditions (e.g., riskScores.bot >= 80)
RuleEngine-->>SignUpAPI: Rule decision (allow/reject/restrict)
SignUpAPI->>Database: Store ProjectUser with risk_scores columns
Database-->>SignUpAPI: User created
SignUpAPI-->>Client: Sign-up result + context
User->>Dashboard: View user or test sign-up rules
Dashboard->>Dashboard: Display risk_scores in UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 introduces a bot-protection framework: two new Key issues found:
Confidence Score: 1/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant SignUpRoute as Sign-Up Route
participant Users as users.tsx
participant RiskScores as risk-scores.tsx
participant CELEval as cel-evaluator.ts
participant SignUpRules as sign-up-rules.ts
participant DB as Database (ProjectUser)
Client->>SignUpRoute: POST /auth/password/sign-up
SignUpRoute->>Users: createOrUpgradeAnonymousUserWithRules(...)
Users->>RiskScores: calculateSignUpRiskScores(tenancy, {email, authMethod, ipAddress, ...})
RiskScores-->>Users: SignUpRiskScores {bot, freeTrialAbuse}
Users->>CELEval: createSignUpRuleContext({email, authMethod, riskScores})
CELEval-->>Users: SignUpRuleContext
Users->>SignUpRules: evaluateSignUpRules(tenancy, context)
SignUpRules-->>Users: {shouldAllow, action}
alt rejected
Users-->>SignUpRoute: throw SIGN_UP_REJECTED
SignUpRoute-->>Client: 403 SIGN_UP_REJECTED
else allowed / restricted
Users->>DB: CREATE ProjectUser (signUpRiskScoreBot, signUpRiskScoreFreeTrialAbuse)
DB-->>Users: created user
Users-->>SignUpRoute: UsersCrud["Admin"]["Read"]
SignUpRoute-->>Client: 200 {user_id, ...}
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export type SignUpRuleOptions = { | ||
| authMethod: 'password' | 'otp' | 'oauth' | 'passkey', | ||
| oauthProvider?: string, | ||
| ipAddress: string | null, |
There was a problem hiding this comment.
Required ipAddress field breaks existing callers
ipAddress: string | null is added as a required (non-optional) property to SignUpRuleOptions. However, two existing callers that were not updated in this PR pass only { authMethod: '...' } without ipAddress, which would be a TypeScript compilation error:
apps/backend/src/app/api/latest/auth/password/sign-up/route.tsxcalls with{ authMethod: 'password' }apps/backend/src/app/api/latest/auth/otp/sign-in/verification-code-handler.tsxcalls with{ authMethod: 'otp' }(and even has a TODO comment acknowledging this)
Either make ipAddress optional (ipAddress?: string | null) or update all callers to explicitly pass null.
| ipAddress: string | null, | |
| ipAddress?: string | null, |
| function parseCondition(expr: string): ConditionNode | null { | ||
| const trimmed = expr.trim(); | ||
|
|
||
| // Match patterns like: field >= 42 | ||
| const greaterOrEqualNumberMatch = trimmed.match(/^([\w.]+)\s*>=\s*(-?\d+(?:\.\d+)?)$/); | ||
| if (greaterOrEqualNumberMatch) { | ||
| return { | ||
| type: 'condition', | ||
| id: generateNodeId(), | ||
| field: greaterOrEqualNumberMatch[1] as ConditionField, | ||
| operator: 'greater_or_equal', | ||
| value: Number(greaterOrEqualNumberMatch[2]), | ||
| }; |
There was a problem hiding this comment.
Unvalidated field names accepted as ConditionField
The new number-comparison regex patterns use [\w.]+ to capture the field name and then cast the result directly as ConditionField without validating that the matched name is actually a member of the union. For example, a handcrafted CEL expression like someArbitraryField.nested >= 50 would be parsed successfully and produce a ConditionNode with an invalid field value.
This is inconsistent with how the rest of the parser works — string-comparison patterns also now use [\w.]+ but only riskScores.bot and riskScores.freeTrialAbuse are valid dotted fields. An invalid field passed to conditionToCel would silently generate a CEL expression referencing a non-existent context property, which would evaluate as false and give users no indication that their rule is broken.
Consider adding a guard after parsing, e.g.:
const VALID_FIELDS = new Set<string>(['email', 'emailDomain', 'authMethod', 'oauthProvider', 'riskScores.bot', 'riskScores.freeTrialAbuse']);
if (!VALID_FIELDS.has(parsedField)) return null;| restrictedByAdminReason: restrictedByAdminReason, | ||
| restrictedByAdminPrivateDetails: restrictedByAdminPrivateDetails, |
There was a problem hiding this comment.
Unsafe property access on optional nested object
data.risk_scores?.sign_up.bot uses optional chaining on risk_scores but not on sign_up. If risk_scores is present but sign_up is somehow absent at runtime (schema coercion edge case, or a future schema change), this would throw TypeError: Cannot read properties of undefined (reading 'bot').
Use consistent optional chaining throughout:
| restrictedByAdminReason: restrictedByAdminReason, | |
| restrictedByAdminPrivateDetails: restrictedByAdminPrivateDetails, | |
| signUpRiskScoreBot: data.risk_scores?.sign_up?.bot, | |
| signUpRiskScoreFreeTrialAbuse: data.risk_scores?.sign_up?.free_trial_abuse, |
There was a problem hiding this comment.
Pull request overview
Adds “sign-up risk scores” (bot + free-trial-abuse) as server-only user fields, persists them in the database, exposes them via server APIs/SDK, and enables rule-building + CEL evaluation against these scores for bot protection workflows.
Changes:
- Persist sign-up risk scores on
ProjectUser(Prisma + migration + seed) and include them in server CRUD read/create/update schemas. - Compute/store risk scores during signup flows and make them available to sign-up rule evaluation (CEL context).
- Update Dashboard rule builder + CEL visual parser to support numeric comparisons on
riskScores.*, and add e2e coverage for persistence/visibility.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/template/src/lib/stack-app/users/index.ts | Adds riskScores to server user types (SDK surface). |
| packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts | Maps CRUD risk_scores into ServerUser.riskScores. |
| packages/stack-shared/src/interface/crud/users.ts | Extends server CRUD schemas to read/create/update risk_scores. |
| apps/e2e/tests/backend/endpoints/api/v1/risk-scores.test.ts | Adds end-to-end tests for risk score persistence, visibility, updates, and rule interaction. |
| apps/e2e/tests/backend/endpoints/api/v1/internal/sign-up-rules-test.test.ts | Verifies internal sign-up rules test endpoint supports risk score conditions. |
| apps/dashboard/src/lib/cel-visual-parser.ts | Adds numeric operators + parsing/serialization for riskScores.* conditions. |
| apps/dashboard/src/lib/cel-visual-parser.test.ts | Tests numeric risk score CEL parsing/serialization. |
| apps/dashboard/src/components/rule-builder/condition-builder.tsx | Adds risk score fields + numeric operators + numeric input UI. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsx | Displays user risk scores in the dashboard user details view. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsx | Adds risk score inputs to the sign-up rules “test rules” UI and displays returned context. |
| apps/backend/src/lib/users.tsx | Computes risk scores during signup/upgrade and injects them into rule context + persisted user data. |
| apps/backend/src/lib/risk-scores.tsx | Introduces risk score calculation module (currently stubbed). |
| apps/backend/src/lib/cel-evaluator.ts | Extends sign-up rule CEL context to include riskScores. |
| apps/backend/src/app/api/latest/users/crud.tsx | Persists risk scores in Prisma and returns them in server user CRUD responses. |
| apps/backend/src/app/api/latest/internal/sign-up-rules-test/route.tsx | Accepts risk scores in test input and returns them in evaluation context. |
| apps/backend/prisma/seed.ts | Ensures seeded users set required risk score columns. |
| apps/backend/prisma/schema.prisma | Adds signUpRiskScoreBot / signUpRiskScoreFreeTrialAbuse columns to ProjectUser. |
| apps/backend/prisma/migrations/20260308000000_add_signup_risk_scores/migration.sql | Migration to add new required SMALLINT risk score columns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| function isNumericField(field: ConditionField): boolean { | ||
| return field === 'riskScores.bot' || field === 'riskScores.freeTrialAbuse'; | ||
| } | ||
|
|
||
| // Get available operators for a field | ||
| function getOperatorsForField(field: ConditionField): ConditionOperator[] { | ||
| if (isNumericField(field)) { | ||
| return ['equals', 'not_equals', 'greater_than', 'greater_or_equal', 'less_than', 'less_or_equal']; | ||
| } |
There was a problem hiding this comment.
isNumericField(...) is duplicated here and in cel-visual-parser.ts. Since both places must stay in sync for correct operator/value handling, consider exporting a shared helper (or a shared Set of numeric fields) from the parser module to avoid divergence over time.
| export type SignUpRuleOptions = { | ||
| authMethod: 'password' | 'otp' | 'oauth' | 'passkey', | ||
| oauthProvider?: string, | ||
| ipAddress: string | null, |
There was a problem hiding this comment.
SignUpRuleOptions now requires ipAddress, but several call sites still pass only { authMethod, oauthProvider } (e.g. password sign-up, OTP verification-code handler). This makes the change a breaking compile-time error. Either make ipAddress optional (defaulting to null) or update all callers to pass a value (or null when unavailable).
| ipAddress: string | null, | |
| ipAddress?: string | null, |
| // TODO | ||
| if (context.primaryEmail === "test@example.com") { | ||
| return { | ||
| bot: 100, | ||
| freeTrialAbuse: 100, | ||
| }; | ||
| } else { | ||
| return { | ||
| bot: 0, | ||
| freeTrialAbuse: 0, | ||
| }; | ||
| } |
There was a problem hiding this comment.
calculateSignUpRiskScores is currently a hard-coded stub that flags only test@example.com as high risk. Shipping this as-is will produce incorrect risk scores in production and makes sign-up rules unreliable. Please either implement the real scoring logic, or gate this stub to test-only (e.g., behind an env/config flag) and default to neutral scores otherwise.
| // TODO | |
| if (context.primaryEmail === "test@example.com") { | |
| return { | |
| bot: 100, | |
| freeTrialAbuse: 100, | |
| }; | |
| } else { | |
| return { | |
| bot: 0, | |
| freeTrialAbuse: 0, | |
| }; | |
| } | |
| // TODO: Implement real scoring logic. For now, only apply the stub behavior in tests | |
| if (process.env.NODE_ENV === "test" && context.primaryEmail === "test@example.com") { | |
| return { | |
| bot: 100, | |
| freeTrialAbuse: 100, | |
| }; | |
| } | |
| // Default to neutral risk scores in non-test environments | |
| return { | |
| bot: 0, | |
| freeTrialAbuse: 0, | |
| }; |
| return `${field} > ${useNumericValue ? valueAsNumber : 0}`; | ||
| } | ||
| case 'greater_or_equal': { | ||
| return `${field} >= ${useNumericValue ? valueAsNumber : 0}`; | ||
| } | ||
| case 'less_than': { | ||
| return `${field} < ${useNumericValue ? valueAsNumber : 0}`; | ||
| } | ||
| case 'less_or_equal': { | ||
| return `${field} <= ${useNumericValue ? valueAsNumber : 0}`; |
There was a problem hiding this comment.
For numeric operators (>, >=, <, <=), conditionToCel falls back to 0 when the field/value isn't treated as numeric. This can silently change rule semantics if a parsed/edited condition ends up with a non-numeric field or non-numeric value (e.g., it will serialize to field > 0). Consider returning null/throwing for invalid numeric comparisons, or ensuring these operators are only ever used with numeric fields at the type/runtime level.
| return `${field} > ${useNumericValue ? valueAsNumber : 0}`; | |
| } | |
| case 'greater_or_equal': { | |
| return `${field} >= ${useNumericValue ? valueAsNumber : 0}`; | |
| } | |
| case 'less_than': { | |
| return `${field} < ${useNumericValue ? valueAsNumber : 0}`; | |
| } | |
| case 'less_or_equal': { | |
| return `${field} <= ${useNumericValue ? valueAsNumber : 0}`; | |
| if (!useNumericValue) { | |
| throw new Error( | |
| `Invalid non-numeric comparison: field "${field}" with operator "greater_than" and value "${String( | |
| value, | |
| )}"`, | |
| ); | |
| } | |
| return `${field} > ${valueAsNumber}`; | |
| } | |
| case 'greater_or_equal': { | |
| if (!useNumericValue) { | |
| throw new Error( | |
| `Invalid non-numeric comparison: field "${field}" with operator "greater_or_equal" and value "${String( | |
| value, | |
| )}"`, | |
| ); | |
| } | |
| return `${field} >= ${valueAsNumber}`; | |
| } | |
| case 'less_than': { | |
| if (!useNumericValue) { | |
| throw new Error( | |
| `Invalid non-numeric comparison: field "${field}" with operator "less_than" and value "${String( | |
| value, | |
| )}"`, | |
| ); | |
| } | |
| return `${field} < ${valueAsNumber}`; | |
| } | |
| case 'less_or_equal': { | |
| if (!useNumericValue) { | |
| throw new Error( | |
| `Invalid non-numeric comparison: field "${field}" with operator "less_or_equal" and value "${String( | |
| value, | |
| )}"`, | |
| ); | |
| } | |
| return `${field} <= ${valueAsNumber}`; |
| // Match patterns like: field >= 42 | ||
| const greaterOrEqualNumberMatch = trimmed.match(/^([\w.]+)\s*>=\s*(-?\d+(?:\.\d+)?)$/); | ||
| if (greaterOrEqualNumberMatch) { | ||
| return { | ||
| type: 'condition', | ||
| id: generateNodeId(), | ||
| field: greaterOrEqualNumberMatch[1] as ConditionField, | ||
| operator: 'greater_or_equal', | ||
| value: Number(greaterOrEqualNumberMatch[2]), | ||
| }; | ||
| } |
There was a problem hiding this comment.
parseCondition casts any ([\w.]+) match to ConditionField without validating that the field is actually supported. This can yield a ConditionNode with an invalid field value, and also accepts numeric comparisons for non-numeric fields; on round-trip, conditionToCel may serialize these as ... > 0, changing the original expression. Recommend validating the matched field against the allowed set and rejecting numeric operators unless isNumericField(field) is true (otherwise return null so the UI can fall back to raw CEL).
| risk_scores: fieldSchema.yupObject({ | ||
| sign_up: fieldSchema.yupObject({ | ||
| bot: fieldSchema.yupNumber().integer().min(0).max(100).defined(), | ||
| free_trial_abuse: fieldSchema.yupNumber().integer().min(0).max(100).defined(), | ||
| }).defined(), | ||
| }).optional(), |
There was a problem hiding this comment.
usersCrudServerCreateSchema is built by concatenating usersCrudServerUpdateSchema, which already defines risk_scores, but it then re-defines risk_scores again in the concat object. This duplication is easy to accidentally let drift and makes the schema harder to maintain; consider removing the duplicate definition and relying on the field inherited from the update schema (or explicitly overriding with a comment explaining why).
| risk_scores: fieldSchema.yupObject({ | |
| sign_up: fieldSchema.yupObject({ | |
| bot: fieldSchema.yupNumber().integer().min(0).max(100).defined(), | |
| free_trial_abuse: fieldSchema.yupNumber().integer().min(0).max(100).defined(), | |
| }).defined(), | |
| }).optional(), |
| /** Server-only risk scores used during sign-up risk evaluation. */ | ||
| readonly riskScores: { | ||
| readonly signUp: { | ||
| readonly bot: number, | ||
| readonly freeTrialAbuse: number, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
riskScores is now exposed on ServerBaseUser, but ServerUserUpdateOptions (and the corresponding update(...) plumbing) doesn’t provide any way to set/update these scores even though the server API supports risk_scores updates. If risk scores are intended to be server-adjustable (as the API allows), consider adding an update option for them (or intentionally disallowing updates and removing/locking down the server PATCH field).
| /** Server-only risk scores used during sign-up risk evaluation. */ | |
| readonly riskScores: { | |
| readonly signUp: { | |
| readonly bot: number, | |
| readonly freeTrialAbuse: number, | |
| }, | |
| }, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/backend/prisma/migrations/20260308000000_add_signup_risk_scores/migration.sql (1)
2-5: Back the 0–100 invariant with database checks.
SMALLINTstill allows values outside the documented range forsignUpRiskScoreBotandsignUpRiskScoreFreeTrialAbuse, while the new read schema now assumes both are integers in[0, 100]. An out-of-band write would let invalid rows persist and then break later reads. Please addCHECKconstraints for both columns in aNOT VALIDmigration and validate them in a separate follow-up migration.As per coding guidelines, "When adding CHECK constraints, use
NOT VALIDin one migration, thenVALIDATE CONSTRAINTin a separate migration file."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/prisma/migrations/20260308000000_add_signup_risk_scores/migration.sql` around lines 2 - 5, Add CHECK constraints on ProjectUser.signUpRiskScoreBot and ProjectUser.signUpRiskScoreFreeTrialAbuse to enforce values are between 0 and 100 using CONSTRAINT ... CHECK (...) NOT VALID in this migration (referencing the column names signUpRiskScoreBot and signUpRiskScoreFreeTrialAbuse and table ProjectUser), and create a separate follow-up migration that runs VALIDATE CONSTRAINT for each new constraint; ensure you add descriptive constraint names (e.g. chk_projectuser_signupriskscorebot_range and chk_projectuser_signupriskscorefreetrialabuse_range) in the NOT VALID step so the subsequent migration can target them for validation.apps/backend/src/lib/users.tsx (1)
129-133: Type cast is documented but consider a safer alternative.The comment explains why the cast is needed (create-only fields like
risk_scoresare stripped byKeyIntersect). While the cast is justified, an alternative would be to define a more precise type for the create path that includes these fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/lib/users.tsx` around lines 129 - 133, The current cast of createOrUpdate to UsersCrud["Admin"]["Create"] when calling usersCrudHandlers.adminCreate is unsafe; instead define an explicit create payload typed as UsersCrud["Admin"]["Create"] (or a small builder/helper that maps createOrUpdate into that type) so you only include/transform the create-only fields (e.g., risk_scores) required for creation; update the call to pass that strongly-typed createPayload to usersCrudHandlers.adminCreate, referencing the existing createOrUpdate variable, the usersCrudHandlers.adminCreate call site, and the UsersCrud["Admin"]["Create"] type to locate where to change.apps/backend/src/lib/risk-scores.tsx (1)
16-29: Placeholder implementation with unused parameter.The
tenancyparameter is declared but not used, and the implementation contains a TODO marker with test-only logic. This is acceptable for initial scaffolding, but the hardcodedtest@example.comcheck should be replaced with actual risk scoring logic before production use.Would you like me to open an issue to track implementing the actual risk scoring logic, or is this intentionally deferred to a follow-up PR?
🤖 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 16 - 29, calculateSignUpRiskScores currently contains a test-only hardcoded email check and never uses the tenancy parameter; replace the placeholder logic so the function no longer special-cases "test@example.com" and instead either implements real scoring using the provided tenancy and context (e.g., consult tenancy.id or tenancy.features and context.primaryEmail, ip, device info) or, if real logic is deferred, return a neutral/default SignUpRiskScores and explicitly reference tenancy (e.g., include tenancy.id in a debug/log statement or use it in a simple conditional) to avoid an unused parameter; also remove the TODO marker or convert it into a clear follow-up comment and open a tracked issue to implement proper risk logic later.
🤖 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/users.tsx`:
- Around line 13-17: SignUpRuleOptions now requires ipAddress but three callers
pass objects without it; either make ipAddress optional on SignUpRuleOptions or
(preferred) update each caller to supply a value (real IP from the request or
null) when constructing the options. Locate the type SignUpRuleOptions and add
ipAddress: string | null where needed, then update the password sign-up handler
(password sign-up), the OTP verification-code handler (otp sign-in
verification), and the OAuth code path that forwards params.signUpRuleOptions to
include an ipAddress field (use request IP extraction or null if unavailable).
Ensure all call sites create a complete SignUpRuleOptions object to satisfy the
type.
In `@apps/e2e/tests/backend/endpoints/api/v1/internal/sign-up-rules-test.test.ts`:
- Around line 85-128: Two earlier tests still call
niceBackendFetch("/api/v1/internal/sign-up-rules-test") without the now-required
risk_scores body and will 400; update those POST bodies to include risk_scores:
{ bot: 0, free_trial_abuse: 0 } so the endpoint receives the required object
before rule evaluation, mirroring the new test that uses risk_scores with
nonzero values.
In `@apps/e2e/tests/backend/endpoints/api/v1/risk-scores.test.ts`:
- Around line 526-568: The two tests "should default risk scores to 0 for
server-created users" and "should allow setting risk scores when creating users
via server API" are missing a dedicated project context and must call
Project.createAndSwitch() before performing requests; update each test to create
and switch to a fresh project (via Project.createAndSwitch()) at the start of
the test so the subsequent niceBackendFetch calls operate in an isolated project
context and avoid cross-test leakage.
---
Nitpick comments:
In
`@apps/backend/prisma/migrations/20260308000000_add_signup_risk_scores/migration.sql`:
- Around line 2-5: Add CHECK constraints on ProjectUser.signUpRiskScoreBot and
ProjectUser.signUpRiskScoreFreeTrialAbuse to enforce values are between 0 and
100 using CONSTRAINT ... CHECK (...) NOT VALID in this migration (referencing
the column names signUpRiskScoreBot and signUpRiskScoreFreeTrialAbuse and table
ProjectUser), and create a separate follow-up migration that runs VALIDATE
CONSTRAINT for each new constraint; ensure you add descriptive constraint names
(e.g. chk_projectuser_signupriskscorebot_range and
chk_projectuser_signupriskscorefreetrialabuse_range) in the NOT VALID step so
the subsequent migration can target them for validation.
In `@apps/backend/src/lib/risk-scores.tsx`:
- Around line 16-29: calculateSignUpRiskScores currently contains a test-only
hardcoded email check and never uses the tenancy parameter; replace the
placeholder logic so the function no longer special-cases "test@example.com" and
instead either implements real scoring using the provided tenancy and context
(e.g., consult tenancy.id or tenancy.features and context.primaryEmail, ip,
device info) or, if real logic is deferred, return a neutral/default
SignUpRiskScores and explicitly reference tenancy (e.g., include tenancy.id in a
debug/log statement or use it in a simple conditional) to avoid an unused
parameter; also remove the TODO marker or convert it into a clear follow-up
comment and open a tracked issue to implement proper risk logic later.
In `@apps/backend/src/lib/users.tsx`:
- Around line 129-133: The current cast of createOrUpdate to
UsersCrud["Admin"]["Create"] when calling usersCrudHandlers.adminCreate is
unsafe; instead define an explicit create payload typed as
UsersCrud["Admin"]["Create"] (or a small builder/helper that maps createOrUpdate
into that type) so you only include/transform the create-only fields (e.g.,
risk_scores) required for creation; update the call to pass that strongly-typed
createPayload to usersCrudHandlers.adminCreate, referencing the existing
createOrUpdate variable, the usersCrudHandlers.adminCreate call site, and the
UsersCrud["Admin"]["Create"] type to locate where to change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a3ed090-6fd1-4398-98c3-1e3c0481ddc4
📒 Files selected for processing (18)
apps/backend/prisma/migrations/20260308000000_add_signup_risk_scores/migration.sqlapps/backend/prisma/schema.prismaapps/backend/prisma/seed.tsapps/backend/src/app/api/latest/internal/sign-up-rules-test/route.tsxapps/backend/src/app/api/latest/users/crud.tsxapps/backend/src/lib/cel-evaluator.tsapps/backend/src/lib/risk-scores.tsxapps/backend/src/lib/users.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sign-up-rules/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/users/[userId]/page-client.tsxapps/dashboard/src/components/rule-builder/condition-builder.tsxapps/dashboard/src/lib/cel-visual-parser.test.tsapps/dashboard/src/lib/cel-visual-parser.tsapps/e2e/tests/backend/endpoints/api/v1/internal/sign-up-rules-test.test.tsapps/e2e/tests/backend/endpoints/api/v1/risk-scores.test.tspackages/stack-shared/src/interface/crud/users.tspackages/template/src/lib/stack-app/apps/implementations/server-app-impl.tspackages/template/src/lib/stack-app/users/index.ts
| export type SignUpRuleOptions = { | ||
| authMethod: 'password' | 'otp' | 'oauth' | 'passkey', | ||
| oauthProvider?: string, | ||
| ipAddress: string | null, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all callers of createOrUpgradeAnonymousUserWithRules and check if they pass ipAddress
rg -n -A 10 'createOrUpgradeAnonymousUserWithRules\(' --type ts | head -100Repository: stack-auth/stack-auth
Length of output: 4091
🏁 Script executed:
# Get the complete call at password sign-up route
sed -n '57,75p' apps/backend/src/app/api/latest/auth/password/sign-up/route.tsxRepository: stack-auth/stack-auth
Length of output: 593
🏁 Script executed:
# Get the complete call at OTP handler
sed -n '110,130p' apps/backend/src/app/api/latest/auth/otp/sign-in/verification-code-handler.tsxRepository: stack-auth/stack-auth
Length of output: 635
🏁 Script executed:
# Get the complete call at OAuth
sed -n '207,225p' apps/backend/src/lib/oauth.tsxRepository: stack-auth/stack-auth
Length of output: 623
🏁 Script executed:
# View the SignUpRuleOptions type definition in users.tsx
sed -n '13,17p' apps/backend/src/lib/users.tsxRepository: stack-auth/stack-auth
Length of output: 211
🏁 Script executed:
# Check if there are any other callers we missed
rg -c 'createOrUpgradeAnonymousUserWithRules\(' --type tsRepository: stack-auth/stack-auth
Length of output: 277
Critical: ipAddress is now required in SignUpRuleOptions but three callers don't provide it.
The SignUpRuleOptions type defines ipAddress: string | null as a required field, but the following callers pass incomplete objects:
apps/backend/src/app/api/latest/auth/password/sign-up/route.tsx:67— passes only{ authMethod: 'password' }apps/backend/src/app/api/latest/auth/otp/sign-in/verification-code-handler.tsx:120— passes only{ authMethod: 'otp' }apps/backend/src/lib/oauth.tsx:217— passesparams.signUpRuleOptionswhich lacksipAddress
This will cause TypeScript compilation errors at all three sites.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/src/lib/users.tsx` around lines 13 - 17, SignUpRuleOptions now
requires ipAddress but three callers pass objects without it; either make
ipAddress optional on SignUpRuleOptions or (preferred) update each caller to
supply a value (real IP from the request or null) when constructing the options.
Locate the type SignUpRuleOptions and add ipAddress: string | null where needed,
then update the password sign-up handler (password sign-up), the OTP
verification-code handler (otp sign-in verification), and the OAuth code path
that forwards params.signUpRuleOptions to include an ipAddress field (use
request IP extraction or null if unavailable). Ensure all call sites create a
complete SignUpRuleOptions object to satisfy the type.
| it("evaluates risk score conditions", async ({ expect }) => { | ||
| await Project.createAndSwitch(); | ||
| await Project.updateConfig({ | ||
| "auth.signUpRules.block-high-bot-score": { | ||
| enabled: true, | ||
| displayName: "Block high bot score", | ||
| priority: 1, | ||
| condition: "riskScores.bot >= 80", | ||
| action: { | ||
| type: "reject", | ||
| message: "High bot risk", | ||
| }, | ||
| }, | ||
| "auth.signUpRulesDefaultAction": "allow", | ||
| }); | ||
|
|
||
| const response = await niceBackendFetch("/api/v1/internal/sign-up-rules-test", { | ||
| method: "POST", | ||
| accessType: "admin", | ||
| body: { | ||
| email: "risk@example.com", | ||
| auth_method: "password", | ||
| risk_scores: { | ||
| bot: 90, | ||
| free_trial_abuse: 10, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.body).toMatchObject({ | ||
| context: { | ||
| risk_scores: { | ||
| bot: 90, | ||
| free_trial_abuse: 10, | ||
| }, | ||
| }, | ||
| outcome: { | ||
| should_allow: false, | ||
| decision: "reject", | ||
| decision_rule_id: "block-high-bot-score", | ||
| }, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Update the older request fixtures for the now-required risk_scores body.
This new case is correct, but the two existing niceBackendFetch("/api/v1/internal/sign-up-rules-test") calls earlier in this file still omit risk_scores. The endpoint contract now requires that object, so those tests will 400 before the rule logic runs. Add a zeroed { bot: 0, free_trial_abuse: 0 } payload to the older request bodies as well.
🧪 Minimal follow-up
body: {
email: "user@example.com",
auth_method: "password",
+ risk_scores: {
+ bot: 0,
+ free_trial_abuse: 0,
+ },
},Apply the same change to the other existing POST body in this file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/e2e/tests/backend/endpoints/api/v1/internal/sign-up-rules-test.test.ts`
around lines 85 - 128, Two earlier tests still call
niceBackendFetch("/api/v1/internal/sign-up-rules-test") without the now-required
risk_scores body and will 400; update those POST bodies to include risk_scores:
{ bot: 0, free_trial_abuse: 0 } so the endpoint receives the required object
before rule evaluation, mirroring the new test that uses risk_scores with
nonzero values.
| describe("server-created users", () => { | ||
| it("should default risk scores to 0 for server-created users", async ({ expect }) => { | ||
| const createResponse = await niceBackendFetch("/api/v1/users", { | ||
| method: "POST", | ||
| accessType: "server", | ||
| body: { | ||
| primary_email: `server-created-${generateSecureRandomString(8)}@example.com`, | ||
| }, | ||
| }); | ||
|
|
||
| expect(createResponse.status).toBe(201); | ||
| expect(createResponse.body.risk_scores).toEqual({ | ||
| sign_up: { | ||
| bot: 0, | ||
| free_trial_abuse: 0, | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| it("should allow setting risk scores when creating users via server API", async ({ expect }) => { | ||
| const createResponse = await niceBackendFetch("/api/v1/users", { | ||
| method: "POST", | ||
| accessType: "server", | ||
| body: { | ||
| primary_email: `risky-${generateSecureRandomString(8)}@example.com`, | ||
| risk_scores: { | ||
| sign_up: { | ||
| bot: 55, | ||
| free_trial_abuse: 42, | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| expect(createResponse.status).toBe(201); | ||
| expect(createResponse.body.risk_scores).toEqual({ | ||
| sign_up: { | ||
| bot: 55, | ||
| free_trial_abuse: 42, | ||
| }, | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing Project.createAndSwitch() in server-created users tests.
Unlike all other test blocks in this file, these two tests don't establish their own project context. They'll use whatever project context remains from previously executed tests, which could lead to flaky behavior or unexpected test isolation issues.
🧪 Proposed fix to add project context
describe("server-created users", () => {
it("should default risk scores to 0 for server-created users", async ({ expect }) => {
+ await Project.createAndSwitch({
+ config: { credential_enabled: true },
+ });
+
const createResponse = await niceBackendFetch("/api/v1/users", {
method: "POST",
accessType: "server", it("should allow setting risk scores when creating users via server API", async ({ expect }) => {
+ await Project.createAndSwitch({
+ config: { credential_enabled: true },
+ });
+
const createResponse = await niceBackendFetch("/api/v1/users", {
method: "POST",
accessType: "server",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/e2e/tests/backend/endpoints/api/v1/risk-scores.test.ts` around lines 526
- 568, The two tests "should default risk scores to 0 for server-created users"
and "should allow setting risk scores when creating users via server API" are
missing a dedicated project context and must call Project.createAndSwitch()
before performing requests; update each test to create and switch to a fresh
project (via Project.createAndSwitch()) at the start of the test so the
subsequent niceBackendFetch calls operate in an isolated project context and
avoid cross-test leakage.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Suggestions:
- Missing required
ipAddressproperty insignUpRuleOptionsobject for OAuth callback routes causes TypeScript build failure.
- TypeScript error:
SignUpRuleOptionstype requiresipAddressproperty but multiple auth handlers don't provide it
- Missing required
ipAddressproperty insignUpRuleOptionsobjects causes TypeScript compilation errors in multiple sign-up routes
Summary by CodeRabbit
New Features
Tests