Webhook receiver saved the signed bytes, then hashed a copy it rewrote

Flow

How each request case was handled

Only the second row was always right in the first version. The first row could still get a 401 when the sender's JSON didn't match JavaScript's.

1. 1Secret set, signature good⌄

   Accept the event. The first version hashed a re-encoded copy, so this could fail for senders whose JSON differs from JavaScript's.

2. 2Secret set, signature wrong⌄

   Return 401. The first version already did this.

3. 3Secret set, signature missing⌄

   First version: log a warning and accept. I want a 401.

4. 4No secret set⌄

   First version: no check. I want that to be a visible, deliberate setting.

5. 5Saved body missing⌄

   The second receiver re-encodes the parsed body. I want the request rejected.

Hard part done right, then ignored: the new webhook receiver managed both. It kept a copy of the exact bytes the sender signed, and then it checked the signature against a different string.

Keeping the raw bytes

The receiver gets alert events from an incident service. The sender hashes each body with a shared secret and puts the result in a header. I hash the same body with the same secret and compare. The first version did the hash and the compare well. It also had a hook that runs before the body is parsed. The hook kept a copy of the raw bytes and passed the body on as normal.

Hashing a re-encoded copy

To get a string to hash, the handler took the parsed body and wrote it back out as JSON. That new text can differ from what was sent.

JSON.stringify(JSON.parse('{ "n": 2.0 }')); // '{"n":2}'

Spaces vanish, numbers get rewritten, repeated keys collapse. The hash changes with them. So the check only works when the sender writes JSON exactly the way JavaScript does. I never saw it turn away a real event; I'm going from the diffs and tests. But the bug would look like a correct secret, a correct signature, and a 401.

Code review caught it, and a one-line change made the handler hash the saved copy. If you build one of these, hash the bytes first and parse them after.

The parts still worth changing

The same code had other problems that a one-line change didn't touch:

• A missing signature header only logged a warning, and the event went through anyway.
• With no secret set, nothing was checked at all.
• A second receiver re-encodes the body if the saved copy is missing, which brings the bug back.
• A shared helper had its own tests, but the route never called it, so the tests couldn't see this.

The receiver also returns success after internal errors, probably so the sender won't retry. That's fine for junk events, but it means a 401 is the only way the sender learns the secret is wrong.

The saved copy is stored as text, which can mangle bad bytes, so I'd keep the raw buffer instead. I'd bet one test that signed a body with an extra space in it would have caught this before review did.