Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe home page includes a truthy stored signup anonymous ID in registration requests. The API stores string values with lengths from 1 to 64 characters in onboarding data. On user activation, a new signal receiver tracks Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to Signup conversion attribution can be inaccurate when a whitespace-only identifier is already stored. This is a narrow, fixable case; the change is mergeable with owner awareness. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change does not visibly alter account permissions, and tracking exceptions are contained. The identifier remains client-controlled and is now linked to the account. Downstream telemetry isolation and activation recovery behavior are not fully verified. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
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 |
github-actions
Bot
added
front-end
github-actions
Bot
added
feature
|
@themis-blindfold review |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8674 +/- ##
==========================================
- Coverage 98.84% 98.84% -0.01%
==========================================
Files 1659 1664 +5
Lines 68541 68414 -127
==========================================
- Hits 67751 67624 -127
Misses 790 790 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
30731320-807e-4df6-ba6a-d200b25da4e5
📒 Files selected for processing (7)
api/custom_auth/signals.pyapi/custom_auth/views.pyapi/tests/unit/custom_auth/test_unit_custom_auth_signals.pyapi/tests/unit/custom_auth/test_unit_custom_auth_views.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.mdfrontend/common/types/requests.tsfrontend/web/components/pages/home-page/HomePage.tsx
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
![themis-blindfold[bot]](https://avatars.githubusercontent.com/in/4278417?s=60&v=4){kind=small}
⚖️ Themis review: 🟠 Fix before mergeObserved: the new conversion path rejects an identity format that the existing exposure hook deliberately generates when
🟠 Majors
📝 Walkthrough
🧪 How to verify
Product take: This is a solid experiment-measurement improvement, but the identifier contract needs alignment before conversion reporting can be complete. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. A conversion only counts when both ends agree on its identity · reviewed at 418d373 |
github-actions
Bot
added
docs
|
@themis-blindfold review |
⚖️ Themis review: ✅ Ship itTL;DR: This connects the experiment exposure identity to the email-activation conversion without exposing it through the current-user response. The completed CI suite passed, including API unit tests, linting, documentation checks, and static analysis.
📝 Walkthrough
🧪 How to verify
Product take: A solid measurement improvement for the signup experiment: it attributes completed email verification to the same anonymous identity that saw the variant. Its product scope is intentionally narrow and low-risk. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. The conversion now survives the long walk from form submit to inbox click · reviewed at c45aa49 |
Docker builds report
|
github-actions
Bot
added
feature
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #21215 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
🗂️ Previous results✅ private-cloud · depot-ubuntu-latest-16 — run #21215 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
✅ oss · depot-ubuntu-latest-arm-16 — run #21215 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
✅ oss · depot-ubuntu-latest-16 — run #21215 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
e4ab75be-d648-400e-b261-599325277c76
📒 Files selected for processing (4)
api/custom_auth/signals.pyapi/custom_auth/views.pyapi/tests/unit/custom_auth/test_unit_custom_auth_signals.pyapi/tests/unit/custom_auth/test_unit_custom_auth_views.py
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
| if ( | ||
| not isinstance(signup_anonymous_id, str) | ||
| or not 0 < len(signup_anonymous_id) <= 64 | ||
| ): |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject whitespace-only signup identifiers.
If signup_anonymous_id is " ", this check stores it. Activation then uses that effectively blank value as the conversion targeting key. Reject values whose stripped form is empty, and add a matching registration case. Preserve the original non-blank identifier so the targeting key still matches the frontend exposure.
Based on learnings, whitespace-only strings must not pass identifier validation as non-empty values.
Source: Learnings
Visual Regression19 screenshots compared. See report for details. |
This branch was successfully deployed
1 participant
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to #8595
Adds a conversion event for the
signup_corporate_onlyexperiment, reconciling the frontend exposure (transient anonymous identity) with the backend activation flow. No migration.signup_anonymous_id(the id used for the exposure) in the register payload when the visitor entered the experiment.FFAdminUser.onboarding_datawhen it is a valid UUID; it is not exposed by/me.user_activated, the backend trackssignup_activatedvia OpenFeature with the anonymous id as targeting key. Tracking failures are logged (signup.conversion.tracking_failed) and never break activation.Storing the id server-side means every activation path (resend from any device, expired link) converts without touching the activation email.
Deploy the backend before or with the frontend merge.
How did you test this code?
signup_activatedis recorded for the anonymous identity.