Skip to content

Allow receipt revocation after embedded payment-request JWT expiry - #241

Open
mjesuele wants to merge 1 commit into
agentcommercekit:mainfrom
mjesuele:experiment/receipt-revocation-lifecycle
Open

mjesuele wants to merge 1 commit into
agentcommercekit:mainfrom
mjesuele:experiment/receipt-revocation-lifecycle

Conversation

@mjesuele

@mjesuele mjesuele commented Oct 5, 2026 •

Copy link
Copy Markdown

In the public v1 example issuer, a receipt can remain valid after its embedded payment-request JWT expires. The DELETE route currently verifies that historical token with expiry enabled, returning HTTP 400 before issuer comparison or revocation.

Verify the historical token with verifyExpiry: false, retaining its signature verification and authenticated issuer recovery. The route still compares that issuer with the revocation command's signer. signedPayloadValidator independently authenticates the command and enforces its JWT expiry when supplied. This follows the lifetime handling already used by receipt verification.

The five lifecycle tests cover:

  • Original seller before quote expiry.
  • Original seller after quote JWT expiry while the receipt still verifies.
  • Rejection of a different seller.
  • Rejection of an expired management command.
  • Rejection of a corrupted historical-token signature.

Validation:

  • pnpm --filter ./examples/issuer exec vitest run src/routes/receipts-lifecycle.test.ts: 5/5 pass; removing the repair yields 2 failures and 3 passes.
  • pnpm run check: 29/29 tasks and 580 tests pass, with zero lint warnings/errors.
  • git diff --check: passes.

Scope: the public v1 example issuer only. The standard createPaymentRequestToken helper currently omits JWT exp, so ordinary helper-generated flows do not reproduce this configuration. Explicit JWT exp is already supported by the verifier and existing tests; it is distinct from PaymentRequest.expiresAt. Persistence is mocked, so these tests prove dispatch to revokeCredential, not a persisted status-bit transition.

AI disclosure: Codex generated the implementation and tests, prepared the PR text, ran validation, and was used for additional code review. I also used ChatGPT for a human-in-the-loop comprehension deep dive on the issue and proposed change. After that review, I independently understand the submitted code and can explain what it does and how it interacts with the surrounding system without AI assistance.

Summary by CodeRabbit

  • Bug Fixes
    • The original seller can now revoke a receipt after its payment request expires, provided the revocation command is valid and authenticated.
    • Revocation remains unavailable to other sellers and is rejected for expired commands or forged payment requests.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5a32ec79-17cf-455b-a8b8-f44bafed8bb6
📥 Commits

Reviewing files that changed from the base of the PR and between c055c56 and 568c0e0.

📒 Files selected for processing (2)
  • examples/issuer/src/routes/receipts-lifecycle.test.ts
  • examples/issuer/src/routes/receipts.ts

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


Walkthrough

The receipt revocation route now authenticates the embedded payment-request token without checking its expiry. Lifecycle tests cover revocation before and after quote expiry, along with rejection of unauthorized, expired, and forged requests.

Changes

Receipt Revocation

Layer / File(s) Summary
Revocation token authentication
examples/issuer/src/routes/receipts.ts
The route verifies the embedded payment-request token with expiry checking disabled. The signed revocation command remains separately validated.
Revocation lifecycle tests
examples/issuer/src/routes/receipts-lifecycle.test.ts
Tests cover revocation before and after quote expiry, rejection of a different seller and an expired management command, and rejection of a forged historical token signature.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 568c0

Receipt revocation can accept an expired historical payment-request token while retaining the separate authorization checks. No merge-blocking issue was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 568c0

The change preserves signature verification and original-issuer authorization when signed commands are required. It restores revocation after quote expiry without changing receipt creation. Database-backed failure and concurrency behavior, and deployed authentication settings, remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — With unsigned mode disabled, the newly reachable mutation concerns stored receipts whose embedded quote JWT has expired and whose original quote issuer can authenticate the revocation command. Possession of an expired quote alone does not grant revocation authority.

Security Findings and Attack Paths

  • observed — The existing unsigned-development fallback accepts a schema-valid raw command and caller-supplied issuer header when ALLOW_UNSIGNED_PAYLOADS is explicitly true. This PR also makes expired-quote receipts reachable through that fallback. The fallback predates the PR, is documented as default-off and forbidden outside local development, and is disabled by the lifecycle tests; deployed configuration was not supplied.

Trust Boundaries and Controls

  • observed — The attacker-selected receipt id does not supply the historical issuer: that identity is recovered from the token in the stored receipt. The handler compares it with the validated command issuer before persistence. Tests assert no revocation call for another seller, an expired command, or a corrupted historical signature.

Resilience and Maintainability Implications

  • inferred — The unchanged persistence owner writes revokedAt before separately reading and replacing a shared status-list bitstring. Partial failure can leave the timestamp updated without the published revocation bit; concurrent replacements can lose another receipt’s bit. Repetition sets the same bit again but updates the timestamp. These weaknesses predate the PR, including for ordinary quotes without JWT expiry, and the mocked tests do not establish stronger guarantees.

Hardening Proposals

  • proposed — For production use, make timestamp and status-bit updates atomic and concurrency-safe, and verify interruption, repetition, and competing revocations against a real database. Keep the unsigned-development fallback disabled and enforce that restriction in deployment configuration.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: allowing receipt revocation after the embedded payment-request JWT expires.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant