fix(auth): validate resolved credentials in status checks

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@stephendolan/ynab-cli",
-  "version": "2.8.2",
+  "version": "2.8.3",
   "description": "A command-line interface for You Need a Budget (YNAB)",
   "type": "module",
   "main": "./dist/cli.js",

src/commands/auth.test.ts

@@ -0,0 +1,73 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+vi.mock('../lib/api-client.js', () => ({
+  client: { checkAuthentication: vi.fn() },
+}));
+
+vi.mock('../lib/output.js', () => ({
+  outputJson: vi.fn(),
+}));
+
+import { client } from '../lib/api-client.js';
+import { outputJson } from '../lib/output.js';
+import { createAuthCommand } from './auth.js';
+
+const mockCheckAuthentication = client.checkAuthentication as ReturnType<typeof vi.fn>;
+const mockOutputJson = outputJson as ReturnType<typeof vi.fn>;
+
+describe('ynab auth status', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  async function runStatus() {
+    await createAuthCommand().parseAsync(['node', 'auth', 'status']);
+  }
+
+  it('renders only the authenticated user ID for a valid credential', async () => {
+    mockCheckAuthentication.mockResolvedValue({
+      authenticated: true,
+      credentialPresent: true,
+      user: { id: 'user-id', name: 'Jane Doe' },
+      token: 'valid-test-token',
+    });
+
+    await runStatus();
+
+    expect(mockOutputJson).toHaveBeenCalledWith({
+      authenticated: true,
+      user: { id: 'user-id' },
+    });
+    expect(JSON.stringify(mockOutputJson.mock.calls)).not.toContain('valid-test-token');
+  });
+
+  it('renders the invalid-token message without exposing the token', async () => {
+    mockCheckAuthentication.mockResolvedValue({
+      authenticated: false,
+      credentialPresent: true,
+      token: 'invalid-test-token',
+    });
+
+    await runStatus();
+
+    expect(mockOutputJson).toHaveBeenCalledWith({
+      authenticated: false,
+      message: 'Token exists but is invalid',
+    });
+    expect(JSON.stringify(mockOutputJson.mock.calls)).not.toContain('invalid-test-token');
+  });
+
+  it('renders the missing-credential message', async () => {
+    mockCheckAuthentication.mockResolvedValue({
+      authenticated: false,
+      credentialPresent: false,
+    });
+
+    await runStatus();
+
+    expect(mockOutputJson).toHaveBeenCalledWith({
+      authenticated: false,
+      message: 'Not authenticated',
+    });
+  });
+});

src/commands/auth.ts

@@ -74,19 +74,17 @@ export function createAuthCommand(): Command {
     .description('Check authentication status')
     .action(
       withErrorHandling(async () => {
-        const isAuthenticated = await auth.isAuthenticated();
+        const status = await client.checkAuthentication();
 
-        if (!isAuthenticated) {
-          outputJson({ authenticated: false, message: 'Not authenticated' });
+        if (!status.authenticated) {
+          outputJson({
+            authenticated: false,
+            message: status.credentialPresent ? 'Token exists but is invalid' : 'Not authenticated',
+          });
           return;
         }
 
-        try {
-          const user = await client.getUser();
-          outputJson({ authenticated: true, user: { id: user?.id } });
-        } catch {
-          outputJson({ authenticated: false, message: 'Token exists but is invalid' });
-        }
+        outputJson({ authenticated: true, user: { id: status.user?.id } });
       })
     );
 

src/lib/api-client.test.ts

@@ -0,0 +1,116 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+vi.mock('ynab', () => ({ API: vi.fn() }));
+
+vi.mock('./auth.js', () => ({
+  auth: { resolveCredential: vi.fn() },
+}));
+
+import * as ynab from 'ynab';
+import { auth } from './auth.js';
+import { YnabClient } from './api-client.js';
+
+const mockApiConstructor = ynab.API as unknown as ReturnType<typeof vi.fn>;
+const mockGetUser = vi.fn();
+const mockResolveCredential = auth.resolveCredential as ReturnType<typeof vi.fn>;
+
+describe('YnabClient authentication status', () => {
+  const validToken = 'valid-test-token';
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockGetUser.mockResolvedValue({ data: { user: { id: 'user-id' } } });
+    mockApiConstructor.mockImplementation(function () {
+      return { user: { getUser: mockGetUser } };
+    });
+  });
+
+  it('returns authenticated for a valid keychain token', async () => {
+    mockResolveCredential.mockResolvedValue({ token: validToken, source: 'keychain' });
+
+    const status = await new YnabClient().checkAuthentication();
+
+    expect(status).toEqual({
+      authenticated: true,
+      credentialPresent: true,
+      user: { id: 'user-id' },
+    });
+    expect(mockApiConstructor).toHaveBeenCalledWith(validToken);
+  });
+
+  it('returns authenticated for a valid YNAB_API_KEY', async () => {
+    mockResolveCredential.mockResolvedValue({ token: validToken, source: 'environment' });
+
+    const status = await new YnabClient().checkAuthentication();
+
+    expect(status.authenticated).toBe(true);
+    expect(mockApiConstructor).toHaveBeenCalledWith(validToken);
+  });
+
+  it('returns unauthenticated for an invalid YNAB_API_KEY', async () => {
+    mockResolveCredential.mockResolvedValue({ token: 'invalid-test-token', source: 'environment' });
+    mockGetUser.mockRejectedValue({
+      error: { id: '401', name: 'unauthorized', detail: 'Unauthorized' },
+    });
+
+    const status = await new YnabClient().checkAuthentication();
+
+    expect(status).toEqual({ authenticated: false, credentialPresent: true });
+  });
+
+  it('propagates network failures', async () => {
+    mockResolveCredential.mockResolvedValue({ token: validToken, source: 'keychain' });
+    mockGetUser.mockRejectedValue(new TypeError('fetch failed'));
+
+    await expect(new YnabClient().checkAuthentication()).rejects.toThrow('fetch failed');
+  });
+
+  it('propagates rate limit failures', async () => {
+    mockResolveCredential.mockResolvedValue({ token: validToken, source: 'keychain' });
+    const rateLimitError = {
+      error: { id: '429', name: 'too_many_requests', detail: 'Too many requests' },
+    };
+    mockGetUser.mockRejectedValue(rateLimitError);
+
+    await expect(new YnabClient().checkAuthentication()).rejects.toEqual(rateLimitError);
+  });
+
+  it('uses the validated API instance after credential rotation', async () => {
+    const client = new YnabClient();
+    mockResolveCredential.mockResolvedValue({ token: 'first-token', source: 'keychain' });
+
+    await client.checkAuthentication();
+
+    mockResolveCredential.mockResolvedValue({ token: 'second-token', source: 'keychain' });
+    await client.checkAuthentication();
+    await client.getUser();
+
+    expect(mockApiConstructor).toHaveBeenCalledTimes(2);
+    expect(mockApiConstructor).toHaveBeenLastCalledWith('second-token');
+  });
+
+  it('returns unauthenticated without making a request when no credential exists', async () => {
+    mockResolveCredential.mockResolvedValue(null);
+
+    const status = await new YnabClient().checkAuthentication();
+
+    expect(status).toEqual({ authenticated: false, credentialPresent: false });
+    expect(mockApiConstructor).not.toHaveBeenCalled();
+    expect(mockGetUser).not.toHaveBeenCalled();
+  });
+
+  it('clears the cached API when no credential exists', async () => {
+    const client = new YnabClient();
+    mockResolveCredential.mockResolvedValue({ token: validToken, source: 'keychain' });
+
+    await client.checkAuthentication();
+
+    mockResolveCredential.mockResolvedValue(null);
+    await expect(client.getApi()).rejects.toMatchObject({ statusCode: 401 });
+
+    mockResolveCredential.mockResolvedValue({ token: validToken, source: 'keychain' });
+    await client.getUser();
+
+    expect(mockApiConstructor).toHaveBeenCalledTimes(2);
+  });
+});

src/lib/api-client.ts

@@ -1,35 +1,41 @@
 import * as ynab from 'ynab';
 import { config } from './config.js';
 import { YnabCliError, sanitizeApiError } from './errors.js';
-import { auth } from './auth.js';
+import { auth, type ResolvedCredential } from './auth.js';
 
 type TransactionTypeFilter = 'uncategorized' | 'unapproved' | undefined;
 
+function isUnauthorizedError(error: unknown): boolean {
+  if (typeof error !== 'object' || error === null) {
+    return false;
+  }
+
+  const apiError = (error as { error?: unknown }).error;
+  if (typeof apiError !== 'object' || apiError === null) {
+    return false;
+  }
+
+  const { id, name } = apiError as { id?: unknown; name?: unknown };
+  return id === '401' && name === 'unauthorized';
+}
+
 export class YnabClient {
   private api: ynab.API | null = null;
+  private apiToken: string | null = null;
   private envVarWarningShown = false;
 
   clearApi(): void {
     this.api = null;
+    this.apiToken = null;
     this.envVarWarningShown = false;
   }
 
-  async getApi(): Promise<ynab.API> {
-    if (this.api) {
+  private getApiForCredential(credential: ResolvedCredential): ynab.API {
+    if (this.api && this.apiToken === credential.token) {
       return this.api;
     }
 
-    const keychainToken = await auth.getAccessToken();
-    const accessToken = keychainToken || process.env.YNAB_API_KEY || null;
-
-    if (!accessToken) {
-      throw new YnabCliError(
-        'Not authenticated. Please run: ynab auth login or set YNAB_API_KEY environment variable',
-        401
-      );
-    }
-
-    if (!keychainToken && process.env.YNAB_API_KEY && !this.envVarWarningShown) {
+    if (credential.source === 'environment' && !this.envVarWarningShown) {
       console.warn(
         '\x1b[33m⚠️  WARNING: Using YNAB_API_KEY environment variable.\n' +
           'Environment variables may be visible to other processes.\n' +
@@ -38,10 +44,28 @@ export class YnabClient {
       this.envVarWarningShown = true;
     }
 
-    this.api = new ynab.API(accessToken);
+    this.api = new ynab.API(credential.token);
+    this.apiToken = credential.token;
     return this.api;
   }
 
+  private async resolveApi(): Promise<{ api: ynab.API; credential: ResolvedCredential }> {
+    const credential = await auth.resolveCredential();
+    if (!credential) {
+      this.clearApi();
+      throw new YnabCliError(
+        'Not authenticated. Please run: ynab auth login or set YNAB_API_KEY environment variable',
+        401
+      );
+    }
+
+    return { api: this.getApiForCredential(credential), credential };
+  }
+
+  async getApi(): Promise<ynab.API> {
+    return (await this.resolveApi()).api;
+  }
+
   async getBudgetId(budgetIdOrDefault?: string): Promise<string> {
     const budgetId = (budgetIdOrDefault && budgetIdOrDefault !== 'default' ? budgetIdOrDefault : undefined) || config.getDefaultBudget() || process.env.YNAB_BUDGET_ID;
 
@@ -61,6 +85,29 @@ export class YnabClient {
     return response.data.user;
   }
 
+  async checkAuthentication() {
+    const credential = await auth.resolveCredential();
+    if (!credential) {
+      this.clearApi();
+      return { authenticated: false, credentialPresent: false } as const;
+    }
+
+    try {
+      const api = this.getApiForCredential(credential);
+      const response = await api.user.getUser();
+      return {
+        authenticated: true,
+        credentialPresent: true,
+        user: response.data.user,
+      } as const;
+    } catch (error) {
+      if (isUnauthorizedError(error)) {
+        return { authenticated: false, credentialPresent: true } as const;
+      }
+      throw error;
+    }
+  }
+
   async getBudgets(includeAccounts = false) {
     const api = await this.getApi();
     const response = await api.plans.getPlans(includeAccounts);
@@ -367,7 +414,7 @@ export class YnabClient {
   }
 
   async rawApiCall(method: string, path: string, data?: unknown, budgetId?: string) {
-    await this.getApi();
+    const { credential } = await this.resolveApi();
 
     let fullPath = path;
     if (path.includes('{budget_id}') || path.includes('{plan_id}')) {
@@ -376,9 +423,8 @@ export class YnabClient {
     }
 
     const url = `https://api.ynab.com/v1${fullPath}`;
-    const accessToken = (await auth.getAccessToken()) || process.env.YNAB_API_KEY;
     const headers = {
-      Authorization: `Bearer ${accessToken}`,
+      Authorization: `Bearer ${credential.token}`,
       'Content-Type': 'application/json',
     };