From bdfac647e7c87527f0411bff321f6163cb45bba5 Mon Sep 17 00:00:00 2001 From: Shubham-Padkonde Date: Thu, 24 Sep 2026 03:28:45 +0000 Subject: [PATCH 1/2] fix: SignatureVerifier.is_valid raises ValueError on non-numeric timestamp is_valid called int(timestamp) without catching ValueError, so any request with a non-numeric X-Slack-Request-Timestamp header (e.g. "not-a-number", an empty string, or a float like "1.5") caused an unhandled exception rather than a clean False return. Fix: wrap the int() conversion in a try/except (ValueError, TypeError) and return False on failure, consistent with how None timestamps are already handled. Add two tests covering the new behaviour: - test_is_valid_non_numeric_timestamp: direct is_valid() calls with invalid timestamp strings - test_is_valid_request_non_numeric_timestamp_header: end-to-end call via is_valid_request() with a crafted header --- slack_sdk/signature/__init__.py | 6 ++++- .../signature/test_signature_verifier.py | 24 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/slack_sdk/signature/__init__.py b/slack_sdk/signature/__init__.py index 52cd7bcb5..4d258a229 100644 --- a/slack_sdk/signature/__init__.py +++ b/slack_sdk/signature/__init__.py @@ -66,7 +66,11 @@ def is_valid( if timestamp is None or signature is None: return False - if abs(self.clock.now() - int(timestamp)) > 60 * 5: + try: + ts = int(timestamp) + except (ValueError, TypeError): + return False + if abs(self.clock.now() - ts) > 60 * 5: return False calculated_signature = self.generate_signature(timestamp=timestamp, body=body) diff --git a/tests/slack_sdk/signature/test_signature_verifier.py b/tests/slack_sdk/signature/test_signature_verifier.py index 05a4de420..e56dbbbec 100644 --- a/tests/slack_sdk/signature/test_signature_verifier.py +++ b/tests/slack_sdk/signature/test_signature_verifier.py @@ -98,6 +98,30 @@ def test_is_valid_none(self): self.assertFalse(verifier.is_valid(self.body, None, None)) self.assertFalse(verifier.is_valid(None, None, None)) + def test_is_valid_non_numeric_timestamp(self): + # A non-integer X-Slack-Request-Timestamp header must cause is_valid to + # return False rather than raising ValueError from int(). + verifier = SignatureVerifier( + signing_secret=self.signing_secret, + clock=MockClock(), + ) + self.assertFalse(verifier.is_valid(self.body, "not-a-number", self.valid_signature)) + self.assertFalse(verifier.is_valid(self.body, "1.5", self.valid_signature)) + self.assertFalse(verifier.is_valid(self.body, "", self.valid_signature)) + + def test_is_valid_request_non_numeric_timestamp_header(self): + # is_valid_request must also return False (not raise) when the + # X-Slack-Request-Timestamp header is non-numeric. + verifier = SignatureVerifier( + signing_secret=self.signing_secret, + clock=MockClock(), + ) + bad_headers = { + "X-Slack-Request-Timestamp": "not-a-number", + "X-Slack-Signature": self.valid_signature, + } + self.assertFalse(verifier.is_valid_request(self.body, bad_headers)) + def test_invalid_signing_secret(self): with self.assertRaises(ValueError): SignatureVerifier("") From af9412986e0290b70c54ce725f5f1d2726733124 Mon Sep 17 00:00:00 2001 From: Shubham Padkonde Date: Thu, 24 Sep 2026 22:49:50 +0530 Subject: [PATCH 2/2] test: pin signature replay-window boundaries --- .../slack_sdk/signature/test_signature_verifier.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/slack_sdk/signature/test_signature_verifier.py b/tests/slack_sdk/signature/test_signature_verifier.py index e56dbbbec..d53777e24 100644 --- a/tests/slack_sdk/signature/test_signature_verifier.py +++ b/tests/slack_sdk/signature/test_signature_verifier.py @@ -1,4 +1,5 @@ import unittest +from unittest.mock import Mock from slack_sdk.signature import SignatureVerifier @@ -99,8 +100,6 @@ def test_is_valid_none(self): self.assertFalse(verifier.is_valid(None, None, None)) def test_is_valid_non_numeric_timestamp(self): - # A non-integer X-Slack-Request-Timestamp header must cause is_valid to - # return False rather than raising ValueError from int(). verifier = SignatureVerifier( signing_secret=self.signing_secret, clock=MockClock(), @@ -110,8 +109,6 @@ def test_is_valid_non_numeric_timestamp(self): self.assertFalse(verifier.is_valid(self.body, "", self.valid_signature)) def test_is_valid_request_non_numeric_timestamp_header(self): - # is_valid_request must also return False (not raise) when the - # X-Slack-Request-Timestamp header is non-numeric. verifier = SignatureVerifier( signing_secret=self.signing_secret, clock=MockClock(), @@ -141,3 +138,11 @@ def test_invalid_signing_secret_reassignment(self): with self.assertRaises(ValueError): verifier.signing_secret = None self.assertEqual(verifier.signing_secret, self.signing_secret) + + def test_is_valid_rejects_numeric_timestamp_outside_replay_window(self): + clock = Mock() + verifier = SignatureVerifier(signing_secret=self.signing_secret, clock=clock) + for elapsed, expected in ((300, True), (301, False), (-301, False)): + with self.subTest(elapsed=elapsed): + clock.now.return_value = int(self.timestamp) + elapsed + self.assertEqual(verifier.is_valid(self.body, self.timestamp, self.valid_signature), expected)