Hardening A/2: close the open routes — decorator order, newsletter relay, request edit link, upload directories, hacker/team PII, deposit amount

CLAUDE.md

@@ -286,3 +286,17 @@ Gunicorn runs `--workers 2`, and every `TTLCache` in this codebase is per-proces
 ## CDN URL prefix contract (Sep 2026)
 
 `common/utils/cdn.py::cdn_server()` is the ONE way to get the public CDN origin: read at call time (not import), `rstrip("/")`, default `https://cdn.ohack.dev`. `upload_to_cdn` / `generate_signed_upload_url` build URLs with it and `api/submissions/submissions_service.py::_cdn_server` delegates to it, so a producer and a validator can never disagree on the prefix (the "Thumbnail: must be an ohack CDN URL under teams/<id>/" bug — a trailing slash or unset `CDN_SERVER` on one side). `_validate_own_cdn_image` logs the rejected URL + expected prefix at WARNING so the next mismatch is diagnosable from Render logs. `api/jobs` and `users_service` still carry their own `os.getenv("CDN_SERVER", …)` copies — migrate them to `cdn_server()` when touched.
+
+## Hardening stack (Sep 2026) — PR-A security
+
+Plan + evidence live in the frontend repo: `frontend-ohack.dev/docs/plans/hardening-security-seo-reliability-2026-09.md`. Every change below landed test-first; the tests are the contract.
+
+- **`@bp.route` must be the OUTERMOST decorator.** Two routes (`GET /hackathon/<event>/<type>/checkins`, `PATCH /api/problem-statements/events`) had `@auth.*` above `@bp.route`, so Flask registered the undecorated function and served volunteer PII (email, phone) to anyone. `test/common/test_view_decorator_order.py` AST-scans every `api/**/*_views.py` and fails the build on a repeat. Route-level proof uses the **rejecting** auth stub in `test/common/auth_stubs.py` (`rejecting_auth_module`: no `Authorization` → 401, no `X-Org-Id` → 403) — the pass-through stub can't detect a dropped decorator.
+- **Newsletter routes** are `volunteer.admin`-gated (they were an open Gmail relay). `POST /<subscribe>/<doc_id>` stays `require_user` on purpose (per-user self-service). The module imports `common.auth` like every other view (its own `init_auth` made it un-importable under test).
+- **Hackathon requests** (public capability-link flow, `/hack/request/<id>`): `update_hackathon_request` filters the body to `HACKATHON_REQUEST_EDITABLE_FIELDS` (= the frontend `HackathonRequestForm` `formData` keys; lockstep test in `api/messages/tests/test_hackathon_requests.py::FRONTEND_FORM_KEYS`), 404s before any email when the doc is missing, and confirms to the STORED `contactEmail` (never the body's). The public GET strips `adminNotes`. New ids are `uuid4`. Admin edits use the separate `admin_update_hackathon_request`.
+- **Uploads (`POST /api/messages/upload-image`)**: one gate, `api/submissions/submissions_service.py::authorize_upload_directory` — `teams/<id>/…` delegates to the #289 membership gate; non-admins may otherwise only write single-segment `USER_UPLOAD_DIRECTORIES` (`images hackers volunteers mentors judges sponsors uploads`) or `hackathons/<event>/planning/…` when `hackathon_planning_service.can_write_plan_for_event` says they edit that plan; everything else (site assets, event galleries, nonprofit logos, blog media) is admin-only (403 `directory_not_allowed`); `..`/odd characters → 400 `invalid_directory`. Non-admins can't overwrite: `upload_image_to_cdn(request, allow_overwrite=False)` answers 409 `file_exists` (multipart path; `cdn.blob_exists` — `upload_to_cdn` itself still overwrites because certificates/hearts/openai rely on it). `MAX_CONTENT_LENGTH` = 32 MiB with a JSON 413 (`api/exception_views.py`) because the app forces JSON content-type and frontend callers do `res.json()`.
+- **Hacker directory (`GET /api/hacker/applications/<event>`)** is `require_user` and projected to `HACKER_DIRECTORY_FIELDS` (`services/volunteers_service.py`) — every key `findteam.js` reads + `teamCode` + `isSelected` (peer votes). Add to the allowlist if the finder needs a new field; phone/deposit/sent_emails must never ship.
+- **Team payloads:** `services/teams_service.py::public_team_view` strips `PUBLIC_TEAM_STRIPPED_FIELDS` (`admin_notes nonprofit_rankings comments communication_history`) in `get_team`, `get_teams_list`, `get_teams_batch`, `get_single_hackathon_event`, `/me`, and (non-admins only) `GET /api/team/<hackathon_id>` whose member docs are also trimmed to `{id,user_id,name,nickname,profile_image}` (full user docs incl. email were shipping to any logged-in user). Admins keep full payloads there; the full single doc is `GET /api/team/admin/<teamid>` (`get_team_admin`), which the frontend's `adminTeamApi` prefers (404 → public fallback). `mentor_*` fields are public BY DESIGN — don't strip them.
+- **Deposit webhook**: `_handle_checkout_session_completed` compares `amount_total` with the event's `default_amount_cents` BEFORE the already-paid shortcut; short → `deposit_status="underpaid"` + Slack audit, never `paid`; missing config/lookup error fails open. Best-effort only (the submit path still accepts deposit fields by design); PaymentIntent verification is the follow-up.
+- Shared-secret headers (`BACKEND_NEWS_TOKEN`, `BACKEND_PRAISE_TOKEN`, store `X-Webhook-Secret`) compare with `hmac.compare_digest` and never match when the env var is unset. `GET /news?limit=` → 400 on non-int, clamped to 1..200.
+- **Running tests locally:** `ENVIRONMENT=test SLACK_WEBHOOK= SLACK_BOT_TOKEN= RESEND_API_KEY= FIREBASE_CERT_CONFIG='<structurally valid throwaway service-account JSON>' python -m pytest <dir>` — Slack vars EMPTY (dummies make `send_slack_audit` raise on `requests.post`), and `common/utils/firebase.py` builds `credentials.Certificate` at import, so `.env.example`'s placeholder cert blocks collection of almost every suite. `api/messages/tests` files run individually. `api/certificates/tests` hangs on network — exclude it from gated runs.

api/__init__.py

@@ -98,6 +98,10 @@ def create_app():
     ##########################################
 
     app = Flask(__name__, instance_relative_config=True)
+    # Hard cap on request bodies (the largest legitimate upload is a 10 MB
+    # planning attachment; videos go straight to GCS via signed URLs).
+    # Over-limit requests get the JSON 413 from api/exception_views.py.
+    app.config["MAX_CONTENT_LENGTH"] = 32 * 1024 * 1024
     logger.info("Started Flask")
 
 

api/exception_views.py

@@ -21,3 +21,13 @@ def _handle_not_found_error(ex):
         return {"message": "Not Found"}, ex.code
     else:
         return ex
+
+
+@bp.app_errorhandler(exceptions.RequestEntityTooLarge)
+def _handle_payload_too_large(ex):
+    # The app forces a JSON content-type on every response, so werkzeug's HTML
+    # 413 page breaks frontend res.json() callers — answer in JSON on /api/.
+    if request.path.startswith('/api/'):
+        from flask import current_app
+        return {"error": "payload_too_large", "max_bytes": current_app.config.get("MAX_CONTENT_LENGTH")}, 413
+    return ex

api/messages/messages_service.py

@@ -438,17 +438,20 @@ def get_user_by_id_old(id):
     return get_profile_by_db_id(id) or {}
 
 
-def upload_image_to_cdn(request):
+def upload_image_to_cdn(request, allow_overwrite=True):
     """
     Upload an image to CDN. Accepts binary data, base64, or standard image formats.
     Returns the CDN URL of the uploaded image.
+
+    allow_overwrite=False (non-admin uploads) makes the multipart path answer
+    409 file_exists instead of replacing an existing blob.
     """
     import base64
     import tempfile
     import mimetypes
     from werkzeug.utils import secure_filename
-    from common.utils.cdn import upload_to_cdn
-    
+    from common.utils import cdn
+
     logger.info("Starting image upload to CDN")
 
     try:
@@ -473,6 +476,11 @@ def upload_image_to_cdn(request):
             if not _is_image_file(filename):
                 logger.warning(f"Upload failed: File is not an image: {filename}")
                 return {"success": False,"error": "File must be an image"}, 400
+
+            # temp basename == filename, so this is the exact blob upload_to_cdn writes.
+            if not allow_overwrite and cdn.blob_exists(_directory, filename):
+                logger.warning(f"Upload refused: {_directory}/{filename} already exists")
+                return {"success": False, "error": "file_exists"}, 409
 
             # Create a properly named temporary file
             import tempfile
@@ -490,7 +498,7 @@ def upload_image_to_cdn(request):
             try:
                 # Upload to CDN using the properly named temp file
                 logger.info(f"Uploading {filename} to CDN from {temp_filepath}")
-                cdn_url = upload_to_cdn(_directory, temp_filepath, destination_filename)
+                cdn_url = cdn.upload_to_cdn(_directory, temp_filepath, destination_filename)
 
                 logger.info(f"Successfully uploaded image to CDN: {cdn_url}")
                 return {"success": True, "url": cdn_url, "message": "Image uploaded successfully"}
@@ -547,7 +555,7 @@ def upload_image_to_cdn(request):
                 try:
                     # Upload to CDN using the properly named temp file
                     logger.info(f"Uploading base64 image {filename} to CDN from {temp_filepath}")
-                    cdn_url = upload_to_cdn("images", temp_filepath, destination_filename)
+                    cdn_url = cdn.upload_to_cdn("images", temp_filepath, destination_filename)
 
                     logger.info(f"Successfully uploaded base64 image to CDN: {cdn_url}")
                     return {"success": True, "url": cdn_url, "message": "Image uploaded successfully"}
@@ -593,7 +601,7 @@ def upload_image_to_cdn(request):
                 try:
                     # Upload to CDN using the properly named temp file
                     logger.info(f"Uploading binary image {filename} to CDN from {temp_filepath}")
-                    cdn_url = upload_to_cdn("images", temp_filepath, destination_filename)
+                    cdn_url = cdn.upload_to_cdn("images", temp_filepath, destination_filename)
 
                     logger.info(f"Successfully uploaded binary image to CDN: {cdn_url}")
                     return {"success": True, "url": cdn_url, "message": "Image uploaded successfully"}
@@ -632,7 +640,7 @@ def upload_image_to_cdn(request):
             try:
                 # Upload to CDN using the properly named temp file
                 logger.info(f"Uploading raw image {filename} to CDN from {temp_filepath}")
-                cdn_url = upload_to_cdn("images", temp_filepath, destination_filename)
+                cdn_url = cdn.upload_to_cdn("images", temp_filepath, destination_filename)
 
                 logger.info(f"Successfully uploaded raw image to CDN: {cdn_url}")
                 return {

api/messages/messages_views.py

@@ -1,12 +1,14 @@
-import os 
+import os
+import hmac
 from common.log import get_logger, debug, error
 import json
 from common.auth import auth, auth_user
 
 from flask import (
     Blueprint,
     request,
-    g
+    g,
+    jsonify
 )
 
 from api.messages.messages_service import (
@@ -91,6 +93,25 @@
     # Get the org_id from the req
     return req.headers.get("X-Org-Id")
 
+NEWS_LIMIT_MAX = 200
+
+# Constant responses for POST /upload-image directory denials (see
+# api/submissions/submissions_service.py::authorize_upload_directory).
+UPLOAD_DENIAL_RESPONSES = {
+    "invalid_directory": ({"error": "invalid_directory"}, 400),
+    "not_team_member": ({"error": "not_team_member"}, 403),
+    "directory_not_allowed": ({"error": "directory_not_allowed"}, 403),
+    "forbidden": ({"error": "forbidden"}, 403),
+}
+
+
+def _api_key_matches(provided, expected):
+    """Constant-time X-Api-Key check; an unset env secret never matches."""
+    if not expected or not provided:
+        return False
+    return hmac.compare_digest(str(expected), str(provided))
+
+
 def get_authenticated_user_id():
     """Helper function to get authenticated user ID with proper error handling"""
     if not auth_user or not auth_user.user_id:
@@ -287,9 +308,9 @@
     if auth_user and auth_user.user_id:
         return vars(single_add_volunteer(event_id, request.get_json(), "hacker", auth_user.user_id))
 
+@bp.route("/hackathon/<event_id>/<volunteer_type>/checkins", methods=["GET"])
 @auth.require_user
 @auth.require_org_member_with_permission("volunteer.admin", req_to_org_id=getOrgId)
-@bp.route("/hackathon/<event_id>/<volunteer_type>/checkins", methods=["GET"])
 def get_volunteers_checked_in_by_event_api(event_id, volunteer_type):
     logger.info(f"GET /hackathon/{event_id}/{volunteer_type}/checked_in called")
     return (get_volunteer_checked_in_by_event(event_id, volunteer_type))
@@ -491,11 +512,9 @@
     # if token is valid, store news
     # else return 401
     token = request.headers.get("X-Api-Key")
-    # Check BACKEND_NEWS_TOKEN
-    if token == None or token != os.getenv("BACKEND_NEWS_TOKEN"):
+    if not _api_key_matches(token, os.getenv("BACKEND_NEWS_TOKEN")):
         return "Unauthorized", 401
-    else:
-        return vars(save_news(request.get_json()))
+    return vars(save_news(request.get_json()))
 
 @bp.route("/news", methods=["GET"])
 def read_news():
@@ -504,10 +523,13 @@
     # Log
     logger.info(f"Processing problem statements list with limit: {limit_arg}")
 
-    # If limit is set, convert to int
-    limit=3
+    limit = 3
     if limit_arg:
-        limit = int(limit_arg)
+        try:
+            limit = int(limit_arg)
+        except (TypeError, ValueError):
+            return {"error": "invalid_limit"}, 400
+        limit = max(1, min(limit, NEWS_LIMIT_MAX))
 
     return vars(get_news(news_limit=limit, news_id=None))  # Pass the 'limit' parameter to the get_news() function
 
@@ -598,8 +620,7 @@
     sender_id = json_data.get("praise_sender")
     receiver_id = json_data.get("praise_receiver")
 
-    # Check BACKEND_NEWS_TOKEN
-    if token == None or token != os.getenv("BACKEND_PRAISE_TOKEN"):
+    if not _api_key_matches(token, os.getenv("BACKEND_PRAISE_TOKEN")):
         return "Unauthorized", 401
     elif sender_id == receiver_id:
         return "You cannot write a praise about yourself", 400
@@ -798,12 +819,18 @@
 @bp.route("/create-hackathon/<request_id>", methods=["GET"])
 def get_submitted_hackathon(request_id):
     logger.info(f"GET /create-hackathon/{request_id} called")
-    return get_hackathon_request_by_id(request_id)
+    result = get_hackathon_request_by_id(request_id)
+    if result is None:
+        return {"error": "not_found"}, 404
+    return result
 
 @bp.route("/create-hackathon/<request_id>", methods=["PATCH"])
 def update_submitted_hackathon(request_id):
     logger.info(f"PATCH /create-hackathon/{request_id} called")
-    return update_hackathon_request(request_id, request.get_json())
+    result = update_hackathon_request(request_id, request.get_json(silent=True))
+    if result is None:
+        return {"error": "not_found"}, 404
+    return result
 
 
 @bp.route("/upload-image", methods=["POST"])
@@ -817,18 +844,29 @@
     from api.messages.messages_service import upload_image_to_cdn
 
     if auth_user and auth_user.user_id:
-        # teams/<id>/... directories are only writable by that team's members
-        # (or an admin) — the team-project thumbnail validator trusts that
-        # prefix. Everything else is unchanged.
-        from services.hackathon_planning_service import is_admin
-        from api.submissions.submissions_service import authorize_team_upload_directory
-
-        blocked = authorize_team_upload_directory(
-            auth_user.user_id, request.form.get("directory"), admin=is_admin(auth_user)
+        # teams/<id>/... is member-only (the project-thumbnail validator
+        # trusts that prefix); other shared directories are admin-only and
+        # non-admins may never overwrite an existing file.
+        from services import hackathon_planning_service as planning
+        from api.submissions.submissions_service import authorize_upload_directory
+
+        admin = bool(planning.is_admin(auth_user))
+        blocked = authorize_upload_directory(
+            auth_user.user_id,
+            request.form.get("directory"),
+            admin=admin,
+            # planning editors (non-admin) may attach files to their event's cards
+            plan_editor_check=lambda event_id: planning.can_write_plan_for_event(auth_user, event_id),
         )
         if blocked:
-            return blocked
-        return upload_image_to_cdn(request)
+            # The gate's payloads are constant, but static analysis follows the
+            # `directory` argument into its return value — answer from a module
+            # constant keyed by the error code so no request data can reach the
+            # response.
+            code = (blocked[0] or {}).get("error")
+            payload, status = UPLOAD_DENIAL_RESPONSES.get(code, UPLOAD_DENIAL_RESPONSES["forbidden"])
+            return jsonify(payload), status
+        return upload_image_to_cdn(request, allow_overwrite=admin)
     else:
         error(logger, "Could not obtain user details for POST /upload-image")
         return {"error": "Unauthorized"}, 401