feat: offline-verifiable decision receipts (Ed25519 + JCS)

[Prashan Sapkota (prashansapkota)]

Summary

Implements offline-verifiable decision receipts for the existing mcp-receipt-governed adapter.

Changes

File	Change
mcp_receipt_governed/receipt.py	Added parent_receipt_hash for hash chaining, RFC 8785 JCS compliance, to_slsa_provenance(), and verify_receipt_chain()
mcp_receipt_governed/adapter.py	Auto-threads parent_receipt_hash from the last stored receipt
mcp_receipt_governed/__init__.py	Exports verify_receipt_chain
tests/test_receipt.py	45 tests (+25 new) covering JCS, hash chaining, SLSA, chain verification, and unsigned receipt detection
scripts/verify_receipts.py	[NEW] Pure Python offline CLI verifier
docs/tutorials/33-offline-verifiable-receipts.md	[NEW] Tutorial covering receipts, chaining, verification, and SLSA

Standards

Standard	Usage
RFC 8032 (Ed25519)	Receipt signing and verification
RFC 8785 (JCS)	Canonical JSON before hashing
SLSA Provenance v1	Optional provenance predicate emission

Testing

• 65 tests passing (20 adapter + 45 receipt)
• black formatting compliant

Closes #1499

[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

Pull Request Review: Offline-Verifiable Decision Receipts (Ed25519 + JCS)

---

🔴 CRITICAL: Security Issues

1. Replay Attack Vector in Hash Chaining:

   • The parent_receipt_hash mechanism ensures hash-chain integrity but does not prevent replay attacks. An attacker could replay a valid receipt chain in a different context without detection.
   • Recommendation: Include a session-specific nonce or identifier in the receipt payload to bind the chain to a specific execution context.

2. Unsigned Receipts:

   • The verify_receipt_chain function flags unsigned receipts but does not enforce fail-closed behavior. Unsigned receipts could bypass verification in certain scenarios.
   • Recommendation: Enforce fail-closed behavior by rejecting chains with unsigned receipts outright. Update the verify_receipt_chain function to raise an exception or return a critical error code when encountering unsigned receipts.

3. Signature Verification:

   • The verify_receipt function does not validate the authenticity of the signer_public_key. An attacker could replace the signer_public_key in a receipt with their own key and provide a valid signature.
   • Recommendation: Implement a mechanism to validate the signer_public_key against a trusted source (e.g., a certificate authority or a pre-shared key registry).

4. Canonicalization and Unicode Handling:

   • While the implementation adheres to RFC 8785 for JSON canonicalization, the use of ensure_ascii=False could introduce vulnerabilities if the Unicode handling is not thoroughly tested against edge cases (e.g., surrogate pairs, invalid UTF-8 sequences).
   • Recommendation: Add tests for edge cases in Unicode handling to ensure compliance with RFC 8785 and prevent potential encoding-related attacks.

---

🟡 WARNING: Breaking Changes

1. Public API Changes:
   • The addition of verify_receipt_chain to the public API (__init__.py) is a breaking change for users who rely on the previous API structure. While this change is beneficial, it should be clearly documented in the release notes.

---

💡 Suggestions for Improvement

1. Thread Safety:

   • The ReceiptStore class uses a list (self._receipts) to store receipts, but there is no explicit synchronization mechanism for concurrent access. This could lead to race conditions in multi-threaded environments.
   • Recommendation: Use thread-safe data structures like queue.Queue or add locks to ensure thread-safe operations on self._receipts.

2. Error Reporting:

   • The verify_receipt_chain function returns a list of error strings. While this is useful for debugging, it may be better to raise structured exceptions or return a structured object containing both errors and metadata about the verification process.
   • Recommendation: Refactor verify_receipt_chain to return a structured result object or raise specific exceptions for better integration into automated workflows.

3. Documentation:

   • The new tutorial (33-offline-verifiable-receipts.md) is a great addition, but it should include a section on the security implications of using the receipt chain, especially regarding replay attacks and unsigned receipts.
   • Recommendation: Expand the tutorial to include best practices for secure receipt handling and verification.

4. Backward Compatibility:

   • While the changes appear to be backward-compatible, the addition of parent_receipt_hash to the GovernanceReceipt class may impact users who rely on the previous receipt structure.
   • Recommendation: Document this change in the release notes and provide migration guidance for users.

5. CLI Tool Enhancements:

   • The verify_receipts.py script provides useful functionality but lacks options for detailed output (e.g., JSON-formatted results for integration into CI/CD pipelines).
   • Recommendation: Add an option to output verification results in JSON format for easier integration into automated workflows.

6. Test Coverage:

   • While the test coverage is extensive, consider adding tests for edge cases in Unicode handling and malformed receipt chains (e.g., cyclic chains, missing receipts).
   • Recommendation: Expand test cases to cover edge scenarios and ensure robustness.

---

Summary

The pull request introduces a robust mechanism for offline-verifiable decision receipts, leveraging Ed25519 signatures, RFC 8785 JCS canonicalization, and hash chaining. While the implementation is well-structured and adheres to relevant standards, there are critical security concerns related to replay attacks, unsigned receipts, and signature verification. Addressing these issues is essential to ensure the security and reliability of the receipt mechanism.

Action Items:

1. Implement session-specific nonce or identifier to prevent replay attacks.
2. Enforce fail-closed behavior for unsigned receipts in verify_receipt_chain.
3. Validate signer_public_key against a trusted source during signature verification.
4. Add tests for Unicode edge cases and malformed receipt chains.
5. Document breaking changes and provide migration guidance.
6. Enhance the CLI tool with JSON output for CI/CD integration.

[github-actions]

🤖 AI Agent: contributor-guide — Welcome to the Microsoft Agent Governance Toolkit project! 👋

Welcome to the Microsoft Agent Governance Toolkit project! 👋

Hi there, and welcome to the community! 🎉 Thank you so much for taking the time to contribute to the project. We’re thrilled to have you here, and we appreciate the effort you’ve put into this pull request. It’s always exciting to see new contributors join the project and bring fresh ideas to the table.

---

What You Did Well 🌟

1. Comprehensive Implementation: Your PR introduces a robust feature for offline-verifiable decision receipts, complete with hash chaining, JCS compliance, and SLSA provenance support. This is a fantastic addition to the mcp-receipt-governed adapter.

2. Thorough Testing: Adding 25 new tests to cover various aspects of the feature, including edge cases like unsigned receipt detection and hash chaining, is excellent. The detailed test cases for JCS compliance and SLSA provenance are particularly impressive.

3. Documentation: Including a new tutorial (docs/tutorials/33-offline-verifiable-receipts.md) is a great way to help users understand and adopt this feature. The level of detail and clarity is commendable.

4. Clean Code: The code is well-structured, and the use of type hints and docstrings makes it easy to follow. The adherence to black formatting is also appreciated.

---

Suggestions for Improvement 🛠️

While this is a strong contribution, there are a few areas where we can make it even better:

1. Linting with Ruff:

   • We use Ruff for linting with specific rules (E, F, W). Please run ruff . --fix to ensure your code adheres to the project's linting standards. This will help catch any minor issues like unused imports or formatting inconsistencies.

2. Commit Message Style:

   • We follow the Conventional Commits standard for commit messages. Your PR title is great (feat: offline-verifiable decision receipts (Ed25519 + JCS)), but please ensure all individual commit messages in this PR follow the same format (e.g., feat: add hash chaining to GovernanceReceipt or test: add tests for hash chaining).

3. Security Considerations:

   • Since this PR introduces security-sensitive features (e.g., Ed25519 signature verification and hash chaining), we’ll need to perform a detailed security review. This is a standard practice for any cryptographic or security-related code in the project. While your implementation looks solid, we’ll double-check for potential edge cases or vulnerabilities.

4. Documentation Updates:

   • The new tutorial is excellent, but it would be helpful to add a reference to it in the docs/QUICKSTART.md file. This will make it easier for users to discover the new feature.

5. Testing Edge Cases:

   • While your tests are comprehensive, consider adding a test for a scenario where the parent_receipt_hash is tampered with in the middle of the chain. This would ensure that the verify_receipt_chain function catches such issues.

---

Next Steps 🚀

1. Run ruff . --fix to address any linting issues.
2. Review your commit messages and update them to follow the Conventional Commits format.
3. Address the feedback above, especially adding a reference to the new tutorial in the QUICKSTART.md file.
4. Once you’ve made the updates, push your changes to this branch, and the CI/CD pipeline will automatically re-run.
5. After the CI checks pass and the review is complete, we’ll proceed with merging your PR.

---

Helpful Resources 📚

• CONTRIBUTING.md: Guidelines for contributing to the project.
• docs/QUICKSTART.md: Quickstart guide for using the toolkit.
• Ruff Documentation: Learn more about our linter and its rules.
• Conventional Commits: Understand the commit message format we use.

---

Thank you again for your contribution! If you have any questions or need further clarification, don’t hesitate to ask. We’re here to help and support you through the process. 😊

[github-actions]

🤖 AI Agent: security-scanner — Security Review of PR: feat: offline-verifiable decision receipts (Ed25519 + JCS)

Security Review of PR: feat: offline-verifiable decision receipts (Ed25519 + JCS)

This PR introduces offline-verifiable decision receipts using Ed25519 signatures, RFC 8785 JSON Canonicalization Scheme (JCS), and optional SLSA provenance predicates. It also adds a hash-chaining mechanism for receipts to ensure integrity and detect tampering. Below is a detailed security review of the changes.

---

Findings

1. Prompt Injection Defense Bypass

• Rating: 🔵 LOW
• Analysis: No direct user input is being processed for natural language understanding or generation in this PR. The changes focus on cryptographic signing, verification, and hash chaining, which are not directly susceptible to prompt injection.
• Recommendation: No action required.

---

2. Policy Engine Circumvention

• Rating: 🟠 HIGH
• Issue: The CedarPolicyEvaluator class includes an inline fallback mechanism for policy evaluation when the agentmesh.governance.cedar module is unavailable. This fallback uses regular expressions to parse and evaluate policies, which is error-prone and may lead to incorrect policy evaluations. For example:
  • The regex-based approach may fail to correctly parse complex policies or policies with nested conditions.
  • The fallback implementation does not validate the context parameter, which could lead to incorrect policy decisions if the context is malformed or incomplete.
• Attack Vector: An attacker could exploit this fallback mechanism by crafting a policy with complex or unexpected syntax that bypasses the inline evaluator, potentially allowing unauthorized actions.
• Recommendation: Remove the inline fallback mechanism and enforce the use of a robust, well-tested policy evaluation library like agentmesh.governance.cedar. If a fallback is necessary, it should be implemented with a more robust parser or explicitly documented as a less secure option.

---

3. Trust Chain Weaknesses

• Rating: 🔴 CRITICAL
• Issue: The verify_receipt_chain function does not enforce the use of trusted keys by default. If trusted_keys is not provided, the function will verify signatures without ensuring they are from a trusted source. This could allow an attacker to inject malicious receipts with valid signatures from untrusted keys.
• Attack Vector: An attacker could generate their own Ed25519 key pair, sign malicious receipts, and inject them into the receipt chain. Without verifying the key against a trusted list, the system would accept these receipts as valid.
• Recommendation: Make trusted_keys a required parameter for verify_receipt_chain. If no trusted keys are provided, the function should raise an error or log a critical warning. Additionally, ensure that the trusted_keys list is securely managed and not hardcoded in the source code.

---

4. Credential Exposure

• Rating: 🟡 MEDIUM
• Issue: The sign_receipt function logs an error message if signing fails, including the exception type. While this does not directly expose sensitive information, it could potentially leak implementation details or be used for reconnaissance by an attacker.
• Attack Vector: An attacker could trigger signing errors and analyze the error messages to gather information about the system's cryptographic implementation or other internal details.
• Recommendation: Avoid logging sensitive information in error messages. Use generic error messages and log detailed errors only in secure, internal logs.

---

5. Sandbox Escape

• Rating: 🔵 LOW
• Analysis: This PR does not introduce any new functionality that interacts with the operating system or external processes, so the risk of a sandbox escape is minimal.
• Recommendation: No action required.

---

6. Deserialization Attacks

• Rating: 🟡 MEDIUM
• Issue: The hash_tool_args function uses json.dumps to serialize tool arguments into canonical JSON. While this is generally safe, there is no validation of the input data. Maliciously crafted input could potentially exploit vulnerabilities in the JSON library or lead to unexpected behavior.
• Attack Vector: An attacker could provide malicious input that exploits a vulnerability in the JSON serialization process, potentially leading to denial-of-service or other attacks.
• Recommendation: Validate tool_args before serialization to ensure it conforms to expected types and structures. Consider using a JSON schema validation library to enforce strict input validation.

---

7. Race Conditions

• Rating: 🟡 MEDIUM
• Issue: The McpReceiptAdapter.govern_tool_call method uses a lock (self.store._lock) to ensure thread safety when accessing the ReceiptStore. However, the lock is not used consistently across all methods that interact with the ReceiptStore, such as get_receipts and get_stats.
• Attack Vector: Concurrent access to the ReceiptStore without proper locking could lead to race conditions, resulting in inconsistent or corrupted receipt data.
• Recommendation: Ensure that all methods interacting with ReceiptStore use the same locking mechanism to guarantee thread safety.

---

8. Supply Chain

• Rating: 🟠 HIGH
• Issue: The PR relies on the cryptography library for Ed25519 signature generation and verification. While cryptography is a widely used and well-regarded library, it is crucial to ensure that the dependency is pinned to a specific version to prevent supply chain attacks or compatibility issues.
• Attack Vector: An attacker could compromise the cryptography package in the supply chain, introducing malicious code that compromises the security of the receipts.
• Recommendation: Pin the cryptography dependency to a specific version in the requirements.txt file or equivalent dependency management file. Regularly review and update dependencies to include security patches.

---

Summary of Findings

Category	Rating	Issue	Recommendation
Prompt Injection Defense Bypass	🔵 LOW	No prompt injection vulnerabilities identified.	No action required.
Policy Engine Circumvention	🟠 HIGH	Inline policy evaluator is error-prone and insecure.	Remove inline evaluator or replace with a robust parser.
Trust Chain Weaknesses	🔴 CRITICAL	verify_receipt_chain does not enforce trusted keys by default.	Require trusted_keys parameter and validate against it.
Credential Exposure	🟡 MEDIUM	Error messages in sign_receipt may leak sensitive implementation info.	Avoid logging sensitive details; use generic error messages.
Sandbox Escape	🔵 LOW	No new sandbox escape risks introduced.	No action required.
Deserialization Attacks	🟡 MEDIUM	Lack of validation for tool_args before JSON serialization.	Validate tool_args using a JSON schema or similar validation mechanism.
Race Conditions	🟡 MEDIUM	Inconsistent use of locks in ReceiptStore methods.	Ensure all ReceiptStore interactions are thread-safe by using the same locking mechanism.
Supply Chain	🟠 HIGH	cryptography dependency is not pinned to a specific version.	Pin cryptography to a specific version and regularly update dependencies to include security patches.

---

General Recommendations

1. Testing: Expand test coverage to include edge cases for the inline policy evaluator and receipt chain verification.
2. Documentation: Clearly document the security guarantees and limitations of the receipt system, especially around the use of trusted keys and the inline policy evaluator.
3. Code Comments: Add comments to clarify the rationale behind critical security decisions, such as the use of hash chaining and the structure of the canonical payload.

This PR introduces critical functionality for offline-verifiable decision receipts, which is a significant improvement for auditability and security. However, the identified issues, particularly around policy evaluation and trust chain validation, must be addressed to ensure the robustness of the implementation.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: feat: offline-verifiable decision receipts (Ed25519 + JCS)

---

🔴 CRITICAL: Security Issues

1. Missing Replay Attack Mitigation:

   • The verify_receipt_chain() function does not validate the uniqueness of receipt_id across the chain. This omission could allow replay attacks where a malicious actor reuses a valid receipt from a previous session.
   • Action: Add a check to ensure all receipt_id values in the chain are unique.

2. Untrusted Key Handling:

   • In verify_receipt_chain(), receipts signed by untrusted keys are flagged but not explicitly rejected. This could lead to ambiguous behavior in downstream systems relying on the verification result.
   • Action: Ensure that receipts signed by untrusted keys are treated as invalid and explicitly fail the chain verification.

3. Fail-Closed Enforcement:

   • The govern_tool_call() method enforces fail-closed behavior when receipt signing fails. However, the error message (receipt.error = f"signing_failed: {type(exc).__name__}") is not propagated to the caller, which could lead to silent failures.
   • Action: Raise an exception or return a clear error code when signing fails.

4. Canonicalization Ambiguity:

   • The canonical_payload() method uses ensure_ascii=False to comply with RFC 8785, but this behavior is not explicitly tested for edge cases like surrogate pairs or invalid Unicode sequences.
   • Action: Add tests for edge cases in Unicode handling to ensure compliance with RFC 8785.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes:

   • The addition of parent_receipt_hash and session_id to GovernanceReceipt changes the canonical payload structure. This could break compatibility with systems relying on the previous payload format.
   • Action: Provide clear migration guidance in the documentation and consider versioning the canonical payload schema.

2. Exported Functionality:

   • The new verify_receipt_chain() function is exported via mcp_receipt_governed/__init__.py. If downstream consumers rely on the previous set of exports, this could cause compatibility issues.
   • Action: Ensure backward compatibility by documenting the new function and its intended use.

---

💡 Suggestions for Improvement

1. Thread Safety:

   • The ReceiptStore class is not thread-safe, which could lead to race conditions in concurrent environments. For example, self.store.add(receipt) in govern_tool_call() assumes exclusive access to the store.
   • Action: Use thread-safe mechanisms like locks or concurrent data structures for ReceiptStore.

2. SLSA Provenance Validation:

   • While the to_slsa_provenance() method generates a valid SLSA predicate, there is no test to validate the generated output against the SLSA schema.
   • Action: Add schema validation tests for the SLSA predicate.

3. CLI Tool Output:

   • The verify_receipts.py script could benefit from more structured output, such as JSON logs for easier integration with CI/CD pipelines.
   • Action: Add a --json flag to output structured results.

4. Error Handling in CLI:

   • The verify_receipts.py script prints error messages directly to the console but does not return detailed error codes for CI/CD integration.
   • Action: Implement detailed error codes for different failure scenarios (e.g., signature failure, hash chain breakage).

5. Test Coverage:

   • While the test suite is extensive, it could benefit from additional tests for edge cases, such as:
     • Receipts with missing fields.
     • Invalid Unicode sequences in canonical_payload().
     • Large chains with thousands of receipts.
   • Action: Expand test coverage to include these cases.

6. Logging:

   • The logging in govern_tool_call() and verify_receipts.py could be improved by including more context, such as the session_id and receipt_id.
   • Action: Enhance logging to include additional metadata for easier debugging.

7. Documentation:

   • The new tutorial (33-offline-verifiable-receipts.md) is comprehensive but could benefit from a section on common errors and troubleshooting tips.
   • Action: Add a troubleshooting section to the tutorial.

---

Summary

This PR introduces a robust mechanism for offline-verifiable decision receipts, leveraging Ed25519 signatures, RFC 8785 JCS canonicalization, and hash chaining. While the implementation is generally solid, there are critical security issues that need to be addressed, particularly around replay attack mitigation and untrusted key handling. Additionally, potential breaking changes should be carefully documented, and thread safety should be improved for concurrent environments.

Priority Actions:

• Address replay attack mitigation and untrusted key handling (🔴 CRITICAL).
• Document migration paths for canonical payload changes (🟡 WARNING).
• Improve thread safety in ReceiptStore (💡 SUGGESTION).

Once these issues are resolved, the feature will be ready for production deployment.

[github-actions]

🤖 AI Agent: code-reviewer

🔴 CRITICAL: Security Issues

1. Missing Replay Attack Mitigation in verify_receipt_chain:

   • The verify_receipt_chain function checks for duplicate receipt_id values but does not enforce uniqueness in the ReceiptStore itself. This leaves the system vulnerable to replay attacks if duplicate receipts are injected into the store before verification.
   • Recommendation: Enforce uniqueness of receipt_id in the ReceiptStore.add() method by checking for duplicates before appending to _receipts.

2. Untrusted Receipt Signing Key Handling:

   • The verify_receipt_chain function allows unsigned receipts to pass with only a warning (⚠️ Unsigned receipt (no signature)), which could lead to security bypasses if an attacker inserts unsigned receipts into the chain.
   • Recommendation: Treat unsigned receipts as a critical error and fail verification. Update the verify_receipt_chain function to enforce that all receipts must be signed.

3. Weak Error Handling in Receipt Signing:

   • The sign_receipt function raises a generic RuntimeError when signing fails. This could result in unclear error messages and make debugging difficult.
   • Recommendation: Use a custom exception (e.g., ReceiptSigningError) to provide more context about the failure.

4. Potential Key Spoofing in verify_receipt_chain:

   • The verify_receipt_chain function checks the signer_public_key against a trusted set but does not validate the format of the key. This could allow an attacker to inject malformed keys that bypass the check.
   • Recommendation: Validate the format of signer_public_key (e.g., length and hex encoding) before comparing it to the trusted set.

---

🟡 WARNING: Potential Breaking Changes

1. Fail-Closed Enforcement in govern_tool_call:
   • The updated govern_tool_call method raises a RuntimeError if receipt signing fails. This is a change from the previous behavior, where the error was logged but execution continued.
   • Impact: This change may break existing integrations that rely on the previous behavior.
   • Recommendation: Document this change clearly in the release notes and consider providing a configuration option to toggle between fail-closed and fail-open behavior.

---

💡 Suggestions for Improvement

1. Thread Safety in ReceiptStore:

   • While the ReceiptStore uses a threading.Lock for thread safety, the query method creates a snapshot of _receipts and then performs filtering outside the lock. This could lead to race conditions if the store is modified concurrently.
   • Recommendation: Perform filtering directly within the locked section to ensure consistency.

2. Error Reporting in verify_receipt_chain:

   • The error messages returned by verify_receipt_chain are human-readable but lack structured metadata for programmatic consumption.
   • Recommendation: Include structured metadata (e.g., error codes) in the returned error list to facilitate automated analysis.

3. Improved Logging in verify_receipts.py:

   • The CLI tool logs verification results but does not provide detailed logs for debugging failed checks.
   • Recommendation: Add an optional --verbose flag to include detailed logs for each verification step.

4. Backward Compatibility for Unsigned Receipts:

   • The stricter enforcement of signed receipts may break compatibility with older systems that emit unsigned receipts.
   • Recommendation: Provide a configuration option to allow unsigned receipts for backward compatibility, with a clear warning about the security risks.

5. Unit Test Coverage for Edge Cases:

   • While the tests cover many scenarios, edge cases like malformed JSON, invalid receipt fields, and corrupted signatures are not explicitly tested.
   • Recommendation: Add unit tests for these edge cases to ensure robust handling.

6. Documentation Enhancements:

   • The tutorial (docs/tutorials/33-offline-verifiable-receipts.md) does not cover the implications of hash chaining and signature verification in detail.
   • Recommendation: Expand the tutorial to include security considerations, such as replay attack prevention and trusted key management.

---

Summary

This PR introduces significant improvements to the mcp-receipt-governed adapter, including offline-verifiable decision receipts with hash chaining, RFC 8785 JCS compliance, and SLSA provenance support. However, there are critical security issues related to replay attack mitigation, unsigned receipt handling, and key validation that must be addressed before merging. Additionally, the fail-closed behavior and stricter signature enforcement may introduce backward compatibility concerns.

[github-actions]

🤖 AI Agent: code-reviewer

Pull Request Review: Offline-Verifiable Decision Receipts

This pull request introduces offline-verifiable decision receipts with hash chaining, RFC 8785 JCS compliance, and optional SLSA provenance support. Below is a detailed review of the changes:

---

🔴 CRITICAL: Security Issues

1. Signature Verification Fallback:

   • The verify_receipt function logs an error and returns False if the cryptography library is not available. This behavior could lead to silent failures in environments where the library is missing, potentially allowing unsigned or invalid receipts to pass unnoticed.
   • Actionable Recommendation: Raise an exception if the cryptography library is unavailable, as signature verification is critical for security.

2. Receipt Signing Error Handling:

   • In McpReceiptAdapter.govern_tool_call, a RuntimeError is raised if receipt signing fails. While this is a fail-closed approach, it would be safer to log the error and include the failed receipt in the store with an explicit error field. This ensures that the failure is auditable.
   • Actionable Recommendation: Store the receipt with an error field indicating the signing failure, and log the error for debugging purposes.

3. Trusted Key Validation:

   • The verify_receipt_chain function optionally checks signer keys against a trusted_keys list. However, the absence of trusted_keys allows any signer to be trusted, which could lead to security bypasses in environments where key validation is expected.
   • Actionable Recommendation: Require trusted_keys to be explicitly provided and fail validation if it is None.

4. Thread Safety:

   • The ReceiptStore class uses a threading lock (self._lock) for concurrent access. However, the lock is only applied in the McpReceiptAdapter.govern_tool_call method, not in other methods like add or query. This could lead to race conditions in multi-threaded environments.
   • Actionable Recommendation: Ensure all methods that modify or access shared state in ReceiptStore use the threading lock.

---

🟡 WARNING: Potential Breaking Changes

1. Public API Changes:

   • The addition of parent_receipt_hash and session_id to GovernanceReceipt changes the canonical payload structure. This could break compatibility with existing systems that rely on the previous payload format.
   • Actionable Recommendation: Clearly document this change in the release notes and provide migration guidance for downstream users.

2. SLSA Provenance Emission:

   • The new to_slsa_provenance method introduces a dependency on the SLSA specification. If downstream systems are not prepared to handle this format, it could cause integration issues.
   • Actionable Recommendation: Make SLSA emission optional and clearly document its usage.

---

💡 Suggestions for Improvement

1. Error Reporting in verify_receipt_chain:

   • The error messages returned by verify_receipt_chain could include more context, such as the receipt ID and session ID, to aid debugging.
   • Suggestion: Enhance error messages with additional context.

2. Canonical JSON Serialization:

   • The canonical_payload method uses ensure_ascii=False for UTF-8 compliance. Consider adding a test case to verify that non-ASCII characters are correctly serialized and hashed.
   • Suggestion: Add tests for non-ASCII characters in receipt fields.

3. Logging Enhancements:

   • The logging in methods like sign_receipt and verify_receipt could include more details, such as the receipt ID and signer public key, to improve traceability.
   • Suggestion: Enhance logging with receipt-specific details.

4. Documentation:

   • The new verify_receipt_chain function is a critical addition but lacks detailed documentation in the tutorial. Include examples of its usage and explain the implications of trusted key validation.
   • Suggestion: Update the tutorial to cover verify_receipt_chain in detail.

5. Test Coverage:

   • While the pull request adds 25 new tests, ensure edge cases are covered, such as:
     • Receipts with missing fields (e.g., parent_receipt_hash).
     • Invalid UTF-8 characters in receipt fields.
     • Large receipt chains to test performance and scalability.
   • Suggestion: Expand test coverage for edge cases.

---

Summary

This pull request significantly enhances the security and verifiability of governance receipts. However, it introduces critical security risks related to signature verification fallback, trusted key validation, and thread safety. Addressing these issues is essential to ensure the robustness of the implementation. Additionally, potential breaking changes should be documented, and improvements to error reporting, logging, and test coverage are recommended.

Overall Rating: Needs improvement before merging.

[github-actions]

🤖 AI Agent: code-reviewer

Review Summary

This pull request introduces offline-verifiable decision receipts with Ed25519 signatures and RFC 8785 JCS canonical JSON compliance. It also adds hash chaining for receipt integrity and optional SLSA provenance emission. The implementation appears well-thought-out and includes extensive testing and documentation. However, there are critical security concerns, potential breaking changes, and areas for improvement.

---

🔴 CRITICAL: Security Issues

1. Receipt Signing Failures Are Not Fail-Safe

   • In McpReceiptAdapter.govern_tool_call, if sign_receipt fails, the error is logged, but the receipt is still added to the store. This creates a potential security bypass where unsigned receipts are stored and could be mistaken for valid ones later.
     Recommendation: Do not add unsigned receipts to the store. Raise an exception and fail the tool call entirely.

   if self._signing_key:
       try:
           sign_receipt(receipt, self._signing_key)
       except Exception as exc:
           _logger.error(f"Receipt signing failed: {type(exc).__name__}")
           raise ReceiptSigningError(
               f"Receipt signing failed for tool={tool_name}: {type(exc).__name__}: {exc}"
           ) from exc

2. No Validation of Trusted Public Keys in verify_receipt_chain

   • The verify_receipt_chain function accepts trusted_keys but does not enforce that all receipts are signed by a trusted key. This allows an attacker to inject receipts signed by untrusted keys into the chain.
     Recommendation: Add a check to ensure that each receipt's signer_public_key is in the trusted_keys set if provided.

   if trusted_set and r.signer_public_key not in trusted_set:
       errors.append(f"[{i}] Receipt signed by untrusted key: {r.signer_public_key}")

3. Potential Replay Attack in Receipt Chain Verification

   • The verify_receipt_chain function checks for duplicate receipt_id but does not validate the uniqueness of parent_receipt_hash across the chain. This could allow an attacker to replay a receipt chain by reusing parent hashes.
     Recommendation: Add a check to ensure that parent_receipt_hash values are unique across the chain.

   seen_hashes: set = set()
   for i, r in enumerate(receipts):
       if r.parent_receipt_hash in seen_hashes:
           errors.append(f"[{i}] Duplicate parent_receipt_hash {r.parent_receipt_hash} — possible replay attack")
       seen_hashes.add(r.parent_receipt_hash)

---

🟡 WARNING: Potential Breaking Changes

1. ReceiptSigningError Exception

   • The introduction of the ReceiptSigningError exception changes the behavior of McpReceiptAdapter.govern_tool_call. Previously, signing failures did not raise exceptions, but now they do. This could break existing code that relies on the method not raising exceptions.
     Recommendation: Document this change clearly in the release notes and consider providing a backward-compatible mode.

2. Changes to GovernanceReceipt Schema

   • The addition of parent_receipt_hash and session_id fields changes the schema of GovernanceReceipt. This could break downstream consumers of the to_dict() method or serialized receipts.
     Recommendation: Document these changes clearly in the release notes and provide migration guidance for downstream users.

---

💡 Suggestions for Improvement

1. Thread Safety in ReceiptStore

   • While ReceiptStore uses a threading lock (self._lock), the lock is not used consistently across all methods. For example, query() and get_stats() do not acquire the lock before accessing self._receipts.
     Recommendation: Ensure all methods that access or modify self._receipts acquire the lock.

   def query(self) -> List[GovernanceReceipt]:
       with self._lock:
           return list(self._receipts)

2. Error Handling in verify_receipt

   • The verify_receipt function catches all exceptions but does not log the error details. This could make debugging difficult.
     Recommendation: Log the exception details before returning False.

   except Exception as exc:
       _logger.error(f"Receipt verification failed: {type(exc).__name__}: {exc}")
       return False

3. Improve Documentation for verify_receipt_chain

   • The function's docstring is incomplete and does not explain the security implications of hash chaining or trusted key validation.
     Recommendation: Expand the docstring to include details about the security checks performed and their importance.

4. Canonical JSON Serialization

   • The canonical_payload method uses json.dumps with ensure_ascii=False, which is correct for RFC 8785 compliance. However, consider adding a comment explicitly referencing RFC 8785 §3.2.2.2 for clarity.

5. SLSA Provenance Emission

   • The to_slsa_provenance method is a valuable addition, but it assumes specific semantics for resolvedDependencies. Consider documenting these assumptions in the method docstring.

---

Conclusion

The feature is well-implemented and aligns with security best practices, but the identified critical issues must be addressed before merging. Additionally, the potential breaking changes should be documented, and the suggested improvements can enhance the robustness and clarity of the implementation.

[Imran Siddique (imran-siddique)]

Offline-verifiable receipts with hash chaining, JCS compliance, SLSA provenance, and CLI verifier. 65 tests. Closes #1499.