Add the support for secret hint #889
Conversation
…s value for all tools.
| /// The default is <see langword="false"/>. | ||
| /// </para> | ||
| /// </remarks> | ||
| public bool Secret { get; init; } = false; |
There was a problem hiding this comment.
Is this coming to mcp? I don't see: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/0cf837e9476c15062034e24858cb2c643cffb7c2/schema/2025-06-18/schema.ts#L838
There was a problem hiding this comment.
There's this: modelcontextprotocol/modelcontextprotocol#711
xiangyan99
requested review from
hallipr,
jongio,
g2vinay,
wbreza,
conniey,
JonathanCrd and
alzimmermsft
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the Secret property to the ToolMetadata class, enabling MCP tools to indicate when they handle or return sensitive information like credentials or API keys.
- Adds a new
Secretboolean property toToolMetadatawith comprehensive documentation - Updates the new command documentation template to include the
Secretproperty - Adds comprehensive unit tests covering the new functionality
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| core/src/AzureMcp.Core/Commands/ToolMetadata.cs | Adds the new Secret property with detailed XML documentation |
| core/tests/AzureMcp.Core.UnitTests/Commands/ToolMetadataTests.cs | Adds comprehensive unit tests for ToolMetadata including the new Secret property |
| docs/new-command.md | Updates the command template to include the Secret property configuration |
| CHANGELOG.md | Documents the new feature addition |
| /// </para> | ||
| /// <para> | ||
| /// This property helps MCP clients understand whether special handling or masking | ||
| /// may be required for the tool's inputs or outputs. |
There was a problem hiding this comment.
This property helps MCP clients understand whether special handling or masking may be required for the tool's inputs or outputs.
Regarding this documentation, my understanding is that we cannot return this flag to the "MCP client", since only flags or properties defined in the ModelContextProtocol.Protocol.ToolAnnotations type (that maps to the spec that Jon linked) will be sent over the channel (stdio or wire) to the "MCP client" by the MCP SDK. If so, we may want to adjust the documentation. My understanding is that such additional flags could help us internally filter the visibility of tools (e.g., service start --disable-secrets-tools).
There was a problem hiding this comment.
You are right. We cannot add the hints into the response yet. Now we use it as the source of truth of the traits of the tools.
There was a problem hiding this comment.
Do you have background info on this change? Like what is the plan, why are we doing this? What are next steps etc? It would be good to include that info in PRs so we know why we are looking it. Thanks
There was a problem hiding this comment.
It's not clear to me why we are adding a hint that isn't supported by the spec. Can you elaborate on the plan please?
There was a problem hiding this comment.
Before these hints are incorporated into the MCP spec, we will rely solely on the code as the source of truth for the tools' traits and may auto-generate documentation from it.
There was a problem hiding this comment.
As discussed with Josh:
We add it for a command line switch and for doc generation.
There was a problem hiding this comment.
@jongio please let me know if you have concerns.
There was a problem hiding this comment.
Are we sure these are going to be added to the spec? It would be great to provide follow up / links to those proposals/discussions so we can link back to it for history sake on this issue.
There was a problem hiding this comment.
Currently, we use this metadata only to filter tools. If modelcontextprotocol/modelcontextprotocol#711 is approved, we may update the tag to "sensitive."
|
Sorry for the inconvenience, but we have moved the Azure MCP Server source code to https://github.com/microsoft/mcp. This change allows us to build any Microsoft MCP server with the same engineering system and allows us to brand any MCP server with either Azure or Microsoft branding. This repo change means that you need to move this PR to that new repo. Please do so and then add a link here to that new PR. We'll close this PR now and we look forward to seeing this over on the new repo. |
github-project-automation
bot
moved this from Untriaged
to Done
in Azure MCP Server (OLD)
What does this PR do?
[Provide a clear, concise description of the changes]Add the support for
secrethint, will have a separate PR to update its value for all tools.Part of #744.
[Any additional context, screenshots, or information that helps reviewers]GitHub issue number?
[Link to the GitHub issue this PR addresses]Pre-merge Checklist
CHANGELOG.mdfor product changes (features, bug fixes, UI/UX, updated dependencies).\eng\common\spelling\Invoke-Cspell.ps1README.mddocumentation/docs/azmcp-commands.md/e2eTests/e2eTestPrompts.mdeng/tools/ToolDescriptionConfidenceScoretool and obtained a result >= 0.4crypto mining, spam, data exfiltration, etc.)/azp run azure - mcpto run Live Test Pipeline