fix(desktop): validate generic oauth token responses

packages/desktop/packages/shared/src/auth/__tests__/generic-oauth.test.ts

@@ -0,0 +1,160 @@
+import { afterEach, beforeEach, describe, expect, it } from 'bun:test';
+import { exchangeGenericOAuth, refreshGenericOAuthToken } from '../generic-oauth.ts';
+import type { OAuthExchangeParams } from '../oauth-flow-types.ts';
+
+const exchangeParams: OAuthExchangeParams = {
+  code: 'code-123',
+  codeVerifier: 'verifier-123',
+  tokenEndpoint: 'https://auth.example.com/oauth/token',
+  clientId: 'client-123',
+  clientSecret: 'secret-123',
+  redirectUri: 'https://app.example.com/callback',
+};
+
+function mockTokenResponse(body: unknown, contentType = 'application/json') {
+  globalThis.fetch = (async () =>
+    new Response(
+      typeof body === 'string' ? body : JSON.stringify(body),
+      { status: 200, headers: { 'Content-Type': contentType } },
+    )) as unknown as typeof globalThis.fetch;
+}
+
+describe('generic OAuth token responses', () => {
+  let originalFetch: typeof globalThis.fetch;
+
+  beforeEach(() => {
+    originalFetch = globalThis.fetch;
+  });
+
+  afterEach(() => {
+    globalThis.fetch = originalFetch;
+  });
+
+  it('exchanges JSON token responses with numeric expires_in', async () => {
+    mockTokenResponse({
+      access_token: 'access-token',
+      refresh_token: 'refresh-token',
+      expires_in: 3600,
+    });
+
+    const result = await exchangeGenericOAuth(exchangeParams);
+
+    expect(result.success).toBe(true);
+    expect(result.accessToken).toBe('access-token');
+    expect(result.refreshToken).toBe('refresh-token');
+    expect(result.expiresAt).toBeGreaterThan(Date.now());
+  });
+
+  it('exchanges form-encoded token responses with string expires_in', async () => {
+    mockTokenResponse(
+      'access_token=access-token&refresh_token=refresh-token&expires_in=3600',
+      'application/x-www-form-urlencoded',
+    );
+
+    const result = await exchangeGenericOAuth(exchangeParams);
+
+    expect(result.success).toBe(true);
+    expect(result.accessToken).toBe('access-token');
+    expect(result.refreshToken).toBe('refresh-token');
+    expect(result.expiresAt).toBeGreaterThan(Date.now());
+  });
+
+  it('rejects exchange responses without access_token', async () => {
+    mockTokenResponse({ refresh_token: 'refresh-token', expires_in: 3600 });
+
+    const result = await exchangeGenericOAuth(exchangeParams);
+
+    expect(result.success).toBe(false);
+    expect(result.error).toContain('missing access_token');
+  });
+
+  it('rejects exchange responses with malformed expires_in', async () => {
+    mockTokenResponse({ access_token: 'access-token', expires_in: '3600abc' });
+
+    const result = await exchangeGenericOAuth(exchangeParams);
+
+    expect(result.success).toBe(false);
+    expect(result.error).toContain('invalid expires_in');
+  });
+
+  it('rejects unsafe JSON number expires_in values', async () => {
+    mockTokenResponse({ access_token: 'access-token', expires_in: Number.MAX_SAFE_INTEGER + 1 });
+
+    const result = await exchangeGenericOAuth(exchangeParams);
+
+    expect(result.success).toBe(false);
+    expect(result.error).toContain('invalid expires_in');
+  });
+
+  it('preserves zero-second expiries instead of treating them as missing', async () => {
+    mockTokenResponse({ access_token: 'access-token', expires_in: 0 });
+    const before = Date.now();
+
+    const result = await exchangeGenericOAuth(exchangeParams);
+
+    expect(result.success).toBe(true);
+    expect(result.expiresAt).toBeGreaterThanOrEqual(before);
+    expect(result.expiresAt).toBeLessThanOrEqual(Date.now());
+  });
+
+  it('rejects refresh responses without access_token', async () => {
+    mockTokenResponse({ refresh_token: 'refresh-token', expires_in: 3600 });
+
+    await expect(
+      refreshGenericOAuthToken(
+        'refresh-token',
+        'https://auth.example.com/oauth/token',
+        'client-123',
+        'secret-123',
+      ),
+    ).rejects.toThrow('missing access_token');
+  });
+
+  it('refreshes JSON token responses with refresh_token and numeric expires_in', async () => {
+    mockTokenResponse({
+      access_token: 'new-access-token',
+      refresh_token: 'new-refresh-token',
+      expires_in: 3600,
+    });
+
+    const result = await refreshGenericOAuthToken(
+      'old-refresh-token',
+      'https://auth.example.com/oauth/token',
+      'client-123',
+      'secret-123',
+    );
+
+    expect(result.accessToken).toBe('new-access-token');
+    expect(result.refreshToken).toBe('new-refresh-token');
+    expect(result.expiresAt).toBeGreaterThan(Date.now());
+  });
+
+  it('preserves zero-second expiries on refresh responses', async () => {
+    mockTokenResponse({ access_token: 'new-access-token', expires_in: 0 });
+    const before = Date.now();
+
+    const result = await refreshGenericOAuthToken(
+      'old-refresh-token',
+      'https://auth.example.com/oauth/token',
+      'client-123',
+      'secret-123',
+    );
+
+    expect(result.accessToken).toBe('new-access-token');
+    expect(result.expiresAt).toBeGreaterThanOrEqual(before);
+    expect(result.expiresAt).toBeLessThanOrEqual(Date.now());
+  });
+
+  it('rejects refresh responses with malformed expires_in', async () => {
+    mockTokenResponse({ access_token: 'access-token', expires_in: '1e3' });
+
+    await expect(
+      refreshGenericOAuthToken(
+        'refresh-token',
+        'https://auth.example.com/oauth/token',
+        'client-123',
+        'secret-123',
+      ),
+    ).rejects.toThrow('invalid expires_in');
+  });
+});

packages/desktop/packages/shared/src/auth/generic-oauth.ts

@@ -17,7 +17,7 @@ import { generatePKCE, generateState } from './pkce.ts';
  * GitHub (and some other providers) return form-encoded unless you send Accept: application/json.
  * We send Accept: application/json but tolerate form-encoded as a fallback.
  */
-function parseTokenResponse(body: string, contentType: string | null): Record<string, string> {
+function parseTokenResponse(body: string, contentType: string | null): Record<string, unknown> {
   if (contentType?.includes('application/json')) {
     return JSON.parse(body);
   }
@@ -31,6 +31,43 @@ function parseTokenResponse(body: string, contentType: string | null): Record<st
   return Object.fromEntries(new URLSearchParams(body));
 }
 
+function readStringField(data: Record<string, unknown>, field: string): string | undefined {
+  const value = data[field];
+  return typeof value === 'string' && value.length > 0
+    ? value
+    : undefined;
+}
+
+function readAccessToken(data: Record<string, unknown>): string | null {
+  return readStringField(data, 'access_token') ?? null;
+}
+
+function parseExpiresIn(data: Record<string, unknown>): number | undefined {
+  const raw = data.expires_in;
+  if (raw == null || raw === '') return undefined;
+
+  if (typeof raw === 'number') {
+    if (Number.isSafeInteger(raw) && raw >= 0) return raw;
+    throw new Error('Token response has invalid expires_in');
+  }
+
+  const trimmed = String(raw).trim();
+  if (!/^\d+$/.test(trimmed)) {
+    throw new Error('Token response has invalid expires_in');
+  }
+
+  const value = Number(trimmed);
+  if (!Number.isSafeInteger(value)) {
+    throw new Error('Token response has invalid expires_in');
+  }
+
+  return value;
+}
+
+function expiresAtFromSeconds(expiresIn: number | undefined): number | undefined {
+  return expiresIn == null ? undefined : Date.now() + expiresIn * 1000;
+}
+
 // ============================================================
 // Prepare
 // ============================================================
@@ -123,16 +160,19 @@ export async function exchangeGenericOAuth(params: OAuthExchangeParams): Promise
     const data = parseTokenResponse(responseBody, response.headers.get('content-type'));
 
     if (data.error) {
-      return { success: false, error: `OAuth error: ${data.error} — ${data.error_description ?? ''}` };
+      return { success: false, error: `OAuth error: ${String(data.error)} — ${String(data.error_description ?? '')}` };
     }
 
-    const expiresIn = data.expires_in ? parseInt(data.expires_in, 10) : undefined;
+    const accessToken = readAccessToken(data);
+    if (!accessToken) {
+      return { success: false, error: 'Token exchange response missing access_token' };
+    }
 
     return {
       success: true,
-      accessToken: data.access_token,
-      refreshToken: data.refresh_token,
-      expiresAt: expiresIn ? Date.now() + expiresIn * 1000 : undefined,
+      accessToken,
+      refreshToken: readStringField(data, 'refresh_token'),
+      expiresAt: expiresAtFromSeconds(parseExpiresIn(data)),
       oauthClientId: params.clientId,
       oauthClientSecret: params.clientSecret,
     };
@@ -186,18 +226,17 @@ export async function refreshGenericOAuthToken(
   const data = parseTokenResponse(responseBody, response.headers.get('content-type'));
 
   if (data.error) {
-    throw new Error(`OAuth refresh error: ${data.error} — ${data.error_description ?? ''}`);
+    throw new Error(`OAuth refresh error: ${String(data.error)} — ${String(data.error_description ?? '')}`);
   }
 
-  if (!data.access_token) {
+  const accessToken = readAccessToken(data);
+  if (!accessToken) {
     throw new Error('Token refresh response missing access_token');
   }
 
-  const expiresIn = data.expires_in ? parseInt(data.expires_in, 10) : undefined;
-
   return {
-    accessToken: data.access_token,
-    refreshToken: data.refresh_token,
-    expiresAt: expiresIn ? Date.now() + expiresIn * 1000 : undefined,
+    accessToken,
+    refreshToken: readStringField(data, 'refresh_token'),
+    expiresAt: expiresAtFromSeconds(parseExpiresIn(data)),
   };
 }