feat(typescript): add MCP protocol governance with full OWASP coverage

[jackbatzner]

Description

Adds MCP (Model Context Protocol) security primitives to the TypeScript Agent Mesh SDK 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 bounded nonce replay cache and concurrent replay protection
• McpSessionManager — rate-limited session lifecycle with serialized issuance and inactive bucket eviction
• McpToolRegistry — schema-validated tool registration with thread-safe operations
• McpInputSanitizer — regex-based input validation with time-budget fail-closed behavior
• CredentialRedactor — full PEM block + secret pattern redaction for audit-safe logging
• Enterprise patterns: persistence seams (SignerNonceStore, SessionStore), clock injection, fail-closed enforcement

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

• Part 1: This PR ← merge first
• Part 2: feat(typescript): add standalone @microsoft/agentmesh-mcp-governance package #827 Standalone @microsoft/agentmesh-mcp-governance package
• Part 3: docs(typescript): add MCP security documentation and examples #825 Documentation and examples ← merge last

Type of Change

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

Package(s) Affected

• agent-mesh

Checklist

• My code follows the project style guidelines
• I have added tests that prove my fix/feature works
• All new and existing tests pass (npm test)
• 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 #827 and #825.

[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: code-reviewer

Code Review for feat(typescript): add MCP protocol governance with full OWASP coverage

---

🔴 CRITICAL: Security Issues

1. Missing Replay Protection in McpMessageSigner:

   • Issue: The McpMessageSigner implementation does not include a mechanism to track and prevent replay attacks. While HMAC-SHA256 is used for signing, without nonce tracking or timestamp validation, an attacker could replay a previously valid signed message.
   • Recommendation: Implement a nonce or timestamp validation mechanism to ensure that each message is unique and cannot be replayed. Store used nonces in a database or in-memory store with a TTL to prevent reuse.

2. Potential Credential Leakage in Audit Logs:

   • Issue: The auditSink.record method in McpGateway includes params in the audit log, even though it is redacted. However, there is no guarantee that the redaction process will catch all sensitive data, especially if custom patterns are not comprehensive.
   • Recommendation: Ensure that the CredentialRedactor is thoroughly tested with a wide range of sensitive data patterns. Additionally, consider hashing or encrypting sensitive fields before logging them, even if they are redacted.

3. Blocked Patterns and Dangerous Patterns:

   • Issue: The checkSanitization method in McpGateway uses hardcoded patterns (BUILTIN_DANGEROUS_PATTERNS) and user-supplied blockedPatterns. However, there is no validation or sanitization of these patterns, which could lead to unexpected behavior or even ReDoS (Regular Expression Denial of Service) vulnerabilities.
   • Recommendation: Validate and sanitize all user-supplied patterns to ensure they are safe and efficient. Consider using libraries like safe-regex to detect potentially unsafe regular expressions.

4. Fail-Closed Behavior in McpGateway:

   • Issue: While the McpGateway is designed to fail closed on errors, the finalize method catches errors and returns a generic "Internal error - access denied (fail-closed)" message. This could potentially mask critical issues in the system.
   • Recommendation: Log detailed error information (excluding sensitive data) to help diagnose the root cause of failures. Consider adding a mechanism to alert administrators when such errors occur frequently.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Stability:

   • Issue: The PR introduces a new package (@microsoft/agentmesh-mcp-governance) marked as "Public Preview" with a disclaimer that APIs may change before GA. This could lead to breaking changes for early adopters.
   • Recommendation: Clearly document the expected timeline for API stabilization and provide guidance for users on how to handle potential breaking changes in future releases.

2. Dual-Package Architecture:

   • Issue: The introduction of a dual-package architecture (@microsoft/agentmesh-sdk and @microsoft/agentmesh-mcp-governance) may confuse users about which package to use.
   • Recommendation: Provide clear documentation and decision trees to help users choose the appropriate package for their use case.

---

💡 Suggestions for Improvement

1. Enhanced OWASP Coverage:

   • Observation: The PR claims compliance with 11 out of 12 sections of the OWASP MCP Security Cheat Sheet. However, §11 (Consent UI) is marked as "N/A" because it is a server-side SDK.
   • Suggestion: Consider adding guidance or APIs to facilitate the implementation of consent UI in client applications that use the SDK. This would provide a more comprehensive solution.

2. Test Coverage:

   • Observation: While the PR includes Jest configuration and mentions tests, the actual test files are not included in the diff.
   • Suggestion: Ensure that all critical components, especially security-related ones like McpMessageSigner, McpGateway, and CredentialRedactor, have comprehensive unit and integration tests. Include tests for edge cases, such as malformed inputs and high-concurrency scenarios.

3. Concurrency and Thread Safety:

   • Observation: The McpGateway class uses an in-memory rate limiter (McpSlidingRateLimiter) by default. This could lead to race conditions in multi-threaded or distributed environments.
   • Suggestion: Clearly document that the in-memory rate limiter is not thread-safe and recommend using a distributed store (e.g., Redis) for production deployments.

4. Type Safety:

   • Observation: The TypeScript types are generally well-defined, but there are some areas where stricter typing could improve robustness. For example, the params argument in evaluateToolCall is typed as Record<string, unknown>, which is very generic.
   • Suggestion: Use stricter types or schemas (e.g., with zod or io-ts) to validate the structure of params and other inputs.

5. Documentation:

   • Observation: The README for @microsoft/agentmesh-mcp-governance provides a good overview but lacks detailed examples for advanced use cases, such as integrating with custom storage backends or implementing approvalHandler.
   • Suggestion: Expand the documentation to include more detailed examples and best practices for using the package in production environments.

6. Performance Considerations:

   • Observation: The CredentialRedactor processes strings and objects recursively, which could lead to performance issues with deeply nested structures.
   • Suggestion: Add a maximum recursion depth to prevent potential stack overflows or performance degradation when processing deeply nested objects.

7. Error Handling:

   • Observation: The McpGateway class has a generic error handling mechanism that catches all errors and returns a fail-closed decision.
   • Suggestion: Differentiate between recoverable and non-recoverable errors. For example, if the rate limiter fails, it might be better to allow the request rather than deny it, depending on the use case.

8. Logging:

   • Observation: The McpGateway class does not include detailed logging for debugging purposes.
   • Suggestion: Add structured logging for key events, such as policy evaluations, rate limit checks, and audit log writes. Ensure that sensitive data is excluded from logs.

---

Summary of Recommendations

• Critical: Address replay protection in McpMessageSigner, validate user-supplied patterns, and ensure audit logs are secure.
• Warning: Clearly communicate the potential for breaking changes in the new package and provide guidance for users.
• Suggestions: Improve test coverage, documentation, type safety, and logging. Address potential performance and concurrency issues.

Let me know if you need further clarification or assistance!

[github-actions]

🤖 AI Agent: security-scanner — Security Review of PR: `feat(typescript): add MCP protocol governance with full OWASP coverage`

Security Review of PR: feat(typescript): add MCP protocol governance with full OWASP coverage

This PR introduces security primitives for the MCP (Model Context Protocol) in the TypeScript Agent Mesh SDK. Below is a detailed security analysis based on the provided diff.

---

Findings

1. Prompt Injection Defense Bypass

• Risk: The MCPGateway class includes a checkSanitization method to block dangerous patterns in input parameters. However:
  • The BUILTIN_DANGEROUS_PATTERNS list and hasShellExpansion function may not cover all potential injection vectors (e.g., nested or obfuscated payloads).
  • The reliance on JSON.stringify for serialization before scanning may allow attackers to bypass detection by encoding payloads in non-JSON-compatible formats.
• Severity: 🔴 CRITICAL
• Attack Vector: An attacker could craft input that bypasses the regex-based sanitization, allowing malicious commands or payloads to execute.
• Recommendation:
  • Use a more robust input validation library or framework (e.g., OWASP JSON Sanitizer).
  • Expand the BUILTIN_DANGEROUS_PATTERNS list to include obfuscated patterns and ensure comprehensive coverage.
  • Add recursive and context-aware sanitization for nested objects.

---

2. Policy Engine Circumvention

• Risk: The policyEvaluator in MCPGateway is optional, and if not provided, no policy checks are enforced.
• Severity: 🟠 HIGH
• Attack Vector: If a developer forgets to configure a policyEvaluator, tools could be executed without any policy validation.
• Recommendation:
  • Make policyEvaluator mandatory or provide a default fail-closed implementation that denies all requests unless explicitly allowed.

---

3. Trust Chain Weaknesses

• Risk: The McpMessageSigner and McpMessageValidator classes rely on HMAC-SHA256 for message signing and verification. However:
  • There is no explicit mention of SPIFFE/SVID validation or certificate pinning for identity verification.
• Severity: 🟠 HIGH
• Attack Vector: Without proper trust chain validation, an attacker could impersonate a trusted agent or replay signed messages.
• Recommendation:
  • Integrate SPIFFE/SVID validation for agent identities.
  • Implement certificate pinning or mutual TLS for secure communication.

---

4. Credential Exposure

• Risk: The CredentialRedactor class is designed to redact sensitive information from logs. However:
  • The BUILTIN_PATTERNS list may not cover all sensitive data formats (e.g., custom API keys, proprietary tokens).
  • Redaction is applied only to explicitly matched patterns, leaving room for accidental exposure.
• Severity: 🟡 MEDIUM
• Attack Vector: Sensitive credentials could be logged if they do not match predefined patterns.
• Recommendation:
  • Allow users to define additional patterns dynamically.
  • Add a fallback mechanism to redact any unrecognized but suspicious-looking strings (e.g., long alphanumeric sequences).

---

5. Sandbox Escape

• Risk: The checkSanitization method in MCPGateway attempts to block shell expansion ($()), but it does not account for other shell injection techniques (e.g., backticks, pipes).
• Severity: 🔴 CRITICAL
• Attack Vector: An attacker could craft input that exploits unhandled shell injection vectors to escape the sandbox.
• Recommendation:
  • Use a comprehensive shell injection detection library (e.g., shell-escape or shescape).
  • Perform strict input validation and enforce a whitelist of allowed characters.

---

6. Deserialization Attacks

• Risk: The MCPGateway class uses JSON.stringify and JSON.parse for serialization and deserialization. However:
  • There is no validation of the structure or type of deserialized objects.
• Severity: 🟠 HIGH
• Attack Vector: An attacker could exploit this to inject malicious objects or overload the system with large payloads.
• Recommendation:
  • Use schema validation (e.g., ajv or zod) to enforce strict type checking on deserialized data.
  • Limit the size of input payloads to prevent resource exhaustion.

---

7. Race Conditions

• Risk: The McpMessageSigner uses a nonce replay cache to prevent replay attacks. However:
  • There is no explicit mention of locking or atomic operations to handle concurrent access to the cache.
• Severity: 🟡 MEDIUM
• Attack Vector: An attacker could exploit a race condition to bypass nonce replay protection.
• Recommendation:
  • Use a thread-safe or distributed cache with atomic operations (e.g., Redis with SETNX).
  • Add unit tests to simulate concurrent access scenarios.

---

8. Supply Chain Risks

• Risk: The PR introduces several new dependencies (e.g., @typescript-eslint, ts-jest). However:
  • There is no evidence of dependency pinning or verification of package integrity.
• Severity: 🟡 MEDIUM
• Attack Vector: An attacker could exploit dependency confusion or typosquatting to introduce malicious code.
• Recommendation:
  • Pin all dependencies to specific versions in package.json.
  • Use a tool like npm audit or Snyk to scan for vulnerabilities.
  • Implement a dependency allowlist to prevent unauthorized packages.

---

Summary of Findings

Category	Severity	Description
Prompt Injection Defense	🔴 CRITICAL	Insufficient sanitization for nested/obfuscated payloads.
Policy Engine Circumvention	🟠 HIGH	Optional policyEvaluator allows bypassing policy checks.
Trust Chain Weaknesses	🟠 HIGH	Lack of SPIFFE/SVID validation and certificate pinning.
Credential Exposure	🟡 MEDIUM	Limited redaction patterns may expose sensitive data in logs.
Sandbox Escape	🔴 CRITICAL	Incomplete shell injection detection allows sandbox escape.
Deserialization Attacks	🟠 HIGH	Lack of schema validation for deserialized objects.
Race Conditions	🟡 MEDIUM	Potential race condition in nonce replay cache.
Supply Chain Risks	🟡 MEDIUM	No dependency pinning or verification against supply chain attacks.

---

Recommendations

1. Prompt Injection Defense:

   • Use a robust input sanitization library.
   • Expand dangerous pattern detection to include nested and obfuscated payloads.

2. Policy Engine:

   • Make policyEvaluator mandatory or provide a default fail-closed implementation.

3. Trust Chain:

   • Integrate SPIFFE/SVID validation and certificate pinning.

4. Credential Redaction:

   • Allow dynamic pattern definitions and implement a fallback redaction mechanism.

5. Sandbox Escape:

   • Use a comprehensive shell injection detection library and enforce strict input validation.

6. Deserialization:

   • Enforce schema validation and limit payload size.

7. Race Conditions:

   • Use a thread-safe cache for nonce replay protection.

8. Supply Chain:

   • Pin dependencies, scan for vulnerabilities, and implement an allowlist.

---

Final Assessment

This PR introduces critical security features but also contains several high-risk vulnerabilities that must be addressed before merging. Immediate attention should be given to prompt injection defense, sandbox escape prevention, and policy engine enforcement.

[jackbatzner]

Resolved. All flagged regex patterns have been replaced with non-regex string scanning in subsequent commits.

[jackbatzner]

Resolved. All flagged regex patterns have been replaced with non-regex string scanning in subsequent commits.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces a new TypeScript package, @microsoft/agentmesh-mcp-governance, which provides standalone MCP (Model Context Protocol) governance primitives. The implementation aims to address 11 out of 12 sections of the OWASP MCP Security Cheat Sheet, with a focus on security, modularity, and extensibility. The PR also includes updates to CI/CD workflows, documentation, and configuration files.

The implementation appears well-structured and adheres to modern TypeScript and security best practices. However, there are some areas that require attention to ensure correctness, security, and maintainability.

---

🔴 CRITICAL

1. Fail-Closed Behavior in McpGateway

   • The McpGateway class has a fail-closed mechanism in the evaluateToolCall method, which is a good practice. However, the finalize method also has a try-catch block that defaults to a fail-closed decision in case of an error. While this is generally a good approach, it is critical to ensure that the auditSink.record and metrics.recordDecision calls cannot throw exceptions that would inadvertently trigger the fail-closed behavior. If these methods throw exceptions, it could result in legitimate requests being denied. Consider wrapping these calls in their own try-catch blocks to ensure that the fail-closed mechanism is only triggered by critical errors.

2. Credential Redaction

   • The CredentialRedactor class uses a default set of patterns for sensitive data redaction. While this is a good starting point, the patterns may not cover all possible sensitive data formats. For example, there is no pattern for credit card CVV codes or other sensitive information. Additionally, the SENSITIVE_KEY_PATTERN regex might produce false positives or false negatives. Consider allowing users to provide their own sensitive key patterns or extending the default patterns to cover more cases.

3. Replay Protection in McpMessageSigner

   • The PR mentions HMAC-SHA256 message signing with timestamp and nonce replay protection, but the implementation of McpMessageSigner is not included in the diff. Ensure that the replay protection mechanism is robust and that nonces are stored securely and checked for uniqueness. If this is not implemented correctly, it could lead to replay attacks. Please confirm that this is handled securely.

4. Blocked Patterns in McpGateway

   • The checkSanitization method in McpGateway uses both user-provided and built-in patterns to detect dangerous or blocked patterns. However, the built-in patterns (BUILTIN_DANGEROUS_PATTERNS) include some generic patterns like \b\d{4}[- ]?\d{4}[- ]?\d{4}[- ]?\d{4}\b (credit card numbers) and ;\s*(?:rm|del|format|mkfs)\b. These patterns could produce false positives, especially in non-English contexts or when processing legitimate data. Consider providing a mechanism to customize or disable built-in patterns to reduce the risk of false positives.

---

🟡 WARNING

1. Potential Breaking Changes

   • The introduction of the new package @microsoft/agentmesh-mcp-governance and its integration into the existing @microsoft/agentmesh-sdk package may introduce breaking changes for users of the SDK. While the PR mentions that the new package is in "Public Preview" and APIs may change before GA, it is important to clearly communicate this to users in the documentation and release notes. Consider adding a prominent warning in the README and changelog.

2. Backward Compatibility

   • The new package introduces several new APIs and changes to the existing SDK. Ensure that these changes are backward-compatible with the current version of the SDK. If any breaking changes are introduced, they should be documented in the changelog, and a major version bump should be considered.

---

💡 SUGGESTIONS

1. Testing Coverage

   • While the PR mentions that tests have been added, the diff does not include the test files. Ensure that the following areas are thoroughly tested:
     • Fail-closed behavior in McpGateway and CredentialRedactor.
     • Edge cases for CredentialRedactor, including nested objects, circular references, and edge cases for regex patterns.
     • Replay protection in McpMessageSigner.
     • Rate limiting behavior in McpSlidingRateLimiter, including edge cases like burst traffic and long-term usage.
     • Integration tests for McpGateway with all security primitives enabled.

2. Documentation

   • The README for @microsoft/agentmesh-mcp-governance is clear and well-written. However, consider adding more examples for advanced use cases, such as customizing patterns in CredentialRedactor or implementing a custom approvalHandler.

3. Type Safety

   • Ensure that all exported types and interfaces are well-documented and include examples of usage. This will help developers understand how to use the new APIs correctly.

4. Thread Safety

   • The PR does not provide details about thread safety, especially for shared resources like the rateLimiter and auditSink. Ensure that these components are thread-safe and can handle concurrent requests without race conditions.

5. Performance

   • The CredentialRedactor and McpGateway classes perform regex operations on potentially large objects. Consider benchmarking these operations to ensure they do not introduce significant performance overhead, especially in high-throughput scenarios.

6. Error Handling

   • While the fail-closed behavior is a good practice, consider adding more granular error messages to help developers debug issues. For example, if the auditSink.record method fails, the error message should indicate that the failure occurred during audit logging.

---

Summary of Actionable Feedback

🔴 CRITICAL

1. Ensure that the fail-closed behavior in McpGateway does not inadvertently deny legitimate requests due to non-critical errors.
2. Expand the default sensitive data patterns in CredentialRedactor and allow users to customize them.
3. Confirm that the replay protection in McpMessageSigner is robust and secure.
4. Provide a mechanism to customize or disable built-in dangerous patterns in McpGateway.

🟡 WARNING

1. Clearly document potential breaking changes and consider a major version bump if necessary.
2. Verify backward compatibility with the existing SDK.

💡 SUGGESTIONS

1. Ensure comprehensive test coverage for all new features and edge cases.
2. Enhance documentation with advanced usage examples.
3. Verify and document thread safety for shared resources.
4. Benchmark performance of regex-heavy operations in CredentialRedactor and McpGateway.
5. Improve error handling to provide more granular and actionable error messages.

Let me know if you need further clarification or additional feedback!

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces a new TypeScript package, @microsoft/agentmesh-mcp-governance, which provides standalone MCP governance primitives for the AgentMesh platform. The implementation is robust and aligns with the OWASP MCP Security Cheat Sheet, covering 11 out of 12 sections. The package includes features like centralized policy enforcement, session authentication, message signing, rate limiting, and credential redaction. The code is well-structured, with a focus on security, modularity, and testability.

However, there are some areas that require attention to ensure the robustness, security, and maintainability of the codebase. Below are the detailed findings and recommendations:

---

🔴 CRITICAL

1. Fail-Closed Behavior in McpGateway

   • The evaluateToolCall method in McpGateway has a catch-all catch block that defaults to a fail-closed decision. While this is a good security practice, the catch block does not log the error or provide any mechanism for debugging. This could make it difficult to identify and fix issues during runtime.
   • Recommendation: Log the error details in the catch block using a secure logging mechanism that ensures sensitive data is not exposed.

   } catch (error) {
       console.error('Error in evaluateToolCall:', error); // Replace with a secure logging mechanism
       return {
           allowed: false,
           reason: 'Internal error - access denied (fail-closed)',
           redactedParams: this.redactor.redact(params).redacted as Record<string, unknown>,
           findings: [],
       };
   }

2. Credential Redaction

   • The CredentialRedactor class uses a default set of patterns for detecting sensitive information. However, there is no mechanism to validate the custom patterns provided in the CredentialRedactorConfig. Malformed or overly broad patterns could lead to unintended behavior, such as excessive redaction or missed sensitive data.
   • Recommendation: Add validation for custom patterns to ensure they are valid regular expressions and do not conflict with built-in patterns.

3. Replay Protection in McpMessageSigner

   • While the McpMessageSigner is mentioned as providing HMAC-SHA256 payload signing with timestamp and nonce replay protection, the actual implementation of replay protection is not included in the provided diff.
   • Recommendation: Ensure that the McpMessageSigner implementation includes robust replay protection mechanisms, such as maintaining a cache of used nonces and timestamps to prevent replay attacks.

4. Blocked Patterns in McpGateway

   • The checkSanitization method in McpGateway uses a combination of string matching and regular expressions to identify blocked patterns. However, there is no validation to ensure that the patterns provided in config.blockedPatterns are safe and do not introduce performance issues or false positives.
   • Recommendation: Validate the blockedPatterns during initialization to ensure they are well-formed and do not contain overly broad or inefficient patterns.

---

🟡 WARNING

1. Potential Breaking Changes
   • The introduction of a new package, @microsoft/agentmesh-mcp-governance, and its integration into the existing @microsoft/agentmesh-sdk package could potentially introduce breaking changes for existing users of the SDK. While the PR mentions that this is a non-breaking change, the dual-package architecture might require users to update their imports or configurations.
   • Recommendation: Clearly document the migration path for existing users and provide examples of how to update their code to use the new package.

---

💡 SUGGESTIONS

1. Test Coverage

   • While the PR mentions that tests have been added, there is no information about the extent of test coverage for the new package.
   • Recommendation: Ensure that all critical paths, especially security-related features like rate limiting, message signing, and credential redaction, are thoroughly tested. Include tests for edge cases, such as malformed input and unexpected errors.

2. Documentation

   • The README for the new package provides a good overview of its features and usage. However, it would be helpful to include more detailed examples, especially for advanced use cases like integrating with custom storage backends or implementing custom approval handlers.
   • Recommendation: Expand the documentation to include detailed examples and best practices for using the package in production environments.

3. Type Safety

   • The codebase appears to use TypeScript effectively, but there are some areas where type safety could be improved. For example, the redactNode method in CredentialRedactor uses unknown as the type for the value parameter, which could lead to runtime errors if the type is not properly checked.
   • Recommendation: Use more specific types or type guards to ensure type safety and reduce the risk of runtime errors.

4. Performance Optimization

   • The redactString method in CredentialRedactor uses matchAll to find all matches for a pattern and then iterates over them to apply redactions. This could be optimized to reduce the number of iterations and improve performance.
   • Recommendation: Consider using a single pass through the string to apply all patterns, or explore more efficient string processing techniques.

5. Concurrency and Thread Safety

   • The McpSlidingRateLimiter and other stateful components like InMemoryMCPAuditSink may not be thread-safe, especially in multi-threaded or distributed environments.
   • Recommendation: Clearly document the thread-safety guarantees of these components and provide guidance on how to use them safely in multi-threaded or distributed systems.

6. Backward Compatibility

   • The PR mentions that APIs may change before GA. This could impact users who adopt the package during the public preview phase.
   • Recommendation: Use semantic versioning to clearly indicate breaking changes and provide a migration guide for each major version update.

---

Summary of Actions

• 🔴 CRITICAL: Add error logging in McpGateway fail-closed behavior.
• 🔴 CRITICAL: Validate custom patterns in CredentialRedactorConfig.
• 🔴 CRITICAL: Ensure robust replay protection in McpMessageSigner.
• 🔴 CRITICAL: Validate blockedPatterns in McpGatewayConfig.
• 🟡 WARNING: Document potential breaking changes and provide a migration guide.
• 💡 SUGGESTION: Expand test coverage for critical paths and edge cases.
• 💡 SUGGESTION: Improve documentation with advanced use case examples.
• 💡 SUGGESTION: Enhance type safety in methods like redactNode.
• 💡 SUGGESTION: Optimize redactString for better performance.
• 💡 SUGGESTION: Document thread-safety guarantees for stateful components.

Let me know if you need further clarification or assistance!

[imran-siddique]

The TypeScript MCP governance code looks good, but this PR modifies ci.yml and publish.yml (same issue as #775). Please remove the .github/ changes and submit the code portion only — we'll handle CI integration for new language targets.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: feat(typescript): add MCP protocol governance with full OWASP coverage

This pull request introduces a new TypeScript package, @microsoft/agentmesh-mcp-governance, which implements comprehensive MCP (Model Context Protocol) governance primitives. The implementation claims to cover 11 out of 12 sections of the OWASP MCP Security Cheat Sheet, with a focus on security, compliance, and extensibility.

---

🔴 CRITICAL

1. Potential Denial of Service via Regex DoS

• File: credential-redactor.ts, gateway.ts
• Issue: The use of regular expressions in CredentialRedactor and McpGateway introduces the risk of Regular Expression Denial of Service (ReDoS) attacks. Specifically:
  • CredentialRedactor uses multiple regex patterns, including user-supplied custom patterns, without validating their complexity.
  • McpGateway uses regex patterns for blocked and dangerous pattern detection, which could be exploited with crafted inputs to cause excessive backtracking.
• Recommendation:
  • Use safe regex libraries like safe-regex to validate regex patterns before use.
  • Implement a timeout mechanism for regex execution to prevent long-running operations from blocking the event loop.

2. Credential Leakage in Audit Logs

• File: gateway.ts
• Issue: While the CredentialRedactor is used to redact sensitive information, there is no guarantee that all sensitive data is redacted before being logged in the audit sink. This could lead to credential leakage in logs.
• Recommendation:
  • Add a validation step to ensure that all audit log entries are free from sensitive information before they are logged.
  • Consider adding a test case to verify that no sensitive information is logged.

3. Insufficient Replay Protection

• File: Not explicitly shown in the diff, but relevant to MCPSessionAuthenticator and MCPMessageSigner.
• Issue: While MCPMessageSigner implements HMAC-SHA256 for message signing, there is no explicit mention of nonce storage or validation to prevent replay attacks.
• Recommendation:
  • Ensure that nonces are stored in a secure, tamper-proof manner (e.g., in a database or in-memory store with expiration).
  • Add tests to verify that replayed requests with the same nonce are rejected.

---

🟡 WARNING

1. Potential Breaking Changes

• Issue: The introduction of a new package, @microsoft/agentmesh-mcp-governance, and its integration into the existing @microsoft/agentmesh-sdk could potentially introduce breaking changes for users of the SDK. While the PR mentions that this is a non-breaking change, the dual-package architecture might lead to confusion or unexpected behavior for existing users.
• Recommendation:
  • Clearly document the migration path for existing users.
  • Ensure backward compatibility by maintaining the existing API surface in @microsoft/agentmesh-sdk while introducing the new functionality.

---

💡 SUGGESTIONS

1. Enhanced Test Coverage

• Observation: While the PR mentions that tests have been added, the diff does not include any test files.
• Recommendation:
  • Ensure that all critical functionalities, especially security-related ones, are covered by unit and integration tests.
  • Add tests for edge cases, such as malformed inputs, large payloads, and concurrent requests.

2. Thread Safety

• Observation: The McpGateway and CredentialRedactor classes use shared resources like WeakMap and RegexScanBudget. There is no explicit mention of thread safety mechanisms.
• Recommendation:
  • Ensure that shared resources are accessed in a thread-safe manner, especially if the library is intended to be used in multi-threaded environments.
  • Consider using thread-safe data structures or synchronization primitives if necessary.

3. Input Validation

• Observation: The evaluateToolCall method in McpGateway accepts params as a Record<string, unknown> without any validation.
• Recommendation:
  • Use a schema validation library (e.g., zod or yup) to validate the structure and types of params.
  • This will help prevent unexpected behavior due to malformed or malicious inputs.

4. Documentation Improvements

• Observation: The README provides a good overview of the package, but it lacks detailed examples for advanced use cases, such as integrating with custom storage backends or implementing custom approval handlers.
• Recommendation:
  • Add detailed examples for advanced use cases to the documentation.
  • Include a section on best practices for secure deployment, such as using durable storage for nonces and rate limits.

5. Error Handling

• Observation: The evaluateToolCall method has a catch-all error handler that returns a generic error message.
• Recommendation:
  • Log the actual error details (excluding sensitive information) for debugging purposes.
  • Consider categorizing errors to provide more specific feedback to developers.

6. Performance Considerations

• Observation: The use of stableStringify in checkSanitization could be a performance bottleneck for large objects.
• Recommendation:
  • Evaluate the performance impact of stableStringify on large payloads.
  • Consider alternative approaches, such as streaming serialization or partial serialization, to improve performance.

7. Type Safety

• Observation: The use of unknown in redact and redactNode methods in CredentialRedactor could lead to runtime errors if the input is not properly validated.
• Recommendation:
  • Use stricter TypeScript types or runtime type checks to ensure that the input conforms to the expected structure.

8. Backward Compatibility

• Observation: The PR introduces a new package and updates the SDK. While it is stated that this is a non-breaking change, there is no explicit mention of how existing users will be impacted.
• Recommendation:
  • Perform a thorough analysis of the impact on existing users.
  • Clearly communicate any changes in the release notes or documentation.

---

Summary

This PR introduces a robust and well-architected MCP governance package with a strong focus on security and extensibility. However, there are critical issues related to regex DoS, potential credential leakage, and replay protection that need to be addressed before merging. Additionally, there are some potential breaking changes and areas for improvement in testing, documentation, and type safety.

Action Items:

1. Address the critical issues related to regex DoS, credential leakage, and replay protection.
2. Add comprehensive test coverage for all new functionalities.
3. Improve documentation with advanced usage examples and best practices.
4. Ensure backward compatibility and communicate changes clearly.

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: Remove .github/ changes

"This PR modifies ci.yml and publish.yml (same issue as #775). Please remove the .github/ changes and submit the code portion only."

Fixed: All .github/ drift has been reverted. After merging latest main, the PR no longer modifies any workflow files. CI integration will be handled separately by maintainers.

✅ Bot Review Fixes (Proactive)

We proactively addressed the same patterns flagged on #774 and #775:

• Nonce cache LRU eviction — McpMessageSigner now evicts oldest entries when cache is full, with concurrent replay protection
• Thread safety — Session issuance and rate-limit updates are serialized; no unsynchronized mutable state
• PEM credential redaction — Full PEM blocks (BEGIN through END) are redacted, not just the header
• Regex timeout fail-closed — Regex scan time-budget triggers DENY on timeout, not silent pass-through
• Rate limiter eviction — Inactive rate-limit bucket eviction added for bounded memory
• Exception message sanitization — Security boundary errors return generic messages, no internal path/stack leaks
• Standalone package cleanup — Removed duplicate SDK-local MCP implementation; standalone package is the single source of truth

Validation

• npm test ✅ (all passing)
• npx tsc --noEmit ✅
• No .github/ changes in diff ✅

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review for feat(typescript): add MCP protocol governance with full OWASP coverage

Summary

This pull request introduces a new TypeScript SDK for the AgentMesh project, which includes a standalone @microsoft/agentmesh-mcp-governance package. The implementation focuses on providing comprehensive security governance for the Model Context Protocol (MCP) and addresses 11 out of 12 sections of the OWASP MCP Security Cheat Sheet. The PR includes new security primitives, enterprise-grade patterns, and a dual-package architecture.

---

Review Feedback

🔴 CRITICAL

1. Fail-Closed Mechanism in MCPGateway:

   • The evaluateToolCall method in MCPGateway implements a fail-closed mechanism by catching all errors and denying access. While this is a good practice, the catch block does not log the error details. This could make debugging and incident response more difficult.
     • Recommendation: Log the error details in the catch block to aid in debugging and auditing. For example:

       } catch (error) {
           console.error('MCPGateway evaluation error:', error);
           return this.finalize(
               agentId,
               toolName,
               params,
               {
                   allowed: false,
                   reason: 'Internal gateway error - access denied (fail closed)',
                   auditParams: this.redactor.redact(params).redacted as Record<string, unknown>,
                   findings: [],
               },
           );
       }

2. Credential Redaction:

   • The CredentialRedactor class uses a hardcoded list of patterns for sensitive data detection. While this is a good starting point, it may not cover all possible sensitive data patterns, especially for custom or less common use cases.
     • Recommendation: Provide a mechanism for users to dynamically update or extend the list of sensitive data patterns at runtime. This will make the redactor more adaptable to different environments.

3. Policy Evaluation Logic:

   • The policyEvaluator in MCPGateway is invoked but not validated for its return type or behavior. If the policyEvaluator returns unexpected results, it could lead to incorrect decisions.
     • Recommendation: Validate the output of policyEvaluator to ensure it adheres to the expected types and values (e.g., allow, deny, review). Log a warning or error if the return value is invalid.

4. Regex Injection Risk:

   • The toGlobalPattern function in CredentialRedactor dynamically constructs regular expressions. If user-provided patterns are passed to this function, it could lead to regex injection vulnerabilities.
     • Recommendation: Sanitize or validate user-provided patterns before constructing the regex. For example, ensure that the pattern does not contain malicious constructs like (?=...).

---

🟡 WARNING

1. Breaking Changes in Public API:

   • The index.ts file introduces several new exports, including CredentialRedactor, MCPResponseScanner, MCPGateway, and others. While these additions are non-breaking, they significantly expand the public API surface.
     • Recommendation: Clearly document these new exports and their intended use cases in the release notes to ensure users are aware of the changes.

2. TypeScript Version Requirement:

   • The package.json specifies TypeScript version 5.7.3. This is a relatively new version and may not be compatible with all existing projects.
     • Recommendation: Consider testing the SDK with older versions of TypeScript (e.g., 4.x) to ensure broader compatibility or explicitly document the minimum required version.

---

💡 SUGGESTIONS

1. Audit Logging Enhancements:

   • The MCPGateway class maintains an in-memory audit log (auditEntries). While this is useful for debugging, it may lead to memory bloat in long-running processes.
     • Suggestion: Provide a mechanism to periodically flush the in-memory audit log to a persistent storage or limit its size.

2. Test Coverage:

   • While the PR mentions that tests have been added, it is not clear if all edge cases, especially for security-critical components like CredentialRedactor and MCPGateway, have been covered.
     • Suggestion: Ensure that test cases cover the following scenarios:
       • All sensitive data patterns in CredentialRedactor.
       • Fail-closed behavior in MCPGateway.
       • Edge cases for policyEvaluator and approvalHandler.
       • Rate-limiting behavior under high load.

3. Documentation:

   • The PR mentions that documentation has been updated, but it is unclear if the new features are comprehensively documented.
     • Suggestion: Ensure that the documentation includes:
       • Detailed examples of how to use the new MCP governance features.
       • Guidance on configuring and extending the CredentialRedactor.
       • Explanation of the fail-closed mechanism and its implications.

4. Thread Safety:

   • The MCPGateway class uses an in-memory audit log and rate limiter. In a multi-threaded or distributed environment, this could lead to race conditions or inconsistent state.
     • Suggestion: Consider using thread-safe data structures or external storage solutions (e.g., Redis) for shared state. Alternatively, document the limitations of using the in-memory implementations in multi-threaded environments.

5. OWASP MCP Security Cheat Sheet Coverage:

   • The PR claims coverage for 11 out of 12 sections of the OWASP MCP Security Cheat Sheet. However, it does not specify which section is not covered.
     • Suggestion: Clearly document the missing section and provide a rationale for its exclusion. If possible, outline a roadmap for addressing it in the future.

---

Conclusion

The PR introduces significant security enhancements and aligns well with OWASP MCP guidelines. However, there are critical areas that need attention, particularly around error handling, regex injection risks, and thread safety. Addressing these issues will improve the robustness and security of the implementation. Additionally, clear documentation and comprehensive test coverage will ensure a smooth adoption of these new features.

[jackbatzner]

Resolved. Replaced the shell-expansion regex with indexOf-based string detection. See 006a9fa and afaa7a9.

[jackbatzner]

Resolved. Replaced HTML/markdown comment detection regex with delimiter-based string scanning. See 006a9fa and afaa7a9.

[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	#791 (this PR)	Core MCP primitives + tests	✅ Ready for review
2	#827	Standalone @microsoft/agentmesh-mcp-governance package	Open
3	#825	Documentation and examples	Open

Merge order: #791 → #827 → #825

All previous review feedback has been addressed in this updated branch.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: feat(typescript): add MCP protocol governance with full OWASP coverage

---

🔴 CRITICAL: Security Issues

1. Replay Protection in McpMessageSigner

   • The PR mentions "bounded nonce replay cache and concurrent replay protection," but the implementation of McpMessageSigner is not visible in this diff. Ensure that:
     • Nonces are cryptographically secure and unique.
     • Nonce expiration and storage are implemented securely to prevent replay attacks.
     • The nonce store is thread-safe and resistant to race conditions.
     • If InMemoryMCPNonceStore is used, it must be clearly documented that it is not suitable for distributed systems or production environments.

2. Credential Redaction (CredentialRedactor)

   • The CredentialRedactor uses regular expressions to identify sensitive data. However:
     • 🔴 Regex DoS Risk: Some of the regex patterns (e.g., /(password|passwd|pwd|secret|token|api[_-]?key|connection.?string|accountkey|sharedaccesssignature|sas)/i) could be vulnerable to Regular Expression Denial of Service (ReDoS) attacks. Consider using more efficient regex patterns or limiting the input size before applying regex.
     • 🔴 Incomplete Coverage: The BUILTIN_PATTERNS list might not cover all sensitive data patterns. For example, it lacks patterns for private keys (e.g., RSA private keys) or other sensitive data formats. Consider adding more comprehensive patterns or allowing users to configure additional patterns dynamically.

3. Policy Evaluation in MCPGateway

   • The policyEvaluator is invoked in the evaluate method, but its implementation is not visible in this PR. Ensure that:
     • The policy evaluation logic is deterministic and free from side effects.
     • The policy evaluation does not introduce any time-of-check-to-time-of-use (TOCTOU) vulnerabilities.
     • The policyEvaluator is thread-safe and does not introduce race conditions when accessed concurrently.

4. Sanitization in MCPGateway

   • The checkSanitization method is not visible in this diff. Ensure that:
     • It uses robust and efficient methods for input sanitization.
     • It prevents injection attacks (e.g., SQL injection, command injection) by properly escaping or rejecting malicious input.
     • The blockedPatterns array and BUILTIN_DANGEROUS_PATTERNS are comprehensive and updated regularly to include new attack vectors.

5. Approval Handler in MCPGateway

   • The approvalHandler is invoked for sensitive tools or when the policy decision is "review." Ensure that:
     • The approval process is auditable and tamper-proof.
     • The approvalHandler implementation is secure and cannot be bypassed.
     • The approval status is validated and cannot be forged by malicious actors.

6. Audit Logging in MCPGateway

   • The auditParams are redacted using CredentialRedactor, but ensure that:
     • The redaction process is fail-safe and does not accidentally expose sensitive data.
     • The audit logs are immutable and stored securely to prevent tampering.
     • The auditSink implementation is thread-safe and resistant to race conditions.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes

   • The PR introduces several new exports in src/index.ts, including CredentialRedactor, MCPGateway, and related types. While these are new additions and do not directly break existing functionality, they expand the public API surface. Ensure that these APIs are well-documented and tested, as they will need to be maintained for backward compatibility in future releases.

2. Behavioral Changes in AgentIdentity

   • The suspend and revoke methods now ignore the reason parameter (_reason is unused). If any consumers rely on this parameter, this change could lead to unexpected behavior. Consider documenting this change or deprecating the parameter explicitly.

---

💡 Suggestions for Improvement

1. Thread Safety

   • Ensure that all shared resources (e.g., InMemoryMCPAuditSink, InMemoryMCPNonceStore, MCPSlidingRateLimiter) are thread-safe, especially in concurrent environments. Consider using locks or atomic operations where necessary.

2. Testing

   • Add comprehensive tests for the new functionality, including edge cases and error scenarios. Specifically:
     • Test the CredentialRedactor with various input sizes and edge cases to ensure it handles all scenarios correctly.
     • Test the MCPGateway with different configurations (e.g., with and without rateLimiter, approvalHandler, and policyEvaluator).
     • Test the McpMessageSigner for replay protection and nonce handling.

3. OWASP MCP Security Cheat Sheet

   • The PR claims compliance with 11 out of 12 sections of the OWASP MCP Security Cheat Sheet. Consider adding a checklist or documentation to explicitly map the implementation to the relevant sections of the cheat sheet.

4. Documentation

   • Provide detailed documentation for the new classes and methods, including usage examples, configuration options, and potential pitfalls. This will help users understand how to use the new features securely and effectively.

5. Regex Optimization

   • Optimize the regex patterns in CredentialRedactor to prevent potential performance issues. For example:
     • Replace /(password|passwd|pwd|secret|token|api[_-]?key|connection.?string|accountkey|sharedaccesssignature|sas)/i with more specific patterns.
     • Use non-capturing groups ((?:...)) where capturing is not needed.

6. Error Handling

   • In the evaluateToolCall method of MCPGateway, the catch block returns a generic error message. Consider logging the actual error (without exposing sensitive information) to aid in debugging.

7. Rate Limiting

   • If MCPSlidingRateLimiter is used as the default rate limiter, ensure that it is well-documented and tested for edge cases, such as high concurrency or distributed deployments.

8. TypeScript Config

   • Ensure that the TypeScript configuration (tsconfig.json) is set to strict mode ("strict": true) to enforce type safety across the codebase.

---

Summary

This PR introduces significant new functionality to the TypeScript Agent Mesh SDK, including MCP protocol governance and OWASP compliance. While the changes are well-structured and align with the project's goals, there are critical security concerns (e.g., replay protection, regex DoS, and audit log integrity) that need to be addressed before merging. Additionally, there are potential breaking changes in the public API and behavioral changes in existing methods that should be documented and communicated to users. Finally, additional testing, documentation, and optimizations are recommended to ensure the robustness and maintainability of the new features.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: feat(typescript): add MCP protocol governance with full OWASP coverage

Summary

This PR introduces MCP (Model Context Protocol) security primitives to the TypeScript Agent Mesh SDK, aiming for comprehensive OWASP MCP Security Cheat Sheet compliance. The changes include new classes and utilities for HMAC signing, session management, input sanitization, credential redaction, and more. The implementation appears to be well-structured and adheres to modern TypeScript practices.

---

🔴 CRITICAL: Security Issues

1. Replay Protection in McpMessageSigner

• Issue: The PR mentions "bounded nonce replay cache and concurrent replay protection" in McpMessageSigner and McpMessageValidator. However, the implementation of SignerNonceStore is not included in this PR. Without reviewing the implementation, it is unclear if the nonce replay protection is robust and resistant to attacks.
• Impact: If the nonce replay protection is improperly implemented, it could allow attackers to replay signed messages, leading to potential security breaches.
• Action: Ensure that SignerNonceStore is implemented securely, with proper eviction policies and resistance to timing attacks. Add unit tests to validate nonce replay protection.

2. Credential Redaction Patterns

• Issue: The CredentialRedactor includes built-in patterns for sensitive data detection (e.g., OpenAI keys, GitHub tokens, AWS keys). However, these patterns may not cover all possible sensitive data formats, and the regex patterns may not be exhaustive or robust against edge cases.
• Impact: Failure to redact sensitive data could lead to accidental leakage of credentials in logs, violating OWASP A3: Sensitive Data Exposure.
• Action:
  • Perform a thorough review of the regex patterns to ensure they cover all common sensitive data formats.
  • Consider adding a mechanism for users to test their custom patterns against sample data.
  • Add tests for edge cases to ensure the patterns work as expected.

3. Policy Evaluation Fail-Closed Behavior

• Issue: The MCPGateway class has a try-catch block in the evaluateToolCall method that defaults to denying access on any error. While this is a good practice, the catch block swallows the error without logging it.
• Impact: Swallowing errors without logging them could make debugging and incident response more difficult.
• Action: Log the error details in the catch block before returning the fail-closed decision.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes

• Issue: The PR introduces new exports in src/index.ts, including CredentialRedactor, MCPResponseScanner, MCPGateway, and others. While these are new features, they expand the public API of the package.
• Impact: Any changes to these new APIs in the future could result in breaking changes for consumers.
• Action: Clearly document these new APIs and mark them as experimental if they are subject to change.

2. Behavioral Changes in AgentIdentity

• Issue: The suspend and revoke methods in AgentIdentity now ignore the reason parameter by prefixing it with an underscore (_reason). This is a breaking change for any consumers relying on the reason parameter.
• Impact: Existing consumers may experience runtime errors or unexpected behavior if they rely on the reason parameter.
• Action: Deprecate the reason parameter instead of removing it outright. Log a warning if the parameter is provided but not used, and document the deprecation in the changelog.

---

💡 Suggestions for Improvement

1. Thread Safety

• Observation: The CredentialRedactor and MCPGateway classes use shared state (e.g., this.auditEntries in MCPGateway and this.patterns in CredentialRedactor). While the PR mentions thread safety, there is no explicit use of synchronization mechanisms to prevent race conditions.
• Suggestion: Use thread-safe data structures or synchronization primitives (e.g., Mutex or Semaphore) to ensure thread safety in concurrent environments. Add tests to simulate concurrent access and verify correctness.

2. Input Sanitization

• Observation: The McpInputSanitizer uses regex-based input validation with a time-budget fail-closed behavior. However, the implementation of this class is not included in the PR.
• Suggestion: Ensure that the regex patterns are designed to prevent ReDoS (Regular Expression Denial of Service) attacks. Add tests to validate the time-budget behavior under various input scenarios.

3. Rate Limiting

• Observation: The MCPSlidingRateLimiter is used for rate-limiting agent requests. However, the implementation of MCPSlidingRateLimiter is not included in this PR.
• Suggestion: Review the implementation of MCPSlidingRateLimiter to ensure it is resistant to timing attacks and other potential vulnerabilities. Add tests to validate its behavior under high concurrency.

4. Error Handling

• Observation: The MCPGateway class has several points where errors could occur (e.g., policy evaluation, rate limiting, approval handling). While the evaluateToolCall method has a fail-closed mechanism, other methods do not.
• Suggestion: Add error handling and logging to all methods in MCPGateway to ensure that unexpected errors do not cause undefined behavior.

5. Backward Compatibility

• Observation: The PR introduces new features and modifies existing methods. While the changes are mostly additive, there are some potential breaking changes (e.g., suspend and revoke methods in AgentIdentity).
• Suggestion: Clearly document all changes in the changelog and consider adding deprecation warnings for modified methods.

---

Tests

Coverage

• The PR includes tests for the new features, but it is unclear if all edge cases are covered.
• Action: Add tests for:
  • Edge cases in regex patterns for CredentialRedactor.
  • Concurrent access to shared state in MCPGateway and CredentialRedactor.
  • Time-budget behavior in McpInputSanitizer.
  • Rate-limiting behavior under high concurrency.

Test Path Ignoring

• The jest.config.js file now includes testPathIgnorePatterns: ['/node_modules/', '/dist/'].
• Action: Ensure that this change does not inadvertently exclude any relevant test files.

---

Documentation

API Documentation

• The new classes and methods introduced in this PR need to be documented thoroughly, especially since they are security-related.
• Action: Add detailed JSDoc comments for all new classes, methods, and types. Include examples of usage where applicable.

OWASP Compliance

• The PR claims to cover 11 out of 12 sections of the OWASP MCP Security Cheat Sheet.
• Action: Provide a detailed mapping of the implemented features to the OWASP MCP Security Cheat Sheet sections in the documentation or PR description.

---

Final Notes

This PR introduces significant security-related functionality to the TypeScript Agent Mesh SDK. While the implementation appears well-structured, there are critical areas that require further review and testing to ensure robustness and security. Addressing the issues and suggestions outlined above will help ensure the reliability and security of the new features.

Priority Actions

1. Address the nonce replay protection issue in McpMessageSigner.
2. Review and enhance the regex patterns in CredentialRedactor.
3. Add error logging in the evaluateToolCall method of MCPGateway.

Once these issues are resolved, the PR will be in a much stronger position for merging.

[jackbatzner]

Addressing Review Feedback — Updated

@imran-siddique — thanks for the review! Here's the current state:

✅ Review 1: Remove .github/ changes

"This PR modifies ci.yml and publish.yml — please remove"

Fixed — All .github/ files removed. PR contains only TypeScript MCP source code and tests.

Additional security hardening (from internal review):

• PEM block redaction added to SDK CredentialRedactor (was only in standalone package)
• HMAC key minimum 32 bytes enforced in McpMessageSigner and McpSessionAuthenticator constructors
• Fail-closed try/catch added to scanTool(), verify(), and rate limiter consume()
• Regex scan budget (100ms timeout) ported from standalone to SDK — prevents ReDoS on crafted inputs
• CodeQL ReDoS fixes — hardened comment/shell-expansion regex patterns across all files

[jackbatzner]

Addressing AI Reviewer Findings

"Replay Protection not visible in diff" — The McpMessageSigner nonce replay implementation is in the diff — bounded nonce cache with LRU eviction, constant-time HMAC comparison, and fail-closed verification. Tests cover concurrent replay, cache overflow, and expired nonce scenarios.

CodeQL ReDoS — Working on a second round of regex hardening to replace remaining polynomial patterns with non-backtracking alternatives. Fix incoming.

"Credential Redaction Incompleteness" — The CredentialRedactor supports custom patterns via constructor. Built-in patterns cover PEM blocks (all key types), API keys, Bearer tokens, and connection strings.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: feat(typescript): add MCP protocol governance with full OWASP coverage

---

🔴 CRITICAL: Security Issues

1. Replay Protection in McpMessageSigner

   • The implementation mentions "bounded nonce replay cache and concurrent replay protection," but the PR does not include the actual code for McpMessageSigner. Ensure that the nonce replay cache is cryptographically secure and resistant to timing attacks. If the nonce cache is implemented in memory, it may be vulnerable to overflow or eviction attacks.
   • Action: Verify the nonce replay cache implementation and ensure it uses a cryptographically secure mechanism with proper eviction policies.

2. Regex-based Input Validation (McpInputSanitizer)

   • The use of regex for input validation (CredentialRedactor, MCPGateway) is prone to catastrophic backtracking, especially for complex patterns like PEM blocks. This can lead to Denial of Service (DoS) attacks if an attacker crafts malicious input.
   • Action: Use a library like re2 for regex processing to prevent catastrophic backtracking.

3. Credential Redaction (CredentialRedactor)

   • The redaction mechanism relies on regex patterns to identify sensitive data. While this is useful, it may miss edge cases or custom formats. For example, proprietary API keys or tokens not matching the predefined patterns will not be redacted.
   • Action: Provide a mechanism for users to dynamically add patterns at runtime or fallback to a heuristic-based approach for unknown sensitive data.

4. Rate Limiting (MCPSlidingRateLimiter)

   • The rate limiter implementation is not included in this PR, but its usage in MCPGateway suggests it operates on an agent-level basis. Ensure that the rate limiter is resistant to bypass techniques, such as distributed attacks using multiple agent IDs.
   • Action: Confirm that the rate limiter has protections against distributed attacks and supports IP-based throttling as a fallback.

5. Sandbox Escape (MCPGateway)

   • The blockedPatterns in MCPGateway include dangerous shell commands (rm, del, mkfs) and command injection patterns ($(), backticks). While this is a good start, it is insufficient for comprehensive sandbox escape prevention.
   • Action: Implement a more robust sandboxing mechanism, such as restricting execution environments or using syscall filtering (e.g., seccomp).

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes (index.ts)

   • The addition of multiple new exports (CredentialRedactor, MCPGateway, MCPSessionAuthenticator, etc.) significantly expands the public API surface. This may lead to compatibility issues for consumers of the SDK who rely on specific imports.
   • Action: Clearly document these changes and provide migration guides for existing users.

2. Behavioral Changes in AgentIdentity

   • The _reason parameter in suspend and revoke methods is now ignored. This could break existing integrations that rely on the reason parameter for audit logging or other purposes.
   • Action: Deprecate the reason parameter instead of removing its functionality outright. Log a warning when it is used.

---

💡 Suggestions for Improvement

1. Thread Safety

   • The PR introduces several classes (CredentialRedactor, MCPGateway, InMemoryMCPAuditSink) that manage shared state (e.g., audit entries, redactions). Ensure these classes are thread-safe, especially in concurrent agent execution scenarios.
   • Action: Use locks or atomic operations where shared state is modified.

2. OWASP MCP Compliance

   • While the PR claims full OWASP MCP Security Cheat Sheet coverage (11/12 sections), the absence of §11 (Consent UI) should be explicitly documented in the SDK's README to avoid confusion.
   • Action: Add a section in the documentation clarifying the scope of OWASP compliance.

3. Type Safety

   • The CredentialRedactor and MCPGateway classes rely heavily on Record<string, unknown> for parameters. This weakens type safety and increases the risk of runtime errors.
   • Action: Use stricter types or Pydantic-style validation for input parameters.

4. Audit Logging

   • The audit logging mechanism in MCPGateway (auditSink) is implemented as an in-memory store. This is not suitable for production environments where persistence and scalability are required.
   • Action: Provide an interface for pluggable audit sinks (e.g., database, cloud storage).

5. Backward Compatibility

   • The removal of LegacyPolicyDecision from audit.ts may break integrations relying on this type. Consider marking it as deprecated instead of removing it outright.
   • Action: Add a deprecation warning for LegacyPolicyDecision.

---

Summary

This PR introduces critical features for MCP protocol governance but has several security and compatibility concerns. Addressing the replay protection, regex vulnerabilities, and sandbox escape vectors should be prioritized. Additionally, ensure backward compatibility and thread safety across the new classes.

---

Recommended Actions

1. 🔴 Address replay protection and regex vulnerabilities.
2. 🟡 Document public API changes and provide migration guides.
3. 💡 Improve thread safety, type validation, and audit logging mechanisms.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: feat(typescript): add MCP protocol governance with full OWASP coverage

---

🔴 CRITICAL: Security Issues

1. Replay Protection in McpMessageSigner and McpMessageValidator:

   • While the PR mentions bounded nonce replay cache and concurrent replay protection, there is no explicit code demonstrating how the nonce cache is implemented or how it handles edge cases like cache eviction or collisions. Without proper implementation, this could lead to replay attacks.
   • Action: Ensure the nonce cache is implemented securely with eviction policies and collision handling. Add tests to validate the behavior under high concurrency and edge cases.

2. Regex Injection in CredentialRedactor and MCPGateway:

   • The blockedPatterns and customPatterns accept user-provided regular expressions. This can lead to potential regex denial-of-service (ReDoS) attacks if the regex patterns are crafted maliciously.
   • Action: Validate and sanitize user-provided regex patterns to prevent ReDoS vulnerabilities. Consider limiting regex complexity (e.g., maximum length, nesting depth).

3. Fail-Closed Behavior in MCPGateway:

   • The evaluateToolCall method defaults to fail-closed on internal errors, which is good. However, the error handling does not log or propagate detailed error information, which could hinder debugging and auditing.
   • Action: Enhance error logging to include detailed stack traces and context for debugging while ensuring sensitive information is redacted.

4. Sensitive Key Redaction in CredentialRedactor:

   • The SENSITIVE_KEY_PATTERN regex may miss certain sensitive keys due to its limited scope. For example, it does not account for variations like "auth_token" or "access_secret".
   • Action: Expand the regex to cover more variations of sensitive keys. Consider allowing configurable patterns for specific use cases.

5. Approval Handler in MCPGateway:

   • The approval handler is optional, and the default behavior is to deny access if no handler is provided. This is a good security practice. However, there is no mechanism to ensure that the approval handler itself is secure or that it cannot be bypassed.
   • Action: Add validation for the approval handler to ensure it meets security requirements. Document best practices for implementing secure approval handlers.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Expansion:

   • The addition of multiple new exports (CredentialRedactor, MCPGateway, MCPSessionAuthenticator, etc.) significantly expands the public API surface. This could lead to backward compatibility issues if these APIs are modified in the future.
   • Action: Clearly document these new APIs and mark them as stable or experimental. Add versioning policies for future changes.

2. Behavior Change in AgentIdentity.suspend and AgentIdentity.revoke:

   • The _reason parameter is now ignored, which changes the behavior of these methods. This could break existing code that relies on the reason parameter.
   • Action: Deprecate the reason parameter with a warning message instead of silently ignoring it.

---

💡 Suggestions for Improvement

1. Thread Safety in CredentialRedactor and MCPGateway:

   • The CredentialRedactor uses a WeakMap for tracking seen objects, which is not inherently thread-safe. Similarly, MCPGateway does not explicitly address thread safety in concurrent evaluations.
   • Action: Add thread safety mechanisms (e.g., locks or atomic operations) for shared resources. Document concurrency guarantees.

2. OWASP MCP Security Cheat Sheet Coverage:

   • While the PR claims coverage of 11/12 sections, it would be beneficial to explicitly list the covered sections and provide tests or documentation demonstrating compliance.
   • Action: Add a compliance matrix to the documentation, mapping implemented features to OWASP sections.

3. Rate Limiter Configuration:

   • The rate limiter in MCPGateway is configurable, but there is no validation for extreme values (e.g., very high or very low limits).
   • Action: Add validation for rate limiter configurations to prevent misconfigurations.

4. Credential Redaction Preview:

   • The truncatePreview function is used to limit the size of redacted text previews. Consider making the truncation length configurable for better flexibility.
   • Action: Add a configuration option for truncation length.

5. Test Coverage:

   • While the PR mentions tests, there is no evidence of comprehensive test coverage for edge cases like high concurrency, large payloads, or malicious inputs.
   • Action: Add tests for edge cases and stress scenarios. Include tests for fail-closed behavior under various error conditions.

---

Summary

This PR introduces significant security features and OWASP compliance improvements to the TypeScript SDK. However, there are critical security issues related to replay protection, regex injection, and sensitive key redaction that need immediate attention. Additionally, potential breaking changes and thread safety concerns should be addressed to ensure robustness and backward compatibility.

Recommended Actions:

• Address 🔴 CRITICAL security issues before merging.
• Resolve 🟡 WARNING breaking changes or document them clearly.
• Implement 💡 Suggestions for improved security, usability, and maintainability.

[jackbatzner]

Resolved. Replaced the regex-based nested-quantifier detector with a pure stack-based character scanner — zero regex literals remain in this file. See afaa7a9.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: feat(typescript): add MCP protocol governance with full OWASP coverage

---

🔴 CRITICAL: Security Issues

1. Replay Protection in McpMessageSigner:

   • The nonce replay cache implementation is not included in this PR. Without a robust mechanism to prevent replay attacks, the HMAC-SHA256 signing can be bypassed by reusing valid signatures.
   • Action: Ensure that the SignerNonceStore implementation is fail-closed and includes bounded storage with eviction policies. Add tests to validate replay protection under concurrent conditions.

2. Regex Injection in CredentialRedactor:

   • The toGlobalPattern function accepts arbitrary regex patterns. If these patterns are user-defined, they could lead to catastrophic backtracking or denial-of-service attacks.
   • Action: Validate regex complexity using a library like safe-regex or implement a timeout mechanism for regex execution.

3. Sanitization in MCPGateway:

   • The checkSanitization method is not fully implemented, leaving potential sandbox escape vectors unaddressed. Dangerous patterns like rm, del, and mkfs are flagged but not comprehensively handled.
   • Action: Expand sanitization checks to cover OWASP Injection flaws comprehensively. Add fail-closed behavior for unrecognized patterns.

4. Approval Handler Trust Boundary:

   • The approvalHandler mechanism relies on external human reviewers but does not enforce cryptographic integrity for the approval decision. This could lead to tampering.
   • Action: Implement cryptographic signing for approval decisions to ensure integrity and authenticity.

---

🟡 WARNING: Potential Breaking Changes

1. Export Changes in index.ts:

   • The addition of new exports (CredentialRedactor, MCPResponseScanner, etc.) changes the public API of the TypeScript SDK. This could break existing consumers of the library if they rely on the previous export structure.
   • Action: Document these changes clearly in the release notes and consider versioning the package appropriately.

2. Behavioral Changes in AgentIdentity:

   • The _reason parameter in suspend and revoke methods is now unused. This could lead to confusion for developers expecting the parameter to be functional.
   • Action: Either remove the parameter entirely or implement its functionality. Update the documentation to reflect the intended behavior.

---

💡 Suggestions for Improvement

1. Thread Safety in CredentialRedactor:

   • The WeakMap used for circular reference detection in redactNode is not inherently thread-safe. While JavaScript is single-threaded, consider documenting this limitation for environments like Node.js where concurrency might be simulated.

2. Rate Limiting in MCPGateway:

   • The rateLimiter implementation is optional, but its absence could lead to abuse. Consider making rate limiting mandatory for sensitive tools.

3. Backward Compatibility:

   • The removal of LegacyPolicyDecision from audit.ts could break compatibility for consumers relying on this type.
   • Action: Deprecate the type first and provide migration guidance.

4. Testing Coverage:

   • Add tests for edge cases in CredentialRedactor, such as deeply nested objects, circular references, and large payloads.

5. OWASP MCP Security Cheat Sheet Compliance:

   • While 11/12 sections are covered, the absence of §11 Consent UI should be explicitly documented as out of scope for server-side SDKs.

---

Summary

This PR introduces critical security primitives for MCP governance but has several gaps in replay protection, regex validation, and sandbox sanitization. Addressing these issues is essential to ensure the robustness and security of the implementation. Additionally, the changes to the public API and behavioral modifications require careful documentation to avoid breaking existing consumers.

Recommended Actions:

• Address the 🔴 CRITICAL issues immediately.
• Document and version changes flagged as 🟡 WARNING.
• Implement the 💡 SUGGESTIONS to improve overall quality and compliance.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: feat(typescript): add MCP protocol governance with full OWASP coverage

---

🔴 CRITICAL: Security Issues

1. Replay Protection in McpMessageSigner and McpMessageValidator:

   • The implementation mentions "bounded nonce replay cache and concurrent replay protection," but the code for nonce management and replay protection is not included in the diff. Ensure that:
     • Nonces are unique and securely stored.
     • Replay protection is enforced across distributed systems (e.g., using a shared nonce store or database).
     • Nonce expiration is handled to prevent memory exhaustion.
   • Missing or flawed nonce handling could allow replay attacks, bypassing security measures.

   Action: Verify nonce storage implementation and add tests for replay protection.

2. Regex Injection in CredentialRedactor:

   • The toGlobalPattern function dynamically compiles regex patterns. If user-provided patterns are passed without validation, this could lead to regex injection vulnerabilities.
   • Although validateRegex is called, ensure it includes safeguards against catastrophic backtracking and denial-of-service attacks.

   Action: Harden validateRegex to reject unsafe patterns and add tests for edge cases.

3. Blocked Patterns in MCPGateway:

   • The blockedPatterns array allows custom regex patterns. If these are user-defined, they could introduce vulnerabilities (e.g., bypassing sanitization or causing denial-of-service).
   • Ensure that all patterns are validated using validateRegex.

   Action: Add stricter validation for blockedPatterns and document safe usage.

4. Fail-Closed Behavior in evaluateToolCall:

   • The evaluateToolCall method defaults to fail-closed behavior on errors, which is good. However, the error message (Internal gateway error - access denied) could leak implementation details if logged or exposed externally.

   Action: Ensure error messages are generic and do not reveal sensitive information.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes in index.ts:

   • The addition of multiple new exports (e.g., CredentialRedactor, MCPGateway, MCPSessionAuthenticator) significantly expands the API surface. This could lead to compatibility issues for downstream consumers relying on the previous API structure.

   Action: Document all new exports and provide migration guidance for existing users.

2. Behavior Change in AgentIdentity.suspend and AgentIdentity.revoke:

   • The _reason parameter is now prefixed with an underscore, indicating it is unused. This change could affect consumers relying on the parameter for logging or audit purposes.

   Action: Deprecate the parameter explicitly and provide alternatives for users who rely on it.

---

💡 Suggestions for Improvement

1. Thread Safety in CredentialRedactor:

   • The redactNode method uses a WeakMap for cycle detection. While this is a good approach, consider documenting its thread safety and ensuring it is not shared across threads.

   Action: Add comments or documentation clarifying thread safety guarantees.

2. OWASP MCP Security Cheat Sheet Compliance:

   • The PR mentions compliance with 11/12 sections of the OWASP MCP Security Cheat Sheet. Consider adding a checklist or documentation showing how each section is addressed.

   Action: Include a compliance matrix in the documentation.

3. Rate Limiting in MCPGateway:

   • The rate limiter (MCPSlidingRateLimiter) is configurable, but its behavior under high concurrency is unclear. Ensure that:
     • Rate limits are enforced consistently across distributed systems.
     • Retry logic is robust and does not introduce race conditions.

   Action: Add stress tests for rate limiting under high concurrency.

4. Credential Redaction Logging:

   • The CredentialRedactor class provides redaction for sensitive data. Consider adding an option to log redacted entries for audit purposes, ensuring logs remain safe for external review.

   Action: Add a configurable logging mechanism for redacted credentials.

5. Test Coverage:

   • While the PR mentions tests, ensure that edge cases (e.g., invalid regex patterns, high-concurrency rate limiting) are covered.

   Action: Expand test coverage for edge cases and document test results.

---

Summary

This PR introduces critical security features for MCP governance, aligning with OWASP guidelines. However, several areas require attention to ensure robustness and backward compatibility:

• 🔴 Address replay protection, regex injection, and blocked pattern validation.
• 🟡 Document API changes and deprecate unused parameters.
• 💡 Improve thread safety, compliance documentation, and test coverage.

Once these issues are resolved, the PR will significantly enhance the security posture of the Agent Mesh SDK.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces MCP (Model Context Protocol) governance primitives to the TypeScript Agent Mesh SDK, aiming for comprehensive OWASP MCP Security Cheat Sheet coverage. While the implementation is robust and security-focused, several areas require attention to ensure compliance with security standards, thread safety, and backward compatibility.

---

🔴 CRITICAL: Security Issues

1. Replay Attack Mitigation in McpMessageSigner and McpMessageValidator:

   • The PR mentions "bounded nonce replay cache and concurrent replay protection," but the implementation details are missing. Ensure that:
     • Nonces are unique and cannot be reused.
     • Nonce expiration is properly handled.
     • Concurrency issues (e.g., race conditions) are addressed when checking and storing nonces.
   • Action: Provide detailed implementation for nonce replay protection and ensure robust testing for edge cases.

2. Regex Injection in CredentialRedactor and MCPGateway:

   • The validateRegex function is used to validate regex patterns, but its implementation is not included in the diff. If improperly implemented, this could lead to regex injection vulnerabilities.
   • Action: Ensure validateRegex properly sanitizes and limits regex patterns to prevent catastrophic backtracking or denial-of-service attacks.

3. Fail-Closed Behavior in MCPGateway:

   • The evaluateToolCall method uses a fail-closed approach when encountering errors. While this is good practice, ensure that all exceptions are logged securely and that sensitive data is not exposed in logs.
   • Action: Review logging mechanisms to ensure sensitive data is redacted and error messages do not leak information.

4. Sensitive Data Redaction in CredentialRedactor:

   • The CredentialRedactor class uses regex patterns to redact sensitive data. Ensure that:
     • The patterns cover all possible sensitive data formats (e.g., JWTs, OAuth tokens, API keys).
     • The redaction process is fail-safe and does not leave sensitive data unredacted in edge cases.
   • Action: Expand test coverage for edge cases, including malformed inputs and large payloads.

5. Policy Engine Evaluation:

   • The policyEvaluator in MCPGateway is optional, and its absence could lead to security bypasses. If no policy evaluator is provided, the SDK should fail closed.
   • Action: Add explicit checks to ensure that the absence of a policy evaluator results in a denied decision.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes:

   • The addition of new exports in index.ts (e.g., CredentialRedactor, MCPGateway, MCPMessageSigner) expands the public API surface. Ensure that these additions are well-documented and backward-compatible.
   • Action: Verify that existing functionality is unaffected and document the new API changes thoroughly.

2. Behavior Change in AgentIdentity.suspend and AgentIdentity.revoke:

   • The _reason parameter is now prefixed with an underscore, which may break existing code relying on the previous method signature.
   • Action: Add deprecation warnings for the old method signature and provide clear migration guidance.

---

💡 Suggestions for Improvement

1. Thread Safety in CredentialRedactor:

   • The redactNode method uses a WeakMap to track seen objects, which is thread-safe in single-threaded environments but may cause issues in concurrent execution.
   • Suggestion: Consider adding explicit thread-safety mechanisms or documenting that this class is not safe for concurrent use.

2. OWASP MCP Compliance:

   • The PR mentions 11/12 sections of OWASP MCP Security Cheat Sheet coverage. Document the missing section (§11 Consent UI) and clarify its exclusion to avoid confusion.
   • Suggestion: Add a roadmap for future compliance with §11 Consent UI.

3. Testing Coverage:

   • While the PR mentions tests, ensure that edge cases for all security-sensitive components (e.g., nonce replay, regex validation, redaction) are covered.
   • Suggestion: Add fuzz testing for regex patterns and nonce handling.

4. Logging Best Practices:

   • Ensure that all logs in MCPGateway and other components are sanitized to prevent sensitive data leakage.
   • Suggestion: Use structured logging and integrate with a secure logging framework.

5. Documentation:

   • The new features (e.g., MCPGateway, CredentialRedactor) should be accompanied by detailed documentation, including usage examples and security considerations.
   • Suggestion: Add inline comments and update the SDK documentation.

6. Backward Compatibility Testing:

   • Run regression tests to ensure that existing functionality is unaffected by the new additions.
   • Suggestion: Add a compatibility matrix to verify support for older versions of the SDK.

---

Conclusion

The PR introduces valuable security features and aligns with OWASP MCP guidelines. However, critical issues related to replay attack mitigation, regex injection, and policy engine correctness must be addressed before merging. Additionally, potential breaking changes and thread safety concerns should be resolved or documented. With these improvements, the PR will significantly enhance the security and functionality of the Agent Mesh SDK.

[jackbatzner]

CI Status Summary — All findings addressed ✅

All CI checks pass except:

• Maintainer approval gate — awaiting your review
• CodeQL status gate — stale fork-PR artifact; the CodeQL Analyze jobs pass, all 25 inline findings were resolved (regex replaced with indexOf/stack-based scanning, codeql-suppress on intentional test patterns)
• check-version-pinning — pre-existing issue tracked in fix(vscode): pin dependency versions in agent-os-vscode package.json #831

This is the Core PR in a 3-PR stack. Merge order: #791 (Core) → #827 (Package) → #825 (Docs).

[imran-siddique]

Approved and ready to merge, but this PR has merge conflicts with main (likely from #774 merging). Please rebase on latest main to resolve conflicts, then we can merge.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces significant security features to the TypeScript Agent Mesh SDK, particularly focusing on the Model Context Protocol (MCP). The implementation appears to be well-structured and adheres to many OWASP guidelines. However, there are several areas that require attention, particularly regarding security, potential breaking changes, and overall code quality.

Actionable Feedback

🔴 CRITICAL Issues

1. Replay Protection: While the McpMessageSigner includes a bounded nonce replay cache, ensure that the nonce generation is truly unique and unpredictable. If the nonce can be predicted or reused, it could lead to replay attacks. Consider using a cryptographically secure random number generator for nonce creation.

2. Input Sanitization: The McpInputSanitizer uses regex-based validation. Ensure that the regex patterns are thoroughly tested against potential injection attacks (e.g., SQL injection, XSS). It’s crucial to validate that the regex patterns do not allow for bypassing the intended sanitization.

3. Error Handling: In the evaluateToolCall method of MCPGateway, ensure that sensitive information is not leaked in error messages. The current implementation logs the error but does not specify how sensitive data is handled in the logs. Implement a mechanism to redact sensitive information from logs.

4. Thread Safety: The McpToolRegistry claims to have thread-safe operations. Ensure that all shared resources are properly synchronized to prevent race conditions, especially in concurrent environments. Consider using locks or other synchronization mechanisms where necessary.

🟡 WARNING Issues

1. Public API Changes: The addition of new classes and methods (e.g., CredentialRedactor, McpMessageSigner) may introduce breaking changes for users relying on previous versions of the SDK. Ensure that the versioning follows semantic versioning principles, and document any breaking changes clearly in the release notes.

2. Type Safety: The introduction of new types in index.ts and other files should be reviewed for consistency with existing types. Ensure that all new types are well-documented and that their usage does not introduce type safety issues.

💡 SUGGESTION Improvements

1. Documentation: Enhance the documentation for the new features, especially around the security primitives. Provide examples of how to use the McpMessageSigner, McpSessionManager, and CredentialRedactor. Clear documentation will help users understand the security implications and usage patterns.

2. Testing: Ensure that comprehensive unit tests cover all new classes and methods. Pay special attention to edge cases, especially in the redact methods and the MCPGateway evaluation logic. Consider adding integration tests to validate the interactions between components.

3. Configuration Validation: In the MCPGateway constructor, consider adding validation for the configuration parameters. For instance, ensure that allowedTools and deniedTools do not overlap, and that patterns in blockedPatterns are valid regex.

4. Performance Considerations: Review the performance implications of using regex for input sanitization and credential redaction. If performance is a concern, consider optimizing the regex patterns or using alternative methods for sanitization.

5. Code Consistency: Maintain consistent formatting and style throughout the codebase. For example, ensure that comments and documentation follow the same style guide. This will improve readability and maintainability.

Conclusion

Overall, the PR introduces valuable security features to the TypeScript Agent Mesh SDK. However, attention to the highlighted critical issues is essential to ensure the security and robustness of the implementation. Addressing the warnings and suggestions will further enhance the quality and usability of the code.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces MCP (Model Context Protocol) governance primitives to the TypeScript Agent Mesh SDK. The implementation appears robust and well-structured, addressing OWASP MCP Security Cheat Sheet coverage (11/12 sections). However, there are several areas that require attention to ensure correctness, security, and maintainability.

---

🔴 CRITICAL Issues

1. Replay Protection in McpMessageSigner

   • The bounded nonce replay cache implementation is not included in the diff. If the nonce cache is not thread-safe or does not enforce strict uniqueness, it could allow replay attacks, which are a critical security vulnerability.
   • Action: Ensure that the nonce cache is thread-safe and strictly enforces uniqueness for nonces. Consider using a cryptographically secure mechanism for nonce generation and validation.

2. Regex Injection in CredentialRedactor

   • The CredentialRedactor allows custom regex patterns via CredentialRedactorConfig. However, there is no validation of these patterns beyond checking if they are valid regex. This opens the door to potential regex denial-of-service (ReDoS) attacks.
   • Action: Implement a mechanism to limit the complexity of user-provided regex patterns. For example, use libraries like safe-regex to validate regex patterns before accepting them.

3. Sanitization Timeout in CredentialRedactor

   • The redactString method uses a time budget (scanTimeoutMs) for regex-based sanitization. If the timeout is exceeded, the method fails closed. However, the behavior in case of timeout is not defined in the code snippet provided.
   • Action: Ensure that a timeout does not lead to partial sanitization or incomplete redaction. Consider logging a critical error and rejecting the input entirely if the timeout is exceeded.

4. Policy Evaluation Fail-Closed Behavior

   • The evaluateToolCall method defaults to fail-closed behavior in case of an error. However, the error message is generic (Internal gateway error - access denied). This could lead to operational challenges in debugging.
   • Action: Log detailed error information securely (e.g., to a secure audit log) while maintaining fail-closed behavior for external responses.

5. Sensitive Key Redaction

   • The SENSITIVE_KEY_PATTERN in CredentialRedactor is case-insensitive but may not cover all sensitive keys. For example, keys like "authToken" or "sessionKey" are not included.
   • Action: Expand the SENSITIVE_KEY_PATTERN to include additional common sensitive key names. Consider making this pattern configurable.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes

   • The addition of new exports in index.ts (e.g., CredentialRedactor, MCPMessageSigner, MCPGateway) expands the public API. While this is expected for new features, ensure that these additions do not conflict with existing APIs or introduce ambiguity.
   • Action: Document these new APIs thoroughly and ensure backward compatibility with existing functionality.

2. Behavioral Changes in AgentIdentity.suspend and AgentIdentity.revoke

   • The _reason parameter is now ignored (_reason?: string). If external consumers relied on this parameter for logging or auditing, this change could break their implementations.
   • Action: Deprecate the parameter instead of ignoring it outright, and provide a migration path for consumers.

---

💡 Suggestions for Improvement

1. Thread Safety in CredentialRedactor

   • The redactNode method uses a WeakMap for tracking seen objects. While this prevents circular references, it does not guarantee thread safety in concurrent environments.
   • Suggestion: Add thread-safety mechanisms (e.g., locks or atomic operations) to ensure correctness in multi-threaded scenarios.

2. Audit Logging in MCPGateway

   • The auditSink implementation (InMemoryMCPAuditSink) stores audit entries in memory. This could lead to memory exhaustion in high-throughput environments.
   • Suggestion: Provide an option for persistent audit logging (e.g., to a database or file system) and implement log rotation mechanisms.

3. Regex Complexity Budget

   • The createRegexScanBudget function is used for sanitization but is not included in the diff. Ensure that it accounts for regex complexity to prevent ReDoS attacks.
   • Suggestion: Use libraries like safe-regex or implement a custom complexity checker.

4. Rate Limiter Configuration

   • The rateLimiter configuration in MCPGateway defaults to undefined if rateLimit is not provided. This could lead to accidental omission of rate limiting in production environments.
   • Suggestion: Log a warning if rate limiting is disabled, and consider enabling it by default with reasonable limits.

5. Testing Coverage

   • While the PR mentions that tests have been added, the diff does not include test files. Ensure that all critical paths (e.g., nonce replay protection, regex sanitization, policy evaluation) are thoroughly tested.
   • Suggestion: Add unit tests for edge cases, such as invalid regex patterns, replay attacks, and rate limit exhaustion.

6. Documentation

   • The PR mentions that documentation will be added in Part 3 (#825). However, the new APIs introduced in this PR should have at least basic documentation (e.g., JSDoc comments) to aid reviewers and future developers.
   • Suggestion: Add JSDoc comments for all new classes, methods, and types.

---

Final Recommendation

• Address the 🔴 CRITICAL issues before merging.
• Consider the 🟡 WARNING items and provide clear documentation for any breaking changes.
• Implement the 💡 SUGGESTIONS to improve security, maintainability, and developer experience.

Once the critical issues are resolved and the necessary tests are added, this PR can be approved for merging.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: feat(typescript): add MCP protocol governance with full OWASP coverage

---

🔴 CRITICAL Issues

1. Replay Protection in McpMessageSigner / McpMessageValidator:

   • The PR mentions "bounded nonce replay cache and concurrent replay protection," but the implementation details are not visible in the diff. Ensure that:
     • Nonces are cryptographically secure and unique per message.
     • The replay cache is robust against race conditions in concurrent environments.
     • Expired nonces are properly evicted to prevent memory exhaustion.
   • Action: Add tests for nonce replay protection under high concurrency and edge cases (e.g., nonce expiration).

2. Regex Injection in CredentialRedactor:

   • The CredentialRedactor accepts custom regex patterns via CredentialRedactorConfig. If these patterns are user-supplied, they could lead to denial-of-service attacks (e.g., catastrophic backtracking).
   • Action: Validate all regex patterns using a strict timeout mechanism or a regex complexity analyzer. Consider rejecting overly complex patterns.

3. Sandbox Escape in MCPGateway:

   • The blockedPatterns feature in MCPGateway includes dangerous patterns like rm, del, and mkfs. However, the sanitization logic does not explicitly prevent command injection or sandbox escapes.
   • Action: Add explicit checks for command injection (e.g., ;, &&, |, >). Ensure that all user-supplied inputs are sanitized before being passed to external tools or APIs.

4. Approval Handler Trust Boundary:

   • The approvalHandler in MCPGateway delegates sensitive decisions to human reviewers. If the handler is compromised, it could allow malicious approvals.
   • Action: Implement audit logging for all approval decisions, including reviewer identity and timestamps. Consider adding cryptographic signing for audit logs.

---

🟡 WARNING: Potential Breaking Changes

1. Export Changes in index.ts:

   • The PR introduces new exports (e.g., CredentialRedactor, MCPGateway, MCPMessageSigner) and modifies existing ones. This could break consumers relying on the previous export structure.
   • Action: Document all new exports and provide migration guidance for existing users.

2. Behavioral Changes in AgentIdentity:

   • The suspend and revoke methods now ignore the reason parameter. This could affect downstream consumers relying on the reason for logging or auditing.
   • Action: Deprecate the reason parameter with a clear warning in the documentation.

---

💡 Suggestions for Improvement

1. Thread Safety in CredentialRedactor:

   • The redactNode method uses a WeakMap for tracking seen objects. While this prevents infinite recursion, it may not be thread-safe in concurrent environments.
   • Suggestion: Add explicit thread safety mechanisms or document that the class is not thread-safe.

2. OWASP MCP Security Cheat Sheet Compliance:

   • While the PR claims coverage of 11/12 sections, it does not address §11 (Consent UI). Consider adding a placeholder or documentation for future implementation.

3. Testing Coverage:

   • Ensure comprehensive tests for:
     • Rate limiting in MCPSlidingRateLimiter.
     • Credential redaction for edge cases (e.g., nested objects, circular references).
     • Gateway decision logic under various configurations (e.g., conflicting allow/deny lists).

4. Performance Optimization:

   • The CredentialRedactor uses matchAll and replace iteratively, which may degrade performance for large inputs. Consider optimizing the regex matching process.

5. Documentation:

   • The new classes (MCPGateway, CredentialRedactor, etc.) lack detailed documentation. Add usage examples and configuration guidelines.

---

Summary

This PR introduces critical security features for the MCP protocol governance in the TypeScript SDK. While the implementation is robust and addresses many OWASP concerns, there are critical issues related to replay protection, regex injection, sandbox escape, and trust boundary handling that must be addressed before merging. Additionally, potential breaking changes should be documented, and further improvements can be made to enhance thread safety, performance, and testing coverage.

Recommended Actions:

• Address the 🔴 CRITICAL issues before merging.
• Document 🟡 WARNING changes to ensure backward compatibility.
• Consider implementing 💡 SUGGESTIONS for better performance, thread safety, and developer experience.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: feat(typescript): add MCP protocol governance with full OWASP coverage

Summary

This PR introduces MCP (Model Context Protocol) security primitives to the TypeScript Agent Mesh SDK, aiming for compliance with 11 out of 12 sections of the OWASP MCP Security Cheat Sheet. The implementation includes several security-focused features such as HMAC-SHA256 signing, nonce replay protection, session management, input sanitization, and credential redaction.

---

🔴 CRITICAL Issues

1. Replay Cache Size and Eviction Policy:

   • The McpMessageSigner and McpMessageValidator use a bounded nonce replay cache for replay protection. However, the PR does not specify the eviction policy or the size of the cache. If the cache size is too small or eviction is not handled properly, this could lead to a security vulnerability where older nonces are reused.
   • Action: Clearly define the eviction policy and ensure that the cache size is configurable. Add tests to verify the behavior when the cache is full.

2. Regex Injection in CredentialRedactor:

   • The CredentialRedactor allows custom regex patterns to be added via CredentialRedactorConfig. While the validateRegex function is used, the implementation does not specify what validation is performed. If the validation is insufficient, this could lead to regex injection vulnerabilities.
   • Action: Ensure validateRegex properly checks for malicious or overly complex regex patterns that could lead to denial-of-service attacks. Add tests to verify the robustness of the validation logic.

3. Sanitization Timeout:

   • The MCPGateway uses a scanTimeoutMs parameter for sanitization, but there is no fallback mechanism if the timeout is exceeded. This could lead to fail-open behavior, allowing malicious input to bypass sanitization.
   • Action: Implement a fail-closed mechanism for cases where the sanitization process exceeds the timeout. Add tests to verify this behavior.

4. Approval Handler Default Behavior:

   • The MCPGateway relies on an approvalHandler for sensitive tools or policy decisions requiring human review. If the handler is not provided, the default behavior is to set approvalStatus to Pending. This could lead to operational delays or fail-open scenarios.
   • Action: Define a clear fallback mechanism for cases where the approvalHandler is not provided or fails to respond. Consider defaulting to ApprovalStatus.Denied for fail-closed behavior.

---

🟡 WARNING Issues

1. Breaking Changes in Public API:

   • The index.ts file introduces several new exports (CredentialRedactor, MCPGateway, etc.) and modifies existing ones. While these additions are non-breaking, they significantly expand the API surface area, which could lead to compatibility issues in downstream projects.
   • Action: Clearly document all new exports and their intended usage. Provide migration guides if necessary.

2. Backward Compatibility for AuditConfig and AuditEntry:

   • The removal of LegacyPolicyDecision from audit.ts may break compatibility for users relying on this type.
   • Action: Deprecate LegacyPolicyDecision instead of removing it outright. Add a warning in the documentation about its future removal.

---

💡 Suggestions for Improvement

1. Thread Safety:

   • The CredentialRedactor uses a WeakMap to track seen objects during recursive redaction. While this approach avoids circular references, it is unclear whether the implementation is thread-safe.
   • Suggestion: Add explicit documentation about thread safety and consider using locks or other synchronization mechanisms if concurrent usage is expected.

2. OWASP MCP Compliance Gap:

   • The PR mentions that §11 (Consent UI) is out of scope for server-side SDKs. While this is reasonable, consider adding a note in the documentation to clarify this limitation for users.
   • Suggestion: Provide examples or guidelines for implementing client-side consent UI to achieve full OWASP MCP compliance.

3. Logging and Debugging:

   • The debugSecurityFailure function is used for logging security failures, but its behavior is not defined in the PR.
   • Suggestion: Ensure that sensitive information is not logged. Add tests to verify that logs do not leak sensitive data.

4. Rate Limiter Configuration:

   • The MCPSlidingRateLimiter is used for rate limiting, but its configuration options are not fully documented.
   • Suggestion: Provide detailed documentation for configuring the rate limiter, including examples of usage.

5. Credential Redaction Coverage:

   • The CredentialRedactor includes a set of built-in patterns for sensitive data. While comprehensive, it may not cover all use cases (e.g., custom API keys or proprietary tokens).
   • Suggestion: Allow users to dynamically update the list of patterns at runtime, and provide a mechanism to validate these patterns.

6. Test Coverage:

   • While the PR mentions that tests have been added, the diff does not include the test files. It is crucial to ensure that all new features are thoroughly tested, especially for edge cases and failure scenarios.
   • Suggestion: Verify that tests cover all critical paths, including fail-closed scenarios, edge cases for regex validation, and concurrent usage.

---

Final Assessment

This PR introduces significant security improvements and new features to the TypeScript Agent Mesh SDK. However, there are critical issues related to replay protection, regex validation, and fail-closed behavior that must be addressed before merging. Additionally, potential breaking changes and areas for improvement should be documented and tested thoroughly.

Recommended Actions Before Merge:

1. Address the 🔴 CRITICAL issues.
2. Provide documentation for new features and API changes.
3. Ensure comprehensive test coverage for all new functionality.

Once these issues are resolved, the PR can be considered for merging.

[github-actions]

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

📝 Documentation Sync Report

Issues Found

1. ❌ CredentialRedactor in packages/agent-mesh/sdks/typescript/src/credential-redactor.ts — missing docstring for the class and its methods.
2. ❌ MCPGateway in packages/agent-mesh/sdks/typescript/src/mcp-gateway.ts — missing docstring for the class and its methods.
3. ❌ InMemoryMCPAuditSink in packages/agent-mesh/sdks/typescript/src/mcp-gateway.ts — missing docstring for the class and its methods.
4. ⚠️ packages/agent-mesh/README.md — does not mention the new MCP-related features, such as CredentialRedactor, MCPGateway, or other MCP-related classes and utilities.
5. ⚠️ CHANGELOG.md — no entry for the addition of MCP protocol governance features.

Suggestions

• 💡 Add docstrings for CredentialRedactor and its methods, explaining the purpose, parameters, return values, and exceptions.
• 💡 Add docstrings for MCPGateway and its methods, including details about its configuration and usage.
• 💡 Add docstrings for InMemoryMCPAuditSink and its methods, describing its purpose and behavior.
• 💡 Update the README.md for the agent-mesh package to include a section on MCP protocol governance, highlighting the new classes and their use cases.
• 💡 Add an entry to CHANGELOG.md summarizing the addition of MCP protocol governance, including the new classes and features.

Example Docstring for CredentialRedactor

/**
 * CredentialRedactor is responsible for identifying and redacting sensitive information
 * such as API keys, tokens, and passwords from strings or objects.
 *
 * @param config - Configuration options for the redactor.
 *   - replacementText: The text to replace sensitive information with. Defaults to '[REDACTED]'.
 *   - redactSensitiveKeys: Whether to redact sensitive keys in objects. Defaults to true.
 *   - customPatterns: Additional custom patterns to match and redact.
 */
export class CredentialRedactor {
  /**
   * Redacts sensitive information from a string.
   *
   * @param value - The string to redact.
   * @param path - Optional path to include in redaction metadata.
   * @returns An object containing the redacted string and metadata about the redactions.
   */
  redactString(value: string, path?: string): CredentialRedactionResult<string>;

  /**
   * Redacts sensitive information from an object or array.
   *
   * @param value - The object or array to redact.
   * @returns An object containing the redacted value and metadata about the redactions.
   */
  redact<T>(value: T): CredentialRedactionResult<T>;
}

If the above issues are addressed, the documentation will be in sync.

[github-actions]

🤖 AI Agent: breaking-change-detector — Summary

🔍 API Compatibility Report

Summary

This pull request introduces new features to the TypeScript SDK for Agent Mesh, specifically adding MCP (Model Context Protocol) governance capabilities. The changes are primarily additive, with new classes, methods, and types introduced. There are no breaking changes identified in the Python packages or the TypeScript SDK.

Findings

Severity	Package	Change	Impact
🔵	agent-mesh	Added CredentialRedactor class	New feature, not breaking
🔵	agent-mesh	Added MCPGateway class	New feature, not breaking
🔵	agent-mesh	Added InMemoryMCPAuditSink class	New feature, not breaking
🔵	agent-mesh	Added multiple MCP-related types to types.ts	New types, not breaking
🔵	agent-mesh	Added new exports in index.ts	New exports, not breaking
🟡	agent-mesh	Changed AgentIdentity.suspend and AgentIdentity.revoke method signatures to rename reason parameter to _reason	May break callers relying on named arguments

Migration Guide

For 🟡 Potentially Breaking Changes:

1. AgentIdentity.suspend and AgentIdentity.revoke parameter name change:
   • If your code uses named arguments to call these methods, update the parameter name from reason to _reason.
   • Example:

     // Before
     agentIdentity.suspend({ reason: "Maintenance" });
     agentIdentity.revoke({ reason: "Violation" });
     
     // After
     agentIdentity.suspend({ _reason: "Maintenance" });
     agentIdentity.revoke({ _reason: "Violation" });

For 🔵 Additive Changes:

• Review the new classes (CredentialRedactor, MCPGateway, InMemoryMCPAuditSink) and types added to the SDK. These additions expand functionality and should be documented for users.

Conclusion

✅ No breaking changes were found in the Python packages. The TypeScript SDK changes are mostly additive, with one minor potentially breaking change due to parameter renaming in two methods.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review for feat(typescript): add MCP protocol governance with full OWASP coverage

---

🔴 CRITICAL: Security Issues

1. Replay Protection in McpMessageSigner and McpMessageValidator:

   • The implementation mentions bounded nonce replay cache and concurrent replay protection but does not provide details about the size of the cache or its eviction policy. If the cache size is too small or eviction is not handled properly, it could lead to replay attacks.
   • Recommendation: Ensure that the nonce cache size is configurable and sufficiently large to handle high-throughput scenarios. Implement a robust eviction policy (e.g., LRU) to avoid cache overflow.

2. Regex Injection in CredentialRedactor:

   • The toGlobalPattern function dynamically compiles regex patterns, which could lead to regex injection if the input is not properly sanitized.
   • Recommendation: Validate and sanitize all user-provided regex patterns before compiling them. Consider rejecting overly complex or potentially malicious patterns.

3. Blocked Patterns in MCPGateway:

   • The blockedPatterns array allows user-defined regex patterns. If these patterns are not properly validated, they could introduce vulnerabilities such as denial-of-service attacks due to catastrophic backtracking.
   • Recommendation: Ensure that all regex patterns in blockedPatterns are validated for performance and safety using a library like safe-regex.

4. Sensitive Key Redaction in CredentialRedactor:

   • The SENSITIVE_KEY_PATTERN regex is case-insensitive but may miss certain variations of sensitive keys (e.g., apiKey vs. api_key).
   • Recommendation: Expand the regex to cover more variations and edge cases. Consider using a curated list of sensitive key patterns.

5. Fail-Closed Behavior in MCPGateway:

   • While the fail-closed behavior is mentioned, there is no explicit test coverage or guarantees for all failure scenarios (e.g., unexpected exceptions in policyEvaluator or approvalHandler).
   • Recommendation: Add comprehensive tests to ensure fail-closed behavior in all possible failure scenarios.

---

🟡 WARNING: Potential Breaking Changes

1. Expanded Exports in index.ts:

   • The addition of numerous new exports in index.ts could lead to unintended consequences for downstream consumers who may not expect these new APIs.
   • Recommendation: Clearly document the new exports and mark them as experimental if they are not yet stable.

2. Changes to AgentIdentity.suspend and AgentIdentity.revoke:

   • The _reason parameter is now prefixed with an underscore, which could break existing code that relies on the previous parameter name.
   • Recommendation: Maintain backward compatibility by supporting both reason and _reason for a deprecation period.

---

💡 Suggestions for Improvement

1. Thread Safety in CredentialRedactor:

   • The redactNode method uses a WeakMap for tracking seen objects but does not explicitly guarantee thread safety.
   • Recommendation: Document whether CredentialRedactor is thread-safe. If not, consider adding thread safety or warning users in the documentation.

2. OWASP MCP Security Cheat Sheet Coverage:

   • While the PR claims coverage of 11/12 sections, it would be helpful to explicitly list the covered sections and provide links to relevant tests or documentation.
   • Recommendation: Add a summary table in the documentation showing OWASP MCP sections and their corresponding implementation/test coverage.

3. Rate Limiting in MCPGateway:

   • The rate limiter implementation is abstracted but does not provide visibility into its configuration or behavior.
   • Recommendation: Add logging or metrics to track rate limiter usage and effectiveness.

4. Credential Redaction Preview Truncation:

   • The truncatePreview function is used to truncate matched text for redaction. Ensure that the truncation logic does not inadvertently expose sensitive information.
   • Recommendation: Add tests to verify that truncated previews do not leak sensitive data.

5. Documentation for CredentialRedactor:

   • The CredentialRedactor class is a critical security feature but lacks detailed documentation.
   • Recommendation: Provide examples and guidelines for using CredentialRedactor, including how to define custom patterns and configure the redaction behavior.

6. Testing Coverage:

   • While the PR mentions that tests have been added, it is unclear whether all edge cases (e.g., regex validation, fail-closed behavior) are covered.
   • Recommendation: Add tests for edge cases, such as invalid regex patterns, large payloads, and high-concurrency scenarios.

---

Summary

• This PR introduces critical security features for MCP governance but has several areas that require attention to ensure robustness and compliance with security best practices.
• Address the identified security issues and potential breaking changes before merging.
• Enhance documentation and testing to improve maintainability and confidence in the implementation.

Let me know if you need further clarification or assistance!

[github-actions]

🤖 AI Agent: test-generator — `packages/agent-compliance/src/agent_compliance/integrity.py`

🧪 Test Coverage Analysis

packages/agent-compliance/src/agent_compliance/integrity.py

• ✅ Existing coverage: The file likely has existing tests that cover basic integrity checks and validation of governance modules.
• ❌ Missing coverage: There may be insufficient coverage for edge cases related to policy evaluation, such as conflicting policies, policy bypass attempts, and validation of module names.
• 💡 Suggested test cases:
  1. test_policy_conflict_resolution — Test the behavior when two conflicting policies are applied to the same governance module and ensure the correct policy is enforced.
  2. test_policy_bypass_attempt — Simulate an attempt to bypass a policy and verify that the integrity checks prevent the bypass.
  3. test_module_name_validation — Test various invalid module names to ensure that the _validate_module_name function correctly rejects them.

packages/agent-mesh/sdks/typescript/src/credential-redactor.ts

• ✅ Existing coverage: The file likely has tests that cover basic credential redaction functionality.
• ❌ Missing coverage: There may be missing tests for edge cases such as malformed inputs, oversized payloads, and specific patterns that should be redacted.
• 💡 Suggested test cases:
  1. test_redact_malformed_input — Test how the redactor handles malformed input strings that do not match any patterns.
  2. test_redact_large_payload — Test the redactor's performance and correctness when processing oversized payloads to ensure it does not crash or hang.
  3. test_redact_custom_patterns — Verify that custom patterns provided in the configuration are correctly applied and redacted.

packages/agent-mesh/sdks/typescript/src/mcp-gateway.ts

• ✅ Existing coverage: The file likely has tests for basic functionality of the MCP Gateway, including tool evaluation and decision-making.
• ❌ Missing coverage: There may be insufficient tests for edge cases such as timeout handling, cascading failures, and race conditions in concurrent evaluations.
• 💡 Suggested test cases:
  1. test_evaluate_tool_call_timeout — Simulate a timeout scenario during tool evaluation and ensure that the gateway handles it gracefully without crashing.
  2. test_cascading_failure_handling — Test how the gateway responds to a series of failures in policy evaluation or tool access, ensuring it does not propagate failures incorrectly.
  3. test_concurrent_evaluations — Test for race conditions by simulating multiple concurrent evaluations of the same tool and ensure that the state remains consistent.

packages/agent-mesh/sdks/typescript/src/identity.ts

• ✅ Existing coverage: The file likely has tests for identity management functions such as suspension and revocation.
• ❌ Missing coverage: There may be missing tests for edge cases related to trust scoring, such as handling expired certificates and revoked trust.
• 💡 Suggested test cases:
  1. test_suspend_revoked_identity — Ensure that attempting to suspend a revoked identity throws the appropriate error.
  2. test_revoke_identity_twice — Test the behavior when revoking an identity that is already revoked to ensure it handles this gracefully.
  3. test_expired_certificate_handling — Verify that the system correctly identifies and handles expired certificates during trust scoring evaluations.

These suggested test cases aim to enhance the robustness of the code by addressing potential edge cases and ensuring comprehensive coverage across critical functionalities.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: feat(typescript): add MCP protocol governance with full OWASP coverage

This PR introduces a significant addition to the TypeScript Agent Mesh SDK, implementing MCP (Model Context Protocol) security primitives. Below is a detailed review of the changes, focusing on the specified areas of concern.

---

🔴 CRITICAL: Security Issues

1. Replay Protection in MCPMessageSigner

   • The MCPMessageSigner implementation relies on a nonce replay cache for replay protection. However, the implementation details are not visible in the provided diff. Ensure that:
     • The nonce cache has a bounded size to prevent memory exhaustion.
     • Nonces are securely generated using a cryptographically secure random number generator.
     • Nonces are stored securely and are not predictable.
     • Nonces are validated against the cache before processing the message.
   • If any of these conditions are not met, it could lead to replay attacks or denial-of-service vulnerabilities.

2. Regex Denial of Service (ReDoS) in CredentialRedactor and MCPGateway

   • The CredentialRedactor and MCPGateway classes use regular expressions for pattern matching. While there is a validateRegex function and a time-budget mechanism (createRegexScanBudget), the implementation details of these mechanisms are not provided. Ensure that:
     • The validateRegex function checks for catastrophic backtracking patterns in the regular expressions.
     • The time-budget mechanism is robust and prevents excessive resource consumption.
   • Without these safeguards, the use of untrusted input in regular expressions could lead to ReDoS vulnerabilities.

3. Shell Expansion Detection in MCPGateway

   • The hasShellExpansion function checks for shell expansion patterns (e.g., $()). However, this check is rudimentary and may not cover all cases. Consider using a more comprehensive approach to detect and mitigate shell injection vulnerabilities.

4. Credential Redaction

   • The CredentialRedactor class attempts to redact sensitive information from logs. However:
     • Ensure that the BUILTIN_PATTERNS list is comprehensive and includes patterns for all sensitive data types (e.g., JWTs, OAuth tokens, etc.).
     • The truncatePreview function is not visible in the diff. Verify that it does not inadvertently expose sensitive information.
     • Consider adding a mechanism to log when sensitive data is redacted to help identify potential leaks.

5. Approval Handler in MCPGateway

   • The approvalHandler is optional, and its absence defaults the approvalStatus to Pending. This could lead to a denial of service if approvals are not processed in a timely manner. Consider adding a configurable timeout or fallback mechanism for approvals.

6. Rate Limiting in MCPGateway

   • The rateLimiter implementation is not visible in the diff. Ensure that:
     • The rate limiter is resistant to bypass techniques, such as IP spoofing or user-agent spoofing.
     • The rate limiter is not vulnerable to timing attacks.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes

   • The index.ts file now exports several new classes and types, which could potentially introduce breaking changes if existing consumers of the SDK are not updated. Ensure that:
     • These changes are documented in the release notes.
     • A migration guide is provided if necessary.

2. Policy Engine Changes

   • The PolicyDecision type has been modified to remove LegacyPolicyDecision. This could break existing integrations that rely on the old type. Consider providing a deprecation period or a compatibility layer.

---

💡 Suggestions for Improvement

1. Thread Safety

   • The CredentialRedactor and MCPGateway classes use shared state (e.g., this.auditEntries and this.redactor). Ensure that these shared resources are accessed in a thread-safe manner, especially if the SDK is used in a multi-threaded environment.

2. Test Coverage

   • While the PR mentions that tests have been added, ensure that:
     • All new functionality is covered by unit tests, including edge cases.
     • Security-critical components (e.g., MCPMessageSigner, MCPGateway) are tested against known attack vectors.
     • Performance tests are included for regex-based operations to detect potential bottlenecks.

3. Backward Compatibility

   • Consider marking deprecated types and methods with comments or annotations to guide developers during migration.

4. Documentation

   • The new features introduced in this PR are complex and security-critical. Ensure that the documentation includes:
     • Detailed usage examples for each new class and method.
     • Security best practices for using the SDK.
     • A clear explanation of the fail-closed behavior in case of errors.

5. OWASP MCP Security Cheat Sheet Compliance

   • While the PR claims compliance with 11 out of 12 sections of the OWASP MCP Security Cheat Sheet, consider adding a compliance matrix to the documentation to explicitly map features to the cheat sheet sections.

---

Summary

This PR introduces robust security primitives for MCP governance, addressing many critical areas of concern. However, there are potential security vulnerabilities (e.g., replay protection, ReDoS) and breaking changes that need to be addressed. Additionally, improvements in thread safety, test coverage, and documentation would enhance the overall quality of the implementation.

Action Items:

• Address the critical security issues flagged above.
• Ensure backward compatibility or provide a migration guide.
• Improve test coverage and documentation.

Let me know if you need further clarification or assistance!

[imran-siddique]

Review: ESCALATED — Major Cross-Cutting Feature 🚨

This PR is part of a series of 18 PRs by @jackbatzner adding MCP protocol governance packages across 5 languages (TypeScript, .NET, Python, Go, Rust) plus documentation, OWASP mappings, and framework integrations.

Per repo policy, this triggers multiple escalation rules:

• ✅ New packages or modules that change the repo structure
• ✅ Cross-cutting changes spanning 3+ packages
• ✅ New framework integrations or SDK additions

Recommendation: These PRs should be reviewed as a cohesive set with a defined merge order. Maintainer design review of the overall MCP governance architecture is needed before merging individual PRs.

Related PRs in this series: #775, #791, #824, #825, #826, #827, #828, #829, #830, #832, #833, #834, #835, #836, #839, #843, #844

[imran-siddique]

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

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

We're closing this PR because we'll implement the TypeScript MCP governance (3,349 lines) in-house following this architecture. Your work directly informed our approach — the OWASP mapping methodology, the gateway/rate-limiter/session-auth pattern, and the trust proxy design all carry forward.

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