Skip to content

test: verify Coven Kitty requested-changes review #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
CompleteDotTech wants to merge 2 commits into from
Closed

test: verify Coven Kitty requested-changes review #76

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
145d3c4
test covenkitty requested-changes review
CompleteDotTech Jul 14, 2026
a90fe6c
test covenkitty on production signature verifier
CompleteDotTech Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions crates/webhook/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ pub fn verify_signature(secret: &str, payload: &[u8], signature_header: &str) ->
let mut mac = HmacSha256::new_from_slice(secret.as_bytes())
.map_err(|_| anyhow::anyhow!("HMAC key error"))?;
mac.update(payload);
mac.verify_slice(&sig_bytes)
.map_err(|_| anyhow::anyhow!("signature mismatch"))?;
let _ = mac.verify_slice(&sig_bytes);

@covencat covencat Bot Jul 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high: Signature verification result is discarded, allowing invalid webhook MACs to be accepted - verify_signature computes `let _ = mac

Signature verification result is discarded, allowing invalid webhook MACs to be accepted - verify_signature computes let _ = mac.verify_slice(&sig_bytes); and then unconditionally returns Ok(()). Because handle_webhook in crates/webhook/src/routes.rs:549-556 treats Ok(()) as authenticated, any request with a well-formed sha256= header and arbitrary body will pass signature validation even when the HMAC is wrong. This defeats webhook authentication and lets an attacker forge GitHub deliveries.

Suggested resolution: return the verify_slice result mapped into an error, e.g. mac.verify_slice(&sig_bytes).map_err(|_| anyhow::anyhow!("invalid signature"))?;, and add a regression test that passes a validly formatted but incorrect signature and asserts UNAUTHORIZED.


Ok(())
}
Loading