Validate PDS service endpoints before use

[germ-mark]

Atproto.DIDDocument.pdsUrl returned the DID document's serviceEndpoint verbatim, and that URL becomes the base for DPoP-authenticated XRPC traffic and for unauthenticated public traffic. Nothing checked the scheme or host, so http://, localhost, 169.254.169.254, and RFC-1918 addresses were all accepted from a resolver-supplied document.

Tokens themselves were never replayable — DPoP proofs cover htu/htm/ath and the nonce cache is keyed by origin — so the exposure was SSRF and disclosure of request bodies, not credential theft.

Now Service.validate(endpoint:) requires https and rejects localhost, loopback, link-local, RFC-1918, CGNAT, IPv6 ULA, unspecified, multicast/broadcast, and IPv4-mapped IPv6 forms. It's called from both pdsUrl and checkServiceForAtproto(), so there's a single choke point.

One wrinkle worth review: a host string can name different addresses depending on who parses it. IPv4Address reads 0177.0.0.1 as 177.0.0.1 while inet_aton reads 127.0.0.1, and 010.0.0.1 splits the other way. Since we don't control which parser the connection ultimately uses, the validator rejects if any interpretation lands in a blocked range.

Not covered, deliberately: an attacker-controlled public https host still receives credentialed traffic (that needs origin pinning against the recorded issuingServer, plus the PDS origin, which isn't stored yet), and DNS rebinding, which URL inspection can't catch.

Test notes

New PDSEndpointTests, 28 tests passing. Covers accepted endpoints, boundary accepts just outside each blocked range (172.32.0.1, 100.128.0.1), scheme rejections, and 18 host rejections including the legacy numeric spellings. Verified with swift test and an iOS Simulator build.

🤖 Generated with Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: 3729867

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@germ-network/atprototypes	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[ThisIsMissEm]

Needs a test case for something like what https://princess.works returns (dns resolves to 127.0.0.1)

[ThisIsMissEm]

Suggested change

	if host == "localhost" || host.hasSuffix(".localhost") { return false }
	if host == "localhost" || host.hasSuffix(".localhost") || host.hasSuffix(".internal")) { return false }

[ThisIsMissEm]

This isn't actually doing DNS resolution on the host, correct? Because that's what you actually need prior to checking the value isn't localhost. See: https://github.com/bluesky-social/atproto/blob/main/packages/internal/fetch-node/src/safe.ts

[germ-mark]

Correct — it isn't resolving, and after digging into the reference I don't think it should here, though the underlying point is right and I've taken the parts of it that work at this layer.

The thing that makes safe.ts work is that the check isn't resolve-then-fetch, it's new Undici6Agent({ connect: { lookup: unicastLookup } }) — the resolution that opens the socket is the one that gets validated. A separate pre-resolution pass doesn't get you that: the attacker serves a clean A record with TTL 0 to the checker and 127.0.0.1 to the connection. URLSession exposes no equivalent hook, and connecting by pre-checked IP instead of by name breaks NAT64/DNS64, Happy Eyeballs, and Private Relay, which is why Apple explicitly steers away from it. So a resolver here would add a lot of machinery and still only raise the bar rather than close the hole — notably Bluesky's own app clients don't do this check either.

What does close most of it on an https-only client: for princess.works -> 127.0.0.1 to yield more than a TCP connect and a ClientHello, whatever is listening on loopback has to complete a TLS handshake with a valid cert for the attacker's chosen name. Nothing — no header, no token, no body — leaves the app before that succeeds, and an attacker holding that key gains nothing from a reserved IP since they could just receive the traffic on a public one. That's the structural difference from the server-side case fetch-node defends, where the canonical target (169.254.169.254) is unauthenticated plain HTTP. iOS also gates connections into RFC1918/link-local behind the local-network entitlement.

So the residual gap is a TCP-level port-probe oracle, which I don't think justifies a bespoke DNS layer. I've documented that boundary explicitly on validate rather than leaving it implied.

Adopted from your review, in a3cacee:

• your .internal suggestion, plus the rest of the list safe.ts rejects — .local, .test, .invalid, .example (RFC 6761/6762 + ICANN)
• single-label hosts (https://pds), which only resolve through local search domains

One follow-on in 3729867: since loopback is now rejected in every spelling, a dev-env PDS on localhost:2583 became unreachable, so there's an opt-in EndpointPolicy.developmentLoopback that forgives http only when the host is loopback. Strict by default.

On the princess.works test — I don't think it can be written meaningfully without the resolver, since there'd be nothing to assert against; the name is public and passes a name-level check by construction. If you'd rather have the guarantee than the documentation, the honest way to get it is a resolver-aware client (AsyncHTTPClient on NIO can hook resolution the way undici does) rather than this static validator, and I'd want that as its own change.

[ThisIsMissEm]

Okay, sure, that works. Sounds like link-local may also prevent localhost connections in this case