* fix(desktop): restore saved outbound proxy settings on app start (#949)
The standalone server hydrates its proxy state in index.ts right after
initDb, but the desktop embedder (desktop/src/server-host.ts) builds the
app without index.ts and never did — so a proxy URL saved through
PUT /api/settings/proxy sat in the settings table while the process
started with an empty proxy, and every restart made the Outbound proxy
fields appear empty until re-saved.
- lib/proxy.ts: new restoreProxySettings() — the single hydration step
(proxy_url / proxy_enabled / proxy_bypass), env-var precedence intact
- index.ts: calls it instead of the three inline apply* calls
- server-host.ts: calls it after initDb, before createApp
- new proxy-restore.test.ts: pins DB → process-state hydration, the
disabled flag, empty-DB defaults, and PROXY_URL env precedence
Checked: server vitest (proxy + proxy-restore: 87 pass; one pre-existing
compression perf failure on clean main, unrelated), desktop vitest,
esbuild bundle of server-host.ts.
* test(desktop): pin the proxy hydration call in the desktop boot path (#949)
The fix in a78e2c4 was one line in desktop/src/server-host.ts, and nothing
tested it: proxy-restore.test.ts proves restoreProxySettings() works, but
deleting the call from the embedder left the whole suite green — exactly
the shape of the original regression.
- new desktop/src/__tests__/server-host-boot.test.ts: mocks the entire
server surface server-host.ts imports and records the boot sequence, so
the call is pinned along with its position (after initDb, before
createApp/listen). Verified it fails when the call is removed.
- proxy-restore.test.ts: cover the tiers the previous test skipped — the
ALL_PROXY → HTTPS_PROXY → HTTP_PROXY fallback order, the lower-case
spellings, the dashboard value still outranking the ambient env, and
NO_PROXY being parsed and re-parsed at restore time.
Checked: desktop vitest 8 pass (2 files); server vitest 2516 pass /
5 skipped (224 files); tsc --noEmit clean for server, and for desktop
apart from two pre-existing server/src/services/media.ts errors.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: tashdroid <319142293+tashdroid@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,111 @@
|
||||
import { describe, it, expect, beforeEach, vi } from 'vitest';
|
||||
import { EventEmitter } from 'node:events';
|
||||
|
||||
// #949: the desktop embedder boots the server without server/src/index.ts, so
|
||||
// every startup step index.ts performs has to be repeated here by hand. The
|
||||
// regression was one missing line — restoreProxySettings() — and no test on
|
||||
// either side of the repo noticed: the server suite proves the function works,
|
||||
// but nothing proved this boot path calls it.
|
||||
//
|
||||
// So mock the whole server surface and record the boot sequence. Deleting the
|
||||
// restoreProxySettings() call, or moving it before initDb (no DB to read) or
|
||||
// after createApp() (the app is built against stale proxy state), fails here.
|
||||
|
||||
const calls: string[] = [];
|
||||
|
||||
vi.mock('../../../server/src/env.js', () => ({}));
|
||||
|
||||
vi.mock('../../../server/src/db/index.js', () => ({
|
||||
initDb: vi.fn(() => {
|
||||
calls.push('initDb');
|
||||
}),
|
||||
getDb: vi.fn(),
|
||||
getUnifiedApiKey: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../../../server/src/lib/proxy.js', () => ({
|
||||
restoreProxySettings: vi.fn(() => {
|
||||
calls.push('restoreProxySettings');
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('../../../server/src/app.js', () => ({
|
||||
createApp: vi.fn(() => {
|
||||
calls.push('createApp');
|
||||
return {
|
||||
listen: (_port: number, _host: string) => {
|
||||
calls.push('listen');
|
||||
const server = new EventEmitter();
|
||||
setImmediate(() => server.emit('listening'));
|
||||
return server;
|
||||
},
|
||||
};
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('../../../server/src/services/health.js', () => ({
|
||||
startHealthChecker: vi.fn(() => {
|
||||
calls.push('startHealthChecker');
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('../../../server/src/services/catalog-sync.js', () => ({
|
||||
startCatalogSync: vi.fn(() => {
|
||||
calls.push('startCatalogSync');
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('../../../server/src/services/auth.js', () => ({
|
||||
userCount: vi.fn(() => 1),
|
||||
createUser: vi.fn(),
|
||||
createSession: vi.fn(() => 'token'),
|
||||
}));
|
||||
|
||||
vi.mock('../../../server/src/lib/scheduler.js', () => ({
|
||||
NodeScheduler: class {},
|
||||
}));
|
||||
|
||||
async function boot(): Promise<void> {
|
||||
const { startServer } = await import('../server-host.js');
|
||||
await startServer({
|
||||
dbPath: ':memory:',
|
||||
clientDist: '/tmp/client-dist',
|
||||
host: '127.0.0.1',
|
||||
preferredPort: 45999,
|
||||
});
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
calls.length = 0;
|
||||
});
|
||||
|
||||
describe('desktop server boot sequence (#949)', () => {
|
||||
it('hydrates the saved proxy settings on every start', async () => {
|
||||
await boot();
|
||||
expect(calls).toContain('restoreProxySettings');
|
||||
});
|
||||
|
||||
it('hydrates after initDb — there is no settings table to read before it', async () => {
|
||||
await boot();
|
||||
expect(calls.indexOf('restoreProxySettings')).toBeGreaterThan(calls.indexOf('initDb'));
|
||||
});
|
||||
|
||||
it('hydrates before the app is built and starts listening', async () => {
|
||||
await boot();
|
||||
const restored = calls.indexOf('restoreProxySettings');
|
||||
expect(restored).toBeLessThan(calls.indexOf('createApp'));
|
||||
expect(restored).toBeLessThan(calls.indexOf('listen'));
|
||||
});
|
||||
|
||||
it('runs the whole startup in the order server/src/index.ts uses', async () => {
|
||||
await boot();
|
||||
expect(calls).toEqual([
|
||||
'initDb',
|
||||
'restoreProxySettings',
|
||||
'createApp',
|
||||
'listen',
|
||||
'startHealthChecker',
|
||||
'startCatalogSync',
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -11,6 +11,7 @@ import crypto from 'node:crypto';
|
||||
import type { Server } from 'node:http';
|
||||
import { createApp } from '../../server/src/app.js';
|
||||
import { initDb, getDb, getUnifiedApiKey } from '../../server/src/db/index.js';
|
||||
import { restoreProxySettings } from '../../server/src/lib/proxy.js';
|
||||
import { startHealthChecker } from '../../server/src/services/health.js';
|
||||
import { startCatalogSync } from '../../server/src/services/catalog-sync.js';
|
||||
import { userCount, createUser, createSession } from '../../server/src/services/auth.js';
|
||||
@@ -40,6 +41,11 @@ export async function startServer(opts: StartOptions): Promise<ServerHandle> {
|
||||
// 0.0.0.0, and a remote viewer must still enter the password.
|
||||
process.env.FREEAPI_DESKTOP = '1';
|
||||
initDb(opts.dbPath);
|
||||
// #949: the standalone server hydrates its proxy state in index.ts after
|
||||
// initDb; this embedder builds the app without index.ts, so without this
|
||||
// the URL saved in the settings table is ignored on every restart and the
|
||||
// outbound proxy fields appear empty until re-saved.
|
||||
restoreProxySettings();
|
||||
const app = createApp();
|
||||
const { server, port } = await listenWithScan(app, opts.host, opts.preferredPort);
|
||||
// Background timers need a Scheduler since the abstraction landed (4cbb571);
|
||||
|
||||
@@ -0,0 +1,248 @@
|
||||
import { describe, it, expect, beforeEach, afterAll } from 'vitest';
|
||||
import { initDb, getDb, setSetting } from '../../db/index.js';
|
||||
import {
|
||||
restoreProxySettings,
|
||||
getProxyUrl,
|
||||
isProxyEnabled,
|
||||
getProxyBypassPlatforms,
|
||||
getNoProxyRules,
|
||||
applyProxyUrl,
|
||||
applyProxyEnabled,
|
||||
applyProxyBypass,
|
||||
} from '../../lib/proxy.js';
|
||||
|
||||
// #949: the desktop embedder builds the app without server/src/index.ts, so
|
||||
// the proxy state it starts with is whatever the module defaults are — an
|
||||
// empty URL. The URL the user saved through PUT /api/settings/proxy sits in
|
||||
// the settings table, ignored until the next re-save. restoreProxySettings()
|
||||
// is the single hydration step both entry points now call after initDb; this
|
||||
// test pins that the DB value actually reaches the process state.
|
||||
|
||||
const PROXY_ENV_VARS = ['PROXY_URL', 'ALL_PROXY', 'HTTPS_PROXY', 'HTTP_PROXY', 'NO_PROXY'];
|
||||
|
||||
function clearProxyEnv(): void {
|
||||
for (const name of PROXY_ENV_VARS) {
|
||||
delete process.env[name];
|
||||
delete process.env[name.toLowerCase()];
|
||||
}
|
||||
}
|
||||
|
||||
let closed = false;
|
||||
|
||||
beforeEach(() => {
|
||||
clearProxyEnv();
|
||||
// Reset to the module defaults so each case starts from "fresh process".
|
||||
applyProxyUrl('');
|
||||
applyProxyEnabled(true);
|
||||
applyProxyBypass('');
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
if (!closed) {
|
||||
getDb().close();
|
||||
closed = true;
|
||||
}
|
||||
});
|
||||
|
||||
describe('restoreProxySettings (desktop embedder hydration, #949)', () => {
|
||||
it('loads a saved proxy URL, enabled flag and bypass list from the settings table', () => {
|
||||
process.env.ENCRYPTION_KEY = '0'.repeat(64);
|
||||
initDb(':memory:');
|
||||
setSetting('proxy_url', 'socks5h://127.0.0.1:9050');
|
||||
setSetting('proxy_enabled', '1');
|
||||
setSetting('proxy_bypass', 'groq,openrouter');
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('socks5h://127.0.0.1:9050');
|
||||
expect(isProxyEnabled()).toBe(true);
|
||||
expect(getProxyBypassPlatforms()).toEqual(['groq', 'openrouter']);
|
||||
});
|
||||
|
||||
it('respects a disabled proxy saved across a restart', () => {
|
||||
process.env.ENCRYPTION_KEY = '0'.repeat(64);
|
||||
initDb(':memory:');
|
||||
setSetting('proxy_url', 'http://127.0.0.1:3128');
|
||||
setSetting('proxy_enabled', '0');
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('http://127.0.0.1:3128');
|
||||
expect(isProxyEnabled()).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps the defaults when nothing was ever saved', () => {
|
||||
process.env.ENCRYPTION_KEY = '0'.repeat(64);
|
||||
initDb(':memory:');
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('');
|
||||
expect(isProxyEnabled()).toBe(true);
|
||||
expect(getProxyBypassPlatforms()).toEqual([]);
|
||||
});
|
||||
|
||||
it('lets the PROXY_URL env var outrank the saved value, as before', () => {
|
||||
process.env.ENCRYPTION_KEY = '0'.repeat(64);
|
||||
initDb(':memory:');
|
||||
setSetting('proxy_url', 'http://saved:3128');
|
||||
process.env.PROXY_URL = 'http://env:8080';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('http://env:8080');
|
||||
});
|
||||
});
|
||||
|
||||
// The env tiers below are resolveProxySource()'s job, but restore time is the
|
||||
// only moment they are consulted on a desktop start: the embedder hydrates
|
||||
// once and then nothing re-reads the environment. A regression that skipped a
|
||||
// tier here would strand a user who has only ever exported HTTPS_PROXY.
|
||||
describe('restoreProxySettings + the standard env fallbacks (#353 x #949)', () => {
|
||||
function freshDb(): void {
|
||||
process.env.ENCRYPTION_KEY = '0'.repeat(64);
|
||||
initDb(':memory:');
|
||||
}
|
||||
|
||||
it('falls back to ALL_PROXY when nothing was saved in the dashboard', () => {
|
||||
freshDb();
|
||||
process.env.ALL_PROXY = 'socks5://all:1080';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('socks5://all:1080');
|
||||
});
|
||||
|
||||
it('falls back to HTTPS_PROXY', () => {
|
||||
freshDb();
|
||||
process.env.HTTPS_PROXY = 'http://https-tier:8443';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('http://https-tier:8443');
|
||||
});
|
||||
|
||||
it('falls back to HTTP_PROXY', () => {
|
||||
freshDb();
|
||||
process.env.HTTP_PROXY = 'http://http-tier:8080';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('http://http-tier:8080');
|
||||
});
|
||||
|
||||
it('prefers ALL_PROXY, then HTTPS_PROXY, then HTTP_PROXY', () => {
|
||||
freshDb();
|
||||
process.env.ALL_PROXY = 'http://all:1';
|
||||
process.env.HTTPS_PROXY = 'http://https:2';
|
||||
process.env.HTTP_PROXY = 'http://http:3';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('http://all:1');
|
||||
});
|
||||
|
||||
it('reads the lower-case spelling too — the one curl/git users actually export', () => {
|
||||
freshDb();
|
||||
process.env.https_proxy = 'http://lower:8443';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('http://lower:8443');
|
||||
});
|
||||
|
||||
it('does NOT let the ambient env override a proxy the user typed into the dashboard', () => {
|
||||
freshDb();
|
||||
setSetting('proxy_url', 'http://saved:3128');
|
||||
process.env.ALL_PROXY = 'http://ambient:1080';
|
||||
process.env.HTTPS_PROXY = 'http://ambient:8443';
|
||||
process.env.HTTP_PROXY = 'http://ambient:8080';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('http://saved:3128');
|
||||
});
|
||||
|
||||
it('still lets PROXY_URL outrank the ambient vars', () => {
|
||||
freshDb();
|
||||
process.env.PROXY_URL = 'http://explicit:9090';
|
||||
process.env.ALL_PROXY = 'http://ambient:1080';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('http://explicit:9090');
|
||||
});
|
||||
|
||||
it('leaves the proxy empty when neither the DB nor any env var has one', () => {
|
||||
freshDb();
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('');
|
||||
});
|
||||
});
|
||||
|
||||
// NO_PROXY is read from the environment inside applyProxyUrl, so on the
|
||||
// desktop path it is parsed exactly once: during restore. If restore stopped
|
||||
// going through applyProxyUrl the saved URL might still land while the direct
|
||||
// list silently emptied, quietly proxying hosts the user excluded.
|
||||
describe('restoreProxySettings + NO_PROXY (#949)', () => {
|
||||
function freshDb(): void {
|
||||
process.env.ENCRYPTION_KEY = '0'.repeat(64);
|
||||
initDb(':memory:');
|
||||
}
|
||||
|
||||
it('parses NO_PROXY at restore time, lower-cased and trimmed', () => {
|
||||
freshDb();
|
||||
setSetting('proxy_url', 'http://127.0.0.1:3128');
|
||||
process.env.NO_PROXY = 'localhost, .Internal.Corp ,, 10.0.0.1';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getNoProxyRules()).toEqual(['localhost', '.internal.corp', '10.0.0.1']);
|
||||
});
|
||||
|
||||
it('normalises the `*.domain` spelling to the leading-dot form', () => {
|
||||
freshDb();
|
||||
setSetting('proxy_url', 'http://127.0.0.1:3128');
|
||||
process.env.NO_PROXY = '*.example.com';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getNoProxyRules()).toEqual(['.example.com']);
|
||||
});
|
||||
|
||||
it('honours the lower-case no_proxy spelling', () => {
|
||||
freshDb();
|
||||
setSetting('proxy_url', 'http://127.0.0.1:3128');
|
||||
process.env.no_proxy = 'example.com';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getNoProxyRules()).toEqual(['example.com']);
|
||||
});
|
||||
|
||||
it('applies NO_PROXY even when the proxy itself came from the env tier', () => {
|
||||
freshDb();
|
||||
process.env.HTTPS_PROXY = 'http://ambient:8443';
|
||||
process.env.NO_PROXY = 'internal.corp';
|
||||
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getProxyUrl()).toBe('http://ambient:8443');
|
||||
expect(getNoProxyRules()).toEqual(['internal.corp']);
|
||||
});
|
||||
|
||||
it('clears stale rules when the env no longer sets NO_PROXY', () => {
|
||||
freshDb();
|
||||
setSetting('proxy_url', 'http://127.0.0.1:3128');
|
||||
process.env.NO_PROXY = 'example.com';
|
||||
restoreProxySettings();
|
||||
expect(getNoProxyRules()).toEqual(['example.com']);
|
||||
|
||||
delete process.env.NO_PROXY;
|
||||
restoreProxySettings();
|
||||
|
||||
expect(getNoProxyRules()).toEqual([]);
|
||||
});
|
||||
});
|
||||
+3
-5
@@ -1,8 +1,8 @@
|
||||
import './env.js';
|
||||
import { createApp } from './app.js';
|
||||
import { initDb, getDb, getSetting } from './db/index.js';
|
||||
import { initDb, getDb } from './db/index.js';
|
||||
import { startHealthChecker, checkAllKeys } from './services/health.js';
|
||||
import { applyProxyUrl, applyProxyEnabled, applyProxyBypass, flushProxyCache } from './lib/proxy.js';
|
||||
import { restoreProxySettings, flushProxyCache } from './lib/proxy.js';
|
||||
import { startWakeDetect } from './lib/wake-detect.js';
|
||||
import { startCatalogSync } from './services/catalog-sync.js';
|
||||
import { startCooldownProbe } from './services/cooldown-probe.js';
|
||||
@@ -64,9 +64,7 @@ async function main() {
|
||||
|
||||
// Load the persisted proxy settings from the DB (env var wins if set).
|
||||
// Must happen after initDb so the settings table is ready.
|
||||
applyProxyUrl(getSetting('proxy_url') ?? '');
|
||||
applyProxyEnabled(getSetting('proxy_enabled') !== '0'); // default: enabled
|
||||
applyProxyBypass(getSetting('proxy_bypass') ?? '');
|
||||
restoreProxySettings();
|
||||
|
||||
const app = createApp(config);
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import http from 'http';
|
||||
import https from 'https';
|
||||
import { AsyncLocalStorage } from 'node:async_hooks';
|
||||
import { getSetting } from '../db/index.js';
|
||||
import { assertProviderUrlAllowed } from './url-guard.js';
|
||||
|
||||
// #590 (per-key proxy): the SAME provider may be reached through different
|
||||
@@ -190,6 +191,22 @@ export function applyProxyUrl(dbValue: string): void {
|
||||
_initialized = true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Hydrate the process-wide proxy state from the settings table.
|
||||
*
|
||||
* The standalone server does this in index.ts after initDb; the desktop
|
||||
* embedder (desktop/src/server-host.ts) builds the app without index.ts and
|
||||
* must call this itself — otherwise the URL saved by PUT /api/settings/proxy
|
||||
* sits in the DB but the process starts with an empty proxy and every
|
||||
* outbound request goes direct until the user re-saves the setting (#949).
|
||||
* Safe to call more than once; it is idempotent.
|
||||
*/
|
||||
export function restoreProxySettings(): void {
|
||||
applyProxyUrl(getSetting('proxy_url') ?? '');
|
||||
applyProxyEnabled(getSetting('proxy_enabled') !== '0'); // default: enabled
|
||||
applyProxyBypass(getSetting('proxy_bypass') ?? '');
|
||||
}
|
||||
|
||||
export function getProxyUrl(): string {
|
||||
return _proxyUrl;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user