From 23574ef60fcc7255358259efd193b29077ad4aa4 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 01:24:43 +0900 Subject: [PATCH 01/46] Add recurse mode and fix lookup separators Added relationship-oriented recursion to `fedify lookup` with `--recurse` and `--recurse-depth`, including parser-level constraints. `--traverse` and `--recurse` are now mutually exclusive, and `--recurse-depth` requires `--recurse`. Implemented recursive chain fetching with visited tracking, depth limits, and best-effort behavior through `--suppress-errors`. Fixed separator rendering in lookup output paths so separators are printed as plain text without extra quotes in recurse/traverse and other multi-object flows. Expanded lookup tests to cover parser behavior, recursion semantics, cycle/depth handling, and error suppression, and updated CLI docs and changelog entries accordingly. Fixes https://github.com/fedify-dev/fedify/issues/606 Co-Authored-By: OpenAI Codex --- CHANGES.md | 17 ++ docs/cli.md | 62 ++++++- packages/cli/src/config.ts | 7 + packages/cli/src/lookup.test.ts | 203 +++++++++++++++++++++ packages/cli/src/lookup.ts | 314 +++++++++++++++++++++++++++++--- 5 files changed, 566 insertions(+), 37 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index bb84e01bb..f31a70c2c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,6 +19,23 @@ To be released. [#473]: https://github.com/fedify-dev/fedify/issues/473 [#589]: https://github.com/fedify-dev/fedify/pull/589 +### @fedify/cli + + - Fixed `fedify lookup` printing separators with extra quotes between + adjacent objects/items in some output paths (e.g., recurse/traverse + flows). Separators are now printed as plain text consistently. + [[#608]] + + - Added `--recurse` and `--recurse-depth` options to `fedify lookup` for + recursively following object relationships (e.g., reply chains via + `replyTarget` / `inReplyTo`). `--traverse` and `--recurse` are now + mutually exclusive, `--recurse-depth` depends on `--recurse`, and + `--suppress-errors` now works in recurse mode as best-effort lookup. + [[#606], [#608]] + +[#606]: https://github.com/fedify-dev/fedify/issues/606 +[#608]: https://github.com/fedify-dev/fedify/pull/608 + ### @fedify/vocab - Fixed `Endpoints.toJsonLd()` to no longer emit invalid diff --git a/docs/cli.md b/docs/cli.md index ba36e62be..612ed203b 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -481,8 +481,9 @@ option. > [!NOTE] > The `fedify lookup` command cannot take multiple argument if -> [`-t`/`--traverse`](#t-traverse-traverse-the-collection) option is turned -> on. +> [`-t`/`--traverse`](#t-traverse-traverse-the-collection) or +> [`--recurse`](#recurse-recurse-through-object-relationships) option is +> turned on. ### `-t`/`--traverse`: Traverse the collection @@ -502,23 +503,66 @@ output the collection object itself. This option only works with a single argument, and it has to be a collection. -### `-S`/`--suppress-errors`: Suppress partial errors during traversal +### `--recurse`: Recurse through object relationships + +*This option is available since Fedify 2.1.0.* + +The `--recurse` option is used to recursively follow an object relationship. +This is useful when you want to walk a reply chain through `replyTarget`. + +~~~~ sh +fedify lookup --recurse=replyTarget https://hollo.social/@fedify/019c8522-b247-79d3-b0e7-c6a2293bb1cf +~~~~ + +You can also provide the fully qualified property IRI: + +~~~~ sh +fedify lookup --recurse=https://www.w3.org/ns/activitystreams#inReplyTo https://hollo.social/@fedify/019c8522-b247-79d3-b0e7-c6a2293bb1cf +~~~~ + +For short names, only Fedify property naming is accepted. For example, +`replyTarget` is accepted, while `inReplyTo` is not accepted as a short form. + +> [!NOTE] +> `--recurse` and [`-t`/`--traverse`](#t-traverse-traverse-the-collection) +> are mutually exclusive. + +### `--recurse-depth`: Set recursion depth limit + +*This option is available since Fedify 2.1.0.* + +The `--recurse-depth` option sets the maximum recursion depth when using +[`--recurse`](#recurse-recurse-through-object-relationships). By default, it +is set to `20`. + +~~~~ sh +fedify lookup --recurse=replyTarget --recurse-depth=10 https://hollo.social/@fedify/019c8522-b247-79d3-b0e7-c6a2293bb1cf +~~~~ + +This option depends on the `--recurse` option. + +### `-S`/`--suppress-errors`: Suppress partial errors during traversal or recursion *This option is available since Fedify 0.14.0.* The `-S`/`--suppress-errors` option is used to suppress partial errors during -traversal. For example, the below command looks up a collection object with -the `-t`/`--traverse` option: +traversal or recursion. + +For traversal mode: ~~~~ sh fedify lookup --traverse --suppress-errors https://fosstodon.org/users/hongminhee/outbox ~~~~ -The difference between with and without the `-S`/`--suppress-errors` option is -that the former will suppress the partial errors during traversal, while the -latter will stop the traversal when an error occurs. +For recursion mode: + +~~~~ sh +fedify lookup --recurse=replyTarget --suppress-errors https://hollo.social/@fedify/019c8522-b247-79d3-b0e7-c6a2293bb1cf +~~~~ -This option depends on the `-t`/`--traverse` option. +The difference between with and without the `-S`/`--suppress-errors` option is +that the former will suppress the partial errors during traversal or recursion, +while the latter will stop on the first such error. ### `-c`/`--compact`: Compact JSON-LD diff --git a/packages/cli/src/config.ts b/packages/cli/src/config.ts index dbdb4057a..2aaf96a97 100644 --- a/packages/cli/src/config.ts +++ b/packages/cli/src/config.ts @@ -31,6 +31,13 @@ const lookupSchema = object({ picklist(["draft-cavage-http-signatures-12", "rfc9421"]), ), traverse: optional(boolean()), + recurse: optional( + picklist([ + "replyTarget", + "https://www.w3.org/ns/activitystreams#inReplyTo", + ]), + ), + recurseDepth: optional(number()), suppressErrors: optional(boolean()), defaultFormat: optional(picklist(["default", "raw", "compact", "expand"])), separator: optional(string()), diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index 9b86a5b1a..536fd48d2 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -10,7 +10,11 @@ import { getContextLoader } from "./docloader.ts"; import { authorizedFetchOption, clearTimeoutSignal, + collectRecursiveObjects, createTimeoutSignal, + getRecursiveTargetId, + lookupCommand, + RecursiveLookupError, TimeoutError, writeObjectToStream, } from "./lookup.ts"; @@ -303,3 +307,202 @@ test("authorizedFetchOption - parses with -a and --tunnel-service", () => { assert.strictEqual(result.value.tunnelService, "serveo.net"); } }); + +test("lookupCommand - parses recurse option", () => { + setActiveConfig(configContext.id, {}); + const result = parse(lookupCommand, [ + "lookup", + "--recurse", + "replyTarget", + "https://example.com/notes/1", + ]); + clearActiveConfig(configContext.id); + assert.ok(result.success); + if (result.success) { + assert.strictEqual(result.value.recurse, "replyTarget"); + assert.strictEqual(result.value.recurseDepth, 20); + assert.strictEqual(result.value.traverse, false); + } +}); + +test("lookupCommand - rejects recurse-depth without recurse", () => { + setActiveConfig(configContext.id, {}); + const result = parse(lookupCommand, [ + "lookup", + "--recurse-depth", + "10", + "https://example.com/notes/1", + ]); + clearActiveConfig(configContext.id); + assert.ok(!result.success); +}); + +test("lookupCommand - rejects traverse with recurse", () => { + setActiveConfig(configContext.id, {}); + const result = parse(lookupCommand, [ + "lookup", + "--traverse", + "--recurse", + "replyTarget", + "https://example.com/notes/1", + ]); + clearActiveConfig(configContext.id); + assert.ok(!result.success); +}); + +test("lookupCommand - rejects short-form inReplyTo", () => { + setActiveConfig(configContext.id, {}); + const result = parse(lookupCommand, [ + "lookup", + "--recurse", + "inReplyTo", + "https://example.com/notes/1", + ]); + clearActiveConfig(configContext.id); + assert.ok(!result.success); +}); + +test("lookupCommand - accepts IRI inReplyTo", () => { + setActiveConfig(configContext.id, {}); + const result = parse(lookupCommand, [ + "lookup", + "--recurse", + "https://www.w3.org/ns/activitystreams#inReplyTo", + "https://example.com/notes/1", + ]); + clearActiveConfig(configContext.id); + assert.ok(result.success); + if (result.success) { + assert.strictEqual( + result.value.recurse, + "https://www.w3.org/ns/activitystreams#inReplyTo", + ); + } +}); + +test("getRecursiveTargetId - returns reply target for short name", () => { + const replyTarget = new URL("https://example.com/notes/0"); + const note = new Note({ + id: new URL("https://example.com/notes/1"), + replyTarget, + }); + assert.equal(getRecursiveTargetId(note, "replyTarget"), replyTarget); +}); + +test("getRecursiveTargetId - returns reply target for IRI", () => { + const replyTarget = new URL("https://example.com/notes/0"); + const note = new Note({ + id: new URL("https://example.com/notes/1"), + replyTarget, + }); + assert.equal( + getRecursiveTargetId( + note, + "https://www.w3.org/ns/activitystreams#inReplyTo", + ), + replyTarget, + ); +}); + +test("collectRecursiveObjects - follows chain up to depth limit", async () => { + const note1 = new Note({ + id: new URL("https://example.com/notes/1"), + replyTarget: new URL("https://example.com/notes/0"), + }); + const note0 = new Note({ + id: new URL("https://example.com/notes/0"), + }); + const objects = new Map([ + ["https://example.com/notes/0", note0], + ]); + const result = await collectRecursiveObjects( + note1, + "replyTarget", + 10, + (url) => Promise.resolve(objects.get(url) ?? null), + { suppressErrors: false }, + ); + assert.deepEqual(result, [note0]); +}); + +test("collectRecursiveObjects - respects recurse depth", async () => { + const note2 = new Note({ + id: new URL("https://example.com/notes/2"), + replyTarget: new URL("https://example.com/notes/1"), + }); + const note1 = new Note({ + id: new URL("https://example.com/notes/1"), + replyTarget: new URL("https://example.com/notes/0"), + }); + const note0 = new Note({ + id: new URL("https://example.com/notes/0"), + }); + const objects = new Map([ + ["https://example.com/notes/1", note1], + ["https://example.com/notes/0", note0], + ]); + const result = await collectRecursiveObjects( + note2, + "replyTarget", + 1, + (url) => Promise.resolve(objects.get(url) ?? null), + { suppressErrors: false }, + ); + assert.deepEqual(result, [note1]); +}); + +test("collectRecursiveObjects - stops on cycle", async () => { + const note2 = new Note({ + id: new URL("https://example.com/notes/2"), + replyTarget: new URL("https://example.com/notes/1"), + }); + const note1 = new Note({ + id: new URL("https://example.com/notes/1"), + replyTarget: new URL("https://example.com/notes/2"), + }); + const objects = new Map([ + ["https://example.com/notes/1", note1], + ["https://example.com/notes/2", note2], + ]); + const visited = new Set(); + const result = await collectRecursiveObjects( + note2, + "replyTarget", + 10, + (url) => Promise.resolve(objects.get(url) ?? null), + { suppressErrors: false, visited }, + ); + assert.deepEqual(result, [note1]); +}); + +test("collectRecursiveObjects - throws when lookup fails", async () => { + const note = new Note({ + id: new URL("https://example.com/notes/1"), + replyTarget: new URL("https://example.com/notes/0"), + }); + await assert.rejects( + collectRecursiveObjects( + note, + "replyTarget", + 10, + () => Promise.resolve(null), + { suppressErrors: false }, + ), + RecursiveLookupError, + ); +}); + +test("collectRecursiveObjects - suppresses errors in best-effort mode", async () => { + const note = new Note({ + id: new URL("https://example.com/notes/1"), + replyTarget: new URL("https://example.com/notes/0"), + }); + const result = await collectRecursiveObjects( + note, + "replyTarget", + 10, + () => Promise.resolve(null), + { suppressErrors: true }, + ); + assert.deepEqual(result, []); +}); diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index bfc784dfa..8f3835371 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -24,6 +24,7 @@ import { flag, float, type InferValue, + integer, map, merge, message, @@ -36,7 +37,7 @@ import { string, withDefault, } from "@optique/core"; -import { path, print, printError } from "@optique/run"; +import { path, printError } from "@optique/run"; import { createWriteStream, type WriteStream } from "node:fs"; import process from "node:process"; import ora from "ora"; @@ -54,6 +55,10 @@ import { colorEnabled, colors, formatObject } from "./utils.ts"; const logger = getLogger(["fedify", "cli", "lookup"]); +const IN_REPLY_TO_IRI = "https://www.w3.org/ns/activitystreams#inReplyTo"; +const recurseProperties = ["replyTarget", IN_REPLY_TO_IRI] as const; +type RecurseProperty = typeof recurseProperties[number]; + export const authorizedFetchOption = withDefault( object("Authorized fetch options", { authorizedFetch: bindConfig( @@ -94,36 +99,95 @@ export const authorizedFetchOption = withDefault( } as const, ); -const traverseOption = object("Traverse options", { - traverse: bindConfig( - flag("-t", "--traverse", { - description: - message`Traverse the given collection(s) to fetch all items.`, +const lookupModeOption = withDefault( + or( + object("Recurse options", { + traverse: constant(false as const), + recurse: bindConfig( + option( + "--recurse", + choice(recurseProperties, { metavar: "PROPERTY" }), + { + description: message`Recursively follow a relationship property (${ + optionNames(["replyTarget"]) + } or ${IN_REPLY_TO_IRI}).`, + }, + ), + { + context: configContext, + key: (config) => config.lookup?.recurse, + }, + ), + recurseDepth: withDefault( + bindConfig( + option( + "--recurse-depth", + integer({ min: 1, metavar: "DEPTH" }), + { + description: message`Maximum recursion depth for ${ + optionNames(["--recurse"]) + }.`, + }, + ), + { + context: configContext, + key: (config) => config.lookup?.recurseDepth, + }, + ), + 20, + ), + suppressErrors: bindConfig( + flag("-S", "--suppress-errors", { + description: + message`Suppress partial errors during traversal or recursion.`, + }), + { + context: configContext, + key: (config) => config.lookup?.suppressErrors ?? false, + default: false, + }, + ), }), - { - context: configContext, - key: (config) => config.lookup?.traverse ?? false, - default: false, - }, - ), - suppressErrors: bindConfig( - flag("-S", "--suppress-errors", { - description: - message`Suppress partial errors while traversing the collection.`, + object("Traverse options", { + traverse: bindConfig( + flag("-t", "--traverse", { + description: + message`Traverse the given collection(s) to fetch all items.`, + }), + { + context: configContext, + key: (config) => config.lookup?.traverse ?? false, + default: false, + }, + ), + recurse: constant(undefined), + recurseDepth: constant(undefined), + suppressErrors: bindConfig( + flag("-S", "--suppress-errors", { + description: + message`Suppress partial errors during traversal or recursion.`, + }), + { + context: configContext, + key: (config) => config.lookup?.suppressErrors ?? false, + default: false, + }, + ), }), - { - context: configContext, - key: (config) => config.lookup?.suppressErrors ?? false, - default: false, - }, ), -}); + { + traverse: false, + recurse: undefined, + recurseDepth: undefined, + suppressErrors: false, + } as const, +); export const lookupCommand = command( "lookup", merge( object({ command: constant("lookup") }), - traverseOption, + lookupModeOption, authorizedFetchOption, merge( "Network options", @@ -226,6 +290,15 @@ export class TimeoutError extends Error { } } +export class RecursiveLookupError extends Error { + target: string; + constructor(target: string) { + super(`Failed to recursively fetch object: ${target}`); + this.name = "RecursiveLookupError"; + this.target = target; + } +} + async function findAllImages(obj: APObject): Promise { const result: URL[] = []; const icon = await obj.getIcon(); @@ -340,6 +413,61 @@ function handleTimeoutError( ); } +export function getRecursiveTargetId( + object: APObject, + recurseProperty: RecurseProperty, +): URL | null { + switch (recurseProperty) { + case "replyTarget": + case IN_REPLY_TO_IRI: + return object.replyTargetId; + default: + return null; + } +} + +export async function collectRecursiveObjects( + initialObject: APObject, + recurseProperty: RecurseProperty, + recurseDepth: number, + lookup: (url: string) => Promise, + options: { suppressErrors: boolean; visited?: Set }, +): Promise { + const visited = options.visited ?? new Set(); + const results: APObject[] = []; + let current = initialObject; + if (current.id != null) { + visited.add(current.id.href); + } + + for (let depth = 0; depth < recurseDepth; depth++) { + const targetId = getRecursiveTargetId(current, recurseProperty); + if (targetId == null) break; + const target = targetId.href; + if (visited.has(target)) break; + visited.add(target); + + let next: APObject | null; + try { + next = await lookup(target); + } catch (error) { + if (options.suppressErrors) break; + throw error; + } + if (next == null) { + if (options.suppressErrors) break; + throw new RecursiveLookupError(target); + } + results.push(next); + if (next.id != null) { + visited.add(next.id.href); + } + current = next; + } + + return results; +} + export async function runLookup( command: InferValue & GlobalOptions, ) { @@ -355,7 +483,9 @@ export async function runLookup( const spinner = ora({ text: `Looking up the ${ - command.traverse + command.recurse != null + ? "object chain" + : command.traverse ? "collection" : command.urls.length > 1 ? "objects" @@ -442,13 +572,141 @@ export async function runLookup( } spinner.text = `Looking up the ${ - command.traverse + command.recurse != null + ? "object chain" + : command.traverse ? "collection" : command.urls.length > 1 ? "objects" : "object" }...`; + if (command.recurse != null) { + let totalObjects = 0; + const visited = new Set(); + const recurseDepth = command.recurseDepth ?? 20; + + for (let urlIndex = 0; urlIndex < command.urls.length; urlIndex++) { + const url = command.urls[urlIndex]; + if (urlIndex > 0) { + spinner.text = `Looking up object chain ${ + urlIndex + 1 + }/${command.urls.length}...`; + } + let current: APObject | null; + try { + current = await lookupObject(url, { + documentLoader: authLoader ?? documentLoader, + contextLoader, + userAgent: command.userAgent, + }); + } catch (error) { + if (error instanceof TimeoutError) { + handleTimeoutError(spinner, command.timeout, url); + } else { + spinner.fail(`Failed to fetch object: ${colors.red(url)}.`); + if (authLoader == null) { + printError( + message`It may be a private object. Try with -a/--authorized-fetch.`, + ); + } + } + await server?.close(); + process.exit(1); + } + if (current == null) { + spinner.fail(`Failed to fetch object: ${colors.red(url)}.`); + if (authLoader == null) { + printError( + message`It may be a private object. Try with -a/--authorized-fetch.`, + ); + } + await server?.close(); + process.exit(1); + } + + if (totalObjects > 0 && !command.output) { + console.log(command.separator); + } + await writeObjectToStream( + current, + command.output, + command.format, + contextLoader, + ); + totalObjects++; + visited.add(url); + if (current.id != null) { + visited.add(current.id.href); + } + + let chain: APObject[]; + try { + chain = await collectRecursiveObjects( + current, + command.recurse, + recurseDepth, + (target) => + lookupObject(target, { + documentLoader: authLoader ?? documentLoader, + contextLoader, + userAgent: command.userAgent, + }), + { suppressErrors: command.suppressErrors, visited }, + ); + } catch (error) { + logger.error( + "Failed to recursively fetch an object in chain: {error}", + { + error, + }, + ); + if (error instanceof TimeoutError) { + handleTimeoutError(spinner, command.timeout); + } else if (error instanceof RecursiveLookupError) { + spinner.fail( + `Failed to recursively fetch object: ${colors.red(error.target)}.`, + ); + if (authLoader == null) { + printError( + message`It may be a private object. Try with -a/--authorized-fetch.`, + ); + } + } else { + spinner.fail("Failed to recursively fetch object."); + if (authLoader == null) { + printError( + message`It may be a private object. Try with -a/--authorized-fetch.`, + ); + } else { + printError( + message`Use the -S/--suppress-errors option to suppress partial errors.`, + ); + } + } + await server?.close(); + process.exit(1); + } + + for (const next of chain) { + if (!command.output) { + console.log(command.separator); + } + await writeObjectToStream( + next, + command.output, + command.format, + contextLoader, + ); + totalObjects++; + } + } + + spinner.succeed("Successfully fetched all reachable objects in the chain."); + await server?.close(); + process.exit(0); + } + if (command.traverse) { let totalItems = 0; @@ -512,7 +770,7 @@ export async function runLookup( }) ) { if (!command.output && (totalItems > 0 || collectionItems > 0)) { - print(message`${command.separator}`); + console.log(command.separator); } await writeObjectToStream( item, @@ -584,7 +842,7 @@ export async function runLookup( let i = 0; for (const obj of objects) { const url = command.urls[i]; - if (i > 0) print(message`${command.separator}`); + if (i > 0) console.log(command.separator); i++; if (obj == null) { spinner.fail(`Failed to fetch ${colors.red(url)}`); @@ -603,7 +861,7 @@ export async function runLookup( contextLoader, ); if (i < command.urls.length - 1) { - print(message`${command.separator}`); + console.log(command.separator); } } } From cd5c522be2444be2727737f505ef1dbc63326438 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 11:28:34 +0900 Subject: [PATCH 02/46] Validate lookup.recurseDepth config as positive integer Align config validation with CLI option constraints for --recurse-depth. https://github.com/fedify-dev/fedify/pull/608#discussion_r2900046881 Co-Authored-By: OpenAI Codex --- packages/cli/src/config.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/config.ts b/packages/cli/src/config.ts index 2aaf96a97..1143be48b 100644 --- a/packages/cli/src/config.ts +++ b/packages/cli/src/config.ts @@ -7,10 +7,13 @@ import { array, boolean, type InferOutput, + integer, + minValue, number, object, optional, picklist, + pipe, string, } from "valibot"; @@ -37,7 +40,7 @@ const lookupSchema = object({ "https://www.w3.org/ns/activitystreams#inReplyTo", ]), ), - recurseDepth: optional(number()), + recurseDepth: optional(pipe(number(), integer(), minValue(1))), suppressErrors: optional(boolean()), defaultFormat: optional(picklist(["default", "raw", "compact", "expand"])), separator: optional(string()), From 10b30f4a1850f7357a61f596e9d02b8cb54fa645 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 11:29:03 +0900 Subject: [PATCH 03/46] Correct lookup docs for traverse/recurse multi-argument behavior Update CLI documentation to match current implementation that accepts multiple inputs in traverse/recurse flows. https://github.com/fedify-dev/fedify/pull/608#discussion_r2900046884 Co-Authored-By: OpenAI Codex --- docs/cli.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 612ed203b..163051cf5 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -479,12 +479,6 @@ As you can see, the outputs are separated by `----` by default. You can change the separator by using the [`-s`/`--separator`](#s-separator-output-separator) option. -> [!NOTE] -> The `fedify lookup` command cannot take multiple argument if -> [`-t`/`--traverse`](#t-traverse-traverse-the-collection) or -> [`--recurse`](#recurse-recurse-through-object-relationships) option is -> turned on. - ### `-t`/`--traverse`: Traverse the collection *This option is available since Fedify 0.14.0.* @@ -501,7 +495,7 @@ The difference between with and without the `-t`/`--traverse` option is that the former will output the objects in the collection, while the latter will output the collection object itself. -This option only works with a single argument, and it has to be a collection. +When this option is enabled, each argument has to resolve to a collection. ### `--recurse`: Recurse through object relationships From ee07874b75cbe37f980e7565b0f851c8b5fe6c91 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 11:30:22 +0900 Subject: [PATCH 04/46] Print exactly one separator between printed lookup objects Emit separators only before each printed object after the first to avoid duplicate separators in multi-object lookup output. Ref: https://github.com/fedify-dev/fedify/pull/608#discussion_r2900046890 Co-Authored-By: OpenAI Codex --- packages/cli/src/lookup.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 8f3835371..f540e5330 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -839,11 +839,9 @@ export async function runLookup( spinner.stop(); let success = true; - let i = 0; - for (const obj of objects) { + let printedCount = 0; + for (const [i, obj] of objects.entries()) { const url = command.urls[i]; - if (i > 0) console.log(command.separator); - i++; if (obj == null) { spinner.fail(`Failed to fetch ${colors.red(url)}`); if (authLoader == null) { @@ -854,15 +852,14 @@ export async function runLookup( success = false; } else { spinner.succeed(`Fetched object: ${colors.green(url)}`); + if (printedCount > 0) console.log(command.separator); await writeObjectToStream( obj, command.output, command.format, contextLoader, ); - if (i < command.urls.length - 1) { - console.log(command.separator); - } + printedCount++; } } if (success) { From acec2d3e5e84e58b886c4a8b6d6c98ef0e5edd14 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 11:56:19 +0900 Subject: [PATCH 05/46] Reuse output stream across lookup multi-object writes Open the output stream once in runLookup() and pass it to writeObjectToStream() so recursive and traversal flows do not truncate the file per object. Also close streams explicitly and add a regression test for stream reuse. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901166808 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901166809 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901169671 Co-Authored-By: OpenAI Codex --- packages/cli/src/lookup.test.ts | 35 +++++++++++ packages/cli/src/lookup.ts | 101 ++++++++++++++++++++++---------- 2 files changed, 106 insertions(+), 30 deletions(-) diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index 536fd48d2..d8ac4c336 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -3,6 +3,7 @@ import { clearActiveConfig, setActiveConfig } from "@optique/config"; import { runWithConfig } from "@optique/config/run"; import { parse } from "@optique/core/parser"; import assert from "node:assert/strict"; +import { createWriteStream } from "node:fs"; import { mkdir, readFile, rm } from "node:fs/promises"; import test from "node:test"; import { configContext } from "./config.ts"; @@ -112,6 +113,40 @@ test("writeObjectToStream - writes object in expanded JSON-LD format", async () await rm(testDir, { recursive: true }); }); +test("writeObjectToStream - supports reusing an output stream", async () => { + const testDir = "./test_output_reused_stream"; + const testFile = `${testDir}/notes.txt`; + + await mkdir(testDir, { recursive: true }); + + const note1 = new Note({ + id: new URL("https://example.com/notes/1"), + content: "First note", + }); + const note2 = new Note({ + id: new URL("https://example.com/notes/2"), + content: "Second note", + }); + + const contextLoader = await getContextLoader({}); + const stream = createWriteStream(testFile); + + await writeObjectToStream(note1, testFile, undefined, contextLoader, stream); + await writeObjectToStream(note2, testFile, undefined, contextLoader, stream); + await new Promise((resolve, reject) => { + stream.end((error?: Error | null) => { + if (error != null) reject(error); + else resolve(); + }); + }); + + const content = await readFile(testFile, { encoding: "utf8" }); + assert.match(content, /First note/); + assert.match(content, /Second note/); + + await rm(testDir, { recursive: true }); +}); + test("writeObjectToStream - writes to stdout when no output file specified", async () => { const note = new Note({ id: new URL("https://example.com/notes/1"), diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index f540e5330..7cbd081c9 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -319,10 +319,13 @@ export async function writeObjectToStream( outputPath: string | undefined, format: string | undefined, contextLoader: DocumentLoader, + stream?: NodeJS.WritableStream, ): Promise { - const stream: WriteStream | NodeJS.WritableStream = outputPath - ? createWriteStream(outputPath) - : process.stdout; + const localStream: WriteStream | NodeJS.WritableStream = stream ?? + (outputPath ? createWriteStream(outputPath) : process.stdout); + const localFileStream = stream == null && outputPath != null + ? localStream as WriteStream + : undefined; let content; let json = true; @@ -350,7 +353,21 @@ export async function writeObjectToStream( const encoder = new TextEncoder(); const bytes = encoder.encode(content + "\n"); - stream.write(bytes); + await new Promise((resolve, reject) => { + localStream.write(bytes, (error) => { + if (error != null) reject(error); + else resolve(); + }); + }); + + if (localFileStream != null) { + await new Promise((resolve, reject) => { + localFileStream.end((error?: Error | null) => { + if (error != null) reject(error); + else resolve(); + }); + }); + } if (object instanceof APObject) { imageUrls = await findAllImages(object); @@ -360,6 +377,16 @@ export async function writeObjectToStream( } } +async function closeWriteStream(stream?: WriteStream): Promise { + if (stream == null) return; + await new Promise((resolve, reject) => { + stream.end((error?: Error | null) => { + if (error != null) reject(error); + else resolve(); + }); + }); +} + const signalTimers = new WeakMap(); export function createTimeoutSignal( @@ -511,6 +538,14 @@ export async function runLookup( ); let authLoader: DocumentLoader | undefined = undefined; + const outputStream = command.output == null + ? undefined + : createWriteStream(command.output); + const finalizeAndExit = async (code: number) => { + await closeWriteStream(outputStream); + await server?.close(); + process.exit(code); + }; if (command.authorizedFetch) { spinner.text = "Generating a one-time key pair..."; @@ -593,7 +628,7 @@ export async function runLookup( urlIndex + 1 }/${command.urls.length}...`; } - let current: APObject | null; + let current: APObject | null = null; try { current = await lookupObject(url, { documentLoader: authLoader ?? documentLoader, @@ -611,8 +646,8 @@ export async function runLookup( ); } } - await server?.close(); - process.exit(1); + await finalizeAndExit(1); + return; } if (current == null) { spinner.fail(`Failed to fetch object: ${colors.red(url)}.`); @@ -621,8 +656,8 @@ export async function runLookup( message`It may be a private object. Try with -a/--authorized-fetch.`, ); } - await server?.close(); - process.exit(1); + await finalizeAndExit(1); + return; } if (totalObjects > 0 && !command.output) { @@ -633,6 +668,7 @@ export async function runLookup( command.output, command.format, contextLoader, + outputStream, ); totalObjects++; visited.add(url); @@ -640,7 +676,7 @@ export async function runLookup( visited.add(current.id.href); } - let chain: APObject[]; + let chain: APObject[] = []; try { chain = await collectRecursiveObjects( current, @@ -684,8 +720,8 @@ export async function runLookup( ); } } - await server?.close(); - process.exit(1); + await finalizeAndExit(1); + return; } for (const next of chain) { @@ -697,14 +733,15 @@ export async function runLookup( command.output, command.format, contextLoader, + outputStream, ); totalObjects++; } } spinner.succeed("Successfully fetched all reachable objects in the chain."); - await server?.close(); - process.exit(0); + await finalizeAndExit(0); + return; } if (command.traverse) { @@ -719,7 +756,7 @@ export async function runLookup( }/${command.urls.length}...`; } - let collection: APObject | null; + let collection: APObject | null = null; try { collection = await lookupObject(url, { documentLoader: authLoader ?? documentLoader, @@ -737,8 +774,8 @@ export async function runLookup( ); } } - await server?.close(); - process.exit(1); + await finalizeAndExit(1); + return; } if (collection == null) { spinner.fail(`Failed to fetch object: ${colors.red(url)}.`); @@ -747,16 +784,16 @@ export async function runLookup( message`It may be a private object. Try with -a/--authorized-fetch.`, ); } - await server?.close(); - process.exit(1); + await finalizeAndExit(1); + return; } if (!(collection instanceof Collection)) { spinner.fail( `Not a collection: ${colors.red(url)}. ` + "The -t/--traverse option requires a collection.", ); - await server?.close(); - process.exit(1); + await finalizeAndExit(1); + return; } spinner.succeed(`Fetched collection: ${colors.green(url)}.`); @@ -777,6 +814,7 @@ export async function runLookup( command.output, command.format, contextLoader, + outputStream, ); collectionItems++; totalItems++; @@ -802,14 +840,14 @@ export async function runLookup( ); } } - await server?.close(); - process.exit(1); + await finalizeAndExit(1); + return; } } spinner.succeed("Successfully fetched all items in the collection."); - await server?.close(); - process.exit(0); + await finalizeAndExit(0); + return; } const promises: Promise[] = []; @@ -829,12 +867,12 @@ export async function runLookup( ); } - let objects: (APObject | null)[]; + let objects: (APObject | null)[] = []; try { objects = await Promise.all(promises); } catch (_error) { - await server?.close(); - process.exit(1); + await finalizeAndExit(1); + return; } spinner.stop(); @@ -858,6 +896,7 @@ export async function runLookup( command.output, command.format, contextLoader, + outputStream, ); printedCount++; } @@ -869,10 +908,12 @@ export async function runLookup( : "Successfully fetched the object.", ); } - await server?.close(); if (!success) { - process.exit(1); + await finalizeAndExit(1); + return; } + await closeWriteStream(outputStream); + await server?.close(); if (success && command.output) { spinner.succeed( `Successfully wrote output to ${colors.green(command.output)}.`, From d41828694eeafe46a19a7534bc817befdb8700ad Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 11:56:58 +0900 Subject: [PATCH 06/46] Avoid poisoning visited set on suppressed lookup failures Do not mark a recursive target as visited until the lookup succeeds. This keeps suppress-errors mode from blocking later attempts to the same target. Add a regression test for visited-set behavior when lookup fails in suppress mode. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901169676 Co-Authored-By: OpenAI Codex --- packages/cli/src/lookup.test.ts | 20 ++++++++++++++++++++ packages/cli/src/lookup.ts | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index d8ac4c336..e35389af8 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -541,3 +541,23 @@ test("collectRecursiveObjects - suppresses errors in best-effort mode", async () ); assert.deepEqual(result, []); }); + +test( + "collectRecursiveObjects - suppress mode does not poison visited set", + async () => { + const note2 = new Note({ + id: new URL("https://example.com/notes/2"), + replyTarget: new URL("https://example.com/notes/1"), + }); + const visited = new Set(); + const result = await collectRecursiveObjects( + note2, + "replyTarget", + 10, + () => Promise.reject(new Error("temporary failure")), + { suppressErrors: true, visited }, + ); + assert.deepEqual(result, []); + assert.equal(visited.has("https://example.com/notes/1"), false); + }, +); diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 7cbd081c9..bc158ada7 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -472,7 +472,6 @@ export async function collectRecursiveObjects( if (targetId == null) break; const target = targetId.href; if (visited.has(target)) break; - visited.add(target); let next: APObject | null; try { @@ -486,6 +485,7 @@ export async function collectRecursiveObjects( throw new RecursiveLookupError(target); } results.push(next); + visited.add(target); if (next.id != null) { visited.add(next.id.href); } From 280502b7500b50cb2797712da1fed7f70851951b Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 11:57:19 +0900 Subject: [PATCH 07/46] Simplify recursive target resolver for validated recurse props Collapse getRecursiveTargetId() to a single return now that recurse properties are validated by the CLI parser. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901166810 Co-Authored-By: OpenAI Codex --- packages/cli/src/lookup.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index bc158ada7..1751633e7 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -442,15 +442,9 @@ function handleTimeoutError( export function getRecursiveTargetId( object: APObject, - recurseProperty: RecurseProperty, + _recurseProperty: RecurseProperty, ): URL | null { - switch (recurseProperty) { - case "replyTarget": - case IN_REPLY_TO_IRI: - return object.replyTargetId; - default: - return null; - } + return object.replyTargetId; } export async function collectRecursiveObjects( From 7bb56054027e1d143348e89c8563c30ebcc776d9 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 12:08:41 +0900 Subject: [PATCH 08/46] Fix recurse visited scope and output separator destination Scope recurse visited tracking per root URL so each input can emit its full chain independently. Also suppress separator stdout output in non-traverse/non-recurse mode when --output is used, matching other lookup modes. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901194821 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901194817 Co-Authored-By: OpenAI Codex --- packages/cli/src/lookup.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 1751633e7..bbcf7a68e 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -612,10 +612,10 @@ export async function runLookup( if (command.recurse != null) { let totalObjects = 0; - const visited = new Set(); const recurseDepth = command.recurseDepth ?? 20; for (let urlIndex = 0; urlIndex < command.urls.length; urlIndex++) { + const visited = new Set(); const url = command.urls[urlIndex]; if (urlIndex > 0) { spinner.text = `Looking up object chain ${ @@ -884,7 +884,7 @@ export async function runLookup( success = false; } else { spinner.succeed(`Fetched object: ${colors.green(url)}`); - if (printedCount > 0) console.log(command.separator); + if (printedCount > 0 && !command.output) console.log(command.separator); await writeObjectToStream( obj, command.output, From d9e97d2effe8725dd8ba278cb50103e33f2dac9d Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 12:27:46 +0900 Subject: [PATCH 09/46] Block recursive private-address fetches in lookup Reject recursive targets that resolve to localhost or private IP ranges before issuing network requests. Also simplify recurse-depth assignment in recurse mode and add tests for private-address detection and recursive blocking behavior. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901202024 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901202025 Co-Authored-By: OpenAI Codex --- packages/cli/src/lookup.test.ts | 32 +++++++++++++++ packages/cli/src/lookup.ts | 71 +++++++++++++++++++++++++++++++-- 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index e35389af8..7ed97e887 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -14,7 +14,9 @@ import { collectRecursiveObjects, createTimeoutSignal, getRecursiveTargetId, + isPrivateAddressTarget, lookupCommand, + PrivateAddressLookupError, RecursiveLookupError, TimeoutError, writeObjectToStream, @@ -542,6 +544,36 @@ test("collectRecursiveObjects - suppresses errors in best-effort mode", async () assert.deepEqual(result, []); }); +test("isPrivateAddressTarget - detects localhost and private IP ranges", () => { + assert.equal(isPrivateAddressTarget("https://localhost/object"), true); + assert.equal(isPrivateAddressTarget("http://127.0.0.1:3000/"), true); + assert.equal(isPrivateAddressTarget("http://169.254.169.254/latest"), true); + assert.equal(isPrivateAddressTarget("https://example.com/object"), false); +}); + +test("collectRecursiveObjects - blocks private address targets", async () => { + const note = new Note({ + id: new URL("https://example.com/notes/1"), + replyTarget: new URL("http://localhost/internal"), + }); + await assert.rejects( + () => + collectRecursiveObjects( + note, + "replyTarget", + 5, + (target) => { + if (isPrivateAddressTarget(target)) { + throw new PrivateAddressLookupError(target); + } + return Promise.resolve(null); + }, + { suppressErrors: false }, + ), + PrivateAddressLookupError, + ); +}); + test( "collectRecursiveObjects - suppress mode does not poison visited set", async () => { diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index bbcf7a68e..e47220d2d 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -39,6 +39,7 @@ import { } from "@optique/core"; import { path, printError } from "@optique/run"; import { createWriteStream, type WriteStream } from "node:fs"; +import { isIP } from "node:net"; import process from "node:process"; import ora from "ora"; import { configContext } from "./config.ts"; @@ -299,6 +300,58 @@ export class RecursiveLookupError extends Error { } } +export class PrivateAddressLookupError extends Error { + target: string; + constructor(target: string) { + super(`Blocked recursive fetch to private address: ${target}`); + this.name = "PrivateAddressLookupError"; + this.target = target; + } +} + +function isPrivateIpv4(host: string): boolean { + const parts = host.split("."); + if (parts.length !== 4) return false; + const octets = parts.map((part) => Number.parseInt(part, 10)); + if (octets.some((value) => Number.isNaN(value) || value < 0 || value > 255)) { + return false; + } + const [a, b] = octets; + return a === 10 || + a === 127 || + a === 0 || + (a === 169 && b === 254) || + (a === 172 && b >= 16 && b <= 31) || + (a === 192 && b === 168); +} + +function isPrivateIpv6(host: string): boolean { + const normalized = host.toLowerCase().replace(/^\[|\]$/g, ""); + if (normalized === "::1" || normalized === "::") return true; + if (normalized.startsWith("::ffff:")) { + const mapped = normalized.slice("::ffff:".length); + return isPrivateIpv4(mapped); + } + return normalized.startsWith("fc") || normalized.startsWith("fd") || + normalized.startsWith("fe8") || normalized.startsWith("fe9") || + normalized.startsWith("fea") || normalized.startsWith("feb"); +} + +export function isPrivateAddressTarget(target: string): boolean { + let url: URL; + try { + url = new URL(target); + } catch { + return false; + } + if (url.protocol !== "http:" && url.protocol !== "https:") return false; + const host = url.hostname.toLowerCase(); + if (host === "localhost" || host.endsWith(".localhost")) return true; + if (isIP(host) === 4) return isPrivateIpv4(host); + if (isIP(host.replace(/^\[|\]$/g, "")) === 6) return isPrivateIpv6(host); + return false; +} + async function findAllImages(obj: APObject): Promise { const result: URL[] = []; const icon = await obj.getIcon(); @@ -612,7 +665,7 @@ export async function runLookup( if (command.recurse != null) { let totalObjects = 0; - const recurseDepth = command.recurseDepth ?? 20; + const recurseDepth = command.recurseDepth!; for (let urlIndex = 0; urlIndex < command.urls.length; urlIndex++) { const visited = new Set(); @@ -676,12 +729,16 @@ export async function runLookup( current, command.recurse, recurseDepth, - (target) => - lookupObject(target, { + (target) => { + if (isPrivateAddressTarget(target)) { + throw new PrivateAddressLookupError(target); + } + return lookupObject(target, { documentLoader: authLoader ?? documentLoader, contextLoader, userAgent: command.userAgent, - }), + }); + }, { suppressErrors: command.suppressErrors, visited }, ); } catch (error) { @@ -693,6 +750,12 @@ export async function runLookup( ); if (error instanceof TimeoutError) { handleTimeoutError(spinner, command.timeout); + } else if (error instanceof PrivateAddressLookupError) { + spinner.fail( + `Blocked recursive fetch to private address: ${ + colors.red(error.target) + }.`, + ); } else if (error instanceof RecursiveLookupError) { spinner.fail( `Failed to recursively fetch object: ${colors.red(error.target)}.`, From 8ee7f87a4708899c0dcc0d050a5e525d639b329b Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 13:01:51 +0900 Subject: [PATCH 10/46] Use strict loader for recursive lookup network fetches Load recursive objects and contexts with allowPrivateAddress set to false, so recursive fetches rely on the document loader's network-level private-address protections. Remove hostname-string private-address checks and related tests that were replaced by strict loader enforcement. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901239544 Co-Authored-By: OpenAI Codex --- packages/cli/src/docloader.ts | 10 ++-- packages/cli/src/lookup.test.ts | 32 ------------ packages/cli/src/lookup.ts | 90 ++++++++------------------------- 3 files changed, 28 insertions(+), 104 deletions(-) diff --git a/packages/cli/src/docloader.ts b/packages/cli/src/docloader.ts index fff28ef9b..2b8ca03c8 100644 --- a/packages/cli/src/docloader.ts +++ b/packages/cli/src/docloader.ts @@ -9,14 +9,16 @@ const documentLoaders: Record = {}; export interface DocumentLoaderOptions { userAgent?: string; + allowPrivateAddress?: boolean; } export async function getDocumentLoader( - { userAgent }: DocumentLoaderOptions = {}, + { userAgent, allowPrivateAddress = true }: DocumentLoaderOptions = {}, ): Promise { - if (documentLoaders[userAgent ?? ""]) return documentLoaders[userAgent ?? ""]; + const cacheKey = `${userAgent ?? ""}:${allowPrivateAddress}`; + if (documentLoaders[cacheKey]) return documentLoaders[cacheKey]; const kv = await getKvStore(); - return documentLoaders[userAgent ?? ""] = kvCache({ + return documentLoaders[cacheKey] = kvCache({ kv, rules: [ [ @@ -54,7 +56,7 @@ export async function getDocumentLoader( ], ], loader: getDefaultDocumentLoader({ - allowPrivateAddress: true, + allowPrivateAddress, userAgent, }), }); diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index 7ed97e887..e35389af8 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -14,9 +14,7 @@ import { collectRecursiveObjects, createTimeoutSignal, getRecursiveTargetId, - isPrivateAddressTarget, lookupCommand, - PrivateAddressLookupError, RecursiveLookupError, TimeoutError, writeObjectToStream, @@ -544,36 +542,6 @@ test("collectRecursiveObjects - suppresses errors in best-effort mode", async () assert.deepEqual(result, []); }); -test("isPrivateAddressTarget - detects localhost and private IP ranges", () => { - assert.equal(isPrivateAddressTarget("https://localhost/object"), true); - assert.equal(isPrivateAddressTarget("http://127.0.0.1:3000/"), true); - assert.equal(isPrivateAddressTarget("http://169.254.169.254/latest"), true); - assert.equal(isPrivateAddressTarget("https://example.com/object"), false); -}); - -test("collectRecursiveObjects - blocks private address targets", async () => { - const note = new Note({ - id: new URL("https://example.com/notes/1"), - replyTarget: new URL("http://localhost/internal"), - }); - await assert.rejects( - () => - collectRecursiveObjects( - note, - "replyTarget", - 5, - (target) => { - if (isPrivateAddressTarget(target)) { - throw new PrivateAddressLookupError(target); - } - return Promise.resolve(null); - }, - { suppressErrors: false }, - ), - PrivateAddressLookupError, - ); -}); - test( "collectRecursiveObjects - suppress mode does not poison visited set", async () => { diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index e47220d2d..1814b3787 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -39,7 +39,6 @@ import { } from "@optique/core"; import { path, printError } from "@optique/run"; import { createWriteStream, type WriteStream } from "node:fs"; -import { isIP } from "node:net"; import process from "node:process"; import ora from "ora"; import { configContext } from "./config.ts"; @@ -300,58 +299,6 @@ export class RecursiveLookupError extends Error { } } -export class PrivateAddressLookupError extends Error { - target: string; - constructor(target: string) { - super(`Blocked recursive fetch to private address: ${target}`); - this.name = "PrivateAddressLookupError"; - this.target = target; - } -} - -function isPrivateIpv4(host: string): boolean { - const parts = host.split("."); - if (parts.length !== 4) return false; - const octets = parts.map((part) => Number.parseInt(part, 10)); - if (octets.some((value) => Number.isNaN(value) || value < 0 || value > 255)) { - return false; - } - const [a, b] = octets; - return a === 10 || - a === 127 || - a === 0 || - (a === 169 && b === 254) || - (a === 172 && b >= 16 && b <= 31) || - (a === 192 && b === 168); -} - -function isPrivateIpv6(host: string): boolean { - const normalized = host.toLowerCase().replace(/^\[|\]$/g, ""); - if (normalized === "::1" || normalized === "::") return true; - if (normalized.startsWith("::ffff:")) { - const mapped = normalized.slice("::ffff:".length); - return isPrivateIpv4(mapped); - } - return normalized.startsWith("fc") || normalized.startsWith("fd") || - normalized.startsWith("fe8") || normalized.startsWith("fe9") || - normalized.startsWith("fea") || normalized.startsWith("feb"); -} - -export function isPrivateAddressTarget(target: string): boolean { - let url: URL; - try { - url = new URL(target); - } catch { - return false; - } - if (url.protocol !== "http:" && url.protocol !== "https:") return false; - const host = url.hostname.toLowerCase(); - if (host === "localhost" || host.endsWith(".localhost")) return true; - if (isIP(host) === 4) return isPrivateIpv4(host); - if (isIP(host.replace(/^\[|\]$/g, "")) === 6) return isPrivateIpv6(host); - return false; -} - async function findAllImages(obj: APObject): Promise { const result: URL[] = []; const icon = await obj.getIcon(); @@ -663,6 +610,23 @@ export async function runLookup( : "object" }...`; + const recursiveBaseDocumentLoader = await getDocumentLoader({ + userAgent: command.userAgent, + allowPrivateAddress: false, + }); + const recursiveDocumentLoader = wrapDocumentLoaderWithTimeout( + recursiveBaseDocumentLoader, + command.timeout, + ); + const recursiveBaseContextLoader = await getContextLoader({ + userAgent: command.userAgent, + allowPrivateAddress: false, + }); + const recursiveContextLoader = wrapDocumentLoaderWithTimeout( + recursiveBaseContextLoader, + command.timeout, + ); + if (command.recurse != null) { let totalObjects = 0; const recurseDepth = command.recurseDepth!; @@ -729,16 +693,12 @@ export async function runLookup( current, command.recurse, recurseDepth, - (target) => { - if (isPrivateAddressTarget(target)) { - throw new PrivateAddressLookupError(target); - } - return lookupObject(target, { - documentLoader: authLoader ?? documentLoader, - contextLoader, + (target) => + lookupObject(target, { + documentLoader: recursiveDocumentLoader, + contextLoader: recursiveContextLoader, userAgent: command.userAgent, - }); - }, + }), { suppressErrors: command.suppressErrors, visited }, ); } catch (error) { @@ -750,12 +710,6 @@ export async function runLookup( ); if (error instanceof TimeoutError) { handleTimeoutError(spinner, command.timeout); - } else if (error instanceof PrivateAddressLookupError) { - spinner.fail( - `Blocked recursive fetch to private address: ${ - colors.red(error.target) - }.`, - ); } else if (error instanceof RecursiveLookupError) { spinner.fail( `Failed to recursively fetch object: ${colors.red(error.target)}.`, From 5f3fd7a0c16c20b6d73837894e5457c089e731bf Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 13:16:53 +0900 Subject: [PATCH 11/46] Write separators to the configured lookup output Ensure multi-object lookup modes write separators to the same destination as object payloads, including -o/--output file output. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901284197 Co-Authored-By: Codex --- packages/cli/src/lookup.test.ts | 20 ++++++++++++++++++++ packages/cli/src/lookup.ts | 32 ++++++++++++++++++++++++-------- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index e35389af8..22b09a3ec 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -18,6 +18,7 @@ import { RecursiveLookupError, TimeoutError, writeObjectToStream, + writeSeparator, } from "./lookup.ts"; test("writeObjectToStream - writes Note object with default options", async () => { @@ -147,6 +148,25 @@ test("writeObjectToStream - supports reusing an output stream", async () => { await rm(testDir, { recursive: true }); }); +test("writeSeparator - writes to provided output stream", async () => { + const testDir = "./test_output_separator"; + const testFile = `${testDir}/separator.txt`; + await mkdir(testDir, { recursive: true }); + + const stream = createWriteStream(testFile); + await writeSeparator("----", stream); + await new Promise((resolve, reject) => { + stream.end((error?: Error | null) => { + if (error != null) reject(error); + else resolve(); + }); + }); + + const content = await readFile(testFile, { encoding: "utf8" }); + assert.strictEqual(content, "----\n"); + await rm(testDir, { recursive: true }); +}); + test("writeObjectToStream - writes to stdout when no output file specified", async () => { const note = new Note({ id: new URL("https://example.com/notes/1"), diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 1814b3787..da84ac18c 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -387,6 +387,22 @@ async function closeWriteStream(stream?: WriteStream): Promise { }); } +export async function writeSeparator( + separator: string, + stream?: NodeJS.WritableStream, +): Promise { + if (stream == null) { + console.log(separator); + return; + } + await new Promise((resolve, reject) => { + stream.write(`${separator}\n`, (error) => { + if (error != null) reject(error); + else resolve(); + }); + }); +} + const signalTimers = new WeakMap(); export function createTimeoutSignal( @@ -671,8 +687,8 @@ export async function runLookup( return; } - if (totalObjects > 0 && !command.output) { - console.log(command.separator); + if (totalObjects > 0) { + await writeSeparator(command.separator, outputStream); } await writeObjectToStream( current, @@ -736,9 +752,7 @@ export async function runLookup( } for (const next of chain) { - if (!command.output) { - console.log(command.separator); - } + await writeSeparator(command.separator, outputStream); await writeObjectToStream( next, command.output, @@ -817,8 +831,8 @@ export async function runLookup( suppressError: command.suppressErrors, }) ) { - if (!command.output && (totalItems > 0 || collectionItems > 0)) { - console.log(command.separator); + if (totalItems > 0 || collectionItems > 0) { + await writeSeparator(command.separator, outputStream); } await writeObjectToStream( item, @@ -901,7 +915,9 @@ export async function runLookup( success = false; } else { spinner.succeed(`Fetched object: ${colors.green(url)}`); - if (printedCount > 0 && !command.output) console.log(command.separator); + if (printedCount > 0) { + await writeSeparator(command.separator, outputStream); + } await writeObjectToStream( obj, command.output, From af550c27303262674468af79f86d08a956b839a7 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 13:17:06 +0900 Subject: [PATCH 12/46] Block private-address image fetches in lookup rendering Validate remote image URLs with public-address checks before fetching so recursive lookup image rendering cannot SSRF private network targets. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901284201 Co-Authored-By: Codex --- packages/cli/src/imagerenderer.test.ts | 41 ++++++++++++++++++++++++++ packages/cli/src/imagerenderer.ts | 5 +++- 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/imagerenderer.test.ts diff --git a/packages/cli/src/imagerenderer.test.ts b/packages/cli/src/imagerenderer.test.ts new file mode 100644 index 000000000..5bc27497d --- /dev/null +++ b/packages/cli/src/imagerenderer.test.ts @@ -0,0 +1,41 @@ +import assert from "node:assert/strict"; +import { rm, stat } from "node:fs/promises"; +import path from "node:path"; +import test from "node:test"; +import { downloadImage } from "./imagerenderer.ts"; + +test("downloadImage - skips private URL without fetching", async () => { + let called = false; + const originalFetch = globalThis.fetch; + globalThis.fetch = ((_input: URL | RequestInfo) => { + called = true; + return Promise.resolve(new Response(new Uint8Array([1, 2, 3]))); + }) as typeof fetch; + try { + const result = await downloadImage("http://localhost/image.png"); + assert.equal(result, null); + assert.equal(called, false); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("downloadImage - writes file for public URL", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = + ((_input: URL | RequestInfo) => + Promise.resolve(new Response(new Uint8Array([1, 2, 3])))) as typeof fetch; + + let result: string | null = null; + try { + result = await downloadImage("https://example.com/image.png"); + assert.notEqual(result, null); + const fileStat = await stat(result!); + assert.equal(fileStat.isFile(), true); + } finally { + globalThis.fetch = originalFetch; + if (result != null) { + await rm(path.dirname(result), { recursive: true, force: true }); + } + } +}); diff --git a/packages/cli/src/imagerenderer.ts b/packages/cli/src/imagerenderer.ts index e48dd3d45..f2ad11e43 100644 --- a/packages/cli/src/imagerenderer.ts +++ b/packages/cli/src/imagerenderer.ts @@ -1,4 +1,5 @@ import { encodeBase64 } from "byte-encodings/base64"; +import { UrlError, validatePublicUrl } from "@fedify/vocab-runtime"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -103,6 +104,7 @@ export async function renderImageITerm2( export async function downloadImage(url: string): Promise { try { + await validatePublicUrl(url); const response = await fetch(url); const imageData = new Uint8Array(await response.arrayBuffer()); const extension = new URL(url).pathname.split(".").pop() || "jpg"; @@ -112,7 +114,8 @@ export async function downloadImage(url: string): Promise { await fs.writeFile(tempPath, imageData); return tempPath; - } catch (_error) { + } catch (error) { + if (error instanceof UrlError) return null; return null; } } From b27d76ae7758a45ddb07bf35dcfe4b1d89a1885d Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 13:17:12 +0900 Subject: [PATCH 13/46] Isolate docloader cache keys by network policy Namespace CLI document-loader KV cache entries by user agent and allowPrivateAddress policy so strict and permissive loaders cannot share cached documents. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901284203 Co-Authored-By: Codex --- packages/cli/src/docloader.test.ts | 15 +++++++++++++++ packages/cli/src/docloader.ts | 14 ++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 packages/cli/src/docloader.test.ts diff --git a/packages/cli/src/docloader.test.ts b/packages/cli/src/docloader.test.ts new file mode 100644 index 000000000..260e36d78 --- /dev/null +++ b/packages/cli/src/docloader.test.ts @@ -0,0 +1,15 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { getDocumentLoaderCachePrefix } from "./docloader.ts"; + +test("getDocumentLoaderCachePrefix - isolates strict and permissive policies", () => { + const strictPrefix = getDocumentLoaderCachePrefix("fedify-cli", false); + const permissivePrefix = getDocumentLoaderCachePrefix("fedify-cli", true); + assert.notDeepEqual(strictPrefix, permissivePrefix); +}); + +test("getDocumentLoaderCachePrefix - includes user agent namespace", () => { + const prefixA = getDocumentLoaderCachePrefix("agent-a", false); + const prefixB = getDocumentLoaderCachePrefix("agent-b", false); + assert.notDeepEqual(prefixA, prefixB); +}); diff --git a/packages/cli/src/docloader.ts b/packages/cli/src/docloader.ts index 2b8ca03c8..623c14339 100644 --- a/packages/cli/src/docloader.ts +++ b/packages/cli/src/docloader.ts @@ -12,6 +12,19 @@ export interface DocumentLoaderOptions { allowPrivateAddress?: boolean; } +export function getDocumentLoaderCachePrefix( + userAgent: string | undefined, + allowPrivateAddress: boolean, +): readonly [string, ...string[]] { + return [ + "_fedify", + "remoteDocument", + "cli", + userAgent ?? "", + allowPrivateAddress ? "allow-private" : "deny-private", + ]; +} + export async function getDocumentLoader( { userAgent, allowPrivateAddress = true }: DocumentLoaderOptions = {}, ): Promise { @@ -20,6 +33,7 @@ export async function getDocumentLoader( const kv = await getKvStore(); return documentLoaders[cacheKey] = kvCache({ kv, + prefix: getDocumentLoaderCachePrefix(userAgent, allowPrivateAddress), rules: [ [ new URLPattern({ From 1db0a8b68449b514f59bbf80666f1b35ab4c6f01 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 13:25:20 +0900 Subject: [PATCH 14/46] Simplify image download error handling in lookup Remove redundant UrlError branching in downloadImage() and keep the same null-on-error behavior with a single catch path. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901302648 Co-Authored-By: Codex --- packages/cli/src/imagerenderer.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/imagerenderer.ts b/packages/cli/src/imagerenderer.ts index f2ad11e43..a858f2ace 100644 --- a/packages/cli/src/imagerenderer.ts +++ b/packages/cli/src/imagerenderer.ts @@ -1,5 +1,5 @@ import { encodeBase64 } from "byte-encodings/base64"; -import { UrlError, validatePublicUrl } from "@fedify/vocab-runtime"; +import { validatePublicUrl } from "@fedify/vocab-runtime"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -114,8 +114,7 @@ export async function downloadImage(url: string): Promise { await fs.writeFile(tempPath, imageData); return tempPath; - } catch (error) { - if (error instanceof UrlError) return null; + } catch (_error) { return null; } } From 5e3be4acc7230416ead3dd9b243745d663f663d5 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 13:40:59 +0900 Subject: [PATCH 15/46] Support quoteUrl recursion targets in lookup Expand --recurse to follow quote relationships via quoteUrl and the ActivityStreams, Misskey, and Fedibird quote IRIs, and document the new accepted values. https://github.com/fedify-dev/fedify/issues/606 https://github.com/fedify-dev/fedify/pull/608 Co-Authored-By: Codex --- CHANGES.md | 7 ++-- docs/cli.md | 13 +++++-- packages/cli/src/config.ts | 4 +++ packages/cli/src/lookup.test.ts | 64 +++++++++++++++++++++++++++++++++ packages/cli/src/lookup.ts | 26 +++++++++++--- 5 files changed, 104 insertions(+), 10 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index f31a70c2c..c9db56107 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -28,9 +28,10 @@ To be released. - Added `--recurse` and `--recurse-depth` options to `fedify lookup` for recursively following object relationships (e.g., reply chains via - `replyTarget` / `inReplyTo`). `--traverse` and `--recurse` are now - mutually exclusive, `--recurse-depth` depends on `--recurse`, and - `--suppress-errors` now works in recurse mode as best-effort lookup. + `replyTarget` / `inReplyTo`, and quote chains via `quoteUrl` and quote + IRIs). `--traverse` and `--recurse` are now mutually exclusive, + `--recurse-depth` depends on `--recurse`, and `--suppress-errors` now + works in recurse mode as best-effort lookup. [[#606], [#608]] [#606]: https://github.com/fedify-dev/fedify/issues/606 diff --git a/docs/cli.md b/docs/cli.md index 163051cf5..c60ec970e 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -502,7 +502,8 @@ When this option is enabled, each argument has to resolve to a collection. *This option is available since Fedify 2.1.0.* The `--recurse` option is used to recursively follow an object relationship. -This is useful when you want to walk a reply chain through `replyTarget`. +This is useful when you want to walk a reply chain through `replyTarget`, or +follow quote relationships through `quoteUrl`. ~~~~ sh fedify lookup --recurse=replyTarget https://hollo.social/@fedify/019c8522-b247-79d3-b0e7-c6a2293bb1cf @@ -514,8 +515,16 @@ You can also provide the fully qualified property IRI: fedify lookup --recurse=https://www.w3.org/ns/activitystreams#inReplyTo https://hollo.social/@fedify/019c8522-b247-79d3-b0e7-c6a2293bb1cf ~~~~ +For quote relationships, both the short form and the full IRI are accepted: + +~~~~ sh +fedify lookup --recurse=quoteUrl https://hollo.social/@fedify/019c8522-b247-79d3-b0e7-c6a2293bb1cf +fedify lookup --recurse=https://www.w3.org/ns/activitystreams#quoteUrl https://hollo.social/@fedify/019c8522-b247-79d3-b0e7-c6a2293bb1cf +~~~~ + For short names, only Fedify property naming is accepted. For example, -`replyTarget` is accepted, while `inReplyTo` is not accepted as a short form. +`replyTarget` and `quoteUrl` are accepted, while `inReplyTo`, `_misskey_quote`, +and `quoteUri` are not accepted as short forms. > [!NOTE] > `--recurse` and [`-t`/`--traverse`](#t-traverse-traverse-the-collection) diff --git a/packages/cli/src/config.ts b/packages/cli/src/config.ts index 1143be48b..95ed9be77 100644 --- a/packages/cli/src/config.ts +++ b/packages/cli/src/config.ts @@ -37,7 +37,11 @@ const lookupSchema = object({ recurse: optional( picklist([ "replyTarget", + "quoteUrl", "https://www.w3.org/ns/activitystreams#inReplyTo", + "https://www.w3.org/ns/activitystreams#quoteUrl", + "https://misskey-hub.net/ns#_misskey_quote", + "http://fedibird.com/ns#quoteUri", ]), ), recurseDepth: optional(pipe(number(), integer(), minValue(1))), diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index 22b09a3ec..35526c55d 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -435,6 +435,44 @@ test("lookupCommand - accepts IRI inReplyTo", () => { } }); +test("lookupCommand - accepts short-form quoteUrl", () => { + setActiveConfig(configContext.id, {}); + const result = parse(lookupCommand, [ + "lookup", + "--recurse", + "quoteUrl", + "https://example.com/notes/1", + ]); + clearActiveConfig(configContext.id); + assert.ok(result.success); + if (result.success) { + assert.strictEqual(result.value.recurse, "quoteUrl"); + } +}); + +for ( + const recurseProperty of [ + "https://www.w3.org/ns/activitystreams#quoteUrl", + "https://misskey-hub.net/ns#_misskey_quote", + "http://fedibird.com/ns#quoteUri", + ] +) { + test(`lookupCommand - accepts IRI ${recurseProperty}`, () => { + setActiveConfig(configContext.id, {}); + const result = parse(lookupCommand, [ + "lookup", + "--recurse", + recurseProperty, + "https://example.com/notes/1", + ]); + clearActiveConfig(configContext.id); + assert.ok(result.success); + if (result.success) { + assert.strictEqual(result.value.recurse, recurseProperty); + } + }); +} + test("getRecursiveTargetId - returns reply target for short name", () => { const replyTarget = new URL("https://example.com/notes/0"); const note = new Note({ @@ -459,6 +497,32 @@ test("getRecursiveTargetId - returns reply target for IRI", () => { ); }); +test("getRecursiveTargetId - returns quote URL for short name", () => { + const quoteUrl = new URL("https://example.com/notes/quoted"); + const note = new Note({ + id: new URL("https://example.com/notes/1"), + quoteUrl, + }); + assert.equal(getRecursiveTargetId(note, "quoteUrl"), quoteUrl); +}); + +for ( + const recurseProperty of [ + "https://www.w3.org/ns/activitystreams#quoteUrl", + "https://misskey-hub.net/ns#_misskey_quote", + "http://fedibird.com/ns#quoteUri", + ] as const +) { + test(`getRecursiveTargetId - returns quote URL for IRI ${recurseProperty}`, () => { + const quoteUrl = new URL("https://example.com/notes/quoted"); + const note = new Note({ + id: new URL("https://example.com/notes/1"), + quoteUrl, + }); + assert.equal(getRecursiveTargetId(note, recurseProperty), quoteUrl); + }); +} + test("collectRecursiveObjects - follows chain up to depth limit", async () => { const note1 = new Note({ id: new URL("https://example.com/notes/1"), diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index da84ac18c..2ba135779 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -56,7 +56,17 @@ import { colorEnabled, colors, formatObject } from "./utils.ts"; const logger = getLogger(["fedify", "cli", "lookup"]); const IN_REPLY_TO_IRI = "https://www.w3.org/ns/activitystreams#inReplyTo"; -const recurseProperties = ["replyTarget", IN_REPLY_TO_IRI] as const; +const QUOTE_URL_IRI = "https://www.w3.org/ns/activitystreams#quoteUrl"; +const MISSKEY_QUOTE_IRI = "https://misskey-hub.net/ns#_misskey_quote"; +const FEDIBIRD_QUOTE_IRI = "http://fedibird.com/ns#quoteUri"; +const recurseProperties = [ + "replyTarget", + "quoteUrl", + IN_REPLY_TO_IRI, + QUOTE_URL_IRI, + MISSKEY_QUOTE_IRI, + FEDIBIRD_QUOTE_IRI, +] as const; type RecurseProperty = typeof recurseProperties[number]; export const authorizedFetchOption = withDefault( @@ -109,8 +119,8 @@ const lookupModeOption = withDefault( choice(recurseProperties, { metavar: "PROPERTY" }), { description: message`Recursively follow a relationship property (${ - optionNames(["replyTarget"]) - } or ${IN_REPLY_TO_IRI}).`, + optionNames(["replyTarget", "quoteUrl"]) + }, ${IN_REPLY_TO_IRI}, ${QUOTE_URL_IRI}, ${MISSKEY_QUOTE_IRI}, or ${FEDIBIRD_QUOTE_IRI}).`, }, ), { @@ -458,9 +468,15 @@ function handleTimeoutError( export function getRecursiveTargetId( object: APObject, - _recurseProperty: RecurseProperty, + recurseProperty: RecurseProperty, ): URL | null { - return object.replyTargetId; + if ( + recurseProperty === "replyTarget" || recurseProperty === IN_REPLY_TO_IRI + ) { + return object.replyTargetId; + } + const quoteUrl = (object as { quoteUrl?: unknown }).quoteUrl; + return quoteUrl instanceof URL ? quoteUrl : null; } export async function collectRecursiveObjects( From 77c8e7e817f1d9b6294a6d1d5cb3be6efc2e452a Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 13:47:19 +0900 Subject: [PATCH 16/46] Let Optique show recurse choices in help output Keep the --recurse description concise and remove redundant allowed-value listing from option help text. https://github.com/fedify-dev/fedify/pull/608 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 2ba135779..9a4fed298 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -118,9 +118,7 @@ const lookupModeOption = withDefault( "--recurse", choice(recurseProperties, { metavar: "PROPERTY" }), { - description: message`Recursively follow a relationship property (${ - optionNames(["replyTarget", "quoteUrl"]) - }, ${IN_REPLY_TO_IRI}, ${QUOTE_URL_IRI}, ${MISSKEY_QUOTE_IRI}, or ${FEDIBIRD_QUOTE_IRI}).`, + description: message`Recursively follow a relationship property.`, }, ), { From 4f2440153a023c8fba3962156c2e7629b95512a6 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 13:57:55 +0900 Subject: [PATCH 17/46] Lazily initialize recurse-only document loaders Move deny-private recurse loader setup inside recurse mode to avoid unnecessary initialization on non-recurse lookup paths. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901333866 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 9a4fed298..8feb3cdfe 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -640,24 +640,23 @@ export async function runLookup( : "object" }...`; - const recursiveBaseDocumentLoader = await getDocumentLoader({ - userAgent: command.userAgent, - allowPrivateAddress: false, - }); - const recursiveDocumentLoader = wrapDocumentLoaderWithTimeout( - recursiveBaseDocumentLoader, - command.timeout, - ); - const recursiveBaseContextLoader = await getContextLoader({ - userAgent: command.userAgent, - allowPrivateAddress: false, - }); - const recursiveContextLoader = wrapDocumentLoaderWithTimeout( - recursiveBaseContextLoader, - command.timeout, - ); - if (command.recurse != null) { + const recursiveBaseDocumentLoader = await getDocumentLoader({ + userAgent: command.userAgent, + allowPrivateAddress: false, + }); + const recursiveDocumentLoader = wrapDocumentLoaderWithTimeout( + recursiveBaseDocumentLoader, + command.timeout, + ); + const recursiveBaseContextLoader = await getContextLoader({ + userAgent: command.userAgent, + allowPrivateAddress: false, + }); + const recursiveContextLoader = wrapDocumentLoaderWithTimeout( + recursiveBaseContextLoader, + command.timeout, + ); let totalObjects = 0; const recurseDepth = command.recurseDepth!; From 9fa76c7250fd369050587bb361f0df699eaef416 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 13:58:20 +0900 Subject: [PATCH 18/46] Apply authorized fetch to recursive follow-up lookups Use the authenticated document loader in recurse-chain fetches when --authorized-fetch is enabled. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901333871 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 8feb3cdfe..bf459012c 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -724,7 +724,7 @@ export async function runLookup( recurseDepth, (target) => lookupObject(target, { - documentLoader: recursiveDocumentLoader, + documentLoader: authLoader ?? recursiveDocumentLoader, contextLoader: recursiveContextLoader, userAgent: command.userAgent, }), From ff2387587d12635f25afe85e70aa89bc0fd14faa Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 14:10:27 +0900 Subject: [PATCH 19/46] Await stdout backpressure in writeSeparator Use process.stdout.write() in writeSeparator() when no stream is provided so the function can await the write callback just like the file-stream branch. Add a regression test that stubs stdout and verifies the separator is written through the awaited stdout path. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901349548 Co-Authored-By: Codex --- packages/cli/src/lookup.test.ts | 21 +++++++++++++++++++++ packages/cli/src/lookup.ts | 7 ++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index 35526c55d..b7f31ada1 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -3,8 +3,10 @@ import { clearActiveConfig, setActiveConfig } from "@optique/config"; import { runWithConfig } from "@optique/config/run"; import { parse } from "@optique/core/parser"; import assert from "node:assert/strict"; +import { Buffer } from "node:buffer"; import { createWriteStream } from "node:fs"; import { mkdir, readFile, rm } from "node:fs/promises"; +import process from "node:process"; import test from "node:test"; import { configContext } from "./config.ts"; import { getContextLoader } from "./docloader.ts"; @@ -167,6 +169,25 @@ test("writeSeparator - writes to provided output stream", async () => { await rm(testDir, { recursive: true }); }); +test("writeSeparator - writes to stdout when no stream is provided", async () => { + const originalWrite = process.stdout.write; + let output = ""; + process.stdout.write = + ((chunk: string | Uint8Array, callback?: () => void) => { + output += typeof chunk === "string" + ? chunk + : Buffer.from(chunk).toString(); + callback?.(); + return true; + }) as typeof process.stdout.write; + try { + await writeSeparator("----"); + assert.strictEqual(output, "----\n"); + } finally { + process.stdout.write = originalWrite; + } +}); + test("writeObjectToStream - writes to stdout when no output file specified", async () => { const note = new Note({ id: new URL("https://example.com/notes/1"), diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index bf459012c..81d09567c 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -400,7 +400,12 @@ export async function writeSeparator( stream?: NodeJS.WritableStream, ): Promise { if (stream == null) { - console.log(separator); + await new Promise((resolve, reject) => { + process.stdout.write(`${separator}\n`, (error) => { + if (error != null) reject(error); + else resolve(); + }); + }); return; } await new Promise((resolve, reject) => { From 21b087ec6797b95a2fbc32a0aa2495dda2ad3724 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 14:12:24 +0900 Subject: [PATCH 20/46] Handle stream error events in lookup output Guard writeObjectToStream() writes and closes with temporary error listeners so stream error events reject the pending operation instead of surfacing as unhandled emitter errors. Add a regression test that injects a failing writable stream and verifies writeObjectToStream() rejects with the write failure. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901349557 Co-Authored-By: Codex --- packages/cli/src/lookup.test.ts | 21 +++++++++++++++++ packages/cli/src/lookup.ts | 40 +++++++++++++++++++++++---------- 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index b7f31ada1..72356c71a 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -7,6 +7,7 @@ import { Buffer } from "node:buffer"; import { createWriteStream } from "node:fs"; import { mkdir, readFile, rm } from "node:fs/promises"; import process from "node:process"; +import { Writable } from "node:stream"; import test from "node:test"; import { configContext } from "./config.ts"; import { getContextLoader } from "./docloader.ts"; @@ -221,6 +222,26 @@ test("writeObjectToStream - handles empty content properly", async () => { await rm(testDir, { recursive: true }); }); +test("writeObjectToStream - rejects when stream emits write error", async () => { + const note = new Note({ + id: new URL("https://example.com/notes/1"), + content: "Test stream error", + }); + const contextLoader = await getContextLoader({}); + const stream = new Writable({ + write(_chunk, _encoding, callback) { + this.emit("error", new Error("write failed")); + callback(); + }, + }); + + await assert.rejects( + () => + writeObjectToStream(note, undefined, undefined, contextLoader, stream), + /write failed/, + ); +}); + test("createTimeoutSignal - returns undefined when no timeout specified", () => { const signal = createTimeoutSignal(); assert.strictEqual(signal, undefined); diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 81d09567c..3f57bfafc 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -334,6 +334,32 @@ export async function writeObjectToStream( const localFileStream = stream == null && outputPath != null ? localStream as WriteStream : undefined; + const writeChunk = (target: NodeJS.WritableStream, chunk: Uint8Array) => + new Promise((resolve, reject) => { + const onError = (error: Error) => { + target.off("error", onError); + reject(error); + }; + target.once("error", onError); + target.write(chunk, (error) => { + target.off("error", onError); + if (error != null) reject(error); + else resolve(); + }); + }); + const endStream = (target: WriteStream) => + new Promise((resolve, reject) => { + const onError = (error: Error) => { + target.off("error", onError); + reject(error); + }; + target.once("error", onError); + target.end((error?: Error | null) => { + target.off("error", onError); + if (error != null) reject(error); + else resolve(); + }); + }); let content; let json = true; @@ -361,20 +387,10 @@ export async function writeObjectToStream( const encoder = new TextEncoder(); const bytes = encoder.encode(content + "\n"); - await new Promise((resolve, reject) => { - localStream.write(bytes, (error) => { - if (error != null) reject(error); - else resolve(); - }); - }); + await writeChunk(localStream, bytes); if (localFileStream != null) { - await new Promise((resolve, reject) => { - localFileStream.end((error?: Error | null) => { - if (error != null) reject(error); - else resolve(); - }); - }); + await endStream(localFileStream); } if (object instanceof APObject) { From c6851fa73d7300019624a286b02f0d6bfab76675 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 14:13:21 +0900 Subject: [PATCH 21/46] Lazily create runLookup output stream Delay output stream creation in runLookup() until the first write so non-output code paths do not initialize a file stream. Attach an error listener when the stream is created and reuse a lazy getter at each separator/object write call site so output stream errors are surfaced consistently. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901349527 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 3f57bfafc..6b06c671e 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -583,9 +583,21 @@ export async function runLookup( ); let authLoader: DocumentLoader | undefined = undefined; - const outputStream = command.output == null - ? undefined - : createWriteStream(command.output); + let outputStream: WriteStream | undefined; + let outputStreamError: Error | undefined; + const getOutputStream = (): WriteStream | undefined => { + if (command.output == null) return undefined; + if (outputStream == null) { + outputStream = createWriteStream(command.output); + outputStream.once("error", (error) => { + outputStreamError = error; + }); + } + if (outputStreamError != null) { + throw outputStreamError; + } + return outputStream; + }; const finalizeAndExit = async (code: number) => { await closeWriteStream(outputStream); await server?.close(); @@ -722,14 +734,14 @@ export async function runLookup( } if (totalObjects > 0) { - await writeSeparator(command.separator, outputStream); + await writeSeparator(command.separator, getOutputStream()); } await writeObjectToStream( current, command.output, command.format, contextLoader, - outputStream, + getOutputStream(), ); totalObjects++; visited.add(url); @@ -786,13 +798,13 @@ export async function runLookup( } for (const next of chain) { - await writeSeparator(command.separator, outputStream); + await writeSeparator(command.separator, getOutputStream()); await writeObjectToStream( next, command.output, command.format, contextLoader, - outputStream, + getOutputStream(), ); totalObjects++; } @@ -866,14 +878,14 @@ export async function runLookup( }) ) { if (totalItems > 0 || collectionItems > 0) { - await writeSeparator(command.separator, outputStream); + await writeSeparator(command.separator, getOutputStream()); } await writeObjectToStream( item, command.output, command.format, contextLoader, - outputStream, + getOutputStream(), ); collectionItems++; totalItems++; @@ -950,14 +962,14 @@ export async function runLookup( } else { spinner.succeed(`Fetched object: ${colors.green(url)}`); if (printedCount > 0) { - await writeSeparator(command.separator, outputStream); + await writeSeparator(command.separator, getOutputStream()); } await writeObjectToStream( obj, command.output, command.format, contextLoader, - outputStream, + getOutputStream(), ); printedCount++; } From e09a2a4a700bf793d9483b621a9fe6a1450fb74c Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 14:44:56 +0900 Subject: [PATCH 22/46] Handle recurse output write failures safely Wrap recurse-mode separator/object writes in try/catch so output stream failures do not escape runLookup() without cleanup. On write failure, log the error, fail the spinner message, and route through finalizeAndExit(1) to close resources consistently. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901378067 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 50 ++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 6b06c671e..bfa8be5b7 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -733,16 +733,23 @@ export async function runLookup( return; } - if (totalObjects > 0) { - await writeSeparator(command.separator, getOutputStream()); + try { + if (totalObjects > 0) { + await writeSeparator(command.separator, getOutputStream()); + } + await writeObjectToStream( + current, + command.output, + command.format, + contextLoader, + getOutputStream(), + ); + } catch (error) { + logger.error("Failed to write lookup output: {error}", { error }); + spinner.fail("Failed to write output."); + await finalizeAndExit(1); + return; } - await writeObjectToStream( - current, - command.output, - command.format, - contextLoader, - getOutputStream(), - ); totalObjects++; visited.add(url); if (current.id != null) { @@ -798,15 +805,22 @@ export async function runLookup( } for (const next of chain) { - await writeSeparator(command.separator, getOutputStream()); - await writeObjectToStream( - next, - command.output, - command.format, - contextLoader, - getOutputStream(), - ); - totalObjects++; + try { + await writeSeparator(command.separator, getOutputStream()); + await writeObjectToStream( + next, + command.output, + command.format, + contextLoader, + getOutputStream(), + ); + totalObjects++; + } catch (error) { + logger.error("Failed to write lookup output: {error}", { error }); + spinner.fail("Failed to write output."); + await finalizeAndExit(1); + return; + } } } From 1248f71bd1c0509daa89a07137aacaf4734190c1 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 14:46:48 +0900 Subject: [PATCH 23/46] Guard multi-object output writes in lookup Protect non-traverse multi-object output writes with try/catch so stream write failures are handled through finalizeAndExit(1). Log write errors and report a user-facing failure message instead of letting exceptions bypass cleanup for the temporary server/output. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901378072 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index bfa8be5b7..a20a78201 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -975,16 +975,23 @@ export async function runLookup( success = false; } else { spinner.succeed(`Fetched object: ${colors.green(url)}`); - if (printedCount > 0) { - await writeSeparator(command.separator, getOutputStream()); + try { + if (printedCount > 0) { + await writeSeparator(command.separator, getOutputStream()); + } + await writeObjectToStream( + obj, + command.output, + command.format, + contextLoader, + getOutputStream(), + ); + } catch (error) { + logger.error("Failed to write lookup output: {error}", { error }); + spinner.fail("Failed to write output."); + await finalizeAndExit(1); + return; } - await writeObjectToStream( - obj, - command.output, - command.format, - contextLoader, - getOutputStream(), - ); printedCount++; } } From 09d23382c61bd8bc3f17c95ef3be93cbe5f7b36a Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 14:47:14 +0900 Subject: [PATCH 24/46] Align recurse rendering with strict context policy Use recursiveContextLoader when rendering recurse-mode objects so JSON-LD context resolution follows the same deny-private-address policy as recursive object fetches. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901378070 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index a20a78201..d9d4fb67d 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -741,7 +741,7 @@ export async function runLookup( current, command.output, command.format, - contextLoader, + recursiveContextLoader, getOutputStream(), ); } catch (error) { @@ -811,7 +811,7 @@ export async function runLookup( next, command.output, command.format, - contextLoader, + recursiveContextLoader, getOutputStream(), ); totalObjects++; From 204cdc653233248a0349f1db9c1ec32fcd743066 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 15:06:45 +0900 Subject: [PATCH 25/46] Harden stream error handling in lookup output Refactor stream writes/closes to shared helpers that listen for error events as well as callback errors, and use them across writeObjectToStream(), writeSeparator(), and closeWriteStream(). Add a regression test for separator writes that fail through the stream error event path. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901388061 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901388063 Co-Authored-By: Codex --- packages/cli/src/lookup.test.ts | 14 ++++++ packages/cli/src/lookup.ts | 86 ++++++++++++++------------------- 2 files changed, 51 insertions(+), 49 deletions(-) diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index 72356c71a..9635d7191 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -189,6 +189,20 @@ test("writeSeparator - writes to stdout when no stream is provided", async () => } }); +test("writeSeparator - rejects when stream emits write error", async () => { + const stream = new Writable({ + write(_chunk, _encoding, callback) { + this.emit("error", new Error("separator write failed")); + callback(); + }, + }); + + await assert.rejects( + () => writeSeparator("----", stream), + /separator write failed/, + ); +}); + test("writeObjectToStream - writes to stdout when no output file specified", async () => { const note = new Note({ id: new URL("https://example.com/notes/1"), diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index d9d4fb67d..b3de6e9c1 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -307,6 +307,39 @@ export class RecursiveLookupError extends Error { } } +function writeToStream( + stream: NodeJS.WritableStream, + chunk: string | Uint8Array, +): Promise { + return new Promise((resolve, reject) => { + const onError = (error: Error) => { + stream.off("error", onError); + reject(error); + }; + stream.once("error", onError); + stream.write(chunk, (error) => { + stream.off("error", onError); + if (error != null) reject(error); + else resolve(); + }); + }); +} + +function endWritableStream(stream: WriteStream): Promise { + return new Promise((resolve, reject) => { + const onError = (error: Error) => { + stream.off("error", onError); + reject(error); + }; + stream.once("error", onError); + stream.end((error?: Error | null) => { + stream.off("error", onError); + if (error != null) reject(error); + else resolve(); + }); + }); +} + async function findAllImages(obj: APObject): Promise { const result: URL[] = []; const icon = await obj.getIcon(); @@ -334,32 +367,6 @@ export async function writeObjectToStream( const localFileStream = stream == null && outputPath != null ? localStream as WriteStream : undefined; - const writeChunk = (target: NodeJS.WritableStream, chunk: Uint8Array) => - new Promise((resolve, reject) => { - const onError = (error: Error) => { - target.off("error", onError); - reject(error); - }; - target.once("error", onError); - target.write(chunk, (error) => { - target.off("error", onError); - if (error != null) reject(error); - else resolve(); - }); - }); - const endStream = (target: WriteStream) => - new Promise((resolve, reject) => { - const onError = (error: Error) => { - target.off("error", onError); - reject(error); - }; - target.once("error", onError); - target.end((error?: Error | null) => { - target.off("error", onError); - if (error != null) reject(error); - else resolve(); - }); - }); let content; let json = true; @@ -387,10 +394,10 @@ export async function writeObjectToStream( const encoder = new TextEncoder(); const bytes = encoder.encode(content + "\n"); - await writeChunk(localStream, bytes); + await writeToStream(localStream, bytes); if (localFileStream != null) { - await endStream(localFileStream); + await endWritableStream(localFileStream); } if (object instanceof APObject) { @@ -403,33 +410,14 @@ export async function writeObjectToStream( async function closeWriteStream(stream?: WriteStream): Promise { if (stream == null) return; - await new Promise((resolve, reject) => { - stream.end((error?: Error | null) => { - if (error != null) reject(error); - else resolve(); - }); - }); + await endWritableStream(stream); } export async function writeSeparator( separator: string, stream?: NodeJS.WritableStream, ): Promise { - if (stream == null) { - await new Promise((resolve, reject) => { - process.stdout.write(`${separator}\n`, (error) => { - if (error != null) reject(error); - else resolve(); - }); - }); - return; - } - await new Promise((resolve, reject) => { - stream.write(`${separator}\n`, (error) => { - if (error != null) reject(error); - else resolve(); - }); - }); + await writeToStream(stream ?? process.stdout, `${separator}\n`); } const signalTimers = new WeakMap(); From dfce3fb33440e31a7b2ac472071fd324beade9a9 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 15:08:42 +0900 Subject: [PATCH 26/46] Apply strict URL policy in recurse lookups Use a recurse-only document loader wrapper that validates every target with validatePublicUrl() before dispatching to authLoader or the deny-private recursive loader. Apply this loader and the deny-private recursive context loader to initial recurse lookup and recursive hops, so --authorized-fetch does not bypass private-address protections. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901388068 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901388074 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index b3de6e9c1..2a2e89071 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -12,7 +12,7 @@ import { Object as APObject, traverseCollection, } from "@fedify/vocab"; -import type { DocumentLoader } from "@fedify/vocab-runtime"; +import { type DocumentLoader, validatePublicUrl } from "@fedify/vocab-runtime"; import type { ResourceDescriptor } from "@fedify/webfinger"; import { getLogger } from "@logtape/logtape"; import { bindConfig } from "@optique/config"; @@ -678,6 +678,13 @@ export async function runLookup( recursiveBaseContextLoader, command.timeout, ); + const recursiveLookupDocumentLoader: DocumentLoader = async ( + url, + options, + ) => { + await validatePublicUrl(url); + return (authLoader ?? recursiveDocumentLoader)(url, options); + }; let totalObjects = 0; const recurseDepth = command.recurseDepth!; @@ -692,8 +699,8 @@ export async function runLookup( let current: APObject | null = null; try { current = await lookupObject(url, { - documentLoader: authLoader ?? documentLoader, - contextLoader, + documentLoader: recursiveLookupDocumentLoader, + contextLoader: recursiveContextLoader, userAgent: command.userAgent, }); } catch (error) { @@ -752,7 +759,7 @@ export async function runLookup( recurseDepth, (target) => lookupObject(target, { - documentLoader: authLoader ?? recursiveDocumentLoader, + documentLoader: recursiveLookupDocumentLoader, contextLoader: recursiveContextLoader, userAgent: command.userAgent, }), From a66d613b5656161176c19d5c6ee7bcc4e9ba2565 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 15:21:10 +0900 Subject: [PATCH 27/46] Deduplicate suppress-errors option definition Extract the repeated --suppress-errors bindConfig() setup into a single suppressErrorsOption constant and reuse it for both recurse and traverse modes. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901403337 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 2a2e89071..cac36a389 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -69,6 +69,18 @@ const recurseProperties = [ ] as const; type RecurseProperty = typeof recurseProperties[number]; +const suppressErrorsOption = bindConfig( + flag("-S", "--suppress-errors", { + description: + message`Suppress partial errors during traversal or recursion.`, + }), + { + context: configContext, + key: (config) => config.lookup?.suppressErrors ?? false, + default: false, + }, +); + export const authorizedFetchOption = withDefault( object("Authorized fetch options", { authorizedFetch: bindConfig( @@ -144,17 +156,7 @@ const lookupModeOption = withDefault( ), 20, ), - suppressErrors: bindConfig( - flag("-S", "--suppress-errors", { - description: - message`Suppress partial errors during traversal or recursion.`, - }), - { - context: configContext, - key: (config) => config.lookup?.suppressErrors ?? false, - default: false, - }, - ), + suppressErrors: suppressErrorsOption, }), object("Traverse options", { traverse: bindConfig( @@ -170,17 +172,7 @@ const lookupModeOption = withDefault( ), recurse: constant(undefined), recurseDepth: constant(undefined), - suppressErrors: bindConfig( - flag("-S", "--suppress-errors", { - description: - message`Suppress partial errors during traversal or recursion.`, - }), - { - context: configContext, - key: (config) => config.lookup?.suppressErrors ?? false, - default: false, - }, - ), + suppressErrors: suppressErrorsOption, }), ), { From 005c4c59f0af5719619ffdb2f0a47c71dba5264a Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 15:22:52 +0900 Subject: [PATCH 28/46] Always exit in finalizeAndExit cleanup path Wrap output-stream and temporary-server cleanup in try/catch inside finalizeAndExit() so process.exit(code) always runs even when cleanup operations fail. Log cleanup failures instead of letting rejections bypass the intended exit code path. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901405586 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index cac36a389..7d7c06f60 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -579,8 +579,23 @@ export async function runLookup( return outputStream; }; const finalizeAndExit = async (code: number) => { - await closeWriteStream(outputStream); - await server?.close(); + try { + await closeWriteStream(outputStream); + } catch (error) { + logger.error("Failed to close output stream during shutdown: {error}", { + error, + }); + } + try { + await server?.close(); + } catch (error) { + logger.error( + "Failed to close temporary server during shutdown: {error}", + { + error, + }, + ); + } process.exit(code); }; From 933784d27915bad9d6b9f16b0698612c23e3f269 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 15:23:26 +0900 Subject: [PATCH 29/46] Remove redundant recurse URL pre-validation Drop the extra validatePublicUrl() call from the recurse lookup wrapper and rely on the underlying strict loaders to enforce private-address URL validation. This avoids duplicate validation and repeated DNS work on each recurse fetch while preserving the deny-private behavior. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901405593 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 7d7c06f60..6a2446091 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -12,7 +12,7 @@ import { Object as APObject, traverseCollection, } from "@fedify/vocab"; -import { type DocumentLoader, validatePublicUrl } from "@fedify/vocab-runtime"; +import type { DocumentLoader } from "@fedify/vocab-runtime"; import type { ResourceDescriptor } from "@fedify/webfinger"; import { getLogger } from "@logtape/logtape"; import { bindConfig } from "@optique/config"; @@ -685,13 +685,8 @@ export async function runLookup( recursiveBaseContextLoader, command.timeout, ); - const recursiveLookupDocumentLoader: DocumentLoader = async ( - url, - options, - ) => { - await validatePublicUrl(url); - return (authLoader ?? recursiveDocumentLoader)(url, options); - }; + const recursiveLookupDocumentLoader: DocumentLoader = authLoader ?? + recursiveDocumentLoader; let totalObjects = 0; const recurseDepth = command.recurseDepth!; From 12ebc6f3a6ccc56181e82065e059a01a55bdd5e7 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 16:40:03 +0900 Subject: [PATCH 30/46] Handle sync stream throw paths in lookup output helpers Guard output stream operations against synchronous throws in both write and end helpers. When a writable stream is already destroyed, Node can throw from stream.write()/stream.end() before callback-based error handling runs. This change wraps those calls in try/catch, removes the temporary error listener on the synchronous-failure path, and rejects via the same Promise channel used for async errors. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901449050 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901449052 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 6a2446091..b025cab67 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -309,11 +309,16 @@ function writeToStream( reject(error); }; stream.once("error", onError); - stream.write(chunk, (error) => { + try { + stream.write(chunk, (error) => { + stream.off("error", onError); + if (error != null) reject(error); + else resolve(); + }); + } catch (error) { stream.off("error", onError); - if (error != null) reject(error); - else resolve(); - }); + reject(error instanceof Error ? error : new Error(String(error))); + } }); } @@ -324,11 +329,16 @@ function endWritableStream(stream: WriteStream): Promise { reject(error); }; stream.once("error", onError); - stream.end((error?: Error | null) => { + try { + stream.end((error?: Error | null) => { + stream.off("error", onError); + if (error != null) reject(error); + else resolve(); + }); + } catch (error) { stream.off("error", onError); - if (error != null) reject(error); - else resolve(); - }); + reject(error instanceof Error ? error : new Error(String(error))); + } }); } From 442f1d5f18e8b54edd5b6818facb6d870555c155 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 16:54:40 +0900 Subject: [PATCH 31/46] Harden lookup output destination and finalization paths Prevent color output from being inferred solely from outputPath and handle cleanup failures in the simple lookup success path. Color rendering is now enabled only when the actual destination is process.stdout, which avoids leaking ANSI escapes into non-stdout streams passed to writeObjectToStream(). The non-traverse/non-recurse success path now wraps output-stream and server finalization in try/catch, logs failures, and exits through finalizeAndExit(1) to avoid unhandled rejections during shutdown. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901499193 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901499201 Co-Authored-By: Codex --- packages/cli/src/lookup.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index b025cab67..da21ec322 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -390,7 +390,7 @@ export async function writeObjectToStream( json = false; } - const enableColors = colorEnabled && outputPath === undefined; + const enableColors = colorEnabled && localStream === process.stdout; content = formatObject(content, enableColors, json); const encoder = new TextEncoder(); @@ -1013,8 +1013,15 @@ export async function runLookup( await finalizeAndExit(1); return; } - await closeWriteStream(outputStream); - await server?.close(); + try { + await closeWriteStream(outputStream); + await server?.close(); + } catch (error) { + logger.error("Failed to finalize lookup resources: {error}", { error }); + spinner.fail("Failed to finalize output."); + await finalizeAndExit(1); + return; + } if (success && command.output) { spinner.succeed( `Successfully wrote output to ${colors.green(command.output)}.`, From 9400c07b65745871c2fa2d3d7f6ae033d19e580c Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 17:13:51 +0900 Subject: [PATCH 32/46] Tighten lookup loader safety and document public helpers Apply follow-up fixes from review for config validation, stream output behavior, and API documentation. The document loader now defaults to deny private addresses, lookup config schema now enforces traverse/recurse mutual exclusion and recurseDepth dependency, and recursive helper exports now include JSDoc. Image rendering now only runs when writing to stdout. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901510813 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901510814 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901510815 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901510816 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901510817 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901515752 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901515762 Co-Authored-By: Codex --- packages/cli/src/config.ts | 58 +++++++++++++++++++++++------------ packages/cli/src/docloader.ts | 6 +++- packages/cli/src/lookup.ts | 11 ++++++- 3 files changed, 53 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/config.ts b/packages/cli/src/config.ts index 95ed9be77..1eaa55115 100644 --- a/packages/cli/src/config.ts +++ b/packages/cli/src/config.ts @@ -6,6 +6,8 @@ import { parse as parseToml } from "smol-toml"; import { array, boolean, + check, + forward, type InferOutput, integer, minValue, @@ -28,28 +30,44 @@ const webfingerSchema = object({ /** * Schema for the lookup command configuration. */ -const lookupSchema = object({ - authorizedFetch: optional(boolean()), - firstKnock: optional( - picklist(["draft-cavage-http-signatures-12", "rfc9421"]), +const lookupSchema = pipe( + object({ + authorizedFetch: optional(boolean()), + firstKnock: optional( + picklist(["draft-cavage-http-signatures-12", "rfc9421"]), + ), + traverse: optional(boolean()), + recurse: optional( + picklist([ + "replyTarget", + "quoteUrl", + "https://www.w3.org/ns/activitystreams#inReplyTo", + "https://www.w3.org/ns/activitystreams#quoteUrl", + "https://misskey-hub.net/ns#_misskey_quote", + "http://fedibird.com/ns#quoteUri", + ]), + ), + recurseDepth: optional(pipe(number(), integer(), minValue(1))), + suppressErrors: optional(boolean()), + defaultFormat: optional(picklist(["default", "raw", "compact", "expand"])), + separator: optional(string()), + timeout: optional(number()), + }), + forward( + check( + (input) => !(input.traverse === true && input.recurse != null), + "lookup.traverse and lookup.recurse cannot be used together.", + ), + ["recurse"], ), - traverse: optional(boolean()), - recurse: optional( - picklist([ - "replyTarget", - "quoteUrl", - "https://www.w3.org/ns/activitystreams#inReplyTo", - "https://www.w3.org/ns/activitystreams#quoteUrl", - "https://misskey-hub.net/ns#_misskey_quote", - "http://fedibird.com/ns#quoteUri", - ]), + forward( + check( + (input) => input.recurse != null || input.recurseDepth == null, + "lookup.recurseDepth requires lookup.recurse.", + ), + ["recurseDepth"], ), - recurseDepth: optional(pipe(number(), integer(), minValue(1))), - suppressErrors: optional(boolean()), - defaultFormat: optional(picklist(["default", "raw", "compact", "expand"])), - separator: optional(string()), - timeout: optional(number()), -}); +); /** * Schema for the inbox command configuration. diff --git a/packages/cli/src/docloader.ts b/packages/cli/src/docloader.ts index 623c14339..223f9b9d1 100644 --- a/packages/cli/src/docloader.ts +++ b/packages/cli/src/docloader.ts @@ -12,6 +12,10 @@ export interface DocumentLoaderOptions { allowPrivateAddress?: boolean; } +/** + * Returns a cache prefix that separates document-loader entries by user agent + * and private-address policy. + */ export function getDocumentLoaderCachePrefix( userAgent: string | undefined, allowPrivateAddress: boolean, @@ -26,7 +30,7 @@ export function getDocumentLoaderCachePrefix( } export async function getDocumentLoader( - { userAgent, allowPrivateAddress = true }: DocumentLoaderOptions = {}, + { userAgent, allowPrivateAddress = false }: DocumentLoaderOptions = {}, ): Promise { const cacheKey = `${userAgent ?? ""}:${allowPrivateAddress}`; if (documentLoaders[cacheKey]) return documentLoaders[cacheKey]; diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index da21ec322..be6f0f519 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -290,6 +290,9 @@ export class TimeoutError extends Error { } } +/** + * Error thrown when a recursive lookup target cannot be fetched. + */ export class RecursiveLookupError extends Error { target: string; constructor(target: string) { @@ -405,7 +408,7 @@ export async function writeObjectToStream( if (object instanceof APObject) { imageUrls = await findAllImages(object); } - if (!outputPath && imageUrls.length > 0) { + if (localStream === process.stdout && imageUrls.length > 0) { await renderImages(imageUrls); } } @@ -475,6 +478,9 @@ function handleTimeoutError( ); } +/** + * Gets the next recursion target URL from an ActivityPub object. + */ export function getRecursiveTargetId( object: APObject, recurseProperty: RecurseProperty, @@ -488,6 +494,9 @@ export function getRecursiveTargetId( return quoteUrl instanceof URL ? quoteUrl : null; } +/** + * Collects recursively linked objects up to a depth limit. + */ export async function collectRecursiveObjects( initialObject: APObject, recurseProperty: RecurseProperty, From ee934f528f9846527566ba306353376118853e87 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 17:30:44 +0900 Subject: [PATCH 33/46] Harden lookup private-address handling and image redirects Secure image downloads in the CLI renderer by validating each redirect hop with manual redirect handling, and add regression tests for private-redirect rejection and valid redirect chains. Add a new lookup option, -p/--allow-private-address, wired to config (lookup.allowPrivateAddress), and use it for explicit lookup/traverse document/context loading while keeping recursive fetches strict. Document the new option in docs/cli.md and add a CHANGES.md entry for the default private-address hardening and opt-in behavior. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901529668 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901534765 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901534774 --- CHANGES.md | 6 +++ docs/cli.md | 16 ++++++++ packages/cli/src/config.ts | 1 + packages/cli/src/imagerenderer.test.ts | 55 ++++++++++++++++++++++++++ packages/cli/src/imagerenderer.ts | 22 +++++++++-- packages/cli/src/lookup.test.ts | 22 +++++++++++ packages/cli/src/lookup.ts | 15 +++++++ 7 files changed, 134 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index c9db56107..65bbc4d29 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -34,6 +34,12 @@ To be released. works in recurse mode as best-effort lookup. [[#606], [#608]] + - Hardened `fedify lookup` by disallowing private/localhost document loads + by default. For local-development workflows, `-p`/`--allow-private-address` + (or `lookup.allowPrivateAddress = true` in config) can re-enable private + address access for explicit lookup/traverse requests. + [[#608]] + [#606]: https://github.com/fedify-dev/fedify/issues/606 [#608]: https://github.com/fedify-dev/fedify/pull/608 diff --git a/docs/cli.md b/docs/cli.md index c60ec970e..d28af279d 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -173,6 +173,7 @@ maxRedirection = 5 [lookup] authorizedFetch = false firstKnock = "draft-cavage-http-signatures-12" # or "rfc9421" +allowPrivateAddress = false traverse = false suppressErrors = false defaultFormat = "default" # "default", "raw", "compact", or "expand" @@ -983,6 +984,21 @@ below command: 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. + +~~~~ sh +fedify lookup --allow-private-address http://localhost:8000/users/alice +~~~~ + +> [!NOTE] +> Recursive fetches enabled by +> [`--recurse`](#recurse-recurse-through-object-relationships) continue to +> disallow private addresses. + ### `-s`/`--separator`: Output separator *This option is available since Fedify 1.3.0.* diff --git a/packages/cli/src/config.ts b/packages/cli/src/config.ts index 1eaa55115..450197c59 100644 --- a/packages/cli/src/config.ts +++ b/packages/cli/src/config.ts @@ -36,6 +36,7 @@ const lookupSchema = pipe( firstKnock: optional( picklist(["draft-cavage-http-signatures-12", "rfc9421"]), ), + allowPrivateAddress: optional(boolean()), traverse: optional(boolean()), recurse: optional( picklist([ diff --git a/packages/cli/src/imagerenderer.test.ts b/packages/cli/src/imagerenderer.test.ts index 5bc27497d..11e3f4ec9 100644 --- a/packages/cli/src/imagerenderer.test.ts +++ b/packages/cli/src/imagerenderer.test.ts @@ -39,3 +39,58 @@ test("downloadImage - writes file for public URL", async () => { } } }); + +test("downloadImage - rejects redirect to private URL", async () => { + let calls = 0; + const originalFetch = globalThis.fetch; + globalThis.fetch = ((input: URL | RequestInfo) => { + calls++; + const target = typeof input === "string" ? input : input.toString(); + if (target === "https://example.com/image.png") { + return Promise.resolve( + new Response(null, { + status: 302, + headers: { location: "http://localhost/secret.png" }, + }), + ); + } + return Promise.resolve(new Response(new Uint8Array([1, 2, 3]))); + }) as typeof fetch; + + try { + const result = await downloadImage("https://example.com/image.png"); + assert.equal(result, null); + assert.equal(calls, 1); + } finally { + globalThis.fetch = originalFetch; + } +}); + +test("downloadImage - follows validated redirects", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = ((input: URL | RequestInfo) => { + const target = typeof input === "string" ? input : input.toString(); + if (target === "https://example.com/image.png") { + return Promise.resolve( + new Response(null, { + status: 302, + headers: { location: "https://cdn.example.com/final.png" }, + }), + ); + } + return Promise.resolve(new Response(new Uint8Array([1, 2, 3]))); + }) as typeof fetch; + + let result: string | null = null; + try { + result = await downloadImage("https://example.com/image.png"); + assert.notEqual(result, null); + const fileStat = await stat(result!); + assert.equal(fileStat.isFile(), true); + } finally { + globalThis.fetch = originalFetch; + if (result != null) { + await rm(path.dirname(result), { recursive: true, force: true }); + } + } +}); diff --git a/packages/cli/src/imagerenderer.ts b/packages/cli/src/imagerenderer.ts index a858f2ace..c893916d7 100644 --- a/packages/cli/src/imagerenderer.ts +++ b/packages/cli/src/imagerenderer.ts @@ -104,10 +104,26 @@ export async function renderImageITerm2( export async function downloadImage(url: string): Promise { try { - await validatePublicUrl(url); - const response = await fetch(url); + let targetUrl = url; + let response: Response | null = null; + for (let redirectCount = 0; redirectCount < 10; redirectCount++) { + await validatePublicUrl(targetUrl); + response = await fetch(targetUrl, { redirect: "manual" }); + if ( + response.status === 301 || response.status === 302 || + response.status === 303 || response.status === 307 || + response.status === 308 + ) { + const location = response.headers.get("location"); + if (location == null) return null; + targetUrl = new URL(location, targetUrl).href; + continue; + } + break; + } + if (response == null || !response.ok) return null; const imageData = new Uint8Array(await response.arrayBuffer()); - const extension = new URL(url).pathname.split(".").pop() || "jpg"; + const extension = new URL(targetUrl).pathname.split(".").pop() || "jpg"; const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "fedify")); const tempPath = path.join(tempDir, `image.${extension}`); diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index 9635d7191..0c5b1fb53 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -419,6 +419,28 @@ test("authorizedFetchOption - parses with -a and --tunnel-service", () => { } }); +test("lookupCommand - parses --allow-private-address", () => { + setActiveConfig(configContext.id, {}); + const result = parse(lookupCommand, [ + "lookup", + "--allow-private-address", + "https://example.com/notes/1", + ]); + clearActiveConfig(configContext.id); + assert.ok(result.success); + if (result.success) { + assert.strictEqual(result.value.allowPrivateAddress, true); + } +}); + +test("lookupCommand - reads allowPrivateAddress from config", async () => { + const result = await runWithConfig(lookupCommand, configContext, { + load: () => ({ lookup: { allowPrivateAddress: true } }), + args: ["lookup", "https://example.com/notes/1"], + }); + assert.strictEqual(result.allowPrivateAddress, true); +}); + test("lookupCommand - parses recurse option", () => { setActiveConfig(configContext.id, {}); const result = parse(lookupCommand, [ diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index be6f0f519..34c2bb74f 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -81,6 +81,18 @@ const suppressErrorsOption = bindConfig( }, ); +const allowPrivateAddressOption = bindConfig( + flag("-p", "--allow-private-address", { + description: + message`Allow private IP addresses for explicit lookup/traverse requests.`, + }), + { + context: configContext, + key: (config) => config.lookup?.allowPrivateAddress ?? false, + default: false, + }, +); + export const authorizedFetchOption = withDefault( object("Authorized fetch options", { authorizedFetch: bindConfig( @@ -193,6 +205,7 @@ export const lookupCommand = command( "Network options", userAgentOption, object({ + allowPrivateAddress: allowPrivateAddressOption, timeout: optional( bindConfig( option( @@ -568,6 +581,7 @@ export async function runLookup( let server: TemporaryServer | undefined = undefined; const baseDocumentLoader = await getDocumentLoader({ userAgent: command.userAgent, + allowPrivateAddress: command.allowPrivateAddress, }); const documentLoader = wrapDocumentLoaderWithTimeout( baseDocumentLoader, @@ -575,6 +589,7 @@ export async function runLookup( ); const baseContextLoader = await getContextLoader({ userAgent: command.userAgent, + allowPrivateAddress: command.allowPrivateAddress, }); const contextLoader = wrapDocumentLoaderWithTimeout( baseContextLoader, From 5101c9ccac7a50d54451d15a5a13ed6283efbb29 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 18:18:27 +0900 Subject: [PATCH 34/46] Fix recurse private-address handling in authorized fetch mode Ensure -p/--allow-private-address is applied consistently in lookup mode, including authorized-fetch paths. In recurse mode, explicit root lookups now use the regular lookup/context loaders (respecting command/config allowPrivateAddress), while recursive follow-up fetches continue to use strict loaders that disallow private addresses. Also consume redirect response bodies in image download redirect handling to avoid keeping redirect bodies alive across manual redirect hops. Add regression tests for redirect-body cancellation in imagerenderer tests. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901549540 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901549543 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901549548 Co-Authored-By: Codex --- packages/cli/src/imagerenderer.test.ts | 55 ++++++++++++++++++++++++++ packages/cli/src/imagerenderer.ts | 6 ++- packages/cli/src/lookup.ts | 44 +++++++++++++++++---- 3 files changed, 96 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/imagerenderer.test.ts b/packages/cli/src/imagerenderer.test.ts index 11e3f4ec9..29c011501 100644 --- a/packages/cli/src/imagerenderer.test.ts +++ b/packages/cli/src/imagerenderer.test.ts @@ -94,3 +94,58 @@ test("downloadImage - follows validated redirects", async () => { } } }); + +test("downloadImage - cancels redirect response body before following", async () => { + let cancelled = 0; + const originalFetch = globalThis.fetch; + globalThis.fetch = ((input: URL | RequestInfo) => { + const target = typeof input === "string" ? input : input.toString(); + if (target === "https://example.com/image.png") { + const body = new ReadableStream({ + cancel() { + cancelled++; + }, + }); + return Promise.resolve( + new Response(body, { + status: 302, + headers: { location: "https://cdn.example.com/final.png" }, + }), + ); + } + return Promise.resolve(new Response(new Uint8Array([1, 2, 3]))); + }) as typeof fetch; + + let result: string | null = null; + try { + result = await downloadImage("https://example.com/image.png"); + assert.notEqual(result, null); + assert.equal(cancelled, 1); + } finally { + globalThis.fetch = originalFetch; + if (result != null) { + await rm(path.dirname(result), { recursive: true, force: true }); + } + } +}); + +test("downloadImage - cancels redirect body when location is missing", async () => { + let cancelled = 0; + const originalFetch = globalThis.fetch; + globalThis.fetch = ((_input: URL | RequestInfo) => { + const body = new ReadableStream({ + cancel() { + cancelled++; + }, + }); + return Promise.resolve(new Response(body, { status: 302 })); + }) as typeof fetch; + + try { + const result = await downloadImage("https://example.com/image.png"); + assert.equal(result, null); + assert.equal(cancelled, 1); + } finally { + globalThis.fetch = originalFetch; + } +}); diff --git a/packages/cli/src/imagerenderer.ts b/packages/cli/src/imagerenderer.ts index c893916d7..81c2057a8 100644 --- a/packages/cli/src/imagerenderer.ts +++ b/packages/cli/src/imagerenderer.ts @@ -115,7 +115,11 @@ export async function downloadImage(url: string): Promise { response.status === 308 ) { const location = response.headers.get("location"); - if (location == null) return null; + if (location == null) { + await response.body?.cancel(); + return null; + } + await response.body?.cancel(); targetUrl = new URL(location, targetUrl).href; continue; } diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 34c2bb74f..50a16de1a 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -597,6 +597,9 @@ export async function runLookup( ); let authLoader: DocumentLoader | undefined = undefined; + let authIdentity: + | { keyId: URL; privateKey: CryptoKey } + | undefined = undefined; let outputStream: WriteStream | undefined; let outputStreamError: Error | undefined; const getOutputStream = (): WriteStream | undefined => { @@ -671,12 +674,15 @@ export async function runLookup( { contextLoader }, ); }, { service: command.tunnelService }); + authIdentity = { + keyId: new URL("#main-key", server.url), + privateKey: key.privateKey, + }; const baseAuthLoader = getAuthenticatedDocumentLoader( + authIdentity, { - keyId: new URL("#main-key", server.url), - privateKey: key.privateKey, - }, - { + allowPrivateAddress: command.allowPrivateAddress, + userAgent: command.userAgent, specDeterminer: { determineSpec() { return command.firstKnock; @@ -719,7 +725,29 @@ export async function runLookup( recursiveBaseContextLoader, command.timeout, ); - const recursiveLookupDocumentLoader: DocumentLoader = authLoader ?? + const recursiveAuthLoader = command.authorizedFetch && + authIdentity != null + ? wrapDocumentLoaderWithTimeout( + getAuthenticatedDocumentLoader( + authIdentity, + { + allowPrivateAddress: false, + userAgent: command.userAgent, + specDeterminer: { + determineSpec() { + return command.firstKnock; + }, + rememberSpec() { + }, + }, + }, + ), + command.timeout, + ) + : undefined; + const initialLookupDocumentLoader: DocumentLoader = authLoader ?? + documentLoader; + const recursiveLookupDocumentLoader: DocumentLoader = recursiveAuthLoader ?? recursiveDocumentLoader; let totalObjects = 0; const recurseDepth = command.recurseDepth!; @@ -735,8 +763,8 @@ export async function runLookup( let current: APObject | null = null; try { current = await lookupObject(url, { - documentLoader: recursiveLookupDocumentLoader, - contextLoader: recursiveContextLoader, + documentLoader: initialLookupDocumentLoader, + contextLoader, userAgent: command.userAgent, }); } catch (error) { @@ -772,7 +800,7 @@ export async function runLookup( current, command.output, command.format, - recursiveContextLoader, + contextLoader, getOutputStream(), ); } catch (error) { From 8f1d18b7e0aa6b0ab1391cc3ba9fefd8e786d6af Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 18:32:36 +0900 Subject: [PATCH 35/46] Improve suppressed recurse diagnostics and output/body error handling Add debug logging for suppressed recursive lookup failures so best-effort runs leave traceable diagnostics for both thrown lookup errors and not-found results. In traverse mode, isolate output writing failures from traversal/fetch failures by handling separator/object write errors in an inner try/catch and failing with an output-specific message. For image downloads, cancel non-OK terminal response bodies before returning null to avoid retaining response body resources in manual-fetch paths. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901592398 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901592399 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901595417 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901595420 Co-Authored-By: Codex --- packages/cli/src/imagerenderer.test.ts | 21 ++++++++++++ packages/cli/src/imagerenderer.ts | 6 +++- packages/cli/src/lookup.ts | 46 ++++++++++++++++++++------ 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/imagerenderer.test.ts b/packages/cli/src/imagerenderer.test.ts index 29c011501..d9b64d736 100644 --- a/packages/cli/src/imagerenderer.test.ts +++ b/packages/cli/src/imagerenderer.test.ts @@ -149,3 +149,24 @@ test("downloadImage - cancels redirect body when location is missing", async () globalThis.fetch = originalFetch; } }); + +test("downloadImage - cancels body for non-ok terminal response", async () => { + let cancelled = 0; + const originalFetch = globalThis.fetch; + globalThis.fetch = ((_input: URL | RequestInfo) => { + const body = new ReadableStream({ + cancel() { + cancelled++; + }, + }); + return Promise.resolve(new Response(body, { status: 500 })); + }) as typeof fetch; + + try { + const result = await downloadImage("https://example.com/image.png"); + assert.equal(result, null); + assert.equal(cancelled, 1); + } finally { + globalThis.fetch = originalFetch; + } +}); diff --git a/packages/cli/src/imagerenderer.ts b/packages/cli/src/imagerenderer.ts index 81c2057a8..2162d1a0c 100644 --- a/packages/cli/src/imagerenderer.ts +++ b/packages/cli/src/imagerenderer.ts @@ -125,7 +125,11 @@ export async function downloadImage(url: string): Promise { } break; } - if (response == null || !response.ok) return null; + if (response == null) return null; + if (!response.ok) { + await response.body?.cancel(); + return null; + } const imageData = new Uint8Array(await response.arrayBuffer()); const extension = new URL(targetUrl).pathname.split(".").pop() || "jpg"; const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "fedify")); diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 50a16de1a..b5fc424ad 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -534,11 +534,25 @@ export async function collectRecursiveObjects( try { next = await lookup(target); } catch (error) { - if (options.suppressErrors) break; + if (options.suppressErrors) { + logger.debug( + "Failed to recursively fetch object {target}, " + + "but suppressing error: {error}", + { target, error }, + ); + break; + } throw error; } if (next == null) { - if (options.suppressErrors) break; + if (options.suppressErrors) { + logger.debug( + "Failed to recursively fetch object {target} " + + "(not found), but suppressing error.", + { target }, + ); + break; + } throw new RecursiveLookupError(target); } results.push(next); @@ -950,16 +964,26 @@ export async function runLookup( suppressError: command.suppressErrors, }) ) { - if (totalItems > 0 || collectionItems > 0) { - await writeSeparator(command.separator, getOutputStream()); + try { + if (totalItems > 0 || collectionItems > 0) { + await writeSeparator(command.separator, getOutputStream()); + } + await writeObjectToStream( + item, + command.output, + command.format, + contextLoader, + getOutputStream(), + ); + } catch (error) { + logger.error("Failed to write output for {url}: {error}", { + url, + error, + }); + spinner.fail(`Failed to write output for: ${colors.red(url)}.`); + await finalizeAndExit(1); + return; } - await writeObjectToStream( - item, - command.output, - command.format, - contextLoader, - getOutputStream(), - ); collectionItems++; totalItems++; } From 3c7eb9b45567922c9f1775661da9bcbfdfdf6fc8 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 18:46:55 +0900 Subject: [PATCH 36/46] Scope private-address default, cleanup exit status, and deterministic tests Restore CLI document-loader default allowPrivateAddress behavior to keep non-lookup commands unchanged, while lookup remains explicitly hardened via lookup-specific option/config wiring. Make finalizeAndExit return a non-zero status when cleanup fails after a nominal success path, so shutdown I/O errors are not reported as success. Update imagerenderer tests to use literal public IP URLs to avoid DNS lookup dependencies and keep unit tests deterministic. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901603971 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901610738 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901610742 Co-Authored-By: Codex --- packages/cli/src/docloader.ts | 2 +- packages/cli/src/imagerenderer.test.ts | 25 ++++++++++++++----------- packages/cli/src/lookup.ts | 5 ++++- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/docloader.ts b/packages/cli/src/docloader.ts index 223f9b9d1..ab18624fc 100644 --- a/packages/cli/src/docloader.ts +++ b/packages/cli/src/docloader.ts @@ -30,7 +30,7 @@ export function getDocumentLoaderCachePrefix( } export async function getDocumentLoader( - { userAgent, allowPrivateAddress = false }: DocumentLoaderOptions = {}, + { userAgent, allowPrivateAddress = true }: DocumentLoaderOptions = {}, ): Promise { const cacheKey = `${userAgent ?? ""}:${allowPrivateAddress}`; if (documentLoaders[cacheKey]) return documentLoaders[cacheKey]; diff --git a/packages/cli/src/imagerenderer.test.ts b/packages/cli/src/imagerenderer.test.ts index d9b64d736..1b915c77c 100644 --- a/packages/cli/src/imagerenderer.test.ts +++ b/packages/cli/src/imagerenderer.test.ts @@ -4,6 +4,9 @@ import path from "node:path"; import test from "node:test"; import { downloadImage } from "./imagerenderer.ts"; +const TEST_PUBLIC_IMAGE_URL = "https://198.51.100.10/image.png"; +const TEST_PUBLIC_REDIRECT_URL = "https://198.51.100.11/final.png"; + test("downloadImage - skips private URL without fetching", async () => { let called = false; const originalFetch = globalThis.fetch; @@ -28,7 +31,7 @@ test("downloadImage - writes file for public URL", async () => { let result: string | null = null; try { - result = await downloadImage("https://example.com/image.png"); + result = await downloadImage(TEST_PUBLIC_IMAGE_URL); assert.notEqual(result, null); const fileStat = await stat(result!); assert.equal(fileStat.isFile(), true); @@ -46,7 +49,7 @@ test("downloadImage - rejects redirect to private URL", async () => { globalThis.fetch = ((input: URL | RequestInfo) => { calls++; const target = typeof input === "string" ? input : input.toString(); - if (target === "https://example.com/image.png") { + if (target === TEST_PUBLIC_IMAGE_URL) { return Promise.resolve( new Response(null, { status: 302, @@ -58,7 +61,7 @@ test("downloadImage - rejects redirect to private URL", async () => { }) as typeof fetch; try { - const result = await downloadImage("https://example.com/image.png"); + const result = await downloadImage(TEST_PUBLIC_IMAGE_URL); assert.equal(result, null); assert.equal(calls, 1); } finally { @@ -70,11 +73,11 @@ test("downloadImage - follows validated redirects", async () => { const originalFetch = globalThis.fetch; globalThis.fetch = ((input: URL | RequestInfo) => { const target = typeof input === "string" ? input : input.toString(); - if (target === "https://example.com/image.png") { + if (target === TEST_PUBLIC_IMAGE_URL) { return Promise.resolve( new Response(null, { status: 302, - headers: { location: "https://cdn.example.com/final.png" }, + headers: { location: TEST_PUBLIC_REDIRECT_URL }, }), ); } @@ -83,7 +86,7 @@ test("downloadImage - follows validated redirects", async () => { let result: string | null = null; try { - result = await downloadImage("https://example.com/image.png"); + result = await downloadImage(TEST_PUBLIC_IMAGE_URL); assert.notEqual(result, null); const fileStat = await stat(result!); assert.equal(fileStat.isFile(), true); @@ -100,7 +103,7 @@ test("downloadImage - cancels redirect response body before following", async () const originalFetch = globalThis.fetch; globalThis.fetch = ((input: URL | RequestInfo) => { const target = typeof input === "string" ? input : input.toString(); - if (target === "https://example.com/image.png") { + if (target === TEST_PUBLIC_IMAGE_URL) { const body = new ReadableStream({ cancel() { cancelled++; @@ -109,7 +112,7 @@ test("downloadImage - cancels redirect response body before following", async () return Promise.resolve( new Response(body, { status: 302, - headers: { location: "https://cdn.example.com/final.png" }, + headers: { location: TEST_PUBLIC_REDIRECT_URL }, }), ); } @@ -118,7 +121,7 @@ test("downloadImage - cancels redirect response body before following", async () let result: string | null = null; try { - result = await downloadImage("https://example.com/image.png"); + result = await downloadImage(TEST_PUBLIC_IMAGE_URL); assert.notEqual(result, null); assert.equal(cancelled, 1); } finally { @@ -142,7 +145,7 @@ test("downloadImage - cancels redirect body when location is missing", async () }) as typeof fetch; try { - const result = await downloadImage("https://example.com/image.png"); + const result = await downloadImage(TEST_PUBLIC_IMAGE_URL); assert.equal(result, null); assert.equal(cancelled, 1); } finally { @@ -163,7 +166,7 @@ test("downloadImage - cancels body for non-ok terminal response", async () => { }) as typeof fetch; try { - const result = await downloadImage("https://example.com/image.png"); + const result = await downloadImage(TEST_PUBLIC_IMAGE_URL); assert.equal(result, null); assert.equal(cancelled, 1); } finally { diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index b5fc424ad..4f3838a32 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -630,9 +630,11 @@ export async function runLookup( return outputStream; }; const finalizeAndExit = async (code: number) => { + let cleanupFailed = false; try { await closeWriteStream(outputStream); } catch (error) { + cleanupFailed = true; logger.error("Failed to close output stream during shutdown: {error}", { error, }); @@ -640,6 +642,7 @@ export async function runLookup( try { await server?.close(); } catch (error) { + cleanupFailed = true; logger.error( "Failed to close temporary server during shutdown: {error}", { @@ -647,7 +650,7 @@ export async function runLookup( }, ); } - process.exit(code); + process.exit(cleanupFailed && code === 0 ? 1 : code); }; if (command.authorizedFetch) { From 95423d90c63029c0ed6233a3222f2d1135575e62 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 19:00:42 +0900 Subject: [PATCH 37/46] Harden image temp path extension handling Reject unsafe extension fragments containing path separators or parent-directory traversal markers before composing the temporary output path. This prevents path traversal through crafted image URLs in downloadImage. Also adds a regression test covering a traversal-style URL suffix. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901618838 --- packages/cli/src/imagerenderer.test.ts | 16 ++++++++++++++++ packages/cli/src/imagerenderer.ts | 6 ++++++ 2 files changed, 22 insertions(+) diff --git a/packages/cli/src/imagerenderer.test.ts b/packages/cli/src/imagerenderer.test.ts index 1b915c77c..a07b182a3 100644 --- a/packages/cli/src/imagerenderer.test.ts +++ b/packages/cli/src/imagerenderer.test.ts @@ -173,3 +173,19 @@ test("downloadImage - cancels body for non-ok terminal response", async () => { globalThis.fetch = originalFetch; } }); + +test("downloadImage - rejects unsafe extension containing path traversal", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = + ((_input: URL | RequestInfo) => + Promise.resolve(new Response(new Uint8Array([1, 2, 3])))) as typeof fetch; + + try { + const result = await downloadImage( + "https://198.51.100.10/image.png/../../../../etc/passwd", + ); + assert.equal(result, null); + } finally { + globalThis.fetch = originalFetch; + } +}); diff --git a/packages/cli/src/imagerenderer.ts b/packages/cli/src/imagerenderer.ts index 2162d1a0c..811143bdf 100644 --- a/packages/cli/src/imagerenderer.ts +++ b/packages/cli/src/imagerenderer.ts @@ -132,6 +132,12 @@ export async function downloadImage(url: string): Promise { } const imageData = new Uint8Array(await response.arrayBuffer()); const extension = new URL(targetUrl).pathname.split(".").pop() || "jpg"; + if ( + extension.includes("/") || extension.includes("\\") || + extension.includes("..") + ) { + return null; + } const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "fedify")); const tempPath = path.join(tempDir, `image.${extension}`); From 764e68d873b945ee056e0546f51c96666a763ea9 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 19:04:41 +0900 Subject: [PATCH 38/46] Make recursive target mapping explicit for quote properties Handle quote recursion properties explicitly in getRecursiveTargetId instead of relying on an implicit non-reply fallback. This keeps the mapping self-documenting and returns null for unexpected values. Also adds a regression test that verifies unknown recurse properties fall back to null. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901618841 --- packages/cli/src/lookup.test.ts | 17 +++++++++++++++++ packages/cli/src/lookup.ts | 11 +++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index 0c5b1fb53..d02283b79 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -601,6 +601,23 @@ for ( }); } +test("getRecursiveTargetId - returns null for unknown recurse property", () => { + const quoteUrl = new URL("https://example.com/notes/quoted"); + const note = new Note({ + id: new URL("https://example.com/notes/1"), + quoteUrl, + }); + assert.equal( + getRecursiveTargetId( + note, + "https://example.com/custom#prop" as Parameters< + typeof getRecursiveTargetId + >[1], + ), + null, + ); +}); + test("collectRecursiveObjects - follows chain up to depth limit", async () => { const note1 = new Note({ id: new URL("https://example.com/notes/1"), diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 4f3838a32..ba8a1b015 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -503,8 +503,15 @@ export function getRecursiveTargetId( ) { return object.replyTargetId; } - const quoteUrl = (object as { quoteUrl?: unknown }).quoteUrl; - return quoteUrl instanceof URL ? quoteUrl : null; + if ( + recurseProperty === "quoteUrl" || recurseProperty === QUOTE_URL_IRI || + recurseProperty === MISSKEY_QUOTE_IRI || + recurseProperty === FEDIBIRD_QUOTE_IRI + ) { + const quoteUrl = (object as { quoteUrl?: unknown }).quoteUrl; + return quoteUrl instanceof URL ? quoteUrl : null; + } + return null; } /** From aab372f440ad1aa890fb2cc6d570db024f470030 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 19:21:37 +0900 Subject: [PATCH 39/46] Clarify private-address policy for recursive lookup docs Document that --allow-private-address only applies to explicit lookup/traverse targets, while recursive fetch steps always disallow private and localhost addresses. This updates both the lookup CLI docs and the @fedify/cli changelog entry for the recurse/private-address hardening. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901636258 --- CHANGES.md | 3 ++- docs/cli.md | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 65bbc4d29..6f8e0d573 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -37,7 +37,8 @@ To be released. - Hardened `fedify lookup` by disallowing private/localhost document loads by default. For local-development workflows, `-p`/`--allow-private-address` (or `lookup.allowPrivateAddress = true` in config) can re-enable private - address access for explicit lookup/traverse requests. + address access for explicit lookup/traverse requests. This option does + not apply to recursive fetches, which always disallow private addresses. [[#608]] [#606]: https://github.com/fedify-dev/fedify/issues/606 diff --git a/docs/cli.md b/docs/cli.md index d28af279d..68c775134 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -530,6 +530,10 @@ and `quoteUri` are not accepted as short forms. > [!NOTE] > `--recurse` and [`-t`/`--traverse`](#t-traverse-traverse-the-collection) > 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. ### `--recurse-depth`: Set recursion depth limit From f00210c098f671a12cd0cd57e3ead00098b29c05 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 19:39:33 +0900 Subject: [PATCH 40/46] Improve lookup hints for private-address validation failures Detect private/localhost URL validation failures and show actionable hints instead of always suggesting authorized fetch. - In lookup/traverse fetch failures, suggest -p/--allow-private-address when the failure is a private-address UrlError. - In recurse step failures, explain that recursive fetches always block private/localhost targets and suggest best-effort suppression or explicit non-recursive fetches. - Keep existing -a/--authorized-fetch guidance for non-UrlError cases. Also adds unit tests for the hint classification helper. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901658495 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901658508 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901658511 --- packages/cli/src/lookup.test.ts | 25 ++++++++++++ packages/cli/src/lookup.ts | 69 +++++++++++++++++++++++++-------- 2 files changed, 77 insertions(+), 17 deletions(-) diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index d02283b79..e96404488 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -2,6 +2,7 @@ import { Activity, Note } from "@fedify/vocab"; import { clearActiveConfig, setActiveConfig } from "@optique/config"; import { runWithConfig } from "@optique/config/run"; import { parse } from "@optique/core/parser"; +import { UrlError } from "@fedify/vocab-runtime"; import assert from "node:assert/strict"; import { Buffer } from "node:buffer"; import { createWriteStream } from "node:fs"; @@ -16,6 +17,7 @@ import { clearTimeoutSignal, collectRecursiveObjects, createTimeoutSignal, + getLookupFailureHint, getRecursiveTargetId, lookupCommand, RecursiveLookupError, @@ -618,6 +620,29 @@ test("getRecursiveTargetId - returns null for unknown recurse property", () => { ); }); +test("getLookupFailureHint - suggests private-address for UrlError", () => { + assert.equal( + getLookupFailureHint(new UrlError("Localhost is not allowed")), + "private-address", + ); +}); + +test("getLookupFailureHint - suggests recursive-private-address in recurse mode", () => { + assert.equal( + getLookupFailureHint(new UrlError("Invalid or private address"), { + recursive: true, + }), + "recursive-private-address", + ); +}); + +test("getLookupFailureHint - suggests authorized-fetch for non-URL errors", () => { + assert.equal( + getLookupFailureHint(new Error("401 Unauthorized")), + "authorized-fetch", + ); +}); + test("collectRecursiveObjects - follows chain up to depth limit", async () => { const note1 = new Note({ id: new URL("https://example.com/notes/1"), diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index ba8a1b015..04c0389f8 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -12,7 +12,7 @@ import { Object as APObject, traverseCollection, } from "@fedify/vocab"; -import type { DocumentLoader } from "@fedify/vocab-runtime"; +import { type DocumentLoader, UrlError } from "@fedify/vocab-runtime"; import type { ResourceDescriptor } from "@fedify/webfinger"; import { getLogger } from "@logtape/logtape"; import { bindConfig } from "@optique/config"; @@ -491,6 +491,53 @@ function handleTimeoutError( ); } +function isPrivateAddressError(error: unknown): boolean { + if (error instanceof UrlError) return true; + const errorMessage = error instanceof Error ? error.message : String(error); + const lowerMessage = errorMessage.toLowerCase(); + return ( + lowerMessage.includes("private address") || + lowerMessage.includes("private ip") || + lowerMessage.includes("localhost") || + lowerMessage.includes("loopback") + ); +} + +export function getLookupFailureHint( + error: unknown, + options: { recursive?: boolean } = {}, +): "private-address" | "recursive-private-address" | "authorized-fetch" { + if (isPrivateAddressError(error)) { + return options.recursive ? "recursive-private-address" : "private-address"; + } + return "authorized-fetch"; +} + +function printLookupFailureHint( + authLoader: DocumentLoader | undefined, + error: unknown, + options: { recursive?: boolean } = {}, +): void { + if (authLoader != null) return; + switch (getLookupFailureHint(error, options)) { + case "private-address": + printError( + message`The URL appears to be private or localhost. Try with -p/--allow-private-address.`, + ); + return; + case "recursive-private-address": + printError( + message`Recursive fetches do not allow private/localhost URLs. Use -S/--suppress-errors to skip blocked steps, or fetch those targets explicitly without --recurse.`, + ); + return; + case "authorized-fetch": + printError( + message`It may be a private object. Try with -a/--authorized-fetch.`, + ); + return; + } +} + /** * Gets the next recursion target URL from an ActivityPub object. */ @@ -796,11 +843,7 @@ export async function runLookup( handleTimeoutError(spinner, command.timeout, url); } else { spinner.fail(`Failed to fetch object: ${colors.red(url)}.`); - if (authLoader == null) { - printError( - message`It may be a private object. Try with -a/--authorized-fetch.`, - ); - } + printLookupFailureHint(authLoader, error); } await finalizeAndExit(1); return; @@ -874,9 +917,7 @@ export async function runLookup( } else { spinner.fail("Failed to recursively fetch object."); if (authLoader == null) { - printError( - message`It may be a private object. Try with -a/--authorized-fetch.`, - ); + printLookupFailureHint(authLoader, error, { recursive: true }); } else { printError( message`Use the -S/--suppress-errors option to suppress partial errors.`, @@ -936,11 +977,7 @@ export async function runLookup( handleTimeoutError(spinner, command.timeout, url); } else { spinner.fail(`Failed to fetch object: ${colors.red(url)}.`); - if (authLoader == null) { - printError( - message`It may be a private object. Try with -a/--authorized-fetch.`, - ); - } + printLookupFailureHint(authLoader, error); } await finalizeAndExit(1); return; @@ -1009,9 +1046,7 @@ export async function runLookup( `Failed to complete the traversal for: ${colors.red(url)}.`, ); if (authLoader == null) { - printError( - message`It may be a private object. Try with -a/--authorized-fetch.`, - ); + printLookupFailureHint(authLoader, error); } else { printError( message`Use the -S/--suppress-errors option to suppress partial errors.`, From b89814802fdaf46bdfe4f99930ac9a0f5ab15227 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 19:41:16 +0900 Subject: [PATCH 41/46] Use switch-based recursion property mapping Switch getRecursiveTargetId() to an explicit switch statement for recurseProperty handling. This keeps the mapping table-style and easier to extend as additional recurse properties are introduced. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901654575 --- packages/cli/src/lookup.ts | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 04c0389f8..f16da9a4c 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -545,20 +545,20 @@ export function getRecursiveTargetId( object: APObject, recurseProperty: RecurseProperty, ): URL | null { - if ( - recurseProperty === "replyTarget" || recurseProperty === IN_REPLY_TO_IRI - ) { - return object.replyTargetId; - } - if ( - recurseProperty === "quoteUrl" || recurseProperty === QUOTE_URL_IRI || - recurseProperty === MISSKEY_QUOTE_IRI || - recurseProperty === FEDIBIRD_QUOTE_IRI - ) { - const quoteUrl = (object as { quoteUrl?: unknown }).quoteUrl; - return quoteUrl instanceof URL ? quoteUrl : null; + switch (recurseProperty) { + case "replyTarget": + case IN_REPLY_TO_IRI: + return object.replyTargetId; + case "quoteUrl": + case QUOTE_URL_IRI: + case MISSKEY_QUOTE_IRI: + case FEDIBIRD_QUOTE_IRI: { + const quoteUrl = (object as { quoteUrl?: unknown }).quoteUrl; + return quoteUrl instanceof URL ? quoteUrl : null; + } + default: + return null; } - return null; } /** From da6a6fc493b9d3f2c9b7d6a28318150dd6096f03 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 19:56:44 +0900 Subject: [PATCH 42/46] Fix lookup failure hint classification and suppression logic UrlError can represent URL validation failures beyond private-address cases, so treating every UrlError as private produced incorrect hints. Also, printLookupFailureHint() suppressed all hints whenever authorized fetch was enabled, which hid private/localhost guidance. This change narrows private-address detection for UrlError and only suppresses the authorized-fetch hint when auth loading is already enabled. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901674513 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901674526 Co-Authored-By: Codex --- packages/cli/src/lookup.test.ts | 28 ++++++++++++++++++++++++++++ packages/cli/src/lookup.ts | 19 ++++++++++++++++--- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index e96404488..bffa7576b 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -21,6 +21,7 @@ import { getRecursiveTargetId, lookupCommand, RecursiveLookupError, + shouldPrintLookupFailureHint, TimeoutError, writeObjectToStream, writeSeparator, @@ -643,6 +644,33 @@ test("getLookupFailureHint - suggests authorized-fetch for non-URL errors", () = ); }); +test("getLookupFailureHint - does not treat all UrlError values as private", () => { + assert.equal( + getLookupFailureHint(new UrlError("Unsupported protocol: ftp:")), + "authorized-fetch", + ); +}); + +test("shouldPrintLookupFailureHint - suppresses only authorized-fetch hint", () => { + const loader = + ((_url: string) => + Promise.reject(new Error("not used"))) as unknown as Parameters< + typeof shouldPrintLookupFailureHint + >[0]; + assert.equal( + shouldPrintLookupFailureHint(loader, "authorized-fetch"), + false, + ); + assert.equal( + shouldPrintLookupFailureHint(loader, "private-address"), + true, + ); + assert.equal( + shouldPrintLookupFailureHint(loader, "recursive-private-address"), + true, + ); +}); + test("collectRecursiveObjects - follows chain up to depth limit", async () => { const note1 = new Note({ id: new URL("https://example.com/notes/1"), diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index f16da9a4c..98cbda8ef 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -492,9 +492,14 @@ function handleTimeoutError( } function isPrivateAddressError(error: unknown): boolean { - if (error instanceof UrlError) return true; const errorMessage = error instanceof Error ? error.message : String(error); const lowerMessage = errorMessage.toLowerCase(); + if (error instanceof UrlError) { + return ( + lowerMessage.includes("invalid or private address") || + lowerMessage.includes("localhost is not allowed") + ); + } return ( lowerMessage.includes("private address") || lowerMessage.includes("private ip") || @@ -513,13 +518,21 @@ export function getLookupFailureHint( return "authorized-fetch"; } +export function shouldPrintLookupFailureHint( + authLoader: DocumentLoader | undefined, + hint: ReturnType, +): boolean { + return hint !== "authorized-fetch" || authLoader == null; +} + function printLookupFailureHint( authLoader: DocumentLoader | undefined, error: unknown, options: { recursive?: boolean } = {}, ): void { - if (authLoader != null) return; - switch (getLookupFailureHint(error, options)) { + const hint = getLookupFailureHint(error, options); + if (!shouldPrintLookupFailureHint(authLoader, hint)) return; + switch (hint) { case "private-address": printError( message`The URL appears to be private or localhost. Try with -p/--allow-private-address.`, From 930340854c34b5da762befcbb438e53fbb6f8320 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 20:12:22 +0900 Subject: [PATCH 43/46] Harden CLI document loader defaults against private-address access Change getDocumentLoader() to default allowPrivateAddress to false. This keeps commands that do not pass an explicit policy on the safer side by default and aligns the shared loader behavior with the hardening intent in this PR. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901683070 Co-Authored-By: Codex --- packages/cli/src/docloader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/docloader.ts b/packages/cli/src/docloader.ts index ab18624fc..223f9b9d1 100644 --- a/packages/cli/src/docloader.ts +++ b/packages/cli/src/docloader.ts @@ -30,7 +30,7 @@ export function getDocumentLoaderCachePrefix( } export async function getDocumentLoader( - { userAgent, allowPrivateAddress = true }: DocumentLoaderOptions = {}, + { userAgent, allowPrivateAddress = false }: DocumentLoaderOptions = {}, ): Promise { const cacheKey = `${userAgent ?? ""}:${allowPrivateAddress}`; if (documentLoaders[cacheKey]) return documentLoaders[cacheKey]; From 7a05536d91b01785307c79a22488f86a7f0839e2 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 20:13:58 +0900 Subject: [PATCH 44/46] Handle extensionless image URLs in downloadImage safely Use the last path segment to derive an extension and fall back to jpg for extensionless single-segment paths like /image. Keep rejecting unsafe or ambiguous paths by bailing out when no valid extension can be derived. This keeps the path-traversal safeguard intact while allowing the extensionless URL shape called out in review. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901690249 Co-Authored-By: Codex --- packages/cli/src/imagerenderer.test.ts | 19 +++++++++++++++++++ packages/cli/src/imagerenderer.ts | 12 +++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/imagerenderer.test.ts b/packages/cli/src/imagerenderer.test.ts index a07b182a3..0e64636ec 100644 --- a/packages/cli/src/imagerenderer.test.ts +++ b/packages/cli/src/imagerenderer.test.ts @@ -189,3 +189,22 @@ test("downloadImage - rejects unsafe extension containing path traversal", async globalThis.fetch = originalFetch; } }); + +test("downloadImage - falls back to jpg when URL has no extension", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = + ((_input: URL | RequestInfo) => + Promise.resolve(new Response(new Uint8Array([1, 2, 3])))) as typeof fetch; + + let result: string | null = null; + try { + result = await downloadImage("https://198.51.100.10/image"); + assert.notEqual(result, null); + assert.equal(path.extname(result!), ".jpg"); + } finally { + globalThis.fetch = originalFetch; + if (result != null) { + await rm(path.dirname(result), { recursive: true, force: true }); + } + } +}); diff --git a/packages/cli/src/imagerenderer.ts b/packages/cli/src/imagerenderer.ts index 811143bdf..96fffd306 100644 --- a/packages/cli/src/imagerenderer.ts +++ b/packages/cli/src/imagerenderer.ts @@ -131,7 +131,17 @@ export async function downloadImage(url: string): Promise { return null; } const imageData = new Uint8Array(await response.arrayBuffer()); - const extension = new URL(targetUrl).pathname.split(".").pop() || "jpg"; + const pathname = new URL(targetUrl).pathname; + const pathSegments = pathname.split("/").filter((segment) => + segment !== "" + ); + const filename = pathSegments[pathSegments.length - 1] ?? ""; + const extension = filename.includes(".") + ? path.extname(filename).slice(1) + : pathSegments.length === 1 + ? "jpg" + : ""; + if (extension.length < 1) return null; if ( extension.includes("/") || extension.includes("\\") || extension.includes("..") From e859672a372dff4ee26cce5d7db0cc70f40a0fdd Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 20:30:13 +0900 Subject: [PATCH 45/46] Keep private-address hints visible with authorized fetch enabled In recurse and traverse error handling, authorized fetch previously suppressed all lookup hints and always fell back to the generic suppress-errors guidance. That hid actionable private/localhost explanations even though authorized fetch cannot bypass private-address blocking. Use hint classification to show suppress-errors guidance only for authorized-fetch failures with auth enabled, while still emitting private-address and recursive-private-address hints in auth mode. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901706940 https://github.com/fedify-dev/fedify/pull/608#discussion_r2901706947 Co-Authored-By: Codex --- packages/cli/src/lookup.test.ts | 28 ++++++++++++++++++++++++++++ packages/cli/src/lookup.ts | 21 +++++++++++++++------ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/lookup.test.ts b/packages/cli/src/lookup.test.ts index bffa7576b..06ecca7c1 100644 --- a/packages/cli/src/lookup.test.ts +++ b/packages/cli/src/lookup.test.ts @@ -22,6 +22,7 @@ import { lookupCommand, RecursiveLookupError, shouldPrintLookupFailureHint, + shouldSuggestSuppressErrorsForLookupFailure, TimeoutError, writeObjectToStream, writeSeparator, @@ -671,6 +672,33 @@ test("shouldPrintLookupFailureHint - suppresses only authorized-fetch hint", () ); }); +test("shouldSuggestSuppressErrorsForLookupFailure - only for authorized-fetch with auth", () => { + const loader = + ((_url: string) => + Promise.reject(new Error("not used"))) as unknown as Parameters< + typeof shouldSuggestSuppressErrorsForLookupFailure + >[0]; + assert.equal( + shouldSuggestSuppressErrorsForLookupFailure(loader, "authorized-fetch"), + true, + ); + assert.equal( + shouldSuggestSuppressErrorsForLookupFailure(loader, "private-address"), + false, + ); + assert.equal( + shouldSuggestSuppressErrorsForLookupFailure( + loader, + "recursive-private-address", + ), + false, + ); + assert.equal( + shouldSuggestSuppressErrorsForLookupFailure(undefined, "authorized-fetch"), + false, + ); +}); + test("collectRecursiveObjects - follows chain up to depth limit", async () => { const note1 = new Note({ id: new URL("https://example.com/notes/1"), diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 98cbda8ef..78fadb878 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -525,6 +525,13 @@ export function shouldPrintLookupFailureHint( return hint !== "authorized-fetch" || authLoader == null; } +export function shouldSuggestSuppressErrorsForLookupFailure( + authLoader: DocumentLoader | undefined, + hint: ReturnType, +): boolean { + return authLoader != null && hint === "authorized-fetch"; +} + function printLookupFailureHint( authLoader: DocumentLoader | undefined, error: unknown, @@ -929,12 +936,13 @@ export async function runLookup( } } else { spinner.fail("Failed to recursively fetch object."); - if (authLoader == null) { - printLookupFailureHint(authLoader, error, { recursive: true }); - } else { + const hint = getLookupFailureHint(error, { recursive: true }); + if (shouldSuggestSuppressErrorsForLookupFailure(authLoader, hint)) { printError( message`Use the -S/--suppress-errors option to suppress partial errors.`, ); + } else { + printLookupFailureHint(authLoader, error, { recursive: true }); } } await finalizeAndExit(1); @@ -1058,12 +1066,13 @@ export async function runLookup( spinner.fail( `Failed to complete the traversal for: ${colors.red(url)}.`, ); - if (authLoader == null) { - printLookupFailureHint(authLoader, error); - } else { + const hint = getLookupFailureHint(error); + if (shouldSuggestSuppressErrorsForLookupFailure(authLoader, hint)) { printError( message`Use the -S/--suppress-errors option to suppress partial errors.`, ); + } else { + printLookupFailureHint(authLoader, error); } } await finalizeAndExit(1); From d3737a8e44e2b1fa4f42579f48afb5a69a411206 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 8 Mar 2026 20:30:22 +0900 Subject: [PATCH 46/46] Support extensionless nested image URLs in downloadImage Some image URLs use nested extensionless paths such as /media/12345. Rejecting all such URLs made image rendering skip common CDN/proxy media endpoints. Derive the extension from the filename when present; otherwise, infer it from Content-Type (with a safe fallback). Keep blocking suspicious encoded traversal patterns in the pathname. https://github.com/fedify-dev/fedify/pull/608#discussion_r2901706950 Co-Authored-By: Codex --- packages/cli/src/imagerenderer.test.ts | 24 +++++++++++++++++- packages/cli/src/imagerenderer.ts | 35 +++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/imagerenderer.test.ts b/packages/cli/src/imagerenderer.test.ts index 0e64636ec..383369919 100644 --- a/packages/cli/src/imagerenderer.test.ts +++ b/packages/cli/src/imagerenderer.test.ts @@ -182,7 +182,7 @@ test("downloadImage - rejects unsafe extension containing path traversal", async try { const result = await downloadImage( - "https://198.51.100.10/image.png/../../../../etc/passwd", + "https://198.51.100.10/image.png/..%2f..%2f..%2fetc%2fpasswd", ); assert.equal(result, null); } finally { @@ -208,3 +208,25 @@ test("downloadImage - falls back to jpg when URL has no extension", async () => } } }); + +test("downloadImage - falls back to content type for extensionless nested path", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = ((_input: URL | RequestInfo) => + Promise.resolve( + new Response(new Uint8Array([1, 2, 3]), { + headers: { "content-type": "image/png" }, + }), + )) as typeof fetch; + + let result: string | null = null; + try { + result = await downloadImage("https://198.51.100.10/media/12345"); + assert.notEqual(result, null); + assert.equal(path.extname(result!), ".png"); + } finally { + globalThis.fetch = originalFetch; + if (result != null) { + await rm(path.dirname(result), { recursive: true, force: true }); + } + } +}); diff --git a/packages/cli/src/imagerenderer.ts b/packages/cli/src/imagerenderer.ts index 96fffd306..bd5d59c31 100644 --- a/packages/cli/src/imagerenderer.ts +++ b/packages/cli/src/imagerenderer.ts @@ -20,6 +20,30 @@ const KITTY_IDENTIFIERS: string[] = [ type KittyCommand = Record; +function getExtensionFromContentType(contentType: string | null): string { + const mime = contentType?.split(";")[0]?.trim().toLowerCase() ?? ""; + switch (mime) { + case "image/jpeg": + case "image/jpg": + return "jpg"; + case "image/png": + return "png"; + case "image/gif": + return "gif"; + case "image/webp": + return "webp"; + case "image/avif": + return "avif"; + case "image/bmp": + return "bmp"; + case "image/svg+xml": + return "svg"; + default: + if (mime.startsWith("image/")) return mime.slice("image/".length); + return "jpg"; + } +} + export function detectTerminalCapabilities(): TerminalType { const termProgram = (process.env.TERM_PROGRAM || "").toLowerCase(); @@ -132,15 +156,20 @@ export async function downloadImage(url: string): Promise { } const imageData = new Uint8Array(await response.arrayBuffer()); const pathname = new URL(targetUrl).pathname; + const lowerPathname = pathname.toLowerCase(); + if ( + lowerPathname.includes("%2f") || lowerPathname.includes("%5c") || + lowerPathname.includes("..") + ) { + return null; + } const pathSegments = pathname.split("/").filter((segment) => segment !== "" ); const filename = pathSegments[pathSegments.length - 1] ?? ""; const extension = filename.includes(".") ? path.extname(filename).slice(1) - : pathSegments.length === 1 - ? "jpg" - : ""; + : getExtensionFromContentType(response.headers.get("content-type")); if (extension.length < 1) return null; if ( extension.includes("/") || extension.includes("\\") ||