diff --git a/config/quality/eslint-suppressions.json b/config/quality/eslint-suppressions.json index 08b6f36d5f..c74f3b1b34 100644 --- a/config/quality/eslint-suppressions.json +++ b/config/quality/eslint-suppressions.json @@ -242,11 +242,6 @@ "count": 1 } }, - "src/app/(dashboard)/dashboard/settings/components/ComboDefaultsTab.tsx": { - "no-restricted-syntax": { - "count": 1 - } - }, "src/app/(dashboard)/dashboard/settings/components/ModelLockoutCard.tsx": { "react-hooks/exhaustive-deps": { "count": 1 @@ -1562,11 +1557,6 @@ "count": 2 } }, - "tests/unit/messages-count-tokens-route.test.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 4 - } - }, "tests/unit/mimocode-executor.test.ts": { "@typescript-eslint/no-explicit-any": { "count": 58 diff --git a/config/quality/test-masking-allowlist.json b/config/quality/test-masking-allowlist.json index 128c05303d..d548c4a40c 100644 --- a/config/quality/test-masking-allowlist.json +++ b/config/quality/test-masking-allowlist.json @@ -25,6 +25,10 @@ "src/shared/components/AutoRoutingBanner.test.tsx": { "replacement": "tests/unit/home-no-autorouting-banner.test.ts", "reason": "v3.8.45 #6164: fix(dashboard) remove the always-on Auto-Routing banner — o COMPONENTE foi deletado junto com o teste (feature removida pelo mantenedor, não mascaramento). O replacement guarda o novo contrato: a home NÃO renderiza o banner e o componente permanece deletado." + }, + "tests/unit/free-provider-rankings-configured-filter.test.ts": { + "replacement": "tests/unit/freeProviderRankings-filters.test.ts", + "reason": "v3.8.45 #6251 supersede #6245: a página Free Provider Rankings migrou do toggle client-side 'Configured Only' (#6245, configuredProviderIds no cliente) para filtros server-side configuredOnly/availableOnly (#6251). O teste antigo pinava a implementação removida (7 asserts quebrados contra código que não existe); o replacement cobre o contrato novo com 11 casos (server-side, lib helper). Verificado legítimo — supersessão documentada no CHANGELOG do #6251." } }, "tests/unit/catalog-updates-v3x.test.ts": "v3.8.45 #6248: fix(providers) remove deprecated MiMo V2 entries — os 5 asserts removidos pinavam specs de modelos mimo-v2-* que deixaram de existir no catálogo (54→49). Asserts seguem a remoção dos modelos, não enfraquecimento. Verificado legítimo. Prune após v3.8.45 mergear para main.", diff --git a/open-sse/services/combo/validateQuality.ts b/open-sse/services/combo/validateQuality.ts index d752a78989..2bab070a21 100644 --- a/open-sse/services/combo/validateQuality.ts +++ b/open-sse/services/combo/validateQuality.ts @@ -97,6 +97,7 @@ export async function validateResponseQuality( let hasContentBlock = false; let hasLifecycleEnd = false; let anyContentFound = false; + let sawAnyBytes = false; const sseLineNormalizer = createSSEDataLineNormalizer(); let pendingEventType = ""; @@ -234,11 +235,13 @@ export async function validateResponseQuality( return { valid: false, reason: "streaming empty content block" }; } - // Non-Claude stream with no recognizable content at all — the stream - // ended without any content deltas (e.g. Gemini returning HTTP 200 - // with an empty body or only metadata chunks). Mark as invalid for - // combo failover so the sibling model gets tried. - if (!anyContentFound && !hasContentBlock) { + // Stream ended with a truly EMPTY body (e.g. Gemini returning HTTP + // 200 with zero bytes) — mark as invalid for combo failover so the + // sibling model gets tried. Streams that carried ANY SSE activity + // (an explicit `data: [DONE]`, ping/metadata events, an incomplete + // Claude lifecycle) keep the pass-through contract (#3399/#3685): + // those are handled by the stream-readiness timeout, not failover. + if (!anyContentFound && !hasContentBlock && !sawAnyBytes) { log.warn?.( "COMBO", "Streaming response ended with no recognized content — marking as invalid for combo failover" @@ -255,6 +258,7 @@ export async function validateResponseQuality( // Accumulate raw bytes for potential replay. bufferedChunks.push(value); + if (value && value.length > 0) sawAnyBytes = true; // Decode incrementally (stream:true keeps multi-byte char state). decodedSoFar += decoder.decode(value, { stream: true }); diff --git a/src/lib/db/proxies.ts b/src/lib/db/proxies.ts index ffe0b85a5b..754259e82f 100755 --- a/src/lib/db/proxies.ts +++ b/src/lib/db/proxies.ts @@ -632,6 +632,59 @@ export async function resolveProxyForScopeFromRegistry(scope: string, scopeId?: } } +/** + * #6246 fail-closed guard. Returns true when a connection would egress DIRECTLY + * ONLY because its ASSIGNED proxy (account/provider/global scope) is dead/inactive + * — i.e. the request must be BLOCKED, not silently sent on the real IP. + * + * Callers use this after `resolveProxyForConnection` returns a direct result: if + * the operator assigned a proxy but every assigned proxy is dead, leaking the IP + * is worse than failing the request. An explicit "proxy off" (global or per + * connection) is a deliberate direct choice and is NOT treated as a leak. Read-only + * and best-effort: any DB error fails OPEN (returns false) so a guard never breaks + * the request path. + */ +export function hasBlockingProxyAssignment(connectionId: string): boolean { + try { + const db = getDbInstance(); + + // Explicit global "proxy off" → direct is intended, never a leak. + const globalRow = db + .prepare("SELECT value FROM key_value WHERE namespace = 'settings' AND key = 'proxyEnabled'") + .get() as { value?: string } | undefined; + if (globalRow?.value) { + try { + if (JSON.parse(globalRow.value) === false) return false; + } catch { + /* malformed → treat as enabled */ + } + } + + // Explicit per-connection "proxy off" → direct is intended. + const conn = db + .prepare("SELECT provider, proxy_enabled FROM provider_connections WHERE id = ?") + .get(connectionId) as { provider?: string | null; proxy_enabled?: number } | undefined; + if (conn && conn.proxy_enabled === 0) return false; + const provider = conn?.provider ?? null; + + // A proxy is assigned to this connection at some scope, but every assigned + // proxy is dead (the alive filter would have resolved a live one already). + const dead = db + .prepare( + `SELECT 1 FROM proxy_assignments a JOIN proxy_registry p ON p.id = a.proxy_id + WHERE ((a.scope = 'account' AND a.scope_id = ?) + OR (a.scope = 'provider' AND a.scope_id = ?) + OR (a.scope = 'global')) + AND NOT ${PROXY_ALIVE_PREDICATE} + LIMIT 1` + ) + .get(connectionId, provider); + return !!dead; + } catch { + return false; + } +} + export async function migrateLegacyProxyConfigToRegistry(options?: { force?: boolean }) { const force = options?.force === true; const db = getDbInstance(); diff --git a/src/lib/proxyHealth/decision.ts b/src/lib/proxyHealth/decision.ts new file mode 100644 index 0000000000..341d404290 --- /dev/null +++ b/src/lib/proxyHealth/decision.ts @@ -0,0 +1,77 @@ +/** + * Pure, network-free decision for the proxy health scheduler (#6246). + * + * Separated from the sweep so the status/removal policy can be unit-tested + * exhaustively without any I/O. The sweep classifies each probe into a tri-state + * {@link ProxyProbeOutcome} and applies the returned {@link ProxyHealthDecision}. + * + * Policy (agreed for #6246): + * A — downgrade only after `removeAfter` CONSECUTIVE conclusive failures. + * B — an `inconclusive` probe (our own timeout/abort, or the probe TARGET + * erroring) never penalizes: it neither counts nor changes status. + * C — by DEFAULT (auto-remove off) the health check NEVER mutates a proxy's + * status. It only counts failures for logging. A proxy is downgraded to + * `inactive` (and removed) only when the operator opts in via + * PROXY_AUTO_REMOVE=true. This mirrors how accounts are only auto-disabled + * when the operator allows it — the operator owns their (often paid) proxies. + */ + +export type ProxyProbeOutcome = "ok" | "fail" | "inconclusive"; + +export interface ProxyHealthDecisionInput { + /** Tri-state result of the reachability probe for this proxy. */ + outcome: ProxyProbeOutcome; + /** Consecutive failure count recorded BEFORE this probe. */ + priorFailures: number; + /** PROXY_AUTO_REMOVE === "true" — operator opted into status management. */ + autoRemove: boolean; + /** Consecutive conclusive failures required before a downgrade/removal. */ + removeAfter: number; +} + +export interface ProxyHealthDecision { + /** New consecutive-failure count to persist for this proxy. */ + failures: number; + /** Whether to drop this proxy from the consecutive-failure map. */ + clearFailures: boolean; + /** Status to write, or `null` to leave the operator-controlled status untouched. */ + setStatus: "active" | "inactive" | null; + /** Whether to auto-remove the proxy (only ever true when autoRemove is on). */ + remove: boolean; +} + +export function decideProxyHealthAction(input: ProxyHealthDecisionInput): ProxyHealthDecision { + const { outcome, priorFailures, autoRemove, removeAfter } = input; + const threshold = Number.isFinite(removeAfter) && removeAfter > 0 ? removeAfter : 3; + + // B: inconclusive probes are neutral — do not touch count or status. + if (outcome === "inconclusive") { + return { failures: priorFailures, clearFailures: false, setStatus: null, remove: false }; + } + + // Success: reset the streak. Only (re)assert "active" when the operator has + // opted into status management; otherwise never touch the user's status (C). + if (outcome === "ok") { + return { + failures: 0, + clearFailures: true, + setStatus: autoRemove ? "active" : null, + remove: false, + }; + } + + // Conclusive failure. + const failures = priorFailures + 1; + + // C: default mode only counts/logs — never downgrades. + if (!autoRemove) { + return { failures, clearFailures: false, setStatus: null, remove: false }; + } + + // A: downgrade + remove only once the consecutive threshold is reached. + if (failures >= threshold) { + return { failures, clearFailures: false, setStatus: "inactive", remove: true }; + } + + return { failures, clearFailures: false, setStatus: null, remove: false }; +} diff --git a/src/lib/proxyHealth/scheduler.ts b/src/lib/proxyHealth/scheduler.ts index 2e6c291125..0921a909a7 100644 --- a/src/lib/proxyHealth/scheduler.ts +++ b/src/lib/proxyHealth/scheduler.ts @@ -14,8 +14,16 @@ import { deleteProxyById, listProxies, updateProxy } from "@/lib/localDb"; import { createProxyDispatcher, clearDispatcherCache } from "@omniroute/open-sse/utils/proxyDispatcher"; import { fetch as undiciFetch } from "undici"; +import { + decideProxyHealthAction, + type ProxyProbeOutcome, +} from "./decision.ts"; -const TEST_TIMEOUT_MS = 5000; +// #6246: a HEAD to the public probe target through a legit (often loaded) proxy +// can exceed a few seconds; the old 5s ceiling produced false negatives that +// flipped healthy proxies to inactive. Raise it and treat our own timeout as +// inconclusive (see testOneProxy) rather than a proxy failure. +const TEST_TIMEOUT_MS = 15000; // Reachability probe target for proxy health checks. Configurable so operators // can point it at an internal/self-hosted endpoint instead of the public default. const TEST_URL = process.env.PROXY_HEALTH_TEST_URL || "https://httpbin.org/ip"; @@ -65,7 +73,21 @@ function isBackgroundServicesDisabled(): boolean { return ["1", "true", "yes", "on"].includes(raw.trim().toLowerCase()); } -async function testOneProxy(proxy: { id: string; type: string; host: string; port: number }): Promise { +/** + * Reachability probe for one proxy, classified into a tri-state so the pure + * decision layer can apply the #6246 policy: + * - "ok" — the proxy relayed and the target answered (<500). + * - "inconclusive" — NOT the proxy's fault: our own timeout/abort, or the probe + * TARGET returned a 5xx (the proxy connected fine). Never + * penalizes the proxy. + * - "fail" — a proxy-level connection error (refused/unreachable/TLS). + */ +async function testOneProxy(proxy: { + id: string; + type: string; + host: string; + port: number; +}): Promise { const proxyUrl = `${proxy.type}://${proxy.host}:${proxy.port}`; const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), TEST_TIMEOUT_MS); @@ -77,9 +99,13 @@ async function testOneProxy(proxy: { id: string; type: string; host: string; por dispatcher, headers: { "User-Agent": "OmniRoute/1.0" }, }); - return resp.status < 500; + // A 5xx from the probe target means the proxy DID relay — the target is at + // fault, not the proxy. Do not penalize the proxy for that. + return resp.status < 500 ? "ok" : "inconclusive"; } catch { - return false; + // Our own deadline elapsed → inconclusive (slow, not necessarily dead). + // Any other error is a genuine proxy-level connection failure. + return controller.signal.aborted ? "inconclusive" : "fail"; } finally { clearTimeout(timeout); } @@ -95,44 +121,54 @@ async function sweep(): Promise { let tested = 0; let alive = 0; + let inconclusive = 0; let removed = 0; for (let i = 0; i < proxies.length; i += CONCURRENCY) { const batch = proxies.slice(i, i + CONCURRENCY); const results = await Promise.allSettled( batch.map(async (proxy) => { - const ok = await testOneProxy(proxy); - return { id: proxy.id, ok }; + const outcome = await testOneProxy(proxy); + return { id: proxy.id, outcome }; }) ); for (const result of results) { if (result.status !== "fulfilled") continue; - const { id, ok } = result.value; + const { id, outcome } = result.value; tested++; + if (outcome === "ok") alive++; + else if (outcome === "inconclusive") inconclusive++; - if (ok) { - alive++; - failureMap.delete(id); - await updateProxy(id, { status: "active" }).catch(() => {}); - } else { - const failures = (failureMap.get(id) ?? 0) + 1; - failureMap.set(id, failures); - await updateProxy(id, { status: "inactive" }).catch(() => {}); + const decision = decideProxyHealthAction({ + outcome, + priorFailures: failureMap.get(id) ?? 0, + autoRemove, + removeAfter, + }); - if (autoRemove && failures >= removeAfter) { - if (await deleteProxyById(id, { force: true }).catch(() => false)) { - failureMap.delete(id); - removed++; - try { clearDispatcherCache(); } catch { /* non-critical */ } - } + if (decision.clearFailures) failureMap.delete(id); + else failureMap.set(id, decision.failures); + + // #6246 (policy C): only mutate the operator-owned status when the decision + // explicitly asks for it. By default (auto-remove off) setStatus is null, so + // a transient probe failure never flips a healthy proxy to inactive. + if (decision.setStatus) { + await updateProxy(id, { status: decision.setStatus }).catch(() => {}); + } + + if (decision.remove) { + if (await deleteProxyById(id, { force: true }).catch(() => false)) { + failureMap.delete(id); + removed++; + try { clearDispatcherCache(); } catch { /* non-critical */ } } } } } console.log( - `${LOG_PREFIX} Sweep complete: ${tested} tested, ${alive} alive, ${removed} auto-removed` + `${LOG_PREFIX} Sweep complete: ${tested} tested, ${alive} alive, ${inconclusive} inconclusive, ${removed} auto-removed` ); } diff --git a/src/sse/handlers/chatHelpers.ts b/src/sse/handlers/chatHelpers.ts index e26581f897..87eb47a389 100644 --- a/src/sse/handlers/chatHelpers.ts +++ b/src/sse/handlers/chatHelpers.ts @@ -28,6 +28,7 @@ import { type AppliedProxySink, } from "@omniroute/open-sse/utils/proxyFetch.ts"; import { resolveProxyForConnection } from "@/lib/localDb"; +import { hasBlockingProxyAssignment } from "@/lib/db/proxies"; import { CircuitBreakerOpenError, getCircuitBreaker, @@ -686,7 +687,22 @@ export function decideProxyResolutionFailure( export async function safeResolveProxy(connectionId: string, apiKeyId?: string) { try { - return await resolveProxyForConnection(connectionId, apiKeyId); + const resolved = await resolveProxyForConnection(connectionId, apiKeyId); + // #6246: a connection that resolves to DIRECT only because its assigned proxy + // is dead/inactive must fail closed — egressing on the real IP leaks it. Reuse + // the existing proxy-resolution-failure policy (blocks by default; PROXY_FAIL_OPEN + // opts back into direct). Explicit "proxy off" is not a leak (see the guard). + if (!(resolved as { proxy?: unknown } | null)?.proxy && hasBlockingProxyAssignment(connectionId)) { + return decideProxyResolutionFailure( + Object.assign( + new Error( + "PROXY_ASSIGNED_UNAVAILABLE: assigned proxy is inactive/unreachable; refusing to egress on a direct connection" + ), + { code: "PROXY_ASSIGNED_UNAVAILABLE" } + ) + ); + } + return resolved; } catch (proxyErr) { return decideProxyResolutionFailure(proxyErr); } diff --git a/tests/unit/agentSkillTools-mcp.test.ts b/tests/unit/agentSkillTools-mcp.test.ts index 8a3985f4a9..a34acea595 100644 --- a/tests/unit/agentSkillTools-mcp.test.ts +++ b/tests/unit/agentSkillTools-mcp.test.ts @@ -43,8 +43,14 @@ test("agentSkillTools exports exactly 3 tools", () => { test("each agentSkillTool has name, description, inputSchema, and handler", () => { for (const toolDef of Object.values(agentSkillTools)) { - assert.ok(typeof toolDef.name === "string" && toolDef.name.length > 0, `${toolDef.name}: name missing`); - assert.ok(typeof toolDef.description === "string" && toolDef.description.length > 0, `${toolDef.name}: description missing`); + assert.ok( + typeof toolDef.name === "string" && toolDef.name.length > 0, + `${toolDef.name}: name missing` + ); + assert.ok( + typeof toolDef.description === "string" && toolDef.description.length > 0, + `${toolDef.name}: description missing` + ); assert.ok(toolDef.inputSchema != null, `${toolDef.name}: inputSchema missing`); assert.ok(typeof toolDef.handler === "function", `${toolDef.name}: handler missing`); } @@ -165,7 +171,7 @@ test("omniroute_agent_skills_coverage({}) returns coverage shape", async () => { assert.equal(result.cli.total, 20); assert.ok(typeof result.api.have === "number"); assert.ok(typeof result.cli.have === "number"); - assert.ok(result.api.have >= 0 && result.api.have <= 22); + assert.ok(result.api.have >= 0 && result.api.have <= 23); assert.ok(result.cli.have >= 0 && result.cli.have <= 20); assert.ok(typeof result.totalSkills === "number"); assert.equal(result.totalSkills, result.api.have + result.cli.have + (result.config?.have ?? 0)); diff --git a/tests/unit/combo-streaming-empty-content-failover.test.ts b/tests/unit/combo-streaming-empty-content-failover.test.ts index f623cd1bd1..1d38c10b76 100644 --- a/tests/unit/combo-streaming-empty-content-failover.test.ts +++ b/tests/unit/combo-streaming-empty-content-failover.test.ts @@ -273,3 +273,22 @@ test("#3685 streaming is preserved for non-empty response: clonedResponse body y assert.ok(decoded.includes("Hello"), "decoded body must contain the actual text content"); assert.ok(decoded.includes(", world!"), "decoded body must contain the full text delta"); }); + +test("#5976 truly EMPTY streaming body (zero bytes) → invalid for combo failover", async () => { + // A 200 SSE response whose body closes without emitting a single byte + // (e.g. Gemini returning HTTP 200 with an empty body) cannot carry content — + // fail over to the sibling model. Streams with ANY SSE activity (an explicit + // [DONE], ping/metadata events) keep the pass-through contract (#3399/#3685). + const emptyBody = new ReadableStream({ + start(controller) { + controller.close(); + }, + }); + const res = new Response(emptyBody, { + status: 200, + headers: { "content-type": "text/event-stream" }, + }); + const out = await validateResponseQuality(res, true, silentLog); + assert.equal(out.valid, false, "zero-byte streaming body must trigger failover"); + assert.equal(out.reason, "streaming no recognized content"); +}); diff --git a/tests/unit/free-provider-rankings-configured-filter.test.ts b/tests/unit/free-provider-rankings-configured-filter.test.ts deleted file mode 100644 index 86b16552de..0000000000 --- a/tests/unit/free-provider-rankings-configured-filter.test.ts +++ /dev/null @@ -1,93 +0,0 @@ -/** - * Unit tests for the "Configured Only" filter on the Free Provider Rankings page. - * - * Phase 1 of #6150 — verifies the toggle state, filtering logic, status column, - * cleanup flag, and i18n keys exist in the source code. - */ - -import test from "node:test"; -import assert from "node:assert/strict"; -import { readFileSync } from "node:fs"; -import { join } from "node:path"; - -const root = join(import.meta.dirname, "../.."); -const read = (p: string) => readFileSync(join(root, p), "utf8"); -const pageSrc = read("src/app/(dashboard)/dashboard/free-provider-rankings/page.tsx"); -const en = JSON.parse(read("src/i18n/messages/en.json")); - -test("page declares configuredOnly state", () => { - assert.ok(pageSrc.includes("useState(false)"), "configuredOnly defaults to false"); - assert.ok(pageSrc.includes("setConfiguredOnly"), "setConfiguredOnly setter exists"); -}); - -test("page declares configuredProviderIds state", () => { - assert.ok(pageSrc.includes("configuredProviderIds"), "configuredProviderIds state exists"); - assert.ok(pageSrc.includes("Set"), "configuredProviderIds is typed as Set"); -}); - -test("page fetches /api/providers on mount", () => { - assert.ok(pageSrc.includes('fetch("/api/providers")'), "fetches /api/providers"); - assert.ok(pageSrc.includes("conn?.provider"), "uses optional chaining for conn.provider"); -}); - -test("useEffect has cleanup flag to prevent stale state updates", () => { - assert.ok(pageSrc.includes("let active = true"), "declares cleanup flag"); - assert.ok(pageSrc.includes("if (!active) return"), "guards state update with active flag"); - assert.ok(pageSrc.includes("active = false"), "cleanup function sets active to false"); -}); - -test("displayedRankings filters by configuredProviderIds when toggle is on", () => { - assert.ok(pageSrc.includes("displayedRankings"), "displayedRankings derived variable exists"); - assert.ok( - pageSrc.includes("configuredProviderIds.has(r.id)"), - "filters rankings by configuredProviderIds.has(r.id)" - ); - assert.ok( - pageSrc.includes("configuredOnly\n ? rankings.filter"), - "conditional: when configuredOnly is true, filters rankings" - ); -}); - -test("toggle switch has accessible attributes", () => { - assert.ok(pageSrc.includes('role="switch"'), "toggle has role=switch"); - assert.ok( - pageSrc.includes("aria-checked={configuredOnly}"), - "toggle has aria-checked bound to configuredOnly" - ); - assert.ok( - pageSrc.includes('htmlFor="configured-only-toggle"'), - "label is linked to toggle via htmlFor" - ); -}); - -test("table has a 'Configured' status column", () => { - assert.ok(pageSrc.includes('t("colConfigured")'), "table header includes colConfigured key"); - assert.ok( - pageSrc.includes("configuredProviderIds.has(provider.id)"), - "status column checks configuredProviderIds" - ); -}); - -test("empty state shows noConfiguredProviders when toggle is on", () => { - assert.ok( - pageSrc.includes('t("noConfiguredProviders")'), - "empty state uses noConfiguredProviders i18n key" - ); - assert.ok( - pageSrc.includes("configuredOnly && rankings.length > 0"), - "shows noConfiguredProviders only when toggle is on and data exists" - ); -}); - -test("i18n: en.json has all required filter keys", () => { - const keys = en.freeProviderRankingsPage; - assert.ok(keys, "freeProviderRankingsPage namespace exists in en.json"); - assert.equal(typeof keys.configuredOnly, "string", "configuredOnly is a string"); - assert.equal(typeof keys.configuredOnlyHint, "string", "configuredOnlyHint is a string"); - assert.equal(typeof keys.noConfiguredProviders, "string", "noConfiguredProviders is a string"); - assert.equal(typeof keys.colConfigured, "string", "colConfigured is a string"); - assert.ok(keys.configuredOnly.length > 0, "configuredOnly is non-empty"); - assert.ok(keys.configuredOnlyHint.length > 0, "configuredOnlyHint is non-empty"); - assert.ok(keys.noConfiguredProviders.length > 0, "noConfiguredProviders is non-empty"); - assert.ok(keys.colConfigured.length > 0, "colConfigured is non-empty"); -}); diff --git a/tests/unit/proxy-assigned-unavailable-6246.test.ts b/tests/unit/proxy-assigned-unavailable-6246.test.ts new file mode 100644 index 0000000000..8aa52f3192 --- /dev/null +++ b/tests/unit/proxy-assigned-unavailable-6246.test.ts @@ -0,0 +1,141 @@ +/** + * TDD — #6246 IP leak (part 1, fail-closed). + * + * When a proxy is ASSIGNED to a connection (account/provider/global scope) but is + * dead/inactive, `resolveProxyForConnection` returns a direct result (no alive + * proxy resolved). The chat path used to egress DIRECTLY in that case, leaking + * the operator's real IP. The fix is a fail-closed guard: if the only reason a + * connection resolves to direct is that its ASSIGNED proxy is dead, block instead + * of leaking. Explicit "proxy off" toggles are a deliberate direct choice and must + * NOT be treated as a leak. + * + * This test exercises the pure DB predicate `hasBlockingProxyAssignment`, which + * encodes exactly that decision (honoring the global + connection proxy toggles). + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-proxy-6246-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = "test-secret"; + +const core = await import("../../src/lib/db/core.ts"); +const proxiesDb = await import("../../src/lib/db/proxies.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); + +async function resetStorage() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +function setGlobalProxyEnabled(enabled: boolean) { + const db = core.getDbInstance(); + db.prepare( + "INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES ('settings', 'proxyEnabled', ?)" + ).run(JSON.stringify(enabled)); +} + +async function makeConnection(): Promise { + const conn = await providersDb.createProviderConnection({ + provider: "openai", + authType: "apiKey", + name: `Conn ${Date.now()} ${Math.random()}`, + apiKey: "sk-test", + }); + return (conn as { id: string }).id; +} + +test.after(async () => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("BLOCKS: an account proxy assigned but marked inactive (the IP-leak case)", async () => { + await resetStorage(); + const connId = await makeConnection(); + const proxy = await proxiesDb.createProxy({ + name: "Dead paid proxy", + type: "http", + host: "127.0.0.1", + port: 9001, + }); + await proxiesDb.updateProxy(proxy!.id, { status: "inactive" }); + await proxiesDb.assignProxyToScope("account", connId, proxy!.id); + + assert.equal( + proxiesDb.hasBlockingProxyAssignment(connId), + true, + "a dead assigned proxy must block, not fall back to a direct egress" + ); +}); + +test("ALLOWS DIRECT: a connection with no proxy assignment at all", async () => { + await resetStorage(); + const connId = await makeConnection(); + assert.equal( + proxiesDb.hasBlockingProxyAssignment(connId), + false, + "no assignment = user never configured a proxy = direct is legitimate" + ); +}); + +test("NOT BLOCKING: an assigned proxy that is still ALIVE", async () => { + await resetStorage(); + const connId = await makeConnection(); + const proxy = await proxiesDb.createProxy({ + name: "Live proxy", + type: "http", + host: "127.0.0.1", + port: 9002, + }); + await proxiesDb.assignProxyToScope("account", connId, proxy!.id); + + assert.equal( + proxiesDb.hasBlockingProxyAssignment(connId), + false, + "an alive assigned proxy resolves normally; nothing to block" + ); +}); + +test("EXPLICIT DIRECT: global proxyEnabled=false is a deliberate choice, not a leak", async () => { + await resetStorage(); + const connId = await makeConnection(); + const proxy = await proxiesDb.createProxy({ + name: "Dead proxy", + type: "http", + host: "127.0.0.1", + port: 9003, + }); + await proxiesDb.updateProxy(proxy!.id, { status: "inactive" }); + await proxiesDb.assignProxyToScope("account", connId, proxy!.id); + setGlobalProxyEnabled(false); + + assert.equal( + proxiesDb.hasBlockingProxyAssignment(connId), + false, + "operator turned proxying off globally — direct is intended, do not block" + ); +}); + +test("BLOCKS: a dead GLOBAL proxy assignment blocks any connection", async () => { + await resetStorage(); + const connId = await makeConnection(); + const proxy = await proxiesDb.createProxy({ + name: "Dead global proxy", + type: "http", + host: "127.0.0.1", + port: 9004, + }); + await proxiesDb.updateProxy(proxy!.id, { status: "error" }); + await proxiesDb.assignProxyToScope("global", null, proxy!.id); + + assert.equal( + proxiesDb.hasBlockingProxyAssignment(connId), + true, + "a dead global proxy assignment must block, not leak direct" + ); +}); diff --git a/tests/unit/proxy-health-decide-action-6246.test.ts b/tests/unit/proxy-health-decide-action-6246.test.ts new file mode 100644 index 0000000000..0967ceed75 --- /dev/null +++ b/tests/unit/proxy-health-decide-action-6246.test.ts @@ -0,0 +1,103 @@ +/** + * TDD — #6246 proxy health regression (part 2, A+B+C). + * + * Before this fix the sweep marked a proxy `inactive` on the FIRST failed probe, + * unconditionally, and treated any error (including our own timeout or the probe + * TARGET being down) as a proxy failure. That flipped healthy paid proxies to + * inactive, which then dropped them from egress selection ("my proxies are not + * being used anymore"). + * + * The decision is extracted into a pure, network-free function so it can be + * unit-tested exhaustively: + * A — only downgrade after `removeAfter` CONSECUTIVE conclusive failures. + * B — an inconclusive probe (our timeout / probe-target error) never penalizes. + * C — by default (PROXY_AUTO_REMOVE off) the health check NEVER mutates status; + * it only counts/logs. Status downgrade happens only when auto-remove is on. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { decideProxyHealthAction } = await import("../../src/lib/proxyHealth/decision.ts"); + +test("C: default (autoRemove off) never mutates status on failure — only counts", () => { + const d = decideProxyHealthAction({ + outcome: "fail", + priorFailures: 0, + autoRemove: false, + removeAfter: 3, + }); + assert.equal(d.setStatus, null, "must not downgrade status by default"); + assert.equal(d.remove, false); + assert.equal(d.failures, 1, "still counts the failure for logging"); +}); + +test("C: default (autoRemove off) never downgrades even after many failures", () => { + const d = decideProxyHealthAction({ + outcome: "fail", + priorFailures: 9, + autoRemove: false, + removeAfter: 3, + }); + assert.equal(d.setStatus, null); + assert.equal(d.remove, false); + assert.equal(d.failures, 10); +}); + +test("A: with autoRemove on, does NOT downgrade before the consecutive threshold", () => { + const d = decideProxyHealthAction({ + outcome: "fail", + priorFailures: 1, // this probe makes it 2, threshold is 3 + autoRemove: true, + removeAfter: 3, + }); + assert.equal(d.setStatus, null, "2 < 3 failures must not flip inactive"); + assert.equal(d.remove, false); + assert.equal(d.failures, 2); +}); + +test("A: with autoRemove on, downgrades + removes at the consecutive threshold", () => { + const d = decideProxyHealthAction({ + outcome: "fail", + priorFailures: 2, // this probe makes it 3 == threshold + autoRemove: true, + removeAfter: 3, + }); + assert.equal(d.setStatus, "inactive"); + assert.equal(d.remove, true); + assert.equal(d.failures, 3); +}); + +test("B: an inconclusive probe never penalizes (no count bump, no status change)", () => { + const d = decideProxyHealthAction({ + outcome: "inconclusive", + priorFailures: 2, + autoRemove: true, + removeAfter: 3, + }); + assert.equal(d.setStatus, null, "inconclusive must not touch status"); + assert.equal(d.remove, false); + assert.equal(d.failures, 2, "failure streak is preserved, not incremented"); + assert.equal(d.clearFailures, false); +}); + +test("ok: resets the failure streak; re-activates only when autoRemove manages status", () => { + const onAuto = decideProxyHealthAction({ + outcome: "ok", + priorFailures: 2, + autoRemove: true, + removeAfter: 3, + }); + assert.equal(onAuto.clearFailures, true); + assert.equal(onAuto.setStatus, "active"); + assert.equal(onAuto.failures, 0); + + const offAuto = decideProxyHealthAction({ + outcome: "ok", + priorFailures: 2, + autoRemove: false, + removeAfter: 3, + }); + assert.equal(offAuto.clearFailures, true); + assert.equal(offAuto.setStatus, null, "default mode never touches user-controlled status"); + assert.equal(offAuto.failures, 0); +});