Add apiSecretKeyFallback to validate inbound HMACs during client-secret rotation#3240
Open
morgan-coded wants to merge 2 commits into
Open
Add apiSecretKeyFallback to validate inbound HMACs during client-secret rotation#3240morgan-coded wants to merge 2 commits into
morgan-coded wants to merge 2 commits into
Conversation
…tation Inbound HMAC validation (webhooks, Flow, fulfillment-service) only ever checked against config.apiSecretKey. During a client-secret rotation Shopify keeps signing requests with the previous (oldest non-revoked) secret for a window observed up to ~30 minutes after revocation, so once an app rotates apiSecretKey to the new secret those in-flight requests fail validation and are dropped — non-trivial volume on high-traffic topics like orders/create. Add an optional apiSecretKeyFallback config option, consulted only for inbound HMAC validation in validateHmacString (the shared path behind webhooks, Flow, and fulfillment-service). The primary secret is checked first; the fallback is tried only if the primary fails. Outbound signing (generateLocalHmac) is unchanged and always uses apiSecretKey. When a request validates via the fallback, a Warning-level log is emitted so operators can observe when Shopify has stopped using the old secret and the fallback can be safely removed. Fixes Shopify#3183 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an optional apiSecretKeyFallback config to support seamless client-secret rotation by accepting inbound HMACs signed with a previous secret, while logging a warning when the fallback is used.
Changes:
- New optional
apiSecretKeyFallbackfield onConfigParams. validateHmacStringfalls back to the secondary secret and emits a warning log when matched.- Added test coverage for the rotation/fallback scenarios and a changeset.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/apps/shopify-api/lib/base-types.ts | Documents and introduces the optional apiSecretKeyFallback config field. |
| packages/apps/shopify-api/lib/utils/hmac-validator.ts | Implements fallback-secret validation with a warning log on match. |
| packages/apps/shopify-api/lib/utils/tests/hmac-validator.test.ts | Adds tests covering primary/fallback/no-match scenarios across webhook and Flow types. |
| .changeset/webhook-hmac-secret-fallback.md | Minor-version changeset describing the new config option. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+111
to
+113
| if (safeCompare(hmac, localHmac)) { | ||
| return true; | ||
| } |
github-actions
Bot
added
the
devtools-gardener
Author
|
I have signed the CLA! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
Fixes #3183.
Shopify signs outbound requests with the oldest non-revoked client secret, and
(as reported in #3183) continues to do so for a window observed up to ~30 minutes
after the old secret is revoked.
@shopify/shopify-apionly ever validates inboundHMACs against the single
apiSecretKey, so the moment an app rotatesapiSecretKeyto the new secret, every still-in-flight request signed with the old secret fails
HMAC validation and is rejected with a 401. On high-volume topics (
orders/create,orders/updated) the drop volume during a rotation is not negligible.WHAT is this pull request doing?
Adds an optional
apiSecretKeyFallbackconfig option, consulted only forinbound HMAC validation (webhooks, Flow, and fulfillment-service — all of which
share
validateHmacString):apiSecretKeyis checked first; the fallback is tried only if theprimary fails. If both fail, the request is rejected exactly as before.
generateLocalHmac) is unchanged and always usesapiSecretKey, so the app never emits requests signed with the old secret.Warning-level log is emitted (viathe existing
logger(config)). Operators watch these logs to know when Shopify hasstopped using the old secret, at which point the fallback can be safely unset.
Rotation procedure with this option:
apiSecretKey = S_new,apiSecretKeyFallback = S_old; deploy.S_oldin Partners.apiSecretKeyFallbackand redeploy.This is additive and backward compatible: when
apiSecretKeyFallbackis unset,behavior is identical to today.
How to test your changes?
Targeted
jest --selectProjects library --testPathPatterns "hmac-validator"(25/25 passing after the hardening patch). The new coverage includes primary
match, fallback match, the warn log, no-warn-on-primary, both-fail rejection,
no-fallback rejection, and a Flow fallback request.
Checklist
.changeset/webhook-hmac-secret-fallback.md)