[Bug] Twilio SMS config-key mismatch — handlers read Twilio:PhoneNumber, but appsettings + deploy docs use Twilio:FromNumber (dead MessagingServiceSid) #264

Closed
opened 2026-06-27 03:23:10 +00:00 by spikerj · 1 comment
Owner

Summary

The Notifications worker's SMS/voice senders read the Twilio sender number from config key Twilio:PhoneNumber, but the worker's appsettings.json and the backend deploy/docker-stack.yml operator docs use Twilio:FromNumber (plus a Twilio:MessagingServiceSid that nothing reads). The keys are dead config. Production currently works only because the live per-service stack happens to set the correct Twilio__PhoneNumber; anyone regenerating the stack from the documented template would silently break all outbound SMS/opt-in/voice with TWILIO_PHONE_NOT_CONFIGURED.

Spun out of the #263 review.

Evidence

Handlers read Twilio:PhoneNumber:

  • SpikerSoft.Workers.Notifications/Handlers/SendSmsCommandHandler.cs:40_configuration["Twilio:PhoneNumber"]
  • SpikerSoft.Workers.Notifications/Handlers/OptInSmsCommandHandler.cs:39 — same
  • SpikerSoft.Workers.Notifications/Handlers/MakeCallCommandHandler.cs:43 — same

SendSmsCommandHandler hard-fails when it is empty:

if (string.IsNullOrEmpty(_twilioPhoneNumber))
    return new SendSmsResponse { Success = false, ErrorCode = "TWILIO_PHONE_NOT_CONFIGURED" };

But the config schema/docs advertise a different key:

  • SpikerSoft.EventHandlers.Notifications/appsettings.jsonTwilio section has "FromNumber": "" and "MessagingServiceSid": "", and no PhoneNumber.
  • spikersoft-backend/deploy/docker-stack.yml notifications comment instructs: Twilio__FromNumber, Twilio__MessagingServiceSid.

Dead keys (read nowhere in the codebase): Twilio:FromNumber, Twilio:MessagingServiceSid.

Impact

  • Latent footgun: the documented/templated env (Twilio__FromNumber) does NOT drive the code; SMS/OptIn/MakeCall would return TWILIO_PHONE_NOT_CONFIGURED.
  • Misleading appsettings.json schema (operators can't discover the real key).
  • Verify/2FA (SendVerificationCode/VerifyCode) is unaffected — it uses Twilio:VerifyServiceSid.
  • The live deployment is fine today purely because the server stack uses Twilio__PhoneNumber.

Proposed fix (align the contract on Twilio:PhoneNumber)

  1. appsettings.json: rename FromNumberPhoneNumber; remove MessagingServiceSid (or actually wire it into the senders if MessagingService-based sending is intended).
  2. spikersoft-backend/deploy/docker-stack.yml: update the operator comment to Twilio__PhoneNumber and drop Twilio__FromNumber / Twilio__MessagingServiceSid.
  3. Optional hardening: log a startup warning when Twilio:Enabled=true but Twilio:PhoneNumber is empty, so the misconfig is visible in logs/Seq instead of only surfacing per-request.
  4. Confirm the live infra stack key stays Twilio__PhoneNumber (it currently is).
## Summary The Notifications worker's SMS/voice senders read the Twilio sender number from config key **`Twilio:PhoneNumber`**, but the worker's `appsettings.json` and the backend `deploy/docker-stack.yml` operator docs use **`Twilio:FromNumber`** (plus a `Twilio:MessagingServiceSid` that nothing reads). The keys are dead config. Production currently works only because the live per-service stack happens to set the correct `Twilio__PhoneNumber`; anyone regenerating the stack from the documented template would silently break all outbound SMS/opt-in/voice with `TWILIO_PHONE_NOT_CONFIGURED`. Spun out of the #263 review. ## Evidence Handlers read `Twilio:PhoneNumber`: - `SpikerSoft.Workers.Notifications/Handlers/SendSmsCommandHandler.cs:40` — `_configuration["Twilio:PhoneNumber"]` - `SpikerSoft.Workers.Notifications/Handlers/OptInSmsCommandHandler.cs:39` — same - `SpikerSoft.Workers.Notifications/Handlers/MakeCallCommandHandler.cs:43` — same `SendSmsCommandHandler` hard-fails when it is empty: ``` if (string.IsNullOrEmpty(_twilioPhoneNumber)) return new SendSmsResponse { Success = false, ErrorCode = "TWILIO_PHONE_NOT_CONFIGURED" }; ``` But the config schema/docs advertise a different key: - `SpikerSoft.EventHandlers.Notifications/appsettings.json` → `Twilio` section has `"FromNumber": ""` and `"MessagingServiceSid": ""`, and **no** `PhoneNumber`. - `spikersoft-backend/deploy/docker-stack.yml` notifications comment instructs: `Twilio__FromNumber`, `Twilio__MessagingServiceSid`. Dead keys (read nowhere in the codebase): `Twilio:FromNumber`, `Twilio:MessagingServiceSid`. ## Impact - Latent footgun: the documented/templated env (`Twilio__FromNumber`) does NOT drive the code; SMS/OptIn/MakeCall would return `TWILIO_PHONE_NOT_CONFIGURED`. - Misleading `appsettings.json` schema (operators can't discover the real key). - Verify/2FA (`SendVerificationCode`/`VerifyCode`) is **unaffected** — it uses `Twilio:VerifyServiceSid`. - The live deployment is fine **today** purely because the server stack uses `Twilio__PhoneNumber`. ## Proposed fix (align the contract on `Twilio:PhoneNumber`) 1. `appsettings.json`: rename `FromNumber` → `PhoneNumber`; remove `MessagingServiceSid` (or actually wire it into the senders if MessagingService-based sending is intended). 2. `spikersoft-backend/deploy/docker-stack.yml`: update the operator comment to `Twilio__PhoneNumber` and drop `Twilio__FromNumber` / `Twilio__MessagingServiceSid`. 3. Optional hardening: log a startup warning when `Twilio:Enabled=true` but `Twilio:PhoneNumber` is empty, so the misconfig is visible in logs/Seq instead of only surfacing per-request. 4. Confirm the live infra stack key stays `Twilio__PhoneNumber` (it currently is).
spikerj added the bug label 2026-06-27 03:23:10 +00:00
Author
Owner

Resolved in spikersoft-backend PR #23 (merged to master, commit 917c67e). Aligned the Twilio sender config on Twilio:PhoneNumber: renamed FromNumberPhoneNumber and dropped the dead MessagingServiceSid in the worker appsettings.json, updated the deploy/docker-stack.yml operator comment to Twilio__PhoneNumber, and added a startup warning when Twilio is enabled but Twilio:PhoneNumber is empty (surfaces in logs/Seq). No infra-stack change needed — the live stack already uses Twilio__PhoneNumber. Closing.

Resolved in spikersoft-backend PR #23 (merged to `master`, commit 917c67e). Aligned the Twilio sender config on `Twilio:PhoneNumber`: renamed `FromNumber` → `PhoneNumber` and dropped the dead `MessagingServiceSid` in the worker `appsettings.json`, updated the `deploy/docker-stack.yml` operator comment to `Twilio__PhoneNumber`, and added a startup warning when Twilio is enabled but `Twilio:PhoneNumber` is empty (surfaces in logs/Seq). No infra-stack change needed — the live stack already uses `Twilio__PhoneNumber`. Closing.
Sign in to join this conversation.