From eda267c08555d6660f4e8d960020638cfe241796 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 7 Jun 2026 23:11:01 +0900 Subject: [PATCH 01/12] Tighten benchmark safety planning Resolve public-looking benchmark targets through DNS before applying the safety gate, and keep unresolved or mixed-public answers in the public tier. Require unsafe public-target overrides to name the target on the command line and to set load and duration explicitly. Make dry-run mode perform discovery planning for runnable scenarios while still avoiding benchmark load, so inbox plans show the resolved actor and inbox destination before a real run. https://github.com/fedify-dev/fedify/issues/744 https://github.com/fedify-dev/fedify/issues/784 Assisted-by: Codex:gpt-5.5 --- packages/cli/src/bench/action.test.ts | 42 ++++- packages/cli/src/bench/action.ts | 190 +++++++++++++++++--- packages/cli/src/bench/safety/gate.test.ts | 70 ++++++++ packages/cli/src/bench/safety/gate.ts | 67 +++++++ packages/cli/src/bench/safety/tiers.test.ts | 26 ++- packages/cli/src/bench/safety/tiers.ts | 64 +++++++ 6 files changed, 432 insertions(+), 27 deletions(-) diff --git a/packages/cli/src/bench/action.test.ts b/packages/cli/src/bench/action.test.ts index c3b7be68c..4640caf1a 100644 --- a/packages/cli/src/bench/action.test.ts +++ b/packages/cli/src/bench/action.test.ts @@ -19,6 +19,7 @@ async function spawnTarget() { benchmarkMode: true, }); let keyPairs: CryptoKeyPair[] | undefined; + const requests: { method: string; path: string }[] = []; federation .setActorDispatcher("/users/{identifier}", async (ctx, identifier) => { if (identifier !== "alice") return null; @@ -51,6 +52,8 @@ async function spawnTarget() { hostname: "127.0.0.1", silent: true, fetch: (request: Request) => { + const url = new URL(request.url); + requests.push({ method: request.method, path: url.pathname }); if (request.method === "POST") { inboxUserAgent = request.headers.get("user-agent"); } @@ -61,6 +64,7 @@ async function spawnTarget() { return { url: new URL(server.url!), inboxUserAgent: () => inboxUserAgent, + requests: () => requests.slice(), close: () => server.close(true), }; } @@ -213,12 +217,48 @@ test("runBench - dry run prints a plan and sends nothing", async () => { }); assert.strictEqual(code, 0); assert.match(output, /dry run/i); - assert.match(output, /No requests were sent/); + assert.match(output, /\/inbox/); + assert.match(output, /No benchmark load was sent/); + const requests = target.requests(); + assert.ok(requests.some((r) => r.method === "GET")); + assert.ok(!requests.some((r) => r.method === "POST")); } finally { await target.close(); } }); +test("runBench - unsafe override requires an explicit CLI target", async () => { + const file = await writeSuite(`version: 1 +target: https://example.com +scenarios: + - name: wf + type: webfinger + recipient: "acct:alice@example.com" + load: { rate: 1/s } + duration: 1ms +`); + let code = -1; + let message = ""; + await runBench(command({ scenario: file, allowUnsafeTarget: true }), { + exit: (c) => { + code = c; + }, + writeOutput: () => Promise.resolve(), + log: (m) => { + message = m; + }, + fetch: (input) => { + const url = new URL(input instanceof Request ? input.url : input); + if (url.pathname.includes("/bench/stats")) { + return Promise.resolve(new Response("not found", { status: 404 })); + } + return Promise.resolve(new Response("ok")); + }, + }); + assert.strictEqual(code, 2); + assert.match(message, /--target/); +}); + test("runBench - refuses an unsafe public target (exit 2)", async () => { const file = await writeSuite(`version: 1 target: https://example.com diff --git a/packages/cli/src/bench/action.ts b/packages/cli/src/bench/action.ts index b91486b7e..1e4b87fa2 100644 --- a/packages/cli/src/bench/action.ts +++ b/packages/cli/src/bench/action.ts @@ -1,8 +1,10 @@ import { writeFile } from "node:fs/promises"; +import type { DocumentLoader } from "@fedify/vocab-runtime"; import process from "node:process"; import { getContextLoader, getDocumentLoader } from "../docloader.ts"; import { buildFleet } from "./actor/fleet.ts"; import type { BenchCommand } from "./command.ts"; +import { discoverInbox, selectInbox } from "./discovery/discover.ts"; import { buildReport, buildScenarioResult, @@ -18,20 +20,25 @@ import { type ResolvedScenario, type ResolvedSuite, } from "./scenario/normalize.ts"; -import type { Suite } from "./scenario/types.ts"; +import type { LoadConfig, Suite } from "./scenario/types.ts"; import { validateSuite } from "./scenario/validate.ts"; import { assertInboxDestinationAllowed, assertTargetAllowed, + assertUnsafeOverrideAllowed, UnsafeTargetError, } from "./safety/gate.ts"; -import { classifyTarget } from "./safety/tiers.ts"; +import { + classifyResolvedTarget, + type ResolveTargetAddresses, +} from "./safety/tiers.ts"; import { runnerFor } from "./scenarios/registry.ts"; import { resolveAdvertiseHost, spawnSyntheticServer, type SyntheticServer, } from "./server/synthetic.ts"; +import { convertUrlIfHandle } from "../webfinger/lib.ts"; /** Injectable dependencies for {@link runBench}, overridable in tests. */ export interface RunBenchDeps { @@ -46,6 +53,8 @@ export interface RunBenchDeps { readonly log?: (message: string) => void; /** Fetch implementation. */ readonly fetch?: typeof fetch; + /** Hostname resolver used for target risk classification. */ + readonly resolveTargetAddresses?: ResolveTargetAddresses; } /** The scenario types that need the synthetic actor/key server. */ @@ -109,19 +118,26 @@ export default async function runBench( return void exit(2); } - if (command.dryRun) { - await writeOutput(renderPlan(suite), command.output); - return void exit(0); - } - - const tier = classifyTarget(suite.target); + const tier = await classifyResolvedTarget( + suite.target, + deps.resolveTargetAddresses, + ); const probe = await probeBenchmarkMode(suite.target, fetchImpl); try { + if (!command.dryRun) { + assertUnsafeOverrideAllowed({ + tier, + benchmarkMode: probe.benchmarkMode, + allowUnsafe: command.allowUnsafeTarget, + explicitCliTarget: command.target != null, + scenarios: unsafeOverrideScenarios(validated), + }); + } assertTargetAllowed({ tier, benchmarkMode: probe.benchmarkMode, allowUnsafe: command.allowUnsafeTarget, - dryRun: false, + dryRun: command.dryRun, }); } catch (error) { if (error instanceof UnsafeTargetError) { @@ -136,20 +152,6 @@ export default async function runBench( // same-machine (loopback) target; a non-loopback target needs an advertised, // reachable host (--advertise-host). Without one, refuse signed scenarios // rather than let every signed delivery fail key lookup. - if ( - tier !== "loopback" && command.advertiseHost == null && - suite.scenarios.some((s) => SIGNED_TYPES.has(s.type)) - ) { - log( - "Signed scenarios (inbox) need the benchmark's synthetic actor server to " + - "be reachable from the target. A loopback target reaches it " + - "automatically; for a non-loopback target, pass --advertise-host with " + - "an address the target can reach (the synthetic server then binds all " + - "interfaces), or use a read scenario such as webfinger.", - ); - return void exit(2); - } - const allowPrivateAddress = tier !== "public"; const documentLoader = await getDocumentLoader({ allowPrivateAddress, @@ -170,6 +172,43 @@ export default async function runBench( advertised: command.advertiseHost != null, }); + if (command.dryRun) { + try { + await writeOutput( + await renderPlan(suite, { + documentLoader, + contextLoader, + allowPrivateAddress, + assertDestinationAllowed, + }), + command.output, + ); + return void exit(0); + } catch (error) { + log(error instanceof Error ? error.message : String(error)); + return void exit(2); + } + } + + // The target dereferences the synthetic actor server while verifying + // signatures. By default that server is loopback-only, reachable just by a + // same-machine (loopback) target; a non-loopback target needs an advertised, + // reachable host (--advertise-host). Without one, refuse signed scenarios + // rather than let every signed delivery fail key lookup. + if ( + tier !== "loopback" && command.advertiseHost == null && + suite.scenarios.some((s) => SIGNED_TYPES.has(s.type)) + ) { + log( + "Signed scenarios (inbox) need the benchmark's synthetic actor server to " + + "be reachable from the target. A loopback target reaches it " + + "automatically; for a non-loopback target, pass --advertise-host with " + + "an address the target can reach (the synthetic server then binds all " + + "interfaces), or use a read scenario such as webfinger.", + ); + return void exit(2); + } + let fleet: SyntheticServer | undefined; const startedAt = new Date().toISOString(); try { @@ -278,7 +317,17 @@ async function defaultWriteOutput( await writeFile(outputPath, content, { encoding: "utf-8" }); } -function renderPlan(suite: ResolvedSuite): string { +interface DryRunPlanContext { + readonly documentLoader: DocumentLoader; + readonly contextLoader: DocumentLoader; + readonly allowPrivateAddress: boolean; + readonly assertDestinationAllowed: (url: URL) => void; +} + +async function renderPlan( + suite: ResolvedSuite, + context: DryRunPlanContext, +): Promise { const lines = [ "Fedify benchmark plan (dry run)", "", @@ -289,8 +338,13 @@ function renderPlan(suite: ResolvedSuite): string { lines.push( `- ${scenario.name} (${scenario.type}): ${describePlan(scenario)}`, ); + lines.push(...await describeDiscoveryPlan(scenario, suite, context)); } - lines.push("", "No requests were sent."); + lines.push( + "", + "No benchmark load was sent. Discovery and stats probe requests may " + + "have been sent.", + ); return `${lines.join("\n")}\n`; } @@ -300,3 +354,89 @@ function describePlan(scenario: ResolvedScenario): string { : `closed-loop concurrency ${scenario.load.concurrency}`; return `${load}, duration ${scenario.durationMs}ms, signing ${scenario.signing}`; } + +async function describeDiscoveryPlan( + scenario: ResolvedScenario, + suite: ResolvedSuite, + context: DryRunPlanContext, +): Promise { + switch (scenario.type) { + case "inbox": + return await describeInboxDiscoveryPlan(scenario, context); + case "webfinger": + return describeWebFingerPlan(scenario, suite.target); + default: + return [" discovery: not available for this scenario type"]; + } +} + +async function describeInboxDiscoveryPlan( + scenario: ResolvedScenario, + context: DryRunPlanContext, +): Promise { + const lines: string[] = []; + for (const recipient of scenario.recipients) { + const discovered = await discoverInbox(recipient, { + documentLoader: context.documentLoader, + contextLoader: context.contextLoader, + allowPrivateAddress: context.allowPrivateAddress, + }); + const inbox = selectInbox(discovered, scenario.inbox); + lines.push( + ` recipient ${recipient}: actor ${discovered.actorUri.href}, ` + + `inbox ${inbox.href}`, + ); + lines.push( + ` destination safety: ${describeDestinationSafety(inbox, context)}`, + ); + } + return lines; +} + +function describeWebFingerPlan( + scenario: ResolvedScenario, + target: URL, +): string[] { + const recipients = scenario.recipients.length > 0 + ? scenario.recipients + : [target.href]; + return recipients.map((recipient) => { + const resource = convertUrlIfHandle(recipient).href; + const url = new URL("/.well-known/webfinger", target); + url.searchParams.set("resource", resource); + return ` webfinger ${resource}: GET ${url.href}`; + }); +} + +function describeDestinationSafety( + inbox: URL, + context: DryRunPlanContext, +): string { + try { + context.assertDestinationAllowed(inbox); + return "allowed"; + } catch (error) { + if (error instanceof UnsafeTargetError) { + return `would be refused: ${error.message}`; + } + throw error; + } +} + +function unsafeOverrideScenarios( + suite: Suite, +): Parameters[0]["scenarios"] { + const defaultDuration = suite.defaults?.duration != null; + const defaultLoad = hasExplicitLoad(suite.defaults?.load); + return suite.scenarios.map((scenario) => ({ + name: scenario.name, + explicitDuration: scenario.duration != null || defaultDuration, + explicitLoad: hasExplicitLoad(scenario.load) || defaultLoad, + })); +} + +function hasExplicitLoad(load: LoadConfig | undefined): boolean { + return load != null && + (("rate" in load && load.rate != null) || + ("concurrency" in load && load.concurrency != null)); +} diff --git a/packages/cli/src/bench/safety/gate.test.ts b/packages/cli/src/bench/safety/gate.test.ts index 6998b7df7..638549665 100644 --- a/packages/cli/src/bench/safety/gate.test.ts +++ b/packages/cli/src/bench/safety/gate.test.ts @@ -3,6 +3,7 @@ import test from "node:test"; import { assertInboxDestinationAllowed, assertTargetAllowed, + assertUnsafeOverrideAllowed, UnsafeTargetError, } from "./gate.ts"; @@ -60,6 +61,75 @@ test("assertTargetAllowed - the unsafe flag overrides the refusal", () => { ); }); +test("assertUnsafeOverrideAllowed - unsafe flag needs an explicit CLI target", () => { + assert.throws( + () => + assertUnsafeOverrideAllowed({ + tier: "public", + benchmarkMode: false, + allowUnsafe: true, + explicitCliTarget: false, + scenarios: [{ + name: "wf", + explicitDuration: true, + explicitLoad: true, + }], + }), + (error: unknown) => + error instanceof UnsafeTargetError && /--target/.test(error.message), + ); +}); + +test("assertUnsafeOverrideAllowed - unsafe public defaults need explicit load", () => { + assert.throws( + () => + assertUnsafeOverrideAllowed({ + tier: "public", + benchmarkMode: false, + allowUnsafe: true, + explicitCliTarget: true, + scenarios: [{ + name: "wf", + explicitDuration: true, + explicitLoad: false, + }], + }), + (error: unknown) => + error instanceof UnsafeTargetError && /load/.test(error.message), + ); +}); + +test("assertUnsafeOverrideAllowed - unsafe public defaults need explicit duration", () => { + assert.throws( + () => + assertUnsafeOverrideAllowed({ + tier: "public", + benchmarkMode: false, + allowUnsafe: true, + explicitCliTarget: true, + scenarios: [{ + name: "wf", + explicitDuration: false, + explicitLoad: true, + }], + }), + (error: unknown) => + error instanceof UnsafeTargetError && /duration/.test(error.message), + ); +}); + +test("assertUnsafeOverrideAllowed - safe targets do not need unsafe metadata", () => { + assert.doesNotThrow(() => + assertUnsafeOverrideAllowed({ + tier: "loopback", + benchmarkMode: false, + allowUnsafe: false, + explicitCliTarget: false, + scenarios: [], + }) + ); +}); + test("assertTargetAllowed - dry-run bypasses the gate", () => { assert.doesNotThrow(() => assertTargetAllowed({ diff --git a/packages/cli/src/bench/safety/gate.ts b/packages/cli/src/bench/safety/gate.ts index c89ed867f..ecb73fa0a 100644 --- a/packages/cli/src/bench/safety/gate.ts +++ b/packages/cli/src/bench/safety/gate.ts @@ -46,6 +46,73 @@ export function assertTargetAllowed(context: GateContext): void { ); } +/** Per-scenario metadata needed when an unsafe public target is overridden. */ +export interface UnsafeOverrideScenario { + /** The scenario name, used in diagnostics. */ + readonly name: string; + /** Whether the scenario or suite explicitly set a duration. */ + readonly explicitDuration: boolean; + /** Whether the scenario or suite explicitly selected a load model. */ + readonly explicitLoad: boolean; +} + +/** The inputs for validating an unsafe public-target override. */ +export interface UnsafeOverrideContext { + /** The target's risk tier. */ + readonly tier: TargetTier; + /** Whether the target advertises benchmark mode. */ + readonly benchmarkMode: boolean; + /** Whether `--allow-unsafe-target` was given. */ + readonly allowUnsafe: boolean; + /** Whether the run supplied `--target` on the command line. */ + readonly explicitCliTarget: boolean; + /** Scenario metadata for explicit-load checks. */ + readonly scenarios: readonly UnsafeOverrideScenario[]; +} + +/** + * Asserts that an unsafe public-target override is specific and bounded. + * + * The override is only meaningful for a public target that does not advertise + * benchmark mode. In that caution tier, the operator must name the target on + * the command line for this run and must explicitly set load and duration, so + * the built-in defaults cannot accidentally create a long public benchmark. + * @param context The unsafe override decision inputs. + * @throws {UnsafeTargetError} If the unsafe override is too broad. + */ +export function assertUnsafeOverrideAllowed( + context: UnsafeOverrideContext, +): void { + if ( + context.tier !== "public" || context.benchmarkMode || + !context.allowUnsafe + ) { + return; + } + if (!context.explicitCliTarget) { + throw new UnsafeTargetError( + "The --allow-unsafe-target override must be paired with an explicit " + + "--target for this run.", + ); + } + for (const scenario of context.scenarios) { + if (!scenario.explicitLoad) { + throw new UnsafeTargetError( + `Scenario "${scenario.name}" uses the built-in benchmark load ` + + "default. Set rate or concurrency explicitly before using " + + "--allow-unsafe-target against a public target.", + ); + } + if (!scenario.explicitDuration) { + throw new UnsafeTargetError( + `Scenario "${scenario.name}" uses the built-in benchmark duration ` + + "default. Set duration explicitly before using " + + "--allow-unsafe-target against a public target.", + ); + } + } +} + /** The inputs to gating a resolved inbox load destination. */ export interface InboxDestinationGateContext { /** diff --git a/packages/cli/src/bench/safety/tiers.test.ts b/packages/cli/src/bench/safety/tiers.test.ts index e5ee4c696..36fddc747 100644 --- a/packages/cli/src/bench/safety/tiers.test.ts +++ b/packages/cli/src/bench/safety/tiers.test.ts @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { classifyTarget } from "./tiers.ts"; +import { classifyResolvedTarget, classifyTarget } from "./tiers.ts"; test("classifyTarget - loopback", () => { for ( @@ -78,3 +78,27 @@ test("classifyTarget - IPv4-mapped IPv6 loopback/private", () => { "private", ); }); + +test("classifyResolvedTarget - classifies a public hostname by resolved private address", async () => { + const tier = await classifyResolvedTarget( + new URL("https://bench.example"), + () => Promise.resolve(["10.0.0.5"]), + ); + assert.strictEqual(tier, "private"); +}); + +test("classifyResolvedTarget - treats mixed public resolutions as public", async () => { + const tier = await classifyResolvedTarget( + new URL("https://bench.example"), + () => Promise.resolve(["10.0.0.5", "8.8.8.8"]), + ); + assert.strictEqual(tier, "public"); +}); + +test("classifyResolvedTarget - treats resolution failure as public", async () => { + const tier = await classifyResolvedTarget( + new URL("https://bench.example"), + () => Promise.reject(new Error("dns down")), + ); + assert.strictEqual(tier, "public"); +}); diff --git a/packages/cli/src/bench/safety/tiers.ts b/packages/cli/src/bench/safety/tiers.ts index e666b4846..d5d410eb6 100644 --- a/packages/cli/src/bench/safety/tiers.ts +++ b/packages/cli/src/bench/safety/tiers.ts @@ -10,9 +10,16 @@ * @module */ +import { lookup } from "node:dns/promises"; + /** The risk tier of a benchmark target. */ export type TargetTier = "loopback" | "private" | "public"; +/** Resolves a hostname to IP addresses for target risk classification. */ +export type ResolveTargetAddresses = ( + hostname: string, +) => Promise; + /** * Classifies a target URL into a risk tier from its host. * @param target The target URL. @@ -31,6 +38,63 @@ export function classifyTarget(target: URL): TargetTier { return "public"; } +/** + * Classifies a target URL, resolving DNS for public-looking hostnames. + * + * Literal addresses and known local hostname suffixes are classified directly. + * Other hostnames are resolved and classified from their addresses; any public + * address in the answer keeps the target public, and DNS failure is treated as + * public so the safety gate remains conservative. + * @param target The target URL. + * @param resolveAddresses Hostname resolver, overridable for tests. + * @returns The resolved target tier. + */ +export async function classifyResolvedTarget( + target: URL, + resolveAddresses: ResolveTargetAddresses = defaultResolveTargetAddresses, +): Promise { + const host = normalizedHost(target); + const direct = classifyTarget(target); + if (direct !== "public" || isIpLiteral(host)) return direct; + let addresses: readonly string[]; + try { + addresses = await resolveAddresses(host); + } catch { + return "public"; + } + if (addresses.length < 1) return "public"; + let aggregate: TargetTier = "loopback"; + for (const address of addresses) { + const tier = classifyTarget(new URL(`http://${hostForAddress(address)}/`)); + if (tier === "public") return "public"; + if (tier === "private") aggregate = "private"; + } + return aggregate; +} + +/** Resolves a hostname with the platform DNS resolver. */ +export async function defaultResolveTargetAddresses( + hostname: string, +): Promise { + const entries = await lookup(hostname, { all: true }); + return entries.map((entry) => entry.address); +} + +function normalizedHost(target: URL): string { + let host = target.hostname.replace(/^\[/, "").replace(/\]$/, "") + .toLowerCase(); + if (host.endsWith(".")) host = host.slice(0, -1); + return host; +} + +function isIpLiteral(host: string): boolean { + return isIpv4(host) || host.includes(":"); +} + +function hostForAddress(address: string): string { + return address.includes(":") ? `[${address}]` : address; +} + function isIpv4(host: string): boolean { const match = host.match(/^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$/); return match != null && match.slice(1).every((octet) => Number(octet) <= 255); From d63f8eceb3db63e3386177edae250435dfbbe32d Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 7 Jun 2026 23:16:25 +0900 Subject: [PATCH 02/12] Extract the benchmark test fixture Move the local benchmark-mode Fedify target used by fedify bench integration tests into test/bench. This gives the scenario tests a shared fixture app that exercises WebFinger, actor discovery, signature verification, and inbox handling through the same benchmark target. https://github.com/fedify-dev/fedify/issues/744 https://github.com/fedify-dev/fedify/issues/784 Assisted-by: Codex:gpt-5.5 --- packages/cli/src/bench/action.test.ts | 72 ++------------------- test/bench/fixture.ts | 90 +++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 67 deletions(-) create mode 100644 test/bench/fixture.ts diff --git a/packages/cli/src/bench/action.test.ts b/packages/cli/src/bench/action.test.ts index 4640caf1a..f05958cc3 100644 --- a/packages/cli/src/bench/action.test.ts +++ b/packages/cli/src/bench/action.test.ts @@ -1,74 +1,12 @@ -import { - createFederation, - generateCryptoKeyPair, - MemoryKvStore, -} from "@fedify/fedify"; -import { Create, Endpoints, Person } from "@fedify/vocab"; import assert from "node:assert/strict"; import { mkdtemp, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import test from "node:test"; -import { serve } from "srvx"; +import { spawnBenchmarkTarget } from "../../../../test/bench/fixture.ts"; import runBench, { withUserAgent } from "./action.ts"; import type { BenchCommand } from "./command.ts"; -async function spawnTarget() { - const federation = createFederation({ - kv: new MemoryKvStore(), - benchmarkMode: true, - }); - let keyPairs: CryptoKeyPair[] | undefined; - const requests: { method: string; path: string }[] = []; - federation - .setActorDispatcher("/users/{identifier}", async (ctx, identifier) => { - if (identifier !== "alice") return null; - const pairs = await ctx.getActorKeyPairs(identifier); - return new Person({ - id: ctx.getActorUri(identifier), - preferredUsername: identifier, - inbox: ctx.getInboxUri(identifier), - endpoints: new Endpoints({ sharedInbox: ctx.getInboxUri() }), - publicKey: pairs[0]?.cryptographicKey, - assertionMethods: pairs.map((p) => p.multikey), - }); - }) - .mapHandle((_ctx, username) => (username === "alice" ? "alice" : null)) - .setKeyPairsDispatcher(async (_ctx, identifier) => { - if (identifier !== "alice") return []; - keyPairs ??= [ - await generateCryptoKeyPair("RSASSA-PKCS1-v1_5"), - await generateCryptoKeyPair("Ed25519"), - ]; - return keyPairs; - }); - federation.setInboxListeners("/users/{identifier}/inbox", "/inbox").on( - Create, - () => {}, - ); - let inboxUserAgent: string | null = null; - const server = serve({ - port: 0, - hostname: "127.0.0.1", - silent: true, - fetch: (request: Request) => { - const url = new URL(request.url); - requests.push({ method: request.method, path: url.pathname }); - if (request.method === "POST") { - inboxUserAgent = request.headers.get("user-agent"); - } - return federation.fetch(request, { contextData: undefined }); - }, - }); - await server.ready(); - return { - url: new URL(server.url!), - inboxUserAgent: () => inboxUserAgent, - requests: () => requests.slice(), - close: () => server.close(true), - }; -} - function command(overrides: Partial): BenchCommand { return { command: "bench", @@ -108,7 +46,7 @@ ${expectLine} } test("runBench - passing gate exits 0 and writes a valid report", async () => { - const target = await spawnTarget(); + const target = await spawnBenchmarkTarget(); try { const file = await writeSuite( inboxSuite(target.url, ' successRate: ">= 99%"'), @@ -177,7 +115,7 @@ test("withUserAgent - does not override an explicit User-Agent", async () => { }); test("runBench - failing gate exits 1", async () => { - const target = await spawnTarget(); + const target = await spawnBenchmarkTarget(); try { // An impossible latency threshold makes the gate fail. const file = await writeSuite( @@ -198,7 +136,7 @@ test("runBench - failing gate exits 1", async () => { }); test("runBench - dry run prints a plan and sends nothing", async () => { - const target = await spawnTarget(); + const target = await spawnBenchmarkTarget(); try { const file = await writeSuite( inboxSuite(target.url, ' successRate: ">= 99%"'), @@ -346,7 +284,7 @@ test("runBench - refuses an inbox destination off the gated target (exit 2)", as // A loopback target passes the gate, but an explicit public `inbox:` is the // actual load destination; it must be gated too, or production could be // benchmarked through the back door. - const target = await spawnTarget(); + const target = await spawnBenchmarkTarget(); try { const file = await writeSuite(`version: 1 target: ${target.url.href} diff --git a/test/bench/fixture.ts b/test/bench/fixture.ts new file mode 100644 index 000000000..13f79d1b9 --- /dev/null +++ b/test/bench/fixture.ts @@ -0,0 +1,90 @@ +import { + createFederation, + generateCryptoKeyPair, + MemoryKvStore, +} from "@fedify/fedify"; +import { Create, Endpoints, Person } from "@fedify/vocab"; + +/** A running local benchmark-mode target used by `fedify bench` tests. */ +export interface BenchmarkTargetFixture { + /** The target base URL. */ + readonly url: URL; + /** The last User-Agent observed on signed inbox load. */ + readonly inboxUserAgent: () => string | null; + /** The HTTP requests the fixture target has received. */ + readonly requests: () => readonly { method: string; path: string }[]; + /** Stops the fixture server. */ + readonly close: () => Promise; +} + +/** + * Starts a local Fedify target in benchmark mode. + * + * The app exposes one actor, `alice`, with a personal and shared inbox, and an + * inbox listener that accepts signed `Create` activities. It is intentionally + * small but exercises the same WebFinger, actor discovery, signature + * verification, and inbox paths that `fedify bench` drives. + * @returns The running fixture. + */ +export function spawnBenchmarkTarget(): BenchmarkTargetFixture { + const federation = createFederation({ + kv: new MemoryKvStore(), + benchmarkMode: true, + }); + let keyPairs: CryptoKeyPair[] | undefined; + const requests: { method: string; path: string }[] = []; + federation + .setActorDispatcher("/users/{identifier}", async (ctx, identifier) => { + if (identifier !== "alice") return null; + const pairs = await ctx.getActorKeyPairs(identifier); + return new Person({ + id: ctx.getActorUri(identifier), + preferredUsername: identifier, + inbox: ctx.getInboxUri(identifier), + endpoints: new Endpoints({ sharedInbox: ctx.getInboxUri() }), + publicKey: pairs[0]?.cryptographicKey, + assertionMethods: pairs.map((p) => p.multikey), + }); + }) + .mapHandle((_ctx, username) => (username === "alice" ? "alice" : null)) + .setKeyPairsDispatcher(async (_ctx, identifier) => { + if (identifier !== "alice") return []; + keyPairs ??= [ + await generateCryptoKeyPair("RSASSA-PKCS1-v1_5"), + await generateCryptoKeyPair("Ed25519"), + ]; + return keyPairs; + }); + federation.setInboxListeners("/users/{identifier}/inbox", "/inbox").on( + Create, + () => {}, + ); + let inboxUserAgent: string | null = null; + const abort = new AbortController(); + const server = Deno.serve( + { + port: 0, + hostname: "127.0.0.1", + signal: abort.signal, + onListen: () => {}, + }, + (request: Request) => { + const url = new URL(request.url); + requests.push({ method: request.method, path: url.pathname }); + if (request.method === "POST") { + inboxUserAgent = request.headers.get("user-agent"); + } + return federation.fetch(request, { contextData: undefined }); + }, + ); + const address = server.addr as Deno.NetAddr; + return { + url: new URL(`http://${address.hostname}:${address.port}/`), + inboxUserAgent: () => inboxUserAgent, + requests: () => requests.slice(), + close: async () => { + abort.abort(); + await server.finished.catch(() => {}); + }, + }; +} From 8c48bcdb67c5fbb0d306166c4485ee220902e76f Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 7 Jun 2026 23:18:37 +0900 Subject: [PATCH 03/12] Document benchmark safety planning Update the benchmark manual and CLI help to describe discovery-aware dry runs, DNS-backed target classification, and the narrower unsafe public-target override. Link deployment guidance to fedify bench so staging and CI checks are part of production preparation. https://github.com/fedify-dev/fedify/issues/744 https://github.com/fedify-dev/fedify/issues/784 Assisted-by: Codex:gpt-5.5 --- CHANGES.md | 6 +++++- docs/manual/benchmarking.md | 25 ++++++++++++++++++++----- docs/manual/deploy.md | 11 +++++++++++ packages/cli/src/bench/command.ts | 2 +- 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index ab426ace9..29fd7c1a0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -286,9 +286,13 @@ To be released. throughput, success rate, and errors, reading server-side metrics from the target's stats endpoint. Benchmarks are described by a YAML or JSON scenario suite validated against a published JSON Schema, with an `expect` - block per scenario that gates a run for CI. [[#744], [#783]] + block per scenario that gates a run for CI. The command refuses public + non-`benchmarkMode` targets without an explicit unsafe override, supports + discovery-aware `--dry-run` planning, and ships with a local benchmark + fixture used by the scenario tests. [[#744], [#783], [#784]] [#783]: https://github.com/fedify-dev/fedify/issues/783 +[#784]: https://github.com/fedify-dev/fedify/issues/784 ### @fedify/fixture diff --git a/docs/manual/benchmarking.md b/docs/manual/benchmarking.md index 3d103d31f..0c04a2387 100644 --- a/docs/manual/benchmarking.md +++ b/docs/manual/benchmarking.md @@ -151,7 +151,10 @@ The `# yaml-language-server:` line gives editors autocomplete and validation against the [published schema]. Override the file's target with `--target`, choose the output with `--format`/`--output`, and inspect a run without sending anything with -`--dry-run`. +`--dry-run`. A dry run still probes the target's benchmark stats endpoint and +resolves scenario discovery, such as WebFinger and actor inbox lookup, so the +printed plan shows the concrete destinations a real run would use. It does +not send benchmark load. An `inbox` scenario's `recipient` may be a single value or a list. With a list, deliveries are rotated across the recipients (and across the synthetic @@ -241,10 +244,22 @@ belongs in a controlled environment, not a shared CI runner. ### Safety `fedify bench` runs without friction against a loopback or private target, or -any target that advertises benchmark mode. A public target that does not -advertise benchmark mode is refused unless you pass `--allow-unsafe-target`, -which is mandatory (never prompted) in CI and any non-interactive context. Use -`--dry-run` to print the plan without sending anything. +any target that advertises benchmark mode. Hostnames are classified from their +resolved addresses when possible, and DNS failures are treated as public so the +gate stays conservative. A public target that does not advertise benchmark +mode is refused unless you pass `--allow-unsafe-target`, which is mandatory +(never prompted) in CI and any non-interactive context. + +The unsafe override is deliberately narrow. It must be paired with an +explicit `--target` on the command line, and every scenario must set its load +(`rate` or `concurrency`) and `duration` explicitly, either in the scenario or +in suite defaults. This prevents a public run from falling back to built-in +defaults by accident. + +Use `--dry-run` as the first step against an unfamiliar target. It performs +the benchmark-mode probe and discovery requests needed to print the planned +WebFinger resources and inbox destinations, but it does not send signed inbox +deliveries or other benchmark load. ### Local targets over HTTP diff --git a/docs/manual/deploy.md b/docs/manual/deploy.md index 66be368d7..24cd88166 100644 --- a/docs/manual/deploy.md +++ b/docs/manual/deploy.md @@ -1287,6 +1287,17 @@ operations across your infrastructure. For production: [OpenTelemetry]: https://opentelemetry.io/ +### Benchmarking before production + +Before changing queue backends, federation handlers, or signature-related +configuration, run [`fedify bench`](./benchmarking.md) against a local or +staging target that enables `benchmarkMode`. The benchmark command drives +signed inbox deliveries and WebFinger lookups, reports latency, throughput, +success rate, and errors, and can gate CI with `expect` thresholds. Do not +enable `benchmarkMode` on production servers; for an unfamiliar target, start +with `--dry-run` to resolve discovery and inspect the planned destinations +without sending benchmark load. + ### Error reporting For error aggregation, the pattern most Fedify applications use is a diff --git a/packages/cli/src/bench/command.ts b/packages/cli/src/bench/command.ts index 3196bc833..553ecc39c 100644 --- a/packages/cli/src/bench/command.ts +++ b/packages/cli/src/bench/command.ts @@ -75,7 +75,7 @@ export const benchCommand = command( dryRun: withDefault( flag("--dry-run", { description: - message`Print the normalized plan without contacting the target or \ + message`Resolve discovery and print the benchmark plan without \ sending load.`, }), false, From e51c5ddf4dde2a836a509033b517c5229a47ad6b Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Sun, 7 Jun 2026 23:38:13 +0900 Subject: [PATCH 04/12] Fix benchmark safety review issues Make the shared benchmark fixture work under Node and Bun by replacing the Deno-only test server with a runtime-neutral node:http bridge. Use the resolved target tier when gating same-origin inbox destinations, and apply the unsafe-target bounds to separate public inbox destinations before signed load can be sent to them. https://github.com/fedify-dev/fedify/issues/744 https://github.com/fedify-dev/fedify/issues/784 Assisted-by: Codex:gpt-5.5 --- packages/cli/src/bench/action.test.ts | 76 ++++++++++++++++++++ packages/cli/src/bench/action.ts | 80 ++++++++++++++++++--- packages/cli/src/bench/safety/gate.test.ts | 28 +++++++- packages/cli/src/bench/safety/gate.ts | 7 +- test/bench/fixture.ts | 83 +++++++++++++++++----- 5 files changed, 242 insertions(+), 32 deletions(-) diff --git a/packages/cli/src/bench/action.test.ts b/packages/cli/src/bench/action.test.ts index f05958cc3..76cb54576 100644 --- a/packages/cli/src/bench/action.test.ts +++ b/packages/cli/src/bench/action.test.ts @@ -314,6 +314,82 @@ scenarios: } }); +test("runBench - unsafe public inbox destination needs an explicit CLI target", async () => { + const target = await spawnBenchmarkTarget(); + try { + const file = await writeSuite(`version: 1 +target: ${target.url.href} +scenarios: + - name: inbox-shared + type: inbox + recipient: "${new URL("/users/alice", target.url).href}" + inbox: "https://prod.example/inbox" + load: { rate: 1/s } + duration: 1ms +`); + let code = -1; + let message = ""; + await runBench( + command({ + scenario: file, + allowUnsafeTarget: true, + advertiseHost: "127.0.0.1", + }), + { + exit: (c) => { + code = c; + }, + writeOutput: () => Promise.resolve(), + log: (m) => { + message = m; + }, + }, + ); + assert.strictEqual(code, 2); + assert.match(message, /--target/); + } finally { + await target.close(); + } +}); + +test("runBench - unsafe public inbox destination needs explicit load", async () => { + const target = await spawnBenchmarkTarget(); + try { + const file = await writeSuite(`version: 1 +target: ${target.url.href} +scenarios: + - name: inbox-shared + type: inbox + recipient: "${new URL("/users/alice", target.url).href}" + inbox: "https://prod.example/inbox" + duration: 1ms +`); + let code = -1; + let message = ""; + await runBench( + command({ + scenario: file, + target: target.url.href, + allowUnsafeTarget: true, + advertiseHost: "127.0.0.1", + }), + { + exit: (c) => { + code = c; + }, + writeOutput: () => Promise.resolve(), + log: (m) => { + message = m; + }, + }, + ); + assert.strictEqual(code, 2); + assert.match(message, /load/); + } finally { + await target.close(); + } +}); + test("runBench - malformed expect assertion exits 2 before any load", async () => { // The expect typo must be caught in preflight, so the run exits 2 (a config // error) without ever probing the target or sending load. diff --git a/packages/cli/src/bench/action.ts b/packages/cli/src/bench/action.ts index 1e4b87fa2..797b8bde3 100644 --- a/packages/cli/src/bench/action.ts +++ b/packages/cli/src/bench/action.ts @@ -30,6 +30,7 @@ import { } from "./safety/gate.ts"; import { classifyResolvedTarget, + classifyTarget, type ResolveTargetAddresses, } from "./safety/tiers.ts"; import { runnerFor } from "./scenarios/registry.ts"; @@ -164,13 +165,24 @@ export default async function runBench( // Gates each resolved inbox destination (which can differ from the suite // target) before the runner sends load to it. - const assertDestinationAllowed = (url: URL): void => + const assertDestinationAllowed = ( + url: URL, + scenario: ResolvedScenario, + ): void => { assertInboxDestinationAllowed(url, { targetOrigin: suite.target.origin, + targetTier: tier, targetBenchmarkMode: probe.benchmarkMode, allowUnsafe: command.allowUnsafeTarget, advertised: command.advertiseHost != null, }); + assertPublicDestinationOverrideAllowed(url, scenario, { + targetOrigin: suite.target.origin, + targetBenchmarkMode: probe.benchmarkMode, + allowUnsafe: command.allowUnsafeTarget, + explicitCliTarget: command.target != null, + }); + }; if (command.dryRun) { try { @@ -229,7 +241,8 @@ export default async function runBench( allowPrivateAddress, fleet: fleet ?? null, fetch: fetchImpl, - assertDestinationAllowed, + assertDestinationAllowed: (url) => + assertDestinationAllowed(url, scenario), }); results.push(buildScenarioResult(scenario, measurement)); } @@ -321,7 +334,10 @@ interface DryRunPlanContext { readonly documentLoader: DocumentLoader; readonly contextLoader: DocumentLoader; readonly allowPrivateAddress: boolean; - readonly assertDestinationAllowed: (url: URL) => void; + readonly assertDestinationAllowed: ( + url: URL, + scenario: ResolvedScenario, + ) => void; } async function renderPlan( @@ -387,7 +403,9 @@ async function describeInboxDiscoveryPlan( `inbox ${inbox.href}`, ); lines.push( - ` destination safety: ${describeDestinationSafety(inbox, context)}`, + ` destination safety: ${ + describeDestinationSafety(inbox, scenario, context) + }`, ); } return lines; @@ -410,10 +428,11 @@ function describeWebFingerPlan( function describeDestinationSafety( inbox: URL, + scenario: ResolvedScenario, context: DryRunPlanContext, ): string { try { - context.assertDestinationAllowed(inbox); + context.assertDestinationAllowed(inbox, scenario); return "allowed"; } catch (error) { if (error instanceof UnsafeTargetError) { @@ -423,16 +442,55 @@ function describeDestinationSafety( } } +interface PublicDestinationOverrideContext { + readonly targetOrigin: string; + readonly targetBenchmarkMode: boolean; + readonly allowUnsafe: boolean; + readonly explicitCliTarget: boolean; +} + +function assertPublicDestinationOverrideAllowed( + url: URL, + scenario: ResolvedScenario, + context: PublicDestinationOverrideContext, +): void { + const inheritsTargetGate = url.origin === context.targetOrigin && + context.targetBenchmarkMode; + if ( + classifyTarget(url) !== "public" || inheritsTargetGate || + !context.allowUnsafe + ) { + return; + } + assertUnsafeOverrideAllowed({ + tier: "public", + benchmarkMode: false, + allowUnsafe: true, + explicitCliTarget: context.explicitCliTarget, + scenarios: [unsafeOverrideScenario(scenario)], + }); +} + function unsafeOverrideScenarios( suite: Suite, ): Parameters[0]["scenarios"] { - const defaultDuration = suite.defaults?.duration != null; - const defaultLoad = hasExplicitLoad(suite.defaults?.load); - return suite.scenarios.map((scenario) => ({ + return suite.scenarios.map((scenario) => + unsafeOverrideScenario(scenario, suite) + ); +} + +function unsafeOverrideScenario( + scenario: ResolvedScenario | Suite["scenarios"][number], + suite?: Suite, +): Parameters[0]["scenarios"][number] { + const defaultDuration = suite?.defaults?.duration != null; + const defaultLoad = hasExplicitLoad(suite?.defaults?.load); + const raw = "raw" in scenario ? scenario.raw : scenario; + return { name: scenario.name, - explicitDuration: scenario.duration != null || defaultDuration, - explicitLoad: hasExplicitLoad(scenario.load) || defaultLoad, - })); + explicitDuration: raw.duration != null || defaultDuration, + explicitLoad: hasExplicitLoad(raw.load) || defaultLoad, + }; } function hasExplicitLoad(load: LoadConfig | undefined): boolean { diff --git a/packages/cli/src/bench/safety/gate.test.ts b/packages/cli/src/bench/safety/gate.test.ts index 638549665..0a0e50dea 100644 --- a/packages/cli/src/bench/safety/gate.test.ts +++ b/packages/cli/src/bench/safety/gate.test.ts @@ -146,6 +146,7 @@ function destContext( ) { return { targetOrigin: "http://127.0.0.1:3000", + targetTier: "loopback" as const, targetBenchmarkMode: false, allowUnsafe: false, advertised: false, @@ -199,6 +200,22 @@ test("assertInboxDestinationAllowed - an inbox on the target origin inherits its ); }); +test("assertInboxDestinationAllowed - same-origin inbox uses the resolved target tier", () => { + // The target hostname may be syntactically public but DNS-resolved private. + // The discovered same-origin inbox should inherit that resolved tier instead + // of being reclassified from the hostname string. + assert.doesNotThrow(() => + assertInboxDestinationAllowed( + new URL("https://staging.example/inbox"), + destContext({ + targetOrigin: "https://staging.example", + targetTier: "private", + advertised: true, + }), + ) + ); +}); + test("assertInboxDestinationAllowed - same host, different scheme does not inherit", () => { // The target is https (its benchmark-mode probe covered port 443); an http // inbox on the same hostname is a different service (port 80), so it must not @@ -225,7 +242,10 @@ test("assertInboxDestinationAllowed - a non-loopback inbox needs an advertised h () => assertInboxDestinationAllowed( new URL("http://10.0.0.5:8000/inbox"), - destContext({ targetOrigin: "http://10.0.0.5:8000" }), + destContext({ + targetOrigin: "http://10.0.0.5:8000", + targetTier: "private", + }), ), (error: unknown) => error instanceof UnsafeTargetError && @@ -234,7 +254,11 @@ test("assertInboxDestinationAllowed - a non-loopback inbox needs an advertised h assert.doesNotThrow(() => assertInboxDestinationAllowed( new URL("http://10.0.0.5:8000/inbox"), - destContext({ targetOrigin: "http://10.0.0.5:8000", advertised: true }), + destContext({ + targetOrigin: "http://10.0.0.5:8000", + targetTier: "private", + advertised: true, + }), ) ); }); diff --git a/packages/cli/src/bench/safety/gate.ts b/packages/cli/src/bench/safety/gate.ts index ecb73fa0a..be79f92bc 100644 --- a/packages/cli/src/bench/safety/gate.ts +++ b/packages/cli/src/bench/safety/gate.ts @@ -122,6 +122,8 @@ export interface InboxDestinationGateContext { * (e.g. an `http://host` inbox does not inherit an `https://host` target). */ readonly targetOrigin: string; + /** The resolved target tier used by the main safety gate. */ + readonly targetTier: TargetTier; /** Whether the gated target advertises benchmark mode. */ readonly targetBenchmarkMode: boolean; /** Whether `--allow-unsafe-target` was given. */ @@ -150,8 +152,9 @@ export function assertInboxDestinationAllowed( url: URL, context: InboxDestinationGateContext, ): void { - const tier = classifyTarget(url); - const inheritsTargetGate = url.origin === context.targetOrigin && + const sameOrigin = url.origin === context.targetOrigin; + const tier = sameOrigin ? context.targetTier : classifyTarget(url); + const inheritsTargetGate = sameOrigin && context.targetBenchmarkMode; if (tier === "public" && !inheritsTargetGate && !context.allowUnsafe) { throw new UnsafeTargetError( diff --git a/test/bench/fixture.ts b/test/bench/fixture.ts index 13f79d1b9..3f5be4d0b 100644 --- a/test/bench/fixture.ts +++ b/test/bench/fixture.ts @@ -4,6 +4,12 @@ import { MemoryKvStore, } from "@fedify/fedify"; import { Create, Endpoints, Person } from "@fedify/vocab"; +import { + createServer, + type IncomingMessage, + type ServerResponse, +} from "node:http"; +import type { AddressInfo } from "node:net"; /** A running local benchmark-mode target used by `fedify bench` tests. */ export interface BenchmarkTargetFixture { @@ -26,7 +32,7 @@ export interface BenchmarkTargetFixture { * verification, and inbox paths that `fedify bench` drives. * @returns The running fixture. */ -export function spawnBenchmarkTarget(): BenchmarkTargetFixture { +export async function spawnBenchmarkTarget(): Promise { const federation = createFederation({ kv: new MemoryKvStore(), benchmarkMode: true, @@ -60,31 +66,74 @@ export function spawnBenchmarkTarget(): BenchmarkTargetFixture { () => {}, ); let inboxUserAgent: string | null = null; - const abort = new AbortController(); - const server = Deno.serve( - { - port: 0, - hostname: "127.0.0.1", - signal: abort.signal, - onListen: () => {}, - }, - (request: Request) => { + const server = createServer( + async (incoming: IncomingMessage, outgoing: ServerResponse) => { + const request = await toFetchRequest(incoming); const url = new URL(request.url); requests.push({ method: request.method, path: url.pathname }); if (request.method === "POST") { inboxUserAgent = request.headers.get("user-agent"); } - return federation.fetch(request, { contextData: undefined }); + const response = await federation.fetch(request, { + contextData: undefined, + }); + await writeFetchResponse(response, outgoing); }, ); - const address = server.addr as Deno.NetAddr; + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address() as AddressInfo; return { - url: new URL(`http://${address.hostname}:${address.port}/`), + url: new URL(`http://${address.address}:${address.port}/`), inboxUserAgent: () => inboxUserAgent, requests: () => requests.slice(), - close: async () => { - abort.abort(); - await server.finished.catch(() => {}); - }, + close: () => new Promise((resolve) => server.close(() => resolve())), }; } + +async function toFetchRequest(incoming: IncomingMessage): Promise { + const host = incoming.headers.host ?? "127.0.0.1"; + const url = new URL(incoming.url ?? "/", `http://${host}`); + const headers = new Headers(); + for (const [name, value] of Object.entries(incoming.headers)) { + if (value == null) continue; + if (Array.isArray(value)) { + for (const item of value) headers.append(name, item); + } else { + headers.set(name, value); + } + } + const method = incoming.method ?? "GET"; + const init: RequestInit = { method, headers }; + if (method !== "GET" && method !== "HEAD") { + init.body = await readBody(incoming); + } + return new Request(url, init); +} + +async function readBody(incoming: IncomingMessage): Promise { + const chunks: Uint8Array[] = []; + for await (const chunk of incoming) { + chunks.push( + typeof chunk === "string" ? new TextEncoder().encode(chunk) : chunk, + ); + } + const length = chunks.reduce((total, chunk) => total + chunk.byteLength, 0); + const body = new Uint8Array(length); + let offset = 0; + for (const chunk of chunks) { + body.set(chunk, offset); + offset += chunk.byteLength; + } + return body.buffer; +} + +async function writeFetchResponse( + response: Response, + outgoing: ServerResponse, +): Promise { + outgoing.statusCode = response.status; + response.headers.forEach((value, name) => { + outgoing.setHeader(name, value); + }); + outgoing.end(new Uint8Array(await response.arrayBuffer())); +} From ef07fd05d0ea291f616c9b225d359ee60fe46d19 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 8 Jun 2026 00:09:04 +0900 Subject: [PATCH 05/12] Honor benchmark suite defaults Pass suite-level load and duration defaults into the unsafe public inbox destination check. Public inbox runs allowed with --allow-unsafe-target now follow the same documented explicit-default contract as the top-level target gate. https://github.com/fedify-dev/fedify/issues/744 https://github.com/fedify-dev/fedify/issues/784 Assisted-by: Codex:gpt-5.5 --- packages/cli/src/bench/action.test.ts | 46 +++++++++++++++++++++++++++ packages/cli/src/bench/action.ts | 4 ++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/bench/action.test.ts b/packages/cli/src/bench/action.test.ts index 76cb54576..55dd6303f 100644 --- a/packages/cli/src/bench/action.test.ts +++ b/packages/cli/src/bench/action.test.ts @@ -390,6 +390,52 @@ scenarios: } }); +test("runBench - unsafe public inbox destination honors suite defaults", async () => { + const target = await spawnBenchmarkTarget(); + try { + const file = await writeSuite(`version: 1 +target: ${target.url.href} +defaults: + duration: 1ms + load: { rate: 1/s } +scenarios: + - name: inbox-shared + type: inbox + recipient: "${new URL("/users/alice", target.url).href}" + inbox: "https://prod.example/inbox" +`); + let code = -1; + let message = ""; + await runBench( + command({ + scenario: file, + target: target.url.href, + allowUnsafeTarget: true, + advertiseHost: "127.0.0.1", + }), + { + exit: (c) => { + code = c; + }, + writeOutput: () => Promise.resolve(), + log: (m) => { + message = m; + }, + fetch: (input) => { + const url = new URL(input instanceof Request ? input.url : input); + if (url.hostname === "prod.example") { + return Promise.resolve(new Response("accepted", { status: 202 })); + } + return fetch(input); + }, + }, + ); + assert.strictEqual(code, 0, message); + } finally { + await target.close(); + } +}); + test("runBench - malformed expect assertion exits 2 before any load", async () => { // The expect typo must be caught in preflight, so the run exits 2 (a config // error) without ever probing the target or sending load. diff --git a/packages/cli/src/bench/action.ts b/packages/cli/src/bench/action.ts index 797b8bde3..472342330 100644 --- a/packages/cli/src/bench/action.ts +++ b/packages/cli/src/bench/action.ts @@ -181,6 +181,7 @@ export default async function runBench( targetBenchmarkMode: probe.benchmarkMode, allowUnsafe: command.allowUnsafeTarget, explicitCliTarget: command.target != null, + suite: validated, }); }; @@ -447,6 +448,7 @@ interface PublicDestinationOverrideContext { readonly targetBenchmarkMode: boolean; readonly allowUnsafe: boolean; readonly explicitCliTarget: boolean; + readonly suite: Suite; } function assertPublicDestinationOverrideAllowed( @@ -467,7 +469,7 @@ function assertPublicDestinationOverrideAllowed( benchmarkMode: false, allowUnsafe: true, explicitCliTarget: context.explicitCliTarget, - scenarios: [unsafeOverrideScenario(scenario)], + scenarios: [unsafeOverrideScenario(scenario, context.suite)], }); } From ea8fc6eb4a08544f58bc66428f6338fb1bdbbb7f Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 8 Jun 2026 00:37:20 +0900 Subject: [PATCH 06/12] Resolve inbox destinations for safety Classify off-origin inbox destinations with the same DNS-aware resolver used for benchmark targets before applying the inbox safety gate. This lets private staging shared inboxes run without the unsafe public override while keeping public destination override checks bounded. https://github.com/fedify-dev/fedify/issues/744 https://github.com/fedify-dev/fedify/issues/784 Assisted-by: Codex:gpt-5.5 --- packages/cli/src/bench/action.test.ts | 60 ++++++++++++++++++++++ packages/cli/src/bench/action.ts | 30 +++++++---- packages/cli/src/bench/safety/gate.test.ts | 13 +++++ packages/cli/src/bench/safety/gate.ts | 6 ++- packages/cli/src/bench/scenarios/inbox.ts | 2 +- packages/cli/src/bench/scenarios/runner.ts | 8 +-- 6 files changed, 101 insertions(+), 18 deletions(-) diff --git a/packages/cli/src/bench/action.test.ts b/packages/cli/src/bench/action.test.ts index 55dd6303f..82e7bd354 100644 --- a/packages/cli/src/bench/action.test.ts +++ b/packages/cli/src/bench/action.test.ts @@ -28,6 +28,10 @@ async function writeSuite(content: string): Promise { return path; } +function resolvePublicHost(_hostname: string): Promise { + return Promise.resolve(["93.184.216.34"]); +} + function inboxSuite(target: URL, expectLine: string): string { // Uses `${{ target.host }}` templating to form the actor URI (WebFinger is // https-only, so an acct: handle would not resolve over http loopback). @@ -246,6 +250,7 @@ scenarios: { headers: { "content-type": "application/json" } }, ), ), + resolveTargetAddresses: resolvePublicHost, }); assert.strictEqual(code, 2); assert.match(message, /advertise-host/); @@ -306,6 +311,7 @@ scenarios: log: (m) => { message = m; }, + resolveTargetAddresses: resolvePublicHost, }); assert.strictEqual(code, 2); assert.match(message, /public inbox|allow-unsafe-target/); @@ -314,6 +320,57 @@ scenarios: } }); +test("runBench - allows a DNS-resolved private inbox off the target", async () => { + const target = await spawnBenchmarkTarget(); + try { + const file = await writeSuite(`version: 1 +target: ${target.url.href} +scenarios: + - name: inbox-shared + type: inbox + recipient: "${new URL("/users/alice", target.url).href}" + inbox: "https://shared.staging.example/inbox" + load: { concurrency: 2 } + duration: 250ms +`); + let code = -1; + let message = ""; + const resolved: string[] = []; + await runBench( + command({ + scenario: file, + advertiseHost: "127.0.0.1", + }), + { + exit: (c) => { + code = c; + }, + writeOutput: () => Promise.resolve(), + log: (m) => { + message = m; + }, + resolveTargetAddresses: (hostname) => { + resolved.push(hostname); + return Promise.resolve( + hostname === "shared.staging.example" ? ["10.0.0.8"] : [], + ); + }, + fetch: (input) => { + const url = new URL(input instanceof Request ? input.url : input); + if (url.hostname === "shared.staging.example") { + return Promise.resolve(new Response("accepted", { status: 202 })); + } + return fetch(input); + }, + }, + ); + assert.strictEqual(code, 0, message); + assert.deepStrictEqual(resolved, ["shared.staging.example"]); + } finally { + await target.close(); + } +}); + test("runBench - unsafe public inbox destination needs an explicit CLI target", async () => { const target = await spawnBenchmarkTarget(); try { @@ -343,6 +400,7 @@ scenarios: log: (m) => { message = m; }, + resolveTargetAddresses: resolvePublicHost, }, ); assert.strictEqual(code, 2); @@ -381,6 +439,7 @@ scenarios: log: (m) => { message = m; }, + resolveTargetAddresses: resolvePublicHost, }, ); assert.strictEqual(code, 2); @@ -421,6 +480,7 @@ scenarios: log: (m) => { message = m; }, + resolveTargetAddresses: resolvePublicHost, fetch: (input) => { const url = new URL(input instanceof Request ? input.url : input); if (url.hostname === "prod.example") { diff --git a/packages/cli/src/bench/action.ts b/packages/cli/src/bench/action.ts index 472342330..4223963a8 100644 --- a/packages/cli/src/bench/action.ts +++ b/packages/cli/src/bench/action.ts @@ -30,8 +30,8 @@ import { } from "./safety/gate.ts"; import { classifyResolvedTarget, - classifyTarget, type ResolveTargetAddresses, + type TargetTier, } from "./safety/tiers.ts"; import { runnerFor } from "./scenarios/registry.ts"; import { @@ -165,13 +165,17 @@ export default async function runBench( // Gates each resolved inbox destination (which can differ from the suite // target) before the runner sends load to it. - const assertDestinationAllowed = ( + const assertDestinationAllowed = async ( url: URL, scenario: ResolvedScenario, - ): void => { + ): Promise => { + const destinationTier = url.origin === suite.target.origin + ? tier + : await classifyResolvedTarget(url, deps.resolveTargetAddresses); assertInboxDestinationAllowed(url, { targetOrigin: suite.target.origin, targetTier: tier, + destinationTier, targetBenchmarkMode: probe.benchmarkMode, allowUnsafe: command.allowUnsafeTarget, advertised: command.advertiseHost != null, @@ -181,6 +185,7 @@ export default async function runBench( targetBenchmarkMode: probe.benchmarkMode, allowUnsafe: command.allowUnsafeTarget, explicitCliTarget: command.target != null, + destinationTier, suite: validated, }); }; @@ -338,7 +343,7 @@ interface DryRunPlanContext { readonly assertDestinationAllowed: ( url: URL, scenario: ResolvedScenario, - ) => void; + ) => Promise; } async function renderPlan( @@ -404,9 +409,11 @@ async function describeInboxDiscoveryPlan( `inbox ${inbox.href}`, ); lines.push( - ` destination safety: ${ - describeDestinationSafety(inbox, scenario, context) - }`, + ` destination safety: ${await describeDestinationSafety( + inbox, + scenario, + context, + )}`, ); } return lines; @@ -427,13 +434,13 @@ function describeWebFingerPlan( }); } -function describeDestinationSafety( +async function describeDestinationSafety( inbox: URL, scenario: ResolvedScenario, context: DryRunPlanContext, -): string { +): Promise { try { - context.assertDestinationAllowed(inbox, scenario); + await context.assertDestinationAllowed(inbox, scenario); return "allowed"; } catch (error) { if (error instanceof UnsafeTargetError) { @@ -448,6 +455,7 @@ interface PublicDestinationOverrideContext { readonly targetBenchmarkMode: boolean; readonly allowUnsafe: boolean; readonly explicitCliTarget: boolean; + readonly destinationTier: TargetTier; readonly suite: Suite; } @@ -459,7 +467,7 @@ function assertPublicDestinationOverrideAllowed( const inheritsTargetGate = url.origin === context.targetOrigin && context.targetBenchmarkMode; if ( - classifyTarget(url) !== "public" || inheritsTargetGate || + context.destinationTier !== "public" || inheritsTargetGate || !context.allowUnsafe ) { return; diff --git a/packages/cli/src/bench/safety/gate.test.ts b/packages/cli/src/bench/safety/gate.test.ts index 0a0e50dea..ea496dbe4 100644 --- a/packages/cli/src/bench/safety/gate.test.ts +++ b/packages/cli/src/bench/safety/gate.test.ts @@ -147,6 +147,7 @@ function destContext( return { targetOrigin: "http://127.0.0.1:3000", targetTier: "loopback" as const, + destinationTier: "public" as const, targetBenchmarkMode: false, allowUnsafe: false, advertised: false, @@ -216,6 +217,18 @@ test("assertInboxDestinationAllowed - same-origin inbox uses the resolved target ); }); +test("assertInboxDestinationAllowed - off-origin inbox uses destination tier", () => { + assert.doesNotThrow(() => + assertInboxDestinationAllowed( + new URL("https://shared.staging.example/inbox"), + destContext({ + destinationTier: "private", + advertised: true, + }), + ) + ); +}); + test("assertInboxDestinationAllowed - same host, different scheme does not inherit", () => { // The target is https (its benchmark-mode probe covered port 443); an http // inbox on the same hostname is a different service (port 80), so it must not diff --git a/packages/cli/src/bench/safety/gate.ts b/packages/cli/src/bench/safety/gate.ts index be79f92bc..11c87ab8a 100644 --- a/packages/cli/src/bench/safety/gate.ts +++ b/packages/cli/src/bench/safety/gate.ts @@ -11,7 +11,7 @@ * @module */ -import { classifyTarget, type TargetTier } from "./tiers.ts"; +import type { TargetTier } from "./tiers.ts"; /** An error raised when a target is refused by the safety gate. */ export class UnsafeTargetError extends Error {} @@ -124,6 +124,8 @@ export interface InboxDestinationGateContext { readonly targetOrigin: string; /** The resolved target tier used by the main safety gate. */ readonly targetTier: TargetTier; + /** The resolved tier for this inbox destination. */ + readonly destinationTier: TargetTier; /** Whether the gated target advertises benchmark mode. */ readonly targetBenchmarkMode: boolean; /** Whether `--allow-unsafe-target` was given. */ @@ -153,7 +155,7 @@ export function assertInboxDestinationAllowed( context: InboxDestinationGateContext, ): void { const sameOrigin = url.origin === context.targetOrigin; - const tier = sameOrigin ? context.targetTier : classifyTarget(url); + const tier = sameOrigin ? context.targetTier : context.destinationTier; const inheritsTargetGate = sameOrigin && context.targetBenchmarkMode; if (tier === "public" && !inheritsTargetGate && !context.allowUnsafe) { diff --git a/packages/cli/src/bench/scenarios/inbox.ts b/packages/cli/src/bench/scenarios/inbox.ts index 9feb62681..e23d88b78 100644 --- a/packages/cli/src/bench/scenarios/inbox.ts +++ b/packages/cli/src/bench/scenarios/inbox.ts @@ -81,7 +81,7 @@ export const inboxRunner: ScenarioRunner = { const inbox = selectInbox(discovered, scenario.inbox); // Gate the actual load destination before sending anything to it: it can // differ from the gated target (a public recipient, or an explicit inbox). - context.assertDestinationAllowed?.(inbox); + await context.assertDestinationAllowed?.(inbox); targets.push({ inbox, actorUri: discovered.actorUri }); } diff --git a/packages/cli/src/bench/scenarios/runner.ts b/packages/cli/src/bench/scenarios/runner.ts index 8a87f6a70..9770de794 100644 --- a/packages/cli/src/bench/scenarios/runner.ts +++ b/packages/cli/src/bench/scenarios/runner.ts @@ -31,11 +31,11 @@ export interface RunContext { readonly fetch?: typeof fetch; /** * Gates a resolved load destination (a discovered or explicit inbox URL) - * before any load is sent to it, throwing if it is not allowed. The suite - * `target` is gated by the orchestrator; this covers destinations that differ - * from it. Optional so direct runner tests need not supply it. + * before any load is sent to it, throwing or rejecting if it is not allowed. + * The suite `target` is gated by the orchestrator; this covers destinations + * that differ from it. Optional so direct runner tests need not supply it. */ - readonly assertDestinationAllowed?: (url: URL) => void; + readonly assertDestinationAllowed?: (url: URL) => void | Promise; } /** A runner for one scenario type. */ From bfd3d6c3d99a1a74da93859d6bfaa3c198be04c1 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 8 Jun 2026 13:37:16 +0900 Subject: [PATCH 07/12] Treat bad resolver output as public Malformed resolver output should not escape the benchmark safety classifier. Fall back to the public tier so the gate stays conservative when a custom resolver returns an address string that cannot be parsed as a URL host. https://github.com/fedify-dev/fedify/pull/795#discussion_r3370799418 Assisted-by: Codex:gpt-5.5 --- packages/cli/src/bench/safety/tiers.test.ts | 8 ++++++++ packages/cli/src/bench/safety/tiers.ts | 7 ++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/bench/safety/tiers.test.ts b/packages/cli/src/bench/safety/tiers.test.ts index 36fddc747..1504d86a5 100644 --- a/packages/cli/src/bench/safety/tiers.test.ts +++ b/packages/cli/src/bench/safety/tiers.test.ts @@ -102,3 +102,11 @@ test("classifyResolvedTarget - treats resolution failure as public", async () => ); assert.strictEqual(tier, "public"); }); + +test("classifyResolvedTarget - treats malformed resolver output as public", async () => { + const tier = await classifyResolvedTarget( + new URL("https://bench.example"), + () => Promise.resolve(["2001:db8:::1"]), + ); + assert.strictEqual(tier, "public"); +}); diff --git a/packages/cli/src/bench/safety/tiers.ts b/packages/cli/src/bench/safety/tiers.ts index d5d410eb6..b42fd4604 100644 --- a/packages/cli/src/bench/safety/tiers.ts +++ b/packages/cli/src/bench/safety/tiers.ts @@ -65,7 +65,12 @@ export async function classifyResolvedTarget( if (addresses.length < 1) return "public"; let aggregate: TargetTier = "loopback"; for (const address of addresses) { - const tier = classifyTarget(new URL(`http://${hostForAddress(address)}/`)); + let tier: TargetTier; + try { + tier = classifyTarget(new URL(`http://${hostForAddress(address)}/`)); + } catch { + return "public"; + } if (tier === "public") return "public"; if (tier === "private") aggregate = "private"; } From a685876f762ca7ad0a818709606b958a76670cfe Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 8 Jun 2026 13:39:33 +0900 Subject: [PATCH 08/12] Harden the benchmark test fixture Make fixture server startup and shutdown reject on errors instead of hanging or resolving unconditionally. Use the Node Buffer path when bridging fetch responses back to the HTTP server. https://github.com/fedify-dev/fedify/pull/795#discussion_r3370826989 https://github.com/fedify-dev/fedify/pull/795#discussion_r3370827022 https://github.com/fedify-dev/fedify/pull/795#discussion_r3370827063 Assisted-by: Codex:gpt-5.5 --- test/bench/fixture.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/test/bench/fixture.ts b/test/bench/fixture.ts index 3f5be4d0b..f9927b582 100644 --- a/test/bench/fixture.ts +++ b/test/bench/fixture.ts @@ -4,6 +4,7 @@ import { MemoryKvStore, } from "@fedify/fedify"; import { Create, Endpoints, Person } from "@fedify/vocab"; +import { Buffer } from "node:buffer"; import { createServer, type IncomingMessage, @@ -80,13 +81,22 @@ export async function spawnBenchmarkTarget(): Promise { await writeFetchResponse(response, outgoing); }, ); - await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", () => { + server.off("error", reject); + resolve(); + }); + }); const address = server.address() as AddressInfo; return { url: new URL(`http://${address.address}:${address.port}/`), inboxUserAgent: () => inboxUserAgent, requests: () => requests.slice(), - close: () => new Promise((resolve) => server.close(() => resolve())), + close: () => + new Promise((resolve, reject) => { + server.close((error) => error == null ? resolve() : reject(error)); + }), }; } @@ -135,5 +145,5 @@ async function writeFetchResponse( response.headers.forEach((value, name) => { outgoing.setHeader(name, value); }); - outgoing.end(new Uint8Array(await response.arrayBuffer())); + outgoing.end(Buffer.from(await response.arrayBuffer())); } From 40828e90a3b358e4d52006f5fd9baf7c8d3ad9c0 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 8 Jun 2026 14:06:44 +0900 Subject: [PATCH 09/12] Keep the bench fixture in the CLI package Move the benchmark target fixture under the CLI package so Node resolves its workspace dependencies from the package that declares them. This keeps the repo-root Node test command from resolving @fedify/fedify and @fedify/vocab through the root package scope. https://github.com/fedify-dev/fedify/pull/795#discussion_r3370883571 Assisted-by: Codex:gpt-5.5 --- packages/cli/src/bench/action.test.ts | 2 +- {test => packages/cli/test}/bench/fixture.ts | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename {test => packages/cli/test}/bench/fixture.ts (100%) diff --git a/packages/cli/src/bench/action.test.ts b/packages/cli/src/bench/action.test.ts index 82e7bd354..7636b6dc9 100644 --- a/packages/cli/src/bench/action.test.ts +++ b/packages/cli/src/bench/action.test.ts @@ -3,7 +3,7 @@ import { mkdtemp, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import test from "node:test"; -import { spawnBenchmarkTarget } from "../../../../test/bench/fixture.ts"; +import { spawnBenchmarkTarget } from "../../test/bench/fixture.ts"; import runBench, { withUserAgent } from "./action.ts"; import type { BenchCommand } from "./command.ts"; diff --git a/test/bench/fixture.ts b/packages/cli/test/bench/fixture.ts similarity index 100% rename from test/bench/fixture.ts rename to packages/cli/test/bench/fixture.ts From a7149a9060bf4ee1d36e6ea628d5f52a6f307414 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 8 Jun 2026 14:20:38 +0900 Subject: [PATCH 10/12] Harden benchmark safety helpers Treat malformed resolver return values as public before reading or iterating the result. Also guard the explicit-load helper against non-object load values before using property checks. https://github.com/fedify-dev/fedify/pull/795#discussion_r3370944044 https://github.com/fedify-dev/fedify/pull/795#discussion_r3370944052 Assisted-by: Codex:gpt-5.5 --- packages/cli/src/bench/action.ts | 1 + packages/cli/src/bench/safety/tiers.test.ts | 8 ++++++++ packages/cli/src/bench/safety/tiers.ts | 3 ++- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/bench/action.ts b/packages/cli/src/bench/action.ts index 4223963a8..98dee60f1 100644 --- a/packages/cli/src/bench/action.ts +++ b/packages/cli/src/bench/action.ts @@ -505,6 +505,7 @@ function unsafeOverrideScenario( function hasExplicitLoad(load: LoadConfig | undefined): boolean { return load != null && + typeof load === "object" && (("rate" in load && load.rate != null) || ("concurrency" in load && load.concurrency != null)); } diff --git a/packages/cli/src/bench/safety/tiers.test.ts b/packages/cli/src/bench/safety/tiers.test.ts index 1504d86a5..904b62989 100644 --- a/packages/cli/src/bench/safety/tiers.test.ts +++ b/packages/cli/src/bench/safety/tiers.test.ts @@ -103,6 +103,14 @@ test("classifyResolvedTarget - treats resolution failure as public", async () => assert.strictEqual(tier, "public"); }); +test("classifyResolvedTarget - treats non-array resolver output as public", async () => { + const tier = await classifyResolvedTarget( + new URL("https://bench.example"), + () => Promise.resolve(null as unknown as readonly string[]), + ); + assert.strictEqual(tier, "public"); +}); + test("classifyResolvedTarget - treats malformed resolver output as public", async () => { const tier = await classifyResolvedTarget( new URL("https://bench.example"), diff --git a/packages/cli/src/bench/safety/tiers.ts b/packages/cli/src/bench/safety/tiers.ts index b42fd4604..fadbaf749 100644 --- a/packages/cli/src/bench/safety/tiers.ts +++ b/packages/cli/src/bench/safety/tiers.ts @@ -58,7 +58,8 @@ export async function classifyResolvedTarget( if (direct !== "public" || isIpLiteral(host)) return direct; let addresses: readonly string[]; try { - addresses = await resolveAddresses(host); + const resolved = await resolveAddresses(host); + addresses = Array.isArray(resolved) ? resolved : []; } catch { return "public"; } From 2bfa70d8956f0da548620da50503f67a0bde68f4 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 8 Jun 2026 14:39:15 +0900 Subject: [PATCH 11/12] Keep dry-run discovery resilient Report inbox discovery failures per recipient during dry runs so one bad actor lookup does not hide the rest of the benchmark plan. https://github.com/fedify-dev/fedify/pull/795#discussion_r3370984755 Assisted-by: Codex:gpt-5.5 --- packages/cli/src/bench/action.test.ts | 39 +++++++++++++++++++++++++++ packages/cli/src/bench/action.ts | 28 ++++++++++++++----- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/bench/action.test.ts b/packages/cli/src/bench/action.test.ts index 7636b6dc9..3c87a6396 100644 --- a/packages/cli/src/bench/action.test.ts +++ b/packages/cli/src/bench/action.test.ts @@ -169,6 +169,45 @@ test("runBench - dry run prints a plan and sends nothing", async () => { } }); +test("runBench - dry run reports inbox discovery failures and continues", async () => { + const target = await spawnBenchmarkTarget(); + try { + const file = await writeSuite(`version: 1 +target: ${target.url.href} +scenarios: + - name: inbox-shared + type: inbox + recipient: + - "${new URL("/users/missing", target.url).href}" + - "${new URL("/users/alice", target.url).href}" + inbox: shared + load: { concurrency: 2 } + duration: 250ms +`); + let code = -1; + let output = ""; + await runBench(command({ scenario: file, dryRun: true }), { + exit: (c) => { + code = c; + }, + writeOutput: (c) => { + output = c; + return Promise.resolve(); + }, + log: () => {}, + }); + assert.strictEqual(code, 0); + assert.match(output, /\/users\/missing/); + assert.match(output, /discovery failed/); + assert.match(output, /\/users\/alice/); + assert.match(output, /\/inbox/); + assert.match(output, /No benchmark load was sent/); + assert.ok(!target.requests().some((r) => r.method === "POST")); + } finally { + await target.close(); + } +}); + test("runBench - unsafe override requires an explicit CLI target", async () => { const file = await writeSuite(`version: 1 target: https://example.com diff --git a/packages/cli/src/bench/action.ts b/packages/cli/src/bench/action.ts index 98dee60f1..b27e69bfa 100644 --- a/packages/cli/src/bench/action.ts +++ b/packages/cli/src/bench/action.ts @@ -4,7 +4,11 @@ import process from "node:process"; import { getContextLoader, getDocumentLoader } from "../docloader.ts"; import { buildFleet } from "./actor/fleet.ts"; import type { BenchCommand } from "./command.ts"; -import { discoverInbox, selectInbox } from "./discovery/discover.ts"; +import { + type DiscoveredInbox, + discoverInbox, + selectInbox, +} from "./discovery/discover.ts"; import { buildReport, buildScenarioResult, @@ -398,11 +402,19 @@ async function describeInboxDiscoveryPlan( ): Promise { const lines: string[] = []; for (const recipient of scenario.recipients) { - const discovered = await discoverInbox(recipient, { - documentLoader: context.documentLoader, - contextLoader: context.contextLoader, - allowPrivateAddress: context.allowPrivateAddress, - }); + let discovered: DiscoveredInbox; + try { + discovered = await discoverInbox(recipient, { + documentLoader: context.documentLoader, + contextLoader: context.contextLoader, + allowPrivateAddress: context.allowPrivateAddress, + }); + } catch (error) { + lines.push( + ` recipient ${recipient}: discovery failed (${describeError(error)})`, + ); + continue; + } const inbox = selectInbox(discovered, scenario.inbox); lines.push( ` recipient ${recipient}: actor ${discovered.actorUri.href}, ` + @@ -419,6 +431,10 @@ async function describeInboxDiscoveryPlan( return lines; } +function describeError(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + function describeWebFingerPlan( scenario: ResolvedScenario, target: URL, From 0f8ba888fafc74d20a2d49747b54fb3c78dbf1c8 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Mon, 8 Jun 2026 23:21:51 +0900 Subject: [PATCH 12/12] Simplify benchmark review helpers Use stable fixture key pairs without a mutable cache, share error message formatting through the CLI utilities, pass suite defaults to unsafe override planning directly, and fold DNS tier fallback handling into a single conservative catch path. https://github.com/fedify-dev/fedify/pull/795#discussion_r3371607908 https://github.com/fedify-dev/fedify/pull/795#discussion_r3371874631 https://github.com/fedify-dev/fedify/pull/795#discussion_r3371930708 https://github.com/fedify-dev/fedify/pull/795#discussion_r3372536421 Assisted-by: Codex:gpt-5.5 --- packages/cli/src/bench/action.ts | 27 ++++++++++++-------------- packages/cli/src/bench/safety/tiers.ts | 25 ++++++++++-------------- packages/cli/src/lookup.ts | 6 +++--- packages/cli/src/runner.ts | 3 ++- packages/cli/src/utils.ts | 4 ++++ packages/cli/test/bench/fixture.ts | 11 +++++------ 6 files changed, 36 insertions(+), 40 deletions(-) diff --git a/packages/cli/src/bench/action.ts b/packages/cli/src/bench/action.ts index b27e69bfa..00798f2d5 100644 --- a/packages/cli/src/bench/action.ts +++ b/packages/cli/src/bench/action.ts @@ -2,6 +2,7 @@ import { writeFile } from "node:fs/promises"; import type { DocumentLoader } from "@fedify/vocab-runtime"; import process from "node:process"; import { getContextLoader, getDocumentLoader } from "../docloader.ts"; +import { describeError } from "../utils.ts"; import { buildFleet } from "./actor/fleet.ts"; import type { BenchCommand } from "./command.ts"; import { @@ -24,7 +25,7 @@ import { type ResolvedScenario, type ResolvedSuite, } from "./scenario/normalize.ts"; -import type { LoadConfig, Suite } from "./scenario/types.ts"; +import type { LoadConfig, Suite, SuiteDefaults } from "./scenario/types.ts"; import { validateSuite } from "./scenario/validate.ts"; import { assertInboxDestinationAllowed, @@ -100,7 +101,7 @@ export default async function runBench( validated = validateSuite(rendered, command.scenario); suite = normalizeSuite(validated, { target: command.target }); } catch (error) { - log(error instanceof Error ? error.message : String(error)); + log(describeError(error)); return void exit(2); } @@ -119,7 +120,7 @@ export default async function runBench( resolveAdvertiseHost(command.advertiseHost); } } catch (error) { - log(error instanceof Error ? error.message : String(error)); + log(describeError(error)); return void exit(2); } @@ -190,7 +191,7 @@ export default async function runBench( allowUnsafe: command.allowUnsafeTarget, explicitCliTarget: command.target != null, destinationTier, - suite: validated, + defaults: validated.defaults, }); }; @@ -207,7 +208,7 @@ export default async function runBench( ); return void exit(0); } catch (error) { - log(error instanceof Error ? error.message : String(error)); + log(describeError(error)); return void exit(2); } } @@ -431,10 +432,6 @@ async function describeInboxDiscoveryPlan( return lines; } -function describeError(error: unknown): string { - return error instanceof Error ? error.message : String(error); -} - function describeWebFingerPlan( scenario: ResolvedScenario, target: URL, @@ -472,7 +469,7 @@ interface PublicDestinationOverrideContext { readonly allowUnsafe: boolean; readonly explicitCliTarget: boolean; readonly destinationTier: TargetTier; - readonly suite: Suite; + readonly defaults?: SuiteDefaults; } function assertPublicDestinationOverrideAllowed( @@ -493,7 +490,7 @@ function assertPublicDestinationOverrideAllowed( benchmarkMode: false, allowUnsafe: true, explicitCliTarget: context.explicitCliTarget, - scenarios: [unsafeOverrideScenario(scenario, context.suite)], + scenarios: [unsafeOverrideScenario(scenario, context.defaults)], }); } @@ -501,16 +498,16 @@ function unsafeOverrideScenarios( suite: Suite, ): Parameters[0]["scenarios"] { return suite.scenarios.map((scenario) => - unsafeOverrideScenario(scenario, suite) + unsafeOverrideScenario(scenario, suite.defaults) ); } function unsafeOverrideScenario( scenario: ResolvedScenario | Suite["scenarios"][number], - suite?: Suite, + defaults?: SuiteDefaults, ): Parameters[0]["scenarios"][number] { - const defaultDuration = suite?.defaults?.duration != null; - const defaultLoad = hasExplicitLoad(suite?.defaults?.load); + const defaultDuration = defaults?.duration != null; + const defaultLoad = hasExplicitLoad(defaults?.load); const raw = "raw" in scenario ? scenario.raw : scenario; return { name: scenario.name, diff --git a/packages/cli/src/bench/safety/tiers.ts b/packages/cli/src/bench/safety/tiers.ts index fadbaf749..6b8fad24d 100644 --- a/packages/cli/src/bench/safety/tiers.ts +++ b/packages/cli/src/bench/safety/tiers.ts @@ -56,26 +56,21 @@ export async function classifyResolvedTarget( const host = normalizedHost(target); const direct = classifyTarget(target); if (direct !== "public" || isIpLiteral(host)) return direct; - let addresses: readonly string[]; try { const resolved = await resolveAddresses(host); - addresses = Array.isArray(resolved) ? resolved : []; + if (!Array.isArray(resolved) || resolved.length < 1) return "public"; + let aggregate: TargetTier = "loopback"; + for (const address of resolved) { + const tier = classifyTarget( + new URL(`http://${hostForAddress(address)}/`), + ); + if (tier === "public") return "public"; + if (tier === "private") aggregate = "private"; + } + return aggregate; } catch { return "public"; } - if (addresses.length < 1) return "public"; - let aggregate: TargetTier = "loopback"; - for (const address of addresses) { - let tier: TargetTier; - try { - tier = classifyTarget(new URL(`http://${hostForAddress(address)}/`)); - } catch { - return "public"; - } - if (tier === "public") return "public"; - if (tier === "private") aggregate = "private"; - } - return aggregate; } /** Resolves a hostname with the platform DNS resolver. */ diff --git a/packages/cli/src/lookup.ts b/packages/cli/src/lookup.ts index 2ebbb455d..48e1125c9 100644 --- a/packages/cli/src/lookup.ts +++ b/packages/cli/src/lookup.ts @@ -59,7 +59,7 @@ import { userAgentOption, } from "./options.ts"; import { spawnTemporaryServer, type TemporaryServer } from "./tempserver.ts"; -import { colorEnabled, colors, formatObject } from "./utils.ts"; +import { colorEnabled, colors, describeError, formatObject } from "./utils.ts"; const logger = getLogger(["fedify", "cli", "lookup"]); @@ -538,7 +538,7 @@ function handleTimeoutError( } function isPrivateAddressError(error: unknown): boolean { - const errorMessage = error instanceof Error ? error.message : String(error); + const errorMessage = describeError(error); const lowerMessage = errorMessage.toLowerCase(); if (error instanceof UrlError) { return ( @@ -600,7 +600,7 @@ function getPrivateContextUrl(error: unknown): URL | null { // a trailing `URL: "..."` segment all at once. If jsonld changes those // details, this helper and the related lookup tests need to be updated // together. - const errorMessage = error instanceof Error ? error.message : String(error); + const errorMessage = describeError(error); if ( !(error instanceof Error) || error.name !== "jsonld.InvalidUrl" || diff --git a/packages/cli/src/runner.ts b/packages/cli/src/runner.ts index db0cc0eaa..3601f7646 100644 --- a/packages/cli/src/runner.ts +++ b/packages/cli/src/runner.ts @@ -16,6 +16,7 @@ import { nodeInfoCommand } from "./nodeinfo.ts"; import { globalOptions } from "./options.ts"; import { relayCommand } from "./relay/command.ts"; import { tunnelCommand } from "./tunnel.ts"; +import { describeError } from "./utils.ts"; import { webFingerCommand } from "./webfinger/mod.ts"; import metadata from "../deno.json" with { type: "json" }; @@ -105,7 +106,7 @@ export function loadConfig( } catch (error) { printError( message`Could not load config file at ${parsed.configPath}: ${ - error instanceof Error ? error.message : String(error) + describeError(error) }`, ); process.exit(1); diff --git a/packages/cli/src/utils.ts b/packages/cli/src/utils.ts index fcb53068d..1cd6dc3f9 100644 --- a/packages/cli/src/utils.ts +++ b/packages/cli/src/utils.ts @@ -81,6 +81,10 @@ export const isNotFoundError = (e: unknown): e is { code: "ENOENT" } => "code" in e && e.code === "ENOENT"; +export function describeError(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + export class CommandError extends Error { public commandLine: string; constructor( diff --git a/packages/cli/test/bench/fixture.ts b/packages/cli/test/bench/fixture.ts index f9927b582..9c7d5a36f 100644 --- a/packages/cli/test/bench/fixture.ts +++ b/packages/cli/test/bench/fixture.ts @@ -38,7 +38,10 @@ export async function spawnBenchmarkTarget(): Promise { kv: new MemoryKvStore(), benchmarkMode: true, }); - let keyPairs: CryptoKeyPair[] | undefined; + const keyPairs = Promise.all([ + generateCryptoKeyPair("RSASSA-PKCS1-v1_5"), + generateCryptoKeyPair("Ed25519"), + ]); const requests: { method: string; path: string }[] = []; federation .setActorDispatcher("/users/{identifier}", async (ctx, identifier) => { @@ -56,11 +59,7 @@ export async function spawnBenchmarkTarget(): Promise { .mapHandle((_ctx, username) => (username === "alice" ? "alice" : null)) .setKeyPairsDispatcher(async (_ctx, identifier) => { if (identifier !== "alice") return []; - keyPairs ??= [ - await generateCryptoKeyPair("RSASSA-PKCS1-v1_5"), - await generateCryptoKeyPair("Ed25519"), - ]; - return keyPairs; + return await keyPairs; }); federation.setInboxListeners("/users/{identifier}/inbox", "/inbox").on( Create,