small improvements to the fetch-node package

packages/fetch-node/package.json

@@ -26,12 +26,10 @@
   "dependencies": {
     "@atproto/fetch": "workspace:*",
     "@atproto/transformer": "workspace:*",
-    "http-errors": "^2.0.0",
     "ipaddr.js": "^2.1.0",
     "tslib": "^2.6.2"
   },
   "devDependencies": {
-    "@types/http-errors": "^2.0.4",
     "typescript": "^5.3.3"
   },
   "scripts": {

packages/fetch-node/src/index.ts

@@ -1 +1,2 @@
-export * from './fetch-wrap.js'
+export * from './safe.js'
+export * from './ssrf.js'

packages/fetch-node/src/fetch-wrap.ts → packages/fetch-node/src/safe.ts

@@ -6,11 +6,16 @@ import {
 } from '@atproto/fetch'
 import { compose } from '@atproto/transformer'
 
-import { ssrfSafeHostname } from './ssrf.js'
+import { ssrfFetchWrap } from './ssrf.js'
 
 export type SafeFetchWrapOptions = NonNullable<
   Parameters<typeof safeFetchWrap>[0]
 >
+
+/**
+ * Wrap a fetch function with safety checks so that it can be safely used
+ * with user provided input (URL).
+ */
 export const safeFetchWrap = ({
   fetch = globalThis.fetch as Fetch,
   responseMaxSize = 512 * 1024, // 512kB
@@ -44,41 +49,11 @@ export const safeFetchWrap = ({
      * input, we need to make sure that the request is not vulnerable to SSRF
      * attacks.
      */
-    ssrfProtection ? ssrfSafeFetchWrap({ fetch }) : fetch,
+    ssrfProtection ? ssrfFetchWrap({ fetch }) : fetch,
 
     /**
      * Since we will be fetching user owned data, we need to make sure that an
      * attacker cannot force us to download a large amounts of data.
      */
     fetchMaxSizeProcessor(responseMaxSize),
   )
-
-export type SsrfSafeFetchWrapOptions = NonNullable<
-  Parameters<typeof ssrfSafeFetchWrap>[0]
->
-export const ssrfSafeFetchWrap = ({
-  fetch = globalThis.fetch as Fetch,
-} = {}): Fetch => {
-  const ssrfSafeFetch: Fetch = async (request) => {
-    const { hostname } = new URL(request.url)
-
-    // Make sure the hostname is a valid IP address
-    const ip = await ssrfSafeHostname(hostname)
-    if (ip) {
-      // Normally we would replace the hostname with the IP address and set the
-      // Host header to the original hostname. However, since we are using
-      // fetch() we can't set the Host header.
-    }
-
-    if (request.redirect === 'follow') {
-      // TODO: actually implement by calling ssrfSafeFetch recursively
-      throw new Error(
-        'Request redirect must be "error" or "manual" when SSRF is enabled',
-      )
-    }
-
-    return fetch(request)
-  }
-
-  return ssrfSafeFetch
-}

packages/fetch-node/src/ssrf.ts

@@ -1,23 +1,51 @@
 import dns, { promises as dnsPromises } from 'node:dns'
 
-import createError from 'http-errors'
+import { Fetch, FetchError } from '@atproto/fetch'
 import ipaddr from 'ipaddr.js'
 
 const { IPv4, IPv6 } = ipaddr
 
-export async function ssrfSafeHostname(hostname: string): Promise<string> {
-  const ip = await hostnameLookup(hostname).catch((cause) => {
-    throw cause?.code === 'ENOTFOUND'
-      ? createError(400, `Invalid hostname ${hostname}`, { cause })
-      : createError(500, `Unable resolve DNS for ${hostname}`, { cause })
-  })
-  if (ip.range() !== 'unicast') {
-    throw createError(400, `Invalid hostname IP address ${ip}`)
+export type SsrfSafeFetchWrapOptions = NonNullable<
+  Parameters<typeof ssrfFetchWrap>[0]
+>
+export const ssrfFetchWrap = ({
+  fetch = globalThis.fetch as Fetch,
+} = {}): Fetch => {
+  const ssrfSafeFetch: Fetch = async (request) => {
+    if (request.redirect === 'follow') {
+      // TODO: actually implement by calling ssrfSafeFetch recursively
+      throw new Error(
+        'Request redirect must be "error" or "manual" when SSRF is enabled',
+      )
+    }
+
+    const { hostname } = new URL(request.url)
+
+    // Make sure the hostname is a unicast IP address
+    const ip = await hostnameLookup(hostname).catch((cause) => {
+      throw cause?.code === 'ENOTFOUND'
+        ? new FetchError(400, `Invalid hostname ${hostname}`, {
+            request,
+            cause,
+          })
+        : new FetchError(500, `Unable resolve DNS for ${hostname}`, {
+            request,
+            cause,
+          })
+    })
+    if (ip.range() !== 'unicast') {
+      throw new FetchError(400, `Invalid hostname IP address ${ip}`, {
+        request,
+      })
+    }
+
+    return fetch(request)
   }
-  return ip.toString()
+
+  return ssrfSafeFetch
 }
 
-export async function hostnameLookup(
+async function hostnameLookup(
   hostname: string,
 ): Promise<ipaddr.IPv4 | ipaddr.IPv6> {
   if (IPv4.isIPv4(hostname)) {
@@ -31,7 +59,7 @@ export async function hostnameLookup(
   return domainLookup(hostname)
 }
 
-export async function domainLookup(
+async function domainLookup(
   domain: string,
 ): Promise<ipaddr.IPv4 | ipaddr.IPv6> {
   const addr = await dnsPromises.lookup(domain, {

packages/fetch/src/fetch-request.ts

@@ -24,7 +24,6 @@ export function forbiddenDomainNameRequestTransform(
   forbiddenDomainNames: Iterable<string>,
 ): RequestTranformer {
   const forbiddenDomainNameSet = new Set<string>(forbiddenDomainNames)
-  if (forbiddenDomainNameSet.size === 0) return (request) => request
 
   return async (request) => {
     const { hostname } = new URL(request.url)

packages/pds/src/config/config.ts

@@ -243,8 +243,9 @@ export const envToCfg = (env: ServerEnvironment): ServerConfig => {
       }
 
   const safeFetchCfg: ServerConfig['safeFetch'] = {
-    allowHttp: false,
+    allowHttp: env.fetchDisableSafeties ? true : false,
     forbiddenDomainNames: [
+      'google.com',
       'example.com',
       'example.org',
       'example.net',