From d72e9c3f7c9afa4241e64fa9af801b5e44ad7c3a Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:19:23 +0200 Subject: [PATCH 1/4] Send the account login through the system proxy The login request went through requestBoundedTextViaNode, the raw Node https transport the auth service requires (#76), which never read the system proxy or HTTP_PROXY/HTTPS_PROXY. Every other launcher request already inherited the proxy through Electron's net.request, so a network that requires one only broke login. Resolves the proxy for the login URL through Electron's own session (session.defaultSession.resolveProxy), the same source net.request reads from, and tunnels an HTTP proxy answer with a plain CONNECT before writing the request. DIRECT keeps the prior path byte for byte. HTTPS_PROXY/HTTP_PROXY are read as a fallback only when the session itself answers DIRECT, with NO_PROXY respected. A SOCKS answer, an HTTPS-secured proxy answer, or a proxy that answers a CONNECT with 407 all fail the login up front with a fixed reason token (proxy-unsupported, proxy-auth-required) instead of the generic failure; both join the network-unreachable family, whose sentence already tells a player to check a proxy. Every log line stays a fixed token, never the resolved host. --- src/domain/net/proxy.ts | 37 +++ src/ipc/handlers/loginFailureReason.ts | 20 +- src/ipc/network.ts | 261 ++++++++++++++++-- tests/domain/net/proxy.test.ts | 46 +++ tests/ipc/helpers/electronMock.ts | 21 +- tests/ipc/loginFailureReason.test.ts | 8 +- tests/ipc/networkProxy.test.ts | 231 ++++++++++++++++ .../sessionButtonFailureFamilies.test.tsx | 26 ++ 8 files changed, 617 insertions(+), 33 deletions(-) create mode 100644 src/domain/net/proxy.ts create mode 100644 tests/domain/net/proxy.test.ts create mode 100644 tests/ipc/networkProxy.test.ts diff --git a/src/domain/net/proxy.ts b/src/domain/net/proxy.ts new file mode 100644 index 00000000..6d93ff10 --- /dev/null +++ b/src/domain/net/proxy.ts @@ -0,0 +1,37 @@ +/** + * Reads one answer from Electron's `session.resolveProxy(url)` (issue #481). + * + * Chromium answers with a PAC-style, semicolon-separated list, most-preferred + * entry first: `"PROXY host:port"`, `"HTTPS host:port"`, `"SOCKS5 host:port"`, + * `"SOCKS4 host:port"` or `"DIRECT"`. Only the first entry is ever read: a + * fallback entry exists for when the first is unreachable, which is a retry + * policy this launcher does not implement, so acting on it would silently + * try a proxy the OS itself only offered as a second choice. + * + * `"HTTPS host:port"` names a proxy reached over its own TLS connection, + * which the login transport has no tunnel for (`src/ipc/network.ts` only + * speaks a plain CONNECT to the proxy itself); it comes back `unsupported`, + * the same as a SOCKS entry the launcher also does not tunnel through, and + * the same as anything that fails to parse. `socks` still gets its own + * shape rather than folding into `unsupported` too: this function's job is + * to say what the OS answered, and a SOCKS entry is not garbage, only + * unimplemented. + */ +export type ProxyResolution = { kind: "direct" } | { kind: "http"; host: string; port: number } | { kind: "socks"; host: string; port: number } | { kind: "unsupported" } + +const PROXY_ENTRY = /^(PROXY|SOCKS4|SOCKS5|SOCKS)\s+([^\s:]+):(\d{1,5})$/i + +export function parseProxyResolution(text: string): ProxyResolution { + const first = text.split(";")[0]?.trim() ?? "" + if (first === "" || /^direct$/i.test(first)) return { kind: "direct" } + + const match = PROXY_ENTRY.exec(first) + if (!match) return { kind: "unsupported" } + + const scheme = match[1] ?? "" + const host = match[2] ?? "" + const port = Number(match[3]) + if (host === "" || !Number.isInteger(port) || port < 1 || port > 65_535) return { kind: "unsupported" } + + return scheme.toUpperCase() === "PROXY" ? { kind: "http", host, port } : { kind: "socks", host, port } +} diff --git a/src/ipc/handlers/loginFailureReason.ts b/src/ipc/handlers/loginFailureReason.ts index 4bd18042..3da7c6e3 100644 --- a/src/ipc/handlers/loginFailureReason.ts +++ b/src/ipc/handlers/loginFailureReason.ts @@ -66,7 +66,14 @@ export class AccountStorageFailure extends Error { export const NETWORK_MESSAGES = new Map([ ["Network request timed out", "timeout"], ["Network response is too large", "response-too-large"], - ["Network response was aborted", "response-aborted"] + ["Network response was aborted", "response-aborted"], + // The system proxy login now goes through (issue #481): Chromium's own proxy resolution + // carries no credentials for this transport to answer a 407 with, and a SOCKS (or otherwise + // unroutable) proxy answer has no client here. Both are thrown by + // `requestBoundedTextViaNode` before or during the CONNECT tunnel, never after, so neither can + // be confused with a refusal from the auth service itself. + ["Login proxy requires authentication", "proxy-auth-required"], + ["Login proxy is not supported", "proxy-unsupported"] ]) /** The literals `assertSecureStorage` throws, reached through {@link AccountStorageFailure}. */ @@ -247,6 +254,13 @@ export type LoginFailureFamily = "network-unreachable" | "certificate-error" | " * connection, or a round trip that ran out of time. This is also where a * proxied network without the proxy configured lands, so the sentence this * family picks is the one that tells a player to check one. + * + * `proxy-auth-required` and `proxy-unsupported` (issue #481) join it for the + * same reason: both mean the login never reached the service either, only + * for a proxy-shaped cause this launcher cannot resolve on its own (a proxy + * asking for credentials nothing here can supply, or a SOCKS/unrouted answer + * with no client for it), and the family's own sentence already names a + * proxy as something to check. */ const NETWORK_UNREACHABLE_REASONS = new Set([ "timeout", @@ -265,7 +279,9 @@ const NETWORK_UNREACHABLE_REASONS = new Set([ "network-ENETDOWN", "network-EPROTO", "network-ERR_SOCKET_CONNECTION_TIMEOUT", - "network-ERR_STREAM_PREMATURE_CLOSE" + "network-ERR_STREAM_PREMATURE_CLOSE", + "proxy-auth-required", + "proxy-unsupported" ]) /** The TLS codes {@link NETWORK_CODES} lists: a certificate this machine will not accept. */ diff --git a/src/ipc/network.ts b/src/ipc/network.ts index 6f90b0c5..ce8abb78 100644 --- a/src/ipc/network.ts +++ b/src/ipc/network.ts @@ -1,10 +1,29 @@ -import { net } from "electron" -import { request as httpRequest } from "node:http" +import { net, session } from "electron" +import { Agent, request as httpRequest } from "node:http" import { request as httpsRequest } from "node:https" +import type { IncomingMessage } from "node:http" +import type { Socket } from "node:net" +import { connect as tlsConnect } from "node:tls" +import { parseProxyResolution } from "@domain/net/proxy" +import type { ProxyResolution } from "@domain/net/proxy" import { MAX_RESPONSE_BYTES } from "@src/ipc/validation" +import { logMessage } from "@src/utils/logManager" const REQUEST_TIMEOUT_MS = 15_000 +/** + * The two literals the login proxy path throws, read back by + * `src/ipc/handlers/loginFailureReason.ts` (issue #481) so the renderer can + * tell a player a proxy is the reason, without either literal ever carrying + * a host: Chromium's `resolveProxy` answer never carries proxy credentials, + * so a 407 is the only answer a login through a proxy can give for one, and + * a SOCKS (or otherwise unrouted) proxy is refused before a socket is ever + * opened, for the same reason `requestBoundedTextViaNode` never got a SOCKS + * client: nothing in this codebase speaks that protocol. + */ +const PROXY_AUTH_REQUIRED_MESSAGE = "Login proxy requires authentication" +const PROXY_UNSUPPORTED_MESSAGE = "Login proxy is not supported" + const DEFAULT_ACCEPT_HEADER = "application/json, text/plain;q=0.9" type BoundedRequestOptions = { @@ -165,12 +184,24 @@ export function requestBoundedBuffer(url: URL, options: BoundedRequestOptions = * `timeoutMs` exists only so tests can trip the timeout branch without a real * 15-second wait; no production caller sets it, so every real call still gets * `REQUEST_TIMEOUT_MS`. + * + * Also honours the system proxy (issue #481), which this transport never + * used to read: `session.defaultSession.resolveProxy` is asked for `url` + * first, the same question Electron's own `net.request` answers itself for + * every other call in this file. An HTTP proxy answer is tunneled through + * with a plain CONNECT (see {@link connectThroughProxy}); `DIRECT` keeps + * this function's byte-for-byte prior behaviour, falling back to + * `HTTPS_PROXY`/`HTTP_PROXY` (`NO_PROXY` respected) only when the session + * itself found nothing to use; a SOCKS answer, an HTTPS-secured proxy answer, + * or anything else this transport cannot route through fails the request up + * front with {@link PROXY_UNSUPPORTED_MESSAGE}, before a socket is ever + * opened. The whole decision, and the tunnel handshake, sit inside the same + * `timeoutMs` wall clock this function already bounded every request with. */ export function requestBoundedTextViaNode(url: URL, options: BoundedRequestOptions & { timeoutMs?: number } = {}): Promise { const method = options.method ?? "GET" const maxBytes = options.maxBytes ?? MAX_RESPONSE_BYTES const timeoutMs = options.timeoutMs ?? REQUEST_TIMEOUT_MS - const transport = url.protocol === "http:" ? httpRequest : httpsRequest const headers: Record = { Accept: options.accept ?? DEFAULT_ACCEPT_HEADER } if (options.body !== undefined) headers["Content-Type"] = "application/x-www-form-urlencoded" @@ -179,19 +210,39 @@ export function requestBoundedTextViaNode(url: URL, options: BoundedRequestOptio let settled = false let responseBytes = 0 const chunks: Buffer[] = [] + // Whichever request is in flight right now (the CONNECT, or the real one), so the + // one timeout below can abort it without knowing which phase it landed in. + let abortInFlight = (): void => {} - const request = transport(url, { method, headers }, (response) => { + const finish = (error?: Error): void => { + if (settled) return + settled = true + clearTimeout(timeout) + + if (error) { + reject(error) + } else { + resolve(Buffer.concat(chunks).toString("utf8")) + } + } + + const timeout = setTimeout(() => { + abortInFlight() + finish(new Error("Network request timed out")) + }, timeoutMs) + + function onResponse(response: IncomingMessage): void { const contentLengthHeader = response.headers["content-length"] const contentLength = Number(contentLengthHeader) if (Number.isFinite(contentLength) && contentLength > maxBytes) { - request.destroy() + abortInFlight() finish(new Error("Network response is too large")) return } if (response.statusCode === undefined || response.statusCode < 200 || response.statusCode >= 300) { - request.destroy() + abortInFlight() finish(new Error(`Network request failed with status ${response.statusCode ?? "unknown"}`)) return } @@ -201,7 +252,7 @@ export function requestBoundedTextViaNode(url: URL, options: BoundedRequestOptio responseBytes += chunkBuffer.length if (responseBytes > maxBytes) { - request.destroy() + abortInFlight() finish(new Error("Network response is too large")) return } @@ -211,31 +262,185 @@ export function requestBoundedTextViaNode(url: URL, options: BoundedRequestOptio response.on("end", () => finish()) response.on("aborted", () => finish(new Error("Network response was aborted"))) response.on("error", (error) => finish(error)) - }) - - const timeout = setTimeout(() => { - request.destroy() - finish(new Error("Network request timed out")) - }, timeoutMs) - - const finish = (error?: Error): void => { - if (settled) return - settled = true - clearTimeout(timeout) + } - if (error) { - reject(error) - } else { - resolve(Buffer.concat(chunks).toString("utf8")) + // A tunneled request is always sent with node:http's own `request`, `agent` and all, + // never `https.request`: the agent already hands back a socket doing TLS on its own + // for an `https:` target (see connectThroughProxy), and a second TLS layer on top is + // not what `https.request` would give it anyway. The URL itself is never passed + // through in that case either: a `URL` carries its own `protocol`, and `http.request` + // refuses one that disagrees with the agent's ("https:" against `TunnelAgent`'s + // inherited "http:"), so the pieces `http.request` actually needs are read off it by + // hand instead. The direct branch is untouched: same call as before this issue. + function send(agent?: Agent): void { + const request = agent + ? httpRequest({ hostname: url.hostname, port: url.port || (url.protocol === "http:" ? 80 : 443), path: `${url.pathname}${url.search}`, method, headers, agent }, onResponse) + : (url.protocol === "http:" ? httpRequest : httpsRequest)(url, { method, headers }, onResponse) + + abortInFlight = (): void => { + request.destroy() } + request.on("error", (error) => finish(error)) + if (options.body !== undefined) request.end(options.body) + else request.end() } - request.on("error", (error) => finish(error)) + decideProxy(url) + .then((decision) => { + if (settled) return + if (decision.kind === "blocked") return finish(decision.error) + if (decision.kind === "direct") return send() + + return connectThroughProxy(decision.proxy, url, (abort) => { + abortInFlight = abort + }).then((agent) => { + if (!settled) send(agent) + }) + }) + .catch((error: unknown) => finish(error instanceof Error ? error : new Error(String(error)))) + }) +} - if (options.body !== undefined) { - request.end(options.body) - } else { - request.end() - } +/** What {@link requestBoundedTextViaNode} does once it knows the proxy for `url`. */ +type ProxyDecision = { kind: "direct" } | { kind: "tunnel"; proxy: { host: string; port: number } } | { kind: "blocked"; error: Error } + +/** + * Asks Electron's default session how `url` should be reached, the same + * question `net.request` answers for itself, and turns the answer into what + * {@link requestBoundedTextViaNode} does next. + * + * A session that fails to answer (no window ever created one, or some + * Electron-internal error) is treated as `DIRECT`: a proxy question this + * transport cannot even ask is not a reason to refuse a login that used to + * work before this issue existed. + * + * Every branch logs exactly one fixed token and nothing else, never the + * host `url` names or the proxy resolved for it (issue #481): `proxy-used` + * and `proxy-direct` here; a blocked resolution logs nothing of its own; its + * `proxy-unsupported` (or a CONNECT 407's `proxy-auth-required`) reaches the + * log the same way every other login failure does, through the reason + * `src/ipc/handlers/loginFailureReason.ts` classifies the thrown error into. + */ +async function decideProxy(url: URL): Promise { + const pacAnswer = await session.defaultSession.resolveProxy(url.toString()).catch(() => "DIRECT") + const resolution = parseProxyResolution(pacAnswer) + const effective = resolution.kind === "direct" ? (environmentProxyResolution(url.hostname) ?? resolution) : resolution + + if (effective.kind === "direct") { + logMessage("debug", "[back] [ipc] [network.ts] [PROXY] proxy-direct") + return { kind: "direct" } + } + if (effective.kind === "http") { + logMessage("debug", "[back] [ipc] [network.ts] [PROXY] proxy-used") + return { kind: "tunnel", proxy: { host: effective.host, port: effective.port } } + } + // socks, or a shape this transport does not recognise: neither has a client here. + return { kind: "blocked", error: new Error(PROXY_UNSUPPORTED_MESSAGE) } +} + +/** + * `HTTPS_PROXY`/`HTTP_PROXY`, read only when the session itself answered + * `DIRECT`: Node's `http(s).request` never consults either variable on its + * own, unlike `net.request`, which is why this transport needed one at all. + * `NO_PROXY` is checked first and wins outright, matching curl and every + * other tool that honours the trio. A value that fails to parse as a URL, + * same as nothing set at all, leaves the caller on the direct path rather + * than failing a login over a malformed environment variable. + */ +function environmentProxyResolution(targetHost: string): ProxyResolution | undefined { + if (hostMatchesNoProxy(targetHost, process.env.NO_PROXY ?? process.env.no_proxy)) return undefined + + const raw = process.env.HTTPS_PROXY ?? process.env.https_proxy ?? process.env.HTTP_PROXY ?? process.env.http_proxy + if (!raw) return undefined + + try { + const proxyUrl = new URL(raw) + return parseProxyResolution(`PROXY ${proxyUrl.hostname}:${proxyUrl.port || "80"}`) + } catch { + return undefined + } +} + +/** `NO_PROXY=a.example.com,.b.example.com,*`: an exact host, a domain suffix (leading dot optional), or `*` for every host. */ +function hostMatchesNoProxy(host: string, noProxy: string | undefined): boolean { + if (!noProxy) return false + const target = host.toLowerCase() + + return noProxy + .split(",") + .map((entry) => entry.trim().toLowerCase().replace(/^\./, "")) + .filter(Boolean) + .some((entry) => entry === "*" || target === entry || target.endsWith(`.${entry}`)) +} + +/** + * An `http.Agent` that hands back one already-established socket instead of + * ever opening a connection of its own: the tunnel {@link connectThroughProxy} + * just built. `http.request` (never `https.request`, even for an `https:` + * target) is what this agent is meant for: once the socket it returns is + * already doing TLS (see {@link connectThroughProxy}), the request built on + * top of it only ever writes plaintext HTTP/1.1 onto a stream, and that + * stream encrypts on the way out on its own. + */ +class TunnelAgent extends Agent { + private readonly socket: Socket + + constructor(socket: Socket) { + super({ keepAlive: false }) + this.socket = socket + } + + override createConnection(): Socket { + return this.socket + } +} + +/** + * Opens the login request's actual connection through an HTTP proxy with a + * CONNECT tunnel: `node:http`'s own `request` sends the CONNECT, and once the + * proxy answers 200 the raw socket it hands back either is the connection + * (a plain-`http` test target) or gets wrapped in TLS to the real origin (an + * `https:` target, every real login). No new dependency: `node:tls`'s + * `connect({ socket })` upgrading an already-open socket, and the tiny + * {@link TunnelAgent} above, are both standard library. + * + * `onAbort` is handed the one thing worth cancelling at each point in time, + * so `requestBoundedTextViaNode`'s single timeout can reach whichever phase + * is actually in flight without knowing which one that is. + * + * A 407 is the only status this maps by itself, to {@link PROXY_AUTH_REQUIRED_MESSAGE}: + * Chromium's proxy resolution never carries credentials for this transport to send back, + * so an authenticated proxy cannot be satisfied and the player is told plainly rather than + * left on a generic connection failure. Known limit, not a bug: there is no prompt for a + * proxy password anywhere in this launcher. + */ +function connectThroughProxy(proxy: { host: string; port: number }, url: URL, onAbort: (abort: () => void) => void): Promise { + const targetPort = Number(url.port) || (url.protocol === "http:" ? 80 : 443) + + return new Promise((resolve, reject) => { + const connectRequest = httpRequest({ + host: proxy.host, + port: proxy.port, + method: "CONNECT", + path: `${url.hostname}:${targetPort}`, + headers: { Host: `${url.hostname}:${targetPort}` } + }) + + onAbort(() => connectRequest.destroy()) + connectRequest.on("error", reject) + connectRequest.on("connect", (response, socket) => { + if (response.statusCode === 200) { + resolve(socket) + return + } + + socket.destroy() + reject(response.statusCode === 407 ? new Error(PROXY_AUTH_REQUIRED_MESSAGE) : new Error(`Login proxy CONNECT failed with status ${response.statusCode ?? "unknown"}`)) + }) + connectRequest.end() + }).then((rawSocket) => { + const tunneledSocket = url.protocol === "http:" ? rawSocket : tlsConnect({ socket: rawSocket, servername: url.hostname }) + onAbort(() => tunneledSocket.destroy()) + return new TunnelAgent(tunneledSocket) }) } diff --git a/tests/domain/net/proxy.test.ts b/tests/domain/net/proxy.test.ts new file mode 100644 index 00000000..ee06a8e6 --- /dev/null +++ b/tests/domain/net/proxy.test.ts @@ -0,0 +1,46 @@ +import assert from "node:assert/strict" +import { describe, it } from "vitest" + +import { parseProxyResolution } from "@domain/net/proxy" + +describe("parseProxyResolution reads one session.resolveProxy answer (#481)", () => { + it("reads DIRECT, case and whitespace insensitive", () => { + assert.deepEqual(parseProxyResolution("DIRECT"), { kind: "direct" }) + assert.deepEqual(parseProxyResolution("direct"), { kind: "direct" }) + assert.deepEqual(parseProxyResolution(" DIRECT "), { kind: "direct" }) + }) + + it("reads a PROXY entry as an http proxy", () => { + assert.deepEqual(parseProxyResolution("PROXY 10.0.0.1:8080"), { kind: "http", host: "10.0.0.1", port: 8080 }) + assert.deepEqual(parseProxyResolution("proxy office-proxy.local:3128"), { kind: "http", host: "office-proxy.local", port: 3128 }) + }) + + it("reads a SOCKS4 or SOCKS5 entry as socks, unimplemented but recognised", () => { + assert.deepEqual(parseProxyResolution("SOCKS5 10.0.0.1:1080"), { kind: "socks", host: "10.0.0.1", port: 1080 }) + assert.deepEqual(parseProxyResolution("SOCKS4 10.0.0.1:1080"), { kind: "socks", host: "10.0.0.1", port: 1080 }) + assert.deepEqual(parseProxyResolution("SOCKS 10.0.0.1:1080"), { kind: "socks", host: "10.0.0.1", port: 1080 }) + }) + + it("only reads the first entry, tolerant of a fallback list and stray whitespace", () => { + assert.deepEqual(parseProxyResolution("PROXY 10.0.0.1:8080; DIRECT"), { kind: "http", host: "10.0.0.1", port: 8080 }) + assert.deepEqual(parseProxyResolution(" PROXY 10.0.0.1:8080 ;SOCKS5 10.0.0.2:1080"), { kind: "http", host: "10.0.0.1", port: 8080 }) + assert.deepEqual(parseProxyResolution("DIRECT; PROXY 10.0.0.1:8080"), { kind: "direct" }) + }) + + it("cannot tunnel an HTTPS-secured proxy, so it comes back unsupported like a truly unknown scheme", () => { + assert.deepEqual(parseProxyResolution("HTTPS 10.0.0.1:443"), { kind: "unsupported" }) + assert.deepEqual(parseProxyResolution("QUIC 10.0.0.1:443"), { kind: "unsupported" }) + }) + + it("treats garbage and out-of-range ports as unsupported rather than throwing", () => { + assert.deepEqual(parseProxyResolution("not a proxy answer"), { kind: "unsupported" }) + assert.deepEqual(parseProxyResolution("PROXY 10.0.0.1"), { kind: "unsupported" }) + assert.deepEqual(parseProxyResolution("PROXY 10.0.0.1:99999"), { kind: "unsupported" }) + assert.deepEqual(parseProxyResolution("PROXY 10.0.0.1:0"), { kind: "unsupported" }) + }) + + it("treats an empty answer as direct rather than throwing", () => { + assert.deepEqual(parseProxyResolution(""), { kind: "direct" }) + assert.deepEqual(parseProxyResolution(" "), { kind: "direct" }) + }) +}) diff --git a/tests/ipc/helpers/electronMock.ts b/tests/ipc/helpers/electronMock.ts index 0d4c3ad5..57f5a5f0 100644 --- a/tests/ipc/helpers/electronMock.ts +++ b/tests/ipc/helpers/electronMock.ts @@ -23,7 +23,10 @@ export { createTrustedEvent, createUntrustedEvent } from "./trustedEvent" */ const DEFAULT_APP_VERSION = "0.0.0-test" -const state = { userDataPath: "", appVersion: DEFAULT_APP_VERSION } +// "DIRECT" so `requestBoundedTextViaNode` (src/ipc/network.ts, issue #481) keeps behaving +// exactly as it did before it started asking, in every test that never calls +// `setElectronProxyResolution` itself. +const state = { userDataPath: "", appVersion: DEFAULT_APP_VERSION, proxyResolution: "DIRECT" } const namedPaths: Record = {} /** @@ -68,6 +71,16 @@ export function setElectronPath(name: "appData" | "home" | "appRoot", path: stri namedPaths[name] = path } +/** + * Points `session.defaultSession.resolveProxy(url)` at `answer`, the PAC-style string Electron's + * real session answers with (`"DIRECT"`, `"PROXY host:port"`, `"SOCKS5 host:port"`, ...). Tests for + * `requestBoundedTextViaNode`'s proxy support (issue #481) set this in place of a real Chromium + * session, which nothing under `tests/` runs. + */ +export function setElectronProxyResolution(answer: string): void { + state.proxyResolution = answer +} + /** * Stands in for the `electron` module so a main-process adapter can be imported * under plain Node instead of a running Electron process. @@ -137,5 +150,9 @@ vi.mock("electron", () => { const dialog = { showSaveDialog: vi.fn(), showOpenDialog: vi.fn() } const shell = { showItemInFolder: vi.fn(), openPath: vi.fn(), openExternal: vi.fn() } - return { app, ipcMain, dialog, shell } + // `resolveProxy` is the one `session` member `requestBoundedTextViaNode` reads (issue #481); + // nothing else on a real `Session` is touched by anything under test. + const session = { defaultSession: { resolveProxy: async (): Promise => state.proxyResolution } } + + return { app, ipcMain, dialog, shell, session } }) diff --git a/tests/ipc/loginFailureReason.test.ts b/tests/ipc/loginFailureReason.test.ts index d30a8980..a605e80b 100644 --- a/tests/ipc/loginFailureReason.test.ts +++ b/tests/ipc/loginFailureReason.test.ts @@ -27,7 +27,9 @@ describe("loginFailureReason names what went wrong", () => { for (const [message, expected] of [ ["Network request timed out", "timeout"], ["Network response is too large", "response-too-large"], - ["Network response was aborted", "response-aborted"] + ["Network response was aborted", "response-aborted"], + ["Login proxy requires authentication", "proxy-auth-required"], + ["Login proxy is not supported", "proxy-unsupported"] ] as const) { it(`reads "${message}" as ${expected}`, () => { assert.equal(loginFailureReason(new Error(message)), expected) @@ -228,6 +230,10 @@ describe("loginFailureFamily places every token loginFailureReason can emit", () "network-EPROTO": "network-unreachable", "network-ERR_SOCKET_CONNECTION_TIMEOUT": "network-unreachable", "network-ERR_STREAM_PREMATURE_CLOSE": "network-unreachable", + // A proxy-shaped cause the login transport cannot resolve on its own (#481): same family, + // since the request never reached the service either way. + "proxy-auth-required": "network-unreachable", + "proxy-unsupported": "network-unreachable", // A certificate this machine would not accept. "network-CERT_HAS_EXPIRED": "certificate-error", "network-CERT_NOT_YET_VALID": "certificate-error", diff --git a/tests/ipc/networkProxy.test.ts b/tests/ipc/networkProxy.test.ts new file mode 100644 index 00000000..f66cb2ca --- /dev/null +++ b/tests/ipc/networkProxy.test.ts @@ -0,0 +1,231 @@ +import assert from "node:assert/strict" +import { createServer as createHttpServer } from "node:http" +import type { IncomingMessage, Server } from "node:http" +import { connect as netConnect } from "node:net" +import type { Socket } from "node:net" +import { afterEach, describe, it } from "vitest" + +import "./helpers/electronMock" +import { setElectronProxyResolution } from "./helpers/electronMock" + +import { requestBoundedTextViaNode } from "@src/ipc/network" + +/** + * `requestBoundedTextViaNode`'s system-proxy support (issue #481): the login + * transport now asks Electron's session for the proxy before opening a + * socket, tunnels an HTTP proxy answer with a plain CONNECT, and refuses a + * proxy shape it cannot route through before ever touching the network. + * + * Every server here (origin and proxy alike) is a plain, unencrypted + * `node:http`/`node:net` server on 127.0.0.1: the tunnel is exercised with an + * `http:` target URL, the one case `requestBoundedTextViaNode` itself already + * skips the TLS wrap for (see its `url.protocol === "http:"` check), which + * proves the CONNECT-and-relay mechanics without a certificate anywhere in + * the test. Every real login target is `https:`, where the same code path + * wraps the tunnelled socket in TLS before the request is written. + */ + +const openSockets = new Set() +let origin: Server | undefined +let proxy: Server | undefined + +afterEach(async () => { + for (const socket of openSockets) socket.destroy() + openSockets.clear() + setElectronProxyResolution("DIRECT") + delete process.env.HTTPS_PROXY + delete process.env.HTTP_PROXY + delete process.env.NO_PROXY + + for (const server of [origin, proxy]) { + if (!server) continue + await new Promise((resolve) => server.close(() => resolve())) + } + origin = undefined + proxy = undefined +}) + +function trackSockets(server: Server): void { + server.on("connection", (socket) => { + openSockets.add(socket) + socket.on("close", () => openSockets.delete(socket)) + }) +} + +function startOrigin(handler: (req: IncomingMessage, res: import("node:http").ServerResponse) => void): Promise { + return new Promise((resolve) => { + origin = createHttpServer(handler) + trackSockets(origin) + origin.listen(0, "127.0.0.1", () => { + const address = origin?.address() + if (address === null || typeof address !== "object") throw new Error("Origin server failed to bind") + resolve(new URL(`http://127.0.0.1:${address.port}/`)) + }) + }) +} + +/** A CONNECT-accepting proxy that relays the tunnel to the real target, and records every CONNECT target it saw. */ +function startRelayProxy(): Promise<{ port: number; connectTargets: string[] }> { + return new Promise((resolve) => { + const connectTargets: string[] = [] + proxy = createHttpServer() + trackSockets(proxy) + proxy.on("connect", (req, clientSocket, head) => { + connectTargets.push(req.url ?? "") + const [host, portText] = (req.url ?? "").split(":") + const upstream = netConnect(Number(portText), host) + openSockets.add(upstream) + upstream.on("close", () => openSockets.delete(upstream)) + upstream.on("connect", () => { + clientSocket.write("HTTP/1.1 200 Connection Established\r\n\r\n") + if (head.length > 0) upstream.write(head) + upstream.pipe(clientSocket) + clientSocket.pipe(upstream) + }) + upstream.on("error", () => clientSocket.destroy()) + clientSocket.on("error", () => upstream.destroy()) + }) + proxy.listen(0, "127.0.0.1", () => { + const address = proxy?.address() + if (address === null || typeof address !== "object") throw new Error("Proxy server failed to bind") + resolve({ port: address.port, connectTargets }) + }) + }) +} + +/** A CONNECT-accepting proxy that never tunnels: it always answers with `status`, e.g. a 407. */ +function startRefusingProxy(status: number): Promise<{ port: number; connectTargets: string[] }> { + return new Promise((resolve) => { + const connectTargets: string[] = [] + proxy = createHttpServer() + trackSockets(proxy) + proxy.on("connect", (req, clientSocket) => { + connectTargets.push(req.url ?? "") + clientSocket.end(`HTTP/1.1 ${status} Proxy Refused\r\n\r\n`) + }) + proxy.listen(0, "127.0.0.1", () => { + const address = proxy?.address() + if (address === null || typeof address !== "object") throw new Error("Proxy server failed to bind") + resolve({ port: address.port, connectTargets }) + }) + }) +} + +describe("requestBoundedTextViaNode reaches the login host through an HTTP proxy (#481)", () => { + it("tunnels the request through the proxy and gets the real response back", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200, { "Content-Type": "text/plain" }) + res.end("ok") + }) + const relay = await startRelayProxy() + setElectronProxyResolution(`PROXY 127.0.0.1:${relay.port}`) + + const result = await requestBoundedTextViaNode(url) + + assert.equal(result, "ok") + assert.deepEqual(relay.connectTargets, [`127.0.0.1:${url.port}`]) + }) + + it("posts the same body and headers through the tunnel as it would send directly", async () => { + let receivedBody = "" + let receivedHeaders: IncomingMessage["headers"] = {} + const url = await startOrigin((req, res) => { + receivedHeaders = req.headers + const chunks: Buffer[] = [] + req.on("data", (chunk: Buffer) => chunks.push(chunk)) + req.on("end", () => { + receivedBody = Buffer.concat(chunks).toString("utf8") + res.writeHead(200) + res.end("ok") + }) + }) + const relay = await startRelayProxy() + setElectronProxyResolution(`PROXY 127.0.0.1:${relay.port}`) + + await requestBoundedTextViaNode(url, { method: "POST", body: "a=b" }) + + assert.equal(receivedBody, "a=b") + assert.equal(receivedHeaders["content-type"], "application/x-www-form-urlencoded") + assert.equal(receivedHeaders["accept"], "application/json, text/plain;q=0.9") + }) +}) + +describe("requestBoundedTextViaNode maps a proxy CONNECT refusal to a fixed reason (#481)", () => { + it("maps a 407 to the proxy-auth-required reason, never reaching the origin", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("should never be reached") + }) + const refusing = await startRefusingProxy(407) + setElectronProxyResolution(`PROXY 127.0.0.1:${refusing.port}`) + + await assert.rejects(requestBoundedTextViaNode(url), /Login proxy requires authentication/) + assert.deepEqual(refusing.connectTargets, [`127.0.0.1:${url.port}`]) + }) +}) + +describe("requestBoundedTextViaNode refuses a SOCKS proxy up front (#481)", () => { + it("rejects with the proxy-unsupported reason before opening any socket", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("should never be reached") + }) + setElectronProxyResolution("SOCKS5 127.0.0.1:1080") + + await assert.rejects(requestBoundedTextViaNode(url), /Login proxy is not supported/) + }) +}) + +describe("requestBoundedTextViaNode keeps the direct path when the session answers DIRECT (#481)", () => { + it("bypasses the tunnel entirely, unchanged from before this issue", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("ok") + }) + setElectronProxyResolution("DIRECT") + + assert.equal(await requestBoundedTextViaNode(url), "ok") + }) +}) + +describe("requestBoundedTextViaNode falls back to HTTPS_PROXY only when the session found nothing (#481)", () => { + it("uses HTTPS_PROXY when the session answers DIRECT", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("ok") + }) + const relay = await startRelayProxy() + setElectronProxyResolution("DIRECT") + process.env.HTTPS_PROXY = `http://127.0.0.1:${relay.port}` + + const result = await requestBoundedTextViaNode(url) + + assert.equal(result, "ok") + assert.deepEqual(relay.connectTargets, [`127.0.0.1:${url.port}`]) + }) + + it("still bypasses HTTPS_PROXY for a host NO_PROXY names", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("ok") + }) + setElectronProxyResolution("DIRECT") + // Nothing is listening on this port: if NO_PROXY failed to bypass it, the request would + // fail to connect instead of succeeding. + process.env.HTTPS_PROXY = "http://127.0.0.1:1" + process.env.NO_PROXY = "127.0.0.1" + + assert.equal(await requestBoundedTextViaNode(url), "ok") + }) + + it("stays on the direct path when HTTPS_PROXY is not a valid URL", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("ok") + }) + setElectronProxyResolution("DIRECT") + process.env.HTTPS_PROXY = "not a url" + + assert.equal(await requestBoundedTextViaNode(url), "ok") + }) +}) diff --git a/tests/renderer-dom/sessionButtonFailureFamilies.test.tsx b/tests/renderer-dom/sessionButtonFailureFamilies.test.tsx index 17f09628..11496351 100644 --- a/tests/renderer-dom/sessionButtonFailureFamilies.test.tsx +++ b/tests/renderer-dom/sessionButtonFailureFamilies.test.tsx @@ -50,6 +50,32 @@ describe("SessionButton names the family a login request failure belongs to", () }) } + /** + * The proxy half of #481: a login that failed because of a proxy the main process has no + * client for (an authenticated proxy answering 407, or a SOCKS answer) is classified as + * `proxy-auth-required`/`proxy-unsupported` in the main process, which join the + * `network-unreachable` family the renderer already had a sentence for (issue #482). + * Both reason tokens collapse into the same wire status before they ever cross the IPC + * boundary, so what a renderer test can prove is this: the sentence the family already + * shows names a proxy, not just a connection or a firewall. en-US and fr-FR both carry the + * wording already, so no new string was needed for either. + */ + it("mentions a proxy in the network-unreachable sentence, the family a proxy failure joins", async () => { + const login = vi.fn(async () => ({ status: "network-unreachable" }) as AccountLoginResult) + installMockWindowApi({ accountManager: { login } }) + + renderWithProviders( + <> + + + + ) + + await submitLogin() + + expect(await screen.findByText(/proxy/i)).toBeTruthy() + }) + it("still shows the generic message for a request failure loginFailureFamily could not place", async () => { const login = vi.fn(async () => { throw new Error("Login failed") From a5d3dab7164a5437df7f72b483898c23a77c8280 Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:12:17 +0200 Subject: [PATCH 2/4] Keep the environment proxy's own scheme instead of flattening it to HTTP HTTPS_PROXY/HTTP_PROXY's fallback used to read a proxy URL and rebuild it as a bare "PROXY host:port" string, discarding the scheme it just parsed. An https proxy then received a plaintext CONNECT instead of one sent over its own TLS connection, and a socks5 URL was silently treated as an HTTP proxy rather than refused. parseProxyUrl (src/domain/net/proxy.ts) now reads that URL's scheme directly: http and https both come back tunnelable, with a new "https" kind marking a proxy connectThroughProxy must reach over TLS before it ever sends the CONNECT; socks comes back the same recognised-but-unimplemented shape a SOCKS PAC answer already gets. Chromium's own PAC answers are untouched, parseProxyResolution still refuses an "HTTPS" PAC entry the same as before. Covered in tests/ipc/networkProxy.test.ts with a self-signed certificate generated at test time (tests/ipc/helpers/tlsFixtures.ts, no new dependency): one test proves the https-proxy CONNECT is actually sent over TLS, the other proves a socks5 HTTPS_PROXY is refused up front instead of dialing out as if it were HTTP. --- src/domain/net/proxy.ts | 62 +++++++++++-- src/ipc/network.ts | 54 ++++++----- tests/ipc/helpers/tlsFixtures.ts | 155 +++++++++++++++++++++++++++++++ tests/ipc/networkProxy.test.ts | 84 +++++++++++++++-- 4 files changed, 312 insertions(+), 43 deletions(-) create mode 100644 tests/ipc/helpers/tlsFixtures.ts diff --git a/src/domain/net/proxy.ts b/src/domain/net/proxy.ts index 6d93ff10..81ad4ba9 100644 --- a/src/domain/net/proxy.ts +++ b/src/domain/net/proxy.ts @@ -8,16 +8,21 @@ * policy this launcher does not implement, so acting on it would silently * try a proxy the OS itself only offered as a second choice. * - * `"HTTPS host:port"` names a proxy reached over its own TLS connection, - * which the login transport has no tunnel for (`src/ipc/network.ts` only - * speaks a plain CONNECT to the proxy itself); it comes back `unsupported`, - * the same as a SOCKS entry the launcher also does not tunnel through, and - * the same as anything that fails to parse. `socks` still gets its own - * shape rather than folding into `unsupported` too: this function's job is - * to say what the OS answered, and a SOCKS entry is not garbage, only - * unimplemented. + * `"HTTPS host:port"` names a proxy reached over its own TLS connection. + * `src/ipc/network.ts` can tunnel one (see {@link parseProxyUrl}, used on the + * `HTTPS_PROXY`/`HTTP_PROXY` fallback path), but Chromium's own answer for it + * comes back `unsupported` here regardless: a PAC answer never carries more + * than the host and port, and this function's job is only to say what the OS + * answered, not to guess at a scheme it was never told to trust for this + * particular proxy. `socks` still gets its own shape rather than folding into + * `unsupported` too: a SOCKS entry is not garbage, only unimplemented. */ -export type ProxyResolution = { kind: "direct" } | { kind: "http"; host: string; port: number } | { kind: "socks"; host: string; port: number } | { kind: "unsupported" } +export type ProxyResolution = + | { kind: "direct" } + | { kind: "http"; host: string; port: number } + | { kind: "https"; host: string; port: number } + | { kind: "socks"; host: string; port: number } + | { kind: "unsupported" } const PROXY_ENTRY = /^(PROXY|SOCKS4|SOCKS5|SOCKS)\s+([^\s:]+):(\d{1,5})$/i @@ -35,3 +40,42 @@ export function parseProxyResolution(text: string): ProxyResolution { return scheme.toUpperCase() === "PROXY" ? { kind: "http", host, port } : { kind: "socks", host, port } } + +/** + * Reads one `HTTPS_PROXY`/`HTTP_PROXY`-style URL (issue #481's environment fallback, + * `src/ipc/network.ts`'s `environmentProxyResolution`), keeping the scheme the URL + * itself names instead of discarding it: an `http:` or `https:` proxy URL both come + * back tunnelable (`connectThroughProxy` reaches an `https:` one over its own TLS + * connection before ever sending the CONNECT), and a `socks:`/`socks4:`/`socks5:` one + * comes back as the same recognised-but-unimplemented `socks` shape + * {@link parseProxyResolution} gives a PAC SOCKS answer, rather than being silently + * folded into `http` the way string surgery on the hostname alone used to. Anything + * else that still parses as a URL (an unknown scheme) comes back `unsupported`; a + * value that fails to parse as a URL at all comes back `undefined`, the caller's cue + * to fall back to the direct path rather than fail a login over a malformed + * environment variable. + */ +export function parseProxyUrl(raw: string): ProxyResolution | undefined { + let proxyUrl: URL + try { + proxyUrl = new URL(raw) + } catch { + return undefined + } + + const host = proxyUrl.hostname + if (host === "") return undefined + + switch (proxyUrl.protocol) { + case "http:": + return { kind: "http", host, port: Number(proxyUrl.port) || 80 } + case "https:": + return { kind: "https", host, port: Number(proxyUrl.port) || 443 } + case "socks:": + case "socks4:": + case "socks5:": + return { kind: "socks", host, port: Number(proxyUrl.port) || 1080 } + default: + return { kind: "unsupported" } + } +} diff --git a/src/ipc/network.ts b/src/ipc/network.ts index ce8abb78..6134f2f0 100644 --- a/src/ipc/network.ts +++ b/src/ipc/network.ts @@ -4,7 +4,7 @@ import { request as httpsRequest } from "node:https" import type { IncomingMessage } from "node:http" import type { Socket } from "node:net" import { connect as tlsConnect } from "node:tls" -import { parseProxyResolution } from "@domain/net/proxy" +import { parseProxyResolution, parseProxyUrl } from "@domain/net/proxy" import type { ProxyResolution } from "@domain/net/proxy" import { MAX_RESPONSE_BYTES } from "@src/ipc/validation" import { logMessage } from "@src/utils/logManager" @@ -301,8 +301,8 @@ export function requestBoundedTextViaNode(url: URL, options: BoundedRequestOptio }) } -/** What {@link requestBoundedTextViaNode} does once it knows the proxy for `url`. */ -type ProxyDecision = { kind: "direct" } | { kind: "tunnel"; proxy: { host: string; port: number } } | { kind: "blocked"; error: Error } +/** What {@link requestBoundedTextViaNode} does once it knows the proxy for `url`. `secure` marks a proxy reached over its own TLS connection (an `https:` proxy URL), as opposed to a plain CONNECT. */ +type ProxyDecision = { kind: "direct" } | { kind: "tunnel"; proxy: { host: string; port: number; secure: boolean } } | { kind: "blocked"; error: Error } /** * Asks Electron's default session how `url` should be reached, the same @@ -324,15 +324,15 @@ type ProxyDecision = { kind: "direct" } | { kind: "tunnel"; proxy: { host: strin async function decideProxy(url: URL): Promise { const pacAnswer = await session.defaultSession.resolveProxy(url.toString()).catch(() => "DIRECT") const resolution = parseProxyResolution(pacAnswer) - const effective = resolution.kind === "direct" ? (environmentProxyResolution(url.hostname) ?? resolution) : resolution + const effective = resolution.kind === "direct" ? (environmentProxyResolution(url) ?? resolution) : resolution if (effective.kind === "direct") { logMessage("debug", "[back] [ipc] [network.ts] [PROXY] proxy-direct") return { kind: "direct" } } - if (effective.kind === "http") { + if (effective.kind === "http" || effective.kind === "https") { logMessage("debug", "[back] [ipc] [network.ts] [PROXY] proxy-used") - return { kind: "tunnel", proxy: { host: effective.host, port: effective.port } } + return { kind: "tunnel", proxy: { host: effective.host, port: effective.port, secure: effective.kind === "https" } } } // socks, or a shape this transport does not recognise: neither has a client here. return { kind: "blocked", error: new Error(PROXY_UNSUPPORTED_MESSAGE) } @@ -343,28 +343,23 @@ async function decideProxy(url: URL): Promise { * `DIRECT`: Node's `http(s).request` never consults either variable on its * own, unlike `net.request`, which is why this transport needed one at all. * `NO_PROXY` is checked first and wins outright, matching curl and every - * other tool that honours the trio. A value that fails to parse as a URL, - * same as nothing set at all, leaves the caller on the direct path rather - * than failing a login over a malformed environment variable. + * other tool that honours the trio. Parsing itself, scheme included, is + * {@link parseProxyUrl}'s job (`src/domain/net/proxy.ts`): this function only + * reads the environment and decides whether `NO_PROXY` bypasses it. */ -function environmentProxyResolution(targetHost: string): ProxyResolution | undefined { - if (hostMatchesNoProxy(targetHost, process.env.NO_PROXY ?? process.env.no_proxy)) return undefined +function environmentProxyResolution(url: URL): ProxyResolution | undefined { + if (hostMatchesNoProxy(url, process.env.NO_PROXY ?? process.env.no_proxy)) return undefined const raw = process.env.HTTPS_PROXY ?? process.env.https_proxy ?? process.env.HTTP_PROXY ?? process.env.http_proxy if (!raw) return undefined - try { - const proxyUrl = new URL(raw) - return parseProxyResolution(`PROXY ${proxyUrl.hostname}:${proxyUrl.port || "80"}`) - } catch { - return undefined - } + return parseProxyUrl(raw) } /** `NO_PROXY=a.example.com,.b.example.com,*`: an exact host, a domain suffix (leading dot optional), or `*` for every host. */ -function hostMatchesNoProxy(host: string, noProxy: string | undefined): boolean { +function hostMatchesNoProxy(url: URL, noProxy: string | undefined): boolean { if (!noProxy) return false - const target = host.toLowerCase() + const target = url.hostname.toLowerCase() return noProxy .split(",") @@ -396,13 +391,15 @@ class TunnelAgent extends Agent { } /** - * Opens the login request's actual connection through an HTTP proxy with a - * CONNECT tunnel: `node:http`'s own `request` sends the CONNECT, and once the - * proxy answers 200 the raw socket it hands back either is the connection - * (a plain-`http` test target) or gets wrapped in TLS to the real origin (an - * `https:` target, every real login). No new dependency: `node:tls`'s - * `connect({ socket })` upgrading an already-open socket, and the tiny - * {@link TunnelAgent} above, are both standard library. + * Opens the login request's actual connection through an HTTP or HTTPS proxy + * with a CONNECT tunnel: `proxy.secure` picks `node:https`'s `request` over + * `node:http`'s to send the CONNECT itself over its own TLS connection to the + * proxy (an `https:` proxy URL, issue #481's `HTTPS_PROXY` scheme fix), and + * once the proxy answers 200 the raw socket it hands back either is the + * connection (a plain-`http` test target) or gets wrapped in a second, separate + * TLS layer to the real origin (an `https:` target, every real login). No new + * dependency: `node:tls`'s `connect({ socket })` upgrading an already-open + * socket, and the tiny {@link TunnelAgent} above, are both standard library. * * `onAbort` is handed the one thing worth cancelling at each point in time, * so `requestBoundedTextViaNode`'s single timeout can reach whichever phase @@ -414,11 +411,12 @@ class TunnelAgent extends Agent { * left on a generic connection failure. Known limit, not a bug: there is no prompt for a * proxy password anywhere in this launcher. */ -function connectThroughProxy(proxy: { host: string; port: number }, url: URL, onAbort: (abort: () => void) => void): Promise { +function connectThroughProxy(proxy: { host: string; port: number; secure: boolean }, url: URL, onAbort: (abort: () => void) => void): Promise { const targetPort = Number(url.port) || (url.protocol === "http:" ? 80 : 443) + const sendConnect = proxy.secure ? httpsRequest : httpRequest return new Promise((resolve, reject) => { - const connectRequest = httpRequest({ + const connectRequest = sendConnect({ host: proxy.host, port: proxy.port, method: "CONNECT", diff --git a/tests/ipc/helpers/tlsFixtures.ts b/tests/ipc/helpers/tlsFixtures.ts new file mode 100644 index 00000000..ffad88b9 --- /dev/null +++ b/tests/ipc/helpers/tlsFixtures.ts @@ -0,0 +1,155 @@ +import { vi } from "vitest" +import { generateKeyPairSync, sign as cryptoSign } from "node:crypto" + +/** + * Everything the proxy tests need to exercise real TLS: a self-signed certificate + * generated fresh at test time (issue #481's HTTPS-through-CONNECT coverage), and a + * way to make `src/ipc/network.ts` trust it for the one test that generated it, + * without touching that module's own call sites or the process-wide certificate + * store. `NODE_EXTRA_CA_CERTS` cannot do the latter: Node only reads it once, at + * process start, before a single line of test code has run, so setting it here would + * either do nothing or leak into every other test file sharing the process. + * `node:tls`'s `connect` and `node:https`'s `request` are what network.ts's tunnel + * wraps a socket with and what it CONNECTs to a secure proxy through, so mocking + * exactly those two, everything else forwarded untouched, gets a real handshake and a + * real certificate check against a CA only this file's tests ever set. + */ + +const trust = vi.hoisted(() => ({ ca: undefined as string | undefined })) + +vi.mock("node:tls", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + connect: (options: Record, ...rest: unknown[]): ReturnType => { + const merged = trust.ca === undefined ? options : { ...options, ca: trust.ca } + return (actual.connect as (...args: unknown[]) => ReturnType)(merged, ...rest) + } + } +}) + +vi.mock("node:https", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + request: (options: Record, ...rest: unknown[]): ReturnType => { + const merged = trust.ca === undefined ? options : { ...options, ca: trust.ca } + return (actual.request as (...args: unknown[]) => ReturnType)(merged, ...rest) + } + } +}) + +/** Trusts `pem` for every `node:tls`/`node:https` call this file mocks, until cleared. Call with no argument (an `afterEach` should) to stop trusting it. */ +export function setTrustedCa(pem?: string): void { + trust.ca = pem +} + +/** + * Builds a minimal DER encoder for exactly the ASN.1 shapes an X.509 certificate + * needs: no library, because Node ships no certificate-issuing API of its own, only + * `X509Certificate` to read one back. + */ +function derLength(n: number): Buffer { + if (n < 0x80) return Buffer.from([n]) + const bytes: number[] = [] + let v = n + while (v > 0) { + bytes.unshift(v & 0xff) + v = Math.floor(v / 256) + } + return Buffer.from([0x80 | bytes.length, ...bytes]) +} +function tlv(tag: number, content: Buffer): Buffer { + return Buffer.concat([Buffer.from([tag]), derLength(content.length), content]) +} +const seq = (...parts: Buffer[]): Buffer => tlv(0x30, Buffer.concat(parts)) +const set = (...parts: Buffer[]): Buffer => tlv(0x31, Buffer.concat(parts)) +function int(n: number): Buffer { + const bytes: number[] = [] + let v = n + if (v === 0) bytes.push(0) + while (v > 0) { + bytes.unshift(v & 0xff) + v = Math.floor(v / 256) + } + if ((bytes[0] ?? 0) & 0x80) bytes.unshift(0) + return tlv(0x02, Buffer.from(bytes)) +} +function oid(dotted: string): Buffer { + const parts = dotted.split(".").map(Number) + const bytes = [(parts[0] ?? 0) * 40 + (parts[1] ?? 0)] + for (const p of parts.slice(2)) { + if (p < 128) { + bytes.push(p) + continue + } + const chunk = [p & 0x7f] + let v = p >> 7 + while (v > 0) { + chunk.unshift((v & 0x7f) | 0x80) + v >>= 7 + } + bytes.push(...chunk) + } + return tlv(0x06, Buffer.from(bytes)) +} +const utf8String = (s: string): Buffer => tlv(0x0c, Buffer.from(s, "utf8")) +function utcTime(date: Date): Buffer { + const p2 = (n: number): string => String(n).padStart(2, "0") + const text = `${p2(date.getUTCFullYear() % 100)}${p2(date.getUTCMonth() + 1)}${p2(date.getUTCDate())}${p2(date.getUTCHours())}${p2(date.getUTCMinutes())}${p2(date.getUTCSeconds())}Z` + return tlv(0x17, Buffer.from(text, "ascii")) +} +const bitString = (content: Buffer): Buffer => tlv(0x03, Buffer.concat([Buffer.from([0]), content])) +const explicit = (n: number, content: Buffer): Buffer => tlv(0xa0 | n, content) +const octetString = (buf: Buffer): Buffer => tlv(0x04, buf) +const ipAddress = (ip: string): Buffer => tlv(0x87, Buffer.from(ip.split(".").map(Number))) +const dnsName = (value: string): Buffer => tlv(0x82, Buffer.from(value, "ascii")) + +const CN_OID = oid("2.5.4.3") +const name = (cn: string): Buffer => seq(set(seq(CN_OID, utf8String(cn)))) + +const ECDSA_SHA256_OID = oid("1.2.840.10045.4.3.2") +const algEcdsaSha256 = (): Buffer => seq(ECDSA_SHA256_OID) + +const SAN_OID = oid("2.5.29.17") +const extensionSAN = (altNames: Buffer[]): Buffer => seq(SAN_OID, octetString(seq(...altNames))) + +function toPem(der: Buffer, label: string): string { + const lines = der.toString("base64").match(/.{1,64}/g) ?? [] + return `-----BEGIN ${label}-----\n${lines.join("\n")}\n-----END ${label}-----\n` +} + +/** + * A fresh self-signed certificate for `127.0.0.1`/`localhost`, valid for the next day, + * built from an EC key pair signed with its own private key: subject and issuer are + * the same name, which is what makes a certificate self-signed rather than merely + * unsigned-by-anyone-trusted, and the only way `openssl verify -CAfile` (or Node's own + * chain check, handed this certificate as its `ca`) accepts it as its own root. + */ +export function createSelfSignedCert(): { cert: string; key: string } { + const { publicKey, privateKey } = generateKeyPairSync("ec", { namedCurve: "prime256v1" }) + const publicKeyDer = publicKey.export({ type: "spki", format: "der" }) + + const notBefore = new Date(Date.now() - 60_000) + const notAfter = new Date(Date.now() + 24 * 60 * 60 * 1000) + const serial = Math.floor(Math.random() * 1e9) + 1 + const subjectAndIssuer = name("127.0.0.1") + + const tbsCertificate = seq( + explicit(0, int(2)), + int(serial), + algEcdsaSha256(), + subjectAndIssuer, + seq(utcTime(notBefore), utcTime(notAfter)), + subjectAndIssuer, + publicKeyDer, + explicit(3, seq(extensionSAN([ipAddress("127.0.0.1"), dnsName("localhost")]))) + ) + const signature = cryptoSign("sha256", tbsCertificate, { key: privateKey }) + const certificate = seq(tbsCertificate, algEcdsaSha256(), bitString(signature)) + + return { + cert: toPem(certificate, "CERTIFICATE"), + key: privateKey.export({ type: "pkcs8", format: "pem" }) as string + } +} diff --git a/tests/ipc/networkProxy.test.ts b/tests/ipc/networkProxy.test.ts index f66cb2ca..40ead351 100644 --- a/tests/ipc/networkProxy.test.ts +++ b/tests/ipc/networkProxy.test.ts @@ -1,12 +1,16 @@ import assert from "node:assert/strict" import { createServer as createHttpServer } from "node:http" import type { IncomingMessage, Server } from "node:http" +import { createServer as createHttpsServer } from "node:https" +import type { Server as HttpsServer } from "node:https" import { connect as netConnect } from "node:net" import type { Socket } from "node:net" import { afterEach, describe, it } from "vitest" import "./helpers/electronMock" import { setElectronProxyResolution } from "./helpers/electronMock" +import "./helpers/tlsFixtures" +import { createSelfSignedCert, setTrustedCa } from "./helpers/tlsFixtures" import { requestBoundedTextViaNode } from "@src/ipc/network" @@ -16,36 +20,42 @@ import { requestBoundedTextViaNode } from "@src/ipc/network" * socket, tunnels an HTTP proxy answer with a plain CONNECT, and refuses a * proxy shape it cannot route through before ever touching the network. * - * Every server here (origin and proxy alike) is a plain, unencrypted - * `node:http`/`node:net` server on 127.0.0.1: the tunnel is exercised with an + * Most servers here (origin and proxy alike) are plain, unencrypted + * `node:http`/`node:net` servers on 127.0.0.1: the tunnel is exercised with an * `http:` target URL, the one case `requestBoundedTextViaNode` itself already * skips the TLS wrap for (see its `url.protocol === "http:"` check), which * proves the CONNECT-and-relay mechanics without a certificate anywhere in - * the test. Every real login target is `https:`, where the same code path - * wraps the tunnelled socket in TLS before the request is written. + * most of the tests. A handful reach for a self-signed certificate from + * `./helpers/tlsFixtures` instead, where the point under test is a real TLS + * connection: an `https:` proxy URL, or an `https:` target reached through + * the tunnel, both real logins hit and neither of which a plain server can + * stand in for. */ const openSockets = new Set() let origin: Server | undefined let proxy: Server | undefined +let secureProxy: HttpsServer | undefined afterEach(async () => { for (const socket of openSockets) socket.destroy() openSockets.clear() setElectronProxyResolution("DIRECT") + setTrustedCa(undefined) delete process.env.HTTPS_PROXY delete process.env.HTTP_PROXY delete process.env.NO_PROXY - for (const server of [origin, proxy]) { + for (const server of [origin, proxy, secureProxy]) { if (!server) continue await new Promise((resolve) => server.close(() => resolve())) } origin = undefined proxy = undefined + secureProxy = undefined }) -function trackSockets(server: Server): void { +function trackSockets(server: Server | HttpsServer): void { server.on("connection", (socket) => { openSockets.add(socket) socket.on("close", () => openSockets.delete(socket)) @@ -93,6 +103,35 @@ function startRelayProxy(): Promise<{ port: number; connectTargets: string[] }> }) } +/** Like {@link startRelayProxy}, but the proxy itself is reached over TLS with `cert`/`key`: an `https:` proxy URL. */ +function startSecureRelayProxy(cert: string, key: string): Promise<{ port: number; connectTargets: string[] }> { + return new Promise((resolve) => { + const connectTargets: string[] = [] + secureProxy = createHttpsServer({ cert, key }) + trackSockets(secureProxy) + secureProxy.on("connect", (req, clientSocket, head) => { + connectTargets.push(req.url ?? "") + const [host, portText] = (req.url ?? "").split(":") + const upstream = netConnect(Number(portText), host) + openSockets.add(upstream) + upstream.on("close", () => openSockets.delete(upstream)) + upstream.on("connect", () => { + clientSocket.write("HTTP/1.1 200 Connection Established\r\n\r\n") + if (head.length > 0) upstream.write(head) + upstream.pipe(clientSocket) + clientSocket.pipe(upstream) + }) + upstream.on("error", () => clientSocket.destroy()) + clientSocket.on("error", () => upstream.destroy()) + }) + secureProxy.listen(0, "127.0.0.1", () => { + const address = secureProxy?.address() + if (address === null || typeof address !== "object") throw new Error("Secure proxy server failed to bind") + resolve({ port: address.port, connectTargets }) + }) + }) +} + /** A CONNECT-accepting proxy that never tunnels: it always answers with `status`, e.g. a 407. */ function startRefusingProxy(status: number): Promise<{ port: number; connectTargets: string[] }> { return new Promise((resolve) => { @@ -229,3 +268,36 @@ describe("requestBoundedTextViaNode falls back to HTTPS_PROXY only when the sess assert.equal(await requestBoundedTextViaNode(url), "ok") }) }) + +describe("requestBoundedTextViaNode keeps HTTPS_PROXY's own scheme (#481)", () => { + it("reaches an https HTTPS_PROXY over its own TLS connection instead of a plaintext CONNECT", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("ok") + }) + const { cert, key } = createSelfSignedCert() + const relay = await startSecureRelayProxy(cert, key) + setElectronProxyResolution("DIRECT") + setTrustedCa(cert) + process.env.HTTPS_PROXY = `https://127.0.0.1:${relay.port}` + + const result = await requestBoundedTextViaNode(url) + + assert.equal(result, "ok") + assert.deepEqual(relay.connectTargets, [`127.0.0.1:${url.port}`]) + }) + + it("rejects a socks5 HTTPS_PROXY as unsupported rather than treating it as an HTTP proxy", async () => { + const url = await startOrigin((_req, res) => { + res.writeHead(200) + res.end("should never be reached") + }) + setElectronProxyResolution("DIRECT") + // Nothing is listening on this port: the old string-surgery fallback treated any + // HTTPS_PROXY as an HTTP proxy and would have tried to CONNECT through it, failing to + // connect instead of being refused up front. + process.env.HTTPS_PROXY = "socks5://127.0.0.1:1" + + await assert.rejects(requestBoundedTextViaNode(url), /Login proxy is not supported/) + }) +}) From bbaf089f1b304cfc470413834612b074e0de138d Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:16:46 +0200 Subject: [PATCH 3/4] Match NO_PROXY entries on host and port, not the bare hostname alone hostMatchesNoProxy only ever received the request's bare hostname, so a ported entry such as auth.vintagestory.at:443 never equalled it and the bypass never triggered; the login went through the proxy regardless of NO_PROXY. It is also pure decision logic with no host dependency, so it belongs in the domain layer next to the rest of this issue's proxy parsing, not in src/ipc/network.ts. Moved to src/domain/net/proxy.ts as matchesNoProxy(url, noProxy): an entry without a port still bypasses every port for that host, matching curl; an entry with one only bypasses the URL's own effective port (defaulted the same way a missing one already is elsewhere in this file). The wildcard, suffix and leading-dot behaviour, case-insensitivity and whitespace tolerance are unchanged. tests/domain/net/proxy.test.ts covers a bare host, a host with a port (matching and not, including a mismatched explicit port), a leading-dot suffix, the * entry, spaces around an entry, and a case difference, as direct unit tests now that the logic lives in domain rather than needing a running proxy to exercise. --- src/domain/net/proxy.ts | 34 +++++++++++++++++++++++++++++ src/ipc/network.ts | 23 +++++-------------- tests/domain/net/proxy.test.ts | 40 +++++++++++++++++++++++++++++++++- 3 files changed, 79 insertions(+), 18 deletions(-) diff --git a/src/domain/net/proxy.ts b/src/domain/net/proxy.ts index 81ad4ba9..87dd331c 100644 --- a/src/domain/net/proxy.ts +++ b/src/domain/net/proxy.ts @@ -79,3 +79,37 @@ export function parseProxyUrl(raw: string): ProxyResolution | undefined { return { kind: "unsupported" } } } + +/** + * `NO_PROXY=a.example.com,.b.example.com,*,auth.vintagestory.at:443` (issue #481's + * environment fallback, `src/ipc/network.ts`'s `environmentProxyResolution`): does + * `url` match any entry? An entry is an exact host, a domain suffix (leading dot + * optional), `*` for every host, or any of those with a `:port` suffix that narrows + * the bypass to that port alone. An entry without a port bypasses every port for that + * host, matching curl; an entry with one only bypasses `url`'s own effective port + * (its `URL.port`, defaulted the same way a missing one is defaulted elsewhere in + * this file: 80 for `http:`, 443 for `https:`), so `auth.vintagestory.at:443` matches + * `https://auth.vintagestory.at` even though neither ever spells the port out. + * Comparison is case-insensitive and tolerant of stray whitespace around an entry. + */ +export function matchesNoProxy(url: URL, noProxy: string | undefined): boolean { + if (!noProxy) return false + const targetHost = url.hostname.toLowerCase() + const targetPort = Number(url.port) || (url.protocol === "http:" ? 80 : 443) + + return noProxy + .split(",") + .map((entry) => entry.trim().toLowerCase()) + .filter(Boolean) + .some((entry) => { + if (entry === "*") return true + + const portSeparator = entry.lastIndexOf(":") + const entryPort = portSeparator === -1 ? undefined : Number(entry.slice(portSeparator + 1)) + const hasEntryPort = entryPort !== undefined && Number.isInteger(entryPort) && entryPort > 0 + if (hasEntryPort && entryPort !== targetPort) return false + + const entryHost = (hasEntryPort ? entry.slice(0, portSeparator) : entry).replace(/^\./, "") + return targetHost === entryHost || targetHost.endsWith(`.${entryHost}`) + }) +} diff --git a/src/ipc/network.ts b/src/ipc/network.ts index 6134f2f0..9f44d445 100644 --- a/src/ipc/network.ts +++ b/src/ipc/network.ts @@ -4,7 +4,7 @@ import { request as httpsRequest } from "node:https" import type { IncomingMessage } from "node:http" import type { Socket } from "node:net" import { connect as tlsConnect } from "node:tls" -import { parseProxyResolution, parseProxyUrl } from "@domain/net/proxy" +import { matchesNoProxy, parseProxyResolution, parseProxyUrl } from "@domain/net/proxy" import type { ProxyResolution } from "@domain/net/proxy" import { MAX_RESPONSE_BYTES } from "@src/ipc/validation" import { logMessage } from "@src/utils/logManager" @@ -343,12 +343,13 @@ async function decideProxy(url: URL): Promise { * `DIRECT`: Node's `http(s).request` never consults either variable on its * own, unlike `net.request`, which is why this transport needed one at all. * `NO_PROXY` is checked first and wins outright, matching curl and every - * other tool that honours the trio. Parsing itself, scheme included, is - * {@link parseProxyUrl}'s job (`src/domain/net/proxy.ts`): this function only - * reads the environment and decides whether `NO_PROXY` bypasses it. + * other tool that honours the trio; the match itself, port included, is + * {@link matchesNoProxy}'s job. Parsing the proxy URL, scheme included, is + * {@link parseProxyUrl}'s job (both `src/domain/net/proxy.ts`): this function + * only reads the environment and wires the two together. */ function environmentProxyResolution(url: URL): ProxyResolution | undefined { - if (hostMatchesNoProxy(url, process.env.NO_PROXY ?? process.env.no_proxy)) return undefined + if (matchesNoProxy(url, process.env.NO_PROXY ?? process.env.no_proxy)) return undefined const raw = process.env.HTTPS_PROXY ?? process.env.https_proxy ?? process.env.HTTP_PROXY ?? process.env.http_proxy if (!raw) return undefined @@ -356,18 +357,6 @@ function environmentProxyResolution(url: URL): ProxyResolution | undefined { return parseProxyUrl(raw) } -/** `NO_PROXY=a.example.com,.b.example.com,*`: an exact host, a domain suffix (leading dot optional), or `*` for every host. */ -function hostMatchesNoProxy(url: URL, noProxy: string | undefined): boolean { - if (!noProxy) return false - const target = url.hostname.toLowerCase() - - return noProxy - .split(",") - .map((entry) => entry.trim().toLowerCase().replace(/^\./, "")) - .filter(Boolean) - .some((entry) => entry === "*" || target === entry || target.endsWith(`.${entry}`)) -} - /** * An `http.Agent` that hands back one already-established socket instead of * ever opening a connection of its own: the tunnel {@link connectThroughProxy} diff --git a/tests/domain/net/proxy.test.ts b/tests/domain/net/proxy.test.ts index ee06a8e6..d1bdce30 100644 --- a/tests/domain/net/proxy.test.ts +++ b/tests/domain/net/proxy.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict" import { describe, it } from "vitest" -import { parseProxyResolution } from "@domain/net/proxy" +import { matchesNoProxy, parseProxyResolution } from "@domain/net/proxy" describe("parseProxyResolution reads one session.resolveProxy answer (#481)", () => { it("reads DIRECT, case and whitespace insensitive", () => { @@ -44,3 +44,41 @@ describe("parseProxyResolution reads one session.resolveProxy answer (#481)", () assert.deepEqual(parseProxyResolution(" "), { kind: "direct" }) }) }) + +describe("matchesNoProxy reads one NO_PROXY entry list (#481)", () => { + it("matches a bare host by exact name", () => { + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at"), "auth.vintagestory.at"), true) + assert.equal(matchesNoProxy(new URL("https://other.example.com"), "auth.vintagestory.at"), false) + }) + + it("matches a host with a port only when the target's own effective port agrees", () => { + // https://auth.vintagestory.at never spells out :443, but that is its effective port. + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at"), "auth.vintagestory.at:443"), true) + assert.equal(matchesNoProxy(new URL("http://auth.vintagestory.at"), "auth.vintagestory.at:443"), false) + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at:8443"), "auth.vintagestory.at:443"), false) + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at:443"), "auth.vintagestory.at:443"), true) + }) + + it("matches a domain suffix, leading dot optional", () => { + assert.equal(matchesNoProxy(new URL("https://api.vintagestory.at"), ".vintagestory.at"), true) + assert.equal(matchesNoProxy(new URL("https://api.vintagestory.at"), "vintagestory.at"), true) + assert.equal(matchesNoProxy(new URL("https://vintagestory.at.evil.com"), "vintagestory.at"), false) + }) + + it("matches every host on the * entry", () => { + assert.equal(matchesNoProxy(new URL("https://anything.example.com"), "*"), true) + }) + + it("tolerates spaces around an entry in a comma-separated list", () => { + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at"), "other.example.com, auth.vintagestory.at ,third.example.com"), true) + }) + + it("matches regardless of case", () => { + assert.equal(matchesNoProxy(new URL("https://Auth.VintageStory.at"), "AUTH.vintagestory.AT"), true) + }) + + it("returns false for an empty or unset NO_PROXY", () => { + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at"), undefined), false) + assert.equal(matchesNoProxy(new URL("https://auth.vintagestory.at"), ""), false) + }) +}) From ebb67ab425202f1ff44788c821be28d8e07425da Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:19:27 +0200 Subject: [PATCH 4/4] Cover an https target reached through a proxy's CONNECT tunnel Every existing proxy test used a plain http target, the one case requestBoundedTextViaNode already skips its TLS wrap for, so the wrap connectThroughProxy puts around a tunnelled socket for a real, https login was never exercised end to end. A break there would have shipped unnoticed. startSecureOrigin (tests/ipc/networkProxy.test.ts) runs a real https server on a self-signed certificate from tests/ipc/helpers/tlsFixtures.ts, trusted for this test only through the ca-merging mock that file installs on node:tls/node:https. The new test tunnels a POST through a plain HTTP proxy to that origin and checks the login body arrives intact on the far side of the TLS-wrapped tunnel, not just that a response comes back. Confirmed against a real regression: temporarily skipping the TLS wrap in connectThroughProxy made this test fail with the tunnelled socket's plain HTTP request landing on a TLS-only server, then passed again once the wrap was restored. --- tests/ipc/networkProxy.test.ts | 43 +++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/tests/ipc/networkProxy.test.ts b/tests/ipc/networkProxy.test.ts index 40ead351..2552974b 100644 --- a/tests/ipc/networkProxy.test.ts +++ b/tests/ipc/networkProxy.test.ts @@ -36,6 +36,7 @@ const openSockets = new Set() let origin: Server | undefined let proxy: Server | undefined let secureProxy: HttpsServer | undefined +let secureOrigin: HttpsServer | undefined afterEach(async () => { for (const socket of openSockets) socket.destroy() @@ -46,13 +47,14 @@ afterEach(async () => { delete process.env.HTTP_PROXY delete process.env.NO_PROXY - for (const server of [origin, proxy, secureProxy]) { + for (const server of [origin, proxy, secureProxy, secureOrigin]) { if (!server) continue await new Promise((resolve) => server.close(() => resolve())) } origin = undefined proxy = undefined secureProxy = undefined + secureOrigin = undefined }) function trackSockets(server: Server | HttpsServer): void { @@ -74,6 +76,19 @@ function startOrigin(handler: (req: IncomingMessage, res: import("node:http").Se }) } +/** Like {@link startOrigin}, but the origin itself is reached over TLS with `cert`/`key`: an `https:` target, the shape a real login always is. */ +function startSecureOrigin(cert: string, key: string, handler: (req: IncomingMessage, res: import("node:http").ServerResponse) => void): Promise { + return new Promise((resolve) => { + secureOrigin = createHttpsServer({ cert, key }, handler) + trackSockets(secureOrigin) + secureOrigin.listen(0, "127.0.0.1", () => { + const address = secureOrigin?.address() + if (address === null || typeof address !== "object") throw new Error("Secure origin server failed to bind") + resolve(new URL(`https://127.0.0.1:${address.port}/`)) + }) + }) +} + /** A CONNECT-accepting proxy that relays the tunnel to the real target, and records every CONNECT target it saw. */ function startRelayProxy(): Promise<{ port: number; connectTargets: string[] }> { return new Promise((resolve) => { @@ -301,3 +316,29 @@ describe("requestBoundedTextViaNode keeps HTTPS_PROXY's own scheme (#481)", () = await assert.rejects(requestBoundedTextViaNode(url), /Login proxy is not supported/) }) }) + +describe("requestBoundedTextViaNode reaches an https target through an HTTP proxy's CONNECT tunnel (#481)", () => { + it("wraps the tunnelled socket in TLS and posts the login body through it", async () => { + const { cert, key } = createSelfSignedCert() + let receivedBody = "" + const url = await startSecureOrigin(cert, key, (req, res) => { + const chunks: Buffer[] = [] + req.on("data", (chunk: Buffer) => chunks.push(chunk)) + req.on("end", () => { + receivedBody = Buffer.concat(chunks).toString("utf8") + res.writeHead(200, { "Content-Type": "text/plain" }) + res.end("ok") + }) + }) + const relay = await startRelayProxy() + setElectronProxyResolution(`PROXY 127.0.0.1:${relay.port}`) + setTrustedCa(cert) + + const body = "email=someone%40example.com&password=hunter2" + const result = await requestBoundedTextViaNode(url, { method: "POST", body }) + + assert.equal(result, "ok") + assert.equal(receivedBody, body) + assert.deepEqual(relay.connectTargets, [`127.0.0.1:${url.port}`]) + }) +})