Repository navigation
fix(api): require HTTPS for custom Google Gemini base URLs (CWE-319) - #2033
easonLiangWorldedtech wants to merge 1 commit into
Conversation
googleGeminiBaseUrl is user-editable (settings, imported profiles) and the @google/genai client keeps API-key authentication for custom endpoints, so a cleartext base URL would send the API key over an insecure endpoint (CWE-319). Validate the endpoint before any request in createMessage and completePrompt: HTTPS is required; plain HTTP is accepted only for loopback test proxies (localhost, [::1], and literal 127.0.0.0/8 IPv4 hosts). Hostnames that merely resemble the loopback range (127.example.test, 127.0.0.a, 127.0.0.1.a, foo127.bar) or fall outside it (10.0.0.1) are rejected, as are non-HTTP schemes such as ftp://localhost and malformed URLs. Violations surface as a structured ApiProviderError before the request is built, so the key never leaves the process. Carved out of the abort-signal provider rollout (fork PR 1303) at the maintainer's request so the endpoint policy can be reviewed independently of the cancellation changes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Regression Evidence |  |
HTTPS custom base URL acceptance lacks focused coverage. The changed helper returns immediately for parsed.protocol === "https:" at src/api/providers/gemini.ts:213-215, and both request paths invo… |
Add focused provider tests with a valid non-loopback https:// googleGeminiBaseUrl. Exercise both completePrompt and createMessage, and assert that each client method is called with the HTTPS base URL. |
| Description check | |
The description provides detailed implementation, security rationale, testing steps, and verification results. However, it does not link an approved GitHub Issue and does not complete the required pre… | Add an approved issue reference, such as "Closes: #123", complete the pre-submission checklist, and fill in the required documentation impact and contributor contact sections. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Security Boundaries No concrete changed path matches the failure conditions. In src/api/providers/gemini.ts:175-226,googleGeminiBaseUrlis parsed withnew URL; HTTPS is accepted, while HTTP is accepted only for `l…Persistence Integrity No changed persistence path exists. The production diff adds URL parsing and validation helpers, plus guards before Gemini requests in createMessageandcompletePrompt. The test-only diff adds val…Lifecycle Resource Cleanup PASS — The pull request adds URL parsing and validation in GeminiHandler.createMessageandcompletePrompt, plus security tests. The changed production lines create no listener, watcher, timer, tas…Title check The title clearly identifies the main change: requiring HTTPS for custom Google Gemini base URLs to address CWE-319.
Full details: Regression Evidence
-
Explanation
HTTPS custom base URL acceptance lacks focused coverage. The changed helper returns immediately for
parsed.protocol === "https:"atsrc/api/providers/gemini.ts:213-215, and both request paths invoke it at lines 354-356 and 654-657. The new Gemini tests cover undefined URLs, loopback HTTP acceptance, and many rejection cases, but they contain nogoogleGeminiBaseUrlvalue usinghttps://. A regression that rejects all custom HTTPS endpoints would pass the current tests.
Full details: Description check
-
Explanation
The description provides detailed implementation, security rationale, testing steps, and verification results. However, it does not link an approved GitHub Issue and does not complete the required pre-submission checklist or template sections.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Review statusThanks for contributing. This comment tracks the review sequence and the next action. Current step: Awaiting fresh human maintainer or CODEOWNER approval. Automated review is complete for the latest commit but does not replace human approval. Review-state labels are managed by this workflow; do not edit them manually. |
github-actions
Bot
added
the
awaiting-maintainer
This branch has not been deployed
2 participants
What this does
googleGeminiBaseUrlis user-editable (settings, imported profiles), and the@google/genaiclient keeps API-key authentication for custom endpoints. Before this PR a cleartext custom base URL was accepted as-is, so the API key could be sent over an insecure endpoint (CWE-319).This PR validates the custom endpoint before any request is built, in both
createMessageandcompletePrompt:http://localhost,http://[::1], and literal127.0.0.0/8IPv4 hosts.127.example.test,127.0.0.a,127.0.a.b,127.0.0.1.a,foo127.bar, and hosts outside the range such as10.0.0.1.ftp://localhost) and malformed URLs are rejected.ApiProviderError(provider/model/operation context) before the request is built, so the key never leaves the process.Content source
Carved out of the fork-side abort-signal provider rollout (fork PR #1303) at the maintainer's request: this endpoint policy changes which custom endpoints are accepted independently of cancellation, so separating it keeps that rollout reviewable. The helpers, guards, and security tests are ported from that rollout branch (the test block is re-nested to this file's structure); the rollout's timeout plumbing stays in that PR.
Budget
main.src/api/providers/gemini.ts(three module-private helpers plus two call-site guards); the remaining 513 lines are the security test matrix insrc/api/providers/__tests__/gemini.spec.ts.How to test
googleGeminiBaseUrltohttp://gemini.example.testand run any Gemini request → it fails with "Google Gemini base URL must use HTTPS (or a loopback HTTP endpoint for local test proxies)" before the request is sent.http://127.0.0.1:8080(orhttp://localhost,http://[::1]) → the request proceeds against the local proxy.src/:vitest run --globals api/providers/__tests__/gemini.spec.ts.Verification at this head
vitest run --globals --no-file-parallelism api/providers/__tests__/gemini.spec.ts api/providers/__tests__/gemini-handler.spec.ts→ 63/63 passed.gemini.tsguards turns exactly the 11 reject tests red while the 5 loopback-allow tests stay green in both directions; byte-exact restore returns all green.eslint --max-warnings=0on both changed files → 0 errors / 0 warnings; suppression counts unchanged.tsc --noEmit→ 0 errors.prettier --check→ clean.