mirror of
https://github.com/tiennm99/ccs.git
synced 2026-09-08 20:19:37 +00:00
fix: validate browser flags before proxy side effects
This commit is contained in:
committed by
Tam Nhu Tran
parent
62a4d44b58
commit
8114ff016e
@@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, jest } from 'bun:test';
|
|||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
import * as os from 'os';
|
import * as os from 'os';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
|
import * as http from 'http';
|
||||||
import { execClaudeWithCLIProxy, hasGitLabTokenLoginFlag, readOptionValue } from '../index';
|
import { execClaudeWithCLIProxy, hasGitLabTokenLoginFlag, readOptionValue } from '../index';
|
||||||
|
|
||||||
describe('readOptionValue', () => {
|
describe('readOptionValue', () => {
|
||||||
@@ -57,6 +58,7 @@ describe('execClaudeWithCLIProxy browser flag validation', () => {
|
|||||||
fs.writeFileSync(fakeClaudePath, '#!/bin/sh\nexit 0\n', { mode: 0o755 });
|
fs.writeFileSync(fakeClaudePath, '#!/bin/sh\nexit 0\n', { mode: 0o755 });
|
||||||
fs.chmodSync(fakeClaudePath, 0o755);
|
fs.chmodSync(fakeClaudePath, 0o755);
|
||||||
originalCcsHome = process.env.CCS_HOME;
|
originalCcsHome = process.env.CCS_HOME;
|
||||||
|
process.exitCode = 0;
|
||||||
process.env.CCS_HOME = tmpHome;
|
process.env.CCS_HOME = tmpHome;
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -66,9 +68,65 @@ describe('execClaudeWithCLIProxy browser flag validation', () => {
|
|||||||
} else {
|
} else {
|
||||||
delete process.env.CCS_HOME;
|
delete process.env.CCS_HOME;
|
||||||
}
|
}
|
||||||
|
process.exitCode = 0;
|
||||||
fs.rmSync(tmpHome, { recursive: true, force: true });
|
fs.rmSync(tmpHome, { recursive: true, force: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('validates conflicting browser launch flags before remote proxy checks', async () => {
|
||||||
|
let requestCount = 0;
|
||||||
|
const server = http.createServer((_req, res) => {
|
||||||
|
requestCount += 1;
|
||||||
|
res.writeHead(200, { 'content-type': 'application/json' });
|
||||||
|
res.end('{"ok":true}');
|
||||||
|
});
|
||||||
|
|
||||||
|
await new Promise<void>((resolve) => {
|
||||||
|
server.listen(0, '127.0.0.1', resolve);
|
||||||
|
});
|
||||||
|
|
||||||
|
const address = server.address();
|
||||||
|
if (!address || typeof address === 'string') {
|
||||||
|
server.close();
|
||||||
|
throw new Error('Test server did not bind to a TCP port');
|
||||||
|
}
|
||||||
|
|
||||||
|
const exitSpy = jest
|
||||||
|
.spyOn(process, 'exit')
|
||||||
|
.mockImplementation((() => undefined as never) as typeof process.exit);
|
||||||
|
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
|
||||||
|
|
||||||
|
try {
|
||||||
|
await execClaudeWithCLIProxy(
|
||||||
|
fakeClaudePath,
|
||||||
|
'gemini',
|
||||||
|
[
|
||||||
|
'--proxy-host',
|
||||||
|
'127.0.0.1',
|
||||||
|
'--proxy-port',
|
||||||
|
String(address.port),
|
||||||
|
'--proxy-auth-token',
|
||||||
|
'SECRET_TOKEN_FOR_VALIDATION',
|
||||||
|
'--remote-only',
|
||||||
|
'--browser',
|
||||||
|
'--no-browser',
|
||||||
|
],
|
||||||
|
{}
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(exitSpy).toHaveBeenCalledWith(1);
|
||||||
|
expect(errorSpy).toHaveBeenCalledWith(
|
||||||
|
'[X] Use either `--browser` or `--no-browser`, not both.'
|
||||||
|
);
|
||||||
|
expect(requestCount).toBe(0);
|
||||||
|
} finally {
|
||||||
|
exitSpy.mockRestore();
|
||||||
|
errorSpy.mockRestore();
|
||||||
|
await new Promise<void>((resolve, reject) => {
|
||||||
|
server.close((error) => (error ? reject(error) : resolve()));
|
||||||
|
});
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
it('exits cleanly when conflicting browser launch flags are provided', async () => {
|
it('exits cleanly when conflicting browser launch flags are provided', async () => {
|
||||||
const exitSpy = jest
|
const exitSpy = jest
|
||||||
.spyOn(process, 'exit')
|
.spyOn(process, 'exit')
|
||||||
|
|||||||
@@ -9,7 +9,7 @@
|
|||||||
* 4. Browser MCP ensure + sync-to-config-dir
|
* 4. Browser MCP ensure + sync-to-config-dir
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { warn } from '../../utils/ui';
|
import { fail, warn } from '../../utils/ui';
|
||||||
import {
|
import {
|
||||||
type BrowserLaunchOverride,
|
type BrowserLaunchOverride,
|
||||||
ensureBrowserMcpOrThrow,
|
ensureBrowserMcpOrThrow,
|
||||||
@@ -49,7 +49,8 @@ export function resolveBrowserLaunchFlags(argsWithoutProxy: string[]): {
|
|||||||
browserLaunchOverride = browserLaunchFlags.override;
|
browserLaunchOverride = browserLaunchFlags.override;
|
||||||
argsWithoutBrowserFlags = browserLaunchFlags.argsWithoutFlags;
|
argsWithoutBrowserFlags = browserLaunchFlags.argsWithoutFlags;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.error(warn((error as Error).message));
|
console.error(fail((error as Error).message));
|
||||||
|
process.exitCode = 1;
|
||||||
process.exit(1);
|
process.exit(1);
|
||||||
return { browserLaunchOverride: undefined, argsWithoutBrowserFlags };
|
return { browserLaunchOverride: undefined, argsWithoutBrowserFlags };
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -63,7 +63,7 @@ import {
|
|||||||
} from './thinking-override-resolver';
|
} from './thinking-override-resolver';
|
||||||
import { shouldStartHttpsTunnel } from './https-tunnel-policy';
|
import { shouldStartHttpsTunnel } from './https-tunnel-policy';
|
||||||
import { filterCcsFlags, parseExecutorFlags, validateFlagCombinations } from './arg-parser';
|
import { filterCcsFlags, parseExecutorFlags, validateFlagCombinations } from './arg-parser';
|
||||||
import { resolveExecutorProxy } from './proxy-resolver';
|
import { resolveExecutorProxy, resolveExecutorProxyConfig } from './proxy-resolver';
|
||||||
import { buildProxyChain } from './proxy-chain-builder';
|
import { buildProxyChain } from './proxy-chain-builder';
|
||||||
import { warnBrokenModels } from './model-warnings';
|
import { warnBrokenModels } from './model-warnings';
|
||||||
import { launchClaude } from './claude-launcher';
|
import { launchClaude } from './claude-launcher';
|
||||||
@@ -129,8 +129,21 @@ export async function execClaudeWithCLIProxy(
|
|||||||
// Collect all providers to validate (default + composite tiers)
|
// Collect all providers to validate (default + composite tiers)
|
||||||
const allProviders = [provider, ...compositeProviders];
|
const allProviders = [provider, ...compositeProviders];
|
||||||
|
|
||||||
|
const proxyResolution = resolveExecutorProxyConfig(args, {
|
||||||
|
unifiedConfig,
|
||||||
|
allProviders,
|
||||||
|
verbose,
|
||||||
|
cfg,
|
||||||
|
log,
|
||||||
|
});
|
||||||
|
|
||||||
|
const { browserLaunchOverride, argsWithoutBrowserFlags } = resolveBrowserLaunchFlags(
|
||||||
|
proxyResolution.argsWithoutProxy
|
||||||
|
);
|
||||||
|
if (process.exitCode === 1) return;
|
||||||
|
|
||||||
const { proxyConfig, useRemoteProxy, localBackend, binaryPath, argsWithoutProxy } =
|
const { proxyConfig, useRemoteProxy, localBackend, binaryPath, argsWithoutProxy } =
|
||||||
await resolveExecutorProxy(args, {
|
await resolveExecutorProxy(proxyResolution, {
|
||||||
unifiedConfig,
|
unifiedConfig,
|
||||||
allProviders,
|
allProviders,
|
||||||
verbose,
|
verbose,
|
||||||
@@ -138,9 +151,6 @@ export async function execClaudeWithCLIProxy(
|
|||||||
log,
|
log,
|
||||||
});
|
});
|
||||||
|
|
||||||
const { browserLaunchOverride, argsWithoutBrowserFlags } =
|
|
||||||
resolveBrowserLaunchFlags(argsWithoutProxy);
|
|
||||||
|
|
||||||
// Setup first-class CCS WebSearch runtime
|
// Setup first-class CCS WebSearch runtime
|
||||||
ensureWebSearchMcpOrThrow();
|
ensureWebSearchMcpOrThrow();
|
||||||
const imageAnalysisMcpReady = ensureImageAnalysisMcpOrThrow();
|
const imageAnalysisMcpReady = ensureImageAnalysisMcpOrThrow();
|
||||||
|
|||||||
@@ -24,20 +24,23 @@ import type { ResolvedProxyConfig } from '../types';
|
|||||||
import type { UnifiedConfig } from '../../config/schemas/unified-config';
|
import type { UnifiedConfig } from '../../config/schemas/unified-config';
|
||||||
import { isNetworkError, handleNetworkError } from './retry-handler';
|
import { isNetworkError, handleNetworkError } from './retry-handler';
|
||||||
|
|
||||||
/** Result returned from resolveExecutorProxy */
|
export interface ResolvedExecutorProxyConfig {
|
||||||
export interface ResolvedProxy {
|
|
||||||
/** Resolved proxy config after merging CLI > ENV > config.yaml > defaults */
|
/** Resolved proxy config after merging CLI > ENV > config.yaml > defaults */
|
||||||
proxyConfig: ResolvedProxyConfig;
|
proxyConfig: ResolvedProxyConfig;
|
||||||
|
/** Args after proxy-related flags are stripped out */
|
||||||
|
argsWithoutProxy: string[];
|
||||||
|
/** Mutated executor config (port resolved and validated) */
|
||||||
|
cfg: ExecutorConfig;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Result returned from resolveExecutorProxy */
|
||||||
|
export interface ResolvedProxy extends ResolvedExecutorProxyConfig {
|
||||||
/** Whether to use the remote proxy (vs spawning a local one) */
|
/** Whether to use the remote proxy (vs spawning a local one) */
|
||||||
useRemoteProxy: boolean;
|
useRemoteProxy: boolean;
|
||||||
/** Which local backend binary to use ('original' | 'plus') */
|
/** Which local backend binary to use ('original' | 'plus') */
|
||||||
localBackend: CLIProxyBackend;
|
localBackend: CLIProxyBackend;
|
||||||
/** Absolute path to CLIProxy binary; undefined when useRemoteProxy=true */
|
/** Absolute path to CLIProxy binary; undefined when useRemoteProxy=true */
|
||||||
binaryPath: string | undefined;
|
binaryPath: string | undefined;
|
||||||
/** Args after proxy-related flags are stripped out */
|
|
||||||
argsWithoutProxy: string[];
|
|
||||||
/** Mutated executor config (port resolved and validated) */
|
|
||||||
cfg: ExecutorConfig;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Dependencies injected by the orchestrator */
|
/** Dependencies injected by the orchestrator */
|
||||||
@@ -50,16 +53,15 @@ export interface ResolveExecutorProxyContext {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Resolves proxy configuration, checks remote reachability, selects the local
|
* Resolves side-effect-free proxy configuration and strips proxy flags.
|
||||||
* backend, and ensures the CLIProxy binary is present when running locally.
|
|
||||||
*
|
*
|
||||||
* Mutates `context.cfg.port` in-place (same as original orchestrator behaviour).
|
* Mutates `context.cfg.port` in-place (same as original orchestrator behaviour).
|
||||||
*/
|
*/
|
||||||
export async function resolveExecutorProxy(
|
export function resolveExecutorProxyConfig(
|
||||||
args: string[],
|
args: string[],
|
||||||
context: ResolveExecutorProxyContext
|
context: ResolveExecutorProxyContext
|
||||||
): Promise<ResolvedProxy> {
|
): ResolvedExecutorProxyConfig {
|
||||||
const { unifiedConfig, allProviders, verbose: _verbose, cfg, log } = context;
|
const { unifiedConfig, cfg, log } = context;
|
||||||
|
|
||||||
// Resolve proxy config from CLI flags > ENV > config.yaml > defaults
|
// Resolve proxy config from CLI flags > ENV > config.yaml > defaults
|
||||||
const cliproxyServerConfig = unifiedConfig.cliproxy_server;
|
const cliproxyServerConfig = unifiedConfig.cliproxy_server;
|
||||||
@@ -98,6 +100,20 @@ export async function resolveExecutorProxy(
|
|||||||
log(`Remote host: ${proxyConfig.host}:${proxyConfig.port} (${proxyConfig.protocol})`);
|
log(`Remote host: ${proxyConfig.host}:${proxyConfig.port} (${proxyConfig.protocol})`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return { proxyConfig, argsWithoutProxy, cfg };
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolves proxy configuration, checks remote reachability, selects the local
|
||||||
|
* backend, and ensures the CLIProxy binary is present when running locally.
|
||||||
|
*/
|
||||||
|
export async function resolveExecutorProxy(
|
||||||
|
resolvedConfig: ResolvedExecutorProxyConfig,
|
||||||
|
context: ResolveExecutorProxyContext
|
||||||
|
): Promise<ResolvedProxy> {
|
||||||
|
const { allProviders, verbose: _verbose } = context;
|
||||||
|
const { proxyConfig, argsWithoutProxy, cfg } = resolvedConfig;
|
||||||
|
|
||||||
// Check remote proxy reachability
|
// Check remote proxy reachability
|
||||||
let useRemoteProxy = false;
|
let useRemoteProxy = false;
|
||||||
let localBackend: CLIProxyBackend = 'original';
|
let localBackend: CLIProxyBackend = 'original';
|
||||||
|
|||||||
@@ -483,9 +483,8 @@ export async function reconcileExhaustedRotationAccounts(
|
|||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
|
|
||||||
const { pauseAccountForQuotaCooldown, restoreExpiredQuotaPauses } = await import(
|
const { pauseAccountForQuotaCooldown, restoreExpiredQuotaPauses } =
|
||||||
'../accounts/account-safety'
|
await import('../accounts/account-safety');
|
||||||
);
|
|
||||||
restoreExpiredQuotaPauses();
|
restoreExpiredQuotaPauses();
|
||||||
|
|
||||||
const config = loadOrCreateUnifiedConfig();
|
const config = loadOrCreateUnifiedConfig();
|
||||||
@@ -593,9 +592,8 @@ export async function preflightCheck(provider: CLIProxyProvider): Promise<Prefli
|
|||||||
return { proceed: true, accountId: defaultAccount?.id || '' };
|
return { proceed: true, accountId: defaultAccount?.id || '' };
|
||||||
}
|
}
|
||||||
|
|
||||||
const { pauseAccountForQuotaCooldown, restoreExpiredQuotaPauses } = await import(
|
const { pauseAccountForQuotaCooldown, restoreExpiredQuotaPauses } =
|
||||||
'../accounts/account-safety'
|
await import('../accounts/account-safety');
|
||||||
);
|
|
||||||
restoreExpiredQuotaPauses();
|
restoreExpiredQuotaPauses();
|
||||||
|
|
||||||
const config = loadOrCreateUnifiedConfig();
|
const config = loadOrCreateUnifiedConfig();
|
||||||
|
|||||||
@@ -112,9 +112,8 @@ export async function runImageAnalysisCheck(results: HealthCheck): Promise<void>
|
|||||||
* Fix image analysis configuration issues
|
* Fix image analysis configuration issues
|
||||||
*/
|
*/
|
||||||
export async function fixImageAnalysisConfig(): Promise<boolean> {
|
export async function fixImageAnalysisConfig(): Promise<boolean> {
|
||||||
const { updateConfig, loadOrCreateUnifiedConfig } = await import(
|
const { updateConfig, loadOrCreateUnifiedConfig } =
|
||||||
'../../config/config-loader-facade'
|
await import('../../config/config-loader-facade');
|
||||||
);
|
|
||||||
|
|
||||||
const config = loadOrCreateUnifiedConfig();
|
const config = loadOrCreateUnifiedConfig();
|
||||||
let fixed = false;
|
let fixed = false;
|
||||||
|
|||||||
Reference in New Issue
Block a user