feat: ScopeBlind protect-mcp integration — Cedar policy enforcement + verifiable receipts#667
Conversation
…nd verifiable decision receipts Adds `packages/agentmesh-integrations/scopeblind-protect-mcp/` with four components: - CedarPolicyBridge: maps Cedar allow/deny decisions into AGT evaluate() — Cedar deny is authoritative and cannot be overridden by trust scores - ReceiptVerifier: validates Ed25519-signed decision receipt structure, converts to AGT-compatible context - SpendingGate: enforces issuer-blind spending authority with trust-score gating and utilization band checks - scopeblind_context(): builds AGT-compatible context from protect-mcp artifacts Key architectural difference from mcp-trust-proxy: mcp-trust-proxy gates on trust scores (soft signals). protect-mcp gates on Cedar policies (formal, deterministic, auditable). Decision receipts provide cryptographic proof via IETF Internet-Draft draft-farley-acta-signed-receipts. 36 tests covering: Cedar decision parsing, policy bridge authorization, receipt validation, spending gate limits/categories/utilization bands, and AGT context shape compatibility. protect-mcp: https://www.npmjs.com/package/protect-mcp (v0.4.6, MIT) Docs: https://scopeblind.com/docs/protect-mcp Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Welcome to the Agent Governance Toolkit! Thanks for your first pull request. |
🤖 AI Agent: contributor-guide — Welcome! 🎉Welcome! 🎉Hi @first-time-contributor! 👋 Welcome to the microsoft/agent-governance-toolkit community, and thank you for taking the time to contribute. We’re thrilled to have you here and really appreciate your effort in submitting this pull request. Your contribution is a fantastic addition to the project, and we’re excited to review it with you! What You Did Well 🌟
Suggestions for Improvement 🛠️While your contribution is excellent, there are a few areas where we can align it better with the project’s conventions and ensure long-term maintainability:
Next Steps 🚀
Once you’ve made these updates, push your changes to this branch, and we’ll take another look. If you have any questions or need help with anything, don’t hesitate to ask — we’re here to help! Thank you again for contributing to the project. We’re excited to collaborate with you! 😊 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Review Summary
This PR introduces a new integration package, scopeblind-protect-mcp, which integrates the protect-mcp library into the Agent Governance Toolkit (AGT). The integration provides runtime Cedar policy enforcement and cryptographically verifiable decision receipts, enhancing the security and auditability of the AGT framework. The PR includes new components such as CedarPolicyBridge, ReceiptVerifier, SpendingGate, and scopeblind_context().
The implementation is well-documented, and the provided tests indicate a good level of coverage. However, there are several areas that require attention to ensure the robustness, security, and maintainability of the code.
🔴 CRITICAL
-
Receipt Verification Delegation
- The
ReceiptVerifierclass delegates cryptographic verification of Ed25519 signatures to external libraries (@veritasacta/verifyorprotect-mcpruntime). While this is acceptable, the absence of direct cryptographic verification in this library introduces a potential security risk if the external library has vulnerabilities or is compromised. - Action: Add a fallback mechanism to verify Ed25519 signatures directly within this library using a trusted Python cryptography library (e.g.,
pynacl). This ensures that the library can independently verify receipts if the external library is unavailable or untrusted.
- The
-
Trust Score Manipulation
- The
CedarPolicyBridgeadjusts the trust score based on Cedar decisions (trust_bonus_per_allowanddeny_penalty). This could lead to unintended consequences, such as trust scores being manipulated by malicious actors to bypass other security checks. - Action: Clearly document the rationale for modifying trust scores and consider adding safeguards to prevent abuse. For example, ensure that trust score adjustments are bounded and cannot be exploited by repeatedly triggering Cedar allow decisions.
- The
-
Receipt Hashing
- The receipt hash is calculated using
hashlib.sha256and truncated to 16 characters. This truncation significantly reduces the entropy of the hash, making it susceptible to collisions. - Action: Use the full hash value or a cryptographically secure truncation method (e.g., HMAC with a secret key) to prevent potential collision attacks.
- The receipt hash is calculated using
-
Receipt Validation
- The
validate_structuremethod inReceiptVerifieronly validates the structure of the receipt and does not perform cryptographic verification. This could lead to a false sense of security if developers assume the receipt is fully validated. - Action: Rename the method to
validate_structure_onlyor similar to make it clear that cryptographic validation is not performed. Additionally, provide explicit documentation about the need for external cryptographic verification.
- The
🟡 WARNING
-
Backward Compatibility
- The introduction of
scopeblind-protect-mcpdoes not appear to modify existing functionality in AGT. However, the integration of Cedar policies as "hard constraints" may impact existing workflows if users are not aware of the implications. - Action: Clearly document the impact of Cedar policy enforcement on existing AGT workflows. Provide migration guidance for users who may need to adapt their configurations to accommodate the new integration.
- The introduction of
-
Dependency Management
- The
protect-mcplibrary is a Node.js package, which introduces a cross-language dependency. This could complicate deployment and increase the risk of dependency-related issues. - Action: Ensure that the integration package includes clear instructions for installing and managing the
protect-mcpdependency. Consider providing a Docker container or other pre-configured environment to simplify deployment.
- The
💡 SUGGESTIONS
-
Type Annotations
- While the code includes some type annotations, they are not comprehensive. For example, the
evaluate_spendmethod inSpendingGatehas parameters without type annotations. - Action: Add type annotations to all methods and parameters to improve code clarity and enable static type checking.
- While the code includes some type annotations, they are not comprehensive. For example, the
-
Thread Safety
- The
CedarPolicyBridge,ReceiptVerifier, andSpendingGateclasses maintain internal state (e.g.,_history,_verified,_decisions) without any thread-safety mechanisms. This could lead to race conditions in concurrent environments. - Action: Use thread-safe data structures (e.g.,
queue.Queue) or add locks to ensure thread safety.
- The
-
Error Handling
- The code does not handle exceptions that may arise from external dependencies (e.g., JSON parsing, receipt validation). This could lead to unhandled exceptions in production.
- Action: Add error handling for external dependencies and provide meaningful error messages to help diagnose issues.
-
Test Coverage
- While the PR mentions 36 tests, it is unclear if they cover edge cases, such as malformed receipts, invalid signatures, or extreme trust score values.
- Action: Add tests for edge cases, including:
- Receipts with missing or invalid fields.
- Trust scores at the boundaries of
trust_floorandtrust_bonus. - Concurrent evaluations to test thread safety.
-
Documentation
- The documentation is comprehensive but could benefit from additional examples and diagrams to illustrate the integration between AGT and
protect-mcp. - Action: Add diagrams or flowcharts to the README to visually explain how
scopeblind-protect-mcpintegrates with AGT.
- The documentation is comprehensive but could benefit from additional examples and diagrams to illustrate the integration between AGT and
Final Assessment
This PR introduces a valuable integration that enhances the security and auditability of AGT. However, the identified critical issues must be addressed to ensure the robustness and security of the implementation. Additionally, addressing the warnings and suggestions will improve the maintainability and usability of the code.
🤖 AI Agent: security-scanner — Security Analysis of the Pull RequestSecurity Analysis of the Pull RequestThis pull request introduces a new integration for the ScopeBlind 1. Prompt Injection Defense Bypass
2. Policy Engine Circumvention
3. Trust Chain Weaknesses
4. Credential Exposure
5. Sandbox Escape
6. Deserialization Attacks
7. Race Conditions
8. Supply Chain Risks
Summary of Findings and Recommendations
General Recommendations
This PR introduces significant security improvements to the AgentMesh framework but also introduces potential risks that need to be addressed before merging. |
|
@microsoft-github-policy-service agree company="ScopeBlind Pty Ltd" |
|
Thanks for the thorough review. Addressing each point: 🔴 CRITICAL responses:
🟡 WARNING responses:
💡 SUGGESTIONS: Will address type annotations, thread safety (adding Updated IETF draft ( |
…ty, edge case tests, full hash Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Follow-up commit addressing all code review items:
56 tests passing (36 original + 20 edge cases). |
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Code Review Feedback for feat: ScopeBlind protect-mcp integration
🔴 CRITICAL: Security Issues
-
Receipt Validation Delegation
- The
ReceiptVerifierclass delegates cryptographic signature verification to external tools (@veritasacta/verifyorprotect-mcp runtime). While this is acceptable for modularity, the absence of in-library cryptographic verification introduces a risk of bypass if the external verifier is misconfigured or compromised. - Recommendation: Add an optional fallback mechanism to perform Ed25519 signature verification directly within the library using a Python cryptography library (e.g.,
pynacl).
- The
-
Thread Safety in ReceiptVerifier
- The
_verifiedlist inReceiptVerifieris accessed and modified using a threading lock. However, the lock does not guarantee atomicity for complex operations like appending and reading. This could lead to race conditions in highly concurrent environments. - Recommendation: Use thread-safe data structures like
queue.Queueorcollections.dequewithmaxlenfor managing the_verifiedlog.
- The
-
SpendingGate Utilization Band Validation
- The
evaluate_spendmethod inSpendingGatedoes not validate theutilization_bandinput against theUTILIZATION_BANDSset. This could allow invalid or malicious inputs to bypass checks. - Recommendation: Add explicit validation for
utilization_bandto ensure it is one of the allowed values (low,medium,high,exceeded).
- The
-
CedarPolicyBridge Trust Adjustment
- The
CedarPolicyBridgeclass adjusts trust scores (trust_bonusanddeny_penalty) without bounds checking. This could lead to unintended behavior if the trust score exceeds the maximum (1000) or drops below zero. - Recommendation: Enforce bounds on
adjusted_trustto ensure it remains within[0, 1000].
- The
🟡 WARNING: Potential Breaking Changes
-
Hard Dependency on
protect-mcp- The integration assumes that
protect-mcpis always available and functioning correctly. Ifprotect-mcpintroduces breaking changes or is unavailable, this integration will fail. - Recommendation: Add version pinning for
protect-mcpin the documentation and provide fallback mechanisms or error handling for cases whereprotect-mcpis unavailable.
- The integration assumes that
-
Cedar Deny as Authoritative
- The design enforces that a Cedar
denydecision is final and cannot be overridden by trust scores. While this is a security feature, it could break workflows that rely on trust scores to override policy decisions. - Recommendation: Clearly document this behavior in the release notes and provide a configuration option to allow overrides if needed.
- The design enforces that a Cedar
💡 Suggestions for Improvement
-
Type Annotations
- While the code includes type hints, some areas lack full type coverage. For example, the
receiptparameter inReceiptVerifier.validate_structure_onlyis typed asDict[str, Any], but its structure is well-defined. - Recommendation: Use
TypedDictorPydanticmodels to define the expected structure of receipts for better type safety and validation.
- While the code includes type hints, some areas lack full type coverage. For example, the
-
Pydantic Model Validation
- The
CedarDecisionand other data structures could benefit from Pydantic models for validation and serialization. This would ensure stricter type safety and reduce the risk of malformed data. - Recommendation: Replace
dataclasswithPydanticmodels where applicable.
- The
-
Logging and Observability
- The library currently lacks logging for critical operations like receipt validation, Cedar policy evaluation, and spending gate decisions. This makes debugging and monitoring difficult.
- Recommendation: Add structured logging (e.g., using
structlog) for key operations to improve observability.
-
Unit Test Coverage
- While the PR mentions 36 tests, it is unclear if edge cases (e.g., malformed receipts, invalid utilization bands, extreme trust scores) are covered.
- Recommendation: Add tests for edge cases and failure scenarios to ensure robustness.
-
Backward Compatibility
- The integration introduces new components but does not modify existing ones. However, the reliance on
protect-mcpand the authoritative nature of Cedardenydecisions could indirectly affect existing workflows. - Recommendation: Provide a migration guide for users to adopt this integration without disrupting existing setups.
- The integration introduces new components but does not modify existing ones. However, the reliance on
-
Documentation
- The documentation is comprehensive but could benefit from additional examples, such as how to handle errors when
protect-mcpfails or how to configure theCedarPolicyBridgefor different trust models. - Recommendation: Expand the README with advanced usage examples and troubleshooting tips.
- The documentation is comprehensive but could benefit from additional examples, such as how to handle errors when
Summary of Feedback
| Category | Severity | Action |
|---|---|---|
| Security | 🔴 CRITICAL | Add in-library Ed25519 verification, thread-safe structures, and input validation. |
| Breaking Changes | 🟡 WARNING | Document Cedar deny behavior and dependency on protect-mcp. |
| Type Safety & Validation | 💡 SUGGESTION | Use Pydantic models and improve type annotations. |
| Observability | 💡 SUGGESTION | Add structured logging for critical operations. |
| Testing | 💡 SUGGESTION | Add edge case tests for malformed inputs and extreme scenarios. |
| Documentation | 💡 SUGGESTION | Expand README with advanced examples and troubleshooting. |
By addressing these issues, the integration will be more robust, secure, and user-friendly. Let me know if you need further clarification on any of the points!
|
@imran-siddique — all items from both code reviews are addressed in commit 9a3be8a. Quick summary:
The second code review re-flagged items that were fixed in the follow-up commit. 56 tests passing. Ready for human review when convenient. Happy to address any additional feedback. |
imran-siddique
left a comment
imran-siddique
left a comment
There was a problem hiding this comment.
Solid code quality with zero runtime deps — nice work! Two blocking issues:
- Broken upstream links — README links to github.com/scopeblind/scopeblind-gateway but that repo doesn't exist (only at github.com/tomjwxf/scopeblind-gateway). Links from a Microsoft repo must resolve.
- Empty signature passes as valid:True — ReceiptVerifier.validate_structure_only returns valid:True for receipts with empty signature strings. For a security adapter, this is misleading. Consider setting valid:False when signature is empty, or renaming the field.
Also worth addressing: unbounded list growth (memory leak), external company as sole author in pyproject.toml, and hardcoded stale version '0.4.6'.
… version - Empty signature/publicKey now returns valid:False (security fix) - All internal lists bounded to MAX_LOG=10000 (prevents memory leak) - README links use correct case: ScopeBlind/scopeblind-gateway - pyproject.toml adds Tom Farley as individual author - Version reference updated from 0.4.6 to 0.5.2 - 56 tests passing
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Review of PR: feat: ScopeBlind protect-mcp integration — Cedar policy enforcement + verifiable receipts
🔴 CRITICAL: Security Issues
-
Receipt Verification Delegation:
- The
ReceiptVerifierclass delegates cryptographic signature verification to external tools (@veritasacta/verifyorprotect-mcpruntime). While this is acceptable, the lack of direct verification in the Python code means that the integration is entirely reliant on external tools for security. If these tools are compromised or misconfigured, the system could be vulnerable to forged receipts. - Recommendation: Consider implementing a fallback mechanism for verifying Ed25519 signatures directly in Python using a library like
pynacl. This would provide an additional layer of security and reduce reliance on external tools.
- The
-
Lack of Replay Protection for Receipts:
- The
ReceiptVerifierdoes not implement any mechanism to detect or prevent replay attacks. An attacker could reuse a valid receipt to bypass policy checks. - Recommendation: Introduce a mechanism to track and reject previously seen receipts. This could involve maintaining a cache of receipt hashes or timestamps, ensuring that each receipt is only used once.
- The
-
Potential Denial of Service (DoS) via Unbounded History Growth:
- Both
CedarPolicyBridgeandReceiptVerifiermaintain in-memory logs (_historyand_verified) with a maximum size of 10,000 entries. While there is a mechanism to truncate the logs, this approach may still be vulnerable to DoS attacks if an attacker floods the system with requests. - Recommendation: Add rate-limiting mechanisms to prevent abuse. Additionally, consider using a more robust storage mechanism (e.g., a database or a time-based eviction cache) for logs to handle larger volumes of data without risking memory exhaustion.
- Both
-
Lack of Cryptographic Key Validation:
- The
validate_structure_onlymethod inReceiptVerifierdoes not validate the format or authenticity of thepublicKeyfield in the receipt. This could allow an attacker to inject malformed or malicious keys. - Recommendation: Add validation for the
publicKeyfield to ensure it adheres to the expected Ed25519 key format.
- The
-
Potential Timing Attacks:
- The
validate_structure_onlymethod performs string comparisons (e.g.,if not sig or not pk) without using constant-time comparison functions. This could make the system vulnerable to timing attacks. - Recommendation: Use constant-time comparison functions (e.g.,
hmac.compare_digest) for sensitive data like signatures and public keys.
- The
🟡 WARNING: Potential Breaking Changes
-
Cedar Deny as Authoritative:
- The
CedarPolicyBridgeenforces Cedardenydecisions as authoritative, overriding any AGT trust scores. This introduces a hard constraint that could break existing workflows relying solely on trust scores. - Recommendation: Clearly document this behavior in the release notes and provide a migration guide for users to adapt their policies and workflows.
- The
-
Receipt Requirement:
- The
require_receiptflag inCedarPolicyBridgeenforces the presence of a receipt for every evaluation. If enabled, this could break existing integrations that do not provide receipts. - Recommendation: Ensure this flag is disabled by default to maintain backward compatibility. Clearly document the implications of enabling this flag.
- The
💡 Suggestions for Improvement
-
Thread Safety:
- The use of
threading.LockinCedarPolicyBridgeandReceiptVerifierensures thread safety for the in-memory logs. However, consider usingcollections.dequewith amaxlenparameter for automatic eviction of old entries, which would simplify the implementation and reduce the risk of errors.
- The use of
-
Type Annotations:
- While the code includes some type annotations, they are not comprehensive. For example, the
evaluatemethod inCedarPolicyBridgecould benefit from more detailed type hints for thereceiptparameter (e.g.,Optional[Dict[str, Any]]). - Recommendation: Add type annotations for all methods and parameters to improve code clarity and leverage static type checking tools like
mypy.
- While the code includes some type annotations, they are not comprehensive. For example, the
-
Pydantic Models for Validation:
- The
CedarDecisionandReceiptVerifierclasses manually validate data structures. Using Pydantic models would simplify validation, ensure type safety, and provide better error messages. - Recommendation: Refactor
CedarDecisionandReceiptVerifierto use Pydantic models for data validation and parsing.
- The
-
Test Coverage:
- While the PR mentions 36 tests, it is unclear if they cover all edge cases, especially for security-critical components like
ReceiptVerifierandCedarPolicyBridge. - Recommendation: Add tests for edge cases, such as malformed receipts, missing fields, invalid signatures, and replay attacks. Use tools like
pytest-covto measure test coverage and ensure critical paths are thoroughly tested.
- While the PR mentions 36 tests, it is unclear if they cover all edge cases, especially for security-critical components like
-
Documentation:
- The documentation is thorough and well-written, but it could benefit from additional examples, especially for advanced use cases like integrating with custom AGT policies or handling complex spending authority scenarios.
- Recommendation: Expand the documentation with more detailed examples and a troubleshooting section for common issues.
-
Dependency Management:
- The
pyproject.tomlfile does not specify any dependencies forprotect-mcpor@veritasacta/verify. This could lead to runtime errors if these dependencies are not installed. - Recommendation: Explicitly list all required dependencies in the
pyproject.tomlfile, including version constraints to ensure compatibility.
- The
-
Performance Considerations:
- The
CedarPolicyBridgeandReceiptVerifierclasses perform JSON serialization (json.dumps) and SHA-256 hashing for receipt references. While this is generally efficient, it could become a bottleneck under high load. - Recommendation: Benchmark the performance of these operations under expected load conditions and optimize if necessary.
- The
-
Error Handling:
- The
evaluatemethod inCedarPolicyBridgeand thevalidate_structure_onlymethod inReceiptVerifierreturn error messages as strings in thereasonfield. This approach may not be sufficient for debugging complex issues. - Recommendation: Use structured error objects or enums to provide more detailed and actionable error information.
- The
Summary
This PR introduces a robust integration with ScopeBlind's protect-mcp, adding Cedar policy enforcement and verifiable decision receipts to AGT. While the implementation is well-structured and aligns with the project's goals, there are critical security concerns and potential breaking changes that need to be addressed before merging. Additionally, there are opportunities to improve type safety, test coverage, and documentation.
Action Items:
- Address the 🔴 CRITICAL security issues, especially around receipt verification and replay protection.
- Mitigate 🟡 WARNING breaking changes by providing clear documentation and ensuring backward compatibility.
- Implement the 💡 SUGGESTIONS to improve code quality, maintainability, and performance.
Let me know if you need further clarification or assistance!
|
@imran-siddique - thanks for the review. All items addressed in 6e538d4:
56 tests passing. |
|
P.S. on the bot reviewer's recurring flag about "receipt verification delegation" -- this is by design, not a gap. The verification is deliberately external because that is what makes the receipts independently verifiable. If I embedded Ed25519 verification inside this adapter, the receipts would only be verifiable through this adapter. By delegating to @veritasacta/verify (Apache-2.0, zero ScopeBlind dependency), anyone can verify receipts offline with any Ed25519 implementation. The trust model is: verify the math, not the vendor. That said, adding an optional pynacl fallback for convenience (not security) is on my roadmap. |
- Public key format validation: rejects keys that aren't valid Ed25519 (64 hex chars or base64url). Catches malformed/injected keys early. - Replay protection: bounded OrderedDict tracks seen receipt hashes. Same receipt submitted twice returns valid:False with replay:True. Configurable: replay_protection=True (default), max_seen_receipts=50000. Evicts oldest entries when window is full. - Both features are on by default, configurable per-instance. - 8 new tests (64 total, all passing): key format validation (too short, non-hex, valid hex, valid base64url), replay detection, replay disabled, bounded window eviction, different receipts not flagged as replay. v0.1.1
|
Two improvements in 42c280a (v0.1.1):
64 tests passing (8 new). |
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Review Summary
This PR introduces a new integration package, scopeblind-protect-mcp, which bridges the ScopeBlind protect-mcp library with the AgentMesh framework. The integration adds runtime Cedar policy enforcement and cryptographically verifiable decision receipts to AgentMesh. The PR includes new components such as CedarPolicyBridge, ReceiptVerifier, SpendingGate, and scopeblind_context() for policy evaluation, receipt validation, and spending authority enforcement.
The PR is well-documented, with detailed explanations of the architecture, design principles, and usage examples. The code appears to be well-structured and adheres to Python best practices. However, there are some areas that require attention, especially regarding security, type safety, and potential backward compatibility issues.
🔴 CRITICAL: Security Issues
-
Replay Protection for Receipts
- The
ReceiptVerifierclass implements replay protection using an in-memoryOrderedDictto track seen receipt hashes. However, this approach has limitations:- Memory Exhaustion: If the
max_seen_receiptslimit is reached, the oldest entries are evicted, potentially allowing replay attacks for those evicted receipts. - Persistence: The replay protection state is not persisted across process restarts, making it ineffective in distributed or long-running systems.
- Memory Exhaustion: If the
- Recommendation: Use a persistent storage mechanism (e.g., Redis, database) for tracking seen receipts. This ensures replay protection is effective even in distributed environments or after restarts.
- The
-
Receipt Signature Verification
- The
ReceiptVerifierclass does not perform cryptographic verification of Ed25519 signatures. While the PR mentions that this is delegated to@veritasacta/verifyor theprotect-mcpruntime, there is no enforcement or validation of this delegation. - Recommendation: Add a mechanism to ensure that cryptographic verification is always performed, either by integrating a Python Ed25519 library or by explicitly verifying that the
protect-mcpruntime has performed the verification.
- The
-
Thread Safety
- The
CedarPolicyBridgeandReceiptVerifierclasses usethreading.Lockfor thread safety. However, the use of locks can lead to potential deadlocks or performance bottlenecks if not handled carefully. - Recommendation: Consider using thread-safe data structures like
queue.Queueorcollections.dequefor managing history and seen receipts. Alternatively, ensure that lock acquisition and release are properly managed to avoid deadlocks.
- The
-
Denial of Service (DoS) Risk
- The
validate_structure_onlymethod inReceiptVerifierprocesses untrusted input (e.g., receipts) and performs operations like JSON serialization and SHA-256 hashing. This could be exploited for DoS attacks by sending large or malformed receipts. - Recommendation: Add input size limits and validation checks to prevent processing excessively large or malformed receipts.
- The
🟡 WARNING: Potential Breaking Changes
-
Cedar Deny as Authoritative
- The
CedarPolicyBridgeenforces Cedar deny decisions as authoritative, overriding any AGT trust scores. This behavior may conflict with existing AGT policies or workflows that rely solely on trust scores. - Recommendation: Clearly document this behavior and provide a configuration option to disable it if needed. This ensures backward compatibility for existing users.
- The
-
Receipt Requirement
- The
require_receiptflag inCedarPolicyBridgeenforces the presence of a receipt for policy evaluation. Enabling this flag without proper preparation could break existing workflows. - Recommendation: Set
require_receipttoFalseby default to avoid breaking changes. Clearly document the implications of enabling this flag.
- The
💡 Suggestions for Improvement
-
Type Safety
- The code lacks type annotations for some method arguments and return values (e.g.,
CedarPolicyBridge.evaluate,CedarDecision.from_receipt, etc.). - Recommendation: Add type annotations to all public methods and classes to improve type safety and compatibility with static type checkers like
mypy.
- The code lacks type annotations for some method arguments and return values (e.g.,
-
Pydantic Model Validation
- The
CedarDecisionand receipt structures are currently validated using custom logic. Usingpydanticmodels would provide more robust validation and better error handling. - Recommendation: Replace custom validation logic with
pydanticmodels forCedarDecisionand receipt structures.
- The
-
Testing Coverage
- While the PR mentions 36 tests, it is unclear if all edge cases are covered, especially for security-critical components like receipt validation and Cedar policy enforcement.
- Recommendation: Add tests for edge cases, such as:
- Invalid or malformed receipts.
- Replay attacks with receipts.
- Boundary conditions for trust scores and spending limits.
-
Documentation
- The documentation is comprehensive, but it could benefit from additional details on:
- How to configure and deploy the integration in a production environment.
- Examples of Cedar policies and their expected behavior in AGT.
- Recommendation: Expand the documentation to include these details.
- The documentation is comprehensive, but it could benefit from additional details on:
-
Performance Optimization
- The
CedarPolicyBridgeandReceiptVerifierclasses maintain in-memory logs and history, which could grow large over time. - Recommendation: Periodically persist logs to disk or a database and clear in-memory data to reduce memory usage.
- The
-
Dependency Management
- The
pyproject.tomlfile does not listprotect-mcpas a dependency, even though it is a core part of this integration. - Recommendation: Add
protect-mcpas a dependency inpyproject.tomlto ensure it is installed automatically.
- The
Summary of Actionable Feedback
Critical
- 🔴 Implement persistent replay protection for receipts.
- 🔴 Ensure cryptographic verification of Ed25519 signatures is always performed.
- 🔴 Review and improve thread safety in
CedarPolicyBridgeandReceiptVerifier. - 🔴 Add input size limits and validation to prevent DoS attacks.
Warnings
- 🟡 Document and provide configuration options for Cedar deny being authoritative.
- 🟡 Set
require_receipttoFalseby default to avoid breaking changes.
Suggestions
- 💡 Add type annotations to all public methods and classes.
- 💡 Use
pydanticmodels forCedarDecisionand receipt validation. - 💡 Expand test coverage for edge cases and security-critical components.
- 💡 Enhance documentation with deployment and policy examples.
- 💡 Optimize memory usage by persisting logs and clearing in-memory data.
- 💡 Add
protect-mcpas a dependency inpyproject.toml.
Please address the critical issues and consider the warnings and suggestions before merging this PR. Let me know if you need further clarification or assistance!
imran-siddique
left a comment
imran-siddique
left a comment
There was a problem hiding this comment.
v0.1.1 improvements look good: key validation + replay protection. Approving.
|
Thanks @imran-siddique for the thorough review and merge. Looking forward to contributing more to AGT. |
3 participants
ScopeBlind protect-mcp Integration
Adds runtime Cedar policy enforcement and cryptographically verifiable decision receipts to AgentMesh, via protect-mcp (v0.4.6, MIT, 821KB).
What this PR adds
packages/agentmesh-integrations/scopeblind-protect-mcp/— a new integration package with:CedarPolicyBridgeevaluate()— Cedar deny is authoritativeReceiptVerifierSpendingGatescopeblind_context()Architecture — complementary to mcp-trust-proxy
AGT's
mcp-trust-proxygates on trust scores (soft signals). protect-mcp gates on Cedar policies (formal, deterministic, auditable). They compose:Cedar deny is a hard constraint — it cannot be overridden by a high trust score. Trust scoring is a soft signal layered on top of Cedar allow decisions.
Key differentiator: issuer-blind verification
protect-mcp receipts use VOPRF (RFC 9497) so the verifier can confirm a receipt is valid without learning which organization issued it. This prevents supply-chain surveillance — you can prove compliance without revealing your org structure.
AGT policy rule example
Tests
36 tests covering: Cedar decision parsing, policy bridge authorization (deny authoritative, trust layering, receipt requirements, trust cap), receipt validation (structure, types, spending authority, AGT context), spending gate (amount limits, categories, utilization bands, trust floors, receipt requirements), and context shape compatibility.
Protocol
Decision receipts follow the Veritas Acta signed receipt format, an IETF Internet-Draft for portable, verifiable decision artifacts.
protect-mcp: npm | GitHub | Docs | Cedar policies