Skip to content

refactor(webhooks): rename misleading SecretHash to ProtectedSecret#1312

Merged
iammukeshm merged 1 commit into
fullstackhero:mainfrom
marcelo-maciel:refactor/webhooks-secret-naming
Jun 24, 2026
Merged

refactor(webhooks): rename misleading SecretHash to ProtectedSecret#1312
iammukeshm merged 1 commit into
fullstackhero:mainfrom
marcelo-maciel:refactor/webhooks-secret-naming

Conversation

@marcelo-maciel

Copy link
Copy Markdown
Contributor

What

WebhookSubscription.SecretHash stores the per-subscription signing secret encrypted at rest via ASP.NET Data Protection (IWebhookSecretProtector.Protect), then recovers it with Unprotect to compute the outbound HMAC signature. The name SecretHash wrongly implies a one-way, irreversible hash, which misrepresents the value's security properties and invites mishandling (treating it as safe-at-rest, or comparing it instead of unprotecting it).

This renames the value to describe what it actually is.

Changes

  • WebhookSubscription.SecretHashProtectedSecret (property + Create parameter), matching the existing protectedSecret local in CreateWebhookSubscriptionCommandHandler.
  • EF RenameColumn migration (webhooks.Subscriptions.SecretHashProtectedSecret) — reversible, preserves existing rows.
  • IWebhookDeliveryService.DeliverAsync parameter secretHashsigningSecret: there the value is the already-unprotected plaintext used for signing, matching the signingSecret local in WebhookDispatchJob.
  • Updated unit + integration test references.

Notes

  • No public contract change — the secret is write-only via the create command and is never returned.
  • Webhooks unit tests pass (54/54); build is clean (TreatWarningsAsErrors).

WebhookSubscription.SecretHash stores the per-subscription signing secret
encrypted at rest via ASP.NET Data Protection (IWebhookSecretProtector.Protect),
then recovered with Unprotect to sign outbound HMAC payloads. The name
"SecretHash" wrongly implies a one-way, irreversible hash, misrepresenting the
value's security properties and inviting mishandling.

- Rename the persisted property/column to ProtectedSecret (matches the existing
  protectedSecret local in CreateWebhookSubscriptionCommandHandler) with a
  RenameColumn migration (reversible, no data loss).
- Rename IWebhookDeliveryService.DeliverAsync secretHash parameter to
  signingSecret: there the value is already unprotected plaintext used for
  signing, matching the signingSecret local in WebhookDispatchJob.

No public contract change (the secret is write-only via command).
@iammukeshm

Copy link
Copy Markdown
Member

Thanks @marcelo-maciel — great catch. SecretHash genuinely misrepresented the value (Data-Protection encrypted, not a one-way hash), and the secondary signingSecret rename makes the protected-vs-plaintext distinction self-documenting. Reversible migration, complete coverage, all green. Merging. 🙏

@iammukeshm iammukeshm merged commit 7d1f944 into fullstackhero:main Jun 24, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants