GitHub OAuth Security Enhancement

[ShashankFC]

Test 4

Summary by CodeRabbit

• New Features

  • Added OAuth-based GitHub login flow integration for seamless authentication

• Bug Fixes

  • Enhanced installation validation to prevent mismatches between authenticated users and installations
  • Improved error messaging and handling for invalid installation requests, conflicts, and pending deletions

• Tests

  • Expanded test coverage for GitHub installation flows and error scenarios

_{✏️ Tip: You can customize this high-level summary in your review settings.}

---

Replicated from ai-code-review-evaluation/sentry-coderabbit#4

[ShashankFC]

@cubic-dev-ai review this pull request

[cubic-dev-ai]

@cubic-dev-ai review this pull request

@ShashankFC I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

2 issues found across 3 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="tests/sentry/integrations/github/test_integration.py">

<violation number="1" location="tests/sentry/integrations/github/test_integration.py:238">
P2: `test_installation_not_found` is missing the init step (`self.client.get(self.init_path)`) that starts the OAuth flow and stores state in the session. Without it, the OAuth state validation fails before the installation 404 is ever reached. The test passes for the wrong reason — it asserts on `"Invalid installation request."` which comes from the state mismatch, not the 404. The `responses.replace(..., status=404)` is never exercised.</violation>
</file>

<file name="src/sentry/integrations/github/integration.py">

<violation number="1" location="src/sentry/integrations/github/integration.py:503">
P1: Unguarded dictionary access `integration.metadata["sender"]["login"]` will raise `KeyError` if the integration was created without a `sender` in its metadata (since `build_integration` only conditionally sets it). This crashes the request with a 500 instead of showing the error page. Use `.get()` for safe access.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P1: Unguarded dictionary access integration.metadata["sender"]["login"] will raise KeyError if the integration was created without a sender in its metadata (since build_integration only conditionally sets it). This crashes the request with a 500 instead of showing the error page. Use .get() for safe access.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/sentry/integrations/github/integration.py, line 503:

<comment>Unguarded dictionary access `integration.metadata["sender"]["login"]` will raise `KeyError` if the integration was created without a `sender` in its metadata (since `build_integration` only conditionally sets it). This crashes the request with a 500 instead of showing the error page. Use `.get()` for safe access.</comment>

<file context>
@@ -374,57 +465,43 @@ def dispatch(self, request: Request, pipeline: Pipeline) -> HttpResponse:
+        # Check that the authenticated GitHub user is the same as who installed the app.
+        if (
+            pipeline.fetch_state("github_authenticated_user")
+            != integration.metadata["sender"]["login"]
+        ):
+            return error(request, self.active_organization)
</file context>

[cubic-dev-ai]

P2: test_installation_not_found is missing the init step (self.client.get(self.init_path)) that starts the OAuth flow and stores state in the session. Without it, the OAuth state validation fails before the installation 404 is ever reached. The test passes for the wrong reason — it asserts on "Invalid installation request." which comes from the state mismatch, not the 404. The responses.replace(..., status=404) is never exercised.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/sentry/integrations/github/test_integration.py, line 238:

<comment>`test_installation_not_found` is missing the init step (`self.client.get(self.init_path)`) that starts the OAuth flow and stores state in the session. Without it, the OAuth state validation fails before the installation 404 is ever reached. The test passes for the wrong reason — it asserts on `"Invalid installation request."` which comes from the state mismatch, not the 404. The `responses.replace(..., status=404)` is never exercised.</comment>

<file context>
@@ -218,14 +229,32 @@ def assert_setup_flow(self):
+            == "client_id=github-client-id&state=9cae5e88803f35ed7970fc131e6e65d3&redirect_uri=http://testserver/extensions/github/setup/"
+        )
+
+        resp = self.client.get(
+            "{}?{}".format(
+                self.setup_path,
</file context>