From 25bcee2fa1994fd4e74f81b007a237dfa3937d46 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Tue, 7 Apr 2026 08:27:52 -0400 Subject: [PATCH] fix(kiro): require fresh token before auth success --- src/cliproxy/auth/oauth-handler.ts | 55 +--- src/cliproxy/auth/oauth-process.ts | 119 ++++++--- src/cliproxy/auth/token-manager.ts | 234 ++++++++++++------ .../oauth-process-error-parser.test.ts | 119 +++++++++ 4 files changed, 365 insertions(+), 162 deletions(-) diff --git a/src/cliproxy/auth/oauth-handler.ts b/src/cliproxy/auth/oauth-handler.ts index fb50989e..2185598e 100644 --- a/src/cliproxy/auth/oauth-handler.ts +++ b/src/cliproxy/auth/oauth-handler.ts @@ -11,7 +11,6 @@ */ import * as fs from 'fs'; -import * as path from 'path'; import { fail, info, warn, color, ok } from '../../utils/ui'; import { ensureCLIProxyBinary } from '../binary-manager'; import { generateConfig } from '../config-generator'; @@ -48,9 +47,11 @@ import { } from './auth-types'; import { isHeadlessEnvironment, killProcessOnPort, showStep } from './environment-detector'; import { + ProviderTokenSnapshot, + findNewTokenSnapshotForAuthAttempt, getProviderTokenDir, isAuthenticated, - isTokenFileForProvider, + listProviderTokenSnapshots, registerAccountFromToken, } from './token-manager'; import { executeOAuthProcess } from './oauth-process'; @@ -79,11 +80,6 @@ interface PasteCallbackStartData { const PASTE_CALLBACK_AUTH_URL_POLL_INTERVAL_MS = 3000; const POLLED_AUTH_LOCAL_TOKEN_GRACE_MS = 15 * 1000; -type ProviderTokenSnapshot = { - file: string; - mtimeMs: number; -}; - export async function requestPasteCallbackStart( provider: CLIProxyProvider, target: ProxyTarget, @@ -146,56 +142,13 @@ function parseAuthUrlState(url: string | null | undefined): string | null { } } -function listProviderTokenSnapshots( - provider: CLIProxyProvider, - tokenDir: string -): ProviderTokenSnapshot[] { - if (!fs.existsSync(tokenDir)) { - return []; - } - - return fs - .readdirSync(tokenDir) - .filter((file) => file.endsWith('.json')) - .map((file): ProviderTokenSnapshot | null => { - const filePath = path.join(tokenDir, file); - if (!isTokenFileForProvider(filePath, provider)) { - return null; - } - - return { - file, - mtimeMs: fs.statSync(filePath).mtimeMs, - }; - }) - .filter((snapshot): snapshot is ProviderTokenSnapshot => snapshot !== null) - .sort((left, right) => right.mtimeMs - left.mtimeMs); -} - export function findNewTokenSnapshotForManualAuth( provider: CLIProxyProvider, tokenDir: string, knownTokenFiles: ProviderTokenSnapshot[], expectedAccountId?: string ): ProviderTokenSnapshot | null { - const knownTokenMtimes = new Map( - knownTokenFiles.map((snapshot) => [snapshot.file, snapshot.mtimeMs]) - ); - - return ( - listProviderTokenSnapshots(provider, tokenDir).find((snapshot) => { - const knownMtime = knownTokenMtimes.get(snapshot.file); - if (knownMtime === undefined) { - return true; - } - - if (!expectedAccountId) { - return false; - } - - return snapshot.mtimeMs > knownMtime + 1; - }) || null - ); + return findNewTokenSnapshotForAuthAttempt(provider, tokenDir, knownTokenFiles, expectedAccountId); } async function waitForManualCallbackToken( diff --git a/src/cliproxy/auth/oauth-process.ts b/src/cliproxy/auth/oauth-process.ts index bce9491c..b4cd3dd1 100644 --- a/src/cliproxy/auth/oauth-process.ts +++ b/src/cliproxy/auth/oauth-process.ts @@ -24,7 +24,12 @@ import { } from '../project-selection-handler'; import { KiroAuthMethod, ProviderOAuthConfig } from './auth-types'; import { getTimeoutTroubleshooting, showStep } from './environment-detector'; -import { isAuthenticated, registerAccountFromToken } from './token-manager'; +import { + type ProviderTokenSnapshot, + findNewTokenSnapshot, + listProviderTokenSnapshots, + registerAccountFromToken, +} from './token-manager'; import { deviceCodeEvents, DEVICE_CODE_TIMEOUT_MS, @@ -476,20 +481,15 @@ function displayUrlFromStderr( const ANSI_ESCAPE_REGEX = /\x1b\[[0-9;]*m/g; -export function extractLikelyAuthFailureFromStderr( +export function extractLikelyAuthFailureFromLogs( provider: CLIProxyProvider, - stderrData: string + logData: string ): string | null { - // Keep this scoped to ghcp to avoid over-classifying other providers. - if (provider !== 'ghcp') { + if (!logData.trim()) { return null; } - if (!stderrData.trim()) { - return null; - } - - const normalizedLines = stderrData + const normalizedLines = logData .split('\n') .map((line) => line.replace(ANSI_ESCAPE_REGEX, '').trim()) .filter(Boolean) @@ -507,11 +507,23 @@ export function extractLikelyAuthFailureFromStderr( return line; }); + const providerPatterns: Partial> = { + ghcp: [ + /github copilot authentication failed:\s*(.+)/i, + /failed to verify copilot access[^:]*:\s*(.+)/i, + ], + kiro: [ + /kiro idc authentication failed:\s*(.+)/i, + /kiro authentication failed:\s*(.+)/i, + /login failed:\s*(.+)/i, + /failed to register client:\s*(.+)/i, + ], + }; + const prioritizedPatterns = [ - /github copilot authentication failed:\s*(.+)/i, - /authentication failed:\s*(.+)/i, - /failed to verify copilot access[^:]*:\s*(.+)/i, - /failed to save auth:\s*(.+)/i, + ...(providerPatterns[provider] || []), + /^authentication failed:\s*(.+)/i, + /^failed to save auth:\s*(.+)/i, ]; for (let i = normalizedLines.length - 1; i >= 0; i--) { @@ -527,6 +539,34 @@ export function extractLikelyAuthFailureFromStderr( return null; } +export function extractLikelyAuthFailureFromStderr( + provider: CLIProxyProvider, + stderrData: string +): string | null { + return extractLikelyAuthFailureFromLogs(provider, stderrData); +} + +export function analyzeSuccessfulAuthExit(options: { + provider: CLIProxyProvider; + knownTokenFiles: ProviderTokenSnapshot[]; + currentTokenFiles: ProviderTokenSnapshot[]; + expectedAccountId?: string; + stdoutData: string; + stderrData: string; +}): { tokenSnapshot: ProviderTokenSnapshot | null; failureReason: string | null } { + const tokenSnapshot = findNewTokenSnapshot( + options.currentTokenFiles, + options.knownTokenFiles, + options.expectedAccountId + ); + const failureReason = extractLikelyAuthFailureFromLogs( + options.provider, + [options.stdoutData, options.stderrData].filter(Boolean).join('\n') + ); + + return { tokenSnapshot, failureReason }; +} + /** Handle token not found after successful process exit */ async function handleTokenNotFound( provider: CLIProxyProvider, @@ -537,9 +577,22 @@ async function handleTokenNotFound( verbose: boolean, failureReason?: string ): Promise { + console.log(''); + + if (failureReason) { + // Sanitize internal URLs/paths from failure reason to avoid leaking infrastructure details + const sanitizedReason = failureReason + .replace(/https?:\/\/(?:localhost|127\.0\.0\.1|0\.0\.0\.0)[^\s]*/gi, '[internal-url]') + .replace(/\/(?:root|home|opt|tmp|var)\/[^\s]*/g, '[path]'); + console.log(fail('Authentication failed before a usable token was saved')); + console.log(` ${sanitizedReason}`); + console.log(''); + console.log(`Try: ccs ${provider} --auth --verbose`); + return null; + } + // Kiro-specific: Try auto-import from Kiro IDE if (provider === 'kiro') { - console.log(''); console.log(warn('Callback redirected to Kiro IDE. Attempting to import token...')); const result = await tryKiroImport(tokenDir, verbose); @@ -558,24 +611,6 @@ async function handleTokenNotFound( return null; } - // Default behavior for other providers - console.log(''); - - if (failureReason) { - // Sanitize internal URLs/paths from failure reason to avoid leaking infrastructure details - const sanitizedReason = failureReason - .replace(/https?:\/\/(?:localhost|127\.0\.0\.1|0\.0\.0\.0)[^\s]*/gi, '[internal-url]') - .replace(/\/(?:root|home|opt|tmp|var)\/[^\s]*/g, '[path]'); - console.log(fail('Authentication completed but token was not persisted')); - console.log(` ${sanitizedReason}`); - console.log(''); - console.log('This usually means provider-side authorization was accepted,'); - console.log('but CLIProxy failed a post-auth verification or token save step.'); - console.log(''); - console.log(`Try: ccs ${provider} --auth --verbose`); - return null; - } - console.log(fail('Token not found after authentication')); console.log(''); console.log('The browser showed success but callback was not received.'); @@ -644,6 +679,7 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise((resolve) => { const flowType = resolveAuthFlowType(options); const isDeviceCodeFlow = flowType === 'device_code'; + const knownTokenFiles = listProviderTokenSnapshots(provider, tokenDir); // Device-code flows can usually inherit stdin, but Kiro's default AWS flow now // prints an intermediate Builder ID vs IDC selector that CCS auto-answers. @@ -814,7 +850,16 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise; + +function buildTokenFingerprint(content: string): string { + return createHash('sha256').update(content).digest('hex'); +} + +function listTokenCandidates(provider: CLIProxyProvider, tokenDir: string): TokenCandidate[] { + if (!fs.existsSync(tokenDir)) { + return []; + } + + const files = fs.readdirSync(tokenDir); + const jsonFiles = files.filter((file) => file.endsWith('.json')); + const existingAccounts = getProviderAccounts(provider); + const rawCandidates: RawTokenCandidate[] = jsonFiles.flatMap((file) => { + const filePath = path.join(tokenDir, file); + if (!isTokenFileForProvider(filePath, provider)) { + return []; + } + + const content = fs.readFileSync(filePath, 'utf-8'); + const data = JSON.parse(content) as { email?: string; project_id?: string }; + const email = data.email || undefined; + const projectId = data.project_id || undefined; + const stats = fs.statSync(filePath); + + return [ + { + file, + filePath, + email, + projectId, + mtimeMs: stats.mtimeMs, + alreadyRegistered: existingAccounts.some((account) => account.tokenFile === file), + }, + ]; + }); + + const duplicateEmailCounts = new Map(); + const duplicateEmailTokenSets = new Map>(); + for (const account of existingAccounts) { + if (!account.email) continue; + const key = account.email.toLowerCase(); + const tokenSet = duplicateEmailTokenSets.get(key) ?? new Set(); + tokenSet.add(account.tokenFile); + duplicateEmailTokenSets.set(key, tokenSet); + } + for (const candidate of rawCandidates) { + if (!candidate.email) continue; + const key = candidate.email.toLowerCase(); + const tokenSet = duplicateEmailTokenSets.get(key) ?? new Set(); + tokenSet.add(candidate.file); + duplicateEmailTokenSets.set(key, tokenSet); + } + for (const [key, tokenSet] of duplicateEmailTokenSets) { + duplicateEmailCounts.set(key, tokenSet.size); + } + + return rawCandidates + .map((rawCandidate) => { + const content = fs.readFileSync(rawCandidate.filePath, 'utf-8'); + const duplicateEmailCount = rawCandidate.email + ? (duplicateEmailCounts.get(rawCandidate.email.toLowerCase()) ?? 1) + : 1; + const accountId = rawCandidate.email + ? buildEmailBackedAccountId( + provider, + rawCandidate.file, + rawCandidate.email, + duplicateEmailCount + ) + : extractAccountIdFromTokenFile(rawCandidate.file, rawCandidate.email); + + return { + ...rawCandidate, + accountId, + fingerprint: buildTokenFingerprint(content), + }; + }) + .sort((left, right) => right.mtimeMs - left.mtimeMs); +} + +export function listProviderTokenSnapshots( + provider: CLIProxyProvider, + tokenDir: string = getProviderTokenDir(provider) +): ProviderTokenSnapshot[] { + return listTokenCandidates(provider, tokenDir).map((candidate) => ({ + file: candidate.file, + mtimeMs: candidate.mtimeMs, + accountId: candidate.accountId, + fingerprint: candidate.fingerprint, + })); +} + +export function findNewTokenSnapshot( + currentTokenFiles: ProviderTokenSnapshot[], + knownTokenFiles: ProviderTokenSnapshot[], + expectedAccountId?: string +): ProviderTokenSnapshot | null { + const knownSnapshotsByFile = new Map( + knownTokenFiles.map((snapshot) => [snapshot.file, snapshot]) + ); + + return ( + currentTokenFiles.find((snapshot) => { + const knownSnapshot = knownSnapshotsByFile.get(snapshot.file); + if (!expectedAccountId) { + return !knownSnapshot; + } + + const matchesExpectedAccount = + snapshot.file === expectedAccountId || snapshot.accountId === expectedAccountId; + if (!matchesExpectedAccount) { + return false; + } + + if (!knownSnapshot) { + return true; + } + + return ( + snapshot.fingerprint !== knownSnapshot.fingerprint || + snapshot.mtimeMs !== knownSnapshot.mtimeMs + ); + }) || null + ); +} + +export function findNewTokenSnapshotForAuthAttempt( + provider: CLIProxyProvider, + tokenDir: string, + knownTokenFiles: ProviderTokenSnapshot[], + expectedAccountId?: string +): ProviderTokenSnapshot | null { + return findNewTokenSnapshot( + listProviderTokenSnapshots(provider, tokenDir), + knownTokenFiles, + expectedAccountId + ); +} + /** * Check if provider has valid authentication * CLIProxyAPI stores OAuth tokens as JSON files in the auth directory. @@ -207,83 +367,11 @@ export function registerAccountFromToken( verbose = false, expectedAccountId?: string ): import('../account-manager').AccountInfo | null { - type TokenCandidate = { - file: string; - filePath: string; - email?: string; - projectId?: string; - accountId: string; - mtimeMs: number; - alreadyRegistered: boolean; - }; - type RawTokenCandidate = Omit; - const { registerAccount } = require('../account-manager'); let selectedCandidate: Omit | null = null; try { - const files = fs.readdirSync(tokenDir); - const jsonFiles = files.filter((f: string) => f.endsWith('.json')); + const candidates = listTokenCandidates(provider, tokenDir); const existingAccounts = getProviderAccounts(provider); - const rawCandidates: RawTokenCandidate[] = jsonFiles.flatMap((file) => { - const filePath = path.join(tokenDir, file); - if (!isTokenFileForProvider(filePath, provider)) return []; - - const content = fs.readFileSync(filePath, 'utf-8'); - const data = JSON.parse(content) as { email?: string; project_id?: string }; - const email = data.email || undefined; - const projectId = data.project_id || undefined; - const stats = fs.statSync(filePath); - - return [ - { - file, - filePath, - email, - projectId, - mtimeMs: stats.mtimeMs, - alreadyRegistered: existingAccounts.some((account) => account.tokenFile === file), - }, - ]; - }); - const duplicateEmailCounts = new Map(); - const duplicateEmailTokenSets = new Map>(); - for (const account of existingAccounts) { - if (!account.email) continue; - const key = account.email.toLowerCase(); - const tokenSet = duplicateEmailTokenSets.get(key) ?? new Set(); - tokenSet.add(account.tokenFile); - duplicateEmailTokenSets.set(key, tokenSet); - } - for (const candidate of rawCandidates) { - if (!candidate.email) continue; - const key = candidate.email.toLowerCase(); - const tokenSet = duplicateEmailTokenSets.get(key) ?? new Set(); - tokenSet.add(candidate.file); - duplicateEmailTokenSets.set(key, tokenSet); - } - for (const [key, tokenSet] of duplicateEmailTokenSets) { - duplicateEmailCounts.set(key, tokenSet.size); - } - const candidates: TokenCandidate[] = rawCandidates - .map((rawCandidate) => { - const duplicateEmailCount = rawCandidate.email - ? (duplicateEmailCounts.get(rawCandidate.email.toLowerCase()) ?? 1) - : 1; - const accountId = rawCandidate.email - ? buildEmailBackedAccountId( - provider, - rawCandidate.file, - rawCandidate.email, - duplicateEmailCount - ) - : extractAccountIdFromTokenFile(rawCandidate.file, rawCandidate.email); - - return { - ...rawCandidate, - accountId, - }; - }) - .sort((a, b) => b.mtimeMs - a.mtimeMs); if (expectedAccountId) { selectedCandidate = diff --git a/tests/unit/cliproxy/oauth-process-error-parser.test.ts b/tests/unit/cliproxy/oauth-process-error-parser.test.ts index a9d7f806..4c1e26e9 100644 --- a/tests/unit/cliproxy/oauth-process-error-parser.test.ts +++ b/tests/unit/cliproxy/oauth-process-error-parser.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from 'bun:test'; import { + analyzeSuccessfulAuthExit, + extractLikelyAuthFailureFromLogs, extractLikelyAuthFailureFromStderr, extractLikelyOAuthAuthorizationUrl, getExpectedLocalCallback, @@ -38,6 +40,123 @@ describe('oauth-process stderr parsing', () => { expect(parsed).not.toBeNull(); expect((parsed as string).length).toBe(240); }); + + it('extracts kiro IDC failures from verbose stdout logs', () => { + const logData = + '[2026-04-07 11:01:21] [--------] [error] [kiro_login.go:236] Kiro IDC authentication failed: login failed: failed to register client: register client failed (status 400)'; + + expect(extractLikelyAuthFailureFromLogs('kiro', logData)).toBe( + 'login failed: failed to register client: register client failed (status 400)' + ); + }); +}); + +describe('oauth-process successful exit analysis', () => { + it('treats unchanged existing kiro tokens as a failed add-account attempt', () => { + const result = analyzeSuccessfulAuthExit({ + provider: 'kiro', + knownTokenFiles: [{ file: 'kiro-existing.json', mtimeMs: 100 }], + currentTokenFiles: [{ file: 'kiro-existing.json', mtimeMs: 100 }], + stdoutData: + '[error] Kiro IDC authentication failed: login failed: failed to register client: register client failed (status 400)', + stderrData: '', + }); + + expect(result.tokenSnapshot).toBeNull(); + expect(result.failureReason).toBe( + 'login failed: failed to register client: register client failed (status 400)' + ); + }); + + it('treats a refreshed token file as success during reauth', () => { + const result = analyzeSuccessfulAuthExit({ + provider: 'kiro', + knownTokenFiles: [ + { + file: 'kiro-existing.json', + mtimeMs: 100, + accountId: 'kiro-existing', + fingerprint: 'before', + }, + ], + currentTokenFiles: [ + { + file: 'kiro-existing.json', + mtimeMs: 250, + accountId: 'kiro-existing', + fingerprint: 'after', + }, + ], + expectedAccountId: 'kiro-existing.json', + stdoutData: '', + stderrData: '', + }); + + expect(result.tokenSnapshot?.file).toBe('kiro-existing.json'); + expect(result.failureReason).toBeNull(); + }); + + it('ignores unrelated new token files during reauth', () => { + const result = analyzeSuccessfulAuthExit({ + provider: 'kiro', + knownTokenFiles: [ + { + file: 'kiro-existing.json', + mtimeMs: 100, + accountId: 'kiro-existing', + fingerprint: 'before', + }, + ], + currentTokenFiles: [ + { + file: 'kiro-existing.json', + mtimeMs: 100, + accountId: 'kiro-existing', + fingerprint: 'before', + }, + { + file: 'kiro-other.json', + mtimeMs: 150, + accountId: 'kiro-other', + fingerprint: 'other-after', + }, + ], + expectedAccountId: 'kiro-existing', + stdoutData: '', + stderrData: '', + }); + + expect(result.tokenSnapshot).toBeNull(); + expect(result.failureReason).toBeNull(); + }); + + it('treats fingerprint changes as success even when mtime is unchanged', () => { + const result = analyzeSuccessfulAuthExit({ + provider: 'kiro', + knownTokenFiles: [ + { + file: 'kiro-existing.json', + mtimeMs: 100, + accountId: 'kiro-existing', + fingerprint: 'before', + }, + ], + currentTokenFiles: [ + { + file: 'kiro-existing.json', + mtimeMs: 100, + accountId: 'kiro-existing', + fingerprint: 'after', + }, + ], + expectedAccountId: 'kiro-existing', + stdoutData: '', + stderrData: '', + }); + + expect(result.tokenSnapshot?.file).toBe('kiro-existing.json'); + expect(result.failureReason).toBeNull(); + }); }); describe('oauth-process manual callback validation', () => {