feat(ai_agents): add Plivo SIP telephony integration#2020
feat(ai_agents): add Plivo SIP telephony integration#2020plutoless merged 3 commits intoTEN-framework:mainfrom
Conversation
Add a new voice assistant example that integrates with Plivo for telephony, mirroring the existing Twilio SIP integration. This enables real-time voice AI applications using Plivo's WebSocket-based media streaming. Key components: - PlivoCallServer for handling calls, webhooks, and WebSocket streaming - Plivo XML generation using plivoxml.ResponseElement - Bidirectional audio streaming with mu-law 8kHz codec - RESTful API for outbound call management - Full STT -> LLM -> TTS voice pipeline integration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PR #2020 Review: Plivo SIP Telephony IntegrationThank you for this substantial contribution! This is a well-structured implementation that mirrors the existing Twilio integration. However, I've identified several security and reliability issues that should be addressed before merging. 🔴 CRITICAL ISSUES (Must Fix)1. WebSocket Resource LeakFile: The WebSocket cleanup in the exception handler doesn't properly remove sessions from except Exception as e:
self._log_error(f"WebSocket error: {e}")
try:
await websocket.close()
except:
pass # Silently ignores errorsFix: Track and clean up properly: finally:
for call_uuid, session in list(self.active_call_sessions.items()):
if session.get("websocket") == websocket:
del self.active_call_sessions[call_uuid]
break
try:
await websocket.close(code=1000)
except Exception as e:
self._log_error(f"Error closing websocket: {e}")2. Race Condition in Call CleanupFile:
if call_uuid in self.server_instance.active_call_sessions:
# ... API call ...
if call_uuid in self.server_instance.active_call_sessions:
del self.server_instance.active_call_sessions[call_uuid]Fix: Use asyncio locks: async with self.server_instance._call_cleanup_lock:
if call_uuid in self.active_call_sessions:
# cleanup
del self.active_call_sessions[call_uuid]🟠 HIGH PRIORITY SECURITY ISSUES3. Overly Permissive CORS ConfigurationFiles: CORS allows all origins, exposing the API to cross-origin attacks: allow_origins=["*"], # ⚠️ Allows any website to make requestsFix: Restrict to specific origins: allow_origins=["http://localhost:3000", os.getenv("ALLOWED_ORIGIN", "")]4. Missing Input ValidationFile: Phone numbers and messages lack validation: phone_number = body.get("phone_number")
if not phone_number:
raise HTTPException(status_code=400, detail="phone_number is required")
# No format validation, length limits, or sanitizationFix: Add proper validation: import re
PHONE_REGEX = re.compile(r'^\+?1?\d{9,15}$')
if not PHONE_REGEX.match(phone_number):
raise HTTPException(status_code=400, detail="Invalid phone number format")
if len(message or "") > 5000:
raise HTTPException(status_code=400, detail="Message too long")5. No API AuthenticationFile: All API endpoints lack authentication - anyone can create, list, or terminate calls. Fix: Implement API key authentication: from fastapi import Header, HTTPException
async def verify_api_key(x_api_key: str = Header(...)):
if x_api_key != os.getenv("API_KEY"):
raise HTTPException(status_code=401, detail="Unauthorized")
@self.app.post("/api/call", dependencies=[Depends(verify_api_key)])
async def create_call(...):
...6. Missing Webhook Signature ValidationFile: Webhook endpoints don't verify requests are actually from Plivo - attackers can spoof webhook calls. Fix: Verify Plivo signatures: import hmac
import hashlib
def verify_plivo_signature(url: str, params: dict, signature: str, auth_token: str) -> bool:
# Implement Plivo's signature verification
# https://www.plivo.com/docs/sms/concepts/signature-validation/
pass🟡 MEDIUM PRIORITY ISSUES7. Memory Leak - No Session CleanupFile: Completed calls remain in Fix: Implement TTL-based cleanup: async def _cleanup_old_sessions(self):
while True:
await asyncio.sleep(300)
now = datetime.now()
for call_uuid, session in list(self.active_call_sessions.items()):
if session.get("status") in ["completed", "failed"]:
age = (now - datetime.fromisoformat(session["created_at"])).total_seconds()
if age > 3600: # 1 hour TTL
del self.active_call_sessions[call_uuid]8. Overly Broad Exception HandlingFile: Generic except Exception as e:
raise HTTPException(status_code=500, detail=str(e)) # ⚠️ Leaks internal errorsFix: Catch specific exceptions and sanitize error messages. 9. Missing Configuration ValidationFile: Startup validation doesn't check if Fix: Add to startup checks: if not config.plivo_public_server_url:
print("Error: plivo_public_server_url is required")
sys.exit(1)10. Unpinned DependenciesFile: Fix: Pin to specific version: 📋 TEST COVERAGEThis PR adds 12,586 lines of code with zero tests. Critical paths that need testing:
Recommendation: Add at least basic tests for: # tests/test_plivo_server.py
@pytest.mark.asyncio
async def test_create_call_missing_phone()
@pytest.mark.asyncio
async def test_websocket_cleanup_on_disconnect()
def test_audio_downsample_maintains_format()🔍 ADDITIONAL OBSERVATIONSCode Quality
Architecture
✅ WHAT'S GOOD
📊 SUMMARY
Recommendation: Request changes to address Critical and High priority issues before merging. The implementation is solid overall, but the security and reliability gaps need to be closed. Happy to discuss any of these findings or help with implementations! 🚀 |
|
thanks for the PR! merged. |
2 participants
Summary
Changes
agents/examples/voice-assistant-sip-plivo/Test plan
🤖 Generated with Claude Code