Skip to content

fix(users): stop exposing email addresses from user search - #385

Open
MOHITKOURAV01 wants to merge 1 commit into
TravellersMeet:mainfrom
MOHITKOURAV01:security/380-user-search-email-exposure
Open

fix(users): stop exposing email addresses from user search#385
MOHITKOURAV01 wants to merge 1 commit into
TravellersMeet:mainfrom
MOHITKOURAV01:security/380-user-search-email-exposure

Conversation

@MOHITKOURAV01

Copy link
Copy Markdown
Contributor

Closes #380

The problem

GET /api/users selected email into every result and matched the ?search= term against the email column:

whereClause.OR = [
  { name:     { contains: search, mode: "insensitive" } },
  { email:    { contains: search, mode: "insensitive" } },
  { location: { contains: search, mode: "insensitive" } },
];

const users = await prisma.user.findMany({
  where: whereClause,
  select: { id: true, name: true, email: true, ... },
});

Two separate problems in one query:

  1. Any authenticated caller could walk the cursor with limit=100 and dump the whole user table's email addresses. No screen in the product shows another user's email — /api/conversations, /api/connections and /api/matches all select id/name/image/bio/location.
  2. email contains <term> is an enumeration oracle. ?search=@gmail.com harvests by domain; ?search=someone@example.com answers whether that address has an account.

What this changes

The leak. email is out of the projection and out of the search predicate. Search matches name and location — what you'd actually use to find a travel companion.

Blocked users. Search never consulted src/lib/blocking.ts. Blocking is symmetric, and /api/conversations already filters with getBlockedUserIds(); user search was the remaining place a blocked user could still surface. Same helper, one query, applied to the exclusion list.

A pagination bug found while writing the tests. The cursor filter and the search filter were both spread into the where object as OR, so the search clause overwrote the cursor clause:

const whereClause: any = {
  ...(cursorWhere ?? {}),   // { OR: [...cursor...] }
};
if (search) {
  whereClause.OR = [...];  // silently replaces it
}

?search=goa&cursor=<next> therefore returned page one forever. Both groups now sit under AND.

Typing. whereClause was any, which is what let the email predicate through unnoticed. It's Prisma.UserWhereInput now.

Input handling. search is trimmed, a whitespace-only term is ignored rather than running a contains "" scan, and terms over 100 characters get a 400. hasMore is returned alongside nextCursor.

Tests

The existing file asserted the old behaviour — that email was selected and was searched — so those assertions are inverted. Its buildTimestampCursorWhere mock also took the cursor as its first argument while the real signature is (field, cursor), so it was handed the string "createdAt", treated it as a cursor and returned a filter with Date { NaN } in it on every call. That's why 3 of the 8 tests in this file fail on main. Fixed, so the cursor assertions mean something.

8 tests → 19. Everything the old file covered is still covered.

Verification

Note on the base branch

main does not currently typecheck — five auth routes have a duplicate rateLimit declaration (#366, fixed by #379). Nothing in this PR touches those files, so it should rebase cleanly once #379 lands.

GET /api/users selected `email` into every result and matched the
`?search=` term against the email column. That gave any authenticated
caller a full address book (walk the cursor with limit=100) and a working
enumeration oracle — `?search=@gmail.com` harvests by domain,
`?search=someone@example.com` answers whether that address has an account.

No screen in the product shows another user's email. The projection now
matches the one /api/conversations and /api/connections already use, and
search matches name and location only.

While in here:

- Blocked users were still returned. Blocking is symmetric (src/lib/blocking.ts)
  and /api/conversations already filters with getBlockedUserIds(); user search
  was the remaining place a blocked user could surface. Same filter applied.
- The cursor filter and the search filter were both spread into the where
  object as `OR`, so the search clause overwrote the cursor clause and paging
  through a search returned page one forever. Both now sit under `AND`.
- `whereClause` was typed `any`, which is what let the email predicate through
  unnoticed. It is `Prisma.UserWhereInput` now.
- `search` is trimmed, a whitespace-only term is ignored, and terms over 100
  characters are rejected with 400 instead of running a wide scan.
- `hasMore` is returned alongside `nextCursor`.

The test file asserted the old behaviour (that email was selected and
searched), so those assertions are inverted. Its `buildTimestampCursorWhere`
mock also took the cursor as its first argument while the real signature is
(field, cursor), so it was being handed the string "createdAt" and returned a
bogus filter on every call; fixed so the cursor assertions mean something.

Closes TravellersMeet#380
@github-actions

Copy link
Copy Markdown

Thanks for opening this PR, @MOHITKOURAV01! 👋

Our maintainers will review it shortly. Estimate Time is 5-8 hrs .Meanwhile please:

  • Ensure CI is green (lint, typecheck, build).
  • Hit the star ⭐ button to show your support!
  • Confirm the PR template checklist is complete.
  • Add screenshots for UI changes.

If anything changes, feel free to push updates—this thread will stay open.

@MOHITKOURAV01

Copy link
Copy Markdown
Contributor Author

On the red build-and-lint

The failure is inherited from main, not introduced here. The typecheck step reports 23 errors, all in files this PR does not touch:

Same 23 errors on main at 39e71eb, and the same failure on #376, #377 and #367. npx tsc --noEmit on this branch adds nothing beyond that set.

Test suite on this branch: no regressions against the main baseline of 9 failing files / 23 failing tests.

CI should go green here once #379 and #378 land — no rebase needed, this branch merges cleanly with both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security] GET /api/users leaks every user's email address and allows account enumeration via ?search=

1 participant