feat(backend): implement payload sanitization and strict validation for Database Row-Level Security Service

backend/src/lib/db-rls.js

@@ -1,25 +1,103 @@
+import { z } from "zod";
 import { pool } from "../lib/db.js";
+import { logger } from "../lib/logger.js";
+
+// ── Validation Schemas ──────────────────────────────────────────────────────
+const MerchantIdSchema = z
+  .string()
+  .trim()
+  .min(1, "merchantId must be a non-empty string")
+  .max(128, "merchantId exceeds maximum length of 128 characters")
+  .regex(
+    /^[a-zA-Z0-9_\-:.@]+$/,
+    "merchantId contains invalid characters"
+  );
+
+// ── Sanitization Helpers ─────────────────────────────────────────────────────
+
+// ── Validation Functions ─────────────────────────────────────────────────────
+
+/**
+ * Strictly validates and sanitizes a merchant ID.
+ * Returns { valid: true, merchantId } or { valid: false, error }
+ */
+export function validateAndSanitizeMerchantId(merchantId) {
+  try {
+    const parsed = MerchantIdSchema.parse(merchantId);
+    return { valid: true, merchantId: parsed };
+  } catch (err) {
+    const errorMessage =
+      err instanceof z.ZodError ? err.errors[0]?.message : "Invalid merchantId";
+    return { valid: false, error: errorMessage };
+  }
+}
+
+// ── Core Functions ──────────────────────────────────────────────────────────
 
 /**
  * Executes a callback within a transaction where the merchant context is set.
  * This ensures Row Level Security (RLS) is enforced for direct pg pool queries.
+ * Includes payload sanitization and strict validation for merchantId.
  */
 export async function withMerchantContext(merchantId, callback) {
+  // Validate and sanitize merchantId
+  const validation = validateAndSanitizeMerchantId(merchantId);
+  if (!validation.valid) {
+    logger.warn(
+      { merchantId, error: validation.error },
+      "[RLS] Invalid merchantId provided to withMerchantContext"
+    );
+    throw new Error(`Invalid merchantId: ${validation.error}`);
+  }
+
+  const sanitizedMerchantId = validation.merchantId;
+
+  // Validate callback is a function
+  if (typeof callback !== "function") {
+    logger.warn(
+      { merchantId: sanitizedMerchantId },
+      "[RLS] Invalid callback provided to withMerchantContext"
+    );
+    throw new Error("callback must be a function");
+  }
+
   const client = await pool.connect();
   try {
     await client.query("BEGIN");
-    if (merchantId) {
-      await client.query("SET LOCAL app.current_merchant_id = $1", [
-        merchantId,
-      ]);
-    }
+    await client.query("SET LOCAL app.current_merchant_id = $1", [
+      sanitizedMerchantId,
+    ]);
     const result = await callback(client);
     await client.query("COMMIT");
     return result;
   } catch (err) {
-    await client.query("ROLLBACK");
+    try {
+      await client.query("ROLLBACK");
+    } catch (rollbackErr) {
+      logger.error(
+        { rollbackErr: rollbackErr.message },
+        "[RLS] Failed to rollback transaction"
+      );
+    }
     throw err;
   } finally {
     client.release();
   }
 }
+
+/**
+ * Sanitizes a merchant ID for logging purposes (non-validation path).
+ * This is useful when you need to safely include merchantId in logs
+ * without throwing validation errors.
+ */
+export function sanitizeMerchantId(merchantId) {
+  if (typeof merchantId !== "string") {
+    return null;
+  }
+  const trimmed = merchantId.trim();
+  if (trimmed.length === 0 || trimmed.length > 128) {
+    return null;
+  }
+  const sanitized = trimmed.replace(/[^a-zA-Z0-9_\-:.@]/g, "");
+  return sanitized.length > 0 ? sanitized : null;
+}

backend/src/lib/db-rls.test.js

@@ -0,0 +1,162 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { withMerchantContext, validateAndSanitizeMerchantId, sanitizeMerchantId } from "./db-rls.js";
+import { pool } from "../lib/db.js";
+
+vi.mock("../lib/db.js", () => ({
+  pool: {
+    connect: vi.fn(),
+  },
+}));
+
+beforeEach(() => {
+  vi.resetAllMocks();
+});
+
+function createMockClient() {
+  const client = {
+    query: vi.fn().mockResolvedValue({ rows: [] }),
+    release: vi.fn(),
+  };
+  return client;
+}
+
+describe("db-rls — payload sanitization and strict validation", () => {
+  it("validates and sanitizes a valid merchantId", () => {
+    const result = validateAndSanitizeMerchantId("merchant-123");
+    expect(result.valid).toBe(true);
+    expect(result.merchantId).toBe("merchant-123");
+  });
+
+  it("trims whitespace from merchantId", () => {
+    const result = validateAndSanitizeMerchantId("  merchant-123  ");
+    expect(result.valid).toBe(true);
+    expect(result.merchantId).toBe("merchant-123");
+  });
+
+  it("rejects null merchantId", () => {
+    const result = validateAndSanitizeMerchantId(null);
+    expect(result.valid).toBe(false);
+    expect(result.error).toBeDefined();
+  });
+
+  it("rejects undefined merchantId", () => {
+    const result = validateAndSanitizeMerchantId(undefined);
+    expect(result.valid).toBe(false);
+    expect(result.error).toBeDefined();
+  });
+
+  it("rejects empty string merchantId", () => {
+    const result = validateAndSanitizeMerchantId("");
+    expect(result.valid).toBe(false);
+    expect(result.error).toBeDefined();
+  });
+
+  it("rejects merchantId with invalid characters", () => {
+    const result = validateAndSanitizeMerchantId("merchant!@#");
+    expect(result.valid).toBe(false);
+  });
+
+  it("rejects merchantId that is too long", () => {
+    const longId = "a".repeat(129);
+    const result = validateAndSanitizeMerchantId(longId);
+    expect(result.valid).toBe(false);
+    expect(result.error).toContain("128");
+  });
+
+  it("accepts merchantId with allowed special characters", () => {
+    const result = validateAndSanitizeMerchantId("merchant_id:sub.domain-123@test");
+    expect(result.valid).toBe(true);
+    expect(result.merchantId).toBe("merchant_id:sub.domain-123@test");
+  });
+
+  it("sanitizes merchantId by stripping dangerous characters", () => {
+    const result = sanitizeMerchantId("merchant'; DROP TABLE merchants; --");
+    expect(result).toBe("merchantDROPTABLEmerchants--");
+  });
+
+  it("sanitizes returns null for non-string input", () => {
+    expect(sanitizeMerchantId(null)).toBeNull();
+    expect(sanitizeMerchantId(undefined)).toBeNull();
+    expect(sanitizeMerchantId(123)).toBeNull();
+  });
+
+  it("sanitizes returns null for empty result after cleaning", () => {
+    expect(sanitizeMerchantId("!!!!!!!!")).toBeNull();
+  });
+
+  it("sanitizes enforces length limit", () => {
+    const longId = "a".repeat(200);
+    expect(sanitizeMerchantId(longId)).toBeNull();
+  });
+});
+
+describe("withMerchantContext — transaction handling with validation", () => {
+  it("sets merchant context in transaction and commits on success", async () => {
+    const client = createMockClient();
+    pool.connect.mockResolvedValue(client);
+
+    const callback = vi.fn().mockResolvedValue({ data: "test" });
+    const result = await withMerchantContext("merchant-123", callback);
+
+    expect(result).toEqual({ data: "test" });
+    expect(pool.connect).toHaveBeenCalledTimes(1);
+    expect(client.query).toHaveBeenCalledWith("BEGIN");
+    expect(client.query).toHaveBeenCalledWith("SET LOCAL app.current_merchant_id = $1", ["merchant-123"]);
+    expect(client.query).toHaveBeenCalledWith("COMMIT");
+    expect(callback).toHaveBeenCalledWith(client);
+    expect(client.release).toHaveBeenCalledTimes(1);
+  });
+
+  it("rolls back on callback error", async () => {
+    const client = createMockClient();
+    pool.connect.mockResolvedValue(client);
+
+    const error = new Error("callback failed");
+    const callback = vi.fn().mockRejectedValue(error);
+
+    await expect(withMerchantContext("merchant-123", callback)).rejects.toThrow("callback failed");
+    expect(client.query).toHaveBeenCalledWith("BEGIN");
+    expect(client.query).toHaveBeenCalledWith("SET LOCAL app.current_merchant_id = $1", ["merchant-123"]);
+    expect(client.query).toHaveBeenCalledWith("ROLLBACK");
+    expect(client.query).not.toHaveBeenCalledWith("COMMIT");
+    expect(client.release).toHaveBeenCalledTimes(1);
+  });
+
+  it("throws if merchantId is invalid", async () => {
+    const callback = vi.fn();
+    await expect(withMerchantContext("", callback)).rejects.toThrow("Invalid merchantId");
+    expect(pool.connect).not.toHaveBeenCalled();
+  });
+
+  it("throws if callback is not a function", async () => {
+    await expect(withMerchantContext("merchant-123", null)).rejects.toThrow("callback must be a function");
+    expect(pool.connect).not.toHaveBeenCalled();
+  });
+
+  it("throws if callback is not a function (string)", async () => {
+    await expect(withMerchantContext("merchant-123", "not-a-function")).rejects.toThrow("callback must be a function");
+    expect(pool.connect).not.toHaveBeenCalled();
+  });
+
+  it("handles rollback failure gracefully", async () => {
+    const client = createMockClient();
+    client.query
+      .mockResolvedValueOnce({ rows: [] }) // BEGIN
+      .mockResolvedValueOnce({ rows: [] }) // SET LOCAL
+      .mockRejectedValueOnce(new Error("rollback failed")); // ROLLBACK fails
+
+    pool.connect.mockResolvedValue(client);
+
+    const error = new Error("callback failed");
+    const callback = vi.fn().mockRejectedValue(error);
+
+    await expect(withMerchantContext("merchant-123", callback)).rejects.toThrow("callback failed");
+    expect(client.release).toHaveBeenCalledTimes(1);
+  });
+
+  it("handles connection errors", async () => {
+    pool.connect.mockRejectedValue(new Error("connection failed"));
+    const callback = vi.fn();
+    await expect(withMerchantContext("merchant-123", callback)).rejects.toThrow("connection failed");
+  });
+});