fix(test,api-keys): Unit Tests job runs jest; near-unique API key prefixes (D-0100, D-0527)

.github/workflows/test.yml

@@ -76,8 +76,10 @@ jobs:
       - name: Install dependencies
         run: pnpm install --frozen-lockfile
 
+      # Not `pnpm test -- <flags>`: pnpm forwards the literal `--`, so jest
+      # reads the flags as path patterns, matches nothing and exits 0 (D-0100).
       - name: Run Jest tests
-        run: pnpm test -- --coverage --ci --passWithNoTests
+        run: pnpm run test:ci
 
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v4

jest.config.js

@@ -64,6 +64,9 @@ const customJestConfig = {
   // Allow next/jest SWC to transpile ESM-only unified/rehype packages
   transformIgnorePatterns: [`/node_modules/(?!(${esmPackages.join('|')})/)`],
   testMatch: ['**/__tests__/**/*.test.ts?(x)', '**/*.test.ts?(x)'],
+  // Ticket worktrees live under .claude/worktrees; without this a run in the
+  // main checkout also runs every worktree's copy of the suite.
+  testPathIgnorePatterns: ['<rootDir>/.claude/'],
   collectCoverageFrom: [
     'src/**/*.{ts,tsx}',
     '!src/**/*.d.ts',

src/lib/api-keys/README.md

@@ -18,14 +18,18 @@ Real implementation of API key management with cryptographic security, database-
 ## API Key Format
 
 ```
-nself_ab_1234567890123456789012345678901234567890
-└─┬─┘ └┬┘ └────────────────┬───────────────────┘
-  │    │                   │
-  │    │                   └─ Secret (40 chars, base62)
-  │    └───────────────────── Random suffix (2 chars)
-  └────────────────────────── Prefix (nself)
+nself_k3x7qa2mzp_123456789012345678901234567890
+└─┬─┘ └───┬────┘ └─────────────┬──────────────┘
+  │       │                    │
+  │       │                    └─ Secret (30 chars, base62)
+  │       └────────────────────── Lookup prefix (10 chars, base32 a-z2-7)
+  └────────────────────────────── Prefix (nself)
 ```
 
+The lookup prefix only narrows the search: validation checks the key against
+every stored record with that prefix by bcrypt hash. Keys issued before the
+10-char prefix carry a 2-letter suffix (`nself_ab_...`) and keep working.
+
 ## Basic Usage
 
 ### Creating an API Key

src/lib/api-keys/__tests__/api-keys.test.ts

@@ -4,6 +4,8 @@
  */
 
 import { getDatabase, initDatabase } from '@/lib/database'
+import type { ApiKey } from '@/types/api-key'
+import bcrypt from 'bcryptjs'
 import * as apiKeys from '../index'
 
 // Ensure database is initialized before tests
@@ -28,9 +30,10 @@ describe('API Key Generation', () => {
     const prefix1 = apiKeys.generateKeyPrefix()
     const prefix2 = apiKeys.generateKeyPrefix()
 
-    expect(prefix1).toMatch(/^nself_[a-z]{2}$/)
-    expect(prefix2).toMatch(/^nself_[a-z]{2}$/)
-    // Low probability of collision but technically possible
+    expect(prefix1).toMatch(/^nself_[a-z2-7]{10}$/)
+    expect(prefix2).toMatch(/^nself_[a-z2-7]{10}$/)
+    // 50 random bits: a collision here means the generator is broken
+    expect(prefix1).not.toBe(prefix2)
   })
 
   test('creates cryptographically secure API keys', async () => {
@@ -42,8 +45,8 @@ describe('API Key Generation', () => {
 
     expect(result.key).toBeDefined()
     expect(result.secretKey).toBeDefined()
-    expect(result.key.keyPrefix).toMatch(/^nself_[a-z]{2}$/)
-    expect(result.secretKey).toMatch(/^nself_[a-z]{2}_[A-Za-z0-9]{30}$/)
+    expect(result.key.keyPrefix).toMatch(/^nself_[a-z2-7]{10}$/)
+    expect(result.secretKey).toMatch(/^nself_[a-z2-7]{10}_[A-Za-z0-9]{30}$/)
     expect(result.key.keyHash).toBeDefined()
     expect(result.key.keyHash).toMatch(/^\$2[aby]\$/) // bcrypt hash pattern
   })
@@ -135,6 +138,91 @@ describe('API Key Validation', () => {
   })
 })
 
+// D-0527: the prefix is only a lookup hint. Records that share it (legacy
+// 2-letter prefixes, or any future collision) must each validate by hash.
+describe('API Key Prefix Collisions', () => {
+  // Synthetic test-only secrets, never real keys
+  const FAKE_SECRET_A = 'FAKEtestSECRETnotArealKEYaaaaa'
+  const FAKE_SECRET_B = 'FAKEtestSECRETnotArealKEYbbbbb'
+  const FAKE_SECRET_C = 'FAKEtestSECRETnotArealKEYccccc'
+
+  async function insertKeyWithSecret(
+    template: ApiKey,
+    id: string,
+    secretKey: string,
+    status: ApiKey['status'] = 'active'
+  ): Promise<void> {
+    const keyPrefix = secretKey.split('_').slice(0, 2).join('_')
+    const keyHash = await bcrypt.hash(secretKey, 4)
+    // Copy only data fields; LokiJS metadata ($loki, meta) must not be reused
+    const {
+      $loki: _loki,
+      meta: _meta,
+      ...fields
+    } = template as ApiKey & {
+      $loki?: number
+      meta?: unknown
+    }
+    getDatabase()
+      ?.getCollection('apiKeys')
+      .insert({ ...fields, id, keyPrefix, keyHash, status })
+  }
+
+  test('two keys with a forced shared prefix both validate', async () => {
+    const created = await apiKeys.apiKeysApi.create({ name: 'Collision A', scope: 'read' })
+    const prefix = created.key.keyPrefix
+    const secretB = `${prefix}_${FAKE_SECRET_B}`
+    await insertKeyWithSecret(created.key, 'key_collision_b', secretB)
+
+    const collection = getDatabase()?.getCollection('apiKeys')
+    expect(collection?.find({ keyPrefix: prefix })).toHaveLength(2)
+
+    const first = await apiKeys.validateApiKey(created.secretKey)
+    expect(first.valid).toBe(true)
+    expect(first.key?.id).toBe(created.key.id)
+
+    const second = await apiKeys.validateApiKey(secretB)
+    expect(second.valid).toBe(true)
+    expect(second.key?.id).toBe('key_collision_b')
+  })
+
+  test('a revoked key sharing the prefix does not block an active one', async () => {
+    const created = await apiKeys.apiKeysApi.create({ name: 'Collision tmpl', scope: 'read' })
+    const revokedSecret = `nself_ab_${FAKE_SECRET_A}`
+    const activeSecret = `nself_ab_${FAKE_SECRET_B}`
+    await insertKeyWithSecret(created.key, 'key_legacy_revoked', revokedSecret, 'revoked')
+    await insertKeyWithSecret(created.key, 'key_legacy_active', activeSecret)
+
+    const active = await apiKeys.validateApiKey(activeSecret)
+    expect(active.valid).toBe(true)
+    expect(active.key?.id).toBe('key_legacy_active')
+
+    const revoked = await apiKeys.validateApiKey(revokedSecret)
+    expect(revoked.valid).toBe(false)
+    expect(revoked.error).toBe('API key has been revoked')
+  })
+
+  test('still accepts an existing short-prefix (legacy) key', async () => {
+    const created = await apiKeys.apiKeysApi.create({ name: 'Legacy tmpl', scope: 'read' })
+    const legacySecret = `nself_zq_${FAKE_SECRET_A}`
+    await insertKeyWithSecret(created.key, 'key_legacy_short', legacySecret)
+
+    const result = await apiKeys.validateApiKey(legacySecret)
+    expect(result.valid).toBe(true)
+    expect(result.key?.id).toBe('key_legacy_short')
+  })
+
+  test('rejects a wrong secret under a shared prefix', async () => {
+    const created = await apiKeys.apiKeysApi.create({ name: 'Wrong tmpl', scope: 'read' })
+    await insertKeyWithSecret(created.key, 'key_legacy_one', `nself_zq_${FAKE_SECRET_A}`)
+    await insertKeyWithSecret(created.key, 'key_legacy_two', `nself_zq_${FAKE_SECRET_B}`)
+
+    const result = await apiKeys.validateApiKey(`nself_zq_${FAKE_SECRET_C}`)
+    expect(result.valid).toBe(false)
+    expect(result.error).toBe('Invalid API key')
+  })
+})
+
 describe('API Key Management', () => {
   test('retrieves all API keys', async () => {
     await apiKeys.apiKeysApi.create({

src/lib/api-keys/index.ts

@@ -92,9 +92,8 @@ async function initApiKeyCollections(): Promise<void> {
 /**
  * Generate a cryptographically secure API key
  * Format: prefix_secret
- * - prefix: 8 chars for identification (nself_XX)
- * - secret: 40 chars cryptographically random
- * Total: ~48 characters
+ * - prefix: nself_ + 10 random base32 chars (see generateKeyPrefix)
+ * - secret: 30 chars cryptographically random (base62)
  */
 function generateSecureApiKey(prefix: string): string {
   // Generate 30 bytes of random data (40 chars in base62)
@@ -115,16 +114,21 @@ function generateSecureApiKey(prefix: string): string {
   return `${prefix}_${secret}`
 }
 
+const KEY_PREFIX_LENGTH = 10
+
 /**
- * Generate an 8-character prefix for API keys
- * Format: nself_XX where XX are random lowercase letters
+ * Generate a near-unique lookup prefix for API keys
+ * Format: nself_ + 10 random lowercase base32 chars (a-z, 2-7), 50 bits.
+ * Keys created before this format carry nself_ + 2 letters; validateApiKey
+ * still accepts them because it matches every record with the prefix by hash.
  */
 export function generateKeyPrefix(): string {
-  const chars = 'abcdefghijklmnopqrstuvwxyz'
-  const randomBytes = crypto.randomBytes(2)
+  const chars = 'abcdefghijklmnopqrstuvwxyz234567'
+  const randomBytes = crypto.randomBytes(KEY_PREFIX_LENGTH)
 
   let suffix = ''
-  for (let i = 0; i < 2; i++) {
+  for (let i = 0; i < KEY_PREFIX_LENGTH; i++) {
+    // 256 is a multiple of 32, so the modulo is unbiased
     suffix += chars[(randomBytes[i] ?? 0) % chars.length]
   }
 
@@ -459,16 +463,22 @@ export async function validateApiKey(
 
   const prefix = `${parts[0]}_${parts[1]}`
 
-  // Find key by prefix
-  const apiKey = apiKeysCollection?.findOne({ keyPrefix: prefix })
+  // The prefix is a lookup hint, not an identity: several records can share
+  // it (legacy 2-letter prefixes collide often), so match each by hash.
+  const candidates = apiKeysCollection?.find({ keyPrefix: prefix }) || []
 
-  if (!apiKey) {
+  if (candidates.length === 0) {
     return { valid: false, error: 'API key not found' }
   }
 
-  // Verify the hash
-  const isValid = await verifyApiKey(key, apiKey.keyHash)
-  if (!isValid) {
+  let apiKey: ApiKey | undefined
+  for (const candidate of candidates) {
+    if (await verifyApiKey(key, candidate.keyHash)) {
+      apiKey = candidate
+      break
+    }
+  }
+  if (!apiKey) {
     return { valid: false, error: 'Invalid API key' }
   }