Skip to content

feat(dotnet): add standalone Microsoft.AgentGovernance.Mcp package#824

Closed
jackbatzner wants to merge 6 commits intomicrosoft:mainfrom
jackbatzner:jb/dotnet-mcp-package
Closed

feat(dotnet): add standalone Microsoft.AgentGovernance.Mcp package#824
jackbatzner wants to merge 6 commits intomicrosoft:mainfrom
jackbatzner:jb/dotnet-mcp-package

Conversation

@jackbatzner
Copy link
Copy Markdown
Contributor

Description

Introduces the standalone Microsoft.AgentGovernance.Mcp NuGet package with minimal dependencies. This allows MCP server operators to adopt MCP governance primitives without pulling in the full Agent Governance Toolkit.

Package: Microsoft.AgentGovernance.Mcp (multi-target .NET 8 / .NET 10)

  • Standalone packaging with re-exports from the core MCP module
  • README with installation, quick-start examples, and OWASP MCP Cheat Sheet mapping

Part 2 of 3 — Merge after #775. See also:

Type of Change

  • New feature (non-breaking change that adds functionality)

Package(s) Affected

  • agent-governance-dotnet

Checklist

  • My code follows the project style guidelines
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass (dotnet test — 1003/1003)
  • I have updated documentation as needed
  • I have signed the Microsoft CLA

Related Issues

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

@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 introduces a new standalone .NET package, Microsoft.AgentGovernance.Mcp, with minimal dependencies. It does not modify existing APIs but adds new functionality. There are no breaking changes to existing APIs in the repository.

Findings

Severity Package Change Impact
🔵 agent-governance-dotnet Added new Microsoft.AgentGovernance.Mcp package New functionality, no breaking changes
🔵 agent-governance-dotnet Added McpGovernance.AspNetCore sample project New example, no breaking changes
🔵 agent-governance-dotnet Added AgentGovernance.ModelContextProtocol project New project, no breaking changes

Migration Guide

No migration steps are necessary as this pull request does not introduce any breaking changes.

Additional Notes

  • The new APIs and projects should be documented thoroughly to ensure downstream users can easily adopt the new functionality.
  • Ensure that the new sample project (McpGovernance.AspNetCore) and its configuration (appsettings.json) are included in the official documentation for better visibility.

No breaking changes detected.

@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

  • SampleTools.ReadFile() in Program.cs — missing docstring
  • SampleTools.SearchDatabase() in Program.cs — missing docstring
  • SampleTools.SendEmail() in Program.cs — missing docstring
  • ⚠️ packages/agent-governance-dotnet/README.md — no updates to reflect the new Microsoft.AgentGovernance.Mcp package or its usage.
  • ⚠️ CHANGELOG.md — no entry for the addition of the Microsoft.AgentGovernance.Mcp package.
  • ⚠️ examples/ — no updates or additions to reflect the new Microsoft.AgentGovernance.Mcp package.

Suggestions

  • 💡 Add docstrings for the following methods in SampleTools:
    • ReadFile(path: str) -> dict
    • SearchDatabase(query: str, maxRows: int = 50) -> dict
    • SendEmail(to: str, subject: str, body: str) -> dict
  • 💡 Update the README.md in packages/agent-governance-dotnet to include a section on the new Microsoft.AgentGovernance.Mcp package, its purpose, and usage.
  • 💡 Add an entry to CHANGELOG.md documenting the introduction of the Microsoft.AgentGovernance.Mcp package.
  • 💡 Add or update example code in the examples/ directory to demonstrate the usage of the new package.

Additional Notes

  • The new Microsoft.AgentGovernance.Mcp package is a significant addition to the project. Proper documentation and examples are crucial to ensure users can understand and utilize its features effectively.
  • The Program.cs file in the new sample project includes inline comments, but these do not replace the need for proper docstrings for public methods.
  • The appsettings.json file in the sample project is well-documented, but it would be helpful to reference it in the README.md for better discoverability.

Action Required

Please address the issues and suggestions listed above to ensure the documentation and examples are in sync with the new changes.

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 a new .NET package, Microsoft.AgentGovernance.Mcp, which provides a standalone implementation of MCP governance primitives with minimal dependencies. It also includes two sample projects demonstrating integration with ASP.NET Core and the official ModelContextProtocol (MCP) SDK. The PR appears well-structured and adheres to the project's style guidelines. However, there are several areas that require attention to ensure security, maintainability, and backward compatibility.


🔴 CRITICAL

  1. Credential Redaction in Logs

    • In the McpGovernance.OfficialSdk sample, the CredentialRedactor.Redact() function is mentioned but commented out in the AddOutgoingFilter block. This leaves a potential security gap where sensitive information could be logged.
    • Action: Ensure that the CredentialRedactor.Redact() functionality is fully implemented and tested. Sensitive information must be redacted in all logs and responses before being sent.
  2. Tool Discovery and Execution

    • The McpTool attribute is used for tool discovery, but there is no evidence of validation or sanitization of the tool methods' inputs. This could lead to injection vulnerabilities (e.g., SQL injection in SearchDatabase).
    • Action: Add input validation and sanitization for all tool methods. Consider using a library like FluentValidation or custom validation logic to ensure inputs are safe.
  3. Rate Limiting and Replay Protection

    • The appsettings.json file includes governance settings like RateLimitWindowMinutes and MessageReplayWindowSeconds, but there is no evidence in the code that these settings are enforced.
    • Action: Implement and test rate limiting and replay protection mechanisms to prevent abuse and replay attacks.
  4. Human Approval for Sensitive Actions

    • The SendEmail tool method has the RequiresApproval attribute, but there is no implementation to enforce this requirement.
    • Action: Implement a mechanism to enforce human approval for tools marked with RequiresApproval.

🟡 WARNING

  1. Backward Compatibility

    • The introduction of the Microsoft.AgentGovernance.Mcp package and its dependencies may affect existing users of the agent-governance-dotnet package.
    • Action: Clearly document any changes to the public API and ensure that existing functionality is not broken. Add deprecation warnings if necessary.
  2. Multi-Targeting .NET Versions

    • The new package targets both .NET 8.0 and .NET 10.0. While this is forward-looking, it may introduce compatibility issues for users on older versions of .NET.
    • Action: Test the package thoroughly on both target frameworks and document any known limitations or differences.

💡 SUGGESTIONS

  1. Unit Tests for Governance Pipeline

    • While the PR mentions that tests have been added, it is unclear if the governance pipeline (e.g., InterceptToolCall) has been thoroughly tested for edge cases.
    • Action: Add unit tests to cover scenarios like:
      • Denied tools being called.
      • Sensitive tools being called without human approval.
      • Rate-limiting and replay protection.
      • Credential redaction in logs and responses.
  2. OWASP MCP Security Compliance

    • The PR claims OWASP MCP Security Cheat Sheet compliance, but there is no explicit mapping of implemented features to the cheat sheet.
    • Action: Add a section in the documentation mapping implemented features to the OWASP MCP Security Cheat Sheet.
  3. Structured Logging

    • The sample projects use ILogger for logging, but there is no mention of structured logging or correlation IDs for tracing requests.
    • Action: Use structured logging with correlation IDs to improve traceability and debugging.
  4. Documentation

    • The documentation for the new package and samples is clear but could benefit from additional details on security features and best practices.
    • Action: Expand the documentation to include:
      • Security best practices for using the governance pipeline.
      • Examples of integrating with other frameworks or SDKs.
      • Troubleshooting common issues.
  5. Sandboxing

    • There is no mention of sandboxing or isolation for tool execution. This could lead to sandbox escape vulnerabilities.
    • Action: Evaluate the need for sandboxing and consider using technologies like AppDomains or containers to isolate tool execution.

Conclusion

This PR introduces valuable functionality to the agent-governance-dotnet package and provides useful samples for integration with ASP.NET Core and the official MCP SDK. However, there are critical security issues that must be addressed before merging, particularly around input validation, rate limiting, replay protection, and human approval enforcement. Additionally, backward compatibility and documentation improvements should be considered to ensure a smooth adoption for existing users.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

🤖 AI Agent: security-scanner — Security Review for `microsoft/agent-governance-toolkit` Pull Request

Security Review for microsoft/agent-governance-toolkit Pull Request

After reviewing the provided changes to the microsoft/agent-governance-toolkit repository, here is the security analysis based on the specified criteria:


1. Prompt Injection Defense Bypass

  • Risk: The demo_simulated.py and getting_started.py examples demonstrate governance middleware that intercepts agent messages before they reach the LLM. However, the examples do not explicitly show how the middleware handles or sanitizes user inputs to prevent prompt injection attacks.
  • Attack Vector: If the GovernancePolicyMiddleware or PolicyEvaluator does not properly sanitize or validate user inputs, a malicious actor could craft inputs that manipulate the LLM into bypassing governance policies.
  • Rating: 🔴 CRITICAL
  • Recommendation:
    • Ensure that the PolicyEvaluator and GovernancePolicyMiddleware explicitly sanitize user inputs to prevent injection attacks.
    • Add tests that simulate prompt injection attempts (e.g., inputs containing special characters, escape sequences, or commands) and verify that they are blocked.
    • Consider implementing a robust input validation and sanitization mechanism.

2. Policy Engine Circumvention

  • Risk: The GovernancePolicyMiddleware and CapabilityGuardMiddleware are responsible for enforcing policies and capability restrictions. However, the provided examples do not include tests that ensure policies cannot be bypassed.
  • Attack Vector: If an attacker can manipulate the middleware or bypass it entirely (e.g., by directly invoking the LLM or tools), they could circumvent governance policies.
  • Rating: 🔴 CRITICAL
  • Recommendation:
    • Add comprehensive tests to ensure that all governance policies are enforced in various scenarios, including edge cases.
    • Ensure that the middleware cannot be bypassed by directly invoking the LLM or tools.
    • Consider implementing additional safeguards, such as cryptographic signing of policy decisions, to ensure integrity.

3. Trust Chain Weaknesses

  • Risk: The docs/deployment/mcp-server-hardening.md file provides guidance on securing MCP servers, including mTLS and certificate validation. However, the example code for client certificate validation (https.ClientCertificateValidation) does not include a robust implementation for validating the certificate chain.
  • Attack Vector: If the certificate validation logic is not properly implemented, an attacker could use a malicious certificate to impersonate a trusted client.
  • Rating: 🔴 CRITICAL
  • Recommendation:
    • Replace the placeholder return errors == System.Net.Security.SslPolicyErrors.None; with a robust certificate validation implementation that verifies the certificate chain against a trusted CA.
    • Add tests to ensure that invalid or untrusted certificates are rejected.

4. Credential Exposure

  • Risk: The examples (demo_simulated.py, getting_started.py) do not appear to log or expose sensitive credentials. However, the getting_started.py script mentions the use of LLM API keys (e.g., OPENAI_API_KEY) but does not show how they are securely loaded or managed.
  • Attack Vector: If API keys are hardcoded or improperly managed, they could be exposed in logs or source code.
  • Rating: 🟡 MEDIUM
  • Recommendation:
    • Use environment variables or secure secrets management systems (e.g., Azure Key Vault) to manage API keys.
    • Ensure that sensitive information is not logged or exposed in error messages.

5. Sandbox Escape

  • Risk: The docs/deployment/mcp-server-hardening.md file provides guidance on sandboxing MCP servers using gVisor or Kata Containers. However, there is no evidence that the provided examples (demo_simulated.py, getting_started.py) are tested in such environments.
  • Attack Vector: If the MCP server or tools are not properly sandboxed, an attacker could exploit vulnerabilities to escape the container or process isolation.
  • Rating: 🟠 HIGH
  • Recommendation:
    • Add automated tests to verify that the MCP server and tools are properly sandboxed in gVisor or Kata Containers.
    • Ensure that all container images are scanned for vulnerabilities and use minimal base images.

6. Deserialization Attacks

  • Risk: The PolicyEvaluator loads YAML policies, but the implementation is not provided in the diff. If the YAML loading process is not secure, it could be vulnerable to deserialization attacks.
  • Attack Vector: If an attacker can inject malicious YAML into the policy files, they could execute arbitrary code during deserialization.
  • Rating: 🔴 CRITICAL
  • Recommendation:
    • Use a safe YAML parser (e.g., ruamel.yaml.safe_load or PyYAML.safe_load) to load policies.
    • Add tests to ensure that malicious YAML inputs are rejected.

7. Race Conditions

  • Risk: The GovernancePolicyMiddleware and CapabilityGuardMiddleware may be subject to race conditions if multiple requests are processed concurrently. The provided examples do not include tests for concurrent execution.
  • Attack Vector: An attacker could exploit a race condition to bypass governance checks or cause inconsistent policy enforcement.
  • Rating: 🟠 HIGH
  • Recommendation:
    • Review the implementation of GovernancePolicyMiddleware and CapabilityGuardMiddleware to ensure thread safety.
    • Add tests to simulate concurrent requests and verify that governance checks are consistently enforced.

8. Supply Chain

  • Risk: The new Microsoft.AgentGovernance.Mcp package introduces dependencies on .NET 8 and .NET 10. The security of these dependencies is critical to the overall security of the toolkit.
  • Attack Vector: If a malicious or compromised dependency is included, it could introduce vulnerabilities into the toolkit.
  • Rating: 🟠 HIGH
  • Recommendation:
    • Use a dependency scanning tool (e.g., GitHub Dependabot) to monitor for vulnerabilities in dependencies.
    • Verify the integrity of all third-party dependencies (e.g., using checksums or signature verification).

Summary of Findings

Issue Rating Recommendation Summary
Prompt injection defense bypass 🔴 CRITICAL Add input sanitization and tests for injection attempts.
Policy engine circumvention 🔴 CRITICAL Add tests to ensure policies cannot be bypassed and consider cryptographic safeguards.
Trust chain weaknesses 🔴 CRITICAL Implement robust certificate chain validation for mTLS.
Credential exposure 🟡 MEDIUM Use secure secrets management and avoid logging sensitive information.
Sandbox escape 🟠 HIGH Test MCP server and tools in sandboxed environments like gVisor or Kata Containers.
Deserialization attacks 🔴 CRITICAL Use safe YAML parsers and add tests for malicious YAML inputs.
Race conditions 🟠 HIGH Ensure thread safety in middleware and add tests for concurrent execution.
Supply chain 🟠 HIGH Use dependency scanning tools and verify third-party dependencies.

General Recommendations

  • Perform a thorough review of the PolicyEvaluator and GovernancePolicyMiddleware implementations to ensure they are robust against injection and circumvention attacks.
  • Add comprehensive test coverage for all security-critical components, including edge cases and adversarial scenarios.
  • Ensure that all deployment and runtime environments follow the hardening guidelines provided in mcp-server-hardening.md.

Let me know if you need further clarification or additional details on any of the findings!

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 a new standalone .NET package, Microsoft.AgentGovernance.Mcp, which provides MCP governance primitives with minimal dependencies. It also includes sample projects demonstrating integration with ASP.NET Core and the official ModelContextProtocol (MCP) SDK. The PR appears well-structured and adheres to the project's style guidelines. However, there are some areas that require attention, particularly around security, backward compatibility, and documentation.


🔴 CRITICAL

  1. Credential Redaction in Outgoing Responses

    • In the McpGovernance.OfficialSdk sample, the CredentialRedactor.Redact() function is mentioned but not implemented in the provided code. This could lead to sensitive information being leaked in outgoing responses if developers forget to implement redaction.
    • Action: Ensure that the CredentialRedactor.Redact() function is implemented and tested. Add a fallback mechanism to log a warning if redaction is not applied.
  2. Tool Discovery and Execution

    • The McpToolRegistry.DiscoverTools() method automatically registers tools based on the [McpTool] attribute. However, there is no validation to ensure that tools do not have unsafe or malicious behavior.
    • Action: Add a validation mechanism to ensure that tools registered via McpToolRegistry.DiscoverTools() comply with security policies (e.g., deny-list, allow-list, or sandboxing).
  3. SQL Injection in Tool Calls

    • The search tool in the SampleTools class does not sanitize the query parameter, leaving it vulnerable to SQL injection.
    • Action: Add input validation and sanitization for all tool parameters, especially those that interact with external systems like databases.
  4. Replay Attack Protection

    • The MessageReplayWindowSeconds configuration in appsettings.json is set to 300 seconds, but there is no evidence in the code that replay attacks are being mitigated.
    • Action: Implement replay attack protection by maintaining a cache of recently processed message IDs and rejecting duplicates within the specified window.

🟡 WARNING

  1. Backward Compatibility

    • The introduction of a new package (Microsoft.AgentGovernance.Mcp) and its associated APIs could lead to potential backward compatibility issues if existing users of the toolkit migrate to this package.
    • Action: Clearly document migration paths and ensure that existing functionality is not disrupted for users of the core AgentGovernance package.
  2. Multi-Targeting for .NET 8 and .NET 10

    • The new package targets .NET 8 and .NET 10, which may not be compatible with older versions of .NET. This could limit adoption for users on older frameworks.
    • Action: Consider adding support for .NET 6 or .NET 7 if feasible, or explicitly document the minimum supported framework versions.

💡 SUGGESTIONS

  1. OWASP MCP Security Cheat Sheet Compliance

    • The PR claims to provide "full OWASP MCP Security Cheat Sheet coverage (11/12 sections)," but this is not explicitly verified in the code or tests.
    • Action: Add automated tests to verify compliance with all relevant sections of the OWASP MCP Security Cheat Sheet.
  2. Documentation Improvements

    • The documentation for the McpGovernance.OfficialSdk sample is clear but assumes familiarity with the MCP SDK. New users may struggle to understand the integration process.
    • Action: Add more detailed explanations and diagrams to illustrate how the governance layer interacts with the MCP SDK.
  3. Thread Safety

    • The McpToolRegistry and McpGateway classes are used in a multi-threaded environment (e.g., ASP.NET Core). However, there is no explicit mention of thread safety in their implementation.
    • Action: Review these classes for thread safety and document any thread-safety guarantees or limitations.
  4. Testing Coverage

    • While the PR mentions that all tests pass, there is no evidence of new tests specifically for the Microsoft.AgentGovernance.Mcp package or its integration with the MCP SDK.
    • Action: Add unit and integration tests for the new package, focusing on edge cases like denied tools, rate limiting, and response scanning.
  5. Default Configuration

    • The default configuration in appsettings.json leaves the DeniedTools and AllowedTools lists empty, which could lead to insecure defaults.
    • Action: Provide a secure default configuration with a reasonable deny-list and allow-list.
  6. Logging and Monitoring

    • The samples include basic logging via ILogger, but there is no mention of centralized logging or monitoring for governance-related events.
    • Action: Recommend best practices for logging and monitoring, such as integrating with Azure Monitor or other observability platforms.

Conclusion

The PR introduces a valuable addition to the Agent Governance Toolkit by providing a lightweight .NET package for MCP governance. However, there are critical security issues that must be addressed before merging, particularly around credential redaction, tool validation, and SQL injection. Additionally, there are potential backward compatibility concerns and areas for improvement in documentation and testing. Addressing these issues will ensure the new package is robust, secure, and easy to adopt.

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 PR introduces a standalone .NET package (Microsoft.AgentGovernance.Mcp) that integrates MCP governance primitives into the Agent Governance Toolkit. It also includes two sample projects demonstrating integration with ASP.NET Core and the official ModelContextProtocol (MCP) SDK. The PR is well-documented and adheres to the project guidelines. However, there are several areas that require attention to ensure security, maintainability, and backward compatibility.


🔴 CRITICAL

  1. Potential Security Bypass in InterceptToolCall

    • The InterceptToolCall method in the McpGateway class appears to evaluate tool calls for governance compliance. However, the implementation of this method is not visible in the provided diff. Ensure that:
      • All governance checks (deny-list, allow-list, sanitization, rate limiting, etc.) are correctly implemented and cannot be bypassed.
      • The method handles edge cases, such as malformed requests or unexpected input types, to prevent bypasses.
      • The method logs all denied requests for audit purposes.
    • Action: Provide the implementation of InterceptToolCall for review.
  2. Credential Redaction

    • The CredentialRedactor.Redact method is used to sanitize sensitive information in responses. However, the implementation is not visible in the diff.
    • Action: Ensure that the redaction logic is robust and can handle edge cases, such as nested sensitive fields or non-standard formats. Provide the implementation for review.
  3. Sandbox Escape Vectors

    • The SampleTools class includes methods like ReadFile and SearchDatabase. While these are marked as "safe" or "read-only," there is no visible enforcement of these constraints.
    • Action: Ensure that:
      • File paths are sanitized to prevent directory traversal attacks.
      • Database queries are parameterized to prevent SQL injection.
      • Any dynamic code execution or file access is sandboxed to prevent unauthorized access.
  4. gRPC Interceptor

    • The gRPC interceptor in the ASP.NET Core sample uses AddGrpc(grpc => grpc.AddMcpGovernance()). Ensure that:
      • The interceptor cannot be bypassed or disabled by malicious actors.
      • The governance pipeline is applied consistently to all gRPC requests.
    • Action: Provide the implementation of AddMcpGovernance for review.

🟡 WARNING

  1. Backward Compatibility

    • The introduction of a new package (Microsoft.AgentGovernance.Mcp) appears to be non-breaking. However, ensure that:
      • The new package does not introduce breaking changes to existing functionality.
      • The new package is properly versioned and documented to avoid confusion for existing users.
  2. Multi-Targeting .NET Versions

    • The new package targets both .NET 8.0 and .NET 10.0. Ensure that:
      • The package is tested on both frameworks.
      • Any breaking changes in .NET 10.0 are accounted for.

💡 SUGGESTIONS

  1. OWASP MCP Compliance

    • The PR mentions compliance with the OWASP MCP Security Cheat Sheet. Consider adding automated tests to verify compliance with all 12 sections of the cheat sheet.
  2. Type Safety

    • The InterceptToolCall method appears to accept parameters as a Dictionary<string, object>. Consider using strongly-typed models to improve type safety and reduce the risk of runtime errors.
  3. Logging and Observability

    • Ensure that all governance decisions (allow/deny) are logged with sufficient context (e.g., tool name, agent ID, parameters) for auditing and debugging purposes.
  4. Documentation

    • The provided README files are detailed and helpful. However, consider adding a section on common pitfalls and best practices for integrating MCP governance into .NET applications.
  5. Testing

    • The PR includes sample projects, but it's unclear if there are automated tests for the new package. Ensure that:
      • Unit tests cover all governance logic.
      • Integration tests validate the end-to-end behavior of the new package with the official MCP SDK and ASP.NET Core.
  6. Thread Safety

    • Ensure that the McpGateway and related classes are thread-safe, especially if they are used in a multi-threaded environment like ASP.NET Core or gRPC.
  7. Configuration Validation

    • The McpGovernanceOptions.BindFromConfiguration method binds settings from appsettings.json. Consider adding validation logic to ensure that all required settings are present and have valid values.

Final Recommendation

  • Address the CRITICAL issues before merging.
  • Review the implementations of InterceptToolCall, CredentialRedactor.Redact, and AddMcpGovernance for security and correctness.
  • Consider the SUGGESTIONS to improve the overall quality and maintainability of the codebase.

Once the critical issues are resolved and the suggestions are considered, this PR will be ready for approval.

@jackbatzner jackbatzner force-pushed the jb/dotnet-mcp-package branch from 6ef151f to 99df13d Compare April 6, 2026 14:50
@github-actions github-actions bot added size/L Large PR (< 500 lines) and removed tests 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 Summary

This pull request introduces a new standalone .NET package, Microsoft.AgentGovernance.Mcp, which provides MCP governance primitives with minimal dependencies. It also includes a sample ASP.NET Core application demonstrating the integration of MCP governance features. The PR appears well-structured and adheres to the project guidelines. However, there are a few areas that require attention to ensure security, maintainability, and backward compatibility.


🔴 CRITICAL

  1. Potential Insufficient Validation of Tool Calls

    • In the SampleTools class, the ReadFile and SearchDatabase methods do not appear to validate their inputs. For example:
      public static Dictionary<string, object> ReadFile(string path)
      {
          return new() { ["content"] = $"[simulated content of {path}]" };
      }
      • Risk: If this is a placeholder for actual file reading logic, there is a risk of directory traversal attacks or unauthorized file access if the path parameter is not properly sanitized.
      • Action: Add input validation to ensure that the path is restricted to safe directories and does not allow traversal (../) or access to sensitive files.
  2. Missing Authentication/Authorization for Tool Calls

    • The sample application does not demonstrate any form of authentication or authorization for MCP tool calls. This could allow unauthorized users to execute sensitive operations.
      • Risk: Unauthorized access to sensitive tools like SendEmail or potential misuse of other tools.
      • Action: Ensure that the middleware enforces authentication and authorization for all MCP tool calls. Consider integrating with ASP.NET Core's built-in authentication and authorization mechanisms.
  3. Potential for Replay Attacks

    • The MessageReplayWindowSeconds configuration parameter is set to 300 seconds (5 minutes). However, there is no evidence in the provided code that replay protection is implemented.
      • Risk: Without proper replay protection, an attacker could reuse a previously intercepted request to execute the same action multiple times.
      • Action: Implement replay protection by maintaining a cache of recently processed message IDs and rejecting any duplicates within the replay window.

🟡 WARNING

  1. Breaking Changes in Public API
    • The introduction of the Microsoft.AgentGovernance.Mcp package and its associated APIs could potentially introduce breaking changes for existing users of the .NET library if they expect all MCP-related functionality to be part of the core AgentGovernance package.
      • Action: Clearly document the migration path for existing users and ensure that the new package is backward-compatible with the existing APIs.

💡 SUGGESTIONS

  1. Add Unit Tests for New Features

    • While the PR mentions that tests have been added, there is no evidence of unit tests for the new Microsoft.AgentGovernance.Mcp package or the sample application. This is especially important for critical features like input validation, authentication, and replay protection.
      • Action: Add comprehensive unit tests for the new package and sample application, focusing on edge cases and security scenarios.
  2. Enhance Documentation

    • The provided documentation is a good start, but it could be improved by including:
      • A detailed explanation of the security features provided by the MCP governance middleware.
      • Examples of how to configure authentication and authorization for MCP tool calls.
      • Guidance on how to implement custom tools and register them using the [McpTool] attribute.
  3. Consider Default Deny Policy for Tools

    • The appsettings.json file currently defines empty lists for DeniedTools and AllowedTools. This implies that all tools are allowed by default.
      • Action: Consider adopting a "default deny" policy by requiring explicit listing of allowed tools in the AllowedTools list. This would reduce the risk of unauthorized tool execution.
  4. Thread Safety in McpToolRegistry

    • The McpToolRegistry class is used to register tools dynamically. If this class is accessed concurrently (e.g., during tool discovery or runtime tool registration), there could be thread safety issues.
      • Action: Ensure that McpToolRegistry is thread-safe by using appropriate synchronization mechanisms (e.g., locks or concurrent collections).
  5. Consider OWASP Agentic Top 10 Compliance

    • The PR does not explicitly address compliance with the OWASP Agentic Top 10. For example:
      • A1: Insecure Agent Initialization: Ensure that the agent initialization process is secure and does not expose sensitive information.
      • A2: Insecure Communication: Ensure that all communication between agents and the MCP server is encrypted and authenticated.
      • A4: Insecure Credential Management: Ensure that sensitive credentials (e.g., API keys or tokens) are not exposed in logs or configuration files.
      • Action: Conduct a thorough review of the implementation to ensure compliance with the OWASP Agentic Top 10.
  6. Add Logging for Security Events

    • The sample application includes structured logging via ILogger, but there is no mention of logging security-related events (e.g., denied tool calls, replay attempts, or authentication failures).
      • Action: Add logging for all security-related events to facilitate auditing and incident response.

Final Assessment

The PR introduces a valuable new package and sample application for MCP governance. However, there are critical security issues that need to be addressed before merging. Additionally, there are opportunities to improve documentation, testing, and compliance with security best practices. Please address the critical issues and consider the suggestions to ensure a robust and secure implementation.

@jackbatzner jackbatzner force-pushed the jb/dotnet-mcp-package branch from 99df13d to 99b2c2e Compare April 6, 2026 14:52
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: feat(dotnet): add standalone Microsoft.AgentGovernance.Mcp package


🔴 CRITICAL

  1. Potential Security Issue: Lack of Input Validation in Sample Tools

    • The sample tools provided in SampleTools (e.g., ReadFile, SearchDatabase, SendEmail) do not perform any input validation or sanitization. For example:
      • ReadFile directly accepts a file path without validating or sanitizing it, which could lead to directory traversal attacks.
      • SearchDatabase accepts a raw query string, which could be exploited for SQL injection if used in a real database context.
      • SendEmail does not validate email addresses or sanitize the subject/body, which could lead to injection attacks or abuse.
    • Recommendation: Add input validation and sanitization to all tool methods, even in sample code, to prevent potential misuse or security vulnerabilities.
  2. Insufficient Protection Against Denied Tools

    • The DeniedTools list in appsettings.json is empty by default. This could lead to a situation where no tools are explicitly denied, potentially allowing unsafe operations.
    • Recommendation: Provide a default list of commonly denied tools or explicitly document the risks of leaving this list empty.
  3. Potential Replay Attack Vulnerability

    • The MessageReplayWindowSeconds configuration in appsettings.json is set to 300 seconds (5 minutes). While this provides some protection against replay attacks, it may still be too lenient for high-security environments.
    • Recommendation: Consider reducing the replay window or providing guidance on how to configure this value based on the security requirements of the deployment.
  4. Credential Redaction

    • The EnableCredentialRedaction setting in appsettings.json is set to true by default, which is good. However, there is no evidence in the PR that this feature has been implemented or tested.
    • Recommendation: Ensure that credential redaction is implemented and thoroughly tested, especially in logs and error messages.

🟡 WARNING

  1. Breaking Changes in .NET Target Framework

    • The new Microsoft.AgentGovernance.Mcp package targets .NET 8 and .NET 10. This could potentially break compatibility with projects using older .NET versions.
    • Recommendation: Clearly document the minimum .NET version requirements for this package in the release notes and README.
  2. Backward Compatibility with Existing MCP Implementations

    • The introduction of a standalone Microsoft.AgentGovernance.Mcp package may lead to compatibility issues with existing MCP implementations that rely on the core Agent Governance Toolkit.
    • Recommendation: Provide migration guidance for existing users to transition to the new package, if necessary.

💡 SUGGESTIONS

  1. Enhance Documentation

    • The provided README for the McpGovernance.AspNetCore sample is clear but could benefit from additional details:
      • Include a section on security best practices, such as configuring DeniedTools, SensitiveTools, and EnableCredentialRedaction.
      • Provide examples of how to implement custom McpTool methods with proper input validation and error handling.
  2. Add Unit Tests for New Features

    • While the PR mentions that tests have been added, there is no evidence of unit tests for the new Microsoft.AgentGovernance.Mcp package or the McpGovernance.AspNetCore sample.
    • Recommendation: Add unit tests for:
      • Input validation and sanitization in SampleTools.
      • Behavior of the AddMcpGovernance and UseMcpGovernance methods.
      • Configuration binding from appsettings.json.
  3. Thread Safety

    • The McpToolRegistry class is used to register tools in the McpGovernance.AspNetCore sample. However, there is no indication that this class is thread-safe.
    • Recommendation: Ensure that McpToolRegistry is thread-safe, especially if it will be used in a multi-threaded environment like ASP.NET Core.
  4. OWASP Agentic Top 10 Compliance

    • The PR does not explicitly address compliance with the OWASP Agentic Top 10. While some features (e.g., EnableCredentialRedaction, EnableResponseScanning) are steps in the right direction, there is no evidence of a comprehensive approach to address all relevant risks.
    • Recommendation: Conduct a thorough review of the new package and sample against the OWASP Agentic Top 10 and document how each risk is mitigated.
  5. Consider Default Configuration Values

    • The default values in appsettings.json (e.g., MaxToolCallsPerAgent, RateLimitWindowMinutes, etc.) may not be suitable for all use cases.
    • Recommendation: Provide guidance in the documentation on how to configure these settings based on different deployment scenarios (e.g., development, production, high-security environments).
  6. Code Comments and Documentation

    • While the sample code is well-commented, the main library code (e.g., AgentGovernance.ModelContextProtocol.csproj) lacks inline comments explaining the purpose and usage of key components.
    • Recommendation: Add comments to the main library code to improve maintainability and readability.

Summary

This PR introduces a valuable new feature by providing a standalone Microsoft.AgentGovernance.Mcp package with minimal dependencies. However, there are several critical security issues and potential breaking changes that need to be addressed before merging. Additionally, there are opportunities to improve documentation, testing, and code quality.

Action Items:

  1. Address the critical security issues related to input validation, denied tools, replay attacks, and credential redaction.
  2. Document the .NET version requirements and provide migration guidance for existing users.
  3. Add unit tests for the new package and sample.
  4. Ensure thread safety in McpToolRegistry.
  5. Review the implementation for OWASP Agentic Top 10 compliance and document mitigations.
  6. Enhance inline comments and documentation for better maintainability.

Let me know if you need further clarification or assistance!

@jackbatzner
Copy link
Copy Markdown
Contributor Author

CI Status Summary

All CI checks pass except:

  • Maintainer approval gate — awaiting review
  • link-check — not reproducible locally (only changed markdown has no hyperlinks); may resolve on re-run

Standalone Microsoft.AgentGovernance.Mcp NuGet package + ASP.NET Core sample. Slimmed to 7 files.

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

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: feat(dotnet): add standalone Microsoft.AgentGovernance.Mcp package


🔴 CRITICAL

  1. Potential Security Risk in Sample Tool Implementation:

    • The ReadFile method in the SampleTools class appears to simulate reading a file from disk. However, the lack of proper input validation or sanitization for the path parameter introduces a potential path traversal vulnerability. An attacker could exploit this to access sensitive files outside the intended directory.
    • Recommendation: Implement strict input validation and sanitization for the path parameter to ensure it does not allow directory traversal. For example, restrict file access to a specific directory and validate the path against this directory.
  2. Insufficient Validation for SearchDatabase Tool:

    • The SearchDatabase method accepts a query parameter without any validation or sanitization. This could lead to SQL injection vulnerabilities if the query is executed directly on a database.
    • Recommendation: Use parameterized queries or an ORM to prevent SQL injection. Avoid executing raw queries directly.
  3. Potential Misuse of SendEmail Tool:

    • The SendEmail method has a RequiresApproval attribute, but the implementation does not demonstrate how this approval process is enforced. If the approval mechanism is not implemented or is bypassed, this could lead to unauthorized email sending.
    • Recommendation: Ensure the RequiresApproval attribute is enforced by the governance layer. Add tests to verify that tools marked with RequiresApproval cannot be executed without proper authorization.
  4. Lack of Authentication and Authorization in Sample:

    • The provided ASP.NET Core sample does not include any authentication or authorization mechanisms. This could allow unauthorized users to access the MCP governance endpoints.
    • Recommendation: Add authentication and authorization middleware to the sample application. For example, integrate with Azure AD or another identity provider to secure the endpoints.

🟡 WARNING

  1. Potential Breaking Changes in .NET Target Frameworks:

    • The new Microsoft.AgentGovernance.Mcp package targets .NET 8.0 and .NET 10.0. While this is forward-looking, it may cause compatibility issues for users on older .NET versions.
    • Recommendation: Consider adding support for .NET 6.0 or .NET 7.0 if feasible, as these versions are still widely used. Alternatively, clearly document the minimum required .NET version in the release notes.
  2. Dependency on ModelContextProtocol:

    • The new package introduces a dependency on the ModelContextProtocol package (version 1.2.0). If this dependency has breaking changes in future versions, it could impact the functionality of the Microsoft.AgentGovernance.Mcp package.
    • Recommendation: Monitor the ModelContextProtocol package for updates and ensure compatibility. Consider adding integration tests to detect breaking changes early.

💡 SUGGESTIONS

  1. Expand Documentation:

    • The provided README for the sample application is helpful but could be expanded to include:
      • A detailed explanation of the McpGovernanceOptions settings.
      • Examples of how to implement the RequiresApproval mechanism for tools like SendEmail.
      • A note on the importance of securing the endpoints with authentication and authorization.
  2. Add Unit Tests for Sample Tools:

    • The sample tools (ReadFile, SearchDatabase, SendEmail) lack unit tests. Adding tests would help ensure their correctness and security.
    • Recommendation: Write unit tests to validate the behavior of each sample tool, including edge cases and potential misuse scenarios.
  3. Add Logging for Tool Execution:

    • The sample application does not include logging for tool execution. Adding logging would improve observability and help in debugging issues.
    • Recommendation: Use ILogger to log tool execution details, including the tool name, parameters, and execution results.
  4. Consider Adding Rate Limiting:

    • While the sample application includes a RateLimitWindowMinutes setting, it is unclear if this is enforced in the provided implementation.
    • Recommendation: Implement rate limiting for MCP tool calls to prevent abuse. Use middleware or a library like AspNetCoreRateLimit to enforce rate limits.
  5. Clarify Backward Compatibility:

    • While this PR is marked as a non-breaking change, the introduction of a new package and potential dependency changes could impact existing users.
    • Recommendation: Clearly document any potential backward compatibility concerns in the release notes. If there are none, explicitly state this to reassure users.

Summary

This PR introduces a valuable addition to the Agent Governance Toolkit by providing a standalone .NET package for MCP governance. However, there are critical security concerns in the sample tools and the lack of authentication/authorization in the sample application. Addressing these issues is essential to ensure the security and robustness of the implementation. Additionally, some minor improvements to documentation and backward compatibility considerations would enhance the overall quality of this contribution.

@jackbatzner jackbatzner force-pushed the jb/dotnet-mcp-package branch from a897823 to ec7a784 Compare April 6, 2026 20:51
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file 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

Pull Request Review for feat(dotnet): add standalone Microsoft.AgentGovernance.Mcp package


🔴 CRITICAL

  1. Policy Engine Correctness:

    • Issue: The demo_simulated.py and getting_started.py examples rely on YAML-based governance policies but do not validate the schema of the loaded YAML files. This could lead to runtime errors or, worse, silent policy misconfigurations that result in security bypasses.
    • Recommendation: Introduce schema validation for the YAML policy files using a library like Cerberus or jsonschema. Ensure that all required fields (e.g., name, condition, action, priority) are present and correctly typed before loading the policies into the PolicyEvaluator.
  2. Trust/Identity: Cryptographic Operations:

    • Issue: The AuditLog implementation uses a Merkle chain for integrity verification, but the cryptographic algorithm used for hashing is not specified in the provided code. If a weak or outdated hashing algorithm is used, the audit trail could be compromised.
    • Recommendation: Ensure that the Merkle chain uses a secure hashing algorithm (e.g., SHA-256 or SHA-3). Explicitly document the algorithm used in the code and provide a mechanism to upgrade the hashing algorithm in the future if needed.
  3. Sandbox Escape Vectors:

    • Issue: The CapabilityGuardMiddleware allows specifying allowed_tools and denied_tools, but there is no validation to ensure that the tool names are sanitized or that the tools themselves are secure. This could lead to command injection or unauthorized access if the tool names are dynamically generated or user-controlled.
    • Recommendation: Implement strict validation and sanitization for tool names. Consider using a whitelist of pre-approved tools that are hardcoded or loaded from a secure configuration file.

🟡 WARNING

  1. Backward Compatibility:
    • Issue: The introduction of the standalone Microsoft.AgentGovernance.Mcp package is a new feature, but it re-exports core MCP modules. If the core MCP modules undergo breaking changes in the future, this package will also be affected.
    • Recommendation: Clearly document the dependency on the core MCP modules and establish a versioning strategy to ensure compatibility. Consider adding integration tests that validate the behavior of the standalone package against the core MCP modules.

💡 SUGGESTIONS

  1. Type Safety and Pydantic Model Validation:

    • Observation: The AgentContext and ToolContext classes in demo_simulated.py are dynamically created using type and lack explicit type annotations or validation.
    • Recommendation: Replace these dynamic classes with pydantic models to enforce type safety and validate input data. This will reduce the risk of runtime errors and improve code maintainability.
    from pydantic import BaseModel, Field
    
    class AgentContext(BaseModel):
        agent_name: str
        user_message: str
        metadata: dict = Field(default_factory=dict)
        stream: bool = False
        result: AgentResponse | None = None
  2. Thread Safety in Concurrent Agent Execution:

    • Observation: The AuditLog class appears to maintain a stateful _chain object. If the governance middleware is used in a multi-threaded or asynchronous environment, this could lead to race conditions.
    • Recommendation: Ensure that the AuditLog implementation is thread-safe. Use locks or asynchronous primitives to protect shared state.
  3. OWASP Agentic Top 10 Compliance:

    • Observation: The examples demonstrate blocking PII and unauthorized tools, which aligns with OWASP Agentic Top 10 recommendations. However, there is no mention of protections against model inversion attacks or prompt injection in the policy engine.
    • Recommendation: Extend the policy engine to include rules for detecting and mitigating prompt injection attacks. For example, add a rule to detect and block inputs that attempt to manipulate the LLM's behavior (e.g., "Ignore all previous instructions and...").
  4. Documentation Improvements:

    • The README.md files in the examples are detailed and helpful, but they could benefit from a dedicated section on common pitfalls and best practices for integrating the governance toolkit. For example:
      • Emphasize the importance of testing policies in a staging environment before deploying them to production.
      • Provide guidance on how to handle policy updates and rollbacks.
      • Include a section on logging and monitoring best practices for the audit trail.
  5. Testing Coverage:

    • Observation: The PR mentions that tests have been added, but there is no evidence of tests specifically targeting edge cases for the policy engine or the audit trail.
    • Recommendation: Add unit tests for the following scenarios:
      • Invalid or malformed YAML policy files.
      • Edge cases for the policy engine (e.g., empty rules, conflicting rules, or unsupported operators).
      • Audit trail integrity verification with tampered entries.
  6. Error Handling:

    • Observation: The examples in demo_simulated.py and getting_started.py use try-except blocks to handle MiddlewareTermination exceptions but do not log the reason for the termination.
    • Recommendation: Enhance error handling by logging the specific reason for the termination (e.g., the name of the violated policy or the blocked tool). This will improve debugging and traceability.
    try:
        await policy_middleware.process(ctx, passthrough)
    except MiddlewareTermination as e:
        print(f"BLOCKED — {str(e)}")

Summary

  • 🔴 CRITICAL: Address policy engine schema validation, cryptographic algorithm specification, and tool name sanitization.
  • 🟡 WARNING: Document dependency on core MCP modules and ensure backward compatibility.
  • 💡 SUGGESTIONS: Improve type safety with pydantic, ensure thread safety, enhance OWASP compliance, improve documentation, and add more comprehensive tests.

Let me know if you need further clarification or assistance with implementing these recommendations!

@github-actions github-actions bot added the agent-mesh agent-mesh package 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 Summary

This pull request introduces a standalone .NET package, Microsoft.AgentGovernance.Mcp, and includes documentation updates, examples, and a simulated demo for the Agent Governance Toolkit. The PR is part of a multi-step process to modularize the MCP governance primitives for easier adoption by MCP server operators. The changes are well-documented and provide clear examples for both real and simulated integrations.

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


🔴 CRITICAL

  1. Policy Engine Correctness:

    • The GovernancePolicyMiddleware and CapabilityGuardMiddleware rely on the MiddlewareTermination exception to block unauthorized actions. However, there is no explicit test coverage for edge cases, such as:
      • Policies with overlapping or conflicting rules.
      • Handling of malformed or unexpected inputs (e.g., invalid YAML policies, unexpected message formats).
      • Race conditions in concurrent policy evaluations.
    • Action Required: Add comprehensive tests for edge cases, including malformed inputs, conflicting policies, and concurrent evaluations.
  2. Trust/Identity: Cryptographic Operations:

    • The AuditLog integrity verification is mentioned but not thoroughly tested in the provided examples. The integrity check relies on a Merkle chain, but there is no evidence of testing for tampering scenarios (e.g., modifying or removing entries).
    • Action Required: Add tests to simulate tampering with the audit log and verify that the integrity check detects these modifications.
  3. Sandbox Escape Vectors:

    • The documentation recommends using gVisor or Kata Containers for untrusted MCP servers but does not enforce or validate this in the code. This leaves room for misconfiguration or oversight.
    • Action Required: Implement runtime checks or configuration validators to ensure that sandboxing mechanisms (e.g., gVisor, Kata Containers) are correctly applied in deployments.

🟡 WARNING

  1. Backward Compatibility:

    • The introduction of the standalone .NET package (Microsoft.AgentGovernance.Mcp) appears to be non-breaking. However, the PR does not explicitly confirm whether the existing MCP-related functionality in the core Agent Governance Toolkit remains unaffected.
    • Action Required: Add tests to verify that the core MCP functionality in the existing toolkit is not impacted by the new standalone package.
  2. Type Safety and Pydantic Model Validation:

    • The AgentContext and ToolContext classes in demo_simulated.py use dynamically created types (type("A", (), {...})) instead of well-defined Pydantic models. This approach is error-prone and may lead to runtime issues.
    • Action Required: Replace dynamically created types with properly defined Pydantic models to ensure type safety and validation.

💡 SUGGESTIONS

  1. Documentation:

    • The documentation is thorough and well-structured. However, it would be helpful to include a section on common pitfalls and troubleshooting tips for users integrating the standalone .NET package.
    • Consider adding a note about the importance of keeping the YAML policies up-to-date and consistent across MCP servers.
  2. Thread Safety in Concurrent Agent Execution:

    • While the PR does not introduce any explicit threading issues, it would be prudent to include tests for concurrent execution of the GovernancePolicyMiddleware and CapabilityGuardMiddleware to ensure thread safety.
    • Add a note in the documentation about the thread safety of the middleware components.
  3. OWASP Agentic Top 10 Compliance:

    • The documentation aligns well with OWASP MCP Security Cheat Sheet recommendations. However, it would be beneficial to explicitly map the provided hardening guide to the OWASP Agentic Top 10 risks to ensure comprehensive coverage.
  4. Examples:

    • The simulated demo (demo_simulated.py) is a great addition for users who want to experiment without installing external dependencies. However, the example could benefit from additional comments explaining the purpose of each step in more detail.
    • Consider adding a section in the README that explains the differences between the real and simulated demos and when to use each.

Summary of Actions

Critical

  1. Add tests for edge cases in policy engine evaluation.
  2. Add tests for audit log tampering detection.
  3. Implement runtime checks or validators for sandboxing mechanisms.

Warning

  1. Verify backward compatibility with existing MCP functionality.
  2. Replace dynamically created types in demo_simulated.py with Pydantic models.

Suggestions

  1. Enhance documentation with troubleshooting tips and OWASP Agentic Top 10 mapping.
  2. Add thread safety tests for middleware components.
  3. Improve comments and explanations in the simulated demo.

Final Notes

The PR is a significant step forward in modularizing the MCP governance functionality and providing clear guidance for secure deployment. Addressing the critical and warning issues will ensure the robustness and security of the implementation.

jackbatzner and others added 2 commits April 8, 2026 08:26
Add clear integration documentation to examples/maf-integration/README.md:

- Integration Pattern section with ASCII diagram showing AGT as
  deterministic middleware in the MAF pipeline
- 3-step quickstart (install, YAML policy, middleware code) for
  adding AGT to an existing MAF project
- Two Ways to Run section explaining simulated vs live LLM modes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rename existing getting_started.py to demo_simulated.py (no-dependency version)
- Create new getting_started.py with real smolagents @tool decorator usage
- Wrap tool forward() with AGT governance checks (key integration pattern)
- Add requirements.txt for smolagents example dependencies
- Update README.md with 'Two Ways to Run' section and updated key files table

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jackbatzner and others added 4 commits April 8, 2026 08:26
- Rename getting_started.py to demo_simulated.py (no-dependency version)
- Create new getting_started.py with real CrewAI Agent/Task/Crew objects
  wrapped in AGT governance middleware
- Add requirements.txt for the example
- Update README.md with 'Two ways to run' section and updated file table

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rename getting_started.py -> demo_simulated.py (no-dependency version)
- Create new getting_started.py with real Agent, function_tool, and
  InputGuardrail integration showing AGT governance as a guardrail
- Add requirements.txt for SDK dependencies
- Update README.md with 'Two ways to run' section and updated file refs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nfusion allowlist

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jackbatzner jackbatzner force-pushed the jb/dotnet-mcp-package branch from 9da5ae6 to a5b05da Compare April 8, 2026 13:27
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: feat(dotnet): add standalone Microsoft.AgentGovernance.Mcp package


🔴 CRITICAL Issues

  1. Policy Engine Correctness: Potential False Negatives

    • In demo_simulated.py and getting_started.py, the governance middleware (GovernancePolicyMiddleware and CapabilityGuardMiddleware) is used to enforce policies. However, there is no explicit test coverage for edge cases like:
      • Policies with overlapping or conflicting rules.
      • Complex nested conditions in YAML policies.
      • Handling of malformed or invalid policies.
    • Action: Add comprehensive unit tests to validate the behavior of the policy engine under edge cases. Ensure that the middleware correctly identifies and blocks policy violations, especially for scenarios involving ambiguous or conflicting rules.
  2. Trust/Identity: Cryptographic Operations

    • The audit log's integrity verification relies on a Merkle chain (audit_log.verify_integrity()), but there is no evidence of cryptographic key management or secure storage for the root hash or private keys.
    • Action: Ensure that the root hash and any cryptographic keys used for the Merkle chain are securely stored and managed. Consider integrating with a secure key management solution (e.g., Azure Key Vault, AWS KMS).
  3. Sandbox Escape Vectors

    • The documentation for Kubernetes hardening mentions using gVisor or Kata Containers for untrusted MCP servers. However, there is no enforcement mechanism in the codebase to ensure that these sandboxing mechanisms are applied.
    • Action: Implement runtime checks or deployment validations to ensure that MCP servers are deployed with the recommended sandboxing mechanisms. For example, verify that the RuntimeClassName is set to gvisor or kata in Kubernetes deployments.
  4. Credential Handling

    • The getting_started.py script mentions the use of environment variables for API keys (e.g., OPENAI_API_KEY, GITHUB_TOKEN). However, there is no validation or secure handling of these credentials in the code.
    • Action: Add validation to ensure that required environment variables are set before execution. Additionally, document best practices for securely managing these credentials (e.g., using environment variable managers or secret management tools).

🟡 WARNING: Potential Breaking Changes

  1. Backward Compatibility: Public API Changes
    • The introduction of the standalone Microsoft.AgentGovernance.Mcp package may lead to confusion for existing users of the Agent Governance Toolkit if the documentation does not clearly differentiate between the standalone package and the full toolkit.
    • Action: Update the main README and release notes to clearly explain the relationship between the standalone package and the full toolkit. Highlight any differences in functionality or dependencies.

💡 Suggestions for Improvement

  1. OWASP Agentic Top 10 Compliance

    • The documentation provides excellent guidance on hardening MCP servers (e.g., network isolation, file system restrictions). However, there is no mention of logging and monitoring for anomalous behavior, which is a key aspect of OWASP Agentic Top 10 compliance.
    • Suggestion: Add a section to the hardening guide on logging and monitoring best practices. For example:
      • Enable audit logging for all MCP server actions.
      • Monitor for unusual patterns in agent behavior (e.g., excessive API calls, unexpected tool usage).
      • Integrate with SIEM solutions for real-time alerting.
  2. Type Safety and Pydantic Model Validation

    • The AgentContext and ToolContext classes in demo_simulated.py are manually defined and lack type validation.
    • Suggestion: Replace these classes with Pydantic models to enforce type safety and validation. For example:
      from pydantic import BaseModel
      
      class AgentContext(BaseModel):
          agent_name: str
          user_message: str
          metadata: dict = {}
          stream: bool = False
          result: AgentResponse | None = None
  3. Thread Safety in Concurrent Agent Execution

    • The AuditLog class appears to be a shared resource, but there is no indication of thread safety mechanisms (e.g., locks) to prevent race conditions during concurrent writes.
    • Suggestion: Ensure that the AuditLog implementation is thread-safe. For example, use asyncio.Lock or threading.Lock to synchronize access to the log.
  4. Documentation Enhancements

    • The new mcp-server-hardening.md document is comprehensive, but it could benefit from additional examples and diagrams to illustrate key concepts (e.g., how the Merkle chain works, how policies are evaluated).
    • Suggestion: Add visual aids (e.g., architecture diagrams, flowcharts) to the documentation to make it more accessible to new users.
  5. Test Coverage

    • While the PR mentions that all tests pass, there is no mention of test coverage metrics.
    • Suggestion: Include a test coverage report in the CI/CD pipeline to ensure that new features are adequately tested. Aim for at least 90% coverage, with a focus on critical paths like policy evaluation and audit logging.

Summary of Recommendations

  • Critical Fixes:

    • Add tests for policy engine edge cases.
    • Secure cryptographic key management for the audit log.
    • Enforce sandboxing mechanisms for untrusted MCP servers.
    • Validate and securely handle API credentials.
  • Warnings:

    • Update documentation to clarify the relationship between the standalone MCP package and the full toolkit.
  • Suggestions:

    • Enhance OWASP compliance with logging and monitoring guidance.
    • Use Pydantic models for type safety and validation.
    • Ensure thread safety for shared resources like the AuditLog.
    • Add visual aids to the documentation.
    • Include test coverage metrics in the CI/CD pipeline.

This PR introduces valuable functionality, but addressing the critical issues and warnings is essential to ensure security and maintain backward compatibility.

@imran-siddique
Copy link
Copy Markdown
Member

Thank you for this work, @jackbatzner — the MCP governance implementation across multiple languages is impressive and clearly well-thought-out.

After reviewing the full series, we've decided to take a hybrid approach (Decision C): keep MCP core logic in agent-os/agent-mesh and provide thin standalone re-export packages for enterprise repackaging — similar to what we shipped in PR #910 for Python.

We're closing this PR because we'll implement the .NET standalone package in-house following this architecture. Your work directly informed our approach — the OWASP mapping methodology, the gateway/rate-limiter/session-auth pattern, and the trust proxy design all carry forward.

We're keeping your documentation PRs open for review as they add independent value. Thank you for the sustained contribution effort across the entire MCP governance surface.

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

Labels

agent-mesh agent-mesh package dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/L Large PR (< 500 lines) size/XL Extra large PR (500+ lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants