Skip to content

Improve validation and robustness of GitHub webhook handler #276

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

Open
Sagarshresti18 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Improve validation and robustness of GitHub webhook handler #276

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e2fd1a
feat: add schema validation and error handling for GitHub webhook han…
Sagarshresti18 Feb 13, 2026
4e08d23
refactor: improve webhook validation with structured JSON and schema …
Sagarshresti18 Feb 14, 2026
3422790
refactor: improve webhook validation and exception handling
Sagarshresti18 Feb 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
42 changes: 31 additions & 11 deletions backend/routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,25 @@
from app.core.events.enums import EventType, PlatformType
from app.core.events.base import BaseEvent
from app.core.handler.handler_registry import HandlerRegistry
from pydantic import BaseModel
from pydantic import BaseModel, ValidationError
from typing import Optional, Dict, Any

router = APIRouter()

class RepoRequest(BaseModel):
repo_url: str

class GitHubWebhookPayload(BaseModel):
action: Optional[str] = None
sender: Optional[Dict[str, Any]] = None
pull_request: Optional[Dict[str, Any]] = None


logging.basicConfig(level=logging.INFO)
handler_registry = HandlerRegistry()
event_bus = EventBus(handler_registry)


# Sample handler function to process events
async def sample_handler(event: BaseEvent):
logging.info(f"Handler received event: {event.event_type} with data: {event.raw_data}")
Expand All @@ -35,17 +42,30 @@ def register_event_handlers():
event_bus.register_handler(EventType.PR_COMMENTED, sample_handler, PlatformType.GITHUB)
event_bus.register_handler(EventType.PR_MERGED, sample_handler, PlatformType.GITHUB)

register_event_handlers()

@router.post("/github/webhook")
async def github_webhook(request: Request):
Comment on lines 47 to 48
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if webhook signature verification exists anywhere in the codebase
rg -n -i "hub.signature\|webhook.secret\|hmac\|compare_digest" --type=py

Repository: AOSSIE-Org/Devr.AI

Length of output: 44

🏁 Script executed:

#!/bin/bash
# Also check for X-Hub-Signature references
rg -n "X-Hub-Signature\|signature" --type=py

Repository: AOSSIE-Org/Devr.AI

Length of output: 44

🏁 Script executed:

#!/bin/bash
# Get the full implementation of the webhook handler
cat -n backend/routes.py | head -100

Repository: AOSSIE-Org/Devr.AI

Length of output: 4624

🏁 Script executed:

#!/bin/bash
# Check if there's more content in routes.py after line 100
wc -l backend/routes.py

Repository: AOSSIE-Org/Devr.AI

Length of output: 83

🏁 Script executed:

#!/bin/bash
# Get the rest of the webhook handler
tail -n +100 backend/routes.py | head -50

Repository: AOSSIE-Org/Devr.AI

Length of output: 645

🏁 Script executed:

#!/bin/bash
# Search for any middleware or decorators that might handle signature verification
rg -n "middleware\|verify\|decorator" --type=py backend/

Repository: AOSSIE-Org/Devr.AI

Length of output: 44

Missing webhook signature verification (HMAC).

The endpoint does not verify the X-Hub-Signature-256 header against a shared secret. Without this, anyone can POST to this endpoint and trigger event processing. This is a significant security gap for a production webhook handler. Consider verifying the signature before processing the payload.

🔧 Example approach 
import hmac
import hashlib

GITHUB_WEBHOOK_SECRET = os.environ.get("GITHUB_WEBHOOK_SECRET", "")

`@router.post`("/github/webhook")
async def github_webhook(request: Request):
    body = await request.body()
    signature = request.headers.get("X-Hub-Signature-256", "")
    expected = "sha256=" + hmac.new(
        GITHUB_WEBHOOK_SECRET.encode(), body, hashlib.sha256
    ).hexdigest()
    if not hmac.compare_digest(signature, expected):
        raise HTTPException(status_code=403, detail="Invalid signature")
    
    raw_payload = json.loads(body)
    # ... rest of handler

Note: when reading the body for HMAC verification, you'll need to parse JSON from the raw bytes yourself rather than calling request.json() twice.

🤖 Prompt for AI Agents 
In `@backend/routes.py` around lines 47 - 48, The github_webhook handler lacks
HMAC verification of the X-Hub-Signature-256 header, so update the
github_webhook function to read the raw request body bytes, compute the expected
"sha256=" HMAC using a shared secret (e.g., GITHUB_WEBHOOK_SECRET from env), and
use hmac.compare_digest to compare the header to the expected value; if it
fails, raise HTTPException(status_code=403) and return early, otherwise parse
the JSON from the raw body and continue processing. Ensure you reference the
header name "X-Hub-Signature-256", avoid calling request.json() before
verification, and use hashlib.sha256 for the HMAC calculation inside
github_webhook.

payload = await request.json()
try:
raw_payload = await request.json()
except Exception as e:
logging.error("Failed to parse JSON payload")
raise HTTPException(status_code=400, detail="Invalid JSON payload") from e

try:
payload = GitHubWebhookPayload(**raw_payload)
except ValidationError as e:
logging.error(f"Webhook schema validation failed: {e}")
raise HTTPException(status_code=422, detail="Invalid webhook schema") from e

event_header = request.headers.get("X-GitHub-Event")
logging.info(f"Received GitHub event: {event_header}")

event_type = None

# Handle issue events
if event_header == "issues":
action = payload.get("action")
action = payload.action
if action == "opened":
event_type = EventType.ISSUE_CREATED
elif action == "closed":
Expand All @@ -55,41 +75,41 @@ async def github_webhook(request: Request):

# Handle issue comment events
elif event_header == "issue_comment":
action = payload.get("action")
action = payload.action
if action == "created":
event_type = EventType.ISSUE_COMMENTED

# Handle pull request events
elif event_header == "pull_request":
action = payload.get("action")
action = payload.action
if action == "opened":
event_type = EventType.PR_CREATED
elif action == "edited":
event_type = EventType.PR_UPDATED
elif action == "closed":
# Determine if the PR was merged or simply closed
if payload.get("pull_request", {}).get("merged"):
if payload.pull_request and payload.pull_request.get("merged"):
event_type = EventType.PR_MERGED
else:
logging.info("Pull request closed without merge; no event dispatched.")

# Handle pull request comment events
elif event_header in ["pull_request_review_comment", "pull_request_comment"]:
action = payload.get("action")
elif event_header in ["pull_request_review_comment", "pull_request_review"]:
action = payload.action
if action == "created":
event_type = EventType.PR_COMMENTED

# Dispatch the event if we have a matching type
if event_type:
event = BaseEvent(
id=str(uuid.uuid4()),
actor_id=str(payload.get("sender", {}).get("id", "unknown")),
actor_id=str(payload.sender.get("id", "unknown")) if payload.sender else "unknown",
event_type=event_type,
platform=PlatformType.GITHUB,
raw_data=payload
raw_data=raw_payload
)
await event_bus.dispatch(event)
else:
logging.info(f"No matching event type for header: {event_header} with action: {payload.get('action')}")
logging.warning(f"No matching event type for header: {event_header} with action: {payload.action}")

return {"status": "ok"}