feat: add bilateral receipt signing (pre-execution + post-execution)#1333
Conversation
🤖 AI Agent: security-scanner — Security Review of `feat: add bilateral receipt signing (pre-execution + post-execution)`Security Review of
|
| Category | Severity | Issue | Fix |
|---|---|---|---|
| Prompt Injection Defense Bypass | 🔵 LOW | Not applicable. | None |
| Policy Engine Circumvention | 🟡 MEDIUM | Lack of strict validation for payload. |
Add schema validation for payload. |
| Trust Chain Weaknesses | 🟠 HIGH | signerKeyId is not validated. |
Validate signerKeyId against a trusted key registry or KMS. |
| Credential Exposure | 🔵 LOW | No credentials exposed. | None |
| Sandbox Escape | 🔵 LOW | Not applicable. | None |
| Deserialization Attacks | 🟡 MEDIUM | Potential risks in _canonicalize() implementation. |
Ensure secure JSON serialization and review _canonicalize() for vulnerabilities. |
| Race Conditions | 🟡 MEDIUM | Potential tampering of authorization_hash between signing and sealing. |
Use locking or secure storage to protect the envelope between steps. |
| Supply Chain Risks | 🟡 MEDIUM | Cryptographic libraries and custom functions not explicitly reviewed. | Ensure all libraries are up-to-date and review custom functions for security vulnerabilities. |
| Replay Attacks | 🟡 MEDIUM | No mechanism to prevent replay attacks. | Add nonce or timestamp validation to ensure receipt uniqueness. |
Final Recommendation:
The PR introduces a critical feature for compliance and verifiability, but there are several medium and high-severity issues that need to be addressed before merging. Specifically:
- Add schema validation for
payloadinsign_authorization(). - Validate
signerKeyIdagainst a trusted key registry or KMS. - Review
_canonicalize()and_b64urlfor security vulnerabilities. - Implement mechanisms to prevent replay attacks.
- Add error handling and logging for failed verifications.
Once these issues are resolved, the PR can be considered for merging.
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Review Summary
This PR introduces a significant enhancement to the receipts.py module by adding support for bilateral receipt signing, which includes pre-execution authorization (sign_authorization) and post-execution result sealing (seal_result). Additionally, it provides a verification mechanism (verify_bilateral_receipt) to validate both the authorization and result signatures. The implementation appears to be backward compatible, as it does not modify the existing single-signature receipt verification logic.
The changes are well-documented, with a detailed proposal included in the PR. The new functionality is critical for ensuring compliance with regulations like the EU AI Act and SOC 2, as it provides independently verifiable compliance evidence.
🔴 CRITICAL
-
Replay Attack Vulnerability in Authorization Signature:
- The
sign_authorizationfunction does not include a unique nonce or identifier in the payload before signing. This could allow an attacker to reuse a valid authorization signature in a different context, leading to a potential security bypass. - Recommendation: Include a unique, cryptographically secure nonce in the
payloadbefore signing. This ensures that each authorization signature is unique and cannot be reused in a replay attack.
- The
-
Lack of Expiry for Authorization Receipts:
- The
sign_authorizationfunction does not enforce an expiration time for the authorization. This could allow an attacker to use an old authorization receipt to execute actions long after it was issued. - Recommendation: Add an
expires_atfield to the authorization payload and validate it during theverify_bilateral_receiptprocess.
- The
-
Insufficient Validation of Input Data:
- The
sign_authorization,seal_result, andverify_bilateral_receiptfunctions do not validate the structure or types of the input data (e.g.,payload,result_data,envelope). This could lead to unexpected behavior or security vulnerabilities if malformed data is passed. - Recommendation: Use Pydantic models to validate the input data for these functions. This will ensure type safety and prevent potential issues from malformed inputs.
- The
-
Canonicalization Process Not Validated:
- The
_canonicalizefunction is used to generate canonical JSON for signing, but there is no validation to ensure that the canonicalization process is consistent and secure. - Recommendation: Add tests to verify that the
_canonicalizefunction produces consistent and correct output for various edge cases, including nested JSON structures, special characters, and large payloads.
- The
🟡 WARNING
-
Potential Breaking Change in
verify_bilateral_receipt:- The
verify_bilateral_receiptfunction falls back toverify_receipt()for non-bilateral envelopes. While this ensures backward compatibility, any changes to theverify_receipt()function in the future could inadvertently affect the behavior ofverify_bilateral_receipt. - Recommendation: Clearly document this dependency in the code and consider adding tests to ensure that changes to
verify_receipt()do not breakverify_bilateral_receipt.
- The
-
Backward Compatibility Assumptions:
- The PR assumes that the addition of new fields (e.g.,
bilateral,result) to the receipt structure will not break existing consumers of thereceipts.pymodule. While this is likely true, it should be explicitly tested. - Recommendation: Add tests to ensure that existing functionality (e.g., single-signature receipt verification) continues to work as expected with the new fields present.
- The PR assumes that the addition of new fields (e.g.,
💡 SUGGESTIONS
-
Improve Error Messages:
- The error messages in
seal_result(e.g., "Cannot seal a non-bilateral receipt") could be more descriptive to aid debugging. - Recommendation: Include additional context in error messages, such as the current state of the envelope or the expected state.
- The error messages in
-
Thread Safety:
- The
sign_authorizationandseal_resultfunctions rely on theSignerobject, which includes aprivate_key. If theSignerobject is shared across threads, this could lead to race conditions or other concurrency issues. - Recommendation: Document whether the
Signerobject is thread-safe. If it is not, consider adding locks or requiring that each thread use its ownSignerinstance.
- The
-
Extend Test Coverage:
- While the PR mentions that 11 tests pass locally, it is unclear whether edge cases (e.g., malformed envelopes, invalid signatures, missing fields) are thoroughly tested.
- Recommendation: Add tests for edge cases, including:
- Missing or malformed
authorizationandresultsections. - Invalid or tampered signatures.
- Receipts with unexpected or additional fields.
- Missing or malformed
-
Performance Considerations:
- The
verify_bilateral_receiptfunction performs multiple cryptographic operations (e.g., signature verification, hash computation). For high-throughput scenarios, this could become a bottleneck. - Recommendation: Benchmark the performance of the verification process and consider optimizations if necessary. For example, caching intermediate results or parallelizing verification steps.
- The
-
Documentation Improvements:
- The proposal document is comprehensive, but the code itself could benefit from more inline comments explaining the rationale behind key design decisions (e.g., why certain fields are included in the receipt).
- Recommendation: Add comments to the code to explain the purpose of each field in the receipt and the reasoning behind the design choices.
Summary of Actionable Feedback
🔴 CRITICAL
- Add a cryptographically secure nonce to the
payloadinsign_authorizationto prevent replay attacks. - Include an
expires_atfield in the authorization payload and validate it during verification. - Use Pydantic models to validate input data for
sign_authorization,seal_result, andverify_bilateral_receipt. - Add tests to validate the
_canonicalizefunction for edge cases.
🟡 WARNING
- Document the dependency of
verify_bilateral_receiptonverify_receipt()and add tests to ensure future changes do not break compatibility. - Add tests to confirm that existing single-signature receipt functionality remains unaffected by the new fields.
💡 SUGGESTION
- Improve error messages in
seal_resultfor better debugging. - Document or ensure thread safety of the
Signerobject. - Extend test coverage to include edge cases and malformed inputs.
- Benchmark and optimize the performance of
verify_bilateral_receiptif necessary. - Add inline comments to explain the rationale behind key design decisions in the code.
Let me know if you need further clarification or assistance!
|
@microsoft-github-policy-service agree |
…rifier Two harness updates from Apr 28-29 GitHub activity: - Nobulex (@arian-gogani) merged @nobulex/crypto into Microsoft Agent Governance Toolkit (microsoft/agent-governance-toolkit#1333, OpenSSF passing badge). Material elevation — bilateral-receipt primitive is now Microsoft-shipped, not vendor-isolated. - msaleme (Michael Saleme) volunteered to add their x402 conformance harness as 6th independent verifier (A2A #1672 comment 2026-04-29). Targeting v4.5 with claim_type-tagged output. The 41 existing x402 tests map onto continuity-layer evidence_basis.evidence_type. payment_execution lane. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Creates ADOPTERS.md as referenced in the README. Nobulex's bilateral receipt primitive was merged into AGT via PRs microsoft#1302 and microsoft#1333 (216 LOC, 11 tests). 8 independent implementations have byte-match validated the JCS/RFC 8785 canonicalization substrate. AAIF Growth-stage proposal filed with TC sponsorship from the CTEF maintainer.
…icrosoft#1333) * Create verifiable-compliance-receipts.md * add bilateral receipt signing (pre-execution + post-execution)
Extends
receipts.pywith bilateral receipt support per the design doc in #1302.Three new functions:
sign_authorization()— signs pre-execution commitment (proves policy evaluated before action ran)seal_result()— seals post-execution outcome (binds actual result to the authorization)verify_bilateral_receipt()— verifies both signatures, falls back toverify_receipt()for standard envelopesBackward compatible. Existing single-signature receipts verify unchanged. Bilateral fields are additive.
The result signature covers the binding of authorization_hash and result_hash together, proving:
11 tests pass locally covering: creation, chain linkage, sealing, tamper detection (payload and result), wrong key rejection, and deny receipts.
Relates to #1249.