diff --git a/backend/src/lib/db-rls.js b/backend/src/lib/db-rls.js index 1eb9d7b..37dafe4 100644 --- a/backend/src/lib/db-rls.js +++ b/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; +} diff --git a/backend/src/lib/db-rls.test.js b/backend/src/lib/db-rls.test.js new file mode 100644 index 0000000..063e2ed --- /dev/null +++ b/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"); + }); +});