From a54bf562887c1b7cc06add130276d92a3b7ef8c2 Mon Sep 17 00:00:00 2001 From: Ismail Pelaseyed Date: Fri, 8 May 2026 08:06:34 +0200 Subject: [PATCH] Require signatures for ssl_check request verification --- .../request_verification.py | 2 +- tests/adapter_tests/wsgi/test_wsgi_http.py | 41 +++++++++++++++++++ tests/scenario_tests/test_slash_command.py | 28 +++++++++++++ .../test_slash_command.py | 29 +++++++++++++ .../test_request_verification.py | 15 +++++++ .../test_request_verification.py | 16 ++++++++ 6 files changed, 130 insertions(+), 1 deletion(-) diff --git a/slack_bolt/middleware/request_verification/request_verification.py b/slack_bolt/middleware/request_verification/request_verification.py index 2cf7e361e..c0f3f5c31 100644 --- a/slack_bolt/middleware/request_verification/request_verification.py +++ b/slack_bolt/middleware/request_verification/request_verification.py @@ -49,7 +49,7 @@ def process( @staticmethod def _can_skip(mode: str, body: Dict[str, Any]) -> bool: - return mode == "socket_mode" or (body is not None and body.get("ssl_check") == "1") + return mode == "socket_mode" @staticmethod def _build_error_response() -> BoltResponse: diff --git a/tests/adapter_tests/wsgi/test_wsgi_http.py b/tests/adapter_tests/wsgi/test_wsgi_http.py index 63ac62627..c7fca2e25 100644 --- a/tests/adapter_tests/wsgi/test_wsgi_http.py +++ b/tests/adapter_tests/wsgi/test_wsgi_http.py @@ -89,6 +89,47 @@ def command_handler(ack): assert response.headers.get("content-type") == "text/plain;charset=utf-8" assert_auth_test_count(self, 1) + def test_ssl_check_param_does_not_bypass_request_verification(self): + app = App( + client=self.web_client, + signing_secret=self.signing_secret, + ssl_check_enabled=False, + ) + command_called = False + + def command_handler(ack): + nonlocal command_called + command_called = True + ack() + + app.command("/hello-world")(command_handler) + + body = ( + "token=verification_token" + "&team_id=T111" + "&team_domain=test-domain" + "&channel_id=C111" + "&channel_name=random" + "&user_id=W111" + "&user_name=primary-owner" + "&command=%2Fhello-world" + "&text=Hi" + "&enterprise_id=E111" + "&enterprise_name=Org+Name" + "&response_url=https%3A%2F%2Fhooks.slack.com%2Fcommands%2FT111%2F111%2Fxxxxx" + "&trigger_id=111.111.xxx" + "&ssl_check=1" + ) + headers = self.build_raw_headers("0", body) + headers["x-slack-signature"] = "v0=invalid" + + wsgi_server = WsgiTestServer(SlackRequestHandler(app)) + response = wsgi_server.http(method="POST", headers=headers, body=body) + + assert response.status == "401 Unauthorized" + assert response.body == """{"error": "invalid request"}""" + assert command_called is False + def test_events(self): app = App( client=self.web_client, diff --git a/tests/scenario_tests/test_slash_command.py b/tests/scenario_tests/test_slash_command.py index 1db13ecca..ae7e7c53b 100644 --- a/tests/scenario_tests/test_slash_command.py +++ b/tests/scenario_tests/test_slash_command.py @@ -93,6 +93,34 @@ def test_failure(self): assert response.status == 404 assert_auth_test_count(self, 1) + def test_ssl_check_param_does_not_bypass_request_verification(self): + app = App( + client=self.web_client, + signing_secret=self.signing_secret, + ssl_check_enabled=False, + ) + command_called = False + + def command_handler(ack): + nonlocal command_called + command_called = True + ack() + + app.command("/hello-world")(command_handler) + + request = BoltRequest( + body=f"{slash_command_body}&ssl_check=1", + headers={ + "content-type": ["application/x-www-form-urlencoded"], + "x-slack-signature": ["v0=invalid"], + "x-slack-request-timestamp": ["0"], + }, + ) + response = app.dispatch(request) + assert response.status == 401 + assert response.body == """{"error": "invalid request"}""" + assert command_called is False + slash_command_body = ( "token=verification_token" diff --git a/tests/scenario_tests_async/test_slash_command.py b/tests/scenario_tests_async/test_slash_command.py index 1ac02bce7..918ccba87 100644 --- a/tests/scenario_tests_async/test_slash_command.py +++ b/tests/scenario_tests_async/test_slash_command.py @@ -100,6 +100,35 @@ async def test_failure(self): assert response.status == 404 await assert_auth_test_count_async(self, 1) + @pytest.mark.asyncio + async def test_ssl_check_param_does_not_bypass_request_verification(self): + app = AsyncApp( + client=self.web_client, + signing_secret=self.signing_secret, + ssl_check_enabled=False, + ) + command_called = False + + async def command_handler(ack): + nonlocal command_called + command_called = True + await ack() + + app.command("/hello-world")(command_handler) + + request = AsyncBoltRequest( + body=f"{slash_command_body}&ssl_check=1", + headers={ + "content-type": ["application/x-www-form-urlencoded"], + "x-slack-signature": ["v0=invalid"], + "x-slack-request-timestamp": ["0"], + }, + ) + response = await app.async_dispatch(request) + assert response.status == 401 + assert response.body == """{"error": "invalid request"}""" + assert command_called is False + slash_command_body = ( "token=verification_token" diff --git a/tests/slack_bolt/middleware/request_verification/test_request_verification.py b/tests/slack_bolt/middleware/request_verification/test_request_verification.py index 2c9adea43..ae163a84d 100644 --- a/tests/slack_bolt/middleware/request_verification/test_request_verification.py +++ b/tests/slack_bolt/middleware/request_verification/test_request_verification.py @@ -45,3 +45,18 @@ def test_invalid(self): resp = middleware.process(req=req, resp=resp, next=next) assert resp.status == 401 assert resp.body == """{"error": "invalid request"}""" + + def test_ssl_check_param_requires_valid_signature(self): + middleware = RequestVerification(signing_secret=self.signing_secret) + req = BoltRequest( + body="token=random&ssl_check=1", + headers={ + "content-type": ["application/x-www-form-urlencoded"], + "x-slack-signature": ["v0=invalid"], + "x-slack-request-timestamp": ["0"], + }, + ) + resp = BoltResponse(status=404) + resp = middleware.process(req=req, resp=resp, next=next) + assert resp.status == 401 + assert resp.body == """{"error": "invalid request"}""" diff --git a/tests/slack_bolt_async/middleware/request_verification/test_request_verification.py b/tests/slack_bolt_async/middleware/request_verification/test_request_verification.py index c097dd146..28921bc87 100644 --- a/tests/slack_bolt_async/middleware/request_verification/test_request_verification.py +++ b/tests/slack_bolt_async/middleware/request_verification/test_request_verification.py @@ -50,3 +50,19 @@ async def test_invalid(self): resp = await middleware.async_process(req=req, resp=resp, next=next) assert resp.status == 401 assert resp.body == """{"error": "invalid request"}""" + + @pytest.mark.asyncio + async def test_ssl_check_param_requires_valid_signature(self): + middleware = AsyncRequestVerification(signing_secret="secret") + req = AsyncBoltRequest( + body="token=random&ssl_check=1", + headers={ + "content-type": ["application/x-www-form-urlencoded"], + "x-slack-signature": ["v0=invalid"], + "x-slack-request-timestamp": ["0"], + }, + ) + resp = BoltResponse(status=404) + resp = await middleware.async_process(req=req, resp=resp, next=next) + assert resp.status == 401 + assert resp.body == """{"error": "invalid request"}"""