fix(api): fail-closed empty messages on tools passthrough - #589
fix(api): fail-closed empty messages on tools passthrough#589cursor[bot] wants to merge 10 commits into
Conversation
…closed otherwise Chat history: message-level audio and legacy function_call are null/empty omit no-ops; non-empty fail closed with named errors (including tools passthrough). Tip substrate from #577 assistant refusal/annotations honesty. Local full unit: 940 passed.
…ed otherwise OpenAI fine-tune style message weight is not applied on this gateway. Accept null/0/1 as honest no-ops; reject other types and values with invalid_message_weight. Tip substrate from #578. Local full unit: 943 passed.
…ion role Reject unsupported message keys with named unknown_message_fields (not silent strip or tools-passthrough smuggle). Reject legacy function role with invalid_message_role migration to tool. Tip substrate from #579. Local full unit: 947 passed.
OpenAI partial-assistant prefix flag is not applied on this gateway. null/false are honest no-ops; true and non-booleans fail closed with invalid_message_prefix. Tip substrate from #580. Local full unit: 950 passed.
…therwise Named invalid_max_tool_calls on /v1/chat/completions instead of opaque unknown_fields. Aligns with Responses max_tool_calls honesty; gateway has no multi-step tool loop.
Validate weight/prefix/refusal/annotations before the tools early-return so SDK tool-calling bodies cannot smuggle unsupported message fields. Strip omit-equivalent max_tool_calls before provider passthrough and name-reject the field on /v1/completions. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Reject non-OpenAI keys on assistant tool_calls objects with named unknown_tool_call_fields / unknown_tool_call_function_fields. Accept optional non-negative index (null omit) for stream-assembled histories. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Developer role, empty user/system content, multimodal part shape, and participant name were only checked in _validate_messages, which the tools/response_format early-return skips. SDK tool-calling bodies could therefore proxy unsupported values. Run the same fail-closed checks before passthrough and cover the realistic tools+field HTTP cases. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Completions and chat applied temperature, top_p, penalties, and max_tokens by mutating shared ModelClient defaults. Concurrent ThreadingHTTPServer workers could observe another request's knobs. request_sampling stores overrides on threading.local and restores them on exit; chat/stream_chat read the calling thread first. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
_validate_messages is skipped on the tools/response_format early-return. Require a non-empty messages array before proxy so SDK tool-calling bodies cannot bill a completion with no prompt. Split the optional tool_calls index case so omit-key is actually tested. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: empty-messages gate is correct; do not merge this head
_require_nonempty_chat_messages at server.py 1436–1445 is called at 3588, before the tools/response_format early-return. Missing, JSON null, non-list, and empty messages now return 400 / invalid_message / messages must be a non-empty array — the same contract as _validate_messages on the orchestration path. Live HTTP cases in tests/test_chat_messages_required_tools_passthrough_http_honesty.py fail if the helper is removed (they would get 200 chat.completion).
No must-fix on this slice.
Next action for the merger
Do not merge #589. Prefer #591 (b7f81a6). That head already contains this empty-messages commit and hoists stream=true, omitted model, stream_options.include_usage=true, and out-of-range temperature / top_p before proxy_completion. Merging #589 first only creates a conflict with the actual stack tip.
Also leave closed: #582–#586, #594 (parallel empty-messages draft).
Independent non-author APPROVE still belongs on #591, after Full unit + Semgrep are green.
Residual fail-open on this head (already closed on #591)
On b4318b3, tools + valid messages still return 200 for omitted model, stream: true (JSON, not SSE), stream_options.include_usage=true, and temperature: 99. Those are out of this slice.
Buyer next action
Send a non-empty messages array with every tools / response_format body. An SDK default of messages: [] or a missing messages key is now a named invalid_message, not a billed empty completion.
Sent by Cursor Automation: Fix Issues
|
Superseded by the later honesty-stack line ending at #611. #594/#597 preserve and extend this PR’s non-empty-message boundary, and #601/#611 carry that behavior on the newer request/security substrate. Keeping #589 open would duplicate an older landing vehicle. No checks, reviews, or approvals transfer. |
Pull request was closed
2 participants
Summary
messagesnow fail closed withinvalid_messagebefore the passthrough early-return._require_nonempty_chat_messagesmatches the orchestration-path contract so an SDK tool-calling body cannot bill a completion with no prompt.tool_callsindexcase so omit-key is actually tested (the previousif index is not None or index is Nonetautology always set the key).This branch is the mergeable repair of #586 (
28ef9d2) and therefore of #582–#585. Do not merge those heads — tools +messages: []/ omitted /null/ non-list still returned 200 on #586.Test plan
python3 tests/test_chat_messages_required_tools_passthrough_http_honesty.pypython3 tests/test_tool_calls_entry_keys_http_honesty.pypython3 tests/test_message_role_content_name_tools_passthrough_http_honesty.pypython3 tests/test_message_weight_http_honesty.pypython3 tests/test_chat_assistant_tool_calls_http_honesty.pypython3 tests/test_openai_passthrough.pypython3 tests/test_paper_contracts.pytest_conventions.pyProduct gates only: Full unit + Semgrep (Strix ignored). Independent non-author APPROVE still required.
References