Skip to content

Commit 85215b0

Browse files
daniel-lxsdaniel-lxs
committed
fix: address PR review comments
- Fix security vulnerability in URL validation by using endsWith instead of includes - Add UI notification for detected API version from Base URL - Add validation for Azure API version format - Improve test coverage with edge cases - Exclude Azure AI Inference Service URLs from Azure OpenAI detection
1 parent e829647 commit 85215b0

File tree

3 files changed

+160
-58
lines changed

3 files changed

+160
-58
lines changed

‎src/utils/__tests__/azure-url-parser.test.ts‎

Lines changed: 117 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,124 +1,190 @@
1-
import { describe, it, expect } from 'vitest'
2-
import { extractApiVersionFromUrl, isAzureOpenAiUrl, removeApiVersionFromUrl } from '../azure-url-parser'
1+
import { describe, it, expect } from "vitest"
2+
import {
3+
extractApiVersionFromUrl,
4+
isAzureOpenAiUrl,
5+
removeApiVersionFromUrl,
6+
isValidAzureApiVersion,
7+
} from "../azure-url-parser"
8+
9+
describe("azure-url-parser", () => {
10+
describe("isValidAzureApiVersion", () => {
11+
it("should return true for valid API version format YYYY-MM-DD", () => {
12+
expect(isValidAzureApiVersion("2024-05-01")).toBe(true)
13+
expect(isValidAzureApiVersion("2023-12-31")).toBe(true)
14+
})
15+
16+
it("should return true for valid API version format YYYY-MM-DD-preview", () => {
17+
expect(isValidAzureApiVersion("2024-05-01-preview")).toBe(true)
18+
expect(isValidAzureApiVersion("2024-12-01-preview")).toBe(true)
19+
})
20+
21+
it("should return false for invalid API version formats", () => {
22+
expect(isValidAzureApiVersion("2024-5-1")).toBe(false) // Missing leading zeros
23+
expect(isValidAzureApiVersion("24-05-01")).toBe(false) // Two-digit year
24+
expect(isValidAzureApiVersion("2024/05/01")).toBe(false) // Wrong separator
25+
expect(isValidAzureApiVersion("2024-05-01-alpha")).toBe(false) // Wrong suffix
26+
expect(isValidAzureApiVersion("invalid-version")).toBe(false)
27+
expect(isValidAzureApiVersion("")).toBe(false)
28+
})
29+
})
330

4-
describe('azure-url-parser', () => {
5-
describe('extractApiVersionFromUrl', () => {
6-
it('should extract API version from Azure OpenAI URL', () => {
7-
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview'
31+
describe("extractApiVersionFromUrl", () => {
32+
it("should extract API version from Azure OpenAI URL", () => {
33+
const url =
34+
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview"
835
const result = extractApiVersionFromUrl(url)
9-
expect(result).toBe('2024-05-01-preview')
36+
expect(result).toBe("2024-05-01-preview")
1037
})
1138

12-
it('should extract API version from URL with multiple query parameters', () => {
13-
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&api-version=2024-12-01-preview&baz=qux'
39+
it("should extract API version from URL with multiple query parameters", () => {
40+
const url =
41+
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&api-version=2024-12-01-preview&baz=qux"
1442
const result = extractApiVersionFromUrl(url)
15-
expect(result).toBe('2024-12-01-preview')
43+
expect(result).toBe("2024-12-01-preview")
1644
})
1745

18-
it('should return null when no api-version parameter exists', () => {
19-
const url = 'https://api.openai.com/v1/chat/completions'
46+
it("should return null when no api-version parameter exists", () => {
47+
const url = "https://api.openai.com/v1/chat/completions"
2048
const result = extractApiVersionFromUrl(url)
2149
expect(result).toBeNull()
2250
})
2351

24-
it('should return null for invalid URLs', () => {
25-
const invalidUrl = 'not-a-valid-url'
52+
it("should return null for invalid URLs", () => {
53+
const invalidUrl = "not-a-valid-url"
2654
const result = extractApiVersionFromUrl(invalidUrl)
2755
expect(result).toBeNull()
2856
})
2957

30-
it('should handle empty api-version parameter', () => {
31-
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version='
58+
it("should handle empty api-version parameter", () => {
59+
const url = "https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version="
3260
const result = extractApiVersionFromUrl(url)
33-
expect(result).toBe('')
61+
expect(result).toBe("")
3462
})
3563

36-
it('should handle URL without query parameters', () => {
37-
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions'
64+
it("should handle URL without query parameters", () => {
65+
const url = "https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions"
3866
const result = extractApiVersionFromUrl(url)
3967
expect(result).toBeNull()
4068
})
69+
70+
it("should handle URL with duplicate api-version parameters", () => {
71+
const url =
72+
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01&api-version=2024-12-01"
73+
const result = extractApiVersionFromUrl(url)
74+
// URL.searchParams.get returns the first value
75+
expect(result).toBe("2024-05-01")
76+
})
77+
78+
it("should handle URL with malformed api-version parameter", () => {
79+
const url =
80+
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=invalid-format"
81+
const result = extractApiVersionFromUrl(url)
82+
expect(result).toBe("invalid-format") // Still extracts it, validation is separate
83+
})
4184
})
4285

43-
describe('isAzureOpenAiUrl', () => {
44-
it('should return true for Azure OpenAI URLs with .openai.azure.com', () => {
45-
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions'
86+
describe("isAzureOpenAiUrl", () => {
87+
it("should return true for Azure OpenAI URLs with .openai.azure.com", () => {
88+
const url = "https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions"
4689
const result = isAzureOpenAiUrl(url)
4790
expect(result).toBe(true)
4891
})
4992

50-
it('should return true for Azure URLs ending with .azure.com', () => {
51-
const url = 'https://myservice.azure.com/api/v1'
93+
it("should return true for Azure URLs ending with .azure.com", () => {
94+
const url = "https://myservice.azure.com/api/v1"
5295
const result = isAzureOpenAiUrl(url)
5396
expect(result).toBe(true)
5497
})
5598

56-
it('should return true for URLs with /openai/deployments/ path', () => {
57-
const url = 'https://custom-domain.com/openai/deployments/mymodel/chat/completions'
99+
it("should return true for URLs with /openai/deployments/ path", () => {
100+
const url = "https://custom-domain.com/openai/deployments/mymodel/chat/completions"
58101
const result = isAzureOpenAiUrl(url)
59102
expect(result).toBe(true)
60103
})
61104

62-
it('should return false for regular OpenAI URLs', () => {
63-
const url = 'https://api.openai.com/v1/chat/completions'
105+
it("should return false for regular OpenAI URLs", () => {
106+
const url = "https://api.openai.com/v1/chat/completions"
64107
const result = isAzureOpenAiUrl(url)
65108
expect(result).toBe(false)
66109
})
67110

68-
it('should return false for other API URLs', () => {
69-
const url = 'https://api.anthropic.com/v1/messages'
111+
it("should return false for other API URLs", () => {
112+
const url = "https://api.anthropic.com/v1/messages"
70113
const result = isAzureOpenAiUrl(url)
71114
expect(result).toBe(false)
72115
})
73116

74-
it('should return false for invalid URLs', () => {
75-
const invalidUrl = 'not-a-valid-url'
117+
it("should return false for invalid URLs", () => {
118+
const invalidUrl = "not-a-valid-url"
76119
const result = isAzureOpenAiUrl(invalidUrl)
77120
expect(result).toBe(false)
78121
})
79122

80-
it('should handle case insensitive hostname matching', () => {
81-
const url = 'https://MYRESOURCE.OPENAI.AZURE.COM/openai/deployments/mymodel'
123+
it("should handle case insensitive hostname matching", () => {
124+
const url = "https://MYRESOURCE.OPENAI.AZURE.COM/openai/deployments/mymodel"
82125
const result = isAzureOpenAiUrl(url)
83126
expect(result).toBe(true)
84127
})
128+
129+
it("should return false for malicious URLs trying to include Azure domain", () => {
130+
const maliciousUrl = "https://evil.openai.azure.com.attacker.com/api/v1"
131+
const result = isAzureOpenAiUrl(maliciousUrl)
132+
expect(result).toBe(false)
133+
})
134+
135+
it("should return true for root openai.azure.com domain", () => {
136+
const url = "https://openai.azure.com/api/v1"
137+
const result = isAzureOpenAiUrl(url)
138+
expect(result).toBe(true)
139+
})
140+
141+
it("should return false for Azure AI Inference Service URLs", () => {
142+
const url = "https://myservice.services.ai.azure.com/models/deployments"
143+
const result = isAzureOpenAiUrl(url)
144+
expect(result).toBe(false)
145+
})
85146
})
86147

87-
describe('removeApiVersionFromUrl', () => {
88-
it('should remove api-version parameter from URL', () => {
89-
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview'
148+
describe("removeApiVersionFromUrl", () => {
149+
it("should remove api-version parameter from URL", () => {
150+
const url =
151+
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview"
90152
const result = removeApiVersionFromUrl(url)
91-
expect(result).toBe('https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions')
153+
expect(result).toBe("https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions")
92154
})
93155

94-
it('should remove api-version parameter while preserving other parameters', () => {
95-
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&api-version=2024-05-01-preview&baz=qux'
156+
it("should remove api-version parameter while preserving other parameters", () => {
157+
const url =
158+
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&api-version=2024-05-01-preview&baz=qux"
96159
const result = removeApiVersionFromUrl(url)
97-
expect(result).toBe('https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&baz=qux')
160+
expect(result).toBe(
161+
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&baz=qux",
162+
)
98163
})
99164

100-
it('should return original URL when no api-version parameter exists', () => {
101-
const url = 'https://api.openai.com/v1/chat/completions?foo=bar'
165+
it("should return original URL when no api-version parameter exists", () => {
166+
const url = "https://api.openai.com/v1/chat/completions?foo=bar"
102167
const result = removeApiVersionFromUrl(url)
103168
expect(result).toBe(url)
104169
})
105170

106-
it('should return original URL for invalid URLs', () => {
107-
const invalidUrl = 'not-a-valid-url'
171+
it("should return original URL for invalid URLs", () => {
172+
const invalidUrl = "not-a-valid-url"
108173
const result = removeApiVersionFromUrl(invalidUrl)
109174
expect(result).toBe(invalidUrl)
110175
})
111176

112-
it('should handle URL with only api-version parameter', () => {
113-
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview'
177+
it("should handle URL with only api-version parameter", () => {
178+
const url =
179+
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview"
114180
const result = removeApiVersionFromUrl(url)
115-
expect(result).toBe('https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions')
181+
expect(result).toBe("https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions")
116182
})
117183

118-
it('should handle URL without query parameters', () => {
119-
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions'
184+
it("should handle URL without query parameters", () => {
185+
const url = "https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions"
120186
const result = removeApiVersionFromUrl(url)
121187
expect(result).toBe(url)
122188
})
123189
})
124-
})
190+
})

‎src/utils/azure-url-parser.ts‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,19 @@
22
* Utility functions for parsing Azure OpenAI URLs and extracting API versions
33
*/
44

5+
/**
6+
* Validates if a string is a valid Azure API version format
7+
* @param version The version string to validate
8+
* @returns True if the version follows Azure API version format (YYYY-MM-DD or YYYY-MM-DD-preview)
9+
*/
10+
export function isValidAzureApiVersion(version: string): boolean {
11+
if (!version) return false
12+
13+
// Azure API versions follow the pattern: YYYY-MM-DD or YYYY-MM-DD-preview
14+
const versionPattern = /^\d{4}-\d{2}-\d{2}(-preview)?$/
15+
return versionPattern.test(version)
16+
}
17+
518
/**
619
* Extracts the API version from an Azure OpenAI URL query parameter
720
* @param url The Azure OpenAI URL that may contain an api-version query parameter
@@ -10,7 +23,14 @@
1023
export function extractApiVersionFromUrl(url: string): string | null {
1124
try {
1225
const urlObj = new URL(url)
13-
return urlObj.searchParams.get('api-version')
26+
const apiVersion = urlObj.searchParams.get("api-version")
27+
28+
// Validate the extracted version format
29+
if (apiVersion && !isValidAzureApiVersion(apiVersion)) {
30+
console.warn(`Invalid Azure API version format: ${apiVersion}`)
31+
}
32+
33+
return apiVersion
1434
} catch (error) {
1535
// Invalid URL format
1636
return null
@@ -26,11 +46,20 @@ export function isAzureOpenAiUrl(url: string): boolean {
2646
try {
2747
const urlObj = new URL(url)
2848
const host = urlObj.host.toLowerCase()
29-
49+
50+
// Exclude Azure AI Inference Service URLs
51+
if (host.endsWith(".services.ai.azure.com")) {
52+
return false
53+
}
54+
3055
// Check for Azure OpenAI hostname patterns
31-
return host.includes('.openai.azure.com') ||
32-
host.endsWith('.azure.com') ||
33-
urlObj.pathname.includes('/openai/deployments/')
56+
// Use endsWith to prevent matching malicious URLs like evil.openai.azure.com.attacker.com
57+
return (
58+
host.endsWith(".openai.azure.com") ||
59+
host === "openai.azure.com" ||
60+
host.endsWith(".azure.com") ||
61+
urlObj.pathname.includes("/openai/deployments/")
62+
)
3463
} catch (error) {
3564
return false
3665
}
@@ -44,10 +73,10 @@ export function isAzureOpenAiUrl(url: string): boolean {
4473
export function removeApiVersionFromUrl(url: string): string {
4574
try {
4675
const urlObj = new URL(url)
47-
urlObj.searchParams.delete('api-version')
76+
urlObj.searchParams.delete("api-version")
4877
return urlObj.toString()
4978
} catch (error) {
5079
// Return original URL if parsing fails
5180
return url
5281
}
53-
}
82+
}

‎webview-ui/src/components/settings/providers/OpenAICompatible.tsx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,13 @@ export const OpenAICompatible = ({
202202
}}>
203203
{t("settings:modelInfo.azureApiVersion")}
204204
</Checkbox>
205+
{showApiVersionExtraction && (
206+
<div className="text-sm text-vscode-descriptionForeground ml-6 mb-2">
207+
API version detected in Base URL: <strong>{extractedApiVersion}</strong>
208+
<br />
209+
This will be used automatically. Enable the checkbox above to override.
210+
</div>
211+
)}
205212
{azureApiVersionSelected && (
206213
<VSCodeTextField
207214
value={apiConfiguration?.azureApiVersion || ""}

0 commit comments

Comments
 (0)