fix(security): rebase KV host allowlist onto the SSE tools tip - #619
fix(security): rebase KV host allowlist onto the SSE tools tip#619cursor[bot] wants to merge 30 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>
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>
Hoist attribution and routing validation before proxy_completion so a tools or response_format body cannot bill a sync completion with an unknown spend dimension or a batch/latency_tolerant hint. Tools passthrough has no batch job plane. Buyer next action: send known sync attribution; omit routing.channel=batch and latency_tolerant=true on tool-calling requests. 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>
Match _validate_messages: tools + user content 123 must 400 invalid_message instead of billing a sync completion. Assistant/tool JSON null stays omit-equivalent. Buyer next action: send user/system content as a non-empty string or a content-parts array. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Match the tools-path honesty contract to the invalid_message behavior so SDK clients send a string or content-parts array. 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>
Hoist the remaining chat request knobs before proxy_completion so an OpenAI SDK tool-calling body cannot bill a sync completion for seed, stop, n>1, logprobs, logit_bias, out-of-range token/penalty values, unsupported reasoning_effort, or a non-default service_tier. Buyer next action: omit those fields on tool-calling requests; they are not applied on this gateway. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Call _validate_messages before proxy_completion so tools + [] / omitted / null / non-list / non-object messages raise invalid_message instead of billing a completion with no prompt. Buyer next action: always send a non-empty messages array of objects on tool-calling requests. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Hoist the orchestration request-knob validators before proxy_completion so a tools or response_format body cannot bill a completion that silently drops seed, stop, n>1, logprobs, logit_bias, or out-of-range penalties. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
stream_chat defaulted to 0.2 even after the HTTP path wrote default_temperature from the request. A streamed invoice summary at temperature=0.8 no longer silently falls back to 0.2. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
… proxy SDK optional defaults serialize omitted fields as JSON null. Accepting those keys without popping them is not omit-equivalent: proxy_completion forwards the body and several providers reject a null JSON Schema object. Pop the keys in place, keep non-null wrong types on invalid_tools, and assert omit-real via mock echo on chat and Responses. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
OpenAI SDK tool callers send stream=true. Returning 400 was honest but left every streaming tool client failing. Pipe a single pool agent as chat.completion.chunk frames so concatenated mock content matches the non-stream JSON body, and pipe live provider text so tool_calls deltas survive. Keep include_usage, empty messages, and unsupported knobs fail-closed. Buyer next action: send stream=true on tool-calling requests; omit stream_options.include_usage. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep tools/response_format SSE proxy from this branch and take #601's null tool.function omit-pop, request-knob HTTP cases, and streamed route temperature. Conflict resolution keeps stream=true as SSE, not 400. CHANGELOG records the combined unreleased slice. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Unknown assistant tool_calls entry/function fields, non-boolean include_orchestration_trace, and unknown mode used to bill a JSON or SSE completion on the tools proxy. They now return the same named 400s as the orchestration path before the first byte. Buyer next action: send only id/type/function/optional index on assistant tool_calls; send a boolean or omit include_orchestration_trace; send auto/route/conduct or omit mode. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
mock:// lookup_balance now returns assistant tool_calls and streams delta.tool_calls with finish_reason=tool_calls so offline SDK clients match the live _stream_raw path. Invoice identifiers bind from the user text (default INV-9). Buyer next action: send stream=true on tool-calling requests and include the invoice id in the user message. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
stream_chat recorded temperature but omitted the request-scoped top_p, presence_penalty, and frequency_penalty that chat() already forwards. A streamed invoice summary at top_p=0.1 no longer drops those knobs. Buyer next action: send the nucleus and penalty values you want on stream=true route requests; streaming matches the JSON path. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Stop reading CONTEXTUAL_ORCHESTRATOR_ALLOWED_PROVIDER_HOSTS in ModelClient._validate_provider. The allowlist lives in provider_egress.allowed_provider_hosts; env is bootstrap transport via seed_provider_egress_from_environ only. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Prove a second seed() after an env edit keeps the first KV set, that _validate_provider ignores env-only allowlists, and that a detached get_config_store() write is not the request-time source. Hold the runtime lock across seed read+write and fuzz the host CSV parser. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
A stored " " used to skip env seed while parsing as an empty set, freezing fail-open public HTTPS. Bootstrap now recopies env for None/""/whitespace. Request-path env-ignore and listed-host tests mock getaddrinfo so CI stays offline and still requires success. 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. |
There was a problem hiding this comment.
Review (unique slice a1823cc..be39a28)
The KV host-allowlist rebase onto the SSE tools tip is sound. ModelClient._validate_provider reads provider_egress.allowed_provider_hosts from the process ConfigStore. Env is bootstrap-only via seed_provider_egress_from_environ under _runtime_lock. Whitespace-only stored values count as empty. Env-ignore and listed-host success paths mock getaddrinfo (offline). Detached get_config_store() writes are not the request-time source.
CodeRabbit CLI is not authenticated in this environment — not a merge blocker. An independent file review of the unique commits found no Critical or Important defects.
Merger next action
Prefer this head over merging #598, #607, #608, #611, or #617 in parallel. Independent non-author APPROVE plus Full unit / Semgrep are still required. This automation will not APPROVE or merge.
Buyer next action
Call set_runtime_config("provider_egress", "allowed_provider_hosts", "api.example.com"), or start once with CONTEXTUAL_ORCHESTRATOR_ALLOWED_PROVIDER_HOSTS set so bootstrap can copy it. Do not write the key only into a new get_config_store(postgres_dsn=...) instance. Changing the env var on a running process no longer changes egress. Send stream=true on tool-calling requests; omit stream_options.include_usage.
Residual (not blocking)
Seed emptiness is str(existing).strip(). An empty sequence ([]) stringifies to a non-empty token, so seed will not recopy env while the parser still yields an empty allowlist. The documented API is a CSV string. Do not open another allowlist-only PR for this.
Next loop
Remaining __main__.py process-bootstrap paths (state/agents sqlite, Clearfolio viewer URL, provider CA bundle) on this tip. Do not fold #621 token KV into that slice. Do not open another allowlist-only PR.
Sent by Cursor Automation: Fix Issues
Pull request was closed
|
Deferring to tip #691 for product-gate focus (Full unit + Semgrep). Reopen if this delta is not on tip substrate. |


Summary
Successor to #617 at
a1823cc(SSE tools proxy, mockdelta.tool_calls, streamedtop_p/penalties, fail-closedtool_callskeys and mode/trace). Unique commits on this branch rebase #611/#607allowlist onto that tip and close the residual #607 review gap:ModelClient._validate_providerreadsprovider_egress.allowed_provider_hostsfrom the process KV. Env is bootstrap transport viaseed_provider_egress_from_environonly._runtime_lockacross read+write. A detachedget_config_store()write is not the request-time source." "cannot freeze fail-open public HTTPS.getaddrinfo(offline) and require success.exercise_host_allowlistfuzz seam for the CSV parser.Buyer next action
Call
set_runtime_config("provider_egress", "allowed_provider_hosts", "api.example.com"), or start the process withCONTEXTUAL_ORCHESTRATOR_ALLOWED_PROVIDER_HOSTSset so bootstrap can copy it once. Do not write the key only into a newget_config_store(postgres_dsn=...)instance. Changing the env var on a running process no longer changes egress. Sendstream=trueon tool-calling requests; omitstream_options.include_usage.Do not merge #598, #607, #608, #611, or #617 in parallel. Prefer this head. Independent non-author APPROVE + Full unit/Semgrep are still required. This automation will not APPROVE or merge.
Test plan
python3 tests/test_provider_host_allowlist_kv.py— printedok(seed-once; whitespace-empty seed; process-store isolation; offline env-ignore)python3 tests/test_security_hardening.pytest_conventions.pytest_self_check.pytest_paper_contracts.pytest_api_contract.pytest_sales_readiness.py— printedokpython3 tests/test_passthrough_sse_tools_http_honesty.pytest_tool_calls_entry_keys_http_honesty.pytest_passthrough_trace_mode_http_honesty.py— printedokDocs
docs/kv-credentials.md,docs/architecture.md,docs/library_research.md,docs/papers/README.md,CHANGELOG.md,conductor/tracks.mdNext loop
Remaining
__main__.pybootstrap tokens, then persistprovider_egresson the credential KV backend. Do not open another allowlist-only PR.