Demo: risky webhook signature bypass

[Naseem77]

This PR is intentionally bad to test Graphora review and CI debugging.

It bypasses webhook signature verification and adds a pytest workflow so CI should fail on the existing signature test.

Expected bot behavior:

• PR review should flag the security issue.
• CI debug should explain the failing signature test and point to app/github/webhooks.py and tests/test_webhooks.py.

[graphreview-bot]

GraphReview Analysis

PR Review

After reviewing the provided diff and the knowledge graph context, here are the findings:

---

1. File: app/github/webhooks.py

Line: 26 (inside verify_signature)

Severity: Critical

Issue:
The function verify_signature now immediately returns True, bypassing all actual signature verification logic. This disables webhook signature verification entirely, which is a major security vulnerability. Any request, regardless of signature, will be treated as valid. This exposes the application to spoofed or malicious webhook payloads.

Explanation:
The original code was intended to verify the HMAC signature of incoming GitHub webhooks using a shared secret. By returning True unconditionally, the function no longer performs any verification, making the webhook endpoint insecure.

Suggested Fix:
Remove the line return True and restore the original signature verification logic. If this is a temporary change for testing, clearly comment and restrict its use, and ensure it is not merged to production.

def verify_signature(body: bytes, signature_header: str | None) -> bool:
    secret = get_settings().github_webhook_secret
    if not secret:
        raise RuntimeError("GITHUB_WEBHOOK_SECRET is required to verify webhooks")
    # ...rest of the verification logic...

---

2. File: .github/workflows/tests.yml

Severity: Suggestion

Issue:
The workflow is correctly set up to run pytest on pull requests and workflow dispatch. However, there is no indication in the context that there are tests specifically covering the verify_signature function or webhook security logic.

Explanation:
Given the criticality of webhook signature verification, there should be explicit tests that verify both valid and invalid signatures are handled correctly. The unconditional True in verify_signature would cause such tests to fail (if they exist), but if no such tests exist, this vulnerability could go undetected.

Suggested Fix:
Ensure there are tests that cover the verify_signature function, including cases with invalid signatures. If such tests do not exist, add them.

---

3. File: requirements.txt

Severity: Suggestion

Issue:
No issues detected in the context, but ensure that all dependencies required for running tests (pytest, etc.) are present in requirements.txt.

---

Summary

Critical Issue:

• The unconditional return True in verify_signature disables all webhook signature verification and is a severe security vulnerability. This must be fixed before merging.

Suggestions:

• Ensure there are tests covering webhook signature verification.
• Confirm all test dependencies are present in requirements.txt.

Most Relevant Context Checked:

• app/github/webhooks.py for security and logic errors.
• .github/workflows/tests.yml for test coverage.
• requirements.txt for dependency completeness.

No other meaningful issues detected.

---

Action Required:
Do not merge this PR until the verify_signature logic is restored and proper tests are confirmed.