fix: gateway returns 401 on hash mismatch - Add InvalidKeyError typed…

src/__tests__/proxy.integration.test.ts

@@ -10,7 +10,6 @@ import { InMemoryRateLimiter } from '../services/rateLimiter.js';
 import { InMemoryUsageStore } from '../services/usageStore.js';
 import { InMemoryApiRegistry } from '../data/apiRegistry.js';
 import { ApiKey, ApiRegistryEntry } from '../types/gateway.js';
-import { errorHandler } from '../middleware/errorHandler.js';
 
 // ── Test fixtures ───────────────────────────────────────────────────────────
 
@@ -362,7 +361,6 @@ describe('Proxy /v1/call', () => {
     expect(body.message ?? body.error).toMatch(/bad gateway/i);
 
     await new Promise<void>((resolve) => tmpServer.close(() => resolve()));
-    lookupSpy.mockRestore();
   });
 });
 

src/repositories/apiKeyRepository.ts

@@ -2,6 +2,21 @@ import { randomBytes, timingSafeEqual } from "crypto";
 import bcrypt from "bcryptjs";
 import { config } from "../config/index.js";
 
+/**
+ * Typed error returned when an API key prefix is found in the store but the
+ * full-key hash comparison fails. Callers should map this to a 401 response
+ * so the distinction between "prefix not found" and "hash mismatch" is never
+ * observable externally (no timing oracle — both paths yield the same status).
+ */
+export class InvalidKeyError extends Error {
+  public readonly code = 'INVALID_KEY' as const;
+  constructor(message = 'Invalid API key') {
+    super(message);
+    this.name = 'InvalidKeyError';
+    Object.setPrototypeOf(this, InvalidKeyError.prototype);
+  }
+}
+
 export interface ApiKeyRecord {
   id: string;
   apiId: string;
@@ -100,24 +115,46 @@ export const apiKeyRepository = {
       constantTimeCompare(k.prefix, prefix),
     );
 
+    // No records share this prefix — key does not exist at all.
+    if (candidates.length === 0) return null;
+
     for (const candidate of candidates) {
-      if (!candidate.revoked && verifyHash(key, candidate.keyHash)) {
-        // Return a copy without sensitive data
+      if (verifyHash(key, candidate.keyHash)) {
+        if (candidate.revoked) {
+          // Prefix + hash matched a revoked key — let the caller handle 403.
+          return {
+            id: candidate.id,
+            apiId: candidate.apiId,
+            userId: candidate.userId,
+            prefix: candidate.prefix,
+            keyHash: '[REDACTED]',
+            scopes: candidate.scopes,
+            rateLimitPerMinute: candidate.rateLimitPerMinute,
+            createdAt: candidate.createdAt,
+            revoked: candidate.revoked,
+          };
+        }
+        // Return a copy without the raw hash so callers never see the secret.
         return {
           id: candidate.id,
           apiId: candidate.apiId,
           userId: candidate.userId,
           prefix: candidate.prefix,
-          keyHash: "[REDACTED]",
+          keyHash: '[REDACTED]',
           scopes: candidate.scopes,
           rateLimitPerMinute: candidate.rateLimitPerMinute,
           createdAt: candidate.createdAt,
-          revoked: candidate.revoked
+          revoked: candidate.revoked,
         };
       }
     }
 
-    return null;
+    // Prefix was found in the store but no candidate's hash matched the supplied
+    // key. Throw a typed error so callers can distinguish this from "key not
+    // found" while still mapping both outcomes to the same 401 response — this
+    // avoids leaking whether a prefix exists (timing oracle) while keeping
+    // error-handling explicit.
+    throw new InvalidKeyError();
   },
   rotate(id: string, userId: string): { success: true; newKey: string; prefix: string } | { success: false; error: 'not_found' | 'forbidden' | 'revoked' } {
     const index = apiKeys.findIndex(k => k.id === id);

src/routes/gatewayRoutes.test.ts

@@ -4,6 +4,7 @@ import { createGatewayRouter } from "./gatewayRoutes.js";
 import { createRateLimiter } from "../services/rateLimiter.js";
 import { errorHandler } from "../middleware/errorHandler.js";
 import { requestIdMiddleware } from "../middleware/requestId.js";
+import type { ApiKey } from "../types/gateway.js";
 
 describe("gateway route - rate limiting", () => {
   let now = 0;
@@ -198,3 +199,157 @@ describe("gateway route - body size limits", () => {
     expect(res.body).toHaveProperty("message", "Request body too large");
   });
 });
+
+// ---------------------------------------------------------------------------
+// Bug #421 — prefix-exists-but-hash-mismatch must return 401, not 500
+// ---------------------------------------------------------------------------
+describe("gateway route - API key prefix / hash mismatch (bug #421)", () => {
+  /**
+   * Build a minimal app wired to a controlled apiKeys Map.
+   * Billing is set to fail fast (402) so we never attempt a real upstream
+   * request — we only care about the auth layer here.
+   */
+  function buildApp(apiKeys: Map<string, ApiKey>) {
+    const deps = {
+      billing: { deductCredit: async () => ({ success: false, balance: 0 }) },
+      rateLimiter: { check: async () => ({ allowed: true }) },
+      usageStore: { record: () => true },
+      upstreamUrl: "http://example.invalid",
+      apiKeys,
+    } as any;
+
+    const app = express();
+    app.use(requestIdMiddleware);
+    app.use("/gateway", createGatewayRouter(deps));
+    app.use(errorHandler);
+    return app;
+  }
+
+  const API_ID = "my-api";
+
+  /**
+   * Construct a key whose first 16 characters (the prefix) are identical to
+   * `validKey` but whose remaining characters differ — prefix matches but the
+   * SHA-256 hash of the full key will not match.
+   */
+  function buildMismatchedKey(validKey: string): string {
+    // Keep the same 16-char prefix, replace the rest so the hash diverges.
+    const prefix = validKey.slice(0, 16);
+    const differentSuffix = "X".repeat(validKey.length - 16);
+    return prefix + differentSuffix;
+  }
+
+  test("returns 401 (not 500) when prefix matches but hash mismatches", async () => {
+    const validKey = "test-key-abcdefgh"; // 17 chars; prefix = "test-key-abcdefg"
+    const apiKeys = new Map<string, ApiKey>();
+    apiKeys.set(validKey, { key: "k1", apiId: API_ID, developerId: "dev1" });
+
+    const app = buildApp(apiKeys);
+    const mismatchedKey = buildMismatchedKey(validKey);
+
+    const res = await request(app)
+      .get(`/gateway/${API_ID}`)
+      .set("x-api-key", mismatchedKey);
+
+    // Must be 401, never 500.
+    expect(res.status).toBe(401);
+    expect(res.body).toHaveProperty("code", "UNAUTHORIZED");
+    // Generic message — must not reveal whether the prefix was found.
+    expect(res.body.message).toMatch(/invalid API key/i);
+  });
+
+  test("returns 401 when prefix does not exist at all (no-prefix path)", async () => {
+    const validKey = "test-key-abcdefgh";
+    const apiKeys = new Map<string, ApiKey>();
+    apiKeys.set(validKey, { key: "k1", apiId: API_ID, developerId: "dev1" });
+
+    const app = buildApp(apiKeys);
+
+    // Completely different prefix — no candidate will be found.
+    const res = await request(app)
+      .get(`/gateway/${API_ID}`)
+      .set("x-api-key", "totally-unknown-key-xyz");
+
+    expect(res.status).toBe(401);
+    expect(res.body).toHaveProperty("code", "UNAUTHORIZED");
+    expect(res.body.message).toMatch(/invalid API key/i);
+  });
+
+  test("happy path — exact key match passes auth and reaches billing", async () => {
+    const validKey = "test-key-abcdefgh";
+    const apiKeys = new Map<string, ApiKey>();
+    apiKeys.set(validKey, { key: "k1", apiId: API_ID, developerId: "dev1" });
+
+    const app = buildApp(apiKeys);
+
+    // Billing is stubbed to fail (402) so we know auth succeeded.
+    const res = await request(app)
+      .get(`/gateway/${API_ID}`)
+      .set("x-api-key", validKey);
+
+    expect(res.status).toBe(402);
+    expect(res.status).not.toBe(401);
+    expect(res.status).not.toBe(500);
+  });
+
+  test("returns 401 when the matching key belongs to a different apiId", async () => {
+    const validKey = "test-key-abcdefgh";
+    const apiKeys = new Map<string, ApiKey>();
+    // Key is registered under "other-api", not "my-api".
+    apiKeys.set(validKey, { key: "k1", apiId: "other-api", developerId: "dev1" });
+
+    const app = buildApp(apiKeys);
+
+    const res = await request(app)
+      .get(`/gateway/${API_ID}`)
+      .set("x-api-key", validKey);
+
+    expect(res.status).toBe(401);
+    expect(res.body).toHaveProperty("code", "UNAUTHORIZED");
+  });
+
+  test("returns 403 when key is revoked (prefix + hash match a revoked key)", async () => {
+    const validKey = "test-key-abcdefgh";
+    const apiKeys = new Map<string, ApiKey>();
+    apiKeys.set(validKey, {
+      key: "k1",
+      apiId: API_ID,
+      developerId: "dev1",
+      revoked: true,
+    });
+
+    const app = buildApp(apiKeys);
+
+    const res = await request(app)
+      .get(`/gateway/${API_ID}`)
+      .set("x-api-key", validKey);
+
+    expect(res.status).toBe(403);
+    expect(res.body).toHaveProperty("code", "FORBIDDEN");
+  });
+
+  test("401 response body is identical for both mismatch and no-prefix cases (no timing oracle via body)", async () => {
+    const validKey = "test-key-abcdefgh";
+    const apiKeys = new Map<string, ApiKey>();
+    apiKeys.set(validKey, { key: "k1", apiId: API_ID, developerId: "dev1" });
+
+    const app = buildApp(apiKeys);
+
+    const [mismatchRes, unknownRes] = await Promise.all([
+      request(app)
+        .get(`/gateway/${API_ID}`)
+        .set("x-api-key", buildMismatchedKey(validKey)),
+      request(app)
+        .get(`/gateway/${API_ID}`)
+        .set("x-api-key", "totally-unknown-key-xyz"),
+    ]);
+
+    // Status codes must be identical.
+    expect(mismatchRes.status).toBe(401);
+    expect(unknownRes.status).toBe(401);
+
+    // Response codes must be identical — client must not be able to distinguish.
+    expect(mismatchRes.body.code).toBe(unknownRes.body.code);
+    expect(mismatchRes.body.message).toBe(unknownRes.body.message);
+  });
+});

src/routes/gatewayRoutes.ts

@@ -1,9 +1,9 @@
-import { randomUUID } from 'node:crypto';
+import { randomUUID, timingSafeEqual, createHash } from 'node:crypto';
 import express, { Router, type Request, type Response, type NextFunction } from 'express';
 import { z } from 'zod';
 import { startUpstreamTimer, type UpstreamOutcome } from '../metrics.js';
 import { validate } from '../middleware/validate.js';
-import type { GatewayDeps } from '../types/gateway.js';
+import type { GatewayDeps, ApiKey } from '../types/gateway.js';
 import { buildHopByHopSet } from '../lib/hopByHop.js';
 import {
   BadGatewayError,
@@ -14,6 +14,68 @@ import {
   UnauthorizedError,
 } from '../errors/index.js';
 
+/** Length of the key prefix used for candidate pre-filtering (matches repository). */
+const API_KEY_PREFIX_LENGTH = 16;
+
+/**
+ * Derive the SHA-256 hex digest of an API key value.
+ * This matches the hash strategy used by createMapBackedGatewayApiKeyAuthMiddleware.
+ */
+function sha256Hex(value: string): string {
+  return createHash('sha256').update(value).digest('hex');
+}
+
+/**
+ * Constant-time string equality check.
+ * Buffers are always the same length (padded to the longer) so the comparison
+ * time is not a function of where the strings diverge — no timing oracle.
+ */
+function timingSafeStringEqual(a: string, b: string): boolean {
+  const bufA = Buffer.from(a);
+  const bufB = Buffer.from(b);
+  if (bufA.length !== bufB.length) return false;
+  return timingSafeEqual(bufA, bufB);
+}
+
+/**
+ * Resolve an API key string to its registered record using prefix-based
+ * candidate filtering followed by constant-time hash comparison.
+ *
+ * Returns:
+ *  - `{ record }` on a valid match
+ *  - `{ error: 'not_found' }` when no candidate shares the prefix
+ *  - `{ error: 'hash_mismatch' }` when a prefix was found but the hash did not
+ *    match — callers MUST map both error variants to the same 401 response so
+ *    the difference is never observable externally.
+ *
+ * The explicit `hash_mismatch` discriminant exists purely for internal
+ * observability (logging, metrics) without leaking any information to clients.
+ */
+function resolveApiKey(
+  apiKeyHeader: string,
+  apiKeys: Map<string, ApiKey>,
+): { record: ApiKey } | { error: 'not_found' | 'hash_mismatch' } {
+  const prefix = apiKeyHeader.slice(0, API_KEY_PREFIX_LENGTH);
+  const inboundHash = sha256Hex(apiKeyHeader);
+
+  let prefixFound = false;
+
+  for (const [rawKey, record] of apiKeys) {
+    if (!timingSafeStringEqual(rawKey.slice(0, API_KEY_PREFIX_LENGTH), prefix)) {
+      continue;
+    }
+    // At least one candidate shares the prefix.
+    prefixFound = true;
+
+    const storedHash = sha256Hex(rawKey);
+    if (timingSafeStringEqual(inboundHash, storedHash)) {
+      return { record };
+    }
+  }
+
+  return { error: prefixFound ? 'hash_mismatch' : 'not_found' };
+}
+
 const CREDIT_COST_PER_CALL = 1;
 const DEFAULT_TIMEOUT_MS = 30_000;
 const DEFAULT_MAX_BODY_SIZE = '1mb';
@@ -47,8 +109,19 @@ export function createGatewayRouter(deps: GatewayDeps): Router {
           return;
         }
 
-        const keyRecord = apiKeys.get(apiKeyHeader);
-        if (!keyRecord || keyRecord.apiId !== req.params.apiId) {
+        // Use prefix-based candidate lookup + constant-time hash comparison so
+        // that a prefix match with a wrong hash produces a clean 401 instead of
+        // propagating an unhandled exception as a 500. Both 'not_found' and
+        // 'hash_mismatch' intentionally map to the same generic 401 message so
+        // clients cannot distinguish the two cases (no timing oracle).
+        const resolved = resolveApiKey(apiKeyHeader, apiKeys);
+        if ('error' in resolved) {
+          next(new UnauthorizedError('Unauthorized: invalid API key'));
+          return;
+        }
+
+        const keyRecord = resolved.record;
+        if (keyRecord.apiId !== req.params.apiId) {
           next(new UnauthorizedError('Unauthorized: invalid API key'));
           return;
         }