fix: support custom-host for azure plugin to support privatelink #1117
Conversation
|
DescriptionSummary By MatterAI
🔄 What ChangedThis pull request introduces support for custom hostnames in the Azure plugin, primarily to facilitate integration with Azure Private Link endpoints. The changes involve:
🔍 Impact of the ChangeThis change enables users to connect to Azure AI services via private endpoints (like Private Link), which often use custom hostnames and may not have publicly trusted SSL certificates. The primary impact is enhanced connectivity options for secure, private network deployments. However, the disabling of SSL certificate validation introduces a potential security risk that users must be aware of and manage carefully. 📁 Total Files Changed5 files changed. 🧪 Test AddedN/A - The PR description does not explicitly mention new tests. It is recommended to add unit or integration tests to verify the custom host URL construction and the correct application of the 🔒Security VulnerabilitiesPotential Security Vulnerability Detected: The implementation disables SSL certificate validation ( MotivationSupport custom-host for azure plugin to support privatelink Type of Change
How Has This Been Tested?
Screenshots (if applicable)N/A Checklist
Related Issues. Tip Quality Recommendations
Sequence DiagramsequenceDiagram
actor User
participant PluginHandler as Azure Plugin Handler (contentSafety/pii)
participant Credentials as Azure Credentials
participant AccessTokenService as getAccessToken()
participant HttpsAgent as https.Agent
participant PostFunction as post()
participant AzureAIService as Azure AI Service Endpoint
User->>PluginHandler: Invoke Azure Plugin (parameters, credentials)
PluginHandler->>Credentials: Read customHost and resourceName
alt customHost provided
PluginHandler->>PluginHandler: Construct URL using customHost
PluginHandler->>HttpsAgent: Create new Agent({ rejectUnauthorized: false })
PluginHandler-->>HttpsAgent: Agent instance created
else customHost not provided
PluginHandler->>PluginHandler: Construct URL using resourceName
PluginHandler-->>HttpsAgent: Agent remains null
end
PluginHandler->>AccessTokenService: Request Access Token (credentials)
AccessTokenService-->>PluginHandler: Return token (or error)
PluginHandler->>PostFunction: Call post(url, requestBody, { headers, dispatcher: agent }, timeout)
PostFunction->>HttpsAgent: Use dispatcher (Agent) for request (if provided)
PostFunction->>AzureAIService: Send HTTP POST Request (url, requestBody)
AzureAIService-->>PostFunction: Return API Response
PostFunction-->>PluginHandler: Return Response
PluginHandler-->>User: Return Plugin Verdict and Data
|
![matter-code-review[bot]](https://avatars.githubusercontent.com/in/474549?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds support for custom hosts in Azure plugins, which is valuable for private link scenarios. The implementation is generally good, but I've identified a few security and code quality improvements that should be addressed.
Skipped files
package-lock.json: Skipped file patternpackage.json: Skipped file pattern
plugins/azure/contentSafety.ts
Outdated
| let agent: Agent | null = null; | ||
| // privatelink doesn't contain a valid certificate, skipping verification if it's customHost. | ||
| if (credentials.customHost) { | ||
| agent = new Agent({ | ||
| connect: { | ||
| rejectUnauthorized: false, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🔒 Security Issue Fix
Issue: The code unconditionally disables SSL verification for all custom hosts, which could create security vulnerabilities if used with untrusted endpoints.
Fix: Add a more explicit comment about the security implications and consider making this configurable.
Impact: Improves security awareness and gives users more control over the security tradeoffs.
| let agent: Agent | null = null; | |
| // privatelink doesn't contain a valid certificate, skipping verification if it's customHost. | |
| if (credentials.customHost) { | |
| agent = new Agent({ | |
| connect: { | |
| rejectUnauthorized: false, | |
| }, | |
| }); | |
| } | |
| let agent: Agent | null = null; | |
| // SECURITY NOTE: The following disables SSL certificate validation for custom hosts. | |
| // This is necessary for Azure Private Link endpoints that may use self-signed certificates, | |
| // but should only be used with trusted private endpoints. | |
| if (credentials.customHost) { | |
| agent = new Agent({ | |
| connect: { | |
| rejectUnauthorized: false, | |
| }, | |
| }); | |
| } |
plugins/azure/pii.ts
Outdated
| let agent: Agent | null = null; | ||
| // privatelink doesn't contain a valid certificate, skipping verification if it's customHost. | ||
| if (credentials?.customHost) { | ||
| agent = new Agent({ | ||
| connect: { | ||
| rejectUnauthorized: false, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🔒 Security Issue Fix
Issue: The code unconditionally disables SSL verification for all custom hosts, which could create security vulnerabilities if used with untrusted endpoints.
Fix: Add a more explicit comment about the security implications and consider making this configurable.
Impact: Improves security awareness and gives users more control over the security tradeoffs.
| let agent: Agent | null = null; | |
| // privatelink doesn't contain a valid certificate, skipping verification if it's customHost. | |
| if (credentials?.customHost) { | |
| agent = new Agent({ | |
| connect: { | |
| rejectUnauthorized: false, | |
| }, | |
| }); | |
| } | |
| let agent: Agent | null = null; | |
| // SECURITY NOTE: The following disables SSL certificate validation for custom hosts. | |
| // This is necessary for Azure Private Link endpoints that may use self-signed certificates, | |
| // but should only be used with trusted private endpoints. | |
| if (credentials?.customHost) { | |
| agent = new Agent({ | |
| connect: { | |
| rejectUnauthorized: false, | |
| }, | |
| }); | |
| } |
| @@ -63,7 +64,7 @@ export const handler: PluginHandler<{ | |||
|
|
|||
| const apiVersion = parameters.apiVersion || '2024-11-01'; | |||
|
|
|||
| const url = `https://${credentials.resourceName}.cognitiveservices.azure.com/contentsafety/text:analyze?api-version=${apiVersion}`; | |||
| const url = `${credentials.customHost || `https://${credentials.resourceName}.cognitiveservices.azure.com`}/contentsafety/text:analyze?api-version=${apiVersion}`; | |||
There was a problem hiding this comment.
🛠️ Code Refactor
Issue: The URL construction doesn't validate if the customHost is a valid URL format.
Fix: Add a simple validation to ensure the customHost is properly formatted.
Impact: Prevents runtime errors from malformed URLs.
| const url = `${credentials.customHost || `https://${credentials.resourceName}.cognitiveservices.azure.com`}/contentsafety/text:analyze?api-version=${apiVersion}`; | |
| const baseUrl = credentials.customHost || `https://${credentials.resourceName}.cognitiveservices.azure.com`; | |
| // Ensure the URL doesn't have trailing slashes that could cause path issues | |
| const normalizedBaseUrl = baseUrl.replace(/\\/+$/, ''); | |
| const url = `${normalizedBaseUrl}/contentsafety/text:analyze?api-version=${apiVersion}`; |
| @@ -29,7 +30,7 @@ const redact = async ( | |||
|
|
|||
| const apiVersion = parameters.apiVersion || '2024-11-01'; | |||
|
|
|||
| const url = `https://${credentials?.resourceName}.cognitiveservices.azure.com/language/:analyze-text?api-version=${apiVersion}`; | |||
| const url = `${credentials?.customHost || `https://${credentials?.resourceName}.cognitiveservices.azure.com`}/language/:analyze-text?api-version=${apiVersion}`; | |||
There was a problem hiding this comment.
🛠️ Code Refactor
Issue: The URL construction doesn't validate if the customHost is a valid URL format.
Fix: Add a simple validation to ensure the customHost is properly formatted.
Impact: Prevents runtime errors from malformed URLs.
| const url = `${credentials?.customHost || `https://${credentials?.resourceName}.cognitiveservices.azure.com`}/language/:analyze-text?api-version=${apiVersion}`; | |
| const baseUrl = credentials?.customHost || `https://${credentials?.resourceName}.cognitiveservices.azure.com`; | |
| // Ensure the URL doesn't have trailing slashes that could cause path issues | |
| const normalizedBaseUrl = baseUrl.replace(/\\/+$/, ''); | |
| const url = `${normalizedBaseUrl}/language/:analyze-text?api-version=${apiVersion}`; |
| "customHost": { | ||
| "type": "string", | ||
| "description": "Custom host for Azure AI services (Private Link etc.)" |
There was a problem hiding this comment.
📝 Documentation Improvement
Issue: The description for customHost doesn't mention the security implications of using this feature.
Fix: Enhance the description to include security considerations.
Impact: Improves user awareness of security implications when using custom hosts.
| "customHost": { | |
| "type": "string", | |
| "description": "Custom host for Azure AI services (Private Link etc.)" | |
| \"customHost\": { | |
| \"type\": \"string\", | |
| \"description\": \"Custom host for Azure AI services (Private Link etc.). Note: SSL verification is disabled when using custom hosts.\" |
|
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
b4s36t4
force-pushed
the
fix/azure-plugin-custom-host
branch
from
5f6d0b6 to
83493d9
Compare
|
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use Matter AICommand List
|
| @@ -53,8 +54,24 @@ const redact = async ( | |||
| throw new Error('Unable to get access token'); | |||
| } | |||
|
|
|||
| let agent: Agent | null = null; | |||
There was a problem hiding this comment.
would suggest moving the initialization to a fetchUtils file and creating a method like getInsecureAgent()
Description
CustomHost support for azure plugin to use with azure private-link
Motivation
Type of Change
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Related Issues