Repository navigation
fix(webhooks): secure local listener - #419
Conversation
There was a problem hiding this comment.
Hey @mayankbohradev Thanks for picking this up again. Checking the signature before printing or forwarding is the right fix, and reusing resend.webhooks.verify with the secret from webhooks.create keeps it small. Before we merge, I'd like to cut the scope back to that.
Please drop:
- RESEND_WEBHOOK_MAX_BODY_BYTES. Resend webhook payloads are small, and we don't want a setting nobody will use. If you keep a body limit, make it a hardcoded constant. maxBodyBytesFromEnv and the invalid_body_limit error can then go.
- --host and the switch to binding 127.0.0.1. The listener is public through the tunnel by design, so binding to loopback only blocks traffic from the local network. Once signatures are checked, that adds very little, and it changes how the command behaves today (all interfaces, IPv6 included). For example, someone whose tunnel runs in Docker would break. Please keep server.listen(port) as it is.
- The tests that go with those two changes. binds to loopback by default checks option.defaultValue and returns early when there's no non-loopback interface, so it probably never tests anything in CI. supports an explicit bind host would pass on main too.
Please keep:
- The signature check and the missing-signature and invalid-signature tests. Asserting the request was never forwarded and nothing was logged is the right thing to test.
- Moving the test mock to extend the real Resend class.
- The 503 while setup is still running is fine.
Please add:
- the temporary webhook's signing secret is never shown, so apps behind --forward-to can't check the Svix headers we pass along. Printing it in the banner and including it in JSON mode would fix that.
- Remove RESEND_WEBHOOK_MAX_BODY_BYTES; keep a fixed 1 MiB body limit. - Remove --host and keep server.listen(port) on all interfaces. - Remove the bind tests that went with those options. - Show the temporary webhook signing secret in the banner and as the first JSON line, so --forward-to apps can verify the Svix headers.
|
Posted from my other account by mistake. The body limit mentioned here is now removed too — see my reply below and the updated PR description. |
Sorry @mayankbohradev the first message was not clear on the body limit cap, I don't think we want this in this PR. can u please remove too? |
Drop the fixed 1 MiB request body cap, its 413 response and its test, as requested in review. readBody matches main again.
No problem, @dielduarte. Done in 64f4b3a. Removed
What the PR now changes
|
- Label the banner line "Secret:" so its value lines up with the other banner values. - Describe signature checks and the JSON startup line in the agent skill reference.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
I've decided to drop that first line too, it would expose the key in environments that are not dev enviroments like CI, polluting stdout as well. Already approved, thanks for the contribution! |
What
resend webhooks listennow checks each request's Svix signature before it prints or forwards the event. It also shows the temporary webhook's signing secret.Why
The listener accepted any POST that reached the tunnel, so it could print or forward forged events. Apps behind
--forward-toreceived the Svix headers but had no secret to check them with.How
resend.webhooks.verifyand the secret returned bywebhooks.create. Missing or invalid signatures get HTTP 400 and are not printed or forwarded.{"object":"webhook","id":"...","signing_secret":"..."}, the same shapewebhooks createreturns. Event lines follow.server.listen(port)is unchanged.Testing
pnpm test(1,472 passed, 1 skipped),pnpm typecheck,pnpm lint,pnpm build.Notes for reviewers
objectinstead oftype, so it can't be read as an event.skills/resend-cli/references/webhooks.mdare updated.Review
Local Codex review (GPT-6.1-Sol, medium): no findings.