Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 26 additions & 26 deletions frontend/actions/quiz.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,6 @@ function calculateIntegrityScore(violations: ViolationEvent[]): number {
return Math.max(0, 100 - penalty);
}

function validateTimeSpent(
startedAt: Date,
completedAt: Date,
questionCount: number
): boolean {
const MIN_SECONDS_PER_QUESTION = 1;
const timeSpentSeconds = Math.floor(
(completedAt.getTime() - startedAt.getTime()) / 1000
);
const minRequiredTime = questionCount * MIN_SECONDS_PER_QUESTION;

return timeSpentSeconds >= minRequiredTime;
}

async function getQuizQuestionIds(quizId: string): Promise<string[]> {
const rows = await db
Expand Down Expand Up @@ -176,18 +163,6 @@ export async function submitQuizAttempt(
return { success: false, error: 'Invalid time values' };
}

const isValidTime = validateTimeSpent(
startedAtDate,
completedAtDate,
questionIds.length
);
if (!isValidTime) {
return {
success: false,
error: 'Invalid time spent: quiz completed too quickly',
};
}

const percentage = (
(correctAnswersCount / questionIds.length) *
100
Expand Down Expand Up @@ -260,8 +235,33 @@ export async function initializeQuizCache(
quizId: string
): Promise<{ success: boolean; error?: string }> {
try {
const { getOrCreateQuizAnswersCache } =
const { getOrCreateQuizAnswersCache, clearVerifiedQuestions } =
await import('@/lib/quiz/quiz-answers-redis');

// Resolve identifier (same logic as verify-answer route)
const { headers } = await import('next/headers');
const { verifyAuthToken } = await import('@/lib/auth');
const headersList = await headers();
let identifier: string;

const cookieHeader = headersList.get('cookie') ?? '';
const authCookie = cookieHeader
.split(';')
.find(c => c.trim().startsWith('auth_session='));

if (authCookie) {
const token = authCookie.split('=').slice(1).join('=').trim();
const payload = verifyAuthToken(token);
identifier = payload?.userId ?? 'unknown';
} else {
identifier =
headersList.get('x-forwarded-for')?.split(',')[0]?.trim() ??
headersList.get('x-real-ip') ??
'unknown';
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fallback identifier 'unknown' is shared across all unresolvable users.

When both the auth cookie is absent and IP headers are missing, identifier falls back to 'unknown'. This means all such users share the same Redis key namespace — one guest's verification marks could block or clear another's.

Consider rejecting the request or generating a per-request ephemeral identifier instead of silently proceeding with a shared sentinel.

🤖 Prompt for AI Agents
In `@frontend/actions/quiz.ts` around lines 255 - 261, The current fallback sets
identifier = 'unknown', creating a shared Redis namespace for unrelated guests;
change this so that when payload?.userId is missing and
headersList.get('x-forwarded-for') and headersList.get('x-real-ip') are missing
you either reject the request (return/throw a 4xx) or generate a per-request
ephemeral identifier instead of the literal 'unknown'. Concretely, replace the
'unknown' fallback used in the identifier assignment (the block referencing
payload?.userId, headersList.get('x-forwarded-for') and
headersList.get('x-real-ip')) with logic to: a) validate that an authenticated
user or client IP exists and return a 400/401 if not, or b) create a strong
ephemeral id (e.g., crypto.randomUUID()-like) scoped to this request and use
that for Redis keys, ensuring no shared sentinel key is used across requests.


await clearVerifiedQuestions(quizId, identifier);

const success = await getOrCreateQuizAnswersCache(quizId);

if (!success) {
Expand Down
4 changes: 2 additions & 2 deletions frontend/app/[locale]/q&a/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import { getTranslations } from 'next-intl/server';
import { Suspense } from 'react';

import QaSection from '@/components/q&a/QaSection';
import { QaLoader } from '@/components/shared/QaLoader';
import { DynamicGridBackground } from '@/components/shared/DynamicGridBackground';
import { Loader } from '@/components/shared/Loader';

export async function generateMetadata({
params,
Expand Down Expand Up @@ -40,7 +40,7 @@ export default async function QAPage({
<Suspense
fallback={
<div className="flex justify-center py-16">
<QaLoader className="mx-auto" size={260} />
<Loader className="mx-auto" size={260} />
</div>
}
>
Expand Down
9 changes: 9 additions & 0 deletions frontend/app/[locale]/quiz/[slug]/loading.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { Loader } from '@/components/shared/Loader';

export default function QuizLoading() {
return (
<div className="flex min-h-screen items-center justify-center bg-white dark:bg-black">
<Loader size={200} />
</div>
);
}
16 changes: 5 additions & 11 deletions frontend/app/[locale]/quiz/[slug]/page.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Metadata } from 'next';
import { notFound, redirect } from 'next/navigation';
import { notFound } from 'next/navigation';
import { getTranslations } from 'next-intl/server';

import { QuizContainer } from '@/components/quiz/QuizContainer';
Expand Down Expand Up @@ -49,16 +49,10 @@ export default async function QuizPage({
notFound();
}

if (!seedParam) {
// eslint-disable-next-line react-hooks/purity -- redirect throws, value never used in render
redirect(`/${locale}/quiz/${slug}?seed=${Date.now()}`);
}

const seed = Number.parseInt(seedParam, 10);
if (Number.isNaN(seed)) {
// eslint-disable-next-line react-hooks/purity -- redirect throws, value never used in render
redirect(`/${locale}/quiz/${slug}?seed=${Date.now()}`);
}
const parsedSeed = seedParam ? Number.parseInt(seedParam, 10) : Number.NaN;
const seed = Number.isFinite(parsedSeed)
? parsedSeed
: crypto.getRandomValues(new Uint32Array(1))[0]!;

const questions = await getQuizQuestionsRandomized(quiz.id, locale, seed);

Expand Down
48 changes: 46 additions & 2 deletions frontend/app/api/quiz/verify-answer/route.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import { headers } from 'next/headers';
import { NextResponse } from 'next/server';

import { getCorrectAnswer, getOrCreateQuizAnswersCache } from '@/lib/quiz/quiz-answers-redis';
import { verifyAuthToken } from '@/lib/auth';
import {
getCorrectAnswer,
getOrCreateQuizAnswersCache,
isQuestionAlreadyVerified,
markQuestionVerified,
} from '@/lib/quiz/quiz-answers-redis';

export const runtime = 'nodejs';

Expand All @@ -15,7 +22,40 @@ export async function POST(req: Request) {
);
}

const { quizId, questionId, selectedAnswerId } = body;
const { quizId, questionId, selectedAnswerId, timeLimitSeconds } = body;

// Identify user: userId for authenticated, IP for guests
const headersList = await headers();
let identifier: string;

const cookieHeader = headersList.get('cookie') ?? '';
const authCookie = cookieHeader
.split(';')
.find(c => c.trim().startsWith('auth_session='));

if (authCookie) {
const token = authCookie.split('=').slice(1).join('=').trim();
const payload = verifyAuthToken(token);
identifier = payload?.userId ?? 'unknown';
} else {
identifier =
headersList.get('x-forwarded-for')?.split(',')[0]?.trim() ??
headersList.get('x-real-ip') ??
'unknown';
}

// Reject duplicate verification (bot protection)
const alreadyVerified = await isQuestionAlreadyVerified(
quizId,
questionId,
identifier
);
if (alreadyVerified) {
return NextResponse.json(
{ success: false, error: 'Question already answered' },
{ status: 409 }
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

TOCTOU gap between duplicate check and mark — concurrent requests can slip through.

isQuestionAlreadyVerified (GET) and markQuestionVerified (SET with EX) are two separate Redis calls. A fast bot issuing parallel requests for the same question can pass the check before either is marked. Use an atomic SET NX (set-if-not-exists) to both check and mark in a single call.

🔒 Atomic check-and-set approach

Replace the two-step check+set with a single SET key value EX ttl NX call. With @upstash/redis, this looks like:

const key = `quiz:verified:${quizId}:${identifier}:${questionId}`;
const wasSet = await redis.set(key, 1, { ex: ttl, nx: true });
if (!wasSet) {
  return NextResponse.json(
    { success: false, error: 'Question already answered' },
    { status: 409 }
  );
}

This eliminates the race window entirely. You could encapsulate this in a single tryMarkQuestionVerified helper that replaces both isQuestionAlreadyVerified and markQuestionVerified.

Also applies to: 76-79

🤖 Prompt for AI Agents
In `@frontend/app/api/quiz/verify-answer/route.ts` around lines 47 - 58, The
duplicate-check uses two separate Redis calls (isQuestionAlreadyVerified and
markQuestionVerified), causing a TOCTOU race where concurrent requests can both
pass the check; replace the two-step pattern with a single atomic SET NX with
TTL (e.g., use redis.set(key, value, { ex: ttl, nx: true })) to both
check-and-mark in one call and return the 409 when the set fails; implement this
logic either inline in route.ts or as a new helper (tryMarkQuestionVerified) and
remove the separate isQuestionAlreadyVerified/markQuestionVerified sequence.

Comment on lines +30 to +42
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

No bot protection when identifier is null.

When resolveRequestIdentifier returns null (e.g., no auth cookie and no IP headers, or an invalid auth cookie — see related comment in resolve-identifier.ts), both the duplicate check (line 30) and the mark (line 65) are skipped. This means those requests bypass bot protection entirely. Consider returning a 400 when the identifier cannot be resolved, rather than silently proceeding without any tracking.

Suggested approach
     const identifier = resolveRequestIdentifier(headersList);
-     if (identifier) {
-      const alreadyVerified = await isQuestionAlreadyVerified(
+    if (!identifier) {
+      return NextResponse.json(
+        { success: false, error: 'Unable to identify request' },
+        { status: 400 }
+      );
+    }
+
+    const alreadyVerified = await isQuestionAlreadyVerified(
         quizId,
         questionId,
         identifier
       );
       if (alreadyVerified) {
         return NextResponse.json(
           { success: false, error: 'Question already answered' },
           { status: 409 }
         );
       }
-    }

And similarly remove the if (identifier) guard around markQuestionVerified.

Also applies to: 65-67

🤖 Prompt for AI Agents
In `@frontend/app/api/quiz/verify-answer/route.ts` around lines 30 - 42, The
request currently skips bot-protection when resolveRequestIdentifier returns
null; change the route handler to reject requests with a missing identifier by
returning a 400 JSON response (use NextResponse.json({ success: false, error:
'Missing identifier' }, { status: 400 })) when resolveRequestIdentifier(...)
yields null, and remove the conditional guards around both
isQuestionAlreadyVerified(...) and markQuestionVerified(...) so these functions
always run with a validated identifier (or never run because the handler already
returned 400). Reference resolveRequestIdentifier, isQuestionAlreadyVerified,
markQuestionVerified, and the route handler's response logic when making the
change.


let correctAnswerId = await getCorrectAnswer(quizId, questionId);

Expand All @@ -33,6 +73,10 @@ export async function POST(req: Request) {
);
}

const ttl = typeof timeLimitSeconds === 'number' && timeLimitSeconds > 0
? timeLimitSeconds + 60
: 900; // 15min fallback
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
await markQuestionVerified(quizId, questionId, identifier, ttl);
const isCorrect = selectedAnswerId === correctAnswerId;

return NextResponse.json({
Expand Down
4 changes: 2 additions & 2 deletions frontend/components/q&a/QaSection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import { useTranslations } from 'next-intl';
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';

import AccordionList from '@/components/q&a/AccordionList';
import { QaLoader } from '@/components/shared/QaLoader';
import { Pagination } from '@/components/q&a/Pagination';
import type { CategorySlug } from '@/components/q&a/types';
import { useQaTabs } from '@/components/q&a/useQaTabs';
import { CategoryTabButton } from '@/components/shared/CategoryTabButton';
import { Loader } from '@/components/shared/Loader';
import { Tabs, TabsContent, TabsList } from '@/components/ui/tabs';
import { categoryData } from '@/data/category';
import { categoryTabStyles } from '@/data/categoryStyles';
Expand Down Expand Up @@ -103,7 +103,7 @@ export default function TabsSection() {
<TabsContent key={category.slug} value={category.slug}>
{isLoading && (
<div className="flex justify-center py-12">
<QaLoader className="mx-auto" size={240} />
<Loader className="mx-auto" size={240} />
</div>
)}
<div
Expand Down
22 changes: 17 additions & 5 deletions frontend/components/quiz/QuizCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useTranslations } from 'next-intl';

import { Badge } from '@/components/ui/badge';
import { categoryTabStyles } from '@/data/categoryStyles';
import { Link } from '@/i18n/routing';
import { useRouter } from '@/i18n/routing';

interface QuizCardProps {
quiz: {
Expand All @@ -25,18 +25,30 @@ interface QuizCardProps {
} | null;
}

function makeSeed(): number {
const buf = new Uint32Array(1);
crypto.getRandomValues(buf);
return buf[0]!;
}

export function QuizCard({ quiz, userProgress }: QuizCardProps) {
const router = useRouter();
const t = useTranslations('quiz.card');
const slug = quiz.categorySlug as keyof typeof categoryTabStyles | null;
const style =
slug && categoryTabStyles[slug] ? categoryTabStyles[slug] : null;
const accentColor = style?.accent ?? '#3B82F6'; // fallback blue
const accentColor = style?.accent ?? '#3B82F6';

const percentage =
userProgress && userProgress.totalQuestions > 0
? Math.round((userProgress.bestScore / userProgress.totalQuestions) * 100)
: 0;

const handleStart = () => {
const seed = makeSeed(); // runs on click, not render
router.push(`/quiz/${quiz.slug}?seed=${seed}`);
};

return (
<div
className="group/card relative flex flex-col overflow-hidden rounded-xl border border-black/10 bg-white p-5 shadow-sm transition-all duration-300 hover:-translate-y-1 hover:!border-[var(--accent)] hover:shadow-xl dark:border-white/10 dark:bg-neutral-900"
Expand Down Expand Up @@ -103,8 +115,8 @@ export function QuizCard({ quiz, userProgress }: QuizCardProps) {
</div>
</div>
)}
<Link
href={`/quiz/${quiz.slug}`}
<button
type="button" onClick={handleStart}
className="group relative block w-full overflow-hidden rounded-xl border px-4 py-2.5 text-center text-sm font-semibold transition-all duration-300"
style={{
borderColor: `${accentColor}50`,
Expand All @@ -117,7 +129,7 @@ export function QuizCard({ quiz, userProgress }: QuizCardProps) {
className="pointer-events-none absolute top-1/2 left-1/2 h-[150%] w-[80%] -translate-x-1/2 -translate-y-1/2 rounded-full opacity-0 blur-[20px] transition-opacity duration-300 group-hover:opacity-30"
style={{ backgroundColor: accentColor }}
/>
</Link>
</button>
</div>
);
}
Loading