fix: API uses .signHash raw hash signing (dangerous)

ape_safe/client/__init__.py

@@ -3,8 +3,11 @@
 from typing import Dict, Iterator, List, Optional, Union, cast
 
 from ape.api import AccountAPI
+from ape.logging import logger
 from ape.types import AddressType, HexBytes, MessageSignature
+from ape_accounts import KeyfileAccount
 from eip712.common import SafeTxV1, SafeTxV2
+from eth_account import Account as EthAccount
 
 from ape_safe.client.base import BaseSafeClient
 from ape_safe.client.mock import MockSafeClient
@@ -197,26 +200,47 @@ def get_delegates(self) -> Dict[AddressType, List[AddressType]]:
         return delegates
 
     def add_delegate(self, delegate: AddressType, label: str, delegator: AccountAPI):
-        msg = self.create_delegate_message(delegate)
-        if not (sig := delegator.sign_message(msg)):
-            raise ActionNotPerformedError("Must sign request to add delegate.")
+        # TODO: Replace this by adding raw hash signing into supported account plugins
+        if not isinstance(delegator, KeyfileAccount):
+            raise ActionNotPerformedError("Need access to private key for this method.")
+
+        logger.warning("Need to unlock account to add a delegate.")
+        delegator.unlock()  # NOTE: Ensures we have the key handy
+
+        msg_hash = self.create_delegate_message(delegate)
+        # NOTE: This is required as Safe API uses an antiquated .signHash method
+        sig = EthAccount.signHash(
+            msg_hash,
+            delegator._KeyfileAccount__cached_key,  # type: ignore[attr-defined]
+        )
 
         payload = {
             "safe": self.address,
             "delegate": delegate,
             "delegator": delegator.address,
             "label": label,
-            "signature": sig.encode_rsv().hex(),
+            "signature": sig.signature.hex(),
         }
         self._post("delegates", json=payload)
 
     def remove_delegate(self, delegate: AddressType, delegator: AccountAPI):
-        msg = self.create_delegate_message(delegate)
-        if not (sig := delegator.sign_message(msg)):
-            raise ActionNotPerformedError("Must sign request to remove delegate.")
+        # TODO: Replace this by adding raw hash signing into supported account plugins
+        if not isinstance(delegator, KeyfileAccount):
+            raise ActionNotPerformedError("Need access to private key for this method.")
+
+        logger.warning("Need to unlock account to add a delegate.")
+        delegator.unlock()  # NOTE: Ensures we have the key handy
+
+        msg_hash = self.create_delegate_message(delegate)
+        # NOTE: This is required as Safe API uses an antiquated .signHash method
+        sig = EthAccount.signHash(
+            msg_hash,
+            delegator._KeyfileAccount__cached_key,  # type: ignore[attr-defined]
+        )
 
         payload = {
-            "signature": sig.encode_rsv().hex(),
+            "delegator": delegator.address,
+            "signature": sig.signature.hex(),
         }
         self._delete(f"delegates/{delegate}", json=payload)
 

ape_safe/client/base.py

@@ -8,8 +8,7 @@
 import requests
 import urllib3
 from ape.api import AccountAPI
-from ape.types import AddressType, MessageSignature, SignableMessage
-from eth_account.messages import encode_defunct as encode_eip191_signable_message
+from ape.types import AddressType, MessageSignature
 from eth_utils import keccak
 from requests import Response
 from requests.adapters import HTTPAdapter
@@ -115,11 +114,11 @@ def get_transactions(
 
             yield txn
 
-    def create_delegate_message(self, delegate: AddressType) -> SignableMessage:
+    def create_delegate_message(self, delegate: AddressType) -> bytes:
         # NOTE: referencing https://github.com/safe-global/safe-eth-py/blob/
         # a0a5771622f143ee6301cfc381c5ed50832ff482/gnosis/safe/api/transaction_service_api.py#L34
         totp = int(time.time()) // 3600
-        return encode_eip191_signable_message(keccak(text=(delegate + str(totp))))
+        return keccak(text=(delegate + str(totp)))
 
     @abstractmethod
     def get_delegates(self) -> dict[AddressType, list[AddressType]]: ...