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 1 commit
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
33 changes: 24 additions & 9 deletions backend/routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,24 @@
from app.core.events.base import BaseEvent
from app.core.handler.handler_registry import HandlerRegistry
from pydantic import BaseModel
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,25 @@ 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

@coderabbitai coderabbitai Bot Feb 14, 2026

Copy link
Copy Markdown
Contributor

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()
payload = GitHubWebhookPayload(**raw_payload)
except Exception as e:
logging.error(f"Invalid webhook payload: {e}")
raise HTTPException(status_code=400, detail="Invalid webhook payload")

@coderabbitai coderabbitai Bot Feb 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Improve exception handling per static analysis findings.

Three valid concerns flagged by Ruff:

  1. Blind Exception catch (BLE001): Narrow to the expected types.
  2. logging.error → logging.exception (TRY400): Includes the traceback automatically.
  3. Missing raise ... from (B904): Preserves the exception chain for debugging.
🔧 Proposed fix 
+    from pydantic import ValidationError
     try:
         raw_payload = await request.json()
         payload = GitHubWebhookPayload(**raw_payload)
-    except Exception as e:
-        logging.error(f"Invalid webhook payload: {e}")
-        raise HTTPException(status_code=400, detail="Invalid webhook payload")
+    except (ValueError, ValidationError) as e:
+        logging.exception("Invalid webhook payload")
+        raise HTTPException(status_code=400, detail="Invalid webhook payload") from e

ValueError covers JSON decode failures from request.json(), and ValidationError covers Pydantic schema violations. The from e clause chains the original exception.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try:
raw_payload = await request.json()
payload = GitHubWebhookPayload(**raw_payload)
except Exception as e:
logging.error(f"Invalid webhook payload: {e}")
raise HTTPException(status_code=400, detail="Invalid webhook payload")
from pydantic import ValidationError
try:
raw_payload = await request.json()
payload = GitHubWebhookPayload(**raw_payload)
except (ValueError, ValidationError) as e:
logging.exception("Invalid webhook payload")
raise HTTPException(status_code=400, detail="Invalid webhook payload") from e
🧰 Tools
🪛 Ruff (0.15.0)

[warning] 52-52: Do not catch blind exception: Exception

(BLE001)

[warning] 53-53: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

[warning] 54-54: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

🤖 Prompt for AI Agents 
In `@backend/routes.py` around lines 49 - 54, Narrow the broad except: catch the
specific errors from parsing and validation (ValueError for request.json() and
pydantic.ValidationError for GitHubWebhookPayload) instead of Exception, log the
failure with logging.exception to include the traceback, and re-raise the
HTTPException using raise HTTPException(status_code=400, detail="Invalid webhook
payload") from e to preserve the exception chain; update the try/except around
request.json() and GitHubWebhookPayload(**raw_payload) accordingly.


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 +70,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")
action = payload.action
if action == "created":
event_type = EventType.PR_COMMENTED

@coderabbitai coderabbitai Bot Feb 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

GitHub webhook pull_request events documentation

💡 Result:

GitHub’s official documentation for pull_request webhook events is here (section “pull_request”):

https://docs.github.com/en/enterprise-cloud@latest/webhooks/webhook-events-and-payloads#pull_request

Key points from the docs:

  • The delivery includes headers like X-GitHub-Event: pull_request (plus delivery ID and signature headers). (docs.github.com)
  • The payload includes an action field indicating what happened (for example: opened, edited, closed, reopened, assigned, unassigned, labeled, unlabeled, review_requested, review_request_removed, synchronize). (docs.github.com)
  • How to create/configure a webhook in the GitHub UI is documented here: (docs.github.com)

Citations:

🏁 Script executed:

# Find and check the EventType enum in the codebase
find . -type f -name "*.py" | xargs rg -l "EventType" | head -5

Repository: AOSSIE-Org/Devr.AI

Length of output: 264

🏁 Script executed:

# Find EventType enum definition
rg -n "class EventType|enum EventType" backend/

Repository: AOSSIE-Org/Devr.AI

Length of output: 125

🌐 Web query:

GitHub webhook pull_request_review_comment pull_request_review event types

💡 Result:

For GitHub webhooks (and the same “activity types” you can filter on in GitHub Actions), the action field can be:

  • pull_request_review

    • submitted
    • edited
    • dismissed [1]

  • pull_request_review_comment (comments on the PR diff / “Files changed”)

    • created
    • edited
    • deleted [1]

Notes: “Approved” / “changes_requested” are not action values; they appear as review.state when pull_request_review has action: submitted. [1]

Sources
[1] GitHub Docs — “Events that trigger workflows” (pull_request_review, pull_request_review_comment) https://docs.github.com/actions/automating-your-workflow-with-github-actions/events-that-trigger-workflows

🏁 Script executed:

# Read the EventType enum to check available event types
cat -n backend/app/core/events/enums.py

Repository: AOSSIE-Org/Devr.AI

Length of output: 1217

"pull_request_comment" is not a valid GitHub webhook event.

GitHub sends "pull_request_review_comment" for inline review comments and "pull_request_review" for review submissions — there is no "pull_request_comment" event. This branch is dead code. If the intent is to also handle review submissions, use "pull_request_review" and map it to PR_REVIEWED (which exists in your EventType enum).

🔧 Suggested fix 
-    elif event_header in ["pull_request_review_comment", "pull_request_comment"]:
+    elif event_header in ["pull_request_review_comment"]:
         action = payload.action
         if action == "created":
             event_type = EventType.PR_COMMENTED

And optionally add a separate handler for review submissions:

    elif event_header == "pull_request_review":
        action = payload.action
        if action == "submitted":
            event_type = EventType.PR_REVIEWED
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
elif event_header in ["pull_request_review_comment", "pull_request_comment"]:
action = payload.get("action")
action = payload.action
if action == "created":
event_type = EventType.PR_COMMENTED
elif event_header in ["pull_request_review_comment"]:
action = payload.action
if action == "created":
event_type = EventType.PR_COMMENTED
🤖 Prompt for AI Agents 
In `@backend/routes.py` around lines 97 - 100, The branch is handling a
non-existent "pull_request_comment" event; remove that dead branch and instead
add handling for the real review-submission event "pull_request_review": keep
the existing handling of "pull_request_review_comment" mapping created ->
EventType.PR_COMMENTED, and add a new branch for event_header ==
"pull_request_review" that checks payload.action == "submitted" and sets
event_type = EventType.PR_REVIEWED (use the existing EventType.PR_REVIEWED
enum); ensure you remove references to "pull_request_comment" so only valid
GitHub event names are used.


# 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"}