Pull requests / #701

#701 serve: a malformed "tools" is a 400 naming the field (#592)

closed · @bsorensen110 · 0 コメント · GitHub で見る

Server & APISecurity

本文

## Summary

A request whose `tools` is not a list of tool objects takes the request thread down with an `AttributeError`/`TypeError`/`KeyError`. The client sees a connection reset (a reverse proxy shows `502`), and it looks like the engine died. This validates `tools` where the request is parsed, so the caller's existing `ValueError → 400` path answers with a message naming the field. Fixes #592.

## Where it broke

`tools` was the one request field that skipped the repo's shape validation:

- `serve/frontend.py` (OpenAI path): `t.get("function", t) if isinstance(t, dict) and ... else t` — a non-dict entry is handed on unchanged;
- `serve/server.py:2422`: `own = {t.get("name") for t in tools or []}` — assumes every entry is a mapping → `AttributeError: 'str' object has no attribute 'get'`;
- `serve/frontend.py` (Anthropic path): `t["name"]` → `TypeError: string indices must be integers` on a string, `KeyError` on a dict without `name`.

The same file already has the right convention for `messages` and `tool_calls` (`_object_list`, #460): decode a double-encoded JSON string, accept `None` as no field, raise `ValueError` for anything else — the server turns that into a 400 naming the field. `tools` just wasn't wired through it.

## The change

- `serve/frontend.py`: new `_tools_of()` (the #460 convention) used by both `openai_to_messages` and `anthropic_to_messages`; the Anthropic path additionally requires a non-empty `"name"` string per tool.
- `serve/server.py`: the consumer keeps `if isinstance(t, dict)` as a second line of defence (the reporter's suggestion).
- `serve/test_server.py`: `ToolsShape`, 4 tests on the mock engine — no GPU, no pack.

No behavior change for valid requests: flat-form tools, `{"type":"function",...}` tools, a double-encoded JSON string, and an absent field all behave exactly as before.

## Evidence

**RED, reproduced on this repo at `99f3dbd`** — mock server (`python -m serve.server --engine mock --port 8095`), the issue's three repros verbatim:

| request `tools` | before | after |
|---|---|---|
| `"auto"` | connection reset, no reply | `400` — `tools must be a list of objects` |
| `["get_weather"]` | connection reset, no reply | `400` — `tools must be a list of objects` |
| `[{"name":"get_weather"}]` (OpenAI) | accepted (200) | accepted (200) — unchanged |
| Anthropic `[{"name":"get_weather"}]` | accepted (200) | accepted (200) — unchanged |
| Anthropic `[{"description":"no name"}]` | `KeyError` → thread dies | `400` — `tools: every tool needs a "name" string` |

Server log before the fix: 4 `Traceback` entries (`AttributeError: 'str' object has no attribute 'get'` at `serve/server.py:2422`), matching the report.

**Unit tests** (`python -m unittest serve.test_server.ToolsShape -v`):

- on `origin/main`: `Ran 4 tests ... FAILED (errors=4)` — every malformed case raises `http.client.RemoteDisconnected: Remote end closed connection without response`, the exact reported symptom;
- with the fix: 4/4 pass.

**Regression** (mock engine, no GPU): full `serve.test_server` 122 tests OK; `serve.test_security`, `test_structured`, `test_monitor`, `test_lifecycle`, `test_detok`, `test_mcp` all OK.

## Test plan

- [x] `ToolsShape` RED on main (4 errors: RemoteDisconnected), GREEN with the fix
- [x] Full `serve.test_server` (122) + security/structured/monitor/lifecycle/detok/mcp suites pass
- [x] Issue's repro commands re-run against a mock server: 400s naming `tools`, server answers the next request normally
- [ ] CI

🤖 Generated with Hermes Agent

関連リンク

インストール・モデル・リリースへの站内リンク。