fix(rest/nodejs): verify request signatures per RFC 9421 instead of accepting everything - #162
Open
vishkaty wants to merge 2 commits into
Open
Conversation
…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
approved these changes
Aug 4, 2026
carolinerg1
self-requested a review
August 4, 2026 15:18
carolinerg1
approved these changes
Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)
REQUIRE_SIGNATURES=true(env, following the existingSIMULATION_SECRETconfig pattern) enforces: 401signature_missing/signature_invalid/key_not_found, 400digest_mismatch/invalid_profile_url, 424profile_unreachable, 422profile_malformed, matching the Python server codes and statuses.UCP-Agentprofile 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=truerelaxes this for localhost demos and CI only.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,algparameter rejected,@signature-paramsechoed verbatim.Route coverage (parity with #122)
Tests
npm run build(tsc) clean;npm test116/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
SignatureandContent-Digestvalues requires canonical padding, matching the Python parser (base64.b64decode(validate=True)).@path/@querycome from the WHATWG URL parser, which percent encodes a few characters Starlette passes through raw. Happy to align either if preferred.signing_keys[]; this follows the merged fix(rest/python): verify request signatures per RFC 9421 instead of accepting everything #122 and ucp#566 in readingkeys[]only.