Merge pull request #220 from SaplingLearn/security/199-newsletter-careers-bounds

fix(security): newsletter exception leak + careers upload bounds (#199)

Author: Jose-Gael-Cruz-Lopez

backend/routes/careers.py

@@ -1,21 +1,52 @@
+import re
 import uuid
 
 import httpx
-from fastapi import APIRouter, UploadFile, File, Form
+from fastapi import APIRouter, UploadFile, File, Form, HTTPException
 from typing import Optional
 
 from db.connection import SUPABASE_URL, SUPABASE_KEY, table
 
 BUCKET = "application_resumes"
 
+# /apply is intentionally public (anyone can apply), so the upload path must be
+# bounded — otherwise it's an unauthenticated, unbounded upload + DB-write sink
+# (#199). Cap size and restrict to document types, mirroring storage_service's
+# avatar validation (415 unsupported / 413 too large).
+MAX_RESUME_SIZE = 5 * 1024 * 1024  # 5 MB
+ALLOWED_RESUME_TYPES = {
+    "application/pdf",
+    "application/msword",
+    "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
+}
+_EMAIL_RE = re.compile(r"^[^@\s]+@[^@\s]+\.[^@\s]+$")
+
 router = APIRouter()
 
 
-def _upload_resume(file: UploadFile) -> str:
-    """Upload PDF to Supabase Storage and return the storage path."""
-    ext = file.filename.rsplit(".", 1)[-1] if "." in file.filename else "pdf"
+def _validate_resume(file: UploadFile) -> bytes:
+    """Enforce content-type + size bounds and return the file bytes.
+
+    Reads at most MAX_RESUME_SIZE + 1 bytes so an oversize upload can't be
+    pulled fully into memory before we reject it.
+    """
+    if (file.content_type or "") not in ALLOWED_RESUME_TYPES:
+        raise HTTPException(
+            status_code=415,
+            detail="Unsupported resume type. Allowed: PDF, DOC, DOCX.",
+        )
+    content = file.file.read(MAX_RESUME_SIZE + 1)
+    if len(content) > MAX_RESUME_SIZE:
+        raise HTTPException(status_code=413, detail="Resume too large. Maximum size is 5 MB.")
+    if not content:
+        raise HTTPException(status_code=400, detail="Resume file is empty.")
+    return content
+
+
+def _upload_resume(file: UploadFile, content: bytes) -> str:
+    """Upload a validated resume to Supabase Storage and return the path."""
+    ext = file.filename.rsplit(".", 1)[-1] if file.filename and "." in file.filename else "pdf"
     path = f"{uuid.uuid4()}.{ext}"
-    content = file.file.read()
     url = f"{SUPABASE_URL}/storage/v1/object/{BUCKET}/{path}"
     r = httpx.put(
         url,
@@ -40,14 +71,31 @@ async def apply(
     portfolio_link: str = Form(""),
     resume: Optional[UploadFile] = File(None),
 ):
-    resume_path = _upload_resume(resume) if resume else None
+    position = position.strip()
+    full_name = full_name.strip()
+    email = email.strip()
+    linkedin_url = linkedin_url.strip()
+    if not position or len(position) > 200:
+        raise HTTPException(status_code=422, detail="A valid position is required.")
+    if not full_name or len(full_name) > 200:
+        raise HTTPException(status_code=422, detail="Full name is required.")
+    if not _EMAIL_RE.match(email) or len(email) > 320:
+        raise HTTPException(status_code=422, detail="A valid email is required.")
+    if not linkedin_url or len(linkedin_url) > 500:
+        raise HTTPException(status_code=422, detail="A LinkedIn URL is required.")
+
+    resume_path = None
+    if resume is not None and resume.filename:
+        content = _validate_resume(resume)
+        resume_path = _upload_resume(resume, content)
+
     row = table("job_applications").insert({
         "position": position,
         "full_name": full_name,
         "email": email,
-        "phone": phone or None,
+        "phone": (phone or "").strip() or None,
         "linkedin_url": linkedin_url,
-        "portfolio_link": portfolio_link or None,
+        "portfolio_link": (portfolio_link or "").strip() or None,
         "resume": resume_path,
     })
     return {"ok": True, "id": row[0]["id"] if row else None}

backend/routes/newsletter.py

@@ -1,7 +1,11 @@
+import logging
+
 from fastapi import APIRouter, HTTPException
 from pydantic import BaseModel, EmailStr, field_validator
 from db.connection import table
 
+logger = logging.getLogger("newsletter")
+
 router = APIRouter()
 
 
@@ -24,6 +28,13 @@ def subscribe(body: SubscribeRequest):
             {"email": body.email},
             on_conflict="email",
         )
-    except Exception as e:
-        raise HTTPException(status_code=500, detail=str(e))
+    except Exception:
+        # #199: never echo the raw DB/PostgREST exception text to the client —
+        # it leaks internal detail (host/table/driver). Log the full error
+        # server-side and return a generic message.
+        logger.exception("newsletter subscribe failed")
+        raise HTTPException(
+            status_code=500,
+            detail="Could not process subscription. Please try again later.",
+        )
     return {"ok": True}

backend/tests/test_newsletter_careers_bounds.py

@@ -0,0 +1,87 @@
+"""
+Regression tests for #199:
+- newsletter.subscribe must not leak raw exception text to the client.
+- careers.apply must bound resume size/type and validate basic inputs.
+
+These assert the security properties that fail on pre-fix code (raw exception
+text echoed back; unbounded/untyped uploads accepted).
+"""
+from unittest.mock import patch
+
+from fastapi.testclient import TestClient
+
+from main import app
+
+client = TestClient(app)
+
+INTERNAL = "SECRET_DB_INTERNALS: connection refused at 10.0.0.5 table=newsletter_emails"
+
+# Minimal valid form for /api/careers/apply.
+VALID_FORM = {
+    "position": "Software Engineer",
+    "full_name": "Ada Lovelace",
+    "email": "ada@example.com",
+    "linkedin_url": "https://www.linkedin.com/in/ada",
+}
+
+
+class TestNewsletterErrorLeak:
+    def test_db_error_returns_generic_message_not_raw_exception(self):
+        with patch("routes.newsletter.table") as t:
+            t.return_value.upsert.side_effect = Exception(INTERNAL)
+            r = client.post("/api/newsletter/subscribe", json={"email": "x@example.com"})
+
+        assert r.status_code == 500
+        detail = r.json().get("detail", "")
+        # Pre-fix: detail == str(e) and contains INTERNAL. Post-fix: generic.
+        assert INTERNAL not in detail
+        assert "10.0.0.5" not in detail
+        assert detail == "Could not process subscription. Please try again later."
+
+
+class TestCareersUploadBounds:
+    def test_oversize_resume_rejected_413(self):
+        big = b"%PDF-" + b"0" * (5 * 1024 * 1024 + 1)
+        with patch("routes.careers.httpx") as hx, patch("routes.careers.table") as t:
+            t.return_value.insert.return_value = [{"id": "app1"}]
+            r = client.post(
+                "/api/careers/apply",
+                data=VALID_FORM,
+                files={"resume": ("resume.pdf", big, "application/pdf")},
+            )
+        # Pre-fix accepted it (200, unbounded read + upload); fix returns 413
+        # and never reaches the storage upload.
+        assert r.status_code == 413
+        hx.put.assert_not_called()
+
+    def test_wrong_content_type_rejected_415(self):
+        with patch("routes.careers.httpx") as hx, patch("routes.careers.table") as t:
+            t.return_value.insert.return_value = [{"id": "app1"}]
+            r = client.post(
+                "/api/careers/apply",
+                data=VALID_FORM,
+                files={"resume": ("resume.exe", b"MZ\x90\x00", "application/octet-stream")},
+            )
+        assert r.status_code == 415
+        hx.put.assert_not_called()
+
+    def test_invalid_email_rejected_422(self):
+        with patch("routes.careers.httpx"), patch("routes.careers.table") as t:
+            t.return_value.insert.return_value = [{"id": "app1"}]
+            r = client.post(
+                "/api/careers/apply",
+                data={**VALID_FORM, "email": "not-an-email"},
+            )
+        assert r.status_code == 422
+
+    def test_valid_application_succeeds(self):
+        with patch("routes.careers.httpx") as hx, patch("routes.careers.table") as t:
+            hx.put.return_value.raise_for_status.return_value = None
+            t.return_value.insert.return_value = [{"id": "app1"}]
+            r = client.post(
+                "/api/careers/apply",
+                data=VALID_FORM,
+                files={"resume": ("resume.pdf", b"%PDF-1.4 small", "application/pdf")},
+            )
+        assert r.status_code == 200
+        assert r.json() == {"ok": True, "id": "app1"}