mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-06 23:32:12 +03:00
fix(proxy-health): include credentials in proxy health check URLs (#8853)
This commit is contained in:
committed by
GitHub
parent
0b70a14a3b
commit
37edd74f2d
1
changelog.d/fixes/8853-fix.plan.md
Normal file
1
changelog.d/fixes/8853-fix.plan.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(proxy-health): include credentials in proxy health check URLs (#8853)
|
||||
@@ -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<TestResult> {
|
||||
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))
|
||||
|
||||
@@ -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<ProxyProbeOutcome> {
|
||||
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<void> {
|
||||
const { items: proxies } = await listProxies({ includeSecrets: false });
|
||||
const { items: proxies } = await listProxies({ includeSecrets: true });
|
||||
if (proxies.length === 0) return;
|
||||
|
||||
const failureMap = getFailureMap();
|
||||
|
||||
120
tests/unit/triage-bugs-2026-08-02.test.ts
Normal file
120
tests/unit/triage-bugs-2026-08-02.test.ts
Normal file
@@ -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<string, unknown>);
|
||||
assert.equal(url, null, "proxyConfigToUrl must return null for partial config without host");
|
||||
});
|
||||
Reference in New Issue
Block a user