Add hardcoded minimum TLS version 1.2 to Storage account creation tool#1445
Conversation
alzimmermsft
requested review from
a team,
jongio and
xiangyan99
as code owners
alzimmermsft
requested review from
JasonYeMSFT,
fanyang-mono,
g2vinay,
sandeep-sen and
tmeschter
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #1270 by adding hardcoded minimum TLS version 1.2 to the Storage account creation tool, enabling storage account creation in Azure subscriptions with policy restrictions that require TLS 1.2 or higher.
Key Changes:
- Added
MinimumTlsVersionproperty toStorageAccountPropertiesmodel and set it to "TLS1_2" in the storage account creation logic - Updated test recordings to reflect the modified request payload
- Added changelog entry documenting the change
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Azure.Mcp.Tools.Storage/src/Services/StorageService.cs | Sets MinimumTlsVersion to "TLS1_2" in the storage account creation request |
| tools/Azure.Mcp.Tools.Storage/src/Services/Models/StorageAccountProperties.cs | Adds MinimumTlsVersion property to the model |
| tools/Azure.Mcp.Tools.Storage/src/Services/Models/StorageAccountCreateOrUpdateContent.cs | Reformats XML documentation comments (unrelated style changes) |
| tools/Azure.Mcp.Tools.Storage/tests/Azure.Mcp.Tools.Storage.LiveTests/assets.json | Updates test recording tag to reflect new test recordings with TLS version in payload |
| servers/Azure.Mcp.Server/changelog-entries/1767737468205.yaml | Adds changelog entry for the TLS version update |
| eng/scripts/Compile-Changelog.ps1 | Inadvertently modifies documentation example (appears unintentional) |
|
Isn't it the default value on the service side? |
It's actually TLS 1.0 based on the Bicep documentation Though I'm guessing that'll change to 1.2 in the future when 1.0 and 1.1 are deprecated |
kyleknap
left a comment
kyleknap
left a comment
There was a problem hiding this comment.
🚢 This looks good to me. TLS 1.0 and 1.1 are not secure and resource set up from the MCP server should be representing best practices (which would entail not allowing those insecure protocols). In the future, we can consider exposing the min TLS version as part of the public MCP tool interface, but I think it makes sense holding off until requested for now as we are doing in the PR.
github-project-automation
Bot
moved this from Untriaged
to In Progress
in Azure MCP Server
#1445) * Add hardcoded minimum TLS version 1.2 to Storage account creation tool * Revert accidental cut and paste, rather than copy and paste
microsoft#1445) * Add hardcoded minimum TLS version 1.2 to Storage account creation tool * Revert accidental cut and paste, rather than copy and paste
What does this PR do?
Updates the
AccountCreateCommandin Storage to hardcode in a minimum TLS version 1.2. This is done as TLS 1.0 and 1.1 will soon be deprecated and a minimum of 1.2 should be used to prevent failures in the future.GitHub issue number?
Resolves #1270
Pre-merge Checklist
servers/Azure.Mcp.Server/CHANGELOG.mdand/orservers/Fabric.Mcp.Server/CHANGELOG.mdfor product changes (features, bug fixes, UI/UX, updated dependencies)servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationeng/scripts/Process-PackageReadMe.ps1. See Package README/servers/Azure.Mcp.Server/docs/azmcp-commands.mdand/or/docs/fabric-commands.md.\eng\scripts\Update-AzCommandsMetadata.ps1to update tool metadata in azmcp-commands.md (required for CI)ToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.json/servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline