fix(tools): reject unknown builtin parameters
This commit is contained in:
@@ -84,9 +84,16 @@ class Schema(ABC):
|
|||||||
for k in schema.get("required", []):
|
for k in schema.get("required", []):
|
||||||
if k not in val:
|
if k not in val:
|
||||||
errors.append(f"missing required {Schema.subpath(path, k)}")
|
errors.append(f"missing required {Schema.subpath(path, k)}")
|
||||||
|
additional = schema.get("additionalProperties", True)
|
||||||
for k, v in val.items():
|
for k, v in val.items():
|
||||||
if k in props:
|
if k in props:
|
||||||
errors.extend(Schema.validate_json_schema_value(v, props[k], Schema.subpath(path, k)))
|
errors.extend(Schema.validate_json_schema_value(v, props[k], Schema.subpath(path, k)))
|
||||||
|
elif additional is False:
|
||||||
|
errors.append(f"unexpected parameter {Schema.subpath(path, k)}")
|
||||||
|
elif isinstance(additional, dict):
|
||||||
|
errors.extend(
|
||||||
|
Schema.validate_json_schema_value(v, additional, Schema.subpath(path, k))
|
||||||
|
)
|
||||||
if t == "array":
|
if t == "array":
|
||||||
if "minItems" in schema and len(val) < schema["minItems"]:
|
if "minItems" in schema and len(val) < schema["minItems"]:
|
||||||
errors.append(f"{label} must have at least {schema['minItems']} items")
|
errors.append(f"{label} must have at least {schema['minItems']} items")
|
||||||
@@ -193,7 +200,16 @@ class Tool(ABC):
|
|||||||
if not isinstance(obj, dict):
|
if not isinstance(obj, dict):
|
||||||
return obj
|
return obj
|
||||||
props = schema.get("properties", {})
|
props = schema.get("properties", {})
|
||||||
return {k: self._cast_value(v, props[k]) if k in props else v for k, v in obj.items()}
|
additional = schema.get("additionalProperties")
|
||||||
|
casted: dict[str, Any] = {}
|
||||||
|
for k, v in obj.items():
|
||||||
|
if k in props:
|
||||||
|
casted[k] = self._cast_value(v, props[k])
|
||||||
|
elif isinstance(additional, dict):
|
||||||
|
casted[k] = self._cast_value(v, additional)
|
||||||
|
else:
|
||||||
|
casted[k] = v
|
||||||
|
return casted
|
||||||
|
|
||||||
def cast_params(self, params: dict[str, Any]) -> dict[str, Any]:
|
def cast_params(self, params: dict[str, Any]) -> dict[str, Any]:
|
||||||
"""Apply safe schema-driven casts before validation."""
|
"""Apply safe schema-driven casts before validation."""
|
||||||
|
|||||||
@@ -222,11 +222,18 @@ def tool_parameters_schema(
|
|||||||
*,
|
*,
|
||||||
required: list[str] | None = None,
|
required: list[str] | None = None,
|
||||||
description: str = "",
|
description: str = "",
|
||||||
|
additional_properties: bool | dict[str, Any] | None = False,
|
||||||
**properties: Any,
|
**properties: Any,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Build root tool parameters ``{"type": "object", "properties": ...}`` for :meth:`Tool.parameters`."""
|
"""Build root tool parameters ``{"type": "object", "properties": ...}`` for :meth:`Tool.parameters`.
|
||||||
|
|
||||||
|
Built-in tools default to strict parameter objects so misspelled tool-call
|
||||||
|
arguments are reported before execution instead of being silently ignored.
|
||||||
|
Pass ``additional_properties=None`` to omit the JSON Schema keyword.
|
||||||
|
"""
|
||||||
return ObjectSchema(
|
return ObjectSchema(
|
||||||
required=required,
|
required=required,
|
||||||
description=description,
|
description=description,
|
||||||
|
additional_properties=additional_properties,
|
||||||
**properties,
|
**properties,
|
||||||
).to_json_schema()
|
).to_json_schema()
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ from __future__ import annotations
|
|||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from nanobot.agent.tools.base import Tool
|
from nanobot.agent.tools.base import Tool
|
||||||
|
from nanobot.agent.tools.filesystem import ReadFileTool
|
||||||
from nanobot.agent.tools.registry import ToolRegistry
|
from nanobot.agent.tools.registry import ToolRegistry
|
||||||
|
|
||||||
|
|
||||||
@@ -235,6 +236,27 @@ def test_prepare_call_other_tools_keep_generic_object_validation() -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_registry_rejects_unknown_builtin_tool_parameters(tmp_path) -> None:
|
||||||
|
(tmp_path / "sample.txt").write_text("one\ntwo\nthree\n", encoding="utf-8")
|
||||||
|
registry = ToolRegistry()
|
||||||
|
registry.register(
|
||||||
|
ReadFileTool(
|
||||||
|
workspace=tmp_path,
|
||||||
|
allowed_dir=tmp_path,
|
||||||
|
restrict_to_workspace=True,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
result = await registry.execute(
|
||||||
|
"read_file",
|
||||||
|
{"path": "sample.txt", "line_limit": 1},
|
||||||
|
)
|
||||||
|
|
||||||
|
assert "Invalid parameters" in result
|
||||||
|
assert "unexpected parameter line_limit" in result
|
||||||
|
assert "one" not in result
|
||||||
|
|
||||||
|
|
||||||
def test_get_definitions_returns_cached_result() -> None:
|
def test_get_definitions_returns_cached_result() -> None:
|
||||||
registry = ToolRegistry()
|
registry = ToolRegistry()
|
||||||
registry.register(_FakeTool("read_file"))
|
registry.register(_FakeTool("read_file"))
|
||||||
|
|||||||
@@ -125,6 +125,7 @@ def test_schema_classes_equivalent_to_sample_tool_parameters() -> None:
|
|||||||
required=["tag"],
|
required=["tag"],
|
||||||
),
|
),
|
||||||
required=["query", "count"],
|
required=["query", "count"],
|
||||||
|
additional_properties=None,
|
||||||
)
|
)
|
||||||
assert built == SampleTool().parameters
|
assert built == SampleTool().parameters
|
||||||
|
|
||||||
@@ -195,6 +196,25 @@ def test_validate_params_ignores_unknown_fields() -> None:
|
|||||||
assert errors == []
|
assert errors == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_tool_parameters_schema_rejects_unknown_fields_by_default() -> None:
|
||||||
|
tool = DecoratedSampleTool()
|
||||||
|
errors = tool.validate_params({"query": "hi", "count": 2, "extra": "x"})
|
||||||
|
assert errors == ["unexpected parameter extra"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_params_validates_typed_additional_properties() -> None:
|
||||||
|
schema = {
|
||||||
|
"type": "object",
|
||||||
|
"properties": {},
|
||||||
|
"additionalProperties": {"type": "integer"},
|
||||||
|
}
|
||||||
|
tool = CastTestTool(schema)
|
||||||
|
|
||||||
|
errors = tool.validate_params({"extra": "2"})
|
||||||
|
|
||||||
|
assert errors == ["extra should be integer"]
|
||||||
|
|
||||||
|
|
||||||
async def test_registry_returns_validation_error() -> None:
|
async def test_registry_returns_validation_error() -> None:
|
||||||
reg = ToolRegistry()
|
reg = ToolRegistry()
|
||||||
reg.register(SampleTool())
|
reg.register(SampleTool())
|
||||||
|
|||||||
Reference in New Issue
Block a user