fix(quiz): scope graph_nodes by user_id in generate/submit (IDOR)#204
Conversation
|
Warning Review limit reached
More reviews will be available in 56 minutes and 49 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
frontend | 38a0399 | Commit Preview URL Branch Preview URL |
Jun 12 2026, 04:47 AM |
generate_quiz, _legacy_generate_quiz, and submit_quiz fetched and wrote graph_nodes by id alone, so a caller could pass their own user_id with a victim's concept_node_id and corrupt the victim's mastery fields. Scope every node read in the quiz flow by the owning user (body.user_id on generate, attempt["user_id"] on submit) and 404 when the node isn't owned by the caller, before any mastery write. The mastery update is now also user-scoped.
Add TestQuizNodeOwnership: user A generating or submitting a quiz against user B's concept node must 404, and submit must write nothing to graph_nodes. The factory models real DB filtering so the tests fail on unscoped reads and pass only once every node read is user-scoped.
Jose-Gael-Cruz-Lopez
force-pushed
the
fix/quiz-node-ownership
branch
from
3f8a8cc to
38a0399
Compare
1 participant
What
Scope every
graph_nodesaccess in the quiz flow by the owning user so a caller can only generate/submit quizzes against their own concept nodes. Foreign or missing nodes now return 404 instead of leaking content or corrupting another user's mastery.generate_quizand_legacy_generate_quiz: node read filtered by{id, user_id: body.user_id}.submit_quiz: the mastery read/write and theconcept_nameread are filtered by{id, user_id: attempt["user_id"]}; a missing/foreign node now raises 404 before any mastery write (previously it silently defaultedmastery_before=0.0and wrote tograph_nodes WHERE id=<victim node>).Why
require_self(body.user_id)only checks that the request body'suser_idequals the session user — it never verifies the targeted node belongs to that user. Everygraph_nodesread/write in the quiz flow filtered byidalone (unlikeservices/graph_service.py, which scopes byuser_id). An authenticated attacker could:POST /api/quiz/generatewith their ownuser_id+ a victim'sconcept_node_id→ 200 + aquiz_attemptsrow they own (and the victim's concept content leaks into the generated quiz).POST /api/quiz/submitfor that attempt →require_self(attempt["user_id"])passes, and the handler overwrites the victim'smastery_score,mastery_tier,times_studied,last_studied_at, andmastery_events.High-severity IDOR / cross-user data-integrity bug.
How verified
cd backend && PYTHONPATH=. python -m pytest tests/test_quiz_routes.py -q→ 32 passed (30 pre-existing + 2 new).TestQuizNodeOwnershipproves user A cannot generate or submit against user B's node (both 404) and that submit writes nothing tograph_nodes. The mock models real DB row-filtering, so both tests fail on the unscoped code and pass only with the fix (verified by stashing the route change).Closes #157