Tracking minor follow-ups from the review of #13 (hash phone numbers in logs). The PR is correct and merged as-is; these are polish items.
1. Extract a hashPhone helper to DRY the ~24 call sites
analytics.HashPhoneNumber(x, h.analyticsHashSalt) is repeated ~24 times across handlers/sms.go and the three voice handlers. A small unexported helper per package would centralize the hashing choice, shorten the log lines (a couple were forced to wrap), and let us drop the new oba-twilio/analytics import from find_stop.go, menu_action.go, and start.go (only handler.go would need it):
// handlers/sms.go
func (h *SMSHandler) hashPhone(p string) string { return analytics.HashPhoneNumber(p, h.analyticsHashSalt) }
// handlers/voice/handler.go
func (h *Handler) hashPhone(p string) string { return analytics.HashPhoneNumber(p, h.analyticsHashSalt) }2. Logged hashes are unsalted when analytics is disabled
ANALYTICS_HASH_SALT is only required when analytics is enabled (analytics/config_loader.go:20-24), and DefaultConfig leaves HashSalt empty (analytics/config.go:54). In the default deployment (analytics disabled) every HashPhoneNumber(phone, "") is an unsalted SHA256 of a low-entropy phone number — trivially reversible via brute force / rainbow tables. This still satisfies #10 (no plaintext numbers in logs) and matches the pre-existing analytics behavior, but the privacy benefit is weak without a salt. Consider requiring/deriving a stable salt independent of whether analytics is enabled.
Tracking minor follow-ups from the review of #13 (hash phone numbers in logs). The PR is correct and merged as-is; these are polish items.
1. Extract a
hashPhonehelper to DRY the ~24 call sitesanalytics.HashPhoneNumber(x, h.analyticsHashSalt)is repeated ~24 times acrosshandlers/sms.goand the three voice handlers. A small unexported helper per package would centralize the hashing choice, shorten the log lines (a couple were forced to wrap), and let us drop the newoba-twilio/analyticsimport fromfind_stop.go,menu_action.go, andstart.go(onlyhandler.gowould need it):2. Logged hashes are unsalted when analytics is disabled
ANALYTICS_HASH_SALTis only required when analytics is enabled (analytics/config_loader.go:20-24), andDefaultConfigleavesHashSaltempty (analytics/config.go:54). In the default deployment (analytics disabled) everyHashPhoneNumber(phone, "")is an unsalted SHA256 of a low-entropy phone number — trivially reversible via brute force / rainbow tables. This still satisfies #10 (no plaintext numbers in logs) and matches the pre-existing analytics behavior, but the privacy benefit is weak without a salt. Consider requiring/deriving a stable salt independent of whether analytics is enabled.