fix(docker): harden proxy with timeout, Bun compat, and test coverage

- Add 30s timeout on proxy requests to prevent indefinite hangs
- Wrap resolveLocalCliproxyPort in try/catch with default port fallback
- Simplify buildProxyBody: remove fragile content-length check that
  caused Bun to fall through to req.pipe() on consumed streams
- Replace proxyRes.pipe(res) with manual streaming for Bun compatibility
  (pipe hangs after writeHead in Bun runtime)
- Replace deprecated req.on('aborted') with res.on('close') cleanup
  (req.on('close') fires with req.destroyed=true in Bun after body
  consumption, prematurely destroying the proxy connection)
- Explicitly end proxy request for bodyless methods (GET/HEAD/OPTIONS)
  instead of piping an already-consumed express stream
- Add server.closeAllConnections() in test cleanup to prevent hangs
- Add GET passthrough and 502 unreachable test cases
This commit is contained in:
Tam Nhu Tran
2026-04-01 11:39:34 -04:00
parent 6471cc55d7
commit 27409b789b
2 changed files with 65 additions and 17 deletions
+34 -17
View File
@@ -19,9 +19,16 @@ export interface CliproxyLocalProxyDeps {
resolveTargetPort?: () => number; resolveTargetPort?: () => number;
} }
/** Proxy request timeout in milliseconds (30 seconds) */
const PROXY_TIMEOUT_MS = 30_000;
function resolveLocalCliproxyPort(): number { function resolveLocalCliproxyPort(): number {
const config = loadOrCreateUnifiedConfig(); try {
return validatePort(config.cliproxy_server?.local?.port ?? CLIPROXY_DEFAULT_PORT); const config = loadOrCreateUnifiedConfig();
return validatePort(config.cliproxy_server?.local?.port ?? CLIPROXY_DEFAULT_PORT);
} catch {
return CLIPROXY_DEFAULT_PORT;
}
} }
function isJsonContentType(contentType: string | string[] | undefined): boolean { function isJsonContentType(contentType: string | string[] | undefined): boolean {
@@ -30,22 +37,17 @@ function isJsonContentType(contentType: string | string[] | undefined): boolean
} }
function buildProxyBody(req: Request): Buffer | undefined { function buildProxyBody(req: Request): Buffer | undefined {
if (!isJsonContentType(req.headers['content-type']) || req.body === undefined) { // If express.json() parsed the body (content-type is JSON and req.body is populated),
return undefined; // re-serialize it since the original request stream was consumed by the middleware.
} if (
!isJsonContentType(req.headers['content-type']) ||
const contentLengthHeader = req.headers['content-length']; req.body === undefined ||
const contentLength = Array.isArray(contentLengthHeader) req.body === null
? contentLengthHeader[0] ) {
: contentLengthHeader;
const hasTransferEncoding = req.headers['transfer-encoding'] !== undefined;
const parsedContentLength =
typeof contentLength === 'string' ? Number.parseInt(contentLength, 10) : NaN;
if (!hasTransferEncoding && (!Number.isFinite(parsedContentLength) || parsedContentLength <= 0)) {
return undefined; return undefined;
} }
// express.json() sets req.body to the parsed value — re-serialize for the proxy target
return Buffer.from(JSON.stringify(req.body)); return Buffer.from(JSON.stringify(req.body));
} }
@@ -100,20 +102,27 @@ export function createCliproxyLocalProxyRouter(deps: CliproxyLocalProxyDeps = {}
path: targetPath, path: targetPath,
method: req.method, method: req.method,
headers: buildProxyHeaders(req.headers, targetPort, bodyBuffer), headers: buildProxyHeaders(req.headers, targetPort, bodyBuffer),
timeout: PROXY_TIMEOUT_MS,
}, },
(proxyRes) => { (proxyRes) => {
res.writeHead(proxyRes.statusCode ?? 502, proxyRes.headers); res.writeHead(proxyRes.statusCode ?? 502, proxyRes.headers);
proxyRes.pipe(res, { end: true }); // Manual streaming instead of pipe() for Bun runtime compatibility
proxyRes.on('data', (chunk: Buffer) => res.write(chunk));
proxyRes.on('end', () => res.end());
} }
); );
proxyReq.on('timeout', () => proxyReq.destroy());
proxyReq.on('error', () => { proxyReq.on('error', () => {
if (!res.headersSent) { if (!res.headersSent) {
res.status(502).json({ error: 'CLIProxy is not reachable' }); res.status(502).json({ error: 'CLIProxy is not reachable' });
} }
}); });
req.on('aborted', () => proxyReq.destroy()); // Clean up proxy connection when client disconnects.
// Only use res.on('close') — req.on('close') fires with req.destroyed=true
// in Bun after body consumption, which would prematurely kill the proxy.
res.on('close', () => { res.on('close', () => {
if (!res.writableEnded) { if (!res.writableEnded) {
proxyReq.destroy(); proxyReq.destroy();
@@ -125,6 +134,14 @@ export function createCliproxyLocalProxyRouter(deps: CliproxyLocalProxyDeps = {}
return; return;
} }
// For methods without a body (GET, HEAD, etc.) or when express.json()
// has already consumed the stream, end the request immediately.
const hasBody = req.method !== 'GET' && req.method !== 'HEAD' && req.method !== 'OPTIONS';
if (!hasBody) {
proxyReq.end();
return;
}
req.pipe(proxyReq, { end: true }); req.pipe(proxyReq, { end: true });
}); });
@@ -56,6 +56,8 @@ afterEach(async () => {
continue; continue;
} }
// Force-close keep-alive connections so server.close() doesn't hang
server.closeAllConnections();
await new Promise<void>((resolve) => server.close(() => resolve())); await new Promise<void>((resolve) => server.close(() => resolve()));
} }
}); });
@@ -118,4 +120,33 @@ describe('cliproxy local proxy route', () => {
path: '/v0/management/test', path: '/v0/management/test',
}); });
}); });
it('forwards GET requests and returns backend response', async () => {
const backend = await createBackendServer((_req, res) => {
res.writeHead(200, { 'Content-Type': 'text/html' });
res.end('<html>management panel</html>');
});
const proxy = await createProxyServer({
resolveTargetPort: () => backend.port,
enforceAccess: () => true,
});
const response = await fetch(`${proxy.baseUrl}/api/cliproxy-local/management.html`);
expect(response.status).toBe(200);
expect(await response.text()).toBe('<html>management panel</html>');
});
it('returns 502 when CLIProxy is not reachable', async () => {
// Use a port with nothing listening
const proxy = await createProxyServer({
resolveTargetPort: () => 19999,
enforceAccess: () => true,
});
const response = await fetch(`${proxy.baseUrl}/api/cliproxy-local/`);
expect(response.status).toBe(502);
expect(await response.json()).toEqual({ error: 'CLIProxy is not reachable' });
});
}); });