Fix answer-key leak and response-time forgery in live polls - #76
Open
tanvishdesai wants to merge 1 commit into
Open
Fix answer-key leak and response-time forgery in live polls#76tanvishdesai wants to merge 1 commit into
tanvishdesai wants to merge 1 commit into
Conversation
A live question's options[].isCorrect was sent to every student who had joined the room, whether or not they had answered it yet, and POST /api/responses trusted the client-supplied responseTime with no server-side timestamp to check it against. Together these let a client read the correct answer straight off the wire and submit it immediately with a forged near-zero responseTime, scoring near-max points regardless of whether the question was still live, and regardless of when it actually launched. - Strip options[].isCorrect (and explanation) from GET /api/questions and GET /api/responses/room/:roomId/student/:studentId for any question a student hasn't answered yet. The /responses route also reveals the key once a question's own answer window has genuinely closed, so the existing "you missed this - the answer was X" review UI keeps working for past questions; only a still-live, unanswered one stays hidden. - Add Question.launchedAt, stamped the moment a question is created with status 'approved' (today, that IS the launch moment - every question-creation path in the app POSTs already-approved). POST /api/responses now rejects (410) anything submitted more than timeToAnswer + 5s after launch, and floors the client-reported responseTime against the server's own observed elapsed time so a forged value can no longer read as more than ~2s faster than physically possible. - Let a teacher create the next question without leaving and reopening the creation dialog: when a launched question's timer ends, the form now resets in place instead of auto-closing. Existing documents have no launchedAt; both routes fall back to createdAt, so no migration is required. Tests: 28 new unit tests covering the answer-key gating and the timing/scoring logic (100/100 backend tests passing). Verified live against a local dev server: an unanswered live question no longer exposes isCorrect, a submission past its window returns 410, and a forged responseTime=1 after ~10s of real elapsed time now scores 73/100 instead of the previous ~97/100.
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
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.
Summary
A live poll's answer key was reachable by any joined student before they answered, and the score a submission earned was computed entirely from client-supplied data with no server-side check against when the question actually launched. Together these let a script read the correct answer off the wire and submit it with a forged near-zero reaction time, scoring near-max points on any question regardless of whether it was still live. This PR closes both holes and, separately, removes a UX dead-end in manual question creation.
Problem
GET /api/questionsandGET /api/responses/room/:roomId/student/:studentIdreturnedoptions[].isCorrectfor every question in the room unconditionally — including a question the requesting student hadn't answered yet. Any client that could reach these (identical calls the student UI already makes on page load and on every poll transition) could read the correct option directly instead of answering it.POST /api/responsesscored a submission using theresponseTimefield exactly as the client sent it, with no timestamp on the question to check it against. A client could submitresponseTime: 1for a question that launched an hour earlier and still receive credit for a near-instant, near-max-points answer — thetimeToAnswerwindow was purely cosmetic on the server side.CreateQuestionOverlayauto-closed the moment a launched question's timer ran out, so a teacher creating several questions in a row had to leave the dialog and reopen it from the room page for every single one.Fix
Answer-key leak —
backend/src/utils/sanitize.jsstripAnswerKey(question, reveal), removingoptions[].isCorrectandexplanationunlessrevealis true.GET /api/questions: reveals per-question only to the teacher or a student who has already answered it.GET /api/responses/room/:roomId/student/:studentId: reveals to the teacher, a student who has answered, or once the question's own answer window has closed — this second condition matters because the same endpoint backs the "you missed this — the correct answer was X" review UI for past questions, so a genuinely closed question still shows its key on review; only a still-live, unanswered one stays hidden.Response-timing enforcement —
backend/src/utils/answerWindow.js(new),backend/src/models/Question.js,backend/src/routes/{questions,responses}.jsQuestion.launchedAt, stamped inPOST /api/questionsthe moment a question is created withstatus: 'approved'. Every question-creation path in the app already POSTs pre-approved (manual creation, AI-approval, text-question-approval all setstatus: 'approved'at the same call that broadcasts the question), so this timestamp is the true launch moment, not an approximation.POST /api/responsesnow rejects with410anything submitted more thantimeToAnswer + 5safterlaunchedAt(grace covers normal network latency and the client's own deliberate 0–2s send-side jitter, seeStudentRoomPage.jsx).responseTime— kept because the client freezes it at click time, before its own send jitter, so a genuinely fast click is never penalized by network delay — is now floored against the server's own observed elapsed time (elapsed - 2s), so a forged low value can no longer read as more than ~2s faster than physically possible.Questiondocuments predatelaunchedAt; both routes fall back tocreatedAt, so no data migration is needed.Question-creation flow —
frontend/src/components/CreateQuestionOverlay.jsxTest plan
backend/src/__tests__/answerKeyLeak.test.js(key-stripping and reveal-gating logic) andbackend/src/__tests__/answerWindow.test.js(deadline enforcement and response-time flooring, including an end-to-end scoring scenario). Full suite: 100/100 passing.options[]no longer carriesisCorrect.timeToAnswer + 5sreturns410.responseTime: 1submitted after ~10s of real elapsed time on a 30s question scored73/100, versus~97/100before this fix.npm run build(frontend) succeeds with theCreateQuestionOverlaychanges.Notes for reviewers
ANSWER_GRACE_S(deadline tolerance) andRESPONSE_TIME_SLACK_S(response-time floor discount) inanswerWindow.jsare deliberately separate constants — using one grace value for both was an intermediate version of this fix that let every submission shave a flat multi-second discount off its score regardless of when it actually arrived. Caught in the live verification pass above; see the module doc comment for the reasoning.