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 <worker-3@openclaude.local>
This commit is contained in:
Vasanth T
2026-04-28 21:00:48 +05:30
committed by GitHub
parent 8106880855
commit d321c8fc6a
4 changed files with 98 additions and 20 deletions

View File

@@ -23,6 +23,23 @@ const TIERS = [
}, },
] as const ] 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 * Check whether a 3p model capability override is set for a model that matches one of
* the pinned ANTHROPIC_DEFAULT_*_MODEL env vars. * the pinned ANTHROPIC_DEFAULT_*_MODEL env vars.
@@ -46,5 +63,5 @@ export const get3PModelCapabilityOverride = memoize(
} }
return undefined return undefined
}, },
(model, capability) => `${model.toLowerCase()}:${capability}`, buildCapabilityOverrideCacheKey,
) )

View File

@@ -97,13 +97,22 @@ describe("Secure Storage Platform Implementations", () => {
expect(options2.input).toContain("token'quote"); 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(); windowsCredentialStorage.delete();
const script = mockExecaSync.mock.calls[1][1][1]; const script = mockExecaSync.mock.calls[1][1][1];
expect(script).toContain("Add-Type -AssemblyName System.Runtime.WindowsRuntime"); expect(script).toContain("Add-Type -AssemblyName System.Runtime.WindowsRuntime");
}); });
test("escapes double quotes in username", () => { test("escapes double quotes in username", () => {
process.env.OPENCLAUDE_ENABLE_LEGACY_WINDOWS_PASSWORDVAULT = "1";
process.env.USER = 'user"name'; process.env.USER = 'user"name';
windowsCredentialStorage.read(); windowsCredentialStorage.read();
const script = mockExecaSync.mock.calls[1][1][1]; const script = mockExecaSync.mock.calls[1][1][1];
@@ -111,7 +120,17 @@ describe("Secure Storage Platform Implementations", () => {
expect(script).not.toContain('user"name'); 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 mockExecaSync
.mockImplementationOnce(() => ({ exitCode: 0, stdout: "{not-json" })) .mockImplementationOnce(() => ({ exitCode: 0, stdout: "{not-json" }))
.mockImplementationOnce(() => ({ .mockImplementationOnce(() => ({
@@ -126,6 +145,7 @@ describe("Secure Storage Platform Implementations", () => {
}); });
test("read() fails closed when the legacy PasswordVault payload is invalid JSON", () => { test("read() fails closed when the legacy PasswordVault payload is invalid JSON", () => {
process.env.OPENCLAUDE_ENABLE_LEGACY_WINDOWS_PASSWORDVAULT = "1";
mockExecaSync mockExecaSync
.mockImplementationOnce(() => ({ exitCode: 1, stdout: "" })) .mockImplementationOnce(() => ({ exitCode: 1, stdout: "" }))
.mockImplementationOnce(() => ({ exitCode: 0, stdout: "{not-json" })); .mockImplementationOnce(() => ({ exitCode: 0, stdout: "{not-json" }));

View File

@@ -30,6 +30,10 @@ function getWindowsSecureStorageFilePath(): string {
return join(getClaudeConfigHomeDir(), `${resourceName}.secure.dpapi`) return join(getClaudeConfigHomeDir(), `${resourceName}.secure.dpapi`)
} }
function shouldUseLegacyPasswordVault(): boolean {
return process.env.OPENCLAUDE_ENABLE_LEGACY_WINDOWS_PASSWORDVAULT === '1'
}
function runPowerShell( function runPowerShell(
script: string, script: string,
options?: { input?: string }, options?: { input?: string },
@@ -61,6 +65,10 @@ function getFailureWarning(
} }
function readLegacyPasswordVault(): SecureStorageData | null { function readLegacyPasswordVault(): SecureStorageData | null {
if (!shouldUseLegacyPasswordVault()) {
return null
}
const resourceName = getLegacyResourceName().replace(/"/g, '`"') const resourceName = getLegacyResourceName().replace(/"/g, '`"')
const username = getUsername().replace(/"/g, '`"') const username = getUsername().replace(/"/g, '`"')
const script = ` const script = `
@@ -204,21 +212,23 @@ export const windowsCredentialStorage: SecureStorage = {
` `
const removeDpapiResult = runPowerShell(removeDpapiScript) const removeDpapiResult = runPowerShell(removeDpapiScript)
const resourceName = getLegacyResourceName().replace(/"/g, '`"') if (shouldUseLegacyPasswordVault()) {
const username = getUsername().replace(/"/g, '`"') const resourceName = getLegacyResourceName().replace(/"/g, '`"')
const removeLegacyScript = ` const username = getUsername().replace(/"/g, '`"')
Add-Type -AssemblyName System.Runtime.WindowsRuntime const removeLegacyScript = `
try { Add-Type -AssemblyName System.Runtime.WindowsRuntime
$vault = New-Object Windows.Security.Credentials.PasswordVault try {
$cred = $vault.Retrieve("${resourceName}", "${username}") $vault = New-Object Windows.Security.Credentials.PasswordVault
$vault.Remove($cred) $cred = $vault.Retrieve("${resourceName}", "${username}")
} catch { $vault.Remove($cred)
exit 0 } catch {
} exit 0
` }
const removeLegacyResult = runPowerShell(removeLegacyScript) `
const removeLegacyResult = runPowerShell(removeLegacyScript)
void removeLegacyResult void removeLegacyResult
}
return (removeDpapiResult?.exitCode ?? 1) === 0 return (removeDpapiResult?.exitCode ?? 1) === 0
}, },

View File

@@ -1,5 +1,12 @@
import { afterEach, beforeEach, describe, expect, test } from 'bun:test' import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test'
import { modelSupportsThinking } from './thinking.js' 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 = [ const ENV_KEYS = [
'CLAUDE_CODE_USE_OPENAI', 'CLAUDE_CODE_USE_OPENAI',
@@ -14,6 +21,13 @@ const ENV_KEYS = [
'OPENAI_MODEL', 'OPENAI_MODEL',
'NVIDIA_NIM', 'NVIDIA_NIM',
'MINIMAX_API_KEY', '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', 'USER_TYPE',
] ]
@@ -24,6 +38,7 @@ beforeEach(() => {
originalEnv[key] = process.env[key] originalEnv[key] = process.env[key]
delete process.env[key] delete process.env[key]
} }
resetSettingsCache()
}) })
afterEach(() => { afterEach(() => {
@@ -34,6 +49,7 @@ afterEach(() => {
process.env[key] = originalEnv[key] process.env[key] = originalEnv[key]
} }
} }
resetSettingsCache()
}) })
describe('modelSupportsThinking — Z.AI GLM', () => { describe('modelSupportsThinking — Z.AI GLM', () => {
@@ -61,4 +77,19 @@ describe('modelSupportsThinking — Z.AI GLM', () => {
expect(modelSupportsThinking('glm-50')).toBe(false) expect(modelSupportsThinking('glm-50')).toBe(false)
}) })
})
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)
})
})