feat(python): add MCP security primitives with OWASP coverage

[jackbatzner]

Description

Adds MCP (Model Context Protocol) security primitives to the Python Agent OS Kernel with full OWASP MCP Security Cheat Sheet coverage (11/12 sections — §11 Consent UI is out of scope for server-side SDKs).

Key additions:

• McpMessageSigner / McpMessageValidator — HMAC-SHA256 signing with LRU nonce replay cache and concurrent replay protection
• McpSessionManager — rate-limited session lifecycle with configurable concurrency limits
• McpToolRegistry — schema-validated tool registration
• McpInputSanitizer — regex-based input validation with fail-closed timeout behavior
• CredentialRedactor — full PEM block + secret pattern redaction for audit-safe logging
• Enterprise patterns: persistence seams (SignerNonceStore, SessionStore), clock injection, fail-closed enforcement

All tests pass (2938 passed, 86 skipped).

Part 1 of 3 — This is the core implementation. See also:

• Part 1: This PR ← merge first
• Part 2: feat(python): add standalone agent-mcp-governance package #829 Standalone agent-mcp-governance package
• Part 3: docs(python): add MCP security documentation and examples #828 Documentation and examples ← merge last

Type of Change

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

Package(s) Affected

• agent-os-kernel

Checklist

• My code follows the project style guidelines (ruff check)
• I have added tests that prove my fix/feature works
• All new and existing tests pass (pytest — 2938 passed, 86 skipped)
• I have updated documentation as needed
• I have signed the Microsoft CLA

Related Issues

Part 1 of 3 (split from original PR for easier review). Followed by #829 and #828.

[github-actions]

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]

🤖 AI Agent: breaking-change-detector — Summary

🔍 API Compatibility Report

Summary

This pull request introduces new security primitives for the Agent OS Kernel, focusing on MCP governance and OWASP compliance. The changes are additive and do not remove or modify existing APIs. No breaking changes were detected.

Findings

Severity	Package	Change	Impact
🔵	agent-os-kernel	Added CredentialRedactor class	New API for credential redaction.
🔵	agent-os-kernel	Added MCPMetricsRecorder protocol	New API for metrics recording.
🔵	agent-os-kernel	Added MCPMetrics and NoOpMCPMetrics	New API for OpenTelemetry metrics integration.
🔵	agent-os-kernel	Added multiple MCP-related classes	New APIs for MCP governance components.
🔵	agent-os-kernel	Updated __init__.py to export new APIs	New exports for MCP governance primitives.

Migration Guide

No migration steps are necessary as no breaking changes were introduced. Downstream users can adopt the new MCP governance primitives without modifying existing code.

Recommendations

• Ensure that the new APIs (CredentialRedactor, MCPMetricsRecorder, etc.) are documented thoroughly.
• Provide examples in the upcoming documentation PR (docs(python): add MCP security documentation and examples #828) to demonstrate usage of the new security primitives.

✅ No breaking changes detected.

[github-actions]

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

📝 Documentation Sync Report

Issues Found

1. ❌ MCPMetricsRecorder in packages/agent-os/src/agent_os/_mcp_metrics.py — missing docstring
2. ❌ NoOpMCPMetrics in packages/agent-os/src/agent_os/_mcp_metrics.py — missing docstring
3. ❌ MCPMetrics in packages/agent-os/src/agent_os/_mcp_metrics.py — missing docstring
4. ❌ CredentialPattern in packages/agent-os/src/agent_os/credential_redactor.py — missing docstring
5. ❌ CredentialMatch in packages/agent-os/src/agent_os/credential_redactor.py — missing docstring
6. ❌ CredentialRedactor in packages/agent-os/src/agent_os/credential_redactor.py — missing docstring for class and methods
7. ⚠️ packages/agent-os/README.md — no mention of the new MCP security primitives or their usage
8. ⚠️ CHANGELOG.md — no entry for the addition of MCP security primitives
9. ⚠️ examples/ — no updates to include examples for new MCP security primitives

Suggestions

• 💡 Add docstrings for the following:
  • MCPMetricsRecorder — explain its purpose and methods like record_decision, record_threats_detected, etc.
  • NoOpMCPMetrics — describe its role as a fallback when OpenTelemetry is unavailable.
  • MCPMetrics — document its initialization and methods for recording metrics.
  • CredentialPattern — describe its attributes and purpose.
  • CredentialMatch — explain its attributes and usage.
  • CredentialRedactor — provide an overview of its purpose and document methods like redact, redact_mapping, find_matches, etc.
• 💡 Update packages/agent-os/README.md to include a section on the new MCP security primitives, their purpose, and how to use them.
• 💡 Add an entry to CHANGELOG.md summarizing the addition of MCP security primitives and their features.
• 💡 Add example usage of MCP security primitives (e.g., MCPMetrics, CredentialRedactor) in the examples/ directory.
• 💡 Ensure all public methods and classes have type hints for parameters and return values.

Additional Notes

• The PR introduces significant new functionality, including security primitives and OpenTelemetry integration. Proper documentation is critical for users to understand and adopt these features.
• The CredentialRedactor class has multiple methods that require detailed explanations of their behavior, especially given their security implications.
• The examples/ directory is currently missing updates to demonstrate the new features.

Please address the above issues and suggestions to ensure the documentation is complete and in sync with the new code.

[github-actions]

🤖 AI Agent: security-scanner — Security Analysis of PR: feat(agent-os): add Python MCP security primitives with OWASP coverage

Security Analysis of PR: feat(agent-os): add Python MCP security primitives with OWASP coverage

This PR introduces several new security primitives to the Python Agent OS Kernel, aiming to enhance the security of the MCP (Model Context Protocol) governance layer. Below is a detailed security analysis of the changes, categorized by potential vulnerabilities.

---

1. Prompt Injection Defense Bypass

Analysis:

• The McpInputSanitizer is mentioned in the PR description as a regex-based input validation mechanism with fail-closed timeout behavior. However, the implementation of McpInputSanitizer is not included in the provided diff. Without reviewing the actual implementation, it is unclear if the regex patterns are robust enough to prevent prompt injection attacks.
• The MCPGateway class includes built-in dangerous parameter patterns for sanitization (e.g., regexes for SSNs, credit card numbers, and shell injection). However, these patterns are hardcoded and may not cover all possible prompt injection vectors, especially if the input is highly obfuscated or encoded.

Risk Level: 🟠 HIGH

• Attack Vector: An attacker could craft inputs that bypass the regex-based sanitization, potentially leading to prompt injection attacks.
• Recommendation:
  • Provide the implementation of McpInputSanitizer for review.
  • Use a more robust input sanitization library or framework that supports advanced techniques like context-aware parsing.
  • Regularly update the regex patterns to account for new attack vectors.
  • Consider integrating a machine learning-based anomaly detection system for input validation.

---

2. Policy Engine Circumvention

Analysis:

• The MCPGateway class enforces governance policies on tool calls, including deny-list checks, parameter sanitization, rate limiting, and human approval for sensitive tools.
• The _evaluate method (truncated in the diff) appears to implement the policy evaluation pipeline. However, the full implementation is not visible, so it is unclear if there are any gaps or race conditions in the policy enforcement logic.
• The approval_callback for sensitive tools is a synchronous callable, which could be a potential bottleneck or point of failure if not implemented securely.

Risk Level: 🟠 HIGH

• Attack Vector: An attacker could exploit gaps in the policy evaluation pipeline to bypass restrictions, such as by exploiting race conditions or unhandled edge cases.
• Recommendation:
  • Provide the full implementation of the _evaluate method for review.
  • Ensure that all policy checks are fail-closed and atomic to prevent race conditions.
  • Validate the approval_callback implementation to ensure it cannot be bypassed or manipulated.

---

3. Trust Chain Weaknesses

Analysis:

• The PR does not include any changes related to SPIFFE/SVID validation, certificate pinning, or other trust chain mechanisms. However, the McpMessageSigner and McpMessageValidator components introduce HMAC-SHA256 signing with nonce replay protection.
• The use of an LRU cache for nonce replay protection is a good practice, but the implementation details (e.g., nonce expiration, cache size) are not provided.

Risk Level: 🟡 MEDIUM

• Attack Vector: If the nonce replay protection is improperly implemented (e.g., insufficient cache size or lack of expiration), attackers could replay signed messages to bypass security controls.
• Recommendation:
  • Ensure that the nonce replay cache has a sufficient size and expiration policy to prevent replay attacks.
  • Provide the implementation of McpMessageSigner and McpMessageValidator for review.

---

4. Credential Exposure

Analysis:

• The CredentialRedactor class is introduced to redact sensitive information (e.g., API keys, tokens, and private keys) from logs and audit payloads. The regex patterns used for detection appear to be comprehensive and cover common credential formats.
• The CredentialRedactor logs the number of redacted values, which could inadvertently leak information about the presence of sensitive data.

Risk Level: 🟡 MEDIUM

• Attack Vector: Logging the number of redacted credentials could provide attackers with information about the presence of sensitive data.
• Recommendation:
  • Avoid logging the number of redacted credentials or ensure that such logs are only accessible to authorized personnel.
  • Regularly review and update the regex patterns to cover new credential formats.

---

5. Sandbox Escape

Analysis:

• No changes in this PR directly relate to sandboxing or process isolation. However, the MCPGateway class includes built-in sanitization for dangerous patterns, such as shell injection commands. These patterns are hardcoded and may not cover all possible sandbox escape vectors.

Risk Level: 🟠 HIGH

• Attack Vector: An attacker could craft inputs that bypass the hardcoded sanitization patterns, potentially leading to sandbox escapes.
• Recommendation:
  • Use a more comprehensive library or framework for detecting and mitigating sandbox escape attempts.
  • Regularly update the dangerous parameter patterns to account for new attack vectors.

---

6. Deserialization Attacks

Analysis:

• The PR does not include any changes related to deserialization. However, the MCPGateway class processes tool call parameters, which could potentially include serialized data.
• If the parameters are deserialized without proper validation, this could lead to deserialization attacks.

Risk Level: 🟠 HIGH

• Attack Vector: An attacker could exploit unsafe deserialization to execute arbitrary code or manipulate data.
• Recommendation:
  • Ensure that all deserialization operations are performed using safe libraries or frameworks.
  • Validate and sanitize all input data before deserialization.

---

7. Race Conditions

Analysis:

• The McpSessionManager includes rate-limited session lifecycle management with configurable concurrency limits. However, the implementation details are not provided.
• The _agent_call_counts dictionary in MCPGateway is used for rate limiting, but it is unclear if this is thread-safe.

Risk Level: 🟠 HIGH

• Attack Vector: An attacker could exploit race conditions in rate limiting or session management to bypass restrictions.
• Recommendation:
  • Use thread-safe data structures (e.g., collections.defaultdict with threading locks) for rate limiting and session management.
  • Provide the implementation of McpSessionManager for review.

---

8. Supply Chain

Analysis:

• The PR introduces a dependency on OpenTelemetry for metrics collection. While OpenTelemetry is a well-known and widely used library, it is important to ensure that the dependency is fetched from a trusted source and that its version is pinned.
• The PR does not include a requirements.txt or pyproject.toml file, so it is unclear if the dependency is pinned.

Risk Level: 🟡 MEDIUM

• Attack Vector: An attacker could exploit dependency confusion or typosquatting to introduce malicious code.
• Recommendation:
  • Pin the OpenTelemetry dependency to a specific version in requirements.txt or pyproject.toml.
  • Use a dependency scanner to detect and mitigate supply chain risks.

---

Summary of Findings

Category	Risk Level	Recommendation
Prompt Injection Defense Bypass	🟠 HIGH	Review McpInputSanitizer and improve regex patterns or use a robust library.
Policy Engine Circumvention	🟠 HIGH	Ensure atomic, fail-closed policy checks and validate approval_callback.
Trust Chain Weaknesses	🟡 MEDIUM	Review nonce replay protection implementation and ensure proper cache handling.
Credential Exposure	🟡 MEDIUM	Avoid logging redaction counts or restrict access to such logs.
Sandbox Escape	🟠 HIGH	Use a comprehensive library for detecting sandbox escape attempts.
Deserialization Attacks	🟠 HIGH	Validate and sanitize all input data before deserialization.
Race Conditions	🟠 HIGH	Use thread-safe data structures for rate limiting and session management.
Supply Chain	🟡 MEDIUM	Pin dependencies and use a dependency scanner to mitigate supply chain risks.

---

Next Steps

1. Provide the full implementation of McpInputSanitizer, McpMessageSigner, McpMessageValidator, and McpSessionManager for review.
2. Address the recommendations for each identified risk.
3. Conduct additional testing to verify the robustness of the new security primitives.

[github-actions]

🤖 AI Agent: contributor-guide — Welcome to the microsoft/agent-governance-toolkit community! 🎉

Welcome to the microsoft/agent-governance-toolkit community! 🎉

Hi @first-time-contributor! Thank you so much for taking the time to contribute to the project. We’re thrilled to have you here and appreciate your effort in enhancing the toolkit. Your work is helping to make this project even better for everyone!

---

What You Did Well 🌟

1. Comprehensive Documentation: Your PR includes detailed updates to the documentation, such as the OWASP mapping, API reference, and the new MCP Tools & Security Guide. This is incredibly helpful for both current and future users of the project.

2. Thorough Explanation: The PR description is detailed and well-structured, providing clear before-and-after comparisons, as well as a breakdown of the OWASP Agentic Top 10 coverage. This makes it easy for reviewers to understand the scope and impact of your changes.

3. Security Enhancements: The addition of the Python MCP security stack is a significant improvement for the project. The new features like MCPResponseScanner, MCPSessionAuthenticator, and MCPMessageSigner are well thought out and address critical security concerns.

4. Testing and Documentation: You’ve added tests to validate your changes and updated the documentation to reflect the new features. This shows great attention to detail and adherence to best practices.

---

Suggestions for Improvement 🛠️

1. Linting: It looks like the checkbox for "My code follows the project style guidelines (ruff check)" is unchecked. We use ruff for linting with the E, F, and W rules enabled. Please run ruff check . locally and address any issues. You can find more details in our CONTRIBUTING.md.

2. Test Placement: While you’ve mentioned adding tests, please ensure they are located in the appropriate directory: packages/{name}/tests/. This helps maintain consistency across the project. If you haven’t already, you can refer to the CONTRIBUTING.md for more details.

3. Commit Messages: We follow the Conventional Commits standard. Your PR title is great (feat(agent-os): add Python MCP OWASP parity), but please ensure all your individual commit messages follow this format. For example:

   • feat(agent-os): add MCPResponseScanner for output safety
   • docs(agent-os): update OWASP mapping for MCP security stack

4. Security Review: Since your PR includes security-sensitive code (e.g., MCPSessionAuthenticator, MCPMessageSigner, and CredentialRedactor), it will undergo an additional layer of scrutiny to ensure robustness. Please double-check that all sensitive operations (e.g., HMAC signing, nonce handling, and credential redaction) are implemented securely.

---

Resources to Help You 🚀

• CONTRIBUTING.md: Our guide for contributing to the project, including coding standards and testing guidelines.
• QUICKSTART.md: A quick guide to get started with the project.
• Ruff Linter: Ruff Documentation for help with linting issues.
• Conventional Commits: Conventional Commits Guide for formatting your commit messages.

---

Next Steps ✅

1. Run ruff check . locally and fix any linting issues.
2. Ensure all tests are placed in the correct directory (packages/agent-os/tests/).
3. Double-check your commit messages for compliance with the Conventional Commits format.
4. If you have any questions or need help, feel free to ask here or check out the resources linked above.

Once you’ve made the updates, push your changes to the same branch, and this pull request will automatically update. We’ll review your changes again and provide feedback if needed.

Thank you again for your contribution — we’re excited to collaborate with you! 😊

[github-actions]

🤖 AI Agent: test-generator — `packages/agent-os/src/agent_os/__init__.py`

🧪 Test Coverage Analysis

packages/agent-os/src/agent_os/__init__.py

• ✅ Existing coverage: This file primarily serves as an entry point for imports and does not contain significant logic. Coverage is not applicable here.
• ❌ Missing coverage: None.
• 💡 Suggested test cases: None required.

---

packages/agent-os/src/agent_os/_mcp_metrics.py

• ✅ Existing coverage: Basic functionality of NoOpMCPMetrics and MCPMetrics classes, including initialization and method calls, appears to be covered.
• ❌ Missing coverage:
  • Behavior when OpenTelemetry is unavailable (_HAS_OTEL = False).
  • Error handling during OpenTelemetry initialization (e.g., exceptions in meter.create_counter).
  • Edge cases for metric recording methods, such as invalid inputs or boundary conditions.
• 💡 Suggested test cases:
  1. test_noop_metrics: Verify that NoOpMCPMetrics methods do not raise errors and perform no operations.
  2. test_metrics_initialization_without_opentelemetry: Simulate the absence of OpenTelemetry and verify that MCPMetrics falls back to NoOpMCPMetrics.
  3. test_metrics_initialization_failure: Simulate exceptions during OpenTelemetry initialization and verify fallback behavior.
  4. test_record_decision_edge_cases: Test record_decision with edge cases, such as empty or malformed inputs.
  5. test_record_threats_detected_edge_cases: Test record_threats_detected with count=0 and negative values.

---

packages/agent-os/src/agent_os/credential_redactor.py

• ✅ Existing coverage: Core functionality of CredentialRedactor methods like redact, redact_mapping, and find_matches appears to be covered.
• ❌ Missing coverage:
  • Edge cases for redact with empty strings, very large strings, or strings with overlapping patterns.
  • Complex nested structures in redact_mapping and redact_data_structure.
  • Performance under high-load scenarios (e.g., large payloads with many matches).
  • False positives and negatives for regex patterns.
• 💡 Suggested test cases:
  1. test_redact_empty_string: Verify that redact returns an empty string for None or empty input.
  2. test_redact_large_string: Test redact with a very large string containing multiple credential patterns.
  3. test_redact_overlapping_patterns: Verify behavior when multiple overlapping patterns match the same substring.
  4. test_redact_mapping_complex_structure: Test redact_mapping with deeply nested dictionaries containing various data types.
  5. test_contains_credentials_false_positive: Verify that contains_credentials does not flag benign strings as containing credentials.
  6. test_find_matches_false_negative: Verify that find_matches correctly identifies all credential patterns in edge cases.

---

packages/agent-os/src/agent_os/mcp_gateway.py

• ✅ Existing coverage: Basic functionality of MCPGateway methods like intercept_tool_call and _evaluate appears to be covered.
• ❌ Missing coverage:
  • Edge cases for _evaluate, such as invalid or malformed inputs.
  • Behavior when approval_callback is None or raises an exception.
  • Handling of built-in dangerous patterns in _evaluate.
  • Concurrency issues with _agent_call_counts and _audit_log.
  • Fail-closed behavior during unexpected errors in _evaluate.
• 💡 Suggested test cases:
  1. test_intercept_tool_call_invalid_inputs: Test intercept_tool_call with invalid or malformed agent_id, tool_name, and params.
  2. test_evaluate_with_no_approval_callback: Verify behavior when approval_callback is None.
  3. test_evaluate_approval_callback_exception: Simulate an exception in approval_callback and verify fail-closed behavior.
  4. test_evaluate_dangerous_patterns: Test _evaluate with inputs matching built-in dangerous patterns.
  5. test_concurrent_access_to_call_counts: Simulate concurrent calls to intercept_tool_call and verify thread safety of _agent_call_counts.
  6. test_concurrent_access_to_audit_log: Simulate concurrent calls to intercept_tool_call and verify thread safety of _audit_log.

---

packages/agent-os/src/agent_os/mcp_message_signer.py

• ✅ Existing coverage: Basic functionality of MCPMessageSigner and MCPMessageValidator methods appears to be covered.
• ❌ Missing coverage:
  • Edge cases for nonce replay protection (e.g., duplicate nonces, expired nonces).
  • Behavior when the SignerNonceStore is unavailable or raises an exception.
  • Handling of malformed or oversized payloads.
• 💡 Suggested test cases:
  1. test_nonce_replay_protection_duplicate_nonce: Verify that duplicate nonces are rejected.
  2. test_nonce_replay_protection_expired_nonce: Verify behavior when a nonce has expired.
  3. test_signer_nonce_store_unavailable: Simulate SignerNonceStore unavailability and verify fail-closed behavior.
  4. test_sign_and_validate_malformed_payload: Test signing and validation with malformed payloads.
  5. test_sign_and_validate_oversized_payload: Test signing and validation with payloads exceeding size limits.

---

packages/agent-os/src/agent_os/mcp_protocols.py

• ✅ Existing coverage: Basic functionality of protocol interfaces like MCPNonceStore, MCPRateLimitStore, and MCPSessionStore appears to be covered.
• ❌ Missing coverage:
  • Edge cases for in-memory implementations (e.g., capacity limits, eviction behavior).
  • Concurrency issues in InMemoryNonceStore, InMemoryRateLimitStore, and InMemorySessionStore.
• 💡 Suggested test cases:
  1. test_in_memory_nonce_store_eviction: Verify eviction behavior when InMemoryNonceStore reaches capacity.
  2. test_in_memory_rate_limit_store_concurrency: Simulate concurrent rate limit checks and updates.
  3. test_in_memory_session_store_concurrency: Simulate concurrent session creation and access.

---

packages/agent-os/src/agent_os/mcp_response_scanner.py

• ✅ Existing coverage: Basic functionality of MCPResponseScanner methods like scan and detect_threats appears to be covered.
• ❌ Missing coverage:
  • Edge cases for scan, such as empty or malformed responses.
  • Handling of large responses or responses with deeply nested structures.
  • False positives and negatives in detect_threats.
• 💡 Suggested test cases:
  1. test_scan_empty_response: Verify behavior when scan is called with an empty response.
  2. test_scan_large_response: Test scan with a very large response containing multiple threats.
  3. test_detect_threats_false_positive: Verify that detect_threats does not flag benign responses as threats.
  4. test_detect_threats_false_negative: Verify that detect_threats correctly identifies all threats in edge cases.

---

packages/agent-os/src/agent_os/mcp_security.py

• ✅ Existing coverage: Basic functionality of security-related methods appears to be covered.
• ❌ Missing coverage:
  • Edge cases for trust scoring (e.g., scores of 0.0 and 1.0).
  • Handling of expired certificates or revoked trust.
• 💡 Suggested test cases:
  1. test_trust_scoring_edge_cases: Verify behavior for trust scores of 0.0 and 1.0.
  2. test_expired_certificate_handling: Test behavior when a certificate has expired.
  3. test_revoked_trust_handling: Test behavior when trust is explicitly revoked.

---

packages/agent-os/src/agent_os/mcp_session_auth.py

• ✅ Existing coverage: Basic functionality of MCPSession and MCPSessionAuthenticator methods appears to be covered.
• ❌ Missing coverage:
  • Edge cases for session lifecycle (e.g., expired sessions, invalid session tokens).
  • Concurrency issues during session creation and validation.
• 💡 Suggested test cases:
  1. test_expired_session_handling: Verify behavior when a session has expired.
  2. test_invalid_session_token: Test behavior when an invalid session token is provided.
  3. test_concurrent_session_creation: Simulate concurrent session creation and verify thread safety.

---

packages/agent-os/src/agent_os/mcp_sliding_rate_limiter.py

• ✅ Existing coverage: Basic functionality of MCPSlidingRateLimiter methods appears to be covered.
• ❌ Missing coverage:
  • Edge cases for rate limiting (e.g., boundary conditions, burst traffic).
  • Concurrency issues during rate limit checks and updates.
• 💡 Suggested test cases:
  1. test_rate_limit_boundary_conditions: Verify behavior at the exact rate limit threshold.
  2. test_rate_limit_burst_traffic: Simulate burst traffic and verify rate limiting behavior.
  3. test_concurrent_rate_limit_checks: Simulate concurrent rate limit checks and updates.

---

packages/agent-os/src/agent_os/policies/async_evaluator.py

• ✅ Existing coverage: Basic functionality of asynchronous policy evaluation appears to be covered.
• ❌ Missing coverage:
  • Boundary conditions for policy evaluation (e.g., conflicting policies, empty policies).
  • Behavior under high concurrency.
• 💡 Suggested test cases:
  1. test_conflicting_policies: Verify behavior when two policies conflict.
  2. test_empty_policy_evaluation: Test behavior when no policies are defined.
  3. test_high_concurrency_policy_evaluation: Simulate high concurrency during policy evaluation.

---

Summary

• High-priority areas for new tests:
  • Edge cases for input validation (e.g., malformed inputs, oversized payloads).
  • Concurrency issues in shared state (e.g., rate limiters, session stores).
  • Fail-closed behavior during unexpected errors.
  • Trust scoring edge cases (e.g., expired certificates, revoked trust).
  • Chaos experiments (e.g., partial failures, cascading failures).

Adding the suggested test cases will significantly improve the robustness and reliability of the MCP security primitives.

[github-actions]

🤖 AI Agent: code-reviewer

Review of PR: feat(agent-os): add Python MCP OWASP parity

Summary

This PR introduces significant enhancements to the agent-os-kernel package by porting MCP security components from the .NET implementation to Python. The changes aim to improve the OWASP Agentic Top 10 compliance, particularly in areas such as tool misuse, response hardening, transport/session integrity, and auditability. The PR also includes updates to documentation and test coverage.

---

🔴 CRITICAL

1. Replay Protection in MCPMessageSigner

   • Issue: The MCPMessageSigner uses a nonce cache for replay protection but does not specify the eviction policy for the cache when it reaches the max_nonce_cache_size. If the cache is full, new nonces may not be stored, potentially allowing replay attacks.
   • Recommendation: Implement a Least Recently Used (LRU) or similar eviction policy for the nonce cache to ensure that it does not grow indefinitely while maintaining replay protection.

2. Credential Redaction

   • Issue: The CredentialRedactor relies on pattern matching to detect and redact sensitive information. This approach may lead to false negatives if new credential patterns are introduced or if the patterns are not comprehensive.
   • Recommendation: Regularly update the credential patterns and consider integrating with a centralized secret detection service or library (e.g., TruffleHog, GitGuardian) for more robust detection.

3. Concurrency in MCPSlidingRateLimiter

   • Issue: The MCPSlidingRateLimiter does not explicitly mention thread safety. If this class is used in a multi-threaded environment, race conditions could occur when multiple threads attempt to update the same agent's rate-limiting state.
   • Recommendation: Use thread-safe data structures or locks to ensure that the rate limiter is safe for concurrent access.

4. Session Token Security

   • Issue: The MCPSessionAuthenticator uses cryptographic session tokens but does not specify the algorithm or key rotation strategy. If the key is compromised, all session tokens could be invalidated.
   • Recommendation: Document the cryptographic algorithm used for session tokens and implement a key rotation mechanism. Consider using a library like cryptography for secure token generation and validation.

5. HMAC Key Management in MCPMessageSigner

   • Issue: The MCPMessageSigner allows the use of a static signing key without enforcing secure storage or rotation.
   • Recommendation: Provide guidance or utilities for securely storing and rotating the HMAC signing key. Consider integrating with a secret management solution like Azure Key Vault or AWS Secrets Manager.

---

🟡 WARNING

1. Potential Breaking Changes

   • The introduction of new classes like MCPGateway, MCPSecurityScanner, and MCPResponseScanner could lead to breaking changes if existing users of the library are expected to adopt these components for MCP-related functionality.
   • Recommendation: Clearly document migration steps for existing users and ensure backward compatibility where possible.

2. API Stability

   • The new classes and methods introduced in this PR significantly expand the public API of agent-os-kernel. Any future changes to these APIs could break existing integrations.
   • Recommendation: Mark these APIs as "experimental" in the documentation if they are subject to change in the near future.

---

💡 SUGGESTIONS

1. Test Coverage

   • While the PR mentions expanded test coverage, it is not clear if all edge cases have been tested, especially for the new security components.
   • Recommendation: Add tests for edge cases, such as:
     • Invalid or malformed inputs to MCPResponseScanner and MCPSecurityScanner.
     • High-concurrency scenarios for MCPSlidingRateLimiter and MCPSessionAuthenticator.
     • Replay attacks and expired tokens for MCPMessageSigner.

2. Documentation Improvements

   • The documentation is comprehensive but could benefit from additional examples, especially for complex use cases involving multiple components (e.g., integrating MCPGateway with MCPSessionAuthenticator and MCPMessageSigner).
   • Recommendation: Add a section in the documentation that provides a complete end-to-end example of securing an MCP integration, including all new components.

3. Performance Considerations

   • The MCPSlidingRateLimiter and MCPMessageSigner involve operations that could become bottlenecks under high load.
   • Recommendation: Benchmark these components under high-concurrency scenarios and optimize as needed. Consider adding metrics for monitoring their performance in production.

4. Backward Compatibility Testing

   • While the PR claims no breaking changes, it's important to verify this claim with automated backward compatibility tests.
   • Recommendation: Add tests to ensure that existing functionality in agent-os-kernel is not affected by the new changes.

5. Type Annotations

   • The new classes and methods include type annotations, which is excellent. However, ensure that these annotations are consistent with the rest of the codebase and are validated using mypy or a similar tool.
   • Recommendation: Run mypy on the updated codebase and address any type-related issues.

6. OpenTelemetry Metrics

   • The PR introduces new OpenTelemetry counters (mcp_decisions, mcp_threats_detected, etc.), but it is unclear if these metrics are being tested.
   • Recommendation: Add tests to verify that these metrics are correctly emitted and contain accurate data.

---

Conclusion

This PR introduces critical security enhancements to the agent-os-kernel package, significantly improving its compliance with the OWASP Agentic Top 10. However, there are some critical issues related to cryptographic operations, concurrency, and credential handling that need to be addressed before merging. Additionally, there are potential breaking changes and areas for improvement in documentation and testing. Once these issues are resolved, this PR will be a strong addition to the project.

[github-actions]

🤖 AI Agent: code-reviewer

Review of PR: feat(agent-os): add Python MCP OWASP parity

General Overview

This pull request introduces significant enhancements to the agent-os-kernel package by porting missing MCP security components from the .NET implementation to Python. The changes aim to improve the OWASP Agentic Top 10 compliance, particularly in areas such as tool misuse, response hardening, transport/session integrity, and auditability. The PR also includes updates to documentation and test coverage.

---

🔴 CRITICAL Issues

1. Replay Protection in MCPMessageSigner

   • The MCPMessageSigner class implements HMAC-SHA256 signing and nonce replay protection. However, the nonce replay protection mechanism relies on a nonce cache with a fixed size (max_nonce_cache_size=10000) and periodic cleanup (nonce_cache_cleanup_interval=timedelta(minutes=10)).
   • Risk: If the cache is full, older nonces may be evicted, potentially allowing replay attacks for evicted nonces within the replay_window.
   • Recommendation: Implement a more robust eviction strategy (e.g., LRU) or dynamically resize the cache to ensure that valid nonces are not prematurely evicted. Additionally, document the implications of the current cache size and cleanup interval for users.

2. Credential Redaction in CredentialRedactor

   • The CredentialRedactor class provides methods to redact sensitive information from strings and nested payloads. However, the effectiveness of the redaction depends on the comprehensiveness of the patterns used to detect credentials.
   • Risk: If the regex patterns used for credential detection are incomplete or overly permissive, sensitive data may be leaked.
   • Recommendation: Include a comprehensive test suite for CredentialRedactor that validates its ability to detect and redact a wide range of credential formats (e.g., API keys, tokens, passwords). Regularly update the patterns to cover new credential formats.

3. Concurrency in MCPSlidingRateLimiter

   • The MCPSlidingRateLimiter class uses a sliding window algorithm to enforce rate limits. However, there is no mention of thread safety in the implementation.
   • Risk: In a multi-threaded or asynchronous environment, race conditions could lead to incorrect rate-limiting behavior.
   • Recommendation: Ensure that the implementation is thread-safe by using appropriate synchronization mechanisms (e.g., locks or thread-safe data structures). Add tests to simulate concurrent access and verify correctness.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Additions
   • This PR introduces several new public classes (MCPGateway, MCPSecurityScanner, MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, CredentialRedactor, MCPSlidingRateLimiter) and methods. While these are non-breaking additions, they set a precedent for the API and should be carefully reviewed for long-term maintainability.
   • Recommendation: Ensure that the new APIs are well-documented, consistent with existing naming conventions, and follow the project's design principles. Consider marking these APIs as "experimental" in the documentation if they are subject to change.

---

💡 Suggestions for Improvement

1. Test Coverage

   • The PR mentions expanded test coverage, but it is not clear if all edge cases are covered, especially for security-critical components like MCPMessageSigner and CredentialRedactor.
   • Suggestion: Add tests for edge cases, such as:
     • Nonce replay attacks with a full cache.
     • Credential redaction for edge-case patterns.
     • Concurrent access to MCPSlidingRateLimiter.

2. Documentation

   • The documentation is comprehensive and well-structured, but it could benefit from additional examples and explanations for certain components.
   • Suggestion: Add more detailed examples for MCPSessionAuthenticator and MCPMessageSigner, including scenarios like session expiration and nonce replay.

3. Backward Compatibility

   • While this PR does not introduce breaking changes, it is important to ensure that the new features do not inadvertently affect existing functionality.
   • Suggestion: Perform a thorough regression test of the entire agent-os-kernel package to ensure that existing features are not impacted.

4. Performance Considerations

   • The new components, especially MCPSecurityScanner and MCPResponseScanner, may introduce performance overhead due to additional scanning and validation steps.
   • Suggestion: Benchmark the performance impact of these components and document any potential trade-offs.

5. OpenTelemetry Integration

   • The PR mentions the addition of new OpenTelemetry counters (mcp_decisions, mcp_threats_detected, etc.). However, there is no information on how these metrics are exposed or consumed.
   • Suggestion: Provide documentation or examples on how users can access and utilize these metrics for monitoring and alerting.

6. Error Handling

   • The PR includes a fix for async_evaluator.py to address timing issues on Windows. However, there is no information on the root cause of the issue or how the fix resolves it.
   • Suggestion: Document the root cause of the timing issue and provide a detailed explanation of the fix in the PR description.

---

Summary

This PR introduces critical security features to the agent-os-kernel package, significantly improving its compliance with the OWASP Agentic Top 10. However, there are some critical issues related to nonce replay protection, credential redaction, and concurrency that need to be addressed. Additionally, there are opportunities to improve test coverage, documentation, and performance benchmarking.

Action Items:

1. Address the critical issues related to nonce replay protection, credential redaction, and concurrency.
2. Add comprehensive tests for edge cases in the new components.
3. Provide additional documentation and examples for new features.
4. Perform regression testing to ensure backward compatibility.
5. Benchmark and document the performance impact of the new components.
6. Clarify the root cause and resolution of the async_evaluator.py timing issue.

Once these issues are addressed, the PR will be ready for approval.

[jackbatzner]

Quick follow-up on the AI bot comments:

• I picked up the one concrete docs item and pushed a changelog update in 8dbf3df.
• For docs-sync-checker: the new public classes already have class docstrings (MCPGateway, MCPSecurityScanner, MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, CredentialRedactor, MCPSlidingRateLimiter), and the README already lists the new MCP security stack explicitly.
• For the security/code-review summaries: the concrete concerns called out are already addressed in code:
  • MCPMessageSigner uses a bounded nonce cache with oldest-entry eviction (popitem(last=False)) plus constant-time signature comparison.
  • MCPSlidingRateLimiter is thread-safe via a bucket map lock and per-bucket locks.
  • MCPSessionAuthenticator enforces the concurrent-session cap under lock and uses opaque random session tokens rather than signed bearer artifacts.
• For test-generator: the suggested coverage for replay detection, expired timestamps, nested redaction, exfiltration detection, metrics emission, and concurrency/session-limit behavior is already present in the new MCP-focused pytest files.

Since these are top-level PR comments / summary reviews rather than inline review threads, there is nothing to resolve in the GitHub thread UI, but I wanted to leave the rationale here so reviewers have a concrete disposition for the generic bot feedback.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces a significant enhancement to the agent-os-kernel package by adding Python MCP (Managed Control Plane) security features to align with the OWASP Agentic Top 10 risks. The PR includes new components for response scanning, session authentication, message signing, credential redaction, and rate limiting. It also updates documentation and expands test coverage.

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

---

🔴 CRITICAL

1. Replay Protection in MCPMessageSigner:

   • The MCPMessageSigner implements HMAC-SHA256 signing and nonce replay protection. However, the nonce replay protection mechanism relies on a cache with a fixed size (max_nonce_cache_size) and a cleanup interval (nonce_cache_cleanup_interval). If the cache size is exceeded or the cleanup interval is not frequent enough, there is a risk of replay attacks.
   • Action: Ensure that the nonce cache is robust against overflow attacks. Consider implementing a strategy to handle cache overflow, such as evicting the oldest entries or using a more scalable storage mechanism. Additionally, document the security implications of the cache size and cleanup interval.

2. Credential Redaction in CredentialRedactor:

   • The CredentialRedactor appears to rely on pattern matching to identify and redact sensitive information. However, there is no mention of how comprehensive the patterns are or whether they are configurable.
   • Action: Ensure that the redaction patterns cover a wide range of credential formats (e.g., API keys, tokens, passwords). Provide a mechanism for users to extend or customize the patterns to suit their specific needs.

3. Concurrency and Thread Safety:

   • The MCPSessionAuthenticator and MCPSlidingRateLimiter maintain in-memory state for session tokens and rate-limiting windows, respectively. There is no mention of thread safety in the implementation.
   • Action: Confirm that these components are thread-safe, especially if they are intended to be used in multi-threaded environments. If not, document the limitations and provide guidance for safe usage.

4. Sandbox Escape Vectors:

   • While the PR mentions that MCPResponseScanner treats tool output as untrusted input, there is no explicit mention of how it handles potential sandbox escape vectors, such as malicious code execution or injection attacks.
   • Action: Review the implementation of MCPResponseScanner to ensure it adequately handles edge cases, such as obfuscated payloads or nested malicious content. Add tests to cover these scenarios.

---

🟡 WARNING

1. Public API Changes:

   • This PR introduces several new public APIs (MCPGateway, MCPSecurityScanner, MCPResponseScanner, etc.). While these are new additions and do not break existing functionality, they set a precedent for future API design.
   • Action: Ensure that the new APIs are well-documented, consistent with existing naming conventions, and designed to be extensible. This will help maintain backward compatibility in future updates.

2. Backward Compatibility:

   • The PR introduces new OpenTelemetry counters (mcp_decisions, mcp_threats_detected, etc.). If existing users are monitoring metrics, these new counters may require updates to their monitoring configurations.
   • Action: Clearly document the new counters in the release notes and provide guidance on how to integrate them into existing monitoring setups.

---

💡 SUGGESTIONS

1. Test Coverage:

   • The PR mentions expanded test coverage, but it is unclear if all edge cases are covered, especially for the new security components.
   • Action: Add tests for edge cases, such as:
     • High-concurrency scenarios for MCPSessionAuthenticator and MCPSlidingRateLimiter.
     • Complex nested payloads for CredentialRedactor.
     • Malicious payloads designed to bypass MCPResponseScanner.

2. Documentation:

   • The documentation updates are comprehensive, but some areas could benefit from additional clarity:
     • Provide examples of how to configure and use the new OpenTelemetry counters.
     • Include a detailed explanation of the nonce replay protection mechanism in MCPMessageSigner and its limitations.
     • Add a section on best practices for integrating the new MCP security components into existing systems.

3. Performance Considerations:

   • The new components (e.g., MCPSlidingRateLimiter, MCPSessionAuthenticator, MCPMessageSigner) may introduce performance overhead, especially in high-throughput scenarios.
   • Action: Benchmark the performance of these components under load and document any potential impact on system performance. Provide guidance on tuning parameters (e.g., cache size, cleanup intervals) for different use cases.

4. Error Handling:

   • The new components introduce several error-handling mechanisms (e.g., PermissionError, RuntimeError). It is important to ensure that these errors are appropriately logged and do not leak sensitive information.
   • Action: Review the error-handling logic to ensure that sensitive information (e.g., details about rejected requests) is not exposed in logs or error messages.

5. Code Style and Consistency:

   • Ensure that all new code adheres to the project's style guidelines. For example, run ruff to verify compliance.
   • Action: Address any style issues flagged by ruff or other linters.

---

Final Assessment

This PR is a significant step forward in aligning the Python MCP implementation with the OWASP Agentic Top 10 risks. However, the following critical issues must be addressed before merging:

• Strengthen nonce replay protection in MCPMessageSigner.
• Ensure comprehensive credential redaction patterns in CredentialRedactor.
• Verify thread safety for MCPSessionAuthenticator and MCPSlidingRateLimiter.
• Review MCPResponseScanner for potential sandbox escape vectors.

Additionally, the potential impact on backward compatibility and monitoring configurations should be documented, and further tests and documentation improvements are recommended.

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

[imran-siddique]

This PR reverts a security fix from our MSRC-111178 audit (PR #731). The temp-file pattern was added intentionally to prevent expression injection via crafted filenames in \changed_files\ output.

While \with:\ args are safer than
un:\ blocks, the lychee-action may shell-expand the arguments internally. The temp-file approach is defense-in-depth.

The MCP OWASP parity code changes look good — please keep those and drop the workflow change.

[github-actions]

🤖 AI Agent: code-reviewer

Review of PR: feat(agent-os): add Python MCP OWASP parity

---

🔴 CRITICAL: Security Issues

1. Replay Protection in MCPMessageSigner:

   • The MCPMessageSigner implementation includes nonce replay protection, but the nonce cache size is limited to 10,000 by default. This could lead to potential vulnerabilities in high-throughput systems where the nonce cache may overflow, allowing replay attacks.
   • Actionable Recommendation: Implement a mechanism to handle nonce cache overflow, such as persisting older nonces to a database or using a rolling hash-based approach to track used nonces.

2. Credential Redaction:

   • The CredentialRedactor implementation appears to rely on pattern matching for detecting and redacting sensitive information. This approach may lead to false negatives (missed sensitive data) or false positives (unnecessary redactions).
   • Actionable Recommendation: Provide a detailed list of supported credential patterns in the documentation and allow users to extend or customize the patterns. Additionally, consider integrating with existing libraries like python-dotenv or python-decouple for better secret management.

3. Session Token Security:

   • The MCPSessionAuthenticator uses cryptographic session tokens but does not specify the algorithm used for token generation or storage. If the tokens are not securely generated or stored, they could be vulnerable to attacks.
   • Actionable Recommendation: Clearly document the cryptographic algorithm used for session token generation and ensure it meets modern security standards (e.g., HMAC-SHA256). Additionally, ensure that tokens are stored securely and are not exposed in logs or error messages.

4. Output Sanitization:

   • The MCPResponseScanner and CredentialRedactor sanitize tool outputs, but there is no indication of how comprehensive the sanitization is, especially for complex or nested data structures.
   • Actionable Recommendation: Add tests to verify the effectiveness of the sanitization process for various edge cases, such as deeply nested structures or obfuscated sensitive data.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes:

   • The addition of new classes and methods (e.g., MCPGateway, MCPSecurityScanner, MCPResponseScanner, etc.) expands the public API of the agent-os package. While these are new features, they could potentially introduce breaking changes if they conflict with existing user implementations.
   • Actionable Recommendation: Clearly document these changes in the release notes and provide migration guides for users who may want to adopt these new features.

2. Behavior Change in test_hash_chain_integrity:

   • The test for hash chain integrity has been updated to check for tampering. While this is a positive change, it may cause existing integrations or tests to fail if they rely on the previous behavior.
   • Actionable Recommendation: Highlight this change in the release notes and provide guidance on how users can update their tests or integrations.

---

💡 Suggestions for Improvement

1. Thread Safety:

   • The MCPSlidingRateLimiter and MCPSessionAuthenticator components appear to maintain in-memory state (e.g., session tokens, rate-limiting data). This could lead to race conditions in multi-threaded or multi-process environments.
   • Actionable Recommendation: Use thread-safe data structures or provide guidance on how to use these components in concurrent environments.

2. Type Safety and Pydantic Models:

   • While the new components seem to use type hints, there is no indication that Pydantic models are being used for input validation (e.g., for MCPResponseScanner or MCPSecurityScanner).
   • Actionable Recommendation: Use Pydantic models for input validation to ensure type safety and prevent potential runtime errors.

3. Test Coverage:

   • While the PR mentions expanded test coverage, it is not clear if all edge cases have been covered, especially for the new security components.
   • Actionable Recommendation: Add tests for edge cases, such as:
     • High-throughput scenarios for MCPSlidingRateLimiter and MCPMessageSigner.
     • Complex or malformed inputs for MCPResponseScanner and CredentialRedactor.
     • Expired or invalid session tokens in MCPSessionAuthenticator.

4. Backward Compatibility:

   • The PR does not explicitly mention whether the new features are opt-in or if they will affect existing users.
   • Actionable Recommendation: Ensure that the new features are opt-in by default and do not impact existing functionality unless explicitly enabled.

5. Documentation:

   • The documentation is comprehensive but could benefit from additional examples and use cases, especially for the new security components.
   • Actionable Recommendation: Include more real-world examples and edge-case scenarios in the documentation to help users understand how to effectively use the new features.

6. Performance Considerations:

   • The new components (e.g., MCPResponseScanner, MCPSlidingRateLimiter) may introduce performance overhead, especially in high-throughput scenarios.
   • Actionable Recommendation: Benchmark the performance of these components and provide guidance on their expected impact in the documentation.

---

Summary

This PR introduces significant improvements to the MCP-related security features in the agent-os-kernel package, addressing several OWASP Agentic Top 10 risks. However, there are critical security concerns related to nonce replay protection, credential redaction, session token security, and output sanitization that need to be addressed before merging. Additionally, potential breaking changes and performance implications should be clearly documented and tested.

[github-actions]

🤖 AI Agent: code-reviewer

Review of PR: feat(agent-os): add Python MCP OWASP parity

---

🔴 CRITICAL: Security Issues

1. Replay Protection in MCPMessageSigner:

   • The MCPMessageSigner introduces HMAC-SHA256 signing, timestamps, and nonce replay detection. However, the implementation details are not visible in the provided diff. Ensure that:
     • Nonces are unique and cannot be reused.
     • Timestamps are validated with a reasonable time window to prevent replay attacks.
     • HMAC keys are securely generated, stored, and rotated periodically.
     • The HMAC implementation uses a secure library (e.g., hmac from Python's standard library) and is resistant to timing attacks.

   Action: Verify the implementation of MCPMessageSigner for proper nonce handling, timestamp validation, and secure HMAC usage.

2. Credential Redaction in CredentialRedactor:

   • The CredentialRedactor is introduced to redact secrets in strings and nested payloads. However, the diff does not provide the implementation details.
     • Ensure that the redaction logic is robust and handles edge cases, such as partial matches, encoded secrets, or secrets embedded in other data structures.
     • Validate that the redaction process does not inadvertently expose sensitive data in logs or error messages.

   Action: Review the CredentialRedactor implementation to ensure it meets security best practices for sensitive data handling.

3. Session Authentication in MCPSessionAuthenticator:

   • The MCPSessionAuthenticator adds agent-bound session tokens, TTL expiry, and concurrent-session limits. However, the implementation is not visible in the diff.
     • Ensure that session tokens are securely generated and stored.
     • Verify that TTL expiry is enforced and cannot be bypassed.
     • Confirm that concurrent-session limits are correctly implemented and tested.

   Action: Review the MCPSessionAuthenticator implementation for secure session management.

4. Sliding-Window Rate Limiting in MCPSlidingRateLimiter:

   • The MCPSlidingRateLimiter introduces per-agent sliding-window rate limiting for MCP traffic. Ensure that:
     • The rate-limiting logic is resistant to bypass (e.g., by using multiple agent identities).
     • The implementation is thread-safe and handles concurrent requests correctly.

   Action: Verify the MCPSlidingRateLimiter implementation for correctness and thread safety.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes:

   • The introduction of new components (MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, CredentialRedactor, MCPSlidingRateLimiter) may affect existing adopters if they rely on MCP-related functionality.
   • Ensure that these changes are backward-compatible and do not break existing integrations.

   Action: Add clear documentation and migration guides for adopters to transition to the new MCP security features.

---

💡 Suggestions for Improvement

1. Test Coverage:

   • The PR mentions expanded test coverage but does not include the test cases for the new MCP components in the provided diff.
   • Ensure that:
     • Unit tests cover all edge cases for MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, CredentialRedactor, and MCPSlidingRateLimiter.
     • Integration tests validate the end-to-end behavior of the MCP security stack.

   Action: Add or verify the presence of comprehensive test cases for all new components.

2. Documentation:

   • The PR mentions updates to the documentation, but the diff does not include the actual changes.
   • Ensure that the documentation:
     • Clearly explains the purpose and usage of each new MCP component.
     • Provides examples of how to integrate the new components into existing workflows.
     • Includes security best practices for MCP adoption.

   Action: Review and enhance the documentation to ensure clarity and completeness.

3. Concurrency and Thread Safety:

   • The new MCP components (e.g., MCPSlidingRateLimiter, MCPSessionAuthenticator) may involve concurrent operations.
   • Ensure that these components are thread-safe and do not introduce race conditions or other concurrency issues.

   Action: Conduct a thorough review of the concurrency model and add tests to verify thread safety.

4. Backward Compatibility Testing:

   • While the PR claims to be non-breaking, it is important to verify this claim through automated tests.
   • Ensure that existing functionality is not affected by the introduction of new MCP components.

   Action: Add backward compatibility tests to validate that existing functionality remains unaffected.

5. Performance Testing:

   • The new MCP components (e.g., MCPSlidingRateLimiter, MCPResponseScanner) may introduce performance overhead.
   • Conduct performance testing to ensure that the new features do not degrade the overall performance of the system.

   Action: Add performance benchmarks for the MCP components and monitor their impact on system performance.

---

Summary of Recommendations

• 🔴 Review the implementations of MCPMessageSigner, CredentialRedactor, MCPSessionAuthenticator, and MCPSlidingRateLimiter for security vulnerabilities.
• 🟡 Ensure backward compatibility and provide migration guides for adopters.
• 💡 Add comprehensive test cases for the new MCP components, including unit, integration, concurrency, and performance tests.
• 💡 Enhance documentation to provide clear guidance on using the new MCP security features.

Please address the critical security issues and provide the implementation details for the new MCP components for further review.

[jackbatzner]

Thanks for the review, @imran-siddique.

I reverted the markdown-link-check workflow change so the PR keeps the temp-file flow from the earlier MSRC fix, and I also merged the latest main into this branch so the PR is up to date.

Appreciate the catch.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces significant enhancements to the agent-os-kernel package by implementing Python MCP OWASP parity, which strengthens the security posture of the system. The PR addresses several OWASP Agentic Top 10 risks, including tool misuse, response hardening, transport/session integrity, and auditability. It also introduces new components such as MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, CredentialRedactor, and MCPSlidingRateLimiter. Additionally, the PR includes updates to documentation and test coverage.

While the PR introduces valuable security features, there are a few areas that require attention to ensure correctness, security, and backward compatibility.

---

🔴 CRITICAL: Security Issues

1. Replay Attack Mitigation in MCPMessageSigner

   • The PR mentions that MCPMessageSigner adds HMAC-SHA256 signing, timestamps, and nonce replay detection. However, the implementation of the nonce replay detection mechanism is not visible in the provided diff. Ensure that:
     • Nonces are unique and cannot be reused.
     • Nonces are stored securely and checked against a database or in-memory store to prevent replay attacks.
     • Nonce expiration is handled appropriately to prevent storage bloat.
   • If this is not implemented or is incomplete, it could lead to a critical vulnerability where attackers can replay signed messages.

2. Credential Redaction

   • The CredentialRedactor is introduced to redact secrets in strings and nested payloads. However, the implementation is not included in the diff. Ensure that:
     • The redaction mechanism is robust and can handle edge cases (e.g., secrets embedded in complex data structures, encoded formats, or obfuscated strings).
     • The redaction logic does not inadvertently expose sensitive data due to incomplete pattern matching or improper handling of edge cases.

3. Session Authentication

   • The MCPSessionAuthenticator introduces agent-bound session tokens, TTL expiry, and concurrent-session limits. Ensure that:
     • Tokens are securely generated using a cryptographically secure random number generator.
     • Token expiration is enforced correctly, and expired tokens are invalidated.
     • Concurrent session limits are implemented in a thread-safe manner to prevent race conditions.

4. Sandbox Escape Vectors

   • The PR mentions improvements to MCP tool-path hardening. However, the diff does not provide details about how these improvements are implemented. Ensure that:
     • The sandboxing mechanism is robust and prevents unauthorized access to the host system.
     • The tool-path hardening measures are comprehensive and prevent directory traversal, symbolic link attacks, and other potential escape vectors.

---

🟡 WARNING: Potential Breaking Changes

1. New Components in MCP

   • The introduction of new components such as MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, CredentialRedactor, and MCPSlidingRateLimiter may introduce breaking changes if existing users rely on the previous behavior of the MCP-related APIs. Ensure that:
     • The changes are backward-compatible, or
     • The breaking changes are clearly documented in the CHANGELOG.md and communicated to users.

2. Updated OWASP Mapping

   • The updated OWASP mapping may require users to update their configurations or workflows. Ensure that the documentation provides clear guidance on how to adapt to these changes.

---

💡 Suggestions for Improvement

1. Test Coverage

   • The PR mentions expanded test coverage, but the diff does not include tests for the new components (MCPResponseScanner, MCPSessionAuthenticator, etc.). Ensure that:
     • Unit tests are added for all new components, covering both expected behavior and edge cases.
     • Integration tests are added to validate the interaction between the new components and the existing system.

2. Documentation

   • The updated documentation should include:
     • Detailed examples of how to use the new MCP components.
     • Guidance on how to configure and enable the new security features.
     • A clear explanation of the changes to the OWASP mapping and their implications for users.

3. Performance Testing

   • The new components, especially MCPSlidingRateLimiter and MCPResponseScanner, may introduce performance overhead. Consider conducting performance testing to ensure that the changes do not negatively impact the system's performance.

4. Thread Safety

   • The PR does not provide details about the thread safety of the new components. Ensure that:
     • Shared resources (e.g., nonce storage, session token store) are accessed in a thread-safe manner.
     • Concurrent execution scenarios are tested to identify potential race conditions.

5. Backward Compatibility

   • If any of the new components or changes affect the public API, consider providing a deprecation path or versioning strategy to minimize disruption for existing users.

---

Summary of Actionable Feedback

1. 🔴 Ensure robust nonce replay protection in MCPMessageSigner.
2. 🔴 Verify the robustness of the CredentialRedactor implementation.
3. 🔴 Ensure secure token generation, expiration, and thread-safe session handling in MCPSessionAuthenticator.
4. 🔴 Validate sandboxing and tool-path hardening measures to prevent escape vectors.
5. 🟡 Document any breaking changes introduced by the new MCP components and OWASP mapping updates.
6. 💡 Add comprehensive unit and integration tests for all new components.
7. 💡 Expand documentation to include detailed guidance on using and configuring the new features.
8. 💡 Conduct performance testing to assess the impact of the new components.
9. 💡 Ensure thread safety in all new components, especially those handling shared resources.

Please address these issues and suggestions to ensure the security, correctness, and usability of the new features. Let me know if you need further clarification or assistance!

[jackbatzner]

Thanks again for the review comments, @imran-siddique.

I followed up by restoring the markdown link workflow file to match main exactly, so that change should no longer show up in the PR diff, and the branch is updated from main as well.

Appreciate the careful review.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This PR introduces significant enhancements to the agent-os-kernel package by implementing Python MCP OWASP parity. The changes include new features for response scanning, session authentication, message signing, credential redaction, rate limiting, and observability. Additionally, the PR updates documentation and includes a minor fix for async_evaluator.py to ensure compatibility with Windows.

The changes are well-documented, and the PR provides a detailed mapping to the OWASP Agentic Top 10 risks, which is commendable. However, there are several areas that require attention to ensure the robustness, security, and maintainability of the code.

---

🔴 CRITICAL

1. Replay Protection in MCPMessageSigner:

   • The PR mentions that MCPMessageSigner adds HMAC-SHA256 signing, timestamps, and nonce replay detection. However, the implementation of replay protection is not shown in the diff. Ensure that the nonce is unique per message and is stored in a secure, tamper-proof manner to prevent replay attacks. If this is not implemented or is improperly implemented, it could lead to security vulnerabilities.

2. Credential Redaction:

   • The CredentialRedactor is a critical component for preventing sensitive data leakage. Ensure that the redaction logic is robust and can handle edge cases, such as encoded or obfuscated credentials. Additionally, verify that the redaction process does not inadvertently remove non-sensitive data or introduce errors.

3. Concurrency and Thread Safety:

   • The introduction of MCPSlidingRateLimiter and MCPSessionAuthenticator raises concerns about thread safety. Ensure that these components are designed to handle concurrent access, especially in multi-threaded or distributed environments. Use thread-safe data structures or synchronization mechanisms as needed.

4. Sandbox Escape Vectors:

   • The PR mentions improvements to MCP tool-path hardening and response scanning. However, the implementation details are not visible in the diff. Verify that the MCPResponseScanner effectively detects and mitigates potential sandbox escape vectors, such as malicious payloads or command injection attempts.

5. HMAC Key Management:

   • The MCPMessageSigner uses HMAC-SHA256 for message signing. Ensure that the key management strategy is secure, with keys stored securely and rotated periodically. Improper key management could compromise the integrity of the signing mechanism.

---

🟡 WARNING

1. Backward Compatibility:

   • The PR introduces several new components (MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, etc.). While these are new features and should not break existing functionality, ensure that the changes do not inadvertently alter the behavior of existing APIs or introduce regressions. Comprehensive testing is critical.

2. Public API Changes:

   • If any of the new components or changes affect the public API, ensure that these changes are backward-compatible or properly documented in the CHANGELOG.md. Additionally, consider adding deprecation warnings for any APIs that are being replaced or modified.

---

💡 SUGGESTIONS

1. Test Coverage:

   • The PR mentions expanded test coverage, but the diff does not include tests for the new MCP-related components. Ensure that there are comprehensive unit and integration tests for MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, CredentialRedactor, and MCPSlidingRateLimiter. Pay special attention to edge cases and error handling.

2. Performance Testing:

   • The new MCPSlidingRateLimiter and MCPResponseScanner could introduce performance overhead. Conduct performance testing to ensure that these components do not significantly impact the overall system performance, especially under high load.

3. Documentation:

   • While the PR mentions updates to documentation, ensure that the new components are thoroughly documented, including their purpose, usage, and configuration options. This will help adopters understand and use the new features effectively.

4. Code Style and Linting:

   • Ensure that the new code adheres to the project's style guidelines and passes ruff checks. This will help maintain code quality and consistency across the project.

5. Dependency Management:

   • If the new components introduce additional dependencies, ensure that these dependencies are secure, actively maintained, and compatible with the project's supported Python versions (3.9-3.12).

6. Error Handling:

   • Review the error handling in the new components to ensure that they fail securely and provide meaningful error messages. Avoid exposing sensitive information in error messages.

---

Final Notes

This PR introduces critical security enhancements to the agent-os-kernel package, addressing several OWASP Agentic Top 10 risks. While the changes are well-documented and appear to be well-implemented, the following areas require further review and testing:

• Replay protection in MCPMessageSigner
• Credential redaction logic
• Thread safety in new components
• Sandbox escape vectors
• Key management for HMAC signing

Additionally, ensure that the changes are backward-compatible and that the new components are thoroughly tested and documented.

[github-actions]

🤖 AI Agent: code-reviewer

Code Review for PR: feat(agent-os): add Python MCP OWASP parity

General Overview

This PR introduces significant enhancements to the agent-os-kernel package, primarily focusing on achieving OWASP Agentic Top 10 parity for the Python MCP (Multi-Channel Protocol) implementation. The changes include new security components, OpenTelemetry counters, expanded test coverage, and updated documentation. The PR also addresses a minor timing issue in async_evaluator.py to ensure consistent test results on Windows.

---

Key Areas of Review

1. Policy Engine Correctness

• 🔴 CRITICAL: Ensure that the MCPResponseScanner correctly identifies and flags malicious responses, such as instruction tags, imperative overrides, credential leakage, and suspicious exfiltration URLs. Any gaps here could lead to security bypasses.

  • Actionable Feedback: Provide detailed unit tests for MCPResponseScanner that cover edge cases, such as obfuscated or encoded malicious payloads, to ensure robust detection.
  • Actionable Feedback: Consider adding logging for detected threats to aid in debugging and auditing.

• 💡 SUGGESTION: Include a mechanism to update the detection rules for MCPResponseScanner dynamically (e.g., from a central policy server) to adapt to evolving threats.

---

2. Trust/Identity: Cryptographic Operations

• 🔴 CRITICAL: The MCPMessageSigner uses HMAC-SHA256 for message signing. Ensure that the implementation uses a secure key management system and that keys are rotated periodically.

  • Actionable Feedback: Verify that the HMAC key is securely stored and not hardcoded or exposed in the codebase. If not already implemented, integrate with a secure key management solution (e.g., Azure Key Vault).
  • Actionable Feedback: Add tests to verify the integrity of signed messages, including scenarios where tampering occurs.

• 💡 SUGGESTION: Consider adding support for asymmetric cryptography (e.g., RSA or ECDSA) for message signing to enable non-repudiation.

---

3. Sandbox Escape Vectors

• 🔴 CRITICAL: The MCPSessionAuthenticator introduces agent-bound session tokens with TTL expiry and concurrent-session limits. Ensure that these tokens are securely generated, stored, and validated to prevent unauthorized access or session hijacking.

  • Actionable Feedback: Confirm that the session tokens are cryptographically secure (e.g., use a library like secrets or cryptography for token generation).
  • Actionable Feedback: Verify that the session tokens are invalidated upon expiry or session termination.

• 💡 SUGGESTION: Add tests to simulate potential session hijacking attempts and ensure that the MCPSessionAuthenticator correctly rejects invalid or expired tokens.

---

4. Thread Safety in Concurrent Agent Execution

• 🔴 CRITICAL: The MCPSlidingRateLimiter introduces per-agent sliding-window rate limiting for MCP traffic. Ensure that the implementation is thread-safe and handles concurrent requests correctly.
  • Actionable Feedback: Review the implementation of MCPSlidingRateLimiter to ensure it uses thread-safe data structures or synchronization mechanisms (e.g., locks or asyncio primitives) to prevent race conditions.
  • Actionable Feedback: Add stress tests with high concurrency to validate the rate limiter's behavior under load.

---

5. OWASP Agentic Top 10 Compliance

• This PR makes significant progress in addressing several OWASP Agentic Top 10 risks, particularly ASI02 (Tool Misuse & Exploitation), ASI07 (Insecure Inter-Agent Communication), and ASI09 (Human-Agent Trust Exploitation).
  • 💡 SUGGESTION: For ASI07, consider adding encryption for MCP messages in transit (e.g., using TLS) to further enhance transport security.
  • 💡 SUGGESTION: For ASI09, ensure that the audit logs are tamper-evident (e.g., by using cryptographic hash chains) and include tests to verify their integrity.

---

6. Type Safety and Pydantic Model Validation

• The PR does not explicitly mention updates to Pydantic models. However, given the introduction of new components like MCPResponseScanner, MCPSessionAuthenticator, and MCPMessageSigner, it's crucial to ensure that any associated data models are properly validated.
  • 💡 SUGGESTION: If not already done, define Pydantic models for any new data structures introduced in this PR and validate them rigorously.

---

7. Backward Compatibility

• 🟡 WARNING: The introduction of new components like MCPResponseScanner, MCPSessionAuthenticator, and MCPMessageSigner could potentially impact existing integrations if they modify existing APIs or behaviors.
  • Actionable Feedback: Clearly document any changes to existing APIs in the CHANGELOG.md and ensure that all dependent packages are updated accordingly.
  • Actionable Feedback: Add deprecation warnings if any existing functionality is being replaced or modified.

---

8. Test Coverage

• The PR includes expanded test coverage, which is commendable. However, there are some areas where additional tests could be beneficial:
  • 💡 SUGGESTION: Add tests for edge cases in MCPResponseScanner, such as handling of large payloads, malformed responses, and responses with mixed safe and unsafe content.
  • 💡 SUGGESTION: Add tests for the MCPSlidingRateLimiter to verify its behavior under various traffic patterns, including burst traffic and sustained high loads.

---

9. Documentation

• The PR includes updates to the documentation, including a detailed OWASP mapping and guidance for adopting the new MCP security components.
  • 💡 SUGGESTION: Include examples of how to use the new components (e.g., MCPResponseScanner, MCPSessionAuthenticator, etc.) in the documentation to help adopters understand their usage.

---

Summary of Feedback

🔴 CRITICAL

1. Verify the correctness of MCPResponseScanner to prevent false negatives in detecting malicious responses.
2. Ensure secure key management and rotation for MCPMessageSigner.
3. Validate the security of session tokens in MCPSessionAuthenticator.
4. Ensure thread safety in MCPSlidingRateLimiter.

🟡 WARNING

1. Document any breaking changes to existing APIs and add deprecation warnings if necessary.

💡 SUGGESTIONS

1. Add dynamic rule updates for MCPResponseScanner.
2. Consider using asymmetric cryptography for MCPMessageSigner.
3. Add encryption for MCP messages in transit (e.g., TLS).
4. Ensure audit logs are tamper-evident and include integrity tests.
5. Define and validate Pydantic models for new data structures.
6. Expand test coverage for edge cases and concurrency scenarios.
7. Include usage examples for new components in the documentation.

---

By addressing the critical issues and suggestions outlined above, this PR can significantly enhance the security and robustness of the agent-os-kernel package while maintaining backward compatibility.

[jackbatzner]

Enterprise Patterns Added

• Persistence seams — new mcp_protocols.py with protocol abstractions for sessions, nonces, rate limits, and audit storage. In-memory defaults, swappable to Redis/DB.
• Clock/nonce injection — no more hardcoded time.time() or uuid.uuid4() in security classes; all injected via constructor for deterministic testing.
• Redaction-safe audit — gateway audit entries run through CredentialRedactor before storage; response scanner stores credential-type labels only, not matched secrets.
• Fail-closed enforcement — verified: all security gates deny on error with no silent pass-throughs.

Standalone Package

• New packages/agent-mcp-governance/ with pyproject.toml (dependencies = []), zero AGT dependency.
• README with installation, quick-start examples (gateway, rate limiter, session auth), and OWASP MCP Cheat Sheet link.

Validation

• python -m pytest packages/agent-os/tests/ -v ✅ all passing
• MIT license headers on all new .py files ✅
• No CRLF/LF diff inflation ✅
• Self-review checklist passed (no secrets, no TODO/HACK, fail-closed, redaction, seams, clock/nonce, trailing newlines) ✅

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This PR introduces significant enhancements to the agent-os-kernel package by adding Python MCP OWASP parity features. The changes include new components for response scanning, session authentication, message signing, credential redaction, rate limiting, and observability metrics. The PR also updates documentation and includes a minor fix for async_evaluator.py to ensure compatibility with Windows.

The changes are well-documented and provide a detailed mapping to the OWASP Agentic Top 10 risks. The PR also includes updates to the test suite, which is critical for verifying the correctness of the new features.

Key Observations

🔴 CRITICAL

1. Replay Protection in MCPMessageSigner:

   • The PR mentions adding HMAC-SHA256 signing, timestamps, and nonce replay detection to MCPMessageSigner. However, there is no evidence in the diff that the nonce replay detection mechanism is implemented or tested. Without proper replay protection, attackers could potentially reuse signed messages to execute unauthorized actions.
   • Action: Ensure that the nonce replay detection mechanism is implemented and thoroughly tested. Add unit tests to verify that replayed messages are correctly rejected.

2. Credential Redaction:

   • The CredentialRedactor is introduced to redact secrets in nested payloads. However, there is no evidence of tests that validate its effectiveness in handling edge cases, such as obfuscated or encoded secrets.
   • Action: Add tests to verify that the CredentialRedactor can handle edge cases, such as secrets embedded in different formats (e.g., Base64-encoded strings).

3. Concurrency and Thread Safety:

   • The PR introduces MCPSlidingRateLimiter and MCPSessionAuthenticator, which likely involve concurrent operations. However, there is no evidence of tests or documentation addressing thread safety or potential race conditions.
   • Action: Review the implementation of these components to ensure thread safety. Add tests to simulate concurrent access and verify correct behavior under load.

🟡 WARNING

1. Backward Compatibility:
   • The PR introduces new components (MCPResponseScanner, MCPSessionAuthenticator, etc.) and modifies existing functionality. While these changes are described as non-breaking, there is a risk of subtle API changes or behavioral differences that could impact existing users.
   • Action: Clearly document any changes to existing APIs or behavior. Consider adding deprecation warnings if any existing functionality is being replaced or modified.

💡 SUGGESTIONS

1. Test Coverage:

   • While the PR includes extensive tests, it would be beneficial to add more tests for edge cases, such as:
     • Large payloads for MCPResponseScanner and CredentialRedactor.
     • High-concurrency scenarios for MCPSlidingRateLimiter and MCPSessionAuthenticator.
     • Malformed or adversarial inputs to the new components.
   • Action: Expand test coverage to include these scenarios.

2. Documentation:

   • The PR updates documentation to include the new MCP security stack and OWASP mapping. However, it would be helpful to include a quickstart guide or example usage for the new components (MCPResponseScanner, MCPSessionAuthenticator, etc.).
   • Action: Add example usage for the new components in the documentation to help adopters understand how to integrate them.

3. Error Handling:

   • The PR mentions a timing fix in async_evaluator.py but does not provide details. If this fix involves changes to error handling, it should be documented and tested.
   • Action: Ensure that the timing fix in async_evaluator.py is well-documented and tested.

4. Code Style and Linting:

   • The PR does not confirm that the code passes ruff checks.
   • Action: Run ruff to ensure the code adheres to the project's style guidelines.

5. Performance Considerations:

   • The introduction of new components like MCPResponseScanner and MCPSlidingRateLimiter could impact performance, especially under high load.
   • Action: Conduct performance testing to ensure that the new components do not introduce significant latency or resource overhead.

6. Type Annotations:

   • The PR does not mention whether the new components are fully type-annotated.
   • Action: Ensure that all new code includes type annotations and that mypy checks pass.

Conclusion

The PR introduces valuable security enhancements to the agent-os-kernel package, addressing several OWASP Agentic Top 10 risks. However, there are critical gaps in replay protection, credential redaction testing, and concurrency handling that must be addressed before merging. Additionally, more comprehensive testing and documentation would improve the robustness and usability of the new features.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces significant new functionality to the agent-os-kernel package, enhancing its compliance with the OWASP Agentic Top 10 by porting MCP security features from the .NET implementation to Python. The PR also includes updates to documentation and test coverage, as well as a minor fix to async_evaluator.py for Windows compatibility.

The changes are well-documented and address several critical security concerns, including tool misuse, transport/session integrity, and auditability. However, there are a few areas that require attention to ensure the robustness, correctness, and security of the implementation.

---

🔴 CRITICAL

1. Replay Attack Mitigation in MCPMessageSigner

   • The PR mentions that MCPMessageSigner adds HMAC-SHA256 signing, timestamps, and nonce replay detection. However, the implementation of replay protection is not visible in the provided diff. Ensure that:
     • Nonces are unique and stored securely for each session.
     • Nonces are checked for reuse to prevent replay attacks.
     • Timestamps are validated to prevent replay attacks within a short time window.
   • If these mechanisms are not implemented or are incomplete, this could lead to a critical security vulnerability.

2. Credential Redaction in CredentialRedactor

   • The CredentialRedactor is described as redacting common secrets across strings and nested payloads. However, the effectiveness of this feature depends on the comprehensiveness of the patterns it matches. Ensure that:
     • The redaction logic covers a wide range of credential patterns (e.g., API keys, passwords, tokens).
     • The redaction logic is tested against edge cases, such as obfuscated or encoded credentials.
     • There is a mechanism to update the redaction patterns as new types of credentials emerge.

3. Concurrency and Thread Safety

   • The introduction of MCPSlidingRateLimiter and MCPSessionAuthenticator raises concerns about thread safety and race conditions, especially in concurrent agent execution scenarios. Ensure that:
     • Shared resources (e.g., session tokens, rate-limiting counters) are properly synchronized.
     • Unit tests and integration tests include scenarios with high concurrency to validate thread safety.

4. Sandbox Escape Vectors

   • The PR mentions improvements to MCP tool-path hardening and response scanning. However, the implementation details are not visible in the diff. Ensure that:
     • The MCPResponseScanner effectively detects and mitigates potential sandbox escape vectors, such as malicious payloads or command injection attempts.
     • The scanner is tested against a comprehensive set of attack vectors.

---

🟡 WARNING

1. Backward Compatibility

   • The addition of new classes like MCPResponseScanner, MCPSessionAuthenticator, and MCPMessageSigner appears to be non-breaking. However, ensure that:
     • Existing public APIs are not affected by these changes.
     • Any changes to existing APIs are clearly documented in the changelog and migration guides.

2. Behavioral Changes in Rate Limiting

   • The introduction of MCPSlidingRateLimiter may alter the behavior of rate limiting for existing users. Ensure that:
     • The new rate-limiting behavior is backward-compatible or clearly documented as a breaking change.
     • Users are provided with migration guidance if necessary.

---

💡 SUGGESTIONS

1. Test Coverage

   • While the PR mentions expanded test coverage, ensure that:
     • All new functionality (e.g., MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, CredentialRedactor, MCPSlidingRateLimiter) is thoroughly tested with unit and integration tests.
     • Tests include edge cases, such as malformed inputs, high concurrency, and potential attack vectors.

2. Documentation

   • The documentation updates are a positive addition. However, consider adding:
     • Examples of how to use the new MCP security features in real-world scenarios.
     • A detailed explanation of how the new features address specific OWASP Agentic Top 10 risks.

3. Code Quality

   • Run ruff to ensure the code adheres to the project's style guidelines.
   • Review the async_evaluator.py timing fix to ensure it doesn't introduce any unintended side effects.

4. Performance Testing

   • The new rate-limiting and session authentication features may introduce performance overhead. Consider conducting performance tests to measure the impact and optimize as needed.

5. Error Handling

   • Ensure that all new components (e.g., MCPResponseScanner, MCPSessionAuthenticator) have robust error handling and logging to aid in debugging and incident response.

---

Final Recommendation

This PR introduces critical security enhancements and aligns the Python implementation with the .NET version for MCP-related OWASP compliance. However, the following actions should be taken before merging:

• Address the critical issues related to replay attack mitigation, credential redaction, thread safety, and sandbox escape vectors.
• Verify backward compatibility and document any breaking changes.
• Expand test coverage and conduct performance testing.
• Ensure all code adheres to the project's style guidelines and includes robust error handling.

Once these issues are resolved, the PR will be ready for merging.

[imran-siddique]

Great work on the MCP OWASP parity implementation! Strong crypto practices and fail-closed design throughout. Two blocking items:

Blocking:

1. CLA not signed — please sign the Microsoft CLA
2. agent-mcp-governance package declares zero dependencies but imports agent_os — any standalone pip install will crash with ModuleNotFoundError. Either add agent-os-kernel to dependencies or document this as a monorepo-only re-export package.

Should fix:

• Nonce store eviction under pressure could enable replay attacks (evicts valid nonces when full)
• list.pop(0) in rate limiter is O(n) per eviction — use collections.deque.popleft()
• requires-python >= 3.12 narrows the 3.10+ support matrix without documentation

Core agent-os modules are well-implemented. See automated review for details.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces significant enhancements to the agent-os-kernel package, focusing on achieving OWASP Agentic Top 10 parity for the Python MCP implementation. It includes new security features such as MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, CredentialRedactor, and MCPSlidingRateLimiter. Additionally, it improves observability with new metrics and updates documentation to reflect these changes. The PR also includes a minor timing fix for async_evaluator.py to ensure compatibility with Windows.

While the PR addresses critical security concerns and improves the robustness of the MCP stack, there are a few areas that require attention to ensure correctness, security, and maintainability.

---

🔴 CRITICAL

1. Replay Protection in MCPMessageSigner

   • The PR mentions that MCPMessageSigner adds HMAC-SHA256 signing, timestamps, and nonce replay detection. However, the implementation details for nonce management and replay detection are not visible in the provided diff. Ensure that:
     • Nonces are unique and securely generated.
     • Nonces are stored and checked against a secure, tamper-proof data store to prevent replay attacks.
     • Nonces have a reasonable expiration time to prevent storage exhaustion attacks.
   • If these aspects are not implemented or are implemented insecurely, it could lead to a critical vulnerability.

2. Credential Redaction

   • The CredentialRedactor is mentioned as a new feature for recursive credential redaction. However, the implementation details are not visible in the diff. Ensure that:
     • The redaction logic is comprehensive and handles edge cases, such as nested structures, various encodings, and obfuscated credentials.
     • The redaction process does not inadvertently expose sensitive information.
     • Unit tests cover a wide range of scenarios, including edge cases.

3. Session Authentication

   • The MCPSessionAuthenticator introduces agent-bound session tokens, TTL expiry, and concurrent-session limits. Ensure that:
     • The session tokens are securely generated and stored.
     • The TTL mechanism is robust and cannot be bypassed.
     • The concurrent-session limits are enforced correctly and tested for edge cases.

4. Sandbox Escape Vectors

   • The PR mentions improvements to MCP tool-path hardening and sandboxing. However, the diff does not provide details on how these are implemented. Ensure that:
     • The sandboxing mechanism is robust and prevents unauthorized access to the host system.
     • The tool-path hardening measures are effective against path traversal and other injection attacks.
     • Comprehensive tests are in place to validate the security of the sandbox.

---

🟡 WARNING

1. Backward Compatibility

   • The addition of new classes and methods (e.g., MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, CredentialRedactor, MCPSlidingRateLimiter) could potentially introduce breaking changes if existing APIs or behaviors are modified. Ensure that:
     • All changes are backward-compatible.
     • Any breaking changes are clearly documented in the CHANGELOG.md and release notes.

2. Public API Changes

   • If any of the new classes or methods are part of the public API, ensure that they are properly versioned and documented. Consider adding deprecation warnings for any APIs that are being replaced or modified.

---

💡 SUGGESTIONS

1. Test Coverage

   • While the PR mentions expanded test coverage, it is not clear if all new features are comprehensively tested. Ensure that:
     • Unit tests cover all edge cases, including invalid inputs, boundary conditions, and potential misuse scenarios.
     • Integration tests validate the interaction between the new components (e.g., MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, etc.) and the existing system.
     • Tests are added for the new OpenTelemetry counters to verify their correctness.

2. Documentation

   • The PR includes updates to the documentation, which is commendable. However, ensure that:
     • The documentation provides clear examples of how to use the new features.
     • The OWASP mapping in owasp-agentic-top10-mapping.md is accurate and up-to-date.
     • Any new configuration options (e.g., for MCPSlidingRateLimiter or MCPSessionAuthenticator) are documented with examples.

3. Performance Testing

   • The introduction of new features like MCPSlidingRateLimiter and MCPResponseScanner could impact performance. Consider conducting performance tests to ensure that the changes do not introduce significant overhead, especially under high load.

4. Thread Safety

   • The PR does not provide details on how thread safety is ensured for the new components. Verify that:
     • Shared resources (e.g., nonce storage, session tokens) are accessed in a thread-safe manner.
     • Concurrency issues are addressed, especially for MCPSlidingRateLimiter and MCPSessionAuthenticator.

5. Error Handling

   • Ensure that all new components have robust error handling. For example:
     • What happens if MCPMessageSigner encounters an error during HMAC generation or verification?
     • How does MCPSessionAuthenticator handle expired or invalid tokens?
     • Are meaningful error messages provided to users and logged for debugging?

6. Code Style and Linting

   • Ensure that the new code adheres to the project's style guidelines and passes ruff checks.

---

Summary of Actions

1. Address Critical Issues

   • Provide implementation details for MCPMessageSigner, CredentialRedactor, MCPSessionAuthenticator, and sandboxing improvements.
   • Ensure robust security measures for nonce management, credential redaction, session authentication, and sandboxing.

2. Review Backward Compatibility

   • Verify that the changes are backward-compatible and document any breaking changes.

3. Expand Test Coverage

   • Add unit and integration tests for all new features and edge cases.

4. Document Thoroughly

   • Ensure that all new features and configuration options are well-documented with examples.

5. Conduct Performance Testing

   • Evaluate the performance impact of the new features under high load.

6. Ensure Thread Safety

   • Review the implementation of new components for potential concurrency issues.

7. Improve Error Handling

   • Add robust error handling and meaningful error messages for all new components.

8. Run Static Analysis

   • Ensure the code passes ruff checks and adheres to the project's style guidelines.

---

Please address the critical issues and warnings before merging this PR. Let me know if you need further clarification or assistance!

[jackbatzner]

Addressing Review Feedback

@imran-siddique — thanks for the review! Here's what we've done:

✅ Imran's Review (workflow revert)

"This PR reverts a security fix from MSRC-111178 audit (PR #731)... please keep the MCP code and drop the workflow change."

Fixed: .github/ changes have been reverted. After merging latest main, the PR no longer modifies any workflow files. The MSRC-111178 temp-file pattern is preserved.

✅ Bot Review: Nonce Cache LRU Eviction

MCPMessageSigner nonce cache needs eviction policy when full

Fixed: Added LRU eviction to the nonce cache — when max_nonce_cache_size is reached, oldest entries are evicted. Covered by new tests.

✅ Bot Review: Thread Safety

MCPSlidingRateLimiter and MCPSessionAuthenticator need thread-safe data structures

Fixed: Added hreading.Lock protection to mutable state in both rate limiter and session authenticator. Added concurrent session lifecycle tests.

✅ Bot Review: PEM Credential Redaction

CredentialRedactor patterns may miss full PEM blocks

Fixed: Regex now matches full PEM blocks from -----BEGIN ... KEY----- through -----END ... KEY----- inclusive. The entire block is redacted, not just the header.

✅ Bot Review: Docstrings & Type Hints

Missing docstrings for new public classes and methods

Fixed: Added comprehensive docstrings to all public MCP classes: MCPResponseScanner, MCPSessionAuthenticator, MCPMessageSigner, CredentialRedactor, MCPSlidingRateLimiter, MCPGateway. Public methods have complete type annotations.

Validation

• python -m pytest packages/agent-os/tests/ -v → 2938 passed, 86 skipped ✅
• python -m ruff check --select E,F,W on MCP files → clean ✅
• No .github/ changes in diff ✅

[github-actions]

🤖 AI Agent: code-reviewer

Review of PR: feat(agent-os): add Python MCP OWASP parity

---

🔴 CRITICAL: Security Issues

1. Credential Redaction Incompleteness

   • The CredentialRedactor class provides a robust mechanism for detecting and redacting sensitive information. However, the PATTERNS list is not exhaustive and may miss certain types of credentials or sensitive data. For example:
     • It does not include patterns for database connection strings (e.g., PostgreSQL, MySQL, etc.).
     • It does not account for custom API keys or secrets that may not follow standard patterns.
   • Recommendation: Consider allowing users to supply custom patterns to the CredentialRedactor class. This would make the redactor extensible and adaptable to different use cases.

2. Replay Protection in MCPMessageSigner

   • While the MCPMessageSigner introduces HMAC-SHA256 signing and nonce replay detection, there is no mention of how nonces are stored or their expiration policy. Without a proper expiration mechanism, the nonce store could grow indefinitely, leading to memory exhaustion.
   • Recommendation: Implement a time-based eviction policy for the InMemoryNonceStore to prevent unbounded growth. Additionally, document the nonce expiration policy to ensure users understand its limitations.

3. Rate Limiting

   • The MCPSlidingRateLimiter introduces a sliding-window rate-limiting mechanism. However, the implementation details are not provided in the diff. If the rate limiter is implemented in-memory, it may not scale well in distributed systems or across multiple instances of the agent.
   • Recommendation: Ensure the rate limiter supports distributed environments by using a shared backend (e.g., Redis) for state storage. If this is not feasible, document the limitations of the in-memory implementation.

4. Audit Log Storage

   • The MCPGateway class stores audit logs in memory (self._audit_log). This approach is not scalable and could lead to memory exhaustion in long-running processes or under high load.
   • Recommendation: Provide an option to store audit logs in a persistent or external storage system (e.g., a database or logging service). Ensure that sensitive data in the logs is redacted before storage.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes

   • The PR introduces several new classes and methods (e.g., CredentialRedactor, MCPMessageSigner, MCPSessionAuthenticator, etc.) and adds them to the __init__.py file. While these additions are non-breaking, they expand the public API surface.
   • Recommendation: Clearly document these new APIs in the library's documentation and ensure they are marked as stable or experimental as appropriate.

2. OpenTelemetry Dependency

   • The MCPMetrics class introduces an optional dependency on OpenTelemetry. While the implementation gracefully falls back to a no-op recorder if OpenTelemetry is not installed, this could lead to unexpected behavior for users who are unaware of the dependency.
   • Recommendation: Update the documentation to explicitly mention the optional dependency on OpenTelemetry and provide installation instructions.

---

💡 Suggestions for Improvement

1. Thread Safety

   • The MCPGateway class uses in-memory data structures (_agent_call_counts, _audit_log) that are not thread-safe. This could lead to race conditions in multi-threaded environments.
   • Recommendation: Use thread-safe data structures (e.g., collections.defaultdict with threading.Lock) or document that the class is not thread-safe and should not be used in multi-threaded contexts without external synchronization.

2. Test Coverage

   • While the PR mentions expanded test coverage, it is not clear if all new components (e.g., CredentialRedactor, MCPMessageSigner, MCPSessionAuthenticator, etc.) have been thoroughly tested.
   • Recommendation: Ensure that all new components have comprehensive unit tests, including edge cases and error conditions. For example:
     • Test CredentialRedactor with edge cases like empty strings, very large inputs, and inputs with overlapping patterns.
     • Test MCPMessageSigner with invalid HMAC keys, expired timestamps, and replayed nonces.
     • Test MCPSlidingRateLimiter under high load and edge cases (e.g., burst traffic).

3. Documentation

   • The PR includes updates to the documentation, but it is unclear if the new components are fully documented.
   • Recommendation: Ensure that all new classes and methods are documented with clear examples. Specifically:
     • How to configure and use the CredentialRedactor with custom patterns.
     • How to integrate MCPMetrics with an OpenTelemetry backend.
     • How to use MCPSessionAuthenticator and MCPMessageSigner in a secure manner.

4. Logging

   • The CredentialRedactor logs the number of redacted values but does not log the actual redacted content (for security reasons). However, it may be useful to log the types of credentials detected (e.g., "Redacted 3 OpenAI API keys and 2 GitHub tokens").
   • Recommendation: Enhance logging to include the types of credentials detected, without exposing the actual values.

5. Backward Compatibility

   • While the PR does not introduce breaking changes, the expanded public API surface increases the risk of future breaking changes.
   • Recommendation: Use semantic versioning to indicate the stability of new APIs. For example, mark new APIs as experimental in the documentation and provide a clear path for deprecation and removal in future versions.

---

Summary of Recommendations

1. 🔴 Address the incompleteness of the CredentialRedactor patterns and allow for user-defined patterns.
2. 🔴 Implement a time-based eviction policy for the InMemoryNonceStore to prevent memory exhaustion.
3. 🔴 Ensure the MCPSlidingRateLimiter supports distributed environments or document its limitations.
4. 🔴 Provide an option for persistent or external storage of audit logs in MCPGateway.
5. 🟡 Clearly document the new public APIs and mark them as stable or experimental.
6. 🟡 Update the documentation to mention the optional dependency on OpenTelemetry.
7. 💡 Use thread-safe data structures in MCPGateway or document its thread-safety limitations.
8. 💡 Expand test coverage for all new components, including edge cases and error conditions.
9. 💡 Enhance logging in CredentialRedactor to include the types of credentials detected.
10. 💡 Use semantic versioning to indicate the stability of new APIs and provide a clear deprecation path.

---

Let me know if you need further clarification or assistance!

[jackbatzner]

PR Split Update

This PR is being updated to contain only the core MCP security primitives and tests for easier review. The original monolithic PR is being split into 3 stacked PRs:

#	PR	Content	Status
1	#774 (this PR)	Core MCP primitives + tests	✅ Ready for review
2	TBD	Standalone agent-mcp-governance package	Coming
3	TBD	Documentation and examples	Coming

Merge order: #774 → Package PR → Docs PR (numbers will be updated once opened)

All previous review feedback from @imran-siddique has been addressed in this updated branch (see earlier comment for details).

[jackbatzner]

PR Split Update

This PR has been updated to contain only the core MCP security primitives and tests for easier review. The original monolithic PR has been split into 3 stacked PRs:

#	PR	Content	Status
1	#774 (this PR)	Core MCP primitives + tests	✅ Ready for review
2	#829	Standalone agent-mcp-governance package	Open
3	#828	Documentation and examples	Open

Merge order: #774 → #829 → #828

All previous review feedback from @imran-siddique has been addressed in this updated branch.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces MCP (Model Context Protocol) security primitives to the agent-os-kernel package, aiming to enhance security and governance capabilities. The implementation includes cryptographic operations, rate-limiting, input sanitization, credential redaction, and OpenTelemetry-based metrics. The changes align with OWASP MCP Security Cheat Sheet recommendations, except for §11 (Consent UI), which is out of scope for server-side SDKs.

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

---

🔴 CRITICAL Issues

1. Replay Attack Protection in McpMessageSigner

• Issue: While the nonce replay cache is implemented, there is no explicit mention of nonce expiration or time-based invalidation. This could allow attackers to replay old messages if the nonce cache grows indefinitely.
• Impact: Replay attacks could bypass security mechanisms, leading to unauthorized actions.
• Action: Implement nonce expiration based on timestamps and enforce a maximum cache duration.

2. Credential Redaction in CredentialRedactor

• Issue: The CredentialRedactor uses regex patterns for credential detection, but regex-based approaches can be prone to bypasses or false negatives. For example, a slight variation in formatting could evade detection.
• Impact: Sensitive credentials might leak into logs or audit trails, violating security policies.
• Action: Complement regex-based detection with heuristic or ML-based approaches for higher accuracy.

3. Fail-Closed Behavior in MCPGateway

• Issue: The _evaluate method in MCPGateway fails closed on exceptions, but the error message logged (Internal gateway error — access denied) could expose internal details to attackers.
• Impact: Revealing internal error messages could aid attackers in crafting targeted exploits.
• Action: Replace detailed error messages with generic ones (e.g., "Access denied due to policy enforcement").

---

🟡 WARNING: Potential Breaking Changes

1. Public API Additions

• Issue: The addition of new classes (McpMessageSigner, CredentialRedactor, etc.) and methods to the agent-os-kernel package expands the public API surface.
• Impact: Downstream consumers relying on the package may experience compatibility issues if these APIs change in future versions.
• Action: Clearly document these APIs as experimental or stable, depending on their maturity.

2. OpenTelemetry Integration

• Issue: The MCPMetrics class introduces optional OpenTelemetry integration. If downstream users rely on this feature but do not have OpenTelemetry installed, it could lead to runtime errors.
• Impact: Breaking changes for users unaware of the optional dependency.
• Action: Add explicit documentation and warnings about the optional dependency.

---

💡 Suggestions for Improvement

1. Thread Safety

• Observation: The MCPGateway class uses a dictionary (_agent_call_counts) for rate-limiting, which may not be thread-safe in concurrent environments.
• Suggestion: Use thread-safe data structures like collections.defaultdict with locks or concurrent.futures for concurrency.

2. OWASP MCP Compliance

• Observation: The implementation covers 11/12 sections of the OWASP MCP Security Cheat Sheet. While §11 (Consent UI) is out of scope, consider documenting this explicitly in the README or documentation.
• Suggestion: Add a compliance matrix to the documentation for transparency.

3. Type Safety

• Observation: The CredentialRedactor class uses Any for nested data structures, which could lead to runtime type errors.
• Suggestion: Use Pydantic models or type hints for stricter validation.

4. Sandbox Escape Prevention

• Observation: The MCPGateway sanitizes parameters but does not explicitly address sandbox escape vectors (e.g., malicious payloads in tool calls).
• Suggestion: Add explicit checks for sandbox escape attempts, such as file system access or process execution.

5. Testing Coverage

• Observation: While the PR mentions that all tests pass, the skipped tests (86) are not explained.
• Suggestion: Review skipped tests and ensure they are not critical for security or functionality.

---

Final Recommendations

• Address the 🔴 CRITICAL issues immediately to prevent security vulnerabilities.
• Document the 🟡 WARNING changes to avoid downstream compatibility issues.
• Implement the 💡 Suggestions to improve robustness, compliance, and usability.

Once the above issues are resolved, this PR can be merged confidently.

[jackbatzner]

Addressing Review Feedback — Updated

@imran-siddique — thanks for both rounds of review! Here's the current state after the split and security fixes:

✅ Review 1: Workflow revert

"This PR reverts a security fix from MSRC-111178 audit"

Fixed — All .github/ files have been removed from this PR. The branch now contains only MCP security primitives and tests.

✅ Review 2: CLA + Dependencies

"CLA not signed"

Fixed — CLA is signed and passing (license/cla: success).

"agent-mcp-governance declares zero dependencies but imports agent_os"

Fixed — The standalone package is now in a separate PR (#829) and declares agent-os-kernel>=3.0.0 as a dependency.

Additional security hardening (from internal review):

• HMAC key minimum raised from 16 → 32 bytes (NIST SP 800-107 compliance)
• MCPGateway rate-limit check now uses per-agent lock (fixes TOCTOU race)
• scan_tool() now wraps in try/except with fail-closed behavior
• Tests updated to cover all new security gates

[jackbatzner]

Addressing AI Reviewer Findings

"Credential Redaction Incompleteness" — The CredentialRedactor is extensible by design. The custom_patterns parameter allows users to add domain-specific patterns (database connection strings, custom API keys, etc.). The built-in patterns cover the most common credential formats (PEM blocks, API keys, Bearer tokens, connection strings). This is documented in the API reference.

"Replay Protection" — Nonce replay protection is implemented with a bounded LRU cache (max 4096 entries) and constant-time HMAC comparison. Nonces are verified before caching to prevent cache pollution attacks. All covered in mcp_message_signer.py tests.

All other findings from the latest review are advisory enhancements — no security blockers remain.

[jackbatzner]

CI Status Summary — All findings addressed ✅

All CI checks pass except:

• Maintainer approval gate — awaiting your review

What this PR adds: Python MCP security primitives (Gateway, Scanner, Signer, Authenticator, Rate Limiter, Response Scanner, Credential Redactor) covering 11/12 OWASP MCP Security Cheat Sheet sections.

This is the Core PR in a 3-PR stack. Merge order: #774 (Core) → #829 (Package) → #828 (Docs).

[imran-siddique]

@jackbatzner — #774 is merged! However, PRs #775, #791, #832, #829, #824, #827, #833, #825, #826, #828, #830, #834, #836 all have merge conflicts with main now. Could you rebase each one on latest main? We're ready to merge them all once conflicts are resolved.

Approved merge order:

1. Core: feat(dotnet): add MCP protocol support with full OWASP coverage #775 (.NET), feat(typescript): add MCP protocol governance with full OWASP coverage #791 (TypeScript), feat(go): add MCP protocol support with full OWASP coverage #832 (Go)
2. Standalone packages: feat(python): add standalone agent-mcp-governance package #829, feat(dotnet): add standalone Microsoft.AgentGovernance.Mcp package #824, feat(typescript): add standalone @microsoft/agentmesh-mcp-governance package #827, feat(go): add standalone MCP governance package #833
3. Docs: docs(typescript): add MCP security documentation and examples #825, docs(dotnet): add MCP security documentation and examples #826, docs(python): add MCP security documentation and examples #828, docs(rust): add MCP security documentation, OWASP mapping, and examples #830, docs(go): add MCP security documentation and examples #834
4. Other: feat(examples): add real framework integration to CrewAI, OpenAI Agents, smolagents #836 (examples)

Thanks for the great MCP governance work across all SDKs!