Skip to content

Commit f0290fe

Browse files
phernandezclaude
andcommitted
fix(mcp): reject malformed JSON-array tag strings in search_notes
strict_search_tags guarded native lists with non-string elements but still delegated JSON-array strings directly to parse_tags, whose recursive JSON handling stringified bad elements ('[42]' -> ["42"]) before Pydantic could validate List[str] — preserving the silent no-result behavior for MCP clients that serialize arrays as strings. Mirror parse_tags' JSON-array detection at the boundary: if the parsed list contains any non-string element, pass the original string through unchanged so Pydantic rejects it with a clear validation error. Valid all-string JSON arrays and plain comma strings keep working as before. parse_tags itself is unchanged (frontmatter callers rely on its lenient behavior). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 72350e5 commit f0290fe

3 files changed

Lines changed: 82 additions & 1 deletion

File tree

src/basic_memory/utils.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,9 +576,25 @@ def strict_search_tags(v: Any) -> Any:
576576
str, all-string lists, and None are valid tag inputs; everything else — including
577577
lists with non-string elements like [42] — passes through unchanged so Pydantic
578578
rejects it with a clear validation error.
579+
580+
JSON array strings (the MCP clients-serialize-arrays-as-strings path) get the same
581+
all-string check: '[42]' or '["ok", 42]' would otherwise be stringified by
582+
parse_tags' recursive JSON handling before Pydantic ever sees the bad elements.
579583
"""
580584
if isinstance(v, list) and not all(isinstance(item, str) for item in v):
581585
return v
586+
# Trigger: a str that looks like a JSON array, mirroring parse_tags' detection.
587+
# Why: parse_tags recursively parses JSON arrays, stringifying non-string elements
588+
# ('[42]' -> ["42"]) and hiding the type error from Pydantic.
589+
# Outcome: malformed arrays pass through unchanged so Pydantic rejects them; valid
590+
# all-string arrays and plain comma strings still delegate to parse_tags.
591+
if isinstance(v, str) and v.strip().startswith("[") and v.strip().endswith("]"):
592+
try:
593+
parsed = json.loads(v)
594+
except json.JSONDecodeError:
595+
parsed = None
596+
if isinstance(parsed, list) and not all(isinstance(item, str) for item in parsed):
597+
return v
582598
if v is None or isinstance(v, (str, list)):
583599
return parse_tags(v)
584600
return v

tests/mcp/test_tool_search.py

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1719,8 +1719,18 @@ def test_search_notes_tags_annotation_rejects_non_string_types():
17191719
with pytest.raises(ValidationError):
17201720
adapter.validate_python(["ok", 42])
17211721

1722-
# All-string lists remain valid.
1722+
# JSON-array strings with non-string elements must fail the same way — parse_tags
1723+
# would otherwise recursively stringify them before Pydantic validates List[str].
1724+
with pytest.raises(ValidationError):
1725+
adapter.validate_python("[42]")
1726+
with pytest.raises(ValidationError):
1727+
adapter.validate_python('[{"a": 1}]')
1728+
with pytest.raises(ValidationError):
1729+
adapter.validate_python('["ok", 42]')
1730+
1731+
# All-string lists and all-string JSON-array strings remain valid.
17231732
assert adapter.validate_python(["a", "b"]) == ["a", "b"]
1733+
assert adapter.validate_python('["a","b"]') == ["a", "b"]
17241734

17251735
# None stays a valid "no filter" input.
17261736
assert adapter.validate_python(None) in (None, [])
@@ -1779,6 +1789,44 @@ async def test_search_notes_tags_invalid_type_rejected_via_mcp(mcp, client, test
17791789
"tags": ["ok", 42],
17801790
},
17811791
)
1792+
# JSON-array strings with non-string elements (clients that serialize arrays as
1793+
# strings) must be rejected too, not recursively stringified by parse_tags.
1794+
with pytest.raises(ToolError):
1795+
await mcp_client.call_tool(
1796+
"search_notes",
1797+
{
1798+
"project": test_project.name,
1799+
"query": "anything",
1800+
"tags": "[42]",
1801+
},
1802+
)
1803+
with pytest.raises(ToolError):
1804+
await mcp_client.call_tool(
1805+
"search_notes",
1806+
{
1807+
"project": test_project.name,
1808+
"query": "anything",
1809+
"tags": '[{"a": 1}]',
1810+
},
1811+
)
1812+
with pytest.raises(ToolError):
1813+
await mcp_client.call_tool(
1814+
"search_notes",
1815+
{
1816+
"project": test_project.name,
1817+
"query": "anything",
1818+
"tags": '["ok", 42]',
1819+
},
1820+
)
1821+
# Sanity: a valid all-string JSON-array string is still accepted.
1822+
await mcp_client.call_tool(
1823+
"search_notes",
1824+
{
1825+
"project": test_project.name,
1826+
"query": "anything",
1827+
"tags": '["a","b"]',
1828+
},
1829+
)
17821830

17831831

17841832
@pytest.mark.asyncio

tests/test_coerce.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,23 @@ def test_mixed_list_passthrough_for_pydantic_rejection(self):
7373
value = ["ok", 42]
7474
assert strict_search_tags(value) is value
7575

76+
def test_json_array_string_with_int_passthrough_for_pydantic_rejection(self):
77+
"""A JSON-array string with non-string elements must not be stringified."""
78+
value = "[42]"
79+
assert strict_search_tags(value) is value
80+
81+
def test_json_array_string_with_dict_passthrough_for_pydantic_rejection(self):
82+
value = '[{"a": 1}]'
83+
assert strict_search_tags(value) is value
84+
85+
def test_json_array_string_mixed_passthrough_for_pydantic_rejection(self):
86+
"""One bad element poisons the whole JSON-array string — no partial parse."""
87+
value = '["ok", 42]'
88+
assert strict_search_tags(value) is value
89+
90+
def test_json_array_string_all_strings_still_parses(self):
91+
assert strict_search_tags('["a","b"]') == ["a", "b"]
92+
7693

7794
class TestCoerceDict:
7895
"""Tests for coerce_dict."""

0 commit comments

Comments
 (0)