From 01b47344774f240d4dfc093a679487853b900b04 Mon Sep 17 00:00:00 2001 From: Gerhard Scheikl Date: Sun, 31 May 2026 09:35:31 +0200 Subject: [PATCH] security hardening --- .dockerignore | 15 ++ Dockerfile | 75 ++++++- app/routes/api.public.girocode[.png].tsx | 7 +- app/routes/api.public.payment-info.tsx | 50 ++++- app/routes/app.settings.tsx | 15 +- app/routes/webhooks.orders.create.tsx | 69 +++--- app/routes/webhooks.orders.fulfilled.tsx | 58 +++-- app/routes/webhooks.orders.updated.tsx | 7 +- app/services/config/env.server.ts | 33 +++ app/services/crypto/fieldCrypto.server.ts | 84 +++++++ app/services/invoice/email.server.ts | 57 ++++- app/services/invoice/girocode.ts | 17 +- app/services/invoice/pdf/InvoiceDocument.tsx | 17 +- .../invoice/productImageCache.server.ts | 34 ++- app/services/invoice/safeFetch.server.ts | 119 +++++++--- app/services/invoice/signedUrl.ts | 24 +- .../session/encryptedSessionStorage.server.ts | 60 +++++ app/services/webhooks/background.server.ts | 115 +++++++++- app/services/webhooks/cleanup.server.ts | 63 ++++++ app/services/webhooks/dedupe.server.ts | 211 +++++++++++++++--- app/shopify.server.ts | 11 +- deploy/.env.dev.example | 16 ++ deploy/Caddyfile.snippet | 9 + deploy/README.md | 19 ++ deploy/docker-compose.dev.yml | 18 ++ package-lock.json | 94 +++++--- package.json | 13 +- .../migration.sql | 16 ++ prisma/schema.prisma | 7 + server.js | 51 ++++- tests/dedupe-and-language.test.ts | 88 +++++++- 31 files changed, 1234 insertions(+), 238 deletions(-) create mode 100644 app/services/config/env.server.ts create mode 100644 app/services/crypto/fieldCrypto.server.ts create mode 100644 app/services/session/encryptedSessionStorage.server.ts create mode 100644 app/services/webhooks/cleanup.server.ts create mode 100644 prisma/migrations/20260530205507_add_processed_webhook_status/migration.sql diff --git a/.dockerignore b/.dockerignore index 840bb3a..057cda4 100644 --- a/.dockerignore +++ b/.dockerignore @@ -8,9 +8,24 @@ node_modules !.env.production.example prisma/dev.sqlite prisma/dev.sqlite-journal +# Any local SQLite DB / journal / WAL must never enter the image. +prisma/*.sqlite* +prisma/dev.sqlite* +data/ .shopify .git .github *.log extensions/*/dist +# Dev-only tooling / docs — not needed at build or runtime. +# NOTE: prisma/schema.prisma and prisma/migrations are intentionally NOT +# excluded (required by `prisma generate` and `prisma migrate deploy`). +tests/ +scripts/ +.cursor/ +.gemini/ +.vscode/ +*.md +**/*.md + diff --git a/Dockerfile b/Dockerfile index 95b7099..09d0130 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,18 +1,83 @@ -FROM node:20-alpine +# syntax=docker/dockerfile:1 + +# --------------------------------------------------------------------------- +# Base image pin +# --------------------------------------------------------------------------- +# Pinned to a specific minor (20.19) so rebuilds are reproducible and satisfy +# the package.json `engines` constraint (">=20.19 <22 || >=22.12"). +# A digest pin is PREFERRED for full immutability, e.g.: +# FROM node:20.19-alpine@sha256: +# Add the real sha256 (from `docker buildx imagetools inspect node:20.19-alpine`) +# when you have network access. We do NOT invent a fake digest here. + +# =========================================================================== +# Stage 1 — builder: install ALL deps, generate Prisma client, build the app +# =========================================================================== +FROM node:20.19-alpine AS builder + +# openssl is required by Prisma's engines. RUN apk add --no-cache openssl -EXPOSE 3000 +WORKDIR /app + +# Install the full dependency tree (incl. devDependencies needed by the +# Vite / React Router build toolchain). NODE_ENV is intentionally left unset +# here so `npm ci` does not prune devDependencies. +COPY package.json package-lock.json* ./ +RUN npm ci + +# Copy the rest of the source and produce the production build + Prisma client. +COPY . . +RUN npx prisma generate \ + && npm run build + +# =========================================================================== +# Stage 2 — runtime: pruned prod deps + only the artifacts needed to run +# =========================================================================== +FROM node:20.19-alpine AS runtime + +# openssl for Prisma engines at runtime (migrate deploy / query engine). +RUN apk add --no-cache openssl WORKDIR /app ENV NODE_ENV=production +# Keep npm + Prisma incidental writes off the (dev) read-only root filesystem: +# both are redirected to the tmpfs-backed /tmp mount. +ENV NPM_CONFIG_CACHE=/tmp/.npm +ENV CHECKPOINT_DISABLE=1 +# Install ONLY production dependencies. COPY package.json package-lock.json* ./ - RUN npm ci --omit=dev && npm cache clean --force -COPY . . +# Prisma schema + migrations are needed for `prisma generate` (below) and for +# `prisma migrate deploy` on container start. +COPY --from=builder /app/prisma ./prisma +# Bake the generated Prisma client into the image so the container start +# command only has to run migrations (no generate at runtime → compatible with +# a read-only root filesystem). +RUN npx prisma generate -RUN npm run build +# Application artifacts required at runtime. +COPY --from=builder /app/build ./build +COPY --from=builder /app/public ./public +COPY --from=builder /app/server.js ./server.js +# Run as the unprivileged, pre-existing `node` user (uid/gid 1000). Ensure the +# app tree is owned by it. NOTE: the SQLite DB is written to the /data bind +# mount — the HOST directory mounted at /data MUST be chown'd to uid 1000 +# (see deploy/README.md), otherwise migrations/writes will fail as non-root. +RUN chown -R node:node /app +USER node + +EXPOSE 3000 + +# Container-level health probe hitting the unauthenticated /healthz route. +# busybox wget ships with node:alpine. +HEALTHCHECK --interval=30s --timeout=5s --start-period=20s --retries=3 \ + CMD wget -qO- http://127.0.0.1:3000/healthz || exit 1 + +# Functionally equivalent to today's start: runs DB migrations then the server. +# (`docker-start` no longer needs `prisma generate` — the client is baked above.) CMD ["npm", "run", "docker-start"] diff --git a/app/routes/api.public.girocode[.png].tsx b/app/routes/api.public.girocode[.png].tsx index 80fa2f1..ab22330 100644 --- a/app/routes/api.public.girocode[.png].tsx +++ b/app/routes/api.public.girocode[.png].tsx @@ -89,7 +89,12 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { headers: { "Content-Type": "image/png", "Cache-Control": "private, max-age=300", - "Access-Control-Allow-Origin": "*", + // No CORS header: the PNG is rendered via an tag in the + // checkout/customer-account extensions (see extensions/*/src/*.tsx), + // i.e. a plain image load, which is not subject to CORS. Dropping the + // previous `Access-Control-Allow-Origin: *` removes the ability for any + // origin to fetch() these bytes cross-origin while keeping the + // legitimate -style loads working. }, }); }; diff --git a/app/routes/api.public.payment-info.tsx b/app/routes/api.public.payment-info.tsx index 5451d46..a4f6e83 100644 --- a/app/routes/api.public.payment-info.tsx +++ b/app/routes/api.public.payment-info.tsx @@ -1,3 +1,4 @@ +import crypto from "node:crypto"; import type { LoaderFunctionArgs } from "react-router"; import { authenticate, unauthenticated } from "../shopify.server"; import db from "../db.server"; @@ -88,11 +89,13 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { } if (!orderInfo && lastErr) throw lastErr; } catch (err) { - const msg = (err as Error)?.message ?? String(err); - console.warn(`payment-info: failed to load order ${orderGid} for ${shop}:`, err); + // Log the upstream detail server-side only — never echo internal error + // messages (which may contain Admin API internals / order data) to the + // public client. + console.error(`payment-info: failed to load order ${orderGid} for ${shop}:`, err); return cors( Response.json( - { showPaymentInstructions: false, error: "order-load-failed", detail: msg.slice(0, 500) }, + { showPaymentInstructions: false, error: "order-load-failed" }, { status: 502 }, ), ); @@ -110,12 +113,28 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { // Without this, any authenticated buyer of the shop could enumerate // arbitrary orderIds and harvest the shop's bank details / amounts. // + // Token claims available (see @shopify/shopify-api `JwtPayload`): only the + // standard JWT fields — iss, dest, aud, sub, exp, nbf, iat, jti, sid. There + // is NO order- or checkout-scoped claim in either the checkout or the + // customer-account session token, so we cannot bind the token to the + // requested orderId directly. We therefore bind by customer identity where + // possible and fall back to a tightened recency window for true guests. + // // - customerAccount tokens always carry a customer GID in `sub`. We require - // that the order's customer matches. - // - Checkout (thank-you page) tokens are issued at the end of checkout and - // are short-lived. For logged-in buyers we still bind to the customer - // GID; for guest checkouts (no customer on the order) we fall back to a - // recency window (the order must have been placed in the last hour). + // that the order's customer matches (strong binding — preferred path). + // - Checkout (thank-you page) tokens for logged-in buyers also carry the + // customer GID in `sub`; we bind to it identically. + // - For guest checkouts (no customer on the order, checkout source only) we + // have nothing in the token to bind against. We accept only when the order + // was placed within a SHORT window — the thank-you page is rendered + // immediately after checkout, so a few minutes is ample for the legitimate + // flow. Residual risk: within this small window an attacker holding ANY + // valid checkout token for this shop could enumerate the numeric ids of + // very-recently-placed guest orders and read their amount/reference. This + // is mitigated (not eliminated) by (a) the short window, (b) per-IP rate + // limiting on /api/public/* (see server.js), and (c) numeric order ids + // being unguessable in the short window. The customer-account path remains + // the preferred, fully-bound surface. const tokenSub = (sessionToken?.sub ?? "").toString(); const tokenCustomerNumeric = tokenSub.replace(/^gid:\/\/shopify\/[^/]+\//, "").replace(/[^0-9]/g, ""); const orderCustomerNumeric = (orderInfo.customerId ?? "").replace(/^gid:\/\/shopify\/[^/]+\//, "").replace(/[^0-9]/g, ""); @@ -124,17 +143,24 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { if (orderCustomerNumeric && tokenCustomerNumeric) { ownershipOk = orderCustomerNumeric === tokenCustomerNumeric; } else if (authSource === "checkout" && !orderCustomerNumeric) { - // Guest checkout: no customer to bind against. Accept only if the order - // is fresh (i.e. the buyer has just completed checkout for it). + // Guest checkout: no customer to bind against. Accept only if the order is + // very fresh (the buyer has just completed checkout for it). Kept short to + // shrink the enumeration window — see residual-risk note above. const placedAtMs = orderInfo.processedAtMs ?? 0; - const RECENT_WINDOW_MS = 60 * 60 * 1000; // 1 hour + const RECENT_WINDOW_MS = 10 * 60 * 1000; // 10 minutes ownershipOk = placedAtMs > 0 && Date.now() - placedAtMs <= RECENT_WINDOW_MS; } if (!ownershipOk) { + // Minimal correlation log: never log raw customer identifiers (PII). Hash + // the token subject (sha256, truncated) so repeated abuse from the same + // principal is still correlatable without storing the GID itself. + const subHash = tokenSub + ? crypto.createHash("sha256").update(tokenSub).digest("hex").slice(0, 12) + : "-"; console.warn( `payment-info: ownership check failed for shop=${shop} order=${orderGid} ` + - `authSource=${authSource} tokenSub=${tokenSub || "-"}`, + `authSource=${authSource} subHash=${subHash}`, ); return cors( Response.json( diff --git a/app/routes/app.settings.tsx b/app/routes/app.settings.tsx index 108afb8..6fdf3dd 100644 --- a/app/routes/app.settings.tsx +++ b/app/routes/app.settings.tsx @@ -9,10 +9,12 @@ import { normaliseIban, } from "../services/invoice/validation"; import { STORED_LOGO_SENTINEL } from "../services/invoice/logoCache.constants"; +import { validateMerchantHttpsUrl } from "../services/invoice/safeFetch.server"; import { deleteStoredLogo, storeUploadedLogo, } from "../services/invoice/logoCache.server"; +import { encryptField } from "../services/crypto/fieldCrypto.server"; import { RichTextEditor } from "../components/RichTextEditor"; import { DEFAULT_EMAIL_BODY_DE, @@ -122,6 +124,13 @@ export const action = async ({ request }: ActionFunctionArgs) => { select: { logoUrl: true }, }); const submittedLogoUrl = str("logoUrl"); + // Validate any merchant-supplied external logo URL at the trust boundary: + // require a syntactically valid https URL whose host is a domain name, not + // an IP literal (SSRF defence-in-depth; safeFetch is the runtime backstop). + if (submittedLogoUrl && submittedLogoUrl !== STORED_LOGO_SENTINEL) { + const urlError = validateMerchantHttpsUrl(submittedLogoUrl); + if (urlError) errors.logo = urlError; + } const removeLogo = bool("removeLogo"); const logoFile = form.get("logoFile"); const hasUpload = @@ -154,13 +163,17 @@ export const action = async ({ request }: ActionFunctionArgs) => { const submittedSmtpPassword = str("smtpPassword"); let nextSmtpPassword: string; if (submittedSmtpPassword === SMTP_PASSWORD_SENTINEL) { + // Unchanged: keep the stored value as-is (already encrypted at rest). const current = await db.shopSettings.findUnique({ where: { shopDomain: session.shop }, select: { smtpPassword: true }, }); nextSmtpPassword = current?.smtpPassword ?? ""; } else { - nextSmtpPassword = submittedSmtpPassword; + // New password (including "" to clear). Encrypt non-empty values at rest. + nextSmtpPassword = submittedSmtpPassword + ? encryptField(submittedSmtpPassword) + : ""; } const data = { diff --git a/app/routes/webhooks.orders.create.tsx b/app/routes/webhooks.orders.create.tsx index 8598d0c..f02d4bc 100644 --- a/app/routes/webhooks.orders.create.tsx +++ b/app/routes/webhooks.orders.create.tsx @@ -5,7 +5,7 @@ import { generateAndEmailInvoice, isManualPaymentOrder, } from "../services/invoice/automations.server"; -import { isDuplicateWebhook } from "../services/webhooks/dedupe.server"; +import { reserveWebhook } from "../services/webhooks/dedupe.server"; import { runWebhookInBackground } from "../services/webhooks/background.server"; /** @@ -18,42 +18,57 @@ export const action = async ({ request }: ActionFunctionArgs) => { const { shop, topic, payload, session, admin } = await authenticate.webhook(request); console.log(`Received ${topic} webhook for ${shop}`); - // Drop Shopify retries we've already processed (prevents the duplicate - // invoice email we saw when the first delivery exceeded Shopify's 5s ack - // timeout — the work still completed, but Shopify resent the webhook). - if (await isDuplicateWebhook(request, shop, topic)) return new Response(); + // Reserve this delivery (status="processing"). `null` => already + // done/in-flight, so short-circuit. The reservation is committed only after + // the background work succeeds, and released on failure so Shopify's retry + // re-runs it (prevents the silent invoice loss we'd get if we recorded the + // id as processed before the slow PDF/email work). + const reservation = await reserveWebhook(request, shop, topic); + if (!reservation) return new Response(); - if (!session || !admin) return new Response(); + if (!session || !admin) { + // App uninstalled before the webhook drained — nothing to do. + await reservation.commit(); + return new Response(); + } const orderId = payload?.id; - if (orderId == null) return new Response(); + if (orderId == null) { + await reservation.commit(); + return new Response(); + } const customerLocale = typeof payload?.customer_locale === "string" ? payload.customer_locale : undefined; // Respond 200 immediately and run the (slow) PDF + email work in the - // background — keeps us well under Shopify's ~5s ack timeout. Dedupe - // above guarantees we don't double-process a retry while the first - // delivery is still working. - runWebhookInBackground(`${topic} order=${orderId} shop=${shop}`, async () => { - const settings = await db.shopSettings.findUnique({ where: { shopDomain: shop } }); - if (!settings?.autoEmailOnWireTransferPlaced) return; + // background — keeps us well under Shopify's ~5s ack timeout. The queue + // commits the reservation on success and releases it on failure. + runWebhookInBackground( + `${topic} order=${orderId} shop=${shop}`, + async () => { + const settings = await db.shopSettings.findUnique({ where: { shopDomain: shop } }); + if (!settings?.autoEmailOnWireTransferPlaced) return; - const orderGid = `gid://shopify/Order/${String(orderId).replace(/^.*\//, "")}`; - if (!(await isManualPaymentOrder(admin, orderGid))) return; + const orderGid = `gid://shopify/Order/${String(orderId).replace(/^.*\//, "")}`; + if (!(await isManualPaymentOrder(admin, orderGid))) return; - const result = await generateAndEmailInvoice({ - shopDomain: shop, - admin, - orderId, - customerLocale, - }); - if (!result.ok) { - console.warn( - `auto-email (wire-transfer placed) failed for order ${orderId} on ${shop}: ${result.reason}`, - ); - } - }); + const result = await generateAndEmailInvoice({ + shopDomain: shop, + admin, + orderId, + customerLocale, + }); + if (!result.ok) { + // Throw so the reservation is released and Shopify retries — don't + // swallow the failure (which would leave the invoice unsent forever). + throw new Error( + `auto-email (wire-transfer placed) failed for order ${orderId} on ${shop}: ${result.reason}`, + ); + } + }, + reservation, + ); return new Response(); }; diff --git a/app/routes/webhooks.orders.fulfilled.tsx b/app/routes/webhooks.orders.fulfilled.tsx index 201ca2d..9f11abd 100644 --- a/app/routes/webhooks.orders.fulfilled.tsx +++ b/app/routes/webhooks.orders.fulfilled.tsx @@ -5,7 +5,7 @@ import { generateAndEmailInvoice, isManualPaymentOrder, } from "../services/invoice/automations.server"; -import { isDuplicateWebhook } from "../services/webhooks/dedupe.server"; +import { reserveWebhook } from "../services/webhooks/dedupe.server"; import { runWebhookInBackground } from "../services/webhooks/background.server"; /** @@ -18,42 +18,56 @@ export const action = async ({ request }: ActionFunctionArgs) => { const { shop, topic, payload, session, admin } = await authenticate.webhook(request); console.log(`Received ${topic} webhook for ${shop}`); - // Idempotency against Shopify retries — see webhooks/dedupe.server.ts. - if (await isDuplicateWebhook(request, shop, topic)) return new Response(); + // Reserve/commit dedupe — see webhooks/dedupe.server.ts. `null` => already + // done/in-flight; commit on success / release on failure happen in the + // background queue so a failed send is retried by Shopify, not dropped. + const reservation = await reserveWebhook(request, shop, topic); + if (!reservation) return new Response(); if (!session || !admin) { // App was uninstalled before the webhook drained — nothing to do. + await reservation.commit(); return new Response(); } const orderId = payload?.id; - if (orderId == null) return new Response(); + if (orderId == null) { + await reservation.commit(); + return new Response(); + } const customerLocale = typeof payload?.customer_locale === "string" ? payload.customer_locale : undefined; // Respond fast; do the heavy lifting after the response (see notes in // webhooks.orders.create.tsx for the rationale). - runWebhookInBackground(`${topic} order=${orderId} shop=${shop}`, async () => { - const settings = await db.shopSettings.findUnique({ where: { shopDomain: shop } }); - if (!settings?.autoEmailOnFulfilledNonWireTransfer) return; + runWebhookInBackground( + `${topic} order=${orderId} shop=${shop}`, + async () => { + const settings = await db.shopSettings.findUnique({ where: { shopDomain: shop } }); + if (!settings?.autoEmailOnFulfilledNonWireTransfer) return; - const orderGid = `gid://shopify/Order/${String(orderId).replace(/^.*\//, "")}`; - if (await isManualPaymentOrder(admin, orderGid)) { - // Manual / wire-transfer order — handled by Automation 1, skip here. - return; - } + const orderGid = `gid://shopify/Order/${String(orderId).replace(/^.*\//, "")}`; + if (await isManualPaymentOrder(admin, orderGid)) { + // Manual / wire-transfer order — handled by Automation 1, skip here. + return; + } - const result = await generateAndEmailInvoice({ - shopDomain: shop, - admin, - orderId, - customerLocale, - }); - if (!result.ok) { - console.warn(`auto-email (fulfilled) failed for order ${orderId} on ${shop}: ${result.reason}`); - } - }); + const result = await generateAndEmailInvoice({ + shopDomain: shop, + admin, + orderId, + customerLocale, + }); + if (!result.ok) { + // Throw so the reservation is released and Shopify retries. + throw new Error( + `auto-email (fulfilled) failed for order ${orderId} on ${shop}: ${result.reason}`, + ); + } + }, + reservation, + ); return new Response(); }; diff --git a/app/routes/webhooks.orders.updated.tsx b/app/routes/webhooks.orders.updated.tsx index 1c47386..03f8692 100644 --- a/app/routes/webhooks.orders.updated.tsx +++ b/app/routes/webhooks.orders.updated.tsx @@ -1,6 +1,6 @@ import type { ActionFunctionArgs } from "react-router"; import { authenticate } from "../shopify.server"; -import { isDuplicateWebhook } from "../services/webhooks/dedupe.server"; +import { reserveWebhook } from "../services/webhooks/dedupe.server"; // Acknowledged but not yet acted on. Future: invalidate cached invoice // snapshots when a relevant field on the order changes. @@ -8,6 +8,9 @@ export const action = async ({ request }: ActionFunctionArgs) => { const { shop, topic } = await authenticate.webhook(request); console.log(`Received ${topic} webhook for ${shop}`); // Idempotency against Shopify retries — see webhooks/dedupe.server.ts. - if (await isDuplicateWebhook(request, shop, topic)) return new Response(); + const reservation = await reserveWebhook(request, shop, topic); + if (!reservation) return new Response(); + // No side-effect work yet, so the delivery is immediately complete. + await reservation.commit(); return new Response(); }; diff --git a/app/services/config/env.server.ts b/app/services/config/env.server.ts new file mode 100644 index 0000000..40e28e3 --- /dev/null +++ b/app/services/config/env.server.ts @@ -0,0 +1,33 @@ +/** + * Small env-access helpers that fail closed. + * + * `requireEnv` throws a clear error (without ever printing the secret value) + * when a required environment variable is missing or empty. `optionalEnv` + * returns the trimmed value or undefined. + */ + +/** + * Returns the value of `name` from `process.env`, throwing if it is unset or + * empty (after trimming). The secret value itself is never included in the + * error message. + */ +export function requireEnv(name: string): string { + const raw = process.env[name]; + if (raw === undefined || raw.trim() === "") { + throw new Error( + `Missing required environment variable "${name}". ` + + `Set it before starting the app (see deploy/.env.dev.example).`, + ); + } + return raw; +} + +/** + * Returns the value of `name` from `process.env`, or `undefined` if it is + * unset or empty (after trimming). + */ +export function optionalEnv(name: string): string | undefined { + const raw = process.env[name]; + if (raw === undefined || raw.trim() === "") return undefined; + return raw; +} diff --git a/app/services/crypto/fieldCrypto.server.ts b/app/services/crypto/fieldCrypto.server.ts new file mode 100644 index 0000000..64fef1b --- /dev/null +++ b/app/services/crypto/fieldCrypto.server.ts @@ -0,0 +1,84 @@ +/** + * Field-level encryption at rest using AES-256-GCM. + * + * Output format: `enc:v1:::` + * + * `decryptField` is backward-compatible: values that do not carry the + * `enc:v1:` prefix are assumed to be legacy plaintext and returned unchanged, + * so an existing (dev) database keeps working without a data migration. + */ +import crypto from "node:crypto"; + +import { requireEnv } from "../config/env.server"; + +const PREFIX = "enc:v1:"; +const IV_BYTES = 12; +const KEY_BYTES = 32; + +let cachedKey: Buffer | null = null; + +/** + * Loads and validates the 32-byte AES key from `DATA_ENCRYPTION_KEY` + * (base64-encoded). Cached after first use. Throws if unset or wrong length. + */ +function getKey(): Buffer { + if (cachedKey) return cachedKey; + const b64 = requireEnv("DATA_ENCRYPTION_KEY"); + let key: Buffer; + try { + key = Buffer.from(b64, "base64"); + } catch { + throw new Error('DATA_ENCRYPTION_KEY must be valid base64 of 32 bytes.'); + } + if (key.length !== KEY_BYTES) { + throw new Error( + `DATA_ENCRYPTION_KEY must decode to ${KEY_BYTES} bytes (got ${key.length}).`, + ); + } + cachedKey = key; + return key; +} + +/** Encrypts `plaintext` and returns the `enc:v1:...` envelope. */ +export function encryptField(plaintext: string): string { + const key = getKey(); + const iv = crypto.randomBytes(IV_BYTES); + const cipher = crypto.createCipheriv("aes-256-gcm", key, iv); + const ciphertext = Buffer.concat([ + cipher.update(plaintext, "utf8"), + cipher.final(), + ]); + const tag = cipher.getAuthTag(); + return ( + PREFIX + + iv.toString("base64") + + ":" + + tag.toString("base64") + + ":" + + ciphertext.toString("base64") + ); +} + +/** + * Decrypts an `enc:v1:...` envelope. If `value` is not in that format it is + * assumed to be legacy plaintext and returned unchanged. + */ +export function decryptField(value: string): string { + if (!value.startsWith(PREFIX)) return value; + const parts = value.slice(PREFIX.length).split(":"); + if (parts.length !== 3) { + throw new Error("Malformed encrypted field (expected iv:tag:ciphertext)."); + } + const [ivB64, tagB64, dataB64] = parts; + const key = getKey(); + const iv = Buffer.from(ivB64, "base64"); + const tag = Buffer.from(tagB64, "base64"); + const ciphertext = Buffer.from(dataB64, "base64"); + const decipher = crypto.createDecipheriv("aes-256-gcm", key, iv); + decipher.setAuthTag(tag); + const plaintext = Buffer.concat([ + decipher.update(ciphertext), + decipher.final(), + ]); + return plaintext.toString("utf8"); +} diff --git a/app/services/invoice/email.server.ts b/app/services/invoice/email.server.ts index cd7e0ea..373bd1a 100644 --- a/app/services/invoice/email.server.ts +++ b/app/services/invoice/email.server.ts @@ -12,6 +12,8 @@ import { } from "./emailTemplates"; import { STORED_LOGO_SENTINEL } from "./logoCache.constants"; import { safeFetch, SafeFetchError, SHOPIFY_CDN_HOSTS } from "./safeFetch.server"; +import { decryptField } from "../crypto/fieldCrypto.server"; +import { optionalEnv } from "../config/env.server"; export interface SendInvoiceEmailArgs { shopDomain: string; @@ -86,7 +88,7 @@ export async function sendInvoiceEmail( const customSubject = (language === "en" ? settings.emailSubjectEn : settings.emailSubjectDe) || (language === "en" ? DEFAULT_EMAIL_SUBJECT_EN : DEFAULT_EMAIL_SUBJECT_DE); - const subject = renderTemplate(customSubject, vars); + const subject = renderTemplate(customSubject, vars, { html: false }); const customBodyHtml = (language === "en" ? settings.emailBodyHtmlEn : settings.emailBodyHtmlDe) || @@ -124,10 +126,13 @@ export async function sendInvoiceEmail( } try { + // Optional archival BCC. Off by default for privacy/GDPR; set INVOICE_BCC + // to a comma-separated address list to opt in. + const bcc = optionalEnv("INVOICE_BCC"); const info = await transporter.sendMail({ from: `"${fromName}" <${fromEmail}>`, to, - bcc: "shop@linumiq.com", + ...(bcc ? { bcc } : {}), replyTo: settings.smtpReplyTo || undefined, subject, text: body.text, @@ -180,7 +185,7 @@ function buildTransport(settings: ShopSettings): Transporter { port: settings.smtpPort, secure: settings.smtpSecure, auth: settings.smtpUser - ? { user: settings.smtpUser, pass: settings.smtpPassword } + ? { user: settings.smtpUser, pass: decryptField(settings.smtpPassword) } : undefined, }); } @@ -286,13 +291,49 @@ function buildTemplateVars(args: { /** * Substitutes {{token}} placeholders in `template`. Unknown tokens are left - * in place so the user notices typos instead of silent blanks. Values are - * inserted verbatim — callers are responsible for HTML-escaping if needed. + * in place so the user notices typos instead of silent blanks. + * + * For HTML output (the default), every interpolated value is HTML-escaped to + * prevent stored-XSS from merchant- or customer-derived data bleeding into the + * email body. URL-valued tokens that land inside `href` attributes are scheme- + * validated first: `shopWebsite` must be an `https:` URL and `shopEmail` must + * look like a bare email address (rendered after a `mailto:` prefix); anything + * else renders empty so a hostile `javascript:`/`data:` value can't be planted. + * + * Pass `{ html: false }` for plain-text contexts (e.g. the subject line), where + * the raw value is substituted without HTML entity encoding. */ -function renderTemplate(template: string, vars: TemplateVars): string { +const URL_TOKENS = new Set(["shopWebsite"]); +const EMAIL_TOKENS = new Set(["shopEmail"]); + +function safeHttpsUrl(value: string): string { + const v = value.trim(); + try { + return new URL(v).protocol === "https:" ? v : ""; + } catch { + return ""; + } +} + +function safeEmailAddress(value: string): string { + const v = value.trim(); + return /^[^\s@<>"'/\\]+@[^\s@<>"'/\\]+\.[^\s@<>"'/\\]+$/.test(v) ? v : ""; +} + +function renderTemplate( + template: string, + vars: TemplateVars, + opts: { html?: boolean } = {}, +): string { + const html = opts.html !== false; // HTML-escape by default return template.replace(/\{\{\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*\}\}/g, (full, key) => { - const v = (vars as unknown as Record)[key]; - return v === undefined ? full : v; + const raw = (vars as unknown as Record)[key]; + if (raw === undefined) return full; + if (!html) return raw; + let value = raw; + if (URL_TOKENS.has(key)) value = safeHttpsUrl(raw); + else if (EMAIL_TOKENS.has(key)) value = safeEmailAddress(raw); + return escapeHtml(value); }); } diff --git a/app/services/invoice/girocode.ts b/app/services/invoice/girocode.ts index e11943e..12c6d77 100644 --- a/app/services/invoice/girocode.ts +++ b/app/services/invoice/girocode.ts @@ -18,6 +18,15 @@ export interface GiroCodeInput { remittance: string; } +/** + * Replaces CR/LF in a free-text EPC field with a single space and collapses + * runs of whitespace, so the line-delimited payload can't be tampered with by + * smuggling newlines into user-supplied text (beneficiary name / remittance). + */ +function sanitizeEpcField(value: string): string { + return value.replace(/[\r\n]+/g, " ").replace(/\s+/g, " ").trim(); +} + export function buildGiroCodePayload(input: GiroCodeInput): string { const currency = input.currency || "EUR"; if (currency !== "EUR") { @@ -25,12 +34,14 @@ export function buildGiroCodePayload(input: GiroCodeInput): string { console.warn(`GiroCode: non-EUR currency ${currency} is non-standard.`); } - // Beneficiary name max 70 chars per spec. - const name = input.beneficiaryName.slice(0, 70); + // Beneficiary name max 70 chars per spec. Strip CR/LF first so injected + // newlines can't forge/add EPC fields (the payload is line-delimited). + const name = sanitizeEpcField(input.beneficiaryName).slice(0, 70); const iban = input.iban.replace(/\s+/g, "").toUpperCase(); const bic = (input.bic || "").replace(/\s+/g, "").toUpperCase(); const amount = input.amount.toFixed(2); - const remittance = input.remittance.slice(0, 140); + // Unstructured remittance max 140 chars; strip CR/LF for the same reason. + const remittance = sanitizeEpcField(input.remittance).slice(0, 140); // Field order is fixed; trailing fields can be empty. // Service tag SCT = SEPA Credit Transfer. diff --git a/app/services/invoice/pdf/InvoiceDocument.tsx b/app/services/invoice/pdf/InvoiceDocument.tsx index 4f4115a..7857927 100644 --- a/app/services/invoice/pdf/InvoiceDocument.tsx +++ b/app/services/invoice/pdf/InvoiceDocument.tsx @@ -22,6 +22,21 @@ const TEXT_DARK = "#1F2933"; const TEXT_MUTED = "#6B7280"; const TABLE_BORDER = "#E5E7EB"; +/** + * Returns true only for syntactically valid http(s) URLs. Used to gate + * carrier/fulfillment-supplied tracking URLs before embedding them as PDF + * link annotations, so non-http schemes (javascript:, file:, data:, …) can't + * be smuggled into the document. + */ +function isHttpUrl(value: string): boolean { + try { + const u = new URL(value); + return u.protocol === "https:" || u.protocol === "http:"; + } catch { + return false; + } +} + const styles = StyleSheet.create({ page: { paddingTop: 40, @@ -348,7 +363,7 @@ export function InvoiceDocument({ invoice }: DocProps) { {t.trackingLabel} {tr.company ? ` (${tr.company})` : ""} - {tr.url ? ( + {tr.url && isHttpUrl(tr.url) ? ( {tr.number} ) : ( {tr.number} diff --git a/app/services/invoice/productImageCache.server.ts b/app/services/invoice/productImageCache.server.ts index 98e1bce..1fc69dc 100644 --- a/app/services/invoice/productImageCache.server.ts +++ b/app/services/invoice/productImageCache.server.ts @@ -7,9 +7,14 @@ * hammering the network when regenerating an invoice multiple times. */ import { safeFetch, SafeFetchError, SHOPIFY_CDN_HOSTS } from "./safeFetch.server"; +import pLimit from "p-limit"; const MAX_BYTES = 2 * 1024 * 1024; // 2 MB cap per image const CACHE_MAX_ENTRIES = 200; +/** Max images fetched/embedded per invoice (DoS bound for large carts). */ +const MAX_IMAGES_PER_INVOICE = 100; +/** Max concurrent image fetches per invoice. */ +const IMAGE_FETCH_CONCURRENCY = 6; const cache = new Map(); // url -> data URL @@ -76,17 +81,28 @@ export async function fetchProductImageDataUrl(url: string): Promise { - await Promise.all( - lines.map(async (line) => { - if (!line.imageUrl) return; - const dataUrl = await fetchProductImageDataUrl(line.imageUrl); - if (dataUrl) line.imageDataUrl = dataUrl; - }), - ); + const limit = pLimit(IMAGE_FETCH_CONCURRENCY); + let budget = MAX_IMAGES_PER_INVOICE; + const tasks: Promise[] = []; + for (const line of lines) { + if (!line.imageUrl) continue; + if (budget <= 0) break; // cap reached — remaining rows render iconless + budget -= 1; + tasks.push( + limit(async () => { + const dataUrl = await fetchProductImageDataUrl(line.imageUrl!); + if (dataUrl) line.imageDataUrl = dataUrl; + }), + ); + } + await Promise.all(tasks); } diff --git a/app/services/invoice/safeFetch.server.ts b/app/services/invoice/safeFetch.server.ts index df906cc..c0d3ab2 100644 --- a/app/services/invoice/safeFetch.server.ts +++ b/app/services/invoice/safeFetch.server.ts @@ -25,6 +25,7 @@ import { Agent as HttpAgent } from "node:http"; import { Agent as HttpsAgent } from "node:https"; import http from "node:http"; import https from "node:https"; +import ipaddr from "ipaddr.js"; export interface SafeFetchOptions { /** Hard cap in bytes; the read aborts as soon as this is exceeded. */ @@ -60,43 +61,62 @@ export class SafeFetchError extends Error { const DEFAULT_TIMEOUT_MS = 5_000; const DEFAULT_MAX_BYTES = 8 * 1024 * 1024; // 8 MB -function isPrivateIpv4(ip: string): boolean { - const parts = ip.split(".").map((n) => parseInt(n, 10)); - if (parts.length !== 4 || parts.some((n) => !Number.isFinite(n) || n < 0 || n > 255)) { - // Treat malformed addresses as unsafe. - return true; +/** + * Default-deny address classifier backed by the well-vetted `ipaddr.js` + * library. An address is considered safe to connect to ONLY if it is a + * clearly public, globally-routable unicast address. Everything else — + * loopback, private (RFC1918), link-local, unique-local, multicast, + * reserved, unspecified, broadcast, carrier-grade NAT, plus the various + * IPv4-in-IPv6 tunnelling/transition forms — is rejected. + * + * This closes IPv6 bypasses that string-prefix checks miss, e.g.: + * - `::ffff:7f00:1` (IPv4-mapped HEX form of 127.0.0.1) + * - `::7f00:1` (deprecated IPv4-compatible ::127.0.0.1) + * - `fe90::` / `fea0::` / `feb0::` (link-local is fe80::/10, not just fe80:) + */ +function isSafePublicAddress(ip: string): boolean { + let addr: ipaddr.IPv4 | ipaddr.IPv6; + try { + addr = ipaddr.parse(ip); + } catch { + // Unparseable => treat as unsafe. + return false; } - const [a, b] = parts; - if (a === 10) return true; - if (a === 127) return true; - if (a === 0) return true; - if (a === 169 && b === 254) return true; // link-local + AWS metadata - if (a === 172 && b >= 16 && b <= 31) return true; - if (a === 192 && b === 168) return true; - if (a === 192 && b === 0) return true; // 192.0.0.0/24, 192.0.2.0/24 - if (a === 198 && (b === 18 || b === 19)) return true; - if (a >= 224) return true; // multicast / reserved - return false; + + if (addr.kind() === "ipv4") { + // Only globally-routable unicast IPv4 is allowed. `range()` returns + // 'unicast' exclusively for public space; private/loopback/linkLocal/ + // carrierGradeNat/reserved/broadcast/multicast/unspecified are all denied. + return (addr as ipaddr.IPv4).range() === "unicast"; + } + + const v6 = addr as ipaddr.IPv6; + + // Unwrap IPv4-mapped (::ffff:a.b.c.d, incl. hex form ::ffff:7f00:1) and + // validate the embedded IPv4 against the v4 policy. + if (v6.isIPv4MappedAddress()) { + return v6.toIPv4Address().range() === "unicast"; + } + + // Deprecated IPv4-compatible addresses live in ::/96 (first 96 bits zero, + // e.g. ::7f00:1 == ::127.0.0.1). ipaddr.js classifies these as plain + // 'unicast', so unwrap the trailing 32 bits and validate as IPv4. This + // also covers :: (unspecified) and ::1 (loopback), which map to + // 0.0.0.0 / 0.0.0.1 and are denied by the IPv4 policy. + const p = v6.parts; + if (p[0] === 0 && p[1] === 0 && p[2] === 0 && p[3] === 0 && p[4] === 0 && p[5] === 0) { + const v4 = new ipaddr.IPv4([(p[6] >> 8) & 0xff, p[6] & 0xff, (p[7] >> 8) & 0xff, p[7] & 0xff]); + return v4.range() === "unicast"; + } + + // Everything else: only true global unicast is allowed. This rejects + // loopback, linkLocal (fe80::/10), uniqueLocal (fc00::/7), multicast, + // reserved, 6to4, teredo, rfc6145/rfc6052 transition ranges, etc. + return v6.range() === "unicast"; } -function isPrivateIpv6(ip: string): boolean { - const lower = ip.toLowerCase(); - if (lower === "::1" || lower === "::") return true; - if (lower.startsWith("fe80:")) return true; // link-local - if (lower.startsWith("fc") || lower.startsWith("fd")) return true; // ULA - if (lower.startsWith("ff")) return true; // multicast - // IPv4-mapped: ::ffff:a.b.c.d — apply IPv4 rules - if (lower.startsWith("::ffff:")) { - const v4 = lower.slice(7); - if (v4.includes(".")) return isPrivateIpv4(v4); - } - return false; -} - -function isPrivateAddress(ip: string, family: number): boolean { - if (family === 4) return isPrivateIpv4(ip); - if (family === 6) return isPrivateIpv6(ip); - return true; +function isPrivateAddress(ip: string): boolean { + return !isSafePublicAddress(ip); } function hostMatchesAllowlist(hostname: string, allowed: string[] | undefined): boolean { @@ -117,7 +137,7 @@ async function resolveSafeAddress(hostname: string): Promise<{ address: string; // If the hostname is already an IP literal, validate it directly. if (net.isIP(hostname)) { const family = net.isIPv6(hostname) ? 6 : 4; - if (isPrivateAddress(hostname, family)) { + if (isPrivateAddress(hostname)) { throw new SafeFetchError("blocked-address", `Refusing to connect to private address ${hostname}`); } return { address: hostname, family }; @@ -129,7 +149,7 @@ async function resolveSafeAddress(hostname: string): Promise<{ address: string; throw new SafeFetchError("dns-failed", `DNS lookup failed for ${hostname}: ${(err as Error).message}`); } for (const r of results) { - if (isPrivateAddress(r.address, r.family)) { + if (isPrivateAddress(r.address)) { throw new SafeFetchError("blocked-address", `${hostname} resolves to private address ${r.address}`); } } @@ -269,3 +289,30 @@ export async function safeFetch(rawUrl: string, opts: SafeFetchOptions = {}): Pr /** Common allowlist for Shopify-served assets (CDN + Files). */ export const SHOPIFY_CDN_HOSTS = ["cdn.shopify.com", "shopifycdn.com", "shopify.com"]; + +/** + * Boundary validation for merchant-supplied URLs (e.g. the logo URL saved in + * settings). Requires a syntactically valid `https:` URL whose host is a DNS + * name rather than an IP literal (v4 or v6). Returns a user-facing error + * string when the URL is unacceptable, or `null` when it is fine to store. + * + * This is a defence-in-depth boundary check; `safeFetch` remains the runtime + * backstop that re-validates the resolved address at fetch time. + */ +export function validateMerchantHttpsUrl(raw: string): string | null { + let url: URL; + try { + url = new URL(raw); + } catch { + return "Enter a valid URL including the https:// prefix."; + } + if (url.protocol !== "https:") { + return "Logo URL must use https://."; + } + // URL.hostname wraps IPv6 literals in brackets; strip them before checking. + const host = url.hostname.replace(/^\[/, "").replace(/\]$/, ""); + if (net.isIP(host) !== 0) { + return "Logo URL must point to a domain name, not an IP address."; + } + return null; +} diff --git a/app/services/invoice/signedUrl.ts b/app/services/invoice/signedUrl.ts index f90534b..29eea7a 100644 --- a/app/services/invoice/signedUrl.ts +++ b/app/services/invoice/signedUrl.ts @@ -1,9 +1,29 @@ import crypto from "node:crypto"; -const SECRET = process.env.SHOPIFY_API_SECRET || ""; +import { optionalEnv } from "../config/env.server"; + +/** + * Resolves the GiroCode URL signing key lazily (per call, not at module load) + * so the process can boot even when only the fallback secret is present. + * + * Prefers the dedicated `GIROCODE_SIGNING_KEY`; falls back to + * `SHOPIFY_API_SECRET` ONLY when the dedicated key is unset, so existing + * signed URLs and deployments keep working. Throws if neither is set + * (fail closed) — an empty key would make signatures forgeable. + */ +function getSigningKey(): string { + const key = optionalEnv("GIROCODE_SIGNING_KEY") ?? optionalEnv("SHOPIFY_API_SECRET"); + if (!key) { + throw new Error( + "GiroCode signing key missing: set GIROCODE_SIGNING_KEY (preferred) " + + "or SHOPIFY_API_SECRET.", + ); + } + return key; +} function hmac(payload: string): string { - return crypto.createHmac("sha256", SECRET).update(payload).digest("hex"); + return crypto.createHmac("sha256", getSigningKey()).update(payload).digest("hex"); } export interface GiroCodeUrlParams { diff --git a/app/services/session/encryptedSessionStorage.server.ts b/app/services/session/encryptedSessionStorage.server.ts new file mode 100644 index 0000000..53f50a0 --- /dev/null +++ b/app/services/session/encryptedSessionStorage.server.ts @@ -0,0 +1,60 @@ +/** + * Thin wrapper around `@shopify/shopify-app-session-storage-prisma` that + * encrypts `accessToken` / `refreshToken` at rest using field-level AES-256-GCM. + * + * Tokens are encrypted before being handed to the underlying storage and + * decrypted after they are loaded back out. `decryptField` is backward + * compatible, so any legacy plaintext tokens already in the DB keep working. + */ +import { PrismaSessionStorage } from "@shopify/shopify-app-session-storage-prisma"; +import type { Session } from "@shopify/shopify-api"; +import type { PrismaClient } from "@prisma/client"; + +import { decryptField, encryptField } from "../crypto/fieldCrypto.server"; + +type SessionTokenFields = { + accessToken?: string; + refreshToken?: string; +}; + +function encryptTokens(session: Session): Session { + const s = session as Session & SessionTokenFields; + if (s.accessToken) s.accessToken = encryptField(s.accessToken); + if (s.refreshToken) s.refreshToken = encryptField(s.refreshToken); + return session; +} + +function decryptTokens(session: Session): Session { + const s = session as Session & SessionTokenFields; + if (s.accessToken) s.accessToken = decryptField(s.accessToken); + if (s.refreshToken) s.refreshToken = decryptField(s.refreshToken); + return session; +} + +/** + * Clones a Session so we never mutate the caller's instance when encrypting + * for storage. The Prisma session storage only reads plain properties, so a + * shallow structured copy via the Session class is sufficient. + */ +function cloneSession(session: Session): Session { + return Object.assign( + Object.create(Object.getPrototypeOf(session)), + session, + ) as Session; +} + +export class EncryptedPrismaSessionStorage extends PrismaSessionStorage { + async storeSession(session: Session): Promise { + return super.storeSession(encryptTokens(cloneSession(session))); + } + + async loadSession(id: string): Promise { + const session = await super.loadSession(id); + return session ? decryptTokens(session) : session; + } + + async findSessionsByShop(shop: string): Promise { + const sessions = await super.findSessionsByShop(shop); + return sessions.map((s) => decryptTokens(s)); + } +} diff --git a/app/services/webhooks/background.server.ts b/app/services/webhooks/background.server.ts index 37c77d7..fd9117f 100644 --- a/app/services/webhooks/background.server.ts +++ b/app/services/webhooks/background.server.ts @@ -1,27 +1,118 @@ +import pLimit from "p-limit"; +import type { WebhookReservation } from "./dedupe.server"; + /** - * Fire-and-forget runner for webhook side-effects. + * Background runner for webhook side-effects. * * Shopify expects a 200 response within ~5 seconds, otherwise it considers * the delivery failed and retries it. Heavy automation work (PDF render, * Shopify Files upload, SMTP send) routinely exceeded that budget, which * caused duplicate invoice emails before we added the dedupe table. * - * Returning the response immediately and letting the work finish in the - * background keeps Shopify happy. Combined with the dedupe table this is - * defence-in-depth: dedupe ensures *correctness* even if a retry sneaks - * through, while async processing makes retries unlikely in the first - * place. + * Returning the response immediately and finishing the work afterwards keeps + * Shopify happy. Two problems with a naive `void work()`: * - * Errors are caught and logged \u2014 they cannot reach a dispatcher because - * the HTTP response is already gone. + * 1. DoS / resource exhaustion — an order burst would spawn unbounded + * concurrent PDF renders + SMTP sends. We cap concurrency with a small + * in-process queue (`p-limit`); excess tasks queue instead of piling up. + * 2. Data loss on restart — `void work()` is invisible to shutdown, so a + * container stop (SIGTERM) killed in-flight invoice work mid-send. We + * track in-flight tasks and drain them (bounded) on SIGTERM/SIGINT. + * + * Reserve/commit dedupe (see dedupe.server.ts) is integrated here: on success + * we `commit()` the reservation (permanently deduped); on failure we + * `release()` it so Shopify's retry re-runs the work instead of being dropped + * as a duplicate. */ + +const CONCURRENCY = Math.max(1, Number(process.env.WEBHOOK_CONCURRENCY) || 4); +const DRAIN_TIMEOUT_MS = Math.max( + 1000, + Number(process.env.WEBHOOK_DRAIN_TIMEOUT_MS) || 25_000, +); + +const limit = pLimit(CONCURRENCY); +const inFlight = new Set>(); +let draining = false; + export function runWebhookInBackground( description: string, work: () => Promise, + reservation?: WebhookReservation | null, ): void { - // `void` so we don't accidentally `await` the floating promise; the - // node event loop keeps the task alive until it settles. - void work().catch((err) => { - console.error(`background webhook task '${description}' failed:`, err); + if (draining) { + // The process is shutting down. We still enqueue so the drain awaits this + // task — the server has already stopped listening, so this is at most the + // tail end of the last accepted request. + console.warn(`[webhook-queue] enqueuing task during shutdown drain: ${description}`); + } + + const task = limit(async () => { + try { + await work(); + await reservation?.commit(); + } catch (err) { + console.error(`background webhook task '${description}' failed:`, err); + // Drop the dedupe reservation so Shopify's retry re-runs the work. + try { + await reservation?.release(); + } catch (releaseErr) { + console.error( + `background webhook task '${description}': failed to release dedupe reservation:`, + releaseErr, + ); + } + } + }); + + inFlight.add(task); + void task.finally(() => inFlight.delete(task)); +} + +/** + * Stop accepting new work (best-effort) and await in-flight + queued tasks, + * bounded by `timeoutMs`, so a container stop drains invoice work instead of + * killing it mid-send. Idempotent. + */ +export async function drainWebhookQueue(timeoutMs = DRAIN_TIMEOUT_MS): Promise { + draining = true; + if (inFlight.size === 0) return; + + console.log( + `[webhook-queue] draining ${inFlight.size} in-flight webhook task(s) (timeout ${timeoutMs}ms)...`, + ); + + let timer: ReturnType | undefined; + const timeout = new Promise((resolve) => { + timer = setTimeout(resolve, timeoutMs); + if (typeof timer.unref === "function") timer.unref(); + }); + + await Promise.race([Promise.allSettled([...inFlight]), timeout]); + if (timer) clearTimeout(timer); + + if (inFlight.size > 0) { + console.warn( + `[webhook-queue] drain timed out with ${inFlight.size} task(s) still running`, + ); + } else { + console.log("[webhook-queue] drain complete"); + } +} + +// Bridge for the custom server (server.js), which loads only the bundled +// build and cannot import this module directly. It awaits this drain before +// calling process.exit during graceful shutdown. +type DrainGlobal = typeof globalThis & { + __linumiqWebhookDrain?: typeof drainWebhookQueue; +}; +(globalThis as DrainGlobal).__linumiqWebhookDrain = drainWebhookQueue; + +// Safety net for runtimes that don't go through server.js (e.g. `shopify app +// dev`): stop accepting work and best-effort drain. The custom server awaits +// the same (idempotent) drain before exiting. +for (const signal of ["SIGTERM", "SIGINT"] as const) { + process.once(signal, () => { + void drainWebhookQueue(); }); } diff --git a/app/services/webhooks/cleanup.server.ts b/app/services/webhooks/cleanup.server.ts new file mode 100644 index 0000000..205c6cd --- /dev/null +++ b/app/services/webhooks/cleanup.server.ts @@ -0,0 +1,63 @@ +import db from "../../db.server"; + +/** + * Periodic TTL cleanup for the `ProcessedWebhook` idempotency table. + * + * The table grows by one row per Shopify webhook delivery and is never read + * after the retry window closes, so without pruning it grows unbounded — + * eventually a disk/space DoS. We only need rows for as long as Shopify might + * retry a delivery (hours), so a generous retention window of a few days is + * ample while keeping the table small. + */ +const RETENTION_MS = 7 * 24 * 60 * 60 * 1000; // 7 days +const INTERVAL_MS = 60 * 60 * 1000; // hourly + +export interface CleanupDeps { + db: { + processedWebhook: { + deleteMany: (args: { + where: { receivedAt: { lt: Date } }; + }) => Promise<{ count: number }>; + }; + }; +} + +let scheduled = false; + +async function runCleanup(deps: CleanupDeps): Promise { + try { + const cutoff = new Date(Date.now() - RETENTION_MS); + const { count } = await deps.db.processedWebhook.deleteMany({ + where: { receivedAt: { lt: cutoff } }, + }); + if (count > 0) { + console.log(`webhook-cleanup: removed ${count} ProcessedWebhook row(s) older than 7d`); + } + } catch (err) { + // Best-effort housekeeping — never throw into the caller. + console.warn("webhook-cleanup: prune failed:", err); + } +} + +/** + * Idempotently schedule the hourly cleanup. Safe to call on every webhook — + * the first call starts a single unref'd interval and runs an immediate + * sweep; subsequent calls are no-ops. + * + * Because this is only ever invoked while handling a live webhook request, it + * never runs during `prisma generate` / `react-router build` or other CLI + * contexts. The interval is `unref`'d so it can never keep the process alive. + */ +export function ensureWebhookCleanupScheduled(deps: CleanupDeps = { db }): void { + if (scheduled) return; + scheduled = true; + + const timer = setInterval(() => { + void runCleanup(deps); + }, INTERVAL_MS); + // Don't let the housekeeping interval keep the event loop alive on shutdown. + if (typeof timer.unref === "function") timer.unref(); + + // Kick off an immediate sweep so a long-lived process prunes promptly. + void runCleanup(deps); +} diff --git a/app/services/webhooks/dedupe.server.ts b/app/services/webhooks/dedupe.server.ts index 272e07c..78d0205 100644 --- a/app/services/webhooks/dedupe.server.ts +++ b/app/services/webhooks/dedupe.server.ts @@ -1,67 +1,204 @@ import db from "../../db.server"; +import { ensureWebhookCleanupScheduled } from "./cleanup.server"; /** - * Minimal shape of the Prisma client surface we use — declared inline so - * the helper can be unit-tested with a tiny stub instead of pulling in a - * real database. + * How long a `status="processing"` reservation is considered "live" before we + * assume the worker that claimed it crashed mid-process. After this window a + * stale reservation may be reclaimed and the work retried. + */ +const STALE_LEASE_MS = 5 * 60 * 1000; // 5 minutes + +interface ProcessedRow { + webhookId: string; + status: string; + receivedAt: Date; +} + +/** + * Minimal shape of the Prisma client surface we use — declared inline so the + * helper can be unit-tested with a tiny stub instead of a real database. */ export interface DedupeDeps { db: { processedWebhook: { create: (args: { - data: { webhookId: string; topic: string; shopDomain: string }; + data: { webhookId: string; topic: string; shopDomain: string; status: string }; }) => Promise; + findUnique: (args: { where: { webhookId: string } }) => Promise; + update: (args: { + where: { webhookId: string }; + data: { status?: string; receivedAt?: Date }; + }) => Promise; + delete: (args: { where: { webhookId: string } }) => Promise; }; }; } /** - * Returns `true` when this Shopify webhook delivery has already been - * processed and the caller should short-circuit without doing the work. + * A claim on a single Shopify webhook delivery. Obtained from + * {@link reserveWebhook}. The caller MUST eventually `commit()` (work + * succeeded — the delivery is permanently deduped) or `release()` (work + * failed — drop the reservation so Shopify's retry re-runs the work). * - * Shopify retries webhook deliveries when it doesn't receive a 200 within - * its (~5s) timeout window. Without dedupe this caused us to email an - * invoice twice for the same order: the first slow delivery completed its - * work but Shopify timed out and re-sent the webhook, which then ran the - * automation a second time. - * - * We key on the `X-Shopify-Webhook-Id` header — Shopify guarantees the same - * value for retries of the same delivery, but a new value for genuinely - * new events. The insert is the lock: a unique-constraint violation - * (Prisma error code `P2002`) means another delivery already claimed this - * id. + * `commit`/`release` are no-ops for reservations without a webhook id (unit + * tests / non-Shopify callers) and for the fail-open path. */ -export async function isDuplicateWebhook( +export interface WebhookReservation { + webhookId: string | null; + commit: () => Promise; + release: () => Promise; +} + +function noopReservation(webhookId: string | null): WebhookReservation { + return { + webhookId, + commit: async () => {}, + release: async () => {}, + }; +} + +function isP2002(err: unknown): boolean { + // Duck-typed so callers can stub the db without pulling in the real + // `Prisma` namespace. P2002 = unique-constraint violation. + return (err as { code?: string } | null)?.code === "P2002"; +} + +function makeReservation( + webhookId: string, + shop: string, + topic: string, + deps: DedupeDeps, +): WebhookReservation { + return { + webhookId, + commit: async () => { + try { + await deps.db.processedWebhook.update({ + where: { webhookId }, + data: { status: "done" }, + }); + } catch (err) { + // The work already succeeded; a failed commit just risks a later + // duplicate (which the side-effect code is expected to tolerate). + console.warn(`dedupe: failed to commit webhook ${webhookId} (${topic}/${shop}):`, err); + } + }, + release: async () => { + try { + await deps.db.processedWebhook.delete({ where: { webhookId } }); + } catch (err) { + console.warn(`dedupe: failed to release webhook ${webhookId} (${topic}/${shop}):`, err); + } + }, + }; +} + +/** + * Reserve this Shopify webhook delivery for processing. + * + * Shopify retries a delivery (re-using the same `X-Shopify-Webhook-Id`) when + * it doesn't receive a 200 within its ~5s timeout. Naively recording the id as + * "processed" *before* doing the work meant that if the heavy background work + * later failed (SMTP/GraphQL/PDF error), Shopify's retry was dropped as a + * duplicate and the invoice was never sent. + * + * This uses a two-phase reserve/commit keyed on the webhook id, with the + * unique `webhookId` primary key as the concurrency lock: + * + * - RESERVE: insert a `status="processing"` row. A unique-constraint + * violation (`P2002`) means the id is already claimed; we then inspect the + * existing row: + * - `done` → genuine duplicate → return `null` (skip). + * - `processing`, fresh → another delivery is in flight → `null`. + * - `processing`, stale → previous worker crashed → reclaim & retry. + * - COMMIT (caller, on success) → flip the row to `status="done"`. + * - RELEASE (caller, on failure) → delete the row so a retry reprocesses. + * + * Returns a {@link WebhookReservation} when the caller should process the + * delivery, or `null` when it must short-circuit (duplicate / concurrent). + * + * Fail-open: a dedupe-table error (other than P2002) never silently drops a + * webhook — we return a no-op reservation and let the work run. + */ +export async function reserveWebhook( request: Request, shop: string, topic: string, deps: DedupeDeps = { db }, -): Promise { +): Promise { + // Opportunistically schedule TTL cleanup (runtime-only; never in build/CLI + // since this is reached only while handling a live webhook request). + ensureWebhookCleanupScheduled(); + const webhookId = request.headers.get("x-shopify-webhook-id"); if (!webhookId) { - // Defensive: in unit tests / non-Shopify callers there is no id. - // Don't dedupe — that would silently drop legitimate calls. - return false; + // No id (unit tests / non-Shopify callers): process without dedupe. + return noopReservation(null); } + + const reservation = makeReservation(webhookId, shop, topic, deps); + try { await deps.db.processedWebhook.create({ - data: { webhookId, topic, shopDomain: shop }, + data: { webhookId, topic, shopDomain: shop, status: "processing" }, }); - return false; + return reservation; } catch (err) { - // Duck-typed P2002 check so callers can stub the db without pulling - // in the real `Prisma` namespace. - if ((err as { code?: string } | null)?.code === "P2002") { - console.log( - `dedupe: skipping duplicate ${topic} delivery for ${shop} (webhookId=${webhookId})`, - ); - return true; + if (!isP2002(err)) { + // Don't fail (or silently drop) a webhook on a logging-table issue. + console.warn(`dedupe: failed to reserve webhook ${webhookId} (${topic}/${shop}):`, err); + return noopReservation(webhookId); } - // Don't fail the webhook on a logging-table issue; just process it. - console.warn( - `dedupe: failed to record webhook ${webhookId} (${topic}/${shop}):`, - err, - ); - return false; } + + // A row already exists. Classify it. + let existing: ProcessedRow | null = null; + try { + existing = await deps.db.processedWebhook.findUnique({ where: { webhookId } }); + } catch (err) { + console.warn(`dedupe: failed to load existing webhook ${webhookId} (${topic}/${shop}):`, err); + // Another worker owns the row and we can't classify it — be safe and skip. + return null; + } + + if (!existing) { + // Raced with a release/delete between create() and findUnique(); reclaim. + return reservation; + } + + if (existing.status === "done") { + console.log( + `dedupe: skipping already-processed ${topic} for ${shop} (webhookId=${webhookId})`, + ); + return null; + } + + const age = Date.now() - new Date(existing.receivedAt).getTime(); + if (age > STALE_LEASE_MS) { + // The worker that reserved this crashed mid-process (or left a stale row). + // Renew the lease and retry the work. + try { + await deps.db.processedWebhook.update({ + where: { webhookId }, + data: { status: "processing", receivedAt: new Date() }, + }); + } catch (err) { + console.warn(`dedupe: failed to reclaim stale webhook ${webhookId}:`, err); + return null; + } + console.log( + `dedupe: reclaiming stale ${topic} reservation for ${shop} ` + + `(webhookId=${webhookId}, age=${Math.round(age / 1000)}s)`, + ); + return reservation; + } + + // A fresh "processing" row: another delivery is actively working on it. + // Skip this concurrent delivery. Shopify will retry; if the active worker + // fails it releases the reservation so a later retry reprocesses. + console.log( + `dedupe: ${topic} for ${shop} already in-flight (webhookId=${webhookId}); ` + + `skipping concurrent delivery`, + ); + return null; } diff --git a/app/shopify.server.ts b/app/shopify.server.ts index 20d8a42..9a40845 100644 --- a/app/shopify.server.ts +++ b/app/shopify.server.ts @@ -4,17 +4,18 @@ import { AppDistribution, shopifyApp, } from "@shopify/shopify-app-react-router/server"; -import { PrismaSessionStorage } from "@shopify/shopify-app-session-storage-prisma"; import prisma from "./db.server"; +import { requireEnv } from "./services/config/env.server"; +import { EncryptedPrismaSessionStorage } from "./services/session/encryptedSessionStorage.server"; const shopify = shopifyApp({ - apiKey: process.env.SHOPIFY_API_KEY, - apiSecretKey: process.env.SHOPIFY_API_SECRET || "", + apiKey: requireEnv("SHOPIFY_API_KEY"), + apiSecretKey: requireEnv("SHOPIFY_API_SECRET"), apiVersion: ApiVersion.October25, scopes: process.env.SCOPES?.split(","), - appUrl: process.env.SHOPIFY_APP_URL || "", + appUrl: requireEnv("SHOPIFY_APP_URL"), authPathPrefix: "/auth", - sessionStorage: new PrismaSessionStorage(prisma), + sessionStorage: new EncryptedPrismaSessionStorage(prisma), distribution: AppDistribution.SingleMerchant, future: { expiringOfflineAccessTokens: true, diff --git a/deploy/.env.dev.example b/deploy/.env.dev.example index a9fe7f3..4da6e0c 100644 --- a/deploy/.env.dev.example +++ b/deploy/.env.dev.example @@ -16,8 +16,24 @@ ALLOWED_SHOP=linumiq-dev.myshopify.com # Must match `scopes` in shopify.app.dev.toml. SCOPES=read_orders,write_orders,read_all_orders,read_customers,read_companies,read_files,write_files +# --- Secrets at rest --- +# Field-level encryption key for secrets stored in the DB (SMTP password, +# Shopify session access/refresh tokens). Must be base64 of exactly 32 bytes. +# Generate with: node -e "console.log(require('crypto').randomBytes(32).toString('base64'))" +DATA_ENCRYPTION_KEY=REPLACE_ME_BASE64_32_BYTES + +# Dedicated HMAC key for signing public GiroCode URLs. base64 of 32 bytes. +# If unset, the app falls back to SHOPIFY_API_SECRET (kept for backward compat). +# Generate with: node -e "console.log(require('crypto').randomBytes(32).toString('base64'))" +GIROCODE_SIGNING_KEY=REPLACE_ME_BASE64_32_BYTES + # --- Runtime --- NODE_ENV=production PORT=3000 # DATABASE_URL is set in docker-compose.dev.yml (file:/data/prod.sqlite on the bind mount). + +# --- Email (optional) --- +# Archival BCC for every invoice email. Off by default for privacy/GDPR. +# Set to a single address or a comma-separated list to opt in. +# INVOICE_BCC=archive@example.com diff --git a/deploy/Caddyfile.snippet b/deploy/Caddyfile.snippet index 5ac4947..f06f249 100644 --- a/deploy/Caddyfile.snippet +++ b/deploy/Caddyfile.snippet @@ -8,6 +8,15 @@ # DEV — installed on linumiq-dev.myshopify.com invoice-app-dev.linumiq.com { encode zstd gzip + # Security response headers. NOTE: deliberately no X-Frame-Options here — + # this is an embedded Shopify app, and framing is governed by the + # Content-Security-Policy `frame-ancestors` directive that the Shopify + # library injects via addDocumentResponseHeaders (see app/entry.server.tsx). + header { + Strict-Transport-Security "max-age=31536000; includeSubDomains" + X-Content-Type-Options nosniff + Referrer-Policy strict-origin-when-cross-origin + } reverse_proxy linumiq-invoice-dev:3000 } diff --git a/deploy/README.md b/deploy/README.md index 7fceaf5..0bd647f 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -48,6 +48,25 @@ docker compose up -d --build Append `Caddyfile.snippet` to your Caddy config and `docker exec caddy caddy reload --config /etc/caddy/Caddyfile`. +## Container runs as a non-root user (uid 1000) + +The image runs as the unprivileged `node` user (uid/gid **1000**), not root. The +SQLite database is written to the `/data` bind mount, so the **host** directory +mounted at `/data` (e.g. `/docker/linumiq-invoice/dev/data` and +`…/prod/data`) must be writable by uid 1000, otherwise `prisma migrate deploy` +and DB writes fail on startup: + +```bash +sudo chown -R 1000:1000 /docker/linumiq-invoice/dev/data +sudo chown -R 1000:1000 /docker/linumiq-invoice/prod/data +``` + +The dev container additionally runs with a **read-only root filesystem** +(`read_only: true` + `tmpfs: /tmp`), `no-new-privileges`, all Linux capabilities +dropped, and memory/pids/cpu limits. The app only writes to the `/data` bind +mount and the tmpfs `/tmp`, so this is safe. (The prod compose is intentionally +left unchanged.) + ## Day-to-day redeploy ```bash diff --git a/deploy/docker-compose.dev.yml b/deploy/docker-compose.dev.yml index 2a6cd50..987d6d0 100644 --- a/deploy/docker-compose.dev.yml +++ b/deploy/docker-compose.dev.yml @@ -6,6 +6,24 @@ services: image: linumiq-invoice:dev container_name: linumiq-invoice-dev restart: unless-stopped + # --- Container hardening (DEV) --------------------------------------- + # Prevent privilege escalation and drop all Linux capabilities (the app + # is a plain Node HTTP server — it needs none). + security_opt: + - "no-new-privileges:true" + cap_drop: + - ALL + # Read-only root filesystem: the app never writes to the image at runtime + # (Prisma client is baked at build; the SQLite DB lives on the /data bind + # mount; logo/image caches live in the DB or in-memory). npm/Prisma + # incidental writes are redirected to the tmpfs /tmp (see Dockerfile env). + read_only: true + tmpfs: + - /tmp + # Resource limits (Compose v2 / docker compose, non-swarm). + mem_limit: 512m + pids_limit: 256 + cpus: 1.5 env_file: - .env.dev environment: diff --git a/package-lock.json b/package-lock.json index 3282f47..eb80d02 100644 --- a/package-lock.json +++ b/package-lock.json @@ -29,9 +29,12 @@ "@types/nodemailer": "^8.0.0", "compression": "^1.8.1", "express": "^4.22.1", + "express-rate-limit": "^8.5.2", + "ipaddr.js": "^2.4.0", "isbot": "^5.1.31", "morgan": "^1.10.1", "nodemailer": "^8.0.7", + "p-limit": "^3.1.0", "prisma": "^6.16.3", "qrcode": "^1.5.4", "react": "^18.3.1", @@ -5939,21 +5942,6 @@ "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==", "license": "MIT" }, - "node_modules/body-parser/node_modules/qs": { - "version": "6.15.1", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.15.1.tgz", - "integrity": "sha512-6YHEFRL9mfgcAvql/XhwTvf5jKcOiiupt2FiJxHkiX1z4j7WL8J/jRHYLluORvc1XxB5rV20KoeK00gVJamspg==", - "license": "BSD-3-Clause", - "dependencies": { - "side-channel": "^1.1.0" - }, - "engines": { - "node": ">=0.6" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/brace-expansion": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.0.tgz", @@ -7864,14 +7852,14 @@ } }, "node_modules/express": { - "version": "4.22.1", - "resolved": "https://registry.npmjs.org/express/-/express-4.22.1.tgz", - "integrity": "sha512-F2X8g9P1X7uCPZMA3MVf9wcTqlyNp7IhH5qPCI0izhaOIYXaW9L535tGA3qmjRzpH+bZczqq7hVKxTR4NWnu+g==", + "version": "4.22.2", + "resolved": "https://registry.npmjs.org/express/-/express-4.22.2.tgz", + "integrity": "sha512-IuL+Elrou2ZvCFHs18/CIzy2Nzvo25nZ1/D2eIZlz7c+QUayAcYoiM2BthCjs+EBHVpjYjcuLDAiCWgeIX3X1Q==", "license": "MIT", "dependencies": { "accepts": "~1.3.8", "array-flatten": "1.1.1", - "body-parser": "~1.20.3", + "body-parser": "~1.20.5", "content-disposition": "~0.5.4", "content-type": "~1.0.4", "cookie": "~0.7.1", @@ -7890,7 +7878,7 @@ "parseurl": "~1.3.3", "path-to-regexp": "~0.1.12", "proxy-addr": "~2.0.7", - "qs": "~6.14.0", + "qs": "~6.15.1", "range-parser": "~1.2.1", "safe-buffer": "5.2.1", "send": "~0.19.0", @@ -7909,6 +7897,24 @@ "url": "https://opencollective.com/express" } }, + "node_modules/express-rate-limit": { + "version": "8.5.2", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-8.5.2.tgz", + "integrity": "sha512-5Kb34ipNX694DH48vN9irak1Qx30nb0PLYHXfJgw4YEjiC3ZEmZJhwOp+VfiCYwFzvFTdB9QkArYS5kXa2cx2A==", + "license": "MIT", + "dependencies": { + "ip-address": "^10.2.0" + }, + "engines": { + "node": ">= 16" + }, + "funding": { + "url": "https://github.com/sponsors/express-rate-limit" + }, + "peerDependencies": { + "express": ">= 4.11" + } + }, "node_modules/express/node_modules/debug": { "version": "2.6.9", "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", @@ -8842,9 +8848,9 @@ } }, "node_modules/graphql-config/node_modules/brace-expansion": { - "version": "5.0.5", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.5.tgz", - "integrity": "sha512-VZznLgtwhn+Mact9tfiwx64fA9erHH/MCXEUfB/0bX/6Fz6ny5EGTXYltMocqg4xFAQZtnO3DHWWXi8RiuN7cQ==", + "version": "5.0.6", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", + "integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==", "dev": true, "license": "MIT", "dependencies": { @@ -9300,13 +9306,22 @@ "resolved": "extensions/invoice-thank-you-payment", "link": true }, - "node_modules/ipaddr.js": { - "version": "1.9.1", - "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz", - "integrity": "sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==", + "node_modules/ip-address": { + "version": "10.2.0", + "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.2.0.tgz", + "integrity": "sha512-/+S6j4E9AHvW9SWMSEY9Xfy66O5PWvVEJ08O0y5JGyEKQpojb0K0GKpz/v5HJ/G0vi3D2sjGK78119oXZeE0qA==", "license": "MIT", "engines": { - "node": ">= 0.10" + "node": ">= 12" + } + }, + "node_modules/ipaddr.js": { + "version": "2.4.0", + "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-2.4.0.tgz", + "integrity": "sha512-9VGk3HGanVE6JoZXHiCpnGy5X0jYDnN4EA4lntFPj+1vIWlFhIylq2CrrCOJH9EAhc5CYhq18F2Av2tgoAPsYQ==", + "license": "MIT", + "engines": { + "node": ">= 10" } }, "node_modules/is-absolute": { @@ -10983,7 +10998,6 @@ "version": "3.1.0", "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", "integrity": "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ==", - "dev": true, "license": "MIT", "dependencies": { "yocto-queue": "^0.1.0" @@ -11540,6 +11554,15 @@ "node": ">= 0.10" } }, + "node_modules/proxy-addr/node_modules/ipaddr.js": { + "version": "1.9.1", + "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz", + "integrity": "sha512-0KI/607xoxSToH7GjN1FfSbLoU0+btTicjsQSWQlh/hZykN8KpmMf7uYwPW3R+akZ6R/w18ZlXSHBYXiYUPO3g==", + "license": "MIT", + "engines": { + "node": ">= 0.10" + } + }, "node_modules/punycode": { "version": "2.3.1", "resolved": "https://registry.npmjs.org/punycode/-/punycode-2.3.1.tgz", @@ -11688,9 +11711,9 @@ } }, "node_modules/qs": { - "version": "6.14.2", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.14.2.tgz", - "integrity": "sha512-V/yCWTTF7VJ9hIh18Ugr2zhJMP01MY7c5kh4J870L7imm6/DIzBsNLTXzMwUA3yZ5b/KBqLx8Kp3uRvd7xSe3Q==", + "version": "6.15.2", + "resolved": "https://registry.npmjs.org/qs/-/qs-6.15.2.tgz", + "integrity": "sha512-Rzq0KEyX/w/tEybncDgdkZrJgVUsUMk3xjh3t5bv3S1HTAtg+uOYt72+ZfwiQwKdysThkTBdL/rTi6HDmX9Ddw==", "license": "BSD-3-Clause", "dependencies": { "side-channel": "^1.1.0" @@ -14225,9 +14248,9 @@ "license": "ISC" }, "node_modules/ws": { - "version": "8.20.0", - "resolved": "https://registry.npmjs.org/ws/-/ws-8.20.0.tgz", - "integrity": "sha512-sAt8BhgNbzCtgGbt2OxmpuryO63ZoDk/sqaB/znQm94T4fCEsy/yV+7CdC1kJhOU9lboAEU7R3kquuycDoibVA==", + "version": "8.21.0", + "resolved": "https://registry.npmjs.org/ws/-/ws-8.21.0.tgz", + "integrity": "sha512-Vsp28b7DRcimFQvrqu2Wek3z1iYxDCWqHYB8Qsnk/S4RfaCQzPGPyBNuVjJV3cd6UiKtUtp6sNM77gWvzcCH+g==", "dev": true, "license": "MIT", "engines": { @@ -14318,7 +14341,6 @@ "version": "0.1.0", "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==", - "dev": true, "license": "MIT", "engines": { "node": ">=10" diff --git a/package.json b/package.json index edb3d05..1abef2a 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "config:use": "shopify app config use", "env": "shopify app env", "start": "node ./server.js", - "docker-start": "npm run setup && npm run start", + "docker-start": "prisma migrate deploy && npm run start", "setup": "prisma generate && prisma migrate deploy", "lint": "eslint --ignore-path .gitignore --cache --cache-location ./node_modules/.cache/eslint .", "shopify": "shopify", @@ -28,13 +28,10 @@ "@prisma/client": "^6.16.3", "@react-pdf/renderer": "^4.5.1", "@react-router/dev": "^7.12.0", + "@react-router/express": "^7.14.2", "@react-router/fs-routes": "^7.12.0", "@react-router/node": "^7.12.0", - "@react-router/express": "^7.14.2", "@react-router/serve": "^7.12.0", - "compression": "^1.8.1", - "express": "^4.22.1", - "morgan": "^1.10.1", "@shopify/app-bridge-react": "^4.2.4", "@shopify/shopify-app-react-router": "^1.1.0", "@shopify/shopify-app-session-storage-prisma": "^8.0.0", @@ -46,8 +43,14 @@ "@tiptap/react": "^3.23.1", "@tiptap/starter-kit": "^3.23.1", "@types/nodemailer": "^8.0.0", + "compression": "^1.8.1", + "express": "^4.22.1", + "express-rate-limit": "^8.5.2", + "ipaddr.js": "^2.4.0", "isbot": "^5.1.31", + "morgan": "^1.10.1", "nodemailer": "^8.0.7", + "p-limit": "^3.1.0", "prisma": "^6.16.3", "qrcode": "^1.5.4", "react": "^18.3.1", diff --git a/prisma/migrations/20260530205507_add_processed_webhook_status/migration.sql b/prisma/migrations/20260530205507_add_processed_webhook_status/migration.sql new file mode 100644 index 0000000..e9ca390 --- /dev/null +++ b/prisma/migrations/20260530205507_add_processed_webhook_status/migration.sql @@ -0,0 +1,16 @@ +-- RedefineTables +PRAGMA defer_foreign_keys=ON; +PRAGMA foreign_keys=OFF; +CREATE TABLE "new_ProcessedWebhook" ( + "webhookId" TEXT NOT NULL PRIMARY KEY, + "topic" TEXT NOT NULL, + "shopDomain" TEXT NOT NULL, + "status" TEXT NOT NULL DEFAULT 'done', + "receivedAt" DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP +); +INSERT INTO "new_ProcessedWebhook" ("receivedAt", "shopDomain", "topic", "webhookId") SELECT "receivedAt", "shopDomain", "topic", "webhookId" FROM "ProcessedWebhook"; +DROP TABLE "ProcessedWebhook"; +ALTER TABLE "new_ProcessedWebhook" RENAME TO "ProcessedWebhook"; +CREATE INDEX "ProcessedWebhook_shopDomain_topic_idx" ON "ProcessedWebhook"("shopDomain", "topic"); +PRAGMA foreign_keys=ON; +PRAGMA defer_foreign_keys=OFF; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 8ca456d..5f77eed 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -188,6 +188,13 @@ model ProcessedWebhook { webhookId String @id topic String shopDomain String + // Reserve/commit lifecycle for at-least-once side-effect processing: + // "processing" — reserved, side-effect work in flight (acts as the lock) + // "done" — work completed successfully; future retries are dropped + // A "processing" row older than the stale lease (see dedupe.server.ts) is + // treated as a crashed reservation and may be reclaimed. Existing rows + // migrated from the old (record-before-work) design default to "done". + status String @default("done") receivedAt DateTime @default(now()) @@index([shopDomain, topic]) diff --git a/server.js b/server.js index 3ff8d74..4027afe 100644 --- a/server.js +++ b/server.js @@ -15,6 +15,7 @@ import { createRequestHandler } from "@react-router/express"; import compression from "compression"; import express from "express"; import morgan from "morgan"; +import rateLimit from "express-rate-limit"; // --------------------------------------------------------------------------- // Console timestamps — patch BEFORE anything else logs. @@ -33,6 +34,11 @@ const build = buildModule.default ?? buildModule; const app = express(); app.disable("x-powered-by"); +// The app runs behind a single reverse proxy (Caddy) that sets +// X-Forwarded-For. Trust ONLY the first proxy hop so req.ip reflects the real +// client IP for rate limiting, without trusting arbitrary client-supplied +// forwarding headers (which `trust proxy: true` would). +app.set("trust proxy", 1); app.use(compression()); // Static assets emitted by the React Router build. @@ -46,15 +52,43 @@ app.use(express.static("public", { maxAge: "1h" })); // Access log: ISO timestamp + standard request info; suppress healthy // /healthz polls so real traffic stays visible. morgan.token("isotime", () => new Date().toISOString()); +// Redacted URL: strip sensitive query parameters (the GiroCode HMAC `sig` +// and any `token`) from the logged URL so signed-URL secrets / bearer tokens +// never land in access logs. Other query params are preserved. +morgan.token("safeurl", (req) => { + const raw = req.originalUrl || req.url || ""; + const qIdx = raw.indexOf("?"); + if (qIdx === -1) return raw; + const path = raw.slice(0, qIdx); + const params = new URLSearchParams(raw.slice(qIdx + 1)); + for (const key of ["sig", "token"]) { + if (params.has(key)) params.set(key, "REDACTED"); + } + const qs = params.toString(); + return qs ? `${path}?${qs}` : path; +}); app.use( morgan( - ":isotime :method :url :status :res[content-length] - :response-time ms", + ":isotime :method :safeurl :status :res[content-length] - :response-time ms", { skip: (req, res) => req.url === "/healthz" && res.statusCode < 400, }, ), ); +// Per-IP rate limiting for the public, unauthenticated-at-the-edge API +// surface only (/api/public/*). Shopify webhooks (/webhooks/*) and the +// embedded admin are intentionally NOT rate limited here — webhooks can burst +// legitimately and are already HMAC-verified. +const publicApiLimiter = rateLimit({ + windowMs: 60 * 1000, // 1 minute + limit: 60, // 60 requests / minute / IP + standardHeaders: true, // RateLimit-* headers + legacyHeaders: false, + message: { error: "rate-limited" }, +}); +app.use("/api/public", publicApiLimiter); + app.all( "*", createRequestHandler({ build, mode: process.env.NODE_ENV }), @@ -71,11 +105,22 @@ const server = HOST : app.listen(PORT, onListen); for (const signal of ["SIGTERM", "SIGINT"]) { - process.once(signal, () => { + process.once(signal, async () => { console.log(`[server] received ${signal}, shutting down`); + // Stop accepting new connections. server.close((err) => { if (err) console.error("[server] close error:", err); - process.exit(0); }); + // Drain in-flight background webhook work (PDF render / SMTP send) before + // exiting so a container stop doesn't lose invoice work mid-send. The + // background queue exposes this bridge because server.js loads only the + // bundled build and can't import the module directly. + try { + const drain = globalThis.__linumiqWebhookDrain; + if (typeof drain === "function") await drain(); + } catch (err) { + console.error("[server] webhook drain error:", err); + } + process.exit(0); }); } diff --git a/tests/dedupe-and-language.test.ts b/tests/dedupe-and-language.test.ts index a10409a..5efbfb9 100644 --- a/tests/dedupe-and-language.test.ts +++ b/tests/dedupe-and-language.test.ts @@ -2,7 +2,7 @@ import { strict as assert } from "node:assert"; import { describe, it } from "node:test"; import { pickLanguage } from "../app/services/invoice/i18n"; -import { isDuplicateWebhook, type DedupeDeps } from "../app/services/webhooks/dedupe.server"; +import { reserveWebhook, type DedupeDeps } from "../app/services/webhooks/dedupe.server"; describe("pickLanguage", () => { it("returns 'de' only for explicit German locales", () => { @@ -38,8 +38,21 @@ function makeRequest(headers: Record = {}): Request { }); } -function makeDeps(behaviour: "ok" | "p2002" | "boom"): DedupeDeps { +type ExistingRow = { webhookId: string; status: string; receivedAt: Date } | null; + +/** + * Build a DedupeDeps stub. + * - "ok" : create() succeeds (fresh reservation). + * - "p2002" : create() conflicts; findUnique() returns `existing`. + * - "boom" : create() throws a non-P2002 error (fail-open). + */ +function makeDeps( + behaviour: "ok" | "p2002" | "boom", + existing: ExistingRow = null, +): DedupeDeps & { calls: { commit: number; release: number; update: number } } { + const calls = { commit: 0, release: 0, update: 0 }; return { + calls, db: { processedWebhook: { create: async () => { @@ -51,29 +64,82 @@ function makeDeps(behaviour: "ok" | "p2002" | "boom"): DedupeDeps { } throw new Error("DB unavailable"); }, + findUnique: async () => existing, + update: async () => { + calls.update += 1; + calls.commit += 1; + return {}; + }, + delete: async () => { + calls.release += 1; + return {}; + }, }, }, }; } -describe("isDuplicateWebhook", () => { - it("returns false on first delivery (insert succeeds)", async () => { +describe("reserveWebhook", () => { + it("returns a reservation on first delivery (insert succeeds)", async () => { const req = makeRequest({ "x-shopify-webhook-id": "abc-123" }); - assert.equal(await isDuplicateWebhook(req, "shop.myshopify.com", "ORDERS_CREATE", makeDeps("ok")), false); + const res = await reserveWebhook(req, "shop.myshopify.com", "ORDERS_CREATE", makeDeps("ok")); + assert.ok(res, "expected a reservation"); + assert.equal(res!.webhookId, "abc-123"); }); - it("returns true on a retried delivery (P2002)", async () => { + it("returns null for an already-processed (done) delivery", async () => { const req = makeRequest({ "x-shopify-webhook-id": "abc-123" }); - assert.equal(await isDuplicateWebhook(req, "shop.myshopify.com", "ORDERS_CREATE", makeDeps("p2002")), true); + const deps = makeDeps("p2002", { + webhookId: "abc-123", + status: "done", + receivedAt: new Date(), + }); + assert.equal(await reserveWebhook(req, "shop.myshopify.com", "ORDERS_CREATE", deps), null); }); - it("returns false (and proceeds) when the dedupe table itself errors \u2014 fail-open, never drop a webhook silently", async () => { + it("returns null for a fresh in-flight (processing) delivery", async () => { const req = makeRequest({ "x-shopify-webhook-id": "abc-123" }); - assert.equal(await isDuplicateWebhook(req, "shop.myshopify.com", "ORDERS_CREATE", makeDeps("boom")), false); + const deps = makeDeps("p2002", { + webhookId: "abc-123", + status: "processing", + receivedAt: new Date(), // fresh lease + }); + assert.equal(await reserveWebhook(req, "shop.myshopify.com", "ORDERS_CREATE", deps), null); }); - it("returns false when the X-Shopify-Webhook-Id header is missing (test harness / non-Shopify caller)", async () => { + it("reclaims a stale (crashed) processing reservation", async () => { + const req = makeRequest({ "x-shopify-webhook-id": "abc-123" }); + const deps = makeDeps("p2002", { + webhookId: "abc-123", + status: "processing", + receivedAt: new Date(Date.now() - 10 * 60 * 1000), // 10 min ago > 5 min lease + }); + const res = await reserveWebhook(req, "shop.myshopify.com", "ORDERS_CREATE", deps); + assert.ok(res, "expected to reclaim the stale reservation"); + assert.equal(deps.calls.update, 1, "stale reclaim should renew the lease via update()"); + }); + + it("commit() flips the row to done; release() deletes it", async () => { + const req = makeRequest({ "x-shopify-webhook-id": "abc-123" }); + const deps = makeDeps("ok"); + const res = await reserveWebhook(req, "shop.myshopify.com", "ORDERS_CREATE", deps); + await res!.commit(); + assert.equal(deps.calls.commit, 1); + await res!.release(); + assert.equal(deps.calls.release, 1); + }); + + it("fails open (returns a no-op reservation) when the dedupe table errors", async () => { + const req = makeRequest({ "x-shopify-webhook-id": "abc-123" }); + const res = await reserveWebhook(req, "shop.myshopify.com", "ORDERS_CREATE", makeDeps("boom")); + assert.ok(res, "fail-open must still process — never silently drop a webhook"); + assert.equal(res!.webhookId, "abc-123"); + }); + + it("returns a no-op reservation when the X-Shopify-Webhook-Id header is missing", async () => { const req = makeRequest(); - assert.equal(await isDuplicateWebhook(req, "shop.myshopify.com", "ORDERS_CREATE", makeDeps("ok")), false); + const res = await reserveWebhook(req, "shop.myshopify.com", "ORDERS_CREATE", makeDeps("ok")); + assert.ok(res, "missing id => process without dedupe"); + assert.equal(res!.webhookId, null); }); });