Skip to content

fix: HMAC-bound challenge IDs for MCP transport + add description/externalId to ChargeRequest#38

Merged
brendanjryan merged 2 commits intomainfrom
brendan/spec-compliance-fixes
Feb 16, 2026
Merged

fix: HMAC-bound challenge IDs for MCP transport + add description/externalId to ChargeRequest#38
brendanjryan merged 2 commits intomainfrom
brendan/spec-compliance-fixes

Conversation

@brendanjryan
Copy link
Collaborator

Summary

Two spec compliance fixes identified during audit of pympp against the MPP specifications.

1. MCP challenge IDs use HMAC-SHA256 instead of random tokens (HIGH severity)

Problem: create_challenge() in mcp/verify.py used secrets.token_urlsafe(16) for challenge IDs. The MCP spec requires challenge IDs to be cryptographically bound to realm, method, intent, request, and expires.

Fix:

  • Replaced random tokens with generate_challenge_id() from mpp.__init__ (HMAC-SHA256)
  • Added secret_key parameter to create_challenge(), verify_or_challenge(), and the @pay decorator
  • verify_or_challenge() now verifies echoed challenge IDs against recomputed HMAC before proceeding to intent verification, matching the pattern used in the HTTP transport (server/verify.py)
  • The @pay decorator auto-detects secret_key from MPP_SECRET_KEY env var via detect_secret_key() if not explicitly provided

2. Add description and externalId to ChargeRequest (LOW severity)

Problem: The charge intent spec (Section 5.1.2) defines description and externalId as optional request fields, but the Pydantic model was missing them.

Fix: Added both as str | None = None fields to ChargeRequest.

Tests

  • All existing tests updated to use HMAC-bound challenge IDs
  • New test: test_rejects_credential_with_invalid_challenge_id verifies forged IDs are rejected
  • New tests: test_challenge_id_is_hmac_bound, test_challenge_id_deterministic
  • New tests: test_charge_request_with_description, test_charge_request_with_external_id, test_charge_request_description_and_external_id_default_none, test_charge_request_serializes_optional_fields
  • 161 tests passing

brendanjryan and others added 2 commits February 16, 2026 15:41
…ernalId to ChargeRequest

Two spec compliance fixes:

1. MCP challenge IDs now use HMAC-SHA256 (HIGH severity)
   - Replaced secrets.token_urlsafe(16) with generate_challenge_id()
     in create_challenge() and verify_or_challenge()
   - Added secret_key parameter to create_challenge(), verify_or_challenge(),
     and the @pay decorator
   - verify_or_challenge() now verifies echoed challenge IDs against
     recomputed HMAC, matching the HTTP transport pattern
   - Challenge IDs are cryptographically bound to realm, method, intent,
     request, and expires per the MCP spec

2. Add description and externalId to ChargeRequest (LOW severity)
   - Added optional description and externalId fields per charge intent
     spec Section 5.1.2

Tests: 68 MCP + tempo tests passing, 161 total
@brendanjryan brendanjryan merged commit 65ad90a into main Feb 16, 2026
@brendanjryan brendanjryan deleted the brendan/spec-compliance-fixes branch February 16, 2026 23:46
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