Repository navigation
.NET: fix(dotnet): validate MCP invocation headers - #9539
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b7854637-38f5-42cd-8e94-bd46f8c06f01
GitHub Actions
Active
GitHub Actions
Active
github-actions
Bot
changed the title
There was a problem hiding this comment.
🟢 Approval recommended
The focused change reuses established validation, tests both client paths, and has no identified blocking issues.
No open issues 🎉
What changed in this PR
Applies shared HTTP header validation to declarative MCP invocations before client creation or cache lookup, without changing public APIs.
Changes:
- Validates header names and values using the existing shared validator.
- Adds 12 regression cases covering provider-backed and cached-client creation paths.
| File | Description |
|---|---|
dotnet/tests/Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests/DefaultMcpToolHandlerTests.cs |
Tests invalid-header rejection before client creation. |
dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/Microsoft.Agents.AI.Workflows.Declarative.Mcp.csproj |
Enables the shared header validator. |
dotnet/src/Microsoft.Agents.AI.Workflows.Declarative.Mcp/DefaultMcpToolHandler.cs |
Validates headers before selecting a client. |
Review effort: Balanced
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (1 commit(s)): e78be891a694
Model: gpt-5.6-sol
Overview
The PR applies the shared CR/LF/NUL header policy before either MCP client path and wires the shared validator into the package without changing public signatures. The regression theory covers header names and values across provider-backed and cached-client modes and verifies rejection occurs before client creation. Existing direct and approval-resume tests also preserve valid-header forwarding, leaving no publishable Critical, High, or Medium concern.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Code coverage overviewLanguages: C# C# / code-coverage/dotnet
View coverage of the 2 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. |
GitHub Actions
Active
Motivation & Context
Apply consistent HTTP header validation when invoking declarative MCP tools, before creating or selecting a client.
Description & Review Guide
Validation from
dotnet:dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests -f net10.0 --filter-method '*InvokeToolAsync_WithInvalidHeader_ShouldRejectBeforeClientCreationAsync*' --no-ansi: 12 failed before the change for the expected missing-validation behavior; 12 passed afterward.dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests -f net10.0 --no-ansi: passed, 102 tests.dotnet test --project tests\Microsoft.Agents.AI.Workflows.Declarative.UnitTests -f net10.0 --filter-class '*InvokeMcpToolExecutorTest' --no-ansi: passed, 88 tests.dotnet build src\Microsoft.Agents.AI.Workflows.Declarative.Mcp --no-restore --tl:off: passed across all five target frameworks, no warnings or errors.dotnet format src\Microsoft.Agents.AI.Workflows.Declarative.Mcp --no-restore: passed.dotnet format tests\Microsoft.Agents.AI.Workflows.Declarative.Mcp.UnitTests --no-restore: passed.Related Issue
N/A - trivial change: apply the existing shared HTTP header validation consistently to the MCP handler without introducing a new API or validation policy. No matching open PR was found.
AI Assistance
AI assistance details: implementation, regression tests, and PR description.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after a language prefix) — a workflow keeps the label and title prefix in sync automatically.