fix(webhooks): reject decoded secrets below 16 bytes in verifySignature#1887
Open
ibondarenko1 wants to merge 1 commit into
Open
fix(webhooks): reject decoded secrets below 16 bytes in verifySignature#1887ibondarenko1 wants to merge 1 commit into
ibondarenko1 wants to merge 1 commit into
Conversation
Buffer.from(str, base64) silently ignores invalid characters, so a misconfigured whsec_xyz! decodes to a near-zero-byte buffer that gets passed to crypto.subtle.importKey as the HMAC key. OpenAI-issued webhook secrets are 32 bytes; this check enforces the 16-byte (128-bit) minimum effective HMAC-SHA256 key length per NIST SP 800-107. The existing invalid-signature test used a 3-byte secret and asserted on the signature-mismatch error path. Update it to use a 16+ byte garbage secret so the test still exercises signature mismatch, which is its intent.
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.
Problem
verifySignatureatsrc/resources/webhooks/webhooks.ts:77-81decodes the webhook secret two ways:Buffer.from(str, 'base64')silently ignores characters outside the base64 alphabet. A misconfiguredwhsec_xyz!(operator typo, missing characters, or a non-base64 placeholder) decodes to a near-zero-byte buffer. The buffer is then passed tocrypto.subtle.importKeyas the HMAC-SHA256 key. A 1-byte key reduces the brute-force keyspace to 256 candidates; verification still completes and the SDK reportscrypto.subtle.verifyresults as if the signature were validated correctly.#validateSecretat line 119 only checkstypeof secret === 'string' && secret.length > 0. It does not validate the post-decode buffer.Change
After the decode at
src/resources/webhooks/webhooks.ts:81, reject any decoded secret shorter than 16 bytes (128 bits). The error message points the consumer at the most likely cause: invalid base64 inside awhsec_secret.InvalidWebhookSignatureErroris the existing error type used elsewhere inverifySignaturefor signature-validity failures, which keeps the error contract consistent.One test in
tests/api-resources/webhooks.test.tspreviously passed a 3-byte secret (Buffer.from('foo').toString('base64')→ 4 utf-8 bytes after the secret-decode fallback) to verify the "invalid signature" code path. The test now uses a 16+ byte garbage secret so it still exercises the signature-mismatch path rather than the new minimum-length check.Impact
whsec_<base64>format are unaffected.Backwards compatibility
whsec_<valid-base64>: unaffected.whsec_followed by an invalid base64 fragment that decoded to less than 16 bytes: now get an explicit error.Tests
yarn prettier --check src/resources/webhooks/webhooks.ts tests/api-resources/webhooks.test.ts: clean.yarn eslint: clean on touched files.yarn jest tests/api-resources/webhooks.test.ts: 14/14 pass, 12/12 snapshots match.Why this is a hardening contribution
I was reading the webhook verifier flow and noticed that the
Buffer.from(..., 'base64')silent-decode behavior could mask short-key configuration. The change converts a silent failure mode into an explicit one. Happy to revise the threshold (16 bytes is conservative; the actual OpenAI-issued length is 32 bytes) if you prefer to enforce the exact issued length instead.