Merge pull request #698 from 2chanhaeng/main

Allow private addresses for explicit URLs in fedify lookup

Author: dahlia

CHANGES.md

@@ -8,6 +8,23 @@ Version 2.1.6
 
 To be released.
 
+### @fedify/cli
+
+ -  Fixed `fedify lookup` failing to look up URLs on private or localhost
+    addresses unless `-p`/`--allow-private-address` was passed, which was a
+    regression introduced in Fedify 2.1.0 when the CLI began forwarding
+    the `allowPrivateAddress` option to the underlying document loader.
+    URLs explicitly provided on the command line now always allow private
+    addresses, while URLs discovered during [`-t`/`--traverse`] honor the
+    option to mitigate SSRF attacks against private addresses.  Recursive
+    fetches via [`--recurse`] continue to always disallow private
+    addresses regardless of the option.  [[#696], [#698] by Chanhaeng Lee]
+
+[`-t`/`--traverse`]: https://fedify.dev/cli#t-traverse-traverse-the-collection
+[`--recurse`]: https://fedify.dev/cli#recurse-recurse-through-object-relationships
+[#696]: https://github.com/fedify-dev/fedify/issues/696
+[#698]: https://github.com/fedify-dev/fedify/pull/698
+
 
 Version 2.1.5
 -------------

docs/cli.md

@@ -522,8 +522,10 @@ and `quoteUri` are not accepted as short forms.
 > are mutually exclusive.
 >
 > Recursive fetches always disallow private/localhost addresses for safety.
-> `-p`/`--allow-private-address` only applies to explicit lookup/traverse
-> targets, not to recursive steps.
+> URLs explicitly provided on the command line always allow private
+> addresses, while
+> [`-p`/`--allow-private-address`](#p-allow-private-address-allow-private-ip-addresses)
+> has no effect on recursive steps.
 
 ### `--recurse-depth`: Set recursion depth limit
 
@@ -980,18 +982,29 @@ fedify lookup --user-agent MyApp/1.0 @fedify@hollo.social
 
 ### `-p`/`--allow-private-address`: Allow private IP addresses
 
-By default, `fedify lookup` does not fetch private or localhost addresses.
-The `-p`/`--allow-private-address` option allows explicit lookup/traverse
-requests to private addresses when needed for local development.
+URLs explicitly provided on the command line always allow private or
+localhost addresses, so local servers can be looked up without any extra
+flags:
 
 ~~~~ sh
-fedify lookup --allow-private-address http://localhost:8000/users/alice
+fedify lookup http://localhost:8000/users/alice
+~~~~
+
+The `-p`/`--allow-private-address` option additionally allows private
+addresses for URLs discovered during traversal.  It only has an effect
+when used together with
+[`-t`/`--traverse`](#t-traverse-traverse-the-collection), since URLs
+embedded in remote responses are otherwise rejected to mitigate SSRF
+attacks against private addresses.
+
+~~~~ sh
+fedify lookup --traverse --allow-private-address http://localhost:8000/users/alice/outbox
 ~~~~
 
 > [!NOTE]
 > Recursive fetches enabled by
-> [`--recurse`](#recurse-recurse-through-object-relationships) continue to
-> disallow private addresses.
+> [`--recurse`](#recurse-recurse-through-object-relationships) always
+> disallow private addresses regardless of this option.
 
 ### `-s`/`--separator`: Output separator
 

packages/cli/src/lookup.ts

@@ -83,8 +83,12 @@ const suppressErrorsOption = bindConfig(
 
 const allowPrivateAddressOption = bindConfig(
   flag("-p", "--allow-private-address", {
-    description:
-      message`Allow private IP addresses for explicit lookup/traverse requests.`,
+    description: message`Allow private IP addresses for URLs discovered \
+during traversal. This option only has an effect when used together \
+with ${optionNames(["-t", "--traverse"])}, since URLs explicitly \
+provided on the command line always allow private addresses and \
+recursive fetches via ${optionNames(["--recurse"])} always disallow \
+them.`,
   }),
   {
     context: configContext,
@@ -716,6 +720,20 @@ export async function runLookup(
   }).start();
 
   let server: TemporaryServer | undefined = undefined;
+  // URLs explicitly provided by the user always allow private addresses,
+  // so that local servers can be looked up without -p/--allow-private-address.
+  // URLs discovered during traversal follow the option to mitigate SSRF
+  // against private addresses, while recursive fetches always disallow
+  // private addresses regardless of the option (see the --recurse branch
+  // below, which hardcodes `allowPrivateAddress: false`).
+  const initialBaseDocumentLoader = await getDocumentLoader({
+    userAgent: command.userAgent,
+    allowPrivateAddress: true,
+  });
+  const initialDocumentLoader = wrapDocumentLoaderWithTimeout(
+    initialBaseDocumentLoader,
+    command.timeout,
+  );
   const baseDocumentLoader = await getDocumentLoader({
     userAgent: command.userAgent,
     allowPrivateAddress: command.allowPrivateAddress,
@@ -734,6 +752,7 @@ export async function runLookup(
   );
 
   let authLoader: DocumentLoader | undefined = undefined;
+  let initialAuthLoader: DocumentLoader | undefined = undefined;
   let authIdentity:
     | { keyId: URL; privateKey: CryptoKey }
     | undefined = undefined;
@@ -836,6 +855,24 @@ export async function runLookup(
       baseAuthLoader,
       command.timeout,
     );
+    const initialBaseAuthLoader = getAuthenticatedDocumentLoader(
+      authIdentity,
+      {
+        allowPrivateAddress: true,
+        userAgent: command.userAgent,
+        specDeterminer: {
+          determineSpec() {
+            return command.firstKnock;
+          },
+          rememberSpec() {
+          },
+        },
+      },
+    );
+    initialAuthLoader = wrapDocumentLoaderWithTimeout(
+      initialBaseAuthLoader,
+      command.timeout,
+    );
   }
 
   spinner.text = `Looking up the ${
@@ -885,8 +922,8 @@ export async function runLookup(
         command.timeout,
       )
       : undefined;
-    const initialLookupDocumentLoader: DocumentLoader = authLoader ??
-      documentLoader;
+    const initialLookupDocumentLoader: DocumentLoader = initialAuthLoader ??
+      initialDocumentLoader;
     const recursiveLookupDocumentLoader: DocumentLoader = recursiveAuthLoader ??
       recursiveDocumentLoader;
     let totalObjects = 0;
@@ -1109,7 +1146,7 @@ export async function runLookup(
       let collection: APObject | null = null;
       try {
         collection = await effectiveDeps.lookupObject(url, {
-          documentLoader: authLoader ?? documentLoader,
+          documentLoader: initialAuthLoader ?? initialDocumentLoader,
           contextLoader,
           userAgent: command.userAgent,
         });
@@ -1248,7 +1285,7 @@ export async function runLookup(
   for (const url of command.urls) {
     promises.push(
       effectiveDeps.lookupObject(url, {
-        documentLoader: authLoader ?? documentLoader,
+        documentLoader: initialAuthLoader ?? initialDocumentLoader,
         contextLoader,
         userAgent: command.userAgent,
       }).catch((error) => {