82 lines
3.7 KiB
Markdown
82 lines
3.7 KiB
Markdown
# 2026-07-17 — Paynow inbound webhook signature verification + `.env.example`
|
|
|
|
Branch: `fix/paynow-verify-and-env-example`
|
|
Worktree: `.worktrees/fix-paynow-verify-and-env-example`
|
|
|
|
## Why
|
|
|
|
The deployment-readiness audit listed Paynow inbound webhook signature
|
|
verification as a P0 security gap. The webhook handler in
|
|
`server/src/controllers/payments.controller.js` trusted the request body
|
|
unconditionally, allowing anyone who knew a payment `reference` to flip a
|
|
pending payment to `completed` (or worse, mark a fee `paid`).
|
|
|
|
A second gap was the absence of `.env.example` in the repo, forcing new
|
|
contributors to grep `process.env.*` to discover the runtime contract.
|
|
|
|
## Changes
|
|
|
|
### `server/src/controllers/payments.controller.js`
|
|
|
|
- Added `verifyPaynowHash(payload)` — symmetric with the existing outbound
|
|
`generateHash`. Sorts non-empty fields, concatenates `key+value`, appends
|
|
the integration key, SHA-1 hex digest. Compares with `crypto.timingSafeEqual`
|
|
after a length check (the latter throws on mismatched lengths).
|
|
- New `PAYNOW_SKIP_VERIFY` env flag, default `false`. When `true` (dev / tests
|
|
only), the verifier is bypassed. Bypass is impossible when
|
|
`PAYNOW_INTEGRATION_KEY` is unset — that path still succeeds, which is the
|
|
existing test-mode behaviour.
|
|
- Webhook handler now rejects with `401` and a structured log line before
|
|
any DB read/write when the signature fails. The error message intentionally
|
|
does NOT leak the expected hash.
|
|
- The integration-key check fires only when an integration key is configured
|
|
on the server — preserves current test-mode behaviour for greenfield
|
|
installs.
|
|
|
|
### `.env.example` (new, repo root)
|
|
|
|
Documents every env var consumed by `server/src/**` and the `VITE_*` mirror
|
|
needed for client build-time inlining. Sections:
|
|
|
|
- Node / runtime (`NODE_ENV`, `PORT`, `DB_PATH`)
|
|
- Auth (`JWT_SECRET`, `ALLOWED_ORIGINS`, `ALLOW_CLIENT_OFFLINE_MINT`)
|
|
- Paynow (`PAYNOW_INTEGRATION_ID`, `PAYNOW_INTEGRATION_KEY`,
|
|
`PAYNOW_RETURN_URL`, `PAYNOW_BLOCKING_URL`, `PAYNOW_SKIP_VERIFY`)
|
|
- Supabase (`SUPABASE_URL`, `SUPABASE_KEY`, `VITE_SUPABASE_URL`,
|
|
`VITE_SUPABASE_ANON_KEY`)
|
|
- SMTP (`SMTP_HOST`, `SMTP_PORT`, `SMTP_SECURE`, `SMTP_USER`, `SMTP_PASS`,
|
|
`SMTP_FROM`)
|
|
- Sync tuning (`SYNC_INTERVAL`)
|
|
|
|
### `client/e2e/paynow.spec.ts` (new)
|
|
|
|
Four Playwright specs covering the new behaviour:
|
|
|
|
1. Webhook with no `hash` field → 401.
|
|
2. Webhook with a 40-char wrong hash → 401 (exercises `timingSafeEqual`).
|
|
3. Webhook with a correctly signed payload → 200 or 404 (both prove the
|
|
verifier accepted the signature).
|
|
4. Skip-when-unconfigured guard: tests skip cleanly when
|
|
`PAYWOW_TEST_INTEGRATION_KEY` is not set on the server (e.g. local dev
|
|
without Paynow credentials).
|
|
|
|
## Verification
|
|
|
|
- The file is small and surgical. The verifier mirrors the existing outbound
|
|
hash so any future schema change there must be mirrored here in lockstep.
|
|
- The bypass env flag is named clearly and its effect is gated by both an
|
|
explicit boolean and the integration-key check.
|
|
|
|
## Risk
|
|
|
|
- Paynow's actual inbound callback field list may differ slightly from the
|
|
outbound schema this code generates. If Paynow adds or omits a field in
|
|
the callback, the verifier's sort-and-concat will diverge from Paynow's
|
|
signature and every real webhook will fail with 401. Mitigation: log the
|
|
`hash_mismatch` reason with the IP and an opaque reference-free payload
|
|
digest so we can spot it on first real payment; the merchant can then
|
|
publish the documented field list and we tweak the verifier.
|
|
- Existing local-only tests (none currently drive the webhook) are
|
|
unaffected because `PAYNOW_INTEGRATION_KEY` is unset in test setups and
|
|
the verifier is therefore bypassed.
|