fix: redact proxy credentials when logging the proxy URL from env

slack_sdk/proxy_env_variable_loader.py

@@ -2,7 +2,9 @@
 
 import logging
 import os
+import re
 from typing import Optional
+from urllib.parse import urlsplit
 
 _default_logger = logging.getLogger(__name__)
 
@@ -21,5 +23,18 @@ def load_http_proxy_from_env(logger: logging.Logger = _default_logger) -> Option
         logger.debug("The Slack SDK ignored the proxy env variable as an empty value is set.")
         return None
 
-    logger.debug(f"HTTP proxy URL has been loaded from an env variable: {proxy_url}")
+    logger.debug(f"HTTP proxy URL has been loaded from an env variable: {_redact_credentials(proxy_url)}")
     return proxy_url
+
+
+def _redact_credentials(url: str) -> str:
+    """Replaces the user info (e.g., user:password@) in a URL so that it can be safely logged."""
+    if "@" in url:
+        scheme = re.match(r"^[a-zA-Z][a-zA-Z0-9+.-]*://", url)
+        prefix = scheme.group(0) if scheme else ""
+        return f"{prefix}***@{url.rpartition('@')[2]}"
+    try:
+        urlsplit(url)
+    except ValueError:
+        return "(unparsable URL)"
+    return url

slack_sdk/socket_mode/builtin/internals.py

@@ -14,6 +14,8 @@
 from typing import Tuple, Optional, Union, List, Callable, Dict
 from urllib.parse import urlparse, unquote
 
+from slack_sdk.proxy_env_variable_loader import _redact_credentials
+
 from .frame_header import FrameHeader
 
 
@@ -86,7 +88,9 @@ def _establish_new_socket_connection(
             log_message = f"Proxy connect response (session id: {session_id}):\n{text}"
             logger.debug(log_message)
         if status != 200:
-            raise Exception(f"Failed to connect to the proxy (proxy: {proxy}, connect status code: {status})")
+            raise Exception(
+                f"Failed to connect to the proxy (proxy: {_redact_credentials(proxy)}, connect status code: {status})"
+            )
 
         sock = ssl_context.wrap_socket(
             sock,

tests/test_proxy_env_variable_loader.py

@@ -1,7 +1,11 @@
+import logging
 import os
 import unittest
+from unittest.mock import Mock, patch
+from threading import Lock
 
 from slack_sdk.proxy_env_variable_loader import load_http_proxy_from_env
+from slack_sdk.socket_mode.builtin.internals import _establish_new_socket_connection
 from tests.helpers import remove_os_env_temporarily, restore_os_env
 
 
@@ -38,3 +42,74 @@ def test_proxy_url_is_none_case(self):
         os.environ.pop("http_proxy", None)
         url = load_http_proxy_from_env()
         self.assertEqual(url, None)
+
+    def test_credentials_are_not_logged(self):
+        os.environ["HTTPS_PROXY"] = "http://bob:secret@example.com:8080"
+        logger = logging.getLogger("test_proxy_env_variable_loader")
+        with self.assertLogs(logger, level="DEBUG") as logs:
+            url = load_http_proxy_from_env(logger)
+        # the proxy URL itself is returned unchanged
+        self.assertEqual(url, "http://bob:secret@example.com:8080")
+        output = "\n".join(logs.output)
+        self.assertNotIn("bob", output)
+        self.assertNotIn("secret", output)
+        self.assertIn("http://***@example.com:8080", output)
+
+    def test_url_without_credentials_is_logged_as_is(self):
+        os.environ["HTTPS_PROXY"] = "http://localhost:9999"
+        logger = logging.getLogger("test_proxy_env_variable_loader")
+        with self.assertLogs(logger, level="DEBUG") as logs:
+            load_http_proxy_from_env(logger)
+        self.assertIn("http://localhost:9999", "\n".join(logs.output))
+
+    def test_credentials_in_unusual_proxy_urls_are_not_logged(self):
+        for value in (
+            "bob:secret@proxy.example.com:3128",
+            "http://bob:pa/ss@proxy.example.com:3128",
+            "http://bob:pa#ss@proxy.example.com:3128",
+            "http://bob:pa?ss@proxy.example.com:3128",
+            "http://token@proxy.example.com:3128",
+            "http://bob:pa@ss@proxy.example.com:3128",
+        ):
+            with self.subTest(value=value):
+                os.environ["HTTPS_PROXY"] = value
+                logger = logging.getLogger("test_proxy_env_variable_loader")
+                with self.assertLogs(logger, level="DEBUG") as logs:
+                    self.assertEqual(load_http_proxy_from_env(logger), value)
+                expected = (
+                    "http://***@proxy.example.com:3128" if value.startswith("http://") else "***@proxy.example.com:3128"
+                )
+                self.assertEqual(
+                    logs.records[0].getMessage(), "HTTP proxy URL has been loaded from an env variable: " + expected
+                )
+
+    def test_unparsable_proxy_url_is_not_logged(self):
+        os.environ["HTTPS_PROXY"] = "http://[invalid"
+        logger = logging.getLogger("test_proxy_env_variable_loader")
+        with self.assertLogs(logger, level="DEBUG") as logs:
+            self.assertEqual(load_http_proxy_from_env(logger), "http://[invalid")
+        self.assertIn("(unparsable URL)", logs.output[0])
+        self.assertNotIn("[invalid", logs.output[0])
+
+    def test_socket_mode_proxy_error_redacts_credentials(self):
+        with patch("slack_sdk.socket_mode.builtin.internals.socket.create_connection"), patch(
+            "slack_sdk.socket_mode.builtin.internals._parse_connect_response",
+            return_value=(407, "Proxy Authentication Required"),
+        ):
+            with self.assertRaises(Exception) as error:
+                _establish_new_socket_connection(
+                    session_id="test",
+                    server_hostname="example.com",
+                    server_port=443,
+                    logger=Mock(),
+                    sock_send_lock=Lock(),
+                    receive_timeout=1,
+                    proxy="http://bob:secret@proxy.example.com:3128",
+                    proxy_headers=None,
+                    trace_enabled=False,
+                    ssl_context=Mock(),
+                )
+        self.assertNotIn("bob", str(error.exception))
+        self.assertNotIn("secret", str(error.exception))
+        self.assertIn("http://***@proxy.example.com:3128", str(error.exception))
+        self.assertIn("407", str(error.exception))