fix: scope recent-achievements query by userId (cross-tenant data leak)#410
Merged
Merged
Conversation
The dashboard recent-achievements query used JavaScript `&&` instead of
Drizzle's `and()` to combine its filters:
.where(eq(achievement.userId, userId) && eq(achievement.isArchived, false))
`eq()` returns a truthy query-fragment object, so `truthy && expr`
evaluates to only the right-hand operand. The effective query was
`WHERE isArchived = false` with no user scoping, so the endpoint
returned the 10 most-recent achievements across ALL users — title,
summary, impact, project and company — on every dashboard load. The
`userId !== user.id` guard above only validates the query-string param;
the DB query ignored it.
Replace `&&` with `and()` so both predicates are applied. Grepped the
codebase for the same bug class (`&&` between Drizzle operators in a
`.where()`); this was the only occurrence.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Braintrust eval reportNo experiments to report |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Security fix — CRITICAL
The dashboard recent-achievements endpoint leaks every user's achievements to any authenticated user.
Root cause
apps/web/app/api/dashboard/recent-achievements/route.tscombined its filters with JavaScript&&instead of Drizzle'sand():eq()returns a truthy query-fragment object, sotruthy && exprevaluates to only the right-hand operand. The effective query wasWHERE isArchived = false— no user scoping. The endpoint returned the 10 most-recent achievements across all users (title, summary, impact, project, company) on every dashboard load.The
if (!userId || userId !== user.id)guard above only validates the query-string parameter; the DB query ignored it entirely, so the check gave a false sense of safety.Fix
Use
and()so both predicates apply:Scope check
Grepped the codebase for the same bug class (
&&between Drizzle operators inside a.where()) — this was the only occurrence.Notes
SECURITY-REVIEW.md(finding CRIT-2). Consider a lint rule to ban&&inside Drizzle.where(...).🤖 Generated with Claude Code