diff --git a/packages/core/src/providers/provider-config.ts b/packages/core/src/providers/provider-config.ts index 6bb0ed7ac..bdfa64de4 100644 --- a/packages/core/src/providers/provider-config.ts +++ b/packages/core/src/providers/provider-config.ts @@ -368,11 +368,20 @@ export function providerMatchesCredentials( } catch (err) { // A throw here is a programming error in the provider's envKey fn, // not an expected "no match" — surface it so a custom provider - // silently vanishing from /doctor / system-info has a trace. + // silently vanishing from /doctor / system-info has a trace. Log + // only the host (a custom baseUrl can embed credentials like + // https://user:sk-secret@host) and the error message, not the raw + // error object / full URL. + let safeHost: string; + try { + safeHost = new URL(baseUrl).hostname; + } catch { + safeHost = '[invalid]'; + } // eslint-disable-next-line no-console -- diagnostic for a misconfigured provider console.warn( - `[providerMatchesCredentials] envKey(${proto}, ${baseUrl}) threw; skipping this protocol:`, - err, + `[providerMatchesCredentials] envKey(${proto}, ${safeHost}) threw; skipping this protocol:`, + err instanceof Error ? err.message : String(err), ); } } diff --git a/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.test.ts b/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.test.ts index 5651d7ab7..22490e8f4 100644 --- a/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.test.ts +++ b/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.test.ts @@ -23,6 +23,9 @@ const { mockWriteCodingPlanConfig, mockWriteModelProvidersConfig, mockClearPersistedAuth, + mockApplyProviderInstallPlanToFile, + mockSnapshotSettingsForRollback, + mockRestoreSettingsSnapshot, slashCommandNotificationCallbackRef, endTurnCallbackRef, streamChunkCallbackRef, @@ -77,6 +80,11 @@ const { mockWriteCodingPlanConfig: vi.fn(() => ({})), mockWriteModelProvidersConfig: vi.fn(), mockClearPersistedAuth: vi.fn(), + mockApplyProviderInstallPlanToFile: vi.fn().mockResolvedValue(undefined), + mockSnapshotSettingsForRollback: vi.fn<() => Record | null>( + () => null, + ), + mockRestoreSettingsSnapshot: vi.fn(), slashCommandNotificationCallbackRef: { current: undefined as | ((event: { @@ -166,9 +174,9 @@ vi.mock('../../services/settingsWriter.js', () => ({ writeModelProvidersConfig: mockWriteModelProvidersConfig, readQwenSettingsForVSCode: mockReadQwenSettingsForVSCode, clearPersistedAuth: mockClearPersistedAuth, - applyProviderInstallPlanToFile: vi.fn().mockResolvedValue(undefined), - snapshotSettingsForRollback: vi.fn().mockReturnValue(null), - restoreSettingsSnapshot: vi.fn(), + applyProviderInstallPlanToFile: mockApplyProviderInstallPlanToFile, + snapshotSettingsForRollback: mockSnapshotSettingsForRollback, + restoreSettingsSnapshot: mockRestoreSettingsSnapshot, })); vi.mock('../../services/qwenAgentManager.js', () => ({ @@ -1685,3 +1693,125 @@ describe('Notification & dot indicator', () => { ); }); }); + +describe('WebViewProvider.handleAuthInteractive credential rollback', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockSnapshotSettingsForRollback.mockReturnValue(null); + }); + + // Minimal real-ish provider config + inputs so the real buildInstallPlan + // (core is not mocked beyond Storage) produces a valid plan. + const providerConfig = { + id: 'deepseek', + label: 'DeepSeek', + protocol: 'openai', + baseUrl: 'https://api.deepseek.com', + envKey: 'DEEPSEEK_API_KEY', + models: [{ id: 'deepseek-v4-flash' }], + modelNamePrefix: 'DeepSeek', + } as unknown as Parameters[0]; + const inputs = { + baseUrl: 'https://api.deepseek.com', + apiKey: 'sk-bad-key', + modelIds: ['deepseek-v4-flash'], + } as unknown as Parameters[1]; + + function makeProvider() { + const provider = new WebViewProvider( + { subscriptions: [] } as never, + { fsPath: '/extension-root' } as never, + ); + // Avoid touching the real webview pipe. + ( + provider as unknown as { sendMessageToWebView: () => void } + ).sendMessageToWebView = vi.fn(); + return provider; + } + + it('restores the snapshot when the reconnect leaves authState !== true', async () => { + const snapshot = { env: { OPENAI_API_KEY: 'sk-old' } }; + mockSnapshotSettingsForRollback.mockReturnValue(snapshot); + + const provider = makeProvider(); + // doInitializeAgentConnection runs but the backend rejects the key, so + // authState stays false. + ( + provider as unknown as { + doInitializeAgentConnection: () => Promise; + authState: boolean; + } + ).doInitializeAgentConnection = vi.fn(async () => { + (provider as unknown as { authState: boolean }).authState = false; + }); + + await ( + provider as unknown as { + handleAuthInteractive: (c: unknown, i: unknown) => Promise; + } + ).handleAuthInteractive(providerConfig, inputs); + + expect(mockApplyProviderInstallPlanToFile).toHaveBeenCalledTimes(1); + expect(mockRestoreSettingsSnapshot).toHaveBeenCalledWith(snapshot); + }); + + it('does NOT restore when the reconnect authenticates (authState === true)', async () => { + mockSnapshotSettingsForRollback.mockReturnValue({ + env: { OPENAI_API_KEY: 'sk-old' }, + }); + + const provider = makeProvider(); + ( + provider as unknown as { + doInitializeAgentConnection: () => Promise; + authState: boolean; + } + ).doInitializeAgentConnection = vi.fn(async () => { + (provider as unknown as { authState: boolean }).authState = true; + }); + + await ( + provider as unknown as { + handleAuthInteractive: (c: unknown, i: unknown) => Promise; + } + ).handleAuthInteractive(providerConfig, inputs); + + expect(mockRestoreSettingsSnapshot).not.toHaveBeenCalled(); + }); + + it('swallows a rollback write failure so the authError message still sends', async () => { + mockSnapshotSettingsForRollback.mockReturnValue({ + env: { OPENAI_API_KEY: 'sk-old' }, + }); + // restore itself throws (e.g. EPERM on Windows renameSync). + mockRestoreSettingsSnapshot.mockImplementation(() => { + throw new Error('EPERM: rename failed'); + }); + + const provider = makeProvider(); + const sendToWebView = ( + provider as unknown as { sendMessageToWebView: ReturnType } + ).sendMessageToWebView; + ( + provider as unknown as { + doInitializeAgentConnection: () => Promise; + authState: boolean; + } + ).doInitializeAgentConnection = vi.fn(async () => { + (provider as unknown as { authState: boolean }).authState = false; + }); + + await expect( + ( + provider as unknown as { + handleAuthInteractive: (c: unknown, i: unknown) => Promise; + } + ).handleAuthInteractive(providerConfig, inputs), + ).resolves.toBeUndefined(); + + // The rollback throw must not prevent the user-facing authError. + expect(sendToWebView).toHaveBeenCalledWith( + expect.objectContaining({ type: 'authError' }), + ); + }); +}); diff --git a/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.ts b/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.ts index 9b494d7b5..07bf5a433 100644 --- a/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.ts +++ b/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.ts @@ -1335,6 +1335,19 @@ export class WebViewProvider { // without this a rejected key would persist and every VS Code restart // would keep retrying it. const rollbackSnapshot = snapshotSettingsForRollback(); + // restoreSettingsSnapshot → writeSettings can itself throw (EPERM on + // Windows renameSync, disk full, EACCES). Never let a rollback failure + // mask the original auth error or skip the user-facing error message. + const safeRollback = () => { + try { + restoreSettingsSnapshot(rollbackSnapshot); + } catch (rollbackErr) { + console.error( + '[WebViewProvider] settings rollback failed:', + rollbackErr, + ); + } + }; try { // Use core's buildInstallPlan to create a standardized install plan, // then apply it via the VSCode settings adapter. @@ -1366,7 +1379,7 @@ export class WebViewProvider { } else { // Auth failed against the live backend — roll the bad credentials // back off disk so a restart doesn't keep retrying them. - restoreSettingsSnapshot(rollbackSnapshot); + safeRollback(); this.sendMessageToWebView({ type: 'authError', data: { @@ -1381,8 +1394,9 @@ export class WebViewProvider { // A throw can land here after the plan committed but before/while // reconnecting — restore the snapshot so partial/bad state doesn't // linger. (Redundant but harmless if the plan's own rollback already - // ran: it just rewrites the same pre-state.) - restoreSettingsSnapshot(rollbackSnapshot); + // ran: it just rewrites the same pre-state.) safeRollback swallows a + // rollback throw so it can't pre-empt the authError message below. + safeRollback(); this.sendMessageToWebView({ type: 'authError', data: { message: `Configuration failed: ${errorMsg}` },