From 3862411bb7ac61fb52a13e53db95ae3b99d49c4b Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sun, 3 May 2026 14:07:31 -0400 Subject: [PATCH] fix(cliproxy): harden custom local port handling --- src/cliproxy/proxy/proxy-target-resolver.ts | 3 +- src/cursor/cursor-profile-executor.ts | 24 +++++++-- src/delegation/headless-executor.ts | 4 +- src/web-server/routes/proxy-routes.ts | 31 +++++++++++ .../cursor/cursor-profile-executor.test.ts | 51 +++++++++++++++++++ .../api-routes-remote-write-guard.test.ts | 50 ++++++++++++++++++ 6 files changed, 155 insertions(+), 8 deletions(-) diff --git a/src/cliproxy/proxy/proxy-target-resolver.ts b/src/cliproxy/proxy/proxy-target-resolver.ts index 1f7875d8..6ca16ebe 100644 --- a/src/cliproxy/proxy/proxy-target-resolver.ts +++ b/src/cliproxy/proxy/proxy-target-resolver.ts @@ -10,6 +10,7 @@ import { CLIPROXY_DEFAULT_PORT, getRemoteDefaultPort, normalizeProtocol, + validatePort, validateRemotePort, } from '../config/port-manager'; import { getProxyEnvVars } from './proxy-config-resolver'; @@ -69,7 +70,7 @@ export function getProxyTarget(): ProxyTarget { }; } - const localPort = config?.local?.port ?? CLIPROXY_DEFAULT_PORT; + const localPort = validatePort(config?.local?.port ?? CLIPROXY_DEFAULT_PORT); return { host: '127.0.0.1', diff --git a/src/cursor/cursor-profile-executor.ts b/src/cursor/cursor-profile-executor.ts index 293bce24..e2b9b531 100644 --- a/src/cursor/cursor-profile-executor.ts +++ b/src/cursor/cursor-profile-executor.ts @@ -3,7 +3,7 @@ import { spawn } from 'child_process'; import type { CursorConfig } from '../config/unified-config-types'; import { ensureCliproxyService } from '../cliproxy'; -import { CLIPROXY_DEFAULT_PORT } from '../cliproxy/config/port-manager'; +import { resolveLifecyclePort } from '../cliproxy/config/port-manager'; import { fail, info, ok } from '../utils/ui'; import { appendThirdPartyWebSearchToolArgs, @@ -22,6 +22,13 @@ interface CursorImageAnalysisResolution { warning: string | null; } +interface CursorImageAnalysisDeps { + getImageAnalysisHookEnv?: typeof getImageAnalysisHookEnv; + resolveImageAnalysisRuntimeStatus?: typeof resolveImageAnalysisRuntimeStatus; + ensureCliproxyService?: typeof ensureCliproxyService; + resolveLifecyclePort?: typeof resolveLifecyclePort; +} + export function generateCursorEnv( config: CursorConfig, claudeConfigDir?: string @@ -45,9 +52,16 @@ export function generateCursorEnv( } export async function resolveCursorImageAnalysisEnv( - verbose = false + verbose = false, + deps: CursorImageAnalysisDeps = {} ): Promise { - const env = getImageAnalysisHookEnv({ + const getImageAnalysisHookEnvFn = deps.getImageAnalysisHookEnv ?? getImageAnalysisHookEnv; + const resolveImageAnalysisRuntimeStatusFn = + deps.resolveImageAnalysisRuntimeStatus ?? resolveImageAnalysisRuntimeStatus; + const ensureCliproxyServiceFn = deps.ensureCliproxyService ?? ensureCliproxyService; + const resolveLifecyclePortFn = deps.resolveLifecyclePort ?? resolveLifecyclePort; + + const env = getImageAnalysisHookEnvFn({ profileName: 'cursor', profileType: 'cursor', }); @@ -56,7 +70,7 @@ export async function resolveCursorImageAnalysisEnv( return { env, warning: null }; } - const status = await resolveImageAnalysisRuntimeStatus({ + const status = await resolveImageAnalysisRuntimeStatusFn({ profileName: 'cursor', profileType: 'cursor', }); @@ -73,7 +87,7 @@ export async function resolveCursorImageAnalysisEnv( } if (status.proxyReadiness === 'stopped') { - const ensureServiceResult = await ensureCliproxyService(CLIPROXY_DEFAULT_PORT, verbose); + const ensureServiceResult = await ensureCliproxyServiceFn(resolveLifecyclePortFn(), verbose); if (!ensureServiceResult.started) { return { env: { diff --git a/src/delegation/headless-executor.ts b/src/delegation/headless-executor.ts index cb97bb32..2ceeaa62 100644 --- a/src/delegation/headless-executor.ts +++ b/src/delegation/headless-executor.ts @@ -43,7 +43,7 @@ import { } from '../utils/hooks/image-analyzer-profile-hook-injector'; import { resolveCliproxyBridgeMetadata } from '../api/services'; import { ensureCliproxyService } from '../cliproxy'; -import { CLIPROXY_DEFAULT_PORT } from '../cliproxy/config/port-manager'; +import { resolveLifecyclePort } from '../cliproxy/config/port-manager'; import { buildOpenAICompatProxyEnv, resolveOpenAICompatProfileConfig, @@ -212,7 +212,7 @@ export class HeadlessExecutor { imageAnalysisProvider && imageAnalysisStatus.proxyReadiness === 'stopped' ) { - const ensureServiceResult = await ensureCliproxyService(CLIPROXY_DEFAULT_PORT, false); + const ensureServiceResult = await ensureCliproxyService(resolveLifecyclePort(), false); if (!ensureServiceResult.started) { console.error( warn( diff --git a/src/web-server/routes/proxy-routes.ts b/src/web-server/routes/proxy-routes.ts index 9b13b1a2..a98867ed 100644 --- a/src/web-server/routes/proxy-routes.ts +++ b/src/web-server/routes/proxy-routes.ts @@ -12,6 +12,7 @@ import { Router, Request, Response } from 'express'; import { testConnection } from '../../cliproxy/services/remote-proxy-client'; import { isProxyRunning } from '../../cliproxy/services/proxy-lifecycle-service'; import { DEFAULT_BACKEND } from '../../cliproxy/binary/platform-detector'; +import { validatePort } from '../../cliproxy/config/port-manager'; import { DEFAULT_CLIPROXY_SERVER_CONFIG, CliproxyServerConfig, @@ -58,6 +59,36 @@ router.get('/', async (_req: Request, res: Response) => { router.put('/', (req: Request, res: Response) => { try { const updates = req.body as Partial; + const currentConfig = loadOrCreateUnifiedConfig(); + const currentLocalPort = validatePort( + currentConfig.cliproxy_server?.local?.port ?? DEFAULT_CLIPROXY_SERVER_CONFIG.local.port + ); + const requestedLocalPort = updates.local?.port; + + if ( + requestedLocalPort !== undefined && + (!Number.isInteger(requestedLocalPort) || + requestedLocalPort < 1 || + requestedLocalPort > 65535) + ) { + res.status(400).json({ + error: 'Invalid local port. Must be an integer between 1 and 65535.', + }); + return; + } + + const nextLocalPort = + requestedLocalPort === undefined ? currentLocalPort : validatePort(requestedLocalPort); + + if (nextLocalPort !== currentLocalPort && isProxyRunning()) { + res.status(409).json({ + error: + 'Proxy is running on the current local port. Stop CLIProxy before changing local.port.', + proxyRunning: true, + currentLocalPort, + }); + return; + } // Atomic read-modify-write — avoids race between load and save const updated = mutateConfig((config) => { diff --git a/tests/unit/cursor/cursor-profile-executor.test.ts b/tests/unit/cursor/cursor-profile-executor.test.ts index c0c2a1fc..4d7d6260 100644 --- a/tests/unit/cursor/cursor-profile-executor.test.ts +++ b/tests/unit/cursor/cursor-profile-executor.test.ts @@ -66,6 +66,57 @@ describe('cursor-profile-executor', () => { expect(warning).toBeNull(); }); + it('starts local CLIProxy on the configured lifecycle port for cursor image analysis', async () => { + let ensuredPort: number | undefined; + + const { env, warning } = await resolveCursorImageAnalysisEnv(false, { + getImageAnalysisHookEnv: () => ({ + CCS_CURRENT_PROVIDER: 'ghcp', + CCS_IMAGE_ANALYSIS_SKIP: '0', + }), + resolveImageAnalysisRuntimeStatus: async () => ({ + enabled: true, + supported: true, + status: 'active', + backendId: 'ghcp', + backendDisplayName: 'GitHub Copilot (OAuth)', + model: 'claude-haiku-4.5', + resolutionSource: 'cursor-alias', + reason: null, + shouldPersistHook: true, + persistencePath: 'cursor.settings.json', + runtimePath: '/api/provider/ghcp', + usesCurrentTarget: true, + usesCurrentAuthToken: true, + hookInstalled: true, + sharedHookInstalled: true, + authReadiness: 'ready', + authProvider: 'ghcp', + authDisplayName: 'GitHub Copilot (OAuth)', + authReason: null, + proxyReadiness: 'stopped', + proxyReason: + 'Local CLIProxy service is idle. CCS will start it automatically when image analysis is needed.', + effectiveRuntimeMode: 'cliproxy-image-analysis', + effectiveRuntimeReason: null, + }), + ensureCliproxyService: async (port: number) => { + ensuredPort = port; + return { + started: true, + alreadyRunning: false, + port, + }; + }, + resolveLifecyclePort: () => 9321, + }); + + expect(ensuredPort).toBe(9321); + expect(env.CCS_CURRENT_PROVIDER).toBe('ghcp'); + expect(env.CCS_IMAGE_ANALYSIS_SKIP).toBe('0'); + expect(warning).toBeNull(); + }); + it('fails fast when Cursor integration is disabled', async () => { const exitCode = await executeCursorProfile({ ...BASE_CONFIG, enabled: false }, []); expect(exitCode).toBe(1); diff --git a/tests/unit/web-server/api-routes-remote-write-guard.test.ts b/tests/unit/web-server/api-routes-remote-write-guard.test.ts index 85d4d6a6..730c8a62 100644 --- a/tests/unit/web-server/api-routes-remote-write-guard.test.ts +++ b/tests/unit/web-server/api-routes-remote-write-guard.test.ts @@ -6,6 +6,8 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { apiRoutes } from '../../../src/web-server/routes'; +import { mutateConfig, loadOrCreateUnifiedConfig } from '../../../src/config/config-loader-facade'; +import { registerSession, deleteSessionLockForPort } from '../../../src/cliproxy/session-tracker'; import { authMiddleware, createSessionMiddleware, @@ -125,6 +127,54 @@ describe('api-routes remote write guard', () => { }); }); + it('rejects invalid local ports at the cliproxy-server API boundary', async () => { + forcedRemoteAddress = '127.0.0.1'; + + const response = await fetch(`${baseUrl}/api/cliproxy-server`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + local: { port: 70000 }, + }), + }); + + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ + error: 'Invalid local port. Must be an integer between 1 and 65535.', + }); + }); + + it('rejects local port changes while the current local proxy session is still running', async () => { + forcedRemoteAddress = '127.0.0.1'; + mutateConfig((config) => { + if (!config.cliproxy_server) { + throw new Error('cliproxy_server defaults were not initialized'); + } + config.cliproxy_server.local.port = 8317; + }); + registerSession(8317, process.pid); + + try { + const response = await fetch(`${baseUrl}/api/cliproxy-server`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + local: { port: 9000 }, + }), + }); + + expect(response.status).toBe(409); + expect(await response.json()).toEqual({ + error: 'Proxy is running on the current local port. Stop CLIProxy before changing local.port.', + proxyRunning: true, + currentLocalPort: 8317, + }); + expect(loadOrCreateUnifiedConfig().cliproxy_server?.local?.port).toBe(8317); + } finally { + deleteSessionLockForPort(8317); + } + }); + it('blocks remote PATCH requests when dashboard auth is disabled', async () => { const response = await fetch(`${baseUrl}/api/codex/config/patch`, { method: 'PATCH',