fix(server): bound every SSRF-guarded fetch in time and size

The ESLint rule from #2610 that requires a signal on outbound fetches only matched the platform fetch. The guarded wrappers hand it a spread init, which the rule trusts, so safeFetch(url, undefined, options) passed lint with no timeout, and the dispatcher left undici's five-minute default in place. Nothing capped the body either. Two callers did exactly that on hosts TREK does not control: the Synology thumbnail read (a user-configured NAS) and the place enrichment photo download (a URL out of third-party place data), both buffering the whole answer. The wrappers now carry defaults. A call without a signal waits 30 s for the response headers; the body stays under undici's idle timeout, so a proxied video that pauses with its player is not cut off. timeoutMs adds a deadline over the whole exchange and joins the caller's signal, once for the whole redirect chain. Every body is capped at 64 MB, refused up front when the declared length is larger and failed mid-read otherwise; callers pass a tighter maxBytes, or null where the body is streamed on (pipeAsset, the doc-sync transfers that count their own bytes, the Ollama pull). The two call sites get a signal and an 8 MB cap. The lint selectors now cover safeFetch, safeFetchFollow and safeFetchAdminConfigured as well, including an explicit undefined init, and are named once so the repository block restates the same list. A new test asks ESLint for the options it applies to a path and runs them over snippets, so a selector that stops matching fails a test instead of passing silently.
2026-10-24 07:53:09 +00:00 · 2026-10-08 11:54:21 +02:00
parent 8e6c30f883
commit 3fbe0418eb
19 changed files with 579 additions and 110 deletions
@@ -256,17 +256,20 @@ function isLinkLocal(ip: string): boolean {
 * works because the check re-runs per hop rather than locking to the first IP.
 *
 * `responseTimeoutMs` raises undici's own five-minute ceiling for callers that
- * legitimately wait longer (see safeFetchLlm). Left unset it keeps undici's
- * default, which is what every other admin-configured endpoint wants.
+ * legitimately wait longer (see safeFetchLlm). Left unset, the caller's own
+ * signal governs, and a call without one waits DEFAULT_RESPONSE_TIMEOUT_MS for
+ * the response headers. `maxBytes` caps the body as in SafeFetchOptions.
 */
 export async function safeFetchAdminConfigured(
  url: string,
  init?: RequestInit,
  maxRedirects = 5,
  responseTimeoutMs?: number,
+  maxBytes: number | null = DEFAULT_MAX_RESPONSE_BYTES,
 ): Promise<Response> {
  let currentUrl = url;
  let hopInit = init;
+  const headersTimeoutMs = responseTimeoutMs || init?.signal ? undefined : DEFAULT_RESPONSE_TIMEOUT_MS;

  for (let hop = 0; ; hop++) {
    let parsed: URL;
@@ -293,14 +296,14 @@ export async function safeFetchAdminConfigured(
      throw new SsrfBlockedError('Requests to link-local / cloud-metadata addresses are not allowed');
    }

-    const dispatcher = createOutboundDispatcher(currentUrl, usableIps, true, responseTimeoutMs);
+    const dispatcher = createOutboundDispatcher(currentUrl, usableIps, true, responseTimeoutMs, headersTimeoutMs);
    const response = await fetch(currentUrl, { ...hopInit, redirect: 'manual', dispatcher } as any);

    // Only a 3xx WITH a Location header is a redirect we follow; anything else
    // (2xx/4xx/5xx, or a 3xx with no Location) is the final response.
    const status = typeof response.status === 'number' ? response.status : 0;
    const location = status >= 300 && status < 400 ? (response.headers?.get('location') ?? null) : null;
-    if (!location) return response;
+    if (!location) return capResponse(response, maxBytes);

    if (hop >= maxRedirects) {
      throw new SsrfBlockedError('Too many redirects');
@@ -331,8 +334,13 @@ export async function safeFetchAdminConfigured(
 * The ceiling is read once per call rather than per hop, so a redirect chain is
 * measured against one value even if the variable changes mid-chain.
 */
-export function safeFetchLlm(url: string, init?: RequestInit, maxRedirects = 5): Promise<Response> {
-  return safeFetchAdminConfigured(url, init, maxRedirects, readEnv().integrations.llmTimeoutMs);
+export function safeFetchLlm(
+  url: string,
+  init?: RequestInit,
+  maxRedirects = 5,
+  maxBytes: number | null = DEFAULT_MAX_RESPONSE_BYTES,
+): Promise<Response> {
+  return safeFetchAdminConfigured(url, init, maxRedirects, readEnv().integrations.llmTimeoutMs, maxBytes);
 }

 /**
@@ -345,8 +353,86 @@ export class SsrfBlockedError extends Error {
  }
 }

+/**
+ * Thrown when a guarded response is larger than its cap: up front when the
+ * declared Content-Length says so, otherwise by the body read that passes it.
+ */
+export class ResponseTooLargeError extends Error {
+  constructor(readonly maxBytes: number) {
+    super(`Response body exceeds ${maxBytes} bytes`);
+    this.name = 'ResponseTooLargeError';
+  }
+}
+
+/**
+ * What a guarded response may hold when the caller does not say. Sized for the
+ * largest JSON a user's own server sends back (an Immich album lists every
+ * asset it holds), and still small enough that one answer cannot take the
+ * process's memory with it.
+ */
+export const DEFAULT_MAX_RESPONSE_BYTES = 64 * 1024 * 1024;
+
+/**
+ * How long a guarded request waits for the response headers when the caller
+ * passes no signal of its own. Only the headers: the body stays under undici's
+ * idle timeout, so a proxied video that pauses with its player is not cut off.
+ */
+export const DEFAULT_RESPONSE_TIMEOUT_MS = 30_000;
+
 export interface SafeFetchOptions {
  rejectUnauthorized?: boolean;
+  /**
+   * A deadline for the whole exchange, body included. Combined with the init's
+   * own signal when there is one. Without either, the request waits
+   * DEFAULT_RESPONSE_TIMEOUT_MS for the response headers.
+   */
+  timeoutMs?: number;
+  /**
+   * The most bytes the body may hold; DEFAULT_MAX_RESPONSE_BYTES when left out.
+   * `null` lifts the cap, for a caller that streams the body on without holding
+   * it (a proxied original) or counts the bytes itself.
+   */
+  maxBytes?: number | null;
+}
+
+/** The caller's signal, joined with the deadline from `timeoutMs` when one is asked for. */
+function deadlineSignal(signal: AbortSignal | null | undefined, timeoutMs: number | undefined): AbortSignal | undefined {
+  if (!timeoutMs) return signal ?? undefined;
+  const deadline = AbortSignal.timeout(timeoutMs);
+  return signal ? AbortSignal.any([signal, deadline]) : deadline;
+}
+
+/**
+ * The response with its body held to `maxBytes`. A declared length over the cap
+ * is refused before anything is read; a body without one (chunked, compressed)
+ * errors as soon as the bytes passing through cross it, so `json()`,
+ * `arrayBuffer()` or a pipe reject instead of buffering the rest. Anything that
+ * is not a platform Response with a stream (a test stub) is returned as it is.
+ */
+export function capResponse(response: Response, maxBytes: number | null): Response {
+  if (maxBytes === null) return response;
+  const declared = Number(response.headers?.get?.('content-length') ?? 0);
+  if (declared > maxBytes) {
+    void response.body?.cancel().catch(() => {});
+    throw new ResponseTooLargeError(maxBytes);
+  }
+  const body = response.body;
+  if (!(response instanceof Response) || !body || typeof body.pipeThrough !== 'function') return response;
+  let received = 0;
+  const counted = body.pipeThrough(
+    new TransformStream<Uint8Array, Uint8Array>({
+      transform(chunk, controller) {
+        received += chunk.byteLength;
+        if (received > maxBytes) controller.error(new ResponseTooLargeError(maxBytes));
+        else controller.enqueue(chunk);
+      },
+    }),
+  );
+  const capped = new Response(counted, { status: response.status, statusText: response.statusText, headers: response.headers });
+  // A constructed Response has an empty url; callers read the final hop from it.
+  Object.defineProperty(capped, 'url', { value: response.url });
+  Object.defineProperty(capped, 'redirected', { value: response.redirected });
+  return capped;
 }

 /**
@@ -359,8 +445,9 @@ export interface SafeFetchOptions {
 * applies — only the TLS certificate check is relaxed.
 *
 * Redirects are followed through safeFetchFollow, so every hop is re-checked and
- * re-pinned. It used to hand the platform a `redirect: 'follow'` with a
- * dispatcher pinned to the FIRST hop only — the same shape as
+ * re-pinned, with safeFetchFollow's defaults: a body cap and, for a call without
+ * a signal, a bounded wait for the headers. It used to hand the platform a
+ * `redirect: 'follow'` with a dispatcher pinned to the FIRST hop only — the same shape as
 * GHSA-8mw6-xphx-886m, and pinning does not help there because Node skips the
 * pinned lookup for an IP-literal host. Sixteen callers ride on this, several
 * with a URL out of a per-user setting (Immich, Synology, AirTrail), and one
@@ -467,7 +554,8 @@ export interface SafeFetchFollowOptions extends SafeFetchOptions {
 *
 * The returned Response is the first non-redirect response (or the last redirect
 * if the hop limit is reached). `response.url` reflects the final hop so callers
- * relying on the resolved URL keep working.
+ * relying on the resolved URL keep working. Its body is capped and the wait is
+ * bounded as SafeFetchOptions describes.
 */
 export async function safeFetchFollow(
  url: string,
@@ -477,9 +565,13 @@ export async function safeFetchFollow(
  const maxRedirects = options?.maxRedirects ?? 5;
  const rejectUnauthorized = options?.rejectUnauthorized ?? true;
  const bypassInternalIpAllowed = options?.bypassInternalIpAllowed ?? false;
+  const maxBytes = options?.maxBytes === undefined ? DEFAULT_MAX_RESPONSE_BYTES : options.maxBytes;
+  // One signal for the whole chain, so a redirect does not restart the deadline.
+  const signal = deadlineSignal(init?.signal, options?.timeoutMs);
+  const headersTimeoutMs = signal ? undefined : DEFAULT_RESPONSE_TIMEOUT_MS;

  let currentUrl = url;
-  let hopInit = init;
+  let hopInit = signal ? { ...init, signal } : init;

  for (let hop = 0; ; hop++) {
    const ssrf = await checkSsrf(currentUrl, bypassInternalIpAllowed);
@@ -487,7 +579,13 @@ export async function safeFetchFollow(
      throw new SsrfBlockedError(ssrf.error ?? 'Request blocked by SSRF guard');
    }

-    const dispatcher = createOutboundDispatcher(currentUrl, ssrf.resolvedIps ?? [ssrf.resolvedIp!], rejectUnauthorized);
+    const dispatcher = createOutboundDispatcher(
+      currentUrl,
+      ssrf.resolvedIps ?? [ssrf.resolvedIp!],
+      rejectUnauthorized,
+      undefined,
+      headersTimeoutMs,
+    );
    const response = await fetch(currentUrl, {
      ...hopInit,
      redirect: 'manual',
@@ -500,7 +598,7 @@ export async function safeFetchFollow(
    const isRedirectStatus = status >= 300 && status < 400;
    const location = isRedirectStatus ? (response.headers?.get('location') ?? null) : null;
    if (!location) {
-      return response;
+      return capResponse(response, maxBytes);
    }

    if (hop >= maxRedirects) {
@@ -561,16 +659,31 @@ export function createOutboundDispatcher(
  resolved: string | readonly string[],
  rejectUnauthorized = true,
  responseTimeoutMs?: number,
+  headersTimeoutMs?: number,
 ): Dispatcher {
  const proxy = proxyFor(url);
-  if (!proxy) return createPinnedDispatcher(resolved, rejectUnauthorized, responseTimeoutMs);
+  if (!proxy) return createPinnedDispatcher(resolved, rejectUnauthorized, responseTimeoutMs, headersTimeoutMs);
  return new ProxyAgent({
    uri: proxy,
    requestTls: { rejectUnauthorized },
-    ...(responseTimeoutMs ? { headersTimeout: responseTimeoutMs, bodyTimeout: responseTimeoutMs } : {}),
+    ...dispatcherTimeouts(responseTimeoutMs, headersTimeoutMs),
  });
 }

+/**
+ * undici's timeouts for one dispatcher. `responseTimeoutMs` bounds both the
+ * wait for the headers and each pause in the body; `headersTimeoutMs` bounds
+ * the headers only. Neither leaves undici its five-minute defaults.
+ */
+function dispatcherTimeouts(
+  responseTimeoutMs?: number,
+  headersTimeoutMs?: number,
+): { headersTimeout?: number; bodyTimeout?: number } {
+  if (responseTimeoutMs) return { headersTimeout: responseTimeoutMs, bodyTimeout: responseTimeoutMs };
+  if (headersTimeoutMs) return { headersTimeout: headersTimeoutMs };
+  return {};
+}
+
 /**
 * Returns an undici Agent whose connect.lookup is pinned to the already-validated
 * addresses. This prevents DNS rebinding (TOCTOU) by ensuring the outbound
@@ -586,6 +699,7 @@ export function createPinnedDispatcher(
  resolved: string | readonly string[],
  rejectUnauthorized = true,
  responseTimeoutMs?: number,
+  headersTimeoutMs?: number,
 ): Agent {
  const addresses = (typeof resolved === 'string' ? [resolved] : [...resolved]).map((address) => ({
    address,
@@ -595,9 +709,7 @@ export function createPinnedDispatcher(
    // undici caps the wait for response headers at 5 minutes by default, and
    // that cap is invisible from the call site: an AbortController set to
    // fifteen still dies at five.
-    ...(responseTimeoutMs
-      ? { headersTimeout: responseTimeoutMs, bodyTimeout: responseTimeoutMs }
-      : {}),
+    ...dispatcherTimeouts(responseTimeoutMs, headersTimeoutMs),
    connect: {
      rejectUnauthorized,
      lookup: (_hostname: string, opts: Record<string, unknown>, callback: Function) => {