Skip to content

Commit 43673cb

Browse files
authored
Handle non-IP DNS lookup results (#957)
* Handle non-IP DNS lookup results Ignore non-IP aliases emitted by Node-compatible DNS implementations when a lookup also returns actual IP addresses. Fail closed for alias-only results and continue checking every returned IP. Assisted-by: Codex:gpt-5 * Apply comment: Fail on DNS failture * Document Cloudflare Workers DNS workaround Clarify that non-IP lookup entries are tolerated only for Workerd issue #6886 while retaining fail-closed validation, and identify the regression source in test names. Assisted-by: Codex:gpt-5
1 parent 4c57bb7 commit 43673cb

3 files changed

Lines changed: 74 additions & 6 deletions

File tree

CHANGES.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,17 @@ Version 2.0.23
88

99
To be released.
1010

11+
### @fedify/vocab-runtime
12+
13+
- Fixed document loaders rejecting public URLs backed by CNAMEs on
14+
Cloudflare Workers. `validatePublicUrl()` now ignores non-IP aliases
15+
returned alongside DNS lookup results while continuing to validate every
16+
resolved IP address, and rejects lookups that return no IP addresses.
17+
[[#956], [#957] by SJang1]
18+
19+
[#956]: https://github.com/fedify-dev/fedify/issues/956
20+
[#957]: https://github.com/fedify-dev/fedify/pull/957
21+
1122
### @fedify/cfworkers
1223

1324
- Fixed `WorkersMessageQueue.enqueueMany()` failing when the given messages

packages/vocab-runtime/src/url.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
import { deepStrictEqual, ok, rejects } from "node:assert";
1+
import { deepStrictEqual, ok, rejects, throws } from "node:assert";
22
import { test } from "node:test";
33
import {
44
expandIPv6Address,
55
isValidPublicIPv4Address,
66
isValidPublicIPv6Address,
77
UrlError,
8+
validateLookupAddresses,
89
validatePublicUrl,
910
} from "./url.ts";
1011

@@ -65,6 +66,32 @@ test("validatePublicUrl()", async () => {
6566
await validatePublicUrl("https://[64:ff9b::8.8.8.8]");
6667
});
6768

69+
test("validateLookupAddresses() tolerates Cloudflare Workers CNAME entries", () => {
70+
validateLookupAddresses([
71+
{ address: "app-host.example.net.", family: 4 },
72+
{ address: "93.184.216.34", family: 4 },
73+
{ address: "2606:2800:220:1:248:1893:25c8:1946", family: 6 },
74+
]);
75+
});
76+
77+
test("validateLookupAddresses() rejects unsafe or CNAME-only Cloudflare Workers results", () => {
78+
throws(
79+
() =>
80+
validateLookupAddresses([
81+
{ address: "private-host.example.net.", family: 4 },
82+
{ address: "127.0.0.1", family: 4 },
83+
]),
84+
UrlError,
85+
);
86+
throws(
87+
() =>
88+
validateLookupAddresses([
89+
{ address: "app-host.example.net.", family: 4 },
90+
]),
91+
UrlError,
92+
);
93+
});
94+
6895
test("isValidPublicIPv4Address()", () => {
6996
ok(isValidPublicIPv4Address("8.8.8.8")); // Google DNS
7097
ok(!isValidPublicIPv4Address("192.168.1.1")); // private

packages/vocab-runtime/src/url.ts

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import { lookup } from "node:dns/promises";
33
import { isIP } from "node:net";
44

55
export class UrlError extends Error {
6-
constructor(message: string) {
7-
super(message);
6+
constructor(message: string, options?: { cause?: unknown }) {
7+
super(message, options);
88
this.name = "UrlError";
99
}
1010
}
@@ -51,12 +51,42 @@ export async function validatePublicUrl(url: string): Promise<void> {
5151
let addresses: LookupAddress[];
5252
try {
5353
addresses = await lookup(hostname, { all: true });
54-
} catch {
55-
addresses = [];
54+
} catch (error) {
55+
throw new UrlError("DNS lookup failed", { cause: error });
5656
}
57-
for (const { address, family } of addresses) {
57+
validateLookupAddresses(addresses);
58+
}
59+
60+
/**
61+
* Validates the IP addresses returned by `node:dns.lookup()`.
62+
*
63+
* Cloudflare Workers' `node:dns` implementation currently maps every record
64+
* in a DNS-over-HTTPS `Answer` array—including CNAME records—to a
65+
* `LookupAddress`, even though Node.js specifies that the `address` field must
66+
* contain an IPv4 or IPv6 literal. See:
67+
* https://github.com/cloudflare/workerd/issues/6886
68+
*
69+
* Work around that bug by ignoring non-IP entries only when the lookup also
70+
* returns at least one actual IP address. This remains fail-closed: a result
71+
* containing no IP addresses is rejected, and every returned IP address is
72+
* still validated and must be public. This workaround can be revisited once
73+
* the Workerd issue is fixed in supported Cloudflare Workers runtimes.
74+
*
75+
* @internal
76+
*/
77+
export function validateLookupAddresses(
78+
addresses: readonly LookupAddress[],
79+
): void {
80+
let ipAddressCount = 0;
81+
for (const { address } of addresses) {
82+
const family = isIP(address);
83+
if (family === 0) continue;
84+
ipAddressCount++;
5885
validatePublicIpAddress(address, family);
5986
}
87+
if (ipAddressCount === 0) {
88+
throw new UrlError("DNS lookup did not return any IP address");
89+
}
6090
}
6191

6292
function validatePublicIpAddress(address: string, family: number): void {

0 commit comments

Comments
 (0)