feat: add WhatsApp channel phase 1#11
Conversation
dc452a3 to
520d2c8
Compare
bazfer
left a comment
There was a problem hiding this comment.
Thanks for the follow-up — the allowlist enforcement on reply now normalizes chat_id before checking and before Twilio send, and the inbound PAIR <code> path is now implemented for unknown WhatsApp numbers.
Blocking: attachment downloads still are not bounded while reading the response body. downloadMedia() checks Content-Length when present, but then calls response.arrayBuffer() and only checks buffer.byteLength after the full payload is already in memory. If Twilio (or a dev/test request with validation skipped) returns a missing/incorrect Content-Length, a large media response can still consume unbounded memory before the byte limit trips. Please stream the response body and stop once WA_CHANNEL_MAX_MEDIA_BYTES is exceeded, then write only the bounded bytes to disk.
Non-blocking docs note: .env.example still labels WEBHOOK_PORT, WEBHOOK_HOST, and WEBHOOK_DB_PATH as the FastAPI webhook server settings immediately under the Twilio wa-channel section. The new Bun wa-channel uses WEBHOOK_PORT but not WEBHOOK_HOST/WEBHOOK_DB_PATH, so that section is easy to misread.
bazfer
left a comment
There was a problem hiding this comment.
Final re-review looks good. The attachment downloader now streams from response.body and enforces WA_CHANNEL_MAX_MEDIA_BYTES during read, aborting/canceling once the cap is exceeded even without Content-Length. Content-Type and Content-Length pre-checks remain in place, tests cover missing Content-Length under/over limit streaming behavior, and .env.example separates Bun wa-channel vars from legacy FastAPI vars.
bazfer
left a comment
There was a problem hiding this comment.
Gate review: changes requested.
CI status checked: both ci check runs are green, and local bun install --frozen-lockfile && bun test && bun run typecheck passes. I also checked for inline chatgpt-codex-connector[bot] comments on this PR; none are currently present.
Blocking issue:
- The pairing flow lets any unknown WhatsApp sender self-authorize. In
webhook.ts, an unallowlisted sender who sends any message is issued a pairing code back to that same number (issuePairingCode(from)), andallowWithPairingCode(from, code)then writes that same sender intoaccess.json. That means possession of the public Twilio WhatsApp number is enough to join the allowlist; the allowlist does not provide an operator-controlled trust boundary. For this channel, pairing codes need to be operator/admin-generated out-of-band, pre-seeded for expected numbers, or otherwise require approval by an already-trusted principal before a sender is added.
The attachment streaming byte cap fix looks good; this is specifically an access-control blocker before merge.
bazfer
left a comment
There was a problem hiding this comment.
Re-reviewed the owner-approved access flow at 78aae11. The self-authorizing pairing blocker is fixed: unknown senders only get the generic owner-approval acknowledgement, inbound PAIR messages from unknown senders do not authorize them, pending requests persist in access.json, and approval by owner-visible code adds allowFrom and removes the pending request.\n\nValidation run locally: bun install --frozen-lockfile, bun test, bun run typecheck, and pytest -q (11 passed, 2 existing FastAPI deprecation warnings).
Summary
Validation
Generated from Luna's Phase 1 spec.