fix(cliproxy): add comprehensive port validation across proxy system

- Export validatePort() from config-generator with full edge case handling
- Add YAML port validation in proxy-config-resolver before use
- Add final port validation in cliproxy-executor after config merge
- Add default port parameter and validation to proxy-detector
- Add 38 new tests covering all port validation edge cases

Fixes undefined port bug when upgrading from 7.11.1 to 7.12.0
This commit is contained in:
kaitranntt
2026-01-01 02:00:30 -05:00
parent 4c35e8a39e
commit e0a1f8f312
6 changed files with 327 additions and 14 deletions
+4
View File
@@ -26,6 +26,7 @@ import {
ensureProviderSettings,
CLIPROXY_DEFAULT_PORT,
getCliproxyWritablePath,
validatePort,
} from './config-generator';
import { checkRemoteProxy } from './remote-proxy-client';
import { isAuthenticated } from './auth-handler';
@@ -172,6 +173,9 @@ export async function execClaudeWithCLIProxy(
cfg.port = proxyConfig.port;
}
// Final port validation - ensure valid port after all resolution
cfg.port = validatePort(cfg.port);
log(`Proxy mode: ${proxyConfig.mode}`);
if (proxyConfig.mode === 'remote') {
log(`Remote host: ${proxyConfig.host}:${proxyConfig.port} (${proxyConfig.protocol})`);
+7 -1
View File
@@ -26,8 +26,14 @@ interface ProviderSettings {
/**
* Validate port is a valid positive integer (1-65535).
* Returns default port if invalid.
*
* @param port - Port number to validate
* @returns Valid port or CLIPROXY_DEFAULT_PORT (8317) if invalid
*/
function validatePort(port: number): number {
export function validatePort(port: number | undefined): number {
if (port === undefined || port === null) {
return CLIPROXY_DEFAULT_PORT;
}
if (!Number.isFinite(port) || port < 1 || port > 65535 || !Number.isInteger(port)) {
return CLIPROXY_DEFAULT_PORT;
}
+4 -2
View File
@@ -8,7 +8,7 @@
*/
import { ResolvedProxyConfig } from './types';
import { CLIPROXY_DEFAULT_PORT } from './config-generator';
import { CLIPROXY_DEFAULT_PORT, validatePort } from './config-generator';
/** CLI flags for proxy configuration */
export const PROXY_CLI_FLAGS = [
@@ -276,10 +276,12 @@ export function resolveProxyConfig(
resolved.host = cliFlags.host ?? envConfig.host ?? yamlConfig.remote?.host;
// Merge port: CLI > ENV > config.yaml (remote or local) > default
// Validate YAML-sourced ports before use (CLI/ENV already validated during parse)
const yamlPort = resolved.mode === 'remote' ? yamlConfig.remote?.port : yamlConfig.local?.port;
resolved.port =
cliFlags.port ??
envConfig.port ??
(resolved.mode === 'remote' ? yamlConfig.remote?.port : yamlConfig.local?.port) ??
(yamlPort !== undefined ? validatePort(yamlPort) : undefined) ??
DEFAULT_PROXY_CONFIG.port;
// Merge protocol: CLI > ENV > config.yaml > default
+16 -11
View File
@@ -18,6 +18,7 @@
import { getExistingProxy, registerSession, getRunningProxyVersion } from './session-tracker';
import { isCliproxyRunning } from './stats-fetcher';
import { getPortProcess, isCLIProxyProcess, PortProcess } from '../utils/port-utils';
import { CLIPROXY_DEFAULT_PORT } from './config-generator';
/** Detection method used to find the proxy */
export type DetectionMethod = 'http' | 'session-lock' | 'port-process' | 'http-retry';
@@ -61,27 +62,31 @@ const noopLog: LogFn = () => {};
* @returns ProxyStatus with detection details
*/
export async function detectRunningProxy(
port: number,
port: number = CLIPROXY_DEFAULT_PORT,
verbose: boolean = false
): Promise<ProxyStatus> {
const log: LogFn = verbose ? (msg) => console.error(`[proxy-detector] ${msg}`) : noopLog;
log(`Detecting proxy on port ${port}...`);
// Validate port - fallback to default if invalid
const validPort =
port && Number.isFinite(port) && port >= 1 && port <= 65535 ? port : CLIPROXY_DEFAULT_PORT;
log(`Detecting proxy on port ${validPort}...`);
// 1. First: HTTP health check (fastest, most reliable)
log('Trying HTTP health check...');
const healthy = await isCliproxyRunning(port);
const healthy = await isCliproxyRunning(validPort);
if (healthy) {
// Proxy is running and responsive
// Try to get PID from session lock if available
const lock = getExistingProxy(port);
const lock = getExistingProxy(validPort);
let pid = lock?.pid;
// If no PID from session lock, try port-process detection
// This handles orphaned proxies (running but no sessions.json)
if (!pid) {
log('No PID from session lock, checking port process...');
const portProcess = await getPortProcess(port);
const portProcess = await getPortProcess(validPort);
if (portProcess) {
pid = portProcess.pid;
log(`Got PID from port process: ${pid}`);
@@ -89,7 +94,7 @@ export async function detectRunningProxy(
}
// Get version from session lock
const runningVersion = getRunningProxyVersion(port);
const runningVersion = getRunningProxyVersion(validPort);
log(
`HTTP check passed, proxy healthy (PID: ${pid ?? 'unknown'}, version: ${runningVersion ?? 'unknown'})`
@@ -107,11 +112,11 @@ export async function detectRunningProxy(
// 2. Second: Check session lock file
log('Checking session lock file...');
const lock = getExistingProxy(port);
const lock = getExistingProxy(validPort);
if (lock) {
// Session lock exists - proxy might be starting up
// The lock validates PID is running, so proxy exists but not ready
const lockVersion = getRunningProxyVersion(port);
const lockVersion = getRunningProxyVersion(validPort);
log(
`Session lock found: PID ${lock.pid}, ${lock.sessions.length} sessions, version: ${lockVersion ?? 'unknown'}`
);
@@ -128,7 +133,7 @@ export async function detectRunningProxy(
// 3. Third: Check port process (fallback for orphaned/untracked proxy)
log('Checking port process...');
const portProcess = await getPortProcess(port);
const portProcess = await getPortProcess(validPort);
if (portProcess) {
log(`Port occupied by: ${portProcess.processName} (PID ${portProcess.pid})`);
@@ -136,7 +141,7 @@ export async function detectRunningProxy(
if (isCLIProxyProcess(portProcess)) {
log('Process name matches CLIProxy, retrying HTTP...');
// Looks like CLIProxy by name - verify with HTTP
const retryHealth = await isCliproxyRunning(port);
const retryHealth = await isCliproxyRunning(validPort);
if (retryHealth) {
log('HTTP retry passed, proxy is healthy');
return {
@@ -159,7 +164,7 @@ export async function detectRunningProxy(
// Process name doesn't match (or Windows returned PID-XXXXX)
// Do one more HTTP check to be sure it's not ours
log('Process name unrecognized, final HTTP check (Windows PID-XXXXX case)...');
const finalHealthCheck = await isCliproxyRunning(port);
const finalHealthCheck = await isCliproxyRunning(validPort);
if (finalHealthCheck) {
// It IS our proxy, just with unrecognized name (Windows case)
log('Final HTTP check passed, reclaiming proxy with unrecognized name');
+229
View File
@@ -0,0 +1,229 @@
/**
* Port Validation Tests
*
* Comprehensive test suite for port validation across the CLIProxy system.
* Tests the fix for undefined port bug (7.11.1 → 7.12.0 regression).
*
* Covers:
* - validatePort() function edge cases
* - Object spread undefined filtering in executor
* - YAML port validation in resolver
* - detectRunningProxy with invalid ports
*/
import { describe, it, expect } from 'bun:test';
import {
validatePort,
CLIPROXY_DEFAULT_PORT,
} from '../../../dist/cliproxy/config-generator';
import { resolveProxyConfig } from '../../../dist/cliproxy/proxy-config-resolver';
describe('Port Validation', () => {
describe('validatePort()', () => {
describe('returns default port for invalid inputs', () => {
it('handles undefined', () => {
expect(validatePort(undefined)).toBe(CLIPROXY_DEFAULT_PORT);
});
it('handles null', () => {
expect(validatePort(null as unknown as number)).toBe(CLIPROXY_DEFAULT_PORT);
});
it('handles NaN', () => {
expect(validatePort(NaN)).toBe(CLIPROXY_DEFAULT_PORT);
});
it('handles Infinity', () => {
expect(validatePort(Infinity)).toBe(CLIPROXY_DEFAULT_PORT);
});
it('handles negative Infinity', () => {
expect(validatePort(-Infinity)).toBe(CLIPROXY_DEFAULT_PORT);
});
it('handles zero', () => {
expect(validatePort(0)).toBe(CLIPROXY_DEFAULT_PORT);
});
it('handles negative numbers', () => {
expect(validatePort(-1)).toBe(CLIPROXY_DEFAULT_PORT);
expect(validatePort(-8317)).toBe(CLIPROXY_DEFAULT_PORT);
});
it('handles ports > 65535', () => {
expect(validatePort(65536)).toBe(CLIPROXY_DEFAULT_PORT);
expect(validatePort(100000)).toBe(CLIPROXY_DEFAULT_PORT);
});
it('handles floating point numbers', () => {
expect(validatePort(8317.5)).toBe(CLIPROXY_DEFAULT_PORT);
expect(validatePort(8317.999)).toBe(CLIPROXY_DEFAULT_PORT);
});
});
describe('returns valid port for valid inputs', () => {
it('handles port 1 (minimum valid)', () => {
expect(validatePort(1)).toBe(1);
});
it('handles port 65535 (maximum valid)', () => {
expect(validatePort(65535)).toBe(65535);
});
it('handles default port 8317', () => {
expect(validatePort(8317)).toBe(8317);
});
it('handles common ports', () => {
expect(validatePort(80)).toBe(80);
expect(validatePort(443)).toBe(443);
expect(validatePort(3000)).toBe(3000);
expect(validatePort(8080)).toBe(8080);
});
it('handles variant ports (8318-8417)', () => {
expect(validatePort(8318)).toBe(8318);
expect(validatePort(8350)).toBe(8350);
expect(validatePort(8417)).toBe(8417);
});
});
});
describe('Object Spread Undefined Filtering', () => {
/**
* Bug: `{ ...DEFAULT_CONFIG, ...{ port: undefined } }` overwrites port with undefined
* Fix: Filter undefined values before spreading
*/
it('demonstrates the original bug pattern', () => {
const DEFAULT_CONFIG = { port: 8317, timeout: 2000 };
const config = { port: undefined, timeout: undefined };
// BUG: Direct spread overwrites with undefined
const bugResult = { ...DEFAULT_CONFIG, ...config };
expect(bugResult.port).toBe(undefined); // This was the bug!
// FIX: Filter undefined values before spreading
const filteredConfig = Object.fromEntries(
Object.entries(config).filter(([, v]) => v !== undefined)
);
const fixResult = { ...DEFAULT_CONFIG, ...filteredConfig };
expect(fixResult.port).toBe(8317); // Fixed!
});
it('preserves explicit values while filtering undefined', () => {
const DEFAULT_CONFIG = { port: 8317, timeout: 2000, protocol: 'http' };
const config = { port: 9000, timeout: undefined, protocol: undefined };
const filteredConfig = Object.fromEntries(
Object.entries(config).filter(([, v]) => v !== undefined)
);
const result = { ...DEFAULT_CONFIG, ...filteredConfig };
expect(result.port).toBe(9000); // Explicit value preserved
expect(result.timeout).toBe(2000); // Default preserved (undefined filtered)
expect(result.protocol).toBe('http'); // Default preserved (undefined filtered)
});
});
describe('YAML Port Validation in resolveProxyConfig', () => {
it('validates YAML remote port', () => {
const { config } = resolveProxyConfig([], {
remote: { host: 'example.com', port: 9000 },
});
expect(config.port).toBe(9000);
});
it('falls back to default for invalid YAML port (negative)', () => {
const { config } = resolveProxyConfig([], {
remote: { host: 'example.com', port: -1 },
});
expect(config.port).toBe(CLIPROXY_DEFAULT_PORT);
});
it('falls back to default for invalid YAML port (> 65535)', () => {
const { config } = resolveProxyConfig([], {
remote: { host: 'example.com', port: 70000 },
});
expect(config.port).toBe(CLIPROXY_DEFAULT_PORT);
});
it('falls back to default for invalid YAML port (zero)', () => {
const { config } = resolveProxyConfig([], {
remote: { host: 'example.com', port: 0 },
});
expect(config.port).toBe(CLIPROXY_DEFAULT_PORT);
});
it('validates YAML local port', () => {
const { config } = resolveProxyConfig([], {
local: { port: 9000 },
});
expect(config.port).toBe(9000);
});
it('falls back to default for invalid YAML local port', () => {
const { config } = resolveProxyConfig([], {
local: { port: -100 },
});
expect(config.port).toBe(CLIPROXY_DEFAULT_PORT);
});
it('CLI port overrides invalid YAML port', () => {
const { config } = resolveProxyConfig(['--proxy-port', '8080'], {
remote: { host: 'example.com', port: -1 },
});
expect(config.port).toBe(8080);
});
});
describe('Priority Chain with Invalid Values', () => {
it('CLI valid > ENV valid > YAML invalid', () => {
process.env.CCS_PROXY_PORT = '9000';
const { config } = resolveProxyConfig(['--proxy-port', '8080'], {
remote: { host: 'example.com', port: -1 },
});
expect(config.port).toBe(8080); // CLI wins
delete process.env.CCS_PROXY_PORT;
});
it('ENV valid > YAML invalid > default', () => {
process.env.CCS_PROXY_PORT = '9000';
const { config } = resolveProxyConfig([], {
remote: { host: 'example.com', port: -1 },
});
expect(config.port).toBe(9000); // ENV wins
delete process.env.CCS_PROXY_PORT;
});
it('YAML valid when CLI and ENV not set', () => {
const { config } = resolveProxyConfig([], {
remote: { host: 'example.com', port: 9000 },
});
expect(config.port).toBe(9000);
});
it('default when all sources invalid/missing', () => {
const { config } = resolveProxyConfig([], {});
expect(config.port).toBe(CLIPROXY_DEFAULT_PORT);
});
});
describe('Boundary Conditions', () => {
it('handles port exactly at minimum boundary (1)', () => {
expect(validatePort(1)).toBe(1);
expect(validatePort(0)).toBe(CLIPROXY_DEFAULT_PORT);
});
it('handles port exactly at maximum boundary (65535)', () => {
expect(validatePort(65535)).toBe(65535);
expect(validatePort(65536)).toBe(CLIPROXY_DEFAULT_PORT);
});
it('handles reserved ports (< 1024)', () => {
// Reserved ports are still valid from validation perspective
expect(validatePort(22)).toBe(22);
expect(validatePort(80)).toBe(80);
expect(validatePort(443)).toBe(443);
});
});
});
@@ -0,0 +1,67 @@
/**
* Proxy Detector Port Validation Tests
*
* Tests for detectRunningProxy with invalid port inputs.
* Verifies the fix handles undefined, null, and invalid ports gracefully.
*/
import { describe, it, expect } from 'bun:test';
import { detectRunningProxy } from '../../../dist/cliproxy/proxy-detector';
import { CLIPROXY_DEFAULT_PORT } from '../../../dist/cliproxy/config-generator';
describe('Proxy Detector Port Validation', () => {
describe('detectRunningProxy with invalid ports', () => {
it('handles undefined port (uses default)', async () => {
// TypeScript allows this at runtime via any/unknown
const result = await detectRunningProxy(undefined as unknown as number);
// Should not throw, should use default port
expect(result).toBeDefined();
expect(typeof result.running).toBe('boolean');
});
it('handles NaN port (uses default)', async () => {
const result = await detectRunningProxy(NaN);
expect(result).toBeDefined();
expect(typeof result.running).toBe('boolean');
});
it('handles negative port (uses default)', async () => {
const result = await detectRunningProxy(-1);
expect(result).toBeDefined();
expect(typeof result.running).toBe('boolean');
});
it('handles zero port (uses default)', async () => {
const result = await detectRunningProxy(0);
expect(result).toBeDefined();
expect(typeof result.running).toBe('boolean');
});
it('handles port > 65535 (uses default)', async () => {
const result = await detectRunningProxy(70000);
expect(result).toBeDefined();
expect(typeof result.running).toBe('boolean');
});
it('handles valid port correctly', async () => {
const result = await detectRunningProxy(CLIPROXY_DEFAULT_PORT);
expect(result).toBeDefined();
expect(typeof result.running).toBe('boolean');
expect(typeof result.verified).toBe('boolean');
});
it('handles variant port correctly', async () => {
const result = await detectRunningProxy(8318);
expect(result).toBeDefined();
expect(typeof result.running).toBe('boolean');
});
});
describe('detectRunningProxy default parameter', () => {
it('works without port argument (uses default)', async () => {
const result = await detectRunningProxy();
expect(result).toBeDefined();
expect(typeof result.running).toBe('boolean');
});
});
});