fix: OAuth tokens secure storage for Windows & Linux (#215)

* fix: OAuth tokens secure storage for Windows & Linux

* fix: OAuth tokens secure storage for Windows & Linux #215

* fix: OAuth tokens secure storage for Windows & Linux #215

* fix: OAuth tokens secure storage for Windows & Linux #215

src/cli/handlers/mcp.tsx

@@ -175,7 +175,7 @@ export async function mcpRemoveHandler(name: string, options: {
   const serverBeforeRemoval = getMcpConfigByName(name);
   const cleanupSecureStorage = () => {
     if (serverBeforeRemoval && (serverBeforeRemoval.type === 'sse' || serverBeforeRemoval.type === 'http')) {
-      clearServerTokensFromLocalStorage(name, serverBeforeRemoval);
+      clearServerTokensFromSecureStorage(name, serverBeforeRemoval);
       clearMcpClientConfig(name, serverBeforeRemoval);
     }
   };

src/services/mcp/auth.ts

@@ -40,7 +40,7 @@ import { logMCPDebug } from '../../utils/log.js'
 import { getPlatform } from '../../utils/platform.js'
 import { getSecureStorage } from '../../utils/secureStorage/index.js'
 import { clearKeychainCache } from '../../utils/secureStorage/macOsKeychainHelpers.js'
-import type { SecureStorageData } from '../../utils/secureStorage/types.js'
+import type { SecureStorageData } from '../../utils/secureStorage/index.js'
 import { sleep } from '../../utils/sleep.js'
 import { jsonParse, jsonStringify } from '../../utils/slowOperations.js'
 import { logEvent } from '../analytics/index.js'
@@ -573,7 +573,7 @@ export async function revokeServerTokens(
   }
 
   // Always clear local tokens, regardless of server-side revocation result.
-  clearServerTokensFromLocalStorage(serverName, serverConfig)
+  clearServerTokensFromSecureStorage(serverName, serverConfig)
 
   // When re-authenticating, preserve step-up auth state (scope + discovery)
   // so the next performMCPOAuthFlow can use cached scope instead of
@@ -617,7 +617,8 @@ export async function revokeServerTokens(
   }
 }
 
-export function clearServerTokensFromLocalStorage(
+// Utilizing platform-specific secure storage to protect sensitive tokens
+export function clearServerTokensFromSecureStorage(
   serverName: string,
   serverConfig: McpSSEServerConfig | McpHTTPServerConfig,
 ): void {
@@ -629,7 +630,7 @@ export function clearServerTokensFromLocalStorage(
   if (existingData.mcpOAuth[serverKey]) {
     delete existingData.mcpOAuth[serverKey]
     storage.update(existingData)
-    logMCPDebug(serverName, 'Cleared stored tokens')
+    logMCPDebug(serverName, 'Cleared stored tokens from secure storage')
   }
 }
 
@@ -913,7 +914,7 @@ export async function performMCPOAuthFlow(
   // Clear any existing stored credentials to ensure fresh client registration.
   // Note: this deletes the entire entry (including discoveryState/stepUpScope),
   // but we already read the cached values above.
-  clearServerTokensFromLocalStorage(serverName, serverConfig)
+  clearServerTokensFromSecureStorage(serverName, serverConfig)
 
   // Use cached step-up scope and resource metadata URL if available.
   // The transport-attached auth provider caches these when it receives a

src/utils/secureStorage/fallbackStorage.ts

@@ -1,4 +1,4 @@
-import type { SecureStorage, SecureStorageData } from './types.js'
+import type { SecureStorage, SecureStorageData } from './index.js'
 
 /**
  * Creates a fallback storage that tries to use the primary storage first,

src/utils/secureStorage/index.ts

@@ -1,17 +1,57 @@
 import { createFallbackStorage } from './fallbackStorage.js'
 import { macOsKeychainStorage } from './macOsKeychainStorage.js'
+import { linuxSecretStorage } from './linuxSecretStorage.js'
+import { windowsCredentialStorage } from './windowsCredentialStorage.js'
 import { plainTextStorage } from './plainTextStorage.js'
-import type { SecureStorage } from './types.js'
+
+export interface SecureStorageData {
+  mcpOAuth?: Record<
+    string,
+    {
+      serverName: string
+      serverUrl: string
+      accessToken: string
+      refreshToken?: string
+      expiresAt: number
+      scope?: string
+      clientId?: string
+      clientSecret?: string
+      discoveryState?: {
+        authorizationServerUrl: string
+        resourceMetadataUrl?: string
+      }
+      stepUpScope?: string
+    }
+  >
+  mcpOAuthClientConfig?: Record<string, { clientSecret: string }>
+  trustedDeviceToken?: string
+  pluginSecrets?: Record<string, Record<string, string>>
+}
+
+export interface SecureStorage {
+  name: string
+  read(): SecureStorageData | null
+  readAsync(): Promise<SecureStorageData | null>
+  update(data: SecureStorageData): { success: boolean; warning?: string }
+  delete(): boolean
+}
 
 /**
- * Get the appropriate secure storage implementation for the current platform
+ * Get the appropriate secure storage implementation for the current platform.
+ * Prefers native OS vaults (Keychain, libsecret, Credential Locker) with a plaintext fallback.
  */
 export function getSecureStorage(): SecureStorage {
   if (process.platform === 'darwin') {
     return createFallbackStorage(macOsKeychainStorage, plainTextStorage)
   }
 
-  // TODO: add libsecret support for Linux
+  if (process.platform === 'linux') {
+    return createFallbackStorage(linuxSecretStorage, plainTextStorage)
+  }
+
+  if (process.platform === 'win32') {
+    return createFallbackStorage(windowsCredentialStorage, plainTextStorage)
+  }
 
   return plainTextStorage
 }

src/utils/secureStorage/linuxSecretStorage.ts

@@ -0,0 +1,86 @@
+import { execaSync } from 'execa'
+import { jsonParse, jsonStringify } from '../slowOperations.js'
+import {
+  CREDENTIALS_SERVICE_SUFFIX,
+  getSecureStorageServiceName,
+  getUsername,
+} from './macOsKeychainHelpers.js'
+import type { SecureStorage, SecureStorageData } from './index.js'
+
+/**
+ * Linux-specific secure storage implementation using the secret-tool CLI.
+ * secret-tool interacts with the Secret Service API (GNOME Keyring, KWallet, etc.).
+ */
+export const linuxSecretStorage: SecureStorage = {
+  name: 'libsecret',
+  read(): SecureStorageData | null {
+    try {
+      const username = getUsername()
+      const serviceName = getSecureStorageServiceName(
+        CREDENTIALS_SERVICE_SUFFIX,
+      )
+      // secret-tool lookup service [service] account [account]
+      const result = execaSync(
+        'secret-tool',
+        ['lookup', 'service', serviceName, 'account', username],
+        { reject: false },
+      )
+
+      if (result.exitCode === 0 && result.stdout) {
+        return jsonParse(result.stdout)
+      }
+    } catch {
+      // fall through
+    }
+    return null
+  },
+  async readAsync(): Promise<SecureStorageData | null> {
+    // Reusing sync implementation for simplicity as it wraps a CLI call
+    return this.read()
+  },
+  update(data: SecureStorageData): { success: boolean; warning?: string } {
+    try {
+      const username = getUsername()
+      const serviceName = getSecureStorageServiceName(
+        CREDENTIALS_SERVICE_SUFFIX,
+      )
+      const payload = jsonStringify(data)
+      // secret-tool store --label=[label] service [service] account [account]
+      // The payload is passed via stdin
+      const result = execaSync(
+        'secret-tool',
+        [
+          'store',
+          '--label',
+          serviceName,
+          'service',
+          serviceName,
+          'account',
+          username,
+        ],
+        { input: payload, reject: false },
+      )
+
+      return { success: result.exitCode === 0 }
+    } catch {
+      return { success: false }
+    }
+  },
+  delete(): boolean {
+    try {
+      const username = getUsername()
+      const serviceName = getSecureStorageServiceName(
+        CREDENTIALS_SERVICE_SUFFIX,
+      )
+      // secret-tool clear service [service] account [account]
+      const result = execaSync(
+        'secret-tool',
+        ['clear', 'service', serviceName, 'account', username],
+        { reject: false },
+      )
+      return result.exitCode === 0
+    } catch {
+      return false
+    }
+  },
+}

src/utils/secureStorage/macOsKeychainHelpers.ts

@@ -18,7 +18,7 @@ import { createHash } from 'crypto'
 import { userInfo } from 'os'
 import { getOauthConfig } from 'src/constants/oauth.js'
 import { getClaudeConfigHomeDir } from '../envUtils.js'
-import type { SecureStorageData } from './types.js'
+import type { SecureStorageData } from './index.js'
 
 // Suffix distinguishing the OAuth credentials keychain entry from the legacy
 // API key entry (which uses no suffix). Both share the service name base.
@@ -26,7 +26,11 @@ import type { SecureStorageData } from './types.js'
 // orphan existing stored credentials.
 export const CREDENTIALS_SERVICE_SUFFIX = '-credentials'
 
-export function getMacOsKeychainStorageServiceName(
+/**
+ * Get the service/resource name for secure storage, scoped by CLAUDE_CONFIG_DIR
+ * if it's set to a non-default location.
+ */
+export function getSecureStorageServiceName(
   serviceSuffix: string = '',
 ): string {
   const configDir = getClaudeConfigHomeDir()
@@ -40,6 +44,12 @@ export function getMacOsKeychainStorageServiceName(
   return `Claude Code${getOauthConfig().OAUTH_FILE_SUFFIX}${serviceSuffix}${dirHash}`
 }
 
+export function getMacOsKeychainStorageServiceName(
+  serviceSuffix: string = '',
+): string {
+  return getSecureStorageServiceName(serviceSuffix)
+}
+
 export function getUsername(): string {
   try {
     return process.env.USER || userInfo().username

src/utils/secureStorage/macOsKeychainStorage.ts

@@ -11,7 +11,7 @@ import {
   KEYCHAIN_CACHE_TTL_MS,
   keychainCacheState,
 } from './macOsKeychainHelpers.js'
-import type { SecureStorage, SecureStorageData } from './types.js'
+import type { SecureStorage, SecureStorageData } from './index.js'
 
 // `security -i` reads stdin with a 4096-byte fgets() buffer (BUFSIZ on darwin).
 // A command line longer than this is truncated mid-argument: the first 4096

src/utils/secureStorage/plainTextStorage.ts

@@ -8,7 +8,7 @@ import {
   jsonStringify,
   writeFileSync_DEPRECATED,
 } from '../slowOperations.js'
-import type { SecureStorage, SecureStorageData } from './types.js'
+import type { SecureStorage, SecureStorageData } from './index.js'
 
 function getStoragePath(): { storageDir: string; storagePath: string } {
   const storageDir = getClaudeConfigHomeDir()

src/utils/secureStorage/platformStorage.test.ts

@@ -0,0 +1,158 @@
+
+import { expect, test, mock, describe, beforeEach, afterEach } from "bun:test";
+import { getSecureStorage } from "./index.js";
+import { linuxSecretStorage } from "./linuxSecretStorage.js";
+import { windowsCredentialStorage } from "./windowsCredentialStorage.js";
+import { getSecureStorageServiceName, CREDENTIALS_SERVICE_SUFFIX } from "./macOsKeychainHelpers.js";
+
+// Mock execaSync
+const mockExecaSync = mock(() => ({ exitCode: 0, stdout: "" }));
+mock.module("execa", () => ({
+  execaSync: mockExecaSync,
+}));
+
+describe("Secure Storage Platform Implementations", () => {
+  const originalEnv = process.env;
+
+  beforeEach(() => {
+    process.env = { ...originalEnv };
+    mockExecaSync.mockClear();
+    // Default mock behavior
+    mockExecaSync.mockImplementation(() => ({ exitCode: 0, stdout: "" }));
+  });
+
+  afterEach(() => {
+    process.env = originalEnv;
+  });
+
+  const testData = { 
+    mcpOAuth: { 
+      "test-server": { 
+        accessToken: "secret-token", 
+        expiresAt: 123456789, 
+        serverName: "test", 
+        serverUrl: "http://test" 
+      } 
+    } 
+  };
+
+  describe("Config-Dir Isolation", () => {
+    test("service name changes with CLAUDE_CONFIG_DIR", () => {
+      const defaultName = getSecureStorageServiceName(CREDENTIALS_SERVICE_SUFFIX);
+      
+      process.env.CLAUDE_CONFIG_DIR = "/tmp/other-config";
+      const otherName = getSecureStorageServiceName(CREDENTIALS_SERVICE_SUFFIX);
+      
+      expect(otherName).not.toBe(defaultName);
+      expect(otherName).toContain("Claude Code");
+      expect(otherName).toContain(CREDENTIALS_SERVICE_SUFFIX);
+    });
+
+    test("Linux storage uses scoped service name", () => {
+      process.env.CLAUDE_CONFIG_DIR = "/tmp/linux-scoped";
+      const expectedName = getSecureStorageServiceName(CREDENTIALS_SERVICE_SUFFIX);
+      
+      linuxSecretStorage.update(testData);
+      
+      const args = mockExecaSync.mock.calls[0];
+      expect(args[1]).toContain(expectedName);
+    });
+
+    test("Windows storage uses scoped resource name", () => {
+      process.env.CLAUDE_CONFIG_DIR = "/tmp/win-scoped";
+      const expectedName = getSecureStorageServiceName(CREDENTIALS_SERVICE_SUFFIX);
+      
+      windowsCredentialStorage.update(testData);
+      
+      const script = mockExecaSync.mock.calls[0][1][1];
+      expect(script).toContain(expectedName);
+      expect(script).toContain("Add-Type -AssemblyName System.Runtime.WindowsRuntime");
+    });
+  });
+
+  describe("Windows PowerShell Escaping", () => {
+    test("escapes single quotes and prevents $ expansion", () => {
+      const dataWithDollar = { 
+        mcpOAuth: { 
+          "server": { 
+            accessToken: "token-with-$env:USERNAME",
+            expiresAt: 123,
+            serverName: "s",
+            serverUrl: "u"
+          } 
+        } 
+      };
+      
+      windowsCredentialStorage.update(dataWithDollar);
+      
+      const script = mockExecaSync.mock.calls[0][1][1];
+      // Should use single quotes for the payload
+      expect(script).toMatch(/'\{.*\}'/);
+      // Should escape ' by doubling it
+      expect(script).not.toContain("'token-with-$env:USERNAME'");
+      // But since it's JSON, the value will be "token-with-$env:USERNAME" inside the single-quoted string
+      // The JSON itself shouldn't have single quotes unless the data has them.
+      
+      const dataWithQuote = { mcpOAuth: { "s": { accessToken: "token'quote", expiresAt: 1, serverName: "s", serverUrl: "u" } } };
+      windowsCredentialStorage.update(dataWithQuote);
+      const script2 = mockExecaSync.mock.calls[1][1][1];
+      expect(script2).toContain("token''quote");
+    });
+
+    test("delete() includes assembly load", () => {
+      windowsCredentialStorage.delete();
+      const script = mockExecaSync.mock.calls[0][1][1];
+      expect(script).toContain("Add-Type -AssemblyName System.Runtime.WindowsRuntime");
+    });
+
+    test("escapes double quotes in username", () => {
+      process.env.USER = 'user"name';
+      windowsCredentialStorage.read();
+      const script = mockExecaSync.mock.calls[0][1][1];
+      expect(script).toContain('user`"name');
+      expect(script).not.toContain('user"name');
+    });
+  });
+
+  describe("Linux secret-tool Interaction", () => {
+    test("update passes payload via stdin", () => {
+      linuxSecretStorage.update(testData);
+      
+      const options = mockExecaSync.mock.calls[0][2];
+      expect(options.input).toContain("secret-token");
+    });
+
+    test("read parses stdout", () => {
+      mockExecaSync.mockReturnValue({ exitCode: 0, stdout: JSON.stringify(testData) });
+      const result = linuxSecretStorage.read();
+      
+      expect(result).toEqual(testData);
+    });
+  });
+
+  describe("Platform Selection", () => {
+    const originalPlatform = process.platform;
+
+    afterEach(() => {
+      Object.defineProperty(process, 'platform', { value: originalPlatform });
+    });
+
+    test("darwin returns keychain with fallback", () => {
+      Object.defineProperty(process, 'platform', { value: 'darwin' });
+      const storage = getSecureStorage();
+      expect(storage.name).toContain("keychain");
+    });
+
+    test("linux returns libsecret with fallback", () => {
+      Object.defineProperty(process, 'platform', { value: 'linux' });
+      const storage = getSecureStorage();
+      expect(storage.name).toContain("libsecret");
+    });
+
+    test("win32 returns credential-locker with fallback", () => {
+      Object.defineProperty(process, 'platform', { value: 'win32' });
+      const storage = getSecureStorage();
+      expect(storage.name).toContain("credential-locker");
+    });
+  });
+});

src/utils/secureStorage/windowsCredentialStorage.ts

@@ -0,0 +1,97 @@
+import { execaSync } from 'execa'
+import { jsonParse, jsonStringify } from '../slowOperations.js'
+import {
+  CREDENTIALS_SERVICE_SUFFIX,
+  getSecureStorageServiceName,
+  getUsername,
+} from './macOsKeychainHelpers.js'
+import type { SecureStorage, SecureStorageData } from './index.js'
+
+/**
+ * Windows-specific secure storage implementation using the Windows Credential Locker.
+ * Accessed via PowerShell's [Windows.Security.Credentials.PasswordVault].
+ */
+export const windowsCredentialStorage: SecureStorage = {
+  name: 'credential-locker',
+  read(): SecureStorageData | null {
+    const resourceName = getSecureStorageServiceName(
+      CREDENTIALS_SERVICE_SUFFIX,
+    ).replace(/"/g, '`"')
+    const username = getUsername().replace(/"/g, '`"')
+    // PowerShell script to retrieve password from vault
+    const script = `
+      Add-Type -AssemblyName System.Runtime.WindowsRuntime
+      $vault = New-Object Windows.Security.Credentials.PasswordVault
+      try {
+        $cred = $vault.Retrieve("${resourceName}", "${username}")
+        $cred.FillPassword()
+        $cred.Password
+      } catch {
+        exit 1
+      }
+    `
+    try {
+      const result = execaSync('powershell.exe', ['-Command', script], {
+        reject: false,
+      })
+      if (result.exitCode === 0 && result.stdout) {
+        return jsonParse(result.stdout)
+      }
+    } catch {
+      // fall through
+    }
+    return null
+  },
+  async readAsync(): Promise<SecureStorageData | null> {
+    return this.read()
+  },
+  update(data: SecureStorageData): { success: boolean; warning?: string } {
+    const resourceName = getSecureStorageServiceName(
+      CREDENTIALS_SERVICE_SUFFIX,
+    ).replace(/"/g, '`"')
+    const username = getUsername().replace(/"/g, '`"')
+    // Use single quotes for the payload and escape ' by doubling it ('').
+    // This prevents PowerShell from expanding $... inside the string.
+    const payload = jsonStringify(data).replace(/'/g, "''")
+    // PowerShell script to add/update credential in vault
+    const script = `
+      Add-Type -AssemblyName System.Runtime.WindowsRuntime
+      $vault = New-Object Windows.Security.Credentials.PasswordVault
+      $cred = New-Object Windows.Security.Credentials.PasswordCredential("${resourceName}", "${username}", '${payload}')
+      $vault.Add($cred)
+    `
+    try {
+      const result = execaSync('powershell.exe', ['-Command', script], {
+        reject: false,
+      })
+      return { success: result.exitCode === 0 }
+    } catch {
+      return { success: false }
+    }
+  },
+  delete(): boolean {
+    const resourceName = getSecureStorageServiceName(
+      CREDENTIALS_SERVICE_SUFFIX,
+    ).replace(/"/g, '`"')
+    const username = getUsername().replace(/"/g, '`"')
+    // PowerShell script to remove credential from vault
+    const script = `
+      Add-Type -AssemblyName System.Runtime.WindowsRuntime
+      $vault = New-Object Windows.Security.Credentials.PasswordVault
+      try {
+        $cred = $vault.Retrieve("${resourceName}", "${username}")
+        $vault.Remove($cred)
+      } catch {
+        exit 0
+      }
+    `
+    try {
+      const result = execaSync('powershell.exe', ['-Command', script], {
+        reject: false,
+      })
+      return result.exitCode === 0
+    } catch {
+      return false
+    }
+  },
+}