CodeRabbit Generated Unit Tests: Add unit tests

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

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

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