fix(quickbooks): address integration review findings

Author: Bill Leoutsakos

apps/docs/content/docs/en/integrations/quickbooks.mdx

@@ -394,6 +394,7 @@ Sparse-update a Service or Non-inventory item in QuickBooks Online
 | --------- | ---- | -------- | ----------- |
 | `itemId` | string | Yes | ID of the item to update |
 | `syncToken` | string | Yes | Current item sync token |
+| `itemType` | string | Yes | Current item type: service or non_inventory |
 | `name` | string | No | Replacement item name |
 | `incomeAccountId` | string | No | Replacement income account ID |
 | `description` | string | No | Replacement sales description |

apps/sim/blocks/blocks/quickbooks.ts

@@ -356,8 +356,8 @@ export const QuickBooksBlock: BlockConfig<QuickBooksResponse> = {
         { label: 'Service', id: 'service' },
         { label: 'Non-inventory', id: 'non_inventory' },
       ],
-      condition: { field: 'operation', value: 'quickbooks_create_item' },
-      required: { field: 'operation', value: 'quickbooks_create_item' },
+      condition: { field: 'operation', value: [...ITEM_OPERATIONS] },
+      required: { field: 'operation', value: [...ITEM_OPERATIONS] },
       value: () => 'service',
     },
     {

apps/sim/lib/core/security/redaction.test.ts

@@ -198,6 +198,19 @@ describe('redactSensitiveValues', () => {
     expect(result).not.toContain('key123456')
   })
 
+  it.concurrent('should preserve workflow-state tokens in serialized JSON', () => {
+    const result = redactSensitiveValues(
+      '{"SyncToken":"3","nextPageToken":"page-2","accessToken":"secret","password":"don\'t leak"}'
+    )
+
+    expect(JSON.parse(result)).toEqual({
+      SyncToken: '3',
+      nextPageToken: 'page-2',
+      accessToken: REDACTED_MARKER,
+      password: REDACTED_MARKER,
+    })
+  })
+
   it.concurrent('should not modify safe strings', () => {
     const input = 'This is a normal string with no secrets'
     const result = redactSensitiveValues(input)

apps/sim/lib/core/security/redaction.ts

@@ -68,21 +68,11 @@ const SENSITIVE_VALUE_PATTERNS: Array<{
     pattern: /\b(sk|pk|api|key)[_-][A-Za-z0-9\-._]{20,}\b/gi,
     replacement: REDACTED_MARKER,
   },
-  // JSON-style password fields: password: "value" or password: 'value'
-  {
-    pattern: /password['":\s]*['"][^'"]+['"]/gi,
-    replacement: `password: "${REDACTED_MARKER}"`,
-  },
-  // JSON-style token fields: token: "value" or token: 'value'
-  {
-    pattern: /token['":\s]*['"][^'"]+['"]/gi,
-    replacement: `token: "${REDACTED_MARKER}"`,
-  },
-  // JSON-style api_key fields: api_key: "value" or api-key: "value"
-  {
-    pattern: /api[_-]?key['":\s]*['"][^'"]+['"]/gi,
-    replacement: `api_key: "${REDACTED_MARKER}"`,
-  },
+]
+
+const STRING_FIELD_PATTERNS = [
+  /(^|[{,\s])(["']?)([A-Za-z0-9_-]+)\2(\s*:\s*)("(?:\\.|[^"\\])*")/gm,
+  /(^|[{,\s])(["']?)([A-Za-z0-9_-]+)\2(\s*:\s*)('(?:\\.|[^'\\])*')/gm,
 ]
 
 export function isSensitiveKey(key: string): boolean {
@@ -104,6 +94,13 @@ export function redactSensitiveValues(value: string): string {
   }
 
   let result = value
+  for (const pattern of STRING_FIELD_PATTERNS) {
+    result = result.replace(pattern, (match, prefix, keyQuote, key, separator, quotedValue) =>
+      isSensitiveKey(key)
+        ? `${prefix}${keyQuote}${key}${keyQuote}${separator}${quotedValue[0]}${REDACTED_MARKER}${quotedValue[0]}`
+        : match
+    )
+  }
   for (const { pattern, replacement } of SENSITIVE_VALUE_PATTERNS) {
     result = result.replace(pattern, replacement)
   }

apps/sim/lib/oauth/oauth.ts

@@ -1016,7 +1016,8 @@ export const OAUTH_PROVIDERS: Record<string, OAuthProviderConfig> = {
     services: {
       quickbooks: {
         name: 'QuickBooks',
-        description: 'Read company, vendor, purchase order, and bill data from QuickBooks Online.',
+        description:
+          'Access company data and manage customers, vendors, and items in QuickBooks Online.',
         providerId: 'quickbooks',
         icon: QuickBooksIcon,
         baseProviderIcon: QuickBooksIcon,

apps/sim/lib/oauth/utils.ts

@@ -87,7 +87,7 @@ export const SCOPE_DESCRIPTIONS: Record<string, string> = {
   profile: 'Access profile information',
   email: 'Access email address',
   'com.intuit.quickbooks.accounting':
-    'Read accounting data from the connected QuickBooks Online company',
+    'Access and manage accounting data in the connected QuickBooks Online company',
 
   // Notion scopes
   'database.read': 'Read database',

apps/sim/tools/error-extractors.ts

@@ -19,6 +19,7 @@
  * 2. Add the ID to ErrorExtractorId constant at the bottom of this file
  */
 
+import { formatQuickBooksFaultDetail, sanitizeQuickBooksFaultData } from '@/lib/quickbooks/fault'
 import { parseGraphErrorFromData } from '@/tools/microsoft_excel/utils'
 
 export interface ErrorInfo {
@@ -378,5 +379,3 @@ export const ErrorExtractorId = {
   PLAIN_TEXT_DATA: 'plain-text-data',
   HTTP_STATUS_TEXT: 'http-status-text',
 } as const
-
-import { formatQuickBooksFaultDetail, sanitizeQuickBooksFaultData } from '@/lib/quickbooks/fault'

apps/sim/tools/quickbooks/create_item.ts

@@ -12,17 +12,13 @@ import {
   getQuickBooksToolHeaders,
   optionalQuickBooksString,
   quickBooksReference,
+  quickBooksWritableItemType,
   requiredQuickBooksString,
   transformQuickBooksMutationResponse,
   validateQuickBooksOptionalNumber,
 } from '@/tools/quickbooks/utils'
 import type { ToolConfig } from '@/tools/types'
 
-const QUICKBOOKS_ITEM_TYPES = {
-  service: 'Service',
-  non_inventory: 'NonInventory',
-} as const
-
 export const quickbooksCreateItemTool: ToolConfig<
   QuickBooksCreateItemParams,
   QuickBooksMutationResponse<QuickBooksItem>
@@ -110,8 +106,7 @@ export const quickbooksCreateItemTool: ToolConfig<
     method: 'POST',
     headers: (params) => getQuickBooksToolHeaders(params.accessToken, 'application/json'),
     body: (params) => {
-      const type = QUICKBOOKS_ITEM_TYPES[params.itemType]
-      if (!type) throw new Error(`Unsupported writable QuickBooks item type: ${params.itemType}`)
+      const type = quickBooksWritableItemType(params.itemType)
       const purchaseDescription = optionalQuickBooksString(params.purchaseDescription)
       const purchaseCost = validateQuickBooksOptionalNumber(params.purchaseCost, 'purchaseCost')
       if (

apps/sim/tools/quickbooks/create_vendor.ts

@@ -14,6 +14,7 @@ import {
   quickBooksEmailAddress,
   quickBooksPhoneNumber,
   requiredQuickBooksString,
+  sanitizeQuickBooksVendor,
   transformQuickBooksMutationResponse,
 } from '@/tools/quickbooks/utils'
 import type { ToolConfig } from '@/tools/types'
@@ -127,7 +128,11 @@ export const quickbooksCreateVendorTool: ToolConfig<
     maxResponseBytes: QUICKBOOKS_MAX_RESPONSE_BYTES,
   },
   transformResponse: (response) =>
-    transformQuickBooksMutationResponse<QuickBooksVendor>(response, 'Vendor'),
+    transformQuickBooksMutationResponse<QuickBooksVendor>(
+      response,
+      'Vendor',
+      sanitizeQuickBooksVendor
+    ),
   outputs: {
     record: {
       type: 'json',

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

@@ -368,6 +368,35 @@ describe('QuickBooks master-data reader', () => {
     expect(byIdResult.output.item).not.toHaveProperty('BankAccountNumber')
   })
 
+  it('removes vendor tax identifiers from list and by-ID output', async () => {
+    const vendor = {
+      Id: '3',
+      SyncToken: '1',
+      DisplayName: 'Sanitized Vendor',
+      TaxIdentifier: 'sensitive-tax-id',
+    }
+    const vendorParams: QuickBooksReadMasterDataParams = {
+      ...listParams,
+      recordType: 'vendor',
+    }
+
+    const listResult = await quickbooksReadMasterDataTool.transformResponse!(
+      Response.json({ QueryResponse: { Vendor: [vendor] } }),
+      vendorParams
+    )
+    expect(listResult.output.items?.[0]).toEqual({
+      Id: '3',
+      SyncToken: '1',
+      DisplayName: 'Sanitized Vendor',
+    })
+
+    const byIdResult = await quickbooksReadMasterDataTool.transformResponse!(
+      Response.json({ Vendor: vendor }),
+      { ...vendorParams, readMode: 'by_id', recordId: '3' }
+    )
+    expect(byIdResult.output.item).not.toHaveProperty('TaxIdentifier')
+  })
+
   it('rejects missing IDs, unknown types and unknown modes before a request', () => {
     const requestUrl = quickbooksReadMasterDataTool.request.url as (
       params: QuickBooksReadMasterDataParams
@@ -504,6 +533,28 @@ describe('QuickBooks customer and vendor mutations', () => {
     expect(() => parseQuickBooksAddress('{"city":123}', 'billingAddress')).toThrow(
       'must be a string'
     )
+    expect(() => parseQuickBooksAddress('{}', 'billingAddress')).toThrow('at least one')
+  })
+
+  it('removes vendor tax identifiers from mutation output', async () => {
+    const response = {
+      Vendor: {
+        Id: '21',
+        SyncToken: '0',
+        DisplayName: 'Sanitized Vendor',
+        TaxIdentifier: 'sensitive-tax-id',
+      },
+      time: 'test-time',
+    }
+
+    for (const tool of [quickbooksCreateVendorTool, quickbooksUpdateVendorTool]) {
+      const result = await tool.transformResponse!(Response.json(response))
+      expect(result.output.record).toEqual({
+        Id: '21',
+        SyncToken: '0',
+        DisplayName: 'Sanitized Vendor',
+      })
+    }
   })
 })
 
@@ -567,6 +618,7 @@ describe('QuickBooks item mutations', () => {
       ...authParams,
       itemId: '44',
       syncToken: '2',
+      itemType: 'service',
       activeStatus: 'unchanged',
       unitPrice: 15.75,
       expenseAccountId: '80',
@@ -578,7 +630,13 @@ describe('QuickBooks item mutations', () => {
       UnitPrice: 15.75,
       ExpenseAccountRef: { value: '80' },
     })
-    expect(quickbooksUpdateItemTool.params).not.toHaveProperty('itemType')
+    expect(quickbooksUpdateItemTool.params.itemType).toMatchObject({ required: true })
+    expect(() =>
+      quickbooksUpdateItemTool.request.body!({
+        ...params,
+        itemType: 'inventory' as QuickBooksUpdateItemParams['itemType'],
+      })
+    ).toThrow('Unsupported writable')
   })
 
   it('returns the native mutation record and convenient identifiers', async () => {
@@ -779,7 +837,7 @@ describe('QuickBooks tool and block boundaries', () => {
     })
     expect(subBlocks.itemType.condition).toEqual({
       field: 'operation',
-      value: 'quickbooks_create_item',
+      value: ['quickbooks_create_item', 'quickbooks_update_item'],
     })
   })
 })