From 95ffed11b58293ef163026ef633e7715c27da898 Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sun, 28 Jun 2026 22:55:04 +0900 Subject: [PATCH 1/2] Address intake follow-up review feedback --- apps/api/src/five08/backend/api.py | 11 ++-- .../worker/crm/intake_form_processor.py | 30 +++++++--- tests/unit/test_backend_api.py | 12 ++++ tests/unit/test_intake_form_processor.py | 55 +++++++++++++++++++ 4 files changed, 95 insertions(+), 13 deletions(-) diff --git a/apps/api/src/five08/backend/api.py b/apps/api/src/five08/backend/api.py index 14a8edaf..5008d0d6 100644 --- a/apps/api/src/five08/backend/api.py +++ b/apps/api/src/five08/backend/api.py @@ -819,13 +819,13 @@ def _strip_url_query(value: str) -> str: return urlunsplit((parsed.scheme, parsed.netloc, parsed.path, "", parsed.fragment)) -def _sanitize_tally_raw_payload(value: Any) -> Any: +def _sanitize_intake_raw_payload(value: Any) -> Any: if isinstance(value, Mapping): return { - str(key): _sanitize_tally_raw_payload(item) for key, item in value.items() + str(key): _sanitize_intake_raw_payload(item) for key, item in value.items() } if isinstance(value, list): - return [_sanitize_tally_raw_payload(item) for item in value] + return [_sanitize_intake_raw_payload(item) for item in value] if isinstance(value, str): return _strip_url_query(value) return value @@ -7701,6 +7701,7 @@ async def google_forms_intake_webhook_handler(request: Request) -> JSONResponse: submitted_at=payload.submitted_at, payload=normalized_payload, ) + normalized_payload["raw_payload"] = _sanitize_intake_raw_payload(payload_data) queue = request.app.state.queue try: @@ -7789,8 +7790,8 @@ async def tally_intake_webhook_handler(request: Request) -> JSONResponse: "email": email, "first_name": first_name, "last_name": last_name, - "raw_payload": _sanitize_tally_raw_payload(payload_data), - "raw_tally_fields": _sanitize_tally_raw_payload(raw_tally_fields), + "raw_payload": _sanitize_intake_raw_payload(payload_data), + "raw_tally_fields": _sanitize_intake_raw_payload(raw_tally_fields), } ) diff --git a/apps/worker/src/five08/worker/crm/intake_form_processor.py b/apps/worker/src/five08/worker/crm/intake_form_processor.py index f36336c9..b50ce0b7 100644 --- a/apps/worker/src/five08/worker/crm/intake_form_processor.py +++ b/apps/worker/src/five08/worker/crm/intake_form_processor.py @@ -94,6 +94,13 @@ class IntakeResumeFile: source_url: str +class _ResumeFileNotProvided: + pass + + +_RESUME_FILE_NOT_PROVIDED = _ResumeFileNotProvided() + + class IntakeFormProcessor: """Process a Google Forms member intake submission against CRM.""" @@ -464,7 +471,9 @@ def _build_intake_updates( payload: Mapping[str, Any], include_email: bool = True, include_last_name: bool = True, - resume_file: IntakeResumeFile | None = None, + resume_file: IntakeResumeFile | None | _ResumeFileNotProvided = ( + _RESUME_FILE_NOT_PROVIDED + ), ) -> dict[str, Any]: updates: dict[str, Any] = {"firstName": first_name} if include_last_name: @@ -698,22 +707,27 @@ def _build_resume_updates( self, payload: Mapping[str, Any], *, - resume_file: IntakeResumeFile | None = None, + resume_file: IntakeResumeFile | None | _ResumeFileNotProvided = ( + _RESUME_FILE_NOT_PROVIDED + ), ) -> dict[str, Any]: - if resume_file is None: - resume_file = self._prepare_resume_file(payload) - if resume_file is None: + prepared_resume_file: IntakeResumeFile | None + if isinstance(resume_file, _ResumeFileNotProvided): + prepared_resume_file = self._prepare_resume_file(payload) + else: + prepared_resume_file = resume_file + if prepared_resume_file is None: return {} try: resume_text = self.document_processor.extract_text( - resume_file.content, - resume_file.filename, + prepared_resume_file.content, + prepared_resume_file.filename, ) except Exception as exc: logger.warning( "Failed to parse resume masked_url=%s error=%s", - self._mask_resume_url_for_log(resume_file.source_url), + self._mask_resume_url_for_log(prepared_resume_file.source_url), exc, ) return {} diff --git a/tests/unit/test_backend_api.py b/tests/unit/test_backend_api.py index 7d7a604f..23a991d3 100644 --- a/tests/unit/test_backend_api.py +++ b/tests/unit/test_backend_api.py @@ -9286,6 +9286,7 @@ def test_google_forms_intake_enqueues_job( "first_name": " Jane ", "last_name": " Doe ", "form_id": "form-1", + "resume_url": "https://drive.google.com/resume.pdf?signature=secret", }, headers=auth_headers, ) @@ -9302,6 +9303,17 @@ def test_google_forms_intake_enqueues_job( assert call_kwargs["args"][0]["email"] == "member@example.com" assert call_kwargs["args"][0]["first_name"] == "Jane" assert call_kwargs["args"][0]["last_name"] == "Doe" + assert call_kwargs["args"][0]["resume_url"] == ( + "https://drive.google.com/resume.pdf?signature=secret" + ) + assert call_kwargs["args"][0]["raw_payload"] == { + **_GOOGLE_FORMS_INTAKE_PAYLOAD, + "email": " member@example.com ", + "first_name": " Jane ", + "last_name": " Doe ", + "form_id": "form-1", + "resume_url": "https://drive.google.com/resume.pdf", + } def test_google_forms_intake_rejects_unapproved_form_id( diff --git a/tests/unit/test_intake_form_processor.py b/tests/unit/test_intake_form_processor.py index cdcff649..032a8cbd 100644 --- a/tests/unit/test_intake_form_processor.py +++ b/tests/unit/test_intake_form_processor.py @@ -136,6 +136,33 @@ def test_intake_form_processor_dry_run_create_reports_resume_upload_plan() -> No mock_persist.assert_not_called() +def test_create_prospect_does_not_retry_failed_resume_prepare() -> None: + """Create flow should not download/scan a failed resume more than once.""" + processor = IntakeFormProcessor() + processor.api = MagicMock() + processor.api.request.side_effect = [ + {"list": []}, + {"id": "contact-1"}, + ] + + with ( + patch.object(processor, "_prepare_resume_file", return_value=None) as prepare, + patch.object(processor, "_persist_intake_submission"), + ): + result = processor.process_intake( + payload={ + "email": "new@example.com", + "first_name": "New", + "last_name": "Person", + "resume_url": "https://tally.so/resume.pdf", + "form_id": "form-1", + } + ) + + assert result["success"] is True + prepare.assert_called_once() + + def test_intake_form_processor_dry_run_update_does_not_write_crm_or_db() -> None: """Dry-run update should return planned updates without PUT or persistence.""" processor = IntakeFormProcessor() @@ -173,6 +200,34 @@ def test_intake_form_processor_dry_run_update_does_not_write_crm_or_db() -> None mock_persist.assert_not_called() +def test_update_prospect_does_not_retry_failed_resume_prepare() -> None: + """Update flow should not download/scan a failed resume more than once.""" + processor = IntakeFormProcessor() + processor.api = MagicMock() + processor.api.request.side_effect = [ + {"list": [{"id": "contact-1", "type": "Prospect"}]}, + {}, + ] + + with ( + patch.object(processor, "_prepare_resume_file", return_value=None) as prepare, + patch.object(processor, "_persist_intake_submission"), + ): + result = processor.process_intake( + payload={ + "email": "existing@example.com", + "first_name": "Existing", + "last_name": "Person", + "github_username": "existing-dev", + "resume_url": "https://tally.so/resume.pdf", + "form_id": "form-1", + } + ) + + assert result["success"] is True + prepare.assert_called_once() + + def test_intake_form_processor_uploads_resume_after_create() -> None: """Created prospects should receive the downloaded Tally resume attachment.""" processor = IntakeFormProcessor() From 7de656c6c2225cdac2b7d78e5c36cbc771830552 Mon Sep 17 00:00:00 2001 From: Michael Wu Date: Sun, 28 Jun 2026 23:10:50 +0900 Subject: [PATCH 2/2] Harden intake raw payload sanitization --- apps/api/src/five08/backend/api.py | 10 ++++++---- .../src/five08/worker/crm/intake_form_processor.py | 6 +++--- tests/unit/test_backend_api.py | 14 +++++++++----- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/apps/api/src/five08/backend/api.py b/apps/api/src/five08/backend/api.py index 5008d0d6..eb1b4ce8 100644 --- a/apps/api/src/five08/backend/api.py +++ b/apps/api/src/five08/backend/api.py @@ -812,11 +812,13 @@ def _tally_intake_dry_run_mode( return "none" -def _strip_url_query(value: str) -> str: +def _strip_url_query_and_fragment(value: str) -> str: parsed = urlsplit(value) - if parsed.scheme not in {"http", "https"} or not parsed.netloc or not parsed.query: + if parsed.scheme not in {"http", "https"} or not parsed.netloc: return value - return urlunsplit((parsed.scheme, parsed.netloc, parsed.path, "", parsed.fragment)) + if not parsed.query and not parsed.fragment: + return value + return urlunsplit((parsed.scheme, parsed.netloc, parsed.path, "", "")) def _sanitize_intake_raw_payload(value: Any) -> Any: @@ -827,7 +829,7 @@ def _sanitize_intake_raw_payload(value: Any) -> Any: if isinstance(value, list): return [_sanitize_intake_raw_payload(item) for item in value] if isinstance(value, str): - return _strip_url_query(value) + return _strip_url_query_and_fragment(value) return value diff --git a/apps/worker/src/five08/worker/crm/intake_form_processor.py b/apps/worker/src/five08/worker/crm/intake_form_processor.py index b50ce0b7..91fcecd5 100644 --- a/apps/worker/src/five08/worker/crm/intake_form_processor.py +++ b/apps/worker/src/five08/worker/crm/intake_form_processor.py @@ -14,7 +14,7 @@ from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path -from typing import Any +from typing import Any, cast from urllib.parse import urljoin, urlsplit, urlunsplit from uuid import NAMESPACE_URL, uuid4, uuid5 @@ -712,10 +712,10 @@ def _build_resume_updates( ), ) -> dict[str, Any]: prepared_resume_file: IntakeResumeFile | None - if isinstance(resume_file, _ResumeFileNotProvided): + if resume_file is _RESUME_FILE_NOT_PROVIDED: prepared_resume_file = self._prepare_resume_file(payload) else: - prepared_resume_file = resume_file + prepared_resume_file = cast(IntakeResumeFile | None, resume_file) if prepared_resume_file is None: return {} diff --git a/tests/unit/test_backend_api.py b/tests/unit/test_backend_api.py index 23a991d3..e0612998 100644 --- a/tests/unit/test_backend_api.py +++ b/tests/unit/test_backend_api.py @@ -9286,7 +9286,10 @@ def test_google_forms_intake_enqueues_job( "first_name": " Jane ", "last_name": " Doe ", "form_id": "form-1", - "resume_url": "https://drive.google.com/resume.pdf?signature=secret", + "resume_url": ( + "https://drive.google.com/resume.pdf?signature=secret" + "#token=fragment-secret" + ), }, headers=auth_headers, ) @@ -9304,7 +9307,7 @@ def test_google_forms_intake_enqueues_job( assert call_kwargs["args"][0]["first_name"] == "Jane" assert call_kwargs["args"][0]["last_name"] == "Doe" assert call_kwargs["args"][0]["resume_url"] == ( - "https://drive.google.com/resume.pdf?signature=secret" + "https://drive.google.com/resume.pdf?signature=secret#token=fragment-secret" ) assert call_kwargs["args"][0]["raw_payload"] == { **_GOOGLE_FORMS_INTAKE_PAYLOAD, @@ -9649,15 +9652,16 @@ def test_tally_intake_strips_signed_urls_from_raw_payload_before_enqueue( """Queued raw Tally payloads should not retain signed URL query tokens.""" tally_payload = json.loads(json.dumps(_TALLY_INTAKE_PAYLOAD)) tally_payload["data"]["submissionPdfUrl"] = ( - "https://tally.so/r/abc.pdf?accessToken=secret&signature=sig" + "https://tally.so/r/abc.pdf?accessToken=secret&signature=sig#token=frag" ) tally_payload["data"]["submissionPreviewUrl"] = ( - "https://tally.so/r/abc?accessToken=secret&signature=sig" + "https://tally.so/r/abc?accessToken=secret&signature=sig#token=frag" ) for field in tally_payload["data"]["fields"]: if field["key"] == "question_resume": field["value"][0]["url"] = ( "https://storage.googleapis.com/tally/resume.pdf?signature=sig" + "#token=frag" ) with ( @@ -9676,7 +9680,7 @@ def test_tally_intake_strips_signed_urls_from_raw_payload_before_enqueue( intake_payload = mock_enqueue.call_args.kwargs["args"][0] assert ( intake_payload["resume_url"] - == "https://storage.googleapis.com/tally/resume.pdf?signature=sig" + == "https://storage.googleapis.com/tally/resume.pdf?signature=sig#token=frag" ) raw_payload = intake_payload["raw_payload"] assert raw_payload["data"]["submissionPdfUrl"] == "https://tally.so/r/abc.pdf"