fix(security): read provider host allowlist from KV at request time - #598
fix(security): read provider host allowlist from KV at request time#598cursor[bot] wants to merge 11 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>
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>
|
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 (#598 unique slice a50911f)
The unique 14-file allowlist change is the right request-time source: ModelClient._validate_provider reads allowed_provider_hosts() only, and seed_provider_egress_from_environ() is on both __main__.main and serve(). Empty process KV still means "no extra hostname filter"; private/loopback/reserved stay fail-closed. No leftover os.getenv on the request path. Naming and citations (NIST SP 800-53 Rev. 5 SC-7; ISO/IEC 27001:2022 A.8.20) are in place.
This PR must not land as the 152-file stack on #589.
Why not this head
- Stacked on #589 (
b4318b3), not on the current honesty-stack landing vehicle #601 (7e9a339, already includes omit-real tool.function nulls and streamed request temperature). test_seed_provider_egress_from_environ_copies_oncenever calledseed()a second time, so a recopying seed would still pass.- Env-only ignore was not proven on
_validate_provider(helper-only). - Docs said "seed the KV" without stating the process-wide
InMemoryConfigStoreis not a detached Postgrescom_configwrite.
Merger next action
Prefer #611 (c91629e on 7e9a339): same unique allowlist, rebased onto #601, with seed-once / env-ignore-on-request-path / detached-store isolation tests, seed lock, and a _parse_host_allowlist fuzz seam.
Do not merge #598, #589, or #591 in parallel. Independent non-author APPROVE is still required on #611. I am not approving this PR.
Buyer next action after #611 lands: set_runtime_config("provider_egress", "allowed_provider_hosts", "api.example.com") (or start the process with the env var set so bootstrap can copy it once). Do not write the key only into a new get_config_store(postgres_dsn=...) instance.
Sent by Cursor Automation: Fix Issues
|
Superseded by #607. #607 explicitly reapplies this PR’s unique KV-backed provider-host allowlist slice ( |
Pull request was closed
2 participants
Summary
os.getenvleftover on provider egress:ModelClient._validate_providernow readsprovider_egress.allowed_provider_hostsfrom the process KV (allowed_provider_hosts()), neverCONTEXTUAL_ORCHESTRATOR_ALLOWED_PROVIDER_HOSTSduring a call.seed_provider_egress_from_environ()copies the env CSV into the KV once atpython -m contextual_orchestrator/serve()when that KV key is still empty. Later env edits on a running process do not change policy.tests/test_provider_host_allowlist_kv.pyproves env-only allowlists are ignored, KV CSV is honored, bootstrap copies once, andhttps://api.openai.comis rejected when the KV lists onlyexample.com.Merger next action
a50911f) onto that tip — or merge this only if you accept the fix(api): fail-closed empty messages on tools passthrough #589 honesty stack without the fix(api): fail-closed stream, model, and sampling range on tools passthrough #591 hoist.Independent non-author APPROVE still required. Product gates: Full unit + Semgrep.
Test plan
python3 tests/test_provider_host_allowlist_kv.pypython3 tests/test_security_hardening.pypython3 tests/test_self_check.pytest_paper_contracts.pytest_conventions.pytest_api_contract.pytest_product_planning_contract.pypython3 tests/test_provider_tls.pytest_provider_reliability.pytest_repository_security_metadata.pyBuyer next action
Seed
provider_egress.allowed_provider_hostsin the KV (or start the process withCONTEXTUAL_ORCHESTRATOR_ALLOWED_PROVIDER_HOSTSset so bootstrap can copy it). Do not expect a later env edit to change egress on a live process.References