Notification Logs RLS Hardening QA Guide

Scope: Manual verification of two related fixes shipped together:

1. #1530: Per-clinic SMS toggles no longer silently fail-open under NOBYPASSRLS — SmsPolicy and WhatsappPolicy now propagate the facility's tenant_id into an RlsContext.with(...) wrapper before looking up FacilitySettings.
2. #1528: Nil-tenant writes to shared.notification_logs are now logged at warn with a grep-able [notification_log.nil_tenant_write] tag and push an AppSignal counter so monitoring can alert on a spike above the genuine pre-auth baseline.

Prerequisites: Staging access, Rails console + AppSignal dashboard access, a test clinic with at least one per-clinic SMS toggle (e.g. appt_reschedule_sms) explicitly set to false.

Related: Issue #1530 · Issue #1528 · Issue #1527 (deferred)

1. #1530 verification — per-clinic toggle survives NOBYPASSRLSCopy Anchor Link

1.1 SetupCopy Anchor Link

Pick a test clinic and set appt_reschedule_sms = false on its FacilitySettings row. Pick another clinic where the same toggle is true or unset.

test_facility = Identity::Facility.find(<id>)
settings = Identity::FacilitySettings.find_or_create_for_facility!(test_facility)
settings.update!(appt_reschedule_sms: false)

1.2 Dispatch from a Sidekiq job (no Shared::Current GUC)Copy Anchor Link

Open Rails console on staging:

# Simulate the cron / Sidekiq job dispatch path — clear the GUC
# explicitly to mimic the bug scenario.
Shared::Current.reset
Shared::Tenancy::RlsContext.clear

# Now dispatch a reschedule SMS to the test_facility patient.
Shared::Notifications::Dispatcher.call(
  event: 'appointment.rescheduled',
  context: { patient_name: 'Test', appointment_time: 1.day.from_now.iso8601 },
  recipient: <patient>,
  channels: [:sms],
  facility: test_facility,
  tenant_id: test_facility.tenant_id
)

1.3 ExpectationCopy Anchor Link

• Before #1530: the SMS fires (toggle silently fails-open).
• After #1530: the SMS does NOT fire. Log contains [Channels::Sms] suppressed by SmsPolicy event=appointment.rescheduled facility=<id>.
• shared.notification_logs shows NO row for this dispatch attempt (the suppression happens before the log insert).

Repeat for the other clinic (toggle true/unset) and confirm the SMS DOES fire. This rules out "fail closed across the board" as the new failure mode.

1.4 Edge case — duck-typed facility without tenant_idCopy Anchor Link

The fix preserves the old fallback path when a caller passes a duck-typed facility without tenant_id. Verify in Rails console:

fake = OpenStruct.new(id: test_facility.id)
# No tenant_id — should fall back to a plain find_by + the existing
# default-allow behavior. The toggle is NOT honored in this path, which
# is the documented limitation.
Shared::Notifications::SmsPolicy.allowed?(
  event: 'appointment.rescheduled',
  facility: fake
)
# => true (default-allow when settings can't be resolved)

This intentionally matches pre-#1530 behavior for that one duck-typed caller path so #1530 doesn't introduce surprise regressions in unrelated specs.

2. #1528 verification — nil-tenant alertingCopy Anchor Link

2.1 Trigger a pre-auth SMS (signup OTP flow)Copy Anchor Link

From the patient app or staging signup form, request an OTP for a new phone number. The OTP send fires through Shared::Notifications::Sms::SmsCode.deliver(...) with tenant_id: nil.

2.2 ExpectationsCopy Anchor Link

App logsCopy Anchor Link

A warn line lands in the Rails log (/var/log/kulcare/ or your aggregator):

[notification_log.nil_tenant_write] channel=sms to=***1234 caller=path/to/caller.rb:NN

The exact tag [notification_log.nil_tenant_write] is the grep key.

AppSignalCopy Anchor Link

Open the AppSignal dashboard → Metrics → notification_log.nil_tenant_write. A new counter increment lands within 60s. Filter by channel:sms to confirm tagging.

shared.notification_logs rowCopy Anchor Link

A row is created with:

• tenant_id: nil
• channel: 'sms'
• content: '[redacted-pre-auth]'
• recipient: the full phone number

This is the genuine pre-auth path — the row IS expected to land.

2.3 The actual regression case: authenticated caller, missing tenantCopy Anchor Link

This is what #1528 was filed to catch. Hard to reproduce intentionally on staging (it requires a code bug), but the alert path is exercised by the metric above. Configure an AppSignal alert:

• Metric: notification_log.nil_tenant_write
• Condition: rate > <baseline + 50%> over 15 minutes
• Pages: oncall channel

The baseline is whatever genuine pre-auth volume looks like in steady state (probably 1-2 per minute per region). A sustained 5x spike is an authenticated caller losing its tenant context.

2.4 WhatsApp parallelCopy Anchor Link

Repeat 2.1 with a WhatsApp OTP (if the staging gateway is wired). Expect the same log shape with channel=whatsapp and an AppSignal counter increment with channel:whatsapp tag.

3. #1527 (deferred) — what's NOT yet possibleCopy Anchor Link

In-app support read access to pre-auth notification_logs rows remains blocked. A support engineer querying with their tenant's GUC active will still see zero pre-auth rows. The workaround for now is DBA tooling or an RlsContext.clear { ... } block from a Rails console.

This is tracked separately — see #1527 for the RLS schema change needed to expose pre-auth rows to a support_role policy.

What "done" looks likeCopy Anchor Link

• #1530 toggle gating works under simulated NOBYPASSRLS (suppressed log line, no notification_logs row, no SMS).
• #1528 warn line lands with the new tag on pre-auth OTP.
• AppSignal dashboard shows the counter incrementing.
• Alert rule configured against the counter.
• No regressions in the dev-side spec suites (sms_policy_spec.rb, whatsapp_policy_spec.rb, sms_service_spec.rb, whatsapp_service_spec.rb).

RollbackCopy Anchor Link

#1530 is a behavioral change — the toggle now actually gates the send where it previously didn't under NOBYPASSRLS. If a clinic reports "missing SMS after deploy" and you confirm their toggle is false, they expected the broken-fail-open behavior. Either:

1. Flip their toggle to true in CMS (the right user-facing fix).
2. Revert the lookup_settings change in SmsPolicy / WhatsappPolicy to restore the silent fail-open. This is the wrong answer but the fastest rollback if a customer is in pain.

#1528 is pure observability — no rollback risk. If the AppSignal counter emission causes any operational issue, the defined?(::Appsignal) guard plus rescue StandardError ensures the send path keeps working; worst case is the counter stops emitting but SMSes still go.