fix(core, a2a-server): prevent hang during OAuth in non-interactive sessions (#21045)

This commit is contained in:
Spencer
2026-03-04 15:35:21 -05:00
committed by GitHub
parent 3640b72947
commit 1bf1637ea2
5 changed files with 335 additions and 13 deletions
+225 -7
View File
@@ -16,6 +16,9 @@ import {
ExperimentFlags, ExperimentFlags,
fetchAdminControlsOnce, fetchAdminControlsOnce,
type FetchAdminControlsResponse, type FetchAdminControlsResponse,
AuthType,
isHeadlessMode,
FatalAuthenticationError,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
// Mock dependencies // Mock dependencies
@@ -50,6 +53,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
startupProfiler: { startupProfiler: {
flush: vi.fn(), flush: vi.fn(),
}, },
isHeadlessMode: vi.fn().mockReturnValue(false),
FileDiscoveryService: vi.fn(), FileDiscoveryService: vi.fn(),
getCodeAssistServer: vi.fn(), getCodeAssistServer: vi.fn(),
fetchAdminControlsOnce: vi.fn(), fetchAdminControlsOnce: vi.fn(),
@@ -62,6 +66,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
vi.mock('../utils/logger.js', () => ({ vi.mock('../utils/logger.js', () => ({
logger: { logger: {
info: vi.fn(), info: vi.fn(),
warn: vi.fn(),
error: vi.fn(), error: vi.fn(),
}, },
})); }));
@@ -73,12 +78,11 @@ describe('loadConfig', () => {
beforeEach(() => { beforeEach(() => {
vi.clearAllMocks(); vi.clearAllMocks();
process.env['GEMINI_API_KEY'] = 'test-key'; vi.stubEnv('GEMINI_API_KEY', 'test-key');
}); });
afterEach(() => { afterEach(() => {
delete process.env['CUSTOM_IGNORE_FILE_PATHS']; vi.unstubAllEnvs();
delete process.env['GEMINI_API_KEY'];
}); });
describe('admin settings overrides', () => { describe('admin settings overrides', () => {
@@ -199,7 +203,7 @@ describe('loadConfig', () => {
it('should set customIgnoreFilePaths when CUSTOM_IGNORE_FILE_PATHS env var is present', async () => { it('should set customIgnoreFilePaths when CUSTOM_IGNORE_FILE_PATHS env var is present', async () => {
const testPath = '/tmp/ignore'; const testPath = '/tmp/ignore';
process.env['CUSTOM_IGNORE_FILE_PATHS'] = testPath; vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', testPath);
const config = await loadConfig(mockSettings, mockExtensionLoader, taskId); const config = await loadConfig(mockSettings, mockExtensionLoader, taskId);
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([ expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([
@@ -224,7 +228,7 @@ describe('loadConfig', () => {
it('should merge customIgnoreFilePaths from settings and env var', async () => { it('should merge customIgnoreFilePaths from settings and env var', async () => {
const envPath = '/env/ignore'; const envPath = '/env/ignore';
const settingsPath = '/settings/ignore'; const settingsPath = '/settings/ignore';
process.env['CUSTOM_IGNORE_FILE_PATHS'] = envPath; vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', envPath);
const settings: Settings = { const settings: Settings = {
fileFiltering: { fileFiltering: {
customIgnoreFilePaths: [settingsPath], customIgnoreFilePaths: [settingsPath],
@@ -240,7 +244,7 @@ describe('loadConfig', () => {
it('should split CUSTOM_IGNORE_FILE_PATHS using system delimiter', async () => { it('should split CUSTOM_IGNORE_FILE_PATHS using system delimiter', async () => {
const paths = ['/path/one', '/path/two']; const paths = ['/path/one', '/path/two'];
process.env['CUSTOM_IGNORE_FILE_PATHS'] = paths.join(path.delimiter); vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', paths.join(path.delimiter));
const config = await loadConfig(mockSettings, mockExtensionLoader, taskId); const config = await loadConfig(mockSettings, mockExtensionLoader, taskId);
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual(paths); expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual(paths);
@@ -254,7 +258,7 @@ describe('loadConfig', () => {
it('should initialize FileDiscoveryService with correct options', async () => { it('should initialize FileDiscoveryService with correct options', async () => {
const testPath = '/tmp/ignore'; const testPath = '/tmp/ignore';
process.env['CUSTOM_IGNORE_FILE_PATHS'] = testPath; vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', testPath);
const settings: Settings = { const settings: Settings = {
fileFiltering: { fileFiltering: {
respectGitIgnore: false, respectGitIgnore: false,
@@ -311,5 +315,219 @@ describe('loadConfig', () => {
}), }),
); );
}); });
describe('interactivity', () => {
it('should set interactive true when not headless', async () => {
vi.mocked(isHeadlessMode).mockReturnValue(false);
await loadConfig(mockSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith(
expect.objectContaining({
interactive: true,
enableInteractiveShell: true,
}),
);
});
it('should set interactive false when headless', async () => {
vi.mocked(isHeadlessMode).mockReturnValue(true);
await loadConfig(mockSettings, mockExtensionLoader, taskId);
expect(Config).toHaveBeenCalledWith(
expect.objectContaining({
interactive: false,
enableInteractiveShell: false,
}),
);
});
});
describe('authentication fallback', () => {
beforeEach(() => {
vi.stubEnv('USE_CCPA', 'true');
vi.stubEnv('GEMINI_API_KEY', '');
});
afterEach(() => {
vi.unstubAllEnvs();
});
it('should fall back to COMPUTE_ADC in Cloud Shell if LOGIN_WITH_GOOGLE fails', async () => {
vi.stubEnv('CLOUD_SHELL', 'true');
vi.mocked(isHeadlessMode).mockReturnValue(false);
const refreshAuthMock = vi.fn().mockImplementation((authType) => {
if (authType === AuthType.LOGIN_WITH_GOOGLE) {
throw new FatalAuthenticationError('Non-interactive session');
}
return Promise.resolve();
});
// Update the mock implementation for this test
vi.mocked(Config).mockImplementation(
(params: unknown) =>
({
...(params as object),
initialize: vi.fn(),
waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(),
}) as unknown as Config,
);
await loadConfig(mockSettings, mockExtensionLoader, taskId);
expect(refreshAuthMock).toHaveBeenCalledWith(
AuthType.LOGIN_WITH_GOOGLE,
);
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.COMPUTE_ADC);
});
it('should not fall back to COMPUTE_ADC if not in cloud environment', async () => {
vi.mocked(isHeadlessMode).mockReturnValue(false);
const refreshAuthMock = vi.fn().mockImplementation((authType) => {
if (authType === AuthType.LOGIN_WITH_GOOGLE) {
throw new FatalAuthenticationError('Non-interactive session');
}
return Promise.resolve();
});
vi.mocked(Config).mockImplementation(
(params: unknown) =>
({
...(params as object),
initialize: vi.fn(),
waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(),
}) as unknown as Config,
);
await expect(
loadConfig(mockSettings, mockExtensionLoader, taskId),
).rejects.toThrow('Non-interactive session');
expect(refreshAuthMock).toHaveBeenCalledWith(
AuthType.LOGIN_WITH_GOOGLE,
);
expect(refreshAuthMock).not.toHaveBeenCalledWith(AuthType.COMPUTE_ADC);
});
it('should skip LOGIN_WITH_GOOGLE and use COMPUTE_ADC directly in headless Cloud Shell', async () => {
vi.stubEnv('CLOUD_SHELL', 'true');
vi.mocked(isHeadlessMode).mockReturnValue(true);
const refreshAuthMock = vi.fn().mockResolvedValue(undefined);
vi.mocked(Config).mockImplementation(
(params: unknown) =>
({
...(params as object),
initialize: vi.fn(),
waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(),
}) as unknown as Config,
);
await loadConfig(mockSettings, mockExtensionLoader, taskId);
expect(refreshAuthMock).not.toHaveBeenCalledWith(
AuthType.LOGIN_WITH_GOOGLE,
);
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.COMPUTE_ADC);
});
it('should skip LOGIN_WITH_GOOGLE and use COMPUTE_ADC directly if GEMINI_CLI_USE_COMPUTE_ADC is true', async () => {
vi.stubEnv('GEMINI_CLI_USE_COMPUTE_ADC', 'true');
vi.mocked(isHeadlessMode).mockReturnValue(false); // Even if not headless
const refreshAuthMock = vi.fn().mockResolvedValue(undefined);
vi.mocked(Config).mockImplementation(
(params: unknown) =>
({
...(params as object),
initialize: vi.fn(),
waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(),
}) as unknown as Config,
);
await loadConfig(mockSettings, mockExtensionLoader, taskId);
expect(refreshAuthMock).not.toHaveBeenCalledWith(
AuthType.LOGIN_WITH_GOOGLE,
);
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.COMPUTE_ADC);
});
it('should throw FatalAuthenticationError in headless mode if no ADC fallback available', async () => {
vi.mocked(isHeadlessMode).mockReturnValue(true);
const refreshAuthMock = vi.fn().mockResolvedValue(undefined);
vi.mocked(Config).mockImplementation(
(params: unknown) =>
({
...(params as object),
initialize: vi.fn(),
waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(),
}) as unknown as Config,
);
await expect(
loadConfig(mockSettings, mockExtensionLoader, taskId),
).rejects.toThrow(
'Interactive terminal required for LOGIN_WITH_GOOGLE. Run in an interactive terminal or set GEMINI_CLI_USE_COMPUTE_ADC=true to use Application Default Credentials.',
);
expect(refreshAuthMock).not.toHaveBeenCalled();
});
it('should include both original and fallback error when COMPUTE_ADC fallback fails', async () => {
vi.stubEnv('CLOUD_SHELL', 'true');
vi.mocked(isHeadlessMode).mockReturnValue(false);
const refreshAuthMock = vi.fn().mockImplementation((authType) => {
if (authType === AuthType.LOGIN_WITH_GOOGLE) {
throw new FatalAuthenticationError('OAuth failed');
}
if (authType === AuthType.COMPUTE_ADC) {
throw new Error('ADC failed');
}
return Promise.resolve();
});
vi.mocked(Config).mockImplementation(
(params: unknown) =>
({
...(params as object),
initialize: vi.fn(),
waitForMcpInit: vi.fn(),
refreshAuth: refreshAuthMock,
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
getRemoteAdminSettings: vi.fn(),
setRemoteAdminSettings: vi.fn(),
}) as unknown as Config,
);
await expect(
loadConfig(mockSettings, mockExtensionLoader, taskId),
).rejects.toThrow(
'OAuth failed. Fallback to COMPUTE_ADC also failed: ADC failed',
);
});
});
}); });
}); });
+60 -3
View File
@@ -23,6 +23,9 @@ import {
fetchAdminControlsOnce, fetchAdminControlsOnce,
getCodeAssistServer, getCodeAssistServer,
ExperimentFlags, ExperimentFlags,
isHeadlessMode,
FatalAuthenticationError,
isCloudShell,
type TelemetryTarget, type TelemetryTarget,
type ConfigParameters, type ConfigParameters,
type ExtensionLoader, type ExtensionLoader,
@@ -103,8 +106,8 @@ export async function loadConfig(
trustedFolder: true, trustedFolder: true,
extensionLoader, extensionLoader,
checkpointing, checkpointing,
interactive: true, interactive: !isHeadlessMode(),
enableInteractiveShell: true, enableInteractiveShell: !isHeadlessMode(),
ptyInfo: 'auto', ptyInfo: 'auto',
}; };
@@ -255,7 +258,61 @@ async function refreshAuthentication(
`[${logPrefix}] USE_CCPA env var is true but unable to resolve GOOGLE_APPLICATION_CREDENTIALS file path ${adcFilePath}. Error ${e}`, `[${logPrefix}] USE_CCPA env var is true but unable to resolve GOOGLE_APPLICATION_CREDENTIALS file path ${adcFilePath}. Error ${e}`,
); );
} }
await config.refreshAuth(AuthType.LOGIN_WITH_GOOGLE);
const useComputeAdc = process.env['GEMINI_CLI_USE_COMPUTE_ADC'] === 'true';
const isHeadless = isHeadlessMode();
const shouldSkipOauth = isHeadless || useComputeAdc;
if (shouldSkipOauth) {
if (isCloudShell() || useComputeAdc) {
logger.info(
`[${logPrefix}] Skipping LOGIN_WITH_GOOGLE due to ${isHeadless ? 'headless mode' : 'GEMINI_CLI_USE_COMPUTE_ADC'}. Attempting COMPUTE_ADC.`,
);
try {
await config.refreshAuth(AuthType.COMPUTE_ADC);
logger.info(`[${logPrefix}] COMPUTE_ADC successful.`);
} catch (adcError) {
const adcMessage =
adcError instanceof Error ? adcError.message : String(adcError);
throw new FatalAuthenticationError(
`COMPUTE_ADC failed: ${adcMessage}. (Skipped LOGIN_WITH_GOOGLE due to ${isHeadless ? 'headless mode' : 'GEMINI_CLI_USE_COMPUTE_ADC'})`,
);
}
} else {
throw new FatalAuthenticationError(
`Interactive terminal required for LOGIN_WITH_GOOGLE. Run in an interactive terminal or set GEMINI_CLI_USE_COMPUTE_ADC=true to use Application Default Credentials.`,
);
}
} else {
try {
await config.refreshAuth(AuthType.LOGIN_WITH_GOOGLE);
} catch (e) {
if (
e instanceof FatalAuthenticationError &&
(isCloudShell() || useComputeAdc)
) {
logger.warn(
`[${logPrefix}] LOGIN_WITH_GOOGLE failed. Attempting COMPUTE_ADC fallback.`,
);
try {
await config.refreshAuth(AuthType.COMPUTE_ADC);
logger.info(`[${logPrefix}] COMPUTE_ADC fallback successful.`);
} catch (adcError) {
logger.error(
`[${logPrefix}] COMPUTE_ADC fallback failed: ${adcError}`,
);
const originalMessage = e instanceof Error ? e.message : String(e);
const adcMessage =
adcError instanceof Error ? adcError.message : String(adcError);
throw new FatalAuthenticationError(
`${originalMessage}. Fallback to COMPUTE_ADC also failed: ${adcMessage}`,
);
}
} else {
throw e;
}
}
}
logger.info( logger.info(
`[${logPrefix}] GOOGLE_CLOUD_PROJECT: ${process.env['GOOGLE_CLOUD_PROJECT']}`, `[${logPrefix}] GOOGLE_CLOUD_PROJECT: ${process.env['GOOGLE_CLOUD_PROJECT']}`,
); );
+27 -1
View File
@@ -40,7 +40,10 @@ import { FORCE_ENCRYPTED_FILE_ENV_VAR } from '../mcp/token-storage/index.js';
import { GEMINI_DIR, homedir as pathsHomedir } from '../utils/paths.js'; import { GEMINI_DIR, homedir as pathsHomedir } from '../utils/paths.js';
import { debugLogger } from '../utils/debugLogger.js'; import { debugLogger } from '../utils/debugLogger.js';
import { writeToStdout } from '../utils/stdio.js'; import { writeToStdout } from '../utils/stdio.js';
import { FatalCancellationError } from '../utils/errors.js'; import {
FatalCancellationError,
FatalAuthenticationError,
} from '../utils/errors.js';
import process from 'node:process'; import process from 'node:process';
import { coreEvents } from '../utils/events.js'; import { coreEvents } from '../utils/events.js';
import { isHeadlessMode } from '../utils/headless.js'; import { isHeadlessMode } from '../utils/headless.js';
@@ -107,6 +110,7 @@ const mockConfig = {
getProxy: () => 'http://test.proxy.com:8080', getProxy: () => 'http://test.proxy.com:8080',
isBrowserLaunchSuppressed: () => false, isBrowserLaunchSuppressed: () => false,
getExperimentalZedIntegration: () => false, getExperimentalZedIntegration: () => false,
isInteractive: () => true,
} as unknown as Config; } as unknown as Config;
// Mock fetch globally // Mock fetch globally
@@ -316,11 +320,31 @@ describe('oauth2', () => {
await eventPromise; await eventPromise;
}); });
it('should throw FatalAuthenticationError in non-interactive session when manual auth is required', async () => {
const mockConfigNonInteractive = {
getNoBrowser: () => true,
getProxy: () => 'http://test.proxy.com:8080',
isBrowserLaunchSuppressed: () => true,
isInteractive: () => false,
} as unknown as Config;
await expect(
getOauthClient(AuthType.LOGIN_WITH_GOOGLE, mockConfigNonInteractive),
).rejects.toThrow(FatalAuthenticationError);
await expect(
getOauthClient(AuthType.LOGIN_WITH_GOOGLE, mockConfigNonInteractive),
).rejects.toThrow(
'Manual authorization is required but the current session is non-interactive.',
);
});
it('should perform login with user code', async () => { it('should perform login with user code', async () => {
const mockConfigWithNoBrowser = { const mockConfigWithNoBrowser = {
getNoBrowser: () => true, getNoBrowser: () => true,
getProxy: () => 'http://test.proxy.com:8080', getProxy: () => 'http://test.proxy.com:8080',
isBrowserLaunchSuppressed: () => true, isBrowserLaunchSuppressed: () => true,
isInteractive: () => true,
} as unknown as Config; } as unknown as Config;
const mockCodeVerifier = { const mockCodeVerifier = {
@@ -391,6 +415,7 @@ describe('oauth2', () => {
getNoBrowser: () => true, getNoBrowser: () => true,
getProxy: () => 'http://test.proxy.com:8080', getProxy: () => 'http://test.proxy.com:8080',
isBrowserLaunchSuppressed: () => true, isBrowserLaunchSuppressed: () => true,
isInteractive: () => true,
} as unknown as Config; } as unknown as Config;
const mockCodeVerifier = { const mockCodeVerifier = {
@@ -1171,6 +1196,7 @@ describe('oauth2', () => {
getNoBrowser: () => true, getNoBrowser: () => true,
getProxy: () => 'http://test.proxy.com:8080', getProxy: () => 'http://test.proxy.com:8080',
isBrowserLaunchSuppressed: () => true, isBrowserLaunchSuppressed: () => true,
isInteractive: () => true,
} as unknown as Config; } as unknown as Config;
const mockOAuth2Client = { const mockOAuth2Client = {
+18 -1
View File
@@ -226,6 +226,13 @@ async function initOauthClient(
} }
if (config.isBrowserLaunchSuppressed()) { if (config.isBrowserLaunchSuppressed()) {
if (!config.isInteractive()) {
throw new FatalAuthenticationError(
'Manual authorization is required but the current session is non-interactive. ' +
'Please run the Gemini CLI in an interactive terminal to log in, ' +
'provide a GEMINI_API_KEY, or ensure Application Default Credentials are configured.',
);
}
let success = false; let success = false;
const maxRetries = 2; const maxRetries = 2;
// Enter alternate buffer // Enter alternate buffer
@@ -412,14 +419,24 @@ async function authWithUserCode(client: OAuth2Client): Promise<boolean> {
'\n\n', '\n\n',
); );
const code = await new Promise<string>((resolve, _) => { const code = await new Promise<string>((resolve, reject) => {
const rl = readline.createInterface({ const rl = readline.createInterface({
input: process.stdin, input: process.stdin,
output: createWorkingStdio().stdout, output: createWorkingStdio().stdout,
terminal: true, terminal: true,
}); });
const timeout = setTimeout(() => {
rl.close();
reject(
new FatalAuthenticationError(
'Authorization timed out after 5 minutes.',
),
);
}, 300000); // 5 minute timeout
rl.question('Enter the authorization code: ', (code) => { rl.question('Enter the authorization code: ', (code) => {
clearTimeout(timeout);
rl.close(); rl.close();
resolve(code.trim()); resolve(code.trim());
}); });
+5 -1
View File
@@ -130,7 +130,11 @@ export * from './skills/skillLoader.js';
export * from './ide/ide-client.js'; export * from './ide/ide-client.js';
export * from './ide/ideContext.js'; export * from './ide/ideContext.js';
export * from './ide/ide-installer.js'; export * from './ide/ide-installer.js';
export { IDE_DEFINITIONS, type IdeInfo } from './ide/detect-ide.js'; export {
IDE_DEFINITIONS,
type IdeInfo,
isCloudShell,
} from './ide/detect-ide.js';
export * from './ide/constants.js'; export * from './ide/constants.js';
export * from './ide/types.js'; export * from './ide/types.js';