include unit tests for forgejo event; fixes

Update packit_service/worker/reporting/reporters/base.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update packit_service/service/api/webhooks.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update packit_service/worker/parser.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Update packit_service/events/forgejo/pr.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

pre-commit and mypy fixes

test(data): add Forgejo webhooks for tests

include unit tests for forgejo event; fixes

add check for prioritising Forgejo before parsing events; fixes

pre-commit

remove redundant entries in parser list; fix bug

Apply suggestion from @gemini-code-assist[bot]

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

implement validate_token for ForgejoWebhooks; correct the pr.Action impl for Forgejo; fix a problem in parser that could have caused keynotfound errors.

implement validate_token for ForgejoWebhooks; correct the pr.Action impl for Forgejo; fix a problem in parser that could have caused keynotfound errors.

Implement Forgejo event handling in Packit Service.

Author: mynk8

packit_service/events/__init__.py

@@ -21,6 +21,7 @@
 __all__ = [
     abstract.__name__,
     anitya.__name__,
+    forgejo.__name__,
     github.__name__,
     gitlab.__name__,
     forgejo.__name__,

packit_service/events/abstract/comment.py

@@ -152,6 +152,7 @@ def __init__(
         tag_name: str = "",
         comment_object: Optional[Comment] = None,
         dist_git_project_url=None,
+        commit_sha: Optional[str] = None,
     ) -> None:
         super().__init__(
             project_url=project_url,
@@ -168,7 +169,7 @@ def __init__(
 
         # Lazy properties
         self._tag_name = tag_name
-        self._commit_sha: Optional[str] = None
+        self._commit_sha: Optional[str] = commit_sha
         self._comment_object = comment_object
         self._issue_object: Optional[OgrIssue] = None
 

packit_service/events/forgejo/__init__.py

@@ -3,4 +3,4 @@
 
 from . import abstract, issue, pr, push
 
-__all__ = [abstract.__name__, push.__name__, issue.__name__, pr.__name__]
+__all__ = [abstract.__name__, issue.__name__, pr.__name__, push.__name__]

packit_service/events/forgejo/issue.py

@@ -1,8 +1,6 @@
 # Copyright Contributors to the Packit project.
 # SPDX-License-Identifier: MIT
 
-# SPDX-License-Identifier: MIT
-
 from typing import Optional
 
 from ogr.abstract import Comment as OgrComment
@@ -50,43 +48,7 @@ def __init__(
     def event_type(cls) -> str:
         return "forgejo.issue.Comment"
 
-    @property
-    def tag_name(self):
-        """
-        For Forgejo issue comments, return the tag_name passed in constructor
-        without making API calls to avoid authentication issues.
-        """
-        return self._tag_name
-
-    @tag_name.setter
-    def tag_name(self, value: str) -> None:
-        self._tag_name = value
-
-    @property
-    def commit_sha(self) -> Optional[str]:
-        """
-        For Forgejo issue comments, return the commit_sha passed in constructor
-        without making API calls to avoid authentication issues.
-        """
-        return self._commit_sha
-
-    @commit_sha.setter
-    def commit_sha(self, value: Optional[str]) -> None:
-        self._commit_sha = value
-
     def get_dict(self, default_dict: Optional[dict] = None) -> dict:
-        """
-        Override get_dict to avoid accessing properties that make API calls.
-        """
-        # Get the basic dict from CommentEvent, not from Issue to avoid tag_name access
-        from ..abstract.comment import CommentEvent
-
-        result = CommentEvent.get_dict(self, default_dict=default_dict)
-
-        # Add the specific fields we need without triggering API calls
+        result = super().get_dict()
         result["action"] = self.action.value
-        result["issue_id"] = self.issue_id
-        result["tag_name"] = self._tag_name  # Use the private attribute directly
-        result["commit_sha"] = self._commit_sha  # Use the private attribute directly
-
         return result

packit_service/events/forgejo/pr.py

@@ -36,12 +36,10 @@ def __init__(
         self.base_ref = base_ref
         self.target_repo_namespace = target_repo_namespace
         self.target_repo_name = target_repo_name
-        self.commit_sha = commit_sha
-        self.commit_sha_before = commit_sha_before
         self.actor = actor
         self.identifier = str(pr_id)
-        self._pr_id = pr_id
-        self.git_ref = None  # use pr_id for checkout
+        self.commit_sha = commit_sha
+        self.commit_sha_before = commit_sha_before
         self.body = body
 
     def get_dict(self, default_dict: Optional[dict] = None) -> dict:
@@ -55,8 +53,8 @@ def event_type(cls) -> str:
 
     def get_base_project(self) -> GitProject:
         return self.project.service.get_project(
-            namespace=self.target_repo_namespace,
-            repo=self.target_repo_name,
+            namespace=self.base_repo_namespace,
+            repo=self.base_repo_name,
         )
 
 
@@ -94,7 +92,6 @@ def __init__(
         self.actor = actor
         self.identifier = str(pr_id)
         self.git_ref = None
-        self.pr_id = pr_id
 
     @classmethod
     def event_type(cls) -> str:
@@ -116,6 +113,6 @@ def get_dict(self, default_dict: Optional[dict] = None) -> dict:
 
     def get_base_project(self) -> GitProject:
         return self.project.service.get_project(
-            namespace=self.target_repo_namespace,
-            repo=self.target_repo_name,
+            namespace=self.base_repo_namespace,
+            repo=self.base_repo_name,
         )

packit_service/service/api/webhooks.py

@@ -373,6 +373,9 @@ class ForgejoWebhook(Resource):
     @ns.response(HTTPStatus.UNAUTHORIZED.value, "X-Forgejo-Signature validation failed")
     @ns.expect(ping_payload_forgejo)
     def post(self):
+        """
+        A webhook used by Packit-as-a-Service Forgejo hook.
+        """
         msg = request.json
 
         if not msg:
@@ -385,17 +388,17 @@ def post(self):
             logger.debug(f"/webhooks/forgejo received ping event: {msg['hook']}")
             forgejo_webhook_calls.labels(result="pong", process_id=os.getpid()).inc()
             return "Pong!", HTTPStatus.OK
-        # TODO
-        # try:
-        #     self.validate_token()
-        # except ValidationFailed as exc:
-        #     logger.info(f"/webhooks/forgejo {exc}")
-        #     forgejo_webhook_calls.labels(
-        #         result="invalid_signature",
-        #         process_id=os.getpid(),
-        #     ).inc()
-        #     return str(exc), HTTPStatus.UNAUTHORIZED
-        #
+
+        try:
+            self.validate_token()
+        except ValidationFailed as exc:
+            logger.info(f"/webhooks/forgejo {exc}")
+            forgejo_webhook_calls.labels(
+                result="invalid_signature",
+                process_id=os.getpid(),
+            ).inc()
+            return str(exc), HTTPStatus.UNAUTHORIZED
+
         if not self.interested(msg):
             forgejo_webhook_calls.labels(
                 result="not_interested",
@@ -419,39 +422,44 @@ def validate_token(self):
         """
         Validate the Forgejo webhook signature.
         The signature is a direct SHA256 HMAC hex digest of the raw request body
-        using the webhook secret as the key, in a similar fashion to Github.
-
+        using the webhook secret as the key, in a similar fashion to GitHub.
         """
         if "X-Forgejo-Signature" not in request.headers:
+            if config.validate_webhooks:
+                msg = "X-Forgejo-Signature not in request.headers"
+                logger.warning(msg)
+                raise ValidationFailed(msg)
+
+            # don't validate signatures when testing locally
             logger.debug("Ain't validating signatures.")
             return
 
-        if not (webhook_secret := getenv("WEBHOOK_SECRET")):
+        if not (webhook_secret := config.webhook_secret):
             msg = "'webhook_secret' not specified in the config."
             logger.error(msg)
             raise ValidationFailed(msg)
 
-        # Get raw payload
-
         payload = request.get_data()
         if not payload:
             msg = "No payload received."
             logger.error(msg)
             raise ValidationFailed(msg)
 
+        # Calculate payload signature using HMAC-SHA256
         data_hmac = hmac.new(webhook_secret.encode(), msg=payload, digestmod=sha256)
         payload_signature = data_hmac.hexdigest()
-        header_sig = request.headers["X-Forgejo-Signature"]
+        header_signature = request.headers["X-Forgejo-Signature"]
 
-        if header_sig != payload_signature:
+        if not hmac.compare_digest(header_signature, payload_signature):
             msg = "Payload signature validation failed."
-
             logger.warning(msg)
             logger.debug(
-                f"X-Forgejo-Signature: {header_sig!r} != computed: {payload_signature}",
+                f"X-Forgejo-Signature: {header_signature!r} != computed: {payload_signature}",
             )
             raise ValidationFailed(msg)
 
+        logger.debug("Payload signature is OK.")
+
     @staticmethod
     def interested(msg):
         """
@@ -475,7 +483,7 @@ def interested(msg):
             "release": action == "published",
             "issues": action in {"opened", "edited", "closed", "reopened"},
             "issue_comment": action in {"created", "edited"},
-            "pull_request": action in {"opened", "edited", "closed", "reopened", "synchronize"},
+            "pull_request": action in {"opened", "reopened", "synchronize"},
         }
 
         _interested = interests.get(event or "", False)