Skip to content

Fix GraphQL injection and webhook verification bypass #283

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
SshauryaaRocks19 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix GraphQL injection and webhook verification bypass #283

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
32b1bc2
Fix: Ensure webhook signature verification fails when no secret is co…
SshauryaaRocks19 Mar 14, 2026
2f99a8f
Refactor GraphQL PR fetching to use variables and ensure only valid d…
SshauryaaRocks19 Mar 14, 2026
832382d
Fix: Prevent PR data upsert and cache invalidation when fetched data …
SshauryaaRocks19 Mar 14, 2026
66faafe
fix: Return a 500 error response when GitHub PR data fails to be fetc…
SshauryaaRocks19 Mar 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
81 changes: 45 additions & 36 deletions src/github_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,14 +376,11 @@ async def fetch_multiple_prs_batch(prs_to_fetch, token=None):
for batch_start in range(0, len(prs_to_fetch), MAX_PRS_PER_BATCH):
batch = prs_to_fetch[batch_start:batch_start + MAX_PRS_PER_BATCH]

# Build GraphQL query with aliases for each PR
# We'll fetch essential PR data in one query
query_parts = []
for i, (owner, repo, pr_number) in enumerate(batch):
alias = f"pr{i}"
query_parts.append(f"""
{alias}: repository(owner: "{owner}", name: "{repo}") {{
pullRequest(number: {pr_number}) {{
var_decls = []
variables = {}

pr_fields = """
title
state
isDraft
Expand All @@ -392,49 +389,62 @@ async def fetch_multiple_prs_batch(prs_to_fetch, token=None):
mergeable
mergeStateStatus
changedFiles
commits {{
commits {
totalCount
}}
author {{
}
author {
login
avatarUrl
}}
baseRepository {{
owner {{
}
baseRepository {
owner {
avatarUrl
}}
}}
}
}
headRefOid
baseRefName
headRefName
headRepository {{
owner {{
headRepository {
owner {
login
}}
}}
reviewThreads(first: 100) {{
nodes {{
}
}
reviewThreads(first: 100) {
nodes {
isResolved
}}
pageInfo {{
}
pageInfo {
hasNextPage
}}
}}
reviews(first: 100) {{
nodes {{
}
}
reviews(first: 100) {
nodes {
state
submittedAt
author {{
author {
login
avatarUrl
}}
}}
}}
}}
}
}
}
"""

for i, (owner, repo, pr_number) in enumerate(batch):
var_decls.extend([
f"$owner{i}: String!",
f"$repo{i}: String!",
f"$pr{i}: Int!"
])
variables[f"owner{i}"] = owner
variables[f"repo{i}"] = repo
variables[f"pr{i}"] = pr_number
query_parts.append(f"""
pr{i}: repository(owner: $owner{i}, name: $repo{i}) {{
pullRequest(number: $pr{i}) {{{pr_fields}}}
}}
""")

query = "query { " + " ".join(query_parts) + " }"
query = f"query({', '.join(var_decls)}) {{ {' '.join(query_parts)} }}"

headers = {
'Accept': 'application/vnd.github+json',
Expand All @@ -446,11 +456,10 @@ async def fetch_multiple_prs_batch(prs_to_fetch, token=None):
headers['Authorization'] = f'Bearer {token}'

try:
# Make GraphQL request
options = to_js({
"method": "POST",
"headers": headers,
"body": json.dumps({"query": query})
"body": json.dumps({"query": query, "variables": variables})
}, dict_converter=Object.fromEntries)

response = await fetch(graphql_url, options)
Expand Down Expand Up @@ -806,7 +815,7 @@ async def verify_github_signature(request, payload_body, secret):
if not secret:
# If no secret is configured, skip verification (development mode)
print("WARNING: Webhook secret not configured - skipping signature verification")
return True
return False

signature_header = request.headers.get('x-hub-signature-256')
if not signature_header:
Expand Down
6 changes: 3 additions & 3 deletions src/handlers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1077,7 +1077,7 @@ async def handle_github_webhook(request, env):
# Fetch fresh PR data and add to tracking
webhook_token = getattr(env, 'GITHUB_TOKEN', None)
fetched_pr_data = await fetch_pr_data(repo_owner, repo_name, pr_number, webhook_token)
if fetched_pr_data:
if fetched_pr_data and not fetched_pr_data.get('not_found') and not fetched_pr_data.get('not_modified'):
await upsert_pr(db, pr_url, repo_owner, repo_name, pr_number, fetched_pr_data)

# Get the newly created PR ID
Expand Down Expand Up @@ -1152,7 +1152,7 @@ async def handle_github_webhook(request, env):
# Fetch fresh PR data
webhook_token = getattr(env, 'GITHUB_TOKEN', None)
fetched_pr_data = await fetch_pr_data(repo_owner, repo_name, pr_number, webhook_token)
if fetched_pr_data:
if fetched_pr_data and not fetched_pr_data.get('not_found') and not fetched_pr_data.get('not_modified'):
await upsert_pr(db, pr_url, repo_owner, repo_name, pr_number, fetched_pr_data)
# Invalidate caches
await invalidate_readiness_cache(env, pr_id)
Expand All @@ -1175,7 +1175,7 @@ async def handle_github_webhook(request, env):
# Fetch fresh PR data
webhook_token = getattr(env, 'GITHUB_TOKEN', None)
fetched_pr_data = await fetch_pr_data(repo_owner, repo_name, pr_number, webhook_token)
if fetched_pr_data:
if fetched_pr_data and not fetched_pr_data.get('not_found') and not fetched_pr_data.get('not_modified'):
await upsert_pr(db, pr_url, repo_owner, repo_name, pr_number, fetched_pr_data)
# Invalidate caches to force fresh analysis
await invalidate_readiness_cache(env, pr_id)
Expand Down
Loading