Background
Since 2.0.23 (#957), validatePublicUrl() throws UrlError in two DNS cases: the lookup itself fails, or it returns no usable IP address. Several callers catch every UrlError and log it as a policy rejection, so an ordinary DNS failure (a dead instance, an expired domain, a flaky resolver) shows up in the logs as if the URL had been blocked for pointing at a private address:
- packages/fedify/src/utils/docloader.ts:
getAuthenticatedDocumentLoader() logs Disallowed private URL: {url} at the error level.
- packages/vocab-runtime/src/docloader.ts:
getDocumentLoader() logs the same message at the error level.
- packages/webfinger/src/lookup.ts:
lookupWebFinger() logs Invalid URL for WebFinger resource descriptor and returns null.
- packages/fedify/src/nodeinfo/client.ts:
getNodeInfo() logs Refused to fetch a private or invalid URL and returns null.
Control flow is correct: these calls fail either way. The problem is diagnostics. An operator who sees Disallowed private URL for a public hostname will reasonably suspect an SSRF attempt or a misconfigured allowPrivateAddress, when the actual cause is that the name didn't resolve. Document loading and WebFinger lookups happen constantly on a busy server, so these misleading error-level logs can pile up.
Expected behavior
Once #1060 lands, UrlError.reason distinguishes "dns" from "disallowed". The callers above should use it to log DNS failures as DNS failures, probably with a message like DNS lookup failed for {url} and the original error attached, and keep the existing messages for "disallowed". Whether a DNS failure deserves the error level or a lower one is open for discussion.
Whether the document loaders should also throw FetchError for DNS failures, as outbound delivery does after #1060, is a separate question. Ordinary network failures from fetch() are not wrapped in FetchError in the loaders today, so changing only the DNS case would be inconsistent. I'd leave that out of this issue.
Affected versions
2.0.23 and later. The 1.x maintenance branches don't wrap DNS failures in UrlError, so they aren't affected. The fix depends on UrlError.reason from #1060, so it should target 2.0-maintenance after that PR is merged.
Background
Since 2.0.23 (#957),
validatePublicUrl()throwsUrlErrorin two DNS cases: the lookup itself fails, or it returns no usable IP address. Several callers catch everyUrlErrorand log it as a policy rejection, so an ordinary DNS failure (a dead instance, an expired domain, a flaky resolver) shows up in the logs as if the URL had been blocked for pointing at a private address:getAuthenticatedDocumentLoader()logsDisallowed private URL: {url}at the error level.getDocumentLoader()logs the same message at the error level.lookupWebFinger()logsInvalid URL for WebFinger resource descriptorand returnsnull.getNodeInfo()logsRefused to fetch a private or invalid URLand returnsnull.Control flow is correct: these calls fail either way. The problem is diagnostics. An operator who sees
Disallowed private URLfor a public hostname will reasonably suspect an SSRF attempt or a misconfiguredallowPrivateAddress, when the actual cause is that the name didn't resolve. Document loading and WebFinger lookups happen constantly on a busy server, so these misleading error-level logs can pile up.Expected behavior
Once #1060 lands,
UrlError.reasondistinguishes"dns"from"disallowed". The callers above should use it to log DNS failures as DNS failures, probably with a message likeDNS lookup failed for {url}and the original error attached, and keep the existing messages for"disallowed". Whether a DNS failure deserves the error level or a lower one is open for discussion.Whether the document loaders should also throw
FetchErrorfor DNS failures, as outbound delivery does after #1060, is a separate question. Ordinary network failures fromfetch()are not wrapped inFetchErrorin the loaders today, so changing only the DNS case would be inconsistent. I'd leave that out of this issue.Affected versions
2.0.23 and later. The 1.x maintenance branches don't wrap DNS failures in
UrlError, so they aren't affected. The fix depends onUrlError.reasonfrom #1060, so it should target 2.0-maintenance after that PR is merged.