From 8e3d79f24697f70d33e897ff8169c87fc2255770 Mon Sep 17 00:00:00 2001 From: Peter Hutterer <peter.hutterer@who-t.net> Date: Thu, 23 Mar 2023 13:57:57 +1000 Subject: [PATCH] Only allow issue events when handling an issue Fail if the hook is triggered by some other Webhook. No reason we can't implement these but there's a good chance the JSON parser isn't happy with the structure, so let's prevent this altogether. --- damspam/__init__.py | 12 +++++++ data/comment-event.json | 70 ++++++++++++++++++++++++++++++++++++++++ data/push-event.json | 71 +++++++++++++++++++++++++++++++++++++++++ tests/test_damspam.py | 22 ++++++++++++- 4 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 data/comment-event.json create mode 100644 data/push-event.json diff --git a/damspam/__init__.py b/damspam/__init__.py index b06cf72..e4f8f1f 100644 --- a/damspam/__init__.py +++ b/damspam/__init__.py @@ -194,6 +194,18 @@ class WebhookIssueEvent: @classmethod def from_json(cls, json: dict) -> "WebhookIssueEvent": payload = Payload.from_json(json) + + # some webhooks have event_type, others have event_name. yay + try: + event_type = payload.get("event_type").value + except InvalidPayload: + raise InvalidPayload("payload does not look like an issue event") + else: + if event_type not in ["issue", "confidential_issue"]: + raise InvalidPayload( + f"event_type '{event_type}' is not permitted for issue events" + ) + project = WebhookGitlabProject.from_json(payload.get("project")) user = WebhookGitlabUser.from_json(payload.get("user")) issue = WebhookGitlabIssue.from_json(payload.get("object_attributes")) diff --git a/data/comment-event.json b/data/comment-event.json new file mode 100644 index 0000000..b3f0890 --- /dev/null +++ b/data/comment-event.json @@ -0,0 +1,70 @@ +{ + "object_kind": "note", + "event_type": "note", + "user": { + "id": 1, + "name": "Administrator", + "username": "root", + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon", + "email": "admin@example.com" + }, + "project_id": 5, + "project":{ + "id": 5, + "name":"Gitlab Test", + "description":"Aut reprehenderit ut est.", + "web_url":"http://example.com/gitlabhq/gitlab-test", + "avatar_url":null, + "git_ssh_url":"git@example.com:gitlabhq/gitlab-test.git", + "git_http_url":"http://example.com/gitlabhq/gitlab-test.git", + "namespace":"GitlabHQ", + "visibility_level":20, + "path_with_namespace":"gitlabhq/gitlab-test", + "default_branch":"master", + "homepage":"http://example.com/gitlabhq/gitlab-test", + "url":"http://example.com/gitlabhq/gitlab-test.git", + "ssh_url":"git@example.com:gitlabhq/gitlab-test.git", + "http_url":"http://example.com/gitlabhq/gitlab-test.git" + }, + "repository":{ + "name": "Gitlab Test", + "url": "http://example.com/gitlab-org/gitlab-test.git", + "description": "Aut reprehenderit ut est.", + "homepage": "http://example.com/gitlab-org/gitlab-test" + }, + "object_attributes": { + "id": 1243, + "note": "This is a commit comment. How does this work?", + "noteable_type": "Commit", + "author_id": 1, + "created_at": "2015-05-17 18:08:09 UTC", + "updated_at": "2015-05-17 18:08:09 UTC", + "project_id": 5, + "attachment":null, + "line_code": "bec9703f7a456cd2b4ab5fb3220ae016e3e394e3_0_1", + "commit_id": "cfe32cf61b73a0d5e9f13e774abde7ff789b1660", + "noteable_id": null, + "system": false, + "st_diff": { + "diff": "--- /dev/null\n+++ b/six\n@@ -0,0 +1 @@\n+Subproject commit 409f37c4f05865e4fb208c771485f211a22c4c2d\n", + "new_path": "six", + "old_path": "six", + "a_mode": "0", + "b_mode": "160000", + "new_file": true, + "renamed_file": false, + "deleted_file": false + }, + "url": "http://example.com/gitlab-org/gitlab-test/commit/cfe32cf61b73a0d5e9f13e774abde7ff789b1660#note_1243" + }, + "commit": { + "id": "cfe32cf61b73a0d5e9f13e774abde7ff789b1660", + "message": "Add submodule\n\nSigned-off-by: Example User \u003cuser@example.com.com\u003e\n", + "timestamp": "2014-02-27T10:06:20+02:00", + "url": "http://example.com/gitlab-org/gitlab-test/commit/cfe32cf61b73a0d5e9f13e774abde7ff789b1660", + "author": { + "name": "Example User", + "email": "user@example.com" + } + } +} diff --git a/data/push-event.json b/data/push-event.json new file mode 100644 index 0000000..4389e11 --- /dev/null +++ b/data/push-event.json @@ -0,0 +1,71 @@ +{ + "object_kind": "push", + "event_name": "push", + "before": "95790bf891e76fee5e1747ab589903a6a1f80f22", + "after": "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + "ref": "refs/heads/master", + "checkout_sha": "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + "user_id": 4, + "user_name": "John Smith", + "user_username": "jsmith", + "user_email": "john@example.com", + "user_avatar": "https://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=8://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=80", + "project_id": 15, + "project":{ + "id": 15, + "name":"Diaspora", + "description":"", + "web_url":"http://example.com/mike/diaspora", + "avatar_url":null, + "git_ssh_url":"git@example.com:mike/diaspora.git", + "git_http_url":"http://example.com/mike/diaspora.git", + "namespace":"Mike", + "visibility_level":0, + "path_with_namespace":"mike/diaspora", + "default_branch":"master", + "homepage":"http://example.com/mike/diaspora", + "url":"git@example.com:mike/diaspora.git", + "ssh_url":"git@example.com:mike/diaspora.git", + "http_url":"http://example.com/mike/diaspora.git" + }, + "repository":{ + "name": "Diaspora", + "url": "git@example.com:mike/diaspora.git", + "description": "", + "homepage": "http://example.com/mike/diaspora", + "git_http_url":"http://example.com/mike/diaspora.git", + "git_ssh_url":"git@example.com:mike/diaspora.git", + "visibility_level":0 + }, + "commits": [ + { + "id": "b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327", + "message": "Update Catalan translation to e38cb41.\n\nSee https://gitlab.com/gitlab-org/gitlab for more information", + "title": "Update Catalan translation to e38cb41.", + "timestamp": "2011-12-12T14:27:31+02:00", + "url": "http://example.com/mike/diaspora/commit/b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327", + "author": { + "name": "Jordi Mallach", + "email": "jordi@softcatala.org" + }, + "added": ["CHANGELOG"], + "modified": ["app/controller/application.rb"], + "removed": [] + }, + { + "id": "da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + "message": "fixed readme", + "title": "fixed readme", + "timestamp": "2012-01-03T23:36:29+02:00", + "url": "http://example.com/mike/diaspora/commit/da1560886d4f094c3e6c9ef40349f7d38b5d27d7", + "author": { + "name": "GitLab dev user", + "email": "gitlabdev@dv6700.(none)" + }, + "added": ["CHANGELOG"], + "modified": ["app/controller/application.rb"], + "removed": [] + } + ], + "total_commits_count": 4 +} diff --git a/tests/test_damspam.py b/tests/test_damspam.py index b7da427..da6504e 100644 --- a/tests/test_damspam.py +++ b/tests/test_damspam.py @@ -28,6 +28,16 @@ def issue_event() -> dict: return json.load(open("data/issue-event.json")) +@pytest.fixture +def push_event() -> dict: + return json.load(open("data/push-event.json")) + + +@pytest.fixture +def comment_event() -> dict: + return json.load(open("data/comment-event.json")) + + @pytest.mark.parametrize("readonly", (True, False)) def test_if_not_readonly(caplog, readonly): caplog.set_level(logging.INFO, "damspam") @@ -78,7 +88,7 @@ def test_payload(): payload = Payload.from_json({}) -def test_webhook_abstractions(issue_event: dict): +def test_webhook_abstractions(issue_event: dict, push_event: dict, comment_event: dict): event = WebhookIssueEvent.from_json(issue_event) assert event.project.id == 1 assert event.project.name == "Gitlab Test" @@ -99,6 +109,16 @@ def test_webhook_abstractions(issue_event: dict): assert event.removed_labels[0].id == 206 assert event.removed_labels[0].title == "API" + with pytest.raises(InvalidPayload) as exc_info: + WebhookIssueEvent.from_json(push_event) + assert exc_info.type is InvalidPayload + assert "does not look like an issue event" in exc_info.value.message + + with pytest.raises(InvalidPayload) as exc_info: + WebhookIssueEvent.from_json(comment_event) + assert exc_info.type is InvalidPayload + assert "not permitted for issue events" in exc_info.value.message + @pytest.mark.parametrize("readonly", (True, False)) @patch("gitlab.Gitlab") -- GitLab