diff --git a/apps/web/src/app/api/reviews/[id]/vote/route.ts b/apps/web/src/app/api/reviews/[id]/vote/route.ts index 2db47da4..97a152ce 100644 --- a/apps/web/src/app/api/reviews/[id]/vote/route.ts +++ b/apps/web/src/app/api/reviews/[id]/vote/route.ts @@ -1,7 +1,11 @@ import { revalidateTag } from 'next/cache'; import { NextRequest, NextResponse } from 'next/server'; -import { RATE_CACHE_TAGS } from '@/server/rate'; import { + masterReviewsCacheTag, + productCacheTag, +} from '@/server/rate/cache-tags'; +import { + BoundedSlidingWindowRateLimiter, resolveReviewVoteInput, sanitizePublicApiError, type ReviewReportReason, @@ -11,6 +15,16 @@ import { createLogger, serializeError } from '@/lib/logger'; const log = createLogger('api.reviews.vote'); +// Cache flushes per voter (DOS/Rate#1306): a vote that changes counts +// revalidates the review caches and its product's page, at most this often +// per voter on one instance. Votes past it still count; the caches catch up +// at their own revalidate interval. +const voteRevalidationLimiter = new BoundedSlidingWindowRateLimiter({ + windowMs: 60_000, + maxRequests: 6, + maxBuckets: 5_000, +}); + const REVIEW_VOTE_CORS_HEADERS: Record = { 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Methods': 'POST, OPTIONS', @@ -203,6 +217,25 @@ export async function POST( ); } } else { + // The same vote again changes nothing: no write, no cache flush. Its + // own row is readable to the voter (RLS); if it is not, vote as before. + const { data: current } = await rateClient + .from('review_votes') + .select('vote, reported, report_reason') + .eq('review_id', reviewId) + .eq('voter_id', voterId) + .maybeSingle(); + const sameVote = + current !== null && + current !== undefined && + (voteType === 'report' + ? current.reported === true && + (current.vote ?? null) === null && + (current.report_reason ?? null) === reportReason + : current.vote === voteType && current.reported !== true); + if (sameVote) votesChanged = false; + } + if (voteType !== 'clear' && votesChanged) { const votePayload: { review_id: string; voter_id: string; @@ -286,13 +319,14 @@ export async function POST( ); } - for (const tag of votesChanged - ? [ - RATE_CACHE_TAGS.REVIEWS, - RATE_CACHE_TAGS.PRODUCTS, - RATE_CACHE_TAGS.STATS, - ] - : []) { + // This product's reviews and page only, not the whole catalogue + // (DOS/Rate#1306); global review lists and stats catch up on their own + // interval. + const revalidate = + votesChanged && voteRevalidationLimiter.allow(voterId) + ? await reviewProductTags(rateClient, reviewId) + : []; + for (const tag of revalidate) { try { revalidateTag(tag, 'max'); } catch { @@ -330,3 +364,45 @@ export async function POST( ); } } + +/** + * The cache tags of a review's product (its variant master): its reviews and + * its page. None when they cannot be read; the caches then catch up on their + * own interval. + */ +async function reviewProductTags( + rateClient: Awaited< + ReturnType + > extends infer A + ? A extends { ok: true; rateClient: infer C } + ? C + : never + : never, + reviewId: string +): Promise { + try { + const { data: review } = await rateClient + .from('reviews') + .select('product_id') + .eq('id', reviewId) + .maybeSingle(); + if (!review?.product_id) return []; + const { data: listing } = await rateClient + .from('products') + .select('cluster_id') + .eq('id', review.product_id) + .maybeSingle(); + if (!listing?.cluster_id) return []; + const { data: master } = await rateClient + .from('master_products') + .select('slug') + .eq('id', listing.cluster_id) + .maybeSingle(); + return [ + masterReviewsCacheTag(listing.cluster_id), + ...(master?.slug ? [productCacheTag(master.slug)] : []), + ]; + } catch { + return []; + } +} diff --git a/apps/web/src/app/api/v1/rate/__tests__/write-paths.test.ts b/apps/web/src/app/api/v1/rate/__tests__/write-paths.test.ts index 7b8e15d6..f6ce674e 100644 --- a/apps/web/src/app/api/v1/rate/__tests__/write-paths.test.ts +++ b/apps/web/src/app/api/v1/rate/__tests__/write-paths.test.ts @@ -15,6 +15,8 @@ const ANON_KEY = 'anon-test-key'; const SERVICE_KEY = 'service-role-test-key'; const GOOD_TOKEN = 'good-user-token'; const VOTER_TOKEN = 'eligible-voter-token'; +// An eligible voter whose stored vote is already "up" (DOS/Rate#1306). +const SAME_VOTE_TOKEN = 'same-vote-token'; // What the stubbed PostgREST answers a VOTER_TOKEN vote with (code 42501). let voteRejectionMessage = 'review_votes: account is not eligible to vote on reviews'; @@ -88,9 +90,19 @@ globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { const { pathname } = new URL(url); // A voter the route accepts but the database guard rejects (DOS/Rate#771). + if ( + pathname === '/rest/v1/review_votes' && + (init?.method ?? 'GET') === 'GET' && + headers.authorization === `Bearer ${SAME_VOTE_TOKEN}` + ) { + return jsonResponse(200, [ + { vote: 'up', reported: false, report_reason: null }, + ]); + } if ( pathname === '/auth/v1/user' && - headers.authorization === `Bearer ${VOTER_TOKEN}` + (headers.authorization === `Bearer ${VOTER_TOKEN}` || + headers.authorization === `Bearer ${SAME_VOTE_TOKEN}`) ) { return jsonResponse(200, { ...USER, @@ -671,6 +683,51 @@ async function testCookieSessions() { ); assert.equal((await grantErrorVoteRes.json()).code, 'VOTE_REJECTED'); + // The same vote again writes nothing, so it cannot flush caches (#1306). + calls.length = 0; + await postVote( + new Request(`https://rate.vn/api/reviews/${reviewId}/vote`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${SAME_VOTE_TOKEN}`, + }, + body: JSON.stringify({ voteType: 'up' }), + }) as VoteReq, + { params: Promise.resolve({ id: reviewId }) } + ); + assert.ok( + calls.some( + (c) => c.method === 'GET' && c.url.includes('/rest/v1/review_votes') + ), + 'the stored vote is read first (#1306)' + ); + assert.ok( + !calls.some( + (c) => c.method === 'POST' && c.url.includes('/rest/v1/review_votes') + ), + 'an unchanged vote is not written again (#1306)' + ); + // A report over a stored up-vote changes the vote (it clears it): written. + calls.length = 0; + await postVote( + new Request(`https://rate.vn/api/reviews/${reviewId}/vote`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${SAME_VOTE_TOKEN}`, + }, + body: JSON.stringify({ voteType: 'report', reportReason: 'spam' }), + }) as VoteReq, + { params: Promise.resolve({ id: reviewId }) } + ); + assert.ok( + calls.some( + (c) => c.method === 'POST' && c.url.includes('/rest/v1/review_votes') + ), + 'a report over an up-vote is written (#1306)' + ); + // An ineligible voter (no created_at, no confirmation) can still clear a vote. const clearRes = await postVote( new Request(`https://rate.vn/api/reviews/${reviewId}/vote`, { diff --git a/apps/web/src/server/rate/__tests__/core-rate-architecture.test.ts b/apps/web/src/server/rate/__tests__/core-rate-architecture.test.ts index 58ecde85..f8abdb23 100644 --- a/apps/web/src/server/rate/__tests__/core-rate-architecture.test.ts +++ b/apps/web/src/server/rate/__tests__/core-rate-architecture.test.ts @@ -5289,11 +5289,16 @@ async function runCoreArchitectureSuite() { path.join(repoRoot, 'apps/web/src/app/api/reviews/[id]/vote/route.ts'), 'utf8' ); + // A vote flushes only the review caches and its product's page, not the + // global stats (DOS/Rate#1306); a new review still refreshes the stats. assert.ok( reviewsRouteSrc402.includes('RATE_CACHE_TAGS.STATS') && - voteRouteSrc402.includes('RATE_CACHE_TAGS.STATS') && + voteRouteSrc402.includes('masterReviewsCacheTag(') && + !voteRouteSrc402.includes('RATE_CACHE_TAGS.REVIEWS') && + voteRouteSrc402.includes('productCacheTag(') && + !voteRouteSrc402.includes('RATE_CACHE_TAGS.PRODUCTS') && voteRouteSrc402.includes('authenticateRateRequest('), - 'POST /api/v1/rate/reviews and POST /api/reviews/[id]/vote must revalidate RATE_CACHE_TAGS.STATS and use authenticateRateRequest (#402)' + 'POST /api/v1/rate/reviews must revalidate RATE_CACHE_TAGS.STATS; POST /api/reviews/[id]/vote must revalidate the review caches and its product tag only, and use authenticateRateRequest (#402, #1306)' ); } diff --git a/apps/web/src/server/rate/cache-tags.ts b/apps/web/src/server/rate/cache-tags.ts index 6ac65857..a8c0c003 100644 --- a/apps/web/src/server/rate/cache-tags.ts +++ b/apps/web/src/server/rate/cache-tags.ts @@ -29,6 +29,18 @@ export function productCacheTag(slug: string): string { .slice(0, 120)}`; } +/** + * One product's (variant master's) reviews and review summary, so a vote can + * refresh them without flushing every product's reviews (DOS/Rate#1306). + */ +export function masterReviewsCacheTag(masterId: string): string { + return `rate:reviews:${String(masterId ?? '') + .replace(/\p{Cc}+/gu, '') + .trim() + .toLowerCase() + .slice(0, 120)}`; +} + export function categoryCacheTag(categorySlug: string): string { return `rate:category:${String(categorySlug ?? '') .replace(/\p{Cc}+/gu, '') diff --git a/apps/web/src/server/rate/index.ts b/apps/web/src/server/rate/index.ts index b7e10e93..a0600c96 100644 --- a/apps/web/src/server/rate/index.ts +++ b/apps/web/src/server/rate/index.ts @@ -84,6 +84,7 @@ import { canonicalizeCacheParams, categoryCacheTag, isSafeRevalidationPath, + masterReviewsCacheTag, merchantCacheTag, productCacheTag, resolveRevalidationTargets, @@ -333,7 +334,7 @@ const _getCachedMasterProductReviewsByKey = cache((cleanId: string) => ['rate-master-product-reviews', cleanId], { revalidate: 300, - tags: [RATE_CACHE_TAGS.REVIEWS], + tags: [RATE_CACHE_TAGS.REVIEWS, masterReviewsCacheTag(cleanId)], } )(cleanId) ); @@ -371,7 +372,7 @@ export const getCachedMasterProductReviewSummary = cache( ['rate-master-product-review-summary', cleanId, reviewsKey], { revalidate: 300, - tags: [RATE_CACHE_TAGS.REVIEWS], + tags: [RATE_CACHE_TAGS.REVIEWS, masterReviewsCacheTag(cleanId)], } )(cleanId); }