From 6471cc55d72deeebcbca70d54983b90ae32c999a Mon Sep 17 00:00:00 2001 From: User Date: Mon, 30 Mar 2026 16:33:32 -0700 Subject: [PATCH] fix(docker): harden cliproxy local proxy with auth guard, dynamic port, and body handling Move reverse proxy under /api/cliproxy-local so it sits behind auth middleware. Enforce localhost-only access when dashboard auth is disabled. Resolve the target port from unified config instead of hardcoding 8317. Re-serialize parsed JSON bodies before forwarding so writes still work behind express.json(). Clean up proxy connections on client abort. On the frontend, point the iframe and health check at the new same-origin API path, probe the proxy directly, and tighten iframe origin/path validation before sending the auto-login secret. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/web-server/index.ts | 8 +- src/web-server/routes/cliproxy-local-proxy.ts | 147 ++++++++++++++---- .../web-server/cliproxy-local-proxy.test.ts | 121 ++++++++++++++ .../cliproxy/control-panel-embed.tsx | 69 ++++---- 4 files changed, 279 insertions(+), 66 deletions(-) create mode 100644 tests/unit/web-server/cliproxy-local-proxy.test.ts diff --git a/src/web-server/index.ts b/src/web-server/index.ts index 6015c004..9aa1ae0d 100644 --- a/src/web-server/index.ts +++ b/src/web-server/index.ts @@ -63,6 +63,10 @@ export async function startServer(options: ServerOptions): Promise http://127.0.0.1:{port}/* */ -import { Router, Request, Response } from 'express'; import http from 'http'; -import { CLIPROXY_DEFAULT_PORT } from '../../cliproxy/config/port-manager'; +import { Request, Response, Router } from 'express'; +import { CLIPROXY_DEFAULT_PORT, validatePort } from '../../cliproxy/config/port-manager'; +import { loadOrCreateUnifiedConfig } from '../../config/unified-config-loader'; +import { requireLocalAccessWhenAuthDisabled } from '../middleware/auth-middleware'; -const router = Router(); +export interface CliproxyLocalProxyDeps { + enforceAccess?: (req: Request, res: Response) => boolean; + request?: typeof http.request; + resolveTargetPort?: () => number; +} -router.all('/*', (req: Request, res: Response) => { - // Strip the mount prefix — req.url already has it removed by Express - const targetPath = req.url || '/'; +function resolveLocalCliproxyPort(): number { + const config = loadOrCreateUnifiedConfig(); + return validatePort(config.cliproxy_server?.local?.port ?? CLIPROXY_DEFAULT_PORT); +} - const options: http.RequestOptions = { - hostname: '127.0.0.1', - port: CLIPROXY_DEFAULT_PORT, - path: targetPath, - method: req.method, - headers: { - ...req.headers, - host: `127.0.0.1:${CLIPROXY_DEFAULT_PORT}`, - }, +function isJsonContentType(contentType: string | string[] | undefined): boolean { + const values = Array.isArray(contentType) ? contentType : [contentType]; + return values.some((value) => value?.toLowerCase().includes('application/json') === true); +} + +function buildProxyBody(req: Request): Buffer | undefined { + if (!isJsonContentType(req.headers['content-type']) || req.body === undefined) { + return undefined; + } + + const contentLengthHeader = req.headers['content-length']; + const contentLength = Array.isArray(contentLengthHeader) + ? 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 Buffer.from(JSON.stringify(req.body)); +} + +function buildProxyHeaders( + headers: http.IncomingHttpHeaders, + port: number, + bodyBuffer?: Buffer +): http.IncomingHttpHeaders { + const proxyHeaders: http.IncomingHttpHeaders = { + ...headers, + host: `127.0.0.1:${port}`, }; - const proxyReq = http.request(options, (proxyRes) => { - res.writeHead(proxyRes.statusCode ?? 502, proxyRes.headers); - proxyRes.pipe(res, { end: true }); - }); + delete proxyHeaders.connection; - proxyReq.on('error', () => { - if (!res.headersSent) { - res.status(502).json({ error: 'CLIProxy is not reachable' }); + if (bodyBuffer) { + delete proxyHeaders['transfer-encoding']; + proxyHeaders['content-length'] = String(bodyBuffer.length); + } + + return proxyHeaders; +} + +export function createCliproxyLocalProxyRouter(deps: CliproxyLocalProxyDeps = {}): Router { + const router = Router(); + const enforceAccess = + deps.enforceAccess ?? + ((req: Request, res: Response) => + requireLocalAccessWhenAuthDisabled( + req, + res, + 'CLIProxy local proxy requires localhost access when dashboard auth is disabled.' + )); + const createRequest = deps.request ?? http.request; + const resolveTargetPort = deps.resolveTargetPort ?? resolveLocalCliproxyPort; + + router.use((req: Request, res: Response, next) => { + if (enforceAccess(req, res)) { + next(); } }); - req.pipe(proxyReq, { end: true }); -}); + router.all('/*', (req: Request, res: Response) => { + const targetPort = resolveTargetPort(); + const targetPath = req.url || '/'; + const bodyBuffer = buildProxyBody(req); -export default router; + const proxyReq = createRequest( + { + hostname: '127.0.0.1', + port: targetPort, + path: targetPath, + method: req.method, + headers: buildProxyHeaders(req.headers, targetPort, bodyBuffer), + }, + (proxyRes) => { + res.writeHead(proxyRes.statusCode ?? 502, proxyRes.headers); + proxyRes.pipe(res, { end: true }); + } + ); + + proxyReq.on('error', () => { + if (!res.headersSent) { + res.status(502).json({ error: 'CLIProxy is not reachable' }); + } + }); + + req.on('aborted', () => proxyReq.destroy()); + res.on('close', () => { + if (!res.writableEnded) { + proxyReq.destroy(); + } + }); + + if (bodyBuffer) { + proxyReq.end(bodyBuffer); + return; + } + + req.pipe(proxyReq, { end: true }); + }); + + return router; +} + +export default createCliproxyLocalProxyRouter(); diff --git a/tests/unit/web-server/cliproxy-local-proxy.test.ts b/tests/unit/web-server/cliproxy-local-proxy.test.ts new file mode 100644 index 00000000..57f39755 --- /dev/null +++ b/tests/unit/web-server/cliproxy-local-proxy.test.ts @@ -0,0 +1,121 @@ +import { afterEach, describe, expect, it } from 'bun:test'; +import express from 'express'; +import http from 'http'; +import type { AddressInfo } from 'net'; + +import { + createCliproxyLocalProxyRouter, + type CliproxyLocalProxyDeps, +} from '../../../src/web-server/routes/cliproxy-local-proxy'; + +const servers: http.Server[] = []; + +async function listen(server: http.Server): Promise { + servers.push(server); + + return await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(0, '127.0.0.1', () => { + server.off('error', reject); + resolve((server.address() as AddressInfo).port); + }); + }); +} + +async function createBackendServer( + handler: http.RequestListener +): Promise<{ port: number; server: http.Server }> { + const server = http.createServer(handler); + const port = await listen(server); + return { port, server }; +} + +async function createProxyServer(options: { + enforceAccess?: CliproxyLocalProxyDeps['enforceAccess']; + resolveTargetPort: () => number; +}): Promise<{ baseUrl: string; server: http.Server }> { + const app = express(); + app.use(express.json()); + app.use( + '/api/cliproxy-local', + createCliproxyLocalProxyRouter({ + enforceAccess: options.enforceAccess, + resolveTargetPort: options.resolveTargetPort, + }) + ); + + const server = http.createServer(app); + const port = await listen(server); + return { baseUrl: `http://127.0.0.1:${port}`, server }; +} + +afterEach(async () => { + while (servers.length > 0) { + const server = servers.pop(); + if (!server) { + continue; + } + + await new Promise((resolve) => server.close(() => resolve())); + } +}); + +describe('cliproxy local proxy route', () => { + it('blocks requests when local-access enforcement fails', async () => { + let backendHit = false; + const backend = await createBackendServer((_req, res) => { + backendHit = true; + res.writeHead(200).end('ok'); + }); + const proxy = await createProxyServer({ + resolveTargetPort: () => backend.port, + enforceAccess: (_req, res) => { + res.status(403).json({ error: 'blocked' }); + return false; + }, + }); + + const response = await fetch(`${proxy.baseUrl}/api/cliproxy-local/management.html`); + + expect(response.status).toBe(403); + expect(await response.json()).toEqual({ error: 'blocked' }); + expect(backendHit).toBe(false); + }); + + it('forwards JSON request bodies after express.json has parsed them', async () => { + const backend = await createBackendServer((req, res) => { + let body = ''; + req.setEncoding('utf8'); + req.on('data', (chunk) => { + body += chunk; + }); + req.on('end', () => { + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.end( + JSON.stringify({ + body: JSON.parse(body), + method: req.method, + path: req.url, + }) + ); + }); + }); + const proxy = await createProxyServer({ + resolveTargetPort: () => backend.port, + enforceAccess: () => true, + }); + + const response = await fetch(`${proxy.baseUrl}/api/cliproxy-local/v0/management/test`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ enabled: true, ids: ['a', 'b'] }), + }); + + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ + body: { enabled: true, ids: ['a', 'b'] }, + method: 'PATCH', + path: '/v0/management/test', + }); + }); +}); diff --git a/ui/src/components/cliproxy/control-panel-embed.tsx b/ui/src/components/cliproxy/control-panel-embed.tsx index 8755ab44..67abc74e 100644 --- a/ui/src/components/cliproxy/control-panel-embed.tsx +++ b/ui/src/components/cliproxy/control-panel-embed.tsx @@ -57,6 +57,7 @@ export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanel // Calculate URLs and settings based on remote or local mode const { managementUrl, checkUrl, authToken, isRemote, displayHost } = useMemo(() => { const remote = cliproxyConfig?.remote; + const localPort = cliproxyConfig?.local?.port ?? port; if (remote?.enabled && remote?.host) { const protocol = remote.protocol || 'http'; @@ -82,11 +83,11 @@ export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanel // (e.g., in Docker the browser cannot reach the internal CLIProxy port directly) const effectiveSecret = authTokens?.managementSecret?.value || 'ccs'; return { - managementUrl: '/cliproxy-local/management.html', - checkUrl: '/cliproxy-local/', + managementUrl: withApiBase('/cliproxy-local/management.html'), + checkUrl: withApiBase('/cliproxy-local/'), authToken: effectiveSecret, isRemote: false, - displayHost: `localhost:${port}`, + displayHost: `localhost:${localPort}`, }; }, [cliproxyConfig, authTokens, port]); @@ -96,6 +97,13 @@ export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanel // Check if CLIProxy is running useEffect(() => { const controller = new AbortController(); + let cancelled = false; + + const updateConnectionState = (connected: boolean, nextError: string | null) => { + if (cancelled) return; + setIsConnected(connected); + setError(nextError); + }; const checkConnection = async () => { try { @@ -109,33 +117,30 @@ export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanel authToken: remote?.auth_token, }); if (result?.reachable) { - setIsConnected(true); - setError(null); + updateConnectionState(true, null); } else { - setIsConnected(false); - setError( + updateConnectionState( + false, result?.error ? `Remote CLIProxy at ${displayHost}: ${result.error}` : `Remote CLIProxy at ${displayHost} returned an error` ); } } else { - // Local mode: use same-origin API to check proxy status (avoids CORS) - const status = await api.cliproxy.proxyStatus(); - if (status.running) { - setIsConnected(true); - setError(null); + // Local mode: probe the proxied control panel root directly. + const response = await fetch(checkUrl, { signal: controller.signal }); + if (response.ok) { + updateConnectionState(true, null); } else { - setIsConnected(false); - setError('CLIProxy is not running'); + updateConnectionState(false, 'CLIProxy returned an error'); } } } catch (e) { // Ignore abort errors (component unmounting) if (e instanceof Error && e.name === 'AbortError') return; - setIsConnected(false); - setError( + updateConnectionState( + false, isRemote ? `Remote CLIProxy at ${displayHost} is not reachable` : 'CLIProxy is not running' @@ -148,8 +153,11 @@ export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanel checkConnection().finally(() => clearTimeout(timeoutId)); // Cleanup: abort fetch on unmount - return () => controller.abort(); - }, [isRemote, displayHost, cliproxyConfig]); + return () => { + cancelled = true; + controller.abort(); + }; + }, [checkUrl, isRemote, displayHost, cliproxyConfig]); const postAutoLoginCredentials = useCallback(() => { // Auto-login can only run when iframe has loaded and authToken is available. @@ -158,21 +166,20 @@ export function ControlPanelEmbed({ port = CLIPROXY_DEFAULT_PORT }: ControlPanel } try { - // Derive apiBase and targetOrigin from checkUrl - // Local mode: checkUrl is a relative path (/cliproxy-local/) → same origin - // Remote mode: checkUrl is an absolute URL (http://host:port/) - const isRelative = checkUrl.startsWith('/'); - const apiBase = isRelative - ? `${window.location.origin}/cliproxy-local` + // Derive apiBase and targetOrigin from checkUrl. + // Local mode uses the same-origin dashboard proxy; remote mode stays absolute. + const apiBase = checkUrl.startsWith('/') + ? new URL(checkUrl.replace(/\/$/, ''), window.location.origin).href : checkUrl.replace(/\/$/, ''); - const targetOrigin = isRelative ? window.location.origin : apiBase; + const apiBaseUrl = new URL(`${apiBase}/`); + const targetOrigin = apiBaseUrl.origin; - // Security: Validate iframe src matches target origin before sending credentials - const iframeSrc = iframeRef.current.src; - const resolvedSrc = isRelative - ? new URL(iframeSrc, window.location.origin).href - : iframeSrc; - if (!resolvedSrc.startsWith(targetOrigin)) { + // Security: Validate iframe src matches the expected origin/path before sending credentials. + const iframeUrl = new URL(iframeRef.current.src, window.location.origin); + if ( + iframeUrl.origin !== apiBaseUrl.origin || + !iframeUrl.pathname.startsWith(apiBaseUrl.pathname) + ) { console.warn('[ControlPanelEmbed] Iframe origin mismatch, skipping postMessage'); return; }