fix: redact proxy authentication in Socket Mode diagnostics

Author: Shubham-Padkonde

slack_sdk/socket_mode/builtin/internals.py

@@ -60,8 +60,11 @@ def _establish_new_socket_connection(
     ssl_context = _use_or_create_ssl_context(ssl_context)
 
     if proxy is not None:
-        parsed_proxy = urlparse(proxy)
-        proxy_host, proxy_port = parsed_proxy.hostname, parsed_proxy.port or 80
+        try:
+            parsed_proxy = urlparse(proxy)
+            proxy_host, proxy_port = parsed_proxy.hostname, parsed_proxy.port or 80
+        except ValueError:
+            raise ValueError("Invalid proxy URL") from None
         sock = socket.create_connection((proxy_host, proxy_port), receive_timeout)
         if hasattr(socket, "TCP_NODELAY"):
             sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
@@ -80,7 +83,11 @@ def _establish_new_socket_connection(
         message.append("")
         req: str = "\r\n".join([line.lstrip() for line in message])
         if trace_enabled:
-            logger.debug(f"Proxy connect request (session id: {session_id}):\n{req}")
+            redacted_req = "\r\n".join(
+                "Proxy-Authorization: ***" if line.partition(":")[0].strip().lower() == "proxy-authorization" else line
+                for line in req.split("\r\n")
+            )
+            logger.debug(f"Proxy connect request (session id: {session_id}):\n{redacted_req}")
         with sock_send_lock:
             sock.send(req.encode("utf-8"))
         status, text = _parse_connect_response(sock)

tests/test_proxy_env_variable_loader.py

@@ -1,10 +1,12 @@
 import logging
 import os
 import unittest
+from base64 import b64encode
 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.connection import Connection
 from slack_sdk.socket_mode.builtin.internals import _establish_new_socket_connection
 from tests.helpers import remove_os_env_temporarily, restore_os_env
 
@@ -113,3 +115,81 @@ def test_socket_mode_proxy_error_redacts_credentials(self):
         self.assertNotIn("secret", str(error.exception))
         self.assertIn("http://***@proxy.example.com:3128", str(error.exception))
         self.assertIn("407", str(error.exception))
+
+    def test_socket_mode_proxy_trace_redacts_authorization(self):
+        basic_credentials = b64encode(b"bob:secret").decode("ascii")
+        for proxy, proxy_port, proxy_headers, authorization in (
+            (
+                "http://bob:secret@proxy.example.com:3128",
+                3128,
+                None,
+                "Proxy-Authorization: Basic " + basic_credentials,
+            ),
+            (
+                "http://proxy.example.com",
+                80,
+                {"pRoXy-AuThOrIzAtIoN": "Bearer proxy-token", "X-Proxy-Request": "trace-test"},
+                "pRoXy-AuThOrIzAtIoN: Bearer proxy-token",
+            ),
+        ):
+            with self.subTest(proxy_headers=proxy_headers):
+                logger = logging.getLogger("test_proxy_env_variable_loader")
+                with patch("slack_sdk.socket_mode.builtin.internals.socket.create_connection") as connect, patch(
+                    "slack_sdk.socket_mode.builtin.internals._parse_connect_response",
+                    return_value=(200, "HTTP/1.0 200 Connection established"),
+                ), self.assertLogs(logger, level="DEBUG") as logs:
+                    _establish_new_socket_connection(
+                        session_id="test",
+                        server_hostname="example.com",
+                        server_port=443,
+                        logger=logger,
+                        sock_send_lock=Lock(),
+                        receive_timeout=1,
+                        proxy=proxy,
+                        proxy_headers=proxy_headers,
+                        trace_enabled=True,
+                        ssl_context=Mock(),
+                    )
+
+                expected_request = "CONNECT example.com:443 HTTP/1.0\r\n" + authorization + "\r\n"
+                if proxy_headers is not None:
+                    expected_request += "X-Proxy-Request: trace-test\r\n"
+                expected_request += "\r\n"
+                connect.assert_called_once_with(("proxy.example.com", proxy_port), 1)
+                connect.return_value.send.assert_called_once_with(expected_request.encode("utf-8"))
+
+                output = "\n".join(logs.output)
+                for sensitive_value in ("bob", "secret", basic_credentials, "proxy-token"):
+                    self.assertNotIn(sensitive_value, output)
+                self.assertIn("Proxy-Authorization: ***", output)
+                self.assertIn("CONNECT example.com:443 HTTP/1.0", output)
+                self.assertIn("HTTP/1.0 200 Connection established", output)
+                if proxy_headers is not None:
+                    self.assertIn("X-Proxy-Request: trace-test", output)
+
+    def test_socket_mode_proxy_parsing_errors_do_not_log_credentials(self):
+        secret = "sensitive-proxy-password"
+        proxies = [f"http://bob:{secret}{delimiter}tail@proxy.example.com:3128" for delimiter in ("/", "#", "?")]
+        for proxy in proxies:
+            for trace_enabled in (False, True):
+                with self.subTest(proxy=proxy, trace_enabled=trace_enabled):
+                    logger = logging.getLogger("test_proxy_env_variable_loader")
+                    on_error = Mock()
+                    connection = Connection(
+                        url="wss://example.com/socket-mode",
+                        logger=logger,
+                        proxy=proxy,
+                        trace_enabled=trace_enabled,
+                        on_error_listener=on_error,
+                        ssl_context=Mock(),
+                    )
+                    with patch("slack_sdk.socket_mode.builtin.internals.socket.create_connection") as connect:
+                        with self.assertLogs(logger, level="ERROR") as logs:
+                            connection.connect()
+                    connect.assert_not_called()
+                    on_error.assert_called_once()
+                    self.assertIsInstance(on_error.call_args[0][0], ValueError)
+                    output = "\n".join(logs.output)
+                    self.assertNotIn(secret, output)
+                    self.assertNotIn("bob", output)
+                    self.assertIn("Invalid proxy URL", output)