Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/fixes/7058-zai-web-proxy-connection-test.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- fix(providers): web-cookie connection-test/cookie-validation probe (zai-web and every other registry-entry web-cookie provider) now honors the configured HTTP/SOCKS proxy — the `/models` probe routed through `directHttpsRequest`'s hardcoded native-fetch bypass, silently skipping proxy resolution even though the executor's actual chat traffic already respected it (#7058)
4 changes: 2 additions & 2 deletions src/lib/providers/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export async function validateWebCookieProvider({
const baseUrl = normalizeBaseUrl(entry.baseUrl || "");
const testUrl = `${baseUrl}/models`;

const res = await directHttpsRequest(
const res = await validationRead(
testUrl,
{
method: "GET",
Expand All @@ -164,7 +164,7 @@ export async function validateWebCookieProvider({
Cookie: cookie,
},
},
10_000
isLocalProvider(provider)
);

if (res.status === 401 || res.status === 403) {
Expand Down
19 changes: 10 additions & 9 deletions tests/unit/provider-validation-web-cookie-auth007.test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import test from "node:test";
import assert from "node:assert/strict";

// The validator probes the provider's /models endpoint via safeOutboundFetch →
// fetchWithTimeout, which binds globalThis.fetch at MODULE LOAD time. The mock MUST be
// installed BEFORE importing validation.ts — a late reassignment (inside a test) is
// ignored and the validator hits the real network instead. (That made the 401/403
// assertions pass only by coincidence — live chatgpt.com returns 401/403 — while the
// 200 case failed.) A mutable `nextResponse` lets each test vary the probe result, and
// `fetchCalls` proves the mocked probe ran rather than the live network.
// The validator probes the provider's /models endpoint via validationRead → safeOutboundFetch
// → fetchWithTimeout, which reads `globalThis.fetch` dynamically at CALL time (#7058 — routed
// through the proxy-aware patched fetch instead of a bypassing directHttpsRequest). Importing
// validation.ts pulls in the proxy-patch module, which installs its own `globalThis.fetch`
// exactly once at import time — so the mock must be (re)installed AFTER the import, not before,
// or the patch silently clobbers it. A mutable `nextResponse` lets each test vary the probe
// result, and `fetchCalls` proves the mocked probe ran rather than the live network.
let nextResponse: { status: number; body: string } = { status: 200, body: "{}" };
let fetchCalls = 0;

const { validateWebCookieProvider } = await import("../../src/lib/providers/validation.ts");

globalThis.fetch = (async () => {
fetchCalls++;
return new Response(nextResponse.body, {
Expand All @@ -18,8 +21,6 @@ globalThis.fetch = (async () => {
});
}) as typeof fetch;

const { validateWebCookieProvider } = await import("../../src/lib/providers/validation.ts");

function mockFetch(status: number, body: string) {
nextResponse = { status, body };
fetchCalls = 0;
Expand Down
98 changes: 98 additions & 0 deletions tests/unit/web-cookie-validation-proxy-7058.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
// Regression test for #7058 — zai-web (and every other entry-bearing web-cookie
// provider) never honored a configured HTTP/SOCKS proxy during connection-test /
// cookie validation.
//
// Root cause: validateWebCookieProvider() probed `${baseUrl}/models` via
// directHttpsRequest(), which hardcodes `bypassProxyPatch: true` — forcing
// safeOutboundFetch to use the pre-patch native fetch and skip proxy-context/
// env-var resolution entirely. That bypass was introduced in #3226 as a narrow,
// documented exception for a single NVIDIA NIM workaround
// (see tests/unit/proxy-bypass-scope-guard-3226.test.ts) but validateWebCookieProvider
// adopted it as its default transport from inception (#4023), silently extending the
// bypass to every web-cookie provider with a registry entry (zai-web among them).
//
// This test proves the cookie-validation probe reaches a local forward proxy
// (via a real CONNECT tunnel — the same mechanism undici uses for both HTTP and
// HTTPS targets) when one is configured via HTTP_PROXY, exactly like the
// specialty web-cookie validators (chatgpt-web, grok-web, ...) already do via
// validationRead/validationWrite.
import test from "node:test";
import assert from "node:assert/strict";
import http from "node:http";
import net from "node:net";

const { validateWebCookieProvider } = await import("../../src/lib/providers/validation.ts");
const { REGISTRY } = await import("../../open-sse/config/providerRegistry.ts");
const { clearDispatcherCache } = await import("../../open-sse/utils/proxyDispatcher.ts");

const zaiWebEntry = REGISTRY["zai-web"] as { baseUrl?: string } | undefined;
const ORIGINAL_BASE_URL = zaiWebEntry?.baseUrl;
const ORIGINAL_HTTP_PROXY = process.env.HTTP_PROXY;

test.after(() => {
if (zaiWebEntry && ORIGINAL_BASE_URL !== undefined) {
zaiWebEntry.baseUrl = ORIGINAL_BASE_URL;
}
if (ORIGINAL_HTTP_PROXY === undefined) {
delete process.env.HTTP_PROXY;
} else {
process.env.HTTP_PROXY = ORIGINAL_HTTP_PROXY;
}
clearDispatcherCache();
});

test("zai-web cookie validation routes through the configured HTTP_PROXY (#7058)", async () => {
assert.ok(zaiWebEntry, "zai-web must have a providerRegistry entry for this test to be meaningful");

// Stand-in for chat.z.ai's /models probe target.
const target = http.createServer((_req, res) => {
res.writeHead(200, { "content-type": "application/json" });
res.end("{}");
});
await new Promise<void>((resolve) => target.listen(0, () => resolve()));
const targetPort = (target.address() as net.AddressInfo).port;

// Minimal forward proxy that only speaks CONNECT (like a real corporate proxy) and
// always tunnels to the local target above, regardless of the requested host — this
// lets the "upstream" host be a non-resolvable placeholder without any real DNS
// dependency, while still proving the request actually reached the proxy.
let sawConnect = false;
const proxy = http.createServer((_req, res) => {
res.writeHead(501);
res.end("CONNECT only");
});
proxy.on("connect", (_req, socket) => {
sawConnect = true;
const upstream = net.connect(targetPort, "127.0.0.1", () => {
socket.write("HTTP/1.1 200 Connection Established\r\n\r\n");
upstream.pipe(socket);
socket.pipe(upstream);
});
upstream.on("error", () => socket.destroy());
socket.on("error", () => upstream.destroy());
});
await new Promise<void>((resolve) => proxy.listen(0, () => resolve()));
const proxyPort = (proxy.address() as net.AddressInfo).port;

// A non-local-looking hostname: isLocalAddress()/resolveProxyForRequest() force a
// direct connection for any 127.*/localhost/LAN target, which would defeat this test.
zaiWebEntry!.baseUrl = "http://zai-web-validation-probe-7058.invalid";
process.env.HTTP_PROXY = `http://127.0.0.1:${proxyPort}`;
clearDispatcherCache();

try {
const result = await validateWebCookieProvider({ provider: "zai-web", apiKey: "token=fake" });

assert.equal(
sawConnect,
true,
"BUG #7058: zai-web cookie validation never reached the configured HTTP_PROXY " +
"(bypassProxyPatch:true unconditionally uses the native, unpatched fetch)"
);
assert.equal(result.valid, true, `expected a valid session, got ${JSON.stringify(result)}`);
} finally {
target.close();
proxy.close();
clearDispatcherCache();
}
Comment on lines +47 to +97

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If proxy.listen(0) fails or any error occurs during the setup before the try block, the target server will remain open and leak its socket/port. Wrapping the server creation, listening, and testing inside a single try...finally block ensures that both target and proxy servers are always closed properly, preventing resource leaks in the test suite.

  // Stand-in for chat.z.ai's /models probe target.
  const target = http.createServer((_req, res) => {
    res.writeHead(200, { "content-type": "application/json" });
    res.end("{}");
  });

  // Minimal forward proxy that only speaks CONNECT (like a real corporate proxy) and
  // always tunnels to the local target above, regardless of the requested host — this
  // lets the "upstream" host be a non-resolvable placeholder without any real DNS
  // dependency, while still proving the request actually reached the proxy.
  const proxy = http.createServer((_req, res) => {
    res.writeHead(501);
    res.end("CONNECT only");
  });

  try {
    await new Promise<void>((resolve, reject) => {
      target.listen(0, () => resolve()).on("error", reject);
    });
    const targetPort = (target.address() as net.AddressInfo).port;

    let sawConnect = false;
    proxy.on("connect", (_req, socket) => {
      sawConnect = true;
      const upstream = net.connect(targetPort, "127.0.0.1", () => {
        socket.write("HTTP/1.1 200 Connection Established\r\n\r\n");
        upstream.pipe(socket);
        socket.pipe(upstream);
      });
      upstream.on("error", () => socket.destroy());
      socket.on("error", () => upstream.destroy());
    });

    await new Promise<void>((resolve, reject) => {
      proxy.listen(0, () => resolve()).on("error", reject);
    });
    const proxyPort = (proxy.address() as net.AddressInfo).port;

    // A non-local-looking hostname: isLocalAddress()/resolveProxyForRequest() force a
    // direct connection for any 127.*/localhost/LAN target, which would defeat this test.
    zaiWebEntry!.baseUrl = "http://zai-web-validation-probe-7058.invalid";
    process.env.HTTP_PROXY = "http://127.0.0.1:" + proxyPort;
    clearDispatcherCache();

    const result = await validateWebCookieProvider({ provider: "zai-web", apiKey: "token=fake" });

    assert.equal(
      sawConnect,
      true,
      "BUG #7058: zai-web cookie validation never reached the configured HTTP_PROXY " +
        "(bypassProxyPatch:true unconditionally uses the native, unpatched fetch)"
    );
    assert.equal(result.valid, true, "expected a valid session, got " + JSON.stringify(result));
  } finally {
    target.close();
    proxy.close();
    clearDispatcherCache();
  }

});
Loading