Skip to content

Validate foundry endpoint parameters#1658

Open
JasonYeMSFT wants to merge 3 commits intomicrosoft:mainfrom
JasonYeMSFT:chuye/validate-foundry-endpoint-parameters
Open

Validate foundry endpoint parameters#1658
JasonYeMSFT wants to merge 3 commits intomicrosoft:mainfrom
JasonYeMSFT:chuye/validate-foundry-endpoint-parameters

Conversation

@JasonYeMSFT
Copy link
Member

What does this PR do?

Validates foundry tool parmeters that can be a user provided url.

GitHub issue number?

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history is clean with descriptive messages (cleanup guide)
    • Added comprehensive tests for new/modified functionality
    • Updated servers/Azure.Mcp.Server/CHANGELOG.md and/or servers/Fabric.Mcp.Server/CHANGELOG.md for product changes (features, bug fixes, UI/UX, updated dependencies)
  • For MCP tool changes:
    • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
    • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation
    • Validate README.md changes using script at eng/scripts/Process-PackageReadMe.ps1. See Package README
    • Updated command list in /servers/Azure.Mcp.Server/docs/azmcp-commands.md and/or /docs/fabric-commands.md
    • Run .\eng\scripts\Update-AzCommandsMetadata.ps1 to update tool metadata in azmcp-commands.md (required for CI)
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
    • For tools with new names, including new tools or renamed tools, update consolidated-tools.json
    • For new tools associated with Azure services or publicly available tools/APIs/products, add URL to documentation in the PR description
  • Extra steps for Azure MCP Server tool changes:
    • Updated test prompts in /servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
    • 👉 For Community (non-Microsoft team member) PRs:
      • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
      • Manual tests run: added comment /azp run mcp - pullrequest - live to run Live Test Pipeline

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds endpoint validation to the Foundry tool to ensure that user-provided URLs conform to expected Azure service patterns, preventing malicious or malformed endpoints from being used. This is a security enhancement that validates both Foundry project endpoints and Azure OpenAI endpoints before they're used in API calls.

Changes:

  • Added two validation methods (ValidateProjectEndpoint and ValidateAzureOpenAiEndpoint) to verify endpoint URL format, domain suffixes, and resource name constraints
  • Integrated validation calls into all public methods that accept endpoint parameters
  • Created comprehensive unit tests to verify invalid endpoints are rejected

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

File Description
tools/Azure.Mcp.Tools.Foundry/src/Services/FoundryService.cs Adds validation logic for Foundry project and Azure OpenAI endpoints, and integrates validation into all public methods accepting endpoint parameters
tools/Azure.Mcp.Tools.Foundry/tests/Azure.Mcp.Tools.Foundry.UnitTests/FoundryServiceEndpointValidationTests.cs Adds unit tests with extensive invalid endpoint test cases to verify validation rejects malformed and malicious URLs
Comments suppressed due to low confidence (7)

tools/Azure.Mcp.Tools.Foundry/src/Services/FoundryService.cs:193

  • The same error message clarity issue exists in ValidateAzureOpenAiEndpoint as in ValidateProjectEndpoint. The Uri constructor on line 146 can throw UriFormatException, which will be caught and re-wrapped with potentially confusing nested exception messages. Consider handling UriFormatException separately for clearer error messages.
    private static void ValidateAzureOpenAiEndpoint(string endpoint)
    {
        ArgumentException.ThrowIfNullOrWhiteSpace(endpoint, nameof(endpoint));

        try
        {
            var parsedUri = new Uri(endpoint);

            // Example: https://{Azure-OpenAI-resource-name}.openai.azure.com/

            if (parsedUri.Scheme != Uri.UriSchemeHttps)
            {
                throw new ArgumentException("Scheme must be https");
            }

            const string knownSuffix = ".openai.azure.com";
            var host = parsedUri.Host;
            if (!host.EndsWith(knownSuffix))
            {
                throw new ArgumentException("Host must end with Azure OpenAI service suffix");
            }

            var azureOpenAIResourceName = host.Substring(0, host.IndexOf(knownSuffix));

            // Validate Azure OpenAI resource name: 2-64 characters, alphanumeric and hyphens only, cannot start or end with hyphen
            if (azureOpenAIResourceName.Length < 2 || azureOpenAIResourceName.Length > 64)
            {
                throw new ArgumentException("Azure OpenAI resource name must be between 2 and 64 characters");
            }

            if (azureOpenAIResourceName.StartsWith('-') || azureOpenAIResourceName.EndsWith('-'))
            {
                throw new ArgumentException("Azure OpenAI resource name cannot start or end with a hyphen");
            }

            if (!azureOpenAIResourceName.All(c => char.IsLetterOrDigit(c) || c == '-'))
            {
                throw new ArgumentException("Azure OpenAI resource name must contain only alphanumeric characters and hyphens");
            }

            // Validate path: should be empty or just "/" (root path)
            var paths = parsedUri.AbsolutePath.Split('/', StringSplitOptions.RemoveEmptyEntries);
            if (paths.Length != 0)
            {
                throw new ArgumentException("Azure OpenAI endpoint should not contain path segments");
            }
        }
        catch (Exception ex)
        {
            throw new ArgumentException(
            $"Invalid Azure OpenAI endpoint: '{TruncateForLogging(endpoint)}'",
            nameof(endpoint), ex);
        }
    }

tools/Azure.Mcp.Tools.Foundry/src/Services/FoundryService.cs:76

  • The endpoint validation uses case-sensitive string comparison with EndsWith for domain suffixes on lines 76 and 157. While Azure domain names are typically lowercase, using case-insensitive comparison (StringComparison.OrdinalIgnoreCase) would be more robust and align with DNS standards where domain names are case-insensitive. This prevents potential issues if users provide URLs with different casing like "https://my-foundry.SERVICES.AI.AZURE.COM/api/projects/test".
            if (!host.EndsWith(knownSuffix))

tools/Azure.Mcp.Tools.Foundry/src/Services/FoundryService.cs:146

  • The validation uses the Uri constructor which throws UriFormatException for invalid URIs. Consider using Uri.TryCreate() instead for better performance and cleaner error handling, consistent with the pattern used in BaseSpeechCommand.cs:29.
            var parsedUri = new Uri(endpoint);

tools/Azure.Mcp.Tools.Foundry/src/Services/FoundryService.cs:162

  • The same issue exists here as in ValidateProjectEndpoint. The use of host.IndexOf(knownSuffix) on line 162 could theoretically return -1 in edge cases even though EndsWith was checked on line 157. Consider using host.Substring(0, host.Length - knownSuffix.Length) instead for a more robust approach.
            var azureOpenAIResourceName = host.Substring(0, host.IndexOf(knownSuffix));

tools/Azure.Mcp.Tools.Foundry/src/Services/FoundryService.cs:185

  • The validation doesn't check for query strings or fragments in the Azure OpenAI endpoint URL. An endpoint like "https://my-openai.openai.azure.com/?param=value" would pass validation. Consider adding validation to ensure parsedUri.Query and parsedUri.Fragment are empty.
            // Validate path: should be empty or just "/" (root path)
            var paths = parsedUri.AbsolutePath.Split('/', StringSplitOptions.RemoveEmptyEntries);
            if (paths.Length != 0)
            {
                throw new ArgumentException("Azure OpenAI endpoint should not contain path segments");
            }

tools/Azure.Mcp.Tools.Foundry/tests/Azure.Mcp.Tools.Foundry.UnitTests/FoundryServiceEndpointValidationTests.cs:50

    public static IEnumerable<object[]> InvalidProjectEndpoints =>
    [
        ["http://my-foundry.services.ai.azure.com/api/projects/my-project"], // HTTP instead of HTTPS
        ["https://-foundry.services.ai.azure.com/api/projects/my-project"], // Foundry resource name starts with hyphen
        ["https://foundry-.services.ai.azure.com/api/projects/my-project"], // Foundry resource name ends with hyphen
        ["https://a.services.ai.azure.com/api/projects/my-project"], // Single character Foundry resource name
        ["https://my_foundry.services.ai.azure.com/api/projects/my-project"], // Foundry resource name contains underscore
        ["https://my-foundry.services.ai.azure.com/projects/my-project"], // Missing /api/
        ["https://my-foundry.services.ai.azure.com/api/projects/MyProject"], // Project name has uppercase
        ["https://my-foundry.services.ai.azure.com/api/projects/my_project"], // Project name has underscore
        ["https://my-foundry.services.ai.azure.com/api/projects/"], // Missing project name
        ["https://my-foundry.wrongdomain.com/api/projects/my-project"], // Wrong domain
        ["my-foundry.services.ai.azure.com/api/projects/my-project"], // Missing protocol
        ["https://a-very-long-resource-name-that-exceeds-the-maximum-length-limit-of-64-characters.services.ai.azure.com/api/projects/my-project"], // Foundry resource name too long
        ["https://167.128.3.12"], // An arbitrary endpoint
        ["https://evil.com/api/projects/steal-data"], // Malicious domain
        ["https://my-foundry.services.ai.azure.com.evil.com/api/projects/my-project"], // Domain spoofing attempt
    ];

tools/Azure.Mcp.Tools.Foundry/src/Services/FoundryService.cs:135

  • The Uri constructor on line 65 can throw UriFormatException if the endpoint string is not a valid URI format. This exception is being caught and re-wrapped, but the specific inner exception type could be ArgumentException (thrown within the try block) or UriFormatException (from the Uri constructor). This could lead to confusing error messages where "Invalid Foundry project endpoint" is reported with an inner exception saying "Invalid URI: The format of the URI could not be determined". Consider catching UriFormatException separately before the try block for clearer error messages.
    private static void ValidateProjectEndpoint(string endpoint)
    {
        ArgumentException.ThrowIfNullOrWhiteSpace(endpoint, nameof(endpoint));

        try
        {
            var parsedUri = new Uri(endpoint);

            // Example: https://{foundry-resource-name}.services.ai.azure.com/api/projects/{project-name}

            if (parsedUri.Scheme != Uri.UriSchemeHttps)
            {
                throw new ArgumentException("Scheme must be https");
            }

            const string knownSuffix = ".services.ai.azure.com";
            var host = parsedUri.Host;
            if (!host.EndsWith(knownSuffix))
            {
                throw new ArgumentException("Host must end with Foundry service suffix");
            }

            var foundryResourceName = host.Substring(0, host.IndexOf(knownSuffix));

            // Validate foundryResourceName: 2-64 characters, alphanumeric and hyphens only, cannot start or end with hyphen
            if (foundryResourceName.Length < 2 || foundryResourceName.Length > 64)
            {
                throw new ArgumentException("Foundry resource name must be between 2 and 64 characters");
            }

            if (foundryResourceName.StartsWith('-') || foundryResourceName.EndsWith('-'))
            {
                throw new ArgumentException("Foundry resource name cannot start or end with a hyphen");
            }

            if (!foundryResourceName.All(c => char.IsLetterOrDigit(c) || c == '-'))
            {
                throw new ArgumentException("Foundry resource name must contain only alphanumeric characters and hyphens");
            }

            var paths = parsedUri.AbsolutePath.Split("/", StringSplitOptions.RemoveEmptyEntries);
            if (paths.Length != 3)
            {
                throw new ArgumentException("Invalid path structure");
            }

            // Validate path structure: /api/projects/{project-name}
            if (paths[0] != "api")
            {
                throw new ArgumentException("Path must start with '/api'");
            }

            if (paths[1] != "projects")
            {
                throw new ArgumentException("Path must contain '/projects' segment");
            }

            // Validate project name: lowercase letters, numbers, and hyphens only
            var projectName = paths[2];
            if (string.IsNullOrWhiteSpace(projectName))
            {
                throw new ArgumentException("Project name cannot be empty");
            }

            if (!projectName.All(c => char.IsLower(c) || char.IsDigit(c) || c == '-'))
            {
                throw new ArgumentException("Project name must contain only lowercase letters, numbers, and hyphens");
            }

        }
        catch (Exception ex)
        {
            throw new ArgumentException(
            $"Invalid Foundry project endpoint: '{TruncateForLogging(endpoint)}'",
            nameof(endpoint), ex);
        }
    }

Increase test coverage
Use case insensitive comparison for suffix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tools-Foundry Azure AI Foundry

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

2 participants