feat: email-based identifier resolution (#27) - #106
Conversation
- Add resolveByEmail() function to resolve users by company or personal email - Update resolveSlug() to detect email format and delegate to resolveByEmail - Add resolvedVia field to SlugResolution type for tracking resolution method Part of #27
reviewer-props-to
left a comment
reviewer-props-to
left a comment
There was a problem hiding this comment.
Review: Request Changes
🔴 Security Concern: User Enumeration
This feature allows unauthenticated users to check if an email exists in the system by visiting props.to/email@example.com. This is a common privacy/security vulnerability that can be exploited for:
- Phishing reconnaissance
- Privacy violations
- Targeted attacks
Recommendation: Before shipping email-based resolution publicly, consider:
- Rate limiting (critical) - Already marked as TODO, but should be in place BEFORE merging this
- Consistent response - Return same response time/behavior regardless of whether email exists
- Consider opt-in - Users could choose whether their email is publicly resolvable
🟡 Accidental File Inclusion
The diff includes .claude/memory/2026-02-15.md which appears to be an internal agent memory file. This should be removed from the PR.
✅ Code Quality
The implementation itself is clean:
- Proper email normalization (lowercase + trim)
- Good error handling with handleError/handleSuccess
- Nice
resolvedViatracking field - Follows existing patterns
Verdict
Cannot approve until:
- Rate limiting is implemented (or at minimum, the email resolution is behind a feature flag disabled by default)
- The accidental memory file is removed from the commit
The code quality is good, but the security implications need addressing first.
|
🚀 App preview deployment successfull at https://app.pr-106.props.build |
Security fix for user enumeration vulnerability: - Add in-memory rate limiter utility (10 req/min per IP) - Apply rate limiting to email-based slug resolution in public pages - Remove accidental memory file from repo - Add .claude/memory/ to .gitignore Addresses Che's review feedback on PR #106.
|
🚀 Web preview deployment successfull at https://web.pr-106.props.build |
reviewer-props-to
left a comment
reviewer-props-to
left a comment
There was a problem hiding this comment.
✅ Approved
Previous concerns addressed:
-
Rate limiting - Now implemented (10 req/min per IP). Good implementation:
- Consistent 404 response when rate limited (no enumeration leak)
- Proper IP extraction for proxy setups
- Memory cleanup to prevent leaks
- Clear comments about Redis for scale
-
Memory file - Properly excluded via .gitignore
Security assessment: The email lookup is now reasonably protected against enumeration attacks. The in-memory rate limiter is fine for initial deployment; you can upgrade to Redis (e.g., @upstash/ratelimit) when scaling.
LGTM - ship it 🚀
|
🚀 Auth preview deployment successfull at https://auth.pr-106.props.build |
|
🚀 Auth preview deployment successfull at https://auth.pr-106.props.build |
|
🚀 App preview deployment successfull at https://app.pr-106.props.build |
|
❌ E2E Tests Failed Tested against: |
|
🚀 Web preview deployment successfull at https://web.pr-106.props.build |
|
🚀 Web preview deployment successfull at https://web.pr-106.props.build |
|
🚀 App preview deployment successfull at https://app.pr-106.props.build |
|
❌ E2E Tests Failed Tested against: |
|
🚀 Auth preview deployment successfull at https://auth.pr-106.props.build |
2 participants
Summary
Adds support for resolving users by email address in public feedback URLs.
Routes supported:
props.to/john@acme.com→ resolves to user's profile pageChanges
packages/data/repos/slug.ts:resolveByEmail()function to lookup users by company or personal emailresolveSlug()to detect email format and delegate appropriatelyresolvedViafield to track resolution method (slug vs email)Testing
pnpm healthpassesRemaining for #27
Closes part of #27