diff --git a/packages/cli/src/config/loadedSettingsAdapter.ts b/packages/cli/src/config/loadedSettingsAdapter.ts index 06c16e6b3..8157d5ecc 100644 --- a/packages/cli/src/config/loadedSettingsAdapter.ts +++ b/packages/cli/src/config/loadedSettingsAdapter.ts @@ -24,6 +24,14 @@ import { getNestedProperty, } from '../utils/settingsUtils.js'; +/** + * Reserved object keys that would let a caller climb into the prototype chain + * when walking a dotted key path. Defense in depth — current install-plan keys + * are derived from static provider config, but the underlying setNestedProperty + * helper does not guard against them. + */ +const UNSAFE_KEY_PARTS = new Set(['__proto__', 'constructor', 'prototype']); + export function createLoadedSettingsAdapter( settings: LoadedSettings, scope?: SettingScope, @@ -40,6 +48,11 @@ export function createLoadedSettingsAdapter( }, setValue(key: string, value: unknown): void { + if (key.split('.').some((p) => UNSAFE_KEY_PARTS.has(p))) { + throw new Error( + `Refusing to write settings key with reserved segment: ${key}`, + ); + } settings.setValue(persistScope, key, value); }, diff --git a/packages/cli/src/utils/doctorChecks.ts b/packages/cli/src/utils/doctorChecks.ts index bd1d432dc..77690fe32 100644 --- a/packages/cli/src/utils/doctorChecks.ts +++ b/packages/cli/src/utils/doctorChecks.ts @@ -8,7 +8,8 @@ import process from 'node:process'; import os from 'node:os'; import { getNpmVersion, getGitVersion } from './systemInfo.js'; import { validateAuthMethod } from '../config/auth.js'; -import { findProviderByCredentials , +import { + findProviderByCredentials, canUseRipgrep, getMCPServerStatus, MCPServerStatus, diff --git a/packages/core/src/providers/install.ts b/packages/core/src/providers/install.ts index 9330f351f..54abd1a32 100644 --- a/packages/core/src/providers/install.ts +++ b/packages/core/src/providers/install.ts @@ -87,10 +87,13 @@ export async function applyProviderInstallPlan( doRefreshAuth = true, } = options; - settings.backup?.(); const previousEnvValues = new Map(); try { + // backup() inside the try so a failure here (e.g. structuredClone on a + // non-serializable adapter) still triggers the catch + env rollback. + settings.backup?.(); + // Set environment variables (snapshot previous values for rollback) for (const [key, value] of Object.entries(plan.env ?? {})) { previousEnvValues.set(key, process.env[key]); diff --git a/packages/core/src/providers/presets/openrouter.ts b/packages/core/src/providers/presets/openrouter.ts index 4b3617aad..40cff65fd 100644 --- a/packages/core/src/providers/presets/openrouter.ts +++ b/packages/core/src/providers/presets/openrouter.ts @@ -24,7 +24,14 @@ export const openRouterProvider: ProviderConfig = { ], modelsEditable: true, modelNamePrefix: 'OpenRouter', - ownsModel: (model) => (model.baseUrl ?? '').includes('openrouter.ai'), + ownsModel: (model) => { + try { + const host = new URL(model.baseUrl ?? '').hostname; + return host === 'openrouter.ai' || host.endsWith('.openrouter.ai'); + } catch { + return false; + } + }, documentationUrl: 'https://openrouter.ai/docs', uiGroup: 'third-party', }; diff --git a/packages/vscode-ide-companion/src/services/settingsWriter.ts b/packages/vscode-ide-companion/src/services/settingsWriter.ts index 1969a4155..2f66946c4 100644 --- a/packages/vscode-ide-companion/src/services/settingsWriter.ts +++ b/packages/vscode-ide-companion/src/services/settingsWriter.ts @@ -101,21 +101,49 @@ function stripJsonComments(text: string): string { } /** - * Read ~/.qwen/settings.json. Returns {} if missing or invalid. - * Handles JSONC (JSON with comments) which is common in hand-edited - * settings files. + * Strip trailing commas inside arrays/objects (`,]` / `,}`) — VSCode's own + * settings.json allows them and they crash strict JSON.parse. Operates on the + * already-comment-stripped text, so a literal `,]` inside a string is safe + * (string-literal copy in stripJsonComments preserves contents). + */ +function stripTrailingCommas(text: string): string { + return text.replace(/,(\s*[}\]])/g, '$1'); +} + +/** + * Read ~/.qwen/settings.json. Returns {} if the file is missing. + * Parse errors are logged and re-thrown so callers don't silently destroy a + * malformed file by treating it as empty and then overwriting it. + * Handles JSONC (JSON with comments + trailing commas) for hand-edited files. */ function readSettings(): Record { + const settingsPath = Storage.getGlobalSettingsPath(); + let content: string; try { - const content = fs.readFileSync(Storage.getGlobalSettingsPath(), 'utf-8'); - return JSON.parse(stripJsonComments(content)) as Record; - } catch { - return {}; + content = fs.readFileSync(settingsPath, 'utf-8'); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + return {}; + } + throw err; + } + try { + return JSON.parse( + stripTrailingCommas(stripJsonComments(content)), + ) as Record; + } catch (err) { + console.error( + `[settingsWriter] Failed to parse ${settingsPath}; refusing to overwrite a malformed file.`, + err, + ); + throw err; } } /** - * Write ~/.qwen/settings.json (creates dir if needed). + * Write ~/.qwen/settings.json atomically (temp file + rename), creating the + * directory if needed. Atomic rename prevents leaving a half-written file + * behind on EACCES / disk-full / process crash mid-write. */ function writeSettings(settings: Record): void { const settingsPath = Storage.getGlobalSettingsPath(); @@ -123,7 +151,9 @@ function writeSettings(settings: Record): void { if (!fs.existsSync(dir)) { fs.mkdirSync(dir, { recursive: true }); } - fs.writeFileSync(settingsPath, JSON.stringify(settings, null, 2), 'utf-8'); + const tmpPath = `${settingsPath}.${process.pid}.${Date.now()}.tmp`; + fs.writeFileSync(tmpPath, JSON.stringify(settings, null, 2), 'utf-8'); + fs.renameSync(tmpPath, settingsPath); } /** @@ -326,12 +356,18 @@ function createFileSettingsAdapter(): ProviderSettingsAdapter { let current = data; for (let i = 0; i < parts.length - 1; i++) { const part = parts[i]!; - if ( - !Object.prototype.hasOwnProperty.call(current, part) || - !current[part] || - typeof current[part] !== 'object' - ) { + const existing = Object.prototype.hasOwnProperty.call(current, part) + ? current[part] + : undefined; + if (existing == null) { current[part] = {}; + } else if (typeof existing !== 'object') { + // Refuse to silently overwrite a scalar at an intermediate segment — + // would destroy user data (e.g. {"env": "legacy-string"} losing the + // string when env.NEW_KEY is written). + throw new Error( + `Cannot write settings key "${key}": segment "${part}" is a ${typeof existing}, not an object`, + ); } current = current[part] as Record; } diff --git a/packages/vscode-ide-companion/src/webview/handlers/AuthMessageHandler.ts b/packages/vscode-ide-companion/src/webview/handlers/AuthMessageHandler.ts index 9b9236ff0..a02fb006e 100644 --- a/packages/vscode-ide-companion/src/webview/handlers/AuthMessageHandler.ts +++ b/packages/vscode-ide-companion/src/webview/handlers/AuthMessageHandler.ts @@ -14,6 +14,7 @@ import { shouldShowStep, resolveBaseUrl, getDefaultModelIds, + type AuthType, type ProviderConfig, type ProviderSetupInputs, type BaseUrlOption, @@ -101,9 +102,17 @@ export class AuthMessageHandler extends BaseMessageHandler { /** * Helper: show a QuickPick and return the selected item's `value`. * Returns undefined if the user cancels. + * + * Items with `kind: Separator` are rendered by VSCode as non-selectable + * group headers; they should be left in `items` to preserve grouping. */ private async pick( - items: Array<{ label: string; description?: string; value: T }>, + items: Array<{ + label: string; + description?: string; + value: T; + kind?: vscode.QuickPickItemKind; + }>, title: string, placeHolder: string, ): Promise { @@ -111,7 +120,7 @@ export class AuthMessageHandler extends BaseMessageHandler { title, placeHolder, }); - if (!choice) { + if (!choice || choice.kind === vscode.QuickPickItemKind.Separator) { this.notifyAuthCancelled(); return undefined; } @@ -194,14 +203,10 @@ export class AuthMessageHandler extends BaseMessageHandler { addGroup('Custom', customProviders); } + // Pass items including separators; VSCode QuickPick renders separator + // entries as non-selectable group headers (mirrors the CLI grouping). const selectedId = await this.pick( - items.filter( - (i) => i.kind !== vscode.QuickPickItemKind.Separator, - ) as Array<{ - label: string; - description?: string; - value: string; - }>, + items, 'Qwen Code: Select Provider', 'Choose how to connect', ); @@ -233,6 +238,25 @@ export class AuthMessageHandler extends BaseMessageHandler { const flowTitle = provider.uiLabels?.flowTitle ?? `Qwen Code: ${provider.label}`; + // Step 0: Protocol (only for providers offering multiple, e.g. custom) + let protocol: AuthType | undefined; + if ( + shouldShowStep(provider, 'protocol') && + provider.protocolOptions && + provider.protocolOptions.length > 1 + ) { + const selected = await this.pick( + provider.protocolOptions.map((p) => ({ + label: String(p), + value: String(p), + })), + `${flowTitle}: Protocol`, + 'Select API protocol', + ); + if (!selected) return; + protocol = selected as AuthType; + } + // Step 1: Base URL (if needed) let baseUrl: string; if (shouldShowStep(provider, 'baseUrl')) { @@ -260,6 +284,16 @@ export class AuthMessageHandler extends BaseMessageHandler { }); if (urlInput === undefined) return; baseUrl = urlInput; + if (!/^https?:\/\//i.test(baseUrl)) { + this.sendToWebView({ + type: 'authError', + data: { + message: 'Base URL must start with http:// or https://.', + }, + }); + this.notifyAuthCancelled(); + return; + } } } else { baseUrl = resolveBaseUrl(provider); @@ -283,6 +317,7 @@ export class AuthMessageHandler extends BaseMessageHandler { type: 'authError', data: { message: validationError }, }); + this.notifyAuthCancelled(); return; } } @@ -334,13 +369,26 @@ export class AuthMessageHandler extends BaseMessageHandler { } // Submit - if (this.authInteractiveHandler) { - await this.authInteractiveHandler(provider, { - baseUrl, - apiKey, - modelIds, - advancedConfig, + if (!this.authInteractiveHandler) { + console.error( + '[AuthMessageHandler] authInteractiveHandler not set; cannot apply provider config.', + ); + this.sendToWebView({ + type: 'authError', + data: { + message: + 'Auth handler not initialized. Please reopen the panel and try again.', + }, }); + this.notifyAuthCancelled(); + return; } + await this.authInteractiveHandler(provider, { + protocol, + baseUrl, + apiKey, + modelIds, + advancedConfig, + }); } } diff --git a/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.ts b/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.ts index 58d1e992c..835f27967 100644 --- a/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.ts +++ b/packages/vscode-ide-companion/src/webview/providers/WebViewProvider.ts @@ -28,11 +28,15 @@ import { type ApprovalModeValue } from '../../types/approvalModeValueTypes.js'; import { isAuthenticationRequiredError } from '../../utils/authErrors.js'; import { getErrorMessage } from '../../utils/errorMessage.js'; import { + applyProviderInstallPlanToFile, writeCodingPlanConfig, readQwenSettingsForVSCode, clearPersistedAuth, } from '../../services/settingsWriter.js'; -import { parseInsightMessage } from '@qwen-code/qwen-code-core'; +import { + buildInstallPlan, + parseInsightMessage, +} from '@qwen-code/qwen-code-core'; /** Threshold (ms) before a completed task triggers a notification. */ const LONG_TASK_THRESHOLD_MS = 20_000; @@ -1315,10 +1319,6 @@ export class WebViewProvider { try { // Use core's buildInstallPlan to create a standardized install plan, // then apply it via the VSCode settings adapter. - const { buildInstallPlan } = await import('@qwen-code/qwen-code-core'); - const { applyProviderInstallPlanToFile } = await import( - '../../services/settingsWriter.js' - ); const plan = buildInstallPlan(providerConfig, inputs); await applyProviderInstallPlanToFile(plan);