Skip to content

fix(rest/nodejs): verify request signatures per RFC 9421 instead of accepting everything - #162

Open
vishkaty wants to merge 2 commits into
Universal-Commerce-Protocol:mainfrom
vishkaty:nodejs-signature-verification
Open

fix(rest/nodejs): verify request signatures per RFC 9421 instead of accepting everything#162
vishkaty wants to merge 2 commits into
Universal-Commerce-Protocol:mainfrom
vishkaty:nodejs-signature-verification

Conversation

@vishkaty

@vishkaty vishkaty commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What

The Node reference server accepts every inbound request without looking at HTTP Message Signatures: there is no verification code anywhere in rest/nodejs/src (observed at a95b928). Expected per docs/specification/signatures.md, businesses verify platform request signatures, with ES256 as the baseline every verifier MUST support. PR #122 added exactly this to the Python reference server; this change brings the Node server to behavioral parity so the two references agree on the wire.

Behavior (mirrors the merged #122 semantics)

  • Default mode is permissive: unsigned requests pass untouched, and a signature that is present is verified with the outcome logged, so existing clients keep working.
  • REQUIRE_SIGNATURES=true (env, following the existing SIMULATION_SECRET config pattern) enforces: 401 signature_missing / signature_invalid / key_not_found, 400 digest_mismatch / invalid_profile_url, 424 profile_unreachable, 422 profile_malformed, matching the Python server codes and statuses.
  • Signer keys are discovered from the UCP-Agent profile keys[] (the RFC 7517 shape per ucp#566), cached 300s, with the same SSRF guards as Python: HTTPS only, no credentials, no loopback/private/link-local hosts, redirects not followed; ALLOW_INSECURE_PROFILE_URLS=true relaxes this for localhost demos and CI only.
  • RFC 9421 base construction with the UCP covered component set, RFC 9530 Content-Digest (sha-256 over the raw body bytes) checked before signature verification, ES256 as fixed width raw r||s (64 bytes, DER rejected) plus Ed25519, alg parameter rejected, @signature-params echoed verbatim.
  • Implementation uses only node:crypto and a hand rolled RFC 8941 subset; no new dependencies.

Route coverage (parity with #122)

Route Python #122 Node here
checkout-sessions create/get/update/complete/cancel verified verified
orders get/update verified verified
testing simulate-shipping verified verified
GET /.well-known/ucp public, not verified public, not verified
/mcp and partner webhook receiver verified routes do not exist in the Node server

Tests

npm run build (tsc) clean; npm test 116/116 (31 pre existing, 85 new), covering: signed ES256 request accepted end to end against a served key profile, tampered body, garbage and DER signatures, unpublished and non verify keys, missing covered components, unsigned allowed when permissive and rejected when enforcing, Ed25519 accepted, byte exact RFC 9421 Appendix B and RFC 9530 vectors. Each verification behavior was mutation tested: disabling it turns specific tests red (verify always pass: 9 red; digest skip: 3 red; DER accepted: 2 red; enforcement removed: 14 red; coverage gate removed: 1 red; key capability filter removed: 2 red). Repo pre-commit hooks pass on the changed files; the Python server suite is untouched and still passes (131 tests).

Notes for reviewers

  • Base64 in Signature and Content-Digest values requires canonical padding, matching the Python parser (base64.b64decode(validate=True)).
  • Two small knowingly accepted divergences from Python, both edge cases, called out rather than hidden: the IPv6 SSRF allowlist here is global unicast only, slightly stricter than the Python address class checks (a NAT64 network would differ); and @path/@query come from the WHATWG URL parser, which percent encodes a few characters Starlette passes through raw. Happy to align either if preferred.
  • signatures.md at the 2026-04-08 tag still says signing_keys[]; this follows the merged fix(rest/python): verify request signatures per RFC 9421 instead of accepting everything #122 and ucp#566 in reading keys[] only.

…ccepting everything

The Node reference server accepted every inbound request without any
signature handling, while signatures.md mandates asymmetric RFC 9421
signatures (ES256 baseline) with key discovery from the UCP-Agent
profile. The Python reference server gained this verification in Universal-Commerce-Protocol#122;
this change brings the Node server to behavioral parity.

src/utils/signature.ts is the twin of the Python ucp_signing module:
RFC 9421 signature base construction with the UCP covered component
set, RFC 9530 Content-Digest over the raw body bytes, ES256 with fixed
width raw r||s signatures (never ASN.1/DER, per the spec MUST) plus
Ed25519, signer key discovery from the UCP-Agent profile keys[] with a
300 second cache and an SSRF guard, and a Hono middleware applying the
same enforcement semantics as the Python verify_signature dependency.
Only node:crypto is used; the RFC 8941 structured field subset is hand
rolled, so no new dependencies are added.

Compatibility is preserved by default:
- REQUIRE_SIGNATURES (default false) and ALLOW_INSECURE_PROFILE_URLS
  (default false) follow the env var pattern the server already uses.
- With enforcement off, signatures are still verified when present and
  the outcome logged, but unsigned or invalid requests are allowed. No
  profile fetch occurs unless a Signature-Input header is present, so
  unsigned traffic incurs no extra work.
- With enforcement on, the spec error codes are returned: 401
  signature_missing / signature_invalid / key_not_found, 400
  digest_mismatch / algorithm_unsupported / invalid_profile_url, 424
  profile_unreachable, 422 profile_malformed.

Every business route (checkout sessions, orders, testing) verifies;
the discovery profile stays public, matching the Python server.

test/signing.test.ts anchors the module to the RFC 9421 Appendix B and
RFC 9530 published vectors, including a byte exact Ed25519 B.2.6 check
and an explicit DER rejection test. test/signature.test.ts proves both
modes end to end against a localhost profile server discovered through
the UCP-Agent header, mirroring the scenarios of the Python
signature_integration_test.py.
@damaz91 damaz91 added the status:needs-triage Signal that the PR is ready for human triage label Aug 3, 2026
@damaz91 damaz91 added status:under-review and removed status:needs-triage Signal that the PR is ready for human triage labels Aug 4, 2026
@carolinerg1
carolinerg1 self-requested a review August 4, 2026 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants