From 0562eeea484a390ecfbc389640f1dca262ade61d Mon Sep 17 00:00:00 2001
From: BenKalsky <ben@digitizer.co.il>
Date: Fri, 1 May 2026 17:49:12 +0300
Subject: [PATCH] fix: harden SUMIT checkout routes for production

---
 README.md                           | 25 ++++++++++++------
 package.json                        |  2 +-
 src/client/useSumitCheckout.test.ts | 29 +++++++++++++++++++++
 src/client/useSumitCheckout.ts      | 15 ++++++++++-
 src/next/createChargeRoute.test.ts  | 40 +++++++++++++++++++++++++++++
 src/next/createChargeRoute.ts       | 38 +++++++++++++++++++++++++++
 src/next/createWebhookRoute.test.ts | 25 +++++++++++++++++-
 src/next/createWebhookRoute.ts      | 16 ++++++++++--
 8 files changed, 177 insertions(+), 13 deletions(-)
 create mode 100644 src/client/useSumitCheckout.test.ts

diff --git a/README.md b/README.md
index 2150867..8f9f70d 100644
--- a/README.md
+++ b/README.md
@@ -65,7 +65,11 @@ export function Checkout() {
         item: { name: "Pro Plan", description: "Monthly", unitPrice: 19, currency: "USD", durationMonths: 1 },
       }),
     });
-    if (!res.ok) checkout.handleError(new Error(await res.text()));
+    if (!res.ok) {
+      checkout.handleError(new Error(await res.text()));
+      return;
+    }
+    checkout.handleSuccess();
   }
 
   return (
@@ -76,10 +80,7 @@ export function Checkout() {
       environment="production"
       language="he-IL"
       onTokenizationStart={checkout.handleStart}
-      onToken={(token) => {
-        checkout.handleToken(token);
-        return handleToken(token);
-      }}
+      onToken={handleToken}
       onError={checkout.handleError}
     >
       <button type="submit" disabled={checkout.status === "submitting"}>
@@ -141,7 +142,15 @@ export const POST = createSumitWebhookRoute({
 });
 ```
 
-Accepts JSON, `application/x-www-form-urlencoded`, and SUMIT's `json=<serialized>` envelope. Returns `200` on success, `401` when verification fails, `500` (without leaking the original error) when your handler throws.
+Accepts JSON, `application/x-www-form-urlencoded`, `multipart/form-data`, and SUMIT's `json=<serialized>` envelope. Returns `200` on success, `401` when verification fails, `500` (without leaking the original error) when your handler throws.
+
+By default, `verifySumitSharedSecret(secret)` accepts only the `x-sumit-secret` header. If an existing SUMIT trigger can only send a URL query secret, opt in explicitly:
+
+```ts
+verify: verifySumitSharedSecret(process.env.SUMIT_WEBHOOK_SECRET!, { queryParam: "secret" })
+```
+
+Header verification is preferred because query strings are commonly stored in access logs.
 
 ---
 
@@ -162,7 +171,7 @@ Accepts JSON, `application/x-www-form-urlencoded`, and SUMIT's `json=<serialized
 | --- | --- |
 | **Card data exposure** | SUMIT's `payments.js` reads card fields directly and returns a `SingleUseToken`. Card numbers, expiry, and CVV never reach your server or your component state. |
 | **Server credential leakage** | The full `apiKey` lives only in `createSumitChargeRoute`; `./client` and `./next` are separate exports so client bundles cannot transitively pull the server secret. |
-| **Webhook spoofing** | `verifySumitSharedSecret` hashes both the candidate and the secret to a fixed 32-byte digest before comparing — the comparison is constant-time **and** length-independent, so response timing leaks neither secret content nor secret length. |
+| **Webhook spoofing** | `verifySumitSharedSecret` checks the `x-sumit-secret` header by default and hashes both the candidate and the secret to a fixed 32-byte digest before comparing — the comparison is constant-time **and** length-independent, so response timing leaks neither secret content nor secret length. Query-string secrets are opt-in only because URLs commonly land in logs. |
 | **Double-submit / token reuse** | `<SumitCheckout />` uses a synchronous ref guard so two rapid submits cannot both fire `CreateToken` (single-use tokens are exactly that — single-use). |
 | **Logging sensitive data** | Every event the route helpers return passes through `redactSumitPayload` from `@digitizers/sumit-api`. |
 
@@ -177,7 +186,7 @@ SumitCheckout(props): JSX.Element
   props.requireCvv?, requireCitizenId?
   props.onToken, onError?, onTokenizationStart?, onTokenizationEnd?
   props.classNames?, style?, labels?
-useSumitCheckout(): { ref, status, error, token, submit, reset, handleToken, handleError, handleStart }
+useSumitCheckout(): { ref, status, error, token, submit, reset, handleToken, handleSuccess, handleError, handleStart, clearToken }
 loadSumitPayments(env?): Promise<SumitPaymentsSdk>
 createSingleUseToken(settings): Promise<string>
 
diff --git a/package.json b/package.json
index 15ace4c..d71c7ee 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@digitizers/sumit-react",
-  "version": "0.1.0",
+  "version": "0.1.1",
   "description": "React components and Next.js route helpers for SUMIT/OfficeGuy/Upay payments.",
   "license": "MIT",
   "type": "module",
diff --git a/src/client/useSumitCheckout.test.ts b/src/client/useSumitCheckout.test.ts
new file mode 100644
index 0000000..d8ac6cd
--- /dev/null
+++ b/src/client/useSumitCheckout.test.ts
@@ -0,0 +1,29 @@
+import { act, renderHook } from "@testing-library/react";
+import { describe, expect, it } from "vitest";
+
+import { useSumitCheckout } from "./useSumitCheckout.js";
+
+describe("useSumitCheckout", () => {
+  it("can mark success without storing a SingleUseToken", () => {
+    const { result } = renderHook(() => useSumitCheckout());
+
+    act(() => {
+      result.current.handleSuccess();
+    });
+
+    expect(result.current.status).toBe("succeeded");
+    expect(result.current.token).toBeNull();
+  });
+
+  it("can clear a stored token after legacy handleToken usage", () => {
+    const { result } = renderHook(() => useSumitCheckout());
+
+    act(() => {
+      result.current.handleToken("tok_xyz");
+      result.current.clearToken();
+    });
+
+    expect(result.current.status).toBe("succeeded");
+    expect(result.current.token).toBeNull();
+  });
+});
diff --git a/src/client/useSumitCheckout.ts b/src/client/useSumitCheckout.ts
index 10c47ce..a80a877 100644
--- a/src/client/useSumitCheckout.ts
+++ b/src/client/useSumitCheckout.ts
@@ -12,8 +12,10 @@ export interface UseSumitCheckoutResult {
   submit: () => void;
   reset: () => void;
   handleToken: (token: string) => void;
+  handleSuccess: () => void;
   handleError: (error: Error) => void;
   handleStart: () => void;
+  clearToken: () => void;
 }
 
 export function useSumitCheckout(): UseSumitCheckoutResult {
@@ -39,6 +41,12 @@ export function useSumitCheckout(): UseSumitCheckoutResult {
     setError(null);
   }, []);
 
+  const handleSuccess = useCallback(() => {
+    setToken(null);
+    setStatus("succeeded");
+    setError(null);
+  }, []);
+
   const handleError = useCallback((value: Error) => {
     setError(value);
     setStatus("failed");
@@ -47,7 +55,12 @@ export function useSumitCheckout(): UseSumitCheckoutResult {
   const handleStart = useCallback(() => {
     setStatus("submitting");
     setError(null);
+    setToken(null);
+  }, []);
+
+  const clearToken = useCallback(() => {
+    setToken(null);
   }, []);
 
-  return { ref, status, error, token, submit, reset, handleToken, handleError, handleStart };
+  return { ref, status, error, token, submit, reset, handleToken, handleSuccess, handleError, handleStart, clearToken };
 }
diff --git a/src/next/createChargeRoute.test.ts b/src/next/createChargeRoute.test.ts
index 7a2a825..095358b 100644
--- a/src/next/createChargeRoute.test.ts
+++ b/src/next/createChargeRoute.test.ts
@@ -87,6 +87,46 @@ describe("createSumitChargeRoute", () => {
     expect(json.ok).toBe(false);
   });
 
+  it("returns 502 when SUMIT responds with a non-2xx status", async () => {
+    const fetchMock = vi.fn(async () =>
+      new Response(JSON.stringify({ UserErrorMessage: "temporary provider failure" }), {
+        status: 503,
+        headers: { "content-type": "application/json" },
+      }),
+    );
+    const handler = createSumitChargeRoute({ companyId: 7, apiKey: "k", fetch: fetchMock as unknown as typeof fetch });
+    const response = await handler(jsonRequest(validBody));
+    expect(response.status).toBe(502);
+    const json = (await response.json()) as Record<string, unknown>;
+    expect(json.ok).toBe(false);
+    expect(json.error).toBe("SUMIT returned an unsuccessful response");
+  });
+
+  it("returns 502 when a direct charge response cannot be mapped to a billing result", async () => {
+    const fetchMock = vi.fn(async () =>
+      new Response(JSON.stringify({ unexpected: "shape" }), {
+        status: 200,
+        headers: { "content-type": "application/json" },
+      }),
+    );
+    const handler = createSumitChargeRoute({ companyId: 7, apiKey: "k", fetch: fetchMock as unknown as typeof fetch });
+    const response = await handler(jsonRequest(validBody));
+    expect(response.status).toBe(502);
+    const json = (await response.json()) as Record<string, unknown>;
+    expect(json.ok).toBe(false);
+    expect(json.error).toBe("SUMIT returned an unmapped charge response");
+  });
+
+  it("returns 400 when nested charge fields are invalid", async () => {
+    const fetchMock = vi.fn();
+    const handler = createSumitChargeRoute({ companyId: 7, apiKey: "k", fetch: fetchMock as unknown as typeof fetch });
+    const response = await handler(jsonRequest({ ...validBody, item: { ...validBody.item, unitPrice: "19" } }));
+    expect(response.status).toBe(400);
+    expect(fetchMock).not.toHaveBeenCalled();
+    const json = (await response.json()) as Record<string, unknown>;
+    expect(json.error).toContain("item.unitPrice");
+  });
+
   it("returns 502 when the upstream call throws", async () => {
     const fetchMock = vi.fn(async () => {
       throw new Error("network");
diff --git a/src/next/createChargeRoute.ts b/src/next/createChargeRoute.ts
index 95222e1..fb39232 100644
--- a/src/next/createChargeRoute.ts
+++ b/src/next/createChargeRoute.ts
@@ -65,6 +65,11 @@ export function createSumitChargeRoute(config: SumitChargeRouteConfig): SumitCha
       return jsonResponse({ ok: false, error: "Missing required fields: singleUseToken, customer, item" }, 400);
     }
 
+    const validationError = validateChargeRequestBody(parsed);
+    if (validationError) {
+      return jsonResponse({ ok: false, error: validationError }, 400);
+    }
+
     const payloadParams: BuildRecurringChargePayloadParams = {
       companyId: config.companyId,
       apiKey: config.apiKey,
@@ -85,12 +90,18 @@ export function createSumitChargeRoute(config: SumitChargeRouteConfig): SumitCha
         body: JSON.stringify(payload),
       });
       upstreamJson = await upstream.json().catch(() => null);
+      if (!upstream.ok) {
+        return jsonResponse({ ok: false, error: "SUMIT returned an unsuccessful response", upstreamStatus: upstream.status }, 502);
+      }
     } catch (error) {
       await safeCall(config.onError, error, request);
       return jsonResponse({ ok: false, error: "Upstream request to SUMIT failed" }, 502);
     }
 
     const event = normalizeRecurringChargeResponse(upstreamJson);
+    if (event.ok === null || event.eventType === "sumit.trigger.unmapped") {
+      return jsonResponse({ ok: false, error: "SUMIT returned an unmapped charge response", event: redactSumitPayload(event) }, 502);
+    }
     await safeCall(config.onResult, event, request);
 
     const status = event.ok === false ? 402 : 200;
@@ -98,6 +109,33 @@ export function createSumitChargeRoute(config: SumitChargeRouteConfig): SumitCha
   };
 }
 
+function validateChargeRequestBody(body: SumitChargeRequestBody): string | null {
+  if (!isNonEmptyString(body.singleUseToken)) return "singleUseToken must be a non-empty string";
+  if (!isNonEmptyString(body.customer.externalIdentifier)) return "customer.externalIdentifier must be a non-empty string";
+  if (!isNonEmptyString(body.customer.name)) return "customer.name must be a non-empty string";
+  if (!isNonEmptyString(body.customer.emailAddress)) return "customer.emailAddress must be a non-empty string";
+  if (!isNonEmptyString(body.item.name)) return "item.name must be a non-empty string";
+  if (!isNonEmptyString(body.item.description)) return "item.description must be a non-empty string";
+  if (!isPositiveFiniteNumber(body.item.unitPrice)) return "item.unitPrice must be a positive number";
+  if (!isPositiveFiniteNumber(body.item.durationMonths)) return "item.durationMonths must be a positive number";
+  if (!["ILS", "USD", "EUR", 0, 1, 2].includes(body.item.currency)) return "item.currency must be one of ILS, USD, EUR, 0, 1, 2";
+  if (body.item.quantity !== undefined && !isPositiveFiniteNumber(body.item.quantity)) return "item.quantity must be a positive number";
+  if (body.item.recurrence !== undefined && !isNonNegativeFiniteNumber(body.item.recurrence)) return "item.recurrence must be a non-negative number";
+  return null;
+}
+
+function isNonEmptyString(value: unknown): value is string {
+  return typeof value === "string" && value.trim().length > 0;
+}
+
+function isPositiveFiniteNumber(value: unknown): value is number {
+  return typeof value === "number" && Number.isFinite(value) && value > 0;
+}
+
+function isNonNegativeFiniteNumber(value: unknown): value is number {
+  return typeof value === "number" && Number.isFinite(value) && value >= 0;
+}
+
 function jsonResponse(body: unknown, status: number): Response {
   return new Response(JSON.stringify(body), {
     status,
diff --git a/src/next/createWebhookRoute.test.ts b/src/next/createWebhookRoute.test.ts
index 929cc24..369a858 100644
--- a/src/next/createWebhookRoute.test.ts
+++ b/src/next/createWebhookRoute.test.ts
@@ -97,12 +97,35 @@ describe("verifySumitSharedSecret", () => {
     expect(ok).toBe(true);
   });
 
-  it("accepts the secret from a query param", async () => {
+  it("rejects query-param secrets by default", async () => {
+    const verifier = verifySumitSharedSecret("s3cret");
+    const ok = await verifier(new Request("https://example.com/?secret=s3cret", { method: "POST" }));
+    expect(ok).toBe(false);
+  });
+
+  it("accepts the secret from a query param only when explicitly configured", async () => {
     const verifier = verifySumitSharedSecret("s3cret", { queryParam: "k" });
     const ok = await verifier(new Request("https://example.com/?k=s3cret", { method: "POST" }));
     expect(ok).toBe(true);
   });
 
+  it("normalizes multipart form-data payloads", async () => {
+    const onEvent = vi.fn();
+    const handler = createSumitWebhookRoute({ onEvent });
+    const body = new FormData();
+    body.set("Payment.Status", "000");
+    body.set("Payment.ValidPayment", "true");
+    body.set("Payment.ID", "p1");
+    body.set("RecurringCustomerItemIDs[0]", "444");
+
+    const response = await handler(new Request("https://example.com/api/sumit/webhook", { method: "POST", body }));
+
+    expect(response.status).toBe(200);
+    const [event] = onEvent.mock.calls[0] as [{ eventType: string; paymentId?: string }];
+    expect(event.eventType).toBe("recurring.charged");
+    expect(event.paymentId).toBe("p1");
+  });
+
   it("rejects mismatching secrets", async () => {
     const verifier = verifySumitSharedSecret("s3cret");
     const ok = await verifier(
diff --git a/src/next/createWebhookRoute.ts b/src/next/createWebhookRoute.ts
index b3d480f..ad9de17 100644
--- a/src/next/createWebhookRoute.ts
+++ b/src/next/createWebhookRoute.ts
@@ -53,10 +53,11 @@ export function createSumitWebhookRoute(config: SumitWebhookRouteConfig): SumitW
 
 export function verifySumitSharedSecret(secret: string, options: { header?: string; queryParam?: string } = {}): SumitWebhookVerifier {
   const headerName = (options.header ?? "x-sumit-secret").toLowerCase();
-  const queryParam = options.queryParam ?? "secret";
+  const queryParam = options.queryParam;
   return async (request) => {
     const headerValue = request.headers.get(headerName);
     if (headerValue && (await timingSafeEqual(headerValue, secret))) return true;
+    if (!queryParam) return false;
     const url = new URL(request.url);
     const queryValue = url.searchParams.get(queryParam);
     return Boolean(queryValue && (await timingSafeEqual(queryValue, secret)));
@@ -68,10 +69,13 @@ async function readPayload(request: Request): Promise<unknown> {
   if (contentType.includes("application/json")) {
     return (await request.json()) as unknown;
   }
-  if (contentType.includes("application/x-www-form-urlencoded") || contentType.includes("multipart/form-data")) {
+  if (contentType.includes("application/x-www-form-urlencoded")) {
     const body = await request.text();
     return new URLSearchParams(body);
   }
+  if (contentType.includes("multipart/form-data")) {
+    return formDataToSearchParams(await request.formData());
+  }
   const text = await request.text();
   if (!text) return {};
   try {
@@ -81,6 +85,14 @@ async function readPayload(request: Request): Promise<unknown> {
   }
 }
 
+function formDataToSearchParams(formData: FormData): URLSearchParams {
+  const params = new URLSearchParams();
+  formData.forEach((value, key) => {
+    params.append(key, typeof value === "string" ? value : value.name);
+  });
+  return params;
+}
+
 // Hash both inputs to a fixed-length digest before comparing so the comparison
 // is constant-time AND independent of secret length — a plain length check
 // leaks the byte-length of the secret via response timing.