nexu-io
diff --git a/‎apps/daemon/src/byok-tools.ts‎
Lines changed: 18 additions & 16 deletions b/‎apps/daemon/src/byok-tools.ts‎
Lines changed: 18 additions & 16 deletions
diff --git a/‎apps/daemon/src/connectionTest.ts‎
Lines changed: 157 additions & 16 deletions b/‎apps/daemon/src/connectionTest.ts‎
Lines changed: 157 additions & 16 deletions
diff --git a/‎apps/daemon/tests/aihubmix-asset-ssrf.test.ts‎
Lines changed: 4 additions & 4 deletions b/‎apps/daemon/tests/aihubmix-asset-ssrf.test.ts‎
Lines changed: 4 additions & 4 deletions
@@ -15,7 +15,7 @@
 import path from 'node:path';
 import { writeFile, readFile, readdir, stat } from 'node:fs/promises';
 import { randomBytes } from 'node:crypto';
-import { assertExternalAssetUrl, assertAndFetchExternalAsset } from './connectionTest.js';
+import { assertAndFetchExternalAsset } from './connectionTest.js';
 import { resolveProviderConfig } from './media/config.js';
 import { IMAGE_MODELS } from './media/models.js';
 import { ensureProject } from './projects.js';
@@ -447,8 +447,15 @@ export interface BYOKToolContext {
   videoPollIntervalMs?: number;
   /** Optional per-request init copied from the live chat turn. Used to
    *  forward the current proxy dispatcher AND the client-cancellation
-   *  signal into every upstream/download fetch the BYOK tool executor
-   *  performs, so a disconnected client stops the tool loop's paid work. */
+   *  signal into every upstream fetch the BYOK tool executor performs,
+   *  so a disconnected client stops the tool loop's paid work.
+   *
+   *  Exception — asset downloads: when a provider result URL is fetched
+   *  through `assertAndFetchExternalAsset` (connectionTest.ts), that
+   *  helper intentionally OVERRIDES `init.dispatcher` with the shared
+   *  asset-validating dispatcher whose connect-time DNS lookup rejects
+   *  non-public addresses (issue #5478). Asset downloads therefore never
+   *  ride the turn proxy dispatcher; submit/poll hops keep it. */
   requestInit?: Pick<RequestInit, 'dispatcher' | 'signal'>;
 }
 
@@ -693,12 +700,11 @@ export async function executeGenerateImage(
     };
   }
 
-  const imageUrlCheck = await assertExternalAssetUrl(imageUrl);
-  if (!imageUrlCheck.ok) return { ok: false, error: imageUrlCheck.error };
-
   let bytes: Buffer;
   try {
-    const imgResp = await fetch(imageUrl, withToolRequestInit(ctx, { redirect: 'error' }));
+    // Use assertAndFetchExternalAsset (validate + pinned-DNS fetch) so a
+    // malicious gateway can't DNS-rebind into loopback/metadata space.
+    const imgResp = await assertAndFetchExternalAsset(imageUrl, withToolRequestInit(ctx, {}));
     if (!imgResp.ok) {
       return { ok: false, error: `image download ${imgResp.status}` };
     }
@@ -923,15 +929,12 @@ export async function executeGenerateVideo(
   }
 
   // Step 3: download the mp4 bytes and persist into the project folder.
-  // Re-validate the returned URL through validateBaseUrlResolved so a
-  // malicious gateway can't point us at 169.254.169.254 (AWS / Azure
-  // metadata service) or RFC1918 hosts via the response payload.
-  const videoUrlCheck = await assertExternalAssetUrl(videoUrl);
-  if (!videoUrlCheck.ok) return { ok: false, error: videoUrlCheck.error };
+  // Use assertAndFetchExternalAsset (validate + pinned-DNS fetch) so a
+  // malicious gateway can't DNS-rebind into loopback / metadata space.
 
   let bytes: Buffer;
   try {
-    const videoResp = await fetch(videoUrl, withToolRequestInit(ctx, { redirect: 'error' }));
+    const videoResp = await assertAndFetchExternalAsset(videoUrl, withToolRequestInit(ctx, {}));
     if (!videoResp.ok) {
       return { ok: false, error: `video download ${videoResp.status}` };
     }
@@ -1314,10 +1317,9 @@ async function resolveAIHubMixReferenceImage(
   if (typeof imageUrl !== 'string' || !imageUrl.trim()) return null;
   const raw = imageUrl.trim();
   if (/^https?:\/\//i.test(raw)) {
-    const check = await assertExternalAssetUrl(raw);
-    if (!check.ok) return null;
     try {
-      const resp = await fetch(raw, withToolRequestInit(ctx, { redirect: 'error' }));
+      // assertAndFetchExternalAsset validates + pins DNS to prevent rebind.
+      const resp = await assertAndFetchExternalAsset(raw, withToolRequestInit(ctx, {}));
       if (!resp.ok) return null;
       const buf = Buffer.from(await resp.arrayBuffer());
       if (!buf.length) return null;
 
@@ -17,7 +17,7 @@
 // contracts so Settings and daemon-side checks reject the same hosts.
 
 import { spawn } from 'node:child_process';
-import { promises as dnsPromises } from 'node:dns';
+import { promises as dnsPromises, lookup as dnsLookupCb } from 'node:dns';
 import { promises as fsp } from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
@@ -143,7 +143,9 @@ export async function validateBaseUrlResolved(
   if (sync.error || !sync.parsed) return sync;
 
   const hostname = sync.parsed.hostname.toLowerCase();
-  if (isLoopbackApiHost(hostname)) return sync;
+  // When forbidLoopback is set, do NOT short-circuit on loopback — let it
+  // fall through to the block check (issue #5478).
+  if (!options.forbidLoopback && isLoopbackApiHost(hostname)) return sync;
   // Issue #3225 — an operator who trusts this hostname has opted it out of the
   // guard entirely, so skip the resolved-IP block even though it points into
   // private space. The sync check above already honored a literal-IP allowlist
@@ -155,12 +157,31 @@ export async function validateBaseUrlResolved(
   try {
     addresses = await lookup(hostname);
   } catch {
+    // When forbidLoopback is set (attacker-controllable asset URLs), a DNS
+    // lookup failure must fail closed. An attacker who controls the resolver
+    // can make the validation lookup throw (ENOTFOUND / ETIMEOUT / SERVFAIL)
+    // and then answer loopback for the fetch-time lookup, bypassing the guard.
+    // (issue #5478)
+    if (options.forbidLoopback) {
+      return { error: 'DNS resolution failed for asset URL', forbidden: true };
+    }
     return sync;
   }
 
   for (const addr of addresses) {
     const ip = String(addr.address).toLowerCase();
-    if (isLoopbackApiHost(ip)) continue;
+    // When forbidLoopback is set (asset download URLs from issue #5478),
+    // a DNS name that resolves to loopback is just as dangerous as a
+    // literal loopback host — reject it instead of skipping.
+    if (isLoopbackApiHost(ip)) {
+      if (options.forbidLoopback) {
+        return {
+          error: `DNS-resolved loopback address blocked (${ip})`,
+          forbidden: true,
+        };
+      }
+      continue;
+    }
     // A resolved address the operator explicitly allowlisted (they listed the
     // IP rather than the hostname) is permitted; everything else in private
     // space is still blocked.
@@ -170,7 +191,11 @@ export async function validateBaseUrlResolved(
     }
   }
 
-  return sync;
+  // Attach validated addresses so the caller can pin the actual fetch to them.
+  // This prevents DNS rebinding: without pinning, the attacker's DNS can return
+  // a public IP here and then 127.0.0.1 at fetch time, so the daemon connects
+  // to loopback despite the validation having passed (issue #5478).
+  return { ...sync, resolvedAddresses: addresses };
 }
 
 /**
@@ -211,14 +236,27 @@ export function validateUserProviderBaseUrl(
  * Both hand the URL straight to `fetch(...)` next, so pair this
  * guard with `redirect: 'error'` on the fetch to also block a
  * 3xx hop into private space.
+ *
+ * Returns the DNS-resolved addresses that passed validation on the `ok` branch
+ * so callers (e.g. {@link assertAndFetchExternalAsset}) can pin the actual
+ * connection to those addresses and prevent DNS rebinding (issue #5478).
  */
 export async function assertExternalAssetUrl(
   rawUrl: string,
-): Promise<{ ok: true } | { ok: false; error: string }> {
+  lookup?: DnsLookupFn,
+): Promise<
+  | { ok: true; resolvedAddresses?: ReadonlyArray<{ address: string; family: number }> }
+  | { ok: false; error: string }
+> {
   if (typeof rawUrl !== 'string' || !rawUrl) {
     return { ok: false, error: 'empty download url' };
   }
-  const validated = await validateBaseUrlResolved(rawUrl);
+  // Asset URLs come from upstream API responses (data.url / data.video_url)
+  // and are attacker-controllable. They MUST NOT point at loopback or
+  // internal addresses, regardless of operator allowlists (issue #5478).
+  const validated = await validateBaseUrlResolved(rawUrl, lookup, {
+    forbidLoopback: true,
+  });
   if (validated.error || !validated.parsed) {
     return {
       ok: false,
@@ -227,28 +265,131 @@ export async function assertExternalAssetUrl(
         : `invalid download url: ${validated.error ?? 'unknown reason'}`,
     };
   }
+  // Only include resolvedAddresses when present — exactOptionalPropertyTypes
+  // forbids assigning `undefined` to an optional property.
+  if (validated.resolvedAddresses) {
+    return { ok: true, resolvedAddresses: validated.resolvedAddresses };
+  }
   return { ok: true };
 }
 
+/**
+ * Connection-time DNS validator for asset-download requests. Wraps `dns.lookup`
+ * and rejects any resolved address that is loopback, RFC1918, link-local,
+ * CGNAT, metadata-service, or multicast — the same predicate used during
+ * pre-validation. Installed as the Undici Agent's `connect.lookup` so the
+ * address we validate IS the address the socket connects to, closing the
+ * DNS-rebinding / TOCTOU gap that a separate pre-validation lookup leaves open
+ * (issue #5478). Same pattern as `brands/safe-fetch.ts` and
+ * `plugins/plugin-asset-cache.ts`.
+ *
+ * Exported so the guard can be unit-tested without a live server.
+ */
+export function createAssetValidatingLookup(
+  lookupImpl: typeof dnsLookupCb = dnsLookupCb,
+): (hostname: string, options: unknown, callback: (...args: unknown[]) => void) => void {
+  return (
+    hostname: string,
+    options: unknown,
+    callback: (...args: unknown[]) => void,
+  ): void => {
+    const cb = (typeof options === 'function' ? options : callback) as (
+      err: Error | null,
+      address?: unknown,
+      family?: number,
+    ) => void;
+    const opts = (typeof options === 'function' ? {} : (options ?? {})) as Record<string, unknown>;
+    lookupImpl(hostname, opts as never, (err, address, family) => {
+      if (err) return cb(err);
+      const list = Array.isArray(address) ? address : [{ address, family }];
+      for (const entry of list) {
+        const addr = typeof entry === 'string' ? entry : (entry as { address: string }).address;
+        if (isLoopbackApiHost(String(addr)) || isBlockedExternalApiHostname(String(addr))) {
+          return cb(new Error(`asset host resolves to non-public address: ${addr}`));
+        }
+      }
+      return cb(null, address, family);
+    });
+  };
+}
+
+// Long-lived dispatcher reused across calls. A per-request Agent leaks
+// keep-alive sockets; a shared dispatcher avoids that while still pinning the
+// connection-time validating lookup (same approach as plugin-asset-cache.ts).
+// Used by `createAssetValidatingLookup` consumers in production; the default
+// fetch in `assertAndFetchExternalAsset` is `globalThis.fetch` so test stubs
+// still intercept.
+const assetDispatcher = new Agent({
+  connect: { lookup: createAssetValidatingLookup() as never },
+});
+
+/**
+ * Test-visible accessor for the shared asset-validating dispatcher attached by
+ * `assertAndFetchExternalAsset`. Tests key dispatcher assertions on the asset
+ * URL and compare against this exact instance (`toBe`), so a regression that
+ * reverts to forwarding the caller's turn-proxy dispatcher on the asset hop
+ * fails loudly (issue #5478).
+ */
+export function getAssetValidatingDispatcher(): NonNullable<RequestInit['dispatcher']> {
+  return assetDispatcher as unknown as NonNullable<RequestInit['dispatcher']>;
+}
+
 /**
  * Validate an upstream-controlled asset URL and fetch it with the SSRF guard
- * pinned through redirects. Runs `assertExternalAssetUrl` on the literal URL
- * and forces `redirect: 'error'`, so a validated public URL that 302s into
- * loopback / RFC1918 / metadata space is rejected before any bytes are read.
+ * pinned through redirects and DNS resolution. Runs `assertExternalAssetUrl`
+ * on the literal URL (fail-closed on DNS errors), forces `redirect: 'error'`
+ * (blocking a 3xx hop into private space), and routes the fetch through a
+ * long-lived Undici dispatcher whose connection-time `lookup` rejects any
+ * non-public address — so even if an attacker's DNS returns a public address
+ * during pre-validation and loopback at connect time, the socket is refused
+ * (issue #5478).
+ *
+ * For non-IP-literal hostnames, if DNS validation did not attach a vetted
+ * address set (e.g., lookup failure), the function throws rather than falling
+ * back to an unpinned fetch. IP literals are safe to fetch unpinned because
+ * they were validated synchronously and have no hostname to rebind.
  *
- * Throws on a blocked host — so the redirect bypass is impossible to forget at
- * call sites — and the platform fetch additionally throws when `redirect:
- * 'error'` encounters a 3xx. Callers keep their own `!resp.ok` HTTP-status
- * handling. The forced `redirect` is spread last so it overrides any value the
- * caller passed in `init`.
+ * Throws on a blocked host or unpinned-fetch refusal. Callers keep their own
+ * `!resp.ok` HTTP-status handling. The forced `redirect` is spread last so it
+ * overrides any value the caller passed in `init`.
  */
 export async function assertAndFetchExternalAsset(
   url: string,
   init: RequestInit = {},
+  lookup?: DnsLookupFn,
+  fetchImpl: typeof fetch = fetch,
 ): Promise<Response> {
-  const check = await assertExternalAssetUrl(url);
+  const check = await assertExternalAssetUrl(url, lookup);
   if (!check.ok) throw new Error(check.error);
-  return fetch(url, { ...init, redirect: 'error' });
+
+  // Determine whether the hostname is an IP literal. If so, the synchronous
+  // validation already vetted it — no DNS rebind is possible.
+  let parsedUrl: URL;
+  try {
+    parsedUrl = new URL(url);
+  } catch {
+    throw new Error(`invalid asset url: ${url}`);
+  }
+  const hostname = parsedUrl.hostname.toLowerCase();
+  const isIpLiteral = looksLikeIpLiteral(hostname);
+
+  // For non-IP-literal hostnames, require validated resolved addresses. If
+  // they are missing (DNS lookup failed and was caught → fail-closed in
+  // validateBaseUrlResolved), never fall back to an unpinned fetch — that
+  // would allow the attacker to rebind at fetch time (issue #5478).
+  if (!isIpLiteral && (!check.resolvedAddresses || check.resolvedAddresses.length === 0)) {
+    throw new Error('asset URL hostname was not DNS-validated — refusing unpinned fetch');
+  }
+
+  // Route through the long-lived asset dispatcher whose connection-time lookup
+  // rejects non-public addresses. The dispatcher is attached to the init object
+  // so injected fetch stubs (vi.stubGlobal) still see redirect:'error' and
+  // can ignore dispatcher, while production globalThis.fetch (Node/undici)
+  // uses it to refuse a connect-time rebind to loopback/metadata (issue #5478).
+  // Same pattern as plugins/plugin-asset-cache.ts safeExternalFetch.
+  const requestInit: RequestInit = { ...init, redirect: 'error' };
+  (requestInit as { dispatcher?: unknown }).dispatcher = assetDispatcher;
+  return fetchImpl(url, requestInit);
 }
 
 // Aggressive but not punitive — happy paths usually return in under 2 s.
 
@@ -100,12 +100,12 @@ describe('AIHubMix asset downloads pin redirect:"error"', () => {
         return new Response(
           JSON.stringify({
             status: 'completed',
-            video_url: 'https://cdn.example.test/video/done.mp4',
+            video_url: 'https://93.184.216.34/video/done.mp4',
           }),
           { status: 200, headers: { 'content-type': 'application/json' } },
         );
       }
-      if (url === 'https://cdn.example.test/video/done.mp4') {
+      if (url === 'https://93.184.216.34/video/done.mp4') {
         downloadInit = init;
         return new Response(mp4Bytes, {
           status: 200,
@@ -128,11 +128,11 @@ describe('AIHubMix asset downloads pin redirect:"error"', () => {
       const url = String(input);
       if (url === 'https://aihubmix.com/v1/images/generations') {
         return new Response(
-          JSON.stringify({ data: [{ url: 'https://cdn.example.test/img/out.png' }] }),
+          JSON.stringify({ data: [{ url: 'https://93.184.216.34/img/out.png' }] }),
           { status: 200, headers: { 'content-type': 'application/json' } },
         );
       }
-      if (url === 'https://cdn.example.test/img/out.png') {
+      if (url === 'https://93.184.216.34/img/out.png') {
         downloadInit = init;
         return new Response(pngBytes, {
           status: 200,