fix: Read transaction service API key from the environment at call time

safe_eth/safe/api/transaction_service_api/transaction_service_api.py

@@ -63,11 +63,22 @@ def __init__(
         network: EthereumNetwork,
         ethereum_client: Optional[EthereumClient] = None,
         base_url: Optional[str] = None,
-        api_key: Optional[str] = os.environ.get("SAFE_TRANSACTION_SERVICE_API_KEY"),
-        request_timeout: int = int(
-            os.environ.get("SAFE_TRANSACTION_SERVICE_REQUEST_TIMEOUT", 10)
-        ),
+        api_key: Optional[str] = None,
+        request_timeout: Optional[int] = None,
     ):
+        # Resolved here rather than as default argument values, so that the
+        # environment is read at call time and an explicit `None` (as passed by
+        # `SafeBaseAPI.from_ethereum_client`) still falls back to it.
+        #
+        # The check is `is None` rather than a truthiness test so that any value
+        # the caller actually passed wins: `api_key=""` forces an anonymous
+        # client even when the environment variable is set.
+        if api_key is None:
+            api_key = os.environ.get("SAFE_TRANSACTION_SERVICE_API_KEY")
+        if request_timeout is None:
+            request_timeout = int(
+                os.environ.get("SAFE_TRANSACTION_SERVICE_REQUEST_TIMEOUT", 10)
+            )
         super().__init__(network, ethereum_client, base_url, api_key, request_timeout)
 
     def _get_url_by_network(self, network: EthereumNetwork) -> Optional[str]:

safe_eth/safe/tests/api/test_transaction_service_api.py

@@ -3,7 +3,7 @@
 from unittest import mock
 from unittest.mock import MagicMock, PropertyMock, patch
 
-from django.test import TestCase
+from django.test import SimpleTestCase, TestCase
 
 import pytest
 from eth_account import Account
@@ -354,3 +354,103 @@ def test_decode_data(self):
                 "Cannot decode tx data:",
                 str(context.exception),
             )
+
+
+class TestTransactionServiceApiEnvConfig(SimpleTestCase):
+    """
+    The API key and request timeout are read from the environment. These tests
+    do not need network access or a real key, so they are kept out of
+    `TestTransactionServiceAPI`, which skips unless a key is configured.
+    """
+
+    ENV_API_KEY = "SAFE_TRANSACTION_SERVICE_API_KEY"
+    ENV_TIMEOUT = "SAFE_TRANSACTION_SERVICE_REQUEST_TIMEOUT"
+
+    @staticmethod
+    def _ethereum_client_mock() -> MagicMock:
+        ethereum_client = MagicMock(spec=EthereumClient)
+        ethereum_client.get_network.return_value = EthereumNetwork.MAINNET
+        return ethereum_client
+
+    def test_api_key_read_from_environment_at_call_time(self):
+        # Set after the module was imported: a default argument value would
+        # have been evaluated at import time and would miss this.
+        with mock.patch.dict(os.environ, {self.ENV_API_KEY: "env-api-key"}, clear=True):
+            self.assertEqual(
+                TransactionServiceApi(EthereumNetwork.MAINNET).api_key, "env-api-key"
+            )
+
+    def test_api_key_from_environment_with_explicit_none(self):
+        # `SafeBaseAPI.from_ethereum_client` forwards `api_key=None`, which must
+        # not discard the environment value.
+        with mock.patch.dict(os.environ, {self.ENV_API_KEY: "env-api-key"}, clear=True):
+            self.assertEqual(
+                TransactionServiceApi(EthereumNetwork.MAINNET, api_key=None).api_key,
+                "env-api-key",
+            )
+            self.assertEqual(
+                TransactionServiceApi.from_ethereum_client(
+                    self._ethereum_client_mock()
+                ).api_key,
+                "env-api-key",
+            )
+
+    def test_explicit_api_key_takes_precedence(self):
+        with mock.patch.dict(os.environ, {self.ENV_API_KEY: "env-api-key"}, clear=True):
+            self.assertEqual(
+                TransactionServiceApi(
+                    EthereumNetwork.MAINNET, api_key="explicit-api-key"
+                ).api_key,
+                "explicit-api-key",
+            )
+            self.assertEqual(
+                TransactionServiceApi.from_ethereum_client(
+                    self._ethereum_client_mock(), api_key="explicit-api-key"
+                ).api_key,
+                "explicit-api-key",
+            )
+
+    def test_api_key_absent(self):
+        with mock.patch.dict(os.environ, {}, clear=True):
+            self.assertIsNone(TransactionServiceApi(EthereumNetwork.MAINNET).api_key)
+
+    def test_empty_api_key_forces_anonymous(self):
+        # Only `None` means "not supplied". An empty string is a deliberate
+        # choice to send no Authorization header, and must survive a set
+        # environment variable.
+        with mock.patch.dict(os.environ, {self.ENV_API_KEY: "env-api-key"}, clear=True):
+            self.assertEqual(
+                TransactionServiceApi(EthereumNetwork.MAINNET, api_key="").api_key, ""
+            )
+            self.assertEqual(
+                TransactionServiceApi.from_ethereum_client(
+                    self._ethereum_client_mock(), api_key=""
+                ).api_key,
+                "",
+            )
+
+    def test_request_timeout_read_from_environment_at_call_time(self):
+        with mock.patch.dict(os.environ, {self.ENV_TIMEOUT: "42"}, clear=True):
+            self.assertEqual(
+                TransactionServiceApi(EthereumNetwork.MAINNET).request_timeout, 42
+            )
+
+        with mock.patch.dict(os.environ, {}, clear=True):
+            self.assertEqual(
+                TransactionServiceApi(EthereumNetwork.MAINNET).request_timeout, 10
+            )
+            self.assertEqual(
+                TransactionServiceApi(
+                    EthereumNetwork.MAINNET, request_timeout=5
+                ).request_timeout,
+                5,
+            )
+
+        # Same reasoning as the empty API key: 0 is a value, not an absence.
+        with mock.patch.dict(os.environ, {self.ENV_TIMEOUT: "42"}, clear=True):
+            self.assertEqual(
+                TransactionServiceApi(
+                    EthereumNetwork.MAINNET, request_timeout=0
+                ).request_timeout,
+                0,
+            )