fix: improve the robustness of the payload extract logic (#1464)

Author: WilliamBergamin

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,25 +231,25 @@ 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
 
 
 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
 

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)