From 4f2a1fd93201cd4c13f0f4d77e01994f7f200620 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Thu, 19 Mar 2026 10:55:34 -0400 Subject: [PATCH] fix: improve the robustness of the payload extract logic --- slack_bolt/request/internals.py | 42 +++++++++++----------- tests/slack_bolt/request/test_internals.py | 25 +++++++++++++ 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/slack_bolt/request/internals.py b/slack_bolt/request/internals.py index 466f5daaf..15d1e7367 100644 --- a/slack_bolt/request/internals.py +++ b/slack_bolt/request/internals.py @@ -65,10 +65,10 @@ def extract_enterprise_id(payload: Dict[str, Any]) -> Optional[str]: return extract_enterprise_id(payload["authorizations"][0]) if "enterprise_id" in payload: return payload.get("enterprise_id") - if payload.get("team") is not None and "enterprise_id" in payload["team"]: + if isinstance(payload.get("team"), dict) and "enterprise_id" in payload["team"]: # In the case where the type is view_submission return payload["team"].get("enterprise_id") - if payload.get("event") is not None: + if isinstance(payload.get("event"), dict): return extract_enterprise_id(payload["event"]) return None @@ -88,13 +88,13 @@ def extract_actor_enterprise_id(payload: Dict[str, Any]) -> Optional[str]: def extract_team_id(payload: Dict[str, Any]) -> Optional[str]: - app_installed_team_id = payload.get("view", {}).get("app_installed_team_id") - if app_installed_team_id is not None: + view = payload.get("view") + if isinstance(view, dict) and view.get("app_installed_team_id") is not None: # view_submission payloads can have `view.app_installed_team_id` when a modal view that was opened # in a different workspace via some operations inside a Slack Connect channel. # Note that the same for enterprise_id does not exist. When you need to know the enterprise_id as well, # you have to run some query toward your InstallationStore to know the org where the team_id belongs to. - return app_installed_team_id + return view["app_installed_team_id"] if payload.get("team") is not None: # With org-wide installations, payload.team in interactivity payloads can be None # You need to extract either payload.user.team_id or payload.view.team_id as below @@ -109,12 +109,12 @@ def extract_team_id(payload: Dict[str, Any]) -> Optional[str]: return extract_team_id(payload["authorizations"][0]) if "team_id" in payload: return payload.get("team_id") - if payload.get("event") is not None: + if isinstance(payload.get("event"), dict): return extract_team_id(payload["event"]) - if payload.get("user") is not None: + if isinstance(payload.get("user"), dict): return payload["user"]["team_id"] - if payload.get("view") is not None: - return payload.get("view", {})["team_id"] + if isinstance(payload.get("view"), dict): + return payload["view"]["team_id"] return None @@ -169,12 +169,12 @@ def extract_user_id(payload: Dict[str, Any]) -> Optional[str]: return user.get("id") if "user_id" in payload: return payload.get("user_id") - if payload.get("event") is not None: + if isinstance(payload.get("event"), dict): return extract_user_id(payload["event"]) - if payload.get("message") is not None: + if isinstance(payload.get("message"), dict): # message_changed: body["event"]["message"] return extract_user_id(payload["message"]) - if payload.get("previous_message") is not None: + if isinstance(payload.get("previous_message"), dict): # message_deleted: body["event"]["previous_message"] return extract_user_id(payload["previous_message"]) return None @@ -202,12 +202,12 @@ def extract_channel_id(payload: Dict[str, Any]) -> Optional[str]: return channel.get("id") if "channel_id" in payload: return payload.get("channel_id") - if payload.get("event") is not None: + if isinstance(payload.get("event"), dict): return extract_channel_id(payload["event"]) - if payload.get("item") is not None: + if isinstance(payload.get("item"), dict): # reaction_added: body["event"]["item"] return extract_channel_id(payload["item"]) - if payload.get("assistant_thread") is not None: + if isinstance(payload.get("assistant_thread"), dict): # assistant_thread_started return extract_channel_id(payload["assistant_thread"]) return None @@ -217,7 +217,7 @@ def extract_thread_ts(payload: Dict[str, Any]) -> Optional[str]: thread_ts = payload.get("thread_ts") if thread_ts is not None: return thread_ts - if payload.get("event") is not None: + if isinstance(payload.get("event"), dict): return extract_thread_ts(payload["event"]) if isinstance(payload.get("assistant_thread"), dict): return extract_thread_ts(payload["assistant_thread"]) @@ -231,9 +231,9 @@ def extract_thread_ts(payload: Dict[str, Any]) -> Optional[str]: def extract_function_execution_id(payload: Dict[str, Any]) -> Optional[str]: if payload.get("function_execution_id") is not None: return payload.get("function_execution_id") - if payload.get("event") is not None: + if isinstance(payload.get("event"), dict): return extract_function_execution_id(payload["event"]) - if payload.get("function_data") is not None: + if isinstance(payload.get("function_data"), dict): return payload["function_data"].get("execution_id") return None @@ -241,15 +241,15 @@ def extract_function_execution_id(payload: Dict[str, Any]) -> Optional[str]: def extract_function_bot_access_token(payload: Dict[str, Any]) -> Optional[str]: if payload.get("bot_access_token") is not None: return payload.get("bot_access_token") - if payload.get("event") is not None: + if isinstance(payload.get("event"), dict): return payload["event"].get("bot_access_token") return None def extract_function_inputs(payload: Dict[str, Any]) -> Optional[Dict[str, Any]]: - if payload.get("event") is not None: + if isinstance(payload.get("event"), dict): return payload["event"].get("inputs") - if payload.get("function_data") is not None: + if isinstance(payload.get("function_data"), dict): return payload["function_data"].get("inputs") return None diff --git a/tests/slack_bolt/request/test_internals.py b/tests/slack_bolt/request/test_internals.py index 0b267e3de..8cccf0431 100644 --- a/tests/slack_bolt/request/test_internals.py +++ b/tests/slack_bolt/request/test_internals.py @@ -1248,3 +1248,28 @@ def test_slack_connect_patterns(self): assert extract_actor_enterprise_id(request) == actor_enterprise_id assert extract_actor_team_id(request) == actor_team_id assert extract_actor_user_id(request) == actor_user_id + + def test_extraction_functions_invalid_dict_keys(self): + invalid_payloads = { + "event": {"event": "some_event_type"}, + "user": {"user": "U12345"}, + "team": {"team": "T12345"}, + "view": {"view": "V12345"}, + "message": {"message": "some text"}, + "item": {"item": "item_id"}, + "function_data": {"function_data": "fd_123"}, + "assistant_thread": {"assistant_thread": "at_123"}, + "previous_message": {"previous_message": "old_msg"}, + } + + for _, payload in invalid_payloads.items(): + # We only verify no TypeError/AttributeError is raised and that functions which + # would try to subscript the string value return None instead of crashing. + extract_enterprise_id(payload) + extract_team_id(payload) + extract_user_id(payload) + extract_channel_id(payload) + extract_thread_ts(payload) + extract_function_execution_id(payload) + extract_function_bot_access_token(payload) + extract_function_inputs(payload)