chore(api-key): remove legacy scan+decrypt auth fallback (#4876)

apps/sim/lib/api-key/auth.test.ts

@@ -10,12 +10,7 @@
  */
 
 import { randomBytes } from 'crypto'
-import {
-  createEncryptedApiKey,
-  createLegacyApiKey,
-  expectApiKeyInvalid,
-  expectApiKeyValid,
-} from '@sim/testing'
+import { createEncryptedApiKey, createLegacyApiKey } from '@sim/testing'
 import { describe, expect, it, vi } from 'vitest'
 
 const cryptoMock = vi.hoisted(() => ({
@@ -40,7 +35,6 @@ const cryptoMock = vi.hoisted(() => ({
 vi.mock('@/lib/api-key/crypto', () => cryptoMock)
 
 import {
-  authenticateApiKey,
   formatApiKeyForDisplay,
   getApiKeyLast4,
   isEncryptedKey,
@@ -113,110 +107,6 @@ describe('isLegacyApiKeyFormat', () => {
   })
 })
 
-describe('authenticateApiKey', () => {
-  describe('encrypted format key (sk-sim-) against encrypted storage', () => {
-    it('should authenticate matching encrypted key', async () => {
-      const plainKey = 'sk-sim-test-key-123'
-      const encryptedStorage = `mock-iv:${Buffer.from(plainKey).toString('hex')}:mock-tag`
-
-      const result = await authenticateApiKey(plainKey, encryptedStorage)
-      expectApiKeyValid(result)
-    })
-
-    it('should reject non-matching encrypted key', async () => {
-      const inputKey = 'sk-sim-test-key-123'
-      const differentKey = 'sk-sim-different-key'
-      const encryptedStorage = `mock-iv:${Buffer.from(differentKey).toString('hex')}:mock-tag`
-
-      const result = await authenticateApiKey(inputKey, encryptedStorage)
-      expectApiKeyInvalid(result)
-    })
-
-    it('should reject encrypted format key against plain text storage', async () => {
-      const inputKey = 'sk-sim-test-key-123'
-      const plainStorage = inputKey // Same key but stored as plain text
-
-      const result = await authenticateApiKey(inputKey, plainStorage)
-      expectApiKeyInvalid(result)
-    })
-  })
-
-  describe('legacy format key (sim_) against storage', () => {
-    it('should authenticate legacy key against encrypted storage', async () => {
-      const plainKey = 'sim_legacy-test-key'
-      const encryptedStorage = `mock-iv:${Buffer.from(plainKey).toString('hex')}:mock-tag`
-
-      const result = await authenticateApiKey(plainKey, encryptedStorage)
-      expectApiKeyValid(result)
-    })
-
-    it('should authenticate legacy key against plain text storage', async () => {
-      const plainKey = 'sim_legacy-test-key'
-      const plainStorage = plainKey
-
-      const result = await authenticateApiKey(plainKey, plainStorage)
-      expectApiKeyValid(result)
-    })
-
-    it('should reject non-matching legacy key', async () => {
-      const inputKey = 'sim_test-key'
-      const storedKey = 'sim_different-key'
-
-      const result = await authenticateApiKey(inputKey, storedKey)
-      expectApiKeyInvalid(result)
-    })
-  })
-
-  describe('unrecognized format keys', () => {
-    it('should authenticate unrecognized key against plain text match', async () => {
-      const plainKey = 'custom-api-key-format'
-      const plainStorage = plainKey
-
-      const result = await authenticateApiKey(plainKey, plainStorage)
-      expectApiKeyValid(result)
-    })
-
-    it('should authenticate unrecognized key against encrypted storage', async () => {
-      const plainKey = 'custom-api-key-format'
-      const encryptedStorage = `mock-iv:${Buffer.from(plainKey).toString('hex')}:mock-tag`
-
-      const result = await authenticateApiKey(plainKey, encryptedStorage)
-      expectApiKeyValid(result)
-    })
-
-    it('should reject non-matching unrecognized key', async () => {
-      const inputKey = 'custom-key-1'
-      const storedKey = 'custom-key-2'
-
-      const result = await authenticateApiKey(inputKey, storedKey)
-      expectApiKeyInvalid(result)
-    })
-  })
-
-  describe('edge cases', () => {
-    it('should reject empty input key', async () => {
-      const result = await authenticateApiKey('', 'sim_stored-key')
-      expectApiKeyInvalid(result)
-    })
-
-    it('should reject empty stored key', async () => {
-      const result = await authenticateApiKey('sim_input-key', '')
-      expectApiKeyInvalid(result)
-    })
-
-    it('should handle keys with special characters', async () => {
-      const specialKey = 'sim_key-with-special+chars/and=more'
-      const result = await authenticateApiKey(specialKey, specialKey)
-      expectApiKeyValid(result)
-    })
-
-    it('should be case-sensitive', async () => {
-      const result = await authenticateApiKey('sim_TestKey', 'sim_testkey')
-      expectApiKeyInvalid(result)
-    })
-  })
-})
-
 describe('isValidApiKeyFormat', () => {
   it('should accept valid length keys', () => {
     expect(isValidApiKeyFormat(`sim_${'a'.repeat(20)}`)).toBe(true)
@@ -330,58 +220,3 @@ describe('generateEncryptedApiKey', () => {
     expect(key.length).toBeLessThan(100)
   })
 })
-
-describe('API key lifecycle', () => {
-  it('should authenticate newly generated legacy key against itself (plain storage)', async () => {
-    const key = generateApiKey()
-    const result = await authenticateApiKey(key, key)
-    expectApiKeyValid(result)
-  })
-
-  it('should authenticate newly generated encrypted key against encrypted storage', async () => {
-    const key = generateEncryptedApiKey()
-    const encryptedStorage = `mock-iv:${Buffer.from(key).toString('hex')}:mock-tag`
-    const result = await authenticateApiKey(key, encryptedStorage)
-    expectApiKeyValid(result)
-  })
-
-  it('should reject key if storage is tampered', async () => {
-    const key = generateApiKey()
-    const lastChar = key.slice(-1)
-    // Ensure tampered character is different from original (handles edge case where key ends in 'X')
-    const tamperedChar = lastChar === 'X' ? 'Y' : 'X'
-    const tamperedStorage = `${key.slice(0, -1)}${tamperedChar}`
-    const result = await authenticateApiKey(key, tamperedStorage)
-    expectApiKeyInvalid(result)
-  })
-})
-
-describe('security considerations', () => {
-  it('should not accept partial key matches', async () => {
-    const fullKey = 'sim_abcdefghijklmnop'
-    const partialKey = 'sim_abcdefgh'
-    const result = await authenticateApiKey(partialKey, fullKey)
-    expectApiKeyInvalid(result)
-  })
-
-  it('should not accept keys with extra characters', async () => {
-    const storedKey = 'sim_abcdefgh'
-    const extendedKey = 'sim_abcdefghXXX'
-    const result = await authenticateApiKey(extendedKey, storedKey)
-    expectApiKeyInvalid(result)
-  })
-
-  it('should not accept key with whitespace variations', async () => {
-    const key = 'sim_testkey'
-    const keyWithSpace = ' sim_testkey'
-    const result = await authenticateApiKey(keyWithSpace, key)
-    expectApiKeyInvalid(result)
-  })
-
-  it('should not accept key with trailing whitespace', async () => {
-    const key = 'sim_testkey'
-    const keyWithTrailing = 'sim_testkey '
-    const result = await authenticateApiKey(keyWithTrailing, key)
-    expectApiKeyInvalid(result)
-  })
-})

apps/sim/lib/api-key/auth.ts

@@ -1,7 +1,6 @@
 import { db } from '@sim/db'
 import { apiKey } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
-import { safeCompare } from '@sim/security/compare'
 import { generateShortId } from '@sim/utils/id'
 import { and, eq } from 'drizzle-orm'
 import {
@@ -32,61 +31,6 @@ export function isEncryptedKey(storedKey: string): boolean {
   return storedKey.includes(':') && storedKey.split(':').length === 3
 }
 
-/**
- * Authenticates an API key against a stored key, supporting both legacy and new encrypted formats
- * @param inputKey - The API key provided by the client
- * @param storedKey - The key stored in the database (may be plain text or encrypted)
- * @returns Promise<boolean> - true if the key is valid
- */
-export async function authenticateApiKey(inputKey: string, storedKey: string): Promise<boolean> {
-  try {
-    // If input key has new encrypted prefix (sk-sim-), only check against encrypted storage
-    if (isEncryptedApiKeyFormat(inputKey)) {
-      if (isEncryptedKey(storedKey)) {
-        try {
-          const { decrypted } = await decryptApiKey(storedKey)
-          return safeCompare(inputKey, decrypted)
-        } catch (decryptError) {
-          logger.error('Failed to decrypt stored API key:', { error: decryptError })
-          return false
-        }
-      }
-      // New format keys should never match against plain text storage
-      return false
-    }
-
-    // If input key has legacy prefix (sim_), check both encrypted and plain text
-    if (isLegacyApiKeyFormat(inputKey)) {
-      if (isEncryptedKey(storedKey)) {
-        try {
-          const { decrypted } = await decryptApiKey(storedKey)
-          return safeCompare(inputKey, decrypted)
-        } catch (decryptError) {
-          logger.error('Failed to decrypt stored API key:', { error: decryptError })
-          // Fall through to plain text comparison if decryption fails
-        }
-      }
-      // Legacy format can match against plain text storage
-      return safeCompare(inputKey, storedKey)
-    }
-
-    // If no recognized prefix, fall back to original behavior
-    if (isEncryptedKey(storedKey)) {
-      try {
-        const { decrypted } = await decryptApiKey(storedKey)
-        return safeCompare(inputKey, decrypted)
-      } catch (decryptError) {
-        logger.error('Failed to decrypt stored API key:', { error: decryptError })
-      }
-    }
-
-    return safeCompare(inputKey, storedKey)
-  } catch (error) {
-    logger.error('API key authentication error:', { error })
-    return false
-  }
-}
-
 /**
  * Encrypts an API key for secure storage
  * @param apiKey - The plain text API key to encrypt

apps/sim/lib/api-key/service.test.ts

@@ -1,10 +1,9 @@
 /**
  * Tests for authenticateApiKeyFromHeader.
  *
- * The path was rewritten to look up rows by the SHA-256 hash of the incoming
- * API key. A fallback loop — full scan + decrypt — is preserved while the
- * `key_hash` backfill runs, and emits a warn log whenever it actually matches
- * a row so we can tell when it's safe to delete.
+ * Authentication looks up a single row by the SHA-256 hash of the incoming
+ * API key and applies the scope / expiry / permission gates. Any miss — no
+ * matching hash or a failed gate — returns an invalid result.
  *
  * @vitest-environment node
  */
@@ -36,14 +35,6 @@ vi.mock('@sim/logger', () => ({
   getRequestContext: vi.fn(() => undefined),
 }))
 
-const { mockAuthenticateApiKey } = vi.hoisted(() => ({
-  mockAuthenticateApiKey: vi.fn(),
-}))
-
-vi.mock('@/lib/api-key/auth', () => ({
-  authenticateApiKey: mockAuthenticateApiKey,
-}))
-
 const { mockGetWorkspaceBillingSettings } = vi.hoisted(() => ({
   mockGetWorkspaceBillingSettings: vi.fn(),
 }))
@@ -63,15 +54,12 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({
 import { hashApiKey } from '@/lib/api-key/crypto'
 import { authenticateApiKeyFromHeader } from '@/lib/api-key/service'
 
-const warnSpy = serviceLogger.warn
-
 function personalKeyRecord(overrides: Partial<Record<string, unknown>> = {}) {
   return {
     id: 'key-1',
     userId: 'user-1',
     workspaceId: null as string | null,
     type: 'personal',
-    key: 'encrypted:stored:value',
     expiresAt: null as Date | null,
     ...overrides,
   }
@@ -80,7 +68,6 @@ function personalKeyRecord(overrides: Partial<Record<string, unknown>> = {}) {
 describe('authenticateApiKeyFromHeader', () => {
   beforeEach(() => {
     vi.clearAllMocks()
-    mockAuthenticateApiKey.mockReset()
     mockGetWorkspaceBillingSettings.mockReset()
     mockGetUserEntityPermissions.mockReset()
   })
@@ -91,7 +78,7 @@ describe('authenticateApiKeyFromHeader', () => {
     expect(dbChainMockFns.where).not.toHaveBeenCalled()
   })
 
-  it('resolves on the fast path when the hash lookup finds a row', async () => {
+  it('resolves when the hash lookup finds a row', async () => {
     const record = personalKeyRecord()
     dbChainMockFns.where.mockResolvedValueOnce([record])
 
@@ -107,8 +94,6 @@ describe('authenticateApiKeyFromHeader', () => {
       workspaceId: undefined,
     })
     expect(dbChainMockFns.where).toHaveBeenCalledTimes(1)
-    expect(mockAuthenticateApiKey).not.toHaveBeenCalled()
-    expect(warnSpy).not.toHaveBeenCalled()
   })
 
   it('returns invalid when the hash lookup finds a row that fails scope checks', async () => {
@@ -121,63 +106,20 @@ describe('authenticateApiKeyFromHeader', () => {
 
     expect(result).toEqual({ success: false, error: 'Invalid API key' })
     expect(dbChainMockFns.where).toHaveBeenCalledTimes(1)
-    expect(mockAuthenticateApiKey).not.toHaveBeenCalled()
   })
 
-  it('falls back to the decrypt loop when no row matches the hash, and warns on success', async () => {
-    const record = personalKeyRecord()
-    dbChainMockFns.where.mockResolvedValueOnce([]).mockResolvedValueOnce([record])
-    mockAuthenticateApiKey.mockResolvedValueOnce(true)
-
-    const result = await authenticateApiKeyFromHeader('sk-sim-plain-key', {
-      userId: 'user-1',
-    })
-
-    expect(result).toEqual({
-      success: true,
-      userId: 'user-1',
-      keyId: 'key-1',
-      keyType: 'personal',
-      workspaceId: undefined,
-    })
-    expect(dbChainMockFns.where).toHaveBeenCalledTimes(2)
-    expect(mockAuthenticateApiKey).toHaveBeenCalledWith(
-      'sk-sim-plain-key',
-      'encrypted:stored:value'
-    )
-    expect(warnSpy).toHaveBeenCalledWith('API key matched via fallback decrypt loop', {
-      keyId: 'key-1',
-    })
-  })
-
-  it('returns invalid when the hash lookup misses and the fallback scan also misses', async () => {
-    dbChainMockFns.where.mockResolvedValueOnce([]).mockResolvedValueOnce([])
+  it('returns invalid when the hash lookup finds no row', async () => {
+    dbChainMockFns.where.mockResolvedValueOnce([])
 
     const result = await authenticateApiKeyFromHeader('sk-sim-plain-key', {
       userId: 'user-1',
     })
 
     expect(result).toEqual({ success: false, error: 'Invalid API key' })
-    expect(dbChainMockFns.where).toHaveBeenCalledTimes(2)
-    expect(mockAuthenticateApiKey).not.toHaveBeenCalled()
-    expect(warnSpy).not.toHaveBeenCalled()
+    expect(dbChainMockFns.where).toHaveBeenCalledTimes(1)
   })
 
-  it('returns invalid when the hash lookup misses and every fallback candidate fails decrypt comparison', async () => {
-    const record = personalKeyRecord()
-    dbChainMockFns.where.mockResolvedValueOnce([]).mockResolvedValueOnce([record])
-    mockAuthenticateApiKey.mockResolvedValueOnce(false)
-
-    const result = await authenticateApiKeyFromHeader('sk-sim-plain-key', {
-      userId: 'user-1',
-    })
-
-    expect(result).toEqual({ success: false, error: 'Invalid API key' })
-    expect(mockAuthenticateApiKey).toHaveBeenCalledTimes(1)
-    expect(warnSpy).not.toHaveBeenCalled()
-  })
-
-  it('queries by the sha256 hash of the incoming header on the fast path', async () => {
+  it('queries by the sha256 hash of the incoming header', async () => {
     dbChainMockFns.where.mockResolvedValueOnce([personalKeyRecord()])
 
     await authenticateApiKeyFromHeader('sk-sim-plain-key', { userId: 'user-1' })

apps/sim/lib/api-key/service.ts

@@ -2,7 +2,6 @@ import { db } from '@sim/db'
 import { apiKey as apiKeyTable } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { and, eq } from 'drizzle-orm'
-import { authenticateApiKey } from '@/lib/api-key/auth'
 import { hashApiKey } from '@/lib/api-key/crypto'
 import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
 import { getWorkspaceBillingSettings, type WorkspaceBillingSettings } from '@/lib/workspaces/utils'
@@ -53,11 +52,9 @@ interface HashCandidate {
 /**
  * Authenticate an API key from header with flexible filtering options.
  *
- * Tries the hash lookup first. If that misses (legacy row not yet backfilled,
- * or writer missed the hash column), falls back to the original scan+decrypt
- * loop. The fallback emits a warn log whenever it actually matches a row so
- * we can confirm the fast path is covering 100% of traffic before deleting
- * the fallback block below in a follow-up PR.
+ * Looks up a single row by `sha256(apiKeyHeader)` and applies the scope /
+ * expiry / permission gates. Any miss — no matching hash or a failed gate —
+ * returns `INVALID`.
  */
 export async function authenticateApiKeyFromHeader(
   apiKeyHeader: string,
@@ -77,180 +74,62 @@ export async function authenticateApiKeyFromHeader(
       }
     }
 
-    const hashResult = await authenticateApiKeyByHash(apiKeyHeader, options, workspaceSettings)
-    if (hashResult !== null) return hashResult
-
-    // LEGACY FALLBACK — delete once `logger.warn('API key matched via fallback
-    // decrypt loop', ...)` count stays at zero in prod. The block below is the
-    // pre-hash-lookup implementation, preserved verbatim as a safety net while
-    // the `key_hash` backfill rolls out.
-    let query = db
+    const keyHash = hashApiKey(apiKeyHeader)
+    const rows: HashCandidate[] = await db
       .select({
         id: apiKeyTable.id,
         userId: apiKeyTable.userId,
         workspaceId: apiKeyTable.workspaceId,
         type: apiKeyTable.type,
-        key: apiKeyTable.key,
         expiresAt: apiKeyTable.expiresAt,
       })
       .from(apiKeyTable)
+      .where(eq(apiKeyTable.keyHash, keyHash))
 
-    const conditions = []
+    if (rows.length === 0) return INVALID
 
-    if (options.userId) {
-      conditions.push(eq(apiKeyTable.userId, options.userId))
+    const record = rows[0]
+    const keyType = record.type as 'personal' | 'workspace'
+
+    if (options.userId && record.userId !== options.userId) return INVALID
+    if (options.keyTypes?.length && !options.keyTypes.includes(keyType)) return INVALID
+    if (record.expiresAt && record.expiresAt < new Date()) return INVALID
+
+    if (
+      options.workspaceId &&
+      keyType === 'workspace' &&
+      record.workspaceId !== options.workspaceId
+    ) {
+      return INVALID
     }
 
-    if (options.keyTypes?.length) {
-      if (options.keyTypes.length === 1) {
-        conditions.push(eq(apiKeyTable.type, options.keyTypes[0]))
-      }
+    if (options.workspaceId && keyType === 'personal') {
+      if (!workspaceSettings?.allowPersonalApiKeys) return INVALID
+      if (!record.userId) return INVALID
+
+      const permission = await getUserEntityPermissions(
+        record.userId,
+        'workspace',
+        options.workspaceId
+      )
+      if (permission === null) return INVALID
     }
 
-    if (conditions.length > 0) {
-      query = query.where(and(...conditions)) as any
+    logger.debug('API key matched via hash lookup', { keyId: record.id, keyType })
+
+    return {
+      success: true,
+      userId: record.userId,
+      keyId: record.id,
+      keyType,
+      workspaceId: record.workspaceId || options.workspaceId || undefined,
     }
-
-    const keyRecords = await query
-
-    const filteredRecords = keyRecords.filter((record) => {
-      const keyType = record.type as 'personal' | 'workspace'
-
-      if (options.keyTypes?.length && !options.keyTypes.includes(keyType)) {
-        return false
-      }
-
-      if (options.workspaceId) {
-        if (keyType === 'workspace') {
-          return record.workspaceId === options.workspaceId
-        }
-
-        if (keyType === 'personal') {
-          return workspaceSettings?.allowPersonalApiKeys ?? false
-        }
-      }
-
-      return true
-    })
-
-    const permissionCache = new Map<string, boolean>()
-
-    for (const storedKey of filteredRecords) {
-      if (storedKey.expiresAt && storedKey.expiresAt < new Date()) {
-        continue
-      }
-
-      if (options.workspaceId && (storedKey.type as 'personal' | 'workspace') === 'personal') {
-        if (!workspaceSettings?.allowPersonalApiKeys) {
-          continue
-        }
-
-        if (!storedKey.userId) {
-          continue
-        }
-
-        if (!permissionCache.has(storedKey.userId)) {
-          const permission = await getUserEntityPermissions(
-            storedKey.userId,
-            'workspace',
-            options.workspaceId
-          )
-          permissionCache.set(storedKey.userId, permission !== null)
-        }
-
-        if (!permissionCache.get(storedKey.userId)) {
-          continue
-        }
-      }
-
-      try {
-        const isValid = await authenticateApiKey(apiKeyHeader, storedKey.key)
-        if (isValid) {
-          logger.warn('API key matched via fallback decrypt loop', { keyId: storedKey.id })
-          return {
-            success: true,
-            userId: storedKey.userId,
-            keyId: storedKey.id,
-            keyType: storedKey.type as 'personal' | 'workspace',
-            workspaceId: storedKey.workspaceId || options.workspaceId || undefined,
-          }
-        }
-      } catch (error) {
-        logger.error('Error authenticating API key:', error)
-      }
-    }
-
-    return INVALID
   } catch (error) {
     logger.error('API key authentication error:', error)
     return { success: false, error: 'Authentication failed' }
   }
 }
 
-/**
- * Fast path: look up a single row by `sha256(apiKeyHeader)` and apply the
- * scope / expiry / permission gates. Returns `null` when no row matched the
- * hash (caller should fall through to the legacy scan+decrypt loop). A hash
- * hit that fails a gate returns a concrete `INVALID` — the key definitely
- * belongs to that row, it's just not authorized in this scope.
- */
-async function authenticateApiKeyByHash(
-  apiKeyHeader: string,
-  options: ApiKeyAuthOptions,
-  workspaceSettings: WorkspaceBillingSettings | null
-): Promise<ApiKeyAuthResult | null> {
-  const keyHash = hashApiKey(apiKeyHeader)
-  const rows: HashCandidate[] = await db
-    .select({
-      id: apiKeyTable.id,
-      userId: apiKeyTable.userId,
-      workspaceId: apiKeyTable.workspaceId,
-      type: apiKeyTable.type,
-      expiresAt: apiKeyTable.expiresAt,
-    })
-    .from(apiKeyTable)
-    .where(eq(apiKeyTable.keyHash, keyHash))
-
-  if (rows.length === 0) return null
-
-  const record = rows[0]
-  const keyType = record.type as 'personal' | 'workspace'
-
-  if (options.userId && record.userId !== options.userId) return INVALID
-  if (options.keyTypes?.length && !options.keyTypes.includes(keyType)) return INVALID
-  if (record.expiresAt && record.expiresAt < new Date()) return INVALID
-
-  if (
-    options.workspaceId &&
-    keyType === 'workspace' &&
-    record.workspaceId !== options.workspaceId
-  ) {
-    return INVALID
-  }
-
-  if (options.workspaceId && keyType === 'personal') {
-    if (!workspaceSettings?.allowPersonalApiKeys) return INVALID
-    if (!record.userId) return INVALID
-
-    const permission = await getUserEntityPermissions(
-      record.userId,
-      'workspace',
-      options.workspaceId
-    )
-    if (permission === null) return INVALID
-  }
-
-  logger.debug('API key matched via hash lookup', { keyId: record.id, keyType })
-
-  return {
-    success: true,
-    userId: record.userId,
-    keyId: record.id,
-    keyType,
-    workspaceId: record.workspaceId || options.workspaceId || undefined,
-  }
-}
-
 /**
  * Update the last used timestamp for an API key
  */