feat(server,web,core): explicit proxy address setting (no env vars needed) (#233)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Yaowei Zheng
@@ -1,7 +1,9 @@
|
||||
/**
|
||||
* Admin server-settings route tests: permission boundary (non-admin 403), the
|
||||
* "use system HTTP proxy" default (absent row reads as on), PUT persistence and
|
||||
* validation, and merge semantics (an omitted field keeps its current value).
|
||||
* Admin server-settings route tests: permission boundary (non-admin 403), the proxy
|
||||
* defaults (absent rows read as both switches on, no explicit address), adoption of the
|
||||
* legacy single-switch key, PUT persistence and validation (proxy-address normalization
|
||||
* and rejection), and merge semantics (an omitted field keeps its current value; a
|
||||
* rejected PUT writes nothing).
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import type { ServerSettingsResponse } from "../src/api/types.js";
|
||||
@@ -30,39 +32,135 @@ describe("admin server settings", () => {
|
||||
const { cookie } = await provisionUser(t.app, "norm");
|
||||
const api = apiClient(t.app, cookie);
|
||||
expect((await api.get("/api/admin/settings")).status).toBe(403);
|
||||
expect((await api.put("/api/admin/settings", { useSystemProxy: false })).status).toBe(403);
|
||||
expect((await api.put("/api/admin/settings", { proxyForApp: false })).status).toBe(403);
|
||||
// The failed PUT changed nothing.
|
||||
expect((await getSettings()).settings.useSystemProxy).toBe(true);
|
||||
expect((await getSettings()).settings.proxyForApp).toBe(true);
|
||||
});
|
||||
|
||||
it("useSystemProxy defaults to on while no row exists", async () => {
|
||||
it("defaults while no rows exist: both switches on, no explicit address", async () => {
|
||||
expect(t.deps.db.prepare("SELECT COUNT(*) AS n FROM server_settings").get()).toMatchObject({
|
||||
n: 0,
|
||||
});
|
||||
expect((await getSettings()).settings.useSystemProxy).toBe(true);
|
||||
const { settings } = await getSettings();
|
||||
expect(settings.proxyForApp).toBe(true);
|
||||
expect(settings.proxyForAgent).toBe(true);
|
||||
expect(settings.proxyUrl).toBeNull();
|
||||
});
|
||||
|
||||
it("PUT persists the toggle and echoes the full settings", async () => {
|
||||
const off = await admin.put("/api/admin/settings", { useSystemProxy: false });
|
||||
expect(off.status).toBe(200);
|
||||
expect(((await off.json()) as ServerSettingsResponse).settings.useSystemProxy).toBe(false);
|
||||
// Round-trips through the repo (the DB row, not process state, is what GET serves).
|
||||
expect((await getSettings()).settings.useSystemProxy).toBe(false);
|
||||
expect(t.deps.serverSettingsRepo.getUseSystemProxy()).toBe(false);
|
||||
it("adopts the legacy use_system_proxy row as the default for BOTH switches", async () => {
|
||||
// The single switch shipped only on unreleased main (#225): a deployment that had
|
||||
// toggled it off must keep that choice for both new switches, with no migration —
|
||||
// the legacy key is read as a fallback, never rewritten.
|
||||
t.deps.serverSettingsRepo.set("use_system_proxy", "false");
|
||||
let { settings } = await getSettings();
|
||||
expect(settings.proxyForApp).toBe(false);
|
||||
expect(settings.proxyForAgent).toBe(false);
|
||||
// Setting one NEW switch writes only its own key: the other still follows legacy.
|
||||
await admin.put("/api/admin/settings", { proxyForApp: true });
|
||||
({ settings } = await getSettings());
|
||||
expect(settings.proxyForApp).toBe(true);
|
||||
expect(settings.proxyForAgent).toBe(false);
|
||||
// The legacy row itself was never touched.
|
||||
expect(t.deps.serverSettingsRepo.get("use_system_proxy")).toBe("false");
|
||||
});
|
||||
|
||||
const on = await admin.put("/api/admin/settings", { useSystemProxy: true });
|
||||
expect(on.status).toBe(200);
|
||||
expect((await getSettings()).settings.useSystemProxy).toBe(true);
|
||||
it("PUT persists the switches independently and echoes the full settings", async () => {
|
||||
const appOff = await admin.put("/api/admin/settings", { proxyForApp: false });
|
||||
expect(appOff.status).toBe(200);
|
||||
const afterAppOff = ((await appOff.json()) as ServerSettingsResponse).settings;
|
||||
expect(afterAppOff.proxyForApp).toBe(false);
|
||||
expect(afterAppOff.proxyForAgent).toBe(true);
|
||||
// Round-trips through the repo (the DB rows, not process state, are what GET serves).
|
||||
expect(t.deps.serverSettingsRepo.getProxyForApp()).toBe(false);
|
||||
expect(t.deps.serverSettingsRepo.getProxyForAgent()).toBe(true);
|
||||
|
||||
const agentOff = await admin.put("/api/admin/settings", { proxyForAgent: false });
|
||||
expect(agentOff.status).toBe(200);
|
||||
const { settings } = await getSettings();
|
||||
expect(settings.proxyForApp).toBe(false);
|
||||
expect(settings.proxyForAgent).toBe(false);
|
||||
});
|
||||
|
||||
it("an omitted field keeps its current value; a non-boolean is 400", async () => {
|
||||
await admin.put("/api/admin/settings", { useSystemProxy: false });
|
||||
await admin.put("/api/admin/settings", { proxyForApp: false });
|
||||
// Empty PUT: no-op, still returns the current settings.
|
||||
const noop = await admin.put("/api/admin/settings", {});
|
||||
expect(noop.status).toBe(200);
|
||||
expect(((await noop.json()) as ServerSettingsResponse).settings.useSystemProxy).toBe(false);
|
||||
expect(((await noop.json()) as ServerSettingsResponse).settings.proxyForApp).toBe(false);
|
||||
// Type check: only booleans are accepted, and a rejected write changes nothing.
|
||||
expect((await admin.put("/api/admin/settings", { useSystemProxy: "on" })).status).toBe(400);
|
||||
expect((await getSettings()).settings.useSystemProxy).toBe(false);
|
||||
expect((await admin.put("/api/admin/settings", { proxyForApp: "on" })).status).toBe(400);
|
||||
expect((await admin.put("/api/admin/settings", { proxyForAgent: 1 })).status).toBe(400);
|
||||
const { settings } = await getSettings();
|
||||
expect(settings.proxyForApp).toBe(false);
|
||||
expect(settings.proxyForAgent).toBe(true);
|
||||
});
|
||||
|
||||
const putProxyUrl = async (proxyUrl: unknown) => admin.put("/api/admin/settings", { proxyUrl });
|
||||
|
||||
it("PUT normalizes the proxy address and stores/echoes only the normalized form", async () => {
|
||||
// Bare host[:port] shorthand → http://.
|
||||
const bare = await putProxyUrl("proxy.corp.example:8080");
|
||||
expect(bare.status).toBe(200);
|
||||
expect(((await bare.json()) as ServerSettingsResponse).settings.proxyUrl).toBe(
|
||||
"http://proxy.corp.example:8080",
|
||||
);
|
||||
expect((await getSettings()).settings.proxyUrl).toBe("http://proxy.corp.example:8080");
|
||||
expect(t.deps.serverSettingsRepo.getProxyUrl()).toBe("http://proxy.corp.example:8080");
|
||||
// https passes through; surrounding whitespace is trimmed.
|
||||
const https = await putProxyUrl(" https://proxy.corp.example:3128 ");
|
||||
expect(https.status).toBe(200);
|
||||
expect(((await https.json()) as ServerSettingsResponse).settings.proxyUrl).toBe(
|
||||
"https://proxy.corp.example:3128",
|
||||
);
|
||||
});
|
||||
|
||||
it("PUT rejects a bad proxy address with invalid_proxy_url and stores nothing", async () => {
|
||||
await putProxyUrl("http://proxy.corp.example:8080");
|
||||
for (const bad of ["socks5://proxy.corp.example:1080", "not a proxy", 42]) {
|
||||
const res = await putProxyUrl(bad);
|
||||
expect(res.status).toBe(400);
|
||||
const body = (await res.json()) as { error: { code: string } };
|
||||
expect(body.error.code).toBe("invalid_proxy_url");
|
||||
}
|
||||
expect((await getSettings()).settings.proxyUrl).toBe("http://proxy.corp.example:8080");
|
||||
});
|
||||
|
||||
it("a rejected combined PUT leaves the other fields untouched too", async () => {
|
||||
// Validation happens before any write: the valid switch halves of a PUT whose
|
||||
// proxyUrl is garbage must not land either — the PUT is atomic.
|
||||
const res = await admin.put("/api/admin/settings", {
|
||||
proxyForApp: false,
|
||||
proxyForAgent: false,
|
||||
proxyUrl: "not a proxy",
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
const { settings } = await getSettings();
|
||||
expect(settings.proxyForApp).toBe(true);
|
||||
expect(settings.proxyForAgent).toBe(true);
|
||||
expect(settings.proxyUrl).toBeNull();
|
||||
});
|
||||
|
||||
it("empty string and null both clear the address back to follow-the-environment", async () => {
|
||||
for (const clear of ["", " ", null]) {
|
||||
await putProxyUrl("http://proxy.corp.example:8080");
|
||||
const res = await putProxyUrl(clear);
|
||||
expect(res.status).toBe(200);
|
||||
expect(((await res.json()) as ServerSettingsResponse).settings.proxyUrl).toBeNull();
|
||||
expect(t.deps.serverSettingsRepo.getProxyUrl()).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it("partial PUTs keep the other fields: address-only keeps the switches and vice versa", async () => {
|
||||
await admin.put("/api/admin/settings", { proxyForApp: false, proxyForAgent: false });
|
||||
await putProxyUrl("proxy.corp.example:8080");
|
||||
let { settings } = await getSettings();
|
||||
expect(settings.proxyForApp).toBe(false);
|
||||
expect(settings.proxyForAgent).toBe(false);
|
||||
expect(settings.proxyUrl).toBe("http://proxy.corp.example:8080");
|
||||
await admin.put("/api/admin/settings", { proxyForApp: true, proxyForAgent: true });
|
||||
({ settings } = await getSettings());
|
||||
expect(settings.proxyForApp).toBe(true);
|
||||
expect(settings.proxyForAgent).toBe(true);
|
||||
expect(settings.proxyUrl).toBe("http://proxy.corp.example:8080");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,11 +1,20 @@
|
||||
/**
|
||||
* Unit tests for the outbound-proxy module's pure parts: the NO_PROXY loopback merge
|
||||
* and the dispatcher choice per switch state. No real network and no global install —
|
||||
* installGlobalProxyDispatcher (fetch replacement) runs only in the production entry.
|
||||
* Unit tests for the outbound-proxy module's pure parts — the NO_PROXY loopback merge,
|
||||
* proxy-address normalization, and the dispatcher choice per settings state — plus one
|
||||
* behavioral check that an explicit address actually routes traffic through that proxy
|
||||
* while loopback stays direct. No global install — installGlobalProxyDispatcher (fetch
|
||||
* replacement) runs only in the production entry.
|
||||
*/
|
||||
import http from "node:http";
|
||||
import type { AddressInfo } from "node:net";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { Agent, EnvHttpProxyAgent } from "undici";
|
||||
import { buildProxyDispatcher, mergedNoProxy } from "../src/net/proxy.js";
|
||||
import { Agent, EnvHttpProxyAgent, fetch as undiciFetch } from "undici";
|
||||
import {
|
||||
buildProxyDispatcher,
|
||||
envProxyAgentOptions,
|
||||
mergedNoProxy,
|
||||
normalizeProxyUrl,
|
||||
} from "../src/net/proxy.js";
|
||||
|
||||
describe("mergedNoProxy", () => {
|
||||
it("yields exactly the loopback names when the environment has no NO_PROXY", () => {
|
||||
@@ -35,17 +44,147 @@ describe("mergedNoProxy", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeProxyUrl", () => {
|
||||
it("passes canonical http/https addresses through", () => {
|
||||
expect(normalizeProxyUrl("http://proxy.corp.example:8080")).toBe(
|
||||
"http://proxy.corp.example:8080",
|
||||
);
|
||||
expect(normalizeProxyUrl("https://proxy.corp.example:3128")).toBe(
|
||||
"https://proxy.corp.example:3128",
|
||||
);
|
||||
expect(normalizeProxyUrl("http://proxy.corp.example")).toBe("http://proxy.corp.example");
|
||||
});
|
||||
|
||||
it("normalizes the bare host[:port] shorthand to http://", () => {
|
||||
expect(normalizeProxyUrl("proxy.corp.example:8080")).toBe("http://proxy.corp.example:8080");
|
||||
expect(normalizeProxyUrl("proxy.corp.example")).toBe("http://proxy.corp.example");
|
||||
expect(normalizeProxyUrl("127.0.0.1:7890")).toBe("http://127.0.0.1:7890");
|
||||
});
|
||||
|
||||
it("canonicalizes case, surrounding whitespace, trailing slash, and default ports", () => {
|
||||
expect(normalizeProxyUrl(" http://proxy.corp.example:8080 ")).toBe(
|
||||
"http://proxy.corp.example:8080",
|
||||
);
|
||||
expect(normalizeProxyUrl("HTTP://PROXY.CORP.EXAMPLE:8080")).toBe(
|
||||
"http://proxy.corp.example:8080",
|
||||
);
|
||||
expect(normalizeProxyUrl("http://proxy.corp.example:8080/")).toBe(
|
||||
"http://proxy.corp.example:8080",
|
||||
);
|
||||
// URL's canonical form drops a scheme-default port; the connection is identical.
|
||||
expect(normalizeProxyUrl("http://proxy.corp.example:80")).toBe("http://proxy.corp.example");
|
||||
});
|
||||
|
||||
it("keeps IPv6 literals bracketed", () => {
|
||||
expect(normalizeProxyUrl("http://[::1]:8080")).toBe("http://[::1]:8080");
|
||||
});
|
||||
|
||||
it("rejects everything that is not an http(s) host[:port]", () => {
|
||||
// Other schemes: the dispatcher speaks only HTTP(S) proxies.
|
||||
expect(normalizeProxyUrl("socks5://proxy.corp.example:1080")).toBeNull();
|
||||
expect(normalizeProxyUrl("ftp://proxy.corp.example")).toBeNull();
|
||||
// Credentials, paths, queries, fragments: a proxy address is host-only.
|
||||
expect(normalizeProxyUrl("http://user:pass@proxy.corp.example:8080")).toBeNull();
|
||||
expect(normalizeProxyUrl("http://proxy.corp.example:8080/path")).toBeNull();
|
||||
expect(normalizeProxyUrl("http://proxy.corp.example:8080?x=1")).toBeNull();
|
||||
expect(normalizeProxyUrl("http://proxy.corp.example:8080#frag")).toBeNull();
|
||||
// Unparseable shapes.
|
||||
expect(normalizeProxyUrl("http://")).toBeNull();
|
||||
expect(normalizeProxyUrl("not a proxy")).toBeNull();
|
||||
expect(normalizeProxyUrl("proxy.corp.example:port")).toBeNull();
|
||||
expect(normalizeProxyUrl("")).toBeNull();
|
||||
expect(normalizeProxyUrl(" ")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("envProxyAgentOptions", () => {
|
||||
it("without an explicit address: only the merged NO_PROXY (env drives the proxies)", () => {
|
||||
expect(envProxyAgentOptions(null, { NO_PROXY: "corp.example" })).toEqual({
|
||||
noProxy: "corp.example,localhost,127.0.0.1,::1",
|
||||
});
|
||||
});
|
||||
|
||||
it("with an explicit address: both httpProxy and httpsProxy pinned to it, NO_PROXY still merged", () => {
|
||||
// httpProxy/httpsProxy take precedence over the environment inside undici's
|
||||
// EnvHttpProxyAgent (its constructor reads `httpProxy ?? env...`), so the ambient
|
||||
// variables in `env` must not leak into the options this module assembles.
|
||||
expect(
|
||||
envProxyAgentOptions("http://explicit.example:3128", {
|
||||
HTTP_PROXY: "http://ambient.example:8080",
|
||||
NO_PROXY: "corp.example",
|
||||
}),
|
||||
).toEqual({
|
||||
httpProxy: "http://explicit.example:3128",
|
||||
httpsProxy: "http://explicit.example:3128",
|
||||
noProxy: "corp.example,localhost,127.0.0.1,::1",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildProxyDispatcher", () => {
|
||||
it("on → EnvHttpProxyAgent (proxy env honored), off → plain Agent (direct)", async () => {
|
||||
const on = buildProxyDispatcher(true, {});
|
||||
const off = buildProxyDispatcher(false, {});
|
||||
it("app switch off → plain Agent; on → EnvHttpProxyAgent, with or without an explicit address", async () => {
|
||||
const off = buildProxyDispatcher({ proxyForApp: false, proxyUrl: null }, {});
|
||||
const onEnv = buildProxyDispatcher({ proxyForApp: true, proxyUrl: null }, {});
|
||||
const onExplicit = buildProxyDispatcher(
|
||||
{ proxyForApp: true, proxyUrl: "http://explicit.example:3128" },
|
||||
{},
|
||||
);
|
||||
try {
|
||||
expect(on).toBeInstanceOf(EnvHttpProxyAgent);
|
||||
expect(off).toBeInstanceOf(Agent);
|
||||
expect(off).not.toBeInstanceOf(EnvHttpProxyAgent);
|
||||
expect(onEnv).toBeInstanceOf(EnvHttpProxyAgent);
|
||||
expect(onExplicit).toBeInstanceOf(EnvHttpProxyAgent);
|
||||
} finally {
|
||||
await on.close();
|
||||
await off.close();
|
||||
await onEnv.close();
|
||||
await onExplicit.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("explicit address: traffic tunnels through that proxy, loopback bypasses it", async () => {
|
||||
// The fake proxy answers CONNECT and then plays the tunneled origin itself, so a
|
||||
// request to an unresolvable name can only succeed by traveling through the proxy.
|
||||
const connects: string[] = [];
|
||||
const proxy = http.createServer((_req, res) => res.end());
|
||||
proxy.on("connect", (req, socket) => {
|
||||
connects.push(req.url ?? "");
|
||||
socket.write("HTTP/1.1 200 Connection Established\r\n\r\n");
|
||||
let buffered = "";
|
||||
socket.on("data", (chunk: Buffer) => {
|
||||
buffered += chunk.toString("utf8");
|
||||
if (!buffered.includes("\r\n\r\n")) return; // wait for the full tunneled request head
|
||||
socket.end(
|
||||
"HTTP/1.1 200 OK\r\ncontent-type: text/plain\r\ncontent-length: 9\r\nconnection: close\r\n\r\nvia-proxy",
|
||||
);
|
||||
});
|
||||
});
|
||||
const direct = http.createServer((_req, res) => res.end("direct"));
|
||||
const listen = (server: http.Server) =>
|
||||
new Promise<number>((resolve) => {
|
||||
// No hostname: dual-stack, so the localhost fetch below works whether the
|
||||
// resolver picks ::1 or 127.0.0.1 first.
|
||||
server.listen(0, () => resolve((server.address() as AddressInfo).port));
|
||||
});
|
||||
const proxyPort = await listen(proxy);
|
||||
const directPort = await listen(direct);
|
||||
// Empty env: no proxy variables — reaching the fake proxy proves the explicit URL
|
||||
// is honored on its own, and the loopback exemption proves the merge alone (not an
|
||||
// ambient NO_PROXY) produced it.
|
||||
const dispatcher = buildProxyDispatcher(
|
||||
{ proxyForApp: true, proxyUrl: `http://127.0.0.1:${proxyPort}` },
|
||||
{},
|
||||
);
|
||||
try {
|
||||
const viaProxy = await undiciFetch("http://proxied.invalid/x", { dispatcher });
|
||||
expect(await viaProxy.text()).toBe("via-proxy");
|
||||
expect(connects).toEqual(["proxied.invalid:80"]);
|
||||
const directRes = await undiciFetch(`http://localhost:${directPort}/`, { dispatcher });
|
||||
expect(await directRes.text()).toBe("direct");
|
||||
expect(connects).toHaveLength(1); // the loopback request never touched the proxy
|
||||
} finally {
|
||||
await dispatcher.close();
|
||||
proxy.close();
|
||||
direct.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user