From 1173803c61c6de6938374edcfee1a54052656dc7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 24 Jun 2026 22:24:26 +0000 Subject: [PATCH 1/2] feat(webhooks): validate https url and normalise event list before submit The webhook registration form accepted any type="url" string and a free-text comma-separated events field with no validation, so http(s) URLs, javascript: schemes, and empty/duplicate event lists could be posted as-is. Validate the URL is https:// and normalise the events CSV (trim, drop empties, de-duplicate) before calling apiPost, with field-level errors surfaced through TextField. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_016Qm9y5cUE6ZMGicMztgekh --- README.md | 7 ++ src/app/webhooks/page.test.tsx | 205 +++++++++++++++++++++++++++++++++ src/app/webhooks/page.tsx | 85 ++++++++++---- 3 files changed, 273 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 3728cee..8c73e46 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,13 @@ Because the variable is `NEXT_PUBLIC_*`, its value is exposed to the browser. Ne A baseline security header set (CSP, `X-Frame-Options: DENY`, `Referrer-Policy`, `X-Content-Type-Options`, `Permissions-Policy`, HSTS) is wired up in `next.config.ts` via `src/lib/securityHeaders.ts`. The CSP `connect-src` directive tracks `NEXT_PUBLIC_AGENTPAY_API_BASE` automatically; `` links to external sites (`https://stellar.org`, etc.) remain navigable. +## Webhook registration validation + +The `/webhooks` form (`src/app/webhooks/page.tsx`) validates client-side before calling the API: + +- **URL**: only `https://` URLs are accepted. `http://`, `javascript:`, and other non-`https` schemes (or values that fail to parse as a URL at all) are rejected with a field-level error surfaced via `TextField`'s `error` prop (`aria-invalid="true"`, message linked through `aria-describedby`). The native `type="url"` / `required` attributes still apply and run first; the `https`-only check augments them. +- **Events**: the comma-separated events field is normalised before submit — each entry is trimmed, empty entries (including whitespace-only and trailing-comma artifacts) are dropped, and duplicates are removed while preserving first-seen order. Submit is blocked with a field error if the normalised list is empty. + ## Event log rendering The `/events` page renders server-supplied JSON payloads. Each payload is serialised through `safeStringify` (`src/lib/format.ts`) with a hard cap (`EVENT_PAYLOAD_MAX_CHARS`, default 5,000 chars) and a visible `…(truncated)` marker. Circular references, `BigInt`, functions, and malformed timestamps are replaced with safe sentinels so a bad payload can't crash the page. diff --git a/src/app/webhooks/page.test.tsx b/src/app/webhooks/page.test.tsx index 89a5154..9ba213b 100644 --- a/src/app/webhooks/page.test.tsx +++ b/src/app/webhooks/page.test.tsx @@ -13,6 +13,26 @@ function mockFetchSuccess() { } as unknown as Response); } +function urlField() { + return screen.getByRole("textbox", { name: /^url\b/i }) as HTMLInputElement; +} + +function eventsField() { + return screen.getByRole("textbox", { + name: /events \(comma-separated\)/i, + }) as HTMLInputElement; +} + +function registerButton() { + return screen.getByRole("button", { name: /^register$/i }); +} + +function postCalls(fetchMock: jest.Mock) { + return fetchMock.mock.calls.filter( + (c: unknown[]) => (c[1] as RequestInit | undefined)?.method === "POST" + ); +} + afterEach(() => jest.restoreAllMocks()); it("does not delete immediately when Remove is clicked", async () => { @@ -69,3 +89,188 @@ it("calls DELETE and closes dialog when confirmed", async () => { }); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); }); + +describe("URL validation", () => { + it("rejects http:// URLs with a field error and does not submit", async () => { + mockFetchSuccess(); + render(); + await screen.findByText("https://example.com/hook"); + const fetchMock = globalThis.fetch as jest.Mock; + fetchMock.mockClear(); + + fireEvent.change(urlField(), { target: { value: "http://example.com/hook" } }); + fireEvent.click(registerButton()); + + expect(await screen.findByText(/enter a valid https:\/\/ url/i)).toBeInTheDocument(); + expect(urlField()).toHaveAttribute("aria-invalid", "true"); + expect(postCalls(fetchMock)).toHaveLength(0); + }); + + it("rejects javascript: URLs with a field error and does not submit", async () => { + mockFetchSuccess(); + render(); + await screen.findByText("https://example.com/hook"); + const fetchMock = globalThis.fetch as jest.Mock; + fetchMock.mockClear(); + + fireEvent.change(urlField(), { target: { value: "javascript:alert(1)" } }); + fireEvent.click(registerButton()); + + expect(await screen.findByText(/enter a valid https:\/\/ url/i)).toBeInTheDocument(); + expect(postCalls(fetchMock)).toHaveLength(0); + }); + + it("rejects malformed URLs with a field error and does not submit", async () => { + mockFetchSuccess(); + render(); + await screen.findByText("https://example.com/hook"); + const fetchMock = globalThis.fetch as jest.Mock; + fetchMock.mockClear(); + + // Syntactically malformed values never reach our handler: the native + // `type="url"` constraint blocks the submit first (required attributes + // augment, not replace, our validation). + fireEvent.change(urlField(), { target: { value: "not-a-url" } }); + fireEvent.click(registerButton()); + + expect(urlField().checkValidity()).toBe(false); + expect(urlField().validity.typeMismatch).toBe(true); + expect(postCalls(fetchMock)).toHaveLength(0); + }); + + it("clears the URL error once the field is edited again", async () => { + mockFetchSuccess(); + render(); + await screen.findByText("https://example.com/hook"); + + fireEvent.change(urlField(), { target: { value: "http://example.com/hook" } }); + fireEvent.click(registerButton()); + expect(await screen.findByText(/enter a valid https:\/\/ url/i)).toBeInTheDocument(); + + fireEvent.change(urlField(), { target: { value: "https://example.com/hook" } }); + expect(screen.queryByText(/enter a valid https:\/\/ url/i)).not.toBeInTheDocument(); + expect(urlField()).toHaveAttribute("aria-invalid", "false"); + }); +}); + +describe("events normalisation", () => { + it("blocks submit when events are whitespace-only after trimming", async () => { + mockFetchSuccess(); + render(); + await screen.findByText("https://example.com/hook"); + const fetchMock = globalThis.fetch as jest.Mock; + fetchMock.mockClear(); + + fireEvent.change(urlField(), { target: { value: "https://example.com/hook" } }); + fireEvent.change(eventsField(), { target: { value: " , ,," } }); + fireEvent.click(registerButton()); + + expect(await screen.findByText(/enter at least one event name/i)).toBeInTheDocument(); + expect(eventsField()).toHaveAttribute("aria-invalid", "true"); + expect(postCalls(fetchMock)).toHaveLength(0); + }); + + it("blocks submit when the events field is an empty string via the required attribute", async () => { + mockFetchSuccess(); + render(); + await screen.findByText("https://example.com/hook"); + const fetchMock = globalThis.fetch as jest.Mock; + fetchMock.mockClear(); + + fireEvent.change(urlField(), { target: { value: "https://example.com/hook" } }); + fireEvent.change(eventsField(), { target: { value: "" } }); + fireEvent.click(registerButton()); + + // A fully empty value never reaches our handler: the `required` + // attribute blocks the submit natively before our normalisation runs. + expect(eventsField().checkValidity()).toBe(false); + expect(eventsField().validity.valueMissing).toBe(true); + expect(postCalls(fetchMock)).toHaveLength(0); + }); + + it("collapses duplicate event names, trims whitespace, and drops trailing commas before posting", async () => { + mockFetchSuccess(); + render(); + await screen.findByText("https://example.com/hook"); + const fetchMock = globalThis.fetch as jest.Mock; + fetchMock.mockClear(); + fetchMock + .mockResolvedValueOnce({ ok: true, status: 201, json: async () => ({ id: "wh_2" }) } as unknown as Response) + .mockResolvedValueOnce({ ok: true, status: 200, json: async () => ({ items: mockItems }) } as unknown as Response); + + fireEvent.change(urlField(), { target: { value: "https://example.com/new-hook" } }); + fireEvent.change(eventsField(), { + target: { value: " usage.recorded , usage.recorded ,usage.settled,, " }, + }); + fireEvent.click(registerButton()); + + await waitFor(() => expect(postCalls(fetchMock)).toHaveLength(1)); + const [, init] = postCalls(fetchMock)[0]; + const body = JSON.parse((init as RequestInit).body as string); + expect(body).toEqual({ + url: "https://example.com/new-hook", + events: ["usage.recorded", "usage.settled"], + }); + }); + + it("submits a valid https URL with a normalised event list", async () => { + mockFetchSuccess(); + render(); + await screen.findByText("https://example.com/hook"); + const fetchMock = globalThis.fetch as jest.Mock; + fetchMock.mockClear(); + fetchMock + .mockResolvedValueOnce({ ok: true, status: 201, json: async () => ({ id: "wh_2" }) } as unknown as Response) + .mockResolvedValueOnce({ ok: true, status: 200, json: async () => ({ items: mockItems }) } as unknown as Response); + + fireEvent.change(urlField(), { target: { value: "https://example.com/new-hook" } }); + fireEvent.change(eventsField(), { target: { value: "usage.recorded,usage.settled" } }); + fireEvent.click(registerButton()); + + await waitFor(() => expect(postCalls(fetchMock)).toHaveLength(1)); + const [url, init] = postCalls(fetchMock)[0]; + expect(url).toContain("/api/v1/webhooks"); + expect(JSON.parse((init as RequestInit).body as string)).toEqual({ + url: "https://example.com/new-hook", + events: ["usage.recorded", "usage.settled"], + }); + await waitFor(() => expect(urlField().value).toBe("")); + }); + + it("clears the events error once the field is edited again", async () => { + mockFetchSuccess(); + render(); + await screen.findByText("https://example.com/hook"); + + fireEvent.change(urlField(), { target: { value: "https://example.com/hook" } }); + fireEvent.change(eventsField(), { target: { value: " " } }); + fireEvent.click(registerButton()); + expect(await screen.findByText(/enter at least one event name/i)).toBeInTheDocument(); + + fireEvent.change(eventsField(), { target: { value: "usage.recorded" } }); + expect(screen.queryByText(/enter at least one event name/i)).not.toBeInTheDocument(); + expect(eventsField()).toHaveAttribute("aria-invalid", "false"); + }); +}); + +it("still allows removing a list item after a blocked submit", async () => { + mockFetchSuccess(); + render(); + await screen.findByText("https://example.com/hook"); + + fireEvent.change(urlField(), { target: { value: "http://bad" } }); + fireEvent.click(registerButton()); + expect(await screen.findByText(/enter a valid https:\/\/ url/i)).toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: /^remove$/i })); + expect(screen.getByRole("dialog")).toBeInTheDocument(); +}); + +it("keeps the required attribute on both fields", async () => { + mockFetchSuccess(); + render(); + await screen.findByText("https://example.com/hook"); + + expect(urlField()).toBeRequired(); + expect(eventsField()).toBeRequired(); +}); diff --git a/src/app/webhooks/page.tsx b/src/app/webhooks/page.tsx index e522772..c659770 100644 --- a/src/app/webhooks/page.tsx +++ b/src/app/webhooks/page.tsx @@ -3,13 +3,40 @@ import { useEffect, useState } from "react"; import { apiGet, apiPost, apiDelete } from "@/lib/apiClient"; import { ConfirmDialog } from "@/components/ConfirmDialog"; +import { TextField } from "@/components/TextField"; type Webhook = { id: string; url: string; events: string[]; createdAt: number }; +// Only https:// targets are accepted — rejects http:// (cleartext) and +// non-network schemes like javascript: outright. +function isHttpsUrl(value: string): boolean { + try { + return new URL(value).protocol === "https:"; + } catch { + return false; + } +} + +// Trims each entry, drops empties (so "a,,b," and "" don't produce blank +// event names), and de-duplicates while preserving first-seen order. +function normaliseEvents(csv: string): string[] { + const seen = new Set(); + const result: string[] = []; + for (const raw of csv.split(",")) { + const trimmed = raw.trim(); + if (!trimmed || seen.has(trimmed)) continue; + seen.add(trimmed); + result.push(trimmed); + } + return result; +} + export default function WebhooksPage() { const [items, setItems] = useState(null); const [url, setUrl] = useState(""); const [eventsCsv, setEventsCsv] = useState("usage.recorded,usage.settled"); + const [urlError, setUrlError] = useState(null); + const [eventsError, setEventsError] = useState(null); const [error, setError] = useState(null); const [pendingRemove, setPendingRemove] = useState(null); @@ -24,10 +51,17 @@ export default function WebhooksPage() { const onCreate = async (e: React.FormEvent) => { e.preventDefault(); setError(null); - const events = eventsCsv - .split(",") - .map((s) => s.trim()) - .filter(Boolean); + + const events = normaliseEvents(eventsCsv); + const nextUrlError = isHttpsUrl(url) + ? null + : "Enter a valid https:// URL."; + const nextEventsError = + events.length === 0 ? "Enter at least one event name." : null; + setUrlError(nextUrlError); + setEventsError(nextEventsError); + if (nextUrlError || nextEventsError) return; + try { await apiPost("/api/v1/webhooks", { url, events }); setUrl(""); @@ -65,26 +99,29 @@ export default function WebhooksPage() { />

Webhooks

- - + { + setUrl(e.target.value); + if (urlError) setUrlError(null); + }} + placeholder="https://example.com/agentpay-hook" + error={urlError ?? undefined} + description={urlError ? undefined : "Must be an https:// URL."} + /> + { + setEventsCsv(e.target.value); + if (eventsError) setEventsError(null); + }} + error={eventsError ?? undefined} + />