mirror of
https://github.com/dograh-hq/dograh.git
synced 2026-06-13 08:15:21 +02:00
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 <noreply@anthropic.com>
* fix: harden cloudonix cdr session validation
* chore: renamed test path
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Sabiha Khan <sabihak89@gmail.com>
This commit is contained in:
parent
e79c3e26f0
commit
a81cccc68b
3 changed files with 125 additions and 3 deletions
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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"),
|
||||
|
|
|
|||
119
api/tests/telephony/cloudonix/test_routes.py
Normal file
119
api/tests/telephony/cloudonix/test_routes.py
Normal file
|
|
@ -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"
|
||||
Loading…
Add table
Add a link
Reference in a new issue