Browse Source

feat: remove strict ARN validation for Bedrock custom ARN users (#10110)

Co-authored-by: Roo Code <[email protected]>
roomote[bot] 2 weeks ago
parent
commit
ef3c88c47e
2 changed files with 122 additions and 22 deletions
  1. 110 5
      webview-ui/src/utils/__tests__/validate.spec.ts
  2. 12 17
      webview-ui/src/utils/validate.ts

+ 110 - 5
webview-ui/src/utils/__tests__/validate.test.ts → webview-ui/src/utils/__tests__/validate.spec.ts

@@ -2,7 +2,23 @@ import type { ProviderSettings, OrganizationAllowList } from "@roo-code/types"
 
 import { RouterModels } from "@roo/api"
 
-import { getModelValidationError, validateApiConfigurationExcludingModelErrors } from "../validate"
+// Mock i18next to return translation keys with interpolated values
+vi.mock("i18next", () => ({
+	default: {
+		t: (key: string, options?: Record<string, string>) => {
+			if (options) {
+				let result = key
+				Object.entries(options).forEach(([k, v]) => {
+					result += ` ${k}=${v}`
+				})
+				return result
+			}
+			return key
+		},
+	},
+}))
+
+import { getModelValidationError, validateApiConfigurationExcludingModelErrors, validateBedrockArn } from "../validate"
 
 describe("Model Validation Functions", () => {
 	const mockRouterModels: RouterModels = {
@@ -70,7 +86,7 @@ describe("Model Validation Functions", () => {
 			}
 
 			const result = getModelValidationError(config, mockRouterModels, allowAllOrganization)
-			expect(result).toBe("validation.modelAvailability")
+			expect(result).toContain("settings:validation.modelAvailability")
 		})
 
 		it("returns error for model not allowed by organization", () => {
@@ -100,7 +116,7 @@ describe("Model Validation Functions", () => {
 			}
 
 			const result = getModelValidationError(config, mockRouterModels, allowAllOrganization)
-			expect(result).toBe("validation.modelId")
+			expect(result).toBe("settings:validation.modelId")
 		})
 
 		it("handles undefined model IDs gracefully", () => {
@@ -110,7 +126,7 @@ describe("Model Validation Functions", () => {
 			}
 
 			const result = getModelValidationError(config, mockRouterModels, allowAllOrganization)
-			expect(result).toBe("validation.modelId")
+			expect(result).toBe("settings:validation.modelId")
 		})
 	})
 
@@ -134,7 +150,7 @@ describe("Model Validation Functions", () => {
 			}
 
 			const result = validateApiConfigurationExcludingModelErrors(config, mockRouterModels, allowAllOrganization)
-			expect(result).toBe("validation.apiKey")
+			expect(result).toBe("settings:validation.apiKey")
 		})
 
 		it("excludes model-specific errors", () => {
@@ -164,3 +180,92 @@ describe("Model Validation Functions", () => {
 		})
 	})
 })
+
+describe("validateBedrockArn", () => {
+	describe("always returns isValid: true (no strict format validation)", () => {
+		it("accepts standard AWS Bedrock ARNs", () => {
+			const result = validateBedrockArn(
+				"arn:aws:bedrock:us-west-2:123456789012:inference-profile/us.anthropic.claude-3-5-sonnet-v2",
+			)
+			expect(result.isValid).toBe(true)
+			expect(result.arnRegion).toBe("us-west-2")
+			expect(result.errorMessage).toBeUndefined()
+		})
+
+		it("accepts AWS GovCloud ARNs", () => {
+			const result = validateBedrockArn(
+				"arn:aws-us-gov:bedrock:us-gov-west-1:123456789012:inference-profile/model",
+			)
+			expect(result.isValid).toBe(true)
+			expect(result.arnRegion).toBe("us-gov-west-1")
+			expect(result.errorMessage).toBeUndefined()
+		})
+
+		it("accepts AWS China ARNs", () => {
+			const result = validateBedrockArn("arn:aws-cn:bedrock:cn-north-1:123456789012:inference-profile/model")
+			expect(result.isValid).toBe(true)
+			expect(result.arnRegion).toBe("cn-north-1")
+			expect(result.errorMessage).toBeUndefined()
+		})
+
+		it("accepts SageMaker ARNs", () => {
+			const result = validateBedrockArn("arn:aws:sagemaker:us-east-1:123456789012:endpoint/my-endpoint")
+			expect(result.isValid).toBe(true)
+			expect(result.arnRegion).toBe("us-east-1")
+			expect(result.errorMessage).toBeUndefined()
+		})
+
+		it("accepts non-standard ARN formats without validation errors", () => {
+			// Users are advanced - trust their input
+			const result = validateBedrockArn("arn:custom:service:region:account:resource")
+			expect(result.isValid).toBe(true)
+			expect(result.arnRegion).toBe("region")
+			expect(result.errorMessage).toBeUndefined()
+		})
+
+		it("accepts completely custom ARN strings", () => {
+			// Even unusual formats should be accepted
+			const result = validateBedrockArn("some-custom-arn-format")
+			expect(result.isValid).toBe(true)
+			// May not be able to extract region from non-standard format
+			expect(result.errorMessage).toBeUndefined()
+		})
+	})
+
+	describe("region mismatch warnings", () => {
+		it("shows warning when ARN region differs from provided region", () => {
+			const result = validateBedrockArn(
+				"arn:aws:bedrock:us-west-2:123456789012:inference-profile/model",
+				"us-east-1",
+			)
+			expect(result.isValid).toBe(true) // Still valid, just a warning
+			expect(result.arnRegion).toBe("us-west-2")
+			expect(result.errorMessage).toBeDefined()
+			expect(result.errorMessage).toContain("us-west-2")
+		})
+
+		it("shows no warning when ARN region matches provided region", () => {
+			const result = validateBedrockArn(
+				"arn:aws:bedrock:us-west-2:123456789012:inference-profile/model",
+				"us-west-2",
+			)
+			expect(result.isValid).toBe(true)
+			expect(result.arnRegion).toBe("us-west-2")
+			expect(result.errorMessage).toBeUndefined()
+		})
+
+		it("shows no warning when no region is provided to check against", () => {
+			const result = validateBedrockArn("arn:aws:bedrock:us-west-2:123456789012:inference-profile/model")
+			expect(result.isValid).toBe(true)
+			expect(result.arnRegion).toBe("us-west-2")
+			expect(result.errorMessage).toBeUndefined()
+		})
+
+		it("shows no warning when region cannot be extracted from ARN", () => {
+			const result = validateBedrockArn("non-arn-format", "us-east-1")
+			expect(result.isValid).toBe(true)
+			expect(result.arnRegion).toBeUndefined()
+			expect(result.errorMessage).toBeUndefined()
+		})
+	})
+})

+ 12 - 17
webview-ui/src/utils/validate.ts

@@ -216,31 +216,26 @@ function getModelIdForProvider(apiConfiguration: ProviderSettings, provider: Pro
 }
 
 /**
- * Validates an Amazon Bedrock ARN format and optionally checks if the region in
+ * Validates an Amazon Bedrock ARN and optionally checks if the region in
  * the ARN matches the provided region.
  *
+ * Note: This function does not perform strict format validation on the ARN.
+ * Users entering custom ARNs are advanced users who should be trusted to
+ * provide valid ARNs without restriction. See issue #10108.
+ *
  * @param arn The ARN string to validate
  * @param region Optional region to check against the ARN's region
  * @returns An object with validation results: { isValid, arnRegion, errorMessage }
  */
 export function validateBedrockArn(arn: string, region?: string) {
-	// Validate ARN format.
-	const arnRegex = /^arn:aws:(?:bedrock|sagemaker):([^:]+):([^:]*):(?:([^/]+)\/([\w.\-:]+)|([^/]+))$/
-	const match = arn.match(arnRegex)
-
-	if (!match) {
-		return {
-			isValid: false,
-			arnRegion: undefined,
-			errorMessage: i18next.t("settings:validation.arn.invalidFormat"),
-		}
-	}
-
-	// Extract region from ARN.
-	const arnRegion = match[1]
+	// Try to extract region from ARN for region mismatch warning.
+	// This is a permissive regex that attempts to find the region component
+	// without enforcing strict ARN format validation.
+	const regionMatch = arn.match(/^arn:[^:]+:[^:]+:([^:]+):/)
+	const arnRegion = regionMatch?.[1]
 
 	// Check if region in ARN matches provided region (if specified).
-	if (region && arnRegion !== region) {
+	if (region && arnRegion && arnRegion !== region) {
 		return {
 			isValid: true,
 			arnRegion,
@@ -248,7 +243,7 @@ export function validateBedrockArn(arn: string, region?: string) {
 		}
 	}
 
-	// ARN is valid and region matches (or no region was provided to check against).
+	// ARN is always considered valid - trust the user to enter valid ARNs.
 	return { isValid: true, arnRegion, errorMessage: undefined }
 }