From cc4463d42ccef1378f0e45dbfb23ab3bb641d8e9 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sun, 13 Sep 2026 05:12:01 +0200 Subject: [PATCH] fix: /questions/detail hands out the answer to anybody signed in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by walking the permission model as a real learner account rather than reading the guards. The bank listing has always nulled the answer side for whoever does not write the question; this route, added for the full-page editor, returned the row whole behind nothing but get_current_user. Any signed-in account could ask for /questions/detail/3869 and be handed the correct option, the explanation, the per-option reasoning and the key points for a question it had never sat — 2,924 questions, one URL, no attempt required. Same rule as the listing now, via the same may_edit_question: the stem still reads, the answer side comes back null, and explanation figures — answer-side by definition — are filtered out with it. Nulled rather than refused, because the stem is legitimately readable. Three tests: a reader gets the stem and none of the answer, whoever writes it gets all of it, and an author counts as writing it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- backend/app/routers/questions.py | 24 +++++--- backend/tests/test_question_detail_access.py | 62 ++++++++++++++++++++ 2 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 backend/tests/test_question_detail_access.py diff --git a/backend/app/routers/questions.py b/backend/app/routers/questions.py index 4977ed4..caddf18 100644 --- a/backend/app/routers/questions.py +++ b/backend/app/routers/questions.py @@ -30,6 +30,7 @@ from app.services import article_service, file_intake, topic_claims, vision_serv from app.services.ai_service import get_configured_model from app.services.search_service import hybrid_ids, hybrid_question_ids, rerank_ids from app.services.question_figures import figures_for as _figures_for +from app.utils.quiz_access import may_edit_question from app.services.prepared_session import prepare_session from app.services.quiz_builder import (bank_query, bank_question_predicate, category_descendants, filtered_bank_query, CreateFromBankRequest, GenerateTestRequest, TestOptions, @@ -1038,23 +1039,32 @@ def get_question_detail( extra = [row[0] for row in db.query(QuestionCategoryLink.category_id).filter( QuestionCategoryLink.question_id == question.id).all()] category = db.get(QuestionCategory, question.question_category_id) if question.question_category_id else None + # The answer side, for the people who write it — the same rule the bank + # listing applies, and this route did not. Being in the bank makes the stem + # yours to read; it has never made the answer yours. Anybody signed in + # could ask for /questions/detail/3869 and be handed the correct option, + # the explanation and the per-option reasoning for a question they had + # never sat. Nulled rather than refused, so the stem still reads. + mine = may_edit_question(db, question, current_user) + withheld = (lambda value: value if mine else None) return { "id": question.id, "question_text": question.question_text, "question_type": question.question_type, "options": question.options, - "correct_answer": question.correct_answer, - "explanation": question.explanation, - "option_explanations": question.option_explanations, - "key_points": question.key_points, - "attending_tip": question.attending_tip, - "figures": _figures_for(db, question.id), + "correct_answer": withheld(question.correct_answer), + "explanation": withheld(question.explanation), + "option_explanations": withheld(question.option_explanations), + "key_points": withheld(question.key_points), + "attending_tip": withheld(question.attending_tip), + "figures": _figures_for(db, question.id) if mine else [ + f for f in _figures_for(db, question.id) if f["role"] == "stem"], "difficulty": question.difficulty, "question_category_id": question.question_category_id, "question_category_name": category.name if category else None, "category_ids": sorted(set(extra) | ({question.question_category_id} if question.question_category_id else set())), "image_path": question.image_path, - "explanation_image_path": question.explanation_image_path, + "explanation_image_path": withheld(question.explanation_image_path), "user_id": question.user_id, "source_quiz_id": question.source_quiz_id, } diff --git a/backend/tests/test_question_detail_access.py b/backend/tests/test_question_detail_access.py new file mode 100644 index 0000000..3f1fa68 --- /dev/null +++ b/backend/tests/test_question_detail_access.py @@ -0,0 +1,62 @@ +"""The answer side of a question, and who is handed it. + +Run: DATABASE_URL=sqlite:///:memory: PYTHONPATH=backend python -m unittest discover -s backend/tests +""" +import unittest + +import test_quiz_builder as fixtures + + +class QuestionDetailAccessTests(unittest.TestCase): + """Being able to reach a question is not the same as having earned its answer. + + The bank listing has always nulled the answer side for anybody who does not + write the question. /questions/detail/{id} did not: anybody signed in could + ask for a question by id and be handed the correct option, the explanation, + the per-option reasoning and the key points for a question they had never + sat. Same rule, both routes. + """ + + def setUp(self): + self.bank = fixtures.BuilderTests() + self.bank.setUp() + self.client = self.bank.client + + def tearDown(self): + self.bank.tearDown() + + def detail(self, question_id=4): + response = self.client.get(f"/questions/detail/{question_id}") + self.assertEqual(response.status_code, 200, response.text) + return response.json() + + def test_a_reader_gets_the_stem_and_none_of_the_answer(self): + # Question 4 belongs to the peer; the owner may read it in the bank and + # has no business writing it. + self.bank.user = self.bank.owner + body = self.detail(4) + self.assertEqual(body["question_text"], "Question 4") + self.assertEqual(body["options"], ["yes", "no"]) + self.assertEqual(body["image_path"], "q.png") + for field in ("correct_answer", "explanation", "option_explanations", + "key_points", "attending_tip", "explanation_image_path"): + self.assertIsNone(body[field], field) + # And no explanation figure, which is answer-side by definition. + self.assertTrue(all(f["role"] == "stem" for f in body["figures"]), body["figures"]) + + def test_whoever_writes_it_gets_all_of_it(self): + self.bank.user = self.bank.mod + body = self.detail(4) + self.assertEqual(body["correct_answer"], "yes") + self.assertEqual(body["explanation"], "Full explanation") + self.assertEqual(body["explanation_image_path"], "answer.png") + + def test_the_author_of_a_question_writes_it(self): + # Question 3 is the owner's own. + self.bank.user = self.bank.owner + body = self.detail(3) + self.assertEqual(body["correct_answer"], "yes") + + +if __name__ == "__main__": + unittest.main()