fix(security) - Use emitFeedback instead of console error (#11954)

Co-authored-by: gemini-cli-robot <gemini-cli-robot@google.com>
This commit is contained in:
shishu314
2025-10-29 10:49:46 -04:00
committed by GitHub
parent 372b588778
commit 6e026bd950
2 changed files with 40 additions and 9 deletions

View File

@@ -8,6 +8,7 @@ import { type Credentials } from 'google-auth-library';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { OAuthCredentialStorage } from './oauth-credential-storage.js';
import type { OAuthCredentials } from '../mcp/token-storage/types.js';
import { coreEvents } from '@google/gemini-cli-core';
import * as path from 'node:path';
import * as os from 'node:os';
@@ -30,6 +31,11 @@ vi.mock('node:fs', () => ({
}));
vi.mock('node:os');
vi.mock('node:path');
vi.mock('@google/gemini-cli-core', () => ({
coreEvents: {
emitFeedback: vi.fn(),
},
}));
describe('OAuthCredentialStorage', () => {
const mockCredentials: Credentials = {
@@ -119,26 +125,36 @@ describe('OAuthCredentialStorage', () => {
});
it('should throw an error if loading fails', async () => {
const mockError = new Error('HybridTokenStorage error');
vi.spyOn(mockHybridTokenStorage, 'getCredentials').mockRejectedValue(
new Error('Loading error'),
mockError,
);
await expect(OAuthCredentialStorage.loadCredentials()).rejects.toThrow(
'Failed to load OAuth credentials',
);
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
'error',
'Failed to load OAuth credentials',
mockError,
);
});
it('should throw an error if read file fails', async () => {
const mockError = new Error('Permission denied');
vi.spyOn(mockHybridTokenStorage, 'getCredentials').mockResolvedValue(
null,
);
vi.spyOn(fs, 'readFile').mockRejectedValue(
new Error('Permission denied'),
);
vi.spyOn(fs, 'readFile').mockRejectedValue(mockError);
await expect(OAuthCredentialStorage.loadCredentials()).rejects.toThrow(
'Failed to load OAuth credentials',
);
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
'error',
'Failed to load OAuth credentials',
mockError,
);
});
it('should not throw error if migration file removal failed', async () => {
@@ -205,13 +221,19 @@ describe('OAuthCredentialStorage', () => {
});
it('should throw an error if clearing from HybridTokenStorage fails', async () => {
const mockError = new Error('Deletion error');
vi.spyOn(mockHybridTokenStorage, 'deleteCredentials').mockRejectedValue(
new Error('Deletion error'),
mockError,
);
await expect(OAuthCredentialStorage.clearCredentials()).rejects.toThrow(
'Failed to clear OAuth credentials',
);
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
'error',
'Failed to clear OAuth credentials',
mockError,
);
});
});
});

View File

@@ -12,6 +12,7 @@ import * as path from 'node:path';
import * as os from 'node:os';
import { promises as fs } from 'node:fs';
import { GEMINI_DIR } from '../utils/paths.js';
import { coreEvents } from '@google/gemini-cli-core';
const KEYCHAIN_SERVICE_NAME = 'gemini-cli-oauth';
const MAIN_ACCOUNT_KEY = 'main-account';
@@ -49,8 +50,12 @@ export class OAuthCredentialStorage {
// Fallback: Try to migrate from old file-based storage
return await this.migrateFromFileStorage();
} catch (error: unknown) {
console.error(error);
throw new Error('Failed to load OAuth credentials');
coreEvents.emitFeedback(
'error',
'Failed to load OAuth credentials',
error,
);
throw new Error('Failed to load OAuth credentials', { cause: error });
}
}
@@ -89,8 +94,12 @@ export class OAuthCredentialStorage {
const oldFilePath = path.join(os.homedir(), GEMINI_DIR, OAUTH_FILE);
await fs.rm(oldFilePath, { force: true }).catch(() => {});
} catch (error: unknown) {
console.error(error);
throw new Error('Failed to clear OAuth credentials');
coreEvents.emitFeedback(
'error',
'Failed to clear OAuth credentials',
error,
);
throw new Error('Failed to clear OAuth credentials', { cause: error });
}
}