Skip to content

fix(auth): add api key auth via sha256 hash lookup#4266

Merged
TheodoreSpeaks merged 5 commits intostagingfrom
fix/api-key-hash
Apr 22, 2026
Merged

fix(auth): add api key auth via sha256 hash lookup#4266
TheodoreSpeaks merged 5 commits intostagingfrom
fix/api-key-hash

Conversation

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator

Summary

Our current api key validation logic is inefficient, storing every api key in memory and decrypting and comparing one at a time. This uses up db connections. Instead, we should validate based on a sha256 hash of the key.

This pr adds a new hashed api key column and tries to validate by hash before falling back on the older api key system. Once we've backfilled the old api keys, we can remove the hash.

This is a multi-step change:

  1. Add new db column and add logic so new api key additions automatically get sha256 hash added. All old api keys are validated using fallback existing logic
  2. Run backfill. From here onwards, the hashed api key can be changed to be non-null
  3. Remove backup logic. Confirm first that no more api keys use fallback, then remove the logic from codebase and set column as non-null.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

Validated locally. First tested that existing api keys work via backup, then ran backfill and tested again and validated it used the hashed key. Also validated new api keys are created with hashed api key column.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Apr 22, 2026 10:22pm

Request Review

@TheodoreSpeaks
Copy link
Copy Markdown
Collaborator Author

@BugBot review

@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 22, 2026

PR Summary

Medium Risk
Touches API-key authentication and adds a new indexed database column; mistakes could reject valid keys or create hash collisions/uniqueness issues during rollout/backfill.

Overview
API key auth is rewritten to use a SHA-256 hash lookup first, falling back to the existing scan+decrypt loop only when no key_hash match is found (and logging a warn when the fallback succeeds).

API key creation now persists keyHash for both personal and workspace keys (API routes and createWorkspaceApiKey), and the DB gains a nullable api_key.key_hash column with a unique index to support fast lookups.

Adds hashApiKey crypto primitive plus targeted Vitest coverage for hashing semantics and the new auth fast-path/fallback behavior.

Reviewed by Cursor Bugbot for commit 8216bad. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 335d662. Configure here.

Comment thread apps/sim/lib/core/config/feature-flags.ts Outdated
@TheodoreSpeaks TheodoreSpeaks marked this pull request as ready for review April 22, 2026 22:10
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 22, 2026

Greptile Summary

This PR replaces the inefficient full-table-scan + AES-GCM-decrypt-loop API key authentication with a single indexed WHERE key_hash = sha256(incoming_key) lookup, while preserving a legacy fallback (with a warn-log signal) for rows not yet backfilled. The migration, schema change, new key-creation paths, backfill script, and test coverage are all consistent and the phased rollout strategy is sound.

Confidence Score: 5/5

Safe to merge — all findings are P2 style/comment improvements with no correctness impact

The auth logic is well-structured and the fast path, fallback, scope gates, and expiry checks are all correct. All remaining comments are P2 (misleading JSDoc comment, missing EOF newline, undocumented detection heuristic). No correctness, data-integrity, or security issues were found.

No files require special attention — service.ts has a misleading comment but no logic bug

Important Files Changed

Filename Overview
apps/sim/lib/api-key/service.ts Core auth rewrite: adds SHA-256 hash fast path via lookupByHash/applyHashGates, preserves legacy decrypt-loop fallback with a warn log; comment on lookupByHash inaccurately claims parallelism with getWorkspaceBillingSettings
apps/sim/lib/api-key/crypto.ts Adds hashApiKey — a thin SHA-256 wrapper using Node's built-in crypto.createHash; deterministic, no side-effects, well-documented
packages/db/scripts/backfill-api-key-hash.ts Idempotent backfill script with dry-run, batch processing, crypto self-test, and graceful error stats; isEncryptedKey heuristic differs from isEncryptedApiKeyFormat but operates on stored blobs rather than plain-text keys — worth a clarifying comment
packages/db/schema.ts Adds nullable key_hash text column and uniqueIndex on it; nullable is correct for the phased migration approach
packages/db/migrations/0195_normal_white_queen.sql Adds key_hash column and btree unique index; missing trailing newline
apps/sim/lib/api-key/auth.ts Adds keyHash field to createWorkspaceApiKey insert; consistent with route-level changes
apps/sim/app/api/users/me/api-keys/route.ts Adds keyHash: hashApiKey(plainKey) to personal key creation insert
apps/sim/app/api/workspaces/[id]/api-keys/route.ts Adds keyHash: hashApiKey(plainKey) to workspace key creation insert
apps/sim/lib/api-key/service.test.ts Good test coverage: fast path, fallback with warn log, scope gate rejections, and SHA-256 hash query verification
apps/sim/lib/api-key/crypto.test.ts Tests determinism, hex format, NIST SHA-256 vector, and backfill idempotency of hashApiKey
packages/db/scripts/backfill-api-key-hash.test.ts Tests isEncryptedKey, hashApiKey, and deriveKeyHashForStoredKey idempotency for both legacy and encrypted row formats

Sequence Diagram

sequenceDiagram
    participant C as Client
    participant S as authenticateApiKeyFromHeader
    participant DB as Database
    participant L as Legacy Fallback

    C->>S: API request with key header
    S->>DB: getWorkspaceBillingSettings (if workspaceId)
    DB-->>S: workspaceSettings

    S->>DB: SELECT ... WHERE key_hash = sha256(header)
    alt Hash row found
        DB-->>S: HashCandidate row
        S->>S: applyHashGates(record, options, workspaceSettings)
        note right of S: Check userId, keyType,<br/>expiry, workspace scope,<br/>personal key permissions
        S-->>C: ApiKeyAuthResult success
    else No hash match (key not yet backfilled)
        DB-->>S: []
        S->>DB: SELECT ... WHERE userId/type conditions (full scan)
        DB-->>S: Encrypted key rows
        loop For each candidate
            S->>L: authenticateApiKey(header, storedKey.key)
            L-->>S: valid?
            alt Valid match
                S->>S: logger.warn matched via fallback decrypt loop
                S-->>C: ApiKeyAuthResult success
            end
        end
        S-->>C: success false error Invalid API key
    end
Loading

Reviews (1): Last reviewed commit: "fix feature flag" | Re-trigger Greptile

Comment thread apps/sim/lib/api-key/service.ts Outdated
Comment thread packages/db/migrations/0195_normal_white_queen.sql
Comment thread packages/db/scripts/backfill-api-key-hash.ts
@TheodoreSpeaks TheodoreSpeaks merged commit 8ce56fe into staging Apr 22, 2026
14 checks passed
@TheodoreSpeaks TheodoreSpeaks deleted the fix/api-key-hash branch April 22, 2026 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant