Skip to content

feat(python): add standalone agent-mcp-governance package#829

Closed
jackbatzner wants to merge 1 commit intomicrosoft:mainfrom
jackbatzner:jb/python-mcp-package
Closed

feat(python): add standalone agent-mcp-governance package#829
jackbatzner wants to merge 1 commit intomicrosoft:mainfrom
jackbatzner:jb/python-mcp-package

Conversation

@jackbatzner
Copy link
Copy Markdown
Contributor

Description

Introduces the standalone agent-mcp-governance Python package with zero AGT dependency. This allows MCP server operators to adopt MCP governance primitives without pulling in the full Agent Governance Toolkit.

Package: agent-mcp-governance (PyPI name: agent_mcp_governance)

  • dependencies = [] — no AGT dependency
  • Re-exports MCP primitives from agent-os-kernel
  • README with installation, quick-start examples, and OWASP MCP Cheat Sheet mapping

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

Type of Change

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

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 2 of 3 (split from original PR for easier review). Merge after #774.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

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

@github-actions github-actions bot added documentation Improvements or additions to documentation dependencies Pull requests that update a dependency file tests size/XL Extra large PR (500+ lines) labels Apr 6, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

🤖 AI Agent: breaking-change-detector — Summary

🔍 API Compatibility Report

Summary

This pull request introduces a new standalone Python package, agent-mcp-governance, which re-exports MCP governance primitives from agent-os-kernel. The package is additive and does not introduce any breaking changes to existing APIs. It is designed to provide a focused surface for MCP governance without dependencies on the broader Agent Governance Toolkit.

Findings

Severity Package Change Impact
🔵 agent-mcp-governance New package with re-exported MCP APIs Not breaking, new functionality

Migration Guide

No migration is required as this is an additive change. Downstream users can adopt the new agent-mcp-governance package without any impact on existing functionality.

Additional Notes

  • The new package has zero dependencies on the Agent Governance Toolkit, making it suitable for standalone use cases.
  • The __init__.py file in agent-mcp-governance explicitly re-exports MCP governance primitives from agent-os-kernel, ensuring a clean and focused API surface.
  • The package version starts at 0.1.0, indicating its Public Preview status.

No breaking changes detected.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

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

📝 Documentation Sync Report

Issues Found

  • agent_mcp_governance public API re-exports in packages/agent-mcp-governance/src/agent_mcp_governance/__init__.py — missing docstrings for re-exported classes and functions. While these are re-exports, the standalone package should include docstrings for clarity and usability.
  • ⚠️ packages/agent-mcp-governance/README.md — the README is generally well-written, but it lacks details about the specific purpose and usage of each re-exported class and function. This could make it harder for users to understand the API without referring to the original agent-os-kernel documentation.
  • ⚠️ CHANGELOG.md — no entry for the addition of the new agent-mcp-governance package.

Suggestions

  • 💡 Add docstrings for all re-exported classes and functions in packages/agent-mcp-governance/src/agent_mcp_governance/__init__.py. Even if these are re-exports, brief descriptions of their purpose and usage should be included for standalone users of this package.
  • 💡 Expand the Quick usage section in packages/agent-mcp-governance/README.md to include examples for all major re-exported classes and functions, such as MCPMessageSigner, MCPSecurityScanner, and MCPResponseScanner.
  • 💡 Add an entry to CHANGELOG.md documenting the introduction of the new agent-mcp-governance package, including its purpose and key features.

Additional Notes

  • The type hints for the re-exported classes and functions are inherited from the original agent-os-kernel package, so no additional type annotations are required here.
  • The README.md does a good job of explaining the package's purpose and installation process, but more detailed examples would improve usability.
  • The pyproject.toml file is well-structured and correctly specifies the dependencies and metadata for the new package.

Action Required

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

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

🤖 AI Agent: test-generator — `packages/agent-mcp-governance/src/agent_mcp_governance/__init__.py`

🧪 Test Coverage Analysis

packages/agent-mcp-governance/src/agent_mcp_governance/__init__.py

  • Existing coverage:

    • The __init__.py file primarily re-exports MCP governance primitives from the agent-os-kernel package. These primitives (e.g., MCPGateway, MCPSessionAuthenticator, MCPSlidingRateLimiter, etc.) are likely already covered by tests in the agent-os-kernel package.
    • If the agent-os-kernel primitives have robust test coverage, the re-exported functionality should indirectly benefit from this coverage.
  • Missing coverage:

    • There are no direct tests for the agent_mcp_governance package itself to verify that the re-exported components are correctly exposed and functional in this standalone package.
    • No tests validate the "zero dependency" guarantee of the agent_mcp_governance package (e.g., ensuring it does not inadvertently pull in dependencies from the broader Agent Governance Toolkit).
  • 💡 Suggested test cases:

    1. test_reexported_primitives — Verify that all expected primitives (e.g., MCPGateway, MCPSessionAuthenticator, MCPSlidingRateLimiter, etc.) are correctly re-exported and accessible from the agent_mcp_governance package.
    2. test_mcp_gateway_basic_functionality — Test a basic use case of MCPGateway (e.g., policy enforcement) to ensure the re-exported class functions as expected in the standalone package.
    3. test_session_authenticator_basic_functionality — Validate session creation and validation using MCPSessionAuthenticator to confirm that the re-exported class operates correctly.
    4. test_rate_limiter_basic_functionality — Test rate-limiting functionality using MCPSlidingRateLimiter to ensure it behaves as expected when used via the standalone package.
    5. test_zero_dependency — Add a test to ensure the agent_mcp_governance package does not introduce unintended dependencies. For example:
      • Verify the pyproject.toml dependencies are empty except for agent-os-kernel.
      • Use a dependency-checking tool to confirm no additional dependencies are pulled in during installation.
    6. test_integration_with_agent_os_kernel — Ensure that the agent_mcp_governance package correctly integrates with the agent-os-kernel primitives it re-exports, particularly for edge cases like:
      • Policy evaluation with conflicting or malformed policies.
      • Trust scoring with edge cases (e.g., expired certificates, revoked trust).
      • Chaos experiments (e.g., cascading failures in MCPGateway or MCPSecurityScanner).
    7. test_input_validation — Validate that re-exported components handle malformed inputs gracefully. For example:
      • Invalid session tokens in MCPSessionAuthenticator.
      • Oversized payloads in MCPGateway or MCPSecurityScanner.
      • Injection attempts in MCPResponseScanner.

By adding these tests, the standalone agent_mcp_governance package can ensure that its re-exported components are functional, reliable, and aligned with the intended "zero dependency" design.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 6, 2026

🤖 AI Agent: security-scanner — Security Review of `feat(python): add standalone agent-mcp-governance package`

Security Review of feat(python): add standalone agent-mcp-governance package


Findings

1. Prompt Injection Defense Bypass

Severity: 🔴 CRITICAL
Issue:
The DemoPolicy class in the example provided in the README.md includes a matches_pattern method that returns an empty list regardless of the input. This method is likely intended to validate or sanitize input against malicious patterns, but its current implementation effectively disables any input validation. This creates a potential vulnerability where malicious input could bypass policy guards, leading to prompt injection attacks.

Attack Vector:
An attacker could craft malicious input that exploits the lack of validation in matches_pattern to bypass security policies enforced by the MCPGateway. This could allow unauthorized access to sensitive tools or data.

Recommendation:

  • Implement a robust input validation mechanism in the matches_pattern method.
  • Use a well-defined set of regular expressions or other validation techniques to detect and block potentially malicious input.
  • Add unit tests to verify that the matches_pattern method correctly identifies and blocks malicious input.

2. Policy Engine Circumvention

Severity: 🟠 HIGH
Issue:
The DemoPolicy class in the example code does not enforce critical security policies, such as allowed_tools and require_human_approval. While this is an example, it sets a dangerous precedent for users who might copy and paste this code into production without implementing proper policy checks.

Attack Vector:
If a developer uses this example without modification, it could allow unauthorized tools to be executed or bypass human approval requirements, leading to potential misuse or abuse of the system.

Recommendation:

  • Update the example to include proper enforcement of allowed_tools and require_human_approval.
  • Add comments in the example code to emphasize the importance of implementing these checks.
  • Provide a more secure default policy implementation in the example.

3. Trust Chain Weaknesses

Severity: 🟡 MEDIUM
Issue:
The MCPMessageSigner and MCPSessionAuthenticator components rely on cryptographic primitives for message integrity and session validation. However, there is no explicit mention of how cryptographic keys are managed, rotated, or stored securely.

Attack Vector:
If cryptographic keys are not securely managed, an attacker could compromise the keys and forge messages or sessions, effectively bypassing the trust chain.

Recommendation:

  • Clearly document best practices for key management, including secure storage, rotation, and revocation.
  • Consider integrating with a secure key management system (e.g., Azure Key Vault, AWS KMS).
  • Add runtime checks to ensure that cryptographic keys meet minimum security requirements (e.g., key length, algorithm strength).

4. Credential Exposure

Severity: 🔵 LOW
Issue:
The CredentialRedactor component is re-exported, but there is no mention of how it is used to prevent sensitive information from being logged or exposed.

Attack Vector:
If sensitive credentials are logged or exposed in error messages, an attacker could use this information to compromise the system.

Recommendation:

  • Ensure that CredentialRedactor is integrated into all logging and error-handling mechanisms.
  • Add tests to verify that sensitive information is redacted in logs and error messages.
  • Update the documentation to highlight the importance of using CredentialRedactor.

5. Sandbox Escape

Severity: 🔵 LOW
Issue:
The package does not include any explicit mechanisms for sandboxing or isolating agent processes. While this may be outside the scope of the package, it is worth noting for users who may assume such protections are included.

Attack Vector:
If an agent process is compromised, an attacker could escape the process or container sandbox and gain access to the host system.

Recommendation:

  • Clearly document that sandboxing and process isolation are not included in this package.
  • Provide guidance on how to implement sandboxing and isolation using external tools (e.g., Docker, Kubernetes).

6. Deserialization Attacks

Severity: 🟠 HIGH
Issue:
The package re-exports components like MCPSessionStore and MCPNonceStore, which may involve deserialization of data. There is no mention of whether these components validate or sanitize deserialized input.

Attack Vector:
If an attacker can inject malicious serialized data, they could exploit deserialization vulnerabilities to execute arbitrary code or cause denial of service.

Recommendation:

  • Ensure that all deserialization operations validate and sanitize input to prevent malicious payloads.
  • Use safe serialization libraries or formats (e.g., JSON with strict schema validation).
  • Add tests to verify that deserialization operations are secure.

7. Race Conditions

Severity: 🟡 MEDIUM
Issue:
The MCPSlidingRateLimiter component may be vulnerable to race conditions if it is used in a multi-threaded or distributed environment without proper synchronization.

Attack Vector:
An attacker could exploit race conditions to exceed rate limits or bypass rate-limiting checks.

Recommendation:

  • Use atomic operations or distributed locks to ensure that rate-limiting checks are thread-safe and consistent across distributed systems.
  • Add tests to verify that the rate limiter behaves correctly under concurrent access.

8. Supply Chain Risks

Severity: 🟠 HIGH
Issue:
The pyproject.toml file specifies a dependency on agent-os-kernel>=3.0.0,<4.0.0. There is no mention of how this dependency is vetted for supply chain risks, such as dependency confusion or typosquatting.

Attack Vector:
If an attacker compromises the agent-os-kernel package or publishes a malicious package with a similar name, they could introduce vulnerabilities into this package.

Recommendation:

  • Pin dependencies to specific, verified versions where possible.
  • Use tools like pip-audit or safety to scan dependencies for known vulnerabilities.
  • Consider implementing a dependency allowlist or using a private package repository to mitigate supply chain risks.

Summary of Findings

Category Severity Description
Prompt Injection Defense 🔴 CRITICAL matches_pattern method disables input validation, allowing malicious input.
Policy Engine Circumvention 🟠 HIGH Example policy does not enforce critical security policies.
Trust Chain Weaknesses 🟡 MEDIUM Lack of documentation on cryptographic key management.
Credential Exposure 🔵 LOW No mention of how CredentialRedactor is used to prevent sensitive data leaks.
Sandbox Escape 🔵 LOW No sandboxing or isolation mechanisms included.
Deserialization Attacks 🟠 HIGH Potential vulnerabilities in deserialization operations.
Race Conditions 🟡 MEDIUM Potential race conditions in MCPSlidingRateLimiter.
Supply Chain Risks 🟠 HIGH Dependency on agent-os-kernel without supply chain vetting.

Overall Recommendation

This pull request introduces a standalone package for MCP governance primitives, which is a valuable addition to the toolkit. However, several critical and high-severity issues need to be addressed before merging. These include implementing proper input validation, enforcing security policies in examples, securing cryptographic key management, and mitigating deserialization and supply chain risks.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces a new standalone Python package, agent-mcp-governance, which provides MCP governance primitives without requiring the full Agent Governance Toolkit (AGT). The package re-exports components from agent-os-kernel and includes a README with installation instructions, usage examples, and security guidance. Additionally, the PR includes new modules for credential redaction and OpenTelemetry-based metrics for MCP governance.

The changes are well-structured and adhere to the project's style guidelines. However, there are some areas of concern and opportunities for improvement, particularly around security, type safety, and backward compatibility.


🔴 CRITICAL: Security Issues

  1. Credential Redaction Incompleteness

    • The CredentialRedactor class uses regex patterns to identify and redact sensitive information. While the patterns cover common credential formats, they are not exhaustive and may miss less common or obfuscated credentials.
    • Actionable Recommendation: Consider allowing users to extend or override the default patterns. Additionally, document the limitations of the current patterns and provide guidance for adding custom patterns.
  2. Potential Credential Leakage in Logs

    • The CredentialRedactor logs the number of redactions applied (logger.info("Credential redaction applied to %s value(s)", redaction_count)), but it does not sanitize the input before logging. If logging is misconfigured, sensitive data could inadvertently be exposed.
    • Actionable Recommendation: Avoid logging any information derived from unredacted input. Replace the log message with a generic statement like "Credential redaction applied."
  3. OpenTelemetry Integration

    • The MCPMetrics class uses OpenTelemetry to record metrics. However, the implementation does not sanitize the labels (e.g., agent_id, tool_name, server_name) before recording them. If these labels contain sensitive information, it could lead to data leakage.
    • Actionable Recommendation: Sanitize all labels before passing them to OpenTelemetry. Use the CredentialRedactor to ensure no sensitive information is logged.
  4. Policy Enforcement

    • The MCPGateway class is critical for enforcing security policies. However, the PR does not include the implementation of this class, making it impossible to verify its correctness.
    • Actionable Recommendation: Ensure that the MCPGateway implementation is thoroughly reviewed for security vulnerabilities, especially around policy enforcement and audit logging.

🟡 WARNING: Potential Breaking Changes

  1. Python Version Requirement

    • The new package requires Python 3.12 or higher (requires-python = ">=3.12"). This is a breaking change for users on older Python versions.
    • Actionable Recommendation: Clearly document this requirement in the release notes and consider whether this restriction is necessary for the initial release.
  2. Re-exports in agent_os

    • The PR adds several new re-exports to the agent_os package. If users rely on the existing __all__ attribute for introspection, these changes could cause unexpected behavior.
    • Actionable Recommendation: Document these additions in the release notes and ensure they are backward-compatible.

💡 Suggestions for Improvement

  1. Type Annotations

    • While the code includes type annotations, some areas could benefit from additional specificity. For example:
      • The CredentialRedactor.redact_data_structure method uses Any for its value parameter, which could be replaced with a Union of supported types (e.g., Union[str, dict, list, tuple]).
    • Actionable Recommendation: Review type annotations for completeness and specificity. Consider using mypy to enforce type correctness.
  2. Test Coverage

    • The PR does not include tests for the new CredentialRedactor and MCPMetrics classes. These components are critical for security and observability, respectively.
    • Actionable Recommendation: Add comprehensive unit tests for these classes, including edge cases (e.g., malformed input, empty strings, and deeply nested data structures).
  3. Thread Safety

    • The MCPMetrics class uses instance variables (e.g., self._decisions) to store OpenTelemetry counters. If the class is used in a multi-threaded environment, these variables could lead to race conditions.
    • Actionable Recommendation: Use thread-safe constructs or document that the class is not thread-safe.
  4. Documentation

    • While the README provides a good overview, it does not include detailed API documentation for the exported classes and methods.
    • Actionable Recommendation: Add API documentation to the README or provide a link to the full documentation.
  5. Backward Compatibility Testing

    • The PR introduces changes to the agent-os package, including new re-exports and a new _mcp_metrics module. These changes could have unintended side effects.
    • Actionable Recommendation: Run backward compatibility tests to ensure existing functionality is not affected.
  6. Error Handling

    • The MCPMetrics class catches all exceptions during initialization (except Exception) but does not provide detailed error handling or recovery mechanisms.
    • Actionable Recommendation: Narrow the exception handling to specific exceptions and log meaningful error messages to aid debugging.

Summary of Actions

Critical

  • Allow users to extend or override CredentialRedactor patterns.
  • Avoid logging sensitive information in CredentialRedactor.
  • Sanitize OpenTelemetry labels in MCPMetrics.
  • Ensure thorough review of MCPGateway implementation for security vulnerabilities.

Warning

  • Document Python 3.12 requirement in release notes.
  • Document new agent_os re-exports in release notes.

Suggestions

  • Improve type annotations and enforce with mypy.
  • Add unit tests for CredentialRedactor and MCPMetrics.
  • Ensure thread safety in MCPMetrics or document its limitations.
  • Expand API documentation in the README or link to external docs.
  • Conduct backward compatibility testing for agent-os changes.
  • Improve error handling in MCPMetrics.

Let me know if you need further clarification or assistance!

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces a new standalone package, agent-mcp-governance, which provides MCP governance primitives without requiring the full Agent Governance Toolkit (AGT). The package re-exports relevant components from agent-os-kernel and includes a README with installation instructions, usage examples, and security guidance. The PR also introduces new functionality for credential redaction and OpenTelemetry-based metrics for MCP governance components.

The changes are well-structured and adhere to the project's style guidelines. However, there are several areas that require attention, including potential security issues, type safety concerns, and opportunities for improvement.


🔴 CRITICAL: Security Issues

  1. Credential Redaction Patterns

    • The CredentialRedactor class introduces several regex patterns for detecting sensitive information. While these patterns are useful, they may not cover all possible formats of sensitive data. For example:
      • The "Bearer token" pattern assumes a specific format, but tokens can vary significantly in structure.
      • The "Generic API secret" pattern may lead to false positives or false negatives due to its broad scope.
    • Recommendation: Consider adding tests to validate the effectiveness of these patterns against a wide range of real-world examples. Additionally, document the limitations of these patterns in the README or code comments.
  2. Credential Redaction Logging

    • The CredentialRedactor.redact method logs the number of redactions applied. However, this could inadvertently leak the presence of sensitive information if the logs are accessible to unauthorized users.
    • Recommendation: Avoid logging the number of redactions or ensure that logs are only accessible to authorized personnel.
  3. OpenTelemetry Integration

    • The MCPMetrics class uses OpenTelemetry for metrics recording. However, the implementation does not validate the inputs (e.g., agent_id, tool_name, stage) before passing them to the OpenTelemetry API. This could lead to injection vulnerabilities if these inputs are not sanitized.
    • Recommendation: Sanitize all inputs before passing them to the OpenTelemetry API to prevent potential injection attacks.
  4. Session Authentication

    • The MCPSessionAuthenticator class is re-exported, but there is no indication of whether it enforces secure practices (e.g., token expiration, revocation). If not already implemented, this could lead to session hijacking or replay attacks.
    • Recommendation: Ensure that the MCPSessionAuthenticator enforces token expiration and supports token revocation. Add tests to verify these behaviors.

🟡 WARNING: Potential Breaking Changes

  1. Python Version Compatibility

    • The new package specifies requires-python = ">=3.12" in its pyproject.toml. This is stricter than the project's overall support for Python 3.9–3.12.
    • Impact: Users on Python versions earlier than 3.12 will not be able to use this package.
    • Recommendation: Consider relaxing the Python version requirement to align with the project's overall compatibility, unless there are specific features in Python 3.12 that are critical to this package.
  2. Dependency on agent-os-kernel

    • The new package depends on agent-os-kernel>=3.0.0. If agent-os-kernel introduces breaking changes in future versions, it could affect this package.
    • Recommendation: Pin the dependency to a specific major version (e.g., agent-os-kernel>=3.0.0,<4.0.0) to avoid unexpected breaking changes.

💡 Suggestions for Improvement

  1. Type Annotations

    • The CredentialRedactor class and its methods lack type annotations for some arguments and return types (e.g., redact_data_structure).
    • Recommendation: Add type annotations to all methods to improve code readability and catch potential type-related issues during development.
  2. Test Coverage

    • While the PR mentions that tests have been added, there is no indication of the specific test cases covered.
    • Recommendation: Ensure that the following scenarios are tested:
      • All credential redaction patterns with both valid and invalid inputs.
      • Edge cases for nested data structures in CredentialRedactor.
      • OpenTelemetry metrics recording with both valid and invalid inputs.
      • Session authentication, including token expiration and revocation.
  3. Documentation

    • The README provides a good overview of the package, but it could benefit from additional details:
      • A section on the limitations of the credential redaction patterns.
      • Examples of how to configure and use the OpenTelemetry integration.
    • Recommendation: Expand the README to include these details.
  4. Thread Safety

    • The MCPMetrics class uses instance variables (e.g., _decisions, _threats_detected) to store OpenTelemetry counters. If this class is used in a multi-threaded environment, there could be race conditions.
    • Recommendation: Use thread-safe constructs (e.g., threading.Lock) to ensure that the counters are accessed and updated safely.
  5. Backward Compatibility

    • The changes to agent-os include new imports and re-exports in __init__.py. While this does not break existing functionality, it increases the surface area of the public API.
    • Recommendation: Clearly document the new exports and consider marking them as experimental if they are not yet stable.

Final Assessment

The PR introduces valuable functionality and is generally well-implemented. However, the identified security issues and potential breaking changes must be addressed before merging. Additionally, improving type annotations, test coverage, and documentation will enhance the maintainability and usability of the new package.

Priority Actions:

  1. 🔴 Address the security issues related to credential redaction, logging, OpenTelemetry integration, and session authentication.
  2. 🟡 Reassess the Python version requirement and dependency pinning for agent-os-kernel.
  3. 💡 Improve type annotations, test coverage, and documentation.

Let me know if you need further clarification or assistance!

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review of Pull Request: feat: add standalone agent-mcp-governance package

General Observations

The addition of the agent-mcp-governance package appears to be a well-structured and thoughtful enhancement to the Agent Governance Toolkit. It provides a focused set of governance primitives without the overhead of the full toolkit, which is beneficial for users who need specific functionalities. The documentation is clear and includes installation instructions and usage examples.

Actionable Feedback

  1. Policy Engine Correctness:

    • Ensure that the MCPGateway and associated classes implement robust validation for policies. It is crucial to verify that policies cannot be bypassed through malformed input or unexpected states. Consider adding unit tests that simulate edge cases to validate policy enforcement.
  2. Trust/Identity:

    • The MCPSessionAuthenticator should ensure that session tokens are securely generated and validated. Review the cryptographic methods used for token creation and ensure they adhere to best practices. If not already implemented, consider using libraries like cryptography for secure token handling.
    • 🔴 CRITICAL: Ensure that the session tokens are not vulnerable to replay attacks. Implement nonce or timestamp checks to prevent reuse.
  3. Sandbox Escape Vectors:

    • Review the MCPGateway and other components for potential sandbox escape vectors. Ensure that any external inputs (e.g., tool calls) are properly sanitized and validated to prevent injection attacks or unauthorized access to sensitive resources.
  4. Thread Safety:

    • If the MCPGateway or any other components are expected to be used in a multi-threaded environment, ensure that shared resources are properly synchronized. Consider using thread-safe data structures or locks where necessary to prevent race conditions.
  5. OWASP Agentic Top 10 Compliance:

    • Ensure that the package addresses all relevant OWASP Agentic Top 10 vulnerabilities. For instance, validate that user inputs are sanitized and that sensitive data is not exposed in logs or error messages. Conduct a thorough review against the OWASP guidelines.
  6. Type Safety and Pydantic Model Validation:

    • Consider using Pydantic models for input validation in classes like MCPGateway and MCPSessionAuthenticator. This will enhance type safety and provide automatic validation of input data structures.
  7. Backward Compatibility:

    • Since this is a new package, backward compatibility is not a concern. However, if there are plans to integrate this package with existing components, ensure that any changes do not break existing functionality. Document any potential breaking changes clearly.
  8. Documentation:

    • The README is well-structured, but consider adding a section on security best practices for using the package. This could include recommendations for securing session tokens, handling sensitive data, and configuring the gateway.
  9. Testing:

    • Ensure that comprehensive tests cover all new functionalities, especially around security features like session management and policy enforcement. Consider using fuzz testing to identify potential vulnerabilities in input handling.
  10. Logging and Monitoring:

    • Implement logging for critical operations within the MCPGateway and other components. This will aid in monitoring and troubleshooting. Ensure that sensitive information is not logged.

Summary

The agent-mcp-governance package is a valuable addition to the toolkit, but it is essential to address the highlighted security concerns and ensure robust testing and documentation. Implementing the suggested improvements will enhance the overall security posture and usability of the package.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This PR introduces a new standalone package, agent-mcp-governance, which provides MCP governance primitives without requiring the full Agent Governance Toolkit (AGT). The package re-exports components from agent-os-kernel and includes a new MCPMetrics class for OpenTelemetry-based metrics and a CredentialRedactor class for sensitive data redaction. The PR is part of a multi-step process and depends on #774 being merged first.

The implementation is generally well-structured and adheres to the project's style guidelines. However, there are some areas that require attention to ensure security, correctness, and maintainability.


🔴 CRITICAL

  1. Credential Redaction Completeness

    • The CredentialRedactor class provides a set of regex patterns for detecting and redacting sensitive information. While the patterns are well-defined, there is a risk of false negatives (missed sensitive data) due to the limited scope of the patterns. For example:
      • The Bearer token pattern assumes a specific format, but tokens can vary widely in structure.
      • The Generic API secret pattern might miss secrets that do not follow the specified naming conventions.
    • Action: Consider allowing users to extend or override the default patterns via configuration or runtime parameters. Additionally, document the limitations of the current patterns and provide guidance for adding custom patterns.
  2. Thread Safety in MCPMetrics

    • The _safe_tool_metric_name method in MCPMetrics uses a threading.Lock to manage access to _tool_metric_names. However, the method does not handle the case where multiple threads might simultaneously add new tool names, potentially leading to race conditions.
    • Action: Use a thread-safe data structure (e.g., collections.defaultdict or collections.Counter) or refactor the logic to ensure atomic updates to _tool_metric_names.
  3. OpenTelemetry Dependency Handling

    • The MCPMetrics class has a fallback to a no-op implementation if OpenTelemetry is not available. However, the _HAS_OTEL flag is set based on the presence of the opentelemetry module, but the actual initialization of the metrics may fail silently.
    • Action: Add explicit error handling for cases where OpenTelemetry initialization fails, and log a warning or error message to inform users.

🟡 WARNING

  1. Backward Compatibility

    • The agent-os package's __init__.py file has been modified to include new imports and exports for MCP governance components. While this change is necessary for the new package, it introduces potential backward compatibility issues for existing users of agent-os.
    • Action: Clearly document these changes in the release notes and ensure that any downstream dependencies are updated to accommodate the new exports.
  2. Python Version Requirement

    • The new package specifies requires-python = ">=3.12" in pyproject.toml. This is a breaking change for users on Python versions 3.9–3.11, which are supported by the rest of the repository.
    • Action: Reassess whether Python 3.12 is strictly necessary for this package. If not, consider lowering the minimum version requirement to align with the rest of the repository.

💡 SUGGESTIONS

  1. Unit Tests for CredentialRedactor

    • While the CredentialRedactor class is a critical component for security, there are no tests included in this PR for its functionality.
    • Action: Add comprehensive unit tests to validate the behavior of CredentialRedactor, including edge cases and potential false negatives/positives.
  2. Metrics Testing

    • The MCPMetrics class introduces OpenTelemetry-based metrics, but there are no tests to verify its behavior.
    • Action: Add tests to ensure that metrics are correctly recorded and that the fallback to NoOpMCPMetrics works as expected when OpenTelemetry is unavailable.
  3. Documentation

    • The README for agent-mcp-governance is clear and well-written, but it could benefit from additional details:
      • A section on extending the CredentialRedactor patterns.
      • Examples of how to configure and use the MCPMetrics class with OpenTelemetry.
    • Action: Expand the documentation to include these details.
  4. Performance Considerations for Credential Redaction

    • The CredentialRedactor class uses a series of regex patterns to scan and redact sensitive data. While this approach is effective, it may introduce performance overhead for large inputs or deeply nested data structures.
    • Action: Consider adding benchmarks to evaluate the performance of the CredentialRedactor and optimize the regex patterns if necessary.
  5. Security Scanner Integration

    • The MCPResponseScanner and MCPSecurityScanner components are re-exported but not directly tested in this PR.
    • Action: Ensure that these components are thoroughly tested in the context of the new package, especially for edge cases and potential bypass scenarios.
  6. Versioning and Dependency Management

    • The pyproject.toml file specifies a dependency on agent-os-kernel>=3.0.0,<4.0.0. This is a reasonable range, but it assumes that future minor versions of agent-os-kernel will not introduce breaking changes.
    • Action: Consider using stricter version pinning (e.g., ~=3.0.0) to minimize the risk of compatibility issues.

Final Recommendation

  • Address the 🔴 CRITICAL issues before merging.
  • Resolve the 🟡 WARNING issues to avoid potential backward compatibility problems.
  • Implement the 💡 SUGGESTIONS to improve the overall quality and maintainability of the package.

Once these changes are made, the PR will be ready for approval.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Pull Request Review: feat: add standalone agent-mcp-governance package


🔴 CRITICAL: Security Issues

  1. Credential Redaction Patterns:

    • The CredentialRedactor class includes patterns for detecting sensitive information like API keys, tokens, and private keys. However:
      • The regex for "Bearer token" (\bBearer\s+[A-Za-z0-9._\-+/=]{16,}\b) might miss tokens with uncommon characters or shorter lengths.
      • The "Generic API secret" regex ((?i)\b(?:api[_-]?key|client[_-]?secret|secret|token)\b\s*[:=]\s*['\"]?[^\s'\";]{6,}) could result in false positives for non-sensitive strings containing "token" or "secret".
    • Recommendation: Tighten regex patterns to reduce false positives and ensure comprehensive coverage. Consider adding unit tests for edge cases.
  2. Credential Redaction in Nested Structures:

    • The CredentialRedactor.redact_mapping method is incomplete in the provided diff. If it doesn't handle deeply nested structures or circular references, sensitive data might remain unredacted.
    • Recommendation: Ensure recursive traversal of nested structures with safeguards against circular references. Add tests for deeply nested and edge-case scenarios.
  3. OpenTelemetry Integration:

    • The MCPMetrics class uses OpenTelemetry for metrics recording. However, the _safe_tool_metric_name method buckets excess tool names into __other__ after reaching _MAX_TOOL_METRIC_CARDINALITY. This could lead to loss of granularity in metrics.
    • Recommendation: Log a warning when the cardinality limit is reached, and consider allowing configurable limits for enterprise use cases.
  4. Session Authentication:

    • The MCPSessionAuthenticator is used for session management, but the implementation details are not visible in this PR. Ensure:
      • Tokens are cryptographically secure and have appropriate expiration times.
      • Session validation is resistant to replay attacks.
      • Recommendation: Review the MCPSessionAuthenticator implementation in the dependent PR (#774) for these concerns.

🟡 WARNING: Potential Breaking Changes

  1. Python Version Requirement:

    • The new package agent-mcp-governance requires Python >=3.12. This is a breaking change for users on older Python versions.
    • Recommendation: Clearly document this requirement in the release notes and consider providing a rationale for requiring Python 3.12.
  2. Dependency on agent-os-kernel:

    • The agent_mcp_governance package depends on agent-os-kernel>=3.0.0,<4.0.0. If agent-os-kernel introduces breaking changes in future versions, it could impact this package.
    • Recommendation: Monitor agent-os-kernel updates and ensure compatibility testing.

💡 Suggestions for Improvement

  1. Documentation:

    • The README.md for agent-mcp-governance is clear but could benefit from:
      • A section on how to extend or customize the package.
      • Examples of integrating with OpenTelemetry for metrics.
      • Security best practices for using the package.
  2. Backward Compatibility:

    • The agent-os package now includes additional exports (e.g., CredentialRedactor, MCPMessageSigner). Ensure these additions do not conflict with existing APIs or introduce unexpected behavior.
  3. Testing:

    • Add comprehensive tests for:
      • Credential redaction, especially edge cases and nested structures.
      • Metrics recording with OpenTelemetry, including scenarios where OpenTelemetry is unavailable.
      • Session authentication and rate limiting.
  4. Thread Safety:

    • The MCPMetrics class uses a threading.Lock for managing tool metric names. While this ensures thread safety, it could become a bottleneck under high concurrency.
    • Recommendation: Evaluate performance under concurrent workloads and consider alternatives like collections.defaultdict with thread-safe access.
  5. OWASP MCP Compliance:

    • The PR references the OWASP MCP Security Cheat Sheet. Ensure:
      • All MCP governance components align with the cheat sheet recommendations.
      • Security hardening guidance is included in the documentation.

Summary

  • The PR introduces a valuable standalone package for MCP governance, but there are critical security concerns around credential redaction and session authentication that need to be addressed.
  • Potential breaking changes, such as the Python version requirement, should be clearly communicated to users.
  • Additional testing and documentation improvements are recommended to ensure robustness and ease of use.

Action Items:

  1. Address the 🔴 CRITICAL issues, especially around credential redaction and session authentication.
  2. Document the Python version requirement and monitor dependency compatibility.
  3. Expand testing coverage for edge cases and concurrency scenarios.
  4. Enhance documentation with examples, customization guidance, and security best practices.

Let me know if you need further clarification or assistance!

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This PR introduces a new standalone package, agent-mcp-governance, which provides MCP governance primitives without requiring the full Agent Governance Toolkit (AGT). The package is designed for enterprise use cases where a lightweight dependency is preferred. The PR also includes updates to the agent-os-kernel package to support this new package, as well as new utility classes for credential redaction and OpenTelemetry-based metrics.

The changes are well-structured and adhere to the project's style guidelines. However, there are a few areas that require attention to ensure security, maintainability, and backward compatibility.


🔴 CRITICAL

  1. Credential Redaction Incompleteness

    • The CredentialRedactor class provides a set of patterns for detecting and redacting sensitive information. However, the patterns may not cover all possible formats of sensitive data, especially for generic API secrets or tokens. For example:
      • The Generic API secret regex may not catch secrets without explicit labels like "key" or "token."
      • The Bearer token regex assumes a specific format, which may not cover all cases.
    • Recommendation: Consider adding more comprehensive patterns or allowing users to supply custom patterns. Additionally, document the limitations of the current patterns to set clear expectations.
  2. Thread Safety in MCPMetrics

    • The _tool_metric_names set and _tool_metric_names_lock are used to limit the cardinality of tool names in metrics. However, the _safe_tool_metric_name method does not handle the case where multiple threads might simultaneously check and add the same tool name, potentially leading to race conditions.
    • Recommendation: Use a thread-safe data structure like collections.defaultdict or ensure atomicity when adding to _tool_metric_names.
  3. Potential for Credential Leakage

    • The CredentialRedactor.redact_mapping method is not fully implemented in the provided diff. If this method is used without proper implementation, it could lead to sensitive data being inadvertently exposed.
    • Recommendation: Ensure the redact_mapping method is fully implemented and tested before merging this PR.

🟡 WARNING

  1. Backward Compatibility

    • The agent-os package now re-exports several MCP-related classes and functions. While this is useful for consistency, it introduces potential backward compatibility issues if these re-exports are removed or modified in the future.
    • Recommendation: Clearly document these re-exports in the release notes and consider marking them as "experimental" or "subject to change" if they are not intended to be permanent.
  2. Python Version Compatibility

    • The new package requires Python 3.12 or higher (requires-python = ">=3.12"). This is a breaking change for users on older Python versions.
    • Recommendation: Clearly communicate this requirement in the release notes and consider whether this version constraint is necessary.

💡 SUGGESTIONS

  1. Test Coverage

    • While the PR mentions that tests have been added, it's unclear if the new MCPMetrics and CredentialRedactor classes are fully covered.
    • Recommendation: Ensure that all new functionality is covered by unit tests, including edge cases like:
      • High cardinality of tool names in MCPMetrics.
      • Complex nested structures in CredentialRedactor.redact_mapping.
  2. OpenTelemetry Dependency

    • The MCPMetrics class conditionally uses OpenTelemetry if available. This is a good design choice, but it might lead to confusion if users expect metrics to work out of the box.
    • Recommendation: Add documentation to clarify that OpenTelemetry is optional and provide installation instructions for enabling it.
  3. Security Documentation

    • The README for agent-mcp-governance references the OWASP MCP Security Cheat Sheet, which is excellent. However, it would be helpful to include a brief summary of the key security practices or a direct link to the relevant sections.
    • Recommendation: Add a "Key Security Practices" section summarizing the most critical points from the OWASP MCP Security Cheat Sheet.
  4. Logging in MCPMetrics

    • The MCPMetrics class logs initialization failures at the debug level. This might not be sufficient for users who rely on metrics for monitoring.
    • Recommendation: Log initialization failures at the warning or error level to ensure they are visible in production environments.
  5. Credential Redaction Placeholder

    • The CredentialRedactor class uses a hardcoded placeholder ([REDACTED]) for redacted values. This might not suit all use cases.
    • Recommendation: Allow users to configure the placeholder text.

Conclusion

The PR is a well-structured addition to the repository and aligns with the project's goals of modularity and security. However, the issues highlighted above, particularly the critical ones, should be addressed before merging to ensure the robustness and security of the new functionality.

@jackbatzner jackbatzner changed the title feat: add standalone agent-mcp-governance package feat(python): add standalone agent-mcp-governance package Apr 6, 2026
@jackbatzner jackbatzner force-pushed the jb/python-mcp-package branch from ebcc7c1 to e52cd2c Compare April 6, 2026 14:49
@github-actions github-actions bot added size/L Large PR (< 500 lines) and removed tests labels Apr 6, 2026
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review of feat(python): add standalone agent-mcp-governance package

This PR introduces a new standalone package, agent-mcp-governance, which provides MCP governance primitives without requiring the full Agent Governance Toolkit. Below is a detailed review of the changes, focusing on the specified areas.


🔴 CRITICAL: Security Issues

  1. Lack of Cryptographic Key Management Details

    • The MCPMessageSigner and MCPSessionAuthenticator components are likely involved in cryptographic operations (e.g., signing, session token generation). However, there is no documentation or code to indicate how cryptographic keys are managed, stored, or rotated. Improper key management is a critical security risk.
    • Actionable Recommendation: Ensure that cryptographic keys are securely stored (e.g., using a hardware security module or a secure key management service). Add documentation on key management practices and ensure that keys are rotated periodically.
  2. Session Token Security

    • The MCPSessionAuthenticator appears to generate and validate session tokens. However, there is no mention of the cryptographic algorithm used for token generation or whether tokens are signed and/or encrypted.
    • Actionable Recommendation: Ensure that session tokens are signed using a secure algorithm (e.g., HMAC-SHA256 or RSA) and optionally encrypted to prevent tampering and replay attacks. Add tests to validate token integrity and expiration handling.
  3. Rate Limiting Bypass

    • The MCPSlidingRateLimiter implementation is not included in this PR, but its usage in the example code raises concerns about potential bypasses. For instance, if the rate_limit_key is predictable or not tied to a specific user/session, attackers could bypass rate limits by spoofing keys.
    • Actionable Recommendation: Ensure that rate_limit_key is securely derived (e.g., using a combination of user ID, session ID, and a cryptographic hash). Add tests to verify that rate limits cannot be bypassed.
  4. Credential Handling

    • The CredentialRedactor is re-exported, but there is no mention of how sensitive data is identified, redacted, or logged. Improper handling of credentials could lead to accidental exposure in logs.
    • Actionable Recommendation: Audit the CredentialRedactor implementation to ensure it reliably detects and redacts sensitive data. Add tests to verify its behavior with various edge cases (e.g., malformed credentials, partial matches).

🟡 WARNING: Potential Breaking Changes

  1. Python Version Requirement

    • The pyproject.toml specifies requires-python = ">=3.12". This is a breaking change for users running Python 3.9–3.11, which are supported by the rest of the toolkit.
    • Actionable Recommendation: Consider lowering the Python version requirement to >=3.9 unless there is a strong justification for requiring Python 3.12.
  2. Dependency on agent-os-kernel

    • The agent-os-kernel dependency is pinned to >=3.0.0,<4.0.0. If agent-os-kernel introduces breaking changes in a future 3.x release, this package will inherit those changes.
    • Actionable Recommendation: Monitor the agent-os-kernel changelog for breaking changes and add integration tests to detect regressions early.

💡 Suggestions for Improvement

  1. OWASP MCP Security Cheat Sheet

    • The README references the OWASP MCP Security Cheat Sheet, but it would be helpful to include a summary of key recommendations (e.g., secure session management, rate limiting best practices) directly in the documentation.
    • Actionable Recommendation: Add a "Security Best Practices" section to the README summarizing key points from the cheat sheet.
  2. Type Annotations and Pydantic Models

    • The example code includes a DemoPolicy class, but it lacks type annotations and validation. This could lead to runtime errors if invalid data is passed.
    • Actionable Recommendation: Use Pydantic models for policy definitions and validation. For example:
      from pydantic import BaseModel
      
      class DemoPolicy(BaseModel):
          name: str
          allowed_tools: list[str]
          max_tool_calls: int
          log_all_calls: bool
          require_human_approval: bool
  3. Thread Safety

    • The InMemoryRateLimitStore and InMemorySessionStore are re-exported, but there is no mention of their thread safety. If these components are used in a multi-threaded environment, race conditions could occur.
    • Actionable Recommendation: Audit these components for thread safety. If they are not thread-safe, document this limitation and recommend alternatives for concurrent environments.
  4. Test Coverage

    • While the PR mentions that tests have been added, there is no evidence of specific tests for critical components like MCPSessionAuthenticator, MCPGateway, or MCPSlidingRateLimiter.
    • Actionable Recommendation: Add unit tests and integration tests for these components, focusing on edge cases (e.g., invalid tokens, rate limit exhaustion, policy mismatches).
  5. Sandbox Escape Prevention

    • The MCPGateway enforces policies, but there is no mention of how it prevents sandbox escapes (e.g., via malicious tool calls or untrusted input).
    • Actionable Recommendation: Add tests to verify that the MCPGateway correctly enforces policies and prevents unauthorized actions. Consider integrating static analysis tools to detect potential sandbox escape vectors.

Summary of Feedback

  • 🔴 Critical Issues: Cryptographic key management, session token security, rate limiting bypass, and credential handling need immediate attention.
  • 🟡 Warnings: Python 3.12 requirement and dependency on agent-os-kernel could cause compatibility issues.
  • 💡 Suggestions: Improve documentation, add type annotations, validate thread safety, and expand test coverage.

By addressing these issues, the agent-mcp-governance package can provide a secure and reliable foundation for MCP governance in standalone scenarios.

@jackbatzner
Copy link
Copy Markdown
Contributor Author

CI Status Summary

All CI checks pass except:

Standalone agent-mcp-governance Python package. Slimmed to 4 files.

⚠️ Depends on #774 (Core) merging first. Merge order: #774#829#828.

@jackbatzner jackbatzner force-pushed the jb/python-mcp-package branch from e52cd2c to e459cd2 Compare April 6, 2026 20:06
@github-actions github-actions bot added the size/M Medium PR (< 200 lines) label Apr 6, 2026
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This PR introduces a new standalone package, agent-mcp-governance, which provides MCP governance primitives without requiring the full Agent Governance Toolkit (AGT). The package re-exports components from agent-os-kernel and is designed for lightweight adoption by MCP server operators. The implementation appears well-structured, with a clear focus on modularity and ease of use. However, there are some areas that require attention to ensure security, maintainability, and backward compatibility.


🔴 CRITICAL

  1. Lack of Input Validation in DemoPolicy.matches_pattern

    • The matches_pattern method in the DemoPolicy class is defined but does not validate or sanitize the _text input. This could lead to potential security vulnerabilities, such as injection attacks, if the method is extended or used in production.
    • Action: Implement input validation and sanitization in the matches_pattern method or provide clear documentation that this is a placeholder and should not be used in production without proper validation.
  2. Session Token Security

    • The MCPSessionAuthenticator.create_session method is used to generate session tokens, but the PR does not provide details about the cryptographic strength of the tokens or how they are stored and validated.
    • Action: Ensure that the session tokens are cryptographically secure (e.g., using a strong random number generator) and that the storage mechanism for tokens is secure against tampering and unauthorized access. Add tests to verify the security of session token generation and validation.
  3. Re-Exporting Security-Sensitive Components

    • The package re-exports several security-sensitive components, such as MCPMessageSigner, MCPSecurityScanner, and MCPSessionAuthenticator. However, there is no documentation or safeguards to ensure these components are used securely.
    • Action: Add security best practices and usage guidelines for these components in the documentation. Consider adding runtime checks or warnings for common misconfigurations.

🟡 WARNING

  1. Potential Breaking Changes in agent-os-kernel

    • The new package depends on agent-os-kernel>=3.0.0,<4.0.0. If agent-os-kernel introduces breaking changes in future minor versions, it could impact this package.
    • Action: Monitor agent-os-kernel for breaking changes and consider pinning to a specific version or range that has been tested with this package.
  2. Python Version Compatibility

    • The package requires Python 3.12 or higher, which may limit adoption for users on older Python versions.
    • Action: Consider supporting Python 3.9–3.11, as these versions are still widely used and supported.

💡 SUGGESTIONS

  1. Type Annotations and Pydantic Models

    • While the re-exported components seem to have type annotations, the DemoPolicy class in the README lacks type annotations for its attributes.
    • Action: Add type annotations to the DemoPolicy class in the README to promote best practices and ensure type safety.
  2. Thread Safety

    • The package includes components like MCPSlidingRateLimiter and MCPSessionAuthenticator, which may be used in concurrent environments. However, there is no explicit mention of thread safety.
    • Action: Review the thread safety of these components and document whether they are thread-safe. If they are not, provide guidance on how to use them safely in concurrent environments.
  3. OWASP MCP Security Cheat Sheet

    • The README references the OWASP MCP Security Cheat Sheet but does not provide specific examples of how the package aligns with its recommendations.
    • Action: Add a section to the README that explicitly maps the package's features to the OWASP MCP Security Cheat Sheet recommendations.
  4. Unit Tests for Re-Exported Components

    • The PR does not include tests for the re-exported components from agent-os-kernel.
    • Action: Add unit tests to verify that the re-exported components behave as expected and are compatible with the standalone package.
  5. Backward Compatibility

    • While this package is new and does not directly affect existing functionality, any future changes to the re-exported components in agent-os-kernel could introduce compatibility issues.
    • Action: Establish a process for testing this package against new versions of agent-os-kernel to ensure backward compatibility.
  6. Sandbox Escape Vectors

    • The package includes components like MCPGateway and MCPSecurityScanner, which may interact with external systems or execute code. There is no mention of safeguards against sandbox escapes.
    • Action: Review these components for potential sandbox escape vectors and document any mitigations.

Additional Notes

  • The documentation is clear and provides a good starting point for users. However, it could benefit from more detailed examples and security considerations.
  • The use of hatchling as the build backend is appropriate for a lightweight package like this.

Conclusion

The PR introduces a valuable addition to the Agent Governance Toolkit ecosystem, but it requires some critical security improvements and additional documentation to ensure safe and effective usage. Addressing the issues flagged as 🔴 CRITICAL should be prioritized before merging.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jackbatzner jackbatzner force-pushed the jb/python-mcp-package branch from e459cd2 to 7f7ae60 Compare April 8, 2026 13:26
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review of feat(python): add standalone agent-mcp-governance package

This PR introduces a new standalone package, agent-mcp-governance, which re-exports MCP governance primitives from agent-os-kernel. The goal is to provide a lightweight package for MCP governance without requiring the full Agent Governance Toolkit (AGT). Below is a detailed review of the changes:


🔴 CRITICAL

1. Lack of Cryptographic Key Management

The MCPMessageSigner and MCPVerificationResult are re-exported, but there is no mention or documentation about how cryptographic keys are managed. This is a critical security concern because:

  • Improper key management can lead to unauthorized access or data tampering.
  • There is no mention of secure storage or rotation of keys.

Recommendation:

  • Ensure that the documentation includes clear guidance on securely managing cryptographic keys.
  • If the package provides any default key management mechanisms, ensure they are secure and well-documented.

2. Session Token Validation

The MCPSessionAuthenticator is used for session management, but there is no mention of how session tokens are generated, validated, or expired. This could lead to vulnerabilities such as session hijacking or replay attacks.

Recommendation:

  • Document the session token generation and validation process.
  • Ensure that tokens are cryptographically secure, have a limited lifespan, and are resistant to replay attacks.

3. Credential Handling

The CredentialRedactor is re-exported, but there is no mention of how sensitive data is handled or stored. If this class is used for redacting sensitive information, it must be explicitly documented to avoid misuse.

Recommendation:

  • Provide documentation on how CredentialRedactor should be used securely.
  • Ensure that sensitive data is not logged or stored in plaintext.

4. Sandbox Escape Vectors

The MCPGateway allows for policy enforcement and tool call interception. However, there is no mention of how the policies are sandboxed or isolated. This could lead to sandbox escape vulnerabilities if malicious agents exploit weak policies.

Recommendation:

  • Ensure that policies are executed in a secure, isolated environment.
  • Document any potential risks and mitigations related to sandboxing.

🟡 WARNING

1. Backward Compatibility

The new package re-exports MCP primitives from agent-os-kernel. If the APIs in agent-os-kernel change in the future, it could break the agent-mcp-governance package.

Recommendation:

  • Clearly document that agent-mcp-governance depends on agent-os-kernel version >=3.0.0,<4.0.0.
  • Add tests to ensure compatibility with future versions of agent-os-kernel.

2. Python Version Restriction

The pyproject.toml specifies requires-python = ">=3.12". This is a breaking change for users on Python 3.9–3.11, which are supported by the rest of the repository.

Recommendation:

  • Evaluate whether the package can support Python 3.9–3.11 to align with the rest of the repository.
  • If not, clearly document the Python 3.12 requirement in the README.

💡 SUGGESTIONS

1. Test Coverage

While the PR mentions that tests have been added, there is no information about the specific tests for this package. Given the security-sensitive nature of the MCP governance primitives, comprehensive test coverage is essential.

Recommendation:

  • Add tests for all re-exported classes and functions, especially those related to cryptographic operations, session management, and policy enforcement.
  • Include edge cases and failure scenarios in the tests.

2. OWASP MCP Security Cheat Sheet Mapping

The README mentions the OWASP MCP Security Cheat Sheet but does not provide specific mappings to the re-exported primitives.

Recommendation:

  • Add a table or section in the README that maps each re-exported primitive to the relevant OWASP MCP Security Cheat Sheet recommendation.

3. Type Annotations

The DemoPolicy class in the README example lacks type annotations for its attributes. This could lead to confusion or misuse.

Recommendation:

  • Add type annotations to the DemoPolicy class in the README example to promote best practices.

4. Rate Limiter Configuration

The MCPSlidingRateLimiter is instantiated with hardcoded values in the README example. This could lead to misuse in production environments.

Recommendation:

  • Add a note in the README to highlight that rate limiter configurations should be carefully chosen based on the deployment environment.

5. Future-proofing the Package

The package is described as "intentionally thin," but this could lead to challenges if additional functionality is needed in the future.

Recommendation:

  • Consider adding a roadmap or guidelines for extending the package while maintaining its lightweight nature.

Summary

This PR introduces a useful and lightweight package for MCP governance. However, there are critical security concerns related to cryptographic key management, session token validation, credential handling, and sandboxing. Additionally, there are potential backward compatibility issues and Python version restrictions to address. With these improvements, the package will be more secure, robust, and aligned with best practices.

Actions Required:

  1. Address the critical security concerns (🔴 CRITICAL).
  2. Evaluate and mitigate the potential backward compatibility and Python version issues (🟡 WARNING).
  3. Implement the suggested improvements (💡 SUGGESTIONS) to enhance the package's usability and security posture.

Let me know if you need further clarification or assistance!

imran-siddique added a commit to imran-siddique/agent-governance-toolkit that referenced this pull request Apr 9, 2026
- Migrate copilot-governance from legacy AT identifiers to OWASP ASI 2026
- Add backward-compatible AT→ASI lookup for existing integrations
- Add comprehensive OWASP Agentic Top 10 reference architecture doc
- Add standalone agent-mcp-governance Python package

Supersedes: microsoft#839, microsoft#843, microsoft#844, microsoft#829

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

Closing in favor of #910 which includes a corrected version of the standalone MCP governance package:

Thank you for the package extraction idea — it's a good architectural pattern.

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

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/L Large PR (< 500 lines) size/M Medium PR (< 200 lines) size/XL Extra large PR (500+ lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants