From c3f57fea80299e60ced2372b2551358b7881bc24 Mon Sep 17 00:00:00 2001 From: Vishal Katyal Date: Fri, 18 Sep 2026 01:40:02 -0400 Subject: [PATCH] fix(rest/nodejs): answer signature failures with the UCP error envelope Signature verification failures returned {detail: {status, errors[{code, message, severity}]}}. That body matched neither shape the specification uses. signatures.md shows a REST signature rejection as a flat {code, content} body, and error_response.json describes the envelope, which requires ucp and messages[] with each message carrying type, code, content and a severity from a four value enum. The emitted severity "critical" is in no enum, the field was named message rather than content, and the whole body sat inside a detail wrapper. The comment on the old helper said the detail wrapper was what this server already used for its 4xx responses. That was accurate when #162 landed, because every 4xx path answered that way then. #174 moved the checkout paths onto ucpErrorResponse and left this helper behind. Both PRs are mine. ucpErrorResponse now has 26 call sites, and test/error_envelope.test.ts already pins that shape for the paths it covers, so the signature path joins them. The severity is unrecoverable because no resource exists to act on when a request is turned away at the signature layer, which is the definition message_error.json gives for that value. Scope. Two other paths still answer outside the envelope and are deliberately left alone: order.ts returns a bare error object on 404, covered by open #214, and validation.ts answers 422 as text, covered by my open #192. The simulation endpoint and the internal error catch-all are not UCP protocol surfaces. The existing assertions read detail.errors[0].code and so encoded the old shape. They now read messages[0].code and also assert ucp.status, and a new test pins the full envelope including the severity enum. Local: npm test 155 passed, was 154. tsc clean. Removing the fix with the tests kept turns 15 tests red. pre-commit run --all-files passes. --- rest/nodejs/src/utils/signature.ts | 27 +++++++------- rest/nodejs/test/signature.test.ts | 58 ++++++++++++++++++++++++++++-- 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/rest/nodejs/src/utils/signature.ts b/rest/nodejs/src/utils/signature.ts index 6868afe..6c99a55 100644 --- a/rest/nodejs/src/utils/signature.ts +++ b/rest/nodejs/src/utils/signature.ts @@ -19,6 +19,7 @@ import { type Context, type MiddlewareHandler } from "hono"; import { type ContentfulStatusCode } from "hono/utils/http-status"; import { signatureConfig } from "./config"; +import { UcpError, ucpErrorResponse } from "./ucp_error"; // RFC 9421 HTTP Message Signatures for UCP, using the default UCP profile. // This is the Node twin of the Python server's ucp_signing.py: RFC 9421 @@ -708,18 +709,20 @@ export async function fetchSigningKeys( } function signatureErrorResponse(c: Context, exc: SignatureError): Response { - // The same wire shape as the Python server's error envelope, inside the - // { detail } wrapper this server already uses for its 4xx responses. - return c.json( - { - detail: { - status: "error", - errors: [ - { code: exc.code, message: exc.message, severity: "critical" }, - ], - }, - }, - exc.statusCode as ContentfulStatusCode + // ucpErrorResponse is what this server answers its other protocol-level + // rejections with, so the signature path uses it too. error_response.json + // requires a ucp object plus messages[], and message_error.json constrains + // severity to the four values in its enum. Unrecoverable is the right one + // here because no resource exists to act on when a request is turned away + // at the signature layer, which is how message_error.json defines it. + return ucpErrorResponse( + c, + new UcpError( + exc.message, + exc.code, + exc.statusCode as ContentfulStatusCode, + "unrecoverable" + ) ); } diff --git a/rest/nodejs/test/signature.test.ts b/rest/nodejs/test/signature.test.ts index a5e10b0..7b1a37c 100644 --- a/rest/nodejs/test/signature.test.ts +++ b/rest/nodejs/test/signature.test.ts @@ -46,9 +46,23 @@ const AUTHORITY = "merchant.test"; const ORIGIN = `http://${AUTHORITY}`; type ErrorEnvelope = { - detail: { status: string; errors: Array<{ code: string; message: string }> }; + ucp?: { version?: string; status?: string }; + messages?: Array<{ + type?: string; + code?: string; + content?: string; + severity?: string; + }>; + detail?: unknown; }; +const ERROR_SEVERITIES = new Set([ + "recoverable", + "requires_buyer_input", + "requires_buyer_review", + "unrecoverable", +]); + // A minimal app wired like src/index.ts (verifySignature ahead of validation) // but without the pino middleware, following the lifecycle.test.ts convention. function buildApp() { @@ -222,10 +236,48 @@ async function assertError( ): Promise { assert.equal(response.status, status, await response.clone().text()); const envelope = (await response.json()) as ErrorEnvelope; - assert.equal(envelope.detail.status, "error"); - assert.equal(envelope.detail.errors[0]?.code, code); + assert.equal(envelope.ucp?.status, "error"); + assert.equal(envelope.messages?.[0]?.code, code); } +test("enforced: a rejection body is the UCP error envelope", async () => { + // error_response.json requires ucp and messages[]; each message is a + // message_error.json with type, code, content and an in-enum severity. The + // rest of this server already answers in that shape (see + // error_envelope.test.ts), so the signature path should not differ. + enforce(); + const body = checkoutBody(); + const response = await postCheckout( + { + "UCP-Agent": `profile="${profileUrl}"`, + "Idempotency-Key": "envelope-1", + "Request-Id": "envelope-1", + "Content-Type": "application/json", + }, + body + ); + + assert.equal(response.status, 401, await response.clone().text()); + const envelope = (await response.json()) as ErrorEnvelope; + + assert.equal(envelope.detail, undefined, "flat detail shape must be gone"); + assert.equal(envelope.ucp?.status, "error"); + assert.ok(envelope.ucp?.version, "ucp.version must be present"); + assert.ok( + Array.isArray(envelope.messages) && envelope.messages.length === 1, + "messages[] must carry exactly the failure" + ); + + const message = envelope.messages![0]; + assert.equal(message.type, "error"); + assert.equal(message.code, "signature_missing"); + assert.ok(message.content, "content must explain the failure"); + assert.ok( + ERROR_SEVERITIES.has(message.severity ?? ""), + `severity ${message.severity} is outside the message_error.json enum` + ); +}); + /* Permissive (default) mode: existing clients keep working. */ test("permissive: an unsigned request skips verification entirely", async () => {