fix(security): phone-verify customer PII endpoints + rate-limit (P0)#208
Conversation
Live-site security review (Mark Willcott). Customer-facing endpoints exposed PII
to anyone who knew/guessed an opaque id — a deal_id/appointment_id is a
capability, not a credential.
- /api/v1/customer/deal/{id} + /download/{note_id}: a deal_id alone returned the
buyer's name + document list and minted a 15-min signed URL to the closing PDF
(SSN/DOB) in GCS. Now require phone-last-4 verification against the deal's
buyer/co-buyer phone BEFORE any data or signed URL; non-existent deal and wrong
phone return the SAME 403 (no existence oracle). Rate-limited 20/min. Fixed the
bare 'except Exception' that was converting HTTPExceptions into 500s.
- /api/appointments/{id}: returned name/phone/email unauthenticated; now requires
phone-last-4 (matching the existing cancel endpoint). Rate-limited 20/min.
- /api/chat/session/{id}: rate-limited 30/min to blunt enumeration of the anon
session ids (customers sometimes paste PII into chat). Full per-browser session
binding tracked as a follow-up.
- SecureHub.jsx: phone-last-4 verification gate before any PII loads; passes phone
to deal + download. a11y label, numeric input, disabled-until-valid.
New regression guard tests/test_pii_endpoint_hardening.py (5 tests). Backend 1098
passed, ruff clean; frontend build + eslint + 48 vitest green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc0e05ca83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async def get_secure_hub_deal(deal_id: str): | ||
| """Fetch deal status and documents for the customer portal.""" | ||
| @limiter.limit("20/minute") | ||
| async def get_secure_hub_deal(deal_id: str, request: Request, phone: str = ""): |
There was a problem hiding this comment.
Because phone is accepted as a GET query parameter here, every successful Secure Hub verification and document download puts the phone last-4—the factor that gates access to buyer PII and signed documents—into the request URL. In production those URLs are commonly captured by access logs, proxies, and browser/network tooling, so the verifier is leaked anywhere the request URL is recorded; use a POST body/header or issue a short-lived verified session/token instead of carrying the digits in the URL.
Useful? React with 👍 / 👎.
1 participant
P0 security: phone-verify customer PII endpoints
From the holistic live-site review (Mark Willcott). Three customer-facing endpoints exposed PII to anyone who knew or guessed an opaque id — a
deal_id/appointment_idis a capability, not a credential.Fixed
/api/v1/customer/deal/{id}+/download/{note_id}(Secure Hub portal): adeal_idalone returned the buyer's name + document list, then minted a 15-minute GCS signed URL to the closing PDF (SSN/DOB). Now requires phone-last-4 verification against the deal's buyer/co-buyer phone before any data or signed URL. Non-existent deal and wrong phone return the same 403 (no existence oracle). Rate-limited 20/min. Also fixed a bareexcept Exceptionthat was convertingHTTPExceptions into 500s./api/appointments/{id}: returned name/phone/email unauthenticated; now requires phone-last-4 (matching the existing cancel endpoint). Rate-limited 20/min./api/chat/session/{id}: rate-limited 30/min to blunt enumeration of the anon session ids (customers paste PII into chat). Per-browser session binding tracked as a follow-up.Verification
tests/test_pii_endpoint_hardening.py(5 tests: helper + endpoint auth + no-oracle).No template/pypdf changes. First of the prioritized remediation batches (P0 security → P1 doc/data → P2 ops → P3 frontend UX).