fix(notifications): null Twilio VerificationCheck.Status causes NRE + dropped audit in 2FA verify (CS8602) #669

New Issue

2026-07-17T17:16:35Z

spikerj commented

2026-07-17 17:16:35 +00:00

Real NRE bug found via CS8602 triage in the Twilio 2FA verify-code path.

VerifyCodeCommandHandler dereferenced verificationCheck.Status.ToString() unguarded in two spots, while every other Status access in the same method used ?. — the author clearly knew Twilio can return a check with a null Status:

Response mapping (VerifyCodeResponse.Status, line 125): a null Status threw an NRE caught by the outer catch, so a valid verification result was reported to the caller as a generic INTERNAL_ERROR instead of the real status/outcome.
Audit write (twilio_verification_attempts.twilioStatus, line 180): the NRE there was swallowed by the logging try/catch, silently dropping the audit record for every null-status verification.

Fix:

Response mapping → verificationCheck.Status?.ToString() (VerifyCodeResponse.Status is string?).
Audit field → BsonValue.Create(verificationCheck.Status?.ToString()). Note: MongoDBs string→BsonValue implicit operator returns C# null (not BsonNull) for a null string, so a bare ?. would still throw ArgumentNullException in BsonDocument.Add — the regression test caught exactly this. BsonValue.Create maps null to BsonNull.Value.

Test: added a regression test via a protected virtual seam over the static Twilio SDK call, asserting a null-Status check (a) does not throw, (b) does not return INTERNAL_ERROR, and (c) still writes the audit record. All 17 handler tests pass. Build clean.

Scope: SpikerSoft.Workers.Notifications/Handlers/VerifyCodeCommandHandler.cs + its test. SonarQube rule external_roslyn:CS8602.

**Real NRE bug found via CS8602 triage** in the Twilio 2FA verify-code path. `VerifyCodeCommandHandler` dereferenced `verificationCheck.Status.ToString()` **unguarded** in two spots, while every other `Status` access in the same method used `?.` — the author clearly knew Twilio can return a check with a null `Status`: 1. **Response mapping** (`VerifyCodeResponse.Status`, line 125): a null `Status` threw an NRE caught by the outer `catch`, so a valid verification result was reported to the caller as a generic `INTERNAL_ERROR` instead of the real status/outcome. 2. **Audit write** (`twilio_verification_attempts.twilioStatus`, line 180): the NRE there was swallowed by the logging `try/catch`, **silently dropping the audit record** for every null-status verification. **Fix:** - Response mapping → `verificationCheck.Status?.ToString()` (`VerifyCodeResponse.Status` is `string?`). - Audit field → `BsonValue.Create(verificationCheck.Status?.ToString())`. Note: MongoDBs `string`→`BsonValue` implicit operator returns **C# null** (not `BsonNull`) for a null string, so a bare `?.` would still throw `ArgumentNullException` in `BsonDocument.Add` — the regression test caught exactly this. `BsonValue.Create` maps null to `BsonNull.Value`. **Test:** added a regression test via a `protected virtual` seam over the static Twilio SDK call, asserting a null-Status check (a) does not throw, (b) does not return `INTERNAL_ERROR`, and (c) still writes the audit record. All 17 handler tests pass. Build clean. Scope: `SpikerSoft.Workers.Notifications/Handlers/VerifyCodeCommandHandler.cs` + its test. SonarQube rule external_roslyn:CS8602.

spikerj commented

2026-07-17 18:03:17 +00:00

Resolved in spikersoft-backend PR #366 (merged to master). Guarded null Twilio VerificationCheck.Status (CS8602): fixed an NRE that turned valid 2FA results into INTERNAL_ERROR and silently dropped audit records; added a regression test. All 17 handler tests pass. All build/test contexts green; merged past the non-blocking notify job. Closing.

Resolved in spikersoft-backend PR #366 (merged to `master`). Guarded null Twilio VerificationCheck.Status (CS8602): fixed an NRE that turned valid 2FA results into INTERNAL_ERROR and silently dropped audit records; added a regression test. All 17 handler tests pass. All build/test contexts green; merged past the non-blocking notify job. Closing.

spikerj closed this issue

2026-07-17 18:03:17 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#669