Skip to content

Security and correctness review: installers, webhook server, Vapi flow, request shapes - #5

Merged
allexp1 merged 1 commit into
mainfrom
fix/codex-review
Oct 13, 2026
Merged

allexp1 merged 1 commit into
mainfrom
fix/codex-review

Conversation

@allexp1

@allexp1 allexp1 commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

Fixes from a security and enhancement review of all nine skills. Every finding was reproduced in code and checked against the vendor's own docs or OpenAPI schema before fixing; a few review claims were corrected on the way (see below), and the check turned up bugs the review missed.

Security

  • Installers (all nine): --dest ~ would have moved the home directory into the backups. They now refuse home, ~/.claude, the skills root, the backups folder, the package itself, folders holding another skill or other files, a bare --dest and unknown options. New scripts/test_installers.py (90 checks) runs every real installer against a temporary HOME.
  • OpenAI webhook server: refused to verify nothing when the secret was unset; now refuses to start. Exact To-number match (was substring), 1 MB body cap, socket timeout, threaded server, webhook-id dedup (recorded only after success so retries still work). Docs: the number match is routing, not authentication; keep the project ID private.
  • xAI: the one-time webhook secret was printed and truncated at 1000 chars; now saved untruncated to a 0600 file and masked on screen.
  • Ultravox: never 0.0.0.0/0; --allow-didww sets DIDWW's ranges; fallback secret redacted unless --show-secrets.
  • LiveKit: http:///ws:// refused except localhost. Monitor: DIDWW_API_BASE must be https; pagination never follows another host.
  • Passwords can come from env vars everywhere; Retell/Vogent no longer send a null password; echoed responses mask passwords.

Correctness

  • Vapi: rebuilt on the flow documented by both Vapi and DIDWW: DIDWW inbound trunk to sip.vapi.ai/sip.eu.vapi.ai, inbound credential with DIDWW's seven signaling IPs, outbound credential, number in E.164 with +, max_transfers 1 + Authorization for REFER. --eu now targets api.eu.vapi.ai (it only changed the printout). --rotate replaces the documented-but-missing --update.
  • Ultravox: allowedAgents is an array of {agentId, toUserPattern} per the OpenAPI schema (strings were sent).
  • ElevenLabs: provider: "sip_trunk" per the OpenAPI schema (provider_type does not exist); outbound optional so inbound can go live first.
  • OpenAI: transfer was documented but not implemented; now a transfer_call tool, call websocket watcher and refer with target_uri.
  • Monitor: failed DID/trunk checks ended in OK; page size 200 exceeded DIDWW's max of 100 and only one page was read; --number was a substring match.

Review claims corrected

  • "Ultravox's empty CIDR list is open": Ultravox's guide shows the allowlist is its inbound authentication; the real risk was our text suggesting to leave it open (i.e. 0.0.0.0/0).

Note for DIDWW docs

doc.didww.com's Vapi guide lists EU Vapi IPs 44.233.34.47/32, 44.233.34.48/32; Vapi's networking page lists EU 63.182.83.170/32. The skills follow Vapi's page.

Versions: openai-realtime, vapi, ultravox, elevenlabs 1.1.0; livekit 1.1.1; connect, retell, vogent, xai 1.0.2. bash scripts/test.sh green locally (248 checks).

…w, request shapes

- All nine installers refuse a --dest they do not own (home, skills root,
  package, foreign folders), a bare --dest and unknown options; new
  scripts/test_installers.py runs every installer against a temp HOME.
- OpenAI webhook server: requires the signing secret, exact To match,
  body cap + timeout + threads, webhook-id dedup, working transfer
  (transfer_call tool, call websocket, refer with target_uri).
- Vapi: rebuilt on the flow documented by Vapi and DIDWW (sip.vapi.ai,
  inbound credential with DIDWW's signaling IPs, E.164 with +,
  max_transfers + Authorization); --eu uses api.eu.vapi.ai; --rotate.
- Ultravox: allowedAgents as {agentId, toUserPattern}; --allow-didww;
  0.0.0.0/0 refused; secrets redacted.
- ElevenLabs: provider (not provider_type) per the OpenAPI schema;
  outbound optional.
- xAI: one-time secret saved to a 0600 file, masked on screen.
- Monitor: failed checks warn, pagination at 100 via links.next,
  exact --number match, https only.
- Retell/Vogent: no null passwords; LiveKit: https/wss except localhost;
  passwords from env vars; strict flag parsing everywhere.
- Version bumps, CHANGELOG, docs pages.
@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b2b13bb0-b7ba-461c-b944-5c3236c3c3ef
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
31.6% Duplication on New Code (required ≤ 3%)
B Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@allexp1
allexp1 merged commit 320806c into main Oct 13, 2026
3 of 4 checks passed
@allexp1
allexp1 deleted the fix/codex-review branch October 13, 2026 09:36
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