mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 04:18:05 +00:00
fix(cliproxy): improve skip-local-auth edge case handling
- Validate authToken with trim() to reject whitespace-only values - Show broken model warning for both remote and local modes - Add context-aware messaging for remote proxy users - Add comprehensive test coverage for authToken edge cases: - Whitespace-only strings - Null values - Tabs/newlines - Tokens with leading/trailing whitespace Co-authored-by: Shun Kakinoki <shunkakinoki@gmail.com>
This commit is contained in:
co-authored by
Shun Kakinoki
parent
1f5d11930e
commit
21e819b590
@@ -500,7 +500,9 @@ export async function execClaudeWithCLIProxy(
|
||||
// 3. Ensure OAuth completed (if provider requires it)
|
||||
// Skip local OAuth check when using remote proxy with auth token
|
||||
// The remote proxy has its own OAuth sessions and handles authentication
|
||||
const skipLocalAuth = useRemoteProxy && proxyConfig.authToken;
|
||||
// Note: Trim authToken to reject whitespace-only values
|
||||
const remoteAuthToken = proxyConfig.authToken?.trim();
|
||||
const skipLocalAuth = useRemoteProxy && !!remoteAuthToken;
|
||||
if (skipLocalAuth) {
|
||||
log(`Using remote proxy authentication (skipping local OAuth)`);
|
||||
}
|
||||
@@ -585,21 +587,23 @@ export async function execClaudeWithCLIProxy(
|
||||
}
|
||||
|
||||
// 5. Check for known broken models and warn user
|
||||
// Skip for remote proxy - model selection is on the remote server
|
||||
if (!skipLocalAuth) {
|
||||
const currentModel = getCurrentModel(provider, cfg.customSettingsPath);
|
||||
if (currentModel && isModelBroken(provider, currentModel)) {
|
||||
const modelEntry = findModel(provider, currentModel);
|
||||
const issueUrl = getModelIssueUrl(provider, currentModel);
|
||||
console.error('');
|
||||
console.error(warn(`${modelEntry?.name || currentModel} has known issues with Claude Code`));
|
||||
console.error(' Tool calls will fail. Use "gemini-3-pro-preview" instead.');
|
||||
if (issueUrl) {
|
||||
console.error(` Tracking: ${issueUrl}`);
|
||||
}
|
||||
console.error(` Run "ccs ${provider} --config" to change model.`);
|
||||
console.error('');
|
||||
// Show warning for both local and remote modes - user should be aware of model issues
|
||||
const currentModel = getCurrentModel(provider, cfg.customSettingsPath);
|
||||
if (currentModel && isModelBroken(provider, currentModel)) {
|
||||
const modelEntry = findModel(provider, currentModel);
|
||||
const issueUrl = getModelIssueUrl(provider, currentModel);
|
||||
console.error('');
|
||||
console.error(warn(`${modelEntry?.name || currentModel} has known issues with Claude Code`));
|
||||
console.error(' Tool calls will fail. Use "gemini-3-pro-preview" instead.');
|
||||
if (issueUrl) {
|
||||
console.error(` Tracking: ${issueUrl}`);
|
||||
}
|
||||
if (skipLocalAuth) {
|
||||
console.error(' Note: Model may be overridden by remote proxy configuration.');
|
||||
} else {
|
||||
console.error(` Run "ccs ${provider} --config" to change model.`);
|
||||
}
|
||||
console.error('');
|
||||
}
|
||||
|
||||
// 6. Ensure user settings file exists (creates from defaults if not)
|
||||
|
||||
@@ -3,54 +3,116 @@
|
||||
*
|
||||
* When --proxy-host and --proxy-auth-token are provided together, the system should
|
||||
* skip local OAuth checks because the remote proxy handles authentication.
|
||||
*
|
||||
* Implementation uses: const remoteAuthToken = proxyConfig.authToken?.trim();
|
||||
* const skipLocalAuth = useRemoteProxy && !!remoteAuthToken;
|
||||
*/
|
||||
import { describe, it, expect } from 'bun:test';
|
||||
|
||||
/**
|
||||
* Helper to compute skipLocalAuth exactly as the implementation does
|
||||
* Mirrors logic from cliproxy-executor.ts lines 503-505
|
||||
*/
|
||||
function computeSkipLocalAuth(
|
||||
useRemoteProxy: boolean,
|
||||
proxyConfig: { authToken?: string | null }
|
||||
): boolean {
|
||||
const remoteAuthToken = proxyConfig.authToken?.trim();
|
||||
return useRemoteProxy && !!remoteAuthToken;
|
||||
}
|
||||
|
||||
describe('skip-local-auth logic', () => {
|
||||
describe('skipLocalAuth flag determination', () => {
|
||||
it('should skip local auth when both useRemoteProxy and authToken are truthy', () => {
|
||||
const useRemoteProxy = true;
|
||||
const proxyConfig = { authToken: 'test-token-123' };
|
||||
|
||||
const skipLocalAuth = useRemoteProxy && proxyConfig.authToken;
|
||||
const skipLocalAuth = computeSkipLocalAuth(useRemoteProxy, proxyConfig);
|
||||
|
||||
expect(skipLocalAuth).toBeTruthy();
|
||||
expect(skipLocalAuth).toBe(true);
|
||||
});
|
||||
|
||||
it('should NOT skip local auth when useRemoteProxy is false', () => {
|
||||
const useRemoteProxy = false;
|
||||
const proxyConfig = { authToken: 'test-token-123' };
|
||||
|
||||
const skipLocalAuth = useRemoteProxy && proxyConfig.authToken;
|
||||
const skipLocalAuth = computeSkipLocalAuth(useRemoteProxy, proxyConfig);
|
||||
|
||||
expect(skipLocalAuth).toBeFalsy();
|
||||
expect(skipLocalAuth).toBe(false);
|
||||
});
|
||||
|
||||
it('should NOT skip local auth when authToken is undefined', () => {
|
||||
const useRemoteProxy = true;
|
||||
const proxyConfig = { authToken: undefined };
|
||||
|
||||
const skipLocalAuth = useRemoteProxy && proxyConfig.authToken;
|
||||
const skipLocalAuth = computeSkipLocalAuth(useRemoteProxy, proxyConfig);
|
||||
|
||||
expect(skipLocalAuth).toBeFalsy();
|
||||
expect(skipLocalAuth).toBe(false);
|
||||
});
|
||||
|
||||
it('should NOT skip local auth when authToken is empty string', () => {
|
||||
const useRemoteProxy = true;
|
||||
const proxyConfig = { authToken: '' };
|
||||
|
||||
const skipLocalAuth = useRemoteProxy && proxyConfig.authToken;
|
||||
const skipLocalAuth = computeSkipLocalAuth(useRemoteProxy, proxyConfig);
|
||||
|
||||
expect(skipLocalAuth).toBeFalsy();
|
||||
expect(skipLocalAuth).toBe(false);
|
||||
});
|
||||
|
||||
it('should NOT skip local auth when both are falsy', () => {
|
||||
const useRemoteProxy = false;
|
||||
const proxyConfig = { authToken: undefined };
|
||||
|
||||
const skipLocalAuth = useRemoteProxy && proxyConfig.authToken;
|
||||
const skipLocalAuth = computeSkipLocalAuth(useRemoteProxy, proxyConfig);
|
||||
|
||||
expect(skipLocalAuth).toBeFalsy();
|
||||
expect(skipLocalAuth).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('authToken edge cases', () => {
|
||||
it('should NOT skip local auth when authToken is whitespace-only', () => {
|
||||
const useRemoteProxy = true;
|
||||
const proxyConfig = { authToken: ' ' };
|
||||
|
||||
const skipLocalAuth = computeSkipLocalAuth(useRemoteProxy, proxyConfig);
|
||||
|
||||
expect(skipLocalAuth).toBe(false);
|
||||
});
|
||||
|
||||
it('should NOT skip local auth when authToken is tabs and newlines', () => {
|
||||
const useRemoteProxy = true;
|
||||
const proxyConfig = { authToken: '\t\n\r' };
|
||||
|
||||
const skipLocalAuth = computeSkipLocalAuth(useRemoteProxy, proxyConfig);
|
||||
|
||||
expect(skipLocalAuth).toBe(false);
|
||||
});
|
||||
|
||||
it('should NOT skip local auth when authToken is null', () => {
|
||||
const useRemoteProxy = true;
|
||||
const proxyConfig = { authToken: null };
|
||||
|
||||
const skipLocalAuth = computeSkipLocalAuth(useRemoteProxy, proxyConfig);
|
||||
|
||||
expect(skipLocalAuth).toBe(false);
|
||||
});
|
||||
|
||||
it('should skip local auth when authToken has leading/trailing whitespace but valid content', () => {
|
||||
const useRemoteProxy = true;
|
||||
const proxyConfig = { authToken: ' valid-token-123 ' };
|
||||
|
||||
const skipLocalAuth = computeSkipLocalAuth(useRemoteProxy, proxyConfig);
|
||||
|
||||
expect(skipLocalAuth).toBe(true);
|
||||
});
|
||||
|
||||
it('should skip local auth when authToken contains special characters', () => {
|
||||
const useRemoteProxy = true;
|
||||
const proxyConfig = { authToken: 'Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.test' };
|
||||
|
||||
const skipLocalAuth = computeSkipLocalAuth(useRemoteProxy, proxyConfig);
|
||||
|
||||
expect(skipLocalAuth).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -152,26 +214,33 @@ describe('skip-local-auth logic', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('broken model warning bypass', () => {
|
||||
it('should skip broken model warning when using remote proxy with auth', () => {
|
||||
describe('broken model warning behavior', () => {
|
||||
it('should show broken model warning in BOTH remote and local modes', () => {
|
||||
// Updated behavior: warnings always shown (with different messaging for remote)
|
||||
// Remote users need to know about broken models too
|
||||
const currentModel = 'some-broken-model';
|
||||
const isModelBroken = true;
|
||||
|
||||
// Warning should show regardless of skipLocalAuth
|
||||
const shouldWarnRemote = currentModel && isModelBroken; // skipLocalAuth=true
|
||||
const shouldWarnLocal = currentModel && isModelBroken; // skipLocalAuth=false
|
||||
|
||||
expect(shouldWarnRemote).toBe(true);
|
||||
expect(shouldWarnLocal).toBe(true);
|
||||
});
|
||||
|
||||
it('should show different message for remote vs local mode', () => {
|
||||
const skipLocalAuth = true;
|
||||
const currentModel = 'some-broken-model';
|
||||
const isModelBroken = true;
|
||||
|
||||
// Logic: only warn if NOT skipping local auth
|
||||
const shouldWarn = !skipLocalAuth && currentModel && isModelBroken;
|
||||
// When remote: "Note: Model may be overridden by remote proxy configuration."
|
||||
// When local: "Run ccs <provider> --config to change model."
|
||||
const remoteMessage = skipLocalAuth
|
||||
? 'Note: Model may be overridden by remote proxy configuration.'
|
||||
: 'Run "ccs provider --config" to change model.';
|
||||
|
||||
expect(shouldWarn).toBe(false);
|
||||
});
|
||||
|
||||
it('should show broken model warning when using local mode', () => {
|
||||
const skipLocalAuth = false;
|
||||
const currentModel = 'some-broken-model';
|
||||
const isModelBroken = true;
|
||||
|
||||
const shouldWarn = !skipLocalAuth && currentModel && isModelBroken;
|
||||
|
||||
expect(shouldWarn).toBe(true);
|
||||
expect(remoteMessage).toContain('remote proxy');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user