Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@ Version 2.0.23

To be released.

### @fedify/vocab-runtime

- Fixed document loaders rejecting public URLs backed by CNAMEs on
Cloudflare Workers. `validatePublicUrl()` now ignores non-IP aliases
returned alongside DNS lookup results while continuing to validate every
resolved IP address, and rejects lookups that return no IP addresses.
[[#956], [#957] by SJang1]

[#956]: https://github.com/fedify-dev/fedify/issues/956
[#957]: https://github.com/fedify-dev/fedify/pull/957

### @fedify/cfworkers

- Fixed `WorkersMessageQueue.enqueueMany()` failing when the given messages
Expand Down
29 changes: 28 additions & 1 deletion packages/vocab-runtime/src/url.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { deepStrictEqual, ok, rejects } from "node:assert";
import { deepStrictEqual, ok, rejects, throws } from "node:assert";
import { test } from "node:test";
import {
expandIPv6Address,
isValidPublicIPv4Address,
isValidPublicIPv6Address,
UrlError,
validateLookupAddresses,
validatePublicUrl,
} from "./url.ts";

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

test("validateLookupAddresses() tolerates Cloudflare Workers CNAME entries", () => {
validateLookupAddresses([
{ address: "app-host.example.net.", family: 4 },
{ address: "93.184.216.34", family: 4 },
{ address: "2606:2800:220:1:248:1893:25c8:1946", family: 6 },
]);
});

test("validateLookupAddresses() rejects unsafe or CNAME-only Cloudflare Workers results", () => {
throws(
() =>
validateLookupAddresses([
{ address: "private-host.example.net.", family: 4 },
{ address: "127.0.0.1", family: 4 },
]),
UrlError,
);
throws(
() =>
validateLookupAddresses([
{ address: "app-host.example.net.", family: 4 },
]),
UrlError,
);
});

test("isValidPublicIPv4Address()", () => {
ok(isValidPublicIPv4Address("8.8.8.8")); // Google DNS
ok(!isValidPublicIPv4Address("192.168.1.1")); // private
Expand Down
40 changes: 35 additions & 5 deletions packages/vocab-runtime/src/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { lookup } from "node:dns/promises";
import { isIP } from "node:net";

export class UrlError extends Error {
constructor(message: string) {
super(message);
constructor(message: string, options?: { cause?: unknown }) {
super(message, options);
this.name = "UrlError";
}
}
Expand Down Expand Up @@ -51,12 +51,42 @@ export async function validatePublicUrl(url: string): Promise<void> {
let addresses: LookupAddress[];
try {
addresses = await lookup(hostname, { all: true });
} catch {
addresses = [];
} catch (error) {
throw new UrlError("DNS lookup failed", { cause: error });
}
for (const { address, family } of addresses) {
validateLookupAddresses(addresses);
}

/**
* Validates the IP addresses returned by `node:dns.lookup()`.
*
* Cloudflare Workers' `node:dns` implementation currently maps every record
* in a DNS-over-HTTPS `Answer` array—including CNAME records—to a
* `LookupAddress`, even though Node.js specifies that the `address` field must
* contain an IPv4 or IPv6 literal. See:
* https://github.com/cloudflare/workerd/issues/6886
*
* Work around that bug by ignoring non-IP entries only when the lookup also
* returns at least one actual IP address. This remains fail-closed: a result
* containing no IP addresses is rejected, and every returned IP address is
* still validated and must be public. This workaround can be revisited once
* the Workerd issue is fixed in supported Cloudflare Workers runtimes.
*
* @internal
*/
export function validateLookupAddresses(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add the required changelog entry

Because this commit fixes #956 in @fedify/vocab-runtime, it falls under the Bugfix process in /workspace/fedify/AGENTS.md, which requires updating CHANGES.md with the issue/PR reference and contributor name. I checked the current Version 2.0.23 unreleased section and it remains empty, so this user-facing fix will be omitted from release notes unless the changelog entry is added before merge.

Useful? React with 👍 / 👎.

addresses: readonly LookupAddress[],
): void {
let ipAddressCount = 0;
for (const { address } of addresses) {
const family = isIP(address);
if (family === 0) continue;
ipAddressCount++;
validatePublicIpAddress(address, family);
}
if (ipAddressCount === 0) {
throw new UrlError("DNS lookup did not return any IP address");
}
}

function validatePublicIpAddress(address: string, family: number): void {
Expand Down
Loading