From a81cccc68b3600666dcb003d4d946b60a7cf3c13 Mon Sep 17 00:00:00 2001 From: Mubashir R <112580905+Mubashirrrr@users.noreply.github.com> Date: Wed, 10 Jun 2026 17:19:36 +0500 Subject: [PATCH] fix(telephony): handle Cloudonix CDR webhooks missing session/disposition (#407) * fix(telephony): handle Cloudonix CDR payloads missing session/disposition The /cloudonix/cdr webhook is a public, unauthenticated endpoint that parses arbitrary external JSON. It dereferenced cdr_data.get("session").get("token") unconditionally, so a partial or malformed CDR payload that omits "session" (or sends "session": null) raised AttributeError -> HTTP 500. The existing "Missing call_id field" guard right below it was unreachable because the crash happened first. StatusCallbackRequest.from_cloudonix_cdr had the same fragility plus a second one: data.get("disposition", "") returns None when the key is present-but-null, and None.upper() then crashed. Navigate both fields defensively so missing/null values fall through to the intended graceful error path instead of crashing. Adds regression tests covering missing session, null session, null disposition, and the well-formed mapping path. Co-Authored-By: Claude Opus 4.8 * fix: harden cloudonix cdr session validation * chore: renamed test path --------- Co-authored-by: Claude Opus 4.8 Co-authored-by: Sabiha Khan --- .../telephony/providers/cloudonix/routes.py | 3 +- api/services/telephony/status_processor.py | 6 +- api/tests/telephony/cloudonix/test_routes.py | 119 ++++++++++++++++++ 3 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 api/tests/telephony/cloudonix/test_routes.py diff --git a/api/services/telephony/providers/cloudonix/routes.py b/api/services/telephony/providers/cloudonix/routes.py index cd4758a6..facf4bdb 100644 --- a/api/services/telephony/providers/cloudonix/routes.py +++ b/api/services/telephony/providers/cloudonix/routes.py @@ -103,7 +103,8 @@ async def handle_cloudonix_cdr(request: Request): return {"status": "error", "message": "Missing domain field"} # Extract call_id to find workflow run - call_id = cdr_data.get("session").get("token") + session = cdr_data.get("session") + call_id = session.get("token") if isinstance(session, dict) else None logger.info(f"Cloudonix CDR data for call id {call_id} - {cdr_data}") if not call_id: logger.warning("Cloudonix CDR missing call_id field") diff --git a/api/services/telephony/status_processor.py b/api/services/telephony/status_processor.py index f1f1f86a..b93a0d9e 100644 --- a/api/services/telephony/status_processor.py +++ b/api/services/telephony/status_processor.py @@ -114,11 +114,13 @@ class StatusCallbackRequest(BaseModel): "NOANSWER": "no-answer", } - disposition = data.get("disposition", "") + disposition = data.get("disposition") or "" status = disposition_map.get(disposition.upper(), disposition.lower()) + session = data.get("session") + call_id = session.get("token") if isinstance(session, dict) else "" return cls( - call_id=data.get("session").get("token"), + call_id=call_id or "", status=status, from_number=data.get("from"), to_number=data.get("to"), diff --git a/api/tests/telephony/cloudonix/test_routes.py b/api/tests/telephony/cloudonix/test_routes.py new file mode 100644 index 00000000..e22b0672 --- /dev/null +++ b/api/tests/telephony/cloudonix/test_routes.py @@ -0,0 +1,119 @@ +"""Regression tests for Cloudonix CDR webhook handling. + +A Cloudonix CDR webhook is a public, unauthenticated endpoint that parses +arbitrary external JSON. A partial / malformed payload (missing ``session``, +or a ``null`` ``session`` / ``disposition``) must produce a graceful error +response, not an unhandled ``AttributeError`` (HTTP 500). +""" + +from unittest.mock import AsyncMock, patch + +import pytest +from starlette.requests import Request + +from api.services.telephony.providers.cloudonix.routes import handle_cloudonix_cdr +from api.services.telephony.status_processor import StatusCallbackRequest + + +def _json_request(body: bytes) -> Request: + async def receive(): + return {"type": "http.request", "body": body, "more_body": False} + + return Request( + { + "type": "http", + "method": "POST", + "scheme": "https", + "server": ("example.test", 443), + "path": "/api/v1/telephony/cloudonix/cdr", + "query_string": b"", + "headers": [(b"content-type", b"application/json")], + }, + receive, + ) + + +@pytest.mark.asyncio +async def test_cdr_route_handles_payload_without_session(): + """A CDR payload missing the ``session`` object returns a graceful error + instead of raising ``AttributeError`` on ``None.get("token")``.""" + request = _json_request(b'{"domain": "acme.cloudonix.io", "disposition": "ANSWER"}') + + with patch( + "api.services.telephony.providers.cloudonix.routes.db_client" + ) as db_client: + db_client.get_workflow_run_by_call_id = AsyncMock(return_value=None) + + result = await handle_cloudonix_cdr(request) + + assert result == {"status": "error", "message": "Missing call_id field"} + + +@pytest.mark.asyncio +async def test_cdr_route_handles_null_session(): + """A CDR payload with an explicit ``null`` session is handled gracefully.""" + request = _json_request(b'{"domain": "acme.cloudonix.io", "session": null}') + + with patch( + "api.services.telephony.providers.cloudonix.routes.db_client" + ) as db_client: + db_client.get_workflow_run_by_call_id = AsyncMock(return_value=None) + + result = await handle_cloudonix_cdr(request) + + assert result == {"status": "error", "message": "Missing call_id field"} + + +@pytest.mark.asyncio +async def test_cdr_route_handles_string_session(): + """A CDR payload with a non-object session is handled gracefully.""" + request = _json_request(b'{"domain": "acme.cloudonix.io", "session": "abc"}') + + with patch( + "api.services.telephony.providers.cloudonix.routes.db_client" + ) as db_client: + db_client.get_workflow_run_by_call_id = AsyncMock(return_value=None) + + result = await handle_cloudonix_cdr(request) + + assert result == {"status": "error", "message": "Missing call_id field"} + + +def test_from_cloudonix_cdr_tolerates_missing_session_and_disposition(): + """``from_cloudonix_cdr`` must not crash on a partial CDR payload.""" + # Missing both session and disposition. + req = StatusCallbackRequest.from_cloudonix_cdr({"domain": "acme.cloudonix.io"}) + assert req.call_id == "" + assert req.status == "" + + # Explicit null values. + req = StatusCallbackRequest.from_cloudonix_cdr( + {"session": None, "disposition": None} + ) + assert req.call_id == "" + assert req.status == "" + + +def test_from_cloudonix_cdr_tolerates_string_session(): + """``from_cloudonix_cdr`` treats a non-object session as missing call_id.""" + req = StatusCallbackRequest.from_cloudonix_cdr( + {"session": "abc", "disposition": "ANSWER"} + ) + assert req.call_id == "" + assert req.status == "completed" + + +def test_from_cloudonix_cdr_maps_disposition_and_session_token(): + """Normal, well-formed CDR payloads still map correctly.""" + req = StatusCallbackRequest.from_cloudonix_cdr( + { + "session": {"token": "abc123"}, + "disposition": "BUSY", + "from": "+15551230001", + "to": "+15551230002", + "billsec": 12, + } + ) + assert req.call_id == "abc123" + assert req.status == "busy" + assert req.duration == "12"