feat: multiple named API keys in /settings/secrets

[stephengpope]

Closes #197

Summary

• DB layer (lib/db/api-keys.js): In-memory cache changed from a single entry to a Map<keyHash, {id, name}>. createApiKeyRecord(createdBy, name) accepts a name and no longer deletes existing keys. getApiKeys() returns an array of all keys. deleteApiKeyById(id) deletes by UUID. verifyApiKey iterates the Map with per-entry timing-safe comparison. Backward-compatible: existing key='api_key' rows appear in the list with name 'api_key'.

• Server actions (lib/chat/actions.js): createNewApiKey(name) validates name server-side (non-empty, max 64 chars). getApiKeys() calls the renamed DB function and returns an array. deleteApiKey(id) accepts a UUID and calls deleteApiKeyById.

• UI (lib/chat/components/settings-secrets-page.jsx): ApiKeySection rewritten as a key manager. Persistent "Add API key" button at top of section. Inline create form with auto-focused name input, client+server validation, loading/error states. Key list with per-row confirm-before-delete (3s timeout, only the clicked row enters confirm state). Empty state dashed card. One-time key banner unchanged. Regenerate button removed — users create a new key then delete the old one.

Test plan

• Create a key with a name — appears in list with correct prefix, created date, never-used label
• Create a second key with a different name — both appear in the list
• Both keys authenticate x-api-key requests to /api
• Delete one key — removed from list, remaining key still authenticates
• Confirm-before-delete: first click shows "Confirm delete" (red), second click within 3s deletes; timeout resets
• Existing single key (legacy key='api_key' row) appears with name 'api_key' and authenticates normally
• Name validation: empty name blocked client-side and server-side; name > 64 chars rejected server-side
• "Cancel" on create form collapses without creating a key

🤖 Generated with Claude Code

[stephengpope]

CTO Review — Security Findings Assessment

Reviewed against the security audit posted on this issue.

HIGH — NOT FIXED: deleteApiKeyById() Missing Type Guard

The security audit required adding AND type='api_key' to the WHERE clause in deleteApiKeyById. The PR code is:

db.delete(settings).where(eq(settings.id, id)).run();

This only filters by id. Any UUID from any settings row could be deleted if an attacker — or a logic bug in a caller — passes a non-API-key row UUID. The settings table stores other data (roles, configurations) that could be silently destroyed this way. The fix is one line:

db.delete(settings).where(and(eq(settings.id, id), eq(settings.type, 'api_key'))).run();

This must be fixed before merge.

---

CRITICAL — Partial: Timing-Safe Comparison Has Remaining Position Leak

verifyApiKey now iterates with timingSafeEqual, which is correct. However, it returns immediately on match:

if (a.length === b.length && timingSafeEqual(a, b)) {
  // ...
  return entry;  // ← early return leaks map position via timing
}

The security review explicitly required continuing the loop after a match to prevent position leakage. The fix is to collect the match and continue:

let matched = null;
for (const [storedHash, entry] of cache) {
  const b = Buffer.from(storedHash, 'hex');
  if (a.length === b.length && timingSafeEqual(a, b)) matched = entry;
  // Do NOT break — continue to prevent position leak
}
// Handle last_used_at update for matched entry here, then return matched

For typical deployments (1–5 keys) the practical exploit surface of this timing leak is minimal. However the fix is straightforward and was explicitly called out in the security review. Recommend addressing before merge.

---

CONFIRMED FIXED: Server-Side Name Validation

createNewApiKey(name) now validates non-empty, trims whitespace, and enforces ≤64 chars. Correct.

---

CONFIRMED FIXED: Timing-Safe Key Comparison (per-entry)

Each entry in the Map is compared with timingSafeEqual. No Map.get() shortcut. Correct — the remaining concern above is about loop control flow only.

---

Merge Recommendation

Block on: The HIGH type-guard fix in deleteApiKeyById.
Recommended before merge: The timing position-leak loop fix.