Skip to content

docs(dotnet): add MCP security documentation and examples#826

Closed
jackbatzner wants to merge 1 commit intomicrosoft:mainfrom
jackbatzner:jb/dotnet-mcp-docs
Closed

docs(dotnet): add MCP security documentation and examples#826
jackbatzner wants to merge 1 commit intomicrosoft:mainfrom
jackbatzner:jb/dotnet-mcp-docs

Conversation

@jackbatzner
Copy link
Copy Markdown
Contributor

Description

Adds comprehensive documentation for .NET MCP security primitives, including API reference docs, OWASP MCP Cheat Sheet compliance mapping, quickstart guides, i18n translations, and usage examples.

Part 3 of 3 — Merge after #824. See also:

Type of Change

  • Documentation update

Package(s) Affected

  • agent-governance-dotnet

Checklist

  • My code follows the project style guidelines
  • I have updated documentation as needed
  • I have signed the Microsoft CLA

Related Issues

Supersedes #775 (split for easier review). Merge after #824.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

Welcome to the Agent Governance Toolkit! Thanks for your first pull request.
Please ensure tests pass, code follows style (ruff check), and you have signed the CLA.
See our Contributing Guide.

@github-actions github-actions bot added documentation Improvements or additions to documentation tests size/XL Extra large PR (500+ lines) labels Apr 6, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

🤖 AI Agent: breaking-change-detector — Summary

🔍 API Compatibility Report

Summary

This pull request primarily introduces documentation updates and examples for .NET MCP security primitives. It does not modify any Python packages or APIs in the microsoft/agent-governance-toolkit repository. No breaking changes were identified in the Python packages published to PyPI.

Findings

Severity Package Change Impact
N/A No changes to Python APIs No impact

Migration Guide

No migration steps are necessary as there are no breaking changes in this pull request.

No breaking changes found.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

🤖 AI Agent: docs-sync-checker — Issues Found

📝 Documentation Sync Report

Issues Found

  • ⚠️ packages/agent-governance-dotnet/README.md — The README for the .NET package should be updated to include the new MCP security primitives, API references, and usage examples added in this PR.
  • ⚠️ CHANGELOG.md — While there is an entry for the new .NET MCP Protocol Support, it does not explicitly mention the addition of the MCP security documentation and examples. This should be clarified.

Suggestions

  • 💡 Update packages/agent-governance-dotnet/README.md to include:
    • Details about the new .NET MCP security primitives (e.g., McpGateway, McpSecurityScanner, McpSessionAuthenticator, etc.).
    • Examples of how to use these new features in .NET.
    • Information about the OWASP MCP Cheat Sheet compliance mapping and quickstart guides.
  • 💡 Add a specific mention in the CHANGELOG.md under the "Added" section about the inclusion of the .NET MCP security documentation and examples.

If these updates are made, the documentation will be in sync with the changes introduced in this PR. Let me know if you need further assistance!

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review of Pull Request: docs(dotnet): add MCP security documentation and examples


🔴 CRITICAL: Security Issues

  1. Missing Details on mTLS Configuration for HTTP Transport

    • The documentation recommends using mTLS for HTTP transport but does not provide detailed guidance or examples for its configuration. This omission could lead to misconfigurations, leaving the system vulnerable to man-in-the-middle (MITM) attacks.
    • Actionable Recommendation: Include a detailed example of how to configure mTLS for MCP servers, including certificate generation, validation, and renewal processes.
  2. Insufficient Details on gVisor/Kata Containers

    • While the documentation mentions using gVisor and Kata Containers for untrusted servers, it does not provide sufficient details on how to configure these sandboxes securely. This could lead to improper implementation and potential sandbox escapes.
    • Actionable Recommendation: Add detailed configuration examples for gVisor and Kata Containers, including specific security settings, and provide guidance on when to use each option.
  3. Lack of Explicit Guidance on Secure Configuration Defaults

    • The documentation does not explicitly state whether the provided .NET SDK McpGateway sanitization and response scanning features are enabled by default. If they are not, users might deploy the SDK without enabling these critical security features.
    • Actionable Recommendation: Clearly document whether these features are enabled by default. If not, provide explicit instructions on how to enable them and emphasize their importance.
  4. Potential for ReDoS in Regular Expressions

    • While the coding conventions mention using matchTimeout: TimeSpan.FromMilliseconds(200) for regex safety, there is no verification mechanism to ensure this is consistently applied.
    • Actionable Recommendation: Add automated tests or static analysis checks to verify that all regular expressions in the codebase adhere to this timeout constraint.

🟡 WARNING: Potential Breaking Changes

  1. Removal of Rust MCP SDK Documentation

    • The removal of references to the Rust MCP SDK (agentmesh-mcp) from the documentation could confuse users who rely on this package.
    • Actionable Recommendation: If the Rust MCP SDK is deprecated, explicitly state this in the documentation and provide guidance for migration or alternatives.
  2. Introduction of .NET 10.0 Target

    • The addition of .NET 10.0 as a target could potentially introduce compatibility issues for users who are currently using .NET 8.0.
    • Actionable Recommendation: Clearly document any breaking changes or additional requirements for users upgrading to .NET 10.0. Ensure backward compatibility with .NET 8.0 is maintained.

💡 Suggestions for Improvement

  1. Expand OWASP MCP Coverage

    • The documentation states that 11 out of 12 OWASP MCP Security Cheat Sheet sections are covered, with §11 (Consent UI) being out of scope. While this is reasonable, consider providing a brief explanation or a link to resources for implementing §11 to guide users who may need it.
  2. Clarify Post-Quantum Cryptography Support

    • The documentation mentions "post-quantum cryptography on .NET 10+," but it is unclear what specific algorithms or libraries are being used. This could lead to confusion or misinterpretation.
    • Actionable Recommendation: Specify the post-quantum cryptographic algorithms used (e.g., ML-DSA-65) and provide references to their security guarantees and compliance with standards like NIST.
  3. Add More Examples for MCP Security Features

    • While the documentation includes examples for some features, it would benefit from additional examples, especially for complex features like McpGateway and McpSecurityScanner.
    • Actionable Recommendation: Include step-by-step examples for setting up and using these features in both ASP.NET Core and gRPC contexts.
  4. Internationalization (i18n)

    • The Japanese translation has been removed from the README, leaving only English and Simplified Chinese. This might alienate Japanese-speaking users.
    • Actionable Recommendation: Either restore the Japanese translation or provide a clear explanation for its removal.
  5. Clarify the Role of AgentGovernance.ModelContextProtocol

    • The new AgentGovernance.ModelContextProtocol sub-package is introduced, but its role and use cases are not clearly explained in the documentation.
    • Actionable Recommendation: Add a dedicated section in the documentation to explain the purpose, features, and usage of this sub-package.
  6. Add Tests for New Features

    • While this PR is primarily documentation-focused, the new .NET MCP Protocol Support features introduced in #824 should be accompanied by comprehensive tests to ensure correctness and prevent regressions.
    • Actionable Recommendation: Verify that the features introduced in #824 are thoroughly tested, especially for edge cases and potential security vulnerabilities.
  7. Clarify the Use of CryptographicOperations.FixedTimeEquals

    • The coding conventions mention using CryptographicOperations.FixedTimeEquals for secret comparison, but there is no verification mechanism to ensure this is consistently applied.
    • Actionable Recommendation: Add automated tests or static analysis checks to verify that all secret comparisons use CryptographicOperations.FixedTimeEquals.
  8. Improve Documentation Formatting

    • There are minor formatting issues in the documentation, such as the missing space in "Runtimegovernance" in the main README.md.
    • Actionable Recommendation: Fix these formatting issues to improve readability and professionalism.

Summary of Recommendations

  • 🔴 Address critical security issues:

    • Provide detailed mTLS configuration examples.
    • Expand guidance on gVisor/Kata Containers.
    • Clarify default settings for .NET SDK McpGateway sanitization and response scanning.
    • Add automated checks for regex safety.
  • 🟡 Mitigate potential breaking changes:

    • Clarify the status of the Rust MCP SDK and provide migration guidance if deprecated.
    • Document any breaking changes or additional requirements for .NET 10.0 compatibility.
  • 💡 Suggestions for improvement:

    • Expand OWASP MCP coverage to include guidance for §11 (Consent UI).
    • Clarify post-quantum cryptography support.
    • Add more examples for MCP security features.
    • Restore or explain the removal of the Japanese translation.
    • Add a dedicated section for AgentGovernance.ModelContextProtocol.
    • Verify comprehensive testing for new .NET MCP Protocol Support features.
    • Add automated checks for CryptographicOperations.FixedTimeEquals.
    • Fix minor formatting issues in the documentation.

This PR provides valuable documentation for the .NET MCP security primitives and aligns well with the project's goals. However, addressing the critical security issues and potential breaking changes is essential before merging.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

🤖 AI Agent: security-scanner — 🔵 **LOW: Potential for Misleading Documentation**

This pull request primarily updates documentation and configuration files, such as .cspell.json, to include additional terms and examples related to .NET MCP (Microsoft Cloud Policy) security primitives. Since this PR does not introduce or modify any functional code, the scope for security vulnerabilities is limited. However, there are still a few areas worth reviewing for potential issues.


🔵 LOW: Potential for Misleading Documentation

Issue

The documentation update introduces new terms and examples but does not provide sufficient context or warnings about potential security pitfalls when using the MCP security primitives. For example, terms like "sandboxing," "SPIFFE," and "SVID" are added to the .cspell.json file, but there is no indication in the PR whether the documentation explains the importance of securely implementing these concepts.

Attack Vector

If the documentation is unclear or incomplete, developers may misuse the MCP security primitives, leading to vulnerabilities such as:

  • Prompt Injection Defense Bypass: Misconfigured sandboxing or trust chain validation could allow malicious input to bypass security policies.
  • Policy Engine Circumvention: Developers might unintentionally weaken policy enforcement due to a lack of understanding of the provided examples.

Recommendation

  1. Ensure that the documentation explicitly highlights common security pitfalls and provides guidance on avoiding them.
  2. Include examples of both correct and incorrect usage of MCP security primitives to educate developers.
  3. Add a section on "Best Practices for Secure Implementation" to the documentation.

🔵 LOW: Potential for Dependency Confusion

Issue

The .cspell.json file includes new terms like "cedarpy," "pydantic," and "setuptools," which are names of Python libraries. While this PR does not directly add any new dependencies, the inclusion of these terms in the spell-check configuration might indicate that these libraries are referenced elsewhere in the project. If these dependencies are not pinned to specific versions, there could be a risk of dependency confusion or typosquatting attacks.

Attack Vector

An attacker could publish a malicious package with a similar name to one of these dependencies. If the project inadvertently installs the malicious package, it could lead to unauthorized code execution or data exfiltration.

Recommendation

  1. Verify that all dependencies, including those referenced in other parts of the project, are pinned to specific, trusted versions.
  2. Use tools like pip-audit or npm audit (depending on the package manager) to identify and address any vulnerabilities in the dependency tree.
  3. Consider implementing a dependency allowlist to prevent unauthorized packages from being installed.

🔵 LOW: Configuration File Hygiene

Issue

The .cspell.json file has been updated with new terms, but some of these terms (e.g., "backdoors," "cmdshell") could be flagged as potentially sensitive or indicative of malicious activity. While these terms may be necessary for the context of the project, their inclusion in a spell-check configuration file could lead to confusion or misinterpretation.

Attack Vector

If these terms are not adequately explained in the documentation, they could be misused or misunderstood by contributors, potentially leading to security oversights.

Recommendation

  1. Add comments in the .cspell.json file to explain why certain sensitive terms (e.g., "backdoors," "cmdshell") are included.
  2. Periodically review the spell-check configuration to ensure that it aligns with the project's security goals and does not inadvertently introduce risks.

Overall Assessment

This PR does not introduce any critical or high-severity security issues, as it primarily focuses on documentation and configuration updates. However, there are opportunities to improve the clarity and security awareness of the documentation to prevent potential misuse of the MCP security primitives.

  • Findings: 3 (all LOW severity)
  • Action Required: Address the recommendations to improve documentation clarity and ensure secure dependency management.

Let me know if you'd like me to review the related PRs (#775 and #824) for a more comprehensive security assessment.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces comprehensive documentation for the .NET MCP (Model Context Protocol) security primitives, including API references, compliance mapping with the OWASP MCP Cheat Sheet, quickstart guides, usage examples, and deployment hardening guidelines. While the PR is primarily documentation-focused, it also includes some minor changes to the .NET solution structure and project files.

Below is the detailed review of the PR, focusing on the specified areas of concern.


🔴 CRITICAL

  1. Missing Cryptographic Algorithm Justification

    • The documentation mentions the use of HMAC-SHA256 for cryptographic signing, with a "migration path for Ed25519 in .NET 9+" and post-quantum cryptography (ML-DSA-65) on .NET 10+. However, there is no clear justification for why HMAC-SHA256 is used instead of Ed25519 or other modern algorithms in the current implementation. This could lead to weaker cryptographic guarantees, especially for scenarios requiring non-repudiation.
    • Action: Provide a detailed security analysis justifying the choice of HMAC-SHA256 over Ed25519 or other algorithms, especially for scenarios involving digital signatures.
  2. Potential Sandbox Escape Vectors

    • The MCP Server Hardening Guide provides excellent recommendations for securing MCP servers, including the use of gVisor/Kata Containers for untrusted servers. However, the guide does not explicitly address sandbox escape vectors, such as:
      • Preventing access to sensitive host resources (e.g., /proc, /sys, or Docker socket).
      • Ensuring that the container runtime itself is not misconfigured or vulnerable.
    • Action: Expand the hardening guide to include specific recommendations for mitigating sandbox escape risks, such as restricting access to sensitive host resources and ensuring the container runtime is hardened.
  3. TOCTOU Race Condition in McpSessionAuthenticator

    • The documentation mentions that the McpSessionAuthenticator is "TOCTOU-safe," but there is no accompanying code or test case to verify this claim. TOCTOU (Time-of-Check to Time-of-Use) vulnerabilities are critical in security-sensitive components like session authentication.
    • Action: Provide a detailed explanation of how the McpSessionAuthenticator mitigates TOCTOU risks. Include unit tests or code snippets that demonstrate and validate this behavior.

🟡 WARNING

  1. Backward Compatibility

    • The .NET SDK now targets both .NET 8.0 and .NET 10.0, with post-quantum cryptography available only on .NET 10+. This change could potentially break compatibility for users who are not yet on .NET 8.0.
    • Action: Clearly document the minimum required .NET version for the new features and provide guidance for users on older versions of .NET.
  2. Removed Rust MCP SDK Documentation

    • The PR removes references to the Rust MCP SDK (agentmesh-mcp) from the README.md and other documentation files. If this is intentional, it should be explicitly mentioned in the PR description to avoid confusion.
    • Action: Confirm whether the removal of Rust MCP SDK documentation is intentional and, if so, provide a clear explanation in the PR description.

💡 SUGGESTIONS

  1. Add Examples for Post-Quantum Cryptography

    • The documentation mentions support for ML-DSA-65 post-quantum signing on .NET 10+, but there are no examples or usage details provided.
    • Suggestion: Add examples demonstrating how to use the post-quantum cryptography features in the .NET SDK.
  2. Improve Documentation for McpGateway

    • The McpGateway is described as a "5-stage pipeline," but the documentation does not provide sufficient detail about each stage (e.g., what "deny-list" and "allow-list" entail, how sanitization works, etc.).
    • Suggestion: Expand the documentation for McpGateway to include detailed descriptions and examples for each stage of the pipeline.
  3. Add Internationalization (i18n) for Japanese

    • The PR removes the Japanese translation link from the main README.md. While this may be intentional, it would be beneficial to include Japanese translations for the new documentation, given the project's existing support for i18n.
    • Suggestion: Add Japanese translations for the new .NET MCP documentation and examples.
  4. Add Tests for MCP Server Hardening

    • While the hardening guide provides excellent recommendations, there are no automated tests or validation scripts to ensure compliance with the guidelines.
    • Suggestion: Add scripts or tests to validate that MCP server deployments comply with the hardening guidelines (e.g., checking for non-root user, read-only filesystem, etc.).
  5. Clarify OWASP MCP Coverage

    • The documentation states that the .NET SDK covers 11 of the 12 OWASP MCP Security Cheat Sheet sections, with §11 (Consent UI) being out of scope. However, it does not specify which section is not covered.
    • Suggestion: Clearly list the covered and uncovered OWASP MCP sections in the documentation.

Additional Notes

  • The PR is well-structured and provides comprehensive documentation for the .NET MCP security primitives. The inclusion of hardening guidelines and OWASP compliance mapping is particularly commendable.
  • The .NET SDK coding conventions and design decisions are well-documented, which will help maintain consistency and security in future contributions.

Summary of Actions

🔴 CRITICAL

  1. Provide justification for the use of HMAC-SHA256 over Ed25519 or other algorithms.
  2. Expand the hardening guide to address sandbox escape vectors.
  3. Verify and demonstrate TOCTOU safety in McpSessionAuthenticator.

🟡 WARNING

  1. Document the minimum required .NET version for new features.
  2. Confirm the intentional removal of Rust MCP SDK documentation.

💡 SUGGESTIONS

  1. Add examples for post-quantum cryptography.
  2. Expand McpGateway documentation with detailed descriptions and examples.
  3. Add Japanese translations for the new documentation.
  4. Add tests or scripts to validate MCP server hardening compliance.
  5. Clarify which OWASP MCP section is not covered.

Let me know if you need further clarification or assistance!

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces comprehensive documentation for the .NET MCP (Model Context Protocol) security primitives, including API references, OWASP compliance mapping, quickstart guides, and usage examples. It also includes a Kubernetes hardening guide for MCP servers and updates to the repository's documentation structure. While the PR is primarily documentation-focused, it also includes some minor code changes and project structure updates.


🔴 CRITICAL

  1. Potential Sandbox Escape in Kubernetes Hardening Guide:

    • The securityContext configuration in the Kubernetes hardening guide does not explicitly set allowPrivilegeEscalation: false for the pod-level security context. While it is correctly set for containers, this omission could allow privilege escalation if a container override is misconfigured.
    • Action: Add allowPrivilegeEscalation: false to the pod-level securityContext in the Kubernetes hardening guide.
  2. Missing Explicit Denial of Cloud Metadata Access:

    • The Kubernetes NetworkPolicy example does not explicitly block access to cloud metadata endpoints (e.g., 169.254.169.254). While the policy states that these are blocked by default, relying on implicit behavior is risky.
    • Action: Add explicit egress rules to deny access to cloud metadata endpoints (e.g., AWS IMDS, Azure IMDS, GCP metadata).

🟡 WARNING

  1. Breaking Changes in Documentation:

    • The removal of Rust MCP SDK references from the README.md and other documentation could be considered a breaking change for users relying on Rust MCP support.
    • Action: Clearly communicate the deprecation/removal of Rust MCP SDK in the changelog and documentation.
  2. Backward Compatibility of .NET SDK Changes:

    • The .NET SDK introduces new components (e.g., McpGateway, McpSecurityScanner, etc.) and modifies existing structures. While these changes are additive, ensure that no existing APIs or behaviors are unintentionally altered.
    • Action: Confirm that all existing .NET SDK APIs remain backward compatible and document any changes explicitly.

💡 SUGGESTIONS

  1. Regex Timeout for ReDoS Prevention:

    • The .NET SDK coding conventions mention a matchTimeout of 200ms for regexes to prevent ReDoS attacks. Ensure that this is consistently applied across all regex usage in the .NET SDK.
    • Action: Audit all regex usage in the .NET SDK to verify compliance with the matchTimeout guideline.
  2. Expand OWASP MCP Compliance:

    • The documentation mentions that 11 out of 12 OWASP MCP Security Cheat Sheet sections are covered. Consider documenting why §11 (Consent UI) is out of scope and if there are plans to address it in the future.
    • Action: Add a roadmap or explanation for the missing OWASP MCP section in the documentation.
  3. Add Threat Model for MCP Components:

    • The .NET MCP Protocol Support introduces several new components (e.g., McpGateway, McpSecurityScanner, etc.). A high-level threat model for these components would help users understand potential risks and mitigations.
    • Action: Include a threat model diagram or section in the documentation for the .NET MCP Protocol Support.
  4. Test Coverage for .NET MCP Protocol Support:

    • While this PR is documentation-focused, it introduces significant new functionality in the .NET SDK. Ensure that comprehensive test coverage exists for all new components, especially for security-critical features like McpSessionAuthenticator and McpMessageSigner.
    • Action: Verify that unit and integration tests cover edge cases, concurrency scenarios, and cryptographic operations.
  5. Internationalization (i18n) Consistency:

    • The Japanese translation (README.ja.md) was removed from the i18n section. Ensure that this is intentional and not an oversight.
    • Action: If the removal is intentional, document the reason in the changelog or PR description.
  6. Clarify .NET SDK Cryptographic Choices:

    • The .NET SDK uses HMAC-SHA256 instead of Ed25519 due to .NET 8 limitations. Consider documenting this decision in the .NET SDK README for transparency.
    • Action: Add a note in the .NET SDK README explaining the cryptographic choices and any future plans for Ed25519 support.

Final Recommendation

  • Address the 🔴 CRITICAL issues before merging to ensure sandbox security and explicit metadata blocking.
  • Review the 🟡 WARNING items for potential breaking changes and backward compatibility concerns.
  • Consider implementing the 💡 SUGGESTIONS to improve documentation clarity, security, and user experience.

Once the critical issues are resolved and the warnings are addressed, this PR can be merged.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This pull request focuses on adding comprehensive documentation for the .NET MCP (Model Context Protocol) security primitives, including API references, compliance mappings, quickstart guides, and usage examples. While this is primarily a documentation update, it introduces critical information about the security model, OWASP compliance, and usage patterns for the .NET MCP package.

Below is a detailed review of the changes, focusing on the specified areas of concern.


🔴 CRITICAL

  1. Lack of Explicit Warnings About Cryptographic Misuse:

    • The documentation mentions cryptographic operations like HMAC-SHA256 and ML-DSA-65 for message integrity and post-quantum signing. However, there is no explicit warning about the risks of improper key management, nonce reuse, or the importance of using secure random number generators. This could lead to insecure implementations by users.
    • Action: Add a dedicated section in the .NET SDK documentation warning users about cryptographic best practices, such as secure key storage, avoiding hardcoding secrets, and ensuring nonces are unique.
  2. Concurrency and TOCTOU Risks in McpSessionAuthenticator:

    • The documentation mentions "TOCTOU-safe concurrency" for McpSessionAuthenticator. However, there is no explanation of how this is achieved or what mechanisms are in place to prevent time-of-check-to-time-of-use (TOCTOU) vulnerabilities.
    • Action: Include a detailed explanation of how TOCTOU safety is ensured in the McpSessionAuthenticator. If this is not implemented, it should be explicitly stated as a limitation.
  3. Sandbox Escape Risks:

    • The .NET SDK documentation does not provide guidance on how to configure the runtime environment to minimize sandbox escape risks. For example, there is no mention of running agents in isolated environments (e.g., containers or VMs).
    • Action: Add a section in the .NET SDK documentation that explicitly recommends running agents in isolated environments and provides best practices for securing the runtime.

🟡 WARNING

  1. Potential Breaking Changes in .NET MCP:

    • The introduction of new classes like McpGateway, McpSecurityScanner, and McpSessionAuthenticator may lead to breaking changes if existing users are required to refactor their code to adopt these new features.
    • Action: Clearly document whether these new classes are optional or mandatory for existing users. If they are mandatory, provide migration guides to help users transition.
  2. Backward Compatibility of .NET SDK:

    • The documentation mentions multi-targeting .NET 8.0 and .NET 10.0. However, it is unclear if this introduces any breaking changes for users on older versions of .NET.
    • Action: Explicitly state whether the new .NET MCP features are backward-compatible with earlier versions of the .NET SDK. If not, provide clear guidance on how users can upgrade.

💡 SUGGESTIONS

  1. OWASP MCP Cheat Sheet Mapping:

    • The documentation claims "11/12 OWASP MCP Security Cheat Sheet coverage." It would be helpful to include a table or matrix explicitly mapping the .NET MCP features to the OWASP MCP Cheat Sheet controls.
    • Action: Add a table in the documentation that maps each .NET MCP feature to the corresponding OWASP MCP control.
  2. Examples for Advanced Scenarios:

    • While the quickstart examples are helpful, they primarily cover basic use cases. Advanced scenarios like multi-agent coordination, custom policy definitions, and integration with external identity providers are not covered.
    • Action: Add advanced usage examples to the .NET SDK documentation, especially for scenarios involving multi-agent coordination and custom policies.
  3. Performance Benchmarks for .NET MCP:

    • The documentation provides performance benchmarks for the Python SDK but not for the .NET MCP implementation.
    • Action: Include performance benchmarks for the .NET MCP implementation, especially for critical components like McpGateway and McpSessionAuthenticator.
  4. i18n Translations:

    • The PR mentions i18n translations but does not provide details about the languages supported or the process for contributing translations.
    • Action: Add a section in the documentation detailing the supported languages and how contributors can add new translations.
  5. Structured Logging Examples:

    • The documentation mentions ILogger<T> for structured logging but does not provide examples.
    • Action: Add examples showing how to configure and use ILogger<T> for logging MCP-related events.
  6. Security Tooling Documentation:

    • The security tooling section in the main README is comprehensive but lacks .NET-specific details.
    • Action: Update the security tooling section to include .NET-specific tools or configurations, such as CodeQL for C# or .NET-specific static analysis tools.
  7. Credential Handling in CredentialRedactor:

    • The CredentialRedactor is mentioned as supporting "10 credential pattern redaction." However, there is no information on how users can extend or customize these patterns.
    • Action: Add documentation on how users can define custom credential patterns for redaction.
  8. OpenTelemetry Integration:

    • The documentation mentions 4 MCP-specific counters for OpenTelemetry but does not provide examples of how to configure or query these metrics.
    • Action: Add a section in the .NET SDK documentation with examples of how to set up and use OpenTelemetry for MCP-specific metrics.

Final Recommendation

  • Merge Readiness: This PR is primarily a documentation update and does not introduce code changes. However, the lack of explicit warnings and detailed explanations for critical security features like cryptographic operations, TOCTOU safety, and sandboxing is concerning.
  • Blocking Issues: Address the 🔴 CRITICAL issues before merging.
  • Post-Merge Enhancements: Consider implementing the 💡 SUGGESTIONS in subsequent updates to improve the usability and security of the .NET MCP documentation.

@jackbatzner
Copy link
Copy Markdown
Contributor Author

Addressing AI Reviewer Findings

"Missing mTLS configuration details" — The documentation intentionally covers mTLS at an architectural guidance level rather than providing implementation-specific cert generation steps, as these vary significantly by deployment environment (Azure, AWS, on-prem). The docs link to Microsoft Learn for platform-specific mTLS configuration.

"gVisor/Kata Containers detail" — Same approach — the docs recommend sandboxing strategies and link to upstream documentation rather than embedding deployment-specific instructions that would go stale.

These are enhancement suggestions for future docs iterations, not blockers.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces comprehensive documentation for the .NET MCP (Model Context Protocol) security primitives, including API references, OWASP MCP Cheat Sheet compliance mapping, quickstart guides, and usage examples. While the PR is documentation-focused, it also includes updates to the .cspell.json file and the CHANGELOG.md.

The documentation is thorough and aligns with the project's goals of providing runtime governance for AI agents. However, there are a few areas that require attention to ensure correctness, clarity, and alignment with the project's focus areas.


🔴 CRITICAL Issues

No critical security issues were identified in this PR, as it primarily involves documentation updates.


🟡 WARNING: Potential Breaking Changes

  1. Backward Compatibility of .NET MCP API:
    • The documentation introduces new .NET MCP primitives such as McpGateway, McpSecurityScanner, McpSessionAuthenticator, etc. While these are new additions, their introduction could potentially lead to breaking changes if existing users of the .NET package expect a different API surface.
    • Action: Ensure that these new APIs are introduced in a backward-compatible manner. If breaking changes are unavoidable, clearly document them in the CHANGELOG.md and provide migration guides.

💡 Suggestions for Improvement

  1. OWASP MCP Cheat Sheet Compliance Mapping:

    • The documentation mentions "11/12 OWASP MCP Security Cheat Sheet coverage." However, it does not specify which item is not covered or why. This could lead to confusion for users.
    • Action: Add a section explicitly listing the OWASP MCP Cheat Sheet items, indicating which ones are covered and providing justifications for any gaps.
  2. Cryptographic Operations:

    • The .NET MCP primitives include cryptographic features like HMAC-SHA256 and ML-DSA-65 post-quantum signing. While these are mentioned in the documentation, there is no reference to how they are implemented or validated.
    • Action: Include a brief explanation or link to documentation about the cryptographic libraries used and their compliance with standards (e.g., NIST, FIPS).
  3. Concurrency and Thread Safety:

    • The documentation mentions TOCTOU-safe concurrency in McpSessionAuthenticator. However, there is no explanation of how thread safety is achieved or tested.
    • Action: Add details about the thread safety mechanisms in place and any testing performed to validate them.
  4. Sandbox Escape Vectors:

    • While the documentation emphasizes execution sandboxing, it does not provide details about how sandbox escape vectors are mitigated.
    • Action: Include a section in the documentation that outlines the measures taken to prevent sandbox escapes and the limitations of the current implementation.
  5. Type Safety and Validation:

    • The .NET MCP primitives seem to rely on configuration files and runtime inputs (e.g., IConfiguration binding). There is no mention of how these inputs are validated or whether type safety is enforced.
    • Action: Clarify how input validation and type safety are handled, especially for critical components like McpGateway and McpSecurityScanner.
  6. Internationalization (i18n):

    • The PR mentions i18n translations but does not specify which languages are supported or how users can contribute translations.
    • Action: Add a section in the documentation listing supported languages and guidelines for contributing translations.
  7. Documentation Structure:

    • The README.md is very long and covers a wide range of topics. This could overwhelm new users.
    • Action: Consider splitting the README.md into smaller, more focused documents (e.g., separate files for installation, quickstart, and advanced topics) and linking them from the main README.
  8. Security Tooling:

    • The documentation lists various security tools (e.g., CodeQL, Gitleaks, ClusterFuzzLite) but does not explain how they are integrated into the CI/CD pipeline.
    • Action: Provide a brief overview or link to a document explaining how these tools are used in the CI/CD pipeline.
  9. Examples and Tutorials:

    • The examples provided are helpful but could benefit from additional context, such as expected outputs or common pitfalls.
    • Action: Enhance the examples with more detailed explanations and include links to the corresponding sections in the documentation.
  10. Regulatory Alignment:

    • The documentation mentions alignment with regulations like the EU AI Act and Colorado AI Act but does not provide detailed mappings.
    • Action: Include a detailed mapping of how the toolkit addresses specific requirements of these regulations.

Additional Notes

  • The .cspell.json update to include "appsettings" is appropriate, given the context of .NET development.
  • The CHANGELOG.md update is detailed and provides a good summary of the new features. However, it could benefit from a clearer distinction between new features and breaking changes.
  • The documentation does a good job of emphasizing OWASP Agentic Top 10 compliance, but it could be more explicit about how each risk is mitigated.

Final Recommendation

  • Merge this PR after addressing the suggestions above, especially those related to backward compatibility and documentation clarity.
  • Conduct a thorough review of the .NET MCP implementation (introduced in #824) to ensure alignment with the project's security and governance goals.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This pull request focuses on adding comprehensive documentation for .NET MCP (Model Context Protocol) security primitives. While the changes are primarily documentation-related, they touch on critical areas such as OWASP MCP compliance, cryptographic operations, and usage examples. Below is a detailed review of the changes, focusing on the specified focus areas.


🔴 CRITICAL

  1. Cryptographic Operations Documentation

    • The documentation mentions the use of HMAC-SHA256 and ML-DSA-65 for message integrity and post-quantum signing. However, there is no detailed explanation of how these cryptographic methods are implemented or used. Without this, users might misuse these primitives, leading to potential vulnerabilities.
      • Actionable Recommendation: Add a detailed section explaining the cryptographic operations, including key management, rotation, and storage best practices. Provide examples of how to securely use these cryptographic primitives in real-world scenarios.
  2. Concurrency and TOCTOU (Time-of-Check to Time-of-Use)

    • The McpSessionAuthenticator is described as providing "TOCTOU-safe concurrency." However, there is no explanation or example of how this is achieved. TOCTOU vulnerabilities are critical and must be explicitly addressed.
      • Actionable Recommendation: Include a detailed explanation of how the McpSessionAuthenticator ensures TOCTOU safety. Provide code examples or pseudocode to demonstrate the implementation.
  3. Sandbox Escape Vectors

    • The documentation mentions "execution sandboxing" and "4-tier privilege rings," but it does not provide sufficient detail on how these mechanisms prevent sandbox escapes.
      • Actionable Recommendation: Add a dedicated section in the documentation that explains the sandboxing architecture, including how it prevents escape vectors. Include examples of common attack scenarios and how the toolkit mitigates them.
  4. OWASP Agentic Top 10 Compliance

    • While the documentation claims full coverage of the OWASP Agentic Top 10, there is no evidence or test cases provided to substantiate these claims.
      • Actionable Recommendation: Link to specific test cases or evidence that demonstrate compliance with each OWASP Agentic Top 10 risk. This will help users trust the claims and understand how to implement the controls.

🟡 WARNING

  1. Backward Compatibility
    • The documentation introduces new APIs and features for .NET MCP security primitives. While this is a documentation update, it indirectly implies changes to the public API.
      • Actionable Recommendation: Clearly document any breaking changes or new requirements introduced by the .NET MCP package. If there are no breaking changes, explicitly state this in the changelog.

💡 SUGGESTIONS

  1. Type Safety and Pydantic Model Validation

    • While this PR is focused on .NET, the Python examples in the README could benefit from explicit type annotations and Pydantic model validation to ensure type safety.
      • Actionable Recommendation: Update Python examples to include type annotations and Pydantic validation where applicable.
  2. i18n Translations

    • The documentation mentions i18n translations but does not provide details on how these are managed or updated.
      • Actionable Recommendation: Include a section explaining the i18n process, including how translations are maintained and updated.
  3. Quickstart Examples

    • The quickstart examples are helpful but could benefit from additional context, such as common pitfalls or best practices.
      • Actionable Recommendation: Add a "Common Pitfalls" or "Best Practices" section to the quickstart guides.
  4. Regulatory Alignment

    • The documentation mentions alignment with regulations like the EU AI Act and Colorado AI Act but does not provide detailed mappings.
      • Actionable Recommendation: Include detailed mappings for each regulation, similar to the OWASP compliance mapping.
  5. Performance Benchmarks

    • The performance benchmarks are impressive but lack details on the testing environment and methodology.
      • Actionable Recommendation: Add a section detailing the testing environment, methodology, and any assumptions made during benchmarking.

Final Notes

This PR is a significant step forward in documenting .NET MCP security primitives and aligning them with OWASP MCP Cheat Sheet compliance. However, the lack of detailed explanations for cryptographic operations, concurrency safety, and sandboxing mechanisms is a critical gap that must be addressed before merging.

  • Priority Actions: Address the 🔴 CRITICAL issues related to cryptographic operations, TOCTOU safety, sandboxing, and OWASP compliance evidence.
  • Secondary Actions: Resolve the 🟡 WARNING on backward compatibility and implement the 💡 SUGGESTIONS to improve usability and clarity.

Let me know if you need further clarification or assistance!

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Feedback on Pull Request: docs(dotnet): add MCP security documentation and examples


🔴 CRITICAL: Security Concerns

  1. HMAC-SHA256 and ML-DSA-65 Signing:

    • The documentation mentions the use of HMAC-SHA256 for message integrity and ML-DSA-65 for post-quantum signing. Ensure that the cryptographic primitives used are up-to-date with current standards and are implemented securely. Specifically:
      • Verify that the key management and storage for HMAC keys are secure and follow best practices (e.g., hardware-backed storage or secure vaults).
      • Confirm that ML-DSA-65 is a vetted post-quantum algorithm. If it is experimental or proprietary, document its limitations and fallback mechanisms.
  2. Concurrency in McpSessionAuthenticator:

    • The documentation mentions "TOCTOU-safe concurrency." Ensure that the implementation of session binding and TTL does not introduce race conditions or time-of-check-to-time-of-use vulnerabilities. Specifically:
      • Validate that session binding operations are atomic and thread-safe.
      • Confirm that the TTL mechanism cannot be bypassed or exploited by malicious actors.
  3. gRPC Server Interceptor:

    • The gRPC server interceptor is mentioned as supporting "all 4 handler types." Ensure that the interceptor properly validates and sanitizes inputs to prevent injection attacks or privilege escalation. Specifically:
      • Verify that the interceptor enforces strict input validation and sanitization for all handler types.
      • Confirm that the interceptor does not inadvertently expose sensitive internal state or metadata.
  4. Credential Redactor:

    • The CredentialRedactor claims to redact 10 credential patterns. Ensure that the redaction mechanism is robust and cannot be bypassed. Specifically:
      • Validate that the redactor handles edge cases, such as credentials embedded in non-standard formats or obfuscated strings.
      • Confirm that the redactor does not inadvertently redact legitimate data or fail to redact sensitive information.

🟡 WARNING: Potential Breaking Changes

  1. Backward Compatibility of .NET MCP Protocol Support:

    • The addition of .NET MCP Protocol Support introduces several new APIs and features. Ensure that these changes do not break existing integrations or workflows. Specifically:
      • Validate that the new APIs are backward-compatible with previous versions of the .NET package.
      • Confirm that existing users can upgrade without requiring major code changes.
  2. OWASP MCP Cheat Sheet Compliance Mapping:

    • The documentation mentions "11/12 OWASP MCP Security Cheat Sheet coverage." Ensure that the compliance mapping does not introduce breaking changes to existing policy configurations or enforcement mechanisms.

💡 Suggestions for Improvement

  1. Documentation Clarity:

    • The documentation is comprehensive but dense. Consider breaking it into smaller, focused sections with clear headings and subheadings. For example:
      • Separate the API reference docs from the quickstart guides and usage examples.
      • Provide a summary table for .NET MCP Protocol Support features.
  2. Internationalization (i18n):

    • The documentation mentions i18n translations. Ensure that the translations are accurate and culturally appropriate. Consider adding a language switcher to the documentation site for easier navigation.
  3. OWASP MCP Cheat Sheet Compliance:

    • The documentation mentions "11/12 OWASP MCP Security Cheat Sheet coverage." Consider explicitly listing the covered items and providing links to the implementation details for each.
  4. Performance Benchmarks:

    • The documentation mentions governance latency of < 0.1 ms per action. Consider providing detailed benchmarks for .NET MCP Protocol Support to demonstrate its performance under different workloads.
  5. Sample Apps:

    • The documentation mentions two sample apps (ASP.NET Core full-stack and Official MCP SDK integration). Consider providing links to these apps and including screenshots or diagrams to illustrate their functionality.

Summary

This pull request adds valuable documentation for .NET MCP Protocol Support and related security primitives. However, it introduces several critical security concerns and potential breaking changes that must be addressed before merging. Additionally, there are opportunities to improve the documentation's clarity, organization, and usability.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Pull Request Review: docs(dotnet): add MCP security documentation and examples

Summary

This pull request adds comprehensive documentation for .NET MCP security primitives, including API reference docs, OWASP MCP Cheat Sheet compliance mapping, quickstart guides, i18n translations, and usage examples. It is part of a multi-step integration process and should be merged after #824.


🔴 CRITICAL: Security Concerns

  1. Sensitive Keywords in .cspell.json
    The addition of sensitive keywords like accesskey, clientsecret, authtoken, apikey, bearertoken, and secretpassword to .cspell.json is concerning. While these entries may be intended for spell-checking purposes, they could inadvertently expose sensitive information if not handled properly. Ensure that these keywords are not hardcoded or logged anywhere in the codebase. Consider implementing automated secret scanning tools like GitHub's secret scanning or Gitleaks to detect accidental leaks.

  2. HMAC-SHA256 and ML-DSA-65
    The documentation mentions the use of HMAC-SHA256 for message integrity and ML-DSA-65 for post-quantum signing. While these algorithms are robust, ensure that:

    • The implementation adheres to the latest cryptographic standards.
    • Key management practices are secure (e.g., keys are rotated periodically, stored securely, and not hardcoded).
    • The cryptographic library used is up-to-date and free from known vulnerabilities.
  3. TOCTOU-Safe Concurrency
    The McpSessionAuthenticator claims to provide TOCTOU-safe concurrency. Ensure that the implementation properly mitigates race conditions, especially when handling cryptographic operations or sensitive data. Consider adding tests to simulate concurrent access and verify the absence of race conditions.

  4. Sandbox Escape Vectors
    The .NET MCP Protocol Support documentation mentions execution sandboxing and privilege rings. Ensure that the sandbox implementation is robust and does not allow for escape vectors such as:

    • Arbitrary code execution via deserialization vulnerabilities.
    • Exploitation of OS-level APIs or system calls.
    • Insecure handling of subprocesses (e.g., cmdshell, popen).
  5. Credential Redaction
    The CredentialRedactor is described as supporting 10 credential pattern redactions. Ensure that this functionality is thoroughly tested against edge cases (e.g., partial matches, encoded credentials) to prevent accidental leakage of sensitive information.


🟡 WARNING: Potential Breaking Changes

  1. Backward Compatibility
    The addition of new APIs and features (e.g., McpGateway, McpSecurityScanner, McpSessionAuthenticator) may introduce breaking changes for existing users of the .NET SDK. Ensure that:

    • The new APIs are backward-compatible with previous versions.
    • Deprecation notices are provided for any removed or modified functionality.
    • Comprehensive migration guides are included in the documentation.
  2. Multi-Targeting .NET Versions
    The documentation mentions multi-targeting .NET 8.0 and .NET 10.0. Ensure that the SDK is compatible with all supported versions and does not introduce runtime issues for users on older versions.


💡 Suggestions for Improvement

  1. OWASP MCP Cheat Sheet Compliance
    The documentation highlights 11/12 OWASP MCP Security Cheat Sheet coverage. Consider adding detailed examples or test cases for the remaining category to achieve full compliance.

  2. Performance Benchmarks
    While the documentation mentions sub-millisecond latency for policy enforcement, consider providing detailed benchmarks specific to the .NET implementation. This will help users understand the performance impact of integrating MCP governance.

  3. i18n Translations
    The pull request includes i18n translations for the documentation. Ensure that these translations are accurate and culturally appropriate. Consider adding a process for community contributions to improve translations over time.

  4. Sample Applications
    The inclusion of two sample apps (ASP.NET Core full-stack and Official MCP SDK integration) is a great addition. Consider expanding these samples to include more complex scenarios, such as multi-agent interactions or integration with third-party frameworks.

  5. OpenTelemetry Integration
    The documentation mentions MCP-specific counters for OpenTelemetry. Consider providing detailed examples or tutorials on how to set up and visualize these metrics in popular observability platforms like Grafana or Azure Monitor.


Action Items

  1. Security Audit
    Conduct a thorough security audit of the .NET MCP Protocol Support implementation, focusing on cryptographic operations, sandboxing, and concurrency.

  2. Backward Compatibility Testing
    Perform regression testing to ensure that the new features do not introduce breaking changes for existing users.

  3. Documentation Enhancements
    Add detailed migration guides, performance benchmarks, and complex sample applications to the documentation.

  4. Automated Secret Scanning
    Integrate tools like Gitleaks or GitHub's secret scanning into the CI/CD pipeline to detect and prevent accidental leaks of sensitive information.


Final Recommendation

This pull request introduces valuable documentation and examples for the .NET MCP Protocol Support. However, the critical security concerns and potential breaking changes must be addressed before merging. Proceed with caution and prioritize the outlined action items.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Pull Request Review: docs(dotnet): add MCP security documentation and examples


🔴 CRITICAL: Security Concerns

  1. Credential Patterns in .cspell.json

    • The addition of sensitive credential patterns like accesskey, apikey, authtoken, clientsecret, secretkey, etc., in .cspell.json could inadvertently expose these patterns to attackers if the file is misused or leaked. While this is intended for spell-checking, it creates a risk of credential enumeration.
    • Actionable Recommendation: Remove sensitive credential patterns from .cspell.json. Instead, use a dedicated secret scanning tool (e.g., Gitleaks or TruffleHog) integrated into CI/CD workflows.
  2. MCP Security Scanner

    • The documentation mentions McpSecurityScanner detecting threats like tool poisoning, typosquatting, and protocol attacks. However, no details are provided about how these detections are implemented or tested.
    • Actionable Recommendation: Ensure that the MCP Security Scanner has robust unit tests and integration tests for all listed threat vectors. Provide documentation on its detection mechanisms and limitations.

🟡 WARNING: Potential Breaking Changes

  1. Backward Compatibility of `.NET MCP Protocol Support
    • The addition of .NET MCP Protocol Support introduces new APIs and features, such as McpGateway, McpSecurityScanner, McpSessionAuthenticator, etc. While these are valuable, they may introduce breaking changes for existing users of the .NET package.
    • Actionable Recommendation: Clearly document all breaking changes in the CHANGELOG.md and provide migration guides for users upgrading from previous versions.

💡 Suggestions for Improvement

  1. OWASP MCP Cheat Sheet Compliance Mapping

    • The documentation claims 11/12 OWASP MCP Security Cheat Sheet coverage but does not specify which item is missing or why. This could lead to confusion or misinterpretation of the toolkit's security capabilities.
    • Actionable Recommendation: Add a detailed table or section in the documentation that explicitly maps each OWASP MCP Cheat Sheet item to the corresponding toolkit feature, including the missing item and rationale.
  2. Concurrency and Thread Safety

    • The .NET MCP Protocol Support documentation mentions features like McpSlidingRateLimiter and McpSessionAuthenticator. These components likely involve concurrent execution. However, there is no mention of thread safety guarantees or testing.
    • Actionable Recommendation: Add documentation and tests to verify thread safety for concurrent agent execution, especially for rate limiting and session management.
  3. Sandbox Escape Vectors

    • The .NET MCP Protocol Support introduces execution sandboxing features. However, the documentation does not address potential sandbox escape vectors or mitigation strategies.
    • Actionable Recommendation: Include a dedicated section in the documentation on sandbox security, detailing how escape vectors are mitigated (e.g., syscall filtering, seccomp profiles).
  4. Type Safety and Validation

    • The .NET MCP Protocol Support documentation does not specify whether the new APIs use strong type validation (e.g., Pydantic models or equivalent in .NET). This is critical for preventing type-related vulnerabilities.
    • Actionable Recommendation: Ensure all public APIs in the .NET package enforce strict type validation and document the validation rules.
  5. Internationalization (i18n)

    • The PR mentions i18n translations but does not provide details on the languages supported or how users can contribute translations.
    • Actionable Recommendation: Add a section in the documentation explaining the supported languages, how translations are managed, and how users can contribute.

Summary of Actions

🔴 CRITICAL

  • Remove sensitive credential patterns from .cspell.json.
  • Ensure robust testing and documentation for MCP Security Scanner.

🟡 WARNING

  • Document breaking changes in .NET MCP Protocol Support and provide migration guides.

💡 SUGGESTIONS

  • Add OWASP MCP Cheat Sheet mapping details.
  • Verify thread safety in concurrent execution features.
  • Document sandbox escape mitigation strategies.
  • Enforce and document type validation for .NET APIs.
  • Expand i18n documentation for translations.

Final Recommendation

This PR introduces valuable documentation and examples for .NET MCP security primitives. However, addressing the flagged security concerns and potential breaking changes is critical before merging. Additionally, implementing the suggested improvements will enhance the toolkit's usability and security posture.

@github-actions github-actions bot removed the tests label Apr 6, 2026
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Feedback for PR: docs(dotnet): add MCP security documentation and examples


🔴 CRITICAL: Security Issues

  1. Replay Protection Window Configuration

    • The ReplayWindow in McpMessageSigner is set to TimeSpan.FromMinutes(5) in the example. While this is reasonable for many use cases, it may be insufficient for high-security environments where shorter replay windows are required. Ensure that the default configuration or documentation explicitly warns users about the trade-offs of longer replay windows, especially in scenarios involving sensitive data or high-value transactions.

    Action: Add a note in the documentation emphasizing the importance of configuring the ReplayWindow based on the specific security requirements of the deployment.

  2. Potential SSRF Vulnerability

    • The McpSecurityScanner documentation mentions scanning for "protocol attacks" but does not explicitly address SSRF (Server-Side Request Forgery) vulnerabilities. MCP servers using HTTP transport should explicitly validate and sanitize URLs to prevent SSRF attacks.

    Action: Update the documentation to include explicit guidance on mitigating SSRF risks, such as validating URLs and restricting outbound requests to trusted domains.

  3. Credential Redaction Scope

    • The CredentialRedactor documentation mentions redacting sensitive information such as API keys and connection strings. However, it does not specify whether it supports recursive redaction for nested data structures (e.g., JSON objects with embedded secrets).

    Action: Ensure the CredentialRedactor implementation supports recursive redaction for nested structures and update the documentation accordingly.


🟡 WARNING: Potential Breaking Changes

  1. Post-Quantum Cryptography Support

    • The introduction of ML-DSA-65 for post-quantum cryptography in .NET 10+ is a significant addition. While this is future-proofing, it introduces a potential breaking change for users who upgrade to .NET 10 and expect backward compatibility with older cryptographic algorithms.

    Action: Clearly document the implications of using ML-DSA-65, including compatibility considerations with systems that rely on classical cryptographic algorithms (e.g., ECDSA/RSA). Provide migration guidance for users transitioning to .NET 10.

  2. Behavior vs Behaviour

    • The documentation inconsistently uses "behavior" and "behaviour." While this is not a functional issue, it may confuse users, especially in international contexts.

    Action: Standardize the spelling across the documentation. Consider aligning with the primary audience's preferred spelling (e.g., "behavior" for US English).


💡 Suggestions for Improvement

  1. Expand OWASP MCP Coverage

    • The documentation mentions 11/12 OWASP MCP Cheat Sheet coverage but does not specify which section is missing. This could lead to confusion or concern among users.

    Action: Explicitly list the covered sections and clarify which section is not covered, along with a rationale or roadmap for addressing it.

  2. Kubernetes Hardening Guide

    • The Kubernetes hardening guide is comprehensive but could benefit from additional details on monitoring and auditing. For example, integrating tools like Falco or Kubernetes Audit Logs to detect policy violations or anomalous behavior.

    Action: Add a section on monitoring and auditing MCP server pods, including recommended tools and configurations.

  3. Credential Redaction Examples

    • The examples for CredentialRedactor are helpful but could be expanded to include real-world scenarios, such as redacting secrets from log files or HTTP request payloads.

    Action: Add more detailed examples demonstrating the use of CredentialRedactor in common scenarios.

  4. Session Authentication

    • The session authentication example is clear but could benefit from additional details on session expiration and renewal strategies.

    Action: Include a subsection on best practices for session management, such as token renewal and invalidation.

  5. Documentation Structure

    • The documentation is dense and may be overwhelming for new users. Consider breaking it into smaller, focused sections or adding a table of contents for easier navigation.

    Action: Reorganize the documentation into modular sections with clear headings and a table of contents.


General Observations

  • The addition of MCP security documentation and examples significantly enhances the usability and security of the .NET MCP Protocol Support. The examples are well-structured and provide clear guidance for developers.
  • The alignment with the OWASP MCP Security Cheat Sheet is commendable and demonstrates a strong commitment to security best practices.
  • The inclusion of i18n translations is a thoughtful touch that improves accessibility for non-English-speaking users.

Summary of Actions

🔴 CRITICAL

  • Add warnings about replay window configuration.
  • Address SSRF mitigation in MCP servers.
  • Ensure recursive credential redaction support.

🟡 WARNING

  • Document implications of post-quantum cryptography.
  • Standardize spelling of "behavior" vs "behaviour."

💡 SUGGESTION

  • Expand OWASP MCP coverage details.
  • Add monitoring/auditing guidance for Kubernetes.
  • Provide more detailed credential redaction examples.
  • Include session management best practices.
  • Reorganize documentation for better readability.

This PR is well-prepared but requires minor adjustments to address critical security concerns and improve usability.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces comprehensive documentation for .NET MCP security primitives, including API references, OWASP MCP Cheat Sheet compliance mapping, quickstart guides, i18n translations, and usage examples. While the PR is documentation-focused, it also includes critical security-related content and implementation details for the MCP governance layer. Below are the findings and recommendations:


🔴 CRITICAL

  1. Replay Protection in McpMessageSigner:

    • The McpMessageSigner implementation mentions nonce-based replay rejection but does not provide details on how nonces are managed or stored. If nonces are not stored securely or are reused, this could lead to replay attacks.
    • Action: Ensure that nonces are stored securely and are unique for each message. Consider using a database or in-memory store with TTL for nonce tracking.
  2. Session Authentication (McpSessionAuthenticator):

    • The ValidateRequest method does not specify how session tokens are validated. If tokens are not cryptographically signed or if the validation mechanism is weak, it could lead to session hijacking or impersonation.
    • Action: Ensure that session tokens are cryptographically signed and include a mechanism to verify their integrity and authenticity. Use a secure algorithm like HMAC-SHA256 or ML-DSA for signing.
  3. Potential Sandbox Escape in gVisor/Kata Containers:

    • The documentation suggests using gVisor or Kata Containers for untrusted MCP servers but does not specify how to handle potential sandbox escape vectors (e.g., syscall filtering, file system access).
    • Action: Include explicit recommendations for syscall filtering and file system access restrictions to mitigate sandbox escape risks. For example, ensure that sensitive directories like /proc and /sys are inaccessible.
  4. Credential Leakage in McpResponseScanner:

    • The McpResponseScanner mentions detecting credential leakage but does not specify the patterns or methods used for detection. If the patterns are incomplete or not updated regularly, sensitive data could be leaked.
    • Action: Provide a detailed list of credential patterns being scanned and ensure regular updates to include new patterns (e.g., cloud provider keys, JWTs).

🟡 WARNING

  1. Breaking Changes in .NET Target Versions:

    • The documentation mentions multi-targeting .NET 8.0 and .NET 10.0, with post-quantum cryptography support on .NET 10+. This could introduce breaking changes for users on older .NET versions.
    • Action: Clearly document the impact of these changes on backward compatibility and provide migration guides for users on .NET versions below 8.0.
  2. Behavior vs Behaviour:

    • The documentation uses inconsistent spelling for "behavior" (e.g., "Behaviour" in some sections). This could lead to confusion for international users.
    • Action: Standardize spelling across all documentation.

💡 SUGGESTIONS

  1. Expand on OWASP MCP Compliance:

    • The documentation mentions compliance with 11/12 OWASP MCP Security Cheat Sheet sections but does not specify which section is missing or why. This could lead to ambiguity for users aiming for full compliance.
    • Action: Clearly identify the missing section and provide guidance on how users can address it.
  2. Add Examples for gRPC Interceptor:

    • The documentation mentions gRPC server interceptors but does not provide usage examples.
    • Action: Include code snippets demonstrating how to use the gRPC interceptors for MCP governance.
  3. Clarify Rate Limiting Behavior:

    • The McpSlidingRateLimiter documentation mentions "per-agent sliding window rate limiting" but does not specify how the sliding window is implemented or configured.
    • Action: Add details on the configuration options for the sliding window (e.g., time intervals, max requests).
  4. Enhance i18n Documentation:

    • The i18n translations for Japanese and Simplified Chinese contain minor formatting inconsistencies (e.g., extra slashes in paths, missing spaces).
    • Action: Review and correct formatting issues in the translated documentation.
  5. Add Unit Tests for MCP Governance Layer:

    • While the documentation is comprehensive, there is no mention of unit tests for the MCP governance layer.
    • Action: Ensure that the MCP-related classes (McpGateway, McpSecurityScanner, McpSessionAuthenticator, etc.) have thorough unit test coverage, especially for edge cases and failure scenarios.
  6. Document Cryptographic Algorithms:

    • The documentation mentions post-quantum cryptography support (ML-DSA-65) but does not explain how it is implemented or its implications for users.
    • Action: Provide a dedicated section explaining the cryptographic algorithms used, their strengths, and any potential limitations.

Final Recommendation

The documentation is well-written and provides valuable insights into the .NET MCP security primitives. However, the critical issues related to replay protection, session authentication, sandbox escape vectors, and credential leakage must be addressed before merging. Additionally, the potential breaking changes should be clearly documented, and the suggested improvements should be considered to enhance the overall quality and usability of the documentation.

@jackbatzner
Copy link
Copy Markdown
Contributor Author

CI Status Summary

All CI checks pass except:

.NET MCP docs + Official SDK sample + mTLS Kestrel example. Slimmed to 9 files.

⚠️ Depends on #824 (Package) merging first. Merge order: #775#824#826.

@imran-siddique
Copy link
Copy Markdown
Member

Reviewed docs - content looks good. This PR is blocked on its dependency feature PR being merged first. Will merge this once the base PR lands.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Pull Request Review: docs(dotnet): add MCP security documentation and examples

Summary

This PR introduces comprehensive documentation for .NET MCP security primitives, including API references, OWASP MCP Cheat Sheet compliance mapping, quickstart guides, international translations, and usage examples. It is part of a series of PRs that aim to enhance the .NET MCP security features of the microsoft/agent-governance-toolkit.


🔴 CRITICAL Issues

  1. Potential Replay Attack in McpMessageSigner

    • The McpMessageSigner implementation uses a nonce-based replay protection mechanism. However, the documentation does not specify how nonce uniqueness is enforced across distributed systems. If the nonce is not globally unique or if it is reused, attackers could replay messages within the ReplayWindow.
    • Actionable Recommendation: Ensure nonce generation uses a cryptographically secure random number generator and document the mechanism for ensuring global uniqueness (e.g., UUIDs combined with timestamps). Additionally, clarify how nonce storage and expiration are handled in distributed environments.
  2. Sandbox Escape Vectors in Kubernetes Deployment

    • The Kubernetes hardening guide mentions using gVisor or Kata Containers for untrusted servers but does not enforce their usage. If these are optional, there is a risk of sandbox escape for MCP servers that execute arbitrary code.
    • Actionable Recommendation: Update the documentation to mandate the use of sandbox runtimes (e.g., gVisor or Kata Containers) for any MCP server that executes untrusted code. Add explicit warnings about the risks of not using sandboxing.
  3. Post-Quantum Cryptography Implementation

    • The documentation mentions ML-DSA-65 for post-quantum signing on .NET 10+, but there is no information about how this algorithm is integrated or whether it has been independently audited for security. Without proper validation, this could introduce vulnerabilities.
    • Actionable Recommendation: Provide details about the implementation of ML-DSA-65, including references to cryptographic audits or standards compliance. If this algorithm is experimental, explicitly warn users about its limitations.

🟡 WARNING Issues

  1. Breaking Changes in .NET MCP Protocol Support

    • The addition of post-quantum cryptography (ML-DSA-65) and multi-targeting for .NET 10+ introduces potential breaking changes for users relying on classical cryptographic algorithms (e.g., ECDSA/RSA).
    • Actionable Recommendation: Clearly document backward compatibility guarantees and provide migration guides for users transitioning from classical cryptography to post-quantum cryptography.
  2. Behavior vs. Behaviour Terminology

    • The documentation inconsistently uses "Behaviour" and "Behavior" in tables and examples. This could confuse users, especially in international contexts.
    • Actionable Recommendation: Standardize terminology across the documentation to use "Behavior" (American English) for consistency with .NET and Microsoft standards.

💡 Suggestions for Improvement

  1. OWASP MCP Cheat Sheet Compliance Mapping

    • While the documentation mentions compliance with OWASP MCP Cheat Sheet, it does not provide detailed mapping for each feature. Users may struggle to understand how specific features align with OWASP recommendations.
    • Actionable Recommendation: Add a dedicated section that explicitly maps each MCP security feature (e.g., McpGateway, McpSecurityScanner) to OWASP MCP Cheat Sheet sections.
  2. Internationalization (i18n) Enhancements

    • The i18n translations for Japanese and Simplified Chinese contain minor formatting inconsistencies (e.g., broken links, extra slashes in paths).
    • Actionable Recommendation: Review and fix all i18n translations for formatting issues. Consider adding automated tests to validate link correctness in translated documentation.
  3. Credential Redaction Documentation

    • The CredentialRedactor section is truncated in the diff, and its functionality is not fully explained. This is a critical feature for audit log security.
    • Actionable Recommendation: Expand the documentation for CredentialRedactor to include examples of supported credential patterns and how users can customize redaction rules.
  4. Resource Limits in Kubernetes

    • The Kubernetes hardening guide provides general recommendations for resource limits but does not specify how to enforce these limits across different environments (e.g., AKS, EKS, GKE).
    • Actionable Recommendation: Include platform-specific examples for enforcing resource limits, such as using Kubernetes admission controllers or policies.
  5. Testing Recommendations

    • The documentation does not mention testing strategies for MCP security features, such as how to validate the effectiveness of McpSecurityScanner or McpResponseScanner.
    • Actionable Recommendation: Add a section on testing MCP security features, including example test cases and integration with CI/CD pipelines.

Final Assessment

This PR significantly enhances the documentation for .NET MCP Protocol Support and aligns with OWASP MCP Cheat Sheet recommendations. However, there are critical security concerns related to nonce replay protection, sandboxing, and post-quantum cryptography implementation. Addressing these issues is essential to ensure the robustness and security of the MCP governance layer.

  • Approval Status: 🚫 Changes Required
  • Priority: High (due to critical security issues)

Please address the flagged issues and suggestions before merging this PR.

@github-actions github-actions bot added size/M Medium PR (< 200 lines) and removed documentation Improvements or additions to documentation labels Apr 6, 2026
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review of Pull Request: docs(dotnet): add MCP security documentation and examples


🔴 CRITICAL

No critical security issues identified in this documentation update. However, ensure that the examples provided in the documentation do not inadvertently introduce insecure practices, especially when dealing with MCP security primitives.


🟡 WARNING

  1. Breaking Changes in Documentation:
    • This PR introduces documentation for .NET MCP security primitives. While this is not a direct code change, any discrepancies between the documentation and the actual implementation could lead to confusion for users. Ensure that the documentation aligns perfectly with the MCP package's API and behavior.

💡 SUGGESTIONS

  1. Expand OWASP MCP Cheat Sheet Compliance Mapping:

    • The documentation mentions OWASP MCP Cheat Sheet compliance mapping. Consider adding a detailed section that explicitly maps each MCP security primitive to the relevant OWASP guidelines. This will help users understand how the library adheres to security best practices.
  2. Include Threat Modeling Examples:

    • Adding threat modeling examples for common use cases would be beneficial. This can help users understand potential attack vectors and how to mitigate them using the MCP primitives.
  3. Add Code Snippets for Common Scenarios:

    • While the documentation mentions usage examples, ensure that these examples cover common scenarios such as credential handling, cryptographic operations, and SPIFFE/SVID integration. This will make the documentation more practical and user-friendly.
  4. Internationalization (i18n):

    • The PR mentions i18n translations. Ensure that translations are accurate and culturally appropriate, especially for security-related terminology.
  5. Spell Check Updates:

    • The .cspell.json file has been updated with new words. While this is helpful for avoiding false positives during spell checks, ensure that these additions are necessary and relevant to the project. For example:
      • Words like "asyncio", "deque", "sandboxing", and "SPIFFE" are relevant to the project.
      • However, some terms like "Portkey" and "thiserror" may need verification to ensure they are directly related to the project scope.
  6. Documentation Style Consistency:

    • Ensure that the documentation adheres to the project's style guidelines. Consistent formatting, terminology, and examples will improve readability and usability.

Summary

This PR primarily focuses on documentation updates, which are critical for user adoption and understanding of the .NET MCP security primitives. While no direct security or breaking issues were identified, the suggestions provided aim to enhance the quality and utility of the documentation. Ensure that the documentation is thoroughly reviewed for accuracy and alignment with the actual implementation before merging.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Feedback on Pull Request: docs(dotnet): add MCP security documentation and examples

General Observations

This PR primarily updates documentation and adds examples for .NET MCP (Microsoft Cloud Policy) security primitives. While it doesn't directly impact the Python codebase, it is important to ensure that the documentation aligns with the security and compliance standards of the project.


🔴 CRITICAL

No critical security issues were identified in this PR.


🟡 WARNING

  1. Potential Breaking Change in Documentation Dependencies
    • The .cspell.json file has been updated with additional words. While this is not a breaking change in itself, if other parts of the repository rely on this file for spell-checking or CI validation, these changes could inadvertently cause issues in unrelated workflows.
    • Action: Ensure that all workflows using .cspell.json are tested to confirm compatibility with the updated word list.

💡 SUGGESTIONS

  1. Ensure OWASP MCP Cheat Sheet Compliance

    • The PR mentions mapping to the OWASP MCP Cheat Sheet. While this is a documentation update, it is critical to ensure that the examples and quickstart guides are fully aligned with OWASP's best practices.
    • Action: Perform a thorough review of the examples to confirm they do not inadvertently introduce insecure patterns or anti-patterns.
  2. Add Security-Specific Examples

    • The documentation could benefit from additional examples that explicitly demonstrate secure usage of MCP primitives, such as:
      • Proper handling of sensitive data.
      • Secure configuration of cryptographic operations.
      • Avoidance of common pitfalls like hardcoded credentials or insecure defaults.
    • Action: Add at least one example for each of the above scenarios to strengthen the documentation.
  3. Internationalization (i18n) Validation

    • The PR mentions i18n translations. Ensure that the translations are accurate and that they do not inadvertently misrepresent security-critical information.
    • Action: If possible, involve native speakers or use automated tools to validate the translations.
  4. Spell-Checking Additions

    • The .cspell.json file has been updated with new words. While this is helpful for avoiding false positives in spell-checking, some of the added words (e.g., "backdoors", "cmdshell", "sandboxing") are security-sensitive terms.
    • Action: Review the added words to ensure they are necessary and appropriate for inclusion.
  5. Documentation Testing

    • While this PR focuses on documentation, it is important to test the examples provided to ensure they are functional and secure.
    • Action: Run all provided examples in a controlled environment to validate their correctness and security.

Summary

This PR is a documentation update and does not introduce direct changes to the Python codebase or the policy engine. However, it is essential to ensure that the documentation aligns with the project's security and compliance standards. The suggestions provided aim to enhance the quality and security of the documentation.

Let me know if you need further assistance!

@imran-siddique
Copy link
Copy Markdown
Member

Closing — PR #910 already added the cspell words needed for the OWASP/MCP documentation. This PR also removes the trailing newline from .cspell.json which would cause a ruff W292 equivalent. Thank you for the cleanup effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR (< 200 lines) size/XL Extra large PR (500+ lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants