Merge pull request #1402 from vybestack/issue1351_1352

feat: KeyringTokenStore replaces plaintext OAuth token storage (Fixes #1351, Fixes #1352)

Author: acoliver

.github/workflows/ci.yml

@@ -374,7 +374,7 @@ jobs:
   # Test: Node (Linux and macOS - always run)
   #
   test:
-    name: 'Test'
+    name: 'Test (${{ matrix.os }}, ${{ matrix.secure-store-mode }})'
     runs-on: '${{ matrix.os }}'
     needs:
       - 'lint'
@@ -393,6 +393,9 @@ jobs:
           - 'macos-latest'
         node-version:
           - '24.x'
+        secure-store-mode:
+          - 'keyring'
+          - 'fallback'
     steps:
       - name: 'Checkout'
         uses: 'actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8' # ratchet:actions/checkout@v5
@@ -464,6 +467,8 @@ jobs:
           CI: true
           # Allow OpenAI integration tests to exceed Vitest's 5s default timeout (issue #338)
           VITEST_TEST_TIMEOUT: 15000
+          # Force SecureStore to use encrypted-file fallback instead of keyring (R15.2)
+          LLXPRT_SECURE_STORE_FORCE_FALLBACK: ${{ matrix.secure-store-mode == 'fallback' && 'true' || '' }}
         run: 'npm run test'
 
       - name: 'Run script harness tests (macOS)'
@@ -499,15 +504,15 @@ jobs:
           ${{ always() && (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository) }}
         uses: 'actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02' # ratchet:actions/upload-artifact@v4
         with:
-          name: 'test-results-fork-${{ matrix.node-version }}-${{ matrix.os }}'
+          name: 'test-results-fork-${{ matrix.node-version }}-${{ matrix.os }}-${{ matrix.secure-store-mode }}'
           path: 'packages/*/junit.xml'
 
       - name: 'Upload coverage reports'
         if: |-
           ${{ always() }}
         uses: 'actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02' # ratchet:actions/upload-artifact@v4
         with:
-          name: 'coverage-reports-${{ matrix.node-version }}-${{ matrix.os }}'
+          name: 'coverage-reports-${{ matrix.node-version }}-${{ matrix.os }}-${{ matrix.secure-store-mode }}'
           path: 'packages/*/coverage'
 
   post_coverage_comment:
@@ -534,7 +539,7 @@ jobs:
       - name: 'Download coverage reports artifact'
         uses: 'actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0' # ratchet:actions/download-artifact@v5
         with:
-          name: 'coverage-reports-${{ matrix.node-version }}-${{ matrix.os }}'
+          name: 'coverage-reports-${{ matrix.node-version }}-${{ matrix.os }}-keyring'
           path: 'coverage_artifact' # Download to a specific directory
 
       - name: 'Post Coverage Comment using Composite Action'

packages/cli/src/auth/__tests__/codex-oauth-provider.test.ts

@@ -7,7 +7,7 @@
 import { describe, it, expect, beforeEach, afterEach } from 'vitest';
 import { CodexOAuthProvider } from '../codex-oauth-provider.js';
 import {
-  MultiProviderTokenStore,
+  KeyringTokenStore,
   CodexOAuthTokenSchema,
 } from '@vybestack/llxprt-code-core';
 import type { TokenStore } from '@vybestack/llxprt-code-core';
@@ -36,7 +36,7 @@ describe('CodexOAuthProvider', () => {
     process.env.HOME = tempDir;
 
     // tokenStore constructor uses homedir() which reads from process.env.HOME
-    tokenStore = new MultiProviderTokenStore();
+    tokenStore = new KeyringTokenStore();
     provider = new CodexOAuthProvider(tokenStore);
   });
 

packages/cli/src/auth/codex-oauth-provider.ts

@@ -302,7 +302,7 @@ export class CodexOAuthProvider implements OAuthProvider {
         `[FLOW] Token received: access_token=${token.access_token.substring(0, 10)}..., account_id=${token.account_id?.substring(0, 8) ?? 'MISSING'}..., expiry=${token.expiry}`,
     );
 
-    // Save to MultiProviderTokenStore location (~/.llxprt/oauth/codex.json)
+    // Save to KeyringTokenStore (keyring or encrypted fallback)
     this.logger.debug(() => '[FLOW] Saving token to tokenStore...');
     await this.tokenStore.saveToken('codex', token);
     this.logger.debug(() => '[FLOW] Token saved to tokenStore');
@@ -436,7 +436,7 @@ export class CodexOAuthProvider implements OAuthProvider {
     this.logger.debug(() => '[FLOW] getToken() called');
     await this.ensureInitialized();
 
-    // Get token from ~/.llxprt/oauth/codex.json
+    // Get token from KeyringTokenStore (keyring or encrypted fallback)
     this.logger.debug(() => '[FLOW] Reading token from tokenStore...');
     const token = await this.tokenStore.getToken('codex');
 

packages/cli/src/auth/oauth-manager-initialization.spec.ts

@@ -6,7 +6,7 @@
 
 import { describe, it, expect, beforeEach, vi } from 'vitest';
 import { OAuthManager } from './oauth-manager.js';
-import { MultiProviderTokenStore } from './types.js';
+import { KeyringTokenStore } from './types.js';
 import { GeminiOAuthProvider } from './gemini-oauth-provider.js';
 import { QwenOAuthProvider } from './qwen-oauth-provider.js';
 import { AnthropicOAuthProvider } from './anthropic-oauth-provider.js';
@@ -28,12 +28,12 @@ vi.mock('node:fs', async (importOriginal) => {
 const mockFs = vi.mocked(fs);
 
 describe('OAuth Provider Premature Initialization', () => {
-  let tokenStore: MultiProviderTokenStore;
+  let tokenStore: KeyringTokenStore;
   let oauthManager: OAuthManager;
 
   beforeEach(() => {
     vi.clearAllMocks();
-    tokenStore = new MultiProviderTokenStore();
+    tokenStore = new KeyringTokenStore();
     oauthManager = new OAuthManager(tokenStore);
 
     // Mock the OAuth credentials file to not exist

packages/cli/src/auth/oauth-manager.refresh-race.spec.ts

@@ -15,7 +15,37 @@ import type { OAuthToken, TokenStore } from './types.js';
 import { promises as fs } from 'fs';
 import { join } from 'path';
 import { tmpdir } from 'os';
-import { MultiProviderTokenStore } from '@vybestack/llxprt-code-core';
+import {
+  KeyringTokenStore,
+  SecureStore,
+  type KeyringAdapter,
+} from '@vybestack/llxprt-code-core';
+
+function createMockKeyring(): KeyringAdapter & { store: Map<string, string> } {
+  const store = new Map<string, string>();
+  return {
+    store,
+    getPassword: async (service: string, account: string) =>
+      store.get(`${service}:${account}`) ?? null,
+    setPassword: async (service: string, account: string, password: string) => {
+      store.set(`${service}:${account}`, password);
+    },
+    deletePassword: async (service: string, account: string) =>
+      store.delete(`${service}:${account}`),
+    findCredentials: async (service: string) => {
+      const results: Array<{ account: string; password: string }> = [];
+      for (const [key, value] of store.entries()) {
+        if (key.startsWith(`${service}:`)) {
+          results.push({
+            account: key.slice(service.length + 1),
+            password: value,
+          });
+        }
+      }
+      return results;
+    },
+  };
+}
 
 describe('OAuthManager - Token Refresh Race Condition (Issue #1159)', () => {
   let tempDir: string;
@@ -35,7 +65,12 @@ describe('OAuthManager - Token Refresh Race Condition (Issue #1159)', () => {
   beforeEach(async () => {
     // Create a temporary directory for testing
     tempDir = await fs.mkdtemp(join(tmpdir(), 'oauth-refresh-race-test-'));
-    tokenStore = new MultiProviderTokenStore(tempDir);
+    const secureStore = new SecureStore('llxprt-code-oauth', {
+      fallbackDir: tempDir,
+      fallbackPolicy: 'allow',
+      keyringLoader: async () => createMockKeyring(),
+    });
+    tokenStore = new KeyringTokenStore({ secureStore });
     oauthManager = new OAuthManager(tokenStore);
 
     // Reset refresh call counter

packages/cli/src/auth/types.ts

@@ -11,4 +11,4 @@ export type {
   TokenStore,
 } from '@vybestack/llxprt-code-core';
 
-export { MultiProviderTokenStore } from '@vybestack/llxprt-code-core';
+export { KeyringTokenStore } from '@vybestack/llxprt-code-core';