Skip to content

feat: ScopeBlind protect-mcp integration — Cedar policy enforcement + verifiable receipts#667

Open
tomjwxf wants to merge 2 commits intomicrosoft:mainfrom
tomjwxf:feat/scopeblind-protect-mcp-integration
Open

feat: ScopeBlind protect-mcp integration — Cedar policy enforcement + verifiable receipts#667
tomjwxf wants to merge 2 commits intomicrosoft:mainfrom
tomjwxf:feat/scopeblind-protect-mcp-integration

Conversation

@tomjwxf
Copy link
Copy Markdown

@tomjwxf tomjwxf commented Apr 1, 2026

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:

Component Purpose
CedarPolicyBridge Maps Cedar allow/deny into AGT evaluate() — Cedar deny is authoritative
ReceiptVerifier Validates Ed25519-signed decision receipt structure
SpendingGate Enforces issuer-blind spending authority with trust gating
scopeblind_context() Builds AGT-compatible context from protect-mcp artifacts

Architecture — complementary to mcp-trust-proxy

AGT's mcp-trust-proxy gates on trust scores (soft signals). protect-mcp gates on Cedar policies (formal, deterministic, auditable). They compose:

Layer AGT (existing) protect-mcp (this PR)
Analysis MCP Security Scanner (static) Cedar WASM (runtime, every call)
Identity DID + trust scores Ed25519 passports + VOPRF
Decisions PolicyEngine evaluate() Cedar allow/deny + signed receipts
Proof Audit log Cryptographic receipts (offline-verifiable)
Privacy Trust scores visible Issuer-blind (verifier learns nothing about issuer)

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

- name: require-cedar-allow
  type: capability
  conditions:
    scopeblind.cedar.effect: 'allow'
    scopeblind.receipt.present: true
  allowed_actions:
    - 'tool_call.*'

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.

============================== 36 passed in 0.04s ==============================

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

…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>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

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

@github-actions github-actions bot added the size/XL Extra large PR (500+ lines) label Apr 1, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

🤖 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 🌟

  1. Thorough Documentation: Your PR description is incredibly detailed and well-structured. The breakdown of components, architecture, and examples in the README.md is excellent and provides a clear understanding of the new functionality.

  2. Test Coverage: Including 36 passing tests is fantastic! It’s clear you’ve put a lot of thought into ensuring the robustness of your implementation.

  3. Security Considerations: You've included a SECURITY.md file and provided details about how cryptographic material is handled, which is crucial for a security-sensitive project like this.

  4. Alignment with Project Goals: The integration of protect-mcp aligns well with the project's mission of enhancing agent governance, and the issuer-blind verification is a great feature that adds significant value.


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:

  1. Linting:

    • We use ruff for linting, specifically with the E, F, and W error codes. Please run ruff on your code and address any issues it flags. You can find the configuration in the root of the repository.
  2. Testing Location:

    • While your test coverage is great, the tests should be placed in the packages/agentmesh-integrations/scopeblind-protect-mcp/tests/ directory to align with our project structure. This ensures consistency across the repository.
  3. Commit Message Format:

    • We follow the Conventional Commits specification. Your commit message feat: ScopeBlind protect-mcp integration — Cedar policy enforcement + verifiable receipts is close, but it would be great if you could simplify it to something like:
      feat(scopeblind-protect-mcp): add Cedar policy enforcement and verifiable receipts
      
      This format helps us maintain a clean and consistent commit history.
  4. Security-Sensitive Code:

    • Since this PR involves cryptographic operations (e.g., Ed25519 signature validation and VOPRF), it will require extra scrutiny. While your SECURITY.md file is a great start, we recommend adding comments in the code to highlight security-critical sections. For example:
      • In ReceiptVerifier.validate_structure, clarify how you ensure the integrity of the receipt payload.
      • In SpendingGate.evaluate_spend, explain how you handle high-value transactions and ensure no sensitive data is leaked.
  5. Dependencies:

    • While protect-mcp is a lightweight dependency, it’s always good to document why it’s necessary and how it aligns with the project’s goals. Consider adding a note in the CONTRIBUTING.md file about this new dependency.

Next Steps 🚀

  1. Run ruff on your code and address any linting issues.
  2. Move your test files to packages/agentmesh-integrations/scopeblind-protect-mcp/tests/.
  3. Update your commit message to follow the Conventional Commits format.
  4. Add inline comments to highlight security-sensitive sections of your code.
  5. If you haven’t already, please review our CONTRIBUTING.md and QUICKSTART.md for additional guidance.

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! 😊

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Review Summary

This PR introduces a new 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

  1. Receipt Verification Delegation

    • The ReceiptVerifier class delegates cryptographic verification of Ed25519 signatures to external libraries (@veritasacta/verify or protect-mcp runtime). 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.
  2. Trust Score Manipulation

    • The CedarPolicyBridge adjusts the trust score based on Cedar decisions (trust_bonus_per_allow and deny_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.
  3. Receipt Hashing

    • The receipt hash is calculated using hashlib.sha256 and 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.
  4. Receipt Validation

    • The validate_structure method in ReceiptVerifier only 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_only or similar to make it clear that cryptographic validation is not performed. Additionally, provide explicit documentation about the need for external cryptographic verification.

🟡 WARNING

  1. Backward Compatibility

    • The introduction of scopeblind-protect-mcp does 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.
  2. Dependency Management

    • The protect-mcp library 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-mcp dependency. Consider providing a Docker container or other pre-configured environment to simplify deployment.

💡 SUGGESTIONS

  1. Type Annotations

    • While the code includes some type annotations, they are not comprehensive. For example, the evaluate_spend method in SpendingGate has parameters without type annotations.
    • Action: Add type annotations to all methods and parameters to improve code clarity and enable static type checking.
  2. Thread Safety

    • The CedarPolicyBridge, ReceiptVerifier, and SpendingGate classes 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.
  3. 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.
  4. 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_floor and trust_bonus.
      • Concurrent evaluations to test thread safety.
  5. 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-mcp integrates with AGT.

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.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

🤖 AI Agent: security-scanner — Security Review of ScopeBlind protect-mcp Integration

Security Review of ScopeBlind protect-mcp Integration

The proposed integration introduces runtime Cedar policy enforcement and cryptographically verifiable decision receipts into AgentMesh. While this is a significant enhancement to the security posture of the toolkit, it also introduces potential attack vectors. Below is a detailed security review based on the specified criteria.


1. Prompt Injection Defense Bypass

Rating: 🔵 LOW

The integration does not directly handle user input or natural language prompts, as it operates at the policy enforcement layer. However, the CedarPolicyBridge and ReceiptVerifier rely on structured input (e.g., Cedar decisions, receipts). These inputs are deterministic and validated, reducing the risk of injection attacks.

Recommendation: Ensure that all inputs to the CedarPolicyBridge and ReceiptVerifier are sanitized and validated against expected schemas. This is already partially addressed by the validate_structure_only method in ReceiptVerifier.


2. Policy Engine Circumvention

Rating: 🟠 HIGH

The CedarPolicyBridge enforces Cedar decisions as hard constraints, but the trust_floor mechanism introduces a potential weakness. If the trust_floor is misconfigured (e.g., set too low), it could allow agents with insufficient trust scores to bypass Cedar policies.

Attack Vector: An attacker could exploit a misconfigured trust_floor to gain unauthorized access to restricted actions.

Recommendation:

  • Add a configuration safeguard to prevent trust_floor from being set below a reasonable minimum (e.g., 300).
  • Log and alert on any overrides or misconfigurations of the trust_floor.

3. Trust Chain Weaknesses

Rating: 🔴 CRITICAL

The integration relies on Ed25519-signed receipts for cryptographic proof, but the ReceiptVerifier does not perform cryptographic signature verification itself. Instead, it delegates this to external tools like @veritasacta/verify or the protect-mcp runtime.

Attack Vector: If the external verifier is compromised or bypassed, invalid receipts could be accepted as valid, undermining the trust chain.

Recommendation:

  • Integrate cryptographic signature verification directly into the ReceiptVerifier to ensure end-to-end trust within the toolkit.
  • Validate the integrity and authenticity of the @veritasacta/verify dependency to prevent supply chain attacks.

4. Credential Exposure

Rating: 🔵 LOW

The integration does not handle sensitive credentials directly. Receipt verification and key management are delegated to the protect-mcp runtime and external verifiers.

Recommendation: Ensure that debug logs and error messages do not inadvertently expose sensitive information, such as receipt contents or cryptographic keys.


5. Sandbox Escape

Rating: 🔵 LOW

The integration does not execute untrusted code or rely on sandboxing mechanisms. The Cedar policy evaluation is performed in a controlled WASM runtime, which is inherently sandboxed.

Recommendation: Periodically review the WASM runtime for vulnerabilities and ensure it is updated to the latest secure version.


6. Deserialization Attacks

Rating: 🟡 MEDIUM

The CedarDecision.from_receipt method deserializes receipt payloads without robust validation. While the validate_structure_only method checks for required fields, it does not validate the payload's schema or enforce strict type checks.

Attack Vector: A maliciously crafted receipt could exploit deserialization weaknesses to inject unexpected data or cause crashes.

Recommendation:

  • Implement strict schema validation for receipt payloads using a library like jsonschema.
  • Reject receipts with unexpected or malformed fields.

7. Race Conditions

Rating: 🟡 MEDIUM

The CedarPolicyBridge and ReceiptVerifier use threading locks to manage access to shared state (e.g., _history and _verified). While this mitigates some risks, concurrent modifications could still lead to race conditions if the locks are not used consistently.

Attack Vector: An attacker could exploit race conditions to manipulate evaluation history or verification logs, potentially bypassing policy enforcement.

Recommendation:

  • Conduct a thorough review of all threading-related code to ensure proper lock usage.
  • Consider using thread-safe data structures (e.g., queue.Queue) to manage shared state.

8. Supply Chain

Rating: 🔴 CRITICAL

The integration introduces a new dependency on protect-mcp (v0.4.6). While the package is open source and published on npm, it represents a potential supply chain risk.

Attack Vector: An attacker could compromise the protect-mcp package or its dependencies to introduce malicious code.

Recommendation:

  • Pin the protect-mcp dependency to a specific version and verify its integrity using a hash.
  • Regularly audit the protect-mcp package and its dependencies for vulnerabilities.
  • Consider implementing a Software Bill of Materials (SBOM) to track dependencies.

Additional Observations

  1. Issuer-Blind Verification: The use of VOPRF for issuer-blind verification is a strong privacy feature. However, its implementation should be reviewed to ensure compliance with RFC 9497.
  2. Logging and Auditing: The CedarPolicyBridge maintains a history of evaluations, which is useful for auditing. Ensure that this log is protected against tampering and unauthorized access.

Summary of Findings

Category Rating Description Recommendation
Prompt Injection Defense Bypass 🔵 LOW No direct user input handling; low risk. Validate all inputs against schemas.
Policy Engine Circumvention 🟠 HIGH Misconfigured trust_floor could allow bypassing Cedar policies. Add safeguards for trust_floor configuration.
Trust Chain Weaknesses 🔴 CRITICAL Delegated cryptographic verification introduces a critical trust gap. Integrate signature verification directly into the toolkit.
Credential Exposure 🔵 LOW No sensitive credentials handled directly. Ensure logs do not expose sensitive information.
Sandbox Escape 🔵 LOW WASM runtime is inherently sandboxed. Keep the WASM runtime up to date.
Deserialization Attacks 🟡 MEDIUM Receipt deserialization lacks strict schema validation. Enforce strict schema validation for receipt payloads.
Race Conditions 🟡 MEDIUM Potential race conditions in shared state management. Use thread-safe data structures and review lock usage.
Supply Chain 🔴 CRITICAL New dependency (protect-mcp) introduces supply chain risk. Pin dependency versions and perform regular audits.

Final Recommendation

This integration provides significant security benefits, but the critical issues related to trust chain weaknesses and supply chain risks must be addressed before merging. Additionally, safeguards against policy circumvention and deserialization attacks should be implemented to ensure robust security.

@tomjwxf
Copy link
Copy Markdown
Author

tomjwxf commented Apr 2, 2026

@microsoft-github-policy-service agree company="ScopeBlind Pty Ltd"

@tomjwxf
Copy link
Copy Markdown
Author

tomjwxf commented Apr 2, 2026

Thanks for the thorough review. Addressing each point:

🔴 CRITICAL responses:

  1. Receipt Verification Delegation — Agreed. This adapter intentionally validates structure only (schema conformance). Cryptographic verification is delegated to @veritasacta/verify (Apache-2.0, zero ScopeBlind dependency) which handles JCS canonicalization + Ed25519. Adding direct pynacl verification as a fallback is a good suggestion — will add in a follow-up.

  2. Trust Score Manipulation — The trust bonus/penalty is bounded: min(1000, score + bonus) and max(0, score - penalty). An attacker triggering repeated Cedar allows gets capped at 1000 regardless. Will add explicit documentation of the bounds.

  3. Receipt Hash Truncation — The 16-char truncation is a reference ID for tracing, not a security-critical hash. The full SHA-256 is in the signed receipt itself. Will rename the field to receipt_ref to clarify this is a short reference, not a security hash.

  4. Receipt Validation Naming — Good call. Will rename validate_structurevalidate_structure_only with docstring clarifying that cryptographic verification requires @veritasacta/verify or equivalent Ed25519 implementation.

🟡 WARNING responses:

  1. Backward Compatibility — Cedar deny as hard constraint is by design (matches Claude Code's own hook precedence: deny > ask > allow). Will add migration guidance noting that existing AGT workflows using trust-score-only gating are unaffected unless they opt into CedarPolicyBridge.

  2. Cross-Language Dependencyprotect-mcp is the Node.js runtime. This Python adapter is standalone — it contains the bridge logic without requiring protect-mcp as a Python dependency. The adapter consumes protect-mcp's output (receipt JSON), not its runtime.

💡 SUGGESTIONS:

Will address type annotations, thread safety (adding threading.Lock for _history/_decisions), error handling, and edge case tests in a follow-up commit. The documentation diagrams suggestion is excellent — will add an architecture flow diagram to the README.

Updated IETF draft (draft-farley-acta-signed-receipts-01) adds two new receipt types relevant to this integration: protectmcp:lifecycle for agent swarm tracking and scopeblind:spending_authority for issuer-blind financial authorization. protect-mcp is now at v0.5.1.

…ty, edge case tests, full hash

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@tomjwxf
Copy link
Copy Markdown
Author

tomjwxf commented Apr 2, 2026

Follow-up commit addressing all code review items:

  1. validate_structurevalidate_structure_only — renamed with expanded docstring clarifying no cryptographic verification
  2. Thread safetythreading.Lock added to _history, _verified, _decisions with all operations wrapped in lock context managers
  3. receipt_hashreceipt_ref — now uses full SHA-256 .hexdigest() (64 chars, no truncation)
  4. 20 edge case tests — empty/malformed receipts, boundary trust scores (0, 1000, exactly at floor), concurrent access (10 threads × 50 ops), malformed payloads

56 tests passing (36 original + 20 edge cases).

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

🤖 AI Agent: code-reviewer

Code Review Feedback for feat: ScopeBlind protect-mcp integration


🔴 CRITICAL: Security Issues

  1. Receipt Validation Delegation

    • The ReceiptVerifier class delegates cryptographic signature verification to external tools (@veritasacta/verify or protect-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).
  2. Thread Safety in ReceiptVerifier

    • The _verified list in ReceiptVerifier is 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.Queue or collections.deque with maxlen for managing the _verified log.
  3. SpendingGate Utilization Band Validation

    • The evaluate_spend method in SpendingGate does not validate the utilization_band input against the UTILIZATION_BANDS set. This could allow invalid or malicious inputs to bypass checks.
    • Recommendation: Add explicit validation for utilization_band to ensure it is one of the allowed values (low, medium, high, exceeded).
  4. CedarPolicyBridge Trust Adjustment

    • The CedarPolicyBridge class adjusts trust scores (trust_bonus and deny_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_trust to ensure it remains within [0, 1000].

🟡 WARNING: Potential Breaking Changes

  1. Hard Dependency on protect-mcp

    • The integration assumes that protect-mcp is always available and functioning correctly. If protect-mcp introduces breaking changes or is unavailable, this integration will fail.
    • Recommendation: Add version pinning for protect-mcp in the documentation and provide fallback mechanisms or error handling for cases where protect-mcp is unavailable.
  2. Cedar Deny as Authoritative

    • The design enforces that a Cedar deny decision 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.

💡 Suggestions for Improvement

  1. Type Annotations

    • While the code includes type hints, some areas lack full type coverage. For example, the receipt parameter in ReceiptVerifier.validate_structure_only is typed as Dict[str, Any], but its structure is well-defined.
    • Recommendation: Use TypedDict or Pydantic models to define the expected structure of receipts for better type safety and validation.
  2. Pydantic Model Validation

    • The CedarDecision and 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 dataclass with Pydantic models where applicable.
  3. 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.
  4. 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.
  5. Backward Compatibility

    • The integration introduces new components but does not modify existing ones. However, the reliance on protect-mcp and the authoritative nature of Cedar deny decisions could indirectly affect existing workflows.
    • Recommendation: Provide a migration guide for users to adopt this integration without disrupting existing setups.
  6. Documentation

    • The documentation is comprehensive but could benefit from additional examples, such as how to handle errors when protect-mcp fails or how to configure the CedarPolicyBridge for different trust models.
    • Recommendation: Expand the README with advanced usage examples and troubleshooting tips.

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!

@tomjwxf
Copy link
Copy Markdown
Author

tomjwxf commented Apr 2, 2026

@imran-siddique — all items from both code reviews are addressed in commit 9a3be8a. Quick summary:

Issue Status
validate_structure naming ✅ Renamed to validate_structure_only with expanded docstring
Thread safety threading.Lock on all mutable state
Receipt hash truncation receipt_ref with full SHA-256 (64 chars)
Trust score bounds ✅ Already bounded: min(1000, ...) / max(0, ...) — now documented
Utilization band validation ✅ Already validated (line 401: if utilization_band not in self.UTILIZATION_BANDS)
Edge case tests ✅ 20 new tests (malformed receipts, boundary scores, 10-thread concurrency)

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.

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

Labels

size/XL Extra large PR (500+ lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants