fix(auth): third round of PR #4287 review fixes (8 comments)

Critical:
- useAuth: handleProviderSubmit now calls setPendingAuthType at the start
  of the try, so handleAuthFailure can record the AuthEvent telemetry on
  applyProviderInstallPlan rejection (previously dropped silently because
  pendingAuthType was undefined).
- settingsWriter: readQwenSettingsForVSCode wraps readSettings in
  try/catch so a malformed settings.json no longer crashes the VSCode
  extension on activation; the write paths (writeCodingPlanConfig,
  writeModelProvidersConfig) deliberately keep propagating to avoid
  silently overwriting a corrupt file with partial data.

Suggestions:
- settingsWriter.setValue: intermediate-segment guard now also rejects
  arrays (typeof [] === 'object' previously slipped through and would
  let us set string keys on an array). Loop restructured so the
  literal-=== prototype-pollution guard runs at every step, satisfying
  CodeQL's sanitiser detector on both the leaf and intermediate writes.
- settingsWriter atomic write: SETTINGS_FILE_MODE = 0o600 +
  SETTINGS_DIR_MODE = 0o700 + best-effort chmod on existing files. API
  keys persisted into env.* are no longer world-readable on multi-user
  systems.
- loadedSettingsAdapter: switched its prototype-pollution guard to the
  same inline literal === pattern so the two adapters stay symmetric
  and CodeQL recognises both as sanitisers (Comment 6 — explicit
  'keep in sync' comment + same shape rather than a shared helper that
  CodeQL wouldn't trace through).
- AuthMessageHandler: protocol QuickPick now shows 'OpenAI Compatible'
  / 'Anthropic' / 'Gemini' instead of the raw AuthType enum values.
- WebViewProvider: authInteractive log now records only the parsed
  hostname, not the full inputs.baseUrl, so credentials embedded in
  userinfo or query strings don't leak into extension-host logs.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

packages/cli/src/config/loadedSettingsAdapter.ts

@@ -24,14 +24,6 @@ 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,
@@ -48,10 +40,22 @@ 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}`,
-        );
+      // Defense in depth: refuse prototype-chain segments before delegating to
+      // LoadedSettings.setValue, which goes through setNestedPropertySafe and
+      // doesn't enforce this itself. Inline literal === comparisons (rather
+      // than Set.has) are what CodeQL's prototype-pollution sanitiser
+      // recognises — keep this list in sync with the matching guard in
+      // `packages/vscode-ide-companion/src/services/settingsWriter.ts`.
+      for (const part of key.split('.')) {
+        if (
+          part === '__proto__' ||
+          part === 'constructor' ||
+          part === 'prototype'
+        ) {
+          throw new Error(
+            `Refusing to write settings key with reserved segment: ${key}`,
+          );
+        }
       }
       settings.setValue(persistScope, key, value);
     },

packages/cli/src/ui/auth/useAuth.ts

@@ -147,7 +147,14 @@ export const useAuthCommand = (
 
   const handleProviderSubmit = useCallback(
     async (providerConfig: ProviderConfig, inputs: ProviderSetupInputs) => {
+      // Resolve the protocol once and store it as pendingAuthType so that if
+      // applyProviderInstallPlan rejects, handleAuthFailure (which gates the
+      // AuthEvent telemetry on pendingAuthType being defined) can record the
+      // failure under the right AuthType bucket instead of silently dropping
+      // it.
+      const protocol = inputs.protocol ?? providerConfig.protocol;
       try {
+        setPendingAuthType(protocol);
         setIsAuthenticating(true);
         setAuthError(null);
 
@@ -173,7 +180,6 @@ export const useAuthCommand = (
           Date.now(),
         );
 
-        const protocol = inputs.protocol ?? providerConfig.protocol;
         logAuth(config, new AuthEvent(protocol, 'manual', 'success'));
       } catch (error) {
         handleAuthFailure(error);

packages/vscode-ide-companion/src/services/settingsWriter.ts

@@ -31,11 +31,14 @@ import {
 // ---------------------------------------------------------------------------
 
 /**
- * Object keys that would let a caller climb into the prototype chain when
- * walking a dotted key path. Any attempt to write into one of these is
- * rejected so install-plan inputs cannot pollute Object.prototype.
+ * Mode for ~/.qwen/settings.json — owner-only read/write. The unified install
+ * plan now persists API keys into `env.*` inside this file, so on multi-user
+ * systems the default 0644 (process umask) would expose those secrets to any
+ * local user. Keep this in sync with `loadedSettingsAdapter`'s expectations.
  */
-const UNSAFE_KEY_PARTS = new Set(['__proto__', 'constructor', 'prototype']);
+const SETTINGS_FILE_MODE = 0o600;
+/** Directory mode for ~/.qwen — owner-only. */
+const SETTINGS_DIR_MODE = 0o700;
 
 // ---------------------------------------------------------------------------
 // Types
@@ -184,11 +187,25 @@ function writeSettings(settings: Record<string, unknown>): void {
   const settingsPath = Storage.getGlobalSettingsPath();
   const dir = path.dirname(settingsPath);
   if (!fs.existsSync(dir)) {
-    fs.mkdirSync(dir, { recursive: true });
+    fs.mkdirSync(dir, { recursive: true, mode: SETTINGS_DIR_MODE });
   }
   const tmpPath = `${settingsPath}.${process.pid}.${Date.now()}.tmp`;
-  fs.writeFileSync(tmpPath, JSON.stringify(settings, null, 2), 'utf-8');
+  // Create the temp file with owner-only permissions so secrets in `env.*`
+  // are never observable to other local users even briefly.
+  fs.writeFileSync(tmpPath, JSON.stringify(settings, null, 2), {
+    encoding: 'utf-8',
+    mode: SETTINGS_FILE_MODE,
+  });
   fs.renameSync(tmpPath, settingsPath);
+  // Tighten any pre-existing settings file inherited from older writes that
+  // used the default umask. chmodSync is a no-op on the just-written file but
+  // covers the case where the file was created earlier with looser bits.
+  try {
+    fs.chmodSync(settingsPath, SETTINGS_FILE_MODE);
+  } catch {
+    // Best effort — surface nothing to the user if the FS rejects chmod
+    // (e.g. Windows, mounted FS without POSIX permissions).
+  }
 }
 
 /**
@@ -379,34 +396,47 @@ function createFileSettingsAdapter(): ProviderSettingsAdapter {
 
     setValue(key: string, value: unknown): void {
       const parts = key.split('.');
-      // Refuse to walk through prototype-pollution keys. ProviderInstallPlan
-      // contents come from in-process callers, but install plans accept
-      // arbitrary string keys (env vars, providerState namespaces) so the
-      // input is untrusted enough to warrant the guard.
-      if (parts.some((p) => UNSAFE_KEY_PARTS.has(p))) {
-        throw new Error(
-          `Refusing to write settings key with reserved segment: ${key}`,
-        );
-      }
       let current = data;
-      for (let i = 0; i < parts.length - 1; i++) {
+      for (let i = 0; i < parts.length; i++) {
         const part = parts[i]!;
+        // Refuse to walk through prototype-pollution keys. ProviderInstallPlan
+        // contents come from in-process callers, but install plans accept
+        // arbitrary string keys (env vars, providerState namespaces) so the
+        // input is untrusted enough to warrant the guard. Literal === checks
+        // (not Set.has) are what CodeQL's prototype-pollution sanitiser
+        // recognises — keep them at the only step that actually writes to
+        // `current`.
+        if (
+          part === '__proto__' ||
+          part === 'constructor' ||
+          part === 'prototype'
+        ) {
+          throw new Error(
+            `Refusing to write settings key with reserved segment: ${key}`,
+          );
+        }
+        if (i === parts.length - 1) {
+          current[part] = value;
+          break;
+        }
         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).
+        } else if (Array.isArray(existing) || typeof existing !== 'object') {
+          // Refuse to silently overwrite a scalar (or treat an array as an
+          // object) at an intermediate segment — would either destroy user
+          // data (e.g. {"env": "legacy-string"} losing the string when
+          // env.NEW_KEY is written) or set string keys on an array.
           throw new Error(
-            `Cannot write settings key "${key}": segment "${part}" is a ${typeof existing}, not an object`,
+            `Cannot write settings key "${key}": segment "${part}" is ${
+              Array.isArray(existing) ? 'an array' : `a ${typeof existing}`
+            }, not an object`,
           );
         }
         current = current[part] as Record<string, unknown>;
       }
-      current[parts[parts.length - 1]!] = value;
     },
 
     getModelProviders(): ModelProvidersConfig {
@@ -457,10 +487,23 @@ export async function applyProviderInstallPlanToFile(
 
 /**
  * Read ~/.qwen/settings.json and extract values for VSCode Settings UI.
- * Returns null if no valid configuration found.
+ * Returns null if no valid configuration found, or if the file is
+ * malformed — the panel falls back to the empty/default state instead of
+ * crashing the extension on activation. `readSettings` itself now throws
+ * on parse failure (so we never silently overwrite a corrupt file in the
+ * write paths), so this caller has to catch.
  */
 export function readQwenSettingsForVSCode(): QwenSettingsForVSCode | null {
-  const settings = readSettings();
+  let settings: Record<string, unknown>;
+  try {
+    settings = readSettings();
+  } catch (error) {
+    console.error(
+      '[settingsWriter] readQwenSettingsForVSCode failed; returning null:',
+      error,
+    );
+    return null;
+  }
 
   const security = settings.security as Record<string, unknown> | undefined;
   const auth = security?.auth as Record<string, unknown> | undefined;

packages/vscode-ide-companion/src/webview/handlers/AuthMessageHandler.ts

@@ -245,9 +245,16 @@ export class AuthMessageHandler extends BaseMessageHandler {
       provider.protocolOptions &&
       provider.protocolOptions.length > 1
     ) {
+      // AuthType's raw string values ('openai' / 'anthropic' / 'gemini') are
+      // implementation detail; QuickPick should show human-readable labels.
+      const protocolLabels: Record<string, string> = {
+        [AuthType.USE_OPENAI]: 'OpenAI Compatible',
+        [AuthType.USE_ANTHROPIC]: 'Anthropic',
+        [AuthType.USE_GEMINI]: 'Gemini',
+      };
       const selected = await this.pick(
         provider.protocolOptions.map((p) => ({
-          label: String(p),
+          label: protocolLabels[String(p)] ?? String(p),
           value: String(p),
         })),
         `${flowTitle}: Protocol`,

packages/vscode-ide-companion/src/webview/providers/WebViewProvider.ts

@@ -1312,8 +1312,18 @@ export class WebViewProvider {
       return;
     }
 
+    // Log only the host so we don't leak credentials embedded in user-info
+    // (`https://user:sk-secret@host/v1`) or query strings into extension-host
+    // logs / diagnostic bundles.
+    const baseUrlHost = (() => {
+      try {
+        return new URL(inputs.baseUrl).hostname;
+      } catch {
+        return '[invalid]';
+      }
+    })();
     console.log(
-      `[WebViewProvider] authInteractive: provider=${providerConfig.id}, baseUrl=${inputs.baseUrl}`,
+      `[WebViewProvider] authInteractive: provider=${providerConfig.id}, host=${baseUrlHost}`,
     );
 
     try {