From d6703572f669cd424e66b652dd8c7d0732102469 Mon Sep 17 00:00:00 2001 From: Vishal Katyal Date: Mon, 26 Oct 2026 13:28:45 -0400 Subject: [PATCH] fix(rest/nodejs): drop a profile webhook_url that is not an http(s) URL instead of storing it parseAgentProfile returned whatever value the platform profile carried under config.webhook_url on the order capability. The schema types the member as a string with format uri (order.json, $defs/platform_schema), but an ftp URL, an integer, an object, an array, a data: URL, a bare string or an http URL carrying credentials was stored on the checkout, echoed under platform.webhook_url on every response, and handed to notifyWebhook on completion, where new URL() throws inside signRequest for an integer or an object, fetch() rejects an ftp scheme after three attempts and 300 ms of backoff inside the complete request, and fetch() refuses to construct a request from a URL that carries credentials, after the same three attempts. A data: URL is worse: fetch resolves it with ok true, so the order_placed event is treated as delivered while nothing left the process. An array holding one URL is coerced to a string by new URL() and fetch() and delivered, so that schema violation is never seen at all. Only an http(s) URL without credentials is a deliverable target, so any other value is now dropped with a log line, the way every other profile fault is treated, and the platform member is left absent. The credentials check is the one assertProfileUrlAllowed applies to a profile URL. An update carrying a malformed value keeps the valid one already stored. The Python reference drops a malformed value the same way since #257; it keeps a URL carrying credentials, which its httpx client delivers to with basic auth, so the two references part on that one shape. Eleven cases in test/agent_profile.test.ts drive the profile through app.request() as a data: URI: the valid control, eight malformed shapes on create and get, each asserting the drop is logged once with the value, the update twin, and a completion that must attempt no delivery. With the gate excised and the tests kept, ten go red. --- rest/nodejs/src/api/checkout.ts | 40 +++- rest/nodejs/test/agent_profile.test.ts | 289 +++++++++++++++++++++++++ 2 files changed, 328 insertions(+), 1 deletion(-) create mode 100644 rest/nodejs/test/agent_profile.test.ts diff --git a/rest/nodejs/src/api/checkout.ts b/rest/nodejs/src/api/checkout.ts index f8346e4..2447cd4 100644 --- a/rest/nodejs/src/api/checkout.ts +++ b/rest/nodejs/src/api/checkout.ts @@ -73,6 +73,37 @@ import { type IdParamContext } from "../utils/validation"; /** * Service for managing checkout sessions. */ +// The order capability config of a platform types webhook_url as a string with +// format uri (order.json, $defs/platform_schema), and notifyWebhook hands the +// stored value to new URL() and fetch(). Only an http(s) URL without +// credentials is a deliverable target, so anything else published in a profile +// is dropped here with a log line, the way every other profile fault is +// treated, instead of being stored, echoed on every response for the checkout, +// and thrown on at delivery. The Python reference drops a malformed value the +// same way; it keeps a URL carrying credentials, which its httpx client can +// deliver to. +function validWebhookUrl(value: unknown): string | undefined { + if (typeof value !== "string") { + return undefined; + } + try { + const target = new URL(value); + if (target.protocol !== "http:" && target.protocol !== "https:") { + return undefined; + } + // fetch() refuses to construct a request from a URL that carries + // credentials, so such a target can never receive a delivery either + // (the same check assertProfileUrlAllowed applies to a profile URL). + if (target.username || target.password) { + return undefined; + } + return value; + } catch { + // Not a URL at all; fall through to the drop below. + } + return undefined; +} + export class CheckoutService { private computeHash( operation: string, @@ -147,7 +178,14 @@ export class CheckoutService { if (orderCaps && orderCaps.length > 0) { const orderCap = orderCaps[0]; if (orderCap && orderCap.config && orderCap.config.webhook_url) { - return { webhook_url: orderCap.config.webhook_url }; + const webhookUrl = validWebhookUrl(orderCap.config.webhook_url); + if (webhookUrl) { + return { webhook_url: webhookUrl }; + } + console.warn( + "Ignoring malformed webhook_url in agent profile", + orderCap.config.webhook_url + ); } } } diff --git a/rest/nodejs/test/agent_profile.test.ts b/rest/nodejs/test/agent_profile.test.ts new file mode 100644 index 0000000..4387686 --- /dev/null +++ b/rest/nodejs/test/agent_profile.test.ts @@ -0,0 +1,289 @@ +// Copyright 2026 UCP Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import assert from "node:assert/strict"; +import { before, test } from "node:test"; + +import { zValidator } from "@hono/zod-validator"; +import { Hono } from "hono"; + +import { CheckoutService } from "../src/api/checkout"; +import { getProductsDb, getTransactionsDb, initDbs } from "../src/data/db"; +import { + CheckoutCompleteRequestSchema, + ExtendedCheckoutCreateRequestSchema, + ExtendedCheckoutUpdateRequestSchema, +} from "../src/models"; +import { IdParamSchema, prettyValidation } from "../src/utils/validation"; + +// The order capability config of a platform carries the webhook URL the business +// delivers order events to (order.md, Webhook URL Configuration). The schema +// types it as a string with format uri (order.json, $defs/platform_schema), +// and the delivery path hands the stored value to new URL() and fetch(). A +// profile that publishes anything else must not be stored as the platform +// webhook target: it would be echoed on every response for that checkout and +// thrown on, or burned through retries, at delivery. This mirrors the +// Python reference, which drops a malformed profile value with a log line. + +// A minimal app wired with just the checkout routes, the same convention as +// lifecycle.test.ts, so the agent profile path is exercised through +// app.request() with a real UCP-Agent header. +function buildApp() { + const svc = new CheckoutService(); + const app = new Hono<{ Variables: { logger: typeof console } }>(); + app.use(async (c, next) => { + c.set("logger", console); + await next(); + }); + app.post( + "/checkout-sessions", + zValidator("json", ExtendedCheckoutCreateRequestSchema, prettyValidation), + svc.createCheckout + ); + app.get( + "/checkout-sessions/:id", + zValidator("param", IdParamSchema, prettyValidation), + svc.getCheckout + ); + app.put( + "/checkout-sessions/:id", + zValidator("param", IdParamSchema, prettyValidation), + zValidator("json", ExtendedCheckoutUpdateRequestSchema, prettyValidation), + svc.updateCheckout + ); + app.post( + "/checkout-sessions/:id/complete", + zValidator("param", IdParamSchema, prettyValidation), + zValidator("json", CheckoutCompleteRequestSchema, prettyValidation), + svc.completeCheckout + ); + return app; +} + +before(() => { + initDbs(":memory:", ":memory:"); + getProductsDb() + .prepare( + "INSERT INTO products (id, title, price, image_url) VALUES (?, ?, ?, ?)" + ) + .run("bouquet_roses", "Red Rose", 3500, ""); + getTransactionsDb() + .prepare("INSERT INTO inventory (product_id, quantity) VALUES (?, ?)") + .run("bouquet_roses", 100); +}); + +const VALID_WEBHOOK = "https://platform.example/ucp-webhook"; +const CREATE_BODY = { + currency: "USD", + line_items: [{ item: { id: "bouquet_roses" }, quantity: 1 }], + payment: {}, + buyer: { email: "john.doe@example.com" }, + fulfillment: { + methods: [{ type: "shipping", selected_destination_id: "addr_1" }], + }, +}; + +// A platform profile whose order capability config carries `webhookUrl`, +// delivered as a data: URI so the server parses it without any network. +function agentHeader(webhookUrl: unknown): Record { + const profile = { + ucp: { + version: "2026-04-08", + capabilities: { + "dev.ucp.shopping.order": [ + { + name: "dev.ucp.shopping.order", + version: "2026-04-08", + config: { webhook_url: webhookUrl }, + }, + ], + }, + }, + }; + const encoded = Buffer.from(JSON.stringify(profile)).toString("base64"); + return { + "Content-Type": "application/json", + "UCP-Agent": `profile="data:application/json;base64,${encoded}"`, + }; +} + +async function create(app: ReturnType, webhookUrl: unknown) { + const res = await app.request("/checkout-sessions", { + method: "POST", + headers: agentHeader(webhookUrl), + body: JSON.stringify(CREATE_BODY), + }); + const text = await res.text(); + assert.equal(res.status, 201, text); + return JSON.parse(text) as { + id: string; + platform?: { webhook_url?: unknown }; + fulfillment: { + methods: { + groups: { + id: string; + line_item_ids: string[]; + options: { id: string }[]; + }[]; + }[]; + }; + }; +} + +// Runs fn with console.warn captured, restoring it afterwards. The drop must +// be loud: the log line is the only signal an operator gets that a platform +// published a value the business cannot deliver to. +async function captureWarnings( + fn: () => Promise +): Promise<[T, unknown[][]]> { + const warned: unknown[][] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => { + warned.push(args); + }; + try { + return [await fn(), warned]; + } finally { + console.warn = originalWarn; + } +} + +test("a profile webhook_url that is a valid http(s) URL is stored and echoed", async () => { + const app = buildApp(); + const created = await create(app, VALID_WEBHOOK); + assert.deepEqual(created.platform, { webhook_url: VALID_WEBHOOK }); +}); + +for (const [label, value] of [ + ["an ftp URL", "ftp://platform.invalid/hook"], + ["an integer", 42], + ["an object", { url: VALID_WEBHOOK }], + ["a data: URL", "data:text/plain,ok"], + ["a mailto: URL", "mailto:ops@platform.invalid"], + ["a string that is not a URL", "xts-not-a-url-2030"], + ["an array holding a valid URL", [VALID_WEBHOOK]], + ["a URL carrying credentials", "http://user:pass@platform.invalid/hook"], +] as const) { + test(`a profile webhook_url that is ${label} is not stored or echoed`, async () => { + const app = buildApp(); + const [created, warned] = await captureWarnings(() => create(app, value)); + assert.equal( + created.platform, + undefined, + `platform must be absent, got ${JSON.stringify(created.platform)}` + ); + assert.deepEqual( + warned.filter( + (args) => args[0] === "Ignoring malformed webhook_url in agent profile" + ), + [["Ignoring malformed webhook_url in agent profile", value]], + "the dropped value must be logged once" + ); + const fetched = await app.request(`/checkout-sessions/${created.id}`); + assert.equal(fetched.status, 200); + const body = (await fetched.json()) as { platform?: unknown }; + assert.equal(body.platform, undefined, "the value must not be persisted"); + }); +} + +test("an update whose profile webhook_url is malformed keeps the stored valid one", async () => { + const app = buildApp(); + const created = await create(app, VALID_WEBHOOK); + const res = await app.request(`/checkout-sessions/${created.id}`, { + method: "PUT", + headers: agentHeader("ftp://platform.invalid/hook"), + body: JSON.stringify({ + currency: "USD", + line_items: [ + { id: "line_1", item: { id: "bouquet_roses" }, quantity: 1 }, + ], + buyer: { email: "john.doe@example.com" }, + }), + }); + const text = await res.text(); + assert.equal(res.status, 200, text); + const body = JSON.parse(text) as { platform?: { webhook_url?: unknown } }; + assert.deepEqual(body.platform, { webhook_url: VALID_WEBHOOK }); +}); + +test("completing a checkout whose profile webhook_url is malformed attempts no delivery", async () => { + const app = buildApp(); + const created = await create(app, "ftp://platform.invalid/hook"); + const group = created.fulfillment.methods[0]!.groups[0]!; + const selected = await app.request(`/checkout-sessions/${created.id}`, { + method: "PUT", + headers: agentHeader("ftp://platform.invalid/hook"), + body: JSON.stringify({ + currency: "USD", + line_items: [ + { id: "line_1", item: { id: "bouquet_roses" }, quantity: 1 }, + ], + buyer: { email: "john.doe@example.com" }, + fulfillment: { + methods: [ + { + type: "shipping", + selected_destination_id: "addr_1", + groups: [ + { + id: group.id, + line_item_ids: group.line_item_ids, + selected_option_id: group.options[0]!.id, + }, + ], + }, + ], + }, + }), + }); + assert.equal(selected.status, 200, await selected.clone().text()); + + const attempted: string[] = []; + const originalFetch = globalThis.fetch; + globalThis.fetch = (async (url: string | URL | Request) => { + attempted.push(String(url)); + return new Response(null, { status: 200 }); + }) as typeof globalThis.fetch; + try { + const res = await app.request(`/checkout-sessions/${created.id}/complete`, { + method: "POST", + headers: agentHeader("ftp://platform.invalid/hook"), + body: JSON.stringify({ + payment: { + instruments: [ + { + id: "pi_1", + handler_id: "mock_payment_handler", + type: "card", + brand: "visa", + last_digits: "4242", + credential: { type: "network_token", token: "success_token" }, + }, + ], + }, + }), + }); + const text = await res.text(); + assert.equal(res.status, 200, text); + const body = JSON.parse(text) as { status: string }; + assert.equal(body.status, "completed"); + } finally { + globalThis.fetch = originalFetch; + } + assert.deepEqual( + attempted, + [], + "no webhook delivery may be attempted to a target that is not a URL" + ); +});