Merge pull request #851 from Jayy4rl/fix/api-gateway-security-audit-logger-765-766-767-768

feat(backend): optimize and harden API Gateway Security + Audit Logge…

Author: emdevelopa

backend/src/lib/api-gateway-signature.js

@@ -1,6 +1,8 @@
 import crypto from "node:crypto";
 
 const DEFAULT_SIGNATURE_WINDOW_SECONDS = 300;
+// Minimum HMAC secret length to prevent signing with trivially weak keys (#767)
+const MIN_SECRET_LENGTH = 16;
 
 function normalizeSignatureHeader(signatureHeader) {
   if (typeof signatureHeader !== "string") return null;
@@ -48,7 +50,7 @@ export function signApiGatewayRequest({
   timestamp,
   body,
 }) {
-  if (!secret || !timestamp) {
+  if (!secret || secret.length < MIN_SECRET_LENGTH || !timestamp) {
     return null;
   }
 
@@ -68,8 +70,8 @@ export function verifyApiGatewayRequestSignature({
     process.env.API_GATEWAY_SIGNATURE_TOLERANCE_SECONDS || DEFAULT_SIGNATURE_WINDOW_SECONDS,
   ),
 }) {
-  if (!secret) {
-    return { valid: false, reason: "Missing signature secret" };
+  if (!secret || secret.length < MIN_SECRET_LENGTH) {
+    return { valid: false, reason: "Missing or insufficient signature secret" };
   }
 
   const timestamp = Number.parseInt(String(timestampHeader || ""), 10);

backend/src/lib/api-gateway-signature.test.js

@@ -4,21 +4,23 @@ import {
   verifyApiGatewayRequestSignature,
 } from "./api-gateway-signature.js";
 
+// All secrets must be >= 16 characters (MIN_SECRET_LENGTH enforcement, issue #767)
+const VALID_SECRET = "test-api-key-secure-32chars-padded";
+
 describe("api-gateway-signature", () => {
   it("signs and verifies request payloads", () => {
     const timestamp = 1713916800;
-    const secret = "test-api-key";
 
     const signature = signApiGatewayRequest({
-      secret,
+      secret: VALID_SECRET,
       method: "POST",
       path: "/api/payments",
       timestamp,
       body: { amount: 12.5, asset: "USDC" },
     });
 
     const result = verifyApiGatewayRequestSignature({
-      secret,
+      secret: VALID_SECRET,
       method: "POST",
       path: "/api/payments",
       timestampHeader: String(timestamp),
@@ -32,18 +34,17 @@ describe("api-gateway-signature", () => {
 
   it("rejects signatures outside timestamp tolerance", () => {
     const timestamp = 1713916800;
-    const secret = "test-api-key";
 
     const signature = signApiGatewayRequest({
-      secret,
+      secret: VALID_SECRET,
       method: "GET",
       path: "/api/metrics/summary",
       timestamp,
       body: {},
     });
 
     const result = verifyApiGatewayRequestSignature({
-      secret,
+      secret: VALID_SECRET,
       method: "GET",
       path: "/api/metrics/summary",
       timestampHeader: String(timestamp),
@@ -59,7 +60,7 @@ describe("api-gateway-signature", () => {
 
   it("rejects malformed signature headers", () => {
     const result = verifyApiGatewayRequestSignature({
-      secret: "abc",
+      secret: VALID_SECRET,
       method: "GET",
       path: "/health",
       timestampHeader: "1713916800",
@@ -71,4 +72,73 @@ describe("api-gateway-signature", () => {
     expect(result.valid).toBe(false);
     expect(result.reason).toMatch(/invalid x-api-signature/i);
   });
+
+  // ── Security audit: minimum secret length (#767) ──────────────────────────
+
+  it("rejects signing with a secret shorter than the minimum length", () => {
+    const result = signApiGatewayRequest({
+      secret: "short",
+      method: "GET",
+      path: "/health",
+      timestamp: 1713916800,
+      body: {},
+    });
+
+    expect(result).toBeNull();
+  });
+
+  it("rejects verification with a secret shorter than the minimum length", () => {
+    const result = verifyApiGatewayRequestSignature({
+      secret: "tooshort",
+      method: "GET",
+      path: "/health",
+      timestampHeader: "1713916800",
+      signatureHeader: "sha256=" + "a".repeat(64),
+      body: {},
+      now: 1713916800 * 1000,
+    });
+
+    expect(result.valid).toBe(false);
+    expect(result.reason).toMatch(/insufficient.*secret/i);
+  });
+
+  it("rejects verification with a missing secret", () => {
+    const result = verifyApiGatewayRequestSignature({
+      secret: "",
+      method: "GET",
+      path: "/health",
+      timestampHeader: "1713916800",
+      signatureHeader: "sha256=" + "a".repeat(64),
+      body: {},
+      now: 1713916800 * 1000,
+    });
+
+    expect(result.valid).toBe(false);
+    expect(result.reason).toMatch(/insufficient.*secret/i);
+  });
+
+  it("detects a tampered body by producing a different signature", () => {
+    const timestamp = 1713916800;
+
+    const signature = signApiGatewayRequest({
+      secret: VALID_SECRET,
+      method: "POST",
+      path: "/api/payments",
+      timestamp,
+      body: { amount: 10 },
+    });
+
+    const result = verifyApiGatewayRequestSignature({
+      secret: VALID_SECRET,
+      method: "POST",
+      path: "/api/payments",
+      timestampHeader: String(timestamp),
+      signatureHeader: `sha256=${signature}`,
+      body: { amount: 99 }, // tampered
+      now: timestamp * 1000,
+    });
+
+    expect(result.valid).toBe(false);
+    expect(result.reason).toMatch(/verification failed/i);
+  });
 });

backend/src/lib/auth.js

@@ -1,9 +1,54 @@
 import bcrypt from "bcryptjs";
 import { recordMerchantApiUsage } from "./api-usage.js";
 import { verifyApiGatewayRequestSignature } from "./api-gateway-signature.js";
+import { queryWithRetry } from "./db.js";
 
 const SALT_ROUNDS = 12;
 
+const MERCHANT_SELECT_COLUMNS =
+  "id, email, business_name, notification_email, branding_config, merchant_settings, webhook_secret, webhook_secret_old, webhook_secret_expiry, webhook_version, payment_limits, api_key, api_key_expires_at, api_key_old, api_key_old_expires_at";
+
+// Auth failure rate limiting per client IP (issue #767)
+const AUTH_FAIL_RATE_LIMIT_MAX = Number(process.env.AUTH_FAIL_RATE_LIMIT_MAX || 10);
+const AUTH_FAIL_RATE_LIMIT_WINDOW_MS = Number(process.env.AUTH_FAIL_RATE_LIMIT_WINDOW_MS || 60_000);
+const _authFailState = new Map();
+
+export function _resetAuthFailStateForTests() {
+  _authFailState.clear();
+}
+
+function isAuthRateLimited(ip, now = Date.now()) {
+  const state = _authFailState.get(ip);
+  if (!state || now >= state.windowStart + AUTH_FAIL_RATE_LIMIT_WINDOW_MS) return false;
+  return state.count >= AUTH_FAIL_RATE_LIMIT_MAX;
+}
+
+function recordAuthFailure(ip, now = Date.now()) {
+  const state = _authFailState.get(ip);
+  if (!state || now >= state.windowStart + AUTH_FAIL_RATE_LIMIT_WINDOW_MS) {
+    _authFailState.set(ip, { count: 1, windowStart: now });
+  } else {
+    state.count += 1;
+  }
+}
+
+// Single-query lookup covering both current and rotated API keys.
+// deleted_at IS NULL applies to both paths — previously missing on the old-key
+// path which allowed deleted merchants to authenticate via a rotated key (#767).
+// queryWithRetry handles transient DB failures automatically (#766).
+async function defaultMerchantLookup(apiKey) {
+  const result = await queryWithRetry(
+    `SELECT ${MERCHANT_SELECT_COLUMNS}
+     FROM merchants
+     WHERE deleted_at IS NULL
+       AND (api_key = $1 OR api_key_old = $1)
+     LIMIT 1`,
+    [apiKey],
+    { label: "auth-merchant-lookup" },
+  );
+  return result.rows[0] || null;
+}
+
 /**
  * Hash a plain-text merchant password with bcrypt.
  * @param {string} plaintext
@@ -24,28 +69,25 @@ export async function verifyPassword(plaintext, hash) {
 }
 
 export function createApiKeyAuth({
-  supabaseClient = null,
+  supabaseClient = null, // unused for API key auth; retained for session-auth compat
   usageRecorder = recordMerchantApiUsage,
   verifyGatewaySignature = verifyApiGatewayRequestSignature,
   requireSignature = false,
+  merchantLookup = defaultMerchantLookup,
 } = {}) {
   return async function requireApiKeyAuth(req, res, next) {
     try {
-      // Another auth layer (for example x402 token bridge) may have already
-      // attached a merchant context. If so, honor it and continue.
+      // Another auth layer (e.g. x402 token bridge) may have already attached a
+      // merchant context. If so, honor it and continue.
       if (req.merchant?.id) {
         try {
-          await usageRecorder({
-            merchantId: req.merchant.id,
-            req,
-          });
+          await usageRecorder({ merchantId: req.merchant.id, req });
         } catch (usageError) {
           console.warn("Failed to record merchant API usage:", usageError.message);
         }
         return next();
       }
 
-      const client = supabaseClient || (await import("./supabase.js")).supabase;
       const headerValue = req.get("x-api-key");
       const apiKey = typeof headerValue === "string" ? headerValue.trim() : "";
       const signatureHeader = req.get("x-api-signature");
@@ -55,11 +97,10 @@ export function createApiKeyAuth({
         return res.status(401).json({ error: "Missing x-api-key header" });
       }
 
-      // Backward-compatible API gateway hardening: verify request HMAC
-      // signature only when signature headers are supplied by the client.
       const hasSignatureHeader =
         typeof signatureHeader === "string" && signatureHeader.trim().startsWith("sha256=");
-      const hasTimestampHeader = typeof timestampHeader === "string" && timestampHeader.trim().length > 0;
+      const hasTimestampHeader =
+        typeof timestampHeader === "string" && timestampHeader.trim().length > 0;
       const signatureProvided = hasSignatureHeader && hasTimestampHeader;
 
       if (requireSignature && !signatureProvided) {
@@ -88,74 +129,49 @@ export function createApiKeyAuth({
         }
       }
 
-// First try to find merchant by current API key
-let { data: merchant, error } = await client
-  .from("merchants")
-  .select(
-    "id, email, business_name, notification_email, branding_config, merchant_settings, webhook_secret, webhook_secret_old, webhook_secret_expiry, webhook_version, payment_limits, api_key, api_key_expires_at, api_key_old, api_key_old_expires_at"
-  )
-  .eq("api_key", apiKey)
-  .is("deleted_at", null)
-  .maybeSingle();
-
-if (error) {
-  error.status = 500;
-  throw error;
-}
+      // Block IPs that have exceeded the failed-attempt threshold (#767)
+      const clientIp = req.ip || "unknown";
+      if (isAuthRateLimited(clientIp)) {
+        return res.status(429).json({
+          error: "Too many failed authentication attempts",
+          code: "AUTH_RATE_LIMITED",
+        });
+      }
 
-// If not found by current key, check if it's the old key during rotation overlap
-if (!merchant) {
-  const { data: oldKeyMerchant, error: oldKeyError } = await client
-    .from("merchants")
-    .select(
-      "id, email, business_name, notification_email, branding_config, merchant_settings, webhook_secret, webhook_secret_old, webhook_secret_expiry, webhook_version, payment_limits, api_key, api_key_expires_at, api_key_old, api_key_old_expires_at"
-    )
-    .eq("api_key_old", apiKey)
-    .maybeSingle();
-
-  if (oldKeyError) {
-    oldKeyError.status = 500;
-    throw oldKeyError;
-  }
+      // Single combined query for current and rotated keys (#765).
+      // Retry logic is provided by queryWithRetry (#766).
+      let merchant;
+      try {
+        merchant = await merchantLookup(apiKey);
+      } catch (err) {
+        err.status = 500;
+        throw err;
+      }
 
-  if (!oldKeyMerchant) {
-    return res.status(401).json({ error: "Invalid API key" });
-  }
+      if (!merchant) {
+        recordAuthFailure(clientIp);
+        return res.status(401).json({ error: "Invalid API key" });
+      }
 
-  // Check if old key has expired (overlap period ended)
-  const now = new Date();
-  if (
-    oldKeyMerchant.api_key_old_expires_at &&
-    new Date(oldKeyMerchant.api_key_old_expires_at) < now
-  ) {
-    return res.status(401).json({
-      error: "API key has expired. Please rotate to a new key.",
-      code: "API_KEY_EXPIRED"
-    });
-  }
+      // Determine which key matched to validate the correct expiry field
+      const now = new Date();
+      const usedCurrentKey = merchant.api_key === apiKey;
+      const expiresAt = usedCurrentKey
+        ? merchant.api_key_expires_at
+        : merchant.api_key_old_expires_at;
 
-  merchant = oldKeyMerchant;
-} else {
-  // Check if current API key has expired
-  const now = new Date();
-  if (
-    merchant.api_key_expires_at &&
-    new Date(merchant.api_key_expires_at) < now
-  ) {
-    return res.status(401).json({
-      error: "API key has expired. Please rotate to a new key.",
-      code: "API_KEY_EXPIRED"
-    });
-  }
-}
+      if (expiresAt && new Date(expiresAt) < now) {
+        recordAuthFailure(clientIp);
+        return res.status(401).json({
+          error: "API key has expired. Please rotate to a new key.",
+          code: "API_KEY_EXPIRED",
+        });
+      }
 
       req.merchant = merchant;
 
       try {
-        await usageRecorder({
-          merchantId: merchant.id,
-          req,
-        });
+        await usageRecorder({ merchantId: merchant.id, req });
       } catch (usageError) {
         // Usage metrics should never block API traffic.
         console.warn("Failed to record merchant API usage:", usageError.message);
@@ -190,7 +206,7 @@ export function requireSessionAuth() {
 
       const client = (await import("./supabase.js")).supabase;
       const merchantId = payload.id || payload.merchant_id;
-      
+
       if (!merchantId) {
         return res.status(401).json({ error: "Invalid token payload: missing merchant identification" });
       }