From abadaf4ede94f67d535c5df077af256b4a9a4913 Mon Sep 17 00:00:00 2001 From: Rohit Date: Fri, 11 Apr 2025 11:34:17 -0400 Subject: [PATCH 1/4] initial commit --- webhook_handlers/constants.py | 1 + webhook_handlers/views/github.py | 23 ++++++++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/webhook_handlers/constants.py b/webhook_handlers/constants.py index 189e7520f1..0f8a5a6014 100644 --- a/webhook_handlers/constants.py +++ b/webhook_handlers/constants.py @@ -3,6 +3,7 @@ class GitHubHTTPHeaders: DELIVERY_TOKEN = "HTTP_X_GITHUB_DELIVERY" SIGNATURE = "HTTP_X_HUB_SIGNATURE" SIGNATURE_256 = "HTTP_X_HUB_SIGNATURE_256" + HOOK_INSTALLATION_TARGET_ID = "HTTP_X_GITHUB_HOOK_INSTALLATION_TARGET_ID" class GitHubWebhookEvents: diff --git a/webhook_handlers/views/github.py b/webhook_handlers/views/github.py index b3d3a56112..6e9a4e1a89 100644 --- a/webhook_handlers/views/github.py +++ b/webhook_handlers/views/github.py @@ -365,6 +365,9 @@ def status(self, request, *args, **kwargs): return Response() def pull_request(self, request, *args, **kwargs): + if request.headers.get(GitHubHTTPHeaders.HOOK_INSTALLATION_TARGET_ID): + return self.check_codecov_ai_auto_enabled_reviews(request) + repo = self._get_repo(request) if not repo.active: @@ -398,6 +401,18 @@ def pull_request(self, request, *args, **kwargs): return Response() + def check_codecov_ai_auto_enabled_reviews(self, request): + org = Owner.objects.get( + service=self.service_name, + service_id=request.data["organization"]["id"], + ) + auto_review_enabled = org.yaml.get("ai_pr_review", {}).get("auto_review", False) + return Response( + data={ + "auto_review_enabled": auto_review_enabled, + } + ) + def _decide_app_name(self, ghapp: GithubAppInstallation) -> str: """Possibly updated the name of a GithubAppInstallation that has been fetched from DB or created. Only the real default installation maybe use the name `GITHUB_APP_INSTALLATION_DEFAULT_NAME` @@ -520,9 +535,11 @@ def _handle_installation_events( AmplitudeEventPublisher().publish( "App Installed", { - "user_ownerid": installer.ownerid - if installer is not None - else owner.ownerid, + "user_ownerid": ( + installer.ownerid + if installer is not None + else owner.ownerid + ), "ownerid": owner.ownerid, }, ) From a388eec1154aba20b4508f186efd3dd24e64390c Mon Sep 17 00:00:00 2001 From: "codecov-ai[bot]" <156709835+codecov-ai[bot]@users.noreply.github.com> Date: Mon, 14 Apr 2025 12:20:43 -0400 Subject: [PATCH 2/4] Add Tests for PR#1285 (#1286) Co-authored-by: codecov-ai[bot] <156709835+codecov-ai[bot]@users.noreply.github.com> Co-authored-by: Rohit --- webhook_handlers/tests/test_github.py | 105 +++++++++++++++++++++++++- webhook_handlers/views/github.py | 10 ++- 2 files changed, 111 insertions(+), 4 deletions(-) diff --git a/webhook_handlers/tests/test_github.py b/webhook_handlers/tests/test_github.py index 9e11cc7e5b..b7c7413217 100644 --- a/webhook_handlers/tests/test_github.py +++ b/webhook_handlers/tests/test_github.py @@ -47,6 +47,7 @@ def __getitem__(self, key): WEBHOOK_SECRET = b"testixik8qdauiab1yiffydimvi72ekq" DEFAULT_APP_ID = 1234 +AI_FEATURES_GH_APP_ID = 9999 class GithubWebhookHandlerTests(APITestCase): @@ -58,16 +59,23 @@ def inject_mocker(request, mocker): def mock_webhook_secret(self, mocker): mock_config_helper(mocker, configs={"github.webhook_secret": WEBHOOK_SECRET}) + @pytest.fixture(autouse=True) + def mock_ai_features_app_id(self, mocker): + mock_config_helper( + mocker, configs={"github.ai_features_app_id": AI_FEATURES_GH_APP_ID} + ) + @pytest.fixture(autouse=True) def mock_default_app_id(self, mocker): mock_config_helper(mocker, configs={"github.integration.id": DEFAULT_APP_ID}) - def _post_event_data(self, event, data={}): + def _post_event_data(self, event, data={}, app_id=DEFAULT_APP_ID): return self.client.post( reverse("github-webhook"), **{ GitHubHTTPHeaders.EVENT: event, GitHubHTTPHeaders.DELIVERY_TOKEN: uuid.UUID(int=5), + GitHubHTTPHeaders.HOOK_INSTALLATION_TARGET_ID: app_id, GitHubHTTPHeaders.SIGNATURE_256: "sha256=" + hmac.new( WEBHOOK_SECRET, @@ -1420,3 +1428,98 @@ def test_repo_creation_doesnt_crash_for_forked_repo(self): ) assert owner.repository_set.filter(name="testrepo").exists() + + def test_check_codecov_ai_auto_enabled_reviews_enabled(self): + # Create an organization with AI PR review enabled + org_with_ai_enabled = OwnerFactory( + service=Service.GITHUB.value, yaml={"ai_pr_review": {"auto_review": True}} + ) + + response = self._post_event_data( + event=GitHubWebhookEvents.PULL_REQUEST, + data={ + "action": "pull_request", + "repository": { + "id": 506003, + "name": "testrepo", + "private": False, + "default_branch": "main", + "owner": {"id": org_with_ai_enabled.service_id}, + "fork": True, + "parent": { + "name": "mainrepo", + "language": "python", + "id": 7940284, + "private": False, + "default_branch": "main", + "owner": {"id": 8495712939, "login": "alogin"}, + }, + }, + }, + app_id=AI_FEATURES_GH_APP_ID, + ) + assert response.data == {"auto_review_enabled": True} + + def test_check_codecov_ai_auto_enabled_reviews_disabled(self): + # Test with AI PR review disabled + org_with_ai_disabled = OwnerFactory( + service=Service.GITHUB.value, yaml={"ai_pr_review": {"auto_review": False}} + ) + + response = self._post_event_data( + event=GitHubWebhookEvents.PULL_REQUEST, + data={ + "action": "pull_request", + "repository": { + "id": 506004, + "name": "testrepo2", + "private": False, + "default_branch": "main", + "owner": {"id": org_with_ai_disabled.service_id}, + }, + }, + app_id=AI_FEATURES_GH_APP_ID, + ) + assert response.data == {"auto_review_enabled": False} + + def test_check_codecov_ai_auto_enabled_reviews_no_config(self): + # Test with no yaml config + org_with_no_config = OwnerFactory(service=Service.GITHUB.value, yaml={}) + + response = self._post_event_data( + event=GitHubWebhookEvents.PULL_REQUEST, + data={ + "action": "pull_request", + "repository": { + "id": 506005, + "name": "testrepo3", + "private": False, + "default_branch": "main", + "owner": {"id": org_with_no_config.service_id}, + }, + }, + app_id=AI_FEATURES_GH_APP_ID, + ) + assert response.data == {"auto_review_enabled": False} + + def test_check_codecov_ai_auto_enabled_reviews_partial_config(self): + # Test with partial yaml config + org_with_partial_config = OwnerFactory( + service=Service.GITHUB.value, yaml={"ai_pr_review": {}} + ) + + response = self._post_event_data( + event=GitHubWebhookEvents.PULL_REQUEST, + data={ + "action": "pull_request", + "repository": { + "id": 506006, + "name": "testrepo4", + "private": False, + "default_branch": "main", + "owner": {"id": org_with_partial_config.service_id}, + }, + }, + app_id=AI_FEATURES_GH_APP_ID, + ) + assert response.data == {"auto_review_enabled": False} diff --git a/webhook_handlers/views/github.py b/webhook_handlers/views/github.py index 6e9a4e1a89..1d278b1fdf 100644 --- a/webhook_handlers/views/github.py +++ b/webhook_handlers/views/github.py @@ -38,6 +38,7 @@ # This should probably go somewhere where it can be easily shared regexp_ci_skip = re.compile(r"\[(ci|skip| |-){3,}\]").search +AI_FEATURES_GH_APP_ID = get_config("github", "ai_features_app_id") class GithubWebhookHandler(APIView): @@ -365,7 +366,10 @@ def status(self, request, *args, **kwargs): return Response() def pull_request(self, request, *args, **kwargs): - if request.headers.get(GitHubHTTPHeaders.HOOK_INSTALLATION_TARGET_ID): + if ( + request.META.get(GitHubHTTPHeaders.HOOK_INSTALLATION_TARGET_ID, "") + == AI_FEATURES_GH_APP_ID + ): return self.check_codecov_ai_auto_enabled_reviews(request) repo = self._get_repo(request) @@ -404,8 +408,9 @@ def pull_request(self, request, *args, **kwargs): def check_codecov_ai_auto_enabled_reviews(self, request): org = Owner.objects.get( service=self.service_name, - service_id=request.data["organization"]["id"], + service_id=request.data["repository"]["owner"]["id"], ) + auto_review_enabled = org.yaml.get("ai_pr_review", {}).get("auto_review", False) return Response( data={ @@ -768,7 +773,6 @@ def post(self, request, *args, **kwargs): delivery=self.request.META.get(GitHubHTTPHeaders.DELIVERY_TOKEN), ), ) - self.validate_signature(request) if handler := getattr(self, self.event, None): From 64e5282690331e461dff46ca1acf55fb31aa6091 Mon Sep 17 00:00:00 2001 From: Rohit Date: Mon, 14 Apr 2025 12:37:07 -0400 Subject: [PATCH 3/4] Add try catch --- webhook_handlers/views/github.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/webhook_handlers/views/github.py b/webhook_handlers/views/github.py index 1d278b1fdf..d51d716cd0 100644 --- a/webhook_handlers/views/github.py +++ b/webhook_handlers/views/github.py @@ -365,11 +365,12 @@ def status(self, request, *args, **kwargs): return Response() + def _is_ai_features_request(self, request): + target_id = request.META.get(GitHubHTTPHeaders.HOOK_INSTALLATION_TARGET_ID, "") + return str(target_id) == str(AI_FEATURES_GH_APP_ID) + def pull_request(self, request, *args, **kwargs): - if ( - request.META.get(GitHubHTTPHeaders.HOOK_INSTALLATION_TARGET_ID, "") - == AI_FEATURES_GH_APP_ID - ): + if self._is_ai_features_request(request): return self.check_codecov_ai_auto_enabled_reviews(request) repo = self._get_repo(request) @@ -406,10 +407,17 @@ def pull_request(self, request, *args, **kwargs): return Response() def check_codecov_ai_auto_enabled_reviews(self, request): - org = Owner.objects.get( - service=self.service_name, - service_id=request.data["repository"]["owner"]["id"], - ) + try: + org = Owner.objects.get( + service=self.service_name, + service_id=request.data["repository"]["owner"]["id"], + ) + except Owner.DoesNotExist: + return Response( + data={ + "auto_review_enabled": False, + } + ) auto_review_enabled = org.yaml.get("ai_pr_review", {}).get("auto_review", False) return Response( From 459bbbfa20c15e5a5821ca3593ce989df6f0a963 Mon Sep 17 00:00:00 2001 From: Rohit Date: Mon, 14 Apr 2025 13:49:48 -0400 Subject: [PATCH 4/4] Update --- webhook_handlers/tests/test_github.py | 12 +++++------- webhook_handlers/views/github.py | 22 +++++++++------------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/webhook_handlers/tests/test_github.py b/webhook_handlers/tests/test_github.py index b7c7413217..09c9fb1889 100644 --- a/webhook_handlers/tests/test_github.py +++ b/webhook_handlers/tests/test_github.py @@ -61,9 +61,7 @@ def mock_webhook_secret(self, mocker): @pytest.fixture(autouse=True) def mock_ai_features_app_id(self, mocker): - mock_config_helper( - mocker, configs={"github.ai_features_app_id": AI_FEATURES_GH_APP_ID} - ) + mock_config_helper(mocker, configs={"github.ai_features_app_id": 9999}) @pytest.fixture(autouse=True) def mock_default_app_id(self, mocker): @@ -1456,7 +1454,7 @@ def test_check_codecov_ai_auto_enabled_reviews_enabled(self): }, }, }, - app_id=AI_FEATURES_GH_APP_ID, + app_id=9999, ) assert response.data == {"auto_review_enabled": True} @@ -1478,7 +1476,7 @@ def test_check_codecov_ai_auto_enabled_reviews_disabled(self): "owner": {"id": org_with_ai_disabled.service_id}, }, }, - app_id=AI_FEATURES_GH_APP_ID, + app_id=9999, ) assert response.data == {"auto_review_enabled": False} @@ -1498,7 +1496,7 @@ def test_check_codecov_ai_auto_enabled_reviews_no_config(self): "owner": {"id": org_with_no_config.service_id}, }, }, - app_id=AI_FEATURES_GH_APP_ID, + app_id=9999, ) assert response.data == {"auto_review_enabled": False} @@ -1520,6 +1518,6 @@ def test_check_codecov_ai_auto_enabled_reviews_partial_config(self): "owner": {"id": org_with_partial_config.service_id}, }, }, - app_id=AI_FEATURES_GH_APP_ID, + app_id=9999, ) assert response.data == {"auto_review_enabled": False} diff --git a/webhook_handlers/views/github.py b/webhook_handlers/views/github.py index d51d716cd0..637c41f36c 100644 --- a/webhook_handlers/views/github.py +++ b/webhook_handlers/views/github.py @@ -38,7 +38,6 @@ # This should probably go somewhere where it can be easily shared regexp_ci_skip = re.compile(r"\[(ci|skip| |-){3,}\]").search -AI_FEATURES_GH_APP_ID = get_config("github", "ai_features_app_id") class GithubWebhookHandler(APIView): @@ -53,6 +52,10 @@ class GithubWebhookHandler(APIView): service_name = "github" + @property + def ai_features_app_id(self): + return get_config("github", "ai_features_app_id") + def _inc_recv(self): action = self.request.data.get("action", "") WEBHOOKS_RECEIVED.labels( @@ -367,7 +370,7 @@ def status(self, request, *args, **kwargs): def _is_ai_features_request(self, request): target_id = request.META.get(GitHubHTTPHeaders.HOOK_INSTALLATION_TARGET_ID, "") - return str(target_id) == str(AI_FEATURES_GH_APP_ID) + return str(target_id) == str(self.ai_features_app_id) def pull_request(self, request, *args, **kwargs): if self._is_ai_features_request(request): @@ -407,17 +410,10 @@ def pull_request(self, request, *args, **kwargs): return Response() def check_codecov_ai_auto_enabled_reviews(self, request): - try: - org = Owner.objects.get( - service=self.service_name, - service_id=request.data["repository"]["owner"]["id"], - ) - except Owner.DoesNotExist: - return Response( - data={ - "auto_review_enabled": False, - } - ) + org = Owner.objects.get( + service=self.service_name, + service_id=request.data["repository"]["owner"]["id"], + ) auto_review_enabled = org.yaml.get("ai_pr_review", {}).get("auto_review", False) return Response(