Skip to content

fix: resolve security-audit findings across both SDKs#163

Merged
nicolotognoni merged 2 commits into
mainfrom
fix/security-audit-fixes
Jun 10, 2026
Merged

fix: resolve security-audit findings across both SDKs#163
nicolotognoni merged 2 commits into
mainfrom
fix/security-audit-fixes

Conversation

@nicolotognoni

Copy link
Copy Markdown
Collaborator

Summary

  • Fixes every confirmed finding from a full-codebase security/bug audit: 2 crash/leak bugs in the TypeScript server, a latency bug that cost ~1.5s on the barge-in window of every Twilio call, tool-history replay polluting LLM context, asyncio loop hygiene on Python 3.12+, and 3 security hardenings (anti-replay alignment, prompt-variable sanitisation parity, log redaction).
  • Resolves all open npm advisories (hono ×4 moderate, vitest dev-only critical) via a lockfile-only bump; npm audit and pip-audit are both clean.

Implementation

Bug fixes

  • telephony/twilio.py / telephony/plivo.py (Python): send_mark now puts the caller-supplied name on the wire instead of a locally generated audio_N, so the first-message pacer's fm_N echo matches and marks resolve via carrier echo instead of burning the 0.5s timeout (×3 marks per call). Matches the TypeScript behaviour.
  • src/server.ts: the three carrier WS message handlers now await handler.handleAudio(...) — a rejection in the audio hot path (~50 frames/s) previously escaped the try/catch and terminated the process as an unhandled rejection.
  • src/server.ts: the Telnyx WS close handler now deletes its activeCallIds entry (Twilio/Plivo already did), fixing an unbounded map leak plus spurious hangup REST calls at graceful shutdown.
  • services/llm_loop.py / src/llm-loop.ts: pipeline context rebuild skips role="tool" history entries instead of replaying them as fabricated user turns containing raw tool JSON.
  • stream_handler.py, providers/elevenlabs_ws_tts.py (Python): deprecated asyncio.get_event_loop() removed from running-loop contexts; the ElevenLabs barge-in cancel schedules the close on the loop captured at stream start, with a done-callback surfacing close errors.
  • stream_handler.py (Python): pre-barge-in inbound audio ring is now deque(maxlen=13) (O(1) eviction in the per-frame path) with a snapshot before the async replay.

Security

  • Telnyx anti-replay aligned across SDKs: accepted timestamp age is now [-30s, +300s] in both. Python previously accepted up to +300s in the future (abs() check — doubled replay window); TypeScript rejected ANY future timestamp (dropped legitimate webhooks on clock-lagged hosts).
  • sanitizeVariables (TS) now strips control chars and caps values at 500 chars, byte-for-byte parity with Python _sanitize_variable_value (caller-supplied carrier custom params reach the system prompt).
  • LLM API error log line truncated to 200 chars (TS) — 401 bodies can embed the rejected key prefix.

Deps: lockfile-only npm audit fix (hono ≥4.12.21, vitest ≥3.2.6), separate commit.

No new dependencies. No public API changes.

Breaking change?

No. The only behaviour changes are: (a) Telnyx webhooks dated >30s in the future are now rejected by Python (previously ≤300s accepted) — within spec for signed-webhook freshness; (b) TS now ACCEPTS ≤30s future skew (previously rejected) — strictly fewer false rejections; (c) TS template variables are sanitised like Python already was.

Test plan

  • Python: pytest tests/ -m "not soak" — 2360 passed, 7 skipped, 2 xfailed
  • TypeScript: npm test (1823 passed across 114 files) + npm run lint + npm run build — clean, re-run after the dependency bump
  • New regression tests in BOTH SDKs: mark-name pass-through (Twilio + Plivo), Telnyx anti-replay window (stale / far-future rejected, ≤30s skew accepted, real Ed25519 keypairs), tool-history skip in context rebuild, sanitizeVariables control-char strip + 500 cap
  • npm audit → 0 vulnerabilities; pip-audit → clean

Docs updates

  • N/A (no public API surface changed; CHANGELOG.md updated under ## Unreleased### Fixed / ### Security)

Bug fixes (confirmed on main by a full-codebase audit):

- Python: TwilioAudioSender.send_mark / Plivo checkpoint discarded the
  caller-supplied mark name and sent a locally generated audio_N instead,
  so the first-message pacer's fm_N echo never matched and every mark
  resolved via the 0.5s fallback timeout (~1.5s guaranteed extra latency
  in the barge-in window of every Twilio call).
- TypeScript: handler.handleAudio(...) was un-awaited in all three carrier
  WS message handlers — a rejection in the audio path escaped the
  try/catch and killed the Node process as an unhandled rejection.
- TypeScript: the Telnyx WS close handler never deleted its activeCallIds
  entry (Twilio/Plivo did), leaking map entries for the server lifetime
  and firing hangups for dead calls on graceful shutdown.
- Both: pipeline context rebuild replayed role="tool" history entries as
  fabricated user turns containing raw tool JSON; they are now skipped.
- Python: deprecated asyncio.get_event_loop() removed from running-loop
  contexts (stream_handler mark waiter, ElevenLabs WS TTS cancel path —
  the latter now schedules close on the loop captured at stream start
  with a done-callback surfacing errors).
- Python: pre-barge-in inbound audio ring converted to deque(maxlen=13)
  (O(1) eviction in the per-frame hot path) with a snapshot before the
  async replay to avoid mutation-during-iteration.

Security hardening:

- Telnyx anti-replay window aligned across SDKs: future-dated timestamps
  rejected beyond a 30s clock-skew allowance (Python previously accepted
  up to +300s via abs(); TypeScript rejected ANY future timestamp,
  dropping legitimate webhooks on clock-lagged hosts).
- TypeScript sanitizeVariables now strips control characters and caps
  values at 500 chars (parity with Python _sanitize_variable_value) —
  caller-supplied template variables reach the system prompt.
- TypeScript LLM API error bodies truncated to 200 chars in logs (401
  bodies can embed the rejected API-key prefix).

Regression tests added/updated in both SDKs for every fix.
Resolves 4 moderate hono advisories (IPv6 deny-rule bypass, Set-Cookie
injection, JWT scheme laxness, percent-encoded mount routing) and the
dev-only critical vitest UI arbitrary-file-read advisory. Lockfile-only
change; both versions move within existing semver ranges. npm audit is
clean after the bump.
@nicolotognoni nicolotognoni merged commit ac65293 into main Jun 10, 2026
14 checks passed
@github-actions github-actions Bot deleted the fix/security-audit-fixes branch June 10, 2026 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant