diff --git a/.changeset/tidy-inspection-errors.md b/.changeset/tidy-inspection-errors.md new file mode 100644 index 0000000..75407dd --- /dev/null +++ b/.changeset/tidy-inspection-errors.md @@ -0,0 +1,5 @@ +--- +"@offering-protocol/agent": patch +--- + +Report connection failures during Service inspection as transport failures rather than blocked destinations, while preserving destination-policy rejections and cancellation causes. diff --git a/packages/agent/src/inspection.ts b/packages/agent/src/inspection.ts index 5b4ce87..2b60d77 100644 --- a/packages/agent/src/inspection.ts +++ b/packages/agent/src/inspection.ts @@ -11,7 +11,7 @@ import { } from "@offering-protocol/core"; import type { OdpCache, OdpCacheRecord } from "./cache.js"; -import { createDefaultTransport } from "./network.js"; +import { createDefaultTransport, DestinationPolicyError } from "./network.js"; import type { OdpTransport } from "./transport.js"; const ODP_MEDIA_TYPE = "application/odp+json"; @@ -309,29 +309,26 @@ async function fetchWithRedirects( } } catch (error) { if (error instanceof OdpInspectionError) throw error; - if (isAbortError(error)) + if (isAbortError(error) || options.signal?.aborted === true) throw new OdpInspectionError( "ODP Service Document request was aborted.", "aborted", undefined, error ); - // A destination-policy rejection is not the same as a connection failure, and the reason has to - // survive: the transport is where the SSRF and transport-security guards live. - if (error instanceof TypeError || error instanceof RangeError) + const rejection = + error instanceof DestinationPolicyError + ? error + : error instanceof Error && error.cause instanceof DestinationPolicyError + ? error.cause + : undefined; + if (rejection !== undefined) throw new OdpInspectionError( - `ODP Service Document request was rejected: ${error.message}`, + `ODP Service Document request was rejected: ${rejection.message}`, "blocked_destination", undefined, error ); - if (options.signal?.aborted === true) - throw new OdpInspectionError( - "ODP Service Document request was aborted.", - "aborted", - undefined, - error - ); throw new OdpInspectionError( "ODP Service Document could not be fetched.", "http_error", diff --git a/packages/agent/src/network.ts b/packages/agent/src/network.ts index 792b701..bbafca0 100644 --- a/packages/agent/src/network.ts +++ b/packages/agent/src/network.ts @@ -17,15 +17,20 @@ const MAX_DECODED_BYTES = 1_048_576; /** Statuses that RFC 9110 defines as carrying no content, which `Response` refuses to pair with a body. */ const NULL_BODY_STATUSES = new Set([101, 103, 204, 205, 304]); +/** @internal */ +export class DestinationPolicyError extends TypeError {} + export function createDefaultTransport(allowLocalNetwork = false): OdpTransport { return async (input, init) => { const url = new URL(String(input)); if (url.username !== "" || url.password !== "") - throw new TypeError("ODP request URL must not contain credentials"); + throw new DestinationPolicyError("ODP request URL must not contain credentials"); const hostname = url.hostname.startsWith("[") ? url.hostname.slice(1, -1) : url.hostname; const local = isLocalDevelopmentHost(hostname); if (url.protocol !== "https:" && !(url.protocol === "http:" && local && allowLocalNetwork)) - throw new TypeError("ODP requests require HTTPS unless local development is enabled"); + throw new DestinationPolicyError( + "ODP requests require HTTPS unless local development is enabled" + ); const records = await resolvePublicAddresses(hostname, local && allowLocalNetwork); const address = records[0]; if (address === undefined) throw new TypeError("ODP request host did not resolve"); @@ -36,7 +41,7 @@ export function createDefaultTransport(allowLocalNetwork = false): OdpTransport // redirect or connection reuse cannot borrow the validated addresses (SEC-10, SEC-11). const requested = requestedHost.startsWith("[") ? requestedHost.slice(1, -1) : requestedHost; if (requested !== hostname) { - callback(new TypeError("ODP request connected to an unvalidated host"), "", 0); + callback(new DestinationPolicyError("ODP request connected to an unvalidated host"), "", 0); return; } if (options.all === true) { @@ -91,10 +96,12 @@ async function resolvePublicAddresses( const range = ipaddr.process(record.address).range(); if (localDevelopment) { if (range !== "loopback") - throw new TypeError("ODP local-development host resolved outside the loopback network"); + throw new DestinationPolicyError( + "ODP local-development host resolved outside the loopback network" + ); } else if (range !== "unicast") { // Rejects the whole target rather than selecting a passing record (SEC-09). - throw new TypeError("ODP request host resolved to a non-public address"); + throw new DestinationPolicyError("ODP request host resolved to a non-public address"); } } return records; diff --git a/packages/agent/test/unit/inspection-cache.test.ts b/packages/agent/test/unit/inspection-cache.test.ts index 164df02..5940479 100644 --- a/packages/agent/test/unit/inspection-cache.test.ts +++ b/packages/agent/test/unit/inspection-cache.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import { createInMemoryOdpCache, type OdpCache, type OdpCacheRecord } from "../../src/cache.js"; import { inspectService, OdpInspectionError } from "../../src/inspection.js"; +import { DestinationPolicyError } from "../../src/network.js"; import type { OdpTransport } from "../../src/transport.js"; const document = { @@ -172,15 +173,43 @@ describe("Service Document caching", () => { describe("Service Document failures", () => { it("reports a destination-policy rejection distinctly and keeps its cause", async () => { const transport: OdpTransport = vi.fn(() => - Promise.reject(new TypeError("ODP request host resolved to a non-public address")) + Promise.reject( + new DestinationPolicyError("ODP request host resolved to a non-public address") + ) ); const failure = await failureOf(inspect(transport)); - // Flattening every transport failure to `http_error` hid exactly the errors worth seeing. expect(failure.code).toBe("blocked_destination"); expect(failure.message).toContain("non-public address"); expect(failure.cause).toBeInstanceOf(TypeError); }); + it("recognizes a destination rejection wrapped by fetch", async () => { + const cause = new DestinationPolicyError("ODP request connected to an unvalidated host"); + const error = new TypeError("fetch failed", { cause }); + const failure = await failureOf(inspect(() => Promise.reject(error))); + expect(failure.code).toBe("blocked_destination"); + expect(failure.message).toContain(cause.message); + expect(failure.cause).toBe(error); + }); + + it.each([ + new TypeError("fetch failed", { cause: new Error("connect ETIMEDOUT") }), + new TypeError("fetch failed", { cause: new AggregateError([new Error("ECONNREFUSED")]) }), + new TypeError("custom transport failure"), + new RangeError("custom transport limit"), + "connection lost" + ])("does not infer destination policy from a generic transport failure: %s", async (error) => { + const failure = await failureOf(inspect(vi.fn().mockRejectedValue(error))); + expect(failure.code).toBe("http_error"); + expect(failure.status).toBeUndefined(); + expect(failure.cause).toBe(error); + }); + + it("preserves explicit errors from a custom transport", async () => { + const error = new OdpInspectionError("Custom destination policy", "blocked_destination"); + expect(await failureOf(inspect(() => Promise.reject(error)))).toBe(error); + }); + it("reports an abort as an abort and preserves its cause", async () => { const transport: OdpTransport = vi.fn(() => Promise.reject(new DOMException("The operation was aborted", "AbortError")) @@ -193,7 +222,7 @@ describe("Service Document failures", () => { it("reports an aborted signal even when the transport rejected with something else", async () => { const controller = new AbortController(); controller.abort(); - const transport: OdpTransport = vi.fn(() => Promise.reject(new Error("connection reset"))); + const transport: OdpTransport = vi.fn(() => Promise.reject(new TypeError("fetch failed"))); const failure = await failureOf(inspect(transport, { signal: controller.signal })); expect(failure.code).toBe("aborted"); expect(failure.cause).toBeInstanceOf(Error); diff --git a/packages/agent/test/unit/network.test.ts b/packages/agent/test/unit/network.test.ts index 69fad74..a831553 100644 --- a/packages/agent/test/unit/network.test.ts +++ b/packages/agent/test/unit/network.test.ts @@ -4,6 +4,7 @@ import { gzipSync } from "node:zlib"; import { afterEach, describe, expect, it } from "vitest"; import { createDefaultTransport } from "../../src/network.js"; +import { inspectService } from "../../src/inspection.js"; const servers: ReturnType[] = []; @@ -17,6 +18,26 @@ afterEach(async () => { }); describe("default ODP transport", () => { + it("classifies an actual fetch socket failure as a connection error", async () => { + const server = createServer((request) => request.socket.destroy()); + servers.push(server); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (address === null || typeof address === "string") throw new Error("server has no TCP port"); + await expect( + inspectService({ + serviceUrl: `http://127.0.0.1:${address.port}`, + allowLocalNetwork: true + }) + ).rejects.toMatchObject({ code: "http_error", cause: { name: "TypeError" } }); + }); + + it("classifies an actual private destination rejection without making a request", async () => { + const failure = inspectService({ serviceUrl: "https://127.0.0.1" }); + await expect(failure).rejects.toMatchObject({ code: "blocked_destination" }); + await expect(failure).rejects.toThrow("non-public address"); + }); + it("rejects non-public destinations", async () => { await expect(createDefaultTransport()(new URL("https://127.0.0.1/"))).rejects.toThrow( "non-public address"