From d321c8fc6a0be6731c1ccfec0fca8023b1a8b67e Mon Sep 17 00:00:00 2001 From: Vasanth T <148849890+Vasanthdev2004@users.noreply.github.com> Date: Tue, 28 Apr 2026 21:00:48 +0530 Subject: [PATCH] fix: avoid legacy Windows PasswordVault reads by default (#941) * fix: avoid legacy Windows PasswordVault reads by default * fix: isolate model capability override cache --------- Co-authored-by: OpenClaude Worker 3 --- src/utils/model/modelSupportOverrides.ts | 19 +++++++++- .../secureStorage/platformStorage.test.ts | 24 +++++++++++- .../secureStorage/windowsCredentialStorage.ts | 38 ++++++++++++------- src/utils/thinking.test.ts | 37 ++++++++++++++++-- 4 files changed, 98 insertions(+), 20 deletions(-) diff --git a/src/utils/model/modelSupportOverrides.ts b/src/utils/model/modelSupportOverrides.ts index d3003272..562fac78 100644 --- a/src/utils/model/modelSupportOverrides.ts +++ b/src/utils/model/modelSupportOverrides.ts @@ -23,6 +23,23 @@ const TIERS = [ }, ] as const +function buildCapabilityOverrideCacheKey( + model: string, + capability: ModelCapabilityOverride, +): string { + const envParts = TIERS.flatMap(tier => [ + process.env[tier.modelEnvVar] ?? '', + process.env[tier.capabilitiesEnvVar] ?? '', + ]) + + return [ + model.toLowerCase(), + capability, + getAPIProvider(), + ...envParts, + ].join('\0') +} + /** * Check whether a 3p model capability override is set for a model that matches one of * the pinned ANTHROPIC_DEFAULT_*_MODEL env vars. @@ -46,5 +63,5 @@ export const get3PModelCapabilityOverride = memoize( } return undefined }, - (model, capability) => `${model.toLowerCase()}:${capability}`, + buildCapabilityOverrideCacheKey, ) diff --git a/src/utils/secureStorage/platformStorage.test.ts b/src/utils/secureStorage/platformStorage.test.ts index 76b14522..e0dd8166 100644 --- a/src/utils/secureStorage/platformStorage.test.ts +++ b/src/utils/secureStorage/platformStorage.test.ts @@ -97,13 +97,22 @@ describe("Secure Storage Platform Implementations", () => { expect(options2.input).toContain("token'quote"); }); - test("delete() includes assembly load", () => { + test("delete() skips legacy PasswordVault by default", () => { + windowsCredentialStorage.delete(); + expect(mockExecaSync).toHaveBeenCalledTimes(1); + const script = mockExecaSync.mock.calls[0][1][1]; + expect(script).not.toContain("System.Runtime.WindowsRuntime"); + }); + + test("delete() includes legacy assembly load when explicitly enabled", () => { + process.env.OPENCLAUDE_ENABLE_LEGACY_WINDOWS_PASSWORDVAULT = "1"; windowsCredentialStorage.delete(); const script = mockExecaSync.mock.calls[1][1][1]; expect(script).toContain("Add-Type -AssemblyName System.Runtime.WindowsRuntime"); }); test("escapes double quotes in username", () => { + process.env.OPENCLAUDE_ENABLE_LEGACY_WINDOWS_PASSWORDVAULT = "1"; process.env.USER = 'user"name'; windowsCredentialStorage.read(); const script = mockExecaSync.mock.calls[1][1][1]; @@ -111,7 +120,17 @@ describe("Secure Storage Platform Implementations", () => { expect(script).not.toContain('user"name'); }); - test("read() falls back to legacy PasswordVault when the DPAPI payload is invalid JSON", () => { + test("read() does not touch legacy PasswordVault by default", () => { + mockExecaSync.mockImplementationOnce(() => ({ exitCode: 1, stdout: "" })); + + const result = windowsCredentialStorage.read(); + + expect(result).toBeNull(); + expect(mockExecaSync).toHaveBeenCalledTimes(1); + }); + + test("read() falls back to legacy PasswordVault when explicitly enabled", () => { + process.env.OPENCLAUDE_ENABLE_LEGACY_WINDOWS_PASSWORDVAULT = "1"; mockExecaSync .mockImplementationOnce(() => ({ exitCode: 0, stdout: "{not-json" })) .mockImplementationOnce(() => ({ @@ -126,6 +145,7 @@ describe("Secure Storage Platform Implementations", () => { }); test("read() fails closed when the legacy PasswordVault payload is invalid JSON", () => { + process.env.OPENCLAUDE_ENABLE_LEGACY_WINDOWS_PASSWORDVAULT = "1"; mockExecaSync .mockImplementationOnce(() => ({ exitCode: 1, stdout: "" })) .mockImplementationOnce(() => ({ exitCode: 0, stdout: "{not-json" })); diff --git a/src/utils/secureStorage/windowsCredentialStorage.ts b/src/utils/secureStorage/windowsCredentialStorage.ts index 938a79ad..fdccc56e 100644 --- a/src/utils/secureStorage/windowsCredentialStorage.ts +++ b/src/utils/secureStorage/windowsCredentialStorage.ts @@ -30,6 +30,10 @@ function getWindowsSecureStorageFilePath(): string { return join(getClaudeConfigHomeDir(), `${resourceName}.secure.dpapi`) } +function shouldUseLegacyPasswordVault(): boolean { + return process.env.OPENCLAUDE_ENABLE_LEGACY_WINDOWS_PASSWORDVAULT === '1' +} + function runPowerShell( script: string, options?: { input?: string }, @@ -61,6 +65,10 @@ function getFailureWarning( } function readLegacyPasswordVault(): SecureStorageData | null { + if (!shouldUseLegacyPasswordVault()) { + return null + } + const resourceName = getLegacyResourceName().replace(/"/g, '`"') const username = getUsername().replace(/"/g, '`"') const script = ` @@ -204,21 +212,23 @@ export const windowsCredentialStorage: SecureStorage = { ` const removeDpapiResult = runPowerShell(removeDpapiScript) - const resourceName = getLegacyResourceName().replace(/"/g, '`"') - const username = getUsername().replace(/"/g, '`"') - const removeLegacyScript = ` - Add-Type -AssemblyName System.Runtime.WindowsRuntime - try { - $vault = New-Object Windows.Security.Credentials.PasswordVault - $cred = $vault.Retrieve("${resourceName}", "${username}") - $vault.Remove($cred) - } catch { - exit 0 - } - ` - const removeLegacyResult = runPowerShell(removeLegacyScript) + if (shouldUseLegacyPasswordVault()) { + const resourceName = getLegacyResourceName().replace(/"/g, '`"') + const username = getUsername().replace(/"/g, '`"') + const removeLegacyScript = ` + Add-Type -AssemblyName System.Runtime.WindowsRuntime + try { + $vault = New-Object Windows.Security.Credentials.PasswordVault + $cred = $vault.Retrieve("${resourceName}", "${username}") + $vault.Remove($cred) + } catch { + exit 0 + } + ` + const removeLegacyResult = runPowerShell(removeLegacyScript) - void removeLegacyResult + void removeLegacyResult + } return (removeDpapiResult?.exitCode ?? 1) === 0 }, diff --git a/src/utils/thinking.test.ts b/src/utils/thinking.test.ts index 8dd5348f..c992e538 100644 --- a/src/utils/thinking.test.ts +++ b/src/utils/thinking.test.ts @@ -1,5 +1,12 @@ -import { afterEach, beforeEach, describe, expect, test } from 'bun:test' -import { modelSupportsThinking } from './thinking.js' +import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test' +import { resetSettingsCache } from './settings/settingsCache.js' + +mock.module('./model/providers.js', () => ({ + getAPIProvider: () => + process.env.CLAUDE_CODE_USE_OPENAI === '1' ? 'openai' : 'firstParty', +})) + +const { modelSupportsThinking } = await import('./thinking.js') const ENV_KEYS = [ 'CLAUDE_CODE_USE_OPENAI', @@ -14,6 +21,13 @@ const ENV_KEYS = [ 'OPENAI_MODEL', 'NVIDIA_NIM', 'MINIMAX_API_KEY', + 'XAI_API_KEY', + 'ANTHROPIC_DEFAULT_OPUS_MODEL', + 'ANTHROPIC_DEFAULT_OPUS_MODEL_SUPPORTED_CAPABILITIES', + 'ANTHROPIC_DEFAULT_SONNET_MODEL', + 'ANTHROPIC_DEFAULT_SONNET_MODEL_SUPPORTED_CAPABILITIES', + 'ANTHROPIC_DEFAULT_HAIKU_MODEL', + 'ANTHROPIC_DEFAULT_HAIKU_MODEL_SUPPORTED_CAPABILITIES', 'USER_TYPE', ] @@ -24,6 +38,7 @@ beforeEach(() => { originalEnv[key] = process.env[key] delete process.env[key] } + resetSettingsCache() }) afterEach(() => { @@ -34,6 +49,7 @@ afterEach(() => { process.env[key] = originalEnv[key] } } + resetSettingsCache() }) describe('modelSupportsThinking — Z.AI GLM', () => { @@ -61,4 +77,19 @@ describe('modelSupportsThinking — Z.AI GLM', () => { expect(modelSupportsThinking('glm-50')).toBe(false) }) -}) \ No newline at end of file + + test('does not reuse stale capability overrides after env changes', () => { + process.env.CLAUDE_CODE_USE_OPENAI = '1' + process.env.OPENAI_BASE_URL = 'https://dashscope.aliyuncs.com/compatible-mode/v1' + process.env.ANTHROPIC_DEFAULT_SONNET_MODEL = 'GLM-5.1' + process.env.ANTHROPIC_DEFAULT_SONNET_MODEL_SUPPORTED_CAPABILITIES = '' + + expect(modelSupportsThinking('GLM-5.1')).toBe(false) + + delete process.env.ANTHROPIC_DEFAULT_SONNET_MODEL + delete process.env.ANTHROPIC_DEFAULT_SONNET_MODEL_SUPPORTED_CAPABILITIES + process.env.OPENAI_BASE_URL = 'https://api.z.ai/api/coding/paas/v4' + + expect(modelSupportsThinking('GLM-5.1')).toBe(true) + }) +})