From ee0090f96c60c9dc903ed7f893f71d1d88842aee Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Tue, 13 Oct 2026 08:32:41 +0800 Subject: [PATCH] fix(api): require HTTPS for custom Google Gemini base URLs (CWE-319) 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. --- src/api/providers/__tests__/gemini.spec.ts | 513 +++++++++++++++++++++ src/api/providers/gemini.ts | 66 +++ 2 files changed, 579 insertions(+) diff --git a/src/api/providers/__tests__/gemini.spec.ts b/src/api/providers/__tests__/gemini.spec.ts index 0920042586..1e830fe181 100644 --- a/src/api/providers/__tests__/gemini.spec.ts +++ b/src/api/providers/__tests__/gemini.spec.ts @@ -12,6 +12,8 @@ vitest.mock("@roo-code/telemetry", () => ({ import { Anthropic } from "@anthropic-ai/sdk" +import type { GenerateContentResponse } from "@google/genai" + import { type ModelInfo, geminiDefaultModelId, ApiProviderError } from "@roo-code/types" import { t } from "i18next" @@ -21,6 +23,11 @@ import { asyncStreamFrom, collectStream } from "../../../test-utils/stream" const GEMINI_MODEL_NAME = geminiDefaultModelId +// @google/genai's GenerateContentResponse exposes `text` via a getter backed by +// `candidates`, so the stub only carries the field the provider reads; the double +// cast is the least-friction way to satisfy the class type in mocks. +const stubGenerateContentResponse = (text: string) => ({ text }) as unknown as GenerateContentResponse + describe("GeminiHandler", () => { let handler: GeminiHandler let mockGenerateContentStream: ReturnType @@ -430,6 +437,512 @@ describe("GeminiHandler", () => { }) }) + describe("googleGeminiBaseUrl security (CWE-319)", () => { + it("should reject a non-HTTPS non-loopback googleGeminiBaseUrl in completePrompt", async () => { + const insecureHandler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl: "http://gemini.example.test", + }) + + insecureHandler["client"] = handler["client"] + + vi.mocked(handler["client"].models.generateContent).mockResolvedValue( + stubGenerateContentResponse("Response"), + ) + + await expect(insecureHandler.completePrompt("Test prompt")).rejects.toThrow( + t("common:errors.gemini.generate_complete_prompt", { + error: "Google Gemini base URL must use HTTPS (or a loopback HTTP endpoint for local test proxies)", + }), + ) + + expect(handler["client"].models.generateContent).not.toHaveBeenCalled() + }) + + it("should allow a loopback HTTP googleGeminiBaseUrl in completePrompt", async () => { + const loopbackHandler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl: "http://127.0.0.1:8080", + }) + + loopbackHandler["client"] = handler["client"] + + vi.mocked(handler["client"].models.generateContent).mockResolvedValue( + stubGenerateContentResponse("Response"), + ) + + const result = await loopbackHandler.completePrompt("Test prompt") + + expect(result).toBe("Response") + + expect(handler["client"].models.generateContent).toHaveBeenCalledWith( + expect.objectContaining({ + config: expect.objectContaining({ + httpOptions: { + baseUrl: "http://127.0.0.1:8080", + }, + }), + }), + ) + }) + + it("should reject a non-HTTPS non-loopback googleGeminiBaseUrl in createMessage", async () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + + content: "Hello", + }, + ] + + const stub = vi.fn().mockReturnValue((async function* () {})()) + + const insecureHandler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl: "http://insecure.example.com", + }) + + insecureHandler["client"] = handler["client"] + + handler["client"].models.generateContentStream = stub + + const stream = insecureHandler.createMessage("You are a helpful assistant", messages) + + const error = await collectStream(stream).catch((e: unknown) => e) + + expect(error).toBeInstanceOf(ApiProviderError) + + expect((error as ApiProviderError).message).toBe( + "Google Gemini base URL must use HTTPS (or a loopback HTTP endpoint for local test proxies)", + ) + + expect((error as ApiProviderError).provider).toBe("Gemini") + + expect((error as ApiProviderError).operation).toBe("createMessage") + + expect(stub).not.toHaveBeenCalled() + }) + + it("should allow a loopback HTTP googleGeminiBaseUrl in createMessage", async () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + + content: "Hello", + }, + ] + + const stub = vi.fn().mockReturnValue((async function* () {})()) + + const loopbackHandler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl: "http://127.0.0.1:8080", + }) + + loopbackHandler["client"] = handler["client"] + + handler["client"].models.generateContentStream = stub + + await collectStream(loopbackHandler.createMessage("You are a helpful assistant", messages)) + + const config = stub.mock.calls[0][0].config + + expect(config.httpOptions).toEqual({ baseUrl: "http://127.0.0.1:8080" }) + }) + + it("should allow an http://localhost googleGeminiBaseUrl in createMessage", async () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + + content: "Hello", + }, + ] + + const stub = vi.fn().mockReturnValue((async function* () {})()) + + const localhostHandler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl: "http://localhost:8080", + }) + + localhostHandler["client"] = handler["client"] + + handler["client"].models.generateContentStream = stub + + await collectStream(localhostHandler.createMessage("You are a helpful assistant", messages)) + + expect(stub.mock.calls[0][0].config.httpOptions).toEqual({ baseUrl: "http://localhost:8080" }) + }) + + it("should allow an http://[::1] googleGeminiBaseUrl (IPv6 loopback host)", async () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + + content: "Hello", + }, + ] + + const stub = vi.fn().mockReturnValue((async function* () {})()) + + const ipv6Handler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl: "http://[::1]:8080", + }) + + ipv6Handler["client"] = handler["client"] + + handler["client"].models.generateContentStream = stub + + await collectStream(ipv6Handler.createMessage("You are a helpful assistant", messages)) + + expect(stub.mock.calls[0][0].config.httpOptions).toEqual({ baseUrl: "http://[::1]:8080" }) + }) + + it("should reject hostnames that only resemble the 127. range", async () => { + // "127a.b" is not a four-part 127.0.0.0/8 literal, so it must not + + // pass the loopback check. A prefix-style "starts with 127." test + + // would have matched it and wrongly allowed cleartext. (Hosts such + + // as a127.0.0.1 are rejected by new URL() outright — its mixed + + // digit-led/letter-led label rule — so they never reach the check.) + + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + + content: "Hello", + }, + ] + + const stub = vi.fn().mockReturnValue((async function* () {})()) + + const restrictedHandler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl: "http://127a.b:8080", + }) + + restrictedHandler["client"] = handler["client"] + + handler["client"].models.generateContentStream = stub + + const error = await collectStream( + restrictedHandler.createMessage("You are a helpful assistant", messages), + ).catch((e: unknown) => e) + + expect(error).toBeInstanceOf(ApiProviderError) + + expect((error as ApiProviderError).message).toBe( + "Google Gemini base URL must use HTTPS (or a loopback HTTP endpoint for local test proxies)", + ) + + expect((error as ApiProviderError).provider).toBe("Gemini") + + expect(stub).not.toHaveBeenCalled() + }) + + it("should reject a valid hostname containing the 127. substring that is not loopback", async () => { + // "foo127.bar" is accepted by new URL() (a syntactically valid + + // hostname) and contains the "127." substring, yet is not a + + // loopback address: its first label is "foo127", not the literal + + // 127 octet, so the four-part 127.0.0.0/8 check rejects it. + + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + + content: "Hello", + }, + ] + + const stub = vi.fn().mockReturnValue((async function* () {})()) + + const restrictedHandler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl: "http://foo127.bar:8080", + }) + + restrictedHandler["client"] = handler["client"] + + handler["client"].models.generateContentStream = stub + + const error = await collectStream( + restrictedHandler.createMessage("You are a helpful assistant", messages), + ).catch((e: unknown) => e) + + expect(error).toBeInstanceOf(ApiProviderError) + + expect((error as ApiProviderError).message).toBe( + "Google Gemini base URL must use HTTPS (or a loopback HTTP endpoint for local test proxies)", + ) + + expect((error as ApiProviderError).provider).toBe("Gemini") + + expect(stub).not.toHaveBeenCalled() + }) + + const INSECURE_BASE_URL_CASES = [ + // "127.example.test" is a syntactically valid hostname (a digit-led + + // first label is legal DNS) that new URL() accepts, but it is a + + // public domain — not a literal 127.0.0.0/8 address — so cleartext + + // must be rejected. A "starts with 127." prefix check would wrongly + + // allow it and leak the API key in cleartext. + + ["a public hostname whose first label is 127", "http://127.example.test:8080"], + + // "127.0.0.a" is accepted by new URL() (a non-digit last label + + // skips IPv4 validation), but the last label is not a decimal + + // octet, so the host is not loopback and cleartext must be + + // rejected. + + ["a four-part host with a non-numeric last octet", "http://127.0.0.a:8080"], + + // "127.0.a.b" is accepted by new URL(), but its third label is + + // not a decimal octet, so the host is not loopback and cleartext + + // must be rejected even though the first and fourth labels are + + // numeric. + + ["a four-part host with a non-numeric middle octet", "http://127.0.a.b:8080"], + + // "127.0.0.1.a" is a hostname (not an IPv4 literal) that new + + // URL() accepts: its five labels mean it must not pass the + + // four-part 127.0.0.0/8 check, so cleartext must be rejected. + + ["a five-part host that starts with a loopback address", "http://127.0.0.1.a:8080"], + + // "10.0.0.1" is a valid IPv4 literal that new URL() accepts, but its + + // first octet is not 127, so it is not loopback and cleartext must + + // be rejected. This pins the 127 label comparison itself. + + ["a four-part IPv4 host outside the 127.0.0.0/8 range", "http://10.0.0.1:8080"], + ] as const + + it.each(INSECURE_BASE_URL_CASES)("should reject %s", async (_name, googleGeminiBaseUrl) => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + + content: "Hello", + }, + ] + + const stub = vi.fn().mockReturnValue((async function* () {})()) + + const restrictedHandler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl, + }) + + restrictedHandler["client"] = handler["client"] + + handler["client"].models.generateContentStream = stub + + const error = await collectStream( + restrictedHandler.createMessage("You are a helpful assistant", messages), + ).catch((e: unknown) => e) + + expect(error).toBeInstanceOf(ApiProviderError) + + expect((error as ApiProviderError).message).toBe( + "Google Gemini base URL must use HTTPS (or a loopback HTTP endpoint for local test proxies)", + ) + + expect((error as ApiProviderError).provider).toBe("Gemini") + + expect(stub).not.toHaveBeenCalled() + }) + + it("should allow the full 127.0.0.0/8 range including 127.255.255.255", async () => { + // The octet boundary is inclusive: 255 is the largest valid decimal + + // octet and must be accepted as loopback. A < instead of <= (or a + + // lowered boundary) would wrongly reject valid loopback endpoints. + + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + + content: "Hello", + }, + ] + + const stub = vi.fn().mockReturnValue((async function* () {})()) + + const loopbackHandler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl: "http://127.255.255.255:8080", + }) + + loopbackHandler["client"] = handler["client"] + + handler["client"].models.generateContentStream = stub + + await collectStream(loopbackHandler.createMessage("You are a helpful assistant", messages)) + + const config = stub.mock.calls[0][0].config + + expect(config.httpOptions).toEqual({ baseUrl: "http://127.255.255.255:8080" }) + }) + + it("should reject a non-HTTP loopback scheme such as ftp://localhost", async () => { + // The protocol operand (left of the && in the loopback exception) must + + // still be enforced: a non-HTTP scheme aimed at a loopback host is not + + // a local test proxy and must be rejected like any other non-HTTPS URL. + + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + + content: "Hello", + }, + ] + + const stub = vi.fn().mockReturnValue((async function* () {})()) + + const ftpHandler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl: "ftp://localhost:8080", + }) + + ftpHandler["client"] = handler["client"] + + handler["client"].models.generateContentStream = stub + + const error = await collectStream(ftpHandler.createMessage("You are a helpful assistant", messages)).catch( + (e: unknown) => e, + ) + + expect(error).toBeInstanceOf(ApiProviderError) + + expect((error as ApiProviderError).message).toBe( + "Google Gemini base URL must use HTTPS (or a loopback HTTP endpoint for local test proxies)", + ) + + expect((error as ApiProviderError).provider).toBe("Gemini") + + expect(stub).not.toHaveBeenCalled() + }) + + it("should reject an invalid googleGeminiBaseUrl in createMessage", async () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + + content: "Hello", + }, + ] + + const stub = vi.fn().mockReturnValue((async function* () {})()) + + const invalidHandler = new GeminiHandler({ + apiKey: "test-key", + + apiModelId: GEMINI_MODEL_NAME, + + geminiApiKey: "test-key", + + googleGeminiBaseUrl: "not a valid url", + }) + + invalidHandler["client"] = handler["client"] + + handler["client"].models.generateContentStream = stub + + const error = await collectStream( + invalidHandler.createMessage("You are a helpful assistant", messages), + ).catch((e: unknown) => e) + + expect(error).toBeInstanceOf(ApiProviderError) + + expect((error as ApiProviderError).message).toBe("Invalid Google Gemini base URL (not a valid URL)") + + expect((error as ApiProviderError).provider).toBe("Gemini") + + expect((error as ApiProviderError).operation).toBe("createMessage") + + expect(stub).not.toHaveBeenCalled() + }) + }) + describe("getModel", () => { it("should return correct model info", () => { const modelInfo = handler.getModel() diff --git a/src/api/providers/gemini.ts b/src/api/providers/gemini.ts index 7674521d13..97e24e3b04 100644 --- a/src/api/providers/gemini.ts +++ b/src/api/providers/gemini.ts @@ -172,6 +172,59 @@ function sanitizeSchemaForGemini( return result } +// googleGeminiBaseUrl is user-editable and can reach non-HTTPS values (settings, +// imported profiles). The @google/genai client keeps API-key authentication for +// custom endpoints, so reject cleartext base URLs before any request — with a +// narrow loopback exception for local test proxies. +// A decimal octet is a 1-3 digit string; a non-numeric part also fails the +// range check, because Number() of a non-numeric string is NaN, which is not +// <= 255. +function isOctet(octet: string): boolean { + // Stryker disable next-line Regex,LogicalOperator,ConditionalExpression: a malformed octet newly accepted by a mutated regex or operand is non-numeric (Number() is NaN, failing <= 255), out of range (e.g. 256), or 4+ digits, all of which are unreachable because new URL() rejects the all-digit host; the ->false and stricter-regex variants are killed by the 127.0.0.1 and 127.255.255.255 accept tests + return /^\d{1,3}$/.test(octet) && Number(octet) <= 255 +} + +// Only literal IPv4 loopback (127.0.0.0/8) qualifies: public hostnames may +// start with a "127." label (e.g. 127.example.test), which a prefix test would +// misclassify as loopback and allow cleartext. assertSecureGeminiBaseUrl has +// already parsed the URL and only reaches this for the HTTP exception, so the +// parsed hostname is passed directly; `new URL` keeps the brackets in IPv6 +// hostnames, so `[::1]` is the loopback host form to compare against. +function isLoopbackHostname(hostname: string): boolean { + if (hostname === "localhost" || hostname === "[::1]") { + return true + } + const parts = hostname.split(".") + // Stryker disable next-line ConditionalExpression: the ->true variants of isOctet(parts[1]) and isOctet(parts[2]) are unobservable because any four-part host with a non-numeric or out-of-range middle octet is rejected by new URL() before reaching this check; the remaining variants are killed by the 127.0.0.1, 127.255.255.255, 10.0.0.1, 127.0.0.a and 127.0.0.1.a tests + return parts.length === 4 && parts[0] === "127" && isOctet(parts[1]) && isOctet(parts[2]) && isOctet(parts[3]) +} + +// Throws an ApiProviderError when baseUrl is not HTTPS (loopback HTTP is the +// narrow exception, for local test proxies). The provider/model/operation +// arguments keep the structured error context consistent with the request-path +// ApiProviderError instances in this file. +function assertSecureGeminiBaseUrl(baseUrl: string, modelId: string, operation: string): void { + let parsed: URL + try { + parsed = new URL(baseUrl) + } catch { + throw new ApiProviderError("Invalid Google Gemini base URL (not a valid URL)", "Gemini", modelId, operation) + } + if (parsed.protocol === "https:") { + return + } + if (parsed.protocol === "http:" && isLoopbackHostname(parsed.hostname)) { + // Loopback endpoints (localhost/127.x/::1) are allowed for local test proxies. + return + } + throw new ApiProviderError( + "Google Gemini base URL must use HTTPS (or a loopback HTTP endpoint for local test proxies)", + "Gemini", + modelId, + operation, + ) +} + export class GeminiHandler extends BaseProvider implements SingleCompletionHandler { protected options: ApiHandlerOptions @@ -296,6 +349,12 @@ export class GeminiHandler extends BaseProvider implements SingleCompletionHandl ? (this.options.modelTemperature ?? info.defaultTemperature ?? 1) : info.defaultTemperature + // Reject cleartext (non-loopback) base URLs before building the request so the + // API key is never sent over an insecure endpoint. + if (this.options.googleGeminiBaseUrl) { + assertSecureGeminiBaseUrl(this.options.googleGeminiBaseUrl, model, "createMessage") + } + const config: GenerateContentConfig = { systemInstruction, httpOptions: this.options.googleGeminiBaseUrl ? { baseUrl: this.options.googleGeminiBaseUrl } : undefined, @@ -590,6 +649,13 @@ export class GeminiHandler extends BaseProvider implements SingleCompletionHandl ? (this.options.modelTemperature ?? info.defaultTemperature ?? 1) : info.defaultTemperature + // Reject cleartext (non-loopback) base URLs before building the request so the + // API key is never sent over an insecure endpoint. + if (this.options.googleGeminiBaseUrl) { + // Stryker disable next-line StringLiteral: completePrompt's catch reads only .message from the thrown error before building its own telemetry error, so this operation argument is unobservable + assertSecureGeminiBaseUrl(this.options.googleGeminiBaseUrl, model, "completePrompt") + } + const promptConfig: GenerateContentConfig = { httpOptions: this.options.googleGeminiBaseUrl ? { baseUrl: this.options.googleGeminiBaseUrl }