Harden summary-PDF endpoint per cross-review

Address GPT-4.1 cross-review of the IAM change:

- Validate the week param (YYYY-WNN) and the DynamoDB-sourced PDF key
  shape before presigning, so a tampered SUMMARY record can't mint URLs
  for other report files (payroll CSVs).
- Narrow the IAM resource from reports/* to
  reports/*/weekly-summary-*.pdf — least privilege over the bucket.
- Reuse a module-level S3 client; name the URL TTL constant.
- Distinguish "week not found" from "PDF key missing" 404s.
- Tests: malformed-week 400 and tampered-key 500 (asserts no presign).

functions/submit_order/handler.py

@@ -1,6 +1,7 @@
 import json
 import logging
 import os
+import re
 import sys
 import time
 import urllib.error
@@ -34,6 +35,7 @@ if not logger.handlers:
 
 EASTERN = ZoneInfo("America/New_York")
 CACHE_TTL_SECONDS = 300  # 5-minute TTL for cached config values
+PRESIGNED_URL_TTL_SECONDS = 300  # summary-PDF presigned URL lifetime
 ALLOWED_DOMAINS = {"seahavenind.com", "seahaven.com"}
 
 
@@ -48,6 +50,7 @@ _settings_ts = 0.0
 _google_client_id = None
 _google_client_id_ts = 0.0
 _lambda = boto3.client("lambda")
+_s3 = boto3.client("s3")
 
 
 def _get_api_key() -> str:
@@ -262,18 +265,28 @@ def handle_admin_summary_pdf(event):
 
     qs = event.get("queryStringParameters") or {}
     week = qs.get("week", "")
-    if not week:
-        return response(400, {"error": "week query param is required"})
+    if not re.match(r"^\d{4}-W\d{2}$", week):
+        return response(400, {"error": "week query param must be YYYY-WNN"})
 
     summary = get_summary(week)
-    pdf_key = (summary or {}).get("weekly_summary_pdf_s3_key")
-    if not pdf_key:
+    if summary is None:
+        return response(404, {"error": "No summary PDF: week not found or not closed"})
+    pdf_key = summary.get("weekly_summary_pdf_s3_key", "")
+    # The key comes from a DynamoDB record; only presign keys matching the
+    # shape aggregate_orders writes, so a tampered record can't expose other
+    # report files (e.g. payroll CSVs).
+    if not re.match(
+        r"^reports/\d{4}-W\d{2}/weekly-summary-\d{4}-W\d{2}\.pdf$", pdf_key
+    ):
+        if pdf_key:
+            logger.error("Unexpected summary PDF key shape for %s: %s", week, pdf_key)
+            return response(500, {"error": "Internal error"})
         return response(404, {"error": "No summary PDF available for this week"})
 
-    url = boto3.client("s3").generate_presigned_url(
+    url = _s3.generate_presigned_url(
         "get_object",
         Params={"Bucket": os.environ["REPORTS_BUCKET"], "Key": pdf_key},
-        ExpiresIn=300,
+        ExpiresIn=PRESIGNED_URL_TTL_SECONDS,
     )
     logger.info("Admin %s requested summary PDF for %s", user["email"], week)
     return response(200, {"week": week, "url": url})

template.yaml

@@ -271,11 +271,12 @@ Resources:
             - Effect: Allow
               Action: ssm:GetParameter
               Resource: !Sub 'arn:aws:ssm:${AWS::Region}:${AWS::AccountId}:parameter/meal-order-manager/*'
-            # Read-only access to weekly report PDFs for the admin
-            # summary-pdf presigned-URL endpoint.
+            # Read-only access to weekly summary PDFs (only — not the
+            # payroll/order CSVs) for the admin summary-pdf presigned-URL
+            # endpoint.
             - Effect: Allow
               Action: s3:GetObject
-              Resource: !Sub '${ReportsBucket.Arn}/reports/*'
+              Resource: !Sub '${ReportsBucket.Arn}/reports/*/weekly-summary-*.pdf'
       Events:
         SubmitOrder:
           Type: HttpApi

tests/test_submit_order.py

@@ -1600,14 +1600,14 @@ def _pdf_event(week=None):
 
 
 @patch.dict(os.environ, {"REPORTS_BUCKET": "test-reports-bucket"})
-@patch("submit_order_handler.boto3.client")
+@patch("submit_order_handler._s3")
 @patch("submit_order_handler.get_summary")
 @patch(
     "submit_order_handler._verify_admin",
     return_value=({"email": "adam@seahavenind.com"}, None),
 )
 def test_admin_summary_pdf_returns_presigned_url(
-    mock_verify, mock_get_summary, mock_boto_client
+    mock_verify, mock_get_summary, mock_s3
 ):
     """Closed week with a stamped PDF key returns a presigned URL."""
     from submit_order_handler import lambda_handler
@@ -1615,9 +1615,7 @@ def test_admin_summary_pdf_returns_presigned_url(
     mock_get_summary.return_value = {
         "weekly_summary_pdf_s3_key": "reports/2026-W22/weekly-summary-2026-W22.pdf"
     }
-    mock_s3 = MagicMock()
     mock_s3.generate_presigned_url.return_value = "https://signed.example/pdf"
-    mock_boto_client.return_value = mock_s3
 
     status, body = _parse_response(lambda_handler(_pdf_event("2026-W22"), None))
 
@@ -1662,6 +1660,39 @@ def test_admin_summary_pdf_400_without_week(mock_verify):
     assert status == 400, f"Expected 400, got {status}: {body}"
 
 
+@patch(
+    "submit_order_handler._verify_admin",
+    return_value=({"email": "adam@seahavenind.com"}, None),
+)
+def test_admin_summary_pdf_400_malformed_week(mock_verify):
+    """Week not matching YYYY-WNN returns 400 before any lookup."""
+    from submit_order_handler import lambda_handler
+
+    status, body = _parse_response(lambda_handler(_pdf_event("../../etc"), None))
+
+    assert status == 400, f"Expected 400, got {status}: {body}"
+
+
+@patch("submit_order_handler._s3")
+@patch("submit_order_handler.get_summary")
+@patch(
+    "submit_order_handler._verify_admin",
+    return_value=({"email": "adam@seahavenind.com"}, None),
+)
+def test_admin_summary_pdf_rejects_tampered_key(mock_verify, mock_get_summary, mock_s3):
+    """A PDF key outside the expected shape is never presigned (500, no URL)."""
+    from submit_order_handler import lambda_handler
+
+    mock_get_summary.return_value = {
+        "weekly_summary_pdf_s3_key": "reports/2026-W22/payroll-deductions.csv"
+    }
+
+    status, body = _parse_response(lambda_handler(_pdf_event("2026-W22"), None))
+
+    assert status == 500, f"Expected 500 for tampered key, got {status}: {body}"
+    mock_s3.generate_presigned_url.assert_not_called()
+
+
 @patch(
     "submit_order_handler._verify_admin",
     return_value=(None, submit_order_handler.response(401, {"error": "Missing token"})),