mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
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
This commit is contained in:
Daniel Riccio
parent
e8296474ae
commit
85215b0fb2
3 changed files with 162 additions and 60 deletions
|
|
@ -1,124 +1,190 @@
|
|||
import { describe, it, expect } from 'vitest'
|
||||
import { extractApiVersionFromUrl, isAzureOpenAiUrl, removeApiVersionFromUrl } from '../azure-url-parser'
|
||||
import { describe, it, expect } from "vitest"
|
||||
import {
|
||||
extractApiVersionFromUrl,
|
||||
isAzureOpenAiUrl,
|
||||
removeApiVersionFromUrl,
|
||||
isValidAzureApiVersion,
|
||||
} from "../azure-url-parser"
|
||||
|
||||
describe('azure-url-parser', () => {
|
||||
describe('extractApiVersionFromUrl', () => {
|
||||
it('should extract API version from Azure OpenAI URL', () => {
|
||||
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview'
|
||||
const result = extractApiVersionFromUrl(url)
|
||||
expect(result).toBe('2024-05-01-preview')
|
||||
describe("azure-url-parser", () => {
|
||||
describe("isValidAzureApiVersion", () => {
|
||||
it("should return true for valid API version format YYYY-MM-DD", () => {
|
||||
expect(isValidAzureApiVersion("2024-05-01")).toBe(true)
|
||||
expect(isValidAzureApiVersion("2023-12-31")).toBe(true)
|
||||
})
|
||||
|
||||
it('should extract API version from URL with multiple query parameters', () => {
|
||||
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&api-version=2024-12-01-preview&baz=qux'
|
||||
const result = extractApiVersionFromUrl(url)
|
||||
expect(result).toBe('2024-12-01-preview')
|
||||
it("should return true for valid API version format YYYY-MM-DD-preview", () => {
|
||||
expect(isValidAzureApiVersion("2024-05-01-preview")).toBe(true)
|
||||
expect(isValidAzureApiVersion("2024-12-01-preview")).toBe(true)
|
||||
})
|
||||
|
||||
it('should return null when no api-version parameter exists', () => {
|
||||
const url = 'https://api.openai.com/v1/chat/completions'
|
||||
it("should return false for invalid API version formats", () => {
|
||||
expect(isValidAzureApiVersion("2024-5-1")).toBe(false) // Missing leading zeros
|
||||
expect(isValidAzureApiVersion("24-05-01")).toBe(false) // Two-digit year
|
||||
expect(isValidAzureApiVersion("2024/05/01")).toBe(false) // Wrong separator
|
||||
expect(isValidAzureApiVersion("2024-05-01-alpha")).toBe(false) // Wrong suffix
|
||||
expect(isValidAzureApiVersion("invalid-version")).toBe(false)
|
||||
expect(isValidAzureApiVersion("")).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("extractApiVersionFromUrl", () => {
|
||||
it("should extract API version from Azure OpenAI URL", () => {
|
||||
const url =
|
||||
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview"
|
||||
const result = extractApiVersionFromUrl(url)
|
||||
expect(result).toBe("2024-05-01-preview")
|
||||
})
|
||||
|
||||
it("should extract API version from URL with multiple query parameters", () => {
|
||||
const url =
|
||||
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&api-version=2024-12-01-preview&baz=qux"
|
||||
const result = extractApiVersionFromUrl(url)
|
||||
expect(result).toBe("2024-12-01-preview")
|
||||
})
|
||||
|
||||
it("should return null when no api-version parameter exists", () => {
|
||||
const url = "https://api.openai.com/v1/chat/completions"
|
||||
const result = extractApiVersionFromUrl(url)
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it('should return null for invalid URLs', () => {
|
||||
const invalidUrl = 'not-a-valid-url'
|
||||
it("should return null for invalid URLs", () => {
|
||||
const invalidUrl = "not-a-valid-url"
|
||||
const result = extractApiVersionFromUrl(invalidUrl)
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it('should handle empty api-version parameter', () => {
|
||||
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version='
|
||||
it("should handle empty api-version parameter", () => {
|
||||
const url = "https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version="
|
||||
const result = extractApiVersionFromUrl(url)
|
||||
expect(result).toBe('')
|
||||
expect(result).toBe("")
|
||||
})
|
||||
|
||||
it('should handle URL without query parameters', () => {
|
||||
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions'
|
||||
it("should handle URL without query parameters", () => {
|
||||
const url = "https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions"
|
||||
const result = extractApiVersionFromUrl(url)
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it("should handle URL with duplicate api-version parameters", () => {
|
||||
const url =
|
||||
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01&api-version=2024-12-01"
|
||||
const result = extractApiVersionFromUrl(url)
|
||||
// URL.searchParams.get returns the first value
|
||||
expect(result).toBe("2024-05-01")
|
||||
})
|
||||
|
||||
it("should handle URL with malformed api-version parameter", () => {
|
||||
const url =
|
||||
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=invalid-format"
|
||||
const result = extractApiVersionFromUrl(url)
|
||||
expect(result).toBe("invalid-format") // Still extracts it, validation is separate
|
||||
})
|
||||
})
|
||||
|
||||
describe('isAzureOpenAiUrl', () => {
|
||||
it('should return true for Azure OpenAI URLs with .openai.azure.com', () => {
|
||||
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions'
|
||||
describe("isAzureOpenAiUrl", () => {
|
||||
it("should return true for Azure OpenAI URLs with .openai.azure.com", () => {
|
||||
const url = "https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions"
|
||||
const result = isAzureOpenAiUrl(url)
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
|
||||
it('should return true for Azure URLs ending with .azure.com', () => {
|
||||
const url = 'https://myservice.azure.com/api/v1'
|
||||
it("should return true for Azure URLs ending with .azure.com", () => {
|
||||
const url = "https://myservice.azure.com/api/v1"
|
||||
const result = isAzureOpenAiUrl(url)
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
|
||||
it('should return true for URLs with /openai/deployments/ path', () => {
|
||||
const url = 'https://custom-domain.com/openai/deployments/mymodel/chat/completions'
|
||||
it("should return true for URLs with /openai/deployments/ path", () => {
|
||||
const url = "https://custom-domain.com/openai/deployments/mymodel/chat/completions"
|
||||
const result = isAzureOpenAiUrl(url)
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
|
||||
it('should return false for regular OpenAI URLs', () => {
|
||||
const url = 'https://api.openai.com/v1/chat/completions'
|
||||
it("should return false for regular OpenAI URLs", () => {
|
||||
const url = "https://api.openai.com/v1/chat/completions"
|
||||
const result = isAzureOpenAiUrl(url)
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
it('should return false for other API URLs', () => {
|
||||
const url = 'https://api.anthropic.com/v1/messages'
|
||||
it("should return false for other API URLs", () => {
|
||||
const url = "https://api.anthropic.com/v1/messages"
|
||||
const result = isAzureOpenAiUrl(url)
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
it('should return false for invalid URLs', () => {
|
||||
const invalidUrl = 'not-a-valid-url'
|
||||
it("should return false for invalid URLs", () => {
|
||||
const invalidUrl = "not-a-valid-url"
|
||||
const result = isAzureOpenAiUrl(invalidUrl)
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
it('should handle case insensitive hostname matching', () => {
|
||||
const url = 'https://MYRESOURCE.OPENAI.AZURE.COM/openai/deployments/mymodel'
|
||||
it("should handle case insensitive hostname matching", () => {
|
||||
const url = "https://MYRESOURCE.OPENAI.AZURE.COM/openai/deployments/mymodel"
|
||||
const result = isAzureOpenAiUrl(url)
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
|
||||
it("should return false for malicious URLs trying to include Azure domain", () => {
|
||||
const maliciousUrl = "https://evil.openai.azure.com.attacker.com/api/v1"
|
||||
const result = isAzureOpenAiUrl(maliciousUrl)
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
it("should return true for root openai.azure.com domain", () => {
|
||||
const url = "https://openai.azure.com/api/v1"
|
||||
const result = isAzureOpenAiUrl(url)
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
|
||||
it("should return false for Azure AI Inference Service URLs", () => {
|
||||
const url = "https://myservice.services.ai.azure.com/models/deployments"
|
||||
const result = isAzureOpenAiUrl(url)
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('removeApiVersionFromUrl', () => {
|
||||
it('should remove api-version parameter from URL', () => {
|
||||
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview'
|
||||
describe("removeApiVersionFromUrl", () => {
|
||||
it("should remove api-version parameter from URL", () => {
|
||||
const url =
|
||||
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview"
|
||||
const result = removeApiVersionFromUrl(url)
|
||||
expect(result).toBe('https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions')
|
||||
expect(result).toBe("https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions")
|
||||
})
|
||||
|
||||
it('should remove api-version parameter while preserving other parameters', () => {
|
||||
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&api-version=2024-05-01-preview&baz=qux'
|
||||
it("should remove api-version parameter while preserving other parameters", () => {
|
||||
const url =
|
||||
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&api-version=2024-05-01-preview&baz=qux"
|
||||
const result = removeApiVersionFromUrl(url)
|
||||
expect(result).toBe('https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&baz=qux')
|
||||
expect(result).toBe(
|
||||
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?foo=bar&baz=qux",
|
||||
)
|
||||
})
|
||||
|
||||
it('should return original URL when no api-version parameter exists', () => {
|
||||
const url = 'https://api.openai.com/v1/chat/completions?foo=bar'
|
||||
it("should return original URL when no api-version parameter exists", () => {
|
||||
const url = "https://api.openai.com/v1/chat/completions?foo=bar"
|
||||
const result = removeApiVersionFromUrl(url)
|
||||
expect(result).toBe(url)
|
||||
})
|
||||
|
||||
it('should return original URL for invalid URLs', () => {
|
||||
const invalidUrl = 'not-a-valid-url'
|
||||
it("should return original URL for invalid URLs", () => {
|
||||
const invalidUrl = "not-a-valid-url"
|
||||
const result = removeApiVersionFromUrl(invalidUrl)
|
||||
expect(result).toBe(invalidUrl)
|
||||
})
|
||||
|
||||
it('should handle URL with only api-version parameter', () => {
|
||||
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview'
|
||||
it("should handle URL with only api-version parameter", () => {
|
||||
const url =
|
||||
"https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions?api-version=2024-05-01-preview"
|
||||
const result = removeApiVersionFromUrl(url)
|
||||
expect(result).toBe('https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions')
|
||||
expect(result).toBe("https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions")
|
||||
})
|
||||
|
||||
it('should handle URL without query parameters', () => {
|
||||
const url = 'https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions'
|
||||
it("should handle URL without query parameters", () => {
|
||||
const url = "https://myresource.openai.azure.com/openai/deployments/mymodel/chat/completions"
|
||||
const result = removeApiVersionFromUrl(url)
|
||||
expect(result).toBe(url)
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -2,6 +2,19 @@
|
|||
* Utility functions for parsing Azure OpenAI URLs and extracting API versions
|
||||
*/
|
||||
|
||||
/**
|
||||
* Validates if a string is a valid Azure API version format
|
||||
* @param version The version string to validate
|
||||
* @returns True if the version follows Azure API version format (YYYY-MM-DD or YYYY-MM-DD-preview)
|
||||
*/
|
||||
export function isValidAzureApiVersion(version: string): boolean {
|
||||
if (!version) return false
|
||||
|
||||
// Azure API versions follow the pattern: YYYY-MM-DD or YYYY-MM-DD-preview
|
||||
const versionPattern = /^\d{4}-\d{2}-\d{2}(-preview)?$/
|
||||
return versionPattern.test(version)
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the API version from an Azure OpenAI URL query parameter
|
||||
* @param url The Azure OpenAI URL that may contain an api-version query parameter
|
||||
|
|
@ -10,7 +23,14 @@
|
|||
export function extractApiVersionFromUrl(url: string): string | null {
|
||||
try {
|
||||
const urlObj = new URL(url)
|
||||
return urlObj.searchParams.get('api-version')
|
||||
const apiVersion = urlObj.searchParams.get("api-version")
|
||||
|
||||
// Validate the extracted version format
|
||||
if (apiVersion && !isValidAzureApiVersion(apiVersion)) {
|
||||
console.warn(`Invalid Azure API version format: ${apiVersion}`)
|
||||
}
|
||||
|
||||
return apiVersion
|
||||
} catch (error) {
|
||||
// Invalid URL format
|
||||
return null
|
||||
|
|
@ -26,11 +46,20 @@ export function isAzureOpenAiUrl(url: string): boolean {
|
|||
try {
|
||||
const urlObj = new URL(url)
|
||||
const host = urlObj.host.toLowerCase()
|
||||
|
||||
|
||||
// Exclude Azure AI Inference Service URLs
|
||||
if (host.endsWith(".services.ai.azure.com")) {
|
||||
return false
|
||||
}
|
||||
|
||||
// Check for Azure OpenAI hostname patterns
|
||||
return host.includes('.openai.azure.com') ||
|
||||
host.endsWith('.azure.com') ||
|
||||
urlObj.pathname.includes('/openai/deployments/')
|
||||
// Use endsWith to prevent matching malicious URLs like evil.openai.azure.com.attacker.com
|
||||
return (
|
||||
host.endsWith(".openai.azure.com") ||
|
||||
host === "openai.azure.com" ||
|
||||
host.endsWith(".azure.com") ||
|
||||
urlObj.pathname.includes("/openai/deployments/")
|
||||
)
|
||||
} catch (error) {
|
||||
return false
|
||||
}
|
||||
|
|
@ -44,10 +73,10 @@ export function isAzureOpenAiUrl(url: string): boolean {
|
|||
export function removeApiVersionFromUrl(url: string): string {
|
||||
try {
|
||||
const urlObj = new URL(url)
|
||||
urlObj.searchParams.delete('api-version')
|
||||
urlObj.searchParams.delete("api-version")
|
||||
return urlObj.toString()
|
||||
} catch (error) {
|
||||
// Return original URL if parsing fails
|
||||
return url
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -202,6 +202,13 @@ export const OpenAICompatible = ({
|
|||
}}>
|
||||
{t("settings:modelInfo.azureApiVersion")}
|
||||
</Checkbox>
|
||||
{showApiVersionExtraction && (
|
||||
<div className="text-sm text-vscode-descriptionForeground ml-6 mb-2">
|
||||
API version detected in Base URL: <strong>{extractedApiVersion}</strong>
|
||||
<br />
|
||||
This will be used automatically. Enable the checkbox above to override.
|
||||
</div>
|
||||
)}
|
||||
{azureApiVersionSelected && (
|
||||
<VSCodeTextField
|
||||
value={apiConfiguration?.azureApiVersion || ""}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue