Skip to content

fix(sdk-python): refuse secrets that decode to an empty key (0.1.2) - #540

Merged
kroqdotdev merged 2 commits into
mainfrom
fix/sdk-python-empty-key
Oct 24, 2026
Merged

kroqdotdev merged 2 commits into
mainfrom
fix/sdk-python-empty-key

Conversation

@kroqdotdev

Copy link
Copy Markdown
Owner

Summary

Security fix for the released Python SDK. decode_standard_webhook_secret turned a secret of whsec_ alone, or only whitespace, into a zero-length HMAC key. An HMAC keyed with nothing is one anyone can compute, so the following accepted forged signatures with such a secret:

  • verify_signature("standard-webhooks" | "clerk" | "resend", ...);
  • verify_forwarded_email, verify_forwarded_request and verify_relayed_request.

I confirmed it against 0.1.1. The realistic way to hit it is a secret read from an empty environment variable or file, for example "whsec_" + os.environ.get(...).

  • An empty key now raises ValueError before any verdict, in verification and in request building alike. This matches the TypeScript SDK, where WebCrypto refuses a zero-length key.
  • Version 0.1.2: pyproject.toml, __version__, uv.lock, PYTHON_SDK_VERSION, and a Python track entry.

An independent review of the Go port (#539) found the bug; that PR has the same fix.

Release

Merge, wait for main's CI, then tag sdk-python-v0.1.2.

Test plan

  • tests/test_empty_key.py covers:
    • four empty secrets across the three providers;
    • the three forwarding checks;
    • signing;
    • a real secret that still verifies.
  • uv run pytest (607 passed), ruff check, ruff format --check, mypy
  • lib/changelog.test.ts

🤖 Generated with Claude Code

decode_standard_webhook_secret turned "whsec_" or a whitespace-only
secret into a zero-length HMAC key, and an HMAC keyed with nothing is one
anyone can compute: verify_signature for standard-webhooks, clerk and
resend, and the forwarding checks, accepted such forged signatures. It
now raises ValueError, before any verdict, and request building refuses
the key too, as WebCrypto does in the TypeScript SDK. A secret read from
an empty variable or file is the realistic way to hit it.

Found by an independent review of the Go port (#539), which has the same
fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c38e313c-6e36-46ce-bed8-16e023a2ad53
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@kroqdotdev

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…cked

From the review of #540: the tests always sent complete, fresh headers,
so removing the early key check in the forwarding functions went
unnoticed; missing headers or a stale timestamp would then have hidden
the misconfigured secret behind missing_headers or
timestamp_out_of_range. The new cases fail without the early check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kroqdotdev added a commit that referenced this pull request Oct 23, 2026
The review of the Python fix (#540) found that no test sent incomplete
headers with an empty-key secret. The same test here showed that
RelayedRequest checked its headers first, so a missing header hid the
misconfigured secret behind missing_headers. It now derives the key
first, like the other checks, and the test covers no headers and a stale
timestamp for every forwarding function and Signature.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kroqdotdev

Copy link
Copy Markdown
Owner Author

CodeRabbit is rate-limited, so a separate review agent reviewed this PR instead.

Verdict: correct, with no blocking problems.

  • Coverage: every path that builds a key from such a secret goes through the shared helper. That covers Standard Webhooks, Clerk and Resend, the three forwarding checks, and template signing.
  • No other way in: no caller can still get True, or a WebhookVerificationError that hides the misuse, for an empty key. No secret other than one that is empty or only ASCII whitespace decodes to an empty key.
  • The tests: 19 of the 20 new tests fail on the old code.
  • The version: 0.1.2 is set consistently everywhere.

One gap closed in 6129da1: the tests always sent complete, fresh headers, so nothing would catch the forwarding check losing its early key check, and with it the empty key would be hidden behind missing_headers or timestamp_out_of_range. New cases with no headers and with a stale timestamp fail without that check. The same test on the Go port (#539) found that RelayedRequest there checked headers first; that is fixed in a627f78.

Noted for later: TypeScript only refuses the key when it imports it, after the header checks. Python and Go now refuse it earlier, which is stricter. Making TypeScript throw early, with a conformance vector for it, is on the follow-up list.

@kroqdotdev
kroqdotdev merged commit 3fb50fd into main Oct 24, 2026
23 checks passed
@kroqdotdev
kroqdotdev deleted the fix/sdk-python-empty-key branch October 24, 2026 15:14
kroqdotdev added a commit that referenced this pull request Oct 24, 2026
The review of the Python fix (#540) found that no test sent incomplete
headers with an empty-key secret. The same test here showed that
RelayedRequest checked its headers first, so a missing header hid the
misconfigured secret behind missing_headers. It now derives the key
first, like the other checks, and the test covers no headers and a stale
timestamp for every forwarding function and Signature.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kroqdotdev added a commit that referenced this pull request Oct 24, 2026
….3) (#539)

* feat: Go SDK webhooks.cc/go 0.1.0, its import path and docs (web 0.42.3)

The Go module webhooks.cc/go in packages/sdk-go, ported from the
TypeScript SDK with the same scope as the Python one. Standard library
only; context.Context and time.Duration throughout.

- webhookscc: the client (endpoints, requests with WaitFor, WaitForAll,
  search, replay and a Subscribe stream to range over, emails with
  EmailQuery, ExtractCode and ExtractLink, forwarding secrets), SendTo and
  BuildRequest signing for 34 providers, matchers, CaptureDuring and
  CaptureEmailDuring. Errors match ErrUnauthorized, ErrNotFound,
  ErrRateLimited and ErrTimeout with errors.Is. Sends never follow
  redirects, whatever the caller's http.Client does.
- verify: signature checks for 32 providers and for what webhooks.cc
  forwards, with no client or API key. PayPal certificates are fetched
  from the URL rebuilt from the checked parse, redirects followed by hand.
- whktest: Client and Endpoint helpers for testing.TB.
- Passes signing.json, verify.json, forwarding.json and matchers.json;
  Go takes time.Duration, so durations.json does not apply.

The web app serves the go-import tag at /go?go-get=1 (and below it), with
the subdirectory field Go 1.25 reads; a plain /go still redirects home.
/docs/sdk/go, a Go track on the changelog, the install page and the docs
nav. CI runs gofmt, vet, staticcheck and the race-enabled tests on Go 1.25
and stable, CodeQL analyses Go, and a tag packages/sdk-go/v* checks itself
against Version and has the module proxy fetch it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(sdk-go): bound rate-limit header numbers before converting them

CodeQL on #539: X-RateLimit-Limit and -Remaining were parsed as 64-bit
numbers and converted to int unchecked. Counts and Retry-After are now
parsed as 32-bit numbers (anything larger reads as absent), and
X-RateLimit-Reset stays 64-bit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(sdk-go): waits poll from server timestamps; cap backoff and stream events

CodeRabbit on #539:
- A wait's next poll started from the local clock, so a client whose
  clock ran ahead of the server's skipped requests that arrived in the
  gap. It now starts five seconds before the newest request seen, on the
  server's clock; the overlap also catches a capture that committed
  after a newer one, and repeats are skipped by id. The TypeScript and
  Python SDKs still use the local clock; that is a follow-up for both.
- Retry and reconnect delays double up to 30 seconds instead of without
  bound, which overflowed time.Duration with many attempts.
- A stream line or event over 64 MiB ends the connection instead of
  growing the buffer without limit (emails reach 10 MiB before escaping).
- The form and UTF-8 decoders that request building and verification
  both need live once, in internal/webtext, so a signed Twilio request
  and its check cannot drift apart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(sdk-go): refuse empty signing keys, plus review fixes

From an independent review of #539:
- A Standard Webhooks, Clerk, Resend or forwarding secret that decodes to
  no key ("whsec_", whitespace) is now an error. Before, an HMAC keyed with
  nothing verified, and anyone can compute one. Signing refuses it too, as
  WebCrypto does in the TypeScript SDK.
- Headers in SendOptions replace built ones of the same name in any case,
  instead of adding a second content-type or signature header.
- Subscribe's idle clock pauses while the loop body runs, so a slow
  consumer is not taken for a silent connection.
- Waits overlap their polls by 30 seconds instead of 5: the receiver stamps
  receivedAt before the capture commits, and captures can queue per account.
- JSON bodies write Go integers exactly (past 2^53 too) and float32 values
  with float32 digits; a value that contains itself fails instead of
  overflowing the stack.
- whktest.Endpoint cuts the test name to the 100 characters endpoint names
  allow; Request.Slug carries the endpoint on search results; a
  response-rule condition always sends its value (eq "" needs it); Null on
  Name, ShowEmailExtracts or ForwardEnabled is refused before the API does.
- Request, Endpoint, Usage and the email types marshal to the API's shape,
  so json.Marshal output reads back.
- Missing conformance vectors fail only where WHK_CONFORMANCE_REQUIRED=1
  (this repository's CI and release), not in a user's go test all.
- The forwarding examples handle every error, not only *verify.Error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(sdk-go): RelayedRequest reports an empty key before checking headers

The review of the Python fix (#540) found that no test sent incomplete
headers with an empty-key secret. The same test here showed that
RelayedRequest checked its headers first, so a missing header hid the
misconfigured secret behind missing_headers. It now derives the key
first, like the other checks, and the test covers no headers and a stale
timestamp for every forwarding function and Signature.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant