diff --git a/changelog.d/fixes/8853-fix.plan.md b/changelog.d/fixes/8853-fix.plan.md new file mode 100644 index 0000000000..e43b144916 --- /dev/null +++ b/changelog.d/fixes/8853-fix.plan.md @@ -0,0 +1 @@ +- fix(proxy-health): include credentials in proxy health check URLs (#8853) \ No newline at end of file diff --git a/src/app/api/settings/proxies/auto-test/route.ts b/src/app/api/settings/proxies/auto-test/route.ts index d496d614d8..9023f4f909 100644 --- a/src/app/api/settings/proxies/auto-test/route.ts +++ b/src/app/api/settings/proxies/auto-test/route.ts @@ -2,7 +2,7 @@ import { z } from "zod"; import { deleteProxyById, listProxies, updateProxy } from "@/lib/localDb"; import { createErrorResponseFromUnknown } from "@/lib/api/errorResponse"; import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; -import { createProxyDispatcher } from "@omniroute/open-sse/utils/proxyDispatcher"; +import { createProxyDispatcher, proxyConfigToUrl } from "@omniroute/open-sse/utils/proxyDispatcher"; import { fetch as undiciFetch } from "undici"; import { resolveHealthCheckStatusWrite } from "@/lib/proxyHealth/statusPolicy"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; @@ -33,8 +33,26 @@ async function testSingleProxy(proxy: { type: string; host: string; port: number; + username?: string; + password?: string; + family?: string; }): Promise { - const proxyUrl = `${proxy.type}://${proxy.host}:${proxy.port}`; + let proxyUrl: string | null; + try { + proxyUrl = proxyConfigToUrl(proxy); + } catch { + proxyUrl = null; + } + if (!proxyUrl) { + return { + proxyId: proxy.id, + host: proxy.host, + port: proxy.port, + alive: false, + latencyMs: null, + error: "Invalid proxy config (check type, host, port)", + }; + } const start = Date.now(); const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), TEST_TIMEOUT_MS); @@ -99,7 +117,7 @@ export async function POST(request: Request) { const { ids: specificIds, autoRemove } = validation.data; try { - const result = await listProxies({ includeSecrets: false }); + const result = await listProxies({ includeSecrets: true }); const allProxies = result.items; const proxiesToTest = specificIds ? allProxies.filter((p) => specificIds.includes(p.id)) diff --git a/src/lib/proxyHealth/scheduler.ts b/src/lib/proxyHealth/scheduler.ts index 2febffb99b..54435e62f2 100644 --- a/src/lib/proxyHealth/scheduler.ts +++ b/src/lib/proxyHealth/scheduler.ts @@ -12,7 +12,7 @@ */ import { deleteProxyById, listProxies, updateProxy } from "@/lib/localDb"; -import { createProxyDispatcher, clearDispatcherCache } from "@omniroute/open-sse/utils/proxyDispatcher"; +import { createProxyDispatcher, clearDispatcherCache, proxyConfigToUrl } from "@omniroute/open-sse/utils/proxyDispatcher"; import { fetch as undiciFetch } from "undici"; import { decideProxyHealthAction, @@ -87,8 +87,17 @@ async function testOneProxy(proxy: { type: string; host: string; port: number; + username?: string; + password?: string; + family?: string; }): Promise { - const proxyUrl = `${proxy.type}://${proxy.host}:${proxy.port}`; + let proxyUrl: string | null; + try { + proxyUrl = proxyConfigToUrl(proxy); + } catch { + proxyUrl = null; + } + if (!proxyUrl) return "fail"; const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), TEST_TIMEOUT_MS); try { @@ -112,7 +121,7 @@ async function testOneProxy(proxy: { } async function sweep(): Promise { - const { items: proxies } = await listProxies({ includeSecrets: false }); + const { items: proxies } = await listProxies({ includeSecrets: true }); if (proxies.length === 0) return; const failureMap = getFailureMap(); diff --git a/tests/unit/triage-bugs-2026-08-02.test.ts b/tests/unit/triage-bugs-2026-08-02.test.ts new file mode 100644 index 0000000000..70127329f5 --- /dev/null +++ b/tests/unit/triage-bugs-2026-08-02.test.ts @@ -0,0 +1,120 @@ +/** + * #8853 — authenticated HTTP proxy health checks drop credentials + * + * Root cause: both the auto-test route and the scheduler build proxy URLs + * manually as `${proxy.type}://${proxy.host}:${proxy.port}`, dropping + * username/password. The `proxyConfigToUrl()` function in proxyDispatcher.ts + * already handles URL-encoded credentials correctly. + * + * We prove the bug by showing that the proxy URL produced by the current + * manual construction lacks credentials, and that `proxyConfigToUrl()` with + * the same config object includes them — therefore the fix is to reuse it. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +// The function that fixes the bug — we import it here to verify it works +import { proxyConfigToUrl } from "@omniroute/open-sse/utils/proxyDispatcher"; + +// ── proxyConfigToUrl credential tests ────────────────────────────────────── + +test("#8853 proxyConfigToUrl encodes username and password into proxy URL", () => { + const url = proxyConfigToUrl({ + type: "http", + host: "127.0.0.1", + port: 3128, + username: "alice", + password: "s3cret", + }); + assert.ok(url, "proxyConfigToUrl must return a URL"); + assert.match(url!, /:\/\/alice:s3cret@/, "URL must contain credentials"); +}); + +test("#8853 proxyConfigToUrl encodes special characters in credentials", () => { + const url = proxyConfigToUrl({ + type: "http", + host: "proxy.example.com", + port: 8080, + username: "user@domain", + password: "p@ss:w0rd", + }); + assert.ok(url, "proxyConfigToUrl must return a URL"); + assert.match(url!, /:\/\/user%40domain:p%40ss%3Aw0rd@/, "URL must URL-encode special chars"); +}); + +test("#8853 proxyConfigToUrl omits auth when no username", () => { + const url = proxyConfigToUrl({ + type: "http", + host: "127.0.0.1", + port: 3128, + }); + assert.ok(url, "proxyConfigToUrl must return a URL"); + assert.doesNotMatch(url!, /@/, "URL must not contain @ (no auth)"); +}); + +test("#8853 proxyConfigToUrl handles IPv6 host with family", () => { + const url = proxyConfigToUrl({ + type: "http", + host: "[::1]", + port: 3128, + family: "ipv6", + }); + assert.ok(url, "proxyConfigToUrl must return a URL"); + assert.match(url!, /\[::1\]/, "IPv6 host must be bracketed"); +}); + +// ── Simulate the buggy construction ───────────────────────────────────────── + +function buggyManualUrl(proxy: { type: string; host: string; port: number }) { + return `${proxy.type}://${proxy.host}:${proxy.port}`; +} + +test("#8853 manual URL construction (current bug) drops credentials", () => { + const proxy = { + type: "http", + host: "127.0.0.1", + port: 3128, + username: "alice", + password: "s3cret", + }; + const manualUrl = buggyManualUrl(proxy); + assert.doesNotMatch(manualUrl, /alice/, "Buggy URL must NOT contain username"); + assert.doesNotMatch(manualUrl, /s3cret/, "Buggy URL must NOT contain password"); + + // Compare with proxyConfigToUrl which includes credentials + const fixedUrl = proxyConfigToUrl(proxy); + assert.ok(fixedUrl); + assert.match(fixedUrl!, /alice/, "Fixed URL must contain username"); + assert.match(fixedUrl!, /s3cret/, "Fixed URL must contain password"); +}); + +// ── Verify the scheduler and auto-test would use proxyConfigToUrl ────────── + +test("#8853 proxyConfigToUrl accepts ProxyRegistryRecord-shaped object", () => { + // Simulating the shape of a proxy record returned by listProxies({ includeSecrets: true }) + const proxyRecord = { + id: "p1", + name: "test", + type: "http", + host: "10.0.0.1", + port: 8888, + username: "bob", + password: "p4ss", + family: "auto", + region: null, + notes: null, + status: "active", + source: "manual", + subscriptionId: null, + createdAt: "2026-01-01", + updatedAt: "2026-01-01", + }; + const url = proxyConfigToUrl(proxyRecord); + assert.ok(url, "proxyConfigToUrl must accept ProxyRegistryRecord-shaped objects"); + assert.match(url!, /bob:p4ss/, "URL must include credentials from the record"); +}); + +test("#8853 proxyConfigToUrl returns null for partial config (no host)", () => { + const url = proxyConfigToUrl({ type: "http", port: 8080 } as Record); + assert.equal(url, null, "proxyConfigToUrl must return null for partial config without host"); +}); \ No newline at end of file