Skip to content

Commit 903240d

Browse files
Abinand Nallathambiroomote[bot]
authored andcommitted
fix: add security validation and enhanced error handling for reranker
- Enforce HTTPS for non-localhost URLs to protect API key transmission - Add custom Zod validation for URL scheme security in UI - Include comprehensive tests for URL validation logic - Add missing translation key for HTTPS requirement error - Enhance Chinese translations for validation messages - Fix IPv6 localhost handling for [::1] addresses This commit builds on the CI fixes from PR RooCodeInc#6621 by roomote bot and adds the security enhancements requested in the PR review. Co-authored-by: roomote[bot] <roomote[bot]@users.noreply.github.com>
1 parent 6eab193 commit 903240d

File tree

6 files changed

+133
-61
lines changed

6 files changed

+133
-61
lines changed

src/services/code-index/rerankers/local.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,19 @@ export class LocalReranker extends BaseReranker {
2222
throw new Error("Local reranker requires an API key")
2323
}
2424

25+
// Validate URL scheme for security
26+
const url = new URL(config.url)
27+
const isLocalhost =
28+
url.hostname === "localhost" ||
29+
url.hostname === "127.0.0.1" ||
30+
url.hostname === "[::1]" || // IPv6 localhost with brackets
31+
url.hostname === "::1" // IPv6 localhost without brackets
32+
33+
// Require HTTPS for non-localhost URLs to protect API key transmission
34+
if (!isLocalhost && url.protocol !== "https:") {
35+
throw new Error("Reranker URL must use HTTPS for secure API key transmission (exception: localhost)")
36+
}
37+
2538
this.baseUrl = config.url.replace(/\/$/, "") // Remove trailing slash
2639
this.apiKey = config.apiKey
2740
this.model = config.model

src/tests/services/code-index/rerankers/local.test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,43 @@ describe("LocalReranker", () => {
6262
expect(() => new LocalReranker(invalidConfig)).toThrow("Local reranker requires an API key")
6363
})
6464

65+
it("should throw error when non-localhost URL uses HTTP", () => {
66+
const insecureConfig = { ...mockConfig, url: "http://example.com" }
67+
68+
expect(() => new LocalReranker(insecureConfig)).toThrow(
69+
"Reranker URL must use HTTPS for secure API key transmission (exception: localhost)",
70+
)
71+
})
72+
73+
it("should allow HTTP for localhost URLs", () => {
74+
const mockAxiosCreate = vi.fn().mockReturnValue({})
75+
;(axios.create as any) = mockAxiosCreate
76+
77+
const localhostConfigs = [
78+
{ ...mockConfig, url: "http://localhost:8080" },
79+
{ ...mockConfig, url: "http://127.0.0.1:8080" },
80+
{ ...mockConfig, url: "http://[::1]:8080" },
81+
]
82+
83+
localhostConfigs.forEach((config) => {
84+
expect(() => new LocalReranker(config)).not.toThrow()
85+
})
86+
})
87+
88+
it("should allow HTTPS for any URL", () => {
89+
const mockAxiosCreate = vi.fn().mockReturnValue({})
90+
;(axios.create as any) = mockAxiosCreate
91+
92+
const httpsConfig = { ...mockConfig, url: "https://api.example.com" }
93+
94+
expect(() => new LocalReranker(httpsConfig)).not.toThrow()
95+
expect(mockAxiosCreate).toHaveBeenCalledWith(
96+
expect.objectContaining({
97+
baseURL: "https://api.example.com",
98+
}),
99+
)
100+
})
101+
65102
it("should remove trailing slash from url", () => {
66103
const mockAxiosCreate = vi.fn().mockReturnValue({})
67104
;(axios.create as any) = mockAxiosCreate

webview-ui/src/components/chat/CodeIndexPopover.tsx

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,25 @@ const createValidationSchema = (
110110
codebaseIndexRerankerUrl: z
111111
.string()
112112
.min(1, t("settings:codeIndex.validation.rerankerUrlRequired"))
113-
.url(t("settings:codeIndex.validation.invalidRerankerUrl")),
113+
.url(t("settings:codeIndex.validation.invalidRerankerUrl"))
114+
.refine((url) => {
115+
try {
116+
const parsedUrl = new URL(url)
117+
const isLocalhost =
118+
parsedUrl.hostname === "localhost" ||
119+
parsedUrl.hostname === "127.0.0.1" ||
120+
parsedUrl.hostname === "[::1]" || // IPv6 localhost with brackets
121+
parsedUrl.hostname === "::1" // IPv6 localhost without brackets
122+
123+
// Require HTTPS for non-localhost URLs
124+
if (!isLocalhost && parsedUrl.protocol !== "https:") {
125+
return false
126+
}
127+
return true
128+
} catch {
129+
return false
130+
}
131+
}, t("settings:codeIndex.validation.rerankerUrlMustBeHttps")),
114132
codebaseIndexRerankerModel: z
115133
.string()
116134
.min(1, t("settings:codeIndex.validation.rerankerModelRequired")),
@@ -255,7 +273,8 @@ export const CodeIndexPopover: React.FC<CodeIndexPopoverProps> = ({
255273
codebaseIndexSearchMinScore:
256274
codebaseIndexConfig.codebaseIndexSearchMinScore ?? CODEBASE_INDEX_DEFAULTS.DEFAULT_SEARCH_MIN_SCORE,
257275
codebaseIndexRerankerEnabled: codebaseIndexConfig.codebaseIndexRerankerEnabled ?? false,
258-
codebaseIndexRerankerProvider: codebaseIndexConfig.codebaseIndexRerankerProvider === "local" ? "local" as const : undefined,
276+
codebaseIndexRerankerProvider:
277+
codebaseIndexConfig.codebaseIndexRerankerProvider === "local" ? ("local" as const) : undefined,
259278
codebaseIndexRerankerUrl: codebaseIndexConfig.codebaseIndexRerankerUrl || "",
260279
codebaseIndexRerankerModel: codebaseIndexConfig.codebaseIndexRerankerModel || "ms-marco-MiniLM-L-6-v2",
261280
codebaseIndexRerankerTopN: codebaseIndexConfig.codebaseIndexRerankerTopN ?? 100,

webview-ui/src/i18n/locales/en/settings.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@
126126
"invalidRerankerUrl": "Please enter a valid URL for the reranker service",
127127
"rerankerUrlRequired": "Reranker URL is required when using local provider",
128128
"rerankerApiKeyRequired": "API key is required for this reranker provider",
129-
"rerankerModelRequired": "Reranker model is required"
129+
"rerankerModelRequired": "Reranker model is required",
130+
"rerankerUrlMustBeHttps": "Reranker URL must use HTTPS for secure API key transmission (exception: localhost)"
130131
},
131132
"rerankerConfigLabel": "Search Enhancement (Reranking)",
132133
"newFeatureBadge": "New",

webview-ui/src/i18n/locales/zh-CN/settings.json

Lines changed: 30 additions & 29 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/i18n/locales/zh-TW/settings.json

Lines changed: 30 additions & 29 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)