Fix null pointer, unauthenticated quote endpoint, race condition, and cache-authorization bypass in Path Payment Service

backend/src/lib/path-payment-recovery.test.js

@@ -30,6 +30,10 @@ vi.mock("stellar-sdk", () => {
 
   return {
     Asset: MockAsset,
+    Networks: {
+      PUBLIC: "Public Global Stellar Network ; September 2015",
+      TESTNET: "Test SDF Network ; September 2015",
+    },
     StrKey: {
       isValidEd25519PublicKey: (value) =>
         typeof value === "string" && value.startsWith("G") && value.length === 56,
@@ -122,4 +126,31 @@ describe("findStrictReceivePaths", () => {
       message: "Horizon returned an invalid path payment quote",
     });
   });
+
+  it("does not throw a null pointer error when Horizon omits `path` on the record (issue #1308)", async () => {
+    mockStrictReceivePaths.mockResolvedValueOnce({
+      records: [
+        {
+          source_amount: "60.1250000",
+          source_asset_type: "native",
+          source_asset_issuer: null,
+          destination_amount: "25.0000000",
+          // No `path` field at all — a shape Horizon can return for a
+          // direct, hop-free route, and one none of the other tests here
+          // exercise (they all set path: [] explicitly).
+        },
+      ],
+    });
+
+    const result = await findStrictReceivePaths({
+      sourceAccount,
+      destAssetCode: "USDC",
+      destAssetIssuer: issuer,
+      destAmount: "25",
+      sourceAssetCode: "XLM",
+      sourceAssetIssuer: null,
+    });
+
+    expect(result.path).toEqual([]);
+  });
 });

backend/src/lib/payment-cache-scoping.test.js

@@ -0,0 +1,109 @@
+import { describe, it, expect, vi } from "vitest";
+import {
+  paymentCacheKey,
+  getCachedPayment,
+  setCachedPayment,
+  invalidatePaymentCache,
+} from "./redis.js";
+
+/**
+ * Payment status cache scoping (issue #1311).
+ *
+ * getPaymentStatus() is called both without a merchant scope (a customer
+ * following their public payment_link) and with one (an authenticated
+ * merchant lookup) for the same payment id. Before this fix, the cache key
+ * was keyed on `id` alone, so a cache hit returned whichever caller's
+ * result was cached first — completely bypassing the merchant_id filter
+ * the uncached query path enforces. These tests pin down that the cache
+ * key, and every function built on it, carries the merchant scope as part
+ * of its identity.
+ */
+describe("paymentCacheKey (issue #1311)", () => {
+  it("produces different keys for different merchant scopes on the same payment id", () => {
+    const publicKey = paymentCacheKey("pay_1", null);
+    const merchantAKey = paymentCacheKey("pay_1", "merchant-a");
+    const merchantBKey = paymentCacheKey("pay_1", "merchant-b");
+
+    expect(new Set([publicKey, merchantAKey, merchantBKey]).size).toBe(3);
+  });
+
+  it("defaults to a stable public scope when no merchantId is given", () => {
+    expect(paymentCacheKey("pay_1")).toBe(paymentCacheKey("pay_1", null));
+  });
+
+  it("produces the same key for the same (id, merchantId) pair", () => {
+    expect(paymentCacheKey("pay_1", "merchant-a")).toBe(
+      paymentCacheKey("pay_1", "merchant-a"),
+    );
+  });
+});
+
+function makeFakeRedisClient() {
+  const store = new Map();
+  return {
+    store,
+    get: vi.fn(async (key) => store.get(key) ?? null),
+    set: vi.fn(async (key, value) => {
+      store.set(key, value);
+    }),
+    del: vi.fn(async (key) => {
+      store.delete(key);
+    }),
+  };
+}
+
+describe("getCachedPayment / setCachedPayment scoping (issue #1311)", () => {
+  it("a payment cached under one merchant's scope is not visible to a different merchant's lookup", async () => {
+    const client = makeFakeRedisClient();
+    const merchantAsPayment = { id: "pay_1", merchant_id: "merchant-a", amount: "10" };
+
+    await setCachedPayment(client, "pay_1", merchantAsPayment, "merchant-a");
+
+    const seenByMerchantB = await getCachedPayment(client, "pay_1", "merchant-b");
+    expect(seenByMerchantB).toBeNull();
+
+    const seenByMerchantA = await getCachedPayment(client, "pay_1", "merchant-a");
+    expect(seenByMerchantA).toEqual(merchantAsPayment);
+  });
+
+  it("a payment cached under the public (unscoped) lookup is not returned for a merchant-scoped lookup", async () => {
+    const client = makeFakeRedisClient();
+    const publicPayment = { id: "pay_1", merchant_id: "merchant-a", amount: "10" };
+
+    await setCachedPayment(client, "pay_1", publicPayment, null);
+
+    const seenByMerchantA = await getCachedPayment(client, "pay_1", "merchant-a");
+    expect(seenByMerchantA).toBeNull();
+
+    const seenPublicly = await getCachedPayment(client, "pay_1", null);
+    expect(seenPublicly).toEqual(publicPayment);
+  });
+});
+
+describe("invalidatePaymentCache scoping (issue #1311)", () => {
+  it("invalidates both the public and the given merchant-scoped entry", async () => {
+    const client = makeFakeRedisClient();
+    const payment = { id: "pay_1", amount: "10" };
+
+    await setCachedPayment(client, "pay_1", payment, null);
+    await setCachedPayment(client, "pay_1", payment, "merchant-a");
+
+    await invalidatePaymentCache(client, "pay_1", "merchant-a");
+
+    expect(await getCachedPayment(client, "pay_1", null)).toBeNull();
+    expect(await getCachedPayment(client, "pay_1", "merchant-a")).toBeNull();
+  });
+
+  it("does not touch a different merchant's cached entry it was not asked to invalidate", async () => {
+    const client = makeFakeRedisClient();
+    const payment = { id: "pay_1", amount: "10" };
+
+    await setCachedPayment(client, "pay_1", payment, "merchant-a");
+    await setCachedPayment(client, "pay_1", payment, "merchant-b");
+
+    await invalidatePaymentCache(client, "pay_1", "merchant-a");
+
+    expect(await getCachedPayment(client, "pay_1", "merchant-a")).toBeNull();
+    expect(await getCachedPayment(client, "pay_1", "merchant-b")).toEqual(payment);
+  });
+});

backend/src/lib/redis.js

@@ -99,19 +99,32 @@ export function resetRedisClientForTests() {
 /** TTL in seconds for payment-status cache entries. */
 export const PAYMENT_STATUS_TTL = 2;
 
-/** Consistent cache key for a payment-status entry. */
-export function paymentCacheKey(id) {
-  return `payment:status:${id}`;
+/**
+ * Consistent cache key for a payment-status entry.
+ *
+ * Scoped by `merchantId` as well as `id` (issue #1311): getPaymentStatus()
+ * is called both without a merchant scope (the public payment_link tracking
+ * page) and with one (an authenticated merchant lookup) for the same
+ * payment id. A key that ignored merchantId meant whichever caller reached
+ * the cache first decided what every later caller saw for that id,
+ * regardless of whether their own `merchant_id` filter would have matched
+ * the row at all — a cache hit bypassed the authorization check the
+ * uncached path enforces. `merchantId` defaults to a fixed "public" bucket
+ * so the unscoped, link-based lookup path still gets its own cache entry.
+ */
+export function paymentCacheKey(id, merchantId = null) {
+  return `payment:status:${merchantId || "public"}:${id}`;
 }
 
 /**
  * Return the cached payment object, or null on miss / Redis unavailable.
  * @param {import("redis").RedisClientType} client
  * @param {string} id  payment UUID
+ * @param {string|null} merchantId  merchant scope this lookup was made under
  */
-export async function getCachedPayment(client, id) {
+export async function getCachedPayment(client, id, merchantId = null) {
   try {
-    const raw = await client.get(paymentCacheKey(id));
+    const raw = await client.get(paymentCacheKey(id, merchantId));
     return raw ? JSON.parse(raw) : null;
   } catch (err) {
     // Never let a cache failure block the request
@@ -125,10 +138,11 @@ export async function getCachedPayment(client, id) {
  * @param {import("redis").RedisClientType} client
  * @param {string} id  payment UUID
  * @param {object} data  the payment row to cache
+ * @param {string|null} merchantId  merchant scope this lookup was made under
  */
-export async function setCachedPayment(client, id, data) {
+export async function setCachedPayment(client, id, data, merchantId = null) {
   try {
-    await client.set(paymentCacheKey(id), JSON.stringify(data), {
+    await client.set(paymentCacheKey(id, merchantId), JSON.stringify(data), {
       EX: PAYMENT_STATUS_TTL,
     });
   } catch (err) {
@@ -137,13 +151,22 @@ export async function setCachedPayment(client, id, data) {
 }
 
 /**
- * Invalidate the cache entry for a payment (call after any write).
+ * Invalidate the cache entries for a payment (call after any write).
+ *
+ * Invalidates both the public (unscoped) entry and, when a merchantId is
+ * supplied, that merchant's scoped entry — a write may be followed by reads
+ * from either lookup path.
  * @param {import("redis").RedisClientType} client
  * @param {string} id  payment UUID
+ * @param {string|null} merchantId  merchant scope to also invalidate, if known
  */
-export async function invalidatePaymentCache(client, id) {
+export async function invalidatePaymentCache(client, id, merchantId = null) {
   try {
-    await client.del(paymentCacheKey(id));
+    const keys = [paymentCacheKey(id, null)];
+    if (merchantId) {
+      keys.push(paymentCacheKey(id, merchantId));
+    }
+    await Promise.all(keys.map((key) => client.del(key)));
   } catch (err) {
     console.error("Redis DEL error:", err.message);
   }

backend/src/lib/stellar.js

@@ -206,7 +206,11 @@ export async function findStrictReceivePaths({
         best.source_asset_type === "native" ? "XLM" : best.source_asset_code,
       source_asset_issuer: best.source_asset_issuer || null,
       destination_amount: best.destination_amount,
-      path: best.path.map((p) => ({
+      // `path` is absent (not just empty) on some Horizon record shapes for
+      // a direct, hop-free route — `.map` on undefined threw a TypeError
+      // here uncaught by the try/catch's Horizon-error handling below, since
+      // it's a plain JS bug, not a rejected promise (issue #1308).
+      path: (best.path || []).map((p) => ({
         asset_code: p.asset_type === "native" ? "XLM" : p.asset_code,
         asset_issuer: p.asset_issuer || null,
       })),

backend/src/routes/payments-path-quote.test.js

@@ -42,13 +42,15 @@ function createSupabaseSelectMock(payment) {
   return chain;
 }
 
-function getPathPaymentQuoteHandler() {
+function getPathPaymentQuoteRoute() {
   const router = createPaymentsRouter();
-  const layer = router.stack.find(
+  return router.stack.find(
     (entry) => entry.route?.path === "/path-payment-quote/:id",
-  );
+  ).route;
+}
 
-  return layer.route.stack.at(-1).handle;
+function getPathPaymentQuoteHandler() {
+  return getPathPaymentQuoteRoute().stack.at(-1).handle;
 }
 
 function createMockResponse() {
@@ -66,6 +68,22 @@ function createMockResponse() {
   };
 }
 
+describe("GET /api/path-payment-quote/:id — authentication (issue #1309)", () => {
+  it("is protected by requireApiKeyAuth, not left unauthenticated", () => {
+    // This route sits outside the `/api/payments` prefix that app.js gates
+    // with requireApiKeyAuth() at the mount level, so it needs its own
+    // per-route auth middleware or it is reachable by anyone. Assert the
+    // middleware stack actually contains an auth layer rather than relying
+    // only on behavioral tests, so a future refactor that silently drops it
+    // fails immediately here instead of shipping an unauthenticated route
+    // again.
+    const route = getPathPaymentQuoteRoute();
+    const middlewareNames = route.stack.map((layer) => layer.name);
+
+    expect(middlewareNames).toContain("requireApiKeyAuth");
+  });
+});
+
 describe("GET /api/path-payment-quote/:id", () => {
   const paymentId = "9f927a2c-02d4-4f76-914c-62cf44d9525e";
   const sourceAccount =