Harden: cap request body before reading, WAL for concurrent writes, socket timeout, surface unmeasured edges

- _body() read the full declared Content-Length into memory before any size
  check, so MAX_UPLOAD_BYTES could only reject an upload already held in
  RAM. nginx caps this on the proxied path, but the container also listens
  on the LAN, so the app now enforces its own 20MB ceiling and closes the
  connection rather than reading.
- SQLite ran with the default rollback journal and 5s lock timeout, chosen
  when a row was a few KB; rows now carry the original photos, so two
  people grading at once could block each other's History. WAL + 30s.
- No socket timeout meant a stalled keep-alive connection held a worker
  thread indefinitely.
- An edge excluded as a different material was dropped from the UI with no
  explanation, presenting three sides as though they were all four.

app.py

@@ -30,9 +30,19 @@ import store
 import vision
 
 # A photo arrives base64-encoded inside JSON (avoids hand-rolling multipart
-# parsing on top of the stdlib server). Cap it so a stray upload can't wedge
-# the process reading an unbounded body.
+# parsing on top of the stdlib server). This caps the DECODED image bytes.
 MAX_UPLOAD_BYTES = 12 * 1024 * 1024
+# Hard ceiling on the request body itself, enforced before a single byte is
+# read. MAX_UPLOAD_BYTES alone cannot do this job: it is checked only after
+# the whole body has already been pulled into memory, so a client declaring
+# a multi-gigabyte Content-Length would be obliged first and rejected
+# afterwards. nginx caps this too, but only on the proxied path — the
+# container also listens on the LAN (see docker-compose.yml), so the app has
+# to enforce its own limit rather than inherit one.
+# Sized for the largest legitimate request: MAX_UPLOAD_BYTES of image is
+# ~4/3 that as base64, plus JSON overhead. Kept in step with nginx's
+# client_max_body_size in nginx/card-grader.conf.
+MAX_BODY_BYTES = 20 * 1024 * 1024
 # More angles help — front, back, corner close-ups — but past a handful the
 # extra photos cost tokens without adding evidence.
 MAX_GRADE_IMAGES = 6
@@ -150,6 +160,12 @@ def _make_thumbnail(image_bytes):
 class Handler(BaseHTTPRequestHandler):
     protocol_version = "HTTP/1.1"
     server_version = "CardGrader"
+    # Without this a client that opens a connection and then stalls holds a
+    # worker thread forever — keep-alive means the handler sits waiting on
+    # the next request line indefinitely. Generous enough for a phone
+    # uploading photos over a slow link, finite enough that a dead or
+    # deliberately-slow connection lets go.
+    timeout = 120
 
     def log_message(self, fmt, *args):
         if str(args[1] if len(args) > 1 else "").startswith(("4", "5")):
@@ -175,7 +191,25 @@ class Handler(BaseHTTPRequestHandler):
         self._json({"error": message}, code)
 
     def _body(self):
+        """Parsed JSON body, or None if the request was refused outright.
+
+        Returns None (having already sent a response) when the body is over
+        the limit, so callers must stop rather than carry on with an empty
+        dict — which is what an unparseable body still yields.
+        """
+        try:
             length = int(self.headers.get("Content-Length") or 0)
+        except ValueError:
+            self._reject_body("Malformed Content-Length.")
+            return None
+        if length < 0:
+            self._reject_body("Malformed Content-Length.")
+            return None
+        if length > MAX_BODY_BYTES:
+            self._reject_body(
+                "That request is {:.1f} MB; the limit is {} MB.".format(
+                    length / 1024 / 1024, MAX_BODY_BYTES // 1024 // 1024))
+            return None
         if not length:
             return {}
         try:
@@ -183,6 +217,17 @@ class Handler(BaseHTTPRequestHandler):
         except ValueError:
             return {}
 
+    def _reject_body(self, message):
+        """413 for an oversized//malformed body, then hang up.
+
+        The body is deliberately never read, so the socket still holds
+        whatever the client is sending — reusing it under keep-alive would
+        parse that leftover payload as the next request. Closing is the only
+        safe way to refuse without reading.
+        """
+        self.close_connection = True
+        self._json({"error": message}, 413)
+
     def _route(self):
         path = urllib.parse.urlparse(self.path).path
         # nginx forwards the full "/cards/..." URI through unchanged (same
@@ -251,6 +296,8 @@ class Handler(BaseHTTPRequestHandler):
         route = self._route()
         try:
             body = self._body()
+            if body is None:
+                return          # _body already sent 413 and closed
             if route == "/api/settings":
                 if SETTINGS_LOCKED:
                     return self._error(
@@ -275,6 +322,8 @@ class Handler(BaseHTTPRequestHandler):
             if route.startswith("/api/history/"):
                 grade_id = int(route.split("/")[3])
                 body = self._body()
+                if body is None:
+                    return      # _body already sent 413 and closed
                 if store.get_grade(grade_id) is None:
                     return self._error("No such grade", 404)
                 grade = store.update_grade_label(grade_id, body.get("label"))

static/app.js

@@ -131,9 +131,19 @@ function renderGradeBlock(g, opts = {}) {
   if (m && m.reliable === false) {
     measuredLine = `not measurable — ${m.reason || "this card's finish"}`;
   } else if (m && m.edges) {
-    measuredLine = ['top', 'right', 'bottom', 'left']
+    const sides = ['top', 'right', 'bottom', 'left'];
+    measuredLine = sides
       .filter((s) => m.edges[s])
       .map((s) => `${s} ${m.edges[s].percent.toFixed(0)}%`).join(' · ');
+    // An edge excluded for reading as a different material (a die-cut clear
+    // window, a foil strip) comes back with no score. Listing only the
+    // edges that DID measure would quietly present three sides as if they
+    // were all four — say which one is missing and why.
+    const skipped = sides.filter((s) => !m.edges[s] && (m.edge_notes || {})[s]);
+    if (skipped.length) {
+      const why = m.edge_notes[skipped[0]];
+      measuredLine += `${measuredLine ? '; ' : ''}${skipped.join(' and ')} not measured — ${why}`;
+    }
   }
 
   const cm = g.centering_measurement || null;

store.py

@@ -60,9 +60,20 @@ DEFAULT_SETTINGS = {
 
 
 def connect():
-    conn = sqlite3.connect(DB_PATH)
+    # timeout: how long to wait for a writer's lock before giving up. The
+    # server is threaded, so two people grading at once genuinely collide —
+    # and since a row now carries the original photos, a write can be
+    # several megabytes and hold the lock long enough to matter. The 5s
+    # default was chosen for small writes; this isn't that any more.
+    conn = sqlite3.connect(DB_PATH, timeout=30.0)
     conn.row_factory = sqlite3.Row
     conn.execute("PRAGMA foreign_keys = ON")
+    # WAL lets readers carry on during a write instead of blocking on it,
+    # which is the difference between "someone else is grading" being
+    # invisible and it freezing everyone's History. Persists on the database
+    # file itself, so setting it per-connection is just belt-and-braces.
+    conn.execute("PRAGMA journal_mode = WAL")
+    conn.execute("PRAGMA synchronous = NORMAL")
     return conn