diff --git a/packages/python-sdk-memwal/memwal/client.py b/packages/python-sdk-memwal/memwal/client.py index fd2895a66..f34904176 100644 --- a/packages/python-sdk-memwal/memwal/client.py +++ b/packages/python-sdk-memwal/memwal/client.py @@ -410,6 +410,29 @@ def _occurred_at_to_wire( ) + +def _check_limit(limit: Any) -> None: + """Reject a limit that is not a positive integer before it is signed (#1218).""" + if isinstance(limit, bool) or not isinstance(limit, int) or limit <= 0: + raise ValueError(f"limit must be a positive integer, got {limit!r}") + + +def _check_vector(vector: Any) -> None: + """Reject a vector that is not a non-empty list of finite numbers (#1218). + + json.dumps would otherwise emit bare NaN/Infinity tokens, which are not + JSON, and the relayer would answer with an opaque parse error. + """ + if ( + not isinstance(vector, (list, tuple)) + or len(vector) == 0 + or not all( + isinstance(v, (int, float)) and not isinstance(v, bool) and math.isfinite(v) + for v in vector + ) + ): + raise ValueError("vector must be a non-empty list of finite numbers") + class MemWal: """Async-native Walrus Memory client. @@ -1049,6 +1072,7 @@ async def recall( sort = params.sort else: query_text = query + _check_limit(limit) body = { "query": query_text, "limit": limit, @@ -1291,6 +1315,7 @@ async def restore(self, namespace: str, limit: int = 10) -> RestoreResult: downloads in parallel, then 3 SEAL decrypts in parallel, then embeddings. Expect seconds-per-blob on cold caches. """ + _check_limit(limit) data = await self._signed_request("POST", "/api/restore", { "namespace": namespace, "limit": limit, @@ -1346,6 +1371,10 @@ async def list_namespaces( Returns: :class:`NamespacesResult`. """ + # Validate before resolving the owner: on a fresh client that lookup is + # a signed POST /api/stats, so a bad limit must not reach it (#1218). + if limit is not None: + _check_limit(limit) owner = await self._resolve_owner() params: Dict[str, str] = {} @@ -1420,6 +1449,7 @@ async def remember_manual(self, opts: RememberManualOptions) -> RememberManualRe Returns: :class:`RememberManualResult` with id, blob_id, owner, namespace. """ + _check_vector(opts.vector) data = await self._signed_request( "POST", "/api/remember/manual", @@ -1449,6 +1479,8 @@ async def recall_manual(self, opts: RecallManualOptions) -> RecallManualResult: Returns: :class:`RecallManualResult` with blob_id + distance pairs. """ + _check_vector(opts.vector) + _check_limit(opts.limit) body: Dict[str, Any] = { "vector": opts.vector, "limit": opts.limit, @@ -1743,7 +1775,9 @@ async def _signed_request( method_upper = method.upper() timestamp = str(int(time.time())) - body_str = "" if method_upper == "GET" else json.dumps(body, separators=(",", ":")) + body_str = "" if method_upper == "GET" else json.dumps( + body, separators=(",", ":"), allow_nan=False + ) body_hash = sha256_hex(body_str) # MED-1 / LOW-23: nonce + account_id are part of the canonical signed # message. Server rejects the request as "unsupported legacy SDK" diff --git a/packages/python-sdk-memwal/memwal/mock.py b/packages/python-sdk-memwal/memwal/mock.py index 7a5d09ca1..a52e38fb7 100644 --- a/packages/python-sdk-memwal/memwal/mock.py +++ b/packages/python-sdk-memwal/memwal/mock.py @@ -12,6 +12,7 @@ from datetime import datetime, timedelta, timezone from typing import Any, Dict, List, Optional, Sequence, Union +from .client import _check_limit from .types import ( AnalyzedFact, AnalyzeResult, @@ -279,8 +280,7 @@ async def recall( sort = None query_text = query _validate_text(query_text, "query") - if not isinstance(limit, int) or limit < 0: - raise ValueError("limit must be a non-negative integer") + _check_limit(limit) if sort not in (None, "relevance", "recent"): raise ValueError("sort must be 'relevance' or 'recent'") resolved_namespace = namespace or self._namespace @@ -381,7 +381,7 @@ async def ask( ) async def restore(self, namespace: str, limit: int = 10) -> RestoreResult: - del limit + _check_limit(limit) return RestoreResult( restored=0, skipped=0, @@ -397,6 +397,8 @@ async def list_namespaces( cursor: Optional[str] = None, limit: Optional[int] = None, ) -> NamespacesResult: + if limit is not None: + _check_limit(limit) grouped: Dict[str, List[_Memory]] = {} for memory in self._memories: grouped.setdefault(memory.namespace, []).append(memory) diff --git a/packages/python-sdk-memwal/tests/test_nonfinite_validation.py b/packages/python-sdk-memwal/tests/test_nonfinite_validation.py new file mode 100644 index 000000000..fddffadda --- /dev/null +++ b/packages/python-sdk-memwal/tests/test_nonfinite_validation.py @@ -0,0 +1,238 @@ +"""#1218: non-finite vectors and bad limits must fail client-side. + +json.dumps emits bare NaN/Infinity tokens, which are not JSON, so the +relayer rejected the body with an opaque parse error. Limits went out as +NaN, -5 or ?limit=nan. +""" + +from __future__ import annotations + +import math + +import httpx +import pytest + +from memwal import ( + MemWal, + MemWalError, + MemWalMock, + RecallManualOptions, + RecallParams, + RememberManualOptions, +) + + +class CaptureMemWal(MemWal): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.captured = [] + + async def _ensure_compatible_relayer(self): + return { + "apiVersion": "1.0.0", + "relayerVersion": "capture", + "minSupportedSdk": {"python": "0.0.0"}, + } + + async def _resolve_owner(self): + return "0xowner" + + async def _signed_request( + self, + method, + path, + body, + accepted_statuses=(200,), + include_seal_session=True, + timeout=None, + ): + self.captured.append((method, path, body)) + if path.startswith("/v1/owners"): + return {"namespaces": [], "has_more": False, "snapshot_version": 1} + if path == "/api/remember/manual": + return {"id": "1", "blob_id": "b", "owner": "0x1", "namespace": "default"} + if path == "/api/restore": + return {"restored": 0, "skipped": 0, "total": 0, "namespace": "ns", "owner": "0x1"} + return {"results": [], "total": 0} + + +class FreshOwnerMemWal(MemWal): + """Keeps the real ``_resolve_owner``, so owner lookup goes through signing. + + Every signed request is recorded and then fails, like a relayer outage: + a bad limit must surface as ValueError, not as this error. + """ + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.signed = [] + + async def _ensure_compatible_relayer(self): + return { + "apiVersion": "1.0.0", + "relayerVersion": "capture", + "minSupportedSdk": {"python": "0.0.0"}, + } + + async def _signed_request( + self, + method, + path, + body, + accepted_statuses=(200,), + include_seal_session=True, + timeout=None, + ): + self.signed.append((method, path)) + raise MemWalError("relayer unavailable") + + +def _client(): + return CaptureMemWal.create( + key="11" * 32, account_id="0x" + "ab" * 32, server_url="https://relayer.example" + ) + + +BAD_VECTORS = [ + [0.1, math.nan], + [math.inf], + [0.1, -math.inf], + [], + [0.1, "0.2"], + [0.1, None], + [True, 0.2], + "0.1,0.2", + None, +] +BAD_LIMITS = [math.nan, math.inf, -5, -1, 0, 2.5, "5", True] + + +@pytest.mark.parametrize("vector", BAD_VECTORS, ids=repr) +async def test_recall_manual_rejects_bad_vector(vector): + c = _client() + with pytest.raises(ValueError, match="vector"): + await c.recall_manual(RecallManualOptions(vector=vector)) + assert c.captured == [] + + +@pytest.mark.parametrize("vector", BAD_VECTORS, ids=repr) +async def test_remember_manual_rejects_bad_vector(vector): + c = _client() + with pytest.raises(ValueError, match="vector"): + await c.remember_manual(RememberManualOptions(encrypted_data="AAAA", vector=vector)) + assert c.captured == [] + + +@pytest.mark.parametrize("limit", BAD_LIMITS, ids=repr) +@pytest.mark.parametrize( + "call", + [ + ("recall", lambda c, n: c.recall("q", limit=n)), + ("recall_params", lambda c, n: c.recall(RecallParams(query="q", limit=n))), + ("recall_manual", lambda c, n: c.recall_manual(RecallManualOptions(vector=[0.1], limit=n))), + ("restore", lambda c, n: c.restore("ns", limit=n)), + ("list_namespaces", lambda c, n: c.list_namespaces(limit=n)), + ], + ids=lambda x: x[0], +) +async def test_bad_limit_is_rejected_before_signing(call, limit): + c = _client() + with pytest.raises(ValueError, match="limit"): + await call[1](c, limit) + assert c.captured == [] + + +async def test_valid_inputs_still_go_out_unchanged(): + c = _client() + await c.recall_manual(RecallManualOptions(vector=[0.1, -2, 0], limit=3)) + await c.remember_manual(RememberManualOptions(encrypted_data="AAAA", vector=[1, 0.5])) + await c.recall("q", limit=1) + await c.restore("ns", limit=50) + await c.list_namespaces(limit=500) + await c.list_namespaces() + bodies = [b for _, _, b in c.captured] + assert bodies[0]["vector"] == [0.1, -2, 0] and bodies[0]["limit"] == 3 + assert bodies[1]["vector"] == [1, 0.5] + assert bodies[2]["limit"] == 1 + assert bodies[3]["limit"] == 50 + assert c.captured[4][1].endswith("?limit=500") + assert "limit" not in c.captured[5][1] + + +async def test_signed_request_never_emits_nan_tokens(monkeypatch): + """Backstop: even if a NaN reaches the signer, it must not be serialised.""" + client = MemWal.create( + key="11" * 32, account_id="0x" + "ab" * 32, server_url="https://relayer.example" + ) + + async def compatible(): + return { + "apiVersion": "1.0.0", + "relayerVersion": "t", + "minSupportedSdk": {"python": "0.0.0"}, + } + + sent = [] + + async def fake_post(*args, **kwargs): # pragma: no cover - must not be reached + sent.append(kwargs) + raise AssertionError("request sent with a non-JSON body") + + monkeypatch.setattr(client, "_ensure_compatible_relayer", compatible) + monkeypatch.setattr(httpx.AsyncClient, "send", fake_post) + with pytest.raises(ValueError): + await client._signed_request( + "POST", "/api/recall", {"limit": math.nan}, include_seal_session=False + ) + assert sent == [] + await client.close() + + +@pytest.mark.parametrize("limit", BAD_LIMITS, ids=repr) +async def test_list_namespaces_bad_limit_raises_before_owner_lookup(limit): + """The owner lookup is a signed POST /api/stats; a bad limit must not reach it.""" + c = FreshOwnerMemWal.create( + key="11" * 32, account_id="0x" + "ab" * 32, server_url="https://relayer.example" + ) + with pytest.raises(ValueError, match="limit"): + await c.list_namespaces(limit=limit) + assert c.signed == [] + await c.close() + + +async def test_list_namespaces_valid_limit_still_resolves_owner(): + c = FreshOwnerMemWal.create( + key="11" * 32, account_id="0x" + "ab" * 32, server_url="https://relayer.example" + ) + with pytest.raises(MemWalError, match="relayer unavailable"): + await c.list_namespaces(limit=5) + assert c.signed == [("POST", "/api/stats")] + await c.close() + + +@pytest.mark.parametrize("limit", BAD_LIMITS, ids=repr) +@pytest.mark.parametrize( + "call", + [ + ("recall", lambda m, n: m.recall("q", limit=n)), + ("recall_params", lambda m, n: m.recall(RecallParams(query="q", limit=n))), + ("ask", lambda m, n: m.ask("q", limit=n)), + ("restore", lambda m, n: m.restore("ns", limit=n)), + ("list_namespaces", lambda m, n: m.list_namespaces(limit=n)), + ], + ids=lambda x: x[0], +) +async def test_mock_rejects_the_same_bad_limits_as_the_client(call, limit): + mock = MemWalMock.create() + await mock.remember("a fact") + with pytest.raises(ValueError, match="limit"): + await call[1](mock, limit) + + +async def test_mock_accepts_positive_integer_limits(): + mock = MemWalMock.create() + await mock.remember("a fact") + assert len((await mock.recall("fact", limit=1)).results) == 1 + assert (await mock.restore("ns", limit=1)).restored == 0 + assert len((await mock.list_namespaces(limit=1)).namespaces) == 1 + assert len((await mock.list_namespaces()).namespaces) == 1