README understates who can replay a signed request: forwarding X-Hub-Signature-256 hands every target a valid (body, signature) pair #241

New Issue

clawbot · 2026-08-20T07:56:32+02:00

clawbot commented

2026-08-20 07:56:32 +02:00

Raised during the second review of #228, which adds inbound signature verification (#67). Not a defect in that PR — the design decision behind it was reviewed and accepted twice. This is about what the documentation claims.

GitHub's X-Hub-Signature-256 is an HMAC over the body, not the shared secret, so it is correctly forwarded to delivery targets while GitLab's X-Gitlab-Token (which IS the secret) is stripped. The key cannot be recovered from any volume of body/signature pairs — HMAC-SHA256 is a PRF — and the README's "The credential is not stored or forwarded" section argues exactly that, correctly.

What it does not say: because the digest IS forwarded and stored, every delivery target operator and every reader of an events-*.db holds a valid (body, signature) pair for that entrypoint. Replay is already unbounded and accepted for 1.0 on the grounds that GitHub and GitLab do not prevent it either — but that acceptance was reasoned about an attacker who captured the request in flight. Forwarding widens the set of parties who can replay from "whoever was on the wire" to "every downstream party you deliver to, plus anyone with a backup".

That is a defensible design, and it matches what a plain reverse proxy would do. It is just not what the current README text leads an operator to conclude.

Definition of done:

the README section that argues key-irrecoverability also states plainly that the digest is forwarded and stored, and that this lets any downstream target or backup holder replay that exact body to the entrypoint
if replay protection is later added (nonce/timestamp window), this section is where it gets described

No code change is required for this issue; if the answer is instead to stop forwarding the digest, that is a behaviour change and should be argued on its own merits rather than folded into a docs fix.

Raised during the second review of https://git.eeqj.de/sneak/webhooker/pulls/228, which adds inbound signature verification (https://git.eeqj.de/sneak/webhooker/issues/67). Not a defect in that PR — the design decision behind it was reviewed and accepted twice. This is about what the documentation claims. GitHub's `X-Hub-Signature-256` is an HMAC over the body, not the shared secret, so it is correctly forwarded to delivery targets while GitLab's `X-Gitlab-Token` (which IS the secret) is stripped. The key cannot be recovered from any volume of body/signature pairs — HMAC-SHA256 is a PRF — and the README's "The credential is not stored or forwarded" section argues exactly that, correctly. What it does not say: because the digest IS forwarded and stored, every delivery target operator and every reader of an `events-*.db` holds a valid `(body, signature)` pair for that entrypoint. Replay is already unbounded and accepted for 1.0 on the grounds that GitHub and GitLab do not prevent it either — but that acceptance was reasoned about an attacker who captured the request in flight. Forwarding widens the set of parties who can replay from "whoever was on the wire" to "every downstream party you deliver to, plus anyone with a backup". That is a defensible design, and it matches what a plain reverse proxy would do. It is just not what the current README text leads an operator to conclude. Definition of done: - the README section that argues key-irrecoverability also states plainly that the digest is forwarded and stored, and that this lets any downstream target or backup holder replay that exact body to the entrypoint - if replay protection is later added (nonce/timestamp window), this section is where it gets described No code change is required for this issue; if the answer is instead to stop forwarding the digest, that is a behaviour change and should be argued on its own merits rather than folded into a docs fix.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/webhooker#241