Merge branch 'dev-v2.0.36' into bugfix/autodev-2502-20261015135804529

Author: endxxxx

src/memos/api/middleware/auth.py

@@ -17,6 +17,13 @@
 import memos.log
 
 
+# Canonical format check; re-exported here so the middleware shares the
+# same logic as ``src/memos/api/utils/api_keys.py``. Keeping one source of
+# truth is what fixes issue #2427 (previously the middleware copy and the
+# utils copy could drift independently).
+from memos.api.utils.api_keys import validate_key_format as validate_key_format_canonical
+
+
 logger = memos.log.get_logger(__name__)
 
 # API key header configuration
@@ -63,17 +70,16 @@ def hash_api_key(key: str) -> str:
 
 
 def validate_key_format(key: str) -> bool:
-    """Validate API key format: krlk_<64-hex>."""
-    if not key or not key.startswith("krlk_"):
-        return False
-    hex_part = key[5:]  # Remove 'krlk_' prefix
-    if len(hex_part) != 64:
-        return False
-    try:
-        int(hex_part, 16)
-        return True
-    except ValueError:
-        return False
+    """Validate API key format: krlk_<64-hex>.
+
+    Re-exported from ``memos.api.utils.api_keys.validate_key_format`` for
+    backwards compatibility with callers that imported this symbol from
+    the middleware path. Issue #2427: the previous copy used ``int(...)``
+    which accepted non-canonical hex (leading signs, ``0x`` prefix, ``_``
+    separators, whitespace) — see the canonical implementation for the
+    fix.
+    """
+    return validate_key_format_canonical(key)
 
 
 def get_key_prefix(key: str) -> str:

src/memos/api/middleware/rate_limit.py

@@ -57,8 +57,13 @@ def _get_client_key(request: Request) -> str:
 
     Uses API key if available, otherwise falls back to IP.
     """
-    # Try to get API key from header
+    # Try to get API key from header. Real clients send a Bearer scheme
+    # per RFC 6750 (``Authorization: Bearer krlk_<hex>``); we strip the
+    # scheme before the prefix check so keyed requests actually land on
+    # a per-key bucket (issue #2428).
     auth_header = request.headers.get("Authorization", "")
+    if auth_header.lower().startswith("bearer "):
+        auth_header = auth_header[len("bearer ") :].lstrip()
     if auth_header.startswith("krlk_"):
         # Use first 20 chars of key as identifier
         return f"ratelimit:key:{auth_header[:20]}"

src/memos/api/utils/api_keys.py

@@ -5,6 +5,7 @@
 """
 
 import hashlib
+import re
 import secrets
 
 from dataclasses import dataclass
@@ -45,11 +46,19 @@ def hash_key(key: str) -> str:
     return hashlib.sha256(key.encode()).hexdigest()
 
 
+# Match the only form ``generate_api_key()`` produces: ``krlk_`` followed by
+# exactly 64 lowercase hex characters. Using ``int(hex_part, 16)`` would also
+# accept leading signs, ``0x`` prefixes, ``_`` separators, and surrounding
+# whitespace, none of which a real key ever carries (issue #2427).
+_KEY_FORMAT_HEX_RE = re.compile(r"[0-9a-f]{64}")
+
+
 def validate_key_format(key: str) -> bool:
     """
     Validate API key format.
 
-    Valid format: krlk_<64-hex-chars>
+    Valid format: ``krlk_<64-hex-chars>`` where the hex part is exactly 64
+    characters from ``[0-9a-f]`` — the only form ``generate_api_key()`` produces.
     """
     if not key or not isinstance(key, str):
         return False
@@ -58,14 +67,7 @@ def validate_key_format(key: str) -> bool:
         return False
 
     hex_part = key[5:]
-    if len(hex_part) != 64:
-        return False
-
-    try:
-        int(hex_part, 16)
-        return True
-    except ValueError:
-        return False
+    return _KEY_FORMAT_HEX_RE.fullmatch(hex_part) is not None
 
 
 def generate_master_key() -> tuple[str, str]:

src/memos/llms/openai_new.py

@@ -50,7 +50,7 @@ def generate(self, messages: MessageList, **kwargs) -> str:
         output_reasoning = [
             item for item in response.output if isinstance(item, ResponseReasoningItem)
         ]
-        summary = output_reasoning[0].summary
+        summary = output_reasoning[0].summary if output_reasoning else None
 
         if self.config.remove_think_prefix:
             return remove_thinking_tags(output_text)
@@ -138,7 +138,7 @@ def generate(self, messages: MessageList, **kwargs) -> str:
         output_reasoning = [
             item for item in response.output if isinstance(item, ResponseReasoningItem)
         ]
-        summary = output_reasoning[0].summary
+        summary = output_reasoning[0].summary if output_reasoning else None
 
         if self.config.remove_think_prefix:
             return remove_thinking_tags(output_text)

src/memos/memories/textual/item.py

@@ -159,6 +159,16 @@ class TextualMemoryMetadata(BaseModel):
         description="Internal algorithm metadata reserved for system use.",
     )
 
+    @field_validator("internal_info", mode="before")
+    @classmethod
+    def _decode_internal_info(cls, value: Any) -> Any:
+        # Neo4j stores dictionaries as JSON strings; decode them before model validation.
+        if isinstance(value, str):
+            value = json.loads(value)
+            if not isinstance(value, dict):
+                raise ValueError("internal_info JSON must decode to an object")
+        return value
+
     model_config = ConfigDict(extra="allow")
 
     covered_history: Any | None = Field(

tests/api/test_rate_limit_bearer.py

@@ -0,0 +1,78 @@
+"""Regression tests for issue #2428.
+
+``_get_client_key`` in ``src/memos/api/middleware/rate_limit.py`` checked
+whether ``request.headers['Authorization']`` started with ``krlk_`` — but
+real clients send ``Authorization: Bearer krlk_<hex>`` per RFC 6750, so
+the prefix check never matched and every request fell through to IP-based
+limiting. Behind a shared egress IP all keyed clients shared one bucket
+and the per-key rate-limit was dead code.
+
+The fix strips the ``Bearer `` scheme before testing the key prefix, so
+``Bearer krlk_<hex>`` correctly maps to ``ratelimit:key:<first 20 of the
+stripped key>``.
+"""
+
+from __future__ import annotations
+
+from types import SimpleNamespace
+
+from memos.api.middleware.rate_limit import _get_client_key
+
+
+def _fake_request(authorization: str | None, *, client_host: str = "10.0.0.1"):
+    headers: dict[str, str] = {}
+    if authorization is not None:
+        headers["Authorization"] = authorization
+    return SimpleNamespace(
+        headers=headers,
+        client=SimpleNamespace(host=client_host),
+    )
+
+
+class TestGetClientKey:
+    def test_bearer_krlk_key_uses_per_key_bucket(self):
+        body = "krlk_" + "a" * 64
+        request = _fake_request("Bearer " + body)
+        key = _get_client_key(request)
+        assert key == "ratelimit:key:" + body[:20]
+        assert "10.0.0.1" not in key
+
+    def test_bearer_krlk_key_first_twenty_chars_of_actual_key(self):
+        body = "krlk_" + "f" * 64
+        request = _fake_request("Bearer " + body)
+        key = _get_client_key(request)
+        assert key == "ratelimit:key:" + body[:20]
+
+    def test_lowercase_bearer_scheme(self):
+        # RFC 7235: the scheme name is case-insensitive. We normalise to
+        # lowercase before the prefix check.
+        body = "krlk_" + "b" * 64
+        request = _fake_request("bearer " + body)
+        key = _get_client_key(request)
+        assert key == "ratelimit:key:" + body[:20]
+
+    def test_bearer_with_non_key_token_falls_back_to_ip(self):
+        request = _fake_request("Bearer some-jwt-token")
+        key = _get_client_key(request)
+        assert key.startswith("ratelimit:ip:")
+        assert "10.0.0.1" in key
+
+    def test_missing_authorization_header_falls_back_to_ip(self):
+        request = _fake_request(None)
+        key = _get_client_key(request)
+        assert key.startswith("ratelimit:ip:")
+        assert "10.0.0.1" in key
+
+    def test_x_forwarded_for_header_preferred_over_ip(self):
+        request = _fake_request(None, client_host="203.0.113.1")
+        request.headers["X-Forwarded-For"] = "198.51.100.1, 10.0.0.1"
+        key = _get_client_key(request)
+        assert key == "ratelimit:ip:198.51.100.1"
+
+    def test_krlk_prefix_without_bearer_scheme_still_matches(self):
+        # Backwards compatibility: clients that sent a bare krlk_ header
+        # (before the Bearer scheme was added) must still match.
+        body = "krlk_" + "c" * 64
+        request = _fake_request(body)
+        key = _get_client_key(request)
+        assert key == "ratelimit:key:" + body[:20]

tests/api/test_validate_key_format.py

@@ -0,0 +1,99 @@
+"""Regression tests for issue #2427.
+
+``validate_key_format`` exists in two copies — ``src/memos/api/utils/api_keys.py``
+(the canonical home) and ``src/memos/api/middleware/auth.py`` (the
+middleware gating every authenticated request). Both check the hex part
+with ``int(hex_part, 16)``, which accepts non-canonical forms a generated
+key can never have:
+
+- leading ``+`` / ``-`` (sign)
+- ``0x`` / ``0X`` prefix
+- ``_`` separators (PEP 515)
+- surrounding whitespace
+
+So the documented format ``krlk_<64-hex-chars>`` is not actually enforced:
+keys with embedded underscores, signs, or ``0x`` prefixes pass the format
+gate and are hashed/looked up as-is, bypassing the canonical form that
+``generate_api_key()`` produces.
+
+These tests pin the new contract on both validator copies:
+
+- the hex part must match ``[0-9a-f]{64}`` exactly
+- the only valid form is the canonical lowercase hex string
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from memos.api.middleware.auth import validate_key_format as validate_in_auth
+from memos.api.utils.api_keys import generate_api_key, validate_key_format
+
+
+CANONICAL = "krlk_" + "0123456789abcdef" * 4  # 64 hex chars, lowercase
+
+
+class TestValidateKeyFormatUtils:
+    def test_canonical_key_passes(self):
+        assert validate_key_format(CANONICAL) is True
+
+    def test_generated_key_always_passes(self):
+        # The validator must accept every key that generate_api_key()
+        # produces, since that is the documented format.
+        for _ in range(20):
+            assert validate_key_format(generate_api_key().key) is True
+
+    @pytest.mark.parametrize(
+        "bad_key",
+        [
+            # Leading sign
+            "krlk_" + "+" + "a" * 63,
+            "krlk_" + "-" + "a" * 63,
+            # 0x prefix
+            "krlk_" + "0x" + "a" * 62,
+            "krlk_" + "0X" + "a" * 62,
+            # Underscore separator (PEP 515)
+            "krlk_" + "a" * 32 + "_" + "b" * 31,
+            # Uppercase hex (the generator emits lowercase only)
+            "krlk_" + "A" * 64,
+            # Surrounding whitespace
+            " krlk_" + "a" * 64,
+            "krlk_" + "a" * 64 + " ",
+            # Wrong total length
+            "krlk_" + "a" * 63,
+            "krlk_" + "a" * 65,
+            # Empty hex part
+            "krlk_",
+            # Missing prefix
+            "0123456789abcdef" * 4,
+            "Bearer " + CANONICAL,
+            # Non-string
+            "",
+        ],
+    )
+    def test_non_canonical_keys_rejected(self, bad_key):
+        assert validate_key_format(bad_key) is False, (
+            f"validate_key_format({bad_key!r}) should return False"
+        )
+
+
+class TestAuthMiddlewareValidator:
+    """The middleware copy must enforce the same contract as the canonical
+    one — issue #2427 documents both copies accepting non-canonical hex."""
+
+    def test_canonical_key_passes(self):
+        assert validate_in_auth(CANONICAL) is True
+
+    def test_signed_hex_rejected(self):
+        # The most surprising bit: ``int("+a..", 16)`` succeeds, so a signed
+        # hex passes the middleware's validator today.
+        assert validate_in_auth("krlk_" + "+" + "a" * 63) is False
+
+    def test_underscore_rejected(self):
+        assert validate_in_auth("krlk_" + "a" * 32 + "_" + "b" * 31) is False
+
+    def test_uppercase_rejected(self):
+        assert validate_in_auth("krlk_" + "A" * 64) is False
+
+    def test_0x_prefix_rejected(self):
+        assert validate_in_auth("krlk_" + "0x" + "a" * 62) is False