security: pin provider discovery transports

Author: YourName

docs-site/src/content/docs/reference/configuration.md

@@ -75,6 +75,31 @@ differing backup and rewrites known legacy namespaced selected ids to bare ids.
 `max_concurrent_threads_per_session` value under `[features.multi_agent_v2]` in Codex's
 `$CODEX_HOME/config.toml`; enable v2 first so that table exists.
 
+## Provider diagnostic outbound safety
+
+The dashboard provider connection test and live model discovery use a bounded GET-only outbound
+transport. Without an outbound proxy, opencodex resolves the provider hostname once and connects
+only to that validated address. HTTPS keeps the original hostname for Host, SNI, and certificate
+verification; certificate verification cannot be disabled by provider config.
+
+When `HTTP_PROXY` or `HTTPS_PROXY` applies, these two operations keep Bun's native fetch so existing
+proxy behavior is not silently bypassed. URL/literal checks still run. Successful local DNS answers
+are classified, but a local DNS failure is allowed through because proxy-only networks commonly
+delegate name resolution to the proxy. The proxy chooses the final route, DNS answer, and peer, so
+opencodex logs that this path cannot pin or verify the proxy-selected peer. This is an explicit
+security limitation, not equivalent protection against DNS rebinding.
+
+Private/local provider destinations require both `allowPrivateNetwork: true` and a matching
+`NO_PROXY` entry whenever an outbound proxy is configured. Loopback entries are added to `NO_PROXY`
+automatically. A LAN provider such as `192.168.1.50` must be added explicitly; otherwise connection
+tests and model discovery reject it with an actionable message instead of sending it to the proxy.
+Metadata and link-local destinations remain blocked even when `allowPrivateNetwork` is enabled.
+
+Both direct and proxied diagnostic paths reject redirects and report a credential-stripped target;
+configure the final provider URL directly. Ordinary provider requests, streaming responses, and
+retry paths are not migrated in this phase. Their redirect handling and per-hop destination review
+remain deferred, so this phase does not close the main-request redirect finding.
+
 ## Combos (`config.combos`)
 
 Failover / round-robin aliases live under `combos.<id>` with `targets` (provider + model), optional

src/codex/catalog/provider-fetch.ts

@@ -39,7 +39,11 @@ import {
   targetKey,
 } from "../../combos";
 import type { NormalizedComboConfig } from "../../combos/types";
-import { providerDestinationResolvedError } from "../../lib/destination-policy";
+import {
+  ProviderOutboundPolicyError,
+  providerOutboundGet,
+  providerRedirectError,
+} from "../../lib/provider-outbound";
 import { redactSecretString } from "../../lib/redact";
 import upstreamModelsSnapshot from "../data/upstream-models.json";
 
@@ -322,21 +326,20 @@ export async function fetchProviderModels(name: string, prov: OcxProviderConfig,
     };
   };
   try {
-    const destinationError = await providerDestinationResolvedError(name, {
-      baseUrl: url,
-      allowPrivateNetwork: prov.allowPrivateNetwork,
+    const res = await providerOutboundGet(name, prov, url, {
+      headers,
+      signal: AbortSignal.timeout(8000),
     });
-    if (destinationError) {
-      const { models, fallback, shouldLog } = failedDiscoveryFallback({ reason: "blocked" });
+    const redirectError = await providerRedirectError(res, url);
+    if (redirectError) {
+      const { models, fallback, shouldLog } = failedDiscoveryFallback({ reason: "http", httpStatus: res.status });
       if (shouldLog) {
         console.warn(
-          `[opencodex] Provider model discovery for "${name}" was blocked by destination policy: ${destinationError} [urlClass=${urlClass}, fallback=${fallback}].`,
+          `[opencodex] Provider model discovery for "${name}" ${redirectError} [urlClass=${urlClass}, fallback=${fallback}].`,
         );
       }
       return models;
     }
-
-    const res = await fetch(url, { headers, signal: AbortSignal.timeout(8000) });
     if (!res.ok) {
       const { models, fallback, shouldLog } = failedDiscoveryFallback({ reason: "http", httpStatus: res.status });
       if (shouldLog) {
@@ -421,6 +424,15 @@ export async function fetchProviderModels(name: string, prov: OcxProviderConfig,
     setCached(name, live);
     return live;
   } catch (error) {
+    if (error instanceof ProviderOutboundPolicyError) {
+      const { models, fallback, shouldLog } = failedDiscoveryFallback({ reason: "blocked" });
+      if (shouldLog) {
+        console.warn(
+          `[opencodex] Provider model discovery for "${name}" was blocked by destination policy: ${error.message} [urlClass=${urlClass}, fallback=${fallback}].`,
+        );
+      }
+      return models;
+    }
     const { models, fallback, shouldLog } = failedDiscoveryFallback({ reason: "network" });
     if (shouldLog) {
       console.warn(

src/images/artifacts.ts

@@ -1,12 +1,12 @@
 import { readdirSync, readFileSync, statSync, unlinkSync, existsSync } from "node:fs";
 import { mkdir, writeFile } from "node:fs/promises";
-import type { IncomingMessage } from "node:http";
-import https from "node:https";
-import type { RequestOptions } from "node:https";
 import { basename, join, resolve, sep } from "node:path";
 import { getConfigDir } from "../config";
 import { assessUrlDestination, resolvePublicAddresses } from "../lib/destination-policy";
 import { recordOwnedConfigPath } from "../lib/config-ownership";
+import { pinnedHttpGet, type PinnedAddress } from "../lib/pinned-http";
+
+export type { PinnedAddress } from "../lib/pinned-http";
 
 const MAX_DECODED_BYTES_PER_IMAGE = 50 * 1024 * 1024;
 const MAX_DECODED_BYTES_PER_RESPONSE = 100 * 1024 * 1024;
@@ -40,8 +40,6 @@ export interface ImageBudget {
   spent: number;
 }
 
-export type PinnedAddress = { address: string; family: number };
-
 /** Test seam / custom transport: must connect to `pinned`, not re-resolve `url`'s hostname. */
 export type PinnedDownloadFn = (
   url: string,
@@ -282,120 +280,14 @@ export function pinnedHttpsGet(
   }
   const maxBytes = options?.maxBytes ?? MAX_DOWNLOAD_BYTES;
   const idleTimeoutMs = options?.idleTimeoutMs ?? DOWNLOAD_IDLE_TIMEOUT_MS;
-
-  return new Promise<Response>((resolve, reject) => {
-    if (signal?.aborted) {
-      reject(signal.reason instanceof Error ? signal.reason : new Error("aborted"));
-      return;
-    }
-
-    let settled = false;
-    const fail = (err: unknown) => {
-      try { req.destroy(); } catch { /* ignore */ }
-      if (settled) return;
-      settled = true;
-      reject(err instanceof Error ? err : new Error(String(err)));
-    };
-
-    const optionsHttps: RequestOptions & { servername?: string } = {
-      protocol: "https:",
-      hostname: parsed.hostname,
-      servername: parsed.hostname,
-      port: parsed.port || 443,
-      path: `${parsed.pathname}${parsed.search}`,
-      method: "GET",
-      headers: { Host: parsed.host },
-      rejectUnauthorized: options?.rejectUnauthorized,
-      lookup(_hostname, lookupOptions, callback) {
-        const opts = typeof lookupOptions === "function" ? undefined : lookupOptions;
-        const cb = typeof lookupOptions === "function" ? lookupOptions : callback;
-        if (!cb) return;
-        // Pin the validated peer — do not call dns.lookup again.
-        // Honor `{ all: true }` array shape used by some Node/Bun https paths.
-        if (opts && typeof opts === "object" && "all" in opts && opts.all) {
-          (cb as (err: NodeJS.ErrnoException | null, addresses: PinnedAddress[]) => void)(
-            null,
-            [{ address: pinned.address, family: pinned.family }],
-          );
-          return;
-        }
-        (cb as (err: NodeJS.ErrnoException | null, address: string, family: 4 | 6) => void)(
-          null,
-          pinned.address,
-          pinned.family as 4 | 6,
-        );
-      },
-    };
-
-    const req = https.request(optionsHttps, (res: IncomingMessage) => {
-      const status = res.statusCode ?? 0;
-      // Any non-2xx must destroy immediately. Returning a streaming Response for
-      // 4xx/5xx (or 3xx) lets callers that only check `Response.ok` abandon an
-      // unread body while the peer keeps sending — a failed-response socket leak.
-      if (status < 200 || status >= 300) {
-        try { res.destroy(); } catch { /* ignore */ }
-        fail(new Error("image download failed: " + status));
-        return;
-      }
-      const headers = new Headers();
-      for (const [key, value] of Object.entries(res.headers)) {
-        if (value === undefined || value === null) continue;
-        if (Array.isArray(value)) {
-          for (const item of value) headers.append(key, String(item));
-        } else {
-          headers.set(key, String(value));
-        }
-      }
-
-      let received = 0;
-      const stream = new ReadableStream<Uint8Array>({
-        start(controller) {
-          res.setTimeout(idleTimeoutMs, () => {
-            fail(new Error("image download stalled"));
-            try { controller.error(new Error("image download stalled")); } catch { /* closed */ }
-          });
-          res.on("data", (chunk: Buffer | string) => {
-            const buf = typeof chunk === "string" ? Buffer.from(chunk) : chunk;
-            received += buf.byteLength;
-            if (received > maxBytes) {
-              const err = new Error(`image download exceeds ${maxBytes} byte cap`);
-              fail(err);
-              try { controller.error(err); } catch { /* closed */ }
-              return;
-            }
-            try { controller.enqueue(buf); } catch { /* closed */ }
-          });
-          res.on("end", () => {
-            try { controller.close(); } catch { /* closed */ }
-          });
-          res.on("error", (err: Error) => {
-            fail(err);
-            try { controller.error(err); } catch { /* closed */ }
-          });
-        },
-        cancel() {
-          req.destroy();
-        },
-      });
-
-      if (settled) return;
-      settled = true;
-      resolve(new Response(stream, { status, headers }));
-    });
-
-    const onAbort = () => {
-      fail(signal?.reason instanceof Error ? signal.reason : new Error("aborted"));
-    };
-    signal?.addEventListener("abort", onAbort, { once: true });
-    req.setTimeout(idleTimeoutMs, () => {
-      fail(new Error("image download timed out"));
-    });
-    req.on("error", (err) => {
-      signal?.removeEventListener("abort", onAbort);
-      fail(err);
-    });
-    req.on("close", () => signal?.removeEventListener("abort", onAbort));
-    req.end();
+  return pinnedHttpGet(url, pinned, signal, {
+    maxBytes,
+    idleTimeoutMs,
+    rejectUnauthorized: options?.rejectUnauthorized,
+    context: "image download",
+  }).then(response => {
+    if (!response.ok) throw new Error("image download failed: " + response.status);
+    return response;
   });
 }
 

src/lib/destination-policy.ts

@@ -204,6 +204,10 @@ export interface UrlDestinationAssessment {
   detail: string;
 }
 
+export class DestinationDnsResolutionError extends Error {
+  override readonly name = "DestinationDnsResolutionError";
+}
+
 /**
  * Synchronous literal URL destination assessment — classifies the hostname
  * without DNS resolution. Returns null for unparseable URLs.
@@ -213,55 +217,74 @@ export function assessUrlDestination(url: string): UrlDestinationAssessment | nu
 }
 
 /**
- * Async DNS-resolved URL safety check. Resolves A/AAAA records and rejects
- * if any address is loopback, private, link-local, unspecified, or metadata.
- * Throws on unsafe destination; returns the validated public addresses on success
- * so callers can pin the connect peer and avoid a second, rebindable resolution.
- * DNS resolution failures are treated as unsafe (fail-closed).
+ * Async DNS-resolved URL safety check. By default, rejects every non-public
+ * address. Provider diagnostics may explicitly admit loopback/private answers;
+ * metadata, link-local, and unspecified addresses remain unconditionally denied.
+ * Returns the validated addresses so direct callers can pin the connect peer and
+ * avoid a second, rebindable resolution. DNS failures remain fail-closed here;
+ * the provider proxy wrapper alone may recognize that typed failure and degrade.
  */
-export async function resolvePublicAddresses(url: string): Promise<{
+export async function resolvePublicAddresses(
+  url: string,
+  options?: { context?: string; allowPrivateNetwork?: boolean },
+): Promise<{
   hostname: string;
   addresses: { address: string; family: number }[];
+  privateNetwork: boolean;
 }> {
+  const context = options?.context?.trim() || "image URL";
+  const privateNetworkAllowed = options?.allowPrivateNetwork === true;
   let hostname: string;
   try {
     hostname = normalizeHostname(new URL(url.trim()).hostname);
   } catch {
-    throw new Error("image URL is not a valid URL");
+    throw new Error(`${context} is not a valid URL`);
   }
-  if (!hostname) throw new Error("image URL has no hostname");
+  if (!hostname) throw new Error(`${context} has no hostname`);
   const literalAssessment = assessDestination(url);
+  let privateNetwork = false;
   if (literalAssessment && literalAssessment.kind !== "public" && literalAssessment.kind !== "hostname") {
-    throw new Error(`image URL targets ${literalAssessment.detail}`);
+    const allowedPrivateLiteral = privateNetworkAllowed
+      && (literalAssessment.kind === "localhost"
+        || literalAssessment.kind === "loopback"
+        || literalAssessment.kind === "private");
+    if (!allowedPrivateLiteral) throw new Error(`${context} targets ${literalAssessment.detail}`);
+    privateNetwork = true;
   }
   // Literal public IPs: no DNS round-trip; pin the literal itself.
   const literalKind = isIP(hostname);
   if (literalKind !== 0) {
-    return { hostname, addresses: [{ address: hostname, family: literalKind }] };
+    return { hostname, addresses: [{ address: hostname, family: literalKind }], privateNetwork };
   }
   let addresses: { address: string; family: number }[];
   try {
     addresses = await lookup(hostname, { all: true, verbatim: true });
   } catch {
     // If DNS fails, we can't verify — fail-closed (unlike provider config-time validation,
     // this is a runtime fetch to an untrusted URL, so be conservative).
-    throw new Error(`image URL hostname ${hostname} could not be resolved`);
+    throw new DestinationDnsResolutionError(`${context} hostname ${hostname} could not be resolved`);
   }
   if (addresses.length === 0) {
-    throw new Error(`image URL hostname ${hostname} could not be resolved`);
+    throw new DestinationDnsResolutionError(`${context} hostname ${hostname} could not be resolved`);
   }
-  const publicAddresses: { address: string; family: number }[] = [];
+  const validatedAddresses: { address: string; family: number }[] = [];
   for (const { address, family } of addresses) {
     // Prefer classifying from the address string itself — do not trust a mislabeled
     // resolver `family` that could skip IPv4/IPv6 private checks.
     const ipKind = isIP(address) || (family === 4 || family === 6 ? family : 0);
     const assessment = ipKind === 4 ? classifyIpv4(address) : ipKind === 6 ? classifyIpv6(normalizeHostname(address)) : null;
     if (!assessment || assessment.kind !== "public") {
-      throw new Error(`image URL hostname ${hostname} resolves to ${assessment?.detail ?? "an unsafe address"} (${address})`);
+      const allowedPrivateAddress = privateNetworkAllowed
+        && assessment
+        && (assessment.kind === "loopback" || assessment.kind === "private");
+      if (!allowedPrivateAddress) {
+        throw new Error(`${context} hostname ${hostname} resolves to ${assessment?.detail ?? "an unsafe address"} (${address})`);
+      }
+      privateNetwork = true;
     }
-    publicAddresses.push({ address, family: ipKind === 4 || ipKind === 6 ? ipKind : (family || 4) });
+    validatedAddresses.push({ address, family: ipKind === 4 || ipKind === 6 ? ipKind : (family || 4) });
   }
-  return { hostname, addresses: publicAddresses };
+  return { hostname, addresses: validatedAddresses, privateNetwork };
 }
 
 /**