From dc7f68f44b2a774e7bac6f731eeaa29c05485e31 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 14:20:40 +0000 Subject: [PATCH] feat(aincrad): endpoint to bulk-update OPAL puzzle PDFs Adds the first aincrad endpoint that takes files rather than JSON, so a directory of PDFs can be pushed to the server without visiting the admin once per puzzle. The client lives elsewhere; this is the server side of it. Change detection is a stored SHA-256 per puzzle, reported by `opal_list` alongside the `content` path it already carried. A client compares those against its local files and sends only what differs, and the server re-checks on arrival, so a stale listing costs a transfer but never a wrong write. A blank hash means "unknown", which makes the client upload; that is what every puzzle looks like until its first sync, so nothing needs backfilling. The admin recomputes the hash whenever the file changes, or an upload made there would leave a sync comparing new bytes against a stale fingerprint. Uploads get their own view because `api` is JSON-only, and take one puzzle per request so a partial sync can report per file. The view swaps in TemporaryFileUploadHandler: the project runs memory-only uploads capped at FILE_UPLOAD_MAX_MEMORY_SIZE, and that handler measures the whole request body, so a PDF over the cap would be dropped with request.FILES empty and a 200 going back. The old file is deleted before the new one is written, since the storage key ends in the basename -- a renamed PDF would otherwise strand the old blob and keep serving it. Creating puzzles and deleting PDFs stay in the admin, both being deliberate acts that a directory listing should not be able to trigger by accident. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YZVa3W45G9x7zzk6Q2wA4f --- aincrad/tests.py | 182 +++++++++++++++++- aincrad/urls.py | 5 +- aincrad/views.py | 108 ++++++++++- opal/admin.py | 25 ++- .../0019_opalpuzzle_content_hash.py | 22 +++ opal/models.py | 24 ++- otisweb/test_view_guards.py | 1 + 7 files changed, 344 insertions(+), 23 deletions(-) create mode 100644 opal/migrations/0019_opalpuzzle_content_hash.py diff --git a/aincrad/tests.py b/aincrad/tests.py index 020f36569..1d9f6a380 100644 --- a/aincrad/tests.py +++ b/aincrad/tests.py @@ -1,6 +1,7 @@ from hashlib import sha256 import pytest +from django.core.files.uploadedfile import SimpleUploadedFile from django.test.utils import override_settings from arch.factories import HintFactory, ProblemFactory @@ -10,7 +11,8 @@ from dashboard.models import Announcement from hanabi.factories import HanabiContestFactory, HanabiPlayerFactory from hanabi.models import HanabiParticipation, HanabiReplay -from opal.factories import OpalPuzzleFactory +from opal.factories import OpalHuntFactory, OpalPuzzleFactory +from opal.models import OpalPuzzle from payments.factories import PaymentLogFactory from roster.factories import ( InvoiceFactory, @@ -25,6 +27,10 @@ TARGET_HASH = sha256(EXAMPLE_PASSWORD.encode("ascii")).hexdigest() +def opal_pdf(body: bytes) -> SimpleUploadedFile: + return SimpleUploadedFile("tetrogram.pdf", b"%PDF-1.4 " + body) + + @pytest.fixture def aincrad_setup(db): active_semester = SemesterFactory.create(name="New") @@ -748,9 +754,8 @@ def test_announcement(otis): @pytest.mark.django_db @override_settings(API_TARGET_HASH=TARGET_HASH) def test_opal_handler(otis): - OpalPuzzleFactory.create( - hunt__slug="teammate", slug="tetrogram", is_metapuzzle=True - ) + hunt = OpalHuntFactory.create(slug="teammate") + OpalPuzzleFactory.create(hunt=hunt, slug="tetrogram", is_metapuzzle=True) resp = otis.post_20x( "api", json={ @@ -763,6 +768,27 @@ def test_opal_handler(otis): assert puzzle_json["hunt__slug"] == "teammate" assert puzzle_json["slug"] == "tetrogram" assert puzzle_json["is_metapuzzle"] is True + # A puzzle with no PDF yet reports no fingerprint, which is what tells a sync + # to upload rather than skip. + assert puzzle_json["content"] == "" + assert puzzle_json["content_hash"] == "" + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +def test_opal_handler_reports_content_hash(otis): + puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram") + puzzle.content.save("tetrogram.pdf", opal_pdf(b"first"), save=False) + puzzle.content_hash = "a" * 64 + puzzle.save() + + resp = otis.post_20x( + "api", + json={"action": "opal_list", "token": EXAMPLE_PASSWORD}, + ) + puzzle_json = resp.json()["puzzles"][0] + assert puzzle_json["content_hash"] == "a" * 64 + assert puzzle_json["content"].endswith("tetrogram.pdf") @pytest.mark.django_db @@ -783,3 +809,151 @@ def test_apply_uuid_handler(otis): ).count() == 1 ) + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +def test_opal_pdf_upload(otis): + puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram") + assert puzzle.content_hash == "" + + resp = otis.post_20x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": puzzle.pk, + "content": opal_pdf(b"first draft"), + }, + ) + assert resp.json()["status"] == "created" + puzzle.refresh_from_db() + assert puzzle.content.read() == b"%PDF-1.4 first draft" + assert puzzle.content.name.startswith("opals/teammate/") + assert puzzle.content.name.endswith("/tetrogram.pdf") + assert puzzle.content_hash == sha256(b"%PDF-1.4 first draft").hexdigest() + assert resp.json()["content_hash"] == puzzle.content_hash + + first_name = puzzle.content.name + + # Same bytes again: the server should recognize them and leave storage alone. + resp = otis.post_20x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": puzzle.pk, + "content": opal_pdf(b"first draft"), + }, + ) + assert resp.json()["status"] == "unchanged" + puzzle.refresh_from_db() + assert puzzle.content.name == first_name + + # Different bytes: rewritten in place, with no suffixed duplicate left behind. + resp = otis.post_20x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": puzzle.pk, + "content": opal_pdf(b"second draft"), + }, + ) + assert resp.json()["status"] == "updated" + puzzle.refresh_from_db() + assert puzzle.content.name == first_name + assert puzzle.content.read() == b"%PDF-1.4 second draft" + assert puzzle.content_hash == sha256(b"%PDF-1.4 second draft").hexdigest() + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +def test_opal_pdf_upload_renamed_file_leaves_no_orphan(otis): + puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram") + otis.post_20x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": puzzle.pk, + "content": SimpleUploadedFile("old_name.pdf", b"%PDF-1.4 draft"), + }, + ) + puzzle.refresh_from_db() + old_name = puzzle.content.name + assert puzzle.content.storage.exists(old_name) + + otis.post_20x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": puzzle.pk, + "content": SimpleUploadedFile("tetrogram.pdf", b"%PDF-1.4 draft"), + }, + ) + puzzle.refresh_from_db() + assert puzzle.content.name.endswith("tetrogram.pdf") + assert not puzzle.content.storage.exists(old_name) + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +@pytest.mark.parametrize( + "upload", + ( + SimpleUploadedFile("tetrogram.tex", b"%PDF-1.4 not really"), + SimpleUploadedFile("tetrogram.pdf", b"\\documentclass{article}"), + ), + ids=("wrong extension", "wrong bytes"), +) +def test_opal_pdf_upload_rejects_non_pdf(otis, upload: SimpleUploadedFile): + puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram") + otis.post_40x( + "opal-pdf-upload", + data={"token": EXAMPLE_PASSWORD, "pk": puzzle.pk, "content": upload}, + ) + puzzle.refresh_from_db() + assert not puzzle.content + assert puzzle.content_hash == "" + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +def test_opal_pdf_upload_failed_auth(otis): + puzzle = OpalPuzzleFactory.create(hunt__slug="teammate", slug="tetrogram") + resp = otis.post_40x( + "opal-pdf-upload", + data={ + "token": "this wrong password is not a puzzle", + "pk": puzzle.pk, + "content": opal_pdf(b"first draft"), + }, + ) + assert resp.status_code == 418 + puzzle.refresh_from_db() + assert not puzzle.content + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +def test_opal_pdf_upload_unknown_puzzle(otis): + otis.post_not_found( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": 1729, + "content": opal_pdf(b"first draft"), + }, + ) + assert not OpalPuzzle.objects.exists() + + +@pytest.mark.django_db +@override_settings(API_TARGET_HASH=TARGET_HASH) +@pytest.mark.parametrize("pk", ("", "tetrogram"), ids=("empty", "not a number")) +def test_opal_pdf_upload_malformed_pk(otis, pk: str): + otis.post_40x( + "opal-pdf-upload", + data={ + "token": EXAMPLE_PASSWORD, + "pk": pk, + "content": opal_pdf(b"first draft"), + }, + ) diff --git a/aincrad/urls.py b/aincrad/urls.py index 857c004a2..55869d4aa 100644 --- a/aincrad/urls.py +++ b/aincrad/urls.py @@ -2,4 +2,7 @@ from . import views -urlpatterns = [path(r"api/", views.api, name="api")] +urlpatterns = [ + path(r"api/", views.api, name="api"), + path(r"api/opal-pdf/", views.opal_pdf_upload, name="opal-pdf-upload"), +] diff --git a/aincrad/views.py b/aincrad/views.py index 630d22d88..906ae5b31 100644 --- a/aincrad/views.py +++ b/aincrad/views.py @@ -1,5 +1,6 @@ import json import logging +import os import string from datetime import timedelta from decimal import Decimal @@ -10,6 +11,7 @@ from allauth.socialaccount.models import SocialAccount from django.conf import settings from django.core.exceptions import PermissionDenied, SuspiciousOperation +from django.core.files.uploadhandler import TemporaryFileUploadHandler from django.db.models.aggregates import Sum from django.db.models.query import QuerySet, prefetch_related_objects from django.db.models.query_utils import Q @@ -24,7 +26,7 @@ from arch.models import Hint, Problem from dashboard.models import Announcement, PSet from hanabi.models import HanabiContest, HanabiParticipation, HanabiPlayer, HanabiReplay -from opal.models import OpalPuzzle +from opal.models import OpalPuzzle, sha256_of from payments.models import Job from roster.models import ( ApplyUUID, @@ -666,6 +668,7 @@ def opal_handler(action: str, data: JSONData) -> JsonResponse: "order", "num_to_unlock", "content", + "content_hash", "is_metapuzzle", "answer", "partial_answers", @@ -677,6 +680,90 @@ def opal_handler(action: str, data: JSONData) -> JsonResponse: ) +@csrf_exempt +def opal_pdf_upload(request: HttpRequest) -> JsonResponse: + """Replace one OPAL puzzle's PDF, skipping the write if the bytes are unchanged. + + Separate from `api` because that view is JSON-only (`json.loads(request.body)`), + and one puzzle per request because that keeps a partial sync legible: the client + gets a per-file verdict instead of one opaque result for the whole batch. + """ + if request.method != "POST": + raise PermissionDenied("Must use POST") + + # Swap the upload handler before anything reads request.POST, which is what + # populates request.FILES. The project configures memory-only uploads capped at + # FILE_UPLOAD_MAX_MEMORY_SIZE, and that handler measures the whole request body: + # a PDF over the cap would be discarded with no error at all, leaving + # request.FILES empty while the sync script saw a 200. Streaming to disk instead + # takes the size limit out of the picture. Safe under @csrf_exempt, which stops + # CsrfViewMiddleware from touching request.POST first. + request.upload_handlers = [TemporaryFileUploadHandler(request)] + + if (bad_token := reject_bad_token(request.POST.get("token"))) is not None: + return bad_token + + # isdigit() rather than leaving it to the ORM, which raises ValueError (a 500) + # on a pk that is not a number at all. + pk = request.POST.get("pk", "") + if not pk.isdigit(): + raise SuspiciousOperation("No valid puzzle pk provided") + puzzle = get_object_or_404(OpalPuzzle, pk=int(pk)) + + upload = request.FILES.get("content") + if upload is None: + raise SuspiciousOperation("No file provided") + + filename = os.path.basename(upload.name or "") + if not filename.lower().endswith(".pdf"): + return JsonResponse( + {"error": f"{filename} is not named like a PDF"}, status=400 + ) + # Cheap guard against a sync pointed at the wrong directory, or at a PDF that + # never finished being written. The model only validates the extension. + header = upload.read(5) + upload.seek(0) + if header != b"%PDF-": + return JsonResponse({"error": f"{filename} is not a PDF"}, status=400) + + digest = sha256_of(upload) + # Same bytes under a new name still has to be written, since the storage key + # ends in the basename and the old one would otherwise stay the live file. + stored_name = os.path.basename(puzzle.content.name or "") if puzzle.content else "" + if digest == puzzle.content_hash and stored_name == filename: + return JsonResponse( + { + "status": "unchanged", + "pk": puzzle.pk, + "hunt": puzzle.hunt.slug, + "slug": puzzle.slug, + "content": puzzle.content.name, + "content_hash": digest, + } + ) + + status = "updated" if puzzle.content else "created" + if puzzle.content: + # Delete the old blob before writing. The storage key ends in the file's + # basename, so a renamed PDF would strand the old one; and locally + # FileSystemStorage would suffix the new name rather than overwrite it. + puzzle.content.delete(save=False) + puzzle.content.save(filename, upload, save=False) + puzzle.content_hash = digest + puzzle.save() + + return JsonResponse( + { + "status": status, + "pk": puzzle.pk, + "hunt": puzzle.hunt.slug, + "slug": puzzle.slug, + "content": puzzle.content.name, + "content_hash": digest, + } + ) + + def announcement_handler(action: str, data: JSONData) -> JsonResponse: del action announcement, is_new = Announcement.objects.update_or_create( @@ -698,6 +785,17 @@ def apply_handler(action: str, data: JSONData) -> JsonResponse: return JsonResponse({"pk": au.pk}) +def reject_bad_token(token: str | None) -> JsonResponse | None: + """Return the response to send if `token` is no good, or None if it checks out.""" + if token is None: + raise SuspiciousOperation("No token provided") + elif settings.API_TARGET_HASH is None: + return JsonResponse({"error": "Not accepting tokens right now"}, status=503) + elif sha256(token.encode("ascii")).hexdigest() != settings.API_TARGET_HASH: + return JsonResponse({"error": "🧋"}, status=418) + return None + + @csrf_exempt def api(request: HttpRequest) -> JsonResponse: if not request.method == "POST": @@ -711,12 +809,8 @@ def api(request: HttpRequest) -> JsonResponse: raise SuspiciousOperation("You need to provide an action, silly") action = data["action"] - if "token" not in data: - raise SuspiciousOperation("No token provided") - elif settings.API_TARGET_HASH is None: - return JsonResponse({"error": "Not accepting tokens right now"}, status=503) - elif sha256(data["token"].encode("ascii")).hexdigest() != settings.API_TARGET_HASH: - return JsonResponse({"error": "🧋"}, status=418) + if (bad_token := reject_bad_token(data.get("token"))) is not None: + return bad_token if action in ( "grade_problem_set", diff --git a/opal/admin.py b/opal/admin.py index be96801fc..cce9e6672 100644 --- a/opal/admin.py +++ b/opal/admin.py @@ -4,7 +4,7 @@ from django.forms import ModelForm from django.http import HttpRequest -from opal.models import OpalAttempt, OpalHunt, OpalPuzzle +from opal.models import OpalAttempt, OpalHunt, OpalPuzzle, sha256_of @admin.register(OpalHunt) @@ -47,6 +47,7 @@ class OpalPuzzleAdmin(admin.ModelAdmin): ("achievement", admin.EmptyFieldListFilter), ) search_fields = ("hunt__name", "slug", "title") + readonly_fields = ("content_hash",) def save_model( self, @@ -55,15 +56,19 @@ def save_model( form: ModelForm[OpalPuzzle], change: bool, ) -> None: - if "content" in form.changed_data and obj.content: - uploaded_name = os.path.basename(obj.content.name or "") - if uploaded_name != obj.slug + ".pdf": - messages.warning( - request, - f"The file {uploaded_name} does not match the slug {obj.slug}; " - "it was saved under that name anyway, but check you uploaded " - "the right file.", - ) + if "content" in form.changed_data: + # Keep the fingerprint in step with the bytes, or the bulk-sync API would + # compare a new PDF against a stale hash and decide nothing had changed. + obj.content_hash = sha256_of(obj.content) if obj.content else "" + if obj.content: + uploaded_name = os.path.basename(obj.content.name or "") + if uploaded_name != obj.slug + ".pdf": + messages.warning( + request, + f"The file {uploaded_name} does not match the slug {obj.slug}; " + "it was saved under that name anyway, but check you uploaded " + "the right file.", + ) super().save_model(request, obj, form, change) diff --git a/opal/migrations/0019_opalpuzzle_content_hash.py b/opal/migrations/0019_opalpuzzle_content_hash.py new file mode 100644 index 000000000..f9f03261b --- /dev/null +++ b/opal/migrations/0019_opalpuzzle_content_hash.py @@ -0,0 +1,22 @@ +# Generated by Django 6.0.8 on 2026-08-23 04:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("opal", "0018_opalhunt_artwork_slug"), + ] + + operations = [ + migrations.AddField( + model_name="opalpuzzle", + name="content_hash", + field=models.CharField( + blank=True, + default="", + help_text="SHA-256 of the puzzle file, so a bulk sync can skip PDFs whose bytes did not change. Blank means unknown, which makes the sync re-upload.", + max_length=64, + ), + ), + ] diff --git a/opal/models.py b/opal/models.py index 45342851a..2321004a7 100644 --- a/opal/models.py +++ b/opal/models.py @@ -1,10 +1,11 @@ import os import string -from hashlib import pbkdf2_hmac +from hashlib import pbkdf2_hmac, sha256 from typing import Any from django.conf import settings from django.contrib.auth.models import User +from django.core.files.base import File from django.core.validators import FileExtensionValidator from django.db import models from django.db.models.query import QuerySet @@ -25,6 +26,20 @@ def answerize(s: str) -> str: return "".join(c for c in s.upper() if c in ALLOWED_ANSWER_CHARACTERS) +def sha256_of(f: File[Any]) -> str: + """Fingerprint a file's bytes, so a sync can tell which PDFs actually changed. + + Reads in chunks rather than all at once, since these are PDFs and the caller + may be holding a handle to remote storage. ``File.chunks()`` rewinds first and + Django rewinds again before writing, so this is safe to call on a file that is + about to be saved. + """ + digest = sha256() + for chunk in f.chunks(): + digest.update(chunk) + return digest.hexdigest() + + class LiveOpalHuntManager(models.Manager): def get_queryset(self) -> QuerySet["OpalHunt"]: now = timezone.now() @@ -120,6 +135,13 @@ class OpalPuzzle(models.Model): null=True, blank=True, ) + content_hash = models.CharField( + max_length=64, + blank=True, + default="", + help_text="SHA-256 of the puzzle file, so a bulk sync can skip PDFs whose " + "bytes did not change. Blank means unknown, which makes the sync re-upload.", + ) guess_limit = models.PositiveSmallIntegerField( default=20, help_text="Maximum number of guesses to allow" diff --git a/otisweb/test_view_guards.py b/otisweb/test_view_guards.py index bd83a3245..cb3219521 100644 --- a/otisweb/test_view_guards.py +++ b/otisweb/test_view_guards.py @@ -72,6 +72,7 @@ # (url name, why) pair; adding to this list should be a conscious decision. PUBLIC_VIEWS: dict[str | None, str] = { "api": "aincrad API; authenticated by a hashed token in the POST body", + "opal-pdf-upload": "aincrad API; authenticated by a hashed token in the POST body", "certify": "capability URL; the checksum is verified before anything renders", "hanabi-contests": "public list of Hanabi contests", "hanabi-replays": "public results, gated on the contest being over",