fix(api): fail-closed empty messages and spend keys on tools passthrough - #597
fix(api): fail-closed empty messages and spend keys on tools passthrough#597cursor[bot] wants to merge 12 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.
…losed otherwise Legacy /v1/completions treated max_tool_calls as unknown_fields. Accept the key for named invalid_max_tool_calls (null/empty/whitespace omit-equivalent), matching chat/Responses honesty so SDKs get a clear migration path.
SDK clients often send include_usage/include_obfuscation as JSON null. Drop null flag values before validation so null (and null+false mixes) match omit / all-false no-ops on chat, Completions, and Responses. True flags remain fail-closed with invalid_stream_options.
…age honesty Null include_usage/include_obfuscation stay omit-equivalent, but unknown stream_options keys no longer become no-ops just because their value is null. Weight, prefix, refusal, annotations, developer role, empty user/system content, and participant name now use the same named errors on the tools passthrough path as on orchestration. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Hoist stream, required model, stream_options, and temperature/top_p range checks before proxy_completion so a tools or response_format body cannot return a billed JSON completion when the SDK asked for SSE, or silently pick a pool model when model is omitted. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Call the orchestration message, max_tokens, attribution, and routing validators before proxy_completion so a tools or response_format body cannot bill a completion with no prompt, crash on a non-object entry, or silently drop unknown spend/routing keys. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Passthrough has no batch job plane. Reject routing.channel=batch and latency_tolerant=true before proxy_completion so a tool-calling body cannot bill a silent sync completion. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
There was a problem hiding this comment.
Stale comment
Review (#597 at
dbfa0e5)Unique work is sound.
_validate_messages,max_tokens/max_completion_tokens,attribution, androutingnow run beforeproxy_completion, androuting.channel=batch/latency_tolerant=truefail closed. Local HTTP on this head: empty / omitted / null / non-list / non-objectmessageswithtoolsis 400invalid_message; batch and unknown spend keys are 400.Do not merge this head. It is still a draft, Full unit + Semgrep were queued at review time, and request knobs that already 400 on the orchestration path still bill a silent
chat.completionon tools passthrough.Live residual on
dbfa0e5:tools+seed: 1returned 200chat.completionwithprompt_tokens: 0. The same fail-open applies tostop,n>1,logprobs,logit_bias, out-of-range penalties, and unknownreasoning_effort/service_tier.Successor that folds those knobs on top of this head: prefer that PR over #597, #596, #594, and #592. Do not merge those heads in parallel.
Buyer next action on this head: send a non-empty
messagesarray of objects; omitrouting.channel=batchandlatency_tolerant=trueon tool-calling requests. Do not sendseed,stop,n>1, orlogprobswithtoolsuntil the successor lands — those knobs are dropped while the call is still billed.Independent non-author APPROVE + green Full unit / Semgrep are still required. This automation does not approve or merge.
Sent by Cursor Automation: Fix Issues
|
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. |
There was a problem hiding this comment.
Review (#597 at dbfa0e5, ready_for_review)
Unique work is sound and locally verified. _validate_messages, spend keys, and routing now run before proxy_completion. Live HTTP on this head:
tools+ empty / omitted / null / non-list / non-objectmessages→ 400invalid_messagetools+routing.channel=batch→ 400invalid_routingpython3 tests/test_chat_messages_array_tools_passthrough_http_honesty.py→ok
Do not merge this head. Request knobs that already 400 on the orchestration path still bill a silent chat.completion on tools passthrough. Live residual on dbfa0e5:
tools+seed: 1→ 200chat.completiontools+stop: ["END"]→ 200chat.completiontools+n: 2→ 200chat.completion
The same fail-open applies to logprobs, logit_bias, out-of-range penalties, and unknown reasoning_effort / service_tier. Those checks still sit after the tools early-return.
Landing vehicle: prefer #601 (18e6263) which hoists _validate_chat_passthrough_request_knobs. Cherry-pick #603 e58cb2f (omit-real tool.function nulls) onto that tip. Do not merge #589, #592, #594, #596, #597, #599, #600, #603, or #605 in parallel.
Buyer next action on this head: send a non-empty messages array of objects; omit routing.channel=batch and latency_tolerant=true on tool-calling requests. Do not send seed, stop, n>1, or logprobs with tools until #601 lands — those knobs are dropped while the call is still billed.
Independent non-author APPROVE + green Full unit / Semgrep are still required. This automation does not approve or merge. Checks were still queued at review time and are not treated as a merge blocker for this residual.
Sent by Cursor Automation: Fix Issues
| "invalid_routing", | ||
| "routing.latency_tolerant=true is not supported with " | ||
| "tools or response_format on this gateway", | ||
| ) |
There was a problem hiding this comment.
Batch / latency_tolerant now fail closed (live: tools + routing.channel=batch → 400). The early-return still skips the orchestration seed / stop / n / logprobs / logit_bias / penalty blocks below. Live on this head: tools + seed: 1 bills chat.completion. Hoist _validate_chat_passthrough_request_knobs before proxy_completion — that is #601 18e6263. Do not merge this head.
|
Superseded by #607 through direct successor #601. #601 explicitly starts from this exact honesty-stack tip and extends its empty-message/spend/routing boundary with the remaining request-knob validation; #607 then carries that stack forward with the KV-backed provider-host policy. Keeping #597 open duplicates an older landing vehicle. No checks, reviews, or approvals transfer. |


Summary
Successor to #592 at
856adb8, and a superset of draft #596's attribution/routing hoist plus #594's empty-messages hoist.The #592 tip closed unknown-null
stream_optionsand hoisted message-field / stream / model / sampling checks, but_validate_chat_message_passthrough_honestystill skipped non-listmessagesand non-object entries. Live HTTP on that head:tools+[]/ omitted /null/ a string billed achat.completion(prompt_tokens: 0);tools+["hi"]returned500internal_error._validate_messagesbeforeproxy_completionso empty, omitted, null, non-list, and non-objectmessagesuse the sameinvalid_messageas orchestration.max_tokens/max_completion_tokens,attribution, androutingso unknown spend keys and negative token budgets fail closed on the tools path.routing.channel=batchandrouting.latency_tolerant=trueon passthrough (no batch job plane) so those hints cannot bill a silent sync completion.tests/test_chat_messages_array_tools_passthrough_http_honesty.py(RED on fix(api): fail-closed unknown stream_options keys and tools-path message honesty #592: empty messages were 200; batch channel was 200).docs/rest_api_design.mdand README tell buyers to send a non-emptymessagesarray of objects and to omit batch routing hints (APA: OpenAI, 2024).Buyer next action: always send a non-empty
messagesarray of objects; keep SDK-default null flags; omitstream(or setfalse) on tool-calling requests; always send a poolmodel; replacedeveloperwithsystem; omitrouting.channel=batchandlatency_tolerant=trueon tool-calling requests. Do not sendtoolswith empty or omittedmessages.Do not merge #587 at
9d10fa9, #589, #591, #592, #593, #594, or draft #596 in parallel. Prefer this head. Merge only after Full unit + Semgrep are green and an independent non-author review lands.Test plan
python3 tests/test_chat_messages_array_tools_passthrough_http_honesty.pypython3 tests/test_message_honesty_tools_passthrough_http_honesty.pypython3 tests/test_passthrough_stream_model_http_honesty.pypython3 tests/test_stream_options_null_flags_noop_http_honesty.pypython3 tests/test_openai_passthrough.pytest_chat_attribution_routing_http_honesty.pypython3 tests/test_paper_contracts.pytest_self_check.pytest_conventions.pytest_api_contract.pyDocs
docs/rest_api_design.mdhonesty contract now includes empty/non-objectmessages, spend keys, and batch-routing reject before passthrough.messagesarray of objects and to omit batch routing hints.