webhooks.js
webhooks.js copied to clipboard
octokit
[FEAT]: Facilitate webhook secret rotation
Describe the need
The current verifyAndReceive function assumes that there is exactly one secret in use at any time:
https://github.com/octokit/webhooks.js/blob/d6286fa324b65716c12ce03efe338faf0e9d2338/src/verify-and-receive.ts#L10-L11
which frustrates secret rotation, which can be a requirement in organizational settings. In particular, the current design essentially forces either programmer suffering (maintaining multiple webhook objects) or a window in which the sender and receiver will disagree about the (singular) secret value.
It would be ideal if, instead, verifyAndReceive accepted a list of secrets and attempted verification against each one. Then key rotation is straightforward:
- Generate a new secret.
- Update the receiver to accept messages MAC'd by this secret. Wait for things to settle.
- Update GitHub to send messages with this secret. Wait for things to settle.
- Remove the old secret from the receiver.
SDK Version
No response
API Version
No response
Relevant log output
No response
Code of Conduct
- [X] I agree to follow this project's Code of Conduct
I think your use case is valid, we could support setting the secret option to an array of strings.
But I would not permit it to be mutable. That would be a different feature, and if we were to decide to support it, I'd make it explicit with a callback function instead of a mutable array.
Oh, yes, sorry, I hadn't meant to suggest a dynamic list of secrets. While I can see it being useful... selfishly, my use case has the WebHook listeners as single-shot Azure Function Apps, so it should suffice to change the secrets registered with those to ensure that all subsequent calls are checked against updated values.
there is now a verifyWithFallback method that we could expose on the webhooks instance: https://github.com/octokit/webhooks-methods.js#verifywithfallback
Yes, sorry; that was always supposed to be the next step after https://github.com/octokit/webhooks-methods.js/pull/134 landed, but I have been transferred internally and my prior projects are being held on minimal life support.