fix(quickbooks): address BugBot findings

Author: Bill Leoutsakos

apps/sim/app/api/auth/oauth/token/route.test.ts

@@ -579,6 +579,33 @@ describe('OAuth Token API Routes', () => {
       expect(data).toHaveProperty('error')
     })
 
+    it('rejects a malformed QuickBooks identity before reporting a missing token', async () => {
+      mockAuthorizeCredentialUse.mockResolvedValueOnce({
+        ok: true,
+        authType: 'session',
+        requesterUserId: 'test-user-id',
+        credentialOwnerUserId: 'test-user-id',
+      })
+      authOAuthUtilsMockFns.mockGetCredential.mockResolvedValueOnce({
+        id: 'credential-id',
+        accountId: 'malformed',
+        accessToken: null,
+        refreshToken: 'refresh-token',
+        providerId: 'quickbooks',
+      })
+
+      const response = await GET(
+        new NextRequest(
+          'http://localhost:3000/api/auth/oauth/token?credentialId=credential-id'
+        ) as any
+      )
+      const data = await response.json()
+
+      expect(response.status).toBe(401)
+      expect(data.error).toMatch(/Reconnect the QuickBooks credential/)
+      expect(authOAuthUtilsMockFns.mockRefreshTokenIfNeeded).not.toHaveBeenCalled()
+    })
+
     it('should handle token refresh failure', async () => {
       mockAuthorizeCredentialUse.mockResolvedValueOnce({
         ok: true,

apps/sim/app/api/auth/oauth/token/route.ts

@@ -371,11 +371,6 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
       return NextResponse.json({ error: 'Credential not found' }, { status: 404 })
     }
 
-    if (!credential.accessToken) {
-      logger.warn(`[${requestId}] No access token available for credential`)
-      return NextResponse.json({ error: 'No access token available' }, { status: 400 })
-    }
-
     const actorId = authz.requesterUserId
     const workspaceId = authz.workspaceId ?? null
 
@@ -396,6 +391,11 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
       }
     }
 
+    if (!credential.accessToken) {
+      logger.warn(`[${requestId}] No access token available for credential`)
+      return NextResponse.json({ error: 'No access token available' }, { status: 400 })
+    }
+
     try {
       const { accessToken } = await refreshTokenIfNeeded(
         requestId,

apps/sim/lib/quickbooks/fault.ts

@@ -11,18 +11,22 @@ export function sanitizeQuickBooksFaultData(data: unknown): SanitizedQuickBooksF
   const errors = (fault as Record<string, unknown>).Error
   if (!Array.isArray(errors)) return null
 
+  const sanitizedErrors = errors.flatMap((entry) => {
+    if (!entry || typeof entry !== 'object' || Array.isArray(entry)) return []
+    const value = entry as Record<string, unknown>
+    const sanitized = Object.fromEntries(
+      ['code', 'Message', 'Detail', 'element'].flatMap((key) => {
+        const field = typeof value[key] === 'string' ? value[key].trim() : ''
+        return field ? [[key, field]] : []
+      })
+    )
+    return Object.keys(sanitized).length > 0 ? [sanitized] : []
+  })
+  if (sanitizedErrors.length === 0) return null
+
   return {
     Fault: {
-      Error: errors.flatMap((entry) => {
-        if (!entry || typeof entry !== 'object' || Array.isArray(entry)) return []
-        const value = entry as Record<string, unknown>
-        const sanitized = Object.fromEntries(
-          ['code', 'Message', 'Detail', 'element'].flatMap((key) =>
-            typeof value[key] === 'string' ? [[key, value[key]]] : []
-          )
-        )
-        return Object.keys(sanitized).length > 0 ? [sanitized] : []
-      }),
+      Error: sanitizedErrors,
     },
   }
 }

apps/sim/tools/index.test.ts

@@ -1517,6 +1517,42 @@ describe('OAuth provider context propagation', () => {
     expect(fetchMock).toHaveBeenCalledTimes(2)
   })
 
+  it('rejects a stale QuickBooks realm when the credential response omits its binding', async () => {
+    mockGenerateInternalToken.mockResolvedValue('internal-token')
+    const fetchMock = vi.fn().mockImplementation(async (url: string) => {
+      if (url.includes('/api/auth/oauth/token')) {
+        return new Response(JSON.stringify({ accessToken: 'fresh-access-token' }), {
+          headers: { 'Content-Type': 'application/json' },
+        })
+      }
+
+      throw new Error('QuickBooks API must not be called with an unbound realm')
+    })
+    global.fetch = Object.assign(fetchMock, { preconnect: vi.fn() }) as typeof fetch
+
+    const result = await executeTool(
+      'test_quickbooks_context',
+      {
+        credential: 'quickbooks-credential',
+        realmId: 'workflow-supplied-company',
+      },
+      {
+        executionContext: createToolExecutionContext({
+          userId: 'user-123',
+          workflowId: 'workflow-123',
+        }),
+      }
+    )
+
+    expect(result).toMatchObject({
+      success: false,
+      error: expect.stringContaining(
+        'QuickBooks company identity is missing. Reconnect the QuickBooks credential.'
+      ),
+    })
+    expect(fetchMock).toHaveBeenCalledTimes(1)
+  })
+
   it('does not expose a non-JSON QuickBooks failure body in tool output', async () => {
     mockGenerateInternalToken.mockResolvedValue('internal-token')
     const fetchMock = vi.fn().mockImplementation(async (url: string) => {

apps/sim/tools/index.ts

@@ -1248,8 +1248,15 @@ export async function executeTool(
         if (data.domain && !contextParams.domain) {
           contextParams.domain = data.domain
         }
-        if (data.realmId) {
-          contextParams.realmId = data.realmId
+        if (tool?.oauth?.provider === 'quickbooks') {
+          const credentialRealmId = typeof data.realmId === 'string' ? data.realmId.trim() : ''
+          contextParams.realmId = undefined
+          if (!credentialRealmId) {
+            throw new Error(
+              'QuickBooks company identity is missing. Reconnect the QuickBooks credential.'
+            )
+          }
+          contextParams.realmId = credentialRealmId
         }
         if (data.authStyle && !contextParams.authStyle) {
           contextParams.authStyle = data.authStyle

apps/sim/tools/quickbooks/error-extractor.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, it } from 'vitest'
+import { sanitizeQuickBooksFaultData } from '@/lib/quickbooks/fault'
 import { ErrorExtractorId, extractErrorMessage } from '@/tools/error-extractors'
 
 describe('QuickBooks fault extraction', () => {
@@ -68,4 +69,15 @@ describe('QuickBooks fault extraction', () => {
       extractErrorMessage({ status: 502, data: null }, ErrorExtractorId.QUICKBOOKS_FAULT)
     ).toBe('QuickBooks request failed with HTTP 502.')
   })
+
+  it('ignores fault envelopes without a usable error entry', () => {
+    expect(sanitizeQuickBooksFaultData({ Fault: { Error: [] } })).toBeNull()
+    expect(
+      sanitizeQuickBooksFaultData({
+        Fault: {
+          Error: [{ code: ' ', Message: '', ignored: 'not a documented fault field' }, null],
+        },
+      })
+    ).toBeNull()
+  })
 })

apps/sim/tools/quickbooks/quickbooks.test.ts

@@ -150,6 +150,23 @@ describe('QuickBooks response contracts', () => {
     })
   })
 
+  it('accepts a usable query response alongside an empty fault envelope', async () => {
+    const result = await transformQuickBooksListResponse(
+      Response.json({
+        Fault: { Error: [] },
+        QueryResponse: {
+          Vendor: [{ Id: '7', DisplayName: 'Sanitized Vendor' }],
+          startPosition: 1,
+          maxResults: 1,
+        },
+      }),
+      { ...authParams, startPosition: 1, maxResults: 1 },
+      'Vendor'
+    )
+
+    expect(result.output.items).toEqual([{ Id: '7', DisplayName: 'Sanitized Vendor' }])
+  })
+
   it.each([
     ['Vendor', { Id: '7', DisplayName: 'Sanitized Vendor' }],
     ['PurchaseOrder', { Id: '8', DocNumber: 'PO-SANITIZED', TotalAmt: 42 }],