From 91da5ca28ec3b92e8cf6cd5407018a5dc8d327c1 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 20 Jul 2026 12:57:01 -0700 Subject: [PATCH] fix(tools): rename provider-illegal property keys in tool schemas, reverse-map at dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cloudflare's flat API MCP ships 61 property keys that violate Anthropic's ^[a-zA-Z0-9_.-]{1,64}$ pattern (query-filter params like 'issue_class~neq' and 'meta.[]'). One bad key anywhere in the tools array 400s the ENTIRE request — measured live: Anthropic, Bedrock, Google Vertex, and Azure all rejected an eager 3,320-tool Cloudflare request at validation, before token limits even applied. - schema_sanitizer: rename non-conforming property keys deterministically (bad chars -> '_', 64-char truncation, collision dedup with numeric suffixes), nested schemas included; required[] remapped alongside - unrename_tool_args(): reverse map applied in coerce_tool_args at dispatch, so the MCP server receives the original wire names; recurses into object values and array items - deterministic on both sides: the rename map is recomputed from the registry's original schema at dispatch time, no state carried E2E on the real capture: 61 -> 0 violations across 3,320 tools; round-trip verified on get_accounts_intel_attacksurfacereport_issues (5 '~neq' keys). --- model_tools.py | 10 +++ tests/tools/test_schema_sanitizer.py | 108 +++++++++++++++++++++++++++ tools/schema_sanitizer.py | 100 +++++++++++++++++++++++-- 3 files changed, 212 insertions(+), 6 deletions(-) diff --git a/model_tools.py b/model_tools.py index 2ac445f8ff5b..60fb8fd82f3b 100644 --- a/model_tools.py +++ b/model_tools.py @@ -715,6 +715,16 @@ def coerce_tool_args(tool_name: str, args: Dict[str, Any]) -> Dict[str, Any]: if not properties: return args + # The model saw the SANITIZED schema — property keys violating provider + # patterns (e.g. Cloudflare's ``issue_class~neq``) were renamed before + # the request. Map any sanitized keys back to the registry's original + # wire names before schema lookup / dispatch. + try: + from tools.schema_sanitizer import unrename_tool_args + args = unrename_tool_args(schema.get("parameters"), args) + except Exception: # pragma: no cover — never break dispatch + pass + for key, value in list(args.items()): prop_schema = properties.get(key) if not prop_schema: diff --git a/tests/tools/test_schema_sanitizer.py b/tests/tools/test_schema_sanitizer.py index 90a6ffa3c048..da6a1e30ad0f 100644 --- a/tests/tools/test_schema_sanitizer.py +++ b/tests/tools/test_schema_sanitizer.py @@ -759,3 +759,111 @@ def test_strip_slash_enum_ignores_non_string_enum_values(): props = tools[0]["function"]["parameters"]["properties"] assert props["level"]["enum"] == [1, 2, 3] assert props["flag"]["enum"] == [True, False] + + +# --------------------------------------------------------------------------- +# Property-key renaming (provider ^[a-zA-Z0-9_.-]{1,64}$ pattern compat) +# Real-world source: Cloudflare flat API MCP ships keys like +# ``issue_class~neq`` and ``meta.[]`` — one bad key anywhere +# in the tools array 400s the whole request on Anthropic/Bedrock/Vertex/Azure. +# --------------------------------------------------------------------------- + +from tools.schema_sanitizer import sanitize_property_key, unrename_tool_args + + +def test_bad_property_keys_renamed_to_conforming(): + tools = [_tool("cf_issues", { + "type": "object", + "properties": { + "issue_class~neq": {"type": "string"}, + "meta.[]": {"type": "string"}, + "normal_key": {"type": "string"}, + }, + "required": ["issue_class~neq"], + })] + out = sanitize_tool_schemas(tools) + props = out[0]["function"]["parameters"]["properties"] + import re + pat = re.compile(r"^[a-zA-Z0-9_.-]{1,64}$") + assert all(pat.match(k) for k in props), list(props) + assert "normal_key" in props + assert "issue_class_neq" in props + # required remapped alongside + assert out[0]["function"]["parameters"]["required"] == ["issue_class_neq"] + + +def test_property_key_over_64_chars_truncated(): + long_key = "k" * 80 + tools = [_tool("t", {"type": "object", "properties": {long_key: {"type": "string"}}})] + out = sanitize_tool_schemas(tools) + props = out[0]["function"]["parameters"]["properties"] + assert list(props) == ["k" * 64] + + +def test_rename_collision_deduped_deterministically(): + tools = [_tool("t", { + "type": "object", + "properties": { + "a~b": {"type": "string"}, + "a b": {"type": "integer"}, + "a_b": {"type": "boolean"}, # already owns the sanitized name + }, + })] + out = sanitize_tool_schemas(tools) + props = out[0]["function"]["parameters"]["properties"] + assert props["a_b"]["type"] == "boolean" # original conforming key untouched + assert set(props) == {"a_b", "a_b_2", "a_b_3"} + # deterministic across repeated runs + out2 = sanitize_tool_schemas(tools) + assert out2[0]["function"]["parameters"]["properties"].keys() == props.keys() + + +def test_nested_bad_property_keys_renamed(): + tools = [_tool("t", { + "type": "object", + "properties": { + "body": { + "type": "object", + "properties": {"filter~gte": {"type": "number"}}, + }, + }, + })] + out = sanitize_tool_schemas(tools) + body = out[0]["function"]["parameters"]["properties"]["body"] + assert "filter_gte" in body["properties"] + assert "filter~gte" not in body["properties"] + + +def test_unrename_tool_args_maps_back_to_wire_names(): + original_params = { + "type": "object", + "properties": { + "issue_class~neq": {"type": "string"}, + "normal": {"type": "string"}, + "body": { + "type": "object", + "properties": {"filter~gte": {"type": "number"}}, + }, + }, + } + model_args = { + "issue_class_neq": "spoofed_dns", + "normal": "x", + "body": {"filter_gte": 5}, + } + restored = unrename_tool_args(original_params, model_args) + assert restored == { + "issue_class~neq": "spoofed_dns", + "normal": "x", + "body": {"filter~gte": 5}, + } + + +def test_unrename_passes_unknown_keys_through(): + params = {"type": "object", "properties": {"a": {"type": "string"}}} + assert unrename_tool_args(params, {"a": 1, "mystery": 2}) == {"a": 1, "mystery": 2} + + +def test_sanitize_property_key_empty_falls_back(): + assert sanitize_property_key("~~~") == "___" + assert sanitize_property_key("") == "param" diff --git a/tools/schema_sanitizer.py b/tools/schema_sanitizer.py index 9f0bc98fcd2c..7dbcb1ef87fa 100644 --- a/tools/schema_sanitizer.py +++ b/tools/schema_sanitizer.py @@ -38,11 +38,85 @@ from __future__ import annotations import copy import logging +import re from typing import Any logger = logging.getLogger(__name__) +# Anthropic (and Bedrock/Vertex/Azure fronting it) reject tool input schemas +# whose property keys don't match this pattern. Cloudflare's flat API MCP +# ships 61 such keys (query-filter params like ``issue_class~neq`` and +# ``meta.[]``) — one bad key anywhere in the tools array +# 400s the entire request. +_PROP_KEY_RE = re.compile(r"^[a-zA-Z0-9_.-]{1,64}$") +_PROP_KEY_BAD_CHARS = re.compile(r"[^a-zA-Z0-9_.-]") + + +def sanitize_property_key(key: str) -> str: + """Deterministically map an arbitrary property key to a conforming one.""" + new = _PROP_KEY_BAD_CHARS.sub("_", key)[:64] + return new or "param" + + +def _rename_property_keys(props: dict, path: str) -> dict[str, str]: + """Return {original_key: conforming_key} for one properties dict. + + Identity entries are omitted. Deterministic: keys are processed in + insertion order and collisions deduped with numeric suffixes, so the + model-visible schema AND the dispatch-time reverse map (computed + independently from the registry's original schema) always agree. + """ + renames: dict[str, str] = {} + taken = {k for k in props if _PROP_KEY_RE.match(k)} + for key in props: + if _PROP_KEY_RE.match(key): + continue + base = sanitize_property_key(key) + candidate, i = base, 2 + while candidate in taken: + suffix = f"_{i}" + candidate = base[: 64 - len(suffix)] + suffix + i += 1 + taken.add(candidate) + renames[key] = candidate + logger.debug( + "schema_sanitizer[%s]: renamed property key %r -> %r " + "(provider key-pattern compat)", path, key, candidate, + ) + return renames + + +def unrename_tool_args(params_schema: Any, args: Any) -> Any: + """Map sanitized property keys in model-emitted args back to wire names. + + ``params_schema`` is the ORIGINAL (unsanitized) parameters schema from the + registry. Recurses into object-typed values and array items so nested + renamed keys are restored too. Unknown keys pass through untouched. + """ + if not isinstance(params_schema, dict) or not isinstance(args, dict): + return args + props = params_schema.get("properties") + if not isinstance(props, dict): + return args + reverse = {v: k for k, v in _rename_property_keys(props, "").items()} + out = {} + for key, value in args.items(): + orig = reverse.get(key, key) + subschema = props.get(orig) + if isinstance(subschema, dict): + if isinstance(value, dict): + value = unrename_tool_args(subschema, value) + elif isinstance(value, list) and isinstance(subschema.get("items"), dict): + value = [ + unrename_tool_args(subschema["items"], item) + if isinstance(item, dict) else item + for item in value + ] + out[orig] = value + return out + + def sanitize_tool_schemas(tools: list[dict]) -> list[dict]: """Return a copy of ``tools`` with each tool's parameter schema sanitized. @@ -268,6 +342,13 @@ def _sanitize_node(node: Any, path: str) -> Any: if not isinstance(node, dict): return node + # Compute property-key renames up front so the ``required`` branch below + # can remap regardless of dict iteration order (``required`` may precede + # ``properties`` in the source dict). + prop_renames: dict[str, str] = {} + if isinstance(node.get("properties"), dict): + prop_renames = _rename_property_keys(node["properties"], f"{path}.properties") + out: dict = {} for key, value in node.items(): # JSON Schema ``type`` arrays (e.g. ``["number", "string"]``, common @@ -306,10 +387,12 @@ def _sanitize_node(node: Any, path: str) -> Any: continue if key in {"properties", "$defs", "definitions"} and isinstance(value, dict): - out[key] = { - sub_k: _sanitize_node(sub_v, f"{path}.{key}.{sub_k}") - for sub_k, sub_v in value.items() - } + renames = prop_renames if key == "properties" else {} + new_props = {} + for sub_k, sub_v in value.items(): + out_k = renames.get(sub_k, sub_k) + new_props[out_k] = _sanitize_node(sub_v, f"{path}.{key}.{out_k}") + out[key] = new_props elif key in {"items", "additionalProperties"}: if isinstance(value, bool): # Keep bool ``additionalProperties`` as-is — it's a valid form @@ -330,8 +413,13 @@ def _sanitize_node(node: Any, path: str) -> Any: # - ``examples``: list of example values (any JSON type) # Recursing into these with _sanitize_node() would mis-interpret # literal strings like "path" as bare-string schemas and replace - # them with {"type": "object"} dicts. Pass through unchanged. - out[key] = copy.deepcopy(value) if isinstance(value, (list, dict)) else value + # them with {"type": "object"} dicts. Pass through unchanged + # (remapping ``required`` entries through the property renames). + if key == "required" and prop_renames and isinstance(value, list): + out[key] = [prop_renames.get(r, r) if isinstance(r, str) else r + for r in value] + else: + out[key] = copy.deepcopy(value) if isinstance(value, (list, dict)) else value else: out[key] = _sanitize_node(value, f"{path}.{key}") if isinstance(value, (dict, list)) else value