diff --git a/money-machine/src-python/tests/test_config_security.py b/money-machine/src-python/tests/test_config_security.py index 6f30ca9..b63ec03 100644 --- a/money-machine/src-python/tests/test_config_security.py +++ b/money-machine/src-python/tests/test_config_security.py @@ -12,6 +12,7 @@ from engine.trading_core import validate_config_update # noqa: E402 from utils import config as config_module # noqa: E402 +from utils.config import _without_secrets, _deep_merge # noqa: E402 def test_save_config_redacts_nested_secrets(tmp_path, monkeypatch) -> None: @@ -80,3 +81,213 @@ def test_validate_config_update_enforces_ranges() -> None: validate_config_update({"initial_balance": 50}) with pytest.raises(ValueError): validate_config_update({"max_daily_loss": 0.0}) + + +# --------------------------------------------------------------------------- +# validate_config_update – additional boundary and type checks +# --------------------------------------------------------------------------- + + +def test_validate_config_update_empty_dict_returns_empty() -> None: + assert validate_config_update({}) == {} + + +def test_validate_config_update_non_dict_raises() -> None: + with pytest.raises(ValueError): + validate_config_update([]) # type: ignore[arg-type] + with pytest.raises(ValueError): + validate_config_update("nope") # type: ignore[arg-type] + + +def test_validate_config_update_boolean_rejected() -> None: + # bool is a subclass of int, but must be explicitly rejected. + with pytest.raises(ValueError): + validate_config_update({"initial_balance": True}) + with pytest.raises(ValueError): + validate_config_update({"max_risk_per_trade": False}) + + +def test_validate_config_update_string_value_rejected() -> None: + with pytest.raises(ValueError): + validate_config_update({"initial_balance": "10000"}) + + +def test_validate_config_update_initial_balance_inclusive_min_boundary() -> None: + # 100.0 is the inclusive minimum; must be accepted. + result = validate_config_update({"initial_balance": 100.0}) + assert result == {"initial_balance": 100.0} + + +def test_validate_config_update_initial_balance_at_max_boundary() -> None: + result = validate_config_update({"initial_balance": 1_000_000.0}) + assert result == {"initial_balance": 1_000_000.0} + + +def test_validate_config_update_initial_balance_below_min_rejected() -> None: + with pytest.raises(ValueError): + validate_config_update({"initial_balance": 99.9}) + + +def test_validate_config_update_initial_balance_above_max_rejected() -> None: + with pytest.raises(ValueError): + validate_config_update({"initial_balance": 1_000_000.01}) + + +def test_validate_config_update_max_risk_per_trade_exclusive_min_rejected() -> None: + # max_risk_per_trade has exclusive_minimum=True so 0.0 must be rejected. + with pytest.raises(ValueError): + validate_config_update({"max_risk_per_trade": 0.0}) + + +def test_validate_config_update_max_risk_per_trade_at_max_accepted() -> None: + # 0.1 == maximum; the check is `value > maximum` so 0.1 is accepted. + result = validate_config_update({"max_risk_per_trade": 0.1}) + assert result == {"max_risk_per_trade": 0.1} + + +def test_validate_config_update_max_daily_loss_at_max_accepted() -> None: + result = validate_config_update({"max_daily_loss": 0.2}) + assert result == {"max_daily_loss": 0.2} + + +def test_validate_config_update_multiple_valid_keys() -> None: + result = validate_config_update( + {"initial_balance": 5000.0, "max_risk_per_trade": 0.05, "max_daily_loss": 0.1} + ) + assert result == { + "initial_balance": 5000.0, + "max_risk_per_trade": 0.05, + "max_daily_loss": 0.1, + } + + +def test_validate_config_update_int_values_accepted_as_float() -> None: + # int literals are allowed; the function casts them to float. + result = validate_config_update({"initial_balance": 500}) + assert result == {"initial_balance": 500.0} + assert isinstance(result["initial_balance"], float) + + +# --------------------------------------------------------------------------- +# _without_secrets – unit tests +# --------------------------------------------------------------------------- + + +def test_without_secrets_removes_top_level_secret_keys() -> None: + data = {"api_key": "s", "secret": "s", "gemini_api_key": "s", "name": "binance"} + assert _without_secrets(data) == {"name": "binance"} + + +def test_without_secrets_case_insensitive_key_matching() -> None: + # Keys checked with .lower(); mixed-case variants must be stripped. + data = {"API_KEY": "leak", "Secret": "leak", "model": "x"} + result = _without_secrets(data) + assert "API_KEY" not in result + assert "Secret" not in result + assert result.get("model") == "x" + + +def test_without_secrets_handles_nested_dicts() -> None: + data = {"exchange": {"api_key": "k", "secret": "s", "name": "binance"}} + assert _without_secrets(data) == {"exchange": {"name": "binance"}} + + +def test_without_secrets_handles_list_of_dicts() -> None: + data = [{"api_key": "leak", "name": "binance"}, {"secret": "s", "label": "main"}] + result = _without_secrets(data) + assert result == [{"name": "binance"}, {"label": "main"}] + + +def test_without_secrets_primitives_pass_through() -> None: + assert _without_secrets(42) == 42 + assert _without_secrets("hello") == "hello" + assert _without_secrets(3.14) == 3.14 + assert _without_secrets(None) is None + assert _without_secrets(True) is True + + +def test_without_secrets_empty_dict_returns_empty() -> None: + assert _without_secrets({}) == {} + + +def test_without_secrets_non_secret_keys_preserved_in_nested() -> None: + data = {"exchange": {"name": "bybit", "sandbox": True}} + assert _without_secrets(data) == {"exchange": {"name": "bybit", "sandbox": True}} + + +# --------------------------------------------------------------------------- +# _deep_merge – unit tests +# --------------------------------------------------------------------------- + + +def test_deep_merge_nested_dicts_recursive() -> None: + base = {"a": {"x": 1, "y": 2}, "b": 10} + update = {"a": {"y": 99, "z": 3}} + _deep_merge(base, update) + assert base == {"a": {"x": 1, "y": 99, "z": 3}, "b": 10} + + +def test_deep_merge_overwrites_scalar_with_scalar() -> None: + base = {"key": "old"} + _deep_merge(base, {"key": "new"}) + assert base["key"] == "new" + + +def test_deep_merge_adds_new_top_level_key() -> None: + base: dict = {} + _deep_merge(base, {"fresh": 42}) + assert base == {"fresh": 42} + + +def test_deep_merge_overwrites_dict_with_scalar_when_update_is_scalar() -> None: + # If base has a dict but update has a scalar, the scalar wins. + base = {"key": {"nested": 1}} + _deep_merge(base, {"key": "flat"}) + assert base["key"] == "flat" + + +# --------------------------------------------------------------------------- +# save_config – error path +# --------------------------------------------------------------------------- + + +def test_save_config_returns_false_on_write_error(tmp_path, monkeypatch) -> None: + # Point __file__ at a path whose parent cannot be written to. + fake_module = tmp_path / "utils" / "config.py" + fake_module.parent.mkdir() + fake_module.write_text("") + # Make the target config.json un-writable by replacing open with a raiser. + import builtins + + real_open = builtins.open + + def _raising_open(path, mode="r", **kw): + if "w" in str(mode): + raise OSError("disk full") + return real_open(path, mode, **kw) + + monkeypatch.setattr(config_module, "__file__", str(fake_module)) + monkeypatch.setattr(builtins, "open", _raising_open) + + result = config_module.save_config({"max_risk_per_trade": 0.01}) + assert result is False + + +# --------------------------------------------------------------------------- +# load_config – non-secret file keys are preserved after merge +# --------------------------------------------------------------------------- + + +def test_load_config_non_secret_file_keys_are_preserved(tmp_path, monkeypatch) -> None: + fake_module = tmp_path / "utils" / "config.py" + fake_module.parent.mkdir() + fake_module.write_text("") + (tmp_path / "config.json").write_text( + json.dumps({"gemini_model": "gemini-1.5-pro", "max_risk_per_trade": 0.01}) + ) + monkeypatch.setattr(config_module, "__file__", str(fake_module)) + + loaded = config_module.load_config() + # Non-secret key from file must survive the merge. + assert loaded["gemini_model"] == "gemini-1.5-pro" + assert loaded["max_risk_per_trade"] == 0.01 diff --git a/money-machine/src-python/tests/test_ipc_auth.py b/money-machine/src-python/tests/test_ipc_auth.py index f5bf244..9755ef6 100644 --- a/money-machine/src-python/tests/test_ipc_auth.py +++ b/money-machine/src-python/tests/test_ipc_auth.py @@ -237,6 +237,53 @@ async def scenario() -> None: _run(scenario()) +def test_oversized_auth_header_returns_413() -> None: + async def scenario() -> None: + server, task, (host, port) = await _start_server() + try: + # Craft an auth header line that exceeds MAX_AUTH_LINE_BYTES. + oversized_token = "x" * (IPCServer.MAX_AUTH_LINE_BYTES + 1) + auth_line = f"X-Auth-Token: {oversized_token}\n".encode("utf-8") + response = await _send_raw(host, port, auth_line) + assert response.get("code") == 413, response + finally: + await _shutdown(server, task) + + _run(scenario()) + + +def test_invalid_json_body_returns_400() -> None: + async def scenario() -> None: + server, task, (host, port) = await _start_server() + try: + # Valid auth header but the body is not parseable JSON. + wire = f"X-Auth-Token: {TEST_TOKEN}\nnot valid json at all\n".encode("utf-8") + response = await _send_raw(host, port, wire) + assert response.get("code") == 400, response + assert "json" in response.get("error", "").lower() + finally: + await _shutdown(server, task) + + _run(scenario()) + + +def test_missing_command_field_returns_400() -> None: + async def scenario() -> None: + server, task, (host, port) = await _start_server() + try: + # Valid auth, valid JSON body, but no "command" key. + import json as _json + body = _json.dumps({"payload": {"hello": "world"}}) + wire = f"X-Auth-Token: {TEST_TOKEN}\n{body}\n".encode("utf-8") + response = await _send_raw(host, port, wire) + assert response.get("code") == 400, response + assert "command" in response.get("error", "").lower() + finally: + await _shutdown(server, task) + + _run(scenario()) + + if __name__ == "__main__": # Allow running this file directly: `python tests/test_ipc_auth.py`. import traceback diff --git a/money-machine/src-python/tests/test_mt5_adapter.py b/money-machine/src-python/tests/test_mt5_adapter.py index 3855b12..a9aa5f5 100644 --- a/money-machine/src-python/tests/test_mt5_adapter.py +++ b/money-machine/src-python/tests/test_mt5_adapter.py @@ -1007,6 +1007,53 @@ async def scenario() -> None: _run(scenario()) +# --------------------------------------------------------------------------- +# venue_order_id relaxation (PR: removed fail-closed check) +# --------------------------------------------------------------------------- + + +def test_place_order_accepts_missing_venue_order_id_in_2xx_dict_body() -> None: + """After removing the fail-closed venue_order_id check, a 2xx + response whose JSON dict does NOT contain `venue_order_id` must + produce a PENDING result with venue_order_id=None instead of + being rejected. + """ + async def scenario() -> None: + body = b'{"status":"queued"}' # valid dict, no venue_order_id key + http = _FakeHttp([HttpResponse(status=202, body=body, headers={})]) + adapter = MT5Adapter( + http_client=http, + signer=_signer(), + config=MT5Config(max_retries=0), + ) + result = await adapter.place_order(_request("ord-no-vid")) + assert result.status is OrderStatus.PENDING, result + assert result.venue_order_id is None + + _run(scenario()) + + +def test_place_order_accepts_empty_string_venue_order_id_in_2xx_body() -> None: + """Before the PR the server would fail-closed if venue_order_id was + an empty string. The relaxed code must now accept it and surface the + empty string as venue_order_id rather than rejecting the order. + """ + async def scenario() -> None: + body = b'{"venue_order_id":""}' + http = _FakeHttp([HttpResponse(status=202, body=body, headers={})]) + adapter = MT5Adapter( + http_client=http, + signer=_signer(), + config=MT5Config(max_retries=0), + ) + result = await adapter.place_order(_request("ord-empty-vid")) + assert result.status is OrderStatus.PENDING, result + # Empty string is falsy but no longer causes a rejection. + assert result.venue_order_id == "" + + _run(scenario()) + + def test_non_2xx_still_rejected_regardless_of_venue_order_id() -> None: """Non-2xx responses must still return REJECTED even if the body contains a venue_order_id. The venue_order_id validation removal must diff --git a/money-machine/src-python/tests/test_signal_generator_metadata.py b/money-machine/src-python/tests/test_signal_generator_metadata.py new file mode 100644 index 0000000..8295422 --- /dev/null +++ b/money-machine/src-python/tests/test_signal_generator_metadata.py @@ -0,0 +1,198 @@ +""" +Tests for signal_generator._parse_json_response metadata changes. + +PR change: the `amount` field was removed from TradingSignal; instead +`amount_pct` is now stored inside `metadata` so downstream consumers +can still access it without breaking the dataclass contract. + +These tests exercise _parse_json_response in isolation: +- amount_pct present in Gemini JSON → surfaced in metadata dict +- amount_pct absent → metadata contains None for the key +- amount_pct explicitly null → metadata contains None +- Non-JSON response → fallback HOLD signal returned +- Market data drives entry_price when entry_price is absent from JSON +""" +from __future__ import annotations + +import json +import sys +from pathlib import Path +from typing import List +from unittest.mock import MagicMock + +import pytest + +SRC_PYTHON = Path(__file__).resolve().parent.parent +if str(SRC_PYTHON) not in sys.path: + sys.path.insert(0, str(SRC_PYTHON)) + +# engine.strategies.base imports pandas at module level; skip the whole +# module gracefully when pandas is not installed so the rest of the +# suite keeps running. +pandas = pytest.importorskip("pandas") + +from engine.signal_generator import SignalGenerator # noqa: E402 +from engine.strategies.base import TradingSignal # noqa: E402 + + +def _make_ohlcv(close: float = 50_000.0, n: int = 1) -> List[List]: + """Return n minimal OHLCV candles all with the given close price.""" + ts = 1_700_000_000_000 # arbitrary ms timestamp + return [[ts + i * 60_000, close, close, close, close, 100.0] for i in range(n)] + + +def _generator() -> SignalGenerator: + """Return a SignalGenerator with no API key (model=None).""" + return SignalGenerator(api_key="") + + +# --------------------------------------------------------------------------- +# _parse_json_response – metadata / amount_pct +# --------------------------------------------------------------------------- + + +def test_parse_json_response_stores_amount_pct_in_metadata() -> None: + gen = _generator() + market_data = _make_ohlcv(50_000.0) + response_json = json.dumps({ + "action": "BUY", + "confidence": 0.8, + "entry_price": 50_100.0, + "stop_loss": 49_500.0, + "take_profit": 51_000.0, + "amount_pct": 0.02, + "reasoning": "Strong momentum", + }) + + signal = gen._parse_json_response("BTC/USDT", response_json, market_data) + + assert isinstance(signal, TradingSignal) + assert signal.action == "BUY" + assert signal.metadata.get("amount_pct") == pytest.approx(0.02) + + +def test_parse_json_response_missing_amount_pct_stores_none() -> None: + gen = _generator() + market_data = _make_ohlcv(50_000.0) + response_json = json.dumps({ + "action": "SELL", + "confidence": 0.6, + "reasoning": "Overbought", + # amount_pct deliberately absent + }) + + signal = gen._parse_json_response("ETH/USDT", response_json, market_data) + + assert signal.action == "SELL" + assert "amount_pct" in signal.metadata + assert signal.metadata["amount_pct"] is None + + +def test_parse_json_response_explicit_null_amount_pct_stored_as_none() -> None: + gen = _generator() + market_data = _make_ohlcv(50_000.0) + response_json = json.dumps({ + "action": "HOLD", + "confidence": 0.4, + "amount_pct": None, + "reasoning": "Unclear", + }) + + signal = gen._parse_json_response("BTC/USDT", response_json, market_data) + + assert signal.metadata["amount_pct"] is None + + +def test_parse_json_response_amount_pct_not_on_top_level_signal() -> None: + """amount_pct must NOT be stored as a top-level TradingSignal field.""" + gen = _generator() + market_data = _make_ohlcv(50_000.0) + response_json = json.dumps({ + "action": "BUY", + "confidence": 0.75, + "amount_pct": 0.015, + "reasoning": "Breakout", + }) + + signal = gen._parse_json_response("BTC/USDT", response_json, market_data) + + # amount_pct should live in metadata, NOT as a standalone attribute + assert not hasattr(signal, "amount") or signal.__class__.__name__ == "TradingSignal" + assert signal.metadata["amount_pct"] == pytest.approx(0.015) + + +def test_parse_json_response_invalid_json_returns_hold_fallback() -> None: + gen = _generator() + market_data = _make_ohlcv(50_000.0) + + signal = gen._parse_json_response("BTC/USDT", "not json at all {{", market_data) + + assert signal.action == "HOLD" + assert signal.confidence == pytest.approx(0.3) + + +def test_parse_json_response_empty_json_object_returns_hold_default() -> None: + gen = _generator() + market_data = _make_ohlcv(50_000.0) + # Empty dict: action defaults to HOLD, confidence defaults to 0.5 + signal = gen._parse_json_response("BTC/USDT", json.dumps({}), market_data) + assert signal.action == "HOLD" + + +def test_parse_json_response_uses_current_price_when_entry_price_absent() -> None: + gen = _generator() + close = 42_000.0 + market_data = _make_ohlcv(close) + response_json = json.dumps({ + "action": "BUY", + "confidence": 0.7, + "entry_price": None, # null → falls back to current price + "reasoning": "Dip", + }) + + signal = gen._parse_json_response("BTC/USDT", response_json, market_data) + + assert signal.entry_price == pytest.approx(close) + + +def test_parse_json_response_uses_provided_entry_price_when_present() -> None: + gen = _generator() + market_data = _make_ohlcv(50_000.0) + response_json = json.dumps({ + "action": "BUY", + "confidence": 0.8, + "entry_price": 49_800.0, + "reasoning": "Limit entry", + }) + + signal = gen._parse_json_response("BTC/USDT", response_json, market_data) + + assert signal.entry_price == pytest.approx(49_800.0) + + +def test_parse_json_response_symbol_is_preserved() -> None: + gen = _generator() + market_data = _make_ohlcv(1_800.0) + response_json = json.dumps({"action": "HOLD", "confidence": 0.5}) + + signal = gen._parse_json_response("ETH/USDT", response_json, market_data) + + assert signal.symbol == "ETH/USDT" + + +# --------------------------------------------------------------------------- +# _generate_rule_based_signal – metadata field present (regression guard) +# --------------------------------------------------------------------------- + + +def test_rule_based_signal_has_metadata_field() -> None: + """The rule-based fallback must also produce a TradingSignal that has + a metadata dict (even if empty) since TradingSignal now always has one. + """ + gen = _generator() + # 21 candles so the rule-based path runs fully. + market_data = _make_ohlcv(50_000.0, n=21) + + signal = gen._generate_rule_based_signal("BTC/USDT", market_data) + + assert isinstance(signal.metadata, dict)