Skip to content

Improvement: Github webhook, CORS, global exception exception handling in FastAPI returns JSON response. #216

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

Draft
RishiGoswami-code wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Improvement: Github webhook, CORS, global exception exception handling in FastAPI returns JSON response. #216

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71588bd
github webhook, CORS, global exception exception handling in FastAPI …
RishiGoswami-code Jan 6, 2026
2089690
Error fixed with logging and uuid
RishiGoswami-code Jan 6, 2026
fcde48c
Header, ProtectedLayout, typo error fixed
RishiGoswami-code Jan 6, 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
9 changes: 9 additions & 0 deletions backend/app/core/config/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class Settings(BaseSettings):

# Platforms
github_token: str = ""
github_webhook_secret: str = ""
discord_bot_token: str = ""

# DB configuration
Expand All @@ -39,6 +40,14 @@ class Settings(BaseSettings):
# Backend URL
backend_url: str = ""

# CORS configuration
allow_origins: list[str] = [
"http://localhost:5173",
"http://localhost:3000",
"http://127.0.0.1:5173",
"http://127.0.0.1:3000",
]

# Onboarding UX toggles
onboarding_show_oauth_button: bool = True

Expand Down
18 changes: 12 additions & 6 deletions backend/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,23 @@ async def lifespan(app: FastAPI):
# Configure CORS
api.add_middleware(
CORSMiddleware,
allow_origins=[
"http://localhost:5173", # Vite default dev server
"http://localhost:3000", # Alternative dev server
"http://127.0.0.1:5173",
"http://127.0.0.1:3000",
],
allow_origins=settings.allow_origins,
allow_credentials=True,
allow_methods=["*"],
allow_headers=["*"],
)


@api.exception_handler(Exception)
async def global_exception_handler(request, exc):
"""Log all unhandled exceptions and return a JSON error response."""
logger.error(f"Unhandled exception: {str(exc)}", exc_info=True)
return Response(
content='{"detail": "Internal Server Error"}',
status_code=500,
media_type="application/json"
)

@api.get("/favicon.ico")
async def favicon():
"""Return empty favicon to prevent 404 logs"""
Expand Down
104 changes: 67 additions & 37 deletions backend/routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import asyncio
import uuid
import hmac
import hashlib
import logging
from fastapi import APIRouter, Request, HTTPException
import uuid
import json
from fastapi import APIRouter, Request, HTTPException, Header
from app.core.events.event_bus import EventBus
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 app.core.config.settings import settings

router = APIRouter()

Expand Down Expand Up @@ -35,50 +38,77 @@ 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)

async def verify_signature(body: bytes, x_hub_signature_256: str = Header(None)):
"""Verify that the payload was sent from GitHub by validating the SHA256 signature."""
if not settings.github_webhook_secret:
# If secret is not configured, we skip verification (not recommended for production)
return
Comment on lines +43 to +45
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add warning log when signature verification is skipped.

Silently skipping signature verification creates a security risk. If accidentally deployed to production without a configured secret, the endpoint will accept unauthenticated webhook requests from any source.

🔎 Proposed fix 
     if not settings.github_webhook_secret:
         # If secret is not configured, we skip verification (not recommended for production)
+        logging.warning("GitHub webhook secret not configured - signature verification skipped. This is insecure for production!")
         return
🤖 Prompt for AI Agents 
In @backend/routes.py around lines 43 - 45, When the code hits the condition "if
not settings.github_webhook_secret" it currently returns silently; add a
warning-level log immediately before the return to surface that signature
verification is being skipped (e.g., logger.warning or app.logger.warning with a
clear message like "GitHub webhook secret not configured; skipping signature
verification — insecure for production."). Update the webhook handler code where
the settings.github_webhook_secret check lives to emit this warning and include
identifying context (environment or endpoint) using the existing logger
instance, then return as before.


if not x_hub_signature_256:
raise HTTPException(status_code=401, detail="X-Hub-Signature-256 header is missing")

signature = hmac.new(
settings.github_webhook_secret.encode(),
body,
hashlib.sha256
).hexdigest()

if not hmac.compare_digest(f"sha256={signature}", x_hub_signature_256):
raise HTTPException(status_code=401, detail="Invalid webhook signature")
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Incorrect use of Header() in helper function.

The Header(None) dependency annotation on line 41 only works in FastAPI route handlers where dependency injection is active. In a helper function called manually (line 69), it will not extract the header value as intended and may cause runtime errors.

The header is already correctly extracted in the github_webhook route handler (line 63). Simply pass it as a regular parameter.

🔎 Proposed fix 
-async def verify_signature(body: bytes, x_hub_signature_256: str = Header(None)):
+async def verify_signature(body: bytes, x_hub_signature_256: str | None):
     """Verify that the payload was sent from GitHub by validating the SHA256 signature."""
     if not settings.github_webhook_secret:
         # If secret is not configured, we skip verification (not recommended for production)
         return
📝 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
async def verify_signature(body: bytes, x_hub_signature_256: str = Header(None)):
"""Verify that the payload was sent from GitHub by validating the SHA256 signature."""
if not settings.github_webhook_secret:
# If secret is not configured, we skip verification (not recommended for production)
return
if not x_hub_signature_256:
raise HTTPException(status_code=401, detail="X-Hub-Signature-256 header is missing")
signature = hmac.new(
settings.github_webhook_secret.encode(),
body,
hashlib.sha256
).hexdigest()
if not hmac.compare_digest(f"sha256={signature}", x_hub_signature_256):
raise HTTPException(status_code=401, detail="Invalid webhook signature")
async def verify_signature(body: bytes, x_hub_signature_256: str | None):
"""Verify that the payload was sent from GitHub by validating the SHA256 signature."""
if not settings.github_webhook_secret:
# If secret is not configured, we skip verification (not recommended for production)
return
if not x_hub_signature_256:
raise HTTPException(status_code=401, detail="X-Hub-Signature-256 header is missing")
signature = hmac.new(
settings.github_webhook_secret.encode(),
body,
hashlib.sha256
).hexdigest()
if not hmac.compare_digest(f"sha256={signature}", x_hub_signature_256):
raise HTTPException(status_code=401, detail="Invalid webhook signature")
🤖 Prompt for AI Agents 
In @backend/routes.py around lines 41 - 57, The helper function verify_signature
incorrectly uses Header(None) for x_hub_signature_256 (dependency injection only
works in route handlers); change verify_signature to accept x_hub_signature_256
as a normal parameter (e.g., x_hub_signature_256: str = None) and remove the
Header import/annotation, then update the github_webhook route to pass the
extracted header value into verify_signature when calling it so the header is
validated correctly.


@router.post("/github/webhook")
async def github_webhook(request: Request):
payload = await request.json()
event_header = request.headers.get("X-GitHub-Event")
logging.info(f"Received GitHub event: {event_header}")
async def github_webhook(
request: Request,
x_github_event: str = Header(None),
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Validate required webhook headers explicitly.

The x_github_event header can be None, but there's no explicit validation. While the code handles None gracefully downstream (it won't match any event mapping), validating required headers upfront provides clearer error messages and better API hygiene.

🔎 Proposed fix 
 async def github_webhook(
     request: Request,
-    x_github_event: str = Header(None),
+    x_github_event: str = Header(...),
     x_hub_signature_256: str = Header(None)
 ):

Using Header(...) instead of Header(None) makes x_github_event required, and FastAPI will automatically return a clear 422 error if the header is missing.

🤖 Prompt for AI Agents 
In @backend/routes.py at line 62, The x_github_event header parameter is
currently optional (Header(None)); make it required by replacing Header(None)
with Header(...) in the route signature where x_github_event: str is declared
(so FastAPI will validate and return a 422 when the header is missing), ensuring
the webhook handler parameter name x_github_event is updated in that function
signature and any callers/tests adjusted if needed.

x_hub_signature_256: str = Header(None)
):
# Read body once to avoid stream exhaustion
body = await request.body()

# Verify GitHub signature
await verify_signature(body, x_hub_signature_256)

try:
payload = json.loads(body.decode())
except json.JSONDecodeError:
raise HTTPException(status_code=400, detail="Invalid JSON payload")
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add exception chaining for better error traceability.

When re-raising or raising a new exception within an except clause, use raise ... from err to preserve the exception chain, or raise ... from None to explicitly suppress it.

🔎 Proposed fix 
     try:
         payload = json.loads(body.decode())
     except json.JSONDecodeError:
-        raise HTTPException(status_code=400, detail="Invalid JSON payload")
+        raise HTTPException(status_code=400, detail="Invalid JSON payload") from None

Based on static analysis hints.

📝 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:
payload = json.loads(body.decode())
except json.JSONDecodeError:
raise HTTPException(status_code=400, detail="Invalid JSON payload")
try:
payload = json.loads(body.decode())
except json.JSONDecodeError:
raise HTTPException(status_code=400, detail="Invalid JSON payload") from None
🧰 Tools
🪛 Ruff (0.14.10)

74-74: 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 71 - 74, The JSON decode except block should
preserve the original exception chain: change the except to "except
json.JSONDecodeError as err:" and re-raise the HTTPException with "from err"
(e.g., raise HTTPException(status_code=400, detail="Invalid JSON payload") from
err) so the original json.loads/body.decode error is chained for traceability.

logging.info(f"Received GitHub event: {x_github_event}")

event_type = None

# Handle issue events
if event_header == "issues":
action = payload.get("action")
if action == "opened":
event_type = EventType.ISSUE_CREATED
elif action == "closed":
event_type = EventType.ISSUE_CLOSED
elif action == "edited":
event_type = EventType.ISSUE_UPDATED

# Handle issue comment events
elif event_header == "issue_comment":
action = payload.get("action")
if action == "created":
event_type = EventType.ISSUE_COMMENTED
# Mapping GitHub events and actions to EventType
event_mapping = {
"issues": {
"opened": EventType.ISSUE_CREATED,
"closed": EventType.ISSUE_CLOSED,
"edited": EventType.ISSUE_UPDATED,
},
"issue_comment": {
"created": EventType.ISSUE_COMMENTED,
},
"pull_request": {
"opened": EventType.PR_CREATED,
"edited": EventType.PR_UPDATED,
},
"pull_request_review_comment": {
"created": EventType.PR_COMMENTED,
},
"pull_request_comment": {
"created": EventType.PR_COMMENTED,
}
}

# Handle pull request events
elif event_header == "pull_request":
if x_github_event in event_mapping:
action = payload.get("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
event_type = event_mapping[x_github_event].get(action)

# Special casing for PR merged
if x_github_event == "pull_request" and action == "closed":
if payload.get("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")
if action == "created":
event_type = EventType.PR_COMMENTED

# Dispatch the event if we have a matching type
if event_type:
event = BaseEvent(
Expand All @@ -90,6 +120,6 @@ async def github_webhook(request: Request):
)
await event_bus.dispatch(event)
else:
logging.info(f"No matching event type for header: {event_header} with action: {payload.get('action')}")
logging.info(f"No matching event type for header: {x_github_event} with action: {payload.get('action')}")

return {"status": "ok"}
Binary file added frontend/Devr logo.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
25 changes: 14 additions & 11 deletions frontend/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="/vite.svg" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Devr.AI</title>
</head>
<body>
<div id="root"></div>
<script type="module" src="/src/main.tsx"></script>
</body>
</html>

<head>
<meta charset="UTF-8" />
<link rel="icon" type="image/svg+xml" href="/DevrAI.svg" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Devr.AI</title>
</head>

<body>
<div id="root"></div>
<script type="module" src="/src/main.tsx"></script>
</body>

</html>
0 DevrAI.svg → frontend/public/DevrAI.svg
View file
Open in desktop
File renamed without changes
136 changes: 38 additions & 98 deletions frontend/src/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useEffect, useState } from 'react';
import { useState } from 'react';
import { BrowserRouter as Router, Routes, Route, Navigate, Outlet } from 'react-router-dom';
import { AnimatePresence } from 'framer-motion';
import toast, { Toaster } from 'react-hot-toast';
import { Toaster } from 'react-hot-toast';

import Sidebar from './components/layout/Sidebar';
import Dashboard from './components/dashboard/Dashboard';
Expand All @@ -15,112 +15,44 @@ import LandingPage from './components/landing/LandingPage';
import LoginPage from './components/pages/LoginPage';
import ProfilePage from './components/pages/ProfilePage';
import SignUpPage from './components/pages/SignUpPage';
import { supabase } from './lib/supabaseClient';
import ForgotPasswordPage from './components/pages/ForgotPasswordPage';
import ResetPasswordPage from './components/pages/ResetPasswordPage';
import { useAuth } from './hooks/useAuth';
import { RepositoryData } from './types';

// Define proper TypeScript interfaces
interface RepositoryData {
id: string;
name: string;
full_name: string;
description?: string;
html_url: string;
stargazers_count: number;
forks_count: number;
language?: string;
created_at: string;
updated_at: string;
owner: {
login: string;
avatar_url: string;
};
}
const ProtectedLayout = ({ isSidebarOpen, setIsSidebarOpen, handleLogout }: any) => (
<div className="flex">
<Sidebar
isOpen={isSidebarOpen}
setIsOpen={setIsSidebarOpen}
onLogout={handleLogout}
/>
<main
className={`transition-all duration-300 flex-1 ${isSidebarOpen ? 'ml-64' : 'ml-20'
}`}
>
<div className="p-8">
<AnimatePresence mode="wait">
<Outlet />
</AnimatePresence>
</div>
</main>
</div>
);

function App() {
const [isSidebarOpen, setIsSidebarOpen] = useState(true);
const [repoData, setRepoData] = useState<RepositoryData | null>(null);
const [isAuthenticated, setIsAuthenticated] = useState(false);

// Auto login if user has already logged in
useEffect(() => {
supabase.auth.getSession().then(({ data, error }) => {
if (error) {
toast.error('User Login Failed');
console.error('Error checking session:', error);
return;
}
setIsAuthenticated(!!data.session);
});

const { data: subscription } = supabase.auth.onAuthStateChange(
(event, session) => {
switch (event) {
case "SIGNED_IN":
setIsAuthenticated(true);
toast.success("Signed in!");
break;

case "SIGNED_OUT":
setIsAuthenticated(false);
setRepoData(null);
toast.success("Signed out!");
break;

case "PASSWORD_RECOVERY":
toast("Check your email to reset your password.");
break;
case "TOKEN_REFRESHED":
// Session refreshed silently
break;
case "USER_UPDATED":
// User profile updated
break;
}
}
);
const { isAuthenticated, loading, logout } = useAuth();

return () => {
subscription.subscription.unsubscribe();
};
}, []);

const handleLogin = () => {
setIsAuthenticated(true);
};
if (loading) {
return <div className="min-h-screen bg-gray-950 flex items-center justify-center text-white">Loading...</div>;
}

const handleLogout = async () => {
const { error } = await supabase.auth.signOut();
if (error) {
toast.error('Logout failed');
console.error('Error during logout:', error);
return;
}
toast.success('Signed out!');
setIsAuthenticated(false);
setRepoData(null);
await logout();
};

const ProtectedLayout = () => (
<div className="flex">
<Sidebar
isOpen={isSidebarOpen}
setIsOpen={setIsSidebarOpen}
onLogout={handleLogout}
/>
<main
className={`transition-all duration-300 flex-1 ${
isSidebarOpen ? 'ml-64' : 'ml-20'
}`}
>
<div className="p-8">
<AnimatePresence mode="wait">
<Outlet />
</AnimatePresence>
</div>
</main>
</div>
);

return (
<Router>
Expand All @@ -133,7 +65,7 @@ function App() {
isAuthenticated ? (
<Navigate to="/" replace />
) : (
<LoginPage onLogin={handleLogin} />
<LoginPage />
)
}
/>
Expand All @@ -157,7 +89,15 @@ function App() {
<Route
path="/"
element={
isAuthenticated ? <ProtectedLayout /> : <Navigate to="/login" replace />
isAuthenticated ? (
<ProtectedLayout
isSidebarOpen={isSidebarOpen}
setIsSidebarOpen={setIsSidebarOpen}
handleLogout={handleLogout}
/>
) : (
<Navigate to="/login" replace />
)
}
>
<Route index element={<LandingPage setRepoData={setRepoData} />} />
Expand Down
Loading