fix(security): strip register-credential --from-env mounted-secret newlines - #671
Closed
seonghobae wants to merge 22 commits into
Closed
fix(security): strip register-credential --from-env mounted-secret newlines#671seonghobae wants to merge 22 commits into
seonghobae wants to merge 22 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>
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>
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>
Irreversible email [REDACTED] on opt-in traces made invoice and HR tickets unworkable. Credential shapes stay redacted. Access control plus audit is the PII control (NIST SP 800-122; SP 800-53 AC-3/AU-2). Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
CONTEXTUAL_ORCHESTRATOR_TOKEN and the split admin/inference vars are bootstrap transport via seed_server_auth_from_environ. Explicit --auth-token still wins. A later env edit does not change who can call the API. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Mounted secrets and echo often append a trailing newline. Seed stored the raw value while authorize() strips the presented Bearer, so bootstrap succeeded and every call was 401. Persist and resolve stripped values. Docs now say restart does not recopy a persisted key; rotate with --auth-token or register-credential. Buyer next action: send the stripped Bearer. Rotate a persisted KV with --auth-token or register-credential, not by editing env and restarting. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
…wlines --from-env stored the raw env value while --value-stdin already stripped. A Docker/K8s secret trailing newline became Authorization: Bearer sk-… plus LF and providers 401. register_credential now persists stripped values and rejects whitespace-only secrets. get_credential strips so a secret stored before this write-path still authorizes. Docs no longer say --auth-token rotates a persisted key. That flag overrides this process only; persist with register-credential. Buyer next action: seed with --from-env or --value-stdin from the mounted secret, then send traffic. Rotate a persisted authenticator with register-credential, not --auth-token plus restart. Grounding: Joint Task Force (2020) NIST SP 800-53 Rev. 5 IA-5; Grassi et al. (2017) NIST SP 800-63B. 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. |
|
Important Review skippedToo many files! This PR contains 165 files, which is 65 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (165)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
enabled auto-merge (squash)
August 16, 2026 17:06
auto-merge was automatically disabled
August 16, 2026 18:22
Pull request was closed
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unique slice
7c58a0don #64447ccf5a.register-credential --from-envstored the raw env value while--value-stdinalready stripped. A Docker/K8s secret trailing newline persisted and becameAuthorization: Bearer sk-…plus LF on the provider request.register_credentialnow stores stripped values and rejects whitespace-only secrets.get_credentialalso strips so a secret stored before this write-path still authorizes.Docs no longer say
--auth-tokenrotates a persisted key. That flag overrides this process only. Persist withregister-credential.Local:
tests/test_kv_credentials.py(mounted-secret newline stores stripped; provider Authorization has no LF) plustests/test_server_auth_kv.py, paper/API/conventions, and security hardening printed ok.Merger next action
This is the register write-path slice, not a third token-KV competitor. Prefer #644 for seed/resolve strip. Prefer this tip for
--from-envpersist strip and the--auth-tokendocs honesty fix. Do not open another register-strip PR. Independent non-author APPROVE plus Full unit / Semgrep are required. This automation will not approve or merge.Buyer next action
Seed with
--from-envor--value-stdinfrom the mounted secret, then send traffic. A trailing newline is not stored and is not sent upstream. Rotate a persisted authenticator withregister-credential, not--auth-tokenplus restart.Grounding: Joint Task Force (2020) NIST SP 800-53 Rev. 5 IA-5; Grassi et al. (2017) NIST SP 800-63B.