feat(dotnet): add MCP security namespace — completes cross-language MCP parity#9
Conversation
CI optimization: - Add paths-ignore for docs to 5 code-only workflows - Add paths filter to Link Check (only run on docs changes) - Add concurrency groups to 7 heavy workflows - Docs-only PRs drop from ~14 checks to ~4 README: - Add v3.1.0 release announcement callout - Add PyPI version badge - Update tutorial count to 31 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Kill switch is no longer placeholder: now implements saga handoff with handoff_success_count tracking (kill_switch.py:69-178) - DeltaEngine verify_chain() is no longer a stub: now performs SHA-256 chain verification (delta.py:67-127) - Move both from Critical/High gaps to new 'Resolved' section - Update Processing Integrity coverage (2 of 4 defects, not 3 of 4) - Update evidence table with current line ranges Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tor, and sanitizer Add AgentGovernance.Mcp namespace implementing full MCP security parity with TypeScript and Rust SDKs: - McpSecurityScanner: tool poisoning, typosquatting, hidden instructions, rug pull, schema abuse, cross-server attack, and description injection detection - McpCredentialRedactor: regex-based redaction of API keys, bearer tokens, connection strings, and secret assignments - McpResponseSanitizer: response scanning for prompt injection tags, imperative phrasing, credential leakage, and exfiltration URLs - McpGateway: policy enforcement pipeline with deny/allow lists, payload sanitization, rate limiting, and human approval gates Includes 46 xUnit tests covering all threat categories. Updates SDK-FEATURE-MATRIX.md to flip .NET MCP Security from — to ✅. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Welcome to the Agent Governance Toolkit! Thanks for your first pull request. |
🤖 AI Agent: breaking-change-detector — Summary🔍 API Compatibility ReportSummaryThis pull request introduces the Findings
Migration GuideNo migration steps are necessary as this pull request does not introduce any breaking changes. Downstream users can adopt the new Additional Notes
✅ No breaking changes detected. |
🤖 AI Agent: docs-sync-checker — Issues Found📝 Documentation Sync ReportIssues Found
Suggestions
SummaryThe PR introduces significant new functionality in the form of the Let me know if you need further assistance with any of these updates! |
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Review Summary
This PR introduces the AgentGovernance.Mcp namespace for the .NET SDK, adding critical features such as McpSecurityScanner, McpCredentialRedactor, McpResponseSanitizer, and McpGateway. It also updates the CI/CD workflows and documentation to reflect these changes. The PR includes 46 new tests, and all 513 tests pass.
The changes are generally well-structured and align with the project's goals of enhancing multi-cloud poisoning (MCP) security capabilities. However, there are some areas that require attention to ensure security, correctness, and maintainability.
🔴 CRITICAL
-
Regex Timeout in
McpCredentialRedactor:- The
RegexTimeoutis set to 200ms, which might be insufficient for complex or maliciously crafted inputs. A short timeout could lead to incomplete processing or denial of service (DoS) vulnerabilities if the regex engine times out prematurely. - Recommendation: Increase the timeout to at least 1 second or implement a mechanism to handle regex timeouts gracefully.
- The
-
Regex Patterns in
McpCredentialRedactor:- The regex patterns used for credential detection are prone to bypasses. For example:
- The
BearerTokenregex assumes tokens are alphanumeric with specific characters and a minimum length of 8. However, tokens can vary in format and length. - The
ApiKeyregex assumes specific keywords likeapi_keyorx-api-key, which can be bypassed by using alternative naming conventions.
- The
- Recommendation: Use a more robust approach for credential detection, such as machine learning models or libraries specifically designed for sensitive data detection (e.g., Microsoft's Presidio or similar tools).
- The regex patterns used for credential detection are prone to bypasses. For example:
-
McpGateway Deny/Allow List Matching:
- The
DenyListandAllowListinMcpGatewayConfigsupport prefix patterns ending with*. However, the implementation of pattern matching is not shown in the diff. If this is implemented using regular expressions or string matching, it could be prone to errors or inefficiencies. - Recommendation: Ensure that the pattern matching logic is robust and well-tested. Consider using a library or well-tested algorithm for pattern matching.
- The
-
Concurrency in
McpGateway:- The
McpGatewayclass is marked as thread-safe, but the implementation of thread safety is not visible in the provided diff. Given the use of shared resources like_rateLimiterand_config, there is a risk of race conditions. - Recommendation: Ensure that all shared resources are properly synchronized or use thread-safe data structures.
- The
-
Rate Limiting in
McpGateway:- The rate-limiting logic is not visible in the diff. If the rate limiter is not implemented securely, it could be bypassed or lead to inconsistent behavior.
- Recommendation: Review the rate-limiting implementation to ensure it is robust against common attacks, such as burst traffic or distributed denial-of-service (DDoS) attacks.
🟡 WARNING
-
Backward Compatibility:
- The addition of the
AgentGovernance.Mcpnamespace introduces new public APIs. While this is an additive change, it is essential to ensure that these APIs do not conflict with existing ones. - Recommendation: Verify that the new APIs are backward-compatible and do not introduce breaking changes for existing users.
- The addition of the
-
Documentation Updates:
- The documentation updates are comprehensive, but the addition of new features like
McpGatewayandMcpCredentialRedactormay require more detailed examples and usage guidelines. - Recommendation: Add detailed usage examples and best practices for the new features in the documentation.
- The documentation updates are comprehensive, but the addition of new features like
💡 SUGGESTIONS
-
Test Coverage:
- While 46 new tests have been added, it is unclear if they cover all edge cases, especially for the regex patterns and
McpGatewaylogic. - Recommendation: Ensure that the test suite includes edge cases for regex patterns, such as unusual or maliciously crafted inputs. Also, test the
McpGatewaypipeline with various configurations and scenarios.
- While 46 new tests have been added, it is unclear if they cover all edge cases, especially for the regex patterns and
-
Error Handling in
McpGateway:- The
McpGatewayclass processes requests through multiple stages (deny-list, allow-list, payload sanitization, etc.). If an error occurs in one stage, it is unclear how the gateway handles it. - Recommendation: Implement and document a clear error-handling strategy for the gateway pipeline. Consider adding tests for error scenarios.
- The
-
Credential Redaction Logging:
- The
McpCredentialRedactordoes not appear to log redaction events. Logging redaction events can be useful for auditing and debugging. - Recommendation: Add optional logging for redaction events, including the types of credentials detected and redacted.
- The
-
Performance Benchmarks:
- The new features, especially
McpCredentialRedactorandMcpGateway, may introduce performance overhead. It is essential to ensure that these components perform well under load. - Recommendation: Conduct performance benchmarks for the new features and optimize them if necessary.
- The new features, especially
-
Cross-Language Consistency:
- The PR aims to achieve cross-language parity for MCP security features. However, it is unclear if the .NET implementation is consistent with other languages (e.g., Python, TypeScript).
- Recommendation: Review the implementations in other languages to ensure consistency in functionality and behavior.
Final Assessment
The PR introduces valuable features that enhance the security and governance capabilities of the toolkit. However, there are critical issues related to regex patterns, timeout handling, and concurrency that need to be addressed to ensure robustness and security. Additionally, some areas require further testing, documentation, and performance evaluation.
- Approval Status: Changes Requested
- Priority: High
🤖 AI Agent: security-scanner — Security Analysis of Pull RequestSecurity Analysis of Pull RequestThis pull request introduces the Findings1. Prompt Injection Defense BypassSeverity: 🔴 CRITICAL Fix:
2. Policy Engine CircumventionSeverity: 🟠 HIGH Fix:
3. Trust Chain WeaknessesSeverity: 🔴 CRITICAL Fix:
4. Credential ExposureSeverity: 🟡 MEDIUM Fix:
5. Sandbox EscapeSeverity: 🔵 LOW Fix:
6. Deserialization AttacksSeverity: 🟠 HIGH Fix:
7. Race ConditionsSeverity: 🔵 LOW Fix:
8. Supply Chain RisksSeverity: 🟠 HIGH Fix:
Summary of Findings
Recommendations
Final Rating:This pull request introduces critical security risks that must be addressed before merging. Immediate fixes are required for prompt injection defense bypass and trust chain weaknesses. Other findings should be prioritized based on their severity. |
Adds AgentGovernance.Mcp namespace with McpSecurityScanner, McpCredentialRedactor, McpResponseSanitizer, and McpGateway. 46 new tests, all 513 pass. Updates SDK-FEATURE-MATRIX.md to flip .NET MCP from — to ✅.