From cd11ff6c332f4f49ea3576b4c1069cdbcc0d7beb Mon Sep 17 00:00:00 2001 From: Sajal Kumar Jana Date: Sat, 26 Sep 2026 22:01:01 +0530 Subject: [PATCH 1/6] fix(utility): reject malformed or unexpected webhook signatures is_valid_signature() raised instead of returning False for several headers: "sha1" with no "=" (ValueError), an unknown algorithm name (TypeError from hmac.new with digestmod=None), "new=..." (hashlib.new used as the digest), and a non-ASCII digest (TypeError from compare_digest on str). Each of these turned the CI web hook into a 500 instead of the configured abort code. Looking the algorithm up in hashlib.__dict__ also meant any hashlib digest was accepted, e.g. md5. Only accept sha1 and sha256, the hashes GitHub signs with, treat anything malformed as an invalid signature, and compare the digests as bytes. --- utility.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/utility.py b/utility.py index ecc8df7e5..9040923e5 100644 --- a/utility.py +++ b/utility.py @@ -149,20 +149,34 @@ def get_cached_web_hook_blocks() -> List[str]: return cached_web_hook_blocks +#: Hash algorithms GitHub signs web hook payloads with. +SIGNATURE_ALGORITHMS = {'sha1': hashlib.sha1, 'sha256': hashlib.sha256} + + def is_valid_signature(x_hub_signature, data, private_key): """ Re-check if the GitHub hook request got valid signature. - :param x_hub_signature: Signature to check + :param x_hub_signature: Signature to check, e.g. ``sha1=`` :type x_hub_signature: str :param data: Signature's data :type data: bytearray :param private_key: Signature's token :type private_key: str + :return: False if the signature is missing, malformed, uses a hash + GitHub does not sign with, or does not match. + :rtype: bool """ - hash_algorithm, github_signature = x_hub_signature.split('=', 1) - algorithm = hashlib.__dict__.get(hash_algorithm) + if not x_hub_signature: + return False + + hash_algorithm, separator, github_signature = x_hub_signature.partition('=') + algorithm = SIGNATURE_ALGORITHMS.get(hash_algorithm) + if not separator or algorithm is None: + return False + encoded_key = bytes(private_key, 'latin-1') mac = hmac.new(encoded_key, msg=data, digestmod=algorithm) - return hmac.compare_digest(mac.hexdigest(), github_signature) + # Compare bytes: compare_digest raises TypeError on non-ASCII str input. + return hmac.compare_digest(mac.hexdigest().encode(), github_signature.encode('utf-8', 'replace')) From 1230df5ac5b409a9edc9697e24fab12b26cf7a8b Mon Sep 17 00:00:00 2001 From: Sajal Kumar Jana Date: Sat, 26 Sep 2026 22:01:46 +0530 Subject: [PATCH 2/6] test(utility): cover webhook signature parsing --- tests/test_utility.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_utility.py b/tests/test_utility.py index db9d8fe80..2fe960524 100644 --- a/tests/test_utility.py +++ b/tests/test_utility.py @@ -48,3 +48,28 @@ def example_function(*args, **kwargs): mock_is_github_ip.assert_called_once() self.assertEqual(mock_abort.call_count, 7) self.assertEqual(response, "Test Success") + + def test_is_valid_signature_accepts_sha1_and_sha256(self): + """Test that signatures made with the hashes GitHub uses are accepted.""" + import hashlib + import hmac + + from utility import is_valid_signature + + data = b'{"zen": "Keep it logically awesome."}' + for name in ('sha1', 'sha256'): + digest = hmac.new(b'secret', msg=data, digestmod=getattr(hashlib, name)).hexdigest() + self.assertTrue(is_valid_signature(f'{name}={digest}', data, 'secret')) + self.assertFalse(is_valid_signature(f'{name}={digest}', data, 'other-secret')) + + def test_is_valid_signature_rejects_malformed_headers(self): + """Test that malformed or unexpected signature headers are rejected instead of raising.""" + import hashlib + import hmac + + from utility import is_valid_signature + + data = b'{}' + md5_digest = hmac.new(b'secret', msg=data, digestmod=hashlib.md5).hexdigest() + for header in (None, '', 'sha1', 'new=abc', 'unknown=abc', 'sha1=\u00e9', f'md5={md5_digest}'): + self.assertFalse(is_valid_signature(header, data, 'secret'), header) From de89515cb570d76d427a71d258b82d1ee6e4d974 Mon Sep 17 00:00:00 2001 From: Sajal Kumar Jana Date: Sun, 27 Sep 2026 13:42:40 +0530 Subject: [PATCH 3/6] fix(ci): verify webhooks with X-Hub-Signature-256 X-Hub-Signature is always HMAC-SHA1. GitHub also sends X-Hub-Signature-256 on every delivery that has a secret, so only accept sha256 signatures and drop SHA-1 from the signature path. --- utility.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/utility.py b/utility.py index 9040923e5..c4b842d8e 100644 --- a/utility.py +++ b/utility.py @@ -72,7 +72,7 @@ def decorated_function(*args, **kwargs): g.log.warning(f"Unauthorized attempt by IP {request_ip}") abort(abort_code) - for header in ['X-GitHub-Event', 'X-GitHub-Delivery', 'X-Hub-Signature', 'User-Agent']: + for header in ['X-GitHub-Event', 'X-GitHub-Delivery', 'X-Hub-Signature-256', 'User-Agent']: if header not in request.headers: g.log.critical(f"{header} not in headers!") abort(abort_code) @@ -149,34 +149,30 @@ def get_cached_web_hook_blocks() -> List[str]: return cached_web_hook_blocks -#: Hash algorithms GitHub signs web hook payloads with. -SIGNATURE_ALGORITHMS = {'sha1': hashlib.sha1, 'sha256': hashlib.sha256} - - def is_valid_signature(x_hub_signature, data, private_key): """ Re-check if the GitHub hook request got valid signature. - :param x_hub_signature: Signature to check, e.g. ``sha1=`` + :param x_hub_signature: Value of the ``X-Hub-Signature-256`` header, + e.g. ``sha256=`` :type x_hub_signature: str :param data: Signature's data :type data: bytearray :param private_key: Signature's token :type private_key: str - :return: False if the signature is missing, malformed, uses a hash - GitHub does not sign with, or does not match. + :return: False if the signature is missing, malformed, not SHA-256, + or does not match. :rtype: bool """ if not x_hub_signature: return False hash_algorithm, separator, github_signature = x_hub_signature.partition('=') - algorithm = SIGNATURE_ALGORITHMS.get(hash_algorithm) - if not separator or algorithm is None: + if not separator or hash_algorithm != 'sha256': return False encoded_key = bytes(private_key, 'latin-1') - mac = hmac.new(encoded_key, msg=data, digestmod=algorithm) + mac = hmac.new(encoded_key, msg=data, digestmod=hashlib.sha256) # Compare bytes: compare_digest raises TypeError on non-ASCII str input. return hmac.compare_digest(mac.hexdigest().encode(), github_signature.encode('utf-8', 'replace')) From 5a75787070aa5f2d4c3e87d71afb8b5ac08a1ef7 Mon Sep 17 00:00:00 2001 From: Sajal Kumar Jana Date: Sun, 27 Sep 2026 13:42:55 +0530 Subject: [PATCH 4/6] fix(ci): read X-Hub-Signature-256 in start_ci --- mod_ci/controllers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mod_ci/controllers.py b/mod_ci/controllers.py index a31765bb3..aaff5f4a4 100755 --- a/mod_ci/controllers.py +++ b/mod_ci/controllers.py @@ -2043,7 +2043,7 @@ def start_ci(): g.log.debug('server ping successful') return json.dumps({'msg': 'Hi!'}) - x_hub_signature = request.headers.get('X-Hub-Signature') + x_hub_signature = request.headers.get('X-Hub-Signature-256') if not is_valid_signature(x_hub_signature, request.data, g.github['ci_key']): g.log.warning(f'CI signature failed: {x_hub_signature}') From db9d900d9fff428e896e6d9ad0722d7cf23c1902 Mon Sep 17 00:00:00 2001 From: Sajal Kumar Jana Date: Sun, 27 Sep 2026 13:43:52 +0530 Subject: [PATCH 5/6] test: sign test webhooks with sha256 --- tests/base.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/base.py b/tests/base.py index 3bbc9bb33..17bb139d4 100644 --- a/tests/base.py +++ b/tests/base.py @@ -240,9 +240,8 @@ def generate_signature(data, private_key): """ import hashlib import hmac - algorithm = hashlib.__dict__.get('sha1') encoded_key = bytes(private_key, 'latin-1') - mac = hmac.new(encoded_key, msg=data, digestmod=algorithm) + mac = hmac.new(encoded_key, msg=data, digestmod=hashlib.sha256) return mac.hexdigest() @@ -252,12 +251,12 @@ def generate_git_api_header(event, sig): :param event: Name of the event type that triggered the delivery. :param sig: The HMAC hex digest of the response body. The HMAC hex digest is generated - using the sha1 hash function and the secret as the HMAC key. + using the sha256 hash function and the secret as the HMAC key. """ return Headers([ ('X-GitHub-Event', event), ('X-GitHub-Delivery', "72d3162e-cc78-11e3-81ab-4c9367dc0958"), - ('X-Hub-Signature', f"sha1={sig}"), + ('X-Hub-Signature-256', f"sha256={sig}"), ('User-Agent', "GitHub-Hookshot/044aadd"), ('Content-Type', "application/json"), ('Content-Length', 6615) From 439cdbcd7ae51eab5ba79573912f16bb4c54b983 Mon Sep 17 00:00:00 2001 From: Sajal Kumar Jana Date: Sun, 27 Sep 2026 13:44:05 +0530 Subject: [PATCH 6/6] test(utility): check sha256 webhook signatures --- tests/test_utility.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/test_utility.py b/tests/test_utility.py index 2fe960524..568603083 100644 --- a/tests/test_utility.py +++ b/tests/test_utility.py @@ -49,27 +49,27 @@ def example_function(*args, **kwargs): self.assertEqual(mock_abort.call_count, 7) self.assertEqual(response, "Test Success") - def test_is_valid_signature_accepts_sha1_and_sha256(self): - """Test that signatures made with the hashes GitHub uses are accepted.""" + def test_is_valid_signature_accepts_sha256(self): + """Test that a matching X-Hub-Signature-256 value is accepted.""" import hashlib import hmac from utility import is_valid_signature data = b'{"zen": "Keep it logically awesome."}' - for name in ('sha1', 'sha256'): - digest = hmac.new(b'secret', msg=data, digestmod=getattr(hashlib, name)).hexdigest() - self.assertTrue(is_valid_signature(f'{name}={digest}', data, 'secret')) - self.assertFalse(is_valid_signature(f'{name}={digest}', data, 'other-secret')) + digest = hmac.new(b'secret', msg=data, digestmod=hashlib.sha256).hexdigest() + self.assertTrue(is_valid_signature(f'sha256={digest}', data, 'secret')) + self.assertFalse(is_valid_signature(f'sha256={digest}', data, 'other-secret')) def test_is_valid_signature_rejects_malformed_headers(self): """Test that malformed or unexpected signature headers are rejected instead of raising.""" - import hashlib import hmac from utility import is_valid_signature data = b'{}' - md5_digest = hmac.new(b'secret', msg=data, digestmod=hashlib.md5).hexdigest() - for header in (None, '', 'sha1', 'new=abc', 'unknown=abc', 'sha1=\u00e9', f'md5={md5_digest}'): + md5_digest = hmac.new(b'secret', msg=data, digestmod='md5').hexdigest() + sha1_digest = hmac.new(b'secret', msg=data, digestmod='sha1').hexdigest() + for header in (None, '', 'sha256', 'new=abc', 'unknown=abc', 'sha256=\u00e9', + f'md5={md5_digest}', f'sha1={sha1_digest}'): self.assertFalse(is_valid_signature(header, data, 'secret'), header)