diff --git a/backend/database_handler/accounts_manager.py b/backend/database_handler/accounts_manager.py index ee34a69ec..bd02ef034 100644 --- a/backend/database_handler/accounts_manager.py +++ b/backend/database_handler/accounts_manager.py @@ -75,12 +75,19 @@ def create_new_account_with_address( def is_valid_address(self, address: str) -> bool: return is_address(address) - def get_account(self, account_address: str) -> CurrentState | None: - """Private method to retrieve an account from the data base""" + @staticmethod + def _normalize_address(account_address: str) -> str: + """Normalize to the checksum form used as the primary key, matching + get_account()/create_new_account_with_address(). Falls back to the + original value if checksumming fails.""" try: - normalized = to_checksum_address(account_address) + return to_checksum_address(account_address) except Exception: - normalized = account_address + return account_address + + def get_account(self, account_address: str) -> CurrentState | None: + """Private method to retrieve an account from the data base""" + normalized = self._normalize_address(account_address) account = ( self.session.query(CurrentState) .filter(CurrentState.id == normalized) @@ -114,16 +121,12 @@ def debit_account_balance(self, account_address: str, amount: int) -> bool: """Atomic conditional debit. Returns False if insufficient balance.""" if amount <= 0: return True - try: - account_address = to_checksum_address(account_address) - except Exception: - pass result = self.session.execute( text( "UPDATE current_state SET balance = balance - :amount " "WHERE id = :addr AND balance >= :amount" ), - {"amount": amount, "addr": account_address}, + {"amount": amount, "addr": self._normalize_address(account_address)}, ) return result.rowcount > 0 @@ -131,10 +134,7 @@ def credit_account_balance(self, account_address: str, amount: int): """Atomic credit. Creates account if it doesn't exist.""" if amount <= 0: return - try: - account_address = to_checksum_address(account_address) - except Exception: - pass + account_address = self._normalize_address(account_address) self.session.execute( text( "INSERT INTO current_state (id, data, balance) " @@ -158,6 +158,7 @@ def credit_tx_value_once( Sets value_credited=true atomically. Returns True if credit was applied.""" if amount <= 0: return False + target_address = self._normalize_address(target_address) # Ensure target account exists self.session.execute( text( diff --git a/backend/protocol_rpc/endpoints.py b/backend/protocol_rpc/endpoints.py index 94f89a9aa..9977c4768 100644 --- a/backend/protocol_rpc/endpoints.py +++ b/backend/protocol_rpc/endpoints.py @@ -17,6 +17,9 @@ NotFoundError, QueueDepthExceeded, ) +from backend.protocol_rpc.redaction import ( + sanitize_rpc_private_keys as _sanitize_rpc_private_keys, +) from backend.protocol_rpc.contract_storage_quota import ( enforce_contract_storage_quota, live_state_column_size, @@ -167,40 +170,6 @@ def __init__( _gen_call_singleflight_lock = asyncio.Lock() -def _show_validator_private_keys_in_rpc() -> bool: - return os.getenv("SHOW_VALIDATOR_PRIVATE_KEYS_IN_RPC", "").strip().lower() in { - "1", - "true", - "yes", - "on", - } - - -def _is_private_key_field(key: Any) -> bool: - if not isinstance(key, str): - return False - normalized = key.replace("_", "").replace("-", "").lower() - return normalized == "privatekey" or normalized.endswith("privatekey") - - -def _sanitize_rpc_private_keys(value: Any) -> Any: - """Return RPC data with private-key fields removed unless explicitly enabled.""" - if _show_validator_private_keys_in_rpc(): - return value - - if isinstance(value, dict): - return { - key: _sanitize_rpc_private_keys(item) - for key, item in value.items() - if not _is_private_key_field(key) - } - if isinstance(value, list): - return [_sanitize_rpc_private_keys(item) for item in value] - if isinstance(value, tuple): - return tuple(_sanitize_rpc_private_keys(item) for item in value) - return value - - def _check_rate_limit(address: str) -> None: """Reject if address exceeds rate limit. Prunes old entries.""" now = time.monotonic() @@ -575,6 +544,8 @@ def fund_account( if normalized_amount <= 0: raise InvalidParams(message="amount must be a positive integer") + account_address = eth_utils.to_checksum_address(account_address) + import secrets nonce = transactions_processor.get_transaction_count(None) @@ -1616,7 +1587,9 @@ async def sim_call( genvm_manager, params, ) - return TransactionsProcessor._json_safe_numbers(receipt.to_dict()) + return TransactionsProcessor._json_safe_numbers( + _sanitize_rpc_private_keys(receipt.to_dict()) + ) async def sim_estimate_transaction_fees( @@ -1920,7 +1893,10 @@ async def execute(active_fee_accounting): raise JSONRPCError( code=-32000, message="execution failed", - data={"receipt": receipt.to_dict(), "params": params}, + data={ + "receipt": _sanitize_rpc_private_keys(receipt.to_dict()), + "params": params, + }, ) return receipt @@ -2474,7 +2450,9 @@ async def eth_call( if receipt.execution_result != ExecutionResultStatus.SUCCESS: raise JSONRPCError( - code=-32000, message="execution failed", data={"receipt": receipt.to_dict()} + code=-32000, + message="execution failed", + data={"receipt": _sanitize_rpc_private_keys(receipt.to_dict())}, ) return eth_utils.hexadecimal.encode_hex(receipt.result[1:]) diff --git a/backend/protocol_rpc/explorer/queries.py b/backend/protocol_rpc/explorer/queries.py index 053c1a7d9..ff01718b6 100644 --- a/backend/protocol_rpc/explorer/queries.py +++ b/backend/protocol_rpc/explorer/queries.py @@ -16,6 +16,7 @@ TransactionStatus, Validators, ) +from backend.protocol_rpc.redaction import sanitize_rpc_private_keys def _serialize_tx( @@ -30,7 +31,7 @@ def _serialize_tx( "to_address": tx.to_address, "input_data": tx.input_data, "data": tx.data, - "consensus_data": tx.consensus_data, + "consensus_data": sanitize_rpc_private_keys(tx.consensus_data), "nonce": tx.nonce, "value": tx.value, "type": tx.type, @@ -42,14 +43,16 @@ def _serialize_tx( "s": tx.s, "v": tx.v, "appeal_failed": tx.appeal_failed, - "consensus_history": tx.consensus_history, + "consensus_history": sanitize_rpc_private_keys(tx.consensus_history), "timestamp_appeal": tx.timestamp_appeal, "appeal_processing_time": tx.appeal_processing_time, "config_rotation_rounds": tx.config_rotation_rounds, "num_of_initial_validators": tx.num_of_initial_validators, "last_vote_timestamp": tx.last_vote_timestamp, "rotation_count": tx.rotation_count, - "leader_timeout_validators": tx.leader_timeout_validators, + "leader_timeout_validators": sanitize_rpc_private_keys( + tx.leader_timeout_validators + ), "sim_config": tx.sim_config, "triggered_by_hash": tx.triggered_by_hash, "triggered_on": tx.triggered_on, diff --git a/backend/protocol_rpc/redaction.py b/backend/protocol_rpc/redaction.py new file mode 100644 index 000000000..b6485abc0 --- /dev/null +++ b/backend/protocol_rpc/redaction.py @@ -0,0 +1,44 @@ +"""Redaction of validator private keys from RPC-facing payloads. + +Receipts carry the executing validator's ``node_config`` (including its +``private_key``); anything that returns a receipt to a client must pass +through :func:`sanitize_rpc_private_keys`. Shared by the JSON-RPC endpoints and +the explorer REST API. +""" + +import os +from typing import Any + + +def show_validator_private_keys_in_rpc() -> bool: + return os.getenv("SHOW_VALIDATOR_PRIVATE_KEYS_IN_RPC", "").strip().lower() in { + "1", + "true", + "yes", + "on", + } + + +def is_private_key_field(key: Any) -> bool: + if not isinstance(key, str): + return False + normalized = key.replace("_", "").replace("-", "").lower() + return normalized == "privatekey" or normalized.endswith("privatekey") + + +def sanitize_rpc_private_keys(value: Any) -> Any: + """Return RPC data with private-key fields removed unless explicitly enabled.""" + if show_validator_private_keys_in_rpc(): + return value + + if isinstance(value, dict): + return { + key: sanitize_rpc_private_keys(item) + for key, item in value.items() + if not is_private_key_field(key) + } + if isinstance(value, list): + return [sanitize_rpc_private_keys(item) for item in value] + if isinstance(value, tuple): + return tuple(sanitize_rpc_private_keys(item) for item in value) + return value diff --git a/docker-compose.yml b/docker-compose.yml index ff3dacffa..e838d06f2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -257,7 +257,7 @@ services: profiles: ["ollama"] image: ollama/ollama:0.17.6 ports: - - 11434:11434 + - "127.0.0.1:11434:11434" volumes: - ./.ollama:/root/.ollama container_name: ollama @@ -275,7 +275,7 @@ services: image: postgres:16-alpine command: sh -c "if [ \"$REMOTE_DATABASE\" = \"true\" ]; then echo 'Postgres disabled in hosted environment' && exec tail -f /dev/null; else exec docker-entrypoint.sh postgres; fi" ports: - - "${DBHOSTPORT:-5432}:5432" + - "127.0.0.1:${DBHOSTPORT:-5432}:5432" environment: - POSTGRES_USER=${DBUSER} - POSTGRES_PASSWORD=${DBPASSWORD} @@ -429,7 +429,7 @@ services: redis: image: redis:8-alpine ports: - - "${REDISPORT:-6379}:6379" + - "127.0.0.1:${REDISPORT:-6379}:6379" volumes: - redis_data:/data healthcheck: diff --git a/tests/unit/protocol_rpc/test_receipt_private_key_redaction.py b/tests/unit/protocol_rpc/test_receipt_private_key_redaction.py new file mode 100644 index 000000000..e5ffff6c4 --- /dev/null +++ b/tests/unit/protocol_rpc/test_receipt_private_key_redaction.py @@ -0,0 +1,232 @@ +"""Receipts returned or raised by the call endpoints and the explorer API must +not expose the executing validator's private key (see the redaction added for +the transaction/block getters in #1731).""" + +from contextlib import asynccontextmanager +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from backend.domain.types import LLMProvider, Validator +from backend.node.genvm import base as genvmbase +from backend.protocol_rpc import endpoints +from backend.protocol_rpc.exceptions import JSONRPCError +from backend.protocol_rpc.explorer import queries +from backend.validators import SingleValidatorSnapshot, Snapshot + +SECRET = "0x" + "ab" * 32 +CONTRACT = "0x8ba1f109551bD432803012645Ac136ddd64DBA72" +SENDER = "0x9F0e84243496AcFB3Cd99D02eA59673c05901501" + + +def _provider() -> LLMProvider: + return LLMProvider( + provider="openai", + model="gpt-4o", + config={}, + plugin="openai-compatible", + plugin_config={ + "api_key_env_var": "OPENAIKEY", + "api_url": "https://api.openai.com", + }, + ) + + +class _FakeContractSnapshot: + contract_address = CONTRACT + balance = 0 + states = {"accepted": {}, "finalized": {}} + genvm_executor_selector = None + + def __init__(self, *_args, **_kwargs): + pass + + +def _fake_run_genvm_host(result): + async def run(host_supplier, **_kwargs): + return genvmbase.ExecutionResult( + result=result, + eq_outputs={}, + pending_transactions=[], + stdout="", + stderr="", + genvm_log=[], + state=host_supplier.keywords["state_proxy"], + processing_time=1, + nondet_disagree=None, + ) + + return run + + +def _call_dependencies(monkeypatch, result): + monkeypatch.delenv("SHOW_VALIDATOR_PRIVATE_KEYS_IN_RPC", raising=False) + monkeypatch.setattr(genvmbase, "run_genvm_host", _fake_run_genvm_host(result)) + monkeypatch.setattr(endpoints, "ContractSnapshot", _FakeContractSnapshot) + monkeypatch.setattr(endpoints, "_check_rate_limit", lambda _address: None) + + validator = Validator( + address=SENDER, stake=1, llmprovider=_provider(), private_key=SECRET + ) + snapshot = Snapshot( + nodes=[SingleValidatorSnapshot(validator, {"node_address": SENDER})] + ) + + @asynccontextmanager + async def _snapshot(): + yield snapshot + + validators_manager = MagicMock() + validators_manager.snapshot = _snapshot + + accounts_manager = MagicMock() + accounts_manager.is_valid_address.return_value = True + + transactions_parser = MagicMock() + transactions_parser.decode_method_call_data.return_value = MagicMock( + calldata=b"\x00" + ) + + msg_handler = MagicMock() + msg_handler.with_client_session.return_value = MagicMock(client_session_id=None) + + return dict( + session=MagicMock(), + accounts_manager=accounts_manager, + msg_handler=msg_handler, + transactions_parser=transactions_parser, + validators_manager=validators_manager, + genvm_manager=MagicMock(url="http://127.0.0.1:1"), + ) + + +@pytest.mark.asyncio +async def test_sim_call_result_redacts_validator_private_key(monkeypatch): + deps = _call_dependencies(monkeypatch, genvmbase.ExecutionReturn(ret=b"\x00")) + + result = await endpoints.sim_call( + **deps, + params={"type": "read", "data": "0x00", "to": CONTRACT, "from": SENDER}, + ) + + assert result["node_config"]["address"] == SENDER + assert "private_key" not in result["node_config"] + + +@pytest.mark.asyncio +async def test_gen_call_execution_failed_error_redacts_validator_private_key( + monkeypatch, +): + deps = _call_dependencies( + monkeypatch, + genvmbase.ExecutionError(message="boom", kind=genvmbase.ResultCode.USER_ERROR), + ) + + with pytest.raises(JSONRPCError) as exc: + await endpoints.gen_call( + **deps, + params={"type": "read", "data": "0x00", "to": CONTRACT, "from": SENDER}, + ) + + receipt = exc.value.to_dict()["data"]["receipt"] + assert receipt["node_config"]["address"] == SENDER + assert "private_key" not in receipt["node_config"] + + +@pytest.mark.asyncio +async def test_eth_call_execution_failed_error_redacts_validator_private_key( + monkeypatch, +): + deps = _call_dependencies( + monkeypatch, + genvmbase.ExecutionError(message="boom", kind=genvmbase.ResultCode.USER_ERROR), + ) + monkeypatch.setattr(endpoints, "handle_consensus_data_call", lambda *_a: None) + + with pytest.raises(JSONRPCError) as exc: + await endpoints.eth_call( + **deps, + transactions_processor=MagicMock(), + params={"to": CONTRACT, "from": SENDER, "data": "0x00"}, + ) + + receipt = exc.value.to_dict()["data"]["receipt"] + assert "private_key" not in receipt["node_config"] + + +def _explorer_transaction(): + node_config = {"address": SENDER, "private_key": SECRET} + return SimpleNamespace( + hash="0xabc", + status=SimpleNamespace(value="FINALIZED"), + from_address=SENDER, + to_address=CONTRACT, + input_data=None, + data=None, + consensus_data={ + "leader_receipt": [{"node_config": dict(node_config)}], + "validators": [{"node_config": dict(node_config)}], + }, + nonce=0, + value=0, + type=2, + gaslimit=None, + created_at=None, + leader_only=False, + execution_mode=None, + r=None, + s=None, + v=None, + appeal_failed=0, + consensus_history={ + "consensus_results": [ + {"leader_result": [{"node_config": dict(node_config)}]} + ] + }, + timestamp_appeal=None, + appeal_processing_time=0, + config_rotation_rounds=None, + num_of_initial_validators=None, + last_vote_timestamp=None, + rotation_count=0, + leader_timeout_validators=[dict(node_config)], + sim_config=None, + triggered_by_hash=None, + triggered_on=None, + appealed=False, + appeal_undetermined=False, + appeal_leader_timeout=False, + appeal_validators_timeout=False, + timestamp_awaiting_finalization=None, + blocked_at=None, + worker_id=None, + ) + + +def test_explorer_serialize_tx_redacts_private_keys(monkeypatch): + monkeypatch.delenv("SHOW_VALIDATOR_PRIVATE_KEYS_IN_RPC", raising=False) + + serialized = queries._serialize_tx(_explorer_transaction()) + + assert serialized["consensus_data"]["leader_receipt"][0]["node_config"] == { + "address": SENDER + } + assert serialized["consensus_data"]["validators"][0]["node_config"] == { + "address": SENDER + } + assert serialized["consensus_history"]["consensus_results"][0]["leader_result"][0][ + "node_config" + ] == {"address": SENDER} + assert serialized["leader_timeout_validators"] == [{"address": SENDER}] + + +def test_explorer_serialize_tx_can_show_private_keys_for_local_debug(monkeypatch): + monkeypatch.setenv("SHOW_VALIDATOR_PRIVATE_KEYS_IN_RPC", "true") + + serialized = queries._serialize_tx(_explorer_transaction()) + + assert ( + serialized["consensus_data"]["leader_receipt"][0]["node_config"]["private_key"] + == SECRET + ) diff --git a/tests/unit/test_accounts_manager_normalization.py b/tests/unit/test_accounts_manager_normalization.py new file mode 100644 index 000000000..b7678b45b --- /dev/null +++ b/tests/unit/test_accounts_manager_normalization.py @@ -0,0 +1,77 @@ +"""Unit tests for AccountsManager address normalization in raw-SQL paths. + +debit_account_balance / credit_account_balance / credit_tx_value_once use +raw SQL and previously received the address verbatim, while ORM paths +(get_account / create_new_account_with_address) store rows keyed by +to_checksum_address(). A lowercase address therefore created a duplicate +account row and split balances. These tests exercise the fix with a mocked +SQLAlchemy session — no database required. +""" + +from unittest.mock import MagicMock + +from eth_utils import to_checksum_address + +from backend.database_handler.accounts_manager import AccountsManager + +RAW_ADDRESS = "0x71c7656ec7ab88b098defb751b7401b5f6d8976f" # lowercase on purpose +CHECKSUMMED = to_checksum_address(RAW_ADDRESS) +assert RAW_ADDRESS != CHECKSUMMED # fixture sanity: casing must differ + + +def _executed_params(session: MagicMock) -> list[dict]: + return [call.args[1] for call in session.execute.call_args_list] + + +def test_debit_normalizes_address(): + session = MagicMock() + session.execute.return_value = MagicMock(rowcount=1) + assert AccountsManager(session).debit_account_balance(RAW_ADDRESS, 5) is True + + (params,) = [p for p in _executed_params(session) if p.get("addr")] + assert params["addr"] == CHECKSUMMED + + +def test_credit_normalizes_address_before_insert_and_update(): + session = MagicMock() + AccountsManager(session).credit_account_balance(RAW_ADDRESS, 7) + + params = [p for p in _executed_params(session) if p.get("addr")] + assert len(params) == 2 # INSERT ... ON CONFLICT + UPDATE + assert all(p["addr"] == CHECKSUMMED for p in params) + + +def test_credit_tx_value_once_normalizes_target(): + session = MagicMock() + # value_credited update must match so the credit path is taken + session.execute.return_value = MagicMock(rowcount=1) + + manager = AccountsManager(session) + trx = manager.credit_tx_value_once("0x12" + "34" * 16, RAW_ADDRESS, 3) + + assert trx is True + params = [p for p in _executed_params(session) if p.get("addr")] + assert params + assert all(p["addr"] == CHECKSUMMED for p in params) + + +def test_credit_zero_or_negative_amount_is_noop(): + session = MagicMock() + manager = AccountsManager(session) + + manager.credit_account_balance(RAW_ADDRESS, 0) + manager.credit_account_balance(RAW_ADDRESS, -10) + assert manager.debit_account_balance(RAW_ADDRESS, 0) is True + + session.execute.assert_not_called() + + +def test_normalize_fallback_keeps_original(): + """A value that to_checksum_address rejects must be used unchanged, + preserving the pre-existing behavior for non-address identifiers.""" + session = MagicMock() + session.execute.return_value = MagicMock(rowcount=1) + AccountsManager(session).debit_account_balance("not-an-address", 5) + + (params,) = [p for p in _executed_params(session) if p.get("addr")] + assert params["addr"] == "not-an-address" diff --git a/tests/unit/test_docker_compose_security.py b/tests/unit/test_docker_compose_security.py new file mode 100644 index 000000000..1c39c62e7 --- /dev/null +++ b/tests/unit/test_docker_compose_security.py @@ -0,0 +1,25 @@ +from pathlib import Path + +import yaml + + +REPOSITORY_ROOT = Path(__file__).resolve().parents[2] + + +def _compose_ports(service: str) -> list[str]: + compose = yaml.safe_load( + (REPOSITORY_ROOT / "docker-compose.yml").read_text(encoding="utf-8") + ) + return compose["services"][service]["ports"] + + +def test_postgres_is_bound_to_loopback_by_default(): + assert _compose_ports("postgres") == ["127.0.0.1:${DBHOSTPORT:-5432}:5432"] + + +def test_redis_is_bound_to_loopback_by_default(): + assert _compose_ports("redis") == ["127.0.0.1:${REDISPORT:-6379}:6379"] + + +def test_ollama_is_bound_to_loopback_by_default(): + assert _compose_ports("ollama") == ["127.0.0.1:11434:11434"] diff --git a/tests/unit/test_simulator_sessions.py b/tests/unit/test_simulator_sessions.py index 88d03aad2..7a3f9bebd 100644 --- a/tests/unit/test_simulator_sessions.py +++ b/tests/unit/test_simulator_sessions.py @@ -1114,3 +1114,42 @@ def transactions_processor_factory(s): new_status=TransactionStatus.FINALIZED, update_current_status_changes=True, ) + + +def test_fund_account_normalizes_lowercase_address(monkeypatch): + session = MagicMock() + accounts_manager_instance = MagicMock() + accounts_manager_instance.is_valid_address.return_value = True + + transactions_processor_instance = MagicMock() + transactions_processor_instance.get_transaction_count.return_value = 5 + transactions_processor_instance.insert_transaction.return_value = None + + def mock_token_hex(n): + return "def" if n == 32 else "xxx" + + monkeypatch.setattr("secrets.token_hex", mock_token_hex) + + monkeypatch.setattr( + endpoints, + "AccountsManager", + lambda s: accounts_manager_instance if s is session else None, + ) + monkeypatch.setattr( + endpoints, + "TransactionsProcessor", + lambda s: transactions_processor_instance if s is session else None, + ) + + lowercase = "0x" + "ab" * 20 + result = endpoints.fund_account(session, lowercase, 100) + + assert result == "0xdef" + checksummed = "0xABaBaBaBABabABabAbAbABAbABabababaBaBABaB" + transactions_processor_instance.insert_transaction.assert_called_once_with( + None, checksummed, None, 100, 0, 5, False, 0, None, "0xdef" + ) + accounts_manager_instance.credit_tx_value_once.assert_called_once_with( + "0xdef", checksummed, 100 + ) + session.commit.assert_called_once()