From 603b6c5b5eab4cc296fd22bd9fe99829524e3f66 Mon Sep 17 00:00:00 2001 From: Ihsan Ullah Date: Sat, 29 Aug 2026 14:59:34 +0500 Subject: [PATCH 1/2] exclude hidden-column scores from leaderboard and submission APIs Scores for columns organizers mark as hidden were still visible to anyone viewing a leaderboard or submission, even though they were correctly hidden from the leaderboard table itself. Hidden scores are now excluded everywhere, for every type of user. Added tests to cover this going forward. --- src/apps/api/serializers/leaderboards.py | 2 +- src/apps/api/serializers/submissions.py | 6 +- src/apps/api/tests/test_leaderboards.py | 92 ++++++++++++++++ src/apps/api/tests/test_submissions.py | 127 ++++++++++++++++++++++- 4 files changed, 224 insertions(+), 3 deletions(-) diff --git a/src/apps/api/serializers/leaderboards.py b/src/apps/api/serializers/leaderboards.py index b4e244e1f..2c4fc7871 100644 --- a/src/apps/api/serializers/leaderboards.py +++ b/src/apps/api/serializers/leaderboards.py @@ -116,7 +116,7 @@ def get_submissions(self, instance): .prefetch_related( Prefetch( 'scores', - queryset=SubmissionScore.objects.select_related( + queryset=SubmissionScore.objects.filter(column__hidden=False).select_related( 'column', 'column__leaderboard', ), diff --git a/src/apps/api/serializers/submissions.py b/src/apps/api/serializers/submissions.py index 9c91737ca..eda7daa10 100644 --- a/src/apps/api/serializers/submissions.py +++ b/src/apps/api/serializers/submissions.py @@ -19,7 +19,7 @@ class SubmissionSerializer(serializers.ModelSerializer): - scores = SubmissionScoreSerializer(many=True) + scores = serializers.SerializerMethodField(read_only=True) filename = serializers.SerializerMethodField(read_only=True) owner = serializers.CharField(source='owner.username') phase_name = serializers.CharField(source='phase.name') @@ -72,6 +72,10 @@ def get_filename(self, instance): # NOTE: if submission data is None, it means it is soft deleted return "Deleted File" + def get_scores(self, instance): + scores = [score for score in instance.scores.all() if not score.column.hidden] + return SubmissionScoreSerializer(scores, many=True, context=self.context).data + def get_auto_run(self, instance): # returns this submission's competition auto_run_submissions Flag return instance.phase.competition.auto_run_submissions diff --git a/src/apps/api/tests/test_leaderboards.py b/src/apps/api/tests/test_leaderboards.py index 5455ef173..c5a899028 100644 --- a/src/apps/api/tests/test_leaderboards.py +++ b/src/apps/api/tests/test_leaderboards.py @@ -128,3 +128,95 @@ def test_anonymous_user_cannot_see_leaderboard_entries(self): self.lb.save() resp = self.get_leaderboard() assert resp.status_code == 200 + + +class HiddenColumnScoreTests(APITestCase): + """ + Column.hidden is meant to hide a score from everyone + viewing the leaderboard (e.g. so participants can't tune submissions + against it), so a hidden column's score must never appear in the + leaderboard-detail (`/api/leaderboards//`) response, for any viewer + - anonymous, logged-in, or organizer/admin alike - consistent with how + the `columns` metadata list already filters hidden columns out. + (Previously `submissions[*].scores` was built from a query with no + hidden-column filter, so a hidden column's score leaked through even + though the UI never rendered a column for it.) + """ + + def setUp(self): + self.creator = factories.UserFactory(username='hcs_creator', password='test') + self.admin = factories.UserFactory(username='hcs_admin', password='test', super_user=True) + self.normal_user = factories.UserFactory(username='hcs_normal', password='test') + self.comp = factories.CompetitionFactory(created_by=self.creator) + self.leaderboard = factories.LeaderboardFactory(hidden=False, primary_index=0) + self.phase = factories.PhaseFactory(competition=self.comp, leaderboard=self.leaderboard) + + self.visible_column = factories.ColumnFactory( + leaderboard=self.leaderboard, index=0, key='visible_col', hidden=False) + self.hidden_column = factories.ColumnFactory( + leaderboard=self.leaderboard, index=1, key='hidden_col', hidden=True) + + self.submission = factories.SubmissionFactory(phase=self.phase, leaderboard=self.leaderboard) + factories.SubmissionScoreFactory(submissions=[self.submission], column=self.visible_column) + factories.SubmissionScoreFactory(submissions=[self.submission], column=self.hidden_column) + + def get_leaderboard(self): + return self.client.get(reverse('leaderboard-detail', kwargs={'pk': self.leaderboard.id})) + + def score_column_keys(self, resp): + submissions = resp.json()['submissions'] + assert len(submissions) == 1 + return {score['column_key'] for score in submissions[0]['scores']} + + def test_anonymous_user_does_not_see_hidden_column_score(self): + """ + An anonymous (unauthenticated) GET on leaderboard-detail must not + include the hidden column's score in the submission's scores array. + Expected: only the visible column's score is returned. + """ + resp = self.get_leaderboard() + assert resp.status_code == 200 + keys = self.score_column_keys(resp) + assert self.hidden_column.key not in keys + assert self.visible_column.key in keys + + def test_normal_authenticated_user_does_not_see_hidden_column_score(self): + """ + A logged-in user with no organizer/collaborator/admin relationship to + the competition must not see the hidden column's score via + leaderboard-detail either. Expected: only the visible column's score + is returned. + """ + self.client.force_login(self.normal_user) + resp = self.get_leaderboard() + assert resp.status_code == 200 + keys = self.score_column_keys(resp) + assert self.hidden_column.key not in keys + assert self.visible_column.key in keys + + def test_competition_creator_does_not_see_hidden_column_score(self): + """ + For consistency with the columns metadata (which already filters out + hidden columns for everyone, organizers included), the competition + creator must not see the hidden column's score via leaderboard-detail + either. Expected: only the visible column's score is returned. + """ + self.client.force_login(self.creator) + resp = self.get_leaderboard() + assert resp.status_code == 200 + keys = self.score_column_keys(resp) + assert self.hidden_column.key not in keys + assert self.visible_column.key in keys + + def test_superuser_does_not_see_hidden_column_score(self): + """ + Even a superuser/site-admin must not see the hidden column's score + via this endpoint, for the same consistency reason. Expected: only + the visible column's score is returned. + """ + self.client.force_login(self.admin) + resp = self.get_leaderboard() + assert resp.status_code == 200 + keys = self.score_column_keys(resp) + assert self.hidden_column.key not in keys + assert self.visible_column.key in keys diff --git a/src/apps/api/tests/test_submissions.py b/src/apps/api/tests/test_submissions.py index 8601f0aa2..a5c6267d0 100644 --- a/src/apps/api/tests/test_submissions.py +++ b/src/apps/api/tests/test_submissions.py @@ -6,7 +6,7 @@ from competitions.models import Submission, CompetitionParticipant from factories import UserFactory, CompetitionFactory, PhaseFactory, CompetitionParticipantFactory, SubmissionFactory, \ - TaskFactory, OrganizationFactory, DataFactory, LeaderboardFactory + TaskFactory, OrganizationFactory, DataFactory, LeaderboardFactory, ColumnFactory, SubmissionScoreFactory from datasets.models import Data from profiles.models import Membership @@ -695,3 +695,128 @@ def test_organization_is_removed_from_soft_deleted_submission(self): self.organization_submission.refresh_from_db() assert self.organization_submission.is_soft_deleted is True assert self.organization_submission.organization is None + + +class HiddenColumnSubmissionScoreTests(APITestCase): + """ + Column.hidden is meant to hide a score from everyone + (e.g. so participants can't tune submissions against it), so a hidden + column's score must never appear in the submission-list + (`/api/submissions/?phase=`) or submission-detail + (`/api/submissions//`) responses, for any viewer - anonymous, the + submission's own owner, or organizer/admin alike - consistent with how + hidden columns are already excluded from leaderboard column metadata. + (Previously SubmissionSerializer.scores serialized every SubmissionScore + row with no awareness of Column.hidden, so hidden-column values leaked + to anyone who could see the submission at all, including anonymous + requests to on-leaderboard, finished submissions.) + """ + + def setUp(self): + self.creator = UserFactory(username='hcss_creator', password='test') + self.superuser = UserFactory(username='hcss_admin', password='test', is_superuser=True, is_staff=True) + self.owner = UserFactory(username='hcss_owner', password='test') + self.comp = CompetitionFactory(created_by=self.creator) + self.leaderboard = LeaderboardFactory(primary_index=0) + self.phase = PhaseFactory(competition=self.comp, leaderboard=self.leaderboard) + + CompetitionParticipantFactory(user=self.owner, competition=self.comp, status=CompetitionParticipant.APPROVED) + + self.visible_column = ColumnFactory(leaderboard=self.leaderboard, index=0, key='visible_col', hidden=False) + self.hidden_column = ColumnFactory(leaderboard=self.leaderboard, index=1, key='hidden_col', hidden=True) + + # SubmissionViewSet.get_queryset intentionally exposes on-leaderboard, + # FINISHED, non-soft-deleted submissions to anonymous requests (that's + # how the public leaderboard page works for logged-out visitors) - so + # status=FINISHED and leaderboard=self.leaderboard here are required + # for the anonymous tests below to even reach this submission, not + # part of the bug being tested. What must stay hidden is only the + # hidden column's score within it, not the submission itself. + self.submission = SubmissionFactory( + phase=self.phase, + owner=self.owner, + leaderboard=self.leaderboard, + status=Submission.FINISHED, + ) + SubmissionScoreFactory(submissions=[self.submission], column=self.visible_column) + SubmissionScoreFactory(submissions=[self.submission], column=self.hidden_column) + + def score_column_keys_from_list(self, resp): + body = resp.json() + submissions = body.get('results', body) + matching = [s for s in submissions if s['id'] == self.submission.id] + assert len(matching) == 1 + return {score['column_key'] for score in matching[0]['scores']} + + def score_column_keys_from_detail(self, resp): + return {score['column_key'] for score in resp.json()['scores']} + + def test_anonymous_user_does_not_see_hidden_column_score_in_submission_list(self): + """ + An anonymous GET on submission-list, filtered to the submission's + phase, must not include the hidden column's score. Expected: only the + visible column's score is present in the submission's scores array. + """ + url = reverse('submission-list') + resp = self.client.get(url, {'phase': self.phase.id}) + assert resp.status_code == 200 + keys = self.score_column_keys_from_list(resp) + assert self.hidden_column.key not in keys + assert self.visible_column.key in keys + + def test_anonymous_user_does_not_see_hidden_column_score_in_submission_detail(self): + """ + An anonymous GET on submission-detail must not include the hidden + column's score. Expected: only the visible column's score is present + in the submission's scores array. + """ + url = reverse('submission-detail', args=(self.submission.pk,)) + resp = self.client.get(url) + assert resp.status_code == 200 + keys = self.score_column_keys_from_detail(resp) + assert self.hidden_column.key not in keys + assert self.visible_column.key in keys + + def test_submission_owner_does_not_see_hidden_column_score(self): + """ + The submission owner is exactly the kind of participant an organizer + wants to keep a hidden metric from, so even the owner viewing their + own submission must not see the hidden column's score. Expected: only + the visible column's score is present. + """ + self.client.force_login(self.owner) + url = reverse('submission-detail', args=(self.submission.pk,)) + resp = self.client.get(url) + assert resp.status_code == 200 + keys = self.score_column_keys_from_detail(resp) + assert self.hidden_column.key not in keys + assert self.visible_column.key in keys + + def test_competition_creator_does_not_see_hidden_column_score(self): + """ + For consistency with how hidden columns are excluded from leaderboard + column metadata for everyone, the competition creator must not see + the hidden column's score via submission-detail either. Expected: + only the visible column's score is present. + """ + self.client.force_login(self.creator) + url = reverse('submission-detail', args=(self.submission.pk,)) + resp = self.client.get(url) + assert resp.status_code == 200 + keys = self.score_column_keys_from_detail(resp) + assert self.hidden_column.key not in keys + assert self.visible_column.key in keys + + def test_superuser_does_not_see_hidden_column_score(self): + """ + Even a superuser/site-admin must not see the hidden column's score + via submission-detail, for the same consistency reason. Expected: + only the visible column's score is present. + """ + self.client.force_login(self.superuser) + url = reverse('submission-detail', args=(self.submission.pk,)) + resp = self.client.get(url) + assert resp.status_code == 200 + keys = self.score_column_keys_from_detail(resp) + assert self.hidden_column.key not in keys + assert self.visible_column.key in keys From d93db891e444bb912618d73db6e24db50c609778 Mon Sep 17 00:00:00 2001 From: didayolo Date: Tue, 15 Sep 2026 17:38:35 +0200 Subject: [PATCH 2/2] Fix tests --- src/apps/api/tests/test_submissions.py | 46 ++++++++++---------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/src/apps/api/tests/test_submissions.py b/src/apps/api/tests/test_submissions.py index a5c6267d0..d6e4c5ad7 100644 --- a/src/apps/api/tests/test_submissions.py +++ b/src/apps/api/tests/test_submissions.py @@ -703,13 +703,16 @@ class HiddenColumnSubmissionScoreTests(APITestCase): (e.g. so participants can't tune submissions against it), so a hidden column's score must never appear in the submission-list (`/api/submissions/?phase=`) or submission-detail - (`/api/submissions//`) responses, for any viewer - anonymous, the - submission's own owner, or organizer/admin alike - consistent with how - hidden columns are already excluded from leaderboard column metadata. + (`/api/submissions//`) responses, for any viewer - the submission's + own owner, or organizer/admin alike - consistent with how hidden columns + are already excluded from leaderboard column metadata. (Previously SubmissionSerializer.scores serialized every SubmissionScore row with no awareness of Column.hidden, so hidden-column values leaked - to anyone who could see the submission at all, including anonymous - requests to on-leaderboard, finished submissions.) + to anyone who could see the submission at all.) + + Anonymous viewers aren't covered here: SubmissionViewSet.get_queryset + returns nothing at all to them, tested separately by + test_anonymous_cannot_list_or_retrieve_submissions. """ def setUp(self): @@ -725,13 +728,9 @@ def setUp(self): self.visible_column = ColumnFactory(leaderboard=self.leaderboard, index=0, key='visible_col', hidden=False) self.hidden_column = ColumnFactory(leaderboard=self.leaderboard, index=1, key='hidden_col', hidden=True) - # SubmissionViewSet.get_queryset intentionally exposes on-leaderboard, - # FINISHED, non-soft-deleted submissions to anonymous requests (that's - # how the public leaderboard page works for logged-out visitors) - so - # status=FINISHED and leaderboard=self.leaderboard here are required - # for the anonymous tests below to even reach this submission, not - # part of the bug being tested. What must stay hidden is only the - # hidden column's score within it, not the submission itself. + # status=FINISHED and leaderboard=self.leaderboard put the submission in + # its most-exposed state (on a leaderboard, done running); what must stay + # hidden is the hidden column's score within it, not the submission itself. self.submission = SubmissionFactory( phase=self.phase, owner=self.owner, @@ -751,12 +750,14 @@ def score_column_keys_from_list(self, resp): def score_column_keys_from_detail(self, resp): return {score['column_key'] for score in resp.json()['scores']} - def test_anonymous_user_does_not_see_hidden_column_score_in_submission_list(self): + def test_submission_owner_does_not_see_hidden_column_score_in_submission_list(self): """ - An anonymous GET on submission-list, filtered to the submission's - phase, must not include the hidden column's score. Expected: only the - visible column's score is present in the submission's scores array. + A GET on submission-list as the submission owner, filtered to the + submission's phase, must not include the hidden column's score. + Expected: only the visible column's score is present in the + submission's scores array. """ + self.client.force_login(self.owner) url = reverse('submission-list') resp = self.client.get(url, {'phase': self.phase.id}) assert resp.status_code == 200 @@ -764,19 +765,6 @@ def test_anonymous_user_does_not_see_hidden_column_score_in_submission_list(self assert self.hidden_column.key not in keys assert self.visible_column.key in keys - def test_anonymous_user_does_not_see_hidden_column_score_in_submission_detail(self): - """ - An anonymous GET on submission-detail must not include the hidden - column's score. Expected: only the visible column's score is present - in the submission's scores array. - """ - url = reverse('submission-detail', args=(self.submission.pk,)) - resp = self.client.get(url) - assert resp.status_code == 200 - keys = self.score_column_keys_from_detail(resp) - assert self.hidden_column.key not in keys - assert self.visible_column.key in keys - def test_submission_owner_does_not_see_hidden_column_score(self): """ The submission owner is exactly the kind of participant an organizer