diff --git a/CHANGELOG.md b/CHANGELOG.md index 3263eea..ad2ce0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,17 @@ file. Install 1.8.2 to get a package whose self-reported version is truthful. Both now describe clamping. ### Fixed +- **`max_tokens` is validated on Solana too.** `validate_max_tokens` was called + from the Base client and nowhere else; the Solana client put the caller's + value straight into paid request bodies at all four chat entry points, so + `max_tokens=2_000_000` raised on Base and was signed and sent on Solana. With + the gateway clamping rather than rejecting, nothing on either side caught it. +- **`bool` no longer passes as a number.** `bool` is an `int` subclass, so + `isinstance(True, int)` is `True` and `max_tokens=True`, `temperature=True` + and `top_p=False` all sailed through and reached the wire as JSON `true` / + `false`. All three numeric validators now reject it, naming `bool` rather + than saying "must be a number" — which reads as wrong to anyone who knows + `bool` is one. - Line endings are normalized repo-wide via `.gitattributes` (`* text=auto`). 17 tracked files were CRLF against an otherwise-LF tree, which turned a 51-line change into a 1045-line diff in #27. diff --git a/blockrun_llm/solana_client.py b/blockrun_llm/solana_client.py index 68b52b2..6598281 100644 --- a/blockrun_llm/solana_client.py +++ b/blockrun_llm/solana_client.py @@ -67,6 +67,7 @@ sanitize_error_response, validate_api_url, validate_image_quality, + validate_max_tokens, validate_video_input_type, ) @@ -710,6 +711,7 @@ def chat_completion( client's chat baseline, ``DEFAULT_CHAT_TIMEOUT``). Raise it for large ``max_tokens`` runs against slow models. """ + validate_max_tokens(max_tokens) body: Dict[str, Any] = {"model": model, "messages": messages, "max_tokens": max_tokens} if temperature is not None: body["temperature"] = temperature @@ -810,6 +812,7 @@ def chat_completion_stream( Note: ``search_parameters`` is rejected by the BlockRun gateway in stream mode (HTTP 400). Codex / GPT-5.4-Pro also can't stream. """ + validate_max_tokens(max_tokens) body: Dict[str, Any] = { "model": model, "messages": messages, @@ -3006,6 +3009,7 @@ async def chat_completion( response_format: Optional[Dict[str, Any]] = None, stop: Optional[Union[str, List[str]]] = None, ) -> ChatResponse: + validate_max_tokens(max_tokens) body: Dict[str, Any] = {"model": model, "messages": messages, "max_tokens": max_tokens} if temperature is not None: body["temperature"] = temperature @@ -3054,6 +3058,7 @@ async def chat_completion_stream( """Async streaming. Same protocol semantics as the sync :meth:`SolanaLLMClient.chat_completion_stream`; only the iteration protocol differs (``async for``).""" + validate_max_tokens(max_tokens) body: Dict[str, Any] = { "model": model, "messages": messages, diff --git a/blockrun_llm/validation.py b/blockrun_llm/validation.py index a9736f4..b82738c 100644 --- a/blockrun_llm/validation.py +++ b/blockrun_llm/validation.py @@ -68,6 +68,17 @@ def _looks_like_solana_key(key: str) -> bool: return any(c in _BASE58_ONLY_CHARS for c in candidate) +# bool is a subclass of int in Python, so `isinstance(True, int)` is True and a +# bare numeric type check lets booleans straight through. Before this was fixed, +# validate_max_tokens(True), validate_temperature(True) and validate_top_p(False) +# all passed — the value then serialized as JSON `true`/`false` and went to the +# gateway as a request parameter. `max_tokens=False` was caught, but by the +# positivity check, so a type error was reported as a range error. +# +# Every numeric validator below excludes bool explicitly. Keep it that way when +# adding one. + + def validate_private_key(key: str) -> None: """ Validate that a private key is properly formatted. @@ -232,6 +243,11 @@ def validate_max_tokens(max_tokens: Optional[int]) -> None: """ Validate max_tokens parameter. + Rejects only values no request could have meant. The gateway does not + reject an over-ceiling ``max_tokens`` — it clamps to the model's own + ceiling and charges for the clamped value — so this guard exists to stop a + typo locally, not to enforce any model's limit. + Args: max_tokens: Maximum number of tokens to generate @@ -244,6 +260,13 @@ def validate_max_tokens(max_tokens: Optional[int]) -> None: if max_tokens is None: return + # bool is an int subclass, so `isinstance(True, int)` is True and a stray + # flag threaded into the wrong keyword would sail through and reach the + # wire as `"max_tokens": true`. Say so explicitly — "must be an integer" + # reads as wrong to anyone who knows bool is one. + if isinstance(max_tokens, bool): + raise ValueError("max_tokens must be an integer, got a bool") + if not isinstance(max_tokens, int): raise ValueError("max_tokens must be an integer") @@ -276,6 +299,9 @@ def validate_temperature(temperature: Optional[float]) -> None: if temperature is None: return + if isinstance(temperature, bool): + raise ValueError("temperature must be a number, got a bool") + if not isinstance(temperature, (int, float)): raise ValueError("temperature must be a number") @@ -299,6 +325,9 @@ def validate_top_p(top_p: Optional[float]) -> None: if top_p is None: return + if isinstance(top_p, bool): + raise ValueError("top_p must be a number, got a bool") + if not isinstance(top_p, (int, float)): raise ValueError("top_p must be a number") diff --git a/tests/unit/test_solana_max_tokens.py b/tests/unit/test_solana_max_tokens.py new file mode 100644 index 0000000..d8d0464 --- /dev/null +++ b/tests/unit/test_solana_max_tokens.py @@ -0,0 +1,86 @@ +"""max_tokens validation on the Solana chain (issue #31). + +The guard was Base-only: `validate_max_tokens` was called from client.py and +nowhere else, while solana_client.py put the caller's value straight into paid +request bodies. `max_tokens=2_000_000` raised on Base and was signed and sent on +Solana. Since the gateway clamps rather than rejects, there was no server-side +backstop behind the missing client-side one. + +Guarded with importorskip: the 3.9 CI job installs without the solana extra, and +an unguarded Solana test file turns that job red (see #19/#20). +""" + +from __future__ import annotations + +import unittest.mock as mock + +import httpx +import pytest + +pytest.importorskip("x402") +pytest.importorskip("solders") + +from blockrun_llm import SolanaLLMClient # noqa: E402 +from blockrun_llm.validation import MAX_TOKENS_SANITY_LIMIT # noqa: E402 + +MESSAGES = [{"role": "user", "content": "hi"}] + + +def _client() -> SolanaLLMClient: + """A client whose transport fails loudly. Validation must reject before any + request is built, so a passing test proves nothing reached the network.""" + + def explode(request: httpx.Request) -> httpx.Response: + raise AssertionError(f"validation let a bad max_tokens reach {request.url}") + + with ( + mock.patch("blockrun_llm.solana_client.register_exact_svm_client"), + mock.patch("blockrun_llm.solana_client._create_signer"), + ): + client = SolanaLLMClient( + private_key="bogus_signer_is_patched", + api_url="https://sol.blockrun.ai/api", + rpc_url="http://test", + ) + client._x402_client = mock.MagicMock() + client._client = httpx.Client(transport=httpx.MockTransport(explode)) + client._address = "11111111111111111111111111111111" + return client + + +class TestSolanaMaxTokensValidation: + """Every Solana chat entry point that puts max_tokens in a paid body.""" + + def test_chat_completion_rejects_implausible(self): + with pytest.raises(ValueError, match="implausibly large"): + _client().chat_completion("a/b", MESSAGES, max_tokens=2_000_000) + + def test_chat_completion_stream_rejects_implausible(self): + with pytest.raises(ValueError, match="implausibly large"): + list(_client().chat_completion_stream("a/b", MESSAGES, max_tokens=2_000_000)) + + def test_rejects_zero_and_negative(self): + for bad in (0, -1): + with pytest.raises(ValueError, match="positive"): + _client().chat_completion("a/b", MESSAGES, max_tokens=bad) + + def test_rejects_bool(self): + with pytest.raises(ValueError, match="bool"): + _client().chat_completion("a/b", MESSAGES, max_tokens=True) + + def test_real_ceilings_are_not_capped(self): + """The bound must never be the binding constraint on either chain. + These reach the transport, which is what the AssertionError proves.""" + for real_ceiling in (128_000, 262_144, MAX_TOKENS_SANITY_LIMIT): + with pytest.raises(AssertionError, match="reach"): + _client().chat_completion("a/b", MESSAGES, max_tokens=real_ceiling) + + def test_both_chains_share_one_bound(self): + """Base and Solana must agree, or the SDK's guard is not an invariant.""" + from blockrun_llm import LLMClient + + base = LLMClient(private_key="0x" + "11" * 32) + with pytest.raises(ValueError, match="implausibly large"): + base.chat_completion("a/b", MESSAGES, max_tokens=2_000_000) + with pytest.raises(ValueError, match="implausibly large"): + _client().chat_completion("a/b", MESSAGES, max_tokens=2_000_000) diff --git a/tests/unit/test_validation.py b/tests/unit/test_validation.py index ae68294..89ea00b 100644 --- a/tests/unit/test_validation.py +++ b/tests/unit/test_validation.py @@ -181,6 +181,14 @@ def test_reject_non_integer(self): with pytest.raises(ValueError, match="integer"): validate_max_tokens(100.5) # type: ignore + def test_reject_bool(self): + """bool is an int subclass, so `isinstance(True, int)` is True and a + flag threaded into the wrong keyword reached the wire as + `"max_tokens": true`. It is not a token count.""" + for bad in (True, False): + with pytest.raises(ValueError, match="bool"): + validate_max_tokens(bad) # type: ignore + class TestValidateTemperature: def test_accept_valid_values(self): @@ -209,6 +217,19 @@ def test_reject_non_number(self): with pytest.raises(ValueError, match="number"): validate_temperature("0.7") # type: ignore + def test_reject_bool(self): + """bool is an int subclass, so `isinstance(True, (int, float))` is True + and `temperature=True` serialized to the wire as JSON `true`.""" + for bad in (True, False): + with pytest.raises(ValueError, match="bool"): + validate_temperature(bad) # type: ignore + + def test_values_next_to_the_bools_still_pass(self): + """The guard must reject the type, not the neighbouring numbers.""" + validate_temperature(0) + validate_temperature(1) + validate_temperature(1.5) + class TestValidateTopP: def test_accept_valid_values(self): @@ -237,6 +258,18 @@ def test_reject_non_number(self): with pytest.raises(ValueError, match="number"): validate_top_p("0.9") # type: ignore + def test_reject_bool(self): + """Same hole as temperature: `top_p=False` reached the gateway as JSON + `false` instead of being caught as the wrong type.""" + for bad in (True, False): + with pytest.raises(ValueError, match="bool"): + validate_top_p(bad) # type: ignore + + def test_values_next_to_the_bools_still_pass(self): + validate_top_p(0) + validate_top_p(1) + validate_top_p(0.5) + class TestSanitizeErrorResponse: def test_extract_safe_fields(self):