trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns16g rEventsFlows

Webhooks only reach public https addresses: one outbound guard resolves the host and refuses private ones #2998

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/2992-webhooks-only-reach-public-https-address → main 3 commits fork 4 threads · 4 unresolved

  • Merged. scout/2992-webhooks-only-reach-public-https-address was merged into main as 708d1fa.

Review

Sign in to review this change.
13 files changed +474 −69
README.md +4 −1
@@ -1596,7 +1596,10 @@ work here.
15961596 say what the local client keeps cloned.
15971597 - **Automation** — a JSON API covering repos, issues, forks, and orgs (the
15981598 label-as-queue surface), a durable event outbox, and **webhooks** (HMAC-signed
1599 delivery with retry).
1599 delivery with retry). Every send goes through `src/kernel/outbound.ts`:
1600 - https URLs only; the host is resolved, private addresses are refused, and no redirect is followed.
1601 - A refused delivery fails at once, with the reason shown on the hooks page.
1602 - An unreachable or erroring endpoint is retried with growing delay.
16001603 - **Where an account came from** — `GET /api/v1/admin/accounts` lists each
16011604 account with `via` (`signup`, `edge`, or null) and `signsInThrough`
16021605 (`"open-platform.sh"` for an account the platform edge made, else null);
src/core/migrations/20261020090000-webhook-refusal.ts added +5
@@ -0,0 +1,5 @@
1 // Why a webhook delivery was not sent at all (#2992): "refused: private
2 // address" is no response status, so it has a column of its own.
3 export default `
4 ALTER TABLE webhook_delivery ADD COLUMN last_error TEXT;
5 `;
src/core/webhooks.ts +43 −19
@@ -13,6 +13,11 @@ import type { Database } from "bun:sqlite";
1313 import { defect } from "../kernel/error.ts";
1414 import { id, secret as randomSecret } from "../kernel/id.ts";
1515 import type { Log } from "../kernel/log.ts";
16 import {
17 outboundFetch,
18 RefusedError,
19 type OutboundOptions,
20 } from "../kernel/outbound.ts";
1621 import { err, ok, type Result } from "../kernel/result.ts";
1722 import type { ForgeEvent } from "./events.ts";
1823
@@ -68,8 +73,8 @@ export function createWebhook(
6873 input: CreateWebhookInput,
6974 ): Result<{ webhook: Webhook; secret: string }, CreateWebhookError> {
7075 const url = input.url.trim();
71 // Only http(s), and never a bare/relative URL — this value is fetched.
72 if (!/^https?:\/\/.+/i.test(url)) return err("invalid_url");
76 // Only https, and never a bare/relative URL — this value is fetched.
77 if (!/^https:\/\/.+/i.test(url)) return err("invalid_url");
7378 try {
7479 // eslint-disable-next-line no-new
7580 new URL(url);
@@ -190,6 +195,7 @@ export async function deliverDue(
190195 db: Database,
191196 log: Log,
192197 now = new Date(),
198 outbound: OutboundOptions = {},
193199 ): Promise<number> {
194200 const nowIso = now.toISOString();
195201 const due = db
@@ -226,7 +232,7 @@ export async function deliverDue(
226232 );
227233 if (claimed.changes !== 1) continue;
228234 try {
229 await deliverOne(db, log, row, now);
235 await deliverOne(db, log, row, now, outbound);
230236 } catch (error) {
231237 // One malformed delivery must not take the batch with it. Without
232238 // this, a single unsendable row stops every delivery behind it — and
@@ -249,6 +255,7 @@ async function deliverOne(
249255 log: Log,
250256 row: DueRow,
251257 now: Date,
258 outbound: OutboundOptions,
252259 ): Promise<void> {
253260 const body = JSON.stringify({
254261 id: row.event_id,
@@ -263,23 +270,36 @@ async function deliverOne(
263270
264271 let status = 0;
265272 try {
266 const response = await fetch(row.url, {
267 method: "POST",
268 headers: {
269 "content-type": "application/json",
270 "user-agent": "forge-webhook",
271 "x-forge-event": row.kind,
272 "x-forge-delivery": row.id,
273 ...(signature === ""
274 ? {}
275 : { "x-forge-signature-256": `sha256=${signature}` }),
273 const response = await outboundFetch(
274 row.url,
junipercrew outdated

The fetch options are not re-indented after wrapping the call, so the block does not match the file. Format it.
275 {
276 method: "POST",
277 headers: {
278 "content-type": "application/json",
279 "user-agent": "forge-webhook",
280 "x-forge-event": row.kind,
281 "x-forge-delivery": row.id,
282 ...(signature === ""
283 ? {}
284 : { "x-forge-signature-256": `sha256=${signature}` }),
285 },
286 body,
287 signal: AbortSignal.timeout(DELIVER_TIMEOUT_MS),
288 redirect: "manual",
276289 },
277 body,
278 signal: AbortSignal.timeout(DELIVER_TIMEOUT_MS),
279 redirect: "manual",
280 });
290 outbound,
291 );
281292 status = response.status;
282293 } catch (error) {
junipercrew outdated

A refused delivery goes through backOff and is retried five times with growing delay, though a private address will not turn public in seconds. Consider failing it at once. At minimum, say in the README that it is retried.
294 if (error instanceof RefusedError) {
295 // A private address or a plain-http URL will not mend itself in
296 // seconds, so there is nothing to retry: fail it now, with the reason.
297 log.warn("webhook delivery refused", { delivery: row.id });
298 db.query(
299 "UPDATE webhook_delivery SET state = 'failed', attempts = ?1, last_status = NULL, last_error = ?2 WHERE id = ?3",
300 ).run(attempt, error.message, row.id);
301 return;
302 }
283303 log.warn("webhook delivery error", {
284304 delivery: row.id,
285305 error: String(error),
@@ -289,7 +309,7 @@ async function deliverOne(
289309 const ok2xx = status >= 200 && status < 300;
290310 if (ok2xx) {
291311 db.query(
292 "UPDATE webhook_delivery SET state = 'ok', attempts = ?1, last_status = ?2 WHERE id = ?3",
312 "UPDATE webhook_delivery SET state = 'ok', attempts = ?1, last_status = ?2, last_error = NULL WHERE id = ?3",
293313 ).run(attempt, status, row.id);
294314 return;
295315 }
@@ -368,6 +388,8 @@ export interface DeliveryInfo {
368388 readonly state: "pending" | "ok" | "failed";
369389 readonly attempts: number;
370390 readonly lastStatus: number | null;
391 /** Why nothing was sent ("refused: private address"), when nothing was. */
392 readonly lastError: string | null;
371393 readonly kind: string;
372394 }
373395
@@ -383,11 +405,12 @@ export function recentDeliveries(
383405 state: "pending" | "ok" | "failed";
384406 attempts: number;
385407 last_status: number | null;
408 last_error: string | null;
386409 kind: string;
387410 },
388411 [string, number]
389412 >(
390 `SELECT d.id, d.state, d.attempts, d.last_status, e.kind
413 `SELECT d.id, d.state, d.attempts, d.last_status, d.last_error, e.kind
391414 FROM webhook_delivery d JOIN event e ON e.id = d.event_id
392415 WHERE d.webhook_id = ?1 ORDER BY d.id DESC LIMIT ?2`,
393416 )
@@ -397,6 +420,7 @@ export function recentDeliveries(
397420 state: r.state,
398421 attempts: r.attempts,
399422 lastStatus: r.last_status,
423 lastError: r.last_error,
400424 kind: r.kind,
401425 }));
402426 }
src/crew/notify.ts +17 −14
@@ -23,7 +23,7 @@
2323 import { newAccountLines } from "../actions/accounts.ts";
2424 import { mailCeilingLines } from "../actions/mail.ts";
2525 import type { Database } from "bun:sqlite";
26 import { lookup } from "node:dns/promises";
26 import { outboundFetch } from "../kernel/outbound.ts";
2727 import type { App } from "../app.ts";
2828 import { isPrivateHost } from "../actions/migrate.ts";
2929 import { getUserSetting, setUserSetting } from "../core/settings.ts";
@@ -141,20 +141,23 @@ export type Sender = (
141141 headers: Record<string, string>,
142142 ) => Promise<boolean>;
143143
144 /** The real sender: the name is resolved and judged first, no redirect is followed. */
145 const post: Sender = async (url, body, headers) => {
146 const host = new URL(url).hostname;
147 const found = await lookup(host, { all: true });
148 if (found.length === 0 || found.some((a) => isPrivateHost(a.address)))
144 /**
145 * The real sender: the name is resolved and judged first, no redirect is
146 * followed, and only https is spoken. A refusal or a transport error is a
147 * message not sent, never a throw.
148 */
149 export const post: Sender = async (url, body, headers) => {
150 try {
151 const res = await outboundFetch(url, {
152 method: "POST",
153 body,
154 headers: { "content-type": "text/plain; charset=utf-8", ...headers },
155 signal: AbortSignal.timeout(SEND_TIMEOUT_MS),
156 });
157 return res.status >= 200 && res.status < 300;
158 } catch {
149159 return false;
150 const res = await fetch(url, {
151 method: "POST",
152 body,
153 headers: { "content-type": "text/plain; charset=utf-8", ...headers },
154 redirect: "manual",
155 signal: AbortSignal.timeout(SEND_TIMEOUT_MS),
156 });
157 return res.status >= 200 && res.status < 300;
160 }
158161 };
159162
160163 /**
src/kernel/outbound.ts added +110
@@ -0,0 +1,110 @@
1 /**
2 * The one guard for anything the forge fetches on somebody else's say-so.
3 * The name is resolved here, every answer is judged, and the request goes to
4 * the address that was judged — the name stays in `Host` and as the TLS
5 * server name — so a second lookup cannot hand back a different answer.
6 * Redirects are never followed: a 302 is an answer, not an instruction.
7 */
8
9 import { lookup } from "node:dns/promises";
10 import { isIP } from "node:net";
11 import { err, ok, type Result } from "./result.ts";
12 import { addressBytes, inCidrs, parseCidr, type Cidr } from "./cidr.ts";
13
14 const REFUSED = [
15 "0.0.0.0/8",
16 "10.0.0.0/8",
17 "100.64.0.0/10",
18 "127.0.0.0/8",
19 "169.254.0.0/16",
20 "172.16.0.0/12",
21 "192.168.0.0/16",
22 "224.0.0.0/4",
23 "::/128",
24 "::1/128",
25 "fc00::/7",
26 "fe80::/10",
27 "ff00::/8",
28 ].map((c) => parseCidr(c)!) satisfies Cidr[];
29
30 /** Is this address one a public service would never have? Unparseable counts. */
31 export function isRefusedAddress(address: string): boolean {
32 return addressBytes(address) === undefined || inCidrs(address, REFUSED);
33 }
34
35 export type Resolve = (host: string) => Promise<readonly string[]>;
36
37 export interface OutboundOptions {
38 /** Name → addresses. Defaults to the system resolver. Injected by tests. */
39 resolve?: Resolve;
40 /** Addresses exempt from the refusal list. For tests only; never read from the environment. */
41 allow?: readonly string[];
42 /** Permit plain http. For tests only; never read from the environment. */
43 plain?: boolean;
44 /** Extra trusted CA certificate (PEM) for https. For tests only. */
45 ca?: string | undefined;
46 }
47
48 const systemResolve: Resolve = async (host) =>
49 (await lookup(host, { all: true })).map((a) => a.address);
50
51 export const REFUSED_SENTENCE = "refused: private address";
52 export const PLAIN_SENTENCE = "refused: not https";
53
54 export class RefusedError extends Error {
55 constructor(sentence = REFUSED_SENTENCE) {
56 super(sentence);
57 }
58 }
59
60 /** The addresses a host resolves to, or an error when any one of them is private. */
61 export async function vetHost(
62 host: string,
63 options: OutboundOptions = {},
64 ): Promise<Result<string[], "refused" | "unresolved">> {
65 const bare = host.replace(/^\[|\]$/g, "");
66 let found: readonly string[];
67 if (isIP(bare) !== 0) found = [bare];
68 else {
69 try {
70 found = await (options.resolve ?? systemResolve)(bare);
71 } catch {
72 return err("unresolved");
73 }
74 }
75 if (found.length === 0) return err("unresolved");
76 const allow = options.allow ?? [];
77 if (found.some((a) => isRefusedAddress(a) && !allow.includes(a)))
78 return err("refused");
79 return ok([...found]);
80 }
81
82 /** fetch() to the vetted address; throws RefusedError when the host is private. */
83 export async function outboundFetch(
84 url: string,
85 init: RequestInit = {},
86 options: OutboundOptions = {},
87 ): Promise<Response> {
88 const target = new URL(url);
89 if (target.protocol !== "https:" && !options.plain)
90 throw new RefusedError(PLAIN_SENTENCE);
91 const vetted = await vetHost(target.hostname, options);
92 if (!vetted.ok) {
93 if (vetted.error === "refused") throw new RefusedError();
94 throw new Error("unresolved host");
95 }
96 const ip = vetted.value[0]!;
97 const name = target.hostname.replace(/^\[|\]$/g, "");
98 const pinned = new URL(url);
99 pinned.hostname = ip.includes(":") ? `[${ip}]` : ip;
junipercrew outdated

outboundFetch does not enforce https. Webhook rows saved as http before this change still send over plain http to public hosts. Either have the webhook delivery refuse non-https URLs or migrate those rows.
100 const headers = new Headers(init.headers);
101 headers.set("host", target.host);
102 return fetch(pinned, {
103 ...init,
104 headers,
105 redirect: "manual",
106 ...(target.protocol === "https:"
107 ? { tls: { serverName: name, ...(options.ca ? { ca: options.ca } : {}) } }
108 : {}),
109 } satisfies BunFetchRequestInit);
110 }
src/web/pages/settings.ts +1 −1
@@ -1640,7 +1640,7 @@ export function hooksPage(ctx: Ctx): Response {
16401640 ${d.state}
16411641 </td>
16421642 <td>${d.attempts}</td>
1643 <td>${d.lastStatus ?? "—"}</td>
1643 <td>${d.lastError ?? d.lastStatus ?? "—"}</td>
junipercrew

Add a test for this cell. The refusal sentence is shown here and it is the only place a person sees why nothing arrived.
16441644 </tr>`,
16451645 )}
16461646 </tbody>
test/agents.test.ts +4 −1
@@ -970,7 +970,10 @@ describe("git process limits and timeouts", () => {
970970
971971 const { deliverDue } = await import("../src/core/webhooks.ts");
972972 const started = Date.now();
973 await deliverDue(app.db, app.log);
973 await deliverDue(app.db, app.log, new Date(), {
974 allow: ["127.0.0.1", "::1"],
975 plain: true,
976 });
974977 const elapsed = Date.now() - started;
975978 // The delivery timeout is 10s; anything under 30 proves a signal fired
976979 // rather than the request hanging on forever.
test/helpers/loopback-hook.ts added +16
@@ -0,0 +1,16 @@
1 /**
2 * Test receivers listen on loopback over plain http, which the forge refuses
3 * to save and refuses to send to. These two get a test around that without
4 * any switch production could carry: a row inserted past `createWebhook`, and
5 * an allow-list passed to `deliverDue` as an argument.
6 */
7 import type { App } from "../../src/app.ts";
8 import { id } from "../../src/kernel/id.ts";
9
10 export const LOOPBACK = { allow: ["127.0.0.1", "::1"], plain: true } as const;
11
12 export function plainHook(app: App, repoId: string, url: string, kinds = "*"): void {
13 app.db
14 .query("INSERT INTO webhook (id, repo_id, url, secret, kinds) VALUES (?1, ?2, ?3, 'test-secret', ?4)")
15 .run(id(), repoId, url, kinds);
16 }
test/helpers/overlap-worker.ts +2 −1
@@ -14,6 +14,7 @@ import { getAgentByName } from "../../src/core/agents.ts";
1414 import { getIssue } from "../../src/core/issues.ts";
1515 import { getRepo } from "../../src/core/repos.ts";
1616 import { deliverDue } from "../../src/core/webhooks.ts";
17 import { LOOPBACK } from "./loopback-hook.ts";
1718 import { queueAgentRun } from "../../src/crew/dispatch.ts";
1819
1920 const [mode, dataDir, startAt, count] = process.argv.slice(2);
@@ -30,7 +31,7 @@ if (mode === "queue") {
3031 } else {
3132 const until = Date.now() + 3000;
3233 while (Date.now() < until) {
33 const sent = await deliverDue(app.db, app.log);
34 const sent = await deliverDue(app.db, app.log, new Date(), LOOPBACK);
3435 done += sent;
3536 if (sent === 0) await Bun.sleep(20);
3637 }
test/notify.test.ts +8
@@ -15,6 +15,7 @@ import type { Repo } from "../src/core/repos.ts";
1515 import { claimRun, createRun, finishRun } from "../src/core/runs.ts";
1616 import { createUser, type User } from "../src/core/users.ts";
1717 import { serve, type Servers } from "../src/http/server.ts";
18 import { post } from "../src/crew/notify.ts";
1819 import {
1920 notifyHost,
2021 notifyUrlOf,
@@ -161,6 +162,13 @@ describe("notify", () => {
161162 });
162163 });
163164
165 describe("the real sender", () => {
166 test("a plain-http or private URL is a message not sent, not a throw", async () => {
167 expect(await post("http://ntfy.example.test/t", "x", {})).toBe(false);
168 expect(await post("https://127.0.0.1:9/t", "x", {})).toBe(false);
169 });
170 });
171
164172 describe("the notify URL", () => {
165173 test("an http: URL is refused; so are credentials, long, private and loopback hosts", () => {
166174 const refused = (u: string) => {
test/outbound.test.ts added +148
@@ -0,0 +1,148 @@
1 import { afterAll, describe, expect, test } from "bun:test";
2 import { mkdtempSync, readFileSync } from "node:fs";
3 import { tmpdir } from "node:os";
4 import { join } from "node:path";
5 import { outboundFetch, vetHost, type Resolve } from "../src/kernel/outbound.ts";
6
7 const to = (...a: string[]): Resolve => () => Promise.resolve(a);
8
9 describe("vetHost", () => {
10 for (const addr of [
11 "127.0.0.1",
12 "10.0.0.1",
13 "172.16.5.5",
14 "192.168.1.1",
15 "169.254.169.254",
16 "100.64.0.1",
17 "0.0.0.0",
18 "224.0.0.1",
19 "::1",
20 "::",
21 "fe80::1",
22 "fd00::1",
23 "ff02::1",
24 "::ffff:127.0.0.1",
25 "::ffff:10.0.0.1",
26 ])
27 test(`refuses ${addr}`, async () => {
28 const r = await vetHost(addr);
29 expect(r.ok).toBe(false);
30 if (!r.ok) expect(r.error).toBe("refused");
31 });
32
33 test("refuses a public name that resolves to 127.0.0.1", async () => {
34 const r = await vetHost("hooks.example.com", { resolve: to("127.0.0.1") });
35 expect(r).toEqual({ ok: false, error: "refused" });
36 });
37
38 test("refuses when any one answer is private", async () => {
39 const r = await vetHost("x.example.com", { resolve: to("93.184.216.34", "10.0.0.1") });
40 expect(r.ok).toBe(false);
41 });
42
43 test("allows a public address", async () => {
44 const r = await vetHost("hooks.example.com", { resolve: to("93.184.216.34") });
45 expect(r).toEqual({ ok: true, value: ["93.184.216.34"] });
46 });
47
48 test("a failed lookup is unresolved, not allowed", async () => {
49 const r = await vetHost("nope.example.com", { resolve: () => Promise.reject(new Error("x")) });
50 expect(r).toEqual({ ok: false, error: "unresolved" });
51 });
52 });
53
54 describe("outboundFetch", () => {
55 const seen: { host: string | null; path: string }[] = [];
56 const server = Bun.serve({
57 port: 0,
58 fetch(req) {
59 seen.push({ host: req.headers.get("host"), path: new URL(req.url).pathname });
60 if (new URL(req.url).pathname === "/redir")
61 return new Response(null, { status: 302, headers: { location: "http://10.0.0.1/secret" } });
62 return new Response("ok");
63 },
64 });
65 afterAll(() => server.stop(true));
66 const allow = ["127.0.0.1"];
67
68 test("throws before connecting to a private address", async () => {
69 const before = seen.length;
70 await expect(
71 outboundFetch(`http://hooks.example.com:${server.port}/x`, {}, { resolve: to("127.0.0.1"), plain: true }),
72 ).rejects.toThrow("refused: private address");
73 expect(seen.length).toBe(before);
74 });
75
76 test("connects to the vetted address and keeps the name in Host", async () => {
77 const res = await outboundFetch(
78 `http://hooks.example.com:${server.port}/x`,
79 {},
80 { resolve: to("127.0.0.1"), allow, plain: true },
81 );
82 expect(res.status).toBe(200);
83 expect(seen.at(-1)).toEqual({ host: `hooks.example.com:${server.port}`, path: "/x" });
84 });
85
86 test("a 302 to a private address is returned, not followed", async () => {
87 const res = await outboundFetch(
88 `http://hooks.example.com:${server.port}/redir`,
89 {},
90 { resolve: to("127.0.0.1"), allow, plain: true },
91 );
92 expect(res.status).toBe(302);
93 expect(seen.filter((s) => s.path === "/secret")).toHaveLength(0);
94 });
95 });
96
97 /** A throwaway self-signed certificate for `name`, made with openssl so no key is committed. */
98 function selfSigned(name: string): { cert: string; key: string } | undefined {
99 const dir = mkdtempSync(join(tmpdir(), "forge-cert-"));
100 const run = Bun.spawnSync([
101 "openssl", "req", "-x509", "-newkey", "ec", "-pkeyopt", "ec_paramgen_curve:prime256v1",
102 "-nodes", "-keyout", join(dir, "k.pem"), "-out", join(dir, "c.pem"), "-days", "2",
103 "-subj", `/CN=${name}`, "-addext", `subjectAltName=DNS:${name}`,
104 ]);
105 if (run.exitCode !== 0) return undefined;
106 return { cert: readFileSync(join(dir, "c.pem"), "utf8"), key: readFileSync(join(dir, "k.pem"), "utf8") };
107 }
108
109 describe("outboundFetch over https", () => {
110 const pair = selfSigned("hooks.example.com");
111 const t = pair ? test : test.skip;
112 const servers: ReturnType<typeof Bun.serve>[] = [];
113 afterAll(() => servers.forEach((s) => s.stop(true)));
114 const serveTls = () => {
115 const s = Bun.serve({ port: 0, tls: { cert: pair!.cert, key: pair!.key }, fetch: () => new Response("secure") });
116 servers.push(s);
117 return s;
118 };
119 const opts = { resolve: to("127.0.0.1"), allow: ["127.0.0.1"], ca: pair?.cert };
120
121 t("connects to the judged address and checks the certificate against the name", async () => {
122 const s = serveTls();
123 const res = await outboundFetch(`https://hooks.example.com:${s.port}/x`, {}, opts);
124 expect(await res.text()).toBe("secure");
125 });
126
127 t("fails when the certificate is not for the name", async () => {
128 const s = serveTls();
129 await expect(outboundFetch(`https://other.example.com:${s.port}/x`, {}, opts)).rejects.toThrow();
130 });
131
132 t("is not trusted without the CA", async () => {
133 const s = serveTls();
134 await expect(
135 outboundFetch(`https://hooks.example.com:${s.port}/x`, {}, { ...opts, ca: undefined }),
136 ).rejects.toThrow();
137 });
138 });
139
140 describe("outboundFetch refuses plain http", () => {
141 test("before resolving anything", async () => {
142 let looked = false;
143 await expect(
144 outboundFetch("http://hooks.example.com/x", {}, { resolve: () => ((looked = true), to("93.184.216.34")("")) }),
145 ).rejects.toThrow("refused: not https");
146 expect(looked).toBe(false);
147 });
148 });
test/overlap.test.ts +5 −15
@@ -16,10 +16,10 @@ import { createApp, type App } from "../src/app.ts";
1616 import { createUser } from "../src/core/users.ts";
1717 import {
1818 CLAIM_MS,
19 createWebhook,
2019 DELIVER_TIMEOUT_MS,
2120 deliverDue,
2221 } from "../src/core/webhooks.ts";
22 import { LOOPBACK, plainHook } from "./helpers/loopback-hook.ts";
2323 import { serve } from "../src/http/server.ts";
2424 import { drainStreams } from "../src/http/sse.ts";
2525
@@ -97,12 +97,7 @@ describe("two processes on one data directory start each run once", () => {
9797 },
9898 });
9999 try {
100 const made = createWebhook(app.db, {
101 repoId,
102 url: `http://127.0.0.1:${receiver.port}/hook`,
103 kinds: "",
104 });
105 if (!made.ok) throw new Error(String(made.error));
100 plainHook(app, repoId, `http://127.0.0.1:${receiver.port}/hook`);
106101 for (let n = 0; n < 60; n++)
107102 app.bus.append({ kind: "push", repoId, payload: { n } });
108103 const pending = (
@@ -150,19 +145,14 @@ describe("two processes on one data directory start each run once", () => {
150145 },
151146 });
152147 try {
153 const made = createWebhook(app.db, {
154 repoId,
155 url: `http://127.0.0.1:${receiver.port}/slow`,
156 kinds: "",
157 });
158 if (!made.ok) throw new Error(String(made.error));
148 plainHook(app, repoId, `http://127.0.0.1:${receiver.port}/slow`);
159149 for (let n = 0; n < 4; n++)
160150 app.bus.append({ kind: "push", repoId, payload: { slow: n } });
161151 // A batch that began a while ago, as one behind a slow endpoint has:
162152 // its claims must still be leased from the moment each is taken.
163 const behind = deliverDue(app.db, app.log, new Date(Date.now() - 2 * CLAIM_MS));
153 const behind = deliverDue(app.db, app.log, new Date(Date.now() - 2 * CLAIM_MS), LOOPBACK);
164154 await Bun.sleep(100);
165 const beside = deliverDue(app.db, app.log);
155 const beside = deliverDue(app.db, app.log, new Date(), LOOPBACK);
166156 await Promise.all([behind, beside]);
167157 expect(hits.size).toBe(4);
168158 expect([...hits.values()].every((n) => n === 1)).toBe(true);
test/webhooks.test.ts +111 −17
@@ -1,10 +1,14 @@
11 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
2 import { mkdtempSync } from "node:fs";
23 import { mkdtemp, rm } from "node:fs/promises";
34 import { tmpdir } from "node:os";
45 import { join } from "node:path";
56 import { createApp, type App } from "../src/app.ts";
67 import { createRepo } from "../src/core/repos.ts";
78 import { createUser } from "../src/core/users.ts";
9 import { createSession } from "../src/core/sessions.ts";
10 import { serve } from "../src/http/server.ts";
11 import { LOOPBACK, plainHook } from "./helpers/loopback-hook.ts";
812 import {
913 createWebhook,
1014 deliverDue,
@@ -80,13 +84,8 @@ describe("webhook delivery", () => {
8084
8185 test("a matching event is delivered, signed, and verifiable", async () => {
8286 received = [];
83 const { secret } = unwrap(
84 createWebhook(app.db, {
85 repoId,
86 url: `${receiverUrl}/ok`,
87 kinds: "push",
88 }),
89 );
87 plainHook(app, repoId, `${receiverUrl}/ok`, "push");
88 const secret = "test-secret";
9089 // A matching event enqueues a delivery via the bus subscriber.
9190 app.bus.append({
9291 kind: "push",
@@ -102,7 +101,7 @@ describe("webhook delivery", () => {
102101 payload: {},
103102 });
104103
105 const attempted = await deliverDue(app.db, app.log);
104 const attempted = await deliverDue(app.db, app.log, new Date(), LOOPBACK);
106105 expect(attempted).toBe(1); // only the push matched "push"
107106 expect(received).toHaveLength(1);
108107 expect(received[0]!.event).toBe("push");
@@ -116,11 +115,11 @@ describe("webhook delivery", () => {
116115 });
117116
118117 test("a failing endpoint retries then gives up, without losing the row", async () => {
119 createWebhook(app.db, { repoId, url: `${receiverUrl}/fail`, kinds: "tag" });
118 plainHook(app, repoId, `${receiverUrl}/fail`, "tag");
120119 app.bus.append({ kind: "tag", repoId, actorId: userId, payload: {} });
121120
122121 // First attempt fails → row goes pending with a future next_try_at.
123 await deliverDue(app.db, app.log);
122 await deliverDue(app.db, app.log, new Date(), LOOPBACK);
124123 let row = app.db
125124 .query<
126125 { state: string; attempts: number; next_try_at: string | null },
@@ -133,7 +132,7 @@ describe("webhook delivery", () => {
133132
134133 // Drive it to exhaustion by delivering far in the future each time.
135134 for (let i = 0; i < 6; i++) {
136 await deliverDue(app.db, app.log, new Date(Date.now() + i * 3_600_000));
135 await deliverDue(app.db, app.log, new Date(Date.now() + i * 3_600_000), LOOPBACK);
137136 }
138137 const failed = app.db
139138 .query<
@@ -147,11 +146,7 @@ describe("webhook delivery", () => {
147146 test("a flaky endpoint eventually succeeds on retry", async () => {
148147 received = [];
149148 failCount = 0;
150 createWebhook(app.db, {
151 repoId,
152 url: `${receiverUrl}/flaky`,
153 kinds: "flaky.*",
154 });
149 plainHook(app, repoId, `${receiverUrl}/flaky`, "flaky.*");
155150 app.bus.append({
156151 kind: "flaky.event",
157152 repoId,
@@ -162,7 +157,7 @@ describe("webhook delivery", () => {
162157 // 503, 503, then 200 — deliver repeatedly with advancing time.
163158 let ok = false;
164159 for (let i = 0; i < 5 && !ok; i++) {
165 await deliverDue(app.db, app.log, new Date(Date.now() + i * 3_600_000));
160 await deliverDue(app.db, app.log, new Date(Date.now() + i * 3_600_000), LOOPBACK);
166161 const row = app.db
167162 .query<
168163 { state: string },
@@ -176,6 +171,105 @@ describe("webhook delivery", () => {
176171 });
177172 });
178173
174 describe("webhook urls and refused deliveries", () => {
175 test("only https is saved, with invalid_url", () => {
176 const app = createApp({ dataDir: tmp(), port: 0 });
177 try {
178 const r = createWebhook(app.db, { repoId: "r", url: "http://example.com/h" });
179 expect(r.ok).toBe(false);
180 if (!r.ok) expect(r.error).toBe("invalid_url");
181 } finally {
182 app.close();
183 }
184 });
185
186 test("a delivery to a private address is refused and records no status", async () => {
187 const app = createApp({ dataDir: tmp(), port: 0 });
188 try {
189 app.db.query("INSERT INTO user (id, username, email, password_hash) VALUES ('u1', 'w', '[email protected]', 'x')").run();
190 app.db.query("INSERT INTO repo (id, owner_id, name) VALUES ('r1', 'u1', 'h')").run();
191 plainHook(app, "r1", "https://127.0.0.1:9/h");
192 app.bus.append({ kind: "push", repoId: "r1" });
193 await deliverDue(app.db, app.log);
194 const row = app.db
195 .query<{ last_status: number | null; last_error: string | null; attempts: number }, []>(
196 "SELECT last_status, last_error, attempts FROM webhook_delivery",
197 )
198 .get();
199 expect(row?.last_status).toBeNull();
200 expect(row?.last_error).toBe("refused: private address");
201 expect(row?.attempts).toBe(1);
202 } finally {
203 app.close();
204 }
205 });
206 test("a refusal fails at once, and the hooks page says why", async () => {
207 const app = createApp({ dataDir: tmp(), port: 0 });
208 const servers = await serve(app);
209 try {
210 const user = unwrap(
211 await createUser(app.db, { origin: "seed", trusted: true, username: "shown", email: "[email protected]", password: "shown-in-settings" }),
212 );
213 const repo = unwrap(await createRepo(app.db, app.reposRoot, { ownerId: user.id, name: "h" }));
214 unwrap(createWebhook(app.db, { repoId: repo.id, url: "https://127.0.0.1:9/h" }));
215 app.bus.append({ kind: "push", repoId: repo.id });
216 await deliverDue(app.db, app.log);
217 const row = app.db
218 .query<{ state: string; attempts: number }, []>("SELECT state, attempts FROM webhook_delivery")
219 .get();
220 expect(row).toEqual({ state: "failed", attempts: 1 });
221 const cookie = `forge_session=${createSession(app.db, user.id).token}`;
222 const page = await fetch(`${servers.url}/shown/h/settings/hooks`, { headers: { cookie } });
223 expect(page.status).toBe(200);
224 expect(await page.text()).toContain("refused: private address");
225 } finally {
226 servers.stop();
227 app.close();
228 }
229 });
230
231 test("a delivery that succeeds clears a stale last_error", async () => {
232 const app = createApp({ dataDir: tmp(), port: 0 });
233 const receiver = Bun.serve({ port: 0, fetch: () => new Response("ok") });
234 try {
235 app.db.query("INSERT INTO user (id, username, email, password_hash) VALUES ('u1', 'w', '[email protected]', 'x')").run();
236 app.db.query("INSERT INTO repo (id, owner_id, name) VALUES ('r1', 'u1', 'h')").run();
237 plainHook(app, "r1", `http://127.0.0.1:${receiver.port}/h`);
238 app.bus.append({ kind: "push", repoId: "r1" });
239 app.db.query("UPDATE webhook_delivery SET last_error = 'old'").run();
240 await deliverDue(app.db, app.log, new Date(), LOOPBACK);
241 const row = app.db
242 .query<{ state: string; last_error: string | null }, []>("SELECT state, last_error FROM webhook_delivery")
243 .get();
244 expect(row).toEqual({ state: "ok", last_error: null });
245 } finally {
246 receiver.stop(true);
247 app.close();
248 }
249 });
250
251 test("a hook saved as http before the https rule is refused, not sent", async () => {
252 const app = createApp({ dataDir: tmp(), port: 0 });
253 try {
254 app.db.query("INSERT INTO user (id, username, email, password_hash) VALUES ('u1', 'w', '[email protected]', 'x')").run();
255 app.db.query("INSERT INTO repo (id, owner_id, name) VALUES ('r1', 'u1', 'h')").run();
256 plainHook(app, "r1", "http://93.184.216.34/h");
257 app.bus.append({ kind: "push", repoId: "r1" });
258 await deliverDue(app.db, app.log);
259 const row = app.db
260 .query<{ state: string; last_error: string | null }, []>("SELECT state, last_error FROM webhook_delivery")
261 .get();
262 expect(row).toEqual({ state: "failed", last_error: "refused: not https" });
263 } finally {
264 app.close();
265 }
266 });
267 });
268
269 function tmp(): string {
270 return mkdtempSync(join(tmpdir(), "forge-hook-r-"));
271 }
272
179273 function unwrap<T>(
180274 r: { ok: true; value: T } | { ok: false; error: unknown },
181275 ): T {

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs